Improve API repo migration (#11)

* API: make migrateOptions consistent.

- There's a subtle difference between the option provided when migrating see e153cf0

* API: use `task.MigrateRepository` when migrating from the API.

- Remove the repo creation step; `task.MigrateRepository` creates repo internally, creating a repo before calling it will cause conflict which will result in migration failure.

* API: make error handling for `migration` consistent with UI.

* fixup! API: make error handling for `migration` consistent with UI.

* API: add migration status API endpoint.

* fixup! API: add migration status API endpoint.

* Clean docker file

* API: fix migration status endpoint.

- Rtuse the same logic for getting task status in the UI.
Co-authored-by: Kaspar Emanuel <[email protected]>

Author: AbdulrhmnGhanem

Dockerfile.gitea.dev

@@ -1,52 +1,49 @@
-
-###################################
-#Build stage
 FROM golang:1.15-alpine3.12
 
 ARG GOPROXY
 ENV GOPROXY ${GOPROXY:-direct}
 
-ARG GITEA_VERSION
 ARG TAGS="sqlite sqlite_unlock_notify"
 ENV TAGS "bindata timetzdata $TAGS"
 ARG CGO_EXTRA_CFLAGS
 
-#Build deps
-RUN apk --no-cache add build-base git nodejs npm
+#Build & runtime deps
+RUN apk --no-cache add \
+  build-base \
+  git \
+  nodejs \
+  npm \
+  bash \
+  ca-certificates \
+  curl \
+  gettext \
+  git \
+  linux-pam \
+  openssh \
+  s6 \
+  sqlite \
+  su-exec \
+  gnupg
+
 
 #Setup repo
 COPY . /go/src/code.gitea.io/gitea
 WORKDIR /go/src/code.gitea.io/gitea
 
-#Checkout version if set
-RUN if [ -n "${GITEA_VERSION}" ]; then git checkout "${GITEA_VERSION}"; fi \
- && make clean-all build
+RUN make build
 
 EXPOSE 22 3000
 
-RUN apk --no-cache add \
-    bash \
-    ca-certificates \
-    curl \
-    gettext \
-    git \
-    linux-pam \
-    openssh \
-    s6 \
-    sqlite \
-    su-exec \
-    gnupg
-
 RUN addgroup \
-    -S -g 1000 \
-    git && \
+  -S -g 1000 \
+  git && \
   adduser \
-    -S -H -D \
-    -h /data/git \
-    -s /bin/bash \
-    -u 1000 \
-    -G git \
-    git && \
+  -S -H -D \
+  -h /data/git \
+  -s /bin/bash \
+  -u 1000 \
+  -G git \
+  git && \
   echo "git:$(dd if=/dev/urandom bs=24 count=1 status=none | base64)" | chpasswd
 
 ENV USER git

modules/task/task.go

@@ -77,7 +77,7 @@ func CreateMigrateTask(doer, u *models.User, opts base.MigrateOptions) (*models.
 	opts.CloneAddr = util.NewStringURLSanitizer(opts.CloneAddr, true).Replace(opts.CloneAddr)
 	opts.AuthPasswordEncrypted, err = secret.EncryptSecret(setting.SecretKey, opts.AuthPassword)
 	if err != nil {
-		return nil, nil.err
+		return nil, nil, err
 	}
 	opts.AuthPassword = ""
 	opts.AuthTokenEncrypted, err = secret.EncryptSecret(setting.SecretKey, opts.AuthToken)

routers/api/v1/api.go

@@ -720,7 +720,10 @@ func Routes() *web.Route {
 
 			m.Get("/issues/search", repo.SearchIssues)
 
-			m.Post("/migrate", reqToken(), bind(api.MigrateRepoOptions{}), repo.Migrate)
+			m.Group("/migrate", func() {
+				m.Post("", reqToken(), bind(api.MigrateRepoOptions{}), repo.Migrate)
+				m.Get("/status", repo.GetMigratingTask)
+			})
 
 			m.Group("/{username}/{reponame}", func() {
 				m.Combo("").Get(reqAnyRepoReader(), repo.Get).

routers/api/v1/repo/migrate.go

@@ -5,24 +5,21 @@
 package repo
 
 import (
-	"bytes"
-	"errors"
 	"fmt"
 	"net/http"
 	"strings"
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/convert"
-	"code.gitea.io/gitea/modules/graceful"
+	auth "code.gitea.io/gitea/modules/forms"
 	"code.gitea.io/gitea/modules/lfs"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/migrations"
 	"code.gitea.io/gitea/modules/migrations/base"
-	"code.gitea.io/gitea/modules/notification"
-	repo_module "code.gitea.io/gitea/modules/repository"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/task"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/services/forms"
@@ -53,6 +50,31 @@ func Migrate(ctx *context.APIContext) {
 	form := web.GetForm(ctx).(*api.MigrateRepoOptions)
 
 	//get repoOwner
+	if setting.Repository.DisableMigrations {
+		ctx.Error(http.StatusForbidden, "MigrationsGlobalDisabled", fmt.Errorf("the site administrator has disabled migrations"))
+		return
+	}
+
+	if form.Mirror && setting.Repository.DisableMirrors {
+		ctx.Error(http.StatusForbidden, "MirrorsGlobalDisabled", fmt.Errorf("the site administrator has disabled mirrors"))
+		return
+	}
+
+	form.LFS = form.LFS && setting.LFS.StartServer
+
+	if form.LFS && len(form.LFSEndpoint) > 0 {
+		ep := lfs.DetermineEndpoint("", form.LFSEndpoint)
+		if ep == nil {
+			ctx.Error(http.StatusInternalServerError, "", ctx.Tr("repo.migrate.invalid_lfs_endpoint"))
+			return
+		}
+		err = migrations.IsMigrateURLAllowed(ep.String(), ctx.User)
+		if err != nil {
+			handleRemoteAddrError(ctx, err)
+			return
+		}
+	}
+
 	var (
 		repoOwner *models.User
 		err       error
@@ -108,31 +130,6 @@ func Migrate(ctx *context.APIContext) {
 
 	gitServiceType := convert.ToGitServiceType(form.Service)
 
-	if form.Mirror && setting.Repository.DisableMirrors {
-		ctx.Error(http.StatusForbidden, "MirrorsGlobalDisabled", fmt.Errorf("the site administrator has disabled mirrors"))
-		return
-	}
-
-	if setting.Repository.DisableMigrations {
-		ctx.Error(http.StatusForbidden, "MigrationsGlobalDisabled", fmt.Errorf("the site administrator has disabled migrations"))
-		return
-	}
-
-	form.LFS = form.LFS && setting.LFS.StartServer
-
-	if form.LFS && len(form.LFSEndpoint) > 0 {
-		ep := lfs.DetermineEndpoint("", form.LFSEndpoint)
-		if ep == nil {
-			ctx.Error(http.StatusInternalServerError, "", ctx.Tr("repo.migrate.invalid_lfs_endpoint"))
-			return
-		}
-		err = migrations.IsMigrateURLAllowed(ep.String(), ctx.User)
-		if err != nil {
-			handleRemoteAddrError(ctx, err)
-			return
-		}
-	}
-
 	var opts = migrations.MigrateOptions{
 		CloneAddr:      remoteAddr,
 		RepoName:       form.RepoName,
@@ -145,7 +142,7 @@ func Migrate(ctx *context.APIContext) {
 		AuthPassword:   form.AuthPassword,
 		AuthToken:      form.AuthToken,
 		Wiki:           form.Wiki,
-		Issues:         form.Issues,
+		Issues:         form.Issues || form.PullRequests,
 		Milestones:     form.Milestones,
 		Labels:         form.Labels,
 		Comments:       true,
@@ -163,63 +160,33 @@ func Migrate(ctx *context.APIContext) {
 		opts.Releases = false
 	}
 
-	repo, err := repo_module.CreateRepository(ctx.User, repoOwner, models.CreateRepoOptions{
-		Name:           opts.RepoName,
-		Description:    opts.Description,
-		OriginalURL:    form.CloneAddr,
-		GitServiceType: gitServiceType,
-		IsPrivate:      opts.Private,
-		IsMirror:       opts.Mirror,
-		Status:         models.RepositoryBeingMigrated,
-	})
-	if err != nil {
-		handleMigrateError(ctx, repoOwner, remoteAddr, err)
+	if err = models.CheckCreateRepository(ctx.User, repoOwner, opts.RepoName, false); err != nil {
+		handleMigrateError(ctx, repoOwner, &opts, err)
 		return
 	}
 
-	opts.MigrateToRepoID = repo.ID
-
-	defer func() {
-		if e := recover(); e != nil {
-			var buf bytes.Buffer
-			fmt.Fprintf(&buf, "Handler crashed with error: %v", log.Stack(2))
-
-			err = errors.New(buf.String())
-		}
-
-		if err == nil {
-			notification.NotifyMigrateRepository(ctx.User, repoOwner, repo)
-			return
-		}
-
-		if repo != nil {
-			if errDelete := models.DeleteRepository(ctx.User, repoOwner.ID, repo.ID); errDelete != nil {
-				log.Error("DeleteRepository: %v", errDelete)
-			}
-		}
-	}()
-
-	if _, err = migrations.MigrateRepository(graceful.GetManager().HammerContext(), ctx.User, repoOwner.Name, opts, nil); err != nil {
-		handleMigrateError(ctx, repoOwner, remoteAddr, err)
+	repo, err := task.MigrateRepository(ctx.User, repoOwner, opts)
+	if err == nil {
+		log.Trace("Repository migrated: %s/%s", repoOwner.Name, form.RepoName)
+		ctx.JSON(http.StatusCreated, convert.ToRepo(repo, models.AccessModeAdmin))
 		return
 	}
 
-	log.Trace("Repository migrated: %s/%s", repoOwner.Name, form.RepoName)
-	ctx.JSON(http.StatusCreated, convert.ToRepo(repo, models.AccessModeAdmin))
+	handleMigrateError(ctx, repoOwner, &opts, err)
 }
 
-func handleMigrateError(ctx *context.APIContext, repoOwner *models.User, remoteAddr string, err error) {
+func handleMigrateError(ctx *context.APIContext, repoOwner *models.User, migrationOpts *migrations.MigrateOptions, err error) {
 	switch {
-	case models.IsErrRepoAlreadyExist(err):
-		ctx.Error(http.StatusConflict, "", "The repository with the same name already exists.")
-	case models.IsErrRepoFilesAlreadyExist(err):
-		ctx.Error(http.StatusConflict, "", "Files already exist for this repository. Adopt them or delete them.")
 	case migrations.IsRateLimitError(err):
 		ctx.Error(http.StatusUnprocessableEntity, "", "Remote visit addressed rate limitation.")
 	case migrations.IsTwoFactorAuthError(err):
 		ctx.Error(http.StatusUnprocessableEntity, "", "Remote visit required two factors authentication.")
 	case models.IsErrReachLimitOfRepo(err):
 		ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("You have already reached your limit of %d repositories.", repoOwner.MaxCreationLimit()))
+	case models.IsErrRepoAlreadyExist(err):
+		ctx.Error(http.StatusConflict, "", "The repository with the same name already exists.")
+	case models.IsErrRepoFilesAlreadyExist(err):
+		ctx.Error(http.StatusConflict, "", "Files already exist for this repository. Adopt them or delete them.")
 	case models.IsErrNameReserved(err):
 		ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("The username '%s' is reserved.", err.(models.ErrNameReserved).Name))
 	case models.IsErrNameCharsNotAllowed(err):
@@ -231,6 +198,7 @@ func handleMigrateError(ctx *context.APIContext, repoOwner *models.User, remoteA
 	case base.IsErrNotSupported(err):
 		ctx.Error(http.StatusUnprocessableEntity, "", err)
 	default:
+		remoteAddr, _ := auth.ParseRemoteAddr(migrationOpts.CloneAddr, migrationOpts.AuthUsername, migrationOpts.AuthPassword)
 		err = util.NewStringURLSanitizedError(err, remoteAddr, true)
 		if strings.Contains(err.Error(), "Authentication failed") ||
 			strings.Contains(err.Error(), "Bad credentials") ||
@@ -267,3 +235,36 @@ func handleRemoteAddrError(ctx *context.APIContext, err error) {
 		ctx.Error(http.StatusInternalServerError, "ParseRemoteAddr", err)
 	}
 }
+
+// GetMigratingTask returns the migrating task by repo's id
+func GetMigratingTask(ctx *context.APIContext) {
+	// swagger:operation GET /repos/migrate/status task
+	// ---
+	// summary: Get the migration status of a repository by its id
+	// produces:
+	// - application/json
+	// parameters:
+	// - name: repo_id
+	//   in: query
+	//   description: repository id
+	//   type: int64
+	// responses:
+	//   "200":
+	//     "$ref": "#/responses/"
+	//   "404":
+	//	   "$ref": "#/response/"
+	t, err := models.GetMigratingTask(ctx.QueryInt64("repo_id"))
+
+	if err != nil {
+		ctx.JSON(http.StatusNotFound, err)
+		return
+	}
+
+	ctx.JSON(200, map[string]interface{}{
+		"status":  t.Status,
+		"err":     t.Errors,
+		"repo-id": t.RepoID,
+		"start":   t.StartTime,
+		"end":     t.EndTime,
+	})
+}

routers/web/repo/migrate.go

@@ -232,7 +232,7 @@ func MigratePost(ctx *context.Context) {
 		return
 	}
 
-	err = task.MigrateRepository(ctx.User, ctxUser, opts)
+	_, err = task.MigrateRepository(ctx.User, ctxUser, opts)
 	if err == nil {
 		ctx.Redirect(ctxUser.HomeLink() + "/" + opts.RepoName)
 		return