Skip to content

Commit 5c315c9

Browse files
jakobmoellerdevjakobmoellerdev
authored
fix: improve credential graph error handling and resolution logic (#602)
<!-- markdownlint-disable MD041 --> #### 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) --------- Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
1 parent dea3c9f commit 5c315c9

5 files changed

Lines changed: 58 additions & 17 deletions

File tree

‎bindings/go/credentials/graph.go‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ import (
1616
// plugins which can be resolved at runtime.
1717
var ErrNoDirectCredentials = errors.New("no direct credentials found in graph")
1818

19+
// ErrNoIndirectCredentials is returned when no indirect credentials are found in the graph.
20+
// This can happen if no repository plugin is configured or if no repository plugin can resolve
21+
// credentials for the given identity.
22+
var ErrNoIndirectCredentials = errors.New("no indirect credentials found in graph")
23+
1924
var scheme = runtime.NewScheme()
2025

2126
func init() {
@@ -29,10 +34,14 @@ type Options struct {
2934
CredentialRepositoryTypeScheme *runtime.Scheme
3035
}
3136

37+
type GraphResolver interface {
38+
Resolve(ctx context.Context, identity runtime.Identity) (map[string]string, error)
39+
}
40+
3241
// ToGraph creates a new credential graph from the provided configuration and options.
3342
// It initializes the graph structure and ingests the configuration into the graph.
3443
// Returns an error if the configuration cannot be properly ingested.
35-
func ToGraph(ctx context.Context, config *cfgRuntime.Config, opts Options) (*Graph, error) {
44+
func ToGraph(ctx context.Context, config *cfgRuntime.Config, opts Options) (GraphResolver, error) {
3645
g := &Graph{
3746
syncedDag: newSyncedDag(),
3847
credentialPluginProvider: opts.CredentialPluginProvider,
@@ -70,16 +79,15 @@ func (g *Graph) Resolve(ctx context.Context, identity runtime.Identity) (map[str
7079
// Attempt direct resolution via the DAG.
7180
creds, err := g.resolveFromGraph(ctx, identity)
7281

73-
switch {
74-
case errors.Is(err, ErrNoDirectCredentials):
75-
// fall back to indirect resolution
76-
return g.resolveFromRepository(ctx, identity)
77-
case err != nil:
78-
return nil, err
82+
// fall back to indirect resolution if we have a repository plugin provider
83+
// otherwise leave error as is
84+
if g.repositoryPluginProvider != nil && errors.Is(err, ErrNoDirectCredentials) {
85+
creds, err = g.resolveFromRepository(ctx, identity)
7986
}
8087

81-
if len(creds) > 0 {
82-
return creds, nil
88+
if err != nil {
89+
return nil, fmt.Errorf("failed to resolve credentials for identity %q: %w", identity.String(), err)
8390
}
84-
return nil, fmt.Errorf("failed to resolve credentials for identity %v", identity)
91+
92+
return creds, nil
8593
}

‎bindings/go/credentials/graph_test.go‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ const invalidRecursionYAML = testYAML + `
144144
secretId: "recursive-creds"
145145
`
146146

147-
func GetGraph(t testing.TB, yaml string) (*credentials.Graph, error) {
147+
func GetGraph(t testing.TB, yaml string) (credentials.GraphResolver, error) {
148148
t.Helper()
149149
r := require.New(t)
150150
scheme := runtime.NewScheme()
@@ -487,3 +487,34 @@ consumers:
487487
})
488488
}
489489
}
490+
491+
func TestResolutionErrors(t *testing.T) {
492+
id := runtime.Identity{
493+
"type": "not exists",
494+
}
495+
496+
r := require.New(t)
497+
g, err := credentials.ToGraph(t.Context(), &credentialruntime.Config{}, credentials.Options{})
498+
require.NoError(t, err)
499+
creds, err := g.Resolve(t.Context(), id)
500+
r.Empty(creds)
501+
r.Error(err)
502+
r.ErrorIs(err, credentials.ErrNoDirectCredentials)
503+
r.ErrorContains(err, fmt.Sprintf("failed to resolve credentials for identity %q: failed to match any node: no direct credentials found in graph", id.String()))
504+
505+
g, err = credentials.ToGraph(t.Context(), &credentialruntime.Config{}, credentials.Options{
506+
RepositoryPluginProvider: credentials.GetRepositoryPluginFn(func(ctx context.Context, repoType runtime.Typed) (credentials.RepositoryPlugin, error) {
507+
return RepositoryPlugin{
508+
RepositoryIdentityFunc: func(config runtime.Typed) (runtime.Identity, error) {
509+
return runtime.Identity{}, nil
510+
},
511+
}, nil
512+
}),
513+
})
514+
r.NoError(err)
515+
creds, err = g.Resolve(t.Context(), id)
516+
r.Empty(creds)
517+
r.Error(err)
518+
r.ErrorIs(err, credentials.ErrNoIndirectCredentials)
519+
r.ErrorContains(err, fmt.Sprintf("failed to resolve credentials for identity %q: no indirect credentials found in graph", id.String()))
520+
}

‎bindings/go/credentials/resolve_direct.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,21 +35,21 @@ func (g *Graph) resolveFromGraph(ctx context.Context, identity runtime.Identity)
3535
for id := range vertex.Edges {
3636
childID, ok := g.getIdentity(id)
3737
if !ok {
38-
return nil, fmt.Errorf("failed to resolve credentials for node %q: child node %q not found", vertex.ID, id)
38+
return nil, fmt.Errorf("no credentials for node %q available: child node %q not found", vertex.ID, id)
3939
}
4040
childCredentials, err := g.resolveFromGraph(ctx, childID)
4141
if err != nil {
4242
return nil, err
4343
}
4444
plugin, err := g.credentialPluginProvider.GetCredentialPlugin(ctx, childID)
4545
if err != nil {
46-
return nil, fmt.Errorf("failed to resolve credentials for node %q: %w", childID, err)
46+
return nil, fmt.Errorf("could not get credential plugin for node %q: %w", childID, err)
4747
}
4848

4949
// Let the plugin resolve the child's credentials.
5050
credentials, err := plugin.Resolve(ctx, childID, childCredentials)
5151
if err != nil {
52-
return nil, fmt.Errorf("failed to resolve credentials for node %q: %w", childID, err)
52+
return nil, fmt.Errorf("no credentials for node %q resolved from plugin: %w", childID, err)
5353
}
5454

5555
// Merge the resolved credentials into the result

‎bindings/go/credentials/resolve_indirect.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package credentials
33
import (
44
"context"
55
"errors"
6-
"fmt"
76
"log/slog"
87
"sync"
98

@@ -79,8 +78,11 @@ func (g *Graph) resolveFromRepository(ctx context.Context, identity runtime.Iden
7978
}
8079

8180
wg.Wait()
81+
8282
if resolved == nil {
83-
return nil, fmt.Errorf("failed to resolve credentials for %q indirectly using repository plugins: %w", identity, errors.Join(errs...))
83+
// If we get here, then all repository plugins failed to resolve credentials.
84+
// This is not an error, but rather a signal that the identity could not be resolved indirectly.
85+
return nil, errors.Join(ErrNoIndirectCredentials, errors.Join(errs...))
8486
}
8587

8688
// Cache the resolved credentials for future use.

‎bindings/go/credentials/synced_dag.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func (g *syncedDag) matchAnyNode(identity runtime.Identity) (*dag.Vertex[string]
8383
return vertex, nil
8484
}
8585
}
86-
return nil, fmt.Errorf("failed to resolve credentials for node %v: %w", node, ErrNoDirectCredentials)
86+
return nil, fmt.Errorf("failed to match any node: %w", ErrNoDirectCredentials)
8787
}
8888

8989
// addIdentity ensures that a given identity is represented as a vertex in the graph.

0 commit comments

Comments
 (0)