From b7d5a5b0d6bdaf72615aaed7f96e7bbe3d5e8628 Mon Sep 17 00:00:00 2001 From: kt Date: Mon, 3 Dec 2018 22:11:42 -0800 Subject: [PATCH 1/4] deprecated azurerm_app_service_plan properties property --- azurerm/resource_arm_app_service.go | 1 + azurerm/resource_arm_app_service_plan.go | 91 +++++++++++++++---- azurerm/resource_arm_app_service_plan_test.go | 82 +++++++++++++++-- 3 files changed, 146 insertions(+), 28 deletions(-) diff --git a/azurerm/resource_arm_app_service.go b/azurerm/resource_arm_app_service.go index 2046f29ef421..6d860a17cf16 100644 --- a/azurerm/resource_arm_app_service.go +++ b/azurerm/resource_arm_app_service.go @@ -447,6 +447,7 @@ func resourceArmAppServiceRead(d *schema.ResourceData, meta interface{}) error { if err := d.Set("app_settings", flattenAppServiceAppSettings(appSettingsResp.Properties)); err != nil { return err } + if err := d.Set("connection_string", flattenAppServiceConnectionStrings(connectionStringsResp.Properties)); err != nil { return err } diff --git a/azurerm/resource_arm_app_service_plan.go b/azurerm/resource_arm_app_service_plan.go index 230bb495008c..237c15d5b901 100644 --- a/azurerm/resource_arm_app_service_plan.go +++ b/azurerm/resource_arm_app_service_plan.go @@ -2,6 +2,7 @@ package azurerm import ( "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" "log" "regexp" @@ -17,6 +18,7 @@ func resourceArmAppServicePlan() *schema.Resource { Read: resourceArmAppServicePlanRead, Update: resourceArmAppServicePlanCreateUpdate, Delete: resourceArmAppServicePlanDelete, + Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, @@ -46,7 +48,7 @@ func resourceArmAppServicePlan() *schema.Resource { "Linux", "Windows", }, true), - DiffSuppressFunc: ignoreCaseDiffSuppressFunc, + DiffSuppressFunc: suppress.CaseDifference, }, "sku": { @@ -73,31 +75,63 @@ func resourceArmAppServicePlan() *schema.Resource { }, "properties": { - Type: schema.TypeList, - Optional: true, - Computed: true, - MaxItems: 1, + Type: schema.TypeList, + Optional: true, + Computed: true, + MaxItems: 1, + Deprecated: "These properties have been moved to the top level", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "app_service_environment_id": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Computed: true, + Deprecated: "This property has been moved to the top level", + ConflictsWith: []string{"app_service_environment_id"}, }, + "reserved": { - Type: schema.TypeBool, - Optional: true, - Default: false, + Type: schema.TypeBool, + Optional: true, + Computed: true, + Deprecated: "This property has been moved to the top level", + ConflictsWith: []string{"reserved"}, }, + "per_site_scaling": { - Type: schema.TypeBool, - Optional: true, - Default: false, + Type: schema.TypeBool, + Optional: true, + Computed: true, + Deprecated: "This property has been moved to the top level", + ConflictsWith: []string{"per_site_scaling"}, }, }, }, }, + "app_service_environment_id": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Computed: true, + ConflictsWith: []string{"properties.0.app_service_environment_id"}, + }, + + "per_site_scaling": { + Type: schema.TypeBool, + Optional: true, + Computed: true, + ConflictsWith: []string{"properties.0.per_site_scaling"}, + }, + + "reserved": { + Type: schema.TypeBool, + Optional: true, + Computed: true, + ConflictsWith: []string{"properties.0.reserved"}, + }, + "maximum_number_of_workers": { Type: schema.TypeInt, Computed: true, @@ -125,10 +159,24 @@ func resourceArmAppServicePlanCreateUpdate(d *schema.ResourceData, meta interfac appServicePlan := web.AppServicePlan{ Location: &location, - AppServicePlanProperties: properties, Kind: &kind, - Tags: expandTags(tags), Sku: &sku, + Tags: expandTags(tags), + AppServicePlanProperties: properties, + } + + if v, exists := d.GetOkExists("app_service_environment_id"); exists { + appServicePlan.AppServicePlanProperties.HostingEnvironmentProfile = &web.HostingEnvironmentProfile{ + ID: utils.String(v.(string)), + } + } + + if v, exists := d.GetOkExists("per_site_scaling"); exists { + appServicePlan.AppServicePlanProperties.PerSiteScaling = utils.Bool(v.(bool)) + } + + if v, exists := d.GetOkExists("reserved"); exists { + appServicePlan.AppServicePlanProperties.Reserved = utils.Bool(v.(bool)) } future, err := client.CreateOrUpdate(ctx, resGroup, name, appServicePlan) @@ -136,8 +184,7 @@ func resourceArmAppServicePlanCreateUpdate(d *schema.ResourceData, meta interfac return fmt.Errorf("Error creating/updating App Service Plan %q (Resource Group %q): %+v", name, resGroup, err) } - err = future.WaitForCompletionRef(ctx, client.Client) - if err != nil { + if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { return fmt.Errorf("Error waiting for the create/update of App Service Plan %q (Resource Group %q): %+v", name, resGroup, err) } @@ -191,9 +238,13 @@ func resourceArmAppServicePlanRead(d *schema.ResourceData, meta interface{}) err return fmt.Errorf("Error setting `properties`: %+v", err) } - if props.MaximumNumberOfWorkers != nil { - d.Set("maximum_number_of_workers", int(*props.MaximumNumberOfWorkers)) + if profile := props.HostingEnvironmentProfile; profile != nil { + d.Set("app_service_environment_id", profile.ID) } + + d.Set("maximum_number_of_workers", props.MaximumNumberOfWorkers) + d.Set("per_site_scaling", props.PerSiteScaling) + d.Set("reserved", props.Reserved) } if err := d.Set("sku", flattenAppServicePlanSku(resp.Sku)); err != nil { diff --git a/azurerm/resource_arm_app_service_plan_test.go b/azurerm/resource_arm_app_service_plan_test.go index 210c9fd73bea..4448c63de495 100644 --- a/azurerm/resource_arm_app_service_plan_test.go +++ b/azurerm/resource_arm_app_service_plan_test.go @@ -89,6 +89,12 @@ func TestAccAzureRMAppServicePlan_basicLinux(t *testing.T) { testCheckAzureRMAppServicePlanExists(resourceName), ), }, + { + Config: testAccAzureRMAppServicePlan_basicLinuxNew(ri, testLocation()), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMAppServicePlanExists(resourceName), + ), + }, { ResourceName: resourceName, ImportState: true, @@ -150,8 +156,6 @@ func TestAccAzureRMAppServicePlan_premiumWindowsUpdated(t *testing.T) { resourceName := "azurerm_app_service_plan.test" ri := acctest.RandInt() location := testLocation() - config := testAccAzureRMAppServicePlan_premiumWindows(ri, location) - updatedConfig := testAccAzureRMAppServicePlan_premiumWindowsUpdated(ri, location) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -159,19 +163,24 @@ func TestAccAzureRMAppServicePlan_premiumWindowsUpdated(t *testing.T) { CheckDestroy: testCheckAzureRMAppServicePlanDestroy, Steps: []resource.TestStep{ { - Config: config, + Config: testAccAzureRMAppServicePlan_premiumWindows(ri, location), Check: resource.ComposeTestCheckFunc( testCheckAzureRMAppServicePlanExists(resourceName), resource.TestCheckResourceAttr(resourceName, "sku.0.capacity", "1"), ), }, { - Config: updatedConfig, + Config: testAccAzureRMAppServicePlan_premiumWindowsUpdated(ri, location), Check: resource.ComposeTestCheckFunc( testCheckAzureRMAppServicePlanExists(resourceName), resource.TestCheckResourceAttr(resourceName, "sku.0.capacity", "2"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } @@ -179,7 +188,6 @@ func TestAccAzureRMAppServicePlan_premiumWindowsUpdated(t *testing.T) { func TestAccAzureRMAppServicePlan_completeWindows(t *testing.T) { resourceName := "azurerm_app_service_plan.test" ri := acctest.RandInt() - config := testAccAzureRMAppServicePlan_completeWindows(ri, testLocation()) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -187,13 +195,21 @@ func TestAccAzureRMAppServicePlan_completeWindows(t *testing.T) { CheckDestroy: testCheckAzureRMAppServicePlanDestroy, Steps: []resource.TestStep{ { - Config: config, + Config: testAccAzureRMAppServicePlan_completeWindows(ri, testLocation()), Check: resource.ComposeTestCheckFunc( testCheckAzureRMAppServicePlanExists(resourceName), resource.TestCheckResourceAttr(resourceName, "properties.0.per_site_scaling", "true"), resource.TestCheckResourceAttr(resourceName, "properties.0.reserved", "false"), ), }, + { + Config: testAccAzureRMAppServicePlan_completeWindowsNew(ri, testLocation()), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMAppServicePlanExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "per_site_scaling", "true"), + resource.TestCheckResourceAttr(resourceName, "reserved", "false"), + ), + }, { ResourceName: resourceName, ImportState: true, @@ -206,7 +222,6 @@ func TestAccAzureRMAppServicePlan_completeWindows(t *testing.T) { func TestAccAzureRMAppServicePlan_consumptionPlan(t *testing.T) { resourceName := "azurerm_app_service_plan.test" ri := acctest.RandInt() - config := testAccAzureRMAppServicePlan_consumptionPlan(ri, testLocation()) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -214,7 +229,7 @@ func TestAccAzureRMAppServicePlan_consumptionPlan(t *testing.T) { CheckDestroy: testCheckAzureRMAppServicePlanDestroy, Steps: []resource.TestStep{ { - Config: config, + Config: testAccAzureRMAppServicePlan_consumptionPlan(ri, testLocation()), Check: resource.ComposeTestCheckFunc( testCheckAzureRMAppServicePlanExists(resourceName), resource.TestCheckResourceAttr(resourceName, "sku.0.tier", "Dynamic"), @@ -326,6 +341,29 @@ resource "azurerm_app_service_plan" "test" { `, rInt, location, rInt) } +func testAccAzureRMAppServicePlan_basicLinuxNew(rInt int, location string) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_app_service_plan" "test" { + name = "acctestASP-%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + kind = "Linux" + + sku { + tier = "Basic" + size = "B1" + } + + reserved = true +} +`, rInt, location, rInt) +} + func testAccAzureRMAppServicePlan_standardWindows(rInt int, location string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { @@ -417,6 +455,34 @@ resource "azurerm_app_service_plan" "test" { `, rInt, location, rInt) } +func testAccAzureRMAppServicePlan_completeWindowsNew(rInt int, location string) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_app_service_plan" "test" { + name = "acctestASP-%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + kind = "Windows" + + sku { + tier = "Standard" + size = "S1" + } + + per_site_scaling = true + reserved = false + + tags { + environment = "Test" + } +} +`, rInt, location, rInt) +} + func testAccAzureRMAppServicePlan_consumptionPlan(rInt int, location string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { From c9d0167320cbbc75a3407380b2cbf6a79a206c3b Mon Sep 17 00:00:00 2001 From: kt Date: Tue, 4 Dec 2018 09:40:52 -0800 Subject: [PATCH 2/4] Updated documentation --- azurerm/resource_arm_app_service_plan_test.go | 4 ++++ website/docs/r/app_service_plan.html.markdown | 16 +++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/azurerm/resource_arm_app_service_plan_test.go b/azurerm/resource_arm_app_service_plan_test.go index 4448c63de495..3fdcf29e394f 100644 --- a/azurerm/resource_arm_app_service_plan_test.go +++ b/azurerm/resource_arm_app_service_plan_test.go @@ -63,6 +63,8 @@ func TestAccAzureRMAppServicePlan_basicWindows(t *testing.T) { Config: testAccAzureRMAppServicePlan_basicWindows(ri, testLocation()), Check: resource.ComposeTestCheckFunc( testCheckAzureRMAppServicePlanExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "per_site_scaling", "false"), + resource.TestCheckResourceAttr(resourceName, "reserved", "false"), ), }, { @@ -93,6 +95,8 @@ func TestAccAzureRMAppServicePlan_basicLinux(t *testing.T) { Config: testAccAzureRMAppServicePlan_basicLinuxNew(ri, testLocation()), Check: resource.ComposeTestCheckFunc( testCheckAzureRMAppServicePlanExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "per_site_scaling", "false"), + resource.TestCheckResourceAttr(resourceName, "reserved", "true"), ), }, { diff --git a/website/docs/r/app_service_plan.html.markdown b/website/docs/r/app_service_plan.html.markdown index a62ba6891ccb..45e98d7e16b7 100644 --- a/website/docs/r/app_service_plan.html.markdown +++ b/website/docs/r/app_service_plan.html.markdown @@ -92,7 +92,13 @@ The following arguments are supported: * `sku` - (Required) A `sku` block as documented below. -* `properties` - (Optional) A `properties` block as documented below. +* `app_service_environment_id` - (Optional) The ID of the App Service Environment where the App Service Plan should be located. Changing forces a new resource to be created. + +~> **NOTE:** Attaching to an App Service Environment requires the App Service Plan use a `Premium` SKU (when using an ASEv1) and the `Isolated` SKU (for an ASEv2). + +* `reserved` - (Optional) Is this App Service Plan `Reserved`. Defaults to `false`. + +* `per_site_scaling` - (Optional) Can Apps assigned to this App Service Plan be scaled independently? If set to `false` apps assigned to this plan will scale to all instances of the plan. Defaults to `false`. * `tags` - (Optional) A mapping of tags to assign to the resource. @@ -104,15 +110,7 @@ The following arguments are supported: * `capacity` - (Optional) Specifies the number of workers associated with this App Service Plan. -`properties` supports the following: -* `app_service_environment_id` - (Optional) The ID of the App Service Environment where the App Service Plan should be located. Changing forces a new resource to be created. - -~> **NOTE:** Attaching to an App Service Environment requires the App Service Plan use a `Premium` SKU (when using an ASEv1) and the `Isolated` SKU (for an ASEv2). - -* `reserved` - (Optional) Is this App Service Plan `Reserved`. Defaults to `false`. - -* `per_site_scaling` - (Optional) Can Apps assigned to this App Service Plan be scaled independently? If set to `false` apps assigned to this plan will scale to all instances of the plan. Defaults to `false`. ## Attributes Reference From 966956ebfc6748a0f3bad3cee5a60e96d4a05500 Mon Sep 17 00:00:00 2001 From: kt Date: Tue, 4 Dec 2018 09:52:17 -0800 Subject: [PATCH 3/4] address PR #2442 comments --- azurerm/resource_arm_app_service_plan.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/azurerm/resource_arm_app_service_plan.go b/azurerm/resource_arm_app_service_plan.go index 237c15d5b901..a4237e9fbc7e 100644 --- a/azurerm/resource_arm_app_service_plan.go +++ b/azurerm/resource_arm_app_service_plan.go @@ -242,7 +242,10 @@ func resourceArmAppServicePlanRead(d *schema.ResourceData, meta interface{}) err d.Set("app_service_environment_id", profile.ID) } - d.Set("maximum_number_of_workers", props.MaximumNumberOfWorkers) + if props.MaximumNumberOfWorkers != nil { + d.Set("maximum_number_of_workers", int(*props.MaximumNumberOfWorkers)) + } + d.Set("per_site_scaling", props.PerSiteScaling) d.Set("reserved", props.Reserved) } From ba43ab6decd310e07068ae97d4b4abaeacef713e Mon Sep 17 00:00:00 2001 From: kt Date: Tue, 4 Dec 2018 09:57:05 -0800 Subject: [PATCH 4/4] make goimport --- azurerm/resource_arm_app_service_plan.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/azurerm/resource_arm_app_service_plan.go b/azurerm/resource_arm_app_service_plan.go index a4237e9fbc7e..747e94a4fa87 100644 --- a/azurerm/resource_arm_app_service_plan.go +++ b/azurerm/resource_arm_app_service_plan.go @@ -2,10 +2,11 @@ package azurerm import ( "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" "log" "regexp" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" + "github.com/Azure/azure-sdk-for-go/services/web/mgmt/2018-02-01/web" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation"