Add support for system-internal-tls in net-istio (#1085)

* Add support for `internal-encryption` in `net-istio`

* fixes

* Enable internal-encryption in config-network

* DEBUG tests

* Change CI step order and add documentation

* review improvements

* propagate annotations and labels from ingress

* Use istio api and client version v1beta1

* Fix lint and shellcheck warnings

* Fix more lint warnings

* Update to aligned networking flags

* Minor fixes

* Run encryption tests only in no-mesh mode

* Update .github/workflows/kind-e2e.yaml

Co-authored-by: Kenjiro Nakayama <[email protected]>

* Update pkg/reconciler/ingress/ingress.go

Co-authored-by: Kenjiro Nakayama <[email protected]>

* Use config flag instead of constant

---------

Co-authored-by: Kenjiro Nakayama <[email protected]>

.github/workflows/kind-e2e.yaml

@@ -31,6 +31,10 @@ jobs:
         istio-version:
         - latest
 
+        encryption:
+          - disabled
+          - system-internal-tls
+
         istio-profile:
         - no-mesh
         - ambient
@@ -90,6 +94,21 @@ jobs:
 
         echo "GATEWAY_NAMESPACE_OVERRIDE=istio-system" >> $GITHUB_ENV
 
+    - name: Generate certificates and enable system-internal-tls
+      if: matrix.encryption == 'system-internal-tls' && matrix.istio-profile == 'no-mesh'
+      run: |
+        set -o pipefail
+        
+        echo ">> Deploy certificate for upstream traffic"
+        ./test/generate-upstream-cert.sh
+        
+        echo ">> Setting environment variables for upstream tls"
+        echo "UPSTREAM_TLS_CERT=serving-certs" >> $GITHUB_ENV
+        echo "SERVER_NAME=kn-user-serving-tests" >> $GITHUB_ENV
+        
+        echo ">> Enabling system-internal-tls in config-network"
+        kubectl apply -f test/config/system-internal-tls
+
     - name: Upload Test Images
       run: |
         # Build and Publish our test images to the docker daemon.
@@ -108,7 +127,7 @@ jobs:
     - name: Run e2e Tests
       run: |
         set -x
-
+        
         # Exclude the control-plane node, which doesn't seem to expose the nodeport service.
         IPS=( $(kubectl get nodes -lkubernetes.io/hostname!=kind-control-plane -ojsonpath='{.items[*].status.addresses[?(@.type=="InternalIP")].address}') )
 

DEVELOPMENT.md

@@ -82,8 +82,8 @@ istioctl install -y
 Run the following command to install Knative
 
 ```shell
-kubectl apply --filename https://storage.googleapis.com/knative-nightly/serving/latest/serving-crds.yaml
-kubectl apply --filename https://storage.googleapis.com/knative-nightly/serving/latest/serving-core.yaml
+kubectl apply -f https://github.com/knative/serving/releases/latest/download/serving-crds.yaml
+kubectl apply -f https://github.com/knative/serving/releases/latest/download/serving-core.yaml
 ```
 
 ### Install Knative net-istio
@@ -93,3 +93,18 @@ Run the following command to install net-istio components
 ```shell
 ko apply -f config/
 ```
+
+### System Internal TLS (optional)
+
+If you want to work with `system-internal-tls` enabled you can either:
+
+* Install `Knative Serving` to automatically generate the certificates. The CA will be injected in [700-istio-secret.yaml](./config/700-istio-secret.yaml).
+* Or use [./test/generate-upstream-cert.sh)](./test/generate-upstream-cert.sh) to manually generate the secrets.
+
+You can then enable `system-internal-tls` in `config-network` like in [our test resources](./test/config/system-internal-tls/config-network.yaml)
+and specify the following environment variables before you run the e2e/conformance tests:
+
+```bash
+export UPSTREAM_TLS_CERT=serving-certs
+export SERVER_NAME=kn-user-serving-tests
+```

cmd/controller/main.go

@@ -32,6 +32,7 @@ func main() {
 	// resilient to clusters containing malformed resources.
 	v1beta1.VirtualServiceUnmarshaler.AllowUnknownFields = true
 	v1beta1.GatewayUnmarshaler.AllowUnknownFields = true
+	v1beta1.DestinationRuleUnmarshaler.AllowUnknownFields = true
 
 	ctx := informerfiltering.GetContextWithFilteringLabelSelector(signals.NewContext())
 	sharedmain.MainWithContext(ctx, "net-istio-controller", ingress.NewController, serverlessservice.NewController)

config/700-istio-secret.yaml

@@ -0,0 +1,23 @@
+# Copyright 2023 The Knative Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: v1
+kind: Secret
+metadata:
+  name: routing-serving-certs
+  namespace: istio-system
+  labels:
+    serving-certs-ctrl: "data-plane-routing"
+    networking.internal.knative.dev/certificate-uid: "serving-certs"
+# The data is populated when system-internal-tls is enabled.

pkg/reconciler/ingress/controller.go

@@ -22,6 +22,7 @@ import (
 	"go.uber.org/zap"
 	v1 "k8s.io/client-go/informers/core/v1"
 	istioclient "knative.dev/net-istio/pkg/client/istio/injection/client"
+	destinationruleinformer "knative.dev/net-istio/pkg/client/istio/injection/informers/networking/v1beta1/destinationrule"
 	gatewayinformer "knative.dev/net-istio/pkg/client/istio/injection/informers/networking/v1beta1/gateway"
 	virtualserviceinformer "knative.dev/net-istio/pkg/client/istio/injection/informers/networking/v1beta1/virtualservice"
 	"knative.dev/net-istio/pkg/reconciler/ingress/config"
@@ -82,18 +83,20 @@ func newControllerWithOptions(
 	ctx = AnnotateLoggerWithName(ctx, controllerAgentName)
 	logger := logging.FromContext(ctx)
 	virtualServiceInformer := virtualserviceinformer.Get(ctx)
+	destinationRuleInformer := destinationruleinformer.Get(ctx)
 	gatewayInformer := gatewayinformer.Get(ctx)
 	secretInformer := getSecretInformer(ctx)
 	serviceInformer := serviceinformer.Get(ctx)
 	ingressInformer := ingressinformer.Get(ctx)
 
 	c := &Reconciler{
-		kubeclient:           kubeclient.Get(ctx),
-		istioClientSet:       istioclient.Get(ctx),
-		virtualServiceLister: virtualServiceInformer.Lister(),
-		gatewayLister:        gatewayInformer.Lister(),
-		secretLister:         secretInformer.Lister(),
-		svcLister:            serviceInformer.Lister(),
+		kubeclient:            kubeclient.Get(ctx),
+		istioClientSet:        istioclient.Get(ctx),
+		virtualServiceLister:  virtualServiceInformer.Lister(),
+		destinationRuleLister: destinationRuleInformer.Lister(),
+		gatewayLister:         gatewayInformer.Lister(),
+		secretLister:          secretInformer.Lister(),
+		svcLister:             serviceInformer.Lister(),
 	}
 	myFilterFunc := reconciler.AnnotationFilterFunc(networking.IngressClassAnnotationKey, netconfig.IstioIngressClassName, true)
 
@@ -123,6 +126,11 @@ func newControllerWithOptions(
 		Handler:    controller.HandleAll(impl.EnqueueControllerOf),
 	})
 
+	destinationRuleInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
+		FilterFunc: controller.FilterController(&v1alpha1.Ingress{}),
+		Handler:    controller.HandleAll(impl.EnqueueControllerOf),
+	})
+
 	endpointsInformer := endpointsinformer.Get(ctx)
 	podInformer := podinformer.Get(ctx)
 	resyncOnIngressReady := func(ing *v1alpha1.Ingress) {

pkg/reconciler/ingress/ingress.go

@@ -24,6 +24,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"go.uber.org/zap"
 	"google.golang.org/protobuf/testing/protocmp"
+	pkgnetwork "knative.dev/pkg/network"
 
 	istiov1beta1 "istio.io/api/networking/v1beta1"
 	"istio.io/client-go/pkg/apis/networking/v1beta1"
@@ -66,22 +67,24 @@ const (
 type Reconciler struct {
 	kubeclient kubernetes.Interface
 
-	istioClientSet       istioclientset.Interface
-	virtualServiceLister istiolisters.VirtualServiceLister
-	gatewayLister        istiolisters.GatewayLister
-	secretLister         corev1listers.SecretLister
-	svcLister            corev1listers.ServiceLister
+	istioClientSet        istioclientset.Interface
+	virtualServiceLister  istiolisters.VirtualServiceLister
+	destinationRuleLister istiolisters.DestinationRuleLister
+	gatewayLister         istiolisters.GatewayLister
+	secretLister          corev1listers.SecretLister
+	svcLister             corev1listers.ServiceLister
 
 	tracker tracker.Interface
 
 	statusManager status.Manager
 }
 
 var (
-	_ ingressreconciler.Interface          = (*Reconciler)(nil)
-	_ ingressreconciler.Finalizer          = (*Reconciler)(nil)
-	_ coreaccessor.SecretAccessor          = (*Reconciler)(nil)
-	_ istioaccessor.VirtualServiceAccessor = (*Reconciler)(nil)
+	_ ingressreconciler.Interface           = (*Reconciler)(nil)
+	_ ingressreconciler.Finalizer           = (*Reconciler)(nil)
+	_ coreaccessor.SecretAccessor           = (*Reconciler)(nil)
+	_ istioaccessor.VirtualServiceAccessor  = (*Reconciler)(nil)
+	_ istioaccessor.DestinationRuleAccessor = (*Reconciler)(nil)
 )
 
 // ReconcileKind compares the actual state with the desired, and attempts to
@@ -186,6 +189,13 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
 	}
 	gatewayNames[v1alpha1.IngressVisibilityExternalIP].Insert(resources.GetQualifiedGatewayNames(ingressGateways)...)
 
+	if config.FromContext(ctx).Network.SystemInternalTLSEnabled() {
+		logger.Info("reconciling DestinationRules for system-internal-tls")
+		if err := r.reconcileDestinationRules(ctx, ing); err != nil {
+			return err
+		}
+	}
+
 	vses, err := resources.MakeVirtualServices(ing, gatewayNames)
 	if err != nil {
 		return err
@@ -347,6 +357,47 @@ func (r *Reconciler) reconcileVirtualServices(ctx context.Context, ing *v1alpha1
 	return nil
 }
 
+func (r *Reconciler) reconcileDestinationRules(ctx context.Context, ing *v1alpha1.Ingress) error {
+	var drs = sets.New[string]()
+	for _, rule := range ing.Spec.Rules {
+		for _, path := range rule.HTTP.Paths {
+			// Currently DomainMappings point to the cluster local domain on the local gateway.
+			// As there is no encryption there yet (https://github.com/knative/serving/issues/13472),
+			// we cannot use upstream TLS here, so we need to skip it for DomainMappings
+			if path.RewriteHost != "" {
+				continue
+			}
+
+			for _, split := range path.Splits {
+				svc, err := r.svcLister.Services(split.ServiceNamespace).Get(split.ServiceName)
+				if err != nil {
+					return fmt.Errorf("failed to get service: %w", err)
+				}
+
+				http2 := false
+				for _, port := range svc.Spec.Ports {
+					if port.Name == "http2" || port.Name == "h2c" {
+						http2 = true
+					}
+				}
+
+				hostname := pkgnetwork.GetServiceHostname(split.ServiceName, split.ServiceNamespace)
+
+				// skip duplicate entries, as we only need one DR per unique upstream k8s service
+				if !drs.Has(hostname) {
+					dr := resources.MakeInternalEncryptionDestinationRule(hostname, ing, http2)
+					if _, err := istioaccessor.ReconcileDestinationRule(ctx, ing, dr, r); err != nil {
+						return fmt.Errorf("failed to reconcile DestinationRule: %w", err)
+					}
+					drs.Insert(hostname)
+				}
+			}
+		}
+	}
+
+	return nil
+}
+
 func (r *Reconciler) FinalizeKind(ctx context.Context, ing *v1alpha1.Ingress) pkgreconciler.Event {
 	logger := logging.FromContext(ctx)
 	istiocfg := config.FromContext(ctx).Istio
@@ -437,6 +488,10 @@ func (r *Reconciler) GetVirtualServiceLister() istiolisters.VirtualServiceLister
 	return r.virtualServiceLister
 }
 
+func (r *Reconciler) GetDestinationRuleLister() istiolisters.DestinationRuleLister {
+	return r.destinationRuleLister
+}
+
 // qualifiedGatewayNamesFromContext get gateway names from context
 func qualifiedGatewayNamesFromContext(ctx context.Context) map[v1alpha1.IngressVisibility]sets.Set[string] {
 	ci := config.FromContext(ctx).Istio