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

Vault 1979: Query API for Irrevocable Leases #11607

Merged
merged 28 commits into from Jun 2, 2021
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
483feae
build out lease count (not fully working), start lease list
swayne275 May 13, 2021
9d9f837
build out irrevocable lease list
swayne275 May 13, 2021
41dfaba
bookkeeping
swayne275 May 13, 2021
167ab90
Merge branch 'master' into vault-1979_expr-irrevocable-query-api
swayne275 May 13, 2021
ed4f011
test irrevocable lease counts for API/CLI
swayne275 May 20, 2021
e76af8a
fix listIrrevocableLeases, test listIrrevocableLeases, cleanup
swayne275 May 20, 2021
6ab77e4
test expiration API limit
swayne275 May 20, 2021
61786f5
namespace tweaks, test force flag on lease list
swayne275 May 20, 2021
7bc6712
integration test leases/count API, plenty of fixes and improvements
swayne275 May 21, 2021
a5450c8
Merge branch 'master' into vault-1979_expr-irrevocable-query-api
swayne275 May 21, 2021
e18fad4
test lease list API, fixes and improvements
swayne275 May 21, 2021
61e8316
test force flag for irrevocable lease list API
swayne275 May 21, 2021
d5164e3
i guess this wasn't saved on the last refactor...
swayne275 May 21, 2021
fb8b995
fixes and improvements found during my review
swayne275 May 21, 2021
b81ea9b
better test error msg
swayne275 May 21, 2021
743a32b
Merge branch 'master' into vault-1979_expr-irrevocable-query-api
swayne275 May 25, 2021
6114caa
Update vault/logical_system_paths.go
swayne275 May 25, 2021
0d61f9c
Update vault/logical_system_paths.go
swayne275 May 25, 2021
cb47c4a
return warning with data if more than default leases to list without …
swayne275 May 25, 2021
e5dafb4
make api doc more generalized
swayne275 May 26, 2021
6f2abe7
list leases in general, not by mount point
swayne275 May 26, 2021
da988f9
change force flag to include_large_results
swayne275 May 26, 2021
d06e923
sort leases by LeaseID for consistent API response
swayne275 May 26, 2021
33c1d4c
switch from bool flag for API limit to string value
swayne275 May 27, 2021
23a1082
sort first by leaseID, then stable sort by expiration
swayne275 Jun 1, 2021
3b618b8
move some utils to be in oss and ent
swayne275 Jun 1, 2021
fd2044d
Merge branch 'master' into vault-1979_expr-irrevocable-query-api
swayne275 Jun 1, 2021
585caea
improve sort efficiency for API response
swayne275 Jun 2, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/11607.txt
@@ -0,0 +1,3 @@
```release-note:improvement
core: add irrevocable lease list and count apis
```
157 changes: 145 additions & 12 deletions vault/expiration.go
Expand Up @@ -73,6 +73,14 @@ const (
genericIrrevocableErrorMessage = "unknown"

outOfRetriesMessage = "out of retries"

// maximum number of irrevocable leases we return to the irrevocable lease
// list API **without** the `force` flag set
MaxIrrevocableLeasesToReturn = 10000
)

var (
errHitMaxIrrevocableLeases = errors.New("Command cancelled because many irrevocable leases were found. To emit the entire list, re-run the command with force set true.")
)

type pendingInfo struct {
Expand Down Expand Up @@ -261,18 +269,7 @@ func (r *revocationJob) OnFailure(err error) {
func expireLeaseStrategyFairsharing(ctx context.Context, m *ExpirationManager, leaseID string, ns *namespace.Namespace) {
nsCtx := namespace.ContextWithNamespace(ctx, ns)

var mountAccessor string
m.coreStateLock.RLock()
mount := m.core.router.MatchingMountEntry(nsCtx, leaseID)
m.coreStateLock.RUnlock()

if mount == nil {
// figure out what this means - if we couldn't find the mount, can we automatically revoke
m.logger.Debug("could not find lease path", "lease_id", leaseID)
mountAccessor = "mount-accessor-not-found"
} else {
mountAccessor = mount.Accessor
}
mountAccessor := m.getLeaseMountAccessor(ctx, leaseID)

job, err := newRevocationJob(nsCtx, leaseID, ns, m)
if err != nil {
Expand Down Expand Up @@ -2418,6 +2415,142 @@ func (m *ExpirationManager) markLeaseIrrevocable(ctx context.Context, le *leaseE
m.nonexpiring.Delete(le.LeaseID)
}

func (m *ExpirationManager) getNamespaceFromLeaseID(ctx context.Context, leaseID string) (*namespace.Namespace, error) {
_, nsID := namespace.SplitIDFromString(leaseID)

// avoid re-declaring leaseNS and err with scope inside the if
leaseNS := namespace.RootNamespace
var err error
if nsID != "" {
leaseNS, err = NamespaceByID(ctx, nsID, m.core)
if err != nil {
return nil, err
}
}

return leaseNS, nil
}

func (m *ExpirationManager) getLeaseMountAccessor(ctx context.Context, leaseID string) string {
m.coreStateLock.RLock()
mount := m.core.router.MatchingMountEntry(ctx, leaseID)
m.coreStateLock.RUnlock()

var mountAccessor string
if mount == nil {
mountAccessor = "mount-accessor-not-found"
} else {
mountAccessor = mount.Accessor
}

return mountAccessor
}

// TODO SW if keep counts as a map, should update the RFC
func (m *ExpirationManager) getIrrevocableLeaseCounts(ctx context.Context, includeChildNamespaces bool) (map[string]interface{}, error) {
requestNS, err := namespace.FromContext(ctx)
if err != nil {
m.logger.Error("could not get namespace from context", "error", err)
return nil, err
}

numMatchingLeasesPerMount := make(map[string]int)
numMatchingLeases := 0
m.irrevocable.Range(func(k, v interface{}) bool {
leaseID := k.(string)
leaseNS, err := m.getNamespaceFromLeaseID(ctx, leaseID)
if err != nil {
// We should probably note that an error occured, but continue counting
m.logger.Warn("could not get lease namespace from ID", "error", err)
return true
}

leaseMatches := (leaseNS == requestNS) || (includeChildNamespaces && leaseNS.HasParent(requestNS))
if !leaseMatches {
// the lease doesn't meet our criteria, so keep looking
return true
}

mountAccessor := m.getLeaseMountAccessor(ctx, leaseID)

if _, ok := numMatchingLeasesPerMount[mountAccessor]; !ok {
numMatchingLeasesPerMount[mountAccessor] = 0
}

numMatchingLeases++
numMatchingLeasesPerMount[mountAccessor]++

return true
})

resp := make(map[string]interface{})
resp["lease_count"] = numMatchingLeases
resp["counts"] = numMatchingLeasesPerMount

return resp, nil
}

type leaseResponse struct {
LeaseID string `json:"lease_id"`
ErrMsg string `json:"error"`
}

func (m *ExpirationManager) listIrrevocableLeases(ctx context.Context, includeChildNamespaces, force bool) (map[string]interface{}, error) {
requestNS, err := namespace.FromContext(ctx)
if err != nil {
m.logger.Error("could not get namespace from context", "error", err)
return nil, err
}

// map of mount point : lease info
matchingLeasesPerMount := make(map[string][]*leaseResponse)
numMatchingLeases := 0
var retErr error
m.irrevocable.Range(func(k, v interface{}) bool {
Copy link
Member

Choose a reason for hiding this comment

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

One downside to using a sync.Map here is that the range scans will be unordered. We could sort the leases before returning them, but in the case where there are > MaxIrrevocableLeasesToReturn leases we may return a different set of leases each call. For now should we sort the values before returning them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, i think there's benefit to that consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've also thought that getting away from sync.Map could be nice so that when there are >10,000 results we can tell the client how many to expect (without doing the work of going through all of them)... a story for another day perhaps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

leaseID := k.(string)
leaseInfo := v.(*leaseEntry)

leaseNS, err := m.getNamespaceFromLeaseID(ctx, leaseID)
if err != nil {
// We probably want to track that an error occured, but continue counting
m.logger.Warn("could not get lease namespace from ID", "error", err)
return true
}

leaseMatches := (leaseNS == requestNS) || (includeChildNamespaces && leaseNS.HasParent(requestNS))
if !leaseMatches {
// the lease doesn't meet our criteria, so keep looking
return true
}

if !force && (numMatchingLeases >= MaxIrrevocableLeasesToReturn) {
m.logger.Warn("hit max irrevocable leases without force flag set")
retErr = errHitMaxIrrevocableLeases
return false
}

mountAccessor := m.getLeaseMountAccessor(ctx, leaseID)

numMatchingLeases++
matchingLeasesPerMount[mountAccessor] = append(matchingLeasesPerMount[mountAccessor], &leaseResponse{
swayne275 marked this conversation as resolved.
Show resolved Hide resolved
LeaseID: leaseID,
ErrMsg: leaseInfo.RevokeErr,
})

return true
})

if retErr != nil {
return nil, retErr
swayne275 marked this conversation as resolved.
Show resolved Hide resolved
}

resp := make(map[string]interface{})
resp["lease_count"] = numMatchingLeases
resp["leases"] = matchingLeasesPerMount

return resp, nil
}

// leaseEntry is used to structure the values the expiration
// manager stores. This is used to handle renew and revocation.
type leaseEntry struct {
Expand Down
185 changes: 185 additions & 0 deletions vault/expiration_test.go
Expand Up @@ -3005,3 +3005,188 @@ func TestExpiration_unrecoverableErrorMakesIrrevocable(t *testing.T) {
}
}
}

// set up multiple mounts, and return a mapping of the path to the mount accessor
func mountNoopBackends(t *testing.T, c *Core, paths []string) map[string]string {
t.Helper()

// enable the noop backend
c.logicalBackends["noop"] = func(ctx context.Context, config *logical.BackendConfig) (logical.Backend, error) {
return &NoopBackend{}, nil
}

pathToMount := make(map[string]string)
for _, path := range paths {
me := &MountEntry{
Table: mountTableType,
Path: path,
Type: "noop",
}
err := c.mount(namespace.RootContext(nil), me)
if err != nil {
t.Fatalf("err mounting backend %s: %v", path, err)
}

mount := c.router.MatchingMountEntry(namespace.RootContext(nil), path)
if mount == nil {
t.Fatalf("couldn't find mount for path %s", path)
}
pathToMount[path] = mount.Accessor
}

return pathToMount
}

func TestExpiration_getIrrevocableLeaseCounts(t *testing.T) {
c, _, _ := TestCoreUnsealed(t)

mountPrefixes := []string{"foo/bar/1/", "foo/bar/2/", "foo/bar/3/"}
pathToMount := mountNoopBackends(t, c, mountPrefixes)

exp := c.expiration

expectedPerMount := 10
for i := 0; i < expectedPerMount; i++ {
for _, mountPrefix := range mountPrefixes {
addIrrevocableLease(t, exp, mountPrefix, namespace.RootNamespace)
}
}

out, err := exp.getIrrevocableLeaseCounts(namespace.RootContext(nil), false)
if err != nil {
t.Fatalf("error getting irrevocable lease counts: %v", err)
}

countRaw, ok := out["lease_count"]
if !ok {
t.Fatal("no lease count")
}

countPerMountRaw, ok := out["counts"]
if !ok {
t.Fatal("no count per mount")
}

count := countRaw.(int)
countPerMount := countPerMountRaw.(map[string]int)

expectedCount := len(mountPrefixes) * expectedPerMount
if count != expectedCount {
t.Errorf("bad count. expected %d, got %d", expectedCount, count)
}

if len(countPerMount) != len(mountPrefixes) {
t.Fatalf("bad mounts. got %#v, expected %v", countPerMount, mountPrefixes)
}

for _, mountPrefix := range mountPrefixes {
mountCount := countPerMount[pathToMount[mountPrefix]]
if mountCount != expectedPerMount {
t.Errorf("bad count for prefix %q. expected %d, got %d", mountPrefix, expectedPerMount, mountCount)
}
}
}

func TestExpiration_listIrrevocableLeases(t *testing.T) {
c, _, _ := TestCoreUnsealed(t)

mountPrefixes := []string{"foo/bar/1/", "foo/bar/2/", "foo/bar/3/"}
pathToMount := mountNoopBackends(t, c, mountPrefixes)

exp := c.expiration

expectedLeasesPerMount := make(map[string][]string)
expectedPerMount := 10
for i := 0; i < expectedPerMount; i++ {
for _, mountPrefix := range mountPrefixes {
leaseID := addIrrevocableLease(t, exp, mountPrefix, namespace.RootNamespace)
expectedLeasesPerMount[mountPrefix] = append(expectedLeasesPerMount[mountPrefix], leaseID)
}
}

out, err := exp.listIrrevocableLeases(namespace.RootContext(nil), false, false)
if err != nil {
t.Fatalf("error listing irrevocable leases: %v", err)
}

countRaw, ok := out["lease_count"]
if !ok {
t.Fatal("no lease count")
}

leasesRaw, ok := out["leases"]
if !ok {
t.Fatal("no leases")
}

count := countRaw.(int)
leases := leasesRaw.(map[string][]*leaseResponse)

expectedCount := len(mountPrefixes) * expectedPerMount
if count != expectedCount {
t.Errorf("bad count. expected %d, got %d", expectedCount, count)
}

for _, mountPrefix := range mountPrefixes {
mount := pathToMount[mountPrefix]

// sort both sets of data for easy comparison
sort.Strings(expectedLeasesPerMount[mountPrefix])
sort.Slice(leases[mount], func(i, j int) bool {
return leases[mount][i].LeaseID < leases[mount][j].LeaseID
})

if len(leases[mount]) != expectedPerMount {
t.Fatalf("bad leases for mount prefix %q: expected %d, got %d", mountPrefix, expectedPerMount, len(leases[mount]))
}

for i, v := range expectedLeasesPerMount[mountPrefix] {
lease := leases[mount][i]
if lease.LeaseID != v {
t.Errorf("bad leaseID for mount prefix %q: expected %s, got %s", mountPrefix, v, lease.LeaseID)
}
if lease.ErrMsg == "" {
t.Errorf("no error message for irrevocable leaseID %q", lease.LeaseID)
}
}
}
}

func TestExpiration_listIrrevocableLeases_force(t *testing.T) {
c, _, _ := TestCoreUnsealed(t)
exp := c.expiration

expectedNumLeases := MaxIrrevocableLeasesToReturn + 10
for i := 0; i < expectedNumLeases; i++ {
addIrrevocableLease(t, exp, "foo/", namespace.RootNamespace)
}

dataRaw, err := exp.listIrrevocableLeases(namespace.RootContext(nil), false, false)
if err == nil {
t.Fatalf("expected error - more than max number of irrevocable leases")
}
if dataRaw != nil {
t.Fatalf("expected nil data, got %#v", dataRaw)
}

dataRaw, err = exp.listIrrevocableLeases(namespace.RootContext(nil), false, true)
if err != nil {
t.Fatalf("got error on force list leases: %v", err)
}
if dataRaw == nil {
t.Fatalf("got nil data on force list leases")
}

numLeasesRaw, ok := dataRaw["lease_count"]
if !ok {
t.Fatalf("lease count data not present")
}
if numLeasesRaw == nil {
t.Fatalf("nil lease count")
}

numLeases := numLeasesRaw.(int)
if numLeases != expectedNumLeases {
t.Errorf("bad lease count. expected %d, got %d", expectedNumLeases, numLeases)
}
}