From 8e02735411039856ec1dc1f0f36471c268ec857f Mon Sep 17 00:00:00 2001 From: Nathan Bugden <34457149+nbugden@users.noreply.github.com> Date: Mon, 15 Apr 2024 16:42:56 -0400 Subject: [PATCH] fix: added gcs name validation (#10426) --- mmv1/products/storage/Bucket.yaml | 2 + .../storage/resource_storage_bucket.go.erb | 5 +- .../terraform/tpgresource/utils.go | 25 ---------- .../terraform/tpgresource/utils_test.go | 42 ---------------- .../terraform/verify/validation.go | 48 +++++++++++++++++++ .../terraform/verify/validation_test.go | 40 ++++++++++++++++ 6 files changed, 92 insertions(+), 70 deletions(-) diff --git a/mmv1/products/storage/Bucket.yaml b/mmv1/products/storage/Bucket.yaml index 979c5716bd58..44427385fb28 100644 --- a/mmv1/products/storage/Bucket.yaml +++ b/mmv1/products/storage/Bucket.yaml @@ -392,6 +392,8 @@ properties: - !ruby/object:Api::Type::String name: 'name' description: 'The name of the bucket' + validation: !ruby/object:Provider::Terraform::Validation + function: 'verify.ValidateGCSName' - !ruby/object:Api::Type::NestedObject name: 'owner' description: | diff --git a/mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.erb b/mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.erb index a7950ee34bc6..aa7371e93fcb 100644 --- a/mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.erb +++ b/mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.erb @@ -16,6 +16,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" @@ -62,6 +63,7 @@ func ResourceStorageBucket() *schema.Resource { Required: true, ForceNew: true, Description: `The name of the bucket.`, + ValidateFunc: verify.ValidateGCSName, }, "encryption": { @@ -578,9 +580,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. diff --git a/mmv1/third_party/terraform/tpgresource/utils.go b/mmv1/third_party/terraform/tpgresource/utils.go index 4a15013c0988..4d16bc9a15eb 100644 --- a/mmv1/third_party/terraform/tpgresource/utils.go +++ b/mmv1/third_party/terraform/tpgresource/utils.go @@ -615,31 +615,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\":\"\"") { diff --git a/mmv1/third_party/terraform/tpgresource/utils_test.go b/mmv1/third_party/terraform/tpgresource/utils_test.go index 8eca921a53bd..19222a730565 100644 --- a/mmv1/third_party/terraform/tpgresource/utils_test.go +++ b/mmv1/third_party/terraform/tpgresource/utils_test.go @@ -9,7 +9,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" @@ -1254,44 +1253,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) - } - } -} diff --git a/mmv1/third_party/terraform/verify/validation.go b/mmv1/third_party/terraform/verify/validation.go index f07d5bcf9cb7..2e9da45a5cf2 100644 --- a/mmv1/third_party/terraform/verify/validation.go +++ b/mmv1/third_party/terraform/verify/validation.go @@ -74,6 +74,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{ @@ -89,6 +99,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)) diff --git a/mmv1/third_party/terraform/verify/validation_test.go b/mmv1/third_party/terraform/verify/validation_test.go index d6789dc6e46c..b03fac489df7 100644 --- a/mmv1/third_party/terraform/verify/validation_test.go +++ b/mmv1/third_party/terraform/verify/validation_test.go @@ -317,3 +317,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) + } +}