Merge pull request #5599 from boumenot/pr-azure-validate-rgn

azure: sanity check resource group names

builder/azure/arm/config.go

@@ -38,9 +38,23 @@ const (
 	DefaultVMSize                            = "Standard_A1"
 )
 
+const (
+	// https://docs.microsoft.com/en-us/azure/architecture/best-practices/naming-conventions#naming-rules-and-restrictions
+	// Regular expressions in Go are not expressive enough, such that the regular expression returned by Azure
+	// can be used (no backtracking).
+	//
+	//  -> ^[^_\W][\w-._]{0,79}(?<![-.])$
+	//
+	// This is not an exhaustive match, but it should be extremely close.
+	validResourceGroupNameRe = "^[^_\\W][\\w-._\\(\\)]{0,63}$"
+	validManagedDiskName     = "^[^_\\W][\\w-._)]{0,79}$"
+)
+
 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}$")
+	reManagedDiskName      = regexp.MustCompile(validManagedDiskName)
+	reResourceGroupName    = regexp.MustCompile(validResourceGroupNameRe)
 )
 
 type Config struct {
@@ -599,6 +613,30 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) {
 		}
 	}
 
+	if c.TempResourceGroupName != "" {
+		if ok, err := assertResourceGroupName(c.TempResourceGroupName, "temp_resource_group_name"); !ok {
+			errs = packer.MultiErrorAppend(errs, err)
+		}
+	}
+
+	if c.BuildResourceGroupName != "" {
+		if ok, err := assertResourceGroupName(c.BuildResourceGroupName, "build_resource_group_name"); !ok {
+			errs = packer.MultiErrorAppend(errs, err)
+		}
+	}
+
+	if c.ManagedImageResourceGroupName != "" {
+		if ok, err := assertResourceGroupName(c.ManagedImageResourceGroupName, "managed_image_resource_group_name"); !ok {
+			errs = packer.MultiErrorAppend(errs, err)
+		}
+	}
+
+	if c.ManagedImageName != "" {
+		if ok, err := assertManagedImageName(c.ManagedImageName, "managed_image_name"); !ok {
+			errs = packer.MultiErrorAppend(errs, err)
+		}
+	}
+
 	if c.VirtualNetworkName == "" && c.VirtualNetworkResourceGroupName != "" {
 		errs = packer.MultiErrorAppend(errs, fmt.Errorf("If virtual_network_resource_group_name is specified, so must virtual_network_name"))
 	}
@@ -627,3 +665,23 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) {
 		errs = packer.MultiErrorAppend(errs, fmt.Errorf("The managed_image_storage_account_type %q is invalid", c.ManagedImageStorageAccountType))
 	}
 }
+
+func assertManagedImageName(name, setting string) (bool, error) {
+	if !isValidAzureName(reManagedDiskName, name) {
+		return false, fmt.Errorf("The setting %s must match the regular expression %q, and not end with a '-' or '.'.", setting, validManagedDiskName)
+	}
+	return true, nil
+}
+
+func assertResourceGroupName(rgn, setting string) (bool, error) {
+	if !isValidAzureName(reResourceGroupName, rgn) {
+		return false, fmt.Errorf("The setting %s must match the regular expression %q, and not end with a '-' or '.'.", setting, validResourceGroupNameRe)
+	}
+	return true, nil
+}
+
+func isValidAzureName(re *regexp.Regexp, rgn string) bool {
+	return re.Match([]byte(rgn)) &&
+		!strings.HasSuffix(rgn, ".") &&
+		!strings.HasSuffix(rgn, "-")
+}

builder/azure/arm/config_test.go

@@ -923,6 +923,139 @@ func TestConfigShouldRejectTempAndBuildResourceGroupName(t *testing.T) {
 	}
 }
 
+func TestConfigShouldRejectInvalidResourceGroupNames(t *testing.T) {
+	config := map[string]interface{}{
+		"capture_name_prefix":    "ignore",
+		"capture_container_name": "ignore",
+		"image_offer":            "ignore",
+		"image_publisher":        "ignore",
+		"image_sku":              "ignore",
+		"location":               "ignore",
+		"storage_account":        "ignore",
+		"resource_group_name":    "ignore",
+		"subscription_id":        "ignore",
+		"communicator":           "none",
+		"os_type":                "linux",
+	}
+
+	tests := []struct {
+		name string
+		ok   bool
+	}{
+		// The Good
+		{"packer-Resource-Group-jt2j3fc", true},
+		{"My", true},
+		{"My-(with-parens)-Resource-Group", true},
+
+		// The Bad
+		{"My Resource Group", false},
+		{"My-Resource-Group-", false},
+		{"My.Resource.Group.", false},
+
+		// The Ugly
+		{"My!@#!@#%$%yM", false},
+		{"   ", false},
+		{"My10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", false},
+	}
+
+	settings := []string{"temp_resource_group_name", "build_resource_group_name"}
+
+	for _, x := range settings {
+		for _, y := range tests {
+			config[x] = y.name
+
+			_, _, err := newConfig(config, getPackerConfiguration())
+			if !y.ok && err == nil {
+				t.Errorf("expected config to reject %q for setting %q", y.name, x)
+			} else if y.ok && err != nil {
+				t.Errorf("expected config to accept %q for setting %q", y.name, x)
+			}
+		}
+
+		delete(config, x)
+	}
+}
+
+func TestConfigShouldRejectManagedDiskNames(t *testing.T) {
+	config := map[string]interface{}{
+		"image_offer":                       "ignore",
+		"image_publisher":                   "ignore",
+		"image_sku":                         "ignore",
+		"location":                          "ignore",
+		"subscription_id":                   "ignore",
+		"communicator":                      "none",
+		"os_type":                           "linux",
+		"managed_image_name":                "ignore",
+		"managed_image_resource_group_name": "ignore",
+	}
+
+	testsResourceGroupNames := []struct {
+		name string
+		ok   bool
+	}{
+		// The Good
+		{"packer-Resource-Group-jt2j3fc", true},
+		{"My", true},
+		{"My-(with-parens)-Resource-Group", true},
+
+		// The Bad
+		{"My Resource Group", false},
+		{"My-Resource-Group-", false},
+		{"My.Resource.Group.", false},
+
+		// The Ugly
+		{"My!@#!@#%$%yM", false},
+		{"   ", false},
+		{"My10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", false},
+	}
+
+	settingUnderTest := "managed_image_resource_group_name"
+	for _, y := range testsResourceGroupNames {
+		config[settingUnderTest] = y.name
+
+		_, _, err := newConfig(config, getPackerConfiguration())
+		if !y.ok && err == nil {
+			t.Errorf("expected config to reject %q for setting %q", y.name, settingUnderTest)
+		} else if y.ok && err != nil {
+			t.Errorf("expected config to accept %q for setting %q", y.name, settingUnderTest)
+		}
+	}
+
+	config["managed_image_resource_group_name"] = "ignored"
+
+	testNames := []struct {
+		name string
+		ok   bool
+	}{
+		// The Good
+		{"ManagedDiskName", true},
+		{"Managed-Disk-Name", true},
+		{"My33", true},
+
+		// The Bad
+		{"Managed Disk Name", false},
+		{"Managed-Disk-Name-", false},
+		{"Managed.Disk.Name.", false},
+
+		// The Ugly
+		{"My!@#!@#%$%yM", false},
+		{"   ", false},
+		{"My10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", false},
+	}
+
+	settingUnderTest = "managed_image_name"
+	for _, y := range testNames {
+		config[settingUnderTest] = y.name
+
+		_, _, err := newConfig(config, getPackerConfiguration())
+		if !y.ok && err == nil {
+			t.Logf("expected config to reject %q for setting %q", y.name, settingUnderTest)
+		} else if y.ok && err != nil {
+			t.Logf("expected config to accept %q for setting %q", y.name, settingUnderTest)
+		}
+	}
+}
+
 func getArmBuilderConfiguration() map[string]string {
 	m := make(map[string]string)
 	for _, v := range requiredConfigValues {