From 94343e7d7ebdddf26b4e46d311396f74c16d8ab2 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Thu, 6 Apr 2023 13:10:06 -0600 Subject: [PATCH 01/17] Initial Check-in... --- .../search/search_service_resource.go | 230 +++++++++++++--- .../search/search_service_resource_test.go | 252 +++++++++++++++--- .../search_service_partition_count.go | 20 ++ 3 files changed, 425 insertions(+), 77 deletions(-) create mode 100644 internal/services/search/validate/search_service_partition_count.go diff --git a/internal/services/search/search_service_resource.go b/internal/services/search/search_service_resource.go index 611433caa580..7b53f1e9a67a 100644 --- a/internal/services/search/search_service_resource.go +++ b/internal/services/search/search_service_resource.go @@ -3,6 +3,7 @@ package search import ( "fmt" "log" + "strings" "time" "github.com/hashicorp/go-azure-helpers/lang/response" @@ -17,6 +18,7 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/helpers/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" + validateSearch "github.com/hashicorp/terraform-provider-azurerm/internal/services/search/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation" "github.com/hashicorp/terraform-provider-azurerm/internal/timeouts" @@ -25,9 +27,9 @@ import ( func resourceSearchService() *pluginsdk.Resource { return &pluginsdk.Resource{ - Create: resourceSearchServiceCreateUpdate, + Create: resourceSearchServiceCreate, Read: resourceSearchServiceRead, - Update: resourceSearchServiceCreateUpdate, + Update: resourceSearchServiceUpdate, Delete: resourceSearchServiceDelete, Timeouts: &pluginsdk.ResourceTimeout{ @@ -64,21 +66,33 @@ func resourceSearchService() *pluginsdk.Resource { string(services.SkuNameStandardTwo), string(services.SkuNameStandardThree), string(services.SkuNameStorageOptimizedLOne), - string(services.SkuNameStorageOptimizedLOne), + string(services.SkuNameStorageOptimizedLTwo), }, false), }, "replica_count": { - Type: pluginsdk.TypeInt, - Optional: true, - Computed: true, + Type: pluginsdk.TypeInt, + Optional: true, + Default: 1, + ValidateFunc: validation.IntBetween(1, 12), }, "partition_count": { Type: pluginsdk.TypeInt, Optional: true, - Computed: true, - ValidateFunc: validation.IntAtMost(12), + Default: 1, + ValidateFunc: validateSearch.PartitionCount, + }, + + "hosting_mode": { + Type: pluginsdk.TypeString, + Optional: true, + ForceNew: true, + Default: services.HostingModeDefault, + ValidateFunc: validation.StringInSlice([]string{ + string(services.HostingModeDefault), + string(services.HostingModeHighDensity), + }, false), }, "primary_key": { @@ -116,7 +130,7 @@ func resourceSearchService() *pluginsdk.Resource { }, "allowed_ips": { - Type: pluginsdk.TypeList, + Type: pluginsdk.TypeSet, Optional: true, Elem: &pluginsdk.Schema{ Type: pluginsdk.TypeString, @@ -134,24 +148,21 @@ func resourceSearchService() *pluginsdk.Resource { } } -func resourceSearchServiceCreateUpdate(d *pluginsdk.ResourceData, meta interface{}) error { +func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Search.ServicesClient subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() id := services.NewSearchServiceID(subscriptionId, d.Get("resource_group_name").(string), d.Get("name").(string)) - if d.IsNewResource() { - existing, err := client.Get(ctx, id, services.GetOperationOptions{}) - if err != nil { - if !response.WasNotFound(existing.HttpResponse) { - return fmt.Errorf("checking for presence of existing %s: %+v", id, err) - } - } - if !response.WasNotFound(existing.HttpResponse) { - return tf.ImportAsExistsError("azurerm_search_service", id.ID()) - } + existing, err := client.Get(ctx, id, services.GetOperationOptions{}) + if err != nil && !response.WasNotFound(existing.HttpResponse) { + return fmt.Errorf("checking for presence of existing %s: %+v", id, err) + } + + if !response.WasNotFound(existing.HttpResponse) { + return tf.ImportAsExistsError("azurerm_search_service", id.ID()) } location := azure.NormalizeLocation(d.Get("location").(string)) @@ -161,13 +172,36 @@ func resourceSearchServiceCreateUpdate(d *pluginsdk.ResourceData, meta interface publicNetworkAccess = services.PublicNetworkAccessDisabled } - expandedIdentity, err := identity.ExpandSystemAssigned(d.Get("identity").([]interface{})) + skuName := services.SkuName(d.Get("sku").(string)) + hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) + + // NOTE: hosting mode is only valid if the SKU is 'standard3' + if skuName != services.SkuNameStandardThree && hostingMode == services.HostingModeHighDensity { + return fmt.Errorf("'hosting_mode' can only be defined if the 'sku' field is set to the 'standard3' SKU, got %q", skuName) + } + + // NOTE: 'partition_count' values greater than 1 are not valid for 'free' or 'basic' SKUs... + partitionCount := int64(d.Get("partition_count").(int)) + + if (skuName == services.SkuNameFree || skuName == services.SkuNameBasic) && partitionCount > 1 { + return fmt.Errorf("'partition_count' values greater than 1 cannot be set for the %q SKU, got %d)", skuName, partitionCount) + } + + // NOTE: 'standard3' services with 'hostingMode' set to 'highDensity' the + // 'partition_count' must be between 1 and 3. + if skuName == services.SkuNameStandardThree && partitionCount > 3 && hostingMode == services.HostingModeHighDensity { + return fmt.Errorf("'standard3' SKUs in 'highDensity' mode can have a maximum of 3 partitions, got %d", partitionCount) + } + + // The number of replicas can be between 1 and 12 for 'standard', 'storage_optimized_l1' and storage_optimized_l2' SKUs + // or between 1 and 3 for 'basic' SKU. Defaults to 1. + replicaCount, err := validateSearchServiceReplicaCount(int64(d.Get("replica_count").(int)), skuName) if err != nil { - return fmt.Errorf("expanding `identity`: %+v", err) + return err } - skuName := services.SkuName(d.Get("sku").(string)) - properties := services.SearchService{ + ipRulesRaw := d.Get("allowed_ips").(*pluginsdk.Set).List() + searchService := services.SearchService{ Location: location, Sku: &services.Sku{ Name: &skuName, @@ -175,32 +209,125 @@ func resourceSearchServiceCreateUpdate(d *pluginsdk.ResourceData, meta interface Properties: &services.SearchServiceProperties{ PublicNetworkAccess: &publicNetworkAccess, NetworkRuleSet: &services.NetworkRuleSet{ - IPRules: expandSearchServiceIPRules(d.Get("allowed_ips").([]interface{})), + IPRules: expandSearchServiceIPRules(ipRulesRaw), }, + HostingMode: &hostingMode, + PartitionCount: &partitionCount, + ReplicaCount: &replicaCount, }, - Identity: expandedIdentity, - Tags: tags.Expand(d.Get("tags").(map[string]interface{})), + Tags: tags.Expand(d.Get("tags").(map[string]interface{})), } - if v, ok := d.GetOk("replica_count"); ok { - replicaCount := int64(v.(int)) - properties.Properties.ReplicaCount = utils.Int64(replicaCount) + expandedIdentity, err := identity.ExpandSystemAssigned(d.Get("identity").([]interface{})) + if err != nil { + return fmt.Errorf("expanding `identity`: %+v", err) } - if v, ok := d.GetOk("partition_count"); ok { - partitionCount := int64(v.(int)) - properties.Properties.PartitionCount = utils.Int64(partitionCount) + // fix for issue #10151, if the identity type is TypeNone do not include it + // in the create call, only in the update call when 'identity' is removed from the + // configuration file... + if expandedIdentity.Type != identity.TypeNone { + searchService.Identity = expandedIdentity } - err = client.CreateOrUpdateThenPoll(ctx, id, properties, services.CreateOrUpdateOperationOptions{}) + err = client.CreateOrUpdateThenPoll(ctx, id, searchService, services.CreateOrUpdateOperationOptions{}) if err != nil { - return fmt.Errorf("creating/updating %s: %+v", id, err) + return fmt.Errorf("creating %s: %+v", id, err) } d.SetId(id.ID()) return resourceSearchServiceRead(d, meta) } +func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Search.ServicesClient + ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) + defer cancel() + + id, err := services.ParseSearchServiceID(d.Id()) + if err != nil { + return err + } + + resp, err := client.Get(ctx, *id, services.GetOperationOptions{}) + if err != nil { + return fmt.Errorf("retrieving %s: %+v", id, err) + } + + if model := resp.Model; model != nil { + if d.HasChange("public_network_access_enabled") { + publicNetworkAccess := services.PublicNetworkAccessEnabled + if enabled := d.Get("public_network_access_enabled").(bool); !enabled { + publicNetworkAccess = services.PublicNetworkAccessDisabled + } + + model.Properties.PublicNetworkAccess = &publicNetworkAccess + } + + if d.HasChange("identity") { + expandedIdentity, err := identity.ExpandSystemAssigned(d.Get("identity").([]interface{})) + if err != nil { + return fmt.Errorf("expanding `identity`: %+v", err) + } + + // NOTE: Passing type 'None' on update will remove all identities from the service. + model.Identity = expandedIdentity + } + + if d.HasChange("hosting_mode") { + hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) + if *model.Sku.Name != services.SkuNameStandardThree && hostingMode == services.HostingModeHighDensity { + return fmt.Errorf("'hosting_mode' can only be set to 'highDensity' if the 'sku' is 'standard3', got %q", *model.Sku.Name) + } + + model.Properties.HostingMode = &hostingMode + } + + if d.HasChange("replica_count") { + replicaCount, err := validateSearchServiceReplicaCount(int64(d.Get("replica_count").(int)), *model.Sku.Name) + if err != nil { + return err + } + + model.Properties.ReplicaCount = utils.Int64(replicaCount) + } + + if d.HasChange("partition_count") { + partitionCount := int64(d.Get("partition_count").(int)) + // NOTE: 'partition_count' values greater than 1 are not valid for 'free' or 'basic' SKUs... + if (*model.Sku.Name == services.SkuNameFree || *model.Sku.Name == services.SkuNameBasic) && partitionCount > 1 { + return fmt.Errorf("'partition_count' values greater than 1 cannot be set for the %q SKU, got %d)", *model.Sku.Name, partitionCount) + } + + // NOTE: If SKU is 'standard3' and the 'hosting_mode' is set to 'highDensity' the maximum number of partitions allowed is 3 + // where if 'hosting_mode' is set to 'default' the maximum number of partitions is 12... + if *model.Sku.Name == services.SkuNameStandardThree && partitionCount > 3 && *model.Properties.HostingMode == services.HostingModeHighDensity { + return fmt.Errorf("'standard3' SKUs in 'highDensity' mode can have a maximum of 3 partitions, got %d", partitionCount) + } + + model.Properties.PartitionCount = utils.Int64(partitionCount) + } + + if d.HasChange("allowed_ips") { + ipRulesRaw := d.Get("allowed_ips").(*pluginsdk.Set).List() + model.Properties.NetworkRuleSet.IPRules = expandSearchServiceIPRules(ipRulesRaw) + } + + if d.HasChange("tags") { + model.Tags = tags.Expand(d.Get("tags").(map[string]interface{})) + } + + err = client.CreateOrUpdateThenPoll(ctx, *id, *model, services.CreateOrUpdateOperationOptions{}) + if err != nil { + return fmt.Errorf("updating %s: %+v", id, err) + } + + return resourceSearchServiceRead(d, meta) + } + + return nil +} + func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Search.ServicesClient ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) @@ -235,9 +362,10 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro d.Set("sku", skuName) if props := model.Properties; props != nil { - partitionCount := 0 - replicaCount := 0 - publicNetworkAccess := false + partitionCount := 1 // Default + replicaCount := 1 // Default + publicNetworkAccess := true // publicNetworkAccess defaults to true... + hostingMode := services.HostingModeDefault if count := props.PartitionCount; count != nil { partitionCount = int(*count) @@ -247,16 +375,25 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro replicaCount = int(*count) } + // NOTE: There is a bug in the API where it returns the PublicNetworkAccess value + // as 'Disabled' instead of with the casing of their const 'disabled' if props.PublicNetworkAccess != nil { - publicNetworkAccess = *props.PublicNetworkAccess != "Disabled" + publicNetworkAccess = strings.EqualFold(string(*props.PublicNetworkAccess), string(services.PublicNetworkAccessEnabled)) + } + + if props.HostingMode != nil { + hostingMode = *props.HostingMode } d.Set("partition_count", partitionCount) d.Set("replica_count", replicaCount) d.Set("public_network_access_enabled", publicNetworkAccess) + d.Set("hosting_mode", hostingMode) d.Set("allowed_ips", flattenSearchServiceIPRules(props.NetworkRuleSet)) } + // NOTE: if the identity has been removed(e.g. by passing type "None" during update), + // this will also remove it from the state... if err = d.Set("identity", identity.FlattenSystemAssigned(model.Identity)); err != nil { return fmt.Errorf("setting `identity`: %s", err) } @@ -361,3 +498,18 @@ func flattenSearchServiceIPRules(input *services.NetworkRuleSet) []interface{} { } return result } + +func validateSearchServiceReplicaCount(replicaCount int64, skuName services.SkuName) (int64, error) { + switch skuName { + case services.SkuNameFree: + if replicaCount > 1 { + return 0, fmt.Errorf("'replica_count' cannot be greater than 1 for the %q SKU, got %d", skuName, replicaCount) + } + case services.SkuNameBasic: + if replicaCount > 3 { + return 0, fmt.Errorf("'replica_count' must be between 1 and 3 for the %q SKU, got %d)", skuName, replicaCount) + } + } + + return replicaCount, nil +} diff --git a/internal/services/search/search_service_resource_test.go b/internal/services/search/search_service_resource_test.go index 4a9b37d97629..56b9331344f0 100644 --- a/internal/services/search/search_service_resource_test.go +++ b/internal/services/search/search_service_resource_test.go @@ -3,6 +3,7 @@ package search_test import ( "context" "fmt" + "regexp" "testing" "github.com/hashicorp/go-azure-sdk/resource-manager/search/2022-09-01/services" @@ -13,11 +14,45 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/utils" ) -type SearchServiceResource struct{} +type SearchServiceResource struct { + sku string +} + +func TestAccSearchService_basicStandard(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "standard"} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("tags.%").HasValue("1"), + ), + }, + data.ImportStep(), + }) +} + +func TestAccSearchService_basicFree(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "free"} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("tags.%").HasValue("1"), + ), + }, + data.ImportStep(), + }) +} -func TestAccSearchService_basic(t *testing.T) { +func TestAccSearchService_basicBasic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{} + r := SearchServiceResource{sku: "basic"} data.ResourceTest(t, r, []acceptance.TestStep{ { @@ -33,7 +68,7 @@ func TestAccSearchService_basic(t *testing.T) { func TestAccSearchService_requiresImport(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{} + r := SearchServiceResource{sku: "standard"} data.ResourceTest(t, r, []acceptance.TestStep{ { Config: r.basic(data), @@ -48,7 +83,7 @@ func TestAccSearchService_requiresImport(t *testing.T) { func TestAccSearchService_complete(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{} + r := SearchServiceResource{sku: "standard"} data.ResourceTest(t, r, []acceptance.TestStep{ { @@ -69,7 +104,7 @@ func TestAccSearchService_complete(t *testing.T) { func TestAccSearchService_update(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{} + r := SearchServiceResource{sku: "standard"} data.ResourceTest(t, r, []acceptance.TestStep{ { @@ -95,7 +130,7 @@ func TestAccSearchService_update(t *testing.T) { func TestAccSearchService_ipRules(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{} + r := SearchServiceResource{sku: "standard"} data.ResourceTest(t, r, []acceptance.TestStep{ { @@ -127,7 +162,7 @@ func TestAccSearchService_ipRules(t *testing.T) { func TestAccSearchService_identity(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{} + r := SearchServiceResource{sku: "standard"} data.ResourceTest(t, r, []acceptance.TestStep{ { @@ -157,6 +192,61 @@ func TestAccSearchService_identity(t *testing.T) { }) } +func TestAccSearchService_hostingMode(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "standard3"} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.hostingMode(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("tags.%").HasValue("1"), + ), + }, + data.ImportStep(), + }) +} + +func TestAccSearchService_hostingModeInvalidSKU(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "standard2"} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.hostingModeInvalidSKU(data), + Check: acceptance.ComposeTestCheckFunc(), + ExpectError: regexp.MustCompile("can only be defined if"), + }, + }) +} + +func TestAccSearchService_partitionCountInvalid(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "free"} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.partitionCountInvalid(data), + Check: acceptance.ComposeTestCheckFunc(), + ExpectError: regexp.MustCompile("values greater than 1 cannot be set"), + }, + }) +} + +func TestAccSearchService_replicaCountInvalid(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "free"} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.replicaCountInvalid(data), + Check: acceptance.ComposeTestCheckFunc(), + ExpectError: regexp.MustCompile("cannot be greater than 1 for the"), + }, + }) +} + func (t SearchServiceResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := services.ParseSearchServiceID(state.ID) if err != nil { @@ -171,34 +261,42 @@ func (t SearchServiceResource) Exists(ctx context.Context, clients *clients.Clie return utils.Bool(resp.Model != nil), nil } -func (SearchServiceResource) basic(data acceptance.TestData) string { +func (SearchServiceResource) template(data acceptance.TestData) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-search-%d" + location = "%s" +} +`, data.RandomInteger, data.Locations.Primary) +} + +func (r SearchServiceResource) basic(data acceptance.TestData) string { + template := r.template(data) return fmt.Sprintf(` provider "azurerm" { features {} } -resource "azurerm_resource_group" "test" { - name = "acctestRG-%d" - location = "%s" -} +%s resource "azurerm_search_service" "test" { name = "acctestsearchservice%d" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location - sku = "standard" + sku = "%s" tags = { environment = "staging" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +`, template, data.RandomInteger, r.sku) } -func (SearchServiceResource) requiresImport(data acceptance.TestData) string { - template := SearchServiceResource{}.basic(data) +func (r SearchServiceResource) requiresImport(data acceptance.TestData) string { + template := r.basic(data) return fmt.Sprintf(` %s + resource "azurerm_search_service" "import" { name = azurerm_search_service.test.name resource_group_name = azurerm_search_service.test.resource_group_name @@ -212,22 +310,20 @@ resource "azurerm_search_service" "import" { `, template) } -func (SearchServiceResource) complete(data acceptance.TestData) string { +func (r SearchServiceResource) complete(data acceptance.TestData) string { + template := r.template(data) return fmt.Sprintf(` provider "azurerm" { features {} } -resource "azurerm_resource_group" "test" { - name = "acctestRG-%d" - location = "%s" -} +%s resource "azurerm_search_service" "test" { name = "acctestsearchservice%d" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location - sku = "standard" + sku = "%s" replica_count = 2 partition_count = 3 @@ -238,25 +334,23 @@ resource "azurerm_search_service" "test" { residential = "Area" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +`, template, data.RandomInteger, r.sku) } -func (SearchServiceResource) ipRules(data acceptance.TestData) string { +func (r SearchServiceResource) ipRules(data acceptance.TestData) string { + template := r.template(data) return fmt.Sprintf(` provider "azurerm" { features {} } -resource "azurerm_resource_group" "test" { - name = "acctestRG-search-%d" - location = "%s" -} +%s resource "azurerm_search_service" "test" { name = "acctestsearchservice%d" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location - sku = "standard" + sku = "%s" allowed_ips = ["168.1.5.65", "1.2.3.0/24"] @@ -264,25 +358,23 @@ resource "azurerm_search_service" "test" { environment = "staging" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +`, template, data.RandomInteger, r.sku) } -func (SearchServiceResource) identity(data acceptance.TestData) string { +func (r SearchServiceResource) identity(data acceptance.TestData) string { + template := r.template(data) return fmt.Sprintf(` provider "azurerm" { features {} } -resource "azurerm_resource_group" "test" { - name = "acctestRG-%d" - location = "%s" -} +%s resource "azurerm_search_service" "test" { name = "acctestsearchservice%d" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location - sku = "standard" + sku = "%s" identity { type = "SystemAssigned" @@ -292,5 +384,89 @@ resource "azurerm_search_service" "test" { environment = "staging" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +`, template, data.RandomInteger, r.sku) +} + +func (r SearchServiceResource) hostingMode(data acceptance.TestData) string { + template := r.template(data) + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%s + +resource "azurerm_search_service" "test" { + name = "acctestsearchservice%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + sku = "%s" + hosting_mode = "highDensity" + + tags = { + environment = "staging" + } +} +`, template, data.RandomInteger, r.sku) +} + +func (r SearchServiceResource) hostingModeInvalidSKU(data acceptance.TestData) string { + template := r.template(data) + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%s + +resource "azurerm_search_service" "test" { + name = "acctestsearchservice%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + sku = "%s" + hosting_mode = "highDensity" + + tags = { + environment = "staging" + } +} +`, template, data.RandomInteger, r.sku) +} + +func (r SearchServiceResource) partitionCountInvalid(data acceptance.TestData) string { + template := r.template(data) + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%s + +resource "azurerm_search_service" "test" { + name = "acctestsearchservice%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + sku = "%s" + partition_count = 3 +} +`, template, data.RandomInteger, r.sku) +} + +func (r SearchServiceResource) replicaCountInvalid(data acceptance.TestData) string { + template := r.template(data) + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%s + +resource "azurerm_search_service" "test" { + name = "acctestsearchservice%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + sku = "%s" + replica_count = 2 +} +`, template, data.RandomInteger, r.sku) } diff --git a/internal/services/search/validate/search_service_partition_count.go b/internal/services/search/validate/search_service_partition_count.go new file mode 100644 index 000000000000..46e6d5c214ff --- /dev/null +++ b/internal/services/search/validate/search_service_partition_count.go @@ -0,0 +1,20 @@ +package validate + +import ( + "fmt" +) + +func PartitionCount(v interface{}, k string) (warnings []string, errors []error) { + partitionCount := v.(int) + + switch partitionCount { + case 5, 7, 8, 9, 10, 11: + errors = append(errors, fmt.Errorf("%q must be 1, 2, 3, 4, 6, or 12, got %d", k, partitionCount)) + } + + if partitionCount > 12 { + errors = append(errors, fmt.Errorf("%q must be 1, 2, 3, 4, 6, or 12, got %d", k, partitionCount)) + } + + return warnings, errors +} From 2570c85290da075eba90a1a509fb7b0073a433fa Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Thu, 6 Apr 2023 13:22:27 -0600 Subject: [PATCH 02/17] Update documentation... --- website/docs/r/search_service.html.markdown | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index fd7ba8b3e1c3..d8f12ca6eb02 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -36,21 +36,25 @@ The following arguments are supported: * `resource_group_name` - (Required) The name of the Resource Group where the Search Service should exist. Changing this forces a new Search Service to be created. -* `sku` - (Required) The SKU which should be used for this Search Service. Possible values are `basic`, `free`, `standard`, `standard2`, `standard3`, `storage_optimized_l1` and `storage_optimized_l2`. Changing this forces a new Search Service to be created. +* `sku` - (Required) The SKU which should be used for this Search Service. Possible values include `basic`, `free`, `standard`, `standard2`, `standard3`, `storage_optimized_l1` and `storage_optimized_l2`. Changing this forces a new Search Service to be created. -> The `basic` and `free` SKUs provision the Search Service in a Shared Cluster - the `standard` SKUs use a Dedicated Cluster. -~> **Note:** The SKUs `standard2` and `standard3` are only available when enabled on the backend by Microsoft. +~> **NOTE:** The SKUs `standard2`, `standard3`, `storage_optimized_l1` and `storage_optimized_l2` are only available by submitting a quota increase request to Microsoft. Please see the [product documentation](https://learn.microsoft.com/azure/azure-resource-manager/troubleshooting/error-resource-quota?tabs=azure-cli) on how to submit a quota increase request. --- +* `hosting_mode` - (Optional) Enable high density partitions that allow for up to a 1000 indexes. Possible values are `highDensity` or `default`. Defaults to `default`. Changing this forces a new Search Service to be created. + +-> **NOTE:** When the Search Service is in `highDensity` mode the maximum number of partitions allowed is `3`, to enable `hosting_mode` you must use a `standard3` SKU. + * `public_network_access_enabled` - (Optional) Whether or not public network access is allowed for this resource. Defaults to `true`. -* `partition_count` - (Optional) The number of partitions which should be created. +* `partition_count` - (Optional) The number of partitions which should be created. Possible values include `1`, `2`, `3`, `4`, `6`, or `12`. Defaults to `1`. * `replica_count` - (Optional) The number of replica's which should be created. --> **Note:** `partition_count` and `replica_count` can only be configured when using a `standard` sku. +-> **NOTE:** `replica_count` cannot be configured when using a `free` SKU, `partition_count` cannot be configured when using a `free` or `basic` SKU. For more information please to the [product documentation](https://learn.microsoft.com/azure/search/search-sku-tier). * `allowed_ips` - (Optional) A list of IPv4 addresses or CIDRs that are allowed access to the search service endpoint. From 04679c5d27c1c4c3615aee8962c1cf199b47aa9e Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Thu, 6 Apr 2023 20:08:27 -0600 Subject: [PATCH 03/17] Checking-in progress... --- .../search/search_service_resource.go | 82 +++++++++++++------ .../search/search_service_resource_test.go | 58 ++++++++++++- website/docs/r/search_service.html.markdown | 4 + 3 files changed, 116 insertions(+), 28 deletions(-) diff --git a/internal/services/search/search_service_resource.go b/internal/services/search/search_service_resource.go index 7b53f1e9a67a..69d405142afb 100644 --- a/internal/services/search/search_service_resource.go +++ b/internal/services/search/search_service_resource.go @@ -6,6 +6,7 @@ import ( "strings" "time" + "github.com/hashicorp/go-azure-helpers/lang/pointer" "github.com/hashicorp/go-azure-helpers/lang/response" "github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema" "github.com/hashicorp/go-azure-helpers/resourcemanager/identity" @@ -84,6 +85,12 @@ func resourceSearchService() *pluginsdk.Resource { ValidateFunc: validateSearch.PartitionCount, }, + "local_authentication_disabled": { + Type: pluginsdk.TypeBool, + Optional: true, + Default: false, + }, + "hosting_mode": { Type: pluginsdk.TypeString, Optional: true, @@ -173,7 +180,9 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er } skuName := services.SkuName(d.Get("sku").(string)) + ipRulesRaw := d.Get("allowed_ips").(*pluginsdk.Set).List() hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) + localAuthDisabled := d.Get("local_authentication_disabled").(bool) // NOTE: hosting mode is only valid if the SKU is 'standard3' if skuName != services.SkuNameStandardThree && hostingMode == services.HostingModeHighDensity { @@ -200,24 +209,29 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er return err } - ipRulesRaw := d.Get("allowed_ips").(*pluginsdk.Set).List() searchService := services.SearchService{ Location: location, - Sku: &services.Sku{ - Name: &skuName, - }, + Sku: pointer.To(services.Sku{ + Name: pointer.To(skuName), + }), Properties: &services.SearchServiceProperties{ - PublicNetworkAccess: &publicNetworkAccess, - NetworkRuleSet: &services.NetworkRuleSet{ + PublicNetworkAccess: pointer.To(publicNetworkAccess), + NetworkRuleSet: pointer.To(services.NetworkRuleSet{ IPRules: expandSearchServiceIPRules(ipRulesRaw), - }, - HostingMode: &hostingMode, - PartitionCount: &partitionCount, - ReplicaCount: &replicaCount, + }), + HostingMode: pointer.To(hostingMode), + DisableLocalAuth: pointer.To(localAuthDisabled), + PartitionCount: pointer.To(partitionCount), + ReplicaCount: pointer.To(replicaCount), }, Tags: tags.Expand(d.Get("tags").(map[string]interface{})), } + // AuthOptions must be null if DisableLocalAuth is true + if localAuthDisabled { + searchService.Properties.AuthOptions = nil + } + expandedIdentity, err := identity.ExpandSystemAssigned(d.Get("identity").([]interface{})) if err != nil { return fmt.Errorf("expanding `identity`: %+v", err) @@ -261,7 +275,7 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er publicNetworkAccess = services.PublicNetworkAccessDisabled } - model.Properties.PublicNetworkAccess = &publicNetworkAccess + model.Properties.PublicNetworkAccess = pointer.To(publicNetworkAccess) } if d.HasChange("identity") { @@ -276,36 +290,46 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er if d.HasChange("hosting_mode") { hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) - if *model.Sku.Name != services.SkuNameStandardThree && hostingMode == services.HostingModeHighDensity { - return fmt.Errorf("'hosting_mode' can only be set to 'highDensity' if the 'sku' is 'standard3', got %q", *model.Sku.Name) + if pointer.From(model.Sku.Name) != services.SkuNameStandardThree && hostingMode == services.HostingModeHighDensity { + return fmt.Errorf("'hosting_mode' can only be set to 'highDensity' if the 'sku' is 'standard3', got %q", pointer.From(model.Sku.Name)) } - model.Properties.HostingMode = &hostingMode + model.Properties.HostingMode = pointer.To(hostingMode) + } + + if d.HasChange("local_authentication_disabled") { + localAuthDisabled := d.Get("local_authentication_disabled").(bool) + model.Properties.DisableLocalAuth = pointer.To(localAuthDisabled) + + // AuthOptions must be null if DisableLocalAuth is true + if localAuthDisabled { + model.Properties.AuthOptions = nil + } } if d.HasChange("replica_count") { - replicaCount, err := validateSearchServiceReplicaCount(int64(d.Get("replica_count").(int)), *model.Sku.Name) + replicaCount, err := validateSearchServiceReplicaCount(int64(d.Get("replica_count").(int)), pointer.From(model.Sku.Name)) if err != nil { return err } - model.Properties.ReplicaCount = utils.Int64(replicaCount) + model.Properties.ReplicaCount = pointer.To(replicaCount) } if d.HasChange("partition_count") { partitionCount := int64(d.Get("partition_count").(int)) // NOTE: 'partition_count' values greater than 1 are not valid for 'free' or 'basic' SKUs... - if (*model.Sku.Name == services.SkuNameFree || *model.Sku.Name == services.SkuNameBasic) && partitionCount > 1 { - return fmt.Errorf("'partition_count' values greater than 1 cannot be set for the %q SKU, got %d)", *model.Sku.Name, partitionCount) + if (pointer.From(model.Sku.Name) == services.SkuNameFree || pointer.From(model.Sku.Name) == services.SkuNameBasic) && partitionCount > 1 { + return fmt.Errorf("'partition_count' values greater than 1 cannot be set for the %q SKU, got %d)", pointer.From(model.Sku.Name), partitionCount) } // NOTE: If SKU is 'standard3' and the 'hosting_mode' is set to 'highDensity' the maximum number of partitions allowed is 3 // where if 'hosting_mode' is set to 'default' the maximum number of partitions is 12... - if *model.Sku.Name == services.SkuNameStandardThree && partitionCount > 3 && *model.Properties.HostingMode == services.HostingModeHighDensity { + if pointer.From(model.Sku.Name) == services.SkuNameStandardThree && partitionCount > 3 && pointer.From(model.Properties.HostingMode) == services.HostingModeHighDensity { return fmt.Errorf("'standard3' SKUs in 'highDensity' mode can have a maximum of 3 partitions, got %d", partitionCount) } - model.Properties.PartitionCount = utils.Int64(partitionCount) + model.Properties.PartitionCount = pointer.To(partitionCount) } if d.HasChange("allowed_ips") { @@ -317,7 +341,7 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er model.Tags = tags.Expand(d.Get("tags").(map[string]interface{})) } - err = client.CreateOrUpdateThenPoll(ctx, *id, *model, services.CreateOrUpdateOperationOptions{}) + err = client.CreateOrUpdateThenPoll(ctx, pointer.From(id), pointer.From(model), services.CreateOrUpdateOperationOptions{}) if err != nil { return fmt.Errorf("updating %s: %+v", id, err) } @@ -366,34 +390,38 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro replicaCount := 1 // Default publicNetworkAccess := true // publicNetworkAccess defaults to true... hostingMode := services.HostingModeDefault + localAthDisabled := false if count := props.PartitionCount; count != nil { - partitionCount = int(*count) + partitionCount = int(pointer.From(count)) } if count := props.ReplicaCount; count != nil { - replicaCount = int(*count) + replicaCount = int(pointer.From(count)) } // NOTE: There is a bug in the API where it returns the PublicNetworkAccess value - // as 'Disabled' instead of with the casing of their const 'disabled' + // as 'Disabled' instead of 'disabled' if props.PublicNetworkAccess != nil { - publicNetworkAccess = strings.EqualFold(string(*props.PublicNetworkAccess), string(services.PublicNetworkAccessEnabled)) + publicNetworkAccess = strings.EqualFold(string(pointer.From(props.PublicNetworkAccess)), string(services.PublicNetworkAccessEnabled)) } if props.HostingMode != nil { hostingMode = *props.HostingMode } + if props.DisableLocalAuth != nil { + localAthDisabled = pointer.From(props.DisableLocalAuth) + } + d.Set("partition_count", partitionCount) d.Set("replica_count", replicaCount) d.Set("public_network_access_enabled", publicNetworkAccess) d.Set("hosting_mode", hostingMode) + d.Set("local_authentication_disabled", localAthDisabled) d.Set("allowed_ips", flattenSearchServiceIPRules(props.NetworkRuleSet)) } - // NOTE: if the identity has been removed(e.g. by passing type "None" during update), - // this will also remove it from the state... if err = d.Set("identity", identity.FlattenSystemAssigned(model.Identity)); err != nil { return fmt.Errorf("setting `identity`: %s", err) } diff --git a/internal/services/search/search_service_resource_test.go b/internal/services/search/search_service_resource_test.go index 56b9331344f0..9c88227855ae 100644 --- a/internal/services/search/search_service_resource_test.go +++ b/internal/services/search/search_service_resource_test.go @@ -15,7 +15,8 @@ import ( ) type SearchServiceResource struct { - sku string + sku string + localAuthDisabled bool } func TestAccSearchService_basicStandard(t *testing.T) { @@ -247,6 +248,36 @@ func TestAccSearchService_replicaCountInvalid(t *testing.T) { }) } +func TestAccSearchService_localAuthenticationDisabledUpdate(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "standard", localAuthDisabled: false} + u := SearchServiceResource{sku: "standard", localAuthDisabled: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.localAuthenticationDisabled(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: u.localAuthenticationDisabled(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.localAuthenticationDisabled(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + func (t SearchServiceResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := services.ParseSearchServiceID(state.ID) if err != nil { @@ -328,6 +359,7 @@ resource "azurerm_search_service" "test" { partition_count = 3 public_network_access_enabled = false + local_authentication_disabled = false tags = { environment = "Production" @@ -470,3 +502,27 @@ resource "azurerm_search_service" "test" { } `, template, data.RandomInteger, r.sku) } + +func (r SearchServiceResource) localAuthenticationDisabled(data acceptance.TestData) string { + template := r.template(data) + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%s + +resource "azurerm_search_service" "test" { + name = "acctestsearchservice%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + sku = "%s" + + local_authentication_disabled = %t + + tags = { + environment = "staging" + } +} +`, template, data.RandomInteger, r.sku, r.localAuthDisabled) +} diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index d8f12ca6eb02..d0ecd415c80b 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -48,6 +48,10 @@ The following arguments are supported: -> **NOTE:** When the Search Service is in `highDensity` mode the maximum number of partitions allowed is `3`, to enable `hosting_mode` you must use a `standard3` SKU. +* `local_authentication_disabled` - (Optional) Should calls to the search service be allowed to utilize API keys for authentication? When `local_authentication_disabled` is set to `true`, calls to the search service *will not* be allowed to use API keys for authentication. Possible values include `true` or `false`. Defaults to `false`. + +**NOTE:** This cannot be set to `true` if `dataPlaneAuthOptions` are defined. + * `public_network_access_enabled` - (Optional) Whether or not public network access is allowed for this resource. Defaults to `true`. * `partition_count` - (Optional) The number of partitions which should be created. Possible values include `1`, `2`, `3`, `4`, `6`, or `12`. Defaults to `1`. From 11574a2de3126fda2770f19074398715214fb01d Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Sun, 9 Apr 2023 02:25:41 -0600 Subject: [PATCH 04/17] Checking-in progress... --- .../search/search_service_resource.go | 187 +++++++++++++++--- .../search/search_service_resource_test.go | 140 ++++++------- website/docs/r/search_service.html.markdown | 16 +- 3 files changed, 235 insertions(+), 108 deletions(-) diff --git a/internal/services/search/search_service_resource.go b/internal/services/search/search_service_resource.go index 69d405142afb..209da844d43e 100644 --- a/internal/services/search/search_service_resource.go +++ b/internal/services/search/search_service_resource.go @@ -85,11 +85,34 @@ func resourceSearchService() *pluginsdk.Resource { ValidateFunc: validateSearch.PartitionCount, }, - "local_authentication_disabled": { - Type: pluginsdk.TypeBool, - Optional: true, - Default: false, - }, + // "api_access_control": { + // Type: pluginsdk.TypeSet, + // Optional: true, + // MaxItems: 1, + // Elem: &pluginsdk.Resource{ + // Schema: map[string]*pluginsdk.Schema{ + // "type": { + // Type: pluginsdk.TypeString, + // Optional: true, + // Default: "api_keys", + // ValidateFunc: validation.StringInSlice([]string{ + // "api_keys", + // "role_based_access_control", + // "role_based_access_control_and_api_keys", + // }, false), + // }, + + // "authentication_failure_mode": { + // Type: pluginsdk.TypeString, + // Optional: true, + // ValidateFunc: validation.StringInSlice([]string{ + // string(services.AadAuthFailureModeHTTPFourZeroOneWithBearerChallenge), + // string(services.AadAuthFailureModeHTTPFourZeroThree), + // }, false), + // }, + // }, + // }, + // }, "hosting_mode": { Type: pluginsdk.TypeString, @@ -102,6 +125,17 @@ func resourceSearchService() *pluginsdk.Resource { }, false), }, + "cmk_enforcement_enabled": { + Type: pluginsdk.TypeBool, + Optional: true, + Default: false, + }, + + "cmk_enforcement_compliance": { + Type: pluginsdk.TypeString, + Computed: true, + }, + "primary_key": { Type: pluginsdk.TypeString, Computed: true, @@ -182,7 +216,21 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er skuName := services.SkuName(d.Get("sku").(string)) ipRulesRaw := d.Get("allowed_ips").(*pluginsdk.Set).List() hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) - localAuthDisabled := d.Get("local_authentication_disabled").(bool) + cmkEnforcementEnabled := d.Get("cmk_enforcement_enabled").(bool) + + // NOTE: Temporarily disable these fields... + // AuthenticationOptions := expandSearchServiceDataPlaneAadOrApiKeyAuthOption(d.Get("api_access_control").(*pluginsdk.Set).List()) + + // localAuthDisabled := false + // if AuthenticationOptions == nil { + // // This means that it is RBAC only + // localAuthDisabled = true + // } + + cmkEnforcement := services.SearchEncryptionWithCmkDisabled + if cmkEnforcementEnabled { + cmkEnforcement = services.SearchEncryptionWithCmkEnabled + } // NOTE: hosting mode is only valid if the SKU is 'standard3' if skuName != services.SkuNameStandardThree && hostingMode == services.HostingModeHighDensity { @@ -219,18 +267,24 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er NetworkRuleSet: pointer.To(services.NetworkRuleSet{ IPRules: expandSearchServiceIPRules(ipRulesRaw), }), - HostingMode: pointer.To(hostingMode), - DisableLocalAuth: pointer.To(localAuthDisabled), - PartitionCount: pointer.To(partitionCount), - ReplicaCount: pointer.To(replicaCount), + EncryptionWithCmk: pointer.To(services.EncryptionWithCmk{ + Enforcement: pointer.To(cmkEnforcement), + }), + HostingMode: pointer.To(hostingMode), + // NOTE: Temporarily disable these fields... + // AuthOptions: AuthenticationOptions, + // DisableLocalAuth: pointer.To(localAuthDisabled), + PartitionCount: pointer.To(partitionCount), + ReplicaCount: pointer.To(replicaCount), }, Tags: tags.Expand(d.Get("tags").(map[string]interface{})), } + // NOTE: Temporarily disable these fields... // AuthOptions must be null if DisableLocalAuth is true - if localAuthDisabled { - searchService.Properties.AuthOptions = nil - } + // if localAuthDisabled { + // searchService.Properties.AuthOptions = nil + // } expandedIdentity, err := identity.ExpandSystemAssigned(d.Get("identity").([]interface{})) if err != nil { @@ -297,16 +351,28 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er model.Properties.HostingMode = pointer.To(hostingMode) } - if d.HasChange("local_authentication_disabled") { - localAuthDisabled := d.Get("local_authentication_disabled").(bool) - model.Properties.DisableLocalAuth = pointer.To(localAuthDisabled) - - // AuthOptions must be null if DisableLocalAuth is true - if localAuthDisabled { - model.Properties.AuthOptions = nil + if d.HasChange("cmk_enforcement_enabled") { + cmkEnforcement := services.SearchEncryptionWithCmkDisabled + if enabled := d.Get("cmk_enforcement_enabled").(bool); enabled { + cmkEnforcement = services.SearchEncryptionWithCmkEnabled } + + model.Properties.EncryptionWithCmk.Enforcement = pointer.To(cmkEnforcement) } + // NOTE: Temporarily disable these fields... + // if d.HasChange("api_access_control") { + // authenticationOptions := expandSearchServiceDataPlaneAadOrApiKeyAuthOption(d.Get("api_access_control").(*pluginsdk.Set).List()) + + // model.Properties.DisableLocalAuth = pointer.To(false) + // if authenticationOptions == nil { + // // This means that it is RBAC only + // model.Properties.DisableLocalAuth = pointer.To(true) + // } + + // model.Properties.AuthOptions = authenticationOptions + // } + if d.HasChange("replica_count") { replicaCount, err := validateSearchServiceReplicaCount(int64(d.Get("replica_count").(int)), pointer.From(model.Sku.Name)) if err != nil { @@ -389,8 +455,9 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro partitionCount := 1 // Default replicaCount := 1 // Default publicNetworkAccess := true // publicNetworkAccess defaults to true... + cmkEnforcement := false // cmkEnforcment defaults to false... hostingMode := services.HostingModeDefault - localAthDisabled := false + // localAthDisabled := false if count := props.PartitionCount; count != nil { partitionCount = int(pointer.From(count)) @@ -410,15 +477,24 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro hostingMode = *props.HostingMode } - if props.DisableLocalAuth != nil { - localAthDisabled = pointer.From(props.DisableLocalAuth) + var cmkCompliance string + if props.EncryptionWithCmk != nil { + cmkEnforcement = strings.EqualFold(string(pointer.From(props.EncryptionWithCmk.Enforcement)), string(services.SearchEncryptionWithCmkEnabled)) + cmkCompliance = string(pointer.From(props.EncryptionWithCmk.EncryptionComplianceStatus)) } + // NOTE: Temporarily disable these fields... + // if props.DisableLocalAuth != nil { + // authenticationOptions := flattenSearchServiceDataPlaneAadOrApiKeyAuthOption(props.AuthOptions, props.DisableLocalAuth) + // d.Set("api_access_control", authenticationOptions) + // } + d.Set("partition_count", partitionCount) d.Set("replica_count", replicaCount) d.Set("public_network_access_enabled", publicNetworkAccess) d.Set("hosting_mode", hostingMode) - d.Set("local_authentication_disabled", localAthDisabled) + d.Set("cmk_enforcement_enabled", cmkEnforcement) + d.Set("cmk_enforcement_compliance", cmkCompliance) d.Set("allowed_ips", flattenSearchServiceIPRules(props.NetworkRuleSet)) } @@ -516,6 +592,39 @@ func expandSearchServiceIPRules(input []interface{}) *[]services.IPRule { return &output } +// NOTE: Temporarily disable these fields... +// func expandSearchServiceDataPlaneAadOrApiKeyAuthOption(input []interface{}) *services.DataPlaneAuthOptions { +// var ApiKeyOnly interface{} + +// // the default(e.g. 'ApiKeyOnly'), only requires an empty DataPlaneAuthOptions.ApiKeyOnly interface... +// defaultAuth := pointer.To(services.DataPlaneAuthOptions{ +// ApiKeyOnly: pointer.To(ApiKeyOnly), +// }) + +// if len(input) == 0 { +// return defaultAuth +// } + +// apiAccessControl := input[0].(map[string]interface{}) +// accessControlType := apiAccessControl["type"].(string) +// authFailureMode := apiAccessControl["authentication_failure_mode"].(string) + +// switch accessControlType { +// case "role_based_access_control": +// return nil +// case "api_keys": +// return defaultAuth +// case "role_based_access_control_and_api_keys": +// return pointer.To(services.DataPlaneAuthOptions{ +// AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ +// AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authFailureMode)), +// }), +// }) +// } + +// return defaultAuth +// } + func flattenSearchServiceIPRules(input *services.NetworkRuleSet) []interface{} { if input == nil || *input.IPRules == nil || len(*input.IPRules) == 0 { return nil @@ -527,6 +636,36 @@ func flattenSearchServiceIPRules(input *services.NetworkRuleSet) []interface{} { return result } +// NOTE: Temporarily disable these fields... +// func flattenSearchServiceDataPlaneAadOrApiKeyAuthOption(input *services.DataPlaneAuthOptions, localAuthenticationDisabled *bool) []interface{} { +// // TODO: Validate what I should be checking here... +// // For RBAC Only DataPlaneAuthOptions will be nil... +// if localAuthenticationDisabled == nil { +// return []interface{}{} +// } + +// result := make(map[string]interface{}) + +// // if localAuthenticationDisabled is disabled that means the this in RBAC Only mode... +// if pointer.From(localAuthenticationDisabled) { +// // Auth is RBAC Only... +// // "localAuthenticationDisabled": true +// result["type"] = "role_based_access_control" +// result["authentication_failure_mode"] = "" +// } else { +// // Auth can be API Only or RBAC and API... +// if input.AadOrApiKey != nil && input.AadOrApiKey.AadAuthFailureMode != nil { +// result["type"] = "role_based_access_control_and_api_keys" +// result["authentication_failure_mode"] = pointer.From(input.AadOrApiKey.AadAuthFailureMode) +// } else { +// result["type"] = "api_keys" +// result["authentication_failure_mode"] = "" +// } +// } + +// return []interface{}{result} +// } + func validateSearchServiceReplicaCount(replicaCount int64, skuName services.SkuName) (int64, error) { switch skuName { case services.SkuNameFree: diff --git a/internal/services/search/search_service_resource_test.go b/internal/services/search/search_service_resource_test.go index 9c88227855ae..806efdee621a 100644 --- a/internal/services/search/search_service_resource_test.go +++ b/internal/services/search/search_service_resource_test.go @@ -15,8 +15,9 @@ import ( ) type SearchServiceResource struct { - sku string - localAuthDisabled bool + sku string + count int + enforceCmk bool } func TestAccSearchService_basicStandard(t *testing.T) { @@ -28,7 +29,6 @@ func TestAccSearchService_basicStandard(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.ImportStep(), @@ -44,7 +44,6 @@ func TestAccSearchService_basicFree(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.ImportStep(), @@ -60,7 +59,6 @@ func TestAccSearchService_basicBasic(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.ImportStep(), @@ -75,7 +73,6 @@ func TestAccSearchService_requiresImport(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.RequiresImportErrorStep(r.requiresImport), @@ -91,12 +88,6 @@ func TestAccSearchService_complete(t *testing.T) { Config: r.complete(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("2"), - check.That(data.ResourceName).Key("replica_count").HasValue("2"), - check.That(data.ResourceName).Key("primary_key").Exists(), - check.That(data.ResourceName).Key("secondary_key").Exists(), - check.That(data.ResourceName).Key("query_keys.#").HasValue("1"), - check.That(data.ResourceName).Key("public_network_access_enabled").HasValue("false"), ), }, data.ImportStep(), @@ -112,20 +103,16 @@ func TestAccSearchService_update(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), - check.That(data.ResourceName).Key("tags.environment").HasValue("staging"), - check.That(data.ResourceName).Key("public_network_access_enabled").HasValue("true"), ), }, + data.ImportStep(), { Config: r.complete(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("2"), - check.That(data.ResourceName).Key("tags.environment").HasValue("Production"), - check.That(data.ResourceName).Key("public_network_access_enabled").HasValue("false"), ), }, + data.ImportStep(), }) } @@ -138,7 +125,6 @@ func TestAccSearchService_ipRules(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.ImportStep(), @@ -146,7 +132,6 @@ func TestAccSearchService_ipRules(t *testing.T) { Config: r.ipRules(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.ImportStep(), @@ -154,7 +139,6 @@ func TestAccSearchService_ipRules(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.ImportStep(), @@ -170,7 +154,6 @@ func TestAccSearchService_identity(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.ImportStep(), @@ -178,7 +161,6 @@ func TestAccSearchService_identity(t *testing.T) { Config: r.identity(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.ImportStep(), @@ -186,7 +168,6 @@ func TestAccSearchService_identity(t *testing.T) { Config: r.basic(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.ImportStep(), @@ -202,7 +183,6 @@ func TestAccSearchService_hostingMode(t *testing.T) { Config: r.hostingMode(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("tags.%").HasValue("1"), ), }, data.ImportStep(), @@ -215,64 +195,89 @@ func TestAccSearchService_hostingModeInvalidSKU(t *testing.T) { data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.hostingModeInvalidSKU(data), + Config: r.hostingMode(data), Check: acceptance.ComposeTestCheckFunc(), ExpectError: regexp.MustCompile("can only be defined if"), }, }) } -func TestAccSearchService_partitionCountInvalid(t *testing.T) { +func TestAccSearchService_partitionCountInvalidBySku(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "free"} + r := SearchServiceResource{sku: "basic", count: 3} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.partitionCountInvalid(data), + Config: r.partitionCount(data), Check: acceptance.ComposeTestCheckFunc(), ExpectError: regexp.MustCompile("values greater than 1 cannot be set"), }, }) } +func TestAccSearchService_partitionCountInvalidByInput(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "standard", count: 5} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.partitionCount(data), + Check: acceptance.ComposeTestCheckFunc(), + ExpectError: regexp.MustCompile("must be 1"), + }, + }) +} + func TestAccSearchService_replicaCountInvalid(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "free"} + // NOTE: The default qouta for the 'free' sku is 1 + // but it is ok to use it here since it will never be created since + // it will fail validation due to the replica count defined in the + // test configuration... + r := SearchServiceResource{sku: "free", count: 2} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.replicaCountInvalid(data), + Config: r.replicaCount(data), Check: acceptance.ComposeTestCheckFunc(), ExpectError: regexp.MustCompile("cannot be greater than 1 for the"), }, }) } -func TestAccSearchService_localAuthenticationDisabledUpdate(t *testing.T) { +func TestAccSearchService_cmkEnforcement(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard", localAuthDisabled: false} - u := SearchServiceResource{sku: "standard", localAuthDisabled: true} + r := SearchServiceResource{sku: "standard", enforceCmk: true} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.localAuthenticationDisabled(data), - Check: acceptance.ComposeTestCheckFunc( - check.That(data.ResourceName).ExistsInAzure(r), - ), + Config: r.cmkEnforcement(data), + Check: acceptance.ComposeTestCheckFunc(), }, data.ImportStep(), + }) +} + +func TestAccSearchService_cmkEnforcementUpdate(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "standard", enforceCmk: false} + u := SearchServiceResource{sku: "standard", enforceCmk: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: u.localAuthenticationDisabled(data), - Check: acceptance.ComposeTestCheckFunc( - check.That(data.ResourceName).ExistsInAzure(r), - ), + // This should create a Search Service with the default 'cmk_enforcement_enabled' value of 'false'... + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc(), }, data.ImportStep(), { - Config: r.localAuthenticationDisabled(data), - Check: acceptance.ComposeTestCheckFunc( - check.That(data.ResourceName).ExistsInAzure(r), - ), + Config: u.cmkEnforcement(data), + Check: acceptance.ComposeTestCheckFunc(), + }, + data.ImportStep(), + { + Config: r.cmkEnforcement(data), + Check: acceptance.ComposeTestCheckFunc(), }, data.ImportStep(), }) @@ -359,7 +364,7 @@ resource "azurerm_search_service" "test" { partition_count = 3 public_network_access_enabled = false - local_authentication_disabled = false + cmk_enforcement_enabled = false tags = { environment = "Production" @@ -442,7 +447,7 @@ resource "azurerm_search_service" "test" { `, template, data.RandomInteger, r.sku) } -func (r SearchServiceResource) hostingModeInvalidSKU(data acceptance.TestData) string { +func (r SearchServiceResource) partitionCount(data acceptance.TestData) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -456,16 +461,12 @@ resource "azurerm_search_service" "test" { resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location sku = "%s" - hosting_mode = "highDensity" - - tags = { - environment = "staging" - } + partition_count = %d } -`, template, data.RandomInteger, r.sku) +`, template, data.RandomInteger, r.sku, r.count) } -func (r SearchServiceResource) partitionCountInvalid(data acceptance.TestData) string { +func (r SearchServiceResource) replicaCount(data acceptance.TestData) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -479,31 +480,12 @@ resource "azurerm_search_service" "test" { resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location sku = "%s" - partition_count = 3 -} -`, template, data.RandomInteger, r.sku) + replica_count = %d } - -func (r SearchServiceResource) replicaCountInvalid(data acceptance.TestData) string { - template := r.template(data) - return fmt.Sprintf(` -provider "azurerm" { - features {} -} - -%s - -resource "azurerm_search_service" "test" { - name = "acctestsearchservice%d" - resource_group_name = azurerm_resource_group.test.name - location = azurerm_resource_group.test.location - sku = "%s" - replica_count = 2 -} -`, template, data.RandomInteger, r.sku) +`, template, data.RandomInteger, r.sku, r.count) } -func (r SearchServiceResource) localAuthenticationDisabled(data acceptance.TestData) string { +func (r SearchServiceResource) cmkEnforcement(data acceptance.TestData) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -518,11 +500,11 @@ resource "azurerm_search_service" "test" { location = azurerm_resource_group.test.location sku = "%s" - local_authentication_disabled = %t + cmk_enforcement_enabled = %t tags = { environment = "staging" } } -`, template, data.RandomInteger, r.sku, r.localAuthDisabled) +`, template, data.RandomInteger, r.sku, r.enforceCmk) } diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index d0ecd415c80b..ac228eae7575 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -19,7 +19,7 @@ resource "azurerm_resource_group" "example" { } resource "azurerm_search_service" "example" { - name = "example-search-service" + name = "example-resource" resource_group_name = azurerm_resource_group.example.name location = azurerm_resource_group.example.location sku = "standard" @@ -44,23 +44,27 @@ The following arguments are supported: --- +* `cmk_enforcement_enabled` - (Optional) Should the Search Service enforce having one or more non customer encrypted resources? Possible values include `true` or `false`. Defaults to `false`. + * `hosting_mode` - (Optional) Enable high density partitions that allow for up to a 1000 indexes. Possible values are `highDensity` or `default`. Defaults to `default`. Changing this forces a new Search Service to be created. -> **NOTE:** When the Search Service is in `highDensity` mode the maximum number of partitions allowed is `3`, to enable `hosting_mode` you must use a `standard3` SKU. -* `local_authentication_disabled` - (Optional) Should calls to the search service be allowed to utilize API keys for authentication? When `local_authentication_disabled` is set to `true`, calls to the search service *will not* be allowed to use API keys for authentication. Possible values include `true` or `false`. Defaults to `false`. + * `public_network_access_enabled` - (Optional) Whether or not public network access is allowed for this resource. Defaults to `true`. * `partition_count` - (Optional) The number of partitions which should be created. Possible values include `1`, `2`, `3`, `4`, `6`, or `12`. Defaults to `1`. +-> **NOTE:** `partition_count` cannot be configured when using a `free` or `basic` SKU. For more information please to the [product documentation](https://learn.microsoft.com/azure/search/search-sku-tier). + * `replica_count` - (Optional) The number of replica's which should be created. --> **NOTE:** `replica_count` cannot be configured when using a `free` SKU, `partition_count` cannot be configured when using a `free` or `basic` SKU. For more information please to the [product documentation](https://learn.microsoft.com/azure/search/search-sku-tier). +-> **NOTE:** `replica_count` cannot be configured when using a `free` SKU. For more information please to the [product documentation](https://learn.microsoft.com/azure/search/search-sku-tier). -* `allowed_ips` - (Optional) A list of IPv4 addresses or CIDRs that are allowed access to the search service endpoint. +* `allowed_ips` - (Optional) A list of IPv4 addresses or CIDRs that are allowed access to the Search Service endpoint. * `identity` - (Optional) An `identity` block as defined below. @@ -78,6 +82,8 @@ In addition to the Arguments listed above - the following Attributes are exporte * `id` - The ID of the Search Service. +* `cmk_enforcement_compliance` - Describes whether the Search Service is `compliant` or not with respect to having non customer encrypted resources. If a Search Service has more than one non customer encrypted resource and the `cmk_enforcement_enabled` field is set to `true` the Search Service will be marked as `nonCompliant`. + * `primary_key` - The Primary Key used for Search Service Administration. * `query_keys` - A `query_keys` block as defined below. From 79ee3e859bb59184be0b1b1ff6f45e7bf7d21b5b Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Sun, 9 Apr 2023 04:25:19 -0600 Subject: [PATCH 05/17] Update docs... --- website/docs/r/search_service.html.markdown | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index ac228eae7575..f7e0a3e27b88 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -64,7 +64,9 @@ The following arguments are supported: -> **NOTE:** `replica_count` cannot be configured when using a `free` SKU. For more information please to the [product documentation](https://learn.microsoft.com/azure/search/search-sku-tier). -* `allowed_ips` - (Optional) A list of IPv4 addresses or CIDRs that are allowed access to the Search Service endpoint. +* `allowed_ips` - (Optional) A list of inbound IPv4 or CIDRs that are allowed to access the Search Service. If the incoming IP request is from an IP address which is not included in the `allowed_ips` it will be blocked by the Search Services firewall. + +-> **NOTE:** The `allowed_ips` are only applied if the `public_network_access_enabled` field has been set to `true`, else all traffic over the public interface will be rejected, even if the `allowed_ips` field has been defined. * `identity` - (Optional) An `identity` block as defined below. From ef6c1c15b49a0f965b589a96cc353597db05323d Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Sun, 9 Apr 2023 04:33:02 -0600 Subject: [PATCH 06/17] Update Docs... --- website/docs/r/search_service.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index f7e0a3e27b88..280f3e838245 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -66,7 +66,7 @@ The following arguments are supported: * `allowed_ips` - (Optional) A list of inbound IPv4 or CIDRs that are allowed to access the Search Service. If the incoming IP request is from an IP address which is not included in the `allowed_ips` it will be blocked by the Search Services firewall. --> **NOTE:** The `allowed_ips` are only applied if the `public_network_access_enabled` field has been set to `true`, else all traffic over the public interface will be rejected, even if the `allowed_ips` field has been defined. +-> **NOTE:** The `allowed_ips` are only applied if the `public_network_access_enabled` field has been set to `true`, else all traffic over the public interface will be rejected, even if the `allowed_ips` field has been defined. When the `public_network_access_enabled` field has been set to `false` the private endpoint connections are the only allowed access point to the Search Service. * `identity` - (Optional) An `identity` block as defined below. From 6563bc1a0299715b1c485ad8d045a155455ca6fd Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Wed, 12 Apr 2023 04:55:07 -0600 Subject: [PATCH 07/17] Check-in progress... --- .../search/search_service_resource.go | 301 +++++++++++------- .../search/search_service_resource_test.go | 1 + 2 files changed, 182 insertions(+), 120 deletions(-) diff --git a/internal/services/search/search_service_resource.go b/internal/services/search/search_service_resource.go index 209da844d43e..58621dc624d5 100644 --- a/internal/services/search/search_service_resource.go +++ b/internal/services/search/search_service_resource.go @@ -85,34 +85,34 @@ func resourceSearchService() *pluginsdk.Resource { ValidateFunc: validateSearch.PartitionCount, }, - // "api_access_control": { - // Type: pluginsdk.TypeSet, - // Optional: true, - // MaxItems: 1, - // Elem: &pluginsdk.Resource{ - // Schema: map[string]*pluginsdk.Schema{ - // "type": { - // Type: pluginsdk.TypeString, - // Optional: true, - // Default: "api_keys", - // ValidateFunc: validation.StringInSlice([]string{ - // "api_keys", - // "role_based_access_control", - // "role_based_access_control_and_api_keys", - // }, false), - // }, - - // "authentication_failure_mode": { - // Type: pluginsdk.TypeString, - // Optional: true, - // ValidateFunc: validation.StringInSlice([]string{ - // string(services.AadAuthFailureModeHTTPFourZeroOneWithBearerChallenge), - // string(services.AadAuthFailureModeHTTPFourZeroThree), - // }, false), - // }, - // }, - // }, - // }, + "api_access_control": { + Type: pluginsdk.TypeSet, + Optional: true, + MaxItems: 1, + Elem: &pluginsdk.Resource{ + Schema: map[string]*pluginsdk.Schema{ + "type": { + Type: pluginsdk.TypeString, + Optional: true, + Default: "api_keys", + ValidateFunc: validation.StringInSlice([]string{ + "api_keys", + "role_based_access_control", + "role_based_access_control_and_api_keys", + }, false), + }, + + "authentication_failure_mode": { + Type: pluginsdk.TypeString, + Optional: true, + ValidateFunc: validation.StringInSlice([]string{ + string(services.AadAuthFailureModeHTTPFourZeroOneWithBearerChallenge), + string(services.AadAuthFailureModeHTTPFourZeroThree), + }, false), + }, + }, + }, + }, "hosting_mode": { Type: pluginsdk.TypeString, @@ -217,15 +217,8 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er ipRulesRaw := d.Get("allowed_ips").(*pluginsdk.Set).List() hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) cmkEnforcementEnabled := d.Get("cmk_enforcement_enabled").(bool) - - // NOTE: Temporarily disable these fields... - // AuthenticationOptions := expandSearchServiceDataPlaneAadOrApiKeyAuthOption(d.Get("api_access_control").(*pluginsdk.Set).List()) - - // localAuthDisabled := false - // if AuthenticationOptions == nil { - // // This means that it is RBAC only - // localAuthDisabled = true - // } + apiAccessControl := d.Get("api_access_control").(*pluginsdk.Set).List() + localAuthDisabled := false cmkEnforcement := services.SearchEncryptionWithCmkDisabled if cmkEnforcementEnabled { @@ -271,8 +264,7 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er Enforcement: pointer.To(cmkEnforcement), }), HostingMode: pointer.To(hostingMode), - // NOTE: Temporarily disable these fields... - // AuthOptions: AuthenticationOptions, + // AuthOptions: authenticationOptions, // DisableLocalAuth: pointer.To(localAuthDisabled), PartitionCount: pointer.To(partitionCount), ReplicaCount: pointer.To(replicaCount), @@ -280,12 +272,6 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er Tags: tags.Expand(d.Get("tags").(map[string]interface{})), } - // NOTE: Temporarily disable these fields... - // AuthOptions must be null if DisableLocalAuth is true - // if localAuthDisabled { - // searchService.Properties.AuthOptions = nil - // } - expandedIdentity, err := identity.ExpandSystemAssigned(d.Get("identity").([]interface{})) if err != nil { return fmt.Errorf("expanding `identity`: %+v", err) @@ -298,12 +284,28 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er searchService.Identity = expandedIdentity } + if len(apiAccessControl) > 0 { + authenticationOptions, err := expandSearchServiceDataPlaneAadOrApiKeyAuthOption(apiAccessControl) + if err != nil { + return err + } + + // This means that it is RBAC only + if authenticationOptions == nil { + localAuthDisabled = true + } + + searchService.Properties.AuthOptions = authenticationOptions + searchService.Properties.DisableLocalAuth = pointer.To(localAuthDisabled) + } + err = client.CreateOrUpdateThenPoll(ctx, id, searchService, services.CreateOrUpdateOperationOptions{}) if err != nil { return fmt.Errorf("creating %s: %+v", id, err) } d.SetId(id.ID()) + return resourceSearchServiceRead(d, meta) } @@ -360,18 +362,24 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er model.Properties.EncryptionWithCmk.Enforcement = pointer.To(cmkEnforcement) } - // NOTE: Temporarily disable these fields... - // if d.HasChange("api_access_control") { - // authenticationOptions := expandSearchServiceDataPlaneAadOrApiKeyAuthOption(d.Get("api_access_control").(*pluginsdk.Set).List()) + apiAccessControl := d.Get("api_access_control").(*pluginsdk.Set).List() - // model.Properties.DisableLocalAuth = pointer.To(false) - // if authenticationOptions == nil { - // // This means that it is RBAC only - // model.Properties.DisableLocalAuth = pointer.To(true) - // } + if d.HasChange("api_access_control") { + authenticationOptions, err := expandSearchServiceDataPlaneAadOrApiKeyAuthOption(apiAccessControl) + if err != nil { + return err + } + + model.Properties.DisableLocalAuth = pointer.To(false) - // model.Properties.AuthOptions = authenticationOptions - // } + // it looks like from my debugging if this is removed it needs to be a pointer to a []interface{map[]} + if authenticationOptions == nil { + // This means that it is RBAC only + model.Properties.DisableLocalAuth = pointer.To(true) + } + + model.Properties.AuthOptions = authenticationOptions + } if d.HasChange("replica_count") { replicaCount, err := validateSearchServiceReplicaCount(int64(d.Get("replica_count").(int)), pointer.From(model.Sku.Name)) @@ -412,6 +420,12 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er return fmt.Errorf("updating %s: %+v", id, err) } + // If you remove this value from your config you need to set this back to the 'default' value in Azure(e.g. 'api_keys') + // in the expand func, but you also need to remove this code block from your state file else you will get a diff... + if len(apiAccessControl) == 0 { + d.Set("api_access_control", apiAccessControl) + } + return resourceSearchServiceRead(d, meta) } @@ -457,7 +471,6 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro publicNetworkAccess := true // publicNetworkAccess defaults to true... cmkEnforcement := false // cmkEnforcment defaults to false... hostingMode := services.HostingModeDefault - // localAthDisabled := false if count := props.PartitionCount; count != nil { partitionCount = int(pointer.From(count)) @@ -483,12 +496,51 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro cmkCompliance = string(pointer.From(props.EncryptionWithCmk.EncryptionComplianceStatus)) } - // NOTE: Temporarily disable these fields... + // If the 'authenticationOptions' are not in the config file + // we need to remove them from the state file... + authenticationOptions := make([]interface{}, 0) + + // ************************************************************************************************************************************** + // I cannot trust props.DisableLocalAuth here, since the service will automatically set a value if it was omitted from the create call... + // ************************************************************************************************************************************** + + // I am using 'DisableLocalAuth' here because when you set your 'api_access_control' + // to 'RBAC only', the 'props.AuthOptions' will be 'nil'... + + if props.DisableLocalAuth != nil { + // since I cannot trust the values coming back from Azure, due to their default values, I will have to pull the values from state... + o, n := d.GetChange("api_access_control") + newValue := n.(*pluginsdk.Set).List() + oldValue := o.(*pluginsdk.Set).List() + + if len(newValue) > 0 { + authenticationOptions = flattenSearchServiceDataPlaneAadOrApiKeyAuthOption(props.AuthOptions, props.DisableLocalAuth) + } + + log.Println("************************************************************************") + log.Println("resourceSearchServiceRead:") + log.Println("************************************************************************") + log.Printf(" 'api_access_control' New Value : %+v\n", newValue) + log.Printf(" 'api_access_control' Old Value : %+v\n", oldValue) + log.Printf(" Flattened 'authenticationOptions' value: %+v\n", authenticationOptions) + log.Printf(" Azure RP value : %+v\n", props.AuthOptions) + log.Printf(" Azure RP 'ApiKeyOnly' value : %+v\n", pointer.From(props.AuthOptions.ApiKeyOnly)) + log.Println("************************************************************************") + } + // if props.DisableLocalAuth != nil { - // authenticationOptions := flattenSearchServiceDataPlaneAadOrApiKeyAuthOption(props.AuthOptions, props.DisableLocalAuth) - // d.Set("api_access_control", authenticationOptions) + // o, _ := d.GetChange("api_access_control") + // oldValue := o.([]interface{}) + + // log.Printf("\n\n\n\n\n\n\n\n\n\n******************************\nOld Value: %+v\n******************************\n\n\n\n\n\n\n\n", oldValue) + + // // Only call flatten if the value exists in state... + // if len(oldValue) > 0 { + // authenticationOptions = flattenSearchServiceDataPlaneAadOrApiKeyAuthOption(props.AuthOptions, props.DisableLocalAuth) + // } // } + d.Set("api_access_control", authenticationOptions) d.Set("partition_count", partitionCount) d.Set("replica_count", replicaCount) d.Set("public_network_access_enabled", publicNetworkAccess) @@ -592,38 +644,48 @@ func expandSearchServiceIPRules(input []interface{}) *[]services.IPRule { return &output } -// NOTE: Temporarily disable these fields... -// func expandSearchServiceDataPlaneAadOrApiKeyAuthOption(input []interface{}) *services.DataPlaneAuthOptions { -// var ApiKeyOnly interface{} - -// // the default(e.g. 'ApiKeyOnly'), only requires an empty DataPlaneAuthOptions.ApiKeyOnly interface... -// defaultAuth := pointer.To(services.DataPlaneAuthOptions{ -// ApiKeyOnly: pointer.To(ApiKeyOnly), -// }) - -// if len(input) == 0 { -// return defaultAuth -// } - -// apiAccessControl := input[0].(map[string]interface{}) -// accessControlType := apiAccessControl["type"].(string) -// authFailureMode := apiAccessControl["authentication_failure_mode"].(string) - -// switch accessControlType { -// case "role_based_access_control": -// return nil -// case "api_keys": -// return defaultAuth -// case "role_based_access_control_and_api_keys": -// return pointer.To(services.DataPlaneAuthOptions{ -// AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ -// AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authFailureMode)), -// }), -// }) -// } - -// return defaultAuth -// } +func expandSearchServiceDataPlaneAadOrApiKeyAuthOption(input []interface{}) (*services.DataPlaneAuthOptions, error) { + var foo interface{} + apiKeyOnlyDefault := make(map[string]interface{}, 0) + foo = apiKeyOnlyDefault + + // the default(e.g. 'ApiKeyOnly'), only requires an empty 'DataPlaneAuthOptions.ApiKeyOnly' interface which must be an empty map... + // Azure RP 'ApiKeyOnly' value : map[] + defaultAuthOptions := pointer.To(services.DataPlaneAuthOptions{ + ApiKeyOnly: pointer.To(foo), + }) + + log.Println("************************************************************************") + log.Println("expandSearchServiceDataPlaneAadOrApiKeyAuthOption:") + log.Println("************************************************************************") + log.Printf(" 'apiKeyOnlyDefault' : %+v\n", apiKeyOnlyDefault) + log.Printf(" 'apiKeyOnly' : %+v\n", foo) + log.Printf(" 'defaultAuthOptions' : %+v\n", defaultAuthOptions) + log.Println("************************************************************************") + + if len(input) == 0 { + return defaultAuthOptions, nil + } + + apiAccessControl := input[0].(map[string]interface{}) + accessControlType := apiAccessControl["type"].(string) + authFailureMode := apiAccessControl["authentication_failure_mode"].(string) + + switch accessControlType { + case "role_based_access_control": + return nil, nil + case "api_keys": + return defaultAuthOptions, nil + case "role_based_access_control_and_api_keys": + return pointer.To(services.DataPlaneAuthOptions{ + AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ + AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authFailureMode)), + }), + }), nil + } + + return defaultAuthOptions, nil +} func flattenSearchServiceIPRules(input *services.NetworkRuleSet) []interface{} { if input == nil || *input.IPRules == nil || len(*input.IPRules) == 0 { @@ -636,35 +698,34 @@ func flattenSearchServiceIPRules(input *services.NetworkRuleSet) []interface{} { return result } -// NOTE: Temporarily disable these fields... -// func flattenSearchServiceDataPlaneAadOrApiKeyAuthOption(input *services.DataPlaneAuthOptions, localAuthenticationDisabled *bool) []interface{} { -// // TODO: Validate what I should be checking here... -// // For RBAC Only DataPlaneAuthOptions will be nil... -// if localAuthenticationDisabled == nil { -// return []interface{}{} -// } - -// result := make(map[string]interface{}) - -// // if localAuthenticationDisabled is disabled that means the this in RBAC Only mode... -// if pointer.From(localAuthenticationDisabled) { -// // Auth is RBAC Only... -// // "localAuthenticationDisabled": true -// result["type"] = "role_based_access_control" -// result["authentication_failure_mode"] = "" -// } else { -// // Auth can be API Only or RBAC and API... -// if input.AadOrApiKey != nil && input.AadOrApiKey.AadAuthFailureMode != nil { -// result["type"] = "role_based_access_control_and_api_keys" -// result["authentication_failure_mode"] = pointer.From(input.AadOrApiKey.AadAuthFailureMode) -// } else { -// result["type"] = "api_keys" -// result["authentication_failure_mode"] = "" -// } -// } - -// return []interface{}{result} -// } +func flattenSearchServiceDataPlaneAadOrApiKeyAuthOption(input *services.DataPlaneAuthOptions, localAuthenticationDisabled *bool) []interface{} { + // TODO: Validate what I should be checking here... + // For RBAC Only DataPlaneAuthOptions will be nil... + if localAuthenticationDisabled == nil { + return []interface{}{} + } + + result := make(map[string]interface{}) + + // if 'localAuthenticationDisabled' is 'true' that means the this in RBAC Only mode... + if pointer.From(localAuthenticationDisabled) { + // Auth is RBAC Only... + // "localAuthenticationDisabled": true + result["type"] = "role_based_access_control" + result["authentication_failure_mode"] = "" + } else { + // Auth can be API Only or RBAC and API... + if input.AadOrApiKey != nil && input.AadOrApiKey.AadAuthFailureMode != nil { + result["type"] = "role_based_access_control_and_api_keys" + result["authentication_failure_mode"] = pointer.From(input.AadOrApiKey.AadAuthFailureMode) + } else { + result["type"] = "api_keys" + result["authentication_failure_mode"] = "" + } + } + + return []interface{}{result} +} func validateSearchServiceReplicaCount(replicaCount int64, skuName services.SkuName) (int64, error) { switch skuName { diff --git a/internal/services/search/search_service_resource_test.go b/internal/services/search/search_service_resource_test.go index 806efdee621a..3a764f54b4a3 100644 --- a/internal/services/search/search_service_resource_test.go +++ b/internal/services/search/search_service_resource_test.go @@ -36,6 +36,7 @@ func TestAccSearchService_basicStandard(t *testing.T) { } func TestAccSearchService_basicFree(t *testing.T) { + // Regression test case for issue #10151 data := acceptance.BuildTestData(t, "azurerm_search_service", "test") r := SearchServiceResource{sku: "free"} From a201b59af0532fa811b2c032d47e0ad0f71d3f92 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Wed, 12 Apr 2023 05:39:17 -0600 Subject: [PATCH 08/17] Update functions names... --- .../services/search/search_service_resource.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/services/search/search_service_resource.go b/internal/services/search/search_service_resource.go index 58621dc624d5..8b23079d9647 100644 --- a/internal/services/search/search_service_resource.go +++ b/internal/services/search/search_service_resource.go @@ -285,7 +285,7 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er } if len(apiAccessControl) > 0 { - authenticationOptions, err := expandSearchServiceDataPlaneAadOrApiKeyAuthOption(apiAccessControl) + authenticationOptions, err := expandSearchServiceAuthOptions(apiAccessControl) if err != nil { return err } @@ -365,7 +365,7 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er apiAccessControl := d.Get("api_access_control").(*pluginsdk.Set).List() if d.HasChange("api_access_control") { - authenticationOptions, err := expandSearchServiceDataPlaneAadOrApiKeyAuthOption(apiAccessControl) + authenticationOptions, err := expandSearchServiceAuthOptions(apiAccessControl) if err != nil { return err } @@ -514,7 +514,7 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro oldValue := o.(*pluginsdk.Set).List() if len(newValue) > 0 { - authenticationOptions = flattenSearchServiceDataPlaneAadOrApiKeyAuthOption(props.AuthOptions, props.DisableLocalAuth) + authenticationOptions = flattenSearchServiceDataPlaneAuthOptions(props.AuthOptions, props.DisableLocalAuth) } log.Println("************************************************************************") @@ -644,13 +644,13 @@ func expandSearchServiceIPRules(input []interface{}) *[]services.IPRule { return &output } -func expandSearchServiceDataPlaneAadOrApiKeyAuthOption(input []interface{}) (*services.DataPlaneAuthOptions, error) { +func expandSearchServiceAuthOptions(input []interface{}) (*services.DataPlaneAuthOptions, error) { var foo interface{} apiKeyOnlyDefault := make(map[string]interface{}, 0) foo = apiKeyOnlyDefault - // the default(e.g. 'ApiKeyOnly'), only requires an empty 'DataPlaneAuthOptions.ApiKeyOnly' interface which must be an empty map... - // Azure RP 'ApiKeyOnly' value : map[] + // the default(e.g. 'ApiKeyOnly'), only requires an empty 'DataPlaneAuthOptions.ApiKeyOnly' + // interface which must be an empty map... defaultAuthOptions := pointer.To(services.DataPlaneAuthOptions{ ApiKeyOnly: pointer.To(foo), }) @@ -698,7 +698,7 @@ func flattenSearchServiceIPRules(input *services.NetworkRuleSet) []interface{} { return result } -func flattenSearchServiceDataPlaneAadOrApiKeyAuthOption(input *services.DataPlaneAuthOptions, localAuthenticationDisabled *bool) []interface{} { +func flattenSearchServiceDataPlaneAuthOptions(input *services.DataPlaneAuthOptions, localAuthenticationDisabled *bool) []interface{} { // TODO: Validate what I should be checking here... // For RBAC Only DataPlaneAuthOptions will be nil... if localAuthenticationDisabled == nil { From ef256965db92c52108ae1a2115edc9b6a0964a63 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Thu, 13 Apr 2023 02:45:33 -0600 Subject: [PATCH 09/17] Check-in progress... --- .../search/search_service_resource.go | 160 ++++++------------ .../search/search_service_resource_test.go | 131 +++++++++++++- website/docs/r/search_service.html.markdown | 24 ++- 3 files changed, 198 insertions(+), 117 deletions(-) diff --git a/internal/services/search/search_service_resource.go b/internal/services/search/search_service_resource.go index 8b23079d9647..e0dc48407386 100644 --- a/internal/services/search/search_service_resource.go +++ b/internal/services/search/search_service_resource.go @@ -88,6 +88,7 @@ func resourceSearchService() *pluginsdk.Resource { "api_access_control": { Type: pluginsdk.TypeSet, Optional: true, + Computed: true, MaxItems: 1, Elem: &pluginsdk.Resource{ Schema: map[string]*pluginsdk.Schema{ @@ -105,9 +106,11 @@ func resourceSearchService() *pluginsdk.Resource { "authentication_failure_mode": { Type: pluginsdk.TypeString, Optional: true, + Default: "", ValidateFunc: validation.StringInSlice([]string{ string(services.AadAuthFailureModeHTTPFourZeroOneWithBearerChallenge), string(services.AadAuthFailureModeHTTPFourZeroThree), + "", }, false), }, }, @@ -218,7 +221,6 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) cmkEnforcementEnabled := d.Get("cmk_enforcement_enabled").(bool) apiAccessControl := d.Get("api_access_control").(*pluginsdk.Set).List() - localAuthDisabled := false cmkEnforcement := services.SearchEncryptionWithCmkDisabled if cmkEnforcementEnabled { @@ -250,6 +252,17 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er return err } + authenticationOptions, err := expandSearchServiceAuthOptions(apiAccessControl) + if err != nil { + return fmt.Errorf("expanding 'api_access_control': %+v", err) + } + + // If 'authenticationOptions' are nil it is RBAC only mode... + localAuthDisabled := false + if authenticationOptions == nil { + localAuthDisabled = true + } + searchService := services.SearchService{ Location: location, Sku: pointer.To(services.Sku{ @@ -263,11 +276,11 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er EncryptionWithCmk: pointer.To(services.EncryptionWithCmk{ Enforcement: pointer.To(cmkEnforcement), }), - HostingMode: pointer.To(hostingMode), - // AuthOptions: authenticationOptions, - // DisableLocalAuth: pointer.To(localAuthDisabled), - PartitionCount: pointer.To(partitionCount), - ReplicaCount: pointer.To(replicaCount), + HostingMode: pointer.To(hostingMode), + AuthOptions: authenticationOptions, + DisableLocalAuth: pointer.To(localAuthDisabled), + PartitionCount: pointer.To(partitionCount), + ReplicaCount: pointer.To(replicaCount), }, Tags: tags.Expand(d.Get("tags").(map[string]interface{})), } @@ -284,21 +297,6 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er searchService.Identity = expandedIdentity } - if len(apiAccessControl) > 0 { - authenticationOptions, err := expandSearchServiceAuthOptions(apiAccessControl) - if err != nil { - return err - } - - // This means that it is RBAC only - if authenticationOptions == nil { - localAuthDisabled = true - } - - searchService.Properties.AuthOptions = authenticationOptions - searchService.Properties.DisableLocalAuth = pointer.To(localAuthDisabled) - } - err = client.CreateOrUpdateThenPoll(ctx, id, searchService, services.CreateOrUpdateOperationOptions{}) if err != nil { return fmt.Errorf("creating %s: %+v", id, err) @@ -367,17 +365,16 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er if d.HasChange("api_access_control") { authenticationOptions, err := expandSearchServiceAuthOptions(apiAccessControl) if err != nil { - return err + return fmt.Errorf("expanding 'api_access_control': %+v", err) } - model.Properties.DisableLocalAuth = pointer.To(false) - - // it looks like from my debugging if this is removed it needs to be a pointer to a []interface{map[]} + // If 'authenticationOptions' are nil it is RBAC only mode... + disableLocalAuth := false if authenticationOptions == nil { - // This means that it is RBAC only - model.Properties.DisableLocalAuth = pointer.To(true) + disableLocalAuth = true } + model.Properties.DisableLocalAuth = pointer.To(disableLocalAuth) model.Properties.AuthOptions = authenticationOptions } @@ -471,6 +468,7 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro publicNetworkAccess := true // publicNetworkAccess defaults to true... cmkEnforcement := false // cmkEnforcment defaults to false... hostingMode := services.HostingModeDefault + authenticationOptions := make([]interface{}, 0) if count := props.PartitionCount; count != nil { partitionCount = int(pointer.From(count)) @@ -496,50 +494,12 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro cmkCompliance = string(pointer.From(props.EncryptionWithCmk.EncryptionComplianceStatus)) } - // If the 'authenticationOptions' are not in the config file - // we need to remove them from the state file... - authenticationOptions := make([]interface{}, 0) - - // ************************************************************************************************************************************** - // I cannot trust props.DisableLocalAuth here, since the service will automatically set a value if it was omitted from the create call... - // ************************************************************************************************************************************** - // I am using 'DisableLocalAuth' here because when you set your 'api_access_control' // to 'RBAC only', the 'props.AuthOptions' will be 'nil'... - if props.DisableLocalAuth != nil { - // since I cannot trust the values coming back from Azure, due to their default values, I will have to pull the values from state... - o, n := d.GetChange("api_access_control") - newValue := n.(*pluginsdk.Set).List() - oldValue := o.(*pluginsdk.Set).List() - - if len(newValue) > 0 { - authenticationOptions = flattenSearchServiceDataPlaneAuthOptions(props.AuthOptions, props.DisableLocalAuth) - } - - log.Println("************************************************************************") - log.Println("resourceSearchServiceRead:") - log.Println("************************************************************************") - log.Printf(" 'api_access_control' New Value : %+v\n", newValue) - log.Printf(" 'api_access_control' Old Value : %+v\n", oldValue) - log.Printf(" Flattened 'authenticationOptions' value: %+v\n", authenticationOptions) - log.Printf(" Azure RP value : %+v\n", props.AuthOptions) - log.Printf(" Azure RP 'ApiKeyOnly' value : %+v\n", pointer.From(props.AuthOptions.ApiKeyOnly)) - log.Println("************************************************************************") + authenticationOptions = flattenSearchServiceDataPlaneAuthOptions(props.AuthOptions, props.DisableLocalAuth) } - // if props.DisableLocalAuth != nil { - // o, _ := d.GetChange("api_access_control") - // oldValue := o.([]interface{}) - - // log.Printf("\n\n\n\n\n\n\n\n\n\n******************************\nOld Value: %+v\n******************************\n\n\n\n\n\n\n\n", oldValue) - - // // Only call flatten if the value exists in state... - // if len(oldValue) > 0 { - // authenticationOptions = flattenSearchServiceDataPlaneAadOrApiKeyAuthOption(props.AuthOptions, props.DisableLocalAuth) - // } - // } - d.Set("api_access_control", authenticationOptions) d.Set("partition_count", partitionCount) d.Set("replica_count", replicaCount) @@ -645,43 +605,39 @@ func expandSearchServiceIPRules(input []interface{}) *[]services.IPRule { } func expandSearchServiceAuthOptions(input []interface{}) (*services.DataPlaneAuthOptions, error) { - var foo interface{} - apiKeyOnlyDefault := make(map[string]interface{}, 0) - foo = apiKeyOnlyDefault - // the default(e.g. 'ApiKeyOnly'), only requires an empty 'DataPlaneAuthOptions.ApiKeyOnly' - // interface which must be an empty map... + // 'interface{}' which must have an embedded empty 'map[string]interface{}'... + var apiKeyOnlyDefault interface{} = make(map[string]interface{}, 0) + defaultAuthOptions := pointer.To(services.DataPlaneAuthOptions{ - ApiKeyOnly: pointer.To(foo), + ApiKeyOnly: pointer.To(apiKeyOnlyDefault), }) - log.Println("************************************************************************") - log.Println("expandSearchServiceDataPlaneAadOrApiKeyAuthOption:") - log.Println("************************************************************************") - log.Printf(" 'apiKeyOnlyDefault' : %+v\n", apiKeyOnlyDefault) - log.Printf(" 'apiKeyOnly' : %+v\n", foo) - log.Printf(" 'defaultAuthOptions' : %+v\n", defaultAuthOptions) - log.Println("************************************************************************") - - if len(input) == 0 { - return defaultAuthOptions, nil - } - - apiAccessControl := input[0].(map[string]interface{}) - accessControlType := apiAccessControl["type"].(string) - authFailureMode := apiAccessControl["authentication_failure_mode"].(string) - - switch accessControlType { - case "role_based_access_control": - return nil, nil - case "api_keys": - return defaultAuthOptions, nil - case "role_based_access_control_and_api_keys": - return pointer.To(services.DataPlaneAuthOptions{ - AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ - AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authFailureMode)), - }), - }), nil + if len(input) > 0 { + apiAccessControl := input[0].(map[string]interface{}) + accessControlType := apiAccessControl["type"].(string) + authFailureMode := apiAccessControl["authentication_failure_mode"].(string) + + if accessControlType != "role_based_access_control_and_api_keys" && authFailureMode != "" { + return nil, fmt.Errorf("it is invalid to define the 'authentication_failure_mode' when the 'api_access_control' 'type' is not set to 'role_based_access_control_and_api_keys', got 'api_access_control' 'type' %q", accessControlType) + } + + if accessControlType == "role_based_access_control_and_api_keys" && authFailureMode == "" { + return nil, fmt.Errorf("it is invalid to define the 'api_access_control' 'type' as 'role_based_access_control_and_api_keys' and not define the 'authentication_failure_mode', got 'authentication_failure_mode' %q", authFailureMode) + } + + switch accessControlType { + case "role_based_access_control": + return nil, nil + case "api_keys": + return defaultAuthOptions, nil + case "role_based_access_control_and_api_keys": + return pointer.To(services.DataPlaneAuthOptions{ + AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ + AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authFailureMode)), + }), + }), nil + } } return defaultAuthOptions, nil @@ -699,8 +655,6 @@ func flattenSearchServiceIPRules(input *services.NetworkRuleSet) []interface{} { } func flattenSearchServiceDataPlaneAuthOptions(input *services.DataPlaneAuthOptions, localAuthenticationDisabled *bool) []interface{} { - // TODO: Validate what I should be checking here... - // For RBAC Only DataPlaneAuthOptions will be nil... if localAuthenticationDisabled == nil { return []interface{}{} } @@ -709,12 +663,10 @@ func flattenSearchServiceDataPlaneAuthOptions(input *services.DataPlaneAuthOptio // if 'localAuthenticationDisabled' is 'true' that means the this in RBAC Only mode... if pointer.From(localAuthenticationDisabled) { - // Auth is RBAC Only... - // "localAuthenticationDisabled": true result["type"] = "role_based_access_control" result["authentication_failure_mode"] = "" } else { - // Auth can be API Only or RBAC and API... + // if 'localAuthenticationDisabled' is 'false' it can be API Keys Only or RBAC and API Keys Only... if input.AadOrApiKey != nil && input.AadOrApiKey.AadAuthFailureMode != nil { result["type"] = "role_based_access_control_and_api_keys" result["authentication_failure_mode"] = pointer.From(input.AadOrApiKey.AadAuthFailureMode) diff --git a/internal/services/search/search_service_resource_test.go b/internal/services/search/search_service_resource_test.go index 3a764f54b4a3..8ee51ec9d677 100644 --- a/internal/services/search/search_service_resource_test.go +++ b/internal/services/search/search_service_resource_test.go @@ -15,9 +15,11 @@ import ( ) type SearchServiceResource struct { - sku string - count int - enforceCmk bool + sku string + count int + enforceCmk bool + apiAccessControlType string + apiAccessControlAuthenticationFailureMode string } func TestAccSearchService_basicStandard(t *testing.T) { @@ -253,7 +255,9 @@ func TestAccSearchService_cmkEnforcement(t *testing.T) { data.ResourceTest(t, r, []acceptance.TestStep{ { Config: r.cmkEnforcement(data), - Check: acceptance.ComposeTestCheckFunc(), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), }, data.ImportStep(), }) @@ -268,17 +272,103 @@ func TestAccSearchService_cmkEnforcementUpdate(t *testing.T) { { // This should create a Search Service with the default 'cmk_enforcement_enabled' value of 'false'... Config: r.basic(data), - Check: acceptance.ComposeTestCheckFunc(), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), }, data.ImportStep(), { Config: u.cmkEnforcement(data), - Check: acceptance.ComposeTestCheckFunc(), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), }, data.ImportStep(), { Config: r.cmkEnforcement(data), - Check: acceptance.ComposeTestCheckFunc(), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccSearchService_apiAccessControlFailureModeWrongTypeError(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "standard", apiAccessControlType: "api_keys", apiAccessControlAuthenticationFailureMode: "http401WithBearerChallenge"} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.apiAccessControl(data), + Check: acceptance.ComposeTestCheckFunc(), + ExpectError: regexp.MustCompile(`is not set to 'role_based_access_control_and_api_keys'`), + }, + }) +} + +func TestAccSearchService_apiAccessControlRBACandAPIKeysNoFailureModeError(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "standard", apiAccessControlType: "role_based_access_control_and_api_keys"} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.apiAccessControl(data), + Check: acceptance.ComposeTestCheckFunc(), + ExpectError: regexp.MustCompile(`and not define the 'authentication_failure_mode'`), + }, + }) +} + +func TestAccSearchService_apiAccessControlUpdate(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{sku: "standard"} + a := SearchServiceResource{sku: "standard", apiAccessControlType: "api_keys"} + rbac := SearchServiceResource{sku: "standard", apiAccessControlType: "role_based_access_control"} + rbacApiFourZeroOne := SearchServiceResource{sku: "standard", apiAccessControlType: "role_based_access_control_and_api_keys", apiAccessControlAuthenticationFailureMode: "http401WithBearerChallenge"} + rbacApiFourZeroThree := SearchServiceResource{sku: "standard", apiAccessControlType: "role_based_access_control_and_api_keys", apiAccessControlAuthenticationFailureMode: "http403"} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: a.apiAccessControl(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: rbac.apiAccessControl(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: rbacApiFourZeroOne.apiAccessControl(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: rbacApiFourZeroThree.apiAccessControl(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), }, data.ImportStep(), }) @@ -509,3 +599,30 @@ resource "azurerm_search_service" "test" { } `, template, data.RandomInteger, r.sku, r.enforceCmk) } + +func (r SearchServiceResource) apiAccessControl(data acceptance.TestData) string { + template := r.template(data) + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%s + +resource "azurerm_search_service" "test" { + name = "acctestsearchservice%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + sku = "%s" + + api_access_control { + type = "%s" + authentication_failure_mode = "%s" + } + + tags = { + environment = "staging" + } +} +`, template, data.RandomInteger, r.sku, r.apiAccessControlType, r.apiAccessControlAuthenticationFailureMode) +} diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index 280f3e838245..23d13456e956 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -44,16 +44,14 @@ The following arguments are supported: --- -* `cmk_enforcement_enabled` - (Optional) Should the Search Service enforce having one or more non customer encrypted resources? Possible values include `true` or `false`. Defaults to `false`. +* `api_access_control` - (Optional) An `api_access_control` block as defined below. + +* `cmk_enforcement_enabled` - (Optional) Should the Search Service enforce having non customer encrypted resources? Possible values include `true` or `false`. If `true` the Search Service will be marked as `non-compliant` if there are one or more non customer encrypted resources, if `false` no enforcement will be made and the Search Service can contain one or more non customer encrypted resources. Defaults to `false`. * `hosting_mode` - (Optional) Enable high density partitions that allow for up to a 1000 indexes. Possible values are `highDensity` or `default`. Defaults to `default`. Changing this forces a new Search Service to be created. -> **NOTE:** When the Search Service is in `highDensity` mode the maximum number of partitions allowed is `3`, to enable `hosting_mode` you must use a `standard3` SKU. - - * `public_network_access_enabled` - (Optional) Whether or not public network access is allowed for this resource. Defaults to `true`. * `partition_count` - (Optional) The number of partitions which should be created. Possible values include `1`, `2`, `3`, `4`, `6`, or `12`. Defaults to `1`. @@ -74,10 +72,24 @@ The following arguments are supported: --- -A `identity` block supports the following: +An `api_access_control` block supports the following: + +* `type` - (Optional) Describes what authentication mode should be used for the Search Service. Possible values inclued `api_keys`, `role_based_access_control`, or `role_based_access_control_and_api_keys`. Defaults to `api_keys`. + +-> **NOTE:** If the `api_access_control` block has not been defined in the configuration file the Search Service will still be defaulted to use the `api_keys` authentication mode. + +* `authentication_failure_mode` - (Optional) Describes what response the Search Service should return for requests that fail authentication. Possible values include `http401WithBearerChallenge` or `http403`. Defaults to an `empty` string. + +-> **NOTE:** `authentication_failure_mode` can only be defined if the `api_access_control` `type` is set to `role_based_access_control_and_api_keys`. + +--- + +An `identity` block supports the following: * `type` - (Required) Specifies the type of Managed Service Identity that should be configured on this Search Service. The only possible value is `SystemAssigned`. +--- + ## Attributes Reference In addition to the Arguments listed above - the following Attributes are exported: From 0d09a5db14bb284c15ab3e37cb1b541a9b6681a8 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Thu, 13 Apr 2023 17:26:56 -0600 Subject: [PATCH 10/17] Address PR comments... --- .../search/search_service_resource.go | 235 ++++++++---------- .../search/search_service_resource_test.go | 199 ++++++++------- .../search_service_partition_count.go | 20 -- website/docs/r/search_service.html.markdown | 24 +- 4 files changed, 211 insertions(+), 267 deletions(-) delete mode 100644 internal/services/search/validate/search_service_partition_count.go diff --git a/internal/services/search/search_service_resource.go b/internal/services/search/search_service_resource.go index e0dc48407386..4c94ea2e6de4 100644 --- a/internal/services/search/search_service_resource.go +++ b/internal/services/search/search_service_resource.go @@ -19,7 +19,6 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/helpers/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" - validateSearch "github.com/hashicorp/terraform-provider-azurerm/internal/services/search/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation" "github.com/hashicorp/terraform-provider-azurerm/internal/timeouts" @@ -79,42 +78,32 @@ func resourceSearchService() *pluginsdk.Resource { }, "partition_count": { - Type: pluginsdk.TypeInt, - Optional: true, - Default: 1, - ValidateFunc: validateSearch.PartitionCount, + Type: pluginsdk.TypeInt, + Optional: true, + Default: 1, + ValidateFunc: validation.IntInSlice([]int{ + 1, + 2, + 3, + 4, + 6, + 12, + }), }, - "api_access_control": { - Type: pluginsdk.TypeSet, + "local_authentication_disabled": { + Type: pluginsdk.TypeBool, Optional: true, - Computed: true, - MaxItems: 1, - Elem: &pluginsdk.Resource{ - Schema: map[string]*pluginsdk.Schema{ - "type": { - Type: pluginsdk.TypeString, - Optional: true, - Default: "api_keys", - ValidateFunc: validation.StringInSlice([]string{ - "api_keys", - "role_based_access_control", - "role_based_access_control_and_api_keys", - }, false), - }, + Default: false, + }, - "authentication_failure_mode": { - Type: pluginsdk.TypeString, - Optional: true, - Default: "", - ValidateFunc: validation.StringInSlice([]string{ - string(services.AadAuthFailureModeHTTPFourZeroOneWithBearerChallenge), - string(services.AadAuthFailureModeHTTPFourZeroThree), - "", - }, false), - }, - }, - }, + "authentication_failure_mode": { + Type: pluginsdk.TypeString, + Optional: true, + ValidateFunc: validation.StringInSlice([]string{ + string(services.AadAuthFailureModeHTTPFourZeroOneWithBearerChallenge), + string(services.AadAuthFailureModeHTTPFourZeroThree), + }, false), }, "hosting_mode": { @@ -128,13 +117,13 @@ func resourceSearchService() *pluginsdk.Resource { }, false), }, - "cmk_enforcement_enabled": { + "customer_managed_key_enforcement_enabled": { Type: pluginsdk.TypeBool, Optional: true, Default: false, }, - "cmk_enforcement_compliance": { + "customer_managed_key_enforcement_compliance": { Type: pluginsdk.TypeString, Computed: true, }, @@ -216,11 +205,13 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er publicNetworkAccess = services.PublicNetworkAccessDisabled } + var apiKeyOnly interface{} = make(map[string]interface{}, 0) skuName := services.SkuName(d.Get("sku").(string)) ipRulesRaw := d.Get("allowed_ips").(*pluginsdk.Set).List() hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) - cmkEnforcementEnabled := d.Get("cmk_enforcement_enabled").(bool) - apiAccessControl := d.Get("api_access_control").(*pluginsdk.Set).List() + cmkEnforcementEnabled := d.Get("customer_managed_key_enforcement_enabled").(bool) + localAuthenticationDisabled := d.Get("local_authentication_disabled").(bool) + authenticationFailureMode := d.Get("authentication_failure_mode").(string) cmkEnforcement := services.SearchEncryptionWithCmkDisabled if cmkEnforcementEnabled { @@ -242,7 +233,7 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er // NOTE: 'standard3' services with 'hostingMode' set to 'highDensity' the // 'partition_count' must be between 1 and 3. if skuName == services.SkuNameStandardThree && partitionCount > 3 && hostingMode == services.HostingModeHighDensity { - return fmt.Errorf("'standard3' SKUs in 'highDensity' mode can have a maximum of 3 partitions, got %d", partitionCount) + return fmt.Errorf("%q SKUs in %q mode can have a maximum of 3 partitions, got %d", services.SkuNameStandardThree, services.HostingModeHighDensity, partitionCount) } // The number of replicas can be between 1 and 12 for 'standard', 'storage_optimized_l1' and storage_optimized_l2' SKUs @@ -252,15 +243,27 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er return err } - authenticationOptions, err := expandSearchServiceAuthOptions(apiAccessControl) - if err != nil { - return fmt.Errorf("expanding 'api_access_control': %+v", err) + if localAuthenticationDisabled && authenticationFailureMode != "" { + return fmt.Errorf("'authentication_failure_mode' cannot be defined if 'local_authentication_disabled' has been set to 'true'") + } + + // API Only Mode (Defalut)(e.g. localAuthenticationDisabled = false)... + authenticationOptions := pointer.To(services.DataPlaneAuthOptions{ + ApiKeyOnly: pointer.To(apiKeyOnly), + }) + + if !localAuthenticationDisabled && authenticationFailureMode != "" { + // API & RBAC Mode.. + authenticationOptions = pointer.To(services.DataPlaneAuthOptions{ + AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ + AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authenticationFailureMode)), + }), + }) } - // If 'authenticationOptions' are nil it is RBAC only mode... - localAuthDisabled := false - if authenticationOptions == nil { - localAuthDisabled = true + if localAuthenticationDisabled { + // RBAC Only Mode... + authenticationOptions = nil } searchService := services.SearchService{ @@ -278,7 +281,7 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er }), HostingMode: pointer.To(hostingMode), AuthOptions: authenticationOptions, - DisableLocalAuth: pointer.To(localAuthDisabled), + DisableLocalAuth: pointer.To(localAuthenticationDisabled), PartitionCount: pointer.To(partitionCount), ReplicaCount: pointer.To(replicaCount), }, @@ -338,43 +341,57 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er return fmt.Errorf("expanding `identity`: %+v", err) } - // NOTE: Passing type 'None' on update will remove all identities from the service. model.Identity = expandedIdentity } if d.HasChange("hosting_mode") { hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) if pointer.From(model.Sku.Name) != services.SkuNameStandardThree && hostingMode == services.HostingModeHighDensity { - return fmt.Errorf("'hosting_mode' can only be set to 'highDensity' if the 'sku' is 'standard3', got %q", pointer.From(model.Sku.Name)) + return fmt.Errorf("'hosting_mode' can only be set to %q if the 'sku' is %q, got %q", services.HostingModeHighDensity, services.SkuNameStandardThree, pointer.From(model.Sku.Name)) } model.Properties.HostingMode = pointer.To(hostingMode) } - if d.HasChange("cmk_enforcement_enabled") { + if d.HasChange("customer_managed_key_enforcement_enabled") { cmkEnforcement := services.SearchEncryptionWithCmkDisabled - if enabled := d.Get("cmk_enforcement_enabled").(bool); enabled { + if enabled := d.Get("customer_managed_key_enforcement_enabled").(bool); enabled { cmkEnforcement = services.SearchEncryptionWithCmkEnabled } model.Properties.EncryptionWithCmk.Enforcement = pointer.To(cmkEnforcement) } - apiAccessControl := d.Get("api_access_control").(*pluginsdk.Set).List() + if d.HasChange("local_authentication_disabled") || d.HasChange("authentication_failure_mode") { + localAuthenticationDisabled := d.Get("local_authentication_disabled").(bool) + authenticationFailureMode := d.Get("authentication_failure_mode").(string) - if d.HasChange("api_access_control") { - authenticationOptions, err := expandSearchServiceAuthOptions(apiAccessControl) - if err != nil { - return fmt.Errorf("expanding 'api_access_control': %+v", err) + if localAuthenticationDisabled && authenticationFailureMode != "" { + return fmt.Errorf("'authentication_failure_mode' cannot be defined if 'local_authentication_disabled' has been set to 'true'") } - // If 'authenticationOptions' are nil it is RBAC only mode... - disableLocalAuth := false - if authenticationOptions == nil { - disableLocalAuth = true + var apiKeyOnly interface{} = make(map[string]interface{}, 0) + + // API Only Mode (Defalut)... + authenticationOptions := pointer.To(services.DataPlaneAuthOptions{ + ApiKeyOnly: pointer.To(apiKeyOnly), + }) + + if !localAuthenticationDisabled && authenticationFailureMode != "" { + // API & RBAC Mode.. + authenticationOptions = pointer.To(services.DataPlaneAuthOptions{ + AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ + AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authenticationFailureMode)), + }), + }) } - model.Properties.DisableLocalAuth = pointer.To(disableLocalAuth) + if localAuthenticationDisabled { + // RBAC Only Mode... + authenticationOptions = nil + } + + model.Properties.DisableLocalAuth = pointer.To(localAuthenticationDisabled) model.Properties.AuthOptions = authenticationOptions } @@ -397,7 +414,7 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er // NOTE: If SKU is 'standard3' and the 'hosting_mode' is set to 'highDensity' the maximum number of partitions allowed is 3 // where if 'hosting_mode' is set to 'default' the maximum number of partitions is 12... if pointer.From(model.Sku.Name) == services.SkuNameStandardThree && partitionCount > 3 && pointer.From(model.Properties.HostingMode) == services.HostingModeHighDensity { - return fmt.Errorf("'standard3' SKUs in 'highDensity' mode can have a maximum of 3 partitions, got %d", partitionCount) + return fmt.Errorf("%q SKUs in %q mode can have a maximum of 3 partitions, got %d", services.SkuNameStandardThree, services.HostingModeHighDensity, partitionCount) } model.Properties.PartitionCount = pointer.To(partitionCount) @@ -417,12 +434,6 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er return fmt.Errorf("updating %s: %+v", id, err) } - // If you remove this value from your config you need to set this back to the 'default' value in Azure(e.g. 'api_keys') - // in the expand func, but you also need to remove this code block from your state file else you will get a diff... - if len(apiAccessControl) == 0 { - d.Set("api_access_control", apiAccessControl) - } - return resourceSearchServiceRead(d, meta) } @@ -468,7 +479,8 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro publicNetworkAccess := true // publicNetworkAccess defaults to true... cmkEnforcement := false // cmkEnforcment defaults to false... hostingMode := services.HostingModeDefault - authenticationOptions := make([]interface{}, 0) + localAuthDisabled := false + authFailureMode := "" if count := props.PartitionCount; count != nil { partitionCount = int(pointer.From(count)) @@ -494,19 +506,30 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro cmkCompliance = string(pointer.From(props.EncryptionWithCmk.EncryptionComplianceStatus)) } - // I am using 'DisableLocalAuth' here because when you set your 'api_access_control' - // to 'RBAC only', the 'props.AuthOptions' will be 'nil'... + // I am using 'DisableLocalAuth' here because when you are in + // RBAC Only Mode, the 'props.AuthOptions' will be 'nil'... if props.DisableLocalAuth != nil { - authenticationOptions = flattenSearchServiceDataPlaneAuthOptions(props.AuthOptions, props.DisableLocalAuth) + localAuthDisabled = pointer.From(props.DisableLocalAuth) + + // if the AuthOptions are nil that means you are in RBAC Only Mode... + if props.AuthOptions != nil { + // If AuthOptions are not nil that means that you are in either + // API Keys Only Mode or RBAC & API Keys Mode... + if props.AuthOptions.AadOrApiKey != nil && props.AuthOptions.AadOrApiKey.AadAuthFailureMode != nil { + // You are in RBAC & API Keys Mode... + authFailureMode = string(pointer.From(props.AuthOptions.AadOrApiKey.AadAuthFailureMode)) + } + } } - d.Set("api_access_control", authenticationOptions) + d.Set("authentication_failure_mode", authFailureMode) + d.Set("local_authentication_disabled", localAuthDisabled) d.Set("partition_count", partitionCount) d.Set("replica_count", replicaCount) d.Set("public_network_access_enabled", publicNetworkAccess) d.Set("hosting_mode", hostingMode) - d.Set("cmk_enforcement_enabled", cmkEnforcement) - d.Set("cmk_enforcement_compliance", cmkCompliance) + d.Set("customer_managed_key_enforcement_enabled", cmkEnforcement) + d.Set("customer_managed_key_enforcement_compliance", cmkCompliance) d.Set("allowed_ips", flattenSearchServiceIPRules(props.NetworkRuleSet)) } @@ -604,45 +627,6 @@ func expandSearchServiceIPRules(input []interface{}) *[]services.IPRule { return &output } -func expandSearchServiceAuthOptions(input []interface{}) (*services.DataPlaneAuthOptions, error) { - // the default(e.g. 'ApiKeyOnly'), only requires an empty 'DataPlaneAuthOptions.ApiKeyOnly' - // 'interface{}' which must have an embedded empty 'map[string]interface{}'... - var apiKeyOnlyDefault interface{} = make(map[string]interface{}, 0) - - defaultAuthOptions := pointer.To(services.DataPlaneAuthOptions{ - ApiKeyOnly: pointer.To(apiKeyOnlyDefault), - }) - - if len(input) > 0 { - apiAccessControl := input[0].(map[string]interface{}) - accessControlType := apiAccessControl["type"].(string) - authFailureMode := apiAccessControl["authentication_failure_mode"].(string) - - if accessControlType != "role_based_access_control_and_api_keys" && authFailureMode != "" { - return nil, fmt.Errorf("it is invalid to define the 'authentication_failure_mode' when the 'api_access_control' 'type' is not set to 'role_based_access_control_and_api_keys', got 'api_access_control' 'type' %q", accessControlType) - } - - if accessControlType == "role_based_access_control_and_api_keys" && authFailureMode == "" { - return nil, fmt.Errorf("it is invalid to define the 'api_access_control' 'type' as 'role_based_access_control_and_api_keys' and not define the 'authentication_failure_mode', got 'authentication_failure_mode' %q", authFailureMode) - } - - switch accessControlType { - case "role_based_access_control": - return nil, nil - case "api_keys": - return defaultAuthOptions, nil - case "role_based_access_control_and_api_keys": - return pointer.To(services.DataPlaneAuthOptions{ - AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ - AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authFailureMode)), - }), - }), nil - } - } - - return defaultAuthOptions, nil -} - func flattenSearchServiceIPRules(input *services.NetworkRuleSet) []interface{} { if input == nil || *input.IPRules == nil || len(*input.IPRules) == 0 { return nil @@ -654,31 +638,6 @@ func flattenSearchServiceIPRules(input *services.NetworkRuleSet) []interface{} { return result } -func flattenSearchServiceDataPlaneAuthOptions(input *services.DataPlaneAuthOptions, localAuthenticationDisabled *bool) []interface{} { - if localAuthenticationDisabled == nil { - return []interface{}{} - } - - result := make(map[string]interface{}) - - // if 'localAuthenticationDisabled' is 'true' that means the this in RBAC Only mode... - if pointer.From(localAuthenticationDisabled) { - result["type"] = "role_based_access_control" - result["authentication_failure_mode"] = "" - } else { - // if 'localAuthenticationDisabled' is 'false' it can be API Keys Only or RBAC and API Keys Only... - if input.AadOrApiKey != nil && input.AadOrApiKey.AadAuthFailureMode != nil { - result["type"] = "role_based_access_control_and_api_keys" - result["authentication_failure_mode"] = pointer.From(input.AadOrApiKey.AadAuthFailureMode) - } else { - result["type"] = "api_keys" - result["authentication_failure_mode"] = "" - } - } - - return []interface{}{result} -} - func validateSearchServiceReplicaCount(replicaCount int64, skuName services.SkuName) (int64, error) { switch skuName { case services.SkuNameFree: diff --git a/internal/services/search/search_service_resource_test.go b/internal/services/search/search_service_resource_test.go index 8ee51ec9d677..daa94e2f601f 100644 --- a/internal/services/search/search_service_resource_test.go +++ b/internal/services/search/search_service_resource_test.go @@ -14,21 +14,15 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/utils" ) -type SearchServiceResource struct { - sku string - count int - enforceCmk bool - apiAccessControlType string - apiAccessControlAuthenticationFailureMode string -} +type SearchServiceResource struct{} func TestAccSearchService_basicStandard(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -40,11 +34,11 @@ func TestAccSearchService_basicStandard(t *testing.T) { func TestAccSearchService_basicFree(t *testing.T) { // Regression test case for issue #10151 data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "free"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data), + Config: r.basic(data, "free"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -55,11 +49,11 @@ func TestAccSearchService_basicFree(t *testing.T) { func TestAccSearchService_basicBasic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "basic"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data), + Config: r.basic(data, "basic"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -70,10 +64,10 @@ func TestAccSearchService_basicBasic(t *testing.T) { func TestAccSearchService_requiresImport(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -84,7 +78,7 @@ func TestAccSearchService_requiresImport(t *testing.T) { func TestAccSearchService_complete(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { @@ -99,11 +93,11 @@ func TestAccSearchService_complete(t *testing.T) { func TestAccSearchService_update(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -121,11 +115,11 @@ func TestAccSearchService_update(t *testing.T) { func TestAccSearchService_ipRules(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -139,7 +133,7 @@ func TestAccSearchService_ipRules(t *testing.T) { }, data.ImportStep(), { - Config: r.basic(data), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -150,11 +144,11 @@ func TestAccSearchService_ipRules(t *testing.T) { func TestAccSearchService_identity(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -168,7 +162,7 @@ func TestAccSearchService_identity(t *testing.T) { }, data.ImportStep(), { - Config: r.basic(data), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -179,11 +173,11 @@ func TestAccSearchService_identity(t *testing.T) { func TestAccSearchService_hostingMode(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard3"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.hostingMode(data), + Config: r.hostingMode(data, "standard3"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -194,11 +188,11 @@ func TestAccSearchService_hostingMode(t *testing.T) { func TestAccSearchService_hostingModeInvalidSKU(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard2"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.hostingMode(data), + Config: r.hostingMode(data, "standard2"), Check: acceptance.ComposeTestCheckFunc(), ExpectError: regexp.MustCompile("can only be defined if"), }, @@ -207,11 +201,11 @@ func TestAccSearchService_hostingModeInvalidSKU(t *testing.T) { func TestAccSearchService_partitionCountInvalidBySku(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "basic", count: 3} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.partitionCount(data), + Config: r.partitionCount(data, "basic", 3), Check: acceptance.ComposeTestCheckFunc(), ExpectError: regexp.MustCompile("values greater than 1 cannot be set"), }, @@ -220,28 +214,41 @@ func TestAccSearchService_partitionCountInvalidBySku(t *testing.T) { func TestAccSearchService_partitionCountInvalidByInput(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard", count: 5} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.partitionCount(data), + Config: r.partitionCount(data, "standard", 5), Check: acceptance.ComposeTestCheckFunc(), ExpectError: regexp.MustCompile("must be 1"), }, }) } +func TestAccSearchService_replicaCount(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + r := SearchServiceResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.replicaCount(data, "basic", 3), + Check: acceptance.ComposeTestCheckFunc(), + ExpectError: regexp.MustCompile("cannot be greater than 1 for the"), + }, + }) +} + func TestAccSearchService_replicaCountInvalid(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - // NOTE: The default qouta for the 'free' sku is 1 + // NOTE: The default quota for the 'free' sku is 1 // but it is ok to use it here since it will never be created since // it will fail validation due to the replica count defined in the // test configuration... - r := SearchServiceResource{sku: "free", count: 2} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.replicaCount(data), + Config: r.replicaCount(data, "free", 2), Check: acceptance.ComposeTestCheckFunc(), ExpectError: regexp.MustCompile("cannot be greater than 1 for the"), }, @@ -250,11 +257,11 @@ func TestAccSearchService_replicaCountInvalid(t *testing.T) { func TestAccSearchService_cmkEnforcement(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard", enforceCmk: true} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.cmkEnforcement(data), + Config: r.cmkEnforcement(data, true), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -265,27 +272,26 @@ func TestAccSearchService_cmkEnforcement(t *testing.T) { func TestAccSearchService_cmkEnforcementUpdate(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard", enforceCmk: false} - u := SearchServiceResource{sku: "standard", enforceCmk: true} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { // This should create a Search Service with the default 'cmk_enforcement_enabled' value of 'false'... - Config: r.basic(data), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: u.cmkEnforcement(data), + Config: r.cmkEnforcement(data, false), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: r.cmkEnforcement(data), + Config: r.cmkEnforcement(data, true), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -294,78 +300,61 @@ func TestAccSearchService_cmkEnforcementUpdate(t *testing.T) { }) } -func TestAccSearchService_apiAccessControlFailureModeWrongTypeError(t *testing.T) { +func TestAccSearchService_apiAccessControlRbacError(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard", apiAccessControlType: "api_keys", apiAccessControlAuthenticationFailureMode: "http401WithBearerChallenge"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.apiAccessControl(data), + Config: r.apiAccessControlBoth(data, true, "http401WithBearerChallenge"), Check: acceptance.ComposeTestCheckFunc(), - ExpectError: regexp.MustCompile(`is not set to 'role_based_access_control_and_api_keys'`), - }, - }) -} - -func TestAccSearchService_apiAccessControlRBACandAPIKeysNoFailureModeError(t *testing.T) { - data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard", apiAccessControlType: "role_based_access_control_and_api_keys"} - - data.ResourceTest(t, r, []acceptance.TestStep{ - { - Config: r.apiAccessControl(data), - Check: acceptance.ComposeTestCheckFunc(), - ExpectError: regexp.MustCompile(`and not define the 'authentication_failure_mode'`), + ExpectError: regexp.MustCompile("cannot be defined"), }, }) } func TestAccSearchService_apiAccessControlUpdate(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - r := SearchServiceResource{sku: "standard"} - a := SearchServiceResource{sku: "standard", apiAccessControlType: "api_keys"} - rbac := SearchServiceResource{sku: "standard", apiAccessControlType: "role_based_access_control"} - rbacApiFourZeroOne := SearchServiceResource{sku: "standard", apiAccessControlType: "role_based_access_control_and_api_keys", apiAccessControlAuthenticationFailureMode: "http401WithBearerChallenge"} - rbacApiFourZeroThree := SearchServiceResource{sku: "standard", apiAccessControlType: "role_based_access_control_and_api_keys", apiAccessControlAuthenticationFailureMode: "http403"} + r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: a.apiAccessControl(data), + Config: r.apiAccessControlApiKeysOrRBAC(data, false), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: rbac.apiAccessControl(data), + Config: r.apiAccessControlApiKeysOrRBAC(data, true), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: rbacApiFourZeroOne.apiAccessControl(data), + Config: r.apiAccessControlBoth(data, false, "http401WithBearerChallenge"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: rbacApiFourZeroThree.apiAccessControl(data), + Config: r.apiAccessControlBoth(data, false, "http403"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: r.basic(data), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -397,7 +386,7 @@ resource "azurerm_resource_group" "test" { `, data.RandomInteger, data.Locations.Primary) } -func (r SearchServiceResource) basic(data acceptance.TestData) string { +func (r SearchServiceResource) basic(data acceptance.TestData, sku string) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -416,11 +405,11 @@ resource "azurerm_search_service" "test" { environment = "staging" } } -`, template, data.RandomInteger, r.sku) +`, template, data.RandomInteger, sku) } func (r SearchServiceResource) requiresImport(data acceptance.TestData) string { - template := r.basic(data) + template := r.basic(data, "standard") return fmt.Sprintf(` %s @@ -450,7 +439,7 @@ resource "azurerm_search_service" "test" { name = "acctestsearchservice%d" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location - sku = "%s" + sku = "standard" replica_count = 2 partition_count = 3 @@ -462,7 +451,7 @@ resource "azurerm_search_service" "test" { residential = "Area" } } -`, template, data.RandomInteger, r.sku) +`, template, data.RandomInteger) } func (r SearchServiceResource) ipRules(data acceptance.TestData) string { @@ -478,7 +467,7 @@ resource "azurerm_search_service" "test" { name = "acctestsearchservice%d" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location - sku = "%s" + sku = "standard" allowed_ips = ["168.1.5.65", "1.2.3.0/24"] @@ -486,7 +475,7 @@ resource "azurerm_search_service" "test" { environment = "staging" } } -`, template, data.RandomInteger, r.sku) +`, template, data.RandomInteger) } func (r SearchServiceResource) identity(data acceptance.TestData) string { @@ -502,7 +491,7 @@ resource "azurerm_search_service" "test" { name = "acctestsearchservice%d" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location - sku = "%s" + sku = "standard" identity { type = "SystemAssigned" @@ -512,10 +501,10 @@ resource "azurerm_search_service" "test" { environment = "staging" } } -`, template, data.RandomInteger, r.sku) +`, template, data.RandomInteger) } -func (r SearchServiceResource) hostingMode(data acceptance.TestData) string { +func (r SearchServiceResource) hostingMode(data acceptance.TestData, sku string) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -535,10 +524,10 @@ resource "azurerm_search_service" "test" { environment = "staging" } } -`, template, data.RandomInteger, r.sku) +`, template, data.RandomInteger, sku) } -func (r SearchServiceResource) partitionCount(data acceptance.TestData) string { +func (r SearchServiceResource) partitionCount(data acceptance.TestData, sku string, count int) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -554,10 +543,10 @@ resource "azurerm_search_service" "test" { sku = "%s" partition_count = %d } -`, template, data.RandomInteger, r.sku, r.count) +`, template, data.RandomInteger, sku, count) } -func (r SearchServiceResource) replicaCount(data acceptance.TestData) string { +func (r SearchServiceResource) replicaCount(data acceptance.TestData, sku string, count int) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -573,10 +562,10 @@ resource "azurerm_search_service" "test" { sku = "%s" replica_count = %d } -`, template, data.RandomInteger, r.sku, r.count) +`, template, data.RandomInteger, sku, count) } -func (r SearchServiceResource) cmkEnforcement(data acceptance.TestData) string { +func (r SearchServiceResource) cmkEnforcement(data acceptance.TestData, enforceCmk bool) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -589,7 +578,7 @@ resource "azurerm_search_service" "test" { name = "acctestsearchservice%d" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location - sku = "%s" + sku = "standard" cmk_enforcement_enabled = %t @@ -597,10 +586,10 @@ resource "azurerm_search_service" "test" { environment = "staging" } } -`, template, data.RandomInteger, r.sku, r.enforceCmk) +`, template, data.RandomInteger, enforceCmk) } -func (r SearchServiceResource) apiAccessControl(data acceptance.TestData) string { +func (r SearchServiceResource) apiAccessControlApiKeysOrRBAC(data acceptance.TestData, localAuthenticationDisabled bool) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -613,16 +602,38 @@ resource "azurerm_search_service" "test" { name = "acctestsearchservice%d" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location - sku = "%s" + sku = "standard" - api_access_control { - type = "%s" - authentication_failure_mode = "%s" + local_authentication_disabled = %t + + tags = { + environment = "staging" } +} +`, template, data.RandomInteger, localAuthenticationDisabled) +} + +func (r SearchServiceResource) apiAccessControlBoth(data acceptance.TestData, localAuthenticationDisabled bool, authenticationFailureMode string) string { + template := r.template(data) + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%s + +resource "azurerm_search_service" "test" { + name = "acctestsearchservice%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + sku = "standard" + + local_authentication_disabled = %t + authentication_failure_mode = %s tags = { environment = "staging" } } -`, template, data.RandomInteger, r.sku, r.apiAccessControlType, r.apiAccessControlAuthenticationFailureMode) +`, template, data.RandomInteger, localAuthenticationDisabled, authenticationFailureMode) } diff --git a/internal/services/search/validate/search_service_partition_count.go b/internal/services/search/validate/search_service_partition_count.go deleted file mode 100644 index 46e6d5c214ff..000000000000 --- a/internal/services/search/validate/search_service_partition_count.go +++ /dev/null @@ -1,20 +0,0 @@ -package validate - -import ( - "fmt" -) - -func PartitionCount(v interface{}, k string) (warnings []string, errors []error) { - partitionCount := v.(int) - - switch partitionCount { - case 5, 7, 8, 9, 10, 11: - errors = append(errors, fmt.Errorf("%q must be 1, 2, 3, 4, 6, or 12, got %d", k, partitionCount)) - } - - if partitionCount > 12 { - errors = append(errors, fmt.Errorf("%q must be 1, 2, 3, 4, 6, or 12, got %d", k, partitionCount)) - } - - return warnings, errors -} diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index 23d13456e956..5910e38d63a0 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -44,14 +44,20 @@ The following arguments are supported: --- -* `api_access_control` - (Optional) An `api_access_control` block as defined below. +* `authentication_failure_mode` - (Optional) Describes what response the Search Service should return for requests that fail authentication. Possible values include `http401WithBearerChallenge` or `http403`. -* `cmk_enforcement_enabled` - (Optional) Should the Search Service enforce having non customer encrypted resources? Possible values include `true` or `false`. If `true` the Search Service will be marked as `non-compliant` if there are one or more non customer encrypted resources, if `false` no enforcement will be made and the Search Service can contain one or more non customer encrypted resources. Defaults to `false`. +-> **NOTE:** `authentication_failure_mode` cannot be defined if the `local_authentication_disabled` is set to `true`. + +* `customer_managed_key_enforcement_enabled` - (Optional) Should the Search Service enforce having non customer encrypted resources? Possible values include `true` or `false`. If `true` the Search Service will be marked as `non-compliant` if there are one or more non customer encrypted resources, if `false` no enforcement will be made and the Search Service can contain one or more non customer encrypted resources. Defaults to `false`. * `hosting_mode` - (Optional) Enable high density partitions that allow for up to a 1000 indexes. Possible values are `highDensity` or `default`. Defaults to `default`. Changing this forces a new Search Service to be created. -> **NOTE:** When the Search Service is in `highDensity` mode the maximum number of partitions allowed is `3`, to enable `hosting_mode` you must use a `standard3` SKU. +* `local_authentication_disabled` - (Optional) Should tha Search Service *not* be allowed to use API keys for authentication? Possible values include `true` or `false`. Defaults to `false`. + +-> **NOTE:** If the `local_authentication_disabled` field is `false` and the `authentication_failure_mode` has not been defined the Search Service will be in `API Keys Only` mode. If the `local_authentication_disabled` field is `false` and the `authentication_failure_mode` has also been set to `http401WithBearerChallenge` or `http403` the Search Service will be in `Role-based access contol and API Keys` mode (e.g. `Both`). If the `local_authentication_disabled` field is `true` the Search Service will be in `Role-based access contol Only` mode. When the `local_authentication_disabled` field is `true` the `authentication_failure_mode` cannot be defined. + * `public_network_access_enabled` - (Optional) Whether or not public network access is allowed for this resource. Defaults to `true`. * `partition_count` - (Optional) The number of partitions which should be created. Possible values include `1`, `2`, `3`, `4`, `6`, or `12`. Defaults to `1`. @@ -72,18 +78,6 @@ The following arguments are supported: --- -An `api_access_control` block supports the following: - -* `type` - (Optional) Describes what authentication mode should be used for the Search Service. Possible values inclued `api_keys`, `role_based_access_control`, or `role_based_access_control_and_api_keys`. Defaults to `api_keys`. - --> **NOTE:** If the `api_access_control` block has not been defined in the configuration file the Search Service will still be defaulted to use the `api_keys` authentication mode. - -* `authentication_failure_mode` - (Optional) Describes what response the Search Service should return for requests that fail authentication. Possible values include `http401WithBearerChallenge` or `http403`. Defaults to an `empty` string. - --> **NOTE:** `authentication_failure_mode` can only be defined if the `api_access_control` `type` is set to `role_based_access_control_and_api_keys`. - ---- - An `identity` block supports the following: * `type` - (Required) Specifies the type of Managed Service Identity that should be configured on this Search Service. The only possible value is `SystemAssigned`. @@ -96,7 +90,7 @@ In addition to the Arguments listed above - the following Attributes are exporte * `id` - The ID of the Search Service. -* `cmk_enforcement_compliance` - Describes whether the Search Service is `compliant` or not with respect to having non customer encrypted resources. If a Search Service has more than one non customer encrypted resource and the `cmk_enforcement_enabled` field is set to `true` the Search Service will be marked as `nonCompliant`. +* `customer_managed_key_enforcement_compliance` - Describes whether the Search Service is `compliant` or not with respect to having non customer encrypted resources. If a Search Service has more than one non customer encrypted resource and the `customer_managed_key_enforcement_enabled` field is set to `true` the Search Service will be marked as `nonCompliant`. * `primary_key` - The Primary Key used for Search Service Administration. From 7f4030fb8c113859f9d6c3a9594acc429e543f77 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Thu, 13 Apr 2023 17:39:00 -0600 Subject: [PATCH 11/17] Fix some test cases... --- .../search/search_service_resource_test.go | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/services/search/search_service_resource_test.go b/internal/services/search/search_service_resource_test.go index daa94e2f601f..2b559dfb9e69 100644 --- a/internal/services/search/search_service_resource_test.go +++ b/internal/services/search/search_service_resource_test.go @@ -255,13 +255,13 @@ func TestAccSearchService_replicaCountInvalid(t *testing.T) { }) } -func TestAccSearchService_cmkEnforcement(t *testing.T) { +func TestAccSearchService_customerManagedKeyEnforcement(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.cmkEnforcement(data, true), + Config: r.customerManagedKeyEnforcement(data, true), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -270,13 +270,13 @@ func TestAccSearchService_cmkEnforcement(t *testing.T) { }) } -func TestAccSearchService_cmkEnforcementUpdate(t *testing.T) { +func TestAccSearchService_customerManagedKeyEnforcementUpdate(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - // This should create a Search Service with the default 'cmk_enforcement_enabled' value of 'false'... + // This should create a Search Service with the default 'customer_managed_key_enforcement_enabled' value of 'false'... Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), @@ -284,14 +284,14 @@ func TestAccSearchService_cmkEnforcementUpdate(t *testing.T) { }, data.ImportStep(), { - Config: r.cmkEnforcement(data, false), + Config: r.customerManagedKeyEnforcement(data, false), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: r.cmkEnforcement(data, true), + Config: r.customerManagedKeyEnforcement(data, true), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -443,8 +443,8 @@ resource "azurerm_search_service" "test" { replica_count = 2 partition_count = 3 - public_network_access_enabled = false - cmk_enforcement_enabled = false + public_network_access_enabled = false + customer_managed_key_enforcement_enabled = false tags = { environment = "Production" @@ -565,7 +565,7 @@ resource "azurerm_search_service" "test" { `, template, data.RandomInteger, sku, count) } -func (r SearchServiceResource) cmkEnforcement(data acceptance.TestData, enforceCmk bool) string { +func (r SearchServiceResource) customerManagedKeyEnforcement(data acceptance.TestData, enforceCustomerManagedKey bool) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -580,13 +580,13 @@ resource "azurerm_search_service" "test" { location = azurerm_resource_group.test.location sku = "standard" - cmk_enforcement_enabled = %t + customer_managed_key_enforcement_enabled = %t tags = { environment = "staging" } } -`, template, data.RandomInteger, enforceCmk) +`, template, data.RandomInteger, enforceCustomerManagedKey) } func (r SearchServiceResource) apiAccessControlApiKeysOrRBAC(data acceptance.TestData, localAuthenticationDisabled bool) string { @@ -629,7 +629,7 @@ resource "azurerm_search_service" "test" { sku = "standard" local_authentication_disabled = %t - authentication_failure_mode = %s + authentication_failure_mode = "%s" tags = { environment = "staging" From cfb6f7594d62bf878078abe076315622ba18e1f4 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Thu, 13 Apr 2023 18:26:03 -0600 Subject: [PATCH 12/17] More test tweaks... --- .../search/search_service_resource_test.go | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/internal/services/search/search_service_resource_test.go b/internal/services/search/search_service_resource_test.go index 2b559dfb9e69..5627985a5f14 100644 --- a/internal/services/search/search_service_resource_test.go +++ b/internal/services/search/search_service_resource_test.go @@ -220,38 +220,40 @@ func TestAccSearchService_partitionCountInvalidByInput(t *testing.T) { { Config: r.partitionCount(data, "standard", 5), Check: acceptance.ComposeTestCheckFunc(), - ExpectError: regexp.MustCompile("must be 1"), + ExpectError: regexp.MustCompile(`expected partition_count`), }, }) } -func TestAccSearchService_replicaCount(t *testing.T) { +func TestAccSearchService_replicaCountInvalid(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") + // NOTE: The default quota for the 'free' sku is 1 + // but it is ok to use it here since it will never be created since + // it will fail validation due to the replica count defined in the + // test configuration... r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.replicaCount(data, "basic", 3), + Config: r.replicaCount(data, "free", 2), Check: acceptance.ComposeTestCheckFunc(), ExpectError: regexp.MustCompile("cannot be greater than 1 for the"), }, }) } -func TestAccSearchService_replicaCountInvalid(t *testing.T) { +func TestAccSearchService_replicaCount(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") - // NOTE: The default quota for the 'free' sku is 1 - // but it is ok to use it here since it will never be created since - // it will fail validation due to the replica count defined in the - // test configuration... r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.replicaCount(data, "free", 2), - Check: acceptance.ComposeTestCheckFunc(), - ExpectError: regexp.MustCompile("cannot be greater than 1 for the"), + Config: r.replicaCount(data, "basic", 3), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), }, + data.ImportStep(), }) } @@ -276,7 +278,8 @@ func TestAccSearchService_customerManagedKeyEnforcementUpdate(t *testing.T) { data.ResourceTest(t, r, []acceptance.TestStep{ { - // This should create a Search Service with the default 'customer_managed_key_enforcement_enabled' value of 'false'... + // This should create a Search Service with the default + // 'customer_managed_key_enforcement_enabled' value of 'false'... Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), @@ -284,14 +287,14 @@ func TestAccSearchService_customerManagedKeyEnforcementUpdate(t *testing.T) { }, data.ImportStep(), { - Config: r.customerManagedKeyEnforcement(data, false), + Config: r.customerManagedKeyEnforcement(data, true), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: r.customerManagedKeyEnforcement(data, true), + Config: r.customerManagedKeyEnforcement(data, false), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), From a3f6b7e684420da9b2feb867d010700557763cbe Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Thu, 13 Apr 2023 19:57:16 -0600 Subject: [PATCH 13/17] Update to docs... --- website/docs/r/search_service.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index 5910e38d63a0..4646e00a38d0 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -90,7 +90,7 @@ In addition to the Arguments listed above - the following Attributes are exporte * `id` - The ID of the Search Service. -* `customer_managed_key_enforcement_compliance` - Describes whether the Search Service is `compliant` or not with respect to having non customer encrypted resources. If a Search Service has more than one non customer encrypted resource and the `customer_managed_key_enforcement_enabled` field is set to `true` the Search Service will be marked as `nonCompliant`. +* `customer_managed_key_enforcement_compliance` - Describes whether the Search Service is `compliant` or not with respect to having non customer encrypted resources. * `primary_key` - The Primary Key used for Search Service Administration. From a1ab2a338453c4d54990502f0e3baffe0251ddbc Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Sat, 15 Apr 2023 01:54:09 -0600 Subject: [PATCH 14/17] Added Setting API Access Control section to docs --- website/docs/r/search_service.html.markdown | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index 4646e00a38d0..49a7b3ca22eb 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -46,7 +46,7 @@ The following arguments are supported: * `authentication_failure_mode` - (Optional) Describes what response the Search Service should return for requests that fail authentication. Possible values include `http401WithBearerChallenge` or `http403`. --> **NOTE:** `authentication_failure_mode` cannot be defined if the `local_authentication_disabled` is set to `true`. +~> **NOTE:** The `authentication_failure_mode` field is only valid if the Search Service is in the `Role-based access control and API Key` mode. For more information on how to correctly configure the Search Services `API Access Control` settings, please see the `Setting API Access Control` section below. * `customer_managed_key_enforcement_enabled` - (Optional) Should the Search Service enforce having non customer encrypted resources? Possible values include `true` or `false`. If `true` the Search Service will be marked as `non-compliant` if there are one or more non customer encrypted resources, if `false` no enforcement will be made and the Search Service can contain one or more non customer encrypted resources. Defaults to `false`. @@ -56,7 +56,7 @@ The following arguments are supported: * `local_authentication_disabled` - (Optional) Should tha Search Service *not* be allowed to use API keys for authentication? Possible values include `true` or `false`. Defaults to `false`. --> **NOTE:** If the `local_authentication_disabled` field is `false` and the `authentication_failure_mode` has not been defined the Search Service will be in `API Keys Only` mode. If the `local_authentication_disabled` field is `false` and the `authentication_failure_mode` has also been set to `http401WithBearerChallenge` or `http403` the Search Service will be in `Role-based access contol and API Keys` mode (e.g. `Both`). If the `local_authentication_disabled` field is `true` the Search Service will be in `Role-based access contol Only` mode. When the `local_authentication_disabled` field is `true` the `authentication_failure_mode` cannot be defined. +-> **NOTE:** For more information on how to correctly configure the Search Services `API Access Control` settings, please see the `Setting API Access Control` section below. * `public_network_access_enabled` - (Optional) Whether or not public network access is allowed for this resource. Defaults to `true`. @@ -114,6 +114,22 @@ An `identity` block exports the following: * `tenant_id` - The Tenant ID associated with this Managed Service Identity. +--- + +## Setting API Access Control: + +The values of the `local_authentication_disabled` and `authentication_failure_mode` fields will determin which `API Access Control` mode the Search Service will operate under. To correctly configure your Search Service to your desired `API Access Control` mode please configure your Search Service based on the field values in the below table. + +| Field Name | Value | API Access Control Mode | +|------------------------------------------------------------------|:---------------------------------------------------------:|------------------------------------------| +| `local_authentication_disabled`
`authentication_failure_mode` | `false`
`` | `API key` (`Default`) | +| `local_authentication_disabled`
`authentication_failure_mode` | `false`
`authentication_failure_mode` as defined above | `Role-based access control and API Key` | +| `local_authentication_disabled`
`authentication_failure_mode` | `true`
`` | `Role-based access control` | + +-> **NOTE:** When the Search Service is in `Role-based access contol` (e.g. `local_authentication_disabled` is `true`) the `authentication_failure_mode` field *cannot* be defined. + +--- + ## Timeouts The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/language/resources/syntax#operation-timeouts) for certain actions: From 5b761d57ff079e58766a349bd20ef35117bfeaa5 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Sat, 15 Apr 2023 01:55:22 -0600 Subject: [PATCH 15/17] Fix spacing in said doc table... --- website/docs/r/search_service.html.markdown | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index 49a7b3ca22eb..61d6c910ca07 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -122,9 +122,9 @@ The values of the `local_authentication_disabled` and `authentication_failure_mo | Field Name | Value | API Access Control Mode | |------------------------------------------------------------------|:---------------------------------------------------------:|------------------------------------------| -| `local_authentication_disabled`
`authentication_failure_mode` | `false`
`` | `API key` (`Default`) | +| `local_authentication_disabled`
`authentication_failure_mode` | `false`
`` | `API key` (`Default`) | | `local_authentication_disabled`
`authentication_failure_mode` | `false`
`authentication_failure_mode` as defined above | `Role-based access control and API Key` | -| `local_authentication_disabled`
`authentication_failure_mode` | `true`
`` | `Role-based access control` | +| `local_authentication_disabled`
`authentication_failure_mode` | `true`
`` | `Role-based access control` | -> **NOTE:** When the Search Service is in `Role-based access contol` (e.g. `local_authentication_disabled` is `true`) the `authentication_failure_mode` field *cannot* be defined. From f4fa384d709c2e61bab2cb8facfdb2c0336cffd9 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Sat, 15 Apr 2023 02:02:05 -0600 Subject: [PATCH 16/17] Center last col in docs... --- website/docs/r/search_service.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index 61d6c910ca07..de47b1782f9d 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -121,7 +121,7 @@ An `identity` block exports the following: The values of the `local_authentication_disabled` and `authentication_failure_mode` fields will determin which `API Access Control` mode the Search Service will operate under. To correctly configure your Search Service to your desired `API Access Control` mode please configure your Search Service based on the field values in the below table. | Field Name | Value | API Access Control Mode | -|------------------------------------------------------------------|:---------------------------------------------------------:|------------------------------------------| +|------------------------------------------------------------------|:---------------------------------------------------------:|:----------------------------------------:| | `local_authentication_disabled`
`authentication_failure_mode` | `false`
`` | `API key` (`Default`) | | `local_authentication_disabled`
`authentication_failure_mode` | `false`
`authentication_failure_mode` as defined above | `Role-based access control and API Key` | | `local_authentication_disabled`
`authentication_failure_mode` | `true`
`` | `Role-based access control` | From 859433ae43664f8443b533d801ab41cefbf96ca6 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 26 Apr 2023 13:42:33 +0200 Subject: [PATCH 17/17] r/search_service: refactoring comments from PR review --- .../search/search_service_data_source.go | 39 ++- .../search/search_service_resource.go | 284 +++++++++--------- .../search/search_service_resource_test.go | 72 ++--- website/docs/r/search_service.html.markdown | 86 +++--- 4 files changed, 231 insertions(+), 250 deletions(-) diff --git a/internal/services/search/search_service_data_source.go b/internal/services/search/search_service_data_source.go index 8498bae1fef6..b66909a84d38 100644 --- a/internal/services/search/search_service_data_source.go +++ b/internal/services/search/search_service_data_source.go @@ -2,8 +2,11 @@ package search import ( "fmt" + "net/http" + "strings" "time" + "github.com/hashicorp/go-azure-helpers/lang/pointer" "github.com/hashicorp/go-azure-helpers/lang/response" "github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema" "github.com/hashicorp/go-azure-helpers/resourcemanager/identity" @@ -13,6 +16,7 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/internal/clients" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" "github.com/hashicorp/terraform-provider-azurerm/internal/timeouts" + "github.com/hashicorp/terraform-provider-azurerm/utils" ) func dataSourceSearchService() *pluginsdk.Resource { @@ -29,7 +33,7 @@ func dataSourceSearchService() *pluginsdk.Resource { Required: true, }, - "resource_group_name": commonschema.ResourceGroupName(), + "resource_group_name": commonschema.ResourceGroupNameForDataSource(), "replica_count": { Type: pluginsdk.TypeInt, @@ -102,9 +106,9 @@ func dataSourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) er if model := resp.Model; model != nil { if props := model.Properties; props != nil { - partitionCount := 0 - replicaCount := 0 - publicNetworkAccess := false + partitionCount := 1 + replicaCount := 1 + publicNetworkAccess := true if count := props.PartitionCount; count != nil { partitionCount = int(*count) @@ -115,7 +119,7 @@ func dataSourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) er } if props.PublicNetworkAccess != nil { - publicNetworkAccess = *props.PublicNetworkAccess != "Disabled" + publicNetworkAccess = strings.EqualFold(string(pointer.From(props.PublicNetworkAccess)), string(services.PublicNetworkAccessEnabled)) } d.Set("partition_count", partitionCount) @@ -128,19 +132,23 @@ func dataSourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) er } } + primaryKey := "" + secondaryKey := "" adminKeysClient := meta.(*clients.Client).Search.AdminKeysClient adminKeysId, err := adminkeys.ParseSearchServiceID(d.Id()) if err != nil { return err } - adminKeysResp, err := adminKeysClient.Get(ctx, *adminKeysId, adminkeys.GetOperationOptions{}) - if err == nil { - if model := adminKeysResp.Model; model != nil { - d.Set("primary_key", model.PrimaryKey) - d.Set("secondary_key", model.SecondaryKey) - } + if err != nil && !response.WasStatusCode(adminKeysResp.HttpResponse, http.StatusForbidden) { + return fmt.Errorf("retrieving Admin Keys for %s: %+v", id, err) + } + if model := adminKeysResp.Model; model != nil { + primaryKey = utils.NormalizeNilableString(model.PrimaryKey) + secondaryKey = utils.NormalizeNilableString(model.SecondaryKey) } + d.Set("primary_key", primaryKey) + d.Set("secondary_key", secondaryKey) queryKeysClient := meta.(*clients.Client).Search.QueryKeysClient queryKeysId, err := querykeys.ParseSearchServiceID(d.Id()) @@ -148,10 +156,11 @@ func dataSourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) er return err } queryKeysResp, err := queryKeysClient.ListBySearchService(ctx, *queryKeysId, querykeys.ListBySearchServiceOperationOptions{}) - if err == nil { - if model := queryKeysResp.Model; model != nil { - d.Set("query_keys", flattenSearchQueryKeys(*model)) - } + if err != nil && !response.WasStatusCode(queryKeysResp.HttpResponse, http.StatusForbidden) { + return fmt.Errorf("retrieving Query Keys for %s: %+v", id, err) + } + if err := d.Set("query_keys", flattenSearchQueryKeys(queryKeysResp.Model)); err != nil { + return fmt.Errorf("setting `query_keys`: %+v", err) } return nil diff --git a/internal/services/search/search_service_resource.go b/internal/services/search/search_service_resource.go index 4c94ea2e6de4..dc3f78c51bb8 100644 --- a/internal/services/search/search_service_resource.go +++ b/internal/services/search/search_service_resource.go @@ -15,7 +15,6 @@ import ( "github.com/hashicorp/go-azure-sdk/resource-manager/search/2022-09-01/adminkeys" "github.com/hashicorp/go-azure-sdk/resource-manager/search/2022-09-01/querykeys" "github.com/hashicorp/go-azure-sdk/resource-manager/search/2022-09-01/services" - "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/helpers/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" @@ -91,10 +90,10 @@ func resourceSearchService() *pluginsdk.Resource { }), }, - "local_authentication_disabled": { + "local_authentication_enabled": { Type: pluginsdk.TypeBool, Optional: true, - Default: false, + Default: true, }, "authentication_failure_mode": { @@ -110,7 +109,7 @@ func resourceSearchService() *pluginsdk.Resource { Type: pluginsdk.TypeString, Optional: true, ForceNew: true, - Default: services.HostingModeDefault, + Default: string(services.HostingModeDefault), ValidateFunc: validation.StringInSlice([]string{ string(services.HostingModeDefault), string(services.HostingModeHighDensity), @@ -123,11 +122,6 @@ func resourceSearchService() *pluginsdk.Resource { Default: false, }, - "customer_managed_key_enforcement_compliance": { - Type: pluginsdk.TypeString, - Computed: true, - }, - "primary_key": { Type: pluginsdk.TypeString, Computed: true, @@ -184,7 +178,7 @@ func resourceSearchService() *pluginsdk.Resource { func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Search.ServicesClient subscriptionId := meta.(*clients.Client).Account.SubscriptionId - ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) + ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d) defer cancel() id := services.NewSearchServiceID(subscriptionId, d.Get("resource_group_name").(string), d.Get("name").(string)) @@ -198,8 +192,6 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er return tf.ImportAsExistsError("azurerm_search_service", id.ID()) } - location := azure.NormalizeLocation(d.Get("location").(string)) - publicNetworkAccess := services.PublicNetworkAccessEnabled if enabled := d.Get("public_network_access_enabled").(bool); !enabled { publicNetworkAccess = services.PublicNetworkAccessDisabled @@ -210,7 +202,7 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er ipRulesRaw := d.Get("allowed_ips").(*pluginsdk.Set).List() hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) cmkEnforcementEnabled := d.Get("customer_managed_key_enforcement_enabled").(bool) - localAuthenticationDisabled := d.Get("local_authentication_disabled").(bool) + localAuthenticationEnabled := d.Get("local_authentication_enabled").(bool) authenticationFailureMode := d.Get("authentication_failure_mode").(string) cmkEnforcement := services.SearchEncryptionWithCmkDisabled @@ -220,20 +212,20 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er // NOTE: hosting mode is only valid if the SKU is 'standard3' if skuName != services.SkuNameStandardThree && hostingMode == services.HostingModeHighDensity { - return fmt.Errorf("'hosting_mode' can only be defined if the 'sku' field is set to the 'standard3' SKU, got %q", skuName) + return fmt.Errorf("'hosting_mode' can only be defined if the 'sku' field is set to the %q SKU, got %q", string(services.SkuNameStandardThree), skuName) } // NOTE: 'partition_count' values greater than 1 are not valid for 'free' or 'basic' SKUs... partitionCount := int64(d.Get("partition_count").(int)) if (skuName == services.SkuNameFree || skuName == services.SkuNameBasic) && partitionCount > 1 { - return fmt.Errorf("'partition_count' values greater than 1 cannot be set for the %q SKU, got %d)", skuName, partitionCount) + return fmt.Errorf("'partition_count' values greater than 1 cannot be set for the %q SKU, got %d)", string(skuName), partitionCount) } // NOTE: 'standard3' services with 'hostingMode' set to 'highDensity' the // 'partition_count' must be between 1 and 3. if skuName == services.SkuNameStandardThree && partitionCount > 3 && hostingMode == services.HostingModeHighDensity { - return fmt.Errorf("%q SKUs in %q mode can have a maximum of 3 partitions, got %d", services.SkuNameStandardThree, services.HostingModeHighDensity, partitionCount) + return fmt.Errorf("%q SKUs in %q mode can have a maximum of 3 partitions, got %d", string(services.SkuNameStandardThree), string(services.HostingModeHighDensity), partitionCount) } // The number of replicas can be between 1 and 12 for 'standard', 'storage_optimized_l1' and storage_optimized_l2' SKUs @@ -243,31 +235,31 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er return err } - if localAuthenticationDisabled && authenticationFailureMode != "" { - return fmt.Errorf("'authentication_failure_mode' cannot be defined if 'local_authentication_disabled' has been set to 'true'") + if !localAuthenticationEnabled && authenticationFailureMode != "" { + return fmt.Errorf("'authentication_failure_mode' cannot be defined if 'local_authentication_enabled' has been set to 'true'") } - // API Only Mode (Defalut)(e.g. localAuthenticationDisabled = false)... + // API Only Mode (Default) (e.g. localAuthenticationEnabled = true)... authenticationOptions := pointer.To(services.DataPlaneAuthOptions{ ApiKeyOnly: pointer.To(apiKeyOnly), }) - if !localAuthenticationDisabled && authenticationFailureMode != "" { + if localAuthenticationEnabled && authenticationFailureMode != "" { // API & RBAC Mode.. authenticationOptions = pointer.To(services.DataPlaneAuthOptions{ AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ - AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authenticationFailureMode)), + AadAuthFailureMode: pointer.To(services.AadAuthFailureMode(authenticationFailureMode)), }), }) } - if localAuthenticationDisabled { + if !localAuthenticationEnabled { // RBAC Only Mode... authenticationOptions = nil } - searchService := services.SearchService{ - Location: location, + payload := services.SearchService{ + Location: location.Normalize(d.Get("location").(string)), Sku: pointer.To(services.Sku{ Name: pointer.To(skuName), }), @@ -281,7 +273,7 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er }), HostingMode: pointer.To(hostingMode), AuthOptions: authenticationOptions, - DisableLocalAuth: pointer.To(localAuthenticationDisabled), + DisableLocalAuth: pointer.To(!localAuthenticationEnabled), PartitionCount: pointer.To(partitionCount), ReplicaCount: pointer.To(replicaCount), }, @@ -297,10 +289,10 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er // in the create call, only in the update call when 'identity' is removed from the // configuration file... if expandedIdentity.Type != identity.TypeNone { - searchService.Identity = expandedIdentity + payload.Identity = expandedIdentity } - err = client.CreateOrUpdateThenPoll(ctx, id, searchService, services.CreateOrUpdateOperationOptions{}) + err = client.CreateOrUpdateThenPoll(ctx, id, payload, services.CreateOrUpdateOperationOptions{}) if err != nil { return fmt.Errorf("creating %s: %+v", id, err) } @@ -312,7 +304,7 @@ func resourceSearchServiceCreate(d *pluginsdk.ResourceData, meta interface{}) er func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Search.ServicesClient - ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) + ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) defer cancel() id, err := services.ParseSearchServiceID(d.Id()) @@ -324,120 +316,128 @@ func resourceSearchServiceUpdate(d *pluginsdk.ResourceData, meta interface{}) er if err != nil { return fmt.Errorf("retrieving %s: %+v", id, err) } + if resp.Model == nil { + return fmt.Errorf("retrieving existing %s: %+v", id, err) + } + model := *resp.Model + if model.Properties == nil { + return fmt.Errorf("retrieving existing %s: `properties` was nil", id) + } - if model := resp.Model; model != nil { - if d.HasChange("public_network_access_enabled") { - publicNetworkAccess := services.PublicNetworkAccessEnabled - if enabled := d.Get("public_network_access_enabled").(bool); !enabled { - publicNetworkAccess = services.PublicNetworkAccessDisabled - } - - model.Properties.PublicNetworkAccess = pointer.To(publicNetworkAccess) + if d.HasChange("customer_managed_key_enforcement_enabled") { + cmkEnforcement := services.SearchEncryptionWithCmkDisabled + if enabled := d.Get("customer_managed_key_enforcement_enabled").(bool); enabled { + cmkEnforcement = services.SearchEncryptionWithCmkEnabled + } + model.Properties.EncryptionWithCmk = &services.EncryptionWithCmk{ + Enforcement: pointer.To(cmkEnforcement), } + } - if d.HasChange("identity") { - expandedIdentity, err := identity.ExpandSystemAssigned(d.Get("identity").([]interface{})) - if err != nil { - return fmt.Errorf("expanding `identity`: %+v", err) - } + if d.HasChange("hosting_mode") { + hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) + if model.Sku == nil { + return fmt.Errorf("updating `hosting_mode` for %s: unable to validate the hosting_mode since `model.Sku` was nil", *id) + } - model.Identity = expandedIdentity + if pointer.From(model.Sku.Name) != services.SkuNameStandardThree && hostingMode == services.HostingModeHighDensity { + return fmt.Errorf("'hosting_mode' can only be set to %q if the 'sku' is %q, got %q", services.HostingModeHighDensity, services.SkuNameStandardThree, pointer.From(model.Sku.Name)) } - if d.HasChange("hosting_mode") { - hostingMode := services.HostingMode(d.Get("hosting_mode").(string)) - if pointer.From(model.Sku.Name) != services.SkuNameStandardThree && hostingMode == services.HostingModeHighDensity { - return fmt.Errorf("'hosting_mode' can only be set to %q if the 'sku' is %q, got %q", services.HostingModeHighDensity, services.SkuNameStandardThree, pointer.From(model.Sku.Name)) - } + model.Properties.HostingMode = pointer.To(hostingMode) + } - model.Properties.HostingMode = pointer.To(hostingMode) + if d.HasChange("identity") { + expandedIdentity, err := identity.ExpandSystemAssigned(d.Get("identity").([]interface{})) + if err != nil { + return fmt.Errorf("expanding `identity`: %+v", err) } - if d.HasChange("customer_managed_key_enforcement_enabled") { - cmkEnforcement := services.SearchEncryptionWithCmkDisabled - if enabled := d.Get("customer_managed_key_enforcement_enabled").(bool); enabled { - cmkEnforcement = services.SearchEncryptionWithCmkEnabled - } + model.Identity = expandedIdentity + } - model.Properties.EncryptionWithCmk.Enforcement = pointer.To(cmkEnforcement) + if d.HasChange("public_network_access_enabled") { + publicNetworkAccess := services.PublicNetworkAccessEnabled + if enabled := d.Get("public_network_access_enabled").(bool); !enabled { + publicNetworkAccess = services.PublicNetworkAccessDisabled } - if d.HasChange("local_authentication_disabled") || d.HasChange("authentication_failure_mode") { - localAuthenticationDisabled := d.Get("local_authentication_disabled").(bool) - authenticationFailureMode := d.Get("authentication_failure_mode").(string) - - if localAuthenticationDisabled && authenticationFailureMode != "" { - return fmt.Errorf("'authentication_failure_mode' cannot be defined if 'local_authentication_disabled' has been set to 'true'") - } + model.Properties.PublicNetworkAccess = pointer.To(publicNetworkAccess) + } - var apiKeyOnly interface{} = make(map[string]interface{}, 0) + if d.HasChanges("authentication_failure_mode", "local_authentication_enabled") { + authenticationFailureMode := d.Get("authentication_failure_mode").(string) + localAuthenticationEnabled := d.Get("local_authentication_enabled").(bool) + if !localAuthenticationEnabled && authenticationFailureMode != "" { + return fmt.Errorf("'authentication_failure_mode' cannot be defined if 'local_authentication_enabled' has been set to 'false'") + } - // API Only Mode (Defalut)... - authenticationOptions := pointer.To(services.DataPlaneAuthOptions{ - ApiKeyOnly: pointer.To(apiKeyOnly), - }) + var apiKeyOnly interface{} = make(map[string]interface{}, 0) - if !localAuthenticationDisabled && authenticationFailureMode != "" { - // API & RBAC Mode.. - authenticationOptions = pointer.To(services.DataPlaneAuthOptions{ - AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ - AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authenticationFailureMode)), - }), - }) - } + // API Only Mode (Default)... + authenticationOptions := pointer.To(services.DataPlaneAuthOptions{ + ApiKeyOnly: pointer.To(apiKeyOnly), + }) - if localAuthenticationDisabled { - // RBAC Only Mode... - authenticationOptions = nil - } + if localAuthenticationEnabled && authenticationFailureMode != "" { + // API & RBAC Mode.. + authenticationOptions = pointer.To(services.DataPlaneAuthOptions{ + AadOrApiKey: pointer.To(services.DataPlaneAadOrApiKeyAuthOption{ + AadAuthFailureMode: (*services.AadAuthFailureMode)(pointer.To(authenticationFailureMode)), + }), + }) + } - model.Properties.DisableLocalAuth = pointer.To(localAuthenticationDisabled) - model.Properties.AuthOptions = authenticationOptions + if !localAuthenticationEnabled { + // RBAC Only Mode... + authenticationOptions = nil } - if d.HasChange("replica_count") { - replicaCount, err := validateSearchServiceReplicaCount(int64(d.Get("replica_count").(int)), pointer.From(model.Sku.Name)) - if err != nil { - return err - } + model.Properties.DisableLocalAuth = pointer.To(!localAuthenticationEnabled) + model.Properties.AuthOptions = authenticationOptions + } - model.Properties.ReplicaCount = pointer.To(replicaCount) + if d.HasChange("replica_count") { + replicaCount, err := validateSearchServiceReplicaCount(int64(d.Get("replica_count").(int)), pointer.From(model.Sku.Name)) + if err != nil { + return err } - if d.HasChange("partition_count") { - partitionCount := int64(d.Get("partition_count").(int)) - // NOTE: 'partition_count' values greater than 1 are not valid for 'free' or 'basic' SKUs... - if (pointer.From(model.Sku.Name) == services.SkuNameFree || pointer.From(model.Sku.Name) == services.SkuNameBasic) && partitionCount > 1 { - return fmt.Errorf("'partition_count' values greater than 1 cannot be set for the %q SKU, got %d)", pointer.From(model.Sku.Name), partitionCount) - } - - // NOTE: If SKU is 'standard3' and the 'hosting_mode' is set to 'highDensity' the maximum number of partitions allowed is 3 - // where if 'hosting_mode' is set to 'default' the maximum number of partitions is 12... - if pointer.From(model.Sku.Name) == services.SkuNameStandardThree && partitionCount > 3 && pointer.From(model.Properties.HostingMode) == services.HostingModeHighDensity { - return fmt.Errorf("%q SKUs in %q mode can have a maximum of 3 partitions, got %d", services.SkuNameStandardThree, services.HostingModeHighDensity, partitionCount) - } + model.Properties.ReplicaCount = pointer.To(replicaCount) + } - model.Properties.PartitionCount = pointer.To(partitionCount) + if d.HasChange("partition_count") { + partitionCount := int64(d.Get("partition_count").(int)) + // NOTE: 'partition_count' values greater than 1 are not valid for 'free' or 'basic' SKUs... + if (pointer.From(model.Sku.Name) == services.SkuNameFree || pointer.From(model.Sku.Name) == services.SkuNameBasic) && partitionCount > 1 { + return fmt.Errorf("'partition_count' values greater than 1 cannot be set for the %q SKU, got %d)", pointer.From(model.Sku.Name), partitionCount) } - if d.HasChange("allowed_ips") { - ipRulesRaw := d.Get("allowed_ips").(*pluginsdk.Set).List() - model.Properties.NetworkRuleSet.IPRules = expandSearchServiceIPRules(ipRulesRaw) + // NOTE: If SKU is 'standard3' and the 'hosting_mode' is set to 'highDensity' the maximum number of partitions allowed is 3 + // where if 'hosting_mode' is set to 'default' the maximum number of partitions is 12... + if pointer.From(model.Sku.Name) == services.SkuNameStandardThree && partitionCount > 3 && pointer.From(model.Properties.HostingMode) == services.HostingModeHighDensity { + return fmt.Errorf("%q SKUs in %q mode can have a maximum of 3 partitions, got %d", string(services.SkuNameStandardThree), string(services.HostingModeHighDensity), partitionCount) } - if d.HasChange("tags") { - model.Tags = tags.Expand(d.Get("tags").(map[string]interface{})) - } + model.Properties.PartitionCount = pointer.To(partitionCount) + } - err = client.CreateOrUpdateThenPoll(ctx, pointer.From(id), pointer.From(model), services.CreateOrUpdateOperationOptions{}) - if err != nil { - return fmt.Errorf("updating %s: %+v", id, err) + if d.HasChange("allowed_ips") { + ipRulesRaw := d.Get("allowed_ips").(*pluginsdk.Set).List() + model.Properties.NetworkRuleSet = &services.NetworkRuleSet{ + IPRules: expandSearchServiceIPRules(ipRulesRaw), } + } - return resourceSearchServiceRead(d, meta) + if d.HasChange("tags") { + model.Tags = tags.Expand(d.Get("tags").(map[string]interface{})) } - return nil + if err = client.CreateOrUpdateThenPoll(ctx, *id, model, services.CreateOrUpdateOperationOptions{}); err != nil { + return fmt.Errorf("updating %s: %+v", id, err) + } + + return resourceSearchServiceRead(d, meta) } func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) error { @@ -479,7 +479,7 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro publicNetworkAccess := true // publicNetworkAccess defaults to true... cmkEnforcement := false // cmkEnforcment defaults to false... hostingMode := services.HostingModeDefault - localAuthDisabled := false + localAuthEnabled := true authFailureMode := "" if count := props.PartitionCount; count != nil { @@ -500,16 +500,14 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro hostingMode = *props.HostingMode } - var cmkCompliance string if props.EncryptionWithCmk != nil { cmkEnforcement = strings.EqualFold(string(pointer.From(props.EncryptionWithCmk.Enforcement)), string(services.SearchEncryptionWithCmkEnabled)) - cmkCompliance = string(pointer.From(props.EncryptionWithCmk.EncryptionComplianceStatus)) } // I am using 'DisableLocalAuth' here because when you are in // RBAC Only Mode, the 'props.AuthOptions' will be 'nil'... if props.DisableLocalAuth != nil { - localAuthDisabled = pointer.From(props.DisableLocalAuth) + localAuthEnabled = !pointer.From(props.DisableLocalAuth) // if the AuthOptions are nil that means you are in RBAC Only Mode... if props.AuthOptions != nil { @@ -523,13 +521,12 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro } d.Set("authentication_failure_mode", authFailureMode) - d.Set("local_authentication_disabled", localAuthDisabled) + d.Set("local_authentication_enabled", localAuthEnabled) d.Set("partition_count", partitionCount) d.Set("replica_count", replicaCount) d.Set("public_network_access_enabled", publicNetworkAccess) d.Set("hosting_mode", hostingMode) d.Set("customer_managed_key_enforcement_enabled", cmkEnforcement) - d.Set("customer_managed_key_enforcement_compliance", cmkCompliance) d.Set("allowed_ips", flattenSearchServiceIPRules(props.NetworkRuleSet)) } @@ -549,11 +546,12 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro } adminKeysResp, err := adminKeysClient.Get(ctx, *adminKeysId, adminkeys.GetOperationOptions{}) - if err == nil { - if model := adminKeysResp.Model; model != nil { - d.Set("primary_key", model.PrimaryKey) - d.Set("secondary_key", model.SecondaryKey) - } + if err != nil { + return fmt.Errorf("retrieving Admin Keys for %s: %+v", *id, err) + } + if model := adminKeysResp.Model; model != nil { + d.Set("primary_key", model.PrimaryKey) + d.Set("secondary_key", model.SecondaryKey) } queryKeysClient := meta.(*clients.Client).Search.QueryKeysClient @@ -562,10 +560,11 @@ func resourceSearchServiceRead(d *pluginsdk.ResourceData, meta interface{}) erro return err } queryKeysResp, err := queryKeysClient.ListBySearchService(ctx, *queryKeysId, querykeys.ListBySearchServiceOperationOptions{}) - if err == nil { - if model := queryKeysResp.Model; model != nil { - d.Set("query_keys", flattenSearchQueryKeys(*model)) - } + if err != nil { + return fmt.Errorf("retrieving Query Keys for %s: %+v", *id, err) + } + if err := d.Set("query_keys", flattenSearchQueryKeys(queryKeysResp.Model)); err != nil { + return fmt.Errorf("setting `query_keys`: %+v", err) } return nil @@ -581,30 +580,23 @@ func resourceSearchServiceDelete(d *pluginsdk.ResourceData, meta interface{}) er return err } - resp, err := client.Delete(ctx, *id, services.DeleteOperationOptions{}) - if err != nil { - if response.WasNotFound(resp.HttpResponse) { - return nil - } - + if _, err := client.Delete(ctx, *id, services.DeleteOperationOptions{}); err != nil { return fmt.Errorf("deleting %s: %+v", *id, err) } return nil } -func flattenSearchQueryKeys(input []querykeys.QueryKey) []interface{} { +func flattenSearchQueryKeys(input *[]querykeys.QueryKey) []interface{} { results := make([]interface{}, 0) - for _, v := range input { - result := make(map[string]interface{}) - - if v.Name != nil { - result["name"] = *v.Name + if input != nil { + for _, v := range *input { + results = append(results, map[string]interface{}{ + "name": utils.NormalizeNilableString(v.Name), + "key": utils.NormalizeNilableString(v.Key), + }) } - result["key"] = *v.Key - - results = append(results, result) } return results @@ -612,9 +604,6 @@ func flattenSearchQueryKeys(input []querykeys.QueryKey) []interface{} { func expandSearchServiceIPRules(input []interface{}) *[]services.IPRule { output := make([]services.IPRule, 0) - if input == nil { - return &output - } for _, rule := range input { if rule != nil { @@ -628,12 +617,11 @@ func expandSearchServiceIPRules(input []interface{}) *[]services.IPRule { } func flattenSearchServiceIPRules(input *services.NetworkRuleSet) []interface{} { - if input == nil || *input.IPRules == nil || len(*input.IPRules) == 0 { - return nil - } result := make([]interface{}, 0) - for _, rule := range *input.IPRules { - result = append(result, rule.Value) + if input != nil || input.IPRules != nil { + for _, rule := range *input.IPRules { + result = append(result, rule.Value) + } } return result } diff --git a/internal/services/search/search_service_resource_test.go b/internal/services/search/search_service_resource_test.go index 5627985a5f14..4bfdcbc82f1a 100644 --- a/internal/services/search/search_service_resource_test.go +++ b/internal/services/search/search_service_resource_test.go @@ -16,13 +16,13 @@ import ( type SearchServiceResource struct{} -func TestAccSearchService_basicStandard(t *testing.T) { +func TestAccSearchService_basicSku(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data, "standard"), + Config: r.basic(data, "basic"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -31,7 +31,7 @@ func TestAccSearchService_basicStandard(t *testing.T) { }) } -func TestAccSearchService_basicFree(t *testing.T) { +func TestAccSearchService_freeSku(t *testing.T) { // Regression test case for issue #10151 data := acceptance.BuildTestData(t, "azurerm_search_service", "test") r := SearchServiceResource{} @@ -47,13 +47,13 @@ func TestAccSearchService_basicFree(t *testing.T) { }) } -func TestAccSearchService_basicBasic(t *testing.T) { +func TestAccSearchService_standardSku(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_search_service", "test") r := SearchServiceResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data, "basic"), + Config: r.basic(data, "standard"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -309,8 +309,7 @@ func TestAccSearchService_apiAccessControlRbacError(t *testing.T) { data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.apiAccessControlBoth(data, true, "http401WithBearerChallenge"), - Check: acceptance.ComposeTestCheckFunc(), + Config: r.apiAccessControlBoth(data, false, "http401WithBearerChallenge"), ExpectError: regexp.MustCompile("cannot be defined"), }, }) @@ -329,28 +328,28 @@ func TestAccSearchService_apiAccessControlUpdate(t *testing.T) { }, data.ImportStep(), { - Config: r.apiAccessControlApiKeysOrRBAC(data, false), + Config: r.apiAccessControlApiKeysOrRBAC(data, true), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: r.apiAccessControlApiKeysOrRBAC(data, true), + Config: r.apiAccessControlApiKeysOrRBAC(data, false), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: r.apiAccessControlBoth(data, false, "http401WithBearerChallenge"), + Config: r.apiAccessControlBoth(data, true, "http401WithBearerChallenge"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: r.apiAccessControlBoth(data, false, "http403"), + Config: r.apiAccessControlBoth(data, true, "http403"), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -366,7 +365,7 @@ func TestAccSearchService_apiAccessControlUpdate(t *testing.T) { }) } -func (t SearchServiceResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { +func (r SearchServiceResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := services.ParseSearchServiceID(state.ID) if err != nil { return nil, err @@ -403,10 +402,6 @@ resource "azurerm_search_service" "test" { resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location sku = "%s" - - tags = { - environment = "staging" - } } `, template, data.RandomInteger, sku) } @@ -421,10 +416,6 @@ resource "azurerm_search_service" "import" { resource_group_name = azurerm_search_service.test.resource_group_name location = azurerm_search_service.test.location sku = azurerm_search_service.test.sku - - tags = { - environment = "staging" - } } `, template) } @@ -471,12 +462,7 @@ resource "azurerm_search_service" "test" { resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location sku = "standard" - - allowed_ips = ["168.1.5.65", "1.2.3.0/24"] - - tags = { - environment = "staging" - } + allowed_ips = ["168.1.5.65", "1.2.3.0/24"] } `, template, data.RandomInteger) } @@ -499,10 +485,6 @@ resource "azurerm_search_service" "test" { identity { type = "SystemAssigned" } - - tags = { - environment = "staging" - } } `, template, data.RandomInteger) } @@ -522,10 +504,6 @@ resource "azurerm_search_service" "test" { location = azurerm_resource_group.test.location sku = "%s" hosting_mode = "highDensity" - - tags = { - environment = "staging" - } } `, template, data.RandomInteger, sku) } @@ -584,15 +562,11 @@ resource "azurerm_search_service" "test" { sku = "standard" customer_managed_key_enforcement_enabled = %t - - tags = { - environment = "staging" - } } `, template, data.RandomInteger, enforceCustomerManagedKey) } -func (r SearchServiceResource) apiAccessControlApiKeysOrRBAC(data acceptance.TestData, localAuthenticationDisabled bool) string { +func (r SearchServiceResource) apiAccessControlApiKeysOrRBAC(data acceptance.TestData, localAuthenticationEnabled bool) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -607,16 +581,12 @@ resource "azurerm_search_service" "test" { location = azurerm_resource_group.test.location sku = "standard" - local_authentication_disabled = %t - - tags = { - environment = "staging" - } + local_authentication_enabled = %t } -`, template, data.RandomInteger, localAuthenticationDisabled) +`, template, data.RandomInteger, localAuthenticationEnabled) } -func (r SearchServiceResource) apiAccessControlBoth(data acceptance.TestData, localAuthenticationDisabled bool, authenticationFailureMode string) string { +func (r SearchServiceResource) apiAccessControlBoth(data acceptance.TestData, localAuthenticationEnabled bool, authenticationFailureMode string) string { template := r.template(data) return fmt.Sprintf(` provider "azurerm" { @@ -631,12 +601,8 @@ resource "azurerm_search_service" "test" { location = azurerm_resource_group.test.location sku = "standard" - local_authentication_disabled = %t - authentication_failure_mode = "%s" - - tags = { - environment = "staging" - } + local_authentication_enabled = %t + authentication_failure_mode = "%s" } -`, template, data.RandomInteger, localAuthenticationDisabled, authenticationFailureMode) +`, template, data.RandomInteger, localAuthenticationEnabled, authenticationFailureMode) } diff --git a/website/docs/r/search_service.html.markdown b/website/docs/r/search_service.html.markdown index de47b1782f9d..29b01303d698 100644 --- a/website/docs/r/search_service.html.markdown +++ b/website/docs/r/search_service.html.markdown @@ -10,7 +10,7 @@ description: |- Manages a Search Service. -## Example Usage +## Example Usage (supporting API Keys) ```hcl resource "azurerm_resource_group" "example" { @@ -26,6 +26,43 @@ resource "azurerm_search_service" "example" { } ``` +## Example Usage (using both AzureAD and API Keys) + +```hcl +resource "azurerm_resource_group" "example" { + name = "example-resources" + location = "West Europe" +} + +resource "azurerm_search_service" "example" { + name = "example-resource" + resource_group_name = azurerm_resource_group.example.name + location = azurerm_resource_group.example.location + sku = "standard" + + local_authentication_enabled = true + authentication_failure_mode = "http403" +} +``` + +## Example Usage (supporting only AzureAD Authentication) + +```hcl +resource "azurerm_resource_group" "example" { + name = "example-resources" + location = "West Europe" +} + +resource "azurerm_search_service" "example" { + name = "example-resource" + resource_group_name = azurerm_resource_group.example.name + location = azurerm_resource_group.example.location + sku = "standard" + + local_authentication_enabled = false +} +``` + ## Arguments Reference The following arguments are supported: @@ -44,37 +81,34 @@ The following arguments are supported: --- -* `authentication_failure_mode` - (Optional) Describes what response the Search Service should return for requests that fail authentication. Possible values include `http401WithBearerChallenge` or `http403`. - -~> **NOTE:** The `authentication_failure_mode` field is only valid if the Search Service is in the `Role-based access control and API Key` mode. For more information on how to correctly configure the Search Services `API Access Control` settings, please see the `Setting API Access Control` section below. +* `allowed_ips` - (Optional) Specifies a list of inbound IPv4 or CIDRs that are allowed to access the Search Service. If the incoming IP request is from an IP address which is not included in the `allowed_ips` it will be blocked by the Search Services firewall. -* `customer_managed_key_enforcement_enabled` - (Optional) Should the Search Service enforce having non customer encrypted resources? Possible values include `true` or `false`. If `true` the Search Service will be marked as `non-compliant` if there are one or more non customer encrypted resources, if `false` no enforcement will be made and the Search Service can contain one or more non customer encrypted resources. Defaults to `false`. +-> **NOTE:** The `allowed_ips` are only applied if the `public_network_access_enabled` field has been set to `true`, else all traffic over the public interface will be rejected, even if the `allowed_ips` field has been defined. When the `public_network_access_enabled` field has been set to `false` the private endpoint connections are the only allowed access point to the Search Service. -* `hosting_mode` - (Optional) Enable high density partitions that allow for up to a 1000 indexes. Possible values are `highDensity` or `default`. Defaults to `default`. Changing this forces a new Search Service to be created. +* `authentication_failure_mode` - (Optional) Specifies the response that the Search Service should return for requests that fail authentication. Possible values include `http401WithBearerChallenge` or `http403`. --> **NOTE:** When the Search Service is in `highDensity` mode the maximum number of partitions allowed is `3`, to enable `hosting_mode` you must use a `standard3` SKU. +-> **NOTE:** `authentication_failure_mode` can only be configured when using `local_authentication_enabled` is set to `true` - which when set together specifies that both API Keys and AzureAD Authentication should be supported. -* `local_authentication_disabled` - (Optional) Should tha Search Service *not* be allowed to use API keys for authentication? Possible values include `true` or `false`. Defaults to `false`. +* `customer_managed_key_enforcement_enabled` - (Optional) Specifies whether the Search Service should enforce that non-customer resources are encrypted. Defaults to `false`. --> **NOTE:** For more information on how to correctly configure the Search Services `API Access Control` settings, please see the `Setting API Access Control` section below. +* `hosting_mode` - (Optional) Specifies the Hosting Mode, which allows for High Density partitions (that allow for up to 1000 indexes) should be supported. Possible values are `highDensity` or `default`. Defaults to `default`. Changing this forces a new Search Service to be created. -* `public_network_access_enabled` - (Optional) Whether or not public network access is allowed for this resource. Defaults to `true`. +-> **NOTE:** `hosting_mode` can only be configured when `sku` is set to `standard3`. -* `partition_count` - (Optional) The number of partitions which should be created. Possible values include `1`, `2`, `3`, `4`, `6`, or `12`. Defaults to `1`. --> **NOTE:** `partition_count` cannot be configured when using a `free` or `basic` SKU. For more information please to the [product documentation](https://learn.microsoft.com/azure/search/search-sku-tier). +* `identity` - (Optional) An `identity` block as defined below. -* `replica_count` - (Optional) The number of replica's which should be created. +* `local_authentication_enabled` - (Optional) Specifies whether the Search Service allows authenticating using API Keys? Defaults to `false`. --> **NOTE:** `replica_count` cannot be configured when using a `free` SKU. For more information please to the [product documentation](https://learn.microsoft.com/azure/search/search-sku-tier). +* `partition_count` - (Optional) Specifies the number of partitions which should be created. This field cannot be set when using a `free` or `basic` sku ([see the Microsoft documentation](https://learn.microsoft.com/azure/search/search-sku-tier)). Possible values include `1`, `2`, `3`, `4`, `6`, or `12`. Defaults to `1`. -* `allowed_ips` - (Optional) A list of inbound IPv4 or CIDRs that are allowed to access the Search Service. If the incoming IP request is from an IP address which is not included in the `allowed_ips` it will be blocked by the Search Services firewall. +-> **NOTE:** when `hosting_mode` is set to `highDensity` the maximum number of partitions allowed is `3`. --> **NOTE:** The `allowed_ips` are only applied if the `public_network_access_enabled` field has been set to `true`, else all traffic over the public interface will be rejected, even if the `allowed_ips` field has been defined. When the `public_network_access_enabled` field has been set to `false` the private endpoint connections are the only allowed access point to the Search Service. +* `public_network_access_enabled` - (Optional) Specifies whether Public Network Access is allowed for this resource. Defaults to `true`. -* `identity` - (Optional) An `identity` block as defined below. +* `replica_count` - (Optional) Specifies the number of Replica's which should be created for this Search Service. This field cannot be set when using a `free` sku ([see the Microsoft documentation](https://learn.microsoft.com/azure/search/search-sku-tier)). -* `tags` - (Optional) A mapping of tags which should be assigned to the Search Service. +* `tags` - (Optional) Specifies a mapping of tags which should be assigned to this Search Service. --- @@ -90,8 +124,6 @@ In addition to the Arguments listed above - the following Attributes are exporte * `id` - The ID of the Search Service. -* `customer_managed_key_enforcement_compliance` - Describes whether the Search Service is `compliant` or not with respect to having non customer encrypted resources. - * `primary_key` - The Primary Key used for Search Service Administration. * `query_keys` - A `query_keys` block as defined below. @@ -116,20 +148,6 @@ An `identity` block exports the following: --- -## Setting API Access Control: - -The values of the `local_authentication_disabled` and `authentication_failure_mode` fields will determin which `API Access Control` mode the Search Service will operate under. To correctly configure your Search Service to your desired `API Access Control` mode please configure your Search Service based on the field values in the below table. - -| Field Name | Value | API Access Control Mode | -|------------------------------------------------------------------|:---------------------------------------------------------:|:----------------------------------------:| -| `local_authentication_disabled`
`authentication_failure_mode` | `false`
`` | `API key` (`Default`) | -| `local_authentication_disabled`
`authentication_failure_mode` | `false`
`authentication_failure_mode` as defined above | `Role-based access control and API Key` | -| `local_authentication_disabled`
`authentication_failure_mode` | `true`
`` | `Role-based access control` | - --> **NOTE:** When the Search Service is in `Role-based access contol` (e.g. `local_authentication_disabled` is `true`) the `authentication_failure_mode` field *cannot* be defined. - ---- - ## Timeouts The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/language/resources/syntax#operation-timeouts) for certain actions: