From b41e983503be89689d7121c3e8d6c876dcf3690c Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 29 Apr 2022 09:52:01 -0400 Subject: [PATCH 01/10] Reorder authz check for sessions (#2042) * Reorder session authz checks This implements a mechanism for service handlers to pass a repo satisfying an interface into the scope IDs listing function. The purpose of this interface is to allow the same function to perform authorization checking against resources rather than requiring service handlers to fetch all resources in the determined set of scopes and then perform this checking. While this could be done in each service handler, this approach centralizes the logic, which has the nice benefit of removing some boilerplate from service handler list functions that have adapted to it. Tests haven't changed, which is expected -- the function should return exactly what was being returned before, simply faster. --- internal/boundary/boundary.go | 13 +- internal/servers/controller/auth/auth.go | 2 +- .../controller/common/scopeids/scope_ids.go | 311 ++++++++++++++---- .../authmethods/authmethod_service.go | 2 +- .../handlers/authtokens/authtoken_service.go | 2 +- .../credentialstore_service.go | 2 +- .../handlers/groups/group_service.go | 2 +- .../host_catalogs/host_catalog_service.go | 2 +- .../controller/handlers/roles/role_service.go | 2 +- .../handlers/scopes/scope_service.go | 2 +- .../handlers/sessions/session_service.go | 48 +-- .../handlers/targets/target_service.go | 2 +- .../controller/handlers/users/user_service.go | 2 +- internal/session/query.go | 7 + internal/session/repository_session.go | 77 ++++- internal/session/session.go | 16 +- 16 files changed, 376 insertions(+), 116 deletions(-) diff --git a/internal/boundary/boundary.go b/internal/boundary/boundary.go index a637c8515..8081495c6 100644 --- a/internal/boundary/boundary.go +++ b/internal/boundary/boundary.go @@ -2,7 +2,9 @@ // define the Boundary domain. package boundary -import "github.com/hashicorp/boundary/internal/db/timestamp" +import ( + "github.com/hashicorp/boundary/internal/db/timestamp" +) // An Entity is an object distinguished by its identity, rather than its // attributes. It can contain value objects and other entities. @@ -25,3 +27,12 @@ type Resource interface { GetName() string GetDescription() string } + +// AuthzProtectedEntity is used by some functions (primarily +// scopeids.AuthzProtectedEntityProvider-conforming implementations) to deliver +// some common information necessary for calculating authz. +type AuthzProtectedEntity interface { + Entity + GetScopeId() string + GetUserId() string +} diff --git a/internal/servers/controller/auth/auth.go b/internal/servers/controller/auth/auth.go index 309f82589..56fa2861c 100644 --- a/internal/servers/controller/auth/auth.go +++ b/internal/servers/controller/auth/auth.go @@ -196,7 +196,7 @@ func Verify(ctx context.Context, opt ...Option) (ret VerifyResults) { return } if scp == nil { - ret.Error = errors.New(ctx, errors.InvalidParameter, op, fmt.Sprint("non-existent scope $q", ret.Scope.Id)) + ret.Error = errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("non-existent scope %q", ret.Scope.Id)) return } ret.Scope = &scopes.ScopeInfo{ diff --git a/internal/servers/controller/common/scopeids/scope_ids.go b/internal/servers/controller/common/scopeids/scope_ids.go index 7a6fec7cb..fd6be78d8 100644 --- a/internal/servers/controller/common/scopeids/scope_ids.go +++ b/internal/servers/controller/common/scopeids/scope_ids.go @@ -3,6 +3,7 @@ package scopeids import ( "context" + "github.com/hashicorp/boundary/internal/boundary" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/perms" @@ -15,70 +16,127 @@ import ( "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/scopes" ) -// GetListingScopeIds, given common parameters for List calls, returns the set of scope -// IDs in which to search for resources. It also returns a memoized map of the -// scopes to their info for populating returned values. -// -// Note: This was originally pulled out 1:1 from the role service. It and other -// tests in the other service handlers test this function extensively as it -// forms the basis for all recursive listing tests; see those tests for list -// functionality in the various service handlers. -func GetListingScopeIds( +type authzProtectedEntityProvider interface { + // Fetches basic resource info for the given scopes. Note that this is a + // "where clause" style of argument: if the set of scopes is populated these + // are the scopes to limit to (e.g. to put in a where clause). An empty set + // of scopes means to look in *all* scopes, not none! + FetchAuthzProtectedEntitiesByScope(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error) +} + +// ResourceInfo contains information about a particular resource +type ResourceInfo struct { + AuthorizedActions action.ActionSet +} + +// ScopeInfoWithResourceIds contains information about a scope and the resources +// found within it +type ScopeInfoWithResourceIds struct { + *scopes.ScopeInfo + Resources map[string]ResourceInfo +} + +// GetListingResourceInformationInput contains input parameters to the function +type GetListingResourceInformationInput struct { + // An IAM repo function to use for a scope listing call + IamRepoFn common.IamRepoFactory + + // The original auth results from the list command + AuthResults auth.VerifyResults + + // The scope ID to use, or the starting point for a recursive search + RootScopeId string + + // The type of resource being listed + Type resource.Type + + // Whether the search is recursive + Recursive bool + + // A repo to fetch resources + AuthzProtectedEntityProvider authzProtectedEntityProvider + + // The available actions for the resource type + ActionSet action.ActionSet +} + +// GetListingResourceInformationOutput contains results from the function +type GetListingResourceInformationOutput struct { + // The calculated list of relevant scope IDs + ScopeIds []string + + // The specific resource IDs calculated to be authorized for listing + ResourceIds []string + + // A map of scope ID to scope information and a map of resource IDs in that + // scope and specific information about that resource, such as available + // actions + ScopeResourceMap map[string]*ScopeInfoWithResourceIds +} + +// GetListingResourceInformation, given common parameters for List calls, +// returns useful information: the set of scope IDs in which to search for +// resources; the IDs of the resources known to be authorized for that user; and +// a memoized map of the scopes to their info for populating returned values. +func GetListingResourceInformation( // The context to use when listing in the DB, if required ctx context.Context, - // An IAM repo function to use for a listing call, if required - repoFn common.IamRepoFactory, - // The original auth results from the list command - authResults auth.VerifyResults, - // The scope ID to use, or to use as the starting point for a recursive - // search - rootScopeId string, - // The type of resource we are listing - typ resource.Type, - // Whether or not the search should be recursive - recursive bool, - // Whether to only return scopes with exact permissions, or whether parent - // scopes with appropriate permissions are sufficient - directOnly bool, -) ([]string, map[string]*scopes.ScopeInfo, error) { - const op = "GetListingScopeIds" + // The input struct + input GetListingResourceInformationInput, +) (*GetListingResourceInformationOutput, error) { + const op = "scopeids.GetListingResourceInformation" + + output := new(GetListingResourceInformationOutput) // Validation switch { - case typ == resource.Unknown: - return nil, nil, errors.New(ctx, errors.InvalidParameter, op, "unknown resource") - case repoFn == nil: - return nil, nil, errors.New(ctx, errors.InvalidParameter, op, "nil iam repo") - case rootScopeId == "": - return nil, nil, errors.New(ctx, errors.InvalidParameter, op, "missing root scope id") - case authResults.Scope == nil: - return nil, nil, errors.New(ctx, errors.InvalidParameter, op, "nil scope in auth results") - } - - // Base case: if not recursive, return the scope we were given and the - // already-looked-up info - if !recursive { - return []string{authResults.Scope.Id}, map[string]*scopes.ScopeInfo{authResults.Scope.Id: authResults.Scope}, nil + case input.Type == resource.Unknown: + return nil, errors.New(ctx, errors.InvalidParameter, op, "unknown resource") + case input.IamRepoFn == nil: + return nil, errors.New(ctx, errors.InvalidParameter, op, "nil iam repo") + case input.RootScopeId == "": + return nil, errors.New(ctx, errors.InvalidParameter, op, "missing root scope id") + case input.AuthResults.Scope == nil: + return nil, errors.New(ctx, errors.InvalidParameter, op, "nil scope in auth results") + case !input.Recursive && input.AuthResults.Scope.Id != input.RootScopeId: + return nil, errors.New(ctx, errors.InvalidParameter, op, "non-recursive search but auth results scope does not match root scope") } // This will be used to memoize scope info so we can put the right scope // info for each returned value - scopeInfoMap := map[string]*scopes.ScopeInfo{} + output.ScopeResourceMap = map[string]*ScopeInfoWithResourceIds{} - repo, err := repoFn() + // Base case: if not recursive, return the scope we were given and the + // already-looked-up info + if !input.Recursive { + output.ScopeResourceMap[input.AuthResults.Scope.Id] = &ScopeInfoWithResourceIds{ScopeInfo: input.AuthResults.Scope} + // If we don't have information do to the resource lookup ourselves, + // return what we have + if input.AuthzProtectedEntityProvider == nil { + output.ScopeIds = []string{input.AuthResults.Scope.Id} + return output, nil + } + // Otherwise filter on this one scope and return + if err := filterAuthorizedResourceIds(ctx, input, output); err != nil { + return nil, errors.Wrap(ctx, err, op, errors.WithMsg("error filtering to only authorized resources")) + } + return output, nil + } + + repo, err := input.IamRepoFn() if err != nil { - return nil, nil, err + return nil, err } // Get all scopes recursively. Start at global because we need to take into // account permissions in parent scopes even if they want to scale back the // returned information to a child scope and its children. scps, err := repo.ListScopesRecursively(ctx, scope.Global.String()) if err != nil { - return nil, nil, err + return nil, err } res := perms.Resource{ - Type: typ, + Type: input.Type, } // For each scope, see if we have permission to list that type in that // scope @@ -88,7 +146,7 @@ func GetListingScopeIds( for _, scp := range scps { scpId := scp.GetPublicId() res.ScopeId = scpId - aSet := authResults.FetchActionSetForType(ctx, + aSet := input.AuthResults.FetchActionSetForType(ctx, // This is overridden by WithResource resource.Unknown, action.ActionSet{action.List}, @@ -97,16 +155,14 @@ func GetListingScopeIds( switch len(aSet) { case 0: // Defer until we've read all scopes. We do this because if the - // ordering coming back isn't in parent-first ording our map + // ordering coming back isn't in parent-first ordering our map // lookup might fail. - if !directOnly { - deferredScopes = append(deferredScopes, scp) - } + deferredScopes = append(deferredScopes, scp) case 1: if aSet[0] != action.List { - return nil, nil, errors.New(ctx, errors.Internal, op, "unexpected action in set") + return nil, errors.New(ctx, errors.Internal, op, "unexpected action in set") } - if scopeInfoMap[scpId] == nil { + if output.ScopeResourceMap[scpId] == nil { scopeInfo := &scopes.ScopeInfo{ Id: scp.GetPublicId(), Type: scp.GetType(), @@ -114,13 +170,13 @@ func GetListingScopeIds( Description: scp.GetDescription(), ParentScopeId: scp.GetParentId(), } - scopeInfoMap[scpId] = scopeInfo + output.ScopeResourceMap[scpId] = &ScopeInfoWithResourceIds{ScopeInfo: scopeInfo} } if scpId == scope.Global.String() { globalHasList = true } default: - return nil, nil, errors.New(ctx, errors.Internal, op, "unexpected number of actions back in set") + return nil, errors.New(ctx, errors.Internal, op, "unexpected number of actions back in set") } } @@ -129,9 +185,9 @@ func GetListingScopeIds( // If they had list on global scope anything else is automatically // included; otherwise if they had list on the parent scope, this // scope is included in the map and is sufficient here. - if globalHasList || scopeInfoMap[scp.GetParentId()] != nil { + if globalHasList || output.ScopeResourceMap[scp.GetParentId()] != nil { scpId := scp.GetPublicId() - if scopeInfoMap[scpId] == nil { + if output.ScopeResourceMap[scpId] == nil { scopeInfo := &scopes.ScopeInfo{ Id: scp.GetPublicId(), Type: scp.GetType(), @@ -139,21 +195,15 @@ func GetListingScopeIds( Description: scp.GetDescription(), ParentScopeId: scp.GetParentId(), } - scopeInfoMap[scpId] = scopeInfo + output.ScopeResourceMap[scpId] = &ScopeInfoWithResourceIds{ScopeInfo: scopeInfo} } } } - // If we have nothing in scopeInfoMap at this point, we aren't authorized - // anywhere so return 403. - if len(scopeInfoMap) == 0 { - return nil, nil, handlers.ForbiddenError() - } - // Now elide out any that aren't under the root scope ID - elideScopes := make([]string, 0, len(scopeInfoMap)) - for scpId, scp := range scopeInfoMap { - switch rootScopeId { + elideScopes := make([]string, 0, len(output.ScopeResourceMap)) + for scpId, scp := range output.ScopeResourceMap { + switch input.RootScopeId { // If the root is global, it matches case scope.Global.String(): // If the current scope matches the root, it matches @@ -168,13 +218,134 @@ func GetListingScopeIds( } for _, scpId := range elideScopes { - delete(scopeInfoMap, scpId) + delete(output.ScopeResourceMap, scpId) } - scopeIds := make([]string, 0, len(scopeInfoMap)) - for k := range scopeInfoMap { - scopeIds = append(scopeIds, k) + // If we have nothing in scopeInfoMap at this point, we aren't authorized + // anywhere so return 403. + if len(output.ScopeResourceMap) == 0 { + return nil, handlers.ForbiddenError() } - return scopeIds, scopeInfoMap, nil + if input.AuthzProtectedEntityProvider == nil { + output.populateScopeIdsFromScopeResourceMap() + return output, nil + } + + if err := filterAuthorizedResourceIds(ctx, input, output); err != nil { + return nil, errors.Wrap(ctx, err, op, errors.WithMsg("error filtering to only authorized resources")) + } + + return output, nil +} + +// filterAuthorizedResourceIds calls the passed in function to get IDs for +// resources in the given scopes and then figures out which ones are actually +// authorized for listing by the user. +// +// It also populates the scope IDs in the output +func filterAuthorizedResourceIds( + // The context to use when listing in the DB, if required + ctx context.Context, + // The input struct + input GetListingResourceInformationInput, + // The scope information to fill out + output *GetListingResourceInformationOutput, +) error { + const op = "scopeids.filterAuthorizedResources" + + // Populate scopeIds and determine if we found global + output.populateScopeIdsFromScopeResourceMap() + + // The calling function is giving us a complete set with any recursive + // lookup already performed (that's the point of the function). + scopedResourceInfo, err := input.AuthzProtectedEntityProvider.FetchAuthzProtectedEntitiesByScope(ctx, output.ScopeIds) + if err != nil { + return errors.Wrap(ctx, err, op) + } + + res := perms.Resource{ + Type: resource.Session, + } + + // Now run authorization checks against each so we know if there is a point + // in fetching the full resource, and cache the authorized actions + for scopeId, resourceInfos := range scopedResourceInfo { + for _, resourceInfo := range resourceInfos { + res.Id = resourceInfo.GetPublicId() + res.ScopeId = scopeId + authorizedActions := input.AuthResults.FetchActionSetForId(ctx, resourceInfo.GetPublicId(), input.ActionSet, auth.WithResource(&res)) + if len(authorizedActions) == 0 { + continue + } + + if resourceInfo.GetUserId() != "" { + if authorizedActions.OnlySelf() && resourceInfo.GetUserId() != input.AuthResults.UserId { + continue + } + } + + if output.ScopeResourceMap[scopeId].Resources == nil { + output.ScopeResourceMap[scopeId].Resources = make(map[string]ResourceInfo) + } + + output.ScopeResourceMap[scopeId].Resources[resourceInfo.GetPublicId()] = ResourceInfo{AuthorizedActions: authorizedActions} + output.ResourceIds = append(output.ResourceIds, resourceInfo.GetPublicId()) + } + } + + return nil +} + +// populateScopeIdsFromScopeResourceMap populates the ScopeIds field and returns +// whether global scope was found +func (i *GetListingResourceInformationOutput) populateScopeIdsFromScopeResourceMap() { + for k := range i.ScopeResourceMap { + i.ScopeIds = append(i.ScopeIds, k) + } +} + +// GetListingScopeIds is provided for backwards compatibility with existing +// services; services should eventually migrate to +// GetListingResourceInformation. +func GetListingScopeIds( + // The context to use when listing in the DB, if required + ctx context.Context, + // An IAM repo function to use for a listing call, if required + repoFn common.IamRepoFactory, + // The original auth results from the list command + authResults auth.VerifyResults, + // The scope ID to use, or to use as the starting point for a recursive + // search + rootScopeId string, + // The type of resource we are listing + typ resource.Type, + // Whether or not the search should be recursive + recursive bool, +) ([]string, map[string]*scopes.ScopeInfo, error) { + const op = "scopeids.GetListingScopeIds" + scopeResourceInfo, err := GetListingResourceInformation(ctx, + GetListingResourceInformationInput{ + IamRepoFn: repoFn, + AuthResults: authResults, + RootScopeId: rootScopeId, + Type: typ, + Recursive: recursive, + }, + ) + if err != nil { + if err == handlers.ForbiddenError() { + return nil, nil, err + } + return nil, nil, errors.Wrap(ctx, err, op) + } + if len(scopeResourceInfo.ScopeIds) == 0 { + // This should have already happened in the other function, but... + return nil, nil, handlers.ForbiddenError() + } + scopeMap := make(map[string]*scopes.ScopeInfo, len(scopeResourceInfo.ScopeResourceMap)) + for k, v := range scopeResourceInfo.ScopeResourceMap { + scopeMap[k] = v.ScopeInfo + } + return scopeResourceInfo.ScopeIds, scopeMap, nil } diff --git a/internal/servers/controller/handlers/authmethods/authmethod_service.go b/internal/servers/controller/handlers/authmethods/authmethod_service.go index 134981896..7d58b43f1 100644 --- a/internal/servers/controller/handlers/authmethods/authmethod_service.go +++ b/internal/servers/controller/handlers/authmethods/authmethod_service.go @@ -130,7 +130,7 @@ func (s Service) ListAuthMethods(ctx context.Context, req *pbs.ListAuthMethodsRe } scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds( - ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.AuthMethod, req.GetRecursive(), false) + ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.AuthMethod, req.GetRecursive()) if err != nil { return nil, err } diff --git a/internal/servers/controller/handlers/authtokens/authtoken_service.go b/internal/servers/controller/handlers/authtokens/authtoken_service.go index 6034645b7..2ee04e1bf 100644 --- a/internal/servers/controller/handlers/authtokens/authtoken_service.go +++ b/internal/servers/controller/handlers/authtokens/authtoken_service.go @@ -81,7 +81,7 @@ func (s Service) ListAuthTokens(ctx context.Context, req *pbs.ListAuthTokensRequ } scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds( - ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.AuthToken, req.GetRecursive(), false) + ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.AuthToken, req.GetRecursive()) if err != nil { return nil, err } diff --git a/internal/servers/controller/handlers/credentialstores/credentialstore_service.go b/internal/servers/controller/handlers/credentialstores/credentialstore_service.go index 8f97b943d..9fb8ff03b 100644 --- a/internal/servers/controller/handlers/credentialstores/credentialstore_service.go +++ b/internal/servers/controller/handlers/credentialstores/credentialstore_service.go @@ -112,7 +112,7 @@ func (s Service) ListCredentialStores(ctx context.Context, req *pbs.ListCredenti } scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds( - ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.CredentialStore, req.GetRecursive(), false) + ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.CredentialStore, req.GetRecursive()) if err != nil { return nil, err } diff --git a/internal/servers/controller/handlers/groups/group_service.go b/internal/servers/controller/handlers/groups/group_service.go index 0f562699e..e7ffb5389 100644 --- a/internal/servers/controller/handlers/groups/group_service.go +++ b/internal/servers/controller/handlers/groups/group_service.go @@ -92,7 +92,7 @@ func (s Service) ListGroups(ctx context.Context, req *pbs.ListGroupsRequest) (*p } scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds( - ctx, s.repoFn, authResults, req.GetScopeId(), resource.Group, req.GetRecursive(), false) + ctx, s.repoFn, authResults, req.GetScopeId(), resource.Group, req.GetRecursive()) if err != nil { return nil, err } diff --git a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go index c83fb1518..3f961f990 100644 --- a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go +++ b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go @@ -129,7 +129,7 @@ func (s Service) ListHostCatalogs(ctx context.Context, req *pbs.ListHostCatalogs } scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds( - ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.HostCatalog, req.GetRecursive(), false) + ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.HostCatalog, req.GetRecursive()) if err != nil { return nil, err } diff --git a/internal/servers/controller/handlers/roles/role_service.go b/internal/servers/controller/handlers/roles/role_service.go index dc94d4d50..c9e867cb4 100644 --- a/internal/servers/controller/handlers/roles/role_service.go +++ b/internal/servers/controller/handlers/roles/role_service.go @@ -96,7 +96,7 @@ func (s Service) ListRoles(ctx context.Context, req *pbs.ListRolesRequest) (*pbs } scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds( - ctx, s.repoFn, authResults, req.GetScopeId(), resource.Role, req.GetRecursive(), false) + ctx, s.repoFn, authResults, req.GetScopeId(), resource.Role, req.GetRecursive()) if err != nil { return nil, err } diff --git a/internal/servers/controller/handlers/scopes/scope_service.go b/internal/servers/controller/handlers/scopes/scope_service.go index f35ee93ac..58eed4ea8 100644 --- a/internal/servers/controller/handlers/scopes/scope_service.go +++ b/internal/servers/controller/handlers/scopes/scope_service.go @@ -133,7 +133,7 @@ func (s Service) ListScopes(ctx context.Context, req *pbs.ListScopesRequest) (*p } scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds( - ctx, s.repoFn, authResults, req.GetScopeId(), resource.Scope, req.GetRecursive(), false) + ctx, s.repoFn, authResults, req.GetScopeId(), resource.Scope, req.GetRecursive()) if err != nil { return nil, err } diff --git a/internal/servers/controller/handlers/sessions/session_service.go b/internal/servers/controller/handlers/sessions/session_service.go index e65d47043..056587996 100644 --- a/internal/servers/controller/handlers/sessions/session_service.go +++ b/internal/servers/controller/handlers/sessions/session_service.go @@ -119,6 +119,8 @@ func (s Service) GetSession(ctx context.Context, req *pbs.GetSessionRequest) (*p // ListSessions implements the interface pbs.SessionServiceServer. func (s Service) ListSessions(ctx context.Context, req *pbs.ListSessionsRequest) (*pbs.ListSessionsResponse, error) { + const op = "session.(Service).ListSessions" + if err := validateListRequest(req); err != nil { return nil, err } @@ -137,17 +139,34 @@ func (s Service) ListSessions(ctx context.Context, req *pbs.ListSessionsRequest) } } - scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds(ctx, - s.iamRepoFn, authResults, req.GetScopeId(), resource.Session, req.GetRecursive(), false) + repo, err := s.repoFn() + if err != nil { + return nil, errors.Wrap(ctx, err, op) + } + + scopeResourceInfo, err := scopeids.GetListingResourceInformation( + ctx, + scopeids.GetListingResourceInformationInput{ + IamRepoFn: s.iamRepoFn, + AuthResults: authResults, + RootScopeId: req.GetScopeId(), + Type: resource.Session, + Recursive: req.GetRecursive(), + AuthzProtectedEntityProvider: repo, + ActionSet: IdActions, + }, + ) if err != nil { return nil, err } - // If no scopes match, return an empty response - if len(scopeIds) == 0 { + // If no scopes match or we match scopes but there are no resources in them + // that we are authorized to see, return an empty response + if len(scopeResourceInfo.ScopeIds) == 0 || + len(scopeResourceInfo.ResourceIds) == 0 { return &pbs.ListSessionsResponse{}, nil } - sesList, err := s.listFromRepo(ctx, scopeIds) + sesList, err := s.listFromRepo(ctx, scopeResourceInfo.ResourceIds) if err != nil { return nil, err } @@ -164,25 +183,14 @@ func (s Service) ListSessions(ctx context.Context, req *pbs.ListSessionsRequest) Type: resource.Session, } for _, item := range sesList { - res.Id = item.GetPublicId() - res.ScopeId = item.ScopeId - authorizedActions := authResults.FetchActionSetForId(ctx, item.GetPublicId(), IdActions, auth.WithResource(&res)) - if len(authorizedActions) == 0 { - continue - } - - if authorizedActions.OnlySelf() && item.UserId != authResults.UserId { - continue - } - outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) if outputFields.Has(globals.ScopeField) { - outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[item.ScopeId])) + outputOpts = append(outputOpts, handlers.WithScope(scopeResourceInfo.ScopeResourceMap[item.ScopeId].ScopeInfo)) } if outputFields.Has(globals.AuthorizedActionsField) { - outputOpts = append(outputOpts, handlers.WithAuthorizedActions(authorizedActions.Strings())) + outputOpts = append(outputOpts, handlers.WithAuthorizedActions(scopeResourceInfo.ScopeResourceMap[item.ScopeId].Resources[item.PublicId].AuthorizedActions.Strings())) } item, err := toProto(ctx, item, outputOpts...) @@ -287,12 +295,12 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*session.Session, return sess, nil } -func (s Service) listFromRepo(ctx context.Context, scopeIds []string) ([]*session.Session, error) { +func (s Service) listFromRepo(ctx context.Context, sessionIds []string) ([]*session.Session, error) { repo, err := s.repoFn() if err != nil { return nil, err } - sesList, err := repo.ListSessions(ctx, session.WithScopeIds(scopeIds)) + sesList, err := repo.ListSessions(ctx, session.WithSessionIds(sessionIds...)) if err != nil { return nil, err } diff --git a/internal/servers/controller/handlers/targets/target_service.go b/internal/servers/controller/handlers/targets/target_service.go index e0b49fa7d..02b0d956c 100644 --- a/internal/servers/controller/handlers/targets/target_service.go +++ b/internal/servers/controller/handlers/targets/target_service.go @@ -164,7 +164,7 @@ func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) ( } scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds( - ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.Target, req.GetRecursive(), false) + ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.Target, req.GetRecursive()) if err != nil { return nil, err } diff --git a/internal/servers/controller/handlers/users/user_service.go b/internal/servers/controller/handlers/users/user_service.go index 349dda738..0b0e417d5 100644 --- a/internal/servers/controller/handlers/users/user_service.go +++ b/internal/servers/controller/handlers/users/user_service.go @@ -94,7 +94,7 @@ func (s Service) ListUsers(ctx context.Context, req *pbs.ListUsersRequest) (*pbs } scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds( - ctx, s.repoFn, authResults, req.GetScopeId(), resource.User, req.GetRecursive(), false) + ctx, s.repoFn, authResults, req.GetScopeId(), resource.User, req.GetRecursive()) if err != nil { return nil, err } diff --git a/internal/session/query.go b/internal/session/query.go index 430ae4a08..541c4b475 100644 --- a/internal/session/query.go +++ b/internal/session/query.go @@ -118,6 +118,13 @@ select expiration_time, connection_limit, current_connection_count from session_connection_limit, session_connection_count; ` + + sessionPublicIdList = ` +select public_id, scope_id, user_id from session +%s +; +` + sessionList = ` select * from diff --git a/internal/session/repository_session.go b/internal/session/repository_session.go index 6e28a42ab..78fee5005 100644 --- a/internal/session/repository_session.go +++ b/internal/session/repository_session.go @@ -8,6 +8,7 @@ import ( "fmt" "strings" + "github.com/hashicorp/boundary/internal/boundary" "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" @@ -205,6 +206,51 @@ func (r *Repository) LookupSession(ctx context.Context, sessionId string, _ ...O return &session, authzSummary, nil } +// FetchAuthzProtectedEntitiesByScope implements boundary.AuthzProtectedEntityProvider +func (r *Repository) FetchAuthzProtectedEntitiesByScope(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error) { + const op = "session.(Repository).FetchAuthzProtectedEntityInfo" + + var where string + var args []interface{} + + inClauseCnt := 0 + + switch len(scopeIds) { + case 0: + return nil, errors.New(ctx, errors.InvalidParameter, op, "no scopes given") + case 1: + inClauseCnt += 1 + where, args = fmt.Sprintf("where scope_id = @%d", inClauseCnt), append(args, sql.Named("1", scopeIds[0])) + default: + idsInClause := make([]string, 0, len(scopeIds)) + for _, id := range scopeIds { + inClauseCnt += 1 + idsInClause, args = append(idsInClause, fmt.Sprintf("@%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), id)) + } + where = fmt.Sprintf("where scope_id in (%s)", strings.Join(idsInClause, ",")) + } + + q := sessionPublicIdList + query := fmt.Sprintf(q, where) + + rows, err := r.reader.Query(ctx, query, args) + if err != nil { + return nil, errors.Wrap(ctx, err, op) + } + defer rows.Close() + + sessionsMap := map[string][]boundary.AuthzProtectedEntity{} + for rows.Next() { + var ses Session + if err := r.reader.ScanRows(ctx, rows, &ses); err != nil { + return nil, errors.Wrap(ctx, err, op, errors.WithMsg("scan row failed")) + } + sessionsMap[ses.GetScopeId()] = append(sessionsMap[ses.GetScopeId()], ses) + } + + return sessionsMap, nil +} + // ListSessions will sessions. Supports the WithLimit, WithScopeId, WithSessionIds, and WithServerId options. func (r *Repository) ListSessions(ctx context.Context, opt ...Option) ([]*Session, error) { const op = "session.(Repository).ListSessions" @@ -213,25 +259,31 @@ func (r *Repository) ListSessions(ctx context.Context, opt ...Option) ([]*Sessio var args []interface{} inClauseCnt := 0 - if len(opts.withScopeIds) != 0 { - switch len(opts.withScopeIds) { - case 1: + switch len(opts.withScopeIds) { + case 0: + case 1: + inClauseCnt += 1 + where, args = append(where, fmt.Sprintf("scope_id = @%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), opts.withScopeIds[0])) + default: + idsInClause := make([]string, 0, len(opts.withScopeIds)) + for _, id := range opts.withScopeIds { inClauseCnt += 1 - where, args = append(where, fmt.Sprintf("scope_id = @%d", inClauseCnt)), append(args, sql.Named("1", opts.withScopeIds[0])) - default: - idsInClause := make([]string, 0, len(opts.withScopeIds)) - for _, id := range opts.withScopeIds { - inClauseCnt += 1 - idsInClause, args = append(idsInClause, fmt.Sprintf("@%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), id)) - } - where = append(where, fmt.Sprintf("scope_id in (%s)", strings.Join(idsInClause, ","))) + idsInClause, args = append(idsInClause, fmt.Sprintf("@%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), id)) } + where = append(where, fmt.Sprintf("scope_id in (%s)", strings.Join(idsInClause, ","))) } + if opts.withUserId != "" { inClauseCnt += 1 where, args = append(where, fmt.Sprintf("user_id = @%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), opts.withUserId)) } - if len(opts.withSessionIds) > 0 { + + switch len(opts.withSessionIds) { + case 0: + case 1: + inClauseCnt += 1 + where, args = append(where, fmt.Sprintf("s.public_id = @%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), opts.withSessionIds[0])) + default: idsInClause := make([]string, 0, len(opts.withSessionIds)) for _, id := range opts.withSessionIds { inClauseCnt += 1 @@ -239,6 +291,7 @@ func (r *Repository) ListSessions(ctx context.Context, opt ...Option) ([]*Sessio } where = append(where, fmt.Sprintf("s.public_id in (%s)", strings.Join(idsInClause, ","))) } + if opts.withServerId != "" { inClauseCnt += 1 where, args = append(where, fmt.Sprintf("server_id = @%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), opts.withServerId)) diff --git a/internal/session/session.go b/internal/session/session.go index baaa5331f..7026656e7 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/hashicorp/boundary/internal/boundary" "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" @@ -119,13 +120,22 @@ type Session struct { tableName string `gorm:"-"` } -func (s *Session) GetPublicId() string { +func (s Session) GetPublicId() string { return s.PublicId } +func (s Session) GetScopeId() string { + return s.ScopeId +} + +func (s Session) GetUserId() string { + return s.UserId +} + var ( - _ Cloneable = (*Session)(nil) - _ db.VetForWriter = (*Session)(nil) + _ Cloneable = (*Session)(nil) + _ db.VetForWriter = (*Session)(nil) + _ boundary.AuthzProtectedEntity = (*Session)(nil) ) // New creates a new in memory session. From a7a2c82931ef6d489d2a0dfced45df357182b37f Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Thu, 28 Apr 2022 13:46:04 -0400 Subject: [PATCH 02/10] feat(dbtest): Add docker-load Makefile target The new docker-load target uses docker buildx to build a local image of each of the Dockerfiles, and load them into the local docker socket image store. --- testing/dbtest/docker/Makefile | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/testing/dbtest/docker/Makefile b/testing/dbtest/docker/Makefile index d29680f52..8818bbb5e 100644 --- a/testing/dbtest/docker/Makefile +++ b/testing/dbtest/docker/Makefile @@ -13,6 +13,7 @@ TEST_CONTAINER_NAME ?= boundary-sql-tests # Generate targets from dockerfiles dockerfiles = $(wildcard Dockerfile.*) docker-buildxs = $(patsubst Dockerfile.%,%-buildx, $(dockerfiles)) +docker-load-buildxs = $(patsubst Dockerfile.%,%-load-buildx, $(dockerfiles)) # Before running this target a builder instance needs to be setup, ie: # docker buildx create --driver docker-container --use @@ -25,6 +26,14 @@ ${docker-buildxs}: %-buildx: -t $(REGISTRY_NAME)/$(TEST_IMAGE_NAME):$* \ -f Dockerfile.$* . +docker-load: ${docker-load-buildxs} + +${docker-load-buildxs}: %-load-buildx: + docker buildx build \ + --load \ + -t $(REGISTRY_NAME)/$(TEST_IMAGE_NAME):$* \ + -f Dockerfile.$* . + database-up: @echo "Using image: $(TEST_IMAGE_TAG)" @echo "Additional postgres configuration: $(PG_OPTS)" @@ -49,4 +58,4 @@ clean: docker stop $(TEST_CONTAINER_NAME) || true docker rm -v $(TEST_CONTAINER_NAME) || true -.PHONY: all docker-build database-up ${docker-buildxs} clean +.PHONY: all docker-build database-up ${docker-buildxs} ${docker-load-buildxs} clean From 7f9b294a7e3e55bbdc3417147e43e380deee0755 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Mon, 25 Apr 2022 15:52:44 -0400 Subject: [PATCH 03/10] refact(all): Use testing.TB for test helpers Our test helper packages were not possible to use with benchmarks. This refactor switches to testing.TB to allow both tests and benchmarks to use the helpers. --- internal/auth/oidc/testing.go | 26 +++--- internal/auth/password/testing.go | 8 +- internal/authtoken/testing.go | 2 +- internal/credential/vault/docker.go | 8 +- internal/credential/vault/supported.go | 14 +-- internal/credential/vault/testing.go | 106 +++++++++++++---------- internal/db/testing.go | 18 ++-- internal/host/plugin/testing.go | 18 ++-- internal/host/static/testing.go | 8 +- internal/iam/testing.go | 39 +++++---- internal/kms/testing.go | 30 +++---- internal/libs/crypto/testing.go | 2 +- internal/observability/event/testing.go | 24 ++--- internal/oplog/oplog_test/db.go | 2 +- internal/oplog/testing.go | 18 ++-- internal/plugin/host/testing.go | 2 +- internal/plugin/testing.go | 2 +- internal/scheduler/job/testing.go | 4 +- internal/scheduler/testing.go | 2 +- internal/servers/controller/testing.go | 10 +-- internal/servers/worker/proxy/testing.go | 2 +- internal/servers/worker/testing.go | 6 +- internal/session/testing.go | 16 ++-- internal/target/tcp/testing.go | 6 +- internal/target/testing.go | 2 +- internal/tests/api/testing.go | 6 +- sdk/pbs/controller/api/testing.go | 2 +- sdk/testutil/free_port.go | 2 +- sdk/wrapper/testing.go | 2 +- 29 files changed, 201 insertions(+), 186 deletions(-) diff --git a/internal/auth/oidc/testing.go b/internal/auth/oidc/testing.go index a122ad0b2..75b4a919f 100644 --- a/internal/auth/oidc/testing.go +++ b/internal/auth/oidc/testing.go @@ -39,7 +39,7 @@ const TestFakeManagedGroupFilter = `"/foo" == "bar"` // WithMaxAge, WithApiUrl, WithIssuer, WithCertificates, WithAudClaims, and // WithSigningAlgs options are supported. func TestAuthMethod( - t *testing.T, + t testing.TB, conn *db.DB, databaseWrapper wrapping.Wrapper, scopeId string, @@ -131,7 +131,7 @@ func TestAuthMethod( // TestSortAuthMethods will sort the provided auth methods by public id and it // will sort each auth method's embedded value objects (algs, auds, certs, // callbacks) -func TestSortAuthMethods(t *testing.T, methods []*AuthMethod) { +func TestSortAuthMethods(t testing.TB, methods []*AuthMethod) { // sort them by public id first... sort.Slice(methods, func(a, b int) bool { return methods[a].PublicId < methods[b].PublicId @@ -158,7 +158,7 @@ func TestSortAuthMethods(t *testing.T, methods []*AuthMethod) { } // TestAccount creates a test oidc auth account. -func TestAccount(t *testing.T, conn *db.DB, am *AuthMethod, subject string, opt ...Option) *Account { +func TestAccount(t testing.TB, conn *db.DB, am *AuthMethod, subject string, opt ...Option) *Account { t.Helper() require := require.New(t) rw := db.New(conn) @@ -179,7 +179,7 @@ func TestAccount(t *testing.T, conn *db.DB, am *AuthMethod, subject string, opt } // TestManagedGroup creates a test oidc managed group. -func TestManagedGroup(t *testing.T, conn *db.DB, am *AuthMethod, filter string, opt ...Option) *ManagedGroup { +func TestManagedGroup(t testing.TB, conn *db.DB, am *AuthMethod, filter string, opt ...Option) *ManagedGroup { t.Helper() require := require.New(t) rw := db.New(conn) @@ -197,7 +197,7 @@ func TestManagedGroup(t *testing.T, conn *db.DB, am *AuthMethod, filter string, } // TestManagedGroupMember adds given account IDs to a managed group -func TestManagedGroupMember(t *testing.T, conn *db.DB, managedGroupId, memberId string, opt ...Option) *ManagedGroupMemberAccount { +func TestManagedGroupMember(t testing.TB, conn *db.DB, managedGroupId, memberId string, opt ...Option) *ManagedGroupMemberAccount { t.Helper() require := require.New(t) rw := db.New(conn) @@ -212,7 +212,7 @@ func TestManagedGroupMember(t *testing.T, conn *db.DB, managedGroupId, memberId // TestConvertToUrls will convert URL string representations to a slice of // *url.URL -func TestConvertToUrls(t *testing.T, urls ...string) []*url.URL { +func TestConvertToUrls(t testing.TB, urls ...string) []*url.URL { t.Helper() require := require.New(t) require.NotEmpty(urls) @@ -228,7 +228,7 @@ func TestConvertToUrls(t *testing.T, urls ...string) []*url.URL { // testGenerateCA will generate a test x509 CA cert, along with it encoded in a // PEM format. -func testGenerateCA(t *testing.T, hosts ...string) (*x509.Certificate, string) { +func testGenerateCA(t testing.TB, hosts ...string) (*x509.Certificate, string) { t.Helper() require := require.New(t) @@ -280,7 +280,7 @@ func testGenerateCA(t *testing.T, hosts ...string) (*x509.Certificate, string) { return c, string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: derBytes})) } -func testProvider(t *testing.T, clientId, clientSecret, allowedRedirectURL string, tp *oidc.TestProvider) *oidc.Provider { +func testProvider(t testing.TB, clientId, clientSecret, allowedRedirectURL string, tp *oidc.TestProvider) *oidc.Provider { t.Helper() require := require.New(t) require.NotEmptyf(clientId, "%s: client id is empty") @@ -307,7 +307,7 @@ func testProvider(t *testing.T, clientId, clientSecret, allowedRedirectURL strin // testState will make a request.State and encrypt/encode within a request.Wrapper. // the returned string can be used as a parameter for functions like: oidc.Callback func testState( - t *testing.T, + t testing.TB, am *AuthMethod, kms *kms.Kms, tokenRequestId string, @@ -344,7 +344,7 @@ func testState( // TestTokenRequestId will make a request.Token and encrypt/encode within a request.Wrapper. // the returned string can be used as a parameter for functions like: oidc.TokenRequest func TestTokenRequestId( - t *testing.T, + t testing.TB, am *AuthMethod, kms *kms.Kms, expIn time.Duration, @@ -371,7 +371,7 @@ func TestTokenRequestId( // TestPendingToken will create a pending auth token for the tokenRequestId (aka public id) func TestPendingToken( - t *testing.T, + t testing.TB, tokenRepo *authtoken.Repository, user *iam.User, acct *Account, @@ -401,11 +401,11 @@ type testControllerSrv struct { iamRepoFn IamRepoFactory atRepoFn AuthTokenRepoFactory httpServer *httptest.Server - t *testing.T + t testing.TB } // startTestControllerSrv returns a running testControllerSrv -func startTestControllerSrv(t *testing.T, oidcRepoFn OidcRepoFactory, iamRepoFn IamRepoFactory, atRepoFn AuthTokenRepoFactory) *testControllerSrv { +func startTestControllerSrv(t testing.TB, oidcRepoFn OidcRepoFactory, iamRepoFn IamRepoFactory, atRepoFn AuthTokenRepoFactory) *testControllerSrv { require := require.New(t) require.NotNil(t) t.Helper() diff --git a/internal/auth/password/testing.go b/internal/auth/password/testing.go index 975ed4cb9..121580d64 100644 --- a/internal/auth/password/testing.go +++ b/internal/auth/password/testing.go @@ -13,7 +13,7 @@ import ( // TestAuthMethod creates a password auth methods to the provided DB with the // provided scope id. If any errors are encountered during the creation of the // auth methods, the test will fail. -func TestAuthMethod(t *testing.T, conn *db.DB, scopeId string, opt ...Option) *AuthMethod { +func TestAuthMethod(t testing.TB, conn *db.DB, scopeId string, opt ...Option) *AuthMethod { t.Helper() assert, require := assert.New(t), require.New(t) w := db.New(conn) @@ -47,7 +47,7 @@ func TestAuthMethod(t *testing.T, conn *db.DB, scopeId string, opt ...Option) *A // TestAuthMethods creates count number of password auth methods to the provided DB // with the provided scope id. If any errors are encountered during the creation of // the auth methods, the test will fail. -func TestAuthMethods(t *testing.T, conn *db.DB, scopeId string, count int) []*AuthMethod { +func TestAuthMethods(t testing.TB, conn *db.DB, scopeId string, count int) []*AuthMethod { t.Helper() var auts []*AuthMethod for i := 0; i < count; i++ { @@ -59,7 +59,7 @@ func TestAuthMethods(t *testing.T, conn *db.DB, scopeId string, count int) []*Au // TestMultipleAccounts creates count number of password account to the provided DB // with the provided auth method id. The auth method must have been created previously. // If any errors are encountered during the creation of the account, the test will fail. -func TestMultipleAccounts(t *testing.T, conn *db.DB, authMethodId string, count int) []*Account { +func TestMultipleAccounts(t testing.TB, conn *db.DB, authMethodId string, count int) []*Account { t.Helper() var auts []*Account for i := 0; i < count; i++ { @@ -72,7 +72,7 @@ func TestMultipleAccounts(t *testing.T, conn *db.DB, authMethodId string, count // auth method id and loginName. The auth method must have been created // previously. See password.NewAccount(...) for a list of supported options. // If any errors are encountered during the creation of the account, the test will fail. -func TestAccount(t *testing.T, conn *db.DB, authMethodId, loginName string, opt ...Option) *Account { +func TestAccount(t testing.TB, conn *db.DB, authMethodId, loginName string, opt ...Option) *Account { t.Helper() assert, require := assert.New(t), require.New(t) require.NotEmpty(loginName) diff --git a/internal/authtoken/testing.go b/internal/authtoken/testing.go index b34cded2f..86d0d7825 100644 --- a/internal/authtoken/testing.go +++ b/internal/authtoken/testing.go @@ -11,7 +11,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestAuthToken(t *testing.T, conn *db.DB, kms *kms.Kms, scopeId string, opt ...Option) *AuthToken { +func TestAuthToken(t testing.TB, conn *db.DB, kms *kms.Kms, scopeId string, opt ...Option) *AuthToken { t.Helper() authMethod := password.TestAuthMethods(t, conn, scopeId, 1)[0] // auth account is only used to join auth method to user. diff --git a/internal/credential/vault/docker.go b/internal/credential/vault/docker.go index ae7a4cb0c..a6fa244ed 100644 --- a/internal/credential/vault/docker.go +++ b/internal/credential/vault/docker.go @@ -3,16 +3,16 @@ package vault import "testing" var ( - newVaultServer func(t *testing.T, opt ...TestOption) *TestVaultServer = skipNewServer - mountDatabase func(t *testing.T, v *TestVaultServer, opt ...TestOption) *TestDatabase = skipMountDatabase + newVaultServer func(t testing.TB, opt ...TestOption) *TestVaultServer = skipNewServer + mountDatabase func(t testing.TB, v *TestVaultServer, opt ...TestOption) *TestDatabase = skipMountDatabase ) -func skipNewServer(t *testing.T, opt ...TestOption) *TestVaultServer { +func skipNewServer(t testing.TB, opt ...TestOption) *TestVaultServer { t.Skip("docker not available") return nil } -func skipMountDatabase(t *testing.T, v *TestVaultServer, opt ...TestOption) *TestDatabase { +func skipMountDatabase(t testing.TB, v *TestVaultServer, opt ...TestOption) *TestDatabase { t.Skip("docker not available") return nil } diff --git a/internal/credential/vault/supported.go b/internal/credential/vault/supported.go index 1cfcf95ac..2bcb103ff 100644 --- a/internal/credential/vault/supported.go +++ b/internal/credential/vault/supported.go @@ -30,9 +30,9 @@ func init() { mountDatabase = gotMountDatabase } -func gotDocker(t *testing.T) {} +func gotDocker(t testing.TB) {} -func gotNewServer(t *testing.T, opt ...TestOption) *TestVaultServer { +func gotNewServer(t testing.TB, opt ...TestOption) *TestVaultServer { const ( serverTlsTemplate = `{ "listener": [ @@ -193,7 +193,7 @@ func gotNewServer(t *testing.T, opt ...TestOption) *TestVaultServer { return server } -func gotMountDatabase(t *testing.T, v *TestVaultServer, opt ...TestOption) *TestDatabase { +func gotMountDatabase(t testing.TB, v *TestVaultServer, opt ...TestOption) *TestDatabase { require := require.New(t) require.Nil(v.postgresContainer, "postgres container exists") require.NotNil(v.network, "Vault server must be created with docker network") @@ -265,8 +265,10 @@ func gotMountDatabase(t *testing.T, v *TestVaultServer, opt ...TestOption) *Test // Mount Database maxTTL := 24 * time.Hour - if deadline, ok := t.Deadline(); ok { - maxTTL = time.Until(deadline) * 2 + if t, ok := t.(*testing.T); ok { + if deadline, ok := t.Deadline(); ok { + maxTTL = time.Until(deadline) * 2 + } } defaultTTL := maxTTL / 2 @@ -348,7 +350,7 @@ grant closed_role to "{{name}}"; } } -func cleanupResource(t *testing.T, pool *dockertest.Pool, resource *dockertest.Resource) { +func cleanupResource(t testing.TB, pool *dockertest.Pool, resource *dockertest.Resource) { t.Helper() var err error for i := 0; i < 10; i++ { diff --git a/internal/credential/vault/testing.go b/internal/credential/vault/testing.go index f60f49d9a..f6e7f4643 100644 --- a/internal/credential/vault/testing.go +++ b/internal/credential/vault/testing.go @@ -33,7 +33,7 @@ import ( // the provided scope, vault address, token, and accessor and any values passed // in through the Options vargs. If any errors are encountered during the // creation of the store, the test will fail. -func TestCredentialStore(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, scopeId, vaultAddr, vaultToken, accessor string, opts ...Option) *CredentialStore { +func TestCredentialStore(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, scopeId, vaultAddr, vaultToken, accessor string, opts ...Option) *CredentialStore { t.Helper() ctx := context.Background() kmsCache := kms.TestKms(t, conn, wrapper) @@ -74,7 +74,7 @@ func TestCredentialStore(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, sc // the provided DB with the provided scope id. If any errors are // encountered during the creation of the credential stores, the test will // fail. -func TestCredentialStores(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, scopeId string, count int) []*CredentialStore { +func TestCredentialStores(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, scopeId string, count int) []*CredentialStore { t.Helper() ctx := context.Background() kmsCache := kms.TestKms(t, conn, wrapper) @@ -112,7 +112,7 @@ func TestCredentialStores(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, s // libraries in the provided DB with the provided store id. If any errors // are encountered during the creation of the credential libraries, the // test will fail. -func TestCredentialLibraries(t *testing.T, conn *db.DB, _ wrapping.Wrapper, storeId string, count int) []*CredentialLibrary { +func TestCredentialLibraries(t testing.TB, conn *db.DB, _ wrapping.Wrapper, storeId string, count int) []*CredentialLibrary { t.Helper() assert, require := assert.New(t), require.New(t) w := db.New(conn) @@ -143,7 +143,7 @@ func TestCredentialLibraries(t *testing.T, conn *db.DB, _ wrapping.Wrapper, stor // TestCredentials creates count number of vault credentials in the provided DB with // the provided library id and session id. If any errors are encountered // during the creation of the credentials, the test will fail. -func TestCredentials(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, libraryId, sessionId string, count int) []*Credential { +func TestCredentials(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, libraryId, sessionId string, count int) []*Credential { t.Helper() assert, require := assert.New(t), require.New(t) rw := db.New(conn) @@ -187,7 +187,7 @@ func TestCredentials(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, librar return credentials } -func createTestToken(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, scopeId, storeId, token, accessor string) *Token { +func createTestToken(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, scopeId, storeId, token, accessor string) *Token { t.Helper() w := db.New(conn) ctx := context.Background() @@ -209,7 +209,7 @@ func createTestToken(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, scopeI return inToken } -func testTokens(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, scopeId, storeId string, count int) []*Token { +func testTokens(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, scopeId, storeId string, count int) []*Token { t.Helper() assert, require := assert.New(t), require.New(t) w := db.New(conn) @@ -267,7 +267,7 @@ type testCert struct { priv *ecdsa.PrivateKey } -func (tc *testCert) ClientCertificate(t *testing.T) *ClientCertificate { +func (tc *testCert) ClientCertificate(t testing.TB) *ClientCertificate { t.Helper() c, err := NewClientCertificate(tc.Cert, tc.Key) require.NoError(t, err) @@ -280,7 +280,7 @@ type testCertBundle struct { } // testCaCert will generate a test x509 CA cert. -func testCaCert(t *testing.T) *testCert { +func testCaCert(t testing.TB) *testCert { t.Helper() require := require.New(t) @@ -292,8 +292,10 @@ func testCaCert(t *testing.T) *testCert { keyUsage := x509.KeyUsageDigitalSignature notBefore, notAfter := time.Now(), time.Now().Add(24*time.Hour) - if deadLine, ok := t.Deadline(); ok { - notAfter = deadLine + if t, ok := t.(*testing.T); ok { + if deadline, ok := t.Deadline(); ok { + notAfter = deadline + } } serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) @@ -331,7 +333,7 @@ func testCaCert(t *testing.T) *testCert { } // testServerCert will generate a test x509 server cert. -func testServerCert(t *testing.T, ca *testCert, hosts ...string) *testCertBundle { +func testServerCert(t testing.TB, ca *testCert, hosts ...string) *testCertBundle { t.Helper() require := require.New(t) @@ -343,8 +345,10 @@ func testServerCert(t *testing.T, ca *testCert, hosts ...string) *testCertBundle keyUsage := x509.KeyUsageDigitalSignature notBefore, notAfter := time.Now(), time.Now().Add(24*time.Hour) - if deadLine, ok := t.Deadline(); ok { - notAfter = deadLine + if t, ok := t.(*testing.T); ok { + if deadline, ok := t.Deadline(); ok { + notAfter = deadline + } } serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) @@ -396,7 +400,7 @@ func testServerCert(t *testing.T, ca *testCert, hosts ...string) *testCertBundle } // testClientCert will generate a test x509 client cert. -func testClientCert(t *testing.T, ca *testCert, opt ...TestOption) *testCertBundle { +func testClientCert(t testing.TB, ca *testCert, opt ...TestOption) *testCertBundle { t.Helper() require := require.New(t) @@ -413,8 +417,10 @@ func testClientCert(t *testing.T, ca *testCert, opt ...TestOption) *testCertBund keyUsage := x509.KeyUsageDigitalSignature notBefore, notAfter := time.Now(), time.Now().Add(24*time.Hour) - if deadLine, ok := t.Deadline(); ok { - notAfter = deadLine + if t, ok := t.(*testing.T); ok { + if deadline, ok := t.Deadline(); ok { + notAfter = deadline + } } serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) @@ -457,7 +463,7 @@ func testClientCert(t *testing.T, ca *testCert, opt ...TestOption) *testCertBund } } -func getTestOpts(t *testing.T, opt ...TestOption) testOptions { +func getTestOpts(t testing.TB, opt ...TestOption) testOptions { t.Helper() opts := getDefaultTestOptions(t) for _, o := range opt { @@ -467,7 +473,7 @@ func getTestOpts(t *testing.T, opt ...TestOption) testOptions { } // TestOption - how Options are passed as arguments. -type TestOption func(*testing.T, *testOptions) +type TestOption func(testing.TB, *testOptions) // options = how options are represented type testOptions struct { @@ -484,11 +490,13 @@ type testOptions struct { clientKey *ecdsa.PrivateKey } -func getDefaultTestOptions(t *testing.T) testOptions { +func getDefaultTestOptions(t testing.TB) testOptions { t.Helper() defaultPeriod := 24 * time.Hour - if deadline, ok := t.Deadline(); ok { - defaultPeriod = time.Until(deadline) + if t, ok := t.(*testing.T); ok { + if deadline, ok := t.Deadline(); ok { + defaultPeriod = time.Until(deadline) + } } return testOptions{ @@ -509,7 +517,7 @@ func getDefaultTestOptions(t *testing.T) testOptions { // is the value of t.Deadline() or 24 hours if // t.Deadline() is nil. func WithTokenPeriod(d time.Duration) TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() o.tokenPeriod = d } @@ -518,7 +526,7 @@ func WithTokenPeriod(d time.Duration) TestOption { // WithPolicies sets the polices to attach to a token. The default policy // attached to tokens is 'default'. func WithPolicies(p []string) TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() o.policies = p } @@ -527,7 +535,7 @@ func WithPolicies(p []string) TestOption { // WithDockerNetwork sets the option to create docker network when creating // a Vault test server. The default is to not create a docker network. func WithDockerNetwork(b bool) TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() o.dockerNetwork = b } @@ -536,7 +544,7 @@ func WithDockerNetwork(b bool) TestOption { // WithTestVaultTLS sets the Vault TLS option. // TestNoTLS is the default TLS option. func WithTestVaultTLS(s TestVaultTLS) TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() o.vaultTLS = s } @@ -545,7 +553,7 @@ func WithTestVaultTLS(s TestVaultTLS) TestOption { // WithClientKey sets the private key that will be used to generate the // client certificate. The option is only valid when used together with TestClientTLS. func WithClientKey(k *ecdsa.PrivateKey) TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() o.clientKey = k } @@ -553,7 +561,7 @@ func WithClientKey(k *ecdsa.PrivateKey) TestOption { // WithTestMountPath sets the mount path option to p. func WithTestMountPath(p string) TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() p = strings.TrimSpace(p) if p != "" { @@ -566,7 +574,7 @@ func WithTestMountPath(p string) TestOption { // WithTestRoleName sets the roleName name to n. // The default role name is boundary. func WithTestRoleName(n string) TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() n = strings.TrimSpace(n) if n == "" { @@ -579,7 +587,7 @@ func WithTestRoleName(n string) TestOption { // WithDontCleanUp causes the resource created to not // be automaticaly cleaned up at the end of the test run. func WithDontCleanUp() TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() o.skipCleanup = true } @@ -588,7 +596,7 @@ func WithDontCleanUp() TestOption { // TestOrphanToken sets the token orphan option to b. // The orphan option is true by default. func TestOrphanToken(b bool) TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() o.orphan = b } @@ -597,7 +605,7 @@ func TestOrphanToken(b bool) TestOption { // TestPeriodicToken sets the token periodic option to b. // The periodic option is true by default. func TestPeriodicToken(b bool) TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() o.periodic = b } @@ -606,18 +614,18 @@ func TestPeriodicToken(b bool) TestOption { // TestRenewableToken sets the token renewable option to b. // The renewable option is true by default. func TestRenewableToken(b bool) TestOption { - return func(t *testing.T, o *testOptions) { + return func(t testing.TB, o *testOptions) { t.Helper() o.renewable = b } } -func (v *TestVaultServer) client(t *testing.T) *client { +func (v *TestVaultServer) client(t testing.TB) *client { t.Helper() return v.clientUsingToken(t, v.RootToken) } -func (v *TestVaultServer) clientUsingToken(t *testing.T, token string) *client { +func (v *TestVaultServer) clientUsingToken(t testing.TB, token string) *client { t.Helper() require := require.New(t) conf := &clientConfig{ @@ -639,7 +647,7 @@ func (v *TestVaultServer) clientUsingToken(t *testing.T, token string) *client { // using v.RootToken. It returns the vault secret containing the token and // the token itself. See // https://www.vaultproject.io/api-docs/auth/token#create-token. -func (v *TestVaultServer) CreateToken(t *testing.T, opt ...TestOption) (*vault.Secret, string) { +func (v *TestVaultServer) CreateToken(t testing.TB, opt ...TestOption) (*vault.Secret, string) { t.Helper() require := require.New(t) opts := getTestOpts(t, opt...) @@ -668,7 +676,7 @@ func (v *TestVaultServer) CreateToken(t *testing.T, opt ...TestOption) (*vault.S // LookupToken calls /auth/token/lookup on v for the token. See // https://www.vaultproject.io/api-docs/auth/token#lookup-a-token. -func (v *TestVaultServer) LookupToken(t *testing.T, token string) *vault.Secret { +func (v *TestVaultServer) LookupToken(t testing.TB, token string) *vault.Secret { t.Helper() require := require.New(t) vc := v.client(t).cl @@ -681,7 +689,7 @@ func (v *TestVaultServer) LookupToken(t *testing.T, token string) *vault.Secret // VerifyTokenInvalid calls /auth/token/lookup on v for the token. It expects the lookup to // fail with a StatusForbidden. See // https://www.vaultproject.io/api-docs/auth/token#lookup-a-token. -func (v *TestVaultServer) VerifyTokenInvalid(t *testing.T, token string) { +func (v *TestVaultServer) VerifyTokenInvalid(t testing.TB, token string) { t.Helper() require, assert := require.New(t), assert.New(t) vc := v.client(t).cl @@ -698,7 +706,7 @@ func (v *TestVaultServer) VerifyTokenInvalid(t *testing.T, token string) { // LookupLease calls the /sys/leases/lookup Vault endpoint and returns // the vault.Secret response. See // https://www.vaultproject.io/api-docs/system/leases#read-lease. -func (v *TestVaultServer) LookupLease(t *testing.T, leaseId string) *vault.Secret { +func (v *TestVaultServer) LookupLease(t testing.TB, leaseId string) *vault.Secret { t.Helper() require := require.New(t) vc := v.client(t).cl @@ -709,7 +717,7 @@ func (v *TestVaultServer) LookupLease(t *testing.T, leaseId string) *vault.Secre return secret } -func (v *TestVaultServer) addPolicy(t *testing.T, name string, pc pathCapabilities) { +func (v *TestVaultServer) addPolicy(t testing.TB, name string, pc pathCapabilities) { t.Helper() require := require.New(t) policy := pc.vaultPolicy() @@ -733,7 +741,7 @@ func (v *TestVaultServer) addPolicy(t *testing.T, name string, pc pathCapabiliti // path "mountPath/*" { // capabilities = ["create", "read", "update", "delete", "list"] // } -func (v *TestVaultServer) MountPKI(t *testing.T, opt ...TestOption) *vault.Secret { +func (v *TestVaultServer) MountPKI(t testing.TB, opt ...TestOption) *vault.Secret { t.Helper() require := require.New(t) opts := getTestOpts(t, opt...) @@ -741,8 +749,10 @@ func (v *TestVaultServer) MountPKI(t *testing.T, opt ...TestOption) *vault.Secre // Mount PKI maxTTL := 24 * time.Hour - if deadline, ok := t.Deadline(); ok { - maxTTL = time.Until(deadline) * 2 + if t, ok := t.(*testing.T); ok { + if deadline, ok := t.Deadline(); ok { + maxTTL = time.Until(deadline) * 2 + } } defaultTTL := maxTTL / 2 @@ -797,7 +807,7 @@ func (v *TestVaultServer) MountPKI(t *testing.T, opt ...TestOption) *vault.Secre // } // // All options are ignored. -func (v *TestVaultServer) AddKVPolicy(t *testing.T, _ ...TestOption) { +func (v *TestVaultServer) AddKVPolicy(t testing.TB, _ ...TestOption) { t.Helper() pc := pathCapabilities{ @@ -811,7 +821,7 @@ func (v *TestVaultServer) AddKVPolicy(t *testing.T, _ ...TestOption) { // `{"data": {"key": "value", "key2": "value2"}}` // See // https://www.vaultproject.io/api-docs/secret/kv/kv-v2#create-update-secret -func (v *TestVaultServer) CreateKVSecret(t *testing.T, p string, data []byte) *vault.Secret { +func (v *TestVaultServer) CreateKVSecret(t testing.TB, p string, data []byte) *vault.Secret { t.Helper() require := require.New(t) @@ -849,7 +859,7 @@ type TestVaultServer struct { // WithTestVaultTLS and WithDockerNetwork are the only valid options. // Setting the WithDockerNetwork option can significantly increase the // amount of time required for a test to run. -func NewTestVaultServer(t *testing.T, opt ...TestOption) *TestVaultServer { +func NewTestVaultServer(t testing.TB, opt ...TestOption) *TestVaultServer { t.Helper() return newVaultServer(t, opt...) } @@ -868,7 +878,7 @@ func NewTestVaultServer(t *testing.T, opt ...TestOption) *TestVaultServer { // // MountDatabase returns a TestDatabase for testing credentials from the // mount. -func (v *TestVaultServer) MountDatabase(t *testing.T, opt ...TestOption) *TestDatabase { +func (v *TestVaultServer) MountDatabase(t testing.TB, opt ...TestOption) *TestDatabase { t.Helper() return mountDatabase(t, v, opt...) } @@ -878,7 +888,7 @@ func (v *TestVaultServer) MountDatabase(t *testing.T, opt ...TestOption) *TestDa type TestDatabaseURL string // Encode encodes the username and password credentials from s into u. -func (u TestDatabaseURL) Encode(t *testing.T, s *vault.Secret) string { +func (u TestDatabaseURL) Encode(t testing.TB, s *vault.Secret) string { t.Helper() require := require.New(t) require.NotNil(s, "vault.Secret") @@ -897,7 +907,7 @@ type TestDatabase struct { // ValidateCredential tests the credentials in s against d. An error is // returned if the credentials are not valid. -func (d *TestDatabase) ValidateCredential(t *testing.T, s *vault.Secret) error { +func (d *TestDatabase) ValidateCredential(t testing.TB, s *vault.Secret) error { t.Helper() require := require.New(t) require.NotNil(s) diff --git a/internal/db/testing.go b/internal/db/testing.go index a9d3e6ce6..34fe68e7f 100644 --- a/internal/db/testing.go +++ b/internal/db/testing.go @@ -25,7 +25,8 @@ import ( // setup the tests (initialize the database one-time and intialized // testDatabaseURL). Do not close the returned db. Supported options: // WithTestLogLevel, WithTestDatabaseUrl -func TestSetup(t *testing.T, dialect string, opt ...TestOption) (*DB, string) { +func TestSetup(t testing.TB, dialect string, opt ...TestOption) (*DB, string) { + t.Helper() var cleanup func() error var url string var err error @@ -73,7 +74,8 @@ func TestSetup(t *testing.T, dialect string, opt ...TestOption) (*DB, string) { } // TestWrapper initializes an AEAD wrapping.Wrapper for testing the oplog -func TestWrapper(t *testing.T) wrapping.Wrapper { +func TestWrapper(t testing.TB) wrapping.Wrapper { + t.Helper() rootKey := make([]byte, 32) n, err := rand.Read(rootKey) if err != nil { @@ -95,7 +97,7 @@ func TestWrapper(t *testing.T) wrapping.Wrapper { // AssertPublicId is a test helper that asserts that the provided id is in // the format of a public id. -func AssertPublicId(t *testing.T, prefix, actual string) { +func AssertPublicId(t testing.TB, prefix, actual string) { t.Helper() assert.NotEmpty(t, actual) parts := strings.Split(actual, "_") @@ -105,7 +107,7 @@ func AssertPublicId(t *testing.T, prefix, actual string) { // TestDeleteWhere allows you to easily delete resources for testing purposes // including all the current resources. -func TestDeleteWhere(t *testing.T, conn *DB, i interface{}, whereClause string, args ...interface{}) { +func TestDeleteWhere(t testing.TB, conn *DB, i interface{}, whereClause string, args ...interface{}) { t.Helper() require := require.New(t) ctx := context.Background() @@ -122,7 +124,7 @@ func TestDeleteWhere(t *testing.T, conn *DB, i interface{}, whereClause string, // WithResourcePrivateId option is provided, the lookup will use the `resource-private-id` tag. // An error is returned if the entry or it's metadata is not found. Returning an error // allows clients to test if an entry was not written, which is a valid use case. -func TestVerifyOplog(t *testing.T, r Reader, resourceId string, opt ...TestOption) error { +func TestVerifyOplog(t testing.TB, r Reader, resourceId string, opt ...TestOption) error { t.Helper() // sql where clauses @@ -264,14 +266,14 @@ const ( // WithTestLogLevel provides a way to specify a test log level for the // underlying database package (if applicable). -func WithTestLogLevel(_ *testing.T, l TestLogLevel) TestOption { +func WithTestLogLevel(_ testing.TB, l TestLogLevel) TestOption { return func(o *testOptions) { o.withLogLevel = l } } // TestCreateTables will create the test tables for the db pkg -func TestCreateTables(t *testing.T, conn *DB) { +func TestCreateTables(t testing.TB, conn *DB) { t.Helper() t.Cleanup(func() { testDropTables(t, conn) }) require := require.New(t) @@ -283,7 +285,7 @@ func TestCreateTables(t *testing.T, conn *DB) { require.NoError(err) } -func testDropTables(t *testing.T, conn *DB) { +func testDropTables(t testing.TB, conn *DB) { t.Helper() require := require.New(t) testCtx := context.Background() diff --git a/internal/host/plugin/testing.go b/internal/host/plugin/testing.go index d15df853f..069d93112 100644 --- a/internal/host/plugin/testing.go +++ b/internal/host/plugin/testing.go @@ -24,7 +24,7 @@ import ( // TestCatalogs creates count number of static host catalogs to the provided DB // with the provided scope id. If any errors are encountered during the creation of // the host catalog, the test will fail. -func TestCatalogs(t *testing.T, conn *db.DB, scopeId, pluginId string, count int) []*HostCatalog { +func TestCatalogs(t testing.TB, conn *db.DB, scopeId, pluginId string, count int) []*HostCatalog { t.Helper() var cats []*HostCatalog for i := 0; i < count; i++ { @@ -36,7 +36,7 @@ func TestCatalogs(t *testing.T, conn *db.DB, scopeId, pluginId string, count int // TestCatalog creates a plugin host catalogs to the provided DB // with the provided scope id. If any errors are encountered during the creation of // the host catalog, the test will fail. -func TestCatalog(t *testing.T, conn *db.DB, scopeId, pluginId string, opt ...Option) *HostCatalog { +func TestCatalog(t testing.TB, conn *db.DB, scopeId, pluginId string, opt ...Option) *HostCatalog { t.Helper() ctx := context.Background() w := db.New(conn) @@ -61,7 +61,7 @@ func TestCatalog(t *testing.T, conn *db.DB, scopeId, pluginId string, opt ...Opt // TestSet creates a plugin host sets in the provided DB // with the provided catalog id. The catalog must have been created // previously. The test will fail if any errors are encountered. -func TestSet(t *testing.T, conn *db.DB, kmsCache *kms.Kms, sched *scheduler.Scheduler, hc *HostCatalog, plgm map[string]plgpb.HostPluginServiceClient, opt ...Option) *HostSet { +func TestSet(t testing.TB, conn *db.DB, kmsCache *kms.Kms, sched *scheduler.Scheduler, hc *HostCatalog, plgm map[string]plgpb.HostPluginServiceClient, opt ...Option) *HostSet { t.Helper() require := require.New(t) ctx := context.Background() @@ -91,7 +91,7 @@ func TestSet(t *testing.T, conn *db.DB, kmsCache *kms.Kms, sched *scheduler.Sche // TestSetMembers adds hosts to the specified setId in the provided DB. // The set and hosts must have been created previously and belong to the // same catalog. The test will fail if any errors are encountered. -func TestSetMembers(t *testing.T, conn *db.DB, setId string, hosts []*Host) []*HostSetMember { +func TestSetMembers(t testing.TB, conn *db.DB, setId string, hosts []*Host) []*HostSetMember { t.Helper() assert := assert.New(t) @@ -112,7 +112,7 @@ func TestSetMembers(t *testing.T, conn *db.DB, setId string, hosts []*Host) []*H // TestHost creates a plugin host in the provided DB in the catalog with the // provided catalog id. The catalog must have been created previously. // The test will fail if any errors are encountered. -func TestHost(t *testing.T, conn *db.DB, catId, externId string, opt ...Option) *Host { +func TestHost(t testing.TB, conn *db.DB, catId, externId string, opt ...Option) *Host { t.Helper() w := db.New(conn) ctx := context.Background() @@ -148,7 +148,7 @@ func TestHost(t *testing.T, conn *db.DB, catId, externId string, opt ...Option) return host1 } -func TestExternalHosts(t *testing.T, catalog *HostCatalog, setIds []string, count int) ([]*plgpb.ListHostsResponseHost, []*Host) { +func TestExternalHosts(t testing.TB, catalog *HostCatalog, setIds []string, count int) ([]*plgpb.ListHostsResponseHost, []*Host) { t.Helper() require := require.New(t) retRH := make([]*plgpb.ListHostsResponseHost, 0, count) @@ -198,7 +198,7 @@ func TestExternalHosts(t *testing.T, catalog *HostCatalog, setIds []string, coun } // TestRunSetSync runs the set sync job a single time. -func TestRunSetSync(t *testing.T, conn *db.DB, kmsCache *kms.Kms, plgm map[string]plgpb.HostPluginServiceClient) { +func TestRunSetSync(t testing.TB, conn *db.DB, kmsCache *kms.Kms, plgm map[string]plgpb.HostPluginServiceClient) { t.Helper() rw := db.New(conn) ctx := context.Background() @@ -208,13 +208,13 @@ func TestRunSetSync(t *testing.T, conn *db.DB, kmsCache *kms.Kms, plgm map[strin require.NoError(t, j.Run(ctx)) } -func testGetDnsName(t *testing.T) string { +func testGetDnsName(t testing.TB) string { dnsName, err := base62.Random(10) require.NoError(t, err) return fmt.Sprintf("%s.example.com", dnsName) } -func testGetIpAddress(t *testing.T) string { +func testGetIpAddress(t testing.TB) string { ipBytes := make([]byte, 4) for { lr := io.LimitReader(rand.Reader, 4) diff --git a/internal/host/static/testing.go b/internal/host/static/testing.go index 8dbca5444..8de982518 100644 --- a/internal/host/static/testing.go +++ b/internal/host/static/testing.go @@ -12,7 +12,7 @@ import ( // TestCatalogs creates count number of static host catalogs to the provided DB // with the provided scope id. If any errors are encountered during the creation of // the host catalog, the test will fail. -func TestCatalogs(t *testing.T, conn *db.DB, scopeId string, count int) []*HostCatalog { +func TestCatalogs(t testing.TB, conn *db.DB, scopeId string, count int) []*HostCatalog { t.Helper() assert := assert.New(t) var cats []*HostCatalog @@ -36,7 +36,7 @@ func TestCatalogs(t *testing.T, conn *db.DB, scopeId string, count int) []*HostC // TestHosts creates count number of static hosts to the provided DB // with the provided catalog id. The catalog must have been created previously. // If any errors are encountered during the creation of the host, the test will fail. -func TestHosts(t *testing.T, conn *db.DB, catalogId string, count int) []*Host { +func TestHosts(t testing.TB, conn *db.DB, catalogId string, count int) []*Host { t.Helper() assert := assert.New(t) var hosts []*Host @@ -62,7 +62,7 @@ func TestHosts(t *testing.T, conn *db.DB, catalogId string, count int) []*Host { // TestSets creates count number of static host sets in the provided DB // with the provided catalog id. The catalog must have been created // previously. The test will fail if any errors are encountered. -func TestSets(t *testing.T, conn *db.DB, catalogId string, count int) []*HostSet { +func TestSets(t testing.TB, conn *db.DB, catalogId string, count int) []*HostSet { t.Helper() assert := assert.New(t) var sets []*HostSet @@ -87,7 +87,7 @@ func TestSets(t *testing.T, conn *db.DB, catalogId string, count int) []*HostSet // TestSetMembers adds hosts to the specified setId in the provided DB. // The set and hosts must have been created previously and belong to the // same catalog. The test will fail if any errors are encountered. -func TestSetMembers(t *testing.T, conn *db.DB, setId string, hosts []*Host) []*HostSetMember { +func TestSetMembers(t testing.TB, conn *db.DB, setId string, hosts []*Host) []*HostSetMember { t.Helper() assert := assert.New(t) diff --git a/internal/iam/testing.go b/internal/iam/testing.go index 2c784bfba..81f9cb3be 100644 --- a/internal/iam/testing.go +++ b/internal/iam/testing.go @@ -17,7 +17,8 @@ import ( // TestRepo creates a repo that can be used for various purposes. Crucially, it // ensures that the global scope contains a valid root key. -func TestRepo(t *testing.T, conn *db.DB, rootWrapper wrapping.Wrapper, opt ...Option) *Repository { +func TestRepo(t testing.TB, conn *db.DB, rootWrapper wrapping.Wrapper, opt ...Option) *Repository { + t.Helper() require := require.New(t) rw := db.New(conn) kmsCache := kms.TestKms(t, conn, rootWrapper) @@ -39,7 +40,7 @@ func TestRepo(t *testing.T, conn *db.DB, rootWrapper wrapping.Wrapper, opt ...Op } // TestSetPrimaryAuthMethod will set the PrimaryAuthMethodId for a scope. -func TestSetPrimaryAuthMethod(t *testing.T, repo *Repository, s *Scope, authMethodId string) { +func TestSetPrimaryAuthMethod(t testing.TB, repo *Repository, s *Scope, authMethodId string) { t.Helper() require := require.New(t) require.NotEmpty(s) @@ -54,7 +55,7 @@ func TestSetPrimaryAuthMethod(t *testing.T, repo *Repository, s *Scope, authMeth } // TestScopes creates an org and project suitable for testing. -func TestScopes(t *testing.T, repo *Repository, opt ...Option) (org *Scope, prj *Scope) { +func TestScopes(t testing.TB, repo *Repository, opt ...Option) (org *Scope, prj *Scope) { t.Helper() require := require.New(t) @@ -77,7 +78,7 @@ func TestScopes(t *testing.T, repo *Repository, opt ...Option) (org *Scope, prj return } -func TestOrg(t *testing.T, repo *Repository, opt ...Option) *Scope { +func TestOrg(t testing.TB, repo *Repository, opt ...Option) *Scope { t.Helper() require := require.New(t) @@ -93,7 +94,7 @@ func TestOrg(t *testing.T, repo *Repository, opt ...Option) *Scope { return org } -func TestProject(t *testing.T, repo *Repository, orgId string, opt ...Option) *Scope { +func TestProject(t testing.TB, repo *Repository, orgId string, opt ...Option) *Scope { t.Helper() require := require.New(t) @@ -109,7 +110,7 @@ func TestProject(t *testing.T, repo *Repository, orgId string, opt ...Option) *S return proj } -func testOrg(t *testing.T, repo *Repository, name, description string) (org *Scope) { +func testOrg(t testing.TB, repo *Repository, name, description string) (org *Scope) { t.Helper() require := require.New(t) @@ -123,7 +124,7 @@ func testOrg(t *testing.T, repo *Repository, name, description string) (org *Sco return o } -func testProject(t *testing.T, repo *Repository, orgId string, opt ...Option) *Scope { +func testProject(t testing.TB, repo *Repository, orgId string, opt ...Option) *Scope { t.Helper() require := require.New(t) @@ -137,14 +138,14 @@ func testProject(t *testing.T, repo *Repository, orgId string, opt ...Option) *S return p } -func testId(t *testing.T) string { +func testId(t testing.TB) string { t.Helper() id, err := uuid.GenerateUUID() require.NoError(t, err) return id } -func testPublicId(t *testing.T, prefix string) string { +func testPublicId(t testing.TB, prefix string) string { t.Helper() publicId, err := db.NewPublicId(prefix) require.NoError(t, err) @@ -153,7 +154,7 @@ func testPublicId(t *testing.T, prefix string) string { // TestUser creates a user suitable for testing. Supports the options: // WithName, WithDescription and WithAccountIds. -func TestUser(t *testing.T, repo *Repository, scopeId string, opt ...Option) *User { +func TestUser(t testing.TB, repo *Repository, scopeId string, opt ...Option) *User { t.Helper() require := require.New(t) @@ -171,7 +172,7 @@ func TestUser(t *testing.T, repo *Repository, scopeId string, opt ...Option) *Us } // TestRole creates a role suitable for testing. -func TestRole(t *testing.T, conn *db.DB, scopeId string, opt ...Option) *Role { +func TestRole(t testing.TB, conn *db.DB, scopeId string, opt ...Option) *Role { t.Helper() require := require.New(t) rw := db.New(conn) @@ -191,7 +192,7 @@ func TestRole(t *testing.T, conn *db.DB, scopeId string, opt ...Option) *Role { return role } -func TestRoleGrant(t *testing.T, conn *db.DB, roleId, grant string, opt ...Option) *RoleGrant { +func TestRoleGrant(t testing.TB, conn *db.DB, roleId, grant string, opt ...Option) *RoleGrant { t.Helper() require := require.New(t) rw := db.New(conn) @@ -204,7 +205,7 @@ func TestRoleGrant(t *testing.T, conn *db.DB, roleId, grant string, opt ...Optio } // TestGroup creates a group suitable for testing. -func TestGroup(t *testing.T, conn *db.DB, scopeId string, opt ...Option) *Group { +func TestGroup(t testing.TB, conn *db.DB, scopeId string, opt ...Option) *Group { t.Helper() require := require.New(t) rw := db.New(conn) @@ -220,7 +221,7 @@ func TestGroup(t *testing.T, conn *db.DB, scopeId string, opt ...Option) *Group return grp } -func TestGroupMember(t *testing.T, conn *db.DB, groupId, userId string, opt ...Option) *GroupMemberUser { +func TestGroupMember(t testing.TB, conn *db.DB, groupId, userId string, opt ...Option) *GroupMemberUser { t.Helper() require := require.New(t) rw := db.New(conn) @@ -233,7 +234,7 @@ func TestGroupMember(t *testing.T, conn *db.DB, groupId, userId string, opt ...O return gm } -func TestUserRole(t *testing.T, conn *db.DB, roleId, userId string, opt ...Option) *UserRole { +func TestUserRole(t testing.TB, conn *db.DB, roleId, userId string, opt ...Option) *UserRole { t.Helper() require := require.New(t) rw := db.New(conn) @@ -245,7 +246,7 @@ func TestUserRole(t *testing.T, conn *db.DB, roleId, userId string, opt ...Optio return r } -func TestGroupRole(t *testing.T, conn *db.DB, roleId, grpId string, opt ...Option) *GroupRole { +func TestGroupRole(t testing.TB, conn *db.DB, roleId, grpId string, opt ...Option) *GroupRole { t.Helper() require := require.New(t) rw := db.New(conn) @@ -257,7 +258,7 @@ func TestGroupRole(t *testing.T, conn *db.DB, roleId, grpId string, opt ...Optio return r } -func TestManagedGroupRole(t *testing.T, conn *db.DB, roleId, managedGrpId string, opt ...Option) *ManagedGroupRole { +func TestManagedGroupRole(t testing.TB, conn *db.DB, roleId, managedGrpId string, opt ...Option) *ManagedGroupRole { t.Helper() require := require.New(t) rw := db.New(conn) @@ -272,7 +273,7 @@ func TestManagedGroupRole(t *testing.T, conn *db.DB, roleId, managedGrpId string // testAccount is a temporary test function. TODO - replace with an auth // subsystem testAccount function. If userId is zero value, then an auth // account will be created with a null IamUserId -func testAccount(t *testing.T, conn *db.DB, scopeId, authMethodId, userId string) *authAccount { +func testAccount(t testing.TB, conn *db.DB, scopeId, authMethodId, userId string) *authAccount { const ( accountPrefix = "aa_" ) @@ -326,7 +327,7 @@ func testAccount(t *testing.T, conn *db.DB, scopeId, authMethodId, userId string // testAuthMethod is a temporary test function. TODO - replace with an auth // subsystem testAuthMethod function. -func testAuthMethod(t *testing.T, conn *db.DB, scopeId string) string { +func testAuthMethod(t testing.TB, conn *db.DB, scopeId string) string { const ( authMethodPrefix = "am_" ) diff --git a/internal/kms/testing.go b/internal/kms/testing.go index b6d446203..aaf2e4ea2 100644 --- a/internal/kms/testing.go +++ b/internal/kms/testing.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestRootKey(t *testing.T, conn *db.DB, scopeId string) *RootKey { +func TestRootKey(t testing.TB, conn *db.DB, scopeId string) *RootKey { t.Helper() require := require.New(t) db.TestDeleteWhere(t, conn, func() interface{} { i := AllocRootKey(); return &i }(), "scope_id = ?", scopeId) @@ -25,7 +25,7 @@ func TestRootKey(t *testing.T, conn *db.DB, scopeId string) *RootKey { return k } -func TestRootKeyVersion(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, rootId string) (kv *RootKeyVersion, kvWrapper wrapping.Wrapper) { +func TestRootKeyVersion(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, rootId string) (kv *RootKeyVersion, kvWrapper wrapping.Wrapper) { t.Helper() require := require.New(t) rw := db.New(conn) @@ -46,7 +46,7 @@ func TestRootKeyVersion(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, roo return k, rootKeyVersionWrapper } -func TestKms(t *testing.T, conn *db.DB, rootWrapper wrapping.Wrapper) *Kms { +func TestKms(t testing.TB, conn *db.DB, rootWrapper wrapping.Wrapper) *Kms { t.Helper() require := require.New(t) rw := db.New(conn) @@ -59,7 +59,7 @@ func TestKms(t *testing.T, conn *db.DB, rootWrapper wrapping.Wrapper) *Kms { return kms } -func TestDatabaseKey(t *testing.T, conn *db.DB, rootKeyId string) *DatabaseKey { +func TestDatabaseKey(t testing.TB, conn *db.DB, rootKeyId string) *DatabaseKey { t.Helper() require := require.New(t) rw := db.New(conn) @@ -75,7 +75,7 @@ func TestDatabaseKey(t *testing.T, conn *db.DB, rootKeyId string) *DatabaseKey { return k } -func TestDatabaseKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, databaseKeyId string, key []byte) *DatabaseKeyVersion { +func TestDatabaseKeyVersion(t testing.TB, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, databaseKeyId string, key []byte) *DatabaseKeyVersion { t.Helper() require := require.New(t) rw := db.New(conn) @@ -95,7 +95,7 @@ func TestDatabaseKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wra return k } -func TestOplogKey(t *testing.T, conn *db.DB, rootKeyId string) *OplogKey { +func TestOplogKey(t testing.TB, conn *db.DB, rootKeyId string) *OplogKey { t.Helper() require := require.New(t) rw := db.New(conn) @@ -111,7 +111,7 @@ func TestOplogKey(t *testing.T, conn *db.DB, rootKeyId string) *OplogKey { return k } -func TestOplogKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, oplogKeyId string, key []byte) *OplogKeyVersion { +func TestOplogKeyVersion(t testing.TB, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, oplogKeyId string, key []byte) *OplogKeyVersion { t.Helper() require := require.New(t) rw := db.New(conn) @@ -131,7 +131,7 @@ func TestOplogKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wrappi return k } -func TestTokenKey(t *testing.T, conn *db.DB, rootKeyId string) *TokenKey { +func TestTokenKey(t testing.TB, conn *db.DB, rootKeyId string) *TokenKey { t.Helper() require := require.New(t) db.TestDeleteWhere(t, conn, func() interface{} { i := AllocTokenKey(); return &i }(), "root_key_id = ?", rootKeyId) @@ -147,7 +147,7 @@ func TestTokenKey(t *testing.T, conn *db.DB, rootKeyId string) *TokenKey { return k } -func TestTokenKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, tokenKeyId string, key []byte) *TokenKeyVersion { +func TestTokenKeyVersion(t testing.TB, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, tokenKeyId string, key []byte) *TokenKeyVersion { t.Helper() require := require.New(t) rw := db.New(conn) @@ -167,7 +167,7 @@ func TestTokenKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wrappi return k } -func TestSessionKey(t *testing.T, conn *db.DB, rootKeyId string) *SessionKey { +func TestSessionKey(t testing.TB, conn *db.DB, rootKeyId string) *SessionKey { t.Helper() require := require.New(t) db.TestDeleteWhere(t, conn, func() interface{} { i := AllocSessionKey(); return &i }(), "root_key_id = ?", rootKeyId) @@ -183,7 +183,7 @@ func TestSessionKey(t *testing.T, conn *db.DB, rootKeyId string) *SessionKey { return k } -func TestSessionKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, sessionKeyId string, key []byte) *SessionKeyVersion { +func TestSessionKeyVersion(t testing.TB, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, sessionKeyId string, key []byte) *SessionKeyVersion { t.Helper() require := require.New(t) rw := db.New(conn) @@ -203,7 +203,7 @@ func TestSessionKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wrap return k } -func TestOidcKey(t *testing.T, conn *db.DB, rootKeyId string) *OidcKey { +func TestOidcKey(t testing.TB, conn *db.DB, rootKeyId string) *OidcKey { t.Helper() require := require.New(t) db.TestDeleteWhere(t, conn, func() interface{} { i := AllocOidcKey(); return &i }(), "root_key_id = ?", rootKeyId) @@ -219,7 +219,7 @@ func TestOidcKey(t *testing.T, conn *db.DB, rootKeyId string) *OidcKey { return k } -func TestOidcKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, oidcKeyId string, key []byte) *OidcKeyVersion { +func TestOidcKeyVersion(t testing.TB, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, oidcKeyId string, key []byte) *OidcKeyVersion { t.Helper() require := require.New(t) rw := db.New(conn) @@ -239,7 +239,7 @@ func TestOidcKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wrappin return k } -func TestAuditKey(t *testing.T, conn *db.DB, rootKeyId string) *AuditKey { +func TestAuditKey(t testing.TB, conn *db.DB, rootKeyId string) *AuditKey { t.Helper() ctx := context.Background() require := require.New(t) @@ -256,7 +256,7 @@ func TestAuditKey(t *testing.T, conn *db.DB, rootKeyId string) *AuditKey { return k } -func TestAuditKeyVersion(t *testing.T, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, auditKeyId string, key []byte) *AuditKeyVersion { +func TestAuditKeyVersion(t testing.TB, conn *db.DB, rootKeyVersionWrapper wrapping.Wrapper, auditKeyId string, key []byte) *AuditKeyVersion { t.Helper() ctx := context.Background() require := require.New(t) diff --git a/internal/libs/crypto/testing.go b/internal/libs/crypto/testing.go index 35d61ba88..73c5e1e9c 100644 --- a/internal/libs/crypto/testing.go +++ b/internal/libs/crypto/testing.go @@ -11,7 +11,7 @@ import ( ) // TestWrapper initializes an AEAD wrapping.Wrapper for testing -func TestWrapper(t *testing.T) wrapping.Wrapper { +func TestWrapper(t testing.TB) wrapping.Wrapper { rootKey := make([]byte, 32) n, err := rand.Read(rootKey) if err != nil { diff --git a/internal/observability/event/testing.go b/internal/observability/event/testing.go index 54b8bde0b..e4220dd67 100644 --- a/internal/observability/event/testing.go +++ b/internal/observability/event/testing.go @@ -15,13 +15,13 @@ import ( ) // TestGetEventerConfig is a test accessor for the eventer's config -func TestGetEventerConfig(t *testing.T, e *Eventer) EventerConfig { +func TestGetEventerConfig(t testing.TB, e *Eventer) EventerConfig { t.Helper() return e.conf } // TestResetSysEventer will reset event.syseventer to an uninitialized state. -func TestResetSystEventer(t *testing.T) { +func TestResetSystEventer(t testing.TB) { t.Helper() sysEventerLock.Lock() defer sysEventerLock.Unlock() @@ -38,7 +38,7 @@ type TestConfig struct { // TestEventerConfig creates a test config and registers a cleanup func for its // test tmp files. -func TestEventerConfig(t *testing.T, testName string, opt ...Option) TestConfig { +func TestEventerConfig(t testing.TB, testName string, opt ...Option) TestConfig { t.Helper() require := require.New(t) tmpAllFile, err := ioutil.TempFile("./", "tmp-all-events-"+testName) @@ -153,7 +153,7 @@ func TestEventerConfig(t *testing.T, testName string, opt ...Option) TestConfig } // TestRequestInfo provides a test RequestInfo -func TestRequestInfo(t *testing.T) *RequestInfo { +func TestRequestInfo(t testing.TB) *RequestInfo { t.Helper() return &RequestInfo{ EventId: "test-event-id", @@ -164,7 +164,7 @@ func TestRequestInfo(t *testing.T) *RequestInfo { } } -func testAuth(t *testing.T) *Auth { +func testAuth(t testing.TB) *Auth { t.Helper() return &Auth{ UserEmail: "test-auth@example.com", @@ -172,7 +172,7 @@ func testAuth(t *testing.T) *Auth { } } -func testRequest(t *testing.T) *Request { +func testRequest(t testing.TB) *Request { t.Helper() return &Request{ Operation: "op", @@ -183,7 +183,7 @@ func testRequest(t *testing.T) *Request { } } -func testResponse(t *testing.T) *Response { +func testResponse(t testing.TB) *Response { t.Helper() return &Response{ StatusCode: 200, @@ -200,7 +200,7 @@ func testResponse(t *testing.T) *Response { } // TestWithBroker is an unexported and a test option for passing in an optional broker -func TestWithBroker(t *testing.T, b broker) Option { +func TestWithBroker(t testing.TB, b broker) Option { t.Helper() return func(o *options) { o.withBroker = b @@ -208,7 +208,7 @@ func TestWithBroker(t *testing.T, b broker) Option { } // TestWithObservationSink is a test option -func TestWithObservationSink(t *testing.T) Option { +func TestWithObservationSink(t testing.TB) Option { t.Helper() return func(o *options) { o.withObservationSink = true @@ -216,7 +216,7 @@ func TestWithObservationSink(t *testing.T) Option { } // TestWithAuditSink is a test option -func TestWithAuditSink(t *testing.T) Option { +func TestWithAuditSink(t testing.TB) Option { t.Helper() return func(o *options) { o.withAuditSink = true @@ -224,7 +224,7 @@ func TestWithAuditSink(t *testing.T) Option { } // TestWithSysSink is a test option -func TestWithSysSink(t *testing.T) Option { +func TestWithSysSink(t testing.TB) Option { t.Helper() return func(o *options) { o.withSysSink = true @@ -232,7 +232,7 @@ func TestWithSysSink(t *testing.T) Option { } // testWithSinkFormat is an unexported and a test option -func testWithSinkFormat(t *testing.T, fmt SinkFormat) Option { +func testWithSinkFormat(t testing.TB, fmt SinkFormat) Option { t.Helper() return func(o *options) { o.withSinkFormat = fmt diff --git a/internal/oplog/oplog_test/db.go b/internal/oplog/oplog_test/db.go index 9f3bc77fa..ffa159596 100644 --- a/internal/oplog/oplog_test/db.go +++ b/internal/oplog/oplog_test/db.go @@ -16,7 +16,7 @@ const ( ) // Init will use gorm migrations to init tables for test models -func Init(t *testing.T, db *dbw.DB) { +func Init(t testing.TB, db *dbw.DB) { const testQueryCreateTables = ` begin; diff --git a/internal/oplog/testing.go b/internal/oplog/testing.go index a8724660e..05cb7f3b4 100644 --- a/internal/oplog/testing.go +++ b/internal/oplog/testing.go @@ -18,7 +18,7 @@ import ( ) // setup the tests (initialize the database one-time and intialized testDatabaseURL) -func setup(t *testing.T) *dbw.DB { +func setup(t testing.TB) *dbw.DB { t.Helper() require := require.New(t) cleanup, url, err := testInitDbInDocker(t) @@ -33,7 +33,7 @@ func setup(t *testing.T) *dbw.DB { return db } -func testOpen(t *testing.T, dbType string, connectionUrl string) *dbw.DB { +func testOpen(t testing.TB, dbType string, connectionUrl string) *dbw.DB { t.Helper() require := require.New(t) var dialect dbw.Dialector @@ -52,7 +52,7 @@ func testOpen(t *testing.T, dbType string, connectionUrl string) *dbw.DB { return db } -func testUser(t *testing.T, db *dbw.DB, name, phoneNumber, email string) *oplog_test.TestUser { +func testUser(t testing.TB, db *dbw.DB, name, phoneNumber, email string) *oplog_test.TestUser { t.Helper() u := &oplog_test.TestUser{ Name: name, @@ -63,21 +63,21 @@ func testUser(t *testing.T, db *dbw.DB, name, phoneNumber, email string) *oplog_ return u } -func testFindUser(t *testing.T, db *dbw.DB, userId uint32) *oplog_test.TestUser { +func testFindUser(t testing.TB, db *dbw.DB, userId uint32) *oplog_test.TestUser { t.Helper() var foundUser oplog_test.TestUser require.NoError(t, dbw.New(db).LookupWhere(context.Background(), &foundUser, "id = ?", []interface{}{userId})) return &foundUser } -func testId(t *testing.T) string { +func testId(t testing.TB) string { t.Helper() id, err := dbw.NewId("i") require.NoError(t, err) return id } -func testInitDbInDocker(t *testing.T) (cleanup func() error, retURL string, err error) { +func testInitDbInDocker(t testing.TB) (cleanup func() error, retURL string, err error) { t.Helper() require := require.New(t) cleanup, retURL, _, err = dbtest.StartUsingTemplate(dbtest.Postgres) @@ -87,7 +87,7 @@ func testInitDbInDocker(t *testing.T) (cleanup func() error, retURL string, err } // testWrapper initializes an AEAD wrapping.Wrapper for testing the oplog -func testWrapper(t *testing.T) wrapping.Wrapper { +func testWrapper(t testing.TB) wrapping.Wrapper { t.Helper() rootKey := make([]byte, 32) n, err := rand.Read(rootKey) @@ -100,7 +100,7 @@ func testWrapper(t *testing.T) wrapping.Wrapper { } // testInitStore will execute the migrations needed to initialize the store for tests -func testInitStore(t *testing.T, cleanup func() error, url string) { +func testInitStore(t testing.TB, cleanup func() error, url string) { t.Helper() ctx := context.Background() dialect := "postgres" @@ -117,7 +117,7 @@ type constraintResults struct { TableName string } -func testListConstraints(t *testing.T, db *dbw.DB, tableName string) []constraintResults { +func testListConstraints(t testing.TB, db *dbw.DB, tableName string) []constraintResults { t.Helper() require := require.New(t) testCtx := context.Background() diff --git a/internal/plugin/host/testing.go b/internal/plugin/host/testing.go index e9a0e2b9e..8440b8801 100644 --- a/internal/plugin/host/testing.go +++ b/internal/plugin/host/testing.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestPlugin(t *testing.T, conn *db.DB, name string) *Plugin { +func TestPlugin(t testing.TB, conn *db.DB, name string) *Plugin { t.Helper() p := NewPlugin(WithName(name)) id, err := newPluginId() diff --git a/internal/plugin/testing.go b/internal/plugin/testing.go index 01b6efca5..02a93f72f 100644 --- a/internal/plugin/testing.go +++ b/internal/plugin/testing.go @@ -41,7 +41,7 @@ func (c *plugin) SetTableName(n string) { c.tableName = n } -func testPlugin(t *testing.T, conn *db.DB, name string) *plugin { +func testPlugin(t testing.TB, conn *db.DB, name string) *plugin { t.Helper() p := newPlugin(name) id, err := db.NewPublicId("plg") diff --git a/internal/scheduler/job/testing.go b/internal/scheduler/job/testing.go index 60455924c..8a2fa6852 100644 --- a/internal/scheduler/job/testing.go +++ b/internal/scheduler/job/testing.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/require" ) -func testJob(t *testing.T, conn *db.DB, name, description string, wrapper wrapping.Wrapper, opt ...Option) *Job { +func testJob(t testing.TB, conn *db.DB, name, description string, wrapper wrapping.Wrapper, opt ...Option) *Job { t.Helper() require := require.New(t) rw := db.New(conn) @@ -88,7 +88,7 @@ func testRunWithUpdateTime(conn *db.DB, pluginId, name, cId string, updateTime t return run, nil } -func testController(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper) *servers.Server { +func testController(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper) *servers.Server { t.Helper() rw := db.New(conn) kms := kms.TestKms(t, conn, wrapper) diff --git a/internal/scheduler/testing.go b/internal/scheduler/testing.go index dac7832ac..e11672b0b 100644 --- a/internal/scheduler/testing.go +++ b/internal/scheduler/testing.go @@ -21,7 +21,7 @@ import ( // // WithRunJobsLimit, WithRunJobsInterval, WithMonitorInterval and WithInterruptThreshold are // the only valid options. -func TestScheduler(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, opt ...Option) *Scheduler { +func TestScheduler(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, opt ...Option) *Scheduler { t.Helper() rw := db.New(conn) diff --git a/internal/servers/controller/testing.go b/internal/servers/controller/testing.go index 19c805f02..39a3663fd 100644 --- a/internal/servers/controller/testing.go +++ b/internal/servers/controller/testing.go @@ -58,7 +58,7 @@ const ( type TestController struct { b *base.Server c *Controller - t *testing.T + t testing.TB apiAddrs []string // The address the Controller API is listening on clusterAddrs []string client *api.Client @@ -418,7 +418,7 @@ type TestControllerOpts struct { SchedulerRunJobInterval time.Duration } -func NewTestController(t *testing.T, opts *TestControllerOpts) *TestController { +func NewTestController(t testing.TB, opts *TestControllerOpts) *TestController { const op = "controller.NewTestController" ctx, cancel := context.WithCancel(context.Background()) @@ -455,7 +455,7 @@ func NewTestController(t *testing.T, opts *TestControllerOpts) *TestController { // TestControllerConfig provides a way to create a config for a TestController. // The tc passed as a parameter will be modified by this func. -func TestControllerConfig(t *testing.T, ctx context.Context, tc *TestController, opts *TestControllerOpts) *Config { +func TestControllerConfig(t testing.TB, ctx context.Context, tc *TestController, opts *TestControllerOpts) *Config { const op = "controller.TestControllerConfig" if opts == nil { opts = new(TestControllerOpts) @@ -667,7 +667,7 @@ func TestControllerConfig(t *testing.T, ctx context.Context, tc *TestController, } } -func (tc *TestController) AddClusterControllerMember(t *testing.T, opts *TestControllerOpts) *TestController { +func (tc *TestController) AddClusterControllerMember(t testing.TB, opts *TestControllerOpts) *TestController { const op = "controller.(TestController).AddClusterControllerMember" if opts == nil { opts = new(TestControllerOpts) @@ -778,7 +778,7 @@ func (tc *TestController) WaitForNextWorkerStatusUpdate(workerId string) error { // test greeter service wrapped with the specified interceptors and return an // initialized client for the service. The test service will be stopped/cleaned // up after the test (or subtests) have completed. -func startTestGreeterService(t *testing.T, greeter interceptor.GreeterServiceServer, interceptors ...grpc.UnaryServerInterceptor) interceptor.GreeterServiceClient { +func startTestGreeterService(t testing.TB, greeter interceptor.GreeterServiceServer, interceptors ...grpc.UnaryServerInterceptor) interceptor.GreeterServiceClient { t.Helper() require := require.New(t) dialCtx := context.Background() diff --git a/internal/servers/worker/proxy/testing.go b/internal/servers/worker/proxy/testing.go index ec062e47b..672af3e00 100644 --- a/internal/servers/worker/proxy/testing.go +++ b/internal/servers/worker/proxy/testing.go @@ -18,7 +18,7 @@ import ( // created during Boundary connect. The proxyConn returned should be used as the connection // passed into the worker proxy handler, while the clientConn can be used to simulate // the local end user connection. -func TestWsConn(t *testing.T, ctx context.Context) (clientConn, proxyConn *websocket.Conn) { +func TestWsConn(t testing.TB, ctx context.Context) (clientConn, proxyConn *websocket.Conn) { t.Helper() require, assert := require.New(t), assert.New(t) diff --git a/internal/servers/worker/testing.go b/internal/servers/worker/testing.go index 71b54ea5d..a007ef6b6 100644 --- a/internal/servers/worker/testing.go +++ b/internal/servers/worker/testing.go @@ -24,7 +24,7 @@ import ( type TestWorker struct { b *base.Server w *Worker - t *testing.T + t testing.TB addrs []string // The address the worker proxies are listening on ctx context.Context cancel context.CancelFunc @@ -193,7 +193,7 @@ type TestWorkerOpts struct { NonceFn randFn } -func NewTestWorker(t *testing.T, opts *TestWorkerOpts) *TestWorker { +func NewTestWorker(t testing.TB, opts *TestWorkerOpts) *TestWorker { const op = "worker.NewTestWorker" ctx, cancel := context.WithCancel(context.Background()) @@ -302,7 +302,7 @@ func NewTestWorker(t *testing.T, opts *TestWorkerOpts) *TestWorker { return tw } -func (tw *TestWorker) AddClusterWorkerMember(t *testing.T, opts *TestWorkerOpts) *TestWorker { +func (tw *TestWorker) AddClusterWorkerMember(t testing.TB, opts *TestWorkerOpts) *TestWorker { const op = "worker.(TestWorker).AddClusterWorkerMember" if opts == nil { opts = new(TestWorkerOpts) diff --git a/internal/session/testing.go b/internal/session/testing.go index c306c1d81..fa501c3df 100644 --- a/internal/session/testing.go +++ b/internal/session/testing.go @@ -24,7 +24,7 @@ import ( ) // TestConnection creates a test connection for the sessionId in the repository. -func TestConnection(t *testing.T, conn *db.DB, sessionId, clientTcpAddr string, clientTcpPort uint32, endpointTcpAddr string, endpointTcpPort uint32, userClientIp string) *Connection { +func TestConnection(t testing.TB, conn *db.DB, sessionId, clientTcpAddr string, clientTcpPort uint32, endpointTcpAddr string, endpointTcpPort uint32, userClientIp string) *Connection { t.Helper() require := require.New(t) rw := db.New(conn) @@ -44,7 +44,7 @@ func TestConnection(t *testing.T, conn *db.DB, sessionId, clientTcpAddr string, } // TestConnectionState creates a test connection state for the connectionId in the repository. -func TestConnectionState(t *testing.T, conn *db.DB, connectionId string, state ConnectionStatus) *ConnectionState { +func TestConnectionState(t testing.TB, conn *db.DB, connectionId string, state ConnectionStatus) *ConnectionState { t.Helper() require := require.New(t) rw := db.New(conn) @@ -56,7 +56,7 @@ func TestConnectionState(t *testing.T, conn *db.DB, connectionId string, state C } // TestState creates a test state for the sessionId in the repository. -func TestState(t *testing.T, conn *db.DB, sessionId string, state Status) *State { +func TestState(t testing.TB, conn *db.DB, sessionId string, state Status) *State { t.Helper() require := require.New(t) rw := db.New(conn) @@ -69,7 +69,7 @@ func TestState(t *testing.T, conn *db.DB, sessionId string, state Status) *State // TestSession creates a test session composed of c in the repository. Options // are passed into New, and withServerId is handled locally. -func TestSession(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, c ComposedOf, opt ...Option) *Session { +func TestSession(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, c ComposedOf, opt ...Option) *Session { t.Helper() ctx := context.Background() opts := getOpts(opt...) @@ -110,7 +110,7 @@ func TestSession(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, c Composed } // TestDefaultSession creates a test session in the repository using defaults. -func TestDefaultSession(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, iamRepo *iam.Repository, opt ...Option) *Session { +func TestDefaultSession(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, iamRepo *iam.Repository, opt ...Option) *Session { t.Helper() composedOf := TestSessionParams(t, conn, wrapper, iamRepo) future := timestamppb.New(time.Now().Add(time.Hour)) @@ -120,7 +120,7 @@ func TestDefaultSession(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, iam // TestSessionParams returns an initialized ComposedOf which can be used to // create a session in the repository. -func TestSessionParams(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, iamRepo *iam.Repository) ComposedOf { +func TestSessionParams(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, iamRepo *iam.Repository) ComposedOf { t.Helper() ctx := context.Background() @@ -166,7 +166,7 @@ func TestSessionParams(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, iamR } // TestTofu will create a test "trust on first use" token -func TestTofu(t *testing.T) []byte { +func TestTofu(t testing.TB) []byte { t.Helper() require := require.New(t) tofu, err := base62.Random(20) @@ -176,7 +176,7 @@ func TestTofu(t *testing.T) []byte { // TestWorker inserts a worker into the db to satisfy foreign key constraints. // Supports the WithServerId option. -func TestWorker(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, opt ...Option) *servers.Server { +func TestWorker(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, opt ...Option) *servers.Server { t.Helper() rw := db.New(conn) kms := kms.TestKms(t, conn, wrapper) diff --git a/internal/target/tcp/testing.go b/internal/target/tcp/testing.go index 6a8e310db..0e73faa38 100644 --- a/internal/target/tcp/testing.go +++ b/internal/target/tcp/testing.go @@ -12,7 +12,7 @@ import ( ) // TestTarget is used to create a Target that can be used by tests in other packages. -func TestTarget(ctx context.Context, t *testing.T, conn *db.DB, scopeId, name string, opt ...target.Option) target.Target { +func TestTarget(ctx context.Context, t testing.TB, conn *db.DB, scopeId, name string, opt ...target.Option) target.Target { t.Helper() opt = append(opt, target.WithName(name)) opts := target.GetOpts(opt...) @@ -48,12 +48,12 @@ func TestTarget(ctx context.Context, t *testing.T, conn *db.DB, scopeId, name st return tar } -func testTargetName(t *testing.T, scopeId string) string { +func testTargetName(t testing.TB, scopeId string) string { t.Helper() return fmt.Sprintf("%s-%s", scopeId, testId(t)) } -func testId(t *testing.T) string { +func testId(t testing.TB) string { t.Helper() id, err := uuid.GenerateUUID() require.NoError(t, err) diff --git a/internal/target/testing.go b/internal/target/testing.go index 9460fd382..964a66dd1 100644 --- a/internal/target/testing.go +++ b/internal/target/testing.go @@ -25,7 +25,7 @@ func TestNewCredentialLibrary(targetId, credentialLibraryId string, purpose cred // TestCredentialLibrary creates a CredentialLibrary for targetId and // libraryId with the credential purpose of application. -func TestCredentialLibrary(t *testing.T, conn *db.DB, targetId, libraryId string) *CredentialLibrary { +func TestCredentialLibrary(t testing.TB, conn *db.DB, targetId, libraryId string) *CredentialLibrary { t.Helper() require := require.New(t) rw := db.New(conn) diff --git a/internal/tests/api/testing.go b/internal/tests/api/testing.go index 2f4640540..0552d10c7 100644 --- a/internal/tests/api/testing.go +++ b/internal/tests/api/testing.go @@ -14,7 +14,7 @@ import ( // CloudEventFromFile will marshal a single cloud event from the provided file // name -func CloudEventFromFile(t *testing.T, fileName string) *cloudevents.Event { +func CloudEventFromFile(t testing.TB, fileName string) *cloudevents.Event { t.Helper() b, err := ioutil.ReadFile(fileName) assert.NoError(t, err) @@ -26,7 +26,7 @@ func CloudEventFromFile(t *testing.T, fileName string) *cloudevents.Event { // GetEventDetails is a testing helper will return the details from the event // payload for a given messageType (request or response) -func GetEventDetails(t *testing.T, e *cloudevents.Event, messageType string) map[string]interface{} { +func GetEventDetails(t testing.TB, e *cloudevents.Event, messageType string) map[string]interface{} { t.Helper() require := require.New(t) require.NotNil(e) @@ -42,7 +42,7 @@ func GetEventDetails(t *testing.T, e *cloudevents.Event, messageType string) map // AssertRedactedValues will assert that the values for the given keys within // the data have been redacted -func AssertRedactedValues(t *testing.T, data interface{}, keys ...string) { +func AssertRedactedValues(t testing.TB, data interface{}, keys ...string) { t.Helper() assert, require := assert.New(t), require.New(t) require.NotNil(data) diff --git a/sdk/pbs/controller/api/testing.go b/sdk/pbs/controller/api/testing.go index 179723ee4..32a3da869 100644 --- a/sdk/pbs/controller/api/testing.go +++ b/sdk/pbs/controller/api/testing.go @@ -12,7 +12,7 @@ import ( // NewEncryptFilter is a copy of event.NewEncryptFilter since importing it would // case circular deps. The primary reason for this test func is to make sure // the proper IgnoreTypes are included for testing. -func NewEncryptFilter(t *testing.T, w wrapping.Wrapper) *encrypt.Filter { +func NewEncryptFilter(t testing.TB, w wrapping.Wrapper) *encrypt.Filter { t.Helper() return &encrypt.Filter{ Wrapper: w, diff --git a/sdk/testutil/free_port.go b/sdk/testutil/free_port.go index c4d999c7c..809d98fc1 100644 --- a/sdk/testutil/free_port.go +++ b/sdk/testutil/free_port.go @@ -8,7 +8,7 @@ import ( ) // TestFreePort just returns an available free localhost port -func TestFreePort(t *testing.T) int { +func TestFreePort(t testing.TB) int { t.Helper() require := require.New(t) addr, err := net.ResolveTCPAddr("tcp", "localhost:0") diff --git a/sdk/wrapper/testing.go b/sdk/wrapper/testing.go index 7154381ce..0bf4943f0 100644 --- a/sdk/wrapper/testing.go +++ b/sdk/wrapper/testing.go @@ -11,7 +11,7 @@ import ( ) // TestWrapper initializes an AEAD wrapping.Wrapper for testing -func TestWrapper(t *testing.T) wrapping.Wrapper { +func TestWrapper(t testing.TB) wrapping.Wrapper { rootKey := make([]byte, 32) n, err := rand.Read(rootKey) if err != nil { From 9e424db0a346b0ae0cb162c86e640b598555988a Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Thu, 28 Apr 2022 13:43:11 -0400 Subject: [PATCH 04/10] test(session): Ignore unique error on session create Sometimes when creating a lot of sessions concurrently we can run into a unique constraint error in the warehouse triggers in the database. Simply retrying the call will allow the session creation to succeed. --- internal/session/testing.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/session/testing.go b/internal/session/testing.go index fa501c3df..311a50b53 100644 --- a/internal/session/testing.go +++ b/internal/session/testing.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/boundary/internal/authtoken" "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/db/timestamp" + "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/host/static" "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/kms" @@ -94,6 +95,11 @@ func TestSession(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, c Composed } require.NoError(err) err = rw.Create(ctx, s, opts.withDbOpts...) + if e, ok := err.(*errors.Err); err != nil && ok && e.Code == errors.NotUnique { + // Sometimes we can get unique constraint errors when creating a session, + // if that happens, just retry. + err = rw.Create(ctx, s, opts.withDbOpts...) + } require.NoError(err) for _, cred := range s.DynamicCredentials { From 5d042c7ec7c19da0431bf5e1d4421376a741c2fa Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Mon, 25 Apr 2022 16:06:06 -0400 Subject: [PATCH 05/10] feat(dbtest): Mount DB dumps and create templates Mount the benchmark dump directory when running the test database and create a new script for creating templates from the dumps. This makes it easy to create pre-populated databases for benchmark tests. Fixes https://hashicorp.atlassian.net/browse/ICU-4040 --- testing/dbtest/docker/Dockerfile.11-alpine | 1 + testing/dbtest/docker/Dockerfile.12-alpine | 1 + testing/dbtest/docker/Dockerfile.13-alpine | 1 + testing/dbtest/docker/Dockerfile.alpine | 1 + testing/dbtest/docker/Makefile | 1 + testing/dbtest/docker/restore-benchmark-dumps.sh | 16 ++++++++++++++++ 6 files changed, 21 insertions(+) create mode 100644 testing/dbtest/docker/restore-benchmark-dumps.sh diff --git a/testing/dbtest/docker/Dockerfile.11-alpine b/testing/dbtest/docker/Dockerfile.11-alpine index f569cac04..bb1098f7a 100644 --- a/testing/dbtest/docker/Dockerfile.11-alpine +++ b/testing/dbtest/docker/Dockerfile.11-alpine @@ -1,6 +1,7 @@ FROM postgres:11-alpine ADD init-db.sh /docker-entrypoint-initdb.d/00-init-db.sh +ADD restore-benchmark-dumps.sh /docker-entrypoint-initdb.d/01-restore-benchmark-dumps.sh ADD postgresql.conf /etc/postgresql/postgresql.conf CMD ["postgres", "-c", "config_file=/etc/postgresql/postgresql.conf"] diff --git a/testing/dbtest/docker/Dockerfile.12-alpine b/testing/dbtest/docker/Dockerfile.12-alpine index 3ffdb3d35..37fc19200 100644 --- a/testing/dbtest/docker/Dockerfile.12-alpine +++ b/testing/dbtest/docker/Dockerfile.12-alpine @@ -1,6 +1,7 @@ FROM postgres:12-alpine ADD init-db.sh /docker-entrypoint-initdb.d/00-init-db.sh +ADD restore-benchmark-dumps.sh /docker-entrypoint-initdb.d/01-restore-benchmark-dumps.sh ADD postgresql.conf /etc/postgresql/postgresql.conf CMD ["postgres", "-c", "config_file=/etc/postgresql/postgresql.conf"] diff --git a/testing/dbtest/docker/Dockerfile.13-alpine b/testing/dbtest/docker/Dockerfile.13-alpine index ddab38a16..8279fe13e 100644 --- a/testing/dbtest/docker/Dockerfile.13-alpine +++ b/testing/dbtest/docker/Dockerfile.13-alpine @@ -1,6 +1,7 @@ FROM postgres:13-alpine ADD init-db.sh /docker-entrypoint-initdb.d/00-init-db.sh +ADD restore-benchmark-dumps.sh /docker-entrypoint-initdb.d/01-restore-benchmark-dumps.sh ADD postgresql.conf /etc/postgresql/postgresql.conf CMD ["postgres", "-c", "config_file=/etc/postgresql/postgresql.conf"] diff --git a/testing/dbtest/docker/Dockerfile.alpine b/testing/dbtest/docker/Dockerfile.alpine index 1655beab9..d8041ece0 100644 --- a/testing/dbtest/docker/Dockerfile.alpine +++ b/testing/dbtest/docker/Dockerfile.alpine @@ -1,6 +1,7 @@ FROM postgres:alpine ADD init-db.sh /docker-entrypoint-initdb.d/00-init-db.sh +ADD restore-benchmark-dumps.sh /docker-entrypoint-initdb.d/01-restore-benchmark-dumps.sh ADD postgresql.conf /etc/postgresql/postgresql.conf CMD ["postgres", "-c", "config_file=/etc/postgresql/postgresql.conf"] diff --git a/testing/dbtest/docker/Makefile b/testing/dbtest/docker/Makefile index 8818bbb5e..31f0ad10b 100644 --- a/testing/dbtest/docker/Makefile +++ b/testing/dbtest/docker/Makefile @@ -47,6 +47,7 @@ database-up: -e PGDATA=/pgdata \ --mount type=tmpfs,destination=/pgdata \ -v "$(CWD)/../../../internal/db/schema/migrations":/migrations \ + -v "$(CWD)/benchmark_dumps":/benchmark_dumps \ $(TEST_IMAGE_TAG) \ -c 'config_file=/etc/postgresql/postgresql.conf' \ $(PG_OPTS) 1> /dev/null diff --git a/testing/dbtest/docker/restore-benchmark-dumps.sh b/testing/dbtest/docker/restore-benchmark-dumps.sh new file mode 100644 index 000000000..324e077cd --- /dev/null +++ b/testing/dbtest/docker/restore-benchmark-dumps.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +set -e +shopt -s globstar + +for file in $(ls -v /benchmark_dumps/*.dump); do + echo "Creating template database based on: ${file}" + db_name="boundary_$(basename ${file} .dump)_template" + psql -v "ON_ERROR_STOP=1" --username "$POSTGRES_USER" --dbname "${POSTGRES_DB}" -q < Date: Mon, 25 Apr 2022 16:07:46 -0400 Subject: [PATCH 06/10] feat(sessions): Add list benchmarks The list benchmark tests uses pre-created database dumps to run its benchmarks. This benchmark will be used to guide optimizations around listing of sessions. Fixes https://hashicorp.atlassian.net/browse/ICU-4042 --- Makefile | 13 ++ go.mod | 6 +- .../sessions/session_list_benchmarks_test.go | 188 ++++++++++++++++++ testing/dbtest/template.go | 40 +++- 4 files changed, 243 insertions(+), 4 deletions(-) create mode 100644 internal/servers/controller/handlers/sessions/session_list_benchmarks_test.go diff --git a/Makefile b/Makefile index 0296e5f6f..23fcff1e4 100644 --- a/Makefile +++ b/Makefile @@ -239,6 +239,19 @@ test-api: .PHONY: test-all test-all: test-sdk test-api test +BENCH_TIME?=1s +BENCH_COUNT?=1s +.PHONY: benchmark +benchmark: + @go test \ + -timeout 60m \ + ./internal/servers/controller/handlers/sessions/ \ + -v \ + -bench '^BenchmarkSessionList$$' \ + -benchtime $(BENCH_TIME) \ + -count $(BENCH_COUNT) \ + -run '^$$' + .PHONY: install-go install-go: ./ci/goinstall.sh diff --git a/go.mod b/go.mod index f84c2a77a..498c9104a 100644 --- a/go.mod +++ b/go.mod @@ -85,7 +85,10 @@ require ( require github.com/hashicorp/go-dbw v0.0.0-20211215222256-2ff0d37184ff // this is a branch and should be updated before merging -require github.com/prometheus/client_golang v1.12.1 +require ( + github.com/prometheus/client_golang v1.12.1 + golang.org/x/sync v0.0.0-20210220032951-036812b2e83c +) require ( github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect @@ -177,7 +180,6 @@ require ( golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd // indirect golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a // indirect - golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect golang.org/x/text v0.3.7 // indirect golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e // indirect golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect diff --git a/internal/servers/controller/handlers/sessions/session_list_benchmarks_test.go b/internal/servers/controller/handlers/sessions/session_list_benchmarks_test.go new file mode 100644 index 000000000..f46c1b231 --- /dev/null +++ b/internal/servers/controller/handlers/sessions/session_list_benchmarks_test.go @@ -0,0 +1,188 @@ +package sessions_test + +import ( + "context" + "fmt" + "testing" + + "github.com/hashicorp/boundary/internal/auth/password" + "github.com/hashicorp/boundary/internal/authtoken" + "github.com/hashicorp/boundary/internal/db" + pbs "github.com/hashicorp/boundary/internal/gen/controller/api/services" + authpb "github.com/hashicorp/boundary/internal/gen/controller/auth" + "github.com/hashicorp/boundary/internal/iam" + "github.com/hashicorp/boundary/internal/kms" + "github.com/hashicorp/boundary/internal/servers" + "github.com/hashicorp/boundary/internal/servers/controller/auth" + "github.com/hashicorp/boundary/internal/servers/controller/handlers/sessions" + "github.com/hashicorp/boundary/internal/session" + "github.com/hashicorp/boundary/internal/types/scope" + "github.com/hashicorp/boundary/testing/dbtest" + "github.com/stretchr/testify/require" + "golang.org/x/sync/errgroup" +) + +type template struct { + name string + sessions int + connsPerSession int + users int +} + +func BenchmarkSessionList(b *testing.B) { + // See the explanation in testing/dbtest/session_list_benchmarks_dump_generation_test.go for + // an overview of the assumptions made when creating these template scenarios. + for _, template := range []template{ + { + name: dbtest.Boundary1000Sessions10ConnsPerSession10Template, + sessions: 1000, + connsPerSession: 10, + users: 10, + }, + { + name: dbtest.Boundary1000Sessions10ConnsPerSession25Template, + sessions: 1000, + connsPerSession: 10, + users: 25, + }, + { + name: dbtest.Boundary1000Sessions10ConnsPerSession50Template, + sessions: 1000, + connsPerSession: 10, + users: 50, + }, + { + name: dbtest.Boundary1000Sessions10ConnsPerSession75Template, + sessions: 1000, + connsPerSession: 10, + users: 75, + }, + { + name: dbtest.Boundary1000Sessions10ConnsPerSession100Template, + sessions: 1000, + connsPerSession: 10, + users: 100, + }, + { + name: dbtest.Boundary1000Sessions10ConnsPerSession500Template, + sessions: 1000, + connsPerSession: 10, + users: 500, + }, + } { + b.Run(fmt.Sprintf("%d_sessions_%d_conns_per_session_%d_users", template.sessions, template.connsPerSession, template.users), func(b *testing.B) { + ctx := context.Background() + conn, _ := db.TestSetup(b, "postgres", db.WithTemplate(template.name)) + rw := db.New(conn) + kmsRepo, err := kms.NewRepository(rw, rw) + require.NoError(b, err) + kmsThing, err := kms.NewKms(kmsRepo) + require.NoError(b, err) + wrap, err := dbtest.GetBoundaryBenchmarksRootKeyWrapper(ctx) + require.NoError(b, err) + err = kmsThing.AddExternalWrappers(ctx, kms.WithRootWrapper(wrap)) + require.NoError(b, err) + + iamRepo, err := iam.NewRepository(rw, rw, kmsThing) + require.NoError(b, err) + + sessRepo, err := session.NewRepository(rw, rw, kmsThing) + require.NoError(b, err) + + authTokenRepo, err := authtoken.NewRepository(rw, rw, kmsThing) + require.NoError(b, err) + + pwRepo, err := password.NewRepository(rw, rw, kmsThing) + require.NoError(b, err) + + serversRepo, err := servers.NewRepository(rw, rw, kmsThing) + require.NoError(b, err) + + iamRepoFn := func() (*iam.Repository, error) { + return iamRepo, nil + } + sessRepoFn := func() (*session.Repository, error) { + return sessRepo, nil + } + authTokenRepoFn := func() (*authtoken.Repository, error) { + return authTokenRepo, nil + } + serversRepoFn := func() (*servers.Repository, error) { + return serversRepo, nil + } + + s, err := sessions.NewService(sessRepoFn, iamRepoFn) + require.NoError(b, err) + + var users []*userWithToken + rows, err := rw.Query(ctx, "select public_id from iam_user where name like 'user%'", nil) + require.NoError(b, err) + var userId string + for rows.Next() { + err = rows.Scan(&userId) + require.NoError(b, err) + require.NotEmpty(b, userId) + u, accountIds, err := iamRepo.LookupUser(ctx, userId) + require.NoError(b, err) + require.NotEmpty(b, accountIds) + account, err := pwRepo.LookupAccount(ctx, accountIds[0]) + require.NoError(b, err) + require.NotNil(b, account) + acct, err := pwRepo.Authenticate(ctx, u.ScopeId, account.AuthMethodId, u.Name, dbtest.BoundaryBenchmarksUserPassword) + require.NoError(b, err) + require.NotNil(b, acct) + tok, err := authTokenRepo.CreateAuthToken(ctx, u, acct.GetPublicId()) + require.NoError(b, err) + require.NotEmpty(b, tok) + tokString, err := authtoken.EncryptToken(ctx, kmsThing, u.ScopeId, tok.PublicId, tok.Token) + require.NoError(b, err) + req := &pbs.ListSessionsRequest{ + ScopeId: scope.Global.String(), + Recursive: true, + Filter: fmt.Sprintf(`"/item/user_id"==%q`, u.PublicId), + } + ctx := auth.NewVerifierContext( + ctx, + iamRepoFn, + authTokenRepoFn, + serversRepoFn, + kmsThing, + &authpb.RequestInfo{ + PublicId: tok.PublicId, + EncryptedToken: tokString, + TokenFormat: uint32(auth.AuthTokenTypeBearer), + }) + users = append(users, &userWithToken{ + User: u, + req: req, + ctx: ctx, + }) + } + require.NoError(b, rows.Close()) + require.NoError(b, rows.Err()) + require.Len(b, users, template.users) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + eg := &errgroup.Group{} + for i := range users { + user := users[i] + eg.Go(func() error { + _, err := s.ListSessions(user.ctx, user.req) + if err != nil { + return fmt.Errorf("list failed: %s", err.Error()) + } + return nil + }) + } + require.NoError(b, eg.Wait()) + } + }) + } +} + +type userWithToken struct { + *iam.User + req *pbs.ListSessionsRequest + ctx context.Context +} diff --git a/testing/dbtest/template.go b/testing/dbtest/template.go index 5a3a9892e..7178b2d1d 100644 --- a/testing/dbtest/template.go +++ b/testing/dbtest/template.go @@ -1,6 +1,8 @@ package dbtest import ( + "context" + "encoding/base64" "fmt" "math/rand" "os" @@ -9,6 +11,8 @@ import ( // postgres dialect "github.com/hashicorp/boundary/internal/db/common" + wrapping "github.com/hashicorp/go-kms-wrapping/v2" + "github.com/hashicorp/go-kms-wrapping/v2/aead" _ "github.com/jackc/pgx/v4/stdlib" ) @@ -31,10 +35,20 @@ const ( Postgres = "postgres" ) +// BoundaryBenchmarksUserPassword is the password used for all users +// created by the boundary benchmarks dump generator. +const BoundaryBenchmarksUserPassword = "testpassword" + // Template Database Names const ( - BoundaryTemplate = "boundary_template" - Template1 = "template1" + BoundaryTemplate = "boundary_template" + Template1 = "template1" + Boundary1000Sessions10ConnsPerSession10Template = "boundary_session_1000_10_10_template" + Boundary1000Sessions10ConnsPerSession25Template = "boundary_session_1000_10_25_template" + Boundary1000Sessions10ConnsPerSession50Template = "boundary_session_1000_10_50_template" + Boundary1000Sessions10ConnsPerSession75Template = "boundary_session_1000_10_75_template" + Boundary1000Sessions10ConnsPerSession100Template = "boundary_session_1000_10_100_template" + Boundary1000Sessions10ConnsPerSession500Template = "boundary_session_1000_10_500_template" ) var supportedDialects = map[string]struct{}{ @@ -44,6 +58,12 @@ var supportedDialects = map[string]struct{}{ var supportedTemplates = map[string]struct{}{ BoundaryTemplate: {}, Template1: {}, + Boundary1000Sessions10ConnsPerSession10Template: {}, + Boundary1000Sessions10ConnsPerSession25Template: {}, + Boundary1000Sessions10ConnsPerSession50Template: {}, + Boundary1000Sessions10ConnsPerSession75Template: {}, + Boundary1000Sessions10ConnsPerSession100Template: {}, + Boundary1000Sessions10ConnsPerSession500Template: {}, } const letterBytes = "abcdefghijklmnopqrstuvwxyz" @@ -53,6 +73,22 @@ const ( letterIdxMax = 63 / letterIdxBits // # of letter indices fitting in 63 bits ) +const boundaryBenchmarksRootKey = "rootkey1rootkey1rootkey1rootkey1" // 32 bytes for AES-256 + +// GetBoundaryBenchmarksRootKeyWrapper returns the root key wrapper used for the benchmark dumps. +func GetBoundaryBenchmarksRootKeyWrapper(ctx context.Context) (wrapping.Wrapper, error) { + wrap := aead.NewWrapper() + _, err := wrap.SetConfig(ctx, wrapping.WithKeyId(base64.StdEncoding.EncodeToString([]byte(boundaryBenchmarksRootKey)))) + if err != nil { + return nil, err + } + err = wrap.SetAesGcmKeyBytes([]byte(boundaryBenchmarksRootKey)) + if err != nil { + return nil, err + } + return wrap, nil +} + func randStr(n int) string { b := make([]byte, n) // A rand.Int63() generates 63 random bits, enough for letterIdxMax letters! From 21b0d584adff3c490e9c8b89423210440cda6b7b Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Mon, 25 Apr 2022 16:11:27 -0400 Subject: [PATCH 07/10] feat(dbtest): Add benchmark dump generator Generate dumps ahead of time so that they can be used for benchmarks without prohibitive startup costs. Fixes https://hashicorp.atlassian.net/browse/ICU-4039 --- Makefile | 4 + testing/dbtest/docker/Makefile | 10 +- ...on_list_benchmarks_dump_generation_test.go | 283 ++++++++++++++++++ 3 files changed, 296 insertions(+), 1 deletion(-) create mode 100644 testing/dbtest/session_list_benchmarks_dump_generation_test.go diff --git a/Makefile b/Makefile index 23fcff1e4..ed2e377cc 100644 --- a/Makefile +++ b/Makefile @@ -214,6 +214,10 @@ test-database-up: test-database-down: make -C testing/dbtest/docker clean +.PHONY: generate-database-dumps +generate-database-dumps: + @$(MAKE) -C testing/dbtest/docker generate-database-dumps + .PHONY: test-ci test-ci: export CI_BUILD=1 test-ci: diff --git a/testing/dbtest/docker/Makefile b/testing/dbtest/docker/Makefile index 31f0ad10b..c93c2e7d0 100644 --- a/testing/dbtest/docker/Makefile +++ b/testing/dbtest/docker/Makefile @@ -59,4 +59,12 @@ clean: docker stop $(TEST_CONTAINER_NAME) || true docker rm -v $(TEST_CONTAINER_NAME) || true -.PHONY: all docker-build database-up ${docker-buildxs} ${docker-load-buildxs} clean +generate-database-dumps: + BOUNDARY_DB_TEST_GENERATE_SESSION_BENCHMARK_TEMPLATE_DUMPS=1 go test \ + -v \ + -run '^TestGenerateSessionBenchmarkTemplateDumps$$' \ + --timeout=1000h \ + -count=1 \ + ../ + +.PHONY: all docker-build database-up ${docker-buildxs} ${docker-load-buildxs} generate-database-dumps clean diff --git a/testing/dbtest/session_list_benchmarks_dump_generation_test.go b/testing/dbtest/session_list_benchmarks_dump_generation_test.go new file mode 100644 index 000000000..9178c559b --- /dev/null +++ b/testing/dbtest/session_list_benchmarks_dump_generation_test.go @@ -0,0 +1,283 @@ +package dbtest_test + +import ( + "context" + "fmt" + "math/rand" + "os" + "os/exec" + "path" + "runtime" + "strconv" + "testing" + "time" + + "github.com/hashicorp/boundary/internal/auth/password" + "github.com/hashicorp/boundary/internal/authtoken" + "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/host/static" + "github.com/hashicorp/boundary/internal/iam" + "github.com/hashicorp/boundary/internal/kms" + "github.com/hashicorp/boundary/internal/servers" + "github.com/hashicorp/boundary/internal/session" + "github.com/hashicorp/boundary/internal/target" + "github.com/hashicorp/boundary/internal/target/tcp" + "github.com/hashicorp/boundary/testing/dbtest" + "github.com/stretchr/testify/require" + "golang.org/x/sync/errgroup" +) + +// TestGenerateSessionBenchmarkTemplateDumps is not really a test, it uses +// the testing framework for easy access to a boundary server for +// populating a database with test data. This database is then dumped +// to predictable files for use in benchmarking. +// +// Each database dump contains: +// * N number of sessions, with each session having M connections. +// * P users, each owning an even amount of the sessions. All users +// use the password "testpassword" to login. +// It is safe for users of these dumps to assume all of this, for the variables +// N, M and P defined in the "scenarios" struct below. +func TestGenerateSessionBenchmarkTemplateDumps(t *testing.T) { + if os.Getenv("BOUNDARY_DB_TEST_GENERATE_SESSION_BENCHMARK_TEMPLATE_DUMPS") == "" { + t.Skip("BOUNDARY_DB_TEST_GENERATE_SESSION_BENCHMARK_TEMPLATE_DUMPS is not set") + return + } + + pgDumpBinary := "pg_dump" + if os.Getenv("PGDUMP_BINARY") != "" { + pgDumpBinary = os.Getenv("PGDUMP_BINARY") + } + _, err := exec.LookPath(pgDumpBinary) + require.NoError(t, err, "failed to find %s, please install it to generate benchmarking template dumps. Set PGDUMP_BINARY to configure a custom binary name", pgDumpBinary) + + // Verify that the version of pg_dump used to create the dumps is the + // same as the LOWEST version of Postgres that we support, since dumps are + // only forward compatible, not backwards compatible, and we want to be able + // to import these dumps into all the versions of Postgres that we support. + out, err := exec.Command(pgDumpBinary, "--version").Output() + require.NoError(t, err, "failed to check pg_dump version") + major, _, err := parsePgDumpVersion(string(out)) + require.NoError(t, err, "failed to parse pg_dump version") + if major != 11 { + t.Fatal("pg_dump version is not 11, please install pg_dump 11 to generate benchmarking template dumps") + } + + scenarios := []struct { + sessions int + connsPerSession int + users int + }{ + { + sessions: 1000, + connsPerSession: 10, + users: 10, + }, + { + sessions: 1000, + connsPerSession: 10, + users: 25, + }, + { + sessions: 1000, + connsPerSession: 10, + users: 50, + }, + { + sessions: 1000, + connsPerSession: 10, + users: 75, + }, + { + sessions: 1000, + connsPerSession: 10, + users: 100, + }, + { + sessions: 1000, + connsPerSession: 10, + users: 500, + }, + } + + // Create a global semaphore to limit the number of concurrent requests + // across all tests. + semaphore := make(chan struct{}, runtime.NumCPU()) + for _, scenario := range scenarios { + scenario := scenario // Parallel test closures act as goroutines, copy iteration variable + t.Run(fmt.Sprintf("Generate-%d-sessions-%d-conns-per-session-%d-users-dump", scenario.sessions, scenario.connsPerSession, scenario.users), func(t *testing.T) { + t.Parallel() // Lets speed things up a bit + ctx := context.Background() + require := require.New(t) + outputPath := fmt.Sprintf("./docker/benchmark_dumps/session_%d_%d_%d.dump", scenario.sessions, scenario.connsPerSession, scenario.users) + if _, err := os.Lstat(outputPath); err == nil { + t.Skipf("%q already exists, skipping", path.Base(outputPath)) + return + } + conn, dbURL := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrap, err := dbtest.GetBoundaryBenchmarksRootKeyWrapper(ctx) + require.NoError(err) + kms := kms.TestKms(t, conn, wrap) + + iamRepo := iam.TestRepo(t, conn, wrap) + authTokenRepo, err := authtoken.NewRepository(rw, rw, kms) + require.NoError(err) + pwRepo, err := password.NewRepository(rw, rw, kms) + require.NoError(err) + sessRepo, err := session.NewRepository(rw, rw, kms) + require.NoError(err) + connRepo, err := session.NewConnectionRepository(ctx, rw, rw, kms) + require.NoError(err) + serversRepo, err := servers.NewRepository(rw, rw, kms) + require.NoError(err) + worker := &servers.Server{ + PrivateId: "test1", + Type: "worker", + Address: "127.0.0.1", + } + _, _, err = serversRepo.UpsertServer(ctx, worker) + require.NoError(err) + + usersStart := time.Now() + t.Logf("Populating %d users", scenario.users) + users := make([]*user, scenario.users) + eg, gCtx := errgroup.WithContext(ctx) + for i := 0; i < scenario.users; i++ { + i := i + // Parallelize user creation + eg.Go(func() error { + select { + case semaphore <- struct{}{}: + case <-gCtx.Done(): + return gCtx.Err() + } + defer func() { + select { + case <-semaphore: + case <-gCtx.Done(): + } + }() + users[i] = newUser(t, gCtx, iamRepo, authTokenRepo, pwRepo, kms, conn, "user"+strconv.Itoa(i)) + return nil + }) + } + require.NoError(eg.Wait()) + t.Logf("Populated %d users in %s", scenario.users, time.Since(usersStart)) + + insertStart := time.Now() + t.Logf("Populating %d sessions", scenario.sessions) + eg, gCtx = errgroup.WithContext(ctx) + for i := 0; i < scenario.sessions; i++ { + i := i + userIndex := i % len(users) + // Parallelize session creation + eg.Go(func() error { + select { + case semaphore <- struct{}{}: + case <-gCtx.Done(): + return gCtx.Err() + } + defer func() { + select { + case <-semaphore: + if i > 0 && i%(scenario.sessions/4) == 0 { + t.Logf("%d%% done in %s", 100*i/scenario.sessions, time.Since(insertStart)) + } + case <-gCtx.Done(): + } + }() + sess := session.TestSession(t, conn, wrap, session.ComposedOf{ + UserId: users[userIndex].id, + HostId: users[userIndex].hostId, + TargetId: users[userIndex].targetId, + HostSetId: users[userIndex].hostSetId, + AuthTokenId: users[userIndex].authTokenId, + ScopeId: users[userIndex].scopeId, + Endpoint: "tcp://127.0.0.1:22", + }) + cycleSessionStates(t, ctx, sess, sessRepo, connRepo, conn, worker, scenario.connsPerSession) + return nil + }) + } + require.NoError(eg.Wait()) + _, err = sessRepo.TerminateCompletedSessions(ctx) + require.NoError(err) + t.Logf("Populated %d sessions in %s", scenario.sessions, time.Since(insertStart)) + + dumpStart := time.Now() + t.Logf("Dumping %d sessions to %s", scenario.sessions, outputPath) + cmd := exec.Command( + pgDumpBinary, + "--format=c", // Set custom postgres format for faster restore + "--file="+outputPath, + dbURL, + ) + out, err := cmd.CombinedOutput() + require.NoError(err, string(out)) + t.Logf("Dumped %d sessions to %s in %s", scenario.sessions, outputPath, time.Since(dumpStart)) + }) + } +} + +func cycleSessionStates(t testing.TB, ctx context.Context, sess *session.Session, sessRepo *session.Repository, connRepo *session.ConnectionRepository, conn *db.DB, worker *servers.Server, numConns int) { + sess, _, err := sessRepo.ActivateSession(ctx, sess.PublicId, sess.Version, worker.PrivateId, worker.Type, []byte(`tofu`)) + require.NoError(t, err) + var closeWiths []session.CloseWith + for i := 0; i < numConns; i++ { + connID := session.TestConnection(t, conn, sess.PublicId, "127.0.0.1", 22, "127.0.0.2", 23, "127.0.0.1").PublicId + closeWiths = append(closeWiths, session.CloseWith{ConnectionId: connID, ClosedReason: session.ConnectionCanceled}) + } + // Cancel 50% of the sessions + if rand.Intn(2) == 0 { + sess, err = sessRepo.CancelSession(ctx, sess.PublicId, sess.Version) + require.NoError(t, err) + _, err = session.CloseConnections(ctx, sessRepo, connRepo, closeWiths) + require.NoError(t, err) + } +} + +func parsePgDumpVersion(in string) (int, int, error) { + var major, minor int + _, err := fmt.Sscanf(in, "pg_dump (PostgreSQL) %d.%d", &major, &minor) + if err != nil { + return 0, 0, err + } + return major, minor, nil +} + +type user struct { + id string + hostId string + targetId string + hostSetId string + authTokenId string + scopeId string +} + +func newUser(t testing.TB, ctx context.Context, iamRepo *iam.Repository, authTokenRepo *authtoken.Repository, pwRepo *password.Repository, kms *kms.Kms, conn *db.DB, name string) *user { + require := require.New(t) + o, pWithSessions := iam.TestScopes(t, iamRepo) + am := password.TestAuthMethod(t, conn, o.GetPublicId()) + acct, err := password.NewAccount(am.GetPublicId(), password.WithLoginName(name)) + require.NoError(err) + acct, err = pwRepo.CreateAccount(ctx, o.PublicId, acct, password.WithPassword(dbtest.BoundaryBenchmarksUserPassword)) + require.NoError(err) + u := iam.TestUser(t, iamRepo, o.GetPublicId(), iam.WithAccountIds(acct.PublicId), iam.WithName(name)) + at, err := authTokenRepo.CreateAuthToken(ctx, u, acct.GetPublicId()) + require.NoError(err) + require.Equal(name, u.Name) + hc := static.TestCatalogs(t, conn, pWithSessions.GetPublicId(), 1)[0] + hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] + h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] + static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) + tar := tcp.TestTarget(ctx, t, conn, pWithSessions.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + return &user{ + id: u.GetPublicId(), + hostId: h.GetPublicId(), + targetId: tar.GetPublicId(), + hostSetId: hs.GetPublicId(), + authTokenId: at.GetPublicId(), + scopeId: pWithSessions.GetPublicId(), + } +} From 4cf2a87e8dba7fe9c1b4307c4bdb09406ee0e611 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 29 Apr 2022 15:55:27 -0400 Subject: [PATCH 08/10] Update targets to new listing method (#2045) This uses the prefetch method to fetch resource IDs and validate them for authz by the user before then fetching the full list of targets. --- ...tory_auth_method_operational_state_test.go | 2 +- .../controller/common/scopeids/scope_ids.go | 6 +- .../handlers/targets/target_service.go | 42 +++++---- internal/session/repository_session.go | 2 +- internal/target/options.go | 8 ++ internal/target/query.go | 6 ++ internal/target/repository.go | 88 +++++++++++++++++-- internal/target/target.go | 19 ++++ 8 files changed, 150 insertions(+), 23 deletions(-) diff --git a/internal/auth/oidc/repository_auth_method_operational_state_test.go b/internal/auth/oidc/repository_auth_method_operational_state_test.go index bb744571a..1ccd14440 100644 --- a/internal/auth/oidc/repository_auth_method_operational_state_test.go +++ b/internal/auth/oidc/repository_auth_method_operational_state_test.go @@ -366,7 +366,7 @@ func Test_MakeInactive_MakePrivate_MakePublic(t *testing.T) { }(), version: 1, wantErrMatch: errors.T(errors.InvalidParameter), - wantErrContains: "certificate signed by unknown authority", + wantErrContains: "certificate", }, } diff --git a/internal/servers/controller/common/scopeids/scope_ids.go b/internal/servers/controller/common/scopeids/scope_ids.go index fd6be78d8..7219d9851 100644 --- a/internal/servers/controller/common/scopeids/scope_ids.go +++ b/internal/servers/controller/common/scopeids/scope_ids.go @@ -2,6 +2,7 @@ package scopeids import ( "context" + "fmt" "github.com/hashicorp/boundary/internal/boundary" "github.com/hashicorp/boundary/internal/errors" @@ -265,7 +266,7 @@ func filterAuthorizedResourceIds( } res := perms.Resource{ - Type: resource.Session, + Type: input.Type, } // Now run authorization checks against each so we know if there is a point @@ -285,6 +286,9 @@ func filterAuthorizedResourceIds( } } + if output.ScopeResourceMap[scopeId] == nil { + return errors.New(ctx, errors.Internal, op, fmt.Sprintf("scope id %s returned from fetching authz protected entities not found in scope resource map", scopeId)) + } if output.ScopeResourceMap[scopeId].Resources == nil { output.ScopeResourceMap[scopeId].Resources = make(map[string]ResourceInfo) } diff --git a/internal/servers/controller/handlers/targets/target_service.go b/internal/servers/controller/handlers/targets/target_service.go index 02b0d956c..d90528da0 100644 --- a/internal/servers/controller/handlers/targets/target_service.go +++ b/internal/servers/controller/handlers/targets/target_service.go @@ -146,6 +146,8 @@ var _ pbs.TargetServiceServer = Service{} // ListTargets implements the interface pbs.TargetServiceServer. func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) (*pbs.ListTargetsResponse, error) { + const op = "targets.(Service).ListSessions" + if err := validateListRequest(req); err != nil { return nil, err } @@ -163,17 +165,34 @@ func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) ( } } - scopeIds, scopeInfoMap, err := scopeids.GetListingScopeIds( - ctx, s.iamRepoFn, authResults, req.GetScopeId(), resource.Target, req.GetRecursive()) + repo, err := s.repoFn() + if err != nil { + return nil, errors.Wrap(ctx, err, op) + } + + scopeResourceInfo, err := scopeids.GetListingResourceInformation( + ctx, + scopeids.GetListingResourceInformationInput{ + IamRepoFn: s.iamRepoFn, + AuthResults: authResults, + RootScopeId: req.GetScopeId(), + Type: resource.Target, + Recursive: req.GetRecursive(), + AuthzProtectedEntityProvider: repo, + ActionSet: IdActions, + }, + ) if err != nil { return nil, err } + // If no scopes match, return an empty response - if len(scopeIds) == 0 { + if len(scopeResourceInfo.ScopeIds) == 0 || + len(scopeResourceInfo.ResourceIds) == 0 { return &pbs.ListTargetsResponse{}, nil } - tl, err := s.listFromRepo(ctx, scopeIds) + tl, err := s.listFromRepo(ctx, scopeResourceInfo.ResourceIds) if err != nil { return nil, err } @@ -190,21 +209,14 @@ func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) ( Type: resource.Target, } for _, item := range tl { - res.Id = item.GetPublicId() - res.ScopeId = item.GetScopeId() - authorizedActions := authResults.FetchActionSetForId(ctx, item.GetPublicId(), IdActions, auth.WithResource(&res)).Strings() - if len(authorizedActions) == 0 { - continue - } - outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) if outputFields.Has(globals.ScopeField) { - outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[item.GetScopeId()])) + outputOpts = append(outputOpts, handlers.WithScope(scopeResourceInfo.ScopeResourceMap[item.GetScopeId()].ScopeInfo)) } if outputFields.Has(globals.AuthorizedActionsField) { - outputOpts = append(outputOpts, handlers.WithAuthorizedActions(authorizedActions)) + outputOpts = append(outputOpts, handlers.WithAuthorizedActions(scopeResourceInfo.ScopeResourceMap[item.GetScopeId()].Resources[item.GetPublicId()].AuthorizedActions.Strings())) } item, err := toProto(ctx, item, nil, nil, outputOpts...) @@ -1346,12 +1358,12 @@ func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { return rows > 0, nil } -func (s Service) listFromRepo(ctx context.Context, scopeIds []string) ([]target.Target, error) { +func (s Service) listFromRepo(ctx context.Context, targetIds []string) ([]target.Target, error) { repo, err := s.repoFn() if err != nil { return nil, err } - ul, err := repo.ListTargets(ctx, target.WithScopeIds(scopeIds)) + ul, err := repo.ListTargets(ctx, target.WithTargetIds(targetIds)) if err != nil { return nil, err } diff --git a/internal/session/repository_session.go b/internal/session/repository_session.go index 78fee5005..f1d5dbcd5 100644 --- a/internal/session/repository_session.go +++ b/internal/session/repository_session.go @@ -220,7 +220,7 @@ func (r *Repository) FetchAuthzProtectedEntitiesByScope(ctx context.Context, sco return nil, errors.New(ctx, errors.InvalidParameter, op, "no scopes given") case 1: inClauseCnt += 1 - where, args = fmt.Sprintf("where scope_id = @%d", inClauseCnt), append(args, sql.Named("1", scopeIds[0])) + where, args = fmt.Sprintf("where scope_id = @%d", inClauseCnt), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), scopeIds[0])) default: idsInClause := make([]string, 0, len(scopeIds)) for _, id := range scopeIds { diff --git a/internal/target/options.go b/internal/target/options.go index 69565cd3c..e3d0421ac 100644 --- a/internal/target/options.go +++ b/internal/target/options.go @@ -35,6 +35,7 @@ type options struct { WithSessionConnectionLimit int32 WithPublicId string WithWorkerFilter string + WithTargetIds []string } func getDefaultOptions() options { @@ -161,3 +162,10 @@ func WithWorkerFilter(filter string) Option { o.WithWorkerFilter = filter } } + +// WithTargetIds provides an option to search by specific target IDs +func WithTargetIds(with []string) Option { + return func(o *options) { + o.WithTargetIds = with + } +} diff --git a/internal/target/query.go b/internal/target/query.go index 054861eee..4fd31c14c 100644 --- a/internal/target/query.go +++ b/internal/target/query.go @@ -43,5 +43,11 @@ final (action, library_id) as ( ) select * from final order by action, library_id; +` + + targetPublicIdList = ` +select public_id, scope_id from target +%s +; ` ) diff --git a/internal/target/repository.go b/internal/target/repository.go index f15279d89..9b3163ec4 100644 --- a/internal/target/repository.go +++ b/internal/target/repository.go @@ -2,14 +2,17 @@ package target import ( "context" + "database/sql" "fmt" "strings" + "github.com/hashicorp/boundary/internal/boundary" "github.com/hashicorp/boundary/internal/db" dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/boundary/internal/types/scope" ) // Cloneable provides a cloning interface @@ -138,21 +141,96 @@ func (r *Repository) LookupTarget(ctx context.Context, publicIdOrName string, op return subtype, hostSources, credSources, nil } +// FetchAuthzProtectedEntitiesByScope implements boundary.AuthzProtectedEntityProvider +func (r *Repository) FetchAuthzProtectedEntitiesByScope(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error) { + const op = "target.(Repository).FetchAuthzProtectedEntitiesByScope" + + var where string + var args []interface{} + + inClauseCnt := 0 + + switch len(scopeIds) { + case 0: + return nil, errors.New(ctx, errors.InvalidParameter, op, "no scopes given") + case 1: + if scopeIds[0] != scope.Global.String() { + inClauseCnt += 1 + where, args = fmt.Sprintf("where scope_id = @%d", inClauseCnt), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), scopeIds[0])) + } + default: + idsInClause := make([]string, 0, len(scopeIds)) + for _, id := range scopeIds { + inClauseCnt += 1 + idsInClause, args = append(idsInClause, fmt.Sprintf("@%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), id)) + } + where = fmt.Sprintf("where scope_id in (%s)", strings.Join(idsInClause, ",")) + } + + q := targetPublicIdList + query := fmt.Sprintf(q, where) + + rows, err := r.reader.Query(ctx, query, args) + if err != nil { + return nil, errors.Wrap(ctx, err, op) + } + defer rows.Close() + + targetsMap := map[string][]boundary.AuthzProtectedEntity{} + for rows.Next() { + var tv targetView + if err := r.reader.ScanRows(ctx, rows, &tv); err != nil { + return nil, errors.Wrap(ctx, err, op, errors.WithMsg("scan row failed")) + } + targetsMap[tv.GetScopeId()] = append(targetsMap[tv.GetScopeId()], tv) + } + + return targetsMap, nil +} + // ListTargets in targets in a scope. Supports the WithScopeId, WithLimit, WithType options. func (r *Repository) ListTargets(ctx context.Context, opt ...Option) ([]Target, error) { const op = "target.(Repository).ListTargets" opts := GetOpts(opt...) - if len(opts.WithScopeIds) == 0 && opts.WithUserId == "" { - return nil, errors.New(ctx, errors.InvalidParameter, op, "must specify either scope id or user id") + if len(opts.WithScopeIds) == 0 && opts.WithUserId == "" && len(opts.WithTargetIds) == 0 { + return nil, errors.New(ctx, errors.InvalidParameter, op, "must specify either scope ids, target ids, or user id") } // TODO (jimlambrt 8/2020) - implement WithUserId() optional filtering. var where []string var args []interface{} - if len(opts.WithScopeIds) != 0 { - where, args = append(where, "scope_id in (?)"), append(args, opts.WithScopeIds) + inClauseCnt := 0 + + switch len(opts.WithScopeIds) { + case 0: + case 1: + inClauseCnt += 1 + where, args = append(where, fmt.Sprintf("scope_id = @%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), opts.WithScopeIds[0])) + default: + idsInClause := make([]string, 0, len(opts.WithScopeIds)) + for _, id := range opts.WithScopeIds { + inClauseCnt += 1 + idsInClause, args = append(idsInClause, fmt.Sprintf("@%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), id)) + } + where = append(where, fmt.Sprintf("scope_id in (%s)", strings.Join(idsInClause, ","))) } + + switch len(opts.WithTargetIds) { + case 0: + case 1: + inClauseCnt += 1 + where, args = append(where, fmt.Sprintf("public_id = @%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), opts.WithTargetIds[0])) + default: + idsInClause := make([]string, 0, len(opts.WithTargetIds)) + for _, id := range opts.WithTargetIds { + inClauseCnt += 1 + idsInClause, args = append(idsInClause, fmt.Sprintf("@%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), id)) + } + where = append(where, fmt.Sprintf("public_id in (%s)", strings.Join(idsInClause, ","))) + } + if opts.WithType != "" { - where, args = append(where, "type = ?"), append(args, opts.WithType.String()) + inClauseCnt += 1 + where, args = append(where, fmt.Sprintf("type = @%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), opts.WithType.String())) } var foundTargets []*targetView diff --git a/internal/target/target.go b/internal/target/target.go index 6916c43e0..0ac89ffd8 100644 --- a/internal/target/target.go +++ b/internal/target/target.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/hashicorp/boundary/internal/boundary" "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/oplog" @@ -44,6 +45,8 @@ const ( targetsViewDefaultTable = "target_all_subtypes" ) +var _ boundary.AuthzProtectedEntity = (*targetView)(nil) + // targetView provides a common way to return targets regardless of their // underlying type. type targetView struct { @@ -77,6 +80,22 @@ func (t *targetView) SetTableName(n string) { } } +// GetPublicId satisfies boundary.AuthzProtectedEntity +func (t targetView) GetPublicId() string { + return t.PublicId +} + +// GetScopeId satisfies boundary.AuthzProtectedEntity +func (t targetView) GetScopeId() string { + return t.ScopeId +} + +// GetUserId satisfies boundary.AuthzProtectedEntity; targets are not associated +// with a user ID so this always returns an empty string +func (t targetView) GetUserId() string { + return "" +} + func (t *targetView) Subtype() subtypes.Subtype { return subtypes.Subtype(t.Type) } From f00af362dc83a9b968f80491ae914019ec806151 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Fri, 29 Apr 2022 15:59:39 -0400 Subject: [PATCH 09/10] fix(session): Include where clause with order and limit When applying the limit and order by, the where clause also needs to be applied to yield the correct results. This can be seen with an example. Assuming a data set where there are 100 sessions, with 10 sessions belonging to a user with id `u_user1` and 90 sessions belonging to `u_user2`. If the constructed query is like: select * from (select public_id from session limit 10) s, session_list ss where s.public_id = ss.public_id and s.user_id = 'u_user1' order by create_time asc; The initial sub-query with the limit could return sessions only belonging to `u_user2`, then it would apply the where clause and exclude all of the rows. Instead if the query is: with session_ids as ( select public_id from sessions as s where s.user_id = 'u_user1' order by create_time asc limit 10 ) select * from session_list where session_list.public_id in (select * from session_ids) order by create_time asc; The CTE applies the where clause ensuring that only sessions for `u_user1` are used for the final select from the view. Note that the CTE is use primarily for readability. This also adds an index on `session (create_time)` to aide the query planner in performing the order by. --- .../28/01_session_create_time_idx.up.sql | 11 ++++++++++ internal/session/query.go | 21 +++++++++++++------ internal/session/repository_session.go | 4 ++-- 3 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 internal/db/schema/migrations/oss/postgres/28/01_session_create_time_idx.up.sql diff --git a/internal/db/schema/migrations/oss/postgres/28/01_session_create_time_idx.up.sql b/internal/db/schema/migrations/oss/postgres/28/01_session_create_time_idx.up.sql new file mode 100644 index 000000000..e60b8ffdb --- /dev/null +++ b/internal/db/schema/migrations/oss/postgres/28/01_session_create_time_idx.up.sql @@ -0,0 +1,11 @@ +begin; +-- Session list queries always use an order by on create_time. +-- This index can aide in performing this order by. +create index + session_create_time +on + session (create_time); + +analyze session; + +commit; diff --git a/internal/session/query.go b/internal/session/query.go index 541c4b475..7a10af70f 100644 --- a/internal/session/query.go +++ b/internal/session/query.go @@ -126,13 +126,22 @@ select public_id, scope_id, user_id from session ` sessionList = ` -select * -from - (select public_id from session %s) s, - session_list ss -where - s.public_id = ss.public_id +with +session_ids as ( + select public_id + from session as s + -- where clause is constructed %s + -- order by clause is constructed + %s + -- limit is constructed + %s +) +select * +from session_list +where + session_list.public_id in (select * from session_ids) +-- order by clause again since order from cte is not guaranteed to be preserved %s ; ` diff --git a/internal/session/repository_session.go b/internal/session/repository_session.go index f1d5dbcd5..d16edfa21 100644 --- a/internal/session/repository_session.go +++ b/internal/session/repository_session.go @@ -319,10 +319,10 @@ func (r *Repository) ListSessions(ctx context.Context, opt ...Option) ([]*Sessio var whereClause string if len(where) > 0 { - whereClause = " and " + strings.Join(where, " and ") + whereClause = " where " + strings.Join(where, " and ") } q := sessionList - query := fmt.Sprintf(q, limit, whereClause, withOrder) + query := fmt.Sprintf(q, whereClause, withOrder, limit, withOrder) rows, err := r.reader.Query(ctx, query, args) if err != nil { From 075ac87a5871de100c44be029c79f1a0924a9e5a Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Fri, 29 Apr 2022 16:50:21 -0400 Subject: [PATCH 10/10] chore(changelog): Add entries for session and target list improvements --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 084320558..846e7f187 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. ### New and Improved * controller: new health endpoint ([PR](https://github.com/hashicorp/boundary/pull/1882)). +* Improve response time for listing sessions and targets. + [PR](https://github.com/hashicorp/boundary/pull/2049) ### Bug Fixes * worker: create new error to prevent `event.newError: missing error: invalid parameter` and handle session cancel @@ -13,6 +15,9 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. [PR](https://github.com/hashicorp/boundary/pull/1929)) * controller: Reconcile DEKs with existing scopes ([Issue](https://github.com/hashicorp/boundary/issues/1856), [PR](https://github.com/hashicorp/boundary/pull/1976)) +* Fix for retrieving sessions that could result in incomplete results when + there is a large number (10k+) of sessions. + [PR](https://github.com/hashicorp/boundary/pull/2049) ## 0.7.6 (2022/03/15)