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

Go: remove git-type sources since pseudo-versions are comparable #6723

Merged
merged 3 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 2 additions & 60 deletions go_modules/lib/dependabot/go_modules/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
module Dependabot
module GoModules
class FileParser < Dependabot::FileParsers::Base
GIT_VERSION_REGEX = /^v\d+\.\d+\.\d+-.*-(?<sha>[0-9a-f]{12})$/

def parse
dependency_set = Dependabot::FileParsers::Base::DependencySet.new

Expand All @@ -35,16 +33,11 @@ def check_required_files
end

def dependency_from_details(details)
source =
if rev_identifier?(details) then git_source(details)
else
{ type: "default", source: details["Path"] }
end

source = { type: "default", source: details["Path"] }
version = details["Version"]&.sub(/^v?/, "")

reqs = [{
requirement: rev_identifier?(details) ? nil : details["Version"],
requirement: details["Version"],
file: go_mod.name,
source: source,
groups: []
Expand Down Expand Up @@ -115,45 +108,6 @@ def handle_parser_error(path, stderr)
raise Dependabot::DependencyFileNotParseable.new(go_mod.path, msg)
end

def rev_identifier?(dep)
dep["Version"]&.match?(GIT_VERSION_REGEX)
end

def git_source(dep)
url = PathConverter.git_url_for_path(dep["Path"])

# Currently, we have no way of knowing whether the commit tagged
# is being used because a branch is being followed or because a
# particular ref is in use. We *assume* that a particular ref is in
# use (which means we'll only propose updates when its included in
# a release)
{
type: "git",
url: url || dep["Path"],
ref: git_revision(dep),
branch: nil
}
rescue Dependabot::SharedHelpers::HelperSubprocessFailed => e
if e.message == "Cannot detect VCS"
# if the dependency is locally replaced, this is not a fatal error
return { type: "default", source: dep["Path"] } if dependency_has_local_replacement(dep)

msg = e.message + " for #{dep['Path']}. Attempted to detect VCS " \
"because the version looks like a git revision: " \
"#{dep['Version']}"
raise Dependabot::DependencyFileNotResolvable, msg
end

raise
end

def git_revision(dep)
raw_version = dep.fetch("Version")
return raw_version unless raw_version.match?(GIT_VERSION_REGEX)

raw_version.match(GIT_VERSION_REGEX).named_captures.fetch("sha")
end

def skip_dependency?(dep)
# Updating replaced dependencies is not supported
return true if dependency_is_replaced(dep)
Expand Down Expand Up @@ -181,18 +135,6 @@ def dependency_is_replaced(details)
end
false
end

def dependency_has_local_replacement(details)
if manifest["Replace"]
has_local_replacement = manifest["Replace"].find do |replace|
replace["New"]["Path"].start_with?("./", "../") &&
replace["Old"]["Path"] == details["Path"]
end

return true if has_local_replacement
end
false
end
end
end
end
Expand Down
39 changes: 1 addition & 38 deletions go_modules/lib/dependabot/go_modules/metadata_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,46 +10,9 @@ class MetadataFinder < Dependabot::MetadataFinders::Base
private

def look_up_source
return look_up_git_dependency_source if git_dependency?

path_str = (specified_source_string || dependency.name)
url = Dependabot::GoModules::PathConverter.
git_url_for_path_without_go_helper(path_str)
url = Dependabot::GoModules::PathConverter.git_url_for_path(dependency.name)
Source.from_url(url) if url
end

def git_dependency?
return false unless declared_source_details

dependency_type =
declared_source_details.fetch(:type, nil) ||
declared_source_details.fetch("type")

dependency_type == "git"
end

def look_up_git_dependency_source
specified_url =
declared_source_details.fetch(:url, nil) ||
declared_source_details.fetch("url")

Source.from_url(specified_url)
end

def specified_source_string
declared_source_details&.fetch(:source, nil) ||
declared_source_details&.fetch("source", nil)
end

def declared_source_details
sources = dependency.requirements.
map { |r| r.fetch(:source) }.
uniq.compact

raise "Multiple sources! #{sources.join(', ')}" if sources.count > 1

sources.first
end
end
end
end
Expand Down
50 changes: 0 additions & 50 deletions go_modules/lib/dependabot/go_modules/path_converter.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
# frozen_string_literal: true

require "excon"
require "nokogiri"

require "dependabot/registry_client"
require "dependabot/source"
require "dependabot/go_modules/native_helpers"

module Dependabot
Expand All @@ -20,51 +15,6 @@ def self.git_url_for_path(path)
args: { import: import_path }
)
end

# rubocop:disable Metrics/PerceivedComplexity
# Used in dependabot-backend, which doesn't have access to any Go
# helpers.
# TODO: remove the need for this.
def self.git_url_for_path_without_go_helper(path)
# Save a query by manually converting golang.org/x names
tmp_path = path.gsub(%r{^golang\.org/x}, "github.com/golang")

# Currently, Dependabot::Source.new will return `nil` if it can't
# find a git SCH associated with a path. If it is ever extended to
# handle non-git sources we'll need to add an additional check here.
return Source.from_url(tmp_path).url if Source.from_url(tmp_path)
return "https://#{tmp_path}" if tmp_path.end_with?(".git")
return unless (metadata_response = fetch_path_metadata(path))

# Look for a GitHub, Bitbucket or GitLab URL in the response
metadata_response.scan(Dependabot::Source::SOURCE_REGEX) do
source_url = Regexp.last_match.to_s
return Source.from_url(source_url).url
end

# If none are found, parse the response and return the go-import path
doc = Nokogiri::XML(metadata_response)
doc.remove_namespaces!
import_details =
doc.xpath("//meta").
find { |n| n.attributes["name"]&.value == "go-import" }&.
attributes&.fetch("content")&.value&.split(/\s+/)
return unless import_details && import_details[1] == "git"

import_details[2]
end
# rubocop:enable Metrics/PerceivedComplexity

def self.fetch_path_metadata(path)
# TODO: This is not robust! Instead, we should shell out to Go and
# use https://github.com/Masterminds/vcs.
response = Dependabot::RegistryClient.get(url: "https://#{path}?go-get=1")

return unless response.status == 200

response.body
end
private_class_method :fetch_path_metadata
end
end
end
34 changes: 2 additions & 32 deletions go_modules/spec/dependabot/go_modules/file_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,10 @@
to eq("0.0.0-20180617042118-027cca12c2d6")
expect(dependency.requirements).to eq(
[{
requirement: nil,
file: "go.mod",
groups: [],
source: {
type: "git",
url: "https://github.com/golang/crypto",
ref: "027cca12c2d6",
branch: nil
}
requirement: "v0.0.0-20180617042118-027cca12c2d6",
source: { source: "golang.org/x/crypto", type: "default" }
}]
)
end
Expand Down Expand Up @@ -309,31 +304,6 @@
its(:length) { is_expected.to eq(0) }
end

context "that is not resolvable" do
let(:go_mod_content) do
fixture("projects", "unknown_vcs", "go.mod")
end

it "raises the correct error" do
expect { parser.parse }.
to raise_error do |err|
expect(err).to be_a(Dependabot::DependencyFileNotResolvable)
expect(err.message).
to start_with("Cannot detect VCS for unknown.doesnotexist/vcs")
end
end
end

context "that is not resolvable, but is locally replaced" do
let(:go_mod_content) do
fixture("projects", "unknown_vcs_local_replacement", "go.mod")
end

it "does not raise an error" do
expect { parser.parse }.not_to raise_error
end
end

context "a monorepo" do
let(:project_name) { "monorepo" }
let(:repo_contents_path) { build_tmp_repo(project_name) }
Expand Down
25 changes: 2 additions & 23 deletions go_modules/spec/dependabot/go_modules/metadata_finder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
end
let(:requirements) do
[{
file: "Gopkg.toml",
file: "go.mod",
requirement: "v2.1.0",
groups: [],
source: source
Expand All @@ -38,14 +38,6 @@
let(:dependency_name) { "github.com/satori/go.uuid" }
let(:source) { nil }

before do
stub_request(:get, "https://example.com/status").to_return(
status: 200,
body: "Not GHES",
headers: {}
)
end

describe "#source_url" do
subject(:source_url) { finder.source_url }

Expand All @@ -68,20 +60,7 @@
}
end

it { is_expected.to eq("https://github.com/alias/go.uuid") }
end

context "with git requirements" do
let(:source) do
{
type: "git",
url: "https://github.com/alias/go.uuid",
branch: "master",
ref: nil
}
end

it { is_expected.to eq("https://github.com/alias/go.uuid") }
it { is_expected.to eq("https://github.com/satori/go.uuid") }
end
end
end
43 changes: 0 additions & 43 deletions go_modules/spec/dependabot/go_modules/path_converter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,47 +44,4 @@
it { is_expected.to eq("https://github.com/drewolson/testflight") }
end
end

describe ".git_url_for_path_without_go_helper" do
subject { described_class.git_url_for_path_without_go_helper(path) }

let(:path) { "gopkg.in/guregu/null.v3" }

context "with a path that is immediately recognisable as a git source" do
let(:path) { "github.com/drewolson/testflight" }
it { is_expected.to eq("https://github.com/drewolson/testflight") }
end

context "with a golang.org path" do
let(:path) { "golang.org/x/tools" }
it { is_expected.to eq("https://github.com/golang/tools") }
end

context "with a path that ends in .git" do
let(:path) { "git.fd.io/govpp.git" }
it { is_expected.to eq("https://git.fd.io/govpp.git") }
end

context "with a vanity URL that needs to be fetched" do
let(:path) { "k8s.io/apimachinery" }

before do
stub_request(:get, "https://k8s.io/apimachinery?go-get=1").
to_return(status: 200, body: vanity_response)
end
let(:vanity_response) do
fixture("repo_responses", "k8s_io_apimachinery.html")
end

it { is_expected.to eq("https://github.com/kubernetes/apimachinery") }

context "and returns a git source hosted with an unknown provider" do
let(:vanity_response) do
fixture("go", "repo_responses", "unknown_git_source.html")

it { is_expected.to eq("https://sf.com/kubernetes/apimachinery") }
end
end
end
end
end
7 changes: 0 additions & 7 deletions go_modules/spec/fixtures/projects/unknown_vcs/go.mod

This file was deleted.

8 changes: 0 additions & 8 deletions go_modules/spec/fixtures/projects/unknown_vcs/main.go

This file was deleted.

This file was deleted.

This file was deleted.