Make route domain error specific (knative#15082) (#685)

* make route domain error specific

* fixes

* fix quote

* move to errors.Is

* lint

pkg/apis/serving/v1/route_lifecycle.go

@@ -117,6 +117,12 @@ func (rs *RouteStatus) MarkUnknownTrafficError(msg string) {
 	routeCondSet.Manage(rs).MarkUnknown(RouteConditionAllTrafficAssigned, "Unknown", msg)
 }
 
+// MarkRevisionTargetTrafficError marks the RouteConditionAllTrafficAssigned condition
+// to indicate an error has occurred wrt a revision target.
+func (rs *RouteStatus) MarkRevisionTargetTrafficError(reason, msg string) {
+	routeCondSet.Manage(rs).MarkFalse(RouteConditionAllTrafficAssigned, reason, msg)
+}
+
 // MarkConfigurationNotReady marks the RouteConditionAllTrafficAssigned
 // condition to indiciate the Revision is not yet ready.
 func (rs *RouteStatus) MarkConfigurationNotReady(name string) {

pkg/reconciler/route/domains/domains.go

@@ -19,6 +19,7 @@ package domains
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 	"text/template"
@@ -40,6 +41,10 @@ import (
 // HTTPScheme is the string representation of http.
 const HTTPScheme string = "http"
 
+var (
+	ErrorDomainName = errors.New("domain name error")
+)
+
 // GetAllDomainsAndTags returns all of the domains and tags(including subdomains) associated with a Route
 func GetAllDomainsAndTags(ctx context.Context, r *v1.Route, names []string, visibility map[string]netv1alpha1.IngressVisibility) (map[string]string, error) {
 	domainTagMap := make(map[string]string)
@@ -95,12 +100,12 @@ func DomainNameFromTemplate(ctx context.Context, r metav1.ObjectMeta, name strin
 	}
 
 	if err := templ.Execute(&buf, data); err != nil {
-		return "", fmt.Errorf("error executing the DomainTemplate: %w", err)
+		return "", fmt.Errorf("%w: error executing the DomainTemplate: %w", ErrorDomainName, err)
 	}
 
 	urlErrs := validation.IsFullyQualifiedDomainName(field.NewPath("url"), buf.String())
 	if urlErrs != nil {
-		return "", fmt.Errorf("invalid domain name %q: %w", buf.String(), urlErrs.ToAggregate())
+		return "", fmt.Errorf("%w: invalid domain name %q: %w", ErrorDomainName, buf.String(), urlErrs.ToAggregate())
 	}
 
 	return buf.String(), nil
@@ -123,7 +128,7 @@ func HostnameFromTemplate(ctx context.Context, name, tag string) (string, error)
 	networkConfig := config.FromContext(ctx).Network
 	buf := bytes.Buffer{}
 	if err := networkConfig.GetTagTemplate().Execute(&buf, data); err != nil {
-		return "", fmt.Errorf("error executing the TagTemplate: %w", err)
+		return "", fmt.Errorf("%w: error executing the TagTemplate: %w", ErrorDomainName, err)
 	}
 	return buf.String(), nil
 }

pkg/reconciler/route/route.go

@@ -79,6 +79,8 @@ type Reconciler struct {
 	enqueueAfter func(interface{}, time.Duration)
 }
 
+const errorConfigMsg = "ErrorConfig"
+
 // Check that our Reconciler implements routereconciler.Interface
 var _ routereconciler.Interface = (*Reconciler)(nil)
 
@@ -118,7 +120,11 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, r *v1.Route) pkgreconcil
 	traffic, err := c.configureTraffic(ctx, r)
 	if traffic == nil || err != nil {
 		if err != nil {
-			r.Status.MarkUnknownTrafficError(err.Error())
+			if errors.Is(err, domains.ErrorDomainName) {
+				r.Status.MarkRevisionTargetTrafficError(errorConfigMsg, err.Error())
+			} else {
+				r.Status.MarkUnknownTrafficError(err.Error())
+			}
 		}
 		// Traffic targets aren't ready, no need to configure child resources.
 		return err

pkg/reconciler/route/table_test.go

@@ -53,6 +53,7 @@ import (
 	routereconciler "knative.dev/serving/pkg/client/injection/reconciler/serving/v1/route"
 	kaccessor "knative.dev/serving/pkg/reconciler/accessor"
 	"knative.dev/serving/pkg/reconciler/route/config"
+	"knative.dev/serving/pkg/reconciler/route/domains"
 	"knative.dev/serving/pkg/reconciler/route/resources"
 	"knative.dev/serving/pkg/reconciler/route/traffic"
 
@@ -2047,7 +2048,7 @@ func TestReconcile(t *testing.T) {
 			Object: Route("default", "tooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo-long",
 				WithConfigTarget("config"), WithRouteObservedGeneration,
 				WithRouteFinalizer, WithInitRouteConditions,
-				MarkUnknownTrafficError(`invalid domain name "tooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo-long.default.example.com": url: Invalid value: "tooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo-long": must be no more than 63 characters`),
+				MarkRevisionTargetTrafficError(errorConfigMsg, domains.ErrorDomainName.Error()+`: invalid domain name "tooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo-long.default.example.com": url: Invalid value: "tooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo-long": must be no more than 63 characters`),
 				WithHost("tooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo-long.default.svc.cluster.local"),
 			),
 		}},

pkg/testing/v1/route.go

@@ -199,10 +199,10 @@ func MarkTrafficAssigned(r *v1.Route) {
 	r.Status.MarkTrafficAssigned()
 }
 
-// MarkUnknownTrafficError calls the method of the same name on .Status
-func MarkUnknownTrafficError(msg string) RouteOption {
+// MarkRevisionTargetTrafficError calls the method of the same name on .Status
+func MarkRevisionTargetTrafficError(reason, msg string) RouteOption {
 	return func(r *v1.Route) {
-		r.Status.MarkUnknownTrafficError(msg)
+		r.Status.MarkRevisionTargetTrafficError(reason, msg)
 	}
 }