Update all versions of the same private module in single terraform file

[szemek]

Hello,
I noticed that dependabot running for one our repositories had issues with updating one dependency.

updater | ERROR <job_468187453> No files changed!
updater | ERROR <job_468187453> /home/dependabot/terraform/lib/dependabot/terraform/file_updater.rb:44:in `updated_dependency_files'
updater | ERROR <job_468187453> /home/dependabot/dependabot-updater/lib/dependabot/updater.rb:746:in `generate_dependency_files_for'
updater | ERROR <job_468187453> /home/dependabot/dependabot-updater/lib/dependabot/updater.rb:311:in `check_and_create_pull_request'
updater | ERROR <job_468187453> /home/dependabot/dependabot-updater/lib/dependabot/updater.rb:109:in `check_and_create_pr_with_error_handling'
updater | ERROR <job_468187453> /home/dependabot/dependabot-updater/lib/dependabot/updater.rb:80:in `block in run'
updater | ERROR <job_468187453> /home/dependabot/dependabot-updater/lib/dependabot/updater.rb:80:in `each'
updater | ERROR <job_468187453> /home/dependabot/dependabot-updater/lib/dependabot/updater.rb:80:in `run'
updater | ERROR <job_468187453> /home/dependabot/dependabot-updater/lib/dependabot/update_files_job.rb:17:in `perform_job'
updater | ERROR <job_468187453> /home/dependabot/dependabot-updater/lib/dependabot/base_job.rb:50:in `run'
updater | ERROR <job_468187453> bin/update_files.rb:22:in `<main>'

After investigation I found out that when single terraform file uses the same private module multiple times, but with different versions, then only first occurrence is found and other are not updated.
This pull request is a bug fix for this situation.

[Nishnha]

Hi @szemek thanks for this contribution!

The symmetric difference check for requirement_changed? was surprising!

Subtracting two arrays does not do what I expected. For two arrays A and B, A-B will remove all instances of elements in array B from array A.

So for two arrays

A = [{:requirement=>"0.9.1", :groups=>[], :file=>"main.tf", :source=>{:type=>"registry", :registry_hostname=>"app.terraform.io", :module_identifier=>"example-org-5d3190/s3-webapp/aws"}},
   {:requirement=>"0.11.0", :groups=>[], :file=>"main.tf", :source=>{:type=>"registry", :registry_hostname=>"app.terraform.io", :module_identifier=>"example-org-5d3190/s3-webapp/aws"}}]

and

B = [{:requirement=>"0.11.0", :groups=>[], :file=>"main.tf", :source=>{:type=>"registry", :registry_hostname=>"app.terraform.io", :module_identifier=>"example-org-5d3190/s3-webapp/aws"}},
   {:requirement=>"0.11.0", :groups=>[], :file=>"main.tf", :source=>{:type=>"registry", :registry_hostname=>"app.terraform.io", :module_identifier=>"example-org-5d3190/s3-webapp/aws"}}]

subtracting the two from each other will give two different results:

A - B = []

while

B - A =
{:requirement=>"0.9.1", :groups=>[], :file=>"main.tf", :source=>{:type=>"registry", :registry_hostname=>"app.terraform.io", :module_identifier=>"example-org-5d3190/s3-webapp/aws"}}

I think this is a special case that only applies to Terraform registry dependencies with the same source and different versions. Most other ecosystems will force you to use a single version of a dependency.

Also, this may resolve #5128,
but #5150 is an issue with Terraform Git dependencies not having unique identifiers.

[szemek]

Hi @szemek thanks for this contribution!

[...]

I think this is a special case that only applies to Terraform registry dependencies with the same source and different versions. Most other ecosystems will force you to use a single version of a dependency.

Also, this may resolve #5128, but #5150 is an issue with Terraform Git dependencies not having unique identifiers.

Thanks @Nishnha for checking my changes!
Yes, I noticed that this behaviour is Terraform-specific. At the beginning I was trying to update requirement_changed? method in base class, but it broke some tests for other ecosystem, so I moved it to subclass.

When it comes to Git dependencies, I can see that there is a different method for that

dependabot-core/terraform/lib/dependabot/terraform/file_updater.rb

Lines 76 to 88 in 12bc6f1

	def update_git_declaration(new_req, old_req, updated_content, filename)
	url = old_req.fetch(:source)[:url].gsub(%r{^https://}, "")
	tag = old_req.fetch(:source)[:ref]
	url_regex = /#{Regexp.quote(url)}.*ref=#{Regexp.quote(tag)}/
	declaration_regex = git_declaration_regex(filename)
	updated_content.sub!(declaration_regex) do |regex_match|
	regex_match.sub(url_regex) do |url_match|
	url_match.sub(old_req[:source][:ref], new_req[:source][:ref])
	end
	end
	end

so I'll prepare a new pull request with tests.

My pull request is approved but my branch is out-of-date with the main branch. Should I merge/rebase my branch, or leave that to you?

[jeffwidman]

My pull request is approved but my branch is out-of-date with the main branch. Should I merge/rebase my branch, or leave that to you?

We can handle that since you've given maintainers edit permission on your PR branch. We generally follow a deploy-then-merge process, so whoever gets to it will handle rebasing right before deploying.

[Nishnha]

so I'll prepare a new pull request with tests.

Thanks!

When it comes to Git dependencies, I can see that there is a different method for that

Here's how we handle that Git dependency vs "native" dependency issue for NPM

https://github.com/dependabot/dependabot-core/blob/main/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser.rb#L41-L50