diff --git a/cmd/sync.go b/cmd/sync.go index 29d2392..3e4971f 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -42,6 +42,7 @@ var ( kubeloginExtraArgs []string kubeloginTokenCacheDir string dryRun bool + includeNotReady bool ) func init() { @@ -74,6 +75,7 @@ func init() { syncCmd.Flags().StringVar(&kubeloginTokenCacheDir, "kubelogin-token-cache-dir", defaultTokenCacheDir, "Directory for OIDC token cache files") syncCmd.Flags().BoolVar(&dryRun, "dry-run", false, "Preview changes without writing to the kubeconfig file") + syncCmd.Flags().BoolVar(&includeNotReady, "include-not-ready", false, "Also merge clusters whose Ready condition is False (spec data must be present)") // BindPFlags can theoretically return an error if called with `nil` as an argument // which should never happen after at least one flag was defined. That's why the output @@ -87,9 +89,13 @@ var syncCmd = &cobra.Command{ Long: `Fetches ClusterKubeconfig resources from a Greenhouse cluster and merges them into your local kubeconfig file. -Only clusters whose Ready condition is True are merged. Clusters that have -been removed from Greenhouse are cleaned up from your local config. Existing -non-managed entries are never touched. +Only clusters whose Ready condition is True are merged by default. Clusters +whose Ready condition is False are preserved in the local kubeconfig so they +remain reachable (e.g. to investigate or recover the cluster). Use +--include-not-ready to also merge their spec data. + +Clusters that have been removed from Greenhouse are cleaned up from your +local config. Existing non-managed entries are never touched. OIDC credentials are preserved across syncs: id-token and refresh-token are carried forward so you do not need to re-authenticate after every sync. @@ -135,6 +141,7 @@ func runSync(cmd *cobra.Command, args []string) error { kubeloginExtraArgs = viper.GetStringSlice("kubelogin-extra-args") kubeloginTokenCacheDir = viper.GetString("kubelogin-token-cache-dir") dryRun = viper.GetBool("dry-run") + includeNotReady = viper.GetBool("include-not-ready") format, err := output.ParseFormat(viper.GetString("output")) if err != nil { @@ -205,10 +212,9 @@ func runSync(cmd *cobra.Command, args []string) error { ready, notReady := partitionReady(allKubeconfigs) - if len(ready) == 0 { + if shouldSkipMerge(ready, notReady, includeNotReady) { return printer.Print(buildSyncResult(nil, notReady)) } - var localConfig *clientcmdapi.Config if remoteClusterKubeconfig != "" { localConfig, err = clientcmd.LoadFromFile(remoteClusterKubeconfig) @@ -223,11 +229,45 @@ func runSync(cmd *cobra.Command, args []string) error { localConfig = clientcmdapi.NewConfig() } - serverConfig, err := buildIncomingKubeconfig(ready) + // merged = ready + notReady entries that were opted into full merge. + // skipped = notReady entries that were NOT merged (preserved in place or absent). + // notReady entries with an empty spec are always kept on the stub/preservation path + // regardless of --include-not-ready: they contribute no incoming data and merging + // them would delete any existing local entry without adding anything in return. + merged := ready + var skipped []v1alpha1.ClusterKubeconfig + if includeNotReady { + for _, ckc := range notReady { + if hasSpecData(ckc) { + merged = append(merged, ckc) + } else { + skipped = append(skipped, ckc) + } + } + } else { + skipped = notReady + } + + serverConfig, err := buildIncomingKubeconfig(merged) if err != nil { return fmt.Errorf("failed to create server config: %w", err) } + // Preserve notReady cluster entries that already exist in the local kubeconfig. + // A cluster whose Ready condition is transiently false (e.g. controller bug, rolling + // upgrade) must not be wiped from the local kubeconfig — the user may need it to + // reach and fix the cluster. We inject stub entries into serverConfig so the stale- + // cleanup pass in mergeKubeconfig treats them as still-present on the server side. + // The stubs only affect deletion logic; they are never written to disk. + // When --include-not-ready is set, notReady entries are fully merged instead. + // stubsToPreserve is only populated when stubs are injected (i.e. !includeNotReady). + // It maps server-side authinfo names to the local managed name so mergeKubeconfig + // can pre-seed authInfoMap and avoid redirecting stub contexts to a wrong authinfo. + var stubsToPreserve map[string]string + if !includeNotReady { + stubsToPreserve = injectNotReadyStubs(serverConfig, skipped, localConfig) + } + // Take a snapshot before merge for dry-run diff — only needed when --dry-run is set. var localConfigBefore *clientcmdapi.Config if dryRun { @@ -239,10 +279,10 @@ func runSync(cmd *cobra.Command, args []string) error { spinnerLabel = "Simulating merge (dry-run)..." } stopMerge := printer.StartSpinner(spinnerLabel) - err = mergeKubeconfig(localConfig, serverConfig) + err = mergeKubeconfig(localConfig, serverConfig, stubsToPreserve) stopMerge() if err != nil { - _ = printer.Print(buildFailedSyncResult(ready, notReady, err)) + _ = printer.Print(buildFailedSyncResult(merged, skipped, err)) return fmt.Errorf(`failed to merge ClusterKubeconfig: %w`, err) } @@ -257,11 +297,11 @@ func runSync(cmd *cobra.Command, args []string) error { } if writeErr := writeConfig(localConfig, writeTarget); writeErr != nil { - _ = printer.Print(buildFailedSyncResult(ready, notReady, writeErr)) + _ = printer.Print(buildFailedSyncResult(merged, skipped, writeErr)) return fmt.Errorf("failed to write merged kubeconfig: %w", writeErr) } - return printer.Print(buildSyncResult(ready, notReady)) + return printer.Print(buildSyncResult(merged, skipped)) } // partitionReady splits ClusterKubeconfigs into ready and notReady slices. @@ -278,6 +318,23 @@ func partitionReady(items []v1alpha1.ClusterKubeconfig) (ready, notReady []v1alp return ready, notReady } +// hasSpecData reports whether the ClusterKubeconfig has at least one cluster, context, or +// authinfo entry in its spec. Resources with an entirely empty spec contribute no entries to +// the merged kubeconfig and should remain on the preservation path even when --include-not-ready +// is set, to avoid their existing local entries being deleted by the stale-cleanup pass. +func hasSpecData(ckc v1alpha1.ClusterKubeconfig) bool { + return len(ckc.Spec.Kubeconfig.Clusters) > 0 || + len(ckc.Spec.Kubeconfig.Contexts) > 0 || + len(ckc.Spec.Kubeconfig.AuthInfo) > 0 +} + +// shouldSkipMerge returns true when there is nothing useful to merge: +// no ready clusters and either the --include-not-ready flag is not set or there are no +// not-ready clusters to merge either. +func shouldSkipMerge(ready, notReady []v1alpha1.ClusterKubeconfig, inclNotReady bool) bool { + return len(ready) == 0 && (!inclNotReady || len(notReady) == 0) +} + // filterReady returns only ClusterKubeconfigs that have Ready condition set to True. // Deprecated: use partitionReady instead. func filterReady(items []v1alpha1.ClusterKubeconfig) []v1alpha1.ClusterKubeconfig { @@ -285,6 +342,74 @@ func filterReady(items []v1alpha1.ClusterKubeconfig) []v1alpha1.ClusterKubeconfi return ready } +// injectNotReadyStubs adds stub entries into serverConfig for each notReady ClusterKubeconfig +// that already has a corresponding managed entry in localConfig. This prevents mergeKubeconfig's +// stale-cleanup pass from deleting local entries for clusters that are temporarily not-ready — +// the user needs those entries to reach and recover the cluster. +// Stubs are derived from the existing local entries (no spec data required) and are never written to disk. +// The returned map (server-side unprefixed authinfo name → local managed name) is passed to +// mergeKubeconfig as a pre-seed so stub context authinfo references are not redirected by deduplication. +func injectNotReadyStubs(serverConfig *clientcmdapi.Config, notReady []v1alpha1.ClusterKubeconfig, localConfig *clientcmdapi.Config) map[string]string { + authPreSeed := make(map[string]string) + for _, ckc := range notReady { + // Collect the set of context names this CKC owns: prefer spec context names when + // available (spec may be populated even if Ready=False), fall back to the resource + // name so we still protect entries written by an older sync. + ctxNames := make([]string, 0, len(ckc.Spec.Kubeconfig.Contexts)+1) + for _, ctxItem := range ckc.Spec.Kubeconfig.Contexts { + ctxNames = append(ctxNames, ctxItem.Name) + } + if len(ctxNames) == 0 { + ctxNames = append(ctxNames, ckc.Name) + } + + for _, ctxName := range ctxNames { + localCtx, ok := localConfig.Contexts[ctxName] + if !ok || localCtx == nil { + continue + } + // skip unmanaged contexts — only protect entries cloudctl wrote. + if !isManaged(localCtx.Cluster) { + continue + } + + // stub context must carry unprefixed cluster/authinfo references so + // mergeKubeconfig's lookup (which strips the prefix before checking serverConfig) + // resolves correctly and does not produce double-prefixed names. + stubCtx := &clientcmdapi.Context{ + Cluster: unmanagedNameFunc(localCtx.Cluster), + AuthInfo: unmanagedNameFunc(localCtx.AuthInfo), + Namespace: localCtx.Namespace, + } + if _, exists := serverConfig.Contexts[ctxName]; !exists { + serverConfig.Contexts[ctxName] = stubCtx + } + + // Inject stub cluster keyed by the unprefixed name. + clusterName := unmanagedNameFunc(localCtx.Cluster) + if _, exists := serverConfig.Clusters[clusterName]; !exists { + if src, ok := localConfig.Clusters[localCtx.Cluster]; ok { + serverConfig.Clusters[clusterName] = src.DeepCopy() + } + } + + // Record the authinfo pre-seed mapping instead of adding the stub to serverConfig. + // Putting the stub authinfo into serverConfig would cause mergeKubeconfig's + // deduplication pass to assign a new hash-derived managed name, redirecting the + // preserved context to a different authinfo entry and altering its effective identity. + // The pre-seed tells mergeKubeconfig to map the stub authinfo name directly to the + // existing local managed name, keeping the context unchanged. + authName := unmanagedNameFunc(localCtx.AuthInfo) + if authName != "" && localCtx.AuthInfo != "" { + if _, exists := serverConfig.AuthInfos[authName]; !exists { + authPreSeed[authName] = localCtx.AuthInfo + } + } + } + } + return authPreSeed +} + // buildSyncResult constructs an output.SyncResult from ready and notReady cluster lists. func buildSyncResult(ready, notReady []v1alpha1.ClusterKubeconfig) output.SyncResult { result := output.SyncResult{} @@ -488,7 +613,7 @@ func buildKubeloginArgs(cfg map[string]string, extra []string, tokenCacheDir str return args } -func mergeKubeconfig(localConfig *clientcmdapi.Config, serverConfig *clientcmdapi.Config) error { +func mergeKubeconfig(localConfig *clientcmdapi.Config, serverConfig *clientcmdapi.Config, preSeedAuthInfoMap map[string]string) error { // Merge Clusters for serverName, serverCluster := range serverConfig.Clusters { managedName := managedNameFunc(serverName) @@ -518,6 +643,13 @@ func mergeKubeconfig(localConfig *clientcmdapi.Config, serverConfig *clientcmdap if mergeIdenticalUsers { authInfoMap = make(map[string]string) + // Pre-seed stub authinfo mappings from notReady preservation. These entries were + // not added to serverConfig (to avoid dedup side effects) but their context + // references must still resolve to the correct local managed name. + for serverName, localManagedName := range preSeedAuthInfoMap { + authInfoMap[serverName] = localManagedName + } + // Build a reverse lookup of unmanaged local auth entries so we can reuse their names // instead of creating new cloudctl:auth- entries. // Store all candidate names per key; at reuse time we pick the lexicographically @@ -641,7 +773,14 @@ func mergeKubeconfig(localConfig *clientcmdapi.Config, serverConfig *clientcmdap localConfig.AuthInfos[managedAuthInfoName] = serverAuth } } else { - managedAuthInfoName = managedNameFunc(serverCtx.AuthInfo) + // Without deduplication, derive the managed name from the server-side name. + // For stub contexts injected by injectNotReadyStubs the authinfo was not added + // to serverConfig; its correct local managed name is in preSeedAuthInfoMap. + if preSeedName, ok := preSeedAuthInfoMap[serverCtx.AuthInfo]; ok { + managedAuthInfoName = preSeedName + } else { + managedAuthInfoName = managedNameFunc(serverCtx.AuthInfo) + } } // Update Cluster name @@ -696,9 +835,14 @@ func mergeKubeconfig(localConfig *clientcmdapi.Config, serverConfig *clientcmdap delete(localConfig.AuthInfos, localName) } } else { - // Derive the server-side name by stripping the prefix + // Derive the server-side name by stripping the prefix. + // Also protect authinfos pre-seeded from notReady stubs — those were not + // added to serverConfig, so the server-side name lookup would otherwise + // cause them to be deleted. serverName := unmanagedNameFunc(localName) - if _, exists := serverConfig.AuthInfos[serverName]; !exists { + _, inServer := serverConfig.AuthInfos[serverName] + _, inPreSeed := preSeedAuthInfoMap[serverName] + if !inServer && !inPreSeed { slog.Debug("removing stale authinfo", "name", localName) delete(localConfig.AuthInfos, localName) } @@ -733,7 +877,12 @@ func mergeKubeconfig(localConfig *clientcmdapi.Config, serverConfig *clientcmdap } expectedAuthInfo = mappedName } else { - expectedAuthInfo = managedNameFunc(serverCtx.AuthInfo) + // Same pre-seed lookup as in context merging above. + if preSeedName, ok := preSeedAuthInfoMap[serverCtx.AuthInfo]; ok { + expectedAuthInfo = preSeedName + } else { + expectedAuthInfo = managedNameFunc(serverCtx.AuthInfo) + } } if localCtx.Cluster != expectedCluster || localCtx.AuthInfo != expectedAuthInfo { diff --git a/cmd/sync_test.go b/cmd/sync_test.go index fc89fb8..07f7446 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -608,7 +608,7 @@ func TestMergeKubeconfig_PrefersExistingLocalAuthName(t *testing.T) { AuthInfo: "server-user", } - err := mergeKubeconfig(localConfig, serverConfig) + err := mergeKubeconfig(localConfig, serverConfig, nil) g.Expect(err).ToNot(HaveOccurred()) // The unmanaged "my-existing-user" should be reused — no cloudctl:auth-* should be created @@ -653,7 +653,7 @@ func TestMergeKubeconfig_DeduplicatesSameOIDCUsers(t *testing.T) { serverConfig.Contexts["cluster-b"] = &clientcmdapi.Context{Cluster: "cluster-b", AuthInfo: "cluster-b-user"} localConfig := clientcmdapi.NewConfig() - err := mergeKubeconfig(localConfig, serverConfig) + err := mergeKubeconfig(localConfig, serverConfig, nil) g.Expect(err).ToNot(HaveOccurred()) // Count managed auth entries — should be exactly 1 @@ -880,7 +880,7 @@ func TestDryRun_MergeAndDiff_NoWrite(t *testing.T) { serverConfig.Contexts["new-cluster"] = &clientcmdapi.Context{Cluster: "new-cluster", AuthInfo: "shared-user"} localConfig := localConfigBefore.DeepCopy() - err := mergeKubeconfig(localConfig, serverConfig) + err := mergeKubeconfig(localConfig, serverConfig, nil) g.Expect(err).ToNot(HaveOccurred()) diff := diffKubeconfig(localConfigBefore, localConfig) @@ -1055,3 +1055,190 @@ func TestWriteTargetFromKUBECONFIG(t *testing.T) { }) } } + +func TestInjectNotReadyStubs_PreservesLocalEntries(t *testing.T) { + g := NewWithT(t) + + orig := prefix + prefix = "cloudctl" + defer func() { prefix = orig }() + + notReadyCkc := greenhousev1alpha1.ClusterKubeconfig{ + ObjectMeta: metav1.ObjectMeta{Name: "broken-cluster"}, + } + + // Local kubeconfig already has a managed entry for broken-cluster. + localConfig := clientcmdapi.NewConfig() + localConfig.Clusters["cloudctl:broken-cluster"] = &clientcmdapi.Cluster{Server: "https://broken.example.com"} + localConfig.AuthInfos["cloudctl:oidc@broken-cluster"] = &clientcmdapi.AuthInfo{} + localConfig.Contexts["broken-cluster"] = &clientcmdapi.Context{ + Cluster: "cloudctl:broken-cluster", + AuthInfo: "cloudctl:oidc@broken-cluster", + } + + serverConfig := clientcmdapi.NewConfig() + + preSeed := injectNotReadyStubs(serverConfig, []greenhousev1alpha1.ClusterKubeconfig{notReadyCkc}, localConfig) + + // Context and cluster stubs must be present so mergeKubeconfig's stale-cleanup does not delete them. + g.Expect(serverConfig.Contexts).To(HaveKey("broken-cluster")) + g.Expect(serverConfig.Clusters).To(HaveKey("broken-cluster")) + // AuthInfo is no longer added to serverConfig (to avoid dedup side-effects); + // instead it is returned via the pre-seed map for authInfoMap population. + g.Expect(serverConfig.AuthInfos).NotTo(HaveKey("oidc@broken-cluster")) + g.Expect(preSeed).To(HaveKeyWithValue("oidc@broken-cluster", "cloudctl:oidc@broken-cluster")) + + // Fix 2: stub context references must be unprefixed to avoid double-prefix on re-merge. + g.Expect(serverConfig.Contexts["broken-cluster"].Cluster).To(Equal("broken-cluster")) + g.Expect(serverConfig.Contexts["broken-cluster"].AuthInfo).To(Equal("oidc@broken-cluster")) +} + +func TestInjectNotReadyStubs_UsesSpecContextNames(t *testing.T) { + g := NewWithT(t) + + orig := prefix + prefix = "cloudctl" + defer func() { prefix = orig }() + + // CKC resource name is "demo" but the context in spec is "ctx1" (as in e2e tests). + notReadyCkc := greenhousev1alpha1.ClusterKubeconfig{ + ObjectMeta: metav1.ObjectMeta{Name: "demo"}, + } + notReadyCkc.Spec.Kubeconfig.Contexts = append(notReadyCkc.Spec.Kubeconfig.Contexts, + greenhousev1alpha1.ClusterKubeconfigContextItem{ + Name: "ctx1", + }, + ) + + localConfig := clientcmdapi.NewConfig() + localConfig.Clusters["cloudctl:ctx1-cluster"] = &clientcmdapi.Cluster{Server: "https://ctx1.example.com"} + localConfig.AuthInfos["cloudctl:oidc@ctx1"] = &clientcmdapi.AuthInfo{} + localConfig.Contexts["ctx1"] = &clientcmdapi.Context{ + Cluster: "cloudctl:ctx1-cluster", + AuthInfo: "cloudctl:oidc@ctx1", + } + + serverConfig := clientcmdapi.NewConfig() + + _ = injectNotReadyStubs(serverConfig, []greenhousev1alpha1.ClusterKubeconfig{notReadyCkc}, localConfig) + + // Stub must be keyed by spec context name "ctx1", not resource name "demo". + g.Expect(serverConfig.Contexts).To(HaveKey("ctx1")) + g.Expect(serverConfig.Contexts).NotTo(HaveKey("demo")) +} + +func TestInjectNotReadyStubs_SkipsUnmanagedContexts(t *testing.T) { + g := NewWithT(t) + + orig := prefix + prefix = "cloudctl" + defer func() { prefix = orig }() + + notReadyCkc := greenhousev1alpha1.ClusterKubeconfig{ + ObjectMeta: metav1.ObjectMeta{Name: "manual-cluster"}, + } + + // Context exists in local but cluster ref is NOT prefixed — it is an unmanaged entry. + localConfig := clientcmdapi.NewConfig() + localConfig.Clusters["manual-cluster"] = &clientcmdapi.Cluster{Server: "https://manual.example.com"} + localConfig.Contexts["manual-cluster"] = &clientcmdapi.Context{ + Cluster: "manual-cluster", // no cloudctl: prefix + } + + serverConfig := clientcmdapi.NewConfig() + + _ = injectNotReadyStubs(serverConfig, []greenhousev1alpha1.ClusterKubeconfig{notReadyCkc}, localConfig) + + // Unmanaged context must not be touched. + g.Expect(serverConfig.Contexts).To(BeEmpty()) + g.Expect(serverConfig.Clusters).To(BeEmpty()) +} + +func TestInjectNotReadyStubs_SkipsUnknownClusters(t *testing.T) { + g := NewWithT(t) + + orig := prefix + prefix = "cloudctl" + defer func() { prefix = orig }() + + // notReady cluster has no corresponding local entry. + notReadyCkc := greenhousev1alpha1.ClusterKubeconfig{ + ObjectMeta: metav1.ObjectMeta{Name: "new-cluster"}, + } + + serverConfig := clientcmdapi.NewConfig() + localConfig := clientcmdapi.NewConfig() + + preSeed := injectNotReadyStubs(serverConfig, []greenhousev1alpha1.ClusterKubeconfig{notReadyCkc}, localConfig) + + // Nothing added — no local entry to preserve. + g.Expect(serverConfig.Contexts).To(BeEmpty()) + g.Expect(serverConfig.Clusters).To(BeEmpty()) + g.Expect(serverConfig.AuthInfos).To(BeEmpty()) + g.Expect(preSeed).To(BeEmpty()) +} + +func TestIncludeNotReady_MergesSpecData(t *testing.T) { + g := NewWithT(t) + + orig := includeNotReady + includeNotReady = true + defer func() { includeNotReady = orig }() + + notReadyCkc := greenhousev1alpha1.ClusterKubeconfig{ + ObjectMeta: metav1.ObjectMeta{Name: "notready-cluster"}, + } + notReadyCkc.Status.Conditions.SetConditions( + greenhousemetav1alpha1.FalseCondition( + greenhousemetav1alpha1.ReadyCondition, + "KubeconfigMissing", + "secret data key greenhousekubeconfig missing or empty", + ), + ) + notReadyCkc.Spec.Kubeconfig.Clusters = append(notReadyCkc.Spec.Kubeconfig.Clusters, + greenhousev1alpha1.ClusterKubeconfigClusterItem{ + Name: "notready-cluster", + Cluster: greenhousev1alpha1.ClusterKubeconfigCluster{ + Server: "https://notready.example.com", + }, + }, + ) + + ready, notReady := partitionReady([]greenhousev1alpha1.ClusterKubeconfig{notReadyCkc}) + g.Expect(ready).To(BeEmpty()) + g.Expect(notReady).To(HaveLen(1)) + + // With includeNotReady=true, notReady entries are appended to the merge set. + toMerge := ready + if includeNotReady { + toMerge = append(toMerge, notReady...) + } + serverConfig, err := buildIncomingKubeconfig(toMerge) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(serverConfig.Clusters).To(HaveKey("notready-cluster")) + g.Expect(serverConfig.Clusters["notready-cluster"].Server).To(Equal("https://notready.example.com")) +} + +func TestIncludeNotReady_BypassesEarlyReturnWhenAllNotReady(t *testing.T) { + g := NewWithT(t) + + notReadyCkc := greenhousev1alpha1.ClusterKubeconfig{ + ObjectMeta: metav1.ObjectMeta{Name: "all-broken"}, + } + notReadyCkc.Status.Conditions.SetConditions( + greenhousemetav1alpha1.FalseCondition( + greenhousemetav1alpha1.ReadyCondition, + "KubeconfigMissing", + "secret data key greenhousekubeconfig missing or empty", + ), + ) + + ready, notReady := partitionReady([]greenhousev1alpha1.ClusterKubeconfig{notReadyCkc}) + g.Expect(ready).To(BeEmpty()) + g.Expect(notReady).To(HaveLen(1)) + + // Exercise the production shouldSkipMerge helper directly so that if runSync's + // early-return logic reverts (e.g. drops the inclNotReady guard), this test fails. + g.Expect(shouldSkipMerge(ready, notReady, false)).To(BeTrue(), "without flag: must skip merge when all clusters not-ready") + g.Expect(shouldSkipMerge(ready, notReady, true)).To(BeFalse(), "with flag: must not skip merge when all clusters not-ready") +}