Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

impl: custom error response #4415

Merged
merged 24 commits into from
Oct 22, 2024
Merged

impl: custom error response #4415

Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c1a7d65
gateway api translation
zhaohuabing Oct 9, 2024
5e68042
add tests for gateway api translation
zhaohuabing Oct 10, 2024
92b050c
xds translation
zhaohuabing Oct 10, 2024
ee8fb4c
test for xds translation
zhaohuabing Oct 11, 2024
96398c3
fix lint
zhaohuabing Oct 11, 2024
0dea08e
fix lint
zhaohuabing Oct 11, 2024
d7b0df1
e2e test
zhaohuabing Oct 14, 2024
d143bd5
add cel validation
zhaohuabing Oct 15, 2024
bb3d59d
fix test
zhaohuabing Oct 15, 2024
9700516
fix test
zhaohuabing Oct 15, 2024
9ef703b
fix test
zhaohuabing Oct 15, 2024
0631fa4
fix lint
zhaohuabing Oct 15, 2024
1f309e0
address comment
zhaohuabing Oct 16, 2024
e820fb2
Merge remote-tracking branch 'origin/main' into custom-error-response
zhaohuabing Oct 16, 2024
e41419b
Merge branch 'main' into custom-error-response
zhaohuabing Oct 16, 2024
e3d528b
address comment
zhaohuabing Oct 17, 2024
dbed3c7
fix lint
zhaohuabing Oct 17, 2024
489503b
Merge branch 'main' into custom-error-response
zhaohuabing Oct 17, 2024
19e123f
Merge branch 'main' into custom-error-response
zhaohuabing Oct 18, 2024
376b710
Merge branch 'main' into custom-error-response
zhaohuabing Oct 18, 2024
72650ea
Merge branch 'main' into custom-error-response
zhaohuabing Oct 18, 2024
9334f53
make Body required
zhaohuabing Oct 21, 2024
26b2c76
Merge branch 'main' into custom-error-response
zhaohuabing Oct 21, 2024
6f7d2c1
fix e2e
zhaohuabing Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion api/v1alpha1/backendtrafficpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ type BackendTrafficPolicySpec struct {
// If multiple configurations are specified, the first one to match wins.
//
// +optional
// +notImplementedHide
ResponseOverride []*ResponseOverride `json:"responseOverride,omitempty"`
zhaohuabing marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
7 changes: 6 additions & 1 deletion api/v1alpha1/envoyproxy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ type EnvoyProxySpec struct {
//
// - envoy.filters.http.ratelimit
//
// - envoy.filters.http.custom_response
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to override the 429 response if we place custom response filter behind ratelimit filterm should this filter be the first one of http filters?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Should we allow users to override the ratelimit response if they added a matching conditing for 429 in the ResponseOrverride? @envoyproxy/gateway-maintainers

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not just ratelimit, does authz/authn has same problem?

Copy link
Member Author

@zhaohuabing zhaohuabing Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the custom response filter has been added to the last because users may want to override the direct responses generated by other filters, if they explicitly adding these status codes in the matching conditions. For example, to set the response body of a denide request to a message format algined with the style of the reponses of the applications.

If we put it at the first, the users won't be able to override them even if they intend to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, it's better to put it at the first but no strong option, @arkodg please chime in as API designer.

cc @envoyproxy/gateway-maintainers

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as a user, I would want the custom response to kick in after all filters have finished processing, to unify the way I deal with status code and error pages, so +1 to adding it to the end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can change the order in the future, maybe we can open a ticket to continue our discussion.

Copy link
Member Author

@zhaohuabing zhaohuabing Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can change the order in the future, maybe we can open a ticket to continue our discussion.

Let's keep the custom response filter at the last since there're use cases to override the responses of other filters. Users can change the default order using EnvoyProxy if they want to reorder the filters.

//
// - envoy.filters.http.router
//
// Note: "envoy.filters.http.router" cannot be reordered, it's always the last filter in the chain.
Expand Down Expand Up @@ -174,7 +176,7 @@ type FilterPosition struct {
}

// EnvoyFilter defines the type of Envoy HTTP filter.
// +kubebuilder:validation:Enum=envoy.filters.http.health_check;envoy.filters.http.fault;envoy.filters.http.cors;envoy.filters.http.ext_authz;envoy.filters.http.basic_auth;envoy.filters.http.oauth2;envoy.filters.http.jwt_authn;envoy.filters.http.stateful_session;envoy.filters.http.ext_proc;envoy.filters.http.wasm;envoy.filters.http.rbac;envoy.filters.http.local_ratelimit;envoy.filters.http.ratelimit
// +kubebuilder:validation:Enum=envoy.filters.http.health_check;envoy.filters.http.fault;envoy.filters.http.cors;envoy.filters.http.ext_authz;envoy.filters.http.basic_auth;envoy.filters.http.oauth2;envoy.filters.http.jwt_authn;envoy.filters.http.stateful_session;envoy.filters.http.ext_proc;envoy.filters.http.wasm;envoy.filters.http.rbac;envoy.filters.http.local_ratelimit;envoy.filters.http.ratelimit;envoy.filters.http.custom_response
type EnvoyFilter string

const (
Expand Down Expand Up @@ -217,6 +219,9 @@ const (
// EnvoyFilterRateLimit defines the Envoy HTTP rate limit filter.
EnvoyFilterRateLimit EnvoyFilter = "envoy.filters.http.ratelimit"

// EnvoyFilterCustomResponse defines the Envoy HTTP custom response filter.
EnvoyFilterCustomResponse EnvoyFilter = "envoy.filters.http.custom_response"

// EnvoyFilterRouter defines the Envoy HTTP router filter.
EnvoyFilterRouter EnvoyFilter = "envoy.filters.http.router"
)
Expand Down
47 changes: 39 additions & 8 deletions api/v1alpha1/shared_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,33 +627,48 @@ type ResponseOverride struct {
// CustomResponseMatch defines the configuration for matching a user response to return a custom one.
type CustomResponseMatch struct {
// Status code to match on. The match evaluates to true if any of the matches are successful.
StatusCode []StatusCodeMatch `json:"statusCode"`
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=50
StatusCodes []StatusCodeMatch `json:"statusCodes"`
}

// StatusCodeValueType defines the types of values for the status code match supported by Envoy Gateway.
// +kubebuilder:validation:Enum=Value;Range
type StatusCodeValueType string

const (
// StatusCodeValueTypeValue defines the "Value" status code match type.
StatusCodeValueTypeValue StatusCodeValueType = "Value"

// StatusCodeValueTypeRange defines the "Range" status code match type.
StatusCodeValueTypeRange StatusCodeValueType = "Range"
)

// StatusCodeMatch defines the configuration for matching a status code.
// +kubebuilder:validation:XValidation:message="value must be set for type Value",rule="(!has(self.type) || self.type == 'Value')? has(self.value) : true"
// +kubebuilder:validation:XValidation:message="range must be set for type Range",rule="(has(self.type) && self.type == 'Range')? has(self.range) : true"
type StatusCodeMatch struct {
// Type is the type of value.
// Valid values are Value and Range, default is Value.
//
// +kubebuilder:default=Value
// +kubebuilder:validation:Enum=Value;Range
// +unionDiscriminator
Type *StatusCodeValueType `json:"type"`

// Value contains the value of the status code.
//
// +optional
Value *string `json:"value,omitempty"`
// ValueRef contains the contents of the body
// specified as a local object reference.
// Only a reference to ConfigMap is supported.
Value *int `json:"value,omitempty"`
zhaohuabing marked this conversation as resolved.
Show resolved Hide resolved

// Range contains the range of status codes.
//
// +optional
Range *StatusCodeRange `json:"range,omitempty"`
}

// StatusCodeRange defines the configuration for define a range of status codes.
// +kubebuilder:validation:XValidation: message="end must be greater than start",rule="self.end > self.start"
type StatusCodeRange struct {
// Start of the range, including the start value.
Start int `json:"start"`
Expand All @@ -669,30 +684,46 @@ type CustomResponse struct {
ContentType *string `json:"contentType,omitempty"`

// Body of the Custom Response
//
// +optional
Body *CustomResponseBody `json:"body,omitempty"`
Body CustomResponseBody `json:"body"`
}

// ResponseValueType defines the types of values for the response body supported by Envoy Gateway.
// +kubebuilder:validation:Enum=Inline;ValueRef
type ResponseValueType string

const (
// ResponseValueTypeInline defines the "Inline" response body type.
ResponseValueTypeInline ResponseValueType = "Inline"

// ResponseValueTypeValueRef defines the "ValueRef" response body type.
ResponseValueTypeValueRef ResponseValueType = "ValueRef"
)

// CustomResponseBody
// +kubebuilder:validation:XValidation:message="inline must be set for type Inline",rule="(!has(self.type) || self.type == 'Inline')? has(self.inline) : true"
// +kubebuilder:validation:XValidation:message="valueRef must be set for type ValueRef",rule="(has(self.type) && self.type == 'ValueRef')? has(self.valueRef) : true"
// +kubebuilder:validation:XValidation:message="only ConfigMap is supported for ValueRef",rule="has(self.valueRef) ? self.valueRef.kind == 'ConfigMap' : true"
type CustomResponseBody struct {
// Type is the type of method to use to read the body value.
// Valid values are Inline and ValueRef, default is Inline.
//
// +kubebuilder:default=Inline
// +kubebuilder:validation:Enum=Inline;ValueRef
// +unionDiscriminator
Type *ResponseValueType `json:"type"`

// Inline contains the value as an inline string.
//
// +optional
Inline *string `json:"inline,omitempty"`

// ValueRef contains the contents of the body
// specified as a local object reference.
// Only a reference to ConfigMap is supported.
//
// The value of key `response.body` in the ConfigMap will be used as the response body.
// If the key is not found, the first value in the ConfigMap will be used.
//
// +optional
ValueRef *gwapiv1.LocalObjectReference `json:"valueRef,omitempty"`
}
12 changes: 4 additions & 8 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

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

70 changes: 54 additions & 16 deletions charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -940,16 +940,15 @@ spec:
match:
description: Match configuration.
properties:
statusCode:
statusCodes:
description: Status code to match on. The match evaluates
to true if any of the matches are successful.
items:
description: StatusCodeMatch defines the configuration
for matching a status code.
properties:
range:
description: |-
ValueRef contains the contents of the body
specified as a local object reference.
Only a reference to ConfigMap is supported.
description: Range contains the range of status codes.
properties:
end:
description: End of the range, including the end
Expand All @@ -963,23 +962,41 @@ spec:
- end
- start
type: object
x-kubernetes-validations:
- message: end must be greater than start
rule: self.end > self.start
type:
allOf:
- enum:
- Value
- Range
- enum:
- Value
- Range
default: Value
description: Type is the type of value.
enum:
- Value
- Range
description: |-
Type is the type of value.
Valid values are Value and Range, default is Value.
type: string
value:
description: Value contains the value of the status
code.
type: string
type: integer
required:
- type
type: object
x-kubernetes-validations:
- message: value must be set for type Value
rule: '(!has(self.type) || self.type == ''Value'')?
has(self.value) : true'
- message: range must be set for type Range
rule: '(has(self.type) && self.type == ''Range'')? has(self.range)
: true'
maxItems: 50
minItems: 1
type: array
required:
- statusCode
- statusCodes
type: object
response:
description: Response configuration.
Expand All @@ -992,17 +1009,26 @@ spec:
string.
type: string
type:
description: Type is the type of method to use to read
the body value.
enum:
- Inline
- ValueRef
allOf:
- enum:
- Inline
- ValueRef
- enum:
- Inline
- ValueRef
default: Inline
description: |-
Type is the type of method to use to read the body value.
Valid values are Inline and ValueRef, default is Inline.
type: string
valueRef:
description: |-
ValueRef contains the contents of the body
specified as a local object reference.
Only a reference to ConfigMap is supported.

The value of key `response.body` in the ConfigMap will be used as the response body.
If the key is not found, the first value in the ConfigMap will be used.
properties:
group:
description: |-
Expand Down Expand Up @@ -1031,10 +1057,22 @@ spec:
required:
- type
type: object
x-kubernetes-validations:
- message: inline must be set for type Inline
rule: '(!has(self.type) || self.type == ''Inline'')? has(self.inline)
: true'
- message: valueRef must be set for type ValueRef
rule: '(has(self.type) && self.type == ''ValueRef'')?
has(self.valueRef) : true'
- message: only ConfigMap is supported for ValueRef
rule: 'has(self.valueRef) ? self.valueRef.kind == ''ConfigMap''
: true'
contentType:
description: Content Type of the response. This will be
set in the Content-Type header.
type: string
required:
- body
type: object
required:
- match
Expand Down
5 changes: 5 additions & 0 deletions charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,8 @@ spec:

- envoy.filters.http.ratelimit

- envoy.filters.http.custom_response

- envoy.filters.http.router

Note: "envoy.filters.http.router" cannot be reordered, it's always the last filter in the chain.
Expand All @@ -330,6 +332,7 @@ spec:
- envoy.filters.http.rbac
- envoy.filters.http.local_ratelimit
- envoy.filters.http.ratelimit
- envoy.filters.http.custom_response
type: string
before:
description: |-
Expand All @@ -349,6 +352,7 @@ spec:
- envoy.filters.http.rbac
- envoy.filters.http.local_ratelimit
- envoy.filters.http.ratelimit
- envoy.filters.http.custom_response
type: string
name:
description: Name of the filter.
Expand All @@ -366,6 +370,7 @@ spec:
- envoy.filters.http.rbac
- envoy.filters.http.local_ratelimit
- envoy.filters.http.ratelimit
- envoy.filters.http.custom_response
type: string
required:
- name
Expand Down
29 changes: 24 additions & 5 deletions charts/gateway-helm/crds/generated/gateway.envoyproxy.io_httproutefilters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,26 @@ spec:
description: Inline contains the value as an inline string.
type: string
type:
description: Type is the type of method to use to read the
body value.
enum:
- Inline
- ValueRef
allOf:
- enum:
- Inline
- ValueRef
- enum:
- Inline
- ValueRef
default: Inline
description: |-
Type is the type of method to use to read the body value.
Valid values are Inline and ValueRef, default is Inline.
type: string
valueRef:
description: |-
ValueRef contains the contents of the body
specified as a local object reference.
Only a reference to ConfigMap is supported.

The value of key `response.body` in the ConfigMap will be used as the response body.
If the key is not found, the first value in the ConfigMap will be used.
properties:
group:
description: |-
Expand Down Expand Up @@ -99,6 +108,16 @@ spec:
required:
- type
type: object
x-kubernetes-validations:
- message: inline must be set for type Inline
rule: '(!has(self.type) || self.type == ''Inline'')? has(self.inline)
: true'
- message: valueRef must be set for type ValueRef
rule: '(has(self.type) && self.type == ''ValueRef'')? has(self.valueRef)
: true'
- message: only ConfigMap is supported for ValueRef
rule: 'has(self.valueRef) ? self.valueRef.kind == ''ConfigMap''
: true'
contentType:
description: Content Type of the response. This will be set in
the Content-Type header.
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ require (
golang.org/x/time v0.5.0 // indirect
golang.org/x/tools v0.24.0 // indirect
gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20241007155032-5fefd90f89a9 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20241007155032-5fefd90f89a9
google.golang.org/genproto/googleapis/rpc v0.0.0-20241007155032-5fefd90f89a9 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
Expand Down
Loading
Loading