Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_eventhub - deprecate namespace_name and resource_group_name in favour of namespace_id #28055

Merged
merged 12 commits into from
Nov 22, 2024
+150 −73
Merged

azurerm_eventhub - deprecate namespace_name and resource_group_name in favour of namespace_id #28055

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9c3fe47
deprecate namespace_name in favour of namespace_id
catriona-m Nov 18, 2024
b404816
update doc
catriona-m Nov 18, 2024
244ec13
fix guide,docs and tests
catriona-m Nov 19, 2024
85b2ff5
remove ForceNew and update test
catriona-m Nov 19, 2024
a3906b6
remove space
catriona-m Nov 19, 2024
647122e
deprecate resource_group_name
catriona-m Nov 21, 2024
3e26c46
Merge branch 'main' into cm/eventhubid
catriona-m Nov 21, 2024
e60b082
remove deprecated resource_group_name from docs
catriona-m Nov 21, 2024
ad8d0d1
lint
catriona-m Nov 21, 2024
05f0ef1
Merge branch 'main' into cm/eventhubid
catriona-m Nov 22, 2024
1efcd20
add deprecation notes
catriona-m Nov 22, 2024
08a148d
Update eventhub.html.markdown
catriona-m Nov 22, 2024
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
57 changes: 50 additions & 7 deletions internal/services/eventhub/eventhub_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/hashicorp/go-azure-sdk/resource-manager/eventhub/2022-01-01-preview/namespaces"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
"github.com/hashicorp/terraform-provider-azurerm/internal/features"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/eventhub/validate"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
Expand All @@ -25,7 +26,7 @@ import (
var eventHubResourceName = "azurerm_eventhub"

func resourceEventHub() *pluginsdk.Resource {
return &pluginsdk.Resource{
r := &pluginsdk.Resource{
Create: resourceEventHubCreate,
Read: resourceEventHubRead,
Update: resourceEventHubUpdate,
Expand All @@ -51,11 +52,11 @@ func resourceEventHub() *pluginsdk.Resource {
ValidateFunc: validate.ValidateEventHubName(),
},

"namespace_name": {
"namespace_id": {
Type: pluginsdk.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validate.ValidateEventHubNamespaceName(),
ValidateFunc: namespaces.ValidateNamespaceID,
},

"resource_group_name": commonschema.ResourceGroupName(),
Copy link
Member

Choose a reason for hiding this comment

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

@catriona-m as pointed out by @CorrenSoft this probably wants to be deprecated as well

Expand Down Expand Up @@ -163,6 +164,27 @@ func resourceEventHub() *pluginsdk.Resource {
},
},
}

if !features.FivePointOhBeta() {
r.Schema["namespace_id"] = &pluginsdk.Schema{
Type: pluginsdk.TypeString,
Optional: true,
Computed: true,
ExactlyOneOf: []string{"namespace_id", "namespace_name"},
ValidateFunc: namespaces.ValidateNamespaceID,
}

r.Schema["namespace_name"] = &pluginsdk.Schema{
Type: pluginsdk.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validate.ValidateEventHubNamespaceName(),
ExactlyOneOf: []string{"namespace_id", "namespace_name"},
Deprecated: "`namespace_name` has been deprecated in favour of `namespace_id`",
Copy link
Member

Choose a reason for hiding this comment

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

Can't believe I didn't catch this in the previous review, but could we update this so it's consistent with other deprecation messages in the provider and also clear in that it will be removed in the next major release

Suggested change
Deprecated: "`namespace_name` has been deprecated in favour of `namespace_id`",
Deprecated: "`namespace_name` has been deprecated in favour of `namespace_id` and will be removed in v5.0 of the AzureRM Provider",

}
}

return r
}

func resourceEventHubCreate(d *pluginsdk.ResourceData, meta interface{}) error {
Expand All @@ -173,7 +195,19 @@ func resourceEventHubCreate(d *pluginsdk.ResourceData, meta interface{}) error {

log.Printf("[INFO] preparing arguments for Azure ARM EventHub creation.")

id := eventhubs.NewEventhubID(subscriptionId, d.Get("resource_group_name").(string), d.Get("namespace_name").(string), d.Get("name").(string))
namespaceName := ""
if v := d.Get("namespace_id").(string); v != "" {
namespaceId, err := namespaces.ParseNamespaceID(v)
if err != nil {
return err
}
namespaceName = namespaceId.NamespaceName
}
if !features.FivePointOhBeta() && namespaceName == "" {
namespaceName = d.Get("namespace_name").(string)
}

id := eventhubs.NewEventhubID(subscriptionId, d.Get("resource_group_name").(string), namespaceName, d.Get("name").(string))

if d.IsNewResource() {
existing, err := client.Get(ctx, id)
Expand Down Expand Up @@ -218,7 +252,10 @@ func resourceEventHubUpdate(d *pluginsdk.ResourceData, meta interface{}) error {

log.Printf("[INFO] preparing arguments for Azure ARM EventHub update.")

id := eventhubs.NewEventhubID(subscriptionId, d.Get("resource_group_name").(string), d.Get("namespace_name").(string), d.Get("name").(string))
id, err := eventhubs.ParseEventhubID(d.Id())
if err != nil {
return err
}

if d.HasChange("partition_count") {
o, n := d.GetChange("partition_count")
Expand Down Expand Up @@ -253,7 +290,7 @@ func resourceEventHubUpdate(d *pluginsdk.ResourceData, meta interface{}) error {
parameters.Properties.CaptureDescription = expandEventHubCaptureDescription(d)
}

if _, err := client.CreateOrUpdate(ctx, id, parameters); err != nil {
if _, err := client.CreateOrUpdate(ctx, *id, parameters); err != nil {
return err
}

Expand Down Expand Up @@ -282,9 +319,15 @@ func resourceEventHubRead(d *pluginsdk.ResourceData, meta interface{}) error {
}

d.Set("name", id.EventhubName)
d.Set("namespace_name", id.NamespaceName)
d.Set("resource_group_name", id.ResourceGroupName)

if !features.FivePointOhBeta() {
d.Set("namespace_name", id.NamespaceName)
}

namespaceId := namespaces.NewNamespaceID(id.SubscriptionId, id.ResourceGroupName, id.NamespaceName)
d.Set("namespace_id", namespaceId.ID())

if model := resp.Model; model != nil {
if props := model.Properties; props != nil {
d.Set("partition_count", props.PartitionCount)
Expand Down
42 changes: 35 additions & 7 deletions internal/services/eventhub/eventhub_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance"
"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
"github.com/hashicorp/terraform-provider-azurerm/internal/features"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/eventhub/validate"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
"github.com/hashicorp/terraform-provider-azurerm/utils"
Expand Down Expand Up @@ -360,7 +361,8 @@ func (EventHubResource) Exists(ctx context.Context, clients *clients.Client, sta
}

func (EventHubResource) basic(data acceptance.TestData, partitionCount int) string {
return fmt.Sprintf(`
if !features.FivePointOhBeta() {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}
Expand All @@ -384,6 +386,32 @@ resource "azurerm_eventhub" "test" {
partition_count = %d
message_retention = 1
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, partitionCount)
}
return fmt.Sprintf(`
provider "azurerm" {
features {}
}

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

resource "azurerm_eventhub_namespace" "test" {
name = "acctesteventhubnamespace-%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
sku = "Basic"
}

resource "azurerm_eventhub" "test" {
name = "acctesteventhub-%d"
namespace_id = azurerm_eventhub_namespace.test.id
Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming all the test configs have been updated to use namespace_id instead of namespace_name now, provided the migration path for users switching from namespace_name -> namespace_id works this is fine but it might be worth keeping one test config with namespace_name to ensure it's still functional for the time being e.g.

func (EventHubResource) basic(data acceptance.TestData, partitionCount int) string {
     if !features.FivePointOhBeta() {
        // return config using namespace_name here
}

// return config using namespace_id here
}

resource_group_name = azurerm_resource_group.test.name
partition_count = %d
message_retention = 1
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, partitionCount)
}

Expand All @@ -394,7 +422,7 @@ func (EventHubResource) requiresImport(data acceptance.TestData) string {

resource "azurerm_eventhub" "import" {
name = azurerm_eventhub.test.name
namespace_name = azurerm_eventhub.test.namespace_name
namespace_id = azurerm_eventhub.test.namespace_id
resource_group_name = azurerm_eventhub.test.resource_group_name
partition_count = azurerm_eventhub.test.partition_count
message_retention = azurerm_eventhub.test.message_retention
Expand Down Expand Up @@ -422,7 +450,7 @@ resource "azurerm_eventhub_namespace" "test" {

resource "azurerm_eventhub" "test" {
name = "acctesteventhub-%d"
namespace_name = azurerm_eventhub_namespace.test.name
namespace_id = azurerm_eventhub_namespace.test.id
resource_group_name = azurerm_resource_group.test.name
partition_count = 10
message_retention = 1
Expand Down Expand Up @@ -450,7 +478,7 @@ resource "azurerm_eventhub_namespace" "test" {

resource "azurerm_eventhub" "test" {
name = "acctest-EH-%d"
namespace_name = azurerm_eventhub_namespace.test.name
namespace_id = azurerm_eventhub_namespace.test.id
resource_group_name = azurerm_resource_group.test.name
partition_count = 2
message_retention = 7
Expand Down Expand Up @@ -493,7 +521,7 @@ resource "azurerm_eventhub_namespace" "test" {

resource "azurerm_eventhub" "test" {
name = "acctest-EH%d"
namespace_name = azurerm_eventhub_namespace.test.name
namespace_id = azurerm_eventhub_namespace.test.id
resource_group_name = azurerm_resource_group.test.name
partition_count = 2
message_retention = 7
Expand Down Expand Up @@ -536,7 +564,7 @@ resource "azurerm_eventhub_namespace" "test" {

resource "azurerm_eventhub" "test" {
name = "acctest-EH-%d"
namespace_name = azurerm_eventhub_namespace.test.name
namespace_id = azurerm_eventhub_namespace.test.id
resource_group_name = azurerm_resource_group.test.name
partition_count = 2
message_retention = 5
Expand Down Expand Up @@ -564,7 +592,7 @@ resource "azurerm_eventhub_namespace" "test" {

resource "azurerm_eventhub" "test" {
name = "acctesteventhub-%d"
namespace_name = azurerm_eventhub_namespace.test.name
namespace_id = azurerm_eventhub_namespace.test.id
resource_group_name = azurerm_resource_group.test.name
partition_count = 5
message_retention = 1
Expand Down
28 changes: 16 additions & 12 deletions website/docs/5.0-upgrade-guide.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,22 @@ Please follow the format in the example below for listing breaking changes in re
* The `example_property_with_changed_default` property now defaults to `NewDefault`.
```

### `azurerm_cdn_frontdoor_custom_domain`

* The `tls.minimum_tls_version` property no longer accepts `TLS10` as a value.

### `azurerm_eventhub`

* The deprecated `namespace_name` property has been removed in favour of the `namespace_id` property.
Copy link
Member

Choose a reason for hiding this comment

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

We need to add a line for the resource_group_name property here as well


### `azurerm_mssql_database`

* The properties `weekly_retention`, `monthly_retention` and `yearly_retention` now default to `PT0S`.

### `azurerm_mssql_managed_database`

* The properties `weekly_retention`, `monthly_retention` and `yearly_retention` now default to `PT0S`.

### `azurerm_sentinel_alert_rule_fusion`

* The deprecated `name` property has been removed.
Expand All @@ -76,18 +92,6 @@ Please follow the format in the example below for listing breaking changes in re
* The deprecated `storage_account_name` property has been removed in favour of the `storage_account_id` property.
* The deprecated `resource_manager_id` property has been removed in favour of the `id` property.

### `azurerm_cdn_frontdoor_custom_domain`

* The `tls.minimum_tls_version` property no longer accepts `TLS10` as a value.

### `azurerm_mssql_database`

* The properties `weekly_retention`, `monthly_retention` and `yearly_retention` now default to `PT0S`.

### `azurerm_mssql_managed_database`

* The properties `weekly_retention`, `monthly_retention` and `yearly_retention` now default to `PT0S`.

## Breaking Changes in Data Sources

Please follow the format in the example below for listing breaking changes in data sources:
Expand Down
4 changes: 2 additions & 2 deletions website/docs/r/eventhub.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ resource "azurerm_eventhub_namespace" "example" {

resource "azurerm_eventhub" "example" {
name = "acceptanceTestEventHub"
namespace_name = azurerm_eventhub_namespace.example.name
namespace_id = azurerm_eventhub_namespace.example.id
resource_group_name = azurerm_resource_group.example.name
partition_count = 2
message_retention = 1
Expand All @@ -45,7 +45,7 @@ The following arguments are supported:

* `name` - (Required) Specifies the name of the EventHub resource. Changing this forces a new resource to be created.

* `namespace_name` - (Required) Specifies the name of the EventHub Namespace. Changing this forces a new resource to be created.
* `namespace_id` - (Optional) Specifies the id of the EventHub Namespace. Changing this forces a new resource to be created.

* `resource_group_name` - (Required) The name of the resource group in which the EventHub's parent Namespace exists. Changing this forces a new resource to be created.

Expand Down
Loading