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

[JENKINS-47101] Add a default EnvironmentExpander that registers password parameters as sensitive variables #110

Merged
merged 1 commit into from
Nov 5, 2020

Conversation

dwnusbaum
Copy link
Member

@dwnusbaum dwnusbaum commented Nov 5, 2020

See JENKINS-47101.

Before the fix for JENKINS-47101 in jenkinsci/workflow-cps-plugin#370, password parameters would have been masked in step arguments, so I think we need to make sure that they continue to be masked, which requires us to integrate with the new API from jenkinsci/workflow-step-api-plugin#57.

We need to add a test for this case over in workflow-cps.

@@ -97,6 +104,9 @@
return castOrNull(key,getExecution());
} else if (FlowNode.class.isAssignableFrom(key)) {
return castOrNull(key, getNode());
} else if (key == EnvironmentExpander.class) {
// Only called when `value == null` so that it does not override expanders contributed by steps.
return key.cast(new PasswordParameterEnvironmentExpander(get(Run.class)));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now we just have single default implementation since I cannot think of any other similar cases to password parameters. If there are similar cases, maybe we should create an extension point for marking sensitive variables that are active from the start of the build or something.

Copy link
Contributor

@car-roll car-roll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@car-roll car-roll marked this pull request as ready for review November 5, 2020 23:26
@car-roll car-roll merged commit 9c7ba70 into jenkinsci:master Nov 5, 2020
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

Successfully merging this pull request may close these issues.

2 participants