Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_api_management_api_policy, azurerm_api_management_api_operation_policy, azurerm_api_management_product_policy : fix parsing resource id error #23128

Merged
merged 2 commits into from
Sep 7, 2023

Conversation

sinbai
Copy link
Contributor

@sinbai sinbai commented Aug 30, 2023

For resources azurerm_api_management_api_policy, azurerm_api_management_api_operation_policy, azurerm_api_management_product_policy, after importing the resource using a version before v3.70.0 of Terraform Provider, the id in state file ends with "/policies/policy", and the id in state file ends with "/policies/xml" when creating resource by Terraform. Therefore, when migrating Pandora SDK, both cases need to be migrated.

In PR #22783 only the case where the ID ends with "/policies/xml" is processed, so submit this PR to process the case where the ID ends with "/policies/policy" to solve the parsing id error.

Might fix issue #23112, #23095.

Local manual testing, the steps are as follows:

  1. Import the existing resources azurerm_api_management_api_policy, azurerm_api_management_api_operation_policy, azurerm_api_management_product_policy using TF provider v3.69.0.
  2. Upgrade TF provider to v3.71.0.
  3. Run "terraform plan" => The parsing id error occurred.
  4. Upgrade the TF provider to the current PR build version.
  5. Run "terraform plan" => No error.

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @sinbai

Thanks for this PR - taking a look through on the whole this looks pretty good but since we're updating these IDs we should also look to parse/reformat the ID too to ensure that the resourceGroups and other segments are correct at the same time. However if we can fix that up then this should otherwise be good to go 👍

Thanks!

Comment on lines 58 to 61
newId := strings.TrimSuffix(oldId, "/policies/policy")

log.Printf("[DEBUG] Updating ID from %q to %q", oldId, newId)
rawState["id"] = newId
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we parse the updated ID and reformat it such that we can ensure the resourceGroups and other segments are correct too? e.g.:

Suggested change
newId := strings.TrimSuffix(oldId, "/policies/policy")
log.Printf("[DEBUG] Updating ID from %q to %q", oldId, newId)
rawState["id"] = newId
newId := strings.TrimSuffix(oldId, "/policies/policy")
parsed, err := policy.ParseServiceID(newID)
if err != nil {
return err
}
newId = parsed.ID()
log.Printf("[DEBUG] Updating ID from %q to %q", oldId, newId)
rawState["id"] = newId

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment on lines 57 to 60
newId := strings.TrimSuffix(oldId, "/policies/policy")

log.Printf("[DEBUG] Updating ID from %q tmakeo %q", oldId, newId)
rawState["id"] = newId
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(same here / as above)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment on lines 64 to 67
newId := strings.TrimSuffix(oldId, "/policies/policy")

log.Printf("[DEBUG] Updating ID from %q tmakeo %q", oldId, newId)
rawState["id"] = newId
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(same here)

Suggested change
newId := strings.TrimSuffix(oldId, "/policies/policy")
log.Printf("[DEBUG] Updating ID from %q tmakeo %q", oldId, newId)
rawState["id"] = newId
newId := strings.TrimSuffix(oldId, "/policies/policy")
log.Printf("[DEBUG] Updating ID from %q to %q", oldId, newId)
rawState["id"] = newId

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@sinbai
Copy link
Contributor Author

sinbai commented Sep 1, 2023

hey @sinbai

Thanks for this PR - taking a look through on the whole this looks pretty good but since we're updating these IDs we should also look to parse/reformat the ID too to ensure that the resourceGroups and other segments are correct at the same time. However if we can fix that up then this should otherwise be good to go 👍

Thanks!

Hi @tombuildsstuff , thanks for your feedback. The code has been updated. Could you please take another look?

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🪐

@stephybun stephybun merged commit 8c6804f into hashicorp:main Sep 7, 2023
@github-actions github-actions bot added this to the v3.72.0 milestone Sep 7, 2023
stephybun added a commit that referenced this pull request Sep 7, 2023
@sinbai sinbai deleted the APIM/fix_parse_id_error branch March 28, 2024 02:47
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants