Validate capture variables to obey Azure's rules. (#3537)

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."))
 	}
 
 	/////////////////////////////////////////////

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 = `{
+	config := map[string]interface{}{
-		"packer_user_variables": {
-			"sa": "my_storage_account"
-		},
 		"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"
+		"packer_template_path": "/home/jenkins/azure-arm-vm/template.json",
-	}`
+	}
-
-	var config interface{}
-	json.Unmarshal([]byte(doc), &config)
 
 	return config
 }

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(

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"
 )
 

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) {

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) {

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) {

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"
 )
 

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) {

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"
 )
 

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) {

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"
 )
 

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) {

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"
 )
 

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) {

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"
 )
 

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) {

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"
 )
 

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"
 )
 

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) {

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 (

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"
 )