cli: Fix for missing provider requirements in JSON plan

[alisdair]

The JSON plan output format includes a serialized, simplified version of the configuration. One component of this config is a map of provider configurations, which includes version constraints.

Until now, only version constraints specified in the provider config blocks were exposed in the JSON plan output. This is a deprecated method of specifying provider versions, and the recommended use of a required_providers block resulted in the version constraints being omitted.

This commit fixes this with two changes:

• When processing the provider configurations from a module, output the fully-merged version constraints for the entire module, instead of any constraints set in the provider configuration block itself;
• After all provider configurations are processed, iterate over the required_providers entries to ensure that any configuration-less providers are output to the JSON plan too.

No changes are necessary to the structure of the JSON plan output, so this is effectively a semantic level bug fix.

[codecov]

Codecov Report

Merging #27697 (7cae763) into master (32d2084) will decrease coverage by 0.00%.
The diff coverage is 14.28%.

Impacted Files	Coverage Δ
command/jsonconfig/config.go	0.00% <0.00%> (ø)
configs/config.go	69.29% <100.00%> (+0.82%)	⬆️

[alisdair]

@vancluever This is in response to a Sentinel-related issue, so I was hoping you could take a quick look and see if these semantics line up with what Sentinel expects from the JSON plan format.

[vancluever]

I have no problems with a no-impact change 😉

[vancluever]

PS: Structure lines up. 👍 We currently expose version_constraint directly without any additional processing, so this works.

To my note above, while I definitely don't have a problem with this approach, generally the pattern has been so far that separate semantic elements in the Terraform config get exposed separately, so I almost expected to see a separate "RequiredProviders" section exposed separately. I don't know if that's entirely needed though, and if we can't see any value in that kind of semantic exposure and there would never be any conflict here, I'm fine with this approach!

[pkolyvas]

PS: Structure lines up. 👍 We currently expose version_constraint directly without any additional processing, so this works.

To my note above, while I definitely don't have a problem with this approach, generally the pattern has been so far that separate semantic elements in the Terraform config get exposed separately, so I almost expected to see a separate "RequiredProviders" section exposed separately. I don't know if that's entirely needed though, and if we can't see any value in that kind of semantic exposure and there would never be any conflict here, I'm fine with this approach!

I should add @vancluever that I asked if there was a way to get this information without breaking existing policies.

[mildwonkey]

LGTM, if @vancluever is happy I'm happy !

[alisdair]

@vancluever Thanks for looking at this! I'm going to merge for now as I think it most expediently addresses the problem the Sentinel user is having, while still being a logical bug fix of the existing JSON config semantics.

For Sentinel to be able to inspect the contents of required_providers (e.g. provider source, version and the upcoming configuration aliases) we'd have to extend the JSON config schema. I'd be happy to help with that if it's an enhancement that you'd like to build on.

[vancluever]

That will definitely something we will want to pursue, but I'll leave it to product to roadmap accordingly.

cc @hcrhall @dylanegan

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.