Vault 18005 plugin api lock status #21925
Conversation
github-actions
bot
added
the
hashicorp-contributed-pr
|
CI Results: All Go tests passed! ✅ |
ccapurso
force-pushed
the
vault-18005-plugin-api-lock-status
branch
from
169bf95 to
24ec8a8
Compare
|
Build Results: |
There was a problem hiding this comment.
Discussed some minor reservations out-of-band relating to the extra boolean abstraction. The design choice was intentionally made to provide a more straightforward method for the caller to consume.
Presumably the StaticSystemView changes are for testing, which was my only other reservation, so I'm going to go ahead and give this a +1.
| } | ||
| ns := mountEntry.Namespace() | ||
|
|
||
| if err := enterpriseBlockRequestIfError(e.core, ns.Path, mountEntry.Path); err != nil { |
There was a problem hiding this comment.
My only concern with this PR was (I think) that this code was previously only being called with the stateLock held, i.e. as part of regular request handling. Now we're calling it without that lock. From what I can tell that won't be a problem, just calling it out so you're aware, in case it helps with potential future issues.
There was a problem hiding this comment.
Ah right, I forgot about the fact that all locks grab a read lock. The API Lock system has its own lock which is obtained through underlying calls of enterpriseBlockRequestIfError. With that said, I'm not sure if there would be any issue caused by not holding the stateLock. I could easily be missing something though.
The KMIP plugin starts its own listener for which requests are not routed through Vault. Requests to this listener are not subjected to API locking if the KMIP mount resides within a namespace that has been locked. This PR introduces the ability, via the
ExtendedSystemViewfor a plugin to determine if requests to it should be blocked based on the lock status of the namespace within it resides. A separate ENT PR will be opened that adds tests as they will require namespace functionality.