diff --git a/api/v1beta2/ratelimitpolicy_types.go b/api/v1beta2/ratelimitpolicy_types.go index 5788c4b1a..7b509c027 100644 --- a/api/v1beta2/ratelimitpolicy_types.go +++ b/api/v1beta2/ratelimitpolicy_types.go @@ -122,6 +122,9 @@ func (l Limit) CountersAsStringList() []string { // RateLimitPolicySpec defines the desired state of RateLimitPolicy // +kubebuilder:validation:XValidation:rule="self.targetRef.kind != 'Gateway' || !has(self.limits) || !self.limits.exists(x, has(self.limits[x].routeSelectors))",message="route selectors not supported when targeting a Gateway" // +kubebuilder:validation:XValidation:rule="!(has(self.defaults) && has(self.limits))",message="Implicit and explicit defaults are mutually exclusive" +// +kubebuilder:validation:XValidation:rule="!(has(self.defaults) && has(self.overrides))",message="Overrides and explicit defaults are mutually exclusive" +// +kubebuilder:validation:XValidation:rule="!(has(self.overrides) && has(self.limits))",message="Overrides and implicit defaults are mutually exclusive" +// +kubebuilder:validation:XValidation:rule="!(has(self.overrides) && self.targetRef.kind != 'Gateway')",message="Overrides are only allowed for policies targeting a Gateway resource" type RateLimitPolicySpec struct { // TargetRef identifies an API object to apply policy to. // +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'",message="Invalid targetRef.group. The only supported value is 'gateway.networking.k8s.io'" @@ -133,6 +136,11 @@ type RateLimitPolicySpec struct { // +optional Defaults *RateLimitPolicyCommonSpec `json:"defaults,omitempty"` + // Overrides define override values for this policy and for policies inheriting this policy. + // Overrides are mutually exclusive with implicit defaults and explicit Defaults defined by RateLimitPolicyCommonSpec. + // +optional + Overrides *RateLimitPolicyCommonSpec `json:"overrides,omitempty"` + // RateLimitPolicyCommonSpec defines implicit default values for this policy and for policies inheriting this policy. // RateLimitPolicyCommonSpec is mutually exclusive with explicit defaults defined by Defaults. RateLimitPolicyCommonSpec `json:""` @@ -286,6 +294,10 @@ func (r *RateLimitPolicySpec) CommonSpec() *RateLimitPolicyCommonSpec { return r.Defaults } + if r.Overrides != nil { + return r.Overrides + } + return &r.RateLimitPolicyCommonSpec } diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 95828e596..747f6278c 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -481,6 +481,11 @@ func (in *RateLimitPolicySpec) DeepCopyInto(out *RateLimitPolicySpec) { *out = new(RateLimitPolicyCommonSpec) (*in).DeepCopyInto(*out) } + if in.Overrides != nil { + in, out := &in.Overrides, &out.Overrides + *out = new(RateLimitPolicyCommonSpec) + (*in).DeepCopyInto(*out) + } in.RateLimitPolicyCommonSpec.DeepCopyInto(&out.RateLimitPolicyCommonSpec) } diff --git a/bundle/manifests/kuadrant.io_ratelimitpolicies.yaml b/bundle/manifests/kuadrant.io_ratelimitpolicies.yaml index 6fb2a0500..76884d89f 100644 --- a/bundle/manifests/kuadrant.io_ratelimitpolicies.yaml +++ b/bundle/manifests/kuadrant.io_ratelimitpolicies.yaml @@ -847,6 +847,400 @@ spec: name maxProperties: 14 type: object + overrides: + description: |- + Overrides define override values for this policy and for policies inheriting this policy. + Overrides are mutually exclusive with implicit defaults and explicit Defaults defined by RateLimitPolicyCommonSpec. + properties: + limits: + additionalProperties: + description: Limit represents a complete rate limit configuration + properties: + counters: + description: |- + Counters defines additional rate limit counters based on context qualifiers and well known selectors + TODO Document properly "Well-known selector" https://github.com/Kuadrant/architecture/blob/main/rfcs/0001-rlp-v2.md#well-known-selectors + items: + description: |- + ContextSelector defines one item from the well known attributes + Attributes: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes + Well-known selectors: https://github.com/Kuadrant/architecture/blob/main/rfcs/0001-rlp-v2.md#well-known-selectors + They are named by a dot-separated path (e.g. request.path) + Example: "request.path" -> The path portion of the URL + maxLength: 253 + minLength: 1 + type: string + type: array + rates: + description: Rates holds the list of limit rates + items: + description: Rate defines the actual rate limit that will + be used when there is a match + properties: + duration: + description: Duration defines the time period for + which the Limit specified above applies. + type: integer + limit: + description: Limit defines the max value allowed for + a given period of time + type: integer + unit: + description: |- + Duration defines the time uni + Possible values are: "second", "minute", "hour", "day" + enum: + - second + - minute + - hour + - day + type: string + required: + - duration + - limit + - unit + type: object + type: array + routeSelectors: + description: RouteSelectors defines semantics for matching + an HTTP request based on conditions + items: + description: |- + RouteSelector defines semantics for matching an HTTP request based on conditions + https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteSpec + properties: + hostnames: + description: |- + Hostnames defines a set of hostname that should match against the HTTP Host header to select a HTTPRoute to process the request + https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteSpec + items: + description: |- + Hostname is the fully qualified domain name of a network host. This matches + the RFC 1123 definition of a hostname with 2 notable exceptions: + + + 1. IPs are not allowed. + 2. A hostname may be prefixed with a wildcard label (`*.`). The wildcard + label must appear by itself as the first label. + + + Hostname can be "precise" which is a domain name without the terminating + dot of a network host (e.g. "foo.example.com") or "wildcard", which is a + domain name prefixed with a single wildcard label (e.g. `*.example.com`). + + + Note that as per RFC1035 and RFC1123, a *label* must consist of lower case + alphanumeric characters or '-', and must start and end with an alphanumeric + character. No other punctuation is allowed. + maxLength: 253 + minLength: 1 + pattern: ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + type: array + matches: + description: |- + Matches define conditions used for matching the rule against incoming HTTP requests. + https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteSpec + items: + description: "HTTPRouteMatch defines the predicate + used to match requests to a given\naction. Multiple + match types are ANDed together, i.e. the match + will\nevaluate to true only if all conditions + are satisfied.\n\n\nFor example, the match below + will match a HTTP request only if its path\nstarts + with `/foo` AND it contains the `version: v1` + header:\n\n\n```\nmatch:\n\n\n\tpath:\n\t value: + \"/foo\"\n\theaders:\n\t- name: \"version\"\n\t + \ value \"v1\"\n\n\n```" + properties: + headers: + description: |- + Headers specifies HTTP request header matchers. Multiple match values are + ANDed together, meaning, a request must match all the specified headers + to select the route. + items: + description: |- + HTTPHeaderMatch describes how to select a HTTP route by matching HTTP request + headers. + properties: + name: + description: |- + Name is the name of the HTTP Header to be matched. Name matching MUST be + case insensitive. (See https://tools.ietf.org/html/rfc7230#section-3.2). + + + If multiple entries specify equivalent header names, only the first + entry with an equivalent name MUST be considered for a match. Subsequent + entries with an equivalent header name MUST be ignored. Due to the + case-insensitivity of header names, "foo" and "Foo" are considered + equivalent. + + + When a header is repeated in an HTTP request, it is + implementation-specific behavior as to how this is represented. + Generally, proxies should follow the guidance from the RFC: + https://www.rfc-editor.org/rfc/rfc7230.html#section-3.2.2 regarding + processing a repeated header, with special handling for "Set-Cookie". + maxLength: 256 + minLength: 1 + pattern: ^[A-Za-z0-9!#$%&'*+\-.^_\x60|~]+$ + type: string + type: + default: Exact + description: |- + Type specifies how to match against the value of the header. + + + Support: Core (Exact) + + + Support: Implementation-specific (RegularExpression) + + + Since RegularExpression HeaderMatchType has implementation-specific + conformance, implementations can support POSIX, PCRE or any other dialects + of regular expressions. Please read the implementation's documentation to + determine the supported dialect. + enum: + - Exact + - RegularExpression + type: string + value: + description: Value is the value of HTTP + Header to be matched. + maxLength: 4096 + minLength: 1 + type: string + required: + - name + - value + type: object + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + method: + description: |- + Method specifies HTTP method matcher. + When specified, this route will be matched only if the request has the + specified method. + + + Support: Extended + enum: + - GET + - HEAD + - POST + - PUT + - DELETE + - CONNECT + - OPTIONS + - TRACE + - PATCH + type: string + path: + default: + type: PathPrefix + value: / + description: |- + Path specifies a HTTP request path matcher. If this field is not + specified, a default prefix match on the "/" path is provided. + properties: + type: + default: PathPrefix + description: |- + Type specifies how to match against the path Value. + + + Support: Core (Exact, PathPrefix) + + + Support: Implementation-specific (RegularExpression) + enum: + - Exact + - PathPrefix + - RegularExpression + type: string + value: + default: / + description: Value of the HTTP path to match + against. + maxLength: 1024 + type: string + type: object + x-kubernetes-validations: + - message: value must be an absolute path and + start with '/' when type one of ['Exact', + 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? self.value.startsWith(''/'') : true' + - message: must not contain '//' when type one + of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''//'') : true' + - message: must not contain '/./' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''/./'') : true' + - message: must not contain '/../' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''/../'') : true' + - message: must not contain '%2f' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''%2f'') : true' + - message: must not contain '%2F' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''%2F'') : true' + - message: must not contain '#' when type one + of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''#'') : true' + - message: must not end with '/..' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.endsWith(''/..'') : true' + - message: must not end with '/.' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.endsWith(''/.'') : true' + - message: type must be one of ['Exact', 'PathPrefix', + 'RegularExpression'] + rule: self.type in ['Exact','PathPrefix'] + || self.type == 'RegularExpression' + - message: must only contain valid characters + (matching ^(?:[-A-Za-z0-9/._~!$&'()*+,;=:@]|[%][0-9a-fA-F]{2})+$) + for types ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? self.value.matches(r"""^(?:[-A-Za-z0-9/._~!$&''()*+,;=:@]|[%][0-9a-fA-F]{2})+$""") + : true' + queryParams: + description: |- + QueryParams specifies HTTP query parameter matchers. Multiple match + values are ANDed together, meaning, a request must match all the + specified query parameters to select the route. + + + Support: Extended + items: + description: |- + HTTPQueryParamMatch describes how to select a HTTP route by matching HTTP + query parameters. + properties: + name: + description: |- + Name is the name of the HTTP query param to be matched. This must be an + exact string match. (See + https://tools.ietf.org/html/rfc7230#section-2.7.3). + + + If multiple entries specify equivalent query param names, only the first + entry with an equivalent name MUST be considered for a match. Subsequent + entries with an equivalent query param name MUST be ignored. + + + If a query param is repeated in an HTTP request, the behavior is + purposely left undefined, since different data planes have different + capabilities. However, it is *recommended* that implementations should + match against the first value of the param if the data plane supports it, + as this behavior is expected in other load balancing contexts outside of + the Gateway API. + + + Users SHOULD NOT route traffic based on repeated query params to guard + themselves against potential differences in the implementations. + maxLength: 256 + minLength: 1 + pattern: ^[A-Za-z0-9!#$%&'*+\-.^_\x60|~]+$ + type: string + type: + default: Exact + description: |- + Type specifies how to match against the value of the query parameter. + + + Support: Extended (Exact) + + + Support: Implementation-specific (RegularExpression) + + + Since RegularExpression QueryParamMatchType has Implementation-specific + conformance, implementations can support POSIX, PCRE or any other + dialects of regular expressions. Please read the implementation's + documentation to determine the supported dialect. + enum: + - Exact + - RegularExpression + type: string + value: + description: Value is the value of HTTP + query param to be matched. + maxLength: 1024 + minLength: 1 + type: string + required: + - name + - value + type: object + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + type: object + maxItems: 8 + type: array + type: object + maxItems: 15 + type: array + when: + description: |- + When holds the list of conditions for the policy to be enforced. + Called also "soft" conditions as route selectors must also match + items: + description: |- + RouteSelector defines semantics for matching an HTTP request based on conditions + https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteSpec + properties: + operator: + description: |- + The binary operator to be applied to the content fetched from the selector + Possible values are: "eq" (equal to), "neq" (not equal to) + enum: + - eq + - neq + - startswith + - endswith + - incl + - excl + - matches + type: string + selector: + description: |- + Selector defines one item from the well known selectors + TODO Document properly "Well-known selector" https://github.com/Kuadrant/architecture/blob/main/rfcs/0001-rlp-v2.md#well-known-selectors + maxLength: 253 + minLength: 1 + type: string + value: + description: The value of reference for the comparison. + type: string + required: + - operator + - selector + - value + type: object + type: array + type: object + description: Limits holds the struct of limits indexed by a unique + name + maxProperties: 14 + type: object + type: object targetRef: description: TargetRef identifies an API object to apply policy to. properties: @@ -896,6 +1290,13 @@ spec: has(self.limits[x].routeSelectors)) - message: Implicit and explicit defaults are mutually exclusive rule: '!(has(self.defaults) && has(self.limits))' + - message: Overrides and explicit defaults are mutually exclusive + rule: '!(has(self.defaults) && has(self.overrides))' + - message: Overrides and implicit defaults are mutually exclusive + rule: '!(has(self.overrides) && has(self.limits))' + - message: Overrides are only allowed for policies targeting a Gateway + resource + rule: '!(has(self.overrides) && self.targetRef.kind != ''Gateway'')' status: description: RateLimitPolicyStatus defines the observed state of RateLimitPolicy properties: diff --git a/config/crd/bases/kuadrant.io_ratelimitpolicies.yaml b/config/crd/bases/kuadrant.io_ratelimitpolicies.yaml index 4b6f276ca..95cebf811 100644 --- a/config/crd/bases/kuadrant.io_ratelimitpolicies.yaml +++ b/config/crd/bases/kuadrant.io_ratelimitpolicies.yaml @@ -846,6 +846,400 @@ spec: name maxProperties: 14 type: object + overrides: + description: |- + Overrides define override values for this policy and for policies inheriting this policy. + Overrides are mutually exclusive with implicit defaults and explicit Defaults defined by RateLimitPolicyCommonSpec. + properties: + limits: + additionalProperties: + description: Limit represents a complete rate limit configuration + properties: + counters: + description: |- + Counters defines additional rate limit counters based on context qualifiers and well known selectors + TODO Document properly "Well-known selector" https://github.com/Kuadrant/architecture/blob/main/rfcs/0001-rlp-v2.md#well-known-selectors + items: + description: |- + ContextSelector defines one item from the well known attributes + Attributes: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes + Well-known selectors: https://github.com/Kuadrant/architecture/blob/main/rfcs/0001-rlp-v2.md#well-known-selectors + They are named by a dot-separated path (e.g. request.path) + Example: "request.path" -> The path portion of the URL + maxLength: 253 + minLength: 1 + type: string + type: array + rates: + description: Rates holds the list of limit rates + items: + description: Rate defines the actual rate limit that will + be used when there is a match + properties: + duration: + description: Duration defines the time period for + which the Limit specified above applies. + type: integer + limit: + description: Limit defines the max value allowed for + a given period of time + type: integer + unit: + description: |- + Duration defines the time uni + Possible values are: "second", "minute", "hour", "day" + enum: + - second + - minute + - hour + - day + type: string + required: + - duration + - limit + - unit + type: object + type: array + routeSelectors: + description: RouteSelectors defines semantics for matching + an HTTP request based on conditions + items: + description: |- + RouteSelector defines semantics for matching an HTTP request based on conditions + https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteSpec + properties: + hostnames: + description: |- + Hostnames defines a set of hostname that should match against the HTTP Host header to select a HTTPRoute to process the request + https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteSpec + items: + description: |- + Hostname is the fully qualified domain name of a network host. This matches + the RFC 1123 definition of a hostname with 2 notable exceptions: + + + 1. IPs are not allowed. + 2. A hostname may be prefixed with a wildcard label (`*.`). The wildcard + label must appear by itself as the first label. + + + Hostname can be "precise" which is a domain name without the terminating + dot of a network host (e.g. "foo.example.com") or "wildcard", which is a + domain name prefixed with a single wildcard label (e.g. `*.example.com`). + + + Note that as per RFC1035 and RFC1123, a *label* must consist of lower case + alphanumeric characters or '-', and must start and end with an alphanumeric + character. No other punctuation is allowed. + maxLength: 253 + minLength: 1 + pattern: ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + type: array + matches: + description: |- + Matches define conditions used for matching the rule against incoming HTTP requests. + https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteSpec + items: + description: "HTTPRouteMatch defines the predicate + used to match requests to a given\naction. Multiple + match types are ANDed together, i.e. the match + will\nevaluate to true only if all conditions + are satisfied.\n\n\nFor example, the match below + will match a HTTP request only if its path\nstarts + with `/foo` AND it contains the `version: v1` + header:\n\n\n```\nmatch:\n\n\n\tpath:\n\t value: + \"/foo\"\n\theaders:\n\t- name: \"version\"\n\t + \ value \"v1\"\n\n\n```" + properties: + headers: + description: |- + Headers specifies HTTP request header matchers. Multiple match values are + ANDed together, meaning, a request must match all the specified headers + to select the route. + items: + description: |- + HTTPHeaderMatch describes how to select a HTTP route by matching HTTP request + headers. + properties: + name: + description: |- + Name is the name of the HTTP Header to be matched. Name matching MUST be + case insensitive. (See https://tools.ietf.org/html/rfc7230#section-3.2). + + + If multiple entries specify equivalent header names, only the first + entry with an equivalent name MUST be considered for a match. Subsequent + entries with an equivalent header name MUST be ignored. Due to the + case-insensitivity of header names, "foo" and "Foo" are considered + equivalent. + + + When a header is repeated in an HTTP request, it is + implementation-specific behavior as to how this is represented. + Generally, proxies should follow the guidance from the RFC: + https://www.rfc-editor.org/rfc/rfc7230.html#section-3.2.2 regarding + processing a repeated header, with special handling for "Set-Cookie". + maxLength: 256 + minLength: 1 + pattern: ^[A-Za-z0-9!#$%&'*+\-.^_\x60|~]+$ + type: string + type: + default: Exact + description: |- + Type specifies how to match against the value of the header. + + + Support: Core (Exact) + + + Support: Implementation-specific (RegularExpression) + + + Since RegularExpression HeaderMatchType has implementation-specific + conformance, implementations can support POSIX, PCRE or any other dialects + of regular expressions. Please read the implementation's documentation to + determine the supported dialect. + enum: + - Exact + - RegularExpression + type: string + value: + description: Value is the value of HTTP + Header to be matched. + maxLength: 4096 + minLength: 1 + type: string + required: + - name + - value + type: object + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + method: + description: |- + Method specifies HTTP method matcher. + When specified, this route will be matched only if the request has the + specified method. + + + Support: Extended + enum: + - GET + - HEAD + - POST + - PUT + - DELETE + - CONNECT + - OPTIONS + - TRACE + - PATCH + type: string + path: + default: + type: PathPrefix + value: / + description: |- + Path specifies a HTTP request path matcher. If this field is not + specified, a default prefix match on the "/" path is provided. + properties: + type: + default: PathPrefix + description: |- + Type specifies how to match against the path Value. + + + Support: Core (Exact, PathPrefix) + + + Support: Implementation-specific (RegularExpression) + enum: + - Exact + - PathPrefix + - RegularExpression + type: string + value: + default: / + description: Value of the HTTP path to match + against. + maxLength: 1024 + type: string + type: object + x-kubernetes-validations: + - message: value must be an absolute path and + start with '/' when type one of ['Exact', + 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? self.value.startsWith(''/'') : true' + - message: must not contain '//' when type one + of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''//'') : true' + - message: must not contain '/./' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''/./'') : true' + - message: must not contain '/../' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''/../'') : true' + - message: must not contain '%2f' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''%2f'') : true' + - message: must not contain '%2F' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''%2F'') : true' + - message: must not contain '#' when type one + of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.contains(''#'') : true' + - message: must not end with '/..' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.endsWith(''/..'') : true' + - message: must not end with '/.' when type + one of ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? !self.value.endsWith(''/.'') : true' + - message: type must be one of ['Exact', 'PathPrefix', + 'RegularExpression'] + rule: self.type in ['Exact','PathPrefix'] + || self.type == 'RegularExpression' + - message: must only contain valid characters + (matching ^(?:[-A-Za-z0-9/._~!$&'()*+,;=:@]|[%][0-9a-fA-F]{2})+$) + for types ['Exact', 'PathPrefix'] + rule: '(self.type in [''Exact'',''PathPrefix'']) + ? self.value.matches(r"""^(?:[-A-Za-z0-9/._~!$&''()*+,;=:@]|[%][0-9a-fA-F]{2})+$""") + : true' + queryParams: + description: |- + QueryParams specifies HTTP query parameter matchers. Multiple match + values are ANDed together, meaning, a request must match all the + specified query parameters to select the route. + + + Support: Extended + items: + description: |- + HTTPQueryParamMatch describes how to select a HTTP route by matching HTTP + query parameters. + properties: + name: + description: |- + Name is the name of the HTTP query param to be matched. This must be an + exact string match. (See + https://tools.ietf.org/html/rfc7230#section-2.7.3). + + + If multiple entries specify equivalent query param names, only the first + entry with an equivalent name MUST be considered for a match. Subsequent + entries with an equivalent query param name MUST be ignored. + + + If a query param is repeated in an HTTP request, the behavior is + purposely left undefined, since different data planes have different + capabilities. However, it is *recommended* that implementations should + match against the first value of the param if the data plane supports it, + as this behavior is expected in other load balancing contexts outside of + the Gateway API. + + + Users SHOULD NOT route traffic based on repeated query params to guard + themselves against potential differences in the implementations. + maxLength: 256 + minLength: 1 + pattern: ^[A-Za-z0-9!#$%&'*+\-.^_\x60|~]+$ + type: string + type: + default: Exact + description: |- + Type specifies how to match against the value of the query parameter. + + + Support: Extended (Exact) + + + Support: Implementation-specific (RegularExpression) + + + Since RegularExpression QueryParamMatchType has Implementation-specific + conformance, implementations can support POSIX, PCRE or any other + dialects of regular expressions. Please read the implementation's + documentation to determine the supported dialect. + enum: + - Exact + - RegularExpression + type: string + value: + description: Value is the value of HTTP + query param to be matched. + maxLength: 1024 + minLength: 1 + type: string + required: + - name + - value + type: object + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + type: object + maxItems: 8 + type: array + type: object + maxItems: 15 + type: array + when: + description: |- + When holds the list of conditions for the policy to be enforced. + Called also "soft" conditions as route selectors must also match + items: + description: |- + RouteSelector defines semantics for matching an HTTP request based on conditions + https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteSpec + properties: + operator: + description: |- + The binary operator to be applied to the content fetched from the selector + Possible values are: "eq" (equal to), "neq" (not equal to) + enum: + - eq + - neq + - startswith + - endswith + - incl + - excl + - matches + type: string + selector: + description: |- + Selector defines one item from the well known selectors + TODO Document properly "Well-known selector" https://github.com/Kuadrant/architecture/blob/main/rfcs/0001-rlp-v2.md#well-known-selectors + maxLength: 253 + minLength: 1 + type: string + value: + description: The value of reference for the comparison. + type: string + required: + - operator + - selector + - value + type: object + type: array + type: object + description: Limits holds the struct of limits indexed by a unique + name + maxProperties: 14 + type: object + type: object targetRef: description: TargetRef identifies an API object to apply policy to. properties: @@ -895,6 +1289,13 @@ spec: has(self.limits[x].routeSelectors)) - message: Implicit and explicit defaults are mutually exclusive rule: '!(has(self.defaults) && has(self.limits))' + - message: Overrides and explicit defaults are mutually exclusive + rule: '!(has(self.defaults) && has(self.overrides))' + - message: Overrides and implicit defaults are mutually exclusive + rule: '!(has(self.overrides) && has(self.limits))' + - message: Overrides are only allowed for policies targeting a Gateway + resource + rule: '!(has(self.overrides) && self.targetRef.kind != ''Gateway'')' status: description: RateLimitPolicyStatus defines the observed state of RateLimitPolicy properties: diff --git a/controllers/authpolicy_authconfig.go b/controllers/authpolicy_authconfig.go index 925fc5307..dfd673cb4 100644 --- a/controllers/authpolicy_authconfig.go +++ b/controllers/authpolicy_authconfig.go @@ -5,10 +5,9 @@ import ( "errors" "fmt" "reflect" + "slices" "strings" - "k8s.io/utils/ptr" - "github.com/go-logr/logr" authorinoapi "github.com/kuadrant/authorino/api/v1beta2" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -18,6 +17,7 @@ import ( api "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/common" + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) @@ -67,14 +67,17 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *api.Au switch obj := targetNetworkObject.(type) { case *gatewayapiv1.HTTPRoute: - ok, err := routeGatewayHasAuthOverrides(ctx, obj, r.Client()) + t, err := r.generateTopology(ctx) if err != nil { + logger.V(1).Info("Failed to generate topology", "error", err) return nil, err } - if ok { + + overrides := routeGatewayAuthOverrides(t, ap) + if len(overrides) != 0 { logger.V(1).Info("targeted gateway has authpolicy with atomic overrides, skipping authorino authconfig for the HTTPRoute authpolicy") utils.TagObjectToDelete(authConfig) - r.OverriddenPolicyMap.SetOverriddenPolicy(ap) + r.AffectedPolicyMap.SetAffectedPolicy(ap, overrides) return authConfig, nil } route = obj @@ -105,7 +108,14 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *api.Au if len(rules) == 0 { logger.V(1).Info("no httproutes attached to the targeted gateway, skipping authorino authconfig for the gateway authpolicy") utils.TagObjectToDelete(authConfig) - r.OverriddenPolicyMap.SetOverriddenPolicy(ap) + obj := targetNetworkObject.(*gatewayapiv1.Gateway) + gatewayWrapper := kuadrant.GatewayWrapper{Gateway: obj, Referrer: ap} + refs := gatewayWrapper.PolicyRefs() + filteredRef := utils.Filter(refs, func(key client.ObjectKey) bool { + return key != client.ObjectKeyFromObject(ap) + }) + + r.AffectedPolicyMap.SetAffectedPolicy(ap, filteredRef) return authConfig, nil } route = &gatewayapiv1.HTTPRoute{ @@ -116,8 +126,8 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *api.Au } } - // AuthPolicy is not overridden if we still need to create an AuthConfig for it - r.OverriddenPolicyMap.RemoveOverriddenPolicy(ap) + // AuthPolicy is not Affected if we still need to create an AuthConfig for it + r.AffectedPolicyMap.RemoveAffectedPolicy(ap) // hosts authConfig.Spec.Hosts = hosts @@ -185,32 +195,43 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *api.Au return mergeConditionsFromRouteSelectorsIntoConfigs(ap, route, authConfig) } -// routeGatewayHasAuthOverrides return true when the gateway which a route is attached to has an attached authPolicy that defines atomic overrides -func routeGatewayHasAuthOverrides(ctx context.Context, route *gatewayapiv1.HTTPRoute, c client.Client) (bool, error) { - for idx := range route.Spec.ParentRefs { - parentRef := route.Spec.ParentRefs[idx] - gw := &gatewayapiv1.Gateway{} - namespace := ptr.Deref(parentRef.Namespace, gatewayapiv1.Namespace(route.GetNamespace())) - err := c.Get(ctx, client.ObjectKey{Name: string(parentRef.Name), Namespace: string(namespace)}, gw) - if err != nil { - return false, err - } +// routeGatewayAuthOverrides returns the GW auth policies that has an override field set +func routeGatewayAuthOverrides(t *kuadrantgatewayapi.Topology, ap *api.AuthPolicy) []client.ObjectKey { + affectedPolicies := getAffectedPolicies(t, ap) - annotation, ok := gw.GetAnnotations()[common.AuthPolicyBackRefAnnotation] + // Filter the policies where: + // 1. targets a gateway + // 2. is not the current AP that is being assessed + // 3. is an overriding policy + affectedPolicies = utils.Filter(affectedPolicies, func(policy kuadrantgatewayapi.Policy) bool { + p, ok := policy.(*api.AuthPolicy) if !ok { - continue - } - otherAP := &api.AuthPolicy{} - err = c.Get(ctx, utils.NamespacedNameToObjectKey(annotation, gw.Namespace), otherAP) - if err != nil { - return false, err + return false } + return kuadrantgatewayapi.IsTargetRefGateway(policy.GetTargetRef()) && + ap.GetUID() != policy.GetUID() && p.IsAtomicOverride() + }) - if otherAP.IsAtomicOverride() { - return true, nil + return utils.Map(affectedPolicies, func(policy kuadrantgatewayapi.Policy) client.ObjectKey { + return client.ObjectKeyFromObject(policy) + }) +} + +func getAffectedPolicies(t *kuadrantgatewayapi.Topology, ap *api.AuthPolicy) []kuadrantgatewayapi.Policy { + topologyIndexes := kuadrantgatewayapi.NewTopologyIndexes(t) + var affectedPolicies []kuadrantgatewayapi.Policy + + // If AP is listed within the policies from gateway, it potentially can be overridden by it + for _, gw := range t.Gateways() { + policyList := topologyIndexes.PoliciesFromGateway(gw.Gateway) + if slices.Contains(utils.Map(policyList, func(p kuadrantgatewayapi.Policy) client.ObjectKey { + return client.ObjectKeyFromObject(p) + }), client.ObjectKeyFromObject(ap)) { + affectedPolicies = append(affectedPolicies, policyList...) } } - return false, nil + + return affectedPolicies } // authConfigName returns the name of Authorino AuthConfig CR. diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index 57856b074..8fb053676 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -26,8 +26,8 @@ const authPolicyFinalizer = "authpolicy.kuadrant.io/finalizer" type AuthPolicyReconciler struct { *reconcilers.BaseReconciler TargetRefReconciler reconcilers.TargetRefReconciler - // OverriddenPolicyMap tracks the overridden policies to report their status. - OverriddenPolicyMap *kuadrant.OverriddenPolicyMap + // AffectedPolicyMap tracks the affected policies to report their status. + AffectedPolicyMap *kuadrant.AffectedPolicyMap } //+kubebuilder:rbac:groups=kuadrant.io,resources=authpolicies,verbs=get;list;watch;create;update;patch;delete @@ -72,7 +72,7 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ if delResErr == nil { delResErr = err } - return r.reconcileStatus(ctx, ap, targetNetworkObject, kuadrant.NewErrTargetNotFound(ap.Kind(), ap.GetTargetRef(), delResErr)) + return r.reconcileStatus(ctx, ap, kuadrant.NewErrTargetNotFound(ap.Kind(), ap.GetTargetRef(), delResErr)) } return ctrl.Result{}, err } @@ -108,7 +108,7 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ specErr := r.reconcileResources(ctx, ap, targetNetworkObject) // reconcile authpolicy status - statusResult, statusErr := r.reconcileStatus(ctx, ap, targetNetworkObject, specErr) + statusResult, statusErr := r.reconcileStatus(ctx, ap, specErr) if specErr != nil { return ctrl.Result{}, specErr diff --git a/controllers/authpolicy_controller_test.go b/controllers/authpolicy_controller_test.go index 15707cbc2..72e718298 100644 --- a/controllers/authpolicy_controller_test.go +++ b/controllers/authpolicy_controller_test.go @@ -39,8 +39,8 @@ const ( var _ = Describe("AuthPolicy controller", func() { var testNamespace string - BeforeEach(func() { - CreateNamespace(&testNamespace) + BeforeEach(func(ctx SpecContext) { + CreateNamespaceWithContext(ctx, &testNamespace) gateway := testBuildBasicGateway(testGatewayName, testNamespace) err := k8sClient.Create(context.Background(), gateway) @@ -49,9 +49,11 @@ var _ = Describe("AuthPolicy controller", func() { Eventually(testGatewayIsReady(gateway), 15*time.Second, 5*time.Second).Should(BeTrue()) ApplyKuadrantCR(testNamespace) - }) + }, NodeTimeout(3*time.Minute)) - AfterEach(DeleteNamespaceCallback(&testNamespace)) + AfterEach(func(ctx SpecContext) { + DeleteNamespaceCallbackWithContext(ctx, &testNamespace) + }, NodeTimeout(3*time.Minute)) policyFactory := func(mutateFns ...func(policy *api.AuthPolicy)) *api.AuthPolicy { policy := &api.AuthPolicy{ @@ -1134,10 +1136,10 @@ var _ = Describe("AuthPolicy controller", func() { ), 30*time.Second, 5*time.Second).Should(BeTrue()) }) - It("Invalid reason", func() { + It("Invalid reason", func(ctx SpecContext) { var otherNamespace string CreateNamespace(&otherNamespace) - defer DeleteNamespaceCallback(&otherNamespace) + defer DeleteNamespaceCallback(&otherNamespace)() policy := policyFactory(func(policy *api.AuthPolicy) { policy.Namespace = otherNamespace // create the policy in a different namespace than the target @@ -1145,10 +1147,10 @@ var _ = Describe("AuthPolicy controller", func() { policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(testGatewayName) policy.Spec.TargetRef.Namespace = ptr.To(gatewayapiv1.Namespace(testNamespace)) }) - Expect(k8sClient.Create(context.Background(), policy)).To(Succeed()) + Expect(k8sClient.Create(ctx, policy)).To(Succeed()) - Eventually(assertAcceptedCondFalseAndEnforcedCondNil(policy, string(gatewayapiv1alpha2.PolicyReasonInvalid), fmt.Sprintf("AuthPolicy target is invalid: invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", testNamespace)), 30*time.Second, 5*time.Second).Should(BeTrue()) - }) + Eventually(assertAcceptedCondFalseAndEnforcedCondNil(policy, string(gatewayapiv1alpha2.PolicyReasonInvalid), fmt.Sprintf("AuthPolicy target is invalid: invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", testNamespace))).WithContext(ctx).Should(BeTrue()) + }, SpecTimeout(time.Minute)) }) Context("AuthPolicy enforced condition reasons", func() { @@ -1243,7 +1245,7 @@ var _ = Describe("AuthPolicy controller", func() { Eventually(isAuthPolicyAccepted(gwPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) Eventually( assertAcceptedCondTrueAndEnforcedCond(gwPolicy, metav1.ConditionFalse, string(kuadrant.PolicyReasonOverridden), - fmt.Sprintf("AuthPolicy is overridden by [{\"Namespace\":\"%s\",\"Name\":\"%s\"}]", testNamespace, routePolicy.Name)), + fmt.Sprintf("AuthPolicy is overridden by [%s/%s]", testNamespace, routePolicy.Name)), 30*time.Second, 5*time.Second).Should(BeTrue()) // check istio authorizationpolicy @@ -1283,7 +1285,7 @@ var _ = Describe("AuthPolicy controller", func() { Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(route)), time.Minute, 5*time.Second).Should(BeTrue()) }) - It("Gateway AuthPolicy has overrides and Route AuthPolicy is added.", func() { + It("Gateway AuthPolicy has overrides and Route AuthPolicy is added.", func(ctx SpecContext) { gatewayPolicy := policyFactory(func(policy *api.AuthPolicy) { policy.Name = "gw-auth" policy.Spec.TargetRef.Group = gatewayapiv1.GroupName @@ -1295,13 +1297,13 @@ var _ = Describe("AuthPolicy controller", func() { policy.Spec.Overrides.AuthScheme.Authentication["apiKey"].ApiKey.Selector.MatchLabels["admin"] = "yes" }) - err := k8sClient.Create(context.Background(), gatewayPolicy) + err := k8sClient.Create(ctx, gatewayPolicy) logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(gatewayPolicy).String(), "error", err) Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyAccepted(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) + Eventually(isAuthPolicyAccepted(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) routePolicy := policyFactory() err = k8sClient.Create(context.Background(), routePolicy) @@ -1309,17 +1311,19 @@ var _ = Describe("AuthPolicy controller", func() { Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyAccepted(routePolicy), 30*time.Second, 5*time.Second).Should(BeFalse()) - }) + Eventually(isAuthPolicyAccepted(routePolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(routePolicy)).WithContext(ctx).Should(BeFalse()) + Eventually(isAuthPolicyEnforcedCondition(client.ObjectKeyFromObject(routePolicy), kuadrant.PolicyReasonOverridden, fmt.Sprintf("AuthPolicy is overridden by [%s]", client.ObjectKeyFromObject(gatewayPolicy)))).WithContext(ctx).Should(BeTrue()) + }, SpecTimeout(2*time.Minute)) - It("Route AuthPolicy exists and Gateway AuthPolicy with overrides is added.", func() { + It("Route AuthPolicy exists and Gateway AuthPolicy with overrides is added.", func(ctx SpecContext) { routePolicy := policyFactory() - err := k8sClient.Create(context.Background(), routePolicy) + err := k8sClient.Create(ctx, routePolicy) logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(routePolicy).String(), "error", err) Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyAccepted(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) + Eventually(isAuthPolicyAccepted(routePolicy)).WithContext(ctx).Should(BeTrue()) gatewayPolicy := policyFactory(func(policy *api.AuthPolicy) { policy.Name = "gw-auth" @@ -1332,24 +1336,26 @@ var _ = Describe("AuthPolicy controller", func() { policy.Spec.Overrides.AuthScheme.Authentication["apiKey"].ApiKey.Selector.MatchLabels["admin"] = "yes" }) - err = k8sClient.Create(context.Background(), gatewayPolicy) + err = k8sClient.Create(ctx, gatewayPolicy) logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(gatewayPolicy).String(), "error", err) Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyAccepted(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(routePolicy), 30*time.Second, 5*time.Second).Should(BeFalse()) - }) + Eventually(isAuthPolicyAccepted(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(routePolicy)).WithContext(ctx).Should(BeFalse()) + Eventually(isAuthPolicyEnforcedCondition(client.ObjectKeyFromObject(routePolicy), kuadrant.PolicyReasonOverridden, fmt.Sprintf("AuthPolicy is overridden by [%s]", client.ObjectKeyFromObject(gatewayPolicy)))).WithContext(ctx).Should(BeTrue()) + }, SpecTimeout(2*time.Minute)) - It("Route AuthPolicy exists and Gateway AuthPolicy with overrides is removed.", func() { + It("Route AuthPolicy exists and Gateway AuthPolicy with overrides is removed.", func(ctx SpecContext) { routePolicy := policyFactory() - err := k8sClient.Create(context.Background(), routePolicy) + err := k8sClient.Create(ctx, routePolicy) logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(routePolicy).String(), "error", err) Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyAccepted(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) + Eventually(isAuthPolicyAccepted(routePolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(routePolicy)).WithContext(ctx).Should(BeTrue()) gatewayPolicy := policyFactory(func(policy *api.AuthPolicy) { policy.Name = "gw-auth" @@ -1362,31 +1368,32 @@ var _ = Describe("AuthPolicy controller", func() { policy.Spec.Overrides.AuthScheme.Authentication["apiKey"].ApiKey.Selector.MatchLabels["admin"] = "yes" }) - err = k8sClient.Create(context.Background(), gatewayPolicy) + err = k8sClient.Create(ctx, gatewayPolicy) logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(gatewayPolicy).String(), "error", err) Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyAccepted(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(routePolicy), 30*time.Second, 5*time.Second).Should(BeFalse()) + Eventually(isAuthPolicyAccepted(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(routePolicy)).WithContext(ctx).Should(BeFalse()) + Eventually(isAuthPolicyEnforcedCondition(client.ObjectKeyFromObject(routePolicy), kuadrant.PolicyReasonOverridden, fmt.Sprintf("AuthPolicy is overridden by [%s]", client.ObjectKeyFromObject(gatewayPolicy)))).WithContext(ctx).Should(BeTrue()) err = k8sClient.Delete(context.Background(), gatewayPolicy) logf.Log.V(1).Info("Deleting AuthPolicy", "key", client.ObjectKeyFromObject(gatewayPolicy).String(), "error", err) Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyEnforced(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - }) + Eventually(isAuthPolicyEnforced(routePolicy)).WithContext(ctx).Should(BeTrue()) + }, SpecTimeout(2*time.Minute)) - It("Route and Gateway AuthPolicies exist. Gateway AuthPolicy updated to include overrides.", func() { + It("Route and Gateway AuthPolicies exist. Gateway AuthPolicy updated to include overrides.", func(ctx SpecContext) { routePolicy := policyFactory() - err := k8sClient.Create(context.Background(), routePolicy) + err := k8sClient.Create(ctx, routePolicy) logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(routePolicy).String(), "error", err) Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyAccepted(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) + Eventually(isAuthPolicyAccepted(routePolicy)).WithContext(ctx).Should(BeTrue()) gatewayPolicy := policyFactory(func(policy *api.AuthPolicy) { policy.Name = "gw-auth" @@ -1396,14 +1403,15 @@ var _ = Describe("AuthPolicy controller", func() { policy.Spec.CommonSpec().AuthScheme.Authentication["apiKey"].ApiKey.Selector.MatchLabels["admin"] = "yes" }) - err = k8sClient.Create(context.Background(), gatewayPolicy) + err = k8sClient.Create(ctx, gatewayPolicy) logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(gatewayPolicy).String(), "error", err) Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyAccepted(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeFalse()) - Eventually(isAuthPolicyEnforced(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) + Eventually(isAuthPolicyAccepted(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(gatewayPolicy)).WithContext(ctx).Should(BeFalse()) + Eventually(isAuthPolicyEnforcedCondition(client.ObjectKeyFromObject(gatewayPolicy), kuadrant.PolicyReasonOverridden, fmt.Sprintf("AuthPolicy is overridden by [%s]", client.ObjectKeyFromObject(routePolicy)))).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(routePolicy)).WithContext(ctx).Should(BeTrue()) Eventually(func() bool { err = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(gatewayPolicy), gatewayPolicy) @@ -1417,23 +1425,24 @@ var _ = Describe("AuthPolicy controller", func() { err = k8sClient.Update(context.Background(), gatewayPolicy) logf.Log.V(1).Info("Updating AuthPolicy", "key", client.ObjectKeyFromObject(gatewayPolicy).String(), "error", err) return err == nil - }, 30*time.Second, 5*time.Second).Should(BeTrue()) + }).WithContext(ctx).Should(BeTrue()) // check policy status - Eventually(isAuthPolicyAccepted(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(routePolicy), 30*time.Second, 5*time.Second).Should(BeFalse()) - - }) + Eventually(isAuthPolicyAccepted(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(routePolicy)).WithContext(ctx).Should(BeFalse()) + Eventually(isAuthPolicyEnforcedCondition(client.ObjectKeyFromObject(routePolicy), kuadrant.PolicyReasonOverridden, fmt.Sprintf("AuthPolicy is overridden by [%s]", client.ObjectKeyFromObject(gatewayPolicy)))).WithContext(ctx).Should(BeTrue()) + }, SpecTimeout(2*time.Minute)) - It("Route and Gateway AuthPolicies exist. Gateway AuthPolicy updated to remove overrides.", func() { + It("Route and Gateway AuthPolicies exist. Gateway AuthPolicy updated to remove overrides.", func(ctx SpecContext) { routePolicy := policyFactory() - err := k8sClient.Create(context.Background(), routePolicy) + err := k8sClient.Create(ctx, routePolicy) logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(routePolicy).String(), "error", err) Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyAccepted(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) + Eventually(isAuthPolicyAccepted(routePolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(routePolicy)).WithContext(ctx).Should(BeFalse()) gatewayPolicy := policyFactory(func(policy *api.AuthPolicy) { policy.Name = "gw-auth" @@ -1446,45 +1455,47 @@ var _ = Describe("AuthPolicy controller", func() { policy.Spec.Overrides.AuthScheme.Authentication["apiKey"].ApiKey.Selector.MatchLabels["admin"] = "yes" }) - err = k8sClient.Create(context.Background(), gatewayPolicy) + err = k8sClient.Create(ctx, gatewayPolicy) logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(gatewayPolicy).String(), "error", err) Expect(err).ToNot(HaveOccurred()) // check policy status - Eventually(isAuthPolicyAccepted(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(routePolicy), 30*time.Second, 5*time.Second).Should(BeFalse()) + Eventually(isAuthPolicyAccepted(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(routePolicy)).WithContext(ctx).Should(BeFalse()) + Eventually(isAuthPolicyEnforcedCondition(client.ObjectKeyFromObject(routePolicy), kuadrant.PolicyReasonOverridden, fmt.Sprintf("AuthPolicy is overridden by [%s]", client.ObjectKeyFromObject(gatewayPolicy)))).WithContext(ctx).Should(BeTrue()) Eventually(func() bool { - err = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(gatewayPolicy), gatewayPolicy) + err = k8sClient.Get(ctx, client.ObjectKeyFromObject(gatewayPolicy), gatewayPolicy) if err != nil { return false } gatewayPolicy.Spec.Overrides = nil gatewayPolicy.Spec.CommonSpec().AuthScheme = testBasicAuthScheme() gatewayPolicy.Spec.CommonSpec().AuthScheme.Authentication["apiKey"].ApiKey.Selector.MatchLabels["admin"] = "yes" - err = k8sClient.Update(context.Background(), gatewayPolicy) + err = k8sClient.Update(ctx, gatewayPolicy) logf.Log.V(1).Info("Updating AuthPolicy", "key", client.ObjectKeyFromObject(gatewayPolicy).String(), "error", err) return err == nil - }, 30*time.Second, 5*time.Second).Should(BeTrue()) + }).WithContext(ctx).Should(BeTrue()) // check policy status - Eventually(isAuthPolicyAccepted(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) - Eventually(isAuthPolicyEnforced(gatewayPolicy), 30*time.Second, 5*time.Second).Should(BeFalse()) - Eventually(isAuthPolicyEnforced(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue()) + Eventually(isAuthPolicyAccepted(gatewayPolicy)).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(gatewayPolicy)).WithContext(ctx).Should(BeFalse()) + Eventually(isAuthPolicyEnforcedCondition(client.ObjectKeyFromObject(gatewayPolicy), kuadrant.PolicyReasonOverridden, fmt.Sprintf("AuthPolicy is overridden by [%s]", client.ObjectKeyFromObject(routePolicy)))).WithContext(ctx).Should(BeTrue()) + Eventually(isAuthPolicyEnforced(routePolicy)).WithContext(ctx).Should(BeTrue()) }) - It("Blocks creation of AuthPolicies with overrides targeting HTTPRoutes", func() { + It("Blocks creation of AuthPolicies with overrides targeting HTTPRoutes", func(ctx SpecContext) { routePolicy := policyFactory(func(policy *api.AuthPolicy) { policy.Spec.Overrides = &api.AuthPolicyCommonSpec{} policy.Spec.Defaults = nil policy.Spec.Overrides.AuthScheme = testBasicAuthScheme() }) - err := k8sClient.Create(context.Background(), routePolicy) + err := k8sClient.Create(ctx, routePolicy) logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(routePolicy).String(), "error", err) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Overrides are not allowed for policies targeting a HTTPRoute resource")) - }) + }, SpecTimeout(2*time.Minute)) }) }) @@ -2043,6 +2054,20 @@ func isAuthPolicyEnforced(policy *api.AuthPolicy) func() bool { return isAuthPolicyConditionTrue(policy, string(kuadrant.PolicyConditionEnforced)) } +func isAuthPolicyEnforcedCondition(key client.ObjectKey, reason gatewayapiv1alpha2.PolicyConditionReason, message string) bool { + p := &api.AuthPolicy{} + if err := k8sClient.Get(context.Background(), key, p); err != nil { + return false + } + + cond := meta.FindStatusCondition(p.Status.Conditions, string(kuadrant.PolicyConditionEnforced)) + if cond == nil { + return false + } + + return cond.Reason == string(reason) && cond.Message == message +} + func isAuthPolicyConditionTrue(policy *api.AuthPolicy, condition string) func() bool { return func() bool { existingPolicy := &api.AuthPolicy{} diff --git a/controllers/authpolicy_status.go b/controllers/authpolicy_status.go index 6823816ad..b2e9409be 100644 --- a/controllers/authpolicy_status.go +++ b/controllers/authpolicy_status.go @@ -2,35 +2,33 @@ package controllers import ( "context" - "encoding/json" "errors" "fmt" "slices" - kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" - "k8s.io/utils/ptr" - "github.com/go-logr/logr" authorinoapi "github.com/kuadrant/authorino/api/v1beta2" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" api "github.com/kuadrant/kuadrant-operator/api/v1beta2" + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) // reconcileStatus makes sure status block of AuthPolicy is up-to-date. -func (r *AuthPolicyReconciler) reconcileStatus(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object, specErr error) (ctrl.Result, error) { +func (r *AuthPolicyReconciler) reconcileStatus(ctx context.Context, ap *api.AuthPolicy, specErr error) (ctrl.Result, error) { logger, _ := logr.FromContext(ctx) logger.V(1).Info("Reconciling AuthPolicy status", "spec error", specErr) - newStatus := r.calculateStatus(ctx, ap, targetNetworkObject, specErr) + newStatus := r.calculateStatus(ctx, ap, specErr) equalStatus := ap.Status.Equals(newStatus, logger) logger.V(1).Info("Status", "status is different", !equalStatus) @@ -62,7 +60,7 @@ func (r *AuthPolicyReconciler) reconcileStatus(ctx context.Context, ap *api.Auth return ctrl.Result{}, nil } -func (r *AuthPolicyReconciler) calculateStatus(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object, specErr error) *api.AuthPolicyStatus { +func (r *AuthPolicyReconciler) calculateStatus(ctx context.Context, ap *api.AuthPolicy, specErr error) *api.AuthPolicyStatus { newStatus := &api.AuthPolicyStatus{ Conditions: slices.Clone(ap.Status.Conditions), ObservedGeneration: ap.Status.ObservedGeneration, @@ -76,7 +74,7 @@ func (r *AuthPolicyReconciler) calculateStatus(ctx context.Context, ap *api.Auth return newStatus } - enforcedCond := r.enforcedCondition(ctx, ap, targetNetworkObject) + enforcedCond := r.enforcedCondition(ctx, ap) meta.SetStatusCondition(&newStatus.Conditions, *enforcedCond) return newStatus @@ -88,21 +86,16 @@ func (r *AuthPolicyReconciler) acceptedCondition(policy kuadrant.Policy, specErr // enforcedCondition checks if the provided AuthPolicy is enforced, ensuring it is properly configured and applied based // on the status of the associated AuthConfig and Gateway. -func (r *AuthPolicyReconciler) enforcedCondition(ctx context.Context, policy *api.AuthPolicy, targetNetworkObject client.Object) *metav1.Condition { +func (r *AuthPolicyReconciler) enforcedCondition(ctx context.Context, policy *api.AuthPolicy) *metav1.Condition { logger, _ := logr.FromContext(ctx) - t, err := r.generateTopology(ctx) - if err != nil { - logger.V(1).Info("Failed to generate topology", "error", err) - return nil - } - // Check if the policy is overridden + // Check if the policy is Affected // Note: This logic assumes synchronous processing, where computing the desired AuthConfig, marking the AuthPolicy - // as overridden, and calculating the Enforced condition happen sequentially. + // as Affected, and calculating the Enforced condition happen sequentially. // Introducing a goroutine in this flow could break this assumption and lead to unexpected behavior. - if r.OverriddenPolicyMap.IsPolicyOverridden(policy) { + if r.AffectedPolicyMap.IsPolicyAffected(policy) { logger.V(1).Info("Gateway Policy is overridden") - return r.handlePolicyOverride(logger, policy, targetNetworkObject, t) + return r.handlePolicyOverride(policy) } // Check if the AuthConfig is ready @@ -138,47 +131,12 @@ func (r *AuthPolicyReconciler) isAuthConfigReady(ctx context.Context, policy *ap return authConfig.Status.Ready(), nil } -func (r *AuthPolicyReconciler) handlePolicyOverride(logger logr.Logger, policy *api.AuthPolicy, targetNetworkObject client.Object, t *kuadrantgatewayapi.Topology) *metav1.Condition { - switch targetNetworkObject.(type) { - case *gatewayapiv1.Gateway: - return r.handleGatewayPolicyOverride(logger, policy, targetNetworkObject) - case *gatewayapiv1.HTTPRoute: - return r.handleHTTPRoutePolicyOverride(logger, policy, targetNetworkObject, t) - default: - logger.Error(errors.New("this point should never be reached"), "failed to match target network object", targetNetworkObject) - return nil +func (r *AuthPolicyReconciler) handlePolicyOverride(policy *api.AuthPolicy) *metav1.Condition { + if !r.AffectedPolicyMap.IsPolicyOverridden(policy) { + return kuadrant.EnforcedCondition(policy, kuadrant.NewErrUnknown(policy.Kind(), errors.New("no free routes to enforce policy")), false) // Maybe this should be a standard condition rather than an unknown condition } -} -// handleGatewayPolicyOverride handles the case where the Gateway Policy is overridden by filtering policy references -// and creating a corresponding error condition. -func (r *AuthPolicyReconciler) handleGatewayPolicyOverride(logger logr.Logger, policy *api.AuthPolicy, targetNetworkObject client.Object) *metav1.Condition { - obj := targetNetworkObject.(*gatewayapiv1.Gateway) - gatewayWrapper := kuadrant.GatewayWrapper{Gateway: obj, Referrer: policy} - refs := gatewayWrapper.PolicyRefs() - filteredRef := utils.Filter(refs, func(key client.ObjectKey) bool { - return key != client.ObjectKeyFromObject(policy) - }) - jsonData, err := json.Marshal(filteredRef) - if err != nil { - logger.Error(err, "Failed to marshal filtered references") - return kuadrant.EnforcedCondition(policy, kuadrant.NewErrUnknown(policy.Kind(), err), false) - } - return kuadrant.EnforcedCondition(policy, kuadrant.NewErrOverridden(policy.Kind(), string(jsonData)), false) -} - -// handleHTTPRoutePolicyOverride handles the case where the HTTPRoute Policy is overridden by filtering policy references -// and creating a corresponding error condition. -func (r *AuthPolicyReconciler) handleHTTPRoutePolicyOverride(logger logr.Logger, policy *api.AuthPolicy, targetNetworkObject client.Object, t *kuadrantgatewayapi.Topology) *metav1.Condition { - obj := targetNetworkObject.(*gatewayapiv1.HTTPRoute) - httpRouteWrapper := kuadrant.HTTPRouteWrapper{HTTPRoute: obj, Referrer: policy} - refs := httpRouteWrapper.PolicyRefs(t) - jsonData, err := json.Marshal(refs) - if err != nil { - logger.Error(err, "Failed to marshal filtered references") - return kuadrant.EnforcedCondition(policy, kuadrant.NewErrUnknown(policy.Kind(), err), false) - } - return kuadrant.EnforcedCondition(policy, kuadrant.NewErrOverridden(policy.Kind(), string(jsonData)), false) + return kuadrant.EnforcedCondition(policy, kuadrant.NewErrOverridden(policy.Kind(), r.AffectedPolicyMap.PolicyAffectedBy(policy)), false) } func (r *AuthPolicyReconciler) generateTopology(ctx context.Context) (*kuadrantgatewayapi.Topology, error) { diff --git a/controllers/helper_test.go b/controllers/helper_test.go index 54f6d3db1..cc1ca51fd 100644 --- a/controllers/helper_test.go +++ b/controllers/helper_test.go @@ -37,6 +37,7 @@ import ( kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" + "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) @@ -88,6 +89,16 @@ func ApplyKuadrantCRWithName(namespace, name string) { }, time.Minute, 5*time.Second).Should(BeTrue()) } +func CreateNamespaceWithContext(ctx context.Context, namespace *string) { + nsObject := &v1.Namespace{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Namespace"}, + ObjectMeta: metav1.ObjectMeta{GenerateName: "test-namespace-"}, + } + Expect(testClient().Create(ctx, nsObject)).To(Succeed()) + + *namespace = nsObject.Name +} + func CreateNamespace(namespace *string) { var generatedTestNamespace = "test-namespace-" + uuid.New().String() @@ -108,6 +119,16 @@ func CreateNamespace(namespace *string) { *namespace = existingNamespace.Name } +func DeleteNamespaceCallbackWithContext(ctx context.Context, namespace *string) { + desiredTestNamespace := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: *namespace}} + Eventually(func(g Gomega) { + err := k8sClient.Delete(ctx, desiredTestNamespace, client.PropagationPolicy(metav1.DeletePropagationForeground)) + g.Expect(err).ToNot(BeNil()) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).WithContext(ctx).Should(Succeed()) + +} + func DeleteNamespaceCallback(namespace *string) func() { return func() { desiredTestNamespace := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: *namespace}} @@ -375,20 +396,25 @@ func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { } func testRLPIsAccepted(rlpKey client.ObjectKey) func() bool { - return func() bool { - existingRLP := &kuadrantv1beta2.RateLimitPolicy{} - err := k8sClient.Get(context.Background(), rlpKey, existingRLP) - if err != nil { - logf.Log.V(1).Info("ratelimitpolicy not read", "rlp", rlpKey, "error", err) - return false - } - if !meta.IsStatusConditionTrue(existingRLP.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) { - logf.Log.V(1).Info("ratelimitpolicy not available", "rlp", rlpKey) - return false - } + return testRLPIsConditionTrue(rlpKey, string(gatewayapiv1alpha2.PolicyConditionAccepted)) +} - return true +func testRLPIsEnforced(rlpKey client.ObjectKey) func() bool { + return testRLPIsConditionTrue(rlpKey, string(kuadrant.PolicyConditionEnforced)) +} + +func testRLPEnforcedCondition(rlpKey client.ObjectKey, reason gatewayapiv1alpha2.PolicyConditionReason, message string) bool { + p := &kuadrantv1beta2.RateLimitPolicy{} + if err := k8sClient.Get(context.Background(), rlpKey, p); err != nil { + return false + } + + cond := meta.FindStatusCondition(p.Status.Conditions, string(kuadrant.PolicyConditionEnforced)) + if cond == nil { + return false } + + return cond.Reason == string(reason) && cond.Message == message } func testRLPIsNotAccepted(rlpKey client.ObjectKey) func() bool { @@ -408,6 +434,14 @@ func testRLPIsNotAccepted(rlpKey client.ObjectKey) func() bool { } } +func testRLPIsConditionTrue(rlpKey client.ObjectKey, condition string) func() bool { + return func() bool { + existingRLP := &kuadrantv1beta2.RateLimitPolicy{} + err := k8sClient.Get(context.Background(), rlpKey, existingRLP) + return err == nil && meta.IsStatusConditionTrue(existingRLP.Status.Conditions, condition) + } +} + func testHTTPRouteWithoutDirectBackReference(routeKey client.ObjectKey, annotationName string) func() bool { return testNetworkResourceWithoutDirectBackReference(routeKey, &gatewayapiv1.HTTPRoute{}, annotationName) } diff --git a/controllers/limitador_cluster_envoyfilter_controller_test.go b/controllers/limitador_cluster_envoyfilter_controller_test.go index 29ce4e12f..5e06dfe09 100644 --- a/controllers/limitador_cluster_envoyfilter_controller_test.go +++ b/controllers/limitador_cluster_envoyfilter_controller_test.go @@ -21,6 +21,7 @@ import ( kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/common" + "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" ) var _ = Describe("Limitador Cluster EnvoyFilter controller", func() { @@ -108,6 +109,8 @@ var _ = Describe("Limitador Cluster EnvoyFilter controller", func() { // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeFalse()) + Expect(testRLPEnforcedCondition(rlpKey, kuadrant.PolicyReasonUnknown, "RateLimitPolicy has encountered some issues: no free routes to enforce policy")) // Check envoy filter Eventually(func() bool { diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index bbb2e37ec..393e50911 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -170,11 +170,11 @@ func (r *RateLimitingWASMPluginReconciler) wasmPluginConfig(ctx context.Context, logger.V(1).Info("wasmPluginConfig", "#RLPS", len(rateLimitPolicies)) // Sort RLPs for consistent comparison with existing objects - sort.Sort(kuadrantgatewayapi.PolicyByCreationTimestamp(rateLimitPolicies)) + sort.Sort(kuadrantgatewayapi.PolicyByTargetRefKindAndCreationTimeStamp(rateLimitPolicies)) for _, policy := range rateLimitPolicies { rlp := policy.(*kuadrantv1beta2.RateLimitPolicy) - wasmRLP, err := r.wasmRateLimitPolicy(ctx, t, rlp, gw) + wasmRLP, err := r.wasmRateLimitPolicy(ctx, t, rlp, gw, rateLimitPolicies) if err != nil { return nil, err } @@ -232,7 +232,9 @@ func (r *RateLimitingWASMPluginReconciler) topologyIndexesFromGateway(ctx contex return kuadrantgatewayapi.NewTopologyIndexes(t), nil } -func (r *RateLimitingWASMPluginReconciler) wasmRateLimitPolicy(ctx context.Context, t *kuadrantgatewayapi.TopologyIndexes, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway) (*wasm.RateLimitPolicy, error) { +func (r *RateLimitingWASMPluginReconciler) wasmRateLimitPolicy(ctx context.Context, t *kuadrantgatewayapi.TopologyIndexes, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway, affectedPolices []kuadrantgatewayapi.Policy) (*wasm.RateLimitPolicy, error) { + logger, _ := logr.FromContext(ctx) + route, err := r.routeFromRLP(ctx, t, rlp, gw) if err != nil { return nil, err @@ -265,6 +267,22 @@ func (r *RateLimitingWASMPluginReconciler) wasmRateLimitPolicy(ctx context.Conte routeWithEffectiveHostnames := route.DeepCopy() routeWithEffectiveHostnames.Spec.Hostnames = hostnames + // Policy limits may be overridden by a gateway policy for route policies + if kuadrantgatewayapi.IsTargetRefHTTPRoute(rlp.GetTargetRef()) { + filteredPolicies := utils.Filter(affectedPolices, func(p kuadrantgatewayapi.Policy) bool { + return kuadrantgatewayapi.IsTargetRefGateway(p.GetTargetRef()) && p.GetUID() != rlp.GetUID() + }) + + for _, policy := range filteredPolicies { + p := policy.(*kuadrantv1beta2.RateLimitPolicy) + if p.Spec.Overrides != nil { + rlp.Spec.CommonSpec().Limits = p.Spec.Overrides.Limits + logger.V(1).Info("applying overrides from parent policy", "parentPolicy", client.ObjectKeyFromObject(p)) + break + } + } + } + rules := rlptools.WasmRules(rlp, routeWithEffectiveHostnames) if len(rules) == 0 { // no need to add the policy if there are no rules; a rlp can return no rules if all its limits fail to match any route rule diff --git a/controllers/rate_limiting_wasmplugin_controller_test.go b/controllers/rate_limiting_wasmplugin_controller_test.go index 3b6a83fef..f8ba3eed7 100644 --- a/controllers/rate_limiting_wasmplugin_controller_test.go +++ b/controllers/rate_limiting_wasmplugin_controller_test.go @@ -21,6 +21,7 @@ import ( kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/common" + "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/rlptools" "github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm" ) @@ -93,6 +94,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKeyFromObject(rlp) Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -253,6 +255,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKeyFromObject(rlp) Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -396,6 +399,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -496,6 +500,8 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeFalse()) + Expect(testRLPEnforcedCondition(rlpKey, kuadrant.PolicyReasonUnknown, "RateLimitPolicy has encountered some issues: no free routes to enforce policy")) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -575,6 +581,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKeyFromObject(rlp) Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -637,6 +644,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpAKey := client.ObjectKey{Name: rlpAName, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) // create httproute C httpRouteC := testBuildBasicHttpRoute(routeCName, gwName, testNamespace, []string{"*.c.example.com"}) @@ -701,6 +709,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpBKey := client.ObjectKey{Name: rlpBName, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpBKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpBKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin only has configuration ONLY from the RLP targeting the gateway // it may take some reconciliation loops to get to that, so checking it with eventually @@ -834,12 +843,15 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeFalse()) + Expect(testRLPEnforcedCondition(rlpKey, kuadrant.PolicyReasonUnknown, "RateLimitPolicy has encountered some issues: no free routes to enforce policy")) // create Route A -> Gw A httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) err = k8sClient.Create(context.Background(), httpRoute) Expect(err).ToNot(HaveOccurred()) Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // create Gateway B gwB := testBuildBasicGateway(gwBName, testNamespace) @@ -1039,6 +1051,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Initial state set. // Check wasm plugin for gateway A has configuration from the route @@ -1334,6 +1347,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Initial state set. // Check wasm plugin has configuration from the route A @@ -1567,6 +1581,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlp1Key := client.ObjectKey{Name: rlp1Name, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlp1Key), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlp1Key), time.Minute, 5*time.Second).Should(BeTrue()) // Initial state set. // Check wasm plugin for gateway A has configuration from the route 1 @@ -1671,6 +1686,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlp2Key := client.ObjectKey{Name: rlp2Name, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlp2Key), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlp2Key), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin has configuration from the route A and RLP 2. // RLP 1 should not add any config to the wasm plugin @@ -1830,6 +1846,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlp1Key := client.ObjectKey{Name: rlp1Name, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlp1Key), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlp1Key), time.Minute, 5*time.Second).Should(BeTrue()) // create RLP 2 -> Route A rlp2 := &kuadrantv1beta2.RateLimitPolicy{ @@ -1861,6 +1878,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlp2Key := client.ObjectKey{Name: rlp2Name, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlp2Key), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlp2Key), time.Minute, 5*time.Second).Should(BeTrue()) // Initial state set. // Check wasm plugin for gateway A has configuration from the route A only affected by RLP 2 @@ -2127,6 +2145,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKeyFromObject(rlp) Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -2178,4 +2197,166 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { })) }) }) + + Context("Gateway defaults & overrides", func() { + var ( + routeName = "toystore-route" + gwRLPName = "gw-rlp" + routeRLPName = "route-rlp" + gwName = "toystore-gw" + gateway *gatewayapiv1.Gateway + ) + + beforeEachCallback := func() { + gateway = testBuildBasicGateway(gwName, testNamespace) + err := k8sClient.Create(context.Background(), gateway) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + } + + expectedWasmPluginConfig := func(rlpKey client.ObjectKey, rlp *kuadrantv1beta2.RateLimitPolicy, key, hostname string) *wasm.Plugin { + return &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlpKey.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlp), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/toy", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: key, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{hostname}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + } + + BeforeEach(beforeEachCallback) + + It("Limit key shifts correctly from Gateway RLP default -> Route RLP -> Gateway RLP overrides", func(ctx SpecContext) { + // create httproute + httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) + Expect(k8sClient.Create(ctx, httpRoute)).To(Succeed()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute))).WithContext(ctx).Should(BeTrue()) + + // create GW ratelimitpolicy with defaults + gwRLP := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: gwRLPName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.GroupName, + Kind: "Gateway", + Name: gatewayapiv1.ObjectName(gwName), + }, + Defaults: &kuadrantv1beta2.RateLimitPolicyCommonSpec{ + Limits: map[string]kuadrantv1beta2.Limit{ + "gateway": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed()) + + // Check RLP status is available + gwRLPKey := client.ObjectKeyFromObject(gwRLP) + Eventually(testRLPIsAccepted(gwRLPKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(gwRLPKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} + Eventually(testWasmPluginIsAvailable(wasmPluginKey), time.Minute, 5*time.Second).Should(BeTrue()) + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + // must exist + Expect(k8sClient.Get(ctx, wasmPluginKey, existingWasmPlugin)).To(Succeed()) + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + Expect(err).ToNot(HaveOccurred()) + Expect(existingWASMConfig).To(Equal(expectedWasmPluginConfig(gwRLPKey, gwRLP, "limit.gateway__4ea5ee68", "*"))) + + // Create Route RLP + routeRLP := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: routeRLPName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.GroupName, + Kind: "HTTPRoute", + Name: gatewayapiv1.ObjectName(routeName), + }, + RateLimitPolicyCommonSpec: kuadrantv1beta2.RateLimitPolicyCommonSpec{ + Limits: map[string]kuadrantv1beta2.Limit{ + "route": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 10, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, routeRLP)).To(Succeed()) + routeRLPKey := client.ObjectKeyFromObject(routeRLP) + Eventually(testRLPIsAccepted(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeFalse()) + // Wasm plugin config should now use route RLP limit key + Expect(k8sClient.Get(ctx, wasmPluginKey, existingWasmPlugin)).To(Succeed()) + existingWASMConfig, err = rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + Expect(err).ToNot(HaveOccurred()) + Expect(existingWASMConfig).To(Equal(expectedWasmPluginConfig(routeRLPKey, routeRLP, "limit.route__8a84e406", "*.example.com"))) + + // Update GW RLP to overrides + Eventually(func(g Gomega) { + Expect(k8sClient.Get(ctx, gwRLPKey, gwRLP)).To(Succeed()) + gwRLP.Spec.Overrides = gwRLP.Spec.Defaults.DeepCopy() + gwRLP.Spec.Defaults = nil + Expect(k8sClient.Update(ctx, gwRLP)).To(Succeed()) + }).WithContext(ctx).Should(Succeed()) + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeFalse()) + // Wasm plugin config should now use GW RLP limit key for route + Expect(k8sClient.Get(ctx, wasmPluginKey, existingWasmPlugin)).To(Succeed()) + existingWASMConfig, err = rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + Expect(err).ToNot(HaveOccurred()) + Expect(existingWASMConfig).To(Equal(expectedWasmPluginConfig(routeRLPKey, routeRLP, "limit.gateway__4ea5ee68", "*.example.com"))) + + }, SpecTimeout(2*time.Minute)) + }) }) diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index b5e8fe3ff..b5a973c3c 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -21,6 +21,7 @@ import ( "encoding/json" "github.com/go-logr/logr" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" apierrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -41,6 +42,8 @@ const rateLimitPolicyFinalizer = "ratelimitpolicy.kuadrant.io/finalizer" type RateLimitPolicyReconciler struct { *reconcilers.BaseReconciler TargetRefReconciler reconcilers.TargetRefReconciler + // AffectedPolicyMap tracks the affected policies to report their status. + AffectedPolicyMap *kuadrant.AffectedPolicyMap } //+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies,verbs=get;list;watch;create;update;patch;delete @@ -226,6 +229,7 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&kuadrantv1beta2.RateLimitPolicy{}). + Watches(&limitadorv1alpha1.Limitador{}, limitadorStatusEventHandler{Client: r.Client(), Logger: r.Logger().WithName("limitadorStatusToRLPsEventHandler")}). Watches( &gatewayapiv1.HTTPRoute{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index 5ee202a80..014f08f26 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -12,6 +12,8 @@ import ( limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -21,6 +23,7 @@ import ( kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/common" + "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/rlptools" ) @@ -54,7 +57,7 @@ var _ = Describe("RateLimitPolicy controller", func() { "l1": { Rates: []kuadrantv1beta2.Rate{ { - Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + Limit: 1, Duration: 3, Unit: "minute", }, }, }, @@ -69,17 +72,19 @@ var _ = Describe("RateLimitPolicy controller", func() { return policy } - beforeEachCallback := func() { - CreateNamespace(&testNamespace) + beforeEachCallback := func(ctx SpecContext) { + CreateNamespaceWithContext(ctx, &testNamespace) gateway = testBuildBasicGateway(gwName, testNamespace) - err := k8sClient.Create(context.Background(), gateway) - Expect(err).ToNot(HaveOccurred()) - Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + + Expect(k8sClient.Create(ctx, gateway)).To(Succeed()) + Eventually(testGatewayIsReady(gateway)).WithContext(ctx).Should(BeTrue()) ApplyKuadrantCR(testNamespace) } - BeforeEach(beforeEachCallback) - AfterEach(DeleteNamespaceCallback(&testNamespace)) + BeforeEach(beforeEachCallback, NodeTimeout(time.Minute)) + AfterEach(func(ctx SpecContext) { + DeleteNamespaceCallbackWithContext(ctx, &testNamespace) + }, NodeTimeout(3*time.Minute)) Context("RLP targeting HTTPRoute", func() { It("Creates all the resources for a basic HTTPRoute and RateLimitPolicy", func() { @@ -97,6 +102,7 @@ var _ = Describe("RateLimitPolicy controller", func() { // Check RLP status is available rlpKey := client.ObjectKeyFromObject(rlp) Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check HTTPRoute direct back reference routeKey := client.ObjectKey{Name: routeName, Namespace: testNamespace} @@ -165,7 +171,7 @@ var _ = Describe("RateLimitPolicy controller", func() { "l1": { Rates: []kuadrantv1beta2.Rate{ { - Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + Limit: 1, Duration: 3, Unit: "minute", }, }, }, @@ -179,6 +185,7 @@ var _ = Describe("RateLimitPolicy controller", func() { // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check Gateway direct back reference gwKey := client.ObjectKeyFromObject(gateway) @@ -226,6 +233,8 @@ var _ = Describe("RateLimitPolicy controller", func() { // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeFalse()) + Expect(testRLPEnforcedCondition(rlpKey, kuadrant.PolicyReasonUnknown, "RateLimitPolicy has encountered some issues: no free routes to enforce policy")) // Check Gateway direct back reference gwKey := client.ObjectKeyFromObject(gateway) @@ -278,6 +287,7 @@ var _ = Describe("RateLimitPolicy controller", func() { Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed()) rlpKey := client.ObjectKey{Name: gwRLP.Name, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey)).WithContext(ctx).Should(BeTrue()) // Create HTTPRoute RLP with new default limits routeRLP := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { @@ -286,7 +296,7 @@ var _ = Describe("RateLimitPolicy controller", func() { "l1": { Rates: []kuadrantv1beta2.Rate{ { - Limit: 10, Duration: 5, Unit: kuadrantv1beta2.TimeUnit("second"), + Limit: 10, Duration: 5, Unit: "second", }, }, }, @@ -295,6 +305,7 @@ var _ = Describe("RateLimitPolicy controller", func() { Expect(k8sClient.Create(ctx, routeRLP)).To(Succeed()) rlpKey = client.ObjectKey{Name: routeRLP.Name, Namespace: testNamespace} Eventually(testRLPIsAccepted(rlpKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey)).WithContext(ctx).Should(BeTrue()) // Check Gateway direct back reference gwKey := client.ObjectKeyFromObject(gateway) @@ -304,84 +315,354 @@ var _ = Describe("RateLimitPolicy controller", func() { gwRLP.DirectReferenceAnnotationName(), client.ObjectKeyFromObject(gwRLP).String())) // check limits - limitadorKey := client.ObjectKey{Name: common.LimitadorName, Namespace: testNamespace} - existingLimitador := &limitadorv1alpha1.Limitador{} - Expect(k8sClient.Get(ctx, limitadorKey, existingLimitador)).To(Succeed()) - Expect(existingLimitador.Spec.Limits).To(ContainElements(limitadorv1alpha1.RateLimit{ + Eventually(func(g Gomega) { + limitadorKey := client.ObjectKey{Name: common.LimitadorName, Namespace: testNamespace} + existingLimitador := &limitadorv1alpha1.Limitador{} + g.Expect(k8sClient.Get(ctx, limitadorKey, existingLimitador)).To(Succeed()) + g.Expect(existingLimitador.Spec.Limits).To(ContainElements(limitadorv1alpha1.RateLimit{ + MaxValue: 10, + Seconds: 5, + Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), + Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Variables: []string{}, + Name: rlptools.LimitsNameFromRLP(routeRLP), + })) + }).WithContext(ctx).Should(Succeed()) + + // Gateway should contain HTTPRoute RLP in backreference + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, gwKey, existingGateway)).To(Succeed()) + serialized, err := json.Marshal(rlpKey) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(existingGateway.GetAnnotations()).To(HaveKey(routeRLP.BackReferenceAnnotationName())) + g.Expect(existingGateway.GetAnnotations()[routeRLP.BackReferenceAnnotationName()]).To(ContainSubstring(string(serialized))) + }).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(time.Minute)) + + It("Explicit defaults - no underlying routes to enforce policy", func(ctx SpecContext) { + gwRLP := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.Spec.TargetRef.Kind = "Gateway" + policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName) + }) + + Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed()) + gwRLPKey := client.ObjectKey{Name: gwRLP.Name, Namespace: testNamespace} + Eventually(testRLPIsAccepted(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeFalse()) + Expect(testRLPEnforcedCondition(gwRLPKey, kuadrant.PolicyReasonUnknown, "RateLimitPolicy has encountered some issues: no free routes to enforce policy")) + }, SpecTimeout(time.Minute)) + + It("Implicit defaults - no underlying routes to enforce policy", func(ctx SpecContext) { + gwRLP := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.Spec.TargetRef.Kind = "Gateway" + policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName) + policy.Spec.RateLimitPolicyCommonSpec = *policy.Spec.Defaults.DeepCopy() + policy.Spec.Defaults = nil + }) + + Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed()) + gwRLPKey := client.ObjectKey{Name: gwRLP.Name, Namespace: testNamespace} + Eventually(testRLPIsAccepted(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeFalse()) + Expect(testRLPEnforcedCondition(gwRLPKey, kuadrant.PolicyReasonUnknown, "RateLimitPolicy has encountered some issues: no free routes to enforce policy")) + }, SpecTimeout(time.Minute)) + }) + + Context("RLP Overrides", func() { + var gwRLP *kuadrantv1beta2.RateLimitPolicy + var routeRLP *kuadrantv1beta2.RateLimitPolicy + + limitadorContainsLimit := func(ctx context.Context, limit limitadorv1alpha1.RateLimit) func(g Gomega) { + return func(g Gomega) { + // check limits - should contain HTTPRoute RLP values + limitadorKey := client.ObjectKey{Name: common.LimitadorName, Namespace: testNamespace} + existingLimitador := &limitadorv1alpha1.Limitador{} + g.Expect(k8sClient.Get(ctx, limitadorKey, existingLimitador)).To(Succeed()) + g.Expect(existingLimitador.Spec.Limits).To(ContainElements(limit)) + } + } + + BeforeEach(func(ctx SpecContext) { + // create httproute + httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) + Expect(k8sClient.Create(ctx, httpRoute)).To(Succeed()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute))).WithContext(ctx).Should(BeTrue()) + + gwRLP = policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.Spec.TargetRef.Kind = "Gateway" + policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName) + policy.Spec.Overrides = policy.Spec.Defaults.DeepCopy() + policy.Spec.Defaults = nil + }) + + routeRLP = policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.Name = "httproute-rlp" + policy.Spec.CommonSpec().Limits = map[string]kuadrantv1beta2.Limit{ + "l1": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 10, Duration: 5, Unit: "second", + }, + }, + }, + } + }) + + }, NodeTimeout(time.Minute)) + + It("Gateway atomic override - gateway overrides exist and then route policy created", func(ctx SpecContext) { + // create GW RLP with overrides + Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed()) + gwRLPKey := client.ObjectKeyFromObject(gwRLP) + Eventually(testRLPIsAccepted(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + + // Create HTTPRoute RLP + Expect(k8sClient.Create(ctx, routeRLP)).To(Succeed()) + routeRLPKey := client.ObjectKeyFromObject(routeRLP) + Eventually(testRLPIsAccepted(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeFalse()) + Expect(testRLPEnforcedCondition(routeRLPKey, kuadrant.PolicyReasonOverridden, fmt.Sprintf("RateLimitPolicy is overridden by [%s]", gwRLPKey))) + + // Check Gateway direct back reference + gwKey := client.ObjectKeyFromObject(gateway) + existingGateway := &gatewayapiv1.Gateway{} + Expect(k8sClient.Get(ctx, gwKey, existingGateway)).To(Succeed()) + Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue( + gwRLP.DirectReferenceAnnotationName(), client.ObjectKeyFromObject(gwRLP).String())) + + // check limits - should contain override values + Eventually(limitadorContainsLimit(ctx, limitadorv1alpha1.RateLimit{ + MaxValue: 1, + Seconds: 180, + Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), + Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Variables: []string{}, + Name: rlptools.LimitsNameFromRLP(routeRLP), + })).WithContext(ctx).Should(Succeed()) + + // Gateway should contain HTTPRoute RLP in backreference + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, gwKey, existingGateway)).To(Succeed()) + serialized, err := json.Marshal(gwRLPKey) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(existingGateway.GetAnnotations()).To(HaveKey(routeRLP.BackReferenceAnnotationName())) + g.Expect(existingGateway.GetAnnotations()[routeRLP.BackReferenceAnnotationName()]).To(ContainSubstring(string(serialized))) + }).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(time.Minute)) + + It("Gateway atomic override - route policy exits and then gateway policy created", func(ctx SpecContext) { + // Create Route RLP + Expect(k8sClient.Create(ctx, routeRLP)).To(Succeed()) + routeRLPKey := client.ObjectKeyFromObject(routeRLP) + Eventually(testRLPIsAccepted(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + + // create GW RLP with override + Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed()) + gwRLPKey := client.ObjectKeyFromObject(gwRLP) + Eventually(testRLPIsAccepted(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + + // Route RLP should no longer be enforced + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeFalse()) + Expect(testRLPEnforcedCondition(routeRLPKey, kuadrant.PolicyReasonOverridden, fmt.Sprintf("RateLimitPolicy is overridden by [%s]", gwRLPKey))) + + // Check Gateway direct back reference + gwKey := client.ObjectKeyFromObject(gateway) + existingGateway := &gatewayapiv1.Gateway{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, gwKey, existingGateway)).To(Succeed()) + g.Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue( + gwRLP.DirectReferenceAnnotationName(), client.ObjectKeyFromObject(gwRLP).String())) + }).WithContext(ctx).Should(Succeed()) + + // Should contain override values + Eventually(limitadorContainsLimit(ctx, limitadorv1alpha1.RateLimit{ + MaxValue: 1, + Seconds: 180, + Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), + Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Variables: []string{}, + Name: rlptools.LimitsNameFromRLP(routeRLP), + })).WithContext(ctx).Should(Succeed()) + + // Gateway should contain HTTPRoute RLP in backreference + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, gwKey, existingGateway)).To(Succeed()) + serialized, err := json.Marshal(routeRLPKey) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(existingGateway.GetAnnotations()).To(HaveKey(routeRLP.BackReferenceAnnotationName())) + g.Expect(existingGateway.GetAnnotations()[routeRLP.BackReferenceAnnotationName()]).To(ContainSubstring(string(serialized))) + }).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(time.Minute)) + + It("Gateway atomic override - gateway defaults turned into overrides later on", func(ctx SpecContext) { + // Create Route RLP + Expect(k8sClient.Create(ctx, routeRLP)).To(Succeed()) + routeRLPKey := client.ObjectKeyFromObject(routeRLP) + Eventually(testRLPIsAccepted(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + + // Create GW RLP with defaults + gwRLP = policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.Spec.TargetRef.Kind = "Gateway" + policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName) + }) + Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed()) + gwRLPKey := client.ObjectKeyFromObject(gwRLP) + Eventually(testRLPIsAccepted(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeFalse()) + Expect(testRLPEnforcedCondition(gwRLPKey, kuadrant.PolicyReasonOverridden, fmt.Sprintf("RateLimitPolicy is overridden by [%s]", routeRLPKey))) + + // Route RLP should still be enforced + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + + // Should contain Route RLP values + Eventually(limitadorContainsLimit(ctx, limitadorv1alpha1.RateLimit{ MaxValue: 10, Seconds: 5, Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), Conditions: []string{`limit.l1__2804bad6 == "1"`}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(routeRLP), - })) + })).WithContext(ctx).Should(Succeed()) + + // Update GW RLP defaults to overrides + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, gwRLPKey, gwRLP)).To(Succeed()) + gwRLP.Spec.Overrides = gwRLP.Spec.Defaults.DeepCopy() + gwRLP.Spec.Defaults = nil + g.Expect(k8sClient.Update(ctx, gwRLP)).To(Succeed()) + }).WithContext(ctx).Should(Succeed()) + + // GW RLP should now be enforced + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeFalse()) + Expect(testRLPEnforcedCondition(routeRLPKey, kuadrant.PolicyReasonOverridden, fmt.Sprintf("RateLimitPolicy is overridden by [%s]", gwRLPKey))) + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + + // Should contain override values + Eventually(limitadorContainsLimit(ctx, limitadorv1alpha1.RateLimit{ + MaxValue: 1, + Seconds: 180, + Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), + Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Variables: []string{}, + Name: rlptools.LimitsNameFromRLP(routeRLP), + })).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(time.Minute)) - // Gateway should contain HTTPRoute RLP in backreference - Expect(k8sClient.Get(ctx, gwKey, existingGateway)).To(Succeed()) - serialized, err := json.Marshal(rlpKey) - Expect(err).ToNot(HaveOccurred()) - Expect(existingGateway.GetAnnotations()).To(HaveKey(routeRLP.BackReferenceAnnotationName())) - Expect(existingGateway.GetAnnotations()[routeRLP.BackReferenceAnnotationName()]).To(ContainSubstring(string(serialized))) + It("Gateway atomic override - gateway overrides turned into defaults later on", func(ctx SpecContext) { + // Create HTTPRoute RLP + Expect(k8sClient.Create(ctx, routeRLP)).To(Succeed()) + routeRLPKey := client.ObjectKeyFromObject(routeRLP) + Eventually(testRLPIsAccepted(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + + // create GW RLP with overrides + Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed()) + gwRLPKey := client.ObjectKey{Name: gwRLP.Name, Namespace: testNamespace} + Eventually(testRLPIsAccepted(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + // Route RLP should not be enforced + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeFalse()) + Expect(testRLPEnforcedCondition(routeRLPKey, kuadrant.PolicyReasonOverridden, fmt.Sprintf("RateLimitPolicy is overridden by [%s]", gwRLPKey))) + + // Should contain override values + Eventually(limitadorContainsLimit(ctx, limitadorv1alpha1.RateLimit{ + MaxValue: 1, + Seconds: 180, + Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), + Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Variables: []string{}, + Name: rlptools.LimitsNameFromRLP(routeRLP), + })).WithContext(ctx).Should(Succeed()) + + // Update GW RLP overrides to defaults + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, gwRLPKey, gwRLP)).To(Succeed()) + gwRLP.Spec.Defaults = gwRLP.Spec.Overrides.DeepCopy() + gwRLP.Spec.Overrides = nil + g.Expect(k8sClient.Update(ctx, gwRLP)).To(Succeed()) + }).WithContext(ctx).Should(Succeed()) + + // Route RLP now takes precedence + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeFalse()) + Expect(testRLPEnforcedCondition(gwRLPKey, kuadrant.PolicyReasonOverridden, fmt.Sprintf("RateLimitPolicy is overridden by [%s]", routeRLPKey))) + Eventually(testRLPIsEnforced(routeRLPKey)).WithContext(ctx).Should(BeTrue()) + + // Should contain Route RLP values + Eventually(limitadorContainsLimit(ctx, limitadorv1alpha1.RateLimit{ + MaxValue: 10, + Seconds: 5, + Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), + Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Variables: []string{}, + Name: rlptools.LimitsNameFromRLP(routeRLP), + })).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(2*time.Minute)) + + It("Gateway atomic override - no underlying routes to enforce policy", func(ctx SpecContext) { + // Delete HTTPRoute + Expect(k8sClient.Delete(ctx, &gatewayapiv1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: routeName, Namespace: testNamespace}})).To(Succeed()) + + // create GW RLP with overrides + Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed()) + gwRLPKey := client.ObjectKey{Name: gwRLP.Name, Namespace: testNamespace} + Eventually(testRLPIsAccepted(gwRLPKey)).WithContext(ctx).Should(BeTrue()) + Eventually(testRLPIsEnforced(gwRLPKey)).WithContext(ctx).Should(BeFalse()) + Expect(testRLPEnforcedCondition(gwRLPKey, kuadrant.PolicyReasonUnknown, "RateLimitPolicy has encountered some issues: no free routes to enforce policy")) }, SpecTimeout(time.Minute)) }) Context("RLP accepted condition reasons", func() { - assertAcceptedConditionFalse := func(rlp *kuadrantv1beta2.RateLimitPolicy, reason, message string) func() bool { - return func() bool { + assertAcceptedConditionFalse := func(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, reason, message string) func(g Gomega) { + return func(g Gomega) { rlpKey := client.ObjectKeyFromObject(rlp) existingRLP := &kuadrantv1beta2.RateLimitPolicy{} - err := k8sClient.Get(context.Background(), rlpKey, existingRLP) - if err != nil { - return false - } + g.Expect(k8sClient.Get(ctx, rlpKey, existingRLP)).To(Succeed()) cond := meta.FindStatusCondition(existingRLP.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) - if cond == nil { - return false - } - - return cond.Status == metav1.ConditionFalse && cond.Reason == reason && cond.Message == message + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status == metav1.ConditionFalse && cond.Reason == reason && cond.Message == message).To(BeTrue()) } } // Accepted reason is already tested generally by the existing tests - It("Target not found reason", func() { + It("Target not found reason", func(ctx SpecContext) { rlp := policyFactory() - err := k8sClient.Create(context.Background(), rlp) - Expect(err).ToNot(HaveOccurred()) + Expect(k8sClient.Create(ctx, rlp)).To(Succeed()) - Eventually(assertAcceptedConditionFalse(rlp, string(gatewayapiv1alpha2.PolicyReasonTargetNotFound), + Eventually(assertAcceptedConditionFalse(ctx, rlp, string(gatewayapiv1alpha2.PolicyReasonTargetNotFound), fmt.Sprintf("RateLimitPolicy target %s was not found", routeName)), - time.Minute, 5*time.Second).Should(BeTrue()) - }) + ).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(time.Minute)) - It("Conflict reason", func() { + It("Conflict reason", func(ctx SpecContext) { httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) - err := k8sClient.Create(context.Background(), httpRoute) - Expect(err).ToNot(HaveOccurred()) - Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) + Expect(k8sClient.Create(ctx, httpRoute)).To(Succeed()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute))).WithContext(ctx).Should(BeTrue()) rlp := policyFactory() - err = k8sClient.Create(context.Background(), rlp) - Expect(err).ToNot(HaveOccurred()) + Expect(k8sClient.Create(ctx, rlp)).To(Succeed()) + Eventually(testRLPIsAccepted(client.ObjectKeyFromObject(rlp))).WithContext(ctx).Should(BeTrue()) rlp2 := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { policy.Name = "conflicting-rlp" }) - err = k8sClient.Create(context.Background(), rlp2) - Expect(err).ToNot(HaveOccurred()) + Expect(k8sClient.Create(ctx, rlp2)).To(Succeed()) - Eventually(assertAcceptedConditionFalse(rlp2, string(gatewayapiv1alpha2.PolicyReasonConflicted), + Eventually(assertAcceptedConditionFalse(ctx, rlp2, string(gatewayapiv1alpha2.PolicyReasonConflicted), fmt.Sprintf("RateLimitPolicy is conflicted by %[1]v/toystore-rlp: the gateway.networking.k8s.io/v1, Kind=HTTPRoute target %[1]v/toystore-route is already referenced by policy %[1]v/toystore-rlp", testNamespace)), - time.Minute, 5*time.Second).Should(BeTrue()) - }) + ).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(2*time.Minute)) - It("Invalid reason", func() { + It("Invalid reason", func(ctx SpecContext) { var otherNamespace string CreateNamespace(&otherNamespace) - defer DeleteNamespaceCallback(&otherNamespace) + defer DeleteNamespaceCallback(&otherNamespace)() policy := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { policy.Namespace = otherNamespace // create the policy in a different namespace than the target @@ -389,10 +670,71 @@ var _ = Describe("RateLimitPolicy controller", func() { policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gateway.Name) policy.Spec.TargetRef.Namespace = ptr.To(gatewayapiv1.Namespace(testNamespace)) }) - Expect(k8sClient.Create(context.Background(), policy)).To(Succeed()) + Expect(k8sClient.Create(ctx, policy)).To(Succeed()) - Eventually(assertAcceptedConditionFalse(policy, string(gatewayapiv1alpha2.PolicyReasonInvalid), fmt.Sprintf("RateLimitPolicy target is invalid: invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", testNamespace)), 30*time.Second, 5*time.Second).Should(BeTrue()) - }) + Eventually(assertAcceptedConditionFalse(ctx, policy, string(gatewayapiv1alpha2.PolicyReasonInvalid), fmt.Sprintf("RateLimitPolicy target is invalid: invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", testNamespace))).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(time.Minute)) + }) + + Context("RLP Enforced Reasons", func() { + assertAcceptedCondTrueAndEnforcedCond := func(policy *kuadrantv1beta2.RateLimitPolicy, conditionStatus metav1.ConditionStatus, reason, message string) func(g Gomega) { + return func(g Gomega) { + existingPolicy := &kuadrantv1beta2.RateLimitPolicy{} + g.Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(policy), existingPolicy)).To(Succeed()) + acceptedCond := meta.FindStatusCondition(existingPolicy.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) + g.Expect(acceptedCond).ToNot(BeNil()) + + acceptedCondMatch := acceptedCond.Status == metav1.ConditionTrue && acceptedCond.Reason == string(gatewayapiv1alpha2.PolicyReasonAccepted) + + enforcedCond := meta.FindStatusCondition(existingPolicy.Status.Conditions, string(kuadrant.PolicyReasonEnforced)) + g.Expect(enforcedCond).ToNot(BeNil()) + enforcedCondMatch := enforcedCond.Status == conditionStatus && enforcedCond.Reason == reason && enforcedCond.Message == message + + g.Expect(acceptedCondMatch && enforcedCondMatch).To(BeTrue()) + } + } + + BeforeEach(func(ctx SpecContext) { + route := testBuildBasicHttpRoute(testHTTPRouteName, testGatewayName, testNamespace, []string{"*.toystore.com"}) + Expect(k8sClient.Create(ctx, route)).To(Succeed()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(route))).WithContext(ctx).Should(BeTrue()) + }, NodeTimeout(time.Minute)) + + It("Enforced Reason", func(ctx SpecContext) { + policy := policyFactory() + Expect(k8sClient.Create(ctx, policy)).To(Succeed()) + + Eventually(assertAcceptedCondTrueAndEnforcedCond(policy, metav1.ConditionTrue, string(kuadrant.PolicyReasonEnforced), + "RateLimitPolicy has been successfully enforced")).WithContext(ctx).Should(Succeed()) + + // Remove limitador deployment to simulate enforcement error + // RLP should transition to enforcement false in this case + Expect(k8sClient.Delete(ctx, &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "limitador-limitador", Namespace: testNamespace}})).To(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(ctx, client.ObjectKey{Name: "limitador-limitador", Namespace: testNamespace}, &appsv1.Deployment{})) + }).WithContext(ctx).Should(BeTrue()) + + Eventually(assertAcceptedCondTrueAndEnforcedCond(policy, metav1.ConditionFalse, string(kuadrant.PolicyReasonUnknown), + "RateLimitPolicy has encountered some issues: limitador is not ready")).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(time.Minute)) + + It("Unknown Reason", func(ctx SpecContext) { + // Remove limitador deployment to simulate enforcement error + Expect(k8sClient.Delete(ctx, &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "limitador-limitador", Namespace: testNamespace}})).To(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(ctx, client.ObjectKey{Name: "limitador-limitador", Namespace: testNamespace}, &appsv1.Deployment{})) + }).WithContext(ctx).Should(BeTrue()) + + // Enforced false as limitador is not ready + policy := policyFactory() + Expect(k8sClient.Create(ctx, policy)).To(Succeed()) + Eventually(assertAcceptedCondTrueAndEnforcedCond(policy, metav1.ConditionFalse, string(kuadrant.PolicyReasonUnknown), + "RateLimitPolicy has encountered some issues: limitador is not ready")).WithContext(ctx).Should(Succeed()) + + // Enforced true once limitador is ready + Eventually(assertAcceptedCondTrueAndEnforcedCond(policy, metav1.ConditionTrue, string(kuadrant.PolicyReasonEnforced), + "RateLimitPolicy has been successfully enforced")).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(time.Minute)) }) Context("When RLP switches target from one HTTPRoute to another HTTPRoute", func() { @@ -428,6 +770,7 @@ var _ = Describe("RateLimitPolicy controller", func() { // Check RLP status is available rlpKey := client.ObjectKeyFromObject(rlp) Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check HTTPRoute A direct back reference routeAKey := client.ObjectKey{Name: routeAName, Namespace: testNamespace} @@ -456,6 +799,7 @@ var _ = Describe("RateLimitPolicy controller", func() { Expect(err).ToNot(HaveOccurred()) // Check RLP status is available Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check HTTPRoute A direct back reference is gone Eventually( @@ -506,6 +850,8 @@ var _ = Describe("RateLimitPolicy controller", func() { // Check RLP status is available rlpKey := client.ObjectKeyFromObject(rlp) Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeFalse()) + Expect(testRLPEnforcedCondition(rlpKey, kuadrant.PolicyReasonUnknown, "RateLimitPolicy has encountered some issues: no free routes to enforce policy")) // Check Gateway direct back reference gwAKey := client.ObjectKey{Name: gwAName, Namespace: testNamespace} @@ -534,6 +880,8 @@ var _ = Describe("RateLimitPolicy controller", func() { Expect(err).ToNot(HaveOccurred()) // Check RLP status is available Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeFalse()) + Expect(testRLPEnforcedCondition(rlpKey, kuadrant.PolicyReasonUnknown, "RateLimitPolicy has encountered some issues: no free routes to enforce policy")) // Check Gw A direct back reference is gone Eventually( @@ -595,6 +943,7 @@ var _ = Describe("RateLimitPolicy controller", func() { rlpAKey := client.ObjectKeyFromObject(rlpA) Eventually(testRLPIsAccepted(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) // create rlpB rlpB := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { @@ -608,6 +957,7 @@ var _ = Describe("RateLimitPolicy controller", func() { // Check RLP status is available rlpBKey := client.ObjectKeyFromObject(rlpB) Eventually(testRLPIsAccepted(rlpBKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpBKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check HTTPRoute A direct back reference routeAKey := client.ObjectKey{Name: routeAName, Namespace: testNamespace} @@ -684,6 +1034,7 @@ var _ = Describe("RateLimitPolicy controller", func() { rlpKey := client.ObjectKeyFromObject(rlp) Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check HTTPRoute A direct back reference routeKey := client.ObjectKey{Name: routeName, Namespace: testNamespace} @@ -746,6 +1097,7 @@ var _ = Describe("RateLimitPolicy controller", func() { rlpAKey := client.ObjectKeyFromObject(rlpA) Eventually(testRLPIsAccepted(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) // Proceed with the update: // new RLP B -> Route A (already taken) @@ -762,6 +1114,7 @@ var _ = Describe("RateLimitPolicy controller", func() { // Check RLP status is not available rlpBKey := client.ObjectKeyFromObject(rlpB) Eventually(testRLPIsNotAccepted(rlpBKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpBKey), time.Minute, 5*time.Second).Should(BeFalse()) // Check HTTPRoute A direct back reference to RLP A routeAKey := client.ObjectKey{Name: routeAName, Namespace: testNamespace} @@ -800,6 +1153,7 @@ var _ = Describe("RateLimitPolicy controller", func() { time.Minute, 5*time.Second).Should(BeTrue()) Eventually(testRLPIsAccepted(rlpBKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpBKey), time.Minute, 5*time.Second).Should(BeTrue()) routeBKey := client.ObjectKey{Name: routeBName, Namespace: testNamespace} // Check HTTPRoute B direct back reference to RLP A @@ -811,6 +1165,7 @@ var _ = Describe("RateLimitPolicy controller", func() { time.Minute, 5*time.Second).Should(BeTrue()) Eventually(testRLPIsAccepted(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsEnforced(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) }) }) }) @@ -879,8 +1234,8 @@ var _ = Describe("RateLimitPolicy CEL Validations", func() { }) }) - Context("Defaults validation", func() { - It("Valid only implicit defaults", func(ctx SpecContext) { + Context("Defaults / Override validation", func() { + It("Valid - only implicit defaults defined", func(ctx SpecContext) { policy := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { policy.Spec.Limits = map[string]kuadrantv1beta2.Limit{ "implicit": { @@ -888,11 +1243,10 @@ var _ = Describe("RateLimitPolicy CEL Validations", func() { }, } }) - err := k8sClient.Create(ctx, policy) - Expect(err).To(BeNil()) + Expect(k8sClient.Create(ctx, policy)).To(Succeed()) }) - It("Valid only explicit defaults", func(ctx SpecContext) { + It("Valid - only explicit defaults defined", func(ctx SpecContext) { policy := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { policy.Spec.Defaults = &kuadrantv1beta2.RateLimitPolicyCommonSpec{ Limits: map[string]kuadrantv1beta2.Limit{ @@ -902,28 +1256,98 @@ var _ = Describe("RateLimitPolicy CEL Validations", func() { }, } }) + Expect(k8sClient.Create(ctx, policy)).To(Succeed()) + }) + + It("Invalid - implicit and explicit defaults are mutually exclusive", func(ctx SpecContext) { + policy := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.Spec.Defaults = &kuadrantv1beta2.RateLimitPolicyCommonSpec{ + Limits: map[string]kuadrantv1beta2.Limit{ + "explicit": { + Rates: []kuadrantv1beta2.Rate{{Limit: 1, Duration: 10, Unit: "second"}}, + }, + }, + } + policy.Spec.Limits = map[string]kuadrantv1beta2.Limit{ + "implicit": { + Rates: []kuadrantv1beta2.Rate{{Limit: 2, Duration: 20, Unit: "second"}}, + }, + } + }) err := k8sClient.Create(ctx, policy) - Expect(err).To(BeNil()) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("Implicit and explicit defaults are mutually exclusive")) }) - It("Invalid implicit and explicit defaults are mutually exclusive", func(ctx SpecContext) { + It("Invalid - explicit default and override defined", func(ctx SpecContext) { policy := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { policy.Spec.Defaults = &kuadrantv1beta2.RateLimitPolicyCommonSpec{ + Limits: map[string]kuadrantv1beta2.Limit{ + "implicit": { + Rates: []kuadrantv1beta2.Rate{{Limit: 2, Duration: 20, Unit: "second"}}, + }, + }, + } + policy.Spec.Overrides = &kuadrantv1beta2.RateLimitPolicyCommonSpec{ Limits: map[string]kuadrantv1beta2.Limit{ "explicit": { Rates: []kuadrantv1beta2.Rate{{Limit: 1, Duration: 10, Unit: "second"}}, }, }, } + }) + err := k8sClient.Create(ctx, policy) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("Overrides and explicit defaults are mutually exclusive")) + }) + + It("Invalid - implicit default and override defined", func(ctx SpecContext) { + policy := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { policy.Spec.Limits = map[string]kuadrantv1beta2.Limit{ "implicit": { Rates: []kuadrantv1beta2.Rate{{Limit: 2, Duration: 20, Unit: "second"}}, }, } + policy.Spec.Overrides = &kuadrantv1beta2.RateLimitPolicyCommonSpec{ + Limits: map[string]kuadrantv1beta2.Limit{ + "overrides": { + Rates: []kuadrantv1beta2.Rate{{Limit: 1, Duration: 10, Unit: "second"}}, + }, + }, + } }) err := k8sClient.Create(ctx, policy) - Expect(err).To(Not(BeNil())) - Expect(strings.Contains(err.Error(), "Implicit and explicit defaults are mutually exclusive")).To(BeTrue()) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("Overrides and implicit defaults are mutually exclusive")) + }) + + It("Invalid - policy override targeting resource other than Gateway", func(ctx SpecContext) { + policy := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.Spec.Overrides = &kuadrantv1beta2.RateLimitPolicyCommonSpec{ + Limits: map[string]kuadrantv1beta2.Limit{ + "implicit": { + Rates: []kuadrantv1beta2.Rate{{Limit: 1, Duration: 10, Unit: "second"}}, + }, + }, + } + }) + err := k8sClient.Create(ctx, policy) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("Overrides are only allowed for policies targeting a Gateway resource")) + }) + + It("Valid - policy override targeting Gateway", func(ctx SpecContext) { + policy := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.Spec.TargetRef.Kind = "Gateway" + policy.Spec.Overrides = &kuadrantv1beta2.RateLimitPolicyCommonSpec{ + Limits: map[string]kuadrantv1beta2.Limit{ + "override": { + Rates: []kuadrantv1beta2.Rate{{Limit: 1, Duration: 10, Unit: "second"}}, + }, + }, + } + }) + Expect(k8sClient.Create(ctx, policy)).To(Succeed()) }) }) @@ -941,7 +1365,7 @@ var _ = Describe("RateLimitPolicy CEL Validations", func() { "l1": { Rates: []kuadrantv1beta2.Rate{ { - Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + Limit: 1, Duration: 3, Unit: "minute", }, }, RouteSelectors: []kuadrantv1beta2.RouteSelector{ diff --git a/controllers/ratelimitpolicy_limitador_status_event_handler.go b/controllers/ratelimitpolicy_limitador_status_event_handler.go new file mode 100644 index 000000000..69ba179aa --- /dev/null +++ b/controllers/ratelimitpolicy_limitador_status_event_handler.go @@ -0,0 +1,84 @@ +package controllers + +import ( + "context" + + "github.com/go-logr/logr" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/util/workqueue" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + + kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" + kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" + "github.com/kuadrant/kuadrant-operator/pkg/common" +) + +var _ handler.EventHandler = &limitadorStatusEventHandler{} + +type limitadorStatusEventHandler struct { + Client client.Client + Logger logr.Logger +} + +func (eh limitadorStatusEventHandler) Create(_ context.Context, _ event.CreateEvent, _ workqueue.RateLimitingInterface) { +} + +func (eh limitadorStatusEventHandler) Update(ctx context.Context, e event.UpdateEvent, limitingInterface workqueue.RateLimitingInterface) { + oldL := e.ObjectOld.(*limitadorv1alpha1.Limitador) + newL := e.ObjectNew.(*limitadorv1alpha1.Limitador) + + if !eh.IsKuadrantInstalled(ctx, oldL) { + return + } + + oldCond := meta.FindStatusCondition(oldL.Status.Conditions, "Ready") + newCond := meta.FindStatusCondition(newL.Status.Conditions, "Ready") + + if oldCond != nil && newCond != nil && oldCond.Status != newCond.Status && oldL.Name == common.LimitadorName { + eh.Logger.V(1).Info("Limitador status Ready condition change event detected") + eh.enqueue(ctx, limitingInterface) + } +} + +func (eh limitadorStatusEventHandler) Delete(ctx context.Context, e event.DeleteEvent, limitingInterface workqueue.RateLimitingInterface) { + eh.Logger.V(1).Info("Limitador delete event detected") + if !eh.IsKuadrantInstalled(ctx, e.Object) || e.Object.GetName() == common.LimitadorName { + eh.enqueue(ctx, limitingInterface) + } +} + +func (eh limitadorStatusEventHandler) Generic(_ context.Context, _ event.GenericEvent, _ workqueue.RateLimitingInterface) { +} + +func (eh limitadorStatusEventHandler) IsKuadrantInstalled(ctx context.Context, obj client.Object) bool { + kuadrantList := &kuadrantv1beta1.KuadrantList{} + if err := eh.Client.List(ctx, kuadrantList, &client.ListOptions{Namespace: obj.GetNamespace()}); err != nil { + eh.Logger.V(1).Error(err, "failed to list kuadrant in namespace", "namespace", obj.GetNamespace()) + return false + } + + // No kuadrant in limitador namespace - skipping as it's not managed by kuadrant + if len(kuadrantList.Items) == 0 { + eh.Logger.V(1).Info("no kuadrant resources found in limitador namespace, skipping") + return false + } + + return true +} +func (eh limitadorStatusEventHandler) enqueue(ctx context.Context, limitingInterface workqueue.RateLimitingInterface) { + // List all RLPs as there's been an event from Limitador which may affect RLP status + rlpList := &kuadrantv1beta2.RateLimitPolicyList{} + if err := eh.Client.List(ctx, rlpList); err != nil { + eh.Logger.V(1).Error(err, "failed to list RLPs") + } + for idx := range rlpList.Items { + eh.Logger.V(1).Info("queueing rate limiting policy", "policy", rlpList.Items[idx].Name) + limitingInterface.Add(ctrl.Request{ + NamespacedName: client.ObjectKeyFromObject(&rlpList.Items[idx]), + }) + } +} diff --git a/controllers/ratelimitpolicy_limits.go b/controllers/ratelimitpolicy_limits.go index f8b421962..a90817570 100644 --- a/controllers/ratelimitpolicy_limits.go +++ b/controllers/ratelimitpolicy_limits.go @@ -2,13 +2,18 @@ package controllers import ( "context" + "slices" + "sort" "github.com/go-logr/logr" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/common" + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" "github.com/kuadrant/kuadrant-operator/pkg/rlptools" @@ -43,8 +48,31 @@ func (r *RateLimitPolicyReconciler) reconcileLimitador(ctx context.Context, rlp if err != nil { return err } - // get the current limitador cr for the kuadrant instance so we can compare if it needs to be updated + limitador, err := r.getLimitador(ctx, rlp) + if err != nil { + return err + } + // return if limitador is up-to-date + if rlptools.Equal(rateLimitIndex.ToRateLimits(), limitador.Spec.Limits) { + logger.V(1).Info("limitador is up to date, skipping update") + return nil + } + + // update limitador + limitador.Spec.Limits = rateLimitIndex.ToRateLimits() + err = r.UpdateResource(ctx, limitador) + logger.V(1).Info("update limitador", "limitador", client.ObjectKeyFromObject(limitador), "err", err) + if err != nil { + return err + } + + return nil +} + +func (r *RateLimitPolicyReconciler) getLimitador(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy) (*limitadorv1alpha1.Limitador, error) { + logger, _ := logr.FromContext(ctx) + logger.V(1).Info("get kuadrant namespace") var kuadrantNamespace string kuadrantNamespace, isSet := kuadrant.GetKuadrantNamespaceFromPolicy(rlp) @@ -53,44 +81,35 @@ func (r *RateLimitPolicyReconciler) reconcileLimitador(ctx context.Context, rlp kuadrantNamespace, err = kuadrant.GetKuadrantNamespaceFromPolicyTargetRef(ctx, r.Client(), rlp) if err != nil { logger.Error(err, "failed to get kuadrant namespace") - return err + return nil, err } kuadrant.AnnotateObject(rlp, kuadrantNamespace) err = r.UpdateResource(ctx, rlp) // @guicassolato: not sure if this belongs to here if err != nil { logger.Error(err, "failed to update policy, re-queuing") - return err + return nil, err } } limitadorKey := client.ObjectKey{Name: common.LimitadorName, Namespace: kuadrantNamespace} limitador := &limitadorv1alpha1.Limitador{} - err = r.Client().Get(ctx, limitadorKey, limitador) + err := r.Client().Get(ctx, limitadorKey, limitador) logger.V(1).Info("get limitador", "limitador", limitadorKey, "err", err) if err != nil { - return err - } - - // return if limitador is up to date - if rlptools.Equal(rateLimitIndex.ToRateLimits(), limitador.Spec.Limits) { - logger.V(1).Info("limitador is up to date, skipping update") - return nil + return nil, err } - // update limitador - limitador.Spec.Limits = rateLimitIndex.ToRateLimits() - err = r.UpdateResource(ctx, limitador) - logger.V(1).Info("update limitador", "limitador", limitadorKey, "err", err) - if err != nil { - return err - } - - return nil + return limitador, nil } func (r *RateLimitPolicyReconciler) buildRateLimitIndex(ctx context.Context, rlpRefs []client.ObjectKey) (*rlptools.RateLimitIndex, error) { logger, _ := logr.FromContext(ctx) logger = logger.WithName("buildRateLimitIndex").WithValues("ratelimitpolicies", rlpRefs) + t, err := r.generateTopology(ctx) + if err != nil { + return nil, err + } + rateLimitIndex := rlptools.NewRateLimitIndex() for _, rlpKey := range rlpRefs { @@ -105,8 +124,137 @@ func (r *RateLimitPolicyReconciler) buildRateLimitIndex(ctx context.Context, rlp return nil, err } + if err := r.applyOverrides(ctx, rlp, t); err != nil { + return nil, err + } + rateLimitIndex.Set(rlpKey, rlptools.LimitadorRateLimitsFromRLP(rlp)) } return rateLimitIndex, nil } + +func (r *RateLimitPolicyReconciler) applyOverrides(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, t *kuadrantgatewayapi.Topology) error { + logger, _ := logr.FromContext(ctx) + logger = logger.WithName("applyOverrides") + + // Retrieve affected policies and the number of untargeted routes + affectedPolicies, numUnTargetedRoutes := r.getAffectedPoliciesInfo(rlp, t) + + // Filter out current policy from affected policies + affectedPolicies = utils.Filter(affectedPolicies, func(policy kuadrantgatewayapi.Policy) bool { + return policy.GetUID() != rlp.GetUID() + }) + + // Apply overrides based on the type of policy (Gateway or Route) + if kuadrantgatewayapi.IsTargetRefGateway(rlp.GetTargetRef()) { + r.applyGatewayOverrides(logger, rlp, numUnTargetedRoutes, affectedPolicies) + } else { + affectedPolicies = r.applyRouteOverrides(logger, rlp, affectedPolicies) + } + + // Reconcile status for affected policies + for _, policy := range affectedPolicies { + p := policy.(*kuadrantv1beta2.RateLimitPolicy) + // Override is set -> affected policy is Affected by rlp + if kuadrantgatewayapi.IsTargetRefGateway(rlp.GetTargetRef()) && rlp.Spec.Overrides != nil { + r.AffectedPolicyMap.SetAffectedPolicy(p, []client.ObjectKey{client.ObjectKeyFromObject(rlp)}) + } + _, err := r.reconcileStatus(ctx, p, nil) + if err != nil { + return err + } + } + + return nil +} + +func (r *RateLimitPolicyReconciler) getAffectedPoliciesInfo(rlp *kuadrantv1beta2.RateLimitPolicy, t *kuadrantgatewayapi.Topology) ([]kuadrantgatewayapi.Policy, int) { + topologyIndexes := kuadrantgatewayapi.NewTopologyIndexes(t) + var affectedPolicies []kuadrantgatewayapi.Policy + var numUnTargetedRoutes int + + for _, gw := range t.Gateways() { + policyList := topologyIndexes.PoliciesFromGateway(gw.Gateway) + numUnTargetedRoutes += len(topologyIndexes.GetUntargetedRoutes(gw.Gateway)) + + if slices.Contains(utils.Map(policyList, func(p kuadrantgatewayapi.Policy) client.ObjectKey { + return client.ObjectKeyFromObject(p) + }), client.ObjectKeyFromObject(rlp)) { + affectedPolicies = append(affectedPolicies, policyList...) + } + } + + return affectedPolicies, numUnTargetedRoutes +} + +// applyGatewayOverrides a Gateway RLP is not affected if there is untargetted routes or affects other policies +// Otherwise, it has no free routes, and therefore "overridden" by the route policies +func (r *RateLimitPolicyReconciler) applyGatewayOverrides(logger logr.Logger, rlp *kuadrantv1beta2.RateLimitPolicy, numUnTargetedRoutes int, affectedPolicies []kuadrantgatewayapi.Policy) { + if rlp.Spec.Overrides == nil && numUnTargetedRoutes == 0 { + r.AffectedPolicyMap.SetAffectedPolicy(rlp, utils.Map(affectedPolicies, func(p kuadrantgatewayapi.Policy) client.ObjectKey { + return client.ObjectKeyFromObject(p) + })) + logger.V(1).Info("policy has no free routes to enforce default policy") + } else if rlp.Spec.Overrides != nil && len(affectedPolicies) == 0 && numUnTargetedRoutes == 0 { + r.AffectedPolicyMap.SetAffectedPolicy(rlp, []client.ObjectKey{}) + logger.V(1).Info("policy has no free routes to enforce override policy") + } else { + r.AffectedPolicyMap.RemoveAffectedPolicy(rlp) + } +} + +func (r *RateLimitPolicyReconciler) applyRouteOverrides(logger logr.Logger, rlp *kuadrantv1beta2.RateLimitPolicy, affectedPolicies []kuadrantgatewayapi.Policy) []kuadrantgatewayapi.Policy { + filteredPolicies := utils.Filter(affectedPolicies, func(policy kuadrantgatewayapi.Policy) bool { + return kuadrantgatewayapi.IsTargetRefGateway(policy.GetTargetRef()) + }) + + sort.Sort(kuadrantgatewayapi.PolicyByTargetRefKindAndCreationTimeStamp(filteredPolicies)) + + for _, policy := range filteredPolicies { + p := policy.(*kuadrantv1beta2.RateLimitPolicy) + if p.Spec.Overrides != nil { + rlp.Spec.CommonSpec().Limits = p.Spec.Overrides.Limits + logger.V(1).Info("applying overrides from parent policy", "parentPolicy", client.ObjectKeyFromObject(p)) + r.AffectedPolicyMap.SetAffectedPolicy(rlp, []client.ObjectKey{client.ObjectKeyFromObject(p)}) + break + } + r.AffectedPolicyMap.RemoveAffectedPolicy(rlp) + } + + return filteredPolicies +} + +func (r *RateLimitPolicyReconciler) generateTopology(ctx context.Context) (*kuadrantgatewayapi.Topology, error) { + logger, _ := logr.FromContext(ctx) + + gwList := &gatewayapiv1.GatewayList{} + err := r.Client().List(ctx, gwList) + logger.V(1).Info("topology: list gateways", "#Gateways", len(gwList.Items), "err", err) + if err != nil { + return nil, err + } + + routeList := &gatewayapiv1.HTTPRouteList{} + err = r.Client().List(ctx, routeList) + logger.V(1).Info("topology: list httproutes", "#HTTPRoutes", len(routeList.Items), "err", err) + if err != nil { + return nil, err + } + + rlpList := &kuadrantv1beta2.RateLimitPolicyList{} + err = r.Client().List(ctx, rlpList) + logger.V(1).Info("topology: list rate limit policies", "#RLPS", len(rlpList.Items), "err", err) + if err != nil { + return nil, err + } + + policies := utils.Map(rlpList.Items, func(p kuadrantv1beta2.RateLimitPolicy) kuadrantgatewayapi.Policy { return &p }) + + return kuadrantgatewayapi.NewTopology( + kuadrantgatewayapi.WithGateways(utils.Map(gwList.Items, ptr.To[gatewayapiv1.Gateway])), + kuadrantgatewayapi.WithRoutes(utils.Map(routeList.Items, ptr.To[gatewayapiv1.HTTPRoute])), + kuadrantgatewayapi.WithPolicies(policies), + kuadrantgatewayapi.WithLogger(logger), + ) +} diff --git a/controllers/ratelimitpolicy_status.go b/controllers/ratelimitpolicy_status.go index 0edffd78e..08abb28dc 100644 --- a/controllers/ratelimitpolicy_status.go +++ b/controllers/ratelimitpolicy_status.go @@ -2,14 +2,17 @@ package controllers import ( "context" + "errors" "fmt" "slices" "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/reconcile" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" @@ -51,7 +54,7 @@ func (r *RateLimitPolicyReconciler) reconcileStatus(ctx context.Context, rlp *ku return ctrl.Result{}, nil } -func (r *RateLimitPolicyReconciler) calculateStatus(_ context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, specErr error) *kuadrantv1beta2.RateLimitPolicyStatus { +func (r *RateLimitPolicyReconciler) calculateStatus(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, specErr error) *kuadrantv1beta2.RateLimitPolicyStatus { newStatus := &kuadrantv1beta2.RateLimitPolicyStatus{ // Copy initial conditions. Otherwise, status will always be updated Conditions: slices.Clone(rlp.Status.Conditions), @@ -62,5 +65,37 @@ func (r *RateLimitPolicyReconciler) calculateStatus(_ context.Context, rlp *kuad meta.SetStatusCondition(&newStatus.Conditions, *acceptedCond) + // Do not set enforced condition if Accepted condition is false + if meta.IsStatusConditionFalse(newStatus.Conditions, string(gatewayapiv1alpha2.PolicyReasonAccepted)) { + return newStatus + } + + enforcedCond := r.enforcedCondition(ctx, rlp) + meta.SetStatusCondition(&newStatus.Conditions, *enforcedCond) + return newStatus } + +func (r *RateLimitPolicyReconciler) enforcedCondition(ctx context.Context, policy *kuadrantv1beta2.RateLimitPolicy) *metav1.Condition { + logger, _ := logr.FromContext(ctx) + + limitador, err := r.getLimitador(ctx, policy) + if err != nil { + logger.V(1).Error(err, "failed to get limitador") + return kuadrant.EnforcedCondition(policy, kuadrant.NewErrUnknown(policy.Kind(), err), false) + } + if meta.IsStatusConditionFalse(limitador.Status.Conditions, "Ready") { + logger.V(1).Info("Limitador is not ready") + return kuadrant.EnforcedCondition(policy, kuadrant.NewErrUnknown(policy.Kind(), errors.New("limitador is not ready")), false) + } + + if r.AffectedPolicyMap.IsPolicyAffected(policy) { + if !r.AffectedPolicyMap.IsPolicyOverridden(policy) { + return kuadrant.EnforcedCondition(policy, kuadrant.NewErrUnknown(policy.Kind(), errors.New("no free routes to enforce policy")), false) // Maybe this should be a standard condition rather than an unknown condition + } + return kuadrant.EnforcedCondition(policy, kuadrant.NewErrOverridden(policy.Kind(), r.AffectedPolicyMap.PolicyAffectedBy(policy)), false) + } + + logger.V(1).Info("RateLimitPolicy is enforced") + return kuadrant.EnforcedCondition(policy, nil, true) +} diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 61edd2e7c..6a3033b2a 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -147,7 +147,7 @@ var _ = BeforeSuite(func() { err = (&AuthPolicyReconciler{ BaseReconciler: authPolicyBaseReconciler, TargetRefReconciler: reconcilers.TargetRefReconciler{Client: mgr.GetClient()}, - OverriddenPolicyMap: kuadrant.NewOverriddenPolicyMap(), + AffectedPolicyMap: kuadrant.NewAffectedPolicyMap(), }).SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred()) @@ -160,6 +160,7 @@ var _ = BeforeSuite(func() { err = (&RateLimitPolicyReconciler{ BaseReconciler: rateLimitPolicyBaseReconciler, TargetRefReconciler: reconcilers.TargetRefReconciler{Client: mgr.GetClient()}, + AffectedPolicyMap: kuadrant.NewAffectedPolicyMap(), }).SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred()) diff --git a/doc/reference/ratelimitpolicy.md b/doc/reference/ratelimitpolicy.md index aac4fb735..72403cb1c 100644 --- a/doc/reference/ratelimitpolicy.md +++ b/doc/reference/ratelimitpolicy.md @@ -18,11 +18,12 @@ ## RateLimitPolicySpec -| **Field** | **Type** | **Required** | **Description** | -|-------------|---------------------------------------------------------------------------------------------------------------------------------------------|--------------|-------------------------------------------------------------------------------------------------------------| -| `targetRef` | [PolicyTargetReference](https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.PolicyTargetReference) | Yes | Reference to a Kubernetes resource that the policy attaches to | -| `defaults` | [RateLimitPolicyCommonSpec](#rateLimitPolicyCommonSpec) | No | Default limit definitions. This field is mutually exclusive with the `limits` field | -| `limits` | Map | No | Limit definitions. This field is mutually exclusive with the [`defaults`](#rateLimitPolicyCommonSpec) field | +| **Field** | **Type** | **Required** | **Description** | +|-------------|---------------------------------------------------------------------------------------------------------------------------------------------|--------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `targetRef` | [PolicyTargetReference](https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.PolicyTargetReference) | Yes | Reference to a Kubernetes resource that the policy attaches to | +| `defaults` | [RateLimitPolicyCommonSpec](#rateLimitPolicyCommonSpec) | No | Default limit definitions. This field is mutually exclusive with the `limits` field | +| `overrides` | [RateLimitPolicyCommonSpec](#rateLimitPolicyCommonSpec) | No | Overrides limit definitions. This field is mutually exclusive with the `limits` field and `defaults` field. This field is only allowed for policies targeting `Gateway` in `targetRef.kind` | +| `limits` | Map | No | Limit definitions. This field is mutually exclusive with the [`defaults`](#rateLimitPolicyCommonSpec) field | ### RateLimitPolicyCommonSpec diff --git a/go.mod b/go.mod index 2a4adfd2e..85961e5e8 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,6 @@ require ( github.com/onsi/gomega v1.30.0 go.uber.org/zap v1.26.0 golang.org/x/net v0.19.0 - golang.org/x/sync v0.5.0 google.golang.org/protobuf v1.33.0 gotest.tools v2.2.0+incompatible istio.io/api v1.20.0 @@ -151,6 +150,7 @@ require ( golang.org/x/crypto v0.17.0 // indirect golang.org/x/exp v0.0.0-20231127185646-65229373498e // indirect golang.org/x/oauth2 v0.15.0 // indirect + golang.org/x/sync v0.5.0 // indirect golang.org/x/sys v0.15.0 // indirect golang.org/x/term v0.15.0 // indirect golang.org/x/text v0.14.0 // indirect diff --git a/main.go b/main.go index 75f44f596..9cdba3237 100644 --- a/main.go +++ b/main.go @@ -154,6 +154,7 @@ func main() { if err = (&controllers.RateLimitPolicyReconciler{ TargetRefReconciler: reconcilers.TargetRefReconciler{Client: mgr.GetClient()}, BaseReconciler: rateLimitPolicyBaseReconciler, + AffectedPolicyMap: kuadrant.NewAffectedPolicyMap(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "RateLimitPolicy") os.Exit(1) @@ -168,7 +169,7 @@ func main() { if err = (&controllers.AuthPolicyReconciler{ TargetRefReconciler: reconcilers.TargetRefReconciler{Client: mgr.GetClient()}, BaseReconciler: authPolicyBaseReconciler, - OverriddenPolicyMap: kuadrant.NewOverriddenPolicyMap(), + AffectedPolicyMap: kuadrant.NewAffectedPolicyMap(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "AuthPolicy") os.Exit(1) diff --git a/pkg/library/gatewayapi/types.go b/pkg/library/gatewayapi/types.go index b2a223baa..751f37bfd 100644 --- a/pkg/library/gatewayapi/types.go +++ b/pkg/library/gatewayapi/types.go @@ -19,7 +19,36 @@ func (a PolicyByCreationTimestamp) Less(i, j int) bool { p1Time := ptr.To(a[i].GetCreationTimestamp()) p2Time := ptr.To(a[j].GetCreationTimestamp()) if !p1Time.Equal(p2Time) { - return ptr.To(a[i].GetCreationTimestamp()).Before(ptr.To(a[j].GetCreationTimestamp())) + return p1Time.Before(p2Time) + } + + // The policy appearing first in alphabetical order by "{namespace}/{name}". + return client.ObjectKeyFromObject(a[i]).String() < client.ObjectKeyFromObject(a[j]).String() +} + +type PolicyByTargetRefKindAndCreationTimeStamp []Policy + +func (a PolicyByTargetRefKindAndCreationTimeStamp) Len() int { return len(a) } +func (a PolicyByTargetRefKindAndCreationTimeStamp) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a PolicyByTargetRefKindAndCreationTimeStamp) Less(i, j int) bool { + targetRef1 := a[i].GetTargetRef() + targetRef2 := a[j].GetTargetRef() + + // Compare kind first + if targetRef1.Kind != targetRef2.Kind { + if targetRef1.Kind == "Gateway" { + return true + } else if targetRef2.Kind == "HTTPRoute" { + return false + } + return targetRef1.Kind < targetRef2.Kind + } + + // Then compare timestamp + p1Time := ptr.To(a[i].GetCreationTimestamp()) + p2Time := ptr.To(a[j].GetCreationTimestamp()) + if !p1Time.Equal(p2Time) { + return p1Time.Before(p2Time) } // The policy appearing first in alphabetical order by "{namespace}/{name}". diff --git a/pkg/library/gatewayapi/types_test.go b/pkg/library/gatewayapi/types_test.go index 00f02d17e..5c500993f 100644 --- a/pkg/library/gatewayapi/types_test.go +++ b/pkg/library/gatewayapi/types_test.go @@ -10,6 +10,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -105,12 +106,81 @@ func TestPolicyByCreationTimestamp(t *testing.T) { } } -func createTestPolicy(name string, creationTime time.Time) *TestPolicy { - return &TestPolicy{ +func TestPolicyByTargetRef(t *testing.T) { + testCases := []struct { + name string + policies []Policy + sortedPolicies []Policy + }{ + { + name: "nil input", + policies: nil, + sortedPolicies: nil, + }, + { + name: "empty slices", + policies: make([]Policy, 0), + sortedPolicies: make([]Policy, 0), + }, + { + name: "by kind, and creation date", + policies: []Policy{ + createTestPolicy("ccc", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Gateway")), + createTestPolicy("bbb", time.Date(2000, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("HTTPRoute")), + createTestPolicy("aaa", time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Gateway")), + createTestPolicy("ddd", time.Date(2005, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Service")), + }, + sortedPolicies: []Policy{ + createTestPolicy("aaa", time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Gateway")), + createTestPolicy("ccc", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Gateway")), + createTestPolicy("bbb", time.Date(2000, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("HTTPRoute")), + createTestPolicy("ddd", time.Date(2005, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Service")), + }, + }, + { + name: "by kind, and then name when creation date equal", + policies: []Policy{ + createTestPolicy("ccc", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Gateway")), + createTestPolicy("bbb", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("HTTPRoute")), + createTestPolicy("aaa", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Gateway")), + createTestPolicy("ddd", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Service")), + }, + sortedPolicies: []Policy{ + createTestPolicy("aaa", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Gateway")), + createTestPolicy("ccc", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Gateway")), + createTestPolicy("bbb", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("HTTPRoute")), + createTestPolicy("ddd", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC), withTargetRefKind("Service")), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(subT *testing.T) { + sort.Sort(PolicyByTargetRefKindAndCreationTimeStamp(tc.policies)) + if !reflect.DeepEqual(tc.policies, tc.sortedPolicies) { + subT.Errorf("expected=%v; got=%v", tc.sortedPolicies, tc.policies) + } + }) + } +} + +func createTestPolicy(name string, creationTime time.Time, mutateFn ...func(p *TestPolicy)) *TestPolicy { + p := &TestPolicy{ ObjectMeta: metav1.ObjectMeta{ Namespace: "testnamespace", Name: name, - CreationTimestamp: metav1.Time{creationTime}, + CreationTimestamp: metav1.Time{Time: creationTime}, }, } + for _, fn := range mutateFn { + fn(p) + } + + return p +} + +func withTargetRefKind(targetRefKind string) func(p *TestPolicy) { + return func(p *TestPolicy) { + p.TargetRef = gatewayapiv1alpha2.PolicyTargetReference{Kind: gatewayapiv1.Kind(targetRefKind)} + } } diff --git a/pkg/library/kuadrant/apimachinery_status_conditions.go b/pkg/library/kuadrant/apimachinery_status_conditions.go index a87123b8a..fb0822ac5 100644 --- a/pkg/library/kuadrant/apimachinery_status_conditions.go +++ b/pkg/library/kuadrant/apimachinery_status_conditions.go @@ -10,6 +10,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -21,38 +22,49 @@ const ( PolicyReasonUnknown gatewayapiv1alpha2.PolicyConditionReason = "Unknown" ) -func NewOverriddenPolicyMap() *OverriddenPolicyMap { - return &OverriddenPolicyMap{ - policies: make(map[types.UID]bool), +func NewAffectedPolicyMap() *AffectedPolicyMap { + return &AffectedPolicyMap{ + policies: make(map[types.UID][]client.ObjectKey), } } -type OverriddenPolicyMap struct { - policies map[types.UID]bool +type AffectedPolicyMap struct { + policies map[types.UID][]client.ObjectKey mu sync.RWMutex } -// SetOverriddenPolicy sets the provided Policy as overridden in the tracking map. -func (o *OverriddenPolicyMap) SetOverriddenPolicy(p Policy) { +// SetAffectedPolicy sets the provided Policy as Affected in the tracking map. +func (o *AffectedPolicyMap) SetAffectedPolicy(p Policy, affectedBy []client.ObjectKey) { o.mu.Lock() defer o.mu.Unlock() if o.policies == nil { - o.policies = make(map[types.UID]bool) + o.policies = make(map[types.UID][]client.ObjectKey) } - o.policies[p.GetUID()] = true + o.policies[p.GetUID()] = affectedBy } -// RemoveOverriddenPolicy removes the provided Policy from the tracking map of overridden policies. -func (o *OverriddenPolicyMap) RemoveOverriddenPolicy(p Policy) { +// RemoveAffectedPolicy removes the provided Policy from the tracking map of Affected policies. +func (o *AffectedPolicyMap) RemoveAffectedPolicy(p Policy) { o.mu.Lock() defer o.mu.Unlock() delete(o.policies, p.GetUID()) } -// IsPolicyOverridden checks if the provided Policy is overridden based on the tracking map maintained. -func (o *OverriddenPolicyMap) IsPolicyOverridden(p Policy) bool { +// IsPolicyAffected checks if the provided Policy is affected based on the tracking map maintained. +func (o *AffectedPolicyMap) IsPolicyAffected(p Policy) bool { + return o.policies[p.GetUID()] != nil +} + +// IsPolicyOverridden checks if the provided Policy is affected based on the tracking map maintained. +// It is overridden if there is policies affecting it +func (o *AffectedPolicyMap) IsPolicyOverridden(p Policy) bool { + return o.IsPolicyAffected(p) && len(o.policies[p.GetUID()]) > 0 +} + +// PolicyAffectedBy returns the clients keys that a policy is Affected by +func (o *AffectedPolicyMap) PolicyAffectedBy(p Policy) []client.ObjectKey { return o.policies[p.GetUID()] } diff --git a/pkg/library/kuadrant/apimachinery_status_conditions_test.go b/pkg/library/kuadrant/apimachinery_status_conditions_test.go index 3c33968cc..5d5b27362 100644 --- a/pkg/library/kuadrant/apimachinery_status_conditions_test.go +++ b/pkg/library/kuadrant/apimachinery_status_conditions_test.go @@ -12,6 +12,7 @@ import ( apiErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -265,13 +266,13 @@ func TestEnforcedCondition(t *testing.T) { name: "enforced false - overridden", args: args{ policy: &FakePolicy{}, - err: NewErrOverridden(policy.Kind(), "ns1/policy1"), + err: NewErrOverridden(policy.Kind(), []client.ObjectKey{{Namespace: "ns1", Name: "policy1"}, {Namespace: "ns2", Name: "policy2"}}), }, want: &metav1.Condition{ Type: string(PolicyConditionEnforced), Status: metav1.ConditionFalse, Reason: string(PolicyReasonOverridden), - Message: "FakePolicy is overridden by ns1/policy1", + Message: "FakePolicy is overridden by [ns1/policy1 ns2/policy2]", }, }, } diff --git a/pkg/library/kuadrant/errors.go b/pkg/library/kuadrant/errors.go index 42847d823..48bde13ab 100644 --- a/pkg/library/kuadrant/errors.go +++ b/pkg/library/kuadrant/errors.go @@ -5,6 +5,7 @@ import ( "fmt" apierrors "k8s.io/apimachinery/pkg/api/errors" + "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -113,7 +114,7 @@ var _ PolicyError = ErrOverridden{} type ErrOverridden struct { Kind string - OverridingPolicies string + OverridingPolicies []client.ObjectKey } func (e ErrOverridden) Error() string { @@ -124,7 +125,7 @@ func (e ErrOverridden) Reason() gatewayapiv1alpha2.PolicyConditionReason { return PolicyReasonOverridden } -func NewErrOverridden(kind, overridingPolicies string) ErrOverridden { +func NewErrOverridden(kind string, overridingPolicies []client.ObjectKey) ErrOverridden { return ErrOverridden{ Kind: kind, OverridingPolicies: overridingPolicies, diff --git a/pkg/library/kuadrant/httproute_wrapper.go b/pkg/library/kuadrant/httproute_wrapper.go deleted file mode 100644 index a839a5bc6..000000000 --- a/pkg/library/kuadrant/httproute_wrapper.go +++ /dev/null @@ -1,41 +0,0 @@ -package kuadrant - -import ( - "github.com/kuadrant/kuadrant-operator/pkg/common" - kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" - gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" -) - -type HTTPRouteWrapper struct { - *gatewayapiv1.HTTPRoute - Referrer -} - -func (r HTTPRouteWrapper) PolicyRefs(t *kuadrantgatewayapi.Topology) []string { - if r.HTTPRoute == nil { - return make([]string, 0) - } - refs := make([]string, 0) - for _, gw := range t.Gateways() { - authPolicyRefs, ok := gw.GetAnnotations()[common.AuthPolicyBackRefAnnotation] - if !ok { - continue - } - refs = append(refs, authPolicyRefs) - } - return refs -} - -// HTTPRouteWrapperList is a list of HTTPRouteWrapper that implements sort.interface -// impl: sort.interface -type HTTPRouteWrapperList []HTTPRouteWrapper - -func (l HTTPRouteWrapperList) Len() int { return len(l) } - -func (l HTTPRouteWrapperList) Less(i, j int) bool { - return l[i].CreationTimestamp.Before(&l[j].CreationTimestamp) -} - -func (l HTTPRouteWrapperList) Swap(i, j int) { - l[i], l[j] = l[j], l[i] -}