Skip to content

poc: Filter bot instances using predicate language #58496

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

Closed
nicholasmarais1158 wants to merge 1 commit into from
Closed

poc: Filter bot instances using predicate language #58496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 13 additions & 3 deletions api/gen/proto/go/teleport/machineid/v1/bot_instance_service.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions api/proto/teleport/machineid/v1/bot_instance_service.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ message ListBotInstancesRequest {
string filter_search_term = 4;
// The sort config to use for the results. If empty, the default sort field and order is used.
types.SortBy sort = 5;
// A query in Teleport predicate language used to filter the results.
string query = 6;
}

// Response for ListBotInstances.
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/authclient/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1330,7 +1330,7 @@ type Cache interface {
GetBotInstance(ctx context.Context, botName, instanceID string) (*machineidv1.BotInstance, error)

// ListBotInstances returns a page of BotInstance resources.
ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy) ([]*machineidv1.BotInstance, string, error)
ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy, query string) ([]*machineidv1.BotInstance, string, error)
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy Aug 29, 2025

Choose a reason for hiding this comment

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

@okraport has been working on standardizing our List APIs and might have some opinions on this

Copy link
Copy Markdown
Contributor

@okraport okraport Aug 29, 2025

Choose a reason for hiding this comment

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

The general idea is to bundle filter related fields into a proto message (example RoleFilter) so that it can be extended without the function signature growing like this.
Since sort and filter are optional we could then use a ...opts functional pattern for all List calls.

It's tricky to do this retroactively, I think in this instance it's understandable to extend the existing API. I will work on a design to standardise these and propose a path forward once I have something concrete. Until then, if we are adding a filter to a List api without preexisting filter terms, let's use a proto message. If we are extending (as is the case here) then it may be necessary evil until we have a backwards compatible migration path.

Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy Aug 29, 2025

Choose a reason for hiding this comment

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

We could always freeze the current ListBotInstances API and add ListBotInstancesV2 which followed that pattern.

Copy link
Copy Markdown
Contributor Author

@nicholasmarais1158 nicholasmarais1158 Aug 29, 2025

Choose a reason for hiding this comment

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

I was thinking the same thing when I added query. I'm more than happy to create a new call that uses a message for non-paging related params (filter and sort).

Is the idea to create a V2 call to cater for the possibility that an older proxy version talks to a newer auth service? Are there any other steps that need to be taken to ensure backwards compatibility?


// ListProvisionTokens returns a paginated list of provision tokens.
ListProvisionTokens(ctx context.Context, pageSize int, pageToken string, anyRoles types.SystemRoles, botName string) ([]types.ProvisionToken, string, error)
Expand Down
4 changes: 2 additions & 2 deletions lib/auth/machineid/machineidv1/bot_instance_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type BotInstancesCache interface {
GetBotInstance(ctx context.Context, botName, instanceID string) (*pb.BotInstance, error)

// ListBotInstances returns a page of BotInstance resources.
ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy) ([]*pb.BotInstance, string, error)
ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy, query string) ([]*pb.BotInstance, string, error)
}

// BotInstanceServiceConfig holds configuration options for the BotInstance gRPC
Expand Down Expand Up @@ -157,7 +157,7 @@ func (b *BotInstanceService) ListBotInstances(ctx context.Context, req *pb.ListB
return nil, trace.Wrap(err)
}

res, nextToken, err := b.cache.ListBotInstances(ctx, req.FilterBotName, int(req.PageSize), req.PageToken, req.FilterSearchTerm, req.Sort)
res, nextToken, err := b.cache.ListBotInstances(ctx, req.FilterBotName, int(req.PageSize), req.PageToken, req.FilterSearchTerm, req.Sort, req.Query)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
55 changes: 18 additions & 37 deletions lib/cache/bot_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package cache

import (
"context"
"slices"
"strings"
"time"

"github.com/gravitational/trace"
Expand All @@ -31,6 +29,8 @@ import (
"github.com/gravitational/teleport/api/utils/clientutils"
"github.com/gravitational/teleport/lib/itertools/stream"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/services/local"
"github.com/gravitational/teleport/lib/utils/typical"
)

type botInstanceIndex string
Expand Down Expand Up @@ -85,7 +85,7 @@ func newBotInstanceCollection(upstream services.BotInstance, w types.WatchKind)
fetcher: func(ctx context.Context, loadSecrets bool) ([]*machineidv1.BotInstance, error) {
out, err := stream.Collect(clientutils.Resources(ctx,
func(ctx context.Context, limit int, start string) ([]*machineidv1.BotInstance, string, error) {
return upstream.ListBotInstances(ctx, "", limit, start, "", nil)
return upstream.ListBotInstances(ctx, "", limit, start, "", nil, "")
},
))
return out, trace.Wrap(err)
Expand Down Expand Up @@ -113,7 +113,7 @@ func (c *Cache) GetBotInstance(ctx context.Context, botName, instanceID string)
}

// ListBotInstances returns a page of BotInstance resources.
func (c *Cache) ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy) ([]*machineidv1.BotInstance, string, error) {
func (c *Cache) ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy, query string) ([]*machineidv1.BotInstance, string, error) {
ctx, span := c.Tracer.Start(ctx, "cache/ListBotInstances")
defer span.End()

Expand All @@ -135,17 +135,29 @@ func (c *Cache) ListBotInstances(ctx context.Context, botName string, pageSize i
}
}

var exp typical.Expression[*local.Environment, bool]
if query != "" {
parser, err := local.NewBotInstanceExpressionParser()
if err != nil {
return nil, "", trace.Wrap(err)
}
exp, err = parser.Parse(query)
if err != nil {
return nil, "", trace.Wrap(err)
}
}

lister := genericLister[*machineidv1.BotInstance, botInstanceIndex]{
cache: c,
collection: c.collections.botInstances,
index: index,
isDesc: isDesc,
defaultPageSize: defaults.DefaultChunkSize,
upstreamList: func(ctx context.Context, limit int, start string) ([]*machineidv1.BotInstance, string, error) {
return c.Config.BotInstanceService.ListBotInstances(ctx, botName, limit, start, search, sort)
return c.Config.BotInstanceService.ListBotInstances(ctx, botName, limit, start, search, sort, query)
},
filter: func(b *machineidv1.BotInstance) bool {
return matchBotInstance(b, botName, search)
return local.MatchBotInstance(b, botName, search, exp)
},
nextToken: func(b *machineidv1.BotInstance) string {
return keyFn(b)
Expand All @@ -157,34 +169,3 @@ func (c *Cache) ListBotInstances(ctx context.Context, botName string, pageSize i
)
return out, next, trace.Wrap(err)
}

func matchBotInstance(b *machineidv1.BotInstance, botName string, search string) bool {
// If updating this, ensure it's consistent with the upstream search logic in `lib/services/local/bot_instance.go`.

if botName != "" && b.Spec.BotName != botName {
return false
}

if search == "" {
return true
}

latestHeartbeats := b.GetStatus().GetLatestHeartbeats()
heartbeat := b.Status.InitialHeartbeat // Use initial heartbeat as a fallback
if len(latestHeartbeats) > 0 {
heartbeat = latestHeartbeats[len(latestHeartbeats)-1]
}

values := []string{
b.Spec.BotName,
b.Spec.InstanceId,
}

if heartbeat != nil {
values = append(values, heartbeat.Hostname, heartbeat.JoinMethod, heartbeat.Version, "v"+heartbeat.Version)
}

return slices.ContainsFunc(values, func(val string) bool {
return strings.Contains(strings.ToLower(val), strings.ToLower(search))
})
}
38 changes: 19 additions & 19 deletions lib/cache/bot_instance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,15 @@ func TestBotInstanceCache(t *testing.T) {
return p.cache.GetBotInstance(ctx, "bot-1", key)
},
cacheList: func(ctx context.Context) ([]*machineidv1.BotInstance, error) {
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil)
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil, "")
return results, err
},
create: func(ctx context.Context, resource *machineidv1.BotInstance) error {
_, err := p.botInstanceService.CreateBotInstance(ctx, resource)
return err
},
list: func(ctx context.Context) ([]*machineidv1.BotInstance, error) {
results, _, err := p.botInstanceService.ListBotInstances(ctx, "", 0, "", "", nil)
results, _, err := p.botInstanceService.ListBotInstances(ctx, "", 0, "", "", nil, "")
return results, err
},
update: func(ctx context.Context, bi *machineidv1.BotInstance) error {
Expand Down Expand Up @@ -107,13 +107,13 @@ func TestBotInstanceCachePaging(t *testing.T) {

// Let the cache catch up
require.EventuallyWithT(t, func(t *assert.CollectT) {
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil)
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil, "")
require.NoError(t, err)
require.Len(t, results, 5)
}, 10*time.Second, 100*time.Millisecond)

// page size equal to total items
results, nextPageToken, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil)
results, nextPageToken, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil, "")
require.NoError(t, err)
require.Empty(t, nextPageToken)
require.Len(t, results, 5)
Expand All @@ -124,7 +124,7 @@ func TestBotInstanceCachePaging(t *testing.T) {
require.Equal(t, "instance-5", results[4].GetMetadata().GetName())

// page size smaller than total items
results, nextPageToken, err = p.cache.ListBotInstances(ctx, "", 3, "", "", nil)
results, nextPageToken, err = p.cache.ListBotInstances(ctx, "", 3, "", "", nil, "")
require.NoError(t, err)
require.Equal(t, "bot-1/instance-4", nextPageToken)
require.Len(t, results, 3)
Expand All @@ -133,7 +133,7 @@ func TestBotInstanceCachePaging(t *testing.T) {
require.Equal(t, "instance-3", results[2].GetMetadata().GetName())

// next page
results, nextPageToken, err = p.cache.ListBotInstances(ctx, "", 3, nextPageToken, "", nil)
results, nextPageToken, err = p.cache.ListBotInstances(ctx, "", 3, nextPageToken, "", nil, "")
require.NoError(t, err)
require.Empty(t, nextPageToken)
require.Len(t, results, 2)
Expand Down Expand Up @@ -166,12 +166,12 @@ func TestBotInstanceCacheBotFilter(t *testing.T) {

// Let the cache catch up
require.EventuallyWithT(t, func(t *assert.CollectT) {
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil)
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil, "")
require.NoError(t, err)
require.Len(t, results, 10)
}, 10*time.Second, 100*time.Millisecond)

results, _, err := p.cache.ListBotInstances(ctx, "bot-1", 0, "", "", nil)
results, _, err := p.cache.ListBotInstances(ctx, "bot-1", 0, "", "", nil, "")
require.NoError(t, err)
require.Len(t, results, 5)

Expand Down Expand Up @@ -213,12 +213,12 @@ func TestBotInstanceCacheSearchFilter(t *testing.T) {

// Let the cache catch up
require.EventuallyWithT(t, func(t *assert.CollectT) {
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil)
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil, "")
require.NoError(t, err)
require.Len(t, results, 10)
}, 10*time.Second, 100*time.Millisecond)

results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "host-1", nil)
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "host-1", nil, "")
require.NoError(t, err)
require.Len(t, results, 5)
}
Expand Down Expand Up @@ -268,7 +268,7 @@ func TestBotInstanceCacheSorting(t *testing.T) {

// Let the cache catch up
require.EventuallyWithT(t, func(t *assert.CollectT) {
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil)
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil, "")
require.NoError(t, err)
require.Len(t, results, 3)
}, 10*time.Second, 100*time.Millisecond)
Expand All @@ -277,7 +277,7 @@ func TestBotInstanceCacheSorting(t *testing.T) {
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{
Field: "active_at_latest",
IsDesc: false,
})
}, "")
require.NoError(t, err)
require.Len(t, results, 3)
require.Equal(t, "instance-3", results[0].GetMetadata().GetName())
Expand All @@ -288,15 +288,15 @@ func TestBotInstanceCacheSorting(t *testing.T) {
results, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{
Field: "active_at_latest",
IsDesc: true,
})
}, "")
require.NoError(t, err)
require.Len(t, results, 3)
require.Equal(t, "instance-2", results[0].GetMetadata().GetName())
require.Equal(t, "instance-1", results[1].GetMetadata().GetName())
require.Equal(t, "instance-3", results[2].GetMetadata().GetName())

// sort ascending by bot_name
results, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", nil) // empty sort should default to `bot_name:asc`
results, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", nil, "") // empty sort should default to `bot_name:asc`
require.NoError(t, err)
require.Len(t, results, 3)
require.Equal(t, "instance-1", results[0].GetMetadata().GetName())
Expand All @@ -307,7 +307,7 @@ func TestBotInstanceCacheSorting(t *testing.T) {
results, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{
Field: "bot_name",
IsDesc: true,
})
}, "")
require.NoError(t, err)
require.Len(t, results, 3)
require.Equal(t, "instance-2", results[0].GetMetadata().GetName())
Expand Down Expand Up @@ -341,7 +341,7 @@ func TestBotInstanceCacheFallback(t *testing.T) {

// Let the cache catch up
require.EventuallyWithT(t, func(t *assert.CollectT) {
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil)
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil, "")
require.NoError(t, err)
require.Len(t, results, 1)
}, 10*time.Second, 100*time.Millisecond)
Expand All @@ -350,23 +350,23 @@ func TestBotInstanceCacheFallback(t *testing.T) {
results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{
Field: "bot_name",
IsDesc: false,
})
}, "")
require.NoError(t, err) // asc by bot_name is the only sort supported by the upstream
require.Len(t, results, 1)

// sort descending by bot_name
_, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{
Field: "bot_name",
IsDesc: true,
})
}, "")
require.Error(t, err)
require.Equal(t, "unsupported sort, only bot_name:asc is supported, but got \"bot_name\" (desc = true)", err.Error())

// sort ascending by active_at_latest
_, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{
Field: "active_at_latest",
IsDesc: false,
})
}, "")
require.Error(t, err)
require.Equal(t, "unsupported sort, only bot_name:asc is supported, but got \"active_at_latest\" (desc = false)", err.Error())

Expand Down
2 changes: 1 addition & 1 deletion lib/services/bot_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type BotInstance interface {
GetBotInstance(ctx context.Context, botName, instanceID string) (*machineidv1.BotInstance, error)

// ListBotInstances
ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy) ([]*machineidv1.BotInstance, string, error)
ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy, query string) ([]*machineidv1.BotInstance, string, error)

// DeleteBotInstance
DeleteBotInstance(ctx context.Context, botName, instanceID string) error
Expand Down
Loading
Loading