fix panic in provider when the direct response body is nil (envoyprox…

…y#4647) body is an optional field, it may be nil Signed-off-by: Arko Dasgupta <[email protected]>
arkodg · Nov 7, 2024 · ad51832 · ad51832
1 parent 54289e3
commit ad51832
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 5 deletions.
diff --git a/internal/provider/kubernetes/filters.go b/internal/provider/kubernetes/filters.go
@@ -66,6 +66,7 @@ func (r *gatewayAPIReconciler) processRouteFilterConfigMapRef(
 	resourceMap *resourceMappings, resourceTree *resource.Resources,
 ) {
 	if filter.Spec.DirectResponse != nil &&
+		filter.Spec.DirectResponse.Body != nil &&
 		filter.Spec.DirectResponse.Body.ValueRef != nil &&
 		string(filter.Spec.DirectResponse.Body.ValueRef.Kind) == resource.KindConfigMap {
 		configMap := new(corev1.ConfigMap)

diff --git a/test/e2e/testdata/direct-response.yaml b/test/e2e/testdata/direct-response.yaml
@@ -27,6 +27,17 @@ spec:
         group: gateway.envoyproxy.io
         kind: HTTPRouteFilter
         name: direct-response-value-ref
+  - matches:
+    - path:
+        type: PathPrefix
+        value: /401
+    filters:
+    - type: ExtensionRef
+      extensionRef:
+        group: gateway.envoyproxy.io
+        kind: HTTPRouteFilter
+        name: direct-response-status
+
 ---
 apiVersion: v1
 kind: ConfigMap
@@ -62,3 +73,12 @@ spec:
         group: ""
         kind: ConfigMap
         name: value-ref-response
+---
+apiVersion: gateway.envoyproxy.io/v1alpha1
+kind: HTTPRouteFilter
+metadata:
+  name: direct-response-status
+  namespace: gateway-conformance-infra
+spec:
+  directResponse:
+    statusCode: 401
diff --git a/test/e2e/tests/direct-response.go b/test/e2e/tests/direct-response.go
@@ -31,8 +31,9 @@ var DirectResponseTest = suite.ConformanceTest{
 			gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN)
 
 			kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN, gwNN)
-			verifyCustomResponse(t, suite.TimeoutConfig, gwAddr, "/inline", "text/plain", "Oops! Your request is not found.")
-			verifyCustomResponse(t, suite.TimeoutConfig, gwAddr, "/value-ref", "application/json", `{"error": "Internal Server Error"}`)
+			verifyCustomResponse(t, suite.TimeoutConfig, gwAddr, "/inline", "text/plain", "Oops! Your request is not found.", 200)
+			verifyCustomResponse(t, suite.TimeoutConfig, gwAddr, "/value-ref", "application/json", `{"error": "Internal Server Error"}`, 200)
+			verifyCustomResponse(t, suite.TimeoutConfig, gwAddr, "/401", "", ``, 401)
 		})
 	},
 }
diff --git a/test/e2e/tests/response-override.go b/test/e2e/tests/response-override.go
@@ -49,13 +49,15 @@ var ResponseOverrideTest = suite.ConformanceTest{
 				Name:      gwapiv1.ObjectName(gwNN.Name),
 			}
 			BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "response-override", Namespace: ns}, suite.ControllerName, ancestorRef)
-			verifyCustomResponse(t, suite.TimeoutConfig, gwAddr, "/status/404", "text/plain", "Oops! Your request is not found.")
-			verifyCustomResponse(t, suite.TimeoutConfig, gwAddr, "/status/500", "application/json", `{"error": "Internal Server Error"}`)
+			verifyCustomResponse(t, suite.TimeoutConfig, gwAddr, "/status/404", "text/plain", "Oops! Your request is not found.", 404)
+			verifyCustomResponse(t, suite.TimeoutConfig, gwAddr, "/status/500", "application/json", `{"error": "Internal Server Error"}`, 500)
 		})
 	},
 }
 
-func verifyCustomResponse(t *testing.T, timeoutConfig config.TimeoutConfig, gwAddr, path, expectedContentType, expectedBody string) {
+func verifyCustomResponse(t *testing.T, timeoutConfig config.TimeoutConfig, gwAddr,
+	path, expectedContentType, expectedBody string, expectedStatusCode int,
+) {
 	reqURL := url.URL{
 		Scheme: "http",
 		Host:   httputils.CalculateHost(t, gwAddr, "http"),
@@ -88,6 +90,11 @@ func verifyCustomResponse(t *testing.T, timeoutConfig config.TimeoutConfig, gwAd
 			return false
 		}
 
+		if expectedStatusCode != rsp.StatusCode {
+			tlog.Logf(t, "expected status code to be %d but got %d", expectedStatusCode, rsp.StatusCode)
+			return false
+		}
+
 		return true
 	})