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.

Sign up for GitHub

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(terraform): update less-than/less-than/equals version constraints #8983

Merged
merged 2 commits into from
Dec 16, 2024
Merged

fix(terraform): update less-than/less-than/equals version constraints #8983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6fee66c
Terraform tests added for less than/equals, '<=', constraint and less…
bryan-bar Oct 17, 2024
f682f15
Terraform requirements updater - update update_greatest_version to ha…
bryan-bar Oct 17, 2024
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
21 changes: 12 additions & 9 deletions terraform/lib/dependabot/terraform/requirements_updater.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,17 +190,20 @@ def update_greatest_version(requirement, version_to_be_permitted)
op, version = requirement.requirements.first
version = version.release if version.prerelease?

index_to_update =
version.segments.map.with_index { |seg, i| seg.zero? ? 0 : i }.max

new_segments = version.segments.map.with_index do |_, index|
if index < index_to_update
# When 'less than'/'<',
# increment the last available segment only so that the new version is within the constraint
if op == "<"
new_segments = version.segments.map.with_index do |_, index|
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't fully understand why the old code was not sufficient. Does the old code fail the new tests?

The new code is simpler and seems to do the trick.

Copy link
Contributor Author

@bryan-bar bryan-bar Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, when removing b946d23 the test that fails in relation to the less-than/< operator is this test since it jumps a minor version to 0.4.0 instead of incrementing the patch version when a patch is set to 0, ">= 0.2.1, < 0.3.0, <= 0.3.0":

        context "when not satisfied, 0 patch version" do
          let(:requirement) { ">= 0.2.1, < 0.3.0, <= 0.3.0" }
          let(:latest_version) { "0.3.7" }

          its([:requirement]) { is_expected.to eq(">= 0.2.1, < 0.3.8, <= 0.3.7") }
        end

Error:

  17) Dependabot::Terraform::RequirementsUpdater#updated_requirements when there is a latest version when a =>,<,<= requirement was previously specified when not satisfied, 0 patch version [:requirement] is expected to eq ">= 0.2.1, < 0.3.8, <= 0.3.7"
      Failure/Error: its([:requirement]) { is_expected.to eq(">= 0.2.1, < 0.3.8, <= 0.3.7") }
      
        expected: ">= 0.2.1, < 0.3.8, <= 0.3.7"
             got: ">= 0.2.1, < 0.4.0, <= 0.4.0"
      
        (compared using ==)
      # ./spec/dependabot/terraform/requirements_updater_spec.rb:134:in `block (6 levels) in <top (required)>'
      # /home/dependabot/common/spec/spec_helper.rb:66:in `block (2 levels) in <top (required)>'
      # /home/dependabot/dependabot-updater/vendor/ruby/3.3.0/gems/webmock-3.23.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'

This revealed a second issue where a non-zero patch would be incremented as expected, ">= 0.2.1, < 0.3.2, <= 0.3.2", besides the original <= issue, unlike the zero-patch version:

        context "when not satisfied, non-0 patch version" do
          let(:requirement) { ">= 0.2.1, < 0.3.2, <= 0.3.2" }
          let(:latest_version) { "0.3.7" }

          its([:requirement]) { is_expected.to eq(">= 0.2.1, < 0.3.8, <= 0.3.7") }
        end

Error:

  16) Dependabot::Terraform::RequirementsUpdater#updated_requirements when there is a latest version when a =>,<,<= requirement was previously specified when not satisfied, non-0 patch version [:requirement] is expected to eq ">= 0.2.1, < 0.3.8, <= 0.3.7"
      Failure/Error: its([:requirement]) { is_expected.to eq(">= 0.2.1, < 0.3.8, <= 0.3.7") }
      
        expected: ">= 0.2.1, < 0.3.8, <= 0.3.7"
             got: ">= 0.2.1, < 0.3.8, <= 0.3.8"
      
        (compared using ==)
      # ./spec/dependabot/terraform/requirements_updater_spec.rb:141:in `block (6 levels) in <top (required)>'
      # /home/dependabot/common/spec/spec_helper.rb:66:in `block (2 levels) in <top (required)>'
      # /home/dependabot/dependabot-updater/vendor/ruby/3.3.0/gems/webmock-3.23.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'

version_to_be_permitted.segments[index]
elsif index == index_to_update
version_to_be_permitted.segments[index].to_i + 1
else
0
end
new_segments[-1] += 1
# When 'less-than/equal'/'<=', use the new version as-is even when previously set as a non-semver version
# Terraform treats shortened versions the same as a version with any remaining segments as 0
# Example: '0.2' is treated as '0.2.0' | '1' is treated as '1.0.0'
elsif op == "<="
new_segments = version_to_be_permitted.segments
else
raise "Unexpected operation: #{op}"
end

requirement_class.new("#{op} #{new_segments.join('.')}")
Expand Down
71 changes: 67 additions & 4 deletions terraform/spec/dependabot/terraform/requirements_updater_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,82 @@
end
end

context "when a =>,< requirement was previously specified" do
context "when <= requirement was previously specified" do
context "when it is satisfied" do
let(:requirement) { "<= 0.3.7" }

it { is_expected.to eq(requirements.first) }
end

context "when it is not satisfied" do
let(:requirement) { "<= 0.1.9" }

its([:requirement]) { is_expected.to eq("<= 0.3.7") }

context "when specifying two version segments" do
let(:requirement) { "<= 0.3" }
let(:latest_version) { version_class.new("2.8.5") }

its([:requirement]) { is_expected.to eq("<= 2.8.5") }
end

context "when specifying three version segments" do
let(:requirement) { "<= 0.3.7" }
let(:latest_version) { version_class.new("2.8.5") }

its([:requirement]) { is_expected.to eq("<= 2.8.5") }
end

context "when minor and patch updated" do
let(:requirement) { "<= 0.3.7" }
let(:latest_version) { version_class.new("0.4.0") }

its([:requirement]) { is_expected.to eq("<= 0.4.0") }
end

context "when major, minor and patch updated" do
let(:requirement) { "<= 0.3.7" }
let(:latest_version) { version_class.new("1.4.0") }

its([:requirement]) { is_expected.to eq("<= 1.4.0") }
end
end
end

context "when a =>,<,<= requirement was previously specified" do
context "when satisfied" do
let(:requirement) { ">= 0.2.1, < 0.4.0" }
let(:latest_version) { "0.3.7" }

its([:requirement]) { is_expected.to eq(">= 0.2.1, < 0.4.0") }
end

context "when not satisfied" do
let(:requirement) { ">= 0.2.1, < 0.3.0" }
context "when not satisfied, 0 patch version" do
let(:requirement) { ">= 0.2.1, < 0.3.0, <= 0.3.0" }
let(:latest_version) { "0.3.7" }

its([:requirement]) { is_expected.to eq(">= 0.2.1, < 0.4.0") }
its([:requirement]) { is_expected.to eq(">= 0.2.1, < 0.3.8, <= 0.3.7") }
end

context "when not satisfied, non-0 patch version" do
let(:requirement) { ">= 0.2.1, < 0.3.2, <= 0.3.2" }
let(:latest_version) { "0.3.7" }

its([:requirement]) { is_expected.to eq(">= 0.2.1, < 0.3.8, <= 0.3.7") }
end

context "when not satisfied, major and minor only" do
let(:requirement) { ">= 0.2.1, < 0.3, <= 0.3" }
let(:latest_version) { "0.3.7" }

its([:requirement]) { is_expected.to eq(">= 0.2.1, < 0.4, <= 0.3.7") }
end

context "when not satisfied, major and minor only" do
let(:requirement) { ">= 0.2.1, < 0.3, <= 0.3" }
let(:latest_version) { "1.4.0" }

its([:requirement]) { is_expected.to eq(">= 0.2.1, < 1.5, <= 1.4.0") }
end
end
end
Expand Down
Loading