Skip to content

Commit

Permalink
catch missing id attribute during interpolation
Browse files Browse the repository at this point in the history
The id attribute can be missing during the destroy operation.
While the new destroy-time ordering of outputs and locals should prevent
resources from having their id attributes set to an empty string,
there's no reason to error out if we have the canonical ID field
available.

This still interrogates the attributes map first to retain any previous
behavior, but in the future we should settle on a single ID location.
  • Loading branch information
jbardin committed Jan 30, 2018
1 parent 99867f0 commit a2f8482
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 2 deletions.
15 changes: 15 additions & 0 deletions terraform/interpolate.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,16 @@ func (i *Interpolater) computeResourceVariable(
return &v, err
}

// special case for the "id" field which is usually also an attribute
if v.Field == "id" && r.Primary.ID != "" {
// This is usually pulled from the attributes, but is sometimes missing
// during destroy. We can return the ID field in this case.
// FIXME: there should only be one ID to rule them all.
log.Printf("[WARN] resource %s missing 'id' attribute", v.ResourceId())
v, err := hil.InterfaceToVariable(r.Primary.ID)
return &v, err
}

// computed list or map attribute
_, isList = r.Primary.Attributes[v.Field+".#"]
_, isMap = r.Primary.Attributes[v.Field+".%"]
Expand Down Expand Up @@ -655,6 +665,11 @@ func (i *Interpolater) computeResourceMultiVariable(
continue
}

if v.Field == "id" && r.Primary.ID != "" {
log.Printf("[WARN] resource %s missing 'id' attribute", v.ResourceId())
values = append(values, r.Primary.ID)
}

// computed list or map attribute
_, isList := r.Primary.Attributes[v.Field+".#"]
_, isMap := r.Primary.Attributes[v.Field+".%"]
Expand Down
38 changes: 36 additions & 2 deletions terraform/interpolate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,40 @@ func TestInterpolater_localVal(t *testing.T) {
})
}

func TestInterpolater_missingID(t *testing.T) {
lock := new(sync.RWMutex)
state := &State{
Modules: []*ModuleState{
&ModuleState{
Path: rootModulePath,
Resources: map[string]*ResourceState{
"aws_instance.web": &ResourceState{
Type: "aws_instance",
Primary: &InstanceState{
ID: "bar",
},
},
},
},
},
}

i := &Interpolater{
Module: testModule(t, "interpolate-resource-variable"),
State: state,
StateLock: lock,
}

scope := &InterpolationScope{
Path: rootModulePath,
}

testInterpolate(t, i, scope, "aws_instance.web.id", ast.Variable{
Value: "bar",
Type: ast.TypeString,
})
}

func TestInterpolater_pathCwd(t *testing.T) {
i := &Interpolater{}
scope := &InterpolationScope{}
Expand Down Expand Up @@ -314,8 +348,8 @@ func TestInterpolater_resourceVariableMissingDuringInput(t *testing.T) {
&ModuleState{
Path: rootModulePath,
Resources: map[string]*ResourceState{
// No resources at all yet, because we're still dealing
// with input and so the resources haven't been created.
// No resources at all yet, because we're still dealing
// with input and so the resources haven't been created.
},
},
},
Expand Down

0 comments on commit a2f8482

Please sign in to comment.