azurerm_nat_gateway_public_ip_association - support associating IPv6 Public IP#32176
azurerm_nat_gateway_public_ip_association - support associating IPv6 Public IP#32176sreallymatt merged 5 commits intohashicorp:mainfrom
azurerm_nat_gateway_public_ip_association - support associating IPv6 Public IP#32176Conversation
wuxu92
left a comment
There was a problem hiding this comment.
Thanks for the PR! I left some comments; we can take another look once they're addressed.
| } | ||
| } | ||
|
|
||
| func resourceNATGatewayPublicIpAssociationCustomizeDiff(ctx context.Context, d *pluginsdk.ResourceDiff, meta any) error { |
There was a problem hiding this comment.
as we are supporting both ipv4 and ipv6 now, this CustomizeDiff is not necessary anymore, we can error out in Create if configured id not eixsts.
| } | ||
|
|
||
| func natGatewayPublicIpAssociationIsIPv6(publicIPAddress *publicipaddresses.PublicIPAddress) bool { | ||
| return pointer.From(publicIPAddress.Properties.PublicIPAddressVersion) == publicipaddresses.IPVersionIPvSix |
There was a problem hiding this comment.
publicIPAddress.Properties can be nil and panic here
| return nil | ||
| } | ||
|
|
||
| func validateNATGatewayPublicIpAssociation(natGateway *natgateways.NatGateway, publicIPAddress *publicipaddresses.PublicIPAddress) error { |
There was a problem hiding this comment.
I'm unsure if this validate is solid and necessary here. if the api call is a light operation, may be we should defer to service side to validate it
There was a problem hiding this comment.
make sense, the validation is removed
|
Hi @wuxu92 , thanks for reviewing this. I have updated the code, please take another look. |
wuxu92
left a comment
There was a problem hiding this comment.
Some comments about the code format, it's good to go once addressed. Thanks!
| if model := natGateway.Model; model != nil && model.Properties != nil { | ||
| if !natGatewayPublicIpAssociationExists(model.Properties, id.Second.ID()) { | ||
| log.Printf("[DEBUG] Association between %s and %s was not found - removing from state", id.First, id.Second) | ||
| d.SetId("") | ||
| return nil | ||
| } | ||
| } else { | ||
| return fmt.Errorf("retrieving %s: `model` or `properties` was nil", id.First) | ||
| } |
There was a problem hiding this comment.
make it clear:
| if model := natGateway.Model; model != nil && model.Properties != nil { | |
| if !natGatewayPublicIpAssociationExists(model.Properties, id.Second.ID()) { | |
| log.Printf("[DEBUG] Association between %s and %s was not found - removing from state", id.First, id.Second) | |
| d.SetId("") | |
| return nil | |
| } | |
| } else { | |
| return fmt.Errorf("retrieving %s: `model` or `properties` was nil", id.First) | |
| } | |
| if natGateway.Model == nil || natGateway.Model.Properties == nil { | |
| return fmt.Errorf("retrieving %s: `model` or `properties` was nil", id.First) | |
| } | |
| if !natGatewayPublicIpAssociationExists(natGateway.Model.Properties, id.Second.ID()) { | |
| log.Printf("[DEBUG] Association between %s and %s was not found - removing from state", id.First, id.Second) | |
| d.SetId("") | |
| return nil | |
| } |
| func natGatewayPublicIpAssociationIsIPv6(publicIPAddress *publicipaddresses.PublicIPAddress) bool { | ||
| if publicIPAddress == nil || publicIPAddress.Properties == nil { | ||
| return false | ||
| } | ||
|
|
||
| return pointer.From(publicIPAddress.Properties.PublicIPAddressVersion) == publicipaddresses.IPVersionIPvSix | ||
| } |
There was a problem hiding this comment.
since only one caller for this method, can we inline it?
| isIPv6 := natGatewayPublicIpAssociationIsIPv6(publicIPAddress.Model) | ||
|
|
||
| if strings.EqualFold(*existingPublicIPAddress.Id, publicIpAddressId.ID()) { | ||
| return tf.ImportAsExistsError("azurerm_nat_gateway_public_ip_association", id.ID()) | ||
| } | ||
| id := commonids.NewCompositeResourceID(natGatewayId, publicIpAddressId) | ||
|
|
||
| publicIpAddresses = append(publicIpAddresses, existingPublicIPAddress) | ||
| publicIpAddresses := pointer.From(natGateway.Model.Properties.PublicIPAddresses) | ||
| if isIPv6 { | ||
| publicIpAddresses = pointer.From(natGateway.Model.Properties.PublicIPAddressesV6) | ||
| } |
There was a problem hiding this comment.
make it clear:
| isIPv6 := natGatewayPublicIpAssociationIsIPv6(publicIPAddress.Model) | |
| if strings.EqualFold(*existingPublicIPAddress.Id, publicIpAddressId.ID()) { | |
| return tf.ImportAsExistsError("azurerm_nat_gateway_public_ip_association", id.ID()) | |
| } | |
| id := commonids.NewCompositeResourceID(natGatewayId, publicIpAddressId) | |
| publicIpAddresses = append(publicIpAddresses, existingPublicIPAddress) | |
| publicIpAddresses := pointer.From(natGateway.Model.Properties.PublicIPAddresses) | |
| if isIPv6 { | |
| publicIpAddresses = pointer.From(natGateway.Model.Properties.PublicIPAddressesV6) | |
| } | |
| isIPv6 := pointer.From(publicIPAddress.Model.Properties.PublicIPAddressVersion) == publicipaddresses.IPVersionIPvSix | |
| id := commonids.NewCompositeResourceID(natGatewayId, publicIpAddressId) | |
| gatewayProps := natGateway.Model.Properties | |
| publicIpAddresses := pointer.From(gatewayProps.PublicIPAddresses) | |
| if isIPv6 { | |
| publicIpAddresses = pointer.From(gagewayProps.PublicIPAddressesV6) | |
| } |
azurerm_nat_gateway_public_ip_association - supports association with IPv6 Public IPazurerm_nat_gateway_public_ip_association - support associating IPv6 Public IP
gerrytan
left a comment
There was a problem hiding this comment.
Thank you, this PR looks good to me.
sreallymatt
left a comment
There was a problem hiding this comment.
Thanks @teowa - LGTM ✅

Community Note
Description
supersede #31321
PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_nat_gateway_public_ip_association- support associating IPv6 Public IPThis is a (please select all that apply):
Related Issue(s)
Fixes #0000
AI Assistance Disclosure
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.