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.

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

Support unordered lists in Terraform. #351

Merged
merged 9 commits into from
Jul 17, 2018
2 changes: 2 additions & 0 deletions products/compute/terraform.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,8 @@ overrides: !ruby/object:Provider::ResourceOverrides
function: 'validateGCPName'
secondaryIpRanges: !ruby/object:Provider::Terraform::PropertyOverride
name: secondaryIpRange
unordered_list: true
default_from_api: true
Copy link
Contributor

Choose a reason for hiding this comment

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

why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem comes from the fact that we're implicitly comparing against the returned value from the API - we wouldn't have any diff problems otherwise.

secondaryIpRanges.rangeName: !ruby/object:Provider::Terraform::PropertyOverride
validation: !ruby/object:Provider::Terraform::Validation
function: 'validateGCPName'
Expand Down
3 changes: 3 additions & 0 deletions provider/terraform/property_override.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ module OverrideFields
attr_reader :state_func # Adds a StateFunc to the schema
attr_reader :sensitive # Adds `Sensitive: true` to the schema
attr_reader :validation # Adds a ValidateFunc to the schema
# Indicates that this is an Array that should have Set diff semantics.
attr_reader :unordered_list

attr_reader :is_set # Uses a Set instead of an Array
# Optional function to determine the unique ID of an item in the set
Expand Down Expand Up @@ -92,6 +94,7 @@ def validate
# Ensures boolean values are set to false if nil
@sensitive ||= false
@is_set ||= false
@unordered_list ||= false
@default_from_api ||= false

check_property :sensitive, :boolean
Expand Down
14 changes: 14 additions & 0 deletions templates/terraform/resource.erb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ func resource<%= resource_name -%>() *schema.Resource {
Update: resource<%= resource_name -%>Update,
<% end -%>
Delete: resource<%= resource_name -%>Delete,
<% if settable_properties.any? {|p| p.unordered_list} && !object.custom_code.resource_definition -%>
CustomizeDiff: resource<%= resource_name -%>CustomizeDiff,
<% end -%>

Importer: &schema.ResourceImporter{
State: resource<%= resource_name -%>Import,
Expand Down Expand Up @@ -74,6 +77,17 @@ func resource<%= resource_name -%>() *schema.Resource {
},
}
}
<% if settable_properties.any? {|p| p.unordered_list} -%>
func resource<%= resource_name -%>CustomizeDiff(diff *schema.ResourceDiff, meta interface{}) error {
<% settable_properties.select {|p| p.unordered_list}.each do |prop| -%>
<%= compile_template('templates/terraform/unordered_list_customize_diff.erb',
prop: prop,
resource_name: resource_name) -%>
<% end -%>

return nil
}
<% end -%>

func resource<%= resource_name -%>Create(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)
Expand Down
1 change: 1 addition & 0 deletions templates/terraform/resource_definition/subnetwork.erb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
CustomizeDiff: customdiff.All(
customdiff.ForceNewIfChange("ip_cidr_range", isShrinkageIpCidr),
resourceComputeSubnetworkCustomizeDiff,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: mind making this line tabs or the previous line spaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

),
44 changes: 44 additions & 0 deletions templates/terraform/unordered_list_customize_diff.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
keys := diff.GetChangedKeysPrefix(<%= go_literal(Google::StringUtils.underscore(prop.name)) -%>)
if len(keys) < 1 {
return nil
}

oldCount, newCount := diff.GetChange("<%= Google::StringUtils.underscore(prop.name) -%>.#")
var count int
if oldCount.(int) < newCount.(int) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to return here if the counts aren't the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duplicates may exist. Added a comment.

count = newCount.(int)
} else {
count = oldCount.(int)
}
if count < 1 {
return nil
}

if count < 1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the same as the lines immediately before it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

return nil
}
old := make([]interface{}, count)
new := make([]interface{}, count)
for i := 0; i < count; i++ {
o, n := diff.GetChange(fmt.Sprintf("<%= Google::StringUtils.underscore(prop.name) -%>.%d", i))

if o != nil {
old = append(old, o)
}
if n != nil {
new = append(new, n)
}
}

if len(old) != len(new) {
return nil
}

oldSet := schema.NewSet(schema.HashResource(resource<%= resource_name -%>().Schema[<%= go_literal(Google::StringUtils.underscore(prop.name)) -%>].Elem.(*schema.Resource)), old)
newSet := schema.NewSet(schema.HashResource(resource<%= resource_name -%>().Schema[<%= go_literal(Google::StringUtils.underscore(prop.name)) -%>].Elem.(*schema.Resource)), new)

if oldSet.Equal(newSet) {
if err := diff.Clear(<%= go_literal(Google::StringUtils.underscore(prop.name)) -%>); err != nil {
return err
}
}