From 3f22bbf8d57b97f200c09a3f7f9f2e90e28ea94d Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 22 Jun 2020 21:04:19 -0400
Subject: [PATCH] don't allow providers in modules using depends_on

Providers themselves don't support depends_on, and therefor a module
with providers cannot use depends_on.
---
 configs/configload/loader_load.go             | 10 +++++++-
 configs/configload/loader_load_test.go        | 21 ++++++++++++++++
 .../.terraform/modules/modules.json           | 24 +++++++++++++++++++
 .../testdata/module-depends-on/child/main.tf  |  3 +++
 .../testdata/module-depends-on/child2/main.tf |  6 +++++
 .../testdata/module-depends-on/root.tf        |  7 ++++++
 6 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 configs/configload/testdata/module-depends-on/.terraform/modules/modules.json
 create mode 100644 configs/configload/testdata/module-depends-on/child/main.tf
 create mode 100644 configs/configload/testdata/module-depends-on/child2/main.tf
 create mode 100644 configs/configload/testdata/module-depends-on/root.tf

diff --git a/configs/configload/loader_load.go b/configs/configload/loader_load.go
index ba9b29ffd..eab38495c 100644
--- a/configs/configload/loader_load.go
+++ b/configs/configload/loader_load.go
@@ -111,7 +111,7 @@ func (l *Loader) moduleWalkerLoad(req *configs.ModuleRequest) (*configs.Module,
 }
 
 func validateProviderConfigs(mc *configs.ModuleCall, mod *configs.Module, parent *configs.Config, diags hcl.Diagnostics) hcl.Diagnostics {
-	if mc.Count != nil || mc.ForEach != nil {
+	if mc.Count != nil || mc.ForEach != nil || mc.DependsOn != nil {
 		for key, pc := range mod.ProviderConfigs {
 			// Use these to track if a provider is configured (not allowed),
 			// or if we've found its matching proxy
@@ -150,6 +150,14 @@ func validateProviderConfigs(mc *configs.ModuleCall, mod *configs.Module, parent
 						Subject:  mc.ForEach.Range().Ptr(),
 					})
 				}
+				if mc.DependsOn != nil {
+					diags = append(diags, &hcl.Diagnostic{
+						Severity: hcl.DiagError,
+						Summary:  "Module does not support depends_on",
+						Detail:   fmt.Sprintf(moduleProviderError, mc.Name, "depends_on", key, pc.NameRange),
+						Subject:  mc.SourceAddrRange.Ptr(),
+					})
+				}
 			}
 		}
 	}
diff --git a/configs/configload/loader_load_test.go b/configs/configload/loader_load_test.go
index c65e40f25..b7f396cf2 100644
--- a/configs/configload/loader_load_test.go
+++ b/configs/configload/loader_load_test.go
@@ -122,3 +122,24 @@ func TestLoaderLoadConfig_moduleExpandValid(t *testing.T) {
 	_, diags := loader.LoadConfig(fixtureDir)
 	assertNoDiagnostics(t, diags)
 }
+
+func TestLoaderLoadConfig_moduleDependsOnProviders(t *testing.T) {
+	// We do not allow providers to be configured in module using depends_on.
+	fixtureDir := filepath.Clean("testdata/module-depends-on")
+	loader, err := NewLoader(&Config{
+		ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"),
+	})
+	if err != nil {
+		t.Fatalf("unexpected error from NewLoader: %s", err)
+	}
+
+	_, diags := loader.LoadConfig(fixtureDir)
+	if !diags.HasErrors() {
+		t.Fatal("success; want error")
+	}
+	got := diags.Error()
+	want := "Module does not support depends_on"
+	if !strings.Contains(got, want) {
+		t.Fatalf("wrong error\ngot:\n%s\n\nwant: containing %q", got, want)
+	}
+}
diff --git a/configs/configload/testdata/module-depends-on/.terraform/modules/modules.json b/configs/configload/testdata/module-depends-on/.terraform/modules/modules.json
new file mode 100644
index 000000000..28f813039
--- /dev/null
+++ b/configs/configload/testdata/module-depends-on/.terraform/modules/modules.json
@@ -0,0 +1,24 @@
+{
+    "Modules": [
+        {
+            "Key": "",
+            "Source": "",
+            "Dir": "testdata/expand-modules/nested-provider"
+        },
+        {
+            "Key": "child",
+            "Source": "./child",
+            "Dir": "testdata/expand-modules/nested-provider/child"
+        },
+        {
+            "Key": "child2",
+            "Source": "./child2",
+            "Dir": "testdata/expand-modules/nested-provider/child2"
+        },
+        {
+            "Key": "child.child2",
+            "Source": "../child2",
+            "Dir": "testdata/expand-modules/nested-provider/child2"
+        }
+    ]
+}
diff --git a/configs/configload/testdata/module-depends-on/child/main.tf b/configs/configload/testdata/module-depends-on/child/main.tf
new file mode 100644
index 000000000..f6b74b7a1
--- /dev/null
+++ b/configs/configload/testdata/module-depends-on/child/main.tf
@@ -0,0 +1,3 @@
+module "child2" {
+  source = "../child2"
+}
diff --git a/configs/configload/testdata/module-depends-on/child2/main.tf b/configs/configload/testdata/module-depends-on/child2/main.tf
new file mode 100644
index 000000000..bea1f0c4f
--- /dev/null
+++ b/configs/configload/testdata/module-depends-on/child2/main.tf
@@ -0,0 +1,6 @@
+provider "aws" {
+}
+
+output "my_output" {
+  value = "my output"
+}
diff --git a/configs/configload/testdata/module-depends-on/root.tf b/configs/configload/testdata/module-depends-on/root.tf
new file mode 100644
index 000000000..b14d24c45
--- /dev/null
+++ b/configs/configload/testdata/module-depends-on/root.tf
@@ -0,0 +1,7 @@
+module "child" {
+  source = "./child"
+  depends_on = [test_resource.a]
+}
+
+resource "test_resource" "a" {
+}