fix cluster-local routes being stalled when external-domain-tls is en…

…abled

pkg/apis/serving/v1/route_lifecycle.go

@@ -202,6 +202,11 @@ 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/route/route.go

@@ -143,7 +143,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, r *v1.Route) pkgreconcil
 		return err
 	}
 
-	tls, acmeChallenges, err := c.externalDomainTLS(ctx, r.Status.URL.Host, r, traffic)
+	tls, acmeChallenges, desiredCerts, err := c.externalDomainTLS(ctx, r.Status.URL.Host, r, traffic)
 	if err != nil {
 		return err
 	}
@@ -154,6 +154,10 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, r *v1.Route) pkgreconcil
 			return err
 		}
 		tls = append(tls, internalTLS...)
+	} else if externalDomainTLSEnabled(ctx, r) && len(desiredCerts) == 0 {
+		// If external TLS is enabled but we have no desired certs then the route
+		// must have only cluster local hosts
+		r.Status.MarkTLSNotEnabled(v1.TLSNotEnabledForClusterLocalMessage)
 	}
 
 	// Reconcile ingress and its children resources.
@@ -195,18 +199,24 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, r *v1.Route) pkgreconcil
 	return nil
 }
 
-func (c *Reconciler) externalDomainTLS(ctx context.Context, host string, r *v1.Route, traffic *traffic.Config) ([]netv1alpha1.IngressTLS, []netv1alpha1.HTTP01Challenge, error) {
+func (c *Reconciler) externalDomainTLS(ctx context.Context, host string, r *v1.Route, traffic *traffic.Config) (
+	[]netv1alpha1.IngressTLS,
+	[]netv1alpha1.HTTP01Challenge,
+	[]*netv1alpha1.Certificate,
+	error,
+) {
+	var desiredCerts []*netv1alpha1.Certificate
 	logger := logging.FromContext(ctx)
 
 	tls := []netv1alpha1.IngressTLS{}
 	if !externalDomainTLSEnabled(ctx, r) {
 		r.Status.MarkTLSNotEnabled(v1.ExternalDomainTLSNotEnabledMessage)
-		return tls, nil, nil
+		return tls, nil, desiredCerts, nil
 	}
 
 	domainToTagMap, err := domains.GetAllDomainsAndTags(ctx, r, getTrafficNames(traffic.Targets), traffic.Visibility)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, desiredCerts, err
 	}
 
 	for domain := range domainToTagMap {
@@ -223,15 +233,15 @@ func (c *Reconciler) externalDomainTLS(ctx context.Context, host string, r *v1.R
 
 	allWildcardCerts, err := c.certificateLister.Certificates(r.Namespace).List(labelSelector)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, desiredCerts, err
 	}
 
 	domainConfig := config.FromContext(ctx).Domain
 	rLabels := r.Labels
 	domain := domainConfig.LookupDomainForLabels(rLabels)
 
 	acmeChallenges := []netv1alpha1.HTTP01Challenge{}
-	desiredCerts := resources.MakeCertificates(r, domainToTagMap, certClass(ctx, r), domain)
+	desiredCerts = resources.MakeCertificates(r, domainToTagMap, certClass(ctx, r), domain)
 	for _, desiredCert := range desiredCerts {
 		dnsNames := sets.New(desiredCert.Spec.DNSNames...)
 		// Look for a matching wildcard cert before provisioning a new one. This saves the
@@ -247,7 +257,7 @@ func (c *Reconciler) externalDomainTLS(ctx context.Context, host string, r *v1.R
 				} else {
 					r.Status.MarkCertificateProvisionFailed(desiredCert)
 				}
-				return nil, nil, err
+				return nil, nil, desiredCerts, err
 			}
 			dnsNames = sets.New(cert.Spec.DNSNames...)
 		}
@@ -306,12 +316,12 @@ func (c *Reconciler) externalDomainTLS(ctx context.Context, host string, r *v1.R
 
 	orphanCerts, err := c.getOrphanRouteCerts(r, domainToTagMap, netcfg.CertificateExternalDomain)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, desiredCerts, err
 	}
 
 	c.deleteOrphanedCerts(ctx, orphanCerts)
 
-	return tls, acmeChallenges, nil
+	return tls, acmeChallenges, desiredCerts, nil
 }
 
 func (c *Reconciler) clusterLocalDomainTLS(ctx context.Context, r *v1.Route, tc *traffic.Config) ([]netv1alpha1.IngressTLS, error) {

pkg/reconciler/route/table_test.go

@@ -75,6 +75,7 @@ const (
 	rolloutDurationKey key = iota
 	externalSchemeKey
 	enableExternalDomainTLSKey
+	domainConfigKey
 )
 
 // This is heavily based on the way the OpenShift Ingress controller tests its reconciliation method.
@@ -3276,6 +3277,7 @@ func TestReconcileEnableExternalDomainTLS(t *testing.T) {
 				WithRouteUID("65-23"),
 				WithRouteGeneration(1), WithRouteObservedGeneration,
 				MarkTrafficAssigned, MarkIngressNotConfigured,
+				WithRouteConditionsTLSNotEnabledForClusterLocalMessage,
 				WithLocalDomain, WithAddress, WithInitRouteConditions,
 				WithRouteLabel(map[string]string{netapi.VisibilityLabelKey: serving.VisibilityClusterLocal}),
 				WithStatusTraffic(
@@ -3286,6 +3288,66 @@ func TestReconcileEnableExternalDomainTLS(t *testing.T) {
 					})),
 		}},
 		Key: "default/becomes-local",
+	}, {
+		Name: "local domain route should mark certificate provisioned TLS disabled",
+		Key:  "default/local-domain",
+		Ctx: context.WithValue(context.Background(), domainConfigKey, &config.Domain{
+			Domains: map[string]config.DomainConfig{
+				"svc.cluster.local": {},
+			},
+		}),
+		Objects: []runtime.Object{
+			Route("default", "local-domain", WithConfigTarget("config"), WithRouteGeneration(1),
+				WithRouteObservedGeneration,
+				WithRouteUID("65-23")),
+			cfg("default", "config",
+				WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")),
+			rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")),
+			simpleIngress(
+				Route("default", "local-domain", WithConfigTarget("config"), WithRouteUID("65-23")),
+				&traffic.Config{
+					Targets: map[string]traffic.RevisionTargets{
+						traffic.DefaultTarget: {{
+							TrafficTarget: v1.TrafficTarget{
+								ConfigurationName: "config",
+								LatestRevision:    ptr.Bool(true),
+								RevisionName:      "config-00001",
+								Percent:           ptr.Int64(100),
+							},
+						}},
+					},
+				},
+				withReadyIngress,
+				// simpleIngress and the test use different 'configs' (limit of reading config from the context)
+				// so we need to delete the external visible host rules
+				func(i *netv1alpha1.Ingress) {
+					localRules := i.Spec.Rules[:0]
+					for _, rule := range i.Spec.Rules {
+						if rule.Visibility == netv1alpha1.IngressVisibilityClusterLocal {
+							localRules = append(localRules, rule)
+						}
+					}
+					i.Spec.Rules = localRules
+				},
+			),
+			simpleK8sService(Route("default", "local-domain", WithConfigTarget("config"),
+				WithRouteUID("65-23"))),
+		},
+		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: Route("default", "local-domain", WithConfigTarget("config"),
+				WithRouteUID("65-23"),
+				WithRouteGeneration(1), WithRouteObservedGeneration,
+				MarkTrafficAssigned,
+				MarkIngressReady,
+				WithRouteConditionsTLSNotEnabledForClusterLocalMessage,
+				WithLocalDomain, WithAddress, WithInitRouteConditions,
+				WithStatusTraffic(
+					v1.TrafficTarget{
+						RevisionName:   "config-00001",
+						Percent:        ptr.Int64(100),
+						LatestRevision: ptr.Bool(true),
+					})),
+		}},
 	}}
 
 	for i, row := range table {
@@ -3323,6 +3385,9 @@ func NewTestReconciler(ctx context.Context, listers *Listers, cmw configmap.Watc
 	if v := ctx.Value(externalSchemeKey); v != nil {
 		cfg.Network.DefaultExternalScheme = v.(string)
 	}
+	if v := ctx.Value(domainConfigKey); v != nil {
+		cfg.Domain = v.(*config.Domain)
+	}
 
 	return routereconciler.NewReconciler(ctx,
 		logging.FromContext(ctx),

pkg/testing/v1/route.go

@@ -180,6 +180,14 @@ func WithRouteConditionsExternalDomainTLSDisabled(rt *v1.Route) {
 	rt.Status.MarkTLSNotEnabled(v1.ExternalDomainTLSNotEnabledMessage)
 }
 
+// WithRouteConditionsTLSNotEnabledForClusterLocalMessage calls
+// MarkTLSNotEnabled with TLSNotEnabledForClusterLocalMessage after initialized
+// the Service's conditions.
+func WithRouteConditionsTLSNotEnabledForClusterLocalMessage(rt *v1.Route) {
+	rt.Status.InitializeConditions()
+	rt.Status.MarkTLSNotEnabled(v1.TLSNotEnabledForClusterLocalMessage)
+}
+
 // WithRouteConditionsHTTPDowngrade calls MarkHTTPDowngrade after initialized the Service's conditions.
 func WithRouteConditionsHTTPDowngrade(rt *v1.Route) {
 	rt.Status.InitializeConditions()