Use a per-hcp router for all hcp ingress traffic

api/fixtures/example.go

@@ -214,13 +214,13 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
 				},
 			}
 		}
-		services = getIngressServicePublishingStrategyMapping(o.NetworkType)
+		services = getIngressServicePublishingStrategyMapping(o.NetworkType, o.ExternalDNSDomain != "")
 		if o.ExternalDNSDomain != "" {
 			for i, svc := range services {
 				switch svc.Service {
 				case hyperv1.APIServer:
 					if endpointAccess != hyperv1.Private {
-						services[i].LoadBalancer = &hyperv1.LoadBalancerPublishingStrategy{
+						services[i].Route = &hyperv1.RoutePublishingStrategy{
 							Hostname: fmt.Sprintf("api-%s.%s", o.Name, o.ExternalDNSDomain),
 						}
 					}
@@ -262,7 +262,7 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
 		if o.None.APIServerAddress != "" {
 			services = getServicePublishingStrategyMappingByAPIServerAddress(o.None.APIServerAddress, o.NetworkType)
 		} else {
-			services = getIngressServicePublishingStrategyMapping(o.NetworkType)
+			services = getIngressServicePublishingStrategyMapping(o.NetworkType, o.ExternalDNSDomain != "")
 		}
 	case o.Agent != nil:
 		platformSpec = hyperv1.PlatformSpec{
@@ -300,7 +300,7 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
 		case "NodePort":
 			services = getServicePublishingStrategyMappingByAPIServerAddress(o.Kubevirt.APIServerAddress, o.NetworkType)
 		case "Ingress":
-			services = getIngressServicePublishingStrategyMapping(o.NetworkType)
+			services = getIngressServicePublishingStrategyMapping(o.NetworkType, o.ExternalDNSDomain != "")
 		default:
 			panic(fmt.Sprintf("service publishing type %s is not supported", o.Kubevirt.ServicePublishingStrategy))
 		}
@@ -337,7 +337,7 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
 				SecurityGroupName: o.Azure.SecurityGroupName,
 			},
 		}
-		services = getIngressServicePublishingStrategyMapping(o.NetworkType)
+		services = getIngressServicePublishingStrategyMapping(o.NetworkType, o.ExternalDNSDomain != "")
 
 	case o.PowerVS != nil:
 		buildIBMCloudCreds := func(name, apikey string) *corev1.Secret {
@@ -394,7 +394,7 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
 				IngressOperatorCloudCreds: corev1.LocalObjectReference{Name: powerVSResources.IngressOperatorCloudCreds.Name},
 			},
 		}
-		services = getIngressServicePublishingStrategyMapping(o.NetworkType)
+		services = getIngressServicePublishingStrategyMapping(o.NetworkType, o.ExternalDNSDomain != "")
 	default:
 		panic("no platform specified")
 	}
@@ -615,13 +615,17 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token
 	}
 }
 
-func getIngressServicePublishingStrategyMapping(netType hyperv1.NetworkType) []hyperv1.ServicePublishingStrategyMapping {
+func getIngressServicePublishingStrategyMapping(netType hyperv1.NetworkType, usesExternalDNS bool) []hyperv1.ServicePublishingStrategyMapping {
 
+	apiServiceStrategy := hyperv1.LoadBalancer
+	if usesExternalDNS {
+		apiServiceStrategy = hyperv1.Route
+	}
 	ret := []hyperv1.ServicePublishingStrategyMapping{
 		{
 			Service: hyperv1.APIServer,
 			ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{
-				Type: hyperv1.LoadBalancer,
+				Type: apiServiceStrategy,
 			},
 		},
 		{

api/v1alpha1/endpointservice_types.go

@@ -47,10 +47,14 @@ type AWSEndpointServiceStatus struct {
 	// +optional
 	EndpointID string `json:"endpointID,omitempty"`
 
-	// DNSName is the name for the record created in the hypershift private zone
+	// Deprecated: Use DNSNames instead
 	// +optional
 	DNSName string `json:"dnsName,omitempty"`
 
+	// DNSName are the names for the records created in the hypershift private zone
+	// +optional
+	DNSNames []string `json:"dnsNames,omitempty"`
+
 	// DNSZoneID is ID for the hypershift private zone
 	// +optional
 	DNSZoneID string `json:"dnsZoneID,omitempty"`

cmd/install/assets/hypershift-operator/hypershift.openshift.io_awsendpointservices.yaml

@@ -152,9 +152,14 @@ spec:
                   type: object
                 type: array
               dnsName:
-                description: DNSName is the name for the record created in the hypershift
-                  private zone
+                description: 'Deprecated: Use DNSNames instead'
                 type: string
+              dnsNames:
+                description: DNSName are the names for the records created in the
+                  hypershift private zone
+                items:
+                  type: string
+                type: array
               dnsZoneID:
                 description: DNSZoneID is ID for the hypershift private zone
                 type: string

control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go

@@ -473,12 +473,8 @@ func reconcileAWSEndpointService(ctx context.Context, awsEndpointService *hyperv
 		return nil
 	}
 
-	var recordName string
-	if awsEndpointService.Name == manifests.KubeAPIServerPrivateService("").Name {
-		recordName = "api"
-	} else if awsEndpointService.Name == manifests.PrivateRouterService("").Name {
-		recordName = "*.apps"
-	} else {
+	recordNames := recordsForService(awsEndpointService, hcp)
+	if len(recordNames) == 0 {
 		log.Info("WARNING: no mapping from AWSEndpointService to DNS")
 		return nil
 	}
@@ -489,19 +485,45 @@ func reconcileAWSEndpointService(ctx context.Context, awsEndpointService *hyperv
 		return err
 	}
 
-	fqdn := fmt.Sprintf("%s.%s", recordName, zoneName)
-	err = createRecord(ctx, route53Client, zoneID, fqdn, *(endpointDNSEntries[0].DnsName))
-	if err != nil {
-		return err
+	var fqdns []string
+	for _, recordName := range recordNames {
+		fqdn := fmt.Sprintf("%s.%s", recordName, zoneName)
+		fqdns = append(fqdns, fqdn)
+		err = createRecord(ctx, route53Client, zoneID, fqdn, *(endpointDNSEntries[0].DnsName))
+		if err != nil {
+			return err
+		}
+		log.Info("DNS record created", "fqdn", fqdn)
 	}
-	log.Info("DNS record created", "fqdn", fqdn)
 
-	awsEndpointService.Status.DNSName = fqdn
+	//lint:ignore SA1019 we reset the deprecated field precicely
+	// because it is deprecated.
+	awsEndpointService.Status.DNSName = ""
+	awsEndpointService.Status.DNSNames = fqdns
 	awsEndpointService.Status.DNSZoneID = zoneID
 
 	return nil
 }
 
+func recordsForService(awsEndpointService *hyperv1.AWSEndpointService, hcp *hyperv1.HostedControlPlane) []string {
+	if awsEndpointService.Name == manifests.KubeAPIServerPrivateService("").Name {
+		return []string{"api"}
+
+	}
+	if awsEndpointService.Name != manifests.PrivateRouterService("").Name {
+		return nil
+	}
+
+	// If the kas is exposed through a route, the router needs to have DNS entries for both
+	// the kas and the apps domain
+	if m := servicePublishingStrategyByType(hcp, hyperv1.APIServer); m != nil && m.Type == hyperv1.Route {
+		return []string{"api", "*.apps"}
+	}
+
+	return []string{"*.apps"}
+
+}
+
 func apiTagToEC2Tag(name string, in []hyperv1.AWSResourceTag) []*ec2.Tag {
 	result := make([]*ec2.Tag, len(in))
 	for _, val := range in {
@@ -538,28 +560,37 @@ func (r *AWSEndpointServiceReconciler) delete(ctx context.Context, awsEndpointSe
 		log.Info("endpoint deleted", "endpointID", endpointID)
 	}
 
-	fqdn := awsEndpointService.Status.DNSName
 	zoneID := awsEndpointService.Status.DNSZoneID
 	if err != nil {
 		return false, err
 	}
-	if fqdn != "" && zoneID != "" {
-		record, err := findRecord(ctx, route53Client, zoneID, fqdn)
-		if err != nil {
-			return false, err
-		}
-		if record != nil {
-			err = deleteRecord(ctx, route53Client, zoneID, record)
+
+	for _, fqdn := range awsEndpointService.Status.DNSNames {
+		if fqdn != "" && zoneID != "" {
+			record, err := findRecord(ctx, route53Client, zoneID, fqdn)
 			if err != nil {
 				return false, err
 			}
-			log.Info("DNS record deleted", "fqdn", fqdn)
-		} else {
-			log.Info("no DNS record found", "fqdn", fqdn)
+			if record != nil {
+				err = deleteRecord(ctx, route53Client, zoneID, record)
+				if err != nil {
+					return false, err
+				}
+				log.Info("DNS record deleted", "fqdn", fqdn)
+			} else {
+				log.Info("no DNS record found", "fqdn", fqdn)
+			}
 		}
-	} else {
-		log.Info("no DNS status set in AWSEndpointService", "name", awsEndpointService.Name)
 	}
 
 	return true, nil
 }
+
+func servicePublishingStrategyByType(hcp *hyperv1.HostedControlPlane, svcType hyperv1.ServiceType) *hyperv1.ServicePublishingStrategy {
+	for _, mapping := range hcp.Spec.Services {
+		if mapping.Service == svcType {
+			return &mapping.ServicePublishingStrategy
+		}
+	}
+	return nil
+}

control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller_test.go

@@ -3,6 +3,10 @@ package awsprivatelink
 import (
 	"reflect"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func Test_diffSubnetIDs(t *testing.T) {
@@ -77,3 +81,46 @@ func Test_diffSubnetIDs(t *testing.T) {
 		})
 	}
 }
+
+func TestRecordForService(t *testing.T) {
+	testCases := []struct {
+		name           string
+		in             *hyperv1.AWSEndpointService
+		serviceMapping []hyperv1.ServicePublishingStrategyMapping
+		expected       []string
+	}{
+		{
+			name: "Unknown service, no entry",
+			in:   &hyperv1.AWSEndpointService{ObjectMeta: metav1.ObjectMeta{Name: "unknown"}},
+		},
+		{
+			name:     "KAS service gets api entry",
+			in:       &hyperv1.AWSEndpointService{ObjectMeta: metav1.ObjectMeta{Name: "kube-apiserver-private"}},
+			expected: []string{"api"},
+		},
+		{
+			name: "Router service gets api and apps entry when kas is exposed through route",
+			in:   &hyperv1.AWSEndpointService{ObjectMeta: metav1.ObjectMeta{Name: "private-router"}},
+			serviceMapping: []hyperv1.ServicePublishingStrategyMapping{{
+				Service:                   hyperv1.APIServer,
+				ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{Type: hyperv1.Route},
+			}},
+			expected: []string{"api", "*.apps"},
+		},
+		{
+			name:     "Router service gets apps entry only when kas is not exposed through route",
+			in:       &hyperv1.AWSEndpointService{ObjectMeta: metav1.ObjectMeta{Name: "private-router"}},
+			expected: []string{"*.apps"},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			hcp := &hyperv1.HostedControlPlane{Spec: hyperv1.HostedControlPlaneSpec{Services: tc.serviceMapping}}
+			actual := recordsForService(tc.in, hcp)
+			if diff := cmp.Diff(actual, tc.expected); diff != "" {
+				t.Errorf("actual differs from expected: %s", diff)
+			}
+		})
+	}
+}

control-plane-operator/controllers/hostedcontrolplane/cno/clusternetworkoperator.go

@@ -98,7 +98,7 @@ func NewParams(hcp *hyperv1.HostedControlPlane, version string, images map[strin
 	p.DeploymentConfig.SetDefaultSecurityContext = setDefaultSecurityContext
 	if util.IsPrivateHCP(hcp) {
 		p.APIServerAddress = fmt.Sprintf("api.%s.hypershift.local", hcp.Name)
-		p.APIServerPort = 6443
+		p.APIServerPort = util.APIPortWithDefault(hcp, config.DefaultAPIServerPort)
 	} else {
 		p.APIServerAddress = hcp.Status.ControlPlaneEndpoint.Host
 		p.APIServerPort = hcp.Status.ControlPlaneEndpoint.Port