conformance: enable HTTPRouteRedirectPortAndScheme (#1601)

* add logic for final redirect port derived Signed-off-by: Shawnh2 <[email protected]> * fix unit test Signed-off-by: Shawnh2 <[email protected]> * resolve conflicts Signed-off-by: sh2 <[email protected]> * change hash sum and unify ths format of nsName Signed-off-by: sh2 <[email protected]> * update unit test Signed-off-by: sh2 <[email protected]> * fix test Signed-off-by: sh2 <[email protected]> * optimize scheme check and fix slash pos Signed-off-by: sh2 <[email protected]> --------- Signed-off-by: Shawnh2 <[email protected]> Signed-off-by: sh2 <[email protected]>
envoyproxy · Aug 11, 2023 · f466430 · f466430
1 parent c0a619c
commit f466430
Show file tree

Hide file tree

Showing 21 changed files with 62 additions and 46 deletions.
diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go
@@ -509,6 +509,24 @@ func (t *Translator) processHTTPRouteParentRefListener(route RouteContext, route
 			}
 
 			for _, routeRoute := range routeRoutes {
+				// If the redirect port is not set, the final redirect port must be derived.
+				if routeRoute.Redirect != nil && routeRoute.Redirect.Port == nil {
+					redirectPort := uint32(listener.Port)
+					// If redirect scheme is not-empty, the redirect post must be the
+					// well-known port associated with the redirect scheme.
+					if scheme := routeRoute.Redirect.Scheme; scheme != nil {
+						switch strings.ToLower(*scheme) {
+						case "http":
+							redirectPort = 80
+						case "https":
+							redirectPort = 443
+						}
+					}
+					// If the redirect scheme does not have a well-known port, or
+					// if the redirect scheme is empty, the redirect port must be the Gateway Listener port.
+					routeRoute.Redirect.Port = &redirectPort
+				}
+
 				hostRoute := &ir.HTTPRoute{
 					Name:                  fmt.Sprintf("%s-%s", routeRoute.Name, host),
 					PathMatch:             routeRoute.PathMatch,

diff --git a/internal/gatewayapi/testdata/httproute-with-redirect-filter-full-path-replace-https.out.yaml b/internal/gatewayapi/testdata/httproute-with-redirect-filter-full-path-replace-https.out.yaml
@@ -127,6 +127,6 @@ xdsIR:
           path:
             fullReplace: /redirected
             prefixMatchReplace: null
-          port: null
+          port: 443
           scheme: https
           statusCode: 301
diff --git a/internal/gatewayapi/testdata/httproute-with-redirect-filter-hostname.out.yaml b/internal/gatewayapi/testdata/httproute-with-redirect-filter-hostname.out.yaml
@@ -123,6 +123,6 @@ xdsIR:
         redirect:
           hostname: redirected.com
           path: null
-          port: null
+          port: 443
           scheme: https
           statusCode: 301
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/configmap.yaml b/internal/infrastructure/kubernetes/proxy/testdata/configmap.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 data:
   xds-certificate.json: '{"resources":[{"@type":"type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret","name":"xds_certificate","tls_certificate":{"certificate_chain":{"filename":"/certs/tls.crt"},"private_key":{"filename":"/certs/tls.key"}}}]}'

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   replicas: 1
@@ -84,7 +84,7 @@ spec:
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler
-      serviceAccountName: envoy-default-64656661
+      serviceAccountName: envoy-default-37a8eec1
       terminationGracePeriodSeconds: 300
       volumes:
         - name: certs
@@ -98,7 +98,7 @@ spec:
                 path: xds-trusted-ca.json
               - key: xds-certificate.json
                 path: xds-certificate.json
-            name: envoy-default-64656661
+            name: envoy-default-37a8eec1
             optional: false
           name: sds
   revisionHistoryLimit: 10

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   replicas: 1
@@ -85,7 +85,7 @@ spec:
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler
-      serviceAccountName: envoy-default-64656661
+      serviceAccountName: envoy-default-37a8eec1
       terminationGracePeriodSeconds: 300
       volumes:
         - name: certs
@@ -99,7 +99,7 @@ spec:
                 path: xds-trusted-ca.json
               - key: xds-certificate.json
                 path: xds-certificate.json
-            name: envoy-default-64656661
+            name: envoy-default-37a8eec1
             optional: false
           name: sds
   revisionHistoryLimit: 10

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   replicas: 2
@@ -194,7 +194,7 @@ spec:
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler
-      serviceAccountName: envoy-default-64656661
+      serviceAccountName: envoy-default-37a8eec1
       terminationGracePeriodSeconds: 300
       securityContext:
         runAsUser: 1000
@@ -210,7 +210,7 @@ spec:
                 path: xds-trusted-ca.json
               - key: xds-certificate.json
                 path: xds-certificate.json
-            name: envoy-default-64656661
+            name: envoy-default-37a8eec1
             optional: false
           name: sds
   revisionHistoryLimit: 10

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default-env.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default-env.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   replicas: 2
@@ -192,7 +192,7 @@ spec:
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler
-      serviceAccountName: envoy-default-64656661
+      serviceAccountName: envoy-default-37a8eec1
       terminationGracePeriodSeconds: 300
       securityContext:
         runAsUser: 1000
@@ -208,7 +208,7 @@ spec:
                 path: xds-trusted-ca.json
               - key: xds-certificate.json
                 path: xds-certificate.json
-            name: envoy-default-64656661
+            name: envoy-default-37a8eec1
             optional: false
           name: sds
   revisionHistoryLimit: 10

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   replicas: 1
@@ -185,7 +185,7 @@ spec:
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler
-      serviceAccountName: envoy-default-64656661
+      serviceAccountName: envoy-default-37a8eec1
       terminationGracePeriodSeconds: 300
       volumes:
         - name: certs
@@ -199,7 +199,7 @@ spec:
                 path: xds-trusted-ca.json
               - key: xds-certificate.json
                 path: xds-certificate.json
-            name: envoy-default-64656661
+            name: envoy-default-37a8eec1
             optional: false
           name: sds
   revisionHistoryLimit: 10

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-prometheus.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-prometheus.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   replicas: 1
@@ -214,7 +214,7 @@ spec:
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler
-      serviceAccountName: envoy-default-64656661
+      serviceAccountName: envoy-default-37a8eec1
       terminationGracePeriodSeconds: 300
       volumes:
         - name: certs
@@ -228,7 +228,7 @@ spec:
                 path: xds-trusted-ca.json
               - key: xds-certificate.json
                 path: xds-certificate.json
-            name: envoy-default-64656661
+            name: envoy-default-37a8eec1
             optional: false
           name: sds
   revisionHistoryLimit: 10

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/extension-env.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/extension-env.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   replicas: 2
@@ -196,7 +196,7 @@ spec:
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler
-      serviceAccountName: envoy-default-64656661
+      serviceAccountName: envoy-default-37a8eec1
       terminationGracePeriodSeconds: 300
       securityContext:
         runAsUser: 1000
@@ -212,7 +212,7 @@ spec:
                 path: xds-trusted-ca.json
               - key: xds-certificate.json
                 path: xds-certificate.json
-            name: envoy-default-64656661
+            name: envoy-default-37a8eec1
             optional: false
           name: sds
   revisionHistoryLimit: 10

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/volumes.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/volumes.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   replicas: 2
@@ -196,7 +196,7 @@ spec:
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler
-      serviceAccountName: envoy-default-64656661
+      serviceAccountName: envoy-default-37a8eec1
       terminationGracePeriodSeconds: 300
       securityContext:
         runAsUser: 1000
@@ -212,7 +212,7 @@ spec:
                 path: xds-trusted-ca.json
               - key: xds-certificate.json
                 path: xds-certificate.json
-            name: envoy-default-64656661
+            name: envoy-default-37a8eec1
             optional: false
           name: sds
   revisionHistoryLimit: 10

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   replicas: 1
@@ -85,7 +85,7 @@ spec:
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler
-      serviceAccountName: envoy-default-64656661
+      serviceAccountName: envoy-default-37a8eec1
       terminationGracePeriodSeconds: 300
       volumes:
         - name: certs
@@ -99,7 +99,7 @@ spec:
                 path: xds-trusted-ca.json
               - key: xds-certificate.json
                 path: xds-certificate.json
-            name: envoy-default-64656661
+            name: envoy-default-37a8eec1
             optional: false
           name: sds
   revisionHistoryLimit: 10

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount.yaml b/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount.yaml
@@ -7,5 +7,5 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/services/custom.yaml b/internal/infrastructure/kubernetes/proxy/testdata/services/custom.yaml
@@ -9,7 +9,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   ports:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/services/default.yaml b/internal/infrastructure/kubernetes/proxy/testdata/services/default.yaml
@@ -7,7 +7,7 @@ metadata:
     app.kubernetes.io/managed-by: envoy-gateway
     gateway.envoyproxy.io/owning-gateway-name: default
     gateway.envoyproxy.io/owning-gateway-namespace: default
-  name: envoy-default-64656661
+  name: envoy-default-37a8eec1
   namespace: envoy-gateway-system
 spec:
   externalTrafficPolicy: Local

diff --git a/internal/infrastructure/kubernetes/proxy_configmap_test.go b/internal/infrastructure/kubernetes/proxy_configmap_test.go
@@ -43,7 +43,7 @@ func TestCreateOrUpdateProxyConfigMap(t *testing.T) {
 			expect: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: cfg.Namespace,
-					Name:      "envoy-test-74657374",
+					Name:      "envoy-test-9f86d081",
 					Labels: map[string]string{
 						"app.kubernetes.io/name":               "envoy",
 						"app.kubernetes.io/component":          "proxy",
@@ -77,7 +77,7 @@ func TestCreateOrUpdateProxyConfigMap(t *testing.T) {
 			expect: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: cfg.Namespace,
-					Name:      "envoy-test-74657374",
+					Name:      "envoy-test-9f86d081",
 					Labels: map[string]string{
 						"app.kubernetes.io/name":               "envoy",
 						"app.kubernetes.io/component":          "proxy",

diff --git a/internal/infrastructure/kubernetes/proxy_serviceaccount_test.go b/internal/infrastructure/kubernetes/proxy_serviceaccount_test.go
@@ -54,7 +54,7 @@ func TestCreateOrUpdateProxyServiceAccount(t *testing.T) {
 				},
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "test",
-					Name:      "envoy-test-74657374",
+					Name:      "envoy-test-9f86d081",
 					Labels: map[string]string{
 						"app.kubernetes.io/name":               "envoy",
 						"app.kubernetes.io/component":          "proxy",
@@ -103,7 +103,7 @@ func TestCreateOrUpdateProxyServiceAccount(t *testing.T) {
 				},
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "test",
-					Name:      "envoy-test-74657374",
+					Name:      "envoy-test-9f86d081",
 					Labels: map[string]string{
 						"app.kubernetes.io/name":               "envoy",
 						"app.kubernetes.io/component":          "proxy",
@@ -152,7 +152,7 @@ func TestCreateOrUpdateProxyServiceAccount(t *testing.T) {
 				},
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "test",
-					Name:      "envoy-very-long-name-that-will-be-hashed-and-cut-off-b-76657279",
+					Name:      "envoy-very-long-name-that-will-be-hashed-and-cut-off-b-5bacc75e",
 					Labels: map[string]string{
 						"app.kubernetes.io/name":               "envoy",
 						"app.kubernetes.io/component":          "proxy",

diff --git a/internal/provider/kubernetes/helpers.go b/internal/provider/kubernetes/helpers.go
@@ -197,12 +197,12 @@ func refsSecret(ref *gwapiv1b1.SecretObjectReference) bool {
 }
 
 func infraServiceName(gateway *gwapiv1b1.Gateway) string {
-	infraName := utils.GetHashedName(fmt.Sprintf("%s-%s", gateway.Namespace, gateway.Name))
+	infraName := utils.GetHashedName(fmt.Sprintf("%s/%s", gateway.Namespace, gateway.Name))
 	return fmt.Sprintf("%s-%s", config.EnvoyPrefix, infraName)
 }
 
 func infraDeploymentName(gateway *gwapiv1b1.Gateway) string {
-	infraName := utils.GetHashedName(fmt.Sprintf("%s-%s", gateway.Namespace, gateway.Name))
+	infraName := utils.GetHashedName(fmt.Sprintf("%s/%s", gateway.Namespace, gateway.Name))
 	return fmt.Sprintf("%s-%s", config.EnvoyPrefix, infraName)
 }
 

diff --git a/internal/provider/utils/utils.go b/internal/provider/utils/utils.go
@@ -22,12 +22,13 @@ func NamespacedName(obj client.Object) types.NamespacedName {
 	}
 }
 
-// GetHashedName returns a partially hashed name for the string including up to 48 characters of the original name before the hash
+// GetHashedName returns a partially hashed name for the string including up to 48 characters of the original name before the hash.
+// Input `nsName` should be formatted as `{Namespace}/{ResourceName}`.
 func GetHashedName(nsName string) string {
 
 	h := sha256.New() // Using sha256 instead of sha1 due to Blocklisted import crypto/sha1: weak cryptographic primitive (gosec)
-	hSum := h.Sum([]byte(nsName))
-	hashedName := strings.ToLower(fmt.Sprintf("%x", hSum))
+	h.Write([]byte(nsName))
+	hashedName := strings.ToLower(fmt.Sprintf("%x", h.Sum(nil)))
 
 	// replace `/` with `-` to create a valid K8s resource name
 	resourceName := strings.ReplaceAll(nsName, "/", "-")