Implement WAL rollback mechanism for Role Assignments #110

davidadeleon · 2022-11-07T18:41:27Z

Overview

Implement a rollback mechanism for Role Assignments to handle a case where on dynamic SP creation we may encounter an error and have to rollback any changes. Currently the AppID will be deleted which also removes the SP, but any successful role assignments will be orphaned. This incurs a hit against the Role Assignment limit. There is no change to the user experience.

Design of Change

Create a separate WAL where the generated UUIDs of the role assignments can be stored, and referred to if needed for rollback. Added a TRACE level log for instances where a 204 response is returned from Azure. A 204 is returned when the Role Assignment ID has already been deleted or never existed.

Test Output

❯ go test -v -run TestRoleAssignmentWALRollback
=== RUN   TestRoleAssignmentWALRollback
=== RUN   TestRoleAssignmentWALRollback/service_principals
=== PAUSE TestRoleAssignmentWALRollback/service_principals
=== CONT  TestRoleAssignmentWALRollback/service_principals
2022-11-07T13:37:24.357-0500 [DEBUG] rolling back role assignments for SP: ID=9cf990ff-39cb-4f20-b373-9afa49c9f752
2022-11-07T13:37:25.505-0500 [TRACE] role assignment already deleted or does not exist: err="error unassigning role: authorization.RoleAssignmentsClient#DeleteByID: Failure responding to request: StatusCode=204 -- Original Error: autorest/azure: error response cannot be parsed: \"\" error: EOF"
--- PASS: TestRoleAssignmentWALRollback (0.00s)
    --- PASS: TestRoleAssignmentWALRollback/service_principals (19.11s)
PASS
ok      github.com/hashicorp/vault-plugin-secrets-azure 19.352s

…nment ID

client.go

path_service_principal.go

path_service_principal_test.go

wal.go

… Roles associated with Role

austingebauer

Looking good! Left a few comments.

path_service_principal_test.go

client.go

path_service_principal_test.go

wal.go

…ize mismatch between number of roles and assignmentIDs, parameterize Resource Group in test

path_service_principal_test.go

…ead of Errorf for AzureRoles and assignmentIDs check

wal.go

austingebauer

LGTM!

…ired

client.go

…t-wal-rollback-for-roleassignments

* Implement Role Assignment WAL and rollback * Improve error handling around unassignment of non-existent role assignment ID * Better error handling in test, and guarding against nil or empty values * Add clarity to rollback log message, and check if there were no Azure Roles associated with Role * Further improve error handling, fix failing test, add guard against size mismatch between number of roles and assignmentIDs, parameterize Resource Group in test * Fix rollback test, and clean up left over debug line * Add missing error check for spRevoke during test, use errors.New instead of Errorf for AzureRoles and assignmentIDs check * Add warning about resources potentially still existing if WAL has expired

* Implement Role Assignment WAL and rollback * Improve error handling around unassignment of non-existent role assignment ID * Better error handling in test, and guarding against nil or empty values * Add clarity to rollback log message, and check if there were no Azure Roles associated with Role * Further improve error handling, fix failing test, add guard against size mismatch between number of roles and assignmentIDs, parameterize Resource Group in test * Fix rollback test, and clean up left over debug line * Add missing error check for spRevoke during test, use errors.New instead of Errorf for AzureRoles and assignmentIDs check * Add warning about resources potentially still existing if WAL has expired Co-authored-by: davidadeleon <56207066+davidadeleon@users.noreply.github.com>

davidadeleon added 2 commits November 7, 2022 11:08

Implement Role Assignment WAL and rollback

b4d5ef0

Improve error handling around unassignment of non-existent role assig…

9443e98

…nment ID

davidadeleon requested review from calvn, jasonodonnell and austingebauer November 7, 2022 18:41