From 53bc65a73f0d8d28028d65faae08ed245ebe0b3f Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Mon, 6 Jul 2026 15:09:10 -0400 Subject: [PATCH 1/2] feat(expressions): add repoCredentials() expression function Add a `repoCredentials()` expression function that resolves repository credentials by repo URL and credential type (`git`, `helm`, `image`) through the same credentials database used by built-in promotion steps. Unlike `secret()`, which returns the raw `Data` of a Secret selected by name, `repoCredentials()` returns the *resolved* credentials -- e.g. a minted, short-lived GitHub App installation token -- so custom and generic steps (such as `http`) can obtain usable credentials for repositories they need to access, instead of only the raw material stored in the Secret. The function is wired into the promotion StepEvaluator, which now carries the credentials database. When credentials are found it always returns a fixed, predictable set of keys (`username`, `password`, `sshPrivateKey`); when none are found it returns an empty map, mirroring `secret()`. Contexts that do not wire a credentials database (Stage verification argument evaluation, the Argo CD selector, and the indexer) register the function but return a clear error if it is invoked. Signed-off-by: Justin Brooks Co-Authored-By: Claude Opus 4.8 Signed-off-by: Justin Brooks --- .../60-reference-docs/40-expressions.md | 57 +++++ pkg/controller/promotions/argocd_selector.go | 2 +- pkg/controller/stages/regular_stages.go | 4 + pkg/expressions/function/functions.go | 140 ++++++++++- pkg/expressions/function/functions_test.go | 227 ++++++++++++++++++ pkg/indexer/indexer.go | 2 +- pkg/promotion/evaluator.go | 33 ++- pkg/promotion/evaluator_test.go | 7 +- pkg/promotion/local_orchestrator.go | 4 +- 9 files changed, 455 insertions(+), 21 deletions(-) diff --git a/docs/docs/50-user-guide/60-reference-docs/40-expressions.md b/docs/docs/50-user-guide/60-reference-docs/40-expressions.md index 722fe553d8..ae9c3a5ff9 100644 --- a/docs/docs/50-user-guide/60-reference-docs/40-expressions.md +++ b/docs/docs/50-user-guide/60-reference-docs/40-expressions.md @@ -463,6 +463,63 @@ config: ::: +### `repoCredentials(repoURL, type)` + +The `repoCredentials()` function resolves repository credentials by repository +URL and credential type, returning them as a `map[string]string`. It takes two +required arguments: + +- `repoURL` (Required): A string representing the URL of the repository whose + credentials should be resolved. + +- `type` (Required): A string representing the credential type. Must be one of + `git`, `helm`, or `image`. The type is required because credentials are + indexed by type, and the same URL may resolve to different credentials + depending on it. + +Unlike `secret()`, which returns the raw `Data` of a `Secret` selected by +_name_, `repoCredentials()` performs a lookup by repository URL through the same +credentials database used by built-in promotion steps like `git-clone`. It +returns the **resolved** credentials — not the raw contents of the underlying +`Secret`. This means it transparently handles credential schemes that yield +narrowly-scoped, short-lived credentials — such as GitHub App installation +tokens or a cloud provider's ambient (Pod identity) credentials — instead of +returning the raw material (e.g. an app ID and private key) from which those +credentials are derived. + +Because the result is normalized, the set of available keys is **fixed** and +does not vary by credential type or provider. When credentials are found, the +returned map always contains all of the following keys: + +| Key | Description | +|-----|-------------| +| `username` | The username identifying the principal. For token-based credentials this is often an inconsequential placeholder. | +| `password` | The password or token used to authenticate. **API keys and personal access tokens are surfaced here.** | +| `sshPrivateKey` | The SSH private key, when applicable. **Deprecated as of v1.10.0** and slated for removal in v1.13.0. | + +If no matching credentials are found, an empty map is returned. + +For details on how each field is populated for a given credential type or +provider (e.g. GitHub App, ECR, or basic username/password), see the +[Managing Secrets](../50-security/30-managing-secrets.md#repository-credentials) +page. + +Examples: + +```yaml +config: + headers: + - name: Authorization + value: Bearer ${{ repoCredentials('https://github.com/example/repo.git', 'git').password }} +``` + +:::note + +`repoCredentials()` is only available within `Promotion` step expressions, where +Kargo's credentials database is available. + +::: + ### `warehouse(name)` The `warehouse()` function returns a `FreightOrigin` object representing a diff --git a/pkg/controller/promotions/argocd_selector.go b/pkg/controller/promotions/argocd_selector.go index c934d0c4e3..90b372b3e7 100644 --- a/pkg/controller/promotions/argocd_selector.go +++ b/pkg/controller/promotions/argocd_selector.go @@ -62,7 +62,7 @@ func promotionSelectorsMatchApp( } promoCtx := promotion.NewContext(promo, stage) - evaluator := promotion.NewStepEvaluator(cl, nil) + evaluator := promotion.NewStepEvaluator(cl, nil, nil) for i, step := range promo.Spec.Steps { if int64(i) > promo.Status.CurrentStep { diff --git a/pkg/controller/stages/regular_stages.go b/pkg/controller/stages/regular_stages.go index 6d52ca1eda..e71d7156a1 100644 --- a/pkg/controller/stages/regular_stages.go +++ b/pkg/controller/stages/regular_stages.go @@ -1386,6 +1386,10 @@ func (r *RegularStageReconciler) startVerification( exprfn.DataOperations( ctx, r.client, + // The Stage reconciler does not wire a credentials database, + // so repoCredentials() is unavailable during verification + // argument evaluation. + nil, gocache.New(gocache.NoExpiration, gocache.NoExpiration), stage.Namespace, ), diff --git a/pkg/expressions/function/functions.go b/pkg/expressions/function/functions.go index 6f9e9bd84d..7145a32682 100644 --- a/pkg/expressions/function/functions.go +++ b/pkg/expressions/function/functions.go @@ -21,6 +21,7 @@ import ( kargoapi "github.com/akuity/kargo/api/v1alpha1" "github.com/akuity/kargo/pkg/api" "github.com/akuity/kargo/pkg/controller/freight" + "github.com/akuity/kargo/pkg/credentials" "github.com/akuity/kargo/pkg/urls" ) @@ -75,12 +76,19 @@ func DiscoveredArtifactsOperations(artifacts *kargoapi.DiscoveredArtifacts) []ex // ConfigMaps and Secrets to avoid repeated API calls. This can // improve performance when the same ConfigMaps and Secrets are accessed // multiple times within the same expression evaluation. -func DataOperations(ctx context.Context, c client.Client, cache *gocache.Cache, project string) []expr.Option { +func DataOperations( + ctx context.Context, + c client.Client, + credsDB credentials.Database, + cache *gocache.Cache, + project string, +) []expr.Option { return []expr.Option{ ConfigMap(ctx, c, cache, project), SharedConfigMap(ctx, c, cache), Secret(ctx, c, cache, project), SharedSecret(ctx, c, cache), + RepoCredentials(ctx, credsDB, cache, project), FreightMetadata(ctx, c, project), StageMetadata(ctx, c, project), } @@ -451,6 +459,28 @@ func Secret(ctx context.Context, c client.Client, cache *gocache.Cache, project ) } +// RepoCredentials returns an expr.Option that provides a `repoCredentials()` +// function for use in expressions. +// +// Unlike secret(), which returns the raw contents of a Kubernetes Secret by +// name, repoCredentials() resolves repository credentials by repository URL and +// credential type through the same credentials database used by built-in +// promotion steps. This means it transparently handles credential schemes that +// yield narrowly-scoped, short-lived credentials, such as GitHub App +// installation tokens or a cloud provider's ambient (Pod identity) credentials. +func RepoCredentials( + ctx context.Context, + credsDB credentials.Database, + cache *gocache.Cache, + project string, +) expr.Option { + return expr.Function( + "repoCredentials", + getRepoCredentials(ctx, credsDB, cache, project), + new(func(repoURL, credType string) map[string]string), + ) +} + // SemverDiff returns an expr.Option that provides a `semverDiff()` function for // use in expressions. // @@ -1060,6 +1090,104 @@ func isGenericSecretType(secret corev1.Secret) bool { return secret.Labels[kargoapi.LabelKeyCredentialType] == kargoapi.LabelValueCredentialTypeGeneric } +// getRepoCredentials returns a function that resolves repository credentials by +// repository URL and credential type within the specified project namespace. It +// delegates to the provided credentials database, which is the same component +// used by built-in promotion steps, so the returned credentials may be +// narrowly-scoped, short-lived credentials derived from the underlying Secret +// rather than the Secret's raw contents. The credentials are returned as a map +// with "username", "password", and (when set) "sshPrivateKey" keys. If no +// credentials are found, an empty map is returned. +// +// If a cache is provided, it will be used to store the resolved credentials to +// avoid repeated lookups within the same evaluation. The cache key is generated +// based on a prefix, project name, credential type, and repository URL, so the +// same cache can be shared with other functions that accept a cache parameter +// without worrying about key collisions. +func getRepoCredentials( + ctx context.Context, + credsDB credentials.Database, + cache *gocache.Cache, + project string, +) exprFn { + return func(a ...any) (any, error) { + if len(a) != 2 { + return nil, fmt.Errorf("expected 2 arguments, got %d", len(a)) + } + + repoURL, ok := a[0].(string) + if !ok { + return nil, fmt.Errorf("first argument must be string, got %T", a[0]) + } + + credTypeStr, ok := a[1].(string) + if !ok { + return nil, fmt.Errorf("second argument must be string, got %T", a[1]) + } + + credType := credentials.Type(credTypeStr) + switch credType { + case credentials.TypeGit, credentials.TypeHelm, credentials.TypeImage: + default: + return nil, fmt.Errorf( + "invalid credential type %q; must be one of %q, %q, or %q", + credTypeStr, + credentials.TypeGit, + credentials.TypeHelm, + credentials.TypeImage, + ) + } + + // This function is registered wherever secret() is available, but not + // every one of those contexts wires a credentials database. Fail + // explicitly rather than silently returning no credentials. + if credsDB == nil { + return nil, fmt.Errorf( + "repoCredentials is not available in this context", + ) + } + + cacheKey := getCacheKey( + cacheKeyPrefixRepoCredentials, + project, + fmt.Sprintf("%s/%s", credType, repoURL), + ) + if cache != nil { + if cachedData, ok := cache.Get(cacheKey); ok { + if cachedData == nil { + return map[string]string{}, nil + } + if data, ok := cachedData.(map[string]string); ok { + return maps.Clone(data), nil + } + } + } + + creds, err := credsDB.Get(ctx, project, credType, repoURL) + if err != nil { + return nil, fmt.Errorf( + "error getting %s credentials for %q: %w", credType, repoURL, err, + ) + } + + // When credentials are found, always return the full, fixed set of keys + // so that expression authors can rely on a predictable shape regardless + // of credential type or provider. When no credentials are found, an empty + // map is returned, mirroring the behavior of secret(). + data := make(map[string]string) + if creds != nil { + data[credentials.FieldUsername] = creds.Username + data[credentials.FieldPassword] = creds.Password + data[credFieldSSHPrivateKey] = creds.SSHPrivateKey + } + + if cache != nil { + cache.Set(cacheKey, maps.Clone(data), gocache.NoExpiration) + } + return data, nil + } +} + func hasFailure(stepExecMetas kargoapi.StepExecutionMetadataList) exprFn { return func(a ...any) (any, error) { if len(a) != 0 { @@ -1171,10 +1299,16 @@ func semverParse(a ...any) (any, error) { } const ( - cacheKeyPrefixConfigMap = "ConfigMap" - cacheKeyPrefixSecret = "Secret" + cacheKeyPrefixConfigMap = "ConfigMap" + cacheKeyPrefixSecret = "Secret" + cacheKeyPrefixRepoCredentials = "RepoCredentials" ) +// credFieldSSHPrivateKey is the key used for the SSH private key in the map +// returned by the repoCredentials() function. Username and password use the +// credentials.FieldUsername and credentials.FieldPassword constants. +const credFieldSSHPrivateKey = "sshPrivateKey" + // getCacheKey generates a cache key for the given prefix, project, and name. // The cache key is a string formatted as "//". func getCacheKey(prefix, project, name string) string { diff --git a/pkg/expressions/function/functions_test.go b/pkg/expressions/function/functions_test.go index 2cd7946cd6..c638cc3d29 100644 --- a/pkg/expressions/function/functions_test.go +++ b/pkg/expressions/function/functions_test.go @@ -1,6 +1,8 @@ package function import ( + "context" + "errors" "testing" "time" @@ -16,6 +18,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" kargoapi "github.com/akuity/kargo/api/v1alpha1" + "github.com/akuity/kargo/pkg/credentials" ) func Test_warehouse(t *testing.T) { @@ -1459,6 +1462,230 @@ func Test_getSecret(t *testing.T) { } } +func Test_getRepoCredentials(t *testing.T) { + const testProject = "fake-project" + const testRepoURL = "https://github.com/example/repo.git" + + cacheKey := getCacheKey( + cacheKeyPrefixRepoCredentials, + testProject, + string(credentials.TypeGit)+"/"+testRepoURL, + ) + + tests := []struct { + name string + credsDB credentials.Database + cache *cache.Cache + args []any + assertions func(t *testing.T, cache *cache.Cache, result any, err error) + }{ + { + name: "no arguments", + credsDB: &credentials.FakeDB{}, + args: []any{}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.ErrorContains(t, err, "expected 2 arguments") + assert.Nil(t, result) + }, + }, + { + name: "too many arguments", + credsDB: &credentials.FakeDB{}, + args: []any{testRepoURL, "git", "extra"}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.ErrorContains(t, err, "expected 2 arguments") + assert.Nil(t, result) + }, + }, + { + name: "invalid repo URL argument type", + credsDB: &credentials.FakeDB{}, + args: []any{123, "git"}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.ErrorContains(t, err, "first argument must be string") + assert.Nil(t, result) + }, + }, + { + name: "invalid credential type argument type", + credsDB: &credentials.FakeDB{}, + args: []any{testRepoURL, 123}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.ErrorContains(t, err, "second argument must be string") + assert.Nil(t, result) + }, + }, + { + name: "invalid credential type value", + credsDB: &credentials.FakeDB{}, + args: []any{testRepoURL, "bogus"}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.ErrorContains(t, err, `invalid credential type "bogus"`) + assert.Nil(t, result) + }, + }, + { + name: "nil credentials database", + credsDB: nil, + args: []any{testRepoURL, "git"}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.ErrorContains(t, err, "repoCredentials is not available") + assert.Nil(t, result) + }, + }, + { + name: "error from credentials database", + credsDB: &credentials.FakeDB{ + GetFn: func( + context.Context, + string, + credentials.Type, + string, + ) (*credentials.Credentials, error) { + return nil, errors.New("something went wrong") + }, + }, + args: []any{testRepoURL, "git"}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.ErrorContains(t, err, "error getting git credentials") + assert.ErrorContains(t, err, "something went wrong") + assert.Nil(t, result) + }, + }, + { + name: "credentials not found", + credsDB: &credentials.FakeDB{}, + args: []any{testRepoURL, "git"}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.NoError(t, err) + assert.Equal(t, map[string]string{}, result) + }, + }, + { + name: "success with username and password", + credsDB: &credentials.FakeDB{ + GetFn: func( + _ context.Context, + namespace string, + credType credentials.Type, + repo string, + ) (*credentials.Credentials, error) { + assert.Equal(t, testProject, namespace) + assert.Equal(t, credentials.TypeGit, credType) + assert.Equal(t, testRepoURL, repo) + return &credentials.Credentials{ + Username: "user", + Password: "token", + }, nil + }, + }, + args: []any{testRepoURL, "git"}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.NoError(t, err) + // The full, fixed set of keys is always returned when credentials + // are found, even when a field is empty. + assert.Equal(t, map[string]string{ + "username": "user", + "password": "token", + "sshPrivateKey": "", + }, result) + }, + }, + { + name: "success includes SSH private key when set", + credsDB: &credentials.FakeDB{ + GetFn: func( + context.Context, + string, + credentials.Type, + string, + ) (*credentials.Credentials, error) { + return &credentials.Credentials{ + Username: "user", + Password: "token", + SSHPrivateKey: "private-key", + }, nil + }, + }, + args: []any{testRepoURL, "git"}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.NoError(t, err) + assert.Equal(t, map[string]string{ + "username": "user", + "password": "token", + "sshPrivateKey": "private-key", + }, result) + }, + }, + { + name: "success with cache", + credsDB: &credentials.FakeDB{ + GetFn: func( + context.Context, + string, + credentials.Type, + string, + ) (*credentials.Credentials, error) { + return &credentials.Credentials{ + Username: "user", + Password: "token", + }, nil + }, + }, + cache: cache.New(cache.NoExpiration, cache.NoExpiration), + args: []any{testRepoURL, "git"}, + assertions: func(t *testing.T, cache *cache.Cache, result any, err error) { + assert.NoError(t, err) + assert.Equal(t, map[string]string{ + "username": "user", + "password": "token", + "sshPrivateKey": "", + }, result) + + data, ok := cache.Get(cacheKey) + assert.True(t, ok) + assert.Equal(t, map[string]string{ + "username": "user", + "password": "token", + "sshPrivateKey": "", + }, data) + }, + }, + { + name: "success from cache", + credsDB: &credentials.FakeDB{ + GetFn: func( + context.Context, + string, + credentials.Type, + string, + ) (*credentials.Credentials, error) { + // This should not be used, as the value comes from the cache. + return &credentials.Credentials{Username: "should-not-be-used"}, nil + }, + }, + cache: cache.NewFrom(cache.NoExpiration, cache.NoExpiration, map[string]cache.Item{ + cacheKey: { + Object: map[string]string{"username": "cached-user"}, + }, + }), + args: []any{testRepoURL, "git"}, + assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { + assert.NoError(t, err) + assert.Equal(t, map[string]string{"username": "cached-user"}, result) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fn := getRepoCredentials(t.Context(), tt.credsDB, tt.cache, testProject) + result, err := fn(tt.args...) + tt.assertions(t, tt.cache, result, err) + }) + } +} + func Test_getConfigMap_getSecret_no_cache_key_collision(t *testing.T) { const testProject = "fake-project" const testIdenticalName = "fake-name" diff --git a/pkg/indexer/indexer.go b/pkg/indexer/indexer.go index 6f7fafb317..b3c9b05419 100644 --- a/pkg/indexer/indexer.go +++ b/pkg/indexer/indexer.go @@ -206,7 +206,7 @@ func RunningPromotionsByArgoCDApplications( Config: rawConfig, } - evaluator := promotion.NewStepEvaluator(cl, nil) + evaluator := promotion.NewStepEvaluator(cl, nil, nil) // As step-level variables are allowed to reference to output, we // need to provide the state. diff --git a/pkg/promotion/evaluator.go b/pkg/promotion/evaluator.go index 556df6116c..1b44af7ef0 100644 --- a/pkg/promotion/evaluator.go +++ b/pkg/promotion/evaluator.go @@ -12,6 +12,7 @@ import ( kargoapi "github.com/akuity/kargo/api/v1alpha1" "github.com/akuity/kargo/pkg/api" + "github.com/akuity/kargo/pkg/credentials" "github.com/akuity/kargo/pkg/expressions" exprfn "github.com/akuity/kargo/pkg/expressions/function" ) @@ -25,19 +26,27 @@ import ( // expressions within the same step access the same data, such as Secrets or // ConfigMaps. type StepEvaluator struct { - client client.Client - cache *gocache.Cache + client client.Client + credsDB credentials.Database + cache *gocache.Cache } // NewStepEvaluator creates a new StepEvaluator instance with the provided -// Kubernetes client and cache. The cache is optional, and can be used to -// store Kubernetes objects that are frequently accessed by the expression -// evaluation logic, such as Secrets and ConfigMaps, to avoid unnecessary API -// calls and improve performance. -func NewStepEvaluator(cl client.Client, cache *gocache.Cache) *StepEvaluator { +// Kubernetes client, credentials database, and cache. The credentials database +// is optional and, when provided, backs the repoCredentials() expression +// function; when nil, that function returns an error if invoked. The cache is +// optional, and can be used to store data that is frequently accessed by the +// expression evaluation logic, such as Secrets and ConfigMaps, to avoid +// unnecessary API calls and improve performance. +func NewStepEvaluator( + cl client.Client, + credsDB credentials.Database, + cache *gocache.Cache, +) *StepEvaluator { return &StepEvaluator{ - client: cl, - cache: cache, + client: cl, + credsDB: credsDB, + cache: cache, } } @@ -172,7 +181,7 @@ func (p *StepEvaluator) Vars(ctx context.Context, promoCtx Context, step Step) ( // evaluation. These functions provide access to data operations, freight // operations, and utility functions. exprOpts := slices.Concat( - exprfn.DataOperations(ctx, p.client, p.cache, promoCtx.Project), + exprfn.DataOperations(ctx, p.client, p.credsDB, p.cache, promoCtx.Project), exprfn.FreightOperations( ctx, p.client, promoCtx.Project, promoCtx.FreightRequests, promoCtx.Freight.References(), ), @@ -246,7 +255,7 @@ func (p *StepEvaluator) ShouldSkip(ctx context.Context, promoCtx Context, step S step.If, env, slices.Concat( - exprfn.DataOperations(ctx, p.client, p.cache, promoCtx.Project), + exprfn.DataOperations(ctx, p.client, p.credsDB, p.cache, promoCtx.Project), exprfn.FreightOperations( ctx, p.client, @@ -304,7 +313,7 @@ func (p *StepEvaluator) Config(ctx context.Context, promoCtx Context, step Step) promoCtx.FreightRequests, promoCtx.Freight.References(), ), - exprfn.DataOperations(ctx, p.client, p.cache, promoCtx.Project), + exprfn.DataOperations(ctx, p.client, p.credsDB, p.cache, promoCtx.Project), exprfn.StatusOperations(step.Alias, promoCtx.StepExecutionMetadata), exprfn.UtilityOperations(), )..., diff --git a/pkg/promotion/evaluator_test.go b/pkg/promotion/evaluator_test.go index 7887e7d91d..e87246866e 100644 --- a/pkg/promotion/evaluator_test.go +++ b/pkg/promotion/evaluator_test.go @@ -934,7 +934,7 @@ func TestStepEvaluator_Vars(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - evaluator := NewStepEvaluator(testClient, nil) + evaluator := NewStepEvaluator(testClient, nil, nil) vars, err := evaluator.Vars( t.Context(), tt.promoCtx, @@ -1098,6 +1098,7 @@ func TestStepEvaluator_ShouldSkip(t *testing.T) { evaluator := NewStepEvaluator( fake.NewClientBuilder().Build(), nil, + nil, ) got, err := evaluator.ShouldSkip( t.Context(), @@ -1674,7 +1675,7 @@ func TestStepEvaluator_Config(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - evaluator := NewStepEvaluator(testClient, nil) + evaluator := NewStepEvaluator(testClient, nil, nil) stepCfg, err := evaluator.Config( t.Context(), tt.promoCtx, @@ -1809,7 +1810,7 @@ func TestStepEvaluator_BuildStepContext(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - evaluator := NewStepEvaluator(testClient, nil) + evaluator := NewStepEvaluator(testClient, nil, nil) stepCtx, err := evaluator.BuildStepContext( t.Context(), tt.promoCtx, diff --git a/pkg/promotion/local_orchestrator.go b/pkg/promotion/local_orchestrator.go index 24358b589b..531fe65ff4 100644 --- a/pkg/promotion/local_orchestrator.go +++ b/pkg/promotion/local_orchestrator.go @@ -21,6 +21,7 @@ type LocalOrchestrator struct { executor StepExecutor registry StepRunnerRegistry client client.Client + credsDB credentials.Database cacheFunc ExprDataCacheFn } @@ -43,6 +44,7 @@ func NewLocalOrchestrator( ), registry: registry, client: kargoClient, + credsDB: credsDB, cacheFunc: cacheFunc, } } @@ -95,7 +97,7 @@ func (o *LocalOrchestrator) ExecuteSteps( // Continue execution if the context is still active. } - processor := NewStepEvaluator(o.client, o.newCache()) + processor := NewStepEvaluator(o.client, o.credsDB, o.newCache()) // Only evaluate the "if" conditio when the step has not yet started. // If the step has already started (on a previous reconciliation), we From 6ecfff2d0288eedce159f9094c44b5da2f4b3fda Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Thu, 9 Jul 2026 09:38:10 -0400 Subject: [PATCH 2/2] feat(credentials): integrate credentials database into RegularStageReconciler Signed-off-by: Justin Brooks --- cmd/controlplane/controller.go | 1 + .../60-reference-docs/40-expressions.md | 30 ++-- pkg/controller/stages/regular_stages.go | 9 +- pkg/controller/stages/regular_stages_test.go | 136 +++++++++++++++++- pkg/expressions/function/functions.go | 38 ++--- pkg/expressions/function/functions_test.go | 37 +++-- 6 files changed, 188 insertions(+), 63 deletions(-) diff --git a/cmd/controlplane/controller.go b/cmd/controlplane/controller.go index 1909985512..7c0335d3ff 100644 --- a/cmd/controlplane/controller.go +++ b/cmd/controlplane/controller.go @@ -457,6 +457,7 @@ func (o *controllerOptions) setupReconcilers( if err := stages.NewRegularStageReconciler( stagesReconcilerCfg, + credentialsDB, health.NewAggregatingChecker(), ).SetupWithManager( ctx, diff --git a/docs/docs/50-user-guide/60-reference-docs/40-expressions.md b/docs/docs/50-user-guide/60-reference-docs/40-expressions.md index ae9c3a5ff9..b914c68d12 100644 --- a/docs/docs/50-user-guide/60-reference-docs/40-expressions.md +++ b/docs/docs/50-user-guide/60-reference-docs/40-expressions.md @@ -466,8 +466,8 @@ config: ### `repoCredentials(repoURL, type)` The `repoCredentials()` function resolves repository credentials by repository -URL and credential type, returning them as a `map[string]string`. It takes two -required arguments: +URL and credential type, returning them as an object whose fields are accessed +by name. It takes two required arguments: - `repoURL` (Required): A string representing the URL of the repository whose credentials should be resolved. @@ -487,17 +487,18 @@ tokens or a cloud provider's ambient (Pod identity) credentials — instead of returning the raw material (e.g. an app ID and private key) from which those credentials are derived. -Because the result is normalized, the set of available keys is **fixed** and -does not vary by credential type or provider. When credentials are found, the -returned map always contains all of the following keys: +Because the result is normalized, the set of available fields is **fixed** and +does not vary by credential type or provider. The returned object always exposes +all of the following fields: -| Key | Description | -|-----|-------------| -| `username` | The username identifying the principal. For token-based credentials this is often an inconsequential placeholder. | -| `password` | The password or token used to authenticate. **API keys and personal access tokens are surfaced here.** | -| `sshPrivateKey` | The SSH private key, when applicable. **Deprecated as of v1.10.0** and slated for removal in v1.13.0. | +| Field | Description | +|-------|-------------| +| `Username` | The username identifying the principal. For token-based credentials this is often an inconsequential placeholder. | +| `Password` | The password or token used to authenticate. **API keys and personal access tokens are surfaced here.** | +| `SSHPrivateKey` | The SSH private key, when applicable. **Deprecated as of v1.10.0** and slated for removal in v1.13.0. | -If no matching credentials are found, an empty map is returned. +If no matching credentials are found, an object with all fields empty is +returned. For details on how each field is populated for a given credential type or provider (e.g. GitHub App, ECR, or basic username/password), see the @@ -510,13 +511,14 @@ Examples: config: headers: - name: Authorization - value: Bearer ${{ repoCredentials('https://github.com/example/repo.git', 'git').password }} + value: Bearer ${{ repoCredentials('https://github.com/example/repo.git', 'git').Password }} ``` :::note -`repoCredentials()` is only available within `Promotion` step expressions, where -Kargo's credentials database is available. +`repoCredentials()` is available wherever Kargo's credentials database is wired: +within `Promotion` step expressions and within a `Stage`'s verification argument +expressions. ::: diff --git a/pkg/controller/stages/regular_stages.go b/pkg/controller/stages/regular_stages.go index e71d7156a1..0cab821612 100644 --- a/pkg/controller/stages/regular_stages.go +++ b/pkg/controller/stages/regular_stages.go @@ -32,6 +32,7 @@ import ( "github.com/akuity/kargo/pkg/conditions" "github.com/akuity/kargo/pkg/controller" argocdapi "github.com/akuity/kargo/pkg/controller/argocd/api/v1alpha1" + "github.com/akuity/kargo/pkg/credentials" kargoEvent "github.com/akuity/kargo/pkg/event" k8sevent "github.com/akuity/kargo/pkg/event/kubernetes" exprfn "github.com/akuity/kargo/pkg/expressions/function" @@ -77,6 +78,7 @@ func ReconcilerConfigFromEnv() ReconcilerConfig { type RegularStageReconciler struct { cfg ReconcilerConfig client client.Client + credentialsDB credentials.Database eventSender kargoEvent.Sender healthChecker health.AggregatingChecker shardPredicate controller.ResponsibleFor[kargoapi.Stage] @@ -87,10 +89,12 @@ type RegularStageReconciler struct { // NewRegularStageReconciler creates a new Stages reconciler. func NewRegularStageReconciler( cfg ReconcilerConfig, + credentialsDB credentials.Database, healthChecker health.AggregatingChecker, ) *RegularStageReconciler { return &RegularStageReconciler{ cfg: cfg, + credentialsDB: credentialsDB, healthChecker: healthChecker, shardPredicate: controller.ResponsibleFor[kargoapi.Stage]{ IsDefaultController: cfg.IsDefaultController, @@ -1386,10 +1390,7 @@ func (r *RegularStageReconciler) startVerification( exprfn.DataOperations( ctx, r.client, - // The Stage reconciler does not wire a credentials database, - // so repoCredentials() is unavailable during verification - // argument evaluation. - nil, + r.credentialsDB, gocache.New(gocache.NoExpiration, gocache.NoExpiration), stage.Namespace, ), diff --git a/pkg/controller/stages/regular_stages_test.go b/pkg/controller/stages/regular_stages_test.go index 7d60ec89ee..10e4b358d9 100644 --- a/pkg/controller/stages/regular_stages_test.go +++ b/pkg/controller/stages/regular_stages_test.go @@ -24,6 +24,7 @@ import ( rolloutsapi "github.com/akuity/kargo/api/stubs/rollouts/v1alpha1" kargoapi "github.com/akuity/kargo/api/v1alpha1" "github.com/akuity/kargo/pkg/conditions" + "github.com/akuity/kargo/pkg/credentials" k8sevent "github.com/akuity/kargo/pkg/event/kubernetes" "github.com/akuity/kargo/pkg/health" "github.com/akuity/kargo/pkg/indexer" @@ -3622,6 +3623,7 @@ func TestRegularStageReconciler_startVerification(t *testing.T) { freightCol kargoapi.FreightCollection req *kargoapi.VerificationRequest objects []client.Object + credsDB credentials.Database rolloutsDisabled bool assertions func(*testing.T, client.Client, *kargoapi.VerificationInfo, error) }{ @@ -3988,6 +3990,137 @@ func TestRegularStageReconciler_startVerification(t *testing.T) { ) }, }, + { + name: "resolves repoCredentials() in verification arguments", + stage: &kargoapi.Stage{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-project", + Name: "test-stage", + }, + Spec: kargoapi.StageSpec{ + Verification: &kargoapi.Verification{ + AnalysisTemplates: []kargoapi.AnalysisTemplateReference{ + {Name: "test-template"}, + }, + Args: []kargoapi.AnalysisRunArgument{ + { + Name: "token", + Value: "${{ repoCredentials(" + + "'https://github.com/example/repo.git', 'git'" + + ").Password }}", + }, + }, + }, + }, + }, + freightCol: kargoapi.FreightCollection{ + ID: "test-collection", + Freight: map[string]kargoapi.FreightReference{ + "warehouse": {Name: "test-freight"}, + }, + }, + objects: []client.Object{ + &kargoapi.Freight{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-freight", + Namespace: "fake-project", + }, + }, + &rolloutsapi.AnalysisTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-template", + Namespace: "fake-project", + }, + Spec: rolloutsapi.AnalysisTemplateSpec{ + Args: []rolloutsapi.Argument{{Name: "token"}}, + }, + }, + }, + credsDB: &credentials.FakeDB{ + GetFn: func( + context.Context, + string, + credentials.Type, + string, + ) (*credentials.Credentials, error) { + return &credentials.Credentials{Password: "s3cr3t"}, nil + }, + }, + assertions: func(t *testing.T, c client.Client, vi *kargoapi.VerificationInfo, err error) { + require.NoError(t, err) + + require.NotNil(t, vi) + assert.Equal(t, kargoapi.VerificationPhasePending, vi.Phase) + require.NotNil(t, vi.AnalysisRun) + + ar := &rolloutsapi.AnalysisRun{} + require.NoError(t, c.Get(t.Context(), types.NamespacedName{ + Namespace: vi.AnalysisRun.Namespace, + Name: vi.AnalysisRun.Name, + }, ar)) + + require.Len(t, ar.Spec.Args, 1) + assert.Equal(t, "token", ar.Spec.Args[0].Name) + require.NotNil(t, ar.Spec.Args[0].Value) + assert.Equal(t, "s3cr3t", *ar.Spec.Args[0].Value) + }, + }, + { + name: "surfaces error when repoCredentials() is unavailable", + stage: &kargoapi.Stage{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-project", + Name: "test-stage", + }, + Spec: kargoapi.StageSpec{ + Verification: &kargoapi.Verification{ + AnalysisTemplates: []kargoapi.AnalysisTemplateReference{ + {Name: "test-template"}, + }, + Args: []kargoapi.AnalysisRunArgument{ + { + Name: "token", + Value: "${{ repoCredentials(" + + "'https://github.com/example/repo.git', 'git'" + + ").Password }}", + }, + }, + }, + }, + }, + freightCol: kargoapi.FreightCollection{ + ID: "test-collection", + Freight: map[string]kargoapi.FreightReference{ + "warehouse": {Name: "test-freight"}, + }, + }, + objects: []client.Object{ + &kargoapi.Freight{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-freight", + Namespace: "fake-project", + }, + }, + &rolloutsapi.AnalysisTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-template", + Namespace: "fake-project", + }, + Spec: rolloutsapi.AnalysisTemplateSpec{ + Args: []rolloutsapi.Argument{{Name: "token"}}, + }, + }, + }, + // credsDB intentionally left nil. + assertions: func(t *testing.T, _ client.Client, vi *kargoapi.VerificationInfo, err error) { + require.NoError(t, err) + + require.NotNil(t, vi) + assert.Equal(t, kargoapi.VerificationPhaseError, vi.Phase) + assert.Contains(t, vi.Message, "error building AnalysisRun") + assert.Contains(t, vi.Message, "repoCredentials is not available") + }, + }, { name: "handles analysis run build error", stage: &kargoapi.Stage{ @@ -4027,7 +4160,8 @@ func TestRegularStageReconciler_startVerification(t *testing.T) { Build() r := &RegularStageReconciler{ - client: c, + client: c, + credentialsDB: tt.credsDB, cfg: ReconcilerConfig{ RolloutsIntegrationEnabled: !tt.rolloutsDisabled, RolloutsControllerInstanceID: "test-instance", diff --git a/pkg/expressions/function/functions.go b/pkg/expressions/function/functions.go index 7145a32682..5703c955b0 100644 --- a/pkg/expressions/function/functions.go +++ b/pkg/expressions/function/functions.go @@ -1095,9 +1095,11 @@ func isGenericSecretType(secret corev1.Secret) bool { // delegates to the provided credentials database, which is the same component // used by built-in promotion steps, so the returned credentials may be // narrowly-scoped, short-lived credentials derived from the underlying Secret -// rather than the Secret's raw contents. The credentials are returned as a map -// with "username", "password", and (when set) "sshPrivateKey" keys. If no -// credentials are found, an empty map is returned. +// rather than the Secret's raw contents. The credentials are returned as a +// credentials.Credentials struct, whose fields are accessed by name (e.g. +// .Username, .Password). If no credentials are found, a zero-value struct (all +// fields empty) is returned, so expression authors can rely on a predictable +// shape regardless of whether credentials were found. // // If a cache is provided, it will be used to store the resolved credentials to // avoid repeated lookups within the same evaluation. The cache key is generated @@ -1154,11 +1156,8 @@ func getRepoCredentials( ) if cache != nil { if cachedData, ok := cache.Get(cacheKey); ok { - if cachedData == nil { - return map[string]string{}, nil - } - if data, ok := cachedData.(map[string]string); ok { - return maps.Clone(data), nil + if creds, ok := cachedData.(credentials.Credentials); ok { + return creds, nil } } } @@ -1170,21 +1169,19 @@ func getRepoCredentials( ) } - // When credentials are found, always return the full, fixed set of keys - // so that expression authors can rely on a predictable shape regardless - // of credential type or provider. When no credentials are found, an empty - // map is returned, mirroring the behavior of secret(). - data := make(map[string]string) + // Return the resolved credentials as a value struct so that expression + // authors can rely on a predictable shape regardless of credential type + // or provider. When no credentials are found, a zero-value struct (all + // fields empty) is returned. + var result credentials.Credentials if creds != nil { - data[credentials.FieldUsername] = creds.Username - data[credentials.FieldPassword] = creds.Password - data[credFieldSSHPrivateKey] = creds.SSHPrivateKey + result = *creds } if cache != nil { - cache.Set(cacheKey, maps.Clone(data), gocache.NoExpiration) + cache.Set(cacheKey, result, gocache.NoExpiration) } - return data, nil + return result, nil } } @@ -1304,11 +1301,6 @@ const ( cacheKeyPrefixRepoCredentials = "RepoCredentials" ) -// credFieldSSHPrivateKey is the key used for the SSH private key in the map -// returned by the repoCredentials() function. Username and password use the -// credentials.FieldUsername and credentials.FieldPassword constants. -const credFieldSSHPrivateKey = "sshPrivateKey" - // getCacheKey generates a cache key for the given prefix, project, and name. // The cache key is a string formatted as "//". func getCacheKey(prefix, project, name string) string { diff --git a/pkg/expressions/function/functions_test.go b/pkg/expressions/function/functions_test.go index c638cc3d29..3ade5fd721 100644 --- a/pkg/expressions/function/functions_test.go +++ b/pkg/expressions/function/functions_test.go @@ -1558,7 +1558,7 @@ func Test_getRepoCredentials(t *testing.T) { args: []any{testRepoURL, "git"}, assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { assert.NoError(t, err) - assert.Equal(t, map[string]string{}, result) + assert.Equal(t, credentials.Credentials{}, result) }, }, { @@ -1582,12 +1582,9 @@ func Test_getRepoCredentials(t *testing.T) { args: []any{testRepoURL, "git"}, assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { assert.NoError(t, err) - // The full, fixed set of keys is always returned when credentials - // are found, even when a field is empty. - assert.Equal(t, map[string]string{ - "username": "user", - "password": "token", - "sshPrivateKey": "", + assert.Equal(t, credentials.Credentials{ + Username: "user", + Password: "token", }, result) }, }, @@ -1610,10 +1607,10 @@ func Test_getRepoCredentials(t *testing.T) { args: []any{testRepoURL, "git"}, assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { assert.NoError(t, err) - assert.Equal(t, map[string]string{ - "username": "user", - "password": "token", - "sshPrivateKey": "private-key", + assert.Equal(t, credentials.Credentials{ + Username: "user", + Password: "token", + SSHPrivateKey: "private-key", }, result) }, }, @@ -1636,18 +1633,16 @@ func Test_getRepoCredentials(t *testing.T) { args: []any{testRepoURL, "git"}, assertions: func(t *testing.T, cache *cache.Cache, result any, err error) { assert.NoError(t, err) - assert.Equal(t, map[string]string{ - "username": "user", - "password": "token", - "sshPrivateKey": "", + assert.Equal(t, credentials.Credentials{ + Username: "user", + Password: "token", }, result) data, ok := cache.Get(cacheKey) assert.True(t, ok) - assert.Equal(t, map[string]string{ - "username": "user", - "password": "token", - "sshPrivateKey": "", + assert.Equal(t, credentials.Credentials{ + Username: "user", + Password: "token", }, data) }, }, @@ -1666,13 +1661,13 @@ func Test_getRepoCredentials(t *testing.T) { }, cache: cache.NewFrom(cache.NoExpiration, cache.NoExpiration, map[string]cache.Item{ cacheKey: { - Object: map[string]string{"username": "cached-user"}, + Object: credentials.Credentials{Username: "cached-user"}, }, }), args: []any{testRepoURL, "git"}, assertions: func(t *testing.T, _ *cache.Cache, result any, err error) { assert.NoError(t, err) - assert.Equal(t, map[string]string{"username": "cached-user"}, result) + assert.Equal(t, credentials.Credentials{Username: "cached-user"}, result) }, }, }