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

Jump to bottom

[WIP] helper/schema: implement ComputedWhen #4846

Closed
wants to merge 1 commit into from
Closed

[WIP] helper/schema: implement ComputedWhen #4846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions builtin/providers/atlas/resource_artifact.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ func resourceArtifact() *schema.Resource {
},

"metadata_full": &schema.Schema{
Type: schema.TypeMap,
Computed: true,
Type: schema.TypeMap,
Computed: true,
ComputedWhen: []string{"name", "build", "version"},
},

"slug": &schema.Schema{
Expand Down
5 changes: 3 additions & 2 deletions builtin/providers/consul/resource_consul_keys.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,9 @@ func resourceConsulKeys() *schema.Resource {
},

"var": &schema.Schema{
Type: schema.TypeMap,
Computed: true,
Type: schema.TypeMap,
Computed: true,
ComputedWhen: []string{"key"},
},
},
}
Expand Down
80 changes: 78 additions & 2 deletions helper/schema/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,18 @@ func (s *Schema) DefaultValue() (interface{}, error) {
return nil, nil
}

// MatchesKey returns whether or not the state key
// matches the schema key this schema type
func (s *Schema) KeysMatch(schemaKey, stateKey string) bool {
if s == nil {
return false
}
if s.Type.IsPrimitive() {
return schemaKey == stateKey
}
return strings.HasPrefix(stateKey, schemaKey)
}

// Returns a zero value for the schema.
func (s *Schema) ZeroValue() interface{} {
// If it's a set then we'll do a bit of extra work to provide the
Expand Down Expand Up @@ -366,9 +378,62 @@ func (m schemaMap) Diff(
}
}

// Go through and detect all of the ComputedWhens now that we've
// Go through and detect/handle all of the ComputedWhens now that we've
// finished the diff.
// TODO
for k, schema := range m {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here be dragons. Lots of looping logic below. Would love to come up with a more elegant way to do this. Considered attempting to thread ComputedWhen knowledge/context into the original diff generation rather than this post-hoc munging, but initial spikes felt even more messy than this.

for _, targetKey := range schema.ComputedWhen {
targetSchema := m[targetKey]
targetChanged := false
for diffKey := range result.Attributes {
if targetSchema.KeysMatch(targetKey, diffKey) {
targetChanged = true
break
}
}

if targetChanged {
// Then the ComputedWhen field should be computed
if schema.Type.IsPrimitive() {
// For primitives, we just tick on the NewComputed bit
d := &terraform.ResourceAttrDiff{}
if v, ok := result.Attributes[k]; ok {
d = v
}
d.NewComputed = true
result.Attributes[k] = d
} else {
// For list-like types, we mark the whole list as computed,
countAttrDiff := &terraform.ResourceAttrDiff{}
if v, ok := result.Attributes[k+".#"]; ok {
countAttrDiff = v
}
countAttrDiff.NewComputed = true

// then clear out any existing field-level diffs,
for diffKey := range result.Attributes {
if strings.HasPrefix(diffKey, k) {
delete(result.Attributes, diffKey)
}
}

result.Attributes[k+".#"] = countAttrDiff

// add back in current -> <computed> diffs for any of this
// attribute's fields currently in the state
if s != nil {
for stateKey, stateVal := range s.Attributes {
if strings.HasPrefix(stateKey, k) {
result.Attributes[stateKey] = &terraform.ResourceAttrDiff{
Old: stateVal,
NewComputed: true,
}
}
}
}
}
}
}
}

if result.Empty() {
// If we don't have any diff elements, just return nil
Expand Down Expand Up @@ -1293,3 +1358,14 @@ func (t ValueType) Zero() interface{} {
panic(fmt.Sprintf("unknown type %s", t))
}
}

// IsPrimitive returns true if the ValueType can be considered "primitive".
// i.e. It does not require dot-notation or multiple fields to store it.
func (t ValueType) IsPrimitive() bool {
switch t {
case TypeBool, TypeInt, TypeFloat, TypeString:
return true
default:
return false
}
}
141 changes: 128 additions & 13 deletions helper/schema/schema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1164,7 +1164,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Err: false,
},

"#30 ComputedWhen": {
"#30 ComputedWhen does nothing when there is no diff on target property": {
Schema: map[string]*Schema{
"availability_zone": &Schema{
Type: TypeString,
Expand Down Expand Up @@ -1194,7 +1194,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Err: false,
},

"#31": {
"#31 ComputedWhen doesn't interfere with normal Computed behavior": {
Schema: map[string]*Schema{
"availability_zone": &Schema{
Type: TypeString,
Expand Down Expand Up @@ -1229,8 +1229,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Err: false,
},

/* TODO
{
"The ComputedWhen attr should be computed when the target attr changes": {
Schema: map[string]*Schema{
"availability_zone": &Schema{
Type: TypeString,
Expand All @@ -1255,10 +1254,9 @@ func TestSchemaMap_Diff(t *testing.T) {
"port": 8080,
},

Diff: &terraform.ResourceDiff{
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"availability_zone": &terraform.ResourceAttrDiff{
Old: "foo",
NewComputed: true,
},
"port": &terraform.ResourceAttrDiff{
Expand All @@ -1270,9 +1268,126 @@ func TestSchemaMap_Diff(t *testing.T) {

Err: false,
},
*/

"#32 Maps": {
"ComputedWhen properly marks TypeMap as computed when the target attr changes": {
Schema: map[string]*Schema{
"mapping": &Schema{
Type: TypeMap,
Computed: true,
ComputedWhen: []string{"port"},
},

"port": &Schema{
Type: TypeInt,
Optional: true,
},
},

State: &terraform.InstanceState{
Attributes: map[string]string{
"mapping.#": "2",
"mapping.foo": "123",
"mapping.bar": "456",
"port": "80",
},
},

Config: map[string]interface{}{
"port": 8080,
},

Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"mapping.#": &terraform.ResourceAttrDiff{
Old: "2",
NewComputed: true,
},
"mapping.foo": &terraform.ResourceAttrDiff{
Old: "123",
NewComputed: true,
},
"mapping.bar": &terraform.ResourceAttrDiff{
Old: "456",
NewComputed: true,
},
"port": &terraform.ResourceAttrDiff{
Old: "80",
New: "8080",
},
},
},

Err: false,
},

"ComputedWhen works when target is a complex set": {
Schema: map[string]*Schema{
"mapping": &Schema{
Type: TypeMap,
Computed: true,
ComputedWhen: []string{"config"},
},

"config": &Schema{
Type: TypeSet,
Optional: true,
Elem: &Resource{
Schema: map[string]*Schema{
"name": &Schema{
Type: TypeString,
Required: true,
},
},
},
},
},

State: &terraform.InstanceState{
Attributes: map[string]string{
"mapping.#": "2",
"mapping.foo": "123",
"mapping.bar": "456",
"config.#": "1",
"config.965232600.name": "foo",
},
},

Config: map[string]interface{}{
"config": []map[string]interface{}{
map[string]interface{}{
"name": "bar",
},
},
},

Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"mapping.#": &terraform.ResourceAttrDiff{
Old: "2",
NewComputed: true,
},
"mapping.foo": &terraform.ResourceAttrDiff{
Old: "123",
NewComputed: true,
},
"mapping.bar": &terraform.ResourceAttrDiff{
Old: "456",
NewComputed: true,
},
"config.965232600.name": &terraform.ResourceAttrDiff{
Old: "foo",
NewRemoved: true,
},
"config.1125683609.name": &terraform.ResourceAttrDiff{
New: "bar",
},
},
},

Err: false,
},

"Map, config and no state": {
Schema: map[string]*Schema{
"config_vars": &Schema{
Type: TypeMap,
Expand Down Expand Up @@ -1306,7 +1421,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Err: false,
},

"#33 Maps": {
"Map, changing keys": {
Schema: map[string]*Schema{
"config_vars": &Schema{
Type: TypeMap,
Expand Down Expand Up @@ -1343,7 +1458,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Err: false,
},

"#34 Maps": {
"Computed Map, changing keys": {
Schema: map[string]*Schema{
"vars": &Schema{
Type: TypeMap,
Expand Down Expand Up @@ -1383,7 +1498,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Err: false,
},

"#35 Maps": {
"Computed Map, no diff": {
Schema: map[string]*Schema{
"vars": &Schema{
Type: TypeMap,
Expand All @@ -1404,7 +1519,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Err: false,
},

"#36 Maps": {
"List of Maps, changing keys": {
Schema: map[string]*Schema{
"config_vars": &Schema{
Type: TypeList,
Expand Down Expand Up @@ -1443,7 +1558,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Err: false,
},

"#37 Maps": {
"List of Maps, removed from config": {
Schema: map[string]*Schema{
"config_vars": &Schema{
Type: TypeList,
Expand Down