Skip to content
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.

Sign up for GitHub

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 Block need handle attribute marked as removed contionally. #359

Open
wants to merge 2 commits into
base: v1-maint
Choose a base branch
from
Open

Type Block need handle attribute marked as removed contionally. #359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b6e3d58
Type Block need handle attribute marked as removed contionally.
magodo Mar 16, 2020
d38953e
Update the zero value comparison logic.
magodo Jul 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions helper/schema/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1305,13 +1305,54 @@ func (m schemaMap) diffSet(
switch t := schema.Elem.(type) {
case *Resource:
// This is a complex resource

// As long as one of the attribute is not marked to be removed, we should unmark the other attributes
// that are marked to be removed just because their new value is not specified in config and their
// old value is the zero value.
isAllSubKNewRemoved := true

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 {
isAllSubKNewRemoved = false
}
}
if !isAllSubKNewRemoved {
for k2 := range t.Schema {
subK := fmt.Sprintf("%s.%s.%s", k, code, k2)
if subV, ok := diff.Attributes[subK]; ok && subV.NewRemoved {
schemaList := addrToSchema(strings.Split(subK, "."), map[string]*Schema{k: schema})
if len(schemaList) == 0 {
continue
}
subSchema := schemaList[len(schemaList)-1]

var verb string
switch subSchema.Type {
case TypeBool:
verb = "%t"
case TypeInt:
verb = "%d"
case TypeFloat:
verb = "%f"
case TypeString:
verb = "%s"
default:
return fmt.Errorf("%s: unknown type %#v", k, schema.Type)
}
if fmt.Sprintf(verb, subSchema.ZeroValue()) == subV.Old {
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
150 changes: 150 additions & 0 deletions helper/schema/schema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3161,6 +3161,156 @@ func TestSchemaMap_Diff(t *testing.T) {
},
},
},
{
Name: "Set element with unset Optional attributes should not be affected when another element get updated",
Schema: map[string]*Schema{
"foo": {
Type: TypeSet,
Required: true,
Elem: &Resource{
Schema: map[string]*Schema{
"a": {
Type: TypeInt,
Required: true,
},
"b": {
Type: TypeInt,
Optional: true,
},
},
},
Set: func(v interface{}) int {
m := v.(map[string]interface{})
return m["a"].(int) + m["b"].(int)
},
},
},

State: &terraform.InstanceState{
Attributes: map[string]string{
"foo.#": "2",
"foo.1.a": "1",
"foo.1.b": "0",
"foo.2.a": "2",
"foo.2.b": "0",
},
},

Config: map[string]interface{}{
"foo": []interface{}{
map[string]interface{}{
"a": 1,
},
map[string]interface{}{
"a": 3,
},
},
},

Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"foo.1.a": {
Old: "1",
New: "1",
},
"foo.1.b": {
Old: "0",
New: "0",
},
"foo.2.a": {
Old: "2",
New: "0",
NewRemoved: true,
},
"foo.2.b": {
Old: "0",
New: "0",
NewRemoved: true,
},
"foo.3.a": {
Old: "",
New: "3",
},
"foo.3.b": {
Old: "",
New: "",
},
},
},

Err: false,
},
{
Name: "Set element with unset Optional attributes should not be affected when new element get added",
Schema: map[string]*Schema{
"foo": {
Type: TypeSet,
Required: true,
Elem: &Resource{
Schema: map[string]*Schema{
"a": {
Type: TypeInt,
Required: true,
},
"b": {
Type: TypeInt,
Optional: true,
},
},
},
Set: func(v interface{}) int {
m := v.(map[string]interface{})
return m["a"].(int) + m["b"].(int)
},
},
},

State: &terraform.InstanceState{
Attributes: map[string]string{
"foo.#": "1",
"foo.1.a": "1",
"foo.1.b": "0",
},
},

Config: map[string]interface{}{
"foo": []interface{}{
map[string]interface{}{
"a": 1,
},
map[string]interface{}{
"a": 2,
},
},
},

Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"foo.#": {
Old: "1",
New: "2",
},
"foo.1.a": {
Old: "1",
New: "1",
},
"foo.1.b": {
Old: "0",
New: "0",
},
"foo.2.a": {
Old: "",
New: "2",
},
"foo.2.b": {
Old: "",
New: "",
},
},
},

Err: false,
},
}

for i, tc := range cases {
Expand Down