From b45725cf7b01a1cc062cece3981c8cb9f641f921 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 22:30:51 +0000 Subject: [PATCH] fix(#943): handle GCP PSC clusters in break-glass cleanup The cleanup command only checked for AWS PrivateLink clusters when deciding whether to delete jump pods on hive. GCP Private Service Connect (PSC) clusters also use jump pods (created by the access command which already handles PSC), but cleanup treated them as non-PrivateLink and only unset KUBECONFIG instead. Add the same PSC detection used in access.go to cleanup.go so PSC clusters are routed to dropPrivateLinkAccess. Update command descriptions, error messages, and log output to reference PSC alongside PrivateLink. Fix pre-existing typo (usualy -> usually) and unnecessary fmt.Sprintf on lines touched by this change. Add generatePSCClusterObjectForTesting helper and a dedicated PSC test suite mirroring the existing PrivateLink cleanup tests. Closes #943 --- cmd/cluster/access/access_test.go | 17 +++++ cmd/cluster/access/cleanup.go | 17 ++--- cmd/cluster/access/cleanup_test.go | 105 +++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 8 deletions(-) diff --git a/cmd/cluster/access/access_test.go b/cmd/cluster/access/access_test.go index c8355a1c6..c682dab03 100644 --- a/cmd/cluster/access/access_test.go +++ b/cmd/cluster/access/access_test.go @@ -336,6 +336,23 @@ func generateClusterObjectForTesting(name string, id string, privateLink bool, p return *cluster } +// generatePSCClusterObjectForTesting creates a non-functional GCP PSC cluster object solely for testing purposes +func generatePSCClusterObjectForTesting(name string, id string) clustersmgmtv1.Cluster { + cluster, err := clustersmgmtv1.NewCluster(). + Name(name). + ID(id). + GCP(clustersmgmtv1.NewGCP().PrivateServiceConnect( + clustersmgmtv1.NewGcpPrivateServiceConnect().ServiceAttachmentSubnet("psc-subnet"), + )). + API(clustersmgmtv1.NewClusterAPI().Listening(clustersmgmtv1.ListeningMethodExternal)). + Build() + + if err != nil { + panic(fmt.Sprintf("Failed to build cluster: %v", err)) + } + return *cluster +} + // generateKubeconfigSecretObjectForTesting creates a Secret containing a kubeconfig file for testing purposes func generateKubeconfigSecretObjectForTesting(name, namespace, key, serverURL string) (corev1.Secret, clientcmdapiv1.Config) { kubeconfig := clientcmdapiv1.Config{ diff --git a/cmd/cluster/access/cleanup.go b/cmd/cluster/access/cleanup.go index 5acc26e90..8234b4a27 100644 --- a/cmd/cluster/access/cleanup.go +++ b/cmd/cluster/access/cleanup.go @@ -25,7 +25,7 @@ func newCmdCleanup(client *k8s.LazyClient, streams genericclioptions.IOStreams) cleanupCmd := &cobra.Command{ Use: "cleanup --cluster-id ", Short: "Drop emergency access to a cluster", - Long: "Relinquish emergency access from the given cluster. If the cluster is PrivateLink, it deletes\nall jump pods in the cluster's namespace (because of this, you must be logged into the hive shard\nwhen dropping access for PrivateLink clusters). For non-PrivateLink clusters, the $KUBECONFIG\nenvironment variable is unset, if applicable.", + Long: "Relinquish emergency access from the given cluster. If the cluster is PrivateLink or\nGCP Private Service Connect (PSC), it deletes all jump pods in the cluster's namespace\n(because of this, you must be logged into the hive shard when dropping access for\nPrivateLink/PSC clusters). For non-PrivateLink/non-PSC clusters, the $KUBECONFIG\nenvironment variable is unset, if applicable.", Example: ` # Drop emergency access to a cluster osdctl cluster break-glass cleanup --cluster-id ${CLUSTER_ID}`, Args: cobra.NoArgs, @@ -36,7 +36,7 @@ func newCmdCleanup(client *k8s.LazyClient, streams genericclioptions.IOStreams) }, } cleanupCmd.Flags().StringVarP(&ops.clusterID, "cluster-id", "C", "", "[Mandatory] Provide the Internal ID of the cluster") - cleanupCmd.Flags().StringVar(&ops.reason, "reason", "", "[Mandatory for PrivateLink clusters] The reason for this command, which requires elevation, to be run (usualy an OHSS or PD ticket)") + cleanupCmd.Flags().StringVar(&ops.reason, "reason", "", "[Mandatory for PrivateLink/PSC clusters] The reason for this command, which requires elevation, to be run (usually an OHSS or PD ticket)") _ = cleanupCmd.MarkFlagRequired("cluster-id") @@ -106,23 +106,24 @@ func (c *cleanupAccessOptions) Run(cmd *cobra.Command) error { return err } c.Println(fmt.Sprintf("Dropping access to cluster '%s'", cluster.Name())) - if cluster.AWS().PrivateLink() { + isPscCluster := cluster.GCP().PrivateServiceConnect().ServiceAttachmentSubnet() != "" + if cluster.AWS().PrivateLink() || isPscCluster { return c.dropPrivateLinkAccess(cluster) } else { return c.dropLocalAccess(cluster) } } -// dropPrivateLinkAccess removes access to a PrivateLink cluster. +// dropPrivateLinkAccess removes access to a PrivateLink or PSC cluster. // This primarily consists of deleting any jump pods found to be running against the cluster in hive. func (c *cleanupAccessOptions) dropPrivateLinkAccess(cluster *clustersmgmtv1.Cluster) error { if c.reason == "" { - c.Errorln("flag \"reason\" not set and is required when Cluster is PrivateLink") - return fmt.Errorf("flag \"reason\" not set and is required when Cluster is PrivateLink") + c.Errorln("flag \"reason\" not set and is required when Cluster is PrivateLink or PSC") + return fmt.Errorf("flag \"reason\" not set and is required when Cluster is PrivateLink or PSC") } - c.kubeCli.Impersonate("backplane-cluster-admin", c.reason, fmt.Sprintf("Elevation required to clean break-glass on PrivateLink Clusters")) + c.kubeCli.Impersonate("backplane-cluster-admin", c.reason, "Elevation required to clean break-glass on PrivateLink/PSC Clusters") - c.Println("Cluster is PrivateLink - removing jump pods in the cluster's namespace.") + c.Println("Cluster is PrivateLink or PSC - removing jump pods in the cluster's namespace.") ns, err := getClusterNamespace(c.kubeCli, cluster.ID()) if err != nil { c.Errorln("Failed to retrieve cluster namespace") diff --git a/cmd/cluster/access/cleanup_test.go b/cmd/cluster/access/cleanup_test.go index ab6725a17..ae3597578 100644 --- a/cmd/cluster/access/cleanup_test.go +++ b/cmd/cluster/access/cleanup_test.go @@ -135,3 +135,108 @@ func TestCleanupAccessOptions_dropPrivateLinkAccess(t *testing.T) { } } } + +func TestCleanupAccessOptions_dropPrivateLinkAccess_PSCCluster(t *testing.T) { + const ( + clusterid = "fake-psc-cluster-uuid-12345" + ) + + tests := []struct { + Name string + Pods []metav1.ObjectMeta + ExpectedPodsAfter []string + }{ + { + Name: "PSC Single Jump Pod", + Pods: []metav1.ObjectMeta{ + { + Name: "jump", + Labels: map[string]string{jumpPodLabelKey: clusterid}, + }, + }, + ExpectedPodsAfter: []string{}, + }, + { + Name: "PSC No pods", + Pods: []metav1.ObjectMeta{}, + ExpectedPodsAfter: []string{}, + }, + { + Name: "PSC Mixed use pods", + Pods: []metav1.ObjectMeta{ + { + Name: "jump", + Labels: map[string]string{jumpPodLabelKey: clusterid}, + }, + { + Name: "provision", + Labels: map[string]string{"a-provisioning-pod-label": "testing"}, + }, + }, + ExpectedPodsAfter: []string{"provision"}, + }, + } + + for _, test := range tests { + fmt.Printf("Testing '%s'\n", test.Name) + + // Generate test objects + objs := []runtime.Object{} + ns := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("uhc-staging-%s", clusterid), + Labels: map[string]string{"api.openshift.com/id": clusterid}, + }, + } + objs = append(objs, &ns) + + for _, objMeta := range test.Pods { + pod := corev1.Pod{ + ObjectMeta: objMeta, + } + pod.Namespace = ns.Name + objs = append(objs, &pod) + } + + // Setup Environment + scheme := runtime.NewScheme() + err := corev1.AddToScheme(scheme) + if err != nil { + t.Fatalf("Failed '%s': to add corev1 to scheme: %v", test.Name, err) + } + + client := k8s.NewFakeClient(fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(objs...)) + streams := genericclioptions.IOStreams{In: strings.NewReader("y\n"), Out: os.Stdout, ErrOut: os.Stderr} + cleanupAccess := newCleanupAccessOptions(client, streams) + + // Set the required "reason" flag for PSC clusters + cleanupAccess.reason = "testing-reason" + + cluster := generatePSCClusterObjectForTesting("fake-psc-cluster", clusterid) + + // Run test + err = cleanupAccess.dropPrivateLinkAccess(&cluster) + + // Verify results + if err != nil { + t.Fatalf("Failed '%s': unexpected error encountered: %v", test.Name, err) + } + + // Verify only expected pods remain + podsAfter := corev1.PodList{} + err = cleanupAccess.kubeCli.List(context.TODO(), &podsAfter) + if err != nil { + t.Fatalf("Failed '%s': error while listing pods after testing: %v", test.Name, err) + } + + if len(podsAfter.Items) != len(test.ExpectedPodsAfter) { + t.Errorf("Failed '%s': unexpected number of pods remain after test: expected %d, got %d", test.Name, len(test.ExpectedPodsAfter), len(podsAfter.Items)) + } + + for _, pod := range podsAfter.Items { + if !slices.Contains(test.ExpectedPodsAfter, pod.Name) { + t.Errorf("Failed '%s': unexpected pod remains after test: %s", test.Name, pod.Name) + } + } + } +}