Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] Corrected kms arn format in data_aws_unity_catalog_policy #3823

Merged
merged 1 commit into from
Jul 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions aws/data_aws_unity_catalog_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"regexp"
"strings"

"github.com/databricks/terraform-provider-databricks/common"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -44,16 +45,18 @@ func generateReadContext(ctx context.Context, d *schema.ResourceData, m *common.
},
}
if kmsKey, ok := d.GetOk("kms_name"); ok {
kmsArn := fmt.Sprintf("arn:aws:kms:%s", kmsKey)
if strings.HasPrefix(kmsKey.(string), "arn:aws") {
kmsArn = kmsKey.(string)
}
policy.Statements = append(policy.Statements, &awsIamPolicyStatement{
Effect: "Allow",
Actions: []string{
"kms:Decrypt",
"kms:Encrypt",
"kms:GenerateDataKey*",
},
Resources: []string{
fmt.Sprintf("arn:aws:kms:%s", kmsKey),
},
Resources: []string{kmsArn},
})
}
policyJSON, err := json.MarshalIndent(policy, "", " ")
Expand All @@ -73,9 +76,6 @@ func validateSchema() map[string]*schema.Schema {
"kms_name": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringMatch(
regexp.MustCompile(`^[0-9a-zA-Z/_-]+$`),
"must contain only alphanumeric, hyphens, forward slashes, and underscores characters"),
},
"bucket_name": {
Type: schema.TypeString,
Expand Down
57 changes: 57 additions & 0 deletions aws/data_aws_unity_catalog_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,63 @@ func TestDataAwsUnityCatalogPolicy(t *testing.T) {
compareJSON(t, j, p)
}

func TestDataAwsUnityCatalogPolicyFullKms(t *testing.T) {
d, err := qa.ResourceFixture{
Read: true,
Resource: DataAwsUnityCatalogPolicy(),
NonWritable: true,
ID: ".",
HCL: `
aws_account_id = "123456789098"
bucket_name = "databricks-bucket"
role_name = "databricks-role"
kms_name = "arn:aws:kms:us-west-2:111122223333:key/databricks-kms"
`,
}.Apply(t)
assert.NoError(t, err)
j := d.Get("json").(string)
p := `{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": [
"s3:GetObject",
"s3:PutObject",
"s3:DeleteObject",
"s3:ListBucket",
"s3:GetBucketLocation"
],
"Resource": [
"arn:aws:s3:::databricks-bucket/*",
"arn:aws:s3:::databricks-bucket"
]
},
{
"Effect": "Allow",
"Action": [
"sts:AssumeRole"
],
"Resource": [
"arn:aws:iam::123456789098:role/databricks-role"
]
},
{
"Effect": "Allow",
"Action": [
"kms:Decrypt",
"kms:Encrypt",
"kms:GenerateDataKey*"
],
"Resource": [
"arn:aws:kms:us-west-2:111122223333:key/databricks-kms"
]
}
]
}`
compareJSON(t, j, p)
}

func TestDataAwsUnityCatalogPolicyWithoutKMS(t *testing.T) {
d, err := qa.ResourceFixture{
Read: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ data "databricks_aws_unity_catalog_policy" "this" {
aws_account_id = var.aws_account_id
bucket_name = "databricks-bucket"
role_name = "${var.prefix}-uc-access"
kms_name = "databricks-kms"
kms_name = "arn:aws:kms:us-west-2:111122223333:key/databricks-kms"
}

data "databricks_aws_unity_catalog_assume_role_policy" "this" {
Expand Down
4 changes: 2 additions & 2 deletions docs/data-sources/aws_unity_catalog_policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ data "databricks_aws_unity_catalog_policy" "this" {
aws_account_id = var.aws_account_id
bucket_name = "databricks-bucket"
role_name = "${var.prefix}-uc-access"
kms_name = "databricks-kms"
kms_name = "arn:aws:kms:us-west-2:111122223333:key/databricks-kms"
}

data "databricks_aws_unity_catalog_assume_role_policy" "this" {
Expand All @@ -40,7 +40,7 @@ resource "aws_iam_role" "metastore_data_access" {
* `aws_account_id` (Required) The Account ID of the current AWS account (not your Databricks account).
* `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.
* `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).
* `kms_name` (Optional) If encryption is enabled, provide the name of the KMS key that encrypts the S3 bucket contents. If encryption is disabled, do not provide this argument.
* `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.

## Attribute Reference

Expand Down
2 changes: 1 addition & 1 deletion docs/guides/unity-catalog.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ resource "aws_iam_policy" "external_data_access" {

resource "aws_iam_role" "external_data_access" {
name = local.uc_iam_role
assume_role_policy = data.aws_iam_policy_document.passrole_for_uc.json
assume_role_policy = data.aws_iam_policy_document.this.json
managed_policy_arns = [aws_iam_policy.external_data_access.arn]
tags = merge(var.tags, {
Name = "${local.prefix}-unity-catalog external access IAM role"
Expand Down
Loading