Fixes #17 make pipeline-compatible #18
Conversation
There was a problem hiding this comment.
Thanks for putting this together! Since I am also interested in rpm signing in jenkinsfiles I wanted to pitch in with a code review.
This PR should also update the README with a basic example using this plugin in a jenkinsfile.
Do this changes still allow the plugin to work with the old style of jenkins job configurations?
| result = 31 * result + (privateKey.getPlainText() != null ? privateKey.getPlainText().hashCode() : 0); | ||
| result = 31 * result + (passphrase.getPlainText() != null ? passphrase.getPlainText().hashCode() : 0); | ||
| result = 31 * result + privateKey.getPlainText().hashCode(); | ||
| result = 31 * result + passphrase.getPlainText().hashCode(); |
There was a problem hiding this comment.
It can not be null.
getPlainText() method of Secret class uses value field and it is annotated as @nonnull.
| this.cmdlineOpts = ""; | ||
| } | ||
|
|
||
| @Deprecated | ||
| public Rpm(String gpgKeyName, String includes, String cmdlineOpts, boolean resign) { |
There was a problem hiding this comment.
Why deprecate the old constructor? Do pipeline uses of this plugin use the setters instead of the constructor? Wouldn't this constructor still be appropriate for non-pipeline configurations?
There was a problem hiding this comment.
pipeline uses constructor, and setter annotated with @DataBoundConstructor, @DataBoundSetter annotations respectively.
that's a valid point. We'll have to check if it's required for non-pipeline configurations.
|
@happyviral Still changes to be added? |
Changes are done. (review comments are addressed, and updated README.) Let me know if specific scenario required to be verified. |
|
@elibus What's the next step? |
|
Next is me reviewing and testing it. Unfortunately are busy times. Have you tested the build? |
|
No worries, I definitely understand. I haven't yet, I got a jenkins install going in docker so I could test but still need a git repo, rpm, and gpg key setup to try this out end-to-end. I'll give it a shot though when I get a chance. |
|
I have a test Jenkins instance with this plugin built and installed from the branch, with a developer eager to test it. It seems the snippet generator hasn't been updated to generate a snippet for this step, unless I've missed it, so it's not obvious what the pipeline syntax should be. |
|
@tony-sweeney Is this what you are seeing? The form for rpmsign shows up, but the "Generate Pipeline Script" button doesn't print out anything. Is there another interface that needs to be implemented to support the generation part? I would have thought that Jenkins knows enough to just fill in the parameters. |
|
Well I spoke too soon, the button click is causing an HTTP 500 response. Here's the stack trace in the response: java.lang.AssertionError: class jenkins.plugins.rpmsign.Rpm is missing its descriptor |
|
@happyviral Does this exception mean anything to you? java.lang.AssertionError: class jenkins.plugins.rpmsign.Rpm is missing its descriptor Edit: Found this description of the issue with the 500 error I think: https://wiki.jenkins.io/display/JENKINS/My+class+is+missing+descriptor |
|
@schwede I simply don't see an RPM sign entry in the Snippet Generator Sample Step dropdown. |
|
As an experiment I tried to build the pipeline-compatible branch of happyviral's fork on GitHub: |
sorry for the late reply. |
|
I have tested on Jenkins 2.164.3 In my case a plugin built from: |
|
Is there any updates on this? or anything I can do to help move it along? |
Fixes #17 make pipeline-compatible
I've passed validation for now without any check.
I've tried with <jenkins.version>2.138.2</jenkins.version>