-
Notifications
You must be signed in to change notification settings - Fork 540
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
Handle out-of-band approle deletion #2142
Handle out-of-band approle deletion #2142
Conversation
@@ -37,6 +37,18 @@ func TestAccAppRoleAuthBackendRoleSecretID_basic(t *testing.T) { | |||
resource.TestCheckResourceAttrSet(secretIDResource, "accessor"), | |||
), | |||
}, | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that this test will fail on main and successfully apply and pass with the fixes in this PR
@@ -388,3 +415,7 @@ func approleAuthBackendRoleSecretIDParseID(id string) (backend, role, accessor s | |||
|
|||
return | |||
} | |||
|
|||
func isAppRoleDoesNotExistError(err error, role string) bool { | |||
return util.Is500(err) && strings.Contains(err.Error(), fmt.Sprintf("role \"%s\" does not exist", role)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I was thinking this week about these types of scenarios and we probably don't always handle them correctly like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Cleans up the AppRole Secret ID resource from TF state when a 500 error is returned from the Vault server upon an out-of-band deletion of the AppRole. Previously, all subsequent runs of TF would fail with the error. Ideally we should remove the non-existent secret ID from state and allow the TFVP to recreate the role and secret ID.
Closes #1683 #1666
Checklist
Output from acceptance testing: