Fix url fetcher when default git profile branch is not master #5638
Conversation
Signed-off-by: Nikita Mathur <nikita.mathur@chef.io>
|
✔️ Deploy Preview for chef-inspec ready! 🔨 Explore the source changes: ca19914 🔍 Inspect the deploy log: https://app.netlify.com/sites/chef-inspec/deploys/612ddd73506d610008b555dc 😎 Browse the preview: https://deploy-preview-5638--chef-inspec.netlify.app |
lib/inspec/fetcher/url.rb
Outdated
| "https://github.com/#{m[:user]}/#{m[:repo]}/archive/#{m[:commit]}.tar.gz" | ||
| elsif m = BITBUCKET_URL_REGEX.match(target) # rubocop:disable Lint/AssignmentInCondition | ||
| "https://bitbucket.org/#{m[:user]}/#{m[:repo]}/get/master.tar.gz" | ||
| default_branch = default_ref(m) |
There was a problem hiding this comment.
default_ref assumes a github address, but here it is being used on a bitbucket repo - this will fail for bitbucket repos.
lib/inspec/fetcher/url.rb
Outdated
|
|
||
| class << self | ||
| def default_ref(match_data) | ||
| remote_url = "https://github.com/#{match_data[:user]}/#{match_data[:repo]}.git" |
There was a problem hiding this comment.
Here is where default ref assumes a github address.
There was a problem hiding this comment.
could we just make the whole URI part of the variable.. what if it is a git on gitlab ... like gitlab.mitre.org?
There was a problem hiding this comment.
we have the whole uri of the repo when we put it in the cli command right?
There was a problem hiding this comment.
and we put the whole uri in the inpsec.yml - basically every time we reference a git repo we do so via a full uri right?
There was a problem hiding this comment.
The existing code was applying transformations to fetch tarballs, if possible, by recognizing github and bitbucket URLs. We can't do that in a generic basis, as we don't know what transformation may need to be applied. There may be a transformation that works for gitlab, but the user can always just supply the correct tarball URL in the first place.
As this is a hotfix PR, to fix an urgent problem, we will not be expanding any functionality on this PR. If you'd like to explore expanding the transformation logic, feel free to do so, but we need to ship this fix.
There was a problem hiding this comment.
Ok. I will open an issue, as I know we have customers using InSpec with other git platforms - GitLab mostly - which we should allow for. In theory, I would have guessed that given the user has to give the full git repo URI that we could use that.
Signed-off-by: Nikita Mathur <nikita.mathur@chef.io>
|
SonarCloud Quality Gate failed.
|
|
So this will work reguardless of the default branch name? |
clintoncwolfe
left a comment
clintoncwolfe
left a comment
There was a problem hiding this comment.
Thanks @Nik08, looks good to me.
Signed-off-by: Nikita Mathur nikita.mathur@chef.io
Description
Fix url fetcher when default git profile branch is not master
Needs strict code review on this
Related Issue
Fixes #5635
Types of changes
Checklist: