mesh: ensure that xRoutes have ParentRefs that have matching Tenancy …

…to the enclosing resource (#19176) We don't want an xRoute controlling traffic for a Service in another tenancy.
hashicorp · Oct 13, 2023 · f0e4897 · f0e4897
1 parent 5fbf0c0
commit f0e4897
Show file tree

Hide file tree

Showing 11 changed files with 486 additions and 342 deletions.
diff --git a/internal/mesh/internal/types/destinations_test.go b/internal/mesh/internal/types/destinations_test.go
@@ -61,19 +61,19 @@ func TestMutateUpstreams(t *testing.T) {
 			},
 		},
 		"dest ref tenancy defaulting": {
-			tenancy: newTestTenancy("foo.bar"),
+			tenancy: resourcetest.Tenancy("foo.bar"),
 			data: &pbmesh.Destinations{
 				Destinations: []*pbmesh.Destination{
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy(""), "api")},
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy(".zim"), "api")},
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy("gir.zim"), "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, "", "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, ".zim", "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, "gir.zim", "api")},
 				},
 			},
 			expect: &pbmesh.Destinations{
 				Destinations: []*pbmesh.Destination{
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy("foo.bar"), "api")},
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy("foo.zim"), "api")},
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy("gir.zim"), "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, "foo.bar", "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, "foo.zim", "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, "gir.zim", "api")},
 				},
 			},
 		},
@@ -138,7 +138,7 @@ func TestValidateUpstreams(t *testing.T) {
 			skipMutate: true,
 			data: &pbmesh.Destinations{
 				Destinations: []*pbmesh.Destination{
-					{DestinationRef: newRefWithTenancy(pbcatalog.WorkloadType, nil, "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.WorkloadType, "default.default", "api")},
 				},
 			},
 			expectErr: `invalid element at index 0 of list "upstreams": invalid "destination_ref" field: invalid "type" field: reference must have type catalog.v2beta1.Service`,
@@ -156,7 +156,7 @@ func TestValidateUpstreams(t *testing.T) {
 			skipMutate: true,
 			data: &pbmesh.Destinations{
 				Destinations: []*pbmesh.Destination{
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy(".bar"), "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, ".bar", "api")},
 				},
 			},
 			expectErr: `invalid element at index 0 of list "upstreams": invalid "destination_ref" field: invalid "tenancy" field: invalid "partition" field: cannot be empty`,
@@ -165,7 +165,7 @@ func TestValidateUpstreams(t *testing.T) {
 			skipMutate: true,
 			data: &pbmesh.Destinations{
 				Destinations: []*pbmesh.Destination{
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy("foo"), "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, "foo", "api")},
 				},
 			},
 			expectErr: `invalid element at index 0 of list "upstreams": invalid "destination_ref" field: invalid "tenancy" field: invalid "namespace" field: cannot be empty`,
@@ -174,17 +174,19 @@ func TestValidateUpstreams(t *testing.T) {
 			skipMutate: true,
 			data: &pbmesh.Destinations{
 				Destinations: []*pbmesh.Destination{
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, &pbresource.Tenancy{Partition: "foo", Namespace: "bar"}, "api")},
+					{DestinationRef: resourcetest.Resource(pbcatalog.ServiceType, "api").
+						WithTenancy(&pbresource.Tenancy{Partition: "foo", Namespace: "bar"}).
+						Reference("")},
 				},
 			},
 			expectErr: `invalid element at index 0 of list "upstreams": invalid "destination_ref" field: invalid "tenancy" field: invalid "peer_name" field: must be set to "local"`,
 		},
 		"normal": {
 			data: &pbmesh.Destinations{
 				Destinations: []*pbmesh.Destination{
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy("foo.bar"), "api")},
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy("foo.zim"), "api")},
-					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, newTestTenancy("gir.zim"), "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, "foo.bar", "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, "foo.zim", "api")},
+					{DestinationRef: newRefWithTenancy(pbcatalog.ServiceType, "gir.zim", "api")},
 				},
 			},
 		},

diff --git a/internal/mesh/internal/types/grpc_route.go b/internal/mesh/internal/types/grpc_route.go
@@ -64,7 +64,7 @@ func ValidateGRPCRoute(res *pbresource.Resource) error {
 	}
 
 	var merr error
-	if err := validateParentRefs(route.ParentRefs); err != nil {
+	if err := validateParentRefs(res.Id, route.ParentRefs); err != nil {
 		merr = multierror.Append(merr, err)
 	}
 

diff --git a/internal/mesh/internal/types/grpc_route_test.go b/internal/mesh/internal/types/grpc_route_test.go
@@ -81,6 +81,7 @@ func TestMutateGRPCRoute(t *testing.T) {
 			WithTenancy(tc.routeTenancy).
 			WithData(t, tc.route).
 			Build()
+		resource.DefaultIDTenancy(res.Id, nil, resource.DefaultNamespacedTenancy())
 
 		err := MutateGRPCRoute(res)
 		require.NoError(t, err)
@@ -103,14 +104,17 @@ func TestMutateGRPCRoute(t *testing.T) {
 
 func TestValidateGRPCRoute(t *testing.T) {
 	type testcase struct {
-		route     *pbmesh.GRPCRoute
-		expectErr string
+		routeTenancy *pbresource.Tenancy
+		route        *pbmesh.GRPCRoute
+		expectErr    string
 	}
 
 	run := func(t *testing.T, tc testcase) {
 		res := resourcetest.Resource(pbmesh.GRPCRouteType, "api").
+			WithTenancy(tc.routeTenancy).
 			WithData(t, tc.route).
 			Build()
+		resource.DefaultIDTenancy(res.Id, nil, resource.DefaultNamespacedTenancy())
 
 		// Ensure things are properly mutated and updated in the inputs.
 		err := MutateGRPCRoute(res)
@@ -582,6 +586,7 @@ func TestValidateGRPCRoute(t *testing.T) {
 	// Add common parent refs test cases.
 	for name, parentTC := range getXRouteParentRefTestCases() {
 		cases["parent-ref: "+name] = testcase{
+			routeTenancy: parentTC.routeTenancy,
 			route: &pbmesh.GRPCRoute{
 				ParentRefs: parentTC.refs,
 			},
@@ -597,6 +602,7 @@ func TestValidateGRPCRoute(t *testing.T) {
 			})
 		}
 		cases["backend-ref: "+name] = testcase{
+			routeTenancy: backendTC.routeTenancy,
 			route: &pbmesh.GRPCRoute{
 				ParentRefs: []*pbmesh.ParentReference{
 					newParentRef(pbcatalog.ServiceType, "web", ""),

diff --git a/internal/mesh/internal/types/http_route.go b/internal/mesh/internal/types/http_route.go
@@ -75,7 +75,7 @@ func ValidateHTTPRoute(res *pbresource.Resource) error {
 	}
 
 	var merr error
-	if err := validateParentRefs(route.ParentRefs); err != nil {
+	if err := validateParentRefs(res.Id, route.ParentRefs); err != nil {
 		merr = multierror.Append(merr, err)
 	}