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

Define Backend.GetRange to be inclusive #50996

Merged
merged 6 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 1 deletion lib/backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ type Backend interface {
// Get returns a single item or not found error
Get(ctx context.Context, key Key) (*Item, error)

// GetRange returns query range
// GetRange returns the items between the start and end keys, including both
// (if present).
GetRange(ctx context.Context, startKey, endKey Key, limit int) (*GetResult, error)

// Delete deletes item by key, returns NotFound error
Expand Down
7 changes: 5 additions & 2 deletions lib/backend/etcdbk/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,6 @@ type eventResult struct {
// effective, this strategy still suffers from a "head of line blocking"-esque issue since event order
// must be preserved.
func (b *EtcdBackend) watchEvents(ctx context.Context) error {

// etcd watch client relies on context cancellation for cleanup,
// so create a new subscope for this function.
ctx, cancel := context.WithCancel(ctx)
Expand Down Expand Up @@ -658,7 +657,11 @@ func (b *EtcdBackend) GetRange(ctx context.Context, startKey, endKey backend.Key
if endKey.IsZero() {
return nil, trace.BadParameter("missing parameter endKey")
}
opts := []clientv3.OpOption{clientv3.WithRange(b.prependPrefix(endKey))}
// etcd's range query includes the start point and excludes the end point,
// but Backend.GetRange is supposed to be inclusive at both ends, so we
// query until the very next key in lexicographic order (i.e., the same key
// followed by a 0 byte)
opts := []clientv3.OpOption{clientv3.WithRange(b.prependPrefix(endKey) + "\x00")}
if limit > 0 {
opts = append(opts, clientv3.WithLimit(int64(limit)))
}
Expand Down
14 changes: 0 additions & 14 deletions lib/backend/memory/item.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,7 @@ func (i *btreeItem) Less(iother btree.Item) bool {
switch other := iother.(type) {
case *btreeItem:
return i.Key.Compare(other.Key) < 0
case *prefixItem:
return !iother.Less(i)
default:
return false
}
}

// prefixItem is used for prefix matches on a B-Tree
type prefixItem struct {
// prefix is a prefix to match
prefix backend.Key
}

// Less is used for Btree operations
func (p *prefixItem) Less(iother btree.Item) bool {
other := iother.(*btreeItem)
return !other.Key.HasPrefix(p.prefix)
}
7 changes: 6 additions & 1 deletion lib/backend/memory/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,12 @@ func (m *Memory) NewWatcher(ctx context.Context, watch backend.Watch) (backend.W

func (m *Memory) getRange(ctx context.Context, startKey, endKey backend.Key, limit int) backend.GetResult {
var res backend.GetResult
m.tree.AscendRange(&btreeItem{Item: backend.Item{Key: startKey}}, &btreeItem{Item: backend.Item{Key: endKey}}, func(item *btreeItem) bool {
startItem := &btreeItem{Item: backend.Item{Key: startKey}}
endItem := &btreeItem{Item: backend.Item{Key: endKey}}
m.tree.AscendGreaterOrEqual(startItem, func(item *btreeItem) bool {
if endItem.Less(item) {
return false
}
res.Items = append(res.Items, item.Item)
if limit > 0 && len(res.Items) >= limit {
return false
Expand Down
5 changes: 5 additions & 0 deletions lib/backend/test/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,11 @@ func testQueryRange(t *testing.T, newBackend Constructor) {
require.NoError(t, err)
RequireItems(t, []backend.Item{c1, c2}, result.Items)

// item at the end of the range
result, err = uut.GetRange(ctx, prefix("prefix", "c", "c1"), prefix("prefix", "c", "c2"), backend.NoLimit)
require.NoError(t, err)
RequireItems(t, []backend.Item{c1, c2}, result.Items)

// pagination
result, err = uut.GetRange(ctx, prefix("prefix"), backend.RangeEnd(prefix("prefix")), 2)
require.NoError(t, err)
Expand Down
23 changes: 4 additions & 19 deletions lib/services/unified_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ func (c *UnifiedResourceCache) getSortTree(sortField string) (*btree.BTreeG[*ite
default:
return nil, trace.NotImplemented("sorting by %v is not supported in unified resources", sortField)
}

}

func (c *UnifiedResourceCache) getRange(ctx context.Context, startKey backend.Key, matchFn func(types.ResourceWithLabels) (bool, error), req *proto.ListUnifiedResourcesRequest) ([]resource, string, error) {
Expand Down Expand Up @@ -503,7 +502,6 @@ func (c *UnifiedResourceCache) getResourcesAndUpdateCurrent(ctx context.Context)
c.stale = false
c.defineCollectorAsInitialized()
return nil

}

// getNodes will get all nodes
Expand Down Expand Up @@ -595,7 +593,6 @@ func (c *UnifiedResourceCache) getSAMLApps(ctx context.Context) ([]types.SAMLIdP

for {
resp, nextKey, err := c.ListSAMLIdPServiceProviders(ctx, apidefaults.DefaultChunkSize, startKey)

if err != nil {
return nil, trace.Wrap(err, "getting SAML apps for unified resource watcher")
}
Expand Down Expand Up @@ -818,25 +815,11 @@ func (i *item) Less(iother btree.Item) bool {
switch other := iother.(type) {
case *item:
return i.Key.Compare(other.Key) < 0
case *prefixItem:
return !iother.Less(i)
default:
return false
}
}

// prefixItem is used for prefix matches on a B-Tree
type prefixItem struct {
// prefix is a prefix to match
prefix backend.Key
}

// Less is used for Btree operations
func (p *prefixItem) Less(iother btree.Item) bool {
other := iother.(*item)
return !other.Key.HasPrefix(p.prefix)
}

type resource interface {
types.ResourceWithLabels
CloneResource() types.ResourceWithLabels
Expand Down Expand Up @@ -946,7 +929,8 @@ func MakePaginatedResource(ctx context.Context, requestType string, r types.Reso
AppServer: appOrSP,
},
},
}, RequiresRequest: requiresRequest}
}, RequiresRequest: requiresRequest,
}
case *types.SAMLIdPServiceProviderV1:
protoResource = &proto.PaginatedResource{
Resource: &proto.PaginatedResource_AppServerOrSAMLIdPServiceProvider{
Expand All @@ -955,7 +939,8 @@ func MakePaginatedResource(ctx context.Context, requestType string, r types.Reso
SAMLIdPServiceProvider: appOrSP,
},
},
}, RequiresRequest: requiresRequest}
}, RequiresRequest: requiresRequest,
}
default:
return nil, trace.BadParameter("%s has invalid type %T", resourceKind, resource)
}
Expand Down
Loading