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.

Sign up for GitHub

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

Vault 18005 plugin api lock status #21925

Merged
merged 7 commits into from
Jul 31, 2023
Merged

Vault 18005 plugin api lock status #21925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
90473f9
add APILockShouldBlockRequest to backend proto
ccapurso Jul 17, 2023
84144e2
make proto
ccapurso Jul 17, 2023
149172c
add APILockShouldBlockRequest to system view
ccapurso Jul 17, 2023
8102dc0
Revert "make proto"
ccapurso Jul 19, 2023
384eb34
Revert "add APILockShouldBlockRequest to backend proto"
ccapurso Jul 19, 2023
a1a09d5
move APILockShouldBlockRequest to extended sys view
ccapurso Jul 19, 2023
24ec8a8
add changelog entry
ccapurso Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/21925.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
kmip (enterprise): Add namespace lock and unlock support
```
41 changes: 25 additions & 16 deletions sdk/logical/system_view.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,27 +107,32 @@ type PasswordPolicy interface {
type ExtendedSystemView interface {
Auditor() Auditor
ForwardGenericRequest(context.Context, *Request) (*Response, error)

// APILockShouldBlockRequest returns whether a namespace for the requested
// mount is locked and should be blocked
APILockShouldBlockRequest() (bool, error)
}

type PasswordGenerator func() (password string, err error)

type StaticSystemView struct {
DefaultLeaseTTLVal time.Duration
MaxLeaseTTLVal time.Duration
SudoPrivilegeVal bool
TaintedVal bool
CachingDisabledVal bool
Primary bool
EnableMlock bool
LocalMountVal bool
ReplicationStateVal consts.ReplicationState
EntityVal *Entity
GroupsVal []*Group
Features license.Features
PluginEnvironment *PluginEnvironment
PasswordPolicies map[string]PasswordGenerator
VersionString string
ClusterUUID string
DefaultLeaseTTLVal time.Duration
MaxLeaseTTLVal time.Duration
SudoPrivilegeVal bool
TaintedVal bool
CachingDisabledVal bool
Primary bool
EnableMlock bool
LocalMountVal bool
ReplicationStateVal consts.ReplicationState
EntityVal *Entity
GroupsVal []*Group
Features license.Features
PluginEnvironment *PluginEnvironment
PasswordPolicies map[string]PasswordGenerator
VersionString string
ClusterUUID string
APILockShouldBlockRequestVal bool
}

type noopAuditor struct{}
Expand Down Expand Up @@ -253,3 +258,7 @@ func (d *StaticSystemView) DeletePasswordPolicy(name string) (existed bool) {
func (d StaticSystemView) ClusterID(ctx context.Context) (string, error) {
return d.ClusterUUID, nil
}

func (d StaticSystemView) APILockShouldBlockRequest() (bool, error) {
return d.APILockShouldBlockRequestVal, nil
}
14 changes: 14 additions & 0 deletions vault/dynamic_system_view.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,20 @@ func (e extendedSystemViewImpl) SudoPrivilege(ctx context.Context, path string,
return authResults.RootPrivs
}

func (e extendedSystemViewImpl) APILockShouldBlockRequest() (bool, error) {
mountEntry := e.mountEntry
if mountEntry == nil {
return false, fmt.Errorf("no mount entry")
}
ns := mountEntry.Namespace()

if err := enterpriseBlockRequestIfError(e.core, ns.Path, mountEntry.Path); err != nil {
akshya96 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

return true, nil
}

return false, nil
}

func (d dynamicSystemView) DefaultLeaseTTL() time.Duration {
def, _ := d.fetchTTLs()
return def
Expand Down