Rename allowed env configuration traits #1137
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request is a partial "redo" of #1000 and attempts to move EG in the right direction, although not completely. It consists of the following changes, some of which require further discussion.
env_whitelistandenv_process_whitelisthave been deprecated in favor ofclient_envsandinherited_envsrespectively. Hopefully, the names are more clear than the previous ones.EDIT - these changes have been reverted per discussion...
3. The use ofenv_whitelist = '*'has been removed.4. The set ofclient_envscan be specified globally, via the EG configuration, but also at the kernelspec level viametadata.client_envs. Kernelspecs retrieved from the EG server will be updated to include BOTH thoseclient_envsdefined in the spec, as well as those configured at the global level.I believe items 3 and 4 require further discussion:
env_whitelist = '*'has been removed.), because the intention of this PR is to (eventually) simplify the gateway client configuration (where it will not require its ownGatewayClient.env_whitelistconfiguration) the model of conveying allowed env variables is essentially changing from a push model (where the client uses its own configuration to determine what envs to include, potentially irrespective of the EG's) to a pull model (where EG advertises the set of allowed envs). As a result, if we allowclient_envs = '*', this would imply that all envs in the client should be included, which we clearly do not want to do as this could "explode" payloads and side-effect the envs that are statically configured in the kernel spec and/or in theinherited_envs.client_envscan be specified globally...), my main concern about sending these in the kernelspec is whether this is too soon. What I don't want to happen is have to change this when proper kernel parameterization is introduced, which will likely need to include the corresponding schema to describe the envs, etc. That said, we could probably adjustGatewayClientto handle this and continue to support them.client_envsmust still be known ahead of the client server's startup so that those envs can be set - although this is really no different than today, except that, today, admins can set their envs, configure theirGatewayClient.env_whitelistvalues and send whatever they have configured only if the EG server is running withenv_whitelist = '*'.client_envsin the kernelspecs, then a similar client-side configuration would be required (like today).env_whitelist, they will still function correctly provided their configuredenv_whitelistis a subset of the EG'sclient_envs.