diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 688aedeb9d9..fd518bd8b56 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -1044,6 +1044,73 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp * } } + createOrUpdate := r.createOrUpdate(hcp) + // Reconcile default service account + r.Log.Info("Reconciling default service account") + if err := r.reconcileDefaultServiceAccount(ctx, hcp, createOrUpdate); err != nil { + return fmt.Errorf("failed to reconcile default service account: %w", err) + } + + // Reconcile PKI + if _, exists := hcp.Annotations[hyperv1.DisablePKIReconciliationAnnotation]; !exists { + r.Log.Info("Reconciling PKI") + if err := r.reconcilePKI(ctx, hcp, infraStatus, createOrUpdate); err != nil { + return fmt.Errorf("failed to reconcile PKI: %w", err) + } + } + + // Reconcile unmanaged etcd + if hcp.Spec.Etcd.ManagementType == hyperv1.Unmanaged { + r.Log.Info("Reconciling unmanaged Etcd") + if err := r.reconcileUnmanagedEtcd(ctx, hcp, createOrUpdate); err != nil { + return fmt.Errorf("failed to reconcile etcd: %w", err) + } + } + + if err := r.reconcileSREMetricsConfig(ctx, createOrUpdate, hcp.Namespace); err != nil { + return fmt.Errorf("failed to reconcile metrics config: %w", err) + } + + if routerv2.UseHCPRouter(hcp) { + if err := r.admitHCPManagedRoutes(ctx, hcp, infraStatus.InternalHCPRouterHost, infraStatus.ExternalHCPRouterHost); err != nil { + return fmt.Errorf("failed to admit HCP managed routes: %w", err) + } + if err := r.cleanupOldRouterResources(ctx, hcp); err != nil { + return fmt.Errorf("failed to cleanup old router resources: %w", err) + } + } + + if _, exists := hcp.Annotations[hyperv1.DisableIgnitionServerAnnotation]; !exists { + // Reconcile Ignition-server configs + r.Log.Info("Reconciling ignition-server configs") + if err := r.reconcileIgnitionServerConfigs(ctx, hcp, createOrUpdate); err != nil { + return fmt.Errorf("failed to reconcile ignition-server configs: %w", err) + } + } + + if util.HCPOAuthEnabled(hcp) { + // Reconcile kubeadmin password + r.Log.Info("Reconciling kubeadmin password secret") + explicitOauthConfig := hcp.Spec.Configuration != nil && hcp.Spec.Configuration.OAuth != nil + if err := r.reconcileKubeadminPassword(ctx, hcp, explicitOauthConfig, createOrUpdate); err != nil { + return fmt.Errorf("failed to ensure control plane: %w", err) + } + + // TODO: move this up with the rest of conditions reconciliation logic? + if err := r.reconcileValidIDPConfigurationCondition(ctx, hcp, releaseImageProvider, infraStatus.OAuthHost, infraStatus.OAuthPort); err != nil { + return fmt.Errorf("failed to reconcile ValidIDPConfiguration condition: %w", err) + } + } + + if err := r.cleanupClusterNetworkOperatorResources(ctx, hcp, r.ManagementClusterCapabilities.Has(capabilities.CapabilityRoute)); err != nil { + return fmt.Errorf("failed to reconcile cluster network operator operands: %w", err) + } + + r.Log.Info("Reconciling default security group") + if err := r.reconcileDefaultSecurityGroup(ctx, hcp); err != nil { + return fmt.Errorf("failed to reconcile default security group: %w", err) + } + cpContext := component.ControlPlaneContext{ Context: ctx, Client: r.Client, @@ -1069,19 +1136,6 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp * return utilerrors.NewAggregate(errs) } -// useHCPRouter returns true if a dedicated common router is created for a HCP to handle ingress for the managed endpoints. -// This is true when the API input specifies intent for the following: -// 1 - AWS endpointAccess is private somehow (i.e. publicAndPrivate or private) or is public and configured with external DNS. -// 2 - When 1 is true, we recommend (and automate via CLI) ServicePublishingStrategy to be "Route" for all endpoints but the KAS -// which needs a dedicated Service type LB external to be exposed if no external DNS is supported. -// Otherwise, the Routes use the management cluster Domain and resolve through the default ingress controller. -func useHCPRouter(hostedControlPlane *hyperv1.HostedControlPlane) bool { - if sharedingress.UseSharedIngress() { - return false - } - return util.IsPrivateHCP(hostedControlPlane) || util.IsPublicWithDNS(hostedControlPlane) -} - func IsStorageAndCSIManaged(hostedControlPlane *hyperv1.HostedControlPlane) bool { if hostedControlPlane.Spec.Platform.Type == hyperv1.IBMCloudPlatform || hostedControlPlane.Spec.Platform.Type == hyperv1.PowerVSPlatform { return false @@ -1229,7 +1283,7 @@ func (r *HostedControlPlaneReconciler) reconcile(ctx context.Context, hostedCont return nil } - if useHCPRouter(hostedControlPlane) { + if routerv2.UseHCPRouter(hostedControlPlane) { r.Log.Info("Reconciling router") if err := r.reconcileRouter(ctx, hostedControlPlane, releaseImageProvider, createOrUpdate); err != nil { return fmt.Errorf("failed to reconcile router: %w", err) @@ -1237,7 +1291,7 @@ func (r *HostedControlPlaneReconciler) reconcile(ctx context.Context, hostedCont } } - if useHCPRouter(hostedControlPlane) { + if routerv2.UseHCPRouter(hostedControlPlane) { if err := r.admitHCPManagedRoutes(ctx, hostedControlPlane, infraStatus.InternalHCPRouterHost, infraStatus.ExternalHCPRouterHost); err != nil { return fmt.Errorf("failed to admit HCP managed routes: %w", err) } @@ -1266,7 +1320,7 @@ func (r *HostedControlPlaneReconciler) reconcile(ctx context.Context, hostedCont config.OwnerRefFrom(hostedControlPlane), openShiftTrustedCABundleConfigMapForCPOExists, r.ReleaseProvider.GetMirroredReleaseImage(), - util.UseDedicatedDNSForIgnition(hostedControlPlane), + util.LabelHCPRoutes(hostedControlPlane), ); err != nil { return fmt.Errorf("failed to reconcile ignition server: %w", err) } @@ -1833,7 +1887,7 @@ func (r *HostedControlPlaneReconciler) reconcileKonnectivityServerService(ctx co if serviceStrategy.Route != nil { hostname = serviceStrategy.Route.Hostname } - return kas.ReconcileKonnectivityExternalRoute(konnectivityRoute, p.OwnerRef, hostname, r.DefaultIngressDomain, util.UseDedicatedDNSForKonnectivity(hcp)) + return kas.ReconcileKonnectivityExternalRoute(konnectivityRoute, p.OwnerRef, hostname, r.DefaultIngressDomain, util.LabelHCPRoutes(hcp)) }); err != nil { return fmt.Errorf("failed to reconcile Konnectivity server external route: %w", err) } @@ -1871,7 +1925,7 @@ func (r *HostedControlPlaneReconciler) reconcileOAuthServerService(ctx context.C if serviceStrategy.Route != nil { hostname = serviceStrategy.Route.Hostname } - return oauth.ReconcileExternalPublicRoute(oauthExternalPublicRoute, p.OwnerRef, hostname, r.DefaultIngressDomain, util.UseDedicatedDNSForOAuth(hcp)) + return oauth.ReconcileExternalPublicRoute(oauthExternalPublicRoute, p.OwnerRef, hostname, r.DefaultIngressDomain, util.LabelHCPRoutes(hcp)) }); err != nil { return fmt.Errorf("failed to reconcile OAuth external public route: %w", err) } @@ -1885,7 +1939,7 @@ func (r *HostedControlPlaneReconciler) reconcileOAuthServerService(ctx context.C // Reconcile the external private route if a hostname is specified if serviceStrategy.Route != nil && serviceStrategy.Route.Hostname != "" { if _, err := createOrUpdate(ctx, r.Client, oauthExternalPrivateRoute, func() error { - return oauth.ReconcileExternalPrivateRoute(oauthExternalPrivateRoute, p.OwnerRef, serviceStrategy.Route.Hostname, r.DefaultIngressDomain, util.UseDedicatedDNSForOAuth(hcp)) + return oauth.ReconcileExternalPrivateRoute(oauthExternalPrivateRoute, p.OwnerRef, serviceStrategy.Route.Hostname, r.DefaultIngressDomain, util.LabelHCPRoutes(hcp)) }); err != nil { return fmt.Errorf("failed to reconcile OAuth external private route: %w", err) } @@ -1948,10 +2002,10 @@ func (r *HostedControlPlaneReconciler) reconcileHCPRouterServices(ctx context.Co } // Create the Service type LB internal for private endpoints. pubSvc := manifests.RouterPublicService(hcp.Namespace) + privSvc := manifests.PrivateRouterService(hcp.Namespace) if util.IsPrivateHCP(hcp) { - svc := manifests.PrivateRouterService(hcp.Namespace) - if _, err := createOrUpdate(ctx, r.Client, svc, func() error { - return ingress.ReconcileRouterService(svc, true, true, hcp) + if _, err := createOrUpdate(ctx, r.Client, privSvc, func() error { + return ingress.ReconcileRouterService(privSvc, true, true, hcp) }); err != nil { return fmt.Errorf("failed to reconcile private router service: %w", err) } @@ -1968,15 +2022,27 @@ func (r *HostedControlPlaneReconciler) reconcileHCPRouterServices(ctx context.Co } } } + } else { + // Clean up stale private router Service when upgrading from buggy behavior + if _, err := util.DeleteIfNeeded(ctx, r.Client, privSvc); err != nil { + return fmt.Errorf("failed to delete private router service: %w", err) + } } - // When Public access endpoint we need to create a Service type LB external. - if util.IsPublicWithDNS(hcp) { + // When Public access endpoint AND routes use HCP router, create a Service type LB external. + // This ensures we only create public router infrastructure when routes are labeled for it. + if util.IsPublicHCP(hcp) && util.LabelHCPRoutes(hcp) { if _, err := createOrUpdate(ctx, r.Client, pubSvc, func() error { return ingress.ReconcileRouterService(pubSvc, false, util.IsPrivateHCP(hcp), hcp) }); err != nil { return fmt.Errorf("failed to reconcile router service: %w", err) } + } else if util.IsPublicHCP(hcp) { + // Clean up stale public router Service when LabelHCPRoutes becomes false + // (e.g., when switching from KAS Route to KAS LoadBalancer or upgrading from buggy behavior) + if _, err := util.DeleteIfNeeded(ctx, r.Client, pubSvc); err != nil { + return fmt.Errorf("failed to delete public router service: %w", err) + } } return nil } @@ -2082,7 +2148,7 @@ func (r *HostedControlPlaneReconciler) reconcileInternalRouterServiceStatus(ctx } func (r *HostedControlPlaneReconciler) reconcileExternalRouterServiceStatus(ctx context.Context, hcp *hyperv1.HostedControlPlane) (host string, needed bool, message string, err error) { - if !util.IsPublicWithDNS(hcp) || sharedingress.UseSharedIngress() || hcp.Spec.Platform.Type == hyperv1.IBMCloudPlatform { + if !util.IsPublicHCP(hcp) || !util.LabelHCPRoutes(hcp) || sharedingress.UseSharedIngress() || hcp.Spec.Platform.Type == hyperv1.IBMCloudPlatform { return } return r.reconcileRouterServiceStatus(ctx, manifests.RouterPublicService(hcp.Namespace), events.NewMessageCollector(ctx, r.Client)) diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go index 3642d3fb216..7bee46648ad 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go @@ -24,6 +24,7 @@ import ( ignitionproxyv2 "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/ignitionserver_proxy" kasv2 "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/kas" oapiv2 "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/oapi" + routerv2 "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/router" "github.com/openshift/hypershift/support/api" autoscalercommon "github.com/openshift/hypershift/support/autoscaler" fakecapabilities "github.com/openshift/hypershift/support/capabilities/fake" @@ -282,11 +283,8 @@ func TestReconcileOAuthService(t *testing.T) { oauthExternalPublicRoute := func(m ...func(*routev1.Route)) routev1.Route { route := routev1.Route{ ObjectMeta: metav1.ObjectMeta{ - Namespace: targetNamespace, - Name: "oauth", - Labels: map[string]string{ - "hypershift.openshift.io/hosted-control-plane": targetNamespace, - }, + Namespace: targetNamespace, + Name: "oauth", OwnerReferences: []metav1.OwnerReference{ownerRef}, }, Spec: routev1.RouteSpec{ @@ -1577,7 +1575,7 @@ func TestReconcileRouter(t *testing.T) { } releaseInfo := &releaseinfo.ReleaseImage{ImageStream: &imagev1.ImageStream{}} - if useHCPRouter(hcp) { + if routerv2.UseHCPRouter(hcp) { if err := r.reconcileRouter(ctx, hcp, imageprovider.New(releaseInfo), controllerutil.CreateOrUpdate); err != nil { t.Fatalf("reconcileRouter failed: %v", err) } diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_ignition_server_route.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_ignition_server_route.yaml new file mode 100644 index 00000000000..0de7a876608 --- /dev/null +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_ignition_server_route.yaml @@ -0,0 +1,26 @@ +apiVersion: route.openshift.io/v1 +kind: Route +metadata: + creationTimestamp: null + labels: + hypershift.openshift.io/hosted-control-plane: hcp-namespace + name: ignition-server + namespace: hcp-namespace + ownerReferences: + - apiVersion: hypershift.openshift.io/v1beta1 + blockOwnerDeletion: true + controller: true + kind: HostedControlPlane + name: hcp + uid: "" + resourceVersion: "1" +spec: + tls: + insecureEdgeTerminationPolicy: None + termination: passthrough + to: + kind: Service + name: ignition-server-proxy + weight: null + wildcardPolicy: None +status: {} diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_component.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_component.yaml index 86168a59189..230ce7c9b80 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_component.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_component.yaml @@ -31,6 +31,9 @@ status: - group: rbac.authorization.k8s.io kind: RoleBinding name: ignition-server + - group: route.openshift.io + kind: Route + name: ignition-server - group: "" kind: Service name: ignition-server diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_component_TechPreviewNoUpgrade.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_component_TechPreviewNoUpgrade.yaml index 86168a59189..230ce7c9b80 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_component_TechPreviewNoUpgrade.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_component_TechPreviewNoUpgrade.yaml @@ -31,6 +31,9 @@ status: - group: rbac.authorization.k8s.io kind: RoleBinding name: ignition-server + - group: route.openshift.io + kind: Route + name: ignition-server - group: "" kind: Service name: ignition-server diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_ignition_server_route.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_ignition_server_route.yaml new file mode 100644 index 00000000000..0de7a876608 --- /dev/null +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/ignition-server/zz_fixture_TestControlPlaneComponents_ignition_server_route.yaml @@ -0,0 +1,26 @@ +apiVersion: route.openshift.io/v1 +kind: Route +metadata: + creationTimestamp: null + labels: + hypershift.openshift.io/hosted-control-plane: hcp-namespace + name: ignition-server + namespace: hcp-namespace + ownerReferences: + - apiVersion: hypershift.openshift.io/v1beta1 + blockOwnerDeletion: true + controller: true + kind: HostedControlPlane + name: hcp + uid: "" + resourceVersion: "1" +spec: + tls: + insecureEdgeTerminationPolicy: None + termination: passthrough + to: + kind: Service + name: ignition-server-proxy + weight: null + wildcardPolicy: None +status: {} diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/ignitionserver/route.go b/control-plane-operator/controllers/hostedcontrolplane/v2/ignitionserver/route.go index e40c73035f3..80de28e71cb 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/ignitionserver/route.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/ignitionserver/route.go @@ -11,10 +11,6 @@ import ( ) func routePredicate(cpContext component.WorkloadContext) bool { - if cpContext.HCP.Spec.Platform.Type != hyperv1.IBMCloudPlatform { - return false - } - strategy := util.ServicePublishingStrategyByTypeForHCP(cpContext.HCP, hyperv1.Ignition) if strategy == nil { return false @@ -23,9 +19,16 @@ func routePredicate(cpContext component.WorkloadContext) bool { } func (ign *ignitionServer) adaptRoute(cpContext component.WorkloadContext, route *routev1.Route) error { + serviceName := "ignition-server-proxy" + // For IBM Cloud, we don't deploy the ignition server proxy. + // Instead, the ignition server service is directly exposed. + if cpContext.HCP.Spec.Platform.Type == hyperv1.IBMCloudPlatform { + serviceName = "ignition-server" + } + hcp := cpContext.HCP if util.IsPrivateHCP(hcp) { - return util.ReconcileInternalRoute(route, hcp.Name, ComponentName) + return util.ReconcileInternalRoute(route, hcp.Name, serviceName) } strategy := util.ServicePublishingStrategyByTypeForHCP(hcp, hyperv1.Ignition) @@ -37,5 +40,5 @@ func (ign *ignitionServer) adaptRoute(cpContext component.WorkloadContext, route if strategy.Route != nil { hostname = strategy.Route.Hostname } - return util.ReconcileExternalRoute(route, hostname, ign.defaultIngressDomain, ComponentName, util.UseDedicatedDNSForIgnition(hcp)) + return util.ReconcileExternalRoute(route, hostname, ign.defaultIngressDomain, serviceName, util.LabelHCPRoutes(hcp)) } diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/ignitionserver_proxy/route.go b/control-plane-operator/controllers/hostedcontrolplane/v2/ignitionserver_proxy/route.go index 28a403548a2..aee36ac7247 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/ignitionserver_proxy/route.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/ignitionserver_proxy/route.go @@ -33,5 +33,5 @@ func (ign *ignitionServerProxy) adaptRoute(cpContext component.WorkloadContext, if strategy.Route != nil { hostname = strategy.Route.Hostname } - return util.ReconcileExternalRoute(route, hostname, ign.defaultIngressDomain, ComponentName, util.UseDedicatedDNSForIgnition(hcp)) + return util.ReconcileExternalRoute(route, hostname, ign.defaultIngressDomain, ComponentName, util.LabelHCPRoutes(hcp)) } diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/router/component.go b/control-plane-operator/controllers/hostedcontrolplane/v2/router/component.go index d93dad1e4c1..acea4173c59 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/router/component.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/router/component.go @@ -1,6 +1,7 @@ package router import ( + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" oapiv2 "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/oapi" "github.com/openshift/hypershift/hypershift-operator/controllers/sharedingress" component "github.com/openshift/hypershift/support/controlplane-component" @@ -46,15 +47,27 @@ func NewComponent() component.ControlPlaneComponent { Build() } -// useHCPRouter returns true if a dedicated common router is created for a HCP to handle ingress for the managed endpoints. -// This is true when the API input specifies intent for the following: -// 1 - AWS endpointAccess is private somehow (i.e. publicAndPrivate or private) or is public and configured with external DNS. -// 2 - When 1 is true, we recommend (and automate via CLI) ServicePublishingStrategy to be "Route" for all endpoints but the KAS -// which needs a dedicated Service type LB external to be exposed if no external DNS is supported. -// Otherwise, the Routes use the management cluster Domain and resolve through the default ingress controller. -func useHCPRouter(cpContext component.WorkloadContext) (bool, error) { +// UseHCPRouter returns true when the HCP routes should be served by a dedicated +// HCP router, as determined by util.LabelHCPRoutes. This occurs when: +// 1. The cluster has no public internet access (Private endpoint access), OR +// 2. The cluster has public internet access (Public or PublicAndPrivate endpoint access) +// but uses a dedicated Route for KAS DNS (rather than a LoadBalancer) +// +// Excludes shared ingress configurations and IBM Cloud platform. +func UseHCPRouter(hcp *hyperv1.HostedControlPlane) bool { if sharedingress.UseSharedIngress() { - return false, nil + return false + } + if hcp.Spec.Platform.Type == hyperv1.IBMCloudPlatform { + return false } - return util.IsPrivateHCP(cpContext.HCP) || util.IsPublicWithDNS(cpContext.HCP), nil + // Router infrastructure is needed when: + // 1. Cluster has private access (Private or PublicAndPrivate) - for internal routes, OR + // 2. External routes are labeled for HCP router (Public with KAS DNS) + return util.IsPrivateHCP(hcp) || util.LabelHCPRoutes(hcp) +} + +// useHCPRouter is an adapter for the component predicate interface. +func useHCPRouter(cpContext component.WorkloadContext) (bool, error) { + return UseHCPRouter(cpContext.HCP), nil } diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/router/component_test.go b/control-plane-operator/controllers/hostedcontrolplane/v2/router/component_test.go new file mode 100644 index 00000000000..b19c09d368e --- /dev/null +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/router/component_test.go @@ -0,0 +1,228 @@ +package router + +import ( + "testing" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/stretchr/testify/assert" +) + +func TestUseHCPRouter(t *testing.T) { + testsCases := []struct { + name string + hcp *hyperv1.HostedControlPlane + expectedResult bool + }{ + { + name: "Provider is IBM Cloud", + hcp: &hyperv1.HostedControlPlane{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tenant1", + Name: "cluster1", + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.IBMCloudPlatform, + }, + }, + }, + expectedResult: false, + }, + { + name: "Provider is NonePlatform, services are exposed with Routes", + hcp: &hyperv1.HostedControlPlane{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tenant1", + Name: "cluster1", + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.NonePlatform, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "cluster1.api.tenant1.com", + }, + }, + }, + { + Service: hyperv1.Konnectivity, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "cluster1.tunnel.tenant1.com", + }, + }, + }, + { + Service: hyperv1.Ignition, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "cluster1.ignition.tenant1.com", + }, + }, + }, + { + Service: hyperv1.OAuthServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "cluster1.oauth.tenant1.com", + }, + }, + }, + }, + }, + }, + expectedResult: true, + }, + { + name: "Provider is AWS, Public and Private, KAS Loadbalancer", + hcp: &hyperv1.HostedControlPlane{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tenant1", + Name: "cluster1", + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: hyperv1.PublicAndPrivate, + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + expectedResult: true, // Router infrastructure needed for internal routes + }, + { + name: "Provider is AWS, Public and Private, KAS Route", + hcp: &hyperv1.HostedControlPlane{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tenant1", + Name: "cluster1", + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: hyperv1.PublicAndPrivate, + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "cluster1.api.tenant1.com", + }, + }, + }, + }, + }, + }, + expectedResult: true, + }, + { + name: "Provider is AWS, Private", + hcp: &hyperv1.HostedControlPlane{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tenant1", + Name: "cluster1", + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: hyperv1.Private, + }, + }, + }, + }, + expectedResult: true, + }, + { + name: "Provider is Agent, KAS LoadBalancer", + hcp: &hyperv1.HostedControlPlane{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tenant1", + Name: "cluster1", + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AgentPlatform, + Agent: &hyperv1.AgentPlatformSpec{ + AgentNamespace: "agent-ns", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + expectedResult: false, // Router infrastructure not needed when KAS uses LoadBalancer + }, + { + name: "Provider is Agent, KAS Route", + hcp: &hyperv1.HostedControlPlane{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tenant1", + Name: "cluster1", + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AgentPlatform, + Agent: &hyperv1.AgentPlatformSpec{ + AgentNamespace: "agent-ns", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "cluster1.api.tenant1.com", + }, + }, + }, + }, + }, + }, + expectedResult: true, + }, + } + for _, tc := range testsCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expectedResult, UseHCPRouter(tc.hcp)) + }) + } +} diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index a658fe62831..d741b1152ed 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -4688,10 +4688,6 @@ func (r *HostedClusterReconciler) validateAWSConfig(hc *hyperv1.HostedCluster) e if !hyperutil.UseDedicatedDNSForKASByHC(hc) && kasPublishingStrategy.Type != hyperv1.LoadBalancer { errs = append(errs, fmt.Errorf("service type %v with publishing strategy %v is not supported, use Route or LoadBalancer", hyperv1.APIServer, kasPublishingStrategy.Type)) } - // When using dedicated DNS, the KAS should be exposed as Route. - if hyperutil.IsPublicWithDNSByHC(hc) && hyperutil.IsLBKASByHC(hc) { - errs = append(errs, fmt.Errorf("service type %v with publishing strategy %v is not supported when any service specifies external DNS, use Route", hyperv1.APIServer, kasPublishingStrategy.Type)) - } } return utilerrors.NewAggregate(errs) diff --git a/support/util/expose.go b/support/util/expose.go index 9ef85267439..485fae4c7c9 100644 --- a/support/util/expose.go +++ b/support/util/expose.go @@ -31,18 +31,6 @@ func UseDedicatedDNSForKAS(hcp *hyperv1.HostedControlPlane) bool { return UseDedicatedDNS(hcp, hyperv1.APIServer) } -func UseDedicatedDNSForOAuth(hcp *hyperv1.HostedControlPlane) bool { - return UseDedicatedDNS(hcp, hyperv1.OAuthServer) -} - -func UseDedicatedDNSForKonnectivity(hcp *hyperv1.HostedControlPlane) bool { - return UseDedicatedDNS(hcp, hyperv1.Konnectivity) -} - -func UseDedicatedDNSForIgnition(hcp *hyperv1.HostedControlPlane) bool { - return UseDedicatedDNS(hcp, hyperv1.Ignition) -} - func UseDedicatedDNS(hcp *hyperv1.HostedControlPlane, svcType hyperv1.ServiceType) bool { svc := ServicePublishingStrategyByTypeForHCP(hcp, svcType) return IsRoute(hcp, svcType) && diff --git a/support/util/route.go b/support/util/route.go index d5665a07886..7182d3bcaab 100644 --- a/support/util/route.go +++ b/support/util/route.go @@ -90,6 +90,8 @@ func hash(s string) string { func ReconcileExternalRoute(route *routev1.Route, hostname string, defaultIngressDomain string, serviceName string, labelHCPRoutes bool) error { if labelHCPRoutes { AddHCPRouteLabel(route) + } else { + RemoveHCPRouteLabel(route) } if hostname != "" { route.Spec.Host = hostname @@ -138,6 +140,14 @@ func AddHCPRouteLabel(target crclient.Object) { target.SetLabels(labels) } +func RemoveHCPRouteLabel(target crclient.Object) { + labels := target.GetLabels() + if labels != nil { + delete(labels, HCPRouteLabel) + target.SetLabels(labels) + } +} + func AddInternalRouteLabel(target crclient.Object) { labels := target.GetLabels() if labels == nil { diff --git a/support/util/route_test.go b/support/util/route_test.go index fd09810d8a2..ff9d4f7c422 100644 --- a/support/util/route_test.go +++ b/support/util/route_test.go @@ -5,6 +5,9 @@ import ( "math/rand" "testing" + routev1 "github.com/openshift/api/route/v1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kvalidation "k8s.io/apimachinery/pkg/util/validation" ) @@ -102,3 +105,364 @@ func randSeq(n int) string { } return string(b) } + +func TestReconcileExternalRoute(t *testing.T) { + tests := []struct { + name string + route *routev1.Route + hostname string + defaultDomain string + serviceName string + labelHCPRoutes bool + expectedLabels map[string]string + expectedHost string + expectedService string + }{ + { + name: "When labelHCPRoutes is true and route has no labels, it should add HCP label", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + }, + }, + hostname: "test.example.com", + defaultDomain: "example.com", + serviceName: "test-service", + labelHCPRoutes: true, + expectedLabels: map[string]string{ + HCPRouteLabel: "test-namespace", + }, + expectedHost: "test.example.com", + expectedService: "test-service", + }, + { + name: "When labelHCPRoutes is true and route has existing labels, it should add HCP label", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + Labels: map[string]string{ + "existing-label": "existing-value", + }, + }, + }, + hostname: "test.example.com", + defaultDomain: "example.com", + serviceName: "test-service", + labelHCPRoutes: true, + expectedLabels: map[string]string{ + "existing-label": "existing-value", + HCPRouteLabel: "test-namespace", + }, + expectedHost: "test.example.com", + expectedService: "test-service", + }, + { + name: "When labelHCPRoutes is false and route has no labels, it should not add HCP label", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + }, + }, + hostname: "test.example.com", + defaultDomain: "example.com", + serviceName: "test-service", + labelHCPRoutes: false, + expectedLabels: nil, + expectedHost: "test.example.com", + expectedService: "test-service", + }, + { + name: "When labelHCPRoutes is false and route has HCP label, it should remove HCP label", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + Labels: map[string]string{ + HCPRouteLabel: "test-namespace", + "existing-label": "existing-value", + }, + }, + }, + hostname: "test.example.com", + defaultDomain: "example.com", + serviceName: "test-service", + labelHCPRoutes: false, + expectedLabels: map[string]string{ + "existing-label": "existing-value", + }, + expectedHost: "test.example.com", + expectedService: "test-service", + }, + { + name: "When labelHCPRoutes is false and route has only HCP label, it should remove HCP label and leave empty map", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + Labels: map[string]string{ + HCPRouteLabel: "test-namespace", + }, + }, + }, + hostname: "test.example.com", + defaultDomain: "example.com", + serviceName: "test-service", + labelHCPRoutes: false, + expectedLabels: map[string]string{}, + expectedHost: "test.example.com", + expectedService: "test-service", + }, + { + name: "When hostname is empty and route name is short, it should leave hostname empty for default generation", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + }, + }, + hostname: "", + defaultDomain: "example.com", + serviceName: "test-service", + labelHCPRoutes: true, + expectedLabels: map[string]string{ + HCPRouteLabel: "test-namespace", + }, + expectedHost: "", // ShortenRouteHostnameIfNeeded returns empty when name is short enough + expectedService: "test-service", + }, + { + name: "When hostname is empty and route already has a hostname, it should preserve existing hostname", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + }, + Spec: routev1.RouteSpec{ + Host: "existing.example.com", + }, + }, + hostname: "", + defaultDomain: "example.com", + serviceName: "test-service", + labelHCPRoutes: true, + expectedLabels: map[string]string{ + HCPRouteLabel: "test-namespace", + }, + expectedHost: "existing.example.com", + expectedService: "test-service", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ReconcileExternalRoute(tt.route, tt.hostname, tt.defaultDomain, tt.serviceName, tt.labelHCPRoutes) + if err != nil { + t.Fatalf("ReconcileExternalRoute() error = %v, wantErr false", err) + } + + // Check labels + if tt.expectedLabels == nil { + if len(tt.route.Labels) != 0 { + t.Errorf("Expected no labels, but got %v", tt.route.Labels) + } + } else { + if len(tt.route.Labels) != len(tt.expectedLabels) { + t.Errorf("Labels length mismatch: got %d, want %d. Got: %v, Want: %v", len(tt.route.Labels), len(tt.expectedLabels), tt.route.Labels, tt.expectedLabels) + } + for k, v := range tt.expectedLabels { + if got, ok := tt.route.Labels[k]; !ok || got != v { + t.Errorf("Label %s: got %v, want %v", k, got, v) + } + } + // Check that HCP label is not present when not expected + if _, ok := tt.expectedLabels[HCPRouteLabel]; !ok { + if _, hasLabel := tt.route.Labels[HCPRouteLabel]; hasLabel { + t.Errorf("Expected HCP label to be removed, but it still exists") + } + } + } + + // Check hostname + if tt.route.Spec.Host != tt.expectedHost { + t.Errorf("Host: got %v, want %v", tt.route.Spec.Host, tt.expectedHost) + } + + // Check service name + if tt.route.Spec.To.Name != tt.expectedService { + t.Errorf("Service name: got %v, want %v", tt.route.Spec.To.Name, tt.expectedService) + } + + // Check TLS config + if tt.route.Spec.TLS == nil { + t.Errorf("Expected TLS config to be set") + } else { + if tt.route.Spec.TLS.Termination != routev1.TLSTerminationPassthrough { + t.Errorf("TLS termination: got %v, want %v", tt.route.Spec.TLS.Termination, routev1.TLSTerminationPassthrough) + } + if tt.route.Spec.TLS.InsecureEdgeTerminationPolicy != routev1.InsecureEdgeTerminationPolicyNone { + t.Errorf("Insecure edge termination policy: got %v, want %v", tt.route.Spec.TLS.InsecureEdgeTerminationPolicy, routev1.InsecureEdgeTerminationPolicyNone) + } + } + }) + } +} + +func TestAddHCPRouteLabel(t *testing.T) { + tests := []struct { + name string + route *routev1.Route + expectedLabels map[string]string + }{ + { + name: "When route has no labels, it should add HCP label", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + }, + }, + expectedLabels: map[string]string{ + HCPRouteLabel: "test-namespace", + }, + }, + { + name: "When route has existing labels, it should add HCP label without removing existing ones", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + Labels: map[string]string{ + "existing-label": "existing-value", + }, + }, + }, + expectedLabels: map[string]string{ + "existing-label": "existing-value", + HCPRouteLabel: "test-namespace", + }, + }, + { + name: "When route already has HCP label, it should update it", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + Labels: map[string]string{ + HCPRouteLabel: "old-namespace", + }, + }, + }, + expectedLabels: map[string]string{ + HCPRouteLabel: "test-namespace", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + AddHCPRouteLabel(tt.route) + + if len(tt.route.Labels) != len(tt.expectedLabels) { + t.Errorf("Labels length mismatch: got %d, want %d. Got: %v, Want: %v", len(tt.route.Labels), len(tt.expectedLabels), tt.route.Labels, tt.expectedLabels) + } + for k, v := range tt.expectedLabels { + if got, ok := tt.route.Labels[k]; !ok || got != v { + t.Errorf("Label %s: got %v, want %v", k, got, v) + } + } + }) + } +} + +func TestRemoveHCPRouteLabel(t *testing.T) { + tests := []struct { + name string + route *routev1.Route + expectedLabels map[string]string + }{ + { + name: "When route has no labels, it should not error", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + }, + }, + expectedLabels: nil, + }, + { + name: "When route has HCP label only, it should remove it and leave empty map", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + Labels: map[string]string{ + HCPRouteLabel: "test-namespace", + }, + }, + }, + expectedLabels: map[string]string{}, + }, + { + name: "When route has HCP label and other labels, it should remove only HCP label", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + Labels: map[string]string{ + HCPRouteLabel: "test-namespace", + "existing-label": "existing-value", + "another-label": "another-value", + }, + }, + }, + expectedLabels: map[string]string{ + "existing-label": "existing-value", + "another-label": "another-value", + }, + }, + { + name: "When route does not have HCP label but has other labels, it should not modify labels", + route: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + Labels: map[string]string{ + "existing-label": "existing-value", + }, + }, + }, + expectedLabels: map[string]string{ + "existing-label": "existing-value", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + RemoveHCPRouteLabel(tt.route) + + if tt.expectedLabels == nil { + if len(tt.route.Labels) != 0 { + t.Errorf("Expected no labels, but got %v", tt.route.Labels) + } + } else { + if len(tt.route.Labels) != len(tt.expectedLabels) { + t.Errorf("Labels length mismatch: got %d, want %d. Got: %v, Want: %v", len(tt.route.Labels), len(tt.expectedLabels), tt.route.Labels, tt.expectedLabels) + } + for k, v := range tt.expectedLabels { + if got, ok := tt.route.Labels[k]; !ok || got != v { + t.Errorf("Label %s: got %v, want %v", k, got, v) + } + } + // Ensure HCP label is not present + if _, hasLabel := tt.route.Labels[HCPRouteLabel]; hasLabel { + t.Errorf("Expected HCP label to be removed, but it still exists") + } + } + }) + } +} diff --git a/support/util/visibility.go b/support/util/visibility.go index 9c5918d3b0d..9fb085bebda 100644 --- a/support/util/visibility.go +++ b/support/util/visibility.go @@ -37,16 +37,95 @@ func IsPublicHC(hc *hyperv1.HostedCluster) bool { return access == hyperv1.PublicAndPrivate || access == hyperv1.Public } -func IsPublicWithDNS(hcp *hyperv1.HostedControlPlane) bool { - return IsPublicHCP(hcp) && (UseDedicatedDNS(hcp, hyperv1.APIServer) || - UseDedicatedDNS(hcp, hyperv1.OAuthServer) || - UseDedicatedDNS(hcp, hyperv1.Konnectivity) || - UseDedicatedDNS(hcp, hyperv1.Ignition)) -} +// LabelHCPRoutes determines if routes should be labeled for admission by the HCP router. +// Routes with the label "hypershift.openshift.io/hosted-control-plane" are served by a +// dedicated HCP router (HAProxy deployment in the HCP namespace). Routes without this label +// are served by the management cluster's default OpenShift ingress controller. +// +// This function is the single source of truth for route labeling decisions and is called by: +// - OAuth route reconciliation (external public/private routes) +// - Konnectivity route reconciliation (external routes) +// - Ignition server route reconciliation (external routes) +// - Router component predicate (determines if router Deployment/ConfigMap/PDB are created) +// - Router service creation (determines if public router LoadBalancer service is created) +// +// The HCP router infrastructure (Deployment, Services) is created when routes need to be labeled. +// This ensures routes and router services stay synchronized. +// +// # Platform-Specific Behavior +// +// AWS Platform: +// - Private: Always labels routes (no public access) +// - PublicAndPrivate + KAS LoadBalancer: Does NOT label external routes (uses mgmt cluster router) +// - PublicAndPrivate + KAS Route: Labels routes (uses HCP router for all routes) +// - Public + KAS LoadBalancer: Does NOT label routes (uses mgmt cluster router) +// - Public + KAS Route: Labels routes (uses HCP router) +// +// Agent Platform (bare metal): +// - No EndpointAccess field (no Private/PublicAndPrivate concept) +// - Labels routes ONLY when KAS uses Route with explicit hostname +// - KAS LoadBalancer/NodePort: Does NOT label routes (uses mgmt cluster router) +// +// KubeVirt, OpenStack, None Platforms: +// - Same behavior as Agent platform +// - Labels routes ONLY when KAS uses Route with explicit hostname +// +// IBM Cloud Platform: +// - Never labels routes (uses different routing mechanism) +// +// # Internal Routes +// +// Note that internal routes (*.apps..hypershift.local) are ALWAYS labeled for +// HCP router regardless of this function's return value. This function only controls +// EXTERNAL route labeling. Internal routes are handled separately in ReconcileInternalRoute(). +// +// # Architecture Reference +// +// For complete details on the HCP ingress architecture, see HCP_INGRESS_ARCHITECTURE.md +// in the repository root, which documents the full decision flow, code references, and +// interaction between route labeling and router service creation. +// +// Returns true when routes should be labeled for HCP router; false when routes should +// use the management cluster router. +func LabelHCPRoutes(hcp *hyperv1.HostedControlPlane) bool { + switch hcp.Spec.Platform.Type { + case hyperv1.AWSPlatform: + // AWS supports endpoint access modes (Private/PublicAndPrivate/Public). + // Label routes for HCP router when: + // 1. Cluster has no public access (Private-only), OR + // 2. Public cluster with dedicated DNS for KAS (KAS uses Route with hostname) + // + // For PublicAndPrivate clusters using LoadBalancer for KAS: + // - Internal routes (Konnectivity, Ignition) are served by internal HCP router + // - External routes (OAuth) use the management cluster router (no public HCP router needed) + // This avoids creating an unnecessary public LoadBalancer service. + return !IsPublicHCP(hcp) || UseDedicatedDNSForKAS(hcp) + + case hyperv1.AgentPlatform, hyperv1.KubevirtPlatform, hyperv1.OpenStackPlatform, hyperv1.NonePlatform: + // These platforms do not have endpoint access mode concepts (no Private/PublicAndPrivate). + // Label routes for HCP router ONLY when KAS explicitly uses Route with a hostname. + // + // This prevents creating HCP router infrastructure when: + // - KAS uses LoadBalancer (routes should use management cluster router) + // - KAS uses NodePort (routes should use management cluster router) + // + // When KAS uses Route with hostname, all routes are labeled for HCP router to ensure + // consistent routing through dedicated infrastructure. + return UseDedicatedDNSForKAS(hcp) + + case hyperv1.IBMCloudPlatform: + // IBM Cloud uses a different routing mechanism (shared ingress with HAProxy and + // kube-apiserver-proxy on worker nodes). Never use HCP router. + return false -func IsPublicWithDNSByHC(hc *hyperv1.HostedCluster) bool { - return IsPublicHC(hc) && (UseDedicatedDNSByHC(hc, hyperv1.APIServer) || - UseDedicatedDNSByHC(hc, hyperv1.OAuthServer) || - UseDedicatedDNSByHC(hc, hyperv1.Konnectivity) || - UseDedicatedDNSByHC(hc, hyperv1.Ignition)) + case hyperv1.PowerVSPlatform: + // PowerVS (IBM Cloud Power Virtual Servers) follows the same pattern as other + // platforms without endpoint access modes. + return UseDedicatedDNSForKAS(hcp) + + default: + // Conservative default for unknown platforms: do not create HCP router infrastructure. + // Routes will use the management cluster router. + return false + } } diff --git a/support/util/visibility_test.go b/support/util/visibility_test.go index 9dfb447ba46..ff88aadf4dd 100644 --- a/support/util/visibility_test.go +++ b/support/util/visibility_test.go @@ -165,3 +165,507 @@ func TestIsPublicHCP(t *testing.T) { }) } } + +func TestLabelHCPRoutes(t *testing.T) { + tests := []struct { + name string + hcp *hyperv1.HostedControlPlane + want bool + }{ + // AWS Platform Tests + { + name: "When AWS cluster is Private, it should label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: hyperv1.Private, + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + want: true, + }, + { + name: "When AWS cluster is PublicAndPrivate with KAS LoadBalancer, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: hyperv1.PublicAndPrivate, + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "When AWS cluster is PublicAndPrivate with KAS Route and hostname, it should label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: hyperv1.PublicAndPrivate, + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "api.example.com", + }, + }, + }, + }, + }, + }, + want: true, + }, + { + name: "When AWS cluster is Public with KAS LoadBalancer, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: hyperv1.Public, + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "When AWS cluster is Public with KAS Route and hostname, it should label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: hyperv1.Public, + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "api.example.com", + }, + }, + }, + }, + }, + }, + want: true, + }, + { + name: "When AWS cluster is Public with KAS Route but no hostname, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: hyperv1.Public, + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + }, + }, + }, + }, + }, + want: false, + }, + + // Agent Platform Tests (Bare Metal) - Critical for OCPBUGS-70152 + { + name: "When Agent cluster has KAS LoadBalancer, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AgentPlatform, + Agent: &hyperv1.AgentPlatformSpec{ + AgentNamespace: "agent-ns", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "When Agent cluster has KAS Route with hostname, it should label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AgentPlatform, + Agent: &hyperv1.AgentPlatformSpec{ + AgentNamespace: "agent-ns", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "api.agent.example.com", + }, + }, + }, + }, + }, + }, + want: true, + }, + { + name: "When Agent cluster has KAS NodePort, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AgentPlatform, + Agent: &hyperv1.AgentPlatformSpec{ + AgentNamespace: "agent-ns", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.NodePort, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "When Agent cluster has OAuth Route with hostname but KAS LoadBalancer, it should not label routes for HCP router (OCPBUGS-70152 bug scenario)", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AgentPlatform, + Agent: &hyperv1.AgentPlatformSpec{ + AgentNamespace: "agent-ns", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + { + Service: hyperv1.OAuthServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "oauth.agent.example.com", + }, + }, + }, + }, + }, + }, + want: false, // Should NOT create HCP router - this was the bug + }, + + // KubeVirt Platform Tests + { + name: "When KubeVirt cluster has KAS LoadBalancer, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtPlatformSpec{ + GenerateID: "kubevirt-cluster", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "When KubeVirt cluster has KAS Route with hostname, it should label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtPlatformSpec{ + GenerateID: "kubevirt-cluster", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "api.kubevirt.example.com", + }, + }, + }, + }, + }, + }, + want: true, + }, + + // OpenStack Platform Tests + { + name: "When OpenStack cluster has KAS LoadBalancer, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.OpenStackPlatform, + OpenStack: &hyperv1.OpenStackPlatformSpec{ + IdentityRef: hyperv1.OpenStackIdentityReference{ + Name: "openstack-creds", + }, + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "When OpenStack cluster has KAS Route with hostname, it should label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.OpenStackPlatform, + OpenStack: &hyperv1.OpenStackPlatformSpec{ + IdentityRef: hyperv1.OpenStackIdentityReference{ + Name: "openstack-creds", + }, + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "api.openstack.example.com", + }, + }, + }, + }, + }, + }, + want: true, + }, + + // IBM Cloud Platform Tests + { + name: "When IBM Cloud cluster has KAS LoadBalancer, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.IBMCloudPlatform, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "When IBM Cloud cluster has KAS Route with hostname, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.IBMCloudPlatform, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "api.ibmcloud.example.com", + }, + }, + }, + }, + }, + }, + want: false, // IBM Cloud never uses HCP router + }, + + // PowerVS Platform Tests + { + name: "When PowerVS cluster has KAS LoadBalancer, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.PowerVSPlatform, + PowerVS: &hyperv1.PowerVSPlatformSpec{ + AccountID: "test-account", + CISInstanceCRN: "test-crn", + ResourceGroup: "test-rg", + Region: "us-south", + Zone: "us-south-1", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "When PowerVS cluster has KAS Route with hostname, it should label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.PowerVSPlatform, + PowerVS: &hyperv1.PowerVSPlatformSpec{ + AccountID: "test-account", + CISInstanceCRN: "test-crn", + ResourceGroup: "test-rg", + Region: "us-south", + Zone: "us-south-1", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "api.powervs.example.com", + }, + }, + }, + }, + }, + }, + want: true, + }, + + // None Platform Tests + { + name: "When None platform cluster has KAS LoadBalancer, it should not label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.NonePlatform, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "When None platform cluster has KAS Route with hostname, it should label routes for HCP router", + hcp: &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.NonePlatform, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: "api.none.example.com", + }, + }, + }, + }, + }, + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := LabelHCPRoutes(tt.hcp) + if got != tt.want { + t.Errorf("LabelHCPRoutes() = %v, want %v", got, tt.want) + } + }) + } +}