terraform-provider-databricks
terraform-provider-databricks copied to clipboard
Enhance DataAwsAssumeRolePolicy to allow the role to assume itself
Changes
AWS roles used to be implicitly able to assume itself. As of somewhen last year this is no longer the case, therefore whenever a role wants to assume itself, it has to be configured explicitly in the assume role policy. We are using the DataAwsAssumeRolePolicy in our terraform stacks for a lot of roles but recently learned that this might cause issues. Therefore we would like to add the capabilities to this terraform resource, that the self assumption is also possible.
When creating roles with terraform, it is a little tricky to do this self assumption. Since ARNs are predictable, you could think, that you just can predict the ARN of your -to-be-created-role and add it to the list of Allow-sts:AssumeRole directly. But AWS will perform a check and will fail if the ARN is not already present. Since we are just about to create the role with that ARN, this is not possible. However, going in and adding the concrete check for the role as Condition-ArnLike, we will postpone the check if the ARN is actually existing to the runtime. At this time the ARN is present and the check will work as expected.
We do this kind of IAM approach for role self assumptions at a lot of other places ever since AWS announced their policy change last year and we didn't had problems with it so far.
Tests
I ran make test and verified that this addition is not breaking existing behaviour/existing tests. I added another test and verified the output policy of my test:
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": "sts:AssumeRole",
"Principal": {
"AWS": "arn:aws:iam::414351767826:root"
},
"Condition": {
"StringEquals": {
"sts:ExternalId": "abc"
}
}
},
{
"Sid": "ExplicitSelfRoleAssumption",
"Effect": "Allow",
"Action": "sts:AssumeRole",
"Principal": {
"AWS": "arn:aws:iam::aws-account:role/root"
},
"Condition": {
"ArnLike": {
"aws:PrincipalArn": "arn:aws:iam::aws-account:role/role-name"
}
}
}
]
}
- [x]
make testrun locally - [ ] relevant change in
docs/folder - [ ] covered with integration tests in
internal/acceptance - [ ] relevant acceptance tests are passing
- [ ] using Go SDK
@unterstein thank you for getting around to this - do you mind updating the doc for the data source as well?
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 82.33%. Comparing base (
fe9747a) to head (1980f46). Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #3615 +/- ##
==========================================
+ Coverage 82.31% 82.33% +0.02%
==========================================
Files 191 191
Lines 19329 19346 +17
==========================================
+ Hits 15910 15929 +19
+ Misses 2487 2486 -1
+ Partials 932 931 -1
| Files | Coverage Δ | |
|---|---|---|
| aws/data_aws_assume_role_policy.go | 86.95% <100.00%> (+7.64%) |
:arrow_up: |
@nkvuong thank you for your feedback. Is the place I added the documentation the right one and is it ok that way? Please feel free to adjust the docs in this PR to your liking.
thank you 🙏
@unterstein I just checked out documentation, and the cross-account policy does not require support for self assumption. It is only the Unity Catalog IAM role that requires support for self assumption, but we currently do not have a data source for that
@nkvuong we were informed by our partner from databricks, that we need to make some of our roles self assuming. For all roles that were called out, we use this particular DataAwsAssumeRolePolicy to generate it's assume role policy. In order to fulfil the request to make those role able to assume themselves, we need to adjust this component.
@unterstein confusingly, there are 2 different trust policies necessary, one for workspace creation & one for Unity Catalog. I've raised a separate PR #3622 for the second type of trust policies
the IAM role for workspace creation does not require self-assumption, but the IAM role for Unity Catalog requires it
@nkvuong we don't use databricks provider components for our unity catalog role and we didn't got those roles flagged from databricks. Where as we got the workspaces (where we use this component) called out that we need to fix them.