Skip to content

fix(credentialgraph): fix decade old credential resolution bug#2511

Merged
fabianburth merged 2 commits into
open-component-model:mainfrom
fabianburth:fix/credential-graph
May 13, 2026
Merged

fix(credentialgraph): fix decade old credential resolution bug#2511
fabianburth merged 2 commits into
open-component-model:mainfrom
fabianburth:fix/credential-graph

Conversation

@fabianburth

@fabianburth fabianburth commented May 13, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

Instead of passing the actual to-be-resolved consumer identity to credential plugins, we passed the credential plugins identity itself.

Which issue(s) this PR fixes

Testing

How to test the changes
Verification
  • I have added/updated tests for my changes (see Test Requirements)
  • Tests pass locally (task test and task test/integration if applicable)
  • If touching multiple modules, go work is enabled (see go.work)
  • My changes do not decrease test coverage
  • I have tested the changes locally by running ocm

@fabianburth
fix(credentialgraph): fix decade old credential resolution bug
68db87b 
Instead of passing the actual to-be-resolved consumer identity to credential plugins, we passed the credential plugins identity itself.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@fabianburth fabianburth marked this pull request as ready for review May 13, 2026 13:53
@netlify

netlify Bot commented May 13, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for ocm-website canceled.

Name Link
🔨 Latest commit afb4b44
🔍 Latest deploy log https://app.netlify.com/projects/ocm-website/deploys/6a0484cf8f344c00074b73a4

@fabianburth fabianburth requested a review from a team as a code owner May 13, 2026 13:53
@coderabbitai

coderabbitai Bot commented May 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Rate limit exceeded

@fabianburth has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 49 minutes and 2 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f98bf2a2-e0e4-4ef3-93ee-f3714a4b4a4a

📥 Commits

Reviewing files that changed from the base of the PR and between 68db87b and afb4b44.

📒 Files selected for processing (1)
  • bindings/go/credentials/graph_test.go
📝 Walkthrough

Walkthrough

The credential plugin resolution contract is updated to pass the current identity instead of childID to the Resolve method. Test credential plugins are updated: AWSSecretsManager validates credentials from the credentials map, and HashiCorpVault extracts vaultHost from repository configuration to support both ingestion-time and resolution-time identity sources.

Changes

Credential Plugin Resolution

Layer / File(s) Summary
Plugin resolution contract update
bindings/go/credentials/resolve_direct.go		 The Graph.resolveFromGraph method now passes identity (the current identity) instead of childID as input to the credential plugin's Resolve method, changing how plugin resolution receives context for child credential resolution.
Test plugin implementations
bindings/go/credentials/graph_test.go		 AWSSecretsManager test plugin removes identity-based secretId checking and validates against the credentials map; HashiCorpVault test plugin extracts vaultHost from repository configuration (supporting both serverURL config and hostname identity sources), then routes and generates secrets using vaultHost instead of direct identity hostname access.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

kind/bugfix, size/s

Suggested reviewers

  • jakobmoellerdev
  • frewilhelm
  • matthiasbruns

Poem

A rabbit hops through credentials so bright,
Passing identity instead of child ID in sight,
Vault hosts extracted with careful care,
Test plugins dancing, a harmonious pair! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: fixing a decade-old credential resolution bug where the wrong identity was passed to credential plugins.
Description check ✅ Passed The description directly addresses the changeset, explaining that credential plugins now receive the correct consumer identity instead of the credential plugin's own identity.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

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 and usage tips.

@fabianburth
fix(credentialgraph): add consumer identity checks to make mock more …
afb4b44 
…intuitive and realistic

Previously, the mock always returned credentials and completely ignored the consumer identity. Now it actually performs a sort of consumer identity matching.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@fabianburth fabianburth merged commit 885b2f6 into open-component-model:main May 13, 2026
18 of 19 checks passed
@fabianburth fabianburth deleted the fix/credential-graph branch May 13, 2026 14:13
@coderabbitai coderabbitai Bot mentioned this pull request May 15, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jakobmoellerdev jakobmoellerdev jakobmoellerdev approved these changes

@morri-son morri-son morri-son approved these changes

Assignees

No one assigned

Labels

kind/bugfix Bug size/s Small

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fabianburth @jakobmoellerdev @morri-son