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

Use write-ahead-logs to cleanup any orphaned Service Principals #42

Merged
merged 10 commits into from
Jul 24, 2020

Conversation

catsby
Copy link
Contributor

@catsby catsby commented Jul 20, 2020

Overview

Use Vault's WAL integration to ensure any service principals that are created when credentials are requested are then cleaned up if we fail to assign the roles. This could happen in instances where eventually consistency or otherwise timeouts occur during role assignment. For most cases, if the assignment succeeds the WAL will be deleted harmlessly. Included is a test TestSP_WAL_Cleanup that simulates a failure to assign a role and ensures the WAL record triggers a deletion.

Contributor Checklist

  • Add output for any tests not ran in CI to the PR description (eg, acceptance tests)

Tests:


$ make test

==> Checking that code complies with gofmt requirements...
go generate
VAULT_ACC=1 go test -tags='vault-plugin-secrets-azure' $(go list ./... | grep -v /vendor/) -v  -timeout 45m
=== RUN   TestRetry
=== PAUSE TestRetry
=== RUN   TestConfig
--- PASS: TestConfig (0.00s)
=== RUN   TestConfigDelete
--- PASS: TestConfigDelete (0.00s)
=== RUN   TestRoleCreate
=== RUN   TestRoleCreate/SP_role
=== RUN   TestRoleCreate/Static_SP_role
=== RUN   TestRoleCreate/Optional_role_TTLs
=== RUN   TestRoleCreate/Role_TTL_Checks
=== RUN   TestRoleCreate/Role_name_lookup
=== RUN   TestRoleCreate/Group_name_lookup
=== RUN   TestRoleCreate/Duplicate_role_name_and_scope
=== RUN   TestRoleCreate/Duplicate_role_name,_different_scope
=== RUN   TestRoleCreate/Duplicate_group_object_ID
=== RUN   TestRoleCreate/Role_name_lookup_(multiple_match)
=== RUN   TestRoleCreate/Group_name_lookup_(multiple_match)
--- PASS: TestRoleCreate (0.00s)
    --- PASS: TestRoleCreate/SP_role (0.00s)
    --- PASS: TestRoleCreate/Static_SP_role (0.00s)
    --- PASS: TestRoleCreate/Optional_role_TTLs (0.00s)
    --- PASS: TestRoleCreate/Role_TTL_Checks (0.00s)
    --- PASS: TestRoleCreate/Role_name_lookup (0.00s)
    --- PASS: TestRoleCreate/Group_name_lookup (0.00s)
    --- PASS: TestRoleCreate/Duplicate_role_name_and_scope (0.00s)
    --- PASS: TestRoleCreate/Duplicate_role_name,_different_scope (0.00s)
    --- PASS: TestRoleCreate/Duplicate_group_object_ID (0.00s)
    --- PASS: TestRoleCreate/Role_name_lookup_(multiple_match) (0.00s)
    --- PASS: TestRoleCreate/Group_name_lookup_(multiple_match) (0.00s)
=== RUN   TestRoleCreateBad
--- PASS: TestRoleCreateBad (0.00s)
=== RUN   TestRoleUpdateError
--- PASS: TestRoleUpdateError (0.00s)
=== RUN   TestRoleList
--- PASS: TestRoleList (0.00s)
=== RUN   TestRoleDelete
--- PASS: TestRoleDelete (0.00s)
=== RUN   TestSP_WAL_Cleanup
=== RUN   TestSP_WAL_Cleanup/Role_assign_fail
2020-07-20T16:42:21.270-0500 [DEBUG] rolling back SP: appID=31782b35-fa2c-5979-b44d-2d79ff12fb14 appObjID=a81bd8ae-8062-d5f7-d4eb-1dda59280e8b
--- PASS: TestSP_WAL_Cleanup (5.00s)
    --- PASS: TestSP_WAL_Cleanup/Role_assign_fail (5.00s)
=== RUN   TestSPRead
=== RUN   TestSPRead/Basic_Role
=== RUN   TestSPRead/Basic_Group
=== RUN   TestSPRead/TTLs
--- PASS: TestSPRead (0.00s)
    --- PASS: TestSPRead/Basic_Role (0.00s)
    --- PASS: TestSPRead/Basic_Group (0.00s)
    --- PASS: TestSPRead/TTLs (0.00s)
=== RUN   TestStaticSPRead
=== RUN   TestStaticSPRead/Basic
=== RUN   TestStaticSPRead/TTLs
--- PASS: TestStaticSPRead (0.00s)
    --- PASS: TestStaticSPRead/Basic (0.00s)
    --- PASS: TestStaticSPRead/TTLs (0.00s)
=== RUN   TestSPRevoke
=== RUN   TestSPRevoke/roles
=== RUN   TestSPRevoke/groups
--- PASS: TestSPRevoke (0.00s)
    --- PASS: TestSPRevoke/roles (0.00s)
    --- PASS: TestSPRevoke/groups (0.00s)
=== RUN   TestStaticSPRevoke
--- PASS: TestStaticSPRevoke (0.00s)
=== RUN   TestSPReadMissingRole
--- PASS: TestSPReadMissingRole (0.00s)
=== RUN   TestCredentialReadProviderError
--- PASS: TestCredentialReadProviderError (0.00s)
=== RUN   TestCredentialInteg
=== RUN   TestCredentialInteg/SP
=== PAUSE TestCredentialInteg/SP
=== RUN   TestCredentialInteg/Static_SP
=== PAUSE TestCredentialInteg/Static_SP
=== CONT  TestCredentialInteg/SP
=== CONT  TestCredentialInteg/Static_SP
--- PASS: TestCredentialInteg (0.00s)
    --- PASS: TestCredentialInteg/Static_SP (22.77s)
    --- PASS: TestCredentialInteg/SP (81.18s)
=== CONT  TestRetry
=== RUN   TestRetry/First_try_success
=== RUN   TestRetry/Three_retries
=== PAUSE TestRetry/Three_retries
=== RUN   TestRetry/Error_on_attempt
=== PAUSE TestRetry/Error_on_attempt
=== RUN   TestRetry/Timeout
=== PAUSE TestRetry/Timeout
=== RUN   TestRetry/Cancellation
=== PAUSE TestRetry/Cancellation
=== CONT  TestRetry/Three_retries
=== CONT  TestRetry/Timeout
=== CONT  TestRetry/Error_on_attempt
=== CONT  TestRetry/Cancellation
--- PASS: TestRetry (0.00s)
    --- PASS: TestRetry/First_try_success (0.00s)
    --- PASS: TestRetry/Error_on_attempt (0.00s)
    --- PASS: TestRetry/Cancellation (7.00s)
    --- PASS: TestRetry/Timeout (10.00s)
    --- PASS: TestRetry/Three_retries (13.21s)
PASS
ok      github.com/hashicorp/vault-plugin-secrets-azure 99.835s
?       github.com/hashicorp/vault-plugin-secrets-azure/cmd/vault-plugin-secrets-azure  [no test files]
  • Backwards compatible

@catsby catsby requested a review from kalafut July 20, 2020 21:47
@catsby catsby changed the title Wal app delete Use write-ahead-logs to cleanup any orphaned Service Principals Jul 21, 2020
@kalafut kalafut requested a review from calvn July 23, 2020 21:00
Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>
Copy link
Contributor

@calvn calvn left a comment

Choose a reason for hiding this comment

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

One minor question, otherwise looks good! Liked the TestSP_WAL_Cleanup test a lot!

path_service_principal.go Outdated Show resolved Hide resolved
Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>
@catsby catsby merged commit 100a12f into master Jul 24, 2020
@jasonodonnell jasonodonnell deleted the wal-app-delete branch August 18, 2020 13:16
catsby added a commit that referenced this pull request Aug 18, 2020
* Use WAL for App cleanup

* Fix typo

* Reduce maxWALAge

* Add StringToTimeHookFunc to decoding of WAL entries

* add new errMockProvider struct to simulate an error

* rough test

* small refactor

* refactor

* Update path_service_principal_test.go

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>

* Update path_service_principal.go

Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>
Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>
catsby added a commit that referenced this pull request Aug 18, 2020
* Use WAL for App cleanup

* Fix typo

* Reduce maxWALAge

* Add StringToTimeHookFunc to decoding of WAL entries

* add new errMockProvider struct to simulate an error

* rough test

* small refactor

* refactor

* Update path_service_principal_test.go

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>

* Update path_service_principal.go

Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>
Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>
catsby added a commit that referenced this pull request Aug 18, 2020
…#44)

* Use WAL for App cleanup

* Fix typo

* Reduce maxWALAge

* Add StringToTimeHookFunc to decoding of WAL entries

* add new errMockProvider struct to simulate an error

* rough test

* small refactor

* refactor

* Update path_service_principal_test.go

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>

* Update path_service_principal.go

Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>
Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>
Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>
kalafut pushed a commit that referenced this pull request Aug 18, 2020
* Use write-ahead-logs to cleanup any orphaned Service Principals (#42)

* Use WAL for App cleanup

* Fix typo

* Reduce maxWALAge

* Add StringToTimeHookFunc to decoding of WAL entries

* add new errMockProvider struct to simulate an error

* rough test

* small refactor

* refactor

* Update path_service_principal_test.go

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>

* Update path_service_principal.go

Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>
Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>

* add time

* update test to use old error words

Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>
Co-authored-by: Calvin Leung Huang <cleung2010@gmail.com>
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