Skip to content

Commit 09c1103

Browse files
authored
Fix validation of S3 bucket name in databricks_aws_unity_catalog_policy and databricks_aws_bucket_policy (#4691)
## Changes <!-- Summary of your changes that are easy to understand --> AWS S3 allows only alphanumeric, hyphens and dots in the S3 bucket names. Full list of naming rules is in the [docs](https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html#general-purpose-bucket-names). Resolves #4689 ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [x] `make test` run locally - [x] relevant change in `docs/` folder - [ ] covered with integration tests in `internal/acceptance` - [ ] using Go SDK - [ ] using TF Plugin Framework
1 parent 235d8f1 commit 09c1103

File tree

7 files changed

+36
-18
lines changed

7 files changed

+36
-18
lines changed

NEXT_CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
### Bug Fixes
88

9+
* Fix validation of S3 bucket name in `databricks_aws_unity_catalog_policy` and `databricks_aws_bucket_policy` [#4691](https://github.com/databricks/terraform-provider-databricks/pull/4691)
10+
911
### Documentation
1012

1113
### Exporter

aws/constants.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package aws
22

3+
import "regexp"
4+
35
var AwsConfig = map[string]map[string]string{
46
"aws": {
57
"accountId": "414351767826",
@@ -23,3 +25,6 @@ var AwsConfig = map[string]map[string]string{
2325

2426
var AwsPartitions = []string{"aws", "aws-us-gov", "aws-us-gov-dod"}
2527
var AwsPartitionsValidationError = "aws_partition must be either 'aws' or 'aws-us-gov' or 'aws-us-gov-dod'"
28+
29+
var AwsBucketNameRegex = regexp.MustCompile(`^[0-9a-z][-0-9a-z\.]{1,61}[0-9a-z]$`)
30+
var AwsBucketNameRegexError = "must contain only alphanumeric, dot, and hyphen characters"

aws/data_aws_bucket_policy.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7-
"regexp"
87

98
"github.com/databricks/terraform-provider-databricks/common"
109
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -88,11 +87,9 @@ func DataAwsBucketPolicy() common.Resource {
8887
Optional: true,
8988
},
9089
"bucket": {
91-
Type: schema.TypeString,
92-
Required: true,
93-
ValidateFunc: validation.StringMatch(
94-
regexp.MustCompile(`^[0-9a-zA-Z_-]+$`),
95-
"must contain only alphanumeric, underscore, and hyphen characters"),
90+
Type: schema.TypeString,
91+
Required: true,
92+
ValidateFunc: validation.StringMatch(AwsBucketNameRegex, AwsBucketNameRegexError),
9693
},
9794
"json": {
9895
Type: schema.TypeString,

aws/data_aws_unity_catalog_policy.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7-
"regexp"
87
"strings"
98

109
"github.com/databricks/terraform-provider-databricks/common"
@@ -140,11 +139,9 @@ func validateSchema() map[string]*schema.Schema {
140139
Optional: true,
141140
},
142141
"bucket_name": {
143-
Type: schema.TypeString,
144-
Required: true,
145-
ValidateFunc: validation.StringMatch(
146-
regexp.MustCompile(`^[0-9a-zA-Z_-]+$`),
147-
"must contain only alphanumeric, underscore, and hyphen characters"),
142+
Type: schema.TypeString,
143+
Required: true,
144+
ValidateFunc: validation.StringMatch(AwsBucketNameRegex, AwsBucketNameRegexError),
148145
},
149146
"role_name": {
150147
Type: schema.TypeString,

aws/data_aws_unity_catalog_policy_test.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package aws
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"testing"
67

78
"github.com/databricks/terraform-provider-databricks/qa"
@@ -16,12 +17,13 @@ func TestDataAwsUnityCatalogPolicy(t *testing.T) {
1617
ID: ".",
1718
HCL: `
1819
aws_account_id = "123456789098"
19-
bucket_name = "databricks-bucket"
20+
bucket_name = "databricks-bucket.2"
2021
role_name = "databricks-role"
2122
kms_name = "databricks-kms"
2223
`,
2324
}.Apply(t)
2425
assert.NoError(t, err)
26+
assert.Equal(t, "databricks-bucket.2-123456789098-databricks-role", d.Id())
2527
j := d.Get("json").(string)
2628
p := `{
2729
"Version": "2012-10-17",
@@ -39,8 +41,8 @@ func TestDataAwsUnityCatalogPolicy(t *testing.T) {
3941
"s3:AbortMultipartUpload"
4042
],
4143
"Resource": [
42-
"arn:aws:s3:::databricks-bucket/*",
43-
"arn:aws:s3:::databricks-bucket"
44+
"arn:aws:s3:::databricks-bucket.2/*",
45+
"arn:aws:s3:::databricks-bucket.2"
4446
]
4547
},
4648
{
@@ -88,7 +90,7 @@ func TestDataAwsUnityCatalogPolicy(t *testing.T) {
8890
"sqs:PurgeQueue"
8991
],
9092
"Resource": [
91-
"arn:aws:s3:::databricks-bucket",
93+
"arn:aws:s3:::databricks-bucket.2",
9294
"arn:aws:sqs:*:123456789098:csms-*",
9395
"arn:aws:sns:*:123456789098:csms-*"
9496
]
@@ -578,6 +580,21 @@ func TestDataAwsUnityCatalogPolicyPartionGovDoD(t *testing.T) {
578580
compareJSON(t, j, p)
579581
}
580582

583+
func TestDataAwsUnityCatalogPolicy_BucketNameInvalid(t *testing.T) {
584+
qa.ResourceFixture{
585+
Read: true,
586+
Resource: DataAwsUnityCatalogPolicy(),
587+
NonWritable: true,
588+
ID: ".",
589+
HCL: `
590+
aws_account_id = "123456789098"
591+
bucket_name = "-databricks-bucket"
592+
role_name = "databricks-role"
593+
kms_name = "databricks-kms"
594+
`,
595+
}.ExpectError(t, fmt.Sprintf("invalid config supplied. [bucket_name] invalid value for bucket_name (%s)", AwsBucketNameRegexError))
596+
}
597+
581598
func compareJSON(t *testing.T, json1 string, json2 string) {
582599
var i1 interface{}
583600
var i2 interface{}

docs/data-sources/aws_bucket_policy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ resource "aws_s3_bucket_policy" "ds" {
7676

7777
## Argument Reference
7878

79-
* `bucket` - (Required) AWS S3 Bucket name for which to generate the policy document.
79+
* `bucket` - (Required) AWS S3 Bucket name for which to generate the policy document. The name must follow the [S3 bucket naming rules](https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html).
8080
* `aws_partition` - (Optional) AWS partition. The options are `aws`, `aws-us-gov`, or `aws-us-gov-dod`. Defaults to `aws`
8181
* `full_access_role` - (Optional) Data access role that can have full access for this bucket
8282
* `databricks_e2_account_id` - (Optional) Your Databricks account ID. Used to generate restrictive IAM policies that will increase the security of your root bucket

docs/data-sources/aws_unity_catalog_policy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ resource "aws_iam_role" "metastore_data_access" {
4141

4242
* `aws_account_id` (Required) The Account ID of the current AWS account (not your Databricks account).
4343
* `aws_partition` - (Optional) AWS partition. The options are `aws`, `aws-us-gov`, or `aws-us-gov-dod`. Defaults to `aws`
44-
* `bucket_name` (Required) The name of the S3 bucket used as root storage location for [managed tables](https://docs.databricks.com/data-governance/unity-catalog/index.html#managed-table) in Unity Catalog.
44+
* `bucket_name` (Required) The name of the S3 bucket used as root storage location for [managed tables](https://docs.databricks.com/data-governance/unity-catalog/index.html#managed-table) in Unity Catalog. The name must follow the [S3 bucket naming rules](https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html).
4545
* `role_name` (Required) The name of the AWS IAM role that you created in the previous step in the [official documentation](https://docs.databricks.com/data-governance/unity-catalog/get-started.html#configure-a-storage-bucket-and-iam-role-in-aws).
4646
* `kms_name` (Optional) If encryption is enabled, provide the ARN of the KMS key that encrypts the S3 bucket contents. If encryption is disabled, do not provide this argument.
4747

0 commit comments

Comments
 (0)