-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add node_envvars
module,
#29
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please
title: Schema for Jenkins Global EnvVars setter | ||
type: object | ||
minProperties: 1 | ||
additionalProperties: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please look here https://spacetelescope.github.io/understanding-json-schema/reference/object.html#pattern-properties ?
AFAIU, hee should be KV with UpperString to string mapping. But now schema allows to pass something like
1: {"foo": {"bar": ["baz" ,2]}}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that using
additionalProperties:
type: string
is actually enough to ensure that the value must be a string.
This is shown by the last example on the doc page you've linked.
I've also made a gist to illustrate this.
Actually, while I've reviewed the schema I've suddenly realized that setting the minimum of one child element for a config to be considered valid will lead to an inability to disable the environment variables at all.
This makes {node-envvars: {"node1": []}}
invalid, while it represents a valid state of environment variables. I'm going to fix that on both PRs.
jenkins_yaml_path: '\n'.join( | ||
[ | ||
'jenkins:', | ||
' slave-envvars:', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something is incorrect here.. You provided example with "node-env-vars", but here "slave-envvars" is used. This is probably because of too permissive schema.
BTW IMHO "node-env-vars" sounds better.
…ariables on Jenkins nodes
@@ -0,0 +1,6 @@ | |||
title: Schema for Jenkins Global EnvVars setter | |||
type: object | |||
additionalProperties: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, looks like a couple of my comments were corrupted |
This module adds an ability to set environment variables on Jenkins nodes