[RELEASE-1.13] Back-port knative-extensions#1228 (#79)

* Host cluster-local-domain-tls on cluster-local gateway with SNI (knative-extensions#1228) * Host cluster-local-domain TLS on local listener with SNI * Use port 8444 for cluster-local TLS traffic (cherry picked from commit 83db165) * Update patch to reflect upstream changes
openshift-knative · Feb 7, 2024 · 0136218 · 0136218
1 parent 01fd6de
commit 0136218
Show file tree

Hide file tree

Showing 8 changed files with 550 additions and 182 deletions.
diff --git a/config/203-local-gateway.yaml b/config/203-local-gateway.yaml
@@ -55,3 +55,6 @@ spec:
     - name: http2
       port: 80
       targetPort: 8081
+    - name: https
+      port: 443
+      targetPort: 8444
diff --git a/openshift/patches/002-add-maistra-annotation.patch b/openshift/patches/002-add-maistra-annotation.patch
@@ -1,21 +1,27 @@
 diff --git a/pkg/reconciler/ingress/resources/gateway.go b/pkg/reconciler/ingress/resources/gateway.go
---- a/pkg/reconciler/ingress/resources/gateway.go	(revision 9914eca315000a757b0f48df69f59cdbb8f7ea23)
-+++ b/pkg/reconciler/ingress/resources/gateway.go	(revision 2b701027bfe7dca09d27c709b0cc5d66069a1f5b)
-@@ -42,9 +42,10 @@
+--- a/pkg/reconciler/ingress/resources/gateway.go	(revision 2971960b1cf6f44bbf9146e22d0e31eb253d812f)
++++ b/pkg/reconciler/ingress/resources/gateway.go	(date 1707288992469)
+@@ -41,12 +41,13 @@
+ )
 
- // GatewayHTTPPort is the HTTP port the gateways listen on.
  const (
--	GatewayHTTPPort       = 80
--	dns1123LabelMaxLength = 63 // Public for testing only.
--	dns1123LabelFmt       = "[a-zA-Z0-9](?:[-a-zA-Z0-9]*[a-zA-Z0-9])?"
+-	GatewayHTTPPort              = 80
+-	ExternalGatewayHTTPSPort     = 443
+-	ClusterLocalGatewayHTTPSPort = 8444
+-	dns1123LabelMaxLength        = 63 // Public for testing only.
+-	dns1123LabelFmt              = "[a-zA-Z0-9](?:[-a-zA-Z0-9]*[a-zA-Z0-9])?"
+-	localGatewayPostfix          = "-local"
 +	GatewayHTTPPort                 = 80
++	ExternalGatewayHTTPSPort        = 443
++	ClusterLocalGatewayHTTPSPort    = 8444
 +	dns1123LabelMaxLength           = 63 // Public for testing only.
 +	dns1123LabelFmt                 = "[a-zA-Z0-9](?:[-a-zA-Z0-9]*[a-zA-Z0-9])?"
++	localGatewayPostfix             = "-local"
 +	MaistraManageRouteAnnotationKey = "maistra.io/manageRoute"
  )
 
  var httpServerPortName = "http-server"
-@@ -199,6 +200,9 @@
+@@ -202,6 +203,9 @@
  				Name:            WildcardGatewayName(secret.Name, gatewayService.Namespace, gatewayService.Name),
  				Namespace:       secret.Namespace,
  				OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(secret, gvk)},
@@ -25,8 +31,8 @@ diff --git a/pkg/reconciler/ingress/resources/gateway.go b/pkg/reconciler/ingres
  			},
  			Spec: istiov1beta1.Gateway{
  				Selector: gatewayService.Spec.Selector,
-@@ -254,6 +258,9 @@
- 				// We need this label to find out all of Gateways of a given Ingress.
+@@ -257,6 +261,9 @@
+ 				// We need this label to find out all Gateways of a given Ingress.
  				networking.IngressLabelKey: ing.GetName(),
  			},
 +			Annotations: map[string]string{
@@ -35,77 +41,85 @@ diff --git a/pkg/reconciler/ingress/resources/gateway.go b/pkg/reconciler/ingres
  		},
  		Spec: istiov1beta1.Gateway{
  			Selector: selector,
+diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go
+--- a/pkg/reconciler/ingress/ingress_test.go	(revision 2971960b1cf6f44bbf9146e22d0e31eb253d812f)
++++ b/pkg/reconciler/ingress/ingress_test.go	(date 1707289100741)
+@@ -1525,8 +1525,9 @@
+ func gateway(name, namespace string, servers []*istiov1beta1.Server, opts ...GatewayOpt) *v1beta1.Gateway {
+ 	gw := &v1beta1.Gateway{
+ 		ObjectMeta: metav1.ObjectMeta{
+-			Name:      name,
+-			Namespace: namespace,
++			Name:        name,
++			Namespace:   namespace,
++			Annotations: map[string]string{resources.MaistraManageRouteAnnotationKey: "false"},
+ 		},
+ 		Spec: istiov1beta1.Gateway{
+ 			Servers: servers,
 diff --git a/pkg/reconciler/ingress/resources/gateway_test.go b/pkg/reconciler/ingress/resources/gateway_test.go
---- a/pkg/reconciler/ingress/resources/gateway_test.go	(revision 9914eca315000a757b0f48df69f59cdbb8f7ea23)
-+++ b/pkg/reconciler/ingress/resources/gateway_test.go	(revision 2b701027bfe7dca09d27c709b0cc5d66069a1f5b)
-@@ -593,6 +593,7 @@
+--- a/pkg/reconciler/ingress/resources/gateway_test.go	(revision 2971960b1cf6f44bbf9146e22d0e31eb253d812f)
++++ b/pkg/reconciler/ingress/resources/gateway_test.go	(date 1707289076385)
+@@ -594,6 +594,7 @@
  				Name:            WildcardGatewayName(wildcardSecret.Name, "istio-system", "istio-ingressgateway"),
  				Namespace:       system.Namespace(),
  				OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(wildcardSecret, secretGVK)},
 +				Annotations:     map[string]string{MaistraManageRouteAnnotationKey: "false"},
  			},
  			Spec: istiov1beta1.Gateway{
  				Selector: selector,
-@@ -630,6 +631,7 @@
+@@ -631,6 +632,7 @@
  				Name:            WildcardGatewayName(wildcardSecret.Name, system.Namespace(), "istio-ingressgateway"),
  				Namespace:       system.Namespace(),
  				OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(wildcardSecret, secretGVK)},
 +				Annotations:     map[string]string{MaistraManageRouteAnnotationKey: "false"},
  			},
  			Spec: istiov1beta1.Gateway{
  				Selector: selector,
-@@ -743,6 +745,7 @@
+@@ -744,6 +746,7 @@
+ 				Labels: map[string]string{
+ 					networking.IngressLabelKey: "ingress",
+ 				},
++				Annotations: map[string]string{MaistraManageRouteAnnotationKey: "false"},
+ 			},
+ 			Spec: istiov1beta1.Gateway{
+ 				Selector: selector,
+@@ -762,6 +765,7 @@
  				Labels: map[string]string{
  					networking.IngressLabelKey: "ingress",
  				},
 +				Annotations: map[string]string{MaistraManageRouteAnnotationKey: "false"},
  			},
  			Spec: istiov1beta1.Gateway{
  				Selector: selector,
-@@ -761,6 +764,7 @@
+@@ -827,6 +831,7 @@
  				Labels: map[string]string{
  					networking.IngressLabelKey: "ingress",
  				},
 +				Annotations: map[string]string{MaistraManageRouteAnnotationKey: "false"},
  			},
  			Spec: istiov1beta1.Gateway{
  				Selector: selector,
-@@ -824,6 +828,7 @@
+@@ -870,6 +875,7 @@
  				Labels: map[string]string{
  					networking.IngressLabelKey: "ingress",
  				},
 +				Annotations: map[string]string{MaistraManageRouteAnnotationKey: "false"},
  			},
  			Spec: istiov1beta1.Gateway{
  				Selector: selector,
-@@ -866,6 +871,7 @@
+@@ -916,6 +922,7 @@
  				Labels: map[string]string{
  					networking.IngressLabelKey: "ingress",
  				},
 +				Annotations: map[string]string{MaistraManageRouteAnnotationKey: "false"},
  			},
  			Spec: istiov1beta1.Gateway{
  				Selector: selector,
-@@ -908,6 +914,7 @@
+@@ -959,6 +966,7 @@
  				Labels: map[string]string{
  					networking.IngressLabelKey: "ingress.com",
  				},
 +				Annotations: map[string]string{MaistraManageRouteAnnotationKey: "false"},
  			},
  			Spec: istiov1beta1.Gateway{
  				Selector: selector,
-diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go
---- a/pkg/reconciler/ingress/ingress_test.go	(revision 9914eca315000a757b0f48df69f59cdbb8f7ea23)
-+++ b/pkg/reconciler/ingress/ingress_test.go	(revision 2b701027bfe7dca09d27c709b0cc5d66069a1f5b)
-@@ -1323,8 +1323,9 @@
- func gateway(name, namespace string, servers []*istiov1beta1.Server, opts ...GatewayOpt) *v1beta1.Gateway {
- 	gw := &v1beta1.Gateway{
- 		ObjectMeta: metav1.ObjectMeta{
--			Name:      name,
--			Namespace: namespace,
-+			Name:        name,
-+			Namespace:   namespace,
-+			Annotations: map[string]string{resources.MaistraManageRouteAnnotationKey: "false"},
- 		},
- 		Spec: istiov1beta1.Gateway{
- 			Servers: servers,
diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go
@@ -57,10 +57,9 @@ import (
 )
 
 const (
-	virtualServiceConditionReconciled = "Reconciled"
-	virtualServiceNotReconciled       = "ReconcileVirtualServiceFailed"
-	notReconciledReason               = "ReconcileIngressFailed"
-	notReconciledMessage              = "Ingress reconciliation failed"
+	virtualServiceNotReconciled = "ReconcileVirtualServiceFailed"
+	notReconciledReason         = "ReconcileIngressFailed"
+	notReconciledMessage        = "Ingress reconciliation failed"
 )
 
 // Reconciler implements the control loop for the Ingress resources.
@@ -106,8 +105,8 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
 	logger := logging.FromContext(ctx)
 
 	// We may be reading a version of the object that was stored at an older version
-	// and may not have had all of the assumed defaults specified.  This won't result
-	// in this getting written back to the API Server, but lets downstream logic make
+	// and may not have had all the assumed defaults specified.  This won't result
+	// in this getting written back to the API Server, but let's downstream logic make
 	// assumptions about defaulting.
 	ing.SetDefaults(ctx)
 
@@ -118,9 +117,9 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
 	gatewayNames[v1alpha1.IngressVisibilityClusterLocal] = qualifiedGatewayNamesFromContext(ctx)[v1alpha1.IngressVisibilityClusterLocal]
 	gatewayNames[v1alpha1.IngressVisibilityExternalIP] = sets.New[string]()
 
-	ingressGateways := []*v1beta1.Gateway{}
-	if shouldReconcileTLS(ing) {
-		originSecrets, err := resources.GetSecrets(ing, r.secretLister)
+	externalIngressGateways := []*v1beta1.Gateway{}
+	if shouldReconcileExternalDomainTLS(ing) {
+		originSecrets, err := resources.GetSecrets(ing, v1alpha1.IngressVisibilityExternalIP, r.secretLister)
 		if err != nil {
 			return err
 		}
@@ -144,7 +143,8 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
 		}
 
 		nonWildcardIngressTLS := resources.GetNonWildcardIngressTLS(ing.GetIngressTLSForVisibility(v1alpha1.IngressVisibilityExternalIP), nonWildcardSecrets)
-		ingressGateways, err = resources.MakeIngressTLSGateways(ctx, ing, nonWildcardIngressTLS, nonWildcardSecrets, r.svcLister)
+		externalIngressGateways, err = resources.MakeIngressTLSGateways(ctx, ing, v1alpha1.IngressVisibilityExternalIP,
+			nonWildcardIngressTLS, nonWildcardSecrets, r.svcLister)
 		if err != nil {
 			return err
 		}
@@ -164,17 +164,38 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
 		gatewayNames[v1alpha1.IngressVisibilityExternalIP].Insert(resources.GetQualifiedGatewayNames(desiredWildcardGateways)...)
 	}
 
+	cfg := config.FromContext(ctx)
+	clusterLocalIngressGateways := []*v1beta1.Gateway{}
+	if cfg.Network.ClusterLocalDomainTLS == netconfig.EncryptionEnabled && shouldReconcileClusterLocalDomainTLS(ing) {
+		originSecrets, err := resources.GetSecrets(ing, v1alpha1.IngressVisibilityClusterLocal, r.secretLister)
+		if err != nil {
+			return err
+		}
+		targetSecrets, err := resources.MakeSecrets(ctx, originSecrets, ing)
+		if err != nil {
+			return err
+		}
+		if err = r.reconcileCertSecrets(ctx, ing, targetSecrets); err != nil {
+			return err
+		}
+		clusterLocalIngressGateways, err = resources.MakeIngressTLSGateways(ctx, ing, v1alpha1.IngressVisibilityClusterLocal,
+			ing.GetIngressTLSForVisibility(v1alpha1.IngressVisibilityClusterLocal), originSecrets, r.svcLister)
+		if err != nil {
+			return err
+		}
+	}
+
 	if shouldReconcileHTTPServer(ing) {
 		httpServer := resources.MakeHTTPServer(ing.Spec.HTTPOption, getPublicHosts(ing))
-		if len(ingressGateways) == 0 {
+		if len(externalIngressGateways) == 0 {
 			var err error
-			if ingressGateways, err = resources.MakeIngressGateways(ctx, ing, []*istiov1beta1.Server{httpServer}, r.svcLister); err != nil {
+			if externalIngressGateways, err = resources.MakeExternalIngressGateways(ctx, ing, []*istiov1beta1.Server{httpServer}, r.svcLister); err != nil {
 				return err
 			}
 		} else {
 			// add HTTP Server into ingressGateways.
-			for i := range ingressGateways {
-				ingressGateways[i].Spec.Servers = append(ingressGateways[i].Spec.Servers, httpServer)
+			for i := range externalIngressGateways {
+				externalIngressGateways[i].Spec.Servers = append(externalIngressGateways[i].Spec.Servers, httpServer)
 			}
 		}
 	} else {
@@ -184,10 +205,15 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
 		gatewayNames[v1alpha1.IngressVisibilityExternalIP].Insert(sets.List(defaultGlobalHTTPGateways)...)
 	}
 
-	if err := r.reconcileIngressGateways(ctx, ingressGateways); err != nil {
+	if err := r.reconcileIngressGateways(ctx, externalIngressGateways); err != nil {
 		return err
 	}
-	gatewayNames[v1alpha1.IngressVisibilityExternalIP].Insert(resources.GetQualifiedGatewayNames(ingressGateways)...)
+	gatewayNames[v1alpha1.IngressVisibilityExternalIP].Insert(resources.GetQualifiedGatewayNames(externalIngressGateways)...)
+
+	if err := r.reconcileIngressGateways(ctx, clusterLocalIngressGateways); err != nil {
+		return err
+	}
+	gatewayNames[v1alpha1.IngressVisibilityClusterLocal].Insert(resources.GetQualifiedGatewayNames(clusterLocalIngressGateways)...)
 
 	if config.FromContext(ctx).Network.SystemInternalTLSEnabled() {
 		logger.Info("reconciling DestinationRules for system-internal-tls")
@@ -410,16 +436,16 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, ing *v1alpha1.Ingress) pk
 		}
 	}
 
-	return r.reconcileDeletion(ctx, ing)
+	return r.cleanupCertificateSecrets(ctx, ing)
 }
 
-func (r *Reconciler) reconcileDeletion(ctx context.Context, ing *v1alpha1.Ingress) error {
-	if !shouldReconcileTLS(ing) {
+func (r *Reconciler) cleanupCertificateSecrets(ctx context.Context, ing *v1alpha1.Ingress) error {
+	if !shouldReconcileExternalDomainTLS(ing) && !shouldReconcileClusterLocalDomainTLS(ing) {
 		return nil
 	}
 
 	errs := []error{}
-	for _, tls := range ing.GetIngressTLSForVisibility(v1alpha1.IngressVisibilityExternalIP) {
+	for _, tls := range ing.Spec.TLS {
 		nameNamespaces, err := resources.GetIngressGatewaySvcNameNamespaces(ctx)
 		if err != nil {
 			errs = append(errs, err)
@@ -541,13 +567,17 @@ func getLBStatus(gatewayServiceURL string) []v1alpha1.LoadBalancerIngressStatus
 	}
 }
 
-func shouldReconcileTLS(ing *v1alpha1.Ingress) bool {
+func shouldReconcileExternalDomainTLS(ing *v1alpha1.Ingress) bool {
 	return isIngressPublic(ing) && len(ing.GetIngressTLSForVisibility(v1alpha1.IngressVisibilityExternalIP)) > 0
 }
 
+func shouldReconcileClusterLocalDomainTLS(ing *v1alpha1.Ingress) bool {
+	return len(ing.GetIngressTLSForVisibility(v1alpha1.IngressVisibilityClusterLocal)) > 0
+}
+
 func shouldReconcileHTTPServer(ing *v1alpha1.Ingress) bool {
-	// We will create a Ingress specific HTTPServer when
-	// 1. auto TLS is enabled as in this case users want us to fully handle the TLS/HTTP behavior,
+	// We will create an Ingress specific HTTPServer when
+	// 1. external-domain-tls is enabled as in this case users want us to fully handle the TLS/HTTP behavior,
 	// 2. HTTPOption is set to Redirected as we don't have default HTTP server supporting HTTP redirection.
 	return isIngressPublic(ing) && (ing.Spec.HTTPOption == v1alpha1.HTTPOptionRedirected || len(ing.GetIngressTLSForVisibility(v1alpha1.IngressVisibilityExternalIP)) > 0)
 }