review fixes

Signed-off-by: Guy Daich <guy.daich@sap.com>
guydc · Oct 8, 2024 · c618373 · c618373
1 parent 1200c13
commit c618373
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 72 deletions.
diff --git 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
diff --git 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
@@ -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.<br />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.<br />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.<br />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<br /> | 
+| `SetFromBackend` | BackendHTTPHostnameModifier indicates that the Host header value would be replaced by the DNS name of the backend if it exists.<br />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<br /> | 
 
 
 #### 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<br />forwarding.<br /><br />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
@@ -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.<br />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.<br />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.<br />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<br /> | 
+| `SetFromBackend` | BackendHTTPHostnameModifier indicates that the Host header value would be replaced by the DNS name of the backend if it exists.<br />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<br /> | 
 
 
 #### 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<br />forwarding.<br /><br />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
@@ -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"},
 		},
 	}