From 22c0e70c4ddaee13014e5a908dc5e429bbc3b0f5 Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Wed, 5 Aug 2026 16:06:26 +0200 Subject: [PATCH] CONSOLE-5209: Add HCP compatibility for IngressComponentRouteLabels e2e tests On HyperShift clusters a ValidatingAdmissionPolicy blocks direct modification of ingresses.config.openshift.io/cluster on the guest cluster. Branch the mutation helpers to modify the HostedCluster CR's spec.configuration.ingress.componentRoutes via the management cluster dynamic client on HCP, while keeping the existing typed client path on standalone OCP. Use 25m/10s poll timeout/interval on HCP to account for the multi-hop reconciliation chain. Co-Authored-By: Claude Opus 4.6 (1M context) --- test/extended/console/label_propagation.go | 266 +++++++++++++++++---- 1 file changed, 221 insertions(+), 45 deletions(-) diff --git a/test/extended/console/label_propagation.go b/test/extended/console/label_propagation.go index d89aea87d635..b19416427850 100644 --- a/test/extended/console/label_propagation.go +++ b/test/extended/console/label_propagation.go @@ -3,6 +3,7 @@ package console import ( "context" "fmt" + "os" "strings" "time" @@ -13,9 +14,13 @@ import ( routev1 "github.com/openshift/api/route/v1" configclient "github.com/openshift/client-go/config/clientset/versioned" routeclient "github.com/openshift/client-go/route/clientset/versioned" + exutil "github.com/openshift/origin/test/extended/util" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/dynamic" "k8s.io/client-go/util/retry" e2e "k8s.io/kubernetes/test/e2e/framework" ) @@ -23,10 +28,29 @@ import ( const ( consoleNamespace = "openshift-console" additionalRouteLabel = "console.openshift.io/additional-route" - pollTimeout = 2 * time.Minute pollInterval = 2 * time.Second + pollTimeout = 2 * time.Minute + hcpPollInterval = 10 * time.Second + // On HCP the reconciliation chain is HostedCluster CR → HostedControlPlane → + // HCCO → guest Ingress → console-operator → Route, which takes longer. + hcpPollTimeout = 25 * time.Minute + + hostedClusterConfigsNamespace = "clusters" ) +var hostedClusterGVR = schema.GroupVersionResource{ + Group: "hypershift.openshift.io", + Version: "v1beta1", + Resource: "hostedclusters", +} + +// hcpContext holds the management cluster state needed to modify +// ingress config on HyperShift clusters through the HostedCluster CR. +type hcpContext struct { + dynamicClient dynamic.Interface + hostedClusterName string +} + var _ = g.Describe("[sig-console][apigroup:config.openshift.io][OCPFeatureGate:IngressComponentRouteLabels][Serial] Console operator route label propagation", func() { defer g.GinkgoRecover() @@ -34,6 +58,9 @@ var _ = g.Describe("[sig-console][apigroup:config.openshift.io][OCPFeatureGate:I configClient configclient.Interface routeV1 routeclient.Interface domain string + hcp *hcpContext + interval time.Duration + timeout time.Duration ) g.BeforeEach(func() { @@ -49,10 +76,21 @@ var _ = g.Describe("[sig-console][apigroup:config.openshift.io][OCPFeatureGate:I ingress, err := configClient.ConfigV1().Ingresses().Get(context.TODO(), "cluster", metav1.GetOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) domain = ingress.Spec.Domain + + isHCP, err := exutil.IsHypershift(context.TODO(), configClient) + o.Expect(err).NotTo(o.HaveOccurred()) + interval = pollInterval + timeout = pollTimeout + if isHCP { + hcp, err = setupHCPContext() + o.Expect(err).NotTo(o.HaveOccurred(), "failed to set up HCP management cluster context") + interval = hcpPollInterval + timeout = hcpPollTimeout + } }) g.AfterEach(func() { - cleanupTestRoutes(configClient, routeV1) + cleanupTestRoutes(configClient, routeV1, hcp) }) g.It("should propagate labels from componentRoute spec to the route object", func() { @@ -61,10 +99,10 @@ var _ = g.Describe("[sig-console][apigroup:config.openshift.io][OCPFeatureGate:I labels := map[string]configv1.LabelValue{"ingress": "shard-test", "env": "ci"} g.By("adding a componentRoute with labels") - addComponentRouteWithLabels(configClient, name, hostname, labels) + addComponentRouteWithLabels(configClient, hcp, name, hostname, labels) g.By("waiting for the route to be created with the expected labels") - route := waitForRouteWithLabels(routeV1, name, hostname, labels) + route := waitForRouteWithLabels(routeV1, name, hostname, labels, interval, timeout) g.By("verifying operator-managed labels are preserved") o.Expect(route.Labels[additionalRouteLabel]).To(o.Equal("true")) @@ -77,17 +115,17 @@ var _ = g.Describe("[sig-console][apigroup:config.openshift.io][OCPFeatureGate:I labels := map[string]configv1.LabelValue{"ingress": "shard-test", "env": "ci"} g.By("adding a componentRoute with initial labels") - addComponentRouteWithLabels(configClient, name, hostname, labels) - waitForRouteWithLabels(routeV1, name, hostname, labels) + addComponentRouteWithLabels(configClient, hcp, name, hostname, labels) + waitForRouteWithLabels(routeV1, name, hostname, labels, interval, timeout) g.By("updating the componentRoute labels") updatedLabels := map[string]configv1.LabelValue{"ingress": "shard-updated", "tier": "frontend"} - updateComponentRouteLabels(configClient, name, updatedLabels) + updateComponentRouteLabels(configClient, hcp, name, updatedLabels) g.By("waiting for the route labels to be reconciled") - ctx, cancel := context.WithTimeout(context.TODO(), pollTimeout) + ctx, cancel := context.WithTimeout(context.TODO(), timeout) defer cancel() - err := wait.PollUntilContextTimeout(ctx, pollInterval, pollTimeout, true, func(ctx context.Context) (bool, error) { + err := wait.PollUntilContextTimeout(ctx, interval, timeout, true, func(ctx context.Context) (bool, error) { route, err := routeV1.RouteV1().Routes(consoleNamespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { return false, nil @@ -103,16 +141,16 @@ var _ = g.Describe("[sig-console][apigroup:config.openshift.io][OCPFeatureGate:I labels := map[string]configv1.LabelValue{"ingress": "shard-test", "env": "ci"} g.By("adding a componentRoute with initial labels") - addComponentRouteWithLabels(configClient, name, hostname, labels) - waitForRouteWithLabels(routeV1, name, hostname, labels) + addComponentRouteWithLabels(configClient, hcp, name, hostname, labels) + waitForRouteWithLabels(routeV1, name, hostname, labels, interval, timeout) g.By("removing the 'env' label from the componentRoute") - updateComponentRouteLabels(configClient, name, map[string]configv1.LabelValue{"ingress": "shard-test"}) + updateComponentRouteLabels(configClient, hcp, name, map[string]configv1.LabelValue{"ingress": "shard-test"}) g.By("waiting for the stale label to be removed from the route") - ctx, cancel := context.WithTimeout(context.TODO(), pollTimeout) + ctx, cancel := context.WithTimeout(context.TODO(), timeout) defer cancel() - err := wait.PollUntilContextTimeout(ctx, pollInterval, pollTimeout, true, func(ctx context.Context) (bool, error) { + err := wait.PollUntilContextTimeout(ctx, interval, timeout, true, func(ctx context.Context) (bool, error) { route, err := routeV1.RouteV1().Routes(consoleNamespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { return false, nil @@ -129,10 +167,10 @@ var _ = g.Describe("[sig-console][apigroup:config.openshift.io][OCPFeatureGate:I labels := map[string]configv1.LabelValue{"custom-key": "custom-value"} g.By("adding a componentRoute with a custom label") - addComponentRouteWithLabels(configClient, name, hostname, labels) + addComponentRouteWithLabels(configClient, hcp, name, hostname, labels) g.By("waiting for the route with expected labels") - route := waitForRouteWithLabels(routeV1, name, hostname, labels) + route := waitForRouteWithLabels(routeV1, name, hostname, labels, interval, timeout) g.By("verifying operator-managed labels are not overridden") o.Expect(route.Labels[additionalRouteLabel]).To(o.Equal("true")) @@ -145,16 +183,16 @@ var _ = g.Describe("[sig-console][apigroup:config.openshift.io][OCPFeatureGate:I labels := map[string]configv1.LabelValue{"ingress": "shard-test"} g.By("adding a componentRoute with labels") - addComponentRouteWithLabels(configClient, name, hostname, labels) - waitForRouteWithLabels(routeV1, name, hostname, labels) + addComponentRouteWithLabels(configClient, hcp, name, hostname, labels) + waitForRouteWithLabels(routeV1, name, hostname, labels, interval, timeout) g.By("removing the componentRoute") - removeComponentRoute(configClient, name) + removeComponentRoute(configClient, hcp, name) g.By("waiting for the route to be garbage collected") - ctx, cancel := context.WithTimeout(context.TODO(), pollTimeout) + ctx, cancel := context.WithTimeout(context.TODO(), timeout) defer cancel() - err := wait.PollUntilContextTimeout(ctx, pollInterval, pollTimeout, true, func(ctx context.Context) (bool, error) { + err := wait.PollUntilContextTimeout(ctx, interval, timeout, true, func(ctx context.Context) (bool, error) { _, err := routeV1.RouteV1().Routes(consoleNamespace).Get(ctx, name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { return true, nil @@ -168,7 +206,51 @@ var _ = g.Describe("[sig-console][apigroup:config.openshift.io][OCPFeatureGate:I }) }) -func addComponentRouteWithLabels(client configclient.Interface, name, hostname string, labels map[string]configv1.LabelValue) { +// setupHCPContext initializes the management cluster client for modifying +// ingress config through the HostedCluster CR on HyperShift clusters. +func setupHCPContext() (*hcpContext, error) { + if os.Getenv("HYPERSHIFT_MANAGEMENT_CLUSTER_KUBECONFIG") == "" || os.Getenv("HYPERSHIFT_MANAGEMENT_CLUSTER_NAMESPACE") == "" { + return nil, fmt.Errorf("HYPERSHIFT_MANAGEMENT_CLUSTER_KUBECONFIG and HYPERSHIFT_MANAGEMENT_CLUSTER_NAMESPACE must be set") + } + + mgmtOC := exutil.NewHypershiftManagementCLI("console-label-test") + _, hcpNamespace, err := exutil.GetHypershiftManagementClusterConfigAndNamespace() + if err != nil { + return nil, fmt.Errorf("failed to get HyperShift management cluster config: %w", err) + } + + hostedClusterName := strings.TrimPrefix(hcpNamespace, hostedClusterConfigsNamespace+"-") + e2e.Logf("HyperShift: HC=%s/%s, HCP NS=%s", hostedClusterConfigsNamespace, hostedClusterName, hcpNamespace) + + return &hcpContext{ + dynamicClient: mgmtOC.AdminDynamicClient(), + hostedClusterName: hostedClusterName, + }, nil +} + +// addComponentRouteWithLabels appends a componentRoute entry with the given +// labels to the ingress config. On HCP it modifies the HostedCluster CR instead. +func addComponentRouteWithLabels(client configclient.Interface, hcp *hcpContext, name, hostname string, labels map[string]configv1.LabelValue) { + if hcp != nil { + err := hcpModifyComponentRoutes(hcp, func(routes []interface{}) []interface{} { + entry := map[string]interface{}{ + "namespace": consoleNamespace, + "name": name, + "hostname": hostname, + } + if len(labels) > 0 { + labelsMap := make(map[string]interface{}, len(labels)) + for k, v := range labels { + labelsMap[k] = string(v) + } + entry["labels"] = labelsMap + } + return append(routes, entry) + }) + o.Expect(err).NotTo(o.HaveOccurred(), "failed to add componentRoute %s via HostedCluster", name) + return + } + err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { ingress, err := client.ConfigV1().Ingresses().Get(context.TODO(), "cluster", metav1.GetOptions{}) if err != nil { @@ -186,7 +268,32 @@ func addComponentRouteWithLabels(client configclient.Interface, name, hostname s o.Expect(err).NotTo(o.HaveOccurred(), "failed to add componentRoute %s", name) } -func updateComponentRouteLabels(client configclient.Interface, name string, labels map[string]configv1.LabelValue) { +// updateComponentRouteLabels replaces the labels on an existing componentRoute. +// On HCP it modifies the HostedCluster CR instead. +func updateComponentRouteLabels(client configclient.Interface, hcp *hcpContext, name string, labels map[string]configv1.LabelValue) { + if hcp != nil { + err := hcpModifyComponentRoutes(hcp, func(routes []interface{}) []interface{} { + for i, r := range routes { + routeMap, ok := r.(map[string]interface{}) + if !ok { + continue + } + if routeMap["name"] == name { + labelsMap := make(map[string]interface{}, len(labels)) + for k, v := range labels { + labelsMap[k] = string(v) + } + routeMap["labels"] = labelsMap + routes[i] = routeMap + break + } + } + return routes + }) + o.Expect(err).NotTo(o.HaveOccurred(), "failed to update labels on componentRoute %s via HostedCluster", name) + return + } + err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { ingress, err := client.ConfigV1().Ingresses().Get(context.TODO(), "cluster", metav1.GetOptions{}) if err != nil { @@ -204,7 +311,28 @@ func updateComponentRouteLabels(client configclient.Interface, name string, labe o.Expect(err).NotTo(o.HaveOccurred(), "failed to update labels on componentRoute %s", name) } -func removeComponentRoute(client configclient.Interface, name string) { +// removeComponentRoute removes a componentRoute entry by name. +// On HCP it modifies the HostedCluster CR instead. +func removeComponentRoute(client configclient.Interface, hcp *hcpContext, name string) { + if hcp != nil { + err := hcpModifyComponentRoutes(hcp, func(routes []interface{}) []interface{} { + var filtered []interface{} + for _, r := range routes { + routeMap, ok := r.(map[string]interface{}) + if !ok { + filtered = append(filtered, r) + continue + } + if routeMap["name"] != name { + filtered = append(filtered, r) + } + } + return filtered + }) + o.Expect(err).NotTo(o.HaveOccurred(), "failed to remove componentRoute %s via HostedCluster", name) + return + } + err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { ingress, err := client.ConfigV1().Ingresses().Get(context.TODO(), "cluster", metav1.GetOptions{}) if err != nil { @@ -226,27 +354,48 @@ func removeComponentRoute(client configclient.Interface, name string) { // cleanupTestRoutes removes test componentRoutes from the Ingress config and // directly deletes any orphaned route objects as a fallback in case the // operator hasn't garbage-collected them yet. -func cleanupTestRoutes(configClient configclient.Interface, routeClient routeclient.Interface) { - err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { - ingress, err := configClient.ConfigV1().Ingresses().Get(context.TODO(), "cluster", metav1.GetOptions{}) +func cleanupTestRoutes(configClient configclient.Interface, routeClient routeclient.Interface, hcp *hcpContext) { + if hcp != nil { + err := hcpModifyComponentRoutes(hcp, func(routes []interface{}) []interface{} { + var filtered []interface{} + for _, r := range routes { + routeMap, ok := r.(map[string]interface{}) + if !ok { + filtered = append(filtered, r) + continue + } + n, _ := routeMap["name"].(string) + if !strings.HasPrefix(n, "console-label-test-") { + filtered = append(filtered, r) + } + } + return filtered + }) if err != nil { - return err + e2e.Logf("warning: failed to clean up test componentRoutes via HostedCluster: %v", err) } - var filtered []configv1.ComponentRouteSpec - for _, cr := range ingress.Spec.ComponentRoutes { - if !strings.HasPrefix(cr.Name, "console-label-test-") { - filtered = append(filtered, cr) + } else { + err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { + ingress, err := configClient.ConfigV1().Ingresses().Get(context.TODO(), "cluster", metav1.GetOptions{}) + if err != nil { + return err } + var filtered []configv1.ComponentRouteSpec + for _, cr := range ingress.Spec.ComponentRoutes { + if !strings.HasPrefix(cr.Name, "console-label-test-") { + filtered = append(filtered, cr) + } + } + if len(filtered) == len(ingress.Spec.ComponentRoutes) { + return nil + } + ingress.Spec.ComponentRoutes = filtered + _, err = configClient.ConfigV1().Ingresses().Update(context.TODO(), ingress, metav1.UpdateOptions{}) + return err + }) + if err != nil { + e2e.Logf("warning: failed to clean up test componentRoutes: %v", err) } - if len(filtered) == len(ingress.Spec.ComponentRoutes) { - return nil - } - ingress.Spec.ComponentRoutes = filtered - _, err = configClient.ConfigV1().Ingresses().Update(context.TODO(), ingress, metav1.UpdateOptions{}) - return err - }) - if err != nil { - e2e.Logf("warning: failed to clean up test componentRoutes: %v", err) } routes, err := routeClient.RouteV1().Routes(consoleNamespace).List(context.TODO(), metav1.ListOptions{}) @@ -265,11 +414,11 @@ func cleanupTestRoutes(configClient configclient.Interface, routeClient routecli // waitForRouteWithLabels polls until a route exists with the expected hostname, // the operator-managed additional-route label, and all expected user labels. -func waitForRouteWithLabels(client routeclient.Interface, name, hostname string, expectedLabels map[string]configv1.LabelValue) *routev1.Route { +func waitForRouteWithLabels(client routeclient.Interface, name, hostname string, expectedLabels map[string]configv1.LabelValue, interval, timeout time.Duration) *routev1.Route { var route *routev1.Route - ctx, cancel := context.WithTimeout(context.TODO(), pollTimeout) + ctx, cancel := context.WithTimeout(context.TODO(), timeout) defer cancel() - err := wait.PollUntilContextTimeout(ctx, pollInterval, pollTimeout, true, func(ctx context.Context) (bool, error) { + err := wait.PollUntilContextTimeout(ctx, interval, timeout, true, func(ctx context.Context) (bool, error) { var err error route, err = client.RouteV1().Routes(consoleNamespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { @@ -288,6 +437,33 @@ func waitForRouteWithLabels(client routeclient.Interface, name, hostname string, } return true, nil }) - o.Expect(err).NotTo(o.HaveOccurred(), "route %s not created with expected labels within %s", name, pollTimeout) + o.Expect(err).NotTo(o.HaveOccurred(), "route %s not created with expected labels within %s", name, timeout) return route } + +// hcpModifyComponentRoutes performs a read-modify-write on the HostedCluster CR's +// spec.configuration.ingress.componentRoutes using the management cluster client. +func hcpModifyComponentRoutes(hcp *hcpContext, modify func([]interface{}) []interface{}) error { + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + hc, err := hcp.dynamicClient.Resource(hostedClusterGVR).Namespace(hostedClusterConfigsNamespace).Get(context.TODO(), hcp.hostedClusterName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to get HostedCluster %s/%s: %w", hostedClusterConfigsNamespace, hcp.hostedClusterName, err) + } + + routes, _, err := unstructured.NestedSlice(hc.Object, "spec", "configuration", "ingress", "componentRoutes") + if err != nil { + return fmt.Errorf("failed to extract componentRoutes from HostedCluster: %w", err) + } + routes = modify(routes) + + if err := unstructured.SetNestedSlice(hc.Object, routes, "spec", "configuration", "ingress", "componentRoutes"); err != nil { + return fmt.Errorf("failed to set componentRoutes on HostedCluster: %w", err) + } + + _, err = hcp.dynamicClient.Resource(hostedClusterGVR).Namespace(hostedClusterConfigsNamespace).Update(context.TODO(), hc, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("failed to update HostedCluster %s/%s: %w", hostedClusterConfigsNamespace, hcp.hostedClusterName, err) + } + return nil + }) +}