Skip to content

fix: match token secrets that keep the provider name in the token name - #1090

Merged
vinokurig merged 2 commits into
mainfrom
fix-scm-provider-name-fallback
Oct 23, 2026
Merged

vinokurig merged 2 commits into
mainfrom
fix-scm-provider-name-fallback

Conversation

@vinokurig

@vinokurig vinokurig commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

isSecretMatchesSearchCriteria read the OAuth provider name from the che.eclipse.org/scm-provider-name annotation only. Secrets stored before that annotation was introduced keep the provider name in che.eclipse.org/scm-personal-access-token-name, so a lookup by provider name stopped matching them. Fall back to the token name when the provider name annotation is absent or empty, the same way getScmProviderName already does.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

https://redhat.atlassian.net/browse/CRW-13991

How to test this PR?

  1. Apply the pull request image: quay.io/eclipse/che-server:pr-1090
  2. Make sure that the Git Config tab in the User Preferences is empty, clear it if it is not.
  3. Manually add a PAT from self hosted git provider by the documentation instructions
  4. Go to the Git Config tab in the User Preferences and see that the data is fetched.

PR Checklist

As the author of this Pull Request I made sure that:

Release Notes

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Summary by CodeRabbit

Summary

  • Bug Fixes
    • Improved matching of saved SCM access tokens when provider-name information is missing or blank. In those cases, the token’s name is used to check whether it matches the requested provider. Matching behavior is unchanged when provider-name information is present, and other matching criteria remain unchanged.

isSecretMatchesSearchCriteria read the OAuth provider name from the
che.eclipse.org/scm-provider-name annotation only. Secrets stored before
that annotation was introduced keep the provider name in
che.eclipse.org/scm-personal-access-token-name, so a lookup by provider
name stopped matching them. Fall back to the token name when the provider
name annotation is absent or empty, the same way getScmProviderName
already does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 581ab83f-7c4b-40c3-a8bd-56e0c6dcfed5
📥 Commits

Reviewing files that changed from the base of the PR and between 237489e and a170929.

📒 Files selected for processing: 1
  • infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

Provider matching now falls back to the token-name annotation when the provider-name annotation is absent or empty. Tests cover matching and non-matching token names. User and server-URL matching remain unchanged.

Changes: SCM token provider matching

Layer / File(s) Summary
Provider-name fallback and tests
infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java, infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java
Provider matching falls back to the token-name annotation when the provider-name annotation is absent or empty. Tests cover absent and empty annotations, and a token name that does not match the requested provider.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a1709

The change covers legacy token matching, with no established merge-blocking risk.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: matching older token secrets that store the provider name in the token name.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Docker image build succeeded: quay.io/eclipse/che-server:pr-1090

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1090", name: che}]}}]"

@vinokurig

Copy link
Copy Markdown
Contributor Author

/retest

String configuredOAuthProviderName =
isNullOrEmpty(secretAnnotations.get(ANNOTATION_SCM_PROVIDER_NAME))
? secretAnnotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME)
: secretAnnotations.get(ANNOTATION_SCM_PROVIDER_NAME);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vinokurig can we make it more readable smth. like:

String providerName = secretAnnotations.get(ANNOTATION_SCM_PROVIDER_NAME);
String configuredOAuthProviderName = isNullOrEmpty(providerName) 
    ? secretAnnotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME) 
    : providerName;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@openshift-ci openshift-ci Bot removed the lgtm label Oct 23, 2026
String providerName = secretAnnotations.get(ANNOTATION_SCM_PROVIDER_NAME);
String configuredOAuthProviderName =
isNullOrEmpty(providerName)
? secretAnnotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vinokurig can we make it more readable smth. like:

String providerName = secretAnnotations.get(ANNOTATION_SCM_PROVIDER_NAME);
String configuredOAuthProviderName = isNullOrEmpty(providerName) 
    ? secretAnnotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME) 
    : providerName;

@github-actions

Copy link
Copy Markdown

Docker image build succeeded: quay.io/eclipse/che-server:pr-1090

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1090", name: che}]}}]"

@artaleks9

Copy link
Copy Markdown
Contributor

Verified on Eclipse Che deployed on OCP v4.22 with quay.io/eclipse/che-server:pr-1090

The functionality works properly.

@openshift-ci

openshift-ci Bot commented Oct 23, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: artaleks9, ibuziuk, vinokurig

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@vinokurig
vinokurig merged commit 4c24db6 into main Oct 23, 2026
17 checks passed
@vinokurig
vinokurig deleted the fix-scm-provider-name-fallback branch October 23, 2026 14:53
vinokurig added a commit that referenced this pull request Oct 23, 2026
#1090) (#1091)

* fix: match token secrets that keep the provider name in the token name

isSecretMatchesSearchCriteria read the OAuth provider name from the
che.eclipse.org/scm-provider-name annotation only. Secrets stored before
that annotation was introduced keep the provider name in
che.eclipse.org/scm-personal-access-token-name, so a lookup by provider
name stopped matching them. Fall back to the token name when the provider
name annotation is absent or empty, the same way getScmProviderName
already does.



* fixup! fix: match token secrets that keep the provider name in the token name

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

3 participants