From c52caae34552bba8714c7f5fd1f2f80cb000e5a4 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Tue, 31 Oct 2023 13:44:17 +0000 Subject: [PATCH] refact(action): Make ActionSet behave as a set instead of a list This switches the implementation of an ActionSet from a slice to a map to ensure the elements in an ActionSet are unique. This allows for other packages to perform set operations such as a union or difference without needing to account for duplicates. --- internal/auth/additional_verification_test.go | 10 +- internal/daemon/controller/auth/auth.go | 10 +- .../controller/common/scopeids/scope_ids.go | 4 +- .../handlers/accounts/account_service.go | 16 +- .../handlers/accounts/account_service_test.go | 110 +++++++- .../authmethods/authmethod_service.go | 5 +- .../authmethods/authmethod_service_test.go | 40 ++- .../controller/handlers/authmethods/ldap.go | 4 +- .../handlers/authmethods/ldap_test.go | 11 +- .../controller/handlers/authmethods/oidc.go | 4 +- .../handlers/authmethods/oidc_test.go | 49 +++- .../handlers/authmethods/password.go | 4 +- .../handlers/authmethods/password_test.go | 11 +- .../handlers/authtokens/authtoken_service.go | 8 +- .../authtokens/authtoken_service_test.go | 24 +- .../credentiallibrary_service.go | 8 +- .../credentiallibrary_service_test.go | 66 ++++- .../credentials/credential_service.go | 8 +- .../credentials/credential_service_test.go | 43 ++- .../credentialstore_service.go | 8 +- .../credentialstore_service_test.go | 66 ++++- .../handlers/groups/group_service.go | 8 +- .../handlers/groups/group_service_test.go | 37 ++- .../host_catalogs/host_catalog_service.go | 12 +- .../host_catalog_service_test.go | 84 +++++- .../handlers/host_sets/host_set_service.go | 12 +- .../host_sets/host_set_service_test.go | 82 +++++- .../controller/handlers/hosts/host_service.go | 12 +- .../handlers/hosts/host_service_test.go | 64 ++++- .../managed_groups/managed_group_service.go | 12 +- .../managed_group_service_test.go | 64 ++++- .../controller/handlers/roles/role_service.go | 8 +- .../handlers/roles/role_service_test.go | 37 ++- .../handlers/scopes/scope_service.go | 21 +- .../handlers/scopes/scope_service_test.go | 94 ++++++- .../session_recording_service.go | 8 +- .../handlers/sessions/session_service.go | 8 +- .../handlers/sessions/session_service_test.go | 37 ++- .../storage_buckets/storage_bucket_service.go | 8 +- .../handlers/targets/target_service.go | 8 +- .../targets/tcp/target_service_test.go | 74 +++++- .../controller/handlers/users/user_service.go | 8 +- .../handlers/users/user_service_test.go | 37 ++- .../handlers/workers/worker_service.go | 8 +- .../handlers/workers/worker_service_test.go | 28 +- internal/perms/acl.go | 6 +- internal/perms/acl_test.go | 38 +-- .../tests/api/authmethods/authmethod_test.go | 30 ++- internal/tests/api/hosts/host_test.go | 14 +- internal/tests/api/hostsets/host_set_test.go | 6 + internal/types/action/action.go | 56 +++- internal/types/action/action_test.go | 245 +++++++++++++++++- 52 files changed, 1401 insertions(+), 274 deletions(-) diff --git a/internal/auth/additional_verification_test.go b/internal/auth/additional_verification_test.go index 9de3e08c2..c4a4ebe72 100644 --- a/internal/auth/additional_verification_test.go +++ b/internal/auth/additional_verification_test.go @@ -67,13 +67,13 @@ func TestFetchActionSetForId(t *testing.T) { { name: "no match", id: "zip", - avail: action.ActionSet{action.Read, action.Update}, + avail: action.NewActionSet(action.Read, action.Update), }, { name: "disjoint match", id: "ttcp_bar", - avail: action.ActionSet{action.Delete, action.AddGrants, action.Read, action.RemoveHostSets}, - allowed: action.ActionSet{action.Delete, action.Read}, + avail: action.NewActionSet(action.Delete, action.AddGrants, action.Read, action.RemoveHostSets), + allowed: action.NewActionSet(action.Delete, action.Read), }, { name: "different type", @@ -84,8 +84,8 @@ func TestFetchActionSetForId(t *testing.T) { name: "type match", id: "anything", typeOverride: resource.Role, - avail: action.ActionSet{action.Read, action.AddGrants}, - allowed: action.ActionSet{action.AddGrants}, + avail: action.NewActionSet(action.Read, action.AddGrants), + allowed: action.NewActionSet(action.AddGrants), }, } for _, tt := range cases { diff --git a/internal/daemon/controller/auth/auth.go b/internal/daemon/controller/auth/auth.go index 0fc8b5920..af2f642a8 100644 --- a/internal/daemon/controller/auth/auth.go +++ b/internal/daemon/controller/auth/auth.go @@ -722,10 +722,10 @@ func (r *VerifyResults) fetchActions(id string, typ resource.Type, availableActi res.Type = typ } - ret := make(action.ActionSet, 0, len(availableActions)) - for _, act := range availableActions { + ret := make(action.ActionSet, len(availableActions)) + for act := range availableActions { if r.v.acl.Allowed(*res, act, *r.UserData.User.Id).Authorized { - ret = append(ret, act) + ret.Add(act) } } if len(ret) == 0 { @@ -854,7 +854,7 @@ func (r *VerifyResults) ScopesAuthorizedForList(ctx context.Context, rootScopeId scpId := scp.GetPublicId() aSet := r.FetchActionSetForType(ctx, resource.Unknown, // This is overridden by `WithResource` option. - action.ActionSet{action.List}, + action.NewActionSet(action.List), WithResource(&perms.Resource{Type: resourceType, ScopeId: scpId}), ) @@ -867,7 +867,7 @@ func (r *VerifyResults) ScopesAuthorizedForList(ctx context.Context, rootScopeId // lookup might fail. deferredScopes = append(deferredScopes, scp) case len(aSet) == 1 || r.UserId == globals.RecoveryUserId: - if aSet[0] != action.List { + if !aSet.HasAction(action.List) { return nil, errors.New(ctx, errors.Internal, op, "unexpected action in set") } if scopeResourceMap[scpId] == nil { diff --git a/internal/daemon/controller/common/scopeids/scope_ids.go b/internal/daemon/controller/common/scopeids/scope_ids.go index 12bf7a5ad..ea0c6a01e 100644 --- a/internal/daemon/controller/common/scopeids/scope_ids.go +++ b/internal/daemon/controller/common/scopeids/scope_ids.go @@ -153,7 +153,7 @@ func GetListingResourceInformation( aSet := input.AuthResults.FetchActionSetForType(ctx, // This is overridden by WithResource resource.Unknown, - action.ActionSet{action.List}, + action.NewActionSet(action.List), auth.WithResource(&res), ) switch len(aSet) { @@ -163,7 +163,7 @@ func GetListingResourceInformation( // lookup might fail. deferredScopes = append(deferredScopes, scp) case 1: - if aSet[0] != action.List { + if !aSet.HasAction(action.List) { return nil, errors.New(ctx, errors.Internal, op, "unexpected action in set") } if output.ScopeResourceMap[scpId] == nil { diff --git a/internal/daemon/controller/handlers/accounts/account_service.go b/internal/daemon/controller/handlers/accounts/account_service.go index d537a4f7f..35051d5cb 100644 --- a/internal/daemon/controller/handlers/accounts/account_service.go +++ b/internal/daemon/controller/handlers/accounts/account_service.go @@ -70,34 +70,34 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources IdActions = map[globals.Subtype]action.ActionSet{ - password.Subtype: { + password.Subtype: action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, action.SetPassword, action.ChangePassword, - }, - oidc.Subtype: { + ), + oidc.Subtype: action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - }, - ldap.Subtype: { + ), + ldap.Subtype: action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - }, + ), } // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) ) func init() { diff --git a/internal/daemon/controller/handlers/accounts/account_service_test.go b/internal/daemon/controller/handlers/accounts/account_service_test.go index cafc57c1e..0cfd2ab5c 100644 --- a/internal/daemon/controller/handlers/accounts/account_service_test.go +++ b/internal/daemon/controller/handlers/accounts/account_service_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/auth/ldap" "github.com/hashicorp/boundary/internal/auth/oidc" @@ -297,7 +298,14 @@ func TestGet(t *testing.T) { return } require.NoError(gErr) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -424,7 +432,15 @@ func TestListPassword(t *testing.T) { } else { require.NoError(gErr) } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform(), protocmp.SortRepeatedFields(got)), "ListAccounts() with scope %q got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + protocmp.SortRepeatedFields(got), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListAccounts() with scope %q got response %q, wanted %q", tc.req, got, tc.res) // Now test with anon if tc.skipAnon { @@ -584,7 +600,14 @@ func TestListOidc(t *testing.T) { return strings.Compare(got.Items[i].GetOidcAccountAttributes().Subject, got.Items[j].GetOidcAccountAttributes().Subject) < 0 }) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListAccounts() with scope %q got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListAccounts() with scope %q got response %q, wanted %q", tc.req, got, tc.res) // Now test with anon if tc.skipAnon { @@ -739,7 +762,14 @@ func TestListLdap(t *testing.T) { return strings.Compare(got.Items[i].GetLdapAccountAttributes().LoginName, got.Items[j].GetLdapAccountAttributes().LoginName) < 0 }) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListAccounts() with scope %q got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListAccounts() with scope %q got response %q, wanted %q", tc.req, got, tc.res) // Now test with anon if tc.skipAnon { @@ -1158,7 +1188,14 @@ func TestCreatePassword(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -1395,7 +1432,14 @@ func TestCreateOidc(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -1674,7 +1718,14 @@ func TestCreateLdap(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -2064,7 +2115,14 @@ func TestUpdatePassword(t *testing.T) { assert.EqualValues(2, got.Item.Version) tc.res.Item.Version = 2 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -2454,7 +2512,14 @@ func TestUpdateOidc(t *testing.T) { assert.EqualValues(2, got.Item.Version) tc.res.Item.Version = 2 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -2828,7 +2893,14 @@ func TestUpdateLdap(t *testing.T) { assert.EqualValues(2, got.Item.Version) tc.res.Item.Version = 2 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateAccount(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -2917,7 +2989,14 @@ func TestSetPassword(t *testing.T) { acct.Version, setResp.GetItem().Version = 0, 0 acct.UpdatedTime, setResp.GetItem().UpdatedTime = nil, nil - assert.Empty(cmp.Diff(acct, setResp.GetItem(), protocmp.Transform())) + assert.Empty(cmp.Diff( + acct, + setResp.GetItem(), + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) }) } @@ -3035,7 +3114,14 @@ func TestChangePassword(t *testing.T) { acct.Version, changeResp.GetItem().Version = 0, 0 acct.UpdatedTime, changeResp.GetItem().UpdatedTime = nil, nil - assert.Empty(cmp.Diff(acct, changeResp.GetItem(), protocmp.Transform())) + assert.Empty(cmp.Diff( + acct, + changeResp.GetItem(), + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) }) t.Run("unauthenticated update", func(t *testing.T) { diff --git a/internal/daemon/controller/handlers/authmethods/authmethod_service.go b/internal/daemon/controller/handlers/authmethods/authmethod_service.go index 7f2b26a63..aae2d8ab2 100644 --- a/internal/daemon/controller/handlers/authmethods/authmethod_service.go +++ b/internal/daemon/controller/handlers/authmethods/authmethod_service.go @@ -64,14 +64,15 @@ const ( var ( // IdActions contains the set of actions that can be performed on // individual resources + // This will be set in init functions in files for each subtype in this package. IdActions = make(map[globals.Subtype]action.ActionSet) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) collectionTypeMap = map[resource.Type]action.ActionSet{ resource.Account: accounts.CollectionActions, diff --git a/internal/daemon/controller/handlers/authmethods/authmethod_service_test.go b/internal/daemon/controller/handlers/authmethods/authmethod_service_test.go index dfce01e40..03542ee95 100644 --- a/internal/daemon/controller/handlers/authmethods/authmethod_service_test.go +++ b/internal/daemon/controller/handlers/authmethods/authmethod_service_test.go @@ -17,6 +17,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/auth/ldap" "github.com/hashicorp/boundary/internal/auth/oidc" @@ -269,7 +270,18 @@ func TestGet(t *testing.T) { if oidcAttrs := got.Item.GetOidcAuthMethodsAttributes(); oidcAttrs != nil { assert.NotEqual("secret", oidcAttrs.ClientSecretHmac) } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform(), protocmp.IgnoreFields(&pb.OidcAuthMethodAttributes{}, "client_secret_hmac")), "GetAuthMethod(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + protocmp.IgnoreFields(&pb.OidcAuthMethodAttributes{}, "client_secret_hmac"), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "GetAuthMethod(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -489,10 +501,19 @@ func TestList(t *testing.T) { } slices.SortFunc(got.Items, sorterFn) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform(), + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), protocmp.IgnoreFields(&pb.OidcAuthMethodAttributes{}, "client_secret_hmac"), - protocmp.IgnoreFields(&pb.LdapAuthMethodAttributes{}, "bind_password_hmac", "client_certificate_key_hmac")), - "ListAuthMethods() for scope %q got response %q, wanted %q", tc.req.GetScopeId(), got, tc.res) + protocmp.IgnoreFields(&pb.LdapAuthMethodAttributes{}, "bind_password_hmac", "client_certificate_key_hmac"), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "ListAuthMethods() for scope %q got response %q, wanted %q", tc.req.GetScopeId(), got, tc.res) // Now check with anonymous user got, gErr = s.ListAuthMethods(requestauth.DisabledAuthTestContext(iamRepoFn, tc.req.GetScopeId(), requestauth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -1460,7 +1481,15 @@ func TestCreate(t *testing.T) { if tc.res == nil { require.Nil(got) } - cmpOptions := []cmp.Option{protocmp.Transform()} + cmpOptions := []cmp.Option{ + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + } if got != nil { assert.Contains(got.GetUri(), tc.res.Uri) assert.True(strings.HasPrefix(got.GetItem().GetId(), tc.idPrefix)) @@ -1486,7 +1515,6 @@ func TestCreate(t *testing.T) { assert.True(matches, "%q doesn't match %q", gVal, exp) cmpOptions = append( cmpOptions, - protocmp.SortRepeatedFields(&pb.OidcAuthMethodAttributes{}, "account_claim_maps"), protocmp.IgnoreFields(&pb.OidcAuthMethodAttributes{}, "client_secret_hmac", "callback_url"), ) } diff --git a/internal/daemon/controller/handlers/authmethods/ldap.go b/internal/daemon/controller/handlers/authmethods/ldap.go index 86aefa275..72f12f338 100644 --- a/internal/daemon/controller/handlers/authmethods/ldap.go +++ b/internal/daemon/controller/handlers/authmethods/ldap.go @@ -36,13 +36,13 @@ func init() { panic(err) } - IdActions[ldap.Subtype] = action.ActionSet{ + IdActions[ldap.Subtype] = action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, action.Authenticate, - } + ) } const ( diff --git a/internal/daemon/controller/handlers/authmethods/ldap_test.go b/internal/daemon/controller/handlers/authmethods/ldap_test.go index 8129cfd28..032c4a8cc 100644 --- a/internal/daemon/controller/handlers/authmethods/ldap_test.go +++ b/internal/daemon/controller/handlers/authmethods/ldap_test.go @@ -14,6 +14,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/auth/ldap" "github.com/hashicorp/boundary/internal/auth/oidc" @@ -850,7 +851,15 @@ func Test_UpdateLdap(t *testing.T) { if tc.res == nil { require.Nil(got) } - cmpOptions := []cmp.Option{protocmp.Transform()} + cmpOptions := []cmp.Option{ + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + } if got != nil { assert.NotNilf(tc.res, "Expected UpdateAuthMethod response to be nil, but was %v", got) gotUpdateTime := got.GetItem().GetUpdatedTime().AsTime() diff --git a/internal/daemon/controller/handlers/authmethods/oidc.go b/internal/daemon/controller/handlers/authmethods/oidc.go index df7d4abaa..7bb3c9df5 100644 --- a/internal/daemon/controller/handlers/authmethods/oidc.go +++ b/internal/daemon/controller/handlers/authmethods/oidc.go @@ -57,14 +57,14 @@ func init() { panic(err) } - IdActions[oidc.Subtype] = action.ActionSet{ + IdActions[oidc.Subtype] = action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, action.ChangeState, action.Authenticate, - } + ) } type oidcState uint diff --git a/internal/daemon/controller/handlers/authmethods/oidc_test.go b/internal/daemon/controller/handlers/authmethods/oidc_test.go index a60275cf5..938fc3d66 100644 --- a/internal/daemon/controller/handlers/authmethods/oidc_test.go +++ b/internal/daemon/controller/handlers/authmethods/oidc_test.go @@ -16,6 +16,7 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" authpb "github.com/hashicorp/boundary/internal/gen/controller/auth" @@ -1076,7 +1077,15 @@ func TestUpdate_OIDC(t *testing.T) { if tc.res == nil { require.Nil(got) } - cmpOptions := []cmp.Option{protocmp.Transform()} + cmpOptions := []cmp.Option{ + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + } if got != nil { assert.NotNilf(tc.res, "Expected UpdateAuthMethod response to be nil, but was %v", got) gotUpdateTime := got.GetItem().GetUpdatedTime().AsTime() @@ -1279,11 +1288,32 @@ func TestUpdate_OIDCDryRun(t *testing.T) { return } require.NoError(t, err) - assert.Empty(t, cmp.Diff(dryUpdated.GetItem(), tc.want, protocmp.Transform(), protocmp.SortRepeatedFields(dryUpdated))) + assert.Empty(t, cmp.Diff( + dryUpdated.GetItem(), + tc.want, + protocmp.Transform(), + protocmp.SortRepeatedFields(dryUpdated), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + )) got, err := tested.GetAuthMethod(auth.DisabledAuthTestContext(iamRepoFn, o.GetPublicId()), &pbs.GetAuthMethodRequest{Id: tc.id}) require.NoError(t, err) - assert.Empty(t, cmp.Diff(got.GetItem(), wireAuthMethod, protocmp.Transform())) + assert.Empty(t, cmp.Diff( + got.GetItem(), + wireAuthMethod, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + )) }) } } @@ -1543,7 +1573,18 @@ func TestChangeState_OIDC(t *testing.T) { got.Item.UpdatedTime, tc.res.Item.UpdatedTime = nil, nil assert.Empty( - cmp.Diff(got, tc.res, protocmp.Transform(), protocmp.IgnoreFields(&pb.OidcAuthMethodAttributes{}, "client_secret_hmac", "callback_url")), + cmp.Diff( + got, + tc.res, + protocmp.Transform(), + protocmp.IgnoreFields(&pb.OidcAuthMethodAttributes{}, "client_secret_hmac", "callback_url"), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "ChangeState() got response %q, wanted %q", got, tc.res, ) }) diff --git a/internal/daemon/controller/handlers/authmethods/password.go b/internal/daemon/controller/handlers/authmethods/password.go index 40d921507..38fda9178 100644 --- a/internal/daemon/controller/handlers/authmethods/password.go +++ b/internal/daemon/controller/handlers/authmethods/password.go @@ -36,13 +36,13 @@ func init() { panic(err) } - IdActions[password.Subtype] = action.ActionSet{ + IdActions[password.Subtype] = action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, action.Authenticate, - } + ) } // createPwInRepo creates a password auth method in a repo and returns the result. diff --git a/internal/daemon/controller/handlers/authmethods/password_test.go b/internal/daemon/controller/handlers/authmethods/password_test.go index 40f83fc02..c4051f79a 100644 --- a/internal/daemon/controller/handlers/authmethods/password_test.go +++ b/internal/daemon/controller/handlers/authmethods/password_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/auth/ldap" "github.com/hashicorp/boundary/internal/auth/oidc" @@ -458,7 +459,15 @@ func TestUpdate_Password(t *testing.T) { require.Nil(got) } - cmpOptions := []cmp.Option{protocmp.Transform()} + cmpOptions := []cmp.Option{ + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + } if got != nil { assert.NotNilf(tc.res, "Expected UpdateAuthMethod response to be nil, but was %v", got) diff --git a/internal/daemon/controller/handlers/authtokens/authtoken_service.go b/internal/daemon/controller/handlers/authtokens/authtoken_service.go index 435524adc..1a87df8bc 100644 --- a/internal/daemon/controller/handlers/authtokens/authtoken_service.go +++ b/internal/daemon/controller/handlers/authtokens/authtoken_service.go @@ -27,19 +27,19 @@ import ( var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.ReadSelf, action.Delete, action.DeleteSelf, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.List, - } + ) ) // Service handles request as described by the pbs.AuthTokenServiceServer interface. diff --git a/internal/daemon/controller/handlers/authtokens/authtoken_service_test.go b/internal/daemon/controller/handlers/authtokens/authtoken_service_test.go index 0091a13ba..393a2223a 100644 --- a/internal/daemon/controller/handlers/authtokens/authtoken_service_test.go +++ b/internal/daemon/controller/handlers/authtokens/authtoken_service_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/authtoken" "github.com/hashicorp/boundary/internal/daemon/controller/auth" @@ -113,7 +114,7 @@ func TestGetSelf(t *testing.T) { require.NotNil(got) assert.Equal(tc.token.GetPublicId(), got.GetItem().GetId()) // Ensure we didn't simply have e.g. read on all tokens - assert.Equal([]string{"read:self", "delete:self"}, got.Item.GetAuthorizedActions()) + assert.ElementsMatch([]string{"read:self", "delete:self"}, got.Item.GetAuthorizedActions()) }) } } @@ -189,7 +190,14 @@ func TestGet(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "GetAuthToken(%+v) got error %v, wanted %v", tc.req, gErr, tc.err) } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetAuthToken(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetAuthToken(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -258,7 +266,7 @@ func TestList_Self(t *testing.T) { require.Len(got.Items, 1) assert.Equal(got.Items[0].GetId(), tc.requester.GetPublicId()) // Ensure we didn't simply have e.g. read on all tokens - assert.Equal(got.Items[0].GetAuthorizedActions(), []string{"read:self", "delete:self"}) + assert.ElementsMatch(got.Items[0].GetAuthorizedActions(), []string{"read:self", "delete:self"}) }) } } @@ -403,7 +411,15 @@ func TestList(t *testing.T) { } else { require.NoError(gErr) } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform(), protocmp.SortRepeatedFields(got)), "ListAuthTokens() with scope %q got response %q, wanted %q", tc.req.GetScopeId(), got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + protocmp.SortRepeatedFields(got), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListAuthTokens() with scope %q got response %q, wanted %q", tc.req.GetScopeId(), got, tc.res) // Now check anon listing got, gErr = s.ListAuthTokens(auth.DisabledAuthTestContext(iamRepoFn, tc.req.GetScopeId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) diff --git a/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service.go b/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service.go index 1313604b9..a72dfbdf5 100644 --- a/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service.go +++ b/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service.go @@ -60,19 +60,19 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) validCredentialTypesVaultGeneric = []globals.CredentialType{ globals.UsernamePasswordCredentialType, diff --git a/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service_test.go b/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service_test.go index f00212920..4f93068fe 100644 --- a/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service_test.go +++ b/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/credential/vault" "github.com/hashicorp/boundary/internal/daemon/controller/auth" @@ -155,7 +156,14 @@ func TestList(t *testing.T) { sort.Slice(want.Items, func(i, j int) bool { return want.Items[i].GetId() < want.Items[j].GetId() }) - assert.Empty(t, cmp.Diff(got, want, protocmp.Transform())) + assert.Empty(t, cmp.Diff( + got, + want, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) // Test anonymous listing got, gErr = s.ListCredentialLibraries(auth.DisabledAuthTestContext(iamRepoFn, prj.GetPublicId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -269,7 +277,14 @@ func TestList_Attributes(t *testing.T) { sort.Slice(want.Items, func(i, j int) bool { return want.Items[i].GetId() < want.Items[j].GetId() }) - assert.Empty(t, cmp.Diff(got, want, protocmp.Transform())) + assert.Empty(t, cmp.Diff( + got, + want, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) // Test anonymous listing got, gErr = s.ListCredentialLibraries(auth.DisabledAuthTestContext(iamRepoFn, prj.GetPublicId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -843,7 +858,15 @@ func TestCreate(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform(), protocmp.SortRepeatedFields(got)), "CreateCredentialLibrary(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + protocmp.SortRepeatedFields(got), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateCredentialLibrary(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -1041,7 +1064,14 @@ func TestGet(t *testing.T) { return } require.NoError(t, gErr) - assert.Empty(t, cmp.Diff(got, tc.res, protocmp.Transform())) + assert.Empty(t, cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) // Test anonymous get got, gErr = s.GetCredentialLibrary(auth.DisabledAuthTestContext(iamRepoFn, prj.GetPublicId(), auth.WithUserId(globals.AnonymousUserId)), req) @@ -1750,7 +1780,14 @@ func TestUpdate(t *testing.T) { assert.EqualValues(2, got.Item.Version) want.Item.Version = 2 - assert.Empty(cmp.Diff(got, want, protocmp.Transform())) + assert.Empty(cmp.Diff( + got, + want, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) }) } @@ -2210,7 +2247,15 @@ func TestCreate_SSHCertificateCredentialLibrary(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform(), protocmp.SortRepeatedFields(got)), "CreateCredentialLibrary(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + protocmp.SortRepeatedFields(got), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateCredentialLibrary(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -2468,7 +2513,14 @@ func TestUpdate_SSHCertificateCredentialLibrary(t *testing.T) { assert.EqualValues(2, got.Item.Version) want.Item.Version = 2 - assert.Empty(cmp.Diff(got, want, protocmp.Transform())) + assert.Empty(cmp.Diff( + got, + want, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) }) } diff --git a/internal/daemon/controller/handlers/credentials/credential_service.go b/internal/daemon/controller/handlers/credentials/credential_service.go index 769787f7c..c6e824baa 100644 --- a/internal/daemon/controller/handlers/credentials/credential_service.go +++ b/internal/daemon/controller/handlers/credentials/credential_service.go @@ -48,19 +48,19 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) ) func init() { diff --git a/internal/daemon/controller/handlers/credentials/credential_service_test.go b/internal/daemon/controller/handlers/credentials/credential_service_test.go index 11821edaa..e46c1db98 100644 --- a/internal/daemon/controller/handlers/credentials/credential_service_test.go +++ b/internal/daemon/controller/handlers/credentials/credential_service_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/credential" "github.com/hashicorp/boundary/internal/credential/static" @@ -186,9 +187,17 @@ func TestList(t *testing.T) { return } require.NoError(t, gErr) - assert.Empty(t, cmp.Diff(got, tc.res, protocmp.Transform(), protocmp.SortRepeated(func(x, y *pb.Credential) bool { - return x.Id < y.Id - }))) + assert.Empty(t, cmp.Diff( + got, + tc.res, + protocmp.Transform(), + protocmp.SortRepeated(func(x, y *pb.Credential) bool { + return x.Id < y.Id + }), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) // Test anonymous listing got, gErr = s.ListCredentials(auth.DisabledAuthTestContext(iamRepoFn, prj.GetPublicId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -388,7 +397,14 @@ func TestGet(t *testing.T) { return } require.NoError(t, gErr) - assert.Empty(t, cmp.Diff(got, tc.res, protocmp.Transform())) + assert.Empty(t, cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) // Test anonymous get got, gErr = s.GetCredential(auth.DisabledAuthTestContext(iamRepoFn, prj.GetPublicId(), auth.WithUserId(globals.AnonymousUserId)), req) @@ -829,7 +845,15 @@ func TestCreate(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform(), protocmp.SortRepeatedFields(got)), "CreateCredential(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + protocmp.SortRepeatedFields(got), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateCredential(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -1276,7 +1300,14 @@ func TestUpdate(t *testing.T) { assert.EqualValues(2, got.Item.Version) want.Item.Version = 2 - assert.Empty(cmp.Diff(got, want, protocmp.Transform())) + assert.Empty(cmp.Diff( + got, + want, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) }) } diff --git a/internal/daemon/controller/handlers/credentialstores/credentialstore_service.go b/internal/daemon/controller/handlers/credentialstores/credentialstore_service.go index 2c87588fc..813fb05e8 100644 --- a/internal/daemon/controller/handlers/credentialstores/credentialstore_service.go +++ b/internal/daemon/controller/handlers/credentialstores/credentialstore_service.go @@ -49,19 +49,19 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) vaultCollectionTypeMap = map[resource.Type]action.ActionSet{ resource.CredentialLibrary: credentiallibraries.CollectionActions, diff --git a/internal/daemon/controller/handlers/credentialstores/credentialstore_service_test.go b/internal/daemon/controller/handlers/credentialstores/credentialstore_service_test.go index 5ec481988..8fd327137 100644 --- a/internal/daemon/controller/handlers/credentialstores/credentialstore_service_test.go +++ b/internal/daemon/controller/handlers/credentialstores/credentialstore_service_test.go @@ -14,6 +14,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" credstatic "github.com/hashicorp/boundary/internal/credential/static" "github.com/hashicorp/boundary/internal/credential/vault" @@ -184,9 +185,20 @@ func TestList(t *testing.T) { return } require.NoError(t, gErr) - assert.Empty(t, cmp.Diff(got, tc.res, protocmp.Transform(), protocmp.SortRepeated(func(x, y *pb.CredentialStore) bool { - return x.Id < y.Id - }))) + assert.Empty(t, cmp.Diff( + got, + tc.res, + protocmp.Transform(), + protocmp.SortRepeated(func(x, y *pb.CredentialStore) bool { + return x.Id < y.Id + }), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + )) // Test anonymous listing got, gErr = s.ListCredentialStores(auth.DisabledAuthTestContext(iamRepoFn, tc.req.GetScopeId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -551,6 +563,12 @@ func TestCreateVault(t *testing.T) { cmpOptions := []cmp.Option{ protocmp.Transform(), protocmp.SortRepeatedFields(got), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), } if got != nil { assert.Contains(got.GetUri(), tc.res.Uri) @@ -728,6 +746,12 @@ func TestCreateStatic(t *testing.T) { cmpOptions := []cmp.Option{ protocmp.Transform(), protocmp.SortRepeatedFields(got), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), } if got != nil { assert.Contains(got.GetUri(), tc.res.Uri) @@ -870,7 +894,17 @@ func TestGet(t *testing.T) { return } require.NoError(t, gErr) - assert.Empty(t, cmp.Diff(got, tc.res, protocmp.Transform())) + assert.Empty(t, cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + )) // Test anonymous get got, gErr = s.GetCredentialStore(auth.DisabledAuthTestContext(iamRepoFn, prj.GetPublicId(), auth.WithUserId(globals.AnonymousUserId)), req) @@ -1221,7 +1255,17 @@ func TestUpdateVault(t *testing.T) { got.Item.GetVaultCredentialStoreAttributes().ClientCertificateKeyHmac = "" } - assert.Empty(cmp.Diff(got, want, protocmp.Transform())) + assert.Empty(cmp.Diff( + got, + want, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + )) }) } @@ -1412,7 +1456,17 @@ func TestUpdateStatic(t *testing.T) { assert.EqualValues(2, got.Item.Version) want.Item.Version = 2 - assert.Empty(cmp.Diff(got, want, protocmp.Transform())) + assert.Empty(cmp.Diff( + got, + want, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + )) }) } diff --git a/internal/daemon/controller/handlers/groups/group_service.go b/internal/daemon/controller/handlers/groups/group_service.go index 95fb1bfca..f67d24064 100644 --- a/internal/daemon/controller/handlers/groups/group_service.go +++ b/internal/daemon/controller/handlers/groups/group_service.go @@ -32,7 +32,7 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, @@ -40,14 +40,14 @@ var ( action.AddMembers, action.SetMembers, action.RemoveMembers, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) ) func init() { diff --git a/internal/daemon/controller/handlers/groups/group_service_test.go b/internal/daemon/controller/handlers/groups/group_service_test.go index 1372eeec4..cee910614 100644 --- a/internal/daemon/controller/handlers/groups/group_service_test.go +++ b/internal/daemon/controller/handlers/groups/group_service_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/daemon/controller/auth" "github.com/hashicorp/boundary/internal/daemon/controller/handlers" @@ -208,7 +209,14 @@ func TestGet(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "GetGroup(%+v) got error %v, wanted %v", req, gErr, tc.err) } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetGroup(%q) got response\n%q, wanted\n%q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetGroup(%q) got response\n%q, wanted\n%q", req, got, tc.res) }) } } @@ -333,7 +341,14 @@ func TestList(t *testing.T) { return } require.NoError(gErr) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListGroups(%q) got response %q, wanted %q", tc.req.GetScopeId(), got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListGroups(%q) got response %q, wanted %q", tc.req.GetScopeId(), got, tc.res) // Test the anon case got, gErr = s.ListGroups(auth.DisabledAuthTestContext(repoFn, tc.req.GetScopeId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -566,7 +581,14 @@ func TestCreate(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateGroup(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateGroup(%q) got response %q, wanted %q", req, got, tc.res) }) } } @@ -973,7 +995,14 @@ func TestUpdate(t *testing.T) { assert.Equal(ver+1, got.GetItem().GetVersion()) tc.res.Item.Version = ver + 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateGroup(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateGroup(%q) got response %q, wanted %q", req, got, tc.res) }) } } diff --git a/internal/daemon/controller/handlers/host_catalogs/host_catalog_service.go b/internal/daemon/controller/handlers/host_catalogs/host_catalog_service.go index b5cbd8ab3..b0070dd3e 100644 --- a/internal/daemon/controller/handlers/host_catalogs/host_catalog_service.go +++ b/internal/daemon/controller/handlers/host_catalogs/host_catalog_service.go @@ -43,19 +43,19 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) collectionTypeMap = map[globals.Subtype]map[resource.Type]action.ActionSet{ static.Subtype: { @@ -64,9 +64,9 @@ var ( }, hostplugin.Subtype: { resource.HostSet: host_sets.CollectionActions, - resource.Host: action.ActionSet{ + resource.Host: action.NewActionSet( action.List, - }, + ), }, } ) diff --git a/internal/daemon/controller/handlers/host_catalogs/host_catalog_service_test.go b/internal/daemon/controller/handlers/host_catalogs/host_catalog_service_test.go index f6c8c8a8a..a33ef8f5a 100644 --- a/internal/daemon/controller/handlers/host_catalogs/host_catalog_service_test.go +++ b/internal/daemon/controller/handlers/host_catalogs/host_catalog_service_test.go @@ -163,7 +163,17 @@ func TestGet_Static(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetHostCatalog(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "GetHostCatalog(%q) got response %q, wanted %q", req, got, tc.res) }) } } @@ -279,7 +289,17 @@ func TestGet_Plugin(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetHostCatalog(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "GetHostCatalog(%q) got response %q, wanted %q", req, got, tc.res) }) } } @@ -473,7 +493,17 @@ func TestList(t *testing.T) { return tc.res.Items[i].GetId() < tc.res.Items[j].GetId() }) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListHostCatalogs() for scope %q got response %q, wanted %q", tc.req.GetScopeId(), got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "ListHostCatalogs() for scope %q got response %q, wanted %q", tc.req.GetScopeId(), got, tc.res) // Test with anon user got, gErr = s.ListHostCatalogs(auth.DisabledAuthTestContext(iamRepoFn, tc.req.GetScopeId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -817,7 +847,17 @@ func TestCreate_Static(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateHostCatalog(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "CreateHostCatalog(%q) got response %q, wanted %q", req, got, tc.res) }) } } @@ -995,7 +1035,17 @@ func TestCreate_Plugin(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateHostCatalog(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "CreateHostCatalog(%q) got response %q, wanted %q", req, got, tc.res) }) } } @@ -1364,7 +1414,17 @@ func TestUpdate_Static(t *testing.T) { if tc.res != nil { tc.res.Item.Version = version + 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateHostCatalog(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "UpdateHostCatalog(%q) got response %q, wanted %q", req, got, tc.res) }) } } @@ -1481,7 +1541,17 @@ func TestUpdate_Plugin(t *testing.T) { check: func(t *testing.T, in *pb.HostCatalog) { assert.Equal(t, "new", in.Name.GetValue()) assert.Equal(t, "desc", in.Description.GetValue()) - assert.Empty(t, cmp.Diff(authorizedCollectionActions[hostplugin.Subtype], in.GetAuthorizedCollectionActions(), cmpopts.IgnoreUnexported(structpb.ListValue{}, structpb.Value{}))) + assert.Empty(t, cmp.Diff( + authorizedCollectionActions[hostplugin.Subtype], + in.GetAuthorizedCollectionActions(), + cmpopts.IgnoreUnexported(structpb.ListValue{}, structpb.Value{}), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + )) }, }, { diff --git a/internal/daemon/controller/handlers/host_sets/host_set_service.go b/internal/daemon/controller/handlers/host_sets/host_set_service.go index 533ceca18..8f3dadc11 100644 --- a/internal/daemon/controller/handlers/host_sets/host_set_service.go +++ b/internal/daemon/controller/handlers/host_sets/host_set_service.go @@ -40,7 +40,7 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources idActionsTypeMap = map[globals.Subtype]action.ActionSet{ - static.Subtype: { + static.Subtype: action.NewActionSet( action.NoOp, action.Read, action.Update, @@ -48,21 +48,21 @@ var ( action.AddHosts, action.SetHosts, action.RemoveHosts, - }, - hostplugin.Subtype: { + ), + hostplugin.Subtype: action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - }, + ), } // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) ) const domain = "host" diff --git a/internal/daemon/controller/handlers/host_sets/host_set_service_test.go b/internal/daemon/controller/handlers/host_sets/host_set_service_test.go index 3184a12ac..89f161e27 100644 --- a/internal/daemon/controller/handlers/host_sets/host_set_service_test.go +++ b/internal/daemon/controller/handlers/host_sets/host_set_service_test.go @@ -13,6 +13,7 @@ import ( "github.com/golang/protobuf/ptypes/wrappers" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/daemon/controller/auth" "github.com/hashicorp/boundary/internal/daemon/controller/handlers" @@ -139,7 +140,14 @@ func TestGet_Static(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetHostSet(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetHostSet(%q) got response %q, wanted %q", req, got, tc.res) }) } } @@ -258,7 +266,14 @@ func TestGet_Plugin(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got.GetItem(), tc.res.GetItem(), protocmp.Transform()), "GetHostSet(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got.GetItem(), + tc.res.GetItem(), + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetHostSet(%q) got response %q, wanted %q", req, got, tc.res) }) } } @@ -347,7 +362,14 @@ func TestList_Static(t *testing.T) { return } require.NoError(gErr) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListHostSets(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListHostSets(%q) got response %q, wanted %q", tc.req, got, tc.res) // Test with anon user got, gErr = s.ListHostSets(auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -461,7 +483,14 @@ func TestList_Plugin(t *testing.T) { return } require.NoError(gErr) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListHostSets(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListHostSets(%q) got response %q, wanted %q", tc.req, got, tc.res) // Test with anon user got, gErr = s.ListHostSets(auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -544,7 +573,14 @@ func TestDelete_Static(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "DeleteHostSet(%+v) got error %v, wanted %v", tc.req, gErr, tc.err) } - assert.Empty(cmp.Diff(tc.res, got, protocmp.Transform()), "DeleteHostSet(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + tc.res, + got, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "DeleteHostSet(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -622,7 +658,14 @@ func TestDelete_Plugin(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "DeleteHostSet(%+v) got error %v, wanted %v", tc.req, gErr, tc.err) } - assert.Empty(cmp.Diff(tc.res, got, protocmp.Transform()), "DeleteHostSet(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + tc.res, + got, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "DeleteHostSet(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -818,7 +861,14 @@ func TestCreate_Static(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateHostSet(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateHostSet(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -1086,7 +1136,14 @@ func TestCreate_Plugin(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateHostSet(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateHostSet(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -1468,7 +1525,14 @@ func TestUpdate_Static(t *testing.T) { }) } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateHostSet(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateHostSet(%q) got response %q, wanted %q", req, got, tc.res) }) } } diff --git a/internal/daemon/controller/handlers/hosts/host_service.go b/internal/daemon/controller/handlers/hosts/host_service.go index 545a2e600..8eeef2617 100644 --- a/internal/daemon/controller/handlers/hosts/host_service.go +++ b/internal/daemon/controller/handlers/hosts/host_service.go @@ -37,24 +37,24 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources idActionsTypeMap = map[globals.Subtype]action.ActionSet{ - static.Subtype: { + static.Subtype: action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - }, - hostplugin.Subtype: { + ), + hostplugin.Subtype: action.NewActionSet( action.NoOp, action.Read, - }, + ), } // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) ) const domain = "host" diff --git a/internal/daemon/controller/handlers/hosts/host_service_test.go b/internal/daemon/controller/handlers/hosts/host_service_test.go index 13dd0189c..8c2a02123 100644 --- a/internal/daemon/controller/handlers/hosts/host_service_test.go +++ b/internal/daemon/controller/handlers/hosts/host_service_test.go @@ -13,6 +13,7 @@ import ( "github.com/golang/protobuf/ptypes/wrappers" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/daemon/controller/auth" "github.com/hashicorp/boundary/internal/daemon/controller/handlers" @@ -134,7 +135,14 @@ func TestGet_Static(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetHost(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetHost(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -253,7 +261,14 @@ func TestGet_Plugin(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetHost(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetHost(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -359,7 +374,14 @@ func TestList_Static(t *testing.T) { return } require.NoError(gErr) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListHosts(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListHosts(%q) got response %q, wanted %q", tc.req, got, tc.res) // Test anonymous listing got, gErr = s.ListHosts(auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -486,7 +508,14 @@ func TestList_Plugin(t *testing.T) { sort.Slice(got.Items, func(i, j int) bool { return got.Items[i].GetId() < got.Items[j].GetId() }) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListHosts(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListHosts(%q) got response %q, wanted %q", tc.req, got, tc.res) // Test anonymous listing got, gErr = s.ListHosts(auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -580,7 +609,14 @@ func TestDelete(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "DeleteHost(%+v) got error %v, wanted %v", tc.req, gErr, tc.err) } - assert.Empty(cmp.Diff(tc.res, got, protocmp.Transform()), "DeleteHost(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + tc.res, + got, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "DeleteHost(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -856,7 +892,14 @@ func TestCreate(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateHost(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateHost(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -1324,7 +1367,14 @@ func TestUpdate_Static(t *testing.T) { // Clear all values which are hard to compare against. got.Item.UpdatedTime, tc.res.Item.UpdatedTime = nil, nil tc.res.Item.Version = version + 1 - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateHost(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateHost(%q) got response %q, wanted %q", req, got, tc.res) }) } } diff --git a/internal/daemon/controller/handlers/managed_groups/managed_group_service.go b/internal/daemon/controller/handlers/managed_groups/managed_group_service.go index b4bd7b2be..55e9466fc 100644 --- a/internal/daemon/controller/handlers/managed_groups/managed_group_service.go +++ b/internal/daemon/controller/handlers/managed_groups/managed_group_service.go @@ -45,26 +45,26 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources IdActions = map[globals.Subtype]action.ActionSet{ - oidc.Subtype: { + oidc.Subtype: action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - }, - ldap.Subtype: { + ), + ldap.Subtype: action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - }, + ), } // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) ) func init() { diff --git a/internal/daemon/controller/handlers/managed_groups/managed_group_service_test.go b/internal/daemon/controller/handlers/managed_groups/managed_group_service_test.go index 6c5e306b9..293b60937 100644 --- a/internal/daemon/controller/handlers/managed_groups/managed_group_service_test.go +++ b/internal/daemon/controller/handlers/managed_groups/managed_group_service_test.go @@ -13,6 +13,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/auth/ldap" "github.com/hashicorp/boundary/internal/auth/oidc" @@ -252,7 +253,14 @@ func TestGet(t *testing.T) { return } require.NoError(gErr) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetManagedGroup(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetManagedGroup(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -393,7 +401,14 @@ func TestListOidc(t *testing.T) { return strings.Compare(got.Items[i].GetName().GetValue(), got.Items[j].GetName().GetValue()) < 0 }) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListManagedGroups() with scope %q got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListManagedGroups() with scope %q got response %q, wanted %q", tc.req, got, tc.res) // Now test with anon if tc.skipAnon { @@ -551,7 +566,14 @@ func TestListLdap(t *testing.T) { return strings.Compare(got.Items[i].GetName().GetValue(), got.Items[j].GetName().GetValue()) < 0 }) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListManagedGroups() with scope %q got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListManagedGroups() with scope %q got response %q, wanted %q", tc.req, got, tc.res) // Now test with anon if tc.skipAnon { @@ -909,7 +931,14 @@ func TestCreateOidc(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateManagedGroup(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateManagedGroup(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -1115,7 +1144,14 @@ func TestCreateLdap(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateManagedGroup(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateManagedGroup(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -1503,7 +1539,14 @@ func TestUpdateOidc(t *testing.T) { assert.EqualValues(2, got.Item.Version) tc.res.Item.Version = 2 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateManagedGroup(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateManagedGroup(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -1897,7 +1940,14 @@ func TestUpdateLdap(t *testing.T) { assert.EqualValues(2, got.Item.Version) tc.res.Item.Version = 2 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateManagedGroup(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateManagedGroup(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } diff --git a/internal/daemon/controller/handlers/roles/role_service.go b/internal/daemon/controller/handlers/roles/role_service.go index 246508648..fb88f43dd 100644 --- a/internal/daemon/controller/handlers/roles/role_service.go +++ b/internal/daemon/controller/handlers/roles/role_service.go @@ -33,7 +33,7 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, @@ -44,14 +44,14 @@ var ( action.AddGrants, action.SetGrants, action.RemoveGrants, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) ) func init() { diff --git a/internal/daemon/controller/handlers/roles/role_service_test.go b/internal/daemon/controller/handlers/roles/role_service_test.go index e332fa01e..687efabb4 100644 --- a/internal/daemon/controller/handlers/roles/role_service_test.go +++ b/internal/daemon/controller/handlers/roles/role_service_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/auth/ldap" "github.com/hashicorp/boundary/internal/auth/oidc" @@ -185,7 +186,14 @@ func TestGet(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "GetRole(%+v) got error %v, wanted %v", req, gErr, tc.err) } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetRole(%q) got response\n%q, wanted\n%q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetRole(%q) got response\n%q, wanted\n%q", req, got, tc.res) }) } } @@ -311,7 +319,14 @@ func TestList(t *testing.T) { return } require.NoError(gErr) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListRoles(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListRoles(%q) got response %q, wanted %q", tc.req, got, tc.res) // Test the anon case got, gErr = s.ListRoles(auth.DisabledAuthTestContext(repoFn, tc.req.GetScopeId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -566,7 +581,14 @@ func TestCreate(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateRole(%q) got response\n%q, wanted\n%q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateRole(%q) got response\n%q, wanted\n%q", req, got, tc.res) }) } } @@ -1009,7 +1031,14 @@ func TestUpdate(t *testing.T) { assert.Equal(ver+1, got.GetItem().GetVersion()) tc.res.Item.Version = ver + 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateRole(%q) got response\n%q,\nwanted\n%q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateRole(%q) got response\n%q,\nwanted\n%q", req, got, tc.res) }) } } diff --git a/internal/daemon/controller/handlers/scopes/scope_service.go b/internal/daemon/controller/handlers/scopes/scope_service.go index eaa165cf0..c87313090 100644 --- a/internal/daemon/controller/handlers/scopes/scope_service.go +++ b/internal/daemon/controller/handlers/scopes/scope_service.go @@ -49,23 +49,23 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, action.ListScopeKeys, action.RotateScopeKeys, action.ListScopeKeyVersionDestructionJobs, action.DestroyScopeKeyVersion, - } + ) scopeCollectionTypeMapMap = map[string]map[resource.Type]action.ActionSet{ scope.Global.String(): { @@ -96,9 +96,14 @@ var ( resource.Group: groups.CollectionActions, resource.HostCatalog: host_catalogs.CollectionActions, resource.Role: roles.CollectionActions, - resource.Scope: CollectionActions[2:], // Only Scope key actions are allowed on the project level - resource.Session: sessions.CollectionActions, - resource.Target: targets.CollectionActions, + resource.Scope: action.NewActionSet( + action.ListScopeKeys, + action.RotateScopeKeys, + action.ListScopeKeyVersionDestructionJobs, + action.DestroyScopeKeyVersion, + ), // Only Scope key actions are allowed on the project level + resource.Session: sessions.CollectionActions, + resource.Target: targets.CollectionActions, }, } ) @@ -252,7 +257,7 @@ func (s Service) GetScope(ctx context.Context, req *pbs.GetScopeRequest) (*pbs.G act := IdActions // Can't delete global so elide it if p.GetPublicId() == scope.Global.String() { - act = act[0:3] + act = action.Difference(act, action.NewActionSet(action.Delete)) } if outputFields.Has(globals.AuthorizedActionsField) { outputOpts = append(outputOpts, handlers.WithAuthorizedActions(authResults.FetchActionSetForId(ctx, p.GetPublicId(), act).Strings())) diff --git a/internal/daemon/controller/handlers/scopes/scope_service_test.go b/internal/daemon/controller/handlers/scopes/scope_service_test.go index 3a070c8ce..b9f67796f 100644 --- a/internal/daemon/controller/handlers/scopes/scope_service_test.go +++ b/internal/daemon/controller/handlers/scopes/scope_service_test.go @@ -13,6 +13,7 @@ import ( "github.com/golang/protobuf/ptypes/wrappers" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/auth/oidc" "github.com/hashicorp/boundary/internal/authtoken" @@ -98,11 +99,11 @@ var globalAuthorizedCollectionActions = map[string]*structpb.ListValue{ "scopes": { Values: []*structpb.Value{ structpb.NewStringValue("create"), + structpb.NewStringValue("destroy-key-version"), structpb.NewStringValue("list"), + structpb.NewStringValue("list-key-version-destruction-jobs"), structpb.NewStringValue("list-keys"), structpb.NewStringValue("rotate-keys"), - structpb.NewStringValue("list-key-version-destruction-jobs"), - structpb.NewStringValue("destroy-key-version"), }, }, "session-recordings": { @@ -160,11 +161,11 @@ var orgAuthorizedCollectionActions = map[string]*structpb.ListValue{ "scopes": { Values: []*structpb.Value{ structpb.NewStringValue("create"), + structpb.NewStringValue("destroy-key-version"), structpb.NewStringValue("list"), + structpb.NewStringValue("list-key-version-destruction-jobs"), structpb.NewStringValue("list-keys"), structpb.NewStringValue("rotate-keys"), - structpb.NewStringValue("list-key-version-destruction-jobs"), - structpb.NewStringValue("destroy-key-version"), }, }, "session-recordings": { @@ -218,10 +219,10 @@ var projectAuthorizedCollectionActions = map[string]*structpb.ListValue{ }, "scopes": { Values: []*structpb.Value{ + structpb.NewStringValue("destroy-key-version"), + structpb.NewStringValue("list-key-version-destruction-jobs"), structpb.NewStringValue("list-keys"), structpb.NewStringValue("rotate-keys"), - structpb.NewStringValue("list-key-version-destruction-jobs"), - structpb.NewStringValue("destroy-key-version"), }, }, "targets": { @@ -329,7 +330,17 @@ func TestGet(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "GetScope(%+v) got error\n%v, wanted\n%v", req, gErr, tc.err) } - assert.Empty(cmp.Diff(tc.res, got, protocmp.Transform()), "GetScope(%q) got response\n%q, wanted\n%q", req, got, tc.res) + assert.Empty(cmp.Diff( + tc.res, + got, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "GetScope(%q) got response\n%q, wanted\n%q", req, got, tc.res) }) } } @@ -414,7 +425,17 @@ func TestList(t *testing.T) { assert.True(errors.Is(gErr, tc.err), "ListScopes(%+v) got error\n%v, wanted\n%v", tc.req, gErr, tc.err) return } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListScopes(%q) got response\n%q\nwanted\n%q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "ListScopes(%q) got response\n%q\nwanted\n%q", tc.req, got, tc.res) // Now test with anonymous listing got, gErr = s.ListScopes(auth.DisabledAuthTestContext(repoFn, tc.scopeId, auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -529,7 +550,17 @@ func TestList(t *testing.T) { return } require.NoError(gErr) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListScopes(%q) got response\n%q, wanted\n%q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "ListScopes(%q) got response\n%q, wanted\n%q", tc.req, got, tc.res) // Now test with anonymous listing got, gErr = s.ListScopes(auth.DisabledAuthTestContext(repoFn, tc.scopeId, auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -965,7 +996,17 @@ func TestCreate(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(tc.res, got, protocmp.Transform()), "CreateScope(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + tc.res, + got, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "CreateScope(%q) got response %q, wanted %q", req, got, tc.res) }) } } @@ -1539,7 +1580,17 @@ func TestUpdate(t *testing.T) { assert.Equal(ver+1, got.GetItem().GetVersion()) tc.res.Item.Version = ver + 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateScope(%q) got response\n%q, wanted\n%q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "UpdateScope(%q) got response\n%q, wanted\n%q", req, got, tc.res) }) } } @@ -2027,6 +2078,9 @@ func TestListKeys(t *testing.T) { protocmp.SortRepeated(func(i, j *pb.Key) bool { return i.GetPurpose() < j.GetPurpose() }), protocmp.IgnoreFields(&pb.Key{}, "id", "created_time"), protocmp.IgnoreFields(&pb.KeyVersion{}, "id", "created_time"), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), ), "ListKeys(%q) got response\n%q, wanted\n%q", tt.req, got, tt.res, ) @@ -2408,6 +2462,12 @@ func TestListKeyVersionDestructionJobs(t *testing.T) { protocmp.Transform(), protocmp.SortRepeated(func(i, j *pb.KeyVersionDestructionJob) bool { return i.GetTotalCount() < j.GetTotalCount() }), protocmp.IgnoreFields(&pb.KeyVersionDestructionJob{}, "key_version_id", "created_time"), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), ), "ListKeyVersionDestructionJobs(%q) got response\n%q, wanted\n%q", tt.req, got, tt.res, ) @@ -2658,7 +2718,17 @@ func TestDestroyKeyVersion(t *testing.T) { require.NoError(gErr) } - assert.Empty(cmp.Diff(tt.res, got, protocmp.Transform()), "DestroyKeyVersion(%q) got response\n%q, wanted\n%q", tt.req, got, tt.res) + assert.Empty(cmp.Diff( + tt.res, + got, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + cmpopts.SortSlices(func(a, b protocmp.Message) bool { + return a.String() < b.String() + }), + ), "DestroyKeyVersion(%q) got response\n%q, wanted\n%q", tt.req, got, tt.res) }) } } diff --git a/internal/daemon/controller/handlers/session_recordings/session_recording_service.go b/internal/daemon/controller/handlers/session_recordings/session_recording_service.go index 63556f95e..5cff170a3 100644 --- a/internal/daemon/controller/handlers/session_recordings/session_recording_service.go +++ b/internal/daemon/controller/handlers/session_recordings/session_recording_service.go @@ -19,17 +19,17 @@ import ( var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Download, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.List, - } + ) ) // NewServiceFn returns a storage bucket service which is not implemented in OSS diff --git a/internal/daemon/controller/handlers/sessions/session_service.go b/internal/daemon/controller/handlers/sessions/session_service.go index 2032234ea..589b4070e 100644 --- a/internal/daemon/controller/handlers/sessions/session_service.go +++ b/internal/daemon/controller/handlers/sessions/session_service.go @@ -29,19 +29,19 @@ import ( var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.ReadSelf, action.Cancel, action.CancelSelf, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.List, - } + ) ) // Service handles request as described by the pbs.SessionServiceServer interface. diff --git a/internal/daemon/controller/handlers/sessions/session_service_test.go b/internal/daemon/controller/handlers/sessions/session_service_test.go index f67027822..f121285a8 100644 --- a/internal/daemon/controller/handlers/sessions/session_service_test.go +++ b/internal/daemon/controller/handlers/sessions/session_service_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/authtoken" "github.com/hashicorp/boundary/internal/daemon/controller/auth" @@ -158,7 +159,14 @@ func TestGetSession(t *testing.T) { assert.True(got.GetItem().GetExpirationTime().AsTime().Sub(tc.res.GetItem().GetExpirationTime().AsTime()) < 10*time.Millisecond) tc.res.GetItem().ExpirationTime = got.GetItem().GetExpirationTime() } - assert.Empty(cmp.Diff(tc.res, got, protocmp.Transform()), "GetSession(%q) got response\n%q, wanted\n%q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + tc.res, + got, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetSession(%q) got response\n%q, wanted\n%q", tc.req, got, tc.res) }) } } @@ -544,7 +552,14 @@ func TestList(t *testing.T) { wantSess.ExpirationTime = got.GetItems()[i].GetExpirationTime() } } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListSessions(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListSessions(%q) got response %q, wanted %q", tc.req, got, tc.res) // Test with other user otherRequestInfo := authpb.RequestInfo{ @@ -760,7 +775,14 @@ func TestCancel(t *testing.T) { EndTime: got.GetItem().GetUpdatedTime(), }, } - assert.Empty(cmp.Diff(got.GetItem().GetStates(), wantState, protocmp.Transform()), "CancelSession(%q) states") + assert.Empty(cmp.Diff( + got.GetItem().GetStates(), + wantState, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CancelSession(%q) states") got.GetItem().States = nil got.GetItem().UpdatedTime = nil @@ -771,7 +793,14 @@ func TestCancel(t *testing.T) { assert.Equal(got.GetItem().HostId, tc.res.GetItem().HostId) assert.Equal(got.GetItem().HostSetId, tc.res.GetItem().HostSetId) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CancelSession(%q) got response\n%q, wanted\n%q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CancelSession(%q) got response\n%q, wanted\n%q", tc.req, got, tc.res) if tc.req != nil { require.NotNil(got) diff --git a/internal/daemon/controller/handlers/storage_buckets/storage_bucket_service.go b/internal/daemon/controller/handlers/storage_buckets/storage_bucket_service.go index 02544a5bc..62eb333b7 100644 --- a/internal/daemon/controller/handlers/storage_buckets/storage_bucket_service.go +++ b/internal/daemon/controller/handlers/storage_buckets/storage_bucket_service.go @@ -17,19 +17,19 @@ import ( var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, action.Delete, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) ) // NewServiceFn returns a storage bucket service which is not implemented in OSS diff --git a/internal/daemon/controller/handlers/targets/target_service.go b/internal/daemon/controller/handlers/targets/target_service.go index a1e5a2ae1..fdccef245 100644 --- a/internal/daemon/controller/handlers/targets/target_service.go +++ b/internal/daemon/controller/handlers/targets/target_service.go @@ -74,7 +74,7 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, @@ -86,14 +86,14 @@ var ( action.SetCredentialSources, action.RemoveCredentialSources, action.AuthorizeSession, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) validateCredentialSourcesFn = func(context.Context, globals.Subtype, []target.CredentialSource) error { return nil } ValidateIngressWorkerFilterFn = IngressWorkerFilterUnsupported diff --git a/internal/daemon/controller/handlers/targets/tcp/target_service_test.go b/internal/daemon/controller/handlers/targets/tcp/target_service_test.go index 2b41fff62..c64602718 100644 --- a/internal/daemon/controller/handlers/targets/tcp/target_service_test.go +++ b/internal/daemon/controller/handlers/targets/tcp/target_service_test.go @@ -17,6 +17,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/auth/ldap" "github.com/hashicorp/boundary/internal/auth/oidc" @@ -241,7 +242,14 @@ func TestGet(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetTarget(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetTarget(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -390,7 +398,14 @@ func TestList(t *testing.T) { for _, t := range got.Items { want, ok := wantById[t.Id] assert.True(ok, "Got unexpected target with id: %s", t.Id) - assert.Empty(cmp.Diff(t, want, protocmp.Transform()), "got %v, wanted %v", t, want) + assert.Empty(cmp.Diff( + t, + want, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "got %v, wanted %v", t, want) } // Test with anon user @@ -490,7 +505,14 @@ func TestDelete(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "DeleteTarget(%+v) got error %v, wanted %v", tc.req, gErr, tc.err) } - assert.Empty(cmp.Diff(tc.res, got, protocmp.Transform()), "DeleteTarget(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + tc.res, + got, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "DeleteTarget(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -772,7 +794,14 @@ func TestCreate(t *testing.T) { if tc.res != nil { tc.res.Item.Version = 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateTarget(%q)\n got response %q\n, wanted %q\n", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateTarget(%q)\n got response %q\n, wanted %q\n", tc.req, got, tc.res) }) } // Reset worker filter func @@ -1266,7 +1295,14 @@ func TestUpdate(t *testing.T) { if tc.res != nil { tc.res.Item.Version = tc.req.Item.Version + 1 } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "UpdateTarget(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateTarget(%q) got response %q, wanted %q", req, got, tc.res) }) } // Reset worker filter funcs @@ -2624,7 +2660,14 @@ func TestAuthorizeSession(t *testing.T) { Id: tar.GetPublicId(), }) require.NoError(t, err) - assert.NotEmpty(t, cmp.Diff(asRes1.GetItem().GetCredentials(), asRes2.GetItem().GetCredentials(), protocmp.Transform()), + assert.NotEmpty(t, cmp.Diff( + asRes1.GetItem().GetCredentials(), + asRes2.GetItem().GetCredentials(), + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "the credentials aren't unique per request authorized session") _, err = s.AuthorizeSession(ctx, &pbs.AuthorizeSessionRequest{ @@ -2700,7 +2743,15 @@ func TestAuthorizeSession(t *testing.T) { gotCred.Secret = nil got.AuthorizationToken, got.SessionId, got.CreatedTime = "", "", nil - assert.Empty(t, cmp.Diff(got, want, protocmp.Transform(), protocmp.IgnoreFields(&pb.SessionAuthorization{}, "expiration"))) + assert.Empty(t, cmp.Diff( + got, + want, + protocmp.Transform(), + protocmp.IgnoreFields(&pb.SessionAuthorization{}, "expiration"), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) }) } } @@ -3293,7 +3344,14 @@ func TestAuthorizeSessionTypedCredentials(t *testing.T) { gotCred.Secret = nil got.AuthorizationToken, got.SessionId, got.CreatedTime = "", "", nil - assert.Empty(t, cmp.Diff(got, want, protocmp.Transform())) + assert.Empty(t, cmp.Diff( + got, + want, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) }) } } diff --git a/internal/daemon/controller/handlers/users/user_service.go b/internal/daemon/controller/handlers/users/user_service.go index b7380fb81..56b5d099c 100644 --- a/internal/daemon/controller/handlers/users/user_service.go +++ b/internal/daemon/controller/handlers/users/user_service.go @@ -32,7 +32,7 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, @@ -40,14 +40,14 @@ var ( action.AddAccounts, action.SetAccounts, action.RemoveAccounts, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.Create, action.List, - } + ) ) func init() { diff --git a/internal/daemon/controller/handlers/users/user_service_test.go b/internal/daemon/controller/handlers/users/user_service_test.go index 77fa88b2b..45c99df4e 100644 --- a/internal/daemon/controller/handlers/users/user_service_test.go +++ b/internal/daemon/controller/handlers/users/user_service_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/auth/ldap" "github.com/hashicorp/boundary/internal/auth/oidc" @@ -152,7 +153,14 @@ func TestGet(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "GetUser(%+v) got error %v, wanted %v", req, gErr, tc.err) } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "GetUser(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetUser(%q) got response %q, wanted %q", req, got, tc.res) }) } } @@ -304,7 +312,14 @@ func TestList(t *testing.T) { sort.Sort(resUsers) tc.res.Items = resUsers.users } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListUsers(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListUsers(%q) got response %q, wanted %q", tc.req, got, tc.res) // Test with anon user got, gErr = s.ListUsers(auth.DisabledAuthTestContext(repoFn, tc.req.GetScopeId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -493,7 +508,14 @@ func TestCreate(t *testing.T) { got.Item.Id, tc.res.Item.Id = "", "" got.Item.CreatedTime, got.Item.UpdatedTime, tc.res.Item.CreatedTime, tc.res.Item.UpdatedTime = nil, nil, nil, nil } - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "CreateUser(%q) got response %q, wanted %q", tc.req, got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "CreateUser(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } @@ -763,7 +785,14 @@ func TestUpdate(t *testing.T) { assert.Equal(version+1, got.GetItem().GetVersion()) tc.res.Item.Version = version + 1 } - assert.Empty(cmp.Diff(tc.res, got, protocmp.Transform()), "UpdateUser(%q) got response %q, wanted %q", req, got, tc.res) + assert.Empty(cmp.Diff( + tc.res, + got, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateUser(%q) got response %q, wanted %q", req, got, tc.res) }) } } diff --git a/internal/daemon/controller/handlers/workers/worker_service.go b/internal/daemon/controller/handlers/workers/worker_service.go index cc6118a35..a62f15911 100644 --- a/internal/daemon/controller/handlers/workers/worker_service.go +++ b/internal/daemon/controller/handlers/workers/worker_service.go @@ -47,7 +47,7 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources - IdActions = action.ActionSet{ + IdActions = action.NewActionSet( action.NoOp, action.Read, action.Update, @@ -55,17 +55,17 @@ var ( action.AddWorkerTags, action.SetWorkerTags, action.RemoveWorkerTags, - } + ) // CollectionActions contains the set of actions that can be performed on // this collection - CollectionActions = action.ActionSet{ + CollectionActions = action.NewActionSet( action.CreateControllerLed, action.CreateWorkerLed, action.List, action.ReadCertificateAuthority, action.ReinitializeCertificateAuthority, - } + ) // downstreamWorkers returns a list of worker ids which are directly // connected downstream of the provided worker. downstreamWorkers = emptyDownstreamWorkers diff --git a/internal/daemon/controller/handlers/workers/worker_service_test.go b/internal/daemon/controller/handlers/workers/worker_service_test.go index d47683332..f50af2633 100644 --- a/internal/daemon/controller/handlers/workers/worker_service_test.go +++ b/internal/daemon/controller/handlers/workers/worker_service_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/globals" wl "github.com/hashicorp/boundary/internal/daemon/common" "github.com/hashicorp/boundary/internal/daemon/controller/auth" @@ -294,7 +295,14 @@ func TestGet(t *testing.T) { } else { require.NoError(t, err) } - assert.Empty(t, cmp.Diff(got, tc.res, protocmp.Transform()), "GetWorker(%q) got response\n%q, wanted\n%q", tc.req, got, tc.res) + assert.Empty(t, cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "GetWorker(%q) got response\n%q, wanted\n%q", tc.req, got, tc.res) }) } } @@ -435,7 +443,14 @@ func TestList(t *testing.T) { sort.Slice(tc.res.Items, func(i, j int) bool { return tc.res.Items[i].GetName().GetValue() < tc.res.Items[j].GetName().GetValue() }) - assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListWorkers(%q) got response %q, wanted %q", tc.req.GetScopeId(), got, tc.res) + assert.Empty(cmp.Diff( + got, + tc.res, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "ListWorkers(%q) got response %q, wanted %q", tc.req.GetScopeId(), got, tc.res) // Test the anon case got, gErr = s.ListWorkers(auth.DisabledAuthTestContext(iamRepoFn, tc.req.GetScopeId(), auth.WithUserId(globals.AnonymousUserId)), tc.req) @@ -1046,7 +1061,14 @@ func TestUpdate(t *testing.T) { expRes.Item.Version = tc.req.Item.Version + 1 - assert.Empty(t, cmp.Diff(got, expRes, protocmp.Transform()), "UpdateWorker(%q) got response %q, wanted %q", req, got, expRes) + assert.Empty(t, cmp.Diff( + got, + expRes, + protocmp.Transform(), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), "UpdateWorker(%q) got response %q, wanted %q", req, got, expRes) }) } } diff --git a/internal/perms/acl.go b/internal/perms/acl.go index 9974e105e..17cc53e46 100644 --- a/internal/perms/acl.go +++ b/internal/perms/acl.go @@ -316,7 +316,7 @@ func (a ACL) ListPermissions(requestedScopes map[string]*scopes.ScopeInfo, if ok := grant.actions[action.All]; ok { found = true } else { - for _, a := range idActions { + for a := range idActions { if ok := grant.actions[a]; ok { found = true break @@ -328,10 +328,10 @@ func (a ACL) ListPermissions(requestedScopes map[string]*scopes.ScopeInfo, } actions, _ := grant.Actions() - excludeList := make(action.ActionSet, 0, len(actions)) + excludeList := make(action.ActionSet, len(actions)) for _, aa := range actions { if aa != action.List { - excludeList = append(excludeList, aa) + excludeList.Add(aa) } } p.OnlySelf = p.OnlySelf && excludeList.OnlySelf() diff --git a/internal/perms/acl_test.go b/internal/perms/acl_test.go index b1d3e4709..f45c76385 100644 --- a/internal/perms/acl_test.go +++ b/internal/perms/acl_test.go @@ -440,7 +440,7 @@ func TestACL_ListPermissions(t *testing.T) { "o_this_one_too": nil, }, resourceType: resource.Session, - actionSet: action.ActionSet{action.Read}, + actionSet: action.NewActionSet(action.Read), expPermissions: []Permission{}, }, { @@ -453,7 +453,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, // We're requesting sessions. - actionSet: action.ActionSet{action.Read}, + actionSet: action.NewActionSet(action.Read), expPermissions: []Permission{}, }, { @@ -466,7 +466,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.Read}, + actionSet: action.NewActionSet(action.Read), expPermissions: []Permission{}, }, { @@ -479,7 +479,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.Read}, + actionSet: action.NewActionSet(action.Read), expPermissions: []Permission{}, skipGrantValidationChecking: true, }, @@ -493,7 +493,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.Read}, + actionSet: action.NewActionSet(action.Read), expPermissions: []Permission{ { ScopeId: "o_1", @@ -515,7 +515,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.ReadSelf}, + actionSet: action.NewActionSet(action.ReadSelf), expPermissions: []Permission{ { ScopeId: "o_1", @@ -540,7 +540,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.Read}, + actionSet: action.NewActionSet(action.Read), expPermissions: []Permission{ { ScopeId: "o_1", @@ -562,7 +562,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.ReadSelf}, + actionSet: action.NewActionSet(action.ReadSelf), expPermissions: []Permission{ { ScopeId: "o_1", @@ -584,7 +584,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.NoOp}, + actionSet: action.NewActionSet(action.NoOp), expPermissions: []Permission{ { ScopeId: "o_1", @@ -606,7 +606,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.Read}, + actionSet: action.NewActionSet(action.Read), expPermissions: []Permission{}, }, { @@ -622,7 +622,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.NoOp}, + actionSet: action.NewActionSet(action.NoOp), expPermissions: []Permission{ { ScopeId: "o_1", @@ -644,7 +644,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Host, - actionSet: action.ActionSet{action.ReadSelf}, + actionSet: action.NewActionSet(action.ReadSelf), expPermissions: []Permission{ { ScopeId: "o_1", @@ -669,7 +669,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.NoOp, action.Read, action.ReadSelf, action.Cancel, action.CancelSelf}, + actionSet: action.NewActionSet(action.NoOp, action.Read, action.ReadSelf, action.Cancel, action.CancelSelf), expPermissions: []Permission{ { ScopeId: "o_1", @@ -691,7 +691,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.Read, action.Create, action.Delete}, + actionSet: action.NewActionSet(action.Read, action.Create, action.Delete), expPermissions: []Permission{ { ScopeId: "o_1", @@ -713,7 +713,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil}, resourceType: resource.Host, - actionSet: action.ActionSet{action.Read, action.Create, action.Delete}, + actionSet: action.NewActionSet(action.Read, action.Create, action.Delete), expPermissions: []Permission{ { ScopeId: "o_1", @@ -742,7 +742,7 @@ func TestACL_ListPermissions(t *testing.T) { }, scopes: map[string]*scopes.ScopeInfo{"o_1": nil, "o_2": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.Read, action.ReadSelf}, + actionSet: action.NewActionSet(action.Read, action.ReadSelf), expPermissions: []Permission{ { ScopeId: "o_1", @@ -767,7 +767,7 @@ func TestACL_ListPermissions(t *testing.T) { userId: globals.RecoveryUserId, scopes: map[string]*scopes.ScopeInfo{"o_1": nil, "o_2": nil}, resourceType: resource.Session, - actionSet: action.ActionSet{action.Read, action.Create, action.Delete}, + actionSet: action.NewActionSet(action.Read, action.Create, action.Delete), expPermissions: []Permission{ { ScopeId: "o_1", @@ -792,7 +792,7 @@ func TestACL_ListPermissions(t *testing.T) { userId: globals.RecoveryUserId, scopes: map[string]*scopes.ScopeInfo{"o_1": nil, "o_2": nil}, resourceType: resource.Target, - actionSet: action.ActionSet{action.Read, action.Create, action.Delete}, + actionSet: action.NewActionSet(action.Read, action.Create, action.Delete), expPermissions: []Permission{ { ScopeId: "o_1", @@ -816,7 +816,7 @@ func TestACL_ListPermissions(t *testing.T) { name: "separate_type_id_resource_grants", scopes: map[string]*scopes.ScopeInfo{"p_1": nil}, resourceType: resource.Target, - actionSet: action.ActionSet{action.Read, action.Cancel}, + actionSet: action.NewActionSet(action.Read, action.Cancel), aclGrants: []scopeGrant{ { scope: "p_1", diff --git a/internal/tests/api/authmethods/authmethod_test.go b/internal/tests/api/authmethods/authmethod_test.go index 4ac555c1e..dc585dac6 100644 --- a/internal/tests/api/authmethods/authmethod_test.go +++ b/internal/tests/api/authmethods/authmethod_test.go @@ -225,6 +225,9 @@ func TestList(t *testing.T) { cmpopts.SortSlices(func(a, b *authmethods.AuthMethod) bool { return a.Name < b.Name }), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), ), ) @@ -251,6 +254,9 @@ func TestList(t *testing.T) { cmpopts.SortSlices(func(a, b *authmethods.AuthMethod) bool { return a.Name < b.Name }), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), ), ) @@ -274,6 +280,9 @@ func TestList(t *testing.T) { cmpopts.SortSlices(func(a, b *authmethods.AuthMethod) bool { return a.Name < b.Name }), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), ), ) @@ -281,7 +290,14 @@ func TestList(t *testing.T) { authmethods.WithFilter(`"/item/attributes/client_id"=="client-id"`)) require.NoError(err) require.Len(result.Items, 1) - assert.Empty(cmp.Diff(oidcAM.Item, result.Items[0], cmpopts.IgnoreUnexported(authmethods.AuthMethod{}))) + assert.Empty(cmp.Diff( + oidcAM.Item, + result.Items[0], + cmpopts.IgnoreUnexported(authmethods.AuthMethod{}), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) result, err = amClient.List(tc.Context(), global, authmethods.WithFilter(`"/item/attributes/min_login_name_length"==3`)) @@ -295,6 +311,9 @@ func TestList(t *testing.T) { cmpopts.SortSlices(func(a, b *authmethods.AuthMethod) bool { return a.Name < b.Name }), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), ), ) @@ -302,7 +321,14 @@ func TestList(t *testing.T) { authmethods.WithFilter(`"/item/attributes/min_password_length"==10`)) require.NoError(err) require.Len(result.Items, 1) - assert.Empty(cmp.Diff(pwAM.Item, result.Items[0], cmpopts.IgnoreUnexported(authmethods.AuthMethod{}))) + assert.Empty(cmp.Diff( + pwAM.Item, + result.Items[0], + cmpopts.IgnoreUnexported(authmethods.AuthMethod{}), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + )) } func TestCustomMethods(t *testing.T) { diff --git a/internal/tests/api/hosts/host_test.go b/internal/tests/api/hosts/host_test.go index 58246fa34..737eb8926 100644 --- a/internal/tests/api/hosts/host_test.go +++ b/internal/tests/api/hosts/host_test.go @@ -9,6 +9,8 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/boundary/api" "github.com/hashicorp/boundary/api/hostcatalogs" "github.com/hashicorp/boundary/api/hosts" @@ -125,7 +127,17 @@ func TestPluginHosts(t *testing.T) { h, err := hClient.Read(tc.Context(), hl.Items[0].Id) require.NoError(err) - assert.Equal(hl.Items[0], h.Item) + assert.Empty( + cmp.Diff( + hl.Items[0], + h.Item, + cmpopts.IgnoreUnexported(hosts.Host{}), + cmpopts.IgnoreFields(hosts.Host{}, "Version", "UpdatedTime"), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), + ), + ) _, err = hClient.Update(tc.Context(), h.Item.Id, h.Item.Version, hosts.WithName("foo")) require.Error(err) diff --git a/internal/tests/api/hostsets/host_set_test.go b/internal/tests/api/hostsets/host_set_test.go index f4f15fa32..fa22b68f1 100644 --- a/internal/tests/api/hostsets/host_set_test.go +++ b/internal/tests/api/hostsets/host_set_test.go @@ -146,6 +146,9 @@ func TestList_Plugin(t *testing.T) { comparableSetSlice(ul.Items), cmpopts.IgnoreUnexported(hostsets.HostSet{}), cmpopts.IgnoreFields(hostsets.HostSet{}, "Version", "UpdatedTime"), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), ), ) @@ -165,6 +168,9 @@ func TestList_Plugin(t *testing.T) { cmpopts.SortSlices(func(x, y *hostsets.HostSet) bool { return x.Id < y.Id }), + cmpopts.SortSlices(func(a, b string) bool { + return a < b + }), ), ) diff --git a/internal/types/action/action.go b/internal/types/action/action.go index f1dbd10e3..71798add5 100644 --- a/internal/types/action/action.go +++ b/internal/types/action/action.go @@ -215,29 +215,61 @@ func (act Type) IsActionOrParent(suspect Type) bool { return strings.HasPrefix(suspect.String(), fmt.Sprintf("%s:", act.String())) } -// ActionSet stores a slice of action types -type ActionSet []Type +// ActionSet is a set of action.Type. +type ActionSet map[Type]struct{} + +// NewActionSet creates an ActionSet. +func NewActionSet(t ...Type) ActionSet { + a := make(ActionSet, len(t)) + a.Add(t...) + return a +} + +// Add adds the provided Types t to a, any duplicates are ignored. +func (a ActionSet) Add(t ...Type) { + for _, tt := range t { + a[tt] = struct{}{} + } +} + +// Union returns a new ActionSet that is the union of the sets. +func Union(sets ...ActionSet) ActionSet { + a := make(ActionSet) + for _, s := range sets { + for k := range s { + a.Add(k) + } + } + return a +} + +// Difference returns a new ActionSet that is the difference of a-b. +func Difference(a ActionSet, b ActionSet) ActionSet { + c := make(ActionSet) + for t := range a { + if !b.HasAction(t) { + c.Add(t) + } + } + return c +} // Strings converts Actions into a slice of the actions' string equivalents func (a ActionSet) Strings() []string { if a == nil { return nil } - ret := make([]string, len(a)) - for i, act := range a { - ret[i] = act.String() + ret := make([]string, 0, len(a)) + for act := range a { + ret = append(ret, act.String()) } return ret } // HasAction returns whether the action set contains the given action. func (a ActionSet) HasAction(act Type) bool { - for _, v := range a { - if v == act { - return true - } - } - return false + _, ok := a[act] + return ok } // OnlySelf returns true if all actions in the action set are self types. An @@ -247,7 +279,7 @@ func (a ActionSet) OnlySelf() bool { if len(a) == 0 { return false } - for _, v := range a { + for v := range a { if !strings.HasSuffix(v.String(), ":self") { return false } diff --git a/internal/types/action/action_test.go b/internal/types/action/action_test.go index c19e43bbd..8112d1e0e 100644 --- a/internal/types/action/action_test.go +++ b/internal/types/action/action_test.go @@ -155,23 +155,26 @@ func TestActionStrings(t *testing.T) { }{ { name: "basic test", - actions: ActionSet{Read, AuthorizeSession}, + actions: NewActionSet(Read, AuthorizeSession), want: []string{"read", "authorize-session"}, }, { name: "reverse test to check ordering", - actions: ActionSet{AuthorizeSession, Read}, + actions: NewActionSet(AuthorizeSession, Read), want: []string{"authorize-session", "read"}, }, { name: "another test", - actions: ActionSet{Delete, AddGrants}, + actions: NewActionSet(Delete, AddGrants), want: []string{"delete", "add-grants"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.actions.Strings(), tt.want) + // TODO: previously the order seemed to matter for these tests, + // Is this really imporant? And if so, is simply having a stable + // order sufficient? + assert.ElementsMatch(t, tt.actions.Strings(), tt.want) }) } } @@ -185,31 +188,31 @@ func TestHasAction(t *testing.T) { }{ { name: "has 1", - actions: ActionSet{Read, AuthorizeSession}, + actions: NewActionSet(Read, AuthorizeSession), action: Read, want: true, }, { name: "has 2", - actions: ActionSet{Read, AuthorizeSession}, + actions: NewActionSet(Read, AuthorizeSession), action: AuthorizeSession, want: true, }, { name: "empty", - actions: ActionSet{}, + actions: NewActionSet(), action: AuthorizeSession, want: false, }, { name: "does not have 1", - actions: ActionSet{Read, AuthorizeSession}, + actions: NewActionSet(Read, AuthorizeSession), action: ReadSelf, want: false, }, { name: "does not have 2", - actions: ActionSet{Read, AuthorizeSession}, + actions: NewActionSet(Read, AuthorizeSession), action: Delete, want: false, }, @@ -229,12 +232,12 @@ func TestOnlySelf(t *testing.T) { }{ { name: "has only self 1", - actions: ActionSet{ReadSelf, CancelSelf}, + actions: NewActionSet(ReadSelf, CancelSelf), want: true, }, { name: "has only self 2", - actions: ActionSet{ReadSelf}, + actions: NewActionSet(ReadSelf), want: true, }, { @@ -244,12 +247,12 @@ func TestOnlySelf(t *testing.T) { }, { name: "does not have only self 1", - actions: ActionSet{Read, AuthorizeSession}, + actions: NewActionSet(Read, AuthorizeSession), want: false, }, { name: "does not have only self 2", - actions: ActionSet{ReadSelf, AuthorizeSession}, + actions: NewActionSet(ReadSelf, AuthorizeSession), want: false, }, } @@ -296,3 +299,219 @@ func TestIsActionOrParent(t *testing.T) { }) } } + +func TestNewActionSet(t *testing.T) { + tests := []struct { + name string + types []Type + want ActionSet + }{ + { + name: "empty", + types: []Type{}, + want: make(ActionSet), + }, + { + name: "single", + types: []Type{List}, + want: ActionSet{ + List: struct{}{}, + }, + }, + { + name: "multiple", + types: []Type{List, Create, Read, Update, Delete}, + want: ActionSet{ + List: struct{}{}, + Create: struct{}{}, + Read: struct{}{}, + Update: struct{}{}, + Delete: struct{}{}, + }, + }, + { + name: "multiple-duplicates", + types: []Type{List, Read, List, Read, Create, Read, Update, Delete}, + want: ActionSet{ + List: struct{}{}, + Create: struct{}{}, + Read: struct{}{}, + Update: struct{}{}, + Delete: struct{}{}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := NewActionSet(tt.types...) + assert.Equal(t, tt.want, s) + }) + } +} + +func TestActionSetUnion(t *testing.T) { + tests := []struct { + name string + sets []ActionSet + want ActionSet + }{ + { + name: "empty", + sets: []ActionSet{}, + want: ActionSet{}, + }, + { + name: "one", + sets: []ActionSet{ + { + List: struct{}{}, + Create: struct{}{}, + Read: struct{}{}, + }, + }, + want: ActionSet{ + List: struct{}{}, + Create: struct{}{}, + Read: struct{}{}, + }, + }, + { + name: "multiple-no-duplicates", + sets: []ActionSet{ + { + List: struct{}{}, + Create: struct{}{}, + }, + { + Update: struct{}{}, + Delete: struct{}{}, + }, + { + Read: struct{}{}, + }, + }, + want: ActionSet{ + List: struct{}{}, + Create: struct{}{}, + Read: struct{}{}, + Update: struct{}{}, + Delete: struct{}{}, + }, + }, + { + name: "multiple-duplicates", + sets: []ActionSet{ + { + List: struct{}{}, + Create: struct{}{}, + }, + { + List: struct{}{}, + Update: struct{}{}, + Delete: struct{}{}, + }, + { + Update: struct{}{}, + Read: struct{}{}, + }, + }, + want: ActionSet{ + List: struct{}{}, + Create: struct{}{}, + Read: struct{}{}, + Update: struct{}{}, + Delete: struct{}{}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := Union(tt.sets...) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestActionSetDifference(t *testing.T) { + tests := []struct { + name string + a ActionSet + b ActionSet + want ActionSet + }{ + { + name: "same-set", + a: ActionSet{ + List: struct{}{}, + Update: struct{}{}, + }, + b: ActionSet{ + List: struct{}{}, + Update: struct{}{}, + }, + want: ActionSet{}, + }, + { + name: "distinct", + a: ActionSet{ + List: struct{}{}, + Update: struct{}{}, + }, + b: ActionSet{ + Read: struct{}{}, + Delete: struct{}{}, + }, + want: ActionSet{ + List: struct{}{}, + Update: struct{}{}, + }, + }, + { + name: "some-overlap", + a: ActionSet{ + List: struct{}{}, + Update: struct{}{}, + }, + b: ActionSet{ + Read: struct{}{}, + List: struct{}{}, + Delete: struct{}{}, + }, + want: ActionSet{ + Update: struct{}{}, + }, + }, + { + name: "nil-a", + a: nil, + b: ActionSet{ + Read: struct{}{}, + List: struct{}{}, + Delete: struct{}{}, + }, + want: ActionSet{}, + }, + { + name: "nil-b", + a: ActionSet{ + Read: struct{}{}, + List: struct{}{}, + Delete: struct{}{}, + }, + b: nil, + want: ActionSet{ + Read: struct{}{}, + List: struct{}{}, + Delete: struct{}{}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := Difference(tt.a, tt.b) + assert.Equal(t, tt.want, got) + }) + } +}