Add Terraform JSON support

[melendezd]

Fix #4080.

Adding support for updating Terraform JSON files with .tf.json file extension. Most of the tests present for .tf files are duplicated for .tf.json files (with some restructuring of file_updater_spec.rb to make this easier and less repetitious) since the functionality should be essentially the same for the two formats.

Currently, the modified file updater pulls the JSON into an object, modifies the object, and JSON.pretty_generate's it out to the file. This may change the formatting of the original JSON file (tab width, for example). I'm not sure if this is okay or if there is a better approach to avoid this issue.

Additionally, I noticed that a couple tests file_updater_spec.rb check the content of the original file rather than the output of the file updater, creating a test that will pass no matter what the updater is doing. I'm not sure if I'm misunderstanding but I feel it's worth mentioning. Here's a snippet from main:

dependabot-core/terraform/spec/dependabot/terraform/file_updater_spec.rb

Lines 810 to 830 in ca9f236

	it "does not update requirements in the `versions.tf` file" do
	updated_file = files.find { |file| file.name == "versions.tf" }
	expect(updated_file.content).to include(
	<<~DEP
	terraform {
	required_providers {
	random = {
	source = "hashicorp/random"
	version = "3.0.0"
	}
	aws = {
	source = "hashicorp/aws"
	version = ">= 3.37.0, < 3.46.0"
	}
	}
	}
	DEP
	)
	end

and the corresponding snippet from my PR:
https://github.com/yoyomeng2/dependabot-core/blob/f6258ad94f53360c2b39920ff6303bcfcfb03094/terraform/spec/dependabot/terraform/file_updater_spec.rb#L887-L894

Finally, I'm unsure if there's more verification that needs to be done other than the unit tests before being able to merge.

Any help or feedback is greatly appreciated. Thanks!

[pavera]

Thanks for the PR @melendezd! I look forward to working through this PR with you.

[pavera]

In general, I think the code would be cleaner if we eliminated the distinction between tf and json files by removing this method and adding f.name.end_with?(".tf", ".tf.json") on line 43. Just include the json files in the terraform_files list. I'm not sure if you attempted this approach and hit more complication that was difficult to overcome?

[pavera]

The goal of the removal of the distinction between tf and json files would be to eliminate the branches here and in registry declaration, we would need to pass the filename into update_registry_declaration to handle the file appropriately, but in general pushing these conditionals down feels better to me.

[pavera]

As you mentioned in your PR comment this may reformat the json which I think we'd prefer to avoid to limit noisy PRs at the very least. I'm not a regex expert, but I think we should take a shot at building a regex to match/sub for json. This would be the final piece to prevent branching as we could just handle this new regex the same as we currently do for terragrunt/terraform in git_declaration_regex

[melendezd]

Thanks for the feedback! I think you got to the bottom of it. Really the only reason for the distinction between .tf and .tf.json files in the fetcher was because I foresaw having to make the distinction in the updater. I didn't initially think about the messy diffs that this approach could create -- I could give the json regex a shot.

[melendezd]

I've updated the file updater to use regex substitutions in the JSON files. The provider declaration regex was quite difficult because the source and version can be declared in either order, but I believe my approach works as long as there aren't any references to named values (like ${var.name}) after the version and before the source. It seems like the case of the version coming before the source isn't handled in the hcl format updater, so I'm wondering how important this is.

[jeffwidman]

@melendezd do you mind rebasing and also squashing down some of the smaller commits? We merge PR's with a merge strategy currently, not a squash strategy, so what's in the PR history is what eventually gets committed.

To be clear: No need to squash to only a single commit--if there are multiple logical changes, feel free to keep them separate for easier review, but I suspect there's not 32 separate logical change sets here... 😀

Tidying this up will also make it easier to review the PR.

[jeffwidman]

gentle nudge @melendezd

[melendezd]

gentle nudge @melendezd

My bad, this got away from me. I'm a bit busy at the moment but will let you know when I get back on this.

[jeffwidman]

@melendezd any chance things have slowed down enough to get this across the finish line?

[melendezd]

How's this look? :)

[njculver]

@jeffwidman - Are there any further updates necessary for this to be put on the schedule for being reviewed?

[deivid-rodriguez]

I started to have a look:

• Some fixtures seem unused: unparseable_json, provider_implicit_source_json, git_tags_013_json.
• Needs a rebase since this conflicts with Fix terraform runtime error if version preceeds source in required_providers block #6537.