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

Fix dependency parse of azurerm_kusto_cluster.id #21243

Merged
merged 3 commits into from
Apr 4, 2023

Conversation

myc2h6o
Copy link
Contributor

@myc2h6o myc2h6o commented Apr 1, 2023

Fix #20868
The segment Clusters in id of azurerm_kusto_cluster has been changed to clusters in #19525. For other resources which has kusto cluster as part of its dependency created in older provider version, the segment is already set to Clusters at service side. This could cause a diff when provider is updated, where azurerm_kusto_cluster.id is auto migrated to clusters in the state updater, while it's still Clusters at service side. Fixing the diff by normalize the ID returned by service.

@@ -462,6 +463,8 @@ func resourceMonitorDiagnosticSettingRead(d *pluginsdk.ResourceData, meta interf
return err
}

id.ResourceUri = normalizeMonitorDiagnosticSettingTargetResourceId(id.ResourceUri)
Copy link
Collaborator

Choose a reason for hiding this comment

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

i'm not sure why this is a function as its only used in a single place? can we inline it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

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.

Thanks for this @myc2h6o, one minor comment but otherwise this is looking good.

Comment on lines 466 to 470
if strings.Contains(strings.ToLower(id.ResourceUri), "microsoft.kusto/clusters/") {
if v, err := kustoParse.ClusterIDInsensitively(id.ResourceUri); err == nil {
id.ResourceUri = v.ID()
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This can probably be simplified to

Suggested change
if strings.Contains(strings.ToLower(id.ResourceUri), "microsoft.kusto/clusters/") {
if v, err := kustoParse.ClusterIDInsensitively(id.ResourceUri); err == nil {
id.ResourceUri = v.ID()
}
}
if v, err := kustoParse.ClusterIDInsensitively(id.ResourceUri); err == nil {
id.ResourceUri = v.ID()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

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.

Thanks @myc2h6o LGTM 💯

@stephybun stephybun merged commit 7eed320 into hashicorp:main Apr 4, 2023
@github-actions github-actions bot added this to the v3.51.0 milestone Apr 4, 2023
stephybun added a commit that referenced this pull request Apr 4, 2023
@myc2h6o myc2h6o deleted the kusto_cluster_ref_id_fix branch April 4, 2023 08:32
@github-actions
Copy link

This functionality has been released in v3.51.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@esener303
Copy link

esener303 commented Aug 8, 2023

I faced same issue but related to different service like azurerm_synapse_managed_private_endpoint and on server side there its Microsoft.Kusto/Clusters which was created by old provider version, after upgrade to 3.51.0 same issue persist and can not find any solution and workarounds so far.

 # module.plantlink_standalone_adx_cluster.azurerm_synapse_managed_private_endpoint.synapse_adx_endpoint must be replaced
-/+ resource "azurerm_synapse_managed_private_endpoint" "synapse_adx_endpoint" {
      ~ id                   = "/subscriptions/xxx-xxx-xxx-xxx/resourceGroups/rg-emea_we_hcpdev-plantlink_workspace_001/providers/Microsoft.Synapse/workspaces/plsynw-emea-we-hcpdev-plantlinksw-dev-001/managedVirtualNetworks/default/managedPrivateEndpoints/kustoplantlinkdev" -> (known after apply)
        name                 = "kustoplantlinkdev"
      ~ target_resource_id   = "/subscriptions/xxx-xxx-xxx-xxx/resourceGroups/rg-emea_we_hcpdev-plantlink_dev_001/providers/Microsoft.Kusto/Clusters/kustoplantlinkdev" -> "/subscriptions/xxx-xxx-xxx-xxx/resourceGroups/rg-emea_we_hcpdev-plantlink_dev_001/providers/Microsoft.Kusto/clusters/kustoplantlinkdev"

Seems like issue was solved only on 'services/monitor/monitor_diagnostic_setting_resource' not globally or other services which could relate or referred.

workaround its to use replace function with replace(azurerm_kusto_cluster.cluster.id, "clusters", "Clusters").

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 May 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Case change in kusto_cluster ID causing resource recreation/updates
4 participants