From e5614e894f8163ecd1deecd00d28e70201429198 Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Fri, 21 Jan 2022 12:50:51 +0000 Subject: [PATCH 1/6] Use the correct ID for api_management_policy --- .../api_management_policy_resource.go | 16 +++++-- .../apimanagement/migration/policy.go | 45 +++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 internal/services/apimanagement/migration/policy.go diff --git a/internal/services/apimanagement/api_management_policy_resource.go b/internal/services/apimanagement/api_management_policy_resource.go index fd21bbe6555d..0574ff9d8f4b 100644 --- a/internal/services/apimanagement/api_management_policy_resource.go +++ b/internal/services/apimanagement/api_management_policy_resource.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/apimanagement/mgmt/2021-08-01/apimanagement" "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" + "github.com/hashicorp/terraform-provider-azurerm/internal/services/apimanagement/migration" "github.com/hashicorp/terraform-provider-azurerm/internal/services/apimanagement/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" "github.com/hashicorp/terraform-provider-azurerm/internal/timeouts" @@ -24,6 +25,11 @@ func resourceApiManagementPolicy() *pluginsdk.Resource { // TODO: replace this with an importer which validates the ID during import Importer: pluginsdk.DefaultImporter(), + SchemaVersion: 1, + StateUpgraders: pluginsdk.StateUpgrades(map[int]pluginsdk.StateUpgrade{ + 0: migration.ApiManagementApiPolicyV0ToV1{}, + }), + Timeouts: &pluginsdk.ResourceTimeout{ Create: pluginsdk.DefaultTimeout(30 * time.Minute), Read: pluginsdk.DefaultTimeout(5 * time.Minute), @@ -64,12 +70,12 @@ func resourceApiManagementPolicyCreateUpdate(d *pluginsdk.ResourceData, meta int defer cancel() apiManagementID := d.Get("api_management_id").(string) - id, err := parse.ApiManagementID(apiManagementID) + apiMgmtId, err := parse.ApiManagementID(apiManagementID) if err != nil { return err } - resourceGroup := id.ResourceGroup - serviceName := id.ServiceName + resourceGroup := apiMgmtId.ResourceGroup + serviceName := apiMgmtId.ServiceName /* Other resources would have a check for d.IsNewResource() at this location, and would error out using `tf.ImportAsExistsError` if the resource already existed. @@ -105,10 +111,12 @@ func resourceApiManagementPolicyCreateUpdate(d *pluginsdk.ResourceData, meta int return fmt.Errorf("Either `xml_content` or `xml_link` must be set") } - if _, err := client.CreateOrUpdate(ctx, resourceGroup, serviceName, parameters, ""); err != nil { + policyResp, err := client.CreateOrUpdate(ctx, resourceGroup, serviceName, parameters, "") + if err != nil { return fmt.Errorf("creating or updating Policy (Resource Group %q / API Management Service %q): %+v", resourceGroup, serviceName, err) } + id := parse.NewPolicyID(apiMgmtId.SubscriptionId, apiMgmtId.ResourceGroup, apiMgmtId.ServiceName, utils.NormalizeNilableString(policyResp.Name)) d.SetId(id.ID()) return resourceApiManagementPolicyRead(d, meta) diff --git a/internal/services/apimanagement/migration/policy.go b/internal/services/apimanagement/migration/policy.go new file mode 100644 index 000000000000..633d55c709b9 --- /dev/null +++ b/internal/services/apimanagement/migration/policy.go @@ -0,0 +1,45 @@ +package migration + +import ( + "context" + + "github.com/hashicorp/terraform-provider-azurerm/internal/services/apimanagement/parse" + "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" +) + +var _ pluginsdk.StateUpgrade = ApiManagementApiPolicyV0ToV1{} + +type ApiManagementApiPolicyV0ToV1 struct{} + +func (ApiManagementApiPolicyV0ToV1) UpgradeFunc() pluginsdk.StateUpgraderFunc { + return func(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + apiMgmtId, err := parse.ApiManagementID(rawState["id"].(string)) + if err != nil { + return rawState, nil + } + id := parse.NewPolicyID(apiMgmtId.SubscriptionId, apiMgmtId.ResourceGroup, apiMgmtId.ServiceName, "policy") + rawState["id"] = id.ID() + return rawState, nil + } +} + +func (ApiManagementApiPolicyV0ToV1) Schema() map[string]*pluginsdk.Schema { + return map[string]*pluginsdk.Schema{ + "api_management_id": { + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + }, + + "xml_content": { + Type: pluginsdk.TypeString, + Optional: true, + Computed: true, + }, + + "xml_link": { + Type: pluginsdk.TypeString, + Optional: true, + }, + } +} From d0fb55a48bd1954246bde3322a7c28f8053b5356 Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Tue, 25 Jan 2022 08:18:49 +0000 Subject: [PATCH 2/6] set the xml_content value by reading the value from the API during migration --- .../services/apimanagement/migration/policy.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/services/apimanagement/migration/policy.go b/internal/services/apimanagement/migration/policy.go index 633d55c709b9..86f3ecafd8a6 100644 --- a/internal/services/apimanagement/migration/policy.go +++ b/internal/services/apimanagement/migration/policy.go @@ -2,7 +2,11 @@ package migration import ( "context" + "fmt" + "html" + "github.com/Azure/azure-sdk-for-go/services/apimanagement/mgmt/2021-08-01/apimanagement" + "github.com/hashicorp/terraform-provider-azurerm/internal/clients" "github.com/hashicorp/terraform-provider-azurerm/internal/services/apimanagement/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" ) @@ -19,6 +23,18 @@ func (ApiManagementApiPolicyV0ToV1) UpgradeFunc() pluginsdk.StateUpgraderFunc { } id := parse.NewPolicyID(apiMgmtId.SubscriptionId, apiMgmtId.ResourceGroup, apiMgmtId.ServiceName, "policy") rawState["id"] = id.ID() + + client := meta.(*clients.Client).ApiManagement.PolicyClient + resp, err := client.Get(ctx, id.ResourceGroup, id.ServiceName, apimanagement.PolicyExportFormatXML) + if err != nil { + return nil, fmt.Errorf("making Read request for API Operation Policy (Resource Group %q / API Management Service %q / API %q / Operation %q): %+v", id.ResourceGroup, id.ServiceName, id.Name, err) + } + + if properties := resp.PolicyContractProperties; properties != nil { + // when you submit an `xml_link` to the API, the API downloads this link and stores it as `xml_content` + // as such there is no way to set `xml_link` and we'll let Terraform handle it + rawState["xml_content"] = html.UnescapeString(*properties.Value) + } return rawState, nil } } From 5316e21236a83b96d3cb03d64c02cf059a9a9c6a Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Tue, 25 Jan 2022 08:39:17 +0000 Subject: [PATCH 3/6] Update changelog to include upgrade notes --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 687200914169..43761829fe43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## 2.94.0 (Unreleased) +UPGRADE NOTES: +* `azurerm_api_management_policy` - resources that were created with v2.92.0 will be marked as tainted due to a [bug](https://github.com/hashicorp/terraform-provider-azurerm/issues/15042). This version addresses the underlying issue, but the actual resource needs to either be untainted (via `terraform untaint`) or allow terraform to delete the resource and create it again. + FEATURES: **New Data Source:** `azurerm_linux_function_app` [GH-15009] @@ -11,6 +14,7 @@ ENHANCEMENTS: BUG FIXES: +* `azurerm_api_management_policy` - use the correct ID for api_management_policy [GH-15060] * `azurerm_bastion_host` - Fix crash by adding nil check for `copy_paste_enabled` [GH-15074] * `azurerm_dev_test_lab` - fix the unexpected diff on `key_vault_id` [GH-15054] * `azurerm_subscription_cost_management_export` - fix the update method by sending the ETag when updating a cost management export [GH-15017] From 2fa684474f426a3d44edb59b0c169d7a90a2c22a Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Tue, 25 Jan 2022 09:05:28 +0000 Subject: [PATCH 4/6] fix error message --- internal/services/apimanagement/migration/policy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/services/apimanagement/migration/policy.go b/internal/services/apimanagement/migration/policy.go index 86f3ecafd8a6..c52c6e5e4898 100644 --- a/internal/services/apimanagement/migration/policy.go +++ b/internal/services/apimanagement/migration/policy.go @@ -27,7 +27,7 @@ func (ApiManagementApiPolicyV0ToV1) UpgradeFunc() pluginsdk.StateUpgraderFunc { client := meta.(*clients.Client).ApiManagement.PolicyClient resp, err := client.Get(ctx, id.ResourceGroup, id.ServiceName, apimanagement.PolicyExportFormatXML) if err != nil { - return nil, fmt.Errorf("making Read request for API Operation Policy (Resource Group %q / API Management Service %q / API %q / Operation %q): %+v", id.ResourceGroup, id.ServiceName, id.Name, err) + return nil, fmt.Errorf("making Read request for API Management Policy (Resource Group %q / API Management Service %q / API %q): %+v", id.ResourceGroup, id.ServiceName, id.Name, err) } if properties := resp.PolicyContractProperties; properties != nil { From 14903cdb2963e492e385c1f19f8a38afdd1a8468 Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Tue, 25 Jan 2022 18:07:32 +0000 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Tom Harvey --- CHANGELOG.md | 5 +++-- .../services/apimanagement/api_management_policy_resource.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43761829fe43..6a4e92709a4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ ## 2.94.0 (Unreleased) UPGRADE NOTES: -* `azurerm_api_management_policy` - resources that were created with v2.92.0 will be marked as tainted due to a [bug](https://github.com/hashicorp/terraform-provider-azurerm/issues/15042). This version addresses the underlying issue, but the actual resource needs to either be untainted (via `terraform untaint`) or allow terraform to delete the resource and create it again. + +* `azurerm_api_management_policy` - resources that were created with v2.92.0 will be marked as tainted due to a [bug](https://github.com/hashicorp/terraform-provider-azurerm/issues/15042). This version addresses the underlying issue, but the actual resource needs to either be untainted (via `terraform untaint`) or allow Terraform to delete the resource and create it again. FEATURES: @@ -14,7 +15,7 @@ ENHANCEMENTS: BUG FIXES: -* `azurerm_api_management_policy` - use the correct ID for api_management_policy [GH-15060] +* `azurerm_api_management_policy` - fixing the Resource ID for `api_management_policy` when this was provisioned using version `2.92.0` of the Azure Provider [GH-15060] * `azurerm_bastion_host` - Fix crash by adding nil check for `copy_paste_enabled` [GH-15074] * `azurerm_dev_test_lab` - fix the unexpected diff on `key_vault_id` [GH-15054] * `azurerm_subscription_cost_management_export` - fix the update method by sending the ETag when updating a cost management export [GH-15017] diff --git a/internal/services/apimanagement/api_management_policy_resource.go b/internal/services/apimanagement/api_management_policy_resource.go index 0574ff9d8f4b..42ec54e92870 100644 --- a/internal/services/apimanagement/api_management_policy_resource.go +++ b/internal/services/apimanagement/api_management_policy_resource.go @@ -116,7 +116,7 @@ func resourceApiManagementPolicyCreateUpdate(d *pluginsdk.ResourceData, meta int return fmt.Errorf("creating or updating Policy (Resource Group %q / API Management Service %q): %+v", resourceGroup, serviceName, err) } - id := parse.NewPolicyID(apiMgmtId.SubscriptionId, apiMgmtId.ResourceGroup, apiMgmtId.ServiceName, utils.NormalizeNilableString(policyResp.Name)) + id := parse.NewPolicyID(apiMgmtId.SubscriptionId, apiMgmtId.ResourceGroup, apiMgmtId.ServiceName, "policy") d.SetId(id.ID()) return resourceApiManagementPolicyRead(d, meta) From 6ebce6cda31d3af5beca8e20e0f2765f0b999420 Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Wed, 26 Jan 2022 13:51:13 +0000 Subject: [PATCH 6/6] fix error --- .../services/apimanagement/api_management_policy_resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/services/apimanagement/api_management_policy_resource.go b/internal/services/apimanagement/api_management_policy_resource.go index 42ec54e92870..da39dea4d4a3 100644 --- a/internal/services/apimanagement/api_management_policy_resource.go +++ b/internal/services/apimanagement/api_management_policy_resource.go @@ -111,7 +111,7 @@ func resourceApiManagementPolicyCreateUpdate(d *pluginsdk.ResourceData, meta int return fmt.Errorf("Either `xml_content` or `xml_link` must be set") } - policyResp, err := client.CreateOrUpdate(ctx, resourceGroup, serviceName, parameters, "") + _, err = client.CreateOrUpdate(ctx, resourceGroup, serviceName, parameters, "") if err != nil { return fmt.Errorf("creating or updating Policy (Resource Group %q / API Management Service %q): %+v", resourceGroup, serviceName, err) }