From e00e3a0bc3e55a5468bdf3415973db910e6e50b4 Mon Sep 17 00:00:00 2001 From: Dan Upton Date: Fri, 17 Jun 2022 10:24:43 +0100 Subject: [PATCH] Move ACLResolveResult into acl/resolver package (#13467) Having this type live in the agent/consul package makes it difficult to put anything that relies on token resolution (e.g. the new gRPC services) in separate packages without introducing import cycles. For example, if package foo imports agent/consul for the ACLResolveResult type it means that agent/consul cannot import foo to register its service. We've previously worked around this by wrapping the ACLResolver to "downgrade" its return type to an acl.Authorizer - aside from the added complexity, this also loses the resolved identity information. In the future, we may want to move the whole ACLResolver into the acl/resolver package. For now, putting the result type there at least, fixes the immediate import cycle issues. --- acl/resolver/result.go | 27 ++++++++ agent/acl_test.go | 9 +-- agent/agent.go | 3 +- agent/agent_endpoint_test.go | 5 +- agent/consul/acl.go | 68 ++++--------------- agent/consul/acl_endpoint.go | 3 +- agent/consul/acl_test.go | 9 +-- agent/consul/catalog_endpoint.go | 7 +- agent/consul/catalog_endpoint_test.go | 21 +++--- agent/consul/kvs_endpoint.go | 3 +- agent/consul/server.go | 6 +- agent/consul/txn_endpoint.go | 7 +- agent/delegate_mock_test.go | 5 +- .../services/connectca/mock_ACLResolver.go | 17 ++--- .../services/connectca/mock_CAManager.go | 5 +- .../grpc/public/services/connectca/server.go | 3 +- .../public/services/connectca/sign_test.go | 17 ++--- .../services/connectca/watch_roots_test.go | 5 +- .../get_envoy_boostrap_params_test.go | 18 ++--- .../dataplane/get_supported_features_test.go | 5 +- .../services/dataplane/mock_ACLResolver.go | 17 ++--- .../grpc/public/services/dataplane/server.go | 3 +- .../serverdiscovery/mock_ACLResolver.go | 26 +++++-- .../public/services/serverdiscovery/server.go | 5 +- .../serverdiscovery/watch_servers_test.go | 20 +++--- agent/grpc/public/testutils/acl.go | 21 ++++-- agent/local/state.go | 4 +- agent/local/state_test.go | 6 +- agent/structs/config_entry.go | 2 +- 29 files changed, 191 insertions(+), 156 deletions(-) create mode 100644 acl/resolver/result.go diff --git a/acl/resolver/result.go b/acl/resolver/result.go new file mode 100644 index 0000000000..abf8505688 --- /dev/null +++ b/acl/resolver/result.go @@ -0,0 +1,27 @@ +package resolver + +import ( + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/structs" +) + +type Result struct { + acl.Authorizer + // TODO: likely we can reduce this interface + ACLIdentity structs.ACLIdentity +} + +func (a Result) AccessorID() string { + if a.ACLIdentity == nil { + return "" + } + return a.ACLIdentity.ID() +} + +func (a Result) Identity() structs.ACLIdentity { + return a.ACLIdentity +} + +func (a Result) ToAllowAuthorizer() acl.AllowAuthorizer { + return acl.AllowAuthorizer{Authorizer: a, AccessorID: a.AccessorID()} +} diff --git a/agent/acl_test.go b/agent/acl_test.go index 3ed7ce325c..2e8664c9f2 100644 --- a/agent/acl_test.go +++ b/agent/acl_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/serf/serf" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/local" @@ -94,15 +95,15 @@ func (a *TestACLAgent) ResolveToken(secretID string) (acl.Authorizer, error) { return authz, err } -func (a *TestACLAgent) ResolveTokenAndDefaultMeta(secretID string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (consul.ACLResolveResult, error) { +func (a *TestACLAgent) ResolveTokenAndDefaultMeta(secretID string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (resolver.Result, error) { authz, err := a.ResolveToken(secretID) if err != nil { - return consul.ACLResolveResult{}, err + return resolver.Result{}, err } identity, err := a.resolveIdentFn(secretID) if err != nil { - return consul.ACLResolveResult{}, err + return resolver.Result{}, err } // Default the EnterpriseMeta based on the Tokens meta or actual defaults @@ -116,7 +117,7 @@ func (a *TestACLAgent) ResolveTokenAndDefaultMeta(secretID string, entMeta *acl. // Use the meta to fill in the ACL authorization context entMeta.FillAuthzContext(authzContext) - return consul.ACLResolveResult{Authorizer: authz, ACLIdentity: identity}, err + return resolver.Result{Authorizer: authz, ACLIdentity: identity}, err } // All of these are stubs to satisfy the interface diff --git a/agent/agent.go b/agent/agent.go index f0b345f951..d337f75f01 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -30,6 +30,7 @@ import ( "google.golang.org/grpc" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/ae" "github.com/hashicorp/consul/agent/cache" cachetype "github.com/hashicorp/consul/agent/cache-types" @@ -177,7 +178,7 @@ type delegate interface { // actions based on the permissions granted to the token. // If either entMeta or authzContext are non-nil they will be populated with the // default partition and namespace from the token. - ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (consul.ACLResolveResult, error) + ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (resolver.Result, error) RPC(method string, args interface{}, reply interface{}) error SnapshotRPC(args *structs.SnapshotRequest, in io.Reader, out io.Writer, replyFn structs.SnapshotReplyFn) error diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index b8b96f3f70..7bde623872 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -28,6 +28,7 @@ import ( "golang.org/x/time/rate" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/connect/ca" @@ -1645,8 +1646,8 @@ type fakeResolveTokenDelegate struct { authorizer acl.Authorizer } -func (f fakeResolveTokenDelegate) ResolveTokenAndDefaultMeta(_ string, _ *acl.EnterpriseMeta, _ *acl.AuthorizerContext) (consul.ACLResolveResult, error) { - return consul.ACLResolveResult{Authorizer: f.authorizer}, nil +func (f fakeResolveTokenDelegate) ResolveTokenAndDefaultMeta(_ string, _ *acl.EnterpriseMeta, _ *acl.AuthorizerContext) (resolver.Result, error) { + return resolver.Result{Authorizer: f.authorizer}, nil } func TestAgent_Reload(t *testing.T) { diff --git a/agent/consul/acl.go b/agent/consul/acl.go index efa9b2665e..bd8c88bd97 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -13,6 +13,7 @@ import ( "golang.org/x/time/rate" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/token" "github.com/hashicorp/consul/logging" @@ -662,26 +663,6 @@ func (r *ACLResolver) synthesizePoliciesForNodeIdentities(nodeIdentities []*stru return syntheticPolicies } -// plainACLResolver wraps ACLResolver so that it can be used in other packages -// that cannot import agent/consul wholesale (e.g. because of import cycles). -// -// TODO(agentless): this pattern was copied from subscribeBackend for expediency -// but we should really refactor ACLResolver so it can be passed as a dependency -// to other packages. -type plainACLResolver struct { - resolver *ACLResolver -} - -func (r plainACLResolver) ResolveTokenAndDefaultMeta( - token string, - entMeta *acl.EnterpriseMeta, - authzContext *acl.AuthorizerContext, -) (acl.Authorizer, error) { - // ACLResolver.ResolveTokenAndDefaultMeta returns a ACLResolveResult which - // can't be used in other packages, but it embeds acl.Authorizer which can. - return r.resolver.ResolveTokenAndDefaultMeta(token, entMeta, authzContext) -} - func mergeStringSlice(a, b []string) []string { out := make([]string, 0, len(a)+len(b)) out = append(out, a...) @@ -1008,13 +989,13 @@ func (r *ACLResolver) resolveLocallyManagedToken(token string) (structs.ACLIdent // ResolveToken to an acl.Authorizer and structs.ACLIdentity. The acl.Authorizer // can be used to check permissions granted to the token, and the ACLIdentity // describes the token and any defaults applied to it. -func (r *ACLResolver) ResolveToken(token string) (ACLResolveResult, error) { +func (r *ACLResolver) ResolveToken(token string) (resolver.Result, error) { if !r.ACLsEnabled() { - return ACLResolveResult{Authorizer: acl.ManageAll()}, nil + return resolver.Result{Authorizer: acl.ManageAll()}, nil } if acl.RootAuthorizer(token) != nil { - return ACLResolveResult{}, acl.ErrRootDenied + return resolver.Result{}, acl.ErrRootDenied } // handle the anonymous token @@ -1023,7 +1004,7 @@ func (r *ACLResolver) ResolveToken(token string) (ACLResolveResult, error) { } if ident, authz, ok := r.resolveLocallyManagedToken(token); ok { - return ACLResolveResult{Authorizer: authz, ACLIdentity: ident}, nil + return resolver.Result{Authorizer: authz, ACLIdentity: ident}, nil } defer metrics.MeasureSince([]string{"acl", "ResolveToken"}, time.Now()) @@ -1034,10 +1015,10 @@ func (r *ACLResolver) ResolveToken(token string) (ACLResolveResult, error) { if IsACLRemoteError(err) { r.logger.Error("Error resolving token", "error", err) ident := &missingIdentity{reason: "primary-dc-down", token: token} - return ACLResolveResult{Authorizer: r.down, ACLIdentity: ident}, nil + return resolver.Result{Authorizer: r.down, ACLIdentity: ident}, nil } - return ACLResolveResult{}, err + return resolver.Result{}, err } // Build the Authorizer @@ -1050,7 +1031,7 @@ func (r *ACLResolver) ResolveToken(token string) (ACLResolveResult, error) { authz, err := policies.Compile(r.cache, &conf) if err != nil { - return ACLResolveResult{}, err + return resolver.Result{}, err } chain = append(chain, authz) @@ -1058,36 +1039,15 @@ func (r *ACLResolver) ResolveToken(token string) (ACLResolveResult, error) { if err != nil { if IsACLRemoteError(err) { r.logger.Error("Error resolving identity defaults", "error", err) - return ACLResolveResult{Authorizer: r.down, ACLIdentity: identity}, nil + return resolver.Result{Authorizer: r.down, ACLIdentity: identity}, nil } - return ACLResolveResult{}, err + return resolver.Result{}, err } else if authz != nil { chain = append(chain, authz) } chain = append(chain, acl.RootAuthorizer(r.config.ACLDefaultPolicy)) - return ACLResolveResult{Authorizer: acl.NewChainedAuthorizer(chain), ACLIdentity: identity}, nil -} - -type ACLResolveResult struct { - acl.Authorizer - // TODO: likely we can reduce this interface - ACLIdentity structs.ACLIdentity -} - -func (a ACLResolveResult) AccessorID() string { - if a.ACLIdentity == nil { - return "" - } - return a.ACLIdentity.ID() -} - -func (a ACLResolveResult) Identity() structs.ACLIdentity { - return a.ACLIdentity -} - -func (a ACLResolveResult) ToAllowAuthorizer() acl.AllowAuthorizer { - return acl.AllowAuthorizer{Authorizer: a, AccessorID: a.AccessorID()} + return resolver.Result{Authorizer: acl.NewChainedAuthorizer(chain), ACLIdentity: identity}, nil } func (r *ACLResolver) ACLsEnabled() bool { @@ -1111,7 +1071,7 @@ func (r *ACLResolver) ResolveTokenAndDefaultMeta( token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext, -) (ACLResolveResult, error) { +) (resolver.Result, error) { return r.ResolveTokenAndDefaultMetaWithPeerName(token, entMeta, structs.DefaultPeerKeyword, authzContext) } @@ -1120,10 +1080,10 @@ func (r *ACLResolver) ResolveTokenAndDefaultMetaWithPeerName( entMeta *acl.EnterpriseMeta, peerName string, authzContext *acl.AuthorizerContext, -) (ACLResolveResult, error) { +) (resolver.Result, error) { result, err := r.ResolveToken(token) if err != nil { - return ACLResolveResult{}, err + return resolver.Result{}, err } if entMeta == nil { diff --git a/agent/consul/acl_endpoint.go b/agent/consul/acl_endpoint.go index 69908c055e..cd35e3667d 100644 --- a/agent/consul/acl_endpoint.go +++ b/agent/consul/acl_endpoint.go @@ -17,6 +17,7 @@ import ( uuid "github.com/hashicorp/go-uuid" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/consul/auth" "github.com/hashicorp/consul/agent/consul/authmethod" "github.com/hashicorp/consul/agent/consul/state" @@ -263,7 +264,7 @@ func (a *ACL) TokenRead(args *structs.ACLTokenGetRequest, reply *structs.ACLToke return err } - var authz ACLResolveResult + var authz resolver.Result if args.TokenIDType == structs.ACLTokenAccessor { var err error diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index 92c9bbc1a3..b0d26831d7 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -17,6 +17,7 @@ import ( msgpackrpc "github.com/hashicorp/consul-net-rpc/net-rpc-msgpackrpc" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/token" "github.com/hashicorp/consul/api" @@ -47,7 +48,7 @@ type asyncResolutionResult struct { err error } -func verifyAuthorizerChain(t *testing.T, expected ACLResolveResult, actual ACLResolveResult) { +func verifyAuthorizerChain(t *testing.T, expected resolver.Result, actual resolver.Result) { t.Helper() expectedChainAuthz, ok := expected.Authorizer.(*acl.ChainedAuthorizer) require.True(t, ok, "expected Authorizer is not a ChainedAuthorizer") @@ -735,7 +736,7 @@ func TestACLResolver_Disabled(t *testing.T) { r := newTestACLResolver(t, delegate, nil) authz, err := r.ResolveToken("does not exist") - require.Equal(t, ACLResolveResult{Authorizer: acl.ManageAll()}, authz) + require.Equal(t, resolver.Result{Authorizer: acl.ManageAll()}, authz) require.Nil(t, err) } @@ -810,7 +811,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { authz, err := r.ResolveToken("foo") require.NoError(t, err) require.NotNil(t, authz) - expected := ACLResolveResult{ + expected := resolver.Result{ Authorizer: acl.DenyAll(), ACLIdentity: &missingIdentity{reason: "primary-dc-down", token: "foo"}, } @@ -838,7 +839,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { authz, err := r.ResolveToken("foo") require.NoError(t, err) require.NotNil(t, authz) - expected := ACLResolveResult{ + expected := resolver.Result{ Authorizer: acl.AllowAll(), ACLIdentity: &missingIdentity{reason: "primary-dc-down", token: "foo"}, } diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index fa940cc7f3..9fe11800b6 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -15,6 +15,7 @@ import ( hashstructure_v2 "github.com/mitchellh/hashstructure/v2" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/ipaddr" @@ -160,7 +161,7 @@ func nodePreApply(nodeName, nodeID string) error { return nil } -func servicePreApply(service *structs.NodeService, authz ACLResolveResult, authzCtxFill func(*acl.AuthorizerContext)) error { +func servicePreApply(service *structs.NodeService, authz resolver.Result, authzCtxFill func(*acl.AuthorizerContext)) error { // Validate the service. This is in addition to the below since // the above just hasn't been moved over yet. We should move it over // in time. @@ -230,7 +231,7 @@ func checkPreApply(check *structs.HealthCheck) { // worst let a service update revert a recent node update, so it doesn't open up // too much abuse). func vetRegisterWithACL( - authz ACLResolveResult, + authz resolver.Result, subj *structs.RegisterRequest, ns *structs.NodeServices, ) error { @@ -396,7 +397,7 @@ func (c *Catalog) Deregister(args *structs.DeregisterRequest, reply *struct{}) e // endpoint. The NodeService for the referenced service must be supplied, and can // be nil; similar for the HealthCheck for the referenced health check. func vetDeregisterWithACL( - authz ACLResolveResult, + authz resolver.Result, subj *structs.DeregisterRequest, ns *structs.NodeService, nc *structs.HealthCheck, diff --git a/agent/consul/catalog_endpoint_test.go b/agent/consul/catalog_endpoint_test.go index 67929575a7..bafdc6f322 100644 --- a/agent/consul/catalog_endpoint_test.go +++ b/agent/consul/catalog_endpoint_test.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/consul-net-rpc/net/rpc" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/lib" @@ -3467,11 +3468,11 @@ func TestVetRegisterWithACL(t *testing.T) { } // With an "allow all" authorizer the update should be allowed. - require.NoError(t, vetRegisterWithACL(ACLResolveResult{Authorizer: acl.ManageAll()}, args, nil)) + require.NoError(t, vetRegisterWithACL(resolver.Result{Authorizer: acl.ManageAll()}, args, nil)) }) var perms acl.Authorizer = acl.DenyAll() - var resolvedPerms ACLResolveResult + var resolvedPerms resolver.Result args := &structs.RegisterRequest{ Node: "nope", @@ -3483,7 +3484,7 @@ func TestVetRegisterWithACL(t *testing.T) { node "node" { policy = "write" } `) - resolvedPerms = ACLResolveResult{Authorizer: perms} + resolvedPerms = resolver.Result{Authorizer: perms} // With that policy, the update should now be blocked for node reasons. err := vetRegisterWithACL(resolvedPerms, args, nil) @@ -3514,7 +3515,7 @@ func TestVetRegisterWithACL(t *testing.T) { ID: "my-id", }, } - err = vetRegisterWithACL(ACLResolveResult{Authorizer: perms}, args, ns) + err = vetRegisterWithACL(resolver.Result{Authorizer: perms}, args, ns) require.True(t, acl.IsErrPermissionDenied(err)) // Chain on a basic service policy. @@ -3522,7 +3523,7 @@ func TestVetRegisterWithACL(t *testing.T) { service "service" { policy = "write" } `) - resolvedPerms = ACLResolveResult{Authorizer: perms} + resolvedPerms = resolver.Result{Authorizer: perms} // With the service ACL, the update should go through. require.NoError(t, vetRegisterWithACL(resolvedPerms, args, ns)) @@ -3549,7 +3550,7 @@ func TestVetRegisterWithACL(t *testing.T) { service "other" { policy = "write" } `) - resolvedPerms = ACLResolveResult{Authorizer: perms} + resolvedPerms = resolver.Result{Authorizer: perms} // Now it should go through. require.NoError(t, vetRegisterWithACL(resolvedPerms, args, ns)) @@ -3655,7 +3656,7 @@ func TestVetRegisterWithACL(t *testing.T) { service "other" { policy = "deny" } `) - resolvedPerms = ACLResolveResult{Authorizer: perms} + resolvedPerms = resolver.Result{Authorizer: perms} // This should get rejected. err = vetRegisterWithACL(resolvedPerms, args, ns) @@ -3682,7 +3683,7 @@ func TestVetRegisterWithACL(t *testing.T) { node "node" { policy = "deny" } `) - resolvedPerms = ACLResolveResult{Authorizer: perms} + resolvedPerms = resolver.Result{Authorizer: perms} // This should get rejected because there's a node-level check in here. err = vetRegisterWithACL(resolvedPerms, args, ns) @@ -3733,7 +3734,7 @@ func TestVetDeregisterWithACL(t *testing.T) { } // With an "allow all" authorizer the update should be allowed. - if err := vetDeregisterWithACL(ACLResolveResult{Authorizer: acl.ManageAll()}, args, nil, nil); err != nil { + if err := vetDeregisterWithACL(resolver.Result{Authorizer: acl.ManageAll()}, args, nil, nil); err != nil { t.Fatalf("err: %v", err) } @@ -3966,7 +3967,7 @@ node "node" { }, } { t.Run(args.Name, func(t *testing.T) { - err = vetDeregisterWithACL(ACLResolveResult{Authorizer: args.Perms}, &args.DeregisterRequest, args.Service, args.Check) + err = vetDeregisterWithACL(resolver.Result{Authorizer: args.Perms}, &args.DeregisterRequest, args.Service, args.Check) if !args.Expected { if err == nil { t.Errorf("expected error with %+v", args.DeregisterRequest) diff --git a/agent/consul/kvs_endpoint.go b/agent/consul/kvs_endpoint.go index 24ee58e7e3..434ebcadaa 100644 --- a/agent/consul/kvs_endpoint.go +++ b/agent/consul/kvs_endpoint.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/go-memdb" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" @@ -32,7 +33,7 @@ type KVS struct { // preApply does all the verification of a KVS update that is performed BEFORE // we submit as a Raft log entry. This includes enforcing the lock delay which // must only be done on the leader. -func kvsPreApply(logger hclog.Logger, srv *Server, authz ACLResolveResult, op api.KVOp, dirEnt *structs.DirEntry) (bool, error) { +func kvsPreApply(logger hclog.Logger, srv *Server, authz resolver.Result, op api.KVOp, dirEnt *structs.DirEntry) (bool, error) { // Verify the entry. if dirEnt.Key == "" && op != api.KVDeleteTree { return false, fmt.Errorf("Must provide key") diff --git a/agent/consul/server.go b/agent/consul/server.go index 0b8702cbae..371beb4f55 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -697,7 +697,7 @@ func NewServer(config *Config, flat Deps, publicGRPCServer *grpc.Server) (*Serve Publisher: s.publisher, GetStore: func() connectca.StateStore { return s.FSM().State() }, Logger: logger.Named("grpc-api.connect-ca"), - ACLResolver: plainACLResolver{s.ACLResolver}, + ACLResolver: s.ACLResolver, CAManager: s.caManager, ForwardRPC: func(info structs.RPCInfo, fn func(*grpc.ClientConn) error) (bool, error) { return s.ForwardGRPC(s.grpcConnPool, info, fn) @@ -709,13 +709,13 @@ func NewServer(config *Config, flat Deps, publicGRPCServer *grpc.Server) (*Serve dataplane.NewServer(dataplane.Config{ GetStore: func() dataplane.StateStore { return s.FSM().State() }, Logger: logger.Named("grpc-api.dataplane"), - ACLResolver: plainACLResolver{s.ACLResolver}, + ACLResolver: s.ACLResolver, Datacenter: s.config.Datacenter, }).Register(s.publicGRPCServer) serverdiscovery.NewServer(serverdiscovery.Config{ Publisher: s.publisher, - ACLResolver: plainACLResolver{s.ACLResolver}, + ACLResolver: s.ACLResolver, Logger: logger.Named("grpc-api.server-discovery"), }).Register(s.publicGRPCServer) diff --git a/agent/consul/txn_endpoint.go b/agent/consul/txn_endpoint.go index 06b9280887..a1977b9196 100644 --- a/agent/consul/txn_endpoint.go +++ b/agent/consul/txn_endpoint.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/go-hclog" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" ) @@ -32,7 +33,7 @@ type Txn struct { // preCheck is used to verify the incoming operations before any further // processing takes place. This checks things like ACLs. -func (t *Txn) preCheck(authorizer ACLResolveResult, ops structs.TxnOps) structs.TxnErrors { +func (t *Txn) preCheck(authorizer resolver.Result, ops structs.TxnOps) structs.TxnErrors { var errors structs.TxnErrors // Perform the pre-apply checks for any KV operations. @@ -109,7 +110,7 @@ func (t *Txn) preCheck(authorizer ACLResolveResult, ops structs.TxnOps) structs. } // vetNodeTxnOp applies the given ACL policy to a node transaction operation. -func vetNodeTxnOp(op *structs.TxnNodeOp, authz ACLResolveResult) error { +func vetNodeTxnOp(op *structs.TxnNodeOp, authz resolver.Result) error { var authzContext acl.AuthorizerContext op.FillAuthzContext(&authzContext) @@ -120,7 +121,7 @@ func vetNodeTxnOp(op *structs.TxnNodeOp, authz ACLResolveResult) error { } // vetCheckTxnOp applies the given ACL policy to a check transaction operation. -func vetCheckTxnOp(op *structs.TxnCheckOp, authz ACLResolveResult) error { +func vetCheckTxnOp(op *structs.TxnCheckOp, authz resolver.Result) error { var authzContext acl.AuthorizerContext op.FillAuthzContext(&authzContext) diff --git a/agent/delegate_mock_test.go b/agent/delegate_mock_test.go index 5498e5f04d..23b93b829a 100644 --- a/agent/delegate_mock_test.go +++ b/agent/delegate_mock_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/lib" @@ -47,9 +48,9 @@ func (m *delegateMock) RemoveFailedNode(node string, prune bool, entMeta *acl.En return m.Called(node, prune, entMeta).Error(0) } -func (m *delegateMock) ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (consul.ACLResolveResult, error) { +func (m *delegateMock) ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (resolver.Result, error) { ret := m.Called(token, entMeta, authzContext) - return ret.Get(0).(consul.ACLResolveResult), ret.Error(1) + return ret.Get(0).(resolver.Result), ret.Error(1) } func (m *delegateMock) RPC(method string, args interface{}, reply interface{}) error { diff --git a/agent/grpc/public/services/connectca/mock_ACLResolver.go b/agent/grpc/public/services/connectca/mock_ACLResolver.go index a1ff427964..24fb26a225 100644 --- a/agent/grpc/public/services/connectca/mock_ACLResolver.go +++ b/agent/grpc/public/services/connectca/mock_ACLResolver.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.11.0. DO NOT EDIT. +// Code generated by mockery v2.12.0. DO NOT EDIT. package connectca @@ -6,6 +6,8 @@ import ( acl "github.com/hashicorp/consul/acl" mock "github.com/stretchr/testify/mock" + resolver "github.com/hashicorp/consul/acl/resolver" + testing "testing" ) @@ -15,16 +17,14 @@ type MockACLResolver struct { } // ResolveTokenAndDefaultMeta provides a mock function with given fields: token, entMeta, authzContext -func (_m *MockACLResolver) ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (acl.Authorizer, error) { +func (_m *MockACLResolver) ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (resolver.Result, error) { ret := _m.Called(token, entMeta, authzContext) - var r0 acl.Authorizer - if rf, ok := ret.Get(0).(func(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) acl.Authorizer); ok { + var r0 resolver.Result + if rf, ok := ret.Get(0).(func(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) resolver.Result); ok { r0 = rf(token, entMeta, authzContext) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(acl.Authorizer) - } + r0 = ret.Get(0).(resolver.Result) } var r1 error @@ -37,9 +37,10 @@ func (_m *MockACLResolver) ResolveTokenAndDefaultMeta(token string, entMeta *acl return r0, r1 } -// NewMockACLResolver creates a new instance of MockACLResolver. It also registers a cleanup function to assert the mocks expectations. +// NewMockACLResolver creates a new instance of MockACLResolver. It also registers the testing.TB interface on the mock and a cleanup function to assert the mocks expectations. func NewMockACLResolver(t testing.TB) *MockACLResolver { mock := &MockACLResolver{} + mock.Mock.Test(t) t.Cleanup(func() { mock.AssertExpectations(t) }) diff --git a/agent/grpc/public/services/connectca/mock_CAManager.go b/agent/grpc/public/services/connectca/mock_CAManager.go index 8839344a2d..2667692c33 100644 --- a/agent/grpc/public/services/connectca/mock_CAManager.go +++ b/agent/grpc/public/services/connectca/mock_CAManager.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.11.0. DO NOT EDIT. +// Code generated by mockery v2.12.0. DO NOT EDIT. package connectca @@ -41,9 +41,10 @@ func (_m *MockCAManager) AuthorizeAndSignCertificate(csr *x509.CertificateReques return r0, r1 } -// NewMockCAManager creates a new instance of MockCAManager. It also registers a cleanup function to assert the mocks expectations. +// NewMockCAManager creates a new instance of MockCAManager. It also registers the testing.TB interface on the mock and a cleanup function to assert the mocks expectations. func NewMockCAManager(t testing.TB) *MockCAManager { mock := &MockCAManager{} + mock.Mock.Test(t) t.Cleanup(func() { mock.AssertExpectations(t) }) diff --git a/agent/grpc/public/services/connectca/server.go b/agent/grpc/public/services/connectca/server.go index f9abd49ee4..fb09ab47fe 100644 --- a/agent/grpc/public/services/connectca/server.go +++ b/agent/grpc/public/services/connectca/server.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/go-memdb" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/consul/stream" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/proto-public/pbconnectca" @@ -41,7 +42,7 @@ type StateStore interface { //go:generate mockery --name ACLResolver --inpackage type ACLResolver interface { - ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (acl.Authorizer, error) + ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (resolver.Result, error) } //go:generate mockery --name CAManager --inpackage diff --git a/agent/grpc/public/services/connectca/sign_test.go b/agent/grpc/public/services/connectca/sign_test.go index a4f891b8ca..aa20458f89 100644 --- a/agent/grpc/public/services/connectca/sign_test.go +++ b/agent/grpc/public/services/connectca/sign_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/require" acl "github.com/hashicorp/consul/acl" + resolver "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/grpc/public/testutils" "github.com/hashicorp/consul/agent/structs" @@ -32,7 +33,7 @@ func TestSign_ConnectDisabled(t *testing.T) { func TestSign_Validation(t *testing.T) { aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(acl.AllowAll(), nil) + Return(testutils.TestAuthorizerAllowAll(t), nil) server := NewServer(Config{ Logger: hclog.NewNullLogger(), @@ -68,7 +69,7 @@ func TestSign_Validation(t *testing.T) { func TestSign_Unauthenticated(t *testing.T) { aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(nil, acl.ErrNotFound) + Return(resolver.Result{}, acl.ErrNotFound) server := NewServer(Config{ Logger: hclog.NewNullLogger(), @@ -89,7 +90,7 @@ func TestSign_Unauthenticated(t *testing.T) { func TestSign_PermissionDenied(t *testing.T) { aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(acl.AllowAll(), nil) + Return(testutils.TestAuthorizerAllowAll(t), nil) caManager := &MockCAManager{} caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything). @@ -115,7 +116,7 @@ func TestSign_PermissionDenied(t *testing.T) { func TestSign_InvalidCSR(t *testing.T) { aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(acl.AllowAll(), nil) + Return(testutils.TestAuthorizerAllowAll(t), nil) caManager := &MockCAManager{} caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything). @@ -141,7 +142,7 @@ func TestSign_InvalidCSR(t *testing.T) { func TestSign_RateLimited(t *testing.T) { aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(acl.AllowAll(), nil) + Return(testutils.TestAuthorizerAllowAll(t), nil) caManager := &MockCAManager{} caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything). @@ -167,7 +168,7 @@ func TestSign_RateLimited(t *testing.T) { func TestSign_InternalError(t *testing.T) { aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(acl.AllowAll(), nil) + Return(testutils.TestAuthorizerAllowAll(t), nil) caManager := &MockCAManager{} caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything). @@ -193,7 +194,7 @@ func TestSign_InternalError(t *testing.T) { func TestSign_Success(t *testing.T) { aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(acl.AllowAll(), nil) + Return(testutils.TestAuthorizerAllowAll(t), nil) caManager := &MockCAManager{} caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything). @@ -219,7 +220,7 @@ func TestSign_Success(t *testing.T) { func TestSign_RPCForwarding(t *testing.T) { aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(acl.AllowAll(), nil) + Return(testutils.TestAuthorizerAllowAll(t), nil) caManager := &MockCAManager{} caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything). diff --git a/agent/grpc/public/services/connectca/watch_roots_test.go b/agent/grpc/public/services/connectca/watch_roots_test.go index d0960da51f..b65bc014bf 100644 --- a/agent/grpc/public/services/connectca/watch_roots_test.go +++ b/agent/grpc/public/services/connectca/watch_roots_test.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/go-uuid" "github.com/hashicorp/consul/acl" + resolver "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/grpc/public" "github.com/hashicorp/consul/agent/grpc/public/testutils" @@ -101,7 +102,7 @@ func TestWatchRoots_InvalidACLToken(t *testing.T) { // Mock the ACL resolver to return ErrNotFound. aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(nil, acl.ErrNotFound) + Return(resolver.Result{}, acl.ErrNotFound) ctx := public.ContextWithToken(context.Background(), testACLToken) @@ -179,7 +180,7 @@ func TestWatchRoots_ACLTokenInvalidated(t *testing.T) { // Simulate removing the `service:write` permission. aclResolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything). - Return(acl.DenyAll(), nil) + Return(testutils.TestAuthorizerDenyAll(t), nil) // Update the ACL token to cause the subscription to be force-closed. err = fsm.GetStore().ACLTokenSet(1, &structs.ACLToken{ diff --git a/agent/grpc/public/services/dataplane/get_envoy_boostrap_params_test.go b/agent/grpc/public/services/dataplane/get_envoy_boostrap_params_test.go index 072068861b..e3a9ce703c 100644 --- a/agent/grpc/public/services/dataplane/get_envoy_boostrap_params_test.go +++ b/agent/grpc/public/services/dataplane/get_envoy_boostrap_params_test.go @@ -4,18 +4,20 @@ import ( "context" "testing" - acl "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/agent/grpc/public" - "github.com/hashicorp/consul/agent/grpc/public/testutils" - structs "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/consul/proto-public/pbdataplane" - "github.com/hashicorp/consul/types" "github.com/hashicorp/go-hclog" mock "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/structpb" + + acl "github.com/hashicorp/consul/acl" + resolver "github.com/hashicorp/consul/acl/resolver" + "github.com/hashicorp/consul/agent/grpc/public" + "github.com/hashicorp/consul/agent/grpc/public/testutils" + structs "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/proto-public/pbdataplane" + "github.com/hashicorp/consul/types" ) const ( @@ -215,7 +217,7 @@ func TestGetEnvoyBootstrapParams_Unauthenticated(t *testing.T) { // Mock the ACL resolver to return ErrNotFound. aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(nil, acl.ErrNotFound) + Return(resolver.Result{}, acl.ErrNotFound) ctx := public.ContextWithToken(context.Background(), testToken) store := testutils.TestStateStore(t, nil) server := NewServer(Config{ @@ -234,7 +236,7 @@ func TestGetEnvoyBootstrapParams_PermissionDenied(t *testing.T) { // Mock the ACL resolver to return a deny all authorizer aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", testToken, mock.Anything, mock.Anything). - Return(acl.DenyAll(), nil) + Return(testutils.TestAuthorizerDenyAll(t), nil) ctx := public.ContextWithToken(context.Background(), testToken) store := testutils.TestStateStore(t, nil) registerReq := structs.TestRegisterRequestProxy(t) diff --git a/agent/grpc/public/services/dataplane/get_supported_features_test.go b/agent/grpc/public/services/dataplane/get_supported_features_test.go index 0dd2b02d0e..bdcd0d455d 100644 --- a/agent/grpc/public/services/dataplane/get_supported_features_test.go +++ b/agent/grpc/public/services/dataplane/get_supported_features_test.go @@ -11,6 +11,7 @@ import ( "google.golang.org/grpc/status" "github.com/hashicorp/consul/acl" + resolver "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/grpc/public" "github.com/hashicorp/consul/agent/grpc/public/testutils" "github.com/hashicorp/consul/proto-public/pbdataplane" @@ -51,7 +52,7 @@ func TestSupportedDataplaneFeatures_Unauthenticated(t *testing.T) { // Mock the ACL resolver to return ErrNotFound. aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(nil, acl.ErrNotFound) + Return(resolver.Result{}, acl.ErrNotFound) ctx := public.ContextWithToken(context.Background(), testACLToken) server := NewServer(Config{ Logger: hclog.NewNullLogger(), @@ -68,7 +69,7 @@ func TestSupportedDataplaneFeatures_PermissionDenied(t *testing.T) { // Mock the ACL resolver to return a deny all authorizer aclResolver := &MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything). - Return(acl.DenyAll(), nil) + Return(testutils.TestAuthorizerDenyAll(t), nil) ctx := public.ContextWithToken(context.Background(), testACLToken) server := NewServer(Config{ Logger: hclog.NewNullLogger(), diff --git a/agent/grpc/public/services/dataplane/mock_ACLResolver.go b/agent/grpc/public/services/dataplane/mock_ACLResolver.go index 1a73abfc81..0408d3a50c 100644 --- a/agent/grpc/public/services/dataplane/mock_ACLResolver.go +++ b/agent/grpc/public/services/dataplane/mock_ACLResolver.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.11.0. DO NOT EDIT. +// Code generated by mockery v2.12.0. DO NOT EDIT. package dataplane @@ -6,6 +6,8 @@ import ( acl "github.com/hashicorp/consul/acl" mock "github.com/stretchr/testify/mock" + resolver "github.com/hashicorp/consul/acl/resolver" + testing "testing" ) @@ -15,16 +17,14 @@ type MockACLResolver struct { } // ResolveTokenAndDefaultMeta provides a mock function with given fields: _a0, _a1, _a2 -func (_m *MockACLResolver) ResolveTokenAndDefaultMeta(_a0 string, _a1 *acl.EnterpriseMeta, _a2 *acl.AuthorizerContext) (acl.Authorizer, error) { +func (_m *MockACLResolver) ResolveTokenAndDefaultMeta(_a0 string, _a1 *acl.EnterpriseMeta, _a2 *acl.AuthorizerContext) (resolver.Result, error) { ret := _m.Called(_a0, _a1, _a2) - var r0 acl.Authorizer - if rf, ok := ret.Get(0).(func(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) acl.Authorizer); ok { + var r0 resolver.Result + if rf, ok := ret.Get(0).(func(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) resolver.Result); ok { r0 = rf(_a0, _a1, _a2) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(acl.Authorizer) - } + r0 = ret.Get(0).(resolver.Result) } var r1 error @@ -37,9 +37,10 @@ func (_m *MockACLResolver) ResolveTokenAndDefaultMeta(_a0 string, _a1 *acl.Enter return r0, r1 } -// NewMockACLResolver creates a new instance of MockACLResolver. It also registers a cleanup function to assert the mocks expectations. +// NewMockACLResolver creates a new instance of MockACLResolver. It also registers the testing.TB interface on the mock and a cleanup function to assert the mocks expectations. func NewMockACLResolver(t testing.TB) *MockACLResolver { mock := &MockACLResolver{} + mock.Mock.Test(t) t.Cleanup(func() { mock.AssertExpectations(t) }) diff --git a/agent/grpc/public/services/dataplane/server.go b/agent/grpc/public/services/dataplane/server.go index b45f6f38ac..4b4aef061e 100644 --- a/agent/grpc/public/services/dataplane/server.go +++ b/agent/grpc/public/services/dataplane/server.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/go-hclog" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/proto-public/pbdataplane" ) @@ -28,7 +29,7 @@ type StateStore interface { //go:generate mockery --name ACLResolver --inpackage type ACLResolver interface { - ResolveTokenAndDefaultMeta(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) (acl.Authorizer, error) + ResolveTokenAndDefaultMeta(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) (resolver.Result, error) } func NewServer(cfg Config) *Server { diff --git a/agent/grpc/public/services/serverdiscovery/mock_ACLResolver.go b/agent/grpc/public/services/serverdiscovery/mock_ACLResolver.go index 909e9c6172..850ec8bb95 100644 --- a/agent/grpc/public/services/serverdiscovery/mock_ACLResolver.go +++ b/agent/grpc/public/services/serverdiscovery/mock_ACLResolver.go @@ -1,10 +1,14 @@ -// Code generated by mockery v1.0.0. DO NOT EDIT. +// Code generated by mockery v2.12.0. DO NOT EDIT. package serverdiscovery import ( acl "github.com/hashicorp/consul/acl" mock "github.com/stretchr/testify/mock" + + resolver "github.com/hashicorp/consul/acl/resolver" + + testing "testing" ) // MockACLResolver is an autogenerated mock type for the ACLResolver type @@ -13,16 +17,14 @@ type MockACLResolver struct { } // ResolveTokenAndDefaultMeta provides a mock function with given fields: _a0, _a1, _a2 -func (_m *MockACLResolver) ResolveTokenAndDefaultMeta(_a0 string, _a1 *acl.EnterpriseMeta, _a2 *acl.AuthorizerContext) (acl.Authorizer, error) { +func (_m *MockACLResolver) ResolveTokenAndDefaultMeta(_a0 string, _a1 *acl.EnterpriseMeta, _a2 *acl.AuthorizerContext) (resolver.Result, error) { ret := _m.Called(_a0, _a1, _a2) - var r0 acl.Authorizer - if rf, ok := ret.Get(0).(func(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) acl.Authorizer); ok { + var r0 resolver.Result + if rf, ok := ret.Get(0).(func(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) resolver.Result); ok { r0 = rf(_a0, _a1, _a2) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(acl.Authorizer) - } + r0 = ret.Get(0).(resolver.Result) } var r1 error @@ -34,3 +36,13 @@ func (_m *MockACLResolver) ResolveTokenAndDefaultMeta(_a0 string, _a1 *acl.Enter return r0, r1 } + +// NewMockACLResolver creates a new instance of MockACLResolver. It also registers the testing.TB interface on the mock and a cleanup function to assert the mocks expectations. +func NewMockACLResolver(t testing.TB) *MockACLResolver { + mock := &MockACLResolver{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/agent/grpc/public/services/serverdiscovery/server.go b/agent/grpc/public/services/serverdiscovery/server.go index ec82b47fa3..c7b2e0e1d4 100644 --- a/agent/grpc/public/services/serverdiscovery/server.go +++ b/agent/grpc/public/services/serverdiscovery/server.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/go-hclog" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/consul/stream" "github.com/hashicorp/consul/proto-public/pbserverdiscovery" ) @@ -24,9 +25,9 @@ type EventPublisher interface { Subscribe(*stream.SubscribeRequest) (*stream.Subscription, error) } -//go:generate mockery -name ACLResolver -inpkg +//go:generate mockery --name ACLResolver --inpackage type ACLResolver interface { - ResolveTokenAndDefaultMeta(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) (acl.Authorizer, error) + ResolveTokenAndDefaultMeta(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) (resolver.Result, error) } func NewServer(cfg Config) *Server { diff --git a/agent/grpc/public/services/serverdiscovery/watch_servers_test.go b/agent/grpc/public/services/serverdiscovery/watch_servers_test.go index 1409431d94..a44520e558 100644 --- a/agent/grpc/public/services/serverdiscovery/watch_servers_test.go +++ b/agent/grpc/public/services/serverdiscovery/watch_servers_test.go @@ -7,7 +7,13 @@ import ( "testing" "time" + mock "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + acl "github.com/hashicorp/consul/acl" + resolver "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/consul/autopilotevents" "github.com/hashicorp/consul/agent/consul/stream" "github.com/hashicorp/consul/agent/grpc/public" @@ -15,10 +21,6 @@ import ( "github.com/hashicorp/consul/proto-public/pbserverdiscovery" "github.com/hashicorp/consul/proto/prototest" "github.com/hashicorp/consul/sdk/testutil" - mock "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) const testACLToken = "eb61f1ed-65a4-4da6-8d3d-0564bd16c965" @@ -193,7 +195,7 @@ func TestWatchServers_ACLToken_PermissionDenied(t *testing.T) { resolver := newMockACLResolver(t) resolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything). - Return(acl.DenyAll(), nil).Once() + Return(testutils.TestAuthorizerDenyAll(t), nil).Once() // add the token to the requests context ctx := public.ContextWithToken(context.Background(), testACLToken) @@ -222,9 +224,9 @@ func TestWatchServers_ACLToken_Unauthenticated(t *testing.T) { // setup the event publisher and snapshot handler _, publisher := setupPublisher(t) - resolver := newMockACLResolver(t) - resolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything). - Return(nil, acl.ErrNotFound).Once() + aclResolver := newMockACLResolver(t) + aclResolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything). + Return(resolver.Result{}, acl.ErrNotFound).Once() // add the token to the requests context ctx := public.ContextWithToken(context.Background(), testACLToken) @@ -233,7 +235,7 @@ func TestWatchServers_ACLToken_Unauthenticated(t *testing.T) { server := NewServer(Config{ Publisher: publisher, Logger: testutil.Logger(t), - ACLResolver: resolver, + ACLResolver: aclResolver, }) // Run the server and get a test client for it diff --git a/agent/grpc/public/testutils/acl.go b/agent/grpc/public/testutils/acl.go index 8caacb1052..fab3646e73 100644 --- a/agent/grpc/public/testutils/acl.go +++ b/agent/grpc/public/testutils/acl.go @@ -6,9 +6,22 @@ import ( "github.com/stretchr/testify/require" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" ) -func TestAuthorizerServiceWriteAny(t *testing.T) acl.Authorizer { +func TestAuthorizerAllowAll(t *testing.T) resolver.Result { + t.Helper() + + return resolver.Result{Authorizer: acl.AllowAll()} +} + +func TestAuthorizerDenyAll(t *testing.T) resolver.Result { + t.Helper() + + return resolver.Result{Authorizer: acl.DenyAll()} +} + +func TestAuthorizerServiceWriteAny(t *testing.T) resolver.Result { t.Helper() policy, err := acl.NewPolicyFromSource(` @@ -21,10 +34,10 @@ func TestAuthorizerServiceWriteAny(t *testing.T) acl.Authorizer { authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil) require.NoError(t, err) - return authz + return resolver.Result{Authorizer: authz} } -func TestAuthorizerServiceRead(t *testing.T, serviceName string) acl.Authorizer { +func TestAuthorizerServiceRead(t *testing.T, serviceName string) resolver.Result { t.Helper() aclRule := &acl.Policy{ @@ -40,5 +53,5 @@ func TestAuthorizerServiceRead(t *testing.T, serviceName string) acl.Authorizer authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{aclRule}, nil) require.NoError(t, err) - return authz + return resolver.Result{Authorizer: authz} } diff --git a/agent/local/state.go b/agent/local/state.go index 9a2e00a943..74641a0683 100644 --- a/agent/local/state.go +++ b/agent/local/state.go @@ -9,6 +9,7 @@ import ( "sync/atomic" "time" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/lib/stringslice" "github.com/armon/go-metrics" @@ -17,7 +18,6 @@ import ( "github.com/mitchellh/copystructure" "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/token" "github.com/hashicorp/consul/api" @@ -154,7 +154,7 @@ func (c *CheckState) CriticalFor() time.Duration { type rpc interface { RPC(method string, args interface{}, reply interface{}) error - ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (consul.ACLResolveResult, error) + ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzContext *acl.AuthorizerContext) (resolver.Result, error) } // State is used to represent the node's services, diff --git a/agent/local/state_test.go b/agent/local/state_test.go index c75d0234c3..686c86a935 100644 --- a/agent/local/state_test.go +++ b/agent/local/state_test.go @@ -14,9 +14,9 @@ import ( "github.com/stretchr/testify/require" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent" "github.com/hashicorp/consul/agent/config" - "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/local" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/token" @@ -2421,6 +2421,6 @@ func (f *fakeRPC) RPC(method string, args interface{}, reply interface{}) error return nil } -func (f *fakeRPC) ResolveTokenAndDefaultMeta(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) (consul.ACLResolveResult, error) { - return consul.ACLResolveResult{}, nil +func (f *fakeRPC) ResolveTokenAndDefaultMeta(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) (resolver.Result, error) { + return resolver.Result{}, nil } diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 58f6fa77ab..05d7480cbb 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -63,7 +63,7 @@ type ConfigEntry interface { // CanRead and CanWrite return whether or not the given Authorizer // has permission to read or write to the config entry, respectively. - // TODO(acl-error-enhancements) This should be ACLResolveResult or similar but we have to wait until we move things to the acl package + // TODO(acl-error-enhancements) This should be resolver.Result or similar but we have to wait until we move things to the acl package CanRead(acl.Authorizer) error CanWrite(acl.Authorizer) error