Skip to content

Fix case where "SecretTokenAsPlainText" is null #324

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
Jun 2, 2023

Conversation

jmini
Copy link
Contributor

@jmini jmini commented May 26, 2023

Fixes #304
Fixes #292

getSecretTokenAsPlainText() can return null, see:
https://github.com/jenkinsci/gitlab-branch-source-plugin/blob/d45c0f4c00428cd0d79ba15af87091708b4b58b8/src/main/java/io/jenkins/plugins/gitlabserverconfig/servers/GitLabServer.java#LL397C1-L407C1

So the code needs to be able to handle the null case.

  • When updating the Hook registration in GitLab we need to send "" (since null will just preserve the current value)
  • When checking the X-Gitlab-Token sent by GitLab in the POST request, when the header is absent, this means that the SecretToken in Jenkins for that server is expected to be null or empty.

Testing done

TODO

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main 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
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@jmini jmini requested a review from jetersen as a code owner May 26, 2023 13:37
@jetersen jetersen added the bug Something isn't working label May 26, 2023
@jetersen
Copy link
Member

@piotrroda
Copy link

@jmini What about your changes? Could you check these errors? This fix is very important. :)

@jmini
Copy link
Contributor Author

jmini commented May 30, 2023

I reverted too much imports changes.

@jmini
Copy link
Contributor Author

jmini commented Jun 2, 2023

@jetersen I have now tested locally

Jenkins log

The <jenkins>/job/<project>/computation/console does not contains a NPE anymore.

I also have tried changing the Secret back and forth in <jenkins>/manage/configure in Manage Jenkins >
Configure System
under the GitLab Server server.

The corresponding secret gets updated in GitLab:

Gitlab admin

And the webhook triggers still works.

Verified as well in the "Scan GitLab Project" view <jenkins>/job/<project>/job/<gitlab-project>/indexing/events

@jetersen jetersen merged commit 9241ae4 into jenkinsci:master Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants