`azurerm_network_manager_network_group` - support for `member_type` property by teowa · Pull Request #30672 · hashicorp/terraform-provider-azurerm

teowa · 2025-09-23T01:13:50Z

Community Note

Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

azurerm_network_manager_network_group - support for member_type property
azurerm_network_manager_static_member - now supports using a subnet as the target resource

swagger

PR Checklist

I have followed the guidelines in our Contributing Documentation.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
I have written new tests for my resource or datasource changes & updated any relevant documentation.
I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
(For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

azurerm_network_manager_network_group - support for member_type property [azurerm_network_manager_network_group - support for member_type property #30672]
azurerm_network_manager_static_member - now supports using a subnet as the target resource [azurerm_network_manager_network_group - support for member_type property #30672]

This is a (please select all that apply):

Bug Fix
New Feature (ie adding a service, resource, or data source)
Enhancement
Breaking Change

Related Issue(s)

Fixes #28811

Rollback Plan

If a change needs to be reverted, we will publish an updated version of the provider.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

gerrytan

Thanks for the PR. Some additional notes on my review:

If you do an entire codebase search on network_manager_network_group over *test.go files, there are heaps more acctest that should be run. Please run them. Please use TeamCity and paste the link to be sure there are no diff with your local env. Also do consider using the acctest impact detection tool: https://github.com/WodansSon/terraform-terracorder.
I see you bundled a bunch of test with a nested map on network_manager_resource_test.go, wouldn't this make test selection harder? I'm not sure about this pattern.
Do not close the issue once this PR merges, there is another resource that needs to be dealt with: azurerm_network_manager_static_member.
The change log needs to be updated: set the GH-00000 placeholder to the correct issue.

gerrytan · 2025-10-13T23:38:30Z

internal/services/network/network_manager_network_group_resource.go

+			Type:         pluginsdk.TypeString,
+			Optional:     true,
+			Default:      string(networkgroups.GroupMemberTypeVirtualNetwork),
+			ValidateFunc: validation.StringInSlice(networkgroups.PossibleValuesForGroupMemberType(), false),


So this does not need ForceNew: true? If that's the case can you please add acctest to cover the update scenario

test cases added

…m into ng-member

teowa · 2025-10-14T06:41:13Z

Thanks for the PR. Some additional notes on my review:

If you do an entire codebase search on network_manager_network_group over *test.go files, there are heaps more acctest that should be run. Please run them. Please use TeamCity and paste the link to be sure there are no diff with your local env. Also do consider using the acctest impact detection tool: https://github.com/WodansSon/terraform-terracorder.

I see you bundled a bunch of test with a nested map on network_manager_resource_test.go, wouldn't this make test selection harder? I'm not sure about this pattern.

Do not close the issue once this PR merges, there is another resource that needs to be dealt with: azurerm_network_manager_static_member.

The change log needs to be updated: set the GH-00000 placeholder to the correct issue.

Thank you for your review!

I will run additional tests; the Teamcity test result is still pending.
Some tests have been split because the API now supports multiple Network Managers under one subscription.
Changes have been made to the static member resource to support the feature requested in the issue.
This has been updated.

gerrytan · 2025-10-14T23:47:41Z

Thank you, PR looks good to me except for acctest results in TeamCity.

teowa · 2025-10-14T23:58:03Z

Tests can pass with
TEST_PREFIX = TestAccNetworkManagerNetworkGroup_|TestAccNetworkManagerNetworkGroupDataSource_|TestAccNetworkManagerStaticMember_, which should cover all the necessary tests.

ms-zhenhua · 2025-10-31T06:33:38Z

internal/services/network/network_manager_network_group_resource.go

 				group.Properties.Description = &model.Description
 			}

+			if model.MemberType != "" {


since member_type has a default value, will it be an empty value?

ms-zhenhua · 2025-10-31T06:34:18Z

internal/services/network/network_manager_network_group_resource.go

 			}

+			if metadata.ResourceData.HasChange("member_type") {
+				if model.MemberType != "" {


since member_type has a default value, will it be an empty value?

teowa · 2025-11-04T23:13:37Z

Hi @ms-zhenhua , Thank you for reviewing this. I've updated the code, could you please check it again? All tests pass except one, which is also failing on the main branch.

ms-zhenhua

LGTM~

catriona-m

Thanks for opening this @teowa, I left a couple of queries inline, but once those are addressed I can take another look. Thanks!

catriona-m · 2025-11-07T16:22:15Z

...r/github.com/hashicorp/go-azure-sdk/resource-manager/search/2025-05-01/services/constants.go

I don't think this file should be in here 🤔 does this pr need rebasing?

catriona-m · 2025-11-07T16:23:51Z

internal/services/network/network_manager_network_group_resource_test.go

 type ManagerNetworkGroupResource struct{}

-func testAccNetworkManagerNetworkGroup_basic(t *testing.T) {
+func TestAccNetworkManagerNetworkGroup_basic(t *testing.T) {


is there a reason why these test should not be sequential anymore?

Previously, the at most 1 subscription only allowed a single network manager. Now, it supports multiple network managers with connection or routing types, but still limits to one securityAdmin network manager per subscription. This means we can now run tests with connection and routing network managers in parallel.

…m into ng-member

teowa · 2025-11-09T23:12:59Z

Hi @catriona-m , thanks for reviewing this! Please take another look.

catriona-m

Thanks @teowa LGTM!

github-actions · 2026-01-05T02:37:30Z

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.

azurerm_network_manager_network_group - support for member type

501256b

teowa requested review from a team, WodansSon and magodo as code owners September 23, 2025 01:13

github-actions bot added enhancement documentation service/network size/S waiting-response labels Sep 23, 2025

fix lint

fe21823

github-actions bot removed the waiting-response label Sep 23, 2025

teowa added 2 commits September 24, 2025 01:09

update doc

56b63a0

fix note

223b315

gerrytan suggested changes Oct 13, 2025

View reviewed changes

github-actions bot assigned gerrytan Oct 13, 2025

github-actions bot added the waiting-response label Oct 13, 2025

support subnet static member; split test cases

4efd570

github-actions bot added size/M waiting-response and removed waiting-response size/S labels Oct 14, 2025

teowa added 2 commits October 14, 2025 06:31

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

78ca0ce

…m into ng-member

fix vendor

c198c80

github-actions bot added dependencies waiting-response and removed waiting-response labels Oct 14, 2025

github-actions bot removed the waiting-response label Oct 14, 2025

fix test

63091ba

github-actions bot removed the size/M label Oct 14, 2025

github-actions bot added the size/L label Oct 14, 2025

ms-zhenhua suggested changes Oct 31, 2025

View reviewed changes

github-actions bot assigned ms-zhenhua Oct 31, 2025

github-actions bot added the waiting-response label Oct 31, 2025

fix

f4702c7

github-actions bot added size/M and removed waiting-response size/L labels Nov 3, 2025

ms-zhenhua approved these changes Nov 7, 2025

View reviewed changes

catriona-m requested changes Nov 7, 2025

View reviewed changes

github-actions bot assigned catriona-m Nov 7, 2025

github-actions bot added the waiting-response label Nov 7, 2025

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

ce501ca

…m into ng-member

github-actions bot removed the waiting-response label Nov 9, 2025

teowa requested a review from catriona-m November 20, 2025 06:19

catriona-m approved these changes Dec 5, 2025

View reviewed changes

catriona-m merged commit 241d5c1 into hashicorp:main Dec 5, 2025
35 checks passed

github-actions bot added this to the v4.56.0 milestone Dec 5, 2025

catriona-m added a commit that referenced this pull request Dec 5, 2025

update changelog #30672

4a0564d

sreallymatt pushed a commit that referenced this pull request Dec 11, 2025

update changelog #30672

68b5b85

github-actions bot locked as resolved and limited conversation to collaborators Jan 5, 2026

Conversation

teowa commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

Rollback Plan

Changes to Security Controls

Uh oh!

gerrytan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teowa commented Oct 14, 2025

Uh oh!

gerrytan commented Oct 14, 2025

Uh oh!

teowa commented Oct 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teowa commented Nov 4, 2025

Uh oh!

ms-zhenhua left a comment

Choose a reason for hiding this comment

Uh oh!

catriona-m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teowa commented Nov 9, 2025

Uh oh!

catriona-m left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

teowa commented Sep 23, 2025 •

edited

Loading