From c1e7caf53c2db7d6da8e191d70f181e6aca49633 Mon Sep 17 00:00:00 2001
From: Christopher Boumenot <boumenot@gmail.com>
Date: Wed, 18 May 2016 17:25:57 -0700
Subject: [PATCH] Validate capture variables to obey Azure's rules. (#3537)

---
 builder/azure/arm/config.go                   |  36 +++++-
 builder/azure/arm/config_test.go              | 112 ++++++++++++++++--
 builder/azure/arm/step.go                     |   2 +-
 builder/azure/arm/step_capture_image.go       |   2 +-
 builder/azure/arm/step_capture_image_test.go  |   2 +-
 .../arm/step_create_resource_group_test.go    |   2 +-
 builder/azure/arm/step_delete_os_disk_test.go |   2 +-
 builder/azure/arm/step_deploy_template.go     |   2 +-
 .../azure/arm/step_deploy_template_test.go    |   2 +-
 builder/azure/arm/step_get_certificate.go     |   2 +-
 .../azure/arm/step_get_certificate_test.go    |   2 +-
 builder/azure/arm/step_get_ip_address.go      |   2 +-
 builder/azure/arm/step_get_ip_address_test.go |   2 +-
 builder/azure/arm/step_power_off_compute.go   |   2 +-
 .../azure/arm/step_power_off_compute_test.go  |   2 +-
 builder/azure/arm/step_set_certificate.go     |   2 +-
 .../azure/arm/step_set_certificate_test.go    |   2 +-
 builder/azure/arm/step_test.go                |   2 +-
 builder/azure/arm/step_validate_template.go   |   2 +-
 .../azure/arm/step_validate_template_test.go  |   2 +-
 builder/azure/common/devicelogin.go           |   2 +-
 builder/azure/common/lin/ssh.go               |   2 +-
 22 files changed, 152 insertions(+), 36 deletions(-)

diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go
index dbf8ea29a..d3e899e9b 100644
--- a/builder/azure/arm/config.go
+++ b/builder/azure/arm/config.go
@@ -14,23 +14,24 @@ import (
 	"io/ioutil"
 	"math/big"
 	"net/http"
+	"regexp"
+	"strings"
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/arm/compute"
+	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/Azure/go-ntlmssp"
+
 	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/packer/builder/azure/pkcs12"
-
 	"github.com/mitchellh/packer/common"
 	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/helper/config"
 	"github.com/mitchellh/packer/packer"
 	"github.com/mitchellh/packer/template/interpolate"
 
-	"github.com/Azure/go-autorest/autorest/azure"
 	"golang.org/x/crypto/ssh"
-	"strings"
 )
 
 const (
@@ -40,6 +41,11 @@ const (
 	DefaultVMSize               = "Standard_A1"
 )
 
+var (
+	reCaptureContainerName = regexp.MustCompile("^[a-z0-9][a-z0-9\\-]{2,62}$")
+	reCaptureNamePrefix    = regexp.MustCompile("^[A-Za-z0-9][A-Za-z0-9_\\-\\.]{0,23}$")
+)
+
 type Config struct {
 	common.PackerConfig `mapstructure:",squash"`
 
@@ -399,11 +405,31 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) {
 	/////////////////////////////////////////////
 	// Capture
 	if c.CaptureContainerName == "" {
-		errs = packer.MultiErrorAppend(errs, fmt.Errorf("An capture_container_name must be specified"))
+		errs = packer.MultiErrorAppend(errs, fmt.Errorf("A capture_container_name must be specified"))
+	}
+
+	if !reCaptureContainerName.MatchString(c.CaptureContainerName) {
+		errs = packer.MultiErrorAppend(errs, fmt.Errorf("A capture_container_name must satisfy the regular expression %q.", reCaptureContainerName.String()))
+	}
+
+	if strings.HasSuffix(c.CaptureContainerName, "-") {
+		errs = packer.MultiErrorAppend(errs, fmt.Errorf("A capture_container_name must not end with a hyphen, e.g. '-'."))
+	}
+
+	if strings.Contains(c.CaptureContainerName, "--") {
+		errs = packer.MultiErrorAppend(errs, fmt.Errorf("A capture_container_name must not contain consecutive hyphens, e.g. '--'."))
 	}
 
 	if c.CaptureNamePrefix == "" {
-		errs = packer.MultiErrorAppend(errs, fmt.Errorf("An capture_name_prefix must be specified"))
+		errs = packer.MultiErrorAppend(errs, fmt.Errorf("A capture_name_prefix must be specified"))
+	}
+
+	if !reCaptureNamePrefix.MatchString(c.CaptureNamePrefix) {
+		errs = packer.MultiErrorAppend(errs, fmt.Errorf("A capture_name_prefix must satisfy the regular expression %q.", reCaptureNamePrefix.String()))
+	}
+
+	if strings.HasSuffix(c.CaptureNamePrefix, "-") || strings.HasSuffix(c.CaptureNamePrefix, ".") {
+		errs = packer.MultiErrorAppend(errs, fmt.Errorf("A capture_name_prefix must not end with a hyphen or period."))
 	}
 
 	/////////////////////////////////////////////
diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go
index 2708e9e2a..b5e86f728 100644
--- a/builder/azure/arm/config_test.go
+++ b/builder/azure/arm/config_test.go
@@ -4,7 +4,6 @@
 package arm
 
 import (
-	"encoding/json"
 	"fmt"
 	"strings"
 	"testing"
@@ -401,10 +400,107 @@ func TestUseDeviceLoginIsDisabledForWindows(t *testing.T) {
 	}
 }
 
+func TestConfigShouldRejectMalformedCaptureNamePrefix(t *testing.T) {
+	config := map[string]string{
+		"capture_container_name": "ignore",
+		"image_offer":            "ignore",
+		"image_publisher":        "ignore",
+		"image_sku":              "ignore",
+		"location":               "ignore",
+		"storage_account":        "ignore",
+		"subscription_id":        "ignore",
+		// Does not matter for this test case, just pick one.
+		"os_type": constants.Target_Linux,
+	}
+
+	wellFormedCaptureNamePrefix := []string{
+		"packer",
+		"AbcdefghijklmnopqrstuvwX",
+		"hypen-hypen",
+		"0leading-number",
+		"v1.core.local",
+	}
+
+	for _, x := range wellFormedCaptureNamePrefix {
+		config["capture_name_prefix"] = x
+		_, _, err := newConfig(config, getPackerConfiguration())
+
+		if err != nil {
+			t.Errorf("Expected test to pass, but it failed with the well-formed capture_name_prefix set to %q.", x)
+		}
+	}
+
+	malformedCaptureNamePrefix := []string{
+		"-leading-hypen",
+		"trailing-hypen-",
+		"trailing-period.",
+		"_leading-underscore",
+		"punc-!@#$%^&*()_+-=-punc",
+		"There-are-too-many-characters-in-the-name-and-the-limit-is-twenty-four",
+	}
+
+	for _, x := range malformedCaptureNamePrefix {
+		config["capture_name_prefix"] = x
+		_, _, err := newConfig(config, getPackerConfiguration())
+
+		if err == nil {
+			t.Errorf("Expected test to fail, but it succeeded with the malformed capture_name_prefix set to %q.", x)
+		}
+	}
+}
+
+func TestConfigShouldRejectMalformedCaptureContainerName(t *testing.T) {
+	config := map[string]string{
+		"capture_name_prefix": "ignore",
+		"image_offer":         "ignore",
+		"image_publisher":     "ignore",
+		"image_sku":           "ignore",
+		"location":            "ignore",
+		"storage_account":     "ignore",
+		"subscription_id":     "ignore",
+		// Does not matter for this test case, just pick one.
+		"os_type": constants.Target_Linux,
+	}
+
+	wellFormedCaptureContainerName := []string{
+		"0leading",
+		"aleading",
+		"hype-hypen",
+		"abcdefghijklmnopqrstuvwxyz0123456789-abcdefghijklmnopqrstuvwxyz", // 63 characters
+	}
+
+	for _, x := range wellFormedCaptureContainerName {
+		config["capture_container_name"] = x
+		_, _, err := newConfig(config, getPackerConfiguration())
+
+		if err != nil {
+			t.Errorf("Expected test to pass, but it failed with the well-formed capture_container_name set to %q.", x)
+		}
+	}
+
+	malformedCaptureContainerName := []string{
+		"No-Capitals",
+		"double--hypens",
+		"-leading-hypen",
+		"trailing-hypen-",
+		"punc-!@#$%^&*()_+-=-punc",
+		"there-are-over-63-characters-in-this-string-and-that-is-a-bad-container-name",
+	}
+
+	for _, x := range malformedCaptureContainerName {
+		config["capture_container_name"] = x
+		_, _, err := newConfig(config, getPackerConfiguration())
+
+		if err == nil {
+			t.Errorf("Expected test to fail, but it succeeded with the malformed capture_container_name set to %q.", x)
+		}
+	}
+}
+
 func getArmBuilderConfiguration() map[string]string {
 	m := make(map[string]string)
 	for _, v := range requiredConfigValues {
-		m[v] = fmt.Sprintf("%s00", v)
+		m[v] = fmt.Sprintf("ignored00")
 	}
 
 	m["communicator"] = "none"
@@ -413,19 +509,13 @@ func getArmBuilderConfiguration() map[string]string {
 }
 
 func getPackerConfiguration() interface{} {
-	var doc = `{
-		"packer_user_variables": {
-			"sa": "my_storage_account"
-		},
+	config := map[string]interface{}{
 		"packer_build_name": "azure-arm-vm",
 		"packer_builder_type": "azure-arm-vm",
 		"packer_debug": "false",
 		"packer_force": "false",
-		"packer_template_path": "/home/jenkins/azure-arm-vm/template.json"
-	}`
-
-	var config interface{}
-	json.Unmarshal([]byte(doc), &config)
+		"packer_template_path": "/home/jenkins/azure-arm-vm/template.json",
+	}
 
 	return config
 }
diff --git a/builder/azure/arm/step.go b/builder/azure/arm/step.go
index 4b14ba459..53da60c3d 100644
--- a/builder/azure/arm/step.go
+++ b/builder/azure/arm/step.go
@@ -4,9 +4,9 @@
 package arm
 
 import (
+	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/builder/azure/common"
 	"github.com/mitchellh/packer/builder/azure/common/constants"
-	"github.com/mitchellh/multistep"
 )
 
 func processInterruptibleResult(
diff --git a/builder/azure/arm/step_capture_image.go b/builder/azure/arm/step_capture_image.go
index f47ebaf3f..e1ab17424 100644
--- a/builder/azure/arm/step_capture_image.go
+++ b/builder/azure/arm/step_capture_image.go
@@ -7,9 +7,9 @@ import (
 	"fmt"
 
 	"github.com/Azure/azure-sdk-for-go/arm/compute"
+	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/builder/azure/common"
 	"github.com/mitchellh/packer/builder/azure/common/constants"
-	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
 )
 
diff --git a/builder/azure/arm/step_capture_image_test.go b/builder/azure/arm/step_capture_image_test.go
index 13040ddb3..66a18c0df 100644
--- a/builder/azure/arm/step_capture_image_test.go
+++ b/builder/azure/arm/step_capture_image_test.go
@@ -8,8 +8,8 @@ import (
 	"testing"
 
 	"github.com/Azure/azure-sdk-for-go/arm/compute"
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 )
 
 func TestStepCaptureImageShouldFailIfCaptureFails(t *testing.T) {
diff --git a/builder/azure/arm/step_create_resource_group_test.go b/builder/azure/arm/step_create_resource_group_test.go
index ba77cad10..02cdac66a 100644
--- a/builder/azure/arm/step_create_resource_group_test.go
+++ b/builder/azure/arm/step_create_resource_group_test.go
@@ -7,8 +7,8 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 )
 
 func TestStepCreateResourceGroupShouldFailIfCreateFails(t *testing.T) {
diff --git a/builder/azure/arm/step_delete_os_disk_test.go b/builder/azure/arm/step_delete_os_disk_test.go
index bb918c6f7..2355bf283 100644
--- a/builder/azure/arm/step_delete_os_disk_test.go
+++ b/builder/azure/arm/step_delete_os_disk_test.go
@@ -7,8 +7,8 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 )
 
 func TestStepDeleteOSDiskShouldFailIfGetFails(t *testing.T) {
diff --git a/builder/azure/arm/step_deploy_template.go b/builder/azure/arm/step_deploy_template.go
index 356558926..6f74cb4db 100644
--- a/builder/azure/arm/step_deploy_template.go
+++ b/builder/azure/arm/step_deploy_template.go
@@ -6,9 +6,9 @@ package arm
 import (
 	"fmt"
 
+	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/builder/azure/common"
 	"github.com/mitchellh/packer/builder/azure/common/constants"
-	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
 )
 
diff --git a/builder/azure/arm/step_deploy_template_test.go b/builder/azure/arm/step_deploy_template_test.go
index f3be502cb..3a4f5b770 100644
--- a/builder/azure/arm/step_deploy_template_test.go
+++ b/builder/azure/arm/step_deploy_template_test.go
@@ -7,8 +7,8 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 )
 
 func TestStepDeployTemplateShouldFailIfDeployFails(t *testing.T) {
diff --git a/builder/azure/arm/step_get_certificate.go b/builder/azure/arm/step_get_certificate.go
index 7af7b2feb..22403ea94 100644
--- a/builder/azure/arm/step_get_certificate.go
+++ b/builder/azure/arm/step_get_certificate.go
@@ -7,8 +7,8 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/packer/packer"
 )
 
diff --git a/builder/azure/arm/step_get_certificate_test.go b/builder/azure/arm/step_get_certificate_test.go
index 57072404c..ebeb5227c 100644
--- a/builder/azure/arm/step_get_certificate_test.go
+++ b/builder/azure/arm/step_get_certificate_test.go
@@ -7,8 +7,8 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 )
 
 func TestStepGetCertificateShouldFailIfGetFails(t *testing.T) {
diff --git a/builder/azure/arm/step_get_ip_address.go b/builder/azure/arm/step_get_ip_address.go
index 0b5f7c510..4b664c4ed 100644
--- a/builder/azure/arm/step_get_ip_address.go
+++ b/builder/azure/arm/step_get_ip_address.go
@@ -6,8 +6,8 @@ package arm
 import (
 	"fmt"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/packer/packer"
 )
 
diff --git a/builder/azure/arm/step_get_ip_address_test.go b/builder/azure/arm/step_get_ip_address_test.go
index bd826c29e..3240083c6 100644
--- a/builder/azure/arm/step_get_ip_address_test.go
+++ b/builder/azure/arm/step_get_ip_address_test.go
@@ -7,8 +7,8 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 )
 
 func TestStepGetIPAddressShouldFailIfGetFails(t *testing.T) {
diff --git a/builder/azure/arm/step_power_off_compute.go b/builder/azure/arm/step_power_off_compute.go
index b48c7b96c..bb702d0c7 100644
--- a/builder/azure/arm/step_power_off_compute.go
+++ b/builder/azure/arm/step_power_off_compute.go
@@ -6,9 +6,9 @@ package arm
 import (
 	"fmt"
 
+	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/builder/azure/common"
 	"github.com/mitchellh/packer/builder/azure/common/constants"
-	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
 )
 
diff --git a/builder/azure/arm/step_power_off_compute_test.go b/builder/azure/arm/step_power_off_compute_test.go
index d305fe4ab..a3ceaa7dd 100644
--- a/builder/azure/arm/step_power_off_compute_test.go
+++ b/builder/azure/arm/step_power_off_compute_test.go
@@ -7,8 +7,8 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 )
 
 func TestStepPowerOffComputeShouldFailIfPowerOffFails(t *testing.T) {
diff --git a/builder/azure/arm/step_set_certificate.go b/builder/azure/arm/step_set_certificate.go
index 7c7ede26f..9be462ce0 100644
--- a/builder/azure/arm/step_set_certificate.go
+++ b/builder/azure/arm/step_set_certificate.go
@@ -4,8 +4,8 @@
 package arm
 
 import (
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/packer/packer"
 )
 
diff --git a/builder/azure/arm/step_set_certificate_test.go b/builder/azure/arm/step_set_certificate_test.go
index 185b24ed4..819d64be5 100644
--- a/builder/azure/arm/step_set_certificate_test.go
+++ b/builder/azure/arm/step_set_certificate_test.go
@@ -6,8 +6,8 @@ package arm
 import (
 	"testing"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 )
 
 func TestStepSetCertificateShouldPassIfGetPasses(t *testing.T) {
diff --git a/builder/azure/arm/step_test.go b/builder/azure/arm/step_test.go
index f13edb545..f0c365bf1 100644
--- a/builder/azure/arm/step_test.go
+++ b/builder/azure/arm/step_test.go
@@ -5,9 +5,9 @@ package arm
 
 import (
 	"fmt"
+	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/builder/azure/common"
 	"github.com/mitchellh/packer/builder/azure/common/constants"
-	"github.com/mitchellh/multistep"
 	"testing"
 )
 
diff --git a/builder/azure/arm/step_validate_template.go b/builder/azure/arm/step_validate_template.go
index 08d709875..66d55e4e9 100644
--- a/builder/azure/arm/step_validate_template.go
+++ b/builder/azure/arm/step_validate_template.go
@@ -6,8 +6,8 @@ package arm
 import (
 	"fmt"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/packer/packer"
 )
 
diff --git a/builder/azure/arm/step_validate_template_test.go b/builder/azure/arm/step_validate_template_test.go
index 88048bd3d..7b874d4e7 100644
--- a/builder/azure/arm/step_validate_template_test.go
+++ b/builder/azure/arm/step_validate_template_test.go
@@ -7,8 +7,8 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 )
 
 func TestStepValidateTemplateShouldFailIfValidateFails(t *testing.T) {
diff --git a/builder/azure/common/devicelogin.go b/builder/azure/common/devicelogin.go
index 37725894c..f6632a99e 100644
--- a/builder/azure/common/devicelogin.go
+++ b/builder/azure/common/devicelogin.go
@@ -11,8 +11,8 @@ import (
 	"github.com/Azure/go-autorest/autorest"
 	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/Azure/go-autorest/autorest/to"
-	"github.com/mitchellh/packer/version"
 	"github.com/mitchellh/go-homedir"
+	"github.com/mitchellh/packer/version"
 )
 
 var (
diff --git a/builder/azure/common/lin/ssh.go b/builder/azure/common/lin/ssh.go
index af86b5f97..a869978fe 100644
--- a/builder/azure/common/lin/ssh.go
+++ b/builder/azure/common/lin/ssh.go
@@ -5,8 +5,8 @@ package lin
 
 import (
 	"fmt"
-	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/builder/azure/common/constants"
 	"golang.org/x/crypto/ssh"
 )