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

Issue 26920 support colon in LakeFormation lf-tag key #28258

Merged
3 changes: 3 additions & 0 deletions .changelog/28258.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_lakeformation_lf_tag: Fix support for lf-tag keys with colons in the name
```
2 changes: 1 addition & 1 deletion internal/service/lakeformation/lf_tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func resourceLFTagDelete(d *schema.ResourceData, meta interface{}) error {
}

func ReadLFTagID(id string) (catalogID string, tagKey string, err error) {
idParts := strings.Split(id, ":")
idParts := strings.SplitN(id, ":", 2)
if len(idParts) != 2 {
return "", "", fmt.Errorf("unexpected format of ID (%q), expected CATALOG-ID:TAG-KEY", id)
}
Expand Down
8 changes: 8 additions & 0 deletions internal/service/lakeformation/lf_tag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ func testAccLFTag_values(t *testing.T) {
acctest.CheckResourceAttrAccountID(resourceName, "catalog_id"),
),
},
{
Config: testAccLFTagConfig_values(rName+":Colon", []string{"value1", "value2"}),
Destroy: false,
Check: resource.ComposeTestCheckFunc(
testAccCheckLFTagExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "key", rName+":Colon"),
),
},
},
})
}
Expand Down
56 changes: 48 additions & 8 deletions internal/service/lakeformation/permissions_data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ func testAccPermissionsDataSource_lfTag(t *testing.T) {
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_lakeformation_permissions.test"
dataSourceName := "data.aws_lakeformation_permissions.test"
keyName := rName + ":colon"

resource.Test(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(lakeformation.EndpointsID, t) },
Expand All @@ -100,7 +101,22 @@ func testAccPermissionsDataSource_lfTag(t *testing.T) {
CheckDestroy: testAccCheckPermissionsDestroy,
Steps: []resource.TestStep{
{
Config: testAccPermissionsDataSourceConfig_lfTag(rName),
Config: testAccPermissionsDataSourceConfig_lfTag(rName, ""),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(resourceName, "principal", dataSourceName, "principal"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag.#", dataSourceName, "lf_tag.#"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag.0.key", dataSourceName, "lf_tag.0.key"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag.0.values", dataSourceName, "lf_tag.0.values"),
resource.TestCheckResourceAttrPair(resourceName, "permissions.#", dataSourceName, "permissions.#"),
resource.TestCheckResourceAttrPair(resourceName, "permissions.0", dataSourceName, "permissions.0"),
resource.TestCheckResourceAttrPair(resourceName, "permissions.1", dataSourceName, "permissions.1"),
resource.TestCheckResourceAttrPair(resourceName, "permissions_with_grant_option.#", dataSourceName, "permissions_with_grant_option.#"),
resource.TestCheckResourceAttrPair(resourceName, "permissions_with_grant_option.0", dataSourceName, "permissions_with_grant_option.0"),
resource.TestCheckResourceAttrPair(resourceName, "permissions_with_grant_option.1", dataSourceName, "permissions_with_grant_option.1"),
),
},
{
Config: testAccPermissionsDataSourceConfig_lfTag(rName, keyName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(resourceName, "principal", dataSourceName, "principal"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag.#", dataSourceName, "lf_tag.#"),
Expand All @@ -122,6 +138,7 @@ func testAccPermissionsDataSource_lfTagPolicy(t *testing.T) {
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_lakeformation_permissions.test"
dataSourceName := "data.aws_lakeformation_permissions.test"
keyName := rName + ":colon"

resource.Test(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(lakeformation.EndpointsID, t) },
Expand All @@ -130,7 +147,24 @@ func testAccPermissionsDataSource_lfTagPolicy(t *testing.T) {
CheckDestroy: testAccCheckPermissionsDestroy,
Steps: []resource.TestStep{
{
Config: testAccPermissionsDataSourceConfig_lfTagPolicy(rName),
Config: testAccPermissionsDataSourceConfig_lfTagPolicy(rName, keyName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(resourceName, "principal", dataSourceName, "principal"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag_policy.#", dataSourceName, "lf_tag_policy.#"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag_policy.0.resource_type", dataSourceName, "lf_tag_policy.0.resource_type"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag_policy.0.expression.#", dataSourceName, "lf_tag_policy.0.expression.#"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag_policy.0.expression.0.key", dataSourceName, "lf_tag_policy.0.expression.0.key"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag_policy.0.expression.0.values", dataSourceName, "lf_tag_policy.0.expression.0.values"),
resource.TestCheckResourceAttrPair(resourceName, "permissions.#", dataSourceName, "permissions.#"),
resource.TestCheckResourceAttrPair(resourceName, "permissions.0", dataSourceName, "permissions.0"),
resource.TestCheckResourceAttrPair(resourceName, "permissions.1", dataSourceName, "permissions.1"),
resource.TestCheckResourceAttrPair(resourceName, "permissions.2", dataSourceName, "permissions.2"),
resource.TestCheckResourceAttrPair(resourceName, "permissions_with_grant_option.#", dataSourceName, "permissions_with_grant_option.#"),
resource.TestCheckResourceAttrPair(resourceName, "permissions_with_grant_option.0", dataSourceName, "permissions_with_grant_option.0"),
),
},
{
Config: testAccPermissionsDataSourceConfig_lfTagPolicy(rName+"2", keyName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(resourceName, "principal", dataSourceName, "principal"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag_policy.#", dataSourceName, "lf_tag_policy.#"),
Expand Down Expand Up @@ -378,7 +412,10 @@ data "aws_lakeformation_permissions" "test" {
`, rName)
}

func testAccPermissionsDataSourceConfig_lfTag(rName string) string {
func testAccPermissionsDataSourceConfig_lfTag(rName string, keyName string) string {
if len(keyName) == 0 {
keyName = rName
}
return fmt.Sprintf(`
data "aws_partition" "current" {}
resource "aws_iam_role" "test" {
Expand Down Expand Up @@ -412,7 +449,7 @@ resource "aws_lakeformation_data_lake_settings" "test" {
}

resource "aws_lakeformation_lf_tag" "test" {
key = %[1]q
key = %[2]q
values = ["value1", "value2"]
# for consistency, ensure that admins are setup before testing
depends_on = [aws_lakeformation_data_lake_settings.test]
Expand Down Expand Up @@ -440,10 +477,13 @@ data "aws_lakeformation_permissions" "test" {
values = aws_lakeformation_lf_tag.test.values
}
}
`, rName)
`, rName, keyName)
}

func testAccPermissionsDataSourceConfig_lfTagPolicy(rName string) string {
func testAccPermissionsDataSourceConfig_lfTagPolicy(rName string, keyName string) string {
if len(keyName) == 0 {
keyName = rName
}
return fmt.Sprintf(`
data "aws_partition" "current" {}

Expand Down Expand Up @@ -478,7 +518,7 @@ resource "aws_lakeformation_data_lake_settings" "test" {
}

resource "aws_lakeformation_lf_tag" "test" {
key = %[1]q
key = %[2]q
values = ["value1", "value2"]

# for consistency, ensure that admins are setup before testing
Expand Down Expand Up @@ -518,7 +558,7 @@ data "aws_lakeformation_permissions" "test" {
}
}
}
`, rName)
`, rName, keyName)
}

func testAccPermissionsDataSourceConfig_table(rName string) string {
Expand Down
55 changes: 46 additions & 9 deletions internal/service/lakeformation/permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,15 @@ func testAccPermissions_lfTag(t *testing.T) {
resourceName := "aws_lakeformation_permissions.test"
roleName := "aws_iam_role.test"
tagName := "aws_lakeformation_lf_tag.test"

keyName := rName + ":colon"
resource.Test(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(lakeformation.EndpointsID, t) },
ErrorCheck: acctest.ErrorCheck(t, lakeformation.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckPermissionsDestroy,
Steps: []resource.TestStep{
{
Config: testAccPermissionsConfig_lfTag(rName),
Config: testAccPermissionsConfig_lfTag(rName, ""),
Check: resource.ComposeTestCheckFunc(
testAccCheckPermissionsExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "principal", roleName, "arn"),
Expand All @@ -230,6 +230,15 @@ func testAccPermissions_lfTag(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "permissions_with_grant_option.1", "DESCRIBE"),
),
},
{
Config: testAccPermissionsConfig_lfTag(rName, keyName),
Check: resource.ComposeTestCheckFunc(
testAccCheckPermissionsExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "lf_tag.#", "1"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag.0.key", tagName, "key"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag.0.values", tagName, "values"),
),
},
},
})
}
Expand All @@ -239,6 +248,7 @@ func testAccPermissions_lfTagPolicy(t *testing.T) {
resourceName := "aws_lakeformation_permissions.test"
roleName := "aws_iam_role.test"
tagName := "aws_lakeformation_lf_tag.test"
keyName := rName + ":colon"

resource.Test(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(lakeformation.EndpointsID, t) },
Expand All @@ -247,7 +257,27 @@ func testAccPermissions_lfTagPolicy(t *testing.T) {
CheckDestroy: testAccCheckPermissionsDestroy,
Steps: []resource.TestStep{
{
Config: testAccPermissionsConfig_lfTagPolicy(rName),
Config: testAccPermissionsConfig_lfTagPolicy(rName, ""),
Check: resource.ComposeTestCheckFunc(
testAccCheckPermissionsExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "principal", roleName, "arn"),
resource.TestCheckResourceAttr(resourceName, "catalog_resource", "false"),
resource.TestCheckResourceAttrPair(resourceName, "principal", roleName, "arn"),
resource.TestCheckResourceAttr(resourceName, "lf_tag_policy.#", "1"),
resource.TestCheckResourceAttr(resourceName, "lf_tag_policy.0.resource_type", "DATABASE"),
resource.TestCheckResourceAttr(resourceName, "lf_tag_policy.0.expression.#", "1"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag_policy.0.expression.0.key", tagName, "key"),
resource.TestCheckResourceAttrPair(resourceName, "lf_tag_policy.0.expression.0.values", tagName, "values"),
resource.TestCheckResourceAttr(resourceName, "permissions.#", "3"),
resource.TestCheckResourceAttr(resourceName, "permissions.0", lakeformation.PermissionAlter),
resource.TestCheckResourceAttr(resourceName, "permissions.1", lakeformation.PermissionCreateTable),
resource.TestCheckResourceAttr(resourceName, "permissions.2", lakeformation.PermissionDrop),
resource.TestCheckResourceAttr(resourceName, "permissions_with_grant_option.#", "1"),
resource.TestCheckResourceAttr(resourceName, "permissions_with_grant_option.0", lakeformation.PermissionCreateTable),
),
},
{
Config: testAccPermissionsConfig_lfTagPolicy(rName+"2", keyName),
Check: resource.ComposeTestCheckFunc(
testAccCheckPermissionsExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "principal", roleName, "arn"),
Expand Down Expand Up @@ -1301,7 +1331,10 @@ resource "aws_lakeformation_permissions" "test" {
`, rName)
}

func testAccPermissionsConfig_lfTag(rName string) string {
func testAccPermissionsConfig_lfTag(rName string, key string) string {
if len(key) == 0 {
key = rName
}
return fmt.Sprintf(`
data "aws_partition" "current" {}

Expand Down Expand Up @@ -1335,8 +1368,9 @@ resource "aws_lakeformation_data_lake_settings" "test" {
admins = [data.aws_iam_session_context.current.issuer_arn]
}


resource "aws_lakeformation_lf_tag" "test" {
key = %[1]q
key = %[2]q
values = ["value1", "value2"]

# for consistency, ensure that admins are setup before testing
Expand All @@ -1356,10 +1390,13 @@ resource "aws_lakeformation_permissions" "test" {
# for consistency, ensure that admins are setup before testing
depends_on = [aws_lakeformation_data_lake_settings.test]
}
`, rName)
`, rName, key)
}

func testAccPermissionsConfig_lfTagPolicy(rName string) string {
func testAccPermissionsConfig_lfTagPolicy(rName string, keyName string) string {
if len(keyName) == 0 {
keyName = rName
}
return fmt.Sprintf(`
data "aws_partition" "current" {}
resource "aws_iam_role" "test" {
Expand Down Expand Up @@ -1393,7 +1430,7 @@ resource "aws_lakeformation_data_lake_settings" "test" {
}

resource "aws_lakeformation_lf_tag" "test" {
key = %[1]q
key = %[2]q
values = ["value1", "value2"]

# for consistency, ensure that admins are setup before testing
Expand All @@ -1420,7 +1457,7 @@ resource "aws_lakeformation_permissions" "test" {
aws_lakeformation_lf_tag.test,
]
}
`, rName)
`, rName, keyName)
}

func testAccPermissionsConfig_tableBasic(rName string) string {
Expand Down
34 changes: 30 additions & 4 deletions internal/service/lakeformation/resource_lf_tags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func testAccResourceLFTags_databaseMultiple(t *testing.T) {
CheckDestroy: testAccCheckDatabaseLFTagsDestroy,
Steps: []resource.TestStep{
{
Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"}, []string{"farm", "theloop", "aintree", "brooklands", "maggotts", "becketts", "vale"}, "woodcote", "theloop"),
Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"}, []string{"farm", "theloop", "aintree", "brooklands", "maggotts", "becketts", "vale"}, "woodcote", "theloop", "copse"),
Destroy: false,
Check: resource.ComposeTestCheckFunc(
testAccCheckDatabaseLFTagsExists(resourceName),
Expand All @@ -103,7 +103,7 @@ func testAccResourceLFTags_databaseMultiple(t *testing.T) {
),
},
{
Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"}, []string{"farm", "theloop", "aintree", "brooklands", "maggotts", "becketts", "vale"}, "stowe", "becketts"),
Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"}, []string{"farm", "theloop", "aintree", "brooklands", "maggotts", "becketts", "vale"}, "stowe", "becketts", "copse"),
Check: resource.ComposeTestCheckFunc(
testAccCheckDatabaseLFTagsExists(resourceName),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{
Expand All @@ -114,6 +114,20 @@ func testAccResourceLFTags_databaseMultiple(t *testing.T) {
"key": fmt.Sprintf("%s-2", rName),
"value": "becketts",
}),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{
"key": fmt.Sprintf("%s:colon", rName),
"value": "copse",
}),
),
},
{
Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"}, []string{"farm", "theloop", "aintree", "brooklands", "maggotts", "becketts", "vale"}, "stowe", "becketts", "copse"),
Check: resource.ComposeTestCheckFunc(
testAccCheckDatabaseLFTagsExists(resourceName),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{
"key": rName,
"value": "stowe",
}),
),
},
},
Expand Down Expand Up @@ -474,7 +488,7 @@ resource "aws_lakeformation_resource_lf_tags" "test" {
`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`)), value)
}

func testAccResourceLFTagsConfig_databaseMultiple(rName string, values1, values2 []string, value1, value2 string) string {
func testAccResourceLFTagsConfig_databaseMultiple(rName string, values1, values2 []string, value1, value2, value3 string) string {
return fmt.Sprintf(`
data "aws_caller_identity" "current" {}

Expand Down Expand Up @@ -506,6 +520,14 @@ resource "aws_lakeformation_lf_tag" "test2" {
depends_on = [aws_lakeformation_data_lake_settings.test]
}

resource "aws_lakeformation_lf_tag" "test3" {
key = "%[1]s:colon"
values = [%[2]s]

# for consistency, ensure that admins are setup before testing
depends_on = [aws_lakeformation_data_lake_settings.test]
}

resource "aws_lakeformation_resource_lf_tags" "test" {
database {
name = aws_glue_catalog_database.test.name
Expand All @@ -521,10 +543,14 @@ resource "aws_lakeformation_resource_lf_tags" "test" {
value = %[5]q
}

lf_tag {
key = aws_lakeformation_lf_tag.test3.key
value = %[6]q
}
# for consistency, ensure that admins are setup before testing
depends_on = [aws_lakeformation_data_lake_settings.test]
}
`, rName, fmt.Sprintf(`"%s"`, strings.Join(values1, `", "`)), fmt.Sprintf(`"%s"`, strings.Join(values2, `", "`)), value1, value2)
`, rName, fmt.Sprintf(`"%s"`, strings.Join(values1, `", "`)), fmt.Sprintf(`"%s"`, strings.Join(values2, `", "`)), value1, value2, value3)
}

func testAccResourceLFTagsConfig_table(rName string, values []string, value string) string {
Expand Down