diff --git a/spinifex/handlers/eks/cluster_meta_to_aws_test.go b/spinifex/handlers/eks/cluster_meta_to_aws_test.go index d8e8a487e..0760f1029 100644 --- a/spinifex/handlers/eks/cluster_meta_to_aws_test.go +++ b/spinifex/handlers/eks/cluster_meta_to_aws_test.go @@ -85,11 +85,13 @@ func TestClusterMetaToAWS_AccessConfig(t *testing.T) { name string mutate func(*ClusterMeta) wantBootstrap bool + wantMode string }{ { - name: "zero-value record predating the field defaults bootstrap-admin true", + name: "zero-value record predating the field defaults bootstrap-admin true and mode API", mutate: func(m *ClusterMeta) {}, wantBootstrap: true, + wantMode: eks.AuthenticationModeApi, }, { name: "caller explicitly requested bootstrap-admin true", @@ -98,6 +100,7 @@ func TestClusterMetaToAWS_AccessConfig(t *testing.T) { m.BootstrapClusterCreatorAdminPermissions = &v }, wantBootstrap: true, + wantMode: eks.AuthenticationModeApi, }, { name: "caller explicitly disabled bootstrap-admin", @@ -106,6 +109,15 @@ func TestClusterMetaToAWS_AccessConfig(t *testing.T) { m.BootstrapClusterCreatorAdminPermissions = &v }, wantBootstrap: false, + wantMode: eks.AuthenticationModeApi, + }, + { + name: "cluster created with API_AND_CONFIG_MAP describes back the same mode", + mutate: func(m *ClusterMeta) { + m.AuthenticationMode = eks.AuthenticationModeApiAndConfigMap + }, + wantBootstrap: true, + wantMode: eks.AuthenticationModeApiAndConfigMap, }, } for _, tt := range tests { @@ -116,7 +128,7 @@ func TestClusterMetaToAWS_AccessConfig(t *testing.T) { out := clusterMetaToAWS(meta) require.NotNil(t, out.AccessConfig) - assert.Equal(t, eks.AuthenticationModeApi, aws.StringValue(out.AccessConfig.AuthenticationMode)) + assert.Equal(t, tt.wantMode, aws.StringValue(out.AccessConfig.AuthenticationMode)) assert.Equal(t, tt.wantBootstrap, aws.BoolValue(out.AccessConfig.BootstrapClusterCreatorAdminPermissions)) }) } diff --git a/spinifex/handlers/eks/cluster_state.go b/spinifex/handlers/eks/cluster_state.go index 1c09d089e..bbe04514a 100644 --- a/spinifex/handlers/eks/cluster_state.go +++ b/spinifex/handlers/eks/cluster_state.go @@ -94,6 +94,10 @@ type ClusterMeta struct { // resolved default-true decision, so DescribeCluster echoes exactly what // CreateCluster actually did. Nil predates this field and also defaults true. BootstrapClusterCreatorAdminPermissions *bool `json:"bootstrapClusterCreatorAdminPermissions,omitempty"` + // AuthenticationMode is the requested accessConfig.authenticationMode + // ("API" or "API_AND_CONFIG_MAP"), stored verbatim; empty predates this + // field and also defaults to API, the only grant path Spinifex ever runs. + AuthenticationMode string `json:"authenticationMode,omitempty"` // UpgradePolicy is the requested extended-support policy, stored verbatim; // nil when the caller specified none (DescribeCluster then reports the // AWS-standard default). diff --git a/spinifex/handlers/eks/create_cluster_test.go b/spinifex/handlers/eks/create_cluster_test.go index 276f32de7..3927b1589 100644 --- a/spinifex/handlers/eks/create_cluster_test.go +++ b/spinifex/handlers/eks/create_cluster_test.go @@ -498,3 +498,41 @@ func TestCreateCluster_DescribeSurfacesReadbackConformanceFields(t *testing.T) { assert.Equal(t, []string{eks.LogTypeApi}, aws.StringValueSlice(c.Logging.ClusterLogging[0].Types)) assert.True(t, aws.BoolValue(c.Logging.ClusterLogging[0].Enabled)) } + +// A cluster created with accessConfig.authenticationMode = API_AND_CONFIG_MAP +// must describe back with that same mode, not the platform's hardcoded API — +// otherwise every subsequent terraform-aws-eks plan would show a permanent +// diff on access_config.authentication_mode. +func TestCreateCluster_EchoesRequestedAuthenticationMode(t *testing.T) { + f := newEKSServiceFixture(t) + + in := createInput("alpha") + in.AccessConfig = &eks.CreateAccessConfigRequest{ + AuthenticationMode: aws.String(eks.AuthenticationModeApiAndConfigMap), + } + + _, err := f.svc.CreateCluster(context.Background(), in, testAccountID, "") + require.NoError(t, err) + f.svc.WaitLaunches() + + out, err := f.svc.DescribeCluster(context.Background(), &eks.DescribeClusterInput{Name: aws.String("alpha")}, testAccountID) + require.NoError(t, err) + require.NotNil(t, out.Cluster.AccessConfig) + assert.Equal(t, eks.AuthenticationModeApiAndConfigMap, aws.StringValue(out.Cluster.AccessConfig.AuthenticationMode)) +} + +// A cluster created with no accessConfig at all must still describe back as +// API — the AWS default, and the only mode a record predating this field can +// mean. +func TestCreateCluster_UnsetAuthenticationModeDescribesAsAPI(t *testing.T) { + f := newEKSServiceFixture(t) + + _, err := f.svc.CreateCluster(context.Background(), createInput("alpha"), testAccountID, "") + require.NoError(t, err) + f.svc.WaitLaunches() + + out, err := f.svc.DescribeCluster(context.Background(), &eks.DescribeClusterInput{Name: aws.String("alpha")}, testAccountID) + require.NoError(t, err) + require.NotNil(t, out.Cluster.AccessConfig) + assert.Equal(t, eks.AuthenticationModeApi, aws.StringValue(out.Cluster.AccessConfig.AuthenticationMode)) +} diff --git a/spinifex/handlers/eks/nodegroup.go b/spinifex/handlers/eks/nodegroup.go index 74cfa5890..9b0033f34 100644 --- a/spinifex/handlers/eks/nodegroup.go +++ b/spinifex/handlers/eks/nodegroup.go @@ -1385,12 +1385,16 @@ func nodegroupRecordToAWS(rec *NodegroupRecord) *eks.Nodegroup { } out.UpdateConfig = cfg } + // AWS always returns Health with an issues list, empty when nothing is + // wrong; a nil slice here would marshal the key away entirely. + issues := make([]*eks.Issue, 0, 1) if rec.StatusReason != "" { - out.Health = &eks.NodegroupHealth{Issues: []*eks.Issue{{ + issues = append(issues, &eks.Issue{ Code: aws.String(eks.NodegroupIssueCodeNodeCreationFailure), Message: aws.String(rec.StatusReason), ResourceIds: aws.StringSlice(rec.InstanceIDs), - }}} + }) } + out.Health = &eks.NodegroupHealth{Issues: issues} return out } diff --git a/spinifex/handlers/eks/nodegroup_test.go b/spinifex/handlers/eks/nodegroup_test.go index c46a7d8fe..37cd25979 100644 --- a/spinifex/handlers/eks/nodegroup_test.go +++ b/spinifex/handlers/eks/nodegroup_test.go @@ -2,6 +2,7 @@ package handlers_eks import ( "context" + "encoding/json" "errors" "fmt" "slices" @@ -11,6 +12,7 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/private/protocol/json/jsonutil" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/eks" "github.com/mulgadc/spinifex/spinifex/arn" @@ -1103,6 +1105,50 @@ func TestNodegroupRecordToAWS_LabelsAlwaysPresent(t *testing.T) { assert.Equal(t, map[string]*string{"team": aws.String("core")}, withLabels.Labels) } +// TestNodegroupRecordToAWS_HealthAlwaysPresent pins Health as a non-nil object +// with a zero-length issues list when healthy, and the NodeCreationFailure +// issue when set, via jsonutil.BuildJSON so a dropped nil-slice key is caught. +func TestNodegroupRecordToAWS_HealthAlwaysPresent(t *testing.T) { + healthy := nodegroupRecordToAWS(&NodegroupRecord{ + ClusterName: "c1", Name: "ng1", Status: eks.NodegroupStatusActive, + }) + require.NotNil(t, healthy) + require.NotNil(t, healthy.Health) + assert.NotNil(t, healthy.Health.Issues) + assert.Empty(t, healthy.Health.Issues) + + healthyJSON, err := jsonutil.BuildJSON(healthy) + require.NoError(t, err) + var healthyDecoded map[string]any + require.NoError(t, json.Unmarshal(healthyJSON, &healthyDecoded)) + health, ok := healthyDecoded["health"].(map[string]any) + require.True(t, ok, "health key must survive marshalling for a healthy node group") + issues, ok := health["issues"].([]any) + require.True(t, ok, "issues key must survive marshalling as an empty list, not be dropped") + assert.Empty(t, issues) + + unhealthy := nodegroupRecordToAWS(&NodegroupRecord{ + ClusterName: "c1", Name: "ng1", Status: eks.NodegroupStatusCreateFailed, + StatusReason: "capacity unavailable", InstanceIDs: []string{"i-aaa"}, + }) + require.NotNil(t, unhealthy) + require.NotNil(t, unhealthy.Health) + require.Len(t, unhealthy.Health.Issues, 1) + assert.Equal(t, eks.NodegroupIssueCodeNodeCreationFailure, aws.StringValue(unhealthy.Health.Issues[0].Code)) + assert.Equal(t, "capacity unavailable", aws.StringValue(unhealthy.Health.Issues[0].Message)) + assert.Equal(t, []string{"i-aaa"}, aws.StringValueSlice(unhealthy.Health.Issues[0].ResourceIds)) + + unhealthyJSON, err := jsonutil.BuildJSON(unhealthy) + require.NoError(t, err) + var unhealthyDecoded map[string]any + require.NoError(t, json.Unmarshal(unhealthyJSON, &unhealthyDecoded)) + uHealth, ok := unhealthyDecoded["health"].(map[string]any) + require.True(t, ok) + uIssues, ok := uHealth["issues"].([]any) + require.True(t, ok) + require.Len(t, uIssues, 1) +} + // TestCreateNodegroup_CapturesLaunchTemplateCapacityTypeReleaseVersion proves // the create path stores exactly what the caller asked for, not a value // resolved at describe time — launchTemplate is force-new, so a mismatch here diff --git a/spinifex/handlers/eks/service_impl.go b/spinifex/handlers/eks/service_impl.go index d9f14f72c..a4a154d45 100644 --- a/spinifex/handlers/eks/service_impl.go +++ b/spinifex/handlers/eks/service_impl.go @@ -597,6 +597,7 @@ func (s *EKSServiceImpl) CreateCluster(ctx context.Context, input *eks.CreateClu }, KubernetesNetworkConfig: clusterNetworkConfigFromInput(input.KubernetesNetworkConfig), BootstrapClusterCreatorAdminPermissions: &bootstrapAdmin, + AuthenticationMode: requestedAuthenticationMode(input), UpgradePolicy: clusterUpgradePolicyFromInput(input.UpgradePolicy), Logging: clusterLoggingFromInput(input.Logging), Tags: aws.StringValueMap(input.Tags), @@ -1021,6 +1022,16 @@ func bootstrapCreatorAdmin(input *eks.CreateClusterInput) bool { return *input.AccessConfig.BootstrapClusterCreatorAdminPermissions } +// requestedAuthenticationMode resolves accessConfig.authenticationMode from an +// already-validated CreateCluster request, defaulting to API when omitted. +// validateCreateClusterInput has rejected anything but "", API, and API_AND_CONFIG_MAP. +func requestedAuthenticationMode(input *eks.CreateClusterInput) string { + if input.AccessConfig == nil { + return eks.AuthenticationModeApi + } + return deref(input.AccessConfig.AuthenticationMode, eks.AuthenticationModeApi) +} + // systemEgressEvent is the wire shape for vpc.add-system-egress / // vpc.delete-system-egress (mirrors network/subscribers.SystemEgressEvent; // kept local to avoid importing the network layer from a handler). @@ -2062,8 +2073,15 @@ func validateCreateClusterInput(input *eks.CreateClusterInput) error { } if input.AccessConfig != nil && input.AccessConfig.AuthenticationMode != nil { mode := *input.AccessConfig.AuthenticationMode - if mode != "" && mode != eks.AuthenticationModeApi { - return errors.New(awserrors.ErrorInvalidParameter) + switch mode { + case "", eks.AuthenticationModeApi, eks.AuthenticationModeApiAndConfigMap: + // Spinifex has no aws-auth ConfigMap path, but that side of + // API_AND_CONFIG_MAP grants nothing here, so the cluster still runs + // as API. Bare CONFIG_MAP falls to the default: no working auth path. + default: + return awserrors.Errorf(awserrors.ErrorInvalidParameter, + "accessConfig.authenticationMode: %q is not a supported value, expected one of %q, %q", + mode, eks.AuthenticationModeApi, eks.AuthenticationModeApiAndConfigMap) } } // AWS rejects disabling both public and private endpoint access — the @@ -2411,10 +2429,14 @@ func clusterMetaToAWS(meta *ClusterMeta) *eks.Cluster { if meta.BootstrapClusterCreatorAdminPermissions != nil { bootstrapAdmin = *meta.BootstrapClusterCreatorAdminPermissions } + // Empty covers a record persisted before this field, or an unset request; + // both mean the AWS-default API mode. + authMode := meta.AuthenticationMode + if authMode == "" { + authMode = eks.AuthenticationModeApi + } out.AccessConfig = &eks.AccessConfigResponse{ - // Spinifex has no aws-auth ConfigMap path; API access-entry auth is the - // only mode it ever actually runs, regardless of what was requested. - AuthenticationMode: aws.String(eks.AuthenticationModeApi), + AuthenticationMode: aws.String(authMode), BootstrapClusterCreatorAdminPermissions: aws.Bool(bootstrapAdmin), } out.PlatformVersion = aws.String(eksPlatformVersion) diff --git a/spinifex/handlers/eks/service_impl_test.go b/spinifex/handlers/eks/service_impl_test.go index 862544027..d9545ddec 100644 --- a/spinifex/handlers/eks/service_impl_test.go +++ b/spinifex/handlers/eks/service_impl_test.go @@ -81,26 +81,45 @@ func TestMissingOrchestrationDeps_IAMProviderSatisfiesGate(t *testing.T) { require.Empty(t, f.svc.missingOrchestrationDeps()) } -// EKS only supports the API authentication mode here; CONFIG_MAP (and the -// API_AND_CONFIG_MAP hybrid) must be rejected with InvalidParameterException. +// Bare CONFIG_MAP consults only the aws-auth ConfigMap, which Spinifex has no +// path for, so a cluster in that mode would have no working authentication at +// all. The refusal must name accessConfig.authenticationMode so a Terraform +// caller sees why, not a bare code. func TestValidateCreateClusterInput_RejectsConfigMapAuthMode(t *testing.T) { in := createInput("alpha") in.AccessConfig = &eks.CreateAccessConfigRequest{ AuthenticationMode: aws.String(eks.AuthenticationModeConfigMap), } err := validateCreateClusterInput(in) - require.EqualError(t, err, awserrors.ErrorInvalidParameter) + code, message, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok) + assert.Equal(t, awserrors.ErrorInvalidParameter, code) + assert.Contains(t, message, "accessConfig.authenticationMode") } -// The API_AND_CONFIG_MAP hybrid still enables the unsupported aws-auth ConfigMap -// path, so it must be rejected the same as plain CONFIG_MAP — the sibling test -// only covers the CONFIG_MAP value. -func TestValidateCreateClusterInput_RejectsAPIAndConfigMapAuthMode(t *testing.T) { +// A junk authentication mode must be rejected the same way as CONFIG_MAP, and +// must also name the parameter rather than surfacing a bare code. +func TestValidateCreateClusterInput_RejectsJunkAuthMode(t *testing.T) { + in := createInput("alpha") + in.AccessConfig = &eks.CreateAccessConfigRequest{ + AuthenticationMode: aws.String("NOT_A_REAL_MODE"), + } + err := validateCreateClusterInput(in) + code, message, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok) + assert.Equal(t, awserrors.ErrorInvalidParameter, code) + assert.Contains(t, message, "accessConfig.authenticationMode") +} + +// API_AND_CONFIG_MAP is both AWS's default since 1.23 and the terraform-aws-eks +// module default. Its ConfigMap side grants nothing on Spinifex, exactly as it +// would on real AWS with an empty aws-auth ConfigMap, so it must be accepted. +func TestValidateCreateClusterInput_AcceptsAPIAndConfigMapAuthMode(t *testing.T) { in := createInput("alpha") in.AccessConfig = &eks.CreateAccessConfigRequest{ AuthenticationMode: aws.String(eks.AuthenticationModeApiAndConfigMap), } - require.EqualError(t, validateCreateClusterInput(in), awserrors.ErrorInvalidParameter) + require.NoError(t, validateCreateClusterInput(in)) } func TestValidateCreateClusterInput_AcceptsAPIAuthMode(t *testing.T) { @@ -111,6 +130,16 @@ func TestValidateCreateClusterInput_AcceptsAPIAuthMode(t *testing.T) { require.NoError(t, validateCreateClusterInput(in)) } +// An unset accessConfig (or an unset authenticationMode within it) must stay +// valid — it defaults to API, exactly as an explicit "API" does. +func TestValidateCreateClusterInput_AcceptsUnsetAuthMode(t *testing.T) { + in := createInput("alpha") + require.NoError(t, validateCreateClusterInput(in)) + + in.AccessConfig = &eks.CreateAccessConfigRequest{} + require.NoError(t, validateCreateClusterInput(in)) +} + // A create request with no subnets is malformed and must be rejected before any // orchestration work (InvalidParameterValue → 400). func TestValidateCreateClusterInput_RejectsMissingSubnetIds(t *testing.T) {