Skip to content

fix: prevent publication with unsupported credential types#351

Merged
timja merged 1 commit into
jenkinsci:masterfrom
pcanilho:bugfix/unsupported-credentials
Aug 22, 2023
Merged

fix: prevent publication with unsupported credential types#351
timja merged 1 commit into
jenkinsci:masterfrom
pcanilho:bugfix/unsupported-credentials

Conversation

@pcanilho
Copy link
Copy Markdown
Contributor

@pcanilho pcanilho commented Jul 5, 2023

fixes: #347

What changed?

Unsupported credential types are now explicitly made incompatible with the github-checks-plugin to avoid downstream unexpected exceptions such as the one reported here.

With this change, the supported credential types are:

  • GitHubAppCredentials
  • VaultUsernamePasswordCredentialImpl

If an unsupported credential type is provided, an early return is done to avoid having to handle downstream exceptions caused by incompatible types. This means that all pipeline console log entries will be suppressed if the pipeline has not been configured with compatible credentials.

Testing done

Procedure
As extracted from here, I have ran the following command to both build & run the test suite:

$ mvn -Penable-jacoco clean verify -B -V -ntp

A regression test was also carried out in our internal Jenkins controller to assert that previous functionality based on the above described credential types is left unaffected.

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

Copy link
Copy Markdown
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

i guess ok

@timja timja requested review from XiongKezhi and uhafner July 5, 2023 16:07
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 5, 2023

Codecov Report

Merging #351 (0e877c2) into master (983fed5) will decrease coverage by 0.22%.
The diff coverage is 33.33%.

@@             Coverage Diff              @@
##             master     #351      +/-   ##
============================================
- Coverage     72.02%   71.80%   -0.22%     
  Complexity      162      162              
============================================
  Files            16       16              
  Lines           529      532       +3     
  Branches         50       51       +1     
============================================
+ Hits            381      382       +1     
- Misses          123      124       +1     
- Partials         25       26       +1     
Impacted Files Coverage Δ
...s/plugins/checks/github/GitHubChecksPublisher.java 94.54% <33.33%> (-3.54%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@meiswjn
Copy link
Copy Markdown

meiswjn commented Jul 7, 2023

@meiswjn
Copy link
Copy Markdown

meiswjn commented Aug 22, 2023

Can this be merged? :) @timja

@timja timja merged commit bee03a0 into jenkinsci:master Aug 22, 2023
@timja timja added the bug Something isn't working label Aug 22, 2023
@timja
Copy link
Copy Markdown
Member

timja commented Aug 22, 2023

heh I was giving @uhafner time to review as he seemed to get annoyed last time I merged same day.

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

Development

Successfully merging this pull request may close these issues.

Broken using SSH credentials with this plugin

3 participants