chore: Adds support for conditionally keepUnknown based on state value (node_count and auto_scaling fix) (#3127)

* refactor: Enhance CopyUnknowns functionality with call for keepUnknown

* refactor: Support calls in CopyUnknowns function

* refactor: Use conditional calls for node_count and auto_scaling attributes

* refactor: Introduce AutoScaling model and enhance TestCopyUnknowns

* chore: minor comment refactor

* refactor: Clarify comments and apply PR suggestions

* refactor: Add additional test steps for advanced_cluster to demonstrate No Plan Changes when removing a block & Unexpected Plan Changes when removing `advanced_configuration` block

* refactor: Comment out Mocked Acceptance Tests in workflow to demonstrate test failures

* fix: non-empty refresh plan for advanced_configuration

* cannot use a Default, that will trigger infinite loop plan change

* Revert "refactor: Comment out Mocked Acceptance Tests in workflow to demonstrate test failures"

This reverts commit 3ae511a.

* use type KeepUnknownFunc

* no need to use calls at root level

* revert change to change_stream_options_pre_and_post_images_expire_after_seconds

* Reapply "refactor: Comment out Mocked Acceptance Tests in workflow to demonstrate test failures"

This reverts commit f1faf85.

* Revert "fix: non-empty refresh plan for advanced_configuration"

This reverts commit a30c3ae.

* comment checks

* Revert "Reapply "refactor: Comment out Mocked Acceptance Tests in workflow to demonstrate test failures""

This reverts commit 2942443.

* comment test step

---------

Co-authored-by: Leo Antoli <[email protected]>

Author: EspenAlbert

internal/common/schemafunc/plan_modifier_state_for_unknown.go

@@ -35,38 +35,58 @@ func HasUnknowns(obj any) bool {
 	return false
 }
 
+type KeepUnknownFunc func(string, attr.Value) bool
+
 // CopyUnknowns use reflection to copy unknown fields from src to dest.
 // The implementation is similar to internal/common/conversion/model_generation.go#CopyModel
 // keepUnknown is a list of fields that should not be copied, should always use the TF config name (snake_case)
 // nestedStructMapping is a map of field names to their type: object, list. (`set` not implemented yet)
-func CopyUnknowns(ctx context.Context, src, dest any, keepUnknown []string) {
+func CopyUnknowns(ctx context.Context, src, dest any, keepUnknown []string, keepUnknownCall KeepUnknownFunc) {
 	validateKeepUnknown(keepUnknown)
+	slicesContains := func(name string, value attr.Value) bool {
+		return slices.Contains(keepUnknown, name)
+	}
+	copyUnknowns(ctx, src, dest, KeepUnknownFuncOr(slicesContains, keepUnknownCall))
+}
+
+func KeepUnknownFuncOr(calls ...KeepUnknownFunc) KeepUnknownFunc {
+	return func(name string, value attr.Value) bool {
+		for _, call := range calls {
+			if call != nil && call(name, value) {
+				return true
+			}
+		}
+		return false
+	}
+}
+
+func copyUnknowns(ctx context.Context, src, dest any, keepUnknownCall KeepUnknownFunc) {
 	valSrc, valDest := validateStructPointers(src, dest)
 	typeSrc := valSrc.Type()
 	typeDest := valDest.Type()
 	for i := range typeDest.NumField() {
 		fieldDest := typeDest.Field(i)
 		name, tfName := fieldNameTFName(&fieldDest)
-		if slices.Contains(keepUnknown, tfName) {
+		srcValue := valSrc.FieldByName(name).Interface()
+		if keepUnknownCall(tfName, srcValue.(attr.Value)) {
 			continue
 		}
 		_, found := typeSrc.FieldByName(name)
 		if !found || !valDest.Field(i).CanSet() {
 			continue
 		}
-		nestedSrc := valSrc.FieldByName(name).Interface()
 		nestedDest := valDest.FieldByName(name).Interface()
-		objValueSrc, okSrc := nestedSrc.(types.Object)
+		objValueSrc, okSrc := srcValue.(types.Object)
 		objValueDest, okDest := nestedDest.(types.Object)
 		if okSrc && okDest {
-			objValueNew := copyUnknownsFromObject(ctx, objValueSrc, objValueDest, keepUnknown)
+			objValueNew := copyUnknownsFromObject(ctx, objValueSrc, objValueDest, keepUnknownCall)
 			valDest.Field(i).Set(reflect.ValueOf(objValueNew))
 			continue
 		}
-		listValueSrc, okSrc := nestedSrc.(types.List)
+		listValueSrc, okSrc := srcValue.(types.List)
 		listValueDest, okDest := nestedDest.(types.List)
 		if okSrc && okDest {
-			listValueNew := copyUnknownsFromList(ctx, listValueSrc, listValueDest, keepUnknown)
+			listValueNew := copyUnknownsFromList(ctx, listValueSrc, listValueDest, keepUnknownCall)
 			valDest.Field(i).Set(reflect.ValueOf(listValueNew))
 			continue
 		}
@@ -130,7 +150,7 @@ func validateKeepUnknown(keepUnknown []string) {
 	}
 }
 
-func copyUnknownsFromObject(ctx context.Context, src, dest types.Object, keepUnknown []string) types.Object {
+func copyUnknownsFromObject(ctx context.Context, src, dest types.Object, keepUnknownCall func(string, attr.Value) bool) types.Object {
 	// if something is null in the state and unknown in plan, we expect it to remain null
 	if src.IsNull() && dest.IsUnknown() {
 		return src
@@ -147,7 +167,8 @@ func copyUnknownsFromObject(ctx context.Context, src, dest types.Object, keepUnk
 		attributesDest = fillUnknowns(ctx, attributesSrc)
 	}
 	for name, attr := range attributesDest {
-		if slices.Contains(keepUnknown, name) {
+		replacement := attributesSrc[name]
+		if keepUnknownCall(name, replacement) {
 			attributesMerged[name] = attr
 			continue
 		}
@@ -157,9 +178,9 @@ func copyUnknownsFromObject(ctx context.Context, src, dest types.Object, keepUnk
 			tflog.Info(ctx, fmt.Sprintf("Copying unknown field: %s\n", name))
 			switch {
 			case isObject:
-				attr = copyUnknownsFromObject(ctx, attributesSrc[name].(types.Object), tfObjectDest, keepUnknown)
+				attr = copyUnknownsFromObject(ctx, attributesSrc[name].(types.Object), tfObjectDest, keepUnknownCall)
 			case isList:
-				attr = copyUnknownsFromList(ctx, attributesSrc[name].(types.List), tfListDest, keepUnknown)
+				attr = copyUnknownsFromList(ctx, attributesSrc[name].(types.List), tfListDest, keepUnknownCall)
 			default:
 				attr = attributesSrc[name]
 			}
@@ -168,11 +189,11 @@ func copyUnknownsFromObject(ctx context.Context, src, dest types.Object, keepUnk
 		}
 		if isList {
 			tfListSrc := attributesSrc[name].(types.List)
-			attr = copyUnknownsFromList(ctx, tfListSrc, tfListDest, keepUnknown)
+			attr = copyUnknownsFromList(ctx, tfListSrc, tfListDest, keepUnknownCall)
 		}
 		if isObject {
 			tfObjectSrc := attributesSrc[name].(types.Object)
-			newObject := copyUnknownsFromObject(ctx, tfObjectSrc, tfObjectDest, keepUnknown)
+			newObject := copyUnknownsFromObject(ctx, tfObjectSrc, tfObjectDest, keepUnknownCall)
 			attr = newObject
 		}
 		attributesMerged[name] = attr
@@ -193,7 +214,7 @@ func fillUnknowns(ctx context.Context, attributesSrc map[string]attr.Value) map[
 	return unknownAttributes
 }
 
-func copyUnknownsFromList(ctx context.Context, src, dest types.List, keepUnknown []string) types.List {
+func copyUnknownsFromList(ctx context.Context, src, dest types.List, keepUnknownCall func(string, attr.Value) bool) types.List {
 	srcElements := src.Elements()
 	destElements := dest.Elements()
 	count := len(srcElements)
@@ -204,7 +225,7 @@ func copyUnknownsFromList(ctx context.Context, src, dest types.List, keepUnknown
 	for i := range count {
 		srcObj := srcElements[i].(types.Object)
 		destObj := destElements[i].(types.Object)
-		newObj := copyUnknownsFromObject(ctx, srcObj, destObj, keepUnknown)
+		newObj := copyUnknownsFromObject(ctx, srcObj, destObj, keepUnknownCall)
 		merged[i] = newObj
 	}
 	return types.ListValueMust(dest.ElementType(ctx), merged)

internal/common/schemafunc/plan_modifier_state_for_unknown_test.go

@@ -2,6 +2,8 @@ package schemafunc_test
 
 import (
 	"context"
+	"fmt"
+	"slices"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-framework/attr"
@@ -85,6 +87,20 @@ func TestHasUnknown(t *testing.T) {
 	}
 }
 
+type TFAutoScalingModel struct {
+	ComputeMinInstanceSize types.String `tfsdk:"compute_min_instance_size"`
+	ComputeMaxInstanceSize types.String `tfsdk:"compute_max_instance_size"`
+	ComputeEnabled         types.Bool   `tfsdk:"compute_enabled"`
+	DiskGBEnabled          types.Bool   `tfsdk:"disk_gb_enabled"`
+}
+
+var AutoScalingObjType = types.ObjectType{AttrTypes: map[string]attr.Type{
+	"compute_enabled":           types.BoolType,
+	"disk_gb_enabled":           types.BoolType,
+	"compute_min_instance_size": types.StringType,
+	"compute_max_instance_size": types.StringType,
+}}
+
 type TFSpec struct {
 	InstanceSize types.String `tfsdk:"instance_size"`
 	NodeCount    types.Int64  `tfsdk:"node_count"`
@@ -96,12 +112,14 @@ var SpecObjType = types.ObjectType{AttrTypes: map[string]attr.Type{
 }}
 
 type TFRegionConfig struct {
+	AutoScaling  types.Object `tfsdk:"auto_scaling"`
 	ProviderName types.String `tfsdk:"provider_name"`
 	RegionName   types.String `tfsdk:"region_name"`
 	Spec         types.Object `tfsdk:"spec"`
 }
 
 var RegionConfigsObjType = types.ObjectType{AttrTypes: map[string]attr.Type{
+	"auto_scaling":  AutoScalingObjType,
 	"provider_name": types.StringType,
 	"region_name":   types.StringType,
 	"spec":          SpecObjType,
@@ -136,35 +154,85 @@ type TFSimpleModel struct {
 var (
 	ctx             = context.Background()
 	regionConfigSrc = TFRegionConfig{
+		AutoScaling:  autoScalingFalseAndNull,
 		ProviderName: types.StringValue("aws"),
 		RegionName:   types.StringValue("US_EAST_1"),
 		Spec:         asObjectValue(ctx, TFSpec{InstanceSize: types.StringValue("M10"), NodeCount: types.Int64Value(3)}, SpecObjType.AttrTypes),
 	}
+	regionConfigNodeCount0 = TFRegionConfig{
+		AutoScaling:  autoScalingFalseAndNull,
+		ProviderName: types.StringValue("aws"),
+		RegionName:   types.StringValue("US_EAST_1"),
+		Spec:         asObjectValue(ctx, TFSpec{InstanceSize: types.StringValue("M10"), NodeCount: types.Int64Value(0)}, SpecObjType.AttrTypes),
+	}
 	regionConfigDest = TFRegionConfig{
+		AutoScaling:  autoScalingFalseAndNull,
 		ProviderName: types.StringUnknown(),
 		RegionName:   types.StringValue("US_EAST_1"),
 		Spec:         types.ObjectUnknown(SpecObjType.AttrTypes),
 	}
 	regionConfigNodeCountUnknown = TFRegionConfig{
+		AutoScaling:  autoScalingFalseAndNull,
 		ProviderName: types.StringValue("aws"),
 		RegionName:   types.StringValue("US_EAST_1"),
 		Spec:         asObjectValue(ctx, TFSpec{InstanceSize: types.StringValue("M10"), NodeCount: types.Int64Unknown()}, SpecObjType.AttrTypes),
 	}
 	regionConfigSpecUnknown = TFRegionConfig{
+		AutoScaling:  autoScalingFalseAndNull,
 		ProviderName: types.StringValue("aws"),
 		RegionName:   types.StringValue("US_EAST_1"),
 		Spec:         types.ObjectUnknown(SpecObjType.AttrTypes),
 	}
-	advancedConfigTrue = asObjectValue(ctx, TFAdvancedConfig{JavascriptEnabled: types.BoolValue(true)}, AdvancedConfigObjType.AttrTypes)
+	advancedConfigTrue       = asObjectValue(ctx, TFAdvancedConfig{JavascriptEnabled: types.BoolValue(true)}, AdvancedConfigObjType.AttrTypes)
+	autoScalingFalseAndEmpty = asObjectValue(ctx, TFAutoScalingModel{
+		ComputeEnabled:         types.BoolValue(false),
+		DiskGBEnabled:          types.BoolValue(false),
+		ComputeMinInstanceSize: types.StringValue(""),
+		ComputeMaxInstanceSize: types.StringValue(""),
+	}, AutoScalingObjType.AttrTypes)
+
+	autoScalingFalseAndNull = asObjectValue(ctx, TFAutoScalingModel{
+		ComputeEnabled:         types.BoolValue(false),
+		DiskGBEnabled:          types.BoolValue(false),
+		ComputeMinInstanceSize: types.StringNull(),
+		ComputeMaxInstanceSize: types.StringNull(),
+	}, AutoScalingObjType.AttrTypes)
+	autoScalingTrueAndNonEmpty = asObjectValue(ctx, TFAutoScalingModel{
+		ComputeEnabled:         types.BoolValue(true),
+		DiskGBEnabled:          types.BoolValue(true),
+		ComputeMinInstanceSize: types.StringValue("M10"),
+		ComputeMaxInstanceSize: types.StringValue("M20"),
+	}, AutoScalingObjType.AttrTypes)
+	autoScalingUnknown      = types.ObjectUnknown(AutoScalingObjType.AttrTypes)
+	autoScalingLeafsUnknown = asObjectValue(ctx, TFAutoScalingModel{
+		ComputeEnabled:         types.BoolUnknown(),
+		DiskGBEnabled:          types.BoolUnknown(),
+		ComputeMinInstanceSize: types.StringUnknown(),
+		ComputeMaxInstanceSize: types.StringUnknown(),
+	}, AutoScalingObjType.AttrTypes)
+	keepProjectIDUnknown = func(name string, value attr.Value) bool {
+		return name == "project_id"
+	}
+	useStateOnlyWhenNodeCount0 = func(name string, value attr.Value) bool {
+		return name == "node_count" && !value.Equal(types.Int64Value(0))
+	}
+
+	autoScalingBoolsKeepUnknown = func(name string, value attr.Value) bool {
+		return slices.Contains([]string{"compute_enabled", "disk_gb_enabled"}, name) && value.Equal(types.BoolValue(true))
+	}
+	autoScalingStringsKeepUnknown = func(name string, value attr.Value) bool {
+		return slices.Contains([]string{"compute_min_instance_size", "compute_max_instance_size"}, name) && value.(types.String).ValueString() != ""
+	}
 )
 
 func TestCopyUnknowns(t *testing.T) {
 	tests := map[string]struct {
-		src          *TFSimpleModel
-		dest         *TFSimpleModel
-		expectedDest *TFSimpleModel
-		panicMessage string
-		keepUnknown  []string
+		src              *TFSimpleModel
+		dest             *TFSimpleModel
+		expectedDest     *TFSimpleModel
+		keepUnknownCalls schemafunc.KeepUnknownFunc
+		panicMessage     string
+		keepUnknown      []string
 	}{
 		"copy unknown basic fields": {
 			src: &TFSimpleModel{
@@ -225,6 +293,93 @@ func TestCopyUnknowns(t *testing.T) {
 			},
 			keepUnknown: []string{"spec"},
 		},
+		"respect keepUnknownCall root": {
+			src: &TFSimpleModel{
+				ProjectID: types.StringValue("src-project"),
+				Name:      types.StringValue("src-name"),
+			},
+			dest: &TFSimpleModel{
+				ProjectID: types.StringUnknown(),
+				Name:      types.StringUnknown(),
+			},
+			expectedDest: &TFSimpleModel{
+				ProjectID: types.StringUnknown(),
+				Name:      types.StringValue("src-name"),
+			},
+			keepUnknownCalls: keepProjectIDUnknown,
+		},
+		"respect keepUnknownCall nested": {
+			src: &TFSimpleModel{
+				ProjectID:        types.StringValue("src-project"),
+				Name:             types.StringValue("src-name"),
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringValue("Zone 1"), []TFRegionConfig{regionConfigSrc}),
+			},
+			dest: &TFSimpleModel{
+				ProjectID:        types.StringUnknown(),
+				Name:             types.StringUnknown(),
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringUnknown(), []TFRegionConfig{regionConfigNodeCountUnknown}),
+			},
+			expectedDest: &TFSimpleModel{
+				ProjectID:        types.StringValue("src-project"),
+				Name:             types.StringValue("src-name"),
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringValue("Zone 1"), []TFRegionConfig{regionConfigNodeCountUnknown}),
+			},
+			keepUnknownCalls: useStateOnlyWhenNodeCount0,
+		},
+		"respect multiple keepUnknownCall": {
+			src: &TFSimpleModel{
+				ProjectID:        types.StringValue("src-project"),
+				Name:             types.StringValue("src-name"),
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringValue("Zone 1"), []TFRegionConfig{regionConfigSrc}),
+			},
+			dest: &TFSimpleModel{
+				ProjectID:        types.StringUnknown(),
+				Name:             types.StringUnknown(),
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringUnknown(), []TFRegionConfig{regionConfigNodeCountUnknown}),
+			},
+			expectedDest: &TFSimpleModel{
+				ProjectID:        types.StringUnknown(),
+				Name:             types.StringValue("src-name"),
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringValue("Zone 1"), []TFRegionConfig{regionConfigNodeCountUnknown}),
+			},
+			keepUnknownCalls: schemafunc.KeepUnknownFuncOr(keepProjectIDUnknown, useStateOnlyWhenNodeCount0),
+		},
+		"copy node_count 0": {
+			src: &TFSimpleModel{
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringValue("Zone 1"), []TFRegionConfig{regionConfigNodeCount0}),
+			},
+			dest: &TFSimpleModel{
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringUnknown(), []TFRegionConfig{regionConfigNodeCountUnknown}),
+			},
+			expectedDest: &TFSimpleModel{
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringValue("Zone 1"), []TFRegionConfig{regionConfigNodeCount0}),
+			},
+			keepUnknownCalls: useStateOnlyWhenNodeCount0,
+		},
+		"keepUnknownCall on string": {
+			src: &TFSimpleModel{
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringValue("Zone 1"), []TFRegionConfig{
+					regionConfigWithAutoScaling(autoScalingFalseAndEmpty),
+					regionConfigWithAutoScaling(autoScalingFalseAndNull),
+					regionConfigWithAutoScaling(autoScalingTrueAndNonEmpty),
+				}),
+			},
+			dest: &TFSimpleModel{
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringUnknown(), []TFRegionConfig{
+					regionConfigWithAutoScaling(autoScalingUnknown),
+					regionConfigWithAutoScaling(autoScalingUnknown),
+					regionConfigWithAutoScaling(autoScalingUnknown),
+				}),
+			},
+			expectedDest: &TFSimpleModel{
+				ReplicationSpecs: newReplicationSpecs(ctx, types.StringValue("Zone 1"), []TFRegionConfig{
+					regionConfigWithAutoScaling(autoScalingFalseAndEmpty),
+					regionConfigWithAutoScaling(autoScalingFalseAndNull),
+					regionConfigWithAutoScaling(autoScalingLeafsUnknown),
+				}),
+			},
+			keepUnknownCalls: schemafunc.KeepUnknownFuncOr(autoScalingStringsKeepUnknown, autoScalingBoolsKeepUnknown),
+		},
 		"non-pointer input": {
 			src:          &TFSimpleModel{},
 			dest:         nil,
@@ -301,11 +456,11 @@ func TestCopyUnknowns(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			if tc.panicMessage != "" {
 				assert.PanicsWithValue(t, tc.panicMessage, func() {
-					schemafunc.CopyUnknowns(ctx, tc.src, tc.dest, tc.keepUnknown)
+					schemafunc.CopyUnknowns(ctx, tc.src, tc.dest, tc.keepUnknown, nil)
 				})
 				return
 			}
-			schemafunc.CopyUnknowns(ctx, tc.src, tc.dest, tc.keepUnknown)
+			schemafunc.CopyUnknowns(ctx, tc.src, tc.dest, tc.keepUnknown, tc.keepUnknownCalls)
 			assert.Equal(t, *tc.expectedDest, *tc.dest)
 		})
 	}
@@ -324,7 +479,7 @@ func newReplicationSpecs(ctx context.Context, zoneName types.String, regionConfi
 	for i, config := range regionConfigs {
 		configObject, diags := types.ObjectValueFrom(ctx, RegionConfigsObjType.AttrTypes, config)
 		if diags.HasError() {
-			panic("failed to create region config object")
+			panic(fmt.Sprintf("failed to create region config object %v", diags))
 		}
 		regionConfigsObjects[i] = configObject
 	}
@@ -346,3 +501,12 @@ func combineReplicationSpecs(specs ...types.List) types.List {
 	}
 	return types.ListValueMust(ReplicationSpecsObjType, combined)
 }
+
+func regionConfigWithAutoScaling(autoScaling types.Object) TFRegionConfig {
+	return TFRegionConfig{
+		AutoScaling:  autoScaling,
+		ProviderName: types.StringValue("aws"),
+		RegionName:   types.StringValue("US_EAST_1"),
+		Spec:         asObjectValue(ctx, TFSpec{InstanceSize: types.StringValue("M10"), NodeCount: types.Int64Value(3)}, SpecObjType.AttrTypes),
+	}
+}