config: validate depends_on with module values

config/config.go

@@ -508,25 +508,8 @@ func (c *Config) Validate() error {
 		}
 		r.RawCount.init()
 
-		// Verify depends on points to resources that all exist
+		// Validate DependsOn
-		for _, d := range r.DependsOn {
+		errs = append(errs, c.validateDependsOn(n, r.DependsOn, resources, modules)...)
-			// Check if we contain interpolations
-			rc, err := NewRawConfig(map[string]interface{}{
-				"value": d,
-			})
-			if err == nil && len(rc.Variables) > 0 {
-				errs = append(errs, fmt.Errorf(
-					"%s: depends on value cannot contain interpolations: %s",
-					n, d))
-				continue
-			}
-
-			if _, ok := resources[d]; !ok {
-				errs = append(errs, fmt.Errorf(
-					"%s: resource depends on non-existent resource '%s'",
-					n, d))
-			}
-		}
 
 		// Verify provider points to a provider that is configured
 		if r.Provider != "" {
@@ -801,6 +784,48 @@ func (c *Config) validateVarContextFn(
 	}
 }
 
+func (c *Config) validateDependsOn(
+	n string,
+	v []string,
+	resources map[string]*Resource,
+	modules map[string]*Module) []error {
+	// Verify depends on points to resources that all exist
+	var errs []error
+	for _, d := range v {
+		// Check if we contain interpolations
+		rc, err := NewRawConfig(map[string]interface{}{
+			"value": d,
+		})
+		if err == nil && len(rc.Variables) > 0 {
+			errs = append(errs, fmt.Errorf(
+				"%s: depends on value cannot contain interpolations: %s",
+				n, d))
+			continue
+		}
+
+		// If it is a module, verify it is a module
+		if strings.HasPrefix(d, "module.") {
+			name := d[len("module."):]
+			if _, ok := modules[name]; !ok {
+				errs = append(errs, fmt.Errorf(
+					"%s: resource depends on non-existent module '%s'",
+					n, name))
+			}
+
+			continue
+		}
+
+		// Check resources
+		if _, ok := resources[d]; !ok {
+			errs = append(errs, fmt.Errorf(
+				"%s: resource depends on non-existent resource '%s'",
+				n, d))
+		}
+	}
+
+	return errs
+}
+
 func (m *Module) mergerName() string {
 	return m.Id()
 }

config/config_test.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"flag"
+	"fmt"
 	"io/ioutil"
 	"log"
 	"os"
@@ -130,11 +131,55 @@ func TestConfig_emptyCollections(t *testing.T) {
 	}
 }
 
-func TestConfigValidate(t *testing.T) {
+// This table test is the preferred way to test validation of configuration.
-	c := testConfig(t, "validate-good")
+// There are dozens of functions below which do not follow this that are
-	if err := c.Validate(); err != nil {
+// there mostly historically. They should be converted at some point.
+func TestConfigValidate_table(t *testing.T) {
+	cases := []struct {
+		Name      string
+		Fixture   string
+		Err       bool
+		ErrString string
+	}{
+		{
+			"basic good",
+			"validate-good",
+			false,
+			"",
+		},
+
+		{
+			"depends on module",
+			"validate-depends-on-module",
+			false,
+			"",
+		},
+
+		{
+			"depends on non-existent module",
+			"validate-depends-on-bad-module",
+			true,
+			"non-existent module 'foo'",
+		},
+	}
+
+	for i, tc := range cases {
+		t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) {
+			c := testConfig(t, tc.Fixture)
+			err := c.Validate()
+			if (err != nil) != tc.Err {
 				t.Fatalf("err: %s", err)
 			}
+			if err != nil {
+				if tc.ErrString != "" && !strings.Contains(err.Error(), tc.ErrString) {
+					t.Fatalf("expected err to contain: %s\n\ngot: %s", tc.ErrString, err)
+				}
+
+				return
+			}
+		})
+	}
+
 }
 
 func TestConfigValidate_badDependsOn(t *testing.T) {

config/test-fixtures/validate-depends-on-bad-module/main.tf

@@ -0,0 +1,7 @@
+module "child" {
+    source = "./child"
+}
+
+resource aws_instance "web" {
+    depends_on = ["module.foo"]
+}

config/test-fixtures/validate-depends-on-module/main.tf

@@ -0,0 +1,7 @@
+module "child" {
+    source = "./child"
+}
+
+resource aws_instance "web" {
+    depends_on = ["module.child"]
+}