New Resource: `azurerm_network_manager_ipam_pool_static_cidr` by teowa · Pull Request #29501 · hashicorp/terraform-provider-azurerm

teowa · 2025-05-08T10:49:43Z

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

New Resource: azurerm_network_manager_ipam_pool_static_cidr

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.

New Resource: azurerm_network_manager_ipam_pool_static_cidr

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 #0000

Note

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

ms-zhenhua

Hi @teowa ,

Thanks for this PR - I've taken a look through and left some comments inline. If we can fix those up, this should be good to go 👍

ms-zhenhua · 2025-05-09T07:47:11Z

internal/services/network/network_manager_ipam_pool_static_cidr_resource.go

+		},
+
+		"number_of_ip_addresses_to_allocate": {
+			Type:         pluginsdk.TypeString,


why not directly define it as an int type?

for IPv6 addresses, this field might be up to 128 bit integer, so we need to use string type for this.

ms-zhenhua · 2025-05-09T07:56:42Z

internal/services/network/network_manager_ipam_pool_static_cidr_resource.go

+			if metadata.ResourceData.HasChange("address_prefixes") {
+				if len(model.AddressPrefixes) > 0 {
+					parameters.Properties.AddressPrefixes = pointer.To(model.AddressPrefixes)
+					parameters.Properties.NumberOfIPAddressesToAllocate = pointer.To("")


why not set NumberOfIPAddressesToAllocate in else branch?

Both address_prefixes and number_of_ip_addresses_to_allocate will be returned by API but only one of them can be set. When ignore_changes is applied to the unused field, it must be explicitly set to an empty value.

ms-zhenhua · 2025-05-09T07:58:49Z

internal/services/network/network_manager_ipam_pool_static_cidr_resource.go

+			if metadata.ResourceData.HasChange("number_of_ip_addresses_to_allocate") {
+				if model.NumberOfIPAddressesToAllocate != "" {
+					parameters.Properties.NumberOfIPAddressesToAllocate = pointer.To(model.NumberOfIPAddressesToAllocate)
+					parameters.Properties.AddressPrefixes = pointer.To([]string{})


same as above

ms-zhenhua · 2025-05-12T01:46:53Z

website/docs/r/network_manager_ipam_pool_static_cidr.html.markdown

+
+* `address_prefixes` - (Optional) Specifies a list of IPv4 or IPv6 IP address prefixes which will be allocated to the Static CIDR.
+
+-> **NOTE:** Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified. If you set either property, the Azure API will automatically configure the other. Therefore, if you set `address_prefixes`, consider adding `number_of_ip_addresses_to_allocate` to `ignore_changes` to avoid plan diff.


Suggested change

-> **NOTE:** Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified. If you set either property, the Azure API will automatically configure the other. Therefore, if you set `address_prefixes`, consider adding `number_of_ip_addresses_to_allocate` to `ignore_changes` to avoid plan diff.

-> **Note:** Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified. If you set either property, the Azure API will automatically configure the other. Therefore, if you set `address_prefixes`, consider adding `number_of_ip_addresses_to_allocate` to `ignore_changes` to avoid plan diff.

ms-zhenhua · 2025-05-12T02:00:11Z

website/docs/r/network_manager_ipam_pool_static_cidr.html.markdown

+
+* `number_of_ip_addresses_to_allocate` - (Optional) The number of IP addresses to allocated to the Static CIDR. The value must be a string that represents a positive number, e.g., `"16"`.
+
+-> **NOTE:** Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified. If you set either property, the Azure API will automatically configure the other. Therefore, if you set `number_of_ip_addresses_to_allocate`, consider adding `address_prefixes` to `ignore_changes` to avoid plan diff.


Suggested change

-> **NOTE:** Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified. If you set either property, the Azure API will automatically configure the other. Therefore, if you set `number_of_ip_addresses_to_allocate`, consider adding `address_prefixes` to `ignore_changes` to avoid plan diff.

-> **Note:** Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified. If you set either property, the Azure API will automatically configure the other. Therefore, if you set `number_of_ip_addresses_to_allocate`, consider adding `address_prefixes` to `ignore_changes` to avoid plan diff.

ms-zhenhua · 2025-05-12T02:00:18Z

website/docs/r/network_manager_ipam_pool_static_cidr.html.markdown

+
+-> **NOTE:** Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified. If you set either property, the Azure API will automatically configure the other. Therefore, if you set `number_of_ip_addresses_to_allocate`, consider adding `address_prefixes` to `ignore_changes` to avoid plan diff.
+
+-> **NOTE:** If `number_of_ip_addresses_to_allocate` is not set to a power of 2, Azure API will return the nearest power of 2. For instance, if `number_of_ip_addresses_to_allocate` is `"20"`, Azure API will automatically update it to `"32"`. In such cases, please update the `number_of_ip_addresses_to_allocate` in the configuration file to this new value.


could you add a validation of a power of 2 in the provider so that the users will not miss configure the unacceptable value ?

Suggested change

-> **NOTE:** If `number_of_ip_addresses_to_allocate` is not set to a power of 2, Azure API will return the nearest power of 2. For instance, if `number_of_ip_addresses_to_allocate` is `"20"`, Azure API will automatically update it to `"32"`. In such cases, please update the `number_of_ip_addresses_to_allocate` in the configuration file to this new value.

-> **Note:** If `number_of_ip_addresses_to_allocate` is not set to a power of 2, Azure API will return the nearest power of 2. For instance, if `number_of_ip_addresses_to_allocate` is `"20"`, Azure API will automatically update it to `"32"`. In such cases, please update the `number_of_ip_addresses_to_allocate` in the configuration file to this new value.

updated, a validation is added

teowa · 2025-05-13T05:31:12Z

Hi @ms-zhenhua , thanks for reviewing! I have updated the code, please kindly take another look.

website/docs/r/network_manager_ipam_pool_static_cidr.html.markdown

…down Co-authored-by: Zhenhua Hu <zhhu@microsoft.com>

ms-zhenhua

Hi @teowa,

Thanks for your updates. LGTM~

…m into nm_ipam_sc

teowa · 2025-06-02T13:31:13Z

harsimranmaan · 2025-07-02T22:33:04Z

internal/services/network/network_manager_ipam_pool_static_cidr_resource_test.go

+  ipam_pool_id     = azurerm_network_manager_ipam_pool.test.id
+  address_prefixes = ["10.0.0.0/26", "10.0.0.128/27"]
+  lifecycle {
+    ignore_changes = [number_of_ip_addresses_to_allocate]


Would an option to split this into two resources work better instead of using ignore_changes?

Hi, I think it isn't necessary to split this into two separate resources, because it's the same resource type with the same function; only the IP address can be set from prefixes or a number.

I have added DiffSuppressFunc to suppress the diff, ignore_changes is not needed now.

…erm into nm_ipam_sc

…m into nm_ipam_sc

teowa · 2025-07-23T08:12:28Z

local test results:

…m into nm_ipam_sc

wyattfry

LGTM 👍

Did you already consider making the number_of_ip_addresses property just the exponent (2^x) instead of having to enter a power-of-two integer? Or the CIDR number prefix / subnet size (following the Portal UI)

teowa · 2025-08-19T05:46:58Z

Hi @wyattfry , thanks for the review! The Portal UI actually maps to the address_prefix field, we can set the value like address_prefix = ["10.0.0.0/30", "10.0.0.128/27"]. It's a good point to expose an exponent number that accepts an integer between 0-64 to set the number of IP addresses. This is more user-friendly, but we might lose some consistency with the API and other resources, such as VNet, that already use the similar number_of_ip_addresses field.

wyattfry · 2025-08-19T12:59:14Z

Hi @wyattfry , thanks for the review! The Portal UI actually maps to the address_prefix field, we can set the value like address_prefix = ["10.0.0.0/30", "10.0.0.128/27"]. It's a good point to expose an exponent number that accepts an integer between 0-64 to set the number of IP addresses. This is more user-friendly, but we might lose some consistency with the API and other resources, such as VNet, that already use the similar number_of_ip_addresses field.

Great point, let's go with consistency. I don't know if it would be worth mentioning anywhere that Terraform / HCL's built-in functions can be useful here, e.g. pow(2, x), which works up to 2^53 before losing precision.

wyattfry · 2025-08-22T15:41:16Z

website/docs/r/network_manager_ipam_pool_static_cidr.html.markdown

+
+* `address_prefixes` - (Optional) Specifies a list of IPv4 or IPv6 IP address prefixes which will be allocated to the Static CIDR.
+
+-> **Note:** Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified.


For information about avoiding errors, we're trying to move everything to use the tilde + greater than combo. Not terribly important, but it would help make the docs more consistent. Don't worry about fixing this instance, just a consideration for the future.

Suggested change

-> **Note:** Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified.

~> **Note:** Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified.

github-actions · 2025-09-22T02:20:58Z

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.

new resource azurerm_network_manager_ipam_pool_static_cidr

01a4409

teowa requested a review from a team as a code owner May 8, 2025 10:49

teowa marked this pull request as draft May 8, 2025 10:49

github-actions bot added documentation service/network size/XL labels May 8, 2025

ms-zhenhua suggested changes May 12, 2025

View reviewed changes

github-actions bot added the waiting-response label May 12, 2025

add validate for number_of_ip_addresses

4924b60

github-actions bot removed the waiting-response label May 12, 2025

ms-zhenhua reviewed May 14, 2025

View reviewed changes

website/docs/r/network_manager_ipam_pool_static_cidr.html.markdown Outdated Show resolved Hide resolved

github-actions bot added the waiting-response label May 14, 2025

Update website/docs/r/network_manager_ipam_pool_static_cidr.html.mark…

95ec49e

…down Co-authored-by: Zhenhua Hu <zhhu@microsoft.com>

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

ms-zhenhua previously approved these changes May 14, 2025

View reviewed changes

teowa marked this pull request as ready for review May 14, 2025 03:28

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

7bde169

…m into nm_ipam_sc

teowa dismissed ms-zhenhua’s stale review via 7bde169 June 27, 2025 00:06

harsimranmaan approved these changes Jul 2, 2025

View reviewed changes

teowa added 3 commits July 23, 2025 08:05

add diff suppress

328c0c2

Merge branch 'nm_ipam_sc' of github.com:teowa/terraform-provider-azur…

13fda1e

…erm into nm_ipam_sc

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

cbc7fd8

…m into nm_ipam_sc

teowa requested review from WodansSon and magodo as code owners July 23, 2025 08:06

rcskosir added the new-resource label Aug 6, 2025

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

3f91a96

…m into nm_ipam_sc

github-actions bot added the waiting-response label Aug 15, 2025

fix doc

2474243

github-actions bot removed the waiting-response label Aug 18, 2025

wyattfry approved these changes Aug 19, 2025

View reviewed changes

wyattfry self-assigned this Aug 19, 2025

wyattfry merged commit 5b7a6a3 into hashicorp:main Aug 19, 2025
33 checks passed

github-actions bot added this to the v4.41.0 milestone Aug 19, 2025

wyattfry added a commit that referenced this pull request Aug 20, 2025

Update CHANGELOG.md #29501

f1a4161

mbfrahry pushed a commit that referenced this pull request Aug 21, 2025

Update CHANGELOG.md #29501

d776c45

wyattfry reviewed Aug 22, 2025

View reviewed changes

github-actions bot added the waiting-response label Aug 22, 2025

github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2025


		* `address_prefixes` - (Optional) Specifies a list of IPv4 or IPv6 IP address prefixes which will be allocated to the Static CIDR.

		-> NOTE: Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified. If you set either property, the Azure API will automatically configure the other. Therefore, if you set `address_prefixes`, consider adding `number_of_ip_addresses_to_allocate` to `ignore_changes` to avoid plan diff.


		* `number_of_ip_addresses_to_allocate` - (Optional) The number of IP addresses to allocated to the Static CIDR. The value must be a string that represents a positive number, e.g., `"16"`.

		-> NOTE: Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified. If you set either property, the Azure API will automatically configure the other. Therefore, if you set `number_of_ip_addresses_to_allocate`, consider adding `address_prefixes` to `ignore_changes` to avoid plan diff.


		-> NOTE: Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified. If you set either property, the Azure API will automatically configure the other. Therefore, if you set `number_of_ip_addresses_to_allocate`, consider adding `address_prefixes` to `ignore_changes` to avoid plan diff.

		-> NOTE: If `number_of_ip_addresses_to_allocate` is not set to a power of 2, Azure API will return the nearest power of 2. For instance, if `number_of_ip_addresses_to_allocate` is `"20"`, Azure API will automatically update it to `"32"`. In such cases, please update the `number_of_ip_addresses_to_allocate` in the configuration file to this new value.

	-> Note: Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified.
	~> Note: Exactly one of `address_prefixes` or `number_of_ip_addresses_to_allocate` must be specified.

Conversation

teowa commented May 8, 2025

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

Uh oh!

ms-zhenhua 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teowa commented May 13, 2025

Uh oh!

Uh oh!

ms-zhenhua left a comment

Choose a reason for hiding this comment

Uh oh!

teowa commented Jun 2, 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!

teowa commented Jul 23, 2025

Uh oh!

wyattfry left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teowa commented Aug 19, 2025

Uh oh!

wyattfry commented Aug 19, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wyattfry left a comment •

edited

Loading