-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
Type check variables between modules #6185
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,75 @@ package terraform | |
|
||
import ( | ||
"fmt" | ||
"strings" | ||
|
||
"github.com/hashicorp/errwrap" | ||
"github.com/hashicorp/terraform/config" | ||
"github.com/hashicorp/terraform/config/module" | ||
"github.com/mitchellh/mapstructure" | ||
) | ||
|
||
// EvalTypeCheckVariable is an EvalNode which ensures that the variable | ||
// values which are assigned as inputs to a module (including the root) | ||
// match the types which are either declared for the variables explicitly | ||
// or inferred from the default values. | ||
// | ||
// In order to achieve this three things are required: | ||
// - a map of the proposed variable values | ||
// - the configuration tree of the module in which the variable is | ||
// declared | ||
// - the path to the module (so we know which part of the tree to | ||
// compare the values against). | ||
// | ||
// Currently since the type system is simple, we currently do not make | ||
// use of the values since it is only valid to pass string values. The | ||
// structure is in place for extension of the type system, however. | ||
type EvalTypeCheckVariable struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you mind doing a prose comment block summarizing the intended use case and context of the EvalNode? I've found these to be helpful when navigating unfamiliar EvalTrees, and the name is often not enough to fully get what the node is supposed to do. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes that seems like a worthwhile effort - I think several others can be documented based on the research that went into this PR too! |
||
Variables map[string]string | ||
ModulePath []string | ||
ModuleTree *module.Tree | ||
} | ||
|
||
func (n *EvalTypeCheckVariable) Eval(ctx EvalContext) (interface{}, error) { | ||
currentTree := n.ModuleTree | ||
for _, pathComponent := range n.ModulePath[1:] { | ||
currentTree = currentTree.Children()[pathComponent] | ||
} | ||
targetConfig := currentTree.Config() | ||
|
||
prototypes := make(map[string]config.VariableType) | ||
for _, variable := range targetConfig.Variables { | ||
prototypes[variable.Name] = variable.Type() | ||
} | ||
|
||
for name, declaredType := range prototypes { | ||
// This is only necessary when we _actually_ check. It is left as a reminder | ||
// that at the current time we are dealing with a type system consisting only | ||
// of strings and maps - where the only valid inter-module variable type is | ||
// string. | ||
// proposedValue := n.Variables[name] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't end up using |
||
|
||
switch declaredType { | ||
case config.VariableTypeString: | ||
// This will need actual verification once we aren't dealing with | ||
// a map[string]string but this is sufficient for now. | ||
continue | ||
default: | ||
// Only display a module if we are not in the root module | ||
modulePathDescription := fmt.Sprintf(" in module %s", strings.Join(n.ModulePath[1:], ".")) | ||
if len(n.ModulePath) == 1 { | ||
modulePathDescription = "" | ||
} | ||
// This will need the actual type substituting when we have more than | ||
// just strings and maps. | ||
return nil, fmt.Errorf("variable %s%s should be type %s, got type string", | ||
name, modulePathDescription, declaredType.Printable()) | ||
} | ||
} | ||
|
||
return nil, nil | ||
} | ||
|
||
// EvalSetVariables is an EvalNode implementation that sets the variables | ||
// explicitly for interpolation later. | ||
type EvalSetVariables struct { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
variable "inner_in" { | ||
type = "map" | ||
default = { | ||
us-west-1 = "ami-12345" | ||
us-west-2 = "ami-67890" | ||
} | ||
} | ||
|
||
resource "null_resource" "inner_noop" {} | ||
|
||
output "inner_out" { | ||
value = "${lookup(var.inner_in, "us-west-1")}" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
module "middle" { | ||
source = "./middle" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
variable "middle_in" { | ||
type = "map" | ||
default = { | ||
eu-west-1 = "ami-12345" | ||
eu-west-2 = "ami-67890" | ||
} | ||
} | ||
|
||
module "inner" { | ||
source = "../inner" | ||
|
||
inner_in = "hello" | ||
} | ||
|
||
resource "null_resource" "middle_noop" {} | ||
|
||
output "middle_out" { | ||
value = "${lookup(var.middle_in, "us-west-1")}" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
variable "map_in" { | ||
type = "map" | ||
default = { | ||
us-west-1 = "ami-12345" | ||
us-west-2 = "ami-67890" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
variable "input" { | ||
type = "string" | ||
default = "hello world" | ||
} | ||
|
||
module "test" { | ||
source = "./inner" | ||
|
||
map_in = "${var.input}" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<3