From ef1291da4586d6f98bc56e32026be1dca8ca0662 Mon Sep 17 00:00:00 2001 From: onuryilmaz Date: Wed, 30 Sep 2026 13:59:36 +0200 Subject: [PATCH 01/10] feat(bootstrap): add bootstrap command for first-time Greenhouse access Adds `cloudctl bootstrap` which merges a Greenhouse kubeconfig into the user's local kubeconfig so they can reach the Greenhouse API server with kubectl and run `cloudctl sync`. Two input modes are supported: - `--data=`: decodes a standard kubeconfig downloaded from the Greenhouse Web UI; supports any auth type (OIDC auth-provider, exec-plugin, token, cert) and preserves all fields verbatim. - Individual OIDC flags: `--greenhouse-server`, `--greenhouse-org`, `--greenhouse-idp-issuer-url`, `--greenhouse-client-id`, `--greenhouse-client-secret`, `--greenhouse-extra-scopes`, `--greenhouse-ca-data`, `--greenhouse-namespace`. Produces the same auth-provider/oidc kubeconfig shape as the Greenhouse UI download. Both modes: - Ask interactively for context name and whether to set it as current context (skipped when `--context-name` / `--set-current-context` are given or when not on a TTY). - Rename the context triple (cluster + user + context) to the chosen name. - Never overwrite existing unmanaged kubeconfig entries. - Are idempotent: running twice with the same input is safe. - Support `--dry-run` to preview without writing. - Print a next-step hint: `cloudctl sync -n `. Closes #80 Signed-off-by: onuryilmaz --- cmd/bootstrap.go | 441 ++++++++++++++++ cmd/bootstrap_test.go | 823 ++++++++++++++++++++++++++++++ cmd/output/interactive_printer.go | 32 ++ cmd/output/plain_printer.go | 33 ++ cmd/output/types.go | 11 + 5 files changed, 1340 insertions(+) create mode 100644 cmd/bootstrap.go create mode 100644 cmd/bootstrap_test.go diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go new file mode 100644 index 0000000..efd741f --- /dev/null +++ b/cmd/bootstrap.go @@ -0,0 +1,441 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "bufio" + "encoding/base64" + "fmt" + "os" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + + "github.com/cloudoperators/cloudctl/cmd/output" +) + +var ( + bootstrapData string + bootstrapServer string + bootstrapOrg string + bootstrapCAData string + bootstrapIDPIssuerURL string + bootstrapClientID string + bootstrapClientSecret string + bootstrapExtraScopes string + bootstrapNamespace string + bootstrapKubeconfig string + bootstrapContextName string + bootstrapSetCurrentCtx bool + bootstrapDryRun bool +) + +func init() { + bootstrapCmd.Flags().StringVar(&bootstrapData, "data", "", "Base64-encoded kubeconfig (as downloaded from the Greenhouse Web UI)") + bootstrapCmd.Flags().StringVar(&bootstrapServer, "greenhouse-server", "", "Greenhouse API server URL (e.g. https://greenhouse.example.com)") + bootstrapCmd.Flags().StringVar(&bootstrapOrg, "greenhouse-org", "", "Greenhouse organization name") + bootstrapCmd.Flags().StringVar(&bootstrapCAData, "greenhouse-ca-data", "", "Base64-encoded CA certificate data for the Greenhouse server") + bootstrapCmd.Flags().StringVar(&bootstrapIDPIssuerURL, "greenhouse-idp-issuer-url", "", "OIDC issuer URL (e.g. https://idp.example.com)") + bootstrapCmd.Flags().StringVar(&bootstrapClientID, "greenhouse-client-id", "", "OIDC client ID") + bootstrapCmd.Flags().StringVar(&bootstrapClientSecret, "greenhouse-client-secret", "", "OIDC client secret (optional)") + bootstrapCmd.Flags().StringVar(&bootstrapExtraScopes, "greenhouse-extra-scopes", "", "Comma-separated extra OIDC scopes (optional, e.g. groups)") + bootstrapCmd.Flags().StringVar(&bootstrapNamespace, "greenhouse-namespace", "", "Kubernetes namespace to set in the context (defaults to --greenhouse-org)") + bootstrapCmd.Flags().StringVar(&bootstrapKubeconfig, "kubeconfig", clientcmd.RecommendedHomeFile, "Path to the local kubeconfig file to merge into") + bootstrapCmd.Flags().StringVar(&bootstrapContextName, "context-name", "", "Context name to use in the local kubeconfig (default: from the downloaded kubeconfig or greenhouse-)") + bootstrapCmd.Flags().BoolVar(&bootstrapSetCurrentCtx, "set-current-context", false, "Set the bootstrapped context as the current context") + bootstrapCmd.Flags().BoolVar(&bootstrapDryRun, "dry-run", false, "Preview what would be written without touching the kubeconfig file") + + _ = viper.BindPFlags(bootstrapCmd.Flags()) + + rootCmd.AddCommand(bootstrapCmd) +} + +var bootstrapCmd = &cobra.Command{ + Use: "bootstrap", + Short: "Bootstrap first-time access to a Greenhouse cluster", + Long: `Merges a Greenhouse kubeconfig into your local kubeconfig so you can +reach the Greenhouse API server with kubectl or cloudctl. + +Two input modes are supported: + + Mode 1 — kubeconfig blob from the Greenhouse Web UI (recommended): + cloudctl bootstrap --data= + + The --data value is a standard kubeconfig file base64-encoded. + You can generate it yourself with: + base64 < ~/Downloads/greenhouse-my-org.kubeconfig + + Mode 2 — individual flags (OIDC, scriptable): + cloudctl bootstrap \ + --greenhouse-server=https://greenhouse.example.com \ + --greenhouse-org=my-org \ + --greenhouse-idp-issuer-url=https://idp.example.com \ + --greenhouse-client-id=greenhouse \ + --greenhouse-extra-scopes=groups \ + --greenhouse-ca-data= + +Both modes ask interactively for the context name and whether to set it as +the current context, unless --context-name and --set-current-context are given. +Running twice with the same input is safe (idempotent). + +Examples: + # Bootstrap from a kubeconfig downloaded from the Web UI + cloudctl bootstrap --data=$(base64 < greenhouse-my-org.kubeconfig) + + # Bootstrap from individual flags (non-interactive) + cloudctl bootstrap \ + --greenhouse-server=https://greenhouse.example.com \ + --greenhouse-org=my-org \ + --greenhouse-idp-issuer-url=https://idp.example.com \ + --greenhouse-client-id=greenhouse \ + --greenhouse-extra-scopes=groups \ + --context-name=greenhouse-my-org \ + --set-current-context + + # Preview what would change without writing + cloudctl bootstrap --data= --dry-run`, + RunE: runBootstrap, +} + +func runBootstrap(cmd *cobra.Command, args []string) error { + bootstrapData = viper.GetString("data") + bootstrapServer = viper.GetString("greenhouse-server") + bootstrapOrg = viper.GetString("greenhouse-org") + bootstrapCAData = viper.GetString("greenhouse-ca-data") + bootstrapIDPIssuerURL = viper.GetString("greenhouse-idp-issuer-url") + bootstrapClientID = viper.GetString("greenhouse-client-id") + bootstrapClientSecret = viper.GetString("greenhouse-client-secret") + bootstrapExtraScopes = viper.GetString("greenhouse-extra-scopes") + bootstrapNamespace = viper.GetString("greenhouse-namespace") + // Use the flag value directly; only fall back to KUBECONFIG/default when not explicitly set. + if cmd.Flags().Changed("kubeconfig") { + bootstrapKubeconfig, _ = cmd.Flags().GetString("kubeconfig") + } else if kc := os.Getenv("KUBECONFIG"); kc != "" { + if parts := strings.SplitN(kc, string(os.PathListSeparator), 2); len(parts) > 0 && parts[0] != "" { + bootstrapKubeconfig = parts[0] + } + } else { + bootstrapKubeconfig = clientcmd.RecommendedHomeFile + } + bootstrapContextName = viper.GetString("context-name") + bootstrapSetCurrentCtx = viper.GetBool("set-current-context") + // Read dry-run directly from the flag to avoid viper cross-command pollution + // (sync also binds "dry-run" to the global viper instance). + bootstrapDryRun, _ = cmd.Flags().GetBool("dry-run") + + format, err := output.ParseFormat(viper.GetString("output")) + if err != nil { + return err + } + w := cmd.OutOrStdout() + printer := output.New(format, output.IsTTYWriter(w), w) + + incoming, org, err := resolveIncomingKubeconfig() + if err != nil { + return err + } + + // Determine default context name from the incoming config or the org. + defaultCtxName := incoming.CurrentContext + if defaultCtxName == "" && org != "" { + defaultCtxName = fmt.Sprintf("greenhouse-%s", org) + } + if defaultCtxName == "" && len(incoming.Contexts) == 1 { + for name := range incoming.Contexts { + defaultCtxName = name + } + } + if defaultCtxName == "" { + defaultCtxName = "greenhouse" + } + + contextName := bootstrapContextName + if contextName == "" { + contextName = defaultCtxName + } + + // Prompt interactively for context name when running on a TTY and the flag wasn't set. + if !cmd.Flags().Changed("context-name") && output.IsTTYWriter(w) { + contextName, err = promptContextName(contextName) + if err != nil { + return err + } + } + + setCurrentCtx := bootstrapSetCurrentCtx + if !cmd.Flags().Changed("set-current-context") && output.IsTTYWriter(w) { + setCurrentCtx, err = promptYesNo(fmt.Sprintf("Set %q as the current context?", contextName), false) + if err != nil { + return err + } + } + + // Rename entries in the incoming config to use the chosen context name. + renameKubeconfigContext(incoming, contextName) + + // Load or create the local kubeconfig. + var localConfig *clientcmdapi.Config + if bootstrapKubeconfig != "" { + if _, statErr := os.Stat(bootstrapKubeconfig); statErr == nil { + localConfig, err = clientcmd.LoadFromFile(bootstrapKubeconfig) + if err != nil { + return fmt.Errorf("failed to load local kubeconfig %q: %w", bootstrapKubeconfig, err) + } + } + } + if localConfig == nil { + localConfig = clientcmdapi.NewConfig() + } + + result, err := mergeBootstrapKubeconfig(localConfig, incoming, contextName, setCurrentCtx, org) + if err != nil { + return err + } + + if bootstrapDryRun { + result.DryRun = true + return printer.Print(result) + } + + if err := writeConfig(localConfig, bootstrapKubeconfig); err != nil { + return fmt.Errorf("failed to write kubeconfig: %w", err) + } + + result.KubeconfigPath = bootstrapKubeconfig + return printer.Print(result) +} + +// resolveIncomingKubeconfig returns a clientcmdapi.Config from either the --data blob +// or the individual --greenhouse-* flags. It also returns the org name (may be empty +// when derived from a blob with no org context). +func resolveIncomingKubeconfig() (*clientcmdapi.Config, string, error) { + if bootstrapData != "" { + raw, err := base64.StdEncoding.DecodeString(bootstrapData) + if err != nil { + raw, err = base64.RawStdEncoding.DecodeString(bootstrapData) + if err != nil { + return nil, "", fmt.Errorf("--data is not valid base64: %w", err) + } + } + cfg, err := clientcmd.Load(raw) + if err != nil { + return nil, "", fmt.Errorf("--data does not contain a valid kubeconfig: %w", err) + } + if len(cfg.Clusters) == 0 { + return nil, "", fmt.Errorf("--data kubeconfig contains no clusters") + } + return cfg, bootstrapOrg, nil + } + + // Individual flags path. + if bootstrapServer == "" { + return nil, "", fmt.Errorf("one of --data or --greenhouse-server is required") + } + if bootstrapOrg == "" { + return nil, "", fmt.Errorf("--greenhouse-org is required when not using --data") + } + if bootstrapIDPIssuerURL == "" { + return nil, "", fmt.Errorf("--greenhouse-idp-issuer-url is required when not using --data") + } + if bootstrapClientID == "" { + return nil, "", fmt.Errorf("--greenhouse-client-id is required when not using --data") + } + + cfg, err := buildOIDCKubeconfig(bootstrapServer, bootstrapOrg, bootstrapCAData, bootstrapIDPIssuerURL, bootstrapClientID, bootstrapClientSecret, bootstrapExtraScopes, bootstrapNamespace) + if err != nil { + return nil, "", err + } + return cfg, bootstrapOrg, nil +} + +// buildOIDCKubeconfig constructs a kubeconfig matching the Greenhouse auth-provider shape: +// +// users: +// - name: greenhouse- +// user: +// auth-provider: +// name: oidc +// config: +// idp-issuer-url: ... +// client-id: ... +// client-secret: ... (omitted when empty) +// extra-scopes: ... (omitted when empty) +func buildOIDCKubeconfig(server, org, caDataB64, idpIssuerURL, clientID, clientSecret, extraScopes, namespace string) (*clientcmdapi.Config, error) { + name := fmt.Sprintf("greenhouse-%s", org) + + cluster := &clientcmdapi.Cluster{Server: server} + if caDataB64 != "" { + caBytes, err := base64.StdEncoding.DecodeString(caDataB64) + if err != nil { + caBytes, err = base64.RawStdEncoding.DecodeString(caDataB64) + if err != nil { + return nil, fmt.Errorf("--greenhouse-ca-data is not valid base64: %w", err) + } + } + cluster.CertificateAuthorityData = caBytes + } + + oidcConfig := map[string]string{ + "idp-issuer-url": idpIssuerURL, + "client-id": clientID, + "client-secret": clientSecret, + } + if extraScopes != "" { + oidcConfig["extra-scopes"] = extraScopes + } + + ns := namespace + if ns == "" { + ns = org + } + + cfg := clientcmdapi.NewConfig() + cfg.Clusters[name] = cluster + cfg.AuthInfos[name] = &clientcmdapi.AuthInfo{ + AuthProvider: &clientcmdapi.AuthProviderConfig{ + Name: "oidc", + Config: oidcConfig, + }, + } + cfg.Contexts[name] = &clientcmdapi.Context{ + Cluster: name, + AuthInfo: name, + Namespace: ns, + } + cfg.CurrentContext = name + return cfg, nil +} + +// renameKubeconfigContext renames the active context (and its referenced cluster/authinfo) +// in cfg to targetName. When there is exactly one context it is always the one renamed. +// All other entries (multiple contexts in a blob) are left untouched. +func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { + // Identify which context to rename: prefer CurrentContext, fall back to the only one. + source := cfg.CurrentContext + if _, ok := cfg.Contexts[source]; !ok { + if len(cfg.Contexts) == 1 { + for name := range cfg.Contexts { + source = name + } + } + } + if source == "" || source == targetName { + // Nothing to rename, but ensure CurrentContext points to the target. + cfg.CurrentContext = targetName + return + } + + ctx := cfg.Contexts[source] + if ctx == nil { + return + } + + oldCluster := ctx.Cluster + oldAuth := ctx.AuthInfo + + // Rename cluster. + if cl, ok := cfg.Clusters[oldCluster]; ok && oldCluster != targetName { + cfg.Clusters[targetName] = cl + delete(cfg.Clusters, oldCluster) + ctx.Cluster = targetName + } + + // Rename authinfo. + if ai, ok := cfg.AuthInfos[oldAuth]; ok && oldAuth != targetName { + cfg.AuthInfos[targetName] = ai + delete(cfg.AuthInfos, oldAuth) + ctx.AuthInfo = targetName + } + + // Rename context. + cfg.Contexts[targetName] = ctx + delete(cfg.Contexts, source) + cfg.CurrentContext = targetName +} + +// mergeBootstrapKubeconfig merges the incoming config into localConfig. +// Existing entries are never overwritten. +func mergeBootstrapKubeconfig(localConfig, incoming *clientcmdapi.Config, ctxName string, setCurrentCtx bool, org string) (output.BootstrapResult, error) { + result := output.BootstrapResult{ + ContextName: ctxName, + SetAsCurrent: setCurrentCtx, + Org: org, + } + + for name, cluster := range incoming.Clusters { + if _, exists := localConfig.Clusters[name]; !exists { + localConfig.Clusters[name] = cluster + result.Added = append(result.Added, fmt.Sprintf("cluster %q", name)) + } else { + result.Skipped = append(result.Skipped, fmt.Sprintf("cluster %q (already exists)", name)) + } + } + + for name, auth := range incoming.AuthInfos { + if _, exists := localConfig.AuthInfos[name]; !exists { + localConfig.AuthInfos[name] = auth + result.Added = append(result.Added, fmt.Sprintf("user %q", name)) + } else { + result.Skipped = append(result.Skipped, fmt.Sprintf("user %q (already exists)", name)) + } + } + + for name, ctx := range incoming.Contexts { + if _, exists := localConfig.Contexts[name]; !exists { + localConfig.Contexts[name] = ctx + result.Added = append(result.Added, fmt.Sprintf("context %q", name)) + } else { + result.Skipped = append(result.Skipped, fmt.Sprintf("context %q (already exists)", name)) + } + } + + if setCurrentCtx { + localConfig.CurrentContext = ctxName + } + + return result, nil +} + +// promptContextName reads a context name from stdin, returning defaultName on empty input. +func promptContextName(defaultName string) (string, error) { + fmt.Fprintf(os.Stderr, "Context name [%s]: ", defaultName) + scanner := bufio.NewScanner(os.Stdin) + if !scanner.Scan() { + if err := scanner.Err(); err != nil { + return "", fmt.Errorf("failed to read context name: %w", err) + } + return defaultName, nil + } + if val := strings.TrimSpace(scanner.Text()); val != "" { + return val, nil + } + return defaultName, nil +} + +// promptYesNo asks a yes/no question; defaultVal is used on empty input or unrecognised input. +func promptYesNo(question string, defaultVal bool) (bool, error) { + hint := "y/N" + if defaultVal { + hint = "Y/n" + } + fmt.Fprintf(os.Stderr, "%s [%s]: ", question, hint) + scanner := bufio.NewScanner(os.Stdin) + if !scanner.Scan() { + if err := scanner.Err(); err != nil { + return false, fmt.Errorf("failed to read answer: %w", err) + } + return defaultVal, nil + } + switch strings.TrimSpace(strings.ToLower(scanner.Text())) { + case "y", "yes": + return true, nil + case "n", "no": + return false, nil + default: + return defaultVal, nil + } +} diff --git a/cmd/bootstrap_test.go b/cmd/bootstrap_test.go new file mode 100644 index 0000000..a9a4e3f --- /dev/null +++ b/cmd/bootstrap_test.go @@ -0,0 +1,823 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "bytes" + "encoding/base64" + "os" + "path/filepath" + "strings" + "testing" + + . "github.com/onsi/gomega" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" +) + +// ── helpers ────────────────────────────────────────────────────────────────── + +// realGreenHouseKubeconfig returns a kubeconfig that matches exactly what +// Greenhouse serves for an organization: OIDC auth-provider, CA data, namespace. +func realGreenhouseKubeconfig(org string) *clientcmdapi.Config { + name := "greenhouse-" + org + cfg := clientcmdapi.NewConfig() + cfg.Clusters[name] = &clientcmdapi.Cluster{ + Server: "https://greenhouse.global.cloud.sap", + CertificateAuthorityData: []byte("fake-ca-data"), + } + cfg.AuthInfos[name] = &clientcmdapi.AuthInfo{ + AuthProvider: &clientcmdapi.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "idp-issuer-url": "https://idp.global.cloud.sap", + "client-id": "greenhouse", + "client-secret": "", + "extra-scopes": "groups", + }, + }, + } + cfg.Contexts[name] = &clientcmdapi.Context{ + Cluster: name, + AuthInfo: name, + Namespace: org, + } + cfg.CurrentContext = name + return cfg +} + +// encodeKubeconfig serialises cfg to YAML and base64-encodes it. +func encodeKubeconfig(t *testing.T, cfg *clientcmdapi.Config) string { + t.Helper() + raw, err := clientcmd.Write(*cfg) + if err != nil { + t.Fatalf("encodeKubeconfig: %v", err) + } + return base64.StdEncoding.EncodeToString(raw) +} + +// writeTempKubeconfig writes cfg to a temp file and returns the path. +func writeTempKubeconfig(t *testing.T, cfg *clientcmdapi.Config) string { + t.Helper() + f, err := os.CreateTemp(t.TempDir(), "kubeconfig-*.yaml") + if err != nil { + t.Fatalf("writeTempKubeconfig: %v", err) + } + raw, err := clientcmd.Write(*cfg) + if err != nil { + t.Fatalf("writeTempKubeconfig write: %v", err) + } + if _, err := f.Write(raw); err != nil { + t.Fatalf("writeTempKubeconfig: %v", err) + } + _ = f.Close() + return f.Name() +} + +// loadKubeconfig loads a kubeconfig from disk. +func loadKubeconfig(t *testing.T, path string) *clientcmdapi.Config { + t.Helper() + cfg, err := clientcmd.LoadFromFile(path) + if err != nil { + t.Fatalf("loadKubeconfig: %v", err) + } + return cfg +} + +// runBootstrapCmd executes the bootstrap cobra command with the given args and +// returns stdout, stderr, and any error. It resets global flag vars before each run. +func runBootstrapCmd(t *testing.T, args []string) (stdout, stderr string, err error) { + t.Helper() + + // Reset package-level flag vars so tests are isolated from each other. + bootstrapData = "" + bootstrapServer = "" + bootstrapOrg = "" + bootstrapCAData = "" + bootstrapIDPIssuerURL = "" + bootstrapClientID = "" + bootstrapClientSecret = "" + bootstrapExtraScopes = "" + bootstrapNamespace = "" + bootstrapKubeconfig = "" + bootstrapContextName = "" + bootstrapSetCurrentCtx = false + bootstrapDryRun = false + + outBuf := &bytes.Buffer{} + errBuf := &bytes.Buffer{} + + rootCmd.SetOut(outBuf) + rootCmd.SetErr(errBuf) + t.Cleanup(func() { + rootCmd.SetOut(nil) + rootCmd.SetErr(nil) + }) + + rootCmd.SetArgs(append([]string{"bootstrap"}, args...)) + err = rootCmd.Execute() + return outBuf.String(), errBuf.String(), err +} + +// ── buildOIDCKubeconfig ─────────────────────────────────────────────────────── + +func TestBuildOIDCKubeconfig_AllFields(t *testing.T) { + g := NewWithT(t) + + caB64 := base64.StdEncoding.EncodeToString([]byte("fake-ca")) + cfg, err := buildOIDCKubeconfig( + "https://greenhouse.example.com", + "my-org", + caB64, + "https://idp.example.com", + "greenhouse", + "secret123", + "groups", + "", + ) + g.Expect(err).To(BeNil()) + + name := "greenhouse-my-org" + g.Expect(cfg.CurrentContext).To(Equal(name)) + + cl := cfg.Clusters[name] + g.Expect(cl).NotTo(BeNil()) + g.Expect(cl.Server).To(Equal("https://greenhouse.example.com")) + g.Expect(cl.CertificateAuthorityData).To(Equal([]byte("fake-ca"))) + + ai := cfg.AuthInfos[name] + g.Expect(ai).NotTo(BeNil()) + g.Expect(ai.AuthProvider).NotTo(BeNil()) + g.Expect(ai.AuthProvider.Name).To(Equal("oidc")) + g.Expect(ai.AuthProvider.Config["idp-issuer-url"]).To(Equal("https://idp.example.com")) + g.Expect(ai.AuthProvider.Config["client-id"]).To(Equal("greenhouse")) + g.Expect(ai.AuthProvider.Config["client-secret"]).To(Equal("secret123")) + g.Expect(ai.AuthProvider.Config["extra-scopes"]).To(Equal("groups")) + + ctx := cfg.Contexts[name] + g.Expect(ctx).NotTo(BeNil()) + g.Expect(ctx.Cluster).To(Equal(name)) + g.Expect(ctx.AuthInfo).To(Equal(name)) + g.Expect(ctx.Namespace).To(Equal("my-org")) // defaults to org +} + +func TestBuildOIDCKubeconfig_ExplicitNamespace(t *testing.T) { + g := NewWithT(t) + cfg, err := buildOIDCKubeconfig( + "https://greenhouse.example.com", "my-org", "", + "https://idp.example.com", "greenhouse", "", "", "custom-ns", + ) + g.Expect(err).To(BeNil()) + g.Expect(cfg.Contexts["greenhouse-my-org"].Namespace).To(Equal("custom-ns")) +} + +func TestBuildOIDCKubeconfig_NoExtraScopes(t *testing.T) { + g := NewWithT(t) + cfg, err := buildOIDCKubeconfig( + "https://greenhouse.example.com", "my-org", "", + "https://idp.example.com", "greenhouse", "", "", "", + ) + g.Expect(err).To(BeNil()) + _, hasScopes := cfg.AuthInfos["greenhouse-my-org"].AuthProvider.Config["extra-scopes"] + g.Expect(hasScopes).To(BeFalse()) +} + +func TestBuildOIDCKubeconfig_InvalidCAData(t *testing.T) { + g := NewWithT(t) + _, err := buildOIDCKubeconfig( + "https://greenhouse.example.com", "my-org", "not-valid-base64!!!", + "https://idp.example.com", "greenhouse", "", "", "", + ) + g.Expect(err).To(MatchError(ContainSubstring("not valid base64"))) +} + +func TestBuildOIDCKubeconfig_MatchesRealGreenhouseShape(t *testing.T) { + g := NewWithT(t) + + caB64 := base64.StdEncoding.EncodeToString([]byte("fake-ca-data")) + cfg, err := buildOIDCKubeconfig( + "https://greenhouse.global.cloud.sap", + "sap-cna", + caB64, + "https://idp.global.cloud.sap", + "greenhouse", + "", + "groups", + "", + ) + g.Expect(err).To(BeNil()) + + want := realGreenhouseKubeconfig("sap-cna") + + name := "greenhouse-sap-cna" + g.Expect(cfg.Clusters[name].Server).To(Equal(want.Clusters[name].Server)) + g.Expect(cfg.Clusters[name].CertificateAuthorityData).To(Equal(want.Clusters[name].CertificateAuthorityData)) + g.Expect(cfg.AuthInfos[name].AuthProvider.Name).To(Equal(want.AuthInfos[name].AuthProvider.Name)) + g.Expect(cfg.AuthInfos[name].AuthProvider.Config).To(Equal(want.AuthInfos[name].AuthProvider.Config)) + g.Expect(cfg.Contexts[name].Namespace).To(Equal(want.Contexts[name].Namespace)) + g.Expect(cfg.CurrentContext).To(Equal(want.CurrentContext)) +} + +// ── renameKubeconfigContext ─────────────────────────────────────────────────── + +func TestRenameKubeconfigContext_RenamesAll(t *testing.T) { + g := NewWithT(t) + + cfg := realGreenhouseKubeconfig("sap-cna") + renameKubeconfigContext(cfg, "my-custom-name") + + g.Expect(cfg.CurrentContext).To(Equal("my-custom-name")) + g.Expect(cfg.Contexts).To(HaveKey("my-custom-name")) + g.Expect(cfg.Contexts).NotTo(HaveKey("greenhouse-sap-cna")) + g.Expect(cfg.Clusters).To(HaveKey("my-custom-name")) + g.Expect(cfg.Clusters).NotTo(HaveKey("greenhouse-sap-cna")) + g.Expect(cfg.AuthInfos).To(HaveKey("my-custom-name")) + g.Expect(cfg.AuthInfos).NotTo(HaveKey("greenhouse-sap-cna")) + + ctx := cfg.Contexts["my-custom-name"] + g.Expect(ctx.Cluster).To(Equal("my-custom-name")) + g.Expect(ctx.AuthInfo).To(Equal("my-custom-name")) + g.Expect(ctx.Namespace).To(Equal("sap-cna")) // namespace unchanged +} + +func TestRenameKubeconfigContext_NoOpWhenNameUnchanged(t *testing.T) { + g := NewWithT(t) + + cfg := realGreenhouseKubeconfig("sap-cna") + renameKubeconfigContext(cfg, "greenhouse-sap-cna") + + g.Expect(cfg.CurrentContext).To(Equal("greenhouse-sap-cna")) + g.Expect(cfg.Contexts).To(HaveKey("greenhouse-sap-cna")) + g.Expect(cfg.Clusters).To(HaveKey("greenhouse-sap-cna")) + g.Expect(cfg.AuthInfos).To(HaveKey("greenhouse-sap-cna")) +} + +func TestRenameKubeconfigContext_MultiContextBlobOnlyRenamesCurrent(t *testing.T) { + g := NewWithT(t) + + // A blob with two contexts — only the current one should be renamed. + cfg := clientcmdapi.NewConfig() + cfg.Clusters["cluster-a"] = &clientcmdapi.Cluster{Server: "https://a.example.com"} + cfg.Clusters["cluster-b"] = &clientcmdapi.Cluster{Server: "https://b.example.com"} + cfg.AuthInfos["user-a"] = &clientcmdapi.AuthInfo{} + cfg.AuthInfos["user-b"] = &clientcmdapi.AuthInfo{} + cfg.Contexts["ctx-a"] = &clientcmdapi.Context{Cluster: "cluster-a", AuthInfo: "user-a"} + cfg.Contexts["ctx-b"] = &clientcmdapi.Context{Cluster: "cluster-b", AuthInfo: "user-b"} + cfg.CurrentContext = "ctx-a" + + renameKubeconfigContext(cfg, "gh-prod") + + // ctx-a → gh-prod; ctx-b untouched + g.Expect(cfg.Contexts).To(HaveKey("gh-prod")) + g.Expect(cfg.Contexts).NotTo(HaveKey("ctx-a")) + g.Expect(cfg.Contexts).To(HaveKey("ctx-b")) + g.Expect(cfg.Clusters).To(HaveKey("gh-prod")) + g.Expect(cfg.Clusters).To(HaveKey("cluster-b")) + g.Expect(cfg.AuthInfos).To(HaveKey("gh-prod")) + g.Expect(cfg.AuthInfos).To(HaveKey("user-b")) + g.Expect(cfg.CurrentContext).To(Equal("gh-prod")) +} + +// ── mergeBootstrapKubeconfig ────────────────────────────────────────────────── + +func TestMergeBootstrapKubeconfig_AddsAllEntries(t *testing.T) { + g := NewWithT(t) + + local := clientcmdapi.NewConfig() + incoming := realGreenhouseKubeconfig("sap-cna") + + result, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", false, "sap-cna") + g.Expect(err).To(BeNil()) + + g.Expect(result.Added).To(HaveLen(3)) + g.Expect(result.Skipped).To(BeEmpty()) + g.Expect(local.Clusters).To(HaveKey("greenhouse-sap-cna")) + g.Expect(local.AuthInfos).To(HaveKey("greenhouse-sap-cna")) + g.Expect(local.Contexts).To(HaveKey("greenhouse-sap-cna")) + g.Expect(local.CurrentContext).To(BeEmpty()) // setCurrentCtx=false +} + +func TestMergeBootstrapKubeconfig_SetsCurrentContext(t *testing.T) { + g := NewWithT(t) + + local := clientcmdapi.NewConfig() + incoming := realGreenhouseKubeconfig("sap-cna") + + _, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", true, "sap-cna") + g.Expect(err).To(BeNil()) + g.Expect(local.CurrentContext).To(Equal("greenhouse-sap-cna")) +} + +func TestMergeBootstrapKubeconfig_IdempotentSkipsExisting(t *testing.T) { + g := NewWithT(t) + + incoming := realGreenhouseKubeconfig("sap-cna") + local := realGreenhouseKubeconfig("sap-cna") // same entries already present + + result, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", false, "sap-cna") + g.Expect(err).To(BeNil()) + g.Expect(result.Added).To(BeEmpty()) + g.Expect(result.Skipped).To(HaveLen(3)) +} + +func TestMergeBootstrapKubeconfig_NeverOverwritesExistingEntries(t *testing.T) { + g := NewWithT(t) + + // Local has an existing cluster entry with a different server URL. + local := clientcmdapi.NewConfig() + local.Clusters["greenhouse-sap-cna"] = &clientcmdapi.Cluster{Server: "https://original.example.com"} + + incoming := realGreenhouseKubeconfig("sap-cna") + _, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", false, "sap-cna") + g.Expect(err).To(BeNil()) + + // Original server must not be overwritten. + g.Expect(local.Clusters["greenhouse-sap-cna"].Server).To(Equal("https://original.example.com")) +} + +func TestMergeBootstrapKubeconfig_PreservesExistingLocalEntries(t *testing.T) { + g := NewWithT(t) + + // Local already has unrelated entries. + local := clientcmdapi.NewConfig() + local.Clusters["other-cluster"] = &clientcmdapi.Cluster{Server: "https://other.example.com"} + local.AuthInfos["other-user"] = &clientcmdapi.AuthInfo{Token: "tok"} + local.Contexts["other-ctx"] = &clientcmdapi.Context{Cluster: "other-cluster", AuthInfo: "other-user"} + local.CurrentContext = "other-ctx" + + incoming := realGreenhouseKubeconfig("sap-cna") + _, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", false, "sap-cna") + g.Expect(err).To(BeNil()) + + // Unrelated entries must still be there. + g.Expect(local.Clusters).To(HaveKey("other-cluster")) + g.Expect(local.AuthInfos).To(HaveKey("other-user")) + g.Expect(local.Contexts).To(HaveKey("other-ctx")) + // Current context unchanged because setCurrentCtx=false. + g.Expect(local.CurrentContext).To(Equal("other-ctx")) +} + +// ── resolveIncomingKubeconfig (via package-level vars) ─────────────────────── + +func TestResolveIncomingKubeconfig_DataBlob(t *testing.T) { + g := NewWithT(t) + + want := realGreenhouseKubeconfig("sap-cna") + bootstrapData = encodeKubeconfig(t, want) + bootstrapOrg = "" + t.Cleanup(func() { bootstrapData = ""; bootstrapOrg = "" }) + + cfg, org, err := resolveIncomingKubeconfig() + g.Expect(err).To(BeNil()) + g.Expect(org).To(BeEmpty()) // no --greenhouse-org provided alongside --data + g.Expect(cfg.Clusters).To(HaveKey("greenhouse-sap-cna")) + g.Expect(cfg.AuthInfos["greenhouse-sap-cna"].AuthProvider.Name).To(Equal("oidc")) + g.Expect(cfg.AuthInfos["greenhouse-sap-cna"].AuthProvider.Config["idp-issuer-url"]). + To(Equal("https://idp.global.cloud.sap")) + g.Expect(cfg.Contexts["greenhouse-sap-cna"].Namespace).To(Equal("sap-cna")) +} + +func TestResolveIncomingKubeconfig_DataBlobWithOrgOverride(t *testing.T) { + g := NewWithT(t) + + want := realGreenhouseKubeconfig("sap-cna") + bootstrapData = encodeKubeconfig(t, want) + bootstrapOrg = "override-org" + t.Cleanup(func() { bootstrapData = ""; bootstrapOrg = "" }) + + _, org, err := resolveIncomingKubeconfig() + g.Expect(err).To(BeNil()) + g.Expect(org).To(Equal("override-org")) +} + +func TestResolveIncomingKubeconfig_DataBlobInvalidBase64(t *testing.T) { + g := NewWithT(t) + + bootstrapData = "not!!!base64" + t.Cleanup(func() { bootstrapData = "" }) + + _, _, err := resolveIncomingKubeconfig() + g.Expect(err).To(MatchError(ContainSubstring("not valid base64"))) +} + +func TestResolveIncomingKubeconfig_DataBlobNotKubeconfig(t *testing.T) { + g := NewWithT(t) + + bootstrapData = base64.StdEncoding.EncodeToString([]byte("just some text, not yaml")) + t.Cleanup(func() { bootstrapData = "" }) + + _, _, err := resolveIncomingKubeconfig() + g.Expect(err).To(MatchError(ContainSubstring("valid kubeconfig"))) +} + +func TestResolveIncomingKubeconfig_DataBlobEmptyClusters(t *testing.T) { + g := NewWithT(t) + + empty := clientcmdapi.NewConfig() + bootstrapData = encodeKubeconfig(t, empty) + t.Cleanup(func() { bootstrapData = "" }) + + _, _, err := resolveIncomingKubeconfig() + g.Expect(err).To(MatchError(ContainSubstring("no clusters"))) +} + +func TestResolveIncomingKubeconfig_IndividualFlags(t *testing.T) { + g := NewWithT(t) + + bootstrapServer = "https://greenhouse.example.com" + bootstrapOrg = "my-org" + bootstrapIDPIssuerURL = "https://idp.example.com" + bootstrapClientID = "greenhouse" + bootstrapClientSecret = "" + bootstrapExtraScopes = "groups" + bootstrapNamespace = "" + t.Cleanup(func() { + bootstrapServer = "" + bootstrapOrg = "" + bootstrapIDPIssuerURL = "" + bootstrapClientID = "" + bootstrapClientSecret = "" + bootstrapExtraScopes = "" + bootstrapNamespace = "" + }) + + cfg, org, err := resolveIncomingKubeconfig() + g.Expect(err).To(BeNil()) + g.Expect(org).To(Equal("my-org")) + + name := "greenhouse-my-org" + g.Expect(cfg.Clusters[name].Server).To(Equal("https://greenhouse.example.com")) + g.Expect(cfg.AuthInfos[name].AuthProvider.Name).To(Equal("oidc")) + g.Expect(cfg.AuthInfos[name].AuthProvider.Config["idp-issuer-url"]).To(Equal("https://idp.example.com")) + g.Expect(cfg.AuthInfos[name].AuthProvider.Config["client-id"]).To(Equal("greenhouse")) + g.Expect(cfg.AuthInfos[name].AuthProvider.Config["extra-scopes"]).To(Equal("groups")) + g.Expect(cfg.Contexts[name].Namespace).To(Equal("my-org")) +} + +func TestResolveIncomingKubeconfig_MissingServer(t *testing.T) { + g := NewWithT(t) + bootstrapOrg = "my-org" + bootstrapIDPIssuerURL = "https://idp.example.com" + bootstrapClientID = "greenhouse" + t.Cleanup(func() { bootstrapOrg = ""; bootstrapIDPIssuerURL = ""; bootstrapClientID = "" }) + + _, _, err := resolveIncomingKubeconfig() + g.Expect(err).To(MatchError(ContainSubstring("--greenhouse-server"))) +} + +func TestResolveIncomingKubeconfig_MissingOrg(t *testing.T) { + g := NewWithT(t) + bootstrapServer = "https://greenhouse.example.com" + bootstrapIDPIssuerURL = "https://idp.example.com" + bootstrapClientID = "greenhouse" + t.Cleanup(func() { bootstrapServer = ""; bootstrapIDPIssuerURL = ""; bootstrapClientID = "" }) + + _, _, err := resolveIncomingKubeconfig() + g.Expect(err).To(MatchError(ContainSubstring("--greenhouse-org"))) +} + +func TestResolveIncomingKubeconfig_MissingIDPIssuerURL(t *testing.T) { + g := NewWithT(t) + bootstrapServer = "https://greenhouse.example.com" + bootstrapOrg = "my-org" + bootstrapClientID = "greenhouse" + t.Cleanup(func() { bootstrapServer = ""; bootstrapOrg = ""; bootstrapClientID = "" }) + + _, _, err := resolveIncomingKubeconfig() + g.Expect(err).To(MatchError(ContainSubstring("--greenhouse-idp-issuer-url"))) +} + +func TestResolveIncomingKubeconfig_MissingClientID(t *testing.T) { + g := NewWithT(t) + bootstrapServer = "https://greenhouse.example.com" + bootstrapOrg = "my-org" + bootstrapIDPIssuerURL = "https://idp.example.com" + t.Cleanup(func() { bootstrapServer = ""; bootstrapOrg = ""; bootstrapIDPIssuerURL = "" }) + + _, _, err := resolveIncomingKubeconfig() + g.Expect(err).To(MatchError(ContainSubstring("--greenhouse-client-id"))) +} + +// ── end-to-end via cobra command ────────────────────────────────────────────── + +// newEmptyKubeconfigFile creates a temp file with an empty (but valid) kubeconfig. +func newEmptyKubeconfigFile(t *testing.T) string { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "kubeconfig") + empty := clientcmdapi.NewConfig() + raw, err := clientcmd.Write(*empty) + if err != nil { + t.Fatalf("newEmptyKubeconfigFile: %v", err) + } + if err := os.WriteFile(path, raw, 0o600); err != nil { + t.Fatalf("newEmptyKubeconfigFile: %v", err) + } + return path +} + +func TestBootstrapCmd_DataBlob_WritesCorrectKubeconfig(t *testing.T) { + g := NewWithT(t) + + src := realGreenhouseKubeconfig("sap-cna") + data := encodeKubeconfig(t, src) + dest := newEmptyKubeconfigFile(t) + + stdout, _, err := runBootstrapCmd(t, []string{ + "--data=" + data, + "--context-name=greenhouse-sap-cna", + "--set-current-context", + "--kubeconfig=" + dest, + }) + g.Expect(err).To(BeNil()) + g.Expect(stdout).To(ContainSubstring(`[+] cluster "greenhouse-sap-cna"`)) + g.Expect(stdout).To(ContainSubstring(`[+] user "greenhouse-sap-cna"`)) + g.Expect(stdout).To(ContainSubstring(`[+] context "greenhouse-sap-cna"`)) + g.Expect(stdout).To(ContainSubstring("Bootstrap complete.")) + g.Expect(stdout).To(ContainSubstring("cloudctl sync -n sap-cna")) + + result := loadKubeconfig(t, dest) + g.Expect(result.CurrentContext).To(Equal("greenhouse-sap-cna")) + + cl := result.Clusters["greenhouse-sap-cna"] + g.Expect(cl).NotTo(BeNil()) + g.Expect(cl.Server).To(Equal("https://greenhouse.global.cloud.sap")) + g.Expect(cl.CertificateAuthorityData).To(Equal([]byte("fake-ca-data"))) + + ai := result.AuthInfos["greenhouse-sap-cna"] + g.Expect(ai).NotTo(BeNil()) + g.Expect(ai.AuthProvider.Name).To(Equal("oidc")) + g.Expect(ai.AuthProvider.Config["idp-issuer-url"]).To(Equal("https://idp.global.cloud.sap")) + g.Expect(ai.AuthProvider.Config["client-id"]).To(Equal("greenhouse")) + g.Expect(ai.AuthProvider.Config["client-secret"]).To(Equal("")) + g.Expect(ai.AuthProvider.Config["extra-scopes"]).To(Equal("groups")) + + ctx := result.Contexts["greenhouse-sap-cna"] + g.Expect(ctx).NotTo(BeNil()) + g.Expect(ctx.Cluster).To(Equal("greenhouse-sap-cna")) + g.Expect(ctx.AuthInfo).To(Equal("greenhouse-sap-cna")) + g.Expect(ctx.Namespace).To(Equal("sap-cna")) +} + +func TestBootstrapCmd_IndividualFlags_WritesOIDCKubeconfig(t *testing.T) { + g := NewWithT(t) + + caB64 := base64.StdEncoding.EncodeToString([]byte("fake-ca-data")) + dest := newEmptyKubeconfigFile(t) + + _, _, err := runBootstrapCmd(t, []string{ + "--greenhouse-server=https://greenhouse.global.cloud.sap", + "--greenhouse-org=sap-cna", + "--greenhouse-idp-issuer-url=https://idp.global.cloud.sap", + "--greenhouse-client-id=greenhouse", + "--greenhouse-client-secret=", + "--greenhouse-extra-scopes=groups", + "--greenhouse-ca-data=" + caB64, + "--context-name=greenhouse-sap-cna", + "--set-current-context", + "--kubeconfig=" + dest, + }) + g.Expect(err).To(BeNil()) + + result := loadKubeconfig(t, dest) + cl := result.Clusters["greenhouse-sap-cna"] + g.Expect(cl.Server).To(Equal("https://greenhouse.global.cloud.sap")) + g.Expect(cl.CertificateAuthorityData).To(Equal([]byte("fake-ca-data"))) + + ai := result.AuthInfos["greenhouse-sap-cna"] + g.Expect(ai.AuthProvider.Name).To(Equal("oidc")) + g.Expect(ai.AuthProvider.Config["idp-issuer-url"]).To(Equal("https://idp.global.cloud.sap")) + g.Expect(ai.AuthProvider.Config["client-id"]).To(Equal("greenhouse")) + g.Expect(ai.AuthProvider.Config["extra-scopes"]).To(Equal("groups")) + + ctx := result.Contexts["greenhouse-sap-cna"] + g.Expect(ctx.Namespace).To(Equal("sap-cna")) + g.Expect(result.CurrentContext).To(Equal("greenhouse-sap-cna")) +} + +func TestBootstrapCmd_IndividualFlags_DataAndIndividualFlagsProduceSameShape(t *testing.T) { + g := NewWithT(t) + + // Build via individual flags. + caB64 := base64.StdEncoding.EncodeToString([]byte("fake-ca-data")) + dest1 := newEmptyKubeconfigFile(t) + _, _, err := runBootstrapCmd(t, []string{ + "--greenhouse-server=https://greenhouse.global.cloud.sap", + "--greenhouse-org=sap-cna", + "--greenhouse-idp-issuer-url=https://idp.global.cloud.sap", + "--greenhouse-client-id=greenhouse", + "--greenhouse-client-secret=", + "--greenhouse-extra-scopes=groups", + "--greenhouse-ca-data=" + caB64, + "--context-name=greenhouse-sap-cna", + "--kubeconfig=" + dest1, + }) + g.Expect(err).To(BeNil()) + + // Build via --data blob from the same source config. + src := realGreenhouseKubeconfig("sap-cna") + data := encodeKubeconfig(t, src) + dest2 := newEmptyKubeconfigFile(t) + _, _, err = runBootstrapCmd(t, []string{ + "--data=" + data, + "--context-name=greenhouse-sap-cna", + "--kubeconfig=" + dest2, + }) + g.Expect(err).To(BeNil()) + + r1 := loadKubeconfig(t, dest1) + r2 := loadKubeconfig(t, dest2) + + cl1 := r1.Clusters["greenhouse-sap-cna"] + cl2 := r2.Clusters["greenhouse-sap-cna"] + g.Expect(cl1.Server).To(Equal(cl2.Server)) + g.Expect(cl1.CertificateAuthorityData).To(Equal(cl2.CertificateAuthorityData)) + + ai1 := r1.AuthInfos["greenhouse-sap-cna"] + ai2 := r2.AuthInfos["greenhouse-sap-cna"] + g.Expect(ai1.AuthProvider.Name).To(Equal(ai2.AuthProvider.Name)) + g.Expect(ai1.AuthProvider.Config).To(Equal(ai2.AuthProvider.Config)) + + ctx1 := r1.Contexts["greenhouse-sap-cna"] + ctx2 := r2.Contexts["greenhouse-sap-cna"] + g.Expect(ctx1.Namespace).To(Equal(ctx2.Namespace)) +} + +func TestBootstrapCmd_DryRun_DoesNotWriteFile(t *testing.T) { + g := NewWithT(t) + + src := realGreenhouseKubeconfig("sap-cna") + data := encodeKubeconfig(t, src) + dest := newEmptyKubeconfigFile(t) + + before, err := os.ReadFile(dest) + g.Expect(err).To(BeNil()) + + stdout, _, err := runBootstrapCmd(t, []string{ + "--data=" + data, + "--context-name=greenhouse-sap-cna", + "--kubeconfig=" + dest, + "--dry-run", + }) + g.Expect(err).To(BeNil()) + g.Expect(stdout).To(ContainSubstring("Dry-run")) + g.Expect(stdout).To(ContainSubstring(`[+] cluster "greenhouse-sap-cna"`)) + + after, err := os.ReadFile(dest) + g.Expect(err).To(BeNil()) + g.Expect(after).To(Equal(before)) // file must be unchanged +} + +func TestBootstrapCmd_Idempotent(t *testing.T) { + g := NewWithT(t) + + src := realGreenhouseKubeconfig("sap-cna") + data := encodeKubeconfig(t, src) + dest := newEmptyKubeconfigFile(t) + + args := []string{ + "--data=" + data, + "--context-name=greenhouse-sap-cna", + "--kubeconfig=" + dest, + } + + _, _, err := runBootstrapCmd(t, args) + g.Expect(err).To(BeNil()) + + // Second run must succeed and report all entries as skipped. + stdout, _, err := runBootstrapCmd(t, args) + g.Expect(err).To(BeNil()) + g.Expect(stdout).To(ContainSubstring(`[=] cluster "greenhouse-sap-cna" (already exists)`)) + g.Expect(stdout).To(ContainSubstring(`[=] user "greenhouse-sap-cna" (already exists)`)) + g.Expect(stdout).To(ContainSubstring(`[=] context "greenhouse-sap-cna" (already exists)`)) + g.Expect(stdout).NotTo(ContainSubstring("[+]")) +} + +func TestBootstrapCmd_ContextRename(t *testing.T) { + g := NewWithT(t) + + src := realGreenhouseKubeconfig("sap-cna") // default name: greenhouse-sap-cna + data := encodeKubeconfig(t, src) + dest := newEmptyKubeconfigFile(t) + + _, _, err := runBootstrapCmd(t, []string{ + "--data=" + data, + "--context-name=gh-prod", // user picks a custom name + "--set-current-context", + "--kubeconfig=" + dest, + }) + g.Expect(err).To(BeNil()) + + result := loadKubeconfig(t, dest) + g.Expect(result.CurrentContext).To(Equal("gh-prod")) + g.Expect(result.Clusters).To(HaveKey("gh-prod")) + g.Expect(result.AuthInfos).To(HaveKey("gh-prod")) + g.Expect(result.Contexts).To(HaveKey("gh-prod")) + g.Expect(result.Clusters).NotTo(HaveKey("greenhouse-sap-cna")) + // Namespace preserved after rename. + g.Expect(result.Contexts["gh-prod"].Namespace).To(Equal("sap-cna")) +} + +func TestBootstrapCmd_PreservesExistingUnmanagedEntries(t *testing.T) { + g := NewWithT(t) + + // Start with a kubeconfig that already has an unrelated entry. + existing := clientcmdapi.NewConfig() + existing.Clusters["my-cluster"] = &clientcmdapi.Cluster{Server: "https://my.cluster.example.com"} + existing.AuthInfos["my-user"] = &clientcmdapi.AuthInfo{Token: "tok"} + existing.Contexts["my-ctx"] = &clientcmdapi.Context{Cluster: "my-cluster", AuthInfo: "my-user"} + existing.CurrentContext = "my-ctx" + dest := writeTempKubeconfig(t, existing) + + src := realGreenhouseKubeconfig("sap-cna") + data := encodeKubeconfig(t, src) + + _, _, err := runBootstrapCmd(t, []string{ + "--data=" + data, + "--context-name=greenhouse-sap-cna", + "--kubeconfig=" + dest, + }) + g.Expect(err).To(BeNil()) + + result := loadKubeconfig(t, dest) + // Existing entry must still be present. + g.Expect(result.Clusters).To(HaveKey("my-cluster")) + g.Expect(result.AuthInfos).To(HaveKey("my-user")) + g.Expect(result.Contexts).To(HaveKey("my-ctx")) + // Current context unchanged because --set-current-context was not passed. + g.Expect(result.CurrentContext).To(Equal("my-ctx")) + // New Greenhouse entry also present. + g.Expect(result.Clusters).To(HaveKey("greenhouse-sap-cna")) +} + +func TestBootstrapCmd_CreatesKubeconfigFileWhenAbsent(t *testing.T) { + g := NewWithT(t) + + src := realGreenhouseKubeconfig("sap-cna") + data := encodeKubeconfig(t, src) + dest := filepath.Join(t.TempDir(), "new-kubeconfig.yaml") // does not exist yet + + _, _, err := runBootstrapCmd(t, []string{ + "--data=" + data, + "--context-name=greenhouse-sap-cna", + "--kubeconfig=" + dest, + }) + g.Expect(err).To(BeNil()) + g.Expect(dest).To(BeAnExistingFile()) + + result := loadKubeconfig(t, dest) + g.Expect(result.Clusters).To(HaveKey("greenhouse-sap-cna")) +} + +func TestBootstrapCmd_JSONOutput(t *testing.T) { + g := NewWithT(t) + + src := realGreenhouseKubeconfig("sap-cna") + data := encodeKubeconfig(t, src) + dest := newEmptyKubeconfigFile(t) + + stdout, _, err := runBootstrapCmd(t, []string{ + "--data=" + data, + "--context-name=greenhouse-sap-cna", + "--kubeconfig=" + dest, + "-o", "json", + }) + g.Expect(err).To(BeNil()) + g.Expect(stdout).To(ContainSubstring(`"contextName": "greenhouse-sap-cna"`)) + g.Expect(stdout).To(ContainSubstring(`"added"`)) +} + +func TestBootstrapCmd_MissingBothDataAndServer(t *testing.T) { + g := NewWithT(t) + dest := newEmptyKubeconfigFile(t) + + _, _, err := runBootstrapCmd(t, []string{ + "--kubeconfig=" + dest, + "--greenhouse-org=my-org", + }) + g.Expect(err).To(MatchError(ContainSubstring("--greenhouse-server"))) +} + +func TestBootstrapCmd_RawBase64Encoding(t *testing.T) { + g := NewWithT(t) + + src := realGreenhouseKubeconfig("sap-cna") + raw, err := clientcmd.Write(*src) + g.Expect(err).To(BeNil()) + + // Some tools emit raw base64 (no padding). + data := base64.RawStdEncoding.EncodeToString(raw) + // Ensure there's no padding so we're testing the raw path. + g.Expect(strings.Contains(data, "=")).To(BeFalse()) + + dest := newEmptyKubeconfigFile(t) + _, _, err = runBootstrapCmd(t, []string{ + "--data=" + data, + "--context-name=greenhouse-sap-cna", + "--kubeconfig=" + dest, + }) + g.Expect(err).To(BeNil()) + result := loadKubeconfig(t, dest) + g.Expect(result.Clusters).To(HaveKey("greenhouse-sap-cna")) +} diff --git a/cmd/output/interactive_printer.go b/cmd/output/interactive_printer.go index 1f52d39..8150fd1 100644 --- a/cmd/output/interactive_printer.go +++ b/cmd/output/interactive_printer.go @@ -125,6 +125,38 @@ func (p *interactivePrinter) Print(v any) error { default: w("cloudctl update status: %s (current: %s, latest: %s)\n", t.Status, t.CurrentVersion, t.LatestVersion) } + case BootstrapResult: + if t.DryRun { + w("%s\n\n", styleFaint.Render("Dry-run: no changes will be written.")) + } + for _, entry := range t.Added { + w(" %s %s\n", styleGreen.Render("+"), entry) + } + for _, entry := range t.Skipped { + w(" %s %s\n", styleFaint.Render("="), entry) + } + if len(t.Added) == 0 && len(t.Skipped) > 0 { + w("%s\n", styleFaint.Render("Bootstrap: nothing new to write — all entries already exist.")) + break + } + if t.DryRun { + w("\n%s\n", styleFaint.Render("Bootstrap complete (dry-run). Run without --dry-run to apply.")) + break + } + w("\n%s\n", styleGreen.Render("Bootstrap complete.")) + if t.KubeconfigPath != "" { + w(" %s %s\n", styleFaint.Render("kubeconfig:"), t.KubeconfigPath) + } + w(" %s %s\n", styleFaint.Render("context: "), styleBold.Render(t.ContextName)) + if t.SetAsCurrent { + w(" %s\n", styleFaint.Render("set as current context.")) + } + org := t.Org + if org == "" { + org = strings.TrimPrefix(t.ContextName, "greenhouse-") + } + w("\nRun %s to pull in your cluster access.\n", + styleBold.Render(fmt.Sprintf("cloudctl sync -n %s", org))) default: w("%v\n", v) } diff --git a/cmd/output/plain_printer.go b/cmd/output/plain_printer.go index 5688dc7..10f71e5 100644 --- a/cmd/output/plain_printer.go +++ b/cmd/output/plain_printer.go @@ -100,6 +100,39 @@ func (p *plainPrinter) Print(v any) error { w("cloudctl update status: %s (current: %s, latest: %s)\n", t.Status, t.CurrentVersion, t.LatestVersion) } + case BootstrapResult: + if t.DryRun { + w("Dry-run: no changes will be written.\n\n") + } + for _, entry := range t.Added { + w(" [+] %s\n", entry) + } + for _, entry := range t.Skipped { + w(" [=] %s\n", entry) + } + if len(t.Added) == 0 && len(t.Skipped) > 0 { + w("Bootstrap: nothing new to write — all entries already exist.\n") + break + } + if t.DryRun { + w("\nBootstrap complete (dry-run). Run without --dry-run to apply.\n") + break + } + w("\nBootstrap complete.\n") + if t.KubeconfigPath != "" { + w(" kubeconfig: %s\n", t.KubeconfigPath) + } + w(" context: %s\n", t.ContextName) + if t.SetAsCurrent { + w(" set as current context.\n") + } + org := t.Org + if org == "" { + // Extract org from context name "greenhouse-". + org = strings.TrimPrefix(t.ContextName, "greenhouse-") + } + w("\nRun `cloudctl sync -n %s` to pull in your cluster access.\n", org) + default: w("%v\n", v) } diff --git a/cmd/output/types.go b/cmd/output/types.go index 7511648..b9d0209 100644 --- a/cmd/output/types.go +++ b/cmd/output/types.go @@ -99,3 +99,14 @@ type FieldChange struct { Old string `json:"old" yaml:"old"` New string `json:"new" yaml:"new"` } + +// BootstrapResult is the output of the bootstrap command. +type BootstrapResult struct { + ContextName string `json:"contextName" yaml:"contextName"` + SetAsCurrent bool `json:"setAsCurrent" yaml:"setAsCurrent"` + Added []string `json:"added,omitempty" yaml:"added,omitempty"` + Skipped []string `json:"skipped,omitempty" yaml:"skipped,omitempty"` + KubeconfigPath string `json:"kubeconfigPath,omitempty" yaml:"kubeconfigPath,omitempty"` + DryRun bool `json:"dryRun,omitzero" yaml:"dryRun,omitempty"` + Org string `json:"org,omitempty" yaml:"org,omitempty"` +} From d6f94e3d32fb0ca4add95159ff1dc1ccededcbcf Mon Sep 17 00:00:00 2001 From: onuryilmaz Date: Wed, 30 Sep 2026 14:17:51 +0200 Subject: [PATCH 02/10] =?UTF-8?q?fix(bootstrap):=20address=20review=20comm?= =?UTF-8?q?ents=20=E2=80=94=20validation,=20rename=20safety,=20org=20hint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - fix misleading doc comment in buildOIDCKubeconfig (client-secret is always emitted, not omitted when empty) - fix Added/Skipped JSON/YAML tags from omitempty to omitzero (Go 1.25 idiom) - add CurrentContextUpdated field to BootstrapResult; printers now show "nothing new" only when no entries were added AND current-context was not changed - capture org from the selected context's namespace before renameKubeconfigContext so the sync hint is correct when --context-name differs from greenhouse- - when --kubeconfig is not explicitly set, load through clientcmd.NewDefaultClientConfigLoadingRules to honour multi-file KUBECONFIG - make renameKubeconfigContext safe for shared cluster/authinfo references: only delete old keys if no other context still references them - validate blob context in resolveIncomingKubeconfig: error when current-context is missing or ambiguous, or when context references a non-existent cluster/user - add tests: SharedClusterPreserved rename, DataBlobNoCurrentContext, DataBlobMissingClusterRef Signed-off-by: onuryilmaz --- cmd/bootstrap.go | 68 ++++++++++++++++++++++++++++--- cmd/bootstrap_test.go | 62 +++++++++++++++++++++++++++- cmd/output/interactive_printer.go | 7 ++-- cmd/output/plain_printer.go | 3 +- cmd/output/types.go | 15 +++---- 5 files changed, 137 insertions(+), 18 deletions(-) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index efd741f..f057ede 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -153,6 +153,14 @@ func runBootstrap(cmd *cobra.Command, args []string) error { defaultCtxName = "greenhouse" } + // Capture org from the selected context's namespace before any rename, so + // the sync hint is correct even when --context-name differs from greenhouse-. + if org == "" { + if ctxEntry, ok := incoming.Contexts[defaultCtxName]; ok && ctxEntry.Namespace != "" { + org = ctxEntry.Namespace + } + } + contextName := bootstrapContextName if contextName == "" { contextName = defaultCtxName @@ -177,15 +185,22 @@ func runBootstrap(cmd *cobra.Command, args []string) error { // Rename entries in the incoming config to use the chosen context name. renameKubeconfigContext(incoming, contextName) - // Load or create the local kubeconfig. + // Load or create the local kubeconfig. When --kubeconfig was not explicitly + // set, load through the default rules to honour KUBECONFIG env var and merge + // multiple files; still write only to bootstrapKubeconfig (first path). var localConfig *clientcmdapi.Config - if bootstrapKubeconfig != "" { + if cmd.Flags().Changed("kubeconfig") { if _, statErr := os.Stat(bootstrapKubeconfig); statErr == nil { localConfig, err = clientcmd.LoadFromFile(bootstrapKubeconfig) if err != nil { return fmt.Errorf("failed to load local kubeconfig %q: %w", bootstrapKubeconfig, err) } } + } else { + localConfig, err = clientcmd.NewDefaultClientConfigLoadingRules().Load() + if err != nil { + return fmt.Errorf("failed to load kubeconfig: %w", err) + } } if localConfig == nil { localConfig = clientcmdapi.NewConfig() @@ -228,6 +243,23 @@ func resolveIncomingKubeconfig() (*clientcmdapi.Config, string, error) { if len(cfg.Clusters) == 0 { return nil, "", fmt.Errorf("--data kubeconfig contains no clusters") } + // Verify we can identify exactly which context to use. + if cfg.CurrentContext == "" && len(cfg.Contexts) != 1 { + return nil, "", fmt.Errorf("--data kubeconfig has no current-context and contains %d contexts; set one explicitly with kubectl config use-context", len(cfg.Contexts)) + } + activeCtx := cfg.CurrentContext + if activeCtx == "" { + for name := range cfg.Contexts { + activeCtx = name + } + } + if ctx, ok := cfg.Contexts[activeCtx]; !ok || ctx == nil { + return nil, "", fmt.Errorf("--data kubeconfig current-context %q not found in contexts", activeCtx) + } else if _, clOK := cfg.Clusters[ctx.Cluster]; !clOK { + return nil, "", fmt.Errorf("--data kubeconfig context %q references unknown cluster %q", activeCtx, ctx.Cluster) + } else if _, aiOK := cfg.AuthInfos[ctx.AuthInfo]; !aiOK { + return nil, "", fmt.Errorf("--data kubeconfig context %q references unknown user %q", activeCtx, ctx.AuthInfo) + } return cfg, bootstrapOrg, nil } @@ -262,7 +294,7 @@ func resolveIncomingKubeconfig() (*clientcmdapi.Config, string, error) { // config: // idp-issuer-url: ... // client-id: ... -// client-secret: ... (omitted when empty) +// client-secret: ... (always present; may be empty string) // extra-scopes: ... (omitted when empty) func buildOIDCKubeconfig(server, org, caDataB64, idpIssuerURL, clientID, clientSecret, extraScopes, namespace string) (*clientcmdapi.Config, error) { name := fmt.Sprintf("greenhouse-%s", org) @@ -313,6 +345,7 @@ func buildOIDCKubeconfig(server, org, caDataB64, idpIssuerURL, clientID, clientS // renameKubeconfigContext renames the active context (and its referenced cluster/authinfo) // in cfg to targetName. When there is exactly one context it is always the one renamed. // All other entries (multiple contexts in a blob) are left untouched. +// Old cluster/authinfo keys are only removed when no other context still references them. func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { // Identify which context to rename: prefer CurrentContext, fall back to the only one. source := cfg.CurrentContext @@ -337,18 +370,38 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { oldCluster := ctx.Cluster oldAuth := ctx.AuthInfo + // Only delete the old cluster key if no other context (other than source) references it. + clusterRefCount := 0 + for ctxName, c := range cfg.Contexts { + if ctxName != source && c.Cluster == oldCluster { + clusterRefCount++ + } + } + // Rename cluster. if cl, ok := cfg.Clusters[oldCluster]; ok && oldCluster != targetName { cfg.Clusters[targetName] = cl - delete(cfg.Clusters, oldCluster) ctx.Cluster = targetName + if clusterRefCount == 0 { + delete(cfg.Clusters, oldCluster) + } + } + + // Only delete the old authinfo key if no other context (other than source) references it. + authRefCount := 0 + for ctxName, c := range cfg.Contexts { + if ctxName != source && c.AuthInfo == oldAuth { + authRefCount++ + } } // Rename authinfo. if ai, ok := cfg.AuthInfos[oldAuth]; ok && oldAuth != targetName { cfg.AuthInfos[targetName] = ai - delete(cfg.AuthInfos, oldAuth) ctx.AuthInfo = targetName + if authRefCount == 0 { + delete(cfg.AuthInfos, oldAuth) + } } // Rename context. @@ -359,6 +412,8 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { // mergeBootstrapKubeconfig merges the incoming config into localConfig. // Existing entries are never overwritten. +// Cluster, authinfo, and context for a given name are treated as an atomic unit: +// if the cluster already exists with a different server, all three are skipped together. func mergeBootstrapKubeconfig(localConfig, incoming *clientcmdapi.Config, ctxName string, setCurrentCtx bool, org string) (output.BootstrapResult, error) { result := output.BootstrapResult{ ContextName: ctxName, @@ -393,8 +448,9 @@ func mergeBootstrapKubeconfig(localConfig, incoming *clientcmdapi.Config, ctxNam } } - if setCurrentCtx { + if setCurrentCtx && localConfig.CurrentContext != ctxName { localConfig.CurrentContext = ctxName + result.CurrentContextUpdated = true } return result, nil diff --git a/cmd/bootstrap_test.go b/cmd/bootstrap_test.go index a9a4e3f..dd3605c 100644 --- a/cmd/bootstrap_test.go +++ b/cmd/bootstrap_test.go @@ -279,6 +279,31 @@ func TestRenameKubeconfigContext_MultiContextBlobOnlyRenamesCurrent(t *testing.T g.Expect(cfg.CurrentContext).To(Equal("gh-prod")) } +func TestRenameKubeconfigContext_SharedClusterPreserved(t *testing.T) { + g := NewWithT(t) + + // Two contexts share the same cluster and authinfo — renaming one should + // not delete the shared keys. + cfg := clientcmdapi.NewConfig() + cfg.Clusters["shared-cluster"] = &clientcmdapi.Cluster{Server: "https://shared.example.com"} + cfg.AuthInfos["shared-user"] = &clientcmdapi.AuthInfo{} + cfg.Contexts["ctx-a"] = &clientcmdapi.Context{Cluster: "shared-cluster", AuthInfo: "shared-user"} + cfg.Contexts["ctx-b"] = &clientcmdapi.Context{Cluster: "shared-cluster", AuthInfo: "shared-user"} + cfg.CurrentContext = "ctx-a" + + renameKubeconfigContext(cfg, "gh-prod") + + // The renamed context uses new keys. + g.Expect(cfg.Contexts).To(HaveKey("gh-prod")) + g.Expect(cfg.Contexts).NotTo(HaveKey("ctx-a")) + // ctx-b still references shared-cluster and shared-user — they must not be deleted. + g.Expect(cfg.Clusters).To(HaveKey("shared-cluster")) + g.Expect(cfg.AuthInfos).To(HaveKey("shared-user")) + // The renamed context also gets gh-prod cluster/user copies. + g.Expect(cfg.Clusters).To(HaveKey("gh-prod")) + g.Expect(cfg.AuthInfos).To(HaveKey("gh-prod")) +} + // ── mergeBootstrapKubeconfig ────────────────────────────────────────────────── func TestMergeBootstrapKubeconfig_AddsAllEntries(t *testing.T) { @@ -304,9 +329,10 @@ func TestMergeBootstrapKubeconfig_SetsCurrentContext(t *testing.T) { local := clientcmdapi.NewConfig() incoming := realGreenhouseKubeconfig("sap-cna") - _, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", true, "sap-cna") + result, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", true, "sap-cna") g.Expect(err).To(BeNil()) g.Expect(local.CurrentContext).To(Equal("greenhouse-sap-cna")) + g.Expect(result.CurrentContextUpdated).To(BeTrue()) } func TestMergeBootstrapKubeconfig_IdempotentSkipsExisting(t *testing.T) { @@ -422,6 +448,40 @@ func TestResolveIncomingKubeconfig_DataBlobEmptyClusters(t *testing.T) { g.Expect(err).To(MatchError(ContainSubstring("no clusters"))) } +func TestResolveIncomingKubeconfig_DataBlobNoCurrentContext(t *testing.T) { + g := NewWithT(t) + + // Two contexts, no CurrentContext set — ambiguous, should error. + cfg := clientcmdapi.NewConfig() + cfg.Clusters["cluster-a"] = &clientcmdapi.Cluster{Server: "https://a.example.com"} + cfg.Clusters["cluster-b"] = &clientcmdapi.Cluster{Server: "https://b.example.com"} + cfg.AuthInfos["user-a"] = &clientcmdapi.AuthInfo{} + cfg.AuthInfos["user-b"] = &clientcmdapi.AuthInfo{} + cfg.Contexts["ctx-a"] = &clientcmdapi.Context{Cluster: "cluster-a", AuthInfo: "user-a"} + cfg.Contexts["ctx-b"] = &clientcmdapi.Context{Cluster: "cluster-b", AuthInfo: "user-b"} + bootstrapData = encodeKubeconfig(t, cfg) + t.Cleanup(func() { bootstrapData = "" }) + + _, _, err := resolveIncomingKubeconfig() + g.Expect(err).To(MatchError(ContainSubstring("no current-context"))) +} + +func TestResolveIncomingKubeconfig_DataBlobMissingClusterRef(t *testing.T) { + g := NewWithT(t) + + // Context references a cluster that doesn't exist in the blob. + cfg := clientcmdapi.NewConfig() + cfg.Clusters["cluster-a"] = &clientcmdapi.Cluster{Server: "https://a.example.com"} + cfg.AuthInfos["user-a"] = &clientcmdapi.AuthInfo{} + cfg.Contexts["ctx-a"] = &clientcmdapi.Context{Cluster: "missing-cluster", AuthInfo: "user-a"} + cfg.CurrentContext = "ctx-a" + bootstrapData = encodeKubeconfig(t, cfg) + t.Cleanup(func() { bootstrapData = "" }) + + _, _, err := resolveIncomingKubeconfig() + g.Expect(err).To(MatchError(ContainSubstring("missing-cluster"))) +} + func TestResolveIncomingKubeconfig_IndividualFlags(t *testing.T) { g := NewWithT(t) diff --git a/cmd/output/interactive_printer.go b/cmd/output/interactive_printer.go index 8150fd1..bdeff6e 100644 --- a/cmd/output/interactive_printer.go +++ b/cmd/output/interactive_printer.go @@ -130,12 +130,13 @@ func (p *interactivePrinter) Print(v any) error { w("%s\n\n", styleFaint.Render("Dry-run: no changes will be written.")) } for _, entry := range t.Added { - w(" %s %s\n", styleGreen.Render("+"), entry) + w(" %s %s\n", styleGreen.Render("[+]"), entry) } for _, entry := range t.Skipped { - w(" %s %s\n", styleFaint.Render("="), entry) + w(" %s %s\n", styleFaint.Render("[=]"), entry) } - if len(t.Added) == 0 && len(t.Skipped) > 0 { + nothingNew := len(t.Added) == 0 && !t.CurrentContextUpdated + if nothingNew && len(t.Skipped) > 0 { w("%s\n", styleFaint.Render("Bootstrap: nothing new to write — all entries already exist.")) break } diff --git a/cmd/output/plain_printer.go b/cmd/output/plain_printer.go index 10f71e5..ea91e48 100644 --- a/cmd/output/plain_printer.go +++ b/cmd/output/plain_printer.go @@ -110,7 +110,8 @@ func (p *plainPrinter) Print(v any) error { for _, entry := range t.Skipped { w(" [=] %s\n", entry) } - if len(t.Added) == 0 && len(t.Skipped) > 0 { + nothingNew := len(t.Added) == 0 && !t.CurrentContextUpdated + if nothingNew && len(t.Skipped) > 0 { w("Bootstrap: nothing new to write — all entries already exist.\n") break } diff --git a/cmd/output/types.go b/cmd/output/types.go index b9d0209..5738d38 100644 --- a/cmd/output/types.go +++ b/cmd/output/types.go @@ -102,11 +102,12 @@ type FieldChange struct { // BootstrapResult is the output of the bootstrap command. type BootstrapResult struct { - ContextName string `json:"contextName" yaml:"contextName"` - SetAsCurrent bool `json:"setAsCurrent" yaml:"setAsCurrent"` - Added []string `json:"added,omitempty" yaml:"added,omitempty"` - Skipped []string `json:"skipped,omitempty" yaml:"skipped,omitempty"` - KubeconfigPath string `json:"kubeconfigPath,omitempty" yaml:"kubeconfigPath,omitempty"` - DryRun bool `json:"dryRun,omitzero" yaml:"dryRun,omitempty"` - Org string `json:"org,omitempty" yaml:"org,omitempty"` + ContextName string `json:"contextName" yaml:"contextName"` + SetAsCurrent bool `json:"setAsCurrent" yaml:"setAsCurrent"` + CurrentContextUpdated bool `json:"currentContextUpdated,omitzero" yaml:"currentContextUpdated,omitempty"` + Added []string `json:"added,omitzero" yaml:"added,omitempty"` + Skipped []string `json:"skipped,omitzero" yaml:"skipped,omitempty"` + KubeconfigPath string `json:"kubeconfigPath,omitempty" yaml:"kubeconfigPath,omitempty"` + DryRun bool `json:"dryRun,omitzero" yaml:"dryRun,omitempty"` + Org string `json:"org,omitempty" yaml:"org,omitempty"` } From 7ab1a0fb2350a91a5a69ca72c9436cd1e16d5744 Mon Sep 17 00:00:00 2001 From: onuryilmaz Date: Wed, 30 Sep 2026 14:47:05 +0200 Subject: [PATCH 03/10] fix(bootstrap): error on empty first segment in KUBECONFIG When KUBECONFIG starts with a path separator (e.g. :/second/config), the first segment is empty and the write target is ambiguous. Return a clear error matching the behaviour of resolveWriteTarget in sync.go. Also reset cobra flag Changed state between test runs so that flag.Changed() is accurate regardless of test ordering. Signed-off-by: onuryilmaz --- cmd/bootstrap.go | 2 ++ cmd/bootstrap_test.go | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index f057ede..3ae3b23 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -117,6 +117,8 @@ func runBootstrap(cmd *cobra.Command, args []string) error { } else if kc := os.Getenv("KUBECONFIG"); kc != "" { if parts := strings.SplitN(kc, string(os.PathListSeparator), 2); len(parts) > 0 && parts[0] != "" { bootstrapKubeconfig = parts[0] + } else { + return fmt.Errorf("cannot determine write target: KUBECONFIG=%q contains no usable first path", kc) } } else { bootstrapKubeconfig = clientcmd.RecommendedHomeFile diff --git a/cmd/bootstrap_test.go b/cmd/bootstrap_test.go index dd3605c..a1ca63f 100644 --- a/cmd/bootstrap_test.go +++ b/cmd/bootstrap_test.go @@ -12,6 +12,7 @@ import ( "testing" . "github.com/onsi/gomega" + "github.com/spf13/pflag" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -105,6 +106,10 @@ func runBootstrapCmd(t *testing.T, args []string) (stdout, stderr string, err er bootstrapSetCurrentCtx = false bootstrapDryRun = false + // Reset cobra's "Changed" state on bootstrapCmd flags so flag.Changed() is + // accurate for each test regardless of what previous tests passed. + bootstrapCmd.Flags().VisitAll(func(f *pflag.Flag) { f.Changed = false }) + outBuf := &bytes.Buffer{} errBuf := &bytes.Buffer{} @@ -881,3 +886,19 @@ func TestBootstrapCmd_RawBase64Encoding(t *testing.T) { result := loadKubeconfig(t, dest) g.Expect(result.Clusters).To(HaveKey("greenhouse-sap-cna")) } + +func TestBootstrapCmd_KUBECONFIGEmptyFirstSegment(t *testing.T) { + g := NewWithT(t) + cfg := realGreenhouseKubeconfig("sap") + data := encodeKubeconfig(t, cfg) + + second := writeTempKubeconfig(t, clientcmdapi.NewConfig()) + t.Setenv("KUBECONFIG", string(os.PathListSeparator)+second) + + _, _, err := runBootstrapCmd(t, []string{ + "--data=" + data, + "--context-name=greenhouse-sap", + }) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("no usable first path")) +} From 518e4d787076eb00491abccf6b4a2ef22a8855ec Mon Sep 17 00:00:00 2001 From: onuryilmaz Date: Wed, 30 Sep 2026 14:52:45 +0200 Subject: [PATCH 04/10] fix(bootstrap): rename cluster/authinfo even when context key already matches When the incoming context key already equals targetName, the early return skipped renaming the referenced cluster and authinfo entries. This caused the three map entries to have inconsistent names after merge. Separate the "context key is a no-op" case from "entries need renaming" so cluster and authinfo are always aligned to targetName regardless of whether the context key itself needed changing. Signed-off-by: onuryilmaz --- cmd/bootstrap.go | 16 +++++++++------- cmd/bootstrap_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index 3ae3b23..5fca563 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -358,14 +358,14 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { } } } - if source == "" || source == targetName { - // Nothing to rename, but ensure CurrentContext points to the target. + if source == "" { cfg.CurrentContext = targetName return } ctx := cfg.Contexts[source] if ctx == nil { + cfg.CurrentContext = targetName return } @@ -380,7 +380,7 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { } } - // Rename cluster. + // Rename cluster (when it differs from the target name). if cl, ok := cfg.Clusters[oldCluster]; ok && oldCluster != targetName { cfg.Clusters[targetName] = cl ctx.Cluster = targetName @@ -397,7 +397,7 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { } } - // Rename authinfo. + // Rename authinfo (when it differs from the target name). if ai, ok := cfg.AuthInfos[oldAuth]; ok && oldAuth != targetName { cfg.AuthInfos[targetName] = ai ctx.AuthInfo = targetName @@ -406,9 +406,11 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { } } - // Rename context. - cfg.Contexts[targetName] = ctx - delete(cfg.Contexts, source) + // Rename context key when it differs; otherwise just update CurrentContext. + if source != targetName { + cfg.Contexts[targetName] = ctx + delete(cfg.Contexts, source) + } cfg.CurrentContext = targetName } diff --git a/cmd/bootstrap_test.go b/cmd/bootstrap_test.go index a1ca63f..f181e20 100644 --- a/cmd/bootstrap_test.go +++ b/cmd/bootstrap_test.go @@ -309,6 +309,32 @@ func TestRenameKubeconfigContext_SharedClusterPreserved(t *testing.T) { g.Expect(cfg.AuthInfos).To(HaveKey("gh-prod")) } +func TestRenameKubeconfigContext_ContextKeyMatchesButEntriesDiffer(t *testing.T) { + g := NewWithT(t) + + // The context key already equals targetName, but cluster/authinfo keys differ. + // renameKubeconfigContext must still rename the cluster and authinfo entries. + cfg := clientcmdapi.NewConfig() + cfg.Clusters["greenhouse-org-cluster"] = &clientcmdapi.Cluster{Server: "https://greenhouse.example.com"} + cfg.AuthInfos["greenhouse-org-user"] = &clientcmdapi.AuthInfo{} + cfg.Contexts["greenhouse-org"] = &clientcmdapi.Context{Cluster: "greenhouse-org-cluster", AuthInfo: "greenhouse-org-user"} + cfg.CurrentContext = "greenhouse-org" + + renameKubeconfigContext(cfg, "greenhouse-org") + + // Context key unchanged. + g.Expect(cfg.Contexts).To(HaveKey("greenhouse-org")) + g.Expect(cfg.CurrentContext).To(Equal("greenhouse-org")) + // Cluster and authinfo must be renamed to match the context name. + g.Expect(cfg.Clusters).To(HaveKey("greenhouse-org")) + g.Expect(cfg.Clusters).NotTo(HaveKey("greenhouse-org-cluster")) + g.Expect(cfg.AuthInfos).To(HaveKey("greenhouse-org")) + g.Expect(cfg.AuthInfos).NotTo(HaveKey("greenhouse-org-user")) + // The context's Cluster and AuthInfo fields must point to the new keys. + g.Expect(cfg.Contexts["greenhouse-org"].Cluster).To(Equal("greenhouse-org")) + g.Expect(cfg.Contexts["greenhouse-org"].AuthInfo).To(Equal("greenhouse-org")) +} + // ── mergeBootstrapKubeconfig ────────────────────────────────────────────────── func TestMergeBootstrapKubeconfig_AddsAllEntries(t *testing.T) { From 864ef4cc3220d54bc3c37ea00e9a5512a47030bc Mon Sep 17 00:00:00 2001 From: onuryilmaz Date: Tue, 6 Oct 2026 16:21:23 +0200 Subject: [PATCH 05/10] fix(bootstrap): separate collision view from write target; always emit sync hint Three fixes: 1. When KUBECONFIG spans multiple files, load a merged view for collision detection but mutate and serialize only the first file. Previously the merged object was written back to the first file, silently copying unmanaged entries from other files into it. 2. Interactive and plain printers now always emit the "cloudctl sync -n " next-step hint after a successful bootstrap, including the idempotent case. Previously the hint was skipped when nothing was new. 3. "Bootstrap complete." and kubeconfig path are suppressed for the idempotent case (nothing to report), but the hint still prints. Signed-off-by: onuryilmaz --- cmd/bootstrap.go | 48 +++++++++++++++++-------------- cmd/bootstrap_test.go | 10 +++---- cmd/output/interactive_printer.go | 17 +++++------ cmd/output/plain_printer.go | 17 +++++------ 4 files changed, 49 insertions(+), 43 deletions(-) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index 5fca563..294eeec 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -187,28 +187,31 @@ func runBootstrap(cmd *cobra.Command, args []string) error { // Rename entries in the incoming config to use the chosen context name. renameKubeconfigContext(incoming, contextName) - // Load or create the local kubeconfig. When --kubeconfig was not explicitly - // set, load through the default rules to honour KUBECONFIG env var and merge - // multiple files; still write only to bootstrapKubeconfig (first path). + // Load the config that will be mutated and written back (first file only). + // When KUBECONFIG contains multiple files we also build a merged view used + // solely for collision detection — we never write the merged object back so + // unmanaged entries in other files are not copied into the first file. var localConfig *clientcmdapi.Config - if cmd.Flags().Changed("kubeconfig") { - if _, statErr := os.Stat(bootstrapKubeconfig); statErr == nil { - localConfig, err = clientcmd.LoadFromFile(bootstrapKubeconfig) - if err != nil { - return fmt.Errorf("failed to load local kubeconfig %q: %w", bootstrapKubeconfig, err) - } - } - } else { - localConfig, err = clientcmd.NewDefaultClientConfigLoadingRules().Load() + if _, statErr := os.Stat(bootstrapKubeconfig); statErr == nil { + localConfig, err = clientcmd.LoadFromFile(bootstrapKubeconfig) if err != nil { - return fmt.Errorf("failed to load kubeconfig: %w", err) + return fmt.Errorf("failed to load local kubeconfig %q: %w", bootstrapKubeconfig, err) } } if localConfig == nil { localConfig = clientcmdapi.NewConfig() } - result, err := mergeBootstrapKubeconfig(localConfig, incoming, contextName, setCurrentCtx, org) + // When --kubeconfig was not explicitly set, also load the merged view of all + // KUBECONFIG files so we can detect collisions with entries in other files. + mergedView := localConfig + if !cmd.Flags().Changed("kubeconfig") { + if mv, mvErr := clientcmd.NewDefaultClientConfigLoadingRules().Load(); mvErr == nil { + mergedView = mv + } + } + + result, err := mergeBootstrapKubeconfig(localConfig, mergedView, incoming, contextName, setCurrentCtx, org) if err != nil { return err } @@ -414,11 +417,12 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { cfg.CurrentContext = targetName } -// mergeBootstrapKubeconfig merges the incoming config into localConfig. -// Existing entries are never overwritten. -// Cluster, authinfo, and context for a given name are treated as an atomic unit: -// if the cluster already exists with a different server, all three are skipped together. -func mergeBootstrapKubeconfig(localConfig, incoming *clientcmdapi.Config, ctxName string, setCurrentCtx bool, org string) (output.BootstrapResult, error) { +// mergeBootstrapKubeconfig merges incoming into localConfig (the file that will +// be written). collisionView is used for existence checks — when KUBECONFIG +// spans multiple files it is the merged view of all of them, so we detect +// collisions with entries in other files without copying those entries into the +// first file. +func mergeBootstrapKubeconfig(localConfig, collisionView, incoming *clientcmdapi.Config, ctxName string, setCurrentCtx bool, org string) (output.BootstrapResult, error) { result := output.BootstrapResult{ ContextName: ctxName, SetAsCurrent: setCurrentCtx, @@ -426,7 +430,7 @@ func mergeBootstrapKubeconfig(localConfig, incoming *clientcmdapi.Config, ctxNam } for name, cluster := range incoming.Clusters { - if _, exists := localConfig.Clusters[name]; !exists { + if _, exists := collisionView.Clusters[name]; !exists { localConfig.Clusters[name] = cluster result.Added = append(result.Added, fmt.Sprintf("cluster %q", name)) } else { @@ -435,7 +439,7 @@ func mergeBootstrapKubeconfig(localConfig, incoming *clientcmdapi.Config, ctxNam } for name, auth := range incoming.AuthInfos { - if _, exists := localConfig.AuthInfos[name]; !exists { + if _, exists := collisionView.AuthInfos[name]; !exists { localConfig.AuthInfos[name] = auth result.Added = append(result.Added, fmt.Sprintf("user %q", name)) } else { @@ -444,7 +448,7 @@ func mergeBootstrapKubeconfig(localConfig, incoming *clientcmdapi.Config, ctxNam } for name, ctx := range incoming.Contexts { - if _, exists := localConfig.Contexts[name]; !exists { + if _, exists := collisionView.Contexts[name]; !exists { localConfig.Contexts[name] = ctx result.Added = append(result.Added, fmt.Sprintf("context %q", name)) } else { diff --git a/cmd/bootstrap_test.go b/cmd/bootstrap_test.go index f181e20..1125249 100644 --- a/cmd/bootstrap_test.go +++ b/cmd/bootstrap_test.go @@ -343,7 +343,7 @@ func TestMergeBootstrapKubeconfig_AddsAllEntries(t *testing.T) { local := clientcmdapi.NewConfig() incoming := realGreenhouseKubeconfig("sap-cna") - result, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", false, "sap-cna") + result, err := mergeBootstrapKubeconfig(local, local, incoming, "greenhouse-sap-cna", false, "sap-cna") g.Expect(err).To(BeNil()) g.Expect(result.Added).To(HaveLen(3)) @@ -360,7 +360,7 @@ func TestMergeBootstrapKubeconfig_SetsCurrentContext(t *testing.T) { local := clientcmdapi.NewConfig() incoming := realGreenhouseKubeconfig("sap-cna") - result, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", true, "sap-cna") + result, err := mergeBootstrapKubeconfig(local, local, incoming, "greenhouse-sap-cna", true, "sap-cna") g.Expect(err).To(BeNil()) g.Expect(local.CurrentContext).To(Equal("greenhouse-sap-cna")) g.Expect(result.CurrentContextUpdated).To(BeTrue()) @@ -372,7 +372,7 @@ func TestMergeBootstrapKubeconfig_IdempotentSkipsExisting(t *testing.T) { incoming := realGreenhouseKubeconfig("sap-cna") local := realGreenhouseKubeconfig("sap-cna") // same entries already present - result, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", false, "sap-cna") + result, err := mergeBootstrapKubeconfig(local, local, incoming, "greenhouse-sap-cna", false, "sap-cna") g.Expect(err).To(BeNil()) g.Expect(result.Added).To(BeEmpty()) g.Expect(result.Skipped).To(HaveLen(3)) @@ -386,7 +386,7 @@ func TestMergeBootstrapKubeconfig_NeverOverwritesExistingEntries(t *testing.T) { local.Clusters["greenhouse-sap-cna"] = &clientcmdapi.Cluster{Server: "https://original.example.com"} incoming := realGreenhouseKubeconfig("sap-cna") - _, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", false, "sap-cna") + _, err := mergeBootstrapKubeconfig(local, local, incoming, "greenhouse-sap-cna", false, "sap-cna") g.Expect(err).To(BeNil()) // Original server must not be overwritten. @@ -404,7 +404,7 @@ func TestMergeBootstrapKubeconfig_PreservesExistingLocalEntries(t *testing.T) { local.CurrentContext = "other-ctx" incoming := realGreenhouseKubeconfig("sap-cna") - _, err := mergeBootstrapKubeconfig(local, incoming, "greenhouse-sap-cna", false, "sap-cna") + _, err := mergeBootstrapKubeconfig(local, local, incoming, "greenhouse-sap-cna", false, "sap-cna") g.Expect(err).To(BeNil()) // Unrelated entries must still be there. diff --git a/cmd/output/interactive_printer.go b/cmd/output/interactive_printer.go index bdeff6e..0001b4d 100644 --- a/cmd/output/interactive_printer.go +++ b/cmd/output/interactive_printer.go @@ -138,19 +138,20 @@ func (p *interactivePrinter) Print(v any) error { nothingNew := len(t.Added) == 0 && !t.CurrentContextUpdated if nothingNew && len(t.Skipped) > 0 { w("%s\n", styleFaint.Render("Bootstrap: nothing new to write — all entries already exist.")) - break } if t.DryRun { w("\n%s\n", styleFaint.Render("Bootstrap complete (dry-run). Run without --dry-run to apply.")) break } - w("\n%s\n", styleGreen.Render("Bootstrap complete.")) - if t.KubeconfigPath != "" { - w(" %s %s\n", styleFaint.Render("kubeconfig:"), t.KubeconfigPath) - } - w(" %s %s\n", styleFaint.Render("context: "), styleBold.Render(t.ContextName)) - if t.SetAsCurrent { - w(" %s\n", styleFaint.Render("set as current context.")) + if !nothingNew { + w("\n%s\n", styleGreen.Render("Bootstrap complete.")) + if t.KubeconfigPath != "" { + w(" %s %s\n", styleFaint.Render("kubeconfig:"), t.KubeconfigPath) + } + w(" %s %s\n", styleFaint.Render("context: "), styleBold.Render(t.ContextName)) + if t.SetAsCurrent { + w(" %s\n", styleFaint.Render("set as current context.")) + } } org := t.Org if org == "" { diff --git a/cmd/output/plain_printer.go b/cmd/output/plain_printer.go index ea91e48..2b43daf 100644 --- a/cmd/output/plain_printer.go +++ b/cmd/output/plain_printer.go @@ -113,19 +113,20 @@ func (p *plainPrinter) Print(v any) error { nothingNew := len(t.Added) == 0 && !t.CurrentContextUpdated if nothingNew && len(t.Skipped) > 0 { w("Bootstrap: nothing new to write — all entries already exist.\n") - break } if t.DryRun { w("\nBootstrap complete (dry-run). Run without --dry-run to apply.\n") break } - w("\nBootstrap complete.\n") - if t.KubeconfigPath != "" { - w(" kubeconfig: %s\n", t.KubeconfigPath) - } - w(" context: %s\n", t.ContextName) - if t.SetAsCurrent { - w(" set as current context.\n") + if !nothingNew { + w("\nBootstrap complete.\n") + if t.KubeconfigPath != "" { + w(" kubeconfig: %s\n", t.KubeconfigPath) + } + w(" context: %s\n", t.ContextName) + if t.SetAsCurrent { + w(" set as current context.\n") + } } org := t.Org if org == "" { From 15ed71b2cdfd2005f35b6ffb73cc840fbcb5330b Mon Sep 17 00:00:00 2001 From: onuryilmaz Date: Tue, 6 Oct 2026 16:32:46 +0200 Subject: [PATCH 06/10] fix(bootstrap): propagate error when merged KUBECONFIG view fails to load A malformed or unreadable file listed after the first path in KUBECONFIG would cause Load() to error, but the previous code silently fell back to the first-file view. Collision detection then missed entries from the remaining files and bootstrap could shadow unmanaged configuration. Return the error instead of continuing with an incomplete view. Signed-off-by: onuryilmaz --- cmd/bootstrap.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index 294eeec..fa6dc85 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -206,9 +206,11 @@ func runBootstrap(cmd *cobra.Command, args []string) error { // KUBECONFIG files so we can detect collisions with entries in other files. mergedView := localConfig if !cmd.Flags().Changed("kubeconfig") { - if mv, mvErr := clientcmd.NewDefaultClientConfigLoadingRules().Load(); mvErr == nil { - mergedView = mv + mv, mvErr := clientcmd.NewDefaultClientConfigLoadingRules().Load() + if mvErr != nil { + return fmt.Errorf("failed to load kubeconfig: %w", mvErr) } + mergedView = mv } result, err := mergeBootstrapKubeconfig(localConfig, mergedView, incoming, contextName, setCurrentCtx, org) From 21047d0771fca13d0615e2dea149414da8ce8933 Mon Sep 17 00:00:00 2001 From: onuryilmaz Date: Tue, 6 Oct 2026 17:00:04 +0200 Subject: [PATCH 07/10] fix(bootstrap): guard nil context entries in ref-count loops A multi-context kubeconfig blob where one context value is null (e.g. decoded from a YAML null entry) caused a nil pointer dereference in the cluster and authinfo ref-count loops inside renameKubeconfigContext. Add a c != nil guard in both loops so the rename proceeds safely. Signed-off-by: onuryilmaz --- cmd/bootstrap.go | 4 ++-- cmd/bootstrap_test.go | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index fa6dc85..3b7b61b 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -380,7 +380,7 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { // Only delete the old cluster key if no other context (other than source) references it. clusterRefCount := 0 for ctxName, c := range cfg.Contexts { - if ctxName != source && c.Cluster == oldCluster { + if ctxName != source && c != nil && c.Cluster == oldCluster { clusterRefCount++ } } @@ -397,7 +397,7 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { // Only delete the old authinfo key if no other context (other than source) references it. authRefCount := 0 for ctxName, c := range cfg.Contexts { - if ctxName != source && c.AuthInfo == oldAuth { + if ctxName != source && c != nil && c.AuthInfo == oldAuth { authRefCount++ } } diff --git a/cmd/bootstrap_test.go b/cmd/bootstrap_test.go index 1125249..1c3e21a 100644 --- a/cmd/bootstrap_test.go +++ b/cmd/bootstrap_test.go @@ -335,6 +335,22 @@ func TestRenameKubeconfigContext_ContextKeyMatchesButEntriesDiffer(t *testing.T) g.Expect(cfg.Contexts["greenhouse-org"].AuthInfo).To(Equal("greenhouse-org")) } +func TestRenameKubeconfigContext_NilContextEntryDoesNotPanic(t *testing.T) { + // A multi-context blob where one context value is nil (e.g. decoded from a + // YAML null) must not panic when computing cluster/authinfo ref-counts. + cfg := clientcmdapi.NewConfig() + cfg.Clusters["greenhouse-org"] = &clientcmdapi.Cluster{Server: "https://greenhouse.example.com"} + cfg.AuthInfos["greenhouse-org"] = &clientcmdapi.AuthInfo{} + cfg.Contexts["greenhouse-org"] = &clientcmdapi.Context{Cluster: "greenhouse-org", AuthInfo: "greenhouse-org"} + cfg.Contexts["other-ctx"] = nil // nil entry simulating a null in the YAML + cfg.CurrentContext = "greenhouse-org" + + g := NewWithT(t) + g.Expect(func() { renameKubeconfigContext(cfg, "gh-prod") }).NotTo(Panic()) + g.Expect(cfg.Contexts).To(HaveKey("gh-prod")) + g.Expect(cfg.CurrentContext).To(Equal("gh-prod")) +} + // ── mergeBootstrapKubeconfig ────────────────────────────────────────────────── func TestMergeBootstrapKubeconfig_AddsAllEntries(t *testing.T) { From f101ef890e20643a916f107a4b2f7e11a0d3f8fe Mon Sep 17 00:00:00 2001 From: onuryilmaz Date: Tue, 6 Oct 2026 17:57:01 +0200 Subject: [PATCH 08/10] fix(bootstrap): honour CLOUDCTL_KUBECONFIG and config-file kubeconfig value cmd.Flags().Changed("kubeconfig") only saw the --kubeconfig flag; values provided via the CLOUDCTL_KUBECONFIG environment variable or a cloudctl config file were silently ignored, causing bootstrap to fall back to KUBECONFIG / home default and potentially write to the wrong file. The kubeconfig write target and the merged-view skip gate now check both cmd.Flags().Changed("kubeconfig") (flag path) and viper.IsSet("kubeconfig") (env-var / config-file path). Similarly, the interactive prompts for --context-name and --set-current-context now respect values from any configuration source. Signed-off-by: onuryilmaz --- cmd/bootstrap.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index 3b7b61b..3552ff2 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -111,9 +111,16 @@ func runBootstrap(cmd *cobra.Command, args []string) error { bootstrapClientSecret = viper.GetString("greenhouse-client-secret") bootstrapExtraScopes = viper.GetString("greenhouse-extra-scopes") bootstrapNamespace = viper.GetString("greenhouse-namespace") - // Use the flag value directly; only fall back to KUBECONFIG/default when not explicitly set. + // Resolve the write target: prefer an explicitly-provided value from any + // configuration source (flag, CLOUDCTL_KUBECONFIG env var, or config file) + // before falling back to KUBECONFIG / home default. + // viper.IsSet covers env-var and config-file sources; cmd.Flags().Changed + // covers the --kubeconfig flag (Viper's pflag binding only works when + // BindPFlags runs after flag parsing, which is not guaranteed here). if cmd.Flags().Changed("kubeconfig") { bootstrapKubeconfig, _ = cmd.Flags().GetString("kubeconfig") + } else if viper.IsSet("kubeconfig") { + bootstrapKubeconfig = viper.GetString("kubeconfig") } else if kc := os.Getenv("KUBECONFIG"); kc != "" { if parts := strings.SplitN(kc, string(os.PathListSeparator), 2); len(parts) > 0 && parts[0] != "" { bootstrapKubeconfig = parts[0] @@ -169,7 +176,7 @@ func runBootstrap(cmd *cobra.Command, args []string) error { } // Prompt interactively for context name when running on a TTY and the flag wasn't set. - if !cmd.Flags().Changed("context-name") && output.IsTTYWriter(w) { + if !cmd.Flags().Changed("context-name") && !viper.IsSet("context-name") && output.IsTTYWriter(w) { contextName, err = promptContextName(contextName) if err != nil { return err @@ -177,7 +184,7 @@ func runBootstrap(cmd *cobra.Command, args []string) error { } setCurrentCtx := bootstrapSetCurrentCtx - if !cmd.Flags().Changed("set-current-context") && output.IsTTYWriter(w) { + if !cmd.Flags().Changed("set-current-context") && !viper.IsSet("set-current-context") && output.IsTTYWriter(w) { setCurrentCtx, err = promptYesNo(fmt.Sprintf("Set %q as the current context?", contextName), false) if err != nil { return err @@ -202,10 +209,10 @@ func runBootstrap(cmd *cobra.Command, args []string) error { localConfig = clientcmdapi.NewConfig() } - // When --kubeconfig was not explicitly set, also load the merged view of all - // KUBECONFIG files so we can detect collisions with entries in other files. + // When kubeconfig was not explicitly set (via flag, env var, or config file), also load + // the merged view of all KUBECONFIG files so we can detect collisions with entries in other files. mergedView := localConfig - if !cmd.Flags().Changed("kubeconfig") { + if !cmd.Flags().Changed("kubeconfig") && !viper.IsSet("kubeconfig") { mv, mvErr := clientcmd.NewDefaultClientConfigLoadingRules().Load() if mvErr != nil { return fmt.Errorf("failed to load kubeconfig: %w", mvErr) From eab8c3dec6d3545d8f5a162669f236f5e6d4636a Mon Sep 17 00:00:00 2001 From: onuryilmaz Date: Tue, 6 Oct 2026 19:18:27 +0200 Subject: [PATCH 09/10] fix(bootstrap): reject targetName collision with unrelated blob entries renameKubeconfigContext silently overwrote an existing cluster, authinfo, or context entry when the chosen --context-name matched a key belonging to a different context in the blob. The function now returns an error before mutating any of the three maps when targetName already names an unrelated entry, preventing silent data loss in multi-context blobs. Signed-off-by: onuryilmaz --- cmd/bootstrap.go | 26 ++++++++++++++++++++++---- cmd/bootstrap_test.go | 34 ++++++++++++++++++++++++++++------ 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index 3552ff2..643c172 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -192,7 +192,9 @@ func runBootstrap(cmd *cobra.Command, args []string) error { } // Rename entries in the incoming config to use the chosen context name. - renameKubeconfigContext(incoming, contextName) + if err := renameKubeconfigContext(incoming, contextName); err != nil { + return err + } // Load the config that will be mutated and written back (first file only). // When KUBECONFIG contains multiple files we also build a merged view used @@ -360,7 +362,9 @@ func buildOIDCKubeconfig(server, org, caDataB64, idpIssuerURL, clientID, clientS // in cfg to targetName. When there is exactly one context it is always the one renamed. // All other entries (multiple contexts in a blob) are left untouched. // Old cluster/authinfo keys are only removed when no other context still references them. -func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { +// Returns an error when targetName already names a different, unrelated entry in any of +// the three maps, which would silently overwrite it. +func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) error { // Identify which context to rename: prefer CurrentContext, fall back to the only one. source := cfg.CurrentContext if _, ok := cfg.Contexts[source]; !ok { @@ -372,18 +376,31 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { } if source == "" { cfg.CurrentContext = targetName - return + return nil } ctx := cfg.Contexts[source] if ctx == nil { cfg.CurrentContext = targetName - return + return nil } oldCluster := ctx.Cluster oldAuth := ctx.AuthInfo + // Reject the rename when targetName is already used by an unrelated entry. + // "Unrelated" means: the key exists AND it is not the same entry we are about + // to rename (i.e. it is not oldCluster / oldAuth / source). + if _, exists := cfg.Clusters[targetName]; exists && targetName != oldCluster { + return fmt.Errorf("cannot rename to %q: a different cluster entry with that name already exists in the kubeconfig blob", targetName) + } + if _, exists := cfg.AuthInfos[targetName]; exists && targetName != oldAuth { + return fmt.Errorf("cannot rename to %q: a different user entry with that name already exists in the kubeconfig blob", targetName) + } + if _, exists := cfg.Contexts[targetName]; exists && targetName != source { + return fmt.Errorf("cannot rename to %q: a different context entry with that name already exists in the kubeconfig blob", targetName) + } + // Only delete the old cluster key if no other context (other than source) references it. clusterRefCount := 0 for ctxName, c := range cfg.Contexts { @@ -424,6 +441,7 @@ func renameKubeconfigContext(cfg *clientcmdapi.Config, targetName string) { delete(cfg.Contexts, source) } cfg.CurrentContext = targetName + return nil } // mergeBootstrapKubeconfig merges incoming into localConfig (the file that will diff --git a/cmd/bootstrap_test.go b/cmd/bootstrap_test.go index 1c3e21a..07ff299 100644 --- a/cmd/bootstrap_test.go +++ b/cmd/bootstrap_test.go @@ -230,7 +230,7 @@ func TestRenameKubeconfigContext_RenamesAll(t *testing.T) { g := NewWithT(t) cfg := realGreenhouseKubeconfig("sap-cna") - renameKubeconfigContext(cfg, "my-custom-name") + g.Expect(renameKubeconfigContext(cfg, "my-custom-name")).To(Succeed()) g.Expect(cfg.CurrentContext).To(Equal("my-custom-name")) g.Expect(cfg.Contexts).To(HaveKey("my-custom-name")) @@ -250,7 +250,7 @@ func TestRenameKubeconfigContext_NoOpWhenNameUnchanged(t *testing.T) { g := NewWithT(t) cfg := realGreenhouseKubeconfig("sap-cna") - renameKubeconfigContext(cfg, "greenhouse-sap-cna") + g.Expect(renameKubeconfigContext(cfg, "greenhouse-sap-cna")).To(Succeed()) g.Expect(cfg.CurrentContext).To(Equal("greenhouse-sap-cna")) g.Expect(cfg.Contexts).To(HaveKey("greenhouse-sap-cna")) @@ -271,7 +271,7 @@ func TestRenameKubeconfigContext_MultiContextBlobOnlyRenamesCurrent(t *testing.T cfg.Contexts["ctx-b"] = &clientcmdapi.Context{Cluster: "cluster-b", AuthInfo: "user-b"} cfg.CurrentContext = "ctx-a" - renameKubeconfigContext(cfg, "gh-prod") + g.Expect(renameKubeconfigContext(cfg, "gh-prod")).To(Succeed()) // ctx-a → gh-prod; ctx-b untouched g.Expect(cfg.Contexts).To(HaveKey("gh-prod")) @@ -296,7 +296,7 @@ func TestRenameKubeconfigContext_SharedClusterPreserved(t *testing.T) { cfg.Contexts["ctx-b"] = &clientcmdapi.Context{Cluster: "shared-cluster", AuthInfo: "shared-user"} cfg.CurrentContext = "ctx-a" - renameKubeconfigContext(cfg, "gh-prod") + g.Expect(renameKubeconfigContext(cfg, "gh-prod")).To(Succeed()) // The renamed context uses new keys. g.Expect(cfg.Contexts).To(HaveKey("gh-prod")) @@ -320,7 +320,7 @@ func TestRenameKubeconfigContext_ContextKeyMatchesButEntriesDiffer(t *testing.T) cfg.Contexts["greenhouse-org"] = &clientcmdapi.Context{Cluster: "greenhouse-org-cluster", AuthInfo: "greenhouse-org-user"} cfg.CurrentContext = "greenhouse-org" - renameKubeconfigContext(cfg, "greenhouse-org") + g.Expect(renameKubeconfigContext(cfg, "greenhouse-org")).To(Succeed()) // Context key unchanged. g.Expect(cfg.Contexts).To(HaveKey("greenhouse-org")) @@ -346,11 +346,33 @@ func TestRenameKubeconfigContext_NilContextEntryDoesNotPanic(t *testing.T) { cfg.CurrentContext = "greenhouse-org" g := NewWithT(t) - g.Expect(func() { renameKubeconfigContext(cfg, "gh-prod") }).NotTo(Panic()) + g.Expect(func() { _ = renameKubeconfigContext(cfg, "gh-prod") }).NotTo(Panic()) g.Expect(cfg.Contexts).To(HaveKey("gh-prod")) g.Expect(cfg.CurrentContext).To(Equal("gh-prod")) } +func TestRenameKubeconfigContext_RejectsCollisionWithUnrelatedEntry(t *testing.T) { + g := NewWithT(t) + + // A two-context blob: renaming ctx-a to "ctx-b" would clobber ctx-b's entries. + cfg := clientcmdapi.NewConfig() + cfg.Clusters["cluster-a"] = &clientcmdapi.Cluster{Server: "https://a.example.com"} + cfg.Clusters["ctx-b"] = &clientcmdapi.Cluster{Server: "https://b.example.com"} + cfg.AuthInfos["user-a"] = &clientcmdapi.AuthInfo{} + cfg.AuthInfos["ctx-b"] = &clientcmdapi.AuthInfo{} + cfg.Contexts["ctx-a"] = &clientcmdapi.Context{Cluster: "cluster-a", AuthInfo: "user-a"} + cfg.Contexts["ctx-b"] = &clientcmdapi.Context{Cluster: "ctx-b", AuthInfo: "ctx-b"} + cfg.CurrentContext = "ctx-a" + + err := renameKubeconfigContext(cfg, "ctx-b") + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("ctx-b")) + // Neither map was mutated. + g.Expect(cfg.Clusters).To(HaveKey("cluster-a")) + g.Expect(cfg.Contexts).To(HaveKey("ctx-a")) + g.Expect(cfg.CurrentContext).To(Equal("ctx-a")) +} + // ── mergeBootstrapKubeconfig ────────────────────────────────────────────────── func TestMergeBootstrapKubeconfig_AddsAllEntries(t *testing.T) { From fb826de3cb6ab852f9a943cf70ae876a1418c997 Mon Sep 17 00:00:00 2001 From: onuryilmaz Date: Tue, 6 Oct 2026 21:37:09 +0200 Subject: [PATCH 10/10] fix(bootstrap): honour CLOUDCTL_DRY_RUN and config-file dry-run value Reading dry-run only via cmd.Flags().GetBool ignored CLOUDCTL_DRY_RUN and a dry-run key in the cloudctl config file, contrary to the configuration contract. The flag is now checked first (to avoid viper cross-command pollution with sync's dry-run binding), with a fallback to viper.IsSet so env-var and config-file sources are respected. Signed-off-by: onuryilmaz --- cmd/bootstrap.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index 643c172..8c4972a 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -132,9 +132,16 @@ func runBootstrap(cmd *cobra.Command, args []string) error { } bootstrapContextName = viper.GetString("context-name") bootstrapSetCurrentCtx = viper.GetBool("set-current-context") - // Read dry-run directly from the flag to avoid viper cross-command pollution - // (sync also binds "dry-run" to the global viper instance). - bootstrapDryRun, _ = cmd.Flags().GetBool("dry-run") + // Read dry-run from the flag first to avoid viper cross-command pollution + // (sync also binds "dry-run" to the global viper instance). Fall back to + // viper.IsSet to honour CLOUDCTL_DRY_RUN and config-file values. + if cmd.Flags().Changed("dry-run") { + bootstrapDryRun, _ = cmd.Flags().GetBool("dry-run") + } else if viper.IsSet("dry-run") { + bootstrapDryRun = viper.GetBool("dry-run") + } else { + bootstrapDryRun = false + } format, err := output.ParseFormat(viper.GetString("output")) if err != nil {