Merge pull request #1022 from sadasu/custom-dns-4.15

[release-4.15] CORS-3169: Add Ingress LB IPs to Infra CR and set DNS unmanaged when BYO DNS is enabled
openshift · Feb 2, 2024 · 0106cb9 · 0106cb9
2 parents 611011b + 712833e
commit 0106cb9
Show file tree

Hide file tree

Showing 84 changed files with 9,599 additions and 396 deletions.
diff --git a/go.mod b/go.mod
@@ -30,6 +30,7 @@ require (
 	github.com/summerwind/h2spec v0.0.0-20200804131034-70ac22940108
 	github.com/tcnksm/go-httpstat v0.2.1-0.20191008022543-e866bb274419
 	go.uber.org/zap v1.25.0
+	golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e
 	golang.org/x/time v0.3.0
 	google.golang.org/api v0.114.0
 	google.golang.org/grpc v1.54.0
@@ -115,7 +116,6 @@ require (
 	go.opencensus.io v0.24.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	golang.org/x/crypto v0.14.0 // indirect
-	golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e // indirect
 	golang.org/x/net v0.17.0 // indirect
 	golang.org/x/oauth2 v0.8.0 // indirect
 	golang.org/x/sys v0.13.0 // indirect
@@ -143,6 +143,6 @@ require (
 // github.com/operator-framework/operator-sdk.
 replace (
 	bitbucket.org/ww/goautoneg => github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d
-	github.com/openshift/api => github.com/openshift/api v0.0.0-20231113114413-39964e6af314
+	github.com/openshift/api => github.com/openshift/api v0.0.0-20240131192415-e18b9cc8aa8b
 	k8s.io/client-go => k8s.io/client-go v0.28.0
 )
diff --git a/go.sum b/go.sum
@@ -918,8 +918,8 @@ github.com/opencontainers/runtime-spec v0.1.2-0.20190507144316-5b71a03e2700/go.m
 github.com/opencontainers/runtime-spec v0.1.2-0.20190618234442-a950415649c7/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
 github.com/opencontainers/runtime-spec v1.0.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
 github.com/opencontainers/runtime-tools v0.0.0-20181011054405-1d69bd0f9c39/go.mod h1:r3f7wjNzSs2extwzU3Y+6pKfobzPh+kKFJ3ofN+3nfs=
-github.com/openshift/api v0.0.0-20231113114413-39964e6af314 h1:0KbLGBd//gKgOEQLxvJsQPZ7h9N0R7QFolBKoJ/YkHQ=
-github.com/openshift/api v0.0.0-20231113114413-39964e6af314/go.mod h1:qNtV0315F+f8ld52TLtPvrfivZpdimOzTi3kn9IVbtU=
+github.com/openshift/api v0.0.0-20240131192415-e18b9cc8aa8b h1:+oWnXf9QvOlVG4Y5NJ18iiWMbbwsp5CND+jjM3M/qRA=
+github.com/openshift/api v0.0.0-20240131192415-e18b9cc8aa8b/go.mod h1:qNtV0315F+f8ld52TLtPvrfivZpdimOzTi3kn9IVbtU=
 github.com/openshift/build-machinery-go v0.0.0-20200211121458-5e3d6e570160/go.mod h1:1CkcsT3aVebzRBzVTSbiKSkJMsC/CASqxesfqEMfJEc=
 github.com/openshift/client-go v0.0.0-20200116152001-92a2713fa240/go.mod h1:4riOwdj99Hd/q+iAcJZfNCsQQQMwURnZV6RL4WHYS5w=
 github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 h1:66uaqNwA+qYyQDwsMWUfjjau8ezmg1dzCqub13KZOcE=

diff --git a/manifests/00-cluster-role.yaml b/manifests/00-cluster-role.yaml
@@ -119,6 +119,13 @@ rules:
   - list
   - watch
 
+- apiGroups:
+  - config.openshift.io
+  resources:
+  - infrastructures/status
+  verbs:
+  - update
+
 - apiGroups:
   - config.openshift.io
   resources:

diff --git a/manifests/00-custom-resource-definition.yaml b/manifests/00-custom-resource-definition.yaml
@@ -742,7 +742,7 @@ spec:
                   description: "tlsSecurityProfile specifies settings for TLS connections for ingresscontrollers. \n If unset, the default is based on the apiservers.config.openshift.io/cluster resource. \n Note that when using the Old, Intermediate, and Modern profile types, the effective profile configuration is subject to change between releases. For example, given a specification to use the Intermediate profile deployed on release X.Y.Z, an upgrade to release X.Y.Z+1 may cause a new profile configuration to be applied to the ingress controller, resulting in a rollout."
                   properties:
                     custom:
-                      description: "custom is a user-defined TLS security profile. Be extremely careful using a custom profile as invalid configurations can be catastrophic. An example custom profile looks like this: \n ciphers: - ECDHE-ECDSA-CHACHA20-POLY1305 - ECDHE-RSA-CHACHA20-POLY1305 - ECDHE-RSA-AES128-GCM-SHA256 - ECDHE-ECDSA-AES128-GCM-SHA256 minTLSVersion: TLSv1.1"
+                      description: "custom is a user-defined TLS security profile. Be extremely careful using a custom profile as invalid configurations can be catastrophic. An example custom profile looks like this: \n ciphers: - ECDHE-ECDSA-CHACHA20-POLY1305 - ECDHE-RSA-CHACHA20-POLY1305 - ECDHE-RSA-AES128-GCM-SHA256 - ECDHE-ECDSA-AES128-GCM-SHA256 minTLSVersion: VersionTLS11"
                       nullable: true
                       properties:
                         ciphers:
@@ -751,7 +751,7 @@ spec:
                             type: string
                           type: array
                         minTLSVersion:
-                          description: "minTLSVersion is used to specify the minimal version of the TLS protocol that is negotiated during the TLS handshake. For example, to use TLS versions 1.1, 1.2 and 1.3 (yaml): \n minTLSVersion: TLSv1.1 \n NOTE: currently the highest minTLSVersion allowed is VersionTLS12"
+                          description: "minTLSVersion is used to specify the minimal version of the TLS protocol that is negotiated during the TLS handshake. For example, to use TLS versions 1.1, 1.2 and 1.3 (yaml): \n minTLSVersion: VersionTLS11 \n NOTE: currently the highest minTLSVersion allowed is VersionTLS12"
                           enum:
                             - VersionTLS10
                             - VersionTLS11
@@ -760,15 +760,15 @@ spec:
                           type: string
                       type: object
                     intermediate:
-                      description: "intermediate is a TLS security profile based on: \n https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29 \n and looks like this (yaml): \n ciphers: - TLS_AES_128_GCM_SHA256 - TLS_AES_256_GCM_SHA384 - TLS_CHACHA20_POLY1305_SHA256 - 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 minTLSVersion: TLSv1.2"
+                      description: "intermediate is a TLS security profile based on: \n https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29 \n and looks like this (yaml): \n ciphers: - TLS_AES_128_GCM_SHA256 - TLS_AES_256_GCM_SHA384 - TLS_CHACHA20_POLY1305_SHA256 - 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 minTLSVersion: VersionTLS12"
                       nullable: true
                       type: object
                     modern:
-                      description: "modern is a TLS security profile based on: \n https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility \n and looks like this (yaml): \n ciphers: - TLS_AES_128_GCM_SHA256 - TLS_AES_256_GCM_SHA384 - TLS_CHACHA20_POLY1305_SHA256 minTLSVersion: TLSv1.3 \n NOTE: Currently unsupported."
+                      description: "modern is a TLS security profile based on: \n https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility \n and looks like this (yaml): \n ciphers: - TLS_AES_128_GCM_SHA256 - TLS_AES_256_GCM_SHA384 - TLS_CHACHA20_POLY1305_SHA256 minTLSVersion: VersionTLS13 \n NOTE: Currently unsupported."
                       nullable: true
                       type: object
                     old:
-                      description: "old is a TLS security profile based on: \n https://wiki.mozilla.org/Security/Server_Side_TLS#Old_backward_compatibility \n and looks like this (yaml): \n ciphers: - TLS_AES_128_GCM_SHA256 - TLS_AES_256_GCM_SHA384 - TLS_CHACHA20_POLY1305_SHA256 - 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 - DHE-RSA-CHACHA20-POLY1305 - ECDHE-ECDSA-AES128-SHA256 - ECDHE-RSA-AES128-SHA256 - ECDHE-ECDSA-AES128-SHA - ECDHE-RSA-AES128-SHA - ECDHE-ECDSA-AES256-SHA384 - ECDHE-RSA-AES256-SHA384 - ECDHE-ECDSA-AES256-SHA - ECDHE-RSA-AES256-SHA - DHE-RSA-AES128-SHA256 - DHE-RSA-AES256-SHA256 - AES128-GCM-SHA256 - AES256-GCM-SHA384 - AES128-SHA256 - AES256-SHA256 - AES128-SHA - AES256-SHA - DES-CBC3-SHA minTLSVersion: TLSv1.0"
+                      description: "old is a TLS security profile based on: \n https://wiki.mozilla.org/Security/Server_Side_TLS#Old_backward_compatibility \n and looks like this (yaml): \n ciphers: - TLS_AES_128_GCM_SHA256 - TLS_AES_256_GCM_SHA384 - TLS_CHACHA20_POLY1305_SHA256 - 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 - DHE-RSA-CHACHA20-POLY1305 - ECDHE-ECDSA-AES128-SHA256 - ECDHE-RSA-AES128-SHA256 - ECDHE-ECDSA-AES128-SHA - ECDHE-RSA-AES128-SHA - ECDHE-ECDSA-AES256-SHA384 - ECDHE-RSA-AES256-SHA384 - ECDHE-ECDSA-AES256-SHA - ECDHE-RSA-AES256-SHA - DHE-RSA-AES128-SHA256 - DHE-RSA-AES256-SHA256 - AES128-GCM-SHA256 - AES256-GCM-SHA384 - AES128-SHA256 - AES256-SHA256 - AES128-SHA - AES256-SHA - DES-CBC3-SHA minTLSVersion: VersionTLS10"
                       nullable: true
                       type: object
                     type:
@@ -1117,7 +1117,7 @@ spec:
                         type: string
                       type: array
                     minTLSVersion:
-                      description: "minTLSVersion is used to specify the minimal version of the TLS protocol that is negotiated during the TLS handshake. For example, to use TLS versions 1.1, 1.2 and 1.3 (yaml): \n minTLSVersion: TLSv1.1 \n NOTE: currently the highest minTLSVersion allowed is VersionTLS12"
+                      description: "minTLSVersion is used to specify the minimal version of the TLS protocol that is negotiated during the TLS handshake. For example, to use TLS versions 1.1, 1.2 and 1.3 (yaml): \n minTLSVersion: VersionTLS11 \n NOTE: currently the highest minTLSVersion allowed is VersionTLS12"
                       enum:
                         - VersionTLS10
                         - VersionTLS11

diff --git a/pkg/operator/controller/ingress/controller.go b/pkg/operator/controller/ingress/controller.go
@@ -8,6 +8,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/pkg/errors"
 
 	logf "github.com/openshift/cluster-ingress-operator/pkg/log"
@@ -448,6 +450,15 @@ func setDefaultPublishingStrategy(ic *operatorv1.IngressController, platformStat
 			effectiveStrategy.LoadBalancer.DNSManagementPolicy = operatorv1.UnmanagedLoadBalancerDNS
 		}
 
+		// When the platform's default DNS solution cannot be used, set the DNSManagementPolicy
+		// accordingly. This feature is currently being implemented first for GCP. Will be
+		// extended to AWS and Azure platforms later.
+		if platformStatus.Type == configv1.GCPPlatformType && platformStatus.GCP != nil && platformStatus.GCP.CloudLoadBalancerConfig != nil {
+			if platformStatus.GCP.CloudLoadBalancerConfig.DNSType == configv1.ClusterHostedDNSType {
+				effectiveStrategy.LoadBalancer.DNSManagementPolicy = operatorv1.UnmanagedLoadBalancerDNS
+			}
+		}
+
 		// Set provider parameters based on the cluster ingress config.
 		setDefaultProviderParameters(effectiveStrategy.LoadBalancer, ingressConfig, alreadyAdmitted)
 
@@ -1061,12 +1072,11 @@ func (r *reconciler) ensureIngressController(ci *operatorv1.IngressController, d
 		Controller: &trueVar,
 	}
 
-	var lbService *corev1.Service
 	var wildcardRecord *iov1.DNSRecord
-	if haveLB, lb, err := r.ensureLoadBalancerService(ci, deploymentRef, platformStatus); err != nil {
+	haveLB, lbService, err := r.ensureLoadBalancerService(ci, deploymentRef, platformStatus)
+	if err != nil {
 		errs = append(errs, fmt.Errorf("failed to ensure load balancer service for %s: %v", ci.Name, err))
 	} else {
-		lbService = lb
 		dnsRecordName := operatorcontroller.WildcardDNSRecordName(ci)
 		icRef := metav1.OwnerReference{
 			APIVersion:         operatorv1.GroupVersion.String(),
@@ -1131,6 +1141,18 @@ func (r *reconciler) ensureIngressController(ci *operatorv1.IngressController, d
 
 	SetIngressControllerNLBMetric(ci)
 
+	// If the lbService exists for the "default" IngressController, then update Infra CR's PlatformStatus with the Ingress LB IPs.
+	if haveLB && ci.Name == manifests.DefaultIngressControllerName {
+		if updated, err := computeUpdatedInfraFromService(lbService, infraConfig); err != nil {
+			errs = append(errs, fmt.Errorf("failed to update Infrastructure PlatformStatus: %w", err))
+		} else if updated {
+			if err := r.client.Status().Update(context.TODO(), infraConfig); err != nil {
+				errs = append(errs, fmt.Errorf("failed to update Infrastructure CR after updating Ingress LB IPs: %w", err))
+			}
+		}
+		log.Info("successfully updated Infra CR with Ingress Load Balancer IPs")
+	}
+
 	errs = append(errs, r.syncRouteStatus(ci)...)
 
 	return retryable.NewMaybeRetryableAggregate(errs)
@@ -1205,3 +1227,43 @@ func (r *reconciler) allRouterPodsDeleted(ingress *operatorv1.IngressController)
 
 	return true, nil
 }
+
+// computeUpdatedInfraFromService updates GCP's PlatformStatus with Ingress LB IPs when the DNSType is `ClusterHosted`.
+func computeUpdatedInfraFromService(service *corev1.Service, infraConfig *configv1.Infrastructure) (bool, error) {
+	platformStatus := infraConfig.Status.PlatformStatus
+	if platformStatus == nil {
+		return false, fmt.Errorf("invalid PlatformStatus within Infrastructure config")
+	}
+	switch platformStatus.Type {
+	case configv1.GCPPlatformType:
+		if platformStatus.GCP != nil && platformStatus.GCP.CloudLoadBalancerConfig != nil && platformStatus.GCP.CloudLoadBalancerConfig.DNSType == configv1.ClusterHostedDNSType {
+			// The cluster has to run its own CoreDNS pod for DNS. Update Infra CR
+			// with the Ingress LB IPs. These values are used to configure the
+			// in-cluster DNS to provide resolution for *.apps.
+			if platformStatus.GCP.CloudLoadBalancerConfig.ClusterHosted == nil {
+				platformStatus.GCP.CloudLoadBalancerConfig.ClusterHosted = &configv1.CloudLoadBalancerIPs{}
+			}
+			ingresses := service.Status.LoadBalancer.Ingress
+			ingressLBIPs := []configv1.IP{}
+			for _, ingress := range ingresses {
+				if len(ingress.IP) > 0 {
+					ingressLBIPs = append(ingressLBIPs, configv1.IP(ingress.IP))
+				}
+			}
+			ipCmpOpts := []cmp.Option{
+				cmpopts.EquateEmpty(),
+				cmpopts.SortSlices(func(a, b configv1.IP) bool {
+					return a < b
+				}),
+			}
+			if !cmp.Equal(platformStatus.GCP.CloudLoadBalancerConfig.ClusterHosted.IngressLoadBalancerIPs, ingressLBIPs, ipCmpOpts...) {
+				platformStatus.GCP.CloudLoadBalancerConfig.ClusterHosted.IngressLoadBalancerIPs = ingressLBIPs
+				return true, nil
+			}
+		}
+		return false, nil
+	default:
+		return false, nil
+	}
+	return false, nil
+}