Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions internal/services/network/nat_gateway_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,27 @@ func resourceNatGatewaySchema() map[string]*pluginsdk.Schema {
},

"sku_name": {
Type: pluginsdk.TypeString,
Optional: true,
Default: string(natgateways.NatGatewaySkuNameStandard),
ValidateFunc: validation.StringInSlice([]string{
string(natgateways.NatGatewaySkuNameStandard),
}, false),
Type: pluginsdk.TypeString,
Optional: true,
Default: string(natgateways.NatGatewaySkuNameStandard),
ValidateFunc: validation.StringInSlice(natgateways.PossibleValuesForNatGatewaySkuName(), false),
},

"zones": commonschema.ZonesMultipleOptionalForceNew(),
"zones": {
Type: schema.TypeSet,
Optional: true,
// NOTE: O+C Azure may return availability zones from the API when not specified by the user
Computed: true,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making this O+C breaks the existing behaviour of replacing the resource when a user removes zones from their deployment (when sku is Standard). We should try to keep this behaviour intact and we could use a DiffSuppressFunc instead.

Additionally, O+C doesn't entirely resolve the zones diff, currently a user could specify zones for StandardV2 in config (e.g. zones = ["1"]) which would lead to a perpetual diff, we should prevent users from setting zones on StandardV2 with a CustomizeDiff

e.g.

DiffSuppress:

		DiffSuppressOnRefresh: true,
			DiffSuppressFunc: func(_, _, _ string, d *schema.ResourceData) bool {
				// when `sku_name` is `StandardV2`, Azure automatically spreads the deployment across all zones in the region
				return d.Get("sku_name").(string) == string(natgateways.NatGatewaySkuNameStandardVTwo)
			},

CustomizeDiff:

		CustomizeDiff: func(_ context.Context, diff *schema.ResourceDiff, _ interface{}) error {
			if diff.Get("sku_name").(string) == string(natgateways.NatGatewaySkuNameStandardVTwo) {
				if !diff.GetRawConfig().AsValueMap()["zones"].IsNull() {
					return fmt.Errorf("%s resources with `sku_name` set to `%s` are zone-redundant by default, Azure automatically deploys across all available zones. The `zones` argument must be omitted", natGatewayResourceName, natgateways.NatGatewaySkuNameStandardVTwo)
				}
			}

			return nil
		},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense, code updated.

ForceNew: true,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: validation.StringInSlice([]string{
"1",
"2",
"3",
}, false),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should start limiting input to zone/zones arguments. There is at least one region that supports 4 (eastus2euap, while a bit of a special case, users could be using it)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated; now we just check that it isn't an empty string.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from PG here - new creates of Basic SKU public IP resources as of March 2025 deprecation date is not supported. We are working on blocking in NRP, just hasn't rolled out yet.

},
},

"resource_guid": {
Type: pluginsdk.TypeString,
Expand Down
35 changes: 35 additions & 0 deletions internal/services/network/nat_gateway_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,21 @@ func TestAccNatGateway_update(t *testing.T) {
})
}

func TestAccNatGateway_standardVTwo(t *testing.T) {
Comment thread
sreallymatt marked this conversation as resolved.
data := acceptance.BuildTestData(t, "azurerm_nat_gateway", "test")
r := NatGatewayResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.standardVTwo(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func (t NatGatewayResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) {
id, err := natgateways.ParseNatGatewayID(state.ID)
if err != nil {
Expand Down Expand Up @@ -208,3 +223,23 @@ resource "azurerm_nat_gateway_public_ip_prefix_association" "test" {
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger)
}

func (NatGatewayResource) standardVTwo(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}

resource "azurerm_resource_group" "test" {
name = "acctestRG-network-%d"
location = "%s"
}

resource "azurerm_nat_gateway" "test" {
name = "acctestnatGateway-%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
sku_name = "StandardV2"
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger)
}
Comment thread
sreallymatt marked this conversation as resolved.
1 change: 1 addition & 0 deletions internal/services/network/public_ip_data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func TestAccDataSourcePublicIP_static(t *testing.T) {
}

func TestAccDataSourcePublicIP_dynamic(t *testing.T) {
skipIfBasicSkuPublicIPDeprecated(t)
data := acceptance.BuildTestData(t, "data.azurerm_public_ip", "test")
r := PublicIPDataSource{}

Expand Down
12 changes: 5 additions & 7 deletions internal/services/network/public_ip_prefix_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,11 @@ func resourcePublicIpPrefix() *pluginsdk.Resource {
},

"sku": {
Type: pluginsdk.TypeString,
Optional: true,
ForceNew: true,
Default: string(publicipprefixes.PublicIPPrefixSkuNameStandard),
ValidateFunc: validation.StringInSlice([]string{
string(publicipprefixes.PublicIPPrefixSkuNameStandard),
}, false),
Type: pluginsdk.TypeString,
Optional: true,
ForceNew: true,
Default: string(publicipprefixes.PublicIPPrefixSkuNameStandard),
ValidateFunc: validation.StringInSlice(publicipprefixes.PossibleValuesForPublicIPPrefixSkuName(), false),
},

"sku_tier": {
Expand Down
28 changes: 24 additions & 4 deletions internal/services/network/public_ip_prefix_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,27 @@ func TestAccPublicIpPrefix_globalTier(t *testing.T) {

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.sku_tier(data, string(publicipprefixes.PublicIPPrefixSkuTierGlobal)),
Config: r.sku_tier(data, string(publicipprefixes.PublicIPPrefixSkuNameStandard), string(publicipprefixes.PublicIPPrefixSkuTierGlobal)),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(string(publicipprefixes.PublicIPPrefixSkuNameStandard)),
check.That(data.ResourceName).Key("sku_tier").HasValue(string(publicipprefixes.PublicIPPrefixSkuTierGlobal)),
),
},
data.ImportStep(),
})
}

func TestAccPublicIpPrefix_standardVTwoGlobalTier(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_public_ip_prefix", "test")
r := PublicIpPrefixResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.sku_tier(data, string(publicipprefixes.PublicIPPrefixSkuNameStandardVTwo), string(publicipprefixes.PublicIPPrefixSkuTierGlobal)),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(string(publicipprefixes.PublicIPPrefixSkuNameStandardVTwo)),
check.That(data.ResourceName).Key("sku_tier").HasValue(string(publicipprefixes.PublicIPPrefixSkuTierGlobal)),
),
},
Expand Down Expand Up @@ -104,9 +122,10 @@ func TestAccPublicIpPrefix_regionalTier(t *testing.T) {

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.sku_tier(data, string(publicipprefixes.PublicIPPrefixSkuTierRegional)),
Config: r.sku_tier(data, string(publicipprefixes.PublicIPPrefixSkuNameStandard), string(publicipprefixes.PublicIPPrefixSkuTierRegional)),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(string(publicipprefixes.PublicIPPrefixSkuNameStandard)),
check.That(data.ResourceName).Key("sku_tier").HasValue(string(publicipprefixes.PublicIPPrefixSkuTierRegional)),
),
},
Expand Down Expand Up @@ -393,7 +412,7 @@ resource "azurerm_public_ip_prefix" "test" {
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger)
}

func (PublicIpPrefixResource) sku_tier(data acceptance.TestData, tier string) string {
func (PublicIpPrefixResource) sku_tier(data acceptance.TestData, sku string, tier string) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
Expand All @@ -408,9 +427,10 @@ resource "azurerm_public_ip_prefix" "test" {
resource_group_name = azurerm_resource_group.test.name
name = "acctestpublicipprefix-%d"
location = azurerm_resource_group.test.location
sku = "%s"
sku_tier = "%s"
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, tier)
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, sku, tier)
}

func (PublicIpPrefixResource) zonesSingle(data acceptance.TestData) string {
Expand Down
31 changes: 27 additions & 4 deletions internal/services/network/public_ip_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package network

import (
"context"
"errors"
"fmt"
"log"
"strings"
Expand Down Expand Up @@ -109,10 +110,7 @@ func resourcePublicIp() *pluginsdk.Resource {
ForceNew: true,
Default: string(publicipaddresses.PublicIPAddressSkuNameStandard),
// https://azure.microsoft.com/en-us/updates/upgrade-to-standard-sku-public-ip-addresses-in-azure-by-30-september-2025-basic-sku-will-be-retired/
Comment thread
sreallymatt marked this conversation as resolved.
ValidateFunc: validation.StringInSlice([]string{
string(publicipaddresses.PublicIPAddressSkuNameBasic),
string(publicipaddresses.PublicIPAddressSkuNameStandard),
}, false),
ValidateFunc: validation.StringInSlice(publicipaddresses.PossibleValuesForPublicIPAddressSkuName(), false),
Comment thread
sreallymatt marked this conversation as resolved.
},

"sku_tier": {
Expand Down Expand Up @@ -182,13 +180,38 @@ func resourcePublicIp() *pluginsdk.Resource {
},

CustomizeDiff: pluginsdk.CustomDiffWithAll(
pluginsdk.CustomizeDiffShim(func(_ context.Context, d *pluginsdk.ResourceDiff, _ interface{}) error {
if !isPublicIPBasicSkuDeprecatedForCreation() {
return nil
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused as to why this function is here, isPublicIPBasicSkuDeprecatedForCreation() is already true so there's no need for it?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated, this is removed.


sku := d.Get("sku").(string)
if strings.EqualFold(sku, string(publicipaddresses.PublicIPAddressSkuNameBasic)) && d.HasChanges("name", "resource_group_name", "location", "allocation_method", "edge_zone", "ip_version", "sku", "sku_tier", "public_ip_prefix_id", "ip_tags", "zones") {
return errors.New(publicIPBasicSkuCreateDeprecationMessage)
}

return nil
}),
pluginsdk.ForceNewIfChange("domain_name_label_scope", func(ctx context.Context, old, new, meta interface{}) bool {
return old.(string) != "" || new.(string) == ""
}),
),
}
}

const publicIPBasicSkuCreateDeprecationMessage = "creation of new `Basic` SKU public IP addresses is no longer permitted following its deprecation on March 31, 2025. This also affects `allocation_method` set to `Dynamic`, as it is only available with the `Basic` SKU. Existing `Basic` SKU public IP addresses can continue to be used until September 30, 2025. For more information, see https://azure.microsoft.com/updates/upgrade-to-standard-sku-public-ip-addresses-in-azure-by-30-september-2025-basic-sku-will-be-retired/"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this part is relevant given this date has passed

Existing Basic SKU public IP addresses can continue to be used until September 30, 2025.

Suggested change
const publicIPBasicSkuCreateDeprecationMessage = "creation of new `Basic` SKU public IP addresses is no longer permitted following its deprecation on March 31, 2025. This also affects `allocation_method` set to `Dynamic`, as it is only available with the `Basic` SKU. Existing `Basic` SKU public IP addresses can continue to be used until September 30, 2025. For more information, see https://azure.microsoft.com/updates/upgrade-to-standard-sku-public-ip-addresses-in-azure-by-30-september-2025-basic-sku-will-be-retired/"
const publicIPBasicSkuCreateDeprecationMessage = "creation of new `Basic` SKU public IP addresses is no longer permitted following its deprecation on March 31, 2025. This also affects `allocation_method` set to `Dynamic`, as it is only available with the `Basic` SKU. For more information, see https://azure.microsoft.com/updates/upgrade-to-standard-sku-public-ip-addresses-in-azure-by-30-september-2025-basic-sku-will-be-retired/"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated


func isPublicIPBasicSkuDeprecatedForCreation() bool {
losAngelesLocation, err := time.LoadLocation("America/Los_Angeles")
if err != nil {
losAngelesLocation = time.UTC
}

deprecationTime := time.Date(2025, time.April, 1, 0, 0, 0, 0, losAngelesLocation)
now := time.Now()
return now.After(deprecationTime.In(now.Location()))
}

func resourcePublicIpCreate(d *pluginsdk.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).Network.PublicIPAddresses
subscriptionId := meta.(*clients.Client).Account.SubscriptionId
Expand Down
Loading