Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

feat: adds possibility to override host for API #598

Merged
merged 16 commits into from Feb 2, 2021
Merged

feat: adds possibility to override host for API #598

merged 16 commits into from Feb 2, 2021

Conversation

lkwg82
Copy link
Contributor

@lkwg82 lkwg82 commented Jan 30, 2021

Description
This PR adds the possibility to add an api_host entry in the config to enable overriding the default repository host.

In my company we split the api frontend from the ssh backend in the dns. So the api tries to call the ssh git backend.

Related Issue

I'm not sure, if I missed one.

How Has This Been Tested?
I debugged and started with non-working to working code.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation
  • Chore (Related to CI or Packaging to platforms)

@codecov
Copy link

codecov bot commented Jan 30, 2021

Codecov Report

Merging #598 (544b5e9) into trunk (a3fda2e) will increase coverage by 14.08%.
The diff coverage is 60.94%.

Impacted file tree graph

@@             Coverage Diff             @@
##            trunk     #598       +/-   ##
===========================================
+ Coverage   46.62%   60.71%   +14.08%     
===========================================
  Files         116       90       -26     
  Lines        6224     6325      +101     
===========================================
+ Hits         2902     3840      +938     
+ Misses       2998     2124      -874     
- Partials      324      361       +37     
Impacted Files Coverage Δ
commands/alias/expand/alias_expand.go 39.21% <0.00%> (ø)
commands/alias/list/alias_list.go 80.00% <ø> (ø)
commands/cmdutils/factory.go 0.00% <0.00%> (-69.24%) ⬇️
commands/cmdutils/repo_override.go 0.00% <0.00%> (-66.67%) ⬇️
commands/config/config.go 37.93% <0.00%> (-23.81%) ⬇️
commands/project/archive/repo_archive.go 0.00% <0.00%> (-26.03%) ⬇️
commands/project/create/project_create.go 0.00% <0.00%> (-11.28%) ⬇️
commands/variable/set/set.go 82.79% <ø> (ø)
commands/variable/variable.go 100.00% <ø> (ø)
commands/version/version.go 100.00% <ø> (ø)
... and 121 more

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 88dfe9c...b5ea30e. Read the comment docs.

Copy link
Owner

@profclems profclems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! In the future, it would be very useful to link to an issue that this is solving. We generally do not accept PRs that are not linked to any particular issue.

Would this be addressing #350?

Copy link
Owner

@profclems profclems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be solving #350

I'm really excited about this. The current changes look good. You can proceed to add the necessary documentation.

The api_host becomes directly attached to the authenticated host

hosts:
    git.name.devops.com:
          api_host: gitlab.name.devops.com
          git_protocol: ssh
          token: xxxxx
...

Note that cfg.Get(repoHost, "api_host") gets the value in this order:

ENV VARS -> config(hosts) -> config(global or non-host) 

@profclems
Copy link
Owner

Also, we may have to add api_host to our default config mapping for the hosts:

Kind: yaml.MappingNode,
Content: []*yaml.Node{
{
Kind: yaml.ScalarNode,
Value: "gitlab.com",
},
{
Kind: yaml.MappingNode,
Content: []*yaml.Node{
{
HeadComment: "What protocol to use to access the api endpoint. Supported values: http, https",
Kind: yaml.ScalarNode,
Value: "api_protocol",
},
{
Kind: yaml.ScalarNode,
Value: "https",
},
{
HeadComment: "Your GitLab access token. Get an access token at https://gitlab.com/profile/personal_access_tokens",
Kind: yaml.ScalarNode,
Value: "token",
},
{
Kind: yaml.ScalarNode,
Value: "",
},
},
},
},

@pull-request-size pull-request-size bot added size/M and removed size/S labels Jan 30, 2021
@lkwg82
Copy link
Contributor Author

lkwg82 commented Jan 30, 2021

@profclems I need help with the tests, I dont understand the relation.

lkwg82 and others added 3 commits January 30, 2021 22:36
Co-authored-by: Clement Sam <clementsam75@gmail.com>
@lkwg82 lkwg82 requested a review from profclems January 30, 2021 21:46
@lkwg82 lkwg82 marked this pull request as draft January 30, 2021 21:46
@lkwg82 lkwg82 marked this pull request as ready for review January 30, 2021 21:47
@lkwg82
Copy link
Contributor Author

lkwg82 commented Jan 30, 2021

Afaik everything is resolved, did I miss sth?

@lkwg82 lkwg82 changed the title RFC: adds possibility to override host for api Draft: adds possibility to override host for api Jan 30, 2021
@lkwg82 lkwg82 marked this pull request as draft January 30, 2021 21:50
@lkwg82 lkwg82 marked this pull request as ready for review January 30, 2021 21:50
Copy link
Owner

@profclems profclems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there @lkwg82!

Just a few things to improve upon

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@profclems
Copy link
Owner

I will open a follow-up PR with changes to glab auth login so that the user is prompted to set the api_host

Lars Gohlke and others added 2 commits February 1, 2021 15:14
Co-authored-by: Clement Sam <clementsam75@gmail.com>
Copy link
Owner

@profclems profclems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @lkwg82... I really appreciate your effort to help make glab better

One last thing before I merge this PR

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
@profclems profclems added enhancement New feature or request core labels Feb 1, 2021
@profclems profclems added this to In progress 📈 in GLab: A GitLab CLI Tool via automation Feb 1, 2021
@profclems profclems changed the title Draft: adds possibility to override host for api feat: adds possibility to override host for API Feb 1, 2021
Co-authored-by: Clement Sam <clementsam75@gmail.com>
@lkwg82 lkwg82 requested a review from profclems February 2, 2021 09:34
Copy link
Owner

@profclems profclems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking in this one @lkwg82

GLab: A GitLab CLI Tool automation moved this from In progress 📈 to Needs to be merged 🎉 Feb 2, 2021
@profclems profclems merged commit b2daa17 into profclems:trunk Feb 2, 2021
GLab: A GitLab CLI Tool automation moved this from Needs to be merged 🎉 to Pending Release 🗞️ Feb 2, 2021
@clemsbot clemsbot moved this from Pending Release 🗞️ to Done 🤝 in GLab: A GitLab CLI Tool Feb 15, 2021
@profclems profclems linked an issue Mar 10, 2021 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core enhancement New feature or request size/M
Projects
Development

Successfully merging this pull request may close these issues.

Cannot get glab working on private instance with different endpoints
2 participants