From c11a444f77641ee2ab5c915c2b96e885b3a0bfb9 Mon Sep 17 00:00:00 2001 From: Miles Crabill Date: Mon, 16 Sep 2019 10:10:37 -0700 Subject: [PATCH 1/3] googlecompute: fail fast when image name is invalid, replace unusable characters w/ -'s --- builder/googlecompute/config.go | 23 ++++++++++++++++++----- builder/googlecompute/template_funcs.go | 2 +- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index add586483..fcead5cfe 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -17,7 +17,8 @@ import ( compute "google.golang.org/api/compute/v1" ) -var reImageFamily = regexp.MustCompile(`^[a-z]([-a-z0-9]{0,61}[a-z0-9])?$`) +// used for ImageName and ImageFamily +var validImageName = regexp.MustCompile(`^[a-z]([-a-z0-9]{0,61}[a-z0-9])?$`) // Config is the configuration structure for the GCE builder. It stores // both the publicly settable state as well as the privately generated @@ -142,17 +143,29 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { } } + // used for ImageName and ImageFamily + imageErrorText := "Invalid image %s: The first character must be a lowercase letter, and all following characters must be a dash, lowercase letter, or digit, except the last character, which cannot be a dash" + + if len(c.ImageName) > 63 { + errs = packer.MultiErrorAppend(errs, + errors.New("Invalid image name: Must not be longer than 63 characters")) + } + + // replaces invalid characters with hyphens + c.ImageName = templateCleanImageName(c.ImageName) + if !validImageName.MatchString(c.ImageName) { + errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf(imageErrorText, "name"))) + } + if len(c.ImageFamily) > 63 { errs = packer.MultiErrorAppend(errs, errors.New("Invalid image family: Must not be longer than 63 characters")) } if c.ImageFamily != "" { - if !reImageFamily.MatchString(c.ImageFamily) { - errs = packer.MultiErrorAppend(errs, - errors.New("Invalid image family: The first character must be a lowercase letter, and all following characters must be a dash, lowercase letter, or digit, except the last character, which cannot be a dash")) + if !validImageName.MatchString(c.ImageFamily) { + errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf(imageErrorText, "family"))) } - } if c.InstanceName == "" { diff --git a/builder/googlecompute/template_funcs.go b/builder/googlecompute/template_funcs.go index 14d1c4b2f..bf57f3d96 100644 --- a/builder/googlecompute/template_funcs.go +++ b/builder/googlecompute/template_funcs.go @@ -20,7 +20,7 @@ func isalphanumeric(b byte) bool { // Clean up image name by replacing invalid characters with "-" // and converting upper cases to lower cases func templateCleanImageName(s string) string { - if reImageFamily.MatchString(s) { + if validImageName.MatchString(s) { return s } b := []byte(strings.ToLower(s)) From 328baced05c8b9d82ec39a5f3eb950cfb6473757 Mon Sep 17 00:00:00 2001 From: Miles Crabill Date: Mon, 16 Sep 2019 10:18:22 -0700 Subject: [PATCH 2/3] add some test cases for image names --- builder/googlecompute/config_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index f247775d6..a884d7f8b 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -156,6 +156,24 @@ func TestConfigPrepare(t *testing.T) { "foo bar", true, }, + { + // underscore will be replaced + "image_name", + "foo_bar", + false, + }, + { + // too long + "image_name", + "foobar123xyz_abc-456-one-two_three_five_nine_seventeen_eleventy-seven", + true, + }, + { + // starts with non-alphabetic character + "image_name", + "1boohoo", + true, + }, { "image_encryption_key", map[string]string{"kmsKeyName": "foo"}, From 2bff60bac8bc695839717afb669a05c6eb487a22 Mon Sep 17 00:00:00 2001 From: Miles Crabill Date: Mon, 16 Sep 2019 13:50:43 -0700 Subject: [PATCH 3/3] address review feedback --- builder/googlecompute/config.go | 8 +++----- builder/googlecompute/config_test.go | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index fcead5cfe..b9d800a45 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -144,17 +144,15 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { } // used for ImageName and ImageFamily - imageErrorText := "Invalid image %s: The first character must be a lowercase letter, and all following characters must be a dash, lowercase letter, or digit, except the last character, which cannot be a dash" + imageErrorText := "Invalid image %s %q: The first character must be a lowercase letter, and all following characters must be a dash, lowercase letter, or digit, except the last character, which cannot be a dash" if len(c.ImageName) > 63 { errs = packer.MultiErrorAppend(errs, errors.New("Invalid image name: Must not be longer than 63 characters")) } - // replaces invalid characters with hyphens - c.ImageName = templateCleanImageName(c.ImageName) if !validImageName.MatchString(c.ImageName) { - errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf(imageErrorText, "name"))) + errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf(imageErrorText, "name", c.ImageName))) } if len(c.ImageFamily) > 63 { @@ -164,7 +162,7 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { if c.ImageFamily != "" { if !validImageName.MatchString(c.ImageFamily) { - errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf(imageErrorText, "family"))) + errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf(imageErrorText, "family", c.ImageFamily))) } } diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index a884d7f8b..b2f05e139 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -157,10 +157,10 @@ func TestConfigPrepare(t *testing.T) { true, }, { - // underscore will be replaced + // underscore is not allowed "image_name", "foo_bar", - false, + true, }, { // too long