From 04a214f6190dbb33029f9c1ceeba7d0b09ecd0f0 Mon Sep 17 00:00:00 2001 From: Claudio Busse Date: Thu, 7 May 2026 14:40:35 +0200 Subject: [PATCH] fix(aws): set aws-load-balancer-scheme on public HCP router service The AWS Load Balancer Controller (used on EKS) defaults to scheme=internal when the aws-load-balancer-scheme annotation is not set, while the in-tree AWS cloud provider (used on OpenShift) defaults to internet-facing. This causes the public HCP router NLB to be created as internal on EKS, making KAS and OAuth unreachable from outside the VPC for PublicAndPrivate clusters. Set the annotation explicitly on the public router service so it works on both OpenShift and EKS management clusters. Also clean up mutually exclusive annotations (aws-load-balancer-internal vs aws-load-balancer-scheme) when switching between internal and external. Signed-off-by: Claudio Busse Commit-Message-Assisted-by: Claude (via Claude Code) --- .../hostedcontrolplane/infra/infra_test.go | 4 +- ...astructure_AWS_PublicAndPrivate_Route.yaml | 1 + ...oncileInfrastructure_AWS_Public_Route.yaml | 1 + .../hostedcontrolplane/ingress/router.go | 9 +++ .../hostedcontrolplane/ingress/router_test.go | 65 +++++++++++++++++++ 5 files changed, 79 insertions(+), 1 deletion(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/infra/infra_test.go b/control-plane-operator/controllers/hostedcontrolplane/infra/infra_test.go index f344b921cc4..1636e722d9b 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/infra/infra_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/infra/infra_test.go @@ -1475,7 +1475,8 @@ func TestReconcileHCPRouterServices(t *testing.T) { Name: "router", Namespace: namespace, Annotations: map[string]string{ - "service.beta.kubernetes.io/aws-load-balancer-type": "nlb", + "service.beta.kubernetes.io/aws-load-balancer-type": "nlb", + "service.beta.kubernetes.io/aws-load-balancer-scheme": "internet-facing", }, Labels: map[string]string{"app": "private-router"}, }, @@ -1497,6 +1498,7 @@ func TestReconcileHCPRouterServices(t *testing.T) { return publicService(append(m, func(s *corev1.Service) { s.Name = "private-router" s.Annotations["service.beta.kubernetes.io/aws-load-balancer-internal"] = "true" + delete(s.Annotations, "service.beta.kubernetes.io/aws-load-balancer-scheme") })...) } withCrossZoneAnnotation := func(svc *corev1.Service) { diff --git a/control-plane-operator/controllers/hostedcontrolplane/infra/testdata/zz_fixture_TestReconcileInfrastructure_AWS_PublicAndPrivate_Route.yaml b/control-plane-operator/controllers/hostedcontrolplane/infra/testdata/zz_fixture_TestReconcileInfrastructure_AWS_PublicAndPrivate_Route.yaml index 452e8b2d7e1..a530d255d63 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/infra/testdata/zz_fixture_TestReconcileInfrastructure_AWS_PublicAndPrivate_Route.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/infra/testdata/zz_fixture_TestReconcileInfrastructure_AWS_PublicAndPrivate_Route.yaml @@ -286,6 +286,7 @@ services: annotations: service.beta.kubernetes.io/aws-load-balancer-attributes: load_balancing.cross_zone.enabled=true service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled: "true" + service.beta.kubernetes.io/aws-load-balancer-scheme: internet-facing service.beta.kubernetes.io/aws-load-balancer-type: nlb labels: app: private-router diff --git a/control-plane-operator/controllers/hostedcontrolplane/infra/testdata/zz_fixture_TestReconcileInfrastructure_AWS_Public_Route.yaml b/control-plane-operator/controllers/hostedcontrolplane/infra/testdata/zz_fixture_TestReconcileInfrastructure_AWS_Public_Route.yaml index 0b603a11a1b..b0eedd091b3 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/infra/testdata/zz_fixture_TestReconcileInfrastructure_AWS_Public_Route.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/infra/testdata/zz_fixture_TestReconcileInfrastructure_AWS_Public_Route.yaml @@ -239,6 +239,7 @@ services: loadBalancer: {} - metadata: annotations: + service.beta.kubernetes.io/aws-load-balancer-scheme: internet-facing service.beta.kubernetes.io/aws-load-balancer-type: nlb labels: app: private-router diff --git a/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go b/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go index cc0e845217e..8421a068550 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go +++ b/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go @@ -29,6 +29,15 @@ func ReconcileRouterService(svc *corev1.Service, internal, crossZoneLoadBalancin svc.Annotations["service.beta.kubernetes.io/aws-load-balancer-type"] = "nlb" if internal { svc.Annotations["service.beta.kubernetes.io/aws-load-balancer-internal"] = "true" + delete(svc.Annotations, "service.beta.kubernetes.io/aws-load-balancer-scheme") + } else { + delete(svc.Annotations, "service.beta.kubernetes.io/aws-load-balancer-internal") + // The AWS Load Balancer Controller (used on EKS) defaults to scheme=internal: + // https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.4/guide/service/annotations/ + // The in-tree AWS cloud provider (used on OpenShift) defaults to internet-facing: + // https://cloud-provider-aws.sigs.k8s.io/service_controller/ + // Set the annotation explicitly so the public router works on both. + svc.Annotations["service.beta.kubernetes.io/aws-load-balancer-scheme"] = "internet-facing" } if crossZoneLoadBalancingEnabled { // In-tree AWS cloud provider annotation for cross-zone load balancing (OpenShift management clusters). diff --git a/control-plane-operator/controllers/hostedcontrolplane/ingress/router_test.go b/control-plane-operator/controllers/hostedcontrolplane/ingress/router_test.go index 81dfec25637..f2df7f8de0d 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/ingress/router_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/ingress/router_test.go @@ -3,6 +3,8 @@ package ingress import ( "testing" + . "github.com/onsi/gomega" + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/support/netutil" @@ -56,6 +58,69 @@ func TestReconcileRouterServiceAnnotations(t *testing.T) { } } +// When reconciling an external (non-internal) AWS router service +// it should set aws-load-balancer-scheme to internet-facing. +func TestReconcileRouterService_WhenExternalAWS_ItShouldSetInternetFacingScheme(t *testing.T) { + g := NewGomegaWithT(t) + + hcp := &hyperv1.HostedControlPlane{} + hcp.Spec.Platform.Type = hyperv1.AWSPlatform + + svc := &corev1.Service{} + + err := ReconcileRouterService(svc, false /* internal */, true /* cross-zone */, hcp) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(svc.Annotations).To(HaveKeyWithValue( + "service.beta.kubernetes.io/aws-load-balancer-scheme", "internet-facing")) + g.Expect(svc.Annotations).ToNot(HaveKey( + "service.beta.kubernetes.io/aws-load-balancer-internal")) +} + +func TestReconcileRouterService_WhenSwitchingMode_ItShouldRemoveConflictingAnnotation(t *testing.T) { + tests := []struct { + name string + internal bool + preExisting map[string]string + expectKey string + expectValue string + expectAbsent string + }{ + { + name: "When switching from internal to external it should remove the internal annotation", + internal: false, + preExisting: map[string]string{"service.beta.kubernetes.io/aws-load-balancer-internal": "true"}, + expectKey: "service.beta.kubernetes.io/aws-load-balancer-scheme", + expectValue: "internet-facing", + expectAbsent: "service.beta.kubernetes.io/aws-load-balancer-internal", + }, + { + name: "When switching from external to internal it should remove the scheme annotation", + internal: true, + preExisting: map[string]string{"service.beta.kubernetes.io/aws-load-balancer-scheme": "internet-facing"}, + expectKey: "service.beta.kubernetes.io/aws-load-balancer-internal", + expectValue: "true", + expectAbsent: "service.beta.kubernetes.io/aws-load-balancer-scheme", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + hcp := &hyperv1.HostedControlPlane{} + hcp.Spec.Platform.Type = hyperv1.AWSPlatform + + svc := &corev1.Service{} + svc.Annotations = tt.preExisting + + err := ReconcileRouterService(svc, tt.internal, true /* cross-zone */, hcp) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(svc.Annotations).To(HaveKeyWithValue(tt.expectKey, tt.expectValue)) + g.Expect(svc.Annotations).ToNot(HaveKey(tt.expectAbsent)) + }) + } +} + // Test that LoadBalancerSourceRanges is applied for external router services with allowedCIDRBlocks func TestReconcileRouterService_AppliesLoadBalancerSourceRanges(t *testing.T) { // Test case 1: External router service should have LoadBalancerSourceRanges set