agent: update proxy upstreams to inherit namespace from service (#10688)

.changelog/10688.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+connect: proxy upstreams inherit namespace from service if none are defined.
+```

agent/agent_endpoint_test.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/hashicorp/go-uuid"
 	"github.com/hashicorp/serf/serf"
+	"github.com/mitchellh/hashstructure"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/time/rate"
@@ -391,15 +392,14 @@ func TestAgent_Service(t *testing.T) {
 	// API struct types.
 	expectProxy := proxy
 	expectProxy.Upstreams =
-		structs.TestAddDefaultsToUpstreams(t, sidecarProxy.Proxy.Upstreams)
+		structs.TestAddDefaultsToUpstreams(t, sidecarProxy.Proxy.Upstreams, *structs.DefaultEnterpriseMetaInDefaultPartition())
 
 	expectedResponse := &api.AgentService{
-		Kind:        api.ServiceKindConnectProxy,
+		Kind:    api.ServiceKindConnectProxy,
-		ID:          "web-sidecar-proxy",
+		ID:      "web-sidecar-proxy",
-		Service:     "web-sidecar-proxy",
+		Service: "web-sidecar-proxy",
-		Port:        8000,
+		Port:    8000,
-		Proxy:       expectProxy.ToAPI(),
+		Proxy:   expectProxy.ToAPI(),
-		ContentHash: "854327a458fe02a6",
 		Weights: api.AgentWeights{
 			Passing: 1,
 			Warning: 1,
@@ -409,11 +409,21 @@ func TestAgent_Service(t *testing.T) {
 		Datacenter: "dc1",
 	}
 	fillAgentServiceEnterpriseMeta(expectedResponse, structs.DefaultEnterpriseMetaInDefaultPartition())
+	hash1, err := hashstructure.Hash(expectedResponse, nil)
+	if err != nil {
+		t.Fatalf("error generating hash: %v", err)
+	}
+	expectedResponse.ContentHash = fmt.Sprintf("%x", hash1)
 
 	// Copy and modify
 	updatedResponse := *expectedResponse
 	updatedResponse.Port = 9999
-	updatedResponse.ContentHash = "b80a4d9370ed1104"
+	updatedResponse.ContentHash = "" // clear field before generating a new hash
+	hash2, err := hashstructure.Hash(updatedResponse, nil)
+	if err != nil {
+		t.Fatalf("error generating hash: %v", err)
+	}
+	updatedResponse.ContentHash = fmt.Sprintf("%x", hash2)
 
 	// Simple response for non-proxy service registered in TestAgent config
 	expectWebResponse := &api.AgentService{
@@ -3537,8 +3547,18 @@ func testAgent_RegisterService_UnmanagedConnectProxy(t *testing.T, extraHCL stri
 	svc := a.State.Service(sid)
 	require.NotNil(t, svc, "has service")
 	require.Equal(t, structs.ServiceKindConnectProxy, svc.Kind)
-	// Registration must set that default type
+
-	args.Proxy.Upstreams[0].DestinationType = api.UpstreamDestTypeService
+	// Registration sets default types and namespaces
+	for i := range args.Proxy.Upstreams {
+		if args.Proxy.Upstreams[i].DestinationType == "" {
+			args.Proxy.Upstreams[i].DestinationType = api.UpstreamDestTypeService
+		}
+		if args.Proxy.Upstreams[i].DestinationNamespace == "" {
+			args.Proxy.Upstreams[i].DestinationNamespace =
+				structs.DefaultEnterpriseMetaInDefaultPartition().NamespaceOrEmpty()
+		}
+	}
+
 	require.Equal(t, args.Proxy, svc.Proxy.ToAPI())
 
 	// Ensure the token was configured

agent/config/runtime_test.go

@@ -5647,9 +5647,10 @@ func TestLoad_FullConfig(t *testing.T) {
 					},
 					Upstreams: structs.Upstreams{
 						{
-							DestinationType: "service", // Default should be explicitly filled
+							DestinationType:      "service", // Default should be explicitly filled
-							DestinationName: "KPtAj2cb",
+							DestinationName:      "KPtAj2cb",
-							LocalBindPort:   4051,
+							DestinationNamespace: defaultEntMeta.NamespaceOrEmpty(),
+							LocalBindPort:        4051,
 							Config: map[string]interface{}{
 								"kzRnZOyd": "nUNKoL8H",
 							},

agent/sidecar_service.go

@@ -18,7 +18,7 @@ func sidecarServiceID(serviceID string) string {
 // config.
 //
 // It assumes the ns has been validated already which means the nested
-// SidecarService is also already validated.It also assumes that any check
+// SidecarService is also already validated. It also assumes that any check
 // definitions within the sidecar service definition have been validated if
 // necessary. If no sidecar service is defined in ns, then nil is returned with
 // nil error.
@@ -35,6 +35,9 @@ func (a *Agent) sidecarServiceFromNodeService(ns *structs.NodeService, token str
 		return nil, nil, "", nil
 	}
 
+	// for now at least these must be identical
+	ns.Connect.SidecarService.EnterpriseMeta = ns.EnterpriseMeta
+
 	// Start with normal conversion from service definition
 	sidecar := ns.Connect.SidecarService.NodeService()
 
@@ -42,9 +45,6 @@ func (a *Agent) sidecarServiceFromNodeService(ns *structs.NodeService, token str
 	// ID. We rely on this for lifecycle management of the nested definition.
 	sidecar.ID = sidecarServiceID(ns.ID)
 
-	// for now at least these must be identical
-	sidecar.EnterpriseMeta = ns.EnterpriseMeta
-
 	// Set some meta we can use to disambiguate between service instances we added
 	// later and are responsible for deregistering.
 	if sidecar.Meta != nil {

agent/sidecar_service_test.go

@@ -129,7 +129,8 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) {
 					LocalServiceAddress:    "127.0.127.0",
 					LocalServicePort:       9999,
 					Config:                 map[string]interface{}{"baz": "qux"},
-					Upstreams:              structs.TestAddDefaultsToUpstreams(t, structs.TestUpstreams(t)),
+					Upstreams: structs.TestAddDefaultsToUpstreams(t, structs.TestUpstreams(t),
+						*structs.DefaultEnterpriseMetaInDefaultPartition()),
 				},
 			},
 			wantChecks: []*structs.CheckType{
@@ -308,7 +309,7 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Set port range to be tiny (one availabl) to test consuming all of it.
+			// Set port range to be tiny (one available) to test consuming all of it.
 			// This allows a single assigned port at 2222 thanks to being inclusive at
 			// both ends.
 			if tt.maxPort == 0 {

agent/structs/service_definition.go

@@ -80,11 +80,17 @@ func (s *ServiceDefinition) NodeService() *NodeService {
 	}
 	if s.Proxy != nil {
 		ns.Proxy = *s.Proxy
-		// Ensure the Upstream type is defaulted
 		for i := range ns.Proxy.Upstreams {
+			// Ensure the Upstream type is defaulted
 			if ns.Proxy.Upstreams[i].DestinationType == "" {
 				ns.Proxy.Upstreams[i].DestinationType = UpstreamDestTypeService
 			}
+
+			// If a proxy's namespace is not defined, inherit the server's namespace.
+			// Applicable only to Consul Enterprise.
+			if ns.Proxy.Upstreams[i].DestinationNamespace == "" {
+				ns.Proxy.Upstreams[i].DestinationNamespace = ns.EnterpriseMeta.NamespaceOrEmpty()
+			}
 		}
 		ns.Proxy.Expose = s.Proxy.Expose
 	}

agent/structs/testing_connect_proxy_config.go

@@ -42,9 +42,9 @@ func TestUpstreams(t testing.T) Upstreams {
 
 // TestAddDefaultsToUpstreams takes an array of upstreams (such as that from
 // TestUpstreams) and adds default values that are populated during
-// refigistration. Use this for generating the expected Upstreams value after
+// registration. Use this for generating the expected Upstreams value after
 // registration.
-func TestAddDefaultsToUpstreams(t testing.T, upstreams []Upstream) Upstreams {
+func TestAddDefaultsToUpstreams(t testing.T, upstreams []Upstream, entMeta EnterpriseMeta) Upstreams {
 	ups := make([]Upstream, len(upstreams))
 	for i := range upstreams {
 		ups[i] = upstreams[i]
@@ -52,6 +52,9 @@ func TestAddDefaultsToUpstreams(t testing.T, upstreams []Upstream) Upstreams {
 		if ups[i].DestinationType == "" {
 			ups[i].DestinationType = UpstreamDestTypeService
 		}
+		if ups[i].DestinationNamespace == "" {
+			ups[i].DestinationNamespace = entMeta.NamespaceOrEmpty()
+		}
 	}
 	return ups
 }