From f600e7305992d51ce512ec5c01ae749e02629de7 Mon Sep 17 00:00:00 2001 From: atefeh Date: Tue, 7 May 2024 11:26:01 -0700 Subject: [PATCH] self review --- app/api/controller/space/list_repositories.go | 17 ++++++------- app/api/controller/space/list_spaces.go | 14 +++++------ app/auth/anonymouse.go | 2 +- app/services/exporter/repository.go | 8 ++++++- app/services/publicaccess/service.go | 2 ++ types/repo.go | 24 +++++++++---------- 6 files changed, 38 insertions(+), 29 deletions(-) diff --git a/app/api/controller/space/list_repositories.go b/app/api/controller/space/list_repositories.go index 01ae06066..7f613e9bc 100644 --- a/app/api/controller/space/list_repositories.go +++ b/app/api/controller/space/list_repositories.go @@ -20,6 +20,7 @@ import ( apiauth "github.com/harness/gitness/app/api/auth" "github.com/harness/gitness/app/api/controller/repo" + repoCtrl "github.com/harness/gitness/app/api/controller/repo" "github.com/harness/gitness/app/auth" "github.com/harness/gitness/store/database/dbtx" "github.com/harness/gitness/types" @@ -58,7 +59,7 @@ func (c *Controller) ListRepositoriesNoAuth( spaceID int64, filter *types.RepoFilter, ) ([]*repo.Repository, int64, error) { - var repos []*repo.Repository + var reposOutput []*repo.Repository var count int64 err := c.tx.WithTx(ctx, func(ctx context.Context) (err error) { @@ -67,23 +68,23 @@ func (c *Controller) ListRepositoriesNoAuth( return fmt.Errorf("failed to count child repos: %w", err) } - reposBase, err := c.repoStore.List(ctx, spaceID, filter) + repos, err := c.repoStore.List(ctx, spaceID, filter) if err != nil { return fmt.Errorf("failed to list child repos: %w", err) } - for _, repoBase := range reposBase { + for _, repo := range repos { // backfill URLs - repoBase.GitURL = c.urlProvider.GenerateGITCloneURL(repoBase.Path) + repo.GitURL = c.urlProvider.GenerateGITCloneURL(repo.Path) // backfill public access mode - isPublic, err := apiauth.CheckRepoIsPublic(ctx, c.publicAccess, repoBase) + isPublic, err := apiauth.CheckRepoIsPublic(ctx, c.publicAccess, repo) if err != nil { return fmt.Errorf("failed to get resource public access mode: %w", err) } - repos = append(repos, &repo.Repository{ - Repository: *repoBase, + reposOutput = append(reposOutput, &repoCtrl.Repository{ + Repository: *repo, IsPublic: isPublic, }) } @@ -93,5 +94,5 @@ func (c *Controller) ListRepositoriesNoAuth( return nil, 0, err } - return repos, count, nil + return reposOutput, count, nil } diff --git a/app/api/controller/space/list_spaces.go b/app/api/controller/space/list_spaces.go index 6b8154a4c..e409edfe6 100644 --- a/app/api/controller/space/list_spaces.go +++ b/app/api/controller/space/list_spaces.go @@ -56,7 +56,7 @@ func (c *Controller) ListSpacesNoAuth( spaceID int64, filter *types.SpaceFilter, ) ([]*Space, int64, error) { - var spaces []*Space + var spacesOutput []*Space var count int64 err := c.tx.WithTx(ctx, func(ctx context.Context) (err error) { @@ -65,20 +65,20 @@ func (c *Controller) ListSpacesNoAuth( return fmt.Errorf("failed to count child spaces: %w", err) } - spacesBase, err := c.spaceStore.List(ctx, spaceID, filter) + spaces, err := c.spaceStore.List(ctx, spaceID, filter) if err != nil { return fmt.Errorf("failed to list child spaces: %w", err) } - for _, spaceBase := range spacesBase { + for _, space := range spaces { // backfill public access mode - isPublic, err := apiauth.CheckSpaceIsPublic(ctx, c.publicAccess, spaceBase) + isPublic, err := apiauth.CheckSpaceIsPublic(ctx, c.publicAccess, space) if err != nil { return fmt.Errorf("failed to get resource public access mode: %w", err) } - spaces = append(spaces, &Space{ - Space: *spaceBase, + spacesOutput = append(spacesOutput, &Space{ + Space: *space, IsPublic: isPublic, }) } @@ -89,5 +89,5 @@ func (c *Controller) ListSpacesNoAuth( return nil, 0, err } - return spaces, count, nil + return spacesOutput, count, nil } diff --git a/app/auth/anonymouse.go b/app/auth/anonymouse.go index ad8e97fa4..f46f3ffc1 100644 --- a/app/auth/anonymouse.go +++ b/app/auth/anonymouse.go @@ -19,7 +19,7 @@ import ( "github.com/harness/gitness/types/enum" ) -// anonymousPrincipal is an in-memory principal for users with no auth data. +// AnonymousPrincipal is an in-memory principal for users with no auth data. // Authorizer is in charge of handling anonymouse access. var AnonymousPrincipal = types.Principal{ ID: -1, diff --git a/app/services/exporter/repository.go b/app/services/exporter/repository.go index c7eb3bcf2..9488f2cd9 100644 --- a/app/services/exporter/repository.go +++ b/app/services/exporter/repository.go @@ -24,6 +24,7 @@ import ( "strings" "time" + "github.com/harness/gitness/app/api/auth" "github.com/harness/gitness/app/api/controller/repo" "github.com/harness/gitness/app/services/publicaccess" "github.com/harness/gitness/app/sse" @@ -116,11 +117,16 @@ func (r *Repository) RunManyForSpace( jobDefinitions := make([]job.Definition, len(repos)) for i, repository := range repos { + isPublic, err := auth.CheckRepoIsPublic(ctx, r.publicAccess, repository) + if err != nil { + return fmt.Errorf("failed to check repo public access: %w", err) + } + repoJobData := Input{ Identifier: repository.Identifier, ID: repository.ID, Description: repository.Description, - IsPublic: false, // todo: use repository.IsPublic once public is available. + IsPublic: isPublic, HarnessCodeInfo: *harnessCodeInfo, } diff --git a/app/services/publicaccess/service.go b/app/services/publicaccess/service.go index 690c00807..c627ba8bf 100644 --- a/app/services/publicaccess/service.go +++ b/app/services/publicaccess/service.go @@ -22,6 +22,7 @@ import ( "github.com/harness/gitness/app/store" gitness_store "github.com/harness/gitness/store" "github.com/harness/gitness/types/enum" + "github.com/rs/zerolog/log" ) type Service struct { @@ -77,6 +78,7 @@ func (s *Service) Set( if enable { err := s.publicAccessStore.Create(ctx, resourceType, pubResID) if errors.Is(err, gitness_store.ErrDuplicate) { + log.Ctx(ctx).Warn().Msgf("repo %d is already set for public access", pubResID) return nil } return err diff --git a/types/repo.go b/types/repo.go index 3ae3704d9..d61fe1dde 100644 --- a/types/repo.go +++ b/types/repo.go @@ -57,6 +57,18 @@ type Repository struct { GitURL string `json:"git_url" yaml:"git_url"` } +// Clone makes deep copy of repository object. +func (r Repository) Clone() Repository { + var deleted *int64 + if r.Deleted != nil { + id := *r.Deleted + deleted = &id + } + r.Deleted = deleted + + return r +} + // TODO [CODE-1363]: remove after identifier migration. func (r Repository) MarshalJSON() ([]byte, error) { // alias allows us to embed the original object while avoiding an infinite loop of marshaling. @@ -70,18 +82,6 @@ func (r Repository) MarshalJSON() ([]byte, error) { }) } -// Clone makes deep copy of repository object. -func (r Repository) Clone() Repository { - var deleted *int64 - if r.Deleted != nil { - id := *r.Deleted - deleted = &id - } - r.Deleted = deleted - - return r -} - type RepositorySizeInfo struct { ID int64 `json:"id"` GitUID string `json:"git_uid"`