Update segment of resource id for network packet capture #8167

neil-yechenwei · 2020-08-19T05:30:15Z

As this resource cannot be deleted anymore for incorrect resource id segment issue, so we have to update segment of resource id here for this resource.

--- PASS: TestAccAzureRMNetworkWatcher/PacketCapture (1924.64s)
--- PASS: TestAccAzureRMNetworkWatcher/PacketCapture/localDisk (453.99s)
--- PASS: TestAccAzureRMNetworkWatcher/PacketCapture/storageAccount (504.75s)
--- PASS: TestAccAzureRMNetworkWatcher/PacketCapture/storageAccountAndLocalDisk (446.00s)
--- PASS: TestAccAzureRMNetworkWatcher/PacketCapture/withFilters (519.90s)
--- PASS: TestAccAzureRMNetworkWatcher/PacketCapture/requiresImport (532.44s)

mbfrahry

We should check both the old and the new path in case users still have the old path.

neil-yechenwei · 2020-08-19T23:58:43Z

@mbfrahry, it's not appropriate for this case. Because the segment of resource id of the 2018-08-01 version terraform initially used is also updated to "packetCaptures". So I think this issue would also happen on older version. After tested with old version, this issue also happened. So I assume we don't need to be aware of compatible problem for this case. Based on above conclusion, I assume all users are blocked on all released azurerm provider versions now.

neil-yechenwei · 2020-08-20T05:49:17Z

@tombuildsstuff ,@mbfrahry, although it's a breaking change, but I assume all users are blocked now by this issue on all released azurerm provider versions. Could we merge it to unblock users first?

tombuildsstuff

hi @neil-yechenwei

Taking a look though here unfortunately this would be a breaking change for existing users - as such we'll need to add a state migration to update these ID's to the new format here.

Thanks!

tombuildsstuff · 2020-08-20T08:39:13Z

azurerm/internal/services/network/network_packet_capture_resource.go

@@ -217,7 +217,7 @@ func resourceArmNetworkPacketCaptureRead(d *schema.ResourceData, meta interface{

 	resourceGroup := id.ResourceGroup
 	watcherName := id.Path["networkWatchers"]
-	name := id.Path["NetworkPacketCaptures"]
+	name := id.Path["packetCaptures"]


unfortunately where this will be different for older resources we'd need to add a state migration to update this ID - as-is this is a breaking change

Updated code

tombuildsstuff · 2020-08-20T08:39:18Z

azurerm/internal/services/network/network_packet_capture_resource.go

@@ -266,7 +266,7 @@ func resourceArmNetworkPacketCaptureDelete(d *schema.ResourceData, meta interfac

 	resourceGroup := id.ResourceGroup
 	watcherName := id.Path["networkWatchers"]
-	name := id.Path["NetworkPacketCaptures"]
+	name := id.Path["packetCaptures"]


unfortunately where this will be different for older resources we'd need to add a state migration to update this ID - as-is this is a breaking change

Updated code

neil-yechenwei · 2020-08-21T07:02:43Z

@tombuildsstuff , thanks for the comment. I've updated code to use MigrateState. After used MigrateState, I assume this PR doesn't belong to "breaking-change", right?

tombuildsstuff

if we can confirm the state migration works then this otherwise LGTM 👍

tombuildsstuff · 2020-08-21T09:05:14Z

azurerm/internal/services/network/network_packet_capture_migration_resource.go

+
+	log.Printf("[DEBUG] ARM Network Packet Capture before Migration: %#v", is.Attributes)
+
+	newID := strings.Replace(is.Attributes["id"], "/NetworkPacketCaptures/", "/packetCaptures/", 1)


can you confirm this migration works correctly when provisioning a resource using 2.0.0 -> that then shows no diff using tis branch?

@tombuildsstuff , yes. I tested the scenario you mentioned and it works fine.
The scenario:

Create Network Packet Captures with 2.0.0 (Also tried 1.27.0)

Run tf plan with this branch

There is no diff.

Hi @neil-yechenwei - Can you post the manual test results / output here? (Ideally with an apply rather than a plan as this will actually update the state and exercise the code.).

Thanks

@jackofallops , sure.

Run tf apply to create Network Packet Capture with provider.azurerm v2.0.0 (I have to manually update the name segment to "NetworkPacketCaptures" since the name segment in all api versions has been updated to "packetCaptures" such as api version 2018-08-01. So we cannot get old value anymore.)

azurerm_network_packet_capture.test: Creation complete after 16s [id=/subscriptions/xx-xx-xx-xx/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctestnw-test01/NetworkPacketCaptures/acctestpc-test02]

2. Switch provider.azurerm from v2.0.0 to this branch
3. Run tf plan

azurerm_network_packet_capture.test: Refreshing state... [id=/subscriptions/xx-xx-xx-xx/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctestnw-test01/packetCaptures/acctestpc-test02]

4. Run tf apply

azurerm_network_packet_capture.test: Refreshing state... [id=/subscriptions/xx-x-xx-xx/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctestnw-test01/packetCaptures/acctestpc-test02]

5. Update the name of network packet capture in tfconfig. (I cannot test update scenario since this resource doesn't support update operation.)
6. Run tf apply

azurerm_network_packet_capture.test: Creation complete after 5s [id=/subscriptions/xx-xx-xx-xxx/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctestnw-test01/packetCaptures/acctestpc-test03]

7. Run tf plan

azurerm_network_packet_capture.test: Refreshing state... [id=/subscriptions/xx-xx-xx-xx/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctestnw-test01/packetCaptures/acctestpc-test03]

8. Run tf destroy

azurerm_network_packet_capture.test: Destroying... [id=/subscriptions/xx-x-x-xx/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctestnw-test01/packetCaptures/acctestpc-test03] azurerm_network_packet_capture.test: Destruction complete after 2s

neil-yechenwei · 2020-09-11T02:37:37Z

@jackofallops , thanks for your comments. I've posted manual test results. Please have a look. Thanks.

jackofallops

Thanks @neil-yechenwei, this LGTM now 👍

outdated

ghost · 2020-09-17T17:46:24Z

This has been released in version 2.28.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.28.0"
}
# ... other configuration ...

ghost · 2020-10-11T14:50:37Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

Update segment of network packet capture

f2b32b2

ghost added the size/XS label Aug 19, 2020

neil-yechenwei changed the title ~~Fix test case for network packet capture~~ Update segment of resource id for network packet capture Aug 19, 2020

tombuildsstuff added the breaking-change label Aug 19, 2020

mbfrahry previously requested changes Aug 19, 2020

View reviewed changes

neil-yechenwei requested a review from mbfrahry August 20, 2020 00:01

tombuildsstuff previously requested changes Aug 20, 2020

View reviewed changes

Update code

4d47527

ghost added size/L and removed size/XS labels Aug 21, 2020

Update code

ef1537f

neil-yechenwei requested a review from tombuildsstuff August 21, 2020 07:02

tombuildsstuff reviewed Aug 21, 2020

View reviewed changes

tombuildsstuff added bug service/network-watcher and removed breaking-change labels Aug 21, 2020

neil-yechenwei requested a review from tombuildsstuff August 21, 2020 16:19

Update code

51260a7

WodansSon added this to the v2.27.0 milestone Sep 4, 2020

jackofallops added the waiting-response label Sep 10, 2020

jackofallops modified the milestones: v2.27.0, v2.28.0 Sep 10, 2020

ghost removed the waiting-response label Sep 11, 2020

neil-yechenwei requested a review from jackofallops September 11, 2020 02:37

neil-yechenwei added 2 commits September 11, 2020 12:52

Update code

b2f02cb

Update code

50d801a

Update code

9564c83

ghost added size/XL and removed size/L labels Sep 11, 2020

jackofallops approved these changes Sep 11, 2020

View reviewed changes

jackofallops merged commit bbaa762 into hashicorp:master Sep 11, 2020

jackofallops added a commit that referenced this pull request Sep 11, 2020

Update CHANGELOG.md for #8167

fdfc39a

ghost locked as resolved and limited conversation to collaborators Oct 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update segment of resource id for network packet capture #8167

Update segment of resource id for network packet capture #8167

neil-yechenwei commented Aug 19, 2020 •

edited

Loading

mbfrahry left a comment

neil-yechenwei commented Aug 19, 2020 •

edited

Loading

neil-yechenwei commented Aug 20, 2020 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Aug 20, 2020

neil-yechenwei Aug 21, 2020

tombuildsstuff Aug 20, 2020

neil-yechenwei Aug 21, 2020

neil-yechenwei commented Aug 21, 2020 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Aug 21, 2020

neil-yechenwei Aug 21, 2020

jackofallops Sep 10, 2020

neil-yechenwei Sep 11, 2020 •

edited

Loading

neil-yechenwei commented Sep 11, 2020

jackofallops left a comment

ghost commented Sep 17, 2020

ghost commented Oct 11, 2020


		log.Printf("[DEBUG] ARM Network Packet Capture before Migration: %#v", is.Attributes)

		newID := strings.Replace(is.Attributes["id"], "/NetworkPacketCaptures/", "/packetCaptures/", 1)

Update segment of resource id for network packet capture #8167

Update segment of resource id for network packet capture #8167

Conversation

neil-yechenwei commented Aug 19, 2020 • edited Loading

mbfrahry left a comment

Choose a reason for hiding this comment

neil-yechenwei commented Aug 19, 2020 • edited Loading

neil-yechenwei commented Aug 20, 2020 • edited Loading

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Aug 20, 2020

Choose a reason for hiding this comment

neil-yechenwei Aug 21, 2020

Choose a reason for hiding this comment

tombuildsstuff Aug 20, 2020

Choose a reason for hiding this comment

neil-yechenwei Aug 21, 2020

Choose a reason for hiding this comment

neil-yechenwei commented Aug 21, 2020 • edited Loading

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Aug 21, 2020

Choose a reason for hiding this comment

neil-yechenwei Aug 21, 2020

Choose a reason for hiding this comment

jackofallops Sep 10, 2020

Choose a reason for hiding this comment

neil-yechenwei Sep 11, 2020 • edited Loading

Choose a reason for hiding this comment

neil-yechenwei commented Sep 11, 2020

jackofallops left a comment

Choose a reason for hiding this comment

ghost commented Sep 17, 2020

ghost commented Oct 11, 2020

neil-yechenwei commented Aug 19, 2020 •

edited

Loading

neil-yechenwei commented Aug 19, 2020 •

edited

Loading

neil-yechenwei commented Aug 20, 2020 •

edited

Loading

neil-yechenwei commented Aug 21, 2020 •

edited

Loading

neil-yechenwei Sep 11, 2020 •

edited

Loading