azurerm_kusto_eventhub_data_connection: support event_system_properties #11006

favoretti · 2021-03-17T13:59:02Z

$ make acctests SERVICE='kusto' TESTARGS='-run=TestAccKustoEventHubDataConnection_eventSystemProperties' TESTTIMEOUT='600m'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./azurerm/internal/services/kusto -run=TestAccKustoEventHubDataConnection_eventSystemProperties -timeout 600m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
2021/03/17 14:33:24 [DEBUG] not using binary driver name, it's no longer needed
2021/03/17 14:33:25 [DEBUG] not using binary driver name, it's no longer needed
=== RUN   TestAccKustoEventHubDataConnection_eventSystemProperties
=== PAUSE TestAccKustoEventHubDataConnection_eventSystemProperties
=== CONT  TestAccKustoEventHubDataConnection_eventSystemProperties
--- PASS: TestAccKustoEventHubDataConnection_eventSystemProperties (1460.05s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/kusto	1461.373s

Fixes #10993

katbyte

thanks @favoretti aside from a couple comments this is looking good

website/docs/r/kusto_eventhub_data_connection.html.markdown

katbyte · 2021-03-18T22:38:22Z

azurerm/internal/services/kusto/kusto_eventhub_data_connection_resource.go

@@ -83,6 +83,16 @@ func resourceKustoEventHubDataConnection() *schema.Resource {
 				ValidateFunc: eventhubValidate.EventHubID,
 			},

+			"event_system_properties": {


event seems very vague here? as it applies to the eventhub:

Suggested change

"event_system_properties": {

"eventhub_system_properties": {

It does apply to eventhub, but those are properties of a single event as far as I understand, not of eventhub on its own.
@derSchtefan since you originally requested this, can you chip in on semantics please? :)

These appear to be (System) Events - so the naming event_system_properties would make more sense than eventhub_system_properties here: https://docs.microsoft.com/en-us/azure/data-explorer/ingest-data-event-hub-overview#event-system-properties-mapping

then perhaps system_event_properties? 🤔 docs will need to be updated either way as it currently states: a list of system properties for the Event Hub.

@katbyte @favoretti EventHub and IoT Hub are based around Kafka, a similar open-source system. EventHub and IoT Hub both attach different SYSTEM properties to their events, that Kafka does not know about. One of them is x-opt-publisher which in conjunction with SAS token locked publisher endpoints allow you to identify who published an event (which is a nightmare to identify and make secure otherwise)

The property should be named event_system_properties because:

Microsoft names them like this not only in the portal but also the json schema

They are not specific to EventHub, but also include IoT Hub system properties

They are system properties of the event, as opposed to custom properties of the event

The list of potential event_system_properties can be found here:

For EventHub based ingestions: https://docs.microsoft.com/en-us/azure/data-explorer/ingest-data-event-hub-overview#event-system-properties-mapping

For IoT hub based ingestions: https://docs.microsoft.com/en-us/azure/data-explorer/ingest-data-iot-hub-overview#system-properties

IoT and EventHub both are mapped to EventHub ingestion internally, and work the same, since IoT hub is just an EventHub with some extra features. Other EventHub based systems might be added in the future.

See also @tombuildsstuff 's comment here: #11006 (comment)

Co-authored-by: kt <kt@katbyte.me>

tombuildsstuff

Left a couple of comments inline but this otherwise LGTM 👍

tombuildsstuff · 2021-03-19T11:46:30Z

azurerm/internal/services/kusto/kusto_eventhub_data_connection_resource.go

+			"event_system_properties": {
+				Type:     schema.TypeList,
+				Optional: true,
+				Computed: true,


does this need to be Computed?

Suggested change

Computed: true,

Yep, since some of them are set implicitly and generated a diff otherwise.

tombuildsstuff · 2021-03-19T11:48:13Z

azurerm/internal/services/kusto/kusto_eventhub_data_connection_resource.go

@@ -83,6 +83,16 @@ func resourceKustoEventHubDataConnection() *schema.Resource {
 				ValidateFunc: eventhubValidate.EventHubID,
 			},

+			"event_system_properties": {


These appear to be (System) Events - so the naming event_system_properties would make more sense than eventhub_system_properties here: https://docs.microsoft.com/en-us/azure/data-explorer/ingest-data-event-hub-overview#event-system-properties-mapping

ghost · 2021-03-26T06:37:54Z

This has been released in version 2.53.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 = "~> 2.53.0"
}
# ... other configuration ...

ghost · 2021-04-24T14:50:42Z

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!

azurerm_kusto_eventhub_data_connection: support event_system_properties

75d511c

Fixes #10993

ghost added size/S documentation labels Mar 17, 2021

katbyte requested changes Mar 18, 2021

View reviewed changes

katbyte added this to the v2.53.0 milestone Mar 18, 2021

katbyte added enhancement service/kusto labels Mar 18, 2021

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

e3d5b10

Co-authored-by: kt <kt@katbyte.me>

tombuildsstuff approved these changes Mar 19, 2021

View reviewed changes

tombuildsstuff assigned favoretti Mar 19, 2021

tombuildsstuff added the waiting-response label Mar 19, 2021

favoretti requested a review from katbyte March 19, 2021 15:38

favoretti removed the waiting-response label Mar 21, 2021

katbyte merged commit da0b1c2 into hashicorp:master Mar 25, 2021

katbyte added a commit that referenced this pull request Mar 25, 2021

CHANGELOG.md for #11006

6711e23

ghost locked as resolved and limited conversation to collaborators Apr 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

azurerm_kusto_eventhub_data_connection: support event_system_properties #11006

azurerm_kusto_eventhub_data_connection: support event_system_properties #11006

favoretti commented Mar 17, 2021

katbyte left a comment

katbyte Mar 18, 2021

favoretti Mar 19, 2021

tombuildsstuff Mar 19, 2021

katbyte Mar 19, 2021

derSchtefan Mar 19, 2021 •

edited

derSchtefan Mar 19, 2021

tombuildsstuff left a comment

tombuildsstuff Mar 19, 2021

favoretti Mar 19, 2021

tombuildsstuff Mar 19, 2021

ghost commented Mar 26, 2021

ghost commented Apr 24, 2021

azurerm_kusto_eventhub_data_connection: support event_system_properties #11006

azurerm_kusto_eventhub_data_connection: support event_system_properties #11006

Conversation

favoretti commented Mar 17, 2021

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

derSchtefan Mar 19, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff 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

ghost commented Mar 26, 2021

ghost commented Apr 24, 2021

derSchtefan Mar 19, 2021 •

edited