Repository: envoyproxy/gateway
Pull request #4212 (2024-09-11 02:33:28)

[release/v1.1] fix: reject invalid backends in route validation (#4209)
fix: rejected invalid backends in route validation


(cherry picked from commit 646da88131e078517c1f97bb8b82ee09583b6ff7)

**What type of PR is this?**
<!--
Your PR title should be descriptive, and generally start with type that contains a subsystem name with `()` if necessary 
and summary followed by a colon. format `chore/docs/api/feat/fix/refactor/style/test: summary`.
Examples:
* "docs: fix grammar error"
* "feat(translator): add new feature"
* "fix: fix xx bug"
* "chore: change ci & build tools etc"
* "api: add xxx fields in ClientTrafficPolicy"
-->

<!--
NOTE: If your PR contains any API changes (changes under `/api`), we recommend you to separate these API changes into
a new PR, and we will review the API part first. It will save you lots of implementation time if the API get accepted.
-->

**What this PR does / why we need it**:

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/envoyproxy/gateway/pull/4212?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 67.35%. Comparing base [(`9a3f8f7`)](https://app.codecov.io/gh/envoyproxy/gateway/commit/9a3f8f79c42fd1fabb8c10d8e037752aa1526763?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy) to head [(`33cf8fc`)](https://app.codecov.io/gh/envoyproxy/gateway/commit/33cf8fc71335c7a630cc332cf1069bfc83ade10d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy).

<details><summary>Additional details and impacted files</summary>


```diff
@@               Coverage Diff                @@
##           release/v1.1    #4212      +/-   ##
================================================
+ Coverage         67.30%   67.35%   +0.04%     
================================================
  Files               183      183              
  Lines             22476    22487      +11     
================================================
+ Hits              15128    15145      +17     
+ Misses             6253     6249       -4     
+ Partials           1095     1093       -2     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/envoyproxy/gateway/pull/4212?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy).


Patch:
diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go
index 190df4f5d..ee47b7601 100644
--- a/internal/gatewayapi/backendtrafficpolicy.go
+++ b/internal/gatewayapi/backendtrafficpolicy.go
@@ -811,14 +811,12 @@ func (t *Translator) buildLoadBalancer(policy *egv1a1.BackendTrafficPolicy) (*ir
 			ConsistentHash: consistentHash,
 		}
 	case egv1a1.LeastRequestLoadBalancerType:
-		lb = &ir.LoadBalancer{}
-		if policy.Spec.LoadBalancer.SlowStart != nil {
-			if policy.Spec.LoadBalancer.SlowStart.Window != nil {
-				lb.LeastRequest = &ir.LeastRequest{
-					SlowStart: &ir.SlowStart{
-						Window: policy.Spec.LoadBalancer.SlowStart.Window,
-					},
-				}
+		lb = &ir.LoadBalancer{
+			LeastRequest: &ir.LeastRequest{},
+		}
+		if policy.Spec.LoadBalancer.SlowStart != nil && policy.Spec.LoadBalancer.SlowStart.Window != nil {
+			lb.LeastRequest.SlowStart = &ir.SlowStart{
+				Window: policy.Spec.LoadBalancer.SlowStart.Window,
 			}
 		}
 	case egv1a1.RandomLoadBalancerType:
@@ -827,17 +825,11 @@ func (t *Translator) buildLoadBalancer(policy *egv1a1.BackendTrafficPolicy) (*ir
 		}
 	case egv1a1.RoundRobinLoadBalancerType:
 		lb = &ir.LoadBalancer{
-			RoundRobin: &ir.RoundRobin{
-				SlowStart: &ir.SlowStart{},
-			},
-		}
-		if policy.Spec.LoadBalancer.SlowStart != nil {
-			if policy.Spec.LoadBalancer.SlowStart.Window != nil {
-				lb.RoundRobin = &ir.RoundRobin{
-					SlowStart: &ir.SlowStart{
-						Window: policy.Spec.LoadBalancer.SlowStart.Window,
-					},
-				}
+			RoundRobin: &ir.RoundRobin{},
+		}
+		if policy.Spec.LoadBalancer.SlowStart != nil && policy.Spec.LoadBalancer.SlowStart.Window != nil {
+			lb.RoundRobin.SlowStart = &ir.SlowStart{
+				Window: policy.Spec.LoadBalancer.SlowStart.Window,
 			}
 		}
 	}
diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go
index 5c442812d..d92dee3bc 100644
--- a/internal/gatewayapi/validate.go
+++ b/internal/gatewayapi/validate.go
@@ -314,6 +314,19 @@ func (t *Translator) validateBackendRefBackend(backendRef *gwapiv1a2.BackendRef,
 		return false
 	}
 
+	if err := validateBackend(backend); err != nil {
+		status.SetRouteStatusCondition(routeStatus,
+			parentRef.routeParentStatusIdx,
+			route.GetGeneration(),
+			gwapiv1.RouteConditionResolvedRefs,
+			metav1.ConditionFalse,
+			"UnsupportedRefAddressFound",
+			fmt.Sprintf("Invalid Backend reference to Backend %s/%s found", backendNamespace,
+				string(backendRef.Name)),
+		)
+		return false
+	}
+
 	for _, bep := range backend.Spec.Endpoints {
 		if bep.Unix != nil {
 			status.SetRouteStatusCondition(routeStatus,


Test patch:
diff --git a/internal/gatewayapi/testdata/httproute-attaching-to-listener-with-backend-backendref.out.yaml b/internal/gatewayapi/testdata/httproute-attaching-to-listener-with-backend-backendref.out.yaml
index b25d2af5d..25ee9e8ba 100644
--- a/internal/gatewayapi/testdata/httproute-attaching-to-listener-with-backend-backendref.out.yaml
+++ b/internal/gatewayapi/testdata/httproute-attaching-to-listener-with-backend-backendref.out.yaml
@@ -272,9 +272,10 @@ httpRoutes:
         status: "True"
         type: Accepted
       - lastTransitionTime: null
-        message: Resolved all the Object references for the Route
-        reason: ResolvedRefs
-        status: "True"
+        message: Invalid Backend reference to Backend default/backend-ip-localhost
+          found
+        reason: UnsupportedRefAddressFound
+        status: "False"
         type: ResolvedRefs
       controllerName: gateway.envoyproxy.io/gatewayclass-controller
       parentRef:
@@ -309,9 +310,10 @@ httpRoutes:
         status: "True"
         type: Accepted
       - lastTransitionTime: null
-        message: Resolved all the Object references for the Route
-        reason: ResolvedRefs
-        status: "True"
+        message: Invalid Backend reference to Backend default/backend-fqdn-localhost
+          found
+        reason: UnsupportedRefAddressFound
+        status: "False"
         type: ResolvedRefs
       controllerName: gateway.envoyproxy.io/gatewayclass-controller
       parentRef:
@@ -413,11 +415,9 @@ xdsIR:
       - destination:
           name: httproute/default/httproute-4/rule/0
           settings:
-          - addressType: IP
-            endpoints:
-            - host: 127.0.0.1
-              port: 3001
-            weight: 1
+          - weight: 1
+        directResponse:
+          statusCode: 500
         hostname: '*'
         isHTTP2: false
         metadata:
@@ -432,11 +432,9 @@ xdsIR:
       - destination:
           name: httproute/default/httproute-5/rule/0
           settings:
-          - addressType: FQDN
-            endpoints:
-            - host: localhost
-              port: 3001
-            weight: 1
+          - weight: 1
+        directResponse:
+          statusCode: 500
         hostname: '*'
         isHTTP2: false
         metadata: