Merge pull request #20108 from hashicorp/b-lakeformation-iam-allowed

lakeformation: Fix various bugs including SELECT permission issues
hashicorp · Jul 8, 2021 · f268ddf · f268ddf
2 parents d116587 + 8b7cb8e
commit f268ddf
Show file tree

Hide file tree

Showing 13 changed files with 1,400 additions and 547 deletions.
diff --git a/.changelog/20108.txt b/.changelog/20108.txt
@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_lakeformation_permissions: Fix various problems with permissions including select-only
+```
+
+```release-note:bug
+data-source/aws_lakeformation_permissions: Fix various problems with permissions including select-only
+```
diff --git a/aws/data_source_aws_lakeformation_data_lake_settings_test.go b/aws/data_source_aws_lakeformation_data_lake_settings_test.go
@@ -8,7 +8,6 @@ import (
 )
 
 func testAccAWSLakeFormationDataLakeSettingsDataSource_basic(t *testing.T) {
-	callerIdentityName := "data.aws_caller_identity.current"
 	resourceName := "data.aws_lakeformation_data_lake_settings.test"
 
 	resource.Test(t, resource.TestCase{
@@ -20,9 +19,9 @@ func testAccAWSLakeFormationDataLakeSettingsDataSource_basic(t *testing.T) {
 			{
 				Config: testAccAWSLakeFormationDataLakeSettingsDataSourceConfig_basic,
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttrPair(resourceName, "catalog_id", callerIdentityName, "account_id"),
+					resource.TestCheckResourceAttrPair(resourceName, "catalog_id", "data.aws_caller_identity.current", "account_id"),
 					resource.TestCheckResourceAttr(resourceName, "admins.#", "1"),
-					resource.TestCheckResourceAttrPair(resourceName, "admins.0", callerIdentityName, "arn"),
+					resource.TestCheckResourceAttrPair(resourceName, "admins.0", "data.aws_iam_session_context.current", "issuer_arn"),
 				),
 			},
 		},
@@ -32,9 +31,13 @@ func testAccAWSLakeFormationDataLakeSettingsDataSource_basic(t *testing.T) {
 const testAccAWSLakeFormationDataLakeSettingsDataSourceConfig_basic = `
 data "aws_caller_identity" "current" {}
 
+data "aws_iam_session_context" "current" {
+  arn = data.aws_caller_identity.current.arn
+}
+
 resource "aws_lakeformation_data_lake_settings" "test" {
   catalog_id = data.aws_caller_identity.current.account_id
-  admins     = [data.aws_caller_identity.current.arn]
+  admins     = [data.aws_iam_session_context.current.issuer_arn]
 }
 
 data "aws_lakeformation_data_lake_settings" "test" {

diff --git a/aws/data_source_aws_lakeformation_permissions.go b/aws/data_source_aws_lakeformation_permissions.go
@@ -277,6 +277,18 @@ func dataSourceAwsLakeFormationPermissionsRead(d *schema.ResourceData, meta inte
 	if v, ok := d.GetOk("table"); ok && len(v.([]interface{})) > 0 {
 		// since perm list could include TableWithColumns, get the right one
 		for _, perm := range cleanPermissions {
+			if perm.Resource == nil {
+				continue
+			}
+
+			if perm.Resource.TableWithColumns != nil && perm.Resource.TableWithColumns.ColumnWildcard != nil {
+				if err := d.Set("table", []interface{}{flattenLakeFormationTableWithColumnsResourceAsTable(perm.Resource.TableWithColumns)}); err != nil {
+					return fmt.Errorf("error setting table: %w", err)
+				}
+				tableSet = true
+				break
+			}
+
 			if perm.Resource.Table != nil {
 				if err := d.Set("table", []interface{}{flattenLakeFormationTableResource(perm.Resource.Table)}); err != nil {
 					return fmt.Errorf("error setting table: %w", err)

diff --git a/aws/data_source_aws_lakeformation_permissions_test.go b/aws/data_source_aws_lakeformation_permissions_test.go
@@ -148,28 +148,28 @@ data "aws_partition" "current" {}
 
 resource "aws_iam_role" "test" {
   name = %[1]q
+  path = "/"
 
-  assume_role_policy = <<EOF
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Action": "sts:AssumeRole",
-      "Principal": {
-        "Service": "lakeformation.${data.aws_partition.current.dns_suffix}"
-      },
-      "Effect": "Allow",
-      "Sid": ""
-    }
-  ]
-}
-EOF
+  assume_role_policy = jsonencode({
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Effect = "Allow"
+      Principal = {
+        Service = "glue.${data.aws_partition.current.dns_suffix}"
+      }
+    }]
+    Version = "2012-10-17"
+  })
 }
 
 data "aws_caller_identity" "current" {}
 
+data "aws_iam_session_context" "current" {
+  arn = data.aws_caller_identity.current.arn
+}
+
 resource "aws_lakeformation_data_lake_settings" "test" {
-  admins = [data.aws_caller_identity.current.arn]
+  admins = [data.aws_iam_session_context.current.issuer_arn]
 }
 
 resource "aws_lakeformation_permissions" "test" {
@@ -196,21 +196,16 @@ resource "aws_iam_role" "test" {
   name = %[1]q
   path = "/"
 
-  assume_role_policy = <<EOF
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Action": "sts:AssumeRole",
-      "Principal": {
-        "Service": "glue.${data.aws_partition.current.dns_suffix}"
-      },
-      "Effect": "Allow",
-      "Sid": ""
-    }
-  ]
-}
-EOF
+  assume_role_policy = jsonencode({
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Effect = "Allow"
+      Principal = {
+        Service = "glue.${data.aws_partition.current.dns_suffix}"
+      }
+    }]
+    Version = "2012-10-17"
+  })
 }
 
 resource "aws_s3_bucket" "test" {
@@ -225,8 +220,12 @@ resource "aws_lakeformation_resource" "test" {
 
 data "aws_caller_identity" "current" {}
 
+data "aws_iam_session_context" "current" {
+  arn = data.aws_caller_identity.current.arn
+}
+
 resource "aws_lakeformation_data_lake_settings" "test" {
-  admins = [data.aws_caller_identity.current.arn]
+  admins = [data.aws_iam_session_context.current.issuer_arn]
 }
 
 resource "aws_lakeformation_permissions" "test" {
@@ -259,25 +258,18 @@ resource "aws_iam_role" "test" {
   name = %[1]q
   path = "/"
 
-  assume_role_policy = <<EOF
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Action": "sts:AssumeRole",
-      "Principal": {
-        "Service": "glue.${data.aws_partition.current.dns_suffix}"
-      },
-      "Effect": "Allow",
-      "Sid": ""
-    }
-  ]
-}
-EOF
+  assume_role_policy = jsonencode({
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Effect = "Allow"
+      Principal = {
+        Service = "glue.${data.aws_partition.current.dns_suffix}"
+      }
+    }]
+    Version = "2012-10-17"
+  })
 }
 
-data "aws_caller_identity" "current" {}
-
 resource "aws_s3_bucket" "test" {
   bucket = %[1]q
 }
@@ -286,8 +278,14 @@ resource "aws_glue_catalog_database" "test" {
   name = %[1]q
 }
 
+data "aws_caller_identity" "current" {}
+
+data "aws_iam_session_context" "current" {
+  arn = data.aws_caller_identity.current.arn
+}
+
 resource "aws_lakeformation_data_lake_settings" "test" {
-  admins = [data.aws_caller_identity.current.arn]
+  admins = [data.aws_iam_session_context.current.issuer_arn]
 }
 
 resource "aws_lakeformation_permissions" "test" {
@@ -321,25 +319,18 @@ resource "aws_iam_role" "test" {
   name = %[1]q
   path = "/"
 
-  assume_role_policy = <<EOF
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Action": "sts:AssumeRole",
-      "Principal": {
-        "Service": "glue.${data.aws_partition.current.dns_suffix}"
-      },
-      "Effect": "Allow",
-      "Sid": ""
-    }
-  ]
-}
-EOF
+  assume_role_policy = jsonencode({
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Effect = "Allow"
+      Principal = {
+        Service = "glue.${data.aws_partition.current.dns_suffix}"
+      }
+    }]
+    Version = "2012-10-17"
+  })
 }
 
-data "aws_caller_identity" "current" {}
-
 resource "aws_s3_bucket" "test" {
   bucket = %[1]q
 }
@@ -353,8 +344,14 @@ resource "aws_glue_catalog_table" "test" {
   database_name = aws_glue_catalog_database.test.name
 }
 
+data "aws_caller_identity" "current" {}
+
+data "aws_iam_session_context" "current" {
+  arn = data.aws_caller_identity.current.arn
+}
+
 resource "aws_lakeformation_data_lake_settings" "test" {
-  admins = [data.aws_caller_identity.current.arn]
+  admins = [data.aws_iam_session_context.current.issuer_arn]
 }
 
 resource "aws_lakeformation_permissions" "test" {
@@ -389,25 +386,18 @@ resource "aws_iam_role" "test" {
   name = %[1]q
   path = "/"
 
-  assume_role_policy = <<EOF
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Action": "sts:AssumeRole",
-      "Principal": {
-        "Service": "glue.${data.aws_partition.current.dns_suffix}"
-      },
-      "Effect": "Allow",
-      "Sid": ""
-    }
-  ]
-}
-EOF
+  assume_role_policy = jsonencode({
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Effect = "Allow"
+      Principal = {
+        Service = "glue.${data.aws_partition.current.dns_suffix}"
+      }
+    }]
+    Version = "2012-10-17"
+  })
 }
 
-data "aws_caller_identity" "current" {}
-
 resource "aws_glue_catalog_database" "test" {
   name = %[1]q
 }
@@ -432,8 +422,14 @@ resource "aws_glue_catalog_table" "test" {
   }
 }
 
+data "aws_caller_identity" "current" {}
+
+data "aws_iam_session_context" "current" {
+  arn = data.aws_caller_identity.current.arn
+}
+
 resource "aws_lakeformation_data_lake_settings" "test" {
-  admins = [data.aws_caller_identity.current.arn]
+  admins = [data.aws_iam_session_context.current.issuer_arn]
 }
 
 resource "aws_lakeformation_permissions" "test" {

diff --git a/aws/internal/service/lakeformation/enum.go b/aws/internal/service/lakeformation/enum.go
@@ -0,0 +1,8 @@
+package lakeformation
+
+const (
+	TableNameAllTables        = "ALL_TABLES"
+	TableTypeTable            = "Table"
+	TableTypeTableWithColumns = "TableWithColumns"
+	IAMAllowedPrincipals      = "IAM_ALLOWED_PRINCIPALS"
+)