fix cleanup for failed repo create, cleanup summary git api (#2064)

app/api/controller/repo/create.go

@@ -125,7 +125,7 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea
  }, sql.TxOptions{Isolation: sql.LevelSerializable})
  if err != nil {
  // best effort cleanup
- if dErr := c.DeleteGitRepository(ctx, session, repo); dErr != nil {
+ if dErr := c.DeleteGitRepository(ctx, session, gitResp.UID); dErr != nil {
  log.Ctx(ctx).Warn().Err(dErr).Msg("failed to delete repo for cleanup")
  }
  return nil, err

app/api/controller/repo/purge.go

@@ -19,10 +19,10 @@ import (
  "fmt"
 
  apiauth "github.com/harness/gitness/app/api/auth"
- "github.com/harness/gitness/app/api/controller"
  "github.com/harness/gitness/app/api/usererror"
  "github.com/harness/gitness/app/auth"
  repoevents "github.com/harness/gitness/app/events/repo"
+ "github.com/harness/gitness/app/githook"
  "github.com/harness/gitness/errors"
  "github.com/harness/gitness/git"
  "github.com/harness/gitness/types"
@@ -76,7 +76,7 @@ func (c *Controller) PurgeNoAuth(
  return fmt.Errorf("failed to delete repo from db: %w", err)
  }
 
- if err := c.DeleteGitRepository(ctx, session, repo); err != nil {
+ if err := c.DeleteGitRepository(ctx, session, repo.GitUID); err != nil {
  log.Ctx(ctx).Err(err).Msg("failed to remove git repository")
  }
 
@@ -92,11 +92,27 @@ func (c *Controller) PurgeNoAuth(
 func (c *Controller) DeleteGitRepository(
  ctx context.Context,
  session *auth.Session,
- repo *types.Repository,
+ gitUID string,
 ) error {
- writeParams, err := controller.CreateRPCInternalWriteParams(ctx, c.urlProvider, session, repo)
+ // create custom write params for delete as repo might or might not exist in db (similar to create).
+ envVars, err := githook.GenerateEnvironmentVariables(
+ ctx,
+ c.urlProvider.GetInternalAPIURL(),
+ 0, // no repoID
+ session.Principal.ID,
+ true,
+ true,
+ )
  if err != nil {
- return fmt.Errorf("failed to create RPC write params: %w", err)
+ return fmt.Errorf("failed to generate git hook environment variables: %w", err)
+ }
+ writeParams := git.WriteParams{
+ Actor: git.Identity{
+ Name: session.Principal.DisplayName,
+ Email: session.Principal.Email,
+ },
+ RepoUID: gitUID,
+ EnvVars: envVars,
  }
 
  err = c.git.DeleteRepository(ctx, &git.DeleteRepositoryParams{
@@ -105,10 +121,10 @@ func (c *Controller) DeleteGitRepository(
 
  // deletion should not fail if repo dir does not exist.
  if errors.IsNotFound(err) {
- log.Ctx(ctx).Warn().Str("repo.git_uid", repo.GitUID).
+ log.Ctx(ctx).Warn().Str("repo.git_uid", gitUID).
  Msg("git repository directory does not exist")
  } else if err != nil {
- return fmt.Errorf("failed to remove git repository %s: %w", repo.GitUID, err)
+ return fmt.Errorf("failed to remove git repository %s: %w", gitUID, err)
  }
  return nil
 }

app/api/controller/repo/summary.go

@@ -35,7 +35,7 @@ func (c *Controller) Summary(
  return nil, fmt.Errorf("access check failed: %w", err)
  }
 
- summary, err := c.git.Summary(ctx, &git.ReadParams{RepoUID: repo.GitUID})
+ summary, err := c.git.Summary(ctx, git.SummaryParams{ReadParams: git.CreateReadParams(repo)})
  if err != nil {
  return nil, fmt.Errorf("failed to get repo summary: %w", err)
  }

app/api/controller/space/purge.go

@@ -80,7 +80,7 @@ func (c *Controller) PurgeNoAuth(
  // permanently purge all repositories in the space and its subspaces after successful space purge tnx.
  // cleanup will handle failed repository deletions.
  for _, repo := range toBeDeletedRepos {
- err := c.repoCtrl.DeleteGitRepository(ctx, session, repo)
+ err := c.repoCtrl.DeleteGitRepository(ctx, session, repo.GitUID)
  if err != nil {
  log.Ctx(ctx).Warn().Err(err).
  Str("repo_identifier", repo.Identifier).

app/api/middleware/authn/authn.go

@@ -29,6 +29,7 @@ import (
 
 // Attempt returns an http.HandlerFunc middleware that authenticates
 // the http.Request if authentication payload is available.
+// Otherwise, an anonymous user session is used instead.
 func Attempt(authenticator authn.Authenticator) func(http.Handler) http.Handler {
  return func(next http.Handler) http.Handler {
  return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

app/auth/authz/membership.go

@@ -55,7 +55,7 @@ func (a *MembershipAuthorizer) Check(
  resource *types.Resource,
  permission enum.Permission,
 ) (bool, error) {
- publicAccessAllowed, err := a.CheckPublicAccess(ctx, scope, resource, permission)
+ publicAccessAllowed, err := CheckPublicAccess(ctx, a.publicAccess, scope, resource, permission)
  if err != nil {
  return false, fmt.Errorf("failed to check public access: %w", err)
  }
@@ -64,15 +64,6 @@ func (a *MembershipAuthorizer) Check(
  return true, nil
  }
 
- if session == nil {
- log.Ctx(ctx).Warn().Msgf(
- "public access request for %s in scope %#v got to authorizer",
- permission,
- scope,
- )
- return false, nil
- }
-
  log.Ctx(ctx).Debug().Msgf(
  "[MembershipAuthorizer] %s with id '%d' requests %s for %s '%s' in scope %#v with metadata %#v",
  session.Principal.Type,

app/auth/authz/public_access.go

@@ -16,34 +16,43 @@ package authz
 
 import (
  "context"
+ "fmt"
 
  "github.com/harness/gitness/app/paths"
+ "github.com/harness/gitness/app/services/publicaccess"
  "github.com/harness/gitness/types"
  "github.com/harness/gitness/types/enum"
 )
 
-func (a *MembershipAuthorizer) CheckPublicAccess(
+// CheckPublicAccess checks if the requested permission is public for the provided scope and resource.
+func CheckPublicAccess(
  ctx context.Context,
+ publicAccess publicaccess.Service,
  scope *types.Scope,
  resource *types.Resource,
  permission enum.Permission,
 ) (bool, error) {
  var pubResType enum.PublicResourceType
+ var pubResPath string
 
  //nolint:exhaustive
  switch resource.Type {
  case enum.ResourceTypeSpace:
  pubResType = enum.PublicResourceTypeSpace
+ pubResPath = paths.Concatenate(scope.SpacePath, resource.Identifier)
 
  case enum.ResourceTypeRepo:
  if resource.Identifier != "" {
  pubResType = enum.PublicResourceTypeRepo
+ pubResPath = paths.Concatenate(scope.SpacePath, resource.Identifier)
  } else { // for spaceScope checks
  pubResType = enum.PublicResourceTypeSpace
+ pubResPath = scope.SpacePath
  }
 
  case enum.ResourceTypePipeline:
  pubResType = enum.PublicResourceTypeRepo
+ pubResPath = paths.Concatenate(scope.SpacePath, scope.Repo)
 
  default:
  return false, nil
@@ -61,7 +70,10 @@ func (a *MembershipAuthorizer) CheckPublicAccess(
  return false, nil
  }
 
- pubResPath := paths.Concatenate(scope.SpacePath, resource.Identifier)
+ resourceIsPublic, err := publicAccess.Get(ctx, pubResType, pubResPath)
+ if err != nil {
+ return false, fmt.Errorf("failed to check public accessabillity of %s %q: %w", pubResType, pubResPath, err)
+ }
 
- return a.publicAccess.Get(ctx, pubResType, pubResPath)
+ return resourceIsPublic, nil
 }

app/pipeline/converter/converter.go

@@ -36,8 +36,11 @@ type converter struct {
  publicAccess publicaccess.Service
 }
 
-func newConverter(fileService file.Service) Service {
- return &converter{fileService: fileService}
+func newConverter(fileService file.Service, publicAccess publicaccess.Service) Service {
+ return &converter{
+ fileService: fileService,
+ publicAccess: publicAccess,
+ }
 }
 
 func (c *converter) Convert(ctx context.Context, args *ConvertArgs) (*file.File, error) {

app/pipeline/converter/wire.go

@@ -16,6 +16,7 @@ package converter
 
 import (
  "github.com/harness/gitness/app/pipeline/file"
+ "github.com/harness/gitness/app/services/publicaccess"
 
  "github.com/google/wire"
 )
@@ -26,6 +27,6 @@ var WireSet = wire.NewSet(
 )
 
 // ProvideService provides a service which can convert templates.
-func ProvideService(fileService file.Service) Service {
- return newConverter(fileService)
+func ProvideService(fileService file.Service, publicAccess publicaccess.Service) Service {
+ return newConverter(fileService, publicAccess)
 }

git/interface.go

@@ -38,6 +38,7 @@ type Interface interface {
  UpdateDefaultBranch(ctx context.Context, params *UpdateDefaultBranchParams) error
  GetRef(ctx context.Context, params GetRefParams) (GetRefResponse, error)
  PathsDetails(ctx context.Context, params PathsDetailsParams) (PathsDetailsOutput, error)
+ Summary(ctx context.Context, params SummaryParams) (SummaryOutput, error)
 
  // GetRepositorySize calculates the size of a repo in KiB.
  GetRepositorySize(ctx context.Context, params *GetRepositorySizeParams) (*GetRepositorySizeOutput, error)
@@ -102,9 +103,4 @@ type Interface interface {
  */
  ScanSecrets(ctx context.Context, param *ScanSecretsParams) (*ScanSecretsOutput, error)
  Archive(ctx context.Context, params ArchiveParams, w io.Writer) error
-
- /*
- * Repo Summary service
- */
- Summary(ctx context.Context, params *ReadParams) (*SummaryOutput, error)
 }

git/summary.go

@@ -24,6 +24,10 @@ import (
  "golang.org/x/sync/errgroup"
 )
 
+type SummaryParams struct {
+ ReadParams
+}
+
 type SummaryOutput struct {
  CommitCount int
  BranchCount int
@@ -32,13 +36,13 @@ type SummaryOutput struct {
 
 func (s *Service) Summary(
  ctx context.Context,
- params *ReadParams,
-) (*SummaryOutput, error) {
+ params SummaryParams,
+) (SummaryOutput, error) {
  repoPath := getFullPathForRepo(s.reposRoot, params.RepoUID)
 
  defaultBranch, err := s.git.GetDefaultBranch(ctx, repoPath)
  if err != nil {
- return nil, err
+ return SummaryOutput{}, err
  }
  defaultBranch = strings.TrimSpace(defaultBranch)
 
@@ -65,10 +69,10 @@ func (s *Service) Summary(
  })
 
  if err := g.Wait(); err != nil {
- return nil, fmt.Errorf("failed to get repo summary: %w", err)
+ return SummaryOutput{}, fmt.Errorf("failed to get repo summary: %w", err)
  }
 
- return &SummaryOutput{
+ return SummaryOutput{
  CommitCount: commitCount,
  BranchCount: branchCount,
  TagCount: tagCount,