From 3376ec8ebdf11d3ab07093c589ce65f4b6e5994a Mon Sep 17 00:00:00 2001 From: RaphaelBut Date: Mon, 6 Jul 2026 14:53:05 +0200 Subject: [PATCH] fix: support PSC cluster cleanup in break-glass and add --hive-ocm-url flag break-glass cleanup was not detecting GCP Private Service Connect (PSC) clusters, causing it to fall through to the local KUBECONFIG path instead of cleaning up jump pods on hive. Also adds --hive-ocm-url flag to the cleanup subcommand so it can connect to the correct hive shard when a non-default OCM environment was used during break-glass. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/cluster/access/access.go | 2 + cmd/cluster/access/cleanup.go | 72 ++++++++--- cmd/cluster/access/cleanup_test.go | 143 ++++++++++++++++++++- docs/README.md | 11 +- docs/osdctl_cluster_break-glass_cleanup.md | 15 ++- 5 files changed, 214 insertions(+), 29 deletions(-) diff --git a/cmd/cluster/access/access.go b/cmd/cluster/access/access.go index a91ce809a..61a7795ab 100644 --- a/cmd/cluster/access/access.go +++ b/cmd/cluster/access/access.go @@ -175,11 +175,13 @@ func (c *clusterAccessOptions) Run(ctx context.Context) error { // Original path - backward compatible hive, err = osdctlutil.GetHiveCluster(cluster.ID()) if err != nil { + c.Errorln("Hint: if the cluster's hive shard is in a different OCM environment, use --hive-ocm-url (e.g. --hive-ocm-url production)") return fmt.Errorf("failed to retrieve hive shard for %q: %w", c.clusterID, err) } hiveClient, err = k8s.NewAsBackplaneClusterAdmin(hive.ID(), kclient.Options{Scheme: scheme.Scheme}, c.reason, fmt.Sprintf("Elevation required to break-glass on %q cluster", c.clusterID)) if err != nil { + c.Errorln("Hint: if the cluster's hive shard is in a different OCM environment, use --hive-ocm-url (e.g. --hive-ocm-url production)") return fmt.Errorf("failed to login to hive shard %q: %w", hive.Name(), err) } } diff --git a/cmd/cluster/access/cleanup.go b/cmd/cluster/access/cleanup.go index 5acc26e90..45ec89976 100644 --- a/cmd/cluster/access/cleanup.go +++ b/cmd/cluster/access/cleanup.go @@ -7,6 +7,7 @@ import ( fpath "path/filepath" "strings" + sdk "github.com/openshift-online/ocm-sdk-go" clustersmgmtv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" "github.com/openshift/osdctl/pkg/k8s" osdctlutil "github.com/openshift/osdctl/pkg/utils" @@ -16,6 +17,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/kubernetes/scheme" cmdutil "k8s.io/kubectl/pkg/cmd/util" kclient "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -25,7 +27,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 Private\nService Connect (PSC), it deletes all jump pods in the cluster's namespace (because of this, you\nmust be logged into the hive shard when dropping access for PrivateLink/PSC clusters). For other\nclusters, the $KUBECONFIG environment 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 +38,8 @@ 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.Flags().StringVar(&ops.hiveOcmUrl, "hive-ocm-url", "", "(optional) OCM environment URL for Hive operations. Aliases: 'production', 'staging', 'integration'. This only changes how the Hive cluster is resolved; the target cluster still comes from the current/default OCM environment.") _ = cleanupCmd.MarkFlagRequired("cluster-id") @@ -48,13 +51,25 @@ func cleanupCmdComplete(cmd *cobra.Command) error { if clusterID == "" { return cmdutil.UsageErrorf(cmd, "The cluster-id flag is required") } - return osdctlutil.IsValidClusterKey(clusterID) + if err := osdctlutil.IsValidClusterKey(clusterID); err != nil { + return err + } + + hiveOcmUrl, _ := cmd.Flags().GetString("hive-ocm-url") + if hiveOcmUrl != "" { + if _, err := osdctlutil.ValidateAndResolveOcmUrl(hiveOcmUrl); err != nil { + return fmt.Errorf("invalid --hive-ocm-url: %w", err) + } + } + + return nil } // cleanupAccessOptions contains the objects and information required to drop access to a cluster type cleanupAccessOptions struct { - reason string - clusterID string + reason string + clusterID string + hiveOcmUrl string genericclioptions.IOStreams kubeCli *k8s.LazyClient @@ -106,8 +121,9 @@ 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() { - return c.dropPrivateLinkAccess(cluster) + isPscCluster := cluster.GCP().PrivateServiceConnect().ServiceAttachmentSubnet() != "" + if cluster.AWS().PrivateLink() || isPscCluster { + return c.dropPrivateLinkAccess(cluster, conn) } else { return c.dropLocalAccess(cluster) } @@ -115,17 +131,41 @@ func (c *cleanupAccessOptions) Run(cmd *cobra.Command) error { // dropPrivateLinkAccess removes access to a PrivateLink 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 { +func (c *cleanupAccessOptions) dropPrivateLinkAccess(cluster *clustersmgmtv1.Cluster, conn *sdk.Connection) 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 Private Service Connect") + return fmt.Errorf("flag \"reason\" not set and is required when Cluster is PrivateLink or Private Service Connect") } - c.kubeCli.Impersonate("backplane-cluster-admin", c.reason, fmt.Sprintf("Elevation required to clean break-glass on PrivateLink Clusters")) - c.Println("Cluster is PrivateLink - removing jump pods in the cluster's namespace.") - ns, err := getClusterNamespace(c.kubeCli, cluster.ID()) + var hiveClient kclient.Client + if c.hiveOcmUrl != "" { + hiveOCM, err := osdctlutil.CreateConnectionWithUrl(c.hiveOcmUrl) + if err != nil { + return fmt.Errorf("failed to create hive OCM connection with URL '%s': %w", c.hiveOcmUrl, err) + } + defer hiveOCM.Close() + + hive, err := osdctlutil.GetHiveClusterWithConn(cluster.ID(), conn, hiveOCM) + if err != nil { + return fmt.Errorf("failed to retrieve hive shard for %q (OCM URL:'%s'): %w", cluster.ID(), c.hiveOcmUrl, err) + } + + hiveClient, err = k8s.NewAsBackplaneClusterAdminWithConn(hive.ID(), kclient.Options{Scheme: scheme.Scheme}, hiveOCM, c.reason, "Elevation required to clean break-glass on PrivateLink/PSC Clusters") + if err != nil { + return fmt.Errorf("failed to login to hive shard %q (OCM URL:'%s'): %w", hive.Name(), c.hiveOcmUrl, err) + } + } else { + c.kubeCli.Impersonate("backplane-cluster-admin", c.reason, "Elevation required to clean break-glass on PrivateLink/PSC Clusters") + hiveClient = c.kubeCli + } + + c.Println("Cluster is PrivateLink or Private Service Connect - removing jump pods in the cluster's namespace.") + ns, err := getClusterNamespace(hiveClient, cluster.ID()) if err != nil { c.Errorln("Failed to retrieve cluster namespace") + if c.hiveOcmUrl == "" { + c.Errorln("Hint: if the cluster's hive shard is in a different OCM environment, use --hive-ocm-url (e.g. --hive-ocm-url production)") + } return err } @@ -139,7 +179,7 @@ func (c *cleanupAccessOptions) dropPrivateLinkAccess(cluster *clustersmgmtv1.Clu listOpts := kclient.ListOptions{Namespace: ns.Name, LabelSelector: selector} pods := corev1.PodList{} - err = c.kubeCli.List(context.TODO(), &pods, &listOpts) + err = hiveClient.List(context.TODO(), &pods, &listOpts) if err != nil { c.Errorln(fmt.Sprintf("Failed to list pods in cluster namespace '%s'", ns.Name)) return err @@ -166,7 +206,7 @@ func (c *cleanupAccessOptions) dropPrivateLinkAccess(cluster *clustersmgmtv1.Clu } if isAffirmative(input) { pod := corev1.Pod{} - err = c.kubeCli.DeleteAllOf(context.TODO(), &pod, &kclient.DeleteAllOfOptions{ListOptions: listOpts}) + err = hiveClient.DeleteAllOf(context.TODO(), &pod, &kclient.DeleteAllOfOptions{ListOptions: listOpts}) if err != nil { c.Errorln("Failed to delete pod(s)") return err @@ -178,7 +218,7 @@ func (c *cleanupAccessOptions) dropPrivateLinkAccess(cluster *clustersmgmtv1.Clu // and we end up waiting for irrelevant pods. I've tried reproducing this bug in other places, but I haven't been able to // figure it out. If someone does, please fix it. pods := corev1.PodList{} - err = c.kubeCli.List(context.TODO(), &pods, &listOpts) + err = hiveClient.List(context.TODO(), &pods, &listOpts) if err != nil || len(pods.Items) != 0 { return false, err } diff --git a/cmd/cluster/access/cleanup_test.go b/cmd/cluster/access/cleanup_test.go index ab6725a17..76d6af694 100644 --- a/cmd/cluster/access/cleanup_test.go +++ b/cmd/cluster/access/cleanup_test.go @@ -8,7 +8,9 @@ import ( "strings" "testing" + clustersmgmtv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" "github.com/openshift/osdctl/pkg/k8s" + osdctlutil "github.com/openshift/osdctl/pkg/utils" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -110,7 +112,7 @@ func TestCleanupAccessOptions_dropPrivateLinkAccess(t *testing.T) { cluster := generateClusterObjectForTesting("fake-cluster", clusterid, true, false) // Run test - err = cleanupAccess.dropPrivateLinkAccess(&cluster) + err = cleanupAccess.dropPrivateLinkAccess(&cluster, nil) // Verify results if err != nil { @@ -135,3 +137,142 @@ func TestCleanupAccessOptions_dropPrivateLinkAccess(t *testing.T) { } } } + +func TestCleanupAccessOptions_dropPscAccess(t *testing.T) { + const clusterid = "fake-psc-cluster-uuid" + + tests := []struct { + Name string + Pods []metav1.ObjectMeta + ExpectedPodsAfter []string + }{ + { + Name: "PSC cluster - single jump pod", + Pods: []metav1.ObjectMeta{ + { + Name: "jump", + Labels: map[string]string{jumpPodLabelKey: clusterid}, + }, + }, + ExpectedPodsAfter: []string{}, + }, + { + Name: "PSC cluster - no pods", + Pods: []metav1.ObjectMeta{}, + ExpectedPodsAfter: []string{}, + }, + { + Name: "PSC cluster - mixed 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) + + 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) + } + + 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) + cleanupAccess.reason = "testing-reason" + + cluster := generatePscClusterObjectForTesting("fake-psc-cluster", clusterid) + + err = cleanupAccess.dropPrivateLinkAccess(&cluster, nil) + if err != nil { + t.Fatalf("Failed '%s': unexpected error encountered: %v", test.Name, err) + } + + 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) + } + } + } +} + +func TestCleanupHiveOcmUrlValidation(t *testing.T) { + tests := []struct { + name string + hiveOcmUrl string + expectError bool + }{ + {name: "Valid (production)", hiveOcmUrl: "production", expectError: false}, + {name: "Valid (staging)", hiveOcmUrl: "staging", expectError: false}, + {name: "Valid (integration)", hiveOcmUrl: "integration", expectError: false}, + {name: "Invalid", hiveOcmUrl: "invalid-environment", expectError: true}, + {name: "Empty (flag omitted)", hiveOcmUrl: "", expectError: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.hiveOcmUrl != "" { + _, err := osdctlutil.ValidateAndResolveOcmUrl(tt.hiveOcmUrl) + if tt.expectError && err == nil { + t.Errorf("expected error for hive-ocm-url %q, got nil", tt.hiveOcmUrl) + } + if !tt.expectError && err != nil { + t.Errorf("unexpected error for hive-ocm-url %q: %v", tt.hiveOcmUrl, err) + } + } + }) + } +} + +func generatePscClusterObjectForTesting(name, id string) clustersmgmtv1.Cluster { + cluster, err := clustersmgmtv1.NewCluster(). + Name(name). + ID(id). + GCP(clustersmgmtv1.NewGCP().PrivateServiceConnect( + clustersmgmtv1.NewGcpPrivateServiceConnect().ServiceAttachmentSubnet("test-subnet"), + )). + API(clustersmgmtv1.NewClusterAPI().Listening(clustersmgmtv1.ListeningMethodExternal)). + Build() + + if err != nil { + panic(fmt.Sprintf("Failed to build PSC cluster: %v", err)) + } + return *cluster +} diff --git a/docs/README.md b/docs/README.md index 3c2de155e..952ea9f20 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1285,10 +1285,10 @@ osdctl cluster break-glass --cluster-id [flags] ### osdctl cluster break-glass cleanup -Relinquish emergency access from the given cluster. If the cluster is PrivateLink, it deletes -all jump pods in the cluster's namespace (because of this, you must be logged into the hive shard -when dropping access for PrivateLink clusters). For non-PrivateLink clusters, the $KUBECONFIG -environment variable is unset, if applicable. +Relinquish emergency access from the given cluster. If the cluster is PrivateLink or Private +Service Connect (PSC), it deletes all jump pods in the cluster's namespace (because of this, you +must be logged into the hive shard when dropping access for PrivateLink/PSC clusters). For other +clusters, the $KUBECONFIG environment variable is unset, if applicable. ``` osdctl cluster break-glass cleanup --cluster-id [flags] @@ -1302,10 +1302,11 @@ osdctl cluster break-glass cleanup --cluster-id [flags] -C, --cluster-id string [Mandatory] Provide the Internal ID of the cluster --context string The name of the kubeconfig context to use -h, --help help for cleanup + --hive-ocm-url string (optional) OCM environment URL for Hive operations. Aliases: 'production', 'staging', 'integration'. This only changes how the Hive cluster is resolved; the target cluster still comes from the current/default OCM environment. --insecure-skip-tls-verify If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure --kubeconfig string Path to the kubeconfig file to use for CLI requests. -o, --output string Valid formats are ['', 'json', 'yaml', 'env'] - --reason string [Mandatory for PrivateLink clusters] The reason for this command, which requires elevation, to be run (usualy an OHSS or PD ticket) + --reason string [Mandatory for PrivateLink/PSC clusters] The reason for this command, which requires elevation, to be run (usually an OHSS or PD ticket) --request-timeout string The length of time to wait before giving up on a single server request. Non-zero values should contain a corresponding time unit (e.g. 1s, 2m, 3h). A value of zero means don't timeout requests. (default "0") -s, --server string The address and port of the Kubernetes API server --skip-aws-proxy-check aws_proxy Don't use the configured aws_proxy value diff --git a/docs/osdctl_cluster_break-glass_cleanup.md b/docs/osdctl_cluster_break-glass_cleanup.md index 6fe838953..ba4cfe9ec 100644 --- a/docs/osdctl_cluster_break-glass_cleanup.md +++ b/docs/osdctl_cluster_break-glass_cleanup.md @@ -4,10 +4,10 @@ Drop emergency access to a cluster ### Synopsis -Relinquish emergency access from the given cluster. If the cluster is PrivateLink, it deletes -all jump pods in the cluster's namespace (because of this, you must be logged into the hive shard -when dropping access for PrivateLink clusters). For non-PrivateLink clusters, the $KUBECONFIG -environment variable is unset, if applicable. +Relinquish emergency access from the given cluster. If the cluster is PrivateLink or Private +Service Connect (PSC), it deletes all jump pods in the cluster's namespace (because of this, you +must be logged into the hive shard when dropping access for PrivateLink/PSC clusters). For other +clusters, the $KUBECONFIG environment variable is unset, if applicable. ``` osdctl cluster break-glass cleanup --cluster-id [flags] @@ -23,9 +23,10 @@ osdctl cluster break-glass cleanup --cluster-id [flags] ### Options ``` - -C, --cluster-id string [Mandatory] Provide the Internal ID of the cluster - -h, --help help for cleanup - --reason string [Mandatory for PrivateLink clusters] The reason for this command, which requires elevation, to be run (usualy an OHSS or PD ticket) + -C, --cluster-id string [Mandatory] Provide the Internal ID of the cluster + -h, --help help for cleanup + --hive-ocm-url string (optional) OCM environment URL for Hive operations. Aliases: 'production', 'staging', 'integration'. This only changes how the Hive cluster is resolved; the target cluster still comes from the current/default OCM environment. + --reason string [Mandatory for PrivateLink/PSC clusters] The reason for this command, which requires elevation, to be run (usually an OHSS or PD ticket) ``` ### Options inherited from parent commands