azurerm_analysis_services_server - fix multiple issues (#31870)

Author: sreallymatt

internal/services/analysisservices/analysis_services_server_resource.go

@@ -92,8 +92,9 @@ func resourceAnalysisServicesServer() *pluginsdk.Resource {
 			},
 
 			"ipv4_firewall_rule": {
-				Type:     pluginsdk.TypeSet,
-				Optional: true,
+				Type:         pluginsdk.TypeSet,
+				Optional:     true,
+				RequiredWith: []string{"power_bi_service_enabled"},
 				Elem: &pluginsdk.Resource{
 					Schema: map[string]*pluginsdk.Schema{
 						"name": {
@@ -177,15 +178,6 @@ func resourceAnalysisServicesServerCreate(d *pluginsdk.ResourceData, meta interf
 		Tags: tags.Expand(d.Get("tags").(map[string]interface{})),
 	}
 
-	if v, ok := d.GetOk("power_bi_service_enabled"); ok {
-		if analysisServicesServer.Properties.IPV4FirewallSettings == nil {
-			analysisServicesServer.Properties.IPV4FirewallSettings = &servers.IPv4FirewallSettings{
-				FirewallRules: pointer.To(make([]servers.IPv4FirewallRule, 0)),
-			}
-		}
-		analysisServicesServer.Properties.IPV4FirewallSettings.EnablePowerBIService = pointer.To(v.(bool))
-	}
-
 	if querypoolConnectionMode, ok := d.GetOk("querypool_connection_mode"); ok {
 		analysisServicesServer.Properties.QuerypoolConnectionMode = pointer.To(servers.ConnectionMode(querypoolConnectionMode.(string)))
 	}
@@ -315,15 +307,6 @@ func resourceAnalysisServicesServerUpdate(d *pluginsdk.ResourceData, meta interf
 		},
 	}
 
-	if d.HasChange("power_bi_service_enabled") {
-		if analysisServicesServer.Properties.IPV4FirewallSettings == nil {
-			analysisServicesServer.Properties.IPV4FirewallSettings = &servers.IPv4FirewallSettings{
-				FirewallRules: pointer.To(make([]servers.IPv4FirewallRule, 0)),
-			}
-		}
-		analysisServicesServer.Properties.IPV4FirewallSettings.EnablePowerBIService = pointer.To(d.Get("power_bi_service_enabled").(bool))
-	}
-
 	if containerUri, ok := d.GetOk("backup_blob_container_uri"); ok {
 		analysisServicesServer.Properties.BackupBlobContainerUri = pointer.To(containerUri.(string))
 	}
@@ -374,25 +357,32 @@ func expandAnalysisServicesServerAdminUsers(d *pluginsdk.ResourceData) *servers.
 }
 
 func expandAnalysisServicesServerFirewallSettings(d *pluginsdk.ResourceData) *servers.IPv4FirewallSettings {
-	firewallRules := d.Get("ipv4_firewall_rule").(*pluginsdk.Set).List()
+	fwRules := make([]servers.IPv4FirewallRule, 0)
+	result := servers.IPv4FirewallSettings{}
 
+	if !pluginsdk.IsExplicitlyNullInConfig(d, "power_bi_service_enabled") {
+		result.EnablePowerBIService = pointer.To(d.Get("power_bi_service_enabled").(bool))
+		// when `power_bi_service_enabled` is specified, we must send at least an empty array for `FirewallRules`
+		// otherwise the API errors out with a 400.
+		result.FirewallRules = &fwRules
+	}
+
+	firewallRules := d.Get("ipv4_firewall_rule").(*pluginsdk.Set).List()
 	if len(firewallRules) == 0 {
-		return nil
+		return &result
 	}
 
-	firewallSettings := servers.IPv4FirewallSettings{}
-	fwRules := make([]servers.IPv4FirewallRule, len(firewallRules))
-	for i, v := range firewallRules {
+	for _, v := range firewallRules {
 		fwRule := v.(map[string]interface{})
-		fwRules[i] = servers.IPv4FirewallRule{
+		fwRules = append(fwRules, servers.IPv4FirewallRule{
 			FirewallRuleName: pointer.To(fwRule["name"].(string)),
 			RangeStart:       pointer.To(fwRule["range_start"].(string)),
 			RangeEnd:         pointer.To(fwRule["range_end"].(string)),
-		}
+		})
 	}
-	firewallSettings.FirewallRules = &fwRules
+	result.FirewallRules = &fwRules
 
-	return &firewallSettings
+	return &result
 }
 
 func flattenAnalysisServicesServerFirewallSettings(serverProperties *servers.AnalysisServicesServerProperties) (*bool, *pluginsdk.Set) {

internal/services/analysisservices/analysis_services_server_resource_test.go

@@ -93,29 +93,52 @@ func TestAccAnalysisServicesServer_firewallSettings(t *testing.T) {
 
 	data.ResourceTest(t, r, []acceptance.TestStep{
 		{
-			Config: r.firewallSettings1(data, true),
+			Config: r.firewallSettingsSingleRule(data, false),
+			Check: acceptance.ComposeTestCheckFunc(
+				check.That(data.ResourceName).ExistsInAzure(r),
+				check.That(data.ResourceName).Key("ipv4_firewall_rule.#").HasValue("1"),
+			),
+		},
+		data.ImportStep(),
+		{
+			Config: r.firewallSettingsNoRules(data, true),
 			Check: acceptance.ComposeTestCheckFunc(
 				check.That(data.ResourceName).ExistsInAzure(r),
 				check.That(data.ResourceName).Key("ipv4_firewall_rule.#").HasValue("0"),
 			),
 		},
 		data.ImportStep(),
 		{
-			Config: r.firewallSettings2(data, false),
+			Config: r.firewallSettingsSingleRule(data, true),
 			Check: acceptance.ComposeTestCheckFunc(
 				check.That(data.ResourceName).ExistsInAzure(r),
 				check.That(data.ResourceName).Key("ipv4_firewall_rule.#").HasValue("1"),
 			),
 		},
 		data.ImportStep(),
 		{
-			Config: r.firewallSettings3(data, true),
+			Config: r.firewallSettingsMultipleRules(data, true),
+			Check: acceptance.ComposeTestCheckFunc(
+				check.That(data.ResourceName).ExistsInAzure(r),
+				check.That(data.ResourceName).Key("ipv4_firewall_rule.#").HasValue("2"),
+			),
+		},
+		data.ImportStep(),
+		{
+			Config: r.firewallSettingsMultipleRules(data, false),
 			Check: acceptance.ComposeTestCheckFunc(
 				check.That(data.ResourceName).ExistsInAzure(r),
 				check.That(data.ResourceName).Key("ipv4_firewall_rule.#").HasValue("2"),
 			),
 		},
 		data.ImportStep(),
+		{
+			Config: r.firewallSettingsRemoved(data),
+			Check: acceptance.ComposeTestCheckFunc(
+				check.That(data.ResourceName).ExistsInAzure(r),
+			),
+		},
+		data.ImportStep(),
 	})
 }
 
@@ -300,7 +323,7 @@ resource "azurerm_analysis_services_server" "test" {
 `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, connectionMode)
 }
 
-func (t AnalysisServicesServerResource) firewallSettings1(data acceptance.TestData, enablePowerBIService bool) string {
+func (t AnalysisServicesServerResource) firewallSettingsNoRules(data acceptance.TestData, enablePowerBIService bool) string {
 	return fmt.Sprintf(`
 provider "azurerm" {
   features {}
@@ -321,7 +344,7 @@ resource "azurerm_analysis_services_server" "test" {
 `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, enablePowerBIService)
 }
 
-func (t AnalysisServicesServerResource) firewallSettings2(data acceptance.TestData, enablePowerBIService bool) string {
+func (t AnalysisServicesServerResource) firewallSettingsSingleRule(data acceptance.TestData, enablePowerBIService bool) string {
 	return fmt.Sprintf(`
 provider "azurerm" {
   features {}
@@ -348,7 +371,7 @@ resource "azurerm_analysis_services_server" "test" {
 `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, enablePowerBIService)
 }
 
-func (t AnalysisServicesServerResource) firewallSettings3(data acceptance.TestData, enablePowerBIService bool) string {
+func (t AnalysisServicesServerResource) firewallSettingsMultipleRules(data acceptance.TestData, enablePowerBIService bool) string {
 	return fmt.Sprintf(`
 provider "azurerm" {
   features {}
@@ -381,6 +404,26 @@ resource "azurerm_analysis_services_server" "test" {
 `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, enablePowerBIService)
 }
 
+func (t AnalysisServicesServerResource) firewallSettingsRemoved(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+provider "azurerm" {
+  features {}
+}
+
+resource "azurerm_resource_group" "test" {
+  name     = "acctestRG-analysis-%[1]d"
+  location = "%[2]s"
+}
+
+resource "azurerm_analysis_services_server" "test" {
+  name                = "acctestass%[1]d"
+  location            = azurerm_resource_group.test.location
+  resource_group_name = azurerm_resource_group.test.name
+  sku                 = "B1"
+}
+`, data.RandomInteger, data.Locations.Primary)
+}
+
 func (t AnalysisServicesServerResource) adminUsers(data acceptance.TestData, adminUsers []string) string {
 	return fmt.Sprintf(`
 provider "azurerm" {

website/docs/r/analysis_services_server.html.markdown

@@ -60,6 +60,8 @@ The following arguments are supported:
 
 * `power_bi_service_enabled` - (Optional) Indicates if the Power BI service is allowed to access or not.
 
+~> **Note:** `power_bi_service_enabled` is required when `ipv4_firewall_rule` is defined.
+
 * `ipv4_firewall_rule` - (Optional) One or more `ipv4_firewall_rule` block(s) as defined below.
 
 * `tags` - (Optional) A mapping of tags to assign to the resource.