Hash ir/infra resources (envoyproxy#559)

* infra: hash resources with long names

Signed-off-by: AliceProxy <alicewasko@datawire.io>

* add tests for hashing resources

Signed-off-by: AliceProxy <alicewasko@datawire.io>

* hashing: replace sha1 with sha256

Signed-off-by: AliceProxy <alicewasko@datawire.io>

* hashing: only use 8 chars

Signed-off-by: AliceProxy <alicewasko@datawire.io>

* ir/infra: always hash resource names

Signed-off-by: AliceProxy <alicewasko@datawire.io>

* update all test manifests with hashed names

Signed-off-by: AliceProxy <alicewasko@datawire.io>

* only hash necessary resources

Signed-off-by: AliceProxy <alicewasko@datawire.io>

* update test manifests

Signed-off-by: AliceProxy <alicewasko@datawire.io>

Signed-off-by: AliceProxy <alicewasko@datawire.io>

internal/envoygateway/config/config.go

@@ -12,12 +12,8 @@ const (
 	EnvoyGatewayNamespace = "envoy-gateway-system"
 	// EnvoyGatewayServiceName is the name of the Envoy Gateway service.
 	EnvoyGatewayServiceName = "envoy-gateway"
-	// EnvoyConfigMapPrefix is the prefix applied to the Envoy ConfigMap.
-	EnvoyConfigMapPrefix = "envoy"
-	// EnvoyServicePrefix is the prefix applied to the Envoy Service.
-	EnvoyServicePrefix = "envoy"
-	// EnvoyDeploymentPrefix is the prefix applied to the Envoy Deployment.
-	EnvoyDeploymentPrefix = "envoy"
+	// EnvoyPrefix is the prefix applied to the Envoy ConfigMap, Service, Deployment, and ServiceAccount.
+	EnvoyPrefix = "envoy"
 )
 
 // Server wraps the EnvoyGateway configuration and additional parameters

internal/infrastructure/kubernetes/configmap.go

@@ -13,6 +13,7 @@ import (
 	"github.com/envoyproxy/gateway/internal/envoygateway/config"
 	"github.com/envoyproxy/gateway/internal/gatewayapi"
 	"github.com/envoyproxy/gateway/internal/ir"
+	"github.com/envoyproxy/gateway/internal/provider/utils"
 )
 
 const (
@@ -113,5 +114,6 @@ func (i *Infra) deleteConfigMap(ctx context.Context, infra *ir.Infra) error {
 }
 
 func expectedConfigMapName(proxyName string) string {
-	return fmt.Sprintf("%s-%s", config.EnvoyConfigMapPrefix, proxyName)
+	cMapName := utils.GetHashedName(proxyName)
+	return fmt.Sprintf("%s-%s", config.EnvoyPrefix, cMapName)
 }

internal/infrastructure/kubernetes/configmap_test.go

@@ -22,6 +22,7 @@ func TestExpectedConfigMap(t *testing.T) {
 	cli := fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).WithObjects().Build()
 	kube := NewInfra(cli)
 	infra := ir.NewInfra()
+
 	infra.Proxy.Name = "test"
 
 	// An infra without Gateway owner labels should trigger
@@ -35,7 +36,7 @@ func TestExpectedConfigMap(t *testing.T) {
 	cm, err := kube.expectedConfigMap(infra)
 	require.NoError(t, err)
 
-	require.Equal(t, "envoy-test", cm.Name)
+	require.Equal(t, "envoy-test-74657374", cm.Name)
 	require.Equal(t, "envoy-gateway-system", cm.Namespace)
 	require.Contains(t, cm.Data, sdsCAFilename)
 	assert.Equal(t, sdsCAConfigMapData, cm.Data[sdsCAFilename])
@@ -65,7 +66,7 @@ func TestCreateOrUpdateConfigMap(t *testing.T) {
 			expect: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: config.EnvoyGatewayNamespace,
-					Name:      "envoy-test",
+					Name:      "envoy-test-74657374",
 					Labels: map[string]string{
 						"app.gateway.envoyproxy.io/name":       "envoy",
 						gatewayapi.OwningGatewayNamespaceLabel: "default",
@@ -92,7 +93,7 @@ func TestCreateOrUpdateConfigMap(t *testing.T) {
 			expect: &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: config.EnvoyGatewayNamespace,
-					Name:      "envoy-test",
+					Name:      "envoy-test-74657374",
 					Labels: map[string]string{
 						"app.gateway.envoyproxy.io/name":       "envoy",
 						gatewayapi.OwningGatewayNamespaceLabel: "default",

internal/infrastructure/kubernetes/deployment.go

@@ -18,6 +18,7 @@ import (
 	"github.com/envoyproxy/gateway/internal/envoygateway/config"
 	"github.com/envoyproxy/gateway/internal/gatewayapi"
 	"github.com/envoyproxy/gateway/internal/ir"
+	"github.com/envoyproxy/gateway/internal/provider/utils"
 	xdsrunner "github.com/envoyproxy/gateway/internal/xds/server/runner"
 )
 
@@ -94,7 +95,8 @@ func (b *bootstrapConfig) render() error {
 }
 
 func expectedDeploymentName(proxyName string) string {
-	return fmt.Sprintf("%s-%s", config.EnvoyDeploymentPrefix, proxyName)
+	deploymentName := utils.GetHashedName(proxyName)
+	return fmt.Sprintf("%s-%s", config.EnvoyPrefix, deploymentName)
 }
 
 // expectedDeployment returns the expected Deployment based on the provided infra.

internal/infrastructure/kubernetes/service.go

@@ -14,10 +14,12 @@ import (
 	"github.com/envoyproxy/gateway/internal/envoygateway/config"
 	"github.com/envoyproxy/gateway/internal/gatewayapi"
 	"github.com/envoyproxy/gateway/internal/ir"
+	"github.com/envoyproxy/gateway/internal/provider/utils"
 )
 
 func expectedServiceName(proxyName string) string {
-	return fmt.Sprintf("%s-%s", config.EnvoyServicePrefix, proxyName)
+	svcName := utils.GetHashedName(proxyName)
+	return fmt.Sprintf("%s-%s", config.EnvoyPrefix, svcName)
 }
 
 // expectedService returns the expected Service based on the provided infra.

internal/infrastructure/kubernetes/serviceaccount.go

@@ -9,16 +9,15 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 
+	"github.com/envoyproxy/gateway/internal/envoygateway/config"
 	"github.com/envoyproxy/gateway/internal/gatewayapi"
 	"github.com/envoyproxy/gateway/internal/ir"
-)
-
-const (
-	envoyServiceAccountPrefix = "envoy"
+	"github.com/envoyproxy/gateway/internal/provider/utils"
 )
 
 func expectedServiceAccountName(proxyName string) string {
-	return fmt.Sprintf("%s-%s", envoyServiceAccountPrefix, proxyName)
+	svcActName := utils.GetHashedName(proxyName)
+	return fmt.Sprintf("%s-%s", config.EnvoyPrefix, svcActName)
 }
 
 // expectedServiceAccount returns the expected proxy serviceAccount.

internal/infrastructure/kubernetes/serviceaccount_test.go

@@ -73,7 +73,7 @@ func TestCreateOrUpdateServiceAccount(t *testing.T) {
 				},
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "test",
-					Name:      "envoy-test",
+					Name:      "envoy-test-74657374",
 					Labels: map[string]string{
 						"app.gateway.envoyproxy.io/name":       "envoy",
 						gatewayapi.OwningGatewayNamespaceLabel: "default",
@@ -118,7 +118,52 @@ func TestCreateOrUpdateServiceAccount(t *testing.T) {
 				},
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "test",
-					Name:      "envoy-test",
+					Name:      "envoy-test-74657374",
+					Labels: map[string]string{
+						"app.gateway.envoyproxy.io/name":       "envoy",
+						gatewayapi.OwningGatewayNamespaceLabel: "default",
+						gatewayapi.OwningGatewayNameLabel:      "gateway-1",
+					},
+				},
+			},
+		},
+		{
+			name: "hashed-name",
+			ns:   "test",
+			in: &ir.Infra{
+				Proxy: &ir.ProxyInfra{
+					Name: "very-long-name-that-will-be-hashed-and-cut-off-because-its-too-long",
+					Metadata: &ir.InfraMetadata{
+						Labels: map[string]string{
+							gatewayapi.OwningGatewayNamespaceLabel: "default",
+							gatewayapi.OwningGatewayNameLabel:      "gateway-1",
+						},
+					},
+				},
+			},
+			current: &corev1.ServiceAccount{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "ServiceAccount",
+					APIVersion: "v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test",
+					Name:      "very-long-name-that-will-be-hashed-and-cut-off-because-its-too-long",
+					Labels: map[string]string{
+						"app.gateway.envoyproxy.io/name":       "envoy",
+						gatewayapi.OwningGatewayNamespaceLabel: "default",
+						gatewayapi.OwningGatewayNameLabel:      "gateway-1",
+					},
+				},
+			},
+			want: &corev1.ServiceAccount{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "ServiceAccount",
+					APIVersion: "v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test",
+					Name:      "envoy-very-long-name-that-will-be-hashed-and-cut-off-b-76657279",
 					Labels: map[string]string{
 						"app.gateway.envoyproxy.io/name":       "envoy",
 						gatewayapi.OwningGatewayNamespaceLabel: "default",

internal/provider/kubernetes/gateway.go

@@ -649,9 +649,11 @@ func (r *gatewayReconciler) subscribeAndUpdateStatus(ctx context.Context) {
 }
 
 func infraServiceName(gateway *gwapiv1b1.Gateway) string {
-	return fmt.Sprintf("%s-%s-%s", config.EnvoyServicePrefix, 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 {
-	return fmt.Sprintf("%s-%s-%s", config.EnvoyDeploymentPrefix, gateway.Namespace, gateway.Name)
+	infraName := utils.GetHashedName(fmt.Sprintf("%s-%s", gateway.Namespace, gateway.Name))
+	return fmt.Sprintf("%s-%s", config.EnvoyPrefix, infraName)
 }

internal/provider/kubernetes/gateway_test.go

@@ -99,6 +99,23 @@ func TestGatewayHasMatchingController(t *testing.T) {
 			},
 			expect: false,
 		},
+		{
+			name: "matching but very long name",
+			obj: &gwapiv1b1.Gateway{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "Gateway",
+					APIVersion: fmt.Sprintf("%s/%s", gwapiv1b1.GroupName, gwapiv1b1.GroupVersion.Version),
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "superdupermegalongnamethatisridiculouslylongandwaylongerthanitshouldeverbeinsideofkubernetes",
+					Namespace: "test",
+				},
+				Spec: gwapiv1b1.GatewaySpec{
+					GatewayClassName: gwapiv1b1.ObjectName(match.Name),
+				},
+			},
+			expect: true,
+		},
 	}
 
 	// Create the reconciler.

internal/provider/kubernetes/kubernetes_test.go

@@ -254,6 +254,106 @@ func testGatewayScheduledStatus(ctx context.Context, t *testing.T, provider *Pro
 	assert.Equal(t, gw.Spec, gws.Spec)
 }
 
+// Test that even when resources such as the Service/Deployment get hashed names (because of a gateway with a very long name)
+func testLongNameHashedResources(ctx context.Context, t *testing.T, provider *Provider, resources *message.ProviderResources) {
+	cli := provider.manager.GetClient()
+
+	gc := getGatewayClass("envoy-gateway-class")
+	require.NoError(t, cli.Create(ctx, gc))
+
+	// Ensure the GatewayClass reports "Ready".
+	require.Eventually(t, func() bool {
+		if err := cli.Get(ctx, types.NamespacedName{Name: gc.Name}, gc); err != nil {
+			return false
+		}
+
+		for _, cond := range gc.Status.Conditions {
+			if cond.Type == string(gwapiv1b1.GatewayClassConditionStatusAccepted) && cond.Status == metav1.ConditionTrue {
+				return true
+			}
+		}
+
+		return false
+	}, defaultWait, defaultTick)
+
+	defer func() {
+		require.NoError(t, cli.Delete(ctx, gc))
+	}()
+
+	// Create the namespace for the Gateway under test.
+	ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "envoy-gateway"}}
+	require.NoError(t, cli.Create(ctx, ns))
+
+	gw := &gwapiv1b1.Gateway{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "gatewaywithaverylongnamethatwillresultinhashedresources",
+			Namespace: ns.Name,
+		},
+		Spec: gwapiv1b1.GatewaySpec{
+			GatewayClassName: gwapiv1b1.ObjectName(gc.Name),
+			Listeners: []gwapiv1b1.Listener{
+				{
+					Name:     "test",
+					Port:     gwapiv1b1.PortNumber(int32(8080)),
+					Protocol: gwapiv1b1.HTTPProtocolType,
+				},
+			},
+		},
+	}
+	require.NoError(t, cli.Create(ctx, gw))
+
+	// Ensure the Gateway is ready and gets an address.
+	ready := false
+	hasAddress := false
+	require.Eventually(t, func() bool {
+		if err := cli.Get(ctx, types.NamespacedName{Namespace: gw.Namespace, Name: gw.Name}, gw); err != nil {
+			return false
+		}
+
+		for _, cond := range gw.Status.Conditions {
+			fmt.Printf("Condition: %v", cond)
+			if cond.Type == string(gwapiv1b1.GatewayConditionReady) && cond.Status == metav1.ConditionTrue {
+				ready = true
+			}
+		}
+
+		if gw.Status.Addresses != nil {
+			hasAddress = len(gw.Status.Addresses) >= 1
+		}
+
+		return ready && hasAddress
+	}, defaultWait, defaultTick)
+
+	defer func() {
+		require.NoError(t, cli.Delete(ctx, gw))
+	}()
+
+	// Ensure the gatewayclass has been finalized.
+	require.NoError(t, cli.Get(ctx, types.NamespacedName{Name: gc.Name}, gc))
+	require.Contains(t, gc.Finalizers, gatewayClassFinalizer)
+
+	// Ensure the number of Gateways in the Gateway resource table is as expected.
+	require.Eventually(t, func() bool {
+		return resources.Gateways.Len() == 1
+	}, defaultWait, defaultTick)
+
+	// Ensure the test Gateway in the Gateway resources is as expected.
+	key := types.NamespacedName{
+		Namespace: gw.Namespace,
+		Name:      gw.Name,
+	}
+	require.Eventually(t, func() bool {
+		return cli.Get(ctx, key, gw) == nil
+	}, defaultWait, defaultTick)
+	gws, _ := resources.Gateways.Load(key)
+	// Only check if the spec is equal
+	// The watchable map will not store a resource
+	// with an updated status if the spec has not changed
+	// to eliminate this endless loop:
+	// reconcile->store->translate->update-status->reconcile
+	assert.Equal(t, gw.Spec, gws.Spec)
+}
+
 func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resources *message.ProviderResources) {
 	cli := provider.manager.GetClient()
 

internal/provider/utils/utils.go

@@ -1,6 +1,10 @@
 package utils
 
 import (
+	"crypto/sha256"
+	"fmt"
+	"strings"
+
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -12,3 +16,16 @@ func NamespacedName(obj client.Object) types.NamespacedName {
 		Name:      obj.GetName(),
 	}
 }
+
+// Returns a partially hashed name for the string including up to 48 characters of the original name before the hash
+func GetHashedName(name string) string {
+
+	h := sha256.New() // Using sha256 instead of sha1 due to Blocklisted import crypto/sha1: weak cryptographic primitive (gosec)
+	hsha := h.Sum([]byte(name))
+	hashedName := strings.ToLower(fmt.Sprintf("%x", hsha))
+
+	if len(name) > 48 {
+		return fmt.Sprintf("%s-%s", name[0:48], hashedName[0:8])
+	}
+	return fmt.Sprintf("%s-%s", name, hashedName[0:8])
+}