Skip to content

Commit

Permalink
Add entries to migration guide, add sdk validations
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-jmichalak committed Jul 22, 2024
1 parent 32c55ad commit 1266952
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 29 deletions.
16 changes: 12 additions & 4 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ across different versions.

## v0.93.0 ➞ v0.94.0

### *(breaking change)* changes in snowflake_scim_integration

In order to fix issues in v0.93.0, when a resource has Azure scim client, `sync_password` field is now set to `default` value in the state. State will be migrated automatically.

### *(new feature)* new snowflake_account_role resource

Already existing `snowflake_role` was deprecated in favor of the new `snowflake_account_role`. The old resource got upgraded to
Expand Down Expand Up @@ -47,10 +51,10 @@ See reference [doc](https://docs.snowflake.com/en/sql-reference/sql/create-secur

### *(new feature)* snowflake_oauth_integration_for_custom_clients and snowflake_oauth_integration_for_partner_applications resources

To enhance clarity and functionality, the new resources `snowflake_oauth_integration_for_custom_clients` and `snowflake_oauth_integration_for_partner_applications` have been introduced
To enhance clarity and functionality, the new resources `snowflake_oauth_integration_for_custom_clients` and `snowflake_oauth_integration_for_partner_applications` have been introduced
to replace the previous `snowflake_oauth_integration`. Recognizing that the old resource carried multiple responsibilities within a single entity, we opted to divide it into two more specialized resources.
The newly introduced resources are aligned with the latest Snowflake documentation at the time of implementation, and adhere to our [new conventions](#general-changes).
This segregation was based on the `oauth_client` attribute, where `CUSTOM` corresponds to `snowflake_oauth_integration_for_custom_clients`,
The newly introduced resources are aligned with the latest Snowflake documentation at the time of implementation, and adhere to our [new conventions](#general-changes).
This segregation was based on the `oauth_client` attribute, where `CUSTOM` corresponds to `snowflake_oauth_integration_for_custom_clients`,
while other attributes align with `snowflake_oauth_integration_for_partner_applications`.

### *(new feature)* snowflake_security_integrations datasource
Expand Down Expand Up @@ -105,14 +109,18 @@ The fields listed below had diff suppress which removed '-' from strings. Now, t
### *(new feature)* snowflake_saml2_integration resource

The new `snowflake_saml2_integration` is introduced and deprecates `snowflake_saml_integration`. It contains new fields
and follows our new conventions making it more stable. The old SAML integration wasn't changed, so no migration needed,
and follows our new conventions making it more stable. The old SAML integration wasn't changed, so no migration needed,
but we recommend to eventually migrate to the newer counterpart.

### snowflake_scim_integration resource changes
#### *(behavior change)* Changed behavior of `sync_password`

Now, the `sync_password` field will set the state value to `default` whenever the value is not set in the config. This indicates that the value on the Snowflake side is set to the Snowflake default.

> [!WARNING]
> This change causes issues for Azure scim client (see [#2946](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2946)). The workaround is to remove the resource from the state with `terraform state rm`, add `sync_password = true` to the config, and import with `terraform import "snowflake_scim_integration.test" "aad_provisioning"`. After these steps, there should be no errors and no diff on this field. This behavior is fixed in v0.94 with state upgrader.

#### *(behavior change)* Renamed fields

Renamed field `provisioner_role` to `run_as_role` to align with Snowflake docs. Please rename this field in your configuration files. State will be migrated automatically.
Expand Down
27 changes: 9 additions & 18 deletions pkg/resources/scim_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func ImportScimIntegration(ctx context.Context, d *schema.ResourceData, meta any
return nil, err
}
}
if scimClient == string(sdk.ScimSecurityIntegrationScimClientAzure) {
if strings.EqualFold(strings.TrimSpace(scimClient), string(sdk.ScimSecurityIntegrationScimClientAzure)) {
if err = d.Set("sync_password", BooleanDefault); err != nil {
return nil, err
}
Expand Down Expand Up @@ -207,7 +207,7 @@ func CreateContextSCIMIntegration(ctx context.Context, d *schema.ResourceData, m
}

if v := d.Get("sync_password").(string); v != BooleanDefault {
if scimClient := d.Get("scim_client").(string); scimClient == string(sdk.ScimSecurityIntegrationScimClientAzure) {
if scimClient := d.Get("scim_client").(string); strings.EqualFold(strings.TrimSpace(scimClient), string(sdk.ScimSecurityIntegrationScimClientAzure)) {
return diag.Diagnostics{
{
Severity: diag.Error,
Expand Down Expand Up @@ -318,7 +318,7 @@ func ReadContextSCIMIntegration(withExternalChangesMarking bool) schema.ReadCont
return diag.FromErr(err)
}

if scimClient != string(sdk.ScimSecurityIntegrationScimClientAzure) {
if !strings.EqualFold(strings.TrimSpace(scimClient), string(sdk.ScimSecurityIntegrationScimClientAzure)) {
syncPasswordProperty, err := collections.FindOne(integrationProperties, func(property sdk.SecurityIntegrationProperty) bool { return property.Name == "SYNC_PASSWORD" })
if err != nil {
return diag.FromErr(err)
Expand All @@ -331,20 +331,11 @@ func ReadContextSCIMIntegration(withExternalChangesMarking bool) schema.ReadCont
}
}

// These are all identity sets, needed for the case where:
// - previous config was empty (therefore Snowflake defaults had been used)
// - new config have the same values that are already in SF
if !d.GetRawConfig().IsNull() {
if v := d.GetRawConfig().AsValueMap()["network_policy"]; !v.IsNull() {
if err = d.Set("network_policy", v.AsString()); err != nil {
return diag.FromErr(err)
}
}
if v := d.GetRawConfig().AsValueMap()["sync_password"]; !v.IsNull() {
if err = d.Set("sync_password", v.AsString()); err != nil {
return diag.FromErr(err)
}
}
if err = setStateToValuesFromConfig(d, saml2IntegrationSchema, []string{
"network_policy",
"sync_password",
}); err != nil {
return diag.FromErr(err)
}

if err = d.Set(ShowOutputAttributeName, []map[string]any{schemas.SecurityIntegrationToSchema(integration)}); err != nil {
Expand Down Expand Up @@ -377,7 +368,7 @@ func UpdateContextSCIMIntegration(ctx context.Context, d *schema.ResourceData, m
}

if d.HasChange("sync_password") {
if scimClient := d.Get("scim_client").(string); scimClient == string(sdk.ScimSecurityIntegrationScimClientAzure) {
if scimClient := d.Get("scim_client").(string); strings.EqualFold(strings.TrimSpace(scimClient), string(sdk.ScimSecurityIntegrationScimClientAzure)) {
return diag.Diagnostics{
{
Severity: diag.Error,
Expand Down
20 changes: 13 additions & 7 deletions pkg/resources/scim_integration_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func TestAcc_ScimIntegration_completeAzure(t *testing.T) {
networkPolicy, networkPolicyCleanup := acc.TestClient().NetworkPolicy.CreateNetworkPolicy(t)
t.Cleanup(networkPolicyCleanup)
id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
role := snowflakeroles.AadProvisioner
role := snowflakeroles.GenericScimProvisioner
m := func() map[string]config.Variable {
return map[string]config.Variable{
"name": config.StringVariable(id.Name()),
Expand Down Expand Up @@ -345,7 +345,7 @@ func TestAcc_ScimIntegration_InvalidCreateWithSyncPasswordWithAzure(t *testing.T
return map[string]config.Variable{
"name": config.StringVariable(id.Name()),
"scim_client": config.StringVariable(string(sdk.ScimSecurityIntegrationScimClientAzure)),
"run_as_role": config.StringVariable(snowflakeroles.AadProvisioner.Name()),
"run_as_role": config.StringVariable(snowflakeroles.GenericScimProvisioner.Name()),
"enabled": config.BoolVariable(true),
"sync_password": config.BoolVariable(false),
"network_policy_name": config.StringVariable(""),
Expand All @@ -359,7 +359,7 @@ func TestAcc_ScimIntegration_InvalidCreateWithSyncPasswordWithAzure(t *testing.T
tfversion.RequireAbove(tfversion.Version1_5_0),
},
ErrorCheck: helpers.AssertErrorContainsPartsFunc(t, []string{
"can not CREATE scim integration with field `sync_password` for scim_client = \"AZURE\"",
"can not CREATE scim integration with field `sync_password`",
}),
Steps: []resource.TestStep{
{
Expand All @@ -376,7 +376,7 @@ func TestAcc_ScimIntegration_InvalidUpdateWithSyncPasswordWithAzure(t *testing.T
c := map[string]config.Variable{
"name": config.StringVariable(id.Name()),
"scim_client": config.StringVariable(string(sdk.ScimSecurityIntegrationScimClientAzure)),
"run_as_role": config.StringVariable(snowflakeroles.AadProvisioner.Name()),
"run_as_role": config.StringVariable(snowflakeroles.GenericScimProvisioner.Name()),
"enabled": config.BoolVariable(true),
}
if complete {
Expand All @@ -394,7 +394,7 @@ func TestAcc_ScimIntegration_InvalidUpdateWithSyncPasswordWithAzure(t *testing.T
tfversion.RequireAbove(tfversion.Version1_5_0),
},
ErrorCheck: helpers.AssertErrorContainsPartsFunc(t, []string{
"can not SET and UNSET field `sync_password` for scim_client = \"AZURE\"",
"can not SET and UNSET field `sync_password`",
}),
Steps: []resource.TestStep{
{
Expand Down Expand Up @@ -499,7 +499,7 @@ func TestAcc_ScimIntegration_migrateFromVersion092EnabledFalse(t *testing.T) {

func TestAcc_ScimIntegration_migrateFromVersion093HandleSyncPassword(t *testing.T) {
id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
role := snowflakeroles.AadProvisioner
role := snowflakeroles.GenericScimProvisioner
resourceName := "snowflake_scim_integration.test"
resource.Test(t, resource.TestCase{
PreCheck: func() { acc.TestAccPreCheck(t) },
Expand All @@ -508,6 +508,7 @@ func TestAcc_ScimIntegration_migrateFromVersion093HandleSyncPassword(t *testing.
},

Steps: []resource.TestStep{
// create resource with v0.92
{
ExternalProviders: map[string]resource.ExternalProvider{
"snowflake": {
Expand All @@ -520,6 +521,7 @@ func TestAcc_ScimIntegration_migrateFromVersion093HandleSyncPassword(t *testing.
resource.TestCheckResourceAttr(resourceName, "name", id.Name()),
),
},
// migrate to v0.93 - there is a diff due to new field sync_password in state
{
ExternalProviders: map[string]resource.ExternalProvider{
"snowflake": {
Expand All @@ -528,14 +530,18 @@ func TestAcc_ScimIntegration_migrateFromVersion093HandleSyncPassword(t *testing.
},
},
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{plancheck.ExpectNonEmptyPlan()},
PreApply: []plancheck.PlanCheck{
plancheck.ExpectNonEmptyPlan(),
planchecks.ExpectChange(resourceName, "sync_password", tfjson.ActionUpdate, nil, sdk.String(r.BooleanDefault)),
},
},
Config: scimIntegrationv093(id.Name(), role.Name(), true, sdk.ScimSecurityIntegrationScimClientAzure),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "name", id.Name()),
),
ExpectError: regexp.MustCompile("invalid property 'SYNC_PASSWORD' for 'INTEGRATION"),
},
// check with newest version - the value in state was set to boolean default, so there should be no diff
{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Config: scimIntegrationv093(id.Name(), role.Name(), true, sdk.ScimSecurityIntegrationScimClientAzure),
Expand Down
7 changes: 7 additions & 0 deletions pkg/sdk/security_integrations_gen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,13 @@ func TestSecurityIntegrations_CreateScim(t *testing.T) {
assertOptsInvalidJoinedErrors(t, opts, errOneOf("CreateScimSecurityIntegrationOptions", "OrReplace", "IfNotExists"))
})

t.Run("validation: conflicting SyncPassword for Azure ScimClient", func(t *testing.T) {
opts := defaultOpts()
opts.ScimClient = ScimSecurityIntegrationScimClientAzure
opts.SyncPassword = Pointer(true)
assertOptsInvalidJoinedErrors(t, opts, NewError("SyncPassword is not supported for Azure scim client"))
})

t.Run("basic", func(t *testing.T) {
opts := defaultOpts()
opts.OrReplace = Pointer(true)
Expand Down
3 changes: 3 additions & 0 deletions pkg/sdk/security_integrations_validations_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ func (opts *CreateScimSecurityIntegrationOptions) validate() error {
if everyValueSet(opts.OrReplace, opts.IfNotExists) {
errs = append(errs, errOneOf("CreateScimSecurityIntegrationOptions", "OrReplace", "IfNotExists"))
}
if opts.ScimClient == ScimSecurityIntegrationScimClientAzure && opts.SyncPassword != nil {
errs = append(errs, NewError("SyncPassword is not supported for Azure scim client"))
}
return JoinErrors(errs...)
}

Expand Down

0 comments on commit 1266952

Please sign in to comment.