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

adapter: context.env should not contain process.env #17

Open
tripodsan opened this issue Dec 31, 2020 · 5 comments
Open

adapter: context.env should not contain process.env #17

tripodsan opened this issue Dec 31, 2020 · 5 comments

Comments

@tripodsan
Copy link
Contributor

having the process.env variables mixed into the context.env object is kind of redundant and might be confusing.
context.env should only contain the information that is specific to the deployed environment.
an action can still use process.env explicitely.

@trieloff
Copy link
Contributor

trieloff commented Jan 3, 2021

Most environments (except OW) use process.env for user-config, so I merged the two. I wouldn't want to have a check in my function code that switches between process.env and context.env depending on the runtime I'm using.

@tripodsan
Copy link
Contributor Author

but having a distinction between environment variables, like API_HOST etc, and the env provided by the secret is useful. no?

@trieloff
Copy link
Contributor

trieloff commented Jan 5, 2021

The biggest advantage would probably be collision prevention, but I think that's quite theoretical.

We would need a (for each runtime) an up-to-date list of runtime-provided environment variables, so that we can disentangle runtime-provided vars from user-provided vars – this could probably be done by deploying an action that echos the context.env without any config, then run an in-container integration test ensuring that the returned object has no keys.

In reality I think only wrapper code (like invoke or version-lock or log setup) should interact with process.env and user code should not be runtime-specific and should neither try to access process.env nor attempt to treat API_HOST as something meaningful (unless it has been configured by the user, which brings us back to collision prevention)

@tripodsan
Copy link
Contributor Author

but for the actions, only the user-provided vars are important:

  • for openwhisk, the package-params / secrets
  • for aws, the params from the store
  • for azue, the params from the params.json

I agree, the actions should never use process.env

@trieloff
Copy link
Contributor

trieloff commented Jan 6, 2021

So yes, cleaning context.env would be a good, but tedious thing to do.

@tripodsan tripodsan transferred this issue from adobe/helix-deploy May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants