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

🐛 Support self-hosted GitLab instances where base URL has a path component #3819

Merged
merged 15 commits into from
Jan 31, 2024

Conversation

spencerschrock
Copy link
Contributor

What kind of change does this PR introduce?

bug fix

What is the current behavior?

Assumes the gitlab API base url doesn't have any part after the domain.

What is the new behavior (if this is a feature change)?**

Adds a new environment variable, GL_HOST to handle these situations.
The env var is needed, otherwise there's ambiguity over what part of the URL is part of the API url, the repo owner, or project.

  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Fixes #3696

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

Fixed a bug which prevented Scorecard from analyzing some self-hosted GitLab repos.

Self-hosted instances which dont use a subdomain result in broken API links.
This change may not be finished, but is intended to evaluate the solution.

Previously, self hosted instances where the instance is part of the path (foo.com/gitlab/owner/repo)
would have their API base URL registered as foo.com/api/v4/ instead of foo.com/gitlab/api/v4/

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
now that repoURL_parse handles GL_HOST, we dont need it elsewhere.

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Merging #3819 (693712c) into main (83ff808) will decrease coverage by 4.80%.
The diff coverage is 66.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3819      +/-   ##
==========================================
- Coverage   75.54%   70.75%   -4.80%     
==========================================
  Files         232      232              
  Lines       15772    15784      +12     
==========================================
- Hits        11915    11168     -747     
- Misses       3121     3930     +809     
+ Partials      736      686      -50     

@spencerschrock
Copy link
Contributor Author

spencerschrock commented Jan 25, 2024

@mwager mind testing this? And seeing if the README instructions are clear enough?

@mwager
Copy link

mwager commented Jan 26, 2024

Confirming that this works and scorecard successfully displays the results:

GL_HOST=https://foo.com/gitlab/ go run main.go --repo https://foo.com/gitlab/org/somerepo

README instructions looking good to me!

@spencerschrock spencerschrock marked this pull request as ready for review January 26, 2024 17:22
@spencerschrock spencerschrock requested a review from a team as a code owner January 26, 2024 17:22
@spencerschrock spencerschrock requested review from raghavkaul and laurentsimon and removed request for a team January 26, 2024 17:22
Copy link
Member

@naveensrinivasan naveensrinivasan left a comment

Choose a reason for hiding this comment

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

Thanks!

Signed-off-by: Spencer Schrock <sschrock@google.com>
if we can avoid an API call, do it.

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
@mwager
Copy link

mwager commented Jan 30, 2024

I tested on the latest commit:

grafik

Works like before.

Doing the curl command without the auth header returns 200 with json data...

grafik

Signed-off-by: Spencer Schrock <sschrock@google.com>
the simpler the better

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
@spencerschrock
Copy link
Contributor Author

Doing the curl command without the auth header returns 200 with json data...

I've reverted one part to keep the PR smaller. if this causes it to fail for you, i will revert the reversion and merge the current state.

@mwager
Copy link

mwager commented Jan 31, 2024

Tested again on commit 693712c, everything fine :)

@spencerschrock spencerschrock merged commit e10dbb1 into ossf:main Jan 31, 2024
38 checks passed
@spencerschrock spencerschrock deleted the fix/gitlab-host-path branch January 31, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot scan self-hosted (private) GitLab repositories
3 participants