From fa9c54f7f71ba663d77817974042e1a5e0d40ff7 Mon Sep 17 00:00:00 2001 From: Lin Yang Date: Mon, 16 Sep 2024 12:34:32 +0800 Subject: [PATCH] fix: resolve filter definition ref Signed-off-by: Lin Yang --- .../extension.gateway.flomesh.io_filters.yaml | 9 +++++++- ...on.gateway.flomesh.io_listenerfilters.yaml | 9 +++++++- pkg/apis/extension/v1alpha1/filter.go | 9 +++++++- pkg/apis/extension/v1alpha1/listenerfilter.go | 9 +++++++- .../v1alpha1/zz_generated.deepcopy.go | 12 ++++++++-- pkg/gateway/processor/v2/filters.go | 19 +++++++++++++++- pkg/gateway/processor/v2/gateway.go | 22 +++++++------------ pkg/gateway/processor/v2/grpcroute.go | 22 +++++++++---------- pkg/gateway/processor/v2/httproute.go | 22 +++++++++---------- pkg/webhook/extension/v1alpha1/filter.go | 14 +++++++----- .../extension/v1alpha1/listenerfilter.go | 14 +++++++----- 11 files changed, 104 insertions(+), 57 deletions(-) diff --git a/cmd/fsm-bootstrap/crds/extension.gateway.flomesh.io_filters.yaml b/cmd/fsm-bootstrap/crds/extension.gateway.flomesh.io_filters.yaml index ba50655d4..072bdb505 100644 --- a/cmd/fsm-bootstrap/crds/extension.gateway.flomesh.io_filters.yaml +++ b/cmd/fsm-bootstrap/crds/extension.gateway.flomesh.io_filters.yaml @@ -102,8 +102,15 @@ spec: - kind - name type: object + type: + description: Type is the type of the Filter in PascalCase, it should + be unique within the namespace + maxLength: 63 + minLength: 1 + pattern: ^[A-Z](([a-z0-9]+[A-Z]?)*)$ + type: string required: - - definitionRef + - type type: object status: description: Status defines the current state of Filter. diff --git a/cmd/fsm-bootstrap/crds/extension.gateway.flomesh.io_listenerfilters.yaml b/cmd/fsm-bootstrap/crds/extension.gateway.flomesh.io_listenerfilters.yaml index 01052a2a6..cdfd1c20e 100644 --- a/cmd/fsm-bootstrap/crds/extension.gateway.flomesh.io_listenerfilters.yaml +++ b/cmd/fsm-bootstrap/crds/extension.gateway.flomesh.io_listenerfilters.yaml @@ -138,9 +138,16 @@ spec: maxItems: 16 minItems: 1 type: array + type: + description: Type is the type of the ListenerFilter in PascalCase, + it should be unique within the namespace + maxLength: 63 + minLength: 1 + pattern: ^[A-Z](([a-z0-9]+[A-Z]?)*)$ + type: string required: - - definitionRef - targetRefs + - type type: object status: description: Status defines the current state of ListenerFilter. diff --git a/pkg/apis/extension/v1alpha1/filter.go b/pkg/apis/extension/v1alpha1/filter.go index 48b65390b..37cdbd059 100644 --- a/pkg/apis/extension/v1alpha1/filter.go +++ b/pkg/apis/extension/v1alpha1/filter.go @@ -7,8 +7,15 @@ import ( // FilterSpec defines the desired state of Filter type FilterSpec struct { + // Type is the type of the Filter in PascalCase, it should be unique within the namespace + // +kubebuilder:validation:Pattern=`^[A-Z](([a-z0-9]+[A-Z]?)*)$` + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 + Type string `json:"type"` + + // +optional // DefinitionRef is the reference to the FilterDefinition - DefinitionRef gwv1.LocalObjectReference `json:"definitionRef"` + DefinitionRef *gwv1.LocalObjectReference `json:"definitionRef"` // +optional // ConfigRef is the reference to the Configurations diff --git a/pkg/apis/extension/v1alpha1/listenerfilter.go b/pkg/apis/extension/v1alpha1/listenerfilter.go index 55a16efe1..b25b090ca 100644 --- a/pkg/apis/extension/v1alpha1/listenerfilter.go +++ b/pkg/apis/extension/v1alpha1/listenerfilter.go @@ -7,13 +7,20 @@ import ( // ListenerFilterSpec defines the desired state of ListenerFilter type ListenerFilterSpec struct { + // Type is the type of the ListenerFilter in PascalCase, it should be unique within the namespace + // +kubebuilder:validation:Pattern=`^[A-Z](([a-z0-9]+[A-Z]?)*)$` + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 + Type string `json:"type"` + // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=16 // TargetRefs is the references to the target resources to which the ListenerFilter is applied TargetRefs []LocalTargetReferenceWithPort `json:"targetRefs"` + // +optional // DefinitionRef is the reference to the FilterDefinition - DefinitionRef gwv1.LocalObjectReference `json:"definitionRef"` + DefinitionRef *gwv1.LocalObjectReference `json:"definitionRef"` // +optional // ConfigRef is the reference to the Configurations diff --git a/pkg/apis/extension/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/extension/v1alpha1/zz_generated.deepcopy.go index c1cd2b49b..8330e9815 100644 --- a/pkg/apis/extension/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/extension/v1alpha1/zz_generated.deepcopy.go @@ -556,7 +556,11 @@ func (in *FilterList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FilterSpec) DeepCopyInto(out *FilterSpec) { *out = *in - out.DefinitionRef = in.DefinitionRef + if in.DefinitionRef != nil { + in, out := &in.DefinitionRef, &out.DefinitionRef + *out = new(apisv1.LocalObjectReference) + **out = **in + } if in.ConfigRef != nil { in, out := &in.ConfigRef, &out.ConfigRef *out = new(apisv1.LocalObjectReference) @@ -667,7 +671,11 @@ func (in *ListenerFilterSpec) DeepCopyInto(out *ListenerFilterSpec) { *out = make([]LocalTargetReferenceWithPort, len(*in)) copy(*out, *in) } - out.DefinitionRef = in.DefinitionRef + if in.DefinitionRef != nil { + in, out := &in.DefinitionRef, &out.DefinitionRef + *out = new(apisv1.LocalObjectReference) + **out = **in + } if in.ConfigRef != nil { in, out := &in.ConfigRef, &out.ConfigRef *out = new(apisv1.LocalObjectReference) diff --git a/pkg/gateway/processor/v2/filters.go b/pkg/gateway/processor/v2/filters.go index a1a4b3d59..2dab59507 100644 --- a/pkg/gateway/processor/v2/filters.go +++ b/pkg/gateway/processor/v2/filters.go @@ -3,6 +3,8 @@ package v2 import ( "context" + "k8s.io/utils/ptr" + fgwv2 "github.com/flomesh-io/fsm/pkg/gateway/fgw" gwutils "github.com/flomesh-io/fsm/pkg/gateway/utils" @@ -15,13 +17,28 @@ import ( "github.com/flomesh-io/fsm/pkg/constants" ) -func (c *ConfigGenerator) resolveFilterDefinition(ref gwv1.LocalObjectReference) *extv1alpha1.FilterDefinition { +func (c *ConfigGenerator) resolveFilterDefinition(filterType string, filterScope extv1alpha1.FilterScope, ref *gwv1.LocalObjectReference) *extv1alpha1.FilterDefinition { + if ref == nil { + return nil + } + definition := &extv1alpha1.FilterDefinition{} if err := c.client.Get(context.Background(), types.NamespacedName{Name: string(ref.Name)}, definition); err != nil { log.Error().Msgf("Failed to resolve FilterDefinition: %s", err) return nil } + if filterType != definition.Spec.Type { + log.Error().Msgf("FilterDefinition %s is not of type %s", definition.Name, filterType) + return nil + } + + definitionScope := ptr.Deref(definition.Spec.Scope, extv1alpha1.FilterScopeRoute) + if filterScope != definitionScope { + log.Error().Msgf("FilterDefinition %s is not of scope %s", definition.Name, filterScope) + return nil + } + return definition } diff --git a/pkg/gateway/processor/v2/gateway.go b/pkg/gateway/processor/v2/gateway.go index 8ec1e8040..38c22bcd3 100644 --- a/pkg/gateway/processor/v2/gateway.go +++ b/pkg/gateway/processor/v2/gateway.go @@ -158,13 +158,15 @@ func (c *ConfigGenerator) processListenerFilters(l gwtypes.Listener, v2l *fgwv2. v2l.Filters = make([]fgwv2.ListenerFilter, 0) for _, f := range list.Items { - definition := c.resolveFilterDefinition(f.Spec.DefinitionRef) - if definition == nil { - continue - } + filterType := f.Spec.Type + v2l.Filters = append(v2l.Filters, fgwv2.ListenerFilter{ + Type: filterType, + ExtensionConfig: c.resolveFilterConfig(f.Spec.ConfigRef), + Key: uuid.NewString(), + }) - scope := ptr.Deref(definition.Spec.Scope, extv1alpha1.FilterScopeRoute) - if scope != extv1alpha1.FilterScopeListener { + definition := c.resolveFilterDefinition(filterType, extv1alpha1.FilterScopeListener, f.Spec.DefinitionRef) + if definition == nil { continue } @@ -179,14 +181,6 @@ func (c *ConfigGenerator) processListenerFilters(l gwtypes.Listener, v2l *fgwv2. // continue //} - filterType := definition.Spec.Type - - v2l.Filters = append(v2l.Filters, fgwv2.ListenerFilter{ - Type: filterType, - ExtensionConfig: c.resolveFilterConfig(f.Spec.ConfigRef), - Key: uuid.NewString(), - }) - if c.filters[filterProtocol] == nil { c.filters[filterProtocol] = map[string]string{} } diff --git a/pkg/gateway/processor/v2/grpcroute.go b/pkg/gateway/processor/v2/grpcroute.go index b93a5d09a..aa9a82515 100644 --- a/pkg/gateway/processor/v2/grpcroute.go +++ b/pkg/gateway/processor/v2/grpcroute.go @@ -149,14 +149,19 @@ func (c *ConfigGenerator) toV2GRPCRouteFilters(grpcRoute *gwv1.GRPCRoute, routeF } case gwv1.GRPCRouteFilterExtensionRef: filter := gwutils.ExtensionRefToFilter(c.client, grpcRoute, f.ExtensionRef) - - definition := c.resolveFilterDefinition(filter.Spec.DefinitionRef) - if definition == nil { + if filter == nil { continue } - scope := ptr.Deref(definition.Spec.Scope, extv1alpha1.FilterScopeRoute) - if scope != extv1alpha1.FilterScopeRoute { + filterType := filter.Spec.Type + filters = append(filters, fgwv2.GRPCRouteFilter{ + Type: gwv1.GRPCRouteFilterType(filterType), + ExtensionConfig: c.resolveFilterConfig(filter.Spec.ConfigRef), + Key: uuid.NewString(), + }) + + definition := c.resolveFilterDefinition(filterType, extv1alpha1.FilterScopeRoute, filter.Spec.DefinitionRef) + if definition == nil { continue } @@ -165,13 +170,6 @@ func (c *ConfigGenerator) toV2GRPCRouteFilters(grpcRoute *gwv1.GRPCRoute, routeF continue } - filterType := definition.Spec.Type - filters = append(filters, fgwv2.GRPCRouteFilter{ - Type: gwv1.GRPCRouteFilterType(filterType), - ExtensionConfig: c.resolveFilterConfig(filter.Spec.ConfigRef), - Key: uuid.NewString(), - }) - if c.filters[filterProtocol] == nil { c.filters[filterProtocol] = map[string]string{} } diff --git a/pkg/gateway/processor/v2/httproute.go b/pkg/gateway/processor/v2/httproute.go index 4ede09aa1..457fb5a24 100644 --- a/pkg/gateway/processor/v2/httproute.go +++ b/pkg/gateway/processor/v2/httproute.go @@ -148,14 +148,19 @@ func (c *ConfigGenerator) toV2HTTPRouteFilters(httpRoute *gwv1.HTTPRoute, routeF } case gwv1.HTTPRouteFilterExtensionRef: filter := gwutils.ExtensionRefToFilter(c.client, httpRoute, f.ExtensionRef) - - definition := c.resolveFilterDefinition(filter.Spec.DefinitionRef) - if definition == nil { + if filter == nil { continue } - scope := ptr.Deref(definition.Spec.Scope, extv1alpha1.FilterScopeRoute) - if scope != extv1alpha1.FilterScopeRoute { + filterType := filter.Spec.Type + filters = append(filters, fgwv2.HTTPRouteFilter{ + Type: gwv1.HTTPRouteFilterType(filterType), + ExtensionConfig: c.resolveFilterConfig(filter.Spec.ConfigRef), + Key: uuid.NewString(), + }) + + definition := c.resolveFilterDefinition(filterType, extv1alpha1.FilterScopeRoute, filter.Spec.DefinitionRef) + if definition == nil { continue } @@ -164,13 +169,6 @@ func (c *ConfigGenerator) toV2HTTPRouteFilters(httpRoute *gwv1.HTTPRoute, routeF continue } - filterType := definition.Spec.Type - filters = append(filters, fgwv2.HTTPRouteFilter{ - Type: gwv1.HTTPRouteFilterType(filterType), - ExtensionConfig: c.resolveFilterConfig(filter.Spec.ConfigRef), - Key: uuid.NewString(), - }) - if c.filters[filterProtocol] == nil { c.filters[filterProtocol] = map[string]string{} } diff --git a/pkg/webhook/extension/v1alpha1/filter.go b/pkg/webhook/extension/v1alpha1/filter.go index 74ef3e18d..bd20e7527 100644 --- a/pkg/webhook/extension/v1alpha1/filter.go +++ b/pkg/webhook/extension/v1alpha1/filter.go @@ -87,15 +87,17 @@ func (r *FilterWebhook) validateSpec(ctx context.Context, spec extv1alpha1.Filte return errs } -func (r *FilterWebhook) validateDefinitionRef(ctx context.Context, definitionRef gwv1.LocalObjectReference, path *field.Path) field.ErrorList { +func (r *FilterWebhook) validateDefinitionRef(ctx context.Context, definitionRef *gwv1.LocalObjectReference, path *field.Path) field.ErrorList { var errs field.ErrorList - if definitionRef.Group != extv1alpha1.GroupName { - errs = append(errs, field.Invalid(path.Child("group"), definitionRef.Group, fmt.Sprintf("group must be %s", extv1alpha1.GroupName))) - } + if definitionRef != nil { + if definitionRef.Group != extv1alpha1.GroupName { + errs = append(errs, field.Invalid(path.Child("group"), definitionRef.Group, fmt.Sprintf("group must be %s", extv1alpha1.GroupName))) + } - if definitionRef.Kind != constants.GatewayAPIExtensionFilterDefinitionKind { - errs = append(errs, field.Invalid(path.Child("kind"), definitionRef.Kind, fmt.Sprintf("kind must be %s", constants.GatewayAPIExtensionFilterDefinitionKind))) + if definitionRef.Kind != constants.GatewayAPIExtensionFilterDefinitionKind { + errs = append(errs, field.Invalid(path.Child("kind"), definitionRef.Kind, fmt.Sprintf("kind must be %s", constants.GatewayAPIExtensionFilterDefinitionKind))) + } } return errs diff --git a/pkg/webhook/extension/v1alpha1/listenerfilter.go b/pkg/webhook/extension/v1alpha1/listenerfilter.go index 8d3b4934c..36c8bcb79 100644 --- a/pkg/webhook/extension/v1alpha1/listenerfilter.go +++ b/pkg/webhook/extension/v1alpha1/listenerfilter.go @@ -106,15 +106,17 @@ func (r *ListenerFilterWebhook) validateTargetRefs(ctx context.Context, refs []e return errs } -func (r *ListenerFilterWebhook) validateDefinitionRef(ctx context.Context, definitionRef gwv1.LocalObjectReference, path *field.Path) field.ErrorList { +func (r *ListenerFilterWebhook) validateDefinitionRef(ctx context.Context, definitionRef *gwv1.LocalObjectReference, path *field.Path) field.ErrorList { var errs field.ErrorList - if definitionRef.Group != extv1alpha1.GroupName { - errs = append(errs, field.Invalid(path.Child("group"), definitionRef.Group, fmt.Sprintf("group must be %s", extv1alpha1.GroupName))) - } + if definitionRef != nil { + if definitionRef.Group != extv1alpha1.GroupName { + errs = append(errs, field.Invalid(path.Child("group"), definitionRef.Group, fmt.Sprintf("group must be %s", extv1alpha1.GroupName))) + } - if definitionRef.Kind != constants.GatewayAPIExtensionFilterDefinitionKind { - errs = append(errs, field.Invalid(path.Child("kind"), definitionRef.Kind, fmt.Sprintf("kind must be %s", constants.GatewayAPIExtensionFilterDefinitionKind))) + if definitionRef.Kind != constants.GatewayAPIExtensionFilterDefinitionKind { + errs = append(errs, field.Invalid(path.Child("kind"), definitionRef.Kind, fmt.Sprintf("kind must be %s", constants.GatewayAPIExtensionFilterDefinitionKind))) + } } return errs