-
Notifications
You must be signed in to change notification settings - Fork 391
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
Enhance DataAwsAssumeRolePolicy to allow the role to assume itself #3615
base: main
Are you sure you want to change the base?
Conversation
@unterstein thank you for getting around to this - do you mind updating the doc for the data source as well? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
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
|
@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 |
@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. |
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 asCondition-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:make test
run locallydocs/
folderinternal/acceptance