diff --git a/pkg/cli/clusterapi/local_service.go b/pkg/cli/clusterapi/local_service.go index d724f5212f..478968285b 100644 --- a/pkg/cli/clusterapi/local_service.go +++ b/pkg/cli/clusterapi/local_service.go @@ -24,6 +24,7 @@ import ( "github.com/devantler-tech/ksail/v7/pkg/svc/credentials" clusterprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster" "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster/clustererr" + "github.com/devantler-tech/ksail/v7/pkg/svc/state" "github.com/devantler-tech/ksail/v7/pkg/webui/api" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -516,6 +517,16 @@ func (s *Service) startJob( } s.mu.Lock() + if current, found := s.jobs[name]; found && jobIsInProgress(current.phase) { + s.mu.Unlock() + + return v1alpha1.Spec{}, fmt.Errorf( + "%w: operation already in progress for %q", + api.ErrAlreadyExists, + name, + ) + } + s.jobs[name] = &job{ distribution: distribution, provider: provider, @@ -529,6 +540,12 @@ func (s *Service) startJob( }, nil } +func jobIsInProgress(phase v1alpha1.ClusterPhase) bool { + return phase == v1alpha1.ClusterPhaseProvisioning || + phase == v1alpha1.ClusterPhaseDeleting || + phase == v1alpha1.ClusterPhaseUpdating +} + // runLifecycle marks a resolved cluster Updating, then runs the supplied provisioner action (Start or // Stop) in the background, clearing the job on success and recording the failure otherwise — the same // async pattern Create/Delete use so the web UI's list reflects progress without a UI change. @@ -588,6 +605,12 @@ func (s *Service) runCreate(ctx context.Context, name string, spec v1alpha1.Spec return p.Create(actionCtx, name) }, ) + if err == nil && spec.Cluster.Distribution == v1alpha1.DistributionEKS { + err = state.SaveClusterSpec(name, &spec.Cluster) + if err != nil { + err = fmt.Errorf("persist local EKS cluster ownership state: %w", err) + } + } s.mu.Lock() defer s.mu.Unlock() @@ -619,6 +642,18 @@ func (s *Service) runDelete(ctx context.Context, name string, spec v1alpha1.Spec return p.Delete(actionCtx, name) }, ) + if spec.Cluster.Distribution == v1alpha1.DistributionEKS && + (err == nil || errors.Is(err, clustererr.ErrClusterNotFound)) { + cleanupErr := state.DeleteClusterState(name) + if cleanupErr != nil { + // The cloud cluster is already gone, so the deletion itself succeeded. Turning a + // local-state cleanup failure (read-only home, permissions) into a job failure would + // pin an undismissable Failed row for a cluster that no longer exists — exactly the + // trap the idempotency note below describes. Warn, but still clear the job. + slog.Warn("failed to clean up local EKS cluster state after deletion", + "cluster", name, "error", cleanupErr) + } + } s.mu.Lock() defer s.mu.Unlock() diff --git a/pkg/cli/clusterapi/local_service_test.go b/pkg/cli/clusterapi/local_service_test.go index 9802f9b591..886e0d2466 100644 --- a/pkg/cli/clusterapi/local_service_test.go +++ b/pkg/cli/clusterapi/local_service_test.go @@ -15,6 +15,7 @@ import ( "github.com/devantler-tech/ksail/v7/pkg/svc/clusterdiscovery" clusterprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster" "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster/clustererr" + "github.com/devantler-tech/ksail/v7/pkg/svc/state" "github.com/devantler-tech/ksail/v7/pkg/webui/api" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -27,6 +28,9 @@ const ( // devClusterName is the discovered-cluster name shared by the List tests. devClusterName = "dev" + + // testEKSRegion is the region the EKS capacity-snapshot tests save and load under. + testEKSRegion = "eu-north-1" ) // Static sentinel errors used to drive provisioner failures in tests (err113 forbids inline @@ -44,6 +48,8 @@ type fakeProvisioner struct { clusters []string createGate chan struct{} deleteGate chan struct{} + startGate chan struct{} + stopGate chan struct{} createErr error deleteErr error startErr error @@ -108,6 +114,10 @@ func (f *fakeProvisioner) List(_ context.Context) ([]string, error) { } func (f *fakeProvisioner) Start(_ context.Context, name string) error { + if f.startGate != nil { + <-f.startGate + } + f.mu.Lock() defer f.mu.Unlock() @@ -117,6 +127,10 @@ func (f *fakeProvisioner) Start(_ context.Context, name string) error { } func (f *fakeProvisioner) Stop(_ context.Context, name string) error { + if f.stopGate != nil { + <-f.stopGate + } + f.mu.Lock() defer f.mu.Unlock() @@ -410,6 +424,203 @@ func TestDeleteIsAsyncAndRemovesCluster(t *testing.T) { assert.Equal(t, []string{"old"}, provisioner.deletedNames()) } +func TestDeleteEKSClearsPersistedOwnershipAndCapacityState(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + const clusterName = "old-eks" + + provisioner := &fakeProvisioner{} + service := newTestService(map[v1alpha1.Distribution]*fakeProvisioner{ + v1alpha1.DistributionEKS: provisioner, + }) + _, err := service.Create( + context.Background(), + clusterFor(clusterName, v1alpha1.DistributionEKS), + ) + require.NoError(t, err) + require.Eventually(t, func() bool { + list, listErr := service.List(context.Background()) + require.NoError(t, listErr) + + phase, found := phaseOf(list, clusterName) + + return found && phase == v1alpha1.ClusterPhaseReady + }, eventuallyTimeout, eventuallyTick) + + require.NoError(t, state.SaveClusterSpec(clusterName, &v1alpha1.ClusterSpec{ + Distribution: v1alpha1.DistributionEKS, + Provider: v1alpha1.ProviderAWS, + })) + saveEKSCapacitySnapshot(t, clusterName) + + require.NoError(t, service.Delete(context.Background(), "default", clusterName)) + require.Eventually(t, func() bool { + _, specErr := state.LoadClusterSpec(clusterName) + _, capacityErr := state.LoadEKSNodegroupState(clusterName, testEKSRegion) + + return errors.Is(specErr, state.ErrStateNotFound) && + errors.Is(capacityErr, state.ErrEKSNodegroupStateNotFound) + }, eventuallyTimeout, eventuallyTick) +} + +func TestDeleteEKSRejectsOverlappingCreate(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + const clusterName = "creating-eks" + + createGate := make(chan struct{}) + provisioner := &fakeProvisioner{createGate: createGate} + service := newTestService(map[v1alpha1.Distribution]*fakeProvisioner{ + v1alpha1.DistributionEKS: provisioner, + }) + _, err := service.Create( + context.Background(), + clusterFor(clusterName, v1alpha1.DistributionEKS), + ) + require.NoError(t, err) + + err = service.Delete(context.Background(), "default", clusterName) + require.ErrorIs(t, err, api.ErrAlreadyExists) + assert.Empty(t, provisioner.deletedNames()) + + close(createGate) + require.Eventually(t, func() bool { + list, listErr := service.List(context.Background()) + require.NoError(t, listErr) + + phase, found := phaseOf(list, clusterName) + + return found && phase == v1alpha1.ClusterPhaseReady + }, eventuallyTimeout, eventuallyTick) + + persisted, loadErr := state.LoadClusterSpec(clusterName) + require.NoError(t, loadErr) + assert.Equal(t, v1alpha1.DistributionEKS, persisted.Distribution) + assert.Equal(t, v1alpha1.ProviderAWS, persisted.Provider) +} + +func TestLifecycleRejectsOverlappingOperation(t *testing.T) { + t.Parallel() + + const clusterName = "busy" + + stopGate := make(chan struct{}) + provisioner := &fakeProvisioner{clusters: []string{clusterName}, stopGate: stopGate} + service := newTestService(map[v1alpha1.Distribution]*fakeProvisioner{ + v1alpha1.DistributionVCluster: provisioner, + }) + + require.NoError(t, service.Stop(context.Background(), "default", clusterName)) + err := service.Start(context.Background(), "default", clusterName) + require.ErrorIs(t, err, api.ErrAlreadyExists) + assert.Empty(t, provisioner.startedNames()) + + close(stopGate) + require.Eventually(t, func() bool { + return slices.Equal(provisioner.stoppedNames(), []string{clusterName}) + }, eventuallyTimeout, eventuallyTick) +} + +func TestDeleteEKSFailurePreservesPersistedOwnershipAndCapacityState(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + const clusterName = "stuck-eks" + + provisioner := &fakeProvisioner{deleteErr: errSimulatedDeleteFailure} + service := newTestService(map[v1alpha1.Distribution]*fakeProvisioner{ + v1alpha1.DistributionEKS: provisioner, + }) + _, err := service.Create( + context.Background(), + clusterFor(clusterName, v1alpha1.DistributionEKS), + ) + require.NoError(t, err) + require.Eventually(t, func() bool { + list, listErr := service.List(context.Background()) + require.NoError(t, listErr) + + phase, found := phaseOf(list, clusterName) + + return found && phase == v1alpha1.ClusterPhaseReady + }, eventuallyTimeout, eventuallyTick) + saveEKSCapacitySnapshot(t, clusterName) + + require.NoError(t, service.Delete(context.Background(), "default", clusterName)) + require.Eventually(t, func() bool { + list, listErr := service.List(context.Background()) + require.NoError(t, listErr) + + phase, found := phaseOf(list, clusterName) + + return found && phase == v1alpha1.ClusterPhaseFailed + }, eventuallyTimeout, eventuallyTick) + + _, specErr := state.LoadClusterSpec(clusterName) + require.NoError(t, specErr) + + _, capacityErr := state.LoadEKSNodegroupState(clusterName, testEKSRegion) + require.NoError(t, capacityErr) +} + +// saveEKSCapacitySnapshot writes a stop-time capacity snapshot for the cluster in the region the +// EKS delete tests use, so a test can assert whether deletion cleaned it up. +func saveEKSCapacitySnapshot(t *testing.T, clusterName string) { + t.Helper() + + require.NoError(t, state.SaveEKSNodegroupState(clusterName, testEKSRegion, + &state.EKSNodegroupState{ + Version: state.EKSNodegroupStateVersion, + ClusterName: clusterName, + Region: testEKSRegion, + Nodegroups: []state.EKSNodegroupCapacity{ + {Name: "workers", DesiredCapacity: 2, MinSize: 1, MaxSize: 3}, + }, + })) +} + +// TestDeleteEKSClearsJobWhenOnlyLocalStateCleanupFails covers the split between the cloud +// operation and local bookkeeping: once the EKS cluster is actually gone, a failure to remove the +// local state directory (read-only home, permissions) must not pin the job Failed. That would leave +// an undismissable row in the web UI for a cluster that no longer exists — the very trap the +// idempotent-delete behaviour exists to avoid. The cleanup failure is a warning, not a job failure. +func TestDeleteEKSClearsJobWhenOnlyLocalStateCleanupFails(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + const clusterName = "cleanup-fails" + + service := newTestService(map[v1alpha1.Distribution]*fakeProvisioner{ + v1alpha1.DistributionEKS: {}, + }) + _, err := service.Create( + context.Background(), + clusterFor(clusterName, v1alpha1.DistributionEKS), + ) + require.NoError(t, err) + require.Eventually(t, func() bool { + list, listErr := service.List(context.Background()) + require.NoError(t, listErr) + + phase, found := phaseOf(list, clusterName) + + return found && phase == v1alpha1.ClusterPhaseReady + }, eventuallyTimeout, eventuallyTick) + + // Force local state cleanup to fail deterministically: with no resolvable home directory the + // state layer cannot locate — and therefore cannot remove — the cluster's state directory. The + // provisioner still reports a clean deletion. + t.Setenv("HOME", "") + + require.NoError(t, service.Delete(context.Background(), "default", clusterName)) + require.Eventually(t, func() bool { + list, listErr := service.List(context.Background()) + require.NoError(t, listErr) + + _, found := phaseOf(list, clusterName) + + return !found + }, eventuallyTimeout, eventuallyTick) +} + // TestDeleteClearsFailedClusterWithNoUnderlyingCluster reproduces the bug where a cluster left in // the Failed phase by a failed create could never be removed from the web UI: deleting it called // the provisioner's Delete, which returned ErrClusterNotFound (there is no cluster to delete), and @@ -856,6 +1067,79 @@ func TestCreateDefaultsEKSProviderToAWS(t *testing.T) { case <-time.After(eventuallyTimeout): t.Fatal("factory was never asked to build the EKS cluster with a defaulted AWS provider") } + + var persisted *v1alpha1.ClusterSpec + + require.Eventually(t, func() bool { + var loadErr error + + persisted, loadErr = state.LoadClusterSpec("prod-eks") + + return loadErr == nil + }, eventuallyTimeout, eventuallyTick) + require.NotNil(t, persisted) + assert.Equal(t, v1alpha1.DistributionEKS, persisted.Distribution) + assert.Equal(t, v1alpha1.ProviderAWS, persisted.Provider) +} + +func TestCreateEKSFailureDoesNotPersistOwnershipState(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + const clusterName = "failed-eks" + + provisioner := &fakeProvisioner{createErr: errSimulatedCreateFailure} + service := newTestService(map[v1alpha1.Distribution]*fakeProvisioner{ + v1alpha1.DistributionEKS: provisioner, + }) + _, err := service.Create( + context.Background(), + clusterFor(clusterName, v1alpha1.DistributionEKS), + ) + require.NoError(t, err) + require.Eventually(t, func() bool { + list, listErr := service.List(context.Background()) + require.NoError(t, listErr) + + phase, found := phaseOf(list, clusterName) + + return found && phase == v1alpha1.ClusterPhaseFailed + }, eventuallyTimeout, eventuallyTick) + + _, loadErr := state.LoadClusterSpec(clusterName) + require.ErrorIs(t, loadErr, state.ErrStateNotFound) +} + +func TestCreateEKSOwnershipPersistenceFailureMarksJobFailed(t *testing.T) { + homeFile := filepath.Join(t.TempDir(), "home-file") + require.NoError(t, os.WriteFile(homeFile, []byte("not a directory"), 0o600)) + t.Setenv("HOME", homeFile) + + const clusterName = "untracked-eks" + + provisioner := &fakeProvisioner{} + service := newTestService(map[v1alpha1.Distribution]*fakeProvisioner{ + v1alpha1.DistributionEKS: provisioner, + }) + _, err := service.Create( + context.Background(), + clusterFor(clusterName, v1alpha1.DistributionEKS), + ) + require.NoError(t, err) + require.Eventually(t, func() bool { + list, listErr := service.List(context.Background()) + require.NoError(t, listErr) + + cluster := clusterNamed(list, clusterName) + + return cluster != nil && cluster.Status.Phase == v1alpha1.ClusterPhaseFailed + }, eventuallyTimeout, eventuallyTick) + + list, listErr := service.List(context.Background()) + require.NoError(t, listErr) + + conditions := conditionsOf(list, clusterName) + require.Len(t, conditions, 1) + assert.Contains(t, conditions[0].Message, "persist local EKS cluster ownership state") } // clusterNamed returns a pointer to the cluster with the given name in the list, or nil if absent. diff --git a/pkg/cli/cmd/cluster/context_test.go b/pkg/cli/cmd/cluster/context_test.go index d5591fce34..c27080d8ef 100644 --- a/pkg/cli/cmd/cluster/context_test.go +++ b/pkg/cli/cmd/cluster/context_test.go @@ -6,10 +6,12 @@ import ( "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" "github.com/devantler-tech/ksail/v7/pkg/cli/cmd/cluster" "github.com/devantler-tech/ksail/v7/pkg/cli/setup/localregistry" + clusterprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster" "github.com/k3d-io/k3d/v5/pkg/config/types" v1alpha5 "github.com/k3d-io/k3d/v5/pkg/config/v1alpha5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/kind/pkg/apis/config/v1alpha4" ) @@ -55,6 +57,23 @@ func TestResolveClusterNameFromContext_K3s(t *testing.T) { require.Equal(t, "k3s-cluster", name) } +func TestResolveClusterNameFromContext_EKS(t *testing.T) { + t.Parallel() + + ctx := &localregistry.Context{ + ClusterCfg: &v1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "stale-top-level-name"}, + Spec: v1alpha1.Spec{ + Cluster: v1alpha1.ClusterSpec{Distribution: v1alpha1.DistributionEKS}, + }, + }, + EKSConfig: &clusterprovisioner.EKSConfig{Name: "actual-eks-name"}, + } + + name := cluster.ExportResolveClusterNameFromContext(ctx) + require.Equal(t, "actual-eks-name", name) +} + func TestResolveClusterNameFromContext_FallbackToContext(t *testing.T) { t.Parallel() diff --git a/pkg/cli/cmd/cluster/create.go b/pkg/cli/cmd/cluster/create.go index 52dd939928..89ad0c68ab 100644 --- a/pkg/cli/cmd/cluster/create.go +++ b/pkg/cli/cmd/cluster/create.go @@ -89,6 +89,11 @@ func handleCreateRunE( return err } + err = validateEKSMutationConfigSource(ctx) + if err != nil { + return err + } + clusterflags.ApplyClusterMutationFlags(cmd, ctx.ClusterCfg) err = validatePostMutationFlags(ctx) @@ -108,7 +113,7 @@ func handleCreateRunE( notify.Warningf(cmd.OutOrStderr(), "failed to save cluster state: %v", saveErr) } - return maybeWaitForTTL(cmd, clusterName, ctx.ClusterCfg) + return maybeWaitForTTL(cmd, clusterName, ctx.ClusterCfg, ctx.EKSConfig) } // newProvisionerFactory returns the cluster provisioner factory, using any test override if set. @@ -577,15 +582,7 @@ func applyClusterNameOverride(ctx *localregistry.Context, name string) error { return nil } - // Update Kind config - if ctx.KindConfig != nil { - ctx.KindConfig.Name = name - } - - // Update K3d config - if ctx.K3dConfig != nil { - ctx.K3dConfig.Name = name - } + applyDirectClusterNameOverrides(ctx, name) // Update Talos config - must regenerate bundle for new cluster name // because cluster name is embedded in PKI and kubeconfig context @@ -598,16 +595,6 @@ func applyClusterNameOverride(ctx *localregistry.Context, name string) error { ctx.TalosConfig = newConfig } - // Update VCluster config - if ctx.VClusterConfig != nil { - ctx.VClusterConfig.Name = name - } - - // Update KWOK config - if ctx.KWOKConfig != nil { - ctx.KWOKConfig.Name = name - } - // Update the ksail.yaml context to match the pattern the created cluster uses. // Must be provider-aware: the Kubernetes (k3k) provider writes a "k3k-" // context for K3s rather than the standalone "k3d-", so post-creation CNI @@ -625,6 +612,28 @@ func applyClusterNameOverride(ctx *localregistry.Context, name string) error { return nil } +// applyDirectClusterNameOverrides updates in-memory distribution configs whose names directly drive +// creation. Talos is handled separately because renaming it must regenerate its PKI-bearing bundle. +// EKS is deliberately excluded: eksctl creates from the unchanged on-disk eks.yaml, so changing only +// EKSConfig.Name would make later state and deletion target a cluster that was never created. +func applyDirectClusterNameOverrides(ctx *localregistry.Context, name string) { + if ctx.KindConfig != nil { + ctx.KindConfig.Name = name + } + + if ctx.K3dConfig != nil { + ctx.K3dConfig.Name = name + } + + if ctx.VClusterConfig != nil { + ctx.VClusterConfig.Name = name + } + + if ctx.KWOKConfig != nil { + ctx.KWOKConfig.Name = name + } +} + // importCachedImages imports container images from a tar archive to the cluster. // This is called after cluster creation but before component installation to ensure // CNI, CSI, metrics-server, and other components can use pre-loaded images. @@ -690,16 +699,25 @@ func resolveClusterNameFromContext(ctx *localregistry.Context) string { return resolveVClusterName(ctx) case v1alpha1.DistributionKWOK: return resolveKWOKName(ctx) - case v1alpha1.DistributionEKS, v1alpha1.DistributionGKE, v1alpha1.DistributionAKS: - // EKS/GKE/AKS configs are owned by their cloud tooling (eks.yaml/ - // gke.yaml/aks.yaml) and not cached on the local registry context; - // fall back to the cluster-level name. + case v1alpha1.DistributionEKS: + return resolveEKSName(ctx) + case v1alpha1.DistributionGKE, v1alpha1.DistributionAKS: + // GKE/AKS configs are owned by their cloud tooling and not cached on the local registry + // context; fall back to the cluster-level name. return resolveFallbackName(ctx) default: return resolveFallbackName(ctx) } } +func resolveEKSName(ctx *localregistry.Context) string { + if ctx.EKSConfig != nil && strings.TrimSpace(ctx.EKSConfig.Name) != "" { + return strings.TrimSpace(ctx.EKSConfig.Name) + } + + return resolveFallbackName(ctx) +} + func resolveVClusterName(ctx *localregistry.Context) string { if ctx.VClusterConfig != nil && ctx.VClusterConfig.Name != "" { return ctx.VClusterConfig.Name @@ -738,6 +756,7 @@ func maybeWaitForTTL( cmd *cobra.Command, clusterName string, clusterCfg *v1alpha1.Cluster, + eksConfig *clusterprovisioner.EKSConfig, ) error { ttlStr, _ := cmd.Flags().GetString("ttl") if ttlStr == "" { @@ -756,6 +775,11 @@ func maybeWaitForTTL( return nil } + clusterName, err = ttlAutoDeleteTargetName(clusterName, clusterCfg, eksConfig) + if err != nil { + return fmt.Errorf("resolve TTL auto-delete target: %w", err) + } + // Persist TTL for informational display (ksail cluster list / info). saveErr := state.SaveClusterTTL(clusterName, ttl) if saveErr != nil { @@ -764,5 +788,5 @@ func maybeWaitForTTL( } // Block and wait for TTL, then auto-destroy. - return waitForTTLAndDelete(cmd, clusterName, clusterCfg, ttl) + return waitForTTLAndDelete(cmd, clusterName, clusterCfg, eksConfig, ttl) } diff --git a/pkg/cli/cmd/cluster/create_test.go b/pkg/cli/cmd/cluster/create_test.go index 6ab0b616c1..aadbcc9947 100644 --- a/pkg/cli/cmd/cluster/create_test.go +++ b/pkg/cli/cmd/cluster/create_test.go @@ -515,6 +515,79 @@ func TestCreate_NoConfigFile_FlagsOnly(t *testing.T) { require.Contains(t, output, "Create cluster", "output should contain cluster lifecycle text") } +// TestCreate_NonEKSNameOverrideReachesProvisioner guards the shared mutation path: rejecting EKS +// source-name drift must not suppress explicit --name overrides for other distributions. +// +//nolint:paralleltest // uses t.Chdir and mutates shared test hooks +func TestCreate_NonEKSNameOverrideReachesProvisioner(t *testing.T) { + workingDir := t.TempDir() + t.Chdir(workingDir) + + writeFile( + t, + workingDir, + "kind.yaml", + "kind: Cluster\napiVersion: kind.x-k8s.io/v1alpha4\nname: source-kind-name\nnodes: []\n", + ) + writeFile(t, workingDir, "kubeconfig", "apiVersion: v1\nkind: Config\n") + setupMockRegistryBackend(t) + + var createdName string + + restoreFactory := cluster.SetProvisionerFactoryForTests(fakeFactory{createName: &createdName}) + defer restoreFactory() + + cmd := cluster.NewCreateCmd() + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetContext(t.Context()) + cmd.SetArgs([]string{ + "--distribution", "Vanilla", + "--name", "explicit-kind-name", + "--kubeconfig", "./kubeconfig", + }) + + require.NoError(t, cmd.Execute()) + assert.Equal(t, "explicit-kind-name", createdName) +} + +// TestCreateRejectsWhitespaceOnlyName preserves explicit-flag validation: whitespace is not an +// absent override and must not silently fall back to the distribution config target. +// +//nolint:paralleltest // uses t.Chdir and mutates shared test hooks +func TestCreateRejectsWhitespaceOnlyName(t *testing.T) { + workingDir := t.TempDir() + t.Chdir(workingDir) + + writeFile( + t, + workingDir, + "kind.yaml", + "kind: Cluster\napiVersion: kind.x-k8s.io/v1alpha4\nname: source-kind-name\nnodes: []\n", + ) + writeFile(t, workingDir, "kubeconfig", "apiVersion: v1\nkind: Config\n") + setupMockRegistryBackend(t) + + var createdName string + + restoreFactory := cluster.SetProvisionerFactoryForTests(fakeFactory{createName: &createdName}) + defer restoreFactory() + + cmd := cluster.NewCreateCmd() + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetContext(t.Context()) + cmd.SetArgs([]string{ + "--distribution", "Vanilla", + "--name", " ", + "--kubeconfig", "./kubeconfig", + }) + + err := cmd.Execute() + require.ErrorContains(t, err, "invalid cluster name") + assert.Empty(t, createdName) +} + // TestCreate_NoConfigFile_WithComponentFlags verifies that component flags // (e.g. --metrics-server Disabled) are respected when no ksail.yaml exists. // diff --git a/pkg/cli/cmd/cluster/delete.go b/pkg/cli/cmd/cluster/delete.go index b149aacb7a..44054b1d61 100644 --- a/pkg/cli/cmd/cluster/delete.go +++ b/pkg/cli/cmd/cluster/delete.go @@ -96,12 +96,19 @@ func runDeleteAction( tmr := timer.New() tmr.Start() - // Resolve cluster info from flags, config, or kubeconfig - resolved, err := lifecycle.ResolveClusterInfo(cmd, flags.Name, flags.Provider, flags.Kubeconfig) + // Strict: delete is destructive, so an unreadable config aborts before anything is removed. + resolved, err := lifecycle.ResolveClusterInfoStrict( + cmd, flags.Name, flags.Provider, flags.Kubeconfig, + ) if err != nil { return fmt.Errorf("failed to resolve cluster info: %w", err) } + err = lifecycle.ValidateStandaloneAWSTarget(resolved) + if err != nil { + return fmt.Errorf("validate standalone AWS target: %w", err) + } + // Refuse to destroy a cluster ksail did not provision. When the resolved context is an unmanaged // cluster (a managed cloud cluster, a kubeadm cluster, a colleague's cluster) the guard rejects // here — before any provisioner is created or the cluster is touched — so ksail never accidentally @@ -116,12 +123,9 @@ func runDeleteAction( detectedInfo, isKindCluster, clusterInfo := detectDeleteClusterInfo(cmd, resolved) // Create provisioner for the provider - provisioner, err := createDeleteProvisioner( - clusterInfo, - resolved.OmniOpts, - resolved.KubernetesOpts, - flags.storage, - ) + options := minimalProvisionerOptions(resolved, flags.storage) + + provisioner, err := createDeleteProvisioner(cmd.Context(), clusterInfo, options) if err != nil { return fmt.Errorf("failed to create provisioner: %w", err) } @@ -157,6 +161,19 @@ func runDeleteAction( return nil } +func minimalProvisionerOptions( + resolved *lifecycle.ResolvedClusterInfo, + deleteStorage bool, +) lifecycle.MinimalProvisionerOptions { + return lifecycle.MinimalProvisionerOptions{ + OmniOpts: resolved.OmniOpts, + KubernetesOpts: resolved.KubernetesOpts, + AWSOpts: resolved.AWSOpts, + AWSRegion: resolved.AWSRegion, + DeleteStorage: deleteStorage, + } +} + // detectClusterDistribution detects the distribution and other cluster info. // This detection must happen before the cluster is deleted to ensure the kubeconfig // entry is still available for reading cluster information. @@ -306,10 +323,9 @@ func promptForDeletion( // createDeleteProvisioner creates the appropriate provisioner for cluster deletion. // It first checks for test overrides, then falls back to creating a minimal provisioner. func createDeleteProvisioner( + ctx context.Context, clusterInfo *clusterdetector.Info, - omniOpts v1alpha1.OptionsOmni, - kubernetesOpts v1alpha1.OptionsKubernetes, - deleteStorage bool, + options lifecycle.MinimalProvisionerOptions, ) (clusterprovisioner.Provisioner, error) { // Check for test factory override clusterProvisionerFactoryMu.RLock() @@ -319,7 +335,7 @@ func createDeleteProvisioner( clusterProvisionerFactoryMu.RUnlock() if factoryOverride != nil { - provisioner, _, err := factoryOverride.Create(context.Background(), nil) + provisioner, _, err := factoryOverride.Create(ctx, nil) if err != nil { return nil, fmt.Errorf("factory override failed: %w", err) } @@ -328,7 +344,7 @@ func createDeleteProvisioner( } provisioner, err := lifecycle.CreateMinimalProvisionerForProvider( - clusterInfo, omniOpts, kubernetesOpts, deleteStorage, + ctx, clusterInfo, options, ) if err != nil { return nil, fmt.Errorf("failed to create provisioner for provider: %w", err) diff --git a/pkg/cli/cmd/cluster/eks_lifecycle_test.go b/pkg/cli/cmd/cluster/eks_lifecycle_test.go new file mode 100644 index 0000000000..88326adfa8 --- /dev/null +++ b/pkg/cli/cmd/cluster/eks_lifecycle_test.go @@ -0,0 +1,1563 @@ +package cluster_test + +import ( + "bytes" + "context" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" + "github.com/devantler-tech/ksail/v7/pkg/cli/cmd/cluster" + "github.com/devantler-tech/ksail/v7/pkg/cli/lifecycle" + "github.com/devantler-tech/ksail/v7/pkg/fsutil" + clusterprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster" + "github.com/devantler-tech/ksail/v7/pkg/svc/state" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const standaloneEKSClusterFixture = `apiVersion: ksail.io/v1alpha1 +kind: Cluster +metadata: + name: config-file-name +spec: + cluster: + distribution: EKS + provider: AWS + distributionConfig: eks.yaml + connection: + kubeconfig: kubeconfig + provider: + aws: + profileEnvVar: KSAIL_PROFILE + regionEnvVar: KSAIL_REGION + accessKeyIdEnvVar: KSAIL_ACCESS + secretAccessKeyEnvVar: KSAIL_SECRET + sessionTokenEnvVar: KSAIL_SESSION +` + +const standaloneEKSEksConfigFixture = `apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + name: config-file-name + region: eu-west-1 +` + +const standaloneEKSEksctlFixture = `#!/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_MARKER" + +if [ "${KSAIL_EKSCTL_FAIL-}" = "$1" ]; then + printf 'forced %s failure\n' "$1" >&2 + exit 49 +fi + +if [ "$1 $2" = "get cluster" ]; then + if [ "${KSAIL_EKSCTL_CLUSTER_ABSENT-}" = "1" ]; then + printf '[]\n' + exit 0 + fi + discovered_cluster="${KSAIL_EKS_DISCOVERED_CLUSTER:-$KSAIL_EKS_CLUSTER}" + if [ "${KSAIL_EKS_DISCOVERED_REGION+x}" = "x" ]; then + discovered_region="$KSAIL_EKS_DISCOVERED_REGION" + else + discovered_region="ap-southeast-2" + fi + eksctl_created="${KSAIL_EKSCTL_CREATED-True}" + printf '[{"Name":"%s","Region":"%s",' "$discovered_cluster" "$discovered_region" + printf '"EksctlCreated":"%s"}]\n' "$eksctl_created" +elif [ "$1 $2" = "get nodegroup" ]; then + current_desired="${KSAIL_EKS_NODEGROUP_DESIRED:-0}" + current_min="${KSAIL_EKS_NODEGROUP_MIN:-2}" + if grep -q -- '--nodes 2 --nodes-min 2' "$KSAIL_EKSCTL_MARKER"; then + current_desired=2 + current_min=2 + fi + printf '[{"Cluster":"%s","Name":"workers","Status":"ACTIVE",' "$KSAIL_EKS_CLUSTER" + printf '"DesiredCapacity":%s,"MinSize":%s,"MaxSize":4,"NodeGroupType":"managed"}]\n' \ + "$current_desired" "$current_min" +fi +` + +type standaloneEKSLifecycleCase struct { + name string + newCommand func() *cobra.Command + extraArgs []string + expectedCalls func(clusterName string) []string +} + +func standaloneEKSLifecycleCases() []standaloneEKSLifecycleCase { + return []standaloneEKSLifecycleCase{ + { + name: "delete", + newCommand: cluster.NewDeleteCmd, + extraArgs: []string{"--force"}, + expectedCalls: func(clusterName string) []string { + return []string{ + fmt.Sprintf( + "get cluster --name %s --output json --region ap-southeast-2", + clusterName, + ), + "get cluster --output json --region ap-southeast-2", + fmt.Sprintf( + "delete cluster --name %s --region ap-southeast-2 --wait", + clusterName, + ), + } + }, + }, + { + name: "start", + newCommand: cluster.NewStartCmd, + expectedCalls: func(clusterName string) []string { + return []string{ + fmt.Sprintf( + "get cluster --name %s --output json --region ap-southeast-2", + clusterName, + ), + fmt.Sprintf( + "get nodegroup --cluster %s --output json --region ap-southeast-2", + clusterName, + ), + fmt.Sprintf( + "scale nodegroup --cluster %s --name workers --nodes 2 --nodes-min 2 --nodes-max 4 --region ap-southeast-2", + clusterName, + ), + } + }, + }, + { + name: "stop", + newCommand: cluster.NewStopCmd, + expectedCalls: func(clusterName string) []string { + return []string{ + fmt.Sprintf( + "get cluster --name %s --output json --region ap-southeast-2", + clusterName, + ), + fmt.Sprintf( + "get nodegroup --cluster %s --output json --region ap-southeast-2", + clusterName, + ), + fmt.Sprintf( + "scale nodegroup --cluster %s --name workers --nodes 0 --nodes-min 0 --nodes-max 4 --region ap-southeast-2", + clusterName, + ), + } + }, + }, + } +} + +// TestStandaloneEKSLifecycleCommandsRouteToEksctl proves the three standalone +// command surfaces reach the existing EKS provisioner, honor the custom AWS +// credential mapping, and prefer the configured region environment variable. +func TestStandaloneEKSLifecycleCommandsRouteToEksctl(t *testing.T) { + for _, testCase := range standaloneEKSLifecycleCases() { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "ksail-eks-" + testCase.name + "-routing-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + configureStandaloneEKSNodegroupAction(t, testCase.name) + + if testCase.name == "start" { + // EksctlCreated is normalized explicitly: case and surrounding whitespace are benign. + t.Setenv("KSAIL_EKSCTL_CREATED", " true ") + } + + cmd := testCase.newCommand() + args := make([]string, 0, 4+len(testCase.extraArgs)) + args = append(args, "--name", clusterName, "--provider", "AWS") + args = append(args, testCase.extraArgs...) + cmd.SetArgs(args) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + require.NoError(t, cmd.Execute()) + + calls, err := os.ReadFile(markerPath) //nolint:gosec // test-private path + require.NoError(t, err) + assert.Equal( + t, + testCase.expectedCalls(clusterName), + strings.Split(strings.TrimSpace(string(calls)), "\n"), + ) + assertParentAWSEnvironmentUnchanged(t) + }) + } +} + +// TestStandaloneEKSStopStartRestoresExactCapacity exercises the user-facing commands across the +// persisted boundary and proves start verifies the restored tuple before clearing its snapshot. +func TestStandaloneEKSStopStartRestoresExactCapacity(t *testing.T) { + const clusterName = "ksail-eks-stop-start-roundtrip-6087" + + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("KSAIL_EKS_NODEGROUP_DESIRED", "2") + t.Setenv("KSAIL_EKS_NODEGROUP_MIN", "2") + runStandaloneEKSCommand(t, cluster.NewStopCmd, "--name", clusterName, "--provider", "AWS") + + t.Setenv("KSAIL_EKS_NODEGROUP_DESIRED", "0") + t.Setenv("KSAIL_EKS_NODEGROUP_MIN", "0") + runStandaloneEKSCommand(t, cluster.NewStartCmd, "--name", clusterName, "--provider", "AWS") + + assert.Equal(t, []string{ + fmt.Sprintf("get cluster --name %s --output json --region ap-southeast-2", clusterName), + fmt.Sprintf( + "get nodegroup --cluster %s --output json --region ap-southeast-2", + clusterName, + ), + fmt.Sprintf( + "scale nodegroup --cluster %s --name workers "+ + "--nodes 0 --nodes-min 0 --nodes-max 4 --region ap-southeast-2", + clusterName, + ), + fmt.Sprintf("get cluster --name %s --output json --region ap-southeast-2", clusterName), + fmt.Sprintf( + "get nodegroup --cluster %s --output json --region ap-southeast-2", + clusterName, + ), + fmt.Sprintf( + "scale nodegroup --cluster %s --name workers "+ + "--nodes 2 --nodes-min 2 --nodes-max 4 --region ap-southeast-2", + clusterName, + ), + fmt.Sprintf( + "get nodegroup --cluster %s --output json --region ap-southeast-2", + clusterName, + ), + }, readStandaloneEKSCalls(t, markerPath)) + + _, err := state.LoadEKSNodegroupState(clusterName, "ap-southeast-2") + require.ErrorIs(t, err, state.ErrEKSNodegroupStateNotFound) +} + +// TestStandaloneEKSLifecycleCommandsPropagateEksctlErrors covers each command's +// user-facing error path after routing has reached eksctl. +func TestStandaloneEKSLifecycleCommandsPropagateEksctlErrors(t *testing.T) { + testCases := []struct { + name string + newCommand func() *cobra.Command + extraArgs []string + failOn string + wantError string + }{ + { + name: "delete", + newCommand: cluster.NewDeleteCmd, + extraArgs: []string{"--force"}, + failOn: "delete", + wantError: "cluster deletion failed", + }, + { + name: "start", + newCommand: cluster.NewStartCmd, + failOn: "scale", + wantError: "failed to starting cluster", + }, + { + name: "stop", + newCommand: cluster.NewStopCmd, + failOn: "scale", + wantError: "failed to stopping cluster", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "ksail-eks-" + testCase.name + "-error-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + configureStandaloneEKSNodegroupAction(t, testCase.name) + t.Setenv("KSAIL_EKSCTL_FAIL", testCase.failOn) + + cmd := testCase.newCommand() + args := make([]string, 0, 4+len(testCase.extraArgs)) + args = append(args, "--name", clusterName, "--provider", "AWS") + args = append(args, testCase.extraArgs...) + cmd.SetArgs(args) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), testCase.wantError) + + calls, readErr := os.ReadFile(markerPath) //nolint:gosec // test-private path + require.NoError(t, readErr) + assert.Contains(t, string(calls), testCase.failOn+" ") + assertParentAWSEnvironmentUnchanged(t) + }) + } +} + +// TestStandaloneEKSLifecycleCommandsRejectUnmanagedEKSContext verifies a real +// eksctl kubeconfig context is not mistaken for a same-named cluster managed by +// another provider. EKS ownership must be checked with an exact query using the +// resolved AWS credentials and region, and a target absent from that query is refused +// before delete or nodegroup scaling can mutate it. +func TestStandaloneEKSLifecycleCommandsRejectUnmanagedEKSContext(t *testing.T) { + for _, testCase := range standaloneEKSLifecycleCases() { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "ksail-eks-" + testCase.name + "-unmanaged-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + writeStandaloneEKSKubeconfig(t, clusterName, "ap-southeast-2") + t.Setenv("KSAIL_EKSCTL_CLUSTER_ABSENT", "1") + + cmd := testCase.newCommand() + args := make([]string, 0, 4+len(testCase.extraArgs)) + args = append(args, "--name", clusterName, "--provider", "AWS") + args = append(args, testCase.extraArgs...) + cmd.SetArgs(args) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.ErrorIs(t, err, cluster.ErrUnmanagedCluster) + + calls := readStandaloneEKSCalls(t, markerPath) + assert.Equal( + t, + []string{fmt.Sprintf( + "get cluster --name %s --output json --region ap-southeast-2", + clusterName, + )}, + calls, + ) + + for _, call := range calls { + assert.NotContains(t, call, "delete cluster") + assert.NotContains(t, call, "scale nodegroup") + } + + assertParentAWSEnvironmentUnchanged(t) + }) + } +} + +// TestStandaloneEKSLifecycleCommandsRejectConfigRegionForDifferentExplicitName +// verifies an explicit target cannot silently inherit the region from an +// unrelated local eks.yaml. Without an explicit region, every mutating EKS +// lifecycle command must fail before invoking eksctl. +func TestStandaloneEKSLifecycleCommandsRejectConfigRegionForDifferentExplicitName(t *testing.T) { + for _, testCase := range standaloneEKSLifecycleCases() { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "ksail-eks-" + testCase.name + "-region-mismatch-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + require.NoError( + t, + os.WriteFile( + "eks.yaml", + []byte(`apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + name: different-config-cluster-6087 + region: eu-west-1 +`), + 0o600, + ), + ) + t.Setenv("KSAIL_REGION", "") + + cmd := testCase.newCommand() + args := make([]string, 0, 4+len(testCase.extraArgs)) + args = append(args, "--name", clusterName, "--provider", "AWS") + args = append(args, testCase.extraArgs...) + cmd.SetArgs(args) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "does not match EKS config cluster") + assert.Empty(t, readStandaloneEKSCalls(t, markerPath)) + assertParentAWSEnvironmentUnchanged(t) + }) + } +} + +// TestStandaloneEKSLifecycleCommandsRequireLocalOwnershipEvidence verifies that an explicit AWS +// region and eksctl's generic creation marker are not enough to mutate a colleague's cluster. The +// target must also match a loaded KSail EKS config or have persisted KSail creation state. +func TestStandaloneEKSLifecycleCommandsRequireLocalOwnershipEvidence(t *testing.T) { + for _, testCase := range standaloneEKSLifecycleCases() { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "ksail-eks-" + testCase.name + "-no-local-owner-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("HOME", t.TempDir()) + writeStandaloneEKSEksConfig(t, "unrelated-local-cluster-6087") + + cmd := testCase.newCommand() + args := make([]string, 0, 4+len(testCase.extraArgs)) + args = append(args, "--name", clusterName, "--provider", "AWS") + args = append(args, testCase.extraArgs...) + cmd.SetArgs(args) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.ErrorIs(t, err, cluster.ErrUnmanagedCluster) + assert.Contains(t, err.Error(), "no local KSail ownership evidence") + assert.Empty(t, readStandaloneEKSCalls(t, markerPath)) + assertParentAWSEnvironmentUnchanged(t) + }) + } +} + +// TestStandaloneEKSRejectsSameNamedNonEKSConfig verifies that another distribution's local config +// cannot authorize an AWS mutation merely because it carries the same cluster name. +func TestStandaloneEKSRejectsSameNamedNonEKSConfig(t *testing.T) { + const clusterName = "same-name-kind-and-eks-6087" + + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("HOME", t.TempDir()) + require.NoError( + t, + os.WriteFile( + "ksail.yaml", + fmt.Appendf(nil, `apiVersion: ksail.io/v1alpha1 +kind: Cluster +metadata: + name: %s +spec: + cluster: + distribution: Vanilla + provider: Docker + distributionConfig: kind.yaml + connection: + kubeconfig: kubeconfig +`, clusterName), + 0o600, + ), + ) + require.NoError( + t, + os.WriteFile( + "kind.yaml", + fmt.Appendf(nil, `kind: Cluster +apiVersion: kind.x-k8s.io/v1alpha4 +name: %s +`, clusterName), + 0o600, + ), + ) + + cmd := cluster.NewStartCmd() + cmd.SetArgs([]string{"--name", clusterName, "--provider", "AWS"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.ErrorIs(t, err, cluster.ErrUnmanagedCluster) + assert.Contains(t, err.Error(), "no local KSail ownership evidence") + assert.Empty(t, readStandaloneEKSCalls(t, markerPath)) +} + +// TestStandaloneEKSRejectsSyntheticNameFromNamelessConfig verifies ConfigManager's fallback +// `eks-default` name is not mistaken for a name explicitly authorized by an actual eks.yaml source. +func TestStandaloneEKSRejectsSyntheticNameFromNamelessConfig(t *testing.T) { + markerPath := setupStandaloneEKSLifecycleFixture(t, "eks-default") + t.Setenv("HOME", t.TempDir()) + require.NoError( + t, + os.WriteFile( + "eks.yaml", + []byte(`apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + region: ap-southeast-2 +`), + 0o600, + ), + ) + + cmd := cluster.NewStartCmd() + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.ErrorIs(t, err, cluster.ErrUnmanagedCluster) + assert.Contains(t, err.Error(), "no local KSail ownership evidence") + assert.Empty(t, readStandaloneEKSCalls(t, markerPath)) +} + +// TestStandaloneEKSRejectsMetadataFromWrongConfigKind verifies arbitrary YAML cannot grant local +// EKS ownership merely by carrying matching metadata. The actual distribution source must be an +// eksctl ClusterConfig before its name or region can reach a lifecycle guard. +// +//nolint:paralleltest // setup mutates process environment and the working directory +func TestStandaloneEKSRejectsMetadataFromWrongConfigKind(t *testing.T) { + testStandaloneEKSLifecycleCommandsRejectConfig( + t, + "wrong-kind-provenance", + func(clusterName string) []byte { + return fmt.Appendf(nil, `apiVersion: v1 +kind: ConfigMap +metadata: + name: %s + region: ap-southeast-2 +`, clusterName) + }, + ) +} + +// TestStandaloneEKSRejectsNonCanonicalConfigName verifies a padded raw source name cannot be +// normalized into local ownership evidence for delete, start, or stop. +// +//nolint:paralleltest // setup mutates process environment and the working directory +func TestStandaloneEKSRejectsNonCanonicalConfigName(t *testing.T) { + testStandaloneEKSLifecycleCommandsRejectConfig( + t, + "noncanonical-name", + func(clusterName string) []byte { + return fmt.Appendf(nil, `apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + name: "%s " + region: ap-southeast-2 +`, clusterName) + }, + ) +} + +func testStandaloneEKSLifecycleCommandsRejectConfig( + t *testing.T, + testSuffix string, + configContent func(string) []byte, +) { + t.Helper() + + for _, testCase := range standaloneEKSLifecycleCases() { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "ksail-eks-" + testCase.name + "-" + testSuffix + "-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + require.NoError(t, os.WriteFile("eks.yaml", configContent(clusterName), 0o600)) + + cmd := testCase.newCommand() + args := make([]string, 0, 4+len(testCase.extraArgs)) + args = append(args, "--name", clusterName, "--provider", "AWS") + args = append(args, testCase.extraArgs...) + cmd.SetArgs(args) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid EKS config file") + assert.Empty(t, readStandaloneEKSCalls(t, markerPath)) + }) + } +} + +// TestStandaloneEKSLifecycleCommandsIgnoreRegionFromNamelessConfig verifies a local eks.yaml may +// only contribute a region when it also names the target it describes. Persisted state authorizes +// the explicit cluster, while its exact eksctl kubeconfig context supplies that cluster's region; +// borrowing the unrelated nameless file's region would redirect the mutation. +func TestStandaloneEKSLifecycleCommandsIgnoreRegionFromNamelessConfig(t *testing.T) { + for _, testCase := range standaloneEKSLifecycleCases() { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "ksail-eks-" + testCase.name + "-nameless-region-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + configureStandaloneEKSNodegroupAction(t, testCase.name) + t.Setenv("HOME", t.TempDir()) + t.Setenv("KSAIL_REGION", "") + + require.NoError( + t, + os.WriteFile( + "eks.yaml", + []byte(`apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + region: eu-west-1 +`), + 0o600, + ), + ) + writeStandaloneEKSKubeconfig(t, clusterName, "ap-southeast-2") + require.NoError(t, state.SaveClusterSpec(clusterName, &v1alpha1.ClusterSpec{ + Distribution: v1alpha1.DistributionEKS, + Provider: v1alpha1.ProviderAWS, + })) + + cmd := testCase.newCommand() + args := make([]string, 0, 4+len(testCase.extraArgs)) + args = append(args, "--name", clusterName, "--provider", "AWS") + args = append(args, testCase.extraArgs...) + cmd.SetArgs(args) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + require.NoError(t, cmd.Execute()) + assert.Equal( + t, + testCase.expectedCalls(clusterName), + readStandaloneEKSCalls(t, markerPath), + ) + assertParentAWSEnvironmentUnchanged(t) + }) + } +} + +// TestStandaloneEKSStartAcceptsPersistedOwnershipWithoutConfig preserves the flag-only standalone +// workflow: when no project files exist, persisted creation state is sufficient local evidence and +// the exact cloud query still corroborates the name, region, and eksctl provenance before scaling. +func TestStandaloneEKSStartAcceptsPersistedOwnershipWithoutConfig(t *testing.T) { + const clusterName = "ksail-eks-start-state-owned-6087" + + workingDir := t.TempDir() + t.Chdir(workingDir) + t.Setenv("HOME", t.TempDir()) + + binDir := t.TempDir() + markerPath := filepath.Join(t.TempDir(), "eksctl-calls") + writeExecutableFixture(t, filepath.Join(binDir, "eksctl"), standaloneEKSEksctlFixture) + + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("KSAIL_EKSCTL_MARKER", markerPath) + t.Setenv("KSAIL_EKS_CLUSTER", clusterName) + t.Setenv("AWS_PROFILE", "selected-profile") + t.Setenv("AWS_REGION", "ap-southeast-2") + t.Setenv("AWS_ACCESS_KEY_ID", "fixture-access") + t.Setenv("AWS_SECRET_ACCESS_KEY", "fixture-secret") + t.Setenv("AWS_SESSION_TOKEN", "fixture-session") + + require.NoError(t, state.SaveClusterSpec(clusterName, &v1alpha1.ClusterSpec{ + Distribution: v1alpha1.DistributionEKS, + Provider: v1alpha1.ProviderAWS, + })) + + cmd := cluster.NewStartCmd() + cmd.SetArgs([]string{"--name", clusterName, "--provider", "AWS"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + require.NoError(t, cmd.Execute()) + assert.Equal( + t, + []string{ + fmt.Sprintf("get cluster --name %s --output json", clusterName), + fmt.Sprintf( + "get nodegroup --cluster %s --output json --region ap-southeast-2", + clusterName, + ), + fmt.Sprintf( + "scale nodegroup --cluster %s --name workers --nodes 2 --nodes-min 2 --nodes-max 4 --region ap-southeast-2", + clusterName, + ), + }, + readStandaloneEKSCalls(t, markerPath), + ) +} + +// TestEKSMutationCommandsRejectNameOverrideMismatch proves create and update cannot claim a name +// that eksctl will ignore because the actual create/delete source is eks.yaml. +// +//nolint:paralleltest // each case changes process environment and working directory +func TestEKSMutationCommandsRejectNameOverrideMismatch(t *testing.T) { + testCases := []struct { + name string + newCommand func() *cobra.Command + }{ + {name: "create", newCommand: cluster.NewCreateCmd}, + {name: "update", newCommand: cluster.NewUpdateCmd}, + } + + //nolint:paralleltest // each case changes process environment and working directory + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + const sourceName = "eks-source-mutation-6087" + + markerPath := setupStandaloneEKSLifecycleFixture(t, sourceName) + + cmd := testCase.newCommand() + cmd.SetArgs([]string{"--name", "different-eks-mutation-6087"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot override EKS config cluster") + assert.Empty(t, readStandaloneEKSCalls(t, markerPath)) + }) + } +} + +// TestEKSMutationCommandsRequireNamedConfig verifies create and update reject a valid-shaped but +// nameless eks.yaml before any provisioner or exact ownership call. Eksctl consumes only that file, +// so KSail cannot safely bind creation, recreation, state, or TTL to a synthetic fallback name. +// +//nolint:paralleltest // setup mutates process environment and the working directory +func TestEKSMutationCommandsRequireNamedConfig(t *testing.T) { + testEKSMutationCommandsRejectConfigSource( + t, + `apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + region: ap-southeast-2 +`, + "EKS config metadata.name is required", + ) +} + +// TestEKSMutationCommandsRejectNonCanonicalConfigName verifies create/update cannot normalize an +// EKS source name for ownership and state while submitting a different raw name to eksctl. That +// split is especially destructive during recreation, where the normalized old target is deleted +// before eksctl consumes the unchanged source. +// +//nolint:paralleltest // setup mutates process environment and the working directory +func TestEKSMutationCommandsRejectNonCanonicalConfigName(t *testing.T) { + testEKSMutationCommandsRejectConfigSource( + t, + `apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + name: "config-file-name " + region: ap-southeast-2 +`, + "invalid EKS config file", + ) +} + +func testEKSMutationCommandsRejectConfigSource(t *testing.T, eksConfig, wantError string) { + t.Helper() + + testCases := []struct { + name string + newCommand func() *cobra.Command + }{ + {name: "create", newCommand: cluster.NewCreateCmd}, + {name: "update", newCommand: cluster.NewUpdateCmd}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + markerPath := setupStandaloneEKSLifecycleFixture(t, "config-file-name") + require.NoError(t, os.WriteFile("eks.yaml", []byte(eksConfig), 0o600)) + + cmd := testCase.newCommand() + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), wantError) + assert.Empty(t, readStandaloneEKSCalls(t, markerPath)) + }) + } +} + +// TestStandaloneEKSLifecycleCommandsUseEKSConfigNameWhenMetadataDrifts verifies the actual source +// eks.yaml name wins over top-level metadata when no --name flag is given, even when region comes +// from its configured environment variable. +// +//nolint:paralleltest // each case changes process environment and working directory +func TestStandaloneEKSLifecycleCommandsUseEKSConfigNameWhenMetadataDrifts(t *testing.T) { + //nolint:paralleltest // each case changes process environment and working directory + for _, testCase := range standaloneEKSLifecycleCases() { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "eks-source-name-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + configureStandaloneEKSNodegroupAction(t, testCase.name) + require.NoError( + t, + os.WriteFile( + "eks.yaml", + []byte(`apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + name: eks-source-name-6087 + region: eu-west-1 +`), + 0o600, + ), + ) + + cmd := testCase.newCommand() + cmd.SetArgs(testCase.extraArgs) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + require.NoError(t, cmd.Execute()) + assert.Equal( + t, + testCase.expectedCalls(clusterName), + readStandaloneEKSCalls(t, markerPath), + ) + assertParentAWSEnvironmentUnchanged(t) + }) + } +} + +// TestStandaloneEKSLifecycleCommandsRejectMalformedPresentConfig verifies a present config load +// error cannot discard custom AWS aliases and fall back to ambient credentials for a destructive +// command. A genuinely absent config remains supported by the standalone flag-only path. +// +//nolint:paralleltest // setup changes process environment and working directory +func TestStandaloneEKSLifecycleCommandsRejectMalformedPresentConfig(t *testing.T) { + //nolint:paralleltest // each case changes process environment and working directory + for _, testCase := range standaloneEKSLifecycleCases() { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "ksail-eks-" + testCase.name + "-malformed-config-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + require.NoError(t, os.WriteFile("ksail.yaml", []byte("spec: [\n"), 0o600)) + + cmd := testCase.newCommand() + args := make([]string, 0, 4+len(testCase.extraArgs)) + args = append(args, "--name", clusterName, "--provider", "AWS") + args = append(args, testCase.extraArgs...) + cmd.SetArgs(args) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "load cluster config") + assert.Empty(t, readStandaloneEKSCalls(t, markerPath)) + assertParentAWSEnvironmentUnchanged(t) + }) + } +} + +// TestEKSUpdateRoutesThroughExactAWSOwnershipContext exercises the real update command through its +// pre-mutation guard. It proves the loaded EKS config, credential aliases, and region reach the AWS +// branch instead of the legacy generic discovery path. +func TestEKSUpdateRoutesThroughExactAWSOwnershipContext(t *testing.T) { + const clusterName = "source-config-eks-update-6087" + + setupStandaloneEKSLifecycleFixture(t, "config-file-name") + t.Setenv("KSAIL_REGION", "ap-southeast-2") + require.NoError( + t, + os.WriteFile( + "eks.yaml", + []byte(`apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + name: source-config-eks-update-6087 + region: eu-west-1 +`), + 0o600, + ), + ) + + var captured *lifecycle.ResolvedClusterInfo + + restore := cluster.ExportSetUpdateUnmanagedGuard( + func(_ context.Context, resolved *lifecycle.ResolvedClusterInfo) error { + captured = resolved + + return cluster.ErrUnmanagedCluster + }, + ) + defer restore() + + cmd := cluster.NewUpdateCmd() + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.ErrorIs(t, err, cluster.ErrUnmanagedCluster) + require.NotNil(t, captured) + assert.Equal(t, clusterName, captured.ClusterName) + assert.Equal(t, clusterName, captured.ConfigClusterName) + assert.Equal(t, v1alpha1.ProviderAWS, captured.Provider) + assert.Equal(t, "ap-southeast-2", captured.AWSRegion) + assert.Equal(t, "KSAIL_PROFILE", captured.AWSOpts.ProfileEnvVar) + assert.Equal(t, "KSAIL_ACCESS", captured.AWSOpts.AccessKeyIDEnvVar) + assert.Equal(t, "KSAIL_SECRET", captured.AWSOpts.SecretAccessKeyEnvVar) + assert.Equal(t, "KSAIL_SESSION", captured.AWSOpts.SessionTokenEnvVar) + + expectedKubeconfig, pathErr := fsutil.EvalCanonicalPath("kubeconfig") + require.NoError(t, pathErr) + assert.Equal(t, expectedKubeconfig, captured.KubeconfigPath) +} + +// TestAutoDeleteEKSUsesCreatedTargetAndCreationRegion verifies TTL cleanup deletes the target +// actually created from eks.yaml, retains its creation-time region even if the environment changes, +// and cleans state for that exact target only. +func TestAutoDeleteEKSUsesCreatedTargetAndCreationRegion(t *testing.T) { + const ( + actualName = "ttl-actual-eks-6087" + staleAlias = "ttl-stale-alias-6087" + ) + + markerPath := setupStandaloneEKSLifecycleFixture(t, actualName) + t.Setenv("HOME", t.TempDir()) + t.Setenv("KSAIL_REGION", "us-east-1") + require.NoError(t, state.SaveClusterSpec(staleAlias, &v1alpha1.ClusterSpec{})) + + clusterCfg := standaloneEKSTTLClusterConfig() + eksConfig := &clusterprovisioner.EKSConfig{ + Name: actualName, + NameFromConfig: true, + Region: "ap-southeast-2", + ConfigPath: "eks.yaml", + KubeconfigPath: "kubeconfig", + } + cmd := &cobra.Command{Use: "ttl-delete"} + + var output bytes.Buffer + + cmd.SetContext(t.Context()) + cmd.SetOut(&output) + cmd.SetErr(&output) + + require.NoError( + t, + cluster.ExportAutoDeleteCluster(cmd, staleAlias, clusterCfg, eksConfig), + ) + assert.Equal( + t, + []string{ + fmt.Sprintf( + "get cluster --name %s --output json --region ap-southeast-2", + actualName, + ), + fmt.Sprintf( + "delete cluster --name %s --region ap-southeast-2 --wait", + actualName, + ), + }, + readStandaloneEKSCalls(t, markerPath), + ) + _, err := state.LoadClusterSpec(actualName) + require.ErrorIs(t, err, state.ErrStateNotFound) + _, err = state.LoadClusterSpec(staleAlias) + require.NoError(t, err) + assert.Contains(t, output.String(), actualName) + assert.NotContains(t, output.String(), staleAlias) + assertParentAWSEnvironmentUnchanged(t) +} + +// TestAutoDeleteEKSFailsClosedWithoutOwnership verifies TTL never constructs a destructive +// provisioner when ownership provenance is absent or the actual EKS target is unavailable. +// +//nolint:paralleltest // subtests mutate process environment and working directory +func TestAutoDeleteEKSFailsClosedWithoutOwnership(t *testing.T) { + //nolint:paralleltest // subtest mutates process environment and working directory + t.Run("unmanaged provenance", testAutoDeleteEKSRejectsUnmanagedProvenance) + //nolint:paralleltest // subtest mutates process environment and working directory + t.Run("missing EKS config", testAutoDeleteEKSRejectsMissingConfig) +} + +func testAutoDeleteEKSRejectsUnmanagedProvenance(t *testing.T) { + const actualName = "ttl-unmanaged-eks-6087" + + markerPath := setupStandaloneEKSLifecycleFixture(t, actualName) + t.Setenv("HOME", t.TempDir()) + t.Setenv("KSAIL_EKSCTL_CREATED", "False") + require.NoError(t, state.SaveClusterSpec(actualName, &v1alpha1.ClusterSpec{})) + + cmd := &cobra.Command{Use: "ttl-delete"} + + var output bytes.Buffer + + cmd.SetContext(t.Context()) + cmd.SetOut(&output) + cmd.SetErr(&output) + + err := cluster.ExportAutoDeleteCluster( + cmd, + "stale-alias", + standaloneEKSTTLClusterConfig(), + &clusterprovisioner.EKSConfig{ + Name: actualName, + NameFromConfig: true, + Region: "ap-southeast-2", + ConfigPath: "eks.yaml", + KubeconfigPath: "kubeconfig", + }, + ) + require.ErrorIs(t, err, cluster.ErrUnmanagedCluster) + assert.Equal( + t, + []string{fmt.Sprintf( + "get cluster --name %s --output json --region ap-southeast-2", + actualName, + )}, + readStandaloneEKSCalls(t, markerPath), + ) + + _, stateErr := state.LoadClusterSpec(actualName) + require.NoError(t, stateErr) + assert.NotContains(t, output.String(), "auto-destroyed") +} + +func testAutoDeleteEKSRejectsMissingConfig(t *testing.T) { + const actualName = "ttl-missing-config-eks-6087" + + markerPath := setupStandaloneEKSLifecycleFixture(t, actualName) + t.Setenv("HOME", t.TempDir()) + require.NoError(t, state.SaveClusterSpec(actualName, &v1alpha1.ClusterSpec{})) + + cmd := &cobra.Command{Use: "ttl-delete"} + + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cluster.ExportAutoDeleteCluster( + cmd, + actualName, + standaloneEKSTTLClusterConfig(), + nil, + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "EKS configuration") + assert.Empty(t, readStandaloneEKSCalls(t, markerPath)) + + _, stateErr := state.LoadClusterSpec(actualName) + require.NoError(t, stateErr) +} + +func standaloneEKSTTLClusterConfig() *v1alpha1.Cluster { + clusterCfg := &v1alpha1.Cluster{} + clusterCfg.Spec.Cluster.Distribution = v1alpha1.DistributionEKS + clusterCfg.Spec.Cluster.Provider = v1alpha1.ProviderAWS + clusterCfg.Spec.Cluster.Connection.Kubeconfig = "kubeconfig" + clusterCfg.Spec.Provider.AWS = v1alpha1.OptionsAWS{ + ProfileEnvVar: "KSAIL_PROFILE", + RegionEnvVar: "KSAIL_REGION", + AccessKeyIDEnvVar: "KSAIL_ACCESS", + SecretAccessKeyEnvVar: "KSAIL_SECRET", + SessionTokenEnvVar: "KSAIL_SESSION", + } + + return clusterCfg +} + +// TestStandaloneEKSLifecycleCommandsFailClosedWhenAWSOwnershipCheckFails +// verifies a failed exact AWS ownership query cannot fall through to a mutating eksctl +// command. The ownership lookup itself is read-only and uses the same resolved +// credential aliases and region as the later lifecycle operation. +func TestStandaloneEKSLifecycleCommandsFailClosedWhenAWSOwnershipCheckFails(t *testing.T) { + for _, testCase := range standaloneEKSLifecycleCases() { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "ksail-eks-" + testCase.name + "-ownership-error-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("KSAIL_EKSCTL_FAIL", "get") + + cmd := testCase.newCommand() + args := make([]string, 0, 4+len(testCase.extraArgs)) + args = append(args, "--name", clusterName, "--provider", "AWS") + args = append(args, testCase.extraArgs...) + cmd.SetArgs(args) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "verify AWS cluster ownership") + assert.Equal( + t, + []string{fmt.Sprintf( + "get cluster --name %s --output json --region ap-southeast-2", + clusterName, + )}, + readStandaloneEKSCalls(t, markerPath), + ) + assertParentAWSEnvironmentUnchanged(t) + }) + } +} + +// TestStandaloneEKSStartRejectsOwnershipRegionMismatch verifies an exact AWS +// lookup that returns the requested name in a different region is a hard stop; +// the command must not continue to nodegroup discovery or scaling. +func TestStandaloneEKSStartRejectsOwnershipRegionMismatch(t *testing.T) { + clusterName := "ksail-eks-start-region-query-mismatch-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("KSAIL_EKS_DISCOVERED_REGION", "us-east-1") + + cmd := cluster.NewStartCmd() + cmd.SetArgs([]string{"--name", clusterName, "--provider", "AWS"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains( + t, + err.Error(), + `exact AWS ownership query returned a different region: got "us-east-1", want "ap-southeast-2"`, + ) + assert.Equal( + t, + []string{fmt.Sprintf( + "get cluster --name %s --output json --region ap-southeast-2", + clusterName, + )}, + readStandaloneEKSCalls(t, markerPath), + ) + assertParentAWSEnvironmentUnchanged(t) +} + +func TestStandaloneEKSStartRejectsOwnershipNameMismatch(t *testing.T) { + clusterName := "ksail-eks-start-name-query-mismatch-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("KSAIL_EKS_DISCOVERED_CLUSTER", "different-visible-cluster") + + cmd := cluster.NewStartCmd() + cmd.SetArgs([]string{"--name", clusterName, "--provider", "AWS"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains( + t, + err.Error(), + `exact AWS ownership query returned a different cluster: got "different-visible-cluster"`, + ) + assert.Equal( + t, + []string{fmt.Sprintf( + "get cluster --name %s --output json --region ap-southeast-2", + clusterName, + )}, + readStandaloneEKSCalls(t, markerPath), + ) + assertParentAWSEnvironmentUnchanged(t) +} + +func TestStandaloneEKSStartUsesUniqueBareContextRegionFallback(t *testing.T) { + clusterName := "ksail-eks-start-context-region-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("KSAIL_REGION", "") + t.Setenv("KSAIL_EKS_DISCOVERED_REGION", "us-west-2") + writeStandaloneEKSEksConfig(t, clusterName) + writeStandaloneEKSKubeconfigContexts( + t, + clusterName, + []string{clusterName + ".us-west-2.eksctl.io"}, + ) + + cmd := cluster.NewStartCmd() + cmd.SetArgs([]string{"--name", clusterName, "--provider", "AWS"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + require.NoError(t, cmd.Execute()) + assert.Equal( + t, + []string{ + fmt.Sprintf( + "get cluster --name %s --output json --region us-west-2", + clusterName, + ), + fmt.Sprintf( + "get nodegroup --cluster %s --output json --region us-west-2", + clusterName, + ), + fmt.Sprintf( + "scale nodegroup --cluster %s --name workers --nodes 2 --nodes-min 2 --nodes-max 4 --region us-west-2", + clusterName, + ), + }, + readStandaloneEKSCalls(t, markerPath), + ) + assertParentAWSEnvironmentUnchanged(t) +} + +func TestStandaloneEKSStartRejectsAmbiguousContextRegions(t *testing.T) { + clusterName := "ksail-eks-start-ambiguous-context-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("KSAIL_REGION", "") + writeStandaloneEKSEksConfig(t, clusterName) + writeStandaloneEKSKubeconfigContexts( + t, + clusterName, + []string{ + "arn:aws:iam::123456789012:role/first@" + clusterName + ".us-east-1.eksctl.io", + clusterName + ".us-west-2.eksctl.io", + }, + ) + + cmd := cluster.NewStartCmd() + cmd.SetArgs([]string{"--name", clusterName, "--provider", "AWS"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "multiple kubeconfig regions") + assert.Empty(t, readStandaloneEKSCalls(t, markerPath)) + assertParentAWSEnvironmentUnchanged(t) +} + +func TestStandaloneEKSStartBindsRegionFromExactQueryWithoutContext(t *testing.T) { + clusterName := "ksail-eks-start-query-region-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("KSAIL_REGION", "") + t.Setenv("KSAIL_EKS_DISCOVERED_REGION", "eu-north-1") + writeStandaloneEKSEksConfig(t, clusterName) + + cmd := cluster.NewStartCmd() + cmd.SetArgs([]string{"--name", clusterName, "--provider", "AWS"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + require.NoError(t, cmd.Execute()) + assert.Equal( + t, + []string{ + fmt.Sprintf("get cluster --name %s --output json", clusterName), + fmt.Sprintf( + "get nodegroup --cluster %s --output json --region eu-north-1", + clusterName, + ), + fmt.Sprintf( + "scale nodegroup --cluster %s --name workers --nodes 2 --nodes-min 2 --nodes-max 4 --region eu-north-1", + clusterName, + ), + }, + readStandaloneEKSCalls(t, markerPath), + ) + assertParentAWSEnvironmentUnchanged(t) +} + +func TestStandaloneEKSStartRejectsEmptyRegionFromExactQuery(t *testing.T) { + clusterName := "ksail-eks-start-empty-query-region-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("KSAIL_REGION", "") + t.Setenv("KSAIL_EKS_DISCOVERED_REGION", "") + writeStandaloneEKSEksConfig(t, clusterName) + + cmd := cluster.NewStartCmd() + cmd.SetArgs([]string{"--name", clusterName, "--provider", "AWS"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "did not report a region") + assert.Equal( + t, + []string{fmt.Sprintf("get cluster --name %s --output json", clusterName)}, + readStandaloneEKSCalls(t, markerPath), + ) + assertParentAWSEnvironmentUnchanged(t) +} + +//nolint:paralleltest // setup mutates process environment and the working directory +func TestStandaloneEKSExplicitRegionWinsContextFallback(t *testing.T) { + clusterName := "ksail-eks-start-explicit-region-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + writeStandaloneEKSKubeconfigContexts( + t, + clusterName, + []string{clusterName + ".us-west-2.eksctl.io"}, + ) + + cmd := cluster.NewStartCmd() + cmd.SetArgs([]string{"--name", clusterName, "--provider", "AWS"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + require.NoError(t, cmd.Execute()) + + for _, call := range readStandaloneEKSCalls(t, markerPath) { + assert.Contains(t, call, "--region ap-southeast-2") + assert.NotContains(t, call, "us-west-2") + } + + assertParentAWSEnvironmentUnchanged(t) +} + +func TestEksctlContextTargetParsing(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + contextName string + wantName string + wantRegion string + wantOK bool + }{ + { + name: "role ARN identity", + contextName: "arn:aws:iam::123456789012:role/operator@demo.eu-west-1.eksctl.io", + wantName: "demo", + wantRegion: "eu-west-1", + wantOK: true, + }, + { + name: "identity containing at signs", + contextName: "user@example.com@demo.us-east-2.eksctl.io", + wantName: "demo", + wantRegion: "us-east-2", + wantOK: true, + }, + { + name: "bare eksctl context", + contextName: "demo.ap-southeast-1.eksctl.io", + wantName: "demo", + wantRegion: "ap-southeast-1", + wantOK: true, + }, + { + name: "similar cluster name remains exact", + contextName: "demo-extra.ap-southeast-1.eksctl.io", + wantName: "demo-extra", + wantRegion: "ap-southeast-1", + wantOK: true, + }, + {name: "missing suffix", contextName: "demo.eu-west-1", wantOK: false}, + {name: "missing region", contextName: "demo.eksctl.io", wantOK: false}, + {name: "malformed dotted region", contextName: "demo.us.east.eksctl.io", wantOK: false}, + {name: "empty target", contextName: ".eksctl.io", wantOK: false}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + name, region, ok := cluster.ExportParseEksctlContextTarget(testCase.contextName) + assert.Equal(t, testCase.wantOK, ok) + assert.Equal(t, testCase.wantName, name) + assert.Equal(t, testCase.wantRegion, region) + }) + } +} + +// TestStandaloneEKSStartRejectsMissingEksctlOwnership verifies the exact AWS +// target is still unmanaged unless eksctl explicitly reports that it created +// the cluster. False, empty, and unknown provenance all stop before scaling. +func TestStandaloneEKSStartRejectsMissingEksctlOwnership(t *testing.T) { + testCases := []struct { + name string + created string + }{ + {name: "false", created: "False"}, + {name: "empty", created: ""}, + {name: "unknown", created: "unknown"}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + clusterName := "ksail-eks-start-" + testCase.name + "-ownership-test-6087" + markerPath := setupStandaloneEKSLifecycleFixture(t, clusterName) + t.Setenv("KSAIL_EKSCTL_CREATED", testCase.created) + + cmd := cluster.NewStartCmd() + cmd.SetArgs([]string{"--name", clusterName, "--provider", "AWS"}) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + require.ErrorIs(t, err, cluster.ErrUnmanagedCluster) + assert.Contains(t, err.Error(), "did not report an eksctl-created cluster") + assert.Equal( + t, + []string{fmt.Sprintf( + "get cluster --name %s --output json --region ap-southeast-2", + clusterName, + )}, + readStandaloneEKSCalls(t, markerPath), + ) + assertParentAWSEnvironmentUnchanged(t) + }) + } +} + +func setupStandaloneEKSLifecycleFixture( + t *testing.T, + clusterName string, +) string { + t.Helper() + + workingDir := t.TempDir() + t.Chdir(workingDir) + + binDir := t.TempDir() + markerPath := filepath.Join(t.TempDir(), "eksctl-calls") + eksctlPath := filepath.Join(binDir, "eksctl") + writeExecutableFixture(t, eksctlPath, standaloneEKSEksctlFixture) + + require.NoError( + t, + os.WriteFile( + filepath.Join(workingDir, "ksail.yaml"), + []byte(standaloneEKSClusterFixture), + 0o600, + ), + ) + eksConfigPath := filepath.Join(workingDir, "eks.yaml") + eksConfig := strings.ReplaceAll(standaloneEKSEksConfigFixture, "config-file-name", clusterName) + require.NoError(t, os.WriteFile(eksConfigPath, []byte(eksConfig), 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_EKS_CLUSTER", clusterName) + t.Setenv("HOME", t.TempDir()) + t.Setenv("KSAIL_PROFILE", "selected-profile") + t.Setenv("KSAIL_REGION", "ap-southeast-2") + 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_REGION", "stale-region") + t.Setenv("AWS_ACCESS_KEY_ID", "stale-access") + t.Setenv("AWS_SECRET_ACCESS_KEY", "stale-secret") + t.Setenv("AWS_SESSION_TOKEN", "stale-session") + + return markerPath +} + +func configureStandaloneEKSNodegroupAction(t *testing.T, action string) { + t.Helper() + + if action == "stop" { + t.Setenv("KSAIL_EKS_NODEGROUP_DESIRED", "2") + t.Setenv("KSAIL_EKS_NODEGROUP_MIN", "2") + } +} + +func runStandaloneEKSCommand( + t *testing.T, + newCommand func() *cobra.Command, + args ...string, +) { + t.Helper() + + cmd := newCommand() + cmd.SetArgs(args) + cmd.SetContext(t.Context()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + require.NoError(t, cmd.Execute()) +} + +func assertParentAWSEnvironmentUnchanged(t *testing.T) { + t.Helper() + + assert.Equal(t, "stale-profile", os.Getenv("AWS_PROFILE")) + assert.Equal(t, "stale-region", os.Getenv("AWS_REGION")) + assert.Equal(t, "stale-access", os.Getenv("AWS_ACCESS_KEY_ID")) + assert.Equal(t, "stale-secret", os.Getenv("AWS_SECRET_ACCESS_KEY")) + assert.Equal(t, "stale-session", os.Getenv("AWS_SESSION_TOKEN")) +} + +func writeStandaloneEKSKubeconfig(t *testing.T, clusterName, region string) { + t.Helper() + + contextName := fmt.Sprintf( + "arn:aws:iam::123456789012:role/ksail-test@%s.%s.eksctl.io", + clusterName, + region, + ) + writeStandaloneEKSKubeconfigContexts(t, clusterName, []string{contextName}) +} + +func writeStandaloneEKSKubeconfigContexts( + t *testing.T, + clusterName string, + contextNames []string, +) { + t.Helper() + + var contexts strings.Builder + for _, contextName := range contextNames { + fmt.Fprintf( + &contexts, + "- name: %s\n context:\n cluster: %s\n user: eksctl-user\n", + contextName, + clusterName, + ) + } + + currentContext := "" + if len(contextNames) > 0 { + currentContext = contextNames[0] + } + + kubeconfig := fmt.Sprintf(`apiVersion: v1 +kind: Config +clusters: +- name: %s + cluster: + server: https://example.invalid +contexts: +%s +current-context: %s +users: +- name: eksctl-user + user: {} +`, clusterName, contexts.String(), currentContext) + + require.NoError(t, os.WriteFile("kubeconfig", []byte(kubeconfig), 0o600)) +} + +func writeStandaloneEKSEksConfig(t *testing.T, clusterName string) { + t.Helper() + + config := fmt.Sprintf(`apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: + name: %s +`, clusterName) + + require.NoError(t, os.WriteFile("eks.yaml", []byte(config), 0o600)) +} + +func readStandaloneEKSCalls(t *testing.T, markerPath string) []string { + t.Helper() + + calls, err := os.ReadFile(markerPath) //nolint:gosec // test-private path + if os.IsNotExist(err) { + return nil + } + + require.NoError(t, err) + + trimmed := strings.TrimSpace(string(calls)) + if trimmed == "" { + return nil + } + + return strings.Split(trimmed, "\n") +} diff --git a/pkg/cli/cmd/cluster/export_test.go b/pkg/cli/cmd/cluster/export_test.go index a1b31ace2c..47c15e4950 100644 --- a/pkg/cli/cmd/cluster/export_test.go +++ b/pkg/cli/cmd/cluster/export_test.go @@ -107,6 +107,11 @@ func ExportEnsureClusterManaged( ) } +// ExportParseEksctlContextTarget exports parseEksctlContextTarget for testing. +func ExportParseEksctlContextTarget(contextName string) (string, string, bool) { + return parseEksctlContextTarget(contextName) +} + // ExportResolveCreatedContextName exports resolveCreatedContextName for testing. func ExportResolveCreatedContextName( distribution v1alpha1.Distribution, @@ -299,7 +304,17 @@ func ExportMaybeWaitForTTL( clusterName string, clusterCfg *v1alpha1.Cluster, ) error { - return maybeWaitForTTL(cmd, clusterName, clusterCfg) + return maybeWaitForTTL(cmd, clusterName, clusterCfg, nil) +} + +// ExportAutoDeleteCluster exports autoDeleteCluster for offline TTL safety tests. +func ExportAutoDeleteCluster( + cmd *cobra.Command, + clusterName string, + clusterCfg *v1alpha1.Cluster, + eksConfig *clusterprovisioner.EKSConfig, +) error { + return autoDeleteCluster(cmd, clusterName, clusterCfg, eksConfig) } // ExportNormalizeVersionTag exposes normalizeVersionTag for testing. @@ -621,6 +636,7 @@ func ExportGuardUpdateTargetManaged( ctx context.Context, clusterCfg *v1alpha1.Cluster, clusterName string, + eksConfig *clusterprovisioner.EKSConfig, ) error { - return guardUpdateTargetManaged(ctx, clusterCfg, clusterName) + return guardUpdateTargetManaged(ctx, clusterCfg, clusterName, eksConfig) } diff --git a/pkg/cli/cmd/cluster/mutation.go b/pkg/cli/cmd/cluster/mutation.go index 7ebf8d8b07..fd121cab89 100644 --- a/pkg/cli/cmd/cluster/mutation.go +++ b/pkg/cli/cmd/cluster/mutation.go @@ -23,10 +23,16 @@ import ( ) var ( - errEKSConfigurationUnavailable = errors.New("EKS configuration is unavailable") - errEKSClusterNameRequired = errors.New("cluster name is required") - errNoMatchingEKSContext = errors.New("no kubeconfig context matches EKS cluster") - errAmbiguousEKSContext = errors.New("multiple kubeconfig contexts match EKS cluster") + errEKSConfigurationUnavailable = errors.New("EKS configuration is unavailable") + errEKSClusterNameRequired = errors.New("cluster name is required") + errEKSMutationConfigNameNeeded = errors.New("EKS config metadata.name is required") + errEKSMutationConfigNameInvalid = errors.New("EKS config metadata.name is invalid") + errEKSNameOverrideMismatch = errors.New("cannot override EKS config cluster name") + errNoMatchingEKSContext = errors.New("no kubeconfig context matches EKS cluster") + errAmbiguousEKSContext = errors.New("multiple kubeconfig contexts match EKS cluster") + errExplicitEKSContextUnobserved = errors.New( + "explicit EKS context was not observed after creation", + ) ) const explicitEKSContextHint = "set spec.cluster.connection.context explicitly" @@ -134,18 +140,12 @@ func loadAndValidateClusterConfig( return nil, "", err } - // Apply cluster name override: --name flag takes priority, then metadata.name - nameOverride := cfgManager.Viper.GetString("name") - if nameOverride == "" { - nameOverride = ctx.ClusterCfg.Name + nameOverride, err := resolveMutationNameOverride(cfgManager, ctx) + if err != nil { + return nil, "", err } if nameOverride != "" { - validationErr := v1alpha1.ValidateClusterName(nameOverride) - if validationErr != nil { - return nil, "", fmt.Errorf("invalid cluster name %q: %w", nameOverride, validationErr) - } - err = applyClusterNameOverride(ctx, nameOverride) if err != nil { return nil, "", err @@ -180,6 +180,105 @@ func loadAndValidateClusterConfig( return ctx, clusterName, nil } +// resolveMutationNameOverride validates the flag/metadata name priority before any distribution +// config is mutated. EKS is the special case: eksctl consumes the name in the on-disk eks.yaml, so +// an explicit contradiction must fail rather than be silently ignored in memory. +func resolveMutationNameOverride( + cfgManager *ksailconfigmanager.ConfigManager, + ctx *localregistry.Context, +) (string, error) { + explicitName := cfgManager.Viper.GetString("name") + if explicitName != "" { + err := validateMutationClusterName(explicitName) + if err != nil { + return "", err + } + + err = validateEKSNameOverride(ctx, explicitName) + if err != nil { + return "", err + } + + return explicitName, nil + } + + metadataName := ctx.ClusterCfg.Name + if metadataName == "" { + return "", nil + } + + return metadataName, validateMutationClusterName(metadataName) +} + +func validateMutationClusterName(name string) error { + err := v1alpha1.ValidateClusterName(name) + if err != nil { + return fmt.Errorf("invalid cluster name %q: %w", name, err) + } + + return nil +} + +// validateEKSMutationConfigSource requires create/update to bind the target to the actual name in +// the eksctl source. Standalone lifecycle commands may instead use persisted KSail state, but an EKS +// create or recreation cannot safely infer the target because the provisioner passes only this file +// to eksctl and deliberately ignores the action's in-memory name argument. +func validateEKSMutationConfigSource(ctx *localregistry.Context) error { + if ctx.ClusterCfg.Spec.Cluster.Distribution != v1alpha1.DistributionEKS { + return nil + } + + if ctx.EKSConfig == nil || strings.TrimSpace(ctx.EKSConfig.ConfigPath) == "" { + return fmt.Errorf( + "%w: provide a named eksctl ClusterConfig source", + errEKSConfigurationUnavailable, + ) + } + + sourceName := ctx.EKSConfig.Name + + canonicalName := strings.TrimSpace(sourceName) + if !ctx.EKSConfig.NameFromConfig || canonicalName == "" { + return fmt.Errorf( + "%w in %s", + errEKSMutationConfigNameNeeded, + ctx.EKSConfig.ConfigPath, + ) + } + + if sourceName != canonicalName { + return fmt.Errorf( + "%w: %q in %s has leading or trailing whitespace", + errEKSMutationConfigNameInvalid, + sourceName, + ctx.EKSConfig.ConfigPath, + ) + } + + err := v1alpha1.ValidateClusterName(sourceName) + if err != nil { + return fmt.Errorf("%w: %w", errEKSMutationConfigNameInvalid, err) + } + + return nil +} + +func validateEKSNameOverride(ctx *localregistry.Context, explicitName string) error { + if ctx.ClusterCfg.Spec.Cluster.Distribution != v1alpha1.DistributionEKS || + ctx.EKSConfig == nil || strings.TrimSpace(ctx.EKSConfig.ConfigPath) == "" || + explicitName == strings.TrimSpace(ctx.EKSConfig.Name) { + return nil + } + + return fmt.Errorf( + "%w: --name %q differs from %q in %s", + errEKSNameOverrideMismatch, + explicitName, + strings.TrimSpace(ctx.EKSConfig.Name), + ctx.EKSConfig.ConfigPath, + ) +} + // runClusterCreationWorkflow performs the full cluster creation workflow. // This is the shared implementation used by both the create handler and // the update command's recreate flow. @@ -327,11 +426,16 @@ func prepareEKSOutputKubeconfigPath(kubeconfigPath string) (string, error) { // prefixes EKS contexts with the AWS identity that created the cluster. func resolvePostCreateContext(ctx *localregistry.Context) error { connection := &ctx.ClusterCfg.Spec.Cluster.Connection + distribution := ctx.ClusterCfg.Spec.Cluster.Distribution + if connection.Context != "" { + if distribution == v1alpha1.DistributionEKS { + return pinObservedExplicitEKSRegion(ctx, connection.Context) + } + return nil } - distribution := ctx.ClusterCfg.Spec.Cluster.Distribution if distribution != v1alpha1.DistributionEKS { connection.Context = resolveCreatedContextName( distribution, @@ -345,6 +449,45 @@ func resolvePostCreateContext(ctx *localregistry.Context) error { return resolveEKSPostCreateContext(ctx) } +// pinObservedExplicitEKSRegion derives a profile-selected region only from the kubeconfig output +// eksctl just made current. Parsing configured text alone could bind TTL cleanup to a stale +// same-named context in another region. +func pinObservedExplicitEKSRegion(ctx *localregistry.Context, contextName string) error { + if ctx.EKSConfig != nil && strings.TrimSpace(ctx.EKSConfig.Region) != "" { + return nil + } + + clusterName, _, config, err := loadEKSContextConfig(ctx) + if err != nil { + return err + } + + observedContext, found := config.Contexts[contextName] + + contextCluster, _, validTarget := parseEksctlContextTarget(contextName) + if !found || observedContext == nil || config.CurrentContext != contextName || + !validTarget || contextCluster != clusterName { + return fmt.Errorf( + "%w: %q for cluster %q", + errExplicitEKSContextUnobserved, + contextName, + clusterName, + ) + } + + pinEKSRegionFromContext(ctx, contextName) + + if strings.TrimSpace(ctx.EKSConfig.Region) == "" { + return fmt.Errorf( + "%w: %q did not supply a region", + errExplicitEKSContextUnobserved, + contextName, + ) + } + + return nil +} + // resolveEKSPostCreateContext selects the identity-qualified context that // eksctl wrote for the configured cluster and region. func resolveEKSPostCreateContext(ctx *localregistry.Context) error { @@ -369,10 +512,27 @@ func resolveEKSPostCreateContext(ctx *localregistry.Context) error { } ctx.ClusterCfg.Spec.Cluster.Connection.Context = selected + pinEKSRegionFromContext(ctx, selected) return nil } +// pinEKSRegionFromContext preserves the exact region eksctl selected after a create whose region was +// delegated to the AWS profile. TTL and later lifecycle calls can then target that created region +// directly even when the kubeconfig also contains a same-named cluster elsewhere. +func pinEKSRegionFromContext(ctx *localregistry.Context, contextName string) { + if ctx.EKSConfig == nil || strings.TrimSpace(ctx.EKSConfig.Region) != "" { + return + } + + clusterName, region, ok := parseEksctlContextTarget(contextName) + if !ok || clusterName != strings.TrimSpace(ctx.EKSConfig.Name) { + return + } + + ctx.EKSConfig.Region = region +} + // loadEKSContextConfig loads the kubeconfig plus the cluster and effective // region needed to select the identity-qualified eksctl context. func loadEKSContextConfig( diff --git a/pkg/cli/cmd/cluster/post_create_context_test.go b/pkg/cli/cmd/cluster/post_create_context_test.go index d4b6f06b25..4debd70762 100644 --- a/pkg/cli/cmd/cluster/post_create_context_test.go +++ b/pkg/cli/cmd/cluster/post_create_context_test.go @@ -153,12 +153,16 @@ func TestResolvePostCreateContext_EKSMissingConfigFailsClosed(t *testing.T) { func TestResolvePostCreateContext_EKSRegionDelegatedToProfile(t *testing.T) { t.Parallel() - const eksctlContext = "arn:aws:iam::123456789012:role/ci@st-eks.eu-west-1.eksctl.io" + const ( + eksctlContext = "arn:aws:iam::123456789012:role/ci@st-eks.eu-west-1.eksctl.io" + otherRegionContext = "arn:aws:iam::123456789012:role/ci@st-eks.us-east-1.eksctl.io" + ) kubeconfigPath := filepath.Join(t.TempDir(), "kubeconfig") config := clientcmdapi.NewConfig() config.CurrentContext = eksctlContext config.Contexts[eksctlContext] = &clientcmdapi.Context{} + config.Contexts[otherRegionContext] = &clientcmdapi.Context{} require.NoError(t, clientcmd.WriteToFile(*config, kubeconfigPath)) ctx := &localregistry.Context{ @@ -177,9 +181,134 @@ func TestResolvePostCreateContext_EKSRegionDelegatedToProfile(t *testing.T) { require.NoError(t, cluster.ExportResolvePostCreateContext(ctx)) assert.Equal(t, eksctlContext, ctx.ClusterCfg.Spec.Cluster.Connection.Context) + assert.Equal(t, "eu-west-1", ctx.EKSConfig.Region) +} + +func TestResolvePostCreateContext_EKSExplicitContextPinsObservedRegion(t *testing.T) { + t.Parallel() + + const explicitContext = "arn:aws:iam::123456789012:role/ci@st-eks.eu-west-1.eksctl.io" + + ctx := newExplicitEKSPostCreateContext( + t, + "st-eks", + explicitContext, + explicitContext, + explicitContext, + ) + + require.NoError(t, cluster.ExportResolvePostCreateContext(ctx)) + assert.Equal(t, explicitContext, ctx.ClusterCfg.Spec.Cluster.Connection.Context) + assert.Equal(t, "eu-west-1", ctx.EKSConfig.Region) +} + +func TestResolvePostCreateContext_EKSRejectsUnobservedExplicitContext(t *testing.T) { + t.Parallel() + + const ( + targetContext = "arn:aws:iam::123456789012:role/ci@st-eks.eu-west-1.eksctl.io" + otherContext = "arn:aws:iam::123456789012:role/ci@st-eks.us-east-1.eksctl.io" + wrongTarget = "arn:aws:iam::123456789012:role/ci@other.eu-west-1.eksctl.io" + ) + + testCases := []struct { + name string + explicit string + current string + contexts []string + }{ + { + name: "missing from output", + explicit: targetContext, + current: otherContext, + contexts: []string{otherContext}, + }, + { + name: "stale non-current context", + explicit: targetContext, + current: otherContext, + contexts: []string{targetContext, otherContext}, + }, + { + name: "wrong cluster", + explicit: wrongTarget, + current: wrongTarget, + contexts: []string{wrongTarget}, + }, + { + name: "malformed context", + explicit: "st-eks.eu.west.eksctl.io", + current: "st-eks.eu.west.eksctl.io", + contexts: []string{"st-eks.eu.west.eksctl.io"}, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + ctx := newExplicitEKSPostCreateContext( + t, + "st-eks", + testCase.explicit, + testCase.current, + testCase.contexts..., + ) + + err := cluster.ExportResolvePostCreateContext(ctx) + require.ErrorContains(t, err, "explicit EKS context was not observed after creation") + assert.Empty(t, ctx.EKSConfig.Region) + }) + } +} + +func TestResolvePostCreateContext_EKSExplicitContextPreservesConfiguredRegion(t *testing.T) { + t.Parallel() + + const explicitContext = "arn:aws:iam::123456789012:role/ci@st-eks.eu-west-1.eksctl.io" + + ctx := &localregistry.Context{ + ClusterCfg: &v1alpha1.Cluster{Spec: v1alpha1.Spec{Cluster: v1alpha1.ClusterSpec{ + Distribution: v1alpha1.DistributionEKS, + Connection: v1alpha1.Connection{Context: explicitContext}, + }}}, + EKSConfig: &clusterprovisioner.EKSConfig{Name: "st-eks", Region: "us-east-1"}, + } + + require.NoError(t, cluster.ExportResolvePostCreateContext(ctx)) + assert.Equal(t, "us-east-1", ctx.EKSConfig.Region) +} + +func newExplicitEKSPostCreateContext( + t *testing.T, + clusterName, explicitContext, currentContext string, + contexts ...string, +) *localregistry.Context { + t.Helper() + + kubeconfigPath := filepath.Join(t.TempDir(), "kubeconfig") + config := clientcmdapi.NewConfig() + config.CurrentContext = currentContext + + for _, contextName := range contexts { + config.Contexts[contextName] = &clientcmdapi.Context{} + } + + require.NoError(t, clientcmd.WriteToFile(*config, kubeconfigPath)) + + return &localregistry.Context{ + ClusterCfg: &v1alpha1.Cluster{Spec: v1alpha1.Spec{Cluster: v1alpha1.ClusterSpec{ + Distribution: v1alpha1.DistributionEKS, + Connection: v1alpha1.Connection{ + Context: explicitContext, + Kubeconfig: kubeconfigPath, + }, + }}}, + EKSConfig: &clusterprovisioner.EKSConfig{Name: clusterName}, + } } -func TestApplyClusterNameOverride_EKSDefersContextResolution(t *testing.T) { +func TestApplyClusterNameOverride_EKSPreservesSourceConfigAndDefersContextResolution(t *testing.T) { t.Parallel() testCases := []struct { @@ -205,9 +334,11 @@ func TestApplyClusterNameOverride_EKSDefersContextResolution(t *testing.T) { }, }, }, + EKSConfig: &clusterprovisioner.EKSConfig{Name: "old-eks-name"}, } require.NoError(t, cluster.ExportApplyClusterNameOverride(ctx, "st-eks")) + assert.Equal(t, "old-eks-name", ctx.EKSConfig.Name) assert.Equal( t, testCase.originalContext, diff --git a/pkg/cli/cmd/cluster/testhelpers_test.go b/pkg/cli/cmd/cluster/testhelpers_test.go index 290292d72c..1e411a953e 100644 --- a/pkg/cli/cmd/cluster/testhelpers_test.go +++ b/pkg/cli/cmd/cluster/testhelpers_test.go @@ -28,9 +28,15 @@ import ( // disableTraefikArg is the K3s argument to disable the built-in Traefik ingress controller. const disableTraefikArg = "--disable=traefik" -type fakeProvisioner struct{} +type fakeProvisioner struct{ createName *string } -func (*fakeProvisioner) Create(context.Context, string) error { return nil } +func (p *fakeProvisioner) Create(_ context.Context, name string) error { + if p.createName != nil { + *p.createName = name + } + + return nil +} func (*fakeProvisioner) Delete(context.Context, string) error { return nil } @@ -44,15 +50,15 @@ func (*fakeProvisioner) List(context.Context) ([]string, error) { func (*fakeProvisioner) Exists(context.Context, string) (bool, error) { return true, nil } -type fakeFactory struct{} +type fakeFactory struct{ createName *string } -func (fakeFactory) Create( +func (f fakeFactory) Create( _ context.Context, _ *v1alpha1.Cluster, ) (clusterprovisioner.Provisioner, any, error) { cfg := &v1alpha4.Cluster{Name: "test"} - return &fakeProvisioner{}, cfg, nil + return &fakeProvisioner{createName: f.createName}, cfg, nil } type fakeInstaller struct{ called bool } diff --git a/pkg/cli/cmd/cluster/ttl.go b/pkg/cli/cmd/cluster/ttl.go index 8e18e5be72..290254e818 100644 --- a/pkg/cli/cmd/cluster/ttl.go +++ b/pkg/cli/cmd/cluster/ttl.go @@ -4,12 +4,15 @@ import ( "context" "fmt" "os/signal" + "strings" "syscall" "time" v1alpha1 "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" + "github.com/devantler-tech/ksail/v7/pkg/cli/lifecycle" "github.com/devantler-tech/ksail/v7/pkg/notify" clusterdetector "github.com/devantler-tech/ksail/v7/pkg/svc/detector/cluster" + clusterprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster" "github.com/devantler-tech/ksail/v7/pkg/svc/state" "github.com/spf13/cobra" ) @@ -25,6 +28,7 @@ func waitForTTLAndDelete( cmd *cobra.Command, clusterName string, clusterCfg *v1alpha1.Cluster, + eksConfig *clusterprovisioner.EKSConfig, ttl time.Duration, ) error { notify.Infof(cmd.OutOrStdout(), @@ -39,7 +43,7 @@ func waitForTTLAndDelete( select { case <-timer.C: - return autoDeleteCluster(cmd, clusterName, clusterCfg) + return autoDeleteCluster(cmd, clusterName, clusterCfg, eksConfig) case <-ctx.Done(): notify.Infof(cmd.OutOrStdout(), "TTL wait cancelled; cluster %q will remain running", clusterName) @@ -55,19 +59,24 @@ func autoDeleteCluster( cmd *cobra.Command, clusterName string, clusterCfg *v1alpha1.Cluster, + eksConfig *clusterprovisioner.EKSConfig, ) error { + clusterName, err := ttlAutoDeleteTargetName(clusterName, clusterCfg, eksConfig) + if err != nil { + return fmt.Errorf("TTL auto-delete: resolve target: %w", err) + } + notify.Infof(cmd.OutOrStdout(), "TTL expired; auto-destroying cluster %q...", clusterName) - info := &clusterdetector.Info{ - ClusterName: clusterName, - Distribution: clusterCfg.Spec.Cluster.Distribution, - Provider: clusterCfg.Spec.Cluster.Provider, + info, options, err := ttlDeleteProvisionerInputs( + cmd.Context(), clusterName, clusterCfg, eksConfig, + ) + if err != nil { + return err } - provisioner, err := createDeleteProvisioner( - info, clusterCfg.Spec.Provider.Omni, clusterCfg.Spec.Provider.Kubernetes, false, - ) + provisioner, err := createDeleteProvisioner(cmd.Context(), info, options) if err != nil { return fmt.Errorf("TTL auto-delete: failed to create provisioner: %w", err) } @@ -93,3 +102,78 @@ func autoDeleteCluster( return nil } + +func ttlDeleteProvisionerInputs( + ctx context.Context, + clusterName string, + clusterCfg *v1alpha1.Cluster, + eksConfig *clusterprovisioner.EKSConfig, +) (*clusterdetector.Info, lifecycle.MinimalProvisionerOptions, error) { + info := &clusterdetector.Info{ + ClusterName: clusterName, + Distribution: clusterCfg.Spec.Cluster.Distribution, + Provider: clusterCfg.Spec.Cluster.Provider, + KubeconfigPath: clusterCfg.Spec.Cluster.Connection.Kubeconfig, + } + options := lifecycle.MinimalProvisionerOptions{ + OmniOpts: clusterCfg.Spec.Provider.Omni, + KubernetesOpts: clusterCfg.Spec.Provider.Kubernetes, + AWSOpts: clusterCfg.Spec.Provider.AWS, + } + + if clusterCfg.Spec.Cluster.Distribution == v1alpha1.DistributionEKS { + if strings.TrimSpace(eksConfig.KubeconfigPath) != "" { + info.KubeconfigPath = strings.TrimSpace(eksConfig.KubeconfigPath) + } + + resolved := &lifecycle.ResolvedClusterInfo{ + ClusterName: clusterName, + ConfigClusterName: clusterName, + EKSConfigSource: eksConfig.NameFromConfig && + strings.TrimSpace(eksConfig.ConfigPath) != "", + Provider: v1alpha1.ProviderAWS, + KubeconfigPath: info.KubeconfigPath, + AWSOpts: clusterCfg.Spec.Provider.AWS, + // EKSConfig.Region was pinned before creation. Do not re-read a mutable region + // environment variable after the TTL wait. + AWSRegion: strings.TrimSpace(eksConfig.Region), + } + + err := ensureAWSClusterManaged(ctx, resolved) + if err != nil { + return nil, lifecycle.MinimalProvisionerOptions{}, fmt.Errorf( + "TTL auto-delete: verify EKS ownership: %w", + err, + ) + } + + eksConfig.Region = resolved.AWSRegion + options.AWSRegion = resolved.AWSRegion + } + + return info, options, nil +} + +// ttlAutoDeleteTargetName returns the exact target the provisioner created. EKS creation consumes +// EKSConfig.Name rather than the lifecycle action's name argument, so TTL cleanup must use that same +// immutable identity and fail closed when it is unavailable. +func ttlAutoDeleteTargetName( + clusterName string, + clusterCfg *v1alpha1.Cluster, + eksConfig *clusterprovisioner.EKSConfig, +) (string, error) { + if clusterCfg == nil || clusterCfg.Spec.Cluster.Distribution != v1alpha1.DistributionEKS { + return clusterName, nil + } + + if eksConfig == nil { + return "", errEKSConfigurationUnavailable + } + + actualName := strings.TrimSpace(eksConfig.Name) + if actualName == "" { + return "", errEKSClusterNameRequired + } + + return actualName, nil +} diff --git a/pkg/cli/cmd/cluster/unmanaged_guard.go b/pkg/cli/cmd/cluster/unmanaged_guard.go index 63d7f70c49..ce3a03cf22 100644 --- a/pkg/cli/cmd/cluster/unmanaged_guard.go +++ b/pkg/cli/cmd/cluster/unmanaged_guard.go @@ -4,12 +4,19 @@ import ( "context" "errors" "fmt" + "os" + "sort" + "strings" v1alpha1 "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" "github.com/devantler-tech/ksail/v7/pkg/cli/lifecycle" + eksctlclient "github.com/devantler-tech/ksail/v7/pkg/client/eksctl" "github.com/devantler-tech/ksail/v7/pkg/fsutil" "github.com/devantler-tech/ksail/v7/pkg/svc/clusterdiscovery" + "github.com/devantler-tech/ksail/v7/pkg/svc/credentials" clusterdetector "github.com/devantler-tech/ksail/v7/pkg/svc/detector/cluster" + clusterprovisioner "github.com/devantler-tech/ksail/v7/pkg/svc/provisioner/cluster" + "github.com/devantler-tech/ksail/v7/pkg/svc/state" ) // ErrUnmanagedCluster indicates a ksail-only lifecycle action (start/stop/…) was attempted against a @@ -18,6 +25,24 @@ import ( // lifecycle actions are refused (ksail#5885, part of the unmanaged-cluster surface epic #5654). var ErrUnmanagedCluster = errors.New("cluster is not managed by ksail") +var ( + errAWSOwnershipNameMismatch = errors.New( + "exact AWS ownership query returned a different cluster", + ) + errAWSOwnershipRegionMismatch = errors.New( + "exact AWS ownership query returned a different region", + ) + errAWSOwnershipRegionMissing = errors.New( + "exact AWS ownership query did not report a region", + ) + errAWSOwnershipRegionAmbiguous = errors.New( + "multiple kubeconfig regions match EKS cluster", + ) + errAWSOwnershipProvenanceMissing = errors.New( + "exact AWS ownership query did not report an eksctl-created cluster", + ) +) + // managedClusterLister enumerates the names of clusters ksail manages across every provider and // reports whether discovery was complete (no provider failed). The guard fails open when discovery // is incomplete so a transient provider error (e.g. a stopped Docker daemon) never wrongly refuses a @@ -84,13 +109,36 @@ func ensureClusterManaged( // context↔name mapping stays defined in exactly one place. Best-effort: a missing/unreadable // kubeconfig yields false (treated as "not an unmanaged cluster, just absent"). func kubeconfigHasClusterContext(kubeconfigPath, clusterName string) bool { - config := clusterdiscovery.LoadKubeconfig(kubeconfigPath) + return kubeconfigHasResolvedClusterContext(&lifecycle.ResolvedClusterInfo{ + ClusterName: clusterName, + KubeconfigPath: kubeconfigPath, + }) +} + +// kubeconfigHasResolvedClusterContext extends the shared prefix-based context mapping with the +// suffix-shaped eksctl context formats: @..eksctl.io and +// ..eksctl.io. +func kubeconfigHasResolvedClusterContext(resolved *lifecycle.ResolvedClusterInfo) bool { + config := clusterdiscovery.LoadKubeconfig(resolved.KubeconfigPath) if config == nil { return false } - matchesClusterName := func(name string) bool { return name == clusterName } + matchesClusterName := func(name string) bool { return name == resolved.ClusterName } + for contextName := range config.Contexts { + if resolved.Provider == v1alpha1.ProviderAWS { + if eksctlContextMatchesCluster( + contextName, + resolved.ClusterName, + resolved.AWSRegion, + ) { + return true + } + + continue + } + if clusterdiscovery.ContextIsManaged(contextName, matchesClusterName) { return true } @@ -99,9 +147,253 @@ func kubeconfigHasClusterContext(kubeconfigPath, clusterName string) bool { return false } -// unmanagedClusterGuard is the SimpleLifecycleConfig.Guard shared by the start and stop commands: it -// rejects an action on a cluster ksail did not provision, using the real cross-provider discoverer. +// parseEksctlContextTarget recognizes both identity-qualified and bare eksctl kubeconfig contexts. +// Parsing from the last @ keeps IAM identities opaque; parsing the target at its last dot avoids +// prefix matches between different cluster names. +func parseEksctlContextTarget(contextName string) (string, string, bool) { + target := contextName + if identityEnd := strings.LastIndex(target, "@"); identityEnd >= 0 { + target = target[identityEnd+1:] + } + + target, found := strings.CutSuffix(target, ".eksctl.io") + if !found { + return "", "", false + } + + regionStart := strings.LastIndex(target, ".") + if regionStart <= 0 || regionStart == len(target)-1 { + return "", "", false + } + + clusterName := target[:regionStart] + region := target[regionStart+1:] + + if strings.Contains(clusterName, ".") || strings.Contains(region, ".") { + return "", "", false + } + + return clusterName, region, true +} + +// eksctlContextMatchesCluster recognizes eksctl's real kubeconfig context shape without treating a +// mere substring as ownership proof. When the resolved region is known it must match exactly. +func eksctlContextMatchesCluster(contextName, clusterName, region string) bool { + contextCluster, contextRegion, ok := parseEksctlContextTarget(contextName) + if !ok || contextCluster != clusterName { + return false + } + + return region == "" || contextRegion == region +} + +// bindAWSRegionFromKubeconfig uses a context region only as a fallback when neither the configured +// environment variable nor eks.yaml supplied one. More than one exact-name region is ambiguous and +// must stop before even the read-only ownership query chooses an AWS profile default. +func bindAWSRegionFromKubeconfig(resolved *lifecycle.ResolvedClusterInfo) error { + if resolved.AWSRegion != "" { + return nil + } + + config := clusterdiscovery.LoadKubeconfig(resolved.KubeconfigPath) + if config == nil { + return nil + } + + regions := make(map[string]struct{}) + + for contextName := range config.Contexts { + clusterName, region, ok := parseEksctlContextTarget(contextName) + if ok && clusterName == resolved.ClusterName { + regions[region] = struct{}{} + } + } + + if len(regions) == 0 { + return nil + } + + if len(regions) == 1 { + for region := range regions { + resolved.AWSRegion = region + } + + return nil + } + + regionNames := make([]string, 0, len(regions)) + for region := range regions { + regionNames = append(regionNames, region) + } + + sort.Strings(regionNames) + + return fmt.Errorf( + "%w %q (%s); set an explicit AWS region", + errAWSOwnershipRegionAmbiguous, + resolved.ClusterName, + strings.Join(regionNames, ", "), + ) +} + +// ensureAWSClusterManaged queries the exact name and region through eksctl under the resolved AWS +// credential aliases. Unlike broad discovery (which may silently skip an unavailable provider), +// every tool, credential, command, and parse error fails closed before a destructive operation. +func ensureAWSClusterManaged( + ctx context.Context, + resolved *lifecycle.ResolvedClusterInfo, +) error { + if !hasLocalKSailEKSTargetEvidence(resolved) { + return fmt.Errorf( + "no local KSail ownership evidence for EKS target %q: %w", + resolved.ClusterName, + unmanagedClusterError(resolved.ClusterName), + ) + } + + err := bindAWSRegionFromKubeconfig(resolved) + if err != nil { + return err + } + + auth := credentials.ResolveAWS(credentials.NewAWSOptionsResolver(resolved.AWSOpts)) + eksctlOptions := credentials.OptionsForAWSChildEnvironment( + auth, + os.Environ(), + eksctlclient.WithEnvironment, + eksctlclient.RequireCredentialValues, + ) + + summary, err := eksctlclient.NewClient(eksctlOptions...).GetCluster( + ctx, + resolved.ClusterName, + resolved.AWSRegion, + ) + if err != nil { + if errors.Is(err, eksctlclient.ErrClusterNotFound) { + return awsOwnershipMismatchError(resolved, err) + } + + return awsOwnershipQueryError(resolved, err) + } + + return validateAWSOwnershipSummary(resolved, summary) +} + +// hasLocalKSailEKSTargetEvidence requires local KSail intent before the cloud-side eksctl marker is +// consulted. Matching an actual loaded eks.yaml authorizes config-backed commands; persisted EKS/AWS +// creation state preserves standalone --name/--provider operation when project files are absent. A +// kubeconfig context or EksctlCreated=True alone never qualifies. These name-based records do not +// bind an immutable AWS account/cluster instance; ksail#6202 tracks that separate hardening. +func hasLocalKSailEKSTargetEvidence(resolved *lifecycle.ResolvedClusterInfo) bool { + if resolved.EKSConfigSource && strings.TrimSpace(resolved.ConfigClusterName) != "" && + strings.TrimSpace(resolved.ConfigClusterName) == strings.TrimSpace(resolved.ClusterName) { + return true + } + + spec, err := state.LoadClusterSpec(resolved.ClusterName) + if err != nil { + return false + } + + return spec.Distribution == v1alpha1.DistributionEKS && spec.Provider == v1alpha1.ProviderAWS +} + +// validateAWSOwnershipSummary requires the exact query to corroborate name, region, and eksctl +// provenance. Every mismatch is a pre-mutation blocker. +func validateAWSOwnershipSummary( + resolved *lifecycle.ResolvedClusterInfo, + summary *eksctlclient.ClusterSummary, +) error { + if summary.Name != resolved.ClusterName { + return awsOwnershipMismatchError( + resolved, + fmt.Errorf( + "%w: got %q, want %q", + errAWSOwnershipNameMismatch, + summary.Name, + resolved.ClusterName, + ), + ) + } + + if resolved.AWSRegion != "" && summary.Region != resolved.AWSRegion { + return awsOwnershipMismatchError( + resolved, + fmt.Errorf( + "%w: got %q, want %q", + errAWSOwnershipRegionMismatch, + summary.Region, + resolved.AWSRegion, + ), + ) + } + + if resolved.AWSRegion == "" && strings.TrimSpace(summary.Region) == "" { + return awsOwnershipQueryError(resolved, errAWSOwnershipRegionMissing) + } + + if !strings.EqualFold(strings.TrimSpace(summary.EKSCTLCreated), "true") { + return fmt.Errorf( + "AWS ownership query returned an unmanaged target: %w", + errors.Join( + fmt.Errorf( + "%w: got EksctlCreated=%q", + errAWSOwnershipProvenanceMissing, + summary.EKSCTLCreated, + ), + unmanagedClusterError(resolved.ClusterName), + ), + ) + } + + if resolved.AWSRegion == "" { + resolved.AWSRegion = summary.Region + } + + return nil +} + +// awsOwnershipMismatchError classifies an absent/mismatched exact result as unmanaged only when a +// strictly parsed eksctl context corroborates the requested name and region. A context identity is +// never treated as ownership proof. +func awsOwnershipMismatchError(resolved *lifecycle.ResolvedClusterInfo, cause error) error { + if kubeconfigHasResolvedClusterContext(resolved) { + return fmt.Errorf( + "AWS ownership query did not return the kubeconfig target: %w", + errors.Join(cause, unmanagedClusterError(resolved.ClusterName)), + ) + } + + return awsOwnershipQueryError(resolved, cause) +} + +// awsOwnershipQueryError preserves tool, credential, command, and parse failures as fail-closed +// diagnostics. Those failures do not prove a kubeconfig target is unmanaged; they only prove the +// destructive command cannot establish ownership safely. +func awsOwnershipQueryError(resolved *lifecycle.ResolvedClusterInfo, cause error) error { + return fmt.Errorf( + "verify AWS cluster ownership for %q: %w", + resolved.ClusterName, + cause, + ) +} + +func unmanagedClusterError(clusterName string) error { + return fmt.Errorf( + "%q is an unmanaged cluster: %w; read-only operations (list, resource browsing, logs, exec) still work", + clusterName, + ErrUnmanagedCluster, + ) +} + +// unmanagedClusterGuard is the SimpleLifecycleConfig.Guard shared by start and stop. AWS uses the +// exact fail-closed ownership query; other providers retain the existing cross-provider guard. func unmanagedClusterGuard(ctx context.Context, resolved *lifecycle.ResolvedClusterInfo) error { + if resolved.Provider == v1alpha1.ProviderAWS { + return ensureAWSClusterManaged(ctx, resolved) + } + return ensureClusterManaged(ctx, resolved, discoverManagedClusters) } @@ -122,6 +414,7 @@ func guardUpdateTargetManaged( ctx context.Context, clusterCfg *v1alpha1.Cluster, clusterName string, + eksConfig *clusterprovisioner.EKSConfig, ) error { kubeconfigPath, err := clusterdetector.ResolveKubeconfigPath( clusterCfg.Spec.Cluster.Connection.Kubeconfig, @@ -138,8 +431,59 @@ func guardUpdateTargetManaged( return fmt.Errorf("canonicalize kubeconfig path %q: %w", kubeconfigPath, err) } - return updateUnmanagedGuardFunc(ctx, &lifecycle.ResolvedClusterInfo{ + resolved := resolveUpdateTarget(clusterCfg, clusterName, canonical, eksConfig) + + err = lifecycle.ValidateStandaloneAWSTarget(resolved) + if err != nil { + return fmt.Errorf("validate AWS update target: %w", err) + } + + err = updateUnmanagedGuardFunc(ctx, resolved) + if err != nil { + return err + } + + // A regionless eks.yaml may be safely bound by an exact kubeconfig context or the exact eksctl + // query. Preserve that verified region so the subsequent update/recreate cannot fall back to a + // different ambient/default AWS region. + if eksConfig != nil && strings.TrimSpace(resolved.AWSRegion) != "" { + eksConfig.Region = strings.TrimSpace(resolved.AWSRegion) + } + + return nil +} + +func resolveUpdateTarget( + clusterCfg *v1alpha1.Cluster, + clusterName, canonicalKubeconfig string, + eksConfig *clusterprovisioner.EKSConfig, +) *lifecycle.ResolvedClusterInfo { + resolved := &lifecycle.ResolvedClusterInfo{ ClusterName: clusterName, - KubeconfigPath: canonical, - }) + Provider: clusterCfg.Spec.Cluster.Provider, + KubeconfigPath: canonicalKubeconfig, + AWSOpts: clusterCfg.Spec.Provider.AWS, + } + if eksConfig == nil { + return resolved + } + + configuredAWSRegion := strings.TrimSpace(eksConfig.Region) + effectiveAWSRegion := lifecycle.ResolveAWSRegion( + clusterCfg.Spec.Provider.AWS, + &clusterprovisioner.DistributionConfig{EKS: eksConfig}, + ) + + // The exact verified name/region drives both in-place operations and recreation deletion. Pin it + // into the distribution config before either later path constructs its provisioner. + eksConfig.Region = effectiveAWSRegion + resolved.ConfigClusterName = strings.TrimSpace(eksConfig.Name) + resolved.EKSConfigSource = eksConfig.NameFromConfig && + strings.TrimSpace(eksConfig.ConfigPath) != "" && + resolved.ConfigClusterName != "" + resolved.AWSRegion = effectiveAWSRegion + resolved.AWSRegionFromConfig = effectiveAWSRegion != "" && + effectiveAWSRegion == configuredAWSRegion + + return resolved } diff --git a/pkg/cli/cmd/cluster/unmanaged_guard_test.go b/pkg/cli/cmd/cluster/unmanaged_guard_test.go index 30957c6416..fd099c9330 100644 --- a/pkg/cli/cmd/cluster/unmanaged_guard_test.go +++ b/pkg/cli/cmd/cluster/unmanaged_guard_test.go @@ -8,6 +8,7 @@ import ( "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" "github.com/devantler-tech/ksail/v7/pkg/cli/cmd/cluster" "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" ) @@ -153,7 +154,9 @@ func TestGuardUpdateTargetManaged_UnmanagedRejected(t *testing.T) { clusterCfg := &v1alpha1.Cluster{} clusterCfg.Spec.Cluster.Connection.Kubeconfig = kubeconfigPath - err := cluster.ExportGuardUpdateTargetManaged(context.Background(), clusterCfg, "my-cluster") + err := cluster.ExportGuardUpdateTargetManaged( + context.Background(), clusterCfg, "my-cluster", nil, + ) require.ErrorIs(t, err, cluster.ErrUnmanagedCluster) require.Contains(t, err.Error(), "my-cluster") @@ -180,6 +183,91 @@ func TestGuardUpdateTargetManaged_ManagedAllows(t *testing.T) { require.NoError( t, - cluster.ExportGuardUpdateTargetManaged(context.Background(), clusterCfg, "my-cluster"), + cluster.ExportGuardUpdateTargetManaged( + context.Background(), clusterCfg, "my-cluster", nil, + ), + ) +} + +// TestGuardUpdateTargetManaged_EKSPreservesAWSOwnershipContext verifies update reaches the exact +// fail-closed AWS guard rather than the legacy cross-provider discovery path. The credential aliases +// and explicit region used by the later EKS update must be identical at the ownership boundary. +func TestGuardUpdateTargetManaged_EKSPreservesAWSOwnershipContext(t *testing.T) { + t.Setenv("KSAIL_UPDATE_REGION", "") + + clusterCfg := &v1alpha1.Cluster{} + clusterCfg.Spec.Cluster.Distribution = v1alpha1.DistributionEKS + clusterCfg.Spec.Cluster.Provider = v1alpha1.ProviderAWS + clusterCfg.Spec.Cluster.Connection.Kubeconfig = writeKubeconfigWithContext( + t, + t.TempDir(), + "update-eks.eu-north-1.eksctl.io", + ) + //nolint:gosec // these strings name test-only environment variables, not credentials + clusterCfg.Spec.Provider.AWS = v1alpha1.OptionsAWS{ + ProfileEnvVar: "KSAIL_UPDATE_PROFILE", + RegionEnvVar: "KSAIL_UPDATE_REGION", + AccessKeyIDEnvVar: "KSAIL_UPDATE_ACCESS", + SecretAccessKeyEnvVar: "KSAIL_UPDATE_SECRET", + SessionTokenEnvVar: "KSAIL_UPDATE_SESSION", + } + eksConfig := &clusterprovisioner.EKSConfig{ + Name: "update-eks", + NameFromConfig: true, + Region: "eu-north-1", + } + + var captured *lifecycle.ResolvedClusterInfo + + restore := cluster.ExportSetUpdateUnmanagedGuard( + func(_ context.Context, resolved *lifecycle.ResolvedClusterInfo) error { + captured = resolved + + return nil + }, + ) + defer restore() + + require.NoError( + t, + cluster.ExportGuardUpdateTargetManaged( + context.Background(), clusterCfg, "update-eks", eksConfig, + ), + ) + require.NotNil(t, captured) + assert.Equal(t, v1alpha1.ProviderAWS, captured.Provider) + assert.Equal(t, "eu-north-1", captured.AWSRegion) + assert.Equal(t, clusterCfg.Spec.Provider.AWS, captured.AWSOpts) +} + +// TestGuardUpdateTargetManaged_EKSPinsRegionBoundByOwnershipGuard verifies a region discovered by +// the exact guard is copied into the distribution config consumed by the later update/recreate +// provisioner. Validation must never bind a region and then discard it before mutation. +func TestGuardUpdateTargetManaged_EKSPinsRegionBoundByOwnershipGuard(t *testing.T) { + t.Setenv("KSAIL_UPDATE_REGION", "") + + clusterCfg := &v1alpha1.Cluster{} + clusterCfg.Spec.Cluster.Distribution = v1alpha1.DistributionEKS + clusterCfg.Spec.Cluster.Provider = v1alpha1.ProviderAWS + clusterCfg.Spec.Cluster.Connection.Kubeconfig = filepath.Join(t.TempDir(), "missing-kubeconfig") + clusterCfg.Spec.Provider.AWS.RegionEnvVar = "KSAIL_UPDATE_REGION" + eksConfig := &clusterprovisioner.EKSConfig{Name: "update-eks"} + + restore := cluster.ExportSetUpdateUnmanagedGuard( + func(_ context.Context, resolved *lifecycle.ResolvedClusterInfo) error { + assert.Empty(t, resolved.AWSRegion) + resolved.AWSRegion = "eu-north-1" + + return nil + }, + ) + defer restore() + + require.NoError( + t, + cluster.ExportGuardUpdateTargetManaged( + context.Background(), clusterCfg, "update-eks", eksConfig, + ), ) + assert.Equal(t, "eu-north-1", eksConfig.Region) } diff --git a/pkg/cli/cmd/cluster/update.go b/pkg/cli/cmd/cluster/update.go index 299b18b893..6a300b3d94 100644 --- a/pkg/cli/cmd/cluster/update.go +++ b/pkg/cli/cmd/cluster/update.go @@ -142,11 +142,16 @@ func handleUpdateRunE( return err } + err = validateEKSMutationConfigSource(ctx) + if err != nil { + return err + } + // Refuse to reconcile configuration to a cluster ksail did not provision. When the target is an // unmanaged cluster (a managed cloud cluster, a kubeadm cluster, a colleague's cluster) the guard // rejects here — before any change is computed or applied — so `cluster update` never mutates a // cluster ksail does not own. Read-only operations still work. (ksail#5885, epic #5654.) - err = guardUpdateTargetManaged(cmd.Context(), ctx.ClusterCfg, clusterName) + err = guardUpdateTargetManaged(cmd.Context(), ctx.ClusterCfg, clusterName, ctx.EKSConfig) if err != nil { return err } diff --git a/pkg/cli/lifecycle/awsregion_test.go b/pkg/cli/lifecycle/awsregion_test.go index b2ecd409cf..c7cae443f1 100644 --- a/pkg/cli/lifecycle/awsregion_test.go +++ b/pkg/cli/lifecycle/awsregion_test.go @@ -18,7 +18,11 @@ import ( // so it must not run in parallel. func TestResolveAWSRegion(t *testing.T) { eksDistCfg := &clusterprovisioner.DistributionConfig{ - EKS: &clusterprovisioner.EKSConfig{Region: "eu-west-1"}, + EKS: &clusterprovisioner.EKSConfig{ + Name: "configured-eks", + NameFromConfig: true, + Region: "eu-west-1", + }, } t.Run("env var overrides eks.yaml", func(t *testing.T) { @@ -50,6 +54,16 @@ func TestResolveAWSRegion(t *testing.T) { got := lifecycle.ResolveAWSRegion(v1alpha1.OptionsAWS{}, nil) assert.Empty(t, got) }) + + t.Run("ignores region from nameless eks config", func(t *testing.T) { + t.Setenv("AWS_REGION", "") + + namelessConfig := &clusterprovisioner.DistributionConfig{ + EKS: &clusterprovisioner.EKSConfig{Region: "eu-west-1"}, + } + got := lifecycle.ResolveAWSRegion(v1alpha1.OptionsAWS{}, namelessConfig) + assert.Empty(t, got) + }) } // TestResolveClusterInfoRetainsAWSCredentialMappings verifies lifecycle diff --git a/pkg/cli/lifecycle/simple.go b/pkg/cli/lifecycle/simple.go index 34f54f4f35..4b35412a77 100644 --- a/pkg/cli/lifecycle/simple.go +++ b/pkg/cli/lifecycle/simple.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "log/slog" "os" "strings" @@ -32,6 +33,9 @@ var ( ErrClusterNameRequired = errors.New( "cluster name is required: use --name flag, create a ksail.yaml config, or set a kubeconfig context", ) + // ErrAWSTargetConfigMismatch indicates that a resolved EKS target would inherit the region + // from a local EKS config describing a different cluster. + ErrAWSTargetConfigMismatch = errors.New("resolved EKS target does not match local config") ) const ( @@ -111,16 +115,23 @@ func BindNameAndProviderFlags( // ResolvedClusterInfo contains the resolved cluster name, provider, and kubeconfig path. type ResolvedClusterInfo struct { - ClusterName string - Provider v1alpha1.Provider - KubeconfigPath string - OmniOpts v1alpha1.OptionsOmni - KubernetesOpts v1alpha1.OptionsKubernetes - // AWSRegion is the resolved AWS region for read-only EKS status lookups. + ClusterName string + ConfigClusterName string + // EKSConfigSource reports that ConfigClusterName came from an actual loaded eks.yaml, rather + // than another distribution config or a synthetic fallback. + EKSConfigSource bool + Provider v1alpha1.Provider + KubeconfigPath string + OmniOpts v1alpha1.OptionsOmni + KubernetesOpts v1alpha1.OptionsKubernetes + // AWSRegion is the resolved AWS region for EKS operations. // 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. + // AWSRegionFromConfig reports that AWSRegion came from the loaded EKS config rather than the + // configured region environment variable. Mutating standalone commands use this provenance to + // avoid applying one config's region to a different resolved target. + AWSRegionFromConfig bool + // AWSOpts retains the credential environment-variable mappings from the loaded cluster config. AWSOpts v1alpha1.OptionsAWS } @@ -138,20 +149,33 @@ func ResolveAWSRegion( awsOpts v1alpha1.OptionsAWS, distCfg *clusterprovisioner.DistributionConfig, ) string { + region, _ := resolveAWSRegion(awsOpts, distCfg) + + return region +} + +// resolveAWSRegion returns the resolved region together with whether it came from eks.yaml. +func resolveAWSRegion( + awsOpts v1alpha1.OptionsAWS, + distCfg *clusterprovisioner.DistributionConfig, +) (string, bool) { envVar := awsOpts.RegionEnvVar if envVar == "" { envVar = awsRegionEnvVarDefault } if region := os.Getenv(envVar); region != "" { - return region + return region, false } - if distCfg != nil && distCfg.EKS != nil { - return distCfg.EKS.Region + // A declarative region is safe to reuse only when the same actual eks.yaml also named the + // cluster it describes. ConfigManager can load a nameless file and synthesize a target name; + // treating that file's region as target-bound could redirect a standalone mutation. + if distCfg != nil && distCfg.EKS != nil && distCfg.EKS.NameFromConfig { + return distCfg.EKS.Region, distCfg.EKS.Region != "" } - return "" + return "", false } // ResolveClusterInfo resolves the cluster name, provider, and kubeconfig from flags, config, or kubeconfig. @@ -160,11 +184,40 @@ func ResolveAWSRegion( // Priority for kubeconfig: flag > env (KUBECONFIG) > config > default (~/.kube/config). // // When cmd is non-nil, the --config persistent flag is honored for config loading. +// +// A config that is present but unreadable is tolerated: resolution falls back to flags and the +// kubeconfig context so read-only commands (cluster info, diagnose, connect) keep working while +// local project configuration is broken. Callers that are about to MUTATE a cluster must use +// ResolveClusterInfoStrict instead, so a malformed config can never silently drop credential +// aliases or region bindings ahead of a destructive operation. func ResolveClusterInfo( cmd *cobra.Command, nameFlag string, providerFlag v1alpha1.Provider, kubeconfigFlag string, +) (*ResolvedClusterInfo, error) { + return resolveClusterInfo(cmd, nameFlag, providerFlag, kubeconfigFlag, false) +} + +// ResolveClusterInfoStrict resolves cluster info like ResolveClusterInfo but fails closed when a +// present config cannot be loaded. Mutating lifecycle commands use it so a malformed ksail.yaml or +// distribution config aborts before any cluster is changed, rather than proceeding on partially +// resolved identity. +func ResolveClusterInfoStrict( + cmd *cobra.Command, + nameFlag string, + providerFlag v1alpha1.Provider, + kubeconfigFlag string, +) (*ResolvedClusterInfo, error) { + return resolveClusterInfo(cmd, nameFlag, providerFlag, kubeconfigFlag, true) +} + +func resolveClusterInfo( + cmd *cobra.Command, + nameFlag string, + providerFlag v1alpha1.Provider, + kubeconfigFlag string, + strict bool, ) (*ResolvedClusterInfo, error) { resolved := ResolvedClusterInfo{ ClusterName: nameFlag, @@ -172,9 +225,19 @@ func ResolveClusterInfo( KubeconfigPath: kubeconfigFlag, } - // 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) + // Always load config to fill missing fields and extract provider options: even when --name is + // provided, provider-specific settings are still needed. + err := resolveFromConfig(cmd, &resolved) + if err != nil { + if strict { + return nil, err + } + + slog.Warn( + "ignoring unreadable configuration; resolving from flags and kubeconfig instead", + "error", err, + ) + } // Fall back to kubeconfig context detection if resolved.ClusterName == "" { @@ -204,26 +267,36 @@ func ResolveClusterInfo( return &resolved, nil } -// loadConfig loads the ksail.yaml config, honoring the --config flag when cmd is non-nil. -// Returns nil if no config is found or loading fails. -func loadConfig(cmd *cobra.Command) (*v1alpha1.Cluster, *clusterprovisioner.DistributionConfig) { +// loadConfig loads the ksail.yaml config, honoring the --config flag when cmd is non-nil. A +// genuinely absent default config is a supported standalone-command case; a present, explicit, or +// discovered config that cannot be loaded is returned as an error so destructive callers fail +// closed instead of falling back to ambient provider credentials. +func loadConfig( + cmd *cobra.Command, +) (*v1alpha1.Cluster, *clusterprovisioner.DistributionConfig, error) { var configFile string if cmd != nil { cfgPath, err := flags.GetConfigPath(cmd) - if err == nil { - configFile = cfgPath + if err != nil { + return nil, nil, fmt.Errorf("resolve config path: %w", err) } + + configFile = cfgPath } cfgManager := ksailconfigmanager.NewConfigManager(nil, configFile) cfg, err := cfgManager.Load(configmanager.LoadOptions{Silent: true, SkipValidation: true}) - if err != nil || cfg == nil || !cfgManager.IsConfigFileFound() { - return nil, nil + if err != nil { + return nil, nil, fmt.Errorf("load cluster config: %w", err) + } + + if cfg == nil || !cfgManager.IsConfigFileFound() { + return nil, nil, nil } - return cfg, cfgManager.DistributionConfig + return cfg, cfgManager.DistributionConfig, nil } // resolveFromConfig fills missing cluster info and provider options from the ksail.yaml config. @@ -232,21 +305,43 @@ func loadConfig(cmd *cobra.Command) (*v1alpha1.Cluster, *clusterprovisioner.Dist func resolveFromConfig( cmd *cobra.Command, resolved *ResolvedClusterInfo, -) { - cfg, distCfg := loadConfig(cmd) - if cfg == nil { - return +) error { + cfg, distCfg, err := loadConfig(cmd) + if err != nil { + return err } - if resolved.ClusterName == "" && cfg.Name != "" { - if v1alpha1.ValidateClusterName(cfg.Name) == nil { - resolved.ClusterName = cfg.Name - } + if cfg == nil { + return nil } - if resolved.ClusterName == "" { - resolved.ClusterName = ClusterNameFromDistributionConfig(distCfg) - } + resolveClusterIdentityFromConfig(resolved, cfg, distCfg) + + resolved.OmniOpts = cfg.Spec.Provider.Omni + resolved.KubernetesOpts = cfg.Spec.Provider.Kubernetes + resolved.AWSOpts = cfg.Spec.Provider.AWS + resolved.AWSRegion, resolved.AWSRegionFromConfig = resolveAWSRegion( + cfg.Spec.Provider.AWS, + distCfg, + ) + + return nil +} + +func resolveClusterIdentityFromConfig( + resolved *ResolvedClusterInfo, + cfg *v1alpha1.Cluster, + distCfg *clusterprovisioner.DistributionConfig, +) { + resolved.ConfigClusterName = resolveConfigClusterName(cfg, distCfg) + resolved.EKSConfigSource = hasLoadedEKSConfigSource(cfg, distCfg, resolved.ConfigClusterName) + resolved.ClusterName = resolveClusterNameFromConfig( + resolved.ClusterName, + resolved.ConfigClusterName, + resolved.EKSConfigSource, + cfg, + distCfg, + ) if resolved.Provider == "" && cfg.Spec.Cluster.Provider != "" { resolved.Provider = cfg.Spec.Cluster.Provider @@ -255,11 +350,95 @@ func resolveFromConfig( if resolved.KubeconfigPath == "" && cfg.Spec.Cluster.Connection.Kubeconfig != "" { resolved.KubeconfigPath = cfg.Spec.Cluster.Connection.Kubeconfig } +} - 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) +func hasLoadedEKSConfigSource( + cfg *v1alpha1.Cluster, + distCfg *clusterprovisioner.DistributionConfig, + configClusterName string, +) bool { + return cfg.Spec.Cluster.Distribution == v1alpha1.DistributionEKS && + distCfg != nil && distCfg.EKS != nil && + distCfg.EKS.NameFromConfig && + strings.TrimSpace(distCfg.EKS.ConfigPath) != "" && + strings.TrimSpace(configClusterName) != "" +} + +func resolveClusterNameFromConfig( + currentName, configClusterName string, + eksConfigSource bool, + cfg *v1alpha1.Cluster, + distCfg *clusterprovisioner.DistributionConfig, +) string { + if currentName != "" { + return currentName + } + + // eksctl creates and deletes the name encoded in the actual eks.yaml source. ConfigManager may + // synthesize "eks-default" when the file is absent, so only grant this priority when ConfigPath + // proved the source exists. + if eksConfigSource { + return configClusterName + } + + if cfg.Name != "" && v1alpha1.ValidateClusterName(cfg.Name) == nil { + return cfg.Name + } + + return ClusterNameFromDistributionConfig(distCfg) +} + +// resolveConfigClusterName returns the distribution config's name when available because it is the +// source of any fallback AWS region. The top-level metadata name is the safe fallback. +func resolveConfigClusterName( + cfg *v1alpha1.Cluster, + distCfg *clusterprovisioner.DistributionConfig, +) string { + if cfg.Spec.Cluster.Distribution == v1alpha1.DistributionEKS && + distCfg != nil && distCfg.EKS != nil { + if strings.TrimSpace(distCfg.EKS.ConfigPath) == "" || !distCfg.EKS.NameFromConfig { + return "" + } + + return strings.TrimSpace(distCfg.EKS.Name) + } + + if name := ClusterNameFromDistributionConfig(distCfg); name != "" { + return name + } + + if v1alpha1.ValidateClusterName(cfg.Name) == nil { + return cfg.Name + } + + return "" +} + +// ValidateStandaloneAWSTarget rejects the ambiguous destructive case where the resolved target +// differs from the cluster described by the local EKS config while the only resolved region came +// from that config. Requiring the configured region environment variable makes the target pair +// deliberate and prevents delete/start/stop from mutating a same-named cluster in the wrong region. +func ValidateStandaloneAWSTarget(resolved *ResolvedClusterInfo) error { + if resolved == nil || resolved.Provider != v1alpha1.ProviderAWS || + !resolved.AWSRegionFromConfig || + resolved.ConfigClusterName == "" || resolved.ConfigClusterName == resolved.ClusterName { + return nil + } + + regionEnvVar := resolved.AWSOpts.RegionEnvVar + if regionEnvVar == "" { + regionEnvVar = awsRegionEnvVarDefault + } + + return fmt.Errorf( + "resolved EKS target %q does not match EKS config cluster %q: "+ + "refusing to reuse region %q from that config; set %s to select the target explicitly: %w", + resolved.ClusterName, + resolved.ConfigClusterName, + resolved.AWSRegion, + regionEnvVar, + ErrAWSTargetConfigMismatch, + ) } // commandContext returns cmd's context, falling back to context.Background() @@ -301,9 +480,15 @@ func runSimpleLifecycleAction( stageWriter := notify.NewStageSeparatingWriter(cmd.OutOrStdout()) cmd.SetOut(stageWriter) - // Resolve cluster info from flags, config, or kubeconfig + // Resolve cluster info from flags, config, or kubeconfig. These commands mutate the cluster, so a + // present-but-unreadable config must abort before any change. // Empty kubeconfig flag - simple lifecycle commands don't need kubeconfig cleanup - resolved, err := ResolveClusterInfo(cmd, nameFlag, providerFlag, "") + resolved, err := ResolveClusterInfoStrict(cmd, nameFlag, providerFlag, "") + if err != nil { + return err + } + + err = ValidateStandaloneAWSTarget(resolved) if err != nil { return err } @@ -351,10 +536,14 @@ func provisionAndAct( } provisioner, err := CreateMinimalProvisionerForProvider( + cmd.Context(), clusterInfo, - resolved.OmniOpts, - resolved.KubernetesOpts, - false, + MinimalProvisionerOptions{ + OmniOpts: resolved.OmniOpts, + KubernetesOpts: resolved.KubernetesOpts, + AWSOpts: resolved.AWSOpts, + AWSRegion: resolved.AWSRegion, + }, ) if err != nil { return fmt.Errorf("failed to create provisioner: %w", err) @@ -394,14 +583,23 @@ func CreateMinimalProvisioner( return provisioner, nil } +// MinimalProvisionerOptions contains the provider-specific context needed to +// construct a standalone lifecycle provisioner from only a resolved target. +type MinimalProvisionerOptions struct { + OmniOpts v1alpha1.OptionsOmni + KubernetesOpts v1alpha1.OptionsKubernetes + AWSOpts v1alpha1.OptionsAWS + AWSRegion string + DeleteStorage bool +} + // CreateMinimalProvisionerForProvider creates provisioners for all distributions // that support the given provider, and returns the first one that can operate on the cluster. // This is used when we only have --name and --provider flags without distribution info. func CreateMinimalProvisionerForProvider( + ctx context.Context, info *clusterdetector.Info, - omniOpts v1alpha1.OptionsOmni, - kubernetesOpts v1alpha1.OptionsKubernetes, - deleteStorage bool, + options MinimalProvisionerOptions, ) (clusterprovisioner.Provisioner, error) { switch info.Provider { case v1alpha1.ProviderDocker, "": @@ -415,7 +613,7 @@ func CreateMinimalProvisionerForProvider( // Pass info.KubeconfigPath as the nested cluster's kubeconfig for context cleanup. return newKubernetesCleanupProvisioner( info.ClusterName, - kubernetesOpts, + options.KubernetesOpts, info.KubeconfigPath, ) @@ -436,21 +634,23 @@ func CreateMinimalProvisionerForProvider( info.Provider, v1alpha1.OptionsTalos{}, v1alpha1.OptionsHetzner{}, - omniOpts, + options.OmniOpts, false, ) if err != nil { return nil, fmt.Errorf("failed to create talos provisioner: %w", err) } - provisioner.WithDeleteStorage(deleteStorage) + provisioner.WithDeleteStorage(options.DeleteStorage) return provisioner, nil - case v1alpha1.ProviderAWS, v1alpha1.ProviderGCP, v1alpha1.ProviderAzure: - // AWS, GCP, and Azure only support their managed distributions - // (EKS/GKE/AKS), which are not docker-based and cannot be produced by - // the minimal multi-provisioner path; their lifecycle goes through the factory. + case v1alpha1.ProviderAWS: + return createMinimalEKSProvisioner(ctx, info, options) + + case v1alpha1.ProviderGCP, v1alpha1.ProviderAzure: + // GCP and Azure only support their managed distributions (GKE/AKS), + // which are not yet wired into this standalone lifecycle path. return nil, fmt.Errorf( "%w: %s provider is only supported via its managed distribution", clusterprovisioner.ErrUnsupportedProvider, @@ -466,6 +666,40 @@ func CreateMinimalProvisionerForProvider( } } +// createMinimalEKSProvisioner builds the name-and-region-only EKS configuration +// needed by standalone delete/start/stop commands, while reusing the default +// factory's credential-isolated eksctl and AWS provider wiring. ConfigPath is +// deliberately empty: an explicit/resolved cluster name must never be replaced +// by a possibly unrelated local eks.yaml during a destructive delete. +func createMinimalEKSProvisioner( + ctx context.Context, + info *clusterdetector.Info, + options MinimalProvisionerOptions, +) (clusterprovisioner.Provisioner, error) { + clusterCfg := &v1alpha1.Cluster{} + clusterCfg.Name = info.ClusterName + clusterCfg.Spec.Cluster.Distribution = v1alpha1.DistributionEKS + clusterCfg.Spec.Cluster.Provider = v1alpha1.ProviderAWS + clusterCfg.Spec.Provider.AWS = options.AWSOpts + + factory := clusterprovisioner.DefaultFactory{ + DistributionConfig: &clusterprovisioner.DistributionConfig{ + EKS: &clusterprovisioner.EKSConfig{ + Name: info.ClusterName, + Region: options.AWSRegion, + KubeconfigPath: info.KubeconfigPath, + }, + }, + } + + provisioner, _, err := factory.Create(ctx, clusterCfg) + if err != nil { + return nil, fmt.Errorf("failed to create EKS provisioner: %w", err) + } + + return provisioner, nil +} + // kubernetesCleanupProvisioner deletes nested clusters on a Kubernetes provider // by removing the ksail- namespace (cascading delete removes all resources). type kubernetesCleanupProvisioner struct { diff --git a/pkg/cli/lifecycle/simple_additional_test.go b/pkg/cli/lifecycle/simple_additional_test.go index f93e67641c..19aa5d7244 100644 --- a/pkg/cli/lifecycle/simple_additional_test.go +++ b/pkg/cli/lifecycle/simple_additional_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "os" "testing" "github.com/devantler-tech/ksail/v7/pkg/apis/cluster/v1alpha1" @@ -67,6 +68,82 @@ func TestResolveClusterInfo(t *testing.T) { }) } +// TestResolveClusterInfoToleratesUnreadableConfigForReadOnlyCallers pins the split between +// read-only and mutating resolution. A broken ksail.yaml must not lock the user out of read-only +// commands (cluster info, diagnose, connect) when they have supplied an explicit --name, but it must +// still abort a mutating command before anything is changed. +// +//nolint:paralleltest // changes the process working directory +func TestResolveClusterInfoToleratesUnreadableConfigForReadOnlyCallers(t *testing.T) { + workingDir := t.TempDir() + t.Chdir(workingDir) + + require.NoError( + t, + os.WriteFile("ksail.yaml", []byte("this: is: not: valid: yaml:\n\t- broken\n"), 0o600), + ) + + // Read-only path: resolution falls back to the explicit flags. + resolved, err := lifecycle.ResolveClusterInfo( + nil, + "explicit-name", + v1alpha1.ProviderDocker, + "/non-existent-kubeconfig", + ) + require.NoError(t, err) + require.NotNil(t, resolved) + assert.Equal(t, "explicit-name", resolved.ClusterName) + + // Mutating path: the same broken config fails closed. + _, strictErr := lifecycle.ResolveClusterInfoStrict( + nil, + "explicit-name", + v1alpha1.ProviderDocker, + "/non-existent-kubeconfig", + ) + require.Error(t, strictErr) +} + +// TestResolveClusterInfoFallsBackToDistributionName verifies an intentionally empty metadata.name +// does not suppress the name carried by the distribution config. +// +//nolint:paralleltest // changes the process working directory +func TestResolveClusterInfoFallsBackToDistributionName(t *testing.T) { + workingDir := t.TempDir() + t.Chdir(workingDir) + + require.NoError( + t, + os.WriteFile( + "ksail.yaml", + []byte(`apiVersion: ksail.io/v1alpha1 +kind: Cluster +spec: + cluster: + distribution: Vanilla + provider: Docker + distributionConfig: kind.yaml +`), + 0o600, + ), + ) + require.NoError( + t, + os.WriteFile( + "kind.yaml", + []byte(`kind: Cluster +apiVersion: kind.x-k8s.io/v1alpha4 +name: distribution-only-name +`), + 0o600, + ), + ) + + resolved, err := lifecycle.ResolveClusterInfo(nil, "", "", "/non-existent-kubeconfig") + require.NoError(t, err) + assert.Equal(t, "distribution-only-name", resolved.ClusterName) +} + // TestNewSimpleLifecycleCmd tests the NewSimpleLifecycleCmd function. func TestNewSimpleLifecycleCmd(t *testing.T) { t.Parallel() diff --git a/pkg/fsutil/configmanager/ksail/distribution.go b/pkg/fsutil/configmanager/ksail/distribution.go index 2f7c29f390..2ec10141a5 100644 --- a/pkg/fsutil/configmanager/ksail/distribution.go +++ b/pkg/fsutil/configmanager/ksail/distribution.go @@ -28,6 +28,8 @@ import ( "sigs.k8s.io/yaml" ) +var errInvalidEKSConfig = errors.New("invalid EKS config file") + // loadKindConfig loads the Kind distribution configuration if it exists. // Returns ErrDistributionConfigNotFound if the file doesn't exist. // Returns error if config loading or validation fails. @@ -677,14 +679,17 @@ func (m *ConfigManager) cacheEKSConfig() error { return err } + nameFromConfig := strings.TrimSpace(name) != "" + if name == "" { name = m.resolveEKSNameFromContext() } m.DistributionConfig.EKS = &clusterprovisioner.EKSConfig{ - Name: name, - Region: region, - ConfigPath: resolvedPath, + Name: name, + NameFromConfig: nameFromConfig, + Region: region, + ConfigPath: resolvedPath, } return nil @@ -693,7 +698,9 @@ func (m *ConfigManager) cacheEKSConfig() error { // readEKSConfigMetadata reads the eksctl config file at configPath (if it // exists) and returns its canonical path plus the parsed metadata.name / // metadata.region fields. A missing file returns empty strings and no error -// so callers can fall back to context-based defaults. +// so callers can fall back to context-based defaults. A present file must be +// the supported eksctl ClusterConfig shape before its metadata can establish +// lifecycle ownership provenance. func readEKSConfigMetadata(configPath string) (string, string, string, error) { _, err := os.Stat(configPath) if err != nil { @@ -714,19 +721,68 @@ func readEKSConfigMetadata(configPath string) (string, string, string, error) { return "", "", "", fmt.Errorf("failed to read EKS config file: %w", err) } - var meta struct { - Metadata struct { - Name string `json:"name"` - Region string `json:"region"` - } `json:"metadata"` + name, region, err := parseEKSConfigMetadata(data) + if err != nil { + return "", "", "", err } - err = yaml.Unmarshal(data, &meta) + return canonical, name, region, nil +} + +type eksConfigMetadata struct { + APIVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Metadata struct { + Name string `json:"name"` + Region string `json:"region"` + } `json:"metadata"` +} + +func parseEKSConfigMetadata(data []byte) (string, string, error) { + var meta eksConfigMetadata + + err := yaml.Unmarshal(data, &meta) if err != nil { - return "", "", "", fmt.Errorf("failed to parse EKS config file: %w", err) + return "", "", fmt.Errorf("failed to parse EKS config file: %w", err) + } + + const ( + eksConfigAPIVersion = "eksctl.io/v1alpha5" + eksConfigKind = "ClusterConfig" + ) + + if meta.APIVersion != eksConfigAPIVersion || meta.Kind != eksConfigKind { + return "", "", fmt.Errorf( + "%w: expected apiVersion %q and kind %q, got %q and %q", + errInvalidEKSConfig, + eksConfigAPIVersion, + eksConfigKind, + meta.APIVersion, + meta.Kind, + ) + } + + name := meta.Metadata.Name + if name != "" { + if name != strings.TrimSpace(name) { + return "", "", fmt.Errorf( + "%w: metadata.name %q has leading or trailing whitespace", + errInvalidEKSConfig, + name, + ) + } + + err = v1alpha1.ValidateClusterName(name) + if err != nil { + return "", "", fmt.Errorf( + "%w: metadata.name: %w", + errInvalidEKSConfig, + err, + ) + } } - return canonical, meta.Metadata.Name, meta.Metadata.Region, nil + return name, meta.Metadata.Region, nil } // resolveEKSNameFromContext extracts the cluster name from an EKS kubeconfig diff --git a/pkg/fsutil/configmanager/ksail/distribution_test.go b/pkg/fsutil/configmanager/ksail/distribution_test.go index ab40ed9526..9a1e122acc 100644 --- a/pkg/fsutil/configmanager/ksail/distribution_test.go +++ b/pkg/fsutil/configmanager/ksail/distribution_test.go @@ -247,8 +247,7 @@ func TestIngressFirewallPatchesSuccess(t *testing.T) { }) } -// TestReadEKSConfigMetadata verifies the eksctl config metadata reader across -// missing-file, populated, metadata-less, and malformed-YAML cases. +// TestReadEKSConfigMetadata verifies the supported absent and valid eksctl config shapes. func TestReadEKSConfigMetadata(t *testing.T) { t.Parallel() @@ -293,17 +292,78 @@ func TestReadEKSConfigMetadata(t *testing.T) { assert.Empty(t, name) assert.Empty(t, region) }) +} - t.Run("malformed YAML returns an error", func(t *testing.T) { - t.Parallel() +func TestReadEKSConfigMetadataRejectsInvalidFiles(t *testing.T) { + t.Parallel() - configPath := filepath.Join(t.TempDir(), "eks.yaml") - // A tab in the indentation is invalid YAML. - writeFileWithParents(t, configPath, "metadata:\n\tname: my-eks\n") + for _, testCase := range invalidEKSConfigMetadataCases() { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() - _, _, _, err := configmanager.ReadEKSConfigMetadataForTest(configPath) - require.Error(t, err) - }) + configPath := filepath.Join(t.TempDir(), "eks.yaml") + writeFileWithParents(t, configPath, testCase.content) + + _, _, _, err := configmanager.ReadEKSConfigMetadataForTest(configPath) + require.Error(t, err) + assert.Contains(t, err.Error(), testCase.wantError) + }) + } +} + +type invalidEKSConfigMetadataCase struct { + name string + content string + wantError string +} + +func invalidEKSConfigMetadataCases() []invalidEKSConfigMetadataCase { + return []invalidEKSConfigMetadataCase{ + { + name: "wrong API version is rejected", + wantError: "invalid EKS config file", + content: "apiVersion: v1\n" + + "kind: ClusterConfig\n" + + "metadata:\n name: my-eks\n region: eu-west-1\n", + }, + { + name: "wrong kind is rejected", + wantError: "invalid EKS config file", + content: "apiVersion: eksctl.io/v1alpha5\n" + + "kind: ConfigMap\n" + + "metadata:\n name: my-eks\n region: eu-west-1\n", + }, + { + name: "missing API version is rejected", + content: "kind: ClusterConfig\nmetadata:\n name: my-eks\n region: eu-west-1\n", + wantError: "invalid EKS config file", + }, + { + name: "missing kind is rejected", + wantError: "invalid EKS config file", + content: "apiVersion: eksctl.io/v1alpha5\n" + + "metadata:\n name: my-eks\n region: eu-west-1\n", + }, + { + name: "padded metadata name is rejected", + wantError: "invalid EKS config file", + content: "apiVersion: eksctl.io/v1alpha5\n" + + "kind: ClusterConfig\n" + + "metadata:\n name: 'my-eks '\n region: eu-west-1\n", + }, + { + name: "invalid metadata name is rejected", + wantError: "invalid EKS config file", + content: "apiVersion: eksctl.io/v1alpha5\n" + + "kind: ClusterConfig\n" + + "metadata:\n name: Invalid_EKS\n region: eu-west-1\n", + }, + { + name: "malformed YAML is rejected", + content: "metadata:\n\tname: my-eks\n", + wantError: "failed to parse EKS config file", + }, + } } // TestResolveKWOKName verifies KWOK cluster-name resolution from the kubeconfig diff --git a/pkg/svc/provider/aws/nodegroup_state.go b/pkg/svc/provider/aws/nodegroup_state.go new file mode 100644 index 0000000000..59d9a0f381 --- /dev/null +++ b/pkg/svc/provider/aws/nodegroup_state.go @@ -0,0 +1,400 @@ +package aws + +import ( + "context" + "errors" + "fmt" + "log/slog" + "sort" + "strings" + + eksctlclient "github.com/devantler-tech/ksail/v7/pkg/client/eksctl" + "github.com/devantler-tech/ksail/v7/pkg/svc/state" +) + +var ( + errNodegroupStateVersion = errors.New("unsupported EKS nodegroup state version") + errNodegroupStateTarget = errors.New("EKS nodegroup state target mismatch") + errNodegroupStateDrift = errors.New("EKS nodegroup capacity state drift") +) + +func newNodegroupState( + clusterName, region string, + nodegroups []eksctlclient.NodegroupSummary, +) (*state.EKSNodegroupState, error) { + region = strings.TrimSpace(region) + if region == "" { + return nil, fmt.Errorf( + "refusing to persist regionless EKS nodegroup state: %w", + errNodegroupStateTarget, + ) + } + + capacities := make([]state.EKSNodegroupCapacity, 0, len(nodegroups)) + seen := make(map[string]struct{}, len(nodegroups)) + + for _, nodegroup := range nodegroups { + err := validateLiveNodegroup(clusterName, nodegroup) + if err != nil { + return nil, err + } + + if _, duplicate := seen[nodegroup.Name]; duplicate { + return nil, fmt.Errorf( + "duplicate nodegroup %q: %w", + nodegroup.Name, + errNodegroupStateDrift, + ) + } + + seen[nodegroup.Name] = struct{}{} + capacities = append(capacities, state.EKSNodegroupCapacity{ + Name: nodegroup.Name, + DesiredCapacity: nodegroup.DesiredCap, + MinSize: nodegroup.MinSize, + MaxSize: nodegroup.MaxSize, + }) + } + + _, err := validateSavedCapacities(capacities) + if err != nil { + return nil, err + } + + sort.Slice(capacities, func(i, j int) bool { return capacities[i].Name < capacities[j].Name }) + + return &state.EKSNodegroupState{ + Version: state.EKSNodegroupStateVersion, + ClusterName: clusterName, + Region: region, + Nodegroups: capacities, + }, nil +} + +func validateNodegroupTransition( + clusterName, region string, + snapshot *state.EKSNodegroupState, + nodegroups []eksctlclient.NodegroupSummary, +) (map[string]eksctlclient.NodegroupSummary, error) { + err := validateNodegroupStateIdentity(clusterName, region, snapshot) + if err != nil { + return nil, err + } + + savedByName, err := validateSavedCapacities(snapshot.Nodegroups) + if err != nil { + return nil, err + } + + if len(nodegroups) != len(savedByName) { + return nil, fmt.Errorf( + "live nodegroup count %d does not match saved count %d: %w", + len(nodegroups), + len(savedByName), + errNodegroupStateDrift, + ) + } + + liveByName := make(map[string]eksctlclient.NodegroupSummary, len(nodegroups)) + + for _, nodegroup := range nodegroups { + err = validateLiveNodegroup(clusterName, nodegroup) + if err != nil { + return nil, err + } + + capacity, found := savedByName[nodegroup.Name] + if !found || (!nodegroupMatchesCapacity(nodegroup, capacity) && + !nodegroupIsStopped(nodegroup, capacity)) { + return nil, fmt.Errorf( + "live nodegroup %q is neither saved nor safely stopped: %w", + nodegroup.Name, + errNodegroupStateDrift, + ) + } + + if _, duplicate := liveByName[nodegroup.Name]; duplicate { + return nil, fmt.Errorf( + "duplicate live nodegroup %q: %w", + nodegroup.Name, + errNodegroupStateDrift, + ) + } + + liveByName[nodegroup.Name] = nodegroup + } + + return liveByName, nil +} + +func validateNodegroupStateIdentity( + clusterName, region string, + snapshot *state.EKSNodegroupState, +) error { + if snapshot == nil { + return fmt.Errorf("nil EKS nodegroup state: %w", errNodegroupStateDrift) + } + + if snapshot.Version != state.EKSNodegroupStateVersion { + return fmt.Errorf( + "got version %d, want %d: %w", + snapshot.Version, + state.EKSNodegroupStateVersion, + errNodegroupStateVersion, + ) + } + + if strings.TrimSpace(clusterName) == "" || snapshot.ClusterName != clusterName || + strings.TrimSpace(region) == "" || snapshot.Region != region { + return fmt.Errorf( + "saved cluster/region %q/%q does not match target %q/%q: %w", + snapshot.ClusterName, + snapshot.Region, + clusterName, + region, + errNodegroupStateTarget, + ) + } + + return nil +} + +func validateSavedCapacities( + capacities []state.EKSNodegroupCapacity, +) (map[string]state.EKSNodegroupCapacity, error) { + if len(capacities) == 0 { + return nil, fmt.Errorf("saved nodegroup set is empty: %w", errNodegroupStateDrift) + } + + byName := make(map[string]state.EKSNodegroupCapacity, len(capacities)) + + for _, capacity := range capacities { + if strings.TrimSpace(capacity.Name) == "" || capacity.MinSize < 0 || + capacity.DesiredCapacity < 0 || + capacity.DesiredCapacity < capacity.MinSize || capacity.MaxSize <= 0 || + capacity.MaxSize < capacity.DesiredCapacity { + return nil, fmt.Errorf( + "invalid saved nodegroup capacity for %q: %w", + capacity.Name, + errNodegroupStateDrift, + ) + } + + if _, duplicate := byName[capacity.Name]; duplicate { + return nil, fmt.Errorf( + "duplicate saved nodegroup %q: %w", + capacity.Name, + errNodegroupStateDrift, + ) + } + + byName[capacity.Name] = capacity + } + + return byName, nil +} + +func validateLiveNodegroup(clusterName string, nodegroup eksctlclient.NodegroupSummary) error { + if strings.TrimSpace(nodegroup.Name) == "" || + (nodegroup.Cluster != "" && nodegroup.Cluster != clusterName) || + nodegroup.MinSize < 0 || nodegroup.DesiredCap < 0 || nodegroup.MaxSize < nodegroup.DesiredCap { + return fmt.Errorf( + "invalid live nodegroup %q for cluster %q: %w", + nodegroup.Name, + clusterName, + errNodegroupStateDrift, + ) + } + + return nil +} + +func verifyNodegroupsRestored( + clusterName, region string, + snapshot *state.EKSNodegroupState, + nodegroups []eksctlclient.NodegroupSummary, +) error { + liveByName, err := validateNodegroupTransition(clusterName, region, snapshot, nodegroups) + if err != nil { + return fmt.Errorf("verify restored EKS nodegroups: %w", err) + } + + for _, capacity := range snapshot.Nodegroups { + if !nodegroupMatchesCapacity(liveByName[capacity.Name], capacity) { + return fmt.Errorf( + "nodegroup %q is not restored: %w", + capacity.Name, + errNodegroupStateDrift, + ) + } + } + + return nil +} + +// snapshotIsAllStopped reports whether every nodegroup in the snapshot is already fully scaled down. +// Such a snapshot carries no recoverable capacity — restoring to it is a no-op — so it must never be +// persisted as the target state for a later start. +func snapshotIsAllStopped(snapshot *state.EKSNodegroupState) bool { + if snapshot == nil || len(snapshot.Nodegroups) == 0 { + return false + } + + for _, capacity := range snapshot.Nodegroups { + if capacity.DesiredCapacity > 0 || capacity.MinSize > 0 { + return false + } + } + + return true +} + +func nodegroupMatchesCapacity( + nodegroup eksctlclient.NodegroupSummary, + capacity state.EKSNodegroupCapacity, +) bool { + return nodegroup.DesiredCap == capacity.DesiredCapacity && + nodegroup.MinSize == capacity.MinSize && + nodegroup.MaxSize == capacity.MaxSize +} + +func nodegroupIsStopped( + nodegroup eksctlclient.NodegroupSummary, + capacity state.EKSNodegroupCapacity, +) bool { + return nodegroup.DesiredCap == 0 && nodegroup.MinSize == 0 && + nodegroup.MaxSize == capacity.MaxSize +} + +func (p *Provider) loadNodegroupState( + clusterName, region string, +) (*state.EKSNodegroupState, bool, error) { + snapshot, err := state.LoadEKSNodegroupState(clusterName, region) + if errors.Is(err, state.ErrEKSNodegroupStateNotFound) { + return nil, false, nil + } + + if err != nil { + return nil, false, fmt.Errorf("load EKS nodegroup state: %w", err) + } + + return snapshot, true, nil +} + +// startNodegroupsWithoutSnapshot preserves the pre-existing lifecycle for groups stopped without a +// capacity snapshot. A KSail stop from this release records the exact desired size first, so its own +// round trip never reaches this compatibility path — but a cluster stopped by an earlier release (or +// scaled to zero outside KSail) has no snapshot, and earlier releases scaled both desired and minimum +// to zero. Those clusters must stay startable after an upgrade, so this path keeps the historical +// max(MinSize, 1) fallback rather than failing closed. Starting nodes is non-destructive and +// reversible; the exact pre-stop size is simply unknown, which the caller is warned about. +func (p *Provider) startNodegroupsWithoutSnapshot( + ctx context.Context, + clusterName string, + nodegroups []eksctlclient.NodegroupSummary, +) error { + seen := make(map[string]struct{}, len(nodegroups)) + + for _, nodegroup := range nodegroups { + err := validateLiveNodegroup(clusterName, nodegroup) + if err != nil { + return err + } + + if _, duplicate := seen[nodegroup.Name]; duplicate { + return fmt.Errorf( + "duplicate live nodegroup %q: %w", + nodegroup.Name, + errNodegroupStateDrift, + ) + } + + seen[nodegroup.Name] = struct{}{} + } + + sort.Slice(nodegroups, func(i, j int) bool { return nodegroups[i].Name < nodegroups[j].Name }) + + for _, nodegroup := range nodegroups { + if nodegroup.DesiredCap > 0 { + continue + } + + // Without a snapshot the pre-stop desired size is unknowable, so fall back to the smallest + // running capacity that honours the group's own minimum — the behaviour every release before + // capacity snapshots used. + target := max(nodegroup.MinSize, 1) + + slog.Warn( + "restoring EKS nodegroup without a capacity snapshot; pre-stop desired size is unknown", + "cluster", clusterName, + "nodegroup", nodegroup.Name, + "target", target, + ) + + err := p.client.ScaleNodegroup( + ctx, + clusterName, + nodegroup.Name, + p.region, + target, + nodegroup.MinSize, + nodegroup.MaxSize, + ) + if err != nil { + return fmt.Errorf("start nodes: scale nodegroup %s: %w", nodegroup.Name, err) + } + } + + return nil +} + +func (p *Provider) restoreSavedNodegroups( + ctx context.Context, + clusterName string, + snapshot *state.EKSNodegroupState, + liveByName map[string]eksctlclient.NodegroupSummary, +) error { + for _, capacity := range snapshot.Nodegroups { + if nodegroupMatchesCapacity(liveByName[capacity.Name], capacity) { + continue + } + + err := p.client.ScaleNodegroup( + ctx, + clusterName, + capacity.Name, + p.region, + capacity.DesiredCapacity, + capacity.MinSize, + capacity.MaxSize, + ) + if err != nil { + return fmt.Errorf("start nodes: scale nodegroup %s: %w", capacity.Name, err) + } + } + + return nil +} + +func (p *Provider) verifyAndClearNodegroupState( + ctx context.Context, + clusterName string, + snapshot *state.EKSNodegroupState, +) error { + restored, err := p.listNodegroupsForScale(ctx, clusterName) + if err != nil { + return fmt.Errorf("verify restored nodegroups: %w", err) + } + + err = verifyNodegroupsRestored(clusterName, p.region, snapshot, restored) + if err != nil { + return err + } + + err = state.DeleteEKSNodegroupState(clusterName, p.region) + if err != nil { + return fmt.Errorf("clear restored EKS nodegroup state: %w", err) + } + + return nil +} diff --git a/pkg/svc/provider/aws/provider.go b/pkg/svc/provider/aws/provider.go index e804b5daa3..51417a9ce3 100644 --- a/pkg/svc/provider/aws/provider.go +++ b/pkg/svc/provider/aws/provider.go @@ -11,6 +11,7 @@ import ( 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/provider" + "github.com/devantler-tech/ksail/v7/pkg/svc/state" ) // NodegroupStatusActive is the EKS nodegroup status that indicates the @@ -89,65 +90,80 @@ func NewProvider(client *eksctlclient.Client, region string, opts ...Option) (*P return prov, nil } -// StartNodes scales all managed nodegroups for the cluster back to their -// configured desired capacity if it is currently zero. Nodegroups already at -// non-zero desired capacity are left alone. -// -// When a nodegroup's desired capacity is zero, this method does not know the -// "correct" target size to scale up to, so it defers to the max(MinSize, 1) -// that eksctl returned. The provisioner — which has the ksail.yaml spec — is -// responsible for restoring the exact DesiredCapacity when needed. +// StartNodes restores every managed nodegroup to the exact desired/minimum/maximum values captured +// before StopNodes zeroed it. The snapshot survives partial failures and is removed only after a +// readback confirms that every group is restored. func (p *Provider) StartNodes(ctx context.Context, clusterName string) error { - nodegroups, err := p.listNodegroupsForScale(ctx, clusterName) + target, nodegroups, snapshot, found, err := p.loadLifecycleNodegroupsAndState(ctx, clusterName) if err != nil { return err } - for _, nodegroup := range nodegroups { - if nodegroup.DesiredCap > 0 { - continue - } + if !found { + return target.startNodegroupsWithoutSnapshot(ctx, clusterName, nodegroups) + } - target := max(nodegroup.MinSize, 1) + liveByName, err := validateNodegroupTransition(clusterName, target.region, snapshot, nodegroups) + if err != nil { + return err + } - err = p.client.ScaleNodegroup( - ctx, - clusterName, - nodegroup.Name, - p.region, - target, - nodegroup.MinSize, - nodegroup.MaxSize, - ) - if err != nil { - return fmt.Errorf("start nodes: scale nodegroup %s: %w", nodegroup.Name, err) - } + err = target.restoreSavedNodegroups(ctx, clusterName, snapshot, liveByName) + if err != nil { + return err } - return nil + return target.verifyAndClearNodegroupState(ctx, clusterName, snapshot) } -// StopNodes scales all managed nodegroups to zero desired capacity. The -// cluster control plane remains running (EKS bills $0.10/hour for it) but -// all node-hour costs stop. +// StopNodes atomically snapshots every managed nodegroup before scaling it to zero. Repeated calls +// preserve the first snapshot and skip already-stopped groups, so a partial stop remains retryable. func (p *Provider) StopNodes(ctx context.Context, clusterName string) error { - nodegroups, err := p.listNodegroupsForScale(ctx, clusterName) + target, nodegroups, snapshot, found, err := p.loadLifecycleNodegroupsAndState(ctx, clusterName) if err != nil { return err } - for _, nodegroup := range nodegroups { - err = p.client.ScaleNodegroup( + if !found { + snapshot, err = newNodegroupState(clusterName, target.region, nodegroups) + if err != nil { + return err + } + + // Never persist an all-stopped snapshot. A cluster already at zero (stopped by an older + // release, or scaled down manually) would otherwise record 0/0 as its "restore to" state: + // the next start finds live already matching the snapshot, skips every scale, clears the + // file, and reports success while leaving the cluster at zero. Withholding the snapshot + // keeps the no-snapshot fallback — which restores max(MinSize, 1) — reachable instead. + if !snapshotIsAllStopped(snapshot) { + err = state.SaveEKSNodegroupState(clusterName, target.region, snapshot) + if err != nil { + return fmt.Errorf("save EKS nodegroup state before stop: %w", err) + } + } + } + + liveByName, err := validateNodegroupTransition(clusterName, target.region, snapshot, nodegroups) + if err != nil { + return err + } + + for _, capacity := range snapshot.Nodegroups { + if nodegroupIsStopped(liveByName[capacity.Name], capacity) { + continue + } + + err = target.client.ScaleNodegroup( ctx, clusterName, - nodegroup.Name, - p.region, + capacity.Name, + target.region, 0, 0, - nodegroup.MaxSize, + capacity.MaxSize, ) if err != nil { - return fmt.Errorf("stop nodes: scale nodegroup %s: %w", nodegroup.Name, err) + return fmt.Errorf("stop nodes: scale nodegroup %s: %w", capacity.Name, err) } } @@ -263,6 +279,75 @@ func (p *Provider) Region() string { return p.region } +// loadLifecycleNodegroupsAndState gives both nodegroup lifecycle paths one exact-region preflight. +func (p *Provider) loadLifecycleNodegroupsAndState( + ctx context.Context, + clusterName string, +) ( + *Provider, + []eksctlclient.NodegroupSummary, + *state.EKSNodegroupState, + bool, + error, +) { + target, err := p.withResolvedLifecycleRegion(ctx, clusterName) + if err != nil { + return nil, nil, nil, false, err + } + + nodegroups, snapshot, found, err := target.loadNodegroupsAndState(ctx, clusterName) + if err != nil { + return nil, nil, nil, false, err + } + + return target, nodegroups, snapshot, found, nil +} + +// withResolvedLifecycleRegion returns a request-local provider pinned to the exact region eksctl +// observed for clusterName. When configuration delegates region selection to an AWS profile, every +// read, mutation, and persisted snapshot in this lifecycle operation must use that same resolved +// identity; mutating the shared Provider would race concurrent requests. +func (p *Provider) withResolvedLifecycleRegion( + ctx context.Context, + clusterName string, +) (*Provider, error) { + configuredRegion := strings.TrimSpace(p.region) + if configuredRegion != "" { + if configuredRegion == p.region { + return p, nil + } + + resolved := *p + resolved.region = configuredRegion + + return &resolved, nil + } + + if p.client == nil { + return nil, provider.ErrProviderUnavailable + } + + summary, err := p.client.GetCluster(ctx, clusterName, "") + if err != nil { + return nil, fmt.Errorf("resolve EKS lifecycle region: %w", translateClientErr(err)) + } + + targetName := strings.TrimSpace(clusterName) + if summary == nil || targetName == "" || strings.TrimSpace(summary.Name) != targetName || + strings.TrimSpace(summary.Region) == "" { + return nil, fmt.Errorf( + "profile-resolved EKS cluster %q did not prove its exact name and region: %w", + clusterName, + errNodegroupStateTarget, + ) + } + + resolved := *p + resolved.region = strings.TrimSpace(summary.Region) + + return &resolved, nil +} + // clusterEndpoint reads the cluster's control-plane endpoint from the AWS SDK, // which eksctl's cluster summary omits — bringing EKS to parity with the GKE // and Azure providers, which both surface the endpoint on their status. The @@ -326,6 +411,27 @@ func (p *Provider) fetchNodegroups( return nodegroups, nil } +// loadNodegroupsAndState gives StartNodes and StopNodes one shared, ordered preflight: first prove +// that the target has managed nodegroups, then load any capacity snapshot before either path mutates it. +func (p *Provider) loadNodegroupsAndState( + ctx context.Context, + clusterName string, +) ([]eksctlclient.NodegroupSummary, *state.EKSNodegroupState, bool, error) { + nodegroups, err := p.listNodegroupsForScale(ctx, clusterName) + if err != nil { + return nil, nil, false, err + } + + // p is the request-local target pinned by withResolvedLifecycleRegion, so p.region is the exact + // region the snapshot was keyed by at stop time. + snapshot, found, err := p.loadNodegroupState(clusterName, p.region) + if err != nil { + return nil, nil, false, err + } + + return nodegroups, snapshot, found, nil +} + // listNodegroupsForScale is a shared prelude for StartNodes and StopNodes // that guards against nil clients, fetches nodegroups, and classifies the // empty result as provider.ErrNoNodes. diff --git a/pkg/svc/provider/aws/provider_test.go b/pkg/svc/provider/aws/provider_test.go index d73effd20c..cb0d01b4c9 100644 --- a/pkg/svc/provider/aws/provider_test.go +++ b/pkg/svc/provider/aws/provider_test.go @@ -3,6 +3,7 @@ package aws_test import ( "context" "errors" + "fmt" "io" "strings" "testing" @@ -12,6 +13,7 @@ import ( eksctlclient "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/provider/aws" + "github.com/devantler-tech/ksail/v7/pkg/svc/state" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -19,7 +21,11 @@ import ( // testEndpoint is the control-plane endpoint the default fake describer // returns so GetClusterStatus success paths never resolve real AWS // credentials. -const testEndpoint = "https://ABCDEF0123456789.gr7.us-east-1.eks.amazonaws.com" +const ( + testEndpoint = "https://ABCDEF0123456789.gr7.us-east-1.eks.amazonaws.com" + scaleSubcommand = "scale" + nodegroupSubcommand = "nodegroup" +) // fakeDescriber is a credential-free stand-in for the AWS-SDK EKS // DescribeCluster seam. It records the requested name and returns the @@ -47,6 +53,8 @@ var errScriptedRunnerEmptyArgs = errors.New("scripted runner: empty args") // GetClusterStatus surfaces a DescribeCluster failure. var errDescribeDenied = errors.New("access denied") +var errScaleDenied = errors.New("scale denied") + // scriptedRunner replays canned responses keyed by the first argument // (`create`, `delete`, `get`, `scale`, `upgrade`). It records every call for // assertions. @@ -95,6 +103,16 @@ func (s *scriptedRunner) Run( func newProvider(t *testing.T, responses map[string][]response) (*aws.Provider, *scriptedRunner) { t.Helper() + return newProviderWithRegion(t, responses, "us-east-1") +} + +func newProviderWithRegion( + t *testing.T, + responses map[string][]response, + region string, +) (*aws.Provider, *scriptedRunner) { + t.Helper() + runner := &scriptedRunner{t: t, responses: responses} client := eksctlclient.NewClient( @@ -105,7 +123,7 @@ func newProvider(t *testing.T, responses map[string][]response) (*aws.Provider, // Inject a happy describer so GetClusterStatus success paths populate the // endpoint without resolving real AWS credentials. Tests that exercise the // endpoint itself construct the provider with their own describer inline. - prov, err := aws.NewProvider(client, "us-east-1", aws.WithClusterDescriber( + prov, err := aws.NewProvider(client, region, aws.WithClusterDescriber( &fakeDescriber{cluster: &ekstypes.Cluster{Endpoint: awssdk.String(testEndpoint)}}, )) require.NoError(t, err) @@ -196,7 +214,7 @@ func TestDeleteNodes_Noop(t *testing.T) { } func TestStopNodes_ScalesAllToZero(t *testing.T) { - t.Parallel() + t.Setenv("HOME", t.TempDir()) prov, runner := newProvider(t, map[string][]response{ "get nodegroup": {{stdout: []byte(`[ @@ -212,7 +230,7 @@ func TestStopNodes_ScalesAllToZero(t *testing.T) { var scales [][]string for _, call := range runner.calls { - if len(call) >= 2 && call[0] == "scale" && call[1] == "nodegroup" { + if len(call) >= 2 && call[0] == scaleSubcommand && call[1] == nodegroupSubcommand { scales = append(scales, call) } } @@ -222,6 +240,130 @@ func TestStopNodes_ScalesAllToZero(t *testing.T) { assert.Contains(t, strings.Join(scales[1], " "), "--nodes 0") } +func TestStopNodesPinsProfileResolvedRegion(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProviderWithRegion(t, map[string][]response{ + "get cluster": {{ + stdout: []byte(`[{"Name":"demo","Region":"eu-north-1","EksctlCreated":"True"}]`), + }}, + "get nodegroup": {{stdout: nodegroupJSON(3, 2)}}, + "scale nodegroup": {{}}, + }, "") + + require.NoError(t, prov.StopNodes(t.Context(), "demo")) + + snapshot, err := state.LoadEKSNodegroupState("demo", "eu-north-1") + require.NoError(t, err) + assert.Equal(t, "eu-north-1", snapshot.Region) + assert.Equal(t, [][]string{ + {"get", "cluster", "--name", "demo", "--output", "json"}, + {"get", "nodegroup", "--cluster", "demo", "--output", "json", "--region", "eu-north-1"}, + { + "scale", "nodegroup", "--cluster", "demo", "--name", "ng-1", "--nodes", "0", + "--nodes-min", "0", "--nodes-max", "5", "--region", "eu-north-1", + }, + }, runner.calls) +} + +func TestStartNodesPinsProfileResolvedRegion(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + require.NoError(t, state.SaveEKSNodegroupState("demo", "eu-north-1", &state.EKSNodegroupState{ + Version: state.EKSNodegroupStateVersion, + ClusterName: "demo", + Region: "eu-north-1", + Nodegroups: []state.EKSNodegroupCapacity{ + {Name: "ng-1", DesiredCapacity: 3, MinSize: 2, MaxSize: 5}, + }, + })) + + prov, runner := newProviderWithRegion(t, map[string][]response{ + "get cluster": {{ + stdout: []byte(`[{"Name":"demo","Region":"eu-north-1","EksctlCreated":"True"}]`), + }}, + "get nodegroup": { + {stdout: nodegroupJSON(0, 0)}, + {stdout: nodegroupJSON(3, 2)}, + }, + "scale nodegroup": {{}}, + }, "") + + require.NoError(t, prov.StartNodes(t.Context(), "demo")) + + for _, call := range runner.calls { + if call[0] == "get" && call[1] == "cluster" { + continue + } + + assert.Contains(t, call, "eu-north-1") + } + + _, err := state.LoadEKSNodegroupState("demo", "eu-north-1") + require.ErrorIs(t, err, state.ErrEKSNodegroupStateNotFound) +} + +func TestStopNodesRejectsProfileResolvedClusterWithoutExactIdentity(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + //nolint:paralleltest // The subtests share the process HOME set by their parent. + for name, summary := range map[string]string{ + "different name": `[{"Name":"other","Region":"eu-north-1","EksctlCreated":"True"}]`, + "missing region": `[{"Name":"demo","Region":"","EksctlCreated":"True"}]`, + } { + t.Run(name, func(t *testing.T) { + prov, runner := newProviderWithRegion(t, map[string][]response{ + "get cluster": {{stdout: []byte(summary)}}, + }, "") + + err := prov.StopNodes(t.Context(), "demo") + require.ErrorContains(t, err, "did not prove its exact name and region") + assert.Len(t, runner.calls, 1) + }) + } +} + +func TestProfileResolvedLifecycleRegionDoesNotMutateSharedProvider(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProviderWithRegion(t, map[string][]response{ + "get cluster": { + {stdout: []byte(`[{"Name":"first","Region":"eu-north-1","EksctlCreated":"True"}]`)}, + {stdout: []byte(`[{"Name":"second","Region":"us-west-2","EksctlCreated":"True"}]`)}, + }, + "get nodegroup": { + { + stdout: []byte( + `[{"Cluster":"first","Name":"workers","DesiredCapacity":2,"MinSize":1,"MaxSize":3}]`, + ), + }, + { + stdout: []byte( + `[{"Cluster":"second","Name":"workers","DesiredCapacity":2,"MinSize":1,"MaxSize":3}]`, + ), + }, + }, + "scale nodegroup": {{}, {}}, + }, "") + + require.NoError(t, prov.StopNodes(t.Context(), "first")) + require.NoError(t, prov.StopNodes(t.Context(), "second")) + assert.Empty(t, prov.Region()) + assert.Equal(t, [][]string{ + {"get", "cluster", "--name", "first", "--output", "json"}, + {"get", "nodegroup", "--cluster", "first", "--output", "json", "--region", "eu-north-1"}, + { + "scale", "nodegroup", "--cluster", "first", "--name", "workers", "--nodes", "0", + "--nodes-min", "0", "--nodes-max", "3", "--region", "eu-north-1", + }, + {"get", "cluster", "--name", "second", "--output", "json"}, + {"get", "nodegroup", "--cluster", "second", "--output", "json", "--region", "us-west-2"}, + { + "scale", "nodegroup", "--cluster", "second", "--name", "workers", "--nodes", "0", + "--nodes-min", "0", "--nodes-max", "3", "--region", "us-west-2", + }, + }, runner.calls) +} + func TestStopNodes_NoNodegroups(t *testing.T) { t.Parallel() @@ -234,13 +376,28 @@ func TestStopNodes_NoNodegroups(t *testing.T) { } func TestStartNodes_ScalesOnlyZeroDesired(t *testing.T) { - t.Parallel() + t.Setenv("HOME", t.TempDir()) + require.NoError(t, state.SaveEKSNodegroupState("demo", "us-east-1", &state.EKSNodegroupState{ + Version: state.EKSNodegroupStateVersion, + ClusterName: "demo", + Region: "us-east-1", + Nodegroups: []state.EKSNodegroupCapacity{ + {Name: "ng-1", DesiredCapacity: 2, MinSize: 2, MaxSize: 5}, + {Name: "ng-2", DesiredCapacity: 3, MinSize: 1, MaxSize: 5}, + }, + })) prov, runner := newProvider(t, map[string][]response{ - "get nodegroup": {{stdout: []byte(`[ - {"Cluster":"demo","Name":"ng-1","DesiredCapacity":0,"MinSize":2,"MaxSize":5}, - {"Cluster":"demo","Name":"ng-2","DesiredCapacity":3,"MinSize":1,"MaxSize":5} - ]`)}}, + "get nodegroup": { + {stdout: []byte(`[ + {"Cluster":"demo","Name":"ng-1","DesiredCapacity":0,"MinSize":0,"MaxSize":5}, + {"Cluster":"demo","Name":"ng-2","DesiredCapacity":3,"MinSize":1,"MaxSize":5} + ]`)}, + {stdout: []byte(`[ + {"Cluster":"demo","Name":"ng-1","DesiredCapacity":2,"MinSize":2,"MaxSize":5}, + {"Cluster":"demo","Name":"ng-2","DesiredCapacity":3,"MinSize":1,"MaxSize":5} + ]`)}, + }, "scale nodegroup": {{}}, }) @@ -250,7 +407,7 @@ func TestStartNodes_ScalesOnlyZeroDesired(t *testing.T) { scales := 0 for _, call := range runner.calls { - if len(call) >= 2 && call[0] == "scale" && call[1] == "nodegroup" { + if len(call) >= 2 && call[0] == scaleSubcommand && call[1] == nodegroupSubcommand { scales++ joined := strings.Join(call, " ") @@ -262,6 +419,373 @@ func TestStartNodes_ScalesOnlyZeroDesired(t *testing.T) { assert.Equal(t, 1, scales, "should scale only nodegroups with desiredCapacity=0") } +// TestStopThenStartRestoresNodegroupCapacities proves the lifecycle round trip does not lose the +// desired/minimum sizes when stop must temporarily set both values to zero. +func TestStopThenStartRestoresNodegroupCapacities(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProvider(t, map[string][]response{ + "get nodegroup": { + {stdout: []byte(`[ + {"Cluster":"demo","Name":"ng-1","DesiredCapacity":3,"MinSize":2,"MaxSize":5} + ]`)}, + {stdout: []byte(`[ + {"Cluster":"demo","Name":"ng-1","DesiredCapacity":0,"MinSize":0,"MaxSize":5} + ]`)}, + {stdout: []byte(`[ + {"Cluster":"demo","Name":"ng-1","DesiredCapacity":3,"MinSize":2,"MaxSize":5} + ]`)}, + }, + "scale nodegroup": {{}, {}}, + }) + + require.NoError(t, prov.StopNodes(t.Context(), "demo")) + require.NoError(t, prov.StartNodes(t.Context(), "demo")) + + var scales []string + + for _, call := range runner.calls { + if len(call) >= 2 && call[0] == scaleSubcommand && call[1] == nodegroupSubcommand { + scales = append(scales, strings.Join(call, " ")) + } + } + + require.Equal(t, []string{ + "scale nodegroup --cluster demo --name ng-1 --nodes 0 --nodes-min 0 --nodes-max 5 --region us-east-1", + "scale nodegroup --cluster demo --name ng-1 --nodes 3 --nodes-min 2 --nodes-max 5 --region us-east-1", + }, scales) + + _, err := state.LoadEKSNodegroupState("demo", "us-east-1") + require.ErrorIs(t, err, state.ErrEKSNodegroupStateNotFound) +} + +func TestStopThenStartPreservesZeroCapacityNodegroup(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProvider(t, map[string][]response{ + "get nodegroup": { + {stdout: []byte(`[ + {"Cluster":"demo","Name":"a-idle","DesiredCapacity":0,"MinSize":0,"MaxSize":5}, + {"Cluster":"demo","Name":"b-active","DesiredCapacity":3,"MinSize":2,"MaxSize":5} + ]`)}, + {stdout: []byte(`[ + {"Cluster":"demo","Name":"a-idle","DesiredCapacity":0,"MinSize":0,"MaxSize":5}, + {"Cluster":"demo","Name":"b-active","DesiredCapacity":0,"MinSize":0,"MaxSize":5} + ]`)}, + {stdout: []byte(`[ + {"Cluster":"demo","Name":"a-idle","DesiredCapacity":0,"MinSize":0,"MaxSize":5}, + {"Cluster":"demo","Name":"b-active","DesiredCapacity":3,"MinSize":2,"MaxSize":5} + ]`)}, + }, + "scale nodegroup": {{}, {}}, + }) + + require.NoError(t, prov.StopNodes(t.Context(), "demo")) + require.NoError(t, prov.StartNodes(t.Context(), "demo")) + assert.Equal(t, []string{ + "scale nodegroup --cluster demo --name b-active --nodes 0 --nodes-min 0 --nodes-max 5 --region us-east-1", + "scale nodegroup --cluster demo --name b-active --nodes 3 --nodes-min 2 --nodes-max 5 --region us-east-1", + }, scaleCalls(runner.calls)) + + _, err := state.LoadEKSNodegroupState("demo", "us-east-1") + require.ErrorIs(t, err, state.ErrEKSNodegroupStateNotFound) +} + +func TestStartNodesRejectsZeroMaximumCapacitySnapshot(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + require.NoError(t, state.SaveEKSNodegroupState("demo", "us-east-1", &state.EKSNodegroupState{ + Version: state.EKSNodegroupStateVersion, + ClusterName: "demo", + Region: "us-east-1", + Nodegroups: []state.EKSNodegroupCapacity{ + {Name: "idle", DesiredCapacity: 0, MinSize: 0, MaxSize: 0}, + }, + })) + + prov, _ := newProvider(t, map[string][]response{ + "get nodegroup": {{stdout: []byte(`[ + {"Cluster":"demo","Name":"idle","DesiredCapacity":0,"MinSize":0,"MaxSize":0} + ]`)}}, + }) + + err := prov.StartNodes(t.Context(), "demo") + require.ErrorContains(t, err, "invalid saved nodegroup capacity") +} + +func TestRepeatedStopPreservesOriginalNodegroupCapacities(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProvider(t, map[string][]response{ + "get nodegroup": { + {stdout: nodegroupJSON(3, 2)}, + {stdout: nodegroupJSON(0, 0)}, + {stdout: nodegroupJSON(0, 0)}, + {stdout: nodegroupJSON(3, 2)}, + }, + "scale nodegroup": {{}, {}}, + }) + + require.NoError(t, prov.StopNodes(t.Context(), "demo")) + require.NoError(t, prov.StopNodes(t.Context(), "demo")) + require.NoError(t, prov.StartNodes(t.Context(), "demo")) + + assert.Equal(t, []string{ + "scale nodegroup --cluster demo --name ng-1 --nodes 0 --nodes-min 0 --nodes-max 5 --region us-east-1", + "scale nodegroup --cluster demo --name ng-1 --nodes 3 --nodes-min 2 --nodes-max 5 --region us-east-1", + }, scaleCalls(runner.calls)) +} + +func TestStopNodesRejectsInvalidCapacityBeforePersisting(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProvider(t, map[string][]response{ + "get nodegroup": {{stdout: nodegroupJSON(1, 2)}}, + }) + + err := prov.StopNodes(t.Context(), "demo") + require.Error(t, err) + assert.Empty(t, scaleCalls(runner.calls)) + + _, loadErr := state.LoadEKSNodegroupState("demo", "us-east-1") + require.ErrorIs(t, loadErr, state.ErrEKSNodegroupStateNotFound) +} + +// TestStartNodesRejectsMismatchedCapacityStateBeforeScaling exercises the snapshot's own region +// identity check as defence in depth. The path is now region-scoped, so a wrong-region snapshot can +// no longer be reached by accident — this covers a corrupted or hand-edited file whose recorded +// region contradicts the target it was loaded for. +func TestStartNodesRejectsMismatchedCapacityStateBeforeScaling(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + saveNodegroupStateAt(t, "demo", "us-east-1", "eu-north-1", 3, 2, 5) + + prov, runner := newProvider(t, map[string][]response{ + "get nodegroup": {{stdout: nodegroupJSON(0, 0)}}, + }) + + err := prov.StartNodes(t.Context(), "demo") + require.Error(t, err) + assert.Contains(t, err.Error(), "region") + assert.Empty(t, scaleCalls(runner.calls)) +} + +func TestStartNodesRetainsCapacityStateWhenScalingFails(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + saveNodegroupState(t, "demo", "us-east-1", 3, 2, 5) + + prov, _ := newProvider(t, map[string][]response{ + "get nodegroup": {{stdout: nodegroupJSON(0, 0)}}, + "scale nodegroup": {{err: errScaleDenied}}, + }) + + err := prov.StartNodes(t.Context(), "demo") + require.ErrorIs(t, err, errScaleDenied) + + saved, loadErr := state.LoadEKSNodegroupState("demo", "us-east-1") + require.NoError(t, loadErr) + assert.Equal(t, 3, saved.Nodegroups[0].DesiredCapacity) + assert.Equal(t, 2, saved.Nodegroups[0].MinSize) +} + +// TestStopNodesDoesNotPersistAnAllStoppedSnapshot guards the stop→start round trip for a cluster +// that is ALREADY at zero (stopped by an older release, or scaled down by hand). Recording its +// all-zero live state as the restore target would make the next start a silent no-op: live already +// matches the snapshot, every scale is skipped, the snapshot is cleared, and success is reported +// with every nodegroup still at zero — while permanently shadowing the no-snapshot fallback. +func TestStopNodesDoesNotPersistAnAllStoppedSnapshot(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProvider(t, map[string][]response{ + "get nodegroup": {{stdout: nodegroupJSON(0, 0)}}, + }) + + require.NoError(t, prov.StopNodes(t.Context(), "demo")) + + // Nothing to scale (already stopped) and, critically, no snapshot written. + assert.Empty(t, scaleCalls(runner.calls)) + + _, err := state.LoadEKSNodegroupState("demo", "us-east-1") + require.ErrorIs(t, err, state.ErrEKSNodegroupStateNotFound, + "an all-stopped snapshot must not be persisted") +} + +// TestStopThenStartOnAlreadyStoppedClusterStillRestores is the end-to-end consequence: after a stop +// on an already-zero cluster, start must actually bring nodes back rather than report a no-op +// success. +func TestStopThenStartOnAlreadyStoppedClusterStillRestores(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProvider(t, map[string][]response{ + "get nodegroup": {{stdout: nodegroupJSON(0, 0)}, {stdout: nodegroupJSON(0, 0)}}, + "scale nodegroup": {{}}, + }) + + require.NoError(t, prov.StopNodes(t.Context(), "demo")) + require.NoError(t, prov.StartNodes(t.Context(), "demo")) + + assert.Equal(t, []string{ + "scale nodegroup --cluster demo --name ng-1 --nodes 1 --nodes-min 0 --nodes-max 5 " + + "--region us-east-1", + }, scaleCalls(runner.calls), "start must restore capacity, not silently no-op") +} + +// TestStopNodesStillSnapshotsPartiallyRunningNodegroups is the negative half: a snapshot is only +// withheld when EVERY group is stopped, so a mixed cluster keeps its recoverable capacity. +func TestStopNodesStillSnapshotsPartiallyRunningNodegroups(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, _ := newProvider(t, map[string][]response{ + "get nodegroup": {{stdout: []byte(`[ + {"Cluster":"demo","Name":"a-stopped","DesiredCapacity":0,"MinSize":0,"MaxSize":5}, + {"Cluster":"demo","Name":"b-running","DesiredCapacity":3,"MinSize":2,"MaxSize":5} + ]`)}}, + "scale nodegroup": {{}}, + }) + + require.NoError(t, prov.StopNodes(t.Context(), "demo")) + + saved, err := state.LoadEKSNodegroupState("demo", "us-east-1") + require.NoError(t, err, "a partially running cluster must keep its snapshot") + assert.Len(t, saved.Nodegroups, 2) +} + +// TestStartNodesWithoutSnapshotFallsBackToMinimumOne pins upgrade compatibility: releases before +// capacity snapshots stopped a cluster by scaling desired AND minimum to zero without recording +// anything. Such a cluster must still start after an upgrade, using the historical max(MinSize, 1) +// fallback rather than failing closed and stranding the user outside KSail. +func TestStartNodesWithoutSnapshotFallsBackToMinimumOne(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProvider(t, map[string][]response{ + "get nodegroup": {{stdout: nodegroupJSON(0, 0)}}, + "scale nodegroup": {{}}, + }) + + err := prov.StartNodes(t.Context(), "demo") + require.NoError(t, err) + assert.Equal(t, []string{ + "scale nodegroup --cluster demo --name ng-1 --nodes 1 --nodes-min 0 --nodes-max 5 " + + "--region us-east-1", + }, scaleCalls(runner.calls)) +} + +// TestStartNodesWithoutSnapshotHonoursEachGroupMinimum covers the mixed case: a group with its own +// non-zero minimum returns to that minimum, while a fully zeroed legacy group returns to one node. +func TestStartNodesWithoutSnapshotHonoursEachGroupMinimum(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProvider(t, map[string][]response{ + "get nodegroup": {{stdout: []byte(`[ + {"Cluster":"demo","Name":"a-recoverable","DesiredCapacity":0,"MinSize":2,"MaxSize":5}, + {"Cluster":"demo","Name":"z-legacy-zeroed","DesiredCapacity":0,"MinSize":0,"MaxSize":5} + ]`)}}, + "scale nodegroup": {{}, {}}, + }) + + err := prov.StartNodes(t.Context(), "demo") + require.NoError(t, err) + assert.Equal(t, []string{ + "scale nodegroup --cluster demo --name a-recoverable --nodes 2 --nodes-min 2 " + + "--nodes-max 5 --region us-east-1", + "scale nodegroup --cluster demo --name z-legacy-zeroed --nodes 1 --nodes-min 0 " + + "--nodes-max 5 --region us-east-1", + }, scaleCalls(runner.calls)) +} + +// TestStartNodesWithoutSnapshotPreflightsEveryNodegroup keeps the preflight guarantee: the whole +// live set is validated before any group is scaled, so a drifted listing mutates nothing. +func TestStartNodesWithoutSnapshotPreflightsEveryNodegroup(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + prov, runner := newProvider(t, map[string][]response{ + "get nodegroup": {{stdout: []byte(`[ + {"Cluster":"demo","Name":"a-recoverable","DesiredCapacity":0,"MinSize":2,"MaxSize":5}, + {"Cluster":"demo","Name":"a-recoverable","DesiredCapacity":0,"MinSize":0,"MaxSize":5} + ]`)}}, + }) + + err := prov.StartNodes(t.Context(), "demo") + require.Error(t, err) + assert.Contains(t, err.Error(), "duplicate live nodegroup") + assert.Empty(t, scaleCalls(runner.calls)) +} + +func TestStartNodesRetainsCapacityStateUntilReadbackMatches(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + saveNodegroupState(t, "demo", "us-east-1", 3, 2, 5) + + prov, _ := newProvider(t, map[string][]response{ + "get nodegroup": { + {stdout: nodegroupJSON(0, 0)}, + {stdout: nodegroupJSON(0, 0)}, + }, + "scale nodegroup": {{}}, + }) + + err := prov.StartNodes(t.Context(), "demo") + require.Error(t, err) + assert.Contains(t, err.Error(), "not restored") + + _, loadErr := state.LoadEKSNodegroupState("demo", "us-east-1") + require.NoError(t, loadErr) +} + +func nodegroupJSON(desiredCapacity, minSize int) []byte { + return fmt.Appendf( + nil, + `[{"Cluster":"demo","Name":"ng-1","DesiredCapacity":%d,"MinSize":%d,"MaxSize":5}]`, + desiredCapacity, + minSize, + ) +} + +func saveNodegroupState( + t *testing.T, + clusterName, region string, + desiredCapacity, minSize, maxSize int, +) { + t.Helper() + + saveNodegroupStateAt(t, clusterName, region, region, desiredCapacity, minSize, maxSize) +} + +// saveNodegroupStateAt writes a snapshot under pathRegion while recording bodyRegion inside it, so +// tests can construct the contradiction the in-file region identity check exists to catch. +func saveNodegroupStateAt( + t *testing.T, + clusterName, pathRegion, bodyRegion string, + desiredCapacity, minSize, maxSize int, +) { + t.Helper() + + snapshot := &state.EKSNodegroupState{ + Version: state.EKSNodegroupStateVersion, + ClusterName: clusterName, + Region: bodyRegion, + Nodegroups: []state.EKSNodegroupCapacity{ + { + Name: "ng-1", + DesiredCapacity: desiredCapacity, + MinSize: minSize, + MaxSize: maxSize, + }, + }, + } + + require.NoError(t, state.SaveEKSNodegroupState(clusterName, pathRegion, snapshot)) +} + +func scaleCalls(calls [][]string) []string { + scales := make([]string, 0) + + for _, call := range calls { + if len(call) >= 2 && call[0] == scaleSubcommand && call[1] == nodegroupSubcommand { + scales = append(scales, strings.Join(call, " ")) + } + } + + return scales +} + func TestStartNodes_NoNodegroups(t *testing.T) { t.Parallel() diff --git a/pkg/svc/provisioner/cluster/eks/provisioner.go b/pkg/svc/provisioner/cluster/eks/provisioner.go index 2a8470d117..f2654c485a 100644 --- a/pkg/svc/provisioner/cluster/eks/provisioner.go +++ b/pkg/svc/provisioner/cluster/eks/provisioner.go @@ -15,18 +15,17 @@ import ( // // All operations delegate to pkg/client/eksctl.Client, which in turn shells // out to the eksctl binary. The provisioner holds the declarative -// eksctl.yaml path so Create/Delete can be driven from the same source of -// truth scaffolded by ksail project init. +// eksctl.yaml path so Create can be driven from the source of truth +// scaffolded by ksail project init. type Provisioner struct { // name is the cluster name derived from the ksail.yaml / eksctl.yaml. name string // region is the AWS region. Cached here so operations that accept - // --region without a --config-file (e.g. GetCluster on a deleted + // --region without a --config-file (e.g. Delete or GetCluster on a deleted // cluster) still work. region string // configPath is the path to the declarative eksctl ClusterConfig. - // Required for Create; preferred over --name/--region for Delete and - // Upgrade because it keeps CloudFormation stack naming consistent. + // Required for Create. configPath string // kubeconfigPath is the exact file eksctl writes and KSail reads after create. kubeconfigPath string @@ -147,9 +146,7 @@ func (p *Provisioner) Create(ctx context.Context, name string) error { return nil } -// Delete tears down the EKS cluster. Prefers --config-file when available so -// eksctl can unwind the same CloudFormation stacks it created; falls back to -// --name/--region for clusters imported without a local config. +// Delete tears down the exact EKS cluster identified by name and region. func (p *Provisioner) Delete(ctx context.Context, name string) error { target := p.resolveName(name) @@ -158,10 +155,11 @@ func (p *Provisioner) Delete(ctx context.Context, name string) error { return fmt.Errorf("eksctl unavailable: %w", err) } - // DeleteCluster prefers configPath over name when configPath is set. + // Keep configPath empty so DeleteCluster cannot replace the validated exact + // target with a name or region from the declarative create configuration. // Wait=true because cluster deletion must complete before ksail // considers the workspace clean. - err = p.client.DeleteCluster(ctx, target, p.region, p.configPath, true) + err = p.client.DeleteCluster(ctx, target, p.region, "", true) if err != nil { return fmt.Errorf("eksctl delete cluster: %w", err) } diff --git a/pkg/svc/provisioner/cluster/eks/provisioner_test.go b/pkg/svc/provisioner/cluster/eks/provisioner_test.go index d0260ba41d..35821cbe04 100644 --- a/pkg/svc/provisioner/cluster/eks/provisioner_test.go +++ b/pkg/svc/provisioner/cluster/eks/provisioner_test.go @@ -151,7 +151,7 @@ func TestCreate_NoConfigPath_ReturnsError(t *testing.T) { require.ErrorIs(t, err, eksprovisioner.ErrConfigPathRequired) } -func TestDelete_PrefersConfigFile(t *testing.T) { +func TestDelete_UsesExactNameAndRegion(t *testing.T) { t.Parallel() prov, runner := newProvisioner(t, map[string][]response{ @@ -161,9 +161,17 @@ func TestDelete_PrefersConfigFile(t *testing.T) { require.NoError(t, prov.Delete(context.Background(), "")) require.Len(t, runner.calls, 1) - // DeleteCluster prefers --config-file over --name when configPath set. - assert.Contains(t, runner.calls[0], "--config-file") - assert.Contains(t, runner.calls[0], "--wait") + assert.Equal( + t, + []string{ + "delete", "cluster", + "--name", "ksail-test", + "--region", "us-east-1", + "--wait", + }, + runner.calls[0], + ) + assert.NotContains(t, runner.calls[0], "--config-file") } func TestStart_DelegatesToProvider(t *testing.T) { diff --git a/pkg/svc/provisioner/cluster/factory.go b/pkg/svc/provisioner/cluster/factory.go index 9278e8ea8b..7139d601be 100644 --- a/pkg/svc/provisioner/cluster/factory.go +++ b/pkg/svc/provisioner/cluster/factory.go @@ -62,6 +62,9 @@ type DistributionConfig struct { type EKSConfig struct { // Name is the cluster name (mirrors eksctl.yaml metadata.name). Name string + // NameFromConfig reports that Name was parsed from metadata.name in the actual eksctl config, + // rather than synthesized from a kubeconfig context or default. + NameFromConfig bool // Region is the AWS region. Region string // ConfigPath is the path to the declarative eksctl.yaml. diff --git a/pkg/svc/state/eks_nodegroup_state.go b/pkg/svc/state/eks_nodegroup_state.go new file mode 100644 index 0000000000..0df9dfe4e0 --- /dev/null +++ b/pkg/svc/state/eks_nodegroup_state.go @@ -0,0 +1,142 @@ +package state + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/devantler-tech/ksail/v7/pkg/fsutil" +) + +const ( + // EKSNodegroupStateVersion is the on-disk schema version for EKS capacity snapshots. + EKSNodegroupStateVersion = 1 + // eksNodegroupStateFileNameFormat is kept separate from spec and TTL state so a successful start + // can clear only the transient capacity snapshot. The region is part of the file name because a + // cluster name is unique only within a region. + eksNodegroupStateFileNameFormat = "eks-nodegroups-%s.json" +) + +var ( + // ErrEKSNodegroupStateNotFound reports that no stop-time EKS capacity snapshot exists. + ErrEKSNodegroupStateNotFound = errors.New("EKS nodegroup state not found") + // ErrInvalidRegion reports a missing region or one containing path separators or '..'. + ErrInvalidRegion = errors.New( + "invalid AWS region: must be non-empty and must not contain path separators or '..'", + ) + errEKSNodegroupStateNil = errors.New("EKS nodegroup state is nil") +) + +// EKSNodegroupCapacity records the exact scaling values a stopped managed nodegroup must regain. +type EKSNodegroupCapacity struct { + Name string `json:"name"` + DesiredCapacity int `json:"desiredCapacity"` + MinSize int `json:"minSize"` + MaxSize int `json:"maxSize"` +} + +// EKSNodegroupState binds a stop-time capacity snapshot to one cluster name and AWS region. +type EKSNodegroupState struct { + Version int `json:"version"` + ClusterName string `json:"clusterName"` + Region string `json:"region"` + Nodegroups []EKSNodegroupCapacity `json:"nodegroups"` +} + +// SaveEKSNodegroupState atomically persists the capacity snapshot before stop mutates AWS. +func SaveEKSNodegroupState(clusterName, region string, snapshot *EKSNodegroupState) error { + if snapshot == nil { + return errEKSNodegroupStateNil + } + + statePath, err := eksNodegroupStatePath(clusterName, region) + if err != nil { + return err + } + + err = os.MkdirAll(filepath.Dir(statePath), dirPermissions) + if err != nil { + return fmt.Errorf("failed to create EKS nodegroup state directory: %w", err) + } + + data, err := json.MarshalIndent(snapshot, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal EKS nodegroup state: %w", err) + } + + err = fsutil.AtomicWriteFile(statePath, data, filePermissions) + if err != nil { + return fmt.Errorf("failed to write EKS nodegroup state: %w", err) + } + + return nil +} + +// LoadEKSNodegroupState reads the stop-time EKS capacity snapshot. +func LoadEKSNodegroupState(clusterName, region string) (*EKSNodegroupState, error) { + statePath, err := eksNodegroupStatePath(clusterName, region) + if err != nil { + return nil, err + } + + //nolint:gosec // clusterStateDir validates the user-controlled path component. + data, err := os.ReadFile(statePath) + if err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("%w: %s", ErrEKSNodegroupStateNotFound, clusterName) + } + + return nil, fmt.Errorf("failed to read EKS nodegroup state: %w", err) + } + + var snapshot EKSNodegroupState + + err = json.Unmarshal(data, &snapshot) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal EKS nodegroup state: %w", err) + } + + return &snapshot, nil +} + +// DeleteEKSNodegroupState removes only the transient EKS capacity snapshot. +func DeleteEKSNodegroupState(clusterName, region string) error { + statePath, err := eksNodegroupStatePath(clusterName, region) + if err != nil { + return err + } + + err = os.Remove(statePath) + if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to remove EKS nodegroup state: %w", err) + } + + return nil +} + +// eksNodegroupStatePath scopes the snapshot to one cluster *and* one region. Cluster names are only +// unique within a region, so keying the file by name alone lets same-named clusters in two regions +// share — and clobber — a single snapshot: a stop in one region would be restored from the other's +// capacities, or rejected outright by the snapshot's own region identity check. +func eksNodegroupStatePath(clusterName, region string) (string, error) { + dir, err := clusterStateDir(clusterName) + if err != nil { + return "", err + } + + region = strings.TrimSpace(region) + if region == "" { + return "", ErrInvalidRegion + } + + if strings.Contains(region, "/") || + strings.Contains(region, "\\") || + strings.Contains(region, "..") { + return "", ErrInvalidRegion + } + + return filepath.Join(dir, fmt.Sprintf(eksNodegroupStateFileNameFormat, region)), nil +} diff --git a/pkg/svc/state/eks_nodegroup_state_test.go b/pkg/svc/state/eks_nodegroup_state_test.go new file mode 100644 index 0000000000..a74117bf22 --- /dev/null +++ b/pkg/svc/state/eks_nodegroup_state_test.go @@ -0,0 +1,126 @@ +package state_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/devantler-tech/ksail/v7/pkg/svc/state" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSaveLoadAndDeleteEKSNodegroupState(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + const clusterName = "eks-capacity-round-trip" + + want := &state.EKSNodegroupState{ + Version: state.EKSNodegroupStateVersion, + ClusterName: clusterName, + Region: "eu-north-1", + Nodegroups: []state.EKSNodegroupCapacity{ + {Name: "workers", DesiredCapacity: 3, MinSize: 2, MaxSize: 5}, + }, + } + + require.NoError(t, state.SaveEKSNodegroupState(clusterName, "eu-north-1", want)) + + got, err := state.LoadEKSNodegroupState(clusterName, "eu-north-1") + require.NoError(t, err) + assert.Equal(t, want, got) + + statePath := filepath.Join( + home, ".ksail", "clusters", clusterName, "eks-nodegroups-eu-north-1.json", + ) + info, err := os.Stat(statePath) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o600), info.Mode().Perm()) + + neighborPath := filepath.Join(filepath.Dir(statePath), "spec.json") + require.NoError(t, os.WriteFile(neighborPath, []byte("neighbor"), 0o600)) + + require.NoError(t, state.DeleteEKSNodegroupState(clusterName, "eu-north-1")) + assert.FileExists(t, neighborPath) + + _, err = state.LoadEKSNodegroupState(clusterName, "eu-north-1") + require.ErrorIs(t, err, state.ErrEKSNodegroupStateNotFound) +} + +// TestEKSNodegroupStateIsScopedPerRegion pins the cross-region isolation the snapshot path provides: +// two same-named clusters in different regions must keep independent capacity snapshots, and +// removing one must leave the other intact. +func TestEKSNodegroupStateIsScopedPerRegion(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + const clusterName = "same-name-two-regions" + + north := &state.EKSNodegroupState{ + Version: state.EKSNodegroupStateVersion, + ClusterName: clusterName, + Region: "eu-north-1", + Nodegroups: []state.EKSNodegroupCapacity{ + {Name: "workers", DesiredCapacity: 3, MinSize: 2, MaxSize: 5}, + }, + } + east := &state.EKSNodegroupState{ + Version: state.EKSNodegroupStateVersion, + ClusterName: clusterName, + Region: "us-east-1", + Nodegroups: []state.EKSNodegroupCapacity{ + {Name: "workers", DesiredCapacity: 7, MinSize: 4, MaxSize: 9}, + }, + } + + require.NoError(t, state.SaveEKSNodegroupState(clusterName, "eu-north-1", north)) + require.NoError(t, state.SaveEKSNodegroupState(clusterName, "us-east-1", east)) + + // Neither region may read the other's capacities. + gotNorth, err := state.LoadEKSNodegroupState(clusterName, "eu-north-1") + require.NoError(t, err) + assert.Equal(t, north, gotNorth) + + gotEast, err := state.LoadEKSNodegroupState(clusterName, "us-east-1") + require.NoError(t, err) + assert.Equal(t, east, gotEast) + + // Clearing one region's snapshot must not discard the other's restore data. + require.NoError(t, state.DeleteEKSNodegroupState(clusterName, "eu-north-1")) + + _, err = state.LoadEKSNodegroupState(clusterName, "eu-north-1") + require.ErrorIs(t, err, state.ErrEKSNodegroupStateNotFound) + + survivor, err := state.LoadEKSNodegroupState(clusterName, "us-east-1") + require.NoError(t, err) + assert.Equal(t, east, survivor) +} + +func TestEKSNodegroupStateRejectsUnusableRegion(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + for _, region := range []string{"", " ", "../escape", "eu/north", `eu\north`} { + _, err := state.LoadEKSNodegroupState("demo", region) + require.ErrorIs(t, err, state.ErrInvalidRegion, "region %q must be rejected", region) + } +} + +func TestLoadEKSNodegroupStateRejectsInvalidJSON(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + stateDir := filepath.Join(home, ".ksail", "clusters", "invalid-json") + require.NoError(t, os.MkdirAll(stateDir, 0o700)) + require.NoError( + t, + os.WriteFile( + filepath.Join(stateDir, "eks-nodegroups-eu-north-1.json"), + []byte("{"), + 0o600, + ), + ) + + _, err := state.LoadEKSNodegroupState("invalid-json", "eu-north-1") + require.Error(t, err) + assert.Contains(t, err.Error(), "unmarshal EKS nodegroup state") +}