From 890787b4b6ceedf9f463563a7b6fee3a67dff47a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Tue, 15 Apr 2025 15:13:56 +0800 Subject: [PATCH 01/16] feat: Add support for HTTPRoutePolicy in HTTPRoute reconciliation --- internal/controller/httproute_controller.go | 51 ++++++++++++++++++- internal/controller/indexer/indexer.go | 49 ++++++++++++++---- internal/provider/adc/translator/httproute.go | 18 +++++++ internal/provider/provider.go | 20 ++++---- 4 files changed, 118 insertions(+), 20 deletions(-) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index afe21a884..059258c5a 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -65,6 +65,9 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { }, ), ). + Watches(&v1alpha1.HTTPRoutePolicy{}, + handler.EnqueueRequestsFromMapFunc(r.listHTTPRouteByHTTPRoutePolicy), + ). Complete(r) } @@ -232,6 +235,36 @@ func (r *HTTPRouteReconciler) listHTTPRoutesForGateway(ctx context.Context, obj return requests } +func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context, obj client.Object) (requests []reconcile.Request) { + httpRoutePolicy, ok := obj.(*v1alpha1.HTTPRoutePolicy) + if !ok { + r.Log.Error(fmt.Errorf("unexpected object type"), "failed to convert object to HTTPRoutePolicy") + return nil + } + + var keys = make(map[client.ObjectKey]struct{}) + for _, ref := range httpRoutePolicy.Spec.TargetRefs { + if ref.Kind == "HTTPRoute" { + keys[client.ObjectKey{ + Namespace: obj.GetNamespace(), + Name: string(ref.Name), + }] = struct{}{} + } + } + for key := range keys { + var httpRoute gatewayv1.HTTPRoute + if err := r.Get(ctx, key, &httpRoute); err != nil { + r.Log.Error(err, "failed to get httproute by HTTPRoutePolicy targetRef", "HTTPRoutePolicy", obj.GetName()) + continue + } + requests = append(requests, reconcile.Request{ + NamespacedName: key, + }) + } + + return requests +} + func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.TranslateContext) error { var terr error for _, backend := range tctx.BackendRefs { @@ -290,7 +323,7 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla return terr } -func (t *HTTPRouteReconciler) processHTTPRoute(tctx *provider.TranslateContext, httpRoute *gatewayv1.HTTPRoute) error { +func (r *HTTPRouteReconciler) processHTTPRoute(tctx *provider.TranslateContext, httpRoute *gatewayv1.HTTPRoute) error { var terror error for _, rule := range httpRoute.Spec.Rules { for _, filter := range rule.Filters { @@ -299,7 +332,7 @@ func (t *HTTPRouteReconciler) processHTTPRoute(tctx *provider.TranslateContext, } if filter.ExtensionRef.Kind == "PluginConfig" { pluginconfig := new(v1alpha1.PluginConfig) - if err := t.Get(context.Background(), client.ObjectKey{ + if err := r.Get(context.Background(), client.ObjectKey{ Namespace: httpRoute.GetNamespace(), Name: string(filter.ExtensionRef.Name), }, pluginconfig); err != nil { @@ -340,6 +373,20 @@ func (t *HTTPRouteReconciler) processHTTPRoute(tctx *provider.TranslateContext, }, }) } + + var httpRoutePolicyList v1alpha1.HTTPRoutePolicyList + var ruleName string + if rule.Name != nil { + ruleName = string(*rule.Name) + } + key := indexer.GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, "HTTPRoute", httpRoute.GetNamespace(), httpRoute.GetName(), ruleName) + if err := r.List(context.Background(), &httpRoutePolicyList, client.MatchingFields{indexer.HTTPRoutePolicy: key}); err != nil { + terror = err + continue + } + for _, item := range httpRoutePolicyList.Items { + tctx.HTTPRoutePolicies[ruleName] = append(tctx.HTTPRoutePolicies[key], item.Spec) + } } return terror diff --git a/internal/controller/indexer/indexer.go b/internal/controller/indexer/indexer.go index 5b20ea121..6b7d90995 100644 --- a/internal/controller/indexer/indexer.go +++ b/internal/controller/indexer/indexer.go @@ -3,22 +3,26 @@ package indexer import ( "context" - "github.com/api7/api7-ingress-controller/api/v1alpha1" networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/api7/api7-ingress-controller/api/v1alpha1" ) const ( - ServiceIndexRef = "serviceRefs" - ExtensionRef = "extensionRef" - ParametersRef = "parametersRef" - ParentRefs = "parentRefs" - IngressClass = "ingressClass" - SecretIndexRef = "secretRefs" - IngressClassRef = "ingressClassRef" - ConsumerGatewayRef = "consumerGatewayRef" + ServiceIndexRef = "serviceRefs" + ExtensionRef = "extensionRef" + ParametersRef = "parametersRef" + ParentRefs = "parentRefs" + IngressClass = "ingressClass" + SecretIndexRef = "secretRefs" + IngressClassRef = "ingressClassRef" + ConsumerGatewayRef = "consumerGatewayRef" + HTTPRoutePolicyTargetRef = "httpRoutePolicyTargetRef" + HTTPRoutePolicy = "httpRoutePolicy" ) func SetupIndexer(mgr ctrl.Manager) error { @@ -128,6 +132,16 @@ func setupHTTPRouteIndexer(mgr ctrl.Manager) error { ); err != nil { return err } + + if err := mgr.GetFieldIndexer().IndexField( + context.Background(), + &v1alpha1.HTTPRoutePolicy{}, + HTTPRoutePolicy, + HTTPRoutePolicyIndexFunc, + ); err != nil { + return err + } + return nil } @@ -281,6 +295,23 @@ func HTTPRouteExtensionIndexFunc(rawObj client.Object) []string { return keys } +func GenHTTPRoutePolicyIndexKey(group, kind, namespace, name, sectionName string) string { + return schema.GroupKind{Group: group, Kind: kind}.String() + "/" + client.ObjectKey{Namespace: namespace, Name: name}.String() + "/" + sectionName +} + +func HTTPRoutePolicyIndexFunc(rawObj client.Object) []string { + hrp := rawObj.(*v1alpha1.HTTPRoutePolicy) + var keys = make([]string, 0, len(hrp.Spec.TargetRefs)) + for i, ref := range hrp.Spec.TargetRefs { + var sectionName string + if ref.SectionName != nil { + sectionName = string(*ref.SectionName) + } + keys[i] = GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, string(ref.Kind), hrp.GetNamespace(), string(ref.Name), sectionName) + } + return keys +} + func GatewayParametersRefIndexFunc(rawObj client.Object) []string { gw := rawObj.(*gatewayv1.Gateway) if gw.Spec.Infrastructure != nil && gw.Spec.Infrastructure.ParametersRef != nil { diff --git a/internal/provider/adc/translator/httproute.go b/internal/provider/adc/translator/httproute.go index 8dea67a97..b69cc16e4 100644 --- a/internal/provider/adc/translator/httproute.go +++ b/internal/provider/adc/translator/httproute.go @@ -231,6 +231,23 @@ func (t *Translator) fillPluginFromHTTPRequestRedirectFilter(plugins adctypes.Pl plugin.URI = uri } +func (t *Translator) fillHTTPRoutePolicies(tctx *provider.TranslateContext, rule gatewayv1.HTTPRouteRule, routes []*adctypes.Route) { + var ruleName string + if rule.Name != nil { + ruleName = string(*rule.Name) + } + specs, ok := tctx.HTTPRoutePolicies[ruleName] + if !ok || len(specs) == 0 { + return + } + for _, route := range routes { + for _, spec := range specs { + route.Priority = spec.Priority + route.Vars = append(route.Vars, spec.Vars...) + } + } +} + func (t *Translator) translateEndpointSlice(weight int, endpointSlices []discoveryv1.EndpointSlice) adctypes.UpstreamNodes { var nodes adctypes.UpstreamNodes if len(endpointSlices) == 0 { @@ -337,6 +354,7 @@ func (t *Translator) TranslateHTTPRoute(tctx *provider.TranslateContext, httpRou route.Labels = labels routes = append(routes, route) } + t.fillHTTPRoutePolicies(tctx, rule, routes) service.Routes = routes result.Services = append(result.Services, service) diff --git a/internal/provider/provider.go b/internal/provider/provider.go index a57675f27..069bd5a48 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -7,9 +7,10 @@ import ( discoveryv1 "k8s.io/api/discovery/v1" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" - "github.com/api7/api7-ingress-controller/api/v1alpha1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/api7/api7-ingress-controller/api/v1alpha1" ) type Provider interface { @@ -18,14 +19,15 @@ type Provider interface { } type TranslateContext struct { - BackendRefs []gatewayv1.BackendRef - GatewayTLSConfig []gatewayv1.GatewayTLSConfig - GatewayProxy *v1alpha1.GatewayProxy - Credentials []v1alpha1.Credential - EndpointSlices map[types.NamespacedName][]discoveryv1.EndpointSlice - Secrets map[types.NamespacedName]*corev1.Secret - PluginConfigs map[types.NamespacedName]*v1alpha1.PluginConfig - Services map[types.NamespacedName]*corev1.Service + BackendRefs []gatewayv1.BackendRef + GatewayTLSConfig []gatewayv1.GatewayTLSConfig + GatewayProxy *v1alpha1.GatewayProxy + Credentials []v1alpha1.Credential + EndpointSlices map[types.NamespacedName][]discoveryv1.EndpointSlice + Secrets map[types.NamespacedName]*corev1.Secret + PluginConfigs map[types.NamespacedName]*v1alpha1.PluginConfig + Services map[types.NamespacedName]*corev1.Service + HTTPRoutePolicies map[string][]v1alpha1.HTTPRoutePolicySpec } func NewDefaultTranslateContext() *TranslateContext { From acee110ac84768ae8ac2aaed3063f5a13c21351a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Tue, 15 Apr 2025 16:49:35 +0800 Subject: [PATCH 02/16] e2e --- internal/provider/adc/translator/httproute.go | 2 + test/e2e/gatewayapi/httproute.go | 47 ++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/internal/provider/adc/translator/httproute.go b/internal/provider/adc/translator/httproute.go index b69cc16e4..1cbe7a707 100644 --- a/internal/provider/adc/translator/httproute.go +++ b/internal/provider/adc/translator/httproute.go @@ -9,6 +9,7 @@ import ( "go.uber.org/zap" discoveryv1 "k8s.io/api/discovery/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/api7/api7-ingress-controller/api/common" @@ -352,6 +353,7 @@ func (t *Translator) TranslateHTTPRoute(tctx *provider.TranslateContext, httpRou route.Name = name route.ID = id.GenID(name) route.Labels = labels + route.EnableWebsocket = ptr.To(true) routes = append(routes, route) } t.fillHTTPRoutePolicies(tctx, rule, routes) diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index 509c1e042..43e7f0a53 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -262,6 +262,7 @@ spec: - path: type: Exact value: /get + name: get backendRefs: - name: httpbin-service-e2e-test port: 80 @@ -288,6 +289,30 @@ spec: backendRefs: - name: httpbin-service-e2e-test port: 80 +` + var httpRoutePolicy = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoutePolicy +metadata: + name: httpRoutePolicy0 +spec: + targetRefs: + - group: gateway.apisix.io + kind: HTTPRoute + name: httpbin + sectionName: get + - group: gateway.apisix.io + kind: HTTPRoute + name: httpbin + sectionName: put +- group: gateway.apisix.io + kind: HTTPRoute + name: httpbin-1 + sectionName: get +priority: 10 +vars: +- ["http_x_hrp_name", "==", "httpRoutePolicy0"] +- ["arg_hrp_name", "==", "httpRoutePolicy0"] ` var prefixRouteByStatus = ` @@ -396,7 +421,7 @@ spec: Status(404) }) - It("HTTPRoute Vars Match", func() { + FIt("HTTPRoute Vars Match", func() { By("create HTTPRoute") ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1) @@ -413,6 +438,26 @@ spec: WithHeader("X-Route-Name", "httpbin"). Expect(). Status(http.StatusOK) + + By("create HTTPRoutePolicy") + ResourceApplied("HTTPRoutePolicy", "httpRoutePolicy0", httpRoutePolicy, 1) + + By("access dataplane to check the HTTPRoutePolicy") + s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + Expect(). + Status(http.StatusNotFound) + + s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + WithHeader("X-HRP-Name", "httpRoutePolicy0"). + WithQuery("hrp_name", "httpRoutePolicy0"). + Expect(). + Status(http.StatusOK) }) }) From e4b2523e689748f8bf16f15b3b6fac3597b373f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 16 Apr 2025 10:32:31 +0800 Subject: [PATCH 03/16] Update HTTPRoutePolicy and related components This commit updates the `HTTPRoutePolicy` CRD with more detailed validation for Kubernetes object references, adds status and ancestor information to the policy, and updates the corresponding controller and test files. Additionally, it refactors the `StringOrSlice` type and adjusts the indexing and translation logic. --- api/common/types.go | 24 ++-- api/common/zz_generated.deepcopy.go | 6 +- api/v1alpha1/httproutepolicy_types.go | 24 ++-- api/v1alpha1/zz_generated.deepcopy.go | 39 ++++-- .../gateway.apisix.io_httproutepolicies.yaml | 120 ++++++++++++++---- internal/controller/httproute_controller.go | 21 +++ internal/controller/indexer/indexer.go | 23 ++-- internal/provider/adc/translator/httproute.go | 9 +- internal/provider/provider.go | 9 +- test/e2e/framework/manifests/ingress.yaml | 15 +++ test/e2e/gatewayapi/httproute.go | 30 ++--- 11 files changed, 224 insertions(+), 96 deletions(-) diff --git a/api/common/types.go b/api/common/types.go index 358857ac3..9b8313aee 100644 --- a/api/common/types.go +++ b/api/common/types.go @@ -31,34 +31,26 @@ func (vars *Vars) UnmarshalJSON(p []byte) error { // StringOrSlice represents a string or a string slice. // TODO Do not use interface{} to avoid the reflection overheads. +// +// +kubebuilder:validation:Schemaless type StringOrSlice struct { - StrVal string `json:"-"` - SliceVal []string `json:"-"` + StrVal string `json:"-"` + SliceVal []StringOrSlice `json:"-"` } func (s *StringOrSlice) MarshalJSON() ([]byte, error) { - var ( - p []byte - err error - ) if s.SliceVal != nil { - p, err = json.Marshal(s.SliceVal) - } else { - p, err = json.Marshal(s.StrVal) + return json.Marshal(s.SliceVal) } - return p, err + return json.Marshal(s.StrVal) } func (s *StringOrSlice) UnmarshalJSON(p []byte) error { - var err error - if len(p) == 0 { return errors.New("empty object") } if p[0] == '[' { - err = json.Unmarshal(p, &s.SliceVal) - } else { - err = json.Unmarshal(p, &s.StrVal) + return json.Unmarshal(p, &s.SliceVal) } - return err + return json.Unmarshal(p, &s.StrVal) } diff --git a/api/common/zz_generated.deepcopy.go b/api/common/zz_generated.deepcopy.go index ee65e53d2..c3046b61c 100644 --- a/api/common/zz_generated.deepcopy.go +++ b/api/common/zz_generated.deepcopy.go @@ -27,8 +27,10 @@ func (in *StringOrSlice) DeepCopyInto(out *StringOrSlice) { *out = *in if in.SliceVal != nil { in, out := &in.SliceVal, &out.SliceVal - *out = make([]string, len(*in)) - copy(*out, *in) + *out = make([]StringOrSlice, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } } diff --git a/api/v1alpha1/httproutepolicy_types.go b/api/v1alpha1/httproutepolicy_types.go index 9da663d36..42c751380 100644 --- a/api/v1alpha1/httproutepolicy_types.go +++ b/api/v1alpha1/httproutepolicy_types.go @@ -17,10 +17,10 @@ limitations under the License. package v1alpha1 import ( + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - - "github.com/api7/api7-ingress-controller/api/common" ) // HTTPRoutePolicySpec defines the desired state of HTTPRoutePolicy. @@ -34,10 +34,20 @@ type HTTPRoutePolicySpec struct { // +listMapKey=name // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=16 - TargetRefs []gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName `json:"targetRefs"` + TargetRefs []TargetReference `json:"targetRefs"` + + Priority *int64 `json:"priority,omitempty" yaml:"priority,omitempty"` + Vars []apiextensionsv1.JSON `json:"vars,omitempty" yaml:"vars,omitempty"` +} - Priority *int64 `json:"priority,omitempty" yaml:"priority,omitempty"` - Vars Vars `json:"vars,omitempty" yaml:"vars,omitempty"` +type TargetReference struct { + Group v1.Group `json:"group"` + Kind v1.Kind `json:"kind"` + // +optional + Namespace *v1.Namespace `json:"namespace,omitempty"` + Name v1.ObjectName `json:"name"` + // +optional + SectionName *v1.SectionName `json:"sectionName,omitempty"` } // +kubebuilder:object:root=true @@ -61,10 +71,6 @@ type HTTPRoutePolicyList struct { Items []HTTPRoutePolicy `json:"items"` } -// Vars represents the route match expressions of APISIX. -// +kubebuilder:object:generate=false -type Vars = common.Vars - func init() { SchemeBuilder.Register(&HTTPRoutePolicy{}, &HTTPRoutePolicyList{}) } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 04ff10e12..527a36441 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -21,10 +21,10 @@ limitations under the License. package v1alpha1 import ( - "github.com/api7/api7-ingress-controller/api/common" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" + apisv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -542,7 +542,7 @@ func (in *HTTPRoutePolicySpec) DeepCopyInto(out *HTTPRoutePolicySpec) { *out = *in if in.TargetRefs != nil { in, out := &in.TargetRefs, &out.TargetRefs - *out = make([]v1alpha2.LocalPolicyTargetReferenceWithSectionName, len(*in)) + *out = make([]TargetReference, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -554,15 +554,9 @@ func (in *HTTPRoutePolicySpec) DeepCopyInto(out *HTTPRoutePolicySpec) { } if in.Vars != nil { in, out := &in.Vars, &out.Vars - *out = make(common.Vars, len(*in)) + *out = make([]v1.JSON, len(*in)) for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = make([]common.StringOrSlice, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } + (*in)[i].DeepCopyInto(&(*out)[i]) } } } @@ -745,6 +739,31 @@ func (in *Status) DeepCopy() *Status { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TargetReference) DeepCopyInto(out *TargetReference) { + *out = *in + if in.Namespace != nil { + in, out := &in.Namespace, &out.Namespace + *out = new(apisv1.Namespace) + **out = **in + } + if in.SectionName != nil { + in, out := &in.SectionName, &out.SectionName + *out = new(apisv1.SectionName) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TargetReference. +func (in *TargetReference) DeepCopy() *TargetReference { + if in == nil { + return nil + } + out := new(TargetReference) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Timeout) DeepCopyInto(out *Timeout) { *out = *in diff --git a/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml b/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml index 39b484fcd..d0d23cf1d 100644 --- a/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml +++ b/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml @@ -49,39 +49,93 @@ spec: target references. items: - description: |- - LocalPolicyTargetReferenceWithSectionName identifies an API object to apply a - direct policy to. This should be used as part of Policy resources that can - target single resources. For more information on how this policy attachment - mode works, and a sample Policy resource, refer to the policy attachment - documentation for Gateway API. - - - Note: This should only be used for direct policy attachment when references - to SectionName are actually needed. In all other cases, - LocalPolicyTargetReference should be used. properties: group: - description: Group is the group of the target resource. + description: |- + Group refers to a Kubernetes Group. It must either be an empty string or a + RFC 1123 subdomain. + + + This validation is based off of the corresponding Kubernetes validation: + https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/util/validation/validation.go#L208 + + + Valid values include: + + + * "" - empty string implies core Kubernetes API group + * "gateway.networking.k8s.io" + * "foo.example.com" + + + Invalid values include: + + + * "example.com/bar" - "/" is an invalid character maxLength: 253 pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string kind: - description: Kind is kind of the target resource. + description: |- + Kind refers to a Kubernetes Kind. + + + Valid values include: + + + * "Service" + * "HTTPRoute" + + + Invalid values include: + + + * "invalid/kind" - "/" is an invalid character maxLength: 63 minLength: 1 pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ type: string name: - description: Name is the name of the target resource. + description: |- + ObjectName refers to the name of a Kubernetes object. + Object names can have a variety of forms, including RFC 1123 subdomains, + RFC 1123 labels, or RFC 1035 labels. maxLength: 253 minLength: 1 type: string + namespace: + description: |- + Namespace refers to a Kubernetes namespace. It must be a RFC 1123 label. + + + This validation is based off of the corresponding Kubernetes validation: + https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/util/validation/validation.go#L187 + + + This is used for Namespace name validation here: + https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/api/validation/generic.go#L63 + + + Valid values include: + + + * "example" + + + Invalid values include: + + + * "example.com" - "." is an invalid character + maxLength: 63 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string sectionName: description: |- - SectionName is the name of a section within the target resource. When - unspecified, this targetRef targets the entire resource. In the following - resources, SectionName is interpreted as the following: + SectionName is the name of a section in a Kubernetes resource. + + + In the following resources, SectionName is interpreted as the following: * Gateway: Listener name @@ -89,9 +143,27 @@ spec: * Service: Port name - If a SectionName is specified, but does not exist on the targeted object, - the Policy must fail to attach, and the policy implementation should record - a `ResolvedRefs` or similar Condition in the Policy's status. + Section names can have a variety of forms, including RFC 1123 subdomains, + RFC 1123 labels, or RFC 1035 labels. + + + This validation is based off of the corresponding Kubernetes validation: + https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/util/validation/validation.go#L208 + + + Valid values include: + + + * "example" + * "foo-example" + * "example.com" + * "foo.example.com" + + + Invalid values include: + + + * "example.com/bar" - "/" is an invalid character maxLength: 253 minLength: 1 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ @@ -110,14 +182,8 @@ spec: - name x-kubernetes-list-type: map vars: - description: Vars represents the route match expressions of APISIX. items: - items: - description: |- - StringOrSlice represents a string or a string slice. - TODO Do not use interface{} to avoid the reflection overheads. - type: object - type: array + x-kubernetes-preserve-unknown-fields: true type: array required: - targetRefs diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 059258c5a..a78ddc24a 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -5,9 +5,11 @@ import ( "fmt" "strings" + "github.com/api7/api7-ingress-controller/internal/controller/config" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -19,6 +21,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/api7/api7-ingress-controller/api/v1alpha1" "github.com/api7/api7-ingress-controller/internal/controller/indexer" @@ -386,6 +389,24 @@ func (r *HTTPRouteReconciler) processHTTPRoute(tctx *provider.TranslateContext, } for _, item := range httpRoutePolicyList.Items { tctx.HTTPRoutePolicies[ruleName] = append(tctx.HTTPRoutePolicies[key], item.Spec) + item.Status.Ancestors = []v1alpha2.PolicyAncestorStatus{ + { + AncestorRef: v1alpha2.ParentReference{ + Group: nil, + Kind: nil, + Namespace: nil, + Name: gatewayv1.ObjectName(httpRoute.GetName()), + SectionName: nil, + Port: nil, + }, + ControllerName: v1alpha2.GatewayController(config.GetControllerName()), + Conditions: []metav1.Condition{}, + }, + } + meta.SetStatusCondition(&item.Status.Ancestors[0].Conditions, NewCondition(item.Generation, true, "Successfully")) + if err := r.Status().Update(context.Background(), &item); err != nil { + r.Log.Error(err, "failed to Update policy status") + } } } diff --git a/internal/controller/indexer/indexer.go b/internal/controller/indexer/indexer.go index 6b7d90995..67fc86d91 100644 --- a/internal/controller/indexer/indexer.go +++ b/internal/controller/indexer/indexer.go @@ -13,16 +13,15 @@ import ( ) const ( - ServiceIndexRef = "serviceRefs" - ExtensionRef = "extensionRef" - ParametersRef = "parametersRef" - ParentRefs = "parentRefs" - IngressClass = "ingressClass" - SecretIndexRef = "secretRefs" - IngressClassRef = "ingressClassRef" - ConsumerGatewayRef = "consumerGatewayRef" - HTTPRoutePolicyTargetRef = "httpRoutePolicyTargetRef" - HTTPRoutePolicy = "httpRoutePolicy" + ServiceIndexRef = "serviceRefs" + ExtensionRef = "extensionRef" + ParametersRef = "parametersRef" + ParentRefs = "parentRefs" + IngressClass = "ingressClass" + SecretIndexRef = "secretRefs" + IngressClassRef = "ingressClassRef" + ConsumerGatewayRef = "consumerGatewayRef" + HTTPRoutePolicy = "httpRoutePolicy" ) func SetupIndexer(mgr ctrl.Manager) error { @@ -302,12 +301,12 @@ func GenHTTPRoutePolicyIndexKey(group, kind, namespace, name, sectionName string func HTTPRoutePolicyIndexFunc(rawObj client.Object) []string { hrp := rawObj.(*v1alpha1.HTTPRoutePolicy) var keys = make([]string, 0, len(hrp.Spec.TargetRefs)) - for i, ref := range hrp.Spec.TargetRefs { + for _, ref := range hrp.Spec.TargetRefs { var sectionName string if ref.SectionName != nil { sectionName = string(*ref.SectionName) } - keys[i] = GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, string(ref.Kind), hrp.GetNamespace(), string(ref.Name), sectionName) + keys = append(keys, GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, string(ref.Kind), hrp.GetNamespace(), string(ref.Name), sectionName)) } return keys } diff --git a/internal/provider/adc/translator/httproute.go b/internal/provider/adc/translator/httproute.go index 1cbe7a707..dead47634 100644 --- a/internal/provider/adc/translator/httproute.go +++ b/internal/provider/adc/translator/httproute.go @@ -244,7 +244,14 @@ func (t *Translator) fillHTTPRoutePolicies(tctx *provider.TranslateContext, rule for _, route := range routes { for _, spec := range specs { route.Priority = spec.Priority - route.Vars = append(route.Vars, spec.Vars...) + for _, data := range spec.Vars { + var v []common.StringOrSlice + if err := json.Unmarshal(data.Raw, &v); err != nil { + log.Errorf("failed to unmarshal spec.Vars item to []StringOrSlice, data: %s", string(data.Raw)) + continue + } + route.Vars = append(route.Vars, v) + } } } } diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 069bd5a48..ec2ac49e6 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -32,9 +32,10 @@ type TranslateContext struct { func NewDefaultTranslateContext() *TranslateContext { return &TranslateContext{ - EndpointSlices: make(map[types.NamespacedName][]discoveryv1.EndpointSlice), - Secrets: make(map[types.NamespacedName]*corev1.Secret), - PluginConfigs: make(map[types.NamespacedName]*v1alpha1.PluginConfig), - Services: make(map[types.NamespacedName]*corev1.Service), + EndpointSlices: make(map[types.NamespacedName][]discoveryv1.EndpointSlice), + Secrets: make(map[types.NamespacedName]*corev1.Secret), + PluginConfigs: make(map[types.NamespacedName]*v1alpha1.PluginConfig), + Services: make(map[types.NamespacedName]*corev1.Service), + HTTPRoutePolicies: make(map[string][]v1alpha1.HTTPRoutePolicySpec), } } diff --git a/test/e2e/framework/manifests/ingress.yaml b/test/e2e/framework/manifests/ingress.yaml index 0a7296821..376c6f448 100644 --- a/test/e2e/framework/manifests/ingress.yaml +++ b/test/e2e/framework/manifests/ingress.yaml @@ -129,6 +129,21 @@ rules: - get - list - watch +- apiGroups: + - gateway.apisix.io + resources: + - httproutepolicies + verbs: + - get + - list + - watch +- apiGroups: + - gateway.apisix.io + resources: + - httproutepolicies/status + verbs: + - get + - update - apiGroups: - gateway.apisix.io resources: diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index 43e7f0a53..6bc0abad9 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -262,7 +262,6 @@ spec: - path: type: Exact value: /get - name: get backendRefs: - name: httpbin-service-e2e-test port: 80 @@ -286,33 +285,34 @@ spec: - type: Exact name: X-Route-Name value: httpbin + # name: get backendRefs: - name: httpbin-service-e2e-test port: 80 ` var httpRoutePolicy = ` -apiVersion: gateway.networking.k8s.io/v1 +apiVersion: gateway.apisix.io/v1alpha1 kind: HTTPRoutePolicy metadata: - name: httpRoutePolicy0 + name: http-route-policy-0 spec: targetRefs: - group: gateway.apisix.io kind: HTTPRoute name: httpbin - sectionName: get + # sectionName: get - group: gateway.apisix.io - kind: HTTPRoute - name: httpbin - sectionName: put -- group: gateway.apisix.io kind: HTTPRoute name: httpbin-1 sectionName: get -priority: 10 -vars: -- ["http_x_hrp_name", "==", "httpRoutePolicy0"] -- ["arg_hrp_name", "==", "httpRoutePolicy0"] + priority: 10 + vars: + - - http_x_hrp_name + - == + - http-route-policy-0 + - - arg_hrp_name + - == + - http-route-policy-0 ` var prefixRouteByStatus = ` @@ -440,7 +440,7 @@ spec: Status(http.StatusOK) By("create HTTPRoutePolicy") - ResourceApplied("HTTPRoutePolicy", "httpRoutePolicy0", httpRoutePolicy, 1) + ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", httpRoutePolicy, 1) By("access dataplane to check the HTTPRoutePolicy") s.NewAPISIXClient(). @@ -454,8 +454,8 @@ spec: GET("/get"). WithHost("httpbin.example"). WithHeader("X-Route-Name", "httpbin"). - WithHeader("X-HRP-Name", "httpRoutePolicy0"). - WithQuery("hrp_name", "httpRoutePolicy0"). + WithHeader("X-HRP-Name", "http-route-policy-0"). + WithQuery("hrp_name", "http-route-policy-0"). Expect(). Status(http.StatusOK) }) From 13f39129e6f7e31e25a1636d7de84660a88ab49f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 16 Apr 2025 15:17:46 +0800 Subject: [PATCH 04/16] Refactor and update types and references This commit refactors the `common` package by moving the `Vars` and `StringOrSlice` types to the `adc` package. It also updates the `TargetReference` type to use `LocalPolicyTargetReferenceWithSectionName` from `gateway-api` and simplifies the CRD documentation. Additionally, it adjusts the import paths and type references in the `httproute.go` file to reflect these changes. --- api/adc/types.go | 68 +++++++++-- api/common/types.go | 56 --------- api/common/zz_generated.deepcopy.go | 72 ----------- api/v1alpha1/httproutepolicy_types.go | 13 +- api/v1alpha1/zz_generated.deepcopy.go | 28 +---- .../gateway.apisix.io_httproutepolicies.yaml | 112 ++++-------------- internal/provider/adc/translator/httproute.go | 34 +++--- 7 files changed, 94 insertions(+), 289 deletions(-) delete mode 100644 api/common/types.go delete mode 100644 api/common/zz_generated.deepcopy.go diff --git a/api/adc/types.go b/api/adc/types.go index 2e510b7f4..39ebbf336 100644 --- a/api/adc/types.go +++ b/api/adc/types.go @@ -10,8 +10,6 @@ import ( "time" "github.com/incubator4/go-resty-expr/expr" - - "github.com/api7/api7-ingress-controller/api/common" ) const ( @@ -115,16 +113,16 @@ type Service struct { type Route struct { Metadata `json:",inline" yaml:",inline"` - EnableWebsocket *bool `json:"enable_websocket,omitempty" yaml:"enable_websocket,omitempty"` - FilterFunc string `json:"filter_func,omitempty" yaml:"filter_func,omitempty"` - Hosts []string `json:"hosts,omitempty" yaml:"hosts,omitempty"` - Methods []string `json:"methods,omitempty" yaml:"methods,omitempty"` - Plugins Plugins `json:"plugins,omitempty" yaml:"plugins,omitempty"` - Priority *int64 `json:"priority,omitempty" yaml:"priority,omitempty"` - RemoteAddrs []string `json:"remote_addrs,omitempty" yaml:"remote_addrs,omitempty"` - Timeout *Timeout `json:"timeout,omitempty" yaml:"timeout,omitempty"` - Uris []string `json:"uris" yaml:"uris"` - Vars common.Vars `json:"vars,omitempty" yaml:"vars,omitempty"` + EnableWebsocket *bool `json:"enable_websocket,omitempty" yaml:"enable_websocket,omitempty"` + FilterFunc string `json:"filter_func,omitempty" yaml:"filter_func,omitempty"` + Hosts []string `json:"hosts,omitempty" yaml:"hosts,omitempty"` + Methods []string `json:"methods,omitempty" yaml:"methods,omitempty"` + Plugins Plugins `json:"plugins,omitempty" yaml:"plugins,omitempty"` + Priority *int64 `json:"priority,omitempty" yaml:"priority,omitempty"` + RemoteAddrs []string `json:"remote_addrs,omitempty" yaml:"remote_addrs,omitempty"` + Timeout *Timeout `json:"timeout,omitempty" yaml:"timeout,omitempty"` + Uris []string `json:"uris" yaml:"uris"` + Vars Vars `json:"vars,omitempty" yaml:"vars,omitempty"` } type Timeout struct { @@ -502,3 +500,49 @@ type ResponseData struct { Value map[string]any `json:"value"` ErrorMsg string `json:"error_msg"` } + +// Vars represents the route match expressions of APISIX. +type Vars [][]StringOrSlice + +// UnmarshalJSON implements json.Unmarshaler interface. +// lua-cjson doesn't distinguish empty array and table, +// and by default empty array will be encoded as '{}'. +// We have to maintain the compatibility. +func (vars *Vars) UnmarshalJSON(p []byte) error { + if p[0] == '{' { + if len(p) != 2 { + return errors.New("unexpected non-empty object") + } + return nil + } + var data [][]StringOrSlice + if err := json.Unmarshal(p, &data); err != nil { + return err + } + *vars = data + return nil +} + +// StringOrSlice represents a string or a string slice. +// TODO Do not use interface{} to avoid the reflection overheads. +type StringOrSlice struct { + StrVal string `json:"-"` + SliceVal []StringOrSlice `json:"-"` +} + +func (s *StringOrSlice) MarshalJSON() ([]byte, error) { + if s.SliceVal != nil { + return json.Marshal(s.SliceVal) + } + return json.Marshal(s.StrVal) +} + +func (s *StringOrSlice) UnmarshalJSON(p []byte) error { + if len(p) == 0 { + return errors.New("empty object") + } + if p[0] == '[' { + return json.Unmarshal(p, &s.SliceVal) + } + return json.Unmarshal(p, &s.StrVal) +} diff --git a/api/common/types.go b/api/common/types.go deleted file mode 100644 index 9b8313aee..000000000 --- a/api/common/types.go +++ /dev/null @@ -1,56 +0,0 @@ -// +kubebuilder:object:generate=true - -package common - -import ( - "encoding/json" - "errors" -) - -// Vars represents the route match expressions of APISIX. -type Vars [][]StringOrSlice - -// UnmarshalJSON implements json.Unmarshaler interface. -// lua-cjson doesn't distinguish empty array and table, -// and by default empty array will be encoded as '{}'. -// We have to maintain the compatibility. -func (vars *Vars) UnmarshalJSON(p []byte) error { - if p[0] == '{' { - if len(p) != 2 { - return errors.New("unexpected non-empty object") - } - return nil - } - var data [][]StringOrSlice - if err := json.Unmarshal(p, &data); err != nil { - return err - } - *vars = data - return nil -} - -// StringOrSlice represents a string or a string slice. -// TODO Do not use interface{} to avoid the reflection overheads. -// -// +kubebuilder:validation:Schemaless -type StringOrSlice struct { - StrVal string `json:"-"` - SliceVal []StringOrSlice `json:"-"` -} - -func (s *StringOrSlice) MarshalJSON() ([]byte, error) { - if s.SliceVal != nil { - return json.Marshal(s.SliceVal) - } - return json.Marshal(s.StrVal) -} - -func (s *StringOrSlice) UnmarshalJSON(p []byte) error { - if len(p) == 0 { - return errors.New("empty object") - } - if p[0] == '[' { - return json.Unmarshal(p, &s.SliceVal) - } - return json.Unmarshal(p, &s.StrVal) -} diff --git a/api/common/zz_generated.deepcopy.go b/api/common/zz_generated.deepcopy.go deleted file mode 100644 index c3046b61c..000000000 --- a/api/common/zz_generated.deepcopy.go +++ /dev/null @@ -1,72 +0,0 @@ -//go:build !ignore_autogenerated - -/* -Copyright 2024. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Code generated by controller-gen. DO NOT EDIT. - -package common - -import () - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *StringOrSlice) DeepCopyInto(out *StringOrSlice) { - *out = *in - if in.SliceVal != nil { - in, out := &in.SliceVal, &out.SliceVal - *out = make([]StringOrSlice, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StringOrSlice. -func (in *StringOrSlice) DeepCopy() *StringOrSlice { - if in == nil { - return nil - } - out := new(StringOrSlice) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in Vars) DeepCopyInto(out *Vars) { - { - in := &in - *out = make(Vars, len(*in)) - for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = make([]StringOrSlice, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Vars. -func (in Vars) DeepCopy() Vars { - if in == nil { - return nil - } - out := new(Vars) - in.DeepCopyInto(out) - return *out -} diff --git a/api/v1alpha1/httproutepolicy_types.go b/api/v1alpha1/httproutepolicy_types.go index 42c751380..d2fd74a3d 100644 --- a/api/v1alpha1/httproutepolicy_types.go +++ b/api/v1alpha1/httproutepolicy_types.go @@ -19,7 +19,6 @@ package v1alpha1 import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -34,22 +33,12 @@ type HTTPRoutePolicySpec struct { // +listMapKey=name // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=16 - TargetRefs []TargetReference `json:"targetRefs"` + TargetRefs []gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName `json:"targetRefs"` Priority *int64 `json:"priority,omitempty" yaml:"priority,omitempty"` Vars []apiextensionsv1.JSON `json:"vars,omitempty" yaml:"vars,omitempty"` } -type TargetReference struct { - Group v1.Group `json:"group"` - Kind v1.Kind `json:"kind"` - // +optional - Namespace *v1.Namespace `json:"namespace,omitempty"` - Name v1.ObjectName `json:"name"` - // +optional - SectionName *v1.SectionName `json:"sectionName,omitempty"` -} - // +kubebuilder:object:root=true // +kubebuilder:subresource:status diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 527a36441..9dc527fa7 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -24,7 +24,6 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" - apisv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -542,7 +541,7 @@ func (in *HTTPRoutePolicySpec) DeepCopyInto(out *HTTPRoutePolicySpec) { *out = *in if in.TargetRefs != nil { in, out := &in.TargetRefs, &out.TargetRefs - *out = make([]TargetReference, len(*in)) + *out = make([]v1alpha2.LocalPolicyTargetReferenceWithSectionName, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -739,31 +738,6 @@ func (in *Status) DeepCopy() *Status { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *TargetReference) DeepCopyInto(out *TargetReference) { - *out = *in - if in.Namespace != nil { - in, out := &in.Namespace, &out.Namespace - *out = new(apisv1.Namespace) - **out = **in - } - if in.SectionName != nil { - in, out := &in.SectionName, &out.SectionName - *out = new(apisv1.SectionName) - **out = **in - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TargetReference. -func (in *TargetReference) DeepCopy() *TargetReference { - if in == nil { - return nil - } - out := new(TargetReference) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Timeout) DeepCopyInto(out *Timeout) { *out = *in diff --git a/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml b/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml index d0d23cf1d..f9afb85c8 100644 --- a/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml +++ b/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml @@ -49,93 +49,39 @@ spec: target references. items: - properties: - group: - description: |- - Group refers to a Kubernetes Group. It must either be an empty string or a - RFC 1123 subdomain. - - - This validation is based off of the corresponding Kubernetes validation: - https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/util/validation/validation.go#L208 - - - Valid values include: - - - * "" - empty string implies core Kubernetes API group - * "gateway.networking.k8s.io" - * "foo.example.com" - - - Invalid values include: + description: |- + LocalPolicyTargetReferenceWithSectionName identifies an API object to apply a + direct policy to. This should be used as part of Policy resources that can + target single resources. For more information on how this policy attachment + mode works, and a sample Policy resource, refer to the policy attachment + documentation for Gateway API. - * "example.com/bar" - "/" is an invalid character + Note: This should only be used for direct policy attachment when references + to SectionName are actually needed. In all other cases, + LocalPolicyTargetReference should be used. + properties: + group: + description: Group is the group of the target resource. maxLength: 253 pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string kind: - description: |- - Kind refers to a Kubernetes Kind. - - - Valid values include: - - - * "Service" - * "HTTPRoute" - - - Invalid values include: - - - * "invalid/kind" - "/" is an invalid character + description: Kind is kind of the target resource. maxLength: 63 minLength: 1 pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ type: string name: - description: |- - ObjectName refers to the name of a Kubernetes object. - Object names can have a variety of forms, including RFC 1123 subdomains, - RFC 1123 labels, or RFC 1035 labels. + description: Name is the name of the target resource. maxLength: 253 minLength: 1 type: string - namespace: - description: |- - Namespace refers to a Kubernetes namespace. It must be a RFC 1123 label. - - - This validation is based off of the corresponding Kubernetes validation: - https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/util/validation/validation.go#L187 - - - This is used for Namespace name validation here: - https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/api/validation/generic.go#L63 - - - Valid values include: - - - * "example" - - - Invalid values include: - - - * "example.com" - "." is an invalid character - maxLength: 63 - minLength: 1 - pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ - type: string sectionName: description: |- - SectionName is the name of a section in a Kubernetes resource. - - - In the following resources, SectionName is interpreted as the following: + SectionName is the name of a section within the target resource. When + unspecified, this targetRef targets the entire resource. In the following + resources, SectionName is interpreted as the following: * Gateway: Listener name @@ -143,27 +89,9 @@ spec: * Service: Port name - Section names can have a variety of forms, including RFC 1123 subdomains, - RFC 1123 labels, or RFC 1035 labels. - - - This validation is based off of the corresponding Kubernetes validation: - https://github.com/kubernetes/apimachinery/blob/02cfb53916346d085a6c6c7c66f882e3c6b0eca6/pkg/util/validation/validation.go#L208 - - - Valid values include: - - - * "example" - * "foo-example" - * "example.com" - * "foo.example.com" - - - Invalid values include: - - - * "example.com/bar" - "/" is an invalid character + If a SectionName is specified, but does not exist on the targeted object, + the Policy must fail to attach, and the policy implementation should record + a `ResolvedRefs` or similar Condition in the Policy's status. maxLength: 253 minLength: 1 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ diff --git a/internal/provider/adc/translator/httproute.go b/internal/provider/adc/translator/httproute.go index dead47634..f9261c963 100644 --- a/internal/provider/adc/translator/httproute.go +++ b/internal/provider/adc/translator/httproute.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/api7/gopkg/pkg/log" "github.com/pkg/errors" "go.uber.org/zap" discoveryv1 "k8s.io/api/discovery/v1" @@ -12,9 +13,6 @@ import ( "k8s.io/utils/ptr" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" - "github.com/api7/api7-ingress-controller/api/common" - "github.com/api7/gopkg/pkg/log" - adctypes "github.com/api7/api7-ingress-controller/api/adc" "github.com/api7/api7-ingress-controller/internal/controller/label" "github.com/api7/api7-ingress-controller/internal/id" @@ -245,7 +243,7 @@ func (t *Translator) fillHTTPRoutePolicies(tctx *provider.TranslateContext, rule for _, spec := range specs { route.Priority = spec.Priority for _, data := range spec.Vars { - var v []common.StringOrSlice + var v []adctypes.StringOrSlice if err := json.Unmarshal(data.Raw, &v); err != nil { log.Errorf("failed to unmarshal spec.Vars item to []StringOrSlice, data: %s", string(data.Raw)) continue @@ -383,14 +381,14 @@ func (t *Translator) translateGatewayHTTPRouteMatch(match *gatewayv1.HTTPRouteMa case gatewayv1.PathMatchPathPrefix: route.Uris = []string{*match.Path.Value + "*"} case gatewayv1.PathMatchRegularExpression: - var this []common.StringOrSlice - this = append(this, common.StringOrSlice{ + var this []adctypes.StringOrSlice + this = append(this, adctypes.StringOrSlice{ StrVal: "uri", }) - this = append(this, common.StringOrSlice{ + this = append(this, adctypes.StringOrSlice{ StrVal: "~~", }) - this = append(this, common.StringOrSlice{ + this = append(this, adctypes.StringOrSlice{ StrVal: *match.Path.Value, }) @@ -405,25 +403,25 @@ func (t *Translator) translateGatewayHTTPRouteMatch(match *gatewayv1.HTTPRouteMa name := strings.ToLower(string(header.Name)) name = strings.ReplaceAll(name, "-", "_") - var this []common.StringOrSlice - this = append(this, common.StringOrSlice{ + var this []adctypes.StringOrSlice + this = append(this, adctypes.StringOrSlice{ StrVal: "http_" + name, }) switch *header.Type { case gatewayv1.HeaderMatchExact: - this = append(this, common.StringOrSlice{ + this = append(this, adctypes.StringOrSlice{ StrVal: "==", }) case gatewayv1.HeaderMatchRegularExpression: - this = append(this, common.StringOrSlice{ + this = append(this, adctypes.StringOrSlice{ StrVal: "~~", }) default: return nil, errors.New("unknown header match type " + string(*header.Type)) } - this = append(this, common.StringOrSlice{ + this = append(this, adctypes.StringOrSlice{ StrVal: header.Value, }) @@ -433,25 +431,25 @@ func (t *Translator) translateGatewayHTTPRouteMatch(match *gatewayv1.HTTPRouteMa if len(match.QueryParams) > 0 { for _, query := range match.QueryParams { - var this []common.StringOrSlice - this = append(this, common.StringOrSlice{ + var this []adctypes.StringOrSlice + this = append(this, adctypes.StringOrSlice{ StrVal: "arg_" + strings.ToLower(fmt.Sprintf("%v", query.Name)), }) switch *query.Type { case gatewayv1.QueryParamMatchExact: - this = append(this, common.StringOrSlice{ + this = append(this, adctypes.StringOrSlice{ StrVal: "==", }) case gatewayv1.QueryParamMatchRegularExpression: - this = append(this, common.StringOrSlice{ + this = append(this, adctypes.StringOrSlice{ StrVal: "~~", }) default: return nil, errors.New("unknown query match type " + string(*query.Type)) } - this = append(this, common.StringOrSlice{ + this = append(this, adctypes.StringOrSlice{ StrVal: query.Value, }) From 1ad0626b1eb42028956b6001c5e656cde3a11f9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 16 Apr 2025 23:32:33 +0800 Subject: [PATCH 05/16] conflicts --- internal/controller/httproute_controller.go | 78 ++++----- internal/controller/httproutepolicy.go | 154 ++++++++++++++++++ internal/provider/adc/translator/httproute.go | 19 +-- internal/provider/provider.go | 4 +- 4 files changed, 196 insertions(+), 59 deletions(-) create mode 100644 internal/controller/httproutepolicy.go diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index a78ddc24a..495679fe0 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -5,11 +5,12 @@ import ( "fmt" "strings" - "github.com/api7/api7-ingress-controller/internal/controller/config" + "github.com/api7/api7-ingress-controller/api/v1alpha1" + "github.com/api7/api7-ingress-controller/internal/controller/indexer" + "github.com/api7/api7-ingress-controller/internal/provider" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -22,10 +23,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" - - "github.com/api7/api7-ingress-controller/api/v1alpha1" - "github.com/api7/api7-ingress-controller/internal/controller/indexer" - "github.com/api7/api7-ingress-controller/internal/provider" ) // HTTPRouteReconciler reconciles a GatewayClass object. @@ -125,6 +122,11 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( acceptStatus.msg = err.Error() } + if err := r.processHTTPRoutePolicies(tctx, hr); err != nil { + acceptStatus.status = false + acceptStatus.msg = err.Error() + } + if err := r.processHTTPRouteBackendRefs(tctx); err != nil { resolveRefStatus = status{ status: false, @@ -245,26 +247,42 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context return nil } - var keys = make(map[client.ObjectKey]struct{}) + var keys = make(map[ancestorRefKey]struct{}) for _, ref := range httpRoutePolicy.Spec.TargetRefs { if ref.Kind == "HTTPRoute" { - keys[client.ObjectKey{ - Namespace: obj.GetNamespace(), - Name: string(ref.Name), - }] = struct{}{} + key := ancestorRefKey{ + Group: gatewayv1.GroupName, + Kind: "HTTPRoute", + Namespace: gatewayv1.Namespace(obj.GetNamespace()), + Name: ref.Name, + } + if ref.SectionName != nil { + key.SectionName = *ref.SectionName + } + keys[key] = struct{}{} } } for key := range keys { var httpRoute gatewayv1.HTTPRoute - if err := r.Get(ctx, key, &httpRoute); err != nil { - r.Log.Error(err, "failed to get httproute by HTTPRoutePolicy targetRef", "HTTPRoutePolicy", obj.GetName()) + if err := r.Get(ctx, client.ObjectKey{Namespace: string(key.Namespace), Name: string(key.Name)}, &httpRoute); err != nil { + r.Log.Error(err, "failed to get httproute by HTTPRoutePolicy targetRef", "namespace", obj.GetNamespace(), "name", obj.GetName()) + if err := r.updateHTTPRoutePolicyStatus(key, *httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found HTTPRoute"); err != nil { + r.Log.Error(err, "failed to update HTTPRoutePolicy Status") + } continue } requests = append(requests, reconcile.Request{ - NamespacedName: key, + NamespacedName: types.NamespacedName{ + Namespace: string(key.Namespace), + Name: string(key.Name), + }, }) } + if err := r.Status().Update(ctx, httpRoutePolicy); err != nil { + r.Log.Error(err, "failed to update HTTPRoutePolicy status", "namespace", obj.GetNamespace(), "name", obj.GetName()) + } + return requests } @@ -376,38 +394,6 @@ func (r *HTTPRouteReconciler) processHTTPRoute(tctx *provider.TranslateContext, }, }) } - - var httpRoutePolicyList v1alpha1.HTTPRoutePolicyList - var ruleName string - if rule.Name != nil { - ruleName = string(*rule.Name) - } - key := indexer.GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, "HTTPRoute", httpRoute.GetNamespace(), httpRoute.GetName(), ruleName) - if err := r.List(context.Background(), &httpRoutePolicyList, client.MatchingFields{indexer.HTTPRoutePolicy: key}); err != nil { - terror = err - continue - } - for _, item := range httpRoutePolicyList.Items { - tctx.HTTPRoutePolicies[ruleName] = append(tctx.HTTPRoutePolicies[key], item.Spec) - item.Status.Ancestors = []v1alpha2.PolicyAncestorStatus{ - { - AncestorRef: v1alpha2.ParentReference{ - Group: nil, - Kind: nil, - Namespace: nil, - Name: gatewayv1.ObjectName(httpRoute.GetName()), - SectionName: nil, - Port: nil, - }, - ControllerName: v1alpha2.GatewayController(config.GetControllerName()), - Conditions: []metav1.Condition{}, - }, - } - meta.SetStatusCondition(&item.Status.Ancestors[0].Conditions, NewCondition(item.Generation, true, "Successfully")) - if err := r.Status().Update(context.Background(), &item); err != nil { - r.Log.Error(err, "failed to Update policy status") - } - } } return terror diff --git a/internal/controller/httproutepolicy.go b/internal/controller/httproutepolicy.go new file mode 100644 index 000000000..68461d5fb --- /dev/null +++ b/internal/controller/httproutepolicy.go @@ -0,0 +1,154 @@ +package controller + +import ( + "context" + "time" + + "github.com/api7/api7-ingress-controller/api/v1alpha1" + "github.com/api7/api7-ingress-controller/internal/controller/config" + "github.com/api7/api7-ingress-controller/internal/controller/indexer" + "github.com/api7/api7-ingress-controller/internal/provider" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateContext, httpRoute *gatewayv1.HTTPRoute) error { + // list HTTPRoutePolices which sectionName is not specified + var ( + checker = conflictChecker{ + httpRoute: httpRoute, + policies: make(map[ancestorRefKey][]v1alpha1.HTTPRoutePolicy), + } + listForAllRules v1alpha1.HTTPRoutePolicyList + key = indexer.GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, "HTTPRoute", httpRoute.GetNamespace(), httpRoute.GetName(), "") + ) + if err := r.List(context.Background(), &listForAllRules, client.MatchingFields{indexer.HTTPRoutePolicy: key}); err != nil { + return err + } + + for _, item := range listForAllRules.Items { + checker.append("", item) + tctx.HTTPRoutePolicies["*"] = append(tctx.HTTPRoutePolicies["*"], item) + } + + for _, rule := range httpRoute.Spec.Rules { + if rule.Name == nil { + continue + } + + var ( + ruleName = string(*rule.Name) + listForSectionRules v1alpha1.HTTPRoutePolicyList + key = indexer.GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, "HTTPRoute", httpRoute.GetNamespace(), httpRoute.GetName(), ruleName) + ) + if err := r.List(context.Background(), &listForSectionRules, client.MatchingFields{indexer.HTTPRoutePolicy: key}); err != nil { + continue + } + for _, item := range listForSectionRules.Items { + checker.append(ruleName, item) + tctx.HTTPRoutePolicies[ruleName] = append(tctx.HTTPRoutePolicies[ruleName], item) + } + } + + var ( + status = true + reason = string(v1alpha2.PolicyReasonAccepted) + message string + ) + if checker.conflict { + status = false + reason = string(v1alpha2.PolicyReasonConflicted) + message = "HTTPRoutePolicy conflict with others target to the HTTPRoute" + + // clear HTTPRoutePolices from TranslateContext + tctx.HTTPRoutePolicies = make(map[string][]v1alpha1.HTTPRoutePolicy) + } + + for key, policies := range checker.policies { + for _, policy := range policies { + if err := r.updateHTTPRoutePolicyStatus(key, policy, status, reason, message); err != nil { + r.Log.Error(err, "failed to update HTTPRoutePolicyStatus") + } + } + } + + return nil +} + +func (r *HTTPRouteReconciler) updateHTTPRoutePolicyStatus(key ancestorRefKey, policy v1alpha1.HTTPRoutePolicy, status bool, reason, message string) error { + condition := metav1.Condition{ + Type: string(v1alpha2.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: policy.GetGeneration(), + LastTransitionTime: metav1.Time{Time: time.Now()}, + Reason: reason, + Message: message, + } + if !status { + condition.Status = metav1.ConditionFalse + } + var hasAncestor bool + for _, ancestor := range policy.Status.Ancestors { + if ancestor.AncestorRef.Kind != nil && *ancestor.AncestorRef.Kind == key.Kind && ancestor.AncestorRef.Name == key.Name { + ancestor.ControllerName = v1alpha2.GatewayController(config.GetControllerName()) + ancestor.Conditions = []metav1.Condition{condition} + hasAncestor = true + } + } + if !hasAncestor { + ref := v1alpha2.ParentReference{ + Group: &key.Group, + Kind: &key.Kind, + Namespace: &key.Namespace, + Name: key.Name, + } + if key.SectionName != "" { + ref.SectionName = &key.SectionName + } + policy.Status.Ancestors = append(policy.Status.Ancestors, v1alpha2.PolicyAncestorStatus{ + AncestorRef: ref, + ControllerName: v1alpha2.GatewayController(config.GetControllerName()), + Conditions: []metav1.Condition{condition}, + }) + } + return r.Status().Update(context.Background(), &policy) +} + +type conflictChecker struct { + httpRoute *gatewayv1.HTTPRoute + policies map[ancestorRefKey][]v1alpha1.HTTPRoutePolicy + conflict bool +} + +type ancestorRefKey struct { + Group gatewayv1.Group + Kind gatewayv1.Kind + Namespace gatewayv1.Namespace + Name gatewayv1.ObjectName + SectionName gatewayv1.SectionName +} + +func (c *conflictChecker) append(sectionName string, policy v1alpha1.HTTPRoutePolicy) { + key := ancestorRefKey{ + Group: gatewayv1.GroupName, + Kind: "HTTPRoute", + Namespace: gatewayv1.Namespace(c.httpRoute.GetNamespace()), + Name: gatewayv1.ObjectName(c.httpRoute.GetName()), + SectionName: gatewayv1.SectionName(sectionName), + } + c.policies[key] = append(c.policies[key], policy) + + if !c.conflict { + Loop: + for _, items := range c.policies { + for _, item := range items { + if item.Spec.Priority != policy.Spec.Priority || *item.Spec.Priority != *policy.Spec.Priority { + c.conflict = true + break Loop + } + } + } + } +} diff --git a/internal/provider/adc/translator/httproute.go b/internal/provider/adc/translator/httproute.go index f9261c963..c3647bcd2 100644 --- a/internal/provider/adc/translator/httproute.go +++ b/internal/provider/adc/translator/httproute.go @@ -231,21 +231,18 @@ func (t *Translator) fillPluginFromHTTPRequestRedirectFilter(plugins adctypes.Pl } func (t *Translator) fillHTTPRoutePolicies(tctx *provider.TranslateContext, rule gatewayv1.HTTPRouteRule, routes []*adctypes.Route) { - var ruleName string + policies := tctx.HTTPRoutePolicies["*"] // policies which not specify a sectionName if rule.Name != nil { - ruleName = string(*rule.Name) + policies = append(policies, tctx.HTTPRoutePolicies[string(*rule.Name)]...) // append policies which specify the sectionName as the same as rule.name } - specs, ok := tctx.HTTPRoutePolicies[ruleName] - if !ok || len(specs) == 0 { - return - } - for _, route := range routes { - for _, spec := range specs { - route.Priority = spec.Priority - for _, data := range spec.Vars { + + for _, policy := range policies { + for _, route := range routes { + route.Priority = policy.Spec.Priority + for _, data := range policy.Spec.Vars { var v []adctypes.StringOrSlice if err := json.Unmarshal(data.Raw, &v); err != nil { - log.Errorf("failed to unmarshal spec.Vars item to []StringOrSlice, data: %s", string(data.Raw)) + log.Errorf("failed to unmarshal spec.Vars item to []StringOrSlice, data: %s", string(data.Raw)) // todo: update status continue } route.Vars = append(route.Vars, v) diff --git a/internal/provider/provider.go b/internal/provider/provider.go index ec2ac49e6..f1db50de9 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -27,7 +27,7 @@ type TranslateContext struct { Secrets map[types.NamespacedName]*corev1.Secret PluginConfigs map[types.NamespacedName]*v1alpha1.PluginConfig Services map[types.NamespacedName]*corev1.Service - HTTPRoutePolicies map[string][]v1alpha1.HTTPRoutePolicySpec + HTTPRoutePolicies map[string][]v1alpha1.HTTPRoutePolicy } func NewDefaultTranslateContext() *TranslateContext { @@ -36,6 +36,6 @@ func NewDefaultTranslateContext() *TranslateContext { Secrets: make(map[types.NamespacedName]*corev1.Secret), PluginConfigs: make(map[types.NamespacedName]*v1alpha1.PluginConfig), Services: make(map[types.NamespacedName]*corev1.Service), - HTTPRoutePolicies: make(map[string][]v1alpha1.HTTPRoutePolicySpec), + HTTPRoutePolicies: make(map[string][]v1alpha1.HTTPRoutePolicy), } } From 5c8aa78452704a4398f4ea3c12d88e4cd371c264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Thu, 17 Apr 2025 11:05:14 +0800 Subject: [PATCH 06/16] Refactor HTTPRoutePolicy status update and ancestor management The `updateHTTPRoutePolicyStatus` function is replaced with `modifyHTTPRoutePolicyStatus` to streamline the process, and a new function `clearHTTPRoutePolicyRedundantAncestor` is introduced to remove redundant ancestors. Additionally, the group in the test's targetRefs is updated to `gateway.networking.k8s.io`. --- internal/controller/httproute_controller.go | 10 +++-- internal/controller/httproutepolicy.go | 46 +++++++++++++++++++-- test/e2e/gatewayapi/httproute.go | 4 +- 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 495679fe0..86043b248 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -265,10 +265,9 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context for key := range keys { var httpRoute gatewayv1.HTTPRoute if err := r.Get(ctx, client.ObjectKey{Namespace: string(key.Namespace), Name: string(key.Name)}, &httpRoute); err != nil { - r.Log.Error(err, "failed to get httproute by HTTPRoutePolicy targetRef", "namespace", obj.GetNamespace(), "name", obj.GetName()) - if err := r.updateHTTPRoutePolicyStatus(key, *httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found HTTPRoute"); err != nil { - r.Log.Error(err, "failed to update HTTPRoutePolicy Status") - } + r.Log.Error(err, "failed to get HTTPRoute by HTTPRoutePolicy targetRef", "namespace", obj.GetNamespace(), "name", obj.GetName()) + r.modifyHTTPRoutePolicyStatus(key, httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found HTTPRoute") + r.Log.Info("status after modified", "key", key, "status", httpRoutePolicy.Status.Ancestors) continue } requests = append(requests, reconcile.Request{ @@ -279,6 +278,9 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context }) } + r.Log.Info("status before clear", "status", httpRoutePolicy.Status.Ancestors) + r.clearHTTPRoutePolicyRedundantAncestor(httpRoutePolicy) + r.Log.Info("status after clear", "status", httpRoutePolicy.Status.Ancestors) if err := r.Status().Update(ctx, httpRoutePolicy); err != nil { r.Log.Error(err, "failed to update HTTPRoutePolicy status", "namespace", obj.GetNamespace(), "name", obj.GetName()) } diff --git a/internal/controller/httproutepolicy.go b/internal/controller/httproutepolicy.go index 68461d5fb..2da296576 100644 --- a/internal/controller/httproutepolicy.go +++ b/internal/controller/httproutepolicy.go @@ -2,6 +2,7 @@ package controller import ( "context" + "slices" "time" "github.com/api7/api7-ingress-controller/api/v1alpha1" @@ -68,7 +69,8 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC for key, policies := range checker.policies { for _, policy := range policies { - if err := r.updateHTTPRoutePolicyStatus(key, policy, status, reason, message); err != nil { + r.modifyHTTPRoutePolicyStatus(key, &policy, status, reason, message) + if err := r.Status().Update(context.Background(), &policy); err != nil { r.Log.Error(err, "failed to update HTTPRoutePolicyStatus") } } @@ -77,7 +79,43 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC return nil } -func (r *HTTPRouteReconciler) updateHTTPRoutePolicyStatus(key ancestorRefKey, policy v1alpha1.HTTPRoutePolicy, status bool, reason, message string) error { +func (r *HTTPRouteReconciler) clearHTTPRoutePolicyRedundantAncestor(policy *v1alpha1.HTTPRoutePolicy) { + var keys = make(map[ancestorRefKey]struct{}) + for _, ref := range policy.Spec.TargetRefs { + key := ancestorRefKey{ + Group: ref.Group, + Kind: ref.Kind, + Namespace: gatewayv1.Namespace(policy.GetNamespace()), + Name: ref.Name, + } + if ref.SectionName != nil { + key.SectionName = *ref.SectionName + } + r.Log.Info("clearHTTPRoutePolicyRedundantAncestor", "keys[]", key) + keys[key] = struct{}{} + } + + policy.Status.Ancestors = slices.DeleteFunc(policy.Status.Ancestors, func(ancestor v1alpha2.PolicyAncestorStatus) bool { + key := ancestorRefKey{ + Namespace: gatewayv1.Namespace(policy.GetNamespace()), + Name: ancestor.AncestorRef.Name, + } + if ancestor.AncestorRef.Group != nil { + key.Group = *ancestor.AncestorRef.Group + } + if ancestor.AncestorRef.Kind != nil { + key.Kind = *ancestor.AncestorRef.Kind + } + if ancestor.AncestorRef.SectionName != nil { + key.SectionName = *ancestor.AncestorRef.SectionName + } + r.Log.Info("clearHTTPRoutePolicyRedundantAncestor", "key", key) + _, ok := keys[key] + return !ok + }) +} + +func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(key ancestorRefKey, policy *v1alpha1.HTTPRoutePolicy, status bool, reason, message string) { condition := metav1.Condition{ Type: string(v1alpha2.PolicyConditionAccepted), Status: metav1.ConditionTrue, @@ -90,12 +128,13 @@ func (r *HTTPRouteReconciler) updateHTTPRoutePolicyStatus(key ancestorRefKey, po condition.Status = metav1.ConditionFalse } var hasAncestor bool - for _, ancestor := range policy.Status.Ancestors { + for i, ancestor := range policy.Status.Ancestors { if ancestor.AncestorRef.Kind != nil && *ancestor.AncestorRef.Kind == key.Kind && ancestor.AncestorRef.Name == key.Name { ancestor.ControllerName = v1alpha2.GatewayController(config.GetControllerName()) ancestor.Conditions = []metav1.Condition{condition} hasAncestor = true } + policy.Status.Ancestors[i] = ancestor } if !hasAncestor { ref := v1alpha2.ParentReference{ @@ -113,7 +152,6 @@ func (r *HTTPRouteReconciler) updateHTTPRoutePolicyStatus(key ancestorRefKey, po Conditions: []metav1.Condition{condition}, }) } - return r.Status().Update(context.Background(), &policy) } type conflictChecker struct { diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index 6bc0abad9..d21696065 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -297,11 +297,11 @@ metadata: name: http-route-policy-0 spec: targetRefs: - - group: gateway.apisix.io + - group: gateway.networking.k8s.io kind: HTTPRoute name: httpbin # sectionName: get - - group: gateway.apisix.io + - group: gateway.networking.k8s.io kind: HTTPRoute name: httpbin-1 sectionName: get From d56a25378936d9bdce4fcacb443ea3b7c95f6459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Thu, 17 Apr 2025 16:18:15 +0800 Subject: [PATCH 07/16] Update HTTPRoutePolicy and Reconciler for Improved Handling Remove unnecessary `kind` fields and list map keys in the HTTPRoutePolicy CRD. Add a new generic event channel and update predicates for more efficient event handling in the HTTPRouteReconciler. --- api/v1alpha1/httproutepolicy_types.go | 4 - .../gateway.apisix.io_httproutepolicies.yaml | 5 - internal/controller/httproute_controller.go | 96 ++++++++++++++++++- internal/controller/httproutepolicy.go | 9 +- 4 files changed, 92 insertions(+), 22 deletions(-) diff --git a/api/v1alpha1/httproutepolicy_types.go b/api/v1alpha1/httproutepolicy_types.go index d2fd74a3d..955f5af2c 100644 --- a/api/v1alpha1/httproutepolicy_types.go +++ b/api/v1alpha1/httproutepolicy_types.go @@ -27,10 +27,6 @@ type HTTPRoutePolicySpec struct { // TargetRef identifies an API object (enum: HTTPRoute, Ingress) to apply HTTPRoutePolicy to. // // target references. - // +listType=map - // +listMapKey=group - // +listMapKey=kind - // +listMapKey=name // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=16 TargetRefs []gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName `json:"targetRefs"` diff --git a/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml b/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml index f9afb85c8..41fbba562 100644 --- a/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml +++ b/config/crd/bases/gateway.apisix.io_httproutepolicies.yaml @@ -104,11 +104,6 @@ spec: maxItems: 16 minItems: 1 type: array - x-kubernetes-list-map-keys: - - group - - kind - - name - x-kubernetes-list-type: map vars: items: x-kubernetes-preserve-unknown-fields: true diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 83595708c..d0be59edf 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -9,6 +9,7 @@ import ( "github.com/api7/api7-ingress-controller/internal/controller/indexer" "github.com/api7/api7-ingress-controller/internal/provider" "github.com/go-logr/logr" + "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -21,6 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -33,10 +35,14 @@ type HTTPRouteReconciler struct { //nolint:revive Log logr.Logger Provider provider.Provider + + genericEvent chan event.GenericEvent } // SetupWithManager sets up the controller with the Manager. func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { + r.genericEvent = make(chan event.GenericEvent, 100) + return ctrl.NewControllerManagedBy(mgr). For(&gatewayv1.HTTPRoute{}). WithEventFilter(predicate.GenerationChangedPredicate{}). @@ -67,10 +73,56 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { ). Watches(&v1alpha1.HTTPRoutePolicy{}, handler.EnqueueRequestsFromMapFunc(r.listHTTPRouteByHTTPRoutePolicy), + builder.WithPredicates( + predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return true + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return true + }, + UpdateFunc: r.httpRoutePolicyPredicateOnUpdate, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, + }, + ), + ). + WatchesRawSource( + source.Channel( + r.genericEvent, + handler.EnqueueRequestsFromMapFunc(r.listHTTPRouteForGenericEvent), + ), ). Complete(r) } +func (r *HTTPRouteReconciler) httpRoutePolicyPredicateOnUpdate(e event.UpdateEvent) bool { + oldPolicy, ok0 := e.ObjectOld.(*v1alpha1.HTTPRoutePolicy) + newPolicy, ok1 := e.ObjectNew.(*v1alpha1.HTTPRoutePolicy) + if !ok0 || !ok1 { + return false + } + var discardsRefs = make(map[string]v1alpha2.LocalPolicyTargetReferenceWithSectionName) + for _, ref := range oldPolicy.Spec.TargetRefs { + key := indexer.GenHTTPRoutePolicyIndexKey(string(ref.Group), string(ref.Kind), e.ObjectOld.GetNamespace(), string(ref.Name), "") + discardsRefs[key] = ref + } + for _, ref := range newPolicy.Spec.TargetRefs { + key := indexer.GenHTTPRoutePolicyIndexKey(string(ref.Group), string(ref.Kind), e.ObjectOld.GetNamespace(), string(ref.Name), "") + delete(discardsRefs, key) + } + if len(discardsRefs) > 0 { + dump := oldPolicy.DeepCopy() + dump.Spec.TargetRefs = make([]v1alpha2.LocalPolicyTargetReferenceWithSectionName, 0, len(discardsRefs)) + for _, ref := range discardsRefs { + dump.Spec.TargetRefs = append(dump.Spec.TargetRefs, ref) + } + r.genericEvent <- event.GenericEvent{Object: dump} + } + return true +} + func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { hr := new(gatewayv1.HTTPRoute) if err := r.Get(ctx, req.NamespacedName, hr); err != nil { @@ -264,7 +316,6 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context if ref.Kind == "HTTPRoute" { key := ancestorRefKey{ Group: gatewayv1.GroupName, - Kind: "HTTPRoute", Namespace: gatewayv1.Namespace(obj.GetNamespace()), Name: ref.Name, } @@ -277,11 +328,25 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context for key := range keys { var httpRoute gatewayv1.HTTPRoute if err := r.Get(ctx, client.ObjectKey{Namespace: string(key.Namespace), Name: string(key.Name)}, &httpRoute); err != nil { - r.Log.Error(err, "failed to get HTTPRoute by HTTPRoutePolicy targetRef", "namespace", obj.GetNamespace(), "name", obj.GetName()) - r.modifyHTTPRoutePolicyStatus(key, httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found HTTPRoute") + r.Log.Error(err, "failed to get HTTPRoute by HTTPRoutePolicy targetRef", "namespace", key.Namespace, "name", key.Name) + r.modifyHTTPRoutePolicyStatus(key, httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found target HTTPRoute") r.Log.Info("status after modified", "key", key, "status", httpRoutePolicy.Status.Ancestors) continue } + if key.SectionName != "" { + var matchRuleName bool + for _, rule := range httpRoute.Spec.Rules { + if rule.Name != nil && *rule.Name == key.SectionName { + matchRuleName = true + break + } + } + if !matchRuleName { + r.Log.Error(errors.Errorf("failed to get HTTPRoute rule by HTTPRoutePolicy targetRef"), "namespace", key.Namespace, "name", key.Name, "sectionName", key.SectionName) + r.modifyHTTPRoutePolicyStatus(key, httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found target HTTPRoute rule") + continue + } + } requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{ Namespace: string(key.Namespace), @@ -293,13 +358,34 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context r.Log.Info("status before clear", "status", httpRoutePolicy.Status.Ancestors) r.clearHTTPRoutePolicyRedundantAncestor(httpRoutePolicy) r.Log.Info("status after clear", "status", httpRoutePolicy.Status.Ancestors) - if err := r.Status().Update(ctx, httpRoutePolicy); err != nil { - r.Log.Error(err, "failed to update HTTPRoutePolicy status", "namespace", obj.GetNamespace(), "name", obj.GetName()) + if httpRoutePolicy.GetDeletionTimestamp().IsZero() { + if err := r.Status().Update(ctx, httpRoutePolicy); err != nil { + r.Log.Error(err, "failed to update HTTPRoutePolicy status", "namespace", obj.GetNamespace(), "name", obj.GetName()) + } } return requests } +func (r *HTTPRouteReconciler) listHTTPRouteForGenericEvent(ctx context.Context, obj client.Object) []reconcile.Request { + switch v := obj.(type) { + case *v1alpha1.HTTPRoutePolicy: + var ( + namespacedNames = make(map[types.NamespacedName]struct{}) + requests []reconcile.Request + ) + for _, ref := range v.Spec.TargetRefs { + namespacedName := types.NamespacedName{Namespace: v.GetNamespace(), Name: string(ref.Name)} + if _, ok := namespacedNames[namespacedName]; !ok { + namespacedNames[namespacedName] = struct{}{} + requests = append(requests, reconcile.Request{NamespacedName: namespacedName}) + } + } + return requests + } + return nil +} + func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.TranslateContext) error { var terr error for _, backend := range tctx.BackendRefs { diff --git a/internal/controller/httproutepolicy.go b/internal/controller/httproutepolicy.go index 39d21e9df..955ee0df5 100644 --- a/internal/controller/httproutepolicy.go +++ b/internal/controller/httproutepolicy.go @@ -84,7 +84,6 @@ func (r *HTTPRouteReconciler) clearHTTPRoutePolicyRedundantAncestor(policy *v1al for _, ref := range policy.Spec.TargetRefs { key := ancestorRefKey{ Group: ref.Group, - Kind: ref.Kind, Namespace: gatewayv1.Namespace(policy.GetNamespace()), Name: ref.Name, } @@ -103,9 +102,6 @@ func (r *HTTPRouteReconciler) clearHTTPRoutePolicyRedundantAncestor(policy *v1al if ancestor.AncestorRef.Group != nil { key.Group = *ancestor.AncestorRef.Group } - if ancestor.AncestorRef.Kind != nil { - key.Kind = *ancestor.AncestorRef.Kind - } if ancestor.AncestorRef.SectionName != nil { key.SectionName = *ancestor.AncestorRef.SectionName } @@ -129,7 +125,7 @@ func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(key ancestorRefKey, po } var hasAncestor bool for i, ancestor := range policy.Status.Ancestors { - if ancestor.AncestorRef.Kind != nil && *ancestor.AncestorRef.Kind == key.Kind && ancestor.AncestorRef.Name == key.Name { + if ancestor.AncestorRef.Name == key.Name { ancestor.ControllerName = v1alpha2.GatewayController(config.GetControllerName()) ancestor.Conditions = []metav1.Condition{condition} hasAncestor = true @@ -139,7 +135,6 @@ func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(key ancestorRefKey, po if !hasAncestor { ref := v1alpha2.ParentReference{ Group: &key.Group, - Kind: &key.Kind, Namespace: &key.Namespace, Name: key.Name, } @@ -162,7 +157,6 @@ type conflictChecker struct { type ancestorRefKey struct { Group gatewayv1.Group - Kind gatewayv1.Kind Namespace gatewayv1.Namespace Name gatewayv1.ObjectName SectionName gatewayv1.SectionName @@ -171,7 +165,6 @@ type ancestorRefKey struct { func (c *conflictChecker) append(sectionName string, policy v1alpha1.HTTPRoutePolicy) { key := ancestorRefKey{ Group: gatewayv1.GroupName, - Kind: "HTTPRoute", Namespace: gatewayv1.Namespace(c.httpRoute.GetNamespace()), Name: gatewayv1.ObjectName(c.httpRoute.GetName()), SectionName: gatewayv1.SectionName(sectionName), From 4f25e924d6f8dc95faa9a96a04a9fcc9b7614ffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Thu, 17 Apr 2025 17:18:24 +0800 Subject: [PATCH 08/16] Refactor HTTPRoutePolicy handling and update tests This commit refactors the handling of HTTPRoutePolicy, including moving the `httpRoutePolicyPredicateOnUpdate` function and updating the type definitions. It also updates the e2e test for HTTPRoute to correct a typo in the test description. --- api/v1alpha1/httproutepolicy_types.go | 4 +- internal/controller/httproute_controller.go | 69 ++++++++---------- internal/controller/httproutepolicy.go | 80 +++------------------ test/e2e/gatewayapi/httproute.go | 2 +- 4 files changed, 43 insertions(+), 112 deletions(-) diff --git a/api/v1alpha1/httproutepolicy_types.go b/api/v1alpha1/httproutepolicy_types.go index 955f5af2c..e307b4f31 100644 --- a/api/v1alpha1/httproutepolicy_types.go +++ b/api/v1alpha1/httproutepolicy_types.go @@ -43,8 +43,8 @@ type HTTPRoutePolicy struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - Spec HTTPRoutePolicySpec `json:"spec,omitempty"` - Status gatewayv1alpha2.PolicyStatus `json:"status,omitempty"` + Spec HTTPRoutePolicySpec `json:"spec,omitempty"` + Status PolicyStatus `json:"status,omitempty"` } // +kubebuilder:object:root=true diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index d0be59edf..3958b394f 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -97,32 +97,6 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -func (r *HTTPRouteReconciler) httpRoutePolicyPredicateOnUpdate(e event.UpdateEvent) bool { - oldPolicy, ok0 := e.ObjectOld.(*v1alpha1.HTTPRoutePolicy) - newPolicy, ok1 := e.ObjectNew.(*v1alpha1.HTTPRoutePolicy) - if !ok0 || !ok1 { - return false - } - var discardsRefs = make(map[string]v1alpha2.LocalPolicyTargetReferenceWithSectionName) - for _, ref := range oldPolicy.Spec.TargetRefs { - key := indexer.GenHTTPRoutePolicyIndexKey(string(ref.Group), string(ref.Kind), e.ObjectOld.GetNamespace(), string(ref.Name), "") - discardsRefs[key] = ref - } - for _, ref := range newPolicy.Spec.TargetRefs { - key := indexer.GenHTTPRoutePolicyIndexKey(string(ref.Group), string(ref.Kind), e.ObjectOld.GetNamespace(), string(ref.Name), "") - delete(discardsRefs, key) - } - if len(discardsRefs) > 0 { - dump := oldPolicy.DeepCopy() - dump.Spec.TargetRefs = make([]v1alpha2.LocalPolicyTargetReferenceWithSectionName, 0, len(discardsRefs)) - for _, ref := range discardsRefs { - dump.Spec.TargetRefs = append(dump.Spec.TargetRefs, ref) - } - r.genericEvent <- event.GenericEvent{Object: dump} - } - return true -} - func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { hr := new(gatewayv1.HTTPRoute) if err := r.Get(ctx, req.NamespacedName, hr); err != nil { @@ -221,6 +195,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.Status().Update(ctx, hr); err != nil { return ctrl.Result{}, err } + UpdateStatus(r.Client, r.Log, tctx) return ctrl.Result{}, nil } @@ -311,10 +286,10 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context return nil } - var keys = make(map[ancestorRefKey]struct{}) + var keys = make(map[targetRefKey]struct{}) for _, ref := range httpRoutePolicy.Spec.TargetRefs { if ref.Kind == "HTTPRoute" { - key := ancestorRefKey{ + key := targetRefKey{ Group: gatewayv1.GroupName, Namespace: gatewayv1.Namespace(obj.GetNamespace()), Name: ref.Name, @@ -329,8 +304,6 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context var httpRoute gatewayv1.HTTPRoute if err := r.Get(ctx, client.ObjectKey{Namespace: string(key.Namespace), Name: string(key.Name)}, &httpRoute); err != nil { r.Log.Error(err, "failed to get HTTPRoute by HTTPRoutePolicy targetRef", "namespace", key.Namespace, "name", key.Name) - r.modifyHTTPRoutePolicyStatus(key, httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found target HTTPRoute") - r.Log.Info("status after modified", "key", key, "status", httpRoutePolicy.Status.Ancestors) continue } if key.SectionName != "" { @@ -343,7 +316,6 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context } if !matchRuleName { r.Log.Error(errors.Errorf("failed to get HTTPRoute rule by HTTPRoutePolicy targetRef"), "namespace", key.Namespace, "name", key.Name, "sectionName", key.SectionName) - r.modifyHTTPRoutePolicyStatus(key, httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found target HTTPRoute rule") continue } } @@ -355,15 +327,6 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context }) } - r.Log.Info("status before clear", "status", httpRoutePolicy.Status.Ancestors) - r.clearHTTPRoutePolicyRedundantAncestor(httpRoutePolicy) - r.Log.Info("status after clear", "status", httpRoutePolicy.Status.Ancestors) - if httpRoutePolicy.GetDeletionTimestamp().IsZero() { - if err := r.Status().Update(ctx, httpRoutePolicy); err != nil { - r.Log.Error(err, "failed to update HTTPRoutePolicy status", "namespace", obj.GetNamespace(), "name", obj.GetName()) - } - } - return requests } @@ -502,3 +465,29 @@ func (r *HTTPRouteReconciler) processHTTPRoute(tctx *provider.TranslateContext, return terror } + +func (r *HTTPRouteReconciler) httpRoutePolicyPredicateOnUpdate(e event.UpdateEvent) bool { + oldPolicy, ok0 := e.ObjectOld.(*v1alpha1.HTTPRoutePolicy) + newPolicy, ok1 := e.ObjectNew.(*v1alpha1.HTTPRoutePolicy) + if !ok0 || !ok1 { + return false + } + var discardsRefs = make(map[string]v1alpha2.LocalPolicyTargetReferenceWithSectionName) + for _, ref := range oldPolicy.Spec.TargetRefs { + key := indexer.GenHTTPRoutePolicyIndexKey(string(ref.Group), string(ref.Kind), e.ObjectOld.GetNamespace(), string(ref.Name), "") + discardsRefs[key] = ref + } + for _, ref := range newPolicy.Spec.TargetRefs { + key := indexer.GenHTTPRoutePolicyIndexKey(string(ref.Group), string(ref.Kind), e.ObjectOld.GetNamespace(), string(ref.Name), "") + delete(discardsRefs, key) + } + if len(discardsRefs) > 0 { + dump := oldPolicy.DeepCopy() + dump.Spec.TargetRefs = make([]v1alpha2.LocalPolicyTargetReferenceWithSectionName, 0, len(discardsRefs)) + for _, ref := range discardsRefs { + dump.Spec.TargetRefs = append(dump.Spec.TargetRefs, ref) + } + r.genericEvent <- event.GenericEvent{Object: dump} + } + return true +} diff --git a/internal/controller/httproutepolicy.go b/internal/controller/httproutepolicy.go index 955ee0df5..96b98f393 100644 --- a/internal/controller/httproutepolicy.go +++ b/internal/controller/httproutepolicy.go @@ -2,11 +2,9 @@ package controller import ( "context" - "slices" "time" "github.com/api7/api7-ingress-controller/api/v1alpha1" - "github.com/api7/api7-ingress-controller/internal/controller/config" "github.com/api7/api7-ingress-controller/internal/controller/indexer" "github.com/api7/api7-ingress-controller/internal/provider" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -20,7 +18,7 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC var ( checker = conflictChecker{ httpRoute: httpRoute, - policies: make(map[ancestorRefKey][]v1alpha1.HTTPRoutePolicy), + policies: make(map[targetRefKey][]v1alpha1.HTTPRoutePolicy), } listForAllRules v1alpha1.HTTPRoutePolicyList key = indexer.GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, "HTTPRoute", httpRoute.GetNamespace(), httpRoute.GetName(), "") @@ -67,51 +65,18 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC tctx.HTTPRoutePolicies = make(map[string][]v1alpha1.HTTPRoutePolicy) } - for key, policies := range checker.policies { - for _, policy := range policies { - r.modifyHTTPRoutePolicyStatus(key, &policy, status, reason, message) - if err := r.Status().Update(context.Background(), &policy); err != nil { - r.Log.Error(err, "failed to update HTTPRoutePolicyStatus") - } + for _, policies := range checker.policies { + for i := range policies { + policy := policies[i] + r.modifyHTTPRoutePolicyStatus(httpRoute, &policy, status, reason, message) + tctx.StatusUpdaters = append(tctx.StatusUpdaters, &policy) } } return nil } -func (r *HTTPRouteReconciler) clearHTTPRoutePolicyRedundantAncestor(policy *v1alpha1.HTTPRoutePolicy) { - var keys = make(map[ancestorRefKey]struct{}) - for _, ref := range policy.Spec.TargetRefs { - key := ancestorRefKey{ - Group: ref.Group, - Namespace: gatewayv1.Namespace(policy.GetNamespace()), - Name: ref.Name, - } - if ref.SectionName != nil { - key.SectionName = *ref.SectionName - } - r.Log.Info("clearHTTPRoutePolicyRedundantAncestor", "keys[]", key) - keys[key] = struct{}{} - } - - policy.Status.Ancestors = slices.DeleteFunc(policy.Status.Ancestors, func(ancestor v1alpha2.PolicyAncestorStatus) bool { - key := ancestorRefKey{ - Namespace: gatewayv1.Namespace(policy.GetNamespace()), - Name: ancestor.AncestorRef.Name, - } - if ancestor.AncestorRef.Group != nil { - key.Group = *ancestor.AncestorRef.Group - } - if ancestor.AncestorRef.SectionName != nil { - key.SectionName = *ancestor.AncestorRef.SectionName - } - r.Log.Info("clearHTTPRoutePolicyRedundantAncestor", "key", key) - _, ok := keys[key] - return !ok - }) -} - -func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(key ancestorRefKey, policy *v1alpha1.HTTPRoutePolicy, status bool, reason, message string) { +func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(httpRoute *gatewayv1.HTTPRoute, policy *v1alpha1.HTTPRoutePolicy, status bool, reason, message string) { condition := metav1.Condition{ Type: string(v1alpha2.PolicyConditionAccepted), Status: metav1.ConditionTrue, @@ -123,39 +88,16 @@ func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(key ancestorRefKey, po if !status { condition.Status = metav1.ConditionFalse } - var hasAncestor bool - for i, ancestor := range policy.Status.Ancestors { - if ancestor.AncestorRef.Name == key.Name { - ancestor.ControllerName = v1alpha2.GatewayController(config.GetControllerName()) - ancestor.Conditions = []metav1.Condition{condition} - hasAncestor = true - } - policy.Status.Ancestors[i] = ancestor - } - if !hasAncestor { - ref := v1alpha2.ParentReference{ - Group: &key.Group, - Namespace: &key.Namespace, - Name: key.Name, - } - if key.SectionName != "" { - ref.SectionName = &key.SectionName - } - policy.Status.Ancestors = append(policy.Status.Ancestors, v1alpha2.PolicyAncestorStatus{ - AncestorRef: ref, - ControllerName: v1alpha2.GatewayController(config.GetControllerName()), - Conditions: []metav1.Condition{condition}, - }) - } + _ = SetAncestors(&policy.Status, httpRoute.Spec.ParentRefs, condition) } type conflictChecker struct { httpRoute *gatewayv1.HTTPRoute - policies map[ancestorRefKey][]v1alpha1.HTTPRoutePolicy + policies map[targetRefKey][]v1alpha1.HTTPRoutePolicy conflict bool } -type ancestorRefKey struct { +type targetRefKey struct { Group gatewayv1.Group Namespace gatewayv1.Namespace Name gatewayv1.ObjectName @@ -163,7 +105,7 @@ type ancestorRefKey struct { } func (c *conflictChecker) append(sectionName string, policy v1alpha1.HTTPRoutePolicy) { - key := ancestorRefKey{ + key := targetRefKey{ Group: gatewayv1.GroupName, Namespace: gatewayv1.Namespace(c.httpRoute.GetNamespace()), Name: gatewayv1.ObjectName(c.httpRoute.GetName()), diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index d21696065..03f64d353 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -421,7 +421,7 @@ spec: Status(404) }) - FIt("HTTPRoute Vars Match", func() { + It("HTTPRoute Vars Match", func() { By("create HTTPRoute") ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1) From 9d83358612a5d42853ec46c49dc4f42bc40ebf35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Thu, 17 Apr 2025 17:30:07 +0800 Subject: [PATCH 09/16] Reorganize import statements for consistency. The import statements in `httproute_controller.go`, `httproutepolicy.go`, and `httproute.go` have been reordered to improve readability and maintain a consistent style across the files. --- internal/controller/httproute_controller.go | 7 ++++--- internal/controller/httproutepolicy.go | 7 ++++--- internal/provider/adc/translator/httproute.go | 3 ++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 3958b394f..c59bd2434 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -5,9 +5,6 @@ import ( "fmt" "strings" - "github.com/api7/api7-ingress-controller/api/v1alpha1" - "github.com/api7/api7-ingress-controller/internal/controller/indexer" - "github.com/api7/api7-ingress-controller/internal/provider" "github.com/go-logr/logr" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -25,6 +22,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/api7/api7-ingress-controller/api/v1alpha1" + "github.com/api7/api7-ingress-controller/internal/controller/indexer" + "github.com/api7/api7-ingress-controller/internal/provider" ) // HTTPRouteReconciler reconciles a GatewayClass object. diff --git a/internal/controller/httproutepolicy.go b/internal/controller/httproutepolicy.go index 96b98f393..ea66e4b03 100644 --- a/internal/controller/httproutepolicy.go +++ b/internal/controller/httproutepolicy.go @@ -4,13 +4,14 @@ import ( "context" "time" - "github.com/api7/api7-ingress-controller/api/v1alpha1" - "github.com/api7/api7-ingress-controller/internal/controller/indexer" - "github.com/api7/api7-ingress-controller/internal/provider" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/api7/api7-ingress-controller/api/v1alpha1" + "github.com/api7/api7-ingress-controller/internal/controller/indexer" + "github.com/api7/api7-ingress-controller/internal/provider" ) func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateContext, httpRoute *gatewayv1.HTTPRoute) error { diff --git a/internal/provider/adc/translator/httproute.go b/internal/provider/adc/translator/httproute.go index 12ea05847..312824565 100644 --- a/internal/provider/adc/translator/httproute.go +++ b/internal/provider/adc/translator/httproute.go @@ -5,7 +5,6 @@ import ( "fmt" "strings" - "github.com/api7/gopkg/pkg/log" "github.com/pkg/errors" "go.uber.org/zap" discoveryv1 "k8s.io/api/discovery/v1" @@ -13,6 +12,8 @@ import ( "k8s.io/utils/ptr" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "github.com/api7/gopkg/pkg/log" + adctypes "github.com/api7/api7-ingress-controller/api/adc" "github.com/api7/api7-ingress-controller/internal/controller/label" "github.com/api7/api7-ingress-controller/internal/id" From bd32c5597600b5e9defc9d463d4535d15df72028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Thu, 17 Apr 2025 17:46:17 +0800 Subject: [PATCH 10/16] resolve comments --- internal/controller/httproute_controller.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index c59bd2434..1c8187dd5 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -342,6 +342,10 @@ func (r *HTTPRouteReconciler) listHTTPRouteForGenericEvent(ctx context.Context, namespacedName := types.NamespacedName{Namespace: v.GetNamespace(), Name: string(ref.Name)} if _, ok := namespacedNames[namespacedName]; !ok { namespacedNames[namespacedName] = struct{}{} + if err := r.Get(ctx, namespacedName, new(gatewayv1.HTTPRoute)); err != nil { + r.Log.Info("failed to Get HTTPRoute", "namespace", namespacedName.Namespace, "name", namespacedName.Name) + continue + } requests = append(requests, reconcile.Request{NamespacedName: namespacedName}) } } From e1186f1caf85b602b31d68f31693257ba078003a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Fri, 18 Apr 2025 14:54:12 +0800 Subject: [PATCH 11/16] enhance policy handling --- charts/values.yaml | 2 +- config/samples/config.yaml | 2 +- docs/configure.md | 2 +- internal/controller/httproutepolicy.go | 18 +++ internal/controller/policies.go | 48 ++++-- test/e2e/crds/consumer.go | 2 +- test/e2e/gatewayapi/httproute.go | 203 ++++++++++++++++++++++++- 7 files changed, 258 insertions(+), 19 deletions(-) diff --git a/charts/values.yaml b/charts/values.yaml index 317896489..6b7122770 100644 --- a/charts/values.yaml +++ b/charts/values.yaml @@ -2,7 +2,7 @@ nameOverride: "" labelsOverride: {} annotations: {} podAnnotations: {} -controllerName: gateway.api7.io/api7-ingress-controller +controllerName: gateway.apisix.io/api7-ingress-controller replicas: 1 admin: key: '' # Pass the admin key generated for the ingress gateway group diff --git a/config/samples/config.yaml b/config/samples/config.yaml index 79fd9977d..c6af5d5f7 100644 --- a/config/samples/config.yaml +++ b/config/samples/config.yaml @@ -1,7 +1,7 @@ log_level: "info" # The log level of the API7 Ingress Controller. # the default value is "info". -controller_name: gateway.api7.io/api7-ingress-controller # The controller name of the API7 Ingress Controller, +controller_name: gateway.apisix.io/api7-ingress-controller # The controller name of the API7 Ingress Controller, # which is used to identify the controller in the GatewayClass. # The default value is "gateway.api7.io/api7-ingress-controller". leader_election_id: "api7-ingress-controller-leader" # The leader election ID for the API7 Ingress Controller. diff --git a/docs/configure.md b/docs/configure.md index 3056257e9..cb3215022 100644 --- a/docs/configure.md +++ b/docs/configure.md @@ -8,7 +8,7 @@ The API7 Ingress Controller is a Kubernetes Ingress Controller that implements t log_level: "info" # The log level of the API7 Ingress Controller. # the default value is "info". -controller_name: gateway.api7.io/api7-ingress-controller # The controller name of the API7 Ingress Controller, +controller_name: gateway.apisix.io/api7-ingress-controller # The controller name of the API7 Ingress Controller, # which is used to identify the controller in the GatewayClass. # The default value is "gateway.api7.io/api7-ingress-controller". diff --git a/internal/controller/httproutepolicy.go b/internal/controller/httproutepolicy.go index ea66e4b03..405aa2441 100644 --- a/internal/controller/httproutepolicy.go +++ b/internal/controller/httproutepolicy.go @@ -52,6 +52,24 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC } } + // todo: unreachable + // if the HTTPRoute is deleted, clear tctx.HTTPRoutePolicies and delete Ancestors from HTTPRoutePolicies status + // if !httpRoute.GetDeletionTimestamp().IsZero() { + // for _, policies := range checker.policies { + // for i := range policies { + // policy := policies[i] + // _ = DeleteAncestors(&policy.Status, httpRoute.Spec.ParentRefs) + // data, _ := json.Marshal(policy.Status) + // r.Log.Info("policy status after delete ancestor", "data", string(data)) + // if err := r.Status().Update(context.Background(), &policy); err != nil { + // r.Log.Error(err, "failed to Update policy status") + // } + // // tctx.StatusUpdaters = append(tctx.StatusUpdaters, &policy) + // } + // } + // return nil + // } + var ( status = true reason = string(v1alpha2.PolicyReasonAccepted) diff --git a/internal/controller/policies.go b/internal/controller/policies.go index ee1770fa8..54837f5ef 100644 --- a/internal/controller/policies.go +++ b/internal/controller/policies.go @@ -2,19 +2,23 @@ package controller import ( "fmt" + "slices" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/utils/ptr" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - "github.com/api7/api7-ingress-controller/api/v1alpha1" - "github.com/api7/api7-ingress-controller/internal/controller/config" - "github.com/api7/api7-ingress-controller/internal/controller/indexer" - "github.com/api7/api7-ingress-controller/internal/provider" "github.com/go-logr/logr" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/api7/api7-ingress-controller/api/v1alpha1" + "github.com/api7/api7-ingress-controller/internal/controller/config" + "github.com/api7/api7-ingress-controller/internal/controller/indexer" + "github.com/api7/api7-ingress-controller/internal/provider" ) type PolicyTargetKey struct { @@ -115,19 +119,37 @@ func SetAncestors(status *v1alpha1.PolicyStatus, parentRefs []gatewayv1.ParentRe } func SetAncestorStatus(status *v1alpha1.PolicyStatus, ancestorStatus gatewayv1alpha2.PolicyAncestorStatus) bool { + if len(ancestorStatus.Conditions) == 0 { + return false + } + condition := ancestorStatus.Conditions[0] for _, c := range status.Ancestors { - if c.AncestorRef == ancestorStatus.AncestorRef { - if len(c.Conditions) == 0 || len(ancestorStatus.Conditions) == 0 { - c.Conditions = ancestorStatus.Conditions - return true + if parentRefValueEqual(c.AncestorRef, ancestorStatus.AncestorRef) && + c.ControllerName == ancestorStatus.ControllerName { + if !VerifyConditions(&c.Conditions, condition) { + return false } - if c.Conditions[0].ObservedGeneration < ancestorStatus.Conditions[0].ObservedGeneration { - c.Conditions = ancestorStatus.Conditions - return true - } - return false + meta.SetStatusCondition(&c.Conditions, condition) + return true } } status.Ancestors = append(status.Ancestors, ancestorStatus) return true } + +func DeleteAncestors(status *v1alpha1.PolicyStatus, parentRefs []gatewayv1.ParentReference) bool { + var length = len(status.Ancestors) + for _, parentRef := range parentRefs { + status.Ancestors = slices.DeleteFunc(status.Ancestors, func(status gatewayv1alpha2.PolicyAncestorStatus) bool { + return parentRefValueEqual(parentRef, status.AncestorRef) + }) + } + return length != len(status.Ancestors) +} + +func parentRefValueEqual(a, b gatewayv1.ParentReference) bool { + return ptr.Equal(a.Group, b.Group) && + ptr.Equal(a.Kind, b.Kind) && + ptr.Equal(a.Namespace, b.Namespace) && + a.Name == b.Name +} diff --git a/test/e2e/crds/consumer.go b/test/e2e/crds/consumer.go index cb0d0b40f..b9634816b 100644 --- a/test/e2e/crds/consumer.go +++ b/test/e2e/crds/consumer.go @@ -67,7 +67,7 @@ spec: filters: - type: ExtensionRef extensionRef: - group: gateway.api7.io + group: gateway.apisix.io kind: PluginConfig name: auth-plugin-config backendRefs: diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index 03f64d353..e3b394db8 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -6,8 +6,10 @@ import ( "strings" "time" + "github.com/gruntwork-io/terratest/modules/retry" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/pkg/errors" "github.com/api7/api7-ingress-controller/test/e2e/framework" "github.com/api7/api7-ingress-controller/test/e2e/scaffold" @@ -290,7 +292,7 @@ spec: - name: httpbin-service-e2e-test port: 80 ` - var httpRoutePolicy = ` + const httpRoutePolicy = ` apiVersion: gateway.apisix.io/v1alpha1 kind: HTTPRoutePolicy metadata: @@ -438,6 +440,18 @@ spec: WithHeader("X-Route-Name", "httpbin"). Expect(). Status(http.StatusOK) + }) + + It("HTTPRoutePolicy in effect", func() { + By("create HTTPRoute") + ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1) + + s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + Expect(). + Status(http.StatusOK) By("create HTTPRoutePolicy") ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", httpRoutePolicy, 1) @@ -458,6 +472,191 @@ spec: WithQuery("hrp_name", "http-route-policy-0"). Expect(). Status(http.StatusOK) + + By("update HTTPRoutePolicy") + const changedHTTPRoutePolicy = ` +apiVersion: gateway.apisix.io/v1alpha1 +kind: HTTPRoutePolicy +metadata: + name: http-route-policy-0 +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httpbin + # sectionName: get + priority: 10 + vars: + - - http_x_hrp_name + - == + - new-hrp-name +` + ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", changedHTTPRoutePolicy, 1) + // use the old vars cannot match any route + s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + WithHeader("X-HRP-Name", "http-route-policy-0"). + WithQuery("hrp_name", "http-route-policy-0"). + Expect(). + Status(http.StatusNotFound) + // use the new vars can match the route + s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + WithHeader("X-HRP-Name", "new-hrp-name"). + Expect(). + Status(http.StatusOK) + + By("delete the HTTPRoutePolicy") + err := s.DeleteResource("HTTPRoutePolicy", "http-route-policy-0") + Expect(err).NotTo(HaveOccurred(), "deleting HTTPRoutePolicy") + Eventually(func() string { + _, err := s.GetResourceYaml("HTTPRoutePolicy", "http-route-policy-0") + return err.Error() + }).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(ContainSubstring(`httproutepolicies.gateway.apisix.io "http-route-policy-0" not found`)) + // access the route without additional vars should be OK + message := retry.DoWithRetry(s.GinkgoT, "", 10, time.Second, func() (string, error) { + statusCode := s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + Expect().Raw().StatusCode + if statusCode != http.StatusOK { + return "", errors.Errorf("unexpected status code: %v", statusCode) + } + return "request OK", nil + }) + s.Logf(message) + }) + + It("HTTPRoutePolicy conflicts", func() { + const httpRoutePolicy0 = ` +apiVersion: gateway.apisix.io/v1alpha1 +kind: HTTPRoutePolicy +metadata: + name: http-route-policy-0 +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httpbin + priority: 10 + vars: + - - http_x_hrp_name + - == + - http-route-policy-0 +` + const httpRoutePolicy1 = ` +apiVersion: gateway.apisix.io/v1alpha1 +kind: HTTPRoutePolicy +metadata: + name: http-route-policy-1 +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httpbin + priority: 20 + vars: + - - http_x_hrp_name + - == + - http-route-policy-0 +` + const httpRoutePolicy2 = ` +apiVersion: gateway.apisix.io/v1alpha1 +kind: HTTPRoutePolicy +metadata: + name: http-route-policy-2 +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httpbin + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httpbin-1 + priority: 30 + vars: + - - http_x_hrp_name + - == + - http-route-policy-0 +` + By("create HTTPRoute") + ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1) + + By("create HTTPRoutePolices") + for _, spec := range []string{httpRoutePolicy0, httpRoutePolicy1, httpRoutePolicy2} { + err := s.CreateResourceFromString(spec) + Expect(err).NotTo(HaveOccurred(), "creating HTTPRoutePolicy") + } + for _, name := range []string{"http-route-policy-0", "http-route-policy-1", "http-route-policy-2"} { + Eventually(func() string { + spec, err := s.GetResourceYaml("HTTPRoutePolicy", name) + Expect(err).NotTo(HaveOccurred(), "getting HTTPRoutePolicy yaml") + return spec + }).WithTimeout(8 * time.Second).ProbeEvery(time.Second). + Should(ContainSubstring("reason: Conflicted")) + } + + // assert that conflict policies are not in effect + s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + Expect(). + Status(http.StatusOK) + }) + + PIt("HTTPRoutePolicy status changes on HTTPRoute deleting", func() { + By("create HTTPRoute") + ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1) + + By("create HTTPRoutePolicy") + ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", httpRoutePolicy, 1) + + By("access dataplane to check the HTTPRoutePolicy") + s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + Expect(). + Status(http.StatusNotFound) + + s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + WithHeader("X-HRP-Name", "http-route-policy-0"). + WithQuery("hrp_name", "http-route-policy-0"). + Expect(). + Status(http.StatusOK) + + By("delete the HTTPRoute, assert the HTTPRoutePolicy's status will be changed") + err := s.DeleteResource("HTTPRoute", "httpbin") + Expect(err).NotTo(HaveOccurred(), "deleting HTTPRoute") + message := retry.DoWithRetry(s.GinkgoT, "request the deleted route", 10, time.Second, func() (string, error) { + statusCode := s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + WithHeader("X-HRP-Name", "http-route-policy-0"). + WithQuery("hrp_name", "http-route-policy-0"). + Expect().Raw().StatusCode + if statusCode != http.StatusNotFound { + return "", errors.Errorf("unexpected status code: %v", statusCode) + } + return "the route is deleted", nil + }) + s.Logf(message) + + Eventually(func() string { + spec, err := s.GetResourceYaml("HTTPRoutePolicy", "http-route-policy-0") + Expect(err).NotTo(HaveOccurred(), "getting HTTPRoutePolicy") + return spec + }).WithTimeout(8 * time.Second).ProbeEvery(time.Second).ShouldNot(ContainSubstring("ancestorRef:")) }) }) @@ -661,7 +860,7 @@ spec: filters: - type: ExtensionRef extensionRef: - group: gateway.api7.io + group: gateway.apisix.io kind: PluginConfig name: example-plugin-config backendRefs: From e20bac6e13b3b653a52b11687dce56a93b2aaeac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Fri, 18 Apr 2025 15:01:08 +0800 Subject: [PATCH 12/16] resolve comments --- internal/controller/httproute_controller.go | 31 ++++++++++----------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 1c8187dd5..f0f291b79 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -287,39 +287,38 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context return nil } - var keys = make(map[targetRefKey]struct{}) + var keys = make(map[types.NamespacedName]struct{}) for _, ref := range httpRoutePolicy.Spec.TargetRefs { - if ref.Kind == "HTTPRoute" { - key := targetRefKey{ - Group: gatewayv1.GroupName, - Namespace: gatewayv1.Namespace(obj.GetNamespace()), - Name: ref.Name, - } - if ref.SectionName != nil { - key.SectionName = *ref.SectionName - } - keys[key] = struct{}{} + if ref.Kind != "HTTPRoute" { + continue } - } - for key := range keys { + key := types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: string(ref.Name), + } + if _, ok := keys[key]; ok { + continue + } + var httpRoute gatewayv1.HTTPRoute if err := r.Get(ctx, client.ObjectKey{Namespace: string(key.Namespace), Name: string(key.Name)}, &httpRoute); err != nil { r.Log.Error(err, "failed to get HTTPRoute by HTTPRoutePolicy targetRef", "namespace", key.Namespace, "name", key.Name) continue } - if key.SectionName != "" { + if ref.SectionName != nil { var matchRuleName bool for _, rule := range httpRoute.Spec.Rules { - if rule.Name != nil && *rule.Name == key.SectionName { + if rule.Name != nil && *rule.Name == *ref.SectionName { matchRuleName = true break } } if !matchRuleName { - r.Log.Error(errors.Errorf("failed to get HTTPRoute rule by HTTPRoutePolicy targetRef"), "namespace", key.Namespace, "name", key.Name, "sectionName", key.SectionName) + r.Log.Error(errors.Errorf("failed to get HTTPRoute rule by HTTPRoutePolicy targetRef"), "namespace", key.Namespace, "name", key.Name, "sectionName", *ref.SectionName) continue } } + keys[key] = struct{}{} requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{ Namespace: string(key.Namespace), From b2ea254296169f8c33b1b31d2c4c724a16ad282f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Fri, 18 Apr 2025 15:05:22 +0800 Subject: [PATCH 13/16] resolve lint --- internal/controller/httproute_controller.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index f0f291b79..471427057 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -301,7 +301,7 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context } var httpRoute gatewayv1.HTTPRoute - if err := r.Get(ctx, client.ObjectKey{Namespace: string(key.Namespace), Name: string(key.Name)}, &httpRoute); err != nil { + if err := r.Get(ctx, client.ObjectKey{Namespace: key.Namespace, Name: key.Name}, &httpRoute); err != nil { r.Log.Error(err, "failed to get HTTPRoute by HTTPRoutePolicy targetRef", "namespace", key.Namespace, "name", key.Name) continue } @@ -321,8 +321,8 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context keys[key] = struct{}{} requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{ - Namespace: string(key.Namespace), - Name: string(key.Name), + Namespace: key.Namespace, + Name: key.Name, }, }) } From 9050538ad226d459b6753e4bb3dcff4a15c848cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Fri, 18 Apr 2025 17:32:27 +0800 Subject: [PATCH 14/16] Update test to use Eventually for status check --- test/e2e/gatewayapi/httproute.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index 56e5ec1a8..b1dcdcc44 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -641,12 +641,13 @@ spec: } // assert that conflict policies are not in effect - s.NewAPISIXClient(). - GET("/get"). - WithHost("httpbin.example"). - WithHeader("X-Route-Name", "httpbin"). - Expect(). - Status(http.StatusOK) + Eventually(func() int { + return s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + Expect().Raw().StatusCode + }).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK)) }) PIt("HTTPRoutePolicy status changes on HTTPRoute deleting", func() { From 80820d67ce32cb85caae839666f3a96a3129dcb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Fri, 18 Apr 2025 18:17:52 +0800 Subject: [PATCH 15/16] fix e2e --- test/e2e/gatewayapi/httproute.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index b1dcdcc44..101674856 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -532,14 +532,16 @@ spec: ` ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", changedHTTPRoutePolicy, 1) // use the old vars cannot match any route - s.NewAPISIXClient(). - GET("/get"). - WithHost("httpbin.example"). - WithHeader("X-Route-Name", "httpbin"). - WithHeader("X-HRP-Name", "http-route-policy-0"). - WithQuery("hrp_name", "http-route-policy-0"). - Expect(). - Status(http.StatusNotFound) + Eventually(func() int { + return s.NewAPISIXClient(). + GET("/get"). + WithHost("httpbin.example"). + WithHeader("X-Route-Name", "httpbin"). + WithHeader("X-HRP-Name", "http-route-policy-0"). + WithQuery("hrp_name", "http-route-policy-0"). + Expect().Raw().StatusCode + }).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusNotFound)) + // use the new vars can match the route s.NewAPISIXClient(). GET("/get"). From ec310a4e51dd45fc95c53a5100cc43d10585908f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Sun, 20 Apr 2025 18:04:06 +0800 Subject: [PATCH 16/16] fix e2e --- Makefile | 2 +- internal/controller/httproutepolicy.go | 7 ++++--- internal/controller/indexer/indexer.go | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 3dada9b39..da8a844b3 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ KIND_NAME ?= api7-ingress-cluster GATEAY_API_VERSION ?= v1.2.0 DASHBOARD_VERSION ?= dev -TEST_TIMEOUT ?= 30m +TEST_TIMEOUT ?= 45m export KUBECONFIG = /tmp/$(KIND_NAME).kubeconfig diff --git a/internal/controller/httproutepolicy.go b/internal/controller/httproutepolicy.go index 405aa2441..9ca9bac14 100644 --- a/internal/controller/httproutepolicy.go +++ b/internal/controller/httproutepolicy.go @@ -5,6 +5,7 @@ import ( "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -22,7 +23,7 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC policies: make(map[targetRefKey][]v1alpha1.HTTPRoutePolicy), } listForAllRules v1alpha1.HTTPRoutePolicyList - key = indexer.GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, "HTTPRoute", httpRoute.GetNamespace(), httpRoute.GetName(), "") + key = indexer.GenHTTPRoutePolicyIndexKey(gatewayv1.GroupName, "HTTPRoute", httpRoute.GetNamespace(), httpRoute.GetName(), "") ) if err := r.List(context.Background(), &listForAllRules, client.MatchingFields{indexer.PolicyTargetRefs: key}); err != nil { return err @@ -41,7 +42,7 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC var ( ruleName = string(*rule.Name) listForSectionRules v1alpha1.HTTPRoutePolicyList - key = indexer.GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, "HTTPRoute", httpRoute.GetNamespace(), httpRoute.GetName(), ruleName) + key = indexer.GenHTTPRoutePolicyIndexKey(gatewayv1.GroupName, "HTTPRoute", httpRoute.GetNamespace(), httpRoute.GetName(), ruleName) ) if err := r.List(context.Background(), &listForSectionRules, client.MatchingFields{indexer.PolicyTargetRefs: key}); err != nil { continue @@ -136,7 +137,7 @@ func (c *conflictChecker) append(sectionName string, policy v1alpha1.HTTPRoutePo Loop: for _, items := range c.policies { for _, item := range items { - if item.Spec.Priority != policy.Spec.Priority || *item.Spec.Priority != *policy.Spec.Priority { + if !ptr.Equal(item.Spec.Priority, policy.Spec.Priority) { c.conflict = true break Loop } diff --git a/internal/controller/indexer/indexer.go b/internal/controller/indexer/indexer.go index 101f9819d..7a606b274 100644 --- a/internal/controller/indexer/indexer.go +++ b/internal/controller/indexer/indexer.go @@ -334,7 +334,7 @@ func HTTPRoutePolicyIndexFunc(rawObj client.Object) []string { if ref.SectionName != nil { sectionName = string(*ref.SectionName) } - keys = append(keys, GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, string(ref.Kind), hrp.GetNamespace(), string(ref.Name), sectionName)) + keys = append(keys, GenHTTPRoutePolicyIndexKey(string(ref.Group), string(ref.Kind), hrp.GetNamespace(), string(ref.Name), sectionName)) } return keys }