New Resource: `azurerm_eventgrid_event_subscription` #2967

mbfrahry · 2019-02-28T01:27:06Z

This PR adds eventgrid event subscription to the azure provider. Addresses #1794.

=== RUN   TestAccAzureRMEventGridEventSubscription_basic
=== PAUSE TestAccAzureRMEventGridEventSubscription_basic
=== RUN   TestAccAzureRMEventGridEventSubscription_eventhub
=== PAUSE TestAccAzureRMEventGridEventSubscription_eventhub
=== RUN   TestAccAzureRMEventGridEventSubscription_update
=== PAUSE TestAccAzureRMEventGridEventSubscription_update
=== RUN   TestAccAzureRMEventGridEventSubscription_filter
=== PAUSE TestAccAzureRMEventGridEventSubscription_filter
=== CONT  TestAccAzureRMEventGridEventSubscription_basic
=== CONT  TestAccAzureRMEventGridEventSubscription_eventhub
=== CONT  TestAccAzureRMEventGridEventSubscription_update
=== CONT  TestAccAzureRMEventGridEventSubscription_filter
--- PASS: TestAccAzureRMEventGridEventSubscription_filter (178.35s)
--- PASS: TestAccAzureRMEventGridEventSubscription_basic (178.35s)
--- PASS: TestAccAzureRMEventGridEventSubscription_eventhub (238.15s)
--- PASS: TestAccAzureRMEventGridEventSubscription_update (259.45s)

katbyte

Thanks for the pr @mbfrahry,

I've left some comments inline, mostly regarding additional validation. Once thats sorted this should be good to merge.

katbyte · 2019-02-28T08:46:37Z

azurerm/resource_arm_eventgrid_event_subscription.go

+				Type:     schema.TypeString,
+				Required: true,
+				ForceNew: true,
+			},


Could we get some validation here? at the very least validate.NoEmptyStrings

katbyte · 2019-02-28T08:46:43Z

azurerm/resource_arm_eventgrid_event_subscription.go

+				Type:     schema.TypeString,
+				Required: true,
+				ForceNew: true,
+			},


Could we get some validation here? at the very least validate.NoEmptyStrings

katbyte · 2019-02-28T08:47:29Z

azurerm/resource_arm_eventgrid_event_subscription.go

+						"storage_account_id": {
+							Type:     schema.TypeString,
+							Required: true,
+						},


Could we validate this resource id? azure.ValidateResourceId

katbyte · 2019-02-28T08:47:37Z

azurerm/resource_arm_eventgrid_event_subscription.go

+						"queue_name": {
+							Type:     schema.TypeString,
+							Required: true,
+						},


Could we get some validation here? at the very least validate.NoEmptyStrings

katbyte · 2019-02-28T08:47:48Z

azurerm/resource_arm_eventgrid_event_subscription.go

+						"eventhub_id": {
+							Type:     schema.TypeString,
+							Required: true,
+						},


COuld we validate this resource id?

katbyte · 2019-02-28T08:49:29Z

azurerm/resource_arm_eventgrid_event_subscription.go

+						"event_time_to_live": {
+							Type:     schema.TypeInt,
+							Required: true,
+						},


Is there a valid rage we could validate here?

katbyte · 2019-02-28T08:53:27Z

azurerm/resource_arm_eventgrid_event_subscription.go

+func expandEventGridEventSubscriptionFilter(d *schema.ResourceData) *eventgrid.EventSubscriptionFilter {
+	filter := &eventgrid.EventSubscriptionFilter{}
+
+	if includedEvents, ok := d.GetOk("included_event_types"); ok {


I think this could be replaced with utils.ExpandeStringArray?

katbyte · 2019-02-28T08:55:00Z

azurerm/resource_arm_eventgrid_event_subscription.go

+	return nil
+}
+
+func expandEventGridEventSubscriptionLabels(d *schema.ResourceData) *[]string {


Could this be replaced with utils.ExpandStringArray?

katbyte · 2019-02-28T08:55:55Z

website/docs/r/eventgrid_event_subscription.html.markdown

+
+resource "azurerm_eventgrid_event_subscription" "default" {
+  name                = "defaultEventSubscription"
+  scope = "${azurerm_resource_group.default.id}"


Could we fix the formatting/alignment here?

katbyte · 2019-02-28T08:56:41Z

website/docs/r/eventgrid_event_subscription.html.markdown

+* `hybrid_connection_endpoint` - (Optional) A `hybrid_connection_endpoint` block as defined below.
+
+* `webhook_endpoint` - (Optional) A `webhook_endpoint` block as defined below.
+


Could we add a note letting readers know that one of the above blocks is required?

tombuildsstuff · 2019-02-28T12:06:32Z

azurerm/resource_arm_eventgrid_event_subscription.go

+						},
+						"case_sensitive": {
+							Type:     schema.TypeBool,
+							Optional: true,


by being Optional this implicitly has a default of false, which is may be worth highlighting in the docs

mbfrahry · 2019-02-28T22:25:15Z

Thanks for the review @katbyte and @tombuildsstuff. I believe I've addressed all the comments

website/docs/r/eventgrid_event_subscription.html.markdown

Co-Authored-By: mbfrahry <mbfrahry@gmail.com>

katbyte · 2019-02-28T22:34:32Z

Thanks for the updates! LGTM now 🚀

ghost · 2019-03-08T18:18:39Z

This has been released in version 1.23.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
	version = "~> 1.23.0"
}
# ... other configuration ...

ghost · 2019-03-31T13:50:25Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

mbfrahry added 6 commits February 12, 2019 22:23

Adding EventGrid Domain resource

595cca2

Start of event subscription

11d4487

Merging with master

419c0a7

Adding endpoints to event subscriptions

33f23db

Add additional attributes

5838f5c

Finalizing eventgrid event subscription

603ae22

ghost added documentation size/XXL labels Feb 28, 2019

mbfrahry requested a review from a team February 28, 2019 01:27

tombuildsstuff added enhancement service/event-grid EventGrid labels Feb 28, 2019

tombuildsstuff added this to the 1.23.0 milestone Feb 28, 2019

katbyte requested changes Feb 28, 2019

View reviewed changes

tombuildsstuff reviewed Feb 28, 2019

View reviewed changes

tombuildsstuff assigned mbfrahry Feb 28, 2019

tombuildsstuff added the waiting-response label Feb 28, 2019

tombuildsstuff changed the title ~~F eventgrid event subscription~~ New Resource: azurerm_eventgrid_subscription Feb 28, 2019

tombuildsstuff changed the title ~~New Resource: azurerm_eventgrid_subscription~~ New Resource: azurerm_eventgrid_event_subscription Feb 28, 2019

Addressing review comments

f75a6d0

katbyte reviewed Feb 28, 2019

View reviewed changes

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

katbyte self-assigned this Feb 28, 2019

Update website/docs/r/eventgrid_event_subscription.html.markdown

9d83080

Co-Authored-By: mbfrahry <mbfrahry@gmail.com>

katbyte approved these changes Feb 28, 2019

View reviewed changes

katbyte merged commit 2468103 into master Feb 28, 2019

katbyte deleted the f-eventgrid-event-subscription branch February 28, 2019 22:42

katbyte added a commit that referenced this pull request Feb 28, 2019

Update CHANGELOG.md to include #2967

17389c7

mbfrahry mentioned this pull request Feb 28, 2019

New Resource: EventGrid Subscriptions #1794

Closed

tombuildsstuff mentioned this pull request Mar 4, 2019

New resource: azurerm_api_management_property #2986

Merged

ghost removed the waiting-response label Mar 8, 2019

ghost locked and limited conversation to collaborators Mar 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_eventgrid_event_subscription` #2967

New Resource: `azurerm_eventgrid_event_subscription` #2967

mbfrahry commented Feb 28, 2019 •

edited

Loading

katbyte left a comment

katbyte Feb 28, 2019

katbyte Feb 28, 2019

katbyte Feb 28, 2019

katbyte Feb 28, 2019

katbyte Feb 28, 2019

katbyte Feb 28, 2019

katbyte Feb 28, 2019

katbyte Feb 28, 2019

katbyte Feb 28, 2019

katbyte Feb 28, 2019

tombuildsstuff Feb 28, 2019

mbfrahry Feb 28, 2019

mbfrahry commented Feb 28, 2019

katbyte commented Feb 28, 2019

ghost commented Mar 8, 2019

ghost commented Mar 31, 2019

		* `hybrid_connection_endpoint` - (Optional) A `hybrid_connection_endpoint` block as defined below.

		* `webhook_endpoint` - (Optional) A `webhook_endpoint` block as defined below.

New Resource: azurerm_eventgrid_event_subscription #2967

New Resource: azurerm_eventgrid_event_subscription #2967

Conversation

mbfrahry commented Feb 28, 2019 • edited Loading

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbfrahry commented Feb 28, 2019

katbyte commented Feb 28, 2019

ghost commented Mar 8, 2019

ghost commented Mar 31, 2019

New Resource: `azurerm_eventgrid_event_subscription` #2967

New Resource: `azurerm_eventgrid_event_subscription` #2967

mbfrahry commented Feb 28, 2019 •

edited

Loading