Allow cloud block overrides

These changes allow cloud blocks to be overridden by backend blocks and
vice versa; the logic follows the current backend behavior of a block
overriding a preceding block in full, with no merges.

internal/configs/module.go

@@ -379,6 +379,7 @@ func (m *Module) mergeFile(file *File) hcl.Diagnostics {
 	if len(file.Backends) != 0 {
 		switch len(file.Backends) {
 		case 1:
+			m.CloudConfig = nil // A backend block is mutually exclusive with a cloud one, and overwrites any cloud config
 			m.Backend = file.Backends[0]
 		default:
 			// An override file with multiple backends is still invalid, even
@@ -392,16 +393,21 @@ func (m *Module) mergeFile(file *File) hcl.Diagnostics {
 		}
 	}
 
-	// TODO: This restriction is temporary. Overrides should be allowed, but have the added
+	if len(file.CloudConfigs) != 0 {
-	// complexity of needing to also override a 'backend' block, so this work is being deferred
+		switch len(file.CloudConfigs) {
-	// for now.
+		case 1:
-	for _, m := range file.CloudConfigs {
+			m.Backend = nil // A cloud block is mutually exclusive with a backend one, and overwrites any backend
-		diags = append(diags, &hcl.Diagnostic{
+			m.CloudConfig = file.CloudConfigs[0]
-			Severity: hcl.DiagError,
+		default:
-			Summary:  "Cannot override 'cloud' configuration",
+			// An override file with multiple cloud blocks is still invalid, even
-			Detail:   "Terraform Cloud configuration blocks can appear only in normal files, not in override files.",
+			// though it can override cloud/backend blocks from _other_ files.
-			Subject:  m.DeclRange.Ptr(),
+			diags = append(diags, &hcl.Diagnostic{
-		})
+				Severity: hcl.DiagError,
+				Summary:  "Duplicate Terraform Cloud configurations",
+				Detail:   fmt.Sprintf("A module may have only one 'cloud' block configuring Terraform Cloud. Terraform Cloud was previously configured at %s.", file.CloudConfigs[0].DeclRange),
+				Subject:  &file.CloudConfigs[1].DeclRange,
+			})
+		}
 	}
 
 	for _, pc := range file.ProviderConfigs {

internal/configs/module_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform/internal/addrs"
+	"github.com/zclconf/go-cty/cty"
 )
 
 // TestNewModule_provider_fqns exercises module.gatherProviderLocalNames()
@@ -309,3 +310,105 @@ func TestImpliedProviderForUnqualifiedType(t *testing.T) {
 		}
 	}
 }
+
+func TestModule_backend_override(t *testing.T) {
+	mod, diags := testModuleFromDir("testdata/valid-modules/override-backend")
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	gotType := mod.Backend.Type
+	wantType := "bar"
+
+	if gotType != wantType {
+		t.Errorf("wrong result for backend type: got %#v, want %#v\n", gotType, wantType)
+	}
+
+	attrs, _ := mod.Backend.Config.JustAttributes()
+
+	gotAttr, diags := attrs["path"].Expr.Value(nil)
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	wantAttr := cty.StringVal("CHANGED/relative/path/to/terraform.tfstate")
+
+	if !gotAttr.RawEquals(wantAttr) {
+		t.Errorf("wrong result for backend 'path': got %#v, want %#v\n", gotAttr, wantAttr)
+	}
+}
+
+// Unlike most other overrides, backend blocks do not require a base configuration in a primary
+// configuration file, as an omitted backend there implies the local backend.
+func TestModule_backend_override_no_base(t *testing.T) {
+	mod, diags := testModuleFromDir("testdata/valid-modules/override-backend-no-base")
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	if mod.Backend == nil {
+		t.Errorf("expected module Backend not to be nil")
+	}
+}
+
+func TestModule_cloud_override_backend(t *testing.T) {
+	mod, diags := testModuleFromDir("testdata/valid-modules/override-backend-with-cloud")
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	if mod.Backend != nil {
+		t.Errorf("expected module Backend to be nil")
+	}
+
+	if mod.CloudConfig == nil {
+		t.Errorf("expected module CloudConfig not to be nil")
+	}
+}
+
+// Unlike most other overrides, cloud blocks do not require a base configuration in a primary
+// configuration file, as an omitted backend there implies the local backend and cloud blocks
+// override backends.
+func TestModule_cloud_override_no_base(t *testing.T) {
+	mod, diags := testModuleFromDir("testdata/valid-modules/override-cloud-no-base")
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	if mod.CloudConfig == nil {
+		t.Errorf("expected module CloudConfig not to be nil")
+	}
+}
+
+func TestModule_cloud_override(t *testing.T) {
+	mod, diags := testModuleFromDir("testdata/valid-modules/override-cloud")
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	attrs, _ := mod.CloudConfig.Config.JustAttributes()
+
+	gotAttr, diags := attrs["organization"].Expr.Value(nil)
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	wantAttr := cty.StringVal("CHANGED")
+
+	if !gotAttr.RawEquals(wantAttr) {
+		t.Errorf("wrong result for Cloud 'organization': got %#v, want %#v\n", gotAttr, wantAttr)
+	}
+
+	// The override should have completely replaced the cloud block in the primary file, no merging
+	if attrs["should_not_be_present_with_override"] != nil {
+		t.Errorf("expected 'should_not_be_present_with_override' attribute to be nil")
+	}
+}
+
+func TestModule_cloud_duplicate_overrides(t *testing.T) {
+	_, diags := testModuleFromDir("testdata/invalid-modules/override-cloud-duplicates")
+	want := `Duplicate Terraform Cloud configurations`
+	if got := diags.Error(); !strings.Contains(got, want) {
+		t.Fatalf("expected module error to contain %q\nerror was:\n%s", want, got)
+	}
+}

internal/configs/testdata/invalid-modules/override-cloud-duplicates/main.tf

@@ -0,0 +1,14 @@
+terraform {
+  cloud {
+    organization = "foo"
+    should_not_be_present_with_override = true
+  }
+}
+
+resource "aws_instance" "web" {
+  ami = "ami-1234"
+  security_groups = [
+    "foo",
+    "bar",
+  ]
+}

internal/configs/testdata/invalid-modules/override-cloud-duplicates/override.tf

@@ -0,0 +1,11 @@
+terraform {
+  cloud {
+    organization = "foo"
+  }
+}
+
+terraform {
+  cloud {
+    organization = "bar"
+  }
+}

internal/configs/testdata/valid-modules/override-backend-no-base/main.tf

@@ -0,0 +1,7 @@
+resource "aws_instance" "web" {
+  ami = "ami-1234"
+  security_groups = [
+    "foo",
+    "bar",
+  ]
+}

internal/configs/testdata/valid-modules/override-backend-no-base/override.tf

@@ -0,0 +1,5 @@
+terraform {
+  backend "bar" {
+    path = "CHANGED/relative/path/to/terraform.tfstate"
+  }
+}

internal/configs/testdata/valid-modules/override-backend-with-cloud/main.tf

@@ -0,0 +1,13 @@
+terraform {
+  backend "foo" {
+    path = "relative/path/to/terraform.tfstate"
+  }
+}
+
+resource "aws_instance" "web" {
+  ami = "ami-1234"
+  security_groups = [
+    "foo",
+    "bar",
+  ]
+}

internal/configs/testdata/valid-modules/override-backend-with-cloud/override.tf

@@ -0,0 +1,5 @@
+terraform {
+  cloud {
+    organization = "foo"
+  }
+}

internal/configs/testdata/valid-modules/override-backend/main.tf

@@ -0,0 +1,13 @@
+terraform {
+  backend "foo" {
+    path = "relative/path/to/terraform.tfstate"
+  }
+}
+
+resource "aws_instance" "web" {
+  ami = "ami-1234"
+  security_groups = [
+    "foo",
+    "bar",
+  ]
+}

internal/configs/testdata/valid-modules/override-backend/override.tf

@@ -0,0 +1,5 @@
+terraform {
+  backend "bar" {
+    path = "CHANGED/relative/path/to/terraform.tfstate"
+  }
+}

internal/configs/testdata/valid-modules/override-cloud-no-base/main.tf

@@ -0,0 +1,7 @@
+resource "aws_instance" "web" {
+  ami = "ami-1234"
+  security_groups = [
+    "foo",
+    "bar",
+  ]
+}

internal/configs/testdata/valid-modules/override-cloud-no-base/override.tf

@@ -0,0 +1,5 @@
+terraform {
+  cloud {
+    organization = "foo"
+  }
+}

internal/configs/testdata/valid-modules/override-cloud/main.tf

@@ -0,0 +1,14 @@
+terraform {
+  cloud {
+    organization = "foo"
+    should_not_be_present_with_override = true
+  }
+}
+
+resource "aws_instance" "web" {
+  ami = "ami-1234"
+  security_groups = [
+    "foo",
+    "bar",
+  ]
+}

internal/configs/testdata/valid-modules/override-cloud/override.tf

@@ -0,0 +1,5 @@
+terraform {
+  cloud {
+    organization = "CHANGED"
+  }
+}