Skip to content

[JENKINS-63761] Certificate upload correction #208

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

Merged
merged 4 commits into from
May 12, 2021

Conversation

Wadeck
Copy link
Contributor

@Wadeck Wadeck commented May 12, 2021

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • [n/a] Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Jira tickets: JENKINS-63761 and JENKINS-64542 (at least)

⁉️ There are multiple things corrected here. The main one is the bug introduced since 2.254 in jenkinsci/jenkins#4910. Due to the COOP header, the new window approach to upload certificate is no longer working. The second problem was about the warning/error message when you are uploading the certificate.
The second part about the validation was filed as a bug in the tracker: JENKINS-65616.

✔️ Both are resolved by putting the file input field "inline" inside the form instead of using a window. Then, by adding some magic from Jelly/hudson-behavior.js, we are able to trigger the validation at the correct moment.

Tested with:

  • 2.253 (before the COOP header)
  • 2.254 (just after the COOP header)
  • 2.277.4 (current LTS)
  • 2.292 (current weekly)
Gifs of the behavior

2.253 Create

creds_2 253_new


2.253 Update

creds_2 253_update


2.292 Create

creds_2 292_new


2.292 Update

creds_2 292_update


Sorry, something went wrong.

@Wadeck Wadeck added the bug label May 12, 2021
Copy link
Member

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

Works in my limited testing on Firefox 88.0.1 on 2.277.4.

Previously reviewed in detail elsewhere, this incorporates my feedback already 👍

@Wadeck Wadeck merged commit 80425d4 into jenkinsci:master May 12, 2021
@saper
Copy link

saper commented May 12, 2021

I think we have typo in the bug number here in the subject - it is also wrong in the release notes https://github.com/jenkinsci/credentials-plugin/releases/tag/credentials-2.4

@daniel-beck daniel-beck changed the title [JENKINS-62761] Certificate upload correction [JENKINS-63761] Certificate upload correction May 12, 2021
@Wadeck
Copy link
Contributor Author

Wadeck commented May 12, 2021

@saper thanks! (and thanks Daniel for the quick adjustment)

@timja
Copy link
Member

timja commented May 13, 2021

Lol I spent far too long trying to integrate / fix a bug with certs in one of my plugins and gave up as I couldn’t figure out how to upload the cert...

Good to know it wasn’t me being dumb, can’t remember where it was though

new Upload(getDivId(), uploadedKeystore), "complete");
return FormValidation.ok("This endpoint is no longer required/supported due to the inlining of the file input. " +
"If you came to this endpoint due to another plugin, you will have to update that plugin to be compatible with Credentials Plugin 2.4+. " +
"It will be deleted soon.");
Copy link
Member

Choose a reason for hiding this comment

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

It will be deleted soon

But #541?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants