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

Feature Request: azurerm_network_security_rule add support for , in source_port_range/destination_port_range #659

Closed
runecalico opened this issue Jan 3, 2018 · 6 comments · Fixed by #692

Comments

@runecalico
Copy link

runecalico commented Jan 3, 2018

Hi there,

Thank you for opening an issue. Please note that we try to keep the Terraform issue tracker reserved for bug reports and feature requests. For general usage questions, please see: https://www.terraform.io/community.html.

Terraform Version

Run terraform -v to show the version. If you are not running the latest version of Terraform, please upgrade because your issue may have already been fixed.
PS C:\OneDrive\git-repo\AzureCloud\supplychain-preprod-us-cmp-uat\compute> .\terraform.exe version
Terraform v0.11.1

Affected Resource(s)

Please list the resources as a list, for example:

  • azurerm_network_security_rule

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

# Copy-paste your Terraform configurations here - for large Terraform configs,
# please use a service like Dropbox and share a link to the ZIP file. For
# security, you can also encrypt the files using our GPG public key.

Debug Output

Please provider a link to a GitHub Gist containing the complete debug output: https://www.terraform.io/docs/internals/debugging.html. Please do NOT paste the debug output in the issue; just paste a link to the Gist.

Panic Output

If Terraform produced a panic, please provide a link to a GitHub Gist containing the output of the crash.log.

Expected Behavior

What should have happened?

source_port_range/destination_port_range support the use of , to separate multiple ports, for example 80,443,989-990 This behavior is supported by the Azure portal when creating an NSG rule as of 01/03/2018.

Actual Behavior

What actually happened?

Only single ports or port ranges like 4000-4004 are supported. The use of a comma throws an error.

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform apply

Important Factoids

Are there anything atypical about your accounts that we should know? For example: Running in EC2 Classic? Custom version of OpenStack? Tight ACLs?

References

Are there any other GitHub issues (open or closed) or Pull Requests that should be linked here? For example:

@tombuildsstuff tombuildsstuff changed the title Feature Request: azurerm_network_security_rule add support for , in source_port_range/destination_port_range Feature Request: azurerm_network_security_rule add support for , in source_port_range/destination_port_range Jan 5, 2018
@runecalico
Copy link
Author

To give an example in terraform config... It would be something like this:
resource "azurerm_network_security_rule" "snCmpUatApp_I_105" {
name = "snmp_161"
description = "All Inbound SNMP traffic on Port 161"
priority = 105
direction = "Inbound"
access = "Allow"
protocol = "Udp"
source_port_range = ""
destination_port_range = ["161","155","88-90"]
source_address_prefix = "
"
destination_address_prefix = "172.30.21.32/28"
resource_group_name = "${var.vnet_rg_name}"
network_security_group_name = "${azurerm_network_security_group.snCmpUatApp.name}"
}

I just used a list as an example as I would expect that might be how you would want to do it vs "161,155,88-90"

@rrudduck
Copy link
Contributor

rrudduck commented Jan 5, 2018

I hit this as well. Looking at the documentation they actually created separate properties for the augmented rules. I'll work on submitting a PR.

https://docs.microsoft.com/en-us/rest/api/virtualnetwork/networksecuritygroups/createorupdate

@tombuildsstuff
Copy link
Contributor

hey @runecalico

Thanks for opening this issue

Taking a look into this, unfortunately this appears to be a limitation within Azure:

* azurerm_network_security_rule.test: network.SecurityRulesClient#CreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="SecurityRuleInvalidPortRange" Message="Security rule has invalid Port range. Value provided: 80,81. Value should be an integer OR integer range with '-' delimiter. Valid range 0-65535." Details=[]

From what I can see - setting these values in the Azure Portal doesn't appear to do anything either - for instance:

screen shot 2018-01-08 at 12 28 15

Checking the respective API it appears setting this value on either the Destination Port Range / Source Port Range fields actually set the value to * under the hood:

screen shot 2018-01-08 at 12 29 03

.. which seems odd, perhaps that's a bug in the Azure Portal, or perhaps it's only possible to specify a single Port or Port Range per Rule?


Instead - it should be possible to achieve the same thing using dashes; for instance 80-80 - like so:

resource "azurerm_resource_group" "test" {
  name     = "examplerg"
  location = "West US"
}

resource "azurerm_network_security_group" "test" {
  name                = "acceptanceTestSecurityGroup1"
  location            = "${azurerm_resource_group.test.location}"
  resource_group_name = "${azurerm_resource_group.test.name}"
}

resource "azurerm_network_security_rule" "test" {
  name                        = "test123"
  priority                    = 100
  direction                   = "Outbound"
  access                      = "Allow"
  protocol                    = "Tcp"
  source_port_range           = "80-80"
  destination_port_range      = "*"
  source_address_prefix       = "*"
  destination_address_prefix  = "*"
  resource_group_name         = "${azurerm_resource_group.test.name}"
  network_security_group_name = "${azurerm_network_security_group.test.name}"
}

Given this appears to be a limitation of Azure (and not something we're able to fix in Terraform) - I'm going to close this issue for the moment. That said - should this become possible in the future we can certainly investigate adding support for this :)

Thanks!

@rrudduck
Copy link
Contributor

rrudduck commented Jan 8, 2018

@tombuildsstuff -

It's actually possible if you look at the rest api (and the go sdk). The property names for the augmented rules are different - you can't use the existing ones.

https://docs.microsoft.com/en-us/rest/api/virtualnetwork/networksecuritygroups/createorupdate

properties.destinationAddressPrefixes | string[] | The destination address prefixes. CIDR or destination IP ranges.

properties.destinationPortRanges | string[] | The destination port.

// SecurityRulePropertiesFormat is security rule resource.
type SecurityRulePropertiesFormat struct {
	Description                          *string                     `json:"description,omitempty"`
	Protocol                             SecurityRuleProtocol        `json:"protocol,omitempty"`
	SourcePortRange                      *string                     `json:"sourcePortRange,omitempty"`
	DestinationPortRange                 *string                     `json:"destinationPortRange,omitempty"`
	SourceAddressPrefix                  *string                     `json:"sourceAddressPrefix,omitempty"`
	SourceAddressPrefixes                *[]string                   `json:"sourceAddressPrefixes,omitempty"`
	SourceApplicationSecurityGroups      *[]ApplicationSecurityGroup `json:"sourceApplicationSecurityGroups,omitempty"`
	DestinationAddressPrefix             *string                     `json:"destinationAddressPrefix,omitempty"`
	DestinationAddressPrefixes           *[]string                   `json:"destinationAddressPrefixes,omitempty"`
	DestinationApplicationSecurityGroups *[]ApplicationSecurityGroup `json:"destinationApplicationSecurityGroups,omitempty"`
	SourcePortRanges                     *[]string                   `json:"sourcePortRanges,omitempty"`
	DestinationPortRanges                *[]string                   `json:"destinationPortRanges,omitempty"`
	Access                               SecurityRuleAccess          `json:"access,omitempty"`
	Priority                             *int32                      `json:"priority,omitempty"`
	Direction                            SecurityRuleDirection       `json:"direction,omitempty"`
	ProvisioningState                    *string                     `json:"provisioningState,omitempty"`
}

I hit this case as well and need to submit a PR but I haven't had time to prepare it. It's basically just adding the additional properties to the provider.

@sgautrin
Copy link

As @rrudduck said, it is indeed a pretty recent feature on Azure (still preview as far as I know), which lets you, with new properties, specify multiple addresses, ports ranges, application security groups (but not multiple service tags).

Thus it would be nice for this issue to be reopened so we can follow it (and so that @rrudduck 's future PR can be linked to it).

@ghost
Copy link

ghost commented Apr 1, 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 Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants