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

Fix updating GitHub actions to major tags that are branches #5963

Merged
Show file tree
Hide file tree
Changes from all 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
95 changes: 54 additions & 41 deletions common/lib/dependabot/git_commit_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ class GitCommitChecker

def initialize(dependency:, credentials:,
ignored_versions: [], raise_on_ignored: false,
requirement_class: nil, version_class: nil)
requirement_class: nil, version_class: nil,
consider_version_branches_pinned: false)
@dependency = dependency
@credentials = credentials
@ignored_versions = ignored_versions
@raise_on_ignored = raise_on_ignored
@requirement_class = requirement_class
@version_class = version_class
@consider_version_branches_pinned = consider_version_branches_pinned
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the approach of parameterising this rather than overriding it 🥇

end

def git_dependency?
Expand All @@ -52,17 +54,17 @@ def pinned?
# If the specified `ref` is actually a tag, we're pinned
return true if local_upload_pack.match?(%r{ refs/tags/#{ref}$})

# If the specified `ref` is actually a branch, we're NOT pinned
return false if local_upload_pack.match?(%r{ refs/heads/#{ref}$})
# Assume we're pinned unless the specified `ref` is actually a branch
return true unless local_upload_pack.match?(%r{ refs/heads/#{ref}$})

# Otherwise, assume we're pinned
true
# TODO: Research whether considering branches that look like versions pinned makes sense for all ecosystems
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

@consider_version_branches_pinned && version_tag?(ref)
end

def pinned_ref_looks_like_version?
return false unless pinned?

dependency_source_details.fetch(:ref).match?(VERSION_REGEX)
version_tag?(dependency_source_details.fetch(:ref))
end

def pinned_ref_looks_like_commit_sha?
Expand Down Expand Up @@ -101,47 +103,23 @@ def head_commit_for_local_branch(name)
end

def local_tag_for_latest_version_matching_existing_precision
max_tag = max_version_tag_for_current_precision(allowed_version_tags)

return unless max_tag

to_local_tag(max_tag)
max_local_tag_for_current_precision(allowed_version_tags)
end

def local_tag_for_latest_version
max_tag = max_version_tag(allowed_version_tags)

return unless max_tag

to_local_tag(max_tag)
def local_ref_for_latest_version_matching_existing_precision
max_local_tag_for_current_precision(allowed_version_refs)
end

def max_version_tag(tags)
tags.
max_by do |t|
version_from_tag(t)
end
end

def max_version_tag_for_current_precision(tags)
current_precision = precision(dependency.version)

# Find the latest version with the same precision as the pinned version.
max_version_tag(tags.select { |tag| precision(scan_version(tag.name)) == current_precision })
def local_tag_for_latest_version
max_local_tag(allowed_version_tags)
end

def allowed_version_tags
tags =
local_tags.
select { |t| version_tag?(t.name) && matches_existing_prefix?(t.name) }
filtered = tags.
reject { |t| tag_included_in_ignore_requirements?(t) }
if @raise_on_ignored && filter_lower_versions(filtered).empty? && filter_lower_versions(tags).any?
raise Dependabot::AllVersionsIgnored
end
allowed_versions(local_tags)
end

filtered.
reject { |t| tag_is_prerelease?(t) && !wants_prerelease? }
def allowed_version_refs
allowed_versions(local_refs)
end

def current_version
Expand Down Expand Up @@ -184,10 +162,37 @@ def git_repo_reachable?

attr_reader :dependency, :credentials, :ignored_versions

def max_local_tag_for_current_precision(tags)
current_precision = precision(dependency.version)

# Find the latest version with the same precision as the pinned version.
max_local_tag(tags.select { |tag| precision(scan_version(tag.name)) == current_precision })
end

def max_local_tag(tags)
max_version_tag = tags.max_by { |t| version_from_tag(t) }

to_local_tag(max_version_tag)
end

def precision(version)
version.split(".").length
end

def allowed_versions(local_tags)
tags =
local_tags.
select { |t| version_tag?(t.name) && matches_existing_prefix?(t.name) }
filtered = tags.
reject { |t| tag_included_in_ignore_requirements?(t) }
if @raise_on_ignored && filter_lower_versions(filtered).empty? && filter_lower_versions(tags).any?
raise Dependabot::AllVersionsIgnored
end

filtered.
reject { |t| tag_is_prerelease?(t) && !wants_prerelease? }
end

def pinned_ref_in_release?(version)
raise "Not a git dependency!" unless git_dependency?

Expand Down Expand Up @@ -226,9 +231,15 @@ def local_upload_pack
local_repo_git_metadata_fetcher.upload_pack
end

def local_refs
handle_tag_prefix(local_repo_git_metadata_fetcher.refs_for_upload_pack)
end

def local_tags
tags = local_repo_git_metadata_fetcher.tags
handle_tag_prefix(local_repo_git_metadata_fetcher.tags_for_upload_pack)
end

def handle_tag_prefix(tags)
if dependency_source_details&.fetch(:ref, nil)&.start_with?("tags/")
tags = tags.map do |tag|
tag.dup.tap { |t| t.name = "tags/#{tag.name}" }
Expand Down Expand Up @@ -332,12 +343,14 @@ def matches_existing_prefix?(tag)
end

def to_local_tag(tag)
return unless tag

version = version_from_tag(tag)
{
tag: tag.name,
version: version,
commit_sha: tag.commit_sha,
tag_sha: tag.tag_sha
tag_sha: tag.ref_sha
}
end

Expand Down
32 changes: 15 additions & 17 deletions common/lib/dependabot/git_metadata_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,21 @@ def upload_pack
def tags
return [] unless upload_pack

@tags ||= tags_for_upload_pack
@tags ||= tags_for_upload_pack.map do |ref|
OpenStruct.new(
name: ref.name,
tag_sha: ref.ref_sha,
commit_sha: ref.commit_sha
)
end
end

def tags_for_upload_pack
@tags_for_upload_pack ||= refs_for_upload_pack.select { |ref| ref.ref_type == :tag }
end

def refs_for_upload_pack
@refs_for_upload_pack ||= parse_refs_for_upload_pack
end

def ref_names
Expand Down Expand Up @@ -108,22 +122,6 @@ def fetch_raw_upload_pack_with_git_for(uri)
end
end

def tags_for_upload_pack
refs_for_upload_pack.
select { |ref| ref.ref_type == :tag }.
map do |ref|
OpenStruct.new(
name: ref.name,
tag_sha: ref.ref_sha,
commit_sha: ref.commit_sha
)
end
end

def refs_for_upload_pack
@refs_for_upload_pack ||= parse_refs_for_upload_pack
end

def parse_refs_for_upload_pack
peeled_lines = []

Expand Down
85 changes: 85 additions & 0 deletions common/spec/dependabot/git_commit_checker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,91 @@
end
end

describe "#local_ref_for_latest_version_matching_existing_precision" do
subject { checker.local_ref_for_latest_version_matching_existing_precision }
let(:repo_url) { "https://github.com/gocardless/business.git" }
let(:service_pack_url) { repo_url + "/info/refs?service=git-upload-pack" }
before do
stub_request(:get, service_pack_url).
to_return(
status: 200,
body: fixture("git", "upload_packs", upload_pack_fixture),
headers: {
"content-type" => "application/x-git-upload-pack-advertisement"
}
)
end

context "with no tags, nor version branches" do
let(:upload_pack_fixture) { "no_tags" }
it { is_expected.to be_nil }
end

context "with no version tags nor version branches" do
let(:upload_pack_fixture) { "no_versions" }
it { is_expected.to be_nil }
end

context "with version tags, and some version branches not matching pinned schema" do
let(:upload_pack_fixture) { "actions-checkout" }
let(:version) { "1.1.1" }

let(:source) do
{
type: "git",
url: "https://github.com/gocardless/business",
branch: "master",
ref: "v#{version}"
}
end

let(:latest_patch) do
{
commit_sha: "5a4ac9002d0be2fb38bd78e4b4dbde5606d7042f",
tag: "v2.3.4",
tag_sha: anything,
version: anything
}
end

it { is_expected.to match(latest_patch) }
end

context "with tags for minor versions and branches for major versions" do
let(:upload_pack_fixture) { "run-vcpkg" }

context "when pinned to a major" do
let(:version) { "7" }

let(:latest_major_branch) do
{
commit_sha: "831e6cd560cc8688a4967c5766e4215afbd196d9",
tag: "v10",
tag_sha: anything,
version: anything
}
end

it { is_expected.to match(latest_major_branch) }
end

context "when pinned to a minor" do
let(:version) { "7.0" }

let(:latest_minor_tag) do
{
commit_sha: "831e6cd560cc8688a4967c5766e4215afbd196d9",
tag: "v10.6",
tag_sha: anything,
version: anything
}
end

it { is_expected.to match(latest_minor_tag) }
end
end
end

describe "#local_tag_for_pinned_version" do
subject { checker.local_tag_for_pinned_version }

Expand Down
Binary file added common/spec/fixtures/git/upload_packs/run-vcpkg
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def latest_version_tag
@latest_version_tag ||= begin
return git_commit_checker.local_tag_for_latest_version if dependency.version.nil?

git_commit_checker.local_tag_for_latest_version_matching_existing_precision
git_commit_checker.local_ref_for_latest_version_matching_existing_precision
end
end

Expand Down Expand Up @@ -176,7 +176,8 @@ def git_commit_checker
dependency: dependency,
credentials: credentials,
ignored_versions: ignored_versions,
raise_on_ignored: raise_on_ignored
raise_on_ignored: raise_on_ignored,
consider_version_branches_pinned: true
)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,15 @@
end
end

context "given a dependency that uses branches to track major releases" do
let(:upload_pack_fixture) { "run-vcpkg" }

context "using the major version" do
let(:reference) { "v7" }
it { is_expected.to eq(Dependabot::GithubActions::Version.new("10")) }
end
end

context "given a dependency with a tag reference and a branch similar to the tag" do
let(:upload_pack_fixture) { "download-artifact" }
let(:reference) { "v2" }
Expand Down
Binary file not shown.