Skip to content

Commit

Permalink
Merge pull request #16916 from chrisbulgaria/master
Browse files Browse the repository at this point in the history
adapted validatePrincipal to all allowed lakeformation principals types, solves #16848
  • Loading branch information
YakDriver authored Apr 29, 2021
2 parents 0f5e74a + 2b47ec6 commit 46d6e0e
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 7 deletions.
3 changes: 3 additions & 0 deletions .changelog/16916.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-notes:enhancement
resource/aws_lakeformation_permissions: Allow `principal` to be an AWS account ID or an IAM group, ou, or organization
```
6 changes: 3 additions & 3 deletions aws/resource_aws_appsync_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,13 +518,13 @@ EOF
}
resource "aws_appsync_datasource" "test" {
api_id = "${aws_appsync_graphql_api.test.id}"
api_id = aws_appsync_graphql_api.test.id
name = %q
service_role_arn = "${aws_iam_role.test.arn}"
service_role_arn = aws_iam_role.test.arn
type = "AWS_LAMBDA"
lambda_config {
function_arn = "${aws_lambda_function.test.arn}"
function_arn = aws_lambda_function.test.arn
}
}
Expand Down
16 changes: 14 additions & 2 deletions aws/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -673,13 +673,25 @@ func validatePrincipal(v interface{}, k string) (ws []string, errors []error) {
return ws, errors
}

// https://docs.aws.amazon.com/lake-formation/latest/dg/lf-permissions-reference.html
// Principal is an AWS account
// --principal DataLakePrincipalIdentifier=111122223333
wsAccount, errorsAccount := validateAwsAccountId(v, k)
if len(errorsAccount) == 0 {
return wsAccount, errorsAccount
}

wsARN, errorsARN := validateArn(v, k)
ws = append(ws, wsARN...)
errors = append(errors, errorsARN...)

pattern := `\d{12}:(role|user)/`
pattern := `:(role|user|group|ou|organization)/`
if !regexp.MustCompile(pattern).MatchString(value) {
errors = append(errors, fmt.Errorf("%q doesn't look like a user or role: %q", k, value))
errors = append(errors, fmt.Errorf("%q does not look like a user, role, group, OU, or organization: %q", k, value))
}

if len(errors) > 0 {
errors = append(errors, errorsAccount...)
}

return ws, errors
Expand Down
9 changes: 8 additions & 1 deletion aws/validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,10 +394,17 @@ func TestValidatePrincipal(t *testing.T) {

validNames := []string{
"IAM_ALLOWED_PRINCIPALS", // Special principal
"123456789012", // lintignore:AWSAT005 // Example Account ID (Valid looking but not real)
"111122223333", // lintignore:AWSAT005 // Example Account ID (Valid looking but not real)
"arn:aws-us-gov:iam::357342307427:role/tf-acc-test-3217321001347236965", // lintignore:AWSAT005 // IAM Role
"arn:aws:iam::123456789012:user/David", // lintignore:AWSAT005 // IAM User
"arn:aws-us-gov:iam:us-west-2:357342307427:role/tf-acc-test-3217321001347236965", // lintignore:AWSAT003,AWSAT005 // Non-global IAM Role?
"arn:aws:iam:us-east-1:123456789012:user/David", // lintignore:AWSAT003,AWSAT005 // Non-global IAM User?
"arn:aws:iam::111122223333:saml-provider/idp1:group/data-scientists", // lintignore:AWSAT005 // SAML group
"arn:aws:iam::111122223333:saml-provider/idp1:user/Paul", // lintignore:AWSAT005 // SAML user
"arn:aws:quicksight:us-east-1:111122223333:group/default/data_scientists", // lintignore:AWSAT003,AWSAT005 // quicksight group
"arn:aws:organizations::111122223333:organization/o-abcdefghijkl", // lintignore:AWSAT005 // organization
"arn:aws:organizations::111122223333:ou/o-abcdefghijkl/ou-ab00-cdefgh", // lintignore:AWSAT005 // ou
}
for _, v := range validNames {
_, errors := validatePrincipal(v, "arn")
Expand All @@ -409,7 +416,7 @@ func TestValidatePrincipal(t *testing.T) {
invalidNames := []string{
"IAM_NOT_ALLOWED_PRINCIPALS", // doesn't exist
"arn",
"123456789012",
"1234567890125", //not an account id
"arn:aws",
"arn:aws:logs", //lintignore:AWSAT005
"arn:aws:logs:region:*:*", //lintignore:AWSAT005
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/lakeformation_permissions.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ resource "aws_lakeformation_permissions" "test" {
The following arguments are required:

* `permissions` – (Required) List of permissions granted to the principal. Valid values may include `ALL`, `ALTER`, `CREATE_DATABASE`, `CREATE_TABLE`, `DATA_LOCATION_ACCESS`, `DELETE`, `DESCRIBE`, `DROP`, `INSERT`, and `SELECT`. For details on each permission, see [Lake Formation Permissions Reference](https://docs.aws.amazon.com/lake-formation/latest/dg/lf-permissions-reference.html).
* `principal` – (Required) Principal to be granted the permissions on the resource. Supported principals include IAM users and IAM roles.
* `principal` – (Required) Principal to be granted the permissions on the resource. Supported principals include IAM roles, users, groups, OUs, and organizations as well as AWS account IDs for cross-account permissions. For more information, see [Lake Formation Permissions Reference](https://docs.aws.amazon.com/lake-formation/latest/dg/lf-permissions-reference.html).

One of the following is required:

Expand Down

0 comments on commit 46d6e0e

Please sign in to comment.