Skip to content

Commit

Permalink
Type Block need handle attribute marked as removed contionally.
Browse files Browse the repository at this point in the history
In some provider, e.g. AzureRM, TypeSet is use quite prevalent to
hold a set of sub-resources (e.g. security_rule in azurerm_network_security_group).
The reason for using TypeSet instead of TypeList might because service
return a collection of info with arbitrary order, or other reasons.

In this case: Given a slice of nested blocks, if one of the nested block (B1)
attributes is optional and has no default value, and user didn't specify a value
for it. Then if another nested block (B2) changed, which triggers some diff,
then B1 will also be replaced. That is because the optional attribute
contribute a diff of "zero value" -> `null`, which changed the hash of
the block.

This fix is to carefully handle this case. We keep attribute marked
as `NewRemoved` after a `diffString` only when all the attributes are
marked as such. Otherwise, as long as one attribute is not marked to be
removed, that means this block will be kept. Then we will manipulate the
attributes in this block, which being marked as removed just because
user didn't specify a value and the original value is the zero value.

This keeps consistent as other Resource types (e.g. List Resource).
(Though those type just remove the attribute from diff set, while for
Set we need to return the complete state as we will not depend on the
old state during diff apply).
  • Loading branch information
magodo committed Mar 16, 2020
1 parent d3b1ac9 commit c2ff9a0
Showing 1 changed file with 25 additions and 0 deletions.
25 changes: 25 additions & 0 deletions helper/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -1269,14 +1269,39 @@ func (m schemaMap) diffSet(
for _, code := range list {
switch t := schema.Elem.(type) {
case *Resource:

// For resource block, we need to check whether the whole resource should be removed.
// If one of the attribute is not marked to be removed, than we need to manipulate
// those be marked just because their new value is not specified and the old value is
// the zero value. So as to keep consistent as other Resource types (e.g. List Resource).
// (Though those type just remove the attribute from diff set, while for Set we need to return the
// complete state as we will not depend on the old state during diff apply).
isSubKNewRemoved := true

// This is a complex resource
for k2, schema := range t.Schema {
subK := fmt.Sprintf("%s.%s.%s", k, code, k2)
err := m.diff(subK, schema, diff, d, true)
if err != nil {
return err
}

if subV, ok := diff.Attributes[subK]; ok && !subV.NewRemoved {
isSubKNewRemoved = false
}
}
if !isSubKNewRemoved {
for k2 := range t.Schema {
subK := fmt.Sprintf("%s.%s.%s", k, code, k2)
if subV, ok := diff.Attributes[subK]; ok && subV.NewRemoved {
if subV.NewRemoved && subV.Old == schema.ZeroValue() {
subV.NewRemoved = false
subV.New = subV.Old
}
}
}
}

case *Schema:
// Copy the schema so that we can set Computed/ForceNew from
// the parent schema (the TypeSet).
Expand Down

0 comments on commit c2ff9a0

Please sign in to comment.