From e1fe39bbb6e22baa95a30eeeeb108c469629436b Mon Sep 17 00:00:00 2001 From: Dean Huynh Date: Fri, 3 Oct 2025 13:18:28 -0700 Subject: [PATCH 1/3] Handle settings drift for all resources --- go.mod | 3 + internal/provider/destination_resource.go | 12 +- .../destination_subscription_resource.go | 11 +- .../insert_function_instance_resource.go | 11 +- .../provider/profiles_warehouse_resource.go | 11 +- internal/provider/source_resource.go | 12 +- internal/provider/utils.go | 36 +++ internal/provider/utils_test.go | 250 ++++++++++++++++++ internal/provider/warehouse_resource.go | 12 +- 9 files changed, 349 insertions(+), 9 deletions(-) create mode 100644 internal/provider/utils_test.go diff --git a/go.mod b/go.mod index a17fe8c..e1ac8bc 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/hashicorp/terraform-plugin-go v0.26.0 github.com/hashicorp/terraform-plugin-testing v1.12.0 github.com/segmentio/public-api-sdk-go v0.0.0-20250113195817-34106b6e08dd + github.com/stretchr/testify v1.10.0 gotest.tools/gotestsum v1.13.0 ) @@ -19,8 +20,10 @@ require ( github.com/Kunde21/markdownfmt/v3 v3.1.0 // indirect github.com/bitfield/gotestdox v0.2.2 // indirect github.com/bmatcuk/doublestar/v4 v4.8.1 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/hashicorp/cli v1.1.7 // indirect github.com/mattn/go-runewidth v0.0.9 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/yuin/goldmark v1.7.7 // indirect github.com/yuin/goldmark-meta v1.1.0 // indirect go.abhg.dev/goldmark/frontmatter v0.2.0 // indirect diff --git a/internal/provider/destination_resource.go b/internal/provider/destination_resource.go index 4dd8210..84d13b1 100644 --- a/internal/provider/destination_resource.go +++ b/internal/provider/destination_resource.go @@ -615,9 +615,17 @@ func (r *destinationResource) Read(ctx context.Context, req resource.ReadRequest return } - // This is to satisfy terraform requirements that the returned fields must match the input ones because new settings can be generated in the response + // Merge settings: keep config-defined settings while ignoring backend-generated ones not in config if !previousState.Settings.IsNull() && !previousState.Settings.IsUnknown() { - state.Settings = previousState.Settings + mergedSettings, err := mergeSettings(previousState.Settings, state.Settings, false) + if err != nil { + resp.Diagnostics.AddError( + "Unable to merge Destination settings", + err.Error(), + ) + return + } + state.Settings = mergedSettings } diags = resp.State.Set(ctx, &state) diff --git a/internal/provider/destination_subscription_resource.go b/internal/provider/destination_subscription_resource.go index be2bf95..ad9cfb6 100644 --- a/internal/provider/destination_subscription_resource.go +++ b/internal/provider/destination_subscription_resource.go @@ -255,8 +255,17 @@ func (r *destinationSubscriptionResource) Read(ctx context.Context, req resource return } + // Merge settings: keep config-defined settings while ignoring backend-generated ones not in config if !previousState.Settings.IsNull() && !previousState.Settings.IsUnknown() { - state.Settings = previousState.Settings + mergedSettings, err := mergeSettings(previousState.Settings, state.Settings, false) + if err != nil { + resp.Diagnostics.AddError( + "Unable to merge Destination subscription settings", + err.Error(), + ) + return + } + state.Settings = mergedSettings } diags = resp.State.Set(ctx, &state) diff --git a/internal/provider/insert_function_instance_resource.go b/internal/provider/insert_function_instance_resource.go index 823da51..5261c5f 100644 --- a/internal/provider/insert_function_instance_resource.go +++ b/internal/provider/insert_function_instance_resource.go @@ -186,8 +186,17 @@ func (r *insertFunctionInstanceResource) Read(ctx context.Context, req resource. return } + // Merge settings: keep config-defined settings while ignoring backend-generated ones not in config if !previousState.Settings.IsNull() && !previousState.Settings.IsUnknown() { - state.Settings = previousState.Settings + mergedSettings, err := mergeSettings(previousState.Settings, state.Settings, false) + if err != nil { + resp.Diagnostics.AddError( + "Unable to merge Insert Function instance settings", + err.Error(), + ) + return + } + state.Settings = mergedSettings } // This is to satisfy terraform requirements that the input fields must match the returned ones. The input FunctionID can be prefixed with "ifnd_" and the returned one is not. diff --git a/internal/provider/profiles_warehouse_resource.go b/internal/provider/profiles_warehouse_resource.go index bf9d47a..2d1d47f 100644 --- a/internal/provider/profiles_warehouse_resource.go +++ b/internal/provider/profiles_warehouse_resource.go @@ -195,8 +195,17 @@ func (r *profilesWarehouseResource) Read(ctx context.Context, req resource.ReadR return } + // Merge settings: keep config-defined settings while ignoring backend-generated ones not in config if !previousState.Settings.IsNull() && !previousState.Settings.IsUnknown() { - state.Settings = previousState.Settings + mergedSettings, err := mergeSettings(previousState.Settings, state.Settings, true) + if err != nil { + resp.Diagnostics.AddError( + "Unable to merge Profiles Warehouse settings", + err.Error(), + ) + return + } + state.Settings = mergedSettings } diags = resp.State.Set(ctx, &state) diff --git a/internal/provider/source_resource.go b/internal/provider/source_resource.go index 7a47e08..ce2f144 100644 --- a/internal/provider/source_resource.go +++ b/internal/provider/source_resource.go @@ -415,9 +415,17 @@ func (r *sourceResource) Read(ctx context.Context, req resource.ReadRequest, res return } - // This is to satisfy terraform requirements that the returned fields must match the input ones because new settings can be generated in the response + // Merge settings: keep config-defined settings while ignoring backend-generated ones not in config if !previousState.Settings.IsNull() && !previousState.Settings.IsUnknown() { - state.Settings = previousState.Settings + mergedSettings, err := mergeSettings(previousState.Settings, state.Settings, false) + if err != nil { + resp.Diagnostics.AddError( + "Unable to merge Source settings", + err.Error(), + ) + return + } + state.Settings = mergedSettings } diags = resp.State.Set(ctx, &state) diff --git a/internal/provider/utils.go b/internal/provider/utils.go index 70dd36c..f320f5a 100644 --- a/internal/provider/utils.go +++ b/internal/provider/utils.go @@ -3,8 +3,13 @@ package provider import ( "bytes" "encoding/json" + "fmt" "io" "net/http" + "strings" + + "github.com/hashicorp/terraform-plugin-framework-jsontypes/jsontypes" + "github.com/segmentio/terraform-provider-segment/internal/provider/models" ) func getError(err error, body *http.Response) string { @@ -23,3 +28,34 @@ func getError(err error, body *http.Response) string { return err.Error() + "\n" + formattedBody.String() } + +// mergeSettings merges config settings with remote settings, preserving only the keys defined in config +func mergeSettings(configSettings, remoteSettings jsontypes.Normalized, isWarehouse bool) (jsontypes.Normalized, error) { + var configMap map[string]interface{} + if diags := configSettings.Unmarshal(&configMap); diags.HasError() { + return jsontypes.NewNormalizedNull(), fmt.Errorf("failed to unmarshal config settings: %s", diags.Errors()) + } + + var remoteMap map[string]interface{} + if diags := remoteSettings.Unmarshal(&remoteMap); diags.HasError() { + return jsontypes.NewNormalizedNull(), fmt.Errorf("failed to unmarshal remote settings: %s", diags.Errors()) + } + + // Create merged map with only config-defined keys that exist in remote (to detect drift) + // Keys in config but not in remote are excluded (they don't exist or aren't supported) + merged := make(map[string]interface{}) + for key := range configMap { + if isWarehouse && key == "password" { // Warehouses do not output password in the response + merged[key] = configMap[key] + } else if value, exists := remoteMap[key]; exists { + strValue, ok := value.(string) + if ok && strings.Contains(strValue, "•") { // If the secret is censored, do not update it + merged[key] = configMap[key] + } else { + merged[key] = value + } + } + } + + return models.GetSettings(merged) +} diff --git a/internal/provider/utils_test.go b/internal/provider/utils_test.go new file mode 100644 index 0000000..c536bfa --- /dev/null +++ b/internal/provider/utils_test.go @@ -0,0 +1,250 @@ +package provider + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-framework-jsontypes/jsontypes" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMergeSettings(t *testing.T) { + t.Run("merges config-defined settings with remote values", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"key1":"configValue1","key2":"configValue2"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"key1":"remoteValue1","key2":"remoteValue2","key3":"remoteValue3"}`) + + result, err := mergeSettings(configSettings, remoteSettings, false) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Should contain config-defined keys with remote values + assert.Equal(t, "remoteValue1", resultMap["key1"]) + assert.Equal(t, "remoteValue2", resultMap["key2"]) + + // Should NOT contain backend-generated keys + assert.NotContains(t, resultMap, "key3") + }) + + t.Run("excludes config keys missing in remote", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"key1":"configValue1","key2":"configValue2"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"key1":"remoteValue1"}`) + + result, err := mergeSettings(configSettings, remoteSettings, false) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Should use remote value when available + assert.Equal(t, "remoteValue1", resultMap["key1"]) + + // Should NOT include keys that are in config but not in remote + assert.NotContains(t, resultMap, "key2") + assert.Len(t, resultMap, 1) + }) + + t.Run("ignores backend-only settings", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"apiKey":"myKey"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"apiKey":"myKey","autoGeneratedField":"autoValue","internalFlag":true}`) + + result, err := mergeSettings(configSettings, remoteSettings, false) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Should only contain the config-defined key + assert.Len(t, resultMap, 1) + assert.Equal(t, "myKey", resultMap["apiKey"]) + assert.NotContains(t, resultMap, "autoGeneratedField") + assert.NotContains(t, resultMap, "internalFlag") + }) + + t.Run("detects drift in config-defined settings", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"key1":"originalValue","key2":"originalValue2"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"key1":"driftedValue","key2":"originalValue2"}`) + + result, err := mergeSettings(configSettings, remoteSettings, false) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Should detect drift by using remote value + assert.Equal(t, "driftedValue", resultMap["key1"]) + assert.Equal(t, "originalValue2", resultMap["key2"]) + }) + + t.Run("handles empty config settings", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"key1":"remoteValue1"}`) + + result, err := mergeSettings(configSettings, remoteSettings, false) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Should return empty when config has no keys + assert.Empty(t, resultMap) + }) + + t.Run("handles complex nested settings", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"nested":{"key1":"value1"},"simple":"value"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"nested":{"key1":"changedValue"},"simple":"value","extra":"ignored"}`) + + result, err := mergeSettings(configSettings, remoteSettings, false) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Should handle nested objects + assert.Contains(t, resultMap, "nested") + assert.Contains(t, resultMap, "simple") + assert.NotContains(t, resultMap, "extra") + }) + + t.Run("returns error for invalid config JSON", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`invalid json`) + remoteSettings := jsontypes.NewNormalizedValue(`{"key":"value"}`) + + _, err := mergeSettings(configSettings, remoteSettings, false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to unmarshal config settings") + }) + + t.Run("returns error for invalid remote JSON", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"key":"value"}`) + remoteSettings := jsontypes.NewNormalizedValue(`invalid json`) + + _, err := mergeSettings(configSettings, remoteSettings, false) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to unmarshal remote settings") + }) + + t.Run("preserves config password when isWarehouse is true", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"password":"secretPassword","username":"user123"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"username":"user123"}`) + + result, err := mergeSettings(configSettings, remoteSettings, true) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Password should be preserved from config even though it's not in remote + assert.Equal(t, "secretPassword", resultMap["password"]) + assert.Equal(t, "user123", resultMap["username"]) + }) + + t.Run("does not preserve password when isWarehouse is false", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"password":"secretPassword","username":"user123"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"username":"user123"}`) + + result, err := mergeSettings(configSettings, remoteSettings, false) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Password should NOT be included when not in remote and not a warehouse + assert.NotContains(t, resultMap, "password") + assert.Equal(t, "user123", resultMap["username"]) + }) + + t.Run("preserves config value when remote secret is censored", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"apiKey":"mySecretKey","publicKey":"publicValue"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"apiKey":"••••••••","publicKey":"publicValue"}`) + + result, err := mergeSettings(configSettings, remoteSettings, false) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Censored secret should use config value + assert.Equal(t, "mySecretKey", resultMap["apiKey"]) + // Non-censored value should use remote value + assert.Equal(t, "publicValue", resultMap["publicKey"]) + }) + + t.Run("handles multiple censored secrets", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"secret1":"value1","secret2":"value2","normal":"normalValue"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"secret1":"••••","secret2":"•••••••","normal":"normalValue"}`) + + result, err := mergeSettings(configSettings, remoteSettings, false) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Both censored secrets should use config values + assert.Equal(t, "value1", resultMap["secret1"]) + assert.Equal(t, "value2", resultMap["secret2"]) + // Normal value should use remote value + assert.Equal(t, "normalValue", resultMap["normal"]) + }) + + t.Run("handles censored secrets in warehouse mode", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"password":"dbPassword","apiKey":"myApiKey","username":"user"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"apiKey":"••••••••","username":"user"}`) + + result, err := mergeSettings(configSettings, remoteSettings, true) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Password should be preserved (warehouse mode) + assert.Equal(t, "dbPassword", resultMap["password"]) + // Censored API key should use config value + assert.Equal(t, "myApiKey", resultMap["apiKey"]) + // Username should use remote value + assert.Equal(t, "user", resultMap["username"]) + }) + + t.Run("only treats strings with bullet character as censored", func(t *testing.T) { + configSettings := jsontypes.NewNormalizedValue(`{"key1":"configValue","key2":"configValue2"}`) + remoteSettings := jsontypes.NewNormalizedValue(`{"key1":"dotdotdot...","key2":"••••"}`) + + result, err := mergeSettings(configSettings, remoteSettings, false) + + require.NoError(t, err) + + var resultMap map[string]interface{} + diags := result.Unmarshal(&resultMap) + require.False(t, diags.HasError()) + + // Regular dots should use remote value + assert.Equal(t, "dotdotdot...", resultMap["key1"]) + // Bullet character should use config value + assert.Equal(t, "configValue2", resultMap["key2"]) + }) +} diff --git a/internal/provider/warehouse_resource.go b/internal/provider/warehouse_resource.go index 77b9fae..bc92b44 100644 --- a/internal/provider/warehouse_resource.go +++ b/internal/provider/warehouse_resource.go @@ -316,9 +316,17 @@ func (r *warehouseResource) Read(ctx context.Context, req resource.ReadRequest, return } - // This is to satisfy terraform requirements that the returned fields must match the input ones because new settings can be generated in the response + // Merge settings: keep config-defined settings while ignoring backend-generated ones not in config if !previousState.Settings.IsNull() && !previousState.Settings.IsUnknown() { - state.Settings = previousState.Settings + mergedSettings, err := mergeSettings(previousState.Settings, state.Settings, true) + if err != nil { + resp.Diagnostics.AddError( + "Unable to merge Warehouse settings", + err.Error(), + ) + return + } + state.Settings = mergedSettings } diags = resp.State.Set(ctx, &state) From e867c6557173a8f13d0109955789377116996cd7 Mon Sep 17 00:00:00 2001 From: Dean Huynh Date: Fri, 3 Oct 2025 13:36:11 -0700 Subject: [PATCH 2/3] Fix lint errors --- internal/provider/destination_resource.go | 1 + .../destination_subscription_resource.go | 1 + .../insert_function_instance_resource.go | 1 + internal/provider/profiles_warehouse_resource.go | 1 + internal/provider/source_resource.go | 1 + internal/provider/utils.go | 9 +++++++-- internal/provider/utils_test.go | 16 ++++++++++++++++ internal/provider/warehouse_resource.go | 1 + 8 files changed, 29 insertions(+), 2 deletions(-) diff --git a/internal/provider/destination_resource.go b/internal/provider/destination_resource.go index 84d13b1..38ab042 100644 --- a/internal/provider/destination_resource.go +++ b/internal/provider/destination_resource.go @@ -623,6 +623,7 @@ func (r *destinationResource) Read(ctx context.Context, req resource.ReadRequest "Unable to merge Destination settings", err.Error(), ) + return } state.Settings = mergedSettings diff --git a/internal/provider/destination_subscription_resource.go b/internal/provider/destination_subscription_resource.go index ad9cfb6..ac385fc 100644 --- a/internal/provider/destination_subscription_resource.go +++ b/internal/provider/destination_subscription_resource.go @@ -263,6 +263,7 @@ func (r *destinationSubscriptionResource) Read(ctx context.Context, req resource "Unable to merge Destination subscription settings", err.Error(), ) + return } state.Settings = mergedSettings diff --git a/internal/provider/insert_function_instance_resource.go b/internal/provider/insert_function_instance_resource.go index 5261c5f..0cf365d 100644 --- a/internal/provider/insert_function_instance_resource.go +++ b/internal/provider/insert_function_instance_resource.go @@ -194,6 +194,7 @@ func (r *insertFunctionInstanceResource) Read(ctx context.Context, req resource. "Unable to merge Insert Function instance settings", err.Error(), ) + return } state.Settings = mergedSettings diff --git a/internal/provider/profiles_warehouse_resource.go b/internal/provider/profiles_warehouse_resource.go index 2d1d47f..ec998b7 100644 --- a/internal/provider/profiles_warehouse_resource.go +++ b/internal/provider/profiles_warehouse_resource.go @@ -203,6 +203,7 @@ func (r *profilesWarehouseResource) Read(ctx context.Context, req resource.ReadR "Unable to merge Profiles Warehouse settings", err.Error(), ) + return } state.Settings = mergedSettings diff --git a/internal/provider/source_resource.go b/internal/provider/source_resource.go index ce2f144..3a81273 100644 --- a/internal/provider/source_resource.go +++ b/internal/provider/source_resource.go @@ -423,6 +423,7 @@ func (r *sourceResource) Read(ctx context.Context, req resource.ReadRequest, res "Unable to merge Source settings", err.Error(), ) + return } state.Settings = mergedSettings diff --git a/internal/provider/utils.go b/internal/provider/utils.go index f320f5a..618cbc8 100644 --- a/internal/provider/utils.go +++ b/internal/provider/utils.go @@ -29,7 +29,7 @@ func getError(err error, body *http.Response) string { return err.Error() + "\n" + formattedBody.String() } -// mergeSettings merges config settings with remote settings, preserving only the keys defined in config +// mergeSettings merges config settings with remote settings, preserving only the keys defined in config. func mergeSettings(configSettings, remoteSettings jsontypes.Normalized, isWarehouse bool) (jsontypes.Normalized, error) { var configMap map[string]interface{} if diags := configSettings.Unmarshal(&configMap); diags.HasError() { @@ -57,5 +57,10 @@ func mergeSettings(configSettings, remoteSettings jsontypes.Normalized, isWareho } } - return models.GetSettings(merged) + result, err := models.GetSettings(merged) + if err != nil { + return jsontypes.Normalized{}, fmt.Errorf("failed to merge settings: %s", err.Error()) + } + + return result, nil } diff --git a/internal/provider/utils_test.go b/internal/provider/utils_test.go index c536bfa..2b101e0 100644 --- a/internal/provider/utils_test.go +++ b/internal/provider/utils_test.go @@ -9,7 +9,10 @@ import ( ) func TestMergeSettings(t *testing.T) { + t.Parallel() + t.Run("merges config-defined settings with remote values", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"key1":"configValue1","key2":"configValue2"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"key1":"remoteValue1","key2":"remoteValue2","key3":"remoteValue3"}`) @@ -30,6 +33,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("excludes config keys missing in remote", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"key1":"configValue1","key2":"configValue2"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"key1":"remoteValue1"}`) @@ -50,6 +54,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("ignores backend-only settings", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"apiKey":"myKey"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"apiKey":"myKey","autoGeneratedField":"autoValue","internalFlag":true}`) @@ -69,6 +74,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("detects drift in config-defined settings", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"key1":"originalValue","key2":"originalValue2"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"key1":"driftedValue","key2":"originalValue2"}`) @@ -86,6 +92,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("handles empty config settings", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{}`) remoteSettings := jsontypes.NewNormalizedValue(`{"key1":"remoteValue1"}`) @@ -102,6 +109,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("handles complex nested settings", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"nested":{"key1":"value1"},"simple":"value"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"nested":{"key1":"changedValue"},"simple":"value","extra":"ignored"}`) @@ -120,6 +128,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("returns error for invalid config JSON", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`invalid json`) remoteSettings := jsontypes.NewNormalizedValue(`{"key":"value"}`) @@ -130,6 +139,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("returns error for invalid remote JSON", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"key":"value"}`) remoteSettings := jsontypes.NewNormalizedValue(`invalid json`) @@ -140,6 +150,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("preserves config password when isWarehouse is true", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"password":"secretPassword","username":"user123"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"username":"user123"}`) @@ -157,6 +168,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("does not preserve password when isWarehouse is false", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"password":"secretPassword","username":"user123"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"username":"user123"}`) @@ -174,6 +186,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("preserves config value when remote secret is censored", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"apiKey":"mySecretKey","publicKey":"publicValue"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"apiKey":"••••••••","publicKey":"publicValue"}`) @@ -192,6 +205,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("handles multiple censored secrets", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"secret1":"value1","secret2":"value2","normal":"normalValue"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"secret1":"••••","secret2":"•••••••","normal":"normalValue"}`) @@ -211,6 +225,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("handles censored secrets in warehouse mode", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"password":"dbPassword","apiKey":"myApiKey","username":"user"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"apiKey":"••••••••","username":"user"}`) @@ -231,6 +246,7 @@ func TestMergeSettings(t *testing.T) { }) t.Run("only treats strings with bullet character as censored", func(t *testing.T) { + t.Parallel() configSettings := jsontypes.NewNormalizedValue(`{"key1":"configValue","key2":"configValue2"}`) remoteSettings := jsontypes.NewNormalizedValue(`{"key1":"dotdotdot...","key2":"••••"}`) diff --git a/internal/provider/warehouse_resource.go b/internal/provider/warehouse_resource.go index bc92b44..0f963d7 100644 --- a/internal/provider/warehouse_resource.go +++ b/internal/provider/warehouse_resource.go @@ -324,6 +324,7 @@ func (r *warehouseResource) Read(ctx context.Context, req resource.ReadRequest, "Unable to merge Warehouse settings", err.Error(), ) + return } state.Settings = mergedSettings From 686ba99abc6f7be4da9618305e0397953803dd04 Mon Sep 17 00:00:00 2001 From: Dean Huynh Date: Fri, 3 Oct 2025 13:42:11 -0700 Subject: [PATCH 3/3] Update internal/provider/utils.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/provider/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/provider/utils.go b/internal/provider/utils.go index 618cbc8..b0c3b33 100644 --- a/internal/provider/utils.go +++ b/internal/provider/utils.go @@ -59,7 +59,7 @@ func mergeSettings(configSettings, remoteSettings jsontypes.Normalized, isWareho result, err := models.GetSettings(merged) if err != nil { - return jsontypes.Normalized{}, fmt.Errorf("failed to merge settings: %s", err.Error()) + return jsontypes.Normalized{}, fmt.Errorf("failed to merge settings: %w", err) } return result, nil