From 225151e9b1eccf6765863f0b7dd259c398ee4085 Mon Sep 17 00:00:00 2001 From: kahirokunn Date: Thu, 27 Aug 2026 00:47:59 +0900 Subject: [PATCH] Use Route traffic for Knative DomainMappings Signed-off-by: kahirokunn --- .../v1beta1/domainmapping_lifecycle.go | 12 + .../v1beta1/domainmapping_lifecycle_test.go | 51 ++ pkg/reconciler/domainmapping/controller.go | 27 +- pkg/reconciler/domainmapping/reconciler.go | 142 ++++- .../domainmapping/resources/ingress.go | 54 +- .../domainmapping/resources/ingress_test.go | 58 ++ .../domainmapping/resources/names/names.go | 25 + pkg/reconciler/domainmapping/table_test.go | 552 +++++++++++++++--- 8 files changed, 834 insertions(+), 87 deletions(-) create mode 100644 pkg/reconciler/domainmapping/resources/names/names.go diff --git a/pkg/apis/serving/v1beta1/domainmapping_lifecycle.go b/pkg/apis/serving/v1beta1/domainmapping_lifecycle.go index 06ede3d3152a..0f1b5233d3a1 100644 --- a/pkg/apis/serving/v1beta1/domainmapping_lifecycle.go +++ b/pkg/apis/serving/v1beta1/domainmapping_lifecycle.go @@ -127,6 +127,18 @@ func (dms *DomainMappingStatus) MarkIngressNotConfigured() { "IngressNotConfigured", "Ingress has not yet been reconciled.") } +// MarkTargetIngressNotConfigured marks DomainMappingConditionIngressReady unknown. +func (dms *DomainMappingStatus) MarkTargetIngressNotConfigured(message string) { + domainMappingCondSet.Manage(dms).MarkUnknown(DomainMappingConditionIngressReady, + "IngressNotConfigured", message) +} + +// MarkTargetNotOwned marks DomainMappingConditionIngressReady false. +func (dms *DomainMappingStatus) MarkTargetNotOwned(message string) { + domainMappingCondSet.Manage(dms).MarkFalse(DomainMappingConditionIngressReady, + "NotOwned", message) +} + // MarkDomainClaimed updates the DomainMappingConditionDomainClaimed condition // to indicate that the domain was successfully claimed. func (dms *DomainMappingStatus) MarkDomainClaimed() { diff --git a/pkg/apis/serving/v1beta1/domainmapping_lifecycle_test.go b/pkg/apis/serving/v1beta1/domainmapping_lifecycle_test.go index 479188140180..8ce480635677 100644 --- a/pkg/apis/serving/v1beta1/domainmapping_lifecycle_test.go +++ b/pkg/apis/serving/v1beta1/domainmapping_lifecycle_test.go @@ -131,6 +131,57 @@ func TestReferenceResolvedCondition(t *testing.T) { apistest.CheckConditionFailed(dms, DomainMappingConditionReady, t) } +func TestTargetIngressConditions(t *testing.T) { + tests := []struct { + name string + mark func(*DomainMappingStatus) + wantStatus corev1.ConditionStatus + wantReason string + wantMsg string + }{ + { + name: "target ingress not configured", + mark: func(dms *DomainMappingStatus) { + dms.MarkTargetIngressNotConfigured("Waiting for target Ingress.") + }, + wantStatus: corev1.ConditionUnknown, + wantReason: "IngressNotConfigured", + wantMsg: "Waiting for target Ingress.", + }, + { + name: "target resource not owned", + mark: func(dms *DomainMappingStatus) { + dms.MarkTargetNotOwned("Route does not own Ingress.") + }, + wantStatus: corev1.ConditionFalse, + wantReason: "NotOwned", + wantMsg: "Route does not own Ingress.", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + dms := &DomainMappingStatus{} + dms.InitializeConditions() + test.mark(dms) + + got := dms.GetCondition(DomainMappingConditionIngressReady) + if got == nil { + t.Fatal("IngressReady condition is nil") + } + if got.Status != test.wantStatus { + t.Errorf("Status = %q, want %q", got.Status, test.wantStatus) + } + if got.Reason != test.wantReason { + t.Errorf("Reason = %q, want %q", got.Reason, test.wantReason) + } + if got.Message != test.wantMsg { + t.Errorf("Message = %q, want %q", got.Message, test.wantMsg) + } + }) + } +} + func TestCertificateNotReady(t *testing.T) { dms := &DomainMappingStatus{} diff --git a/pkg/reconciler/domainmapping/controller.go b/pkg/reconciler/domainmapping/controller.go index e22eb3e99021..74e810466d6a 100644 --- a/pkg/reconciler/domainmapping/controller.go +++ b/pkg/reconciler/domainmapping/controller.go @@ -20,6 +20,7 @@ import ( "context" "k8s.io/client-go/tools/cache" + netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1" netclient "knative.dev/networking/pkg/client/injection/client" certificateinformer "knative.dev/networking/pkg/client/injection/informers/networking/v1alpha1/certificate" domainclaiminformer "knative.dev/networking/pkg/client/injection/informers/networking/v1alpha1/clusterdomainclaim" @@ -29,7 +30,10 @@ import ( "knative.dev/pkg/controller" "knative.dev/pkg/logging" "knative.dev/pkg/resolver" + servingv1 "knative.dev/serving/pkg/apis/serving/v1" "knative.dev/serving/pkg/apis/serving/v1beta1" + routeinformer "knative.dev/serving/pkg/client/injection/informers/serving/v1/route" + serviceinformer "knative.dev/serving/pkg/client/injection/informers/serving/v1/service" "knative.dev/serving/pkg/client/injection/informers/serving/v1beta1/domainmapping" kindreconciler "knative.dev/serving/pkg/client/injection/reconciler/serving/v1beta1/domainmapping" "knative.dev/serving/pkg/reconciler/domainmapping/config" @@ -42,11 +46,15 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl domainmappingInformer := domainmapping.Get(ctx) ingressInformer := ingressinformer.Get(ctx) domainClaimInformer := domainclaiminformer.Get(ctx) + routeInformer := routeinformer.Get(ctx) + serviceInformer := serviceinformer.Get(ctx) r := &Reconciler{ certificateLister: certificateInformer.Lister(), ingressLister: ingressInformer.Lister(), domainClaimLister: domainClaimInformer.Lister(), + routeLister: routeInformer.Lister(), + serviceLister: serviceInformer.Lister(), netclient: netclient.Get(ctx), } @@ -71,7 +79,24 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl certificateInformer.Informer().AddEventHandler(handleControllerOf) ingressInformer.Informer().AddEventHandler(handleControllerOf) - r.resolver = resolver.NewURIResolverFromTracker(ctx, impl.Tracker) + r.tracker = impl.Tracker + r.resolver = resolver.NewURIResolverFromTracker(ctx, r.tracker) + + // Track Route and Ingress changes because their HTTP paths are copied into + // DomainMapping Ingresses. The resolver already tracks referenced Services. + // Informer events may omit TypeMeta, so populate it before notifying the tracker. + routeInformer.Informer().AddEventHandler(controller.HandleAll( + controller.EnsureTypeMeta(r.tracker.OnChanged, servingv1.SchemeGroupVersion.WithKind("Route")), + )) + ingressInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: controller.FilterController(&servingv1.Route{}), + Handler: controller.HandleAll( + controller.EnsureTypeMeta(r.tracker.OnChanged, netv1alpha1.SchemeGroupVersion.WithKind("Ingress")), + ), + }) + domainmappingInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + DeleteFunc: r.tracker.OnDeletedObserver, + }) return impl } diff --git a/pkg/reconciler/domainmapping/reconciler.go b/pkg/reconciler/domainmapping/reconciler.go index 481886bafe2a..4e3cf9979e34 100644 --- a/pkg/reconciler/domainmapping/reconciler.go +++ b/pkg/reconciler/domainmapping/reconciler.go @@ -18,6 +18,7 @@ package domainmapping import ( "context" + "errors" "fmt" "slices" "sort" @@ -31,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/api/equality" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" netapi "knative.dev/networking/pkg/apis/networking" netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1" @@ -44,13 +46,18 @@ import ( "knative.dev/pkg/network" "knative.dev/pkg/reconciler" "knative.dev/pkg/resolver" - v1 "knative.dev/serving/pkg/apis/serving/v1" + "knative.dev/pkg/tracker" + "knative.dev/serving/pkg/apis/serving" + servingv1 "knative.dev/serving/pkg/apis/serving/v1" "knative.dev/serving/pkg/apis/serving/v1beta1" domainmappingreconciler "knative.dev/serving/pkg/client/injection/reconciler/serving/v1beta1/domainmapping" + servinglisters "knative.dev/serving/pkg/client/listers/serving/v1" servingnetworking "knative.dev/serving/pkg/networking" "knative.dev/serving/pkg/reconciler/domainmapping/config" "knative.dev/serving/pkg/reconciler/domainmapping/resources" routeresources "knative.dev/serving/pkg/reconciler/route/resources" + routenames "knative.dev/serving/pkg/reconciler/route/resources/names" + servicenames "knative.dev/serving/pkg/reconciler/service/resources/names" ) // Reconciler implements controller.Reconciler for DomainMapping resources. @@ -58,8 +65,11 @@ type Reconciler struct { certificateLister networkinglisters.CertificateLister ingressLister networkinglisters.IngressLister domainClaimLister networkinglisters.ClusterDomainClaimLister + serviceLister servinglisters.ServiceLister + routeLister servinglisters.RouteLister netclient netclientset.Interface resolver *resolver.URIResolver + tracker tracker.Interface } // Check that our Reconciler implements Interface @@ -116,12 +126,32 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, dm *v1beta1.DomainMappin return err } + // For Knative targets, preserve Route traffic instead of sending everything + // to the Service resolved from the Addressable URL. + targetKind, preserveRouteTraffic := servingReferenceKind(dm.Spec.Ref) + // Resolve the spec.Ref to a URI following the Addressable contract. targetHost, targetBackendSvc, err := r.resolveRef(ctx, dm) if err != nil { + if !preserveRouteTraffic { + return err + } + dm.Status.MarkIngressNotConfigured() return err } + var routePaths []netv1alpha1.HTTPIngressPath + if preserveRouteTraffic { + var found bool + routePaths, found, err = r.routeHTTPPaths(dm, targetKind, targetHost) + if err != nil { + return err + } + if !found { + return nil + } + } + // HTTPOption can be set via annotations or in the config map. httpOption, err := servingnetworking.GetHTTPOption(ctx, config.FromContext(ctx).Network, dm.GetAnnotations()) if err != nil { @@ -130,7 +160,12 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, dm *v1beta1.DomainMappin // Reconcile the Ingress resource corresponding to the requested Mapping. logger.Debugf("Mapping %s to ref %s/%s (host: %q, svc: %q)", url, dm.Spec.Ref.Namespace, dm.Spec.Ref.Name, targetHost, targetBackendSvc) - desired := resources.MakeIngress(dm, targetBackendSvc, targetHost, ingressClass, httpOption, tls, acmeChallenges...) + var desired *netv1alpha1.Ingress + if preserveRouteTraffic { + desired = resources.MakeIngressWithHTTPPaths(dm, routePaths, ingressClass, httpOption, tls, acmeChallenges...) + } else { + desired = resources.MakeIngress(dm, targetBackendSvc, targetHost, ingressClass, httpOption, tls, acmeChallenges...) + } ingress, err := r.reconcileIngress(ctx, dm, desired) if err != nil { return err @@ -205,7 +240,7 @@ func (r *Reconciler) tls(ctx context.Context, dm *v1beta1.DomainMapping) ([]netv } if !externalDomainTLSEnabled(ctx, dm) { - dm.Status.MarkTLSNotEnabled(v1.ExternalDomainTLSNotEnabledMessage) + dm.Status.MarkTLSNotEnabled(servingv1.ExternalDomainTLSNotEnabledMessage) return nil, nil, nil } @@ -317,6 +352,107 @@ func (r *Reconciler) resolveRef(ctx context.Context, dm *v1beta1.DomainMapping) return resolved.Host, parts[0], nil } +// routeHTTPPaths gets the first matching cluster-local paths for a Service or +// Route target. If no expected source can be found, it updates IngressReady and +// returns found=false. The source Ingress is responsible for validating paths. +// The returned paths belong to the informer cache and must not be mutated. +func (r *Reconciler) routeHTTPPaths(dm *v1beta1.DomainMapping, kind, targetHost string) (paths []netv1alpha1.HTTPIngressPath, found bool, err error) { + ref := dm.Spec.Ref + + if r.tracker == nil { + return nil, false, errors.New("DomainMapping target tracker is not configured") + } + + routeName := ref.Name + var service *servingv1.Service + if kind == "Service" { + service, err = r.serviceLister.Services(ref.Namespace).Get(ref.Name) + if apierrs.IsNotFound(err) { + dm.Status.MarkTargetIngressNotConfigured(fmt.Sprintf("Waiting for target Service %s/%s to be observed.", ref.Namespace, ref.Name)) + return nil, false, nil + } else if err != nil { + return nil, false, fmt.Errorf("getting target Service %s/%s: %w", ref.Namespace, ref.Name, err) + } + routeName = servicenames.Route(service) + } + + if err := r.tracker.TrackReference(tracker.Reference{ + APIVersion: servingv1.SchemeGroupVersion.String(), + Kind: "Route", + Namespace: ref.Namespace, + Name: routeName, + }, dm); err != nil { + return nil, false, fmt.Errorf("tracking target Route: %w", err) + } + + route, err := r.routeLister.Routes(ref.Namespace).Get(routeName) + if apierrs.IsNotFound(err) { + dm.Status.MarkTargetIngressNotConfigured(fmt.Sprintf("Waiting for target Route %s/%s to be created.", ref.Namespace, routeName)) + return nil, false, nil + } else if err != nil { + return nil, false, fmt.Errorf("getting target Route %s/%s: %w", ref.Namespace, routeName, err) + } + + if service != nil && !metav1.IsControlledBy(route, service) { + dm.Status.MarkTargetNotOwned(fmt.Sprintf("Service %s/%s does not own Route %s/%s.", service.Namespace, service.Name, route.Namespace, route.Name)) + return nil, false, nil + } + + ingressName := routenames.Ingress(route) + if err := r.tracker.TrackReference(tracker.Reference{ + APIVersion: netv1alpha1.SchemeGroupVersion.String(), + Kind: "Ingress", + Namespace: route.Namespace, + Name: ingressName, + }, dm); err != nil { + return nil, false, fmt.Errorf("tracking target Ingress: %w", err) + } + + ingress, err := r.ingressLister.Ingresses(route.Namespace).Get(ingressName) + if apierrs.IsNotFound(err) { + dm.Status.MarkTargetIngressNotConfigured(fmt.Sprintf("Waiting for target Route %s/%s to configure Ingress %s/%s.", route.Namespace, route.Name, route.Namespace, ingressName)) + return nil, false, nil + } else if err != nil { + return nil, false, fmt.Errorf("getting target Ingress %s/%s: %w", route.Namespace, ingressName, err) + } + if !metav1.IsControlledBy(ingress, route) { + dm.Status.MarkTargetNotOwned(fmt.Sprintf("Route %s/%s does not own Ingress %s/%s.", route.Namespace, route.Name, ingress.Namespace, ingress.Name)) + return nil, false, nil + } + + for i := range ingress.Spec.Rules { + rule := &ingress.Spec.Rules[i] + if rule.Visibility == netv1alpha1.IngressVisibilityClusterLocal && slices.Contains(rule.Hosts, targetHost) { + if rule.HTTP == nil { + return nil, false, fmt.Errorf("target Ingress %s/%s has a matching cluster-local rule without HTTP", ingress.Namespace, ingress.Name) + } + return rule.HTTP.Paths, true, nil + } + } + + dm.Status.MarkTargetIngressNotConfigured(fmt.Sprintf("Ingress %s/%s has no cluster-local rule for host %q.", ingress.Namespace, ingress.Name, targetHost)) + return nil, false, nil +} + +func servingReferenceKind(ref duckv1.KReference) (string, bool) { + group := ref.Group + if ref.APIVersion != "" { + gv, err := schema.ParseGroupVersion(ref.APIVersion) + if err != nil { + return "", false + } + group = gv.Group + } + if group != serving.GroupName { + return "", false + } + switch ref.Kind { + case "Service", "Route": + return ref.Kind, true + } + return "", false +} + func (r *Reconciler) reconcileDomainClaim(ctx context.Context, dm *v1beta1.DomainMapping) error { dc, err := r.domainClaimLister.Get(dm.Name) if err != nil && !apierrs.IsNotFound(err) { diff --git a/pkg/reconciler/domainmapping/resources/ingress.go b/pkg/reconciler/domainmapping/resources/ingress.go index 758a22bb55b0..c10acfb0dcbd 100644 --- a/pkg/reconciler/domainmapping/resources/ingress.go +++ b/pkg/reconciler/domainmapping/resources/ingress.go @@ -26,6 +26,7 @@ import ( "knative.dev/pkg/kmeta" "knative.dev/serving/pkg/apis/serving" servingv1beta1 "knative.dev/serving/pkg/apis/serving/v1beta1" + "knative.dev/serving/pkg/reconciler/domainmapping/resources/names" routeresources "knative.dev/serving/pkg/reconciler/route/resources" ) @@ -35,25 +36,46 @@ import ( // KIngress). The created ingress will contain a RewriteHost rule to cause the // given hostName to be used as the host. func MakeIngress(dm *servingv1beta1.DomainMapping, backendServiceName, hostName, ingressClass string, httpOption netv1alpha1.HTTPOption, tls []netv1alpha1.IngressTLS, acmeChallenges ...netv1alpha1.HTTP01Challenge) *netv1alpha1.Ingress { - // Traffic rule + paths := []netv1alpha1.HTTPIngressPath{{ + RewriteHost: hostName, + Splits: []netv1alpha1.IngressBackendSplit{{ + Percent: 100, + AppendHeaders: map[string]string{ + netheader.OriginalHostKey: dm.Name, + }, + IngressBackend: netv1alpha1.IngressBackend{ + ServiceNamespace: dm.Namespace, + ServiceName: backendServiceName, + ServicePort: intstr.FromInt(80), + }, + }}, + }} + return makeIngress(dm, paths, ingressClass, httpOption, tls, acmeChallenges...) +} + +// MakeIngressWithHTTPPaths creates an Ingress for a DomainMapping using HTTP +// paths from its target Route without modifying sourcePaths. +func MakeIngressWithHTTPPaths(dm *servingv1beta1.DomainMapping, sourcePaths []netv1alpha1.HTTPIngressPath, ingressClass string, httpOption netv1alpha1.HTTPOption, tls []netv1alpha1.IngressTLS, acmeChallenges ...netv1alpha1.HTTP01Challenge) *netv1alpha1.Ingress { + paths := make([]netv1alpha1.HTTPIngressPath, len(sourcePaths)) + for i := range sourcePaths { + sourcePaths[i].DeepCopyInto(&paths[i]) + paths[i].RewriteHost = "" + for j := range paths[i].Splits { + if paths[i].Splits[j].AppendHeaders == nil { + paths[i].Splits[j].AppendHeaders = make(map[string]string, 1) + } + paths[i].Splits[j].AppendHeaders[netheader.OriginalHostKey] = dm.Name + } + } + return makeIngress(dm, paths, ingressClass, httpOption, tls, acmeChallenges...) +} + +func makeIngress(dm *servingv1beta1.DomainMapping, paths []netv1alpha1.HTTPIngressPath, ingressClass string, httpOption netv1alpha1.HTTPOption, tls []netv1alpha1.IngressTLS, acmeChallenges ...netv1alpha1.HTTP01Challenge) *netv1alpha1.Ingress { rules := []netv1alpha1.IngressRule{{ Hosts: []string{dm.Name}, Visibility: netv1alpha1.IngressVisibilityExternalIP, HTTP: &netv1alpha1.HTTPIngressRuleValue{ - Paths: []netv1alpha1.HTTPIngressPath{{ - RewriteHost: hostName, - Splits: []netv1alpha1.IngressBackendSplit{{ - Percent: 100, - AppendHeaders: map[string]string{ - netheader.OriginalHostKey: dm.Name, - }, - IngressBackend: netv1alpha1.IngressBackend{ - ServiceNamespace: dm.Namespace, - ServiceName: backendServiceName, - ServicePort: intstr.FromInt(80), - }, - }}, - }}, + Paths: paths, }, }} @@ -78,7 +100,7 @@ func MakeIngress(dm *servingv1beta1.DomainMapping, backendServiceName, hostName, return &netv1alpha1.Ingress{ ObjectMeta: metav1.ObjectMeta{ - Name: kmeta.ChildName(dm.GetName(), ""), + Name: names.Ingress(dm), Namespace: dm.Namespace, Annotations: kmeta.FilterMap(kmeta.UnionMaps(map[string]string{ netapi.IngressClassAnnotationKey: ingressClass, diff --git a/pkg/reconciler/domainmapping/resources/ingress_test.go b/pkg/reconciler/domainmapping/resources/ingress_test.go index a22e24859ce2..cf247c092e73 100644 --- a/pkg/reconciler/domainmapping/resources/ingress_test.go +++ b/pkg/reconciler/domainmapping/resources/ingress_test.go @@ -315,3 +315,61 @@ func TestMakeIngress(t *testing.T) { }) } } + +func TestMakeIngressWithHTTPPaths(t *testing.T) { + dm := &v1beta1.DomainMapping{ObjectMeta: metav1.ObjectMeta{ + Name: "mapping.com", + Namespace: "default", + }} + source := []netv1alpha1.HTTPIngressPath{{ + RewriteHost: "old.internal.example", + AppendHeaders: map[string]string{ + "path-header": "preserved", + }, + Splits: []netv1alpha1.IngressBackendSplit{{ + IngressBackend: netv1alpha1.IngressBackend{ + ServiceNamespace: "default", + ServiceName: "app-00001", + ServicePort: intstr.FromInt(443), + }, + Percent: 75, + AppendHeaders: map[string]string{ + "Knative-Serving-Revision": "app-00001", + netheader.OriginalHostKey: "stale.example", + }, + }, { + IngressBackend: netv1alpha1.IngressBackend{ + ServiceNamespace: "default", + ServiceName: "app-00002", + ServicePort: intstr.FromInt(80), + }, + Percent: 25, + }}, + }} + + got := MakeIngressWithHTTPPaths(dm, source, "example.net/ingress", netv1alpha1.HTTPOptionEnabled, nil) + paths := got.Spec.Rules[0].HTTP.Paths + if got, want := paths[0].RewriteHost, ""; got != want { + t.Errorf("RewriteHost = %q, want %q", got, want) + } + if got, want := paths[0].Splits[0].ServicePort, intstr.FromInt(443); got != want { + t.Errorf("TLS backend port = %v, want %v", got, want) + } + if got, want := paths[0].Splits[0].AppendHeaders["Knative-Serving-Revision"], "app-00001"; got != want { + t.Errorf("revision header = %q, want %q", got, want) + } + for i := range paths[0].Splits { + if got, want := paths[0].Splits[i].AppendHeaders[netheader.OriginalHostKey], dm.Name; got != want { + t.Errorf("split %d original host = %q, want %q", i, got, want) + } + } + + paths[0].AppendHeaders["path-header"] = "changed" + paths[0].Splits[0].AppendHeaders["Knative-Serving-Revision"] = "changed" + if got, want := source[0].AppendHeaders["path-header"], "preserved"; got != want { + t.Errorf("source path header was mutated: got %q, want %q", got, want) + } + if got, want := source[0].Splits[0].AppendHeaders["Knative-Serving-Revision"], "app-00001"; got != want { + t.Errorf("source split header was mutated: got %q, want %q", got, want) + } +} diff --git a/pkg/reconciler/domainmapping/resources/names/names.go b/pkg/reconciler/domainmapping/resources/names/names.go new file mode 100644 index 000000000000..d7a6fe93e95f --- /dev/null +++ b/pkg/reconciler/domainmapping/resources/names/names.go @@ -0,0 +1,25 @@ +/* +Copyright 2026 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 + + http://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. +*/ + +package names + +import "knative.dev/pkg/kmeta" + +// Ingress returns the name for the Ingress +// child resource for the given DomainMapping. +func Ingress(dm kmeta.Accessor) string { + return kmeta.ChildName(dm.GetName(), "") +} diff --git a/pkg/reconciler/domainmapping/table_test.go b/pkg/reconciler/domainmapping/table_test.go index f782f2081fef..fed9c0543835 100644 --- a/pkg/reconciler/domainmapping/table_test.go +++ b/pkg/reconciler/domainmapping/table_test.go @@ -51,6 +51,9 @@ import ( domainmappingreconciler "knative.dev/serving/pkg/client/injection/reconciler/serving/v1beta1/domainmapping" "knative.dev/serving/pkg/reconciler/domainmapping/config" "knative.dev/serving/pkg/reconciler/domainmapping/resources" + routenames "knative.dev/serving/pkg/reconciler/route/resources/names" + servicenames "knative.dev/serving/pkg/reconciler/service/resources/names" + testingv1 "knative.dev/serving/pkg/testing/v1" "knative.dev/pkg/client/injection/ducks/duck/v1/addressable" . "knative.dev/pkg/reconciler/testing" @@ -65,6 +68,7 @@ const externalSchemeKey key = iota func TestReconcile(t *testing.T) { now := metav1.Now() + const longDomainMappingName = "this-is-a-very-long-domain-mapping-name.for-a-subdomain.example.com" table := TableTest{{ Name: "bad workqueue key", @@ -78,7 +82,7 @@ func TestReconcile(t *testing.T) { Name: "first reconcile", Key: "default/first-reconcile.com", Objects: []runtime.Object{ - ksvc("default", "target", "the-target-svc.default.svc.cluster.local", ""), + addressableMapping("default", "target", "the-target-svc.default.svc.cluster.local", ""), domainMapping("default", "first-reconcile.com", withRef("default", "target")), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ @@ -105,6 +109,195 @@ func TestReconcile(t *testing.T) { Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "first-reconcile.com"), Eventf(corev1.EventTypeNormal, "Created", "Created Ingress %q", "first-reconcile.com"), }, + }, { + Name: "Knative Service uses HTTP paths from its Route Ingress", + Key: "default/service.example.com", + Objects: func() []runtime.Object { + svc := knativeService("default", "website") + route := serviceRoute(svc) + return []runtime.Object{ + svc, + route, + routeIngress(route), + domainMapping("default", "service.example.com", withRef("default", "website", asKnativeServiceRef)), + } + }(), + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: domainMapping("default", "service.example.com", + withRef("default", "website", asKnativeServiceRef), + withURL("http", "service.example.com"), + withAddress("http", "service.example.com"), + withInitDomainMappingConditions, + withTLSNotEnabled, + withDomainClaimed, + withIngressNotConfigured, + withReferenceResolved, + ), + }}, + SkipNamespaceValidation: true, + WantCreates: []runtime.Object{ + resources.MakeDomainClaim(domainMapping("default", "service.example.com")), + resources.MakeIngressWithHTTPPaths( + domainMapping("default", "service.example.com", withRef("default", "website", asKnativeServiceRef)), + routeIngressHTTPPaths(), "the-ingress-class", netv1alpha1.HTTPOptionEnabled, nil), + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchAddFinalizerAction("default", "service.example.com"), + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "service.example.com"), + Eventf(corev1.EventTypeNormal, "Created", "Created Ingress %q", "service.example.com"), + }, + PostConditions: []func(*testing.T, *TableRow){ + AssertTrackingObject(servingv1.SchemeGroupVersion.WithKind("Route"), "default", "website"), + AssertTrackingObject(netv1alpha1.SchemeGroupVersion.WithKind("Ingress"), "default", "website"), + }, + }, { + Name: "Knative Route uses HTTP paths from its Ingress", + Key: "default/route.example.com", + Objects: func() []runtime.Object { + route := standaloneRoute("default", "website") + return []runtime.Object{ + route, + routeIngress(route), + domainMapping("default", "route.example.com", withRef("default", "website", asKnativeRouteRef)), + } + }(), + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: domainMapping("default", "route.example.com", + withRef("default", "website", asKnativeRouteRef), + withURL("http", "route.example.com"), + withAddress("http", "route.example.com"), + withInitDomainMappingConditions, + withTLSNotEnabled, + withDomainClaimed, + withIngressNotConfigured, + withReferenceResolved, + ), + }}, + SkipNamespaceValidation: true, + WantCreates: []runtime.Object{ + resources.MakeDomainClaim(domainMapping("default", "route.example.com")), + resources.MakeIngressWithHTTPPaths( + domainMapping("default", "route.example.com", withRef("default", "website", asKnativeRouteRef)), + routeIngressHTTPPaths(), "the-ingress-class", netv1alpha1.HTTPOptionEnabled, nil), + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchAddFinalizerAction("default", "route.example.com"), + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "route.example.com"), + Eventf(corev1.EventTypeNormal, "Created", "Created Ingress %q", "route.example.com"), + }, + }, { + Name: "Knative Service waits for Route Ingress", + Key: "default/waiting.example.com", + Objects: func() []runtime.Object { + svc := knativeService("default", "website") + dm := domainMapping("default", "waiting.example.com", withRef("default", "website", asKnativeServiceRef)) + return []runtime.Object{ + svc, + serviceRoute(svc), + resources.MakeIngress(dm, "old-revision", "website.default.svc.cluster.local", "the-ingress-class", netv1alpha1.HTTPOptionEnabled, nil), + dm, + } + }(), + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: domainMapping("default", "waiting.example.com", + withRef("default", "website", asKnativeServiceRef), + withURL("http", "waiting.example.com"), + withAddress("http", "waiting.example.com"), + withInitDomainMappingConditions, + withTLSNotEnabled, + withDomainClaimed, + withReferenceResolved, + withTargetIngressNotConfigured("Waiting for target Route default/website to configure Ingress default/website."), + ), + }}, + SkipNamespaceValidation: true, + WantCreates: []runtime.Object{ + resources.MakeDomainClaim(domainMapping("default", "waiting.example.com")), + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchAddFinalizerAction("default", "waiting.example.com"), + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "waiting.example.com"), + }, + PostConditions: []func(*testing.T, *TableRow){ + AssertTrackingObject(netv1alpha1.SchemeGroupVersion.WithKind("Ingress"), "default", "website"), + }, + }, { + Name: "Knative Route rejects an Ingress owned by another Route", + Key: "default/not-routable.example.com", + Objects: func() []runtime.Object { + route := standaloneRoute("default", "website") + targetIngress := routeIngress(route) + targetIngress.OwnerReferences = []metav1.OwnerReference{*kmeta.NewControllerRef(standaloneRoute("default", "another-route"))} + dm := domainMapping("default", "not-routable.example.com", withRef("default", "website", asKnativeRouteRef)) + return []runtime.Object{ + route, + targetIngress, + resources.MakeIngress(dm, "old-revision", "website.default.svc.cluster.local", "the-ingress-class", netv1alpha1.HTTPOptionEnabled, nil), + dm, + } + }(), + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: domainMapping("default", "not-routable.example.com", + withRef("default", "website", asKnativeRouteRef), + withURL("http", "not-routable.example.com"), + withAddress("http", "not-routable.example.com"), + withInitDomainMappingConditions, + withTLSNotEnabled, + withDomainClaimed, + withReferenceResolved, + withTargetNotOwned("Route default/website does not own Ingress default/website."), + ), + }}, + SkipNamespaceValidation: true, + WantCreates: []runtime.Object{ + resources.MakeDomainClaim(domainMapping("default", "not-routable.example.com")), + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchAddFinalizerAction("default", "not-routable.example.com"), + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "not-routable.example.com"), + }, + }, { + Name: "Knative Service resolution failure preserves an existing Ingress with a long mapping name", + Key: "default/" + longDomainMappingName, + WantErr: true, + Objects: func() []runtime.Object { + dm := domainMapping("default", longDomainMappingName, withRef("default", "missing", asKnativeServiceRef)) + return []runtime.Object{ + resources.MakeIngress(dm, "old-revision", "missing.default.svc.cluster.local", "the-ingress-class", netv1alpha1.HTTPOptionEnabled, nil), + dm, + } + }(), + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: domainMapping("default", longDomainMappingName, + withRef("default", "missing", asKnativeServiceRef), + withURL("http", longDomainMappingName), + withAddress("http", longDomainMappingName), + withInitDomainMappingConditions, + withTLSNotEnabled, + withDomainClaimed, + withReferenceNotResolved(`failed to get object default/missing: services.serving.knative.dev "missing" not found`), + withIngressNotConfigured, + ), + }}, + SkipNamespaceValidation: true, + WantCreates: []runtime.Object{ + resources.MakeDomainClaim(domainMapping("default", longDomainMappingName)), + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchAddFinalizerAction("default", longDomainMappingName), + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", longDomainMappingName), + Eventf(corev1.EventTypeWarning, "InternalError", `resolving reference: failed to get object default/missing: services.serving.knative.dev "missing" not found`), + }, }, { Name: "finalize cleans up claim", Key: "default/cleanup.on.aisle-three", @@ -220,7 +413,7 @@ func TestReconcile(t *testing.T) { withInitDomainMappingConditions, withTLSNotEnabled, withDomainClaimed, - withReferenceNotResolved(`failed to get object default/target: services.serving.knative.dev "target" not found`), + withReferenceNotResolved(`failed to get object default/target: domainmappings.serving.knative.dev "target" not found`), ), }}, SkipNamespaceValidation: true, // allow creation of ClusterDomainClaim. @@ -232,13 +425,13 @@ func TestReconcile(t *testing.T) { }, WantEvents: []string{ Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "first-reconcile.com"), - Eventf(corev1.EventTypeWarning, "InternalError", `resolving reference: failed to get object default/target: services.serving.knative.dev "target" not found`), + Eventf(corev1.EventTypeWarning, "InternalError", `resolving reference: failed to get object default/target: domainmappings.serving.knative.dev "target" not found`), }, }, { Name: "first reconcile, ref has a path", Key: "default/first-reconcile.com", Objects: []runtime.Object{ - ksvc("default", "target", "the-target-svc.svc.cluster.local", "path"), + addressableMapping("default", "target", "the-target-svc.svc.cluster.local", "path"), domainMapping("default", "first-reconcile.com", withRef("default", "target")), }, WantErr: true, @@ -268,7 +461,7 @@ func TestReconcile(t *testing.T) { Name: "first reconcile, ref doesn't end in cluster suffix", Key: "default/first-reconcile.com", Objects: []runtime.Object{ - ksvc("default", "target", "notasvc.cluster.local", ""), + addressableMapping("default", "target", "notasvc.cluster.local", ""), domainMapping("default", "first-reconcile.com", withRef("default", "target")), }, WantErr: true, @@ -298,7 +491,7 @@ func TestReconcile(t *testing.T) { Name: "first reconcile, resolved URL in wrong namespace", Key: "default/first-reconcile.com", Objects: []runtime.Object{ - ksvc("default", "target", "name.anothernamespace.svc.cluster.local", ""), + addressableMapping("default", "target", "name.anothernamespace.svc.cluster.local", ""), domainMapping("default", "first-reconcile.com", withRef("default", "target")), }, WantErr: true, @@ -328,7 +521,7 @@ func TestReconcile(t *testing.T) { Name: "first reconcile, pre-owned domain claim", Key: "default/first-reconcile.com", Objects: []runtime.Object{ - ksvc("default", "target", "the-target-svc.default.svc.cluster.local", ""), + addressableMapping("default", "target", "the-target-svc.default.svc.cluster.local", ""), domainMapping("default", "first-reconcile.com", withRef("default", "target")), resources.MakeDomainClaim(domainMapping("default", "first-reconcile.com", withRef("default", "target"))), }, @@ -416,7 +609,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile with ingressClass annotation", Key: "default/ingressclass.first-reconcile.com", Objects: []runtime.Object{ - ksvc("default", "target", "the-target-svc.default.svc.cluster.local", ""), + addressableMapping("default", "target", "the-target-svc.default.svc.cluster.local", ""), domainMapping("default", "ingressclass.first-reconcile.com", withRef("default", "target"), withAnnotations(map[string]string{ netapi.IngressClassAnnotationKey: "overridden-ingress-class", @@ -455,7 +648,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile with new label", Key: "default/ingressclass.first-reconcile.com", Objects: []runtime.Object{ - ksvc("default", "target", "the-target-svc.default.svc.cluster.local", ""), + addressableMapping("default", "target", "the-target-svc.default.svc.cluster.local", ""), domainMapping("default", "ingressclass.first-reconcile.com", withRef("default", "target"), withLabels(map[string]string{ netapi.IngressLabelKey: "new-label", @@ -494,7 +687,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile changed ref", Key: "default/ingress-exists.org", Objects: []runtime.Object{ - ksvc("default", "changed", "changed.default.svc.cluster.local", ""), + addressableMapping("default", "changed", "changed.default.svc.cluster.local", ""), domainMapping("default", "ingress-exists.org", withRef("default", "changed")), resources.MakeIngress(domainMapping("default", "ingress-exists.org", withRef("default", "changed")), "previous", "previous.default.svc.cluster.local", "the-ingress-class", netv1alpha1.HTTPOptionEnabled, nil /* tls */), resources.MakeDomainClaim(domainMapping("default", "ingress-exists.org", withRef("default", "changed"))), @@ -524,7 +717,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile failed ingress", Key: "default/ingress-failed.me", Objects: []runtime.Object{ - ksvc("default", "failed", "failed.default.svc.cluster.local", ""), + addressableMapping("default", "failed", "failed.default.svc.cluster.local", ""), domainMapping("default", "ingress-failed.me", withRef("default", "failed"), withURL("http", "ingress-failed.me"), @@ -557,7 +750,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile unknown ingress", Key: "default/ingress-unknown.me", Objects: []runtime.Object{ - ksvc("default", "unknown", "unknown.default.svc.cluster.local", ""), + addressableMapping("default", "unknown", "unknown.default.svc.cluster.local", ""), domainMapping("default", "ingress-unknown.me", withRef("default", "unknown"), withRef("default", "unknown"), withURL("http", "ingress-unknown.me"), @@ -590,7 +783,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile ready ingress", Key: "default/ingress-ready.me", Objects: []runtime.Object{ - ksvc("default", "ready", "ready.default.svc.cluster.local", ""), + addressableMapping("default", "ready", "ready.default.svc.cluster.local", ""), domainMapping("default", "ingress-ready.me", withRef("default", "ready"), withURL("http", "ingress-ready.me"), @@ -626,7 +819,7 @@ func TestReconcile(t *testing.T) { InduceFailure("create", "ingresses"), }, Objects: []runtime.Object{ - ksvc("default", "cantcreate", "cantcreate.default.svc.cluster.local", ""), + addressableMapping("default", "cantcreate", "cantcreate.default.svc.cluster.local", ""), domainMapping("default", "cantcreate.this", withRef("default", "cantcreate"), withURL("http", "cantcreate.this"), @@ -673,7 +866,7 @@ func TestReconcile(t *testing.T) { InduceFailure("update", "ingresses"), }, Objects: []runtime.Object{ - ksvc("default", "cantupdate", "cantupdate.default.svc.cluster.local", ""), + addressableMapping("default", "cantupdate", "cantupdate.default.svc.cluster.local", ""), domainMapping("default", "cantupdate.this", withRef("default", "cantupdate"), withURL("http", "cantupdate.this"), @@ -718,7 +911,7 @@ func TestReconcile(t *testing.T) { Key: "default/ingress-ready.me", Ctx: context.WithValue(context.Background(), externalSchemeKey, "ws"), Objects: []runtime.Object{ - ksvc("default", "ready", "ready.default.svc.cluster.local", ""), + addressableMapping("default", "ready", "ready.default.svc.cluster.local", ""), domainMapping("default", "ingress-ready.me", withRef("default", "ready"), withURL("ws", "ingress-ready.me"), @@ -742,15 +935,11 @@ func TestReconcile(t *testing.T) { }, }} + table = append(table, routeIngressEdgeCaseRows()...) + table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { ctx = addressable.WithDuck(ctx) - r := &Reconciler{ - certificateLister: listers.GetCertificateLister(), - ingressLister: listers.GetIngressLister(), - netclient: networkingclient.Get(ctx), - resolver: resolver.NewURIResolverFromTracker(ctx, tracker.New(func(types.NamespacedName) {}, 0)), - domainClaimLister: listers.GetDomainClaimLister(), - } + r := makeTestReconciler(ctx, listers) cfg := &config.Config{ Network: &netcfg.Config{ @@ -780,7 +969,7 @@ func TestReconcileAutocreateClaimsDisabled(t *testing.T) { Name: "first reconcile, no existing claim", Key: "default/first-reconcile.com", Objects: []runtime.Object{ - ksvc("default", "target", "the-target-svc.default.svc.cluster.local", ""), + addressableMapping("default", "target", "the-target-svc.default.svc.cluster.local", ""), domainMapping("default", "first-reconcile.com", withRef("default", "target")), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ @@ -805,7 +994,7 @@ func TestReconcileAutocreateClaimsDisabled(t *testing.T) { Name: "first reconcile, claim exists and is owned", Key: "default/first-reconcile.com", Objects: []runtime.Object{ - ksvc("default", "target", "the-target-svc.default.svc.cluster.local", ""), + addressableMapping("default", "target", "the-target-svc.default.svc.cluster.local", ""), domainMapping("default", "first-reconcile.com", withRef("default", "target")), resources.MakeDomainClaim(domainMapping("default", "first-reconcile.com", withRef("default", "target"))), }, @@ -877,13 +1066,7 @@ func TestReconcileAutocreateClaimsDisabled(t *testing.T) { table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { ctx = addressable.WithDuck(ctx) - r := &Reconciler{ - certificateLister: listers.GetCertificateLister(), - ingressLister: listers.GetIngressLister(), - netclient: networkingclient.Get(ctx), - resolver: resolver.NewURIResolverFromTracker(ctx, tracker.New(func(types.NamespacedName) {}, 0)), - domainClaimLister: listers.GetDomainClaimLister(), - } + r := makeTestReconciler(ctx, listers) return domainmappingreconciler.NewReconciler(ctx, logging.FromContext(ctx), servingclient.Get(ctx), listers.GetDomainMappingLister(), controller.GetEventRecorder(ctx), r, @@ -906,7 +1089,7 @@ func TestReconcileTLSEnabled(t *testing.T) { Name: "first reconcile", Key: "default/first.reconcile.io", Objects: []runtime.Object{ - ksvc("default", "ready", "ready.default.svc.cluster.local", ""), + addressableMapping("default", "ready", "ready.default.svc.cluster.local", ""), domainMapping("default", "first.reconcile.io", withRef("default", "ready"), withURL("http", "first.reconcile.io"), @@ -946,7 +1129,7 @@ func TestReconcileTLSEnabled(t *testing.T) { Name: "becomes ready", Key: "default/becomes.ready.run", Objects: []runtime.Object{ - ksvc("default", "ready", "ready.default.svc.cluster.local", ""), + addressableMapping("default", "ready", "ready.default.svc.cluster.local", ""), domainMapping("default", "becomes.ready.run", withRef("default", "ready"), withURL("http", "becomes.ready.run"), @@ -1011,7 +1194,7 @@ func TestReconcileTLSEnabled(t *testing.T) { WantErr: true, Key: "default/cert.not.owned.ru", Objects: []runtime.Object{ - ksvc("default", "ready", "ready.default.svc.cluster.local", ""), + addressableMapping("default", "ready", "ready.default.svc.cluster.local", ""), domainMapping("default", "cert.not.owned.ru", withRef("default", "ready"), withURL("http", "cert.not.owned.ru"), @@ -1061,7 +1244,7 @@ func TestReconcileTLSEnabled(t *testing.T) { ), "the-cert-class"), }, Objects: []runtime.Object{ - ksvc("default", "ready", "ready.default.svc.cluster.local", ""), + addressableMapping("default", "ready", "ready.default.svc.cluster.local", ""), domainMapping("default", "cert.creation.failed.ly", withRef("default", "ready"), withURL("http", "cert.creation.failed.ly"), @@ -1092,7 +1275,7 @@ func TestReconcileTLSEnabled(t *testing.T) { Name: "with challenges", Key: "default/challenged.com", Objects: []runtime.Object{ - ksvc("default", "ready", "ready.default.svc.cluster.local", ""), + addressableMapping("default", "ready", "ready.default.svc.cluster.local", ""), domainMapping("default", "challenged.com", withRef("default", "ready"), withURL("http", "challenged.com"), @@ -1195,7 +1378,7 @@ func TestReconcileTLSEnabled(t *testing.T) { Name: "TLS secret provided", Key: "default/certificateless.com", Objects: []runtime.Object{ - ksvc("default", "ready", "ready.default.svc.cluster.local", ""), + addressableMapping("default", "ready", "ready.default.svc.cluster.local", ""), domainMapping("default", "certificateless.com", withTLSSecret("tls-secret"), withRef("default", "ready"), @@ -1238,13 +1421,7 @@ func TestReconcileTLSEnabled(t *testing.T) { table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { ctx = addressable.WithDuck(ctx) - r := &Reconciler{ - certificateLister: listers.GetCertificateLister(), - ingressLister: listers.GetIngressLister(), - domainClaimLister: listers.GetDomainClaimLister(), - netclient: networkingclient.Get(ctx), - resolver: resolver.NewURIResolverFromTracker(ctx, tracker.New(func(types.NamespacedName) {}, 0)), - } + r := makeTestReconciler(ctx, listers) return domainmappingreconciler.NewReconciler(ctx, logging.FromContext(ctx), servingclient.Get(ctx), listers.GetDomainMappingLister(), controller.GetEventRecorder(ctx), r, @@ -1268,7 +1445,7 @@ func TestReconcileTLSEnabledButDowngraded(t *testing.T) { Name: "ingress ready, cert not ready, downgraded to HTTP", Key: "default/http.downgraded.com", Objects: []runtime.Object{ - ksvc("default", "ready", "ready.default.svc.cluster.local", ""), + addressableMapping("default", "ready", "ready.default.svc.cluster.local", ""), domainMapping("default", "http.downgraded.com", withRef("default", "ready"), withURL("http", "http.downgraded.com"), @@ -1305,13 +1482,7 @@ func TestReconcileTLSEnabledButDowngraded(t *testing.T) { table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { ctx = addressable.WithDuck(ctx) - r := &Reconciler{ - certificateLister: listers.GetCertificateLister(), - domainClaimLister: listers.GetDomainClaimLister(), - ingressLister: listers.GetIngressLister(), - netclient: networkingclient.Get(ctx), - resolver: resolver.NewURIResolverFromTracker(ctx, tracker.New(func(types.NamespacedName) {}, 0)), - } + r := makeTestReconciler(ctx, listers) return domainmappingreconciler.NewReconciler(ctx, logging.FromContext(ctx), servingclient.Get(ctx), listers.GetDomainMappingLister(), controller.GetEventRecorder(ctx), r, @@ -1369,8 +1540,8 @@ func withRef(namespace, name string, opt ...refOption) domainMappingOption { return func(dm *v1beta1.DomainMapping) { dm.Spec.Ref.Namespace = namespace dm.Spec.Ref.Name = name - dm.Spec.Ref.APIVersion = "serving.knative.dev/v1" - dm.Spec.Ref.Kind = "Service" + dm.Spec.Ref.APIVersion = v1beta1.SchemeGroupVersion.String() + dm.Spec.Ref.Kind = "DomainMapping" for _, o := range opt { o(&dm.Spec.Ref) @@ -1385,6 +1556,11 @@ func withAPIVersionKind(apiVersion, kind string) refOption { } } +var ( + asKnativeServiceRef = withAPIVersionKind(servingv1.SchemeGroupVersion.String(), "Service") + asKnativeRouteRef = withAPIVersionKind(servingv1.SchemeGroupVersion.String(), "Route") +) + func withURL(scheme, host string) domainMappingOption { return func(dm *v1beta1.DomainMapping) { dm.Status.URL = &apis.URL{Scheme: scheme, Host: host} @@ -1404,6 +1580,18 @@ func withIngressNotConfigured(dm *v1beta1.DomainMapping) { dm.Status.MarkIngressNotConfigured() } +func withTargetIngressNotConfigured(message string) domainMappingOption { + return func(dm *v1beta1.DomainMapping) { + dm.Status.MarkTargetIngressNotConfigured(message) + } +} + +func withTargetNotOwned(message string) domainMappingOption { + return func(dm *v1beta1.DomainMapping) { + dm.Status.MarkTargetNotOwned(message) + } +} + func withPropagatedStatus(status netv1alpha1.IngressStatus) domainMappingOption { return func(r *v1beta1.DomainMapping) { r.Status.PropagateIngressStatus(status) @@ -1523,26 +1711,242 @@ func withIngressHTTPOption(httpOpt netv1alpha1.HTTPOption) IngressOption { } } -func ksvc(ns, name, host, path string) *servingv1.Service { - return &servingv1.Service{ +func addressableMapping(ns, name, host, path string) *v1beta1.DomainMapping { + return &v1beta1.DomainMapping{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: ns, }, - Status: servingv1.ServiceStatus{ - RouteStatusFields: servingv1.RouteStatusFields{ - Address: &duckv1.Addressable{ - URL: &apis.URL{ - Scheme: "http", - Host: host, - Path: path, - }, - }, - }, + Status: v1beta1.DomainMappingStatus{ + URL: &apis.URL{Scheme: "http", Host: host, Path: path}, + Address: &duckv1.Addressable{URL: &apis.URL{Scheme: "http", Host: host, Path: path}}, }, } } +func knativeService(ns, name string) *servingv1.Service { + return testingv1.Service(name, ns, testingv1.WithSvcStatusAddress) +} + +func serviceRoute(service *servingv1.Service) *servingv1.Route { + route := standaloneRoute(service.Namespace, servicenames.Route(service)) + route.OwnerReferences = []metav1.OwnerReference{*kmeta.NewControllerRef(service)} + return route +} + +func standaloneRoute(ns, name string) *servingv1.Route { + return testingv1.Route(ns, name, testingv1.WithRouteUID(types.UID(name+"-route-uid")), testingv1.WithAddress) +} + +func routeIngress(route *servingv1.Route) *netv1alpha1.Ingress { + return &netv1alpha1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: routenames.Ingress(route), + Namespace: route.Namespace, + OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(route)}, + }, + Spec: netv1alpha1.IngressSpec{ + Rules: []netv1alpha1.IngressRule{{ + Hosts: []string{routenames.K8sServiceFullname(route)}, + Visibility: netv1alpha1.IngressVisibilityClusterLocal, + HTTP: &netv1alpha1.HTTPIngressRuleValue{Paths: routeIngressHTTPPaths()}, + }}, + }, + } +} + +func routeIngressHTTPPaths() []netv1alpha1.HTTPIngressPath { + return []netv1alpha1.HTTPIngressPath{{ + RewriteHost: "source-host.example", + AppendHeaders: map[string]string{ + "Knative-Serving-Default-Route": "true", + }, + Splits: []netv1alpha1.IngressBackendSplit{{ + IngressBackend: netv1alpha1.IngressBackend{ + ServiceNamespace: "default", + ServiceName: "website-00001", + ServicePort: intstr.FromInt(80), + }, + Percent: 80, + AppendHeaders: map[string]string{ + "Knative-Serving-Namespace": "default", + "Knative-Serving-Revision": "website-00001", + }, + }, { + IngressBackend: netv1alpha1.IngressBackend{ + ServiceNamespace: "default", + ServiceName: "website-00002", + ServicePort: intstr.FromInt(80), + }, + Percent: 20, + AppendHeaders: map[string]string{ + "Knative-Serving-Namespace": "default", + "Knative-Serving-Revision": "website-00002", + }, + }}, + }} +} + +func routeIngressEdgeCaseRows() TableTest { + const targetHost = "website.default.svc.cluster.local" + + service := knativeService("default", "website") + service.UID = types.UID("website-service-uid") + route := serviceRoute(service) + anotherService := knativeService("default", "another-service") + anotherService.UID = types.UID("another-service-uid") + route.OwnerReferences = []metav1.OwnerReference{*kmeta.NewControllerRef(anotherService)} + dm := domainMapping("default", "route-not-owned.example.com", withRef("default", "website", asKnativeServiceRef)) + rows := TableTest{{ + Name: "Knative Service rejects a Route owned by another Service", + Key: "default/route-not-owned.example.com", + Objects: []runtime.Object{ + service, + route, + resources.MakeIngress(dm, "old-revision", targetHost, "the-ingress-class", netv1alpha1.HTTPOptionEnabled, nil), + dm, + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: domainMapping("default", "route-not-owned.example.com", + withRef("default", "website", asKnativeServiceRef), + withURL("http", "route-not-owned.example.com"), + withAddress("http", "route-not-owned.example.com"), + withInitDomainMappingConditions, + withTLSNotEnabled, + withDomainClaimed, + withReferenceResolved, + withTargetNotOwned("Service default/website does not own Route default/website."), + ), + }}, + SkipNamespaceValidation: true, + WantCreates: []runtime.Object{ + resources.MakeDomainClaim(domainMapping("default", "route-not-owned.example.com")), + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchAddFinalizerAction("default", "route-not-owned.example.com"), + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "route-not-owned.example.com"), + }, + }, { + Name: "Knative Route uses the first matching cluster-local rule", + Key: "default/multiple-matching-rules.example.com", + Objects: func() []runtime.Object { + route := standaloneRoute("default", "website") + targetIngress := routeIngress(route) + second := targetIngress.Spec.Rules[0].DeepCopy() + second.HTTP.Paths[0].Splits[0].ServiceName = "unexpected-revision" + targetIngress.Spec.Rules = append(targetIngress.Spec.Rules, *second) + return []runtime.Object{ + route, + targetIngress, + domainMapping("default", "multiple-matching-rules.example.com", withRef("default", "website", asKnativeRouteRef)), + } + }(), + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: domainMapping("default", "multiple-matching-rules.example.com", + withRef("default", "website", asKnativeRouteRef), + withURL("http", "multiple-matching-rules.example.com"), + withAddress("http", "multiple-matching-rules.example.com"), + withInitDomainMappingConditions, + withTLSNotEnabled, + withDomainClaimed, + withIngressNotConfigured, + withReferenceResolved, + ), + }}, + SkipNamespaceValidation: true, + WantCreates: []runtime.Object{ + resources.MakeDomainClaim(domainMapping("default", "multiple-matching-rules.example.com")), + resources.MakeIngressWithHTTPPaths( + domainMapping("default", "multiple-matching-rules.example.com", withRef("default", "website", asKnativeRouteRef)), + routeIngressHTTPPaths(), "the-ingress-class", netv1alpha1.HTTPOptionEnabled, nil), + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchAddFinalizerAction("default", "multiple-matching-rules.example.com"), + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "multiple-matching-rules.example.com"), + Eventf(corev1.EventTypeNormal, "Created", "Created Ingress %q", "multiple-matching-rules.example.com"), + }, + }, { + Name: "Knative Route reports a matching cluster-local rule without HTTP", + Key: "default/missing-http.example.com", + WantErr: true, + Objects: func() []runtime.Object { + route := standaloneRoute("default", "website") + targetIngress := routeIngress(route) + targetIngress.Spec.Rules[0].HTTP = nil + return []runtime.Object{ + route, + targetIngress, + domainMapping("default", "missing-http.example.com", withRef("default", "website", asKnativeRouteRef)), + } + }(), + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: domainMapping("default", "missing-http.example.com", + withRef("default", "website", asKnativeRouteRef), + withURL("http", "missing-http.example.com"), + withAddress("http", "missing-http.example.com"), + withInitDomainMappingConditions, + withTLSNotEnabled, + withDomainClaimed, + withReferenceResolved, + ), + }}, + SkipNamespaceValidation: true, + WantCreates: []runtime.Object{ + resources.MakeDomainClaim(domainMapping("default", "missing-http.example.com")), + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchAddFinalizerAction("default", "missing-http.example.com"), + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "missing-http.example.com"), + Eventf(corev1.EventTypeWarning, "InternalError", "target Ingress default/website has a matching cluster-local rule without HTTP"), + }, + }, { + Name: "target Ingress has no matching cluster-local rule", + Key: "default/no-matching-rule.example.com", + Objects: func() []runtime.Object { + route := standaloneRoute("default", "website") + targetIngress := routeIngress(route) + targetIngress.Spec.Rules[0].Hosts = []string{"another.default.svc.cluster.local"} + dm := domainMapping("default", "no-matching-rule.example.com", withRef("default", "website", asKnativeRouteRef)) + return []runtime.Object{ + route, + targetIngress, + resources.MakeIngress(dm, "old-revision", targetHost, "the-ingress-class", netv1alpha1.HTTPOptionEnabled, nil), + dm, + } + }(), + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: domainMapping("default", "no-matching-rule.example.com", + withRef("default", "website", asKnativeRouteRef), + withURL("http", "no-matching-rule.example.com"), + withAddress("http", "no-matching-rule.example.com"), + withInitDomainMappingConditions, + withTLSNotEnabled, + withDomainClaimed, + withReferenceResolved, + withTargetIngressNotConfigured(`Ingress default/website has no cluster-local rule for host "website.default.svc.cluster.local".`), + ), + }}, + SkipNamespaceValidation: true, + WantCreates: []runtime.Object{ + resources.MakeDomainClaim(domainMapping("default", "no-matching-rule.example.com")), + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchAddFinalizerAction("default", "no-matching-rule.example.com"), + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "no-matching-rule.example.com"), + }, + }} + + return rows +} + func readyCertStatus() netv1alpha1.CertificateStatus { certStatus := &netv1alpha1.CertificateStatus{} certStatus.MarkReady() @@ -1585,6 +1989,20 @@ func patchRemoveFinalizerAction(namespace, name string) clientgotesting.PatchAct } } +func makeTestReconciler(ctx context.Context, listers *Listers) *Reconciler { + testTracker := ctx.Value(TrackerKey).(tracker.Interface) + return &Reconciler{ + certificateLister: listers.GetCertificateLister(), + ingressLister: listers.GetIngressLister(), + serviceLister: listers.GetServiceLister(), + routeLister: listers.GetRouteLister(), + domainClaimLister: listers.GetDomainClaimLister(), + netclient: networkingclient.Get(ctx), + resolver: resolver.NewURIResolverFromTracker(ctx, testTracker), + tracker: testTracker, + } +} + type testConfigStore struct { config *config.Config }