git-proxy icon indicating copy to clipboard operation
git-proxy copied to clipboard

fix: add gitlab support

Open msagi opened this issue 1 year ago • 6 comments

Fix #511 .

This is my first stab at adding multi host support to GitProxy. The main idea behind it is: not to lose the original git host during repointing the repo to GitProxy via git remote set-url command but keep the it in the repointed url as the first path segment, e.g.

$git remote -v
origin	[email protected]:finos/git-proxy.git (fetch)
origin	[email protected]:finos/git-proxy.git (push)

$git remote set-url origin https://localhost:8000/github.com/finos/git-proxy.git

$git remote -v
origin	http://localhost:8000/github.com/finos/git-proxy.git (fetch)
origin	http://localhost:8000/github.com/finos/git-proxy.git (push)

Since the original git host is preserved, specific routes can be configured (in proxy.config.json) connecting the first path segment to the original host, e.g.

  "proxyConfig": [
    {
      "path": "/github.com",
      "url": "https://github.com",
      "enabled": true
    },
    {
      "path": "/gitlab.com",
      "url": "https://gitlab.com",
      "enabled": true
    }
  ],

I've added GitHub and GitLab repos to the authorisedList and tested git push with both host, works fine.

GitHub.com is hardcoded in several places, and whilst the proxy now supports GitHub and GitLab too, perhaps we shall sit down and chat how we can evolve the UI component (e.g., GitHub account is hardcoded, etc.) forward.

Note: due to multi host support, the project/name is not unique ID any more. A repo can uniquely identified only by its URL, e.g. https://gitlab.com/finos/git-proxy.git

@abinash2512 @JamieSlome - let me know your thoughts

msagi avatar May 04 '24 23:05 msagi

Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name Link
Latest commit e56f8940679d0c5048e784ca31aae4712b967796
Latest deploy log https://app.netlify.com/sites/endearing-brigadeiros-63f9d0/deploys/6645da30848d6a0009a54104

netlify[bot] avatar May 04 '24 23:05 netlify[bot]

Codecov Report

Attention: Patch coverage is 56.91057% with 53 lines in your changes missing coverage. Please review.

Project coverage is 58.31%. Comparing base (fe59198) to head (e56f894). Report is 267 commits behind head on main.

Files Patch % Lines
src/proxy/routes/index.js 31.14% 42 Missing :warning:
.../processors/push-action/checkUserPushPermission.js 0.00% 5 Missing :warning:
src/db/file/pushes.js 0.00% 2 Missing :warning:
src/db/file/repo.js 80.00% 2 Missing :warning:
src/proxy/processors/pre-processor/parseAction.js 89.47% 2 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #545      +/-   ##
==========================================
+ Coverage   56.96%   58.31%   +1.35%     
==========================================
  Files          46       48       +2     
  Lines        1566     1605      +39     
==========================================
+ Hits          892      936      +44     
+ Misses        674      669       -5     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 04 '24 23:05 codecov[bot]

@msagi - since the wonderful @eddie-knight contributed automated releases, are you able to take a look at the latest status check failure? All PRs will not require a valid label prior to merge. This ensures that we can automate our semantic versioning based on the chosen and agreed titles of improvements, i.e. fix, feature, etc.

JamieSlome avatar May 10 '24 14:05 JamieSlome

@msagi - since the wonderful @eddie-knight contributed automated releases, are you able to take a look at the latest status check failure? All PRs will not require a valid label prior to merge. This ensures that we can automate our semantic versioning based on the chosen and agreed titles of improvements, i.e. fix, feature, etc.

fixed @JamieSlome

msagi avatar May 10 '24 19:05 msagi

hi @JamieSlome , would you like me to make any further changes?

msagi avatar May 15 '24 19:05 msagi

@msagi - getting some breaches in the UI (admin/push). We need to think about how this implementation will break existing databases already using previous models. How do we translate between the two and/or support both?

This is a fabulous PR and love the way you've implemented multiple endpoint support at the same point. This is very desirable. The updates to the UI should be fairly trivial as there only a few hyperlinks here and there that directly reference GitHub.com. Still need to do a more in-depth review of the PR but a great start for now!

JamieSlome avatar May 16 '24 10:05 JamieSlome