more PR feedback

internal/mesh/internal/controllers/register.go

@@ -4,19 +4,14 @@
 package controllers
 
 import (
-	"github.com/hashicorp/consul/internal/mesh/internal/controllers/sidecar-proxy/cache"
-	"github.com/hashicorp/consul/internal/mesh/internal/controllers/sidecar-proxy/mapper"
-
 	"github.com/hashicorp/consul/internal/catalog"
 	"github.com/hashicorp/consul/internal/controller"
 	"github.com/hashicorp/consul/internal/mesh/internal/cache/sidecarproxycache"
 	"github.com/hashicorp/consul/internal/mesh/internal/controllers/sidecarproxy"
-	"github.com/hashicorp/consul/internal/mesh/internal/controllers/sidecarproxy/cache"
-	"github.com/hashicorp/consul/internal/mesh/internal/controllers/sidecarproxy/mapper"
 	"github.com/hashicorp/consul/internal/mesh/internal/controllers/xds"
+	"github.com/hashicorp/consul/internal/mesh/internal/mappers/sidecarproxymapper"
 	"github.com/hashicorp/consul/internal/mesh/internal/types"
 	"github.com/hashicorp/consul/internal/resource/mappers/bimapper"
-	"github.com/hashicorp/consul/internal/mesh/internal/mappers/sidecarproxymapper"
 )
 
 type Dependencies struct {

internal/mesh/internal/controllers/sidecarproxy/builder/builder_test.go

@@ -3,18 +3,13 @@ package builder
 import (
 	"flag"
 	"os"
-	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
 )
 
-var (
-	update = flag.Bool("update", false, "update the golden files of this test")
-)
-
 func TestMain(m *testing.M) {
 	flag.Parse()
 	os.Exit(m.Run())
@@ -29,19 +24,3 @@ func protoToJSON(t *testing.T, pb proto.Message) string {
 	require.NoError(t, err)
 	return string(gotJSON)
 }
-
-func goldenValue(t *testing.T, goldenFile string, actual string, update bool) string {
-	t.Helper()
-	goldenPath := filepath.Join("testdata", goldenFile) + ".golden"
-
-	if update {
-		err := os.WriteFile(goldenPath, []byte(actual), 0644)
-		require.NoError(t, err)
-
-		return actual
-	}
-
-	content, err := os.ReadFile(goldenPath)
-	require.NoError(t, err)
-	return string(content)
-}

internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder.go

@@ -1,3 +1,6 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
 package builder
 
 import (

internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder_test.go

@@ -3,14 +3,16 @@ package builder
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/hashicorp/consul/internal/catalog"
 	"github.com/hashicorp/consul/internal/mesh/internal/types/intermediate"
 	"github.com/hashicorp/consul/internal/resource"
 	"github.com/hashicorp/consul/internal/resource/resourcetest"
+	"github.com/hashicorp/consul/internal/testing/golden"
 	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
 	pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v1alpha1"
 	"github.com/hashicorp/consul/proto-public/pbresource"
-	"github.com/stretchr/testify/require"
 )
 
 var (
@@ -32,10 +34,10 @@ var (
 
 func TestBuildExplicitDestinations(t *testing.T) {
 	api1Endpoints := resourcetest.Resource(catalog.ServiceEndpointsType, "api-1").
-		WithData(t, endpointsData).Build()
+		WithData(t, endpointsData).WithTenancy(resource.DefaultNamespacedTenancy()).Build()
 
 	api2Endpoints := resourcetest.Resource(catalog.ServiceEndpointsType, "api-2").
-		WithData(t, endpointsData).Build()
+		WithData(t, endpointsData).WithTenancy(resource.DefaultNamespacedTenancy()).Build()
 
 	api1Identity := &pbresource.Reference{
 		Name:    "api1-identity",
@@ -99,7 +101,7 @@ func TestBuildExplicitDestinations(t *testing.T) {
 			Build()
 
 		actual := protoToJSON(t, proxyTmpl)
-		expected := goldenValue(t, name, actual, *update)
+		expected := golden.Get(t, actual, name+".golden")
 
 		require.JSONEq(t, expected, actual)
 	}

internal/mesh/internal/controllers/sidecarproxy/builder/local_app.go

@@ -1,3 +1,6 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
 package builder
 
 import (

internal/mesh/internal/controllers/sidecarproxy/builder/local_app_test.go

@@ -3,11 +3,14 @@ package builder
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/hashicorp/consul/internal/mesh/internal/types"
+	"github.com/hashicorp/consul/internal/resource"
 	"github.com/hashicorp/consul/internal/resource/resourcetest"
+	"github.com/hashicorp/consul/internal/testing/golden"
 	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
 	"github.com/hashicorp/consul/proto-public/pbresource"
-	"github.com/stretchr/testify/require"
 )
 
 func TestBuildLocalApp(t *testing.T) {
@@ -68,15 +71,17 @@ func TestBuildLocalApp(t *testing.T) {
 			proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), "foo.consul").BuildLocalApp(c.workload).
 				Build()
 			actual := protoToJSON(t, proxyTmpl)
-			expected := goldenValue(t, name, actual, *update)
+			expected := golden.Get(t, actual, name+".golden")
 
 			require.JSONEq(t, expected, actual)
 		})
 	}
 }
 
 func testProxyStateTemplateID() *pbresource.ID {
-	return resourcetest.Resource(types.ProxyStateTemplateType, "test").ID()
+	return resourcetest.Resource(types.ProxyStateTemplateType, "test").
+		WithTenancy(resource.DefaultNamespacedTenancy()).
+		ID()
 }
 
 func testIdentityRef() *pbresource.Reference {

internal/mesh/internal/controllers/sidecarproxy/controller.go

@@ -1,11 +1,14 @@
 // Copyright (c) HashiCorp, Inc.
-// SPDX-License-Identifier: MPL-2.0
+// SPDX-License-Identifier: BUSL-1.1
 
 package sidecarproxy
 
 import (
 	"context"
 
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/anypb"
+
 	"github.com/hashicorp/consul/internal/catalog"
 	"github.com/hashicorp/consul/internal/controller"
 	"github.com/hashicorp/consul/internal/mesh/internal/cache/sidecarproxycache"
@@ -16,8 +19,6 @@ import (
 	"github.com/hashicorp/consul/internal/mesh/internal/types/intermediate"
 	"github.com/hashicorp/consul/internal/resource"
 	"github.com/hashicorp/consul/proto-public/pbresource"
-	"google.golang.org/protobuf/proto"
-	"google.golang.org/protobuf/types/known/anypb"
 )
 
 // ControllerName is the name for this controller. It's used for logging or status keys.
@@ -44,14 +45,14 @@ type reconciler struct {
 func (r *reconciler) Reconcile(ctx context.Context, rt controller.Runtime, req controller.Request) error {
 	rt.Logger = rt.Logger.With("resource-id", req.ID, "controller", ControllerName)
 
-	rt.Logger.Trace("reconciling proxy state template", "id", req.ID)
+	rt.Logger.Trace("reconciling proxy state template")
 
 	// Instantiate a data fetcher to fetch all reconciliation data.
 	dataFetcher := fetcher.Fetcher{Client: rt.Client, Cache: r.cache}
 
 	// Check if the workload exists.
 	workloadID := resource.ReplaceType(catalog.WorkloadType, req.ID)
-	workload, err := dataFetcher.FetchWorkload(ctx, resource.ReplaceType(catalog.WorkloadType, req.ID))
+	workload, err := dataFetcher.FetchWorkload(ctx, workloadID)
 	if err != nil {
 		rt.Logger.Error("error reading the associated workload", "error", err)
 		return err
@@ -71,15 +72,15 @@ func (r *reconciler) Reconcile(ctx context.Context, rt controller.Runtime, req c
 
 	if proxyStateTemplate == nil {
 		// If proxy state template has been deleted, we will need to generate a new one.
-		rt.Logger.Trace("proxy state template for this workload doesn't yet exist; generating a new one", "id", req.ID)
+		rt.Logger.Trace("proxy state template for this workload doesn't yet exist; generating a new one")
 	}
 
 	if !workload.Workload.IsMeshEnabled() {
 		// Skip non-mesh workloads.
 
 		// If there's existing proxy state template, delete it.
 		if proxyStateTemplate != nil {
-			rt.Logger.Trace("deleting existing proxy state template because workload is no longer on the mesh", "id", req.ID)
+			rt.Logger.Trace("deleting existing proxy state template because workload is no longer on the mesh")
 			_, err = rt.Client.Delete(ctx, &pbresource.DeleteRequest{Id: req.ID})
 			if err != nil {
 				rt.Logger.Error("error deleting existing proxy state template", "error", err)
@@ -107,7 +108,7 @@ func (r *reconciler) Reconcile(ctx context.Context, rt controller.Runtime, req c
 	destinationsRefs := r.cache.DestinationsBySourceProxy(req.ID)
 	destinationsData, statuses, err := dataFetcher.FetchDestinationsData(ctx, destinationsRefs)
 	if err != nil {
-		rt.Logger.Error("error fetching destinations for this proxy", "id", req.ID, "error", err)
+		rt.Logger.Error("error fetching destinations for this proxy", "error", err)
 		return err
 	}
 
@@ -121,7 +122,7 @@ func (r *reconciler) Reconcile(ctx context.Context, rt controller.Runtime, req c
 			rt.Logger.Error("error creating proxy state template data", "error", err)
 			return err
 		}
-		rt.Logger.Trace("updating proxy state template", "id", req.ID)
+		rt.Logger.Trace("updating proxy state template")
 		_, err = rt.Client.Write(ctx, &pbresource.WriteRequest{
 			Resource: &pbresource.Resource{
 				Id:    req.ID,
@@ -134,7 +135,7 @@ func (r *reconciler) Reconcile(ctx context.Context, rt controller.Runtime, req c
 			return err
 		}
 	} else {
-		rt.Logger.Trace("proxy state template data has not changed, skipping update", "id", req.ID)
+		rt.Logger.Trace("proxy state template data has not changed, skipping update")
 	}
 
 	// Update any statuses.

internal/mesh/internal/controllers/sidecarproxy/fetcher/data_fetcher_test.go

@@ -4,6 +4,11 @@ import (
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+
 	svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing"
 	"github.com/hashicorp/consul/internal/catalog"
 	"github.com/hashicorp/consul/internal/controller"
@@ -18,10 +23,6 @@ import (
 	"github.com/hashicorp/consul/proto-public/pbresource"
 	"github.com/hashicorp/consul/proto/private/prototest"
 	"github.com/hashicorp/consul/sdk/testutil"
-	"github.com/stretchr/testify/require"
-	"github.com/stretchr/testify/suite"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 func TestIsMeshEnabled(t *testing.T) {
@@ -368,7 +369,9 @@ func (suite *dataFetcherSuite) TestFetcher_FetchDestinationsData() {
 	})
 
 	suite.T().Run("service endpoints not found", func(t *testing.T) {
-		notFoundServiceRef := resourcetest.Resource(catalog.ServiceType, "not-found").ReferenceNoSection()
+		notFoundServiceRef := resourcetest.Resource(catalog.ServiceType, "not-found").
+			WithTenancy(resource.DefaultNamespacedTenancy()).
+			ReferenceNoSection()
 		destinationNoServiceEndpoints := intermediate.CombinedDestinationRef{
 			ServiceRef:             notFoundServiceRef,
 			Port:                   "tcp",

internal/mesh/internal/mappers/sidecarproxymapper/service_endpoints_mapper_test.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/hashicorp/consul/internal/catalog"
 	"github.com/hashicorp/consul/internal/controller"
 	"github.com/hashicorp/consul/internal/mesh/internal/cache/sidecarproxycache"
@@ -13,12 +15,13 @@ import (
 	"github.com/hashicorp/consul/internal/resource/resourcetest"
 	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
 	"github.com/hashicorp/consul/proto/private/prototest"
-	"github.com/stretchr/testify/require"
 )
 
 func TestMapServiceEndpointsToProxyStateTemplate(t *testing.T) {
-	workload1 := resourcetest.Resource(catalog.WorkloadType, "workload-1").Build()
-	workload2 := resourcetest.Resource(catalog.WorkloadType, "workload-2").Build()
+	workload1 := resourcetest.Resource(catalog.WorkloadType, "workload-1").
+		WithTenancy(resource.DefaultNamespacedTenancy()).Build()
+	workload2 := resourcetest.Resource(catalog.WorkloadType, "workload-2").
+		WithTenancy(resource.DefaultNamespacedTenancy()).Build()
 	serviceEndpoints := resourcetest.Resource(catalog.ServiceEndpointsType, "service").
 		WithData(t, &pbcatalog.ServiceEndpoints{
 			Endpoints: []*pbcatalog.Endpoint{
@@ -39,15 +42,22 @@ func TestMapServiceEndpointsToProxyStateTemplate(t *testing.T) {
 					},
 				},
 			},
-		}).Build()
-	proxyTmpl1ID := resourcetest.Resource(types.ProxyStateTemplateType, "workload-1").ID()
-	proxyTmpl2ID := resourcetest.Resource(types.ProxyStateTemplateType, "workload-2").ID()
+		}).
+		WithTenancy(resource.DefaultNamespacedTenancy()).
+		Build()
+	proxyTmpl1ID := resourcetest.Resource(types.ProxyStateTemplateType, "workload-1").
+		WithTenancy(resource.DefaultNamespacedTenancy()).ID()
+	proxyTmpl2ID := resourcetest.Resource(types.ProxyStateTemplateType, "workload-2").
+		WithTenancy(resource.DefaultNamespacedTenancy()).ID()
 
 	c := sidecarproxycache.New()
 	mapper := &Mapper{cache: c}
-	sourceProxy1 := resourcetest.Resource(types.ProxyStateTemplateType, "workload-3").ID()
-	sourceProxy2 := resourcetest.Resource(types.ProxyStateTemplateType, "workload-4").ID()
-	sourceProxy3 := resourcetest.Resource(types.ProxyStateTemplateType, "workload-5").ID()
+	sourceProxy1 := resourcetest.Resource(types.ProxyStateTemplateType, "workload-3").
+		WithTenancy(resource.DefaultNamespacedTenancy()).ID()
+	sourceProxy2 := resourcetest.Resource(types.ProxyStateTemplateType, "workload-4").
+		WithTenancy(resource.DefaultNamespacedTenancy()).ID()
+	sourceProxy3 := resourcetest.Resource(types.ProxyStateTemplateType, "workload-5").
+		WithTenancy(resource.DefaultNamespacedTenancy()).ID()
 	destination1 := intermediate.CombinedDestinationRef{
 		ServiceRef: resourcetest.Resource(catalog.ServiceType, "service").ReferenceNoSection(),
 		Port:       "tcp1",

internal/resource/mappers/bimapper/bimapper.go

@@ -215,30 +215,6 @@ func (m *Mapper) LinkIDsForItem(item *pbresource.ID) []*pbresource.ID {
 	return out
 }
 
-// LinkIDsForItem returns IDs to links related to the requested item.
-func (m *Mapper) LinkIDsForItem(item *pbresource.ID) []*pbresource.ID {
-	if !resource.EqualType(item.Type, m.itemType) {
-		panic(fmt.Sprintf("expected item type %q got %q",
-			resource.TypeToString(m.itemType),
-			resource.TypeToString(item.Type),
-		))
-	}
-
-	m.lock.Lock()
-	defer m.lock.Unlock()
-
-	links, ok := m.itemToLink[resource.NewReferenceKey(item)]
-	if !ok {
-		return nil
-	}
-
-	out := make([]*pbresource.ID, 0, len(links))
-	for l := range links {
-		out = append(out, l.ToID())
-	}
-	return out
-}
-
 // ItemsForLink returns item ids for items related to the provided link.
 // Deprecated: use ItemIDsForLink
 func (m *Mapper) ItemsForLink(link *pbresource.ID) []*pbresource.ID {