Skip to content

Commit

Permalink
Revert "Fix permadiff that reorders stateful_external_ip blocks on …
Browse files Browse the repository at this point in the history 
…`google_compute_instance_group_manager` and `google_compute_region_instance_group_manager` resources" (GoogleCloudPlatform#9754)

This reverts commit de43d70.
  • Loading branch information
@ScottSuarez
ScottSuarez authored Jan 4, 2024
1 parent 89f2396 commit 24ff156
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 460 deletions.
70 changes: 19 additions & 51 deletions mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager.go.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -778,10 +778,10 @@ func resourceComputeInstanceGroupManagerRead(d *schema.ResourceData, meta interf
if err = d.Set("stateful_disk", flattenStatefulPolicy(manager.StatefulPolicy)); err != nil {
return fmt.Errorf("Error setting stateful_disk in state: %s", err.Error())
}
if err = d.Set("stateful_internal_ip", flattenStatefulPolicyStatefulInternalIps(d, manager.StatefulPolicy)); err != nil {
if err = d.Set("stateful_internal_ip", flattenStatefulPolicyStatefulInternalIps(manager.StatefulPolicy)); err != nil {
return fmt.Errorf("Error setting stateful_internal_ip in state: %s", err.Error())
}
if err = d.Set("stateful_external_ip", flattenStatefulPolicyStatefulExternalIps(d, manager.StatefulPolicy)); err != nil {
if err = d.Set("stateful_external_ip", flattenStatefulPolicyStatefulExternalIps(manager.StatefulPolicy)); err != nil {
return fmt.Errorf("Error setting stateful_external_ip in state: %s", err.Error())
}
if err := d.Set("fingerprint", manager.Fingerprint); err != nil {
Expand Down Expand Up @@ -1306,68 +1306,36 @@ func flattenStatefulPolicy(statefulPolicy *compute.StatefulPolicy) []map[string]
return result
}

func flattenStatefulPolicyStatefulInternalIps(d *schema.ResourceData, statefulPolicy *compute.StatefulPolicy) []map[string]interface{} {
func flattenStatefulPolicyStatefulInternalIps(statefulPolicy *compute.StatefulPolicy) []map[string]interface{} {
if statefulPolicy == nil || statefulPolicy.PreservedState == nil || statefulPolicy.PreservedState.InternalIPs == nil {
return make([]map[string]interface{}, 0, 0)
}
result := make([]map[string]interface{}, 0, len(statefulPolicy.PreservedState.InternalIPs))
for interfaceName, internalIp := range statefulPolicy.PreservedState.InternalIPs {
data := map[string]interface{}{
"interface_name": interfaceName,
"delete_rule": internalIp.AutoDelete,
}

return flattenStatefulPolicyStatefulIps(d, "stateful_internal_ip", statefulPolicy.PreservedState.InternalIPs)
}

func flattenStatefulPolicyStatefulExternalIps(d *schema.ResourceData, statefulPolicy *compute.StatefulPolicy) []map[string]interface{} {
if statefulPolicy == nil || statefulPolicy.PreservedState == nil || statefulPolicy.PreservedState.ExternalIPs == nil {
return make([]map[string]interface{}, 0)
result = append(result, data)
}

return flattenStatefulPolicyStatefulIps(d, "stateful_external_ip", statefulPolicy.PreservedState.ExternalIPs)
return result
}

func flattenStatefulPolicyStatefulIps(d *schema.ResourceData, ipfieldName string, ips map[string]compute.StatefulPolicyPreservedStateNetworkIp) []map[string]interface{} {

// statefulPolicy.PreservedState.ExternalIPs and statefulPolicy.PreservedState.InternalIPs are affected by API-side reordering
// of external/internal IPs, where ordering is done by the interface_name value.
// Below we intend to reorder the IPs to match the order in the config.
// Also, data is converted from a map (client library's statefulPolicy.PreservedState.ExternalIPs, or .InternalIPs) to a slice (stored in state).
// Any IPs found from the API response that aren't in the config are appended to the end of the slice.

configIpOrder := d.Get(ipfieldName).([]interface{})
order := map[string]int{} // record map of interface name to index
for i, el := range configIpOrder {
ip := el.(map[string]interface{})
interfaceName := ip["interface_name"].(string)
order[interfaceName] = i
func flattenStatefulPolicyStatefulExternalIps(statefulPolicy *compute.StatefulPolicy) []map[string]interface{} {
if statefulPolicy == nil || statefulPolicy.PreservedState == nil || statefulPolicy.PreservedState.ExternalIPs == nil {
return make([]map[string]interface{}, 0, 0)
}

orderedResult := make([]map[string]interface{}, len(configIpOrder))
unexpectedIps := []map[string]interface{}{}
for interfaceName, ip := range ips {
result := make([]map[string]interface{}, 0, len(statefulPolicy.PreservedState.ExternalIPs))
for interfaceName, externalIp := range statefulPolicy.PreservedState.ExternalIPs {
data := map[string]interface{}{
"interface_name": interfaceName,
"delete_rule": ip.AutoDelete,
}

index, found := order[interfaceName]
if !found {
unexpectedIps = append(unexpectedIps, data)
continue
"delete_rule": externalIp.AutoDelete,
}
orderedResult[index] = data // Put elements from API response in order that matches the config
}

// Remove any nils from the ordered list. This can occur if the API doesn't include an interface present in the config.
finalResult := []map[string]interface{}{}
for _, item := range orderedResult {
if item != nil {
finalResult = append(finalResult, item)
}
}

if len(unexpectedIps) > 0 {
// Additional IPs returned from API but not in the config are appended to the end of the slice
finalResult = append(finalResult, unexpectedIps...)
result = append(result, data)
}

return finalResult
return result
}

func flattenUpdatePolicy(updatePolicy *compute.InstanceGroupManagerUpdatePolicy) []map[string]interface{} {
Expand Down
78 changes: 78 additions & 0 deletions ...party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package compute

import (
"testing"
)

func TestInstanceGroupManager_parseUniqueId(t *testing.T) {
expectations := map[string][]string{
"projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": {"projects/imre-test/global/instanceTemplates/example-template-custom", "123"},
"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": {"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", "123"},
"projects/imre-test/global/instanceTemplates/example-template-custom": {"projects/imre-test/global/instanceTemplates/example-template-custom", ""},
"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": {"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", ""},
"example-template-custom?uniqueId=123": {"example-template-custom", "123"},

// this test demonstrates that uniqueIds can't override eachother
"projects/imre-test/global/instanceTemplates/example?uniqueId=123?uniqueId=456": {"projects/imre-test/global/instanceTemplates/example", "123?uniqueId=456"},
}

for k, v := range expectations {
aName, aUniqueId := parseUniqueId(k)
if v[0] != aName {
t.Errorf("parseUniqueId failed; name of %v should be %v, not %v", k, v[0], aName)
}
if v[1] != aUniqueId {
t.Errorf("parseUniqueId failed; uniqueId of %v should be %v, not %v", k, v[1], aUniqueId)
}
}
}

func TestInstanceGroupManager_compareInstanceTemplate(t *testing.T) {
shouldAllMatch := []string{
// uniqueId not present
"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom",
"projects/imre-test/global/instanceTemplates/example-template-custom",
// uniqueId present
"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123",
"projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123",
}
shouldNotMatch := map[string]string{
// mismatching name
"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": "projects/imre-test/global/instanceTemplates/example-template-custom2",
"projects/imre-test/global/instanceTemplates/example-template-custom": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom2",
// matching name, but mismatching uniqueId
"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=1234",
"projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=1234",
}
for _, v1 := range shouldAllMatch {
for _, v2 := range shouldAllMatch {
if !compareSelfLinkRelativePathsIgnoreParams("", v1, v2, nil) {
t.Fatalf("compareSelfLinkRelativePathsIgnoreParams did not match (and should have) %v and %v", v1, v2)
}
}
}

for v1, v2 := range shouldNotMatch {
if compareSelfLinkRelativePathsIgnoreParams("", v1, v2, nil) {
t.Fatalf("compareSelfLinkRelativePathsIgnoreParams did match (and shouldn't) %v and %v", v1, v2)
}
}
}

func TestInstanceGroupManager_convertUniqueId(t *testing.T) {
matches := map[string]string{
// uniqueId not present (should return the same)
"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom",
"projects/imre-test/global/instanceTemplates/example-template-custom": "projects/imre-test/global/instanceTemplates/example-template-custom",
// uniqueId present (should return the last component replaced)
"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/123",
"projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "projects/imre-test/global/instanceTemplates/123",
"tf-test-igm-8amncgtq22?uniqueId=8361222501423044003": "8361222501423044003",
}
for input, expected := range matches {
actual := ConvertToUniqueIdWhenPresent(input)
if actual != expected {
t.Fatalf("invalid return value by ConvertToUniqueIdWhenPresent for input %v; expected: %v, actual: %v", input, expected, actual)
}
}
}
Loading

0 comments on commit 24ff156

Please sign in to comment.