chore: Handles replication specs updates as in SDKv2 and enables most of remaining SDKv2 Acc Tests (#2895)

* chore: Refactor fail_index_key_too_long to be compatible with old implementation and use zero values in advanced_configuration

* chore: fix formatting

* fix: nil pointer error

* chore: Refactor fail_index_key_too_long to be compatible with old implementation and use zero values in advanced_configuration

* chore: fix formatting

* fix: nil pointer error

* chore: Fixes converting to TPF by including all attributes in replication_specs

* fix: Prevent increasing num_shards from 1 when state doesn't use it

* refactor: Normalizing state can lead to replication specs updates when nodeCount=0 in an electable spec, therefore, not safe

* refactor: Normalizing state can lead to replication specs updates when nodeCount=0 in an electable spec, therefore, not safe

* fix: Update EbsVolumeType conversion to handle nil or empty string cases

* test: Support updating data files with updated diff requests

* chore: regenerate mock data

* refactor: Enable more tests in TPF

* refactor: Enhance patch operations to support suffix and prefix ignore/include options

* refactor: Add patch options to ignore and include specific state fields

* chore: Re-generate diff payloads

* refactor:use existing variable when no variable exist in config, probably there will be an import error if `{groupId`} is used as projectId

* refactor: Enable TestAccClusterAdvancedCluster_singleShardedMultiCloud test case

* test: Enable another test

* refactor: Add AllowOutOfOrder option to MockHTTPDataConfig and update MockRoundTripper behavior

* test: Enable more tests for TPF

* fix lint error

* chore: Update failIndexKeyTooLong retrieval to use getter method

* test: Skip tests for Advanced Cluster V2 schema due to unimplemented features

* refactor: Replace string checks with strconv.ParseBool for environment variable validation

* refactor: Simplify variable declarations in newAttrPatchOperations function

* Use empty string when legacyID is not set

* add workaround for TPF data source is not fully  implemented yet

* remove err check in ParseBool

* fix: num_shards updates detection

* fix: handle nil patch in tenant upgrade request

* feat: add forceUpdateAttr option to force update of replication specs when num_shards have changed

* revert earlier num_shards fix

* chore: remove debugging code [ci skip]

* fix: update patch options to use ForceUpdateAttr for replicationSpecs

* Revert "add workaround for TPF data source is not fully  implemented yet"

This reverts commit 6c2516c.

* remove: todo for `id`

* feat: Support isSchemaUpgrade to handle legacy to new schema

* test: Skip TestAccClusterAdvancedClusterConfig_asymmetricGeoShardedNewSchemaAddingRemovingShard until full replication_specs support is added

Author: EspenAlbert

internal/common/conversion/error_framework.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 
 	"github.com/hashicorp/terraform-plugin-framework/diag"
+
+	legacyDiag "github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 )
 
 type ErrBody interface {
@@ -32,3 +34,13 @@ func AddJSONBodyErrorToDiagnostics(msgPrefix string, err error, diags *diag.Diag
 	errorJSON := string(errorBytes)
 	diags.AddError(msgPrefix, errorJSON)
 }
+
+func AddLegacyDiags(diags *diag.Diagnostics, legacyDiags legacyDiag.Diagnostics) {
+	for _, diag := range legacyDiags {
+		if diag.Severity == legacyDiag.Error {
+			diags.AddError(diag.Summary, diag.Detail)
+		} else {
+			diags.AddWarning(diag.Summary, diag.Detail)
+		}
+	}
+}

internal/common/update/patch_payload.go

@@ -12,28 +12,55 @@ import (
 )
 
 type attrPatchOperations struct {
-	data          map[string][]jsondiff.Operation
-	ignoreInState []string
+	data                 map[string][]jsondiff.Operation
+	ignoreInStateSuffix  []string
+	ignoreInStatePrefix  []string
+	includeInStateSuffix []string
+	forceUpdateAttr      []string
 }
 
 func (m *attrPatchOperations) ignoreInStatePath(path string) bool {
-	for _, ignore := range m.ignoreInState {
+	for _, include := range m.includeInStateSuffix {
+		suffix := "/" + include
+		if strings.HasSuffix(path, suffix) {
+			return false
+		}
+	}
+	for _, ignore := range m.ignoreInStateSuffix {
 		suffix := "/" + ignore
 		if strings.HasSuffix(path, suffix) {
 			return true
 		}
 	}
+	for _, ignore := range m.ignoreInStatePrefix {
+		for _, part := range strings.Split(path, "/") {
+			if ignore == part {
+				return true
+			}
+		}
+	}
 	return false
 }
 
 func newAttrPatchOperations(patch jsondiff.Patch, options []PatchOptions) *attrPatchOperations {
-	ignoreInState := []string{}
+	var (
+		ignoreSuffixInState  []string
+		ignorePrefixInState  []string
+		includeSuffixInState []string
+		forceUpdateAttr      []string
+	)
 	for _, option := range options {
-		ignoreInState = append(ignoreInState, option.IgnoreInState...)
+		ignoreSuffixInState = append(ignoreSuffixInState, option.IgnoreInStateSuffix...)
+		ignorePrefixInState = append(ignorePrefixInState, option.IgnoreInStatePrefix...)
+		includeSuffixInState = append(includeSuffixInState, option.IncludeInStateSuffix...)
+		forceUpdateAttr = append(forceUpdateAttr, option.ForceUpdateAttr...)
 	}
 	self := &attrPatchOperations{
-		data:          map[string][]jsondiff.Operation{},
-		ignoreInState: ignoreInState,
+		data:                 map[string][]jsondiff.Operation{},
+		ignoreInStateSuffix:  ignoreSuffixInState,
+		ignoreInStatePrefix:  ignorePrefixInState,
+		includeInStateSuffix: includeSuffixInState,
+		forceUpdateAttr:      forceUpdateAttr,
 	}
 	for _, op := range patch {
 		if op.Path == "" {
@@ -81,7 +108,7 @@ func (m *attrPatchOperations) hasChanged(attr string) bool {
 func (m *attrPatchOperations) ChangedAttributes() []string {
 	attrs := []string{}
 	for attr := range m.data {
-		if m.hasChanged(attr) {
+		if m.hasChanged(attr) || slices.Contains(m.forceUpdateAttr, attr) {
 			attrs = append(attrs, attr)
 		}
 	}
@@ -136,7 +163,10 @@ func convertJSONDiffToJSONPatch(patch jsondiff.Patch) (jsonpatch.Patch, error) {
 
 // Current limitation if the field is set as part of a nested attribute in a map
 type PatchOptions struct {
-	IgnoreInState []string
+	IgnoreInStateSuffix  []string
+	IgnoreInStatePrefix  []string
+	IncludeInStateSuffix []string
+	ForceUpdateAttr      []string
 }
 
 // PatchPayload uses the state and plan to changes to find the patch request, including changes only when:

internal/common/update/patch_payload_test.go

@@ -22,17 +22,18 @@ func TestPatchReplicationSpecs(t *testing.T) {
 		replicationSpec2ZoneName    = "replicationSpec2_zoneName"
 		rootName                    = "my-cluster"
 		rootNameUpdated             = "my-cluster-updated"
-		state                       = admin.ClusterDescription20240805{
-			Id:   &idGlobal,
-			Name: &rootName,
-			ReplicationSpecs: &[]admin.ReplicationSpec20240805{
-				{
-					Id:       &idReplicationSpec1,
-					ZoneId:   &replicationSpec1ZoneID,
-					ZoneName: &replicationSpec1ZoneNameOld,
-				},
+		stateReplicationSpecs       = []admin.ReplicationSpec20240805{
+			{
+				Id:       &idReplicationSpec1,
+				ZoneId:   &replicationSpec1ZoneID,
+				ZoneName: &replicationSpec1ZoneNameOld,
 			},
 		}
+		state = admin.ClusterDescription20240805{
+			Id:               &idGlobal,
+			Name:             &rootName,
+			ReplicationSpecs: &stateReplicationSpecs,
+		}
 		planOptionalUpdated = admin.ClusterDescription20240805{
 			Name: &rootName,
 			ReplicationSpecs: &[]admin.ReplicationSpec20240805{
@@ -188,6 +189,16 @@ func TestPatchReplicationSpecs(t *testing.T) {
 				plan:          &planNoChanges,
 				patchExpected: nil,
 			},
+			"Forced changes when forceUpdateAttr set": {
+				state: &state,
+				plan:  &planNoChanges,
+				patchExpected: &admin.ClusterDescription20240805{
+					ReplicationSpecs: &stateReplicationSpecs,
+				},
+				options: []update.PatchOptions{
+					{ForceUpdateAttr: []string{"replicationSpecs"}},
+				},
+			},
 			"Empty array should return no changes": {
 				state: &admin.ClusterDescription20240805{
 					Labels: &[]admin.ComponentLabel{},
@@ -198,12 +209,23 @@ func TestPatchReplicationSpecs(t *testing.T) {
 				patchExpected: nil,
 			},
 			"diskSizeGb ignored in state": {
-				state:         clusterDescriptionDiskSizeNodeCount(50.0, 3, conversion.Pointer(50.0), 0),
-				plan:          clusterDescriptionDiskSizeNodeCount(55.0, 3, nil, 0),
-				patchExpected: clusterDescriptionDiskSizeNodeCount(55.0, 3, nil, 0),
+				state:         clusterDescriptionDiskSizeNodeCount(50.0, 3, conversion.Pointer(50.0), 0, conversion.Pointer(3500)),
+				plan:          clusterDescriptionDiskSizeNodeCount(55.0, 3, nil, 0, nil),
+				patchExpected: clusterDescriptionDiskSizeNodeCount(55.0, 3, nil, 0, conversion.Pointer(3500)),
+				options: []update.PatchOptions{
+					{
+						IgnoreInStateSuffix: []string{"diskSizeGB"},
+					},
+				},
+			},
+			"regionConfigs ignored in state but diskIOPS included": {
+				state:         clusterDescriptionDiskSizeNodeCount(50.0, 3, conversion.Pointer(50.0), 0, conversion.Pointer(3500)),
+				plan:          clusterDescriptionDiskSizeNodeCount(55.0, 3, nil, 0, nil),
+				patchExpected: clusterDescriptionDiskSizeNodeCount(55.0, 3, nil, 0, conversion.Pointer(3500)),
 				options: []update.PatchOptions{
 					{
-						IgnoreInState: []string{"diskSizeGB"},
+						IgnoreInStatePrefix:  []string{"regionConfigs"},
+						IncludeInStateSuffix: []string{"diskIOPS"},
 					},
 				},
 			},
@@ -280,7 +302,7 @@ func TestIsEmpty(t *testing.T) {
 	assert.False(t, update.IsZeroValues(&admin.ClusterDescription20240805{Name: conversion.Pointer("my-cluster")}))
 }
 
-func clusterDescriptionDiskSizeNodeCount(diskSizeGBElectable float64, nodeCountElectable int, diskSizeGBReadOnly *float64, nodeCountReadOnly int) *admin.ClusterDescription20240805 {
+func clusterDescriptionDiskSizeNodeCount(diskSizeGBElectable float64, nodeCountElectable int, diskSizeGBReadOnly *float64, nodeCountReadOnly int, diskIopsState *int) *admin.ClusterDescription20240805 {
 	return &admin.ClusterDescription20240805{
 		ReplicationSpecs: &[]admin.ReplicationSpec20240805{
 			{
@@ -289,10 +311,12 @@ func clusterDescriptionDiskSizeNodeCount(diskSizeGBElectable float64, nodeCountE
 						ElectableSpecs: &admin.HardwareSpec20240805{
 							NodeCount:  &nodeCountElectable,
 							DiskSizeGB: &diskSizeGBElectable,
+							DiskIOPS:   diskIopsState,
 						},
 						ReadOnlySpecs: &admin.DedicatedHardwareSpec20240805{
 							NodeCount:  &nodeCountReadOnly,
 							DiskSizeGB: diskSizeGBReadOnly,
+							DiskIOPS:   diskIopsState,
 						},
 					},
 				},

internal/service/advancedcluster/resource_advanced_cluster_migration_test.go

@@ -27,25 +27,18 @@ func TestMigAdvancedCluster_replicaSetMultiCloud(t *testing.T) {
 }
 
 func TestMigAdvancedCluster_singleShardedMultiCloud(t *testing.T) {
+	acc.SkipIfAdvancedClusterV2Schema(t) // AttributeName("advanced_configuration"): invalid JSON, expected "{", got "["
 	testCase := singleShardedMultiCloudTestCase(t, false)
 	mig.CreateAndRunTest(t, &testCase)
 }
 
 func TestMigAdvancedCluster_symmetricGeoShardedOldSchema(t *testing.T) {
+	acc.SkipIfAdvancedClusterV2Schema(t) // AttributeName("advanced_configuration"): invalid JSON, expected "{", got "["
 	testCase := symmetricGeoShardedOldSchemaTestCase(t, false)
 	mig.CreateAndRunTest(t, &testCase)
 }
 
 func TestMigAdvancedCluster_asymmetricShardedNewSchema(t *testing.T) {
-	// TODO: Already prepared for TPF but getting this error, note that TestAccClusterAdvancedClusterConfig_asymmetricShardedNewSchema is passing though:
-	// resource_advanced_cluster_migration_test.go:39: Step 1/2 error: Check failed: Check 2/15 error: mongodbatlas_advanced_cluster.test: Attribute 'replication_specs.0.region_configs.0.electable_specs.disk_iops' not found
-	// Check 5/15 error: mongodbatlas_advanced_cluster.test: Attribute 'replication_specs.0.region_configs.0.electable_specs.instance_size' not found
-	// Check 6/15 error: mongodbatlas_advanced_cluster.test: Attribute 'replication_specs.1.region_configs.0.electable_specs.instance_size' not found
-	// Check 7/15 error: mongodbatlas_advanced_cluster.test: Attribute 'replication_specs.1.region_configs.0.electable_specs.disk_size_gb' not found
-	// Check 8/15 error: mongodbatlas_advanced_cluster.test: Attribute 'replication_specs.0.region_configs.0.analytics_specs.disk_size_gb' not found
-	// Check 9/15 error: mongodbatlas_advanced_cluster.test: Attribute 'replication_specs.1.region_configs.0.analytics_specs.disk_size_gb' not found
-	// Check 10/15 error: mongodbatlas_advanced_cluster.test: Attribute 'replication_specs.0.region_configs.0.electable_specs.disk_size_gb' not found
-	// Check 11/15 error: mongodbatlas_advanced_cluster.test: Attribute 'replication_specs.1.region_configs.0.electable_specs.disk_iops' not found
 	acc.SkipIfAdvancedClusterV2Schema(t)
 	mig.SkipIfVersionBelow(t, "1.23.0") // version where sharded cluster tier auto-scaling was introduced
 	testCase := asymmetricShardedNewSchemaTestCase(t, false)