address cr suggestions

levikobi · levikobi · commit 5d7a94ca0f70 · 2024-04-30T08:29:37.000+03:00
Signed-off-by: Kobi Levi &lt;kobilevi503@gmail.com&gt;
diff --git a/internal/cmd/egctl/testdata/translate/out/default-resources.all.yaml b/internal/cmd/egctl/testdata/translate/out/default-resources.all.yaml
@@ -158,6 +158,28 @@ gatewayClass:
       reason: Accepted
       status: "True"
       type: Accepted
+    supportedFeatures:
+    - Gateway
+    - GatewayPort8080
+    - GatewayStaticAddresses
+    - HTTPRoute
+    - HTTPRouteBackendProtocolH2C
+    - HTTPRouteBackendProtocolWebSocket
+    - HTTPRouteBackendTimeout
+    - HTTPRouteDestinationPortMatching
+    - HTTPRouteHostRewrite
+    - HTTPRouteMethodMatching
+    - HTTPRoutePathRedirect
+    - HTTPRoutePathRewrite
+    - HTTPRoutePortRedirect
+    - HTTPRouteQueryParamMatching
+    - HTTPRouteRequestMirror
+    - HTTPRouteRequestMultipleMirrors
+    - HTTPRouteRequestTimeout
+    - HTTPRouteResponseHeaderModification
+    - HTTPRouteSchemeRedirect
+    - ReferenceGrant
+    - TLSRoute
 gateways:
 - metadata:
     creationTimestamp: null
diff --git a/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.cluster.yaml b/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.cluster.yaml
@@ -12,6 +12,28 @@ gatewayClass:
       reason: Accepted
       status: "True"
       type: Accepted
+    supportedFeatures:
+    - Gateway
+    - GatewayPort8080
+    - GatewayStaticAddresses
+    - HTTPRoute
+    - HTTPRouteBackendProtocolH2C
+    - HTTPRouteBackendProtocolWebSocket
+    - HTTPRouteBackendTimeout
+    - HTTPRouteDestinationPortMatching
+    - HTTPRouteHostRewrite
+    - HTTPRouteMethodMatching
+    - HTTPRoutePathRedirect
+    - HTTPRoutePathRewrite
+    - HTTPRoutePortRedirect
+    - HTTPRouteQueryParamMatching
+    - HTTPRouteRequestMirror
+    - HTTPRouteRequestMultipleMirrors
+    - HTTPRouteRequestTimeout
+    - HTTPRouteResponseHeaderModification
+    - HTTPRouteSchemeRedirect
+    - ReferenceGrant
+    - TLSRoute
 gateways:
 - metadata:
     creationTimestamp: null
diff --git a/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.route.json b/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.route.json
@@ -17,6 +17,29 @@
           "reason": "Accepted",
           "message": "Valid GatewayClass"
         }
+      ],
+      "supportedFeatures": [
+        "Gateway",
+        "GatewayPort8080",
+        "GatewayStaticAddresses",
+        "HTTPRoute",
+        "HTTPRouteBackendProtocolH2C",
+        "HTTPRouteBackendProtocolWebSocket",
+        "HTTPRouteBackendTimeout",
+        "HTTPRouteDestinationPortMatching",
+        "HTTPRouteHostRewrite",
+        "HTTPRouteMethodMatching",
+        "HTTPRoutePathRedirect",
+        "HTTPRoutePathRewrite",
+        "HTTPRoutePortRedirect",
+        "HTTPRouteQueryParamMatching",
+        "HTTPRouteRequestMirror",
+        "HTTPRouteRequestMultipleMirrors",
+        "HTTPRouteRequestTimeout",
+        "HTTPRouteResponseHeaderModification",
+        "HTTPRouteSchemeRedirect",
+        "ReferenceGrant",
+        "TLSRoute"
       ]
     }
   },
diff --git a/internal/cmd/egctl/testdata/translate/out/invalid-envoyproxy.all.yaml b/internal/cmd/egctl/testdata/translate/out/invalid-envoyproxy.all.yaml
@@ -38,6 +38,28 @@ gatewayClass:
       reason: InvalidParameters
       status: "False"
       type: Accepted
+    supportedFeatures:
+    - Gateway
+    - GatewayPort8080
+    - GatewayStaticAddresses
+    - HTTPRoute
+    - HTTPRouteBackendProtocolH2C
+    - HTTPRouteBackendProtocolWebSocket
+    - HTTPRouteBackendTimeout
+    - HTTPRouteDestinationPortMatching
+    - HTTPRouteHostRewrite
+    - HTTPRouteMethodMatching
+    - HTTPRoutePathRedirect
+    - HTTPRoutePathRewrite
+    - HTTPRoutePortRedirect
+    - HTTPRouteQueryParamMatching
+    - HTTPRouteRequestMirror
+    - HTTPRouteRequestMultipleMirrors
+    - HTTPRouteRequestTimeout
+    - HTTPRouteResponseHeaderModification
+    - HTTPRouteSchemeRedirect
+    - ReferenceGrant
+    - TLSRoute
 gateways:
 - metadata:
     creationTimestamp: null
diff --git a/internal/cmd/egctl/testdata/translate/out/rejected-http-route.route.yaml b/internal/cmd/egctl/testdata/translate/out/rejected-http-route.route.yaml
@@ -12,6 +12,28 @@ gatewayClass:
       reason: Accepted
       status: "True"
       type: Accepted
+    supportedFeatures:
+    - Gateway
+    - GatewayPort8080
+    - GatewayStaticAddresses
+    - HTTPRoute
+    - HTTPRouteBackendProtocolH2C
+    - HTTPRouteBackendProtocolWebSocket
+    - HTTPRouteBackendTimeout
+    - HTTPRouteDestinationPortMatching
+    - HTTPRouteHostRewrite
+    - HTTPRouteMethodMatching
+    - HTTPRoutePathRedirect
+    - HTTPRoutePathRewrite
+    - HTTPRoutePortRedirect
+    - HTTPRouteQueryParamMatching
+    - HTTPRouteRequestMirror
+    - HTTPRouteRequestMultipleMirrors
+    - HTTPRouteRequestTimeout
+    - HTTPRouteResponseHeaderModification
+    - HTTPRouteSchemeRedirect
+    - ReferenceGrant
+    - TLSRoute
 gateways:
 - metadata:
     creationTimestamp: null
diff --git a/internal/cmd/egctl/testdata/translate/out/valid-envoyproxy.all.yaml b/internal/cmd/egctl/testdata/translate/out/valid-envoyproxy.all.yaml
@@ -31,6 +31,28 @@ gatewayClass:
       reason: Accepted
       status: "True"
       type: Accepted
+    supportedFeatures:
+    - Gateway
+    - GatewayPort8080
+    - GatewayStaticAddresses
+    - HTTPRoute
+    - HTTPRouteBackendProtocolH2C
+    - HTTPRouteBackendProtocolWebSocket
+    - HTTPRouteBackendTimeout
+    - HTTPRouteDestinationPortMatching
+    - HTTPRouteHostRewrite
+    - HTTPRouteMethodMatching
+    - HTTPRoutePathRedirect
+    - HTTPRoutePathRewrite
+    - HTTPRoutePortRedirect
+    - HTTPRouteQueryParamMatching
+    - HTTPRouteRequestMirror
+    - HTTPRouteRequestMultipleMirrors
+    - HTTPRouteRequestTimeout
+    - HTTPRouteResponseHeaderModification
+    - HTTPRouteSchemeRedirect
+    - ReferenceGrant
+    - TLSRoute
 gateways:
 - metadata:
     creationTimestamp: null
diff --git a/internal/cmd/egctl/translate.go b/internal/cmd/egctl/translate.go
@@ -319,13 +319,11 @@ func translateGatewayAPIToGatewayAPI(resources *gatewayapi.Resources) (gatewayap
 			epInvalid = true
 			msg := fmt.Sprintf("%s: %v", status.MsgGatewayClassInvalidParams, err)
 			status.SetGatewayClassAccepted(resources.GatewayClass, false, string(gwapiv1.GatewayClassReasonInvalidParameters), msg)
-			status.SetGatewayClassSupportedFeatures(resources.GatewayClass)
 		}
 		gRes.EnvoyProxy = resources.EnvoyProxy
 	}
 	if !epInvalid {
 		status.SetGatewayClassAccepted(resources.GatewayClass, true, string(gwapiv1.GatewayClassReasonAccepted), status.MsgValidGatewayClass)
-		status.SetGatewayClassSupportedFeatures(resources.GatewayClass)
 	}
 
 	gRes.GatewayClass = resources.GatewayClass
diff --git a/internal/cmd/egctl/translate_test.go b/internal/cmd/egctl/translate_test.go
@@ -22,6 +22,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
 
+	"github.com/envoyproxy/gateway/internal/status"
 	"github.com/envoyproxy/gateway/internal/utils/field"
 	"github.com/envoyproxy/gateway/internal/utils/file"
 )
@@ -349,6 +350,13 @@ func TestTranslate(t *testing.T) {
 			}
 			want := &TranslationResult{}
 			mustUnmarshal(t, requireTestDataOutFile(t, fn), want)
+
+			// Supported features are dynamic, instead of hard-coding them in the output files
+			// we define them here.
+			if want.GatewayClass != nil {
+				want.GatewayClass.Status.SupportedFeatures = status.GatewaySupportedFeatures
+			}
+
 			opts := cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")
 			require.Empty(t, cmp.Diff(want, got, opts))
 		})
diff --git a/internal/provider/kubernetes/controller_test.go b/internal/provider/kubernetes/controller_test.go
@@ -190,7 +190,7 @@ func TestHasManagedClass(t *testing.T) {
 								Status: metav1.ConditionTrue,
 							},
 						},
-						SupportedFeatures: status.GetSupportedFeatures(),
+						SupportedFeatures: status.GatewaySupportedFeatures,
 					},
 				},
 			},
@@ -225,7 +225,7 @@ func TestHasManagedClass(t *testing.T) {
 								Status: metav1.ConditionTrue,
 							},
 						},
-						SupportedFeatures: status.GetSupportedFeatures(),
+						SupportedFeatures: status.GatewaySupportedFeatures,
 					},
 				},
 			},
@@ -278,7 +278,7 @@ func TestHasManagedClass(t *testing.T) {
 								Status: metav1.ConditionTrue,
 							},
 						},
-						SupportedFeatures: status.GetSupportedFeatures(),
+						SupportedFeatures: status.GatewaySupportedFeatures,
 					},
 				},
 				{
@@ -301,7 +301,7 @@ func TestHasManagedClass(t *testing.T) {
 								Status: metav1.ConditionFalse,
 							},
 						},
-						SupportedFeatures: status.GetSupportedFeatures(),
+						SupportedFeatures: status.GatewaySupportedFeatures,
 					},
 				},
 			},
diff --git a/internal/status/gatewayclass.go b/internal/status/gatewayclass.go
@@ -16,23 +16,17 @@ package status
 import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
-	"sigs.k8s.io/gateway-api/conformance/utils/suite"
-)
 
-// SetGatewayClassAccepted inserts or updates the Accepted condition
-// for the provided GatewayClass.
-func SetGatewayClassAccepted(gc *gwapiv1.GatewayClass, accepted bool, reason, msg string) *gwapiv1.GatewayClass {
-	gc.Status.Conditions = MergeConditions(gc.Status.Conditions, computeGatewayClassAcceptedCondition(gc, accepted, reason, msg))
-	return gc
-}
+	"github.com/envoyproxy/gateway/test/conformance"
+)
 
-// GetSupportedFeatures returns a list of supported Gateway-API features,
+// GatewaySupportedFeatures is a list of supported Gateway-API features,
 // based on the running conformance tests suite.
-func GetSupportedFeatures() []gwapiv1.SupportedFeature {
+var GatewaySupportedFeatures = getSupportedFeatures()
 
-	// TODO(levikobi): This must be in sync with the cSuite supported features.
-	supportedFeatures := suite.AllFeatures
-	supportedFeatures.Delete(suite.MeshCoreFeatures.UnsortedList()...)
+func getSupportedFeatures() []gwapiv1.SupportedFeature {
+	supportedFeatures := conformance.EnvoyGatewaySuite.SupportedFeatures
+	supportedFeatures.Delete(conformance.EnvoyGatewaySuite.ExemptFeatures.UnsortedList()...)
 
 	ret := sets.New[gwapiv1.SupportedFeature]()
 	for _, feature := range supportedFeatures.UnsortedList() {
@@ -41,9 +35,10 @@ func GetSupportedFeatures() []gwapiv1.SupportedFeature {
 	return sets.List(ret)
 }
 
-// SetGatewayClassSupportedFeatures insert or updates the SupportedFeatures field
+// SetGatewayClassAccepted inserts or updates the Accepted condition
 // for the provided GatewayClass.
-func SetGatewayClassSupportedFeatures(gc *gwapiv1.GatewayClass) *gwapiv1.GatewayClass {
-	gc.Status.SupportedFeatures = GetSupportedFeatures()
+func SetGatewayClassAccepted(gc *gwapiv1.GatewayClass, accepted bool, reason, msg string) *gwapiv1.GatewayClass {
+	gc.Status.Conditions = MergeConditions(gc.Status.Conditions, computeGatewayClassAcceptedCondition(gc, accepted, reason, msg))
+	gc.Status.SupportedFeatures = GatewaySupportedFeatures
 	return gc
 }
diff --git a/test/conformance/conformance_features.go b/test/conformance/conformance_features.go
@@ -0,0 +1,17 @@
+// Copyright Envoy Gateway Authors
+// SPDX-License-Identifier: Apache-2.0
+// The full text of the Apache license is available in the LICENSE file at
+// the root of the repo.
+
+package conformance
+
+import (
+	"sigs.k8s.io/gateway-api/conformance/tests"
+	"sigs.k8s.io/gateway-api/conformance/utils/suite"
+)
+
+var EnvoyGatewaySuite = suite.Options{
+	SupportedFeatures: suite.AllFeatures,
+	ExemptFeatures:    suite.MeshCoreFeatures,
+	SkipTests:         []string{tests.GatewayStaticAddresses.ShortName},
+}
diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go
@@ -46,11 +46,9 @@ func TestGatewayAPIConformance(t *testing.T) {
 		Debug:                *flags.ShowDebug,
 		Clientset:            clientset,
 		CleanupBaseResources: *flags.CleanupBaseResources,
-		SupportedFeatures:    suite.AllFeatures,
-		SkipTests: []string{
-			tests.GatewayStaticAddresses.ShortName,
-		},
-		ExemptFeatures: suite.MeshCoreFeatures,
+		SupportedFeatures:    EnvoyGatewaySuite.SupportedFeatures,
+		SkipTests:            EnvoyGatewaySuite.SkipTests,
+		ExemptFeatures:       EnvoyGatewaySuite.ExemptFeatures,
 	})
 	cSuite.Setup(t)
 	cSuite.Run(t, tests.ConformanceTests)
diff --git a/test/conformance/experimental_conformance_test.go b/test/conformance/experimental_conformance_test.go
@@ -95,11 +95,9 @@ func experimentalConformance(t *testing.T) {
 				GatewayClassName:     *flags.GatewayClassName,
 				Debug:                *flags.ShowDebug,
 				CleanupBaseResources: *flags.CleanupBaseResources,
-				SupportedFeatures:    suite.AllFeatures,
-				SkipTests: []string{
-					tests.GatewayStaticAddresses.ShortName,
-				},
-				ExemptFeatures: suite.MeshCoreFeatures,
+				SupportedFeatures:    EnvoyGatewaySuite.SupportedFeatures,
+				SkipTests:            EnvoyGatewaySuite.SkipTests,
+				ExemptFeatures:       EnvoyGatewaySuite.ExemptFeatures,
 			},
 			Implementation:      *implementation,
 			ConformanceProfiles: conformanceProfiles,

Original file line number	Diff line number	Diff line change
`@@ -319,13 +319,11 @@ func translateGatewayAPIToGatewayAPI(resources *gatewayapi.Resources) (gatewayap`
`319`	`319`	`epInvalid = true`
`320`	`320`	`msg := fmt.Sprintf("%s: %v", status.MsgGatewayClassInvalidParams, err)`
`321`	`321`	`status.SetGatewayClassAccepted(resources.GatewayClass, false, string(gwapiv1.GatewayClassReasonInvalidParameters), msg)`
`322`		`- status.SetGatewayClassSupportedFeatures(resources.GatewayClass)`
`323`	`322`	`}`
`324`	`323`	`gRes.EnvoyProxy = resources.EnvoyProxy`
`325`	`324`	`}`
`326`	`325`	`if !epInvalid {`
`327`	`326`	`status.SetGatewayClassAccepted(resources.GatewayClass, true, string(gwapiv1.GatewayClassReasonAccepted), status.MsgValidGatewayClass)`
`328`		`- status.SetGatewayClassSupportedFeatures(resources.GatewayClass)`
`329`	`327`	`}`
`330`	`328`
`331`	`329`	`gRes.GatewayClass = resources.GatewayClass`