aio-cli-plugin-runtime
aio-cli-plugin-runtime copied to clipboard
CLI tool is not replacing '$' placeholders with environment variables
Describe the bug CLI tool is not replacing '$' placeholders (indented more than one level) with environment variables.
To Reproduce Example manifest file:
packages:
__APP_PACKAGE__:
version: 1.0.0
license: Adobe-2006
actions:
someAction:
function: dist/someAction
runtime: nodejs:10
web-export: true
annotations:
final: true
inputs:
eventingConnection:
bootstrapServers: $BOOTSTRAP_SERVERS
saslUsername: $SASL_USERNAME
Expected behavior I expect aio cli tool to replace $ placeholders with environment variables regardless of the indentation level.
Additional context An example error:
Error: Error connecting to the host: $BOOTSTRAP_SERVERS failed - getaddrinfo ENOTFOUND
I'm not sure this is valid input.
According to the spec, an action
takes an optional inputs
key, which takes in a list of parameter
s: https://github.com/apache/openwhisk-wskdeploy/blob/master/specification/html/spec_actions.md#actions
parameter
s are specified as: https://github.com/apache/openwhisk-wskdeploy/blob/master/specification/html/spec_parameters.md
in the parameter
s spec above, there is a sub-key called properties
that takes in a list of parameter
s, but that's different than your case.
It's possible that the spec hasn't been updated, but the tests have been (I encountered this for https://github.com/adobe/aio-cli-plugin-runtime/pull/138#issuecomment-609721659).
However, I went through all the test fixtures and none of them had the scenario you had, which makes me believe it is not supported.
does structuring inputs like:
inputs:
bootstrapServers: $BOOTSTRAP_SERVERS
saslUsername: $SASL_USERNAME
work?
@tysonnorris yes this is supported, $VAR being an env variable.
What's not supported, is what the issue mentions, i.e. multi level input objects with env vars
@shazron in that case should we mark this as an enhancement instead of a bug? As the openwhisk api seems to support multi level input objects, it would be nice if we would too?
EDIT: more details: what I mean is that as of now we can set object inputs in the manifest (even if we don't officially support it) but $VAR are not replaced in such objects. So my question is should we support input objects officially in which case I feel we should also support the $VAR?
Since we are currently following the wskdeploy
manifest format spec, adding this enhancement will break compatibility. We need to decide if compatibility is paramount for our manifest, and to weigh that with any future breaking enhancements.
Compatibility is as of today a paramount for our manifest and I'm not in favor of deviating from the original specifications unless there's a very good reason and benefit for our end-users.
I propose then to close the issue for the time being and reevaluate if we see this as a recurring need.
Reopening as @ekremney has some more information to provide
According to the specification, parameter types include object
which is defined as:
The parameter itself is an object with the associated defined Parameters (schemas).
Object type is explained at the bottom of the page
The Object type allows for complex objects to be declared as parameters with an optional validatable schema.
As a developer who uses Openwhisk, I would like to be able to group my parameters so that I can address them differently in my code. It especially comes very handy if you use openwhisk wrappers. Ie:
inputs:
logging:
host: <host>
token: <token>
index: <index>
service:
host: <host>
token: <token>
...
Then you can just initialize your wrappers:
...
new LoggingWrapper(params.logging);
...
FYI, we are migrating from wskdeploy
to aio
. wskdeploy
tool injects ENV variables into nested parameters.
@meryllblanchet @tysonnorris @shazron
Also, wskdeploy
features sample multi-level config and associated test:
multi-level inputs test file: https://github.com/apache/openwhisk-wskdeploy/blob/master/tests/dat/manifest_validate_multiline_params.yaml
and the test case: https://github.com/apache/openwhisk-wskdeploy/blob/db5e34e35825900c52017b545e56935c5bea5578/parsers/manifest_parser_test.go#L288
I kindly ask you to add feature parity with wskdeploy in this case. single-level configurations are super ugly and unreadable.
Thanks @ekremney, it is clear now. I didn't see this file. I'll send a PR to the spec_parameters.md
doc to link the docs to the type schema at spec_parameter_types.md
.
There doesn't seem to be any test fixtures in wskdeploy
that test inputs with object types (as I mentioned in a previous comment, all are type string), so I'm surprised it works with wskdeploy (probably just an omission in the tests).
PR filed: https://github.com/apache/openwhisk-wskdeploy/pull/1094
@solaris007 I don't think there is any question that we will try to achieve parity with wskdeploy, we were trying to determine if it is in the spec, which it has been clarified that it is.
With regards to the tests you mentioned, the closest I see is for type json
, which is entirely different than type object
. There is no test for object
in that file, thus no test for "nested objects" (the file tests multi-line params).
Why not simply replace env vars in all of the manifest.yml in a first pass of the text file itself before parsing the yaml? Makes the code simpler and it works everywhere. If done right you can also do arrays etc. in the replacement.
I don’t know if that will create a conflict with the wskdeploy spec but aio is different already.
Here is my workaround for that using npm scripts:
Install envsub:
npm install --save-dev envsub
In package.json add:
"scripts": {
"deploy": "aio app deploy",
"predeploy": "cp manifest.yml manifest.backup.yml; envsub -f .env -s dollar-basic manifest.yml",
"postdeploy": "mv manifest.backup.yml manifest.yml"
}
Then run npm run deploy
to invoke aio app deploy
. to pass arguments to aio app deploy, run e.g. npm run deploy -- --skip-static
.
Note it is slightly dangerous since if deploy fails then it will not run postdeploy and restore the original manifest. So you might have a modified manifest with e.g. credentials present, and could mistakenly commit them. So watch out.
Hi @alexkli,
This issue just pinpoints the discrepancy between wskdeploy
and aio
. wskdeploy
supports object
type parameters whereas aio
does not.
That's a nice workaround you mention. In fact, we used envsubst
for a while, then we decide to use yaml anchors to group input parameters so that we won't have to repeat inputs for each action while keeping them top-level (for aio
to work well). For example:
inputs:
auth: &auth
public-key: $PUBLIC_KEY
db: &db
host: $DB_HOST
password: $DB_PASS
logging: &logging
host: $LOGGING_HOST
token: $LOGGING_TOKEN
packages:
__APP_PACKAGE__:
actions:
login:
function: actions/login
inputs:
<<: *auth
<<: *logging
getSomething:
function: actions/getSomething
inputs:
<<: *db
<<: *logging
doSomething:
function: actions/doSomething
inputs:
<<: *logging
@ekremney but your setup doesn't create nested input objects, it just ends up being flat. if you really want nested inputs/params in your actions then this doesn't help... or am I missing something?