mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
fix(eks): accept API_AND_CONFIG_MAP and always return nodegroup health (#1045)
CreateCluster refused accessConfig.authenticationMode = API_AND_CONFIG_MAP, which is both AWS's default since 1.23 and the terraform-aws-eks module default, and named no parameter in the refusal. The mode is now stored on the cluster record and echoed by DescribeCluster, so a caller does not see a permanent access_config diff on every subsequent plan; bare CONFIG_MAP still fails, since Spinifex has no aws-auth ConfigMap path. The refusal now names accessConfig.authenticationMode via awserrors.Errorf. nodegroupRecordToAWS also only set Health when a node group had a status reason, so a healthy node group described back with health: null instead of the empty-issues object AWS always returns. Health is now always constructed with a non-nil issues slice.
This commit is contained in:
@@ -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))
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user