From 7e999f49038d65f262263c3065fbf6c9728a7c71 Mon Sep 17 00:00:00 2001 From: Matthew Date: Wed, 27 Sep 2023 16:07:31 -0700 Subject: [PATCH] error when service_plan_id is the same as the parent service_plan_id --- .../appservice/linux_web_app_slot_resource.go | 45 ++++++++++++---- .../linux_web_app_slot_resource_test.go | 50 ++++++++++++++++++ .../windows_web_app_slot_resource.go | 45 ++++++++++++---- .../windows_web_app_slot_resource_test.go | 52 ++++++++++++++++++- .../docs/r/linux_web_app_slot.html.markdown | 2 + .../docs/r/windows_web_app_slot.html.markdown | 2 + 6 files changed, 177 insertions(+), 19 deletions(-) diff --git a/internal/services/appservice/linux_web_app_slot_resource.go b/internal/services/appservice/linux_web_app_slot_resource.go index 93821b25053c..6f2c4d9cddfe 100644 --- a/internal/services/appservice/linux_web_app_slot_resource.go +++ b/internal/services/appservice/linux_web_app_slot_resource.go @@ -285,20 +285,28 @@ func (r LinuxWebAppSlotResource) Create() sdk.ResourceFunc { } var servicePlanId *parse.ServicePlanId + if webApp.SiteProperties == nil || webApp.SiteProperties.ServerFarmID == nil { + return fmt.Errorf("could not determine Service Plan ID for %s: %+v", id, err) + } + + servicePlanId, err = parse.ServicePlanID(*webApp.SiteProperties.ServerFarmID) + if err != nil { + return err + } + if webAppSlot.ServicePlanID != "" { - servicePlanId, err = parse.ServicePlanID(webAppSlot.ServicePlanID) + newServicePlanId, err := parse.ServicePlanID(webAppSlot.ServicePlanID) if err != nil { return err } - } else { - if props := webApp.SiteProperties; props == nil || props.ServerFarmID == nil { - return fmt.Errorf("could not determine Service Plan ID for %s: %+v", id, err) - } else { - servicePlanId, err = parse.ServicePlanID(*props.ServerFarmID) - if err != nil { - return err - } + // we only set `service_plan_id` when it differs from the parent `service_plan_id` which is causing issues + // https://github.com/hashicorp/terraform-provider-azurerm/issues/21024 + // we'll error here if the `service_plan_id` equals the parent `service_plan_id` + if strings.EqualFold(newServicePlanId.ID(), servicePlanId.ID()) { + return fmt.Errorf("`service_plan_id` should only be specified when it differs from the `service_plan_id` of the associated Web App") } + + servicePlanId = newServicePlanId } existing, err := client.GetSlot(ctx, id.ResourceGroup, id.SiteName, id.SlotName) @@ -678,6 +686,18 @@ func (r LinuxWebAppSlotResource) Update() sdk.ResourceFunc { } if metadata.ResourceData.HasChange("service_plan_id") { + webApp, err := client.Get(ctx, id.ResourceGroup, id.SiteName) + if err != nil { + return fmt.Errorf("reading parent Windows Web App for %s: %+v", id, err) + } + if webApp.SiteProperties == nil || webApp.SiteProperties.ServerFarmID == nil { + return fmt.Errorf("could not determine Service Plan ID for %s: %+v", id, err) + } + parentServicePlanId, err := parse.ServicePlanID(*webApp.SiteProperties.ServerFarmID) + if err != nil { + return err + } + o, n := metadata.ResourceData.GetChange("service_plan_id") oldPlan, err := parse.ServicePlanID(o.(string)) if err != nil { @@ -688,6 +708,13 @@ func (r LinuxWebAppSlotResource) Update() sdk.ResourceFunc { if err != nil { return err } + + // we only set `service_plan_id` when it differs from the parent `service_plan_id` which is causing issues + // https://github.com/hashicorp/terraform-provider-azurerm/issues/21024 + // we'll error here if the `service_plan_id` equals the parent `service_plan_id` + if strings.EqualFold(newPlan.ID(), parentServicePlanId.ID()) { + return fmt.Errorf("`service_plan_id` should only be specified when it differs from the `service_plan_id` of the associated Web App") + } locks.ByID(oldPlan.ID()) defer locks.UnlockByID(oldPlan.ID()) locks.ByID(newPlan.ID()) diff --git a/internal/services/appservice/linux_web_app_slot_resource_test.go b/internal/services/appservice/linux_web_app_slot_resource_test.go index 42d209d61a05..2d1bdac070e6 100644 --- a/internal/services/appservice/linux_web_app_slot_resource_test.go +++ b/internal/services/appservice/linux_web_app_slot_resource_test.go @@ -6,6 +6,7 @@ package appservice_test import ( "context" "fmt" + "regexp" "testing" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance" @@ -101,6 +102,29 @@ func TestAccLinuxWebAppSlot_separateStandardPlanUpdate(t *testing.T) { }) } +func TestAccLinuxWebAppSlot_parentStandardPlanError(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_linux_web_app_slot", "test") + r := LinuxWebAppSlotResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.parentStandardPlanError(data), + ExpectError: regexp.MustCompile("`service_plan_id` should only be specified when it differs from the `service_plan_id` of the associated Web App"), + }, + { + Config: r.separatePlan(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.parentStandardPlanError(data), + ExpectError: regexp.MustCompile("`service_plan_id` should only be specified when it differs from the `service_plan_id` of the associated Web App"), + }, + }) +} + // Complete func TestAccLinuxWebAppSlot_complete(t *testing.T) { @@ -2519,6 +2543,32 @@ resource "azurerm_linux_web_app_slot" "test" { `, r.baseTemplate(data), data.RandomInteger, SkuStandardPlan, SkuPremiumPlan) } +func (r LinuxWebAppSlotResource) parentStandardPlanError(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%s + +resource "azurerm_service_plan" "test2" { + name = "acctestASP2-%[2]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + os_type = "Linux" + sku_name = "%[3]s" +} + +resource "azurerm_linux_web_app_slot" "test" { + name = "acctestWAS-%[2]d" + app_service_id = azurerm_linux_web_app.test.id + service_plan_id = azurerm_service_plan.test.id + + site_config {} +} +`, r.baseTemplate(data), data.RandomInteger, SkuStandardPlan) +} + func (r LinuxWebAppSlotResource) publicNetworkAccessDisabled(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { diff --git a/internal/services/appservice/windows_web_app_slot_resource.go b/internal/services/appservice/windows_web_app_slot_resource.go index 0b19e1406f70..2a15932db35a 100644 --- a/internal/services/appservice/windows_web_app_slot_resource.go +++ b/internal/services/appservice/windows_web_app_slot_resource.go @@ -276,20 +276,28 @@ func (r WindowsWebAppSlotResource) Create() sdk.ResourceFunc { } var servicePlanId *parse.ServicePlanId + if webApp.SiteProperties == nil || webApp.SiteProperties.ServerFarmID == nil { + return fmt.Errorf("could not determine Service Plan ID for %s: %+v", id, err) + } + + servicePlanId, err = parse.ServicePlanID(*webApp.SiteProperties.ServerFarmID) + if err != nil { + return err + } + if webAppSlot.ServicePlanID != "" { - servicePlanId, err = parse.ServicePlanID(webAppSlot.ServicePlanID) + newServicePlanId, err := parse.ServicePlanID(webAppSlot.ServicePlanID) if err != nil { return err } - } else { - if props := webApp.SiteProperties; props == nil || props.ServerFarmID == nil { - return fmt.Errorf("could not determine Service Plan ID for %s: %+v", id, err) - } else { - servicePlanId, err = parse.ServicePlanID(*props.ServerFarmID) - if err != nil { - return err - } + // we only set `service_plan_id` when it differs from the parent `service_plan_id` which is causing issues + // https://github.com/hashicorp/terraform-provider-azurerm/issues/21024 + // we'll error here if the `service_plan_id` equals the parent `service_plan_id` + if strings.EqualFold(newServicePlanId.ID(), servicePlanId.ID()) { + return fmt.Errorf("`service_plan_id` should only be specified when it differs from the `service_plan_id` of the associated Web App") } + + servicePlanId = newServicePlanId } existing, err := client.GetSlot(ctx, id.ResourceGroup, id.SiteName, id.SlotName) @@ -697,6 +705,18 @@ func (r WindowsWebAppSlotResource) Update() sdk.ResourceFunc { } if metadata.ResourceData.HasChange("service_plan_id") { + webApp, err := client.Get(ctx, id.ResourceGroup, id.SiteName) + if err != nil { + return fmt.Errorf("reading parent Windows Web App for %s: %+v", id, err) + } + if webApp.SiteProperties == nil || webApp.SiteProperties.ServerFarmID == nil { + return fmt.Errorf("could not determine Service Plan ID for %s: %+v", id, err) + } + parentServicePlanId, err := parse.ServicePlanID(*webApp.SiteProperties.ServerFarmID) + if err != nil { + return err + } + o, n := metadata.ResourceData.GetChange("service_plan_id") oldPlan, err := parse.ServicePlanID(o.(string)) if err != nil { @@ -707,6 +727,13 @@ func (r WindowsWebAppSlotResource) Update() sdk.ResourceFunc { if err != nil { return err } + + // we only set `service_plan_id` when it differs from the parent `service_plan_id` which is causing issues + // https://github.com/hashicorp/terraform-provider-azurerm/issues/21024 + // we'll error here if the `service_plan_id` equals the parent `service_plan_id` + if strings.EqualFold(newPlan.ID(), parentServicePlanId.ID()) { + return fmt.Errorf("`service_plan_id` should only be specified when it differs from the `service_plan_id` of the associated Web App") + } locks.ByID(oldPlan.ID()) defer locks.UnlockByID(oldPlan.ID()) locks.ByID(newPlan.ID()) diff --git a/internal/services/appservice/windows_web_app_slot_resource_test.go b/internal/services/appservice/windows_web_app_slot_resource_test.go index 48889ee497e3..a4f244349f40 100644 --- a/internal/services/appservice/windows_web_app_slot_resource_test.go +++ b/internal/services/appservice/windows_web_app_slot_resource_test.go @@ -6,6 +6,7 @@ package appservice_test import ( "context" "fmt" + "regexp" "testing" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance" @@ -64,7 +65,7 @@ func TestAccWindowsWebAppSlot_autoSwap(t *testing.T) { }) } -func TestAccWidowsWebAppSlot_separateStandardPlan(t *testing.T) { +func TestAccWindowsWebAppSlot_separateStandardPlan(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_windows_web_app_slot", "test") r := WindowsWebAppSlotResource{} @@ -101,6 +102,29 @@ func TestAccWindowsWebAppSlot_separateStandardPlanUpdate(t *testing.T) { }) } +func TestAccWindowsWebAppSlot_parentStandardPlanError(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_windows_web_app_slot", "test") + r := WindowsWebAppSlotResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.parentStandardPlanError(data), + ExpectError: regexp.MustCompile("`service_plan_id` should only be specified when it differs from the `service_plan_id` of the associated Web App"), + }, + { + Config: r.separatePlan(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.parentStandardPlanError(data), + ExpectError: regexp.MustCompile("`service_plan_id` should only be specified when it differs from the `service_plan_id` of the associated Web App"), + }, + }) +} + // Complete func TestAccWindowsWebAppSlot_complete(t *testing.T) { @@ -2142,6 +2166,32 @@ resource "azurerm_windows_web_app_slot" "test" { `, r.baseTemplate(data), data.RandomInteger, SkuStandardPlan, SkuPremiumPlan) } +func (r WindowsWebAppSlotResource) parentStandardPlanError(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +%s + +resource "azurerm_service_plan" "test2" { + name = "acctestASP2-%[2]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + os_type = "Windows" + sku_name = "%[3]s" +} + +resource "azurerm_windows_web_app_slot" "test" { + name = "acctestWAS-%[2]d" + app_service_id = azurerm_windows_web_app.test.id + service_plan_id = azurerm_service_plan.test.id + + site_config {} +} +`, r.baseTemplate(data), data.RandomInteger, SkuStandardPlan) +} + func (r WindowsWebAppSlotResource) publicNetworkAccessDisabled(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { diff --git a/website/docs/r/linux_web_app_slot.html.markdown b/website/docs/r/linux_web_app_slot.html.markdown index 76b859ea0972..2cd119a5c41a 100644 --- a/website/docs/r/linux_web_app_slot.html.markdown +++ b/website/docs/r/linux_web_app_slot.html.markdown @@ -94,6 +94,8 @@ The following arguments are supported: * `service_plan_id` - (Optional) The ID of the Service Plan in which to run this slot. If not specified the same Service Plan as the Linux Web App will be used. +~> **Note:** `service_plan_id` should only be specified if it differs from the Service Plan of the associated Linux Web App. + * `storage_account` - (Optional) One or more `storage_account` blocks as defined below. * `virtual_network_subnet_id` - (Optional) The subnet id which will be used by this Web App Slot for [regional virtual network integration](https://docs.microsoft.com/en-us/azure/app-service/overview-vnet-integration#regional-virtual-network-integration). diff --git a/website/docs/r/windows_web_app_slot.html.markdown b/website/docs/r/windows_web_app_slot.html.markdown index e0f1af0a5563..b44f33cad4aa 100644 --- a/website/docs/r/windows_web_app_slot.html.markdown +++ b/website/docs/r/windows_web_app_slot.html.markdown @@ -94,6 +94,8 @@ The following arguments are supported: * `service_plan_id` - (Optional) The ID of the Service Plan in which to run this slot. If not specified the same Service Plan as the Windows Web App will be used. +~> **Note:** `service_plan_id` should only be specified if it differs from the Service Plan of the associated Windows Web App. + * `storage_account` - (Optional) One or more `storage_account` blocks as defined below. ~> **Note:** Using this value requires `WEBSITE_RUN_FROM_PACKAGE=1` to be set on the App in `app_settings`. Refer to the [Azure docs](https://docs.microsoft.com/en-us/azure/app-service/deploy-run-package) for further details.