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

configs: deprecate version argument inside provider configuration blocks #26135

Merged
merged 3 commits into from
Sep 8, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions configs/config_test.go
Expand Up @@ -115,7 +115,12 @@ func TestConfigResolveAbsProviderAddr(t *testing.T) {

func TestConfigProviderRequirements(t *testing.T) {
cfg, diags := testNestedModuleConfigFromDir(t, "testdata/provider-reqs")
assertNoDiagnostics(t, diags)
// TODO: Version Constraint Deprecation.
// Once we've removed the version argument from provider configuration
// blocks, this can go back to expected 0 diagnostics.
// assertNoDiagnostics(t, diags)
assertDiagnosticCount(t, diags, 1)
assertDiagnosticSummary(t, diags, "Version constraints inside provider configuration blocks are deprecated")

tlsProvider := addrs.NewProvider(
addrs.DefaultRegistryHost,
Expand Down Expand Up @@ -153,7 +158,12 @@ func TestConfigProviderRequirements(t *testing.T) {

func TestConfigProviderRequirementsByModule(t *testing.T) {
cfg, diags := testNestedModuleConfigFromDir(t, "testdata/provider-reqs")
assertNoDiagnostics(t, diags)
// TODO: Version Constraint Deprecation.
// Once we've removed the version argument from provider configuration
// blocks, this can go back to expected 0 diagnostics.
// assertNoDiagnostics(t, diags)
assertDiagnosticCount(t, diags, 1)
assertDiagnosticSummary(t, diags, "Version constraints inside provider configuration blocks are deprecated")

tlsProvider := addrs.NewProvider(
addrs.DefaultRegistryHost,
Expand Down
@@ -1,8 +1,7 @@
provider "aws" {
version = "1.0"
region = "us-west-2"
}

output "my_output" {
value = "my output"
}

5 changes: 3 additions & 2 deletions configs/parser_test.go
Expand Up @@ -83,13 +83,12 @@ func testNestedModuleConfigFromDir(t *testing.T, path string) (*Config, hcl.Diag

parser := NewParser(nil)
mod, diags := parser.LoadConfigDir(path)
assertNoDiagnostics(t, diags)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a little odd: It doesn't really make sense for a helper to call another helper, or to bail out early instead of letting the caller decide what to do with diagnostics.

There are also only 2 tests using this code, which I eyeballed, and I think I wrote this anyway so it was probably a thoughtless copy-pasta.

if mod == nil {
t.Fatal("got nil root module; want non-nil")
}

versionI := 0
cfg, diags := BuildConfig(mod, ModuleWalkerFunc(
cfg, nestedDiags := BuildConfig(mod, ModuleWalkerFunc(
func(req *ModuleRequest) (*Module, *version.Version, hcl.Diagnostics) {
// For the sake of this test we're going to just treat our
// SourceAddr as a path relative to the calling module.
Expand All @@ -111,6 +110,8 @@ func testNestedModuleConfigFromDir(t *testing.T, path string) (*Config, hcl.Diag
return mod, version, diags
},
))

diags = append(diags, nestedDiags...)
return cfg, diags
}

Expand Down
6 changes: 6 additions & 0 deletions configs/provider.go
Expand Up @@ -69,6 +69,12 @@ func decodeProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) {
}

if attr, exists := content.Attributes["version"]; exists {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagWarning,
Summary: "Version constraints inside provider configuration blocks are deprecated",
Detail: "Terraform 0.13 and earlier allowed provider version constraints inside the provider configuration block, but that is now deprecated and will be removed in a future version of Terraform. To silence this warning, move the provider version constraint into the required_providers block.",
Subject: attr.Expr.Range().Ptr(),
})
var versionDiags hcl.Diagnostics
provider.Version, versionDiags = decodeVersionConstraint(attr)
diags = append(diags, versionDiags...)
Expand Down
2 changes: 2 additions & 0 deletions configs/provider_test.go
Expand Up @@ -21,6 +21,8 @@ func TestProviderReservedNames(t *testing.T) {
_, diags := parser.LoadConfigFile("config.tf")

assertExactDiagnostics(t, diags, []string{
//TODO: This deprecation warning will be removed in terraform v0.15.
`config.tf:4,13-20: Version constraints inside provider configuration blocks are deprecated; Terraform 0.13 and earlier allowed provider version constraints inside the provider configuration block, but that is now deprecated and will be removed in a future version of Terraform. To silence this warning, move the provider version constraint into the required_providers block.`,
`config.tf:10,3-8: Reserved argument name in provider block; The provider argument name "count" is reserved for use by Terraform in a future version.`,
`config.tf:11,3-13: Reserved argument name in provider block; The provider argument name "depends_on" is reserved for use by Terraform in a future version.`,
`config.tf:12,3-11: Reserved argument name in provider block; The provider argument name "for_each" is reserved for use by Terraform in a future version.`,
Expand Down
2 changes: 0 additions & 2 deletions configs/testdata/valid-files/provider-configs.tf
Expand Up @@ -3,8 +3,6 @@ provider "foo" {
}

provider "bar" {
version = ">= 1.0.2"

other = 12
}

Expand Down
6 changes: 3 additions & 3 deletions website/docs/configuration/providers.html.md
Expand Up @@ -177,12 +177,12 @@ works the same way as the `version` argument in a
constraint in a provider configuration is only used if `required_providers`
does not include one for that provider.

**We do not recommend using the `version` argument in provider configurations.**
**The `version` argument in provider configurations is deprecated.**
In Terraform 0.13 and later, version constraints should always be declared in
[the `required_providers` block](./provider-requirements.html).
[the `required_providers` block](./provider-requirements.html). The `version`
argument will be removed in a future version of Terraform.

-> **Note:** The `version` meta-argument made sense before Terraform 0.13, since
Terraform could only install providers that were distributed by HashiCorp. Now
that Terraform can install providers from multiple sources, it makes more sense
to keep version constraints and provider source addresses together.