From 2d07f79c5a8ff2368ed811f02591e7ab4fd21929 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 24 Sep 2014 19:31:30 -0700 Subject: [PATCH] terraform: validation validates the submodules --- terraform/context.go | 235 +++++++++--------- terraform/context_test.go | 59 +++++ .../validate-bad-module-output/child/main.tf | 0 .../validate-bad-module-output/main.tf | 7 + .../validate-good-module/child/main.tf | 3 + .../validate-good-module/main.tf | 7 + .../validate-module-bad-rc/child/main.tf | 1 + .../validate-module-bad-rc/main.tf | 3 + 8 files changed, 204 insertions(+), 111 deletions(-) create mode 100644 terraform/test-fixtures/validate-bad-module-output/child/main.tf create mode 100644 terraform/test-fixtures/validate-bad-module-output/main.tf create mode 100644 terraform/test-fixtures/validate-good-module/child/main.tf create mode 100644 terraform/test-fixtures/validate-good-module/main.tf create mode 100644 terraform/test-fixtures/validate-module-bad-rc/child/main.tf create mode 100644 terraform/test-fixtures/validate-module-bad-rc/main.tf diff --git a/terraform/context.go b/terraform/context.go index 9abd4b180..1d7bd9c3b 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -215,45 +215,37 @@ func (c *Context) Stop() { func (c *Context) Validate() ([]string, []error) { var rerr *multierror.Error - if config := c.module.Config(); config != nil { - // Validate the configuration itself - if err := config.Validate(); err != nil { - rerr = multierror.ErrorAppend(rerr, err) - } + // Validate the configuration itself + if err := c.module.Validate(); err != nil { + rerr = multierror.ErrorAppend(rerr, err) + } + // TODO: modules + if config := c.module.Config(); config != nil { // Validate the user variables if errs := smcUserVariables(config, c.variables); len(errs) > 0 { rerr = multierror.ErrorAppend(rerr, errs...) } } - // Validate the graph - g, err := c.graph() - if err != nil { - rerr = multierror.ErrorAppend(rerr, fmt.Errorf( - "Error creating graph: %s", err)) + // Validate the entire graph + walkMeta := new(walkValidateMeta) + wc := c.walkContext(walkValidate, rootModulePath) + wc.Meta = walkMeta + if err := wc.Walk(); err != nil { + rerr = multierror.ErrorAppend(rerr, err) + } + + if len(walkMeta.Errs) > 0 { + rerr = multierror.ErrorAppend(rerr, walkMeta.Errs...) } - // Walk the graph and validate all the configs - var warns []string var errs []error - if g != nil { - err = g.Walk(c.validateWalkFn(&warns, &errs)) - if err != nil { - rerr = multierror.ErrorAppend(rerr, fmt.Errorf( - "Error validating resources in graph: %s", err)) - } - if len(errs) > 0 { - rerr = multierror.ErrorAppend(rerr, errs...) - } - } - - errs = nil if rerr != nil && len(rerr.Errors) > 0 { errs = rerr.Errors } - return warns, errs + return walkMeta.Warns, errs } func (c *Context) graph() (*depgraph.Graph, error) { @@ -292,92 +284,6 @@ func (c *Context) releaseRun(ch chan<- struct{}) { c.sh.Reset() } -func (c *Context) validateWalkFn(rws *[]string, res *[]error) depgraph.WalkFunc { - var l sync.Mutex - - return func(n *depgraph.Noun) error { - // If it is the root node, ignore - if n.Name == GraphRootNode { - return nil - } - - switch rn := n.Meta.(type) { - case *GraphNodeResource: - if rn.Resource == nil { - panic("resource should never be nil") - } - - // If it doesn't have a provider, that is a different problem - if rn.Resource.Provider == nil { - return nil - } - - // Don't validate orphans since they never have a config - if rn.Resource.Flags&FlagOrphan != 0 { - return nil - } - - log.Printf("[INFO] Validating resource: %s", rn.Resource.Id) - ws, es := rn.Resource.Provider.ValidateResource( - rn.Resource.Info.Type, rn.Resource.Config) - for i, w := range ws { - ws[i] = fmt.Sprintf("'%s' warning: %s", rn.Resource.Id, w) - } - for i, e := range es { - es[i] = fmt.Errorf("'%s' error: %s", rn.Resource.Id, e) - } - - l.Lock() - *rws = append(*rws, ws...) - *res = append(*res, es...) - l.Unlock() - - for idx, p := range rn.Resource.Provisioners { - ws, es := p.Provisioner.Validate(p.Config) - for i, w := range ws { - ws[i] = fmt.Sprintf("'%s.provisioner.%d' warning: %s", rn.Resource.Id, idx, w) - } - for i, e := range es { - es[i] = fmt.Errorf("'%s.provisioner.%d' error: %s", rn.Resource.Id, idx, e) - } - - l.Lock() - *rws = append(*rws, ws...) - *res = append(*res, es...) - l.Unlock() - } - - case *GraphNodeResourceProvider: - sharedProvider := rn.Provider - - var raw *config.RawConfig - if sharedProvider.Config != nil { - raw = sharedProvider.Config.RawConfig - } - - rc := NewResourceConfig(raw) - - for k, p := range sharedProvider.Providers { - log.Printf("[INFO] Validating provider: %s", k) - ws, es := p.Validate(rc) - for i, w := range ws { - ws[i] = fmt.Sprintf("Provider '%s' warning: %s", k, w) - } - for i, e := range es { - es[i] = fmt.Errorf("Provider '%s' error: %s", k, e) - } - - l.Lock() - *rws = append(*rws, ws...) - *res = append(*res, es...) - l.Unlock() - } - } - - return nil - } -} - func (c *Context) walkContext(op walkOperation, path []string) *walkContext { // Get the config structure m := c.module @@ -435,8 +341,14 @@ const ( walkPlan walkPlanDestroy walkRefresh + walkValidate ) +type walkValidateMeta struct { + Errs []error + Warns []string +} + func (c *walkContext) Walk() error { g := c.graph if g == nil { @@ -467,6 +379,8 @@ func (c *walkContext) Walk() error { walkFn = c.planDestroyWalkFn() case walkRefresh: walkFn = c.refreshWalkFn() + case walkValidate: + walkFn = c.validateWalkFn() default: panic(fmt.Sprintf("unknown operation: %s", c.Operation)) } @@ -845,6 +759,105 @@ func (c *walkContext) refreshWalkFn() depgraph.WalkFunc { return c.genericWalkFn(cb) } +func (c *walkContext) validateWalkFn() depgraph.WalkFunc { + var l sync.Mutex + + meta := c.Meta.(*walkValidateMeta) + + return func(n *depgraph.Noun) error { + // If it is the root node, ignore + if n.Name == GraphRootNode { + return nil + } + + switch rn := n.Meta.(type) { + case *GraphNodeModule: + // Build another walkContext for this module and walk it. + wc := c.Context.walkContext(walkValidate, rn.Path) + + // Set the graph to specifically walk this subgraph + wc.graph = rn.Graph + + // Preserve the meta + wc.Meta = c.Meta + + return wc.Walk() + case *GraphNodeResource: + if rn.Resource == nil { + panic("resource should never be nil") + } + + // If it doesn't have a provider, that is a different problem + if rn.Resource.Provider == nil { + return nil + } + + // Don't validate orphans since they never have a config + if rn.Resource.Flags&FlagOrphan != 0 { + return nil + } + + log.Printf("[INFO] Validating resource: %s", rn.Resource.Id) + ws, es := rn.Resource.Provider.ValidateResource( + rn.Resource.Info.Type, rn.Resource.Config) + for i, w := range ws { + ws[i] = fmt.Sprintf("'%s' warning: %s", rn.Resource.Id, w) + } + for i, e := range es { + es[i] = fmt.Errorf("'%s' error: %s", rn.Resource.Id, e) + } + + l.Lock() + meta.Warns = append(meta.Warns, ws...) + meta.Errs = append(meta.Errs, es...) + l.Unlock() + + for idx, p := range rn.Resource.Provisioners { + ws, es := p.Provisioner.Validate(p.Config) + for i, w := range ws { + ws[i] = fmt.Sprintf("'%s.provisioner.%d' warning: %s", rn.Resource.Id, idx, w) + } + for i, e := range es { + es[i] = fmt.Errorf("'%s.provisioner.%d' error: %s", rn.Resource.Id, idx, e) + } + + l.Lock() + meta.Warns = append(meta.Warns, ws...) + meta.Errs = append(meta.Errs, es...) + l.Unlock() + } + + case *GraphNodeResourceProvider: + sharedProvider := rn.Provider + + var raw *config.RawConfig + if sharedProvider.Config != nil { + raw = sharedProvider.Config.RawConfig + } + + rc := NewResourceConfig(raw) + + for k, p := range sharedProvider.Providers { + log.Printf("[INFO] Validating provider: %s", k) + ws, es := p.Validate(rc) + for i, w := range ws { + ws[i] = fmt.Sprintf("Provider '%s' warning: %s", k, w) + } + for i, e := range es { + es[i] = fmt.Errorf("Provider '%s' error: %s", k, e) + } + + l.Lock() + meta.Warns = append(meta.Warns, ws...) + meta.Errs = append(meta.Errs, es...) + l.Unlock() + } + } + + return nil + } +} + func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { // This will keep track of whether we're stopped or not var stop uint32 = 0 diff --git a/terraform/context_test.go b/terraform/context_test.go index 1b33744e9..b1e28b4db 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -50,6 +50,44 @@ func TestContextValidate(t *testing.T) { } } +func TestContextValidate_goodModule(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "validate-good-module") + c := testContext(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + w, e := c.Validate() + if len(w) > 0 { + t.Fatalf("bad: %#v", w) + } + if len(e) > 0 { + t.Fatalf("bad: %#v", e) + } +} + +func TestContextValidate_badModuleOutput(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "validate-bad-module-output") + c := testContext(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + w, e := c.Validate() + if len(w) > 0 { + t.Fatalf("bad: %#v", w) + } + if len(e) == 0 { + t.Fatalf("bad: %#v", e) + } +} + func TestContextValidate_badVar(t *testing.T) { p := testProvider("aws") m := testModule(t, "validate-bad-var") @@ -69,6 +107,27 @@ func TestContextValidate_badVar(t *testing.T) { } } +func TestContextValidate_moduleBadResource(t *testing.T) { + m := testModule(t, "validate-module-bad-rc") + p := testProvider("aws") + c := testContext(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + p.ValidateResourceReturnErrors = []error{fmt.Errorf("bad")} + + w, e := c.Validate() + if len(w) > 0 { + t.Fatalf("bad: %#v", w) + } + if len(e) == 0 { + t.Fatalf("bad: %#v", e) + } +} + func TestContextValidate_orphans(t *testing.T) { p := testProvider("aws") m := testModule(t, "validate-good") diff --git a/terraform/test-fixtures/validate-bad-module-output/child/main.tf b/terraform/test-fixtures/validate-bad-module-output/child/main.tf new file mode 100644 index 000000000..e69de29bb diff --git a/terraform/test-fixtures/validate-bad-module-output/main.tf b/terraform/test-fixtures/validate-bad-module-output/main.tf new file mode 100644 index 000000000..bda34f51a --- /dev/null +++ b/terraform/test-fixtures/validate-bad-module-output/main.tf @@ -0,0 +1,7 @@ +module "child" { + source = "./child" +} + +resource "aws_instance" "bar" { + foo = "${module.child.bad}" +} diff --git a/terraform/test-fixtures/validate-good-module/child/main.tf b/terraform/test-fixtures/validate-good-module/child/main.tf new file mode 100644 index 000000000..17d8c60a7 --- /dev/null +++ b/terraform/test-fixtures/validate-good-module/child/main.tf @@ -0,0 +1,3 @@ +output "good" { + value = "great" +} diff --git a/terraform/test-fixtures/validate-good-module/main.tf b/terraform/test-fixtures/validate-good-module/main.tf new file mode 100644 index 000000000..439d20210 --- /dev/null +++ b/terraform/test-fixtures/validate-good-module/main.tf @@ -0,0 +1,7 @@ +module "child" { + source = "./child" +} + +resource "aws_instance" "bar" { + foo = "${module.child.good}" +} diff --git a/terraform/test-fixtures/validate-module-bad-rc/child/main.tf b/terraform/test-fixtures/validate-module-bad-rc/child/main.tf new file mode 100644 index 000000000..919f140bb --- /dev/null +++ b/terraform/test-fixtures/validate-module-bad-rc/child/main.tf @@ -0,0 +1 @@ +resource "aws_instance" "foo" {} diff --git a/terraform/test-fixtures/validate-module-bad-rc/main.tf b/terraform/test-fixtures/validate-module-bad-rc/main.tf new file mode 100644 index 000000000..0f6991c53 --- /dev/null +++ b/terraform/test-fixtures/validate-module-bad-rc/main.tf @@ -0,0 +1,3 @@ +module "child" { + source = "./child" +}