Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for migrating from Gitlab #9084

Open
wants to merge 25 commits into
base: master
from

Conversation

@aqtrans
Copy link
Contributor

aqtrans commented Nov 20, 2019

This adds a new migration interface to migrations from Gitlab, either self-hosted or hosted.

Another fix for all migrations is included, in eaf52a4.
While working on this, I noticed due to OriginalURL being set to CloneAddr in CreateMigrateTask that all migrated repos, Github as well, included the credentials in the database, as well as any "Migrated from" links in comments and such.

I didn't want to change the UI, so the self-hosted Gitlab detection is based on a '#gitlab' URL fragment added to the end of the Clone Address. Not the best solution, but it does the trick for now and doesn't interfere with the migration process.

The Gitlab calls are all powered by github.com/xanzy/go-gitlab, which has been vendored.

All objects in the migrations API are supported.
There are some odd hacks related to comments and PRs, due to PRs and Issues sharing IDs in Gitea.

aqtrans added 14 commits Nov 18, 2019
Still need to figure out how to hide tokens/etc from showing up in opts.CloneAddr
CloneAddr was being used as OriginalURL.
Now passing OriginalURL through from the form and saving it.
Correct CloneURL.
This should be functioning!
Previous commits fixed "Migrated from"
from including the migration credentials.
RepoID is grabbed in NewGitlabDownloader
Properly set milestone deadline time.
Consistently use Gitlab username for 'Name'.
- Count of issues is kept to set a non-conflicting PR.ID
- Bool is used to tell whether to fetch Issue or PR comments
Copy link
Member

techknowlogick left a comment

Thank you for your contribution!! This will be so helpful for many users.

I haven't done a thorough review of this PR, but I found a few nits that would need to be resolved.

modules/migrations/gitlab.go Outdated Show resolved Hide resolved
modules/migrations/gitlab.go Outdated Show resolved Hide resolved
modules/migrations/gitlab.go Outdated Show resolved Hide resolved
modules/migrations/gitlab.go Outdated Show resolved Hide resolved
@lunny

This comment has been minimized.

Copy link
Member

lunny commented Nov 20, 2019

I think we have to add a selection on UI for self-hosted git services.

@lunny

This comment has been minimized.

Copy link
Member

lunny commented Nov 20, 2019

For make the PR easier to merge I think maybe you could just support gitlab.com on this PR so that you could not change the UI and move self-host part for another PR.

@lunny lunny added the kind/feature label Nov 20, 2019
@lunny lunny added this to the 1.12.0 milestone Nov 20, 2019
@aqtrans

This comment has been minimized.

Copy link
Contributor Author

aqtrans commented Nov 20, 2019

For make the PR easier to merge I think maybe you could just support gitlab.com on this PR so that you could not change the UI and move self-host part for another PR.

Ah yeah that makes sense. I'll get on that.
For a UI, I was thinking maybe a "Migrating from [enabled services]?" dropdown could be added. But I haven't dug into what changes that'd require on the backend.

@aqtrans

This comment has been minimized.

Copy link
Contributor Author

aqtrans commented Nov 20, 2019

Alright. Hopefully I did that right. Self-hosted support has been ripped out of this PR, and a new PR has been opened to re-add self-hosted support.

@6543 6543 mentioned this pull request Nov 20, 2019
1 of 6 tasks complete
@mrsdizzie

This comment has been minimized.

Copy link
Contributor

mrsdizzie commented Nov 20, 2019

The fix in eaf52a4 should go it its own small bugfix PR imo and should get back ported (where as this PR can't be)

@aqtrans

This comment has been minimized.

Copy link
Contributor Author

aqtrans commented Nov 20, 2019

The fix in eaf52a4 should go it its own small bugfix PR imo and should get back ported (where as this PR can't be)

Good call. I thought about that but wasn't sure how to go about doing that haha...
Now that I'm getting the hang of git, I've gone ahead and done so. PR#9097

Copy link
Member

lunny left a comment

And could you create a test repo on gitlab.com, keep it readonly after you created some data and add a test like we did with github.

for k, asset := range rel.Assets.Links {
u, _ := url.Parse(asset.URL)
r.Assets = append(r.Assets, base.ReleaseAsset{
URL: u.String(),

This comment has been minimized.

Copy link
@lunny

lunny Nov 21, 2019

Member

Why not just asset.URL?

case "opened":
merged = false
case "closed":
merged = true

This comment has been minimized.

Copy link
@lunny

lunny Nov 21, 2019

Member

false?

aqtrans added 2 commits Nov 25, 2019
Opened PRs should fall through to false.
Allow importing using Personal Tokens or anonymous access.
@aqtrans

This comment has been minimized.

Copy link
Contributor Author

aqtrans commented Nov 27, 2019

And could you create a test repo on gitlab.com, keep it readonly after you created some data and add a test like we did with github.

I just got all the existing Github migration tests ported over, but the Gitlab API requires authentication for almost everything, so it can't run anonymously like the Github tests appear to do.

Are there any best practices for authentication in tests? Generate a deploy token?

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Nov 27, 2019

We should probably create test_repo in gitlab.com. I created https://gitlab.com/gitea-mirror/ (as gitea is taken), but we should probably create that repo under it like we have such in https://github.com/go-gitea/test_repo

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Nov 28, 2019

I moved everything to https://gitlab.com/gitea/ org now and also created test_repo with same structure as we have in github org: https://gitlab.com/gitea/test_repo

@aqtrans

This comment has been minimized.

Copy link
Contributor Author

aqtrans commented Dec 3, 2019

I moved everything to https://gitlab.com/gitea/ org now and also created test_repo with same structure as we have in github org: https://gitlab.com/gitea/test_repo

Perfect, thanks!

What about the authentication part? Per the Gitlab docs, and my own personal testing, public/unauthenticated access doesn't return much of anything for most API calls, even basic issue and PR calls.

@aqtrans aqtrans requested a review from lunny Dec 3, 2019
@lunny

This comment has been minimized.

Copy link
Member

lunny commented Dec 3, 2019

Does gitlab support readonly token? If that, we could create one.

@aqtrans

This comment has been minimized.

Copy link
Contributor Author

aqtrans commented Dec 3, 2019

Not that I was able to find. They have Deploy Tokens, but those only allows repo cloning, no API access.

"Personal Access Tokens" work, but those are tied to a specific Gitlab user.

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Dec 3, 2019

I will create giteabot user

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Dec 3, 2019

I have created user and added it to gitlab.com gitea org. Also created read token for it. that should be accessible from drone secrets by name gitlab_read_token

@guillep2k

This comment has been minimized.

Copy link
Member

guillep2k commented Dec 3, 2019

I have created user and added it to gitlab.com gitea org. Also created read token for it. that should be accessible from drone secrets by name gitlab_read_token

I think the test should be skipped if the token is not available. Otherwise, nobody would be able to run integration tests locally.

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Dec 4, 2019

You can add token here:
https://github.com/go-gitea/gitea/blob/master/.drone.yml#L107

    environment:
      GOPROXY: off
      TAGS: bindata sqlite sqlite_unlock_notify
      GITLAB_READ_TOKEN:
        from_secret: gitlab_read_token
@aqtrans

This comment has been minimized.

Copy link
Contributor Author

aqtrans commented Dec 4, 2019

Great, thanks all!

I've pushed those changes, and was able to successfully test locally, passing my own token in.

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Dec 4, 2019

Please resolve conflicts

aqtrans added 3 commits Dec 5, 2019
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 5, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@e80fe20). Click here to learn what that means.
The diff coverage is 66.49%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #9084   +/-   ##
=========================================
  Coverage          ?   41.17%           
=========================================
  Files             ?      557           
  Lines             ?    72929           
  Branches          ?        0           
=========================================
  Hits              ?    30027           
  Misses            ?    39129           
  Partials          ?     3773
Impacted Files Coverage Δ
modules/structs/repo.go 54.54% <ø> (ø)
modules/migrations/gitlab.go 66.49% <66.49%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e80fe20...52cfcde. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
8 participants
You can’t perform that action at this time.