allow the config-gateway to accepted supported features (#710)

This allows us to use experimental features safely

config/config-gateway.yaml

@@ -50,9 +50,13 @@ data:
       - class: istio
         gateway: istio-system/knative-gateway
         service: istio-system/istio-ingressgateway
+        supported-features:
+        - HTTPRouteRequestTimeout
 
     # local-gateways defines the Gateway to be used for cluster local traffic
     local-gateways: |
       - class: istio
         gateway: istio-system/knative-local-gateway
         service: istio-system/knative-local-gateway
+        supported-features:
+        - HTTPRouteRequestTimeout

hack/update-codegen.sh

@@ -36,14 +36,16 @@ EXTERNAL_INFORMER_PKG="sigs.k8s.io/gateway-api/pkg/client/informers/externalvers
   "apis:v1beta1,v1" \
   --go-header-file "${boilerplate}"
 
+# Deepcopy is broken for fields that use generics - so we generate the code
+# ignore failures and then clean it up ourselves with sed until k8s upstream
+# fixes the issue
 group "Deepcopy Gen"
 go run k8s.io/code-generator/cmd/deepcopy-gen \
   -O zz_generated.deepcopy \
   --go-header-file "${boilerplate}" \
   --input-dirs knative.dev/net-gateway-api/pkg/reconciler/ingress/config
 
-group "Update deps post-codegen"
-
+# group "Update deps post-codegen"
 # Make sure our dependencies are up-to-date
 "${REPO_ROOT_DIR}"/hack/update-deps.sh
 group "Update tested version docs"

pkg/reconciler/ingress/config/doc.go

@@ -14,6 +14,4 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// +k8s:deepcopy-gen=package
-
 package config

pkg/reconciler/ingress/config/gateway.go

@@ -22,12 +22,12 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"knative.dev/pkg/configmap"
+	"sigs.k8s.io/gateway-api/pkg/features"
 	"sigs.k8s.io/yaml"
 )
 
-// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
-
 const (
 	// GatewayConfigName is the config map name for the gateway configuration.
 	GatewayConfigName = "config-gateway"
@@ -42,12 +42,14 @@ func defaultExternalGateways() []Gateway {
 			Name:      "knative-gateway",
 			Namespace: "istio-system",
 		},
-
 		Class: "istio",
 		Service: &types.NamespacedName{
 			Name:      "istio-ingressgateway",
 			Namespace: "istio-system",
 		},
+		SupportedFeatures: sets.New(
+			features.SupportHTTPRouteRequestTimeout,
+		),
 	}}
 }
 
@@ -62,6 +64,9 @@ func defaultLocalGateways() []Gateway {
 			Name:      "knative-local-gateway",
 			Namespace: "istio-system",
 		},
+		SupportedFeatures: sets.New(
+			features.SupportHTTPRouteRequestTimeout,
+		),
 	}}
 }
 
@@ -79,11 +84,14 @@ func (g *GatewayPlugin) LocalGateway() Gateway {
 	return g.LocalGateways[0]
 }
 
+// Note deepcopy gen is broken for sets.Set[features.SupportedFeatures]
+// So I've disabled the generator in this package for now
 type Gateway struct {
 	types.NamespacedName
 
-	Class   string
-	Service *types.NamespacedName
+	Class             string
+	Service           *types.NamespacedName
+	SupportedFeatures sets.Set[features.SupportedFeature]
 }
 
 // FromConfigMap creates a GatewayPlugin config from the supplied ConfigMap
@@ -126,27 +134,43 @@ func FromConfigMap(cm *corev1.ConfigMap) (*GatewayPlugin, error) {
 	return config, nil
 }
 
+type gatewayEntry struct {
+	Gateway           string                      `json:"gateway"`
+	Service           *string                     `json:"service"`
+	Class             string                      `json:"class"`
+	SupportedFeatures []features.SupportedFeature `json:"supported-features"`
+}
+
 func parseGatewayConfig(data string) ([]Gateway, error) {
-	var entries []map[string]string
+	var entries []gatewayEntry
 
 	if err := yaml.Unmarshal([]byte(data), &entries); err != nil {
 		return nil, err
 	}
 
 	gws := make([]Gateway, 0, len(entries))
-
 	for i, entry := range entries {
-		gw := Gateway{}
+		gw := Gateway{
+			Class:             entry.Class,
+			SupportedFeatures: sets.New(entry.SupportedFeatures...),
+		}
 
-		err := configmap.Parse(entry,
-			configmap.AsString("class", &gw.Class),
+		names := map[string]string{
+			"gateway": entry.Gateway,
+		}
+
+		if entry.Service != nil {
+			names["service"] = *entry.Service
+		}
+
+		err := configmap.Parse(names,
 			configmap.AsNamespacedName("gateway", &gw.NamespacedName),
 			configmap.AsOptionalNamespacedName("service", &gw.Service),
 		)
+
 		if err != nil {
 			return nil, err
 		}
-
 		if len(strings.TrimSpace(gw.Class)) == 0 {
 			return nil, fmt.Errorf(`entry [%d] field "class" is required`, i)
 		}

pkg/reconciler/ingress/config/gateway_test.go

@@ -56,13 +56,25 @@ func TestFromConfigMapErrors(t *testing.T) {
 	}, {
 		name: "external-gateways multiple entries",
 		data: map[string]string{
-			"external-gateways": `[{"class":"boo"},{"class":"boo"}]`,
+			"external-gateways": `[{
+					"class":"boo",
+					"gateway": "ns/n"
+				},{
+					"class":"boo",
+					"gateway": "ns/n"
+				}]`,
 		},
 		want: `only a single external gateway is supported`,
 	}, {
 		name: "local-gateways multiple entries",
 		data: map[string]string{
-			"local-gateways": `[{"class":"boo"},{"class":"boo"}]`,
+			"local-gateways": `[{
+					"class":"boo",
+					"gateway": "ns/n"
+				},{
+					"class":"boo",
+					"gateway": "ns/n"
+				}]`,
 		},
 		want: `only a single local gateway is supported`,
 	}, {

pkg/reconciler/ingress/resources/httproute.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 	gatewayapi "sigs.k8s.io/gateway-api/apis/v1"
+	"sigs.k8s.io/gateway-api/pkg/features"
 
 	"knative.dev/net-gateway-api/pkg/reconciler/ingress/config"
 	"knative.dev/networking/pkg/apis/networking"
@@ -225,8 +226,6 @@ func makeHTTPRouteSpec(
 		hostnames = append(hostnames, gatewayapi.Hostname(hostname))
 	}
 
-	rules := makeHTTPRouteRule(rule)
-
 	pluginConfig := config.FromContext(ctx).GatewayPlugin
 
 	var gateway config.Gateway
@@ -237,6 +236,8 @@ func makeHTTPRouteSpec(
 		gateway = pluginConfig.ExternalGateway()
 	}
 
+	rules := makeHTTPRouteRule(gateway, rule)
+
 	gatewayRef := gatewayapi.ParentReference{
 		Group:     (*gatewayapi.Group)(&gatewayapi.GroupVersion.Group),
 		Kind:      (*gatewayapi.Kind)(ptr.To("Gateway")),
@@ -253,10 +254,11 @@ func makeHTTPRouteSpec(
 	}
 }
 
-func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayapi.HTTPRouteRule {
+func makeHTTPRouteRule(gw config.Gateway, rule *netv1alpha1.IngressRule) []gatewayapi.HTTPRouteRule {
 	rules := []gatewayapi.HTTPRouteRule{}
 
 	for _, path := range rule.HTTP.Paths {
+		path := path
 		backendRefs := make([]gatewayapi.HTTPBackendRef, 0, len(path.Splits))
 		var preFilters []gatewayapi.HTTPRouteFilter
 
@@ -351,6 +353,13 @@ func makeHTTPRouteRule(rule *netv1alpha1.IngressRule) []gatewayapi.HTTPRouteRule
 			Filters:     preFilters,
 			Matches:     matches,
 		}
+
+		if gw.SupportedFeatures.Has(features.SupportHTTPRouteRequestTimeout) {
+			rule.Timeouts = &gatewayapi.HTTPRouteTimeouts{
+				Request: ptr.To[gatewayapi.Duration]("0s"),
+			}
+		}
+
 		rules = append(rules, rule)
 	}
 	return rules

pkg/reconciler/ingress/resources/httproute_test.go

@@ -18,12 +18,14 @@ package resources
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/utils/ptr"
 	"knative.dev/net-gateway-api/pkg/reconciler/ingress/config"
 	"knative.dev/networking/pkg/apis/networking"
@@ -32,6 +34,7 @@ import (
 	"knative.dev/pkg/kmeta"
 	"knative.dev/pkg/reconciler"
 	gatewayapi "sigs.k8s.io/gateway-api/apis/v1"
+	"sigs.k8s.io/gateway-api/pkg/features"
 )
 
 const (
@@ -102,9 +105,10 @@ var (
 
 func TestMakeHTTPRoute(t *testing.T) {
 	for _, tc := range []struct {
-		name     string
-		ing      *v1alpha1.Ingress
-		expected []*gatewayapi.HTTPRoute
+		name         string
+		ing          *v1alpha1.Ingress
+		expected     []*gatewayapi.HTTPRoute
+		changeConfig func(gw *config.Config)
 	}{
 		{
 			name: "single external domain with split and cluster local",
@@ -535,11 +539,78 @@ func TestMakeHTTPRoute(t *testing.T) {
 					},
 				},
 			}},
+		}, {
+			name: "gateway supports HTTPRouteRequestTimeout",
+			changeConfig: func(c *config.Config) {
+				gateways := c.GatewayPlugin.ExternalGateways
+
+				for _, gateway := range gateways {
+					gateway.SupportedFeatures.Insert(features.SupportHTTPRouteRequestTimeout)
+				}
+			},
+			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{{
+							Path: "/",
+						}},
+					},
+				}}},
+			},
+			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{{
+						Timeouts: &gatewayapi.HTTPRouteTimeouts{
+							Request: ptr.To[gatewayapi.Duration]("0s"),
+						},
+						BackendRefs: []gatewayapi.HTTPBackendRef{},
+						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"),
+						}},
+					},
+				},
+			}},
 		}} {
 		t.Run(tc.name, func(t *testing.T) {
 			for i, rule := range tc.ing.Spec.Rules {
 				rule := rule
-				tcs := &testConfigStore{config: testConfig}
+				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)
@@ -1125,12 +1196,14 @@ func (t *testConfigStore) ToContext(ctx context.Context) context.Context {
 var testConfig = &config.Config{
 	GatewayPlugin: &config.GatewayPlugin{
 		ExternalGateways: []config.Gateway{{
-			NamespacedName: types.NamespacedName{Namespace: "test-ns", Name: "foo"},
-			Class:          testGatewayClass,
+			NamespacedName:    types.NamespacedName{Namespace: "test-ns", Name: "foo"},
+			Class:             testGatewayClass,
+			SupportedFeatures: sets.New[features.SupportedFeature](),
 		}},
 		LocalGateways: []config.Gateway{{
-			NamespacedName: types.NamespacedName{Namespace: "test-ns", Name: "foo-local"},
-			Class:          testGatewayClass,
+			NamespacedName:    types.NamespacedName{Namespace: "test-ns", Name: "foo-local"},
+			Class:             testGatewayClass,
+			SupportedFeatures: sets.New[features.SupportedFeature](),
 		}},
 	},
 }