Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow the config-gateway to accepted supported features #710

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/config-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 4 additions & 2 deletions hack/update-codegen.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions pkg/reconciler/ingress/config/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,4 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

// +k8s:deepcopy-gen=package

package config
46 changes: 35 additions & 11 deletions pkg/reconciler/ingress/config/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
),
}}
}

Expand All @@ -62,6 +64,9 @@ func defaultLocalGateways() []Gateway {
Name: "knative-local-gateway",
Namespace: "istio-system",
},
SupportedFeatures: sets.New(
features.SupportHTTPRouteRequestTimeout,
),
}}
}

Expand All @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
16 changes: 14 additions & 2 deletions pkg/reconciler/ingress/config/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
}, {
Expand Down
9 changes: 9 additions & 0 deletions pkg/reconciler/ingress/config/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 12 additions & 3 deletions pkg/reconciler/ingress/resources/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -225,8 +226,6 @@ func makeHTTPRouteSpec(
hostnames = append(hostnames, gatewayapi.Hostname(hostname))
}

rules := makeHTTPRouteRule(rule)

pluginConfig := config.FromContext(ctx).GatewayPlugin

var gateway config.Gateway
Expand All @@ -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")),
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
89 changes: 81 additions & 8 deletions pkg/reconciler/ingress/resources/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 (
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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](),
}},
},
}
Expand Down
Loading
Loading