From e423ffda3cf44567a100caa13aba451bd3ec5ae6 Mon Sep 17 00:00:00 2001 From: Johannes Batzill Date: Mon, 17 Oct 2022 19:09:46 -0700 Subject: [PATCH] Fix Default Branch Creation (#37) While the HEAD is pointed to the correct branch (might not exist), we created the initial files during repo creation still on the master branch (as it's an empty repo and clone by default sets up master when cloning an empty repo) --- cli/server/standalone.wire_gen.go | 2 +- internal/api/controller/repo/controller.go | 12 +++++----- internal/api/controller/repo/create.go | 26 +++++++++++----------- internal/api/controller/repo/wire.go | 5 +++-- internal/api/handler/repo/create.go | 8 +------ internal/api/openapi/resource.go | 4 ++-- internal/gitrpc/gitea.go | 17 ++++++++++++-- internal/gitrpc/interface.go | 2 +- internal/gitrpc/repoService.go | 7 +++--- internal/router/api.go | 13 ++++++----- 10 files changed, 54 insertions(+), 42 deletions(-) diff --git a/cli/server/standalone.wire_gen.go b/cli/server/standalone.wire_gen.go index 7d8f6209a..b15d22f37 100644 --- a/cli/server/standalone.wire_gen.go +++ b/cli/server/standalone.wire_gen.go @@ -46,7 +46,7 @@ func initSystem(ctx context.Context, config *types.Config) (*system, error) { if err != nil { return nil, err } - repoController := repo.ProvideController(authorizer, spaceStore, repoStore, serviceAccountStore, gitrpcInterface) + repoController := repo.ProvideController(config, authorizer, spaceStore, repoStore, serviceAccountStore, gitrpcInterface) spaceController := space.NewController(authorizer, spaceStore, repoStore, serviceAccountStore) serviceaccountController := serviceaccount.NewController(authorizer, serviceAccountStore, spaceStore, repoStore, tokenStore) apiHandler := router.ProvideAPIHandler(systemStore, authenticator, repoController, spaceController, serviceaccountController, controller) diff --git a/internal/api/controller/repo/controller.go b/internal/api/controller/repo/controller.go index ee5ee04ba..ef85fc83f 100644 --- a/internal/api/controller/repo/controller.go +++ b/internal/api/controller/repo/controller.go @@ -20,6 +20,7 @@ type Controller struct { } func NewController( + defaultBranch string, authorizer authz.Authorizer, spaceStore store.SpaceStore, repoStore store.RepoStore, @@ -27,10 +28,11 @@ func NewController( gitRPCClient gitrpc.Interface, ) *Controller { return &Controller{ - authorizer: authorizer, - spaceStore: spaceStore, - repoStore: repoStore, - saStore: saStore, - gitRPCClient: gitRPCClient, + defaultBranch: defaultBranch, + authorizer: authorizer, + spaceStore: spaceStore, + repoStore: repoStore, + saStore: saStore, + gitRPCClient: gitRPCClient, } } diff --git a/internal/api/controller/repo/create.go b/internal/api/controller/repo/create.go index d5e970cc3..18b6d53d0 100644 --- a/internal/api/controller/repo/create.go +++ b/internal/api/controller/repo/create.go @@ -25,16 +25,16 @@ import ( ) type CreateInput struct { - PathName string `json:"pathName"` - SpaceID int64 `json:"spaceId"` - Name string `json:"name"` - Branch string `json:"branch"` - Description string `json:"description"` - IsPublic bool `json:"isPublic"` - ForkID int64 `json:"forkId"` - Readme bool `json:"readme"` - License string `json:"license"` - GitIgnore string `json:"gitIgnore"` + PathName string `json:"pathName"` + SpaceID int64 `json:"spaceId"` + Name string `json:"name"` + DefaultBranch string `json:"defaultBranch"` + Description string `json:"description"` + IsPublic bool `json:"isPublic"` + ForkID int64 `json:"forkId"` + Readme bool `json:"readme"` + License string `json:"license"` + GitIgnore string `json:"gitIgnore"` } // Create creates a new repository. @@ -68,8 +68,8 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea } // set default branch in case it wasn't passed - if in.Branch == "" { - in.Branch = c.defaultBranch + if in.DefaultBranch == "" { + in.DefaultBranch = c.defaultBranch } // create new repo object @@ -83,7 +83,7 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea Created: time.Now().UnixMilli(), Updated: time.Now().UnixMilli(), ForkID: in.ForkID, - DefaultBranch: in.Branch, + DefaultBranch: in.DefaultBranch, } // validate repo diff --git a/internal/api/controller/repo/wire.go b/internal/api/controller/repo/wire.go index e69834fc4..4a174e613 100644 --- a/internal/api/controller/repo/wire.go +++ b/internal/api/controller/repo/wire.go @@ -9,6 +9,7 @@ import ( "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/gitrpc" "github.com/harness/gitness/internal/store" + "github.com/harness/gitness/types" ) // WireSet provides a wire set for this package. @@ -16,7 +17,7 @@ var WireSet = wire.NewSet( ProvideController, ) -func ProvideController(authorizer authz.Authorizer, spaceStore store.SpaceStore, +func ProvideController(config *types.Config, authorizer authz.Authorizer, spaceStore store.SpaceStore, repoStore store.RepoStore, saStore store.ServiceAccountStore, rpcClient gitrpc.Interface) *Controller { - return NewController(authorizer, spaceStore, repoStore, saStore, rpcClient) + return NewController(config.Git.DefaultBranch, authorizer, spaceStore, repoStore, saStore, rpcClient) } diff --git a/internal/api/handler/repo/create.go b/internal/api/handler/repo/create.go index 152bd8a29..5aef741c9 100644 --- a/internal/api/handler/repo/create.go +++ b/internal/api/handler/repo/create.go @@ -11,11 +11,10 @@ import ( "github.com/harness/gitness/internal/api/controller/repo" "github.com/harness/gitness/internal/api/render" "github.com/harness/gitness/internal/api/request" - "github.com/harness/gitness/types" ) // HandleCreate returns a http.HandlerFunc that creates a new repository. -func HandleCreate(config *types.Config, repoCtrl *repo.Controller) http.HandlerFunc { +func HandleCreate(repoCtrl *repo.Controller) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() session, _ := request.AuthSessionFrom(ctx) @@ -27,11 +26,6 @@ func HandleCreate(config *types.Config, repoCtrl *repo.Controller) http.HandlerF return } - // set default branch if it wasn't provided by input - if in.Branch == "" { - in.Branch = config.Git.DefaultBranch - } - repo, err := repoCtrl.Create(ctx, session, in) if err != nil { render.TranslatedUserError(w, err) diff --git a/internal/api/openapi/resource.go b/internal/api/openapi/resource.go index 2077a9eaf..71af92122 100644 --- a/internal/api/openapi/resource.go +++ b/internal/api/openapi/resource.go @@ -5,12 +5,12 @@ package openapi import ( + "net/http" + "github.com/harness/gitness/internal/api/usererror" "github.com/swaggest/openapi-go/openapi3" - "net/http" ) -//nolint:funlen func resourceOperations(reflector *openapi3.Reflector) { opListGitignore := openapi3.Operation{} opListGitignore.WithTags("resource") diff --git a/internal/gitrpc/gitea.go b/internal/gitrpc/gitea.go index 5a8337e56..9989bc1e8 100644 --- a/internal/gitrpc/gitea.go +++ b/internal/gitrpc/gitea.go @@ -38,14 +38,27 @@ func (g giteaAdapter) InitRepository(ctx context.Context, repoPath string, bare } // SetDefaultBranch sets the default branch of a repo. -func (g giteaAdapter) SetDefaultBranch(ctx context.Context, repoPath string, defaultBranch string) error { +func (g giteaAdapter) SetDefaultBranch(ctx context.Context, repoPath string, + defaultBranch string, allowEmpty bool) error { giteaRepo, err := gitea.OpenRepository(ctx, repoPath) if err != nil { return err } defer giteaRepo.Close() - return giteaRepo.SetDefaultBranch(defaultBranch) + // if requested, error out if branch doesn't exist. Otherwise, blindly set it. + if !allowEmpty && !giteaRepo.IsBranchExist(defaultBranch) { + // TODO: ensure this returns not found error to caller + return fmt.Errorf("branch '%s' does not exist", defaultBranch) + } + + // change default branch + err = giteaRepo.SetDefaultBranch(defaultBranch) + if err != nil { + return fmt.Errorf("failed to set new default branch: %w", err) + } + + return nil } func (g giteaAdapter) Clone(ctx context.Context, from, to string, opts cloneRepoOption) error { diff --git a/internal/gitrpc/interface.go b/internal/gitrpc/interface.go index eb87b41b1..0dfcafff1 100644 --- a/internal/gitrpc/interface.go +++ b/internal/gitrpc/interface.go @@ -18,7 +18,7 @@ type Interface interface { // gitAdapter for accessing git commands from gitea. type gitAdapter interface { InitRepository(ctx context.Context, path string, bare bool) error - SetDefaultBranch(ctx context.Context, repoPath string, defaultBranch string) error + SetDefaultBranch(ctx context.Context, repoPath string, defaultBranch string, allowEmpty bool) error Clone(ctx context.Context, from, to string, opts cloneRepoOption) error AddFiles(repoPath string, all bool, files ...string) error Commit(repoPath string, opts commitChangesOptions) error diff --git a/internal/gitrpc/repoService.go b/internal/gitrpc/repoService.go index dae1488af..ac5b05690 100644 --- a/internal/gitrpc/repoService.go +++ b/internal/gitrpc/repoService.go @@ -92,8 +92,8 @@ func (s repositoryService) CreateRepository(stream rpc.RepositoryService_CreateR return fmt.Errorf("CreateRepository error: %w", err) } - // update default branch - err = s.adapter.SetDefaultBranch(ctx, repoPath, header.GetDefaultBranch()) + // update default branch (currently set to non-existent branch) + err = s.adapter.SetDefaultBranch(ctx, repoPath, header.GetDefaultBranch(), true) if err != nil { return fmt.Errorf("error updating default branch for repo %s: %w", header.GetUid(), err) } @@ -139,8 +139,9 @@ func (s repositoryService) CreateRepository(stream rpc.RepositoryService_CreateR } if len(filePaths) > 0 { + // NOTE: This creates the branch in origin repo (as it doesn't exist as of now) // TODO: this should at least be a constant and not hardcoded? - if err = s.AddFilesAndPush(ctx, tempDir, filePaths, "", SystemIdentity, SystemIdentity, + if err = s.AddFilesAndPush(ctx, tempDir, filePaths, "HEAD:"+header.GetDefaultBranch(), SystemIdentity, SystemIdentity, "origin", "initial commit"); err != nil { return err } diff --git a/internal/router/api.go b/internal/router/api.go index 9ab7eaac4..fa53eaaaf 100644 --- a/internal/router/api.go +++ b/internal/router/api.go @@ -7,9 +7,10 @@ package router import ( "context" "fmt" - "github.com/harness/gitness/internal/api/handler/resource" "net/http" + "github.com/harness/gitness/internal/api/handler/resource" + "github.com/harness/gitness/internal/api/controller/repo" "github.com/harness/gitness/internal/api/controller/serviceaccount" "github.com/harness/gitness/internal/api/controller/space" @@ -77,7 +78,7 @@ func NewAPIHandler( r.Use(middlewareauthn.Attempt(authenticator)) r.Route("/v1", func(r chi.Router) { - setupRoutesV1(r, config, repoCtrl, spaceCtrl, saCtrl, userCtrl) + setupRoutesV1(r, repoCtrl, spaceCtrl, saCtrl, userCtrl) }) // wrap router in terminatedPath encoder. @@ -97,10 +98,10 @@ func corsHandler(config *types.Config) func(http.Handler) http.Handler { ).Handler } -func setupRoutesV1(r chi.Router, config *types.Config, repoCtrl *repo.Controller, spaceCtrl *space.Controller, +func setupRoutesV1(r chi.Router, repoCtrl *repo.Controller, spaceCtrl *space.Controller, saCtrl *serviceaccount.Controller, userCtrl *user.Controller) { setupSpaces(r, spaceCtrl) - setupRepos(r, config, repoCtrl) + setupRepos(r, repoCtrl) setupUsers(r, userCtrl) setupServiceAccounts(r, saCtrl) setupAdmin(r, userCtrl) @@ -139,10 +140,10 @@ func setupSpaces(r chi.Router, spaceCtrl *space.Controller) { }) } -func setupRepos(r chi.Router, config *types.Config, repoCtrl *repo.Controller) { +func setupRepos(r chi.Router, repoCtrl *repo.Controller) { r.Route("/repos", func(r chi.Router) { // Create takes path and parentId via body, not uri - r.Post("/", handlerrepo.HandleCreate(config, repoCtrl)) + r.Post("/", handlerrepo.HandleCreate(repoCtrl)) r.Route(fmt.Sprintf("/{%s}", request.PathParamRepoRef), func(r chi.Router) { // repo level operations r.Get("/", handlerrepo.HandleFind(repoCtrl))