From b9d4a54341bcd36e1ffb1d546ee13bd1b24c7aa5 Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Mon, 9 Jan 2017 11:22:47 -0500 Subject: [PATCH] provider/aws: Add S3 Bucket name validation Adds region specific S3 bucket name validation. Currently all regions except for us-east-1 force a dns-compliant naming convention. Thus we cannot utilize the standard `SchemaValidateFunc` interface to validate an S3 bucket name. This change creates a helper function outside of the schema validation interface so we can validate S3 bucket names for both naming conventions. At a later date, when the us-east-1 region is updated to conform to a dns-compliant naming scheme, we can refactor the `validateS3BucketName` function to fit the `SchemaValidateFunc` interface. --- .../providers/aws/resource_aws_s3_bucket.go | 40 ++++++++++++ .../aws/resource_aws_s3_bucket_test.go | 64 +++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/builtin/providers/aws/resource_aws_s3_bucket.go b/builtin/providers/aws/resource_aws_s3_bucket.go index 729ae8117..4f47a1be1 100644 --- a/builtin/providers/aws/resource_aws_s3_bucket.go +++ b/builtin/providers/aws/resource_aws_s3_bucket.go @@ -6,6 +6,8 @@ import ( "fmt" "log" "net/url" + "regexp" + "strings" "time" "github.com/aws/aws-sdk-go/aws" @@ -408,6 +410,10 @@ func resourceAwsS3BucketCreate(d *schema.ResourceData, meta interface{}) error { } } + if err := validateS3BucketName(bucket, awsRegion); err != nil { + return fmt.Errorf("Error validating S3 bucket name: %s", err) + } + err := resource.Retry(5*time.Minute, func() *resource.RetryError { log.Printf("[DEBUG] Trying to create new S3 bucket: %q", bucket) _, err := s3conn.CreateBucket(req) @@ -1728,6 +1734,40 @@ func validateS3BucketRequestPayerType(v interface{}, k string) (ws []string, err return } +// validateS3BucketName validates any S3 bucket name that is not inside the us-east-1 region. +// Buckets outside of this region have to be DNS-compliant. After the same restrictions are +// applied to buckets in the us-east-1 region, this function can be refactored as a SchemaValidateFunc +func validateS3BucketName(value string, region string) error { + if region != "us-east-1" { + if (len(value) < 3) || (len(value) > 63) { + return fmt.Errorf("%q must contain from 3 to 63 characters", value) + } + if !regexp.MustCompile(`^[0-9a-z-.]+$`).MatchString(value) { + return fmt.Errorf("only lowercase alphanumeric characters and hyphens allowed in %q", value) + } + if regexp.MustCompile(`^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$`).MatchString(value) { + return fmt.Errorf("%q must not be formatted as an IP address", value) + } + if strings.HasPrefix(value, `.`) { + return fmt.Errorf("%q cannot start with a period", value) + } + if strings.HasSuffix(value, `.`) { + return fmt.Errorf("%q cannot end with a period", value) + } + if strings.Contains(value, `..`) { + return fmt.Errorf("%q can be only one period between labels", value) + } + } else { + if len(value) > 255 { + return fmt.Errorf("%q must contain less than 256 characters", value) + } + if !regexp.MustCompile(`^[0-9a-zA-Z-._]+$`).MatchString(value) { + return fmt.Errorf("only alphanumeric characters, hyphens, periods, and underscores allowed in %q", value) + } + } + return nil +} + func expirationHash(v interface{}) int { var buf bytes.Buffer m := v.(map[string]interface{}) diff --git a/builtin/providers/aws/resource_aws_s3_bucket_test.go b/builtin/providers/aws/resource_aws_s3_bucket_test.go index 38a60ee5a..f4872e91a 100644 --- a/builtin/providers/aws/resource_aws_s3_bucket_test.go +++ b/builtin/providers/aws/resource_aws_s3_bucket_test.go @@ -12,6 +12,8 @@ import ( "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + "strings" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/s3" @@ -690,6 +692,68 @@ func TestAccAWSS3Bucket_ReplicationExpectVersioningValidationError(t *testing.T) }) } +func TestAWSS3BucketName(t *testing.T) { + validDnsNames := []string{ + "foobar", + "foo.bar", + "foo.bar.baz", + "1234", + "foo-bar", + strings.Repeat("x", 63), + } + + for _, v := range validDnsNames { + if err := validateS3BucketName(v, "us-west-2"); err != nil { + t.Fatalf("%q should be a valid S3 bucket name", v) + } + } + + invalidDnsNames := []string{ + "foo..bar", + "Foo.Bar", + "192.168.0.1", + "127.0.0.1", + ".foo", + "bar.", + "foo_bar", + strings.Repeat("x", 64), + } + + for _, v := range invalidDnsNames { + if err := validateS3BucketName(v, "us-west-2"); err == nil { + t.Fatalf("%q should not be a valid S3 bucket name", v) + } + } + + validEastNames := []string{ + "foobar", + "foo_bar", + "127.0.0.1", + "foo..bar", + "foo_bar_baz", + "foo.bar.baz", + "Foo.Bar", + strings.Repeat("x", 255), + } + + for _, v := range validEastNames { + if err := validateS3BucketName(v, "us-east-1"); err != nil { + t.Fatalf("%q should be a valid S3 bucket name", v) + } + } + + invalidEastNames := []string{ + "foo;bar", + strings.Repeat("x", 256), + } + + for _, v := range invalidEastNames { + if err := validateS3BucketName(v, "us-east-1"); err == nil { + t.Fatalf("%q should not be a valid S3 bucket name", v) + } + } +} + func testAccCheckAWSS3BucketDestroy(s *terraform.State) error { return testAccCheckInstanceDestroyWithProvider(s, testAccProvider) }