From 5cfef99ae811a7dd6bdb3b1c2f8c3a8cf978585a Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 14 Sep 2020 10:13:01 -0400 Subject: [PATCH 01/25] Working NSG module and sample test --- .../terraform-azure-nsg-example/README.md | 129 ++++++++++++ examples/terraform-azure-nsg-example/main.tf | 118 +++++++++++ .../terraform-azure-nsg-example/outputs.tf | 11 ++ .../terraform-azure-nsg-example/variables.tf | 44 +++++ modules/azure/nsg.go | 187 ++++++++++++++++++ .../azure/terraform_azure_nsg_example_test.go | 46 +++++ 6 files changed, 535 insertions(+) create mode 100644 examples/terraform-azure-nsg-example/README.md create mode 100644 examples/terraform-azure-nsg-example/main.tf create mode 100644 examples/terraform-azure-nsg-example/outputs.tf create mode 100644 examples/terraform-azure-nsg-example/variables.tf create mode 100644 modules/azure/nsg.go create mode 100644 test/azure/terraform_azure_nsg_example_test.go diff --git a/examples/terraform-azure-nsg-example/README.md b/examples/terraform-azure-nsg-example/README.md new file mode 100644 index 000000000..502a16fba --- /dev/null +++ b/examples/terraform-azure-nsg-example/README.md @@ -0,0 +1,129 @@ +# Terraform Azure @Module@ Example + +This folder contains a simple Terraform module that deploys resources in [Azure](https://azure.microsoft.com/) to demonstrate how you can use Terratest to write automated tests for your Azure Terraform code. This module deploys the following: + +* A [Virtual Machine](https://azure.microsoft.com/en-us/services/virtual-machines/) that gives the module the following: + * [Virtual Machine](https://docs.microsoft.com/en-us/azure/virtual-machines/) with the value specified in the `vm_name` variable. + * A [Network Security Group](https://docs.microsoft.com/en-us/azure/virtual-network/network-security-groups-overview) created with a single custom rule to allow SSH (port 22) with the nsg name specified in the `nsg_name` variable. + +Check out [test/azure/terraform_azure_nsg_example_test.go](/test/azure/terraform_azure_nsg_example_test.go) to see how you can write +automated tests for this module. + +Note that the resources deployed in this module don't actually do anything; it just runs the resources for +demonstration purposes. + +**WARNING**: This module and the automated tests for it deploy real resources into your Azure account which can cost you +money. The resources are all part of the [Azure Free Account](https://azure.microsoft.com/en-us/free/), so if you haven't used that up, +it should be free, but you are completely responsible for all Azure charges. + +## Running this module manually + +1. Sign up for [Azure](https://azure.microsoft.com/). +1. Configure your Azure credentials using one of the [supported methods for Azure CLI + tools](https://docs.microsoft.com/en-us/cli/azure/azure-cli-configuration?view=azure-cli-latest). +1. Install [Terraform](https://www.terraform.io/) and make sure it's on your `PATH`. +1. Run `terraform init`. +1. Run `terraform apply`. +1. When you're done, run `terraform destroy`. + +## Running automated tests against this module + +1. Sign up for [Azure](https://azure.microsoft.com/). +1. Configure your Azure credentials using one of the [supported methods for Azure CLI + tools](https://docs.microsoft.com/en-us/cli/azure/azure-cli-configuration?view=azure-cli-latest). +1. Install [Terraform](https://www.terraform.io/) and make sure it's on your `PATH`. +1. [Review environment variables](#review-environment-variables). +1. Install [Golang](https://golang.org/) and make sure this code is checked out into your `GOPATH`. +1. `cd test` +1. Make sure [the azure-sdk-for-go versions match](#check-go-dependencies) in [/test/go.mod](/test/go.mod) and in [test/azure/terraform_azure_@module@_test.go](/test/terraform_azure_nic_test.go). +1. `go build terraform_azure_@module@_test.go` +1. `go test -v -run TestTerraformAzure@Module@Example` + +## Module test APIs + +This module exposes the following top-level API methods: + +- GetDefaultNsgRulesClientE returns a rules client that retuns the _default_ NSG rules
+`func GetDefaultNsgRulesClientE(subscriptionID string) (network.DefaultSecurityRulesClient, error)` +- GetCustomNsgRulesClientE returns a rules client that returns the _custom_ NSG rules
+`func GetCustomNsgRulesClientE(subscriptionID string) (network.SecurityRulesClient, error)` +- GetAllNSGRulesE returns a slice containing all rules from the NSG (including defauls and custom)
+`func GetAllNSGRulesE(resourceGroupName, nsgName, subscriptionID string) (NsgRuleSummaryList, error)` + +Note that the `GetAllNsgRulesE` method returns an `NsgRuleSummaryList`, which is defined as follows: + +- NsgRuleSummaryList holds a colleciton of NsgRuleSummary structs
+` type NsgRuleSummaryList struct { SummarizedRules []NsgRuleSummary } ` + +This collection wrapper has the following method: + +- FindRuleByName looks for a matching rule name
+`func (summarizedRules *NsgRuleSummaryList) FindRuleByName(t *testing.T, name string) NsgRuleSummary` + +Finally, the inner collection is defined as an `NsgRuleSummary` struct, defined as follows: + +``` +type NsgRuleSummary struct { + Name string + Description string + Protocol string + SourcePortRange string + DestinationPortRange string + SourceAddressPrefix string + DestinationAddressPrefix string + Access string + Priority int32 + Direction string +} +``` + +This wrapper has the following methods available: + +- AllowsDestinationPort checks to see if the rule allows a specific destination port
+`func (summarizedRule *NsgRuleSummary) AllowsDestinationPort(port string) bool` +- AllowsSourcePort checks to see if the rule allows a specific source port
+`func (summarizedRule *NsgRuleSummary) AllowsSourcePort(port string) bool` + +## Check Go Dependencies + +Check that the `github.com/Azure/azure-sdk-for-go` version in your generated `go.mod` for this test matches the version in the terratest [go.mod](https://github.com/gruntwork-io/terratest/blob/master/go.mod) file. + +> This was tested with **go1.14.4**. + +### Check Azure-sdk-for-go version + +Let's make sure [go.mod](https://github.com/gruntwork-io/terratest/blob/master/go.mod) includes the appropriate [azure-sdk-for-go version](https://github.com/Azure/azure-sdk-for-go/releases/tag/v38.1.0): + +```go +require ( + ... + github.com/Azure/azure-sdk-for-go v38.1.0+incompatible + ... +) +``` + +If we make changes to either the **go.mod** or the **go test file**, we should make sure that the go build command works still. + +```powershell +go build terraform_azure_@module_test.go +``` + +## Review Environment Variables + +As part of configuring terraform for Azure, we'll want to check that we have set the appropriate [credentials](https://docs.microsoft.com/en-us/azure/terraform/terraform-install-configure?toc=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fazure%2Fterraform%2Ftoc.json&bc=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fazure%2Fbread%2Ftoc.json#set-up-terraform-access-to-azure) and also that we set the [environment variables](https://docs.microsoft.com/en-us/azure/terraform/terraform-install-configure?toc=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fazure%2Fterraform%2Ftoc.json&bc=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fazure%2Fbread%2Ftoc.json#configure-terraform-environment-variables) on the testing host. + +```bash +export ARM_CLIENT_ID=your_app_id +export ARM_CLIENT_SECRET=your_password +export ARM_SUBSCRIPTION_ID=your_subscription_id +export ARM_TENANT_ID=your_tenant_id +``` + +Note, in a Windows environment, these should be set as **system environment variables**. We can use a PowerShell console with administrative rights to update these environment variables: + +```powershell +[System.Environment]::SetEnvironmentVariable("ARM_CLIENT_ID",$your_app_id,[System.EnvironmentVariableTarget]::Machine) +[System.Environment]::SetEnvironmentVariable("ARM_CLIENT_SECRET",$your_password,[System.EnvironmentVariableTarget]::Machine) +[System.Environment]::SetEnvironmentVariable("ARM_SUBSCRIPTION_ID",$your_subscription_id,[System.EnvironmentVariableTarget]::Machine) +[System.Environment]::SetEnvironmentVariable("ARM_TENANT_ID",$your_tenant_id,[System.EnvironmentVariableTarget]::Machine) +``` diff --git a/examples/terraform-azure-nsg-example/main.tf b/examples/terraform-azure-nsg-example/main.tf new file mode 100644 index 000000000..89d2bd68f --- /dev/null +++ b/examples/terraform-azure-nsg-example/main.tf @@ -0,0 +1,118 @@ +provider "azurerm" { + version = "=2.20.0" + features {} +} + +# --------------------------------------------------------------------------------------------------------------------- +# PIN TERRAFORM VERSION TO >= 0.12 +# The examples have been upgraded to 0.12 syntax +# --------------------------------------------------------------------------------------------------------------------- + +terraform { + required_version = ">= 0.12" +} + +# --------------------------------------------------------------------------------------------------------------------- +# DEPLOY A RESOURCE GROUP +# See test/terraform_azure_example_test.go for how to write automated tests for this code. +# --------------------------------------------------------------------------------------------------------------------- + +resource "azurerm_resource_group" "main" { + name = "${var.prefix}-resources" + location = "East US" +} + +# --------------------------------------------------------------------------------------------------------------------- +# DEPLOY VIRTUAL NETWORK RESOURCES +# --------------------------------------------------------------------------------------------------------------------- + +resource "azurerm_virtual_network" "main" { + name = "${var.prefix}-network" + address_space = ["10.0.0.0/16"] + location = azurerm_resource_group.main.location + resource_group_name = azurerm_resource_group.main.name +} + +resource "azurerm_subnet" "internal" { + name = "internal" + resource_group_name = azurerm_resource_group.main.name + virtual_network_name = azurerm_virtual_network.main.name + address_prefixes = ["10.0.17.0/24"] +} + +resource "azurerm_network_interface" "main" { + name = "${var.prefix}-nic" + location = azurerm_resource_group.main.location + resource_group_name = azurerm_resource_group.main.name + + ip_configuration { + name = "terratestconfiguration1" + subnet_id = azurerm_subnet.internal.id + private_ip_address_allocation = "Dynamic" + } +} + +resource "azurerm_network_security_group" "main" { + name = "testSecurityGroup1" + location = azurerm_resource_group.main.location + resource_group_name = azurerm_resource_group.main.name +} + +resource "azurerm_network_interface_security_group_association" "main" { + network_interface_id = azurerm_network_interface.main.id + network_security_group_id = azurerm_network_security_group.main.id +} + +resource "azurerm_network_security_rule" "main" { + name = "allowSSH" + description = "allowSSH" + priority = 100 + direction = "Inbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_range = 22 + source_address_prefix = "*" + destination_address_prefix = "*" + resource_group_name = azurerm_resource_group.main.name + network_security_group_name = azurerm_network_security_group.main.name +} + +# --------------------------------------------------------------------------------------------------------------------- +# DEPLOY A VIRTUAL MACHINE RUNNING UBUNTU +# --------------------------------------------------------------------------------------------------------------------- + +resource "azurerm_virtual_machine" "main" { + name = "${var.prefix}-vm" + location = azurerm_resource_group.main.location + resource_group_name = azurerm_resource_group.main.name + network_interface_ids = [azurerm_network_interface.main.id] + vm_size = "Standard_B1s" + delete_os_disk_on_termination = true + delete_data_disks_on_termination = true + + storage_image_reference { + publisher = "Canonical" + offer = "UbuntuServer" + sku = "16.04-LTS" + version = "latest" + } + + storage_os_disk { + name = "terratestosdisk1" + caching = "ReadWrite" + create_option = "FromImage" + managed_disk_type = "Standard_LRS" + } + + os_profile { + computer_name = var.hostname + admin_username = var.username + admin_password = var.password + } + + os_profile_linux_config { + disable_password_authentication = false + } +} + diff --git a/examples/terraform-azure-nsg-example/outputs.tf b/examples/terraform-azure-nsg-example/outputs.tf new file mode 100644 index 000000000..ae8d7bcd0 --- /dev/null +++ b/examples/terraform-azure-nsg-example/outputs.tf @@ -0,0 +1,11 @@ +output "vm_name" { + value = azurerm_virtual_machine.main.name +} + +output "resource_group_name" { + value = azurerm_resource_group.main.name +} + +output "nsg_name" { + value = azurerm_network_security_group.main.name +} diff --git a/examples/terraform-azure-nsg-example/variables.tf b/examples/terraform-azure-nsg-example/variables.tf new file mode 100644 index 000000000..8c231c502 --- /dev/null +++ b/examples/terraform-azure-nsg-example/variables.tf @@ -0,0 +1,44 @@ +# --------------------------------------------------------------------------------------------------------------------- +# ENVIRONMENT VARIABLES +# Define these secrets as environment variables +# --------------------------------------------------------------------------------------------------------------------- + +# ARM_CLIENT_ID +# ARM_CLIENT_SECRET +# ARM_SUBSCRIPTION_ID +# ARM_TENANT_ID + +# --------------------------------------------------------------------------------------------------------------------- +# REQUIRED PARAMETERS +# You must provide a value for each of these parameters. +# --------------------------------------------------------------------------------------------------------------------- + +# --------------------------------------------------------------------------------------------------------------------- +# OPTIONAL PARAMETERS +# These parameters have reasonable defaults. +# --------------------------------------------------------------------------------------------------------------------- + +variable "hostname" { + description = "The hostname of the new VM to be configured" + type = string + default = "terratest-vm" +} + +variable "password" { + description = "The password to configure for SSH access" + type = string + default = "HorriblePassword1234!" +} + +variable "prefix" { + description = "The prefix that will be attached to all resources deployed" + type = string + default = "terratest-nsg-example" +} + +variable "username" { + description = "The username to be provisioned into your VM" + type = string + default = "testadmin" +} + diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go new file mode 100644 index 000000000..6e201595c --- /dev/null +++ b/modules/azure/nsg.go @@ -0,0 +1,187 @@ +package azure + +import ( + "context" + "strconv" + "strings" + "testing" + + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-09-01/network" +) + +// GetDefaultNsgRulesClientE returns a rules client +func GetDefaultNsgRulesClientE(subscriptionID string) (network.DefaultSecurityRulesClient, error) { + // Validate Azure subscription ID + subscriptionID, err := getTargetAzureSubscription(subscriptionID) + if err != nil { + return network.DefaultSecurityRulesClient{}, err + } + + nsgClient := network.NewDefaultSecurityRulesClient(subscriptionID) + + // Get an authorizer + auth, err := NewAuthorizer() + if err != nil { + return network.DefaultSecurityRulesClient{}, err + } + + nsgClient.Authorizer = *auth + return nsgClient, nil +} + +// GetCustomNsgRulesClientE returns a rules client +func GetCustomNsgRulesClientE(subscriptionID string) (network.SecurityRulesClient, error) { + // Validate Azure subscription ID + subscriptionID, err := getTargetAzureSubscription(subscriptionID) + if err != nil { + return network.SecurityRulesClient{}, err + } + + nsgClient := network.NewSecurityRulesClient(subscriptionID) + + // Get an authorizer + auth, err := NewAuthorizer() + if err != nil { + return network.SecurityRulesClient{}, err + } + + nsgClient.Authorizer = *auth + return nsgClient, nil +} + +// GetAllNSGRulesE returns a slice containing all rules from the NSG (including defauls and custom) +func GetAllNSGRulesE(resourceGroupName, nsgName, subscriptionID string) (NsgRuleSummaryList, error) { + defaultRulesClient, err := GetDefaultNsgRulesClientE(subscriptionID) + if err != nil { + return NsgRuleSummaryList{}, err + } + + customRulesClient, err := GetCustomNsgRulesClientE(subscriptionID) + if err != nil { + return NsgRuleSummaryList{}, err + } + + defaultRuleList, err := defaultRulesClient.ListComplete(context.Background(), resourceGroupName, nsgName) + if err != nil { + return NsgRuleSummaryList{}, err + } + + customRuleList, err := customRulesClient.ListComplete(context.Background(), resourceGroupName, nsgName) + if err != nil { + return NsgRuleSummaryList{}, err + } + + rules := make([]NsgRuleSummary, 0) + bindRuleList(&rules, defaultRuleList) + bindRuleList(&rules, customRuleList) + ruleList := NsgRuleSummaryList{} + ruleList.SummarizedRules = rules + return ruleList, nil +} + +// NsgRuleSummaryList holds a colleciton of NsgRuleSummary structs +type NsgRuleSummaryList struct { + SummarizedRules []NsgRuleSummary +} + +// NsgRuleSummary is a string-based summary of an NSG rule with methods attached. +type NsgRuleSummary struct { + Name string + Description string + Protocol string + SourcePortRange string + DestinationPortRange string + SourceAddressPrefix string + DestinationAddressPrefix string + Access string + Priority int32 + Direction string +} + +func bindRuleList(dest *[]NsgRuleSummary, source network.SecurityRuleListResultIterator) { + for source.NotDone() { + v := source.Value() + *dest = append(*dest, convertToNsgRuleSummary(v.Name, v.SecurityRulePropertiesFormat)) + source.NextWithContext(context.Background()) + } +} + +func convertToNsgRuleSummary(name *string, rule *network.SecurityRulePropertiesFormat) NsgRuleSummary { + summary := NsgRuleSummary{} + + if rule.Description != nil { + summary.Description = *rule.Description + } else { + summary.Description = "" + } + summary.Name = *name + summary.Protocol = string(rule.Protocol) + summary.SourcePortRange = *rule.SourcePortRange + summary.DestinationPortRange = *rule.DestinationPortRange + summary.SourceAddressPrefix = *rule.SourceAddressPrefix + summary.DestinationAddressPrefix = *rule.DestinationAddressPrefix + summary.Access = string(rule.Access) + summary.Priority = *rule.Priority + summary.Direction = string(rule.Direction) + return summary +} + +// FindRuleByName looks for a matching rule name +func (summarizedRules *NsgRuleSummaryList) FindRuleByName(t *testing.T, name string) NsgRuleSummary { + for _, r := range summarizedRules.SummarizedRules { + if r.Name == name { + return r + } + } + + t.Error("Supplied rule name not found") + return NsgRuleSummary{} +} + +// AllowsDestinationPort checks to see if the rule allows a specific destination port. +func (summarizedRule *NsgRuleSummary) AllowsDestinationPort(port string) bool { + if summarizedRule.DestinationPortRange == "*" { + return true + } + + low, high := parsePortRangeString(summarizedRule.DestinationPortRange) + portAsInt, _ := strconv.ParseInt(port, 10, 16) + + if (port == "*") && (low == 0) && (high == 65535) { + return true + } + + return ((low <= uint16(portAsInt)) && (uint16(portAsInt) <= high)) +} + +// AllowsSourcePort checks to see if the rule allows a specific source port. +func (summarizedRule *NsgRuleSummary) AllowsSourcePort(port string) bool { + if summarizedRule.SourcePortRange == "*" { + return true + } + + low, high := parsePortRangeString(summarizedRule.SourcePortRange) + portAsInt, _ := strconv.ParseInt(port, 10, 16) + + if (port == "*") && (low == 0) && (high == 65535) { + return true + } + + return ((low <= uint16(portAsInt)) && (uint16(portAsInt) <= high)) +} + +// parsePortRangeString decodes a range string ("2-100") or a single digit ("22") and returns +// a tuple in [low, hi] form. Note that if a single digit is supplied, both members of the +// return tuple will be the same value (e.g., "22" returns (22, 22)) +func parsePortRangeString(rangeString string) (low uint16, hight uint16) { + // Is this a range? + if strings.Index(rangeString, "-") == -1 { + val, _ := strconv.ParseInt(rangeString, 10, 16) + return uint16(val), uint16(val) + } + + parts := strings.Split(rangeString, "-") + lowVal, _ := strconv.ParseInt(parts[0], 10, 16) + highVal, _ := strconv.ParseInt(parts[1], 10, 16) + return uint16(lowVal), uint16(highVal) +} diff --git a/test/azure/terraform_azure_nsg_example_test.go b/test/azure/terraform_azure_nsg_example_test.go new file mode 100644 index 000000000..a1eedc564 --- /dev/null +++ b/test/azure/terraform_azure_nsg_example_test.go @@ -0,0 +1,46 @@ +// +build azure + +// NOTE: We use build tags to differentiate azure testing because we currently do not have azure access setup for +// CircleCI. + +package test + +import ( + "testing" + + "github.com/gruntwork-io/terratest/modules/azure" + "github.com/gruntwork-io/terratest/modules/terraform" + "github.com/stretchr/testify/assert" +) + +func TestTerraformAzureNsgExample(t *testing.T) { + t.Parallel() + + terraformOptions := &terraform.Options{ + // The path to where our Terraform code is located + TerraformDir: "../../examples/terraform-azure-nsg-example", + } + + defer terraform.Destroy(t, terraformOptions) + terraform.InitAndApply(t, terraformOptions) + + nsgName := terraform.Output(t, terraformOptions, "nsg_name") + resourceGroupName := terraform.Output(t, terraformOptions, "resource_group_name") + + // A default NSG has 6 rules, and we have one custom rule for a total of 7 + rules, err := azure.GetAllNSGRulesE(resourceGroupName, nsgName, "") + assert.NoError(t, err) + assert.Equal(t, 7, len(rules.SummarizedRules)) + + // We should have a rule named "allowSSH" + sshRule := rules.FindRuleByName(t, "allowSSH") + + // That rule should allow port 22 inbound + assert.True(t, sshRule.AllowsDestinationPort("22")) + + // But should not allow 80 inbound + assert.False(t, sshRule.AllowsDestinationPort("80")) + + // SSh is allowed from any port + assert.True(t, sshRule.AllowsSourcePort("*")) +} From 82577a2f6a1ebbd9d16d15cf3837819940fc8900 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 14 Sep 2020 10:33:48 -0400 Subject: [PATCH 02/25] Moved example folder to correct location --- examples/{ => azure}/terraform-azure-nsg-example/README.md | 0 examples/{ => azure}/terraform-azure-nsg-example/main.tf | 0 examples/{ => azure}/terraform-azure-nsg-example/outputs.tf | 0 examples/{ => azure}/terraform-azure-nsg-example/variables.tf | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename examples/{ => azure}/terraform-azure-nsg-example/README.md (100%) rename examples/{ => azure}/terraform-azure-nsg-example/main.tf (100%) rename examples/{ => azure}/terraform-azure-nsg-example/outputs.tf (100%) rename examples/{ => azure}/terraform-azure-nsg-example/variables.tf (100%) diff --git a/examples/terraform-azure-nsg-example/README.md b/examples/azure/terraform-azure-nsg-example/README.md similarity index 100% rename from examples/terraform-azure-nsg-example/README.md rename to examples/azure/terraform-azure-nsg-example/README.md diff --git a/examples/terraform-azure-nsg-example/main.tf b/examples/azure/terraform-azure-nsg-example/main.tf similarity index 100% rename from examples/terraform-azure-nsg-example/main.tf rename to examples/azure/terraform-azure-nsg-example/main.tf diff --git a/examples/terraform-azure-nsg-example/outputs.tf b/examples/azure/terraform-azure-nsg-example/outputs.tf similarity index 100% rename from examples/terraform-azure-nsg-example/outputs.tf rename to examples/azure/terraform-azure-nsg-example/outputs.tf diff --git a/examples/terraform-azure-nsg-example/variables.tf b/examples/azure/terraform-azure-nsg-example/variables.tf similarity index 100% rename from examples/terraform-azure-nsg-example/variables.tf rename to examples/azure/terraform-azure-nsg-example/variables.tf From 836686c01fc26b35c54dc4b316e62ea7bde3afe3 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 14 Sep 2020 15:21:14 -0400 Subject: [PATCH 03/25] Added unit tests for nsg module --- modules/azure/nsg.go | 18 +++++---- test/azure/nsg_test.go | 91 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 test/azure/nsg_test.go diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go index 6e201595c..32bad124d 100644 --- a/modules/azure/nsg.go +++ b/modules/azure/nsg.go @@ -4,7 +4,6 @@ import ( "context" "strconv" "strings" - "testing" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-09-01/network" ) @@ -114,6 +113,7 @@ func convertToNsgRuleSummary(name *string, rule *network.SecurityRulePropertiesF } else { summary.Description = "" } + summary.Name = *name summary.Protocol = string(rule.Protocol) summary.SourcePortRange = *rule.SourcePortRange @@ -127,14 +127,13 @@ func convertToNsgRuleSummary(name *string, rule *network.SecurityRulePropertiesF } // FindRuleByName looks for a matching rule name -func (summarizedRules *NsgRuleSummaryList) FindRuleByName(t *testing.T, name string) NsgRuleSummary { +func (summarizedRules *NsgRuleSummaryList) FindRuleByName(name string) NsgRuleSummary { for _, r := range summarizedRules.SummarizedRules { if r.Name == name { return r } } - t.Error("Supplied rule name not found") return NsgRuleSummary{} } @@ -144,7 +143,7 @@ func (summarizedRule *NsgRuleSummary) AllowsDestinationPort(port string) bool { return true } - low, high := parsePortRangeString(summarizedRule.DestinationPortRange) + low, high := ParsePortRangeString(summarizedRule.DestinationPortRange) portAsInt, _ := strconv.ParseInt(port, 10, 16) if (port == "*") && (low == 0) && (high == 65535) { @@ -160,7 +159,7 @@ func (summarizedRule *NsgRuleSummary) AllowsSourcePort(port string) bool { return true } - low, high := parsePortRangeString(summarizedRule.SourcePortRange) + low, high := ParsePortRangeString(summarizedRule.SourcePortRange) portAsInt, _ := strconv.ParseInt(port, 10, 16) if (port == "*") && (low == 0) && (high == 65535) { @@ -170,10 +169,15 @@ func (summarizedRule *NsgRuleSummary) AllowsSourcePort(port string) bool { return ((low <= uint16(portAsInt)) && (uint16(portAsInt) <= high)) } -// parsePortRangeString decodes a range string ("2-100") or a single digit ("22") and returns +// ParsePortRangeString decodes a range string ("2-100") or a single digit ("22") and returns // a tuple in [low, hi] form. Note that if a single digit is supplied, both members of the // return tuple will be the same value (e.g., "22" returns (22, 22)) -func parsePortRangeString(rangeString string) (low uint16, hight uint16) { +func ParsePortRangeString(rangeString string) (low uint16, hight uint16) { + // Is this an asterisk? + if rangeString == "*" { + return uint16(0), uint16(65535) + } + // Is this a range? if strings.Index(rangeString, "-") == -1 { val, _ := strconv.ParseInt(rangeString, 10, 16) diff --git a/test/azure/nsg_test.go b/test/azure/nsg_test.go new file mode 100644 index 000000000..173a9fb7e --- /dev/null +++ b/test/azure/nsg_test.go @@ -0,0 +1,91 @@ +// +build azure + +// NOTE: We use build tags to differentiate azure testing because we currently do not have azure access setup for +// CircleCI. + +package test + +import ( + "fmt" + "math/rand" + "testing" + "time" + + "github.com/gruntwork-io/terratest/modules/azure" + "github.com/stretchr/testify/assert" +) + +func TestRangeParsingSinglePort(t *testing.T) { + lo, hi := azure.ParsePortRangeString("22") + assert.Equal(t, uint16(22), lo) + assert.Equal(t, uint16(22), hi) +} + +func TestRangeParsingPortRange(t *testing.T) { + lo, hi := azure.ParsePortRangeString("22-80") + assert.Equal(t, uint16(22), lo) + assert.Equal(t, uint16(80), hi) +} + +func TestRangeParsingAsterisk(t *testing.T) { + lo, hi := azure.ParsePortRangeString("*") + assert.Equal(t, uint16(0), lo) + assert.Equal(t, uint16(65535), hi) +} + +func TestRuleSummaryAllowSourcePort(t *testing.T) { + summary := azure.NsgRuleSummary{} + summary.SourcePortRange = "22" + + result := summary.AllowsSourcePort("22") + assert.True(t, result) +} + +func TestRuleSummaryAllowSourcePortAsterisk(t *testing.T) { + summary := azure.NsgRuleSummary{} + summary.SourcePortRange = "*" + + rand := rand.New(rand.NewSource(time.Now().UnixNano())) + + result := summary.AllowsSourcePort(string(uint16(rand.Int()))) + assert.True(t, result) +} + +func TestRuleSummaryAllowDestinationPort(t *testing.T) { + summary := azure.NsgRuleSummary{} + summary.DestinationPortRange = "80" + + result := summary.AllowsDestinationPort("80") + assert.True(t, result) +} + +func TestRuleSummaryAllowDestinationPortAsterisk(t *testing.T) { + summary := azure.NsgRuleSummary{} + summary.DestinationPortRange = "*" + + rand := rand.New(rand.NewSource(time.Now().UnixNano())) + + result := summary.AllowsDestinationPort(string(uint16(rand.Int()))) + assert.True(t, result) +} + +func TestFindSummarizedRule(t *testing.T) { + ruleList := azure.NsgRuleSummaryList{} + rules := make([]azure.NsgRuleSummary, 0) + + // Create some rules + for i := 1; i <= 10; i++ { + rule := azure.NsgRuleSummary{} + rule.Name = fmt.Sprintf("rule_%d", i) + rules = append(rules, rule) + } + ruleList.SummarizedRules = rules + + // Look for a rule that exists + match1 := ruleList.FindRuleByName("rule_5") + assert.Equal(t, "rule_5", match1.Name) + + // Look for a rule that doesn't exist + match2 := ruleList.FindRuleByName("foo") + assert.Equal(t, azure.NsgRuleSummary{}, match2) +} From 92854a782dc0fa73c4bcdc6acf7035d202b4b49f Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 14 Sep 2020 15:27:11 -0400 Subject: [PATCH 04/25] Moving TF config into variables to help config updates --- examples/azure/terraform-azure-nsg-example/main.tf | 6 +++--- .../azure/terraform-azure-nsg-example/variables.tf | 12 ++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/examples/azure/terraform-azure-nsg-example/main.tf b/examples/azure/terraform-azure-nsg-example/main.tf index 89d2bd68f..dc87b88b8 100644 --- a/examples/azure/terraform-azure-nsg-example/main.tf +++ b/examples/azure/terraform-azure-nsg-example/main.tf @@ -1,5 +1,5 @@ provider "azurerm" { - version = "=2.20.0" + version = "~>2.20" features {} } @@ -19,7 +19,7 @@ terraform { resource "azurerm_resource_group" "main" { name = "${var.prefix}-resources" - location = "East US" + location = var.location } # --------------------------------------------------------------------------------------------------------------------- @@ -87,7 +87,7 @@ resource "azurerm_virtual_machine" "main" { location = azurerm_resource_group.main.location resource_group_name = azurerm_resource_group.main.name network_interface_ids = [azurerm_network_interface.main.id] - vm_size = "Standard_B1s" + vm_size = var.vm_size delete_os_disk_on_termination = true delete_data_disks_on_termination = true diff --git a/examples/azure/terraform-azure-nsg-example/variables.tf b/examples/azure/terraform-azure-nsg-example/variables.tf index 8c231c502..ce58a7881 100644 --- a/examples/azure/terraform-azure-nsg-example/variables.tf +++ b/examples/azure/terraform-azure-nsg-example/variables.tf @@ -18,6 +18,12 @@ # These parameters have reasonable defaults. # --------------------------------------------------------------------------------------------------------------------- +variable "location" { + description = "The Azure region in which to deploy this sample" + type = string + default = "East US" +} + variable "hostname" { description = "The hostname of the new VM to be configured" type = string @@ -42,3 +48,9 @@ variable "username" { default = "testadmin" } +variable "vm_size" { + description = "The size of the VM to deploy" + type = string + default = "Standard_B1s" +} + From 64f433754b65378c9ae07590f6047bbc2ceb88b7 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 14 Sep 2020 15:56:24 -0400 Subject: [PATCH 05/25] Updated README fixes; fixed func signature update --- .../azure/terraform-azure-nsg-example/README.md | 14 +++++++------- test/azure/terraform_azure_nsg_example_test.go | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/examples/azure/terraform-azure-nsg-example/README.md b/examples/azure/terraform-azure-nsg-example/README.md index 502a16fba..476d64f1e 100644 --- a/examples/azure/terraform-azure-nsg-example/README.md +++ b/examples/azure/terraform-azure-nsg-example/README.md @@ -1,4 +1,4 @@ -# Terraform Azure @Module@ Example +# Terraform Azure NSG Example This folder contains a simple Terraform module that deploys resources in [Azure](https://azure.microsoft.com/) to demonstrate how you can use Terratest to write automated tests for your Azure Terraform code. This module deploys the following: @@ -35,9 +35,9 @@ it should be free, but you are completely responsible for all Azure charges. 1. [Review environment variables](#review-environment-variables). 1. Install [Golang](https://golang.org/) and make sure this code is checked out into your `GOPATH`. 1. `cd test` -1. Make sure [the azure-sdk-for-go versions match](#check-go-dependencies) in [/test/go.mod](/test/go.mod) and in [test/azure/terraform_azure_@module@_test.go](/test/terraform_azure_nic_test.go). -1. `go build terraform_azure_@module@_test.go` -1. `go test -v -run TestTerraformAzure@Module@Example` +1. Make sure [the azure-sdk-for-go versions match](#check-go-dependencies) in [/test/go.mod](/test/go.mod) and in [test/azure/terraform_azure_nsg_example_test.go](/test/terraform_azure_nsg_example_test.go). +1. `go build terraform_azure_nsg_example_test.go` +1. `go test -v -run TestTerraformAzureNsgExample` ## Module test APIs @@ -92,12 +92,12 @@ Check that the `github.com/Azure/azure-sdk-for-go` version in your generated `go ### Check Azure-sdk-for-go version -Let's make sure [go.mod](https://github.com/gruntwork-io/terratest/blob/master/go.mod) includes the appropriate [azure-sdk-for-go version](https://github.com/Azure/azure-sdk-for-go/releases/tag/v38.1.0): +Let's make sure [go.mod](https://github.com/gruntwork-io/terratest/blob/master/go.mod) includes the appropriate [azure-sdk-for-go version](https://github.com/Azure/azure-sdk-for-go/releases/tag/v46.0.0): ```go require ( ... - github.com/Azure/azure-sdk-for-go v38.1.0+incompatible + github.com/Azure/azure-sdk-for-go v46.0.0+incompatible ... ) ``` @@ -105,7 +105,7 @@ require ( If we make changes to either the **go.mod** or the **go test file**, we should make sure that the go build command works still. ```powershell -go build terraform_azure_@module_test.go +go build terraform_azure_nsg_example_test.go ``` ## Review Environment Variables diff --git a/test/azure/terraform_azure_nsg_example_test.go b/test/azure/terraform_azure_nsg_example_test.go index a1eedc564..43a61471e 100644 --- a/test/azure/terraform_azure_nsg_example_test.go +++ b/test/azure/terraform_azure_nsg_example_test.go @@ -33,7 +33,7 @@ func TestTerraformAzureNsgExample(t *testing.T) { assert.Equal(t, 7, len(rules.SummarizedRules)) // We should have a rule named "allowSSH" - sshRule := rules.FindRuleByName(t, "allowSSH") + sshRule := rules.FindRuleByName("allowSSH") // That rule should allow port 22 inbound assert.True(t, sshRule.AllowsDestinationPort("22")) From bd54eb9cebbf8bdb8cca559b23828175da2a8054 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 14 Sep 2020 16:09:48 -0400 Subject: [PATCH 06/25] Fixed bad terraform example path --- test/azure/terraform_azure_nsg_example_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/azure/terraform_azure_nsg_example_test.go b/test/azure/terraform_azure_nsg_example_test.go index 43a61471e..d57c2e397 100644 --- a/test/azure/terraform_azure_nsg_example_test.go +++ b/test/azure/terraform_azure_nsg_example_test.go @@ -18,7 +18,7 @@ func TestTerraformAzureNsgExample(t *testing.T) { terraformOptions := &terraform.Options{ // The path to where our Terraform code is located - TerraformDir: "../../examples/terraform-azure-nsg-example", + TerraformDir: "../../examples/azure/terraform-azure-nsg-example", } defer terraform.Destroy(t, terraformOptions) From 32aece80a54048e05fe105e336c437bf6d788639 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 14 Sep 2020 19:34:18 -0400 Subject: [PATCH 07/25] Fix linter errors --- modules/azure/nsg.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go index 32bad124d..2e6e8a054 100644 --- a/modules/azure/nsg.go +++ b/modules/azure/nsg.go @@ -179,7 +179,7 @@ func ParsePortRangeString(rangeString string) (low uint16, hight uint16) { } // Is this a range? - if strings.Index(rangeString, "-") == -1 { + if !strings.Contains(rangeString, "-") { val, _ := strconv.ParseInt(rangeString, 10, 16) return uint16(val), uint16(val) } From 259f361e49c3523e16d3ca89f7adaf0b3144df9f Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 14 Sep 2020 20:09:08 -0400 Subject: [PATCH 08/25] Updated linter and fixed remaining issues --- modules/azure/nsg.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go index 2e6e8a054..b1be3f0f5 100644 --- a/modules/azure/nsg.go +++ b/modules/azure/nsg.go @@ -101,7 +101,10 @@ func bindRuleList(dest *[]NsgRuleSummary, source network.SecurityRuleListResultI for source.NotDone() { v := source.Value() *dest = append(*dest, convertToNsgRuleSummary(v.Name, v.SecurityRulePropertiesFormat)) - source.NextWithContext(context.Background()) + err := source.NextWithContext(context.Background()) + if err != nil { + panic(err) + } } } From ae1da94a512c86ff85a7a80fbd4684f3c974cfae Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Fri, 18 Sep 2020 14:58:49 -0400 Subject: [PATCH 09/25] Working through PR feedback items --- .../terraform-azure-nsg-example/README.md | 91 +----------- modules/azure/nsg.go | 137 ++++++++++++------ {test => modules}/azure/nsg_test.go | 35 ++--- 3 files changed, 111 insertions(+), 152 deletions(-) rename {test => modules}/azure/nsg_test.go (67%) diff --git a/examples/azure/terraform-azure-nsg-example/README.md b/examples/azure/terraform-azure-nsg-example/README.md index 476d64f1e..6418200ef 100644 --- a/examples/azure/terraform-azure-nsg-example/README.md +++ b/examples/azure/terraform-azure-nsg-example/README.md @@ -37,93 +37,4 @@ it should be free, but you are completely responsible for all Azure charges. 1. `cd test` 1. Make sure [the azure-sdk-for-go versions match](#check-go-dependencies) in [/test/go.mod](/test/go.mod) and in [test/azure/terraform_azure_nsg_example_test.go](/test/terraform_azure_nsg_example_test.go). 1. `go build terraform_azure_nsg_example_test.go` -1. `go test -v -run TestTerraformAzureNsgExample` - -## Module test APIs - -This module exposes the following top-level API methods: - -- GetDefaultNsgRulesClientE returns a rules client that retuns the _default_ NSG rules
-`func GetDefaultNsgRulesClientE(subscriptionID string) (network.DefaultSecurityRulesClient, error)` -- GetCustomNsgRulesClientE returns a rules client that returns the _custom_ NSG rules
-`func GetCustomNsgRulesClientE(subscriptionID string) (network.SecurityRulesClient, error)` -- GetAllNSGRulesE returns a slice containing all rules from the NSG (including defauls and custom)
-`func GetAllNSGRulesE(resourceGroupName, nsgName, subscriptionID string) (NsgRuleSummaryList, error)` - -Note that the `GetAllNsgRulesE` method returns an `NsgRuleSummaryList`, which is defined as follows: - -- NsgRuleSummaryList holds a colleciton of NsgRuleSummary structs
-` type NsgRuleSummaryList struct { SummarizedRules []NsgRuleSummary } ` - -This collection wrapper has the following method: - -- FindRuleByName looks for a matching rule name
-`func (summarizedRules *NsgRuleSummaryList) FindRuleByName(t *testing.T, name string) NsgRuleSummary` - -Finally, the inner collection is defined as an `NsgRuleSummary` struct, defined as follows: - -``` -type NsgRuleSummary struct { - Name string - Description string - Protocol string - SourcePortRange string - DestinationPortRange string - SourceAddressPrefix string - DestinationAddressPrefix string - Access string - Priority int32 - Direction string -} -``` - -This wrapper has the following methods available: - -- AllowsDestinationPort checks to see if the rule allows a specific destination port
-`func (summarizedRule *NsgRuleSummary) AllowsDestinationPort(port string) bool` -- AllowsSourcePort checks to see if the rule allows a specific source port
-`func (summarizedRule *NsgRuleSummary) AllowsSourcePort(port string) bool` - -## Check Go Dependencies - -Check that the `github.com/Azure/azure-sdk-for-go` version in your generated `go.mod` for this test matches the version in the terratest [go.mod](https://github.com/gruntwork-io/terratest/blob/master/go.mod) file. - -> This was tested with **go1.14.4**. - -### Check Azure-sdk-for-go version - -Let's make sure [go.mod](https://github.com/gruntwork-io/terratest/blob/master/go.mod) includes the appropriate [azure-sdk-for-go version](https://github.com/Azure/azure-sdk-for-go/releases/tag/v46.0.0): - -```go -require ( - ... - github.com/Azure/azure-sdk-for-go v46.0.0+incompatible - ... -) -``` - -If we make changes to either the **go.mod** or the **go test file**, we should make sure that the go build command works still. - -```powershell -go build terraform_azure_nsg_example_test.go -``` - -## Review Environment Variables - -As part of configuring terraform for Azure, we'll want to check that we have set the appropriate [credentials](https://docs.microsoft.com/en-us/azure/terraform/terraform-install-configure?toc=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fazure%2Fterraform%2Ftoc.json&bc=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fazure%2Fbread%2Ftoc.json#set-up-terraform-access-to-azure) and also that we set the [environment variables](https://docs.microsoft.com/en-us/azure/terraform/terraform-install-configure?toc=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fazure%2Fterraform%2Ftoc.json&bc=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fazure%2Fbread%2Ftoc.json#configure-terraform-environment-variables) on the testing host. - -```bash -export ARM_CLIENT_ID=your_app_id -export ARM_CLIENT_SECRET=your_password -export ARM_SUBSCRIPTION_ID=your_subscription_id -export ARM_TENANT_ID=your_tenant_id -``` - -Note, in a Windows environment, these should be set as **system environment variables**. We can use a PowerShell console with administrative rights to update these environment variables: - -```powershell -[System.Environment]::SetEnvironmentVariable("ARM_CLIENT_ID",$your_app_id,[System.EnvironmentVariableTarget]::Machine) -[System.Environment]::SetEnvironmentVariable("ARM_CLIENT_SECRET",$your_password,[System.EnvironmentVariableTarget]::Machine) -[System.Environment]::SetEnvironmentVariable("ARM_SUBSCRIPTION_ID",$your_subscription_id,[System.EnvironmentVariableTarget]::Machine) -[System.Environment]::SetEnvironmentVariable("ARM_TENANT_ID",$your_tenant_id,[System.EnvironmentVariableTarget]::Machine) -``` +1. `go test -v -run TestTerraformAzureNsgExample` \ No newline at end of file diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go index b1be3f0f5..cf307c397 100644 --- a/modules/azure/nsg.go +++ b/modules/azure/nsg.go @@ -2,13 +2,18 @@ package azure import ( "context" + "fmt" "strconv" "strings" + "testing" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-09-01/network" + "github.com/stretchr/testify/assert" ) -// GetDefaultNsgRulesClientE returns a rules client +// GetDefaultNsgRulesClientE returns a rules client which can be used to read the list of *default* security rules +// defined on an network security group. Note that the "default" rules are those provided implicitly +// by the Azure platform. func GetDefaultNsgRulesClientE(subscriptionID string) (network.DefaultSecurityRulesClient, error) { // Validate Azure subscription ID subscriptionID, err := getTargetAzureSubscription(subscriptionID) @@ -28,7 +33,9 @@ func GetDefaultNsgRulesClientE(subscriptionID string) (network.DefaultSecurityRu return nsgClient, nil } -// GetCustomNsgRulesClientE returns a rules client +// GetCustomNsgRulesClientE returns a rules client which can be used to read the list of *custom* security rules +// defined on an network security group. Note that the "custom" rules are those defined by +// end users. func GetCustomNsgRulesClientE(subscriptionID string) (network.SecurityRulesClient, error) { // Validate Azure subscription ID subscriptionID, err := getTargetAzureSubscription(subscriptionID) @@ -48,42 +55,58 @@ func GetCustomNsgRulesClientE(subscriptionID string) (network.SecurityRulesClien return nsgClient, nil } -// GetAllNSGRulesE returns a slice containing all rules from the NSG (including defauls and custom) +// GetAllNSGRulesE returns a slice containing the combined "default" and "custom" rules from a network +// security group. func GetAllNSGRulesE(resourceGroupName, nsgName, subscriptionID string) (NsgRuleSummaryList, error) { defaultRulesClient, err := GetDefaultNsgRulesClientE(subscriptionID) if err != nil { return NsgRuleSummaryList{}, err } + // Get a client instance customRulesClient, err := GetCustomNsgRulesClientE(subscriptionID) if err != nil { return NsgRuleSummaryList{}, err } + // Read all default (platform) rules. defaultRuleList, err := defaultRulesClient.ListComplete(context.Background(), resourceGroupName, nsgName) if err != nil { return NsgRuleSummaryList{}, err } + // Read any custom (user provided) rules customRuleList, err := customRulesClient.ListComplete(context.Background(), resourceGroupName, nsgName) if err != nil { return NsgRuleSummaryList{}, err } - rules := make([]NsgRuleSummary, 0) - bindRuleList(&rules, defaultRuleList) - bindRuleList(&rules, customRuleList) + // Convert the default list to our summary type + boundDefaultRules, err := bindRuleList(defaultRuleList) + if err != nil { + return NsgRuleSummaryList{}, err + } + + // Convert the custom list to our summary type + boundCustomRules, err := bindRuleList(customRuleList) + if err != nil { + return NsgRuleSummaryList{}, err + } + + // Join the summarized lists and wrap in NsgRuleSummaryList struct + allRules := append(boundDefaultRules, boundCustomRules...) ruleList := NsgRuleSummaryList{} - ruleList.SummarizedRules = rules + ruleList.SummarizedRules = allRules return ruleList, nil } -// NsgRuleSummaryList holds a colleciton of NsgRuleSummary structs +// NsgRuleSummaryList holds a colleciton of NsgRuleSummary rules type NsgRuleSummaryList struct { SummarizedRules []NsgRuleSummary } -// NsgRuleSummary is a string-based summary of an NSG rule with methods attached. +// NsgRuleSummary is a string-based (non-pointer) summary of an NSG rule with several helper methods attached +// to help with verification of rule configuratoin. type NsgRuleSummary struct { Name string Description string @@ -97,24 +120,28 @@ type NsgRuleSummary struct { Direction string } -func bindRuleList(dest *[]NsgRuleSummary, source network.SecurityRuleListResultIterator) { +// bindRuleList takes a raw list of security rules from the SDK and converts them into a string-based +// summary struct. +func bindRuleList(source network.SecurityRuleListResultIterator) ([]NsgRuleSummary, error) { + rules := make([]NsgRuleSummary, 0) for source.NotDone() { v := source.Value() - *dest = append(*dest, convertToNsgRuleSummary(v.Name, v.SecurityRulePropertiesFormat)) + rules = append(rules, convertToNsgRuleSummary(v.Name, v.SecurityRulePropertiesFormat)) err := source.NextWithContext(context.Background()) if err != nil { - panic(err) + return []NsgRuleSummary{}, err } } + return rules, nil } +// convertToNsgRuleSummary converst the raw SDK security rule type into a summarized struct, flattening the +// rules properties and name into a single, string-based struct. func convertToNsgRuleSummary(name *string, rule *network.SecurityRulePropertiesFormat) NsgRuleSummary { summary := NsgRuleSummary{} if rule.Description != nil { summary.Description = *rule.Description - } else { - summary.Description = "" } summary.Name = *name @@ -129,7 +156,7 @@ func convertToNsgRuleSummary(name *string, rule *network.SecurityRulePropertiesF return summary } -// FindRuleByName looks for a matching rule name +// FindRuleByName looks for a matching rule by name within a collection of rules. func (summarizedRules *NsgRuleSummaryList) FindRuleByName(name string) NsgRuleSummary { for _, r := range summarizedRules.SummarizedRules { if r.Name == name { @@ -140,55 +167,79 @@ func (summarizedRules *NsgRuleSummaryList) FindRuleByName(name string) NsgRuleSu return NsgRuleSummary{} } -// AllowsDestinationPort checks to see if the rule allows a specific destination port. -func (summarizedRule *NsgRuleSummary) AllowsDestinationPort(port string) bool { - if summarizedRule.DestinationPortRange == "*" { - return true - } +// AllowsDestinationPort checks to see if the rule allows a specific destination port. This is helpful when verifying +// that a given rule is configured properly for a given port. +func (summarizedRule *NsgRuleSummary) AllowsDestinationPort(t *testing.T, port string) bool { + allowed, err := portRangeAllowsPort(summarizedRule.DestinationPortRange, port) + assert.NoError(t, err) + return allowed +} - low, high := ParsePortRangeString(summarizedRule.DestinationPortRange) - portAsInt, _ := strconv.ParseInt(port, 10, 16) +// AllowsSourcePort checks to see if the rule allows a specific source port. This is helpful when verifying +// that a given rule is configured properly for a given port. +func (summarizedRule *NsgRuleSummary) AllowsSourcePort(t *testing.T, port string) bool { + allowed, err := portRangeAllowsPort(summarizedRule.SourcePortRange, port) + assert.NoError(t, err) + return allowed +} - if (port == "*") && (low == 0) && (high == 65535) { - return true +// portRangeAllowsPort is the internal impelmentation of AllowsSourcePort and AllowsDestinationPort. +func portRangeAllowsPort(portRange string, port string) (bool, error) { + if portRange == "*" { + return true, nil } - return ((low <= uint16(portAsInt)) && (uint16(portAsInt) <= high)) -} - -// AllowsSourcePort checks to see if the rule allows a specific source port. -func (summarizedRule *NsgRuleSummary) AllowsSourcePort(port string) bool { - if summarizedRule.SourcePortRange == "*" { - return true + // Decode the provided port range + low, high, parseErr := parsePortRangeString(portRange) + if parseErr != nil { + return false, parseErr } - low, high := ParsePortRangeString(summarizedRule.SourcePortRange) - portAsInt, _ := strconv.ParseInt(port, 10, 16) + // Decode user-provided port + portAsInt, parseErr := strconv.ParseInt(port, 10, 16) + if (parseErr != nil) && (port != "*") { + return false, parseErr + } if (port == "*") && (low == 0) && (high == 65535) { - return true + return true, nil } - return ((low <= uint16(portAsInt)) && (uint16(portAsInt) <= high)) + return ((uint16(portAsInt) >= low) && (uint16(portAsInt) <= high)), nil } -// ParsePortRangeString decodes a range string ("2-100") or a single digit ("22") and returns +// parsePortRangeString decodes a range string ("2-100") or a single digit ("22") and returns // a tuple in [low, hi] form. Note that if a single digit is supplied, both members of the // return tuple will be the same value (e.g., "22" returns (22, 22)) -func ParsePortRangeString(rangeString string) (low uint16, hight uint16) { +func parsePortRangeString(rangeString string) (uint16, uint16, error) { // Is this an asterisk? if rangeString == "*" { - return uint16(0), uint16(65535) + return uint16(0), uint16(65535), nil } // Is this a range? if !strings.Contains(rangeString, "-") { - val, _ := strconv.ParseInt(rangeString, 10, 16) - return uint16(val), uint16(val) + val, parseErr := strconv.ParseInt(rangeString, 10, 16) + if parseErr != nil { + return 0, 0, parseErr + } + return uint16(val), uint16(val), nil } parts := strings.Split(rangeString, "-") - lowVal, _ := strconv.ParseInt(parts[0], 10, 16) - highVal, _ := strconv.ParseInt(parts[1], 10, 16) - return uint16(lowVal), uint16(highVal) + if len(parts) != 2 { + return 0, 0, fmt.Errorf("Invalid port range specified; must be of the format '{low port}-{high port}'") + } + + lowVal, parseErr := strconv.ParseInt(parts[0], 10, 16) + if parseErr != nil { + return 0, 0, parseErr + } + + highVal, parseErr := strconv.ParseInt(parts[1], 10, 16) + if parseErr != nil { + return 0, 0, parseErr + } + + return uint16(lowVal), uint16(highVal), nil } diff --git a/test/azure/nsg_test.go b/modules/azure/nsg_test.go similarity index 67% rename from test/azure/nsg_test.go rename to modules/azure/nsg_test.go index 173a9fb7e..d8edaba7b 100644 --- a/test/azure/nsg_test.go +++ b/modules/azure/nsg_test.go @@ -1,9 +1,7 @@ -// +build azure - // NOTE: We use build tags to differentiate azure testing because we currently do not have azure access setup for // CircleCI. -package test +package azure import ( "fmt" @@ -11,71 +9,70 @@ import ( "testing" "time" - "github.com/gruntwork-io/terratest/modules/azure" "github.com/stretchr/testify/assert" ) func TestRangeParsingSinglePort(t *testing.T) { - lo, hi := azure.ParsePortRangeString("22") + lo, hi, _ := parsePortRangeString("22") assert.Equal(t, uint16(22), lo) assert.Equal(t, uint16(22), hi) } func TestRangeParsingPortRange(t *testing.T) { - lo, hi := azure.ParsePortRangeString("22-80") + lo, hi, _ := parsePortRangeString("22-80") assert.Equal(t, uint16(22), lo) assert.Equal(t, uint16(80), hi) } func TestRangeParsingAsterisk(t *testing.T) { - lo, hi := azure.ParsePortRangeString("*") + lo, hi, _ := parsePortRangeString("*") assert.Equal(t, uint16(0), lo) assert.Equal(t, uint16(65535), hi) } func TestRuleSummaryAllowSourcePort(t *testing.T) { - summary := azure.NsgRuleSummary{} + summary := NsgRuleSummary{} summary.SourcePortRange = "22" - result := summary.AllowsSourcePort("22") + result := summary.AllowsSourcePort(t, "22") assert.True(t, result) } func TestRuleSummaryAllowSourcePortAsterisk(t *testing.T) { - summary := azure.NsgRuleSummary{} + summary := NsgRuleSummary{} summary.SourcePortRange = "*" rand := rand.New(rand.NewSource(time.Now().UnixNano())) - result := summary.AllowsSourcePort(string(uint16(rand.Int()))) + result := summary.AllowsSourcePort(t, string(uint16(rand.Int()))) assert.True(t, result) } func TestRuleSummaryAllowDestinationPort(t *testing.T) { - summary := azure.NsgRuleSummary{} + summary := NsgRuleSummary{} summary.DestinationPortRange = "80" - result := summary.AllowsDestinationPort("80") + result := summary.AllowsDestinationPort(t, "80") assert.True(t, result) } func TestRuleSummaryAllowDestinationPortAsterisk(t *testing.T) { - summary := azure.NsgRuleSummary{} + summary := NsgRuleSummary{} summary.DestinationPortRange = "*" rand := rand.New(rand.NewSource(time.Now().UnixNano())) - result := summary.AllowsDestinationPort(string(uint16(rand.Int()))) + result := summary.AllowsDestinationPort(t, string(uint16(rand.Int()))) assert.True(t, result) } func TestFindSummarizedRule(t *testing.T) { - ruleList := azure.NsgRuleSummaryList{} - rules := make([]azure.NsgRuleSummary, 0) + ruleList := NsgRuleSummaryList{} + rules := make([]NsgRuleSummary, 0) // Create some rules for i := 1; i <= 10; i++ { - rule := azure.NsgRuleSummary{} + rule := NsgRuleSummary{} rule.Name = fmt.Sprintf("rule_%d", i) rules = append(rules, rule) } @@ -87,5 +84,5 @@ func TestFindSummarizedRule(t *testing.T) { // Look for a rule that doesn't exist match2 := ruleList.FindRuleByName("foo") - assert.Equal(t, azure.NsgRuleSummary{}, match2) + assert.Equal(t, NsgRuleSummary{}, match2) } From 07ee3c780ac80996d8a21f0c90cbbc2b849f65ca Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 21 Sep 2020 10:35:49 -0400 Subject: [PATCH 10/25] Switch tests over to table-based testing --- modules/azure/nsg.go | 85 +++++++++++++--------- modules/azure/nsg_test.go | 147 +++++++++++++++++++++++--------------- 2 files changed, 143 insertions(+), 89 deletions(-) diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go index cf307c397..1697defd3 100644 --- a/modules/azure/nsg.go +++ b/modules/azure/nsg.go @@ -11,6 +11,26 @@ import ( "github.com/stretchr/testify/assert" ) +// NsgRuleSummaryList holds a colleciton of NsgRuleSummary rules +type NsgRuleSummaryList struct { + SummarizedRules []NsgRuleSummary +} + +// NsgRuleSummary is a string-based (non-pointer) summary of an NSG rule with several helper methods attached +// to help with verification of rule configuratoin. +type NsgRuleSummary struct { + Name string + Description string + Protocol string + SourcePortRange string + DestinationPortRange string + SourceAddressPrefix string + DestinationAddressPrefix string + Access string + Priority int32 + Direction string +} + // GetDefaultNsgRulesClientE returns a rules client which can be used to read the list of *default* security rules // defined on an network security group. Note that the "default" rules are those provided implicitly // by the Azure platform. @@ -55,7 +75,7 @@ func GetCustomNsgRulesClientE(subscriptionID string) (network.SecurityRulesClien return nsgClient, nil } -// GetAllNSGRulesE returns a slice containing the combined "default" and "custom" rules from a network +// GetAllNSGRulesE returns an NsgRuleSummaryList instance containing the combined "default" and "custom" rules from a network // security group. func GetAllNSGRulesE(resourceGroupName, nsgName, subscriptionID string) (NsgRuleSummaryList, error) { defaultRulesClient, err := GetDefaultNsgRulesClientE(subscriptionID) @@ -100,26 +120,6 @@ func GetAllNSGRulesE(resourceGroupName, nsgName, subscriptionID string) (NsgRule return ruleList, nil } -// NsgRuleSummaryList holds a colleciton of NsgRuleSummary rules -type NsgRuleSummaryList struct { - SummarizedRules []NsgRuleSummary -} - -// NsgRuleSummary is a string-based (non-pointer) summary of an NSG rule with several helper methods attached -// to help with verification of rule configuratoin. -type NsgRuleSummary struct { - Name string - Description string - Protocol string - SourcePortRange string - DestinationPortRange string - SourceAddressPrefix string - DestinationAddressPrefix string - Access string - Priority int32 - Direction string -} - // bindRuleList takes a raw list of security rules from the SDK and converts them into a string-based // summary struct. func bindRuleList(source network.SecurityRuleListResultIterator) ([]NsgRuleSummary, error) { @@ -139,24 +139,36 @@ func bindRuleList(source network.SecurityRuleListResultIterator) ([]NsgRuleSumma // rules properties and name into a single, string-based struct. func convertToNsgRuleSummary(name *string, rule *network.SecurityRulePropertiesFormat) NsgRuleSummary { summary := NsgRuleSummary{} - - if rule.Description != nil { - summary.Description = *rule.Description - } - - summary.Name = *name + summary.Description = safePtrToString(rule.Description) + summary.Name = safePtrToString(name) summary.Protocol = string(rule.Protocol) - summary.SourcePortRange = *rule.SourcePortRange - summary.DestinationPortRange = *rule.DestinationPortRange - summary.SourceAddressPrefix = *rule.SourceAddressPrefix - summary.DestinationAddressPrefix = *rule.DestinationAddressPrefix + summary.SourcePortRange = safePtrToString(rule.SourcePortRange) + summary.DestinationPortRange = safePtrToString(rule.DestinationPortRange) + summary.SourceAddressPrefix = safePtrToString(rule.SourceAddressPrefix) + summary.DestinationAddressPrefix = safePtrToString(rule.DestinationAddressPrefix) summary.Access = string(rule.Access) - summary.Priority = *rule.Priority + summary.Priority = safePtrToInt32(rule.Priority) summary.Direction = string(rule.Direction) return summary } -// FindRuleByName looks for a matching rule by name within a collection of rules. +// safePtrToString converts a string pointer to a non-pointer string value, or to "" if the pointer is nil. +func safePtrToString(raw *string) string { + if raw == nil { + return "" + } + return *raw +} + +// safePtrToInt32 converts a int32 pointer to a non-pointer int32 value, or to 0 if the pointer is nil. +func safePtrToInt32(raw *int32) int32 { + if raw == nil { + return 0 + } + return *raw +} + +// FindRuleByName looks for a matching rule by name within the current collection of rules. func (summarizedRules *NsgRuleSummaryList) FindRuleByName(name string) NsgRuleSummary { for _, r := range summarizedRules.SummarizedRules { if r.Name == name { @@ -241,5 +253,12 @@ func parsePortRangeString(rangeString string) (uint16, uint16, error) { return 0, 0, parseErr } + // Normalize ordering + if lowVal > highVal { + temp := lowVal + lowVal = highVal + highVal = temp + } + return uint16(lowVal), uint16(highVal), nil } diff --git a/modules/azure/nsg_test.go b/modules/azure/nsg_test.go index d8edaba7b..df4816477 100644 --- a/modules/azure/nsg_test.go +++ b/modules/azure/nsg_test.go @@ -5,84 +5,119 @@ package azure import ( "fmt" - "math/rand" "testing" - "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestRangeParsingSinglePort(t *testing.T) { - lo, hi, _ := parsePortRangeString("22") - assert.Equal(t, uint16(22), lo) - assert.Equal(t, uint16(22), hi) -} - -func TestRangeParsingPortRange(t *testing.T) { - lo, hi, _ := parsePortRangeString("22-80") - assert.Equal(t, uint16(22), lo) - assert.Equal(t, uint16(80), hi) -} - -func TestRangeParsingAsterisk(t *testing.T) { - lo, hi, _ := parsePortRangeString("*") - assert.Equal(t, uint16(0), lo) - assert.Equal(t, uint16(65535), hi) -} - -func TestRuleSummaryAllowSourcePort(t *testing.T) { - summary := NsgRuleSummary{} - summary.SourcePortRange = "22" - - result := summary.AllowsSourcePort(t, "22") - assert.True(t, result) -} - -func TestRuleSummaryAllowSourcePortAsterisk(t *testing.T) { - summary := NsgRuleSummary{} - summary.SourcePortRange = "*" - - rand := rand.New(rand.NewSource(time.Now().UnixNano())) +func TestPortRangeParsing(t *testing.T) { + var cases = []struct { + portRange string + expectedLo int + expectedHi int + expectsError bool + }{ + {"22", 22, 22, false}, + {"22-80", 22, 80, false}, + {"*", 0, 65535, false}, + {"*-*", 0, 0, true}, + {"22-", 0, 0, true}, + {"-80", 0, 0, true}, + {"-", 0, 0, true}, + {"80-22", 22, 80, false}, + } - result := summary.AllowsSourcePort(t, string(uint16(rand.Int()))) - assert.True(t, result) + for _, tt := range cases { + t.Run(tt.portRange, func(t *testing.T) { + lo, hi, err := parsePortRangeString(tt.portRange) + if !tt.expectsError { + require.NoError(t, err) + } + assert.Equal(t, tt.expectedLo, int(lo)) + assert.Equal(t, tt.expectedHi, int(hi)) + }) + } } -func TestRuleSummaryAllowDestinationPort(t *testing.T) { - summary := NsgRuleSummary{} - summary.DestinationPortRange = "80" +func TestAllowSourcePort(t *testing.T) { + var cases = []struct { + CaseName string + SourcePortRange string + TestPort string + Result bool + }{ + {"22 allows 22", "22", "22", true}, + {"22 doesn't allow 80", "22", "80", false}, + {"Any allows any", "*", "*", true}, + } - result := summary.AllowsDestinationPort(t, "80") - assert.True(t, result) + for _, tt := range cases { + t.Run(tt.CaseName, func(t *testing.T) { + summary := NsgRuleSummary{} + summary.SourcePortRange = tt.SourcePortRange + result := summary.AllowsSourcePort(t, tt.TestPort) + assert.Equal(t, result, tt.Result) + }) + } } -func TestRuleSummaryAllowDestinationPortAsterisk(t *testing.T) { - summary := NsgRuleSummary{} - summary.DestinationPortRange = "*" - - rand := rand.New(rand.NewSource(time.Now().UnixNano())) +func TestAllowDestinationPort(t *testing.T) { + var cases = []struct { + CaseName string + SourcePortRange string + TestPort string + Result bool + }{ + {"22 allows 22", "22", "22", true}, + {"22 doesn't allow 80", "22", "80", false}, + {"Any allows any", "*", "*", true}, + } - result := summary.AllowsDestinationPort(t, string(uint16(rand.Int()))) - assert.True(t, result) + for _, tt := range cases { + t.Run(tt.CaseName, func(t *testing.T) { + summary := NsgRuleSummary{} + summary.DestinationPortRange = tt.SourcePortRange + result := summary.AllowsDestinationPort(t, tt.TestPort) + assert.Equal(t, result, tt.Result) + }) + } } func TestFindSummarizedRule(t *testing.T) { + var cases = []struct { + SearchString string + Result bool + }{ + {"rule_1", true}, + {"rule_2", true}, + {"rule_3", true}, + {"rule_4", true}, + {"rule_5", true}, + {"rule_6", false}, + {"", false}, + {"foo", false}, + } + ruleList := NsgRuleSummaryList{} rules := make([]NsgRuleSummary, 0) - // Create some rules - for i := 1; i <= 10; i++ { + // Create some base rules + for i := 1; i <= 5; i++ { rule := NsgRuleSummary{} rule.Name = fmt.Sprintf("rule_%d", i) rules = append(rules, rule) } ruleList.SummarizedRules = rules - // Look for a rule that exists - match1 := ruleList.FindRuleByName("rule_5") - assert.Equal(t, "rule_5", match1.Name) - - // Look for a rule that doesn't exist - match2 := ruleList.FindRuleByName("foo") - assert.Equal(t, NsgRuleSummary{}, match2) + for _, tt := range cases { + t.Run(tt.SearchString, func(t *testing.T) { + match := ruleList.FindRuleByName(tt.SearchString) + if tt.Result { + assert.Equal(t, tt.SearchString, match.Name) + } else { + assert.Equal(t, match, NsgRuleSummary{}) + } + }) + } } From 870af30aaaa3bf38f4b808e76669897ee9a9c23c Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 21 Sep 2020 11:05:55 -0400 Subject: [PATCH 11/25] Fixing up some additional cases; considering the Allow/Deny state of the rules now --- modules/azure/nsg.go | 18 +++++++++++---- modules/azure/nsg_test.go | 48 ++++++++++++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go index 1697defd3..aba925638 100644 --- a/modules/azure/nsg.go +++ b/modules/azure/nsg.go @@ -184,7 +184,7 @@ func (summarizedRules *NsgRuleSummaryList) FindRuleByName(name string) NsgRuleSu func (summarizedRule *NsgRuleSummary) AllowsDestinationPort(t *testing.T, port string) bool { allowed, err := portRangeAllowsPort(summarizedRule.DestinationPortRange, port) assert.NoError(t, err) - return allowed + return allowed && (summarizedRule.Access == "Allow") } // AllowsSourcePort checks to see if the rule allows a specific source port. This is helpful when verifying @@ -192,7 +192,7 @@ func (summarizedRule *NsgRuleSummary) AllowsDestinationPort(t *testing.T, port s func (summarizedRule *NsgRuleSummary) AllowsSourcePort(t *testing.T, port string) bool { allowed, err := portRangeAllowsPort(summarizedRule.SourcePortRange, port) assert.NoError(t, err) - return allowed + return allowed && (summarizedRule.Access == "Allow") } // portRangeAllowsPort is the internal impelmentation of AllowsSourcePort and AllowsDestinationPort. @@ -213,10 +213,12 @@ func portRangeAllowsPort(portRange string, port string) (bool, error) { return false, parseErr } + // If the user wants to check "all", make sure we parsed input range to include all ports. if (port == "*") && (low == 0) && (high == 65535) { return true, nil } + // Evaluate and return return ((uint16(portAsInt) >= low) && (uint16(portAsInt) <= high)), nil } @@ -224,12 +226,12 @@ func portRangeAllowsPort(portRange string, port string) (bool, error) { // a tuple in [low, hi] form. Note that if a single digit is supplied, both members of the // return tuple will be the same value (e.g., "22" returns (22, 22)) func parsePortRangeString(rangeString string) (uint16, uint16, error) { - // Is this an asterisk? + // An asterisk means all ports if rangeString == "*" { return uint16(0), uint16(65535), nil } - // Is this a range? + // Check for range string that contains hyphen separator if !strings.Contains(rangeString, "-") { val, parseErr := strconv.ParseInt(rangeString, 10, 16) if parseErr != nil { @@ -238,27 +240,33 @@ func parsePortRangeString(rangeString string) (uint16, uint16, error) { return uint16(val), uint16(val), nil } + // Split the rang into parts and validate parts := strings.Split(rangeString, "-") if len(parts) != 2 { return 0, 0, fmt.Errorf("Invalid port range specified; must be of the format '{low port}-{high port}'") } + // Assume the low port is listed first; parse it lowVal, parseErr := strconv.ParseInt(parts[0], 10, 16) if parseErr != nil { return 0, 0, parseErr } + // Assume the hi port is listed first; parse it highVal, parseErr := strconv.ParseInt(parts[1], 10, 16) if parseErr != nil { return 0, 0, parseErr } - // Normalize ordering + // Normalize ordering in the case that low and hi were reversed. + // This should _never_ happen, as the Azure API's won't allow it, but + // we shouldn't fail if it's the case. if lowVal > highVal { temp := lowVal lowVal = highVal highVal = temp } + // Return values return uint16(lowVal), uint16(highVal), nil } diff --git a/modules/azure/nsg_test.go b/modules/azure/nsg_test.go index df4816477..f03360580 100644 --- a/modules/azure/nsg_test.go +++ b/modules/azure/nsg_test.go @@ -9,6 +9,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-09-01/network" ) func TestPortRangeParsing(t *testing.T) { @@ -40,24 +42,51 @@ func TestPortRangeParsing(t *testing.T) { } } +func TestNsgRuleSummaryConverstion(t *testing.T) { + // Quick test to make sure the safe nil handling is working + var name = "test name" + var sdkStruct = network.SecurityRulePropertiesFormat{} + sdkStruct.Description = nil + sdkStruct.Protocol = network.SecurityRuleProtocolTCP + sdkStruct.SourcePortRange = nil + sdkStruct.DestinationPortRange = nil + sdkStruct.SourceAddressPrefix = nil + sdkStruct.DestinationAddressPrefix = nil + sdkStruct.Access = network.SecurityRuleAccessAllow + sdkStruct.Priority = nil + sdkStruct.Direction = network.SecurityRuleDirectionInbound + + // Verify the nil values were correctly defaulted to "" without a panic + result := convertToNsgRuleSummary(&name, &sdkStruct) + assert.Equal(t, "", result.Description) + assert.Equal(t, "", result.SourcePortRange) + assert.Equal(t, "", result.DestinationPortRange) + assert.Equal(t, "", result.SourceAddressPrefix) + assert.Equal(t, "", result.DestinationAddressPrefix) + assert.Equal(t, int32(0), result.Priority) +} + func TestAllowSourcePort(t *testing.T) { var cases = []struct { CaseName string SourcePortRange string + Access string TestPort string Result bool }{ - {"22 allows 22", "22", "22", true}, - {"22 doesn't allow 80", "22", "80", false}, - {"Any allows any", "*", "*", true}, + {"22 allowed", "22", "Allow", "22", true}, + {"22 denied", "22", "Deny", "22", false}, + {"22 doesn't allow 80", "22", "Allow", "80", false}, + {"Any allows any", "*", "Allow", "*", true}, } for _, tt := range cases { t.Run(tt.CaseName, func(t *testing.T) { summary := NsgRuleSummary{} summary.SourcePortRange = tt.SourcePortRange + summary.Access = tt.Access result := summary.AllowsSourcePort(t, tt.TestPort) - assert.Equal(t, result, tt.Result) + assert.Equal(t, tt.Result, result) }) } } @@ -66,20 +95,23 @@ func TestAllowDestinationPort(t *testing.T) { var cases = []struct { CaseName string SourcePortRange string + Access string TestPort string Result bool }{ - {"22 allows 22", "22", "22", true}, - {"22 doesn't allow 80", "22", "80", false}, - {"Any allows any", "*", "*", true}, + {"22 allowed", "22", "Allow", "22", true}, + {"22 denied", "22", "Deny", "22", false}, + {"22 doesn't allow 80", "22", "Allow", "80", false}, + {"Any allows any", "*", "Allow", "*", true}, } for _, tt := range cases { t.Run(tt.CaseName, func(t *testing.T) { summary := NsgRuleSummary{} summary.DestinationPortRange = tt.SourcePortRange + summary.Access = tt.Access result := summary.AllowsDestinationPort(t, tt.TestPort) - assert.Equal(t, result, tt.Result) + assert.Equal(t, tt.Result, result) }) } } From 095131a34adc559a9729464b9d229c4df5530e4a Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 21 Sep 2020 11:08:38 -0400 Subject: [PATCH 12/25] Fixing doc links --- examples/azure/terraform-azure-nsg-example/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/azure/terraform-azure-nsg-example/README.md b/examples/azure/terraform-azure-nsg-example/README.md index 6418200ef..ba842947b 100644 --- a/examples/azure/terraform-azure-nsg-example/README.md +++ b/examples/azure/terraform-azure-nsg-example/README.md @@ -34,7 +34,7 @@ it should be free, but you are completely responsible for all Azure charges. 1. Install [Terraform](https://www.terraform.io/) and make sure it's on your `PATH`. 1. [Review environment variables](#review-environment-variables). 1. Install [Golang](https://golang.org/) and make sure this code is checked out into your `GOPATH`. -1. `cd test` +1. `cd test/azure` 1. Make sure [the azure-sdk-for-go versions match](#check-go-dependencies) in [/test/go.mod](/test/go.mod) and in [test/azure/terraform_azure_nsg_example_test.go](/test/terraform_azure_nsg_example_test.go). 1. `go build terraform_azure_nsg_example_test.go` 1. `go test -v -run TestTerraformAzureNsgExample` \ No newline at end of file From 1eafe941ac97e65ae2b8e1858724ea3c8787fabf Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Mon, 21 Sep 2020 20:36:06 -0400 Subject: [PATCH 13/25] Working on variable uniqueness --- .../azure/terraform-azure-nsg-example/main.tf | 21 +++--- .../terraform-azure-nsg-example/variables.tf | 71 +++++++++++++++---- .../azure/terraform_azure_nsg_example_test.go | 53 +++++++++++++- 3 files changed, 120 insertions(+), 25 deletions(-) diff --git a/examples/azure/terraform-azure-nsg-example/main.tf b/examples/azure/terraform-azure-nsg-example/main.tf index dc87b88b8..a06316a09 100644 --- a/examples/azure/terraform-azure-nsg-example/main.tf +++ b/examples/azure/terraform-azure-nsg-example/main.tf @@ -18,7 +18,7 @@ terraform { # --------------------------------------------------------------------------------------------------------------------- resource "azurerm_resource_group" "main" { - name = "${var.prefix}-resources" + name = var.resource_group_name location = var.location } @@ -27,33 +27,33 @@ resource "azurerm_resource_group" "main" { # --------------------------------------------------------------------------------------------------------------------- resource "azurerm_virtual_network" "main" { - name = "${var.prefix}-network" + name = var.vnet_name address_space = ["10.0.0.0/16"] location = azurerm_resource_group.main.location resource_group_name = azurerm_resource_group.main.name } resource "azurerm_subnet" "internal" { - name = "internal" + name = var.subnet_name resource_group_name = azurerm_resource_group.main.name virtual_network_name = azurerm_virtual_network.main.name address_prefixes = ["10.0.17.0/24"] } resource "azurerm_network_interface" "main" { - name = "${var.prefix}-nic" + name = var.vm_nic_name location = azurerm_resource_group.main.location resource_group_name = azurerm_resource_group.main.name ip_configuration { - name = "terratestconfiguration1" + name = var.vm_nic_ip_config_name subnet_id = azurerm_subnet.internal.id private_ip_address_allocation = "Dynamic" } } resource "azurerm_network_security_group" "main" { - name = "testSecurityGroup1" + name = var.nsg_name location = azurerm_resource_group.main.location resource_group_name = azurerm_resource_group.main.name } @@ -64,8 +64,8 @@ resource "azurerm_network_interface_security_group_association" "main" { } resource "azurerm_network_security_rule" "main" { - name = "allowSSH" - description = "allowSSH" + name = var.nsg_rule_name + description = var.nsg_rule_name priority = 100 direction = "Inbound" access = "Allow" @@ -80,10 +80,11 @@ resource "azurerm_network_security_rule" "main" { # --------------------------------------------------------------------------------------------------------------------- # DEPLOY A VIRTUAL MACHINE RUNNING UBUNTU +# This VM does not actually do anything and is the smallest size VM available with an Ubuntu image # --------------------------------------------------------------------------------------------------------------------- resource "azurerm_virtual_machine" "main" { - name = "${var.prefix}-vm" + name = var.vm_name location = azurerm_resource_group.main.location resource_group_name = azurerm_resource_group.main.name network_interface_ids = [azurerm_network_interface.main.id] @@ -99,7 +100,7 @@ resource "azurerm_virtual_machine" "main" { } storage_os_disk { - name = "terratestosdisk1" + name = var.os_disk_name caching = "ReadWrite" create_option = "FromImage" managed_disk_type = "Standard_LRS" diff --git a/examples/azure/terraform-azure-nsg-example/variables.tf b/examples/azure/terraform-azure-nsg-example/variables.tf index ce58a7881..f3a5140c4 100644 --- a/examples/azure/terraform-azure-nsg-example/variables.tf +++ b/examples/azure/terraform-azure-nsg-example/variables.tf @@ -18,34 +18,58 @@ # These parameters have reasonable defaults. # --------------------------------------------------------------------------------------------------------------------- +variable "resource_group_name" { + description = "Name for the resource group holding resources for this example" + type = string + default = "terratest-nsg-example" +} + variable "location" { description = "The Azure region in which to deploy this sample" type = string default = "East US" } -variable "hostname" { - description = "The hostname of the new VM to be configured" +variable "vnet_name" { + description = "Name for the example virtual network" type = string - default = "terratest-vm" + default = "terratest-nsg-example-network" } -variable "password" { - description = "The password to configure for SSH access" +variable "subnet_name" { + description = "Name for the example virtual network default subnet" type = string - default = "HorriblePassword1234!" + default = "terratest-nsg-example-subnet" } -variable "prefix" { - description = "The prefix that will be attached to all resources deployed" +variable "vm_nic_name" { + description = "Name for the NIC attached to our example VM." type = string - default = "terratest-nsg-example" + default = "terratest-nsg-example-nic" } -variable "username" { - description = "The username to be provisioned into your VM" +variable "vm_nic_ip_config_name" { + description = "Name for the NIC IP configuration attached to our example VM." type = string - default = "testadmin" + default = "terratest-nsg-example-nic-ip-config" +} + +variable "nsg_name" { + description = "Name for the example NSG." + type = string + default = "terratest-nsg-example-nsg" +} + +variable "nsg_rule_name" { + description = "Name for the example NSG rule used in this example." + type = string + default = "terratest-nsg-example-nsg-rule" +} + +variable "vm_name" { + description = "The name of the VM used in this example" + type = string + default = "terratest-nsg-example-vm" } variable "vm_size" { @@ -54,3 +78,26 @@ variable "vm_size" { default = "Standard_B1s" } +variable "hostname" { + description = "The hostname of the new VM to be configured" + type = string + default = "terratest-nsg-example-vm" +} + +variable "os_disk_name" { + description = "The of the OS disk to use on our example VM." + type = string + default = "terratest-nsg-example-os-disk" +} + +variable "password" { + description = "The password to configure for SSH access" + type = string + default = "HorriblePassword1234!" +} + +variable "username" { + description = "The username to be provisioned into your VM" + type = string + default = "testadmin" +} diff --git a/test/azure/terraform_azure_nsg_example_test.go b/test/azure/terraform_azure_nsg_example_test.go index d57c2e397..518e3306d 100644 --- a/test/azure/terraform_azure_nsg_example_test.go +++ b/test/azure/terraform_azure_nsg_example_test.go @@ -6,9 +6,11 @@ package test import ( + "fmt" "testing" "github.com/gruntwork-io/terratest/modules/azure" + "github.com/gruntwork-io/terratest/modules/random" "github.com/gruntwork-io/terratest/modules/terraform" "github.com/stretchr/testify/assert" ) @@ -16,9 +18,54 @@ import ( func TestTerraformAzureNsgExample(t *testing.T) { t.Parallel() + // + // Setup our variables to be unique per test-run: + // + + // "resource_group_name" + expectedResourceGroupName := fmt.Sprintf("terratest-nsg-example-%s", random.UniqueId()) + + // "vnet_name" + // "subnet_name" + expectedVnetName := fmt.Sprintf("vnet_name_%s", random.UniqueId()) + expectedSubnetName := fmt.Sprintf("subnet_name_%s", random.UniqueId()) + + // "vm_nic_name" + // "vm_nic_ip_config_name" + expectedNICName := fmt.Sprintf("vm_nic_name_%s", random.UniqueId()) + expectedIPConfigName := fmt.Sprintf("vm_nic_ip_config_name_%s", random.UniqueId()) + + // "nsg_name" + // "nsg_rule_name" + expectedNSGName := fmt.Sprintf("nsg_name_%s", random.UniqueId()) + expectedNSGRuleName := fmt.Sprintf("nsg_rule_name_%s", random.UniqueId()) + + // "vm_name" + // "hostname" + // "os_disk_name" + expectedVMName := fmt.Sprintf("vm_name_%s", random.UniqueId()) + expectedHostName := fmt.Sprintf("hostname_%s", random.UniqueId()) + expectedOSDiskName := fmt.Sprintf("os_disk_name_%s", random.UniqueId()) + + // "password" + // "username" + + // Construct options for TF apply terraformOptions := &terraform.Options{ // The path to where our Terraform code is located TerraformDir: "../../examples/azure/terraform-azure-nsg-example", + Vars: map[string]interface{}{ + "resource_group_name": expectedResourceGroupName, + "vnet_name": expectedVnetName, + "subnet_name": expectedSubnetName, + "vm_nic_name": expectedNICName, + "vm_nic_ip_config_name": expectedIPConfigName, + "nsg_name": expectedNSGName, + "nsg_rule_name": expectedNSGRuleName, + "vm_name": expectedVMName, + "hostname": expectedHostName, + "os_disk_name": expectedOSDiskName, + }, } defer terraform.Destroy(t, terraformOptions) @@ -36,11 +83,11 @@ func TestTerraformAzureNsgExample(t *testing.T) { sshRule := rules.FindRuleByName("allowSSH") // That rule should allow port 22 inbound - assert.True(t, sshRule.AllowsDestinationPort("22")) + assert.True(t, sshRule.AllowsDestinationPort(t, "22")) // But should not allow 80 inbound - assert.False(t, sshRule.AllowsDestinationPort("80")) + assert.False(t, sshRule.AllowsDestinationPort(t, "80")) // SSh is allowed from any port - assert.True(t, sshRule.AllowsSourcePort("*")) + assert.True(t, sshRule.AllowsSourcePort(t, "*")) } From f10d02d8b4caf29eebd6aa8ffff8a83ddec87f54 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 09:48:09 -0400 Subject: [PATCH 14/25] Fixed up unique naming constraints in terraform files; --- .../azure/terraform-azure-nsg-example/main.tf | 37 +++++++---- .../terraform-azure-nsg-example/outputs.tf | 8 +++ .../terraform-azure-nsg-example/variables.tf | 47 +++++++++----- modules/azure/nsg_test.go | 9 --- .../azure/terraform_azure_nsg_example_test.go | 63 +++++-------------- 5 files changed, 81 insertions(+), 83 deletions(-) diff --git a/examples/azure/terraform-azure-nsg-example/main.tf b/examples/azure/terraform-azure-nsg-example/main.tf index a06316a09..62f6b8431 100644 --- a/examples/azure/terraform-azure-nsg-example/main.tf +++ b/examples/azure/terraform-azure-nsg-example/main.tf @@ -18,7 +18,7 @@ terraform { # --------------------------------------------------------------------------------------------------------------------- resource "azurerm_resource_group" "main" { - name = var.resource_group_name + name = "${var.resource_group_name}-${var.postfix}" location = var.location } @@ -27,33 +27,33 @@ resource "azurerm_resource_group" "main" { # --------------------------------------------------------------------------------------------------------------------- resource "azurerm_virtual_network" "main" { - name = var.vnet_name + name = "${var.vnet_name}-${var.postfix}" address_space = ["10.0.0.0/16"] location = azurerm_resource_group.main.location resource_group_name = azurerm_resource_group.main.name } resource "azurerm_subnet" "internal" { - name = var.subnet_name + name = "${var.subnet_name}-${var.postfix}" resource_group_name = azurerm_resource_group.main.name virtual_network_name = azurerm_virtual_network.main.name address_prefixes = ["10.0.17.0/24"] } resource "azurerm_network_interface" "main" { - name = var.vm_nic_name + name = "${var.vm_nic_name}-${var.postfix}" location = azurerm_resource_group.main.location resource_group_name = azurerm_resource_group.main.name ip_configuration { - name = var.vm_nic_ip_config_name + name = "${var.vm_nic_ip_config_name}-${var.postfix}" subnet_id = azurerm_subnet.internal.id private_ip_address_allocation = "Dynamic" } } resource "azurerm_network_security_group" "main" { - name = var.nsg_name + name = "${var.nsg_name}-${var.postfix}" location = azurerm_resource_group.main.location resource_group_name = azurerm_resource_group.main.name } @@ -63,9 +63,9 @@ resource "azurerm_network_interface_security_group_association" "main" { network_security_group_id = azurerm_network_security_group.main.id } -resource "azurerm_network_security_rule" "main" { - name = var.nsg_rule_name - description = var.nsg_rule_name +resource "azurerm_network_security_rule" "allowSSH" { + name = "${var.nsg_ssh_rule_name}-${var.postfix}" + description = "${var.nsg_ssh_rule_name}-${var.postfix}" priority = 100 direction = "Inbound" access = "Allow" @@ -78,13 +78,28 @@ resource "azurerm_network_security_rule" "main" { network_security_group_name = azurerm_network_security_group.main.name } +resource "azurerm_network_security_rule" "blockHTTP" { + name = "${var.nsg_http_rule_name}-${var.postfix}" + description = "${var.nsg_http_rule_name}-${var.postfix}" + priority = 200 + direction = "Inbound" + access = "Deny" + protocol = "Tcp" + source_port_range = "*" + destination_port_range = 80 + source_address_prefix = "*" + destination_address_prefix = "*" + resource_group_name = azurerm_resource_group.main.name + network_security_group_name = azurerm_network_security_group.main.name +} + # --------------------------------------------------------------------------------------------------------------------- # DEPLOY A VIRTUAL MACHINE RUNNING UBUNTU # This VM does not actually do anything and is the smallest size VM available with an Ubuntu image # --------------------------------------------------------------------------------------------------------------------- resource "azurerm_virtual_machine" "main" { - name = var.vm_name + name = "${var.vm_name}-${var.postfix}" location = azurerm_resource_group.main.location resource_group_name = azurerm_resource_group.main.name network_interface_ids = [azurerm_network_interface.main.id] @@ -100,7 +115,7 @@ resource "azurerm_virtual_machine" "main" { } storage_os_disk { - name = var.os_disk_name + name = "${var.os_disk_name}-${var.postfix}" caching = "ReadWrite" create_option = "FromImage" managed_disk_type = "Standard_LRS" diff --git a/examples/azure/terraform-azure-nsg-example/outputs.tf b/examples/azure/terraform-azure-nsg-example/outputs.tf index ae8d7bcd0..dd60ae037 100644 --- a/examples/azure/terraform-azure-nsg-example/outputs.tf +++ b/examples/azure/terraform-azure-nsg-example/outputs.tf @@ -9,3 +9,11 @@ output "resource_group_name" { output "nsg_name" { value = azurerm_network_security_group.main.name } + +output "ssh_rule_name" { + value = azurerm_network_security_rule.allowSSH.name +} + +output "http_rule_name" { + value = azurerm_network_security_rule.blockHTTP.name +} diff --git a/examples/azure/terraform-azure-nsg-example/variables.tf b/examples/azure/terraform-azure-nsg-example/variables.tf index f3a5140c4..67b4f2f36 100644 --- a/examples/azure/terraform-azure-nsg-example/variables.tf +++ b/examples/azure/terraform-azure-nsg-example/variables.tf @@ -18,6 +18,13 @@ # These parameters have reasonable defaults. # --------------------------------------------------------------------------------------------------------------------- +variable "postfix" { + description = "Random postfix string used for each test run; set from the test file at runtime." + type = string + default = "qwefgt" +} + + variable "resource_group_name" { description = "Name for the resource group holding resources for this example" type = string @@ -33,43 +40,49 @@ variable "location" { variable "vnet_name" { description = "Name for the example virtual network" type = string - default = "terratest-nsg-example-network" + default = "vnet01" } variable "subnet_name" { description = "Name for the example virtual network default subnet" type = string - default = "terratest-nsg-example-subnet" + default = "subnet01" } variable "vm_nic_name" { description = "Name for the NIC attached to our example VM." type = string - default = "terratest-nsg-example-nic" + default = "nic01" } variable "vm_nic_ip_config_name" { description = "Name for the NIC IP configuration attached to our example VM." type = string - default = "terratest-nsg-example-nic-ip-config" + default = "nic_ipconfig01" } variable "nsg_name" { description = "Name for the example NSG." type = string - default = "terratest-nsg-example-nsg" + default = "nsg01" } -variable "nsg_rule_name" { - description = "Name for the example NSG rule used in this example." +variable "nsg_ssh_rule_name" { + description = "Name for the example SSH NSG rule used in this example." type = string - default = "terratest-nsg-example-nsg-rule" + default = "nsgrule01" +} + +variable "nsg_http_rule_name" { + description = "Name for the example HTTP NSG rule used in this example." + type = string + default = "nsgrule02" } variable "vm_name" { description = "The name of the VM used in this example" type = string - default = "terratest-nsg-example-vm" + default = "vm01" } variable "vm_size" { @@ -81,19 +94,13 @@ variable "vm_size" { variable "hostname" { description = "The hostname of the new VM to be configured" type = string - default = "terratest-nsg-example-vm" + default = "vm01" } variable "os_disk_name" { description = "The of the OS disk to use on our example VM." type = string - default = "terratest-nsg-example-os-disk" -} - -variable "password" { - description = "The password to configure for SSH access" - type = string - default = "HorriblePassword1234!" + default = "osdisk01" } variable "username" { @@ -101,3 +108,9 @@ variable "username" { type = string default = "testadmin" } + +variable "password" { + description = "The password to configure for SSH access" + type = string + default = "!@#PasswordSetInCode!@#" +} diff --git a/modules/azure/nsg_test.go b/modules/azure/nsg_test.go index f03360580..268385b5b 100644 --- a/modules/azure/nsg_test.go +++ b/modules/azure/nsg_test.go @@ -46,15 +46,6 @@ func TestNsgRuleSummaryConverstion(t *testing.T) { // Quick test to make sure the safe nil handling is working var name = "test name" var sdkStruct = network.SecurityRulePropertiesFormat{} - sdkStruct.Description = nil - sdkStruct.Protocol = network.SecurityRuleProtocolTCP - sdkStruct.SourcePortRange = nil - sdkStruct.DestinationPortRange = nil - sdkStruct.SourceAddressPrefix = nil - sdkStruct.DestinationAddressPrefix = nil - sdkStruct.Access = network.SecurityRuleAccessAllow - sdkStruct.Priority = nil - sdkStruct.Direction = network.SecurityRuleDirectionInbound // Verify the nil values were correctly defaulted to "" without a panic result := convertToNsgRuleSummary(&name, &sdkStruct) diff --git a/test/azure/terraform_azure_nsg_example_test.go b/test/azure/terraform_azure_nsg_example_test.go index 518e3306d..e7fb02bc5 100644 --- a/test/azure/terraform_azure_nsg_example_test.go +++ b/test/azure/terraform_azure_nsg_example_test.go @@ -18,69 +18,34 @@ import ( func TestTerraformAzureNsgExample(t *testing.T) { t.Parallel() - // - // Setup our variables to be unique per test-run: - // - - // "resource_group_name" - expectedResourceGroupName := fmt.Sprintf("terratest-nsg-example-%s", random.UniqueId()) - - // "vnet_name" - // "subnet_name" - expectedVnetName := fmt.Sprintf("vnet_name_%s", random.UniqueId()) - expectedSubnetName := fmt.Sprintf("subnet_name_%s", random.UniqueId()) - - // "vm_nic_name" - // "vm_nic_ip_config_name" - expectedNICName := fmt.Sprintf("vm_nic_name_%s", random.UniqueId()) - expectedIPConfigName := fmt.Sprintf("vm_nic_ip_config_name_%s", random.UniqueId()) - - // "nsg_name" - // "nsg_rule_name" - expectedNSGName := fmt.Sprintf("nsg_name_%s", random.UniqueId()) - expectedNSGRuleName := fmt.Sprintf("nsg_rule_name_%s", random.UniqueId()) - - // "vm_name" - // "hostname" - // "os_disk_name" - expectedVMName := fmt.Sprintf("vm_name_%s", random.UniqueId()) - expectedHostName := fmt.Sprintf("hostname_%s", random.UniqueId()) - expectedOSDiskName := fmt.Sprintf("os_disk_name_%s", random.UniqueId()) - - // "password" - // "username" + randomPostfixValue := random.UniqueId() + vmPassword := fmt.Sprintf("%s@#$%s", random.UniqueId(), random.UniqueId()) // Construct options for TF apply terraformOptions := &terraform.Options{ // The path to where our Terraform code is located TerraformDir: "../../examples/azure/terraform-azure-nsg-example", Vars: map[string]interface{}{ - "resource_group_name": expectedResourceGroupName, - "vnet_name": expectedVnetName, - "subnet_name": expectedSubnetName, - "vm_nic_name": expectedNICName, - "vm_nic_ip_config_name": expectedIPConfigName, - "nsg_name": expectedNSGName, - "nsg_rule_name": expectedNSGRuleName, - "vm_name": expectedVMName, - "hostname": expectedHostName, - "os_disk_name": expectedOSDiskName, + "postfix": randomPostfixValue, + "password": vmPassword, }, } defer terraform.Destroy(t, terraformOptions) terraform.InitAndApply(t, terraformOptions) - nsgName := terraform.Output(t, terraformOptions, "nsg_name") resourceGroupName := terraform.Output(t, terraformOptions, "resource_group_name") + nsgName := terraform.Output(t, terraformOptions, "nsg_name") + sshRuleName := terraform.Output(t, terraformOptions, "ssh_rule_name") + httpRuleName := terraform.Output(t, terraformOptions, "http_rule_name") - // A default NSG has 6 rules, and we have one custom rule for a total of 7 + // A default NSG has 6 rules, and we have two custom rules for a total of 8 rules, err := azure.GetAllNSGRulesE(resourceGroupName, nsgName, "") assert.NoError(t, err) - assert.Equal(t, 7, len(rules.SummarizedRules)) + assert.Equal(t, 8, len(rules.SummarizedRules)) - // We should have a rule named "allowSSH" - sshRule := rules.FindRuleByName("allowSSH") + // We should have a rule for allowing ssh + sshRule := rules.FindRuleByName(sshRuleName) // That rule should allow port 22 inbound assert.True(t, sshRule.AllowsDestinationPort(t, "22")) @@ -90,4 +55,10 @@ func TestTerraformAzureNsgExample(t *testing.T) { // SSh is allowed from any port assert.True(t, sshRule.AllowsSourcePort(t, "*")) + + // We should have a rule for blocking HTTP + httpRule := rules.FindRuleByName(httpRuleName) + + // This rule should BLOCK port 80 inbound + assert.False(t, httpRule.AllowsDestinationPort(t, "80")) } From 630e88795c75beab91ec05ee5de00ddfb777fe18 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 09:51:01 -0400 Subject: [PATCH 15/25] Updated links to core Azure README --- .../terraform-azure-nsg-example/README.md | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/examples/azure/terraform-azure-nsg-example/README.md b/examples/azure/terraform-azure-nsg-example/README.md index ba842947b..86992110a 100644 --- a/examples/azure/terraform-azure-nsg-example/README.md +++ b/examples/azure/terraform-azure-nsg-example/README.md @@ -18,23 +18,22 @@ it should be free, but you are completely responsible for all Azure charges. ## Running this module manually -1. Sign up for [Azure](https://azure.microsoft.com/). +1. Sign up for [Azure](https://azure.microsoft.com/) 1. Configure your Azure credentials using one of the [supported methods for Azure CLI - tools](https://docs.microsoft.com/en-us/cli/azure/azure-cli-configuration?view=azure-cli-latest). -1. Install [Terraform](https://www.terraform.io/) and make sure it's on your `PATH`. -1. Run `terraform init`. -1. Run `terraform apply`. -1. When you're done, run `terraform destroy`. + tools](https://docs.microsoft.com/en-us/cli/azure/azure-cli-configuration?view=azure-cli-latest) +1. Install [Terraform](https://www.terraform.io/) and make sure it's on your `PATH` +1. Ensure [environment variables](../README.md#review-environment-variables) are available +1. Run `terraform init` +1. Run `terraform apply` +1. When you're done, run `terraform destroy` ## Running automated tests against this module -1. Sign up for [Azure](https://azure.microsoft.com/). +1. Sign up for [Azure](https://azure.microsoft.com/) 1. Configure your Azure credentials using one of the [supported methods for Azure CLI - tools](https://docs.microsoft.com/en-us/cli/azure/azure-cli-configuration?view=azure-cli-latest). -1. Install [Terraform](https://www.terraform.io/) and make sure it's on your `PATH`. -1. [Review environment variables](#review-environment-variables). -1. Install [Golang](https://golang.org/) and make sure this code is checked out into your `GOPATH`. + tools](https://docs.microsoft.com/en-us/cli/azure/azure-cli-configuration?view=azure-cli-latest) +1. Install [Terraform](https://www.terraform.io/) and make sure it's on your `PATH` +1. Configure your Terratest [Go test environment](../README.md) 1. `cd test/azure` -1. Make sure [the azure-sdk-for-go versions match](#check-go-dependencies) in [/test/go.mod](/test/go.mod) and in [test/azure/terraform_azure_nsg_example_test.go](/test/terraform_azure_nsg_example_test.go). 1. `go build terraform_azure_nsg_example_test.go` -1. `go test -v -run TestTerraformAzureNsgExample` \ No newline at end of file +1. `go test -v -run TestTerraformAzureNsgExample` From 5610339844f9071ebdf10ac41c2fb03666e23af6 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 10:38:55 -0400 Subject: [PATCH 16/25] Moved safePtrXXX methods to azure common.go file --- modules/azure/common.go | 16 ++++++++++++++++ modules/azure/nsg.go | 16 ---------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/modules/azure/common.go b/modules/azure/common.go index d73923cc5..e664d5a41 100644 --- a/modules/azure/common.go +++ b/modules/azure/common.go @@ -52,3 +52,19 @@ func getTargetAzureResourceGroupName(resourceGroupName string) (string, error) { return resourceGroupName, nil } + +// safePtrToString converts a string pointer to a non-pointer string value, or to "" if the pointer is nil. +func safePtrToString(raw *string) string { + if raw == nil { + return "" + } + return *raw +} + +// safePtrToInt32 converts a int32 pointer to a non-pointer int32 value, or to 0 if the pointer is nil. +func safePtrToInt32(raw *int32) int32 { + if raw == nil { + return 0 + } + return *raw +} diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go index aba925638..3495a56b9 100644 --- a/modules/azure/nsg.go +++ b/modules/azure/nsg.go @@ -152,22 +152,6 @@ func convertToNsgRuleSummary(name *string, rule *network.SecurityRulePropertiesF return summary } -// safePtrToString converts a string pointer to a non-pointer string value, or to "" if the pointer is nil. -func safePtrToString(raw *string) string { - if raw == nil { - return "" - } - return *raw -} - -// safePtrToInt32 converts a int32 pointer to a non-pointer int32 value, or to 0 if the pointer is nil. -func safePtrToInt32(raw *int32) int32 { - if raw == nil { - return 0 - } - return *raw -} - // FindRuleByName looks for a matching rule by name within the current collection of rules. func (summarizedRules *NsgRuleSummaryList) FindRuleByName(name string) NsgRuleSummary { for _, r := range summarizedRules.SummarizedRules { From 8541270d1b23d92eda13d48d8870e90580bb036a Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 10:56:43 -0400 Subject: [PATCH 17/25] Added non-error returning methods to module --- modules/azure/nsg.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go index 3495a56b9..27cf08f97 100644 --- a/modules/azure/nsg.go +++ b/modules/azure/nsg.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-09-01/network" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // NsgRuleSummaryList holds a colleciton of NsgRuleSummary rules @@ -31,6 +32,15 @@ type NsgRuleSummary struct { Direction string } +// GetDefaultNsgRulesClient returns a rules client which can be used to read the list of *default* security rules +// defined on an network security group. Note that the "default" rules are those provided implicitly +// by the Azure platform. +func GetDefaultNsgRulesClient(t *testing.T, subscriptionID string) network.DefaultSecurityRulesClient { + client, err := GetDefaultNsgRulesClientE(subscriptionID) + require.NoError(t, err) + return client +} + // GetDefaultNsgRulesClientE returns a rules client which can be used to read the list of *default* security rules // defined on an network security group. Note that the "default" rules are those provided implicitly // by the Azure platform. @@ -53,6 +63,15 @@ func GetDefaultNsgRulesClientE(subscriptionID string) (network.DefaultSecurityRu return nsgClient, nil } +// GetCustomNsgRulesClient returns a rules client which can be used to read the list of *custom* security rules +// defined on an network security group. Note that the "custom" rules are those defined by +// end users. +func GetCustomNsgRulesClient(t *testing.T, subscriptionID string) network.SecurityRulesClient { + client, err := GetCustomNsgRulesClientE(subscriptionID) + require.NoError(t, err) + return client +} + // GetCustomNsgRulesClientE returns a rules client which can be used to read the list of *custom* security rules // defined on an network security group. Note that the "custom" rules are those defined by // end users. @@ -75,6 +94,14 @@ func GetCustomNsgRulesClientE(subscriptionID string) (network.SecurityRulesClien return nsgClient, nil } +// GetAllNSGRules returns an NsgRuleSummaryList instance containing the combined "default" and "custom" rules from a network +// security group. +func GetAllNSGRules(t *testing.T, resourceGroupName, nsgName, subscriptionID string) NsgRuleSummaryList { + results, err := GetAllNSGRulesE(resourceGroupName, nsgName, subscriptionID) + require.NoError(t, err) + return results +} + // GetAllNSGRulesE returns an NsgRuleSummaryList instance containing the combined "default" and "custom" rules from a network // security group. func GetAllNSGRulesE(resourceGroupName, nsgName, subscriptionID string) (NsgRuleSummaryList, error) { From bd54d5a9b8c3a009cdcf9185e6f1a8e2e2b804ed Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 11:14:04 -0400 Subject: [PATCH 18/25] Added stand-alone helpers for safePtr methods --- modules/azure/common_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/modules/azure/common_test.go b/modules/azure/common_test.go index 403dd01bf..71bc609db 100644 --- a/modules/azure/common_test.go +++ b/modules/azure/common_test.go @@ -9,6 +9,7 @@ import ( "os" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -74,3 +75,27 @@ func TestGetTargetAzureResourceGroupName(t *testing.T) { }) } } + +func TestSafePtrToString(t *testing.T) { + // When given a nil, should always return an empty string + var nilPtr *string = nil + nilResult := safePtrToString(nilPtr) + assert.Equal(t, "", nilResult) + + // When given a string, should just de-ref and return + stringPtr := "Test" + stringResult := safePtrToString(&stringPtr) + assert.Equal(t, "Test", stringResult) +} + +func TestSafePtrToInt32(t *testing.T) { + // When given a nil, should always return an zero value int32 + var nilPtr *int32 = nil + nilResult := safePtrToInt32(nilPtr) + assert.Equal(t, int32(0), nilResult) + + // When given a string, should just de-ref and return + intPtr := int32(42) + intResult := safePtrToInt32(&intPtr) + assert.Equal(t, int32(42), intResult) +} From 34ac108866cd3d41cd5cbc15a3ac49e6107d5419 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 11:32:10 -0400 Subject: [PATCH 19/25] Updated terraform resource names; --- .../azure/terraform-azure-nsg-example/main.tf | 38 +++++++++---------- .../terraform-azure-nsg-example/outputs.tf | 6 +-- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/examples/azure/terraform-azure-nsg-example/main.tf b/examples/azure/terraform-azure-nsg-example/main.tf index 62f6b8431..ca84c1f9b 100644 --- a/examples/azure/terraform-azure-nsg-example/main.tf +++ b/examples/azure/terraform-azure-nsg-example/main.tf @@ -17,7 +17,7 @@ terraform { # See test/terraform_azure_example_test.go for how to write automated tests for this code. # --------------------------------------------------------------------------------------------------------------------- -resource "azurerm_resource_group" "main" { +resource "azurerm_resource_group" "nsg_rg" { name = "${var.resource_group_name}-${var.postfix}" location = var.location } @@ -26,24 +26,24 @@ resource "azurerm_resource_group" "main" { # DEPLOY VIRTUAL NETWORK RESOURCES # --------------------------------------------------------------------------------------------------------------------- -resource "azurerm_virtual_network" "main" { +resource "azurerm_virtual_network" "vnet" { name = "${var.vnet_name}-${var.postfix}" address_space = ["10.0.0.0/16"] - location = azurerm_resource_group.main.location - resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.nsg_rg.location + resource_group_name = azurerm_resource_group.nsg_rg.name } resource "azurerm_subnet" "internal" { name = "${var.subnet_name}-${var.postfix}" - resource_group_name = azurerm_resource_group.main.name - virtual_network_name = azurerm_virtual_network.main.name + resource_group_name = azurerm_resource_group.nsg_rg.name + virtual_network_name = azurerm_virtual_network.vnet.name address_prefixes = ["10.0.17.0/24"] } resource "azurerm_network_interface" "main" { name = "${var.vm_nic_name}-${var.postfix}" - location = azurerm_resource_group.main.location - resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.nsg_rg.location + resource_group_name = azurerm_resource_group.nsg_rg.name ip_configuration { name = "${var.vm_nic_ip_config_name}-${var.postfix}" @@ -52,15 +52,15 @@ resource "azurerm_network_interface" "main" { } } -resource "azurerm_network_security_group" "main" { +resource "azurerm_network_security_group" "nsg_example" { name = "${var.nsg_name}-${var.postfix}" - location = azurerm_resource_group.main.location - resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.nsg_rg.location + resource_group_name = azurerm_resource_group.nsg_rg.name } resource "azurerm_network_interface_security_group_association" "main" { network_interface_id = azurerm_network_interface.main.id - network_security_group_id = azurerm_network_security_group.main.id + network_security_group_id = azurerm_network_security_group.nsg_example.id } resource "azurerm_network_security_rule" "allowSSH" { @@ -74,8 +74,8 @@ resource "azurerm_network_security_rule" "allowSSH" { destination_port_range = 22 source_address_prefix = "*" destination_address_prefix = "*" - resource_group_name = azurerm_resource_group.main.name - network_security_group_name = azurerm_network_security_group.main.name + resource_group_name = azurerm_resource_group.nsg_rg.name + network_security_group_name = azurerm_network_security_group.nsg_example.name } resource "azurerm_network_security_rule" "blockHTTP" { @@ -89,8 +89,8 @@ resource "azurerm_network_security_rule" "blockHTTP" { destination_port_range = 80 source_address_prefix = "*" destination_address_prefix = "*" - resource_group_name = azurerm_resource_group.main.name - network_security_group_name = azurerm_network_security_group.main.name + resource_group_name = azurerm_resource_group.nsg_rg.name + network_security_group_name = azurerm_network_security_group.nsg_example.name } # --------------------------------------------------------------------------------------------------------------------- @@ -98,10 +98,10 @@ resource "azurerm_network_security_rule" "blockHTTP" { # This VM does not actually do anything and is the smallest size VM available with an Ubuntu image # --------------------------------------------------------------------------------------------------------------------- -resource "azurerm_virtual_machine" "main" { +resource "azurerm_virtual_machine" "vm_example" { name = "${var.vm_name}-${var.postfix}" - location = azurerm_resource_group.main.location - resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.nsg_rg.location + resource_group_name = azurerm_resource_group.nsg_rg.name network_interface_ids = [azurerm_network_interface.main.id] vm_size = var.vm_size delete_os_disk_on_termination = true diff --git a/examples/azure/terraform-azure-nsg-example/outputs.tf b/examples/azure/terraform-azure-nsg-example/outputs.tf index dd60ae037..d96abf0be 100644 --- a/examples/azure/terraform-azure-nsg-example/outputs.tf +++ b/examples/azure/terraform-azure-nsg-example/outputs.tf @@ -1,13 +1,13 @@ output "vm_name" { - value = azurerm_virtual_machine.main.name + value = azurerm_virtual_machine.vm_example.name } output "resource_group_name" { - value = azurerm_resource_group.main.name + value = azurerm_resource_group.nsg_rg.name } output "nsg_name" { - value = azurerm_network_security_group.main.name + value = azurerm_network_security_group.nsg_example.name } output "ssh_rule_name" { From d212e9626b021cb2acd1d6741e964d2883a3d9e6 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 11:50:26 -0400 Subject: [PATCH 20/25] Added range testing cases to NsgRuleSummary type --- modules/azure/nsg_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/modules/azure/nsg_test.go b/modules/azure/nsg_test.go index 268385b5b..0e61f2e55 100644 --- a/modules/azure/nsg_test.go +++ b/modules/azure/nsg_test.go @@ -69,6 +69,12 @@ func TestAllowSourcePort(t *testing.T) { {"22 denied", "22", "Deny", "22", false}, {"22 doesn't allow 80", "22", "Allow", "80", false}, {"Any allows any", "*", "Allow", "*", true}, + {"Allows a range of ports", "80-90", "Allow", "80", true}, + {"Allows a range of ports", "80-90", "Allow", "85", true}, + {"Allows a range of ports", "80-90", "Allow", "90", true}, + {"Blocks a range of ports", "80-90", "Deny", "80", false}, + {"Blocks a range of ports", "80-90", "Deny", "85", false}, + {"Blocks a range of ports", "80-90", "Deny", "90", false}, } for _, tt := range cases { @@ -94,6 +100,12 @@ func TestAllowDestinationPort(t *testing.T) { {"22 denied", "22", "Deny", "22", false}, {"22 doesn't allow 80", "22", "Allow", "80", false}, {"Any allows any", "*", "Allow", "*", true}, + {"Allows a range of ports", "80-90", "Allow", "80", true}, + {"Allows a range of ports", "80-90", "Allow", "85", true}, + {"Allows a range of ports", "80-90", "Allow", "90", true}, + {"Blocks a range of ports", "80-90", "Deny", "80", false}, + {"Blocks a range of ports", "80-90", "Deny", "85", false}, + {"Blocks a range of ports", "80-90", "Deny", "90", false}, } for _, tt := range cases { From 968070521e588b20a8eb48aa069e50ec46ae1c6f Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 12:01:03 -0400 Subject: [PATCH 21/25] Updated resource group name per naming guidelines --- examples/azure/terraform-azure-nsg-example/variables.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/azure/terraform-azure-nsg-example/variables.tf b/examples/azure/terraform-azure-nsg-example/variables.tf index 67b4f2f36..f65383766 100644 --- a/examples/azure/terraform-azure-nsg-example/variables.tf +++ b/examples/azure/terraform-azure-nsg-example/variables.tf @@ -28,7 +28,7 @@ variable "postfix" { variable "resource_group_name" { description = "Name for the resource group holding resources for this example" type = string - default = "terratest-nsg-example" + default = "terratest-nsg-rg" } variable "location" { From 48d96c4d39445d5cfb0926aa07a1d1cd74c8eba6 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 12:42:56 -0400 Subject: [PATCH 22/25] Added comment indicated test failure for non-E suffix methods --- modules/azure/nsg.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go index 27cf08f97..c0d6f06a2 100644 --- a/modules/azure/nsg.go +++ b/modules/azure/nsg.go @@ -35,6 +35,7 @@ type NsgRuleSummary struct { // GetDefaultNsgRulesClient returns a rules client which can be used to read the list of *default* security rules // defined on an network security group. Note that the "default" rules are those provided implicitly // by the Azure platform. +// This function would fail the test if there is an error. func GetDefaultNsgRulesClient(t *testing.T, subscriptionID string) network.DefaultSecurityRulesClient { client, err := GetDefaultNsgRulesClientE(subscriptionID) require.NoError(t, err) @@ -66,6 +67,7 @@ func GetDefaultNsgRulesClientE(subscriptionID string) (network.DefaultSecurityRu // GetCustomNsgRulesClient returns a rules client which can be used to read the list of *custom* security rules // defined on an network security group. Note that the "custom" rules are those defined by // end users. +// This function would fail the test if there is an error. func GetCustomNsgRulesClient(t *testing.T, subscriptionID string) network.SecurityRulesClient { client, err := GetCustomNsgRulesClientE(subscriptionID) require.NoError(t, err) @@ -96,6 +98,7 @@ func GetCustomNsgRulesClientE(subscriptionID string) (network.SecurityRulesClien // GetAllNSGRules returns an NsgRuleSummaryList instance containing the combined "default" and "custom" rules from a network // security group. +// This function would fail the test if there is an error. func GetAllNSGRules(t *testing.T, resourceGroupName, nsgName, subscriptionID string) NsgRuleSummaryList { results, err := GetAllNSGRulesE(resourceGroupName, nsgName, subscriptionID) require.NoError(t, err) From 91ea95c9286a2289fb56d42a773603418e5c1cee Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 13:00:13 -0400 Subject: [PATCH 23/25] Fixing formatting issues with main.tf in example folder --- examples/azure/terraform-azure-nsg-example/main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/azure/terraform-azure-nsg-example/main.tf b/examples/azure/terraform-azure-nsg-example/main.tf index ca84c1f9b..d420fec4d 100644 --- a/examples/azure/terraform-azure-nsg-example/main.tf +++ b/examples/azure/terraform-azure-nsg-example/main.tf @@ -37,7 +37,7 @@ resource "azurerm_subnet" "internal" { name = "${var.subnet_name}-${var.postfix}" resource_group_name = azurerm_resource_group.nsg_rg.name virtual_network_name = azurerm_virtual_network.vnet.name - address_prefixes = ["10.0.17.0/24"] + address_prefixes = ["10.0.17.0/24"] } resource "azurerm_network_interface" "main" { From bc29ba0caabbe4d51271dbb10cfc2f510d216904 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Thu, 24 Sep 2020 15:24:51 -0400 Subject: [PATCH 24/25] Added manual depends-on to fix NSG security rule association issue --- examples/azure/terraform-azure-nsg-example/main.tf | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/examples/azure/terraform-azure-nsg-example/main.tf b/examples/azure/terraform-azure-nsg-example/main.tf index d420fec4d..26b1cbf18 100644 --- a/examples/azure/terraform-azure-nsg-example/main.tf +++ b/examples/azure/terraform-azure-nsg-example/main.tf @@ -130,5 +130,10 @@ resource "azurerm_virtual_machine" "vm_example" { os_profile_linux_config { disable_password_authentication = false } + + # Correctly setup the dependencies to make sure resources are correctly destroyed. + depends_on = [ + azurerm_network_interface_security_group_association.main + ] } From 960fdd0a61f1d23341058fe5675453e873ce4d75 Mon Sep 17 00:00:00 2001 From: Mike Yeaney Date: Fri, 25 Sep 2020 17:10:11 -0400 Subject: [PATCH 25/25] Fixes per PR feedback --- .../terraform-azure-nsg-example/README.md | 4 +-- .../azure/terraform-azure-nsg-example/main.tf | 25 ++++++++++++++++--- .../terraform-azure-nsg-example/outputs.tf | 4 +-- .../terraform-azure-nsg-example/variables.tf | 1 - modules/azure/nsg.go | 2 +- modules/azure/nsg_test.go | 6 ++--- .../azure/terraform_azure_nsg_example_test.go | 5 +--- 7 files changed, 30 insertions(+), 17 deletions(-) diff --git a/examples/azure/terraform-azure-nsg-example/README.md b/examples/azure/terraform-azure-nsg-example/README.md index 86992110a..1cee389c3 100644 --- a/examples/azure/terraform-azure-nsg-example/README.md +++ b/examples/azure/terraform-azure-nsg-example/README.md @@ -3,8 +3,8 @@ This folder contains a simple Terraform module that deploys resources in [Azure](https://azure.microsoft.com/) to demonstrate how you can use Terratest to write automated tests for your Azure Terraform code. This module deploys the following: * A [Virtual Machine](https://azure.microsoft.com/en-us/services/virtual-machines/) that gives the module the following: - * [Virtual Machine](https://docs.microsoft.com/en-us/azure/virtual-machines/) with the value specified in the `vm_name` variable. - * A [Network Security Group](https://docs.microsoft.com/en-us/azure/virtual-network/network-security-groups-overview) created with a single custom rule to allow SSH (port 22) with the nsg name specified in the `nsg_name` variable. + * [Virtual Machine](https://docs.microsoft.com/en-us/azure/virtual-machines/) with the value specified in the `vm_name` variable along with a random value for the `postfix` variable (set from test code). + * A [Network Security Group](https://docs.microsoft.com/en-us/azure/virtual-network/network-security-groups-overview) created with a single custom rule to allow SSH (port 22) with the nsg name specified in the `nsg_name` variable along with a random value for the `postfix` variable (set from test code). Check out [test/azure/terraform_azure_nsg_example_test.go](/test/azure/terraform_azure_nsg_example_test.go) to see how you can write automated tests for this module. diff --git a/examples/azure/terraform-azure-nsg-example/main.tf b/examples/azure/terraform-azure-nsg-example/main.tf index 26b1cbf18..04f39841d 100644 --- a/examples/azure/terraform-azure-nsg-example/main.tf +++ b/examples/azure/terraform-azure-nsg-example/main.tf @@ -1,3 +1,11 @@ +# --------------------------------------------------------------------------------------------------------------------- +# DEPLOY AN AZURE VM ALONG WITH AN EXAMPLE NETWORK SECURITY GROUP (NSG) +# This is an example of how to deploy an NSG along with the minimum networking resources +# to support a basic virtual machine. +# --------------------------------------------------------------------------------------------------------------------- +# See test/azure/terraform_azure_nsg_example_test.go for how to write automated tests for this code. +# --------------------------------------------------------------------------------------------------------------------- + provider "azurerm" { version = "~>2.20" features {} @@ -14,7 +22,7 @@ terraform { # --------------------------------------------------------------------------------------------------------------------- # DEPLOY A RESOURCE GROUP -# See test/terraform_azure_example_test.go for how to write automated tests for this code. +# See test/terraform_azure_nsg_example_test.go for how to write automated tests for this code. # --------------------------------------------------------------------------------------------------------------------- resource "azurerm_resource_group" "nsg_rg" { @@ -63,7 +71,7 @@ resource "azurerm_network_interface_security_group_association" "main" { network_security_group_id = azurerm_network_security_group.nsg_example.id } -resource "azurerm_network_security_rule" "allowSSH" { +resource "azurerm_network_security_rule" "allow_ssh" { name = "${var.nsg_ssh_rule_name}-${var.postfix}" description = "${var.nsg_ssh_rule_name}-${var.postfix}" priority = 100 @@ -78,7 +86,7 @@ resource "azurerm_network_security_rule" "allowSSH" { network_security_group_name = azurerm_network_security_group.nsg_example.name } -resource "azurerm_network_security_rule" "blockHTTP" { +resource "azurerm_network_security_rule" "block_http" { name = "${var.nsg_http_rule_name}-${var.postfix}" description = "${var.nsg_http_rule_name}-${var.postfix}" priority = 200 @@ -124,7 +132,7 @@ resource "azurerm_virtual_machine" "vm_example" { os_profile { computer_name = var.hostname admin_username = var.username - admin_password = var.password + admin_password = random_password.nsg.result } os_profile_linux_config { @@ -137,3 +145,12 @@ resource "azurerm_virtual_machine" "vm_example" { ] } +resource "random_password" "nsg" { + length = 16 + override_special = "-_%@" + min_upper = "1" + min_lower = "1" + min_numeric = "1" + min_special = "1" +} + diff --git a/examples/azure/terraform-azure-nsg-example/outputs.tf b/examples/azure/terraform-azure-nsg-example/outputs.tf index d96abf0be..f69279fea 100644 --- a/examples/azure/terraform-azure-nsg-example/outputs.tf +++ b/examples/azure/terraform-azure-nsg-example/outputs.tf @@ -11,9 +11,9 @@ output "nsg_name" { } output "ssh_rule_name" { - value = azurerm_network_security_rule.allowSSH.name + value = azurerm_network_security_rule.allow_ssh.name } output "http_rule_name" { - value = azurerm_network_security_rule.blockHTTP.name + value = azurerm_network_security_rule.block_http.name } diff --git a/examples/azure/terraform-azure-nsg-example/variables.tf b/examples/azure/terraform-azure-nsg-example/variables.tf index f65383766..1f52eb3ec 100644 --- a/examples/azure/terraform-azure-nsg-example/variables.tf +++ b/examples/azure/terraform-azure-nsg-example/variables.tf @@ -24,7 +24,6 @@ variable "postfix" { default = "qwefgt" } - variable "resource_group_name" { description = "Name for the resource group holding resources for this example" type = string diff --git a/modules/azure/nsg.go b/modules/azure/nsg.go index c0d6f06a2..934456337 100644 --- a/modules/azure/nsg.go +++ b/modules/azure/nsg.go @@ -165,7 +165,7 @@ func bindRuleList(source network.SecurityRuleListResultIterator) ([]NsgRuleSumma return rules, nil } -// convertToNsgRuleSummary converst the raw SDK security rule type into a summarized struct, flattening the +// convertToNsgRuleSummary converts the raw SDK security rule type into a summarized struct, flattening the // rules properties and name into a single, string-based struct. func convertToNsgRuleSummary(name *string, rule *network.SecurityRulePropertiesFormat) NsgRuleSummary { summary := NsgRuleSummary{} diff --git a/modules/azure/nsg_test.go b/modules/azure/nsg_test.go index 0e61f2e55..a65d7f065 100644 --- a/modules/azure/nsg_test.go +++ b/modules/azure/nsg_test.go @@ -42,10 +42,10 @@ func TestPortRangeParsing(t *testing.T) { } } -func TestNsgRuleSummaryConverstion(t *testing.T) { +func TestNsgRuleSummaryConversion(t *testing.T) { // Quick test to make sure the safe nil handling is working - var name = "test name" - var sdkStruct = network.SecurityRulePropertiesFormat{} + name := "test name" + sdkStruct := network.SecurityRulePropertiesFormat{} // Verify the nil values were correctly defaulted to "" without a panic result := convertToNsgRuleSummary(&name, &sdkStruct) diff --git a/test/azure/terraform_azure_nsg_example_test.go b/test/azure/terraform_azure_nsg_example_test.go index e7fb02bc5..cb641ba86 100644 --- a/test/azure/terraform_azure_nsg_example_test.go +++ b/test/azure/terraform_azure_nsg_example_test.go @@ -6,7 +6,6 @@ package test import ( - "fmt" "testing" "github.com/gruntwork-io/terratest/modules/azure" @@ -19,15 +18,13 @@ func TestTerraformAzureNsgExample(t *testing.T) { t.Parallel() randomPostfixValue := random.UniqueId() - vmPassword := fmt.Sprintf("%s@#$%s", random.UniqueId(), random.UniqueId()) // Construct options for TF apply terraformOptions := &terraform.Options{ // The path to where our Terraform code is located TerraformDir: "../../examples/azure/terraform-azure-nsg-example", Vars: map[string]interface{}{ - "postfix": randomPostfixValue, - "password": vmPassword, + "postfix": randomPostfixValue, }, }