From 8b9f54d71ca869281f09ed7976e126a44256ea7b Mon Sep 17 00:00:00 2001 From: Wodans Son <20408400+WodansSon@users.noreply.github.com> Date: Thu, 4 Apr 2024 20:43:01 -0600 Subject: [PATCH] Including #25508 * Initial Check-in... * Slight tweak to behavior... * Fix lint errors... * Update documentation, comments and test case... * Update documentation and fix test case... * Update internal/services/mssql/mssql_database_resource.go --------- Co-authored-by: kt --- internal/services/mssql/helper/database.go | 4 +- .../services/mssql/mssql_database_resource.go | 64 ++++++++++++------- .../mssql/mssql_database_resource_test.go | 18 ++++-- .../mssql/mssql_elasticpool_resource.go | 46 ++++++++----- .../mssql/mssql_elasticpool_resource_test.go | 13 +++- website/docs/r/mssql_database.html.markdown | 10 +-- .../docs/r/mssql_elasticpool.html.markdown | 8 ++- 7 files changed, 109 insertions(+), 54 deletions(-) diff --git a/internal/services/mssql/helper/database.go b/internal/services/mssql/helper/database.go index a8d02585097b..9bbba8c6822b 100644 --- a/internal/services/mssql/helper/database.go +++ b/internal/services/mssql/helper/database.go @@ -108,11 +108,11 @@ func FindDatabaseReplicationPartners(ctx context.Context, databasesClient *datab } if partnerDatabase.Id != nil && partnerDatabase.Properties != nil && partnerDatabase.Properties.PreferredEnclaveType != nil { - if primaryEnclaveType == *partnerDatabase.Properties.PreferredEnclaveType { + if primaryEnclaveType != "" && primaryEnclaveType == *partnerDatabase.Properties.PreferredEnclaveType { log.Printf("[INFO] Found Partner %s", partnerDatabaseId) partnerDatabases = append(partnerDatabases, *partnerDatabase) } else { - log.Printf("[INFO] Mismatch of possible Partner Database based on enclave type (%s vs %s) for %s", string(primaryEnclaveType), string(*partnerDatabase.Properties.PreferredEnclaveType), id) + log.Printf("[INFO] Mismatch of possible Partner Database based on enclave type (%q vs %q) for %s", primaryEnclaveType, string(*partnerDatabase.Properties.PreferredEnclaveType), id) } } } diff --git a/internal/services/mssql/mssql_database_resource.go b/internal/services/mssql/mssql_database_resource.go index 565b51bf1445..bf746a51b7c4 100644 --- a/internal/services/mssql/mssql_database_resource.go +++ b/internal/services/mssql/mssql_database_resource.go @@ -21,6 +21,7 @@ import ( "github.com/hashicorp/go-azure-sdk/resource-manager/sql/2023-02-01-preview/backupshorttermretentionpolicies" "github.com/hashicorp/go-azure-sdk/resource-manager/sql/2023-02-01-preview/databases" "github.com/hashicorp/go-azure-sdk/resource-manager/sql/2023-02-01-preview/databasesecurityalertpolicies" + "github.com/hashicorp/go-azure-sdk/resource-manager/sql/2023-02-01-preview/elasticpools" "github.com/hashicorp/go-azure-sdk/resource-manager/sql/2023-02-01-preview/geobackuppolicies" "github.com/hashicorp/go-azure-sdk/resource-manager/sql/2023-02-01-preview/longtermretentionpolicies" "github.com/hashicorp/go-azure-sdk/resource-manager/sql/2023-02-01-preview/servers" @@ -69,9 +70,20 @@ func resourceMsSqlDatabase() *pluginsdk.Resource { CustomizeDiff: pluginsdk.CustomDiffWithAll( pluginsdk.ForceNewIfChange("sku_name", func(ctx context.Context, old, new, _ interface{}) bool { - // "hyperscale can not change to other sku + // hyperscale can not be changed to another sku return strings.HasPrefix(old.(string), "HS") && !strings.HasPrefix(new.(string), "HS") }), + pluginsdk.ForceNewIfChange("enclave_type", func(ctx context.Context, old, new, _ interface{}) bool { + // enclave_type cannot be removed once it has been set + // but can be changed between VBS and Default... + // this Diff will not work until 4.0 when we remove + // the computed property from the field scheam. + if old.(string) != "" && new.(string) == "" { + return true + } + + return false + }), func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { transparentDataEncryption := d.Get("transparent_data_encryption_enabled").(bool) skuName := d.Get("sku_name").(string) @@ -102,10 +114,12 @@ func resourceMsSqlDatabaseImporter(ctx context.Context, d *pluginsdk.ResourceDat return nil, err } - enclaveType := databases.AlwaysEncryptedEnclaveTypeDefault - if v := d.Get("enclave_type").(string); v != "" { - enclaveType = databases.AlwaysEncryptedEnclaveTypeVBS + // NOTE: The service default is actually nil/empty which indicates enclave is disabled. the value `Default` is NOT the default. + var enclaveType databases.AlwaysEncryptedEnclaveType + if v, ok := d.GetOk("enclave_type"); ok && v.(string) != "" { + enclaveType = databases.AlwaysEncryptedEnclaveType(v.(string)) } + d.Set("enclave_type", enclaveType) partnerDatabases, err := helper.FindDatabaseReplicationPartners(ctx, client, legacyreplicationLinksClient, resourcesClient, *id, enclaveType, []sql.ReplicationRole{sql.ReplicationRolePrimary}) if err != nil { @@ -200,11 +214,10 @@ func resourceMsSqlDatabaseCreate(d *pluginsdk.ResourceData, meta interface{}) er locks.ByID(id.ID()) defer locks.UnlockByID(id.ID()) - // NOTE: Set the default value, if the field exists in the config the only value - // that it could be is 'VBS'... - enclaveType := databases.AlwaysEncryptedEnclaveTypeDefault - if _, ok := d.GetOk("enclave_type"); ok { - enclaveType = databases.AlwaysEncryptedEnclaveTypeVBS + // NOTE: The service default is actually nil/empty which indicates enclave is disabled. the value `Default` is NOT the default. + var enclaveType databases.AlwaysEncryptedEnclaveType + if v, ok := d.GetOk("enclave_type"); ok && v.(string) != "" { + enclaveType = databases.AlwaysEncryptedEnclaveType(v.(string)) } skuName := d.Get("sku_name").(string) @@ -247,7 +260,7 @@ func resourceMsSqlDatabaseCreate(d *pluginsdk.ResourceData, meta interface{}) er } // NOTE: If the database is being added to an elastic pool, we need to GET the elastic pool and check - // if the 'enclave_type' match. If they don't we need to raise an error stating that they must match. + // if the 'enclave_type' matches. If they don't we need to raise an error stating that they must match. elasticPoolId := d.Get("elastic_pool_id").(string) if elasticPoolId != "" { elasticId, err := commonids.ParseSqlElasticPoolID(elasticPoolId) @@ -291,7 +304,7 @@ func resourceMsSqlDatabaseCreate(d *pluginsdk.ResourceData, meta interface{}) er } // NOTE: The 'PreferredEnclaveType' field cannot be passed to the APIs Create if the 'sku_name' is a DW or DC-series SKU... - if !strings.HasPrefix(strings.ToLower(skuName), "dw") && !strings.Contains(strings.ToLower(skuName), "_dc_") { + if !strings.HasPrefix(strings.ToLower(skuName), "dw") && !strings.Contains(strings.ToLower(skuName), "_dc_") && enclaveType != "" { input.Properties.PreferredEnclaveType = pointer.To(enclaveType) } @@ -687,15 +700,17 @@ func resourceMsSqlDatabaseUpdate(d *pluginsdk.ResourceData, meta interface{}) er } if d.HasChange("enclave_type") { - enclaveType := databases.AlwaysEncryptedEnclaveTypeDefault - if v := d.Get("enclave_type").(string); v != "" { - enclaveType = databases.AlwaysEncryptedEnclaveTypeVBS + var enclaveType databases.AlwaysEncryptedEnclaveType + if v, ok := d.GetOk("enclave_type"); ok && v.(string) != "" { + enclaveType = databases.AlwaysEncryptedEnclaveType(v.(string)) } // The 'PreferredEnclaveType' field cannot be passed to the APIs Update if the // 'sku_name' is a DW or DC-series SKU... - if !strings.HasPrefix(strings.ToLower(skuName), "dw") && !strings.Contains(strings.ToLower(skuName), "_dc_") { + if !strings.HasPrefix(strings.ToLower(skuName), "dw") && !strings.Contains(strings.ToLower(skuName), "_dc_") && enclaveType != "" { props.PreferredEnclaveType = pointer.To(enclaveType) + } else { + props.PreferredEnclaveType = nil } // If the database belongs to an elastic pool, we need to GET the elastic pool and check @@ -712,12 +727,14 @@ func resourceMsSqlDatabaseUpdate(d *pluginsdk.ResourceData, meta interface{}) er return fmt.Errorf("retrieving %s: %s", elasticId, err) } + var elasticEnclaveType elasticpools.AlwaysEncryptedEnclaveType if elasticPool.Model != nil && elasticPool.Model.Properties != nil && elasticPool.Model.Properties.PreferredEnclaveType != nil { - elasticEnclaveType := string(pointer.From(elasticPool.Model.Properties.PreferredEnclaveType)) - databaseEnclaveType := string(enclaveType) + elasticEnclaveType = pointer.From(elasticPool.Model.Properties.PreferredEnclaveType) + } - if !strings.EqualFold(elasticEnclaveType, databaseEnclaveType) { - return fmt.Errorf("updating the %s with enclave type %q to the %s with enclave type %q is not supported. Before updating a database that belongs to an elastic pool please ensure that the 'enclave_type' is the same for both the database and the elastic pool", id, databaseEnclaveType, elasticId, elasticEnclaveType) + if elasticEnclaveType != "" || enclaveType != "" { + if !strings.EqualFold(string(elasticEnclaveType), string(enclaveType)) { + return fmt.Errorf("updating the %s with enclave type %q to the %s with enclave type %q is not supported. Before updating a database that belongs to an elastic pool please ensure that the 'enclave_type' is the same for both the database and the elastic pool", id, enclaveType, elasticId, elasticEnclaveType) } } } @@ -776,7 +793,7 @@ func resourceMsSqlDatabaseUpdate(d *pluginsdk.ResourceData, meta interface{}) er // Place a lock for the current database so any partner resources can't bump its SKU out of band if skuName != "" { - existingEnclaveType := databases.AlwaysEncryptedEnclaveTypeDefault + var existingEnclaveType databases.AlwaysEncryptedEnclaveType if model := existing.Model; model != nil && model.Properties != nil && model.Properties.PreferredEnclaveType != nil { existingEnclaveType = *model.Properties.PreferredEnclaveType } @@ -1128,8 +1145,9 @@ func resourceMsSqlDatabaseRead(d *pluginsdk.ResourceData, meta interface{}) erro ledgerEnabled = *props.IsLedgerOn } - // NOTE: Always set the PreferredEnclaveType to an empty string unless it isn't 'Default'... - if v := props.PreferredEnclaveType; v != nil && pointer.From(v) != databases.AlwaysEncryptedEnclaveTypeDefault { + // NOTE: Always set the PreferredEnclaveType to an empty string + // if not in the properties that were returned from Azure... + if v := props.PreferredEnclaveType; v != nil { enclaveType = string(pointer.From(v)) } @@ -1506,8 +1524,10 @@ func resourceMsSqlDatabaseSchema() map[string]*pluginsdk.Schema { "enclave_type": { Type: pluginsdk.TypeString, Optional: true, + Computed: true, // TODO: Remove Computed in 4.0 ValidateFunc: validation.StringInSlice([]string{ string(databases.AlwaysEncryptedEnclaveTypeVBS), + string(databases.AlwaysEncryptedEnclaveTypeDefault), }, false), }, diff --git a/internal/services/mssql/mssql_database_resource_test.go b/internal/services/mssql/mssql_database_resource_test.go index 73134aebdfda..56620a9bf40d 100644 --- a/internal/services/mssql/mssql_database_resource_test.go +++ b/internal/services/mssql/mssql_database_resource_test.go @@ -90,7 +90,7 @@ func TestAccMsSqlDatabase_complete(t *testing.T) { check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("license_type").HasValue("LicenseIncluded"), check.That(data.ResourceName).Key("max_size_gb").HasValue("2"), - check.That(data.ResourceName).Key("enclave_type").IsEmpty(), + check.That(data.ResourceName).Key("enclave_type").HasValue("Default"), check.That(data.ResourceName).Key("tags.%").HasValue("1"), check.That(data.ResourceName).Key("tags.ENV").HasValue("Staging"), ), @@ -844,12 +844,13 @@ func TestAccMsSqlDatabase_enclaveType(t *testing.T) { } func TestAccMsSqlDatabase_enclaveTypeUpdate(t *testing.T) { + // NOTE: Once the enclave_type field has be set it cannot be changed... data := acceptance.BuildTestData(t, "azurerm_mssql_database", "test") r := MsSqlDatabaseResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.enclaveType(data, ""), + Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("enclave_type").IsEmpty(), @@ -865,10 +866,18 @@ func TestAccMsSqlDatabase_enclaveTypeUpdate(t *testing.T) { }, data.ImportStep(), { - Config: r.enclaveType(data, ""), + Config: r.enclaveType(data, ` enclave_type = "Default"`), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("enclave_type").IsEmpty(), + check.That(data.ResourceName).Key("enclave_type").HasValue("Default"), + ), + }, + data.ImportStep(), + { + Config: r.enclaveType(data, ` enclave_type = "VBS"`), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("enclave_type").HasValue("VBS"), ), }, data.ImportStep(), @@ -1067,6 +1076,7 @@ resource "azurerm_mssql_database" "test" { license_type = "LicenseIncluded" max_size_gb = 2 sku_name = "GP_Gen5_2" + enclave_type = "Default" storage_account_type = "Zone" diff --git a/internal/services/mssql/mssql_elasticpool_resource.go b/internal/services/mssql/mssql_elasticpool_resource.go index bd25127e48d7..a78b061fe01e 100644 --- a/internal/services/mssql/mssql_elasticpool_resource.go +++ b/internal/services/mssql/mssql_elasticpool_resource.go @@ -182,8 +182,10 @@ func resourceMsSqlElasticPool() *pluginsdk.Resource { "enclave_type": { Type: pluginsdk.TypeString, Optional: true, + Computed: true, // TODO: Remove Computed in 4.0 ValidateFunc: validation.StringInSlice([]string{ string(databases.AlwaysEncryptedEnclaveTypeVBS), + string(databases.AlwaysEncryptedEnclaveTypeDefault), }, false), }, @@ -200,13 +202,27 @@ func resourceMsSqlElasticPool() *pluginsdk.Resource { "tags": commonschema.Tags(), }, - CustomizeDiff: pluginsdk.CustomizeDiffShim(func(ctx context.Context, diff *pluginsdk.ResourceDiff, v interface{}) error { - if err := helper.MSSQLElasticPoolValidateSKU(diff); err != nil { - return err - } + CustomizeDiff: pluginsdk.CustomDiffWithAll( + func(ctx context.Context, diff *pluginsdk.ResourceDiff, v interface{}) error { + if err := helper.MSSQLElasticPoolValidateSKU(diff); err != nil { + return err + } - return nil - }), + return nil + }, + + pluginsdk.ForceNewIfChange("enclave_type", func(ctx context.Context, old, new, _ interface{}) bool { + // enclave_type cannot be removed once it has been set + // but can be changed between VBS and Default... + // this Diff will not work until 4.0 when we remove + // the computed property from the field scheam. + if old.(string) != "" && new.(string) == "" { + return true + } + + return false + }), + ), } } @@ -236,13 +252,6 @@ func resourceMsSqlElasticPoolCreateUpdate(d *pluginsdk.ResourceData, meta interf location := azure.NormalizeLocation(d.Get("location").(string)) sku := expandMsSqlElasticPoolSku(d) - // NOTE: Set the default value, if the field exists in the config the only value - // that it could be is 'VBS'... - enclaveType := elasticpools.AlwaysEncryptedEnclaveTypeDefault - if v, ok := d.GetOk("enclave_type"); ok { - enclaveType = elasticpools.AlwaysEncryptedEnclaveType(v.(string)) - } - maintenanceConfigId := publicmaintenanceconfigurations.NewPublicMaintenanceConfigurationID(subscriptionId, d.Get("maintenance_configuration_name").(string)) elasticPool := elasticpools.ElasticPool{ Name: pointer.To(id.ElasticPoolName), @@ -252,12 +261,17 @@ func resourceMsSqlElasticPoolCreateUpdate(d *pluginsdk.ResourceData, meta interf Properties: &elasticpools.ElasticPoolProperties{ LicenseType: pointer.To(elasticpools.ElasticPoolLicenseType(d.Get("license_type").(string))), PerDatabaseSettings: expandMsSqlElasticPoolPerDatabaseSettings(d), - PreferredEnclaveType: pointer.To(enclaveType), ZoneRedundant: pointer.To(d.Get("zone_redundant").(bool)), MaintenanceConfigurationId: pointer.To(maintenanceConfigId.ID()), + PreferredEnclaveType: nil, }, } + // NOTE: The service default is actually nil/empty which indicates enclave is disabled. the value `Default` is NOT the default. + if v, ok := d.GetOk("enclave_type"); ok && v.(string) != "" { + elasticPool.Properties.PreferredEnclaveType = pointer.To(elasticpools.AlwaysEncryptedEnclaveType(v.(string))) + } + if d.HasChange("max_size_gb") { if v, ok := d.GetOk("max_size_gb"); ok { maxSizeBytes := v.(float64) * 1073741824 @@ -308,9 +322,7 @@ func resourceMsSqlElasticPoolRead(d *pluginsdk.ResourceData, meta interface{}) e if props := model.Properties; props != nil { enclaveType := "" - - // NOTE: Always set the PreferredEnclaveType to an empty string unless it isn't 'Default'... - if v := props.PreferredEnclaveType; v != nil && pointer.From(v) != elasticpools.AlwaysEncryptedEnclaveTypeDefault { + if v := props.PreferredEnclaveType; v != nil { enclaveType = string(pointer.From(v)) } d.Set("enclave_type", enclaveType) diff --git a/internal/services/mssql/mssql_elasticpool_resource_test.go b/internal/services/mssql/mssql_elasticpool_resource_test.go index b375d506dd16..a8684ae06c1f 100644 --- a/internal/services/mssql/mssql_elasticpool_resource_test.go +++ b/internal/services/mssql/mssql_elasticpool_resource_test.go @@ -369,6 +369,7 @@ func TestAccMsSqlElasticPool_vCoreToStandardDTU(t *testing.T) { } func TestAccMsSqlElasticPool_enclaveTypeUpdate(t *testing.T) { + // NOTE: Once the enclave_type has be set it cannot be removed... data := acceptance.BuildTestData(t, "azurerm_mssql_elasticpool", "test") r := MsSqlElasticPoolResource{} @@ -390,10 +391,18 @@ func TestAccMsSqlElasticPool_enclaveTypeUpdate(t *testing.T) { }, data.ImportStep("max_size_gb"), { - Config: r.basicDTU(data, ""), + Config: r.basicDTU(data, `enclave_type = "Default"`), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("enclave_type").IsEmpty(), + check.That(data.ResourceName).Key("enclave_type").HasValue("Default"), + ), + }, + data.ImportStep("max_size_gb"), + { + Config: r.basicDTU(data, `enclave_type = "VBS"`), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("enclave_type").HasValue("VBS"), ), }, data.ImportStep("max_size_gb"), diff --git a/website/docs/r/mssql_database.html.markdown b/website/docs/r/mssql_database.html.markdown index 14e45f6a8c2b..2dcf25a18613 100644 --- a/website/docs/r/mssql_database.html.markdown +++ b/website/docs/r/mssql_database.html.markdown @@ -190,11 +190,13 @@ The following arguments are supported: * `elastic_pool_id` - (Optional) Specifies the ID of the elastic pool containing this database. -* `enclave_type` - (Optional) Specifies the type of enclave to be used by the database. Possible value `VBS`. +* `enclave_type` - (Optional) Specifies the type of enclave to be used by the elastic pool. When `enclave_type` is not specified (e.g., the default) enclaves are not enabled on the database. Possible values are `Default` or `VBS`. -~> **NOTE:** `enclave_type` is currently not supported for DW (e.g, DataWarehouse) and DC-series SKUs. +-> **NOTE:** `enclave_type` is currently not supported for DW (e.g, DataWarehouse) and DC-series SKUs. -~> **NOTE:** Geo Replicated and Failover databases must have the same `enclave_type`. +-> **NOTE:** Geo Replicated and Failover databases must have the same `enclave_type`. + +~> **NOTE:** The default value for the `enclave_type` field is unset not `Default`. * `geo_backup_enabled` - (Optional) A boolean that specifies if the Geo Backup Policy is enabled. Defaults to `true`. @@ -236,7 +238,7 @@ The following arguments are supported: * `sku_name` - (Optional) Specifies the name of the SKU used by the database. For example, `GP_S_Gen5_2`,`HS_Gen4_1`,`BC_Gen5_2`, `ElasticPool`, `Basic`,`S0`, `P2` ,`DW100c`, `DS100`. Changing this from the HyperScale service tier to another service tier will create a new resource. -~> **NOTE:** The default `sku_name` value may differ between Azure locations depending on local availability of Gen4/Gen5 capacity. When databases are replicated using the `creation_source_database_id` property, the source (primary) database cannot have a higher SKU service tier than any secondary databases. When changing the `sku_name` of a database having one or more secondary databases, this resource will first update any secondary databases as necessary. In such cases it's recommended to use the same `sku_name` in your configuration for all related databases, as not doing so may cause an unresolvable diff during subsequent plans. +-> **NOTE:** The default `sku_name` value may differ between Azure locations depending on local availability of Gen4/Gen5 capacity. When databases are replicated using the `creation_source_database_id` property, the source (primary) database cannot have a higher SKU service tier than any secondary databases. When changing the `sku_name` of a database having one or more secondary databases, this resource will first update any secondary databases as necessary. In such cases it's recommended to use the same `sku_name` in your configuration for all related databases, as not doing so may cause an unresolvable diff during subsequent plans. * `storage_account_type` - (Optional) Specifies the storage account type used to store backups for this database. Possible values are `Geo`, `GeoZone`, `Local` and `Zone`. Defaults to `Geo`. diff --git a/website/docs/r/mssql_elasticpool.html.markdown b/website/docs/r/mssql_elasticpool.html.markdown index aa7fef12a8b0..a29e003a1fae 100644 --- a/website/docs/r/mssql_elasticpool.html.markdown +++ b/website/docs/r/mssql_elasticpool.html.markdown @@ -73,11 +73,13 @@ The following arguments are supported: -> **NOTE:** One of either `max_size_gb` or `max_size_bytes` must be specified. -* `enclave_type` - (Optional) Specifies the type of enclave to be used by the elastic pool. Possible value `VBS`. +* `enclave_type` - (Optional) Specifies the type of enclave to be used by the elastic pool. When `enclave_type` is not specified (e.g., the default) enclaves are not enabled on the elastic pool. Possible values are `Default` or `VBS`. -~> **NOTE:** All databases that are added to the elastic pool must have the same `enclave_type` as the elastic pool. +-> **NOTE:** All databases that are added to the elastic pool must have the same `enclave_type` as the elastic pool. -~> **NOTE:** `enclave_type` is not supported for DC-series SKUs. +-> **NOTE:** `enclave_type` is not supported for DC-series SKUs. + +~> **NOTE:** The default value for `enclave_type` field is unset not `Default`. * `tags` - (Optional) A mapping of tags to assign to the resource.