From 8c37fb06ddc09148573f44b43bc2e1d5ad1d6ef6 Mon Sep 17 00:00:00 2001 From: Chin2691 Date: Wed, 10 Jun 2026 09:36:00 +0530 Subject: [PATCH] RFE-9274: add disable-external-dns-management annotation When loadBalancer.hostname is configured, the CPO unconditionally injects external-dns.alpha.kubernetes.io/hostname on LoadBalancer services. In environments without an external-dns controller this causes a permanent false-negative ExternalDNSReachable condition. Add hypershift.openshift.io/disable-external-dns-management annotation that suppresses the external-dns hostname annotation injection on KAS, Konnectivity, and OAuth LoadBalancer services. On day-2 opt-out the annotation is actively removed from pre-existing services. The ExternalDNSReachable condition reports True with reason ExternalDNSManagementDisabled when opted out. Co-authored-by: Cursor --- api/hypershift/v1beta1/hostedcluster_types.go | 5 + .../hostedcontrolplane_controller.go | 12 ++ .../hostedcontrolplane/infra/infra.go | 2 +- .../hostedcontrolplane/kas/service.go | 12 +- .../hostedcontrolplane/kas/service_test.go | 125 ++++++++++++++++++ .../hostedcontrolplane/oauth/service.go | 8 +- .../hostedcontrolplane/oauth/service_test.go | 45 ++++++- .../hostedcluster/hostedcluster_controller.go | 1 + .../hypershift/v1beta1/hostedcluster_types.go | 5 + 9 files changed, 209 insertions(+), 6 deletions(-) diff --git a/api/hypershift/v1beta1/hostedcluster_types.go b/api/hypershift/v1beta1/hostedcluster_types.go index 74b50f24ad3..ef6ff5bd058 100644 --- a/api/hypershift/v1beta1/hostedcluster_types.go +++ b/api/hypershift/v1beta1/hostedcluster_types.go @@ -159,6 +159,11 @@ const ( // ExternalDNSHostnameAnnotation is the annotation external-dns uses to register DNS name for different HCP services. ExternalDNSHostnameAnnotation = "external-dns.alpha.kubernetes.io/hostname" + // DisableExternalDNSManagementAnnotation when set to "true" on a HostedCluster prevents the + // control-plane-operator from adding the external-dns hostname annotation to LoadBalancer services. + // This allows users to manage their own DNS records for the API and Konnectivity endpoints. + DisableExternalDNSManagementAnnotation = "hypershift.openshift.io/disable-external-dns-management" + // ForceUpgradeToAnnotation is the annotation that forces HostedCluster upgrade even if the underlying ClusterVersion // is reporting it is not Upgradeable. The annotation value must be set to the release image being forced. ForceUpgradeToAnnotation = "hypershift.openshift.io/force-upgrade-to" diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 29e98532508..de3fff79a22 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -773,6 +773,18 @@ func (r *HostedControlPlaneReconciler) reconcileExternalDNSStatusCondition(ctx c Reason: hyperv1.StatusUnknownReason, } + if hostedControlPlane.Annotations[hyperv1.DisableExternalDNSManagementAnnotation] == "true" { + newCondition = metav1.Condition{ + Type: string(hyperv1.ExternalDNSReachable), + Status: metav1.ConditionTrue, + Reason: "ExternalDNSManagementDisabled", + Message: "External DNS management is disabled by annotation", + } + newCondition.ObservedGeneration = hostedControlPlane.Generation + meta.SetStatusCondition(&hostedControlPlane.Status.Conditions, newCondition) + return + } + kasExternalHostname := netutil.ServiceExternalDNSHostname(hostedControlPlane, hyperv1.APIServer) if kasExternalHostname != "" { if err := netutil.ResolveDNSHostname(ctx, kasExternalHostname); err != nil { diff --git a/control-plane-operator/controllers/hostedcontrolplane/infra/infra.go b/control-plane-operator/controllers/hostedcontrolplane/infra/infra.go index 6f5f85a5190..86fba3e5da8 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/infra/infra.go +++ b/control-plane-operator/controllers/hostedcontrolplane/infra/infra.go @@ -348,7 +348,7 @@ func (r *Reconciler) reconcileOAuthServerService(ctx context.Context, hcp *hyper p := oauth.NewOAuthServiceParams(hcp) oauthServerService := manifests.OauthServerService(hcp.Namespace) if _, err := createOrUpdate(ctx, r.Client, oauthServerService, func() error { - return oauth.ReconcileService(oauthServerService, p.OwnerRef, serviceStrategy, hcp.Spec.Platform.Type, netutil.IsPrivateHCP(hcp)) + return oauth.ReconcileService(oauthServerService, p.OwnerRef, serviceStrategy, hcp.Spec.Platform.Type, netutil.IsPrivateHCP(hcp), hcp.Annotations[hyperv1.DisableExternalDNSManagementAnnotation] == "true") }); err != nil { return fmt.Errorf("failed to reconcile OAuth service: %w", err) } diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go index 1c69a7d4697..591f0e81ccd 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go @@ -77,7 +77,11 @@ func ReconcileService(svc *corev1.Service, strategy *hyperv1.ServicePublishingSt svc.Annotations[AWSNLBAnnotation] = "nlb" } if strategy.LoadBalancer != nil && strategy.LoadBalancer.Hostname != "" { - svc.Annotations[hyperv1.ExternalDNSHostnameAnnotation] = strategy.LoadBalancer.Hostname + if hcp.Annotations[hyperv1.DisableExternalDNSManagementAnnotation] != "true" { + svc.Annotations[hyperv1.ExternalDNSHostnameAnnotation] = strategy.LoadBalancer.Hostname + } else { + delete(svc.Annotations, hyperv1.ExternalDNSHostnameAnnotation) + } } if !azureutil.IsAroHCP() { svc.Spec.LoadBalancerSourceRanges = apiAllowedCIDRBlocks @@ -309,7 +313,11 @@ func ReconcileKonnectivityServerService(svc *corev1.Service, ownerRef config.Own if svc.Annotations == nil { svc.Annotations = map[string]string{} } - svc.Annotations[hyperv1.ExternalDNSHostnameAnnotation] = strategy.LoadBalancer.Hostname + if hcp.Annotations[hyperv1.DisableExternalDNSManagementAnnotation] != "true" { + svc.Annotations[hyperv1.ExternalDNSHostnameAnnotation] = strategy.LoadBalancer.Hostname + } else { + delete(svc.Annotations, hyperv1.ExternalDNSHostnameAnnotation) + } } case hyperv1.NodePort: svc.Spec.Type = corev1.ServiceTypeNodePort diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/service_test.go b/control-plane-operator/controllers/hostedcontrolplane/kas/service_test.go index 67dfe1e8d4b..e40f1ee01ff 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/service_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/service_test.go @@ -438,3 +438,128 @@ func TestKonnectivityServiceReconcile(t *testing.T) { }) } } + +func TestReconcileServiceDisableExternalDNS(t *testing.T) { + host := "api.example.com" + + testCases := []struct { + name string + hcpAnnotations map[string]string + existingAnnotations map[string]string + expectAnnotation bool + }{ + { + name: "ExternalDNS annotation is set when disable annotation is absent", + hcpAnnotations: nil, + expectAnnotation: true, + }, + { + name: "ExternalDNS annotation is not set when disable annotation is true", + hcpAnnotations: map[string]string{hyperv1.DisableExternalDNSManagementAnnotation: "true"}, + expectAnnotation: false, + }, + { + name: "ExternalDNS annotation is set when disable annotation is false", + hcpAnnotations: map[string]string{hyperv1.DisableExternalDNSManagementAnnotation: "false"}, + expectAnnotation: true, + }, + { + name: "ExternalDNS annotation is deleted on day-2 opt-out", + hcpAnnotations: map[string]string{hyperv1.DisableExternalDNSManagementAnnotation: "true"}, + existingAnnotations: map[string]string{ + hyperv1.ExternalDNSHostnameAnnotation: host, + }, + expectAnnotation: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: v1.ObjectMeta{ + Annotations: tc.hcpAnnotations, + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: hyperv1.Public, + }, + }, + }, + } + svc := &corev1.Service{ + ObjectMeta: v1.ObjectMeta{ + Annotations: tc.existingAnnotations, + }, + } + strategy := &hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + Hostname: host, + }, + } + + err := ReconcileService(svc, strategy, &v1.OwnerReference{}, 6443, []string{}, hcp) + g.Expect(err).ToNot(HaveOccurred()) + + if tc.expectAnnotation { + g.Expect(svc.Annotations).To(HaveKeyWithValue(hyperv1.ExternalDNSHostnameAnnotation, host)) + } else { + g.Expect(svc.Annotations).ToNot(HaveKey(hyperv1.ExternalDNSHostnameAnnotation)) + } + }) + } +} + +func TestReconcileKonnectivityServiceDisableExternalDNS(t *testing.T) { + host := "konnectivity.example.com" + + testCases := []struct { + name string + hcpAnnotations map[string]string + expectAnnotation bool + }{ + { + name: "ExternalDNS annotation is set when disable annotation is absent", + hcpAnnotations: nil, + expectAnnotation: true, + }, + { + name: "ExternalDNS annotation is not set when disable annotation is true", + hcpAnnotations: map[string]string{hyperv1.DisableExternalDNSManagementAnnotation: "true"}, + expectAnnotation: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: v1.ObjectMeta{ + Annotations: tc.hcpAnnotations, + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{Type: hyperv1.AWSPlatform}, + }, + } + svc := &corev1.Service{} + strategy := &hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + Hostname: host, + }, + } + + err := ReconcileKonnectivityServerService(svc, config.OwnerRef{}, strategy, hcp) + g.Expect(err).ToNot(HaveOccurred()) + + if tc.expectAnnotation { + g.Expect(svc.Annotations).To(HaveKeyWithValue(hyperv1.ExternalDNSHostnameAnnotation, host)) + } else { + g.Expect(svc.Annotations).ToNot(HaveKey(hyperv1.ExternalDNSHostnameAnnotation)) + } + }) + } +} diff --git a/control-plane-operator/controllers/hostedcontrolplane/oauth/service.go b/control-plane-operator/controllers/hostedcontrolplane/oauth/service.go index 3060fe63c64..f69e1795b26 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/oauth/service.go +++ b/control-plane-operator/controllers/hostedcontrolplane/oauth/service.go @@ -27,7 +27,7 @@ var ( } ) -func ReconcileService(svc *corev1.Service, ownerRef config.OwnerRef, strategy *hyperv1.ServicePublishingStrategy, platformType hyperv1.PlatformType, isPrivate bool) error { +func ReconcileService(svc *corev1.Service, ownerRef config.OwnerRef, strategy *hyperv1.ServicePublishingStrategy, platformType hyperv1.PlatformType, isPrivate bool, disableExternalDNS bool) error { ownerRef.ApplyTo(svc) if svc.Spec.Selector == nil { svc.Spec.Selector = oauthServerLabels @@ -69,7 +69,11 @@ func ReconcileService(svc *corev1.Service, ownerRef config.OwnerRef, strategy *h svc.Annotations = map[string]string{} } if strategy.LoadBalancer != nil && strategy.LoadBalancer.Hostname != "" { - svc.Annotations[hyperv1.ExternalDNSHostnameAnnotation] = strategy.LoadBalancer.Hostname + if !disableExternalDNS { + svc.Annotations[hyperv1.ExternalDNSHostnameAnnotation] = strategy.LoadBalancer.Hostname + } else { + delete(svc.Annotations, hyperv1.ExternalDNSHostnameAnnotation) + } } if isPrivate { svc.Annotations[azureutil.InternalLoadBalancerAnnotation] = azureutil.InternalLoadBalancerValue diff --git a/control-plane-operator/controllers/hostedcontrolplane/oauth/service_test.go b/control-plane-operator/controllers/hostedcontrolplane/oauth/service_test.go index 68ce94c5292..9ae51130935 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/oauth/service_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/oauth/service_test.go @@ -213,7 +213,7 @@ func TestOauthServiceReconcile(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - err := ReconcileService(&tc.svc_in, config.OwnerRef{}, &tc.strategy, tc.platform, tc.isPrivate) + err := ReconcileService(&tc.svc_in, config.OwnerRef{}, &tc.strategy, tc.platform, tc.isPrivate, false) if tc.err == nil { g.Expect(err).ToNot(HaveOccurred()) @@ -351,3 +351,46 @@ func TestReconcileServiceStatus(t *testing.T) { }) } } + +func TestOauthServiceDisableExternalDNS(t *testing.T) { + hostname := "oauth.example.com" + + testCases := []struct { + name string + disableExtDNS bool + expectAnnotation bool + }{ + { + name: "ExternalDNS annotation is set when not disabled", + disableExtDNS: false, + expectAnnotation: true, + }, + { + name: "ExternalDNS annotation is not set when disabled", + disableExtDNS: true, + expectAnnotation: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + svc := &corev1.Service{} + strategy := &v1beta1.ServicePublishingStrategy{ + Type: v1beta1.LoadBalancer, + LoadBalancer: &v1beta1.LoadBalancerPublishingStrategy{ + Hostname: hostname, + }, + } + + err := ReconcileService(svc, config.OwnerRef{}, strategy, v1beta1.AzurePlatform, false, tc.disableExtDNS) + g.Expect(err).ToNot(HaveOccurred()) + + if tc.expectAnnotation { + g.Expect(svc.Annotations).To(HaveKeyWithValue(v1beta1.ExternalDNSHostnameAnnotation, hostname)) + } else { + g.Expect(svc.Annotations).ToNot(HaveKey(v1beta1.ExternalDNSHostnameAnnotation)) + } + }) + } +} diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 095522e7bd2..473c22c54d3 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -2372,6 +2372,7 @@ func reconcileHostedControlPlaneAnnotations(hcp *hyperv1.HostedControlPlane, hcl "hypershift.openshift.io/aws-termination-handler-queue-url", hyperv1.SwiftPodNetworkInstanceAnnotation, hyperv1.EnableMetricsForwarding, + hyperv1.DisableExternalDNSManagementAnnotation, } for _, key := range mirroredAnnotations { val, hasVal := hcluster.Annotations[key] diff --git a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go index 74b50f24ad3..ef6ff5bd058 100644 --- a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go +++ b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go @@ -159,6 +159,11 @@ const ( // ExternalDNSHostnameAnnotation is the annotation external-dns uses to register DNS name for different HCP services. ExternalDNSHostnameAnnotation = "external-dns.alpha.kubernetes.io/hostname" + // DisableExternalDNSManagementAnnotation when set to "true" on a HostedCluster prevents the + // control-plane-operator from adding the external-dns hostname annotation to LoadBalancer services. + // This allows users to manage their own DNS records for the API and Konnectivity endpoints. + DisableExternalDNSManagementAnnotation = "hypershift.openshift.io/disable-external-dns-management" + // ForceUpgradeToAnnotation is the annotation that forces HostedCluster upgrade even if the underlying ClusterVersion // is reporting it is not Upgradeable. The annotation value must be set to the release image being forced. ForceUpgradeToAnnotation = "hypershift.openshift.io/force-upgrade-to"