From ab8798c566fbe3631ea0eca4ca09e18f30fba932 Mon Sep 17 00:00:00 2001 From: Aris van Ommeren Date: Fri, 24 Sep 2021 19:02:59 +0200 Subject: [PATCH 1/3] `d/azurerm_public_ips`: Deprecate `attached` for `attachment_status` --- .../network/public_ips_data_source.go | 32 +++++++++++++++++-- .../network/public_ips_data_source_test.go | 24 ++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/internal/services/network/public_ips_data_source.go b/internal/services/network/public_ips_data_source.go index b3eec7c52950..aece3b0112e4 100644 --- a/internal/services/network/public_ips_data_source.go +++ b/internal/services/network/public_ips_data_source.go @@ -30,9 +30,27 @@ func dataSourcePublicIPs() *pluginsdk.Resource { Optional: true, }, + // TODO - Remove in 3.0. "attached": { - Type: pluginsdk.TypeBool, + Type: pluginsdk.TypeBool, + Optional: true, + Deprecated: "This property has been deprecated in favour of `attachment_status` to improve filtering", + ConflictsWith: []string{ + "attachment_status", + }, + }, + + "attachment_status": { + Type: pluginsdk.TypeString, Optional: true, + ValidateFunc: validation.StringInSlice([]string{ + "Attached", + "Unattached", + "All", + }, false), + ConflictsWith: []string{ + "attached", + }, }, "allocation_type": { @@ -99,8 +117,18 @@ func dataSourcePublicIPsRead(d *pluginsdk.ResourceData, meta interface{}) error } } + attachmentStatus, attachmentStatusOk := d.GetOk("attachment_status") + if attachmentStatus.(string) == "Attached" && !nicIsAttached { + continue + } + if attachmentStatus.(string) == "Unattached" && nicIsAttached { + continue + } + + // Deprecated for `attachment_status`, remove in 3.0 + // Removal will also change behaviour for data sources without `attachment_status` set attachedOnly := d.Get("attached").(bool) - if attachedOnly != nicIsAttached { + if !attachmentStatusOk && attachedOnly != nicIsAttached { continue } diff --git a/internal/services/network/public_ips_data_source_test.go b/internal/services/network/public_ips_data_source_test.go index 8501e0ac7a27..4a31165f0e4a 100644 --- a/internal/services/network/public_ips_data_source_test.go +++ b/internal/services/network/public_ips_data_source_test.go @@ -34,7 +34,10 @@ func TestAccDataSourcePublicIPs_assigned(t *testing.T) { r := PublicIPsResource{} attachedDataSourceName := "data.azurerm_public_ips.attached" + attachedDataSourceName2 := "data.azurerm_public_ips.attached2" unattachedDataSourceName := "data.azurerm_public_ips.unattached" + unattachedDataSourceName2 := "data.azurerm_public_ips.unattached2" + unattachedAndAttachedDataSourceName := "data.azurerm_public_ips.unattached_and_attached" data.DataSourceTest(t, []acceptance.TestStep{ { @@ -45,8 +48,14 @@ func TestAccDataSourcePublicIPs_assigned(t *testing.T) { Check: acceptance.ComposeTestCheckFunc( acceptance.TestCheckResourceAttr(attachedDataSourceName, "public_ips.#", "3"), acceptance.TestCheckResourceAttr(attachedDataSourceName, "public_ips.0.name", fmt.Sprintf("acctestpip%s-0", data.RandomString)), + acceptance.TestCheckResourceAttr(attachedDataSourceName2, "public_ips.#", "3"), + acceptance.TestCheckResourceAttr(attachedDataSourceName2, "public_ips.0.name", fmt.Sprintf("acctestpip%s-0", data.RandomString)), acceptance.TestCheckResourceAttr(unattachedDataSourceName, "public_ips.#", "4"), acceptance.TestCheckResourceAttr(unattachedDataSourceName, "public_ips.0.name", fmt.Sprintf("acctestpip%s-3", data.RandomString)), + acceptance.TestCheckResourceAttr(unattachedDataSourceName2, "public_ips.#", "4"), + acceptance.TestCheckResourceAttr(unattachedDataSourceName2, "public_ips.0.name", fmt.Sprintf("acctestpip%s-3", data.RandomString)), + acceptance.TestCheckResourceAttr(unattachedAndAttachedDataSourceName, "public_ips.#", "7"), + acceptance.TestCheckResourceAttr(unattachedAndAttachedDataSourceName, "public_ips.0.name", fmt.Sprintf("acctestpip%s-0", data.RandomString)), ), }, }) @@ -118,11 +127,26 @@ func (r PublicIPsResource) attachedDataSource(data acceptance.TestData) string { %s data "azurerm_public_ips" "unattached" { + resource_group_name = azurerm_resource_group.test.name + attachment_status = "Unattached" +} + +data "azurerm_public_ips" "unattached2" { resource_group_name = azurerm_resource_group.test.name attached = false } +data "azurerm_public_ips" "unattached_and_attached" { + resource_group_name = azurerm_resource_group.test.name + attachment_status = "All" +} + data "azurerm_public_ips" "attached" { + resource_group_name = azurerm_resource_group.test.name + attachment_status = "Attached" +} + +data "azurerm_public_ips" "attached2" { resource_group_name = azurerm_resource_group.test.name attached = true } From 630661bbaa4f33b0de7de8efd592e9c528f8a282 Mon Sep 17 00:00:00 2001 From: Aris van Ommeren Date: Fri, 24 Sep 2021 19:50:29 +0200 Subject: [PATCH 2/3] Docs --- internal/services/network/public_ips_data_source.go | 2 +- website/docs/d/public_ips.html.markdown | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/services/network/public_ips_data_source.go b/internal/services/network/public_ips_data_source.go index aece3b0112e4..973b05c0be66 100644 --- a/internal/services/network/public_ips_data_source.go +++ b/internal/services/network/public_ips_data_source.go @@ -46,7 +46,7 @@ func dataSourcePublicIPs() *pluginsdk.Resource { ValidateFunc: validation.StringInSlice([]string{ "Attached", "Unattached", - "All", + "All", // TODO - Remove "All" in 3.0. }, false), ConflictsWith: []string{ "attached", diff --git a/website/docs/d/public_ips.html.markdown b/website/docs/d/public_ips.html.markdown index efed020d3f68..aa96d3ae5b60 100644 --- a/website/docs/d/public_ips.html.markdown +++ b/website/docs/d/public_ips.html.markdown @@ -22,7 +22,8 @@ data "azurerm_public_ips" "example" { ## Argument Reference * `resource_group_name` - Specifies the name of the resource group. -* `attached` - (Optional) Filter to include IP Addresses which are attached to a device, such as a VM/LB (`true`) or unattached (`false`). +* `attached` - (Optional) Filter to include IP Addresses which are attached to a device, such as a VM/LB (`true`) or unattached (`false`). This option is deprecated for `attachment_status`. Default value is `false`, unless `attachment_status` is set. +* `attachment_status` - (Optional) Filter to include IP Addresses which are attached to a device, such as a VM/LB (`Attached`) or unattached (`Unattached`). To allow for both, use `All`. * `name_prefix` - (Optional) A prefix match used for the IP Addresses `name` field, case sensitive. * `allocation_type` - (Optional) The Allocation Type for the Public IP Address. Possible values include `Static` or `Dynamic`. From 3bd636290c5cf067966b865152e2d7eacf20e6a3 Mon Sep 17 00:00:00 2001 From: Aris van Ommeren Date: Mon, 4 Oct 2021 17:28:18 +0200 Subject: [PATCH 3/3] Fix comments --- internal/services/network/public_ips_data_source.go | 4 ++-- website/docs/d/public_ips.html.markdown | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/services/network/public_ips_data_source.go b/internal/services/network/public_ips_data_source.go index 973b05c0be66..6b4956ffe642 100644 --- a/internal/services/network/public_ips_data_source.go +++ b/internal/services/network/public_ips_data_source.go @@ -118,10 +118,10 @@ func dataSourcePublicIPsRead(d *pluginsdk.ResourceData, meta interface{}) error } attachmentStatus, attachmentStatusOk := d.GetOk("attachment_status") - if attachmentStatus.(string) == "Attached" && !nicIsAttached { + if attachmentStatusOk && attachmentStatus.(string) == "Attached" && !nicIsAttached { continue } - if attachmentStatus.(string) == "Unattached" && nicIsAttached { + if attachmentStatusOk && attachmentStatus.(string) == "Unattached" && nicIsAttached { continue } diff --git a/website/docs/d/public_ips.html.markdown b/website/docs/d/public_ips.html.markdown index aa96d3ae5b60..fd4a168390c7 100644 --- a/website/docs/d/public_ips.html.markdown +++ b/website/docs/d/public_ips.html.markdown @@ -22,7 +22,6 @@ data "azurerm_public_ips" "example" { ## Argument Reference * `resource_group_name` - Specifies the name of the resource group. -* `attached` - (Optional) Filter to include IP Addresses which are attached to a device, such as a VM/LB (`true`) or unattached (`false`). This option is deprecated for `attachment_status`. Default value is `false`, unless `attachment_status` is set. * `attachment_status` - (Optional) Filter to include IP Addresses which are attached to a device, such as a VM/LB (`Attached`) or unattached (`Unattached`). To allow for both, use `All`. * `name_prefix` - (Optional) A prefix match used for the IP Addresses `name` field, case sensitive. * `allocation_type` - (Optional) The Allocation Type for the Public IP Address. Possible values include `Static` or `Dynamic`.