hashicorp / terraform-provider-azurerm

Terraform provider for Azure Resource Manager
https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs
Mozilla Public License 2.0
4.59k stars 4.62k forks source link

Add network security group attribute in azurerm_subnet resource #9161

Closed LaurentLesle closed 3 years ago

LaurentLesle commented 3 years ago

We have some enterprise scenario (Cloud Adoption Framework Enterprise Scale) that prevent the creation of a subnet if the NSG is not associated during the creation. This issue proposes to add the following implementation:

Add attribute : network_security_group_id


resource "azurerm_subnet" "example" {
  name                 = "testsubnet"
  resource_group_name  = azurerm_resource_group.example.name
  virtual_network_name = azurerm_virtual_network.example.name
  address_prefixes     = ["10.0.1.0/24"]

  network_security_group_id = azurerm_network_security_group.example.id

  delegation {
    name = "acctestdelegation"

    service_delegation {
      name    = "Microsoft.ContainerInstance/containerGroups"
      actions = ["Microsoft.Network/virtualNetworks/subnets/join/action", "Microsoft.Network/virtualNetworks/subnets/prepareNetworkPolicies/action"]
    }
  }
}
krowlandson commented 3 years ago

It would also be useful to add an attribute for azurerm_route_table association at the same time as creation of a subnet:

resource "azurerm_subnet" "example" {
  name                 = "testsubnet"
  resource_group_name  = azurerm_resource_group.example.name
  virtual_network_name = azurerm_virtual_network.example.name
  address_prefixes     = ["10.0.1.0/24"]

  network_security_group_id = azurerm_network_security_group.example.id
  route_table_id            = azurerm_route_table.example.id

  delegation {
    name = "acctestdelegation"

    service_delegation {
      name    = "Microsoft.ContainerInstance/containerGroups"
      actions = ["Microsoft.Network/virtualNetworks/subnets/join/action", "Microsoft.Network/virtualNetworks/subnets/prepareNetworkPolicies/action"]
    }
  }
}

This is another common policy requirement we work with.

tombuildsstuff commented 3 years ago

hi @LaurentLesle

Thanks for opening this issue.

The azurerm_subnet_network_security_group_association resource exists to workaround failures at destroy-time, where it's possible to create resources but not destroy them - due to the design of the Azure API (which returns an error along the lines of InUseSubnetCannotBeDeleted during deletion).

Whilst we could look to reintroduce the older network_security_group_id field within the azurerm_subnet resource, and support both this and the azurerm_subnet_network_security_group_association resource (or potentially as a separate resource) - unfortunately this'd just reintroduce this issue during destroy-time and so ultimately isn't as quick a fix as it may appear.

Instead issue #9022 looks to add an example of how to use Azure Policy with these "association" resources - which I'm going to close this issue in favour of, would you mind subscribing to that issue for updates?

Thanks!

krowlandson commented 3 years ago

@tombuildsstuff - I think this may still present an issue when working in an Enterprise environment using strict Azure Policy for enforcement of standards.

Unless these associations are created at subnet creation as a single API call, deny actions in the Azure Policy will prevent creation of the subnet, so Terraform will never get to processing the associations.

Looking at the Azure SDK for Go, I can see this may also be something which needs updating to support this in Terraform so I'm reaching out internally to the developer responsible for this.

tombuildsstuff commented 3 years ago

@krowlandson as mentioned above unfortunately we have no plans to reintroduce this field - since this causes issues during deletion for users.

Since this is an Azure Policy issue, #9022 will introduce an example of how to achieve this in Azure Policy (taking effect only when something is created within the Subnet, which has the same effect) - but unfortunately we can't reintroduce this field without re-introducing these issues (due to a design bug in the Azure API).

ghost commented 3 years ago

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!