Skip to content
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

Access to folder (multi-branch) name #22

Open
iwarapter opened this issue Nov 10, 2022 · 10 comments · May be fixed by jenkinsci/branch-api-plugin#344
Open

Access to folder (multi-branch) name #22

iwarapter opened this issue Nov 10, 2022 · 10 comments · May be fixed by jenkinsci/branch-api-plugin#344
Labels
enhancement New feature or request

Comments

@iwarapter
Copy link
Contributor

What feature do you want to see added?

When using build scoped claims on a multi-branch pipeline the JOB_BASE_NAME will always be present which makes the sub always unique per branch.

Given the following template: job:${JOB_NAME}

Would produce a sub like:

  "sub": "job:random-branch-test/master",

However depending on your target system this can or cannot be handled, for example AWS iam conditions are flexible enough:

"StringLike": {
  "jenkins.at.my.issuer.com:sub": "job:random-branch-test/*"
},

However azuread federated credentials are not, it needs to be an explicit match meaning id need to configure the name of every branch (and pull request) ahead of it running.

Proposal

Can we allow the claims template to utilise some groovy scripts to do much deeper configuration.

This would allow us todo things like determine if the multi-branch job is triggered from a pull request where would could create similar subjects to githubs pull_request events, e.g: repo:<orgName/repoName>:pull_request but job bound like job:random-branch-test:pull_request.

Given that the build templates are secured at the Jenkins administrator security level would this be acceptable?

Upstream changes

No response

@iwarapter iwarapter added the enhancement New feature or request label Nov 10, 2022
@iwarapter
Copy link
Contributor Author

hey @jglick i'd be happy to have a look at this if you could give any pointers on how to go about implementing this.

@jglick
Copy link
Member

jglick commented Nov 11, 2022

I do not exactly follow what sorts of claims you are looking to define and why you cannot already create them given https://github.com/jenkinsci/branch-api-plugin/blob/32f0861d6b50a9cbf39789f1d713e6979d412e4e/src/main/resources/jenkins/branch/BranchNameContributor/buildEnv.properties#L23-L37.

@iwarapter
Copy link
Contributor Author

hi @jglick, thanks for getting back,

I have two problems ultimately around multi-branch projects:

1

Given say I have 3 branches master, example1, example2.
With a sub claim defined as job:${JOB_NAME}, i'd get 3 different subs:

  "sub": "job:random-branch-test/master"

  "sub": "job:random-branch-test/example1"

  "sub": "job:random-branch-test/example1"

Whats more if I create a branch called PR-2 I would get a sub that looks like it came from a pull request:

  "sub": "job:random-branch-test/PR-2",

None of these would be deterministic within AzureAD as it requires full qualified subject identifiers, i'd need to define every possible branch that would ever exist.

2

The pull-request side could be game'd by a bad actor, if you are using AWS for example with a condition policy like:

"StringLike": {
  "jenkins.at.my.issuer.com:sub": "job:random-branch-test/PR-*"
},

This would pick up any branch called PR, which is why I was interested to see if we could potentially configure the oidc provider to allow pull requests to have a specific sub just like github actions do: https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect#filtering-for-pull_request-events

@jglick
Copy link
Member

jglick commented Nov 11, 2022

it requires full qualified subject identifiers, i'd need to define every possible branch that would ever exist

What I am not following is what sub (or set of possible subs) you want to have instead.

The pull-request side could be game'd by a bad actor […] This would pick up any branch called PR

I am not following this concern either. The “bad actor” would need to have write access to your repository (not a fork).

allow pull requests to have a specific sub

Well, CHANGE_ID will be defined on PR builds and not on (origin) branch builds. Currently the plugin will just leave undefined variables unexpanded, so something:${CHANGE_ID}; would probably make sense to be able to offer multiple variants of a claim where earlier variants in the list will be skipped if they refer to undefined variables, or something like that.

@iwarapter
Copy link
Contributor Author

iwarapter commented Nov 11, 2022

What I am not following is what sub (or set of possible subs) you want to have instead.

I'd like to be able to access the actual multibranch folder name:

  "sub": "job:random-branch-test"

Effectively JOB_NAME minus JOB_BASE_NAME

I haven't found any environment variables for this

But having some sort of programmable logic in the claim templates would mean I could potentially do something like:
(pseudo code)

if CHANGE_ID != "" {
  return "job:${FOLDER_NAME}:pull_request
} if TAG_NAME != "" {
  return "job:${FOLDER_NAME}:tag:${TAG_NAME}
} else {
  return "job:${FOLDER_NAME}:branch:${BRANCH_NAME}
}

@jglick
Copy link
Member

jglick commented Nov 11, 2022

Effectively JOB_NAME minus JOB_BASE_NAME

I haven't found any environment variables for this

That seems like something that should be added to branch-api. (Almost. In some corner cases I think the MultiBranchProject.name might not match the GitHub repository name, which is what you really want.)

programmable logic

Something I would prefer to avoid. Given a standard environment variable for repository name or at least multibranch folder name (which I think would be broadly useful), then I think the proposed logic could be handled by a more declarative format. My previous proposal

offer multiple variants of a claim where earlier variants in the list will be skipped if they refer to undefined variables

would not handle the first clause if you do not actually want the value of ${CHANGE_ID} included, but perhaps a claim template could specify variables which must be defined in order to process it (even if their value is not used), and earlier valid definitions of a given claim would take precedence. Possibly this could even be used to deprecate the buildClaimTemplates & globalClaimTemplates fields in #18; for example the default claim templates would be:

  1. sub=${JOB_URL} (JOB_URL)
  2. build_number=${BUILD_NUMBER} (BUILD_NUMBER)
  3. sub=${JENKINS_URL} ()

Your config could be something like

  1. sub=job:${GITHUB_REPO}:pull_request (GITHUB_REPO, CHANGE_ID)
  2. sub=job:${GITHUB_REPO}:tag:${TAG_NAME} (GITHUB_REPO, TAG_NAME)
  3. sub=job:${GITHUB_REPO}:tag:${BRANCH_NAME} (GITHUB_REPO, BRANCH_NAME)
  4. sub=job:${JOB_NAME}:other (JOB_NAME)
  5. sub=${JENKINS_URL} ()

@iwarapter
Copy link
Contributor Author

That seems like something that should be added to branch-api. (Almost. In some corner cases I think the MultiBranchProject.name might not match the GitHub repository name, which is what you really want.)

Thanks, I'll take a look at that see, if its simple enough to impl. The folder name is what I need for this particular use-case, and we actually have two multibranch projects using the same GH repo (with different config/pipelines) so the GITHUB_REPO for that would the same and I dont wont both jobs being able to get tokens (if the sub is job:${GITHUB_REPO} for example)

offer multiple variants of a claim where earlier variants in the list will be skipped if they refer to undefined variables

Yes, i think this would work 👍

@jglick
Copy link
Member

jglick commented Nov 11, 2022

You would probably want to define ${GITHUB_ORG} also. TBD where that should live. branch-api could trivially define the MultiBranchProject.name. Better would be to define general APIs for “repo” and “org” name applicable to various SCM sources which have such concepts (in which case the variable name would be more generic as well); not sure if scm-api already defines this info or not. (If not, then we would need changes to both scm-api and impls like github-branch-source.) Or github-branch-source could contribute these variables only for GitHub, though we prefer to avoid that.

iwarapter added a commit to iwarapter/oidc-provider-plugin that referenced this issue Nov 12, 2022
@iwarapter
Copy link
Contributor Author

hey @jglick had a quick go at what this might look like, its rough currently but does the basics of allowing precedence based on environment variables.

iwarapter added a commit to iwarapter/oidc-provider-plugin that referenced this issue Nov 14, 2022
iwarapter added a commit to iwarapter/oidc-provider-plugin that referenced this issue Nov 22, 2022
fix feedback :+1

switch @DataBoundConstructor back to original constructor

Update src/main/java/io/jenkins/plugins/oidc_provider/IdTokenCredentials.java

Co-authored-by: Francisco Javier Fernandez <31063239+fcojfernandez@users.noreply.github.com>

fix missing import

revert StringUtils.isNotBlank change

pr feedback

fix jelly template
@iwarapter
Copy link
Contributor Author

hey @jglick if you get a chance could you take a look at the supporting PR jenkinsci/branch-api-plugin#344 many thanks!

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

Successfully merging a pull request may close this issue.

2 participants