Add support for HTTPOption

knative-extensions · Jan 18, 2023 · 73ce526 · 73ce526
1 parent 3090431
commit 73ce526
Show file tree

Hide file tree

Showing 10 changed files with 558 additions and 138 deletions.
diff --git a/config/config-gateway.yaml b/config/config-gateway.yaml
@@ -40,14 +40,17 @@ data:
     #     gatewayClass: GatewayClass Name
     #     gateway: the namespace/name of Gateway
     #     service: the namespace/name of Service for the Gateway
+    #     httpListenerName: the name of the listener for HTTP traffic in the Gateway (required)
     #
     # The gateway configuration for the default visibility.
     visibility: |
       ExternalIP:
         class: istio
         gateway: istio-system/knative-gateway
         service: istio-system/istio-ingressgateway
+        httpListenerName: default
       ClusterLocal:
         class: istio
         gateway: istio-system/knative-local-gateway
         service: istio-system/knative-local-gateway
+        httpListenerName: http2
diff --git a/hack/test-env.sh b/hack/test-env.sh
@@ -16,6 +16,6 @@
 
 export GATEWAY_API_VERSION="v0.6.0"
 export ISTIO_VERSION="1.16.1"
-export ISTIO_UNSUPPORTED_E2E_TESTS="retry,httpoption,host-rewrite"
+export ISTIO_UNSUPPORTED_E2E_TESTS="retry,host-rewrite"
 export CONTOUR_VERSION="v1.23.0"
-export CONTOUR_UNSUPPORTED_E2E_TESTS="httpoption,basics/http2,websocket,websocket/split,grpc,grpc/split,update,host-rewrite"
+export CONTOUR_UNSUPPORTED_E2E_TESTS="basics/http2,websocket,websocket/split,grpc,grpc/split,update,host-rewrite"
diff --git a/pkg/reconciler/ingress/config/gateway.go b/pkg/reconciler/ingress/config/gateway.go
@@ -37,6 +37,12 @@ const (
 
 	// defaultGatewayClass is the gatewayclass name for the gateway.
 	defaultGatewayClass = "istio"
+
+	// defaultClusterLocalHTTPListener is the name of the listener for HTTP traffic
+	defaultClusterLocalHTTPListener = "http2"
+
+	// defaultExternalIPHTTPListener
+	defaultExternalIPHTTPListener = "default"
 )
 
 var (
@@ -54,15 +60,17 @@ var (
 )
 
 type GatewayConfig struct {
-	GatewayClass string
-	Gateway      *types.NamespacedName
-	Service      *types.NamespacedName
+	GatewayClass     string
+	Gateway          *types.NamespacedName
+	Service          *types.NamespacedName
+	HTTPListenerName string
 }
 
 type visibilityValue struct {
-	GatewayClass string `json:"class,omitempty"`
-	Gateway      string `json:"gateway,omitempty"`
-	Service      string `json:"service,omitempty"`
+	GatewayClass     string `json:"class,omitempty"`
+	Gateway          string `json:"gateway,omitempty"`
+	Service          string `json:"service,omitempty"`
+	HTTPListenerName string `json:"httpListenerName,omitempty"`
 }
 
 // Gateway maps gateways to routes by matching the gateway's
@@ -82,8 +90,18 @@ func NewGatewayFromConfigMap(configMap *corev1.ConfigMap) (*Gateway, error) {
 		// These are the defaults.
 		return &Gateway{
 			Gateways: map[v1alpha1.IngressVisibility]GatewayConfig{
-				v1alpha1.IngressVisibilityExternalIP:   {GatewayClass: defaultGatewayClass, Gateway: defaultIstioGateway, Service: defaultGatewayService},
-				v1alpha1.IngressVisibilityClusterLocal: {GatewayClass: defaultGatewayClass, Gateway: defaultIstioLocalGateway, Service: defaultLocalGatewayService},
+				v1alpha1.IngressVisibilityExternalIP: {
+					GatewayClass:     defaultGatewayClass,
+					Gateway:          defaultIstioGateway,
+					Service:          defaultGatewayService,
+					HTTPListenerName: defaultExternalIPHTTPListener,
+				},
+				v1alpha1.IngressVisibilityClusterLocal: {
+					GatewayClass:     defaultGatewayClass,
+					Gateway:          defaultIstioLocalGateway,
+					Service:          defaultLocalGatewayService,
+					HTTPListenerName: defaultClusterLocalHTTPListener,
+				},
 			},
 		}, nil
 	}
@@ -121,10 +139,14 @@ func NewGatewayFromConfigMap(configMap *corev1.ConfigMap) (*Gateway, error) {
 		if err != nil {
 			return nil, fmt.Errorf("visibility %q failed to parse service: %w", key, err)
 		}
+		if value.HTTPListenerName == "" {
+			return nil, fmt.Errorf("visibility %q must set httpListenerName", key)
+		}
 		entry[key] = GatewayConfig{
-			GatewayClass: value.GatewayClass,
-			Gateway:      gateway,
-			Service:      service,
+			GatewayClass:     value.GatewayClass,
+			Gateway:          gateway,
+			Service:          service,
+			HTTPListenerName: value.HTTPListenerName,
 		}
 	}
 	return &Gateway{Gateways: entry}, nil

diff --git a/pkg/reconciler/ingress/config/testdata/config-gateway.yaml b/pkg/reconciler/ingress/config/testdata/config-gateway.yaml
@@ -35,14 +35,17 @@ data:
     #     gatewayClass: GatewayClass Name
     #     gateway: the namespace/name of Gateway
     #     service: the namespace/name of Service for the Gateway
+    #     httpListenerName: the name of the listener for HTTP traffic in the Gateway (required)
     #
     # The gateway configuration for the default visibility.
     visibility: |
       ExternalIP:
         class: istio
         gateway: istio-system/knative-gateway
         service: istio-system/istio-ingressgateway
+        httpListenerName: default
       ClusterLocal:
         class: istio
         gateway: istio-system/knative-local-gateway
         service: istio-system/knative-local-gateway
+        httpListenerName: http2
diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go
@@ -98,12 +98,20 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
 	for _, rule := range ing.Spec.Rules {
 		rule := rule
 
-		httproutes, err := c.reconcileHTTPRoute(ctx, ing, &rule)
+		workloadHTTPRoute, err := c.reconcileWorkloadRoute(ctx, ing, &rule)
 		if err != nil {
 			return err
 		}
 
-		if isHTTPRouteReady(httproutes) {
+		var redirectHTTPRoute *gatewayapi.HTTPRoute
+		if ing.Spec.HTTPOption == v1alpha1.HTTPOptionRedirected {
+			redirectHTTPRoute, err = c.reconcileRedirectHTTPRoute(ctx, ing, &rule)
+			if err != nil {
+				return err
+			}
+		}
+
+		if isHTTPRouteReady(workloadHTTPRoute) && (redirectHTTPRoute == nil || isHTTPRouteReady(redirectHTTPRoute)) {
 			ing.Status.MarkNetworkConfigured()
 		} else {
 			ing.Status.MarkIngressNotReady("HTTPRouteNotReady", "Waiting for HTTPRoute becomes Ready.")

diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go
@@ -56,6 +56,13 @@ var (
 	privateSvcIP = "5.6.7.8"
 	publicSvc    = network.GetServiceHostname(publicName, testNamespace)
 	privateSvc   = network.GetServiceHostname(privateName, testNamespace)
+
+	gatewayRef = gatewayapi.ParentReference{
+		Group:     (*gatewayapi.Group)(pointer.String("gateway.networking.k8s.io")),
+		Kind:      (*gatewayapi.Kind)(pointer.String("Gateway")),
+		Namespace: (*gatewayapi.Namespace)(pointer.String("istio-system")),
+		Name:      gatewayapi.ObjectName("istio-gateway"),
+	}
 )
 
 var (
@@ -149,19 +156,19 @@ func TestReconcile(t *testing.T) {
 		Name: "skip ingress marked for deletion",
 		Key:  "ns/name",
 		Objects: []runtime.Object{
-			ing(withBasicSpec, withGatewayAPIclass, func(i *v1alpha1.Ingress) {
+			ing(withBasicSpec, withGatewayAPIClass, func(i *v1alpha1.Ingress) {
 				i.SetDeletionTimestamp(&metav1.Time{Time: time.Now()})
 			}),
 		},
 	}, {
 		Name: "first reconcile basic ingress",
 		Key:  "ns/name",
 		Objects: append([]runtime.Object{
-			ing(withBasicSpec, withGatewayAPIclass),
+			ing(withBasicSpec, withGatewayAPIClass),
 		}, servicesAndEndpoints...),
-		WantCreates: []runtime.Object{httpRoute(t, ing(withBasicSpec, withGatewayAPIclass))},
+		WantCreates: []runtime.Object{httpRoute(t, ing(withBasicSpec, withGatewayAPIClass))},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: ing(withBasicSpec, withGatewayAPIclass, func(i *v1alpha1.Ingress) {
+			Object: ing(withBasicSpec, withGatewayAPIClass, func(i *v1alpha1.Ingress) {
 				// These are the things we expect to change in status.
 				i.Status.InitializeConditions()
 				i.Status.MarkLoadBalancerReady(
@@ -188,8 +195,8 @@ func TestReconcile(t *testing.T) {
 		Name: "reconcile ready ingress",
 		Key:  "ns/name",
 		Objects: append([]runtime.Object{
-			ing(withBasicSpec, withGatewayAPIclass, makeItReady, withFinalizer),
-			httpRoute(t, ing(withBasicSpec, withGatewayAPIclass)),
+			ing(withBasicSpec, withGatewayAPIClass, makeItReady, withFinalizer),
+			httpRoute(t, ing(withBasicSpec, withGatewayAPIClass)),
 		}, servicesAndEndpoints...),
 		// no extra update
 	}}
@@ -223,11 +230,11 @@ func TestReconcileProberNotReady(t *testing.T) {
 		Name: "first reconcile basic ingress wth prober",
 		Key:  "ns/name",
 		Objects: append([]runtime.Object{
-			ing(withBasicSpec, withGatewayAPIclass),
+			ing(withBasicSpec, withGatewayAPIClass),
 		}, servicesAndEndpoints...),
-		WantCreates: []runtime.Object{httpRoute(t, ing(withBasicSpec, withGatewayAPIclass))},
+		WantCreates: []runtime.Object{httpRoute(t, ing(withBasicSpec, withGatewayAPIClass))},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: ing(withBasicSpec, withGatewayAPIclass, func(i *v1alpha1.Ingress) {
+			Object: ing(withBasicSpec, withGatewayAPIClass, func(i *v1alpha1.Ingress) {
 				i.Status.InitializeConditions()
 				i.Status.MarkLoadBalancerNotReady()
 			}),
@@ -389,6 +396,47 @@ func TestReconcileTLS(t *testing.T) {
 			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`),
 		},
+	}, {
+		Name: "TLS ingress with httpOption redirected",
+		Key:  "ns/name",
+		Objects: append([]runtime.Object{
+			ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS(secretName)),
+			secret(secretName, nsName),
+			gw(defaultListener),
+		}, servicesAndEndpoints...),
+		WantCreates: []runtime.Object{
+			httpRoute(t, ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS(secretName)), withSectionName("kni-")),
+			httpRedirectRoute(t, ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS(secretName)), withSectionName("http")),
+			rp(secret(secretName, nsName)),
+		},
+		WantUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: gw(defaultListener, tlsListener("secure.example.com", nsName, secretName)),
+		}},
+		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: ing(withBasicSpec, withGatewayAPIClass, withHTTPOptionRedirected, withTLS(secretName), func(i *v1alpha1.Ingress) {
+				// These are the things we expect to change in status.
+				i.Status.InitializeConditions()
+				i.Status.MarkLoadBalancerReady(
+					[]v1alpha1.LoadBalancerIngressStatus{{
+						DomainInternal: publicSvc,
+					}},
+					[]v1alpha1.LoadBalancerIngressStatus{{
+						DomainInternal: privateSvc,
+					}})
+			}),
+		}},
+		WantPatches: []clientgotesting.PatchActionImpl{{
+			ActionImpl: clientgotesting.ActionImpl{
+				Namespace: "ns",
+			},
+			Name:  "name",
+			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\""),
+			Eventf(corev1.EventTypeNormal, "Created", "Created HTTPRoute \"example.com-redirect\""),
+		},
 	}}
 
 	table.Test(t, GatewayFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher, tr *TableRow) controller.Reconciler {
@@ -433,11 +481,11 @@ func TestReconcileProbeError(t *testing.T) {
 		Key:     "ns/name",
 		WantErr: true,
 		Objects: append([]runtime.Object{
-			ing(withBasicSpec, withGatewayAPIclass),
+			ing(withBasicSpec, withGatewayAPIClass),
 		}, servicesAndEndpoints...),
-		WantCreates: []runtime.Object{httpRoute(t, ing(withBasicSpec, withGatewayAPIclass))},
+		WantCreates: []runtime.Object{httpRoute(t, ing(withBasicSpec, withGatewayAPIClass))},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: ing(withBasicSpec, withGatewayAPIclass, func(i *v1alpha1.Ingress) {
+			Object: ing(withBasicSpec, withGatewayAPIClass, func(i *v1alpha1.Ingress) {
 				i.Status.InitializeConditions()
 				i.Status.MarkIngressNotReady(notReconciledReason, notReconciledMessage)
 			}),
@@ -493,20 +541,31 @@ func makeItReady(i *v1alpha1.Ingress) {
 func httpRoute(t *testing.T, i *v1alpha1.Ingress, opts ...HTTPRouteOption) runtime.Object {
 	t.Helper()
 	ingress.InsertProbe(i)
-	ctx := (&testConfigStore{config: defaultConfig}).ToContext(context.Background())
-	httpRoute, _ := resources.MakeHTTPRoute(ctx, i, &i.Spec.Rules[0])
+
+	httpRoute, _ := resources.MakeHTTPRoute(i, &i.Spec.Rules[0], gatewayRef)
 	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)
+
+	httpRedirectRoute, _ := resources.MakeRedirectHTTPRoute(i, &i.Spec.Rules[0], gatewayRef)
+	for _, opt := range opts {
+		opt(httpRedirectRoute)
+	}
+	return httpRedirectRoute
+}
+
 type HTTPRouteOption func(h *gatewayapi.HTTPRoute)
 
-func withGatewayAPIclass(i *v1alpha1.Ingress) {
-	withAnnotation(map[string]string{
-		networking.IngressClassAnnotationKey: gatewayAPIIngressClassName,
-	})(i)
+func withSectionName(sectionName string) HTTPRouteOption {
+	return func(h *gatewayapi.HTTPRoute) {
+		h.Spec.CommonRouteSpec.ParentRefs[0].SectionName = (*gatewayapi.SectionName)(pointer.String(sectionName))
+	}
 }
 
 type fakeStatusManager struct {
@@ -566,7 +625,7 @@ func defaultListener(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",
@@ -644,8 +703,8 @@ func rp(to *corev1.Secret) *gatewayapialpha.ReferenceGrant {
 				Namespace: gatewayapialpha.Namespace(testNamespace),
 			}},
 			To: []gatewayapialpha.ReferenceGrantTo{{
-				Group: gatewayapialpha.Group(""),
-				Kind:  gatewayapialpha.Kind("Secret"),
+				Group: "",
+				Kind:  "Secret",
 				Name:  (*gatewayapialpha.ObjectName)(&to.Name),
 			}},
 		},
@@ -658,12 +717,14 @@ var (
 		Gateway: &config.Gateway{
 			Gateways: map[v1alpha1.IngressVisibility]config.GatewayConfig{
 				v1alpha1.IngressVisibilityExternalIP: {
-					Service: &types.NamespacedName{Namespace: "istio-system", Name: "istio-gateway"},
-					Gateway: &types.NamespacedName{Namespace: "istio-system", Name: "istio-gateway"},
+					Service:          &types.NamespacedName{Namespace: "istio-system", Name: "istio-gateway"},
+					Gateway:          &types.NamespacedName{Namespace: "istio-system", Name: "istio-gateway"},
+					HTTPListenerName: "http",
 				},
 				v1alpha1.IngressVisibilityClusterLocal: {
-					Service: &types.NamespacedName{Namespace: "istio-system", Name: "knative-local-gateway"},
-					Gateway: &types.NamespacedName{Namespace: "istio-system", Name: "knative-local-gateway"},
+					Service:          &types.NamespacedName{Namespace: "istio-system", Name: "knative-local-gateway"},
+					Gateway:          &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
@@ -313,6 +313,10 @@ func withBasicSpec(i *v1alpha1.Ingress) {
 	})
 }
 
+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"},