diff --git a/config/config-gateway.yaml b/config/config-gateway.yaml index 5d217f82d..b31a83eae 100644 --- a/config/config-gateway.yaml +++ b/config/config-gateway.yaml @@ -50,6 +50,7 @@ data: - class: istio gateway: istio-system/knative-gateway service: istio-system/istio-ingressgateway + http-listener-name: http supported-features: - HTTPRouteRequestTimeout @@ -58,5 +59,6 @@ data: - class: istio gateway: istio-system/knative-local-gateway service: istio-system/knative-local-gateway + http-listener-name: http supported-features: - HTTPRouteRequestTimeout diff --git a/docs/test-version.md b/docs/test-version.md index 4208e35b6..67dc6818a 100644 --- a/docs/test-version.md +++ b/docs/test-version.md @@ -15,5 +15,5 @@ The following Gateway API version and Ingress were tested as part of the release | Ingress | Tested version | Unavailable features | | ------- | ----------------------- | ------------------------------ | -| Istio | v1.22.1 | retry,httpoption | -| Contour | v1.29.1 | httpoption | +| Istio | v1.22.1 | retry | +| Contour | v1.29.1 | | diff --git a/hack/test-env.sh b/hack/test-env.sh index 3de8ee1e9..222127e3c 100755 --- a/hack/test-env.sh +++ b/hack/test-env.sh @@ -16,9 +16,9 @@ export GATEWAY_API_VERSION="v1.1.0" export ISTIO_VERSION="1.22.1" -export ISTIO_UNSUPPORTED_E2E_TESTS="retry,httpoption" +export ISTIO_UNSUPPORTED_E2E_TESTS="retry" export CONTOUR_VERSION="v1.29.1" -export CONTOUR_UNSUPPORTED_E2E_TESTS="httpoption" +export CONTOUR_UNSUPPORTED_E2E_TESTS="" export ENVOY_GATEWAY_VERSION="latest" -export ENVOY_GATEWAY_UNSUPPORTED_E2E_TESTS="httpoption,host-rewrite" +export ENVOY_GATEWAY_UNSUPPORTED_E2E_TESTS="host-rewrite" diff --git a/pkg/reconciler/ingress/config/gateway.go b/pkg/reconciler/ingress/config/gateway.go index 82cdc58d2..fab9985f5 100644 --- a/pkg/reconciler/ingress/config/gateway.go +++ b/pkg/reconciler/ingress/config/gateway.go @@ -47,6 +47,7 @@ func defaultExternalGateways() []Gateway { Name: "istio-ingressgateway", Namespace: "istio-system", }, + HTTPListenerName: "http", SupportedFeatures: sets.New( features.SupportHTTPRouteRequestTimeout, ), @@ -64,6 +65,7 @@ func defaultLocalGateways() []Gateway { Name: "knative-local-gateway", Namespace: "istio-system", }, + HTTPListenerName: "http", SupportedFeatures: sets.New( features.SupportHTTPRouteRequestTimeout, ), @@ -90,6 +92,7 @@ type Gateway struct { types.NamespacedName Class string + HTTPListenerName string Service *types.NamespacedName SupportedFeatures sets.Set[features.SupportedFeature] } @@ -138,6 +141,7 @@ type gatewayEntry struct { Gateway string `json:"gateway"` Service *string `json:"service"` Class string `json:"class"` + HTTPListenerName string `json:"http-listener-name"` SupportedFeatures []features.SupportedFeature `json:"supported-features"` } @@ -152,6 +156,7 @@ func parseGatewayConfig(data string) ([]Gateway, error) { for i, entry := range entries { gw := Gateway{ Class: entry.Class, + HTTPListenerName: entry.HTTPListenerName, SupportedFeatures: sets.New(entry.SupportedFeatures...), } @@ -174,6 +179,9 @@ func parseGatewayConfig(data string) ([]Gateway, error) { if len(strings.TrimSpace(gw.Class)) == 0 { return nil, fmt.Errorf(`entry [%d] field "class" is required`, i) } + if len(strings.TrimSpace(gw.HTTPListenerName)) == 0 { + return nil, fmt.Errorf(`entry [%d] field "http-listener-name" is required`, i) + } gws = append(gws, gw) } diff --git a/pkg/reconciler/ingress/config/gateway_test.go b/pkg/reconciler/ingress/config/gateway_test.go index 3cc9dc275..86f7b0906 100644 --- a/pkg/reconciler/ingress/config/gateway_test.go +++ b/pkg/reconciler/ingress/config/gateway_test.go @@ -21,11 +21,11 @@ import ( "testing" corev1 "k8s.io/api/core/v1" - . "knative.dev/pkg/configmap/testing" + ktesting "knative.dev/pkg/configmap/testing" ) func TestFromConfigMap(t *testing.T) { - cm, example := ConfigMapsFromTestFile(t, GatewayConfigName) + cm, example := ktesting.ConfigMapsFromTestFile(t, GatewayConfigName) if _, err := FromConfigMap(cm); err != nil { t.Error("FromConfigMap(actual) =", err) @@ -58,9 +58,11 @@ func TestFromConfigMapErrors(t *testing.T) { data: map[string]string{ "external-gateways": `[{ "class":"boo", + "http-listener-name":"http", "gateway": "ns/n" },{ "class":"boo", + "http-listener-name":"http", "gateway": "ns/n" }]`, }, @@ -70,9 +72,11 @@ func TestFromConfigMapErrors(t *testing.T) { data: map[string]string{ "local-gateways": `[{ "class":"boo", + "http-listener-name":"http", "gateway": "ns/n" },{ "class":"boo", + "http-listener-name":"http", "gateway": "ns/n" }]`, }, @@ -119,6 +123,12 @@ func TestFromConfigMapErrors(t *testing.T) { "local-gateways": `[{"class": "class", "gateway": "ns/n", "service":"name"}]`, }, want: `unable to parse "local-gateways"`, + }, { + name: "missing gateway http-listener-name", + data: map[string]string{ + "local-gateways": `[{"class":"class", "gateway": "namespace/name"}]`, + }, + want: `unable to parse "local-gateways": entry [0] field "http-listener-name" is required`, }} for _, tc := range cases { @@ -141,9 +151,11 @@ func TestGatewayNoService(t *testing.T) { Data: map[string]string{ "external-gateways": ` - class: istio + http-listener-name: http gateway: istio-system/knative-gateway`, "local-gateways": ` - class: istio + http-listener-name: http gateway: istio-system/knative-local-gateway`, }, }) diff --git a/pkg/reconciler/ingress/controller_test.go b/pkg/reconciler/ingress/controller_test.go index 7f0df8904..b50a6676d 100644 --- a/pkg/reconciler/ingress/controller_test.go +++ b/pkg/reconciler/ingress/controller_test.go @@ -33,11 +33,11 @@ import ( _ "knative.dev/pkg/client/injection/kube/informers/core/v1/endpoints/fake" _ "knative.dev/pkg/client/injection/kube/informers/core/v1/pod/fake" - . "knative.dev/pkg/reconciler/testing" + ktesting "knative.dev/pkg/reconciler/testing" ) func TestNew(t *testing.T) { - ctx, _ := SetupFakeContext(t) + ctx, _ := ktesting.SetupFakeContext(t) c := NewController(ctx, configmap.NewStaticWatcher(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index 6fc4c7aa4..744867f49 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -107,12 +107,22 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress for _, rule := range ing.Spec.Rules { rule := rule - httproute, probeTargets, err := c.reconcileHTTPRoute(ctx, ingressHash, ing, &rule) + workloadRoute, probeTargets, err := c.reconcileWorkloadRoute(ctx, ingressHash, ing, &rule) if err != nil { return err } - if isHTTPRouteReady(httproute) { + // For now, we only generate the redirected HTTPRoute for external visibility, + // because currently we do not (yet) support HTTPs for cluster-local domains in net-gateway-api. + var redirectRoute *gatewayapi.HTTPRoute + if ing.Spec.HTTPOption == v1alpha1.HTTPOptionRedirected && rule.Visibility == v1alpha1.IngressVisibilityExternalIP { + redirectRoute, err = c.reconcileRedirectHTTPRoute(ctx, ing, &rule) + if err != nil { + return err + } + } + + if isHTTPRouteReady(workloadRoute) && (redirectRoute == nil || isHTTPRouteReady(redirectRoute)) { ing.Status.MarkNetworkConfigured() state, err := c.statusManager.DoProbes(ctx, probeTargets) diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index 643257d32..2c9a6faa9 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -27,7 +27,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" clientgotesting "k8s.io/client-go/testing" - "k8s.io/utils/pointer" "k8s.io/utils/ptr" fakegwapiclientset "knative.dev/net-gateway-api/pkg/client/injection/client/fake" @@ -49,8 +48,8 @@ import ( gatewayapi "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" - . "knative.dev/net-gateway-api/pkg/reconciler/testing" - . "knative.dev/pkg/reconciler/testing" + gwtesting "knative.dev/net-gateway-api/pkg/reconciler/testing" + ktesting "knative.dev/pkg/reconciler/testing" ) var ( @@ -139,7 +138,7 @@ var ( // TODO: Add more tests - e.g. invalid ingress, delete ingress, etc. func TestReconcile(t *testing.T) { - table := TableTest{{ + table := ktesting.TableTest{{ Name: "bad workqueue key", Key: "too/many/parts", }, { @@ -184,8 +183,8 @@ func TestReconcile(t *testing.T) { Patch: []byte(`{"metadata":{"finalizers":["ingresses.networking.internal.knative.dev"],"resourceVersion":""}}`), }}, WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), - Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"example.com\""), + ktesting.Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), + ktesting.Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"example.com\""), }, }, { Name: "reconcile ready ingress", @@ -197,7 +196,7 @@ func TestReconcile(t *testing.T) { // no extra update }} - table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { + table.Test(t, gwtesting.MakeFactory(func(ctx context.Context, listers *gwtesting.Listers, _ configmap.Watcher) controller.Reconciler { r := &Reconciler{ gwapiclient: fakegwapiclientset.Get(ctx), // Listers index properties about resources @@ -229,7 +228,7 @@ func TestReconcileTLS(t *testing.T) { secretName := "name-WE-STICK-A-LONG-UID-HERE" nsName := "ns" deleteTime := time.Now().Add(-10 * time.Second) - table := TableTest{{ + table := ktesting.TableTest{{ Name: "Happy TLS", Key: "ns/name", Objects: []runtime.Object{ @@ -259,8 +258,8 @@ func TestReconcileTLS(t *testing.T) { }), }}, WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), - Eventf(corev1.EventTypeNormal, "Created", `Created HTTPRoute "example.com"`), + ktesting.Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), + ktesting.Eventf(corev1.EventTypeNormal, "Created", `Created HTTPRoute "example.com"`), }, }, { Name: "Already Configured", @@ -325,14 +324,111 @@ func TestReconcileTLS(t *testing.T) { }), }}, WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), - Eventf(corev1.EventTypeNormal, "Created", `Created HTTPRoute "example.com"`), - Eventf(corev1.EventTypeWarning, "GatewayMissing", `Unable to update Gateway istio-system/istio-gateway`), - Eventf(corev1.EventTypeWarning, "InternalError", `Gateway istio-system/istio-gateway does not exist: gateway.gateway.networking.k8s.io "istio-gateway" not found`), + ktesting.Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), + ktesting.Eventf(corev1.EventTypeNormal, "Created", `Created HTTPRoute "example.com"`), + ktesting.Eventf(corev1.EventTypeWarning, "GatewayMissing", `Unable to update Gateway istio-system/istio-gateway`), + ktesting.Eventf(corev1.EventTypeWarning, "InternalError", `Gateway istio-system/istio-gateway does not exist: gateway.gateway.networking.k8s.io "istio-gateway" not found`), }, + }, { + Name: "TLS ingress with httpOption redirected", + Key: "ns/name", + Objects: append([]runtime.Object{ + ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS()), + secret(secretName, nsName), + gw(defaultListener), + }, servicesAndEndpoints...), + WantCreates: []runtime.Object{ + httpRoute(t, ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS()), withSectionName("kni-")), + httpRedirectRoute(t, ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS()), withSectionName("http")), + rp(secret(secretName, nsName)), + }, + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: gw(defaultListener, tlsListener("example.com", nsName, secretName)), + }}, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS(), func(i *v1alpha1.Ingress) { + // These are the things we expect to change in status. + i.Status.InitializeConditions() + i.Status.MarkIngressNotReady("HTTPRouteNotReady", "Waiting for HTTPRoute becomes Ready.") + i.Status.MarkLoadBalancerNotReady() + }), + }}, + WantPatches: []clientgotesting.PatchActionImpl{{ + ActionImpl: clientgotesting.ActionImpl{ + Namespace: "ns", + }, + Name: "name", + Patch: []byte(`{"metadata":{"finalizers":["ingresses.networking.internal.knative.dev"],"resourceVersion":""}}`), + }}, + WantEvents: []string{ + ktesting.Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), + ktesting.Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"example.com\""), + ktesting.Eventf(corev1.EventTypeNormal, "Created", "Created redirect HTTPRoute \"example.com-redirect\""), + }, + }, { + Name: "Cluster local TLS ingress with httpOption redirected", + Key: "ns/name", + Objects: append([]runtime.Object{ + ing(withInternalSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS()), + secret(secretName, nsName), + gw(defaultListener), + }, servicesAndEndpoints...), + WantCreates: []runtime.Object{ + httpRoute(t, ing(withInternalSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS())), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: ing(withInternalSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS(), func(i *v1alpha1.Ingress) { + // These are the things we expect to change in status. + i.Status.InitializeConditions() + i.Status.MarkIngressNotReady("HTTPRouteNotReady", "Waiting for HTTPRoute becomes Ready.") + i.Status.MarkLoadBalancerNotReady() + }), + }}, + WantPatches: []clientgotesting.PatchActionImpl{{ + ActionImpl: clientgotesting.ActionImpl{ + Namespace: "ns", + }, + Name: "name", + Patch: []byte(`{"metadata":{"finalizers":["ingresses.networking.internal.knative.dev"],"resourceVersion":""}}`), + }}, + WantEvents: []string{ + ktesting.Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), + ktesting.Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"foo.svc.cluster.local\""), + }, + }, { + Name: "No TLS ingress with httpOption redirected", + Key: "ns/name", + Objects: append([]runtime.Object{ + ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected), + secret(secretName, nsName), + gw(defaultListener), + }, servicesAndEndpoints...), + WantCreates: []runtime.Object{ + httpRoute(t, ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected), withSectionName("kni-")), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected, func(i *v1alpha1.Ingress) { + // These are the things we expect to change in status. + i.Status.InitializeConditions() + i.Status.MarkIngressNotReady("ReconcileIngressFailed", "Ingress reconciliation failed") + }), + }}, + WantPatches: []clientgotesting.PatchActionImpl{{ + ActionImpl: clientgotesting.ActionImpl{ + Namespace: "ns", + }, + Name: "name", + Patch: []byte(`{"metadata":{"finalizers":["ingresses.networking.internal.knative.dev"],"resourceVersion":""}}`), + }}, + WantEvents: []string{ + ktesting.Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), + ktesting.Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"example.com\""), + ktesting.Eventf(corev1.EventTypeWarning, "InternalError", "no TLS configuration provided in `spec.tls`. Failed to create HTTPRoute for HTTPS redirection"), + }, + WantErr: true, }} - table.Test(t, GatewayFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher, tr *TableRow) controller.Reconciler { + table.Test(t, GatewayFactory(func(ctx context.Context, listers *gwtesting.Listers, _ configmap.Watcher, tr *ktesting.TableRow) controller.Reconciler { r := &Reconciler{ gwapiclient: fakegwapiclientset.Get(ctx), httprouteLister: listers.GetHTTPRouteLister(), @@ -372,7 +468,7 @@ func TestReconcileTLS(t *testing.T) { } func TestReconcileProbing(t *testing.T) { - table := TableTest{{ + table := ktesting.TableTest{{ Name: "first reconciler probe returns false", Key: "ns/name", Ctx: withStatusManager(&fakeStatusManager{ @@ -401,8 +497,8 @@ func TestReconcileProbing(t *testing.T) { Patch: []byte(`{"metadata":{"finalizers":["ingresses.networking.internal.knative.dev"],"resourceVersion":""}}`), }}, WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), - Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"example.com\""), + ktesting.Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "name" finalizers`), + ktesting.Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"example.com\""), }, }, { Name: "prober callback all endpoints ready", @@ -2122,7 +2218,7 @@ func TestReconcileProbing(t *testing.T) { Version: "tr-9333a9a68409bb44f2a5f538d2d7c617e5338b6b6c1ebc5e00a19612a5c962c2", }, false }, - FakeDoProbes: func(ctx context.Context, s status.Backends) (status.ProbeState, error) { + FakeDoProbes: func(_ context.Context, s status.Backends) (status.ProbeState, error) { state := status.ProbeState{} expectedHash := "tr-9333a9a68409bb44f2a5f538d2d7c617e5338b6b6c1ebc5e00a19612a5c962c2" @@ -2185,7 +2281,7 @@ func TestReconcileProbing(t *testing.T) { WantUpdates: nil, // No updates }} - table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { + table.Test(t, gwtesting.MakeFactory(func(ctx context.Context, listers *gwtesting.Listers, _ configmap.Watcher) controller.Reconciler { statusManager := ctx.Value(fakeStatusKey).(status.Manager) r := &Reconciler{ gwapiclient: fakegwapiclientset.Get(ctx), @@ -2220,7 +2316,7 @@ func makeLoadBalancerNotReady(i *v1alpha1.Ingress) { i.Status.MarkLoadBalancerNotReady() } func TestReconcileProbingOffClusterGateway(t *testing.T) { - table := TableTest{{ + table := ktesting.TableTest{{ Name: "prober callback all endpoints ready", Key: "ns/name", Ctx: withStatusManager(&fakeStatusManager{ @@ -2292,7 +2388,7 @@ func TestReconcileProbingOffClusterGateway(t *testing.T) { )}, }, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", `no address found in status of Gateway istio-system/istio-gateway`), + ktesting.Eventf(corev1.EventTypeWarning, "InternalError", `no address found in status of Gateway istio-system/istio-gateway`), }, }, { Name: "gateway doesn't exist", @@ -2320,7 +2416,7 @@ func TestReconcileProbingOffClusterGateway(t *testing.T) { })}}, }} - table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { + table.Test(t, gwtesting.MakeFactory(func(ctx context.Context, listers *gwtesting.Listers, _ configmap.Watcher) controller.Reconciler { statusManager := ctx.Value(fakeStatusKey).(status.Manager) r := &Reconciler{ gwapiclient: fakegwapiclientset.Get(ctx), @@ -2378,13 +2474,30 @@ func httpRoute(t *testing.T, i *v1alpha1.Ingress, opts ...HTTPRouteOption) runti t.Helper() ingress.InsertProbe(i) ctx := (&testConfigStore{config: defaultConfig}).ToContext(context.Background()) - httpRoute, _ := resources.MakeHTTPRoute(ctx, i, &i.Spec.Rules[0]) + httpRoute, _ := resources.MakeHTTPRoute(ctx, i, &i.Spec.Rules[0], nil) for _, opt := range opts { opt(httpRoute) } return httpRoute } +func httpRedirectRoute(t *testing.T, i *v1alpha1.Ingress, opts ...HTTPRouteOption) runtime.Object { + t.Helper() + ingress.InsertProbe(i) + ctx := (&testConfigStore{config: defaultConfig}).ToContext(context.Background()) + httpRedirectRoute, _ := resources.MakeRedirectHTTPRoute(ctx, i, &i.Spec.Rules[0]) + for _, opt := range opts { + opt(httpRedirectRoute) + } + return httpRedirectRoute +} + +func withSectionName(sectionName string) HTTPRouteOption { + return func(h *gatewayapi.HTTPRoute) { + h.Spec.CommonRouteSpec.ParentRefs[0].SectionName = (*gatewayapi.SectionName)(ptr.To(sectionName)) + } +} + func httpRouteReady(h *gatewayapi.HTTPRoute) { h.Status.Parents = []gatewayapi.RouteParentStatus{{ Conditions: []metav1.Condition{{ @@ -2428,14 +2541,14 @@ func (t *testConfigStore) ToContext(ctx context.Context) context.Context { } // We need to inject the row's `Objects` to work-around improper pluralization in UnsafeGuessKindToResource -func GatewayFactory(ctor func(context.Context, *Listers, configmap.Watcher, *TableRow) controller.Reconciler) Factory { - return func(t *testing.T, r *TableRow) ( - controller.Reconciler, ActionRecorderList, EventList, +func GatewayFactory(ctor func(context.Context, *gwtesting.Listers, configmap.Watcher, *ktesting.TableRow) controller.Reconciler) ktesting.Factory { + return func(t *testing.T, r *ktesting.TableRow) ( + controller.Reconciler, ktesting.ActionRecorderList, ktesting.EventList, ) { - shim := func(c context.Context, l *Listers, cw configmap.Watcher) controller.Reconciler { + shim := func(c context.Context, l *gwtesting.Listers, cw configmap.Watcher) controller.Reconciler { return ctor(c, l, cw, r) } - return MakeFactory(shim)(t, r) + return gwtesting.MakeFactory(shim)(t, r) } } @@ -2493,15 +2606,15 @@ func setStatusPublicAddressHostname(g *gatewayapi.Gateway) { func tlsListener(hostname, nsName, secretName string) GatewayOption { return func(g *gatewayapi.Gateway) { g.Spec.Listeners = append(g.Spec.Listeners, gatewayapi.Listener{ - Name: gatewayapi.SectionName("kni-"), + Name: "kni-", Hostname: (*gatewayapi.Hostname)(&hostname), Port: 443, Protocol: "HTTPS", TLS: &gatewayapi.GatewayTLSConfig{ - Mode: (*gatewayapi.TLSModeType)(pointer.String("Terminate")), + Mode: (*gatewayapi.TLSModeType)(ptr.To("Terminate")), CertificateRefs: []gatewayapi.SecretObjectReference{{ - Group: (*gatewayapi.Group)(pointer.String("")), - Kind: (*gatewayapi.Kind)(pointer.String("Secret")), + Group: (*gatewayapi.Group)(ptr.To("")), + Kind: (*gatewayapi.Kind)(ptr.To("Secret")), Name: gatewayapi.ObjectName(secretName), Namespace: (*gatewayapi.Namespace)(&nsName), }}, @@ -2509,7 +2622,7 @@ func tlsListener(hostname, nsName, secretName string) GatewayOption { }, AllowedRoutes: &gatewayapi.AllowedRoutes{ Namespaces: &gatewayapi.RouteNamespaces{ - From: (*gatewayapi.FromNamespaces)(pointer.String("Selector")), + From: (*gatewayapi.FromNamespaces)(ptr.To("Selector")), Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "kubernetes.io/metadata.name": nsName, @@ -2587,12 +2700,14 @@ var ( Network: &networkcfg.Config{}, GatewayPlugin: &config.GatewayPlugin{ ExternalGateways: []config.Gateway{{ - Service: &types.NamespacedName{Namespace: "istio-system", Name: "istio-gateway"}, - NamespacedName: types.NamespacedName{Namespace: "istio-system", Name: "istio-gateway"}, + Service: &types.NamespacedName{Namespace: "istio-system", Name: "istio-gateway"}, + NamespacedName: types.NamespacedName{Namespace: "istio-system", Name: "istio-gateway"}, + HTTPListenerName: "http", }}, LocalGateways: []config.Gateway{{ - Service: &types.NamespacedName{Namespace: "istio-system", Name: "knative-local-gateway"}, - NamespacedName: types.NamespacedName{Namespace: "istio-system", Name: "knative-local-gateway"}, + Service: &types.NamespacedName{Namespace: "istio-system", Name: "knative-local-gateway"}, + NamespacedName: types.NamespacedName{Namespace: "istio-system", Name: "knative-local-gateway"}, + HTTPListenerName: "http", }}, }, } @@ -2601,10 +2716,12 @@ var ( Network: &networkcfg.Config{}, GatewayPlugin: &config.GatewayPlugin{ ExternalGateways: []config.Gateway{{ - NamespacedName: types.NamespacedName{Namespace: "istio-system", Name: "istio-gateway"}, + NamespacedName: types.NamespacedName{Namespace: "istio-system", Name: "istio-gateway"}, + HTTPListenerName: "http", }}, LocalGateways: []config.Gateway{{ - NamespacedName: types.NamespacedName{Namespace: "istio-system", Name: "knative-local-gateway"}, + NamespacedName: types.NamespacedName{Namespace: "istio-system", Name: "knative-local-gateway"}, + HTTPListenerName: "http", }}, }, } diff --git a/pkg/reconciler/ingress/lister_test.go b/pkg/reconciler/ingress/lister_test.go index e44e0e69a..78817fe38 100644 --- a/pkg/reconciler/ingress/lister_test.go +++ b/pkg/reconciler/ingress/lister_test.go @@ -36,7 +36,7 @@ import ( "knative.dev/networking/pkg/apis/networking/v1alpha1" "knative.dev/pkg/kmeta" - . "knative.dev/net-gateway-api/pkg/reconciler/testing" + rtesting "knative.dev/net-gateway-api/pkg/reconciler/testing" ) var ( @@ -256,7 +256,7 @@ func TestBackendsToProbeTargets(t *testing.T) { for _, test := range cases { t.Run(test.name, func(t *testing.T) { - tl := NewListers(test.objects) + tl := rtesting.NewListers(test.objects) l := &gatewayPodTargetLister{ endpointsLister: tl.GetEndpointsLister(), @@ -404,7 +404,7 @@ func TestListProbeTargetsNoService(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - tl := NewListers(test.objects) + tl := rtesting.NewListers(test.objects) l := &gatewayPodTargetLister{ endpointsLister: tl.GetEndpointsLister(), @@ -598,6 +598,10 @@ func withBackendAppendHeaders(key, val string) IngressOption { } } +func withHTTPOptionRedirected(i *v1alpha1.Ingress) { + i.Spec.HTTPOption = v1alpha1.HTTPOptionRedirected +} + func withInternalSpec(i *v1alpha1.Ingress) { i.Spec.Rules = append(i.Spec.Rules, v1alpha1.IngressRule{ Hosts: []string{"foo.svc", "foo.svc.cluster.local"}, diff --git a/pkg/reconciler/ingress/reconcile_resources.go b/pkg/reconciler/ingress/reconcile_resources.go index f0229b7e0..be06badcd 100644 --- a/pkg/reconciler/ingress/reconcile_resources.go +++ b/pkg/reconciler/ingress/reconcile_resources.go @@ -89,19 +89,26 @@ func probeTargets( return backends } -// reconcileHTTPRoute reconciles HTTPRoute. -func (c *Reconciler) reconcileHTTPRoute( +// reconcileWorkloadRoute reconciles the workloads HTTPRoute. +func (c *Reconciler) reconcileWorkloadRoute( ctx context.Context, hash string, ing *netv1alpha1.Ingress, rule *netv1alpha1.IngressRule, ) (*gatewayapi.HTTPRoute, status.Backends, error) { - recorder := controller.GetEventRecorder(ctx) + // If http > https redirect is enabled, this route must only be bound to the TLS listener on the gateway. + // For now, we only generate the TLS Listener on the external traffic gateway + // because there's no way to provide TLS for internal listeners. + var sectionName *gatewayapi.SectionName + if ing.Spec.HTTPOption == netv1alpha1.HTTPOptionRedirected && rule.Visibility == netv1alpha1.IngressVisibilityExternalIP { + sectionName = ptr.To(gatewayapi.SectionName(listenerPrefix + ing.GetUID())) + } + httproute, err := c.httprouteLister.HTTPRoutes(ing.Namespace).Get(resources.LongestHost(rule.Hosts)) if apierrs.IsNotFound(err) { - desired, err := resources.MakeHTTPRoute(ctx, ing, rule) + desired, err := resources.MakeHTTPRoute(ctx, ing, rule, sectionName) if err != nil { return nil, status.Backends{}, err } @@ -117,7 +124,7 @@ func (c *Reconciler) reconcileHTTPRoute( return nil, status.Backends{}, err } - return c.reconcileHTTPRouteUpdate(ctx, hash, ing, rule, httproute.DeepCopy()) + return c.reconcileHTTPRouteUpdate(ctx, hash, ing, rule, sectionName, httproute.DeepCopy()) } func (c *Reconciler) reconcileHTTPRouteUpdate( @@ -125,8 +132,8 @@ func (c *Reconciler) reconcileHTTPRouteUpdate( hash string, ing *netv1alpha1.Ingress, rule *netv1alpha1.IngressRule, - httproute *gatewayapi.HTTPRoute, -) (*gatewayapi.HTTPRoute, status.Backends, error) { + sectionName *gatewayapi.SectionName, + httproute *gatewayapi.HTTPRoute) (*gatewayapi.HTTPRoute, status.Backends, error) { const ( endpointPrefix = "ep-" @@ -156,11 +163,11 @@ func (c *Reconciler) reconcileHTTPRouteUpdate( newBackends, oldBackends := computeBackends(httproute, rule) if wasTransitionProbe && probeHash == hash && probe.Ready { - desired, err = resources.MakeHTTPRoute(ctx, ing, rule) + desired, err = resources.MakeHTTPRoute(ctx, ing, rule, sectionName) } else if wasEndpointProbe && probeHash == hash && probe.Ready { hash = transitionPrefix + hash - desired, err = resources.MakeHTTPRoute(ctx, ing, rule) + desired, err = resources.MakeHTTPRoute(ctx, ing, rule, sectionName) resources.UpdateProbeHash(desired, hash) resources.RemoveEndpointProbes(httproute) @@ -187,7 +194,7 @@ func (c *Reconciler) reconcileHTTPRouteUpdate( } } else { // Ingress changed with the same backends - desired, err = resources.MakeHTTPRoute(ctx, ing, rule) + desired, err = resources.MakeHTTPRoute(ctx, ing, rule, sectionName) } if err != nil { @@ -216,6 +223,60 @@ func (c *Reconciler) reconcileHTTPRouteUpdate( return httproute, probeTargets(hash, ing, rule, httproute), nil } +func (c *Reconciler) reconcileRedirectHTTPRoute(ctx context.Context, + ing *netv1alpha1.Ingress, + rule *netv1alpha1.IngressRule) (*gatewayapi.HTTPRoute, error) { + + // Don't create a redirect route for cluster-local visibility + if rule.Visibility == netv1alpha1.IngressVisibilityClusterLocal { + return nil, nil + } + + if ing.Spec.TLS == nil || len(ing.Spec.TLS) == 0 { + return nil, fmt.Errorf("no TLS configuration provided in `spec.tls`. Failed to create HTTPRoute for HTTPS redirection") + } + + recorder := controller.GetEventRecorder(ctx) + + desired, err := resources.MakeRedirectHTTPRoute(ctx, ing, rule) + if err != nil { + return nil, err + } + + redirectRoute, err := c.httprouteLister.HTTPRoutes(desired.Namespace).Get(desired.Name) + if apierrs.IsNotFound(err) { + redirectRoute, err = c.gwapiclient.GatewayV1().HTTPRoutes(desired.Namespace).Create(ctx, desired, metav1.CreateOptions{}) + if err != nil { + recorder.Eventf(ing, corev1.EventTypeWarning, "CreationFailed", "Failed to create redirect HTTPRoute: %v", err) + return nil, fmt.Errorf("failed to create redirect HTTPRoute: %w", err) + } + recorder.Eventf(ing, corev1.EventTypeNormal, "Created", "Created redirect HTTPRoute %q", redirectRoute.GetName()) + + } else if err != nil { + return nil, err + } + + if !equality.Semantic.DeepEqual(redirectRoute.Spec, desired.Spec) || + !equality.Semantic.DeepEqual(redirectRoute.Annotations, desired.Annotations) || + !equality.Semantic.DeepEqual(redirectRoute.Labels, desired.Labels) { + + // Don't modify the informers copy. + origin := redirectRoute.DeepCopy() + origin.Spec = desired.Spec + origin.Annotations = desired.Annotations + origin.Labels = desired.Labels + + updated, err := c.gwapiclient.GatewayV1().HTTPRoutes(origin.Namespace).Update(ctx, origin, metav1.UpdateOptions{}) + if err != nil { + recorder.Eventf(ing, corev1.EventTypeWarning, "UpdateFailed", "Failed to update redirect HTTPRoute: %v", err) + return nil, fmt.Errorf("failed to update redirect HTTPRoute: %w", err) + } + return updated, nil + } + + return redirectRoute, nil +} + func (c *Reconciler) reconcileTLS( ctx context.Context, tls *netv1alpha1.IngressTLS, ing *netv1alpha1.Ingress, ) ( diff --git a/pkg/reconciler/ingress/resources/httproute.go b/pkg/reconciler/ingress/resources/httproute.go index ad0d99380..a2fafab40 100644 --- a/pkg/reconciler/ingress/resources/httproute.go +++ b/pkg/reconciler/ingress/resources/httproute.go @@ -19,6 +19,7 @@ package resources import ( "context" "fmt" + "net/http" "slices" "sort" "strings" @@ -27,6 +28,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + "knative.dev/pkg/kmap" gatewayapi "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/pkg/features" @@ -37,6 +39,8 @@ import ( "knative.dev/pkg/kmeta" ) +const redirectHTTPRoutePostfix = "-redirect" + func UpdateProbeHash(r *gatewayapi.HTTPRoute, hash string) { // Note: we use indices and references to avoid mutating copies for rIdx := range r.Spec.Rules { @@ -189,11 +193,7 @@ func HTTPRouteKey(ing *netv1alpha1.Ingress, rule *netv1alpha1.IngressRule) types } // MakeHTTPRoute creates HTTPRoute to set up routing rules. -func MakeHTTPRoute( - ctx context.Context, - ing *netv1alpha1.Ingress, - rule *netv1alpha1.IngressRule, -) (*gatewayapi.HTTPRoute, error) { +func MakeHTTPRoute(ctx context.Context, ing *netv1alpha1.Ingress, rule *netv1alpha1.IngressRule, sectionName *gatewayapi.SectionName) (*gatewayapi.HTTPRoute, error) { visibility := "" if rule.Visibility == netv1alpha1.IngressVisibilityClusterLocal { @@ -204,22 +204,41 @@ func MakeHTTPRoute( ObjectMeta: metav1.ObjectMeta{ Name: LongestHost(rule.Hosts), Namespace: ing.Namespace, - Labels: kmeta.UnionMaps(ing.Labels, map[string]string{ + Labels: kmap.Union(ing.Labels, map[string]string{ networking.VisibilityLabelKey: visibility, }), - Annotations: kmeta.FilterMap(ing.GetAnnotations(), func(key string) bool { + Annotations: kmap.Filter(ing.GetAnnotations(), func(key string) bool { return key == corev1.LastAppliedConfigAnnotation }), OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(ing)}, }, - Spec: makeHTTPRouteSpec(ctx, rule), + Spec: makeHTTPRouteSpec(ctx, rule, sectionName), }, nil } -func makeHTTPRouteSpec( +// MakeRedirectHTTPRoute creates a HTTPRoute with a redirection filter. +func MakeRedirectHTTPRoute( ctx context.Context, + ing *netv1alpha1.Ingress, rule *netv1alpha1.IngressRule, -) gatewayapi.HTTPRouteSpec { +) (*gatewayapi.HTTPRoute, error) { + return &gatewayapi.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: LongestHost(rule.Hosts) + redirectHTTPRoutePostfix, + Namespace: ing.Namespace, + Labels: kmap.Union(ing.Labels, map[string]string{ + networking.VisibilityLabelKey: "", + }), + Annotations: kmap.Filter(ing.GetAnnotations(), func(key string) bool { + return key == corev1.LastAppliedConfigAnnotation + }), + OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(ing)}, + }, + Spec: makeRedirectHTTPRouteSpec(ctx, rule), + }, nil +} + +func makeHTTPRouteSpec(ctx context.Context, rule *netv1alpha1.IngressRule, sectionName *gatewayapi.SectionName) gatewayapi.HTTPRouteSpec { hostnames := make([]gatewayapi.Hostname, 0, len(rule.Hosts)) for _, hostname := range rule.Hosts { @@ -245,6 +264,10 @@ func makeHTTPRouteSpec( Name: gatewayapi.ObjectName(gateway.Name), } + if sectionName != nil { + gatewayRef.SectionName = sectionName + } + return gatewayapi.HTTPRouteSpec{ Hostnames: hostnames, Rules: rules, @@ -365,6 +388,95 @@ func makeHTTPRouteRule(gw config.Gateway, rule *netv1alpha1.IngressRule) []gatew return rules } +func makeRedirectHTTPRouteSpec( + ctx context.Context, + rule *netv1alpha1.IngressRule, +) gatewayapi.HTTPRouteSpec { + hostnames := make([]gatewayapi.Hostname, 0, len(rule.Hosts)) + for _, hostname := range rule.Hosts { + hostnames = append(hostnames, gatewayapi.Hostname(hostname)) + } + + pluginConfig := config.FromContext(ctx).GatewayPlugin + + var gateway config.Gateway + + if rule.Visibility == netv1alpha1.IngressVisibilityClusterLocal { + gateway = pluginConfig.LocalGateway() + } else { + gateway = pluginConfig.ExternalGateway() + } + + rules := makeRedirectHTTPRouteRule(rule) + + gatewayRef := gatewayapi.ParentReference{ + Group: (*gatewayapi.Group)(&gatewayapi.GroupVersion.Group), + Kind: (*gatewayapi.Kind)(ptr.To("Gateway")), + Namespace: ptr.To(gatewayapi.Namespace(gateway.Namespace)), + Name: gatewayapi.ObjectName(gateway.Name), + // Redirect routes are only added on to the http listener + SectionName: ptr.To(gatewayapi.SectionName(gateway.HTTPListenerName)), + } + + return gatewayapi.HTTPRouteSpec{ + Hostnames: hostnames, + Rules: rules, + CommonRouteSpec: gatewayapi.CommonRouteSpec{ParentRefs: []gatewayapi.ParentReference{ + gatewayRef, + }}, + } +} + +func makeRedirectHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayapi.HTTPRouteRule { + rules := []gatewayapi.HTTPRouteRule{} + + for _, path := range rule.HTTP.Paths { + preFilters := []gatewayapi.HTTPRouteFilter{ + { + Type: gatewayapi.HTTPRouteFilterRequestRedirect, + RequestRedirect: &gatewayapi.HTTPRequestRedirectFilter{ + Scheme: ptr.To("https"), + Port: ptr.To(gatewayapi.PortNumber(443)), + StatusCode: ptr.To(http.StatusMovedPermanently), + }, + }} + + matches := matchesFromRulePath(path) + rule := gatewayapi.HTTPRouteRule{ + Filters: preFilters, + Matches: matches, + } + rules = append(rules, rule) + } + return rules +} + +func matchesFromRulePath(path netv1alpha1.HTTPIngressPath) []gatewayapi.HTTPRouteMatch { + pathPrefix := "/" + if path.Path != "" { + pathPrefix = path.Path + } + pathMatch := gatewayapi.HTTPPathMatch{ + Type: ptr.To(gatewayapi.PathMatchPathPrefix), + Value: ptr.To(pathPrefix), + } + + var headerMatchList []gatewayapi.HTTPHeaderMatch //nolint:all + for k, v := range path.Headers { + headerMatch := gatewayapi.HTTPHeaderMatch{ + Type: ptr.To(gatewayapi.HeaderMatchExact), + Name: gatewayapi.HTTPHeaderName(k), + Value: v.Exact, + } + headerMatchList = append(headerMatchList, headerMatch) + } + + // Sort HTTPHeaderMatch as the order is random. + sort.Sort(HTTPHeaderMatchList(headerMatchList)) + + return []gatewayapi.HTTPRouteMatch{{Path: &pathMatch, Headers: headerMatchList}} +} + type HTTPHeaderList []gatewayapi.HTTPHeader func (h HTTPHeaderList) Len() int { diff --git a/pkg/reconciler/ingress/resources/httproute_test.go b/pkg/reconciler/ingress/resources/httproute_test.go index 2a59a6f23..9886e6836 100644 --- a/pkg/reconciler/ingress/resources/httproute_test.go +++ b/pkg/reconciler/ingress/resources/httproute_test.go @@ -19,6 +19,7 @@ package resources import ( "context" "fmt" + "net/http" "testing" "github.com/google/go-cmp/cmp" @@ -613,7 +614,288 @@ func TestMakeHTTPRoute(t *testing.T) { tcs := &testConfigStore{config: cfg} ctx := tcs.ToContext(context.Background()) - route, err := MakeHTTPRoute(ctx, tc.ing, &rule) + route, err := MakeHTTPRoute(ctx, tc.ing, &rule, nil) + if err != nil { + t.Fatal("MakeHTTPRoute failed:", err) + } + tc.expected[i].OwnerReferences = []metav1.OwnerReference{*kmeta.NewControllerRef(tc.ing)} + if diff := cmp.Diff(tc.expected[i], route); diff != "" { + t.Error("Unexpected HTTPRoute (-want +got):", diff) + } + } + }) + } +} + +func TestMakeHTTPRouteWithSectionName(t *testing.T) { + for _, tc := range []struct { + name string + ing *v1alpha1.Ingress + expected []*gatewayapi.HTTPRoute + changeConfig func(gw *config.Config) + }{ + { + name: "single external domain with split and cluster local", + ing: &v1alpha1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: testIngressName, + Namespace: testNamespace, + Labels: map[string]string{ + networking.IngressLabelKey: testIngressName, + }, + }, + Spec: v1alpha1.IngressSpec{ + Rules: []v1alpha1.IngressRule{ + { + Hosts: testHosts, + Visibility: v1alpha1.IngressVisibilityExternalIP, + HTTP: &v1alpha1.HTTPIngressRuleValue{ + Paths: []v1alpha1.HTTPIngressPath{{ + AppendHeaders: map[string]string{ + "Foo": "bar", + }, + Splits: []v1alpha1.IngressBackendSplit{{ + IngressBackend: v1alpha1.IngressBackend{ + ServiceName: "goo", + ServicePort: intstr.FromInt(123), + }, + Percent: 12, + AppendHeaders: map[string]string{ + "Baz": "blah", + "Bleep": "bloop", + }, + }, { + IngressBackend: v1alpha1.IngressBackend{ + ServiceName: "doo", + ServicePort: intstr.FromInt(124), + }, + Percent: 88, + AppendHeaders: map[string]string{ + "Baz": "blurg", + }, + }}, + }}, + }, + }, { + Hosts: testLocalHosts, + Visibility: v1alpha1.IngressVisibilityClusterLocal, + HTTP: &v1alpha1.HTTPIngressRuleValue{ + Paths: []v1alpha1.HTTPIngressPath{{ + AppendHeaders: map[string]string{ + "Foo": "bar", + }, + Splits: []v1alpha1.IngressBackendSplit{{ + IngressBackend: v1alpha1.IngressBackend{ + ServiceName: "goo", + ServicePort: intstr.FromInt(123), + }, + Percent: 12, + AppendHeaders: map[string]string{ + "Bleep": "bloop", + "Baz": "blah", + }, + }, { + IngressBackend: v1alpha1.IngressBackend{ + ServiceName: "doo", + ServicePort: intstr.FromInt(124), + }, + Percent: 88, + AppendHeaders: map[string]string{ + "Baz": "blurg", + }, + }}, + }}, + }, + }, + }}, + }, + expected: []*gatewayapi.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: LongestHost(testHosts), + Namespace: testNamespace, + Labels: map[string]string{ + networking.IngressLabelKey: testIngressName, + "networking.knative.dev/visibility": "", + }, + Annotations: map[string]string{}, + }, + Spec: gatewayapi.HTTPRouteSpec{ + Hostnames: []gatewayapi.Hostname{externalHost}, + Rules: []gatewayapi.HTTPRouteRule{{ + BackendRefs: []gatewayapi.HTTPBackendRef{{ + BackendRef: gatewayapi.BackendRef{ + BackendObjectReference: gatewayapi.BackendObjectReference{ + Group: (*gatewayapi.Group)(ptr.To("")), + Kind: (*gatewayapi.Kind)(ptr.To("Service")), + Name: gatewayapi.ObjectName("goo"), + Port: ptr.To[gatewayapi.PortNumber](123), + }, + Weight: ptr.To(int32(12)), + }, + Filters: []gatewayapi.HTTPRouteFilter{{ + Type: gatewayapi.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayapi.HTTPHeaderFilter{ + Set: []gatewayapi.HTTPHeader{ + { + Name: "Baz", + Value: "blah", + }, + { + Name: "Bleep", + Value: "bloop", + }, + }}}}, + }, { + BackendRef: gatewayapi.BackendRef{ + BackendObjectReference: gatewayapi.BackendObjectReference{ + Group: (*gatewayapi.Group)(ptr.To("")), + Kind: (*gatewayapi.Kind)(ptr.To("Service")), + Port: ptr.To[gatewayapi.PortNumber](124), + Name: gatewayapi.ObjectName("doo"), + }, + Weight: ptr.To(int32(88)), + }, + Filters: []gatewayapi.HTTPRouteFilter{{ + Type: gatewayapi.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayapi.HTTPHeaderFilter{ + Set: []gatewayapi.HTTPHeader{ + { + Name: "Baz", + Value: "blurg", + }, + }, + }}}, + }}, + Filters: []gatewayapi.HTTPRouteFilter{{ + Type: gatewayapi.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayapi.HTTPHeaderFilter{ + Set: []gatewayapi.HTTPHeader{ + { + Name: "Foo", + Value: "bar", + }, + }, + }}}, + Matches: []gatewayapi.HTTPRouteMatch{ + { + Path: &gatewayapi.HTTPPathMatch{ + Type: ptr.To(gatewayapi.PathMatchPathPrefix), + Value: ptr.To("/"), + }, + }, + }, + }}, + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{{ + Group: (*gatewayapi.Group)(ptr.To("gateway.networking.k8s.io")), + Kind: (*gatewayapi.Kind)(ptr.To("Gateway")), + Namespace: ptr.To[gatewayapi.Namespace]("test-ns"), + Name: gatewayapi.ObjectName("foo"), + SectionName: ptr.To[gatewayapi.SectionName]("http"), + }}, + }, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: LongestHost(testLocalHosts), + Namespace: testNamespace, + Labels: map[string]string{ + networking.IngressLabelKey: testIngressName, + "networking.knative.dev/visibility": "cluster-local", + }, + Annotations: map[string]string{}, + }, + Spec: gatewayapi.HTTPRouteSpec{ + Hostnames: []gatewayapi.Hostname{localHostShortest, localHostShort, localHostFull}, + Rules: []gatewayapi.HTTPRouteRule{{ + BackendRefs: []gatewayapi.HTTPBackendRef{{ + BackendRef: gatewayapi.BackendRef{ + BackendObjectReference: gatewayapi.BackendObjectReference{ + Group: (*gatewayapi.Group)(ptr.To("")), + Kind: (*gatewayapi.Kind)(ptr.To("Service")), + Port: ptr.To[gatewayapi.PortNumber](123), + Name: gatewayapi.ObjectName("goo"), + }, + Weight: ptr.To(int32(12)), + }, + Filters: []gatewayapi.HTTPRouteFilter{{ + Type: gatewayapi.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayapi.HTTPHeaderFilter{ + Set: []gatewayapi.HTTPHeader{ + { + Name: "Baz", + Value: "blah", + }, + { + Name: "Bleep", + Value: "bloop", + }, + }}}}, + }, { + BackendRef: gatewayapi.BackendRef{ + BackendObjectReference: gatewayapi.BackendObjectReference{ + Group: (*gatewayapi.Group)(ptr.To("")), + Kind: (*gatewayapi.Kind)(ptr.To("Service")), + Port: ptr.To[gatewayapi.PortNumber](124), + Name: gatewayapi.ObjectName("doo"), + }, + Weight: ptr.To(int32(88)), + }, + Filters: []gatewayapi.HTTPRouteFilter{{ + Type: gatewayapi.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayapi.HTTPHeaderFilter{ + Set: []gatewayapi.HTTPHeader{ + { + Name: "Baz", + Value: "blurg", + }, + }, + }}}, + }}, + Filters: []gatewayapi.HTTPRouteFilter{{ + Type: gatewayapi.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayapi.HTTPHeaderFilter{ + Set: []gatewayapi.HTTPHeader{ + { + Name: "Foo", + Value: "bar", + }, + }, + }}}, + Matches: []gatewayapi.HTTPRouteMatch{{ + Path: &gatewayapi.HTTPPathMatch{ + Type: ptr.To(gatewayapi.PathMatchPathPrefix), + Value: ptr.To("/"), + }, + }}, + }}, + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{{ + Group: (*gatewayapi.Group)(ptr.To("gateway.networking.k8s.io")), + Kind: (*gatewayapi.Kind)(ptr.To("Gateway")), + Namespace: ptr.To[gatewayapi.Namespace]("test-ns"), + Name: gatewayapi.ObjectName("foo-local"), + SectionName: ptr.To[gatewayapi.SectionName]("http"), + }}, + }, + }, + }, + }, + }} { + t.Run(tc.name, func(t *testing.T) { + for i, rule := range tc.ing.Spec.Rules { + rule := rule + cfg := testConfig.DeepCopy() + if tc.changeConfig != nil { + tc.changeConfig(cfg) + + fmt.Printf("%#v", cfg.GatewayPlugin.ExternalGateways) + } + tcs := &testConfigStore{config: cfg} + ctx := tcs.ToContext(context.Background()) + + route, err := MakeHTTPRoute(ctx, tc.ing, &rule, ptr.To[gatewayapi.SectionName]("http")) if err != nil { t.Fatal("MakeHTTPRoute failed:", err) } @@ -632,7 +914,7 @@ func TestAddEndpointProbes(t *testing.T) { ing := testIngress.DeepCopy() rule := &ing.Spec.Rules[0] - route, err := MakeHTTPRoute(ctx, ing, rule) + route, err := MakeHTTPRoute(ctx, ing, rule, nil) if err != nil { t.Fatal("MakeHTTPRoute failed:", err) } @@ -819,7 +1101,7 @@ func TestRemoveEndpointProbes(t *testing.T) { ing := testIngress.DeepCopy() rule := &ing.Spec.Rules[0] - route, err := MakeHTTPRoute(ctx, ing, rule) + route, err := MakeHTTPRoute(ctx, ing, rule, nil) if err != nil { t.Fatal("MakeHTTPRoute failed:", err) } @@ -840,7 +1122,7 @@ func TestUpdateProbeHash(t *testing.T) { ctx := tcs.ToContext(context.Background()) ing := testIngress.DeepCopy() rule := &ing.Spec.Rules[0] - route, err := MakeHTTPRoute(ctx, ing, rule) + route, err := MakeHTTPRoute(ctx, ing, rule, nil) if err != nil { t.Fatal("MakeHTTPRoute failed:", err) } @@ -1028,7 +1310,7 @@ func TestAddOldBackend(t *testing.T) { ing := testIngress.DeepCopy() rule := &ing.Spec.Rules[0] - route, err := MakeHTTPRoute(ctx, ing, rule) + route, err := MakeHTTPRoute(ctx, ing, rule, nil) if err != nil { t.Fatal("MakeHTTPRoute failed:", err) } @@ -1185,6 +1467,266 @@ func TestAddOldBackend(t *testing.T) { } } +func TestMakeRedirectHTTPRoute(t *testing.T) { + for _, tc := range []struct { + name string + ing *v1alpha1.Ingress + expected []*gatewayapi.HTTPRoute + }{ + { + name: "cluster local domain only", + ing: &v1alpha1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: testIngressName, + Namespace: testNamespace, + Labels: map[string]string{ + networking.IngressLabelKey: testIngressName, + }, + }, + Spec: v1alpha1.IngressSpec{ + Rules: []v1alpha1.IngressRule{ + { + Hosts: testLocalHosts, + Visibility: v1alpha1.IngressVisibilityClusterLocal, + HTTP: &v1alpha1.HTTPIngressRuleValue{ + Paths: []v1alpha1.HTTPIngressPath{{ + Splits: []v1alpha1.IngressBackendSplit{{ + IngressBackend: v1alpha1.IngressBackend{ + ServiceName: "goo", + ServicePort: intstr.FromInt(123), + }, + Percent: 100, + }}, + }}, + }, + }, + }}, + }, + expected: []*gatewayapi.HTTPRoute{}, + }, { + name: "single external domain and cluster local", + ing: &v1alpha1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: testIngressName, + Namespace: testNamespace, + Labels: map[string]string{ + networking.IngressLabelKey: testIngressName, + }, + }, + Spec: v1alpha1.IngressSpec{ + Rules: []v1alpha1.IngressRule{ + { + Hosts: testHosts, + Visibility: v1alpha1.IngressVisibilityExternalIP, + HTTP: &v1alpha1.HTTPIngressRuleValue{ + Paths: []v1alpha1.HTTPIngressPath{{ + Splits: []v1alpha1.IngressBackendSplit{{ + IngressBackend: v1alpha1.IngressBackend{ + ServiceName: "goo", + ServicePort: intstr.FromInt(123), + }, + Percent: 100, + }}, + }}, + }, + }, { + Hosts: testLocalHosts, + Visibility: v1alpha1.IngressVisibilityClusterLocal, + HTTP: &v1alpha1.HTTPIngressRuleValue{ + Paths: []v1alpha1.HTTPIngressPath{{ + Splits: []v1alpha1.IngressBackendSplit{{ + IngressBackend: v1alpha1.IngressBackend{ + ServiceName: "goo", + ServicePort: intstr.FromInt(123), + }, + Percent: 100, + }}, + }}, + }, + }, + }}, + }, + expected: []*gatewayapi.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: LongestHost(testHosts) + redirectHTTPRoutePostfix, + Namespace: testNamespace, + Labels: map[string]string{ + networking.IngressLabelKey: testIngressName, + "networking.knative.dev/visibility": "", + }, + Annotations: map[string]string{}, + }, + Spec: gatewayapi.HTTPRouteSpec{ + Hostnames: []gatewayapi.Hostname{externalHost}, + Rules: []gatewayapi.HTTPRouteRule{{ + Filters: []gatewayapi.HTTPRouteFilter{{ + Type: gatewayapi.HTTPRouteFilterRequestRedirect, + RequestRedirect: &gatewayapi.HTTPRequestRedirectFilter{ + Scheme: ptr.To("https"), + Port: ptr.To(gatewayapi.PortNumber(443)), + StatusCode: ptr.To(http.StatusMovedPermanently), + }, + }}, + Matches: []gatewayapi.HTTPRouteMatch{ + { + Path: &gatewayapi.HTTPPathMatch{ + Type: ptr.To(gatewayapi.PathMatchPathPrefix), + Value: ptr.To("/"), + }, + }, + }, + }}, + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{{ + Group: (*gatewayapi.Group)(ptr.To("gateway.networking.k8s.io")), + Kind: (*gatewayapi.Kind)(ptr.To("Gateway")), + Namespace: ptr.To[gatewayapi.Namespace]("test-ns"), + Name: gatewayapi.ObjectName("foo"), + SectionName: ptr.To[gatewayapi.SectionName]("http"), + }}, + }, + }, + }, + }, + }, { + name: "multiple paths with header conditions", + ing: &v1alpha1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: testIngressName, + Namespace: testNamespace, + Labels: map[string]string{ + networking.IngressLabelKey: testIngressName, + }, + }, + Spec: v1alpha1.IngressSpec{Rules: []v1alpha1.IngressRule{{ + Hosts: testHosts, + Visibility: v1alpha1.IngressVisibilityExternalIP, + HTTP: &v1alpha1.HTTPIngressRuleValue{ + Paths: []v1alpha1.HTTPIngressPath{{ + Headers: map[string]v1alpha1.HeaderMatch{ + "tag": { + Exact: "goo", + }, + }, + Splits: []v1alpha1.IngressBackendSplit{{ + IngressBackend: v1alpha1.IngressBackend{ + ServiceName: "goo", + ServicePort: intstr.FromInt(123), + }, + Percent: 100, + }}, + }, { + Path: "/doo", + Headers: map[string]v1alpha1.HeaderMatch{ + "tag": { + Exact: "doo", + }, + }, + Splits: []v1alpha1.IngressBackendSplit{{ + IngressBackend: v1alpha1.IngressBackend{ + ServiceName: "doo", + ServicePort: intstr.FromInt(124), + }, + Percent: 100, + }}, + }}, + }, + }}}, + }, + expected: []*gatewayapi.HTTPRoute{{ + ObjectMeta: metav1.ObjectMeta{ + Name: LongestHost(testHosts) + redirectHTTPRoutePostfix, + Namespace: testNamespace, + Labels: map[string]string{ + networking.IngressLabelKey: testIngressName, + "networking.knative.dev/visibility": "", + }, + Annotations: map[string]string{}, + }, + Spec: gatewayapi.HTTPRouteSpec{ + Hostnames: []gatewayapi.Hostname{externalHost}, + Rules: []gatewayapi.HTTPRouteRule{ + { + Filters: []gatewayapi.HTTPRouteFilter{{ + Type: gatewayapi.HTTPRouteFilterRequestRedirect, + RequestRedirect: &gatewayapi.HTTPRequestRedirectFilter{ + Scheme: ptr.To("https"), + Port: ptr.To(gatewayapi.PortNumber(443)), + StatusCode: ptr.To(http.StatusMovedPermanently), + }, + }}, + Matches: []gatewayapi.HTTPRouteMatch{ + { + Path: &gatewayapi.HTTPPathMatch{ + Type: ptr.To(gatewayapi.PathMatchPathPrefix), + Value: ptr.To("/"), + }, + Headers: []gatewayapi.HTTPHeaderMatch{{ + Type: ptr.To(gatewayapi.HeaderMatchExact), + Name: gatewayapi.HTTPHeaderName("tag"), + Value: "goo", + }}, + }}, + }, { + Filters: []gatewayapi.HTTPRouteFilter{{ + Type: gatewayapi.HTTPRouteFilterRequestRedirect, + RequestRedirect: &gatewayapi.HTTPRequestRedirectFilter{ + Scheme: ptr.To("https"), + Port: ptr.To(gatewayapi.PortNumber(443)), + StatusCode: ptr.To(http.StatusMovedPermanently), + }, + }}, + Matches: []gatewayapi.HTTPRouteMatch{ + { + Path: &gatewayapi.HTTPPathMatch{ + Type: ptr.To(gatewayapi.PathMatchPathPrefix), + Value: ptr.To("/doo"), + }, + Headers: []gatewayapi.HTTPHeaderMatch{{ + Type: ptr.To(gatewayapi.HeaderMatchExact), + Name: gatewayapi.HTTPHeaderName("tag"), + Value: "doo", + }}, + }}, + }, + }, + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{{ + Group: (*gatewayapi.Group)(ptr.To("gateway.networking.k8s.io")), + Kind: (*gatewayapi.Kind)(ptr.To("Gateway")), + Namespace: ptr.To[gatewayapi.Namespace]("test-ns"), + Name: gatewayapi.ObjectName("foo"), + SectionName: ptr.To[gatewayapi.SectionName]("http"), + }}, + }, + }, + }}, + }} { + t.Run(tc.name, func(t *testing.T) { + for _, exp := range tc.expected { + exp.OwnerReferences = []metav1.OwnerReference{*kmeta.NewControllerRef(tc.ing)} + } + for i, rule := range tc.ing.Spec.Rules { + if rule.Visibility == v1alpha1.IngressVisibilityExternalIP { + rule := rule + cfg := testConfig.DeepCopy() + tcs := &testConfigStore{config: cfg} + ctx := tcs.ToContext(context.Background()) + + route, err := MakeRedirectHTTPRoute(ctx, tc.ing, &rule) + if err != nil { + t.Fatal("MakeRedirectHTTPRoute failed:", err) + } + if diff := cmp.Diff(tc.expected[i], route); diff != "" { + t.Error("Unexpected redirect HTTPRoute (-want +got):", diff) + } + } + } + }) + } +} + type testConfigStore struct { config *config.Config } @@ -1198,11 +1740,13 @@ var testConfig = &config.Config{ ExternalGateways: []config.Gateway{{ NamespacedName: types.NamespacedName{Namespace: "test-ns", Name: "foo"}, Class: testGatewayClass, + HTTPListenerName: "http", SupportedFeatures: sets.New[features.SupportedFeature](), }}, LocalGateways: []config.Gateway{{ NamespacedName: types.NamespacedName{Namespace: "test-ns", Name: "foo-local"}, Class: testGatewayClass, + HTTPListenerName: "http", SupportedFeatures: sets.New[features.SupportedFeature](), }}, }, diff --git a/test/e2e/testdata/contour-no-service-vis.yaml b/test/e2e/testdata/contour-no-service-vis.yaml index 83879d565..a4a914aae 100644 --- a/test/e2e/testdata/contour-no-service-vis.yaml +++ b/test/e2e/testdata/contour-no-service-vis.yaml @@ -27,11 +27,13 @@ data: external-gateways: | - class: contour-external gateway: contour-external/knative-external + http-listener-name: http supported-features: - HTTPRouteRequestTimeout local-gateways: | - class: contour-internal gateway: contour-internal/knative-local service: contour-internal/envoy-knative-local + http-listener-name: http supported-features: - HTTPRouteRequestTimeout diff --git a/test/e2e/testdata/envoy-gateway-no-service-vis.yaml b/test/e2e/testdata/envoy-gateway-no-service-vis.yaml index 9c9c6b674..1b1b56f25 100644 --- a/test/e2e/testdata/envoy-gateway-no-service-vis.yaml +++ b/test/e2e/testdata/envoy-gateway-no-service-vis.yaml @@ -25,6 +25,7 @@ data: external-gateways: | - class: eg-external gateway: eg-external/eg-external + http-listener-name: http supported-features: - HTTPRouteRequestTimeout @@ -33,5 +34,6 @@ data: - class: eg-internal gateway: eg-internal/eg-internal service: envoy-gateway-system/knative-internal + http-listener-name: http supported-features: - HTTPRouteRequestTimeout diff --git a/test/e2e/testdata/istio-no-service-vis.yaml b/test/e2e/testdata/istio-no-service-vis.yaml index 60a08f930..e30bc0c6d 100644 --- a/test/e2e/testdata/istio-no-service-vis.yaml +++ b/test/e2e/testdata/istio-no-service-vis.yaml @@ -23,10 +23,12 @@ data: external-gateways: | - class: istio gateway: istio-system/knative-gateway + http-listener-name: http supported-features: - HTTPRouteRequestTimeout local-gateways: | - class: istio gateway: istio-system/knative-local-gateway + http-listener-name: http supported-features: - HTTPRouteRequestTimeout diff --git a/third_party/contour/config-gateway.yaml b/third_party/contour/config-gateway.yaml index 7c4343920..28a9a15f7 100644 --- a/third_party/contour/config-gateway.yaml +++ b/third_party/contour/config-gateway.yaml @@ -26,6 +26,7 @@ data: - class: contour-external gateway: contour-external/knative-external service: contour-external/envoy-knative-external + http-listener-name: http supported-features: - HTTPRouteRequestTimeout @@ -34,5 +35,6 @@ data: - class: contour-internal gateway: contour-internal/knative-local service: contour-internal/envoy-knative-local + http-listener-name: http supported-features: - HTTPRouteRequestTimeout diff --git a/third_party/envoy-gateway/config-gateway.yaml b/third_party/envoy-gateway/config-gateway.yaml index 98050a1c2..8b71705a6 100644 --- a/third_party/envoy-gateway/config-gateway.yaml +++ b/third_party/envoy-gateway/config-gateway.yaml @@ -26,6 +26,7 @@ data: - class: eg-external gateway: eg-external/eg-external service: envoy-gateway-system/knative-external + http-listener-name: http supported-features: - HTTPRouteRequestTimeout @@ -34,5 +35,6 @@ data: - class: eg-internal gateway: eg-internal/eg-internal service: envoy-gateway-system/knative-internal + http-listener-name: http supported-features: - HTTPRouteRequestTimeout diff --git a/third_party/istio/300-gateway-local.yaml b/third_party/istio/300-gateway-local.yaml index 0e8da1215..83f346a10 100644 --- a/third_party/istio/300-gateway-local.yaml +++ b/third_party/istio/300-gateway-local.yaml @@ -23,7 +23,7 @@ spec: - type: Hostname value: knative-local-gateway listeners: - - name: default + - name: http port: 80 protocol: HTTP allowedRoutes: diff --git a/third_party/istio/300-gateway.yaml b/third_party/istio/300-gateway.yaml index 6d0885ef6..7c1c55f7f 100644 --- a/third_party/istio/300-gateway.yaml +++ b/third_party/istio/300-gateway.yaml @@ -23,7 +23,7 @@ spec: - type: Hostname value: istio-ingressgateway listeners: - - name: default + - name: http port: 80 protocol: HTTP allowedRoutes: