From e99f274a47ef934159106b75abbd19e590cc54a8 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Mon, 13 Jul 2026 13:00:54 +0200 Subject: [PATCH 1/4] fix(eks): honor custom AWS credential mappings --- pkg/cli/cmd/cluster/info.go | 30 +- pkg/cli/cmd/cluster/info_test.go | 107 +++++++ pkg/cli/lifecycle/awsregion_test.go | 58 ++++ pkg/cli/lifecycle/simple.go | 16 +- pkg/client/eks/client.go | 79 ++++- pkg/client/eks/client_test.go | 91 ++++++ pkg/client/eks/errors.go | 11 + pkg/client/eksctl/client.go | 186 +++++++++++- pkg/client/eksctl/client_test.go | 204 +++++++++++++ pkg/client/eksctl/errors.go | 19 ++ .../clusterdiscovery/aws_environment_test.go | 94 ++++++ pkg/svc/clusterdiscovery/cloud.go | 24 +- pkg/svc/credentials/aws_environment_test.go | 277 ++++++++++++++++++ pkg/svc/credentials/credentials.go | 192 +++++++++++- pkg/svc/credentials/credentials_test.go | 1 + pkg/svc/credentials/environment_name_test.go | 15 + pkg/svc/provider/aws/provider.go | 42 ++- pkg/svc/provisioner/cluster/eks/connector.go | 7 +- .../provisioner/cluster/eks/connector_test.go | 18 ++ .../provisioner/cluster/eks/provisioner.go | 40 ++- pkg/svc/provisioner/cluster/factory_eks.go | 48 ++- .../provisioner/cluster/factory_eks_test.go | 46 ++- 22 files changed, 1563 insertions(+), 42 deletions(-) create mode 100644 pkg/svc/clusterdiscovery/aws_environment_test.go create mode 100644 pkg/svc/credentials/aws_environment_test.go create mode 100644 pkg/svc/credentials/environment_name_test.go diff --git a/pkg/cli/cmd/cluster/info.go b/pkg/cli/cmd/cluster/info.go index 3549479761..c02c1d3468 100644 --- a/pkg/cli/cmd/cluster/info.go +++ b/pkg/cli/cmd/cluster/info.go @@ -17,6 +17,7 @@ import ( "github.com/devantler-tech/ksail/v7/pkg/client/kubectl" "github.com/devantler-tech/ksail/v7/pkg/fsutil" "github.com/devantler-tech/ksail/v7/pkg/notify" + "github.com/devantler-tech/ksail/v7/pkg/svc/credentials" clusterdetector "github.com/devantler-tech/ksail/v7/pkg/svc/detector/cluster" "github.com/devantler-tech/ksail/v7/pkg/svc/provider" awsprovider "github.com/devantler-tech/ksail/v7/pkg/svc/provider/aws" @@ -81,6 +82,7 @@ func runInfoCmd( resolved.Provider, resolved.ClusterName, resolved.OmniOpts, + resolved.AWSOpts, resolved.AWSRegion, ) @@ -211,6 +213,7 @@ func getProviderStatus( prov v1alpha1.Provider, clusterName string, omniOpts v1alpha1.OptionsOmni, + awsOpts v1alpha1.OptionsAWS, awsRegion string, ) (*provider.ClusterStatus, error) { switch prov { @@ -221,7 +224,7 @@ func getProviderStatus( case v1alpha1.ProviderOmni: return getOmniProviderStatus(cmd.Context(), clusterName, omniOpts) case v1alpha1.ProviderAWS: - return getAWSProviderStatus(cmd.Context(), clusterName, awsRegion) + return getAWSProviderStatus(cmd.Context(), clusterName, awsOpts, awsRegion) case v1alpha1.ProviderGCP, v1alpha1.ProviderAzure: // GCP/GKE and Azure/AKS status inspection is not yet implemented. Return // a minimal stub so callers that rely on this helper do not fail for them. @@ -335,9 +338,32 @@ func getOmniProviderStatus( func getAWSProviderStatus( ctx context.Context, clusterName string, + awsOpts v1alpha1.OptionsAWS, region string, ) (*provider.ClusterStatus, error) { - return awsProviderStatus(ctx, eksctlclient.NewClient(), clusterName, region) + auth := credentials.ResolveAWS(credentials.NewAWSOptionsResolver(awsOpts)) + eksctlOptions := []eksctlclient.Option{ + eksctlclient.WithEnvironment(auth.ChildEnvironment(os.Environ())), + } + + providerOptions := []awsprovider.Option{awsprovider.WithCredentialValues( + auth.Profile, + auth.AccessKeyID, + auth.SecretAccessKey, + auth.SessionToken, + )} + if auth.HasCustomCredentialSources() { + eksctlOptions = append(eksctlOptions, eksctlclient.RequireCredentialValues()) + providerOptions = append(providerOptions, awsprovider.RequireCredentialValues()) + } + + return awsProviderStatus( + ctx, + eksctlclient.NewClient(eksctlOptions...), + clusterName, + region, + providerOptions..., + ) } // awsProviderStatus is the injectable core of getAWSProviderStatus: it accepts diff --git a/pkg/cli/cmd/cluster/info_test.go b/pkg/cli/cmd/cluster/info_test.go index f6125a20f1..c3506e3bf7 100644 --- a/pkg/cli/cmd/cluster/info_test.go +++ b/pkg/cli/cmd/cluster/info_test.go @@ -5,6 +5,7 @@ import ( "errors" "io" "os" + "path/filepath" "testing" awssdk "github.com/aws/aws-sdk-go-v2/aws" @@ -25,6 +26,45 @@ var errEksctlStubEmptyArgs = errors.New("eksctl stub: empty args") // the AWS status tests never resolve real AWS credentials. const stubEndpoint = "https://ABCDEF.gr7.us-east-1.eks.amazonaws.com" +const mappedAWSEksctlFixture = `#!/bin/sh +[ "${AWS_PROFILE-}" = "selected-profile" ] || exit 41 +[ "${AWS_ACCESS_KEY_ID-}" = "fixture-access" ] || exit 42 +[ "${AWS_SECRET_ACCESS_KEY-}" = "fixture-secret" ] || exit 43 +[ "${AWS_SESSION_TOKEN-}" = "fixture-session" ] || exit 44 +[ -z "${KSAIL_PROFILE+x}" ] || exit 45 +[ -z "${KSAIL_ACCESS+x}" ] || exit 46 +[ -z "${KSAIL_SECRET+x}" ] || exit 47 +[ -z "${KSAIL_SESSION+x}" ] || exit 48 +printf mapped > "$KSAIL_EKSCTL_MARKER" +printf 'null\n' +` + +const mappedAWSClusterFixture = `apiVersion: ksail.io/v1alpha1 +kind: Cluster +metadata: + name: mapped-eks +spec: + cluster: + distribution: EKS + provider: AWS + distributionConfig: eks.yaml + connection: + kubeconfig: kubeconfig + provider: + aws: + profileEnvVar: KSAIL_PROFILE + accessKeyIdEnvVar: KSAIL_ACCESS + secretAccessKeyEnvVar: KSAIL_SECRET + sessionTokenEnvVar: KSAIL_SESSION +` + +const mappedAWSEksConfigFixture = `apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + name: mapped-eks + region: eu-west-1 +` + // stubDescriber is a credential-free stand-in for the EKS DescribeCluster // seam, returning a cluster carrying stubEndpoint. type stubDescriber struct{} @@ -179,3 +219,70 @@ func TestAWSProviderStatus_ForwardsRegion(t *testing.T) { runner.gotArgs, ) } + +func TestInfoCommandMapsCustomAWSCredentialsIntoEksctl(t *testing.T) { + workingDir := t.TempDir() + t.Chdir(workingDir) + + binDir := t.TempDir() + markerPath := filepath.Join(t.TempDir(), "mapped") + eksctlPath := filepath.Join(binDir, "eksctl") + writeExecutableFixture(t, eksctlPath, mappedAWSEksctlFixture) + require.NoError( + t, + os.WriteFile( + filepath.Join(workingDir, "ksail.yaml"), + []byte(mappedAWSClusterFixture), + 0o600, + ), + ) + require.NoError( + t, + os.WriteFile( + filepath.Join(workingDir, "eks.yaml"), + []byte(mappedAWSEksConfigFixture), + 0o600, + ), + ) + require.NoError( + t, + os.WriteFile( + filepath.Join(workingDir, "kubeconfig"), + []byte("apiVersion: v1\nkind: Config\n"), + 0o600, + ), + ) + + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("KSAIL_EKSCTL_MARKER", markerPath) + t.Setenv("KSAIL_PROFILE", "selected-profile") + t.Setenv("KSAIL_ACCESS", "fixture-access") + t.Setenv("KSAIL_SECRET", "fixture-secret") + t.Setenv("KSAIL_SESSION", "fixture-session") + t.Setenv("AWS_PROFILE", "stale-profile") + t.Setenv("AWS_ACCESS_KEY_ID", "stale-access") + t.Setenv("AWS_SECRET_ACCESS_KEY", "stale-secret") + t.Setenv("AWS_SESSION_TOKEN", "stale-session") + + cmd := cluster.NewInfoCmd() + cmd.SetArgs([]string{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + _ = cmd.Execute() + + marker, err := os.ReadFile(markerPath) //nolint:gosec // path is test-private. + require.NoError(t, err) + assert.Equal(t, "mapped", string(marker)) + assert.Equal(t, "stale-profile", os.Getenv("AWS_PROFILE")) +} + +func writeExecutableFixture(t *testing.T, path, contents string) { + t.Helper() + + require.NoError(t, os.WriteFile(path, []byte(contents), 0o600)) + require.NoError( + t, + //nolint:gosec // owner execute is required for the fixture. + os.Chmod(path, 0o700), + ) +} diff --git a/pkg/cli/lifecycle/awsregion_test.go b/pkg/cli/lifecycle/awsregion_test.go index 5ed15f34af..126e12f1a7 100644 --- a/pkg/cli/lifecycle/awsregion_test.go +++ b/pkg/cli/lifecycle/awsregion_test.go @@ -1,12 +1,15 @@ package lifecycle_test import ( + "os" + "path/filepath" "testing" "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" "github.com/devantler-tech/ksail/v7/pkg/cli/lifecycle" clusterprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestResolveAWSRegion verifies the documented precedence: the env var named by @@ -48,3 +51,58 @@ func TestResolveAWSRegion(t *testing.T) { assert.Empty(t, got) }) } + +func TestResolveClusterInfoRetainsAWSCredentialMappings(t *testing.T) { + workingDir := t.TempDir() + t.Chdir(workingDir) + t.Setenv("KSAIL_AWS_REGION", "ap-southeast-1") + + require.NoError( + t, + os.WriteFile(filepath.Join(workingDir, "ksail.yaml"), []byte(`apiVersion: ksail.io/v1alpha1 +kind: Cluster +metadata: + name: mapped-eks +spec: + cluster: + distribution: EKS + provider: AWS + distributionConfig: eks.yaml + connection: + kubeconfig: kubeconfig + provider: + aws: + profileEnvVar: KSAIL_PROFILE + regionEnvVar: KSAIL_AWS_REGION + accessKeyIdEnvVar: KSAIL_ACCESS + secretAccessKeyEnvVar: KSAIL_SECRET + sessionTokenEnvVar: KSAIL_SESSION +`), 0o600), + ) + require.NoError( + t, + os.WriteFile(filepath.Join(workingDir, "eks.yaml"), []byte(`apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + name: mapped-eks + region: eu-west-1 +`), 0o600), + ) + require.NoError( + t, + os.WriteFile( + filepath.Join(workingDir, "kubeconfig"), + []byte("apiVersion: v1\nkind: Config\n"), + 0o600, + ), + ) + + resolved, err := lifecycle.ResolveClusterInfo(nil, "", "", "") + require.NoError(t, err) + assert.Equal(t, "mapped-eks", resolved.ClusterName) + assert.Equal(t, "KSAIL_PROFILE", resolved.AWSOpts.ProfileEnvVar) + assert.Equal(t, "KSAIL_ACCESS", resolved.AWSOpts.AccessKeyIDEnvVar) + assert.Equal(t, "KSAIL_SECRET", resolved.AWSOpts.SecretAccessKeyEnvVar) + assert.Equal(t, "KSAIL_SESSION", resolved.AWSOpts.SessionTokenEnvVar) + assert.Equal(t, "ap-southeast-1", resolved.AWSRegion) +} diff --git a/pkg/cli/lifecycle/simple.go b/pkg/cli/lifecycle/simple.go index 6f59c14e69..8869d058c1 100644 --- a/pkg/cli/lifecycle/simple.go +++ b/pkg/cli/lifecycle/simple.go @@ -119,6 +119,9 @@ type ResolvedClusterInfo struct { // AWSRegion is the resolved AWS region for read-only EKS status lookups. // Empty defers region resolution to eksctl (AWS_REGION env / active profile). AWSRegion string + // AWSOpts retains the credential environment-variable mappings from the + // loaded cluster config for read-only EKS status lookups. + AWSOpts v1alpha1.OptionsAWS } // awsRegionEnvVarDefault is the fallback environment variable name for the AWS @@ -172,11 +175,19 @@ func ResolveClusterInfo( var ( omniOpts v1alpha1.OptionsOmni kubernetesOpts v1alpha1.OptionsKubernetes + awsOpts v1alpha1.OptionsAWS awsRegion string ) resolveFromConfig( - cmd, &clusterName, &provider, &kubeconfigPath, &omniOpts, &kubernetesOpts, &awsRegion, + cmd, + &clusterName, + &provider, + &kubeconfigPath, + &omniOpts, + &kubernetesOpts, + &awsOpts, + &awsRegion, ) // Fall back to kubeconfig context detection @@ -203,6 +214,7 @@ func ResolveClusterInfo( KubeconfigPath: resolvedPath, OmniOpts: omniOpts, KubernetesOpts: kubernetesOpts, + AWSOpts: awsOpts, AWSRegion: awsRegion, }, nil } @@ -239,6 +251,7 @@ func resolveFromConfig( kubeconfigPath *string, omniOpts *v1alpha1.OptionsOmni, kubernetesOpts *v1alpha1.OptionsKubernetes, + awsOpts *v1alpha1.OptionsAWS, awsRegion *string, ) { cfg, distCfg := loadConfig(cmd) @@ -266,6 +279,7 @@ func resolveFromConfig( *omniOpts = cfg.Spec.Provider.Omni *kubernetesOpts = cfg.Spec.Provider.Kubernetes + *awsOpts = cfg.Spec.Provider.AWS *awsRegion = ResolveAWSRegion(cfg.Spec.Provider.AWS, distCfg) } diff --git a/pkg/client/eks/client.go b/pkg/client/eks/client.go index 0023982d29..b47b2b5b73 100644 --- a/pkg/client/eks/client.go +++ b/pkg/client/eks/client.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" "github.com/aws/aws-sdk-go-v2/config" + awscredentials "github.com/aws/aws-sdk-go-v2/credentials" awseks "github.com/aws/aws-sdk-go-v2/service/eks" ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" "github.com/aws/aws-sdk-go-v2/service/sts" @@ -56,8 +57,12 @@ type callerIdentityPresigner interface { // Client reads EKS cluster connection details and mints bearer tokens for // them, hiding the SDK's request shapes and the token encoding scheme. type Client struct { - describer clusterDescriber - presigner callerIdentityPresigner + describer clusterDescriber + presigner callerIdentityPresigner + loadOptions []func(*config.LoadOptions) error + credentialValuesAvailable bool + requireCredentialValues bool + optionErr error } // Option customises a Client. @@ -79,21 +84,85 @@ func WithCallerIdentityPresigner(presigner callerIdentityPresigner) Option { } } +// WithCredentialValues pins the AWS identity used by the SDK-backed EKS and +// STS clients without mutating process environment. A complete static pair +// takes precedence over profile, matching the canonical AWS credential chain. +// Partial static credentials fail closed rather than falling back to an +// unrelated ambient identity. +func WithCredentialValues(profile, accessKeyID, secretAccessKey, sessionToken string) Option { + return func(client *Client) { + hasAccessKey := accessKeyID != "" + hasSecretKey := secretAccessKey != "" + + if hasAccessKey != hasSecretKey || (sessionToken != "" && !hasAccessKey) { + client.optionErr = ErrIncompleteStaticCredentials + + return + } + + if hasAccessKey { + client.credentialValuesAvailable = true + client.loadOptions = append( + client.loadOptions, + config.WithCredentialsProvider(awscredentials.NewStaticCredentialsProvider( + accessKeyID, + secretAccessKey, + sessionToken, + )), + ) + + return + } + + if profile != "" { + client.credentialValuesAvailable = true + client.loadOptions = append(client.loadOptions, config.WithSharedConfigProfile(profile)) + } + } +} + +// RequireCredentialValues makes a custom credential selection fail closed +// when it resolves neither a profile nor a complete static key pair. This +// prevents the SDK from silently falling back to stale canonical environment +// credentials that the corresponding eksctl child environment removed. +func RequireCredentialValues() Option { + return func(client *Client) { + client.requireCredentialValues = true + } +} + // NewClient constructs a Client. Unless both seams are injected, it resolves // the AWS default configuration (env, shared config, IRSA / instance // role) once and builds the real SDK clients from it. func NewClient(ctx context.Context, region string, opts ...Option) (*Client, error) { client := &Client{ - describer: nil, - presigner: nil, + describer: nil, + presigner: nil, + loadOptions: nil, + credentialValuesAvailable: false, + requireCredentialValues: false, + optionErr: nil, } for _, opt := range opts { opt(client) } + if client.optionErr != nil { + return nil, client.optionErr + } + + if client.requireCredentialValues && !client.credentialValuesAvailable { + return nil, ErrExplicitCredentialsUnavailable + } + if client.describer == nil || client.presigner == nil { - cfg, err := config.LoadDefaultConfig(ctx, config.WithRegion(region)) + loadOptions := append( + []func(*config.LoadOptions) error{config.WithRegion(region)}, + client.loadOptions..., + ) + + cfg, err := config.LoadDefaultConfig(ctx, loadOptions...) if err != nil { return nil, fmt.Errorf("loading aws configuration: %w", err) } diff --git a/pkg/client/eks/client_test.go b/pkg/client/eks/client_test.go index 20ec26eb3c..76995fcc04 100644 --- a/pkg/client/eks/client_test.go +++ b/pkg/client/eks/client_test.go @@ -5,6 +5,8 @@ import ( "encoding/base64" "errors" "net/url" + "os" + "path/filepath" "strings" "testing" @@ -175,3 +177,92 @@ func TestMintTokenSignsClusterBindingHeaders(t *testing.T) { assert.Equal(t, "60", query.Get("X-Amz-Expires")) assert.Contains(t, query.Get("X-Amz-SignedHeaders"), "x-k8s-aws-id") } + +func TestWithCredentialValues_StaticCredentialsOverrideAmbientIdentity(t *testing.T) { + // Not parallel: t.Setenv changes the process environment. + t.Setenv("AWS_ACCESS_KEY_ID", "STALEAMBIENT") + t.Setenv("AWS_SECRET_ACCESS_KEY", "stale-ambient-secret") + + client, err := eksclient.NewClient( + t.Context(), + "eu-central-1", + eksclient.WithClusterDescriber(fakeDescriber{}), + eksclient.WithCredentialValues( + "ignored-profile", + "SELECTEDACCESS", + "selected-secret", + "selected-session", + ), + ) + require.NoError(t, err) + + token, err := client.MintToken(t.Context(), "eks-default") + require.NoError(t, err) + assertTokenCredentialPrefix(t, token, "SELECTEDACCESS/") +} + +func TestWithCredentialValues_ProfileOverridesAmbientStaticCredentials(t *testing.T) { + // Not parallel: t.Setenv changes the process environment. + credentialsFile := filepath.Join(t.TempDir(), "credentials") + require.NoError(t, os.WriteFile( + credentialsFile, + []byte( + "[selected-profile]\naws_access_key_id = PROFILEACCESS\naws_secret_access_key = profile-secret\n", + ), + 0o600, + )) + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", credentialsFile) + t.Setenv("AWS_CONFIG_FILE", filepath.Join(t.TempDir(), "config")) + t.Setenv("AWS_ACCESS_KEY_ID", "STALEAMBIENT") + t.Setenv("AWS_SECRET_ACCESS_KEY", "stale-ambient-secret") + + client, err := eksclient.NewClient( + t.Context(), + "eu-central-1", + eksclient.WithClusterDescriber(fakeDescriber{}), + eksclient.WithCredentialValues("selected-profile", "", "", ""), + ) + require.NoError(t, err) + + token, err := client.MintToken(t.Context(), "eks-default") + require.NoError(t, err) + assertTokenCredentialPrefix(t, token, "PROFILEACCESS/") +} + +func TestWithCredentialValues_RejectsPartialStaticCredentials(t *testing.T) { + t.Parallel() + + _, err := eksclient.NewClient( + t.Context(), + "eu-central-1", + eksclient.WithClusterDescriber(fakeDescriber{}), + eksclient.WithCallerIdentityPresigner(fakePresigner{}), + eksclient.WithCredentialValues("", "access-without-secret", "", ""), + ) + require.ErrorIs(t, err, eksclient.ErrIncompleteStaticCredentials) +} + +func TestRequireCredentialValuesRejectsAmbientFallback(t *testing.T) { + t.Parallel() + + _, err := eksclient.NewClient( + t.Context(), + "eu-central-1", + eksclient.WithClusterDescriber(fakeDescriber{}), + eksclient.WithCallerIdentityPresigner(fakePresigner{}), + eksclient.WithCredentialValues("", "", "", ""), + eksclient.RequireCredentialValues(), + ) + require.ErrorIs(t, err, eksclient.ErrExplicitCredentialsUnavailable) +} + +func assertTokenCredentialPrefix(t *testing.T, token, expected string) { + t.Helper() + + decoded, err := base64.RawURLEncoding.DecodeString(strings.TrimPrefix(token, "k8s-aws-v1.")) + require.NoError(t, err) + + signedURL, err := url.Parse(string(decoded)) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(signedURL.Query().Get("X-Amz-Credential"), expected)) +} diff --git a/pkg/client/eks/errors.go b/pkg/client/eks/errors.go index 00f0be5e4d..a4d187d8ea 100644 --- a/pkg/client/eks/errors.go +++ b/pkg/client/eks/errors.go @@ -5,3 +5,14 @@ import "errors" // ErrClusterNotFound is returned when DescribeCluster succeeds but the // response carries no cluster payload. var ErrClusterNotFound = errors.New("eks cluster not found") + +// ErrIncompleteStaticCredentials is returned when an explicit AWS credential +// selection contains only part of the access-key/secret-key pair (or a session +// token without that pair). Failing here prevents fallback to an ambient identity. +var ErrIncompleteStaticCredentials = errors.New("incomplete explicit AWS static credentials") + +// ErrExplicitCredentialsUnavailable is returned when custom credential +// sources were selected but resolved no usable profile or static key pair. +var ErrExplicitCredentialsUnavailable = errors.New( + "explicit AWS credential selection resolved no credentials", +) diff --git a/pkg/client/eksctl/client.go b/pkg/client/eksctl/client.go index a4a5f348c1..95896cb9e7 100644 --- a/pkg/client/eksctl/client.go +++ b/pkg/client/eksctl/client.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os/exec" + "sort" "strings" ) @@ -24,6 +25,20 @@ type Runner interface { ) (stdout, stderr []byte, err error) } +// EnvironmentRunner is the optional extension implemented by runners that can +// execute with an explicit child-process environment. It keeps the original +// Runner interface source-compatible while allowing credential-isolated EKS +// clients to fail closed when an injected runner cannot honor their mapping. +type EnvironmentRunner interface { + RunWithEnvironment( + ctx context.Context, + name string, + args []string, + stdin io.Reader, + environment []string, + ) (stdout, stderr []byte, err error) +} + // ExecRunner is the default Runner that shells out via os/exec. type ExecRunner struct{} @@ -34,12 +49,25 @@ func (ExecRunner) Run( name string, args []string, stdin io.Reader, +) ([]byte, []byte, error) { + return ExecRunner{}.RunWithEnvironment(ctx, name, args, stdin, nil) +} + +// RunWithEnvironment executes the command with environment. A nil environment +// preserves os/exec's default inheritance; a non-nil slice is used verbatim. +func (ExecRunner) RunWithEnvironment( + ctx context.Context, + name string, + args []string, + stdin io.Reader, + environment []string, ) ([]byte, []byte, error) { // #nosec G204 -- This uses os/exec directly with a program name and argv // slice; it does not invoke a shell, so user-influenced values in args // (cluster name, region, config file paths from ksail.yaml) are passed // as literal arguments rather than shell-interpreted command text. cmd := exec.CommandContext(ctx, name, args...) + cmd.Env = environment var stdout, stderr bytes.Buffer @@ -60,8 +88,11 @@ func (ExecRunner) Run( // Client is the eksctl CLI wrapper. type Client struct { - binary string - runner Runner + binary string + runner Runner + environment []string + + requireCredentialValues bool } // Option configures a Client at construction time. @@ -87,12 +118,33 @@ func WithRunner(runner Runner) Option { } } +// WithEnvironment sets the complete environment passed to every eksctl child +// process. The slice is cloned at construction and again per invocation so +// callers, concurrent commands, and injected runners cannot mutate each other. +func WithEnvironment(environment []string) Option { + return func(c *Client) { + c.environment = cloneStrings(environment) + } +} + +// RequireCredentialValues makes execution fail closed unless the explicit +// child environment contains either a profile or a complete static credential +// pair. Use it when custom source names were configured so an unset alias +// cannot silently fall back to another ambient identity. +func RequireCredentialValues() Option { + return func(c *Client) { + c.requireCredentialValues = true + } +} + // NewClient returns a Client using the eksctl binary on PATH and the default // ExecRunner. Use the Options to override either. func NewClient(opts ...Option) *Client { client := &Client{ - binary: DefaultBinary, - runner: ExecRunner{}, + binary: DefaultBinary, + runner: ExecRunner{}, + environment: nil, + requireCredentialValues: false, } for _, opt := range opts { @@ -127,7 +179,9 @@ func (c *Client) CheckAvailable() error { // escape hatch used by all higher-level methods on this client and can be // used directly when a helper has not been written yet. func (c *Client) Exec(ctx context.Context, args ...string) ([]byte, []byte, error) { - stdout, stderr, err := c.runner.Run(ctx, c.binary, args, nil) + stdout, stderr, err := c.run(ctx, args, nil) + + stderr = c.redactCredentialValues(stderr) if err != nil { return stdout, stderr, wrapExecErr(args, stderr, err) } @@ -142,7 +196,9 @@ func (c *Client) ExecWithStdin( stdin io.Reader, args ...string, ) ([]byte, []byte, error) { - stdout, stderr, err := c.runner.Run(ctx, c.binary, args, stdin) + stdout, stderr, err := c.run(ctx, args, stdin) + + stderr = c.redactCredentialValues(stderr) if err != nil { return stdout, stderr, wrapExecErr(args, stderr, err) } @@ -150,6 +206,124 @@ func (c *Client) ExecWithStdin( return stdout, stderr, nil } +func (c *Client) run( + ctx context.Context, + args []string, + stdin io.Reader, +) ([]byte, []byte, error) { + err := c.validateCredentialValues() + if err != nil { + return nil, nil, err + } + + if c.environment == nil { + stdout, stderr, err := c.runner.Run(ctx, c.binary, args, stdin) + if err != nil { + return stdout, stderr, fmt.Errorf("run eksctl: %w", err) + } + + return stdout, stderr, nil + } + + environmentRunner, ok := c.runner.(EnvironmentRunner) + if !ok { + return nil, nil, ErrRunnerEnvironmentUnsupported + } + + stdout, stderr, err := environmentRunner.RunWithEnvironment( + ctx, + c.binary, + args, + stdin, + cloneStrings(c.environment), + ) + if err != nil { + return stdout, stderr, fmt.Errorf("run eksctl with explicit environment: %w", err) + } + + return stdout, stderr, nil +} + +func (c *Client) validateCredentialValues() error { + if !c.requireCredentialValues { + return nil + } + + values := environmentValues(c.environment) + profile := values["AWS_PROFILE"] + accessKeyID := values["AWS_ACCESS_KEY_ID"] + secretAccessKey := values["AWS_SECRET_ACCESS_KEY"] + sessionToken := values["AWS_SESSION_TOKEN"] + hasAccessKey := accessKeyID != "" + hasSecretKey := secretAccessKey != "" + + if hasAccessKey != hasSecretKey || (sessionToken != "" && !hasAccessKey) { + return ErrIncompleteStaticCredentials + } + + if profile == "" && !hasAccessKey { + return ErrExplicitCredentialsUnavailable + } + + return nil +} + +func cloneStrings(values []string) []string { + if values == nil { + return nil + } + + return append([]string{}, values...) +} + +func (c *Client) redactCredentialValues(stderr []byte) []byte { + if len(stderr) == 0 || len(c.environment) == 0 { + return stderr + } + + uniqueValues := make(map[string]struct{}) + + for _, entry := range c.environment { + name, value, found := strings.Cut(entry, "=") + if !found || value == "" { + continue + } + + switch name { + case "AWS_PROFILE", "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN": + uniqueValues[value] = struct{}{} + } + } + + values := make([]string, 0, len(uniqueValues)) + for value := range uniqueValues { + values = append(values, value) + } + + sort.Slice(values, func(i, j int) bool { + return len(values[i]) > len(values[j]) + }) + + redacted := string(stderr) + for _, value := range values { + redacted = strings.ReplaceAll(redacted, value, "[REDACTED]") + } + + return []byte(redacted) +} + +func environmentValues(environment []string) map[string]string { + values := make(map[string]string, len(environment)) + for _, entry := range environment { + name, value, found := strings.Cut(entry, "=") + if found { + values[name] = value + } + } + + return values +} + // wrapExecErr annotates an exec failure with the invoked arguments and the // first line of stderr (if any) to produce actionable error messages without // leaking the full eksctl output into Go error strings. diff --git a/pkg/client/eksctl/client_test.go b/pkg/client/eksctl/client_test.go index 44ba0a9b90..51394fa688 100644 --- a/pkg/client/eksctl/client_test.go +++ b/pkg/client/eksctl/client_test.go @@ -26,17 +26,56 @@ type fakeRunner struct { lastName string lastArgs []string lastStdin []byte + lastEnv []string + + mutateEnvironment bool +} + +type legacyRunner struct{} + +func (legacyRunner) Run( + context.Context, + string, + []string, + io.Reader, +) ([]byte, []byte, error) { + return nil, nil, nil } func (f *fakeRunner) Run( + ctx context.Context, + name string, + args []string, + stdin io.Reader, +) ([]byte, []byte, error) { + return f.run(ctx, name, args, stdin, nil) +} + +func (f *fakeRunner) RunWithEnvironment( + ctx context.Context, + name string, + args []string, + stdin io.Reader, + environment []string, +) ([]byte, []byte, error) { + return f.run(ctx, name, args, stdin, environment) +} + +func (f *fakeRunner) run( _ context.Context, name string, args []string, stdin io.Reader, + environment []string, ) ([]byte, []byte, error) { f.lastName = name f.lastArgs = append([]string(nil), args...) + f.lastEnv = append([]string(nil), environment...) + + if f.mutateEnvironment && len(environment) > 0 { + environment[0] = "MUTATED_BY_RUNNER" + } if stdin != nil { buf, _ := io.ReadAll(stdin) @@ -74,6 +113,171 @@ func TestWithBinary_EmptyIgnored(t *testing.T) { assert.Equal(t, eksctl.DefaultBinary, client.Binary()) } +func TestWithEnvironment_ForwardsAnIsolatedSnapshot(t *testing.T) { + t.Parallel() + + environment := []string{"HOME=/tmp/ksail", "AWS_PROFILE=custom-profile"} + runner := &fakeRunner{mutateEnvironment: true} + client := eksctl.NewClient( + eksctl.WithRunner(runner), + eksctl.WithEnvironment(environment), + ) + + // Construction must snapshot the caller's slice. + environment[0] = "HOME=/mutated-by-caller" + + _, _, err := client.Exec(t.Context(), "get", "cluster") + require.NoError(t, err) + assert.Equal(t, []string{"HOME=/tmp/ksail", "AWS_PROFILE=custom-profile"}, runner.lastEnv) + + // A runner must not be able to mutate the environment reused by the next + // command on the same client. + _, _, err = client.ExecWithStdin( + t.Context(), + bytes.NewBufferString("config"), + "create", + "cluster", + ) + require.NoError(t, err) + assert.Equal(t, []string{"HOME=/tmp/ksail", "AWS_PROFILE=custom-profile"}, runner.lastEnv) +} + +func TestNewClient_DefaultEnvironmentInheritsParent(t *testing.T) { + t.Parallel() + + runner := &fakeRunner{} + client := eksctl.NewClient(eksctl.WithRunner(runner)) + + _, _, err := client.Exec(t.Context(), "get", "cluster") + require.NoError(t, err) + assert.Nil(t, runner.lastEnv) +} + +func TestWithEnvironment_FailsClosedForLegacyRunner(t *testing.T) { + t.Parallel() + + client := eksctl.NewClient( + eksctl.WithRunner(legacyRunner{}), + eksctl.WithEnvironment([]string{"PATH=/usr/bin", "AWS_PROFILE=selected"}), + ) + + _, _, err := client.Exec(t.Context(), "get", "cluster") + require.ErrorIs(t, err, eksctl.ErrRunnerEnvironmentUnsupported) +} + +func TestExec_RedactsCredentialValuesFromStderrErrors(t *testing.T) { + t.Parallel() + + runner := &fakeRunner{ + stderr: []byte("provider rejected fixture-secret-value"), + err: errExitStatus1, + } + client := eksctl.NewClient( + eksctl.WithRunner(runner), + eksctl.WithEnvironment([]string{ + "PATH=/usr/bin", + "AWS_SECRET_ACCESS_KEY=fixture-secret-value", + }), + ) + + _, stderr, err := client.Exec(t.Context(), "get", "cluster") + require.Error(t, err) + assert.NotContains(t, string(stderr), "fixture-secret-value") + assert.NotContains(t, err.Error(), "fixture-secret-value") + assert.Contains(t, err.Error(), "[REDACTED]") +} + +func TestExec_RedactsOverlappingCredentialValuesLongestFirst(t *testing.T) { + t.Parallel() + + runner := &fakeRunner{ + stderr: []byte("provider rejected fixture-secret-long"), + err: errExitStatus1, + } + client := eksctl.NewClient( + eksctl.WithRunner(runner), + eksctl.WithEnvironment([]string{ + "AWS_PROFILE=fixture-secret", + "AWS_SECRET_ACCESS_KEY=fixture-secret-long", + }), + ) + + _, stderr, err := client.Exec(t.Context(), "get", "cluster") + require.Error(t, err) + assert.Equal(t, "provider rejected [REDACTED]", string(stderr)) + assert.NotContains(t, err.Error(), "-long") +} + +func TestRequireCredentialValuesRejectsMissingAndPartialSelections(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + environment []string + expectedErr error + }{ + "missing": { + environment: []string{"PATH=/usr/bin"}, + expectedErr: eksctl.ErrExplicitCredentialsUnavailable, + }, + "access without secret": { + environment: []string{"AWS_ACCESS_KEY_ID=fixture-access"}, + expectedErr: eksctl.ErrIncompleteStaticCredentials, + }, + "secret without access": { + environment: []string{"AWS_SECRET_ACCESS_KEY=fixture-secret"}, + expectedErr: eksctl.ErrIncompleteStaticCredentials, + }, + "session without static pair": { + environment: []string{"AWS_SESSION_TOKEN=fixture-session"}, + expectedErr: eksctl.ErrIncompleteStaticCredentials, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + runner := &fakeRunner{} + client := eksctl.NewClient( + eksctl.WithRunner(runner), + eksctl.WithEnvironment(test.environment), + eksctl.RequireCredentialValues(), + ) + + _, _, err := client.Exec(t.Context(), "get", "cluster") + require.ErrorIs(t, err, test.expectedErr) + assert.Empty(t, runner.lastArgs, "invalid credentials must fail before invoking eksctl") + }) + } +} + +func TestRequireCredentialValuesAcceptsProfileOrStaticPair(t *testing.T) { + t.Parallel() + + for name, environment := range map[string][]string{ + "profile": {"AWS_PROFILE=selected-profile"}, + "static pair": { + "AWS_ACCESS_KEY_ID=fixture-access", + "AWS_SECRET_ACCESS_KEY=fixture-secret", + }, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + runner := &fakeRunner{} + client := eksctl.NewClient( + eksctl.WithRunner(runner), + eksctl.WithEnvironment(environment), + eksctl.RequireCredentialValues(), + ) + + _, _, err := client.Exec(t.Context(), "get", "cluster") + require.NoError(t, err) + assert.Equal(t, []string{"get", "cluster"}, runner.lastArgs) + }) + } +} + func TestCreateCluster_InvokesCorrectArgs(t *testing.T) { t.Parallel() diff --git a/pkg/client/eksctl/errors.go b/pkg/client/eksctl/errors.go index 3e44ba27a3..80a8192f65 100644 --- a/pkg/client/eksctl/errors.go +++ b/pkg/client/eksctl/errors.go @@ -13,6 +13,25 @@ var ErrBinaryNotFound = errors.New( // ErrExecFailed wraps a non-zero exit from the eksctl binary. var ErrExecFailed = errors.New("eksctl command failed") +// ErrRunnerEnvironmentUnsupported is returned when a client has an explicit +// child environment but its injected legacy Runner cannot accept one. Failing +// closed prevents a test/custom runner from silently inheriting the wrong AWS +// identity; existing runners remain source-compatible when no environment is set. +var ErrRunnerEnvironmentUnsupported = errors.New( + "eksctl runner does not support an explicit environment", +) + +// ErrIncompleteStaticCredentials is returned when an explicit child +// environment contains only part of the AWS static credential tuple. +var ErrIncompleteStaticCredentials = errors.New("incomplete static AWS credentials") + +// ErrExplicitCredentialsUnavailable is returned when a credential-isolated +// client was required to use resolved values but neither a profile nor a +// complete static credential pair was available. +var ErrExplicitCredentialsUnavailable = errors.New( + "configured AWS credential sources resolved no credentials", +) + // ErrClusterNotFound is returned when an `eksctl get cluster` lookup for a // named cluster returns an empty result set. var ErrClusterNotFound = errors.New("eks cluster not found") diff --git a/pkg/svc/clusterdiscovery/aws_environment_test.go b/pkg/svc/clusterdiscovery/aws_environment_test.go new file mode 100644 index 0000000000..f726784e84 --- /dev/null +++ b/pkg/svc/clusterdiscovery/aws_environment_test.go @@ -0,0 +1,94 @@ +package clusterdiscovery_test + +import ( + "os" + "path/filepath" + "testing" + + v1alpha1 "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" + "github.com/devantler-tech/ksail/v7/pkg/svc/clusterdiscovery" + "github.com/devantler-tech/ksail/v7/pkg/svc/credentials" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type namedAWSResolver struct { + values map[credentials.Key]string + names map[credentials.Key]string +} + +func (r namedAWSResolver) Value(key credentials.Key) string { return r.values[key] } + +func (r namedAWSResolver) EnvVar(key credentials.Key) string { + if name := r.names[key]; name != "" { + return name + } + + return credentials.DefaultEnvVar(key) +} + +func TestDiscoverAWS_UsesCanonicalIsolatedChildEnvironment(t *testing.T) { + // Not parallel: the real ExecRunner resolves the fixture from PATH. + binDir := t.TempDir() + eksctlPath := filepath.Join(binDir, "eksctl") + writeExecutableFixture(t, eksctlPath, `#!/bin/sh +[ "${AWS_PROFILE-}" = "selected-profile" ] || exit 41 +[ "${AWS_ACCESS_KEY_ID-}" = "fixture-access" ] || exit 42 +[ "${AWS_SECRET_ACCESS_KEY-}" = "fixture-secret" ] || exit 43 +[ "${AWS_SESSION_TOKEN-}" = "fixture-session" ] || exit 44 +[ -z "${KSAIL_PROFILE+x}" ] || exit 45 +[ -z "${KSAIL_ACCESS+x}" ] || exit 46 +[ -z "${KSAIL_SECRET+x}" ] || exit 47 +[ -z "${KSAIL_SESSION+x}" ] || exit 48 +printf '[{"Name":"mapped-eks","Region":"eu-west-1","EksctlCreated":"True"}]\n' +`) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("KSAIL_PROFILE", "parent-custom-profile") + t.Setenv("KSAIL_ACCESS", "parent-custom-access") + t.Setenv("KSAIL_SECRET", "parent-custom-secret") + t.Setenv("KSAIL_SESSION", "parent-custom-session") + t.Setenv("AWS_PROFILE", "parent-stale-profile") + t.Setenv("AWS_ACCESS_KEY_ID", "parent-stale-access") + t.Setenv("AWS_SECRET_ACCESS_KEY", "parent-stale-secret") + t.Setenv("AWS_SESSION_TOKEN", "parent-stale-session") + + discoverer := &clusterdiscovery.Discoverer{ + Resolver: namedAWSResolver{ + values: map[credentials.Key]string{ + credentials.AWSProfile: "selected-profile", + credentials.AWSAccessKeyID: "fixture-access", + credentials.AWSSecretAccessKey: "fixture-secret", + credentials.AWSSessionToken: "fixture-session", + }, + names: map[credentials.Key]string{ + credentials.AWSProfile: "KSAIL_PROFILE", + credentials.AWSAccessKeyID: "KSAIL_ACCESS", + credentials.AWSSecretAccessKey: "KSAIL_SECRET", + credentials.AWSSessionToken: "KSAIL_SESSION", + }, + }, + LookPath: func(string) (string, error) { return eksctlPath, nil }, + } + + clusters, failures := discoverer.Discover( + t.Context(), + []v1alpha1.Provider{v1alpha1.ProviderAWS}, + ) + + require.Empty(t, failures) + require.Len(t, clusters, 1) + assert.Equal(t, "mapped-eks", clusters[0].Name) + assert.Equal(t, "parent-stale-profile", os.Getenv("AWS_PROFILE")) + assert.Equal(t, "parent-custom-profile", os.Getenv("KSAIL_PROFILE")) +} + +func writeExecutableFixture(t *testing.T, path, contents string) { + t.Helper() + + require.NoError(t, os.WriteFile(path, []byte(contents), 0o600)) + require.NoError( + t, + //nolint:gosec // owner execute is required for the fixture. + os.Chmod(path, 0o700), + ) +} diff --git a/pkg/svc/clusterdiscovery/cloud.go b/pkg/svc/clusterdiscovery/cloud.go index 34af8e2a7f..6421e4bfba 100644 --- a/pkg/svc/clusterdiscovery/cloud.go +++ b/pkg/svc/clusterdiscovery/cloud.go @@ -85,9 +85,29 @@ func (d *Discoverer) listAWS(ctx context.Context) ([]Cluster, error) { return nil, nil } - client := eksctlclient.NewClient() + auth := credentials.ResolveAWS(d.resolver()) + eksctlOptions := []eksctlclient.Option{ + eksctlclient.WithEnvironment(auth.ChildEnvironment(os.Environ())), + } + + providerOptions := []awsprovider.Option{awsprovider.WithCredentialValues( + auth.Profile, + auth.AccessKeyID, + auth.SecretAccessKey, + auth.SessionToken, + )} + if auth.HasCustomCredentialSources() { + eksctlOptions = append(eksctlOptions, eksctlclient.RequireCredentialValues()) + providerOptions = append(providerOptions, awsprovider.RequireCredentialValues()) + } - provider, err := awsprovider.NewProvider(client, d.resolver().Value(credentials.AWSRegion)) + client := eksctlclient.NewClient(eksctlOptions...) + + provider, err := awsprovider.NewProvider( + client, + d.resolver().Value(credentials.AWSRegion), + providerOptions..., + ) if err != nil { return nil, fmt.Errorf("create AWS provider: %w", err) } diff --git a/pkg/svc/credentials/aws_environment_test.go b/pkg/svc/credentials/aws_environment_test.go new file mode 100644 index 0000000000..45037f4592 --- /dev/null +++ b/pkg/svc/credentials/aws_environment_test.go @@ -0,0 +1,277 @@ +package credentials_test + +import ( + "os" + "slices" + "strconv" + "strings" + "sync" + "testing" + + "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" + "github.com/devantler-tech/ksail/v7/pkg/svc/credentials" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type awsResolverFixture struct { + envVars map[credentials.Key]string + values map[credentials.Key]string +} + +func (f awsResolverFixture) EnvVar(key credentials.Key) string { + if name := f.envVars[key]; name != "" { + return name + } + + return credentials.DefaultEnvVar(key) +} + +func (f awsResolverFixture) Value(key credentials.Key) string { return f.values[key] } + +func TestNewAWSOptionsResolver_UsesConfiguredNamesWithoutMutatingParent(t *testing.T) { + // Not parallel: t.Setenv changes the process environment. + t.Setenv("KSAIL_PROFILE", "selected-profile") + t.Setenv("KSAIL_ACCESS", "fixture-access") + t.Setenv("KSAIL_SECRET", "fixture-secret") + t.Setenv("KSAIL_SESSION", "fixture-session") + t.Setenv("AWS_PROFILE", "stale-profile") + + resolver := credentials.NewAWSOptionsResolver(v1alpha1.OptionsAWS{ + ProfileEnvVar: "KSAIL_PROFILE", + AccessKeyIDEnvVar: "KSAIL_ACCESS", + SecretAccessKeyEnvVar: "KSAIL_SECRET", + SessionTokenEnvVar: "KSAIL_SESSION", + }) + resolved := credentials.ResolveAWS(resolver) + + assert.Equal(t, "KSAIL_PROFILE", resolver.EnvVar(credentials.AWSProfile)) + assert.Equal(t, "selected-profile", resolved.Profile) + assert.Equal(t, "fixture-access", resolved.AccessKeyID) + assert.NotEmpty(t, resolved.SecretAccessKey) + assert.NotEmpty(t, resolved.SessionToken) + assert.Equal(t, "stale-profile", os.Getenv("AWS_PROFILE")) +} + +func TestAWSResolution_ChildEnvironmentCanonicalizesAndIsolatesCredentials(t *testing.T) { + t.Parallel() + + parent := []string{ + "PATH=/usr/bin", + "HOME=/home/ksail", + "AWS_CONFIG_FILE=/home/ksail/.aws/config", + "AWS_PROFILE=stale-profile", + "AWS_ACCESS_KEY_ID=stale-access", + "AWS_SECRET_ACCESS_KEY=stale-secret", + "AWS_SESSION_TOKEN=stale-session", + "KSAIL_PROFILE=selected-profile", + "KSAIL_ACCESS=fixture-access", + "KSAIL_SECRET=fixture-secret", + "KSAIL_SESSION=fixture-session", + } + before := slices.Clone(parent) + resolver := awsResolverFixture{ + envVars: map[credentials.Key]string{ + credentials.AWSProfile: "KSAIL_PROFILE", + credentials.AWSAccessKeyID: "KSAIL_ACCESS", + credentials.AWSSecretAccessKey: "KSAIL_SECRET", + credentials.AWSSessionToken: "KSAIL_SESSION", + }, + values: map[credentials.Key]string{ + credentials.AWSProfile: "selected-profile", + credentials.AWSAccessKeyID: "fixture-access", + credentials.AWSSecretAccessKey: "fixture-secret", + credentials.AWSSessionToken: "fixture-session", + }, + } + + child := credentials.ResolveAWS(resolver).ChildEnvironment(parent) + + assert.Equal(t, before, parent, "building a child environment must not mutate its input") + assertEnvEntry(t, child, "PATH", "usr/bin") + assertEnvEntry(t, child, "HOME", "home/ksail") + assertEnvEntry(t, child, "AWS_CONFIG_FILE", ".aws/config") + assertEnvEntry(t, child, "AWS_PROFILE", "selected-profile") + assertEnvEntry(t, child, "AWS_ACCESS_KEY_ID", "fixture-access") + assertEnvEntry(t, child, "AWS_SECRET_ACCESS_KEY", "fixture-secret") + assertEnvEntry(t, child, "AWS_SESSION_TOKEN", "fixture-session") + assertEnvKeyCount(t, child, "AWS_PROFILE", 1) + assertEnvKeyCount(t, child, "AWS_ACCESS_KEY_ID", 1) + assertEnvKeyCount(t, child, "AWS_SECRET_ACCESS_KEY", 1) + assertEnvKeyCount(t, child, "AWS_SESSION_TOKEN", 1) + assertEnvKeyCount(t, child, "KSAIL_PROFILE", 0) + assertEnvKeyCount(t, child, "KSAIL_ACCESS", 0) + assertEnvKeyCount(t, child, "KSAIL_SECRET", 0) + assertEnvKeyCount(t, child, "KSAIL_SESSION", 0) +} + +func TestAWSResolution_ChildEnvironmentRemovesStaleOptionalValues(t *testing.T) { + t.Parallel() + + resolver := awsResolverFixture{ + envVars: map[credentials.Key]string{ + credentials.AWSProfile: "KSAIL_PROFILE", + credentials.AWSSessionToken: "KSAIL_SESSION", + }, + values: map[credentials.Key]string{ + credentials.AWSProfile: "selected-profile", + }, + } + child := credentials.ResolveAWS(resolver).ChildEnvironment([]string{ + "PATH=/usr/bin", + "AWS_PROFILE=stale-profile", + "AWS_SESSION_TOKEN=stale-session", + "KSAIL_SESSION=stale-custom-session", + }) + + assertEnvEntry(t, child, "AWS_PROFILE", "selected-profile") + assertEnvKeyCount(t, child, "AWS_SESSION_TOKEN", 0) + assertEnvKeyCount(t, child, "KSAIL_SESSION", 0) +} + +func TestAWSResolution_CustomSourcesRemoveCompetingAmbientCredentialProviders(t *testing.T) { + t.Parallel() + + resolution := credentials.ResolveAWS(awsResolverFixture{ + envVars: map[credentials.Key]string{ + credentials.AWSProfile: "KSAIL_PROFILE", + }, + values: map[credentials.Key]string{ + credentials.AWSProfile: "selected-profile", + }, + }) + child := resolution.ChildEnvironment([]string{ + "PATH=/usr/bin", + "AWS_DEFAULT_PROFILE=stale-default-profile", + "AWS_ACCESS_KEY=stale-legacy-access", + "AWS_SECRET_KEY=stale-legacy-secret", + "AWS_WEB_IDENTITY_TOKEN_FILE=/tmp/stale-token", + "AWS_ROLE_ARN=arn:aws:iam::123456789012:role/stale", + "AWS_ROLE_SESSION_NAME=stale-session", + "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI=/v2/credentials/selected", + "AWS_CONTAINER_CREDENTIALS_FULL_URI=http://127.0.0.1/selected", + "AWS_CONTAINER_AUTHORIZATION_TOKEN=selected-token", + "AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE=/tmp/selected-auth-token", + "KSAIL_PROFILE=selected-profile", + }) + + assertEnvEntry(t, child, "AWS_PROFILE", "selected-profile") + + for _, key := range []string{ + "AWS_DEFAULT_PROFILE", + "AWS_ACCESS_KEY", + "AWS_SECRET_KEY", + "AWS_WEB_IDENTITY_TOKEN_FILE", + "AWS_ROLE_ARN", + "AWS_ROLE_SESSION_NAME", + "KSAIL_PROFILE", + } { + assertEnvKeyCount(t, child, key, 0) + } + + assertEnvEntry(t, child, "PATH", "usr/bin") + assertEnvEntry(t, child, "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI", "credentials/selected") + assertEnvEntry(t, child, "AWS_CONTAINER_CREDENTIALS_FULL_URI", "127.0.0.1/selected") + assertEnvEntry(t, child, "AWS_CONTAINER_AUTHORIZATION_TOKEN", "selected-token") + assertEnvEntry(t, child, "AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE", "selected-auth-token") +} + +func TestAWSResolution_ReportsCustomCredentialSourcesEvenWhenUnset(t *testing.T) { + t.Parallel() + + resolution := credentials.ResolveAWS(awsResolverFixture{ + envVars: map[credentials.Key]string{ + credentials.AWSProfile: "KSAIL_PROFILE", + }, + values: map[credentials.Key]string{}, + }) + + assert.True(t, resolution.HasCustomCredentialSources()) +} + +func TestAWSResolution_ChildEnvironmentPreservesCanonicalDefaults(t *testing.T) { + t.Parallel() + + resolver := awsResolverFixture{values: map[credentials.Key]string{ + credentials.AWSProfile: "default-profile", + credentials.AWSAccessKeyID: "fixture-access", + credentials.AWSSecretAccessKey: "fixture-secret", + }} + child := credentials.ResolveAWS(resolver).ChildEnvironment([]string{ + "AWS_PROFILE=default-profile", + "AWS_ACCESS_KEY_ID=fixture-access", + "AWS_SECRET_ACCESS_KEY=fixture-secret", + "AWS_WEB_IDENTITY_TOKEN_FILE=/tmp/token", + "AWS_ROLE_ARN=arn:aws:iam::123456789012:role/default", + "HOME=/home/ksail", + }) + + assertEnvEntry(t, child, "AWS_PROFILE", "default-profile") + assertEnvEntry(t, child, "AWS_ACCESS_KEY_ID", "fixture-access") + assertEnvEntry(t, child, "AWS_SECRET_ACCESS_KEY", "fixture-secret") + assertEnvKeyCount(t, child, "AWS_PROFILE", 1) + assertEnvKeyCount(t, child, "AWS_ACCESS_KEY_ID", 1) + assertEnvKeyCount(t, child, "AWS_SECRET_ACCESS_KEY", 1) + assertEnvEntry(t, child, "AWS_WEB_IDENTITY_TOKEN_FILE", "/tmp/token") + assertEnvEntry(t, child, "AWS_ROLE_ARN", "role/default") +} + +func TestAWSResolution_ChildEnvironmentIsSafeForConcurrentInvocations(t *testing.T) { + t.Parallel() + + resolution := credentials.ResolveAWS(awsResolverFixture{ + values: map[credentials.Key]string{credentials.AWSProfile: "concurrent-profile"}, + }) + + const invocations = 32 + + errCh := make(chan error, invocations) + + var waitGroup sync.WaitGroup + for index := range invocations { + waitGroup.Go(func() { + child := resolution.ChildEnvironment([]string{"INVOCATION=" + strconv.Itoa(index)}) + if envKeyCount(child, "AWS_PROFILE") != 1 { + errCh <- assert.AnError + } + }) + } + + waitGroup.Wait() + close(errCh) + require.Empty(t, errCh) +} + +func assertEnvEntry(t *testing.T, environment []string, key, valueFragment string) { + t.Helper() + + for _, entry := range environment { + name, value, found := strings.Cut(entry, "=") + if found && name == key { + assert.Contains(t, value, valueFragment) + + return + } + } + + assert.Fail(t, "expected environment key is missing", key) +} + +func assertEnvKeyCount(t *testing.T, environment []string, key string, expected int) { + t.Helper() + + assert.Equal(t, expected, envKeyCount(environment, key), "unexpected count for %s", key) +} + +func envKeyCount(environment []string, key string) int { + count := 0 + + for _, entry := range environment { + name, _, found := strings.Cut(entry, "=") + if found && name == key { + count++ + } + } + + return count +} diff --git a/pkg/svc/credentials/credentials.go b/pkg/svc/credentials/credentials.go index 72844d6196..48b635a6a2 100644 --- a/pkg/svc/credentials/credentials.go +++ b/pkg/svc/credentials/credentials.go @@ -10,6 +10,8 @@ package credentials import ( "os" + "runtime" + "strings" "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" ) @@ -126,10 +128,29 @@ type Resolver interface { EnvVar(key Key) string } -// EnvResolver resolves purely from the process environment using the default variable names. It is -// the zero-config resolver used when no secure store / Settings overrides are configured. +// EnvResolver resolves purely from the process environment using canonical +// variable names. Its zero value remains the default resolver. type EnvResolver struct{} +// AWSOptionsResolver resolves from immutable per-cluster AWS variable-name +// overrides without mutating the process environment. +type AWSOptionsResolver struct { + envVars map[Key]string +} + +// NewAWSOptionsResolver returns an environment-only resolver honoring the +// variable names configured on one AWS provider spec. Empty names retain their +// canonical defaults. The resolver owns its map and is safe for concurrent use. +func NewAWSOptionsResolver(options v1alpha1.OptionsAWS) AWSOptionsResolver { + return AWSOptionsResolver{envVars: map[Key]string{ + AWSRegion: options.RegionEnvVar, + AWSProfile: options.ProfileEnvVar, + AWSAccessKeyID: options.AccessKeyIDEnvVar, + AWSSecretAccessKey: options.SecretAccessKeyEnvVar, + AWSSessionToken: options.SessionTokenEnvVar, + }} +} + // EnvVar returns the default environment-variable name for key. func (EnvResolver) EnvVar(key Key) string { return DefaultEnvVar(key) } @@ -138,6 +159,170 @@ func (EnvResolver) Value(key Key) string { return resolveEnvValue(key, DefaultEnvVar(key)) } +// EnvVar returns the configured environment-variable name for key, falling +// back to its canonical default. +func (r AWSOptionsResolver) EnvVar(key Key) string { + if name := r.envVars[key]; name != "" { + return name + } + + return DefaultEnvVar(key) +} + +// Value returns the process-environment value for key's configured variable, or "". +func (r AWSOptionsResolver) Value(key Key) string { + return resolveEnvValue(key, r.EnvVar(key)) +} + +// AWSResolution is an immutable snapshot of the credential selection for one +// AWS operation. Source variable names are retained privately so child process +// environments can remove both stale canonical values and custom aliases. +type AWSResolution struct { + Profile string + AccessKeyID string + SecretAccessKey string + SessionToken string + + sourceEnvVars [4]string + hasCustomCredentialEnv bool +} + +// ResolveAWS snapshots all AWS credential values and their configured source +// names from resolver. A nil resolver uses the canonical process environment. +func ResolveAWS(resolver Resolver) AWSResolution { + if resolver == nil { + resolver = EnvResolver{} + } + + // Resolve every value before constructing the child environment. No parent + // environment mutation is needed, so concurrent invocations remain isolated. + resolution := AWSResolution{ + Profile: resolver.Value(AWSProfile), + AccessKeyID: resolver.Value(AWSAccessKeyID), + SecretAccessKey: resolver.Value(AWSSecretAccessKey), + SessionToken: resolver.Value(AWSSessionToken), + sourceEnvVars: [4]string{ + resolver.EnvVar(AWSProfile), + resolver.EnvVar(AWSAccessKeyID), + resolver.EnvVar(AWSSecretAccessKey), + resolver.EnvVar(AWSSessionToken), + }, + } + + canonicalNames := [...]string{ + defaultAWSProfileEnvVar, + defaultAWSAccessKeyIDEnvVar, + defaultAWSSecretAccessEnvVar, + defaultAWSSessionTokenEnvVar, + } + for index, sourceName := range resolution.sourceEnvVars { + if sourceName != "" && sourceName != canonicalNames[index] { + resolution.hasCustomCredentialEnv = true + + break + } + } + + return resolution +} + +// HasCustomCredentialSources reports whether at least one credential is +// configured to resolve from a non-canonical variable name. Callers use this +// to fail closed instead of letting an SDK read stale canonical credentials +// when every configured custom source is unset. +func (r AWSResolution) HasCustomCredentialSources() bool { + return r.hasCustomCredentialEnv +} + +// ChildEnvironment returns a copy of parent with AWS credential aliases and +// stale canonical values removed, followed by the non-empty resolved values +// under the canonical names eksctl understands. When a custom credential source +// is configured, competing environment-based identity providers are also +// removed so they cannot override the explicit selection. Unrelated entries +// such as PATH, HOME, and AWS_CONFIG_FILE are preserved. +func (r AWSResolution) ChildEnvironment(parent []string) []string { + caseInsensitiveNames := runtime.GOOS == "windows" + strippedNames := r.strippedEnvironmentNames(caseInsensitiveNames) + child := filterEnvironment(parent, strippedNames, caseInsensitiveNames) + + for _, binding := range []struct { + name string + value string + }{ + {name: defaultAWSProfileEnvVar, value: r.Profile}, + {name: defaultAWSAccessKeyIDEnvVar, value: r.AccessKeyID}, + {name: defaultAWSSecretAccessEnvVar, value: r.SecretAccessKey}, + {name: defaultAWSSessionTokenEnvVar, value: r.SessionToken}, + } { + if binding.value != "" { + child = append(child, binding.name+"="+binding.value) + } + } + + return child +} + +func (r AWSResolution) strippedEnvironmentNames(caseInsensitive bool) map[string]struct{} { + strippedNames := make(map[string]struct{}) + for _, name := range []string{ + defaultAWSProfileEnvVar, + defaultAWSAccessKeyIDEnvVar, + defaultAWSSecretAccessEnvVar, + defaultAWSSessionTokenEnvVar, + } { + strippedNames[normalizeEnvironmentName(name, caseInsensitive)] = struct{}{} + } + + for _, name := range r.sourceEnvVars { + if name != "" { + strippedNames[normalizeEnvironmentName(name, caseInsensitive)] = struct{}{} + } + } + + if r.hasCustomCredentialEnv { + for _, name := range []string{ + "AWS_DEFAULT_PROFILE", + "AWS_ACCESS_KEY", + "AWS_SECRET_KEY", + "AWS_WEB_IDENTITY_TOKEN_FILE", + "AWS_ROLE_ARN", + "AWS_ROLE_SESSION_NAME", + } { + strippedNames[normalizeEnvironmentName(name, caseInsensitive)] = struct{}{} + } + } + + return strippedNames +} + +func filterEnvironment( + parent []string, + strippedNames map[string]struct{}, + caseInsensitive bool, +) []string { + child := make([]string, 0, len(parent)) + for _, entry := range parent { + name, _, found := strings.Cut(entry, "=") + if found { + if _, stripped := strippedNames[normalizeEnvironmentName(name, caseInsensitive)]; stripped { + continue + } + } + + child = append(child, entry) + } + + return child +} + +func normalizeEnvironmentName(name string, caseInsensitive bool) string { + if caseInsensitive { + return strings.ToUpper(name) + } + + return name +} + // resolveEnvValue reads key's value from the process environment under envVar, applying the Copilot // secondary-variable fallback (COPILOT_TOKEN) when the primary default is unset — matching // webchat.copilotToken(), so a COPILOT_TOKEN-only setup still resolves (and reports as "env"). @@ -160,3 +345,6 @@ func resolveEnvValue(key Key, envVar string) string { // Ensure EnvResolver satisfies Resolver. var _ Resolver = EnvResolver{} + +// Ensure AWSOptionsResolver satisfies Resolver. +var _ Resolver = AWSOptionsResolver{} diff --git a/pkg/svc/credentials/credentials_test.go b/pkg/svc/credentials/credentials_test.go index 3bf6bd3ba8..fc0a48c026 100644 --- a/pkg/svc/credentials/credentials_test.go +++ b/pkg/svc/credentials/credentials_test.go @@ -65,6 +65,7 @@ func TestEnvResolver_CopilotTokenFallback(t *testing.T) { // Not parallel: mutates process env via t.Setenv. t.Setenv("KSAIL_COPILOT_TOKEN", "") t.Setenv("COPILOT_TOKEN", "from-secondary") + t.Setenv(v1alpha1.DefaultHetznerTokenEnvVar, "") var resolver credentials.EnvResolver diff --git a/pkg/svc/credentials/environment_name_test.go b/pkg/svc/credentials/environment_name_test.go new file mode 100644 index 0000000000..c8f0be0c3b --- /dev/null +++ b/pkg/svc/credentials/environment_name_test.go @@ -0,0 +1,15 @@ +package credentials //nolint:testpackage // directly exercises the platform-neutral name normalizer. + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNormalizeEnvironmentNameHonorsPlatformCaseSensitivity(t *testing.T) { + t.Parallel() + + assert.Equal(t, "aws_profile", normalizeEnvironmentName("aws_profile", false)) + assert.Equal(t, "AWS_PROFILE", normalizeEnvironmentName("aws_profile", true)) + assert.Equal(t, "AWS_PROFILE", normalizeEnvironmentName("AwS_PrOfIlE", true)) +} diff --git a/pkg/svc/provider/aws/provider.go b/pkg/svc/provider/aws/provider.go index e2255b3d74..97e8202b8d 100644 --- a/pkg/svc/provider/aws/provider.go +++ b/pkg/svc/provider/aws/provider.go @@ -27,9 +27,11 @@ type clusterDescriber interface { // Provider implements provider.Provider for Amazon EKS via eksctl. type Provider struct { - client *eksctlclient.Client - region string - describer clusterDescriber + client *eksctlclient.Client + region string + describer clusterDescriber + eksClientOptions []eksclient.Option + requireCredentialValues bool } // Option customises a Provider. @@ -44,6 +46,25 @@ func WithClusterDescriber(describer clusterDescriber) Option { } } +// WithCredentialValues pins the credentials used by the provider's lazy +// AWS-SDK EKS client without mutating process environment. The eksctl client +// is configured separately with the matching canonical child environment. +func WithCredentialValues(profile, accessKeyID, secretAccessKey, sessionToken string) Option { + return func(p *Provider) { + p.eksClientOptions = []eksclient.Option{ + eksclient.WithCredentialValues(profile, accessKeyID, secretAccessKey, sessionToken), + } + } +} + +// RequireCredentialValues prevents the lazy SDK client from falling back to +// ambient canonical credentials when custom sources resolved no values. +func RequireCredentialValues() Option { + return func(p *Provider) { + p.requireCredentialValues = true + } +} + // NewProvider returns a Provider using the given eksctl client and AWS region. // Pass region="" to defer to eksctl's own region resolution (AWS_REGION env, // active AWS profile, etc.). A nil client returns ErrClientRequired so callers @@ -53,7 +74,13 @@ func NewProvider(client *eksctlclient.Client, region string, opts ...Option) (*P return nil, ErrClientRequired } - prov := &Provider{client: client, region: region, describer: nil} + prov := &Provider{ + client: client, + region: region, + describer: nil, + eksClientOptions: nil, + requireCredentialValues: false, + } for _, opt := range opts { opt(prov) @@ -265,7 +292,12 @@ func (p *Provider) resolveDescriber(ctx context.Context) (clusterDescriber, erro return p.describer, nil } - client, err := eksclient.NewClient(ctx, p.region) + options := append([]eksclient.Option{}, p.eksClientOptions...) + if p.requireCredentialValues { + options = append(options, eksclient.RequireCredentialValues()) + } + + client, err := eksclient.NewClient(ctx, p.region, options...) if err != nil { return nil, fmt.Errorf("creating aws eks client: %w", err) } diff --git a/pkg/svc/provisioner/cluster/eks/connector.go b/pkg/svc/provisioner/cluster/eks/connector.go index 5668abca1d..a8246e9b62 100644 --- a/pkg/svc/provisioner/cluster/eks/connector.go +++ b/pkg/svc/provisioner/cluster/eks/connector.go @@ -116,7 +116,12 @@ func (p *Provisioner) resolveAWSClient(ctx context.Context) (AWSClusterAPI, erro return p.awsClient, nil } - client, err := eksclient.NewClient(ctx, p.region) + options := append([]eksclient.Option{}, p.eksClientOptions...) + if p.requireCredentialValues { + options = append(options, eksclient.RequireCredentialValues()) + } + + client, err := eksclient.NewClient(ctx, p.region, options...) if err != nil { return nil, fmt.Errorf("creating aws eks client: %w", err) } diff --git a/pkg/svc/provisioner/cluster/eks/connector_test.go b/pkg/svc/provisioner/cluster/eks/connector_test.go index 856d14cd5b..c28f17e2cb 100644 --- a/pkg/svc/provisioner/cluster/eks/connector_test.go +++ b/pkg/svc/provisioner/cluster/eks/connector_test.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" + eksclient "github.com/devantler-tech/ksail/v7/pkg/client/eks" eksctlclient "github.com/devantler-tech/ksail/v7/pkg/client/eksctl" "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster/clustererr" eksprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster/eks" @@ -173,6 +174,23 @@ func TestKubeconfigRequiresClusterName(t *testing.T) { require.ErrorIs(t, err, eksprovisioner.ErrClusterNameRequired) } +func TestKubeconfigFailsClosedOnIncompleteMappedCredentials(t *testing.T) { + t.Parallel() + + provisioner, err := eksprovisioner.NewProvisioner( + "eks-default", + "eu-central-1", + "", + eksctlclient.NewClient(), + nil, + eksprovisioner.WithCredentialValues("", "access-without-secret", "", ""), + ) + require.NoError(t, err) + + _, err = provisioner.Kubeconfig(t.Context(), "") + require.ErrorIs(t, err, eksclient.ErrIncompleteStaticCredentials) +} + func TestKubeconfigPropagatesDescribeClusterError(t *testing.T) { t.Parallel() diff --git a/pkg/svc/provisioner/cluster/eks/provisioner.go b/pkg/svc/provisioner/cluster/eks/provisioner.go index 1e7624ffc0..2a8470d117 100644 --- a/pkg/svc/provisioner/cluster/eks/provisioner.go +++ b/pkg/svc/provisioner/cluster/eks/provisioner.go @@ -5,6 +5,7 @@ import ( "fmt" "sync" + eksclient "github.com/devantler-tech/ksail/v7/pkg/client/eks" "github.com/devantler-tech/ksail/v7/pkg/client/eksctl" "github.com/devantler-tech/ksail/v7/pkg/svc/provider" "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster/clustererr" @@ -38,6 +39,11 @@ type Provisioner struct { // minting via the AWS SDK). Injected in tests; lazily resolved from the // operator's AWS credentials otherwise. awsClient AWSClusterAPI + // eksClientOptions pins the credential selection used when awsClient is + // resolved lazily for the Connector capability. + eksClientOptions []eksclient.Option + // requireCredentialValues prevents ambient fallback when custom sources are unset. + requireCredentialValues bool // awsMu guards the lazy awsClient resolution. awsMu sync.Mutex } @@ -53,6 +59,24 @@ func WithAWSClusterAPI(api AWSClusterAPI) Option { } } +// WithCredentialValues pins the credentials used by the provisioner's lazy +// AWS-SDK DescribeCluster/STS client without mutating process environment. +func WithCredentialValues(profile, accessKeyID, secretAccessKey, sessionToken string) Option { + return func(p *Provisioner) { + p.eksClientOptions = []eksclient.Option{ + eksclient.WithCredentialValues(profile, accessKeyID, secretAccessKey, sessionToken), + } + } +} + +// RequireCredentialValues prevents the lazy connector SDK client from falling +// back to ambient canonical credentials when custom sources resolved no values. +func RequireCredentialValues() Option { + return func(p *Provisioner) { + p.requireCredentialValues = true + } +} + // WithKubeconfigPath pins where eksctl writes the created cluster context. func WithKubeconfigPath(path string) Option { return func(p *Provisioner) { @@ -73,13 +97,15 @@ func NewProvisioner( } provisioner := &Provisioner{ - name: name, - region: region, - configPath: configPath, - client: client, - infraProvider: infraProvider, - awsClient: nil, - awsMu: sync.Mutex{}, + name: name, + region: region, + configPath: configPath, + client: client, + infraProvider: infraProvider, + awsClient: nil, + eksClientOptions: nil, + requireCredentialValues: false, + awsMu: sync.Mutex{}, } for _, opt := range opts { diff --git a/pkg/svc/provisioner/cluster/factory_eks.go b/pkg/svc/provisioner/cluster/factory_eks.go index 2c059419c6..d766529175 100644 --- a/pkg/svc/provisioner/cluster/factory_eks.go +++ b/pkg/svc/provisioner/cluster/factory_eks.go @@ -2,15 +2,17 @@ package clusterprovisioner import ( "fmt" + "os" "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" eksctlclient "github.com/devantler-tech/ksail/v7/pkg/client/eksctl" + "github.com/devantler-tech/ksail/v7/pkg/svc/credentials" awsprovider "github.com/devantler-tech/ksail/v7/pkg/svc/provider/aws" eksprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster/eks" ) func (f DefaultFactory) createEKSProvisioner( - _ *v1alpha1.Cluster, + cluster *v1alpha1.Cluster, ) (Provisioner, any, error) { if f.DistributionConfig.EKS == nil { return nil, nil, fmt.Errorf( @@ -20,9 +22,18 @@ func (f DefaultFactory) createEKSProvisioner( } eksConfig := f.DistributionConfig.EKS - client := eksctlclient.NewClient() + client, providerOptions, provisionerOptions := resolveEKSCredentialOptions( + cluster.Spec.Provider.AWS, + ) + provisionerOptions = append(provisionerOptions, + eksprovisioner.WithKubeconfigPath(eksConfig.KubeconfigPath), + ) - infraProvider, err := awsprovider.NewProvider(client, eksConfig.Region) + infraProvider, err := awsprovider.NewProvider( + client, + eksConfig.Region, + providerOptions..., + ) if err != nil { return nil, nil, fmt.Errorf("failed to create AWS provider: %w", err) } @@ -33,7 +44,7 @@ func (f DefaultFactory) createEKSProvisioner( eksConfig.ConfigPath, client, infraProvider, - eksprovisioner.WithKubeconfigPath(eksConfig.KubeconfigPath), + provisionerOptions..., ) if err != nil { return nil, nil, fmt.Errorf("failed to create EKS provisioner: %w", err) @@ -41,3 +52,32 @@ func (f DefaultFactory) createEKSProvisioner( return provisioner, eksConfig, nil } + +func resolveEKSCredentialOptions( + awsOptions v1alpha1.OptionsAWS, +) (*eksctlclient.Client, []awsprovider.Option, []eksprovisioner.Option) { + auth := credentials.ResolveAWS(credentials.NewAWSOptionsResolver(awsOptions)) + eksctlOptions := []eksctlclient.Option{ + eksctlclient.WithEnvironment(auth.ChildEnvironment(os.Environ())), + } + providerOptions := []awsprovider.Option{awsprovider.WithCredentialValues( + auth.Profile, + auth.AccessKeyID, + auth.SecretAccessKey, + auth.SessionToken, + )} + + provisionerOptions := []eksprovisioner.Option{eksprovisioner.WithCredentialValues( + auth.Profile, + auth.AccessKeyID, + auth.SecretAccessKey, + auth.SessionToken, + )} + if auth.HasCustomCredentialSources() { + eksctlOptions = append(eksctlOptions, eksctlclient.RequireCredentialValues()) + providerOptions = append(providerOptions, awsprovider.RequireCredentialValues()) + provisionerOptions = append(provisionerOptions, eksprovisioner.RequireCredentialValues()) + } + + return eksctlclient.NewClient(eksctlOptions...), providerOptions, provisionerOptions +} diff --git a/pkg/svc/provisioner/cluster/factory_eks_test.go b/pkg/svc/provisioner/cluster/factory_eks_test.go index cf0881bef0..abeb971b38 100644 --- a/pkg/svc/provisioner/cluster/factory_eks_test.go +++ b/pkg/svc/provisioner/cluster/factory_eks_test.go @@ -31,14 +31,35 @@ func TestCreateEKSProvisionerPinsKubeconfigPath(t *testing.T) { binDir := t.TempDir() argsPath := filepath.Join(t.TempDir(), "args") eksctlPath := filepath.Join(binDir, "eksctl") - //nolint:gosec // the test fixture must be executable and contains no user input - require.NoError(t, os.WriteFile( - eksctlPath, - []byte("#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$KSAIL_EKSCTL_ARGS\"\n"), - 0o700, - )) + writeExecutableFixture(t, eksctlPath, `#!/bin/sh +[ "${AWS_PROFILE-}" = "selected-profile" ] || exit 41 +[ "${AWS_ACCESS_KEY_ID-}" = "fixture-access" ] || exit 42 +[ "${AWS_SECRET_ACCESS_KEY-}" = "fixture-secret" ] || exit 43 +[ "${AWS_SESSION_TOKEN-}" = "fixture-session" ] || exit 44 +[ -z "${KSAIL_PROFILE+x}" ] || exit 45 +[ -z "${KSAIL_ACCESS+x}" ] || exit 46 +[ -z "${KSAIL_SECRET+x}" ] || exit 47 +[ -z "${KSAIL_SESSION+x}" ] || exit 48 +printf '%s\n' "$@" > "$KSAIL_EKSCTL_ARGS" +`) t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) t.Setenv("KSAIL_EKSCTL_ARGS", argsPath) + t.Setenv("KSAIL_PROFILE", "selected-profile") + t.Setenv("KSAIL_ACCESS", "fixture-access") + t.Setenv("KSAIL_SECRET", "fixture-secret") + t.Setenv("KSAIL_SESSION", "fixture-session") + t.Setenv("AWS_PROFILE", "stale-profile") + t.Setenv("AWS_ACCESS_KEY_ID", "stale-access") + t.Setenv("AWS_SECRET_ACCESS_KEY", "stale-secret") + t.Setenv("AWS_SESSION_TOKEN", "stale-session") + + cluster := eksTestCluster() + cluster.Spec.Provider.AWS = v1alpha1.OptionsAWS{ + ProfileEnvVar: "KSAIL_PROFILE", + AccessKeyIDEnvVar: "KSAIL_ACCESS", + SecretAccessKeyEnvVar: "KSAIL_SECRET", + SessionTokenEnvVar: "KSAIL_SESSION", + } factory := clusterprovisioner.DefaultFactory{ DistributionConfig: &clusterprovisioner.DistributionConfig{ @@ -51,7 +72,7 @@ func TestCreateEKSProvisionerPinsKubeconfigPath(t *testing.T) { }, } - provisioner, _, err := factory.Create(context.Background(), eksTestCluster()) + provisioner, _, err := factory.Create(context.Background(), cluster) require.NoError(t, err) require.NoError(t, provisioner.Create(t.Context(), "test-eks")) @@ -66,6 +87,17 @@ func TestCreateEKSProvisionerPinsKubeconfigPath(t *testing.T) { }, strings.Fields(string(args))) } +func writeExecutableFixture(t *testing.T, path, contents string) { + t.Helper() + + require.NoError(t, os.WriteFile(path, []byte(contents), 0o600)) + require.NoError( + t, + //nolint:gosec // owner execute is required for the fixture. + os.Chmod(path, 0o700), + ) +} + // TestCreateEKSProvisionerWithConfig asserts a populated EKSConfig yields an EKS // provisioner. The eksctl client shells out to the binary only at call time and // the AWS provider resolves credentials lazily, so no live AWS access is needed From e41b526ec23c12afc502424ce625852aecd9aaf5 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Mon, 13 Jul 2026 13:24:05 +0200 Subject: [PATCH 2/4] refactor(eks): centralize AWS credential options --- pkg/cli/cmd/cluster/info.go | 20 ++--- pkg/client/eks/client.go | 16 ++++ pkg/client/eks/client_test.go | 29 +++++++ pkg/svc/clusterdiscovery/cloud.go | 20 ++--- pkg/svc/credentials/aws_environment_test.go | 84 ++++++++++++++++++++ pkg/svc/credentials/credentials.go | 51 ++++++++++++ pkg/svc/provider/aws/provider.go | 12 +-- pkg/svc/provisioner/cluster/eks/connector.go | 12 +-- pkg/svc/provisioner/cluster/factory_eks.go | 30 +++---- 9 files changed, 213 insertions(+), 61 deletions(-) diff --git a/pkg/cli/cmd/cluster/info.go b/pkg/cli/cmd/cluster/info.go index c02c1d3468..c1d5367c76 100644 --- a/pkg/cli/cmd/cluster/info.go +++ b/pkg/cli/cmd/cluster/info.go @@ -342,20 +342,12 @@ func getAWSProviderStatus( region string, ) (*provider.ClusterStatus, error) { auth := credentials.ResolveAWS(credentials.NewAWSOptionsResolver(awsOpts)) - eksctlOptions := []eksctlclient.Option{ - eksctlclient.WithEnvironment(auth.ChildEnvironment(os.Environ())), - } - - providerOptions := []awsprovider.Option{awsprovider.WithCredentialValues( - auth.Profile, - auth.AccessKeyID, - auth.SecretAccessKey, - auth.SessionToken, - )} - if auth.HasCustomCredentialSources() { - eksctlOptions = append(eksctlOptions, eksctlclient.RequireCredentialValues()) - providerOptions = append(providerOptions, awsprovider.RequireCredentialValues()) - } + eksctlOptions := credentials.OptionsForAWSChildEnvironment( + auth, os.Environ(), eksctlclient.WithEnvironment, eksctlclient.RequireCredentialValues, + ) + providerOptions := credentials.OptionsForAWSResolution( + auth, awsprovider.WithCredentialValues, awsprovider.RequireCredentialValues, + ) return awsProviderStatus( ctx, diff --git a/pkg/client/eks/client.go b/pkg/client/eks/client.go index b47b2b5b73..ad23faacc4 100644 --- a/pkg/client/eks/client.go +++ b/pkg/client/eks/client.go @@ -131,6 +131,22 @@ func RequireCredentialValues() Option { } } +// NewClientWithCredentialRequirement constructs a Client from an immutable +// option snapshot, adding the fail-closed credential requirement when needed. +func NewClientWithCredentialRequirement( + ctx context.Context, + region string, + required bool, + options ...Option, +) (*Client, error) { + result := append([]Option(nil), options...) + if required { + result = append(result, RequireCredentialValues()) + } + + return NewClient(ctx, region, result...) +} + // NewClient constructs a Client. Unless both seams are injected, it resolves // the AWS default configuration (env, shared config, IRSA / instance // role) once and builds the real SDK clients from it. diff --git a/pkg/client/eks/client_test.go b/pkg/client/eks/client_test.go index 76995fcc04..5d913c7fb9 100644 --- a/pkg/client/eks/client_test.go +++ b/pkg/client/eks/client_test.go @@ -256,6 +256,35 @@ func TestRequireCredentialValuesRejectsAmbientFallback(t *testing.T) { require.ErrorIs(t, err, eksclient.ErrExplicitCredentialsUnavailable) } +func TestNewClientWithCredentialRequirementRejectsAmbientFallbackWhenRequired(t *testing.T) { + t.Parallel() + + base := []eksclient.Option{ + eksclient.WithClusterDescriber(fakeDescriber{}), + eksclient.WithCallerIdentityPresigner(fakePresigner{}), + eksclient.WithCredentialValues("", "", "", ""), + } + _, err := eksclient.NewClientWithCredentialRequirement( + t.Context(), "eu-central-1", true, base..., + ) + require.ErrorIs(t, err, eksclient.ErrExplicitCredentialsUnavailable) + require.Len(t, base, 3, "the caller-owned option slice must remain unchanged") +} + +func TestNewClientWithCredentialRequirementPreservesDefaultChainWhenOptional(t *testing.T) { + t.Parallel() + + base := []eksclient.Option{ + eksclient.WithClusterDescriber(fakeDescriber{}), + eksclient.WithCallerIdentityPresigner(fakePresigner{}), + } + client, err := eksclient.NewClientWithCredentialRequirement( + t.Context(), "eu-central-1", false, base..., + ) + require.NoError(t, err) + assert.NotNil(t, client) +} + func assertTokenCredentialPrefix(t *testing.T, token, expected string) { t.Helper() diff --git a/pkg/svc/clusterdiscovery/cloud.go b/pkg/svc/clusterdiscovery/cloud.go index 6421e4bfba..8690c703bd 100644 --- a/pkg/svc/clusterdiscovery/cloud.go +++ b/pkg/svc/clusterdiscovery/cloud.go @@ -86,20 +86,12 @@ func (d *Discoverer) listAWS(ctx context.Context) ([]Cluster, error) { } auth := credentials.ResolveAWS(d.resolver()) - eksctlOptions := []eksctlclient.Option{ - eksctlclient.WithEnvironment(auth.ChildEnvironment(os.Environ())), - } - - providerOptions := []awsprovider.Option{awsprovider.WithCredentialValues( - auth.Profile, - auth.AccessKeyID, - auth.SecretAccessKey, - auth.SessionToken, - )} - if auth.HasCustomCredentialSources() { - eksctlOptions = append(eksctlOptions, eksctlclient.RequireCredentialValues()) - providerOptions = append(providerOptions, awsprovider.RequireCredentialValues()) - } + eksctlOptions := credentials.OptionsForAWSChildEnvironment( + auth, os.Environ(), eksctlclient.WithEnvironment, eksctlclient.RequireCredentialValues, + ) + providerOptions := credentials.OptionsForAWSResolution( + auth, awsprovider.WithCredentialValues, awsprovider.RequireCredentialValues, + ) client := eksctlclient.NewClient(eksctlOptions...) diff --git a/pkg/svc/credentials/aws_environment_test.go b/pkg/svc/credentials/aws_environment_test.go index 45037f4592..cec80a037c 100644 --- a/pkg/svc/credentials/aws_environment_test.go +++ b/pkg/svc/credentials/aws_environment_test.go @@ -189,6 +189,90 @@ func TestAWSResolution_ReportsCustomCredentialSourcesEvenWhenUnset(t *testing.T) assert.True(t, resolution.HasCustomCredentialSources()) } +func TestOptionsForAWSResolutionMapsValuesAndCustomRequirement(t *testing.T) { + t.Parallel() + + type option struct { + profile string + accessKeyID string + secretKey string + sessionToken string + required bool + } + + resolution := credentials.ResolveAWS(awsResolverFixture{ + envVars: map[credentials.Key]string{ + credentials.AWSProfile: "KSAIL_PROFILE", + }, + values: map[credentials.Key]string{ + credentials.AWSProfile: "selected-profile", + credentials.AWSAccessKeyID: "selected-access", + credentials.AWSSecretAccessKey: "selected-secret", + credentials.AWSSessionToken: "selected-session", + }, + }) + options := credentials.OptionsForAWSResolution( + resolution, + func(profile, accessKeyID, secretKey, sessionToken string) option { + return option{ + profile: profile, + accessKeyID: accessKeyID, + secretKey: secretKey, + sessionToken: sessionToken, + required: false, + } + }, + func() option { return option{required: true} }, + ) + + require.Len(t, options, 2) + assert.Equal(t, option{ + profile: "selected-profile", + accessKeyID: "selected-access", + secretKey: "selected-secret", + sessionToken: "selected-session", + required: false, + }, options[0]) + assert.True(t, options[1].required) +} + +func TestOptionsForAWSResolutionPreservesDefaultCredentialChain(t *testing.T) { + t.Parallel() + + resolution := credentials.ResolveAWS(awsResolverFixture{}) + options := credentials.OptionsForAWSResolution( + resolution, + func(_, _, _, _ string) string { return "values" }, + func() string { return "required" }, + ) + + assert.Equal(t, []string{"values"}, options) +} + +func TestOptionsForAWSChildEnvironmentCanonicalizesAndRequiresCustomSources(t *testing.T) { + t.Parallel() + + resolution := credentials.ResolveAWS(awsResolverFixture{ + envVars: map[credentials.Key]string{ + credentials.AWSProfile: "KSAIL_PROFILE", + }, + values: map[credentials.Key]string{ + credentials.AWSProfile: "selected-profile", + }, + }) + options := credentials.OptionsForAWSChildEnvironment( + resolution, + []string{"PATH=/usr/bin", "AWS_PROFILE=stale-profile"}, + func(environment []string) []string { return environment }, + func() []string { return []string{"required"} }, + ) + + require.Len(t, options, 2) + assertEnvEntry(t, options[0], "PATH", "usr/bin") + assertEnvEntry(t, options[0], "AWS_PROFILE", "selected-profile") + assert.Equal(t, []string{"required"}, options[1]) +} + func TestAWSResolution_ChildEnvironmentPreservesCanonicalDefaults(t *testing.T) { t.Parallel() diff --git a/pkg/svc/credentials/credentials.go b/pkg/svc/credentials/credentials.go index 48b635a6a2..6411ad1f2c 100644 --- a/pkg/svc/credentials/credentials.go +++ b/pkg/svc/credentials/credentials.go @@ -234,6 +234,57 @@ func (r AWSResolution) HasCustomCredentialSources() bool { return r.hasCustomCredentialEnv } +// OptionsForAWSResolution maps a resolved AWS identity into a consumer's +// option type. Custom source names add the consumer's fail-closed option so an +// unset alias cannot silently fall back to an unrelated ambient identity. +func OptionsForAWSResolution[T any]( + resolution AWSResolution, + withCredentialValues func(profile, accessKeyID, secretAccessKey, sessionToken string) T, + requireCredentialValues func() T, +) []T { + option := withCredentialValues( + resolution.Profile, + resolution.AccessKeyID, + resolution.SecretAccessKey, + resolution.SessionToken, + ) + + return optionsWithCredentialRequirement( + option, + resolution.HasCustomCredentialSources(), + requireCredentialValues, + ) +} + +// OptionsForAWSChildEnvironment maps an AWS resolution's isolated child +// environment into a consumer's option type and adds its fail-closed option +// when credential aliases are custom. +func OptionsForAWSChildEnvironment[T any]( + resolution AWSResolution, + parent []string, + withEnvironment func(environment []string) T, + requireCredentialValues func() T, +) []T { + return optionsWithCredentialRequirement( + withEnvironment(resolution.ChildEnvironment(parent)), + resolution.HasCustomCredentialSources(), + requireCredentialValues, + ) +} + +func optionsWithCredentialRequirement[T any]( + option T, + required bool, + requireCredentialValues func() T, +) []T { + options := []T{option} + if required { + options = append(options, requireCredentialValues()) + } + + return options +} + // ChildEnvironment returns a copy of parent with AWS credential aliases and // stale canonical values removed, followed by the non-empty resolved values // under the canonical names eksctl understands. When a custom credential source diff --git a/pkg/svc/provider/aws/provider.go b/pkg/svc/provider/aws/provider.go index 97e8202b8d..e804b5daa3 100644 --- a/pkg/svc/provider/aws/provider.go +++ b/pkg/svc/provider/aws/provider.go @@ -292,12 +292,12 @@ func (p *Provider) resolveDescriber(ctx context.Context) (clusterDescriber, erro return p.describer, nil } - options := append([]eksclient.Option{}, p.eksClientOptions...) - if p.requireCredentialValues { - options = append(options, eksclient.RequireCredentialValues()) - } - - client, err := eksclient.NewClient(ctx, p.region, options...) + client, err := eksclient.NewClientWithCredentialRequirement( + ctx, + p.region, + p.requireCredentialValues, + p.eksClientOptions..., + ) if err != nil { return nil, fmt.Errorf("creating aws eks client: %w", err) } diff --git a/pkg/svc/provisioner/cluster/eks/connector.go b/pkg/svc/provisioner/cluster/eks/connector.go index a8246e9b62..b46384b9b7 100644 --- a/pkg/svc/provisioner/cluster/eks/connector.go +++ b/pkg/svc/provisioner/cluster/eks/connector.go @@ -116,12 +116,12 @@ func (p *Provisioner) resolveAWSClient(ctx context.Context) (AWSClusterAPI, erro return p.awsClient, nil } - options := append([]eksclient.Option{}, p.eksClientOptions...) - if p.requireCredentialValues { - options = append(options, eksclient.RequireCredentialValues()) - } - - client, err := eksclient.NewClient(ctx, p.region, options...) + client, err := eksclient.NewClientWithCredentialRequirement( + ctx, + p.region, + p.requireCredentialValues, + p.eksClientOptions..., + ) if err != nil { return nil, fmt.Errorf("creating aws eks client: %w", err) } diff --git a/pkg/svc/provisioner/cluster/factory_eks.go b/pkg/svc/provisioner/cluster/factory_eks.go index d766529175..887645b94b 100644 --- a/pkg/svc/provisioner/cluster/factory_eks.go +++ b/pkg/svc/provisioner/cluster/factory_eks.go @@ -57,27 +57,15 @@ func resolveEKSCredentialOptions( awsOptions v1alpha1.OptionsAWS, ) (*eksctlclient.Client, []awsprovider.Option, []eksprovisioner.Option) { auth := credentials.ResolveAWS(credentials.NewAWSOptionsResolver(awsOptions)) - eksctlOptions := []eksctlclient.Option{ - eksctlclient.WithEnvironment(auth.ChildEnvironment(os.Environ())), - } - providerOptions := []awsprovider.Option{awsprovider.WithCredentialValues( - auth.Profile, - auth.AccessKeyID, - auth.SecretAccessKey, - auth.SessionToken, - )} - - provisionerOptions := []eksprovisioner.Option{eksprovisioner.WithCredentialValues( - auth.Profile, - auth.AccessKeyID, - auth.SecretAccessKey, - auth.SessionToken, - )} - if auth.HasCustomCredentialSources() { - eksctlOptions = append(eksctlOptions, eksctlclient.RequireCredentialValues()) - providerOptions = append(providerOptions, awsprovider.RequireCredentialValues()) - provisionerOptions = append(provisionerOptions, eksprovisioner.RequireCredentialValues()) - } + eksctlOptions := credentials.OptionsForAWSChildEnvironment( + auth, os.Environ(), eksctlclient.WithEnvironment, eksctlclient.RequireCredentialValues, + ) + providerOptions := credentials.OptionsForAWSResolution( + auth, awsprovider.WithCredentialValues, awsprovider.RequireCredentialValues, + ) + provisionerOptions := credentials.OptionsForAWSResolution( + auth, eksprovisioner.WithCredentialValues, eksprovisioner.RequireCredentialValues, + ) return eksctlclient.NewClient(eksctlOptions...), providerOptions, provisionerOptions } From ba5437bc783391a70b8b566f3ee7fcf9d3d3b8f4 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Mon, 13 Jul 2026 15:18:10 +0200 Subject: [PATCH 3/4] fix(eks): address credential review findings --- pkg/cli/cmd/cluster/info.go | 13 +-- pkg/cli/cmd/cluster/info_test.go | 3 + pkg/cli/lifecycle/awsregion_test.go | 2 + pkg/cli/lifecycle/simple.go | 91 +++++++------------ pkg/client/eks/client_test.go | 7 ++ pkg/client/eksctl/client.go | 7 +- pkg/client/eksctl/client_test.go | 41 ++++++++- .../clusterdiscovery/aws_environment_test.go | 5 + pkg/svc/clusterdiscovery/cloud.go | 13 +-- pkg/svc/credentials/aws_environment_test.go | 70 ++++++++++++++ pkg/svc/credentials/credentials.go | 63 ++++++++++--- pkg/svc/credentials/environment_name_test.go | 2 + .../provisioner/cluster/eks/connector_test.go | 1 + pkg/svc/provisioner/cluster/factory_eks.go | 15 +-- .../provisioner/cluster/factory_eks_test.go | 1 + 15 files changed, 243 insertions(+), 91 deletions(-) diff --git a/pkg/cli/cmd/cluster/info.go b/pkg/cli/cmd/cluster/info.go index c1d5367c76..99e298634f 100644 --- a/pkg/cli/cmd/cluster/info.go +++ b/pkg/cli/cmd/cluster/info.go @@ -341,12 +341,13 @@ func getAWSProviderStatus( awsOpts v1alpha1.OptionsAWS, region string, ) (*provider.ClusterStatus, error) { - auth := credentials.ResolveAWS(credentials.NewAWSOptionsResolver(awsOpts)) - eksctlOptions := credentials.OptionsForAWSChildEnvironment( - auth, os.Environ(), eksctlclient.WithEnvironment, eksctlclient.RequireCredentialValues, - ) - providerOptions := credentials.OptionsForAWSResolution( - auth, awsprovider.WithCredentialValues, awsprovider.RequireCredentialValues, + _, eksctlOptions, providerOptions := credentials.ResolveAWSClientOptions( + credentials.NewAWSOptionsResolver(awsOpts), + os.Environ(), + eksctlclient.WithEnvironment, + eksctlclient.RequireCredentialValues, + awsprovider.WithCredentialValues, + awsprovider.RequireCredentialValues, ) return awsProviderStatus( diff --git a/pkg/cli/cmd/cluster/info_test.go b/pkg/cli/cmd/cluster/info_test.go index c3506e3bf7..2b28cd4b6c 100644 --- a/pkg/cli/cmd/cluster/info_test.go +++ b/pkg/cli/cmd/cluster/info_test.go @@ -220,6 +220,8 @@ func TestAWSProviderStatus_ForwardsRegion(t *testing.T) { ) } +// TestInfoCommandMapsCustomAWSCredentialsIntoEksctl verifies custom aliases +// reach eksctl canonically without mutating ambient credentials. func TestInfoCommandMapsCustomAWSCredentialsIntoEksctl(t *testing.T) { workingDir := t.TempDir() t.Chdir(workingDir) @@ -276,6 +278,7 @@ func TestInfoCommandMapsCustomAWSCredentialsIntoEksctl(t *testing.T) { assert.Equal(t, "stale-profile", os.Getenv("AWS_PROFILE")) } +// writeExecutableFixture writes a private executable used to stand in for eksctl. func writeExecutableFixture(t *testing.T, path, contents string) { t.Helper() diff --git a/pkg/cli/lifecycle/awsregion_test.go b/pkg/cli/lifecycle/awsregion_test.go index 126e12f1a7..b2ecd409cf 100644 --- a/pkg/cli/lifecycle/awsregion_test.go +++ b/pkg/cli/lifecycle/awsregion_test.go @@ -52,6 +52,8 @@ func TestResolveAWSRegion(t *testing.T) { }) } +// TestResolveClusterInfoRetainsAWSCredentialMappings verifies lifecycle +// resolution preserves immutable AWS credential-name mappings. func TestResolveClusterInfoRetainsAWSCredentialMappings(t *testing.T) { workingDir := t.TempDir() t.Chdir(workingDir) diff --git a/pkg/cli/lifecycle/simple.go b/pkg/cli/lifecycle/simple.go index 8869d058c1..34f54f4f35 100644 --- a/pkg/cli/lifecycle/simple.go +++ b/pkg/cli/lifecycle/simple.go @@ -166,57 +166,42 @@ func ResolveClusterInfo( providerFlag v1alpha1.Provider, kubeconfigFlag string, ) (*ResolvedClusterInfo, error) { - clusterName := nameFlag - provider := providerFlag - kubeconfigPath := kubeconfigFlag - - // Always load config to fill missing fields and extract Omni/Kubernetes options. - // Even when --name is provided, we still need Omni endpoint from config. - var ( - omniOpts v1alpha1.OptionsOmni - kubernetesOpts v1alpha1.OptionsKubernetes - awsOpts v1alpha1.OptionsAWS - awsRegion string - ) + resolved := ResolvedClusterInfo{ + ClusterName: nameFlag, + Provider: providerFlag, + KubeconfigPath: kubeconfigFlag, + } - resolveFromConfig( - cmd, - &clusterName, - &provider, - &kubeconfigPath, - &omniOpts, - &kubernetesOpts, - &awsOpts, - &awsRegion, - ) + // Always load config to fill missing fields and extract provider options. + // Even when --name is provided, provider-specific settings are still needed. + resolveFromConfig(cmd, &resolved) // Fall back to kubeconfig context detection - if clusterName == "" { - resolveFromKubecontext(commandContext(cmd), &clusterName, &provider, kubeconfigPath) + if resolved.ClusterName == "" { + resolveFromKubecontext( + commandContext(cmd), + &resolved.ClusterName, + &resolved.Provider, + resolved.KubeconfigPath, + ) } - if clusterName == "" { + if resolved.ClusterName == "" { return nil, ErrClusterNameRequired } - if provider == "" { - provider = v1alpha1.ProviderDocker + if resolved.Provider == "" { + resolved.Provider = v1alpha1.ProviderDocker } - resolvedPath, err := clusterdetector.ResolveKubeconfigPath(kubeconfigPath) + resolvedPath, err := clusterdetector.ResolveKubeconfigPath(resolved.KubeconfigPath) if err != nil { return nil, fmt.Errorf("resolve kubeconfig path: %w", err) } - return &ResolvedClusterInfo{ - ClusterName: clusterName, - Provider: provider, - KubeconfigPath: resolvedPath, - OmniOpts: omniOpts, - KubernetesOpts: kubernetesOpts, - AWSOpts: awsOpts, - AWSRegion: awsRegion, - }, nil + resolved.KubeconfigPath = resolvedPath + + return &resolved, nil } // loadConfig loads the ksail.yaml config, honoring the --config flag when cmd is non-nil. @@ -246,41 +231,35 @@ func loadConfig(cmd *cobra.Command) (*v1alpha1.Cluster, *clusterprovisioner.Dist // Fields that already have values (from flags) are not overwritten. func resolveFromConfig( cmd *cobra.Command, - clusterName *string, - provider *v1alpha1.Provider, - kubeconfigPath *string, - omniOpts *v1alpha1.OptionsOmni, - kubernetesOpts *v1alpha1.OptionsKubernetes, - awsOpts *v1alpha1.OptionsAWS, - awsRegion *string, + resolved *ResolvedClusterInfo, ) { cfg, distCfg := loadConfig(cmd) if cfg == nil { return } - if *clusterName == "" && cfg.Name != "" { + if resolved.ClusterName == "" && cfg.Name != "" { if v1alpha1.ValidateClusterName(cfg.Name) == nil { - *clusterName = cfg.Name + resolved.ClusterName = cfg.Name } } - if *clusterName == "" { - *clusterName = ClusterNameFromDistributionConfig(distCfg) + if resolved.ClusterName == "" { + resolved.ClusterName = ClusterNameFromDistributionConfig(distCfg) } - if *provider == "" && cfg.Spec.Cluster.Provider != "" { - *provider = cfg.Spec.Cluster.Provider + if resolved.Provider == "" && cfg.Spec.Cluster.Provider != "" { + resolved.Provider = cfg.Spec.Cluster.Provider } - if *kubeconfigPath == "" && cfg.Spec.Cluster.Connection.Kubeconfig != "" { - *kubeconfigPath = cfg.Spec.Cluster.Connection.Kubeconfig + if resolved.KubeconfigPath == "" && cfg.Spec.Cluster.Connection.Kubeconfig != "" { + resolved.KubeconfigPath = cfg.Spec.Cluster.Connection.Kubeconfig } - *omniOpts = cfg.Spec.Provider.Omni - *kubernetesOpts = cfg.Spec.Provider.Kubernetes - *awsOpts = cfg.Spec.Provider.AWS - *awsRegion = ResolveAWSRegion(cfg.Spec.Provider.AWS, distCfg) + resolved.OmniOpts = cfg.Spec.Provider.Omni + resolved.KubernetesOpts = cfg.Spec.Provider.Kubernetes + resolved.AWSOpts = cfg.Spec.Provider.AWS + resolved.AWSRegion = ResolveAWSRegion(cfg.Spec.Provider.AWS, distCfg) } // commandContext returns cmd's context, falling back to context.Background() diff --git a/pkg/client/eks/client_test.go b/pkg/client/eks/client_test.go index 5d913c7fb9..2ee26722d1 100644 --- a/pkg/client/eks/client_test.go +++ b/pkg/client/eks/client_test.go @@ -178,6 +178,7 @@ func TestMintTokenSignsClusterBindingHeaders(t *testing.T) { assert.Contains(t, query.Get("X-Amz-SignedHeaders"), "x-k8s-aws-id") } +// TestWithCredentialValues_StaticCredentialsOverrideAmbientIdentity verifies explicit static credentials win over ambient identity. func TestWithCredentialValues_StaticCredentialsOverrideAmbientIdentity(t *testing.T) { // Not parallel: t.Setenv changes the process environment. t.Setenv("AWS_ACCESS_KEY_ID", "STALEAMBIENT") @@ -201,6 +202,7 @@ func TestWithCredentialValues_StaticCredentialsOverrideAmbientIdentity(t *testin assertTokenCredentialPrefix(t, token, "SELECTEDACCESS/") } +// TestWithCredentialValues_ProfileOverridesAmbientStaticCredentials verifies an explicit profile wins over ambient static credentials. func TestWithCredentialValues_ProfileOverridesAmbientStaticCredentials(t *testing.T) { // Not parallel: t.Setenv changes the process environment. credentialsFile := filepath.Join(t.TempDir(), "credentials") @@ -229,6 +231,7 @@ func TestWithCredentialValues_ProfileOverridesAmbientStaticCredentials(t *testin assertTokenCredentialPrefix(t, token, "PROFILEACCESS/") } +// TestWithCredentialValues_RejectsPartialStaticCredentials verifies incomplete explicit key pairs fail closed. func TestWithCredentialValues_RejectsPartialStaticCredentials(t *testing.T) { t.Parallel() @@ -242,6 +245,7 @@ func TestWithCredentialValues_RejectsPartialStaticCredentials(t *testing.T) { require.ErrorIs(t, err, eksclient.ErrIncompleteStaticCredentials) } +// TestRequireCredentialValuesRejectsAmbientFallback verifies required explicit credentials cannot fall back to ambient identity. func TestRequireCredentialValuesRejectsAmbientFallback(t *testing.T) { t.Parallel() @@ -256,6 +260,7 @@ func TestRequireCredentialValuesRejectsAmbientFallback(t *testing.T) { require.ErrorIs(t, err, eksclient.ErrExplicitCredentialsUnavailable) } +// TestNewClientWithCredentialRequirementRejectsAmbientFallbackWhenRequired verifies required construction rejects empty resolved credentials. func TestNewClientWithCredentialRequirementRejectsAmbientFallbackWhenRequired(t *testing.T) { t.Parallel() @@ -271,6 +276,7 @@ func TestNewClientWithCredentialRequirementRejectsAmbientFallbackWhenRequired(t require.Len(t, base, 3, "the caller-owned option slice must remain unchanged") } +// TestNewClientWithCredentialRequirementPreservesDefaultChainWhenOptional verifies optional construction retains the AWS default chain. func TestNewClientWithCredentialRequirementPreservesDefaultChainWhenOptional(t *testing.T) { t.Parallel() @@ -285,6 +291,7 @@ func TestNewClientWithCredentialRequirementPreservesDefaultChainWhenOptional(t * assert.NotNil(t, client) } +// assertTokenCredentialPrefix verifies the minted token was signed by the expected access key. func assertTokenCredentialPrefix(t *testing.T, token, expected string) { t.Helper() diff --git a/pkg/client/eksctl/client.go b/pkg/client/eksctl/client.go index 95896cb9e7..5f75ba3b28 100644 --- a/pkg/client/eksctl/client.go +++ b/pkg/client/eksctl/client.go @@ -206,6 +206,7 @@ func (c *Client) ExecWithStdin( return stdout, stderr, nil } +// run validates the credential boundary before invoking the runner with a defensive environment snapshot. func (c *Client) run( ctx context.Context, args []string, @@ -244,6 +245,7 @@ func (c *Client) run( return stdout, stderr, nil } +// validateCredentialValues rejects incomplete or required-but-empty explicit selections before process execution. func (c *Client) validateCredentialValues() error { if !c.requireCredentialValues { return nil @@ -268,6 +270,7 @@ func (c *Client) validateCredentialValues() error { return nil } +// cloneStrings returns a defensive copy while preserving nil as the parent-environment inheritance sentinel. func cloneStrings(values []string) []string { if values == nil { return nil @@ -276,6 +279,7 @@ func cloneStrings(values []string) []string { return append([]string{}, values...) } +// redactCredentialValues removes resolved secret values from stderr before it reaches callers or wrapped errors. func (c *Client) redactCredentialValues(stderr []byte) []byte { if len(stderr) == 0 || len(c.environment) == 0 { return stderr @@ -290,7 +294,7 @@ func (c *Client) redactCredentialValues(stderr []byte) []byte { } switch name { - case "AWS_PROFILE", "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN": + case "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN": uniqueValues[value] = struct{}{} } } @@ -312,6 +316,7 @@ func (c *Client) redactCredentialValues(stderr []byte) []byte { return []byte(redacted) } +// environmentValues parses child-environment entries with the last value for each name taking precedence. func environmentValues(environment []string) map[string]string { values := make(map[string]string, len(environment)) for _, entry := range environment { diff --git a/pkg/client/eksctl/client_test.go b/pkg/client/eksctl/client_test.go index 51394fa688..9c3c4d844e 100644 --- a/pkg/client/eksctl/client_test.go +++ b/pkg/client/eksctl/client_test.go @@ -33,6 +33,7 @@ type fakeRunner struct { type legacyRunner struct{} +// Run implements the legacy runner contract without explicit-environment support for fail-closed tests. func (legacyRunner) Run( context.Context, string, @@ -51,6 +52,7 @@ func (f *fakeRunner) Run( return f.run(ctx, name, args, stdin, nil) } +// RunWithEnvironment records an explicitly supplied environment through the shared fake execution path. func (f *fakeRunner) RunWithEnvironment( ctx context.Context, name string, @@ -61,6 +63,7 @@ func (f *fakeRunner) RunWithEnvironment( return f.run(ctx, name, args, stdin, environment) } +// run records one fake invocation and can mutate its input to exercise defensive-copy guarantees. func (f *fakeRunner) run( _ context.Context, name string, @@ -113,6 +116,8 @@ func TestWithBinary_EmptyIgnored(t *testing.T) { assert.Equal(t, eksctl.DefaultBinary, client.Binary()) } +// TestWithEnvironment_ForwardsAnIsolatedSnapshot verifies callers and runners +// cannot mutate the client's saved environment. func TestWithEnvironment_ForwardsAnIsolatedSnapshot(t *testing.T) { t.Parallel() @@ -142,6 +147,8 @@ func TestWithEnvironment_ForwardsAnIsolatedSnapshot(t *testing.T) { assert.Equal(t, []string{"HOME=/tmp/ksail", "AWS_PROFILE=custom-profile"}, runner.lastEnv) } +// TestNewClient_DefaultEnvironmentInheritsParent verifies a nil explicit +// environment retains normal process inheritance. func TestNewClient_DefaultEnvironmentInheritsParent(t *testing.T) { t.Parallel() @@ -153,6 +160,7 @@ func TestNewClient_DefaultEnvironmentInheritsParent(t *testing.T) { assert.Nil(t, runner.lastEnv) } +// TestWithEnvironment_FailsClosedForLegacyRunner verifies isolation is never silently dropped by an older runner. func TestWithEnvironment_FailsClosedForLegacyRunner(t *testing.T) { t.Parallel() @@ -165,6 +173,8 @@ func TestWithEnvironment_FailsClosedForLegacyRunner(t *testing.T) { require.ErrorIs(t, err, eksctl.ErrRunnerEnvironmentUnsupported) } +// TestExec_RedactsCredentialValuesFromStderrErrors verifies secret values are +// absent from returned stderr and wrapped errors. func TestExec_RedactsCredentialValuesFromStderrErrors(t *testing.T) { t.Parallel() @@ -187,6 +197,32 @@ func TestExec_RedactsCredentialValuesFromStderrErrors(t *testing.T) { assert.Contains(t, err.Error(), "[REDACTED]") } +// TestExec_PreservesProfileNamesWhileRedactingSecrets verifies diagnostic +// selectors remain readable while secrets are removed. +func TestExec_PreservesProfileNamesWhileRedactingSecrets(t *testing.T) { + t.Parallel() + + runner := &fakeRunner{ + stderr: []byte("provider rejected selected-profile using fixture-secret-value"), + err: errExitStatus1, + } + client := eksctl.NewClient( + eksctl.WithRunner(runner), + eksctl.WithEnvironment([]string{ + "AWS_PROFILE=selected-profile", + "AWS_SECRET_ACCESS_KEY=fixture-secret-value", + }), + ) + + _, stderr, err := client.Exec(t.Context(), "get", "cluster") + require.Error(t, err) + assert.Contains(t, string(stderr), "selected-profile") + assert.NotContains(t, string(stderr), "fixture-secret-value") + assert.Contains(t, err.Error(), "selected-profile") + assert.NotContains(t, err.Error(), "fixture-secret-value") +} + +// TestExec_RedactsOverlappingCredentialValuesLongestFirst verifies longer secrets are removed before their prefixes. func TestExec_RedactsOverlappingCredentialValuesLongestFirst(t *testing.T) { t.Parallel() @@ -197,7 +233,7 @@ func TestExec_RedactsOverlappingCredentialValuesLongestFirst(t *testing.T) { client := eksctl.NewClient( eksctl.WithRunner(runner), eksctl.WithEnvironment([]string{ - "AWS_PROFILE=fixture-secret", + "AWS_ACCESS_KEY_ID=fixture-secret", "AWS_SECRET_ACCESS_KEY=fixture-secret-long", }), ) @@ -208,6 +244,8 @@ func TestExec_RedactsOverlappingCredentialValuesLongestFirst(t *testing.T) { assert.NotContains(t, err.Error(), "-long") } +// TestRequireCredentialValuesRejectsMissingAndPartialSelections verifies every +// unusable explicit selection fails before execution. func TestRequireCredentialValuesRejectsMissingAndPartialSelections(t *testing.T) { t.Parallel() @@ -251,6 +289,7 @@ func TestRequireCredentialValuesRejectsMissingAndPartialSelections(t *testing.T) } } +// TestRequireCredentialValuesAcceptsProfileOrStaticPair verifies either supported complete credential form can execute. func TestRequireCredentialValuesAcceptsProfileOrStaticPair(t *testing.T) { t.Parallel() diff --git a/pkg/svc/clusterdiscovery/aws_environment_test.go b/pkg/svc/clusterdiscovery/aws_environment_test.go index f726784e84..81bbfeb322 100644 --- a/pkg/svc/clusterdiscovery/aws_environment_test.go +++ b/pkg/svc/clusterdiscovery/aws_environment_test.go @@ -17,8 +17,10 @@ type namedAWSResolver struct { names map[credentials.Key]string } +// Value returns the fixture value associated with a credential key. func (r namedAWSResolver) Value(key credentials.Key) string { return r.values[key] } +// EnvVar returns the fixture's configured source name or the credential's canonical default. func (r namedAWSResolver) EnvVar(key credentials.Key) string { if name := r.names[key]; name != "" { return name @@ -27,6 +29,8 @@ func (r namedAWSResolver) EnvVar(key credentials.Key) string { return credentials.DefaultEnvVar(key) } +// TestDiscoverAWS_UsesCanonicalIsolatedChildEnvironment verifies discovery +// canonicalizes aliases without leaking ambient credentials. func TestDiscoverAWS_UsesCanonicalIsolatedChildEnvironment(t *testing.T) { // Not parallel: the real ExecRunner resolves the fixture from PATH. binDir := t.TempDir() @@ -82,6 +86,7 @@ printf '[{"Name":"mapped-eks","Region":"eu-west-1","EksctlCreated":"True"}]\n' assert.Equal(t, "parent-custom-profile", os.Getenv("KSAIL_PROFILE")) } +// writeExecutableFixture writes a private executable used to stand in for eksctl. func writeExecutableFixture(t *testing.T, path, contents string) { t.Helper() diff --git a/pkg/svc/clusterdiscovery/cloud.go b/pkg/svc/clusterdiscovery/cloud.go index 8690c703bd..86217044d5 100644 --- a/pkg/svc/clusterdiscovery/cloud.go +++ b/pkg/svc/clusterdiscovery/cloud.go @@ -85,12 +85,13 @@ func (d *Discoverer) listAWS(ctx context.Context) ([]Cluster, error) { return nil, nil } - auth := credentials.ResolveAWS(d.resolver()) - eksctlOptions := credentials.OptionsForAWSChildEnvironment( - auth, os.Environ(), eksctlclient.WithEnvironment, eksctlclient.RequireCredentialValues, - ) - providerOptions := credentials.OptionsForAWSResolution( - auth, awsprovider.WithCredentialValues, awsprovider.RequireCredentialValues, + _, eksctlOptions, providerOptions := credentials.ResolveAWSClientOptions( + d.resolver(), + os.Environ(), + eksctlclient.WithEnvironment, + eksctlclient.RequireCredentialValues, + awsprovider.WithCredentialValues, + awsprovider.RequireCredentialValues, ) client := eksctlclient.NewClient(eksctlOptions...) diff --git a/pkg/svc/credentials/aws_environment_test.go b/pkg/svc/credentials/aws_environment_test.go index cec80a037c..3fd10a65dc 100644 --- a/pkg/svc/credentials/aws_environment_test.go +++ b/pkg/svc/credentials/aws_environment_test.go @@ -19,6 +19,7 @@ type awsResolverFixture struct { values map[credentials.Key]string } +// EnvVar returns the fixture's configured source name or the credential's canonical default. func (f awsResolverFixture) EnvVar(key credentials.Key) string { if name := f.envVars[key]; name != "" { return name @@ -27,8 +28,11 @@ func (f awsResolverFixture) EnvVar(key credentials.Key) string { return credentials.DefaultEnvVar(key) } +// Value returns the fixture value associated with a credential key. func (f awsResolverFixture) Value(key credentials.Key) string { return f.values[key] } +// TestNewAWSOptionsResolver_UsesConfiguredNamesWithoutMutatingParent verifies +// immutable alias lookup leaves process state untouched. func TestNewAWSOptionsResolver_UsesConfiguredNamesWithoutMutatingParent(t *testing.T) { // Not parallel: t.Setenv changes the process environment. t.Setenv("KSAIL_PROFILE", "selected-profile") @@ -53,6 +57,8 @@ func TestNewAWSOptionsResolver_UsesConfiguredNamesWithoutMutatingParent(t *testi assert.Equal(t, "stale-profile", os.Getenv("AWS_PROFILE")) } +// TestAWSResolution_ChildEnvironmentCanonicalizesAndIsolatesCredentials +// verifies aliases become canonical values in an isolated child environment. func TestAWSResolution_ChildEnvironmentCanonicalizesAndIsolatesCredentials(t *testing.T) { t.Parallel() @@ -105,6 +111,8 @@ func TestAWSResolution_ChildEnvironmentCanonicalizesAndIsolatesCredentials(t *te assertEnvKeyCount(t, child, "KSAIL_SESSION", 0) } +// TestAWSResolution_ChildEnvironmentRemovesStaleOptionalValues verifies omitted +// optional credentials cannot survive from ambient state. func TestAWSResolution_ChildEnvironmentRemovesStaleOptionalValues(t *testing.T) { t.Parallel() @@ -129,6 +137,8 @@ func TestAWSResolution_ChildEnvironmentRemovesStaleOptionalValues(t *testing.T) assertEnvKeyCount(t, child, "KSAIL_SESSION", 0) } +// TestAWSResolution_CustomSourcesRemoveCompetingAmbientCredentialProviders +// verifies aliases suppress competing environment identity providers. func TestAWSResolution_CustomSourcesRemoveCompetingAmbientCredentialProviders(t *testing.T) { t.Parallel() @@ -176,6 +186,8 @@ func TestAWSResolution_CustomSourcesRemoveCompetingAmbientCredentialProviders(t assertEnvEntry(t, child, "AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE", "selected-auth-token") } +// TestAWSResolution_ReportsCustomCredentialSourcesEvenWhenUnset verifies +// configured aliases remain fail-closed when their values are absent. func TestAWSResolution_ReportsCustomCredentialSourcesEvenWhenUnset(t *testing.T) { t.Parallel() @@ -189,6 +201,8 @@ func TestAWSResolution_ReportsCustomCredentialSourcesEvenWhenUnset(t *testing.T) assert.True(t, resolution.HasCustomCredentialSources()) } +// TestOptionsForAWSResolutionMapsValuesAndCustomRequirement verifies resolved +// values and fail-closed intent reach credential consumers together. func TestOptionsForAWSResolutionMapsValuesAndCustomRequirement(t *testing.T) { t.Parallel() @@ -236,6 +250,8 @@ func TestOptionsForAWSResolutionMapsValuesAndCustomRequirement(t *testing.T) { assert.True(t, options[1].required) } +// TestOptionsForAWSResolutionPreservesDefaultCredentialChain verifies canonical +// defaults do not impose an explicit credential requirement. func TestOptionsForAWSResolutionPreservesDefaultCredentialChain(t *testing.T) { t.Parallel() @@ -249,6 +265,8 @@ func TestOptionsForAWSResolutionPreservesDefaultCredentialChain(t *testing.T) { assert.Equal(t, []string{"values"}, options) } +// TestOptionsForAWSChildEnvironmentCanonicalizesAndRequiresCustomSources +// verifies child-process isolation and its requirement stay paired. func TestOptionsForAWSChildEnvironmentCanonicalizesAndRequiresCustomSources(t *testing.T) { t.Parallel() @@ -273,6 +291,53 @@ func TestOptionsForAWSChildEnvironmentCanonicalizesAndRequiresCustomSources(t *t assert.Equal(t, []string{"required"}, options[1]) } +// TestResolveAWSClientOptionsBuildsBothCredentialBoundaries verifies one +// snapshot configures child-process and SDK consumers consistently. +func TestResolveAWSClientOptionsBuildsBothCredentialBoundaries(t *testing.T) { + t.Parallel() + + type environmentOption struct { + environment []string + required bool + } + + type credentialOption struct { + profile string + required bool + } + + resolution, environmentOptions, credentialOptions := credentials.ResolveAWSClientOptions( + awsResolverFixture{ + envVars: map[credentials.Key]string{ + credentials.AWSProfile: "KSAIL_PROFILE", + }, + values: map[credentials.Key]string{ + credentials.AWSProfile: "selected-profile", + }, + }, + []string{"PATH=/usr/bin", "AWS_PROFILE=stale-profile"}, + func(environment []string) environmentOption { + return environmentOption{environment: environment} + }, + func() environmentOption { return environmentOption{required: true} }, + func(profile, _, _, _ string) credentialOption { + return credentialOption{profile: profile} + }, + func() credentialOption { return credentialOption{required: true} }, + ) + + assert.Equal(t, "selected-profile", resolution.Profile) + require.Len(t, environmentOptions, 2) + assertEnvEntry(t, environmentOptions[0].environment, "PATH", "usr/bin") + assertEnvEntry(t, environmentOptions[0].environment, "AWS_PROFILE", "selected-profile") + assert.True(t, environmentOptions[1].required) + require.Len(t, credentialOptions, 2) + assert.Equal(t, "selected-profile", credentialOptions[0].profile) + assert.True(t, credentialOptions[1].required) +} + +// TestAWSResolution_ChildEnvironmentPreservesCanonicalDefaults verifies +// canonical selection retains unrelated default-chain inputs. func TestAWSResolution_ChildEnvironmentPreservesCanonicalDefaults(t *testing.T) { t.Parallel() @@ -300,6 +365,8 @@ func TestAWSResolution_ChildEnvironmentPreservesCanonicalDefaults(t *testing.T) assertEnvEntry(t, child, "AWS_ROLE_ARN", "role/default") } +// TestAWSResolution_ChildEnvironmentIsSafeForConcurrentInvocations verifies +// immutable resolution safely produces independent environments. func TestAWSResolution_ChildEnvironmentIsSafeForConcurrentInvocations(t *testing.T) { t.Parallel() @@ -326,6 +393,7 @@ func TestAWSResolution_ChildEnvironmentIsSafeForConcurrentInvocations(t *testing require.Empty(t, errCh) } +// assertEnvEntry verifies exactly one environment entry carries the expected value fragment. func assertEnvEntry(t *testing.T, environment []string, key, valueFragment string) { t.Helper() @@ -341,12 +409,14 @@ func assertEnvEntry(t *testing.T, environment []string, key, valueFragment strin assert.Fail(t, "expected environment key is missing", key) } +// assertEnvKeyCount verifies an environment name occurs the expected number of times. func assertEnvKeyCount(t *testing.T, environment []string, key string, expected int) { t.Helper() assert.Equal(t, expected, envKeyCount(environment, key), "unexpected count for %s", key) } +// envKeyCount counts exact environment names independently of their values. func envKeyCount(environment []string, key string) int { count := 0 diff --git a/pkg/svc/credentials/credentials.go b/pkg/svc/credentials/credentials.go index 6411ad1f2c..258c2fac41 100644 --- a/pkg/svc/credentials/credentials.go +++ b/pkg/svc/credentials/credentials.go @@ -201,25 +201,23 @@ func ResolveAWS(resolver Resolver) AWSResolution { AccessKeyID: resolver.Value(AWSAccessKeyID), SecretAccessKey: resolver.Value(AWSSecretAccessKey), SessionToken: resolver.Value(AWSSessionToken), - sourceEnvVars: [4]string{ - resolver.EnvVar(AWSProfile), - resolver.EnvVar(AWSAccessKeyID), - resolver.EnvVar(AWSSecretAccessKey), - resolver.EnvVar(AWSSessionToken), - }, } - canonicalNames := [...]string{ - defaultAWSProfileEnvVar, - defaultAWSAccessKeyIDEnvVar, - defaultAWSSecretAccessEnvVar, - defaultAWSSessionTokenEnvVar, + credentialSources := [...]struct { + key Key + canonicalName string + }{ + {key: AWSProfile, canonicalName: defaultAWSProfileEnvVar}, + {key: AWSAccessKeyID, canonicalName: defaultAWSAccessKeyIDEnvVar}, + {key: AWSSecretAccessKey, canonicalName: defaultAWSSecretAccessEnvVar}, + {key: AWSSessionToken, canonicalName: defaultAWSSessionTokenEnvVar}, } - for index, sourceName := range resolution.sourceEnvVars { - if sourceName != "" && sourceName != canonicalNames[index] { - resolution.hasCustomCredentialEnv = true + for index, source := range credentialSources { + sourceName := resolver.EnvVar(source.key) + resolution.sourceEnvVars[index] = sourceName - break + if sourceName != "" && sourceName != source.canonicalName { + resolution.hasCustomCredentialEnv = true } } @@ -272,6 +270,37 @@ func OptionsForAWSChildEnvironment[T any]( ) } +// ResolveAWSClientOptions snapshots one AWS identity and builds the paired +// option sets that isolate an environment-based client and pin an SDK-backed +// client to that same identity. Both option sets inherit the same fail-closed +// custom-source requirement, preventing call sites from wiring only half of +// the credential boundary. The returned resolution can feed additional AWS +// consumers without re-reading mutable process state. +func ResolveAWSClientOptions[EnvironmentOption, CredentialOption any]( + resolver Resolver, + parentEnvironment []string, + withEnvironment func(environment []string) EnvironmentOption, + requireEnvironmentCredentialValues func() EnvironmentOption, + withCredentialValues func(profile, accessKeyID, secretAccessKey, sessionToken string) CredentialOption, + requireCredentialValues func() CredentialOption, +) (AWSResolution, []EnvironmentOption, []CredentialOption) { + resolution := ResolveAWS(resolver) + + return resolution, + OptionsForAWSChildEnvironment( + resolution, + parentEnvironment, + withEnvironment, + requireEnvironmentCredentialValues, + ), + OptionsForAWSResolution( + resolution, + withCredentialValues, + requireCredentialValues, + ) +} + +// optionsWithCredentialRequirement appends the fail-closed option only when custom credential sources require it. func optionsWithCredentialRequirement[T any]( option T, required bool, @@ -313,6 +342,8 @@ func (r AWSResolution) ChildEnvironment(parent []string) []string { return child } +// strippedEnvironmentNames returns normalized aliases and competing provider +// names that must not reach the child process. func (r AWSResolution) strippedEnvironmentNames(caseInsensitive bool) map[string]struct{} { strippedNames := make(map[string]struct{}) for _, name := range []string{ @@ -346,6 +377,7 @@ func (r AWSResolution) strippedEnvironmentNames(caseInsensitive bool) map[string return strippedNames } +// filterEnvironment copies parent entries whose normalized names are not marked for removal. func filterEnvironment( parent []string, strippedNames map[string]struct{}, @@ -366,6 +398,7 @@ func filterEnvironment( return child } +// normalizeEnvironmentName folds case only when the target platform treats environment names case-insensitively. func normalizeEnvironmentName(name string, caseInsensitive bool) string { if caseInsensitive { return strings.ToUpper(name) diff --git a/pkg/svc/credentials/environment_name_test.go b/pkg/svc/credentials/environment_name_test.go index c8f0be0c3b..378ce90f90 100644 --- a/pkg/svc/credentials/environment_name_test.go +++ b/pkg/svc/credentials/environment_name_test.go @@ -6,6 +6,8 @@ import ( "github.com/stretchr/testify/assert" ) +// TestNormalizeEnvironmentNameHonorsPlatformCaseSensitivity verifies filtering +// follows the host's environment-name semantics. func TestNormalizeEnvironmentNameHonorsPlatformCaseSensitivity(t *testing.T) { t.Parallel() diff --git a/pkg/svc/provisioner/cluster/eks/connector_test.go b/pkg/svc/provisioner/cluster/eks/connector_test.go index c28f17e2cb..d41dc0f0d6 100644 --- a/pkg/svc/provisioner/cluster/eks/connector_test.go +++ b/pkg/svc/provisioner/cluster/eks/connector_test.go @@ -174,6 +174,7 @@ func TestKubeconfigRequiresClusterName(t *testing.T) { require.ErrorIs(t, err, eksprovisioner.ErrClusterNameRequired) } +// TestKubeconfigFailsClosedOnIncompleteMappedCredentials verifies connector setup rejects partial aliased static credentials. func TestKubeconfigFailsClosedOnIncompleteMappedCredentials(t *testing.T) { t.Parallel() diff --git a/pkg/svc/provisioner/cluster/factory_eks.go b/pkg/svc/provisioner/cluster/factory_eks.go index 887645b94b..6478d7bd9a 100644 --- a/pkg/svc/provisioner/cluster/factory_eks.go +++ b/pkg/svc/provisioner/cluster/factory_eks.go @@ -53,15 +53,18 @@ func (f DefaultFactory) createEKSProvisioner( return provisioner, eksConfig, nil } +// resolveEKSCredentialOptions snapshots one AWS resolution and derives aligned +// eksctl, provider, and provisioner options. func resolveEKSCredentialOptions( awsOptions v1alpha1.OptionsAWS, ) (*eksctlclient.Client, []awsprovider.Option, []eksprovisioner.Option) { - auth := credentials.ResolveAWS(credentials.NewAWSOptionsResolver(awsOptions)) - eksctlOptions := credentials.OptionsForAWSChildEnvironment( - auth, os.Environ(), eksctlclient.WithEnvironment, eksctlclient.RequireCredentialValues, - ) - providerOptions := credentials.OptionsForAWSResolution( - auth, awsprovider.WithCredentialValues, awsprovider.RequireCredentialValues, + auth, eksctlOptions, providerOptions := credentials.ResolveAWSClientOptions( + credentials.NewAWSOptionsResolver(awsOptions), + os.Environ(), + eksctlclient.WithEnvironment, + eksctlclient.RequireCredentialValues, + awsprovider.WithCredentialValues, + awsprovider.RequireCredentialValues, ) provisionerOptions := credentials.OptionsForAWSResolution( auth, eksprovisioner.WithCredentialValues, eksprovisioner.RequireCredentialValues, diff --git a/pkg/svc/provisioner/cluster/factory_eks_test.go b/pkg/svc/provisioner/cluster/factory_eks_test.go index abeb971b38..4ba6aa8956 100644 --- a/pkg/svc/provisioner/cluster/factory_eks_test.go +++ b/pkg/svc/provisioner/cluster/factory_eks_test.go @@ -87,6 +87,7 @@ printf '%s\n' "$@" > "$KSAIL_EKSCTL_ARGS" }, strings.Fields(string(args))) } +// writeExecutableFixture writes a private executable used to stand in for eksctl. func writeExecutableFixture(t *testing.T, path, contents string) { t.Helper() From 8e93831f68042d341fbb9a41b17a0581c2fd8411 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Mon, 13 Jul 2026 15:36:44 +0200 Subject: [PATCH 4/4] fix(ci): wrap credential test documentation --- pkg/client/eks/client_test.go | 15 ++++++++++----- pkg/svc/provisioner/cluster/eks/connector_test.go | 3 ++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/client/eks/client_test.go b/pkg/client/eks/client_test.go index 2ee26722d1..c9814afec0 100644 --- a/pkg/client/eks/client_test.go +++ b/pkg/client/eks/client_test.go @@ -178,7 +178,8 @@ func TestMintTokenSignsClusterBindingHeaders(t *testing.T) { assert.Contains(t, query.Get("X-Amz-SignedHeaders"), "x-k8s-aws-id") } -// TestWithCredentialValues_StaticCredentialsOverrideAmbientIdentity verifies explicit static credentials win over ambient identity. +// TestWithCredentialValues_StaticCredentialsOverrideAmbientIdentity verifies +// explicit static credentials win over ambient identity. func TestWithCredentialValues_StaticCredentialsOverrideAmbientIdentity(t *testing.T) { // Not parallel: t.Setenv changes the process environment. t.Setenv("AWS_ACCESS_KEY_ID", "STALEAMBIENT") @@ -202,7 +203,8 @@ func TestWithCredentialValues_StaticCredentialsOverrideAmbientIdentity(t *testin assertTokenCredentialPrefix(t, token, "SELECTEDACCESS/") } -// TestWithCredentialValues_ProfileOverridesAmbientStaticCredentials verifies an explicit profile wins over ambient static credentials. +// TestWithCredentialValues_ProfileOverridesAmbientStaticCredentials verifies an +// explicit profile wins over ambient static credentials. func TestWithCredentialValues_ProfileOverridesAmbientStaticCredentials(t *testing.T) { // Not parallel: t.Setenv changes the process environment. credentialsFile := filepath.Join(t.TempDir(), "credentials") @@ -245,7 +247,8 @@ func TestWithCredentialValues_RejectsPartialStaticCredentials(t *testing.T) { require.ErrorIs(t, err, eksclient.ErrIncompleteStaticCredentials) } -// TestRequireCredentialValuesRejectsAmbientFallback verifies required explicit credentials cannot fall back to ambient identity. +// TestRequireCredentialValuesRejectsAmbientFallback verifies required explicit +// credentials cannot fall back to ambient identity. func TestRequireCredentialValuesRejectsAmbientFallback(t *testing.T) { t.Parallel() @@ -260,7 +263,8 @@ func TestRequireCredentialValuesRejectsAmbientFallback(t *testing.T) { require.ErrorIs(t, err, eksclient.ErrExplicitCredentialsUnavailable) } -// TestNewClientWithCredentialRequirementRejectsAmbientFallbackWhenRequired verifies required construction rejects empty resolved credentials. +// TestNewClientWithCredentialRequirementRejectsAmbientFallbackWhenRequired +// verifies required construction rejects empty resolved credentials. func TestNewClientWithCredentialRequirementRejectsAmbientFallbackWhenRequired(t *testing.T) { t.Parallel() @@ -276,7 +280,8 @@ func TestNewClientWithCredentialRequirementRejectsAmbientFallbackWhenRequired(t require.Len(t, base, 3, "the caller-owned option slice must remain unchanged") } -// TestNewClientWithCredentialRequirementPreservesDefaultChainWhenOptional verifies optional construction retains the AWS default chain. +// TestNewClientWithCredentialRequirementPreservesDefaultChainWhenOptional +// verifies optional construction retains the AWS default chain. func TestNewClientWithCredentialRequirementPreservesDefaultChainWhenOptional(t *testing.T) { t.Parallel() diff --git a/pkg/svc/provisioner/cluster/eks/connector_test.go b/pkg/svc/provisioner/cluster/eks/connector_test.go index d41dc0f0d6..d35feb8779 100644 --- a/pkg/svc/provisioner/cluster/eks/connector_test.go +++ b/pkg/svc/provisioner/cluster/eks/connector_test.go @@ -174,7 +174,8 @@ func TestKubeconfigRequiresClusterName(t *testing.T) { require.ErrorIs(t, err, eksprovisioner.ErrClusterNameRequired) } -// TestKubeconfigFailsClosedOnIncompleteMappedCredentials verifies connector setup rejects partial aliased static credentials. +// TestKubeconfigFailsClosedOnIncompleteMappedCredentials verifies connector +// setup rejects partial aliased static credentials. func TestKubeconfigFailsClosedOnIncompleteMappedCredentials(t *testing.T) { t.Parallel()