Add azurerm_eventhub_namespace_network_rule_set resource

[kvendingoldo]

No description provided.

[kvendingoldo]

Probably this issue (Azure/azure-cli#12903) is related ...

[mbfrahry]

Hey @kvendingoldo, thanks for opening this PR! Unfortunately, it doesn't do anything at the moment. You've declared it in the schema but we need to grab it in the CreateUpdate method and set it in the read method. We'll also want to update the documentation.

[mbfrahry]

This is risky and we need to confirm that this value is actually set to false when this resource is created. If it isn't false on creation, this will break every user using this resource because Terraform will try and update this value to false.

[kvendingoldo]

it's false by default

[mbfrahry]

Ahhh, did a bit more digging and it looks like it's not supported in the sdk at the moment. Is that right @kvendingoldo?

[kvendingoldo]

@mbfrahry it's true and I'm trying to clarify what should be do for getting support from azure sdk side. As we can see for now Terraform is using eventhub lib with version 2018-01-01-preview. Probably issue that I highlighted below will help us (Azure/azure-cli#12903)

[mbfrahry]

It looks like it's even missing from their api-specs. It's probably worth opening an issue on that repo and hopefully those maintainers can also push the service team to help.

[kvendingoldo]

Issue Azure/azure-rest-api-specs#11425 created, let's wait for an answer.

[kvendingoldo]

@mbfrahry looks like that it is supported via 2018-01-01-preview: trustedServiceAccessEnabled.
I've updated my change. Can you help me to find mapping between tf schema and az schema? Right now it isn't working.

[mbfrahry]

I'm not seeing trustedServiceAccessEnabled in the api-specs or the sdk. Did you see that somewhere?

[kvendingoldo]

@mbfrahry will introduce new change today a little bit later, looks like that I found new solution

[kvendingoldo]

@mbfrahry I've decided to implement new resource: azurerm_eventhub_namespace_network_rule_set that based on this spec: https://github.com/Azure/azure-rest-api-specs/blob/master/specification/eventhub/resource-manager/Microsoft.EventHub/preview/2018-01-01-preview/networkrulessets-preview.json

It's a draft.

[kvendingoldo]

1. Unit tests fail: Error: unable to parse EventHub Namespace Network Rule Set ID "/subscriptions/XXXXX/resourceGroups/acctestRG-sb-201028220042101558/providers/Microsoft.EventHub/namespaces/acctest-sb-namespace-201028220042101558/networkRuleSets/default": ID was missing the networkrulesets element
2. Option trusted_service_access_enabled is not working. Looks like that it caused by the following structure:

type NetworkRuleSetProperties struct {
	// DefaultAction - Default Action for Network Rule Set. Possible values include: 'Allow', 'Deny'
	DefaultAction DefaultAction `json:"defaultAction,omitempty"`
	// VirtualNetworkRules - List VirtualNetwork Rules
	VirtualNetworkRules *[]NWRuleSetVirtualNetworkRules `json:"virtualNetworkRules,omitempty"`
	// IPRules - List of IpRules
	IPRules *[]NWRuleSetIPRules `json:"ipRules,omitempty"`
}

@mbfrahry WDYT? Should I update sdk lib or what?

[mbfrahry]

1. It looks like you actually want networkRuleSets instead of networkrulesets in your parse function and tests. This is from here in the sdk.
2. We do already support network_rule_sets so I don't think writing out a separate resource will unblock us here.

At the moment we just have to wait because it's on the services team to provide the attribute we need since it's not in the sdk or the api-specs where the sdk is generated from.

[biggles007]

Not sure if this will help or not but using the following page we were able to script out a post terraform deployment fix. We just add the trustedServiceAccessEnabled property and set it to true.
https://docs.microsoft.com/en-us/rest/api/eventhub/2018-01-01-preview/network%20rule%20sets/createorupdatenetworkruleset

[kvendingoldo]

@bigglesuk69 yes, it's a good as ad-hoc solution.

[katbyte]

Hey @kvendingoldo - it looks like this is still blocked, as such i am going to close it. Let me know if i'm mistaken otherwise we can revisit this once the SDK has bene updated. Thanks again!

[ghost]

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 👉 [email protected]. Thanks!