DXCDT-432: auth0_role_permissions resource
#583
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #583 +/- ##
==========================================
- Coverage 87.28% 87.24% -0.04%
==========================================
Files 69 70 +1
Lines 10559 10693 +134
==========================================
+ Hits 9216 9329 +113
- Misses 1021 1037 +16
- Partials 322 327 +5
|
| @@ -113,11 +113,11 @@ resource auth0_role the_one { | |||
| } | |||
| ` | |||
|
|
|||
| func TestAccRolePermissions(t *testing.T) { | |||
| func TestAccRoleResourcePermissions(t *testing.T) { | |||
There was a problem hiding this comment.
Necessary to rename this test to not collide with the new resource.
| mutex.Lock(roleID) | ||
| defer mutex.Unlock(roleID) | ||
|
|
||
| permissions, err := api.Role.Permissions(roleID) |
There was a problem hiding this comment.
Hm... 🤔 This will remove all the permissions, not just the ones assigned through this resource. I think it should be okay, however in the auth0_user_roles we do
userRolesToRemove := data.Get("roles").(*schema.Set).List()
var rmRoles []*management.Role
for _, rmRole := range userRolesToRemove {
role := &management.Role{ID: auth0.String(rmRole.(string))}
rmRoles = append(rmRoles, role)
}
The two approaches technically should be exactly the same as we should manage all the permissions through this and have nothing left on the API.
However, there is one edge case where:
- You attach permissions to a role. (but the role has actually more permissions set on the API)
- You do a read in another terraform plan and you're gonna see the diff between what you have in the config and what's on remote.
- If you don't add those extra permissions to the config and immediately do a terraform destroy, with the logic we have now it will remove even the others that are untracked, whereas the solution above to fetch them again from the config will only remove the ones that we set in the config and leave the others that were untracked and shown in the diff.
What do you think we should do?
There was a problem hiding this comment.
While I think it would be an anti-pattern for folks to adopt that workflow, it's a safer and low-cost to only delete the ones within management purview.
| Config: acctest.ParseTestName(testAccRolePermissionNoneAssigned, strings.ToLower(t.Name())), | ||
| }, | ||
| { | ||
| RefreshState: true, |
There was a problem hiding this comment.
This is necessary otherwise the data source doesn't get the change reflected.
There was a problem hiding this comment.
Ah, I see that now 👍🏻 , indeed the execution flow on a terraform apply is:
- read and refresh state
- creations
- updates
- deletions
so we need another refresh state after the deletion.
sergiught
force-pushed
the
DXCDT-432-role-permissions-resource
branch
from
86ab1f8
to
9f7a550
Compare
🔧 Changes
Introducing the
auth0_role_permissionsresource which enables management of all role permissions in a single block.Unlike the
auth0_role_permissionresource (#582) which manages on a per-permission basis, this resource manages all of a role's permissions. In addition to offering a flexible way of expressing role permissions, out-of-band changes can be observed with a single import command rather than multiple imports per permission.Remaining a draft until split into two smaller PRs
📚 References
auth0_role_permission- DXCDT-431:auth0_role_permissionresource #582🔬 Testing
Added suite of integration tests.
📝 Checklist