diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 0042e1300b..a7af7f165d 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -4,6 +4,31 @@ This document is meant to help you migrate your Terraform config to the new newe describe deprecations or breaking changes and help you to change your configuration to keep the same (or similar) behavior across different versions. +## v0.91.0 ➞ v0.92.0 +### snowflake_database new alternatives +As part of the [preparation for v1](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#preparing-essential-ga-objects-for-the-provider-v1), we split up the database resource into multiple ones: +- Standard database (in progress) +- Shared database - can be used as `snowflake_shared_database` (used to create databases from externally defined shares) +- Secondary database - can be used as `snowflake_secondary_database` (used to create replicas of databases from external sources) +From now on, please migrate and use the new database resources for their unique use cases. For more information, see the documentation for those resources on the [Terraform Registry](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs). + +The split was done (and will be done for several objects during the refactor) to simplify the resource on maintainability and usage level. +Its purpose was also to divide the resources by their specific purpose rather than cramping every use case of an object into one resource. + +### snowflake_scim_integration resource changes +#### *(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. + +#### *(behavior change)* Changed behavior of `enabled` + +Field `enabled` is now required. Previously the default value during create in Snowflake was `true`. If you created a resource with Terraform, please add `enabled = true` to have the same value. + +#### *(behavior change)* Force new for multiple attributes +Force new was added for the following attributes (because no usable SQL alter statements for them): +- `scim_client` +- `run_as_role` + ## v0.89.0 ➞ v0.90.0 ### snowflake_table resource changes #### *(behavior change)* Validation to column type added @@ -23,7 +48,7 @@ resource "snowflake_tag_masking_policy_association" "name" { masking_policy_id = snowflake_masking_policy.example_masking_policy.id } ``` - + After ```terraform resource "snowflake_tag_masking_policy_association" "name" { @@ -128,7 +153,7 @@ Descriptions of attributes were altered. More examples were added (both for old Previously, in `snowflake_database` when creating a database form share, it was possible to provide `from_share.provider` in the format of `.`. It worked even though we expected account locator because our "external" identifier wasn't quoting its string representation. -To be consistent with other identifier types, we quoted the output of "external" identifiers which makes such configurations break +To be consistent with other identifier types, we quoted the output of "external" identifiers which makes such configurations break (previously, they were working "by accident"). To fix it, the previous format of `.` has to be changed to account locator format `` (mind that it's now case-sensitive). The account locator can be retrieved by calling `select current_account();` on the sharing account. In the future we would like to eventually come back to the `.` format as it's recommended by Snowflake. diff --git a/docs/resources/scim_integration.md b/docs/resources/scim_integration.md index 67291774b0..f9d68cbb94 100644 --- a/docs/resources/scim_integration.md +++ b/docs/resources/scim_integration.md @@ -12,11 +12,22 @@ description: |- ## Example Usage ```terraform -resource "snowflake_scim_integration" "aad" { - name = "AAD_PROVISIONING" - network_policy = "AAD_NETWORK_POLICY" - provisioner_role = "AAD_PROVISIONER" - scim_client = "AZURE" +# basic resource +resource "snowflake_scim_integration" "test" { + name = "test" + enabled = true + scim_client = "GENERIC" + sync_password = true +} +# resource with all fields set +resource "snowflake_scim_integration" "test" { + name = "test" + enabled = true + scim_client = "GENERIC" + sync_password = true + network_policy = "network_policy_test" + run_as_role = "GENERIC_SCIM_PROVISIONER" + comment = "foo" } ``` @@ -25,13 +36,16 @@ resource "snowflake_scim_integration" "aad" { ### Required -- `name` (String) Specifies the name of the SCIM integration. This name follows the rules for Object Identifiers. The name should be unique among security integrations in your account. -- `provisioner_role` (String) Specify the SCIM role in Snowflake that owns any users and roles that are imported from the identity provider into Snowflake using SCIM. -- `scim_client` (String) Specifies the client type for the scim integration +- `enabled` (Boolean) Specify whether the security integration is enabled. +- `name` (String) String that specifies the identifier (i.e. name) for the integration; must be unique in your account. +- `run_as_role` (String) Specify the SCIM role in Snowflake that owns any users and roles that are imported from the identity provider into Snowflake using SCIM. Provider assumes that the specified role is already provided. Valid options are: [OKTA_PROVISIONER AAD_PROVISIONER GENERIC_SCIM_PROVISIONER]. +- `scim_client` (String) Specifies the client type for the scim integration. Valid options are: [OKTA AZURE GENERIC]. ### Optional -- `network_policy` (String) Specifies an existing network policy active for your account. The network policy restricts the list of user IP addresses when exchanging an authorization code for an access or refresh token and when using a refresh token to obtain a new access token. If this parameter is not set, the network policy for the account (if any) is used instead. +- `comment` (String) Specifies a comment for the integration. +- `network_policy` (String) Specifies an existing network policy that controls SCIM network traffic. +- `sync_password` (Boolean) Specifies whether to enable or disable the synchronization of a user password from an Okta SCIM client as part of the API request to Snowflake. ### Read-Only diff --git a/examples/resources/snowflake_scim_integration/resource.tf b/examples/resources/snowflake_scim_integration/resource.tf index 4d2f73f23a..8e3417fae2 100644 --- a/examples/resources/snowflake_scim_integration/resource.tf +++ b/examples/resources/snowflake_scim_integration/resource.tf @@ -1,6 +1,17 @@ -resource "snowflake_scim_integration" "aad" { - name = "AAD_PROVISIONING" - network_policy = "AAD_NETWORK_POLICY" - provisioner_role = "AAD_PROVISIONER" - scim_client = "AZURE" -} \ No newline at end of file +# basic resource +resource "snowflake_scim_integration" "test" { + name = "test" + enabled = true + scim_client = "GENERIC" + sync_password = true +} +# resource with all fields set +resource "snowflake_scim_integration" "test" { + name = "test" + enabled = true + scim_client = "GENERIC" + sync_password = true + network_policy = "network_policy_test" + run_as_role = "GENERIC_SCIM_PROVISIONER" + comment = "foo" +} diff --git a/pkg/acceptance/check_destroy.go b/pkg/acceptance/check_destroy.go index 31273ce056..f9c46a102e 100644 --- a/pkg/acceptance/check_destroy.go +++ b/pkg/acceptance/check_destroy.go @@ -136,6 +136,9 @@ var showByIdFunctions = map[resources.Resource]showByIdFunc{ resources.Schema: func(ctx context.Context, client *sdk.Client, id sdk.ObjectIdentifier) error { return runShowById(ctx, id, client.Schemas.ShowByID) }, + resources.ScimSecurityIntegration: func(ctx context.Context, client *sdk.Client, id sdk.ObjectIdentifier) error { + return runShowById(ctx, id, client.SecurityIntegrations.ShowByID) + }, resources.SecondaryDatabase: func(ctx context.Context, client *sdk.Client, id sdk.ObjectIdentifier) error { return runShowById(ctx, id, client.Databases.ShowByID) }, diff --git a/pkg/acceptance/helpers/common.go b/pkg/acceptance/helpers/common.go index a6057b1730..4c4aa95fb6 100644 --- a/pkg/acceptance/helpers/common.go +++ b/pkg/acceptance/helpers/common.go @@ -4,8 +4,14 @@ import ( "context" "fmt" "log" + "regexp" + "strings" + "testing" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/stretchr/testify/assert" ) func EnsureQuotedIdentifiersIgnoreCaseIsSetToFalse(client *sdk.Client, ctx context.Context) error { @@ -19,3 +25,59 @@ func EnsureQuotedIdentifiersIgnoreCaseIsSetToFalse(client *sdk.Client, ctx conte } return nil } + +func EnsureScimProvisionerRolesExist(client *sdk.Client, ctx context.Context) error { + log.Printf("[DEBUG] Making sure Scim Provisioner roles exist") + roleIDs := []sdk.AccountObjectIdentifier{snowflakeroles.GenericScimProvisioner, snowflakeroles.OktaProvisioner} + currentRoleID, err := client.ContextFunctions.CurrentRole(ctx) + if err != nil { + return err + } + for _, roleID := range roleIDs { + _, err := client.Roles.ShowByID(ctx, roleID) + if err != nil { + return err + } + grants, err := client.Grants.Show(ctx, &sdk.ShowGrantOptions{ + Of: &sdk.ShowGrantsOf{ + Role: roleID, + }, + }) + if err != nil { + return err + } + if !hasGranteeName(grants, currentRoleID) { + return fmt.Errorf("role %s not granted to %s", currentRoleID.Name(), roleID.Name()) + } + } + return nil +} + +func hasGranteeName(grants []sdk.Grant, role sdk.AccountObjectIdentifier) bool { + for _, grant := range grants { + if grant.GranteeName == role { + return true + } + } + return false +} + +// MatchAllStringsInOrderNonOverlapping returns a regex matching every string in parts. Matchings are non overlapping. +func MatchAllStringsInOrderNonOverlapping(parts []string) *regexp.Regexp { + escapedParts := make([]string, len(parts)) + for i := range parts { + escapedParts[i] = regexp.QuoteMeta(parts[i]) + } + return regexp.MustCompile(strings.Join(escapedParts, "((.|\n)*)")) +} + +// AssertErrorContainsPartsFunc returns a function asserting error message contains each string in parts +func AssertErrorContainsPartsFunc(t *testing.T, parts []string) resource.ErrorCheckFunc { + t.Helper() + return func(err error) error { + for _, part := range parts { + assert.Contains(t, err.Error(), part) + } + return nil + } +} diff --git a/pkg/acceptance/helpers/common_test.go b/pkg/acceptance/helpers/common_test.go new file mode 100644 index 0000000000..ccad8dcb51 --- /dev/null +++ b/pkg/acceptance/helpers/common_test.go @@ -0,0 +1,58 @@ +package helpers + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestMatchAllStringsInOrderNonOverlapping(t *testing.T) { + testCases := map[string]struct { + parts []string + text string + wantMatch bool + }{ + "empty parts and text": { + parts: []string{}, + text: "", + wantMatch: true, + }, + "empty parts": { + parts: []string{}, + text: "xyz", + wantMatch: true, + }, + "empty text": { + parts: []string{"a", "b"}, + text: "", + }, + "matching non empty": { + parts: []string{"a", "b"}, + text: "xyaxyb", + wantMatch: true, + }, + "partial matching": { + parts: []string{"a", "b"}, + text: "axyz", + }, + "not matching": { + parts: []string{"a", "b"}, + text: "xyz", + }, + "wrong order": { + parts: []string{"a", "b"}, + text: "ba", + }, + "overlapping match": { + parts: []string{"abb", "bba"}, + text: "abba", + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + regex := MatchAllStringsInOrderNonOverlapping(tc.parts) + require.Equal(t, tc.wantMatch, regex.Match([]byte(tc.text))) + }) + } +} diff --git a/pkg/acceptance/helpers/security_integration_client.go b/pkg/acceptance/helpers/security_integration_client.go index 1b47feaaa6..08d854fbb4 100644 --- a/pkg/acceptance/helpers/security_integration_client.go +++ b/pkg/acceptance/helpers/security_integration_client.go @@ -45,7 +45,7 @@ func (c *SecurityIntegrationClient) CreateSaml2WithRequest(t *testing.T, request func (c *SecurityIntegrationClient) CreateScim(t *testing.T) (*sdk.SecurityIntegration, func()) { t.Helper() - return c.CreateScimWithRequest(t, sdk.NewCreateScimSecurityIntegrationRequest(c.ids.RandomAccountObjectIdentifier(), false, sdk.ScimSecurityIntegrationScimClientGeneric, sdk.ScimSecurityIntegrationRunAsRoleGenericScimProvisioner)) + return c.CreateScimWithRequest(t, sdk.NewCreateScimSecurityIntegrationRequest(c.ids.RandomAccountObjectIdentifier(), sdk.ScimSecurityIntegrationScimClientGeneric, sdk.ScimSecurityIntegrationRunAsRoleGenericScimProvisioner)) } func (c *SecurityIntegrationClient) CreateScimWithRequest(t *testing.T, request *sdk.CreateScimSecurityIntegrationRequest) (*sdk.SecurityIntegration, func()) { diff --git a/pkg/acceptance/testing.go b/pkg/acceptance/testing.go index c826515dff..510b881439 100644 --- a/pkg/acceptance/testing.go +++ b/pkg/acceptance/testing.go @@ -156,6 +156,14 @@ func TestAccPreCheck(t *testing.T) { if err := helpers.EnsureQuotedIdentifiersIgnoreCaseIsSetToFalse(atc.secondaryClient, ctx); err != nil { t.Fatal(err) } + + if err := helpers.EnsureScimProvisionerRolesExist(atc.client, ctx); err != nil { + t.Fatal(err) + } + + if err := helpers.EnsureScimProvisionerRolesExist(atc.secondaryClient, ctx); err != nil { + t.Fatal(err) + } }) } diff --git a/pkg/datasources/system_generate_scim_access_token_acceptance_test.go b/pkg/datasources/system_generate_scim_access_token_acceptance_test.go index 6cc9dc446a..f7ecbaefc2 100644 --- a/pkg/datasources/system_generate_scim_access_token_acceptance_test.go +++ b/pkg/datasources/system_generate_scim_access_token_acceptance_test.go @@ -51,8 +51,9 @@ func generateAccessTokenConfig(name string) string { resource "snowflake_scim_integration" "azured" { name = "%s" + enabled = true scim_client = "AZURE" - provisioner_role = snowflake_role.azured.name + run_as_role = snowflake_role.azured.name depends_on = [ snowflake_grant_privileges_to_account_role.azure_grants, snowflake_grant_account_role.azured diff --git a/pkg/provider/resources/resources.go b/pkg/provider/resources/resources.go index 550836803e..bd97c4e572 100644 --- a/pkg/provider/resources/resources.go +++ b/pkg/provider/resources/resources.go @@ -28,6 +28,7 @@ const ( Role resource = "snowflake_role" RowAccessPolicy resource = "snowflake_row_access_policy" Schema resource = "snowflake_schema" + ScimSecurityIntegration resource = "snowflake_scim_integration" SecondaryDatabase resource = "snowflake_secondary_database" Sequence resource = "snowflake_sequence" Share resource = "snowflake_share" diff --git a/pkg/resources/custom_diffs.go b/pkg/resources/custom_diffs.go index 270766a957..80800e46e8 100644 --- a/pkg/resources/custom_diffs.go +++ b/pkg/resources/custom_diffs.go @@ -4,6 +4,7 @@ import ( "context" "strconv" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" @@ -47,3 +48,21 @@ func NestedValueComputedIf(key string, showParam func(client *sdk.Client) (*sdk. return param.Value != valueToString(stateValue[0].(map[string]any)["value"]) }) } + +func BoolComputedIf(key string, getDefault func(client *sdk.Client, id sdk.AccountObjectIdentifier) (string, error)) schema.CustomizeDiffFunc { + return customdiff.ComputedIf(key, func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) bool { + configValue := d.GetRawConfig().AsValueMap()[key] + if !configValue.IsNull() { + return false + } + + client := meta.(*provider.Context).Client + + def, err := getDefault(client, helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier)) + if err != nil { + return false + } + stateValue := d.Get(key).(bool) + return def != strconv.FormatBool(stateValue) + }) +} diff --git a/pkg/resources/helpers.go b/pkg/resources/helpers.go index 9400cdb5aa..45c9389416 100644 --- a/pkg/resources/helpers.go +++ b/pkg/resources/helpers.go @@ -36,6 +36,14 @@ func ignoreTrimSpaceSuppressFunc(_, old, new string, _ *schema.ResourceData) boo return strings.TrimSpace(old) == strings.TrimSpace(new) } +func ignoreCaseSuppressFunc(_, old, new string, _ *schema.ResourceData) bool { + return strings.EqualFold(old, new) +} + +func ignoreCaseAndTrimSpaceSuppressFunc(_, old, new string, _ *schema.ResourceData) bool { + return strings.EqualFold(strings.TrimSpace(old), strings.TrimSpace(new)) +} + func setIntProperty(d *schema.ResourceData, key string, property *sdk.IntProperty) error { if property != nil && property.Value != nil { if err := d.Set(key, *property.Value); err != nil { diff --git a/pkg/resources/scim_integration.go b/pkg/resources/scim_integration.go index 50242926d1..b6d83b3ece 100644 --- a/pkg/resources/scim_integration.go +++ b/pkg/resources/scim_integration.go @@ -1,13 +1,19 @@ package resources import ( + "context" + "errors" "fmt" "log" "strings" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake" + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) @@ -17,29 +23,31 @@ var scimIntegrationSchema = map[string]*schema.Schema{ Type: schema.TypeString, Required: true, ForceNew: true, - Description: "Specifies the name of the SCIM integration. This name follows the rules for Object Identifiers. The name should be unique among security integrations in your account.", + Description: "String that specifies the identifier (i.e. name) for the integration; must be unique in your account.", }, - "scim_client": { - Type: schema.TypeString, + "enabled": { + Type: schema.TypeBool, Required: true, - Description: "Specifies the client type for the scim integration", - ValidateFunc: validation.StringInSlice([]string{ - "OKTA", "AZURE", "GENERIC", - }, true), - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - normalize := func(s string) string { - return strings.ToUpper(strings.ReplaceAll(s, "-", "")) - } - return normalize(old) == normalize(new) + Description: "Specify whether the security integration is enabled. ", + DiffSuppressFunc: func(k, oldValue, newValue string, d *schema.ResourceData) bool { + return d.GetRawConfig().AsValueMap()["enabled"].IsNull() }, }, - "provisioner_role": { - Type: schema.TypeString, - Required: true, - Description: "Specify the SCIM role in Snowflake that owns any users and roles that are imported from the identity provider into Snowflake using SCIM.", - ValidateFunc: validation.StringInSlice([]string{ - "OKTA_PROVISIONER", "AAD_PROVISIONER", "GENERIC_SCIM_PROVISIONER", - }, true), + "scim_client": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: fmt.Sprintf("Specifies the client type for the scim integration. Valid options are: %v.", sdk.AsStringList(sdk.AllScimSecurityIntegrationScimClients)), + ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllScimSecurityIntegrationScimClients), true), + DiffSuppressFunc: ignoreCaseAndTrimSpaceSuppressFunc, + }, + "run_as_role": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: fmt.Sprintf("Specify the SCIM role in Snowflake that owns any users and roles that are imported from the identity provider into Snowflake using SCIM."+ + " Provider assumes that the specified role is already provided. Valid options are: %v.", sdk.AllScimSecurityIntegrationRunAsRoles), + ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllScimSecurityIntegrationRunAsRoles), true), DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { normalize := func(s string) string { return strings.ToUpper(strings.ReplaceAll(s, "-", "")) @@ -48,9 +56,24 @@ var scimIntegrationSchema = map[string]*schema.Schema{ }, }, "network_policy": { + Type: schema.TypeString, + ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](), + Optional: true, + Description: "Specifies an existing network policy that controls SCIM network traffic.", + DiffSuppressFunc: func(_, old, new string, d *schema.ResourceData) bool { + return sdk.NewAccountObjectIdentifierFromFullyQualifiedName(old) == sdk.NewAccountObjectIdentifierFromFullyQualifiedName(new) + }, + }, + "sync_password": { + Type: schema.TypeBool, + Optional: true, + Computed: true, + Description: "Specifies whether to enable or disable the synchronization of a user password from an Okta SCIM client as part of the API request to Snowflake.", + }, + "comment": { Type: schema.TypeString, Optional: true, - Description: "Specifies an existing network policy active for your account. The network policy restricts the list of user IP addresses when exchanging an authorization code for an access or refresh token and when using a refresh token to obtain a new access token. If this parameter is not set, the network policy for the account (if any) is used instead.", + Description: "Specifies a comment for the integration.", }, "created_on": { Type: schema.TypeString, @@ -59,155 +82,206 @@ var scimIntegrationSchema = map[string]*schema.Schema{ }, } -// SCIMIntegration returns a pointer to the resource representing a network policy. func SCIMIntegration() *schema.Resource { return &schema.Resource{ - Create: CreateSCIMIntegration, - Read: ReadSCIMIntegration, - Update: UpdateSCIMIntegration, - Delete: DeleteSCIMIntegration, - + SchemaVersion: 1, + + CreateContext: CreateContextSCIMIntegration, + ReadContext: ReadContextSCIMIntegration, + UpdateContext: UpdateContextSCIMIntegration, + DeleteContext: DeleteContextSCIMIntegration, + CustomizeDiff: customdiff.All( + BoolComputedIf("sync_password", func(client *sdk.Client, id sdk.AccountObjectIdentifier) (string, error) { + props, err := client.SecurityIntegrations.Describe(context.Background(), id) + if err != nil { + return "", err + } + for _, prop := range props { + if prop.GetName() == "SYNC_PASSWORD" { + return prop.GetDefault(), nil + } + } + return "", fmt.Errorf("") + }), + ), Schema: scimIntegrationSchema, Importer: &schema.ResourceImporter{ StateContext: schema.ImportStatePassthroughContext, }, + StateUpgraders: []schema.StateUpgrader{ + { + Version: 0, + // setting type to cty.EmptyObject is a bit hacky here but following https://developer.hashicorp.com/terraform/plugin/framework/migrating/resources/state-upgrade#sdkv2-1 would require lots of repetitive code; this should work with cty.EmptyObject + Type: cty.EmptyObject, + Upgrade: v091ScimIntegrationStateUpgrader, + }, + }, } } -// CreateSCIMIntegration implements schema.CreateFunc. -func CreateSCIMIntegration(d *schema.ResourceData, meta interface{}) error { +func CreateContextSCIMIntegration(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*provider.Context).Client - db := client.GetConn().DB name := d.Get("name").(string) - - stmt := snowflake.NewSCIMIntegrationBuilder(name).Create() - - // Set required fields - stmt.SetRaw(`TYPE=SCIM`) - stmt.SetString(`SCIM_CLIENT`, d.Get("scim_client").(string)) - stmt.SetString(`RUN_AS_ROLE`, d.Get("provisioner_role").(string)) - - // Set optional fields - if _, ok := d.GetOk("network_policy"); ok { - stmt.SetString(`NETWORK_POLICY`, d.Get("network_policy").(string)) + id := sdk.NewAccountObjectIdentifier(name) + scimClientRaw := d.Get("scim_client").(string) + runAsRoleRaw := d.Get("run_as_role").(string) + scimClient, err := sdk.ToScimSecurityIntegrationScimClientOption(scimClientRaw) + if err != nil { + return diag.FromErr(err) + } + runAsRole, err := sdk.ToScimSecurityIntegrationRunAsRoleOption(runAsRoleRaw) + if err != nil { + return diag.FromErr(err) } + req := sdk.NewCreateScimSecurityIntegrationRequest(id, scimClient, runAsRole).WithEnabled(d.Get("enabled").(bool)) - if err := snowflake.Exec(db, stmt.Statement()); err != nil { - return fmt.Errorf("error creating security integration") + if v, ok := d.GetOk("network_policy"); ok { + req.WithNetworkPolicy(sdk.NewAccountObjectIdentifier(v.(string))) + } + if !d.GetRawConfig().AsValueMap()["sync_password"].IsNull() { + req.WithSyncPassword(d.Get("sync_password").(bool)) + } + if v, ok := d.GetOk("comment"); ok { + req.WithComment(v.(string)) + } + if err := client.SecurityIntegrations.CreateScim(ctx, req); err != nil { + return diag.FromErr(err) } - d.SetId(name) + d.SetId(helpers.EncodeSnowflakeID(id)) - return ReadSCIMIntegration(d, meta) + return ReadContextSCIMIntegration(ctx, d, meta) } -// ReadSCIMIntegration implements schema.ReadFunc. -func ReadSCIMIntegration(d *schema.ResourceData, meta interface{}) error { +func ReadContextSCIMIntegration(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*provider.Context).Client - db := client.GetConn().DB - id := d.Id() - - stmt := snowflake.NewSCIMIntegrationBuilder(id).Show() - row := snowflake.QueryRow(db, stmt) + id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) - // Some properties can come from the SHOW INTEGRATION call - - s, err := snowflake.ScanScimIntegration(row) + integration, err := client.SecurityIntegrations.ShowByID(ctx, id) if err != nil { - return fmt.Errorf("could not show security integration") + if errors.Is(err, sdk.ErrObjectNotFound) { + d.SetId("") + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Warning, + Summary: "Failed to query security integration. Marking the resource as removed.", + Detail: fmt.Sprintf("Security integration name: %s, Err: %s", id.FullyQualifiedName(), err), + }, + } + } + return diag.FromErr(err) } - // Note: category must be Security or something is broken - if c := s.Category.String; c != "SECURITY" { - return fmt.Errorf("expected %v to be an Security integration, got %v", id, c) + if c := integration.Category; c != sdk.SecurityIntegrationCategory { + return diag.FromErr(fmt.Errorf("expected %v to be a SECURITY integration, got %v", id, c)) } - if err := d.Set("scim_client", strings.TrimPrefix(s.IntegrationType.String, "SCIM - ")); err != nil { - return err + if err := d.Set("name", integration.Name); err != nil { + return diag.FromErr(err) } - if err := d.Set("name", s.Name.String); err != nil { - return err + if err := d.Set("comment", integration.Comment); err != nil { + return diag.FromErr(err) } - if err := d.Set("created_on", s.CreatedOn.String); err != nil { - return err + if err := d.Set("created_on", integration.CreatedOn.String()); err != nil { + return diag.FromErr(err) } - // Some properties come from the DESCRIBE INTEGRATION call - // We need to grab them in a loop - var k, pType string - var v, unused interface{} - stmt = snowflake.NewSCIMIntegrationBuilder(id).Describe() - rows, err := db.Query(stmt) + if err := d.Set("enabled", integration.Enabled); err != nil { + return diag.FromErr(err) + } + + scimClient, err := integration.SubType() if err != nil { - return fmt.Errorf("could not describe security integration") + return diag.FromErr(err) } - defer rows.Close() - for rows.Next() { - if err := rows.Scan(&k, &pType, &v, &unused); err != nil { - return fmt.Errorf("unable to parse security integration rows") - } - switch k { + if err := d.Set("scim_client", scimClient); err != nil { + return diag.FromErr(err) + } + integrationProperties, err := client.SecurityIntegrations.Describe(ctx, id) + if err != nil { + return diag.FromErr(err) + } + for _, property := range integrationProperties { + name := property.Name + value := property.Value + switch name { + case "ENABLED", "COMMENT": + // We set this using the SHOW INTEGRATION call so let's ignore it here case "NETWORK_POLICY": - if err := d.Set("network_policy", v.(string)); err != nil { - return fmt.Errorf("unable to set network policy for security integration") + networkPolicyID := sdk.NewAccountObjectIdentifier(value) + if err := d.Set("network_policy", networkPolicyID.FullyQualifiedName()); err != nil { + return diag.FromErr(err) + } + case "SYNC_PASSWORD": + if err := d.Set("sync_password", helpers.StringToBool(value)); err != nil { + return diag.FromErr(err) } case "RUN_AS_ROLE": - if err := d.Set("provisioner_role", v.(string)); err != nil { - return fmt.Errorf("unable to set provisioner role for security integration") + if err := d.Set("run_as_role", value); err != nil { + return diag.FromErr(err) } default: - log.Printf("[WARN] unexpected security integration property %v returned from Snowflake", k) + log.Printf("[WARN] unexpected property %v returned from Snowflake", name) } } - return err + return nil } -// UpdateSCIMIntegration implements schema.UpdateFunc. -func UpdateSCIMIntegration(d *schema.ResourceData, meta interface{}) error { +func UpdateContextSCIMIntegration(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*provider.Context).Client - db := client.GetConn().DB - id := d.Id() - - stmt := snowflake.NewSCIMIntegrationBuilder(id).Alter() - - var runSetStatement bool - - if d.HasChange("scim_client") { - runSetStatement = true - stmt.SetString(`SCIM_CLIENT`, d.Get("scim_client").(string)) - } - - if d.HasChange("provisioner_role") { - runSetStatement = true - stmt.SetString(`RUN_AS_ROLE`, d.Get("provisioner_role").(string)) + id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) + set, unset := sdk.NewScimIntegrationSetRequest(), sdk.NewScimIntegrationUnsetRequest() + if d.HasChange("enabled") { + set.WithEnabled(d.Get("enabled").(bool)) } - - // We need to UNSET this if we remove all api blocked prefixes. if d.HasChange("network_policy") { - v := d.Get("network_policy").(string) - if len(v) == 0 { - if err := snowflake.Exec(db, fmt.Sprintf(`ALTER SECURITY INTEGRATION %v UNSET NETWORK_POLICY`, id)); err != nil { - return fmt.Errorf("error unsetting network_policy") - } + networkPolicyID := sdk.NewAccountObjectIdentifier(d.Get("network_policy").(string)) + if networkPolicyID.Name() != "" { + set.WithNetworkPolicy(networkPolicyID) } else { - runSetStatement = true - stmt.SetString(`NETWORK_POLICY`, d.Get("network_policy").(string)) + unset.WithNetworkPolicy(true) } } + if !d.GetRawConfig().AsValueMap()["sync_password"].IsNull() { + set.WithSyncPassword(d.Get("sync_password").(bool)) + } else { + unset.WithSyncPassword(true) + } - if runSetStatement { - if err := snowflake.Exec(db, stmt.Statement()); err != nil { - return fmt.Errorf("error updating security integration") + if d.HasChange("comment") { + set.WithComment(sdk.StringAllowEmpty{Value: d.Get("comment").(string)}) + } + if (*set != sdk.ScimIntegrationSetRequest{}) { + if err := client.SecurityIntegrations.AlterScim(ctx, sdk.NewAlterScimSecurityIntegrationRequest(id).WithSet(*set)); err != nil { + return diag.FromErr(err) } } - - return ReadSCIMIntegration(d, meta) + if (*unset != sdk.ScimIntegrationUnsetRequest{}) { + if err := client.SecurityIntegrations.AlterScim(ctx, sdk.NewAlterScimSecurityIntegrationRequest(id).WithUnset(*unset)); err != nil { + return diag.FromErr(err) + } + } + return ReadContextSCIMIntegration(ctx, d, meta) } -// DeleteSCIMIntegration implements schema.DeleteFunc. -func DeleteSCIMIntegration(d *schema.ResourceData, meta interface{}) error { - return DeleteResource("", snowflake.NewSCIMIntegrationBuilder)(d, meta) +func DeleteContextSCIMIntegration(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) + client := meta.(*provider.Context).Client + + err := client.SecurityIntegrations.Drop(ctx, sdk.NewDropSecurityIntegrationRequest(sdk.NewAccountObjectIdentifier(id.Name())).WithIfExists(true)) + if err != nil { + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Error deleting integration", + Detail: fmt.Sprintf("id %v err = %v", id.Name(), err), + }, + } + } + + d.SetId("") + return nil } diff --git a/pkg/resources/scim_integration_acceptance_test.go b/pkg/resources/scim_integration_acceptance_test.go index b69b729eba..e71c95885c 100644 --- a/pkg/resources/scim_integration_acceptance_test.go +++ b/pkg/resources/scim_integration_acceptance_test.go @@ -2,78 +2,265 @@ package resources_test import ( "fmt" + "strings" "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-testing/config" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/tfversion" ) -func TestAcc_ScimIntegration(t *testing.T) { - scimIntName := acc.TestClient().Ids.Alpha() - scimProvisionerRole := "AAD_PROVISIONER" - scimNetworkPolicy := acc.TestClient().Ids.Alpha() - +func TestAcc_ScimIntegration_basic(t *testing.T) { + networkPolicy, networkPolicyCleanup := acc.TestClient().NetworkPolicy.CreateNetworkPolicy(t) + t.Cleanup(networkPolicyCleanup) + role, role2 := snowflakeroles.GenericScimProvisioner, snowflakeroles.OktaProvisioner + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + m := func(enabled bool, scimClient sdk.ScimSecurityIntegrationScimClientOption, runAsRole sdk.AccountObjectIdentifier, complete bool) map[string]config.Variable { + c := map[string]config.Variable{ + "name": config.StringVariable(id.Name()), + "scim_client": config.StringVariable(string(scimClient)), + "run_as_role": config.StringVariable(runAsRole.Name()), + "enabled": config.BoolVariable(enabled), + } + if complete { + c["sync_password"] = config.BoolVariable(false) + c["network_policy_name"] = config.StringVariable(networkPolicy.Name) + c["comment"] = config.StringVariable("foo") + } + return c + } resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, PreCheck: func() { acc.TestAccPreCheck(t) }, TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, - CheckDestroy: nil, + CheckDestroy: acc.CheckDestroy(t, resources.ScimSecurityIntegration), Steps: []resource.TestStep{ { - Config: scimIntegrationConfigAzure(scimIntName, scimProvisionerRole, scimNetworkPolicy), + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_ScimIntegration/basic"), + ConfigVariables: m(false, sdk.ScimSecurityIntegrationScimClientGeneric, role, false), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "enabled", "false"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "scim_client", "GENERIC"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "run_as_role", role.Name()), + resource.TestCheckResourceAttrSet("snowflake_scim_integration.test", "sync_password"), + resource.TestCheckResourceAttrSet("snowflake_scim_integration.test", "created_on"), + ), + }, + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_ScimIntegration/complete"), + ConfigVariables: m(true, sdk.ScimSecurityIntegrationScimClientOkta, role2, true), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_scim_integration.test", "name", scimIntName), - resource.TestCheckResourceAttr("snowflake_scim_integration.test", "scim_client", "AZURE"), - resource.TestCheckResourceAttr("snowflake_scim_integration.test", "provisioner_role", scimProvisionerRole), - resource.TestCheckResourceAttr("snowflake_scim_integration.test", "network_policy", scimNetworkPolicy), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "enabled", "true"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "scim_client", "OKTA"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "run_as_role", role2.Name()), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "network_policy", sdk.NewAccountObjectIdentifier(networkPolicy.Name).FullyQualifiedName()), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "sync_password", "false"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "comment", "foo"), resource.TestCheckResourceAttrSet("snowflake_scim_integration.test", "created_on"), ), }, { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_ScimIntegration/basic"), + ConfigVariables: m(true, sdk.ScimSecurityIntegrationScimClientOkta, role2, true), ResourceName: "snowflake_scim_integration.test", ImportState: true, ImportStateVerify: true, }, + // unset + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_ScimIntegration/basic"), + ConfigVariables: m(true, sdk.ScimSecurityIntegrationScimClientOkta, role2, false), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "enabled", "true"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "scim_client", "OKTA"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "run_as_role", role2.Name()), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "network_policy", ""), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "sync_password", "true"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "comment", ""), + resource.TestCheckResourceAttrSet("snowflake_scim_integration.test", "created_on"), + ), + }, }, }) } -func scimIntegrationConfigAzure(name string, role string, policy string) string { - return fmt.Sprintf(` - resource "snowflake_role" "azure" { - name = "%s" - comment = "test comment" +func TestAcc_ScimIntegration_complete(t *testing.T) { + networkPolicy, networkPolicyCleanup := acc.TestClient().NetworkPolicy.CreateNetworkPolicy(t) + t.Cleanup(networkPolicyCleanup) + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + role := snowflakeroles.GenericScimProvisioner + m := func() map[string]config.Variable { + return map[string]config.Variable{ + "name": config.StringVariable(id.Name()), + "enabled": config.BoolVariable(false), + "scim_client": config.StringVariable(strings.ToLower(string(sdk.ScimSecurityIntegrationScimClientGeneric))), + "sync_password": config.BoolVariable(false), + "network_policy_name": config.StringVariable(networkPolicy.Name), + "run_as_role": config.StringVariable(role.Name()), + "comment": config.StringVariable("foo"), + } } + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.ScimSecurityIntegration), + Steps: []resource.TestStep{ + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_ScimIntegration/complete"), + ConfigVariables: m(), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "enabled", "false"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "scim_client", "GENERIC"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "run_as_role", role.Name()), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "network_policy", sdk.NewAccountObjectIdentifier(networkPolicy.Name).FullyQualifiedName()), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "sync_password", "false"), + resource.TestCheckResourceAttr("snowflake_scim_integration.test", "comment", "foo"), + resource.TestCheckResourceAttrSet("snowflake_scim_integration.test", "created_on"), + ), + }, + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_ScimIntegration/complete"), + ConfigVariables: m(), + ResourceName: "snowflake_scim_integration.test", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} - resource "snowflake_grant_privileges_to_account_role" "azure_grants" { - account_role_name = snowflake_role.azure.name - privileges = ["CREATE USER", "CREATE ROLE"] - on_account = true +func TestAcc_ScimIntegration_invalid(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + m := func() map[string]config.Variable { + return map[string]config.Variable{ + "name": config.StringVariable(id.Name()), + "enabled": config.BoolVariable(false), + "scim_client": config.StringVariable("invalid"), + "sync_password": config.BoolVariable(false), + "network_policy_name": config.StringVariable("foo"), + "run_as_role": config.StringVariable("invalid"), + "comment": config.StringVariable("foo"), + } } + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + Steps: []resource.TestStep{ + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_ScimIntegration/complete"), + ConfigVariables: m(), + ExpectError: helpers.MatchAllStringsInOrderNonOverlapping([]string{ + `expected scim_client to be one of ["OKTA" "AZURE" "GENERIC"], got invalid`, + `expected run_as_role to be one of ["OKTA_PROVISIONER" "AAD_PROVISIONER" "GENERIC_SCIM_PROVISIONER"], got invalid`, + }), + }, + }, + }) +} - resource "snowflake_grant_account_role" "azure" { - role_name = snowflake_role.azure.name - parent_role_name = "ACCOUNTADMIN" +func TestAcc_ScimIntegration_InvalidIncomplete(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + m := func() map[string]config.Variable { + return map[string]config.Variable{ + "name": config.StringVariable(id.Name()), + } } + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + ErrorCheck: helpers.AssertErrorContainsPartsFunc(t, []string{ + `The argument "scim_client" is required, but no definition was found.`, + `The argument "run_as_role" is required, but no definition was found.`, + }), + Steps: []resource.TestStep{ + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_ScimIntegration/invalid"), + ConfigVariables: m(), + }, + }, + }) +} - resource "snowflake_network_policy" "azure" { - name = "%s" - allowed_ip_list = ["192.168.0.100/24", "29.254.123.20"] - } +func TestAcc_ScimIntegration_migrateFromVersion091(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + role := snowflakeroles.GenericScimProvisioner + resourceName := "snowflake_scim_integration.test" + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, - resource "snowflake_scim_integration" "test" { - name = "%s" - scim_client = "AZURE" - provisioner_role = snowflake_role.azure.name - network_policy = snowflake_network_policy.azure.name - depends_on = [ - snowflake_grant_privileges_to_account_role.azure_grants, - snowflake_grant_account_role.azure - ] - } - `, role, policy, name) + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.91.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: scimIntegrationv091(id.Name(), role.Name()), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "provisioner_role", role.Name()), + ), + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + Config: scimIntegrationv092(id.Name(), role.Name()), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{plancheck.ExpectEmptyPlan()}, + }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "run_as_role", role.Name()), + ), + }, + }, + }) +} + +func scimIntegrationv091(name, roleName string) string { + s := ` +resource "snowflake_scim_integration" "test" { + name = "%s" + scim_client = "%s" + provisioner_role = "%s" +} +` + return fmt.Sprintf(s, name, sdk.ScimSecurityIntegrationScimClientGeneric, roleName) +} + +func scimIntegrationv092(name, roleName string) string { + s := ` +resource "snowflake_scim_integration" "test" { + name = "%s" + enabled = true + scim_client = "%s" + run_as_role = "%s" +} +` + return fmt.Sprintf(s, name, sdk.ScimSecurityIntegrationScimClientGeneric, roleName) } diff --git a/pkg/resources/scim_integration_state_upgraders.go b/pkg/resources/scim_integration_state_upgraders.go new file mode 100644 index 0000000000..130ae41076 --- /dev/null +++ b/pkg/resources/scim_integration_state_upgraders.go @@ -0,0 +1,13 @@ +package resources + +import "context" + +func v091ScimIntegrationStateUpgrader(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + if rawState == nil { + return rawState, nil + } + + rawState["run_as_role"] = rawState["provisioner_role"] + delete(rawState, "provisioner_role") + return rawState, nil +} diff --git a/pkg/resources/scim_integration_test.go b/pkg/resources/scim_integration_test.go deleted file mode 100644 index 5ef9edfd5a..0000000000 --- a/pkg/resources/scim_integration_test.go +++ /dev/null @@ -1,91 +0,0 @@ -package resources_test - -import ( - "database/sql" - "testing" - - internalprovider "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" - - sqlmock "github.com/DATA-DOG/go-sqlmock" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/resources" - . "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/testhelpers" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/stretchr/testify/require" -) - -func TestSCIMIntegration(t *testing.T) { - r := require.New(t) - err := resources.SCIMIntegration().InternalValidate(provider.Provider().Schema, true) - r.NoError(err) -} - -func TestSCIMIntegrationCreate(t *testing.T) { - r := require.New(t) - - in := map[string]interface{}{ - "name": "test_scim_integration", - "scim_client": "AZURE", - "provisioner_role": "AAD_PROVISIONER", - "network_policy": "AAD_NETWORK_POLICY", - } - d := schema.TestResourceDataRaw(t, resources.SCIMIntegration().Schema, in) - r.NotNil(d) - - WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec( - `^CREATE SECURITY INTEGRATION "test_scim_integration" TYPE=SCIM NETWORK_POLICY='AAD_NETWORK_POLICY' RUN_AS_ROLE='AAD_PROVISIONER' SCIM_CLIENT='AZURE'$`, - ).WillReturnResult(sqlmock.NewResult(1, 1)) - expectReadSCIMIntegration(mock) - - err := resources.CreateSCIMIntegration(d, &internalprovider.Context{ - Client: sdk.NewClientFromDB(db), - }) - r.NoError(err) - }) -} - -func TestSCIMIntegrationRead(t *testing.T) { - r := require.New(t) - - d := scimIntegration(t, "test_scim_integration", map[string]interface{}{"name": "test_scim_integration"}) - - WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - expectReadSCIMIntegration(mock) - - err := resources.ReadSCIMIntegration(d, &internalprovider.Context{ - Client: sdk.NewClientFromDB(db), - }) - r.NoError(err) - }) -} - -func TestSCIMIntegrationDelete(t *testing.T) { - r := require.New(t) - - d := scimIntegration(t, "drop_it", map[string]interface{}{"name": "drop_it"}) - - WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec(`DROP SECURITY INTEGRATION "drop_it"`).WillReturnResult(sqlmock.NewResult(1, 1)) - err := resources.DeleteSCIMIntegration(d, &internalprovider.Context{ - Client: sdk.NewClientFromDB(db), - }) - r.NoError(err) - }) -} - -func expectReadSCIMIntegration(mock sqlmock.Sqlmock) { - showRows := sqlmock.NewRows([]string{ - "name", "type", "category", "created_on", - }, - ).AddRow("test_scim_integration", "SCIM - AZURE", "SECURITY", "now") - mock.ExpectQuery(`^SHOW SECURITY INTEGRATIONS LIKE 'test_scim_integration'$`).WillReturnRows(showRows) - - descRows := sqlmock.NewRows([]string{ - "property", "property_type", "property_value", "property_default", - }).AddRow("NETWORK_POLICY", "String", "AAD_NETWORK_POLICY", nil). - AddRow("RUN_AS_ROLE", "String", "AAD_PROVISIONER", nil) - - mock.ExpectQuery(`DESCRIBE SECURITY INTEGRATION "test_scim_integration"$`).WillReturnRows(descRows) -} diff --git a/pkg/resources/share.go b/pkg/resources/share.go index fe4a3872d0..a1f13c2f4d 100644 --- a/pkg/resources/share.go +++ b/pkg/resources/share.go @@ -36,7 +36,7 @@ var shareSchema = map[string]*schema.Schema{ Optional: true, Description: "A list of accounts to be added to the share. Values should not be the account locator, but " + "in the form of 'organization_name.account_name", - DiffSuppressFunc: diffCaseInsensitive, + DiffSuppressFunc: ignoreCaseSuppressFunc, }, } diff --git a/pkg/resources/testdata/TestAcc_ScimIntegration/basic/test.tf b/pkg/resources/testdata/TestAcc_ScimIntegration/basic/test.tf new file mode 100644 index 0000000000..5a58953f98 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_ScimIntegration/basic/test.tf @@ -0,0 +1,6 @@ +resource "snowflake_scim_integration" "test" { + name = var.name + scim_client = var.scim_client + run_as_role = var.run_as_role + enabled = var.enabled +} diff --git a/pkg/resources/testdata/TestAcc_ScimIntegration/basic/variables.tf b/pkg/resources/testdata/TestAcc_ScimIntegration/basic/variables.tf new file mode 100644 index 0000000000..f92c52349b --- /dev/null +++ b/pkg/resources/testdata/TestAcc_ScimIntegration/basic/variables.tf @@ -0,0 +1,12 @@ +variable "name" { + type = string +} +variable "scim_client" { + type = string +} +variable "run_as_role" { + type = string +} +variable "enabled" { + type = bool +} diff --git a/pkg/resources/testdata/TestAcc_ScimIntegration/complete/test.tf b/pkg/resources/testdata/TestAcc_ScimIntegration/complete/test.tf new file mode 100644 index 0000000000..45be7c7095 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_ScimIntegration/complete/test.tf @@ -0,0 +1,9 @@ +resource "snowflake_scim_integration" "test" { + name = var.name + enabled = var.enabled + scim_client = var.scim_client + sync_password = var.sync_password + network_policy = var.network_policy_name + run_as_role = var.run_as_role + comment = var.comment +} diff --git a/pkg/resources/testdata/TestAcc_ScimIntegration/complete/variables.tf b/pkg/resources/testdata/TestAcc_ScimIntegration/complete/variables.tf new file mode 100644 index 0000000000..b1a9dd64a1 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_ScimIntegration/complete/variables.tf @@ -0,0 +1,21 @@ +variable "name" { + type = string +} +variable "enabled" { + type = bool +} +variable "scim_client" { + type = string +} +variable "sync_password" { + type = bool +} +variable "network_policy_name" { + type = string +} +variable "run_as_role" { + type = string +} +variable "comment" { + type = string +} diff --git a/pkg/resources/testdata/TestAcc_ScimIntegration/invalid/test.tf b/pkg/resources/testdata/TestAcc_ScimIntegration/invalid/test.tf new file mode 100644 index 0000000000..c41dda5584 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_ScimIntegration/invalid/test.tf @@ -0,0 +1,3 @@ +resource "snowflake_scim_integration" "test" { + name = var.name +} diff --git a/pkg/resources/testdata/TestAcc_ScimIntegration/invalid/variables.tf b/pkg/resources/testdata/TestAcc_ScimIntegration/invalid/variables.tf new file mode 100644 index 0000000000..77e5cc9698 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_ScimIntegration/invalid/variables.tf @@ -0,0 +1,3 @@ +variable "name" { + type = string +} diff --git a/pkg/resources/user.go b/pkg/resources/user.go index fd539f0188..d54c146bc5 100644 --- a/pkg/resources/user.go +++ b/pkg/resources/user.go @@ -14,10 +14,6 @@ import ( "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" ) -var diffCaseInsensitive = func(k, old, new string, d *schema.ResourceData) bool { - return strings.EqualFold(old, new) -} - var userSchema = map[string]*schema.Schema{ "name": { Type: schema.TypeString, @@ -32,7 +28,7 @@ var userSchema = map[string]*schema.Schema{ Sensitive: false, Description: "The name users use to log in. If not supplied, snowflake will use name instead.", // login_name is case-insensitive - DiffSuppressFunc: diffCaseInsensitive, + DiffSuppressFunc: ignoreCaseSuppressFunc, }, "comment": { Type: schema.TypeString, @@ -59,14 +55,14 @@ var userSchema = map[string]*schema.Schema{ "default_namespace": { Type: schema.TypeString, Optional: true, - DiffSuppressFunc: diffCaseInsensitive, + DiffSuppressFunc: ignoreCaseSuppressFunc, Description: "Specifies the namespace (database only or database and schema) that is active by default for the user’s session upon login.", }, "default_role": { Type: schema.TypeString, Optional: true, Computed: true, - DiffSuppressFunc: diffCaseInsensitive, + DiffSuppressFunc: ignoreCaseSuppressFunc, Description: "Specifies the role that is active by default for the user’s session upon login.", }, "default_secondary_roles": { diff --git a/pkg/sdk/common_types.go b/pkg/sdk/common_types.go index e95d15671d..2f069da78e 100644 --- a/pkg/sdk/common_types.go +++ b/pkg/sdk/common_types.go @@ -257,3 +257,8 @@ var AllTraceLevels = []TraceLevel{ TraceLevelOnEvent, TraceLevelOff, } + +// StringAllowEmpty is a wrapper on string to allow using empty strings in SQL. +type StringAllowEmpty struct { + Value string `ddl:"keyword,single_quotes"` +} diff --git a/pkg/sdk/security_integrations_def.go b/pkg/sdk/security_integrations_def.go index e95b579652..8c87adb64c 100644 --- a/pkg/sdk/security_integrations_def.go +++ b/pkg/sdk/security_integrations_def.go @@ -1,9 +1,16 @@ package sdk -import g "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/poc/generator" +import ( + "fmt" + "strings" + + g "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/poc/generator" +) //go:generate go run ./poc/main.go +const SecurityIntegrationCategory = "SECURITY" + type ApiAuthenticationSecurityIntegrationOauthClientAuthMethodOption string const ( @@ -64,6 +71,26 @@ const ( ScimSecurityIntegrationScimClientGeneric ScimSecurityIntegrationScimClientOption = "GENERIC" ) +var AllScimSecurityIntegrationScimClients = []ScimSecurityIntegrationScimClientOption{ + ScimSecurityIntegrationScimClientOkta, + ScimSecurityIntegrationScimClientAzure, + ScimSecurityIntegrationScimClientGeneric, +} + +func ToScimSecurityIntegrationScimClientOption(s string) (ScimSecurityIntegrationScimClientOption, error) { + s = strings.ToUpper(s) + switch s { + case "OKTA": + return ScimSecurityIntegrationScimClientOkta, nil + case "AZURE": + return ScimSecurityIntegrationScimClientAzure, nil + case "GENERIC": + return ScimSecurityIntegrationScimClientGeneric, nil + default: + return "", fmt.Errorf("invalid ScimSecurityIntegrationScimClientOption: %s", s) + } +} + type ScimSecurityIntegrationRunAsRoleOption string const ( @@ -72,6 +99,26 @@ const ( ScimSecurityIntegrationRunAsRoleGenericScimProvisioner ScimSecurityIntegrationRunAsRoleOption = "GENERIC_SCIM_PROVISIONER" ) +var AllScimSecurityIntegrationRunAsRoles = []ScimSecurityIntegrationRunAsRoleOption{ + ScimSecurityIntegrationRunAsRoleOktaProvisioner, + ScimSecurityIntegrationRunAsRoleAadProvisioner, + ScimSecurityIntegrationRunAsRoleGenericScimProvisioner, +} + +func ToScimSecurityIntegrationRunAsRoleOption(s string) (ScimSecurityIntegrationRunAsRoleOption, error) { + s = strings.ToUpper(s) + switch s { + case "OKTA_PROVISIONER": + return ScimSecurityIntegrationRunAsRoleOktaProvisioner, nil + case "AAD_PROVISIONER": + return ScimSecurityIntegrationRunAsRoleAadProvisioner, nil + case "GENERIC_SCIM_PROVISIONER": + return ScimSecurityIntegrationRunAsRoleGenericScimProvisioner, nil + default: + return "", fmt.Errorf("invalid ScimSecurityIntegrationRunAsRoleOption: %s", s) + } +} + var ( allowedScopeDef = g.NewQueryStruct("AllowedScope").Text("Scope", g.KeywordOptions().SingleQuotes().Required()) userDomainDef = g.NewQueryStruct("UserDomain").Text("Domain", g.KeywordOptions().SingleQuotes().Required()) @@ -307,15 +354,15 @@ var scimIntegrationSetDef = g.NewQueryStruct("ScimIntegrationSet"). OptionalBooleanAssignment("ENABLED", g.ParameterOptions()). OptionalIdentifier("NetworkPolicy", g.KindOfT[AccountObjectIdentifier](), g.IdentifierOptions().Equals().SQL("NETWORK_POLICY")). OptionalBooleanAssignment("SYNC_PASSWORD", g.ParameterOptions()). - OptionalComment(). + // TODO(SNOW-1461780): use COMMENT in unset and here use OptionalComment + OptionalAssignment("COMMENT", "StringAllowEmpty", g.ParameterOptions()). WithValidation(g.AtLeastOneValueSet, "Enabled", "NetworkPolicy", "SyncPassword", "Comment") var scimIntegrationUnsetDef = g.NewQueryStruct("ScimIntegrationUnset"). OptionalSQL("ENABLED"). OptionalSQL("NETWORK_POLICY"). OptionalSQL("SYNC_PASSWORD"). - OptionalSQL("COMMENT"). - WithValidation(g.AtLeastOneValueSet, "Enabled", "NetworkPolicy", "SyncPassword", "Comment") + WithValidation(g.AtLeastOneValueSet, "Enabled", "NetworkPolicy", "SyncPassword") var SecurityIntegrationsDef = g.NewInterface( "SecurityIntegrations", @@ -520,7 +567,7 @@ var SecurityIntegrationsDef = g.NewInterface( createSecurityIntegrationOperation("CreateScim", func(qs *g.QueryStruct) *g.QueryStruct { return qs. PredefinedQueryStructField("integrationType", "string", g.StaticOptions().SQL("TYPE = SCIM")). - BooleanAssignment("ENABLED", g.ParameterOptions().Required()). + OptionalBooleanAssignment("ENABLED", g.ParameterOptions()). Assignment( "SCIM_CLIENT", g.KindOfT[ScimSecurityIntegrationScimClientOption](), diff --git a/pkg/sdk/security_integrations_dto_builders_gen.go b/pkg/sdk/security_integrations_dto_builders_gen.go index 297d6798b0..2f27153cbb 100644 --- a/pkg/sdk/security_integrations_dto_builders_gen.go +++ b/pkg/sdk/security_integrations_dto_builders_gen.go @@ -521,13 +521,11 @@ func (s *CreateSaml2SecurityIntegrationRequest) WithComment(Comment string) *Cre func NewCreateScimSecurityIntegrationRequest( name AccountObjectIdentifier, - Enabled bool, ScimClient ScimSecurityIntegrationScimClientOption, RunAsRole ScimSecurityIntegrationRunAsRoleOption, ) *CreateScimSecurityIntegrationRequest { s := CreateScimSecurityIntegrationRequest{} s.name = name - s.Enabled = Enabled s.ScimClient = ScimClient s.RunAsRole = RunAsRole return &s @@ -543,6 +541,11 @@ func (s *CreateScimSecurityIntegrationRequest) WithIfNotExists(IfNotExists bool) return s } +func (s *CreateScimSecurityIntegrationRequest) WithEnabled(Enabled bool) *CreateScimSecurityIntegrationRequest { + s.Enabled = &Enabled + return s +} + func (s *CreateScimSecurityIntegrationRequest) WithNetworkPolicy(NetworkPolicy AccountObjectIdentifier) *CreateScimSecurityIntegrationRequest { s.NetworkPolicy = &NetworkPolicy return s @@ -1402,7 +1405,7 @@ func (s *ScimIntegrationSetRequest) WithSyncPassword(SyncPassword bool) *ScimInt return s } -func (s *ScimIntegrationSetRequest) WithComment(Comment string) *ScimIntegrationSetRequest { +func (s *ScimIntegrationSetRequest) WithComment(Comment StringAllowEmpty) *ScimIntegrationSetRequest { s.Comment = &Comment return s } @@ -1426,11 +1429,6 @@ func (s *ScimIntegrationUnsetRequest) WithSyncPassword(SyncPassword bool) *ScimI return s } -func (s *ScimIntegrationUnsetRequest) WithComment(Comment bool) *ScimIntegrationUnsetRequest { - s.Comment = &Comment - return s -} - func NewDropSecurityIntegrationRequest( name AccountObjectIdentifier, ) *DropSecurityIntegrationRequest { diff --git a/pkg/sdk/security_integrations_dto_gen.go b/pkg/sdk/security_integrations_dto_gen.go index a12864f467..d788094ce5 100644 --- a/pkg/sdk/security_integrations_dto_gen.go +++ b/pkg/sdk/security_integrations_dto_gen.go @@ -182,8 +182,8 @@ func (r *CreateSaml2SecurityIntegrationRequest) GetName() AccountObjectIdentifie type CreateScimSecurityIntegrationRequest struct { OrReplace *bool IfNotExists *bool - name AccountObjectIdentifier // required - Enabled bool // required + name AccountObjectIdentifier // required + Enabled *bool ScimClient ScimSecurityIntegrationScimClientOption // required RunAsRole ScimSecurityIntegrationRunAsRoleOption // required NetworkPolicy *AccountObjectIdentifier @@ -414,14 +414,13 @@ type ScimIntegrationSetRequest struct { Enabled *bool NetworkPolicy *AccountObjectIdentifier SyncPassword *bool - Comment *string + Comment *StringAllowEmpty } type ScimIntegrationUnsetRequest struct { Enabled *bool NetworkPolicy *bool SyncPassword *bool - Comment *bool } type DropSecurityIntegrationRequest struct { diff --git a/pkg/sdk/security_integrations_gen.go b/pkg/sdk/security_integrations_gen.go index 332d4ae1c1..3d64607a73 100644 --- a/pkg/sdk/security_integrations_gen.go +++ b/pkg/sdk/security_integrations_gen.go @@ -3,6 +3,8 @@ package sdk import ( "context" "database/sql" + "fmt" + "strings" "time" ) @@ -236,7 +238,7 @@ type CreateScimSecurityIntegrationOptions struct { IfNotExists *bool `ddl:"keyword" sql:"IF NOT EXISTS"` name AccountObjectIdentifier `ddl:"identifier"` integrationType string `ddl:"static" sql:"TYPE = SCIM"` - Enabled bool `ddl:"parameter" sql:"ENABLED"` + Enabled *bool `ddl:"parameter" sql:"ENABLED"` ScimClient ScimSecurityIntegrationScimClientOption `ddl:"parameter,single_quotes" sql:"SCIM_CLIENT"` RunAsRole ScimSecurityIntegrationRunAsRoleOption `ddl:"parameter,single_quotes" sql:"RUN_AS_ROLE"` NetworkPolicy *AccountObjectIdentifier `ddl:"identifier,equals" sql:"NETWORK_POLICY"` @@ -487,14 +489,13 @@ type ScimIntegrationSet struct { Enabled *bool `ddl:"parameter" sql:"ENABLED"` NetworkPolicy *AccountObjectIdentifier `ddl:"identifier,equals" sql:"NETWORK_POLICY"` SyncPassword *bool `ddl:"parameter" sql:"SYNC_PASSWORD"` - Comment *string `ddl:"parameter,single_quotes" sql:"COMMENT"` + Comment *StringAllowEmpty `ddl:"parameter" sql:"COMMENT"` } type ScimIntegrationUnset struct { Enabled *bool `ddl:"keyword" sql:"ENABLED"` NetworkPolicy *bool `ddl:"keyword" sql:"NETWORK_POLICY"` SyncPassword *bool `ddl:"keyword" sql:"SYNC_PASSWORD"` - Comment *bool `ddl:"keyword" sql:"COMMENT"` } // DropSecurityIntegrationOptions is based on https://docs.snowflake.com/en/sql-reference/sql/drop-integration. @@ -526,6 +527,14 @@ type SecurityIntegrationProperty struct { Default string } +func (s SecurityIntegrationProperty) GetName() string { + return s.Name +} + +func (s SecurityIntegrationProperty) GetDefault() string { + return s.Default +} + // ShowSecurityIntegrationOptions is based on https://docs.snowflake.com/en/sql-reference/sql/show-integrations. type ShowSecurityIntegrationOptions struct { show bool `ddl:"static" sql:"SHOW"` @@ -550,3 +559,11 @@ type SecurityIntegration struct { Comment string CreatedOn time.Time } + +func (s *SecurityIntegration) SubType() (string, error) { + typeParts := strings.Split(s.IntegrationType, "-") + if len(typeParts) < 2 { + return "", fmt.Errorf("expected \" - \", got: %s", s.IntegrationType) + } + return strings.TrimSpace(typeParts[1]), nil +} diff --git a/pkg/sdk/security_integrations_gen_test.go b/pkg/sdk/security_integrations_gen_test.go index 5a82b9e6b5..940c9956fe 100644 --- a/pkg/sdk/security_integrations_gen_test.go +++ b/pkg/sdk/security_integrations_gen_test.go @@ -1,7 +1,10 @@ package sdk import ( + "errors" "testing" + + "github.com/stretchr/testify/require" ) func TestSecurityIntegrations_CreateApiAuthenticationWithClientCredentialsFlow(t *testing.T) { @@ -395,7 +398,6 @@ func TestSecurityIntegrations_CreateScim(t *testing.T) { defaultOpts := func() *CreateScimSecurityIntegrationOptions { return &CreateScimSecurityIntegrationOptions{ name: id, - Enabled: true, ScimClient: "GENERIC", RunAsRole: "GENERIC_SCIM_PROVISIONER", } @@ -416,12 +418,13 @@ func TestSecurityIntegrations_CreateScim(t *testing.T) { t.Run("basic", func(t *testing.T) { opts := defaultOpts() opts.OrReplace = Pointer(true) - assertOptsValidAndSQLEquals(t, opts, "CREATE OR REPLACE SECURITY INTEGRATION %s TYPE = SCIM ENABLED = true SCIM_CLIENT = 'GENERIC' RUN_AS_ROLE = 'GENERIC_SCIM_PROVISIONER'", id.FullyQualifiedName()) + assertOptsValidAndSQLEquals(t, opts, "CREATE OR REPLACE SECURITY INTEGRATION %s TYPE = SCIM SCIM_CLIENT = 'GENERIC' RUN_AS_ROLE = 'GENERIC_SCIM_PROVISIONER'", id.FullyQualifiedName()) }) t.Run("all options", func(t *testing.T) { opts := defaultOpts() networkPolicyID := randomAccountObjectIdentifier() + opts.Enabled = Pointer(true) opts.IfNotExists = Pointer(true) opts.NetworkPolicy = Pointer(networkPolicyID) opts.SyncPassword = Pointer(true) @@ -1282,7 +1285,7 @@ func TestSecurityIntegrations_AlterScim(t *testing.T) { t.Run("validation: at least one of the fields [opts.Unset.*] should be set", func(t *testing.T) { opts := defaultOpts() opts.Unset = &ScimIntegrationUnset{} - assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterScimSecurityIntegrationOptions.Unset", "Enabled", "NetworkPolicy", "SyncPassword", "Comment")) + assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterScimSecurityIntegrationOptions.Unset", "Enabled", "NetworkPolicy", "SyncPassword")) }) t.Run("all options - set", func(t *testing.T) { @@ -1292,21 +1295,28 @@ func TestSecurityIntegrations_AlterScim(t *testing.T) { Enabled: Pointer(true), NetworkPolicy: Pointer(networkPolicyID), SyncPassword: Pointer(true), - Comment: Pointer("test"), + Comment: Pointer(StringAllowEmpty{Value: "test"}), } assertOptsValidAndSQLEquals(t, opts, "ALTER SECURITY INTEGRATION %s SET ENABLED = true, NETWORK_POLICY = %s, SYNC_PASSWORD = true, COMMENT = 'test'", id.FullyQualifiedName(), networkPolicyID.FullyQualifiedName()) }) + t.Run("set empty comment", func(t *testing.T) { + opts := defaultOpts() + opts.Set = &ScimIntegrationSet{ + Comment: Pointer(StringAllowEmpty{Value: ""}), + } + assertOptsValidAndSQLEquals(t, opts, "ALTER SECURITY INTEGRATION %s SET COMMENT = ''", id.FullyQualifiedName()) + }) + t.Run("all options - unset", func(t *testing.T) { opts := defaultOpts() opts.Unset = &ScimIntegrationUnset{ Enabled: Pointer(true), NetworkPolicy: Pointer(true), SyncPassword: Pointer(true), - Comment: Pointer(true), } - assertOptsValidAndSQLEquals(t, opts, "ALTER SECURITY INTEGRATION %s UNSET ENABLED, NETWORK_POLICY, SYNC_PASSWORD, COMMENT", id.FullyQualifiedName()) + assertOptsValidAndSQLEquals(t, opts, "ALTER SECURITY INTEGRATION %s UNSET ENABLED, NETWORK_POLICY, SYNC_PASSWORD", id.FullyQualifiedName()) }) t.Run("set tags", func(t *testing.T) { @@ -1413,3 +1423,36 @@ func TestSecurityIntegrations_Show(t *testing.T) { assertOptsValidAndSQLEquals(t, opts, "SHOW SECURITY INTEGRATIONS LIKE 'some pattern'") }) } + +func TestSecurityIntegration_SubType(t *testing.T) { + testCases := map[string]struct { + integration SecurityIntegration + subType string + err error + }{ + "subtype for scim integration": { + integration: SecurityIntegration{IntegrationType: "SCIM - AZURE"}, + subType: "AZURE", + }, + "invalid integration type": { + integration: SecurityIntegration{IntegrationType: "invalid"}, + err: errors.New("expected \" - \", got: invalid"), + }, + "empty integration type": { + integration: SecurityIntegration{IntegrationType: ""}, + err: errors.New("expected \" - \", got: "), + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + subType, err := tc.integration.SubType() + if err != nil { + require.Equal(t, tc.err, err) + } else { + require.NoError(t, tc.err) + require.Equal(t, tc.subType, subType) + } + }) + } +} diff --git a/pkg/sdk/security_integrations_impl_gen.go b/pkg/sdk/security_integrations_impl_gen.go index 07b14fdf25..c2244cb30d 100644 --- a/pkg/sdk/security_integrations_impl_gen.go +++ b/pkg/sdk/security_integrations_impl_gen.go @@ -595,7 +595,6 @@ func (r *AlterScimSecurityIntegrationRequest) toOpts() *AlterScimSecurityIntegra Enabled: r.Unset.Enabled, NetworkPolicy: r.Unset.NetworkPolicy, SyncPassword: r.Unset.SyncPassword, - Comment: r.Unset.Comment, } } return opts diff --git a/pkg/sdk/security_integrations_validations_gen.go b/pkg/sdk/security_integrations_validations_gen.go index 5d79fef708..2e6129b157 100644 --- a/pkg/sdk/security_integrations_validations_gen.go +++ b/pkg/sdk/security_integrations_validations_gen.go @@ -340,8 +340,8 @@ func (opts *AlterScimSecurityIntegrationOptions) validate() error { } } if valueSet(opts.Unset) { - if !anyValueSet(opts.Unset.Enabled, opts.Unset.NetworkPolicy, opts.Unset.SyncPassword, opts.Unset.Comment) { - errs = append(errs, errAtLeastOneOf("AlterScimSecurityIntegrationOptions.Unset", "Enabled", "NetworkPolicy", "SyncPassword", "Comment")) + if !anyValueSet(opts.Unset.Enabled, opts.Unset.NetworkPolicy, opts.Unset.SyncPassword) { + errs = append(errs, errAtLeastOneOf("AlterScimSecurityIntegrationOptions.Unset", "Enabled", "NetworkPolicy", "SyncPassword")) } } return JoinErrors(errs...) diff --git a/pkg/sdk/testint/security_integrations_gen_integration_test.go b/pkg/sdk/testint/security_integrations_gen_integration_test.go index 0fe7a7910d..993d6500a4 100644 --- a/pkg/sdk/testint/security_integrations_gen_integration_test.go +++ b/pkg/sdk/testint/security_integrations_gen_integration_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/internal/collections" "github.com/stretchr/testify/assert" @@ -145,12 +144,9 @@ func TestInt_SecurityIntegrations(t *testing.T) { createSCIMIntegration := func(t *testing.T, with func(*sdk.CreateScimSecurityIntegrationRequest)) (*sdk.SecurityIntegration, sdk.AccountObjectIdentifier) { t.Helper() - role, roleCleanup := testClientHelper().Role.CreateRoleWithRequest(t, sdk.NewCreateRoleRequest(snowflakeroles.GenericScimProvisioner).WithOrReplace(true)) - t.Cleanup(roleCleanup) - testClientHelper().Role.GrantRoleToCurrentRole(t, role.ID()) id := testClientHelper().Ids.RandomAccountObjectIdentifier() - scimReq := sdk.NewCreateScimSecurityIntegrationRequest(id, false, sdk.ScimSecurityIntegrationScimClientGeneric, sdk.ScimSecurityIntegrationRunAsRoleGenericScimProvisioner) + scimReq := sdk.NewCreateScimSecurityIntegrationRequest(id, sdk.ScimSecurityIntegrationScimClientGeneric, sdk.ScimSecurityIntegrationRunAsRoleGenericScimProvisioner) if with != nil { with(scimReq) } @@ -590,11 +586,11 @@ func TestInt_SecurityIntegrations(t *testing.T) { details, err := client.SecurityIntegrations.Describe(ctx, id) require.NoError(t, err) - assertSCIMDescribe(details, "false", networkPolicy.Name, "GENERIC_SCIM_PROVISIONER", "false", "a") + assertSCIMDescribe(details, "true", networkPolicy.Name, "GENERIC_SCIM_PROVISIONER", "false", "a") si, err := client.SecurityIntegrations.ShowByID(ctx, id) require.NoError(t, err) - assertSecurityIntegration(t, si, id, "SCIM - GENERIC", false, "a") + assertSecurityIntegration(t, si, id, "SCIM - GENERIC", true, "a") }) t.Run("AlterApiAuthenticationWithClientCredentialsFlow", func(t *testing.T) { @@ -1264,9 +1260,9 @@ func TestInt_SecurityIntegrations(t *testing.T) { WithSet( *sdk.NewScimIntegrationSetRequest(). WithNetworkPolicy(sdk.NewAccountObjectIdentifier(networkPolicy.Name)). - WithEnabled(true). + WithEnabled(false). WithSyncPassword(false). - WithComment("altered"), + WithComment(sdk.StringAllowEmpty{Value: "altered"}), ) err := client.SecurityIntegrations.AlterScim(ctx, setRequest) require.NoError(t, err) @@ -1274,21 +1270,32 @@ func TestInt_SecurityIntegrations(t *testing.T) { details, err := client.SecurityIntegrations.Describe(ctx, id) require.NoError(t, err) - assertSCIMDescribe(details, "true", networkPolicy.Name, "GENERIC_SCIM_PROVISIONER", "false", "altered") + assertSCIMDescribe(details, "false", networkPolicy.Name, "GENERIC_SCIM_PROVISIONER", "false", "altered") unsetRequest := sdk.NewAlterScimSecurityIntegrationRequest(id). WithUnset( *sdk.NewScimIntegrationUnsetRequest(). + WithEnabled(true). WithNetworkPolicy(true). WithSyncPassword(true), ) err = client.SecurityIntegrations.AlterScim(ctx, unsetRequest) require.NoError(t, err) + // check setting empty comment because of lacking UNSET COMMENT + // TODO(SNOW-1461780): change this to UNSET + setRequest = sdk.NewAlterScimSecurityIntegrationRequest(id). + WithSet( + *sdk.NewScimIntegrationSetRequest(). + WithComment(sdk.StringAllowEmpty{Value: ""}), + ) + err = client.SecurityIntegrations.AlterScim(ctx, setRequest) + require.NoError(t, err) + details, err = client.SecurityIntegrations.Describe(ctx, id) require.NoError(t, err) - assertSCIMDescribe(details, "true", "", "GENERIC_SCIM_PROVISIONER", "true", "altered") + assertSCIMDescribe(details, "false", "", "GENERIC_SCIM_PROVISIONER", "true", "") }) t.Run("AlterSCIMIntegration - set and unset tags", func(t *testing.T) { @@ -1354,7 +1361,7 @@ func TestInt_SecurityIntegrations(t *testing.T) { details, err := client.SecurityIntegrations.Describe(ctx, id) require.NoError(t, err) - assertSCIMDescribe(details, "false", "", "GENERIC_SCIM_PROVISIONER", "true", "") + assertSCIMDescribe(details, "true", "", "GENERIC_SCIM_PROVISIONER", "true", "") }) t.Run("ShowByID", func(t *testing.T) { @@ -1362,7 +1369,7 @@ func TestInt_SecurityIntegrations(t *testing.T) { si, err := client.SecurityIntegrations.ShowByID(ctx, id) require.NoError(t, err) - assertSecurityIntegration(t, si, id, "SCIM - GENERIC", false, "") + assertSecurityIntegration(t, si, id, "SCIM - GENERIC", true, "") }) t.Run("Show ExternalOauth", func(t *testing.T) { diff --git a/pkg/sdk/testint/setup_test.go b/pkg/sdk/testint/setup_test.go index e913e60781..3e7425aa51 100644 --- a/pkg/sdk/testint/setup_test.go +++ b/pkg/sdk/testint/setup_test.go @@ -192,6 +192,15 @@ func (itc *integrationTestContext) initialize() error { return err } + err = helpers.EnsureScimProvisionerRolesExist(itc.client, itc.ctx) + if err != nil { + return err + } + err = helpers.EnsureScimProvisionerRolesExist(itc.secondaryClient, itc.secondaryCtx) + if err != nil { + return err + } + return nil } diff --git a/v1-preparations/CHANGES_BEFORE_V1.md b/v1-preparations/CHANGES_BEFORE_V1.md new file mode 100644 index 0000000000..1bfab19df5 --- /dev/null +++ b/v1-preparations/CHANGES_BEFORE_V1.md @@ -0,0 +1,17 @@ +# Changes before v1 + +This document is a changelog of resources and datasources as part of the https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#preparing-essential-ga-objects-for-the-provider-v1. Each provider version lists changes made in resources and datasources definitions during v1 preparations, like added, modified and removed fields. + +## v0.91.0 ➞ v0.92.0 +### snowflake_scim_integration resource changes + +New fields: +- `enabled` +- `sync_password` +- `comment` + +Changed fields: +- `provisioner_role` renamed to `run_as_role` + +Other changes: +- `scim_client` and `run_as_role` marked as `ForceNew`