Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asset paths should be available in the context of commands #3662

Closed
cwjohnston opened this issue Apr 1, 2020 · 8 comments · Fixed by #3740
Closed

Asset paths should be available in the context of commands #3662

cwjohnston opened this issue Apr 1, 2020 · 8 comments · Fixed by #3740
Assignees
Labels
Milestone

Comments

@cwjohnston
Copy link
Contributor

Feature Suggestion

Resources which invoke commands (e.g. checks, pipe handlers, mutators, hooks) should have access to the full path(s) for assets attached to the resource.

Possible Implementation

Expose asset paths as environment variables in the command execution context. For example, given this check:

---
type: CheckConfig
api_version: core/v2
metadata:
namespace: default
name: win-cpu-check
spec:
command: powershell.exe -ExecutionPolicy ByPass -f %ASSET_PATH%\bin\check-windows-cpu-load.ps1 90
95
subscriptions:
- windows
handlers:
- slack
- email
runtime_assets:
- sensu-plugins-windows
interval: 10
publish: true

The resulting command execution would have access to the file path for the named runtime asset as %ASSET_PATH%.

For checks with multiple assets, perhaps we could iteratively create incrementing variables like ASSET_PATH_1, ASSET_PATH_2 or similar?

Context

Sensu reports unexpected check status when executing PowerShell plugins, unless the -f or -File flag is used. This flag requires the absolute path to the PowerShell script for execution, making it difficult to use this flag with PowerShell scripts distributed as assets.

This is not a new issue in Sensu Go, it was also a consideration in Sensu Core (see sensu/sensu#1919).

See sensu-plugins/sensu-plugins-windows#101 for examples of how this is affecting Sensu Go users.

@echlebek
Copy link
Contributor

echlebek commented Apr 2, 2020

I could see this working via token substitution.

command: powershell.exe -ExecutionPolicy ByPass -f {{ assetPath "sensu-plugin-windows" }}\bin\check-windows-cpu-load.ps1 90 95

@calebhailey
Copy link

What would be the benefit of exposing this value as a token vs. as an environment variable? Seems the latter would be the more natural fit?

@calebhailey calebhailey added the component:assets Sensu Asset improvements label Apr 5, 2020
@calebhailey
Copy link

Related: #3492

@echlebek
Copy link
Contributor

echlebek commented Apr 6, 2020

The benefit is that by exposing assetPath as a function, we can easily address named assets without having to export multiple environment variables. Additionally, the parameter passed to the assetPath could be identical to the name of the asset, rather than needing to know an implicit mapping to an environment variable based on the name.

func assetPath(asset string) (string, error) {
  // lookup asset
  // return its path, or an error
}

@palourde
Copy link
Contributor

palourde commented Apr 6, 2020

We need to schedule a discussion to expand on Eric's proposal. @nikkixdev will schedule something.

@calebhailey
Copy link

@amdprophet made a really interesting point here that a check can reference multiple assets, so a solution here would need to offer some control over which asset path the user wishes to reference.

@nikkictl
Copy link

nikkictl commented Apr 15, 2020

Feature spec:

  • add a template function to retrieve the asset path via token substitution, ex. SOME_VAR: {{ assetPath "sensu/ruby:1.1.1" }}
  • create a new go template function, assetPath (see defaultFunc as an example)
  • expose the absolute base path of the asset through this function
    • document that it is operator responsibility to append/prepend any other strings, ex. bin
    • the absolute path of the asset depends on the agent configuration fields, so this information will need to be injected somehow for the standalone library to reference

Requires #3639

@palourde
Copy link
Contributor

palourde commented May 5, 2020

Some additional context:

The end goal here is to expose the asset path to resources that consume assets. For resources executed on the agent (checks & hooks), we will expose the agent path directly via a custom function of token substitution. However, since token substitution is not supported yet for backend resources (mutators & handlers), we will have to rely on an additional feature to expose it via environment variables, #3492, which in turns relies on #3639.

Acceptance Criteria

  • A new template function for token substitution can return the path of an asset (e.g. {{ assetPath "sensu/ruby:1.1.1" }}
  • This new template function can be called directly from assets, checks & hooks.

Tasks

  • Create a public function that returns the right asset build based on the provided entity, so we can retrieve the right hash.
    • It also need to support "old" assets that do not use asset builds.
  • Determine how this new template function will retrieve the actual asset path
    • Right now we only provide the resource config (the template) with the entity (the data).
    • The asset might not be installed yet, so we can't rely on the asset manager to know about the entity, since token substitution happens before the asset is downloaded for the first time
    • We could possibly inject it via the entity labels or annotations?
    • We could also modify the token pkg functions so they accept an optional list of interfaces that would be appended to the existing data, so we could pass the asset path & name.
  • Implement asset path for assets
  • Implement asset path for checks
  • Implement asset path for hooks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants