diff --git a/pkg/internal/provider/sdkv2enhancements/resource_data.go b/pkg/internal/provider/sdkv2enhancements/resource_data.go new file mode 100644 index 0000000000..c2f450c908 --- /dev/null +++ b/pkg/internal/provider/sdkv2enhancements/resource_data.go @@ -0,0 +1,36 @@ +package sdkv2enhancements + +import ( + "reflect" + "unsafe" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" +) + +// CreateResourceDataFromResourceDiff allows to create schema.ResourceData out of schema.ResourceDiff. +// Unfortunately, schema.resourceDiffer is an unexported interface, and functions like schema.SchemaDiffSuppressFunc do not use the interface but a concrete implementation. +// One use case in which we needed to have schema.ResourceData from schema.ResourceDiff was to run schema.SchemaDiffSuppressFunc from inside schema.CustomizeDiffFunc. +// This implementation uses: +// - schema.InternalMap that exposes hidden schema.schemaMap (a wrapper over map[string]*schema.Schema) +// - (m schemaMap) Data method allowing to create schema.ResourceData from terraform.InstanceState and terraform.InstanceDiff +// - terraform.InstanceState and terraform.InstanceDiff are unexported in schema.ResourceDiff, so we get them using reflection +func CreateResourceDataFromResourceDiff(resourceSchema schema.InternalMap, diff *schema.ResourceDiff) (*schema.ResourceData, bool) { + unexportedState := reflect.ValueOf(diff).Elem().FieldByName("state") + stateFromResourceDiff := reflect.NewAt(unexportedState.Type(), unsafe.Pointer(unexportedState.UnsafeAddr())).Elem().Interface() + unexportedDiff := reflect.ValueOf(diff).Elem().FieldByName("diff") + diffFroResourceDif := reflect.NewAt(unexportedDiff.Type(), unsafe.Pointer(unexportedDiff.UnsafeAddr())).Elem().Interface() + castState, ok := stateFromResourceDiff.(*terraform.InstanceState) + if !ok { + return nil, false + } + castDiff, ok := diffFroResourceDif.(*terraform.InstanceDiff) + if !ok { + return nil, false + } + resourceData, err := resourceSchema.Data(castState, castDiff) + if err != nil { + return nil, false + } + return resourceData, true +} diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 2f41737bdd..0fef63ce97 100644 --- a/pkg/resources/account.go +++ b/pkg/resources/account.go @@ -216,7 +216,7 @@ func Account() *schema.Resource { Delete: DeleteAccount, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(accountSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: accountSchema, diff --git a/pkg/resources/account_role.go b/pkg/resources/account_role.go index e7cc6f7fcf..28922572db 100644 --- a/pkg/resources/account_role.go +++ b/pkg/resources/account_role.go @@ -50,9 +50,8 @@ func AccountRole() *schema.Resource { Description: "The resource is used for role management, where roles can be assigned privileges and, in turn, granted to users and other roles. When granted to roles they can create hierarchies of privilege structures. For more details, refer to the [official documentation](https://docs.snowflake.com/en/user-guide/security-access-control-overview).", CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment"), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(accountRoleSchema, ShowOutputAttributeName, "comment", "name"), + ComputedIfAnyAttributeChanged(accountRoleSchema, FullyQualifiedNameAttributeName, "name"), ), Importer: &schema.ResourceImporter{ diff --git a/pkg/resources/api_authentication_integration_with_authorization_code_grant.go b/pkg/resources/api_authentication_integration_with_authorization_code_grant.go index a1fb3df121..c2086e4608 100644 --- a/pkg/resources/api_authentication_integration_with_authorization_code_grant.go +++ b/pkg/resources/api_authentication_integration_with_authorization_code_grant.go @@ -46,8 +46,8 @@ func ApiAuthenticationIntegrationWithAuthorizationCodeGrant() *schema.Resource { ForceNewIfChangeToEmptyString("oauth_token_endpoint"), ForceNewIfChangeToEmptyString("oauth_authorization_endpoint"), ForceNewIfChangeToEmptyString("oauth_client_auth_method"), - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity", + ComputedIfAnyAttributeChanged(apiAuthAuthorizationCodeGrantSchema, ShowOutputAttributeName, "enabled", "comment"), + ComputedIfAnyAttributeChanged(apiAuthAuthorizationCodeGrantSchema, DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity", "oauth_client_id", "oauth_client_auth_method", "oauth_authorization_endpoint", "oauth_token_endpoint", "oauth_allowed_scopes"), ), diff --git a/pkg/resources/api_authentication_integration_with_client_credentials.go b/pkg/resources/api_authentication_integration_with_client_credentials.go index 53231d8cf3..c98039dcfc 100644 --- a/pkg/resources/api_authentication_integration_with_client_credentials.go +++ b/pkg/resources/api_authentication_integration_with_client_credentials.go @@ -41,8 +41,8 @@ func ApiAuthenticationIntegrationWithClientCredentials() *schema.Resource { CustomizeDiff: customdiff.All( ForceNewIfChangeToEmptyString("oauth_token_endpoint"), ForceNewIfChangeToEmptyString("oauth_client_auth_method"), - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity", + ComputedIfAnyAttributeChanged(apiAuthClientCredentialsSchema, ShowOutputAttributeName, "enabled", "comment"), + ComputedIfAnyAttributeChanged(apiAuthClientCredentialsSchema, DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity", "oauth_client_id", "oauth_client_auth_method", "oauth_token_endpoint", "oauth_allowed_scopes"), ), Importer: &schema.ResourceImporter{ diff --git a/pkg/resources/api_authentication_integration_with_jwt_bearer.go b/pkg/resources/api_authentication_integration_with_jwt_bearer.go index 23c4a68ee5..b3a508cb46 100644 --- a/pkg/resources/api_authentication_integration_with_jwt_bearer.go +++ b/pkg/resources/api_authentication_integration_with_jwt_bearer.go @@ -45,8 +45,8 @@ func ApiAuthenticationIntegrationWithJwtBearer() *schema.Resource { ForceNewIfChangeToEmptyString("oauth_token_endpoint"), ForceNewIfChangeToEmptyString("oauth_authorization_endpoint"), ForceNewIfChangeToEmptyString("oauth_client_auth_method"), - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity", + ComputedIfAnyAttributeChanged(apiAuthJwtBearerSchema, ShowOutputAttributeName, "enabled", "comment"), + ComputedIfAnyAttributeChanged(apiAuthJwtBearerSchema, DescribeOutputAttributeName, "enabled", "comment", "oauth_access_token_validity", "oauth_refresh_token_validity", "oauth_client_id", "oauth_client_auth_method", "oauth_authorization_endpoint", "oauth_token_endpoint", "oauth_assertion_issuer"), ), diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index d495e7cd93..a10c78d4d6 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -2,12 +2,13 @@ package resources import ( "context" + "fmt" "log" "strconv" "strings" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" - + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider/sdkv2enhancements" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -83,33 +84,38 @@ func ForceNewIfChangeToEmptyString(key string) schema.CustomizeDiffFunc { }) } -func ComputedIfAnyAttributeChanged(key string, changedAttributeKeys ...string) schema.CustomizeDiffFunc { +// ComputedIfAnyAttributeChanged marks the given fields as computed if any of the listed fields changes. +// It takes field-level diffSuppress into consideration based on the schema passed. +// If the field is not found in the given schema, it continues without error. +func ComputedIfAnyAttributeChanged(resourceSchema map[string]*schema.Schema, key string, changedAttributeKeys ...string) schema.CustomizeDiffFunc { return customdiff.ComputedIf(key, func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool { var result bool - for _, changedKey := range changedAttributeKeys { - if diff.HasChange(changedKey) { - old, new := diff.GetChange(changedKey) - log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: changed key: %s old: %s new: %s\n", changedKey, old, new) - } - result = result || diff.HasChange(changedKey) - } - return result - }) -} - -// TODO(SNOW-1629468): Adjust the function to make it more flexible -func ComputedIfAnyAttributeChangedWithSuppressDiff(key string, suppressDiffFunc schema.SchemaDiffSuppressFunc, changedAttributeKeys ...string) schema.CustomizeDiffFunc { - return customdiff.ComputedIf(key, func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool { for _, changedKey := range changedAttributeKeys { if diff.HasChange(changedKey) { oldValue, newValue := diff.GetChange(changedKey) - if !suppressDiffFunc(key, oldValue.(string), newValue.(string), nil) { - log.Printf("[DEBUG] ComputedIfAnyAttributeChangedWithSuppressDiff: changed key: %s", changedKey) - return true + log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: changed key: %s old: %s new: %s\n", changedKey, oldValue, newValue) + + if v, ok := resourceSchema[changedKey]; ok { + if diffSuppressFunc := v.DiffSuppressFunc; diffSuppressFunc != nil { + resourceData, resourceDataOk := sdkv2enhancements.CreateResourceDataFromResourceDiff(resourceSchema, diff) + if !resourceDataOk { + log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: did not create resource data correctly, skipping\n") + continue + } + if !diffSuppressFunc(key, fmt.Sprintf("%v", oldValue), fmt.Sprintf("%v", newValue), resourceData) { + log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: key %s was changed and the diff is not suppressed", changedKey) + result = true + } else { + log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: key %s was changed but the diff is suppresed", changedKey) + } + } else { + log.Printf("[DEBUG] ComputedIfAnyAttributeChanged: key %s was changed and it does not have a diff suppressor", changedKey) + result = true + } } } } - return false + return result }) } diff --git a/pkg/resources/custom_diffs_test.go b/pkg/resources/custom_diffs_test.go index 5bd28733aa..acecdfee11 100644 --- a/pkg/resources/custom_diffs_test.go +++ b/pkg/resources/custom_diffs_test.go @@ -345,10 +345,40 @@ func TestForceNewIfChangeToEmptySet(t *testing.T) { } } -func TestComputedIfAnyAttributeChangedWithSuppressDiff(t *testing.T) { - suppressFunc := schema.SchemaDiffSuppressFunc(func(k, oldValue, newValue string, d *schema.ResourceData) bool { +func Test_ComputedIfAnyAttributeChanged(t *testing.T) { + testSuppressFunc := schema.SchemaDiffSuppressFunc(func(k, oldValue, newValue string, d *schema.ResourceData) bool { return strings.Trim(oldValue, `"`) == strings.Trim(newValue, `"`) }) + testSchema := map[string]*schema.Schema{ + "value_with_diff_suppress": { + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: testSuppressFunc, + }, + "value_without_diff_suppress": { + Type: schema.TypeString, + Optional: true, + }, + "computed_value": { + Type: schema.TypeString, + Computed: true, + }, + } + testCustomDiff := resources.ComputedIfAnyAttributeChanged( + testSchema, + "computed_value", + "value_with_diff_suppress", + "value_without_diff_suppress", + ) + testProvider := &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "test": { + Schema: testSchema, + CustomizeDiff: testCustomDiff, + }, + }, + } + tests := []struct { name string stateValue map[string]string @@ -356,98 +386,171 @@ func TestComputedIfAnyAttributeChangedWithSuppressDiff(t *testing.T) { expectDiff bool }{ { - name: "no change", + name: "no change on both fields", stateValue: map[string]string{ - "value": "foo", - "computed_value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", }, expectDiff: false, }, { - name: "no change - quotes in config", + name: "change on field with diff suppression - suppressed (quotes in config added)", stateValue: map[string]string{ - "value": "foo", - "computed_value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "\"foo\"", + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "foo", }, expectDiff: false, }, { - name: "no change - quotes in state", + name: "change on field with diff suppression - suppressed (quotes in config removed)", stateValue: map[string]string{ - "value": "\"foo\"", - "computed_value": "foo", + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", }, expectDiff: false, }, { - name: "name change", + name: "change on field with diff suppression - not suppressed (value change)", stateValue: map[string]string{ - "value": "foo", - "computed_value": "foo", + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "bar", + "value_with_diff_suppress": "bar", + "value_without_diff_suppress": "foo", }, expectDiff: true, }, { - name: "name and quoting change", + name: "change on field with diff suppression - not suppressed (value and quotes changed)", stateValue: map[string]string{ - "value": "\"foo\"", - "computed_value": "foo", + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "foo", + "computed_value": "foo", }, rawConfigValue: map[string]any{ - "value": "bar", + "value_with_diff_suppress": "bar", + "value_without_diff_suppress": "foo", + }, + expectDiff: true, + }, + { + name: "change on field without diff suppression", + stateValue: map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + rawConfigValue: map[string]any{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "bar", + }, + expectDiff: true, + }, + { + name: "change on field without diff suppression, suppressed change on field with diff suppression", + stateValue: map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + rawConfigValue: map[string]any{ + "value_with_diff_suppress": "\"foo\"", + "value_without_diff_suppress": "bar", + }, + expectDiff: true, + }, + { + name: "change on field without diff suppression, not suppressed change on field with diff suppression", + stateValue: map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + rawConfigValue: map[string]any{ + "value_with_diff_suppress": "\"bar\"", + "value_without_diff_suppress": "bar", }, expectDiff: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - customDiff := resources.ComputedIfAnyAttributeChangedWithSuppressDiff( - "computed_value", - suppressFunc, - "value", - ) - provider := &schema.Provider{ - ResourcesMap: map[string]*schema.Resource{ - "test": { - Schema: map[string]*schema.Schema{ - "value": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: suppressFunc, - }, - "computed_value": { - Type: schema.TypeString, - Computed: true, - }, - }, - CustomizeDiff: customDiff, - }, - }, - } + tt := tt diff := calculateDiffFromAttributes( t, - provider, + testProvider, tt.stateValue, tt.rawConfigValue, ) if tt.expectDiff { require.NotNil(t, diff) + require.NotNil(t, diff.Attributes["computed_value"]) assert.True(t, diff.Attributes["computed_value"].NewComputed) } else { require.Nil(t, diff) } }) } + + t.Run("attributes not found in schema, both fields changed", func(t *testing.T) { + otherTestSchema := map[string]*schema.Schema{ + "value": { + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: testSuppressFunc, + }, + "computed_value": { + Type: schema.TypeString, + Computed: true, + }, + } + otherTestCustomDiff := resources.ComputedIfAnyAttributeChanged( + otherTestSchema, + "computed_value", + "value_with_diff_suppress", + "value_without_diff_suppress", + ) + otherTestProvider := &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "test": { + Schema: testSchema, + CustomizeDiff: otherTestCustomDiff, + }, + }, + } + + diff := calculateDiffFromAttributes( + t, + otherTestProvider, + map[string]string{ + "value_with_diff_suppress": "foo", + "value_without_diff_suppress": "foo", + "computed_value": "foo", + }, + map[string]any{ + "value_with_diff_suppress": "\"bar\"", + "value_without_diff_suppress": "bar", + }, + ) + + require.NotNil(t, diff) + assert.Nil(t, diff.Attributes["computed_value"]) + }) } diff --git a/pkg/resources/database.go b/pkg/resources/database.go index ec89d89db9..80923faaf2 100644 --- a/pkg/resources/database.go +++ b/pkg/resources/database.go @@ -101,7 +101,7 @@ func Database() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(databaseSchema, FullyQualifiedNameAttributeName, "name"), databaseParametersCustomDiff, ), diff --git a/pkg/resources/database_role.go b/pkg/resources/database_role.go index ee98900a7b..c070152ea9 100644 --- a/pkg/resources/database_role.go +++ b/pkg/resources/database_role.go @@ -60,8 +60,7 @@ func DatabaseRole() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment"), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(databaseRoleSchema, ShowOutputAttributeName, "comment", "name"), ), SchemaVersion: 1, diff --git a/pkg/resources/external_oauth_integration.go b/pkg/resources/external_oauth_integration.go index c8b7ffbead..b454e3fa67 100644 --- a/pkg/resources/external_oauth_integration.go +++ b/pkg/resources/external_oauth_integration.go @@ -167,8 +167,8 @@ func ExternalOauthIntegration() *schema.Resource { ForceNewIfChangeToEmptyString("external_oauth_scope_mapping_attribute"), ForceNewIfChangeToEmptySet("external_oauth_jws_keys_url"), ForceNewIfChangeToEmptySet("external_oauth_token_user_mapping_claim"), - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "external_oauth_type", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "external_oauth_issuer", "external_oauth_jws_keys_url", "external_oauth_any_role_mode", + ComputedIfAnyAttributeChanged(externalOauthIntegrationSchema, ShowOutputAttributeName, "enabled", "external_oauth_type", "comment"), + ComputedIfAnyAttributeChanged(externalOauthIntegrationSchema, DescribeOutputAttributeName, "enabled", "external_oauth_issuer", "external_oauth_jws_keys_url", "external_oauth_any_role_mode", "external_oauth_rsa_public_key", "external_oauth_rsa_public_key_2", "external_oauth_blocked_roles_list", "external_oauth_allowed_roles_list", "external_oauth_audience_list", "external_oauth_token_user_mapping_claim", "external_oauth_snowflake_user_mapping_attribute", "external_oauth_scope_delimiter", "comment"), diff --git a/pkg/resources/file_format.go b/pkg/resources/file_format.go index adc751fca6..35b2a08205 100644 --- a/pkg/resources/file_format.go +++ b/pkg/resources/file_format.go @@ -319,7 +319,7 @@ func FileFormat() *schema.Resource { Delete: DeleteFileFormat, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(fileFormatSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: fileFormatSchema, diff --git a/pkg/resources/function.go b/pkg/resources/function.go index e9f1b43c53..40709a0feb 100644 --- a/pkg/resources/function.go +++ b/pkg/resources/function.go @@ -173,7 +173,7 @@ func Function() *schema.Resource { CustomizeDiff: customdiff.All( // TODO(SNOW-1348103): add `arguments` to ComputedIfAnyAttributeChanged. This can't be done now because this function compares values without diff suppress. - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(functionSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: functionSchema, diff --git a/pkg/resources/masking_policy.go b/pkg/resources/masking_policy.go index b962f8cf81..f10a1321c9 100644 --- a/pkg/resources/masking_policy.go +++ b/pkg/resources/masking_policy.go @@ -122,7 +122,7 @@ func MaskingPolicy() *schema.Resource { Delete: DeleteMaskingPolicy, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(maskingPolicySchema, FullyQualifiedNameAttributeName, "name"), ), Schema: maskingPolicySchema, diff --git a/pkg/resources/materialized_view.go b/pkg/resources/materialized_view.go index e280150418..a280aef202 100644 --- a/pkg/resources/materialized_view.go +++ b/pkg/resources/materialized_view.go @@ -76,7 +76,7 @@ func MaterializedView() *schema.Resource { Delete: DeleteMaterializedView, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(materializedViewSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: materializedViewSchema, diff --git a/pkg/resources/network_policy.go b/pkg/resources/network_policy.go index d04b44bf86..8283f22b63 100644 --- a/pkg/resources/network_policy.go +++ b/pkg/resources/network_policy.go @@ -99,6 +99,7 @@ func NetworkPolicy() *schema.Resource { // for complex types like Sets, Lists, and Maps. When every element of the Set is suppressed in custom diff, // it returns true for d.HasChange anyway (it returns false for suppressed changes on primitive types like Number, Bool, String, etc.). ComputedIfAnyAttributeChanged( + networkPolicySchema, ShowOutputAttributeName, // "allowed_network_rule_list", // "blocked_network_rule_list", @@ -107,13 +108,14 @@ func NetworkPolicy() *schema.Resource { "comment", ), ComputedIfAnyAttributeChanged( + networkPolicySchema, DescribeOutputAttributeName, // "allowed_network_rule_list", // "blocked_network_rule_list", "allowed_ip_list", "blocked_ip_list", ), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(networkPolicySchema, FullyQualifiedNameAttributeName, "name"), ), Importer: &schema.ResourceImporter{ diff --git a/pkg/resources/oauth_integration_for_custom_clients.go b/pkg/resources/oauth_integration_for_custom_clients.go index 7d7828626b..4462651bf2 100644 --- a/pkg/resources/oauth_integration_for_custom_clients.go +++ b/pkg/resources/oauth_integration_for_custom_clients.go @@ -161,11 +161,13 @@ func OauthIntegrationForCustomClients() *schema.Resource { CustomizeDiff: customdiff.All( ComputedIfAnyAttributeChanged( + oauthIntegrationForCustomClientsSchema, ShowOutputAttributeName, "enabled", "comment", ), ComputedIfAnyAttributeChanged( + oauthIntegrationForCustomClientsSchema, DescribeOutputAttributeName, "oauth_client_type", "oauth_redirect_uri", diff --git a/pkg/resources/oauth_integration_for_partner_applications.go b/pkg/resources/oauth_integration_for_partner_applications.go index de031dab6c..d92d9d4dee 100644 --- a/pkg/resources/oauth_integration_for_partner_applications.go +++ b/pkg/resources/oauth_integration_for_partner_applications.go @@ -121,11 +121,13 @@ func OauthIntegrationForPartnerApplications() *schema.Resource { CustomizeDiff: customdiff.All( ComputedIfAnyAttributeChanged( + oauthIntegrationForPartnerApplicationsSchema, ShowOutputAttributeName, "enabled", "comment", ), ComputedIfAnyAttributeChanged( + oauthIntegrationForPartnerApplicationsSchema, DescribeOutputAttributeName, "oauth_client", "enabled", diff --git a/pkg/resources/password_policy.go b/pkg/resources/password_policy.go index 91ebb5b03b..0aa5aaff98 100644 --- a/pkg/resources/password_policy.go +++ b/pkg/resources/password_policy.go @@ -145,7 +145,7 @@ func PasswordPolicy() *schema.Resource { Delete: DeletePasswordPolicy, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(passwordPolicySchema, FullyQualifiedNameAttributeName, "name"), ), Schema: passwordPolicySchema, diff --git a/pkg/resources/procedure.go b/pkg/resources/procedure.go index 295f3a1b67..f1337acd2b 100644 --- a/pkg/resources/procedure.go +++ b/pkg/resources/procedure.go @@ -189,7 +189,7 @@ func Procedure() *schema.Resource { // TODO(SNOW-1348106): add `arguments` to ComputedIfAnyAttributeChanged for FullyQualifiedNameAttributeName. // This can't be done now because this function compares values without diff suppress. CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(procedureSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: procedureSchema, diff --git a/pkg/resources/saml2_integration.go b/pkg/resources/saml2_integration.go index 3939d2249a..6712fd23fe 100644 --- a/pkg/resources/saml2_integration.go +++ b/pkg/resources/saml2_integration.go @@ -172,8 +172,8 @@ func SAML2Integration() *schema.Resource { ForceNewIfChangeToEmptyString("saml2_snowflake_issuer_url"), ForceNewIfChangeToEmptyString("saml2_snowflake_acs_url"), ForceNewIfChangeToEmptyString("saml2_sp_initiated_login_page_label"), - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "saml2_issuer", "saml2_sso_url", "saml2_provider", "saml2_x509_cert", + ComputedIfAnyAttributeChanged(saml2IntegrationSchema, ShowOutputAttributeName, "enabled", "comment"), + ComputedIfAnyAttributeChanged(saml2IntegrationSchema, DescribeOutputAttributeName, "saml2_issuer", "saml2_sso_url", "saml2_provider", "saml2_x509_cert", "saml2_sp_initiated_login_page_label", "saml2_enable_sp_initiated", "saml2_sign_request", "saml2_requtedted_nameid_format", "saml2_post_logout_redirect_url", "saml2_force_authn", "saml2_snowflake_issuer_url", "saml2_snowflake_acs_url", "allowed_user_domains", "allowed_email_patterns"), diff --git a/pkg/resources/saml2_integration_acceptance_test.go b/pkg/resources/saml2_integration_acceptance_test.go index d27061762e..352c58c884 100644 --- a/pkg/resources/saml2_integration_acceptance_test.go +++ b/pkg/resources/saml2_integration_acceptance_test.go @@ -416,7 +416,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { }, CheckDestroy: acc.CheckDestroy(t, resources.Saml2SecurityIntegration), Steps: []resource.TestStep{ - // set up with concrete type + // set up with concrete saml2_force_authn { ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ @@ -432,7 +432,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "true"), ), }, - // import when type in config + // import when saml2_force_authn in config { ResourceName: "snowflake_saml2_integration.test", ImportState: true, @@ -442,7 +442,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { importchecks.TestCheckResourceAttrInstanceState(resourcehelpers.EncodeResourceIdentifier(id), "describe_output.0.saml2_force_authn.0.value", "true"), ), }, - // change type in config + // change saml2_force_authn in config { ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ @@ -462,7 +462,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { { Config: saml2ConfigWithAuthn(id.Name(), issuer, string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom), validUrl, cert, true), }, - // remove non-default type from config + // remove non-default saml2_force_authn from config { Config: saml2Config(id.Name(), issuer, string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom), validUrl, cert), ConfigPlanChecks: resource.ConfigPlanChecks{ @@ -479,23 +479,26 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"), ), }, - // add config + // add saml2_force_authn to config (false - which is a default in Snowflake) - no changes expected { ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ planchecks.PrintPlanDetails("snowflake_saml2_integration.test", "saml2_force_authn", "describe_output"), - planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String(r.BooleanDefault), sdk.String(r.BooleanDefault)), - planchecks.ExpectComputed("snowflake_saml2_integration.test", "describe_output", true), + plancheck.ExpectEmptyPlan(), }, }, Config: saml2ConfigWithAuthn(id.Name(), issuer, string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom), validUrl, cert, false), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", "false"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", r.BooleanDefault), resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.#", "1"), resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"), ), }, - // remove type from config but update externally to default (still expecting non-empty plan because we do not know the default) + // change back to non-default + { + Config: saml2ConfigWithAuthn(id.Name(), issuer, string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom), validUrl, cert, true), + }, + // remove saml2_force_authn from config but update externally to default (still expecting non-empty plan because we do not know the default) { PreConfig: func() { acc.TestClient().SecurityIntegration.UpdateSaml2ForceAuthn(t, id, false) @@ -515,7 +518,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"), ), }, - // change the size externally + // change the saml2_force_authn externally { PreConfig: func() { // we change the type to the type different from default, expecting action @@ -536,7 +539,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) { resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"), ), }, - // import when no type in config + // import when no saml2_force_authn in config { ResourceName: "snowflake_saml2_integration.test", ImportState: true, @@ -1002,15 +1005,32 @@ func TestAcc_Saml2Integration_DefaultValues(t *testing.T) { resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"), ), }, + // set to "non-zero" values + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Saml2Integration/non_zero_values"), + ConfigVariables: configVariables, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "enabled", "true"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", "true"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", "http://example.com"), + + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "show_output.#", "1"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "show_output.0.enabled", "true"), + + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.#", "1"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_post_logout_redirect_url.0.value", "http://example.com"), + resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "true"), + ), + }, // add valid "zero" values again (to validate if set is run correctly) { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Saml2Integration/zero_values"), ConfigVariables: configVariables, ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ - planchecks.ExpectChange("snowflake_saml2_integration.test", "enabled", tfjson.ActionUpdate, sdk.String(r.BooleanDefault), sdk.String(r.BooleanDefault)), - planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String(r.BooleanDefault), sdk.String(r.BooleanDefault)), - planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", tfjson.ActionUpdate, sdk.String(""), sdk.String("")), + planchecks.ExpectChange("snowflake_saml2_integration.test", "enabled", tfjson.ActionUpdate, sdk.String(r.BooleanTrue), sdk.String(r.BooleanFalse)), + planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String(r.BooleanTrue), sdk.String(r.BooleanFalse)), + planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", tfjson.ActionUpdate, sdk.String("http://example.com"), sdk.String("")), planchecks.ExpectComputed("snowflake_saml2_integration.test", "show_output", true), planchecks.ExpectComputed("snowflake_saml2_integration.test", "describe_output", true), }, diff --git a/pkg/resources/schema.go b/pkg/resources/schema.go index 55f54d02cf..de3c4e2e48 100644 --- a/pkg/resources/schema.go +++ b/pkg/resources/schema.go @@ -96,11 +96,10 @@ func Schema() *schema.Resource { Description: "Resource used to manage schema objects. For more information, check [schema documentation](https://docs.snowflake.com/en/sql-reference/sql/create-schema).", CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment", "with_managed_access", "is_transient"), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChangedWithSuppressDiff(DescribeOutputAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChanged(ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllSchemaParameters), strings.ToLower)...), + ComputedIfAnyAttributeChanged(schemaSchema, ShowOutputAttributeName, "name", "comment", "with_managed_access", "is_transient"), + ComputedIfAnyAttributeChanged(schemaSchema, DescribeOutputAttributeName, "name"), + ComputedIfAnyAttributeChanged(schemaSchema, FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(schemaParametersSchema, ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllSchemaParameters), strings.ToLower)...), schemaParametersCustomDiff, ), diff --git a/pkg/resources/scim_integration.go b/pkg/resources/scim_integration.go index fab3d51ae0..8ca1aeedb2 100644 --- a/pkg/resources/scim_integration.go +++ b/pkg/resources/scim_integration.go @@ -109,8 +109,8 @@ func SCIMIntegration() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "enabled", "scim_client", "comment"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "enabled", "comment", "network_policy", "run_as_role", "sync_password"), + ComputedIfAnyAttributeChanged(scimIntegrationSchema, ShowOutputAttributeName, "enabled", "scim_client", "comment"), + ComputedIfAnyAttributeChanged(scimIntegrationSchema, DescribeOutputAttributeName, "enabled", "comment", "network_policy", "run_as_role", "sync_password"), ), SchemaVersion: 2, diff --git a/pkg/resources/secondary_database.go b/pkg/resources/secondary_database.go index 361db4ab48..155699d1c9 100644 --- a/pkg/resources/secondary_database.go +++ b/pkg/resources/secondary_database.go @@ -53,7 +53,7 @@ func SecondaryDatabase() *schema.Resource { CustomizeDiff: customdiff.All( databaseParametersCustomDiff, - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(secondaryDatabaseSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: helpers.MergeMaps(secondaryDatabaseSchema, databaseParametersSchema), Importer: &schema.ResourceImporter{ diff --git a/pkg/resources/shared_database.go b/pkg/resources/shared_database.go index b6da4eaf36..8339148656 100644 --- a/pkg/resources/shared_database.go +++ b/pkg/resources/shared_database.go @@ -53,7 +53,7 @@ func SharedDatabase() *schema.Resource { Description: "A shared database creates a database from a share provided by another Snowflake account. For more information about shares, see [Introduction to Secure Data Sharing](https://docs.snowflake.com/en/user-guide/data-sharing-intro).", CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(sharedDatabaseSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: helpers.MergeMaps(sharedDatabaseSchema, sharedDatabaseParametersSchema), diff --git a/pkg/resources/streamlit.go b/pkg/resources/streamlit.go index e386f10f6c..830e7c0924 100644 --- a/pkg/resources/streamlit.go +++ b/pkg/resources/streamlit.go @@ -118,10 +118,9 @@ func Streamlit() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "title", "comment", "query_warehouse"), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), - ComputedIfAnyAttributeChanged(DescribeOutputAttributeName, "title", "comment", "root_location", "main_file", "query_warehouse", "external_access_integrations"), + ComputedIfAnyAttributeChanged(streamlitSchema, ShowOutputAttributeName, "name", "title", "comment", "query_warehouse"), + ComputedIfAnyAttributeChanged(streamlitSchema, FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(streamlitSchema, DescribeOutputAttributeName, "title", "comment", "root_location", "main_file", "query_warehouse", "external_access_integrations"), ), SchemaVersion: 1, diff --git a/pkg/resources/table.go b/pkg/resources/table.go index ad10836b74..100d89cffa 100644 --- a/pkg/resources/table.go +++ b/pkg/resources/table.go @@ -212,7 +212,7 @@ func Table() *schema.Resource { Delete: DeleteTable, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(tableSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: tableSchema, diff --git a/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/test.tf b/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/test.tf new file mode 100644 index 0000000000..7cc95b2ebe --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/test.tf @@ -0,0 +1,11 @@ +resource "snowflake_saml2_integration" "test" { + name = var.name + saml2_issuer = var.saml2_issuer + saml2_sso_url = var.saml2_sso_url + saml2_provider = var.saml2_provider + saml2_x509_cert = var.saml2_x509_cert + + enabled = true + saml2_force_authn = true + saml2_post_logout_redirect_url = "http://example.com" +} diff --git a/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/variables.tf b/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/variables.tf new file mode 100644 index 0000000000..351581f56c --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Saml2Integration/non_zero_values/variables.tf @@ -0,0 +1,15 @@ +variable "name" { + type = string +} +variable "saml2_issuer" { + type = string +} +variable "saml2_provider" { + type = string +} +variable "saml2_sso_url" { + type = string +} +variable "saml2_x509_cert" { + type = string +} diff --git a/pkg/resources/user.go b/pkg/resources/user.go index 05108193e4..eebb3dbf5e 100644 --- a/pkg/resources/user.go +++ b/pkg/resources/user.go @@ -199,10 +199,11 @@ func User() *schema.Resource { }, CustomizeDiff: customdiff.All( - // TODO [SNOW-1629468 - next pr]: handle diff suppression correctly; add "default_role", "default_secondary_roles" - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "password", "login_name", "display_name", "first_name", "middle_name", "last_name", "email", "must_change_password", "disabled", "days_to_expiry", "mins_to_unlock", "default_warehouse", "default_namespace", "mins_to_bypass_mfa", "rsa_public_key", "rsa_public_key_2", "comment", "disable_mfa"), - ComputedIfAnyAttributeChanged(ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllUserParameters), strings.ToLower)...), - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + // TODO [SNOW-1629468 - next pr]: test "default_role", "default_secondary_roles" + // TODO [SNOW-TODO]: "default_secondary_roles" have to stay commented out because of how the SDKv2 handles diff suppressions and custom diffs for sets + ComputedIfAnyAttributeChanged(userSchema, ShowOutputAttributeName, "password", "login_name", "display_name", "first_name", "middle_name", "last_name", "email", "must_change_password", "disabled", "days_to_expiry", "mins_to_unlock", "default_warehouse", "default_namespace", "default_role", "mins_to_bypass_mfa", "rsa_public_key", "rsa_public_key_2", "comment", "disable_mfa"), + ComputedIfAnyAttributeChanged(userParametersSchema, ParametersAttributeName, collections.Map(sdk.AsStringList(sdk.AllUserParameters), strings.ToLower)...), + ComputedIfAnyAttributeChanged(userSchema, FullyQualifiedNameAttributeName, "name"), userParametersCustomDiff, // TODO [SNOW-1645348]: revisit with service user work func(_ context.Context, diff *schema.ResourceDiff, _ interface{}) error { diff --git a/pkg/resources/view.go b/pkg/resources/view.go index e9e929f2da..5090e2b9ee 100644 --- a/pkg/resources/view.go +++ b/pkg/resources/view.go @@ -280,8 +280,8 @@ func View() *schema.Resource { Description: "Resource used to manage view objects. For more information, check [view documentation](https://docs.snowflake.com/en/sql-reference/sql/create-view).", CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment", "change_tracking", "is_secure", "is_temporary", "is_recursive", "statement"), - ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(viewSchema, ShowOutputAttributeName, "comment", "change_tracking", "is_secure", "is_temporary", "is_recursive", "statement"), + ComputedIfAnyAttributeChanged(viewSchema, FullyQualifiedNameAttributeName, "name"), ), Schema: viewSchema, diff --git a/pkg/resources/warehouse.go b/pkg/resources/warehouse.go index 9623cd947b..6232b0bac1 100644 --- a/pkg/resources/warehouse.go +++ b/pkg/resources/warehouse.go @@ -204,10 +204,9 @@ func Warehouse() *schema.Resource { }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "warehouse_type", "warehouse_size", "max_cluster_count", "min_cluster_count", "scaling_policy", "auto_suspend", "auto_resume", "comment", "enable_query_acceleration", "query_acceleration_max_scale_factor"), - ComputedIfAnyAttributeChanged(ParametersAttributeName, strings.ToLower(string(sdk.ObjectParameterMaxConcurrencyLevel)), strings.ToLower(string(sdk.ObjectParameterStatementQueuedTimeoutInSeconds)), strings.ToLower(string(sdk.ObjectParameterStatementTimeoutInSeconds))), - ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name", "resource_monitor"), - ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"), + ComputedIfAnyAttributeChanged(warehouseSchema, ShowOutputAttributeName, "name", "warehouse_type", "warehouse_size", "max_cluster_count", "min_cluster_count", "scaling_policy", "auto_suspend", "auto_resume", "resource_monitor", "comment", "enable_query_acceleration", "query_acceleration_max_scale_factor"), + ComputedIfAnyAttributeChanged(warehouseSchema, ParametersAttributeName, strings.ToLower(string(sdk.ObjectParameterMaxConcurrencyLevel)), strings.ToLower(string(sdk.ObjectParameterStatementQueuedTimeoutInSeconds)), strings.ToLower(string(sdk.ObjectParameterStatementTimeoutInSeconds))), + ComputedIfAnyAttributeChanged(warehouseSchema, FullyQualifiedNameAttributeName, "name"), customdiff.ForceNewIfChange("warehouse_size", func(ctx context.Context, old, new, meta any) bool { return old.(string) != "" && new.(string) == "" diff --git a/pkg/resources/warehouse_acceptance_test.go b/pkg/resources/warehouse_acceptance_test.go index 3e96684a7b..2d49146eed 100644 --- a/pkg/resources/warehouse_acceptance_test.go +++ b/pkg/resources/warehouse_acceptance_test.go @@ -3,6 +3,7 @@ package resources_test import ( "fmt" "regexp" + "strconv" "strings" "testing" @@ -188,41 +189,15 @@ func TestAcc_Warehouse_BasicFlows(t *testing.T) { resource.TestCheckResourceAttr("snowflake_warehouse.w", "fully_qualified_name", warehouseId2.FullyQualifiedName()), ), }, - // Change config but use defaults for every attribute (but not the parameters) - expect no changes (because these are already SF values) except computed show_output (follow-up why suppress diff is not taken into account in has changes?) + // Change config but use defaults for every attribute (but not the parameters) - expect no changes (because these are already SF values) { ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ planchecks.PrintPlanDetails("snowflake_warehouse.w", "warehouse_type", "warehouse_size", "max_cluster_count", "min_cluster_count", "scaling_policy", "auto_suspend", "auto_resume", "enable_query_acceleration", "query_acceleration_max_scale_factor", "max_concurrency_level", "statement_queued_timeout_in_seconds", "statement_timeout_in_seconds", r.ShowOutputAttributeName), - plancheck.ExpectResourceAction("snowflake_warehouse.w", plancheck.ResourceActionUpdate), - planchecks.ExpectComputed("snowflake_warehouse.w", r.ShowOutputAttributeName, true), + plancheck.ExpectEmptyPlan(), }, }, Config: warehouseFullDefaultWithoutParametersConfig(name2, comment), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_warehouse.w", "warehouse_type", string(sdk.WarehouseTypeStandard)), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "warehouse_size", string(sdk.WarehouseSizeXSmall)), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "max_cluster_count", "1"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "min_cluster_count", "1"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "scaling_policy", string(sdk.ScalingPolicyStandard)), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "auto_suspend", "600"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "auto_resume", "true"), - resource.TestCheckNoResourceAttr("snowflake_warehouse.w", "initially_suspended"), - resource.TestCheckNoResourceAttr("snowflake_warehouse.w", "resource_monitor"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "comment", comment), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "enable_query_acceleration", "false"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "query_acceleration_max_scale_factor", "8"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "max_concurrency_level", "8"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "statement_queued_timeout_in_seconds", "0"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "statement_timeout_in_seconds", "172800"), - - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.#", "1"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.max_concurrency_level.0.value", "8"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.max_concurrency_level.0.level", ""), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.statement_queued_timeout_in_seconds.0.value", "0"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.statement_queued_timeout_in_seconds.0.level", ""), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.statement_timeout_in_seconds.0.value", "172800"), - resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.statement_timeout_in_seconds.0.level", ""), - ), }, // add parameters - update expected (different level even with same values) { @@ -316,6 +291,15 @@ func TestAcc_Warehouse_BasicFlows(t *testing.T) { resource.TestCheckResourceAttr("snowflake_warehouse.w", "parameters.0.statement_timeout_in_seconds.0.level", string(sdk.ParameterTypeWarehouse)), ), }, + // change resource monitor - wrap in quotes (no change expected) + { + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + Config: warehouseFullConfigNoDefaultsStringId(name2, newComment, strconv.Quote(resourceMonitorId.FullyQualifiedName())), + }, // CHANGE max_concurrency_level EXTERNALLY (proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2318) { PreConfig: func() { acc.TestClient().Warehouse.UpdateMaxConcurrencyLevel(t, warehouseId2, 10) }, @@ -2136,6 +2120,30 @@ resource "snowflake_warehouse" "w" { `, name, comment, id.Name()) } +func warehouseFullConfigNoDefaultsStringId(name string, comment string, id string) string { + return fmt.Sprintf(` +resource "snowflake_warehouse" "w" { + name = "%[1]s" + warehouse_type = "SNOWPARK-OPTIMIZED" + warehouse_size = "MEDIUM" + max_cluster_count = 4 + min_cluster_count = 2 + scaling_policy = "ECONOMY" + auto_suspend = 1200 + auto_resume = false + initially_suspended = false + resource_monitor = %[3]s + comment = "%[2]s" + enable_query_acceleration = true + query_acceleration_max_scale_factor = 4 + + max_concurrency_level = 4 + statement_queued_timeout_in_seconds = 5 + statement_timeout_in_seconds = 86400 +} +`, name, comment, id) +} + func warehouseWithSizeConfig(name string, size string) string { return fmt.Sprintf(` resource "snowflake_warehouse" "w" {