use common plumbing

hashicorp · Oct 16, 2023 · 761090d · 761090d
1 parent 584b656
commit 761090d
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 66 deletions.
diff --git a/internal/mesh/internal/types/xroute_test.go b/internal/mesh/internal/types/xroute_test.go
@@ -12,7 +12,6 @@ import (
 	"google.golang.org/protobuf/types/known/durationpb"
 	"google.golang.org/protobuf/types/known/wrapperspb"
 
-	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/internal/resource"
 	"github.com/hashicorp/consul/internal/resource/resourcetest"
@@ -381,37 +380,12 @@ func testXRouteACLs[R XRouteData](t *testing.T, newRoute func(t *testing.T, pare
 		return res
 	}
 
-	type testcase struct {
-		res     *pbresource.Resource
-		rules   string
-		check   func(t *testing.T, authz acl.Authorizer, res *pbresource.Resource)
-		readOK  string
-		writeOK string
-	}
-
 	const (
-		DENY    = "deny"
-		ALLOW   = "allow"
-		DEFAULT = "default"
+		DENY    = resourcetest.DENY
+		ALLOW   = resourcetest.ALLOW
+		DEFAULT = resourcetest.DEFAULT
 	)
 
-	checkF := func(t *testing.T, name string, expect string, got error) {
-		switch expect {
-		case ALLOW:
-			if acl.IsErrPermissionDenied(got) {
-				t.Fatal(name + " should be allowed")
-			}
-		case DENY:
-			if !acl.IsErrPermissionDenied(got) {
-				t.Fatal(name + " should be denied")
-			}
-		case DEFAULT:
-			require.Nil(t, got, name+" expected fallthrough decision")
-		default:
-			t.Fatalf(name+" unexpected expectation: %q", expect)
-		}
-	}
-
 	serviceRef := func(tenancy, name string) *pbresource.Reference {
 		return newRefWithTenancy(pbcatalog.ServiceType, tenancy, name)
 	}
@@ -461,26 +435,9 @@ func testXRouteACLs[R XRouteData](t *testing.T, newRoute func(t *testing.T, pare
 		)
 	}
 
-	run := func(t *testing.T, name string, tc testcase) {
+	run := func(t *testing.T, name string, tc resourcetest.ACLTestCase) {
 		t.Run(name, func(t *testing.T) {
-			config := acl.Config{
-				WildcardName: structs.WildcardSpecifier,
-			}
-			authz, err := acl.NewAuthorizerFromRules(tc.rules, &config, nil)
-			require.NoError(t, err)
-			authz = acl.NewChainedAuthorizer([]acl.Authorizer{authz, acl.DenyAll()})
-
-			reg, ok := registry.Resolve(tc.res.Id.GetType())
-			require.True(t, ok)
-
-			authCtx := &acl.AuthorizerContext{} // unused
-
-			err = reg.ACLs.Read(authz, authCtx, tc.res.Id, nil)
-			require.ErrorIs(t, err, resource.ErrNeedResource, "read hook should require the data payload")
-
-			checkF(t, "read", tc.readOK, reg.ACLs.Read(authz, authCtx, tc.res.Id, tc.res))
-			checkF(t, "write", tc.writeOK, reg.ACLs.Write(authz, authCtx, tc.res))
-			checkF(t, "list", DEFAULT, reg.ACLs.List(authz, authCtx))
+			resourcetest.RunACLTestCase(t, tc, registry)
 		})
 	}
 
@@ -500,11 +457,13 @@ func testXRouteACLs[R XRouteData](t *testing.T, newRoute func(t *testing.T, pare
 	}
 
 	assert := func(t *testing.T, name string, rules string, res *pbresource.Resource, readOK, writeOK string) {
-		tc := testcase{
-			res:     res,
-			rules:   rules,
-			readOK:  readOK,
-			writeOK: writeOK,
+		tc := resourcetest.ACLTestCase{
+			Rules:                    rules,
+			Res:                      res,
+			ReadOK:                   readOK,
+			WriteOK:                  writeOK,
+			ListOK:                   DEFAULT,
+			ReadHookRequiresResource: true,
 		}
 		run(t, name, tc)
 	}

diff --git a/internal/resource/resourcetest/acls.go b/internal/resource/resourcetest/acls.go
@@ -39,27 +39,49 @@ var checkF = func(t *testing.T, expect string, got error) {
 }
 
 type ACLTestCase struct {
-	Rules   string
-	Data    protoreflect.ProtoMessage
-	Owner   *pbresource.ID
-	Typ     *pbresource.Type
+	Rules string
+
+	// One of either Res or Data/Owner/Typ should be set.
+	Res   *pbresource.Resource
+	Data  protoreflect.ProtoMessage
+	Owner *pbresource.ID
+	Typ   *pbresource.Type
+
 	ReadOK  string
 	WriteOK string
 	ListOK  string
+
+	ReadHookRequiresResource bool
 }
 
 func RunACLTestCase(t *testing.T, tc ACLTestCase, registry resource.Registry) {
-	reg, ok := registry.Resolve(tc.Typ)
-	require.True(t, ok)
+	var (
+		typ *pbresource.Type
+		res *pbresource.Resource
+	)
+	if tc.Res != nil {
+		require.Nil(t, tc.Data)
+		require.Nil(t, tc.Owner)
+		require.Nil(t, tc.Typ)
+		typ = tc.Res.Id.GetType()
+		res = tc.Res
+	} else {
+		require.NotNil(t, tc.Data)
+		require.NotNil(t, tc.Typ)
+		typ = tc.Typ
 
-	resolvedType, ok := registry.Resolve(tc.Typ)
-	require.True(t, ok)
+		resolvedType, ok := registry.Resolve(typ)
+		require.True(t, ok)
 
-	res := Resource(tc.Typ, "test").
-		WithTenancy(DefaultTenancyForType(t, resolvedType)).
-		WithOwner(tc.Owner).
-		WithData(t, tc.Data).
-		Build()
+		res = Resource(tc.Typ, "test").
+			WithTenancy(DefaultTenancyForType(t, resolvedType)).
+			WithOwner(tc.Owner).
+			WithData(t, tc.Data).
+			Build()
+	}
+
+	reg, ok := registry.Resolve(typ)
+	require.True(t, ok)
 
 	ValidateAndNormalize(t, registry, res)
 
@@ -70,6 +92,11 @@ func RunACLTestCase(t *testing.T, tc ACLTestCase, registry resource.Registry) {
 	require.NoError(t, err)
 	authz = acl.NewChainedAuthorizer([]acl.Authorizer{authz, acl.DenyAll()})
 
+	if tc.ReadHookRequiresResource {
+		err = reg.ACLs.Read(authz, &acl.AuthorizerContext{}, res.Id, nil)
+		require.ErrorIs(t, err, resource.ErrNeedResource, "read hook should require the data payload")
+	}
+
 	t.Run("read", func(t *testing.T) {
 		err := reg.ACLs.Read(authz, &acl.AuthorizerContext{}, res.Id, res)
 		checkF(t, tc.ReadOK, err)