Skip to content

[BEE-62243] Use AccessSpecifiedRepositories as default access strategy for GitHub App credentials #890

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.

Sign up for GitHub

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

Jump to bottom
Merged
jeromepochat merged 8 commits into from
Sep 11, 2025
Merged

[BEE-62243] Use AccessSpecifiedRepositories as default access strategy for GitHub App credentials #890

jeromepochat merged 8 commits into from
Sep 11, 2025

Conversation

@jeromepochat
Copy link
Contributor

@jeromepochat jeromepochat commented Sep 8, 2025

Description

#822 introduced new options to dynamically restricts access to the repositories. This comes with backward compatibility limitations.

This aims to improve backwards compatibility by using AccessSpecifiedRepositories as default access strategy instead of AccessInferredOwner.

The existing credentials is migrated to AccessSpecifiedRepositories:

  • the owner is set with the original credentials owner
  • the list of repository is set with empty list

In case of empty owner and when the credentials are used in a context where inference is supported, the inferred owner is used to compute permissions.

See JENKINS-76056 for further information.

Submitter checklist

  • Link to JIRA ticket in description, if appropriate.
  • Change is code complete and matches issue description
  • Automated tests have been added to exercise the changes
  • Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

  • Run the changes and verify that the change matches the issue description
  • Reviewed the code
  • Verified that the appropriate tests have been written or valid explanation given

Documentation changes

  • Link to jenkins.io PR, or an explanation for why no doc changes are needed

Users/aliases to notify

dwnusbaum
dwnusbaum reviewed Sep 8, 2025
View reviewed changes
@jeromepochat jeromepochat marked this pull request as ready for review September 9, 2025 08:24
@jeromepochat jeromepochat requested a review from a team as a code owner September 9, 2025 08:24
dwnusbaum
dwnusbaum reviewed Sep 9, 2025
View reviewed changes
Copy link
Member

@dwnusbaum dwnusbaum left a comment

Choose a reason for hiding this comment

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

Looks mostly good, but I added a few comments, and can you also update https://github.com/jenkinsci/github-branch-source-plugin/blob/master/docs/github-app.adoc#backwards-compatibility to account for the changes in this PR?

@jeromepochat jeromepochat mentioned this pull request Sep 10, 2025
Merged
6 tasks
dwnusbaum
dwnusbaum approved these changes Sep 10, 2025
View reviewed changes
Copy link
Member

@dwnusbaum dwnusbaum left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks good. One other thing I noticed is that I think the UI picks AccessInferredRepository as the default option when you create a new credential. Maybe we can/should set Extension.ordinal so that AccessSpecifiedRepositories is the default, or perhaps we need to create a Descriptor.doFill* method? Not sure without trying it out. Also I think AccessSpecifiedRepositories/help-owner.html could be updated to mention the new logic. I think help.html is ok though.

I labeled this as bug since we're trying to reduce the breakage from the earlier change, but I'm not really sure what the best label would be. I would maybe update the release notes to link to the earlier release to make it easier to see what is changing.

@dwnusbaum dwnusbaum added the bug label Sep 10, 2025
@jeromepochat jeromepochat merged commit 1afc22a into jenkinsci:master Sep 11, 2025
17 checks passed
@jeromepochat jeromepochat deleted the BEE-62243-update-migration-path branch September 11, 2025 14:28
@KyriosGN0
Copy link

KyriosGN0 commented Nov 18, 2025

Hey, was this PR released? if not when it is planned to be released ?

@jeromepochat
Copy link
Contributor Author

jeromepochat commented Nov 18, 2025 •
edited
Loading

Hey, was this PR released?

Hi!
This was released in 1862.v1a_fc22a_d3788.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dwnusbaum dwnusbaum dwnusbaum approved these changes

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jeromepochat @KyriosGN0 @dwnusbaum