-
Notifications
You must be signed in to change notification settings - Fork 242
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
A secret step #34
A secret step #34
Conversation
That can decrypt a string literal. The Snippetizer page provides a way to encrypt a plain text string
|
||
@Override | ||
public Step newInstance(@CheckForNull StaplerRequest req, @Nonnull JSONObject formData) throws FormException { | ||
if (req.findAncestor(Snippetizer.class) != null) { |
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.
Are you sure you do not just want to declare the parameter to be of type Secret
? Would need a little work in structs
but probably easy enough.
String plainText = "Bobby`s lil secret"; | ||
String script = "pipeline {\n" + | ||
" environment {\n" + | ||
" FOO = secret('"+encrypt(plainText)+"')\n" + |
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.
This is not being run as a regular Step
in a stage
, so I have no idea how this is implemented. Does not look like a bug in SecretStep
per se.
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
fwiw, I think |
Ok, found the problem - |
I guess we now need to figure out if this is what @kohsuke intended :) |
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 suppose this tries to implement JENKINS-27386, though it makes little sense to do it in this plugin, since there is no apparent reason why this would not be usable in non-declarative scripts.
I believe JENKINS-27398 would be a better approach as it integrates with Credentials and allows for better safety.
|
||
@Override | ||
public String run() throws Exception { | ||
return DescriptorImpl.decrypt(step.getText()); |
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.
This will leave the plaintext in program.dat
. Also it does nothing to prevent you from accidentally disclosing the secret in the log.
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.
Well the intention of this step as I understand it is to avoid having plain text secrets in your repo.
Whatever happens to it on the Jenkins master is of a lot lower concern. And afaiu the same this could be said about withCredentials
.
@jglick Implementing credentials is a different task from this one |
No, that step both masks log occurrences, and keeps the secret encrypted in
My argument is that implementing Credentials is the only reasonable way to accomplish the goal here. You want to not merely load some passwords in Groovy, you want interoperability with the wide array of plugins which expect secrets to be passed in as a |
So decision is that this approach is not feasible. |
That can decrypt a string literal.
The Snippetizer page provides a way to encrypt a plain text string.
This can be considered a PoC for how I interpreted the examples of this step.
@reviewbybees
The test case
worksInEnvironment
currently fails, I'm trying to figure out why. Somehow the descriptor is complaining that the parameter is of the wrong type when called from insideenvironment {}