From c61837345166561c8b80251c0ee88bea4f2ed9b8 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Tue, 8 Oct 2024 12:38:40 -0500 Subject: [PATCH] review fixes Signed-off-by: Guy Daich --- api/v1alpha1/httproutefilter_types.go | 19 ++++------- api/v1alpha1/zz_generated.deepcopy.go | 5 --- ...ateway.envoyproxy.io_httproutefilters.yaml | 17 ++-------- site/content/en/latest/api/extension_types.md | 8 ++--- site/content/zh/latest/api/extension_types.md | 8 ++--- test/cel-validation/httproutefilter_test.go | 32 ++----------------- 6 files changed, 17 insertions(+), 72 deletions(-) diff --git a/api/v1alpha1/httproutefilter_types.go b/api/v1alpha1/httproutefilter_types.go index fcd708a5275..7f56ca07d7c 100644 --- a/api/v1alpha1/httproutefilter_types.go +++ b/api/v1alpha1/httproutefilter_types.go @@ -40,9 +40,8 @@ type HTTPURLRewriteFilter struct { // Hostname is the value to be used to replace the Host header value during // forwarding. // - // Support: Extended - // // +optional + // +notImplementedHide Hostname *HTTPHostnameModifier `json:"hostname,omitempty"` // Path defines a path rewrite. // @@ -64,7 +63,11 @@ const ( type HTTPHostnameModifierType string const ( - HeaderHTTPHostnameModifier HTTPHostnameModifierType = "SetFromHeader" + // HeaderHTTPHostnameModifier indicates that the Host header value would be replaced with the value of the header specified in setFromHeader. + // https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-host-rewrite-header + HeaderHTTPHostnameModifier HTTPHostnameModifierType = "SetFromHeader" + // BackendHTTPHostnameModifier indicates that the Host header value would be replaced by the DNS name of the backend if it exists. + // https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-auto-host-rewrite BackendHTTPHostnameModifier HTTPHostnameModifierType = "SetFromBackend" ) @@ -108,22 +111,14 @@ type HTTPPathModifier struct { // +kubebuilder:validation:XValidation:message="setFromHeader must be nil if the type is not SetFromHeader",rule="!(has(self.setFromHeader) && self.type != 'SetFromHeader')" // +kubebuilder:validation:XValidation:message="setFromHeader must be specified for SetFromHeader type",rule="!(!has(self.setFromHeader) && self.type == 'SetFromHeader')" -// +kubebuilder:validation:XValidation:message="setFromBackend must be nil if the type is not SetFromBackend",rule="!(has(self.setFromBackend) && self.type != 'SetFromBackend')" -// +kubebuilder:validation:XValidation:message="setFromBackend must be specified for SetFromBackend type",rule="!(!has(self.setFromBackend) && self.type == 'SetFromBackend')" type HTTPHostnameModifier struct { // +kubebuilder:validation:Enum=SetFromHeader;SetFromBackend // +kubebuilder:validation:Required Type HTTPHostnameModifierType `json:"type"` - // SetFromHeader indicates that the Host header value would be replaced with the value of the header specified in setFromHeader. - // https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-host-rewrite-header + // SetFromHeader is the name of the header whose value would be used to rewrite the Host header // +optional SetFromHeader *string `json:"setFromHeader,omitempty"` - - // SetFromBackend indicates that the Host header value would be replaced by the DNS name of the backend if it exists. - // https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-auto-host-rewrite - // +optional - SetFromBackend *bool `json:"setFromBackend,omitempty"` } //+kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 9af23c7970b..ed5df681ad2 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -2717,11 +2717,6 @@ func (in *HTTPHostnameModifier) DeepCopyInto(out *HTTPHostnameModifier) { *out = new(string) **out = **in } - if in.SetFromBackend != nil { - in, out := &in.SetFromBackend, &out.SetFromBackend - *out = new(bool) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HTTPHostnameModifier. diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_httproutefilters.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_httproutefilters.yaml index b3b6ef171af..7a55ec8871f 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_httproutefilters.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_httproutefilters.yaml @@ -57,18 +57,10 @@ spec: description: |- Hostname is the value to be used to replace the Host header value during forwarding. - - Support: Extended properties: - setFromBackend: - description: |- - SetFromBackend indicates that the Host header value would be replaced by the DNS name of the backend if it exists. - https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-auto-host-rewrite - type: boolean setFromHeader: - description: |- - SetFromHeader indicates that the Host header value would be replaced with the value of the header specified in setFromHeader. - https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-host-rewrite-header + description: SetFromHeader is the name of the header whose + value would be used to rewrite the Host header type: string type: description: HTTPPathModifierType defines the type of Hostname @@ -85,11 +77,6 @@ spec: rule: '!(has(self.setFromHeader) && self.type != ''SetFromHeader'')' - message: setFromHeader must be specified for SetFromHeader type rule: '!(!has(self.setFromHeader) && self.type == ''SetFromHeader'')' - - message: setFromBackend must be nil if the type is not SetFromBackend - rule: '!(has(self.setFromBackend) && self.type != ''SetFromBackend'')' - - message: setFromBackend must be specified for SetFromBackend - type - rule: '!(!has(self.setFromBackend) && self.type == ''SetFromBackend'')' path: description: Path defines a path rewrite. properties: diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 77b25ec8f8b..bc01189920d 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1950,8 +1950,7 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | | `type` | _[HTTPHostnameModifierType](#httphostnamemodifiertype)_ | true | | -| `setFromHeader` | _string_ | false | SetFromHeader indicates that the Host header value would be replaced with the value of the header specified in setFromHeader.
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-host-rewrite-header | -| `setFromBackend` | _boolean_ | false | SetFromBackend indicates that the Host header value would be replaced by the DNS name of the backend if it exists.
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-auto-host-rewrite | +| `setFromHeader` | _string_ | false | SetFromHeader is the name of the header whose value would be used to rewrite the Host header | #### HTTPHostnameModifierType @@ -1965,8 +1964,8 @@ _Appears in:_ | Value | Description | | ----- | ----------- | -| `SetFromHeader` | | -| `SetFromBackend` | | +| `SetFromHeader` | HeaderHTTPHostnameModifier indicates that the Host header value would be replaced with the value of the header specified in setFromHeader.
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-host-rewrite-header
| +| `SetFromBackend` | BackendHTTPHostnameModifier indicates that the Host header value would be replaced by the DNS name of the backend if it exists.
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-auto-host-rewrite
| #### HTTPPathModifier @@ -2084,7 +2083,6 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `hostname` | _[HTTPHostnameModifier](#httphostnamemodifier)_ | false | Hostname is the value to be used to replace the Host header value during
forwarding.

Support: Extended | | `path` | _[HTTPPathModifier](#httppathmodifier)_ | false | Path defines a path rewrite. | diff --git a/site/content/zh/latest/api/extension_types.md b/site/content/zh/latest/api/extension_types.md index 77b25ec8f8b..bc01189920d 100644 --- a/site/content/zh/latest/api/extension_types.md +++ b/site/content/zh/latest/api/extension_types.md @@ -1950,8 +1950,7 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | | `type` | _[HTTPHostnameModifierType](#httphostnamemodifiertype)_ | true | | -| `setFromHeader` | _string_ | false | SetFromHeader indicates that the Host header value would be replaced with the value of the header specified in setFromHeader.
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-host-rewrite-header | -| `setFromBackend` | _boolean_ | false | SetFromBackend indicates that the Host header value would be replaced by the DNS name of the backend if it exists.
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-auto-host-rewrite | +| `setFromHeader` | _string_ | false | SetFromHeader is the name of the header whose value would be used to rewrite the Host header | #### HTTPHostnameModifierType @@ -1965,8 +1964,8 @@ _Appears in:_ | Value | Description | | ----- | ----------- | -| `SetFromHeader` | | -| `SetFromBackend` | | +| `SetFromHeader` | HeaderHTTPHostnameModifier indicates that the Host header value would be replaced with the value of the header specified in setFromHeader.
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-host-rewrite-header
| +| `SetFromBackend` | BackendHTTPHostnameModifier indicates that the Host header value would be replaced by the DNS name of the backend if it exists.
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-auto-host-rewrite
| #### HTTPPathModifier @@ -2084,7 +2083,6 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `hostname` | _[HTTPHostnameModifier](#httphostnamemodifier)_ | false | Hostname is the value to be used to replace the Host header value during
forwarding.

Support: Extended | | `path` | _[HTTPPathModifier](#httppathmodifier)_ | false | Path defines a path rewrite. | diff --git a/test/cel-validation/httproutefilter_test.go b/test/cel-validation/httproutefilter_test.go index 10051a216a4..978d97a9374 100644 --- a/test/cel-validation/httproutefilter_test.go +++ b/test/cel-validation/httproutefilter_test.go @@ -106,8 +106,7 @@ func TestHTTPRouteFilter(t *testing.T) { httproutefilter.Spec = egv1a1.HTTPRouteFilterSpec{ URLRewrite: &egv1a1.HTTPURLRewriteFilter{ Hostname: &egv1a1.HTTPHostnameModifier{ - Type: egv1a1.BackendHTTPHostnameModifier, - SetFromBackend: ptr.To(true), + Type: egv1a1.BackendHTTPHostnameModifier, }, }, } @@ -127,19 +126,6 @@ func TestHTTPRouteFilter(t *testing.T) { }, wantErrors: []string{"spec.urlRewrite.hostname: Invalid value: \"object\": setFromHeader must be specified for SetFromHeader type"}, }, - { - desc: "invalid SetFromBackend missing settings", - mutate: func(httproutefilter *egv1a1.HTTPRouteFilter) { - httproutefilter.Spec = egv1a1.HTTPRouteFilterSpec{ - URLRewrite: &egv1a1.HTTPURLRewriteFilter{ - Hostname: &egv1a1.HTTPHostnameModifier{ - Type: egv1a1.BackendHTTPHostnameModifier, - }, - }, - } - }, - wantErrors: []string{"spec.urlRewrite.hostname: Invalid value: \"object\": setFromBackend must be specified for SetFromBackend type"}, - }, { desc: "invalid SetFromBackend type", mutate: func(httproutefilter *egv1a1.HTTPRouteFilter) { @@ -152,21 +138,7 @@ func TestHTTPRouteFilter(t *testing.T) { }, } }, - wantErrors: []string{"spec.urlRewrite.hostname: Invalid value: \"object\": setFromHeader must be nil if the type is not SetFromHeader, spec.urlRewrite.hostname: Invalid value: \"object\": setFromBackend must be specified for SetFromBackend type"}, - }, - { - desc: "invalid SetFromHeader type", - mutate: func(httproutefilter *egv1a1.HTTPRouteFilter) { - httproutefilter.Spec = egv1a1.HTTPRouteFilterSpec{ - URLRewrite: &egv1a1.HTTPURLRewriteFilter{ - Hostname: &egv1a1.HTTPHostnameModifier{ - Type: egv1a1.HeaderHTTPHostnameModifier, - SetFromBackend: ptr.To(true), - }, - }, - } - }, - wantErrors: []string{"spec.urlRewrite.hostname: Invalid value: \"object\": setFromHeader must be specified for SetFromHeader type, spec.urlRewrite.hostname: Invalid value: \"object\": setFromBackend must be nil if the type is not SetFromBackend]"}, + wantErrors: []string{"spec.urlRewrite.hostname: Invalid value: \"object\": setFromHeader must be nil if the type is not SetFromHeader"}, }, }