From 5f0aac5e4b6f7a22957b734955cc0ba796114ee6 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Thu, 21 Jul 2022 11:10:40 -0400 Subject: [PATCH 1/8] Expose through router --- api/fixtures/example.go | 18 +- api/v1alpha1/endpointservice_types.go | 6 +- api/v1alpha1/zz_generated.deepcopy.go | 5 + ...hift.openshift.io_awsendpointservices.yaml | 9 +- .../awsprivatelink_controller.go | 81 +- .../awsprivatelink_controller_test.go | 47 + .../hostedcontrolplane_controller.go | 120 ++- .../hostedcontrolplane_controller_test.go | 572 ++++++++++-- .../ignitionserver/ignitionserver.go | 12 +- .../hostedcontrolplane/ingress/router.go | 77 +- .../ingress/router.template | 839 ++++++++++++++++++ .../hostedcontrolplane/kas/service.go | 26 + .../konnectivity/reconcile.go | 2 + .../hostedcontrolplane/manifests/infra.go | 18 + .../hostedcontrolplane/manifests/ingress.go | 34 +- .../hostedcontrolplane/oauth/route.go | 9 +- hack/app-sre/saas_template.yaml | 9 +- .../controllers/platform/aws/controller.go | 3 +- support/testutil/testutil.go | 53 ++ support/testutil/testutil_test.go | 66 ++ test/e2e/chaos_test.go | 4 + test/e2e/control_plane_upgrade_test.go | 21 + test/e2e/util/fixture.go | 1 + test/e2e/util/util.go | 29 + 24 files changed, 1885 insertions(+), 176 deletions(-) create mode 100644 control-plane-operator/controllers/hostedcontrolplane/ingress/router.template create mode 100644 support/testutil/testutil_test.go diff --git a/api/fixtures/example.go b/api/fixtures/example.go index 895b5d03499c..7f7d7d23318e 100644 --- a/api/fixtures/example.go +++ b/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), } } @@ -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, }, }, { diff --git a/api/v1alpha1/endpointservice_types.go b/api/v1alpha1/endpointservice_types.go index f9ee2044a561..31c8fa71b797 100644 --- a/api/v1alpha1/endpointservice_types.go +++ b/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"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 422ca6c9bcb0..7b253f40d70b 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -202,6 +202,11 @@ func (in *AWSEndpointServiceSpec) DeepCopy() *AWSEndpointServiceSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AWSEndpointServiceStatus) DeepCopyInto(out *AWSEndpointServiceStatus) { *out = *in + if in.DNSNames != nil { + in, out := &in.DNSNames, &out.DNSNames + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) diff --git a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_awsendpointservices.yaml b/cmd/install/assets/hypershift-operator/hypershift.openshift.io_awsendpointservices.yaml index 4b3c7bf289b3..a728b0754024 100644 --- a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_awsendpointservices.yaml +++ b/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 diff --git a/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go b/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go index 97a232d605b2..3cbd4910324a 100644 --- a/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go +++ b/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 +} diff --git a/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller_test.go b/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller_test.go index 1005b2d2b50d..c81526c11c89 100644 --- a/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller_test.go +++ b/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) + } + }) + } +} diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 3c358921afcb..2379bef13809 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -732,16 +732,19 @@ func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControl return fmt.Errorf("failed to reconcile ingress operator: %w", err) } - // Reconcile private router + // Reconcile router + kasServiceStrategy := servicePublishingStrategyByType(hostedControlPlane, hyperv1.APIServer) if util.IsPrivateHCP(hostedControlPlane) { r.Log.Info("Removing private IngressController") // Ensure that if an ingress controller exists from a previous version, it is removed if err = r.reconcilePrivateIngressController(ctx, hostedControlPlane); err != nil { return fmt.Errorf("failed to reconcile private ingresscontroller: %w", err) } - r.Log.Info("Reconciling private router") - if err = r.reconcilePrivateRouter(ctx, hostedControlPlane, releaseImage, createOrUpdate); err != nil { - return fmt.Errorf("failed to reconcile private ingresscontroller: %w", err) + } + if util.IsPrivateHCP(hostedControlPlane) || kasServiceStrategy.Type == hyperv1.Route { + r.Log.Info("Reconciling router") + if err = r.reconcileRouter(ctx, hostedControlPlane, releaseImage, createOrUpdate); err != nil { + return fmt.Errorf("failed to reconcile router: %w", err) } } @@ -808,7 +811,7 @@ func (r *HostedControlPlaneReconciler) reconcileDefaultServiceAccount(ctx contex func (r *HostedControlPlaneReconciler) reconcileAPIServerService(ctx context.Context, hcp *hyperv1.HostedControlPlane, createOrUpdate upsert.CreateOrUpdateFN) error { serviceStrategy := servicePublishingStrategyByType(hcp, hyperv1.APIServer) if serviceStrategy == nil { - return fmt.Errorf("APIServer service strategy not specified") + return errors.New("APIServer service strategy not specified") } p := kas.NewKubeAPIServerServiceParams(hcp) apiServerService := manifests.KubeAPIServerService(hcp.Namespace) @@ -818,7 +821,33 @@ func (r *HostedControlPlaneReconciler) reconcileAPIServerService(ctx context.Con return fmt.Errorf("failed to reconcile API server service: %w", err) } - if util.IsPrivateHCP(hcp) { + if serviceStrategy.Type == hyperv1.Route { + if util.IsPublicHCP(hcp) { + externalRoute := manifests.KubeAPIServerExternalRoute(hcp.Namespace) + if _, err := createOrUpdate(ctx, r.Client, externalRoute, func() error { + kas.ReconcileRoute(externalRoute, serviceStrategy.Route.Hostname) + if externalRoute.Annotations == nil { + externalRoute.Annotations = map[string]string{} + } + externalRoute.Annotations["external-dns.alpha.kubernetes.io/hostname"] = serviceStrategy.Route.Hostname + return nil + }); err != nil { + return fmt.Errorf("failed to reconcile apiserver external route: %w", err) + } + } + + // We do not need to enumerate all possible addresses, because we use the KAS as default backend through a custom + // router template. That in turn was needed to work around SNI not supporting IP addresses, only hostnames: + // https://www.rfc-editor.org/rfc/rfc6066#section-3 + route := manifests.KubeAPIServerInternalRoute(hcp.Namespace) + if _, err := createOrUpdate(ctx, r.Client, route, func() error { + kas.ReconcileRoute(route, "kubernetes.default") + return nil + }); err != nil { + return fmt.Errorf("failed to reconcile apiserver route %s: %w", route.Name, err) + } + + } else if util.IsPrivateHCP(hcp) { apiServerPrivateService := manifests.KubeAPIServerPrivateService(hcp.Namespace) if _, err := createOrUpdate(ctx, r.Client, apiServerPrivateService, func() error { return kas.ReconcilePrivateService(apiServerPrivateService, p.OwnerReference) @@ -872,7 +901,7 @@ func (r *HostedControlPlaneReconciler) reconcileOAuthServerService(ctx context.C } oauthRoute := manifests.OauthServerRoute(hcp.Namespace) if _, err := createOrUpdate(ctx, r.Client, oauthRoute, func() error { - return oauth.ReconcileRoute(oauthRoute, p.OwnerRef, serviceStrategy, r.DefaultIngressDomain) + return oauth.ReconcileRoute(oauthRoute, p.OwnerRef, serviceStrategy, r.DefaultIngressDomain, hcp) }); err != nil { return fmt.Errorf("failed to reconcile OAuth route: %w", err) } @@ -2355,39 +2384,76 @@ func (r *HostedControlPlaneReconciler) reconcilePrivateIngressController(ctx con return nil } -func (r *HostedControlPlaneReconciler) reconcilePrivateRouter(ctx context.Context, hcp *hyperv1.HostedControlPlane, releaseInfo *releaseinfo.ReleaseImage, createOrUpdate upsert.CreateOrUpdateFN) error { - sa := manifests.PrivateRouterServiceAccount(hcp.Namespace) +func (r *HostedControlPlaneReconciler) reconcileRouter(ctx context.Context, hcp *hyperv1.HostedControlPlane, releaseInfo *releaseinfo.ReleaseImage, createOrUpdate upsert.CreateOrUpdateFN) error { + sa := manifests.RouterServiceAccount(hcp.Namespace) if _, err := createOrUpdate(ctx, r.Client, sa, func() error { - return ingress.ReconcilePrivateRouterServiceAccount(sa, config.OwnerRefFrom(hcp)) + return ingress.ReconcileRouterServiceAccount(sa, config.OwnerRefFrom(hcp)) }); err != nil { - return fmt.Errorf("failed to reconcile private router service account: %w", err) + return fmt.Errorf("failed to reconcile router service account: %w", err) } - role := manifests.PrivateRouterRole(hcp.Namespace) + role := manifests.RouterRole(hcp.Namespace) if _, err := createOrUpdate(ctx, r.Client, role, func() error { - return ingress.ReconcilePrivateRouterRole(role, config.OwnerRefFrom(hcp)) + return ingress.ReconcileRouterRole(role, config.OwnerRefFrom(hcp)) }); err != nil { - return fmt.Errorf("failed to reconcile private router role: %w", err) + return fmt.Errorf("failed to reconcile router role: %w", err) } - rb := manifests.PrivateRouterRoleBinding(hcp.Namespace) + rb := manifests.RouterRoleBinding(hcp.Namespace) if _, err := createOrUpdate(ctx, r.Client, rb, func() error { - return ingress.ReconcilePrivateRouterRoleBinding(rb, config.OwnerRefFrom(hcp)) + return ingress.ReconcileRouterRoleBinding(rb, config.OwnerRefFrom(hcp)) + }); err != nil { + return fmt.Errorf("failed to reconcile router rolebinding: %w", err) + } + + var canonicalHostname string + if util.IsPrivateHCP(hcp) { + svc := manifests.PrivateRouterService(hcp.Namespace) + if _, err := createOrUpdate(ctx, r.Client, svc, func() error { + return ingress.ReconcileRouterService(svc, config.OwnerRefFrom(hcp), util.APIPortWithDefault(hcp, config.DefaultAPIServerPort), true) + }); err != nil { + return fmt.Errorf("failed to reconcile router service: %w", err) + } + if !util.IsPublicHCP(hcp) && len(svc.Status.LoadBalancer.Ingress) > 0 { + canonicalHostname = svc.Status.LoadBalancer.Ingress[0].Hostname + } + } + + if util.IsPublicHCP(hcp) { + pubSvc := manifests.RouterPublicService(hcp.Namespace) + if _, err := createOrUpdate(ctx, r.Client, pubSvc, func() error { + return ingress.ReconcileRouterService(pubSvc, config.OwnerRefFrom(hcp), util.APIPortWithDefault(hcp, config.DefaultAPIServerPort), false) + }); err != nil { + return fmt.Errorf("failed to reconcile router service: %w", err) + } + if len(pubSvc.Status.LoadBalancer.Ingress) > 0 { + canonicalHostname = pubSvc.Status.LoadBalancer.Ingress[0].Hostname + } + } + + routerTemplate := manifests.RouterTemplateConfigMap(hcp.Namespace) + if _, err := createOrUpdate(ctx, r.Client, routerTemplate, func() error { + ingress.ReconcileRouterTemplateConfigmap(routerTemplate) + return nil }); err != nil { - return fmt.Errorf("failed to reconcile private router rolebinding: %w", err) + return fmt.Errorf("failed to reconcile router template configmap: %w", err) } - deployment := manifests.PrivateRouterDeployment(hcp.Namespace) + + // We have to wait for the LB to be ready, otherwise the router might already admit the KAS route but not populate + // the routerCanonicalHostname field, causing external DNS to not create the DNS entry. It doesn't add that field + // later for already-admitted routes. + if canonicalHostname == "" { + return nil + } + + deployment := manifests.RouterDeployment(hcp.Namespace) if _, err := createOrUpdate(ctx, r.Client, deployment, func() error { - return ingress.ReconcilePrivateRouterDeployment(deployment, + return ingress.ReconcileRouterDeployment(deployment, config.OwnerRefFrom(hcp), ingress.PrivateRouterConfig(hcp, r.SetDefaultSecurityContext), - ingress.PrivateRouterImage(releaseInfo.ComponentImages())) - }); err != nil { - return fmt.Errorf("failed to reconcile private router deployment: %w", err) - } - svc := manifests.PrivateRouterService(hcp.Namespace) - if _, err := createOrUpdate(ctx, r.Client, svc, func() error { - return ingress.ReconcilePrivateRouterService(svc, config.OwnerRefFrom(hcp)) + ingress.PrivateRouterImage(releaseInfo.ComponentImages()), + canonicalHostname, + ) }); err != nil { - return fmt.Errorf("failed to reconcile private router service: %w", err) + return fmt.Errorf("failed to reconcile router deployment: %w", err) } return nil } diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go index 78aa0eea9d7d..7a315140eef5 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go @@ -7,14 +7,20 @@ import ( "github.com/go-logr/zapr" . "github.com/onsi/gomega" + imagev1 "github.com/openshift/api/image/v1" + routev1 "github.com/openshift/api/route/v1" + "github.com/openshift/hypershift/api" hyperv1 "github.com/openshift/hypershift/api/v1alpha1" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/autoscaler" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/common" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/ingress" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" - "github.com/openshift/hypershift/support/api" fakecapabilities "github.com/openshift/hypershift/support/capabilities/fake" + "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/globalconfig" + "github.com/openshift/hypershift/support/releaseinfo" fakereleaseprovider "github.com/openshift/hypershift/support/releaseinfo/fake" + "github.com/openshift/hypershift/support/testutil" "go.uber.org/zap/zaptest" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -139,15 +145,229 @@ func TestReconcileAPIServerService(t *testing.T) { hostname := "test.example.com" allowCIDR := []hyperv1.CIDRBlock{"1.2.3.4/24"} allowCIDRString := []string{"1.2.3.4/24"} + + ownerRef := metav1.OwnerReference{ + APIVersion: "hypershift.openshift.io/v1alpha1", + Kind: "HostedControlPlane", + Name: "test", + Controller: pointer.Bool(true), + BlockOwnerDeletion: pointer.Bool(true), + } + kasPublicService := func(m ...func(*corev1.Service)) corev1.Service { + svc := corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: targetNamespace, + Name: manifests.KubeAPIServerService(targetNamespace).Name, + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-type": "nlb", + hyperv1.ExternalDNSHostnameAnnotation: hostname, + }, + Labels: map[string]string{ + "app": "kube-apiserver", + "hypershift.openshift.io/control-plane-component": "kube-apiserver", + }, + OwnerReferences: []metav1.OwnerReference{ownerRef}, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Ports: []corev1.ServicePort{ + { + Protocol: corev1.ProtocolTCP, + Port: apiPort, + TargetPort: intstr.FromInt(int(apiPort)), + }, + }, + LoadBalancerSourceRanges: allowCIDRString, + Selector: map[string]string{ + "app": "kube-apiserver", + "hypershift.openshift.io/control-plane-component": "kube-apiserver", + }, + }, + } + for _, m := range m { + m(&svc) + } + return svc + } + kasPrivateService := func(m ...func(*corev1.Service)) corev1.Service { + return kasPublicService(append(m, func(s *corev1.Service) { + s.Name = manifests.KubeAPIServerPrivateService(targetNamespace).Name + + delete(s.Annotations, hyperv1.ExternalDNSHostnameAnnotation) + s.Annotations["service.beta.kubernetes.io/aws-load-balancer-internal"] = "true" + + s.Labels = nil + + s.Spec.LoadBalancerSourceRanges = nil + + s.Spec.Ports[0].Port = 6443 + s.Spec.Ports[0].TargetPort = intstr.FromInt(6443) + })...) + } + kasPublicRoute := routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: targetNamespace, + Name: "kube-apiserver", + Labels: map[string]string{ + "hypershift.openshift.io/hosted-control-plane": targetNamespace, + }, + Annotations: map[string]string{ + "external-dns.alpha.kubernetes.io/hostname": hostname, + }, + }, + Spec: routev1.RouteSpec{ + Host: hostname, + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: manifests.KubeAPIServerService("").Name, + }, + TLS: &routev1.TLSConfig{ + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + Termination: routev1.TLSTerminationPassthrough, + }, + }, + } + kasInternalRoute := routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: targetNamespace, + Name: "kube-apiserver-internal", + Labels: map[string]string{ + "hypershift.openshift.io/hosted-control-plane": targetNamespace, + }, + }, + Spec: routev1.RouteSpec{ + Host: "kubernetes.default", + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: manifests.KubeAPIServerService("").Name, + }, + TLS: &routev1.TLSConfig{ + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + Termination: routev1.TLSTerminationPassthrough, + }, + }, + } testsCases := []struct { - name string - hcp *hyperv1.HostedControlPlane - expectedServices []*corev1.Service + name string + endpointAccess hyperv1.AWSEndpointAccessType + apiPublishingStrategy hyperv1.ServicePublishingStrategy + + expectedServices []corev1.Service + expectedRoutes []routev1.Route }{ { - name: "EndpointAccess PublicAndPrivate, ServicePublishingStrategy LoadBalancer, hostname, custom port, and allowed CIDR blocks", - hcp: &hyperv1.HostedControlPlane{ - TypeMeta: metav1.TypeMeta{}, + name: "LB strategy, public", + endpointAccess: hyperv1.Public, + apiPublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + Hostname: hostname, + }, + }, + + expectedServices: []corev1.Service{ + kasPublicService(), + }, + }, + { + name: "LB strategy, publicPrivate", + endpointAccess: hyperv1.PublicAndPrivate, + apiPublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + Hostname: hostname, + }, + }, + + expectedServices: []corev1.Service{ + kasPublicService(), + kasPrivateService(), + }, + }, + { + name: "LB strategy, private", + endpointAccess: hyperv1.Private, + apiPublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + Hostname: hostname, + }, + }, + + expectedServices: []corev1.Service{ + kasPublicService(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeClusterIP + delete(s.Annotations, "external-dns.alpha.kubernetes.io/hostname") + }), + kasPrivateService(), + }, + }, + { + name: "Route strategy, public", + endpointAccess: hyperv1.Public, + apiPublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: hostname, + }, + }, + + expectedServices: []corev1.Service{ + kasPublicService(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeClusterIP + delete(s.Annotations, "external-dns.alpha.kubernetes.io/hostname") + }), + }, + expectedRoutes: []routev1.Route{ + kasPublicRoute, + kasInternalRoute, + }, + }, + { + name: "Route strategy, publicPrivate", + endpointAccess: hyperv1.PublicAndPrivate, + apiPublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: hostname, + }, + }, + + expectedServices: []corev1.Service{ + kasPublicService(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeClusterIP + delete(s.Annotations, "external-dns.alpha.kubernetes.io/hostname") + }), + }, + expectedRoutes: []routev1.Route{ + kasPublicRoute, + kasInternalRoute, + }, + }, + { + name: "Route strategy, private", + endpointAccess: hyperv1.Private, + apiPublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + Route: &hyperv1.RoutePublishingStrategy{ + Hostname: hostname, + }, + }, + + expectedServices: []corev1.Service{ + kasPublicService(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeClusterIP + delete(s.Annotations, "external-dns.alpha.kubernetes.io/hostname") + }), + }, + expectedRoutes: []routev1.Route{ + kasInternalRoute, + }, + }, + } + for _, tc := range testsCases { + t.Run(tc.name, func(t *testing.T) { + hcp := &hyperv1.HostedControlPlane{ ObjectMeta: metav1.ObjectMeta{ Namespace: targetNamespace, Name: "test", @@ -162,88 +382,43 @@ func TestReconcileAPIServerService(t *testing.T) { Platform: hyperv1.PlatformSpec{ Type: hyperv1.AWSPlatform, AWS: &hyperv1.AWSPlatformSpec{ - EndpointAccess: hyperv1.PublicAndPrivate, - }, - }, - Services: []hyperv1.ServicePublishingStrategyMapping{ - { - Service: hyperv1.APIServer, - ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ - Type: hyperv1.LoadBalancer, - LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ - Hostname: hostname, - }, - }, - }, - }, - }, - }, - expectedServices: []*corev1.Service{ - { - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Namespace: targetNamespace, - Name: manifests.KubeAPIServerService(targetNamespace).Name, - Annotations: map[string]string{ - "service.beta.kubernetes.io/aws-load-balancer-type": "nlb", - hyperv1.ExternalDNSHostnameAnnotation: hostname, - }, - }, - Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeLoadBalancer, - Ports: []corev1.ServicePort{ - { - Protocol: corev1.ProtocolTCP, - Port: apiPort, - TargetPort: intstr.FromInt(int(apiPort)), - }, - }, - LoadBalancerSourceRanges: allowCIDRString, - }, - }, - { - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Namespace: targetNamespace, - Name: manifests.KubeAPIServerPrivateService(targetNamespace).Name, - Annotations: map[string]string{ - "service.beta.kubernetes.io/aws-load-balancer-type": "nlb", - "service.beta.kubernetes.io/aws-load-balancer-internal": "true", - }, - }, - Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeLoadBalancer, - Ports: []corev1.ServicePort{ - { - Protocol: corev1.ProtocolTCP, - Port: 6443, - TargetPort: intstr.FromInt(6443), - }, + EndpointAccess: tc.endpointAccess, }, }, + Services: []hyperv1.ServicePublishingStrategyMapping{{ + Service: hyperv1.APIServer, + ServicePublishingStrategy: tc.apiPublishingStrategy, + }}, }, - }, - }, - } - for _, tc := range testsCases { - t.Run(tc.name, func(t *testing.T) { - g := NewGomegaWithT(t) + } - fakeClient := fake.NewClientBuilder().Build() + ctx := ctrl.LoggerInto(context.Background(), zapr.NewLogger(zaptest.NewLogger(t))) + + fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).Build() r := &HostedControlPlaneReconciler{ Client: fakeClient, - Log: ctrl.LoggerFrom(context.TODO()), + Log: ctrl.LoggerFrom(ctx), } - err := r.reconcileAPIServerService(context.Background(), tc.hcp, controllerutil.CreateOrUpdate) - g.Expect(err).NotTo(HaveOccurred()) - var actualService corev1.Service - for _, expectedService := range tc.expectedServices { - err = r.Get(context.Background(), client.ObjectKeyFromObject(expectedService), &actualService) - g.Expect(err).NotTo(HaveOccurred()) - actualService.Spec.Selector = nil - g.Expect(actualService.Spec).To(Equal(expectedService.Spec)) - g.Expect(actualService.Annotations).To(Equal(expectedService.Annotations)) + if err := r.reconcileAPIServerService(ctx, hcp, controllerutil.CreateOrUpdate); err != nil { + t.Fatalf("reconcileAPIServerService failed: %v", err) + } + + var actualServices corev1.ServiceList + if err := fakeClient.List(ctx, &actualServices); err != nil { + t.Fatalf("failed to list services: %v", err) + } + + if diff := testutil.MarshalYamlAndDiff(&actualServices, &corev1.ServiceList{Items: tc.expectedServices}, t); diff != "" { + t.Errorf("actual services differ from expected: %s", diff) + } + + var actualRoutes routev1.RouteList + if err := fakeClient.List(ctx, &actualRoutes); err != nil { + t.Fatalf("failed to list routes: %v", err) + } + if diff := testutil.MarshalYamlAndDiff(&actualRoutes, &routev1.RouteList{Items: tc.expectedRoutes}, t); diff != "" { + t.Errorf("actual routes differ from expected: %s", diff) } }) } @@ -751,3 +926,238 @@ type createTrackingWorkqueue struct { func (c *createTrackingWorkqueue) Add(item interface{}) { c.items = append(c.items, item.(reconcile.Request)) } + +func TestReconcileRouter(t *testing.T) { + t.Parallel() + + const namespace = "test" + + publicService := func(m ...func(*corev1.Service)) *corev1.Service { + svc := corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "router", + Namespace: namespace, + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-type": "nlb", + }, + Labels: map[string]string{"app": "private-router"}, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Selector: map[string]string{"app": "private-router"}, + Ports: []corev1.ServicePort{ + {Name: "http", Port: 80, TargetPort: intstr.FromString("http"), Protocol: corev1.ProtocolTCP}, + {Name: "https", Port: 443, TargetPort: intstr.FromString("https"), Protocol: corev1.ProtocolTCP}, + {Name: "kube-apiserver", Port: 6443, TargetPort: intstr.FromString("https"), Protocol: corev1.ProtocolTCP}, + }, + }, + } + + for _, m := range m { + m(&svc) + } + return &svc + } + privateService := func(m ...func(*corev1.Service)) *corev1.Service { + return publicService(append(m, func(s *corev1.Service) { + s.Name = "private-router" + s.Annotations["service.beta.kubernetes.io/aws-load-balancer-internal"] = "true" + })...) + } + testCases := []struct { + name string + endpointAccess hyperv1.AWSEndpointAccessType + existingObjects []client.Object + expectedServices []corev1.Service + expectedDeploynments []appsv1.Deployment + }{ + { + name: "Public HCP gets public LB ony", + endpointAccess: hyperv1.Public, + expectedServices: []corev1.Service{ + *publicService(), + }, + }, + { + name: "PublicPrivate gets public and private LB", + endpointAccess: hyperv1.PublicAndPrivate, + expectedServices: []corev1.Service{ + *privateService(), + *publicService(), + }, + }, + { + name: "Private gets private LB only", + endpointAccess: hyperv1.Private, + expectedServices: []corev1.Service{ + *privateService(), + }, + }, + { + name: "Public HCP, deployment is created when service has Ingress hostname set", + endpointAccess: hyperv1.Public, + existingObjects: []client.Object{publicService(func(s *corev1.Service) { + s.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: "a27252241e22343d4a704f1ca560e4aa-9ab9cf5317a99da5.elb.ca-central-1.amazonaws.com", + }} + })}, + expectedServices: []corev1.Service{ + *publicService(func(s *corev1.Service) { + s.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: "a27252241e22343d4a704f1ca560e4aa-9ab9cf5317a99da5.elb.ca-central-1.amazonaws.com", + }} + }), + }, + expectedDeploynments: []appsv1.Deployment{ + func() appsv1.Deployment { + dep := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "router", + }} + ingress.ReconcileRouterDeployment(dep, + config.OwnerRefFrom(&hyperv1.HostedControlPlane{ObjectMeta: metav1.ObjectMeta{ + Name: "hcp", + Namespace: namespace, + }}), + ingress.PrivateRouterConfig(&hyperv1.HostedControlPlane{ObjectMeta: metav1.ObjectMeta{Namespace: namespace}}, false), + "", + "a27252241e22343d4a704f1ca560e4aa-9ab9cf5317a99da5.elb.ca-central-1.amazonaws.com", + ) + + return *dep + }(), + }, + }, + { + name: "PublicPrivate HCP, deployment gets hostname from public service", + endpointAccess: hyperv1.PublicAndPrivate, + existingObjects: []client.Object{ + publicService(func(s *corev1.Service) { + s.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: "a27252241e22343d4a704f1ca560e4aa-9ab9cf5317a99da5.elb.ca-central-1.amazonaws.com", + }} + }), + privateService(func(s *corev1.Service) { + s.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: "private-lb", + }} + }), + }, + expectedServices: []corev1.Service{ + *privateService(func(s *corev1.Service) { + s.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: "private-lb", + }} + }), + *publicService(func(s *corev1.Service) { + s.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: "a27252241e22343d4a704f1ca560e4aa-9ab9cf5317a99da5.elb.ca-central-1.amazonaws.com", + }} + }), + }, + expectedDeploynments: []appsv1.Deployment{ + func() appsv1.Deployment { + dep := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "router", + }} + ingress.ReconcileRouterDeployment(dep, + config.OwnerRefFrom(&hyperv1.HostedControlPlane{ObjectMeta: metav1.ObjectMeta{ + Name: "hcp", + Namespace: namespace, + }}), + ingress.PrivateRouterConfig(&hyperv1.HostedControlPlane{ObjectMeta: metav1.ObjectMeta{Namespace: namespace}}, false), + "", + "a27252241e22343d4a704f1ca560e4aa-9ab9cf5317a99da5.elb.ca-central-1.amazonaws.com", + ) + + return *dep + }(), + }, + }, + { + name: "Private HCP, deployment gets hostname from private service", + endpointAccess: hyperv1.Private, + existingObjects: []client.Object{ + privateService(func(s *corev1.Service) { + s.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: "private-lb", + }} + }), + }, + expectedServices: []corev1.Service{ + *privateService(func(s *corev1.Service) { + s.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{ + Hostname: "private-lb", + }} + }), + }, + expectedDeploynments: []appsv1.Deployment{ + func() appsv1.Deployment { + dep := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "router", + }} + ingress.ReconcileRouterDeployment(dep, + config.OwnerRefFrom(&hyperv1.HostedControlPlane{ObjectMeta: metav1.ObjectMeta{ + Name: "hcp", + Namespace: namespace, + }}), + ingress.PrivateRouterConfig(&hyperv1.HostedControlPlane{ObjectMeta: metav1.ObjectMeta{Namespace: namespace}}, false), + "", + "private-lb", + ) + + return *dep + }(), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hcp", + Namespace: namespace, + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + EndpointAccess: tc.endpointAccess, + }, + }, + }, + } + + ctx := ctrl.LoggerInto(context.Background(), zapr.NewLogger(zaptest.NewLogger(t))) + client := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(append(tc.existingObjects, hcp)...).Build() + + r := HostedControlPlaneReconciler{ + Client: client, + Log: ctrl.LoggerFrom(ctx), + } + + if err := r.reconcileRouter(ctx, hcp, &releaseinfo.ReleaseImage{ImageStream: &imagev1.ImageStream{}}, controllerutil.CreateOrUpdate); err != nil { + t.Fatalf("reconcileRouter failed: %v", err) + } + + var services corev1.ServiceList + if err := client.List(ctx, &services); err != nil { + t.Fatalf("failed to list services: %v", err) + } + if diff := testutil.MarshalYamlAndDiff(&services, &corev1.ServiceList{Items: tc.expectedServices}, t); diff != "" { + t.Errorf("actual services differ from expected: %s", diff) + } + + var deployments appsv1.DeploymentList + if err := client.List(ctx, &deployments); err != nil { + t.Fatalf("failed to list deployments: %v", err) + } + if diff := testutil.MarshalYamlAndDiff(&deployments, &appsv1.DeploymentList{Items: tc.expectedDeploynments}, t); diff != "" { + t.Errorf("actual deployments differ from expected: %s", diff) + } + }) + } +} diff --git a/control-plane-operator/controllers/hostedcontrolplane/ignitionserver/ignitionserver.go b/control-plane-operator/controllers/hostedcontrolplane/ignitionserver/ignitionserver.go index 4ce8eaebe840..543394a33498 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/ignitionserver/ignitionserver.go +++ b/control-plane-operator/controllers/hostedcontrolplane/ignitionserver/ignitionserver.go @@ -10,6 +10,7 @@ import ( routev1 "github.com/openshift/api/route/v1" hyperv1 "github.com/openshift/hypershift/api/v1alpha1" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/ingress" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests/controlplaneoperator" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests/ignitionserver" hyperutil "github.com/openshift/hypershift/hypershift-operator/controllers/util" @@ -67,8 +68,10 @@ func ReconcileIgnitionServer(ctx context.Context, switch { case !util.ConnectsThroughInternetToControlplane(hcp.Spec.Platform): ignitionServerRoute.Spec.Host = fmt.Sprintf("%s.apps.%s.hypershift.local", ignitionServerRoute.Name, hcp.Name) + ingress.AddRouteLabel(ignitionServerRoute) case serviceStrategy.Route != nil && serviceStrategy.Route.Hostname != "": ignitionServerRoute.Spec.Host = serviceStrategy.Route.Hostname + ingress.AddRouteLabel(ignitionServerRoute) default: ignitionServerRoute.Spec.Host = util.ShortenRouteHostnameIfNeeded(ignitionServerRoute.Name, ignitionServerRoute.Namespace, defaultIngressDomain) } @@ -77,14 +80,7 @@ func ReconcileIgnitionServer(ctx context.Context, if ignitionServerRoute.Annotations == nil { ignitionServerRoute.Annotations = map[string]string{} } - if hcp.Spec.Platform.Type == hyperv1.AWSPlatform && - (hcp.Spec.Platform.AWS.EndpointAccess == hyperv1.PublicAndPrivate || - hcp.Spec.Platform.AWS.EndpointAccess == hyperv1.Private) { - if ignitionServerRoute.Labels == nil { - ignitionServerRoute.Labels = map[string]string{} - } - ignitionServerRoute.Labels[hyperutil.HypershiftRouteLabel] = controlPlaneNamespace - } else if serviceStrategy.Route != nil && serviceStrategy.Route.Hostname != "" { + if serviceStrategy.Route != nil && serviceStrategy.Route.Hostname != "" { ignitionServerRoute.ObjectMeta.Annotations[hyperv1.ExternalDNSHostnameAnnotation] = serviceStrategy.Route.Hostname } ignitionServerRoute.Spec.TLS = &routev1.TLSConfig{ diff --git a/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go b/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go index 442f28deb285..241812447d8f 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go +++ b/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go @@ -1,6 +1,8 @@ package ingress import ( + "bytes" + _ "embed" "fmt" appsv1 "k8s.io/api/apps/v1" @@ -16,6 +18,7 @@ import ( "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/util" + crclient "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -92,7 +95,19 @@ func PrivateRouterImage(images map[string]string) string { return images["haproxy-router"] } -func ReconcilePrivateRouterDeployment(deployment *appsv1.Deployment, ownerRef config.OwnerRef, deploymentConfig config.DeploymentConfig, image string) error { +const ( + routerTemplateConfigMapKey = "haproxy-config.template" + routerTemplateVolumeName = "happroxy-config" +) + +func ReconcileRouterTemplateConfigmap(cm *corev1.ConfigMap) { + if cm.Data == nil { + cm.Data = map[string]string{} + } + cm.Data[routerTemplateConfigMapKey] = string(bytes.Replace(routerTemplate, []byte(`<>`), []byte(cm.Namespace), 1)) +} + +func ReconcileRouterDeployment(deployment *appsv1.Deployment, ownerRef config.OwnerRef, deploymentConfig config.DeploymentConfig, image string, canonicalHostname string) error { deployment.Spec = appsv1.DeploymentSpec{ Selector: &metav1.LabelSelector{ MatchLabels: privateRouterLabels(), @@ -103,9 +118,10 @@ func ReconcilePrivateRouterDeployment(deployment *appsv1.Deployment, ownerRef co }, Spec: corev1.PodSpec{ Containers: []corev1.Container{ - util.BuildContainer(privateRouterContainerMain(), buildPrivateRouterContainerMain(image, deployment.Namespace)), + util.BuildContainer(privateRouterContainerMain(), buildPrivateRouterContainerMain(image, deployment.Namespace, canonicalHostname)), }, - ServiceAccountName: manifests.PrivateRouterServiceAccount("").Name, + ServiceAccountName: manifests.RouterServiceAccount("").Name, + Volumes: []corev1.Volume{{Name: routerTemplateVolumeName, VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{LocalObjectReference: corev1.LocalObjectReference{Name: manifests.RouterTemplateConfigMap("").Name}}}}}, }, }, } @@ -122,7 +138,8 @@ func privateRouterContainerMain() *corev1.Container { } } -func buildPrivateRouterContainerMain(image, namespace string) func(*corev1.Container) { +func buildPrivateRouterContainerMain(image, namespace, canonicalHostname string) func(*corev1.Container) { + const haproxyTemplateMountPath = "/usr/local/haproxy/hypershift-template" return func(c *corev1.Container) { c.Env = []corev1.EnvVar{ { @@ -133,6 +150,10 @@ func buildPrivateRouterContainerMain(image, namespace string) func(*corev1.Conta Name: "ROUTER_ALLOW_WILDCARD_ROUTES", Value: "false", }, + { + Name: "ROUTER_CANONICAL_HOSTNAME", + Value: canonicalHostname, + }, { Name: "ROUTER_CIPHERS", Value: "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384", @@ -159,7 +180,7 @@ func buildPrivateRouterContainerMain(image, namespace string) func(*corev1.Conta }, { Name: "ROUTER_SERVICE_NAME", - Value: manifests.PrivateRouterService("").Name, + Value: manifests.RouterPublicService("").Name, }, { Name: "ROUTER_SERVICE_NAMESPACE", @@ -201,6 +222,7 @@ func buildPrivateRouterContainerMain(image, namespace string) func(*corev1.Conta c.Image = image c.Args = []string{ "--namespace", namespace, + "--template=" + haproxyTemplateMountPath + "/" + routerTemplateConfigMapKey, } c.StartupProbe = &corev1.Probe{ FailureThreshold: 120, @@ -232,13 +254,16 @@ func buildPrivateRouterContainerMain(image, namespace string) func(*corev1.Conta Protocol: corev1.ProtocolTCP, }, } + c.VolumeMounts = []corev1.VolumeMount{ + {Name: routerTemplateVolumeName, MountPath: haproxyTemplateMountPath}, + } // Needed for the router pods to work: https://github.com/openshift/cluster-ingress-operator/blob/649fe5dfe2c6f795651592a045be901b00a1f93a/assets/router/deployment.yaml#L22-L23 c.SecurityContext = &corev1.SecurityContext{AllowPrivilegeEscalation: utilpointer.Bool(true)} } } -func ReconcilePrivateRouterRole(role *rbacv1.Role, ownerRef config.OwnerRef) error { +func ReconcileRouterRole(role *rbacv1.Role, ownerRef config.OwnerRef) error { ownerRef.ApplyTo(role) role.Rules = []rbacv1.PolicyRule{ { @@ -299,34 +324,36 @@ func ReconcilePrivateRouterRole(role *rbacv1.Role, ownerRef config.OwnerRef) err return nil } -func ReconcilePrivateRouterRoleBinding(rb *rbacv1.RoleBinding, ownerRef config.OwnerRef) error { +func ReconcileRouterRoleBinding(rb *rbacv1.RoleBinding, ownerRef config.OwnerRef) error { ownerRef.ApplyTo(rb) rb.Subjects = []rbacv1.Subject{ { Kind: "ServiceAccount", - Name: manifests.PrivateRouterServiceAccount("").Name, + Name: manifests.RouterServiceAccount("").Name, }, } rb.RoleRef = rbacv1.RoleRef{ APIGroup: rbacv1.SchemeGroupVersion.Group, Kind: "Role", - Name: manifests.PrivateRouterRole("").Name, + Name: manifests.RouterRole("").Name, } return nil } -func ReconcilePrivateRouterServiceAccount(sa *corev1.ServiceAccount, ownerRef config.OwnerRef) error { +func ReconcileRouterServiceAccount(sa *corev1.ServiceAccount, ownerRef config.OwnerRef) error { ownerRef.ApplyTo(sa) util.EnsurePullSecret(sa, common.PullSecret("").Name) return nil } -func ReconcilePrivateRouterService(svc *corev1.Service, ownerRef config.OwnerRef) error { +func ReconcileRouterService(svc *corev1.Service, ownerRef config.OwnerRef, kasPort int32, internal bool) error { if svc.Annotations == nil { svc.Annotations = map[string]string{} } svc.Annotations["service.beta.kubernetes.io/aws-load-balancer-type"] = "nlb" - svc.Annotations["service.beta.kubernetes.io/aws-load-balancer-internal"] = "true" + if internal { + svc.Annotations["service.beta.kubernetes.io/aws-load-balancer-internal"] = "true" + } if svc.Labels == nil { svc.Labels = map[string]string{} @@ -338,6 +365,7 @@ func ReconcilePrivateRouterService(svc *corev1.Service, ownerRef config.OwnerRef svc.Spec.Selector = privateRouterLabels() foundHTTP := false foundHTTPS := false + foundKAS := false for i, port := range svc.Spec.Ports { switch port.Name { case "http": @@ -350,6 +378,11 @@ func ReconcilePrivateRouterService(svc *corev1.Service, ownerRef config.OwnerRef svc.Spec.Ports[i].TargetPort = intstr.FromString("https") svc.Spec.Ports[i].Protocol = corev1.ProtocolTCP foundHTTPS = true + case "kube-apiserver": + svc.Spec.Ports[i].Port = kasPort + svc.Spec.Ports[i].TargetPort = intstr.FromString("https") + svc.Spec.Ports[i].Protocol = corev1.ProtocolTCP + foundKAS = true } } if !foundHTTP { @@ -369,5 +402,25 @@ func ReconcilePrivateRouterService(svc *corev1.Service, ownerRef config.OwnerRef Protocol: corev1.ProtocolTCP, }) } + if !foundKAS { + svc.Spec.Ports = append(svc.Spec.Ports, corev1.ServicePort{ + Name: "kube-apiserver", + Port: kasPort, + TargetPort: intstr.FromString("https"), + Protocol: corev1.ProtocolTCP, + }) + } return nil } + +//go:embed router.template +var routerTemplate []byte + +func AddRouteLabel(target crclient.Object) { + labels := target.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels[HypershiftRouteLabel] = target.GetNamespace() + target.SetLabels(labels) +} diff --git a/control-plane-operator/controllers/hostedcontrolplane/ingress/router.template b/control-plane-operator/controllers/hostedcontrolplane/ingress/router.template new file mode 100644 index 000000000000..5545ffbd2089 --- /dev/null +++ b/control-plane-operator/controllers/hostedcontrolplane/ingress/router.template @@ -0,0 +1,839 @@ +{{/* Upstream diff is surrounded by HYPERSHIFT CHANGE comments, other than that this template is identical to the one at https://github.com/openshift/router/blob/601ba575b7fadc1e05e7b4e2f35a660859db9788/images/router/haproxy/conf/haproxy-config.template */}} +{{/* + haproxy-config.cfg: contains the main config with helper backends that are used to terminate + encryption before finally sending to a host_be which is the backend that is the final + backend for a route and contains all the endpoints for the service +*/}} +{{- define "conf/haproxy.config" }} +{{- $workingDir := .WorkingDir }} +{{- $defaultDestinationCA := .DefaultDestinationCA }} +{{- $dynamicConfigManager := .DynamicConfigManager }} +{{- $router_ip_v4_v6_mode := env "ROUTER_IP_V4_V6_MODE" "v4" }} +{{- $router_disable_http2 := env "ROUTER_DISABLE_HTTP2" "false" }} + + +{{- /* A bunch of regular expressions. Each should be wrapped in (?:) so that it is safe to include bare */}} +{{- /* quadPattern: Match a quad in an IP address; e.g. 123 */}} +{{- $quadPattern := `(?:[0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])` -}} + +{{- /* cookie name pattern: */}} +{{- $cookieNamePattern := `[a-zA-Z0-9_-]+` -}} + +{{- /* balanceAlgoPattern matches valid options for the haproxy.router.openshift.io/balance annotation. */}} +{{- $balanceAlgoPattern := "roundrobin|leastconn|source|random" -}} + +{{- $timeSpecPattern := `[1-9][0-9]*(us|ms|s|m|h|d)?` }} + +{{- /* hsts header in response: */}} +{{- /* Not fully compliant to RFC6797#6.1 yet: has to accept not conformant directives */}} +{{- $hstsOptionalTokenPattern := `(?:includeSubDomains|preload)` }} +{{- $hstsPattern := printf `(?i)(?:%[1]s\s*[;]\s*)*max-age\s*=\s*(?:\d+|"\d+")(?:\s*[;]\s*%[1]s)*` $hstsOptionalTokenPattern -}} + +{{- /* setForwardedHeadersPattern matches valid options for how and when Forwarded: and X-Forwarded-*: headers are set. */}} +{{- $setForwardedHeadersPattern := `(?:append|replace|if-none|never)` -}} + +{{- /* Route-Specific Annotations */}} +{{- /* setForwardedHeadersAnnotation configures how Forwarded: and X-Forwarded-*: headers are set. */}} +{{- $setForwardedHeadersAnnotation := "haproxy.router.openshift.io/set-forwarded-headers" }} +{{- /* setForwardedHeadersDefaultValue is the default value if a route does not have the setForwardedHeadersAnnotation annotation. */}} +{{- $setForwardedHeadersDefaultValue := firstMatch $setForwardedHeadersPattern (env "ROUTER_SET_FORWARDED_HEADERS" "append") "append" -}} + +{{- /* pathRewriteTargetPattern: Match path rewrite-Target */}} +{{- $pathRewriteTargetPattern := `^/.*$` -}} + +global +{{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (env "ROUTER_HARD_STOP_AFTER")) }} + hard-stop-after {{ $value }} +{{- end }} +{{- with $value := env "ROUTER_MAX_CONNECTIONS" "20000" }} + {{- if isInteger $value }} + maxconn {{ $value }} + {{- end }} +{{- end }} +{{- $threads := env "ROUTER_THREADS" }} +{{- if ne "" (firstMatch "[1-9][0-9]*" $threads) }} + nbthread {{ $threads }} +{{- end }} + + + + daemon +{{- with (env "ROUTER_SYSLOG_ADDRESS") }} + log {{ . }} len {{ env "ROUTER_LOG_MAX_LENGTH" "1024" }} {{ env "ROUTER_LOG_FACILITY" "local1" }} {{ env "ROUTER_LOG_LEVEL" "warning" }} + log-send-hostname +{{- end }} + ca-base /etc/ssl + crt-base /etc/ssl + # TODO: Check if we can get reload to be faster by saving server state. + # server-state-file /var/lib/haproxy/run/haproxy.state + stats socket /var/lib/haproxy/run/haproxy.sock mode 600 level admin expose-fd listeners + stats timeout 2m + + # Increase the default request size to be comparable to modern cloud load balancers (ALB: 64kb), affects + # total memory use when large numbers of connections are open. + # In OCP 4.8, this value is adjustable via the IngressController API. + # Cluster administrators are still encouraged to use the default values provided below. + tune.maxrewrite {{ env "ROUTER_MAX_REWRITE_SIZE" "8192" }} + tune.bufsize {{ env "ROUTER_BUF_SIZE" "32768" }} + +{{- range $idx, $adjustment := .HTTPHeaderNameCaseAdjustments }} + h1-case-adjust {{ $adjustment.From }} {{ $adjustment.To }} +{{- end }} + + # Configure the TLS versions we support + ssl-default-bind-options ssl-min-ver {{ env "SSL_MIN_VERSION" "TLSv1.2" }} +{{- if ne (env "SSL_MAX_VERSION" "") "" }} ssl-max-ver {{env "SSL_MAX_VERSION" }}{{ end }} + +# The default cipher suite can be selected from the three sets recommended by https://wiki.mozilla.org/Security/Server_Side_TLS, +# or the user can provide one using the ROUTER_CIPHERS environment variable. +# By default when a cipher set is not provided, intermediate is used. + {{- if eq (env "ROUTER_CIPHERS" "intermediate") "modern" }} + # Modern cipher suite (no legacy browser support) from https://wiki.mozilla.org/Security/Server_Side_TLS + tune.ssl.default-dh-param 2048 + ssl-default-bind-ciphers ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256 + {{ else }} + + {{- if eq (env "ROUTER_CIPHERS" "intermediate") "intermediate" }} + # Intermediate cipher suite (default) from https://wiki.mozilla.org/Security/Server_Side_TLS + tune.ssl.default-dh-param 2048 + ssl-default-bind-ciphers ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS + {{ else }} + + {{- if eq (env "ROUTER_CIPHERS" "intermediate") "old" }} + + # Old cipher suite (maximum compatibility but insecure) from https://wiki.mozilla.org/Security/Server_Side_TLS + tune.ssl.default-dh-param 1024 + ssl-default-bind-ciphers ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:kEDH+AESGCM:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA:ECDHE-ECDSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA:DHE-RSA-AES256-SHA:ECDHE-RSA-DES-CBC3-SHA:ECDHE-ECDSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:AES:DES-CBC3-SHA:HIGH:SEED:!aNULL:!eNULL:!EXPORT:!DES:!RC4:!MD5:!PSK:!RSAPSK:!aDH:!aECDH:!EDH-DSS-DES-CBC3-SHA:!KRB5-DES-CBC3-SHA:!SRP + + {{- else }} + # user provided list of ciphers (Colon separated list as seen above) + # the env default is not used here since we can't get here with empty ROUTER_CIPHERS + tune.ssl.default-dh-param 2048 + ssl-default-bind-ciphers {{ env "ROUTER_CIPHERS" "ECDHE-ECDSA-CHACHA20-POLY1305" }} + {{- end }} + {{- end }} + {{- end }} + {{/* + The ssl-default-bind-ciphers option above configures ciphers for TLSv1.0, + TLSv1.1, and TLSv1.2; for TLSv1.3, cipher suites are configured using the + ssl-default-bind-ciphersuites option below. + */}} + {{- with $ciphersuites := (env "ROUTER_CIPHERSUITES") }} + ssl-default-bind-ciphersuites {{ $ciphersuites }} + {{- end }} + +defaults + {{- with $value := env "ROUTER_MAX_CONNECTIONS" "20000" }} + {{- if isInteger $value }} + maxconn {{ $value }} + {{- end }} + {{- end }} + + {{- if ne (env "ROUTER_SYSLOG_ADDRESS") "" }} + {{- if ne (env "ROUTER_SYSLOG_FORMAT") "" }} + log-format {{ env "ROUTER_SYSLOG_FORMAT" }} + {{- else }} + option httplog + {{- end }} + log global + {{- end }} + + # To configure custom default errors, you can either uncomment the + # line below (server ... 127.0.0.1:8080) and point it to your custom + # backend service or alternatively, you can send a custom 503 or 404 error. + # + # server openshift_backend 127.0.0.1:8080 + errorfile 503 {{ env "ROUTER_ERRORFILE_503" "/var/lib/haproxy/conf/error-page-503.http" }} + errorfile 404 {{ env "ROUTER_ERRORFILE_404" "/var/lib/haproxy/conf/error-page-404.http" }} + + timeout connect {{ firstMatch $timeSpecPattern (env "ROUTER_DEFAULT_CONNECT_TIMEOUT") "5s" }} + timeout client {{ firstMatch $timeSpecPattern (env "ROUTER_DEFAULT_CLIENT_TIMEOUT") "30s" }} + timeout client-fin {{ firstMatch $timeSpecPattern (env "ROUTER_CLIENT_FIN_TIMEOUT") "1s" }} + timeout server {{ firstMatch $timeSpecPattern (env "ROUTER_DEFAULT_SERVER_TIMEOUT") "30s" }} + timeout server-fin {{ firstMatch $timeSpecPattern (env "ROUTER_DEFAULT_SERVER_FIN_TIMEOUT") "1s" }} + timeout http-request {{ firstMatch $timeSpecPattern (env "ROUTER_SLOWLORIS_TIMEOUT") "10s" }} + timeout http-keep-alive {{ firstMatch $timeSpecPattern (env "ROUTER_SLOWLORIS_HTTP_KEEPALIVE") "300s" }} + + # Long timeout for WebSocket connections. + timeout tunnel {{ firstMatch $timeSpecPattern (env "ROUTER_DEFAULT_TUNNEL_TIMEOUT") "1h" }} + + {{- if isTrue (env "ROUTER_ENABLE_COMPRESSION") }} + compression algo gzip + compression type {{ env "ROUTER_COMPRESSION_MIME" "text/html text/plain text/css" }} + {{- end }} + + {{- if isTrue (env "ROUTER_DONT_LOG_NULL") }} + option dontlognull + {{- end }} + {{- if isTrue (env "ROUTER_HTTP_IGNORE_PROBES") }} + option http-ignore-probes + {{- end }} + {{- if .HTTPHeaderNameCaseAdjustments }} + option h1-case-adjust-bogus-client + {{- end }} + + {{ if (gt .StatsPort -1) }} +listen stats + bind :{{ if (gt .StatsPort 0) }}{{ .StatsPort }}{{ else }}1936{{ end }} + mode http + # Health check monitoring uri. + monitor-uri /healthz + + {{- if and (and (ne .StatsUser "") (ne .StatsPassword "")) (gt .StatsPort 0) }} + # Add your custom health check monitoring failure condition here. + # monitor fail if + stats enable + stats hide-version + stats realm Haproxy\ Statistics + stats uri / + stats auth {{ .StatsUser }}:{{ .StatsPassword }} + {{- end }} + {{- end }} + + {{ if .BindPorts -}} +frontend public + {{ if eq "v4v6" $router_ip_v4_v6_mode }} + bind :{{ env "ROUTER_SERVICE_HTTP_PORT" "80" }}{{ if isTrue (env "ROUTER_USE_PROXY_PROTOCOL") }} accept-proxy{{ end }} + bind :::{{ env "ROUTER_SERVICE_HTTP_PORT" "80" }} v6only{{ if isTrue (env "ROUTER_USE_PROXY_PROTOCOL") }} accept-proxy{{ end }} + {{- else if eq "v6" $router_ip_v4_v6_mode }} + bind :::{{ env "ROUTER_SERVICE_HTTP_PORT" "80" }} v6only{{ if isTrue (env "ROUTER_USE_PROXY_PROTOCOL") }} accept-proxy{{ end }} + {{- else }} + bind :{{ env "ROUTER_SERVICE_HTTP_PORT" "80" }}{{ if isTrue (env "ROUTER_USE_PROXY_PROTOCOL") }} accept-proxy{{ end }} + {{- end }} + mode http + tcp-request inspect-delay {{ firstMatch $timeSpecPattern (env "ROUTER_INSPECT_DELAY") "5s" }} + tcp-request content accept if HTTP + + {{- if (eq .StatsPort -1) }} + monitor-uri /_______internal_router_healthz + {{- end }} + + {{- range $idx, $captureHeader := .CaptureHTTPRequestHeaders }} + capture request header {{ $captureHeader.Name }} len {{ $captureHeader.MaxLength }} + {{- end }} + {{- range $idx, $captureHeader := .CaptureHTTPResponseHeaders }} + capture response header {{ $captureHeader.Name }} len {{ $captureHeader.MaxLength }} + {{- end }} + {{- with $captureCookie := .CaptureHTTPCookie }} + capture cookie {{ $captureCookie.Name }}{{ if eq $captureCookie.MatchType "exact" }}={{ end }} len {{ $captureCookie.MaxLength }} + {{- end }} + + # Strip off Proxy headers to prevent HTTpoxy (https://httpoxy.org/) + http-request del-header Proxy + + # DNS labels are case insensitive (RFC 4343), we need to convert the hostname into lowercase + # before matching, or any requests containing uppercase characters will never match. + http-request set-header Host %[req.hdr(Host),lower] + + {{- if and (ne (env "ROUTER_UNIQUE_ID_FORMAT") "") (ne (env "ROUTER_UNIQUE_ID_HEADER_NAME") "") }} + unique-id-format {{ env "ROUTER_UNIQUE_ID_FORMAT" }} + unique-id-header {{ env "ROUTER_UNIQUE_ID_HEADER_NAME" }} + {{- end }} + + # check if we need to redirect/force using https. + acl secure_redirect base,map_reg_int(/var/lib/haproxy/conf/os_route_http_redirect.map) -m bool + redirect scheme https if secure_redirect + + use_backend %[base,map_reg(/var/lib/haproxy/conf/os_http_be.map)] + + default_backend openshift_default + +# public ssl accepts all connections and isn't checking certificates yet certificates to use will be +# determined by the next backend in the chain which may be an app backend (passthrough termination) or a backend +# that terminates encryption in this router (edge) +frontend public_ssl + {{- if ne (env "ROUTER_SYSLOG_ADDRESS") "" }} + option tcplog + {{- end }} + {{ if eq "v4v6" $router_ip_v4_v6_mode }} + bind :{{ env "ROUTER_SERVICE_HTTPS_PORT" "443" }}{{ if isTrue (env "ROUTER_USE_PROXY_PROTOCOL") }} accept-proxy{{ end }} + bind :::{{ env "ROUTER_SERVICE_HTTPS_PORT" "443" }} v6only{{ if isTrue (env "ROUTER_USE_PROXY_PROTOCOL") }} accept-proxy{{ end }} + {{- else if eq "v6" $router_ip_v4_v6_mode }} + bind :::{{ env "ROUTER_SERVICE_HTTPS_PORT" "443" }} v6only{{ if isTrue (env "ROUTER_USE_PROXY_PROTOCOL") }} accept-proxy{{ end }} + {{- else }} + bind :{{ env "ROUTER_SERVICE_HTTPS_PORT" "443" }}{{ if isTrue (env "ROUTER_USE_PROXY_PROTOCOL") }} accept-proxy{{ end }} + {{- end }} + tcp-request inspect-delay {{ firstMatch $timeSpecPattern (env "ROUTER_INSPECT_DELAY") "5s" }} + tcp-request content accept if { req_ssl_hello_type 1 } + + # if the connection is SNI and the route is a passthrough don't use the termination backend, just use the tcp backend + # for the SNI case, we also need to compare it in case-insensitive mode (by converting it to lowercase) as RFC 4343 says + acl sni req.ssl_sni -m found + acl sni_passthrough req.ssl_sni,lower,map_reg(/var/lib/haproxy/conf/os_sni_passthrough.map) -m found + use_backend %[req.ssl_sni,lower,map_reg(/var/lib/haproxy/conf/os_tcp_be.map)] if sni sni_passthrough + + # HYPERSHIFT CHANGE START + # Disabling this breaks routes that are not pass through, which is ok because we only have passthrough routes. + # if the route is SNI and NOT passthrough enter the termination flow + # use_backend be_sni if sni + + # non SNI requests should enter a default termination backend rather than the custom cert SNI backend since it + # will not be able to match a cert to an SNI host + #default_backend be_no_sni + default_backend be_tcp:<>:kube-apiserver + # HYPERSHIFT CHANGE END + +########################################################################## +# TLS SNI +# +# When using SNI we can terminate encryption with custom certificates. +# Certs will be stored in a directory and will be matched with the SNI host header +# which must exist in the CN of the certificate. Certificates must be concatenated +# as a single file (handled by the plugin writer) per the haproxy documentation. +# +# Finally, check re-encryption settings and re-encrypt or just pass along the unencrypted +# traffic +########################################################################## +backend be_sni + server fe_sni unix@/var/lib/haproxy/run/haproxy-sni.sock weight 1 send-proxy + +frontend fe_sni + # terminate ssl on edge + bind unix@/var/lib/haproxy/run/haproxy-sni.sock ssl + {{- if isTrue (env "ROUTER_STRICT_SNI") }} strict-sni {{ end }} + {{- "" }} crt {{firstMatch ".+" .DefaultCertificate "/var/lib/haproxy/conf/default_pub_keys.pem" }} + {{- "" }} crt-list /var/lib/haproxy/conf/cert_config.map accept-proxy + {{- with (env "ROUTER_MUTUAL_TLS_AUTH") }} + {{- "" }} verify {{. }} + {{- with (env "ROUTER_MUTUAL_TLS_AUTH_CA") }} ca-file {{. }} {{ else }} ca-file /etc/ssl/certs/ca-bundle.trust.crt {{ end }} + {{- with (env "ROUTER_MUTUAL_TLS_AUTH_CRL") }} crl-file {{. }} {{ end }} + {{- end }} + mode http + + {{- range $idx, $captureHeader := .CaptureHTTPRequestHeaders }} + capture request header {{ $captureHeader.Name }} len {{ $captureHeader.MaxLength }} + {{- end }} + {{- range $idx, $captureHeader := .CaptureHTTPResponseHeaders }} + capture response header {{ $captureHeader.Name }} len {{ $captureHeader.MaxLength }} + {{- end }} + {{- with $captureCookie := .CaptureHTTPCookie }} + capture cookie {{ $captureCookie.Name }}{{ if eq $captureCookie.MatchType "exact" }}={{ end }} len {{ $captureCookie.MaxLength }} + {{- end }} + + # Strip off Proxy headers to prevent HTTpoxy (https://httpoxy.org/) + http-request del-header Proxy + + # DNS labels are case insensitive (RFC 4343), we need to convert the hostname into lowercase + # before matching, or any requests containing uppercase characters will never match. + http-request set-header Host %[req.hdr(Host),lower] + + {{- if and (ne (env "ROUTER_UNIQUE_ID_FORMAT") "") (ne (env "ROUTER_UNIQUE_ID_HEADER_NAME") "") }} + unique-id-format {{ env "ROUTER_UNIQUE_ID_FORMAT" }} + unique-id-header {{ env "ROUTER_UNIQUE_ID_HEADER_NAME" }} + {{- end }} + + {{ if ne (env "ROUTER_MUTUAL_TLS_AUTH" "none") "none" }} + {{- with (env "ROUTER_MUTUAL_TLS_AUTH_FILTER") }} + # If a mutual TLS auth subject filter environment variable is set, we deny + # requests if the DN field in the client certificate doesn't match that value. + # Please note that this match is a regular expression match. + # Example: For DN set to: /CN=header.test/ST=CA/C=US/O=Security/OU=OpenShift3, + # A. ROUTER_MUTUAL_TLS_AUTH_FILTER="header.test" OR + # ROUTER_MUTUAL_TLS_AUTH_FILTER="head" OR + # ROUTER_MUTUAL_TLS_AUTH_FILTER="^/CN=header.test/ST=CA/C=US/O=Security/OU=OpenShift3$" /* exact match example */ + # the filter would match the DN field (substring or exact match) + # and the request will be passed on to the backend. + # B. ROUTER_MUTUAL_TLS_AUTH_FILTER="legacy-web-client", the request + # will be rejected. + acl cert_cn_matches ssl_c_s_dn -m reg {{ . }} + http-request deny unless cert_cn_matches + {{- end }} + + # Add X-SSL* headers to pass client certificate information to the backend. + http-request set-header X-SSL %[ssl_fc] + http-request set-header X-SSL-Client-Verify %[ssl_c_verify] + http-request set-header X-SSL-Client-Serial %{+Q}[ssl_c_serial,hex] + http-request set-header X-SSL-Client-Version %{+Q}[ssl_c_version] + http-request set-header X-SSL-Client-SHA1 %{+Q}[ssl_c_sha1,hex] + http-request set-header X-SSL-Client-DN %{+Q}[ssl_c_s_dn] + http-request set-header X-SSL-Client-CN %{+Q}[ssl_c_s_dn(cn)] + http-request set-header X-SSL-Issuer %{+Q}[ssl_c_i_dn] + http-request set-header X-SSL-Client-NotBefore %{+Q}[ssl_c_notbefore] + http-request set-header X-SSL-Client-NotAfter %{+Q}[ssl_c_notafter] + http-request set-header X-SSL-Client-DER %{+Q}[ssl_c_der,base64] + {{- end }} + + # map to backend + # Search from most specific to general path (host case). + # Note: If no match, haproxy uses the default_backend, no other + # use_backend directives below this will be processed. + use_backend %[base,map_reg(/var/lib/haproxy/conf/os_edge_reencrypt_be.map)] + + default_backend openshift_default + +########################################################################## +# END TLS SNI +########################################################################## + +########################################################################## +# TLS NO SNI +# +# When we don't have SNI the only thing we can try to do is terminate the encryption +# using our wild card certificate. Once that is complete we can either re-encrypt +# the traffic or pass it on to the backends +########################################################################## +# backend for when sni does not exist, or ssl term needs to happen on the edge +backend be_no_sni + server fe_no_sni unix@/var/lib/haproxy/run/haproxy-no-sni.sock weight 1 send-proxy + +frontend fe_no_sni + # terminate ssl on edge + bind unix@/var/lib/haproxy/run/haproxy-no-sni.sock ssl crt {{ firstMatch ".+" .DefaultCertificate "/var/lib/haproxy/conf/default_pub_keys.pem" }} accept-proxy + {{- with (env "ROUTER_MUTUAL_TLS_AUTH") }} + {{- "" }} verify {{. }} + {{- with (env "ROUTER_MUTUAL_TLS_AUTH_CA") }} ca-file {{. }} {{ else }} ca-file /etc/ssl/certs/ca-bundle.trust.crt {{ end }} + {{- with (env "ROUTER_MUTUAL_TLS_AUTH_CRL") }} crl-file {{. }} {{ end }} + {{- end }} + mode http + + {{- range $idx, $captureHeader := .CaptureHTTPRequestHeaders }} + capture request header {{ $captureHeader.Name }} len {{ $captureHeader.MaxLength }} + {{- end }} + {{- range $idx, $captureHeader := .CaptureHTTPResponseHeaders }} + capture response header {{ $captureHeader.Name }} len {{ $captureHeader.MaxLength }} + {{- end }} + {{- with $captureCookie := .CaptureHTTPCookie }} + capture cookie {{ $captureCookie.Name }}{{ if eq $captureCookie.MatchType "exact" }}={{ end }} len {{ $captureCookie.MaxLength }} + {{- end }} + + # Strip off Proxy headers to prevent HTTpoxy (https://httpoxy.org/) + http-request del-header Proxy + + # DNS labels are case insensitive (RFC 4343), we need to convert the hostname into lowercase + # before matching, or any requests containing uppercase characters will never match. + http-request set-header Host %[req.hdr(Host),lower] + + {{- if and (ne (env "ROUTER_UNIQUE_ID_FORMAT") "") (ne (env "ROUTER_UNIQUE_ID_HEADER_NAME") "") }} + unique-id-format {{ env "ROUTER_UNIQUE_ID_FORMAT" }} + unique-id-header {{ env "ROUTER_UNIQUE_ID_HEADER_NAME" }} + {{- end }} + + {{ if ne (env "ROUTER_MUTUAL_TLS_AUTH" "none") "none" }} + {{- with (env "ROUTER_MUTUAL_TLS_AUTH_FILTER") }} + # If a mutual TLS auth subject filter environment variable is set, we deny + # requests if the DN field in the client certificate doesn't match that value. + # Please note that this match is a regular expression match. + # See the config section 'frontend fe_sni' for examples. + acl cert_cn_matches ssl_c_s_dn -m reg {{ . }} + http-request deny unless cert_cn_matches + {{- end }} + + # Add X-SSL* headers to pass client certificate information to the backend. + http-request set-header X-SSL %[ssl_fc] + http-request set-header X-SSL-Client-Verify %[ssl_c_verify] + http-request set-header X-SSL-Client-Serial %{+Q}[ssl_c_serial,hex] + http-request set-header X-SSL-Client-Version %{+Q}[ssl_c_version] + http-request set-header X-SSL-Client-SHA1 %{+Q}[ssl_c_sha1,hex] + http-request set-header X-SSL-Client-DN %{+Q}[ssl_c_s_dn] + http-request set-header X-SSL-Client-CN %{+Q}[ssl_c_s_dn(cn)] + http-request set-header X-SSL-Issuer %{+Q}[ssl_c_i_dn] + http-request set-header X-SSL-Client-NotBefore %{+Q}[ssl_c_notbefore] + http-request set-header X-SSL-Client-NotAfter %{+Q}[ssl_c_notafter] + http-request set-header X-SSL-Client-DER %{+Q}[ssl_c_der,base64] + {{- end }} + + # map to backend + # Search from most specific to general path (host case). + # Note: If no match, haproxy uses the default_backend, no other + # use_backend directives below this will be processed. + use_backend %[base,map_reg(/var/lib/haproxy/conf/os_edge_reencrypt_be.map)] + + default_backend openshift_default + +########################################################################## +# END TLS NO SNI +########################################################################## + +backend openshift_default + mode http + option forwardfor + #option http-keep-alive + option http-pretend-keepalive + {{- if ne "" (env "ROUTER_ERRORFILE_404") }} + http-request deny deny_status 404 + {{- end }} + +##-------------- app level backends ---------------- + {{/* + 1. If termination is not set: This is plain http -> http. Create a be_http: backend. + Incoming http traffic is terminated and sent as http to the pods. + + 2. If termination is type 'edge': This is https -> http. Create a be_edge_http: backend. + Incoming https traffic is terminated and sent as http to the pods. + + 3. If termination is type 'reencrypt': This is https -> https. Create a be_secure: backend. + Incoming https traffic is terminated and then sent as https to the pods. + + 4. If termination is type 'passthrough': This is https (or any SNI TLS connection) passthrough. + Create a be_tcp: backend. + Incoming traffic is inspected to get the hostname from the SNI header, but then all traffic is + passed through to the backend pod by just looking at the TCP headers. +*/}} + {{- range $cfgIdx, $cfg := .State }} + {{- if matchValues (print $cfg.TLSTermination) "" "edge" "reencrypt" }} + +# Plain http backend or backend with TLS terminated at the edge or a +# secure backend with re-encryption. +backend {{ genBackendNamePrefix $cfg.TLSTermination }}:{{ $cfgIdx }} + mode http + option redispatch + {{- with $setHeaders := firstMatch $setForwardedHeadersPattern (index $cfg.Annotations $setForwardedHeadersAnnotation) $setForwardedHeadersDefaultValue }} + {{- if eq $setHeaders "append" }} + option forwardfor + {{- else if eq $setHeaders "if-none" }} + option forwardfor if-none + {{- end }} + {{- end }} + + {{- with $adjustments := $.HTTPHeaderNameCaseAdjustments }} + {{- if isTrue (index $cfg.Annotations "haproxy.router.openshift.io/h1-adjust-case") }} + option h1-case-adjust-bogus-server + {{- end }} + {{- end }} + + {{- with $balanceAlgo := firstMatch $balanceAlgoPattern (index $cfg.Annotations "haproxy.router.openshift.io/balance") }} + balance {{ $balanceAlgo }} + {{- else }} + balance {{ if gt $cfg.ActiveServiceUnits 1 }}roundrobin{{ else }}{{ firstMatch $balanceAlgoPattern (env "ROUTER_LOAD_BALANCE_ALGORITHM") "random" }}{{ end }} + {{- end }} + {{- with $ip_whiteList := parseIPList (index $cfg.Annotations "haproxy.router.openshift.io/ip_whitelist") }} + {{- if validateHAProxyWhiteList $ip_whiteList }} + acl whitelist src {{ $ip_whiteList }} + {{- else }} + {{- with $whiteListFileName := generateHAProxyWhiteListFile $workingDir $cfgIdx $ip_whiteList }} + acl whitelist src -f {{ $whiteListFileName }} + {{- end }} + {{- end }} + tcp-request content reject if !whitelist + {{- end }} + {{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout")) }} + timeout server {{ $value }} + {{- end }} + {{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout-tunnel")) }} + timeout tunnel {{ $value }} + {{- end }} + + {{- if isTrue (index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections") }} + stick-table type ip size 100k expire 30s store conn_cur,conn_rate(3s),http_req_rate(10s) + tcp-request content track-sc2 src + {{- if (isInteger (index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections.concurrent-tcp")) }} + tcp-request content reject if { src_conn_cur ge {{ index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections.concurrent-tcp" }} } + {{- else }} + # concurrent TCP connections not restricted + {{- end }} + + {{- if (isInteger (index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections.rate-tcp")) }} + tcp-request content reject if { src_conn_rate ge {{ index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections.rate-tcp" }} } + {{- else }} + #TCP connection rate not restricted + {{- end }} + + {{- if (isInteger (index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections.rate-http")) }} + tcp-request content reject if { src_http_req_rate ge {{ index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections.rate-http" }} } + {{- else }} + #HTTP request rate not restricted + {{- end }} + {{- end }} + + timeout check 5000ms + {{- with $setHeaders := firstMatch $setForwardedHeadersPattern (index $cfg.Annotations $setForwardedHeadersAnnotation) $setForwardedHeadersDefaultValue }} + {{- if eq $setHeaders "append" }} + {{- /* X-Forwarded-For: is handled by "option forwardfor" above. */}} + http-request add-header X-Forwarded-Host %[req.hdr(host)] + http-request add-header X-Forwarded-Port %[dst_port] + http-request add-header X-Forwarded-Proto http if !{ ssl_fc } + http-request add-header X-Forwarded-Proto https if { ssl_fc } + http-request add-header X-Forwarded-Proto-Version h2 if { ssl_fc_alpn -i h2 } + {{- if eq "v4v6" $router_ip_v4_v6_mode }} + # See the quoting rules in https://tools.ietf.org/html/rfc7239 for IPv6 addresses (v4 addresses get translated to v6 when in hybrid mode) + acl ipv6_addr src -m sub : + http-request add-header Forwarded for=\"[%[src]]\";host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] if ipv6_addr + http-request add-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] if !ipv6_addr + {{- else if eq "v6" $router_ip_v4_v6_mode }} + http-request add-header Forwarded for=\"[%[src]]\";host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] + {{- else }} + http-request add-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] + {{- end }} + {{- else if eq $setHeaders "replace" }} + http-request set-header X-Forwarded-For %[src] + http-request set-header X-Forwarded-Host %[req.hdr(host)] + http-request set-header X-Forwarded-Port %[dst_port] + http-request set-header X-Forwarded-Proto http if !{ ssl_fc } + http-request set-header X-Forwarded-Proto https if { ssl_fc } + http-request set-header X-Forwarded-Proto-Version h2 if { ssl_fc_alpn -i h2 } + {{- if eq "v4v6" $router_ip_v4_v6_mode }} + # See the quoting rules in https://tools.ietf.org/html/rfc7239 for IPv6 addresses (v4 addresses get translated to v6 when in hybrid mode) + acl ipv6_addr src -m sub : + http-request set-header Forwarded for=\"[%[src]]\";host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] if ipv6_addr + http-request set-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] if !ipv6_addr + {{- else if eq "v6" $router_ip_v4_v6_mode }} + http-request set-header Forwarded for=\"[%[src]]\";host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] + {{- else }} + http-request set-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] + {{- end }} + {{- else if eq $setHeaders "if-none" }} + {{- /* X-Forwarded-For: is handled by "option forwardfor if-none" above. */}} + http-request set-header X-Forwarded-Host %[req.hdr(host)] if !{ req.hdr(X-Forwarded-Host) -m found } + http-request set-header X-Forwarded-Port %[dst_port] if !{ req.hdr(X-Forwarded-Port) -m found } + http-request set-header X-Forwarded-Proto http if !{ ssl_fc } !{ req.hdr(X-Forwarded-Proto) -m found } + http-request set-header X-Forwarded-Proto https if { ssl_fc } !{ req.hdr(X-Forwarded-Proto) -m found } + http-request set-header X-Forwarded-Proto-Version h2 if { ssl_fc_alpn -i h2 } !{ req.hdr(X-Forwarded-Proto-Version) -m found } + {{- if eq "v4v6" $router_ip_v4_v6_mode }} + # See the quoting rules in https://tools.ietf.org/html/rfc7239 for IPv6 addresses (v4 addresses get translated to v6 when in hybrid mode) + acl ipv6_addr src -m sub : + http-request set-header Forwarded for=\"[%[src]]\";host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] if ipv6_addr !{ req.hdr(Forwarded) -m found } + http-request set-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] if !ipv6_addr !{ req.hdr(Forwarded) -m found } + {{- else if eq "v6" $router_ip_v4_v6_mode }} + http-request set-header Forwarded for=\"[%[src]]\";host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] if !{ req.hdr(Forwarded) -m found } + {{- else }} + http-request set-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] if !{ req.hdr(Forwarded) -m found } + {{- end }} + {{- else if eq $setHeaders "never" }} + {{- /* No Forward headers set. */}} + {{- end }} + {{- end }} + + {{- with $pathRewriteTarget := firstMatch $pathRewriteTargetPattern (index $cfg.Annotations "haproxy.router.openshift.io/rewrite-target") }} + # Path rewrite target + {{- if eq $pathRewriteTarget "/" }} + http-request replace-path ^{{ $cfg.Path }}/?(.*)$ {{ $pathRewriteTarget }}\1 + {{- else }} + http-request replace-path ^{{ $cfg.Path }}(.*)$ {{ $pathRewriteTarget }}\1 + {{- end }} + {{- end }}{{/* rewrite target */}} + + {{- if not (isTrue (index $cfg.Annotations "haproxy.router.openshift.io/disable_cookies")) }} + cookie {{ firstMatch $cookieNamePattern (index $cfg.Annotations "router.openshift.io/cookie_name") (env "ROUTER_COOKIE_NAME" "") $cfg.RoutingKeyName }} insert indirect nocache httponly + {{- if and (matchValues (print $cfg.TLSTermination) "edge" "reencrypt") (ne $cfg.InsecureEdgeTerminationPolicy "Allow") }} + {{- with $samesite := firstMatch "Lax|Strict|None" (index $cfg.Annotations "router.openshift.io/cookie-same-site") "None" }} + {{- "" }} secure attr SameSite={{ $samesite }} + {{- end }} + {{- end }} + {{- end }}{{/* end disable cookies check */}} + + {{- if matchValues (print $cfg.TLSTermination) "edge" "reencrypt" }} + {{- with $hsts := firstMatch $hstsPattern (index $cfg.Annotations "haproxy.router.openshift.io/hsts_header") }} + http-response set-header Strict-Transport-Security '{{ $hsts }}' + {{- end }}{{/* hsts header */}} + {{- end }}{{/* is "edge" or "reencrypt" */}} + + {{- range $serviceUnitName, $weight := $cfg.ServiceUnitNames }} + {{- if ge $weight 0 }}{{/* weight=0 is reasonable to keep existing connections to backends with cookies as we can see the HTTP headers */}} + {{- with $serviceUnit := index $.ServiceUnits $serviceUnitName }} + {{- range $idx, $endpoint := processEndpointsForAlias $cfg $serviceUnit (env "ROUTER_BACKEND_PROCESS_ENDPOINTS" "") }} + server {{ $endpoint.ID }} {{ $endpoint.IP }}:{{ $endpoint.Port }} cookie {{ $endpoint.IdHash }} weight {{ $weight }} + {{- if (eq $cfg.TLSTermination "reencrypt") }} ssl + {{- if not (isTrue $router_disable_http2) }} alpn h2,http/1.1 + {{- end }} + {{- if $cfg.VerifyServiceHostname }} verifyhost {{ $serviceUnit.Hostname }} + {{- end }} + {{- if gt (len (index $cfg.Certificates (printf "%s_pod" $cfg.Host)).Contents) 0 }} verify required ca-file {{ $workingDir }}/router/cacerts/{{$cfgIdx }}.pem + {{- else }} + {{- if gt (len $defaultDestinationCA) 0 }} verify required ca-file {{ $defaultDestinationCA }} + {{- else }} verify none + {{- end }} + {{- end }} + {{- else if or (eq $cfg.TLSTermination "") (eq $cfg.TLSTermination "edge") }} + {{- if eq $endpoint.AppProtocol "h2c" }} proto h2 + {{- end }} + {{- end }}{{/* end type specific options*/}} + + {{- if and (not $endpoint.NoHealthCheck) (gt $cfg.ActiveEndpoints 1) }} check inter {{firstMatch $timeSpecPattern (index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval") (env "ROUTER_BACKEND_CHECK_INTERVAL") "5000ms" }} + {{- end }}{{/* end else no health check */}} + {{- with $podMaxConn := index $cfg.Annotations "haproxy.router.openshift.io/pod-concurrent-connections" }} + {{- if (isInteger (index $cfg.Annotations "haproxy.router.openshift.io/pod-concurrent-connections")) }} maxconn {{$podMaxConn }} {{- end }} + {{- end }}{{/* end pod-concurrent-connections annotation */}} + + {{- end }}{{/* end if cg.TLSTermination */}} + {{- end }}{{/* end range processEndpointsForAlias */}} + {{- end }}{{/* end get serviceUnit from its name */}} + {{- end }}{{/* end range over serviceUnitNames */}} + + {{- with $dynamicConfigManager }} + {{- if (eq $cfg.TLSTermination "reencrypt") }} + {{- range $idx, $serverName := $dynamicConfigManager.GenerateDynamicServerNames $cfgIdx }} + server {{ $serverName }} 172.4.0.4:8765 weight 0 ssl disabled check inter {{ firstMatch $timeSpecPattern (index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval") (env "ROUTER_BACKEND_CHECK_INTERVAL") "5000ms" }} + {{- if gt (len (index $cfg.Certificates (printf "%s_pod" $cfg.Host)).Contents) 0 }} verify required ca-file {{ $workingDir }}/router/cacerts/{{$cfgIdx }}.pem + {{- else }} + {{- if gt (len $defaultDestinationCA) 0 }} verify required ca-file {{ $defaultDestinationCA }} + {{- else }} verify none + {{- end }} + {{- end }} + {{- with $podMaxConn := index $cfg.Annotations "haproxy.router.openshift.io/pod-concurrent-connections" }} + {{- if (isInteger (index $cfg.Annotations "haproxy.router.openshift.io/pod-concurrent-connections")) }} maxconn {{$podMaxConn }} {{- end }} + {{- end }}{{/* end pod-concurrent-connections annotation */}} + {{- end }}{{/* end range over dynamic server names */}} + + {{- else }} + {{- with $name := $dynamicConfigManager.ServerTemplateName $cfgIdx }} + {{- with $size := $dynamicConfigManager.ServerTemplateSize $cfgIdx }} + dynamic-cookie-key {{ $cfg.RoutingKeyName }} + server-template {{ $name }}- 1-{{ $size }} 172.4.0.4:8765 check disabled + {{- end }} + {{- end }} + {{- end }} + {{- end }} + + {{- end }}{{/* end if tls==edge/none/reencrypt */}} + + {{- if eq $cfg.TLSTermination "passthrough" }} + +# Secure backend, pass through +backend {{ genBackendNamePrefix $cfg.TLSTermination }}:{{ $cfgIdx }} + {{- with $balanceAlgo := firstMatch $balanceAlgoPattern (index $cfg.Annotations "haproxy.router.openshift.io/balance") }} + balance {{ $balanceAlgo }} + {{- else }} + balance {{ if gt $cfg.ActiveServiceUnits 1 }}roundrobin{{ else }}{{ firstMatch $balanceAlgoPattern (env "ROUTER_TCP_BALANCE_SCHEME") (env "ROUTER_LOAD_BALANCE_ALGORITHM") "source" }}{{ end }} + {{- end }} + {{- with $ip_whiteList := parseIPList (index $cfg.Annotations "haproxy.router.openshift.io/ip_whitelist") }} + {{- if validateHAProxyWhiteList $ip_whiteList }} + acl whitelist src {{ $ip_whiteList }} + {{- else }} + {{- with $whiteListFileName := generateHAProxyWhiteListFile $workingDir $cfgIdx $ip_whiteList }} + acl whitelist src -f {{ $whiteListFileName }} + {{- end }} + {{- end }} + tcp-request content reject if !whitelist + {{- end }} + {{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout-tunnel") (index $cfg.Annotations "haproxy.router.openshift.io/timeout")) }} + timeout tunnel {{ $value }} + {{- end }} + + {{- if isTrue (index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections") }} + stick-table type ip size 100k expire 30s store conn_cur,conn_rate(3s),http_req_rate(10s) + tcp-request content track-sc2 src + {{- if (isInteger (index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections.concurrent-tcp")) }} + tcp-request content reject if { src_conn_cur ge {{ index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections.concurrent-tcp" }} } + {{- else }} + # concurrent TCP connections not restricted + {{- end }} + + {{- if (isInteger (index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections.rate-tcp")) }} + tcp-request content reject if { src_conn_rate ge {{ index $cfg.Annotations "haproxy.router.openshift.io/rate-limit-connections.rate-tcp" }} } + {{- else }} + #TCP connection rate not restricted + {{- end }} + {{- end }} + + hash-type consistent + timeout check 5000ms + {{- range $serviceUnitName, $weight := $cfg.ServiceUnitNames }} + {{- if ne $weight 0 }}{{/* drop connections where weight=0 as we can't use cookies, leaving only r-r and src-ip as dispatch methods and weight make no sense there */}} + {{- with $serviceUnit := index $.ServiceUnits $serviceUnitName }} + {{- range $idx, $endpoint := processEndpointsForAlias $cfg $serviceUnit (env "ROUTER_BACKEND_PROCESS_ENDPOINTS" "") }} + server {{ $endpoint.ID }} {{ $endpoint.IP }}:{{ $endpoint.Port }} weight {{ $weight }} + {{- if and (not $endpoint.NoHealthCheck) (gt $cfg.ActiveEndpoints 1) }} check inter {{firstMatch $timeSpecPattern (index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval") (env "ROUTER_BACKEND_CHECK_INTERVAL") "5000ms" }} + {{- end }}{{/* end else no health check */}} + {{- with $podMaxConn := index $cfg.Annotations "haproxy.router.openshift.io/pod-concurrent-connections" }} + {{- if (isInteger (index $cfg.Annotations "haproxy.router.openshift.io/pod-concurrent-connections")) }} maxconn {{$podMaxConn }} {{- end }} + {{- end }}{{/* end pod-concurrent-connections annotation */}} + + {{- end }}{{/* end range processEndpointsForAlias */}} + {{- end }}{{/* end get ServiceUnit from serviceUnitName */}} + {{- end }}{{/* end if weight != 0 */}} + {{- end }}{{/* end iterate over services*/}} + + {{- with $dynamicConfigManager }} + {{- with $name := $dynamicConfigManager.ServerTemplateName $cfgIdx }} + {{- with $size := $dynamicConfigManager.ServerTemplateSize $cfgIdx }} + dynamic-cookie-key {{ $cfg.RoutingKeyName }} + server-template {{ $name }}- 1-{{ $size }} 172.4.0.4:8765 check disabled + {{- end }} + {{- end }} + {{- end }} + + {{- end }}{{/*end tls==passthrough*/}} + + {{- end }}{{/* end loop over routes */}} + {{- else }} +# Avoiding binding ports until routing configuration has been synchronized. + {{- end }}{{/* end bind ports after sync */}} +{{ end }}{{/* end haproxy config template */}} + +{{/*--------------------------------- END OF HAPROXY CONFIG, BELOW ARE MAPPING FILES ------------------------*/}} +{{/* + os_wildcard_domain.map: contains a mapping of wildcard hosts for a + [sub]domain regexps. This map is used to check if + a host matches a [sub]domain with has wildcard support. +*/}} +{{ define "conf/os_wildcard_domain.map" -}} +{{ if isTrue (env "ROUTER_ALLOW_WILDCARD_ROUTES") -}} + {{ range $idx, $line := generateHAProxyMap . -}} + {{ $line }} + {{ end -}} +{{ end -}}{{/* end if router allows wildcard routes */ -}} +{{ end -}}{{/* end wildcard domain map template */}} + + +{{/* + os_http_be.map : contains a mapping of www.example.com -> . This map is used to discover the correct backend + by attaching a prefix: be_http for http routes + be_edge_http for edge routes with InsecureEdgeTerminationPolicy Allow + be_secure for reencrypt routes with InsecureEdgeTerminationPolicy Allow +*/}} +{{ define "conf/os_http_be.map" -}} +{{ range $idx, $line := generateHAProxyMap . -}} + {{ $line }} +{{ end -}} +{{ end -}}{{/* end http host map template */}} + + + +{{/* + os_edge_reencrypt_be.map : contains a mapping of www.example.com -> . This map is similar to os_http_be.map but for tls routes. + by attaching prefix: be_edge_http for edge terminated routes + be_secure for reencrypt routes +*/}} +{{ define "conf/os_edge_reencrypt_be.map" -}} +{{ range $idx, $line := generateHAProxyMap . -}} + {{ $line }} +{{ end -}} +{{ end -}}{{/* end edge http host map template */}} + + +{{/* + os_route_http_redirect.map: contains a mapping of www.example.com -> . + Map is used to redirect insecure traffic to use a secure scheme (https) + if acls match for routes that have the insecure option set to redirect. +*/}} +{{ define "conf/os_route_http_redirect.map" -}} +{{ range $idx, $line := generateHAProxyMap . -}} + {{ $line }} +{{ end -}} +{{ end -}}{{/* end redirect http host map template */}} + + +{{/* + os_tcp_be.map: contains a mapping of www.example.com -> . This map is used to discover the correct backend + by use_backend statements if acls are matched. +*/}} +{{ define "conf/os_tcp_be.map" -}} +{{ range $idx, $line := generateHAProxyMap . -}} + {{ $line }} +{{ end -}} +{{ end -}}{{/* end tcp host map template */}} + + +{{/* + os_sni_passthrough.map: contains a mapping of routes that expect to have an sni header and should be passed + through to the host_be. Driven by the termination type of the ServiceAliasConfigs +*/}} +{{ define "conf/os_sni_passthrough.map" -}} +{{ range $idx, $line := generateHAProxyMap . -}} + {{ $line }} +{{ end -}} +{{ end -}}{{/* end sni passthrough map template */}} + +{{/* + cert_config.map: contains a mapping of -> example.org + This map is used to present the appropriate cert + based on the sni header. + Note: It is sort of a reverse map for our case but the order + ": " is important as this allows us to use + wildcards and/or use a deny set with ! in the future. +*/}} +{{ define "conf/cert_config.map" -}} +{{ range $idx, $line := generateHAProxyMap . -}} + {{ $line }} +{{ end -}} +{{ end -}}{{/* end cert_config map template */}} diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go index 8166cb3374cd..0b51ee5e0a57 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go @@ -10,7 +10,10 @@ import ( "k8s.io/apimachinery/pkg/util/duration" "k8s.io/apimachinery/pkg/util/intstr" + routev1 "github.com/openshift/api/route/v1" hyperv1 "github.com/openshift/hypershift/api/v1alpha1" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/ingress" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" "github.com/openshift/hypershift/support/events" "github.com/openshift/hypershift/support/util" ) @@ -57,6 +60,8 @@ func ReconcileService(svc *corev1.Service, strategy *hyperv1.ServicePublishingSt if portSpec.NodePort == 0 && strategy.NodePort != nil { portSpec.NodePort = strategy.NodePort.Port } + case hyperv1.Route: + svc.Spec.Type = corev1.ServiceTypeClusterIP default: return fmt.Errorf("invalid publishing strategy for Kube API server service: %s", strategy.Type) } @@ -103,6 +108,9 @@ func ReconcileServiceStatus(svc *corev1.Service, strategy *hyperv1.ServicePublis } port = svc.Spec.Ports[0].NodePort host = strategy.NodePort.Address + case hyperv1.Route: + host = strategy.Route.Hostname + port = int32(apiServerPort) } return } @@ -133,3 +141,21 @@ func ReconcilePrivateService(svc *corev1.Service, owner *metav1.OwnerReference) func ReconcilePrivateServiceStatus(hcpName string) (host string, port int32, err error) { return fmt.Sprintf("api.%s.hypershift.local", hcpName), 6443, nil } + +func ReconcileRoute(route *routev1.Route, hostname string) { + if route.Labels == nil { + route.Labels = map[string]string{} + } + route.Labels[ingress.HypershiftRouteLabel] = route.Namespace + if route.CreationTimestamp.IsZero() { + route.Spec.Host = hostname + } + route.Spec.To = routev1.RouteTargetReference{ + Kind: "Service", + Name: manifests.KubeAPIServerService("").Name, + } + route.Spec.TLS = &routev1.TLSConfig{ + Termination: routev1.TLSTerminationPassthrough, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + } +} diff --git a/control-plane-operator/controllers/hostedcontrolplane/konnectivity/reconcile.go b/control-plane-operator/controllers/hostedcontrolplane/konnectivity/reconcile.go index 60232093e589..dd1d04adb858 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/konnectivity/reconcile.go +++ b/control-plane-operator/controllers/hostedcontrolplane/konnectivity/reconcile.go @@ -220,7 +220,9 @@ func ReconcileRoute(route *routev1.Route, ownerRef config.OwnerRef, private bool switch { case !private && strategy.Route != nil && strategy.Route.Hostname != "": route.Spec.Host = strategy.Route.Hostname + ingress.AddRouteLabel(route) case private: + ingress.AddRouteLabel(route) route.Spec.Host = fmt.Sprintf("%s.apps.%s.hypershift.local", route.Name, ownerRef.Reference.Name) default: route.Spec.Host = util.ShortenRouteHostnameIfNeeded(route.Name, route.Namespace, defaultIngressDomain) diff --git a/control-plane-operator/controllers/hostedcontrolplane/manifests/infra.go b/control-plane-operator/controllers/hostedcontrolplane/manifests/infra.go index 43d66c48d7e3..0ab802dbf2fd 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/manifests/infra.go +++ b/control-plane-operator/controllers/hostedcontrolplane/manifests/infra.go @@ -35,6 +35,24 @@ func KubeAPIServerPrivateService(hostedClusterNamespace string) *corev1.Service } } +func KubeAPIServerExternalRoute(hostedClusterNamespace string) *routev1.Route { + return &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kube-apiserver", + Namespace: hostedClusterNamespace, + }, + } +} + +func KubeAPIServerInternalRoute(hostedClusterNamespace string) *routev1.Route { + return &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kube-apiserver-internal", + Namespace: hostedClusterNamespace, + }, + } +} + func OauthServerService(hostedClusterNamespace string) *corev1.Service { return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ diff --git a/control-plane-operator/controllers/hostedcontrolplane/manifests/ingress.go b/control-plane-operator/controllers/hostedcontrolplane/manifests/ingress.go index 749ae3fc13cc..30c6ba4c7710 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/manifests/ingress.go +++ b/control-plane-operator/controllers/hostedcontrolplane/manifests/ingress.go @@ -37,37 +37,37 @@ func IngressPrivateIngressController(name string) *operatorv1.IngressController } } -func PrivateRouterServiceAccount(ns string) *corev1.ServiceAccount { +func RouterServiceAccount(ns string) *corev1.ServiceAccount { return &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ - Name: "private-router", + Name: "router", Namespace: ns, }, } } -func PrivateRouterRole(ns string) *rbacv1.Role { +func RouterRole(ns string) *rbacv1.Role { return &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ - Name: "private-router", + Name: "router", Namespace: ns, }, } } -func PrivateRouterRoleBinding(ns string) *rbacv1.RoleBinding { +func RouterRoleBinding(ns string) *rbacv1.RoleBinding { return &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ - Name: "private-router", + Name: "router", Namespace: ns, }, } } -func PrivateRouterDeployment(ns string) *appsv1.Deployment { +func RouterDeployment(ns string) *appsv1.Deployment { return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "private-router", + Name: "router", Namespace: ns, }, } @@ -82,6 +82,24 @@ func PrivateRouterService(ns string) *corev1.Service { } } +func RouterPublicService(ns string) *corev1.Service { + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "router", + Namespace: ns, + }, + } +} + +func RouterTemplateConfigMap(ns string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "router-template", + Namespace: ns, + }, + } +} + func DNSConfig() *configv1.DNS { return &configv1.DNS{ ObjectMeta: metav1.ObjectMeta{ diff --git a/control-plane-operator/controllers/hostedcontrolplane/oauth/route.go b/control-plane-operator/controllers/hostedcontrolplane/oauth/route.go index a9d067202250..ddbf7f08ddfa 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/oauth/route.go +++ b/control-plane-operator/controllers/hostedcontrolplane/oauth/route.go @@ -1,15 +1,18 @@ package oauth import ( + "fmt" + routev1 "github.com/openshift/api/route/v1" hyperv1 "github.com/openshift/hypershift/api/v1alpha1" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/ingress" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/util" ) -func ReconcileRoute(route *routev1.Route, ownerRef config.OwnerRef, strategy *hyperv1.ServicePublishingStrategy, defaultIngressDomain string) error { +func ReconcileRoute(route *routev1.Route, ownerRef config.OwnerRef, strategy *hyperv1.ServicePublishingStrategy, defaultIngressDomain string, hcp *hyperv1.HostedControlPlane) error { ownerRef.ApplyTo(route) // The route host is considered immutable, so set it only once upon creation @@ -18,6 +21,10 @@ func ReconcileRoute(route *routev1.Route, ownerRef config.OwnerRef, strategy *hy switch { case strategy.Route != nil && strategy.Route.Hostname != "": route.Spec.Host = strategy.Route.Hostname + ingress.AddRouteLabel(route) + case !util.IsPublicHCP(hcp): + route.Spec.Host = fmt.Sprintf("oauth.apps.%s.hypershift.local", hcp.Name) + ingress.AddRouteLabel(route) default: route.Spec.Host = util.ShortenRouteHostnameIfNeeded(route.Name, route.Namespace, defaultIngressDomain) } diff --git a/hack/app-sre/saas_template.yaml b/hack/app-sre/saas_template.yaml index 1a8c7662b4ce..a5fa7533ab62 100644 --- a/hack/app-sre/saas_template.yaml +++ b/hack/app-sre/saas_template.yaml @@ -20007,9 +20007,14 @@ objects: 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 diff --git a/hypershift-operator/controllers/platform/aws/controller.go b/hypershift-operator/controllers/platform/aws/controller.go index ea3cd31c6a84..b27f9a4b4a36 100644 --- a/hypershift-operator/controllers/platform/aws/controller.go +++ b/hypershift-operator/controllers/platform/aws/controller.go @@ -302,7 +302,6 @@ func (r *AWSEndpointServiceReconciler) reconcileAWSEndpointServiceStatus(ctx con hasPrivateRouterEPService := false hasPrivateIngressControllerEPService := false for _, eps := range endpointServices.Items { - // If a private-router AWSEndpointService exists, it means that if eps.Name == manifests.PrivateRouterService("").Name { hasPrivateRouterEPService = true } @@ -310,7 +309,7 @@ func (r *AWSEndpointServiceReconciler) reconcileAWSEndpointServiceStatus(ctx con hasPrivateIngressControllerEPService = true } } - // Only if both private router and private ingress controller AWSEndpointServices exist, delete the obsolete one + // Only if both router and private ingress controller AWSEndpointServices exist, delete the obsolete one if hasPrivateRouterEPService && hasPrivateIngressControllerEPService { privateIngressControllerEPService := &hyperv1.AWSEndpointService{ ObjectMeta: metav1.ObjectMeta{ diff --git a/support/testutil/testutil.go b/support/testutil/testutil.go index db84c105c573..88c8b1526271 100644 --- a/support/testutil/testutil.go +++ b/support/testutil/testutil.go @@ -1,6 +1,7 @@ package testutil import ( + "fmt" "io/ioutil" "os" "path/filepath" @@ -8,6 +9,10 @@ import ( "testing" "github.com/google/go-cmp/cmp" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/yaml" ) @@ -90,3 +95,51 @@ func sanitizeFilename(s string) string { } return "zz_fixture_" + result.String() } + +// RuntimeObjectIgnoreRvTypeMeta compares two kubernetes objects, ignoring their resource +// version and TypeMeta. It is what you want 99% of the time. +var RuntimeObjectIgnoreRvTypeMeta = cmp.Comparer(func(x, y runtime.Object) bool { + xCopy := x.DeepCopyObject() + yCopy := y.DeepCopyObject() + CleanRVAndTypeMeta(xCopy) + CleanRVAndTypeMeta(yCopy) + return cmp.Diff(xCopy, yCopy) == "" +}) + +func CleanRVAndTypeMeta(r runtime.Object) { + if metaObject, ok := r.(metav1.Object); ok { + metaObject.SetResourceVersion("") + } + if typeObject, ok := r.(interface{ SetGroupVersionKind(schema.GroupVersionKind) }); ok { + typeObject.SetGroupVersionKind(schema.GroupVersionKind{}) + } + if _, isList := r.(metav1.ListInterface); isList { + objects, err := apimeta.ExtractList(r) + // ExtractList only errors if the list is not a list, so this + // should never error. + if err != nil { + panic(fmt.Sprintf("extract list failed: %v", err)) + } + for _, item := range objects { + CleanRVAndTypeMeta(item) + } + } +} + +// MarshalYamlAndDiff diffs the yaml representation of two runtime.Objects, +// useful for getting a human-readable diff for bigger objects. +func MarshalYamlAndDiff(a, b runtime.Object, t *testing.T) string { + t.Helper() + + CleanRVAndTypeMeta(a) + CleanRVAndTypeMeta(b) + aYAML, err := yaml.Marshal(a) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + bYAML, err := yaml.Marshal(b) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + return cmp.Diff(string(aYAML), string(bYAML)) +} diff --git a/support/testutil/testutil_test.go b/support/testutil/testutil_test.go new file mode 100644 index 000000000000..8fd226b5ddfa --- /dev/null +++ b/support/testutil/testutil_test.go @@ -0,0 +1,66 @@ +package testutil + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func TestCompareRuntimObjectIgnoreRvTypeMeta(t *testing.T) { + tests := []struct { + name string + x runtime.Object + y runtime.Object + expectEquality bool + }{ + { + name: "Different RV, equal", + x: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "1"}}, + y: &corev1.Pod{}, + expectEquality: true, + }, + { + name: "Different obj and different RV, not equal", + x: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "1"}}, + y: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "other"}}, + }, + { + name: "Different TypeMeta, equal", + x: &corev1.Pod{TypeMeta: metav1.TypeMeta{Kind: "Pod"}}, + y: &corev1.Pod{}, + expectEquality: true, + }, + { + name: "Different TypeMeta and object, not equal", + x: &corev1.Pod{TypeMeta: metav1.TypeMeta{Kind: "Pod"}}, + y: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "other"}}, + }, + { + name: "Lists with items with different type meta and rv, equal", + x: &corev1.PodList{Items: []corev1.Pod{ + {TypeMeta: metav1.TypeMeta{Kind: "Secret"}, ObjectMeta: metav1.ObjectMeta{ResourceVersion: "1"}}, + }}, + y: &corev1.PodList{Items: []corev1.Pod{{}}}, + expectEquality: true, + }, + { + name: "Lists with different items, not equal", + x: &corev1.PodList{Items: []corev1.Pod{ + {Spec: corev1.PodSpec{ServiceAccountName: "foo"}}, + }}, + y: &corev1.PodList{Items: []corev1.Pod{{}}}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + diff := cmp.Diff(tc.x, tc.y, RuntimeObjectIgnoreRvTypeMeta) + if diff == "" != tc.expectEquality { + t.Errorf("expectEquality: %t, got diff: %s", tc.expectEquality, diff) + } + }) + } +} diff --git a/test/e2e/chaos_test.go b/test/e2e/chaos_test.go index e8bea9d8db0e..232332be4a83 100644 --- a/test/e2e/chaos_test.go +++ b/test/e2e/chaos_test.go @@ -40,6 +40,8 @@ func TestHAEtcdChaos(t *testing.T) { clusterOpts := globalOpts.DefaultClusterOptions(t) clusterOpts.ControlPlaneAvailabilityPolicy = string(hyperv1.HighlyAvailable) clusterOpts.NodePoolReplicas = 0 + // We have no nodes, enabling private mode is just useless work here + clusterOpts.AWSPlatform.EndpointAccess = string(hyperv1.Public) cluster := e2eutil.CreateCluster(t, ctx, client, &clusterOpts, hyperv1.NonePlatform, globalOpts.ArtifactDir) @@ -63,6 +65,8 @@ func TestEtcdChaos(t *testing.T) { clusterOpts := globalOpts.DefaultClusterOptions(t) clusterOpts.ControlPlaneAvailabilityPolicy = string(hyperv1.SingleReplica) clusterOpts.NodePoolReplicas = 0 + // We have no nodes, enabling private mode is just useless work here + clusterOpts.AWSPlatform.EndpointAccess = string(hyperv1.Public) cluster := e2eutil.CreateCluster(t, ctx, client, &clusterOpts, hyperv1.NonePlatform, globalOpts.ArtifactDir) diff --git a/test/e2e/control_plane_upgrade_test.go b/test/e2e/control_plane_upgrade_test.go index 7618970a86c5..767573c778bb 100644 --- a/test/e2e/control_plane_upgrade_test.go +++ b/test/e2e/control_plane_upgrade_test.go @@ -29,6 +29,27 @@ func TestUpgradeControlPlane(t *testing.T) { clusterOpts.ReleaseImage = globalOpts.PreviousReleaseImage clusterOpts.ControlPlaneAvailabilityPolicy = string(hyperv1.HighlyAvailable) + // TODO @alvaroaleman: Remove once n-1 supports exposing apiserver through route + clusterOpts.BeforeApply = func(o crclient.Object) { + if hcluster, ok := o.(*hyperv1.HostedCluster); ok && hcluster.Spec.Platform.Type == hyperv1.AWSPlatform { + for idx, service := range hcluster.Spec.Services { + if service.Service != hyperv1.APIServer { + continue + } + hcluster.Spec.Services[idx] = hyperv1.ServicePublishingStrategyMapping{ + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{}, + }, + } + if service.Route != nil { + hcluster.Spec.Services[idx].LoadBalancer.Hostname = service.Route.Hostname + } + } + } + } + hostedCluster := e2eutil.CreateCluster(t, ctx, client, &clusterOpts, globalOpts.Platform, globalOpts.ArtifactDir) // Sanity check the cluster by waiting for the nodes to report ready diff --git a/test/e2e/util/fixture.go b/test/e2e/util/fixture.go index 32e51677aa98..b0b742bcd343 100644 --- a/test/e2e/util/fixture.go +++ b/test/e2e/util/fixture.go @@ -97,6 +97,7 @@ func CreateCluster(t *testing.T, ctx context.Context, client crclient.Client, op t.Cleanup(func() { EnsureHCPContainersHaveResourceRequests(t, context.Background(), client, hc) }) t.Cleanup(func() { EnsureNoPodsWithTooHighPriority(t, context.Background(), client, hc) }) t.Cleanup(func() { NoticePreemptionOrFailedScheduling(t, context.Background(), client, hc) }) + t.Cleanup(func() { EnsureAllRoutesUseHCPRouter(t, context.Background(), client, hc) }) return hc } diff --git a/test/e2e/util/util.go b/test/e2e/util/util.go index 158e1cd93ee3..0b2d0afbb588 100644 --- a/test/e2e/util/util.go +++ b/test/e2e/util/util.go @@ -9,6 +9,7 @@ import ( "github.com/go-logr/logr" "github.com/go-logr/zapr" + "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" configv1 "github.com/openshift/api/config/v1" routev1 "github.com/openshift/api/route/v1" @@ -23,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" k8s "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" @@ -30,6 +32,7 @@ import ( crclient "sigs.k8s.io/controller-runtime/pkg/client" hyperv1 "github.com/openshift/hypershift/api/v1alpha1" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/ingress" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests" ) @@ -596,6 +599,32 @@ func EnsureAPIBudget(t *testing.T, ctx context.Context, client crclient.Client, }) } +func EnsureAllRoutesUseHCPRouter(t *testing.T, ctx context.Context, hostClient crclient.Client, hostedCluster *hyperv1.HostedCluster) { + t.Run("EnsureAllRoutesUseHCPRouter", func(t *testing.T) { + for _, svc := range hostedCluster.Spec.Services { + if svc.Service == hyperv1.APIServer && svc.Type != hyperv1.Route { + t.Skip("skipping test because APIServer is not exposed through a route") + } + } + // TODO alvaroaleman: This needs to be fixed up in the CNO + exceptions := sets.NewString("ovnkube-sbdb") + var routes routev1.RouteList + if err := hostClient.List(ctx, &routes, crclient.InNamespace(manifests.HostedControlPlaneNamespace(hostedCluster.Namespace, hostedCluster.Name).Name)); err != nil { + t.Fatalf("failed to list routes: %v", err) + } + for _, route := range routes.Items { + if exceptions.Has(route.Name) { + continue + } + original := route.DeepCopy() + ingress.AddRouteLabel(&route) + if diff := cmp.Diff(route.GetLabels(), original.GetLabels()); diff != "" { + t.Errorf("route %s is missing the label to use the per-HCP router: %s", route.Name, diff) + } + } + }) +} + func getPrometheusToken(ctx context.Context, secretName string, client crclient.Client) ([]byte, error) { if secretName == "" { return createPrometheusToken(ctx) From c20381e2fecfb2b158baa575e91d26e3d445c9a6 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Wed, 27 Jul 2022 13:23:20 -0400 Subject: [PATCH 2/8] KAS service reporting: Report on router service if exposed through route --- .../hostedcontrolplane_controller.go | 8 ++++++- .../hostedcontrolplane/kas/service.go | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 2379bef13809..3850b65a34b1 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -1018,7 +1018,12 @@ func (r *HostedControlPlaneReconciler) reconcileAPIServerServiceStatus(ctx conte } if util.IsPublicHCP(hcp) { - svc := manifests.KubeAPIServerService(hcp.Namespace) + var svc *corev1.Service + if serviceStrategy.Type == hyperv1.Route { + svc = manifests.RouterPublicService(hcp.Namespace) + } else { + svc = manifests.KubeAPIServerService(hcp.Namespace) + } if err = r.Get(ctx, client.ObjectKeyFromObject(svc), svc); err != nil { if apierrors.IsNotFound(err) { err = nil @@ -2441,6 +2446,7 @@ func (r *HostedControlPlaneReconciler) reconcileRouter(ctx context.Context, hcp // the routerCanonicalHostname field, causing external DNS to not create the DNS entry. It doesn't add that field // later for already-admitted routes. if canonicalHostname == "" { + r.Log.Info("Waiting for load balancer to be ready before creating router deployment") return nil } diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go index 0b51ee5e0a57..216ca8f26d8c 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go @@ -73,6 +73,9 @@ func ReconcileService(svc *corev1.Service, strategy *hyperv1.ServicePublishingSt func ReconcileServiceStatus(svc *corev1.Service, strategy *hyperv1.ServicePublishingStrategy, apiServerPort int, messageCollector events.MessageCollector) (host string, port int32, message string, err error) { switch strategy.Type { case hyperv1.LoadBalancer: + if message, err := collectLBMessageIfNotProvisioned(svc, messageCollector); err != nil || message != "" { + return host, port, message, err + } if len(svc.Status.LoadBalancer.Ingress) == 0 { message = fmt.Sprintf("Kubernetes APIServer load balancer is not provisioned; %v since creation.", duration.ShortHumanDuration(time.Since(svc.ObjectMeta.CreationTimestamp.Time))) var eventMessages []string @@ -111,10 +114,31 @@ func ReconcileServiceStatus(svc *corev1.Service, strategy *hyperv1.ServicePublis case hyperv1.Route: host = strategy.Route.Hostname port = int32(apiServerPort) + if message, err := collectLBMessageIfNotProvisioned(svc, messageCollector); err != nil || message != "" { + return host, port, message, err + } } return } +func collectLBMessageIfNotProvisioned(svc *corev1.Service, messageCollector events.MessageCollector) (string, error) { + if len(svc.Status.LoadBalancer.Ingress) > 0 { + return "", nil + + } + message := fmt.Sprintf("Kubernetes APIServer load balancer is not provisioned; %v since creation.", duration.ShortHumanDuration(time.Since(svc.ObjectMeta.CreationTimestamp.Time))) + var eventMessages []string + eventMessages, err := messageCollector.ErrorMessages(svc) + if err != nil { + return message, fmt.Errorf("failed to get events for service %s/%s: %w", svc.Namespace, svc.Name, err) + } + if len(eventMessages) > 0 { + message = fmt.Sprintf("Kubernetes APIServer load balancer is not provisioned: %s", strings.Join(eventMessages, "; ")) + } + + return message, nil +} + func ReconcilePrivateService(svc *corev1.Service, owner *metav1.OwnerReference) error { apiServerPort := 6443 util.EnsureOwnerRef(svc, owner) From 86e8c884cc83cec6e165e5d33a916e63d2eeddc4 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Wed, 27 Jul 2022 14:28:17 -0400 Subject: [PATCH 3/8] Revert "KAS service reporting: Report on router service if exposed through route" This reverts commit 219b149ff42dbd3464994a51469a41d5e5933fea. --- .../hostedcontrolplane_controller.go | 8 +------ .../hostedcontrolplane/kas/service.go | 21 ------------------- 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 3850b65a34b1..2379bef13809 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -1018,12 +1018,7 @@ func (r *HostedControlPlaneReconciler) reconcileAPIServerServiceStatus(ctx conte } if util.IsPublicHCP(hcp) { - var svc *corev1.Service - if serviceStrategy.Type == hyperv1.Route { - svc = manifests.RouterPublicService(hcp.Namespace) - } else { - svc = manifests.KubeAPIServerService(hcp.Namespace) - } + svc := manifests.KubeAPIServerService(hcp.Namespace) if err = r.Get(ctx, client.ObjectKeyFromObject(svc), svc); err != nil { if apierrors.IsNotFound(err) { err = nil @@ -2446,7 +2441,6 @@ func (r *HostedControlPlaneReconciler) reconcileRouter(ctx context.Context, hcp // the routerCanonicalHostname field, causing external DNS to not create the DNS entry. It doesn't add that field // later for already-admitted routes. if canonicalHostname == "" { - r.Log.Info("Waiting for load balancer to be ready before creating router deployment") return nil } diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go index 216ca8f26d8c..84f3220ab0bb 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go @@ -114,31 +114,10 @@ func ReconcileServiceStatus(svc *corev1.Service, strategy *hyperv1.ServicePublis case hyperv1.Route: host = strategy.Route.Hostname port = int32(apiServerPort) - if message, err := collectLBMessageIfNotProvisioned(svc, messageCollector); err != nil || message != "" { - return host, port, message, err - } } return } -func collectLBMessageIfNotProvisioned(svc *corev1.Service, messageCollector events.MessageCollector) (string, error) { - if len(svc.Status.LoadBalancer.Ingress) > 0 { - return "", nil - - } - message := fmt.Sprintf("Kubernetes APIServer load balancer is not provisioned; %v since creation.", duration.ShortHumanDuration(time.Since(svc.ObjectMeta.CreationTimestamp.Time))) - var eventMessages []string - eventMessages, err := messageCollector.ErrorMessages(svc) - if err != nil { - return message, fmt.Errorf("failed to get events for service %s/%s: %w", svc.Namespace, svc.Name, err) - } - if len(eventMessages) > 0 { - message = fmt.Sprintf("Kubernetes APIServer load balancer is not provisioned: %s", strings.Join(eventMessages, "; ")) - } - - return message, nil -} - func ReconcilePrivateService(svc *corev1.Service, owner *metav1.OwnerReference) error { apiServerPort := 6443 util.EnsureOwnerRef(svc, owner) From 79817d4f160c5a1faef44c9830f695e07f9920bc Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Wed, 27 Jul 2022 14:27:57 -0400 Subject: [PATCH 4/8] Default KAS port to 443 when exposing through router This entails having to listen on a different port than what is used externally, as the KAS doesn't run as route and thus can not bind to a port < 1025. --- .../cno/clusternetworkoperator.go | 2 +- .../hostedcontrolplane_controller.go | 4 +-- .../hostedcontrolplane_controller_test.go | 5 +-- .../hostedcontrolplane/ingress/router.go | 4 +++ .../hostedcontrolplane/kas/config.go | 2 +- .../hostedcontrolplane/kas/kubeconfig.go | 2 +- .../hostedcontrolplane/kas/params.go | 8 +++-- .../hostedcontrolplane/kas/service.go | 13 +++---- .../controllers/resources/manifests/config.go | 9 +++++ .../controllers/resources/resources.go | 29 +++++++++++++++ .../hostedcluster/hostedcluster_controller.go | 24 ++++++++++--- .../controllers/nodepool/haproxy.go | 36 +------------------ .../nodepool/nodepool_controller_test.go | 2 +- support/util/networking.go | 7 ++++ 14 files changed, 89 insertions(+), 58 deletions(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/cno/clusternetworkoperator.go b/control-plane-operator/controllers/hostedcontrolplane/cno/clusternetworkoperator.go index 3fc46df37575..de354659989d 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/cno/clusternetworkoperator.go +++ b/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 diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 2379bef13809..e0d9ab209e6d 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -850,7 +850,7 @@ func (r *HostedControlPlaneReconciler) reconcileAPIServerService(ctx context.Con } else if util.IsPrivateHCP(hcp) { apiServerPrivateService := manifests.KubeAPIServerPrivateService(hcp.Namespace) if _, err := createOrUpdate(ctx, r.Client, apiServerPrivateService, func() error { - return kas.ReconcilePrivateService(apiServerPrivateService, p.OwnerReference) + return kas.ReconcilePrivateService(apiServerPrivateService, hcp, p.OwnerReference) }); err != nil { return fmt.Errorf("failed to reconcile API server private service: %w", err) } @@ -1031,7 +1031,7 @@ func (r *HostedControlPlaneReconciler) reconcileAPIServerServiceStatus(ctx conte return kas.ReconcileServiceStatus(svc, serviceStrategy, p.APIServerPort, events.NewMessageCollector(ctx, r.Client)) } - host, port, err = kas.ReconcilePrivateServiceStatus(hcp.Name) + host, port, err = kas.ReconcilePrivateServiceStatus(hcp) return } diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go index 7a315140eef5..9fb16b9f3c41 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go @@ -174,7 +174,7 @@ func TestReconcileAPIServerService(t *testing.T) { { Protocol: corev1.ProtocolTCP, Port: apiPort, - TargetPort: intstr.FromInt(int(apiPort)), + TargetPort: intstr.FromInt(6443), }, }, LoadBalancerSourceRanges: allowCIDRString, @@ -199,9 +199,6 @@ func TestReconcileAPIServerService(t *testing.T) { s.Labels = nil s.Spec.LoadBalancerSourceRanges = nil - - s.Spec.Ports[0].Port = 6443 - s.Spec.Ports[0].TargetPort = intstr.FromInt(6443) })...) } kasPublicRoute := routev1.Route{ diff --git a/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go b/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go index 241812447d8f..54515d36ad22 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go +++ b/control-plane-operator/controllers/hostedcontrolplane/ingress/router.go @@ -366,6 +366,10 @@ func ReconcileRouterService(svc *corev1.Service, ownerRef config.OwnerRef, kasPo foundHTTP := false foundHTTPS := false foundKAS := false + + if kasPort == 443 { + foundKAS = true + } for i, port := range svc.Spec.Ports { switch port.Name { case "http": diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/config.go b/control-plane-operator/controllers/hostedcontrolplane/kas/config.go index 0e671cc42388..41b2625893a2 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/config.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/config.go @@ -112,7 +112,7 @@ func generateConfig(p KubeAPIServerConfigParams, version semver.Version) *kcpv1. KeyFile: path.Join(volumeMounts.Path(kasContainerMain().Name, kasVolumeServerCert().Name), corev1.TLSPrivateKeyKey), }, NamedCertificates: globalconfig.GetConfigNamedCertificates(p.NamedCertificates, kasNamedCertificateMountPathPrefix), - BindAddress: fmt.Sprintf("0.0.0.0:%d", p.APIServerPort), + BindAddress: fmt.Sprintf("0.0.0.0:%d", apiServerListenPort), BindNetwork: "tcp4", CipherSuites: hcpconfig.CipherSuites(p.TLSSecurityProfile), MinTLSVersion: hcpconfig.MinTLSVersion(p.TLSSecurityProfile), diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/kubeconfig.go b/control-plane-operator/controllers/hostedcontrolplane/kas/kubeconfig.go index 72a4b860792f..b546009ededd 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/kubeconfig.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/kubeconfig.go @@ -55,7 +55,7 @@ func InClusterKASReadyURL(namespace string, securePort *int32) string { } func ReconcileLocalhostKubeconfigSecret(secret, cert, ca *corev1.Secret, ownerRef config.OwnerRef, apiServerPort int32) error { - localhostURL := fmt.Sprintf("https://localhost:%d", apiServerPort) + localhostURL := fmt.Sprintf("https://localhost:%d", apiServerListenPort) return reconcileKubeconfig(secret, cert, ca, localhostURL, "", manifests.KubeconfigScopeLocal, ownerRef) } diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/params.go b/control-plane-operator/controllers/hostedcontrolplane/kas/params.go index d10b2639b671..573dde300e94 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/params.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/params.go @@ -69,6 +69,8 @@ type KubeAPIServerServiceParams struct { OwnerReference *metav1.OwnerReference } +const apiServerListenPort = 6443 + func NewKubeAPIServerParams(ctx context.Context, hcp *hyperv1.HostedControlPlane, images map[string]string, externalAPIAddress string, externalAPIPort int32, externalOAuthAddress string, externalOAuthPort int32, setDefaultSecurityContext bool) *KubeAPIServerParams { dns := globalconfig.DNSConfig() globalconfig.ReconcileDNSConfig(dns, hcp) @@ -76,7 +78,6 @@ func NewKubeAPIServerParams(ctx context.Context, hcp *hyperv1.HostedControlPlane ExternalAddress: externalAPIAddress, ExternalPort: externalAPIPort, InternalAddress: fmt.Sprintf("api.%s.hypershift.local", hcp.Name), - InternalPort: 6443, ExternalOAuthAddress: externalOAuthAddress, ExternalOAuthPort: externalOAuthPort, ServiceAccountIssuer: hcp.Spec.IssuerURL, @@ -104,6 +105,7 @@ func NewKubeAPIServerParams(ctx context.Context, hcp *hyperv1.HostedControlPlane } params.AdvertiseAddress = util.AdvertiseAddressWithDefault(hcp, config.DefaultAdvertiseAddress) params.APIServerPort = util.APIPortWithDefault(hcp, config.DefaultAPIServerPort) + params.InternalPort = util.APIPortWithDefault(hcp, config.DefaultAPIServerPort) if _, ok := hcp.Annotations[hyperv1.PortierisImageAnnotation]; ok { params.Images.Portieris = hcp.Annotations[hyperv1.PortierisImageAnnotation] } @@ -123,7 +125,7 @@ func NewKubeAPIServerParams(ctx context.Context, hcp *hyperv1.HostedControlPlane ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Scheme: corev1.URISchemeHTTPS, - Port: intstr.FromInt(int(params.APIServerPort)), + Port: intstr.FromInt(int(apiServerListenPort)), Path: "livez?exclude=etcd", }, }, @@ -224,7 +226,7 @@ func NewKubeAPIServerParams(ctx context.Context, hcp *hyperv1.HostedControlPlane ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Scheme: corev1.URISchemeHTTPS, - Port: intstr.FromInt(int(params.APIServerPort)), + Port: intstr.FromInt(int(apiServerListenPort)), Path: "readyz", }, }, diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go index 84f3220ab0bb..7ae6387684eb 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go @@ -14,6 +14,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/v1alpha1" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/ingress" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" + "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/events" "github.com/openshift/hypershift/support/util" ) @@ -40,7 +41,7 @@ func ReconcileService(svc *corev1.Service, strategy *hyperv1.ServicePublishingSt } portSpec.Port = int32(apiServerPort) portSpec.Protocol = corev1.ProtocolTCP - portSpec.TargetPort = intstr.FromInt(apiServerPort) + portSpec.TargetPort = intstr.FromInt(apiServerListenPort) if svc.Annotations == nil { svc.Annotations = map[string]string{} } @@ -118,8 +119,8 @@ func ReconcileServiceStatus(svc *corev1.Service, strategy *hyperv1.ServicePublis return } -func ReconcilePrivateService(svc *corev1.Service, owner *metav1.OwnerReference) error { - apiServerPort := 6443 +func ReconcilePrivateService(svc *corev1.Service, hcp *hyperv1.HostedControlPlane, owner *metav1.OwnerReference) error { + apiServerPort := util.APIPortWithDefault(hcp, config.DefaultAPIServerPort) util.EnsureOwnerRef(svc, owner) svc.Spec.Selector = kasLabels() var portSpec corev1.ServicePort @@ -130,7 +131,7 @@ func ReconcilePrivateService(svc *corev1.Service, owner *metav1.OwnerReference) } portSpec.Port = int32(apiServerPort) portSpec.Protocol = corev1.ProtocolTCP - portSpec.TargetPort = intstr.FromInt(apiServerPort) + portSpec.TargetPort = intstr.FromInt(apiServerListenPort) svc.Spec.Type = corev1.ServiceTypeLoadBalancer if svc.Annotations == nil { svc.Annotations = map[string]string{} @@ -141,8 +142,8 @@ func ReconcilePrivateService(svc *corev1.Service, owner *metav1.OwnerReference) return nil } -func ReconcilePrivateServiceStatus(hcpName string) (host string, port int32, err error) { - return fmt.Sprintf("api.%s.hypershift.local", hcpName), 6443, nil +func ReconcilePrivateServiceStatus(hcp *hyperv1.HostedControlPlane) (host string, port int32, err error) { + return fmt.Sprintf("api.%s.hypershift.local", hcp.Name), util.APIPortWithDefault(hcp, config.DefaultAPIServerPort), nil } func ReconcileRoute(route *routev1.Route, hostname string) { diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/config.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/config.go index b201425d92cd..65757a4d9f4d 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/config.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/config.go @@ -13,3 +13,12 @@ func InstallConfigConfigMap() *corev1.ConfigMap { }, } } + +func APIServerEndpoints() *corev1.Endpoints { + return &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "kubernetes", + }, + } +} diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go index 56f53c9fb3d6..a592c9dd95dd 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go @@ -4,6 +4,8 @@ import ( "context" "crypto/md5" "fmt" + "net/url" + "strconv" prometheusoperatorv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" @@ -75,6 +77,7 @@ type reconciler struct { oauthPort int32 versions map[string]string operateOnReleaseImage string + apiServerPort int32 } // eventHandler is the handler used throughout. As this controller reconciles all kind of different resources @@ -90,6 +93,19 @@ func Setup(opts *operator.HostedClusterConfigOperatorConfig) error { if err := imageregistryv1.AddToScheme(opts.Manager.GetScheme()); err != nil { return fmt.Errorf("failed to add to scheme: %w", err) } + + apiServerPort := int32(443) + apiServerURL, err := url.Parse(opts.Manager.GetConfig().Host) + if err != nil { + return fmt.Errorf("failed to parse apiserver host %s as url: %w", opts.Manager.GetConfig().Host, err) + } + if p := apiServerURL.Port(); p != "" { + numericPort, err := strconv.Atoi(p) + if err != nil { + return fmt.Errorf("failed to parse apiserver port string %s as int: %w", p, err) + } + apiServerPort = int32(numericPort) + } c, err := controller.New(ControllerName, opts.Manager, controller.Options{Reconciler: &reconciler{ client: opts.Manager.GetClient(), CreateOrUpdateProvider: opts.TargetCreateOrUpdateProvider, @@ -106,6 +122,7 @@ func Setup(opts *operator.HostedClusterConfigOperatorConfig) error { oauthPort: opts.OAuthPort, versions: opts.Versions, operateOnReleaseImage: opts.OperateOnReleaseImage, + apiServerPort: apiServerPort, }}) if err != nil { return fmt.Errorf("failed to construct controller: %w", err) @@ -181,6 +198,18 @@ func (r *reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result errs = append(errs, fmt.Errorf("failed to reconcile crds: %w", err)) } + log.Info("reconciling kubernetes.default endpoints") + endpoints := manifests.APIServerEndpoints() + if _, err := r.CreateOrUpdate(ctx, r.client, endpoints, func() error { + if len(endpoints.Subsets) == 0 || len(endpoints.Subsets[0].Ports) == 0 { + return nil + } + endpoints.Subsets[0].Ports[0].Port = r.apiServerPort + return nil + }); err != nil { + errs = append(errs, fmt.Errorf("failed to reconcile kubernetes.default endpoints: %w", err)) + } + log.Info("reconciling guest cluster alert rules") if err := r.reconcileGuestClusterAlertRules(ctx); err != nil { errs = append(errs, fmt.Errorf("failed to reconcile guest cluster alert rules: %w", err)) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 0effb08ce95a..8413e2a29c72 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -4309,6 +4309,26 @@ func isUpgradeable(hcluster *hyperv1.HostedCluster) (bool, string, error) { // defaultAPIPortIfNeeded defaults the apiserver port on Azure management clusters as a workaround // for https://bugzilla.redhat.com/show_bug.cgi?id=2060650: Azure LBs with port 6443 don't work func (r *HostedClusterReconciler) defaultAPIPortIfNeeded(ctx context.Context, hcluster *hyperv1.HostedCluster) error { + if hcluster.Spec.Networking.APIServer != nil && hcluster.Spec.Networking.APIServer.Port != nil { + return nil + } + for _, publishingStrategy := range hcluster.Spec.Services { + if publishingStrategy.Service != hyperv1.APIServer { + continue + } + if publishingStrategy.Type == hyperv1.Route { + if hcluster.Spec.Networking.APIServer == nil { + hcluster.Spec.Networking.APIServer = &hyperv1.APIServerNetworking{} + } + + hcluster.Spec.Networking.APIServer.Port = k8sutilspointer.Int32(443) + if err := r.Update(ctx, hcluster); err != nil { + return fmt.Errorf("failed to update hostedcluster after defaulting the apiserver port: %w", err) + } + } + break + } + if !r.ManagementClusterCapabilities.Has(capabilities.CapabilityInfrastructure) { return nil } @@ -4324,10 +4344,6 @@ func (r *HostedClusterReconciler) defaultAPIPortIfNeeded(ctx context.Context, hc hcluster.Spec.Networking.APIServer = &hyperv1.APIServerNetworking{} } - if hcluster.Spec.Networking.APIServer.Port != nil { - return nil - } - hcluster.Spec.Networking.APIServer.Port = k8sutilspointer.Int32Ptr(7443) if err := r.Update(ctx, hcluster); err != nil { return fmt.Errorf("failed to update hostedcluster after defaulting the apiserver port: %w", err) diff --git a/hypershift-operator/controllers/nodepool/haproxy.go b/hypershift-operator/controllers/nodepool/haproxy.go index b394bec051d6..b2c2568f1c6e 100644 --- a/hypershift-operator/controllers/nodepool/haproxy.go +++ b/hypershift-operator/controllers/nodepool/haproxy.go @@ -6,7 +6,6 @@ import ( "embed" "fmt" "html/template" - "net/url" "strconv" "strings" @@ -27,9 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/client-go/tools/clientcmd" "k8s.io/utils/pointer" - crclient "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -62,40 +59,9 @@ func (r *NodePoolReconciler) isHAProxyIgnitionConfigManaged(ctx context.Context, func (r *NodePoolReconciler) reconcileHAProxyIgnitionConfig(ctx context.Context, releaseImage *releaseinfo.ReleaseImage, hcluster *hyperv1.HostedCluster, controlPlaneOperatorImage string) (cfg string, missing bool, err error) { var apiServerExternalAddress string - var apiServerExternalPort int32 + apiServerExternalPort := util.APIPortWithDefaultFromHostedCluster(hcluster, config.DefaultAPIServerPort) if util.IsPrivateHC(hcluster) { apiServerExternalAddress = fmt.Sprintf("api.%s.hypershift.local", hcluster.Name) - apiServerExternalPort = 6443 - } else { - if hcluster.Status.KubeConfig == nil { - return "", true, nil - } - var kubeconfig corev1.Secret - if err := r.Get(ctx, crclient.ObjectKey{Namespace: hcluster.Namespace, Name: hcluster.Status.KubeConfig.Name}, &kubeconfig); err != nil { - return "", true, fmt.Errorf("failed to get kubeconfig: %w", err) - } - kubeconfigBytes, found := kubeconfig.Data["kubeconfig"] - if !found { - return "", true, fmt.Errorf("kubeconfig secret %s has no 'kubeconfig' key", crclient.ObjectKeyFromObject(&kubeconfig)) - } - restConfig, err := clientcmd.RESTConfigFromKubeConfig(kubeconfigBytes) - if err != nil { - return "", true, fmt.Errorf("failed to parse kubeconfig from secret %s: %w", crclient.ObjectKeyFromObject(&kubeconfig), err) - } - hostURL, err := url.Parse(restConfig.Host) - if err != nil { - return "", true, fmt.Errorf("failed to parse host in kubeconfig from secret %s as url: %w", crclient.ObjectKeyFromObject(&kubeconfig), err) - } - apiServerExternalAddress = hostURL.Hostname() - - apiServerExternalPort = 443 - if portFromKubeconfig := hostURL.Port(); portFromKubeconfig != "" { - numericPortFromKubeconfig, err := strconv.Atoi(portFromKubeconfig) - if err != nil { - return "", true, fmt.Errorf("failed to parse port string %q from kubeconfig %s as int: %w", portFromKubeconfig, crclient.ObjectKeyFromObject(&kubeconfig), err) - } - apiServerExternalPort = int32(numericPortFromKubeconfig) - } } haProxyImage, ok := releaseImage.ComponentImages()[haProxyRouterImageName] diff --git a/hypershift-operator/controllers/nodepool/nodepool_controller_test.go b/hypershift-operator/controllers/nodepool/nodepool_controller_test.go index 9637b4f6a298..3258f3483ef8 100644 --- a/hypershift-operator/controllers/nodepool/nodepool_controller_test.go +++ b/hypershift-operator/controllers/nodepool/nodepool_controller_test.go @@ -422,7 +422,7 @@ spec: overwrite: true path: /usr/local/bin/teardown-apiserver-ip.sh - contents: - source: data:text/plain;charset=utf-8;base64,Z2xvYmFsCiAgbWF4Y29ubiA3MDAwCiAgbG9nIHN0ZG91dCBsb2NhbDAKICBsb2cgc3Rkb3V0IGxvY2FsMSBub3RpY2UKCmRlZmF1bHRzCiAgbW9kZSB0Y3AKICB0aW1lb3V0IGNsaWVudCAxMG0KICB0aW1lb3V0IHNlcnZlciAxMG0KICB0aW1lb3V0IGNvbm5lY3QgMTBzCiAgdGltZW91dCBjbGllbnQtZmluIDVzCiAgdGltZW91dCBzZXJ2ZXItZmluIDVzCiAgdGltZW91dCBxdWV1ZSA1cwogIHJldHJpZXMgMwoKZnJvbnRlbmQgbG9jYWxfYXBpc2VydmVyCiAgYmluZCAxNzIuMjAuMC4xOjY0NDMKICBsb2cgZ2xvYmFsCiAgbW9kZSB0Y3AKICBvcHRpb24gdGNwbG9nCiAgZGVmYXVsdF9iYWNrZW5kIHJlbW90ZV9hcGlzZXJ2ZXIKCmJhY2tlbmQgcmVtb3RlX2FwaXNlcnZlcgogIG1vZGUgdGNwCiAgbG9nIGdsb2JhbAogIG9wdGlvbiBodHRwY2hrIEdFVCAvdmVyc2lvbgogIG9wdGlvbiBsb2ctaGVhbHRoLWNoZWNrcwogIGRlZmF1bHQtc2VydmVyIGludGVyIDEwcyBmYWxsIDMgcmlzZSAzCiAgc2VydmVyIGNvbnRyb2xwbGFuZSBsb2NhbGhvc3Q6ODA4MAo= + source: data:text/plain;charset=utf-8;base64,Z2xvYmFsCiAgbWF4Y29ubiA3MDAwCiAgbG9nIHN0ZG91dCBsb2NhbDAKICBsb2cgc3Rkb3V0IGxvY2FsMSBub3RpY2UKCmRlZmF1bHRzCiAgbW9kZSB0Y3AKICB0aW1lb3V0IGNsaWVudCAxMG0KICB0aW1lb3V0IHNlcnZlciAxMG0KICB0aW1lb3V0IGNvbm5lY3QgMTBzCiAgdGltZW91dCBjbGllbnQtZmluIDVzCiAgdGltZW91dCBzZXJ2ZXItZmluIDVzCiAgdGltZW91dCBxdWV1ZSA1cwogIHJldHJpZXMgMwoKZnJvbnRlbmQgbG9jYWxfYXBpc2VydmVyCiAgYmluZCAxNzIuMjAuMC4xOjY0NDMKICBsb2cgZ2xvYmFsCiAgbW9kZSB0Y3AKICBvcHRpb24gdGNwbG9nCiAgZGVmYXVsdF9iYWNrZW5kIHJlbW90ZV9hcGlzZXJ2ZXIKCmJhY2tlbmQgcmVtb3RlX2FwaXNlcnZlcgogIG1vZGUgdGNwCiAgbG9nIGdsb2JhbAogIG9wdGlvbiBodHRwY2hrIEdFVCAvdmVyc2lvbgogIG9wdGlvbiBsb2ctaGVhbHRoLWNoZWNrcwogIGRlZmF1bHQtc2VydmVyIGludGVyIDEwcyBmYWxsIDMgcmlzZSAzCiAgc2VydmVyIGNvbnRyb2xwbGFuZSA6NjQ0Mwo= mode: 420 overwrite: true path: /etc/kubernetes/apiserver-proxy-config/haproxy.cfg diff --git a/support/util/networking.go b/support/util/networking.go index c01b7fb824e3..10518857ad69 100644 --- a/support/util/networking.go +++ b/support/util/networking.go @@ -66,6 +66,13 @@ func APIPortWithDefault(hcp *hyperv1.HostedControlPlane, defaultValue int32) int return defaultValue } +func APIPortWithDefaultFromHostedCluster(hc *hyperv1.HostedCluster, defaultValue int32) int32 { + if hc.Spec.Networking.APIServer != nil && hc.Spec.Networking.APIServer.Port != nil { + return *hc.Spec.Networking.APIServer.Port + } + return defaultValue +} + func AdvertiseAddress(hcp *hyperv1.HostedControlPlane) *string { if hcp != nil && hcp.Spec.Networking.APIServer != nil { return hcp.Spec.Networking.APIServer.AdvertiseAddress From ade79db1566c89913f1ef71e27d903adfe9b1125 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Fri, 29 Jul 2022 14:23:01 -0400 Subject: [PATCH 5/8] Create router earlier so lb services get provisioned asap --- .../hostedcontrolplane_controller.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index e0d9ab209e6d..310786b1c81f 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -556,6 +556,14 @@ func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControl createOrUpdate := r.createOrUpdate(hostedControlPlane) + kasServiceStrategy := servicePublishingStrategyByType(hostedControlPlane, hyperv1.APIServer) + if util.IsPrivateHCP(hostedControlPlane) || kasServiceStrategy.Type == hyperv1.Route { + r.Log.Info("Reconciling router") + if err = r.reconcileRouter(ctx, hostedControlPlane, releaseImage, createOrUpdate); err != nil { + return fmt.Errorf("failed to reconcile router: %w", err) + } + } + r.Log.Info("Reconciling autoscaler") if err := r.reconcileAutoscaler(ctx, hostedControlPlane, releaseImage.ComponentImages(), createOrUpdate); err != nil { return fmt.Errorf("failed to reconcile autoscaler: %w", err) @@ -733,7 +741,6 @@ func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControl } // Reconcile router - kasServiceStrategy := servicePublishingStrategyByType(hostedControlPlane, hyperv1.APIServer) if util.IsPrivateHCP(hostedControlPlane) { r.Log.Info("Removing private IngressController") // Ensure that if an ingress controller exists from a previous version, it is removed @@ -741,12 +748,6 @@ func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControl return fmt.Errorf("failed to reconcile private ingresscontroller: %w", err) } } - if util.IsPrivateHCP(hostedControlPlane) || kasServiceStrategy.Type == hyperv1.Route { - r.Log.Info("Reconciling router") - if err = r.reconcileRouter(ctx, hostedControlPlane, releaseImage, createOrUpdate); err != nil { - return fmt.Errorf("failed to reconcile router: %w", err) - } - } // Reconcile hosted cluster config operator r.Log.Info("Reconciling Hosted Cluster Config Operator") From d472029e02066205f358df2ba5ad75e5e70dfe26 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Fri, 29 Jul 2022 14:28:09 -0400 Subject: [PATCH 6/8] Revert "Revert "KAS service reporting: Report on router service if exposed through route"" This reverts commit 9f528741e41f49e5538e6c4441b0e8613418f04d. --- .../hostedcontrolplane_controller.go | 8 ++++- .../hostedcontrolplane/kas/service.go | 34 ++++++++++++------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 310786b1c81f..80e1ac9428d1 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -1019,7 +1019,12 @@ func (r *HostedControlPlaneReconciler) reconcileAPIServerServiceStatus(ctx conte } if util.IsPublicHCP(hcp) { - svc := manifests.KubeAPIServerService(hcp.Namespace) + var svc *corev1.Service + if serviceStrategy.Type == hyperv1.Route { + svc = manifests.RouterPublicService(hcp.Namespace) + } else { + svc = manifests.KubeAPIServerService(hcp.Namespace) + } if err = r.Get(ctx, client.ObjectKeyFromObject(svc), svc); err != nil { if apierrors.IsNotFound(err) { err = nil @@ -2442,6 +2447,7 @@ func (r *HostedControlPlaneReconciler) reconcileRouter(ctx context.Context, hcp // the routerCanonicalHostname field, causing external DNS to not create the DNS entry. It doesn't add that field // later for already-admitted routes. if canonicalHostname == "" { + r.Log.Info("Waiting for load balancer to be ready before creating router deployment") return nil } diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go index 7ae6387684eb..4d62e55400e6 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go @@ -77,19 +77,6 @@ func ReconcileServiceStatus(svc *corev1.Service, strategy *hyperv1.ServicePublis if message, err := collectLBMessageIfNotProvisioned(svc, messageCollector); err != nil || message != "" { return host, port, message, err } - if len(svc.Status.LoadBalancer.Ingress) == 0 { - message = fmt.Sprintf("Kubernetes APIServer load balancer is not provisioned; %v since creation.", duration.ShortHumanDuration(time.Since(svc.ObjectMeta.CreationTimestamp.Time))) - var eventMessages []string - eventMessages, err = messageCollector.ErrorMessages(svc) - if err != nil { - err = fmt.Errorf("failed to get events for service %s/%s: %w", svc.Namespace, svc.Name, err) - return - } - if len(eventMessages) > 0 { - message = fmt.Sprintf("Kubernetes APIServer load balancer is not provisioned: %s", strings.Join(eventMessages, "; ")) - } - return - } port = int32(apiServerPort) switch { case strategy.LoadBalancer != nil && strategy.LoadBalancer.Hostname != "": @@ -113,6 +100,9 @@ func ReconcileServiceStatus(svc *corev1.Service, strategy *hyperv1.ServicePublis port = svc.Spec.Ports[0].NodePort host = strategy.NodePort.Address case hyperv1.Route: + if message, err := collectLBMessageIfNotProvisioned(svc, messageCollector); err != nil || message != "" { + return host, port, message, err + } host = strategy.Route.Hostname port = int32(apiServerPort) } @@ -142,6 +132,24 @@ func ReconcilePrivateService(svc *corev1.Service, hcp *hyperv1.HostedControlPlan return nil } +func collectLBMessageIfNotProvisioned(svc *corev1.Service, messageCollector events.MessageCollector) (string, error) { + if len(svc.Status.LoadBalancer.Ingress) > 0 { + return "", nil + + } + message := fmt.Sprintf("Kubernetes APIServer load balancer is not provisioned; %v since creation.", duration.ShortHumanDuration(time.Since(svc.ObjectMeta.CreationTimestamp.Time))) + var eventMessages []string + eventMessages, err := messageCollector.ErrorMessages(svc) + if err != nil { + return message, fmt.Errorf("failed to get events for service %s/%s: %w", svc.Namespace, svc.Name, err) + } + if len(eventMessages) > 0 { + message = fmt.Sprintf("Kubernetes APIServer load balancer is not provisioned: %s", strings.Join(eventMessages, "; ")) + } + + return message, nil +} + func ReconcilePrivateServiceStatus(hcp *hyperv1.HostedControlPlane) (host string, port int32, err error) { return fmt.Sprintf("api.%s.hypershift.local", hcp.Name), util.APIPortWithDefault(hcp, config.DefaultAPIServerPort), nil } From 37787c92bc6b4f490be645d1136117ec54e07f59 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Tue, 2 Aug 2022 09:16:22 -0400 Subject: [PATCH 7/8] Rebase --- api/fixtures/example.go | 2 +- test/e2e/chaos_test.go | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/api/fixtures/example.go b/api/fixtures/example.go index 7f7d7d23318e..fa8e954f30e3 100644 --- a/api/fixtures/example.go +++ b/api/fixtures/example.go @@ -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{ diff --git a/test/e2e/chaos_test.go b/test/e2e/chaos_test.go index 232332be4a83..e8bea9d8db0e 100644 --- a/test/e2e/chaos_test.go +++ b/test/e2e/chaos_test.go @@ -40,8 +40,6 @@ func TestHAEtcdChaos(t *testing.T) { clusterOpts := globalOpts.DefaultClusterOptions(t) clusterOpts.ControlPlaneAvailabilityPolicy = string(hyperv1.HighlyAvailable) clusterOpts.NodePoolReplicas = 0 - // We have no nodes, enabling private mode is just useless work here - clusterOpts.AWSPlatform.EndpointAccess = string(hyperv1.Public) cluster := e2eutil.CreateCluster(t, ctx, client, &clusterOpts, hyperv1.NonePlatform, globalOpts.ArtifactDir) @@ -65,8 +63,6 @@ func TestEtcdChaos(t *testing.T) { clusterOpts := globalOpts.DefaultClusterOptions(t) clusterOpts.ControlPlaneAvailabilityPolicy = string(hyperv1.SingleReplica) clusterOpts.NodePoolReplicas = 0 - // We have no nodes, enabling private mode is just useless work here - clusterOpts.AWSPlatform.EndpointAccess = string(hyperv1.Public) cluster := e2eutil.CreateCluster(t, ctx, client, &clusterOpts, hyperv1.NonePlatform, globalOpts.ArtifactDir) From 70f040699e52bc6ab8b985f93787af0a72e8fdb9 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Tue, 2 Aug 2022 09:24:09 -0400 Subject: [PATCH 8/8] Be a bit more explicit --- .../hostedcontrolplane_controller.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 80e1ac9428d1..3a899ef53788 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -556,6 +556,15 @@ func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControl createOrUpdate := r.createOrUpdate(hostedControlPlane) + if util.IsPrivateHCP(hostedControlPlane) { + r.Log.Info("Removing private IngressController") + // Ensure that if an ingress controller exists from a previous version, it is removed + if err = r.reconcilePrivateIngressController(ctx, hostedControlPlane); err != nil { + return fmt.Errorf("failed to reconcile private ingresscontroller: %w", err) + } + } + + // Reconcile router kasServiceStrategy := servicePublishingStrategyByType(hostedControlPlane, hyperv1.APIServer) if util.IsPrivateHCP(hostedControlPlane) || kasServiceStrategy.Type == hyperv1.Route { r.Log.Info("Reconciling router") @@ -740,15 +749,6 @@ func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControl return fmt.Errorf("failed to reconcile ingress operator: %w", err) } - // Reconcile router - if util.IsPrivateHCP(hostedControlPlane) { - r.Log.Info("Removing private IngressController") - // Ensure that if an ingress controller exists from a previous version, it is removed - if err = r.reconcilePrivateIngressController(ctx, hostedControlPlane); err != nil { - return fmt.Errorf("failed to reconcile private ingresscontroller: %w", err) - } - } - // Reconcile hosted cluster config operator r.Log.Info("Reconciling Hosted Cluster Config Operator") if err = r.reconcileHostedClusterConfigOperator(ctx, hostedControlPlane, releaseImage, infraStatus, createOrUpdate); err != nil { @@ -848,7 +848,7 @@ func (r *HostedControlPlaneReconciler) reconcileAPIServerService(ctx context.Con return fmt.Errorf("failed to reconcile apiserver route %s: %w", route.Name, err) } - } else if util.IsPrivateHCP(hcp) { + } else if serviceStrategy.Type == hyperv1.LoadBalancer && util.IsPrivateHCP(hcp) { apiServerPrivateService := manifests.KubeAPIServerPrivateService(hcp.Namespace) if _, err := createOrUpdate(ctx, r.Client, apiServerPrivateService, func() error { return kas.ReconcilePrivateService(apiServerPrivateService, hcp, p.OwnerReference)