Skip to content

Commit

Permalink
Make route domain error specific (#15082)
Browse files Browse the repository at this point in the history
* make route domain error specific

* fixes

* fix quote

* move to errors.Is

* lint
  • Loading branch information
skonto authored Apr 11, 2024
1 parent 614caa0 commit dc8bf98
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 8 deletions.
6 changes: 6 additions & 0 deletions pkg/apis/serving/v1/route_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
11 changes: 8 additions & 3 deletions pkg/reconciler/route/domains/domains.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package domains
import (
"bytes"
"context"
"errors"
"fmt"
"strings"
"text/template"
Expand All @@ -42,6 +43,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)
Expand Down Expand Up @@ -119,12 +124,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
Expand All @@ -147,7 +152,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
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/reconciler/route/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion pkg/reconciler/route/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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"),
),
}},
Expand Down
6 changes: 3 additions & 3 deletions pkg/testing/v1/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,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)
}
}

Expand Down

0 comments on commit dc8bf98

Please sign in to comment.