aherman
/
terraform
mirror of https://github.com/hashicorp/terraform
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

actions: return an error if config is omitted but the schema has required attrs (#37626)

Browse Source 
* actions: improve handling of null config so we can properly report missing required arguments

There are various methods in terraform that will let you know if you are missing required attributes - none of which work with nil inputs. This commit changes the handling, passing in an empty object if the config was null, and adding additional context to the error message when the config block is missing.
pull/37645/head
Kristin Laemmert 8 months ago committed by GitHub
parent 0dfa115d7d
commit e241e1ace1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 135 additions and 66 deletions
Show Stats Download Patch File Download Diff File

172
internal/terraform/context_validate_test.go
Unescape View File

@ -3575,60 +3575,114 @@ func TestContext2Validate_queryList(t *testing.T) {
// Action Validation is largely exercised in context_plan_actions_test.go
func TestContext2Validate_action(t *testing.T) {
tests := map[string]struct {
config string
wantErr string
config string
wantErr string
expectValidateCalled bool
}{
"valid config": {
`
terraform {
required_providers {
test = {
source = "hashicorp/test"
version = "1.0.0"
terraform {
required_providers {
test = {
source = "hashicorp/test"
version = "1.0.0"
}
}
}
action "test_register" "foo" {
config {
host = "cmdb.snot"
}
}
resource "test_instance" "foo" {
lifecycle {
action_trigger {
events = [after_create]
actions = [action.test_register.foo]
}
}
}
`,
"",
true,
},
"validly null config": { // this doesn't seem likely, but let's make sure nothing panics.
`
terraform {
required_providers {
test = {
source = "hashicorp/test"
version = "1.0.0"
}
}
}
}
}
action "test_register" "foo" {
config {
host = "cmdb.snot"
}
}
resource "test_instance" "foo" {
lifecycle {
action_trigger {
events = [after_create]
actions = [action.test_register.foo]
}
}
}
`,
action "test_other" "foo" {
}
resource "test_instance" "foo" {
lifecycle {
action_trigger {
events = [after_create]
actions = [action.test_other.foo]
}
}
}
`,
"",
true,
},
"missing required config": {
`
terraform {
required_providers {
test = {
source = "hashicorp/test"
version = "1.0.0"
terraform {
required_providers {
test = {
source = "hashicorp/test"
version = "1.0.0"
}
}
}
}
}
action "test_register" "foo" {
config {}
}
resource "test_instance" "foo" {
lifecycle {
action_trigger {
events = [after_create]
actions = [action.test_register.foo]
}
}
}
`,
"host is null",
action "test_register" "foo" {
config {}
}
resource "test_instance" "foo" {
lifecycle {
action_trigger {
events = [after_create]
actions = [action.test_register.foo]
}
}
}
`,
"Missing required argument: The argument \"host\" is required, but no definition was found.",
false,
},
"invalid required config (provider validation)": {
`
terraform {
required_providers {
test = {
source = "hashicorp/test"
version = "1.0.0"
}
}
}
action "test_register" "foo" {
config {
host = "invalid"
}
}
resource "test_instance" "foo" {
lifecycle {
action_trigger {
events = [after_create]
actions = [action.test_register.foo]
}
}
}
`,
"Invalid value for required argument \"host\" because I said so",
true,
},
"invalid nil config config": {
"invalid nil config": {
`
terraform {
required_providers {
@ -3649,7 +3703,8 @@ resource "test_instance" "foo" {
}
}
`,
"config is null",
"Missing required argument: The argument \"host\" is required, but was not set.",
false,
},
}
@ -3669,6 +3724,14 @@ resource "test_instance" "foo" {
},
Actions: map[string]*providers.ActionSchema{
"test_register": {
ConfigSchema: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"host": {Type: cty.String, Required: true},
"output": {Type: cty.String, Computed: true, Optional: true},
},
},
},
"test_other": {
ConfigSchema: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"host": {Type: cty.String, Optional: true},
@ -3678,12 +3741,8 @@ resource "test_instance" "foo" {
},
})
p.ValidateActionConfigFn = func(r providers.ValidateActionConfigRequest) (resp providers.ValidateActionConfigResponse) {
if r.Config.IsNull() {
resp.Diagnostics = resp.Diagnostics.Append(errors.New("config is null"))
return
}
if r.Config.GetAttr("host").IsNull() {
resp.Diagnostics = resp.Diagnostics.Append(errors.New("host is null"))
if r.Config.GetAttr("host") == cty.StringVal("invalid") {
resp.Diagnostics = resp.Diagnostics.Append(errors.New("Invalid value for required argument \"host\" because I said so"))
}
return
}
@ -3695,10 +3754,6 @@ resource "test_instance" "foo" {
})
diags := ctx.Validate(m, nil)
if !p.ValidateActionConfigCalled {
t.Fatal("ValidateAction RPC was not called")
}
if test.wantErr != "" {
if !diags.HasErrors() {
t.Errorf("unexpected success\nwant errors: %s", test.wantErr)
@ -3710,6 +3765,13 @@ resource "test_instance" "foo" {
t.Errorf("unexpected error\ngot: %s", diags.Err().Error())
}
}
if p.ValidateActionConfigCalled != test.expectValidateCalled {
if test.expectValidateCalled {
t.Fatal("provider Validate RPC was expected, but not called")
} else {
t.Fatal("unexpected Validate RCP call")
}
}
})
}
}

5
internal/terraform/node_action_abstract.go
Unescape View File

@ -44,9 +44,8 @@ func (n NodeAbstractAction) Name() string {
// abstract action to a concrete one of some type.
type ConcreteActionNodeFunc func(*NodeAbstractAction) dag.Vertex
// I'm not sure why my ConcreteActionNodeFUnction kept being nil in tests, but
// this is much more robust. If it isn't a validate walk, we need
// nodeExpandActionDeclaration.
// DefaultConcreteActionNodeFunc is the default ConcreteActionNodeFunc used by
// everything except validate.
func DefaultConcreteActionNodeFunc(a *NodeAbstractAction) dag.Vertex {
return &nodeExpandActionDeclaration{
NodeAbstractAction: a,

24
internal/terraform/node_action_validate.go
Unescape View File

@ -82,16 +82,24 @@ func (n *NodeValidatableAction) Execute(ctx EvalContext, _ walkOperation) tfdiag
return diags
}
var configVal cty.Value
var valDiags tfdiags.Diagnostics
if n.Config.Config != nil {
configVal, _, valDiags = ctx.EvaluateBlock(n.Config.Config, schema.ConfigSchema, nil, keyData)
diags = diags.Append(valDiags)
if valDiags.HasErrors() {
config := n.Config.Config
if n.Config.Config == nil {
config = hcl.EmptyBody()
}
configVal, _, valDiags := ctx.EvaluateBlock(config, schema.ConfigSchema, nil, keyData)
if valDiags.HasErrors() {
// If there was no config block at all, we'll add a Context range to the returned diagnostic
if n.Config.Config == nil {
for _, diag := range valDiags.ToHCL() {
diag.Context = &n.Config.DeclRange
diags = diags.Append(diag)
}
return diags
} else {
diags = diags.Append(valDiags)
return diags
}
} else {
configVal = cty.NullVal(n.Schema.ConfigSchema.ImpliedType())
}
// Use unmarked value for validate request

Write Preview
Loading…
Cancel
Save