pr feedback

projects/gateway/pkg/syncer/translator_syncer.go

@@ -52,7 +52,7 @@ var (
 	// Previously, proxies would be identified with:
 	//   created_by: gateway
 	// Now, proxies are identified with:
-	//   proxy_type: gloo-gateway
+	//   created_by: gloo-gateway
 	//
 	// We need to ensure that users can successfully upgrade from versions
 	// where the previous labels were used, to versions with the new labels.

projects/gateway2/proxy_syncer/proxy_syncer.go

@@ -61,6 +61,11 @@ func NewGatewayInputChannels() *GatewayInputChannels {
 	}
 }
 
+/*
+ProxySyncer is responsible for translating Gateway CRs into Gloo Proxies and syncing the proxyClient with
+the newly translated proxies. The proxy sync is triggered by the `genericEvent` which is kicked when
+we reconcile gateway in the gateway controller. The `secretEvent` is kicked when a secret is created, updated,
+*/
 func NewProxySyncer(
 	controllerName, writeNamespace string,
 	translator translator.Translator,

projects/gateway2/translator/gateway_translator.go

@@ -91,8 +91,10 @@ func proxyMetadata(gateway *gwv1.Gateway, writeNamespace string) *core.Metadata
 	// which is equal to role defined on proxy-deployment ConfigMap:
 	// gloo-kube-api~{{ .Release.Namespace }}~{{ $gateway.gatewayNamespace }}-{{ $gateway.gatewayName | default (include "gloo-gateway.gateway.fullname" .) }}
 	return &core.Metadata{
-		Name:      fmt.Sprintf("%s-%s", gateway.GetNamespace(), gateway.GetName()),
-		Namespace: writeNamespace,
+		// Add the gateway name to the proxy name to ensure uniqueness of proxies
+		Name: fmt.Sprintf("%s-%s", gateway.GetNamespace(), gateway.GetName()),
+		// All proxies are created in the writeNamespace (ie. gloo-system).
+		// This needs to match the writeNamespace because the proxyClient will only looksat namespaces in the whitelisted namespace list
 		Labels: map[string]string{
 			utils.ProxyTypeKey:   utils.GlooGatewayProxyValue,
 			utils.NamespaceLabel: gateway.GetNamespace(),

projects/gloo/pkg/syncer/envoy_translator_syncer.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/gorilla/mux"
 	"github.com/solo-io/gloo/pkg/utils/syncutil"
+	gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
 	v1snap "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/gloosnapshot"
 	"github.com/solo-io/gloo/projects/gloo/pkg/plugins"
 	syncerstats "github.com/solo-io/gloo/projects/gloo/pkg/syncer/stats"
@@ -121,16 +122,7 @@ func (s *translatorSyncer) syncEnvoy(ctx context.Context, snap *v1snap.ApiSnapsh
 	}
 	for _, proxy := range snap.Proxies {
 		proxyCtx := ctx
-		meta := proxy.GetMetadata()
-		metaKey := meta.Ref().Key()
-		labels := proxy.GetMetadata().GetLabels()
-		if labels != nil && labels[utils.ProxyTypeKey] == utils.GlooGatewayProxyValue {
-			proxyNamespace := labels[utils.NamespaceLabel]
-			if proxyNamespace != "" {
-				meta.Namespace = proxyNamespace
-				metaKey = meta.Ref().Key()
-			}
-		}
+		metaKey := GetKeyFromProxyMeta(proxy)
 		if ctxWithTags, err := tag.New(proxyCtx, tag.Insert(syncerstats.ProxyNameKey, metaKey)); err == nil {
 			proxyCtx = ctxWithTags
 		}
@@ -222,3 +214,17 @@ func prettify(original interface{}) string {
 
 	return string(b)
 }
+
+func GetKeyFromProxyMeta(proxy *gloov1.Proxy) string {
+	meta := proxy.GetMetadata()
+	metaKey := meta.Ref().Key()
+	labels := proxy.GetMetadata().GetLabels()
+	if labels != nil && labels[utils.ProxyTypeKey] == utils.GlooGatewayProxyValue {
+		proxyNamespace := labels[utils.NamespaceLabel]
+		if proxyNamespace != "" {
+			meta.Namespace = proxyNamespace
+			metaKey = meta.Ref().Key()
+		}
+	}
+	return metaKey
+}

projects/gloo/pkg/syncer/translator_syncer_test.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 
 	"github.com/solo-io/gloo/pkg/bootstrap/leaderelector/singlereplica"
+	gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
+	"github.com/solo-io/gloo/projects/gloo/pkg/utils"
 
 	gloo_translator "github.com/solo-io/gloo/projects/gloo/pkg/translator"
 
@@ -334,6 +336,68 @@ var _ = Describe("Translate multiple proxies with errors", func() {
 
 		Expect(xdsCache.Called).To(BeTrue())
 	})
+
+	Describe("correctl returns metaKey for proxies", func() {
+		It("returns the correct key", func() {
+			// Create a proxy with metadata and labels
+			proxy := &gloov1.Proxy{
+				Metadata: &core.Metadata{
+					Name:      "test-proxy",
+					Namespace: "test-namespace",
+					Labels: map[string]string{
+						utils.ProxyTypeKey:   utils.GlooGatewayProxyValue,
+						utils.NamespaceLabel: "custom-namespace",
+					},
+				},
+			}
+
+			key := GetKeyFromProxyMeta(proxy)
+
+			// Assert that the returned key matches the expected value
+			expectedKey := "custom-namespace.test-proxy"
+			Expect(key).To(Equal(expectedKey))
+		})
+
+		It("returns the correct key when proxy has no custom namespace label", func() {
+
+			// Create a proxy with metadata and labels
+			proxy := &gloov1.Proxy{
+				Metadata: &core.Metadata{
+					Name:      "test-proxy",
+					Namespace: "test-namespace",
+					Labels: map[string]string{
+						utils.ProxyTypeKey: utils.GlooGatewayProxyValue,
+					},
+				},
+			}
+
+			key := GetKeyFromProxyMeta(proxy)
+
+			// Assert that the returned key matches the expected value
+			expectedKey := "test-namespace.test-proxy"
+			Expect(key).To(Equal(expectedKey))
+		})
+
+		It("returns the correct key when proxy is not a Gloo gateway proxy", func() {
+			// Create a proxy with metadata and labels
+			proxy := &gloov1.Proxy{
+				Metadata: &core.Metadata{
+					Name:      "test-proxy",
+					Namespace: "test-namespace",
+					Labels: map[string]string{
+						utils.ProxyTypeKey:   "some-other-value",
+						utils.NamespaceLabel: "custom-namespace",
+					},
+				},
+			}
+
+			key := GetKeyFromProxyMeta(proxy)
+
+			// Assert that the returned key matches the expected value
+			expectedKey := "test-namespace.test-proxy"
+			Expect(key).To(Equal(expectedKey))
+		})
+	})
 })
 
 type mockTranslator struct {

projects/gloo/pkg/xds/cache.go

@@ -15,7 +15,7 @@ func SnapshotCacheKey(proxy *v1.Proxy) string {
 	namespace, name := proxy.GetMetadata().Ref().Strings()
 	owner := proxy.GetMetadata().GetLabels()[utils.ProxyTypeKey]
 	if owner == utils.GlooGatewayProxyValue {
-		// Gloo Gateway proxies can live in different namespaces from writeNamespace
+		// If namespace label is not set, default to proxy label for backwards compatability
 		namespaceLabel := proxy.GetMetadata().GetLabels()[utils.NamespaceLabel]
 		if namespaceLabel != "" {
 			namespace = namespaceLabel

projects/gloo/pkg/xds/node_hash.go

@@ -33,7 +33,7 @@ func NewNodeRoleHasher() *nodeRoleHasher {
 	return &nodeRoleHasher{}
 }
 
-// classicEdgeNodeHash identifies a node based on the values provided in the `node.metadata.role`
+// nodeRoleHasher identifies a node based on the values provided in the `node.metadata.role`
 type nodeRoleHasher struct{}
 
 func (h *nodeRoleHasher) ID(node *envoy_config_core_v3.Node) string {
@@ -46,15 +46,3 @@ func (h *nodeRoleHasher) ID(node *envoy_config_core_v3.Node) string {
 
 	return FallbackNodeCacheKey
 }
-
-// isProxyWorkloadRole returns true if the provided role fits the format that is used by Proxy workloads
-// This format is: "NAMESPACE~NAME".
-// In classic Edge, nodes could also identify themselves with a node.metadata.role that do not fit this structure
-// This is useful in the case of additional services (like Enterprise ext-auth-service, and rate-limit-service)
-// who follow a format of "NAME".
-func (c nodeRoleHasher) isProxyWorkloadRole(role string) bool {
-	if strings.Contains(role, KeyDelimiter) {
-		return true
-	}
-	return false
-}

test/kube2e/gateway/gateway_test.go

@@ -214,7 +214,7 @@ var _ = Describe("Kube2e: gateway", func() {
 				func(resource resources.Resource) resources.Resource {
 					proxy := resource.(*gloov1.Proxy)
 					proxy.Metadata.Labels = map[string]string{
-						utils.ProxyTypeKey: "gateway",
+						"created_by": "gateway",
 					}
 					return proxy
 				},