Repository: envoyproxy/gateway
Pull request #4154 (2024-09-04 12:42:23)

refactor: return 500 when EnvoyExtensionTrafficPolicy translation fails
#### What type of PR is this?

Refactor(translator)

#### What this PR does / why we need it:
Return 500 when EnvoyExtensionTrafficPolicy translation fails

#### Which issue(s) this PR fixes:
Fixes https://github.com/envoyproxy/gateway/issues/4062

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/envoyproxy/gateway/pull/4154?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.93%. Comparing base [(`301eedd`)](https://app.codecov.io/gh/envoyproxy/gateway/commit/301eeddfa4c2f7e4e612874a9c5b13a976156c3e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy) to head [(`ca982e0`)](https://app.codecov.io/gh/envoyproxy/gateway/commit/ca982e010e5bd8c95673ec1cc89e1268e44287d8?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             @@
##             main    #4154      +/-   ##
==========================================
- Coverage   67.94%   67.93%   -0.02%     
==========================================
  Files         187      187              
  Lines       23019    23023       +4     
==========================================
- Hits        15641    15640       -1     
- Misses       6264     6268       +4     
- Partials     1114     1115       +1     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/envoyproxy/gateway/pull/4154?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).

alexwo: /retest
alexwo: /retest
alexwo: /retest
guydc: /retest

Patch:
diff --git a/internal/gatewayapi/envoyextensionpolicy.go b/internal/gatewayapi/envoyextensionpolicy.go
index 5cbe148db..f8c06ed0c 100644
--- a/internal/gatewayapi/envoyextensionpolicy.go
+++ b/internal/gatewayapi/envoyextensionpolicy.go
@@ -300,11 +300,6 @@ func (t *Translator) translateEnvoyExtensionPolicyForRoute(
 		errs = errors.Join(errs, err)
 	}
 
-	// Early return if got any errors
-	if errs != nil {
-		return errs
-	}
-
 	// Apply IR to all relevant routes
 	prefix := irRoutePrefix(route)
 	parentRefs := GetParentReferences(route)
@@ -326,6 +321,13 @@ func (t *Translator) translateEnvoyExtensionPolicyForRoute(
 			if irListener != nil {
 				for _, r := range irListener.Routes {
 					if strings.HasPrefix(r.Name, prefix) {
+						// return 500 and do not configure EnvoyExtensions in this case
+						if errs != nil {
+							r.DirectResponse = &ir.DirectResponse{
+								StatusCode: 500,
+							}
+							continue
+						}
 						r.EnvoyExtensions = &ir.EnvoyExtensionFeatures{
 							ExtProcs: extProcs,
 							Wasms:    wasms,
@@ -361,11 +363,6 @@ func (t *Translator) translateEnvoyExtensionPolicyForGateway(
 		errs = errors.Join(errs, err)
 	}
 
-	// Early return if got any errors
-	if errs != nil {
-		return errs
-	}
-
 	irKey := t.getIRKey(gateway.Gateway)
 	// Should exist since we've validated this
 	x := xdsIR[irKey]
@@ -386,6 +383,14 @@ func (t *Translator) translateEnvoyExtensionPolicyForGateway(
 				continue
 			}
 
+			// return 500 and do not configure EnvoyExtensions in this case
+			if errs != nil {
+				r.DirectResponse = &ir.DirectResponse{
+					StatusCode: 500,
+				}
+				continue
+			}
+
 			r.EnvoyExtensions = &ir.EnvoyExtensionFeatures{
 				ExtProcs: extProcs,
 				Wasms:    wasms,
@@ -393,7 +398,7 @@ func (t *Translator) translateEnvoyExtensionPolicyForGateway(
 		}
 	}
 
-	return nil
+	return errs
 }
 
 func (t *Translator) buildExtProcs(policy *egv1a1.EnvoyExtensionPolicy, resources *Resources, envoyProxy *egv1a1.EnvoyProxy) ([]ir.ExtProc, error) {


Test patch:
diff --git a/internal/gatewayapi/testdata/backend-invalid-feature-disabled.out.yaml b/internal/gatewayapi/testdata/backend-invalid-feature-disabled.out.yaml
index 643df0503..c1425425b 100644
--- a/internal/gatewayapi/testdata/backend-invalid-feature-disabled.out.yaml
+++ b/internal/gatewayapi/testdata/backend-invalid-feature-disabled.out.yaml
@@ -174,7 +174,6 @@ xdsIR:
           - weight: 1
         directResponse:
           statusCode: 500
-        envoyExtensions: {}
         hostname: '*'
         isHTTP2: false
         metadata:
diff --git a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-matching-port.out.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-matching-port.out.yaml
index 6c7cfa256..beac28da5 100644
--- a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-matching-port.out.yaml
+++ b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-matching-port.out.yaml
@@ -154,6 +154,8 @@ xdsIR:
               port: 8080
             protocol: HTTP
             weight: 1
+        directResponse:
+          statusCode: 500
         hostname: www.foo.com
         isHTTP2: false
         metadata:
diff --git a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-port.out.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-port.out.yaml
index c5c71bb50..efd62e1e0 100644
--- a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-port.out.yaml
+++ b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-port.out.yaml
@@ -154,6 +154,8 @@ xdsIR:
               port: 8080
             protocol: HTTP
             weight: 1
+        directResponse:
+          statusCode: 500
         hostname: www.foo.com
         isHTTP2: false
         metadata:
diff --git a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-reference-grant.out.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-reference-grant.out.yaml
index a8a19dffa..ba93c2dec 100644
--- a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-reference-grant.out.yaml
+++ b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-reference-grant.out.yaml
@@ -156,6 +156,8 @@ xdsIR:
               port: 8080
             protocol: HTTP
             weight: 1
+        directResponse:
+          statusCode: 500
         hostname: www.foo.com
         isHTTP2: false
         metadata:
diff --git a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-service.out.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-service.out.yaml
index 4405c28c0..066917dd1 100644
--- a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-service.out.yaml
+++ b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-invalid-no-service.out.yaml
@@ -155,6 +155,8 @@ xdsIR:
               port: 8080
             protocol: HTTP
             weight: 1
+        directResponse:
+          statusCode: 500
         hostname: www.foo.com
         isHTTP2: false
         metadata: