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.

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

Initial commit for storage_account_network_rules_association #3741

Closed
wants to merge 4 commits into from
Closed

Initial commit for storage_account_network_rules_association #3741

wants to merge 4 commits into from

Conversation

MattMencel
Copy link
Contributor

This is an initial PR for #3522. Just the test file for azurerm_storage_account_network_rule_association. Looking for feedback and direction for this before I move forward. If I'm totally going in the wrong direction let me know.

Since network rules can be applied to Keyvaults and Storage Accounts.... I'm wondering if the network rule resource should be azurerm_network_rules? That could be too similar and be confused with azurerm_network_security_rule used with NSGs.

resource "azurerm_network_rules" testnr" {
  name                     = "acctestnr%d"
  resource_group_name      = "${azurerm_resource_group.testrg.name}"
  location                 = "${azurerm_resource_group.testrg.location}"
	
  network_rules {
    default_action             = "Deny"
    ip_rules                   = ["127.0.0.1"]
    virtual_network_subnet_ids = ["${azurerm_subnet.test.id}"]
  }
}

resource "azurerm_storage_account_network_rules_association" "testsanra" {
  storage_account_id        = "${azurerm_storage_account_testsa.id}"
  network_rules_id          = "${azurerm_network_rules.testnr.id}"
}

Maybe azurerm_network_acls then instead?

resource "azurerm_network_acls" testna" {
  name                     = "acctestnr%d"
  resource_group_name      = "${azurerm_resource_group.testrg.name}"
  location                 = "${azurerm_resource_group.testrg.location}"
	
  network_rules {
    default_action             = "Deny"
    ip_rules                   = ["127.0.0.1"]
    virtual_network_subnet_ids = ["${azurerm_subnet.test.id}"]
  }
}

resource "azurerm_storage_account_network_acls_association" "testsanaa" {
  storage_account_id = "${azurerm_storage_account_testsa.id}"
  network_acls_id = "${azurerm_network_rules.testna.id}"
}

@ghost ghost added the size/L label Jun 27, 2019
@magodo
Copy link
Collaborator

magodo commented Oct 22, 2019

I also looked into this #3522 recently. I came up same idea as your solution but encountered some issue.

I found all association resources in azure binds two concrete resources. However, there doesn't exist azurerm_network_rule (or anything alike) resource in azure. So I wonder service team should first provide such a resource, then you can proceed to implement both azurerm_network_rule and azurerm_storage_account_network_rules_association resources in terrafrom.

IMHO, the association resources apply to the case that there already exist two resources in azure, as time goes by, some use case shows high correlation between those two resources, so some one create an associate resource to associate those two concrete resources by id, so as to tell terraform there dependencies.

On the other hand, I have some further info to share with you. I find some existing resources(i will refer to them as attaching resources), e.g. azurerm_servicebus_queue_authorization_rule, play as part of some concrete resource (e.g. azurerm_servicebus_queue). The implementation to those attaching resources are based on the concrete resources' backend client in azure Go SDK.

One precondition in this case is that these backend services have native support for those attaching resources in API spec (i.e. they have dedicated API path). However, azurerm_storage_account has no such support for network rules... 😥

So I wonder how are you going to preceed?

@mbfrahry
Copy link
Member

mbfrahry commented Dec 6, 2019

Hey @MattMencel. Thanks for taking the time to write out this test file and think about how to add this resource into the provider. Apologies for not taking a look at it sooner but we just added #5082 which adds azurerm_storage_account_network_rules to the provider. As of now we're just going to mirror the functionality of the current network rules inside azurerm_storage_account into its own resource. That should unlock the circular dependency chain that was occurring. I'm going to close this down but let me know if that PR isn't going to do what we were looking to solve.

@mbfrahry mbfrahry closed this Dec 6, 2019
@ghost
Copy link

ghost commented Jan 6, 2020

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!

@ghost ghost locked and limited conversation to collaborators Jan 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants