[TEST-ONLY] testing cluster-local-domain-tls in Serving

.github/workflows/kind-e2e.yaml

@@ -87,17 +87,17 @@
       fail-fast: false # Keep running if one leg fails.
       matrix:
         k8s-version:
-        - v1.26.x
-        - v1.27.x
+#        - v1.26.x
+#        - v1.27.x
         - v1.28.x
 
         ingress:
-        - kourier
+#        - kourier
         - kourier-tls
-        - istio
-        - istio-tls
-        - istio-ambient
-        - contour
+#        - istio
+#        - istio-tls
+#        - istio-ambient
+#        - contour
         # Disabled due to consistent failures
         # - gateway_istio
 
@@ -113,22 +113,22 @@
         #   test-flags: -enable-alpha
         #   namespace-resources: httproute
 
-        - ingress: contour
-          namespace-resources: httpproxy
-
-        - ingress: istio
-          namespace-resources: virtualservices
-
-        - ingress: istio-tls
-          ingress-class: istio
-          namespace-resources: virtualservices
-          enable-tls: 1
-
-        - ingress: istio-ambient
-          namespace-resources: virtualservices
-          ingress-class: istio
-          ambient: 1
-
+#        - ingress: contour
+#          namespace-resources: httpproxy
+#
+#        - ingress: istio
+#          namespace-resources: virtualservices
+#
+#        - ingress: istio-tls
+#          ingress-class: istio
+#          namespace-resources: virtualservices
+#          enable-tls: 1
+#
+#        - ingress: istio-ambient
+#          namespace-resources: virtualservices
+#          ingress-class: istio
+#          ambient: 1
+#
         - ingress: kourier-tls
           ingress-class: kourier
           enable-tls: 1
@@ -146,7 +146,7 @@
       KIND: 1
       INGRESS_CLASS: ${{ matrix.ingress-class || matrix.ingress }}.ingress.networking.knative.dev
       ENABLE_TLS: ${{ matrix.enable-tls || 0 }}
       AMBIENT: ${{ matrix.ambient || 0 }}
 
     steps:
     - name: Set up Go 1.21.x
@@ -217,8 +217,6 @@
         echo "SYSTEM_NAMESPACE=$SYSTEM_NAMESPACE" >> $GITHUB_ENV
         echo "GATEWAY_OVERRIDE=$GATEWAY_OVERRIDE" >> $GITHUB_ENV
         echo "GATEWAY_NAMESPACE_OVERRIDE=$GATEWAY_NAMESPACE_OVERRIDE" >> $GITHUB_ENV
-        echo "CA_CERT=$CA_CERT" >> $GITHUB_ENV
-        echo "SERVER_NAME=$SERVER_NAME" >> $GITHUB_ENV
 
     - name: Test ${{ matrix.test-suite }}
       run: |
@@ -235,5 +233,5 @@
       if: ${{ failure() }}
       with:
         cluster-resources: nodes,namespaces,crds
         namespace-resources: configmaps,pods,svc,ksvc,route,configuration,revision,king,${{ matrix.namespace-resources || '' }}
         artifact-name: logs-${{ matrix.k8s-version}}-${{ matrix.ingress }}-${{ matrix.test-suite }}

cmd/controller/main.go

@@ -20,6 +20,8 @@ import (
 	// The set of controllers this controller process runs.
 	"flag"
 
+	"knative.dev/networking/pkg/certificates"
+	kncertificate "knative.dev/networking/pkg/certificates/kncertreconciler"
 	certificate "knative.dev/networking/pkg/certificates/reconciler"
 	"knative.dev/pkg/reconciler"
 	"knative.dev/pkg/signals"
@@ -40,10 +42,6 @@ import (
 	"knative.dev/serving/pkg/reconciler/domainmapping"
 )
 
-const (
-	secretLabelNamePostfix = "-ctrl"
-)
-
 var ctors = []injection.ControllerConstructor{
 	configuration.NewController,
 	labeler.NewController,
@@ -54,6 +52,7 @@ var ctors = []injection.ControllerConstructor{
 	gc.NewController,
 	nscert.NewController,
 	certificate.NewControllerFactory(networking.ServingCertName),
+	kncertificate.NewControllerFactory(networking.ServingCertName),
 	domainmapping.NewController,
 }
 
@@ -62,7 +61,10 @@ func main() {
 		"reconciliation-timeout", reconciler.DefaultTimeout,
 		"The amount of time to give each reconciliation of a resource to complete before its context is canceled.")
 
-	labelName := networking.ServingCertName + secretLabelNamePostfix
-	ctx := filteredFactory.WithSelectors(signals.NewContext(), labelName)
+	labels := []string{
+		networking.ServingCertName + certificates.SystemInternalTLSSecretLabelPostfix,
+		networking.ServingCertName + certificates.KnativeCertIssuerSecretLabelPostfix,
+	}
+	ctx := filteredFactory.WithSelectors(signals.NewContext(), labels...)
 	sharedmain.MainWithContext(ctx, "controller", ctors...)
 }

config/core/300-secret.yaml

@@ -45,3 +45,14 @@
     serving-certs-ctrl: "data-plane-routing"
     networking.internal.knative.dev/certificate-uid: "serving-certs"
 # The data is populated when knative-internal-tls is enabled.
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: serving-certs-cluster-local-domain-ca
+  namespace: knative-serving
+  labels:
+    serving-certs-knative-issuer: "cluster-local-domain-ca"
+    networking.internal.knative.dev/certificate-uid: "serving-certs"
+# The data is populated when knative-internal-tls is enabled.
+---

go.mod

@@ -153,3 +153,5 @@ require (
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
 )
+
+replace knative.dev/networking => /Users/rlehmann/code/knative/networking

go.sum

@@ -929,8 +929,6 @@ knative.dev/caching v0.0.0-20231017130712-54d0758671ef h1:92Gn5HUcgMJ78mbSpkCfUx
 knative.dev/caching v0.0.0-20231017130712-54d0758671ef/go.mod h1:plGN+mIBKRtVxZ0vQeZ3Gt02RIaj0niwIMnQNkQHycw=
 knative.dev/hack v0.0.0-20231016131700-2c938d4918da h1:xy+fvuz2LDOMsZ5UwXRaMF70NYUs9fsG+EF5/ierYBg=
 knative.dev/hack v0.0.0-20231016131700-2c938d4918da/go.mod h1:yk2OjGDsbEnQjfxdm0/HJKS2WqTLEFg/N6nUs6Rqx3Q=
-knative.dev/networking v0.0.0-20231017124814-2a7676e912b7 h1:6+1icZuxiZO1paFZ4d/ysKWVG2M4WB7OxNJNyLG0P/E=
-knative.dev/networking v0.0.0-20231017124814-2a7676e912b7/go.mod h1:1gcHoIVG47ekQWjkddqRq+/7tWRh+CB9W4k/NAcdRbk=
 knative.dev/pkg v0.0.0-20231023151236-29775d7c9e5c h1:xyPoEToTWeBdn6tinhLxXfnhJhTNQt5WzHiTNiFphRw=
 knative.dev/pkg v0.0.0-20231023151236-29775d7c9e5c/go.mod h1:HHRXEd7ZlFpthgE+rwAZ6MUVnuJOAeolnaFSthXloUQ=
 pgregory.net/rapid v1.1.0 h1:CMa0sjHSru3puNx+J0MIAuiiEV4N0qj8/cMWGBBCsjw=

pkg/apis/serving/register.go

@@ -138,10 +138,15 @@ const (
 	QueueSidecarEphemeralStorageResourceLimitAnnotationKey = "queue.sidecar." + GroupName + "/ephemeral-storage-resource-limit"
 
 	// VisibilityClusterLocal is the label value for VisibilityLabelKey
-	// that will result to the Route/KService getting a cluster local
+	// that will result to the Route/KService/KCert getting a cluster local
 	// domain suffix.
 	VisibilityClusterLocal = "cluster-local"
 
+	// VisibilityExternalIP is the label value for VisibilityLabelKey
+	// that will result to the Route/KService/KCert getting an external
+	// domain suffix.
+	VisibilityExternalIP = "external-ip"
+
 	// ProgressDeadlineAnnotationKey is the label key for the per revision progress deadline to set for the deployment
 	ProgressDeadlineAnnotationKey = GroupName + "/progress-deadline"
 )

pkg/apis/serving/v1/route_lifecycle.go

@@ -195,11 +195,6 @@ const (
 	// RouteConditionCertificateProvisioned condition when it is set to True
 	// because external-domain-tls was not enabled.
 	ExternalDomainTLSNotEnabledMessage = "external-domain-tls is not enabled"
-
-	// TLSNotEnabledForClusterLocalMessage is the message which is set on the
-	// RouteConditionCertificateProvisioned condition when it is set to True
-	// because the domain is cluster-local.
-	TLSNotEnabledForClusterLocalMessage = "TLS is not enabled for cluster-local"
 )
 
 // MarkTLSNotEnabled sets RouteConditionCertificateProvisioned to true when

pkg/reconciler/accessor/networking/certificate.go

@@ -63,10 +63,11 @@ func ReconcileCertificate(ctx context.Context, owner kmeta.Accessor, desired *v1
 		return nil, kaccessor.NewAccessorError(
 			fmt.Errorf("owner: %s with Type %T does not own Certificate: %q", owner.GetName(), owner, cert.Name),
 			kaccessor.NotOwnResource)
-	} else if !equality.Semantic.DeepEqual(cert.Spec, desired.Spec) {
+	} else if !equality.Semantic.DeepEqual(cert.Spec, desired.Spec) || !equality.Semantic.DeepEqual(cert.Annotations, desired.Annotations) {
 		// Don't modify the informers copy
 		existing := cert.DeepCopy()
 		existing.Spec = desired.Spec
+		existing.Annotations = desired.Annotations
 		cert, err = certAccessor.GetNetworkingClient().NetworkingV1alpha1().Certificates(existing.Namespace).Update(ctx, existing, metav1.UpdateOptions{})
 		if err != nil {
 			recorder.Eventf(owner, corev1.EventTypeWarning, "UpdateFailed",

pkg/reconciler/domainmapping/reconciler.go

@@ -228,7 +228,7 @@ func (r *Reconciler) tls(ctx context.Context, dm *v1beta1.DomainMapping) ([]netv
 	}
 	if cert.IsReady() {
 		dm.Status.MarkCertificateReady(cert.Name)
-		return []netv1alpha1.IngressTLS{routeresources.MakeIngressTLS(cert, desiredCert.Spec.DNSNames)}, nil, nil
+		return []netv1alpha1.IngressTLS{routeresources.MakeIngressTLS(cert, desiredCert.Spec.DNSNames, netv1alpha1.IngressVisibilityExternalIP)}, nil, nil
 	}
 	if config.FromContext(ctx).Network.HTTPProtocol == netcfg.HTTPEnabled {
 		// When httpProtocol is enabled, downgrade http scheme.

pkg/reconciler/domainmapping/table_test.go

@@ -997,6 +997,7 @@ func TestReconcileTLSEnabled(t *testing.T) {
 					Hosts:           []string{"becomes.ready.run"},
 					SecretName:      "becomes.ready.run",
 					SecretNamespace: "default",
+					Visibility:      netv1alpha1.IngressVisibilityExternalIP,
 				})),
 		}},
 		WantPatches: []clientgotesting.PatchActionImpl{

pkg/reconciler/route/domains/domains.go

@@ -24,11 +24,13 @@
 	"text/template"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	netapi "knative.dev/networking/pkg/apis/networking"
 	netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
 	netcfg "knative.dev/networking/pkg/config"
+	"knative.dev/networking/pkg/ingress"
 	"knative.dev/pkg/apis"
 	pkgnet "knative.dev/pkg/network"
 	"knative.dev/serving/pkg/apis/serving"
@@ -63,6 +65,29 @@
 	return domainTagMap, nil
 }
 
+// GetAllClusterLocalDomains return all cluster local domains associated with a Route
+// TODO: RLE check this, possibly it returns more, before it was named `routeDomain`
+func GetAllClusterLocalDomains(ctx context.Context, targetName string, r *v1.Route, visibility netv1alpha1.IngressVisibility) (sets.String, error) {
+	hostname, err := HostnameFromTemplate(ctx, r.Name, targetName)
+	if err != nil {
+		return nil, err
+	}
+
+	meta := r.ObjectMeta.DeepCopy()
+	isClusterLocal := visibility == netv1alpha1.IngressVisibilityClusterLocal
+	labels.SetVisibility(meta, isClusterLocal)
+
+	domain, err := DomainNameFromTemplate(ctx, *meta, hostname)
+	if err != nil {
+		return nil, err
+	}
+	domains := []string{domain}
+	if isClusterLocal {
+		domains = ingress.ExpandedHosts(sets.NewString(domains...)).List()
+	}
+	return sets.NewString(domains...), err
+}
+
 // DomainNameFromTemplate generates domain name base on the template specified in the `config-network` ConfigMap.
 // name is the "subdomain" which will be referred as the "name" in the template
 func DomainNameFromTemplate(ctx context.Context, r metav1.ObjectMeta, name string) (string, error) {

pkg/reconciler/route/resources/certificate.go

@@ -21,7 +21,10 @@
 	"hash/adler32"
 	"sort"
 
+	"k8s.io/apimachinery/pkg/util/sets"
 	"knative.dev/networking/pkg/apis/networking"
+	"knative.dev/pkg/kmap"
+	"knative.dev/pkg/network"
 	"knative.dev/serving/pkg/apis/serving"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -31,6 +34,10 @@
 	"knative.dev/serving/pkg/reconciler/route/resources/names"
 )
 
+const (
+	internalCertificateSuffix = "-internal"
+)
+
 // MakeCertificate creates a Certificate, inheriting the certClass
 // annotations from the owner, as well as the namespaces. If owner
 // does not have a certClass, use the provided `certClass` parameter.
@@ -72,17 +79,52 @@
 	certs := make([]*networkingv1alpha1.Certificate, 0, len(order))
 	for _, dnsName := range order {
 		tag := domainTagMap[dnsName]
-
-		// k8s supports cert name only up to 63 chars and so is constructed as route-[UID]-[tag digest]
-		// where route-[UID] will take 42 characters and leaves 20 characters for tag digest (need to include `-`).
-		// We use https://golang.org/pkg/hash/adler32/#Checksum to compute the digest which returns a uint32.
-		// We represent the digest in unsigned integer format with maximum value of 4,294,967,295 which are 10 digits.
-		// The "-[tag digest]" is computed only if there's a tag
-		certName := names.Certificate(route)
-		if tag != "" {
-			certName += fmt.Sprint("-", adler32.Checksum([]byte(tag)))
-		}
-		certs = append(certs, MakeCertificate(route, serving.RouteLabelKey, dnsName, certName, certClass, domain))
+		certs = append(certs, MakeCertificate(route, serving.RouteLabelKey, dnsName, certNameFromRouteAndTag(route, tag), certClass, domain))
 	}
 	return certs
 }
+
+func MakeInternalCertificate(route *v1.Route, tag string, domains sets.String, certClass string) *networkingv1alpha1.Certificate {
+	domainsOrdered := make(sort.StringSlice, 0, len(domains))
+	for dnsName := range domains {
+		domainsOrdered = append(domainsOrdered, dnsName)
+	}
+	domainsOrdered.Sort()
+
+	certName := certNameFromRouteAndTag(route, tag) + internalCertificateSuffix
+
+	return &networkingv1alpha1.Certificate{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:            certName,
+			Namespace:       route.GetNamespace(),
+			OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(route)},
+			Annotations: kmap.Filter(kmap.Union(map[string]string{
+				networking.CertificateClassAnnotationKey: certClass,
+			}, route.GetAnnotations()), ExcludedAnnotations.Has),
+			Labels: map[string]string{
+				serving.RouteLabelKey:         route.GetName(),
+				networking.VisibilityLabelKey: serving.VisibilityClusterLocal,
+			},
+		},
+		Spec: networkingv1alpha1.CertificateSpec{
+			DNSNames:   domainsOrdered,
+			Domain:     "svc." + network.GetClusterDomainName(),
+			SecretName: certName,
+		},
+	}
+}
+
+// certNameFromRouteAndTag returns a possibly shortended certName as
+// k8s supports cert name only up to 63 chars and so is constructed as route-[UID]-[tag digest]
+// where route-[UID] will take 42 characters and leaves 20 characters for tag digest (need to include `-`).
+// We use https://golang.org/pkg/hash/adler32/#Checksum to compute the digest which returns a uint32.
+// We represent the digest in unsigned integer format with maximum value of 4,294,967,295 which are 10 digits.
+// The "-[tag digest]" is computed only if there's a tag
+func certNameFromRouteAndTag(route *v1.Route, tag string) string {
+
+	certName := names.Certificate(route)
+	if tag != "" {
+		certName += fmt.Sprint("-", adler32.Checksum([]byte(tag)))
+	}
+	return certName
+}