fix(adhoc-sweep-fixes): 3 review findings across 3 files

internal/cluster/prerequisites/k3d/k3d_test.go

@@ -2,6 +2,7 @@ package k3d
 
 import (
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/flamingo-stack/openframe-cli/internal/shared/download"
@@ -25,15 +26,15 @@ func TestK3dInstaller_GetInstallHelp(t *testing.T) {
 
 	switch runtime.GOOS {
 	case "darwin":
-		if !containsSubstring(help, "brew") && !containsSubstring(help, "https://") {
+		if !strings.Contains(help, "brew") && !strings.Contains(help, "https://") {
 			t.Errorf("macOS help should contain brew or https reference: %s", help)
 		}
 	case "linux":
-		if !containsSubstring(help, "curl") && !containsSubstring(help, "https://") {
+		if !strings.Contains(help, "curl") && !strings.Contains(help, "https://") {
 			t.Errorf("Linux help should contain curl or https reference: %s", help)
 		}
 	case "windows":
-		if !containsSubstring(help, "https://") && !containsSubstring(help, "chocolatey") {
+		if !strings.Contains(help, "https://") && !strings.Contains(help, "chocolatey") {
 			t.Errorf("Windows help should contain https or chocolatey reference: %s", help)
 		}
 	}
@@ -59,7 +60,7 @@ func TestK3dInstaller_Install(t *testing.T) {
 	if err == nil {
 		t.Fatal("expected an error when no install tooling is available")
 	}
-	if !containsSubstring(err.Error(), "Homebrew") {
+	if !strings.Contains(err.Error(), "Homebrew") {
 		t.Errorf("expected a Homebrew hint, got: %v", err)
 	}
 }
@@ -89,16 +90,3 @@ func TestVerifiedInstallHasPinnedAsset(t *testing.T) {
 		t.Errorf("no pinned k3d asset for %s/%s", runtime.GOOS, runtime.GOARCH)
 	}
 }
-
-// Helper function to check if a string contains a substring
-func containsSubstring(str, substr string) bool {
-	return len(str) >= len(substr) &&
-		func() bool {
-			for i := 0; i <= len(str)-len(substr); i++ {
-				if str[i:i+len(substr)] == substr {
-					return true
-				}
-			}
-			return false
-		}()
-}

internal/cluster/providers/gke/kubeconfig.go

@@ -1,13 +1,11 @@
 package gke
 
 import (
-	"encoding/base64"
-	"fmt"
-
 	tfengine "github.com/flamingo-stack/openframe-cli/internal/cluster/providers/terraform"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
+
+	kubeconfighelper "github.com/flamingo-stack/openframe-cli/internal/cluster/providers/kubeconfig"
 )
 
 // GKE kubeconfig entries carry no static credentials: authentication runs
@@ -25,43 +23,23 @@
 
 // caData decodes the base64 CA bundle the GKE module outputs.
 func caData(rec tfengine.Record) ([]byte, error) {
-	ca, err := base64.StdEncoding.DecodeString(rec.CACert)
-	if err != nil {
-		return nil, fmt.Errorf("decoding cluster CA for %s: %w", rec.Name, err)
-	}
-	return ca, nil
+	return kubeconfighelper.CAData(rec)
 }
 
 // kubeconfigFor renders an in-memory kubeconfig with a single context named
 // after the cluster — the plain name so the rest of the CLI resolves it by
 // exact match.
 func kubeconfigFor(rec tfengine.Record) (*clientcmdapi.Config, error) {
-	ca, err := caData(rec)
-	if err != nil {
-		return nil, err
-	}
-	cfg := clientcmdapi.NewConfig()
-	cfg.Clusters[rec.Name] = &clientcmdapi.Cluster{
-		Server:                   rec.Endpoint,
-		CertificateAuthorityData: ca,
-	}
-	cfg.AuthInfos[rec.Name] = &clientcmdapi.AuthInfo{Exec: execConfig()}
-	cfg.Contexts[rec.Name] = &clientcmdapi.Context{Cluster: rec.Name, AuthInfo: rec.Name}
-	cfg.CurrentContext = rec.Name
-	return cfg, nil
+	return kubeconfighelper.KubeconfigFor(rec, func(tfengine.Record) *clientcmdapi.ExecConfig {
+		return execConfig()
+	})
 }
 
 // restConfigFor builds a rest.Config straight from the record.
 func restConfigFor(rec tfengine.Record) (*rest.Config, error) {
-	ca, err := caData(rec)
-	if err != nil {
-		return nil, err
-	}
-	return &rest.Config{
-		Host:            rec.Endpoint,
-		TLSClientConfig: rest.TLSClientConfig{CAData: ca},
-		ExecProvider:    execConfig(),
-	}, nil
+	return kubeconfighelper.RestConfigFor(rec, func(tfengine.Record) *clientcmdapi.ExecConfig {
+		return execConfig()
+	})
 }
 
 // mergeIntoDefaultKubeconfig writes the cluster's context into the user's
@@ -70,50 +48,15 @@
 // server: that context belongs to something else (another cluster, another
 // tool) and silently clobbering it would break the user's access to it.
 func mergeIntoDefaultKubeconfig(rec tfengine.Record) error {
-	pathOpts := clientcmd.NewDefaultPathOptions()
-	existing, err := pathOpts.GetStartingConfig()
-	if err != nil {
-		return fmt.Errorf("loading kubeconfig: %w", err)
-	}
-	if prior, ok := existing.Contexts[rec.Name]; ok {
-		if cluster, ok := existing.Clusters[prior.Cluster]; ok && cluster.Server != rec.Endpoint {
-			return fmt.Errorf("kubeconfig context '%s' already exists and points at %s — refusing to overwrite it; rename the existing context or pick another cluster name", rec.Name, cluster.Server)
-		}
-	}
-	generated, err := kubeconfigFor(rec)
-	if err != nil {
-		return err
-	}
-	existing.Clusters[rec.Name] = generated.Clusters[rec.Name]
-	existing.AuthInfos[rec.Name] = generated.AuthInfos[rec.Name]
-	existing.Contexts[rec.Name] = generated.Contexts[rec.Name]
-	existing.CurrentContext = rec.Name
-	if err := clientcmd.ModifyConfig(pathOpts, *existing, true); err != nil {
-		return fmt.Errorf("writing kubeconfig: %w", err)
-	}
-	return nil
+	return kubeconfighelper.MergeIntoDefaultKubeconfig(rec, func(tfengine.Record) *clientcmdapi.ExecConfig {
+		return execConfig()
+	})
 }
 
 // removeFromDefaultKubeconfig drops the cluster's context after a destroy —
 // but ONLY when the entry still points at OUR endpoint. If the user repointed
 // or recreated a same-named context toward another server since the create,
 // it is no longer ours to delete (the create-side no-clobber guard's mirror).
 func removeFromDefaultKubeconfig(rec tfengine.Record) error {
-	pathOpts := clientcmd.NewDefaultPathOptions()
-	existing, err := pathOpts.GetStartingConfig()
-	if err != nil {
-		return err
-	}
-	if prior, ok := existing.Contexts[rec.Name]; ok {
-		if cluster, ok := existing.Clusters[prior.Cluster]; ok && cluster.Server != rec.Endpoint {
-			return nil // same name, different server — not ours anymore
-		}
-	}
-	delete(existing.Clusters, rec.Name)
-	delete(existing.AuthInfos, rec.Name)
-	delete(existing.Contexts, rec.Name)
-	if existing.CurrentContext == rec.Name {
-		existing.CurrentContext = ""
-	}
-	return clientcmd.ModifyConfig(pathOpts, *existing, true)
+	return kubeconfighelper.RemoveFromDefaultKubeconfig(rec)
 }

internal/cluster/providers/gke/teardown.go

@@ -6,29 +6,24 @@
 	"strings"
 	"time"
 
+	"github.com/flamingo-stack/openframe-cli/internal/cluster/providers/shared"
 	tfengine "github.com/flamingo-stack/openframe-cli/internal/cluster/providers/terraform"
 	sharedUI "github.com/flamingo-stack/openframe-cli/internal/shared/ui"
 	"github.com/pterm/pterm"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
 )
 
-// systemNamespaces and systemNamespacePrefixes are never deleted during
+// systemNamespacePrefixes lists the GKE-specific system namespace prefixes
+// (in addition to the shared "kube-" prefix) that are never deleted during
 // teardown. They are the cluster's own control-plane/system namespaces (torn
 // down with the cluster anyway) and — critically — kube-system hosts the GKE PD
 // CSI controller that must keep running to delete the Persistent Disks as their
 // PVCs go away.
-var systemNamespaces = map[string]struct{}{
-	"default":         {},
-	"kube-system":     {},
-	"kube-public":     {},
-	"kube-node-lease": {},
-}
-
-var systemNamespacePrefixes = []string{"kube-", "gke-", "gmp-"}
+var systemNamespacePrefixes = []string{"gke-", "gmp-"}
 
 const (
 	// diskDrainTimeout bounds how long a delete waits for PVC-backed disks to be
@@ -41,59 +36,6 @@
 	kubeCallTimeout = 20 * time.Second
 )
 
-// isSystemNamespace reports whether ns is a cluster/system namespace that
-// teardown must never delete.
-func isSystemNamespace(ns string) bool {
-	if _, ok := systemNamespaces[ns]; ok {
-		return true
-	}
-	for _, p := range systemNamespacePrefixes {
-		if strings.HasPrefix(ns, p) {
-			return true
-		}
-	}
-	return false
-}
-
-// appNamespacesToDelete returns the application namespaces (everything that is
-// not a system namespace), with argocd first. OpenFrame's stateful services
-// (Kafka, MongoDB, Cassandra, Pinot, … in the 'datasources' namespace) hold the
-// PVCs whose backing disks must be released; deleting by discovery rather than a
-// hardcoded list keeps this correct as the platform layout changes. argocd goes
-// first so its controller stops re-syncing before the workloads it manages are
-// deleted, otherwise self-heal could recreate a StatefulSet (and its PVC)
-// mid-teardown.
-func appNamespacesToDelete(all []string) []string {
-	var argocd []string
-	var rest []string
-	for _, ns := range all {
-		if isSystemNamespace(ns) {
-			continue
-		}
-		if ns == "argocd" {
-			argocd = append(argocd, ns)
-		} else {
-			rest = append(rest, ns)
-		}
-	}
-	return append(argocd, rest...)
-}
-
-// countDeletablePVs counts PersistentVolumes whose reclaim policy is Delete.
-// These are the volumes whose backing cloud disk the CSI driver removes once
-// their PVC is gone, so the release step waits for this to reach zero.
-// Retain-policy PVs are excluded on purpose — their disks are meant to survive,
-// and the post-destroy sweep reports (never silently drops) them.
-func countDeletablePVs(pvs []corev1.PersistentVolume) int {
-	var n int
-	for _, pv := range pvs {
-		if pv.Spec.PersistentVolumeReclaimPolicy == corev1.PersistentVolumeReclaimDelete {
-			n++
-		}
-	}
-	return n
-}
-
 // releaseWorkloadDisks deletes every application namespace on the cluster and
 // waits (bounded) for the Delete-reclaim PersistentVolumes to drain, so the GKE
 // CSI driver deletes the backing Persistent Disks BEFORE terraform destroys the
@@ -130,7 +72,7 @@
 	for _, ns := range nsList.Items {
 		names = append(names, ns.Name)
 	}
-	targets := appNamespacesToDelete(names)
+	targets := shared.AppNamespacesToDelete(names, systemNamespacePrefixes)
 	if len(targets) == 0 {
 		return // no application namespaces — nothing to release
 	}
@@ -155,7 +97,7 @@
 		if err != nil {
 			return false, nil // transient — keep polling until the outer timeout
 		}
-		return countDeletablePVs(pvs.Items) == 0, nil
+		return shared.CountDeletablePVs(pvs.Items) == 0, nil
 	})
 }