VCR body comparison fixes (#3461) (#2047)

* Remove body comparison during VCR matching if content type is multipart

* Fix other tests for vcr-style

* Fix ssl cert test, IDP tests for VCR

* Add json comparison

* Sorts! Deterministic ordering of IAM bindings and compute metadata for VCR testing

* Introduce sort to stringSliceFromGolangSet

* Sort string list after set conversion

* Tabs v spaces

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

.changelog/3461.txt

@@ -0,0 +1,3 @@
+```release-note:none
+
+```

google-beta/config.go

@@ -70,11 +70,12 @@ type Config struct {
 	// It controls the interval at which we poll for successful operations
 	PollInterval time.Duration
 
-	client              *http.Client
-	wrappedPubsubClient *http.Client
-	context             context.Context
-	terraformVersion    string
-	userAgent           string
+	client                *http.Client
+	wrappedBigQueryClient *http.Client
+	wrappedPubsubClient   *http.Client
+	context               context.Context
+	terraformVersion      string
+	userAgent             string
 
 	tokenSource oauth2.TokenSource
 
@@ -524,6 +525,7 @@ func (c *Config) LoadAndValidate(ctx context.Context) error {
 	bigQueryClientBasePath := c.BigQueryBasePath
 	log.Printf("[INFO] Instantiating Google Cloud BigQuery client for path %s", bigQueryClientBasePath)
 	wrappedBigQueryClient := ClientWithAdditionalRetries(client, retryTransport, iamMemberMissing)
+	c.wrappedBigQueryClient = wrappedBigQueryClient
 	c.clientBigQuery, err = bigquery.NewService(ctx, option.WithHTTPClient(wrappedBigQueryClient))
 	if err != nil {
 		return err

google-beta/iam.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"log"
 	"reflect"
+	"sort"
 	"strings"
 	"time"
 
@@ -265,7 +266,17 @@ func createIamBindingsMap(bindings []*cloudresourcemanager.Binding) map[iamBindi
 // Return list of Bindings for a map of role to member sets
 func listFromIamBindingMap(bm map[iamBindingKey]map[string]struct{}) []*cloudresourcemanager.Binding {
 	rb := make([]*cloudresourcemanager.Binding, 0, len(bm))
-	for key, members := range bm {
+	var keys []iamBindingKey
+	for k := range bm {
+		keys = append(keys, k)
+	}
+	sort.Slice(keys, func(i, j int) bool {
+		keyI := keys[i]
+		keyJ := keys[j]
+		return fmt.Sprintf("%s%s", keyI.Role, keyI.Condition.String()) < fmt.Sprintf("%s%s", keyJ.Role, keyJ.Condition.String())
+	})
+	for _, key := range keys {
+		members := bm[key]
 		if len(members) == 0 {
 			continue
 		}

google-beta/metadata.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"sort"
 
 	computeBeta "google.golang.org/api/compute/v0.beta"
 	"google.golang.org/api/compute/v1"
@@ -144,10 +145,15 @@ func resourceInstanceMetadata(d TerraformResourceData) (*computeBeta.Metadata, e
 	}
 	if len(mdMap) > 0 {
 		m.Items = make([]*computeBeta.MetadataItems, 0, len(mdMap))
-		for key, val := range mdMap {
-			v := val.(string)
+		var keys []string
+		for k := range mdMap {
+			keys = append(keys, k)
+		}
+		sort.Strings(keys)
+		for _, k := range keys {
+			v := mdMap[k].(string)
 			m.Items = append(m.Items, &computeBeta.MetadataItems{
-				Key:   key,
+				Key:   k,
 				Value: &v,
 			})
 		}

google-beta/provider.go

@@ -895,7 +895,7 @@ func ResourceMapWithErrors() (map[string]*schema.Resource, error) {
 			"google_dns_record_set":                        resourceDnsRecordSet(),
 			"google_endpoints_service":                     resourceEndpointsService(),
 			"google_folder":                                resourceGoogleFolder(),
-			"google_folder_iam_binding":                    ResourceIamBinding(IamFolderSchema, NewFolderIamUpdater, FolderIdParseFunc),
+			"google_folder_iam_binding":                    ResourceIamBindingWithBatching(IamFolderSchema, NewFolderIamUpdater, FolderIdParseFunc, IamBatchingEnabled),
 			"google_folder_iam_member":                     ResourceIamMember(IamFolderSchema, NewFolderIamUpdater, FolderIdParseFunc),
 			"google_folder_iam_policy":                     ResourceIamPolicy(IamFolderSchema, NewFolderIamUpdater, FolderIdParseFunc),
 			"google_folder_organization_policy":            resourceGoogleFolderOrganizationPolicy(),

google-beta/provider_test.go

@@ -2,6 +2,7 @@ package google
 
 import (
 	"bytes"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io/ioutil"
@@ -10,6 +11,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"strconv"
 	"strings"
@@ -143,21 +145,48 @@ func getCachedConfig(d *schema.ResourceData, configureFunc func(d *schema.Resour
 	// Defines how VCR will match requests to responses.
 	rec.SetMatcher(func(r *http.Request, i cassette.Request) bool {
 		// Default matcher compares method and URL only
-		defaultMatch := cassette.DefaultMatcher(r, i)
+		if !cassette.DefaultMatcher(r, i) {
+			return false
+		}
 		if r.Body == nil {
-			return defaultMatch
+			return true
 		}
+		contentType := r.Header.Get("Content-Type")
+		// If body contains media, don't try to compare
+		if strings.Contains(contentType, "multipart/related") {
+			return true
+		}
+
 		var b bytes.Buffer
 		if _, err := b.ReadFrom(r.Body); err != nil {
 			log.Printf("[DEBUG] Failed to read request body from cassette: %v", err)
 			return false
 		}
 		r.Body = ioutil.NopCloser(&b)
-		// body must match recorded body
-		return defaultMatch && b.String() == i.Body
+		reqBody := b.String()
+		// If body matches identically, we are done
+		if reqBody == i.Body {
+			return true
+		}
+
+		// JSON might be the same, but reordered. Try parsing json and comparing
+		if strings.Contains(contentType, "application/json") {
+			var reqJson, cassetteJson interface{}
+			if err := json.Unmarshal([]byte(reqBody), &reqJson); err != nil {
+				log.Printf("[DEBUG] Failed to unmarshall request json: %v", err)
+				return false
+			}
+			if err := json.Unmarshal([]byte(i.Body), &cassetteJson); err != nil {
+				log.Printf("[DEBUG] Failed to unmarshall cassette json: %v", err)
+				return false
+			}
+			return reflect.DeepEqual(reqJson, cassetteJson)
+		}
+		return false
 	})
 	config.client.Transport = rec
 	config.wrappedPubsubClient.Transport = rec
+	config.wrappedBigQueryClient.Transport = rec
 	configs[testName] = config
 	return config, err
 }

google-beta/resource_google_folder_iam_binding_test.go

@@ -26,7 +26,7 @@ func TestAccFolderIamBinding_basic(t *testing.T) {
 			{
 				Config: testAccFolderIamBasic(org, fname),
 				Check: resource.ComposeTestCheckFunc(
-					testAccFolderExistingPolicy(org, fname),
+					testAccFolderExistingPolicy(t, org, fname),
 				),
 			},
 			// Apply an IAM binding
@@ -57,7 +57,7 @@ func TestAccFolderIamBinding_multiple(t *testing.T) {
 			{
 				Config: testAccFolderIamBasic(org, fname),
 				Check: resource.ComposeTestCheckFunc(
-					testAccFolderExistingPolicy(org, fname),
+					testAccFolderExistingPolicy(t, org, fname),
 				),
 			},
 			// Apply an IAM binding
@@ -102,7 +102,7 @@ func TestAccFolderIamBinding_multipleAtOnce(t *testing.T) {
 			{
 				Config: testAccFolderIamBasic(org, fname),
 				Check: resource.ComposeTestCheckFunc(
-					testAccFolderExistingPolicy(org, fname),
+					testAccFolderExistingPolicy(t, org, fname),
 				),
 			},
 			// Apply an IAM binding
@@ -137,7 +137,7 @@ func TestAccFolderIamBinding_update(t *testing.T) {
 			{
 				Config: testAccFolderIamBasic(org, fname),
 				Check: resource.ComposeTestCheckFunc(
-					testAccFolderExistingPolicy(org, fname),
+					testAccFolderExistingPolicy(t, org, fname),
 				),
 			},
 			// Apply an IAM binding
@@ -188,7 +188,7 @@ func TestAccFolderIamBinding_remove(t *testing.T) {
 			{
 				Config: testAccFolderIamBasic(org, fname),
 				Check: resource.ComposeTestCheckFunc(
-					testAccFolderExistingPolicy(org, fname),
+					testAccFolderExistingPolicy(t, org, fname),
 				),
 			},
 			// Apply multiple IAM bindings
@@ -209,7 +209,7 @@ func TestAccFolderIamBinding_remove(t *testing.T) {
 			{
 				Config: testAccFolderIamBasic(org, fname),
 				Check: resource.ComposeTestCheckFunc(
-					testAccFolderExistingPolicy(org, fname),
+					testAccFolderExistingPolicy(t, org, fname),
 				),
 			},
 		},

google-beta/resource_google_folder_iam_member_test.go

@@ -22,7 +22,7 @@ func TestAccFolderIamMember_basic(t *testing.T) {
 			{
 				Config: testAccFolderIamBasic(org, fname),
 				Check: resource.ComposeTestCheckFunc(
-					testAccFolderExistingPolicy(org, fname),
+					testAccFolderExistingPolicy(t, org, fname),
 				),
 			},
 			// Apply an IAM binding
@@ -53,7 +53,7 @@ func TestAccFolderIamMember_multiple(t *testing.T) {
 			{
 				Config: testAccFolderIamBasic(org, fname),
 				Check: resource.ComposeTestCheckFunc(
-					testAccFolderExistingPolicy(org, fname),
+					testAccFolderExistingPolicy(t, org, fname),
 				),
 			},
 			// Apply an IAM binding
@@ -94,7 +94,7 @@ func TestAccFolderIamMember_remove(t *testing.T) {
 			{
 				Config: testAccFolderIamBasic(org, fname),
 				Check: resource.ComposeTestCheckFunc(
-					testAccFolderExistingPolicy(org, fname),
+					testAccFolderExistingPolicy(t, org, fname),
 				),
 			},
 			// Apply multiple IAM bindings
@@ -111,7 +111,7 @@ func TestAccFolderIamMember_remove(t *testing.T) {
 			{
 				Config: testAccFolderIamBasic(org, fname),
 				Check: resource.ComposeTestCheckFunc(
-					testAccFolderExistingPolicy(org, fname),
+					testAccFolderExistingPolicy(t, org, fname),
 				),
 			},
 		},

google-beta/resource_google_folder_iam_policy_test.go

@@ -86,9 +86,9 @@ func testAccCheckGoogleFolderIamPolicyDestroyProducer(t *testing.T) func(s *terr
 }
 
 // Confirm that a folder has an IAM policy with at least 1 binding
-func testAccFolderExistingPolicy(org, fname string) resource.TestCheckFunc {
+func testAccFolderExistingPolicy(t *testing.T, org, fname string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
-		c := testAccProvider.Meta().(*Config)
+		c := googleProviderConfig(t)
 		var err error
 		originalPolicy, err = getFolderIamPolicyByParentAndDisplayName("organizations/"+org, fname, c)
 		if err != nil {

google-beta/resource_google_project_iam_audit_config_test.go

@@ -32,7 +32,7 @@ func TestAccProjectIamAuditConfig_basic(t *testing.T) {
 			{
 				Config: testAccProject_create(pid, pname, org),
 				Check: resource.ComposeTestCheckFunc(
-					testAccProjectExistingPolicy(pid),
+					testAccProjectExistingPolicy(t, pid),
 				),
 			},
 			// Apply an IAM audit config
@@ -61,7 +61,7 @@ func TestAccProjectIamAuditConfig_multiple(t *testing.T) {
 			{
 				Config: testAccProject_create(pid, pname, org),
 				Check: resource.ComposeTestCheckFunc(
-					testAccProjectExistingPolicy(pid),
+					testAccProjectExistingPolicy(t, pid),
 				),
 			},
 			// Apply an IAM audit config
@@ -95,7 +95,7 @@ func TestAccProjectIamAuditConfig_multipleAtOnce(t *testing.T) {
 			{
 				Config: testAccProject_create(pid, pname, org),
 				Check: resource.ComposeTestCheckFunc(
-					testAccProjectExistingPolicy(pid),
+					testAccProjectExistingPolicy(t, pid),
 				),
 			},
 			// Apply an IAM audit config
@@ -124,7 +124,7 @@ func TestAccProjectIamAuditConfig_update(t *testing.T) {
 			{
 				Config: testAccProject_create(pid, pname, org),
 				Check: resource.ComposeTestCheckFunc(
-					testAccProjectExistingPolicy(pid),
+					testAccProjectExistingPolicy(t, pid),
 				),
 			},
 			// Apply an IAM audit config
@@ -165,7 +165,7 @@ func TestAccProjectIamAuditConfig_remove(t *testing.T) {
 			{
 				Config: testAccProject_create(pid, pname, org),
 				Check: resource.ComposeTestCheckFunc(
-					testAccProjectExistingPolicy(pid),
+					testAccProjectExistingPolicy(t, pid),
 				),
 			},
 			// Apply multiple IAM audit configs
@@ -179,7 +179,7 @@ func TestAccProjectIamAuditConfig_remove(t *testing.T) {
 			{
 				Config: testAccProject_create(pid, pname, org),
 				Check: resource.ComposeTestCheckFunc(
-					testAccProjectExistingPolicy(pid),
+					testAccProjectExistingPolicy(t, pid),
 				),
 			},
 		},
@@ -204,7 +204,7 @@ func TestAccProjectIamAuditConfig_addFirstExemptMember(t *testing.T) {
 			{
 				Config: testAccProject_create(pid, pname, org),
 				Check: resource.ComposeTestCheckFunc(
-					testAccProjectExistingPolicy(pid),
+					testAccProjectExistingPolicy(t, pid),
 				),
 			},
 			// Apply IAM audit config with no members
@@ -240,7 +240,7 @@ func TestAccProjectIamAuditConfig_removeLastExemptMember(t *testing.T) {
 			{
 				Config: testAccProject_create(pid, pname, org),
 				Check: resource.ComposeTestCheckFunc(
-					testAccProjectExistingPolicy(pid),
+					testAccProjectExistingPolicy(t, pid),
 				),
 			},
 			// Apply IAM audit config with member
@@ -276,7 +276,7 @@ func TestAccProjectIamAuditConfig_updateNoExemptMembers(t *testing.T) {
 			{
 				Config: testAccProject_create(pid, pname, org),
 				Check: resource.ComposeTestCheckFunc(
-					testAccProjectExistingPolicy(pid),
+					testAccProjectExistingPolicy(t, pid),
 				),
 			},
 			// Apply IAM audit config with DATA_READ