diff --git a/app/api/controller/pullreq/codeowner.go b/app/api/controller/pullreq/codeowner.go index c1bf433dc..0cdbd0044 100644 --- a/app/api/controller/pullreq/codeowner.go +++ b/app/api/controller/pullreq/codeowner.go @@ -67,17 +67,34 @@ func mapCodeOwnerEvaluation(ownerEvaluation *codeowners.Evaluation) []types.Code codeOwnerEvaluationEntries := make([]types.CodeOwnerEvaluationEntry, len(ownerEvaluation.EvaluationEntries)) for i, entry := range ownerEvaluation.EvaluationEntries { ownerEvaluations := make([]types.OwnerEvaluation, len(entry.OwnerEvaluations)) + userGroupOwnerEvaluations := make([]types.UserGroupOwnerEvaluation, len(entry.UserGroupOwnerEvaluations)) for j, owner := range entry.OwnerEvaluations { - ownerEvaluations[j] = types.OwnerEvaluation{ - Owner: owner.Owner, - ReviewDecision: owner.ReviewDecision, - ReviewSHA: owner.ReviewSHA, + ownerEvaluations[j] = mapOwner(owner) + } + for j, userGroupOwnerEvaluation := range entry.UserGroupOwnerEvaluations { + userGroupEvaluations := make([]types.OwnerEvaluation, len(userGroupOwnerEvaluation.Evaluations)) + for k, userGroupOwner := range userGroupOwnerEvaluation.Evaluations { + userGroupEvaluations[k] = mapOwner(userGroupOwner) + } + userGroupOwnerEvaluations[j] = types.UserGroupOwnerEvaluation{ + ID: userGroupOwnerEvaluation.ID, + Name: userGroupOwnerEvaluation.Name, + Evaluations: userGroupEvaluations, } } codeOwnerEvaluationEntries[i] = types.CodeOwnerEvaluationEntry{ - Pattern: entry.Pattern, - OwnerEvaluations: ownerEvaluations, + Pattern: entry.Pattern, + OwnerEvaluations: ownerEvaluations, + UserGroupOwnerEvaluations: userGroupOwnerEvaluations, } } return codeOwnerEvaluationEntries } + +func mapOwner(owner codeowners.OwnerEvaluation) types.OwnerEvaluation { + return types.OwnerEvaluation{ + Owner: owner.Owner, + ReviewDecision: owner.ReviewDecision, + ReviewSHA: owner.ReviewSHA, + } +} diff --git a/app/services/codeowners/service.go b/app/services/codeowners/service.go index be0128b40..e18685486 100644 --- a/app/services/codeowners/service.go +++ b/app/services/codeowners/service.go @@ -21,6 +21,7 @@ import ( "io" "strings" + "github.com/harness/gitness/app/services/usergroup" "github.com/harness/gitness/app/store" "github.com/harness/gitness/errors" "github.com/harness/gitness/git" @@ -37,6 +38,8 @@ const ( // maxGetContentFileSize specifies the maximum number of bytes a file content response contains. // If a file is any larger, the content is truncated. maxGetContentFileSize = oneMegabyte * 4 // 4 MB + // userGroupPrefixMarker is a prefix which will be used to identify if a given codeowner is usergroup. + userGroupPrefixMarker = "@" ) var ( @@ -71,10 +74,11 @@ type Config struct { } type Service struct { - repoStore store.RepoStore - git git.Interface - principalStore store.PrincipalStore - config Config + repoStore store.RepoStore + git git.Interface + principalStore store.PrincipalStore + config Config + userGroupResolver usergroup.Resolver } type File struct { @@ -99,8 +103,15 @@ type Evaluation struct { } type EvaluationEntry struct { - Pattern string - OwnerEvaluations []OwnerEvaluation + Pattern string + OwnerEvaluations []OwnerEvaluation + UserGroupOwnerEvaluations []UserGroupOwnerEvaluation +} + +type UserGroupOwnerEvaluation struct { + ID string + Name string + Evaluations []OwnerEvaluation } type OwnerEvaluation struct { @@ -114,12 +125,14 @@ func New( git git.Interface, config Config, principalStore store.PrincipalStore, + userGroupResolver usergroup.Resolver, ) *Service { service := &Service{ - repoStore: repoStore, - git: git, - config: config, - principalStore: principalStore, + repoStore: repoStore, + git: git, + config: config, + principalStore: principalStore, + userGroupResolver: userGroupResolver, } return service } @@ -284,11 +297,12 @@ func (s *Service) getApplicableCodeOwnersForPR( }, err } +//nolint:gocognit func (s *Service) Evaluate( ctx context.Context, repo *types.Repository, pr *types.PullReq, - reviewer []*types.PullReqReviewer, + reviewers []*types.PullReqReviewer, ) (*Evaluation, error) { owners, err := s.getApplicableCodeOwnersForPR(ctx, repo, pr) if err != nil { @@ -299,36 +313,41 @@ func (s *Service) Evaluate( return &Evaluation{}, nil } - flattenedReviewers := flattenReviewers(reviewer) evaluationEntries := make([]EvaluationEntry, len(owners.Entries)) for i, entry := range owners.Entries { ownerEvaluations := make([]OwnerEvaluation, 0, len(owners.Entries)) + userGroupOwnerEvaluations := make([]UserGroupOwnerEvaluation, 0, len(owners.Entries)) for _, owner := range entry.Owners { - if pullreqReviewer, ok := flattenedReviewers[owner]; ok { - ownerEvaluations = append(ownerEvaluations, OwnerEvaluation{ - Owner: pullreqReviewer.Reviewer, - ReviewDecision: pullreqReviewer.ReviewDecision, - ReviewSHA: pullreqReviewer.SHA, - }) + // check for usrgrp + if strings.HasPrefix(owner, userGroupPrefixMarker) { + userGroupCodeOwner, err := s.resolveUserGroupCodeOwner(ctx, owner[1:], reviewers) + if errors.Is(err, usergroup.ErrNotFound) { + log.Ctx(ctx).Debug().Msgf("usergroup %q not found hence skipping for code owner", owner) + continue + } + if err != nil { + return nil, fmt.Errorf("error resolving usergroup :%w", err) + } + userGroupOwnerEvaluations = append(userGroupOwnerEvaluations, *userGroupCodeOwner) continue } - principal, err := s.principalStore.FindByEmail(ctx, owner) + // user email based codeowner + userCodeOwner, err := s.resolveUserCodeOwnerByEmail(ctx, owner, reviewers) if errors.Is(err, gitness_store.ErrResourceNotFound) { - log.Ctx(ctx).Info().Msgf("user %s not found in database hence skipping for code owner: %v", - owner, err) + log.Ctx(ctx).Debug().Msgf("user %q not found in database hence skipping for code owner", owner) continue } if err != nil { - return &Evaluation{}, fmt.Errorf("error finding user by email: %w", err) + return nil, fmt.Errorf("error resolving user by email : %w", err) } - ownerEvaluations = append(ownerEvaluations, OwnerEvaluation{ - Owner: *principal.ToPrincipalInfo(), - }) + ownerEvaluations = append(ownerEvaluations, *userCodeOwner) } + evaluationEntries[i] = EvaluationEntry{ - Pattern: entry.Pattern, - OwnerEvaluations: ownerEvaluations, + Pattern: entry.Pattern, + OwnerEvaluations: ownerEvaluations, + UserGroupOwnerEvaluations: userGroupOwnerEvaluations, } } @@ -338,6 +357,62 @@ func (s *Service) Evaluate( }, nil } +func (s *Service) resolveUserGroupCodeOwner( + ctx context.Context, + owner string, + reviewers []*types.PullReqReviewer, +) (*UserGroupOwnerEvaluation, error) { + usrgrp, err := s.userGroupResolver.Resolve(ctx, owner) + if err != nil { + return nil, fmt.Errorf("not able to resolve usergroup : %w", err) + } + userGroupEvaluation := &UserGroupOwnerEvaluation{ + ID: usrgrp.ID, + Name: usrgrp.Name, + } + ownersEvaluations := make([]OwnerEvaluation, 0, len(usrgrp.Users)) + for _, uid := range usrgrp.Users { + pullreqReviewer := findReviewerInList("", uid, reviewers) + // we don't append all the user of the user group in the owner evaluations and + // append it only if it is reviewed by a user. + if pullreqReviewer != nil { + ownersEvaluations = append(ownersEvaluations, + OwnerEvaluation{ + Owner: pullreqReviewer.Reviewer, + ReviewDecision: pullreqReviewer.ReviewDecision, + ReviewSHA: pullreqReviewer.SHA, + }, + ) + continue + } + } + userGroupEvaluation.Evaluations = ownersEvaluations + + return userGroupEvaluation, nil +} + +func (s *Service) resolveUserCodeOwnerByEmail( + ctx context.Context, + owner string, + reviewers []*types.PullReqReviewer, +) (*OwnerEvaluation, error) { + pullreqReviewer := findReviewerInList(owner, "", reviewers) + if pullreqReviewer != nil { + return &OwnerEvaluation{ + Owner: pullreqReviewer.Reviewer, + ReviewDecision: pullreqReviewer.ReviewDecision, + ReviewSHA: pullreqReviewer.SHA, + }, nil + } + principal, err := s.principalStore.FindByEmail(ctx, owner) + if err != nil { + return nil, fmt.Errorf("error finding user by email: %w", err) + } + return &OwnerEvaluation{ + Owner: *principal.ToPrincipalInfo(), + }, nil +} + func (s *Service) Validate( ctx context.Context, repo *types.Repository, @@ -353,6 +428,10 @@ func (s *Service) Validate( for _, entry := range codeowners.Entries { // check for users in file for _, owner := range entry.Owners { + // todo: handle usergroup better + if strings.HasPrefix(owner, userGroupPrefixMarker) { + continue + } _, err := s.principalStore.FindByEmail(ctx, owner) if errors.Is(err, gitness_store.ErrResourceNotFound) { codeOwnerValidation.Addf(enum.CodeOwnerViolationCodeUserNotFound, @@ -381,12 +460,14 @@ func (s *Service) Validate( return &codeOwnerValidation, nil } -func flattenReviewers(reviewers []*types.PullReqReviewer) map[string]*types.PullReqReviewer { - r := make(map[string]*types.PullReqReviewer) +func findReviewerInList(email string, uid string, reviewers []*types.PullReqReviewer) *types.PullReqReviewer { for _, reviewer := range reviewers { - r[reviewer.Reviewer.Email] = reviewer + if uid == reviewer.Reviewer.UID || email == reviewer.Reviewer.Email { + return reviewer + } } - return r + + return nil } // We match a pattern list against a target diff --git a/app/services/codeowners/wire.go b/app/services/codeowners/wire.go index ebac1cf03..d3b361ebd 100644 --- a/app/services/codeowners/wire.go +++ b/app/services/codeowners/wire.go @@ -15,6 +15,7 @@ package codeowners import ( + "github.com/harness/gitness/app/services/usergroup" "github.com/harness/gitness/app/store" "github.com/harness/gitness/git" @@ -30,6 +31,7 @@ func ProvideCodeOwners( repoStore store.RepoStore, config Config, principalStore store.PrincipalStore, + userGroupResolver usergroup.Resolver, ) *Service { - return New(repoStore, git, config, principalStore) + return New(repoStore, git, config, principalStore, userGroupResolver) } diff --git a/app/services/protection/verify_pullreq.go b/app/services/protection/verify_pullreq.go index ef1d10f1c..d75d4072d 100644 --- a/app/services/protection/verify_pullreq.go +++ b/app/services/protection/verify_pullreq.go @@ -108,7 +108,7 @@ func (v *DefPullReq) MergeVerify( if v.Approvals.RequireCodeOwners { for _, entry := range in.CodeOwners.EvaluationEntries { - reviewDecision, approvers := getCodeOwnerApprovalStatus(entry.OwnerEvaluations) + reviewDecision, approvers := getCodeOwnerApprovalStatus(entry) if reviewDecision == enum.PullReqReviewDecisionPending { violations.Addf(codePullReqApprovalReqCodeOwnersNoApproval, @@ -295,10 +295,12 @@ func (v *DefPullReq) Sanitize() error { } func getCodeOwnerApprovalStatus( - ownerStatus []codeowners.OwnerEvaluation, + entry codeowners.EvaluationEntry, ) (enum.PullReqReviewDecision, []codeowners.OwnerEvaluation) { approvers := make([]codeowners.OwnerEvaluation, 0) - for _, o := range ownerStatus { + + // users + for _, o := range entry.OwnerEvaluations { if o.ReviewDecision == enum.PullReqReviewDecisionChangeReq { return enum.PullReqReviewDecisionChangeReq, nil } @@ -306,6 +308,19 @@ func getCodeOwnerApprovalStatus( approvers = append(approvers, o) } } + + // usergroups + for _, u := range entry.UserGroupOwnerEvaluations { + for _, o := range u.Evaluations { + if o.ReviewDecision == enum.PullReqReviewDecisionChangeReq { + return enum.PullReqReviewDecisionChangeReq, nil + } + if o.ReviewDecision == enum.PullReqReviewDecisionApproved { + approvers = append(approvers, o) + } + } + } + if len(approvers) > 0 { return enum.PullReqReviewDecisionApproved, approvers } diff --git a/app/services/usergroup/service.go b/app/services/usergroup/resolver.go similarity index 73% rename from app/services/usergroup/service.go rename to app/services/usergroup/resolver.go index 984a7a26b..078188dcd 100644 --- a/app/services/usergroup/service.go +++ b/app/services/usergroup/resolver.go @@ -17,14 +17,19 @@ package usergroup import ( "context" - "github.com/harness/gitness/store" "github.com/harness/gitness/types" ) -type Service struct { +var _ Resolver = (*GitnessResolver)(nil) + +type GitnessResolver struct { } -func (s *Service) Resolve(context.Context, string) (*types.UserGroup, error) { - // todo: implement once usergroup is supported - return nil, store.ErrResourceNotFound +func NewGitnessResolver() *GitnessResolver { + return &GitnessResolver{} +} + +func (s *GitnessResolver) Resolve(context.Context, string) (*types.UserGroup, error) { + // todo: implement once usergroup is supported + return nil, ErrNotFound } diff --git a/app/services/usergroup/user_group_resolver.go b/app/services/usergroup/user_group_resolver.go index dcd3c61da..d63851a1f 100644 --- a/app/services/usergroup/user_group_resolver.go +++ b/app/services/usergroup/user_group_resolver.go @@ -16,10 +16,13 @@ package usergroup import ( "context" + "errors" "github.com/harness/gitness/types" ) +var ErrNotFound = errors.New("usergroup not found") + type Resolver interface { Resolve(ctx context.Context, scopedID string) (*types.UserGroup, error) } diff --git a/app/services/usergroup/wire.go b/app/services/usergroup/wire.go new file mode 100644 index 000000000..fc937c612 --- /dev/null +++ b/app/services/usergroup/wire.go @@ -0,0 +1,28 @@ +// Copyright 2023 Harness, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package usergroup + +import ( + "github.com/google/wire" +) + +// WireSet provides a wire set for this package. +var WireSet = wire.NewSet( + ProvideUserGroupResolver, +) + +func ProvideUserGroupResolver() Resolver { + return NewGitnessResolver() +} diff --git a/cmd/gitness/wire.go b/cmd/gitness/wire.go index 75bad0f59..7c97bbe07 100644 --- a/cmd/gitness/wire.go +++ b/cmd/gitness/wire.go @@ -59,6 +59,7 @@ import ( "github.com/harness/gitness/app/services/protection" pullreqservice "github.com/harness/gitness/app/services/pullreq" "github.com/harness/gitness/app/services/trigger" + "github.com/harness/gitness/app/services/usergroup" "github.com/harness/gitness/app/services/webhook" "github.com/harness/gitness/app/sse" "github.com/harness/gitness/app/store" @@ -164,6 +165,7 @@ func initSystem(ctx context.Context, config *types.Config) (*cliserver.System, e cliserver.ProvideKeywordSearchConfig, keywordsearch.WireSet, controllerkeywordsearch.WireSet, + usergroup.WireSet, ) return &cliserver.System{}, nil } diff --git a/cmd/gitness/wire_gen.go b/cmd/gitness/wire_gen.go index 075ab6f2d..9d6d7ba37 100644 --- a/cmd/gitness/wire_gen.go +++ b/cmd/gitness/wire_gen.go @@ -58,6 +58,7 @@ import ( "github.com/harness/gitness/app/services/protection" "github.com/harness/gitness/app/services/pullreq" trigger2 "github.com/harness/gitness/app/services/trigger" + "github.com/harness/gitness/app/services/usergroup" "github.com/harness/gitness/app/services/webhook" "github.com/harness/gitness/app/sse" "github.com/harness/gitness/app/store" @@ -161,7 +162,8 @@ func initSystem(ctx context.Context, config *types.Config) (*server.System, erro return nil, err } codeownersConfig := server.ProvideCodeOwnerConfig(config) - codeownersService := codeowners.ProvideCodeOwners(gitInterface, repoStore, codeownersConfig, principalStore) + resolver := usergroup.ProvideUserGroupResolver() + codeownersService := codeowners.ProvideCodeOwners(gitInterface, repoStore, codeownersConfig, principalStore, resolver) eventsConfig := server.ProvideEventsConfig(config) eventsSystem, err := events.ProvideSystem(eventsConfig, universalClient) if err != nil { diff --git a/types/codeowners.go b/types/codeowners.go index 08b05ac4f..7b30befd8 100644 --- a/types/codeowners.go +++ b/types/codeowners.go @@ -26,8 +26,15 @@ type CodeOwnerEvaluation struct { } type CodeOwnerEvaluationEntry struct { - Pattern string `json:"pattern"` - OwnerEvaluations []OwnerEvaluation `json:"owner_evaluations"` + Pattern string `json:"pattern"` + OwnerEvaluations []OwnerEvaluation `json:"owner_evaluations"` + UserGroupOwnerEvaluations []UserGroupOwnerEvaluation `json:"user_group_owner_evaluations"` +} + +type UserGroupOwnerEvaluation struct { + ID string `json:"id"` + Name string `json:"name"` + Evaluations []OwnerEvaluation `json:"evaluations"` } type OwnerEvaluation struct {