feat: SCIM integration v1 readiness (#2846)

- Added missing fields - Renamed fields to match docs - Migrated to Context functions - Added acceptance tests - Fixed handling empty comments in SDK - changed UNSET to SET with empty value ## Test Plan  * [x] acceptance tests ## References  https://docs.snowflake.com/en/sql-reference/sql/create-security-integration-scim
Snowflake-Labs · Jun 12, 2024 · 269df6b · 269df6b
1 parent 5df6d3d
commit 269df6b
Show file tree

Hide file tree

Showing 35 changed files with 900 additions and 316 deletions.
diff --git 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 `<org_name>.<account_name>`. 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 `<org_name>.<account_name>` has to be changed
 to account locator format `<account_locator>` (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 `<org_name>.<account_name>` format as it's recommended by Snowflake.

diff --git 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
@@ -1,6 +1,17 @@
-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"
+}
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {