update based on reviews

Author: salonichf5

tests/suite/manifests/snippets-filter/cafe-routes.yaml

@@ -95,4 +95,4 @@ spec:
     protocol: TCP
     name: http
   selector:
-    app: soda
+    app: soda

tests/suite/manifests/snippets-filter/cafe.yaml

@@ -9,3 +9,4 @@ spec:
     port: 80
     protocol: HTTP
     hostname: "*.example.com"
+

tests/suite/manifests/snippets-filter/gateway.yaml

@@ -37,3 +37,4 @@ spec:
         resources:
           requests:
             cpu: 10m
+

tests/suite/manifests/snippets-filter/grpc-backend.yaml

@@ -7,7 +7,7 @@ spec:
     - context: http
       value: aio on;
     - context: http.server
-      value: wrker_priority 0;  # wrong context for the directive, should be main.
+      value: worker_priority 0;  # wrong context for the directive, should be main.
     - context: http.server.location
       value: keepalive_time 10s;
 ---
@@ -34,4 +34,4 @@ spec:
           name: invalid-context
     backendRefs:
     - name: soda
-      port: 80
+      port: 80

tests/suite/manifests/snippets-filter/grpc-route.yaml

@@ -36,4 +36,4 @@ spec:
           name: duplicate-directive
     backendRefs:
     - name: tea
-      port: 80
+      port: 80

tests/suite/manifests/snippets-filter/invalid-context-sf.yaml

@@ -26,4 +26,52 @@ spec:
     - context: http.server
       value: 	server_tokens on;
     - context: http.server.location
-      value: 	tcp_nodelay on;
+      value: 	tcp_nodelay on;
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: coffee
+spec:
+  parentRefs:
+  - name: gateway
+    sectionName: http
+  hostnames:
+  - "cafe.example.com"
+  rules:
+  - matches:
+    - path:
+        type: PathPrefix
+        value: /coffee
+    filters:
+      - type: ExtensionRef
+        extensionRef:
+          group: gateway.nginx.org
+          kind: SnippetsFilter
+          name: all-contexts
+    backendRefs:
+    - name: coffee
+      port: 80
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: GRPCRoute
+metadata:
+  name: grpc-route
+spec:
+  parentRefs:
+  - name: gateway
+    sectionName: http
+  rules:
+  - matches:
+    - method:
+        service: helloworld.Greeter
+        method: SayHello
+    filters:
+      - type: ExtensionRef
+        extensionRef:
+          group: gateway.nginx.org
+          kind: SnippetsFilter
+          name: grpc-all-contexts
+    backendRefs:
+    - name: grpc-backend
+      port: 8080

tests/suite/manifests/snippets-filter/invalid-duplicate-sf.yaml

@@ -49,21 +49,12 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter
 	})
 
 	When("SnippetsFilters are applied to the resources", func() {
-		var (
-			snippetsFilter = []string{
-				"snippets-filter/valid-sf.yaml",
-			}
-			baseURL string
-		)
+		snippetsFilter := []string{
+			"snippets-filter/valid-sf.yaml",
+		}
 
 		BeforeAll(func() {
 			Expect(resourceManager.ApplyFromFiles(snippetsFilter, namespace)).To(Succeed())
-
-			port := 80
-			if portFwdPort != 0 {
-				port = portFwdPort
-			}
-			baseURL = fmt.Sprintf("http://cafe.example.com:%d%s", port, "/coffee")
 		})
 
 		AfterAll(func() {
@@ -86,22 +77,33 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter
 
 		Context("verify working traffic", func() {
 			It("should return a 200 response for HTTPRoute", func() {
+				port := 80
+				if portFwdPort != 0 {
+					port = portFwdPort
+				}
+				baseURL := fmt.Sprintf("http://cafe.example.com:%d%s", port, "/coffee")
+
 				Eventually(
 					func() error {
 						return expectRequestToSucceed(baseURL, address, "URI: /coffee")
 					}).
-					WithTimeout(timeoutConfig.CreateTimeout).
-					WithPolling(5 * time.Second).
+					WithTimeout(timeoutConfig.RequestTimeout).
+					WithPolling(500 * time.Millisecond).
 					Should(Succeed())
 			})
 		})
 
 		Context("nginx directives", func() {
 			var conf *framework.Payload
-			snippetsFilterPrefix := "SnippetsFilter_"
-			filePrefix := "/etc/nginx/includes/"
-			httpRoutePrefix := fmt.Sprintf("%s_all-contexts.conf", namespace)
-			grpcRoutePrefix := fmt.Sprintf("%s_grpc-all-contexts.conf", namespace)
+			snippetsFilterFilePrefix := "/etc/nginx/includes/SnippetsFilter_"
+
+			mainContext := fmt.Sprintf("%ssmain_", snippetsFilterFilePrefix)
+			httpContext := fmt.Sprintf("%shttp_", snippetsFilterFilePrefix)
+			httpServerContext := fmt.Sprintf("%shttp.server_", snippetsFilterFilePrefix)
+			httpServerLocationContext := fmt.Sprintf("%shttp.server.location_", snippetsFilterFilePrefix)
+
+			httpRouteSuffix := fmt.Sprintf("%s_all-contexts.conf", namespace)
+			grpcRouteSuffix := fmt.Sprintf("%s_grpc-all-contexts.conf", namespace)
 
 			BeforeAll(func() {
 				podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout)
@@ -121,70 +123,70 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter
 					}
 				},
 				Entry("HTTPRoute", []framework.ExpectedNginxField{
+					{
+						Directive: "worker_priority",
+						Value:     "0",
+						File:      fmt.Sprintf("%s%s", mainContext, httpRouteSuffix),
+					},
 					{
 						Directive: "aio",
 						Value:     "on",
-						File:      fmt.Sprintf("%s%shttp_%s", filePrefix, snippetsFilterPrefix, httpRoutePrefix),
+						File:      fmt.Sprintf("%s%s", httpContext, httpRouteSuffix),
 					},
 					{
 						Directive: "auth_delay",
 						Value:     "10s",
-						File:      fmt.Sprintf("%s%shttp.server_%s", filePrefix, snippetsFilterPrefix, httpRoutePrefix),
-					},
-					{
-						Directive: "keepalive_time",
-						Value:     "10s",
-						File:      fmt.Sprintf("%s%shttp.server.location_%s", filePrefix, snippetsFilterPrefix, httpRoutePrefix),
-					},
-					{
-						Directive: "worker_priority",
-						Value:     "0",
-						File:      fmt.Sprintf("%s%smain_%s", filePrefix, snippetsFilterPrefix, httpRoutePrefix),
+						File:      fmt.Sprintf("%s%s", httpServerContext, httpRouteSuffix),
 					},
 					{
 						Directive: "include",
-						Value:     fmt.Sprintf("%s%shttp.server_%s", filePrefix, snippetsFilterPrefix, httpRoutePrefix),
+						Value:     fmt.Sprintf("%s%s", httpServerContext, httpRouteSuffix),
 						Servers:   []string{"cafe.example.com"},
 						File:      "http.conf",
 					},
 					{
 						Directive: "include",
-						Value:     fmt.Sprintf("%s%shttp.server.location_%s", filePrefix, snippetsFilterPrefix, httpRoutePrefix),
+						Value:     fmt.Sprintf("%s%s", httpServerLocationContext, httpRouteSuffix),
 						File:      "http.conf",
 						Location:  "/coffee",
 						Servers:   []string{"cafe.example.com"},
 					},
+					{
+						Directive: "keepalive_time",
+						Value:     "10s",
+						File:      fmt.Sprintf("%s%s", httpServerLocationContext, httpRouteSuffix),
+					},
 				}),
 				Entry("GRPCRoute", []framework.ExpectedNginxField{
 					{
 						Directive: "worker_shutdown_timeout",
 						Value:     "120s",
-						File:      fmt.Sprintf("%s%smain_%s", filePrefix, snippetsFilterPrefix, grpcRoutePrefix),
+						File:      fmt.Sprintf("%s%s", mainContext, grpcRouteSuffix),
 					},
 					{
 						Directive: "types_hash_bucket_size",
 						Value:     "64",
-						File:      fmt.Sprintf("%s%shttp_%s", filePrefix, snippetsFilterPrefix, grpcRoutePrefix),
+						File:      fmt.Sprintf("%s%s", httpContext, grpcRouteSuffix),
 					},
 					{
 						Directive: "server_tokens",
 						Value:     "on",
-						File:      fmt.Sprintf("%s%shttp.server_%s", filePrefix, snippetsFilterPrefix, grpcRoutePrefix),
+						File:      fmt.Sprintf("%s%s", httpServerContext, grpcRouteSuffix),
 					},
 					{
 						Directive: "tcp_nodelay",
 						Value:     "on",
-						File:      fmt.Sprintf("%s%shttp.server.location_%s", filePrefix, snippetsFilterPrefix, grpcRoutePrefix),
+						File:      fmt.Sprintf("%s%s", httpServerLocationContext, grpcRouteSuffix),
 					},
 					{
 						Directive: "include",
-						Value:     fmt.Sprintf("%s%shttp.server_%s", filePrefix, snippetsFilterPrefix, grpcRoutePrefix),
+						Value:     fmt.Sprintf("%s%s", httpServerContext, grpcRouteSuffix),
 						Servers:   []string{"*.example.com"},
 						File:      "http.conf",
 					},
 					{
 						Directive: "include",
-						Value:     fmt.Sprintf("%s%shttp.server.location_%s", filePrefix, snippetsFilterPrefix, grpcRoutePrefix),
+						Value:     fmt.Sprintf("%s%s", httpServerLocationContext, grpcRouteSuffix),
 						File:      "http.conf",
 						Location:  "/helloworld.Greeter/SayHello",
 						Servers:   []string{"*.example.com"},
@@ -219,19 +221,15 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter
 	})
 })
 
-func waitForHTTPRouteToHaveGatewayNotProgrammedCond(httpRouteNsNames types.NamespacedName) error {
-	ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.CreateTimeout*2)
+func waitForHTTPRouteToHaveGatewayNotProgrammedCond(httpRouteNsName types.NamespacedName) error {
+	ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout)
 	defer cancel()
 
 	GinkgoWriter.Printf(
 		"Waiting for HTTPRoute %q to have the condition Accepted/True/GatewayNotProgrammed\n",
-		httpRouteNsNames,
+		httpRouteNsName,
 	)
 
-	return waitForHTTPRouteStatus(ctx, httpRouteNsNames)
-}
-
-func waitForHTTPRouteStatus(ctx context.Context, httpRouteNsNames types.NamespacedName) error {
 	return wait.PollUntilContextCancel(
 		ctx,
 		time.Second*3,
@@ -240,12 +238,12 @@ func waitForHTTPRouteStatus(ctx context.Context, httpRouteNsNames types.Namespac
 			var hr v1.HTTPRoute
 			var err error
 
-			if err = k8sClient.Get(ctx, httpRouteNsNames, &hr); err != nil {
+			if err = k8sClient.Get(ctx, httpRouteNsName, &hr); err != nil {
 				return false, err
 			}
 
 			if len(hr.Status.Parents) == 0 {
-				GinkgoWriter.Printf("HTTPRoute %q does not have a status yet\n", httpRouteNsNames)
+				GinkgoWriter.Printf("HTTPRoute %q does not have a status yet\n", httpRouteNsName)
 
 				return false, nil
 			}
@@ -285,20 +283,6 @@ func waitForSnippetsFilterToBeAccepted(snippetsFilterNsNames types.NamespacedNam
 		snippetsFilterNsNames,
 	)
 
-	return waitForSnippetsFilterStatus(
-		ctx,
-		snippetsFilterNsNames,
-		metav1.ConditionTrue,
-		ngfAPI.SnippetsFilterConditionReasonAccepted,
-	)
-}
-
-func waitForSnippetsFilterStatus(
-	ctx context.Context,
-	snippetsFilterNsNames types.NamespacedName,
-	condStatus metav1.ConditionStatus,
-	condReason ngfAPI.SnippetsFilterConditionReason,
-) error {
 	return wait.PollUntilContextCancel(
 		ctx,
 		1000*time.Millisecond,
@@ -331,12 +315,16 @@ func waitForSnippetsFilterStatus(
 				return false, fmt.Errorf("expected condition type to be Accepted, got %s", condition.Type)
 			}
 
-			if status.Conditions[0].Status != condStatus {
-				return false, fmt.Errorf("expected condition status to be %s, got %s", condStatus, condition.Status)
+			if status.Conditions[0].Status != metav1.ConditionTrue {
+				return false, fmt.Errorf("expected condition status to be %s, got %s", metav1.ConditionTrue, condition.Status)
 			}
 
-			if status.Conditions[0].Reason != string(condReason) {
-				return false, fmt.Errorf("expected condition reason to be %s, got %s", condReason, condition.Reason)
+			if status.Conditions[0].Reason != string(ngfAPI.SnippetsFilterConditionReasonAccepted) {
+				return false, fmt.Errorf(
+					"expected condition reason to be %s, got %s",
+					ngfAPI.SnippetsFilterConditionReasonAccepted,
+					condition.Reason,
+				)
 			}
 
 			return err == nil, err