Skip to content
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

DXCDT-431: auth0_role_permission resource #582

Merged
merged 12 commits into from
May 18, 2023

Conversation

willvedd
Copy link
Contributor

@willvedd willvedd commented May 16, 2023

🔧 Changes

Introducing the auth0_role_permission resource which enables management of role permissions through a 1-1 relationship.

This PR shares similarities with the auth0_user_permission resource (#574) but with extra considerations for the already-existing permissions field on the role resource like the migration guide and deprecation.

📚 References

Similar PR: #574

🔬 Testing

Added full suite of integration tests for this resource.

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@willvedd willvedd requested a review from a team as a code owner May 16, 2023 16:12
# relationship between the user and its roles.
resource auth0_user_roles user_roles {
user_id = auth0_user.user.id
roles = [auth0_role.admin.id]
}

# Or the auth0_user_roles to manage a 1:1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick fix

@codecov-commenter
Copy link

Codecov Report

Merging #582 (d42fc17) into main (0579003) will decrease coverage by 0.19%.
The diff coverage is 73.33%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #582      +/-   ##
==========================================
- Coverage   87.44%   87.26%   -0.19%     
==========================================
  Files          68       69       +1     
  Lines       10412    10547     +135     
==========================================
+ Hits         9105     9204      +99     
- Misses        988     1021      +33     
- Partials      319      322       +3     
Impacted Files Coverage Δ
internal/provider/provider.go 100.00% <ø> (ø)
internal/auth0/role/resource_permission.go 72.72% <72.72%> (ø)
internal/auth0/role/resource.go 80.58% <100.00%> (+0.34%) ⬆️

@auth0 auth0 deleted a comment from sergiught May 17, 2023
@willvedd willvedd requested a review from sergiught May 17, 2023 17:12
Config: acctest.ParseTestName(testAccRolePermissionsOneAssigned, strings.ToLower(t.Name())),
},
{
RefreshState: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
RefreshState: true,
RefreshState: true,

image

Should this be here? I think we can move the checks in the config above if we use the data source as the last elements that gets refreshed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be here for tests to pass. The data source gets executed after (or at best, during) the deletions of the auth0_role_permission resources. AFAICT, there is no way to depend on a deleted resource.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@willvedd willvedd requested a review from sergiught May 17, 2023 22:51
Copy link
Contributor

@sergiught sergiught left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪🏻

@sergiught sergiught merged commit ac04ae9 into main May 18, 2023
@sergiught sergiught deleted the DXCDT-431-role-permission-resource branch May 18, 2023 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants