Allow custom environment variables in assets to make use of downloaded asset path

nixwiz commented 4 years ago

To allow the possibility of other scripting runtimes or functionality it would be helpful to surface additional/custom environment variables for downloaded assets.

Feature Suggestion

There should be a way to have an asset create (or append to) custom environment variables to be surfaced when that asset is run. This could be part of the asset definition itself.

Possible Implementation

Adding env_vars to the asset definition, where each key is the env variable and the value is a path relative to the PATH_TO_ASSET to populate into the variable.

Something along the lines of the following where in this case the PERL5LIB environment variable will include the appropriate PATH_TO_ASSET/lib directory when executed (e.g. PERL5LIB=/var/cache/sensu/sensu-agent/sha512sumvalue/lib).

  "type": "Asset",
  "api_version": "core/v2",
  "metadata": {
    "name": "perl-runtime",
    "namespace": "default",
    "labels": {
      "origin": "bonsai"
    }
  },
  "spec": {
    "env_vars":  {
      "PERL5LIB": "lib"
      },
     [remainder deleted for brevity]

Context

calebhailey commented 4 years ago

Love this! Great idea.

calebhailey commented 4 years ago

nikkictl commented 4 years ago

Required by https://github.com/sensu/sensu-go/issues/3619

Feature spec:

[ ] add custom environment variables to the asset resource
[ ] for types that have a single value only, last one wins in the event of a conflict
[ ] log overwrites, debug level
[ ] do not allow built-in asset env vars to be overridden e.g. LD_LIBRARY_PATH, PATH, CPATH etc. (this can be done in the Validate() function of the asset)
[ ] do not auto-namespace env vars (note to document that the onus is on asset creators to make their global variables unique by whatever means necessary, ex. prefix)
[ ] identify if any changes are required in sensuctl due to the bonsai asset system

portertech commented 4 years ago

@palourde to check with @nixwiz if we've already addressed the user stories with the PR for https://github.com/sensu/sensu-go/issues/3662.

palourde commented 4 years ago

@nixwiz So I'm wondering if this issue is still relevant, now that resources that consume assets can retrieve the path of these assets via a token substitution and/or an environment variable. WDYT?

For reference: https://github.com/sensu/sensu-docs/issues/2483

nixwiz commented 4 years ago

Unless I can do something like this:

  "type": "Asset",
  "api_version": "core/v2",
  "metadata": {
    "name": "perl-runtime",
    "namespace": "default",
    "labels": {
      "origin": "bonsai"
    }
  },
  "spec": {
    "env_vars":  {
      "PERL5LIB": "{{assetPath}}/lib"
      },

Then the changes implemented don't exactly address the use case I specified. If I want to surface a custom env variable based on the asset path, I either need to write a wrapper script around my asset or otherwise incorporate its retrieval in my asset.

jspaleta commented 3 years ago

Update on this.

So i'm prototyping a new python runtime asset to aid with users who want to use python based checks

I'm running into a similar need. To make this work I need to manipulate the PYTHONPATH variable and extend it with additional paths relative to each asset's install directory. The workaround right now is to write wrapper scripts for each python check command. Being able to do the envvar manipulation as part of the asset definition would simplify things greatly.

Straw User Scenario for python asset building

User need:

User wants to use in-house python check that needs to use python module not included in python runtime

Assets used in check

sensu-python-runtime -> base runtime for python python-inhouse-check -> in-house asset with python check and needed python modules

steps to build python-inhouse-check asset currently

User populates asset lib/ using pip --target to install missing python modules
place python check scripts in asset libexec/ which is not in executable path
place wrapper script in bin/ for each python check script, wrapper extends PYTHONPATH envvar to include assets lib/

If we had a way to extend PYTHONPATH as part of asset definition, python assets wouldn't need to do the complicated wrapper in bin/

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

sensu / sensu-go