From ef01947f189674ff5df260b7a146bcc4dd3b8e8a Mon Sep 17 00:00:00 2001 From: Trey Hyde Date: Fri, 18 Jul 2025 10:45:09 -0700 Subject: [PATCH 1/7] first stab at creating a delegate VS for ksvcs so we can skip a hop or two. --- .gitignore | 3 ++ .../ingress/resources/names/names.go | 7 +++++ .../ingress/resources/virtual_service.go | 28 +++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/.gitignore b/.gitignore index 85baa82ae0..afb90a10b6 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,6 @@ # Temporary output of build tools bazel-* *.out + +# binaries +bin/ \ No newline at end of file diff --git a/pkg/reconciler/ingress/resources/names/names.go b/pkg/reconciler/ingress/resources/names/names.go index 8c584ad7b0..6cdc6bcecc 100644 --- a/pkg/reconciler/ingress/resources/names/names.go +++ b/pkg/reconciler/ingress/resources/names/names.go @@ -33,3 +33,10 @@ func IngressVirtualService(i kmeta.Accessor) string { func MeshVirtualService(i kmeta.Accessor) string { return kmeta.ChildName(i.GetName(), "-mesh") } + +// DelegateVirtualService returns the name of the VirtualService child +// resource for given Ingress that programs traffic for +// delgate the route for other VirtualServices. +func DelegateVirtualService(i kmeta.Accessor) string { + return kmeta.ChildName(i.GetName(), "-delegate") +} diff --git a/pkg/reconciler/ingress/resources/virtual_service.go b/pkg/reconciler/ingress/resources/virtual_service.go index 7636535316..10749d4bdf 100644 --- a/pkg/reconciler/ingress/resources/virtual_service.go +++ b/pkg/reconciler/ingress/resources/virtual_service.go @@ -95,6 +95,29 @@ func MakeMeshVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.Ingress return vs } +// MakeDelegateVirtualService creates a mesh Virtual Service +func MakeDelegateVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string]) *v1beta1.VirtualService { + emptyHosts := sets.New[string]() + vs := &v1beta1.VirtualService{ + ObjectMeta: metav1.ObjectMeta{ + Name: names.DelegateVirtualService(ing), + Namespace: VirtualServiceNamespace(ing), + OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(ing)}, + Annotations: ing.GetAnnotations(), + }, + Spec: *makeVirtualServiceSpec(ing, map[v1alpha1.IngressVisibility]sets.Set[string]{ + v1alpha1.IngressVisibilityExternalIP: sets.New("mesh"), + v1alpha1.IngressVisibilityClusterLocal: sets.New("mesh"), + }, emptyHosts), + } + // Populate the Ingress labels. + vs.Labels = kmeta.FilterMap(ing.GetLabels(), func(k string) bool { + return k != RouteLabelKey && k != RouteNamespaceLabelKey + }) + vs.Labels[networking.IngressLabelKey] = ing.Name + return vs +} + // MakeVirtualServices creates a mesh VirtualService and a virtual service for each gateway func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string]) ([]*v1beta1.VirtualService, error) { // Insert probe header @@ -106,6 +129,11 @@ func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVis if meshVs := MakeMeshVirtualService(ing, gateways); meshVs != nil { vss = append(vss, meshVs) } + + if meshVs := MakeDelegateVirtualService(ing, gateways); meshVs != nil { + vss = append(vss, meshVs) + } + requiredGatewayCount := 0 if len(getPublicIngressRules(ing)) > 0 { requiredGatewayCount += gateways[v1alpha1.IngressVisibilityExternalIP].Len() From 8179adc3f4d236b5990b1b7c5f6a4303be31e24d Mon Sep 17 00:00:00 2001 From: Trey Hyde Date: Fri, 18 Jul 2025 11:02:00 -0700 Subject: [PATCH 2/7] Apply suggestion from @gemini-code-assist[bot] Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- pkg/reconciler/ingress/resources/virtual_service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/ingress/resources/virtual_service.go b/pkg/reconciler/ingress/resources/virtual_service.go index 10749d4bdf..36ebbf6bc8 100644 --- a/pkg/reconciler/ingress/resources/virtual_service.go +++ b/pkg/reconciler/ingress/resources/virtual_service.go @@ -130,8 +130,8 @@ func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVis vss = append(vss, meshVs) } - if meshVs := MakeDelegateVirtualService(ing, gateways); meshVs != nil { - vss = append(vss, meshVs) +if delegateVs := MakeDelegateVirtualService(ing, gateways); delegateVs != nil { + vss = append(vss, delegateVs) } requiredGatewayCount := 0 From b542319ca3c1883179915d02ba55a5bf87b07cb0 Mon Sep 17 00:00:00 2001 From: Trey Hyde Date: Fri, 18 Jul 2025 11:02:19 -0700 Subject: [PATCH 3/7] Apply suggestion from @gemini-code-assist[bot] Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- pkg/reconciler/ingress/resources/names/names.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/ingress/resources/names/names.go b/pkg/reconciler/ingress/resources/names/names.go index 6cdc6bcecc..8c9fbd55a9 100644 --- a/pkg/reconciler/ingress/resources/names/names.go +++ b/pkg/reconciler/ingress/resources/names/names.go @@ -36,7 +36,7 @@ func MeshVirtualService(i kmeta.Accessor) string { // DelegateVirtualService returns the name of the VirtualService child // resource for given Ingress that programs traffic for -// delgate the route for other VirtualServices. +// delegate the route for other VirtualServices. func DelegateVirtualService(i kmeta.Accessor) string { return kmeta.ChildName(i.GetName(), "-delegate") } From d4d9c5411e398b5a14e08b04015fcd68fa48bda4 Mon Sep 17 00:00:00 2001 From: Trey Hyde Date: Fri, 18 Jul 2025 11:05:40 -0700 Subject: [PATCH 4/7] address some warnings --- pkg/reconciler/ingress/resources/virtual_service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/ingress/resources/virtual_service.go b/pkg/reconciler/ingress/resources/virtual_service.go index 36ebbf6bc8..f2bd9c1466 100644 --- a/pkg/reconciler/ingress/resources/virtual_service.go +++ b/pkg/reconciler/ingress/resources/virtual_service.go @@ -96,7 +96,7 @@ func MakeMeshVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.Ingress } // MakeDelegateVirtualService creates a mesh Virtual Service -func MakeDelegateVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string]) *v1beta1.VirtualService { +func MakeDelegateVirtualService(ing *v1alpha1.Ingress) *v1beta1.VirtualService { emptyHosts := sets.New[string]() vs := &v1beta1.VirtualService{ ObjectMeta: metav1.ObjectMeta{ @@ -130,7 +130,7 @@ func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVis vss = append(vss, meshVs) } -if delegateVs := MakeDelegateVirtualService(ing, gateways); delegateVs != nil { + if delegateVs := MakeDelegateVirtualService(ing); delegateVs != nil { vss = append(vss, delegateVs) } From 5217145a754eff31bd83e9d47526d4313838e698 Mon Sep 17 00:00:00 2001 From: Trey Hyde Date: Mon, 21 Jul 2025 09:28:09 -0700 Subject: [PATCH 5/7] make sure host AND gateways are cleared for delegate VS --- pkg/reconciler/ingress/ingress.go | 1 + .../ingress/resources/virtual_service.go | 52 +++++++++++++------ 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index 86c2cdf85a..ed027152a2 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -339,6 +339,7 @@ func (r *Reconciler) reconcileSystemGeneratedGateway(ctx context.Context, desire func (r *Reconciler) reconcileVirtualServices(ctx context.Context, ing *v1alpha1.Ingress, desired []*v1beta1.VirtualService, ) error { + // First, create all needed VirtualServices. kept := sets.New[string]() for _, d := range desired { diff --git a/pkg/reconciler/ingress/resources/virtual_service.go b/pkg/reconciler/ingress/resources/virtual_service.go index f2bd9c1466..157b22c38e 100644 --- a/pkg/reconciler/ingress/resources/virtual_service.go +++ b/pkg/reconciler/ingress/resources/virtual_service.go @@ -5,7 +5,7 @@ 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 + 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, @@ -20,6 +20,8 @@ import ( "fmt" "strings" + "go.uber.org/zap" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -95,26 +97,30 @@ func MakeMeshVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.Ingress return vs } -// MakeDelegateVirtualService creates a mesh Virtual Service +// MakeDelegateVirtualService creates a delegate Virtual Service func MakeDelegateVirtualService(ing *v1alpha1.Ingress) *v1beta1.VirtualService { - emptyHosts := sets.New[string]() + // Delegate VirtualServices should have empty hosts + hosts := sets.New[string]() + + // Delegate VirtualServices should have no gateways specified + ownerRef := kmeta.NewControllerRef(ing) + // Pass an empty gateways map to makeVirtualServiceSpec + emptyGateways := map[v1alpha1.IngressVisibility]sets.Set[string]{} vs := &v1beta1.VirtualService{ ObjectMeta: metav1.ObjectMeta{ Name: names.DelegateVirtualService(ing), Namespace: VirtualServiceNamespace(ing), - OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(ing)}, + OwnerReferences: []metav1.OwnerReference{*ownerRef}, Annotations: ing.GetAnnotations(), }, - Spec: *makeVirtualServiceSpec(ing, map[v1alpha1.IngressVisibility]sets.Set[string]{ - v1alpha1.IngressVisibilityExternalIP: sets.New("mesh"), - v1alpha1.IngressVisibilityClusterLocal: sets.New("mesh"), - }, emptyHosts), + Spec: *makeVirtualServiceSpec(ing, emptyGateways, hosts), // Pass empty hosts and gateways for delegate } // Populate the Ingress labels. vs.Labels = kmeta.FilterMap(ing.GetLabels(), func(k string) bool { return k != RouteLabelKey && k != RouteNamespaceLabelKey }) vs.Labels[networking.IngressLabelKey] = ing.Name + return vs } @@ -151,6 +157,8 @@ func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVis } func makeVirtualServiceSpec(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string], hosts sets.Set[string]) *istiov1beta1.VirtualService { + logger := zap.L().With(zap.String("namespace", ing.Namespace), zap.String("name", ing.Name)) + spec := istiov1beta1.VirtualService{ Hosts: sets.List(hosts), } @@ -159,8 +167,10 @@ func makeVirtualServiceSpec(ing *v1alpha1.Ingress, gateways map[v1alpha1.Ingress for _, rule := range ing.Spec.Rules { for i := range rule.HTTP.Paths { p := rule.HTTP.Paths[i] - hosts := hosts.Intersection(sets.New(rule.Hosts...)) - if hosts.Len() != 0 { + + // Skip host filtering for delegate VirtualServices + if len(hosts) == 0 || hosts.Intersection(sets.New(rule.Hosts...)).Len() != 0 { + logger.Debug("Creating HTTP route for path", zap.Int("pathIndex", i)) http := makeVirtualServiceRoute(hosts, &p, gateways, rule.Visibility) // Add all the Gateways that exist inside the http.match section of // the VirtualService. @@ -179,11 +189,16 @@ func makeVirtualServiceSpec(ing *v1alpha1.Ingress, gateways map[v1alpha1.Ingress func makeVirtualServiceRoute(hosts sets.Set[string], http *v1alpha1.HTTPIngressPath, gateways map[v1alpha1.IngressVisibility]sets.Set[string], visibility v1alpha1.IngressVisibility) *istiov1beta1.HTTPRoute { matches := []*istiov1beta1.HTTPMatchRequest{} - // Deduplicate hosts to avoid excessive matches, which cause a combinatorial expansion in Istio - distinctHosts := getDistinctHostPrefixes(hosts) - for _, host := range sets.List(distinctHosts) { - matches = append(matches, makeMatch(host, http.Path, http.Headers, gateways[visibility])) + // Handle empty hosts for delegate VirtualServices + if len(hosts) == 0 { + matches = append(matches, makeMatch("", http.Path, http.Headers, gateways[visibility])) + } else { + // Deduplicate hosts to avoid excessive matches, which cause a combinatorial expansion in Istio + distinctHosts := getDistinctHostPrefixes(hosts) + for _, host := range sets.List(distinctHosts) { + matches = append(matches, makeMatch(host, http.Path, http.Headers, gateways[visibility])) + } } weights := []*istiov1beta1.HTTPRouteDestination{} @@ -278,11 +293,16 @@ func keepLocalHostnames(hosts sets.Set[string]) sets.Set[string] { func makeMatch(host, path string, headers map[string]v1alpha1.HeaderMatch, gateways sets.Set[string]) *istiov1beta1.HTTPMatchRequest { match := &istiov1beta1.HTTPMatchRequest{ Gateways: sets.List(gateways), - Authority: &istiov1beta1.StringMatch{ + } + + // Skip authority match for delegate VirtualServices + if host != "" { + match.Authority = &istiov1beta1.StringMatch{ // Do not use Regex as Istio 1.4 or later has 100 bytes limitation. MatchType: &istiov1beta1.StringMatch_Prefix{Prefix: host}, - }, + } } + // Empty path is considered match all path. We only need to consider path // when it's non-empty. if path != "" { From 75434c7585e399daee7e8478f9c4f6ebbdda2551 Mon Sep 17 00:00:00 2001 From: Trey Hyde Date: Tue, 24 Feb 2026 16:04:43 -0800 Subject: [PATCH 6/7] Add delegate VirtualService support and update tests, addressing automated quality gate / lint issues --- pkg/reconciler/ingress/ingress.go | 1 - pkg/reconciler/ingress/ingress_test.go | 21 ++++++++++++ .../ingress/resources/virtual_service.go | 15 ++++----- .../ingress/resources/virtual_service_test.go | 32 +++++++++++++++++++ 4 files changed, 59 insertions(+), 10 deletions(-) diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index ed027152a2..86c2cdf85a 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -339,7 +339,6 @@ func (r *Reconciler) reconcileSystemGeneratedGateway(ctx context.Context, desire func (r *Reconciler) reconcileVirtualServices(ctx context.Context, ing *v1alpha1.Ingress, desired []*v1beta1.VirtualService, ) error { - // First, create all needed VirtualServices. kept := sets.New[string]() for _, d := range desired { diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index c742d3ce1d..cf845401da 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -335,6 +335,7 @@ func TestReconcile(t *testing.T) { }, WantCreates: []runtime.Object{ resources.MakeMeshVirtualService(insertProbe(ing("reconcile-failed")), gateways), + resources.MakeDelegateVirtualService(insertProbe(ing("reconcile-failed"))), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: resources.MakeIngressVirtualService(insertProbe(ing("reconcile-failed")), makeGatewayMap([]string{"knative-testing/knative-test-gateway", "knative-testing/" + config.KnativeIngressGateway}, nil)), @@ -366,6 +367,7 @@ func TestReconcile(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "reconcile-failed"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconcile-failed-mesh"), + Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconcile-failed-delegate"), Eventf(corev1.EventTypeWarning, "InternalError", "failed to update VirtualService: inducing failure for update virtualservices"), }, WantPatches: []clientgotesting.PatchActionImpl{ @@ -407,6 +409,7 @@ func TestReconcile(t *testing.T) { }}, WantCreates: []runtime.Object{ resources.MakeMeshVirtualService(insertProbe(ing("reconcile-virtualservice")), gateways), + resources.MakeDelegateVirtualService(insertProbe(ing("reconcile-virtualservice"))), }, WantDeletes: []clientgotesting.DeleteActionImpl{{ ActionImpl: clientgotesting.ActionImpl{ @@ -450,6 +453,7 @@ func TestReconcile(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "reconcile-virtualservice"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconcile-virtualservice-mesh"), + Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconcile-virtualservice-delegate"), Eventf(corev1.EventTypeNormal, "Updated", "Updated VirtualService %s/%s", "test-ns", "reconcile-virtualservice-ingress"), }, @@ -465,6 +469,7 @@ func TestReconcile(t *testing.T) { Objects: []runtime.Object{ basicReconciledIngress("ingress-ready"), resources.MakeMeshVirtualService(insertProbe(ing("ingress-ready")), makeGatewayMap([]string{"knative-testing/knative-test-gateway", "knative-testing/" + config.KnativeIngressGateway}, nil)), + resources.MakeDelegateVirtualService(insertProbe(ing("ingress-ready"))), resources.MakeIngressVirtualService(insertProbe(ing("ingress-ready")), makeGatewayMap([]string{"knative-testing/knative-test-gateway", "knative-testing/" + config.KnativeIngressGateway}, nil)), }, PostConditions: []func(*testing.T, *TableRow){proberCalledTimes(0)}, @@ -504,6 +509,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { withOwnerRef(ingressWithTLS("reconciling-ingress", externalIngressTLS)), withLabels(gwLabels), withSelector(selector)), resources.MakeMeshVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", externalIngressTLS)), externalIngressGateway), + resources.MakeDelegateVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", externalIngressTLS))), resources.MakeIngressVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", externalIngressTLS)), makeGatewayMap([]string{"test-ns/" + externalIngressTLSGatewayName}, nil)), }, WantPatches: []clientgotesting.PatchActionImpl{ @@ -544,6 +550,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "reconciling-ingress"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-mesh"), + Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-delegate"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-ingress"), }, Key: "test-ns/reconciling-ingress", @@ -566,6 +573,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { withLabels(gwLabels), withSelector(selector)), resources.MakeMeshVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", externalIngressTLS)), externalIngressGateway), + resources.MakeDelegateVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", externalIngressTLS))), resources.MakeIngressVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", externalIngressTLS)), makeGatewayMap([]string{"test-ns/" + externalIngressTLSGatewayName}, nil)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ @@ -611,6 +619,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "reconciling-ingress"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-mesh"), + Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-delegate"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-ingress"), }, Key: "test-ns/reconciling-ingress", @@ -632,6 +641,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { withLabels(gwLabels), withSelector(selector)), resources.MakeMeshVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", externalIngressTLS)), externalIngressGateway), + resources.MakeDelegateVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", externalIngressTLS))), resources.MakeIngressVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", externalIngressTLS)), makeGatewayMap([]string{ "istio-system/" + resources.WildcardGatewayName(wildcardCert.Name, ingressService.Namespace, ingressService.Name), "test-ns/" + externalIngressTLSGatewayName, @@ -675,6 +685,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "reconciling-ingress"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-mesh"), + Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-delegate"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-ingress"), }, Key: "test-ns/reconciling-ingress", @@ -808,6 +819,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { withLabels(gwLabels), withSelector(selector)), resources.MakeMeshVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", ingressTLSWithSecretNamespace("knative-serving"))), externalIngressGateway), + resources.MakeDelegateVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", ingressTLSWithSecretNamespace("knative-serving")))), resources.MakeIngressVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", ingressTLSWithSecretNamespace("knative-serving"))), makeGatewayMap([]string{"test-ns/" + externalIngressTLSGatewayName}, nil)), // The secret copy under istio-system. @@ -856,6 +868,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "reconciling-ingress"), Eventf(corev1.EventTypeNormal, "Created", "Created Secret %s/%s", "istio-system", targetSecretName), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-mesh"), + Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-delegate"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-ingress"), }, Key: "test-ns/reconciling-ingress", @@ -898,6 +911,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { withLabels(gwLabels), withSelector(selector)), resources.MakeMeshVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", ingressTLSWithSecretNamespace("knative-serving"))), externalIngressGateway), + resources.MakeDelegateVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", ingressTLSWithSecretNamespace("knative-serving")))), resources.MakeIngressVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", ingressTLSWithSecretNamespace("knative-serving"))), makeGatewayMap([]string{"test-ns/" + externalIngressTLSGatewayName}, nil)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ @@ -954,6 +968,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "reconciling-ingress"), Eventf(corev1.EventTypeNormal, "Updated", "Updated Secret %s/%s", "istio-system", targetSecretName), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-mesh"), + Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-delegate"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-ingress"), }, Key: "test-ns/reconciling-ingress", @@ -967,6 +982,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { }, WantCreates: []runtime.Object{ resources.MakeMeshVirtualService(insertProbe(ingressWithTLSClusterLocal("reconciling-ingress", externalIngressTLS)), externalIngressGateway), + resources.MakeDelegateVirtualService(insertProbe(ingressWithTLSClusterLocal("reconciling-ingress", externalIngressTLS))), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: ingressWithTLSAndStatusClusterLocal("reconciling-ingress", @@ -1003,6 +1019,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "reconciling-ingress"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-mesh"), + Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-delegate"), }, WantPatches: []clientgotesting.PatchActionImpl{ patchAddFinalizerAction("reconciling-ingress", ingressFinalizer), @@ -1071,6 +1088,7 @@ func TestReconcile_ClusterLocalDomainTLS(t *testing.T) { withOwnerRef(ingressWithTLS("reconciling-ingress", localIngressTLS)), withLabels(gwLabels), withSelector(selector)), resources.MakeMeshVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", localIngressTLS)), localIngressGateway), + resources.MakeDelegateVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", localIngressTLS))), resources.MakeIngressVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", localIngressTLS)), makeGatewayMap([]string{"knative-testing/" + config.KnativeIngressGateway}, []string{"knative-testing/" + config.KnativeLocalGateway, "test-ns/" + localIngressTLSGatewayName})), }, @@ -1115,6 +1133,7 @@ func TestReconcile_ClusterLocalDomainTLS(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "reconciling-ingress"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-mesh"), + Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-delegate"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-ingress"), }, Key: "test-ns/reconciling-ingress", @@ -1137,6 +1156,7 @@ func TestReconcile_ClusterLocalDomainTLS(t *testing.T) { withLabels(gwLabels), withSelector(selector)), resources.MakeMeshVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", localIngressTLS)), localIngressGateway), + resources.MakeDelegateVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", localIngressTLS))), resources.MakeIngressVirtualService(insertProbe(ingressWithTLS("reconciling-ingress", localIngressTLS)), makeGatewayMap([]string{"knative-testing/" + config.KnativeIngressGateway}, []string{"knative-testing/" + config.KnativeLocalGateway, "test-ns/" + localIngressTLSGatewayName})), }, @@ -1186,6 +1206,7 @@ func TestReconcile_ClusterLocalDomainTLS(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "reconciling-ingress"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-mesh"), + Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-delegate"), Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-ingress-ingress"), }, Key: "test-ns/reconciling-ingress", diff --git a/pkg/reconciler/ingress/resources/virtual_service.go b/pkg/reconciler/ingress/resources/virtual_service.go index 157b22c38e..d8aa744089 100644 --- a/pkg/reconciler/ingress/resources/virtual_service.go +++ b/pkg/reconciler/ingress/resources/virtual_service.go @@ -5,7 +5,7 @@ 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 + 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, @@ -20,8 +20,6 @@ import ( "fmt" "strings" - "go.uber.org/zap" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -157,8 +155,6 @@ func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVis } func makeVirtualServiceSpec(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string], hosts sets.Set[string]) *istiov1beta1.VirtualService { - logger := zap.L().With(zap.String("namespace", ing.Namespace), zap.String("name", ing.Name)) - spec := istiov1beta1.VirtualService{ Hosts: sets.List(hosts), } @@ -167,11 +163,12 @@ func makeVirtualServiceSpec(ing *v1alpha1.Ingress, gateways map[v1alpha1.Ingress for _, rule := range ing.Spec.Rules { for i := range rule.HTTP.Paths { p := rule.HTTP.Paths[i] + intersectedHosts := hosts.Intersection(sets.New(rule.Hosts...)) - // Skip host filtering for delegate VirtualServices - if len(hosts) == 0 || hosts.Intersection(sets.New(rule.Hosts...)).Len() != 0 { - logger.Debug("Creating HTTP route for path", zap.Int("pathIndex", i)) - http := makeVirtualServiceRoute(hosts, &p, gateways, rule.Visibility) + // For delegate VirtualServices (empty hosts), include all routes. + // For normal VirtualServices, only include routes whose rule hosts intersect. + if len(hosts) == 0 || intersectedHosts.Len() != 0 { + http := makeVirtualServiceRoute(intersectedHosts, &p, gateways, rule.Visibility) // Add all the Gateways that exist inside the http.match section of // the VirtualService. // This ensures that we are only using the Gateways that actually appear diff --git a/pkg/reconciler/ingress/resources/virtual_service_test.go b/pkg/reconciler/ingress/resources/virtual_service_test.go index bea2c83233..2a3b6ead64 100644 --- a/pkg/reconciler/ingress/resources/virtual_service_test.go +++ b/pkg/reconciler/ingress/resources/virtual_service_test.go @@ -96,6 +96,14 @@ func TestMakeVirtualServices_CorrectMetadata(t *testing.T) { RouteLabelKey: "test-route", RouteNamespaceLabelKey: "test-ns", }, + }, { + Name: "test-delegate", + Namespace: system.Namespace(), + Labels: map[string]string{ + networking.IngressLabelKey: "test", + RouteLabelKey: "test-route", + RouteNamespaceLabelKey: "test-ns", + }, }, { Name: "test-ingress", Namespace: system.Namespace(), @@ -126,6 +134,14 @@ func TestMakeVirtualServices_CorrectMetadata(t *testing.T) { }}}, }, expected: []metav1.ObjectMeta{{ + Name: "test-delegate", + Namespace: system.Namespace(), + Labels: map[string]string{ + networking.IngressLabelKey: "test", + RouteLabelKey: "test-route", + RouteNamespaceLabelKey: "test-ns", + }, + }, { Name: "test-ingress", Namespace: system.Namespace(), Labels: map[string]string{ @@ -162,6 +178,14 @@ func TestMakeVirtualServices_CorrectMetadata(t *testing.T) { RouteLabelKey: "test-route", RouteNamespaceLabelKey: "test-ns", }, + }, { + Name: "test-ingress-delegate", + Namespace: system.Namespace(), + Labels: map[string]string{ + networking.IngressLabelKey: "test-ingress", + RouteLabelKey: "test-route", + RouteNamespaceLabelKey: "test-ns", + }, }}, }, { name: "mesh only with namespace", @@ -191,6 +215,14 @@ func TestMakeVirtualServices_CorrectMetadata(t *testing.T) { RouteLabelKey: "test-route", RouteNamespaceLabelKey: "test-ns", }, + }, { + Name: "test-ingress-delegate", + Namespace: "test-ns", + Labels: map[string]string{ + networking.IngressLabelKey: "test-ingress", + RouteLabelKey: "test-route", + RouteNamespaceLabelKey: "test-ns", + }, }}, }} { t.Run(tc.name, func(t *testing.T) { From 269379e9ec1c044056e1c1790c8f3871425f0d4d Mon Sep 17 00:00:00 2001 From: Trey Hyde Date: Tue, 7 Apr 2026 17:51:05 -0700 Subject: [PATCH 7/7] Add support for enabling delegate VirtualService in Istio configuration --- config/400-config-istio.yaml | 6 ++ pkg/reconciler/ingress/config/istio.go | 15 ++++ pkg/reconciler/ingress/config/istio_test.go | 44 ++++++++++++ pkg/reconciler/ingress/ingress.go | 2 +- pkg/reconciler/ingress/ingress_test.go | 3 + .../ingress/resources/virtual_service.go | 8 ++- .../ingress/resources/virtual_service_test.go | 70 +++++++++++++++---- 7 files changed, 131 insertions(+), 17 deletions(-) diff --git a/config/400-config-istio.yaml b/config/400-config-istio.yaml index 476b8d6206..a8c3bea380 100644 --- a/config/400-config-istio.yaml +++ b/config/400-config-istio.yaml @@ -119,3 +119,9 @@ data: # Please use the new configuration format `local-gateways` for future compatibility. # This configuration will raise an error if either `external-gateways` or `local-gateways` is defined. local-gateway.knative-serving.knative-local-gateway: "knative-local-gateway.istio-system.svc.cluster.local" + + # enable-delegate-virtual-service controls whether a delegate VirtualService + # is created for each Ingress. When enabled, a VirtualService with no hosts + # or gateways is created, allowing other VirtualServices to delegate routing + # directly to Knative services. Disabled by default. + enable-delegate-virtual-service: "false" diff --git a/pkg/reconciler/ingress/config/istio.go b/pkg/reconciler/ingress/config/istio.go index 1e664461ef..d644a6f0a1 100644 --- a/pkg/reconciler/ingress/config/istio.go +++ b/pkg/reconciler/ingress/config/istio.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "sort" + "strconv" "strings" corev1 "k8s.io/api/core/v1" @@ -58,6 +59,9 @@ const ( // IstioNamespace is the namespace containing Istio IstioNamespace = "istio-system" + + // EnableDelegateVirtualServiceKey is the configmap key to enable delegate VirtualService creation. + EnableDelegateVirtualServiceKey = "enable-delegate-virtual-service" ) func defaultIngressGateways() []Gateway { @@ -121,6 +125,10 @@ type Istio struct { // LocalGateways specifies the gateway urls for public & private Ingress. LocalGateways []Gateway + + // EnableDelegateVirtualService enables the creation of delegate VirtualServices + // that allow direct ingress to Knative services. + EnableDelegateVirtualService bool } func (i Istio) Validate() error { @@ -188,6 +196,13 @@ func NewIstioFromConfigMap(configMap *corev1.ConfigMap) (*Istio, error) { defaultValues(ret) } + if v, ok := configMap.Data[EnableDelegateVirtualServiceKey]; ok { + ret.EnableDelegateVirtualService, err = strconv.ParseBool(v) + if err != nil { + return nil, fmt.Errorf("failed parsing %q: %w", EnableDelegateVirtualServiceKey, err) + } + } + err = ret.Validate() if err != nil { return nil, fmt.Errorf("invalid configuration: %w", err) diff --git a/pkg/reconciler/ingress/config/istio_test.go b/pkg/reconciler/ingress/config/istio_test.go index 88d3806248..82d4735514 100644 --- a/pkg/reconciler/ingress/config/istio_test.go +++ b/pkg/reconciler/ingress/config/istio_test.go @@ -589,6 +589,50 @@ func TestGatewayConfiguration(t *testing.T) { }, }, wantErr: true, + }, { + name: "enable-delegate-virtual-service set to true", + wantIstio: &Istio{ + IngressGateways: defaultIngressGateways(), + LocalGateways: defaultLocalGateways(), + EnableDelegateVirtualService: true, + }, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "enable-delegate-virtual-service": "true", + }, + }, + }, { + name: "enable-delegate-virtual-service set to false", + wantIstio: &Istio{ + IngressGateways: defaultIngressGateways(), + LocalGateways: defaultLocalGateways(), + EnableDelegateVirtualService: false, + }, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "enable-delegate-virtual-service": "false", + }, + }, + }, { + name: "enable-delegate-virtual-service with invalid value", + wantErr: true, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: IstioConfigName, + }, + Data: map[string]string{ + "enable-delegate-virtual-service": "notabool", + }, + }, }} for _, tt := range gatewayConfigTests { diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index 86c2cdf85a..f9e15fe2bb 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -224,7 +224,7 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress } gatewayNames[v1alpha1.IngressVisibilityClusterLocal].Insert(resources.GetQualifiedGatewayNames(clusterLocalIngressGateways)...) - vses, err := resources.MakeVirtualServices(ing, gatewayNames) + vses, err := resources.MakeVirtualServices(ing, gatewayNames, cfg.Istio.EnableDelegateVirtualService) if err != nil { return err } diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index cf845401da..edff1dff88 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -1063,6 +1063,7 @@ func TestReconcile_ExternalDomainTLS(t *testing.T) { Name: config.KnativeIngressGateway, ServiceURL: pkgnet.GetServiceHostname("istio-ingressgateway", "istio-system"), }}, + EnableDelegateVirtualService: true, }, Network: &netconfig.Config{ HTTPProtocol: netconfig.HTTPDisabled, @@ -1286,6 +1287,7 @@ func TestReconcile_ClusterLocalDomainTLS(t *testing.T) { Name: config.KnativeLocalGateway, ServiceURL: pkgnet.GetServiceHostname("istio-ingressgateway", "istio-system"), }}, + EnableDelegateVirtualService: true, }, Network: &netconfig.Config{ ClusterLocalDomainTLS: netconfig.EncryptionEnabled, @@ -1439,6 +1441,7 @@ func ReconcilerTestConfig() *config.Config { Name: config.KnativeIngressGateway, ServiceURL: pkgnet.GetServiceHostname("istio-ingressgateway", "istio-system"), }}, + EnableDelegateVirtualService: true, }, Network: &netconfig.Config{ ExternalDomainTLS: false, diff --git a/pkg/reconciler/ingress/resources/virtual_service.go b/pkg/reconciler/ingress/resources/virtual_service.go index d8aa744089..81d43781c6 100644 --- a/pkg/reconciler/ingress/resources/virtual_service.go +++ b/pkg/reconciler/ingress/resources/virtual_service.go @@ -123,7 +123,7 @@ func MakeDelegateVirtualService(ing *v1alpha1.Ingress) *v1beta1.VirtualService { } // MakeVirtualServices creates a mesh VirtualService and a virtual service for each gateway -func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string]) ([]*v1beta1.VirtualService, error) { +func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string], enableDelegateVS bool) ([]*v1beta1.VirtualService, error) { // Insert probe header ing = ing.DeepCopy() if _, err := ingress.InsertProbe(ing); err != nil { @@ -134,8 +134,10 @@ func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVis vss = append(vss, meshVs) } - if delegateVs := MakeDelegateVirtualService(ing); delegateVs != nil { - vss = append(vss, delegateVs) + if enableDelegateVS { + if delegateVs := MakeDelegateVirtualService(ing); delegateVs != nil { + vss = append(vss, delegateVs) + } } requiredGatewayCount := 0 diff --git a/pkg/reconciler/ingress/resources/virtual_service_test.go b/pkg/reconciler/ingress/resources/virtual_service_test.go index 2a3b6ead64..5e00a46bfc 100644 --- a/pkg/reconciler/ingress/resources/virtual_service_test.go +++ b/pkg/reconciler/ingress/resources/virtual_service_test.go @@ -63,13 +63,15 @@ var ( func TestMakeVirtualServices_CorrectMetadata(t *testing.T) { for _, tc := range []struct { - name string - gateways map[v1alpha1.IngressVisibility]sets.Set[string] - ci *v1alpha1.Ingress - expected []metav1.ObjectMeta + name string + gateways map[v1alpha1.IngressVisibility]sets.Set[string] + ci *v1alpha1.Ingress + enableDelegateVS bool + expected []metav1.ObjectMeta }{{ - name: "mesh and ingress", - gateways: makeGatewayMap([]string{"gateway"}, []string{"private-gateway"}), + name: "mesh and ingress", + gateways: makeGatewayMap([]string{"gateway"}, []string{"private-gateway"}), + enableDelegateVS: true, ci: &v1alpha1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -114,8 +116,9 @@ func TestMakeVirtualServices_CorrectMetadata(t *testing.T) { }, }}, }, { - name: "ingress only", - gateways: makeGatewayMap([]string{"gateway"}, []string{"private-gateway"}), + name: "ingress only", + gateways: makeGatewayMap([]string{"gateway"}, []string{"private-gateway"}), + enableDelegateVS: true, ci: &v1alpha1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -151,8 +154,9 @@ func TestMakeVirtualServices_CorrectMetadata(t *testing.T) { }, }}, }, { - name: "mesh only", - gateways: nil, + name: "mesh only", + gateways: nil, + enableDelegateVS: true, ci: &v1alpha1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ingress", @@ -188,8 +192,9 @@ func TestMakeVirtualServices_CorrectMetadata(t *testing.T) { }, }}, }, { - name: "mesh only with namespace", - gateways: nil, + name: "mesh only with namespace", + gateways: nil, + enableDelegateVS: true, ci: &v1alpha1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ingress", @@ -224,9 +229,48 @@ func TestMakeVirtualServices_CorrectMetadata(t *testing.T) { RouteNamespaceLabelKey: "test-ns", }, }}, + }, { + name: "delegate disabled - mesh and ingress", + gateways: makeGatewayMap([]string{"gateway"}, []string{"private-gateway"}), + enableDelegateVS: false, + ci: &v1alpha1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: system.Namespace(), + Labels: map[string]string{ + networking.IngressLabelKey: "test-ingress", + RouteLabelKey: "test-route", + RouteNamespaceLabelKey: "test-ns", + }, + }, + Spec: v1alpha1.IngressSpec{Rules: []v1alpha1.IngressRule{{ + Hosts: []string{ + "test-route.test-ns.svc.cluster.local", + }, + Visibility: v1alpha1.IngressVisibilityExternalIP, + HTTP: &v1alpha1.HTTPIngressRuleValue{}, + }}}, + }, + expected: []metav1.ObjectMeta{{ + Name: "test-mesh", + Namespace: system.Namespace(), + Labels: map[string]string{ + networking.IngressLabelKey: "test", + RouteLabelKey: "test-route", + RouteNamespaceLabelKey: "test-ns", + }, + }, { + Name: "test-ingress", + Namespace: system.Namespace(), + Labels: map[string]string{ + networking.IngressLabelKey: "test", + RouteLabelKey: "test-route", + RouteNamespaceLabelKey: "test-ns", + }, + }}, }} { t.Run(tc.name, func(t *testing.T) { - vss, err := MakeVirtualServices(tc.ci, tc.gateways) + vss, err := MakeVirtualServices(tc.ci, tc.gateways, tc.enableDelegateVS) if err != nil { t.Fatal("MakeVirtualServices failed:", err) }