fix: added gcs name validation (#10426) (#17858)

[upstream:8e02735411039856ec1dc1f0f36471c268ec857f]

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

.changelog/10426.txt

@@ -0,0 +1,3 @@
+```release-note: bug
+storage: added validation to `name` field in `google_storage_bucket` resource
+```

google/services/storage/resource_storage_bucket.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/hashicorp/terraform-provider-google/google/tpgresource"
 	transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport"
+	"github.com/hashicorp/terraform-provider-google/google/verify"
 
 	"github.com/gammazero/workerpool"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
@@ -59,10 +60,11 @@ func ResourceStorageBucket() *schema.Resource {
 
 		Schema: map[string]*schema.Schema{
 			"name": {
-				Type:        schema.TypeString,
-				Required:    true,
-				ForceNew:    true,
-				Description: `The name of the bucket.`,
+				Type:         schema.TypeString,
+				Required:     true,
+				ForceNew:     true,
+				Description:  `The name of the bucket.`,
+				ValidateFunc: verify.ValidateGCSName,
 			},
 
 			"encryption": {
@@ -579,9 +581,6 @@ func resourceStorageBucketCreate(d *schema.ResourceData, meta interface{}) error
 
 	// Get the bucket and location
 	bucket := d.Get("name").(string)
-	if err := tpgresource.CheckGCSName(bucket); err != nil {
-		return err
-	}
 	location := d.Get("location").(string)
 
 	// Create a bucket, setting the labels, location and name.

google/tpgresource/utils.go

@@ -617,31 +617,6 @@ func Fake404(reasonResourceType, resourceName string) *googleapi.Error {
 	}
 }
 
-// validate name of the gcs bucket. Guidelines are located at https://cloud.google.com/storage/docs/naming-buckets
-// this does not attempt to check for IP addresses or close misspellings of "google"
-func CheckGCSName(name string) error {
-	if strings.HasPrefix(name, "goog") {
-		return fmt.Errorf("error: bucket name %s cannot start with %q", name, "goog")
-	}
-
-	if strings.Contains(name, "google") {
-		return fmt.Errorf("error: bucket name %s cannot contain %q", name, "google")
-	}
-
-	valid, _ := regexp.MatchString("^[a-z0-9][a-z0-9_.-]{1,220}[a-z0-9]$", name)
-	if !valid {
-		return fmt.Errorf("error: bucket name validation failed %v. See https://cloud.google.com/storage/docs/naming-buckets", name)
-	}
-
-	for _, str := range strings.Split(name, ".") {
-		valid, _ := regexp.MatchString("^[a-z0-9_-]{1,63}$", str)
-		if !valid {
-			return fmt.Errorf("error: bucket name validation failed %v", str)
-		}
-	}
-	return nil
-}
-
 // CheckGoogleIamPolicy makes assertions about the contents of a google_iam_policy data source's policy_data attribute
 func CheckGoogleIamPolicy(value string) error {
 	if strings.Contains(value, "\"description\":\"\"") {

google/tpgresource/utils_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 
-	"github.com/hashicorp/terraform-provider-google/google/acctest"
 	"github.com/hashicorp/terraform-provider-google/google/tpgresource"
 	transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport"
 
@@ -1256,44 +1255,3 @@ func TestReplaceVars(t *testing.T) {
 		})
 	}
 }
-
-func TestCheckGCSName(t *testing.T) {
-	valid63 := acctest.RandString(t, 63)
-	cases := map[string]bool{
-		// Valid
-		"foobar":       true,
-		"foobar1":      true,
-		"12345":        true,
-		"foo_bar_baz":  true,
-		"foo-bar-baz":  true,
-		"foo-bar_baz1": true,
-		"foo--bar":     true,
-		"foo__bar":     true,
-		"foo-goog":     true,
-		"foo.goog":     true,
-		valid63:        true,
-		fmt.Sprintf("%s.%s.%s", valid63, valid63, valid63): true,
-
-		// Invalid
-		"goog-foobar":             false,
-		"foobar-google":           false,
-		"-foobar":                 false,
-		"foobar-":                 false,
-		"_foobar":                 false,
-		"foobar_":                 false,
-		"fo":                      false,
-		"foo$bar":                 false,
-		"foo..bar":                false,
-		acctest.RandString(t, 64): false,
-		fmt.Sprintf("%s.%s.%s.%s", valid63, valid63, valid63, valid63): false,
-	}
-
-	for bucketName, valid := range cases {
-		err := tpgresource.CheckGCSName(bucketName)
-		if valid && err != nil {
-			t.Errorf("The bucket name %s was expected to pass validation and did not pass.", bucketName)
-		} else if !valid && err == nil {
-			t.Errorf("The bucket name %s was NOT expected to pass validation and passed.", bucketName)
-		}
-	}
-}

google/verify/validation.go

@@ -76,6 +76,16 @@ var (
 	Rfc6996Asn32BitMin  = int64(4200000000)
 	Rfc6996Asn32BitMax  = int64(4294967294)
 	GcpRouterPartnerAsn = int64(16550)
+
+	// Format of GCS Bucket Name
+	// https://cloud.google.com/storage/docs/naming-buckets
+	GCSNameValidChars     = "^[a-z0-9_.-]*$"
+	GCSNameStartEndChars  = "^[a-z|0-9].*[a-z|0-9]$"
+	GCSNameLength         = "^.{3,222}"
+	GCSNameLengthSplit    = "^.{1,63}$"
+	GCSNameCidr           = "^[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}$"
+	GCSNameGoogPrefix     = "^goog.*$"
+	GCSNameContainsGoogle = "^.*google.*$"
 )
 
 var Rfc1918Networks = []string{
@@ -91,6 +101,44 @@ func ValidateGCEName(v interface{}, k string) (ws []string, errors []error) {
 	return ValidateRegexp(re)(v, k)
 }
 
+// validateGCSName ensures the name of a gcs bucket matches the requirements for GCS Buckets
+// https://cloud.google.com/storage/docs/naming-buckets
+func ValidateGCSName(v interface{}, k string) (ws []string, errors []error) {
+	value := v.(string)
+
+	if !regexp.MustCompile(GCSNameValidChars).MatchString(value) {
+		errors = append(errors, fmt.Errorf("%q name value can only contain lowercase letters, numeric characters, dashes (-), underscores (_), and dots (.)", value))
+	}
+
+	if !regexp.MustCompile(GCSNameStartEndChars).MatchString(value) {
+		errors = append(errors, fmt.Errorf("%q name value must start and end with a number or letter", value))
+	}
+
+	if !regexp.MustCompile(GCSNameLength).MatchString(value) {
+		errors = append(errors, fmt.Errorf("%q name value must contain 3-63 characters. Names containing dots can contain up to 222 characters, but each dot-separated component can be no longer than 63 characters", value))
+	}
+
+	for _, str := range strings.Split(value, ".") {
+		if !regexp.MustCompile(GCSNameLengthSplit).MatchString(str) {
+			errors = append(errors, fmt.Errorf("%q name value must contain 3-63 characters. Names containing dots can contain up to 222 characters, but each dot-separated component can be no longer than 63 characters", value))
+		}
+	}
+
+	if regexp.MustCompile(GCSNameCidr).MatchString(value) {
+		errors = append(errors, fmt.Errorf("%q name value cannot be represented as an IP address in dotted-decimal notation (for example, 192.168.5.4)", value))
+	}
+
+	if regexp.MustCompile(GCSNameGoogPrefix).MatchString(value) {
+		errors = append(errors, fmt.Errorf("%q name value cannot begin with the \"goog\" prefix", value))
+	}
+
+	if regexp.MustCompile(GCSNameContainsGoogle).MatchString(strings.ReplaceAll(value, "0", "o")) {
+		errors = append(errors, fmt.Errorf("%q name value cannot contain \"google\" or close misspellings, such as \"g00gle\"", value))
+	}
+
+	return
+}
+
 // Ensure that the BGP ASN value of Cloud Router is a valid value as per RFC6996 or a value of 16550
 func ValidateRFC6996Asn(v interface{}, k string) (ws []string, errors []error) {
 	value := int64(v.(int))

google/verify/validation_test.go

@@ -319,3 +319,43 @@ func TestValidateIAMCustomRoleIDRegex(t *testing.T) {
 		t.Errorf("Failed to validate IAMCustomRole IDs: %v", es)
 	}
 }
+
+func TestValidateGCSName(t *testing.T) {
+	x := []StringValidationTestCase{
+		// No errors
+		{TestName: "basic", Value: "foobar"},
+		{TestName: "has number", Value: "foobar1"},
+		{TestName: "all numbers", Value: "12345"},
+		{TestName: "all _", Value: "foo_bar_baz"},
+		{TestName: "all -", Value: "foo-bar-baz"},
+		{TestName: "begins with number", Value: "1foo-bar_baz"},
+		{TestName: "ends with number", Value: "foo-bar_baz1"},
+		{TestName: "almost an ip", Value: "192.168.5.foo"},
+		{TestName: "has _", Value: "foo-bar_baz"},
+		{TestName: "--", Value: "foo--bar"},
+		{TestName: "__", Value: "foo__bar"},
+		{TestName: "-goog", Value: "foo-goog"},
+		{TestName: ".goog", Value: "foo.goog"},
+
+		// With errors
+		{TestName: "invalid char $", Value: "foo$bar", ExpectError: true},
+		{TestName: "has uppercase", Value: "fooBar", ExpectError: true},
+		{TestName: "begins with -", Value: "-foobar", ExpectError: true},
+		{TestName: "ends with -", Value: "foobar-", ExpectError: true},
+		{TestName: "begins with _", Value: "_foobar", ExpectError: true},
+		{TestName: "ends with _", Value: "foobar_", ExpectError: true},
+		{TestName: "less than 3 chars", Value: "fo", ExpectError: true},
+		{TestName: "..", Value: "foo..bar", ExpectError: true},
+		{TestName: "greater than 63 chars with no .", Value: "my-really-long-bucket-name-with-invalid-that-does-not-contain-a-period", ExpectError: true},
+		{TestName: "greater than 63 chars between .", Value: "my.really-long-bucket-name-with-invalid-that-does-contain-a-period-but.is-too-long", ExpectError: true},
+		{TestName: "has goog prefix", Value: "goog-foobar", ExpectError: true},
+		{TestName: "almost an ip", Value: "192.168.5.1", ExpectError: true},
+		{TestName: "contains google", Value: "foobar-google", ExpectError: true},
+		{TestName: "contains close misspelling of google", Value: "foo-go0gle-bar", ExpectError: true},
+	}
+
+	es := TestStringValidationCases(x, ValidateGCSName)
+	if len(es) > 0 {
+		t.Errorf("Failed to validate GCS names: %v", es)
+	}
+}