Skip to content

fix: improve credential graph error handling and resolution logic#602

Merged
jakobmoellerdev merged 5 commits into
open-component-model:mainfrom
jakobmoellerdev:credentials-indirect-fix
Sep 3, 2025
Merged

fix: improve credential graph error handling and resolution logic#602
jakobmoellerdev merged 5 commits into
open-component-model:mainfrom
jakobmoellerdev:credentials-indirect-fix

Conversation

@jakobmoellerdev

Copy link
Copy Markdown
Member

What this PR does / why we need it

  • Refined error messages for better clarity when credential resolution fails.
  • Introduced ErrNoIndirectCredentials for clear distinction between direct and indirect resolution failures.
  • Enhanced ToGraph to handle nil configurations and added fallback logic for indirect resolution.
  • Updated method signatures to support the new GraphResolver interface.
  • Adjusted indirect resolution logic to signal failure without excessive verbosity.
  • Incorporated tests to validate updated error handling and resolution workflows.

Which issue(s) this PR fixes

Improves the readability and maintainability of credential graph resolution errors and ensures consistency across direct and indirect resolution paths. (noticed during testing of CLI interaction with credential graph)

#### What this PR does / why we need it

- Refined error messages for better clarity when credential resolution fails.
- Introduced `ErrNoIndirectCredentials` for clear distinction between direct and indirect resolution failures.
- Enhanced `ToGraph` to handle `nil` configurations and added fallback logic for indirect resolution.
- Updated method signatures to support the new `GraphResolver` interface.
- Adjusted indirect resolution logic to signal failure without excessive verbosity.
- Incorporated tests to validate updated error handling and resolution workflows.

#### Which issue(s) this PR fixes
Improves the readability and maintainability of credential graph resolution errors and ensures consistency across direct and indirect resolution paths.

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
@jakobmoellerdev jakobmoellerdev requested a review from a team as a code owner August 21, 2025 16:45
Comment thread bindings/go/credentials/graph.go
Comment thread bindings/go/credentials/graph.go Outdated
#### What this commit does and why we need it

- Removed implicit initialization of `nil` configurations in `ToGraph` implementation.
- Updated tests to pass a valid configuration explicitly.
- This change enforces clearer and more explicit usage patterns by requiring valid configuration objects to be provided.

This enhances code clarity and prevents potential misuse of the `ToGraph` function by relying on explicit input parameters.

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
fabianburth
fabianburth previously approved these changes Sep 3, 2025
Comment thread bindings/go/credentials/graph.go Outdated
#### What this commit does and why we need it

- Refactored `resolveFromGraph` to streamline handling of indirect resolution fallback.
- Consolidated condition checks to avoid redundant error handling.
- Ensures fallback to repository plugin provider is only attempted when direct resolution fails with `ErrNoDirectCredentials`.

This simplifies the resolution logic, improves code readability, and reduces unnecessary branching during credential resolution.

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
@jakobmoellerdev jakobmoellerdev merged commit 5c315c9 into open-component-model:main Sep 3, 2025
16 checks passed
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.

2 participants