New Resouce `azurerm_network_manager_deployment` #20451

teowa · 2023-02-14T08:09:33Z

This is a special resouce with POST API to CRUD.
Reference:

Test:

make acctests SERVICE="network" TESTARGS="-parallel 20 -test.run=TestAccNetworkManager" TESTTIMEOUT='1440m' 
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./internal/services/network -parallel 20 -test.run=TestAccNetworkManager -timeout 1440m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccNetworkManager
=== RUN   TestAccNetworkManager/Commit
=== RUN   TestAccNetworkManager/Commit/keepOnDestroy
=== RUN   TestAccNetworkManager/Commit/purgeOnDestroy
=== RUN   TestAccNetworkManager/Commit/requiresImport
=== RUN   TestAccNetworkManager/Commit/basic
=== RUN   TestAccNetworkManager/Commit/basicAdmin
=== RUN   TestAccNetworkManager/Commit/complete
=== RUN   TestAccNetworkManager/Commit/update
--- PASS: TestAccNetworkManager (710.27s)
    --- PASS: TestAccNetworkManager/Commit (710.27s)
        --- PASS: TestAccNetworkManager/Commit/keepOnDestroy (123.07s)
        --- PASS: TestAccNetworkManager/Commit/purgeOnDestroy (130.61s)
        --- PASS: TestAccNetworkManager/Commit/requiresImport (86.63s)
        --- PASS: TestAccNetworkManager/Commit/basic (86.06s)
        --- PASS: TestAccNetworkManager/Commit/basicAdmin (84.48s)
        --- PASS: TestAccNetworkManager/Commit/complete (85.02s)
        --- PASS: TestAccNetworkManager/Commit/update (114.41s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/network       711.518s

…m into network_manager_commit

tombuildsstuff

hey @teowa

Thanks for this PR.

Taking a look through here it appears this resource has a 1:1 relationship with Network Manager and as such should be embedded within that resource, rather than being a separate resource (potentially as two blocks/fields for connectivity and security_admin), which would also mean that we could remove the flag from the features block.

Would you be able to take a look into supporting this functionality within the azurerm_network_manager resource instead?

Thanks!

tombuildsstuff · 2023-02-14T09:36:13Z

internal/services/network/network_manager_commit_resource.go

+		"network_manager_id": {
+			Type:         pluginsdk.TypeString,
+			Required:     true,
+			ValidateFunc: validate.NetworkManagerID,
+		},
+
+		"location": commonschema.Location(),
+
+		"scope_access": {
+			Type:     pluginsdk.TypeString,
+			Required: true,
+			ValidateFunc: validation.StringInSlice([]string{
+				string(network.ConfigurationTypeConnectivity),
+				string(network.ConfigurationTypeSecurityAdmin),
+			}, false),
+		},
+
+		"configuration_ids": {
+			Type:     pluginsdk.TypeList,
+			Required: true,
+			Elem: &pluginsdk.Schema{
+				Type:         pluginsdk.TypeString,
+				ValidateFunc: validation.StringIsNotEmpty,
+			},


since this resource is 1:1 with the Network Manager, this should be embedded within the Network Manager resource as blocks rather than being a separate resource - presumably with one for connectivity and one for security_admin - which also removes the need for the flag within the features block, can we update this?

A Network Manager can have more than one commit. Actually the commit is used to deploy specfic type of configuration to specific location follow a goal state model, so we design the ID format to be like /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resourceGroup1/providers/Microsoft.Network/networkManagers/networkManager1/commit|eastus|Connectivity. From Portal:

wouldn't that just mean you allow multiple commit blocks in the network manager resource then?

The dependency is that Commit depends on the Configurations, Configurations depends on Network Manager, and a Commit can contains more than one Configurations. An example is like:

resource "azurerm_network_manager" "test" { name = "acctest-nm" resource_group_name = azurerm_resource_group.test.name } resource "azurerm_network_manager_connectivity_configuration" "test" { name = "acctest-nmcc" network_manager_id = azurerm_network_manager.test.id } resource "azurerm_network_manager_commit" "test" { network_manager_id = azurerm_network_manager.test.id location = "eastus" scope_access = "Connectivity" configuration_ids = [azurerm_network_manager_connectivity_configuration.test.id] }

Allowing commits as blocks in the Network Manager will lead to cycle import.

@teowa per the link above, it appears that the Goal State Model is contrary to how Terraform Resources are expected to work - since at the end of the terraform apply the resource has to be in a fully provisioned/stable state - so the question becomes how does this resource make sense in Terraform?

@katbyte , from the doc, Security admin rules are evaluated before network security rules. And I have also confirmed with service team that Network Manager will NOT touch NSGs. They are in different layers.

As for possible conflict with peering resource, can we addnotes in the provider docs, if users don't config Network Peering and Network Manger Commit together, (they can also do this if they know how the Network Manager scope works), conflict can be avoid in their config file. Per above, I think no computed or ignore_changes needs to be added. Is this OK?

from the docs there, it seems like it starts an "eventually consistent" deployment. how do we, terraform, know when that is completed/done given there is nothign to poll on an ensure the create finished at the same time all these deployments finish?

We can retrieve the deployment status from another API, a state.WaitForStateContext is added at https://github.com/hashicorp/terraform-provider-azurerm/pull/20451/files#diff-f7faa7358d61518bec904e6d50a8453bf5220356bdae2d40b01027c61babfabeR245, we will wait until the create /update and delete get fully finished.

👍 thanks for the details

please continue to review this, thanks.

teowa · 2023-03-08T11:17:05Z

temp close this, please merge #20840 before this one.

…m into network_manager_commit

teowa · 2023-04-14T05:18:55Z

Hi, is there any update on this?

teowa · 2023-04-28T09:37:50Z

TF_ACC=1 go test -v ./internal/services/network -parallel 20 -test.run=TestAccNetworkManager -timeout 1440m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccNetworkManager
=== RUN   TestAccNetworkManager/Deployment
=== RUN   TestAccNetworkManager/Deployment/requiresImport
=== RUN   TestAccNetworkManager/Deployment/basic
=== RUN   TestAccNetworkManager/Deployment/basicAdmin
=== RUN   TestAccNetworkManager/Deployment/complete
=== RUN   TestAccNetworkManager/Deployment/update
=== RUN   TestAccNetworkManager/Deployment/withTriggers
--- PASS: TestAccNetworkManager (900.32s)
    --- PASS: TestAccNetworkManager/Deployment (900.32s)
        --- PASS: TestAccNetworkManager/Deployment/requiresImport (137.46s)
        --- PASS: TestAccNetworkManager/Deployment/basic (125.70s)
        --- PASS: TestAccNetworkManager/Deployment/basicAdmin (129.49s)
        --- PASS: TestAccNetworkManager/Deployment/complete (126.64s)
        --- PASS: TestAccNetworkManager/Deployment/update (255.89s)
        --- PASS: TestAccNetworkManager/Deployment/withTriggers (125.15s)
PASS
ok  	github.com/hashicorp/terraform-provider-azurerm/internal/services/network	902.363s

teowa · 2023-04-28T09:38:56Z

Hi @manicminer, thanks for reviewing this. I have changed code per review comment, please kindly take another look.

internal/services/network/network_manager_deployment_resource.go

…m into network_manager_commit

teowa · 2023-05-09T07:47:06Z

manicminer

Many thanks for making the changes @teowa, this looks good to merge.

SDK is updated

github-actions · 2023-05-12T00:06:33Z

This functionality has been released in v3.56.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!

github-actions · 2024-05-31T02:06:10Z

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.

teowa added 2 commits February 13, 2023 19:50

new resource azurerm_network_manager_commit

27446b6

fix

12bdc8c

github-actions bot added documentation service/network size/XL labels Feb 14, 2023

teowa added 2 commits February 14, 2023 16:47

fix lint

9219c6b

Merge branch 'main' of github.com:hashicorp/terraform-provider-azurer…

3e8b689

…m into network_manager_commit

tombuildsstuff previously requested changes Feb 14, 2023

View reviewed changes

github-actions bot added the waiting-response label Feb 14, 2023

fix unit test

0730ca9

github-actions bot removed the waiting-response label Feb 15, 2023

katbyte added the waiting-response label Feb 28, 2023

github-actions bot added waiting-response and removed waiting-response labels Feb 28, 2023

teowa closed this Mar 8, 2023

teowa added 2 commits March 29, 2023 11:02

add triggers

7b70cc8

Merge branch 'main' of github.com:hashicorp/terraform-provider-azurer…

c1116d5

…m into network_manager_commit

teowa reopened this Mar 29, 2023

teowa added 3 commits March 29, 2023 11:05

remove features doc

3751ccb

add triggers doc

77fee01

add note in doc

b755f62

katbyte added the waiting-response label Apr 3, 2023

github-actions bot removed the waiting-response label Apr 4, 2023

katbyte added the waiting-response label Apr 4, 2023

github-actions bot added waiting-response and removed waiting-response labels Apr 4, 2023

github-actions bot added waiting-response and removed waiting-response labels Apr 28, 2023

review comment

1af4c6d

recover test

f67e489

teowa changed the title ~~New Resouce azurerm_network_manager_commit~~ New Resouce azurerm_network_manager_deployment Apr 28, 2023

fix golangcli-lint

71022ec

tombuildsstuff reviewed Apr 28, 2023

View reviewed changes

internal/services/network/network_manager_deployment_resource.go Outdated Show resolved Hide resolved

github-actions bot added waiting-response and removed waiting-response labels Apr 28, 2023

check exact 1 commit per ID

d07985c

github-actions bot added waiting-response and removed waiting-response labels Apr 28, 2023

teowa added 2 commits May 4, 2023 17:08

Merge branch 'main' of github.com:hashicorp/terraform-provider-azurer…

f5cda21

…m into network_manager_commit

switch to use go-azure-sdk

3541fa3

github-actions bot added dependencies and removed waiting-response labels May 4, 2023

Add TODO for triggers property

6b8a846

manicminer approved these changes May 9, 2023

View reviewed changes

manicminer merged commit cb1665a into hashicorp:main May 9, 2023

github-actions bot added this to the v3.56.0 milestone May 9, 2023

manicminer added a commit that referenced this pull request May 9, 2023

Changelog for #20451

b0bef5e

iwahbe mentioned this pull request May 15, 2023

Upgrade terraform-provider-azurerm to v3.56.0 pulumi/pulumi-azure#1271

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resouce `azurerm_network_manager_deployment` #20451

New Resouce `azurerm_network_manager_deployment` #20451

teowa commented Feb 14, 2023 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Feb 14, 2023

teowa Feb 14, 2023

katbyte Feb 28, 2023

teowa Feb 28, 2023

tombuildsstuff Feb 28, 2023

teowa Apr 4, 2023 •

edited

Loading

katbyte Apr 4, 2023

teowa Apr 4, 2023

katbyte Apr 5, 2023

teowa Apr 7, 2023

teowa commented Mar 8, 2023

teowa commented Apr 14, 2023

teowa commented Apr 28, 2023

teowa commented Apr 28, 2023

teowa commented May 9, 2023

manicminer left a comment

github-actions bot commented May 12, 2023

github-actions bot commented May 31, 2024

New Resouce azurerm_network_manager_deployment #20451

New Resouce azurerm_network_manager_deployment #20451

Conversation

teowa commented Feb 14, 2023 • edited Loading

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teowa Apr 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teowa commented Mar 8, 2023

teowa commented Apr 14, 2023

teowa commented Apr 28, 2023

teowa commented Apr 28, 2023

teowa commented May 9, 2023

manicminer left a comment

Choose a reason for hiding this comment

github-actions bot commented May 12, 2023

github-actions bot commented May 31, 2024

New Resouce `azurerm_network_manager_deployment` #20451

New Resouce `azurerm_network_manager_deployment` #20451

teowa commented Feb 14, 2023 •

edited

Loading

teowa Apr 4, 2023 •

edited

Loading