Skip to content

Commit c45fff0

Browse files
authored
feat(runs): accept a "search" ListRuns filter matching run name or task name (#8035)
* runs: accept a "search" ListRuns filter matching run name or task name The console's Runs page search box sends a single `task_name CONTAINS` filter, so searching by run ID never worked. The Filter proto has no OR and ListRuns ANDs every filter, so the console cannot express "run name OR task name" on its own. Add a synthetic "search" filter field, resolved by the run service before the generic column allow-list conversion sees the request: search CONTAINS_CASE_INSENSITIVE x -> (LOWER(run_name) LIKE LOWER('%x%')) OR (LOWER(task_name) LIKE LOWER('%x%')) search CONTAINS x -> (run_name LIKE '%x%') OR (task_name LIKE '%x%') search EQUAL x -> (run_name = x) OR (task_name = x) Any other function, or a missing, empty or multi-value term, is an InvalidArgument. No IDL change: Filter.field is a free-form string validated server-side. The Union cloud run service accepts the same field, so one console request works against both backends; the console change follows separately and must ship after this. WatchGroups is left alone: the OSS grouped view does not consume request filters at all. * make devbox-build Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com> --------- Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>
1 parent 05b0655 commit c45fff0

6 files changed

Lines changed: 315 additions & 89 deletions

File tree

docker/devbox-bundled/manifests/complete.yaml

Lines changed: 0 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -7110,17 +7110,6 @@ metadata:
71107110
namespace: flyte
71117111
---
71127112
apiVersion: v1
7113-
kind: ServiceAccount
7114-
metadata:
7115-
labels:
7116-
app.kubernetes.io/instance: flyte-devbox
7117-
app.kubernetes.io/managed-by: Helm
7118-
app.kubernetes.io/name: flyteconnector
7119-
helm.sh/chart: flyteconnector-v2.0.0
7120-
name: flyteconnector
7121-
namespace: flyte
7122-
---
7123-
apiVersion: v1
71247113
automountServiceAccountToken: true
71257114
kind: ServiceAccount
71267115
metadata:
@@ -8372,30 +8361,6 @@ spec:
83728361
---
83738362
apiVersion: v1
83748363
kind: Service
8375-
metadata:
8376-
annotations:
8377-
projectcontour.io/upstream-protocol.h2c: grpc
8378-
labels:
8379-
app.kubernetes.io/instance: flyte-devbox
8380-
app.kubernetes.io/managed-by: Helm
8381-
app.kubernetes.io/name: flyteconnector
8382-
helm.sh/chart: flyteconnector-v2.0.0
8383-
name: flyteconnector
8384-
namespace: flyte
8385-
spec:
8386-
ports:
8387-
- appProtocol: TCP
8388-
name: connector-grpc
8389-
port: 8000
8390-
protocol: TCP
8391-
targetPort: connector-grpc
8392-
selector:
8393-
app.kubernetes.io/instance: flyte-devbox
8394-
app.kubernetes.io/name: flyteconnector
8395-
type: ClusterIP
8396-
---
8397-
apiVersion: v1
8398-
kind: Service
83998364
metadata:
84008365
labels:
84018366
app.kubernetes.io/name: embedded-postgresql
@@ -8877,59 +8842,6 @@ spec:
88778842
---
88788843
apiVersion: apps/v1
88798844
kind: Deployment
8880-
metadata:
8881-
labels:
8882-
app.kubernetes.io/instance: flyte-devbox
8883-
app.kubernetes.io/managed-by: Helm
8884-
app.kubernetes.io/name: flyteconnector
8885-
helm.sh/chart: flyteconnector-v2.0.0
8886-
name: flyteconnector
8887-
namespace: flyte
8888-
spec:
8889-
replicas: 1
8890-
selector:
8891-
matchLabels:
8892-
app.kubernetes.io/instance: flyte-devbox
8893-
app.kubernetes.io/name: flyteconnector
8894-
template:
8895-
metadata:
8896-
annotations: null
8897-
labels:
8898-
app.kubernetes.io/instance: flyte-devbox
8899-
app.kubernetes.io/managed-by: Helm
8900-
app.kubernetes.io/name: flyteconnector
8901-
helm.sh/chart: flyteconnector-v2.0.0
8902-
spec:
8903-
containers:
8904-
- command:
8905-
- c0
8906-
image: ghcr.io/flyteorg/flyte-connectors:py3.12-v2.3.6
8907-
imagePullPolicy: IfNotPresent
8908-
name: flyteconnector
8909-
ports:
8910-
- containerPort: 8000
8911-
name: connector-grpc
8912-
readinessProbe:
8913-
grpc:
8914-
port: 8000
8915-
initialDelaySeconds: 1
8916-
periodSeconds: 3
8917-
resources:
8918-
limits:
8919-
cpu: 500m
8920-
ephemeral-storage: 200Mi
8921-
memory: 300Mi
8922-
requests:
8923-
cpu: 500m
8924-
ephemeral-storage: 200Mi
8925-
memory: 200Mi
8926-
securityContext:
8927-
allowPrivilegeEscalation: false
8928-
securityContext: {}
8929-
serviceAccountName: flyteconnector
8930-
---
8931-
apiVersion: apps/v1
8932-
kind: Deployment
89338845
metadata:
89348846
labels:
89358847
app.kubernetes.io/instance: flyte-devbox

runs/repository/impl/action_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,57 @@ func TestListRuns_HasPausedActionFilter(t *testing.T) {
488488
assert.False(t, runs[0].ParentActionName.Valid, "only the root action should be returned")
489489
}
490490

491+
// TestListRuns_SearchFilter runs the console's run search predicate against a real
492+
// database: one term must match either the run name or the task name, case-insensitively,
493+
// and EQUAL on a full run name must select exactly that run.
494+
func TestListRuns_SearchFilter(t *testing.T) {
495+
db := setupActionDB(t)
496+
defer func() { db.Exec("DELETE FROM actions") }()
497+
actionRepo, err := NewActionRepo(db, testDbConfig)
498+
require.NoError(t, err)
499+
ctx := context.Background()
500+
501+
// Run names are unrelated to task names so a match is attributable to one column.
502+
for _, a := range []*models.Action{
503+
{Project: "proj1", Domain: "domain1", RunName: "runone-abc", Name: rootActionName,
504+
TaskName: sql.NullString{String: "env1.searchfn", Valid: true},
505+
Phase: int32(common.ActionPhase_ACTION_PHASE_RUNNING)},
506+
{Project: "proj1", Domain: "domain1", RunName: "runtwo-abc", Name: rootActionName,
507+
TaskName: sql.NullString{String: "env2.otherfn", Valid: true},
508+
Phase: int32(common.ActionPhase_ACTION_PHASE_RUNNING)},
509+
} {
510+
_, err := actionRepo.CreateAction(ctx, a, false)
511+
require.NoError(t, err)
512+
}
513+
514+
search := func(t *testing.T, fn common.Filter_Function, term string) []string {
515+
t.Helper()
516+
filter, err := NewSearchFilter(fn, []string{term})
517+
require.NoError(t, err)
518+
runs, err := actionRepo.ListActions(ctx, interfaces.ListResourceInput{
519+
Filter: NewIsRootActionFilter().And(filter),
520+
Limit: 50,
521+
})
522+
require.NoError(t, err)
523+
names := make([]string, 0, len(runs))
524+
for _, r := range runs {
525+
names = append(names, r.RunName)
526+
}
527+
return names
528+
}
529+
530+
assert.ElementsMatch(t, []string{"runone-abc"}, search(t, common.Filter_CONTAINS_CASE_INSENSITIVE, "RUNONE"),
531+
"matches the run name case-insensitively")
532+
assert.ElementsMatch(t, []string{"runone-abc"}, search(t, common.Filter_CONTAINS_CASE_INSENSITIVE, "searchfn"),
533+
"matches the task name")
534+
assert.ElementsMatch(t, []string{"runone-abc", "runtwo-abc"}, search(t, common.Filter_CONTAINS_CASE_INSENSITIVE, "abc"),
535+
"matches either column across runs")
536+
assert.ElementsMatch(t, []string{"runtwo-abc"}, search(t, common.Filter_EQUAL, "runtwo-abc"),
537+
"EQUAL selects exactly the named run")
538+
assert.Empty(t, search(t, common.Filter_CONTAINS_CASE_INSENSITIVE, "nomatch"),
539+
"a term matching neither column returns nothing")
540+
}
541+
491542
// TestListActions_KeysetPagination covers the O(n) keyset paging used by the
492543
// WatchActions snapshot: pages continue after the previous page's (created_at, name)
493544
// instead of by OFFSET. It forces tied created_at (the bulk-created map-task case) so

runs/repository/impl/filters.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,69 @@ func NewDeployedByFilter(deployedBy string) interfaces.Filter {
263263
return NewEqualFilter("deployed_by", deployedBy)
264264
}
265265

266+
// SearchField is the synthetic filter field behind the console's run search box. A filter
267+
// on it matches runs whose run name OR task name satisfies the predicate, which the flat,
268+
// AND-ed common.Filter list cannot express on its own. It is not a column: the run service
269+
// resolves it with SplitSearchFilters before the generic allow-list conversion
270+
// (ConvertProtoFilters) sees the request. The Union cloud run service accepts the same
271+
// field, so one console request works against both backends.
272+
const SearchField = "search"
273+
274+
// searchFunctions maps the proto filter functions accepted on SearchField to their SQL
275+
// expression. Contains (case-sensitive or not) is what the search box sends; EQUAL lets a
276+
// caller holding a full run name match it exactly instead of with a wildcard scan.
277+
var searchFunctions = map[common.Filter_Function]interfaces.FilterExpression{
278+
common.Filter_EQUAL: interfaces.FilterExpressionEqual,
279+
common.Filter_CONTAINS: interfaces.FilterExpressionContains,
280+
common.Filter_CONTAINS_CASE_INSENSITIVE: interfaces.FilterExpressionContainsCaseInsensitive,
281+
}
282+
283+
// NewSearchFilter builds the `run_name <op> ? OR task_name <op> ?` predicate for a
284+
// SearchField filter. The OR is parenthesized by compositeFilter, so it composes safely
285+
// with the AND-ed scope filters callers add around it.
286+
func NewSearchFilter(fn common.Filter_Function, values []string) (interfaces.Filter, error) {
287+
expression, ok := searchFunctions[fn]
288+
if !ok {
289+
return nil, fmt.Errorf("unsupported filter function %s for field %q; expected EQUAL, CONTAINS or CONTAINS_CASE_INSENSITIVE", fn, SearchField)
290+
}
291+
if len(values) != 1 {
292+
return nil, fmt.Errorf("filter on field %q expects a single value, got %d", SearchField, len(values))
293+
}
294+
term := values[0]
295+
if term == "" {
296+
return nil, fmt.Errorf("filter on field %q requires a non-empty value", SearchField)
297+
}
298+
runName := &basicFilter{field: "run_name", expression: expression, value: term}
299+
taskName := &basicFilter{field: "task_name", expression: expression, value: term}
300+
return runName.Or(taskName), nil
301+
}
302+
303+
// SplitSearchFilters partitions proto filters into a single combined SearchField predicate
304+
// (nil when there is none; several are AND-ed) and the remaining plain column filters.
305+
// Callers pass the remaining filters to ConvertProtoFilters, which validates fields against
306+
// the column allow-list and would otherwise reject the synthetic field, and AND the returned
307+
// predicate in separately.
308+
func SplitSearchFilters(filters []*common.Filter) (interfaces.Filter, []*common.Filter, error) {
309+
var search interfaces.Filter
310+
remaining := make([]*common.Filter, 0, len(filters))
311+
for _, f := range filters {
312+
if f.GetField() != SearchField {
313+
remaining = append(remaining, f)
314+
continue
315+
}
316+
sf, err := NewSearchFilter(f.GetFunction(), f.GetValues())
317+
if err != nil {
318+
return nil, nil, err
319+
}
320+
if search == nil {
321+
search = sf
322+
} else {
323+
search = search.And(sf)
324+
}
325+
}
326+
return search, remaining, nil
327+
}
328+
266329
// ConvertProtoFilters converts proto filters to our Filter interfaces.
267330
// allowedColumns is checked to prevent SQL injection via user-supplied field names.
268331
func ConvertProtoFilters(protoFilters []*common.Filter, allowedColumns sets.Set[string]) (interfaces.Filter, error) {

runs/repository/impl/filters_test.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,3 +185,127 @@ func TestParseStringFilters_ValueInState(t *testing.T) {
185185
assert.Contains(t, fmt.Sprintf("%v", expr.Args[0]), "1")
186186
assert.Contains(t, fmt.Sprintf("%v", expr.Args[0]), "2")
187187
}
188+
189+
// TestNewSearchFilter verifies the run search predicate matches the term against the run
190+
// name OR the task name, using the SQL operator selected by the proto function.
191+
func TestNewSearchFilter(t *testing.T) {
192+
cases := []struct {
193+
name string
194+
fn common.Filter_Function
195+
query string
196+
wantArgs []interface{}
197+
}{
198+
{
199+
name: "contains case insensitive",
200+
fn: common.Filter_CONTAINS_CASE_INSENSITIVE,
201+
query: "(LOWER(run_name) LIKE LOWER(?)) OR (LOWER(task_name) LIKE LOWER(?))",
202+
wantArgs: []interface{}{"%abc%", "%abc%"},
203+
},
204+
{
205+
name: "contains case sensitive",
206+
fn: common.Filter_CONTAINS,
207+
query: "(run_name LIKE ?) OR (task_name LIKE ?)",
208+
wantArgs: []interface{}{"%abc%", "%abc%"},
209+
},
210+
{
211+
name: "equal",
212+
fn: common.Filter_EQUAL,
213+
query: "(run_name = ?) OR (task_name = ?)",
214+
wantArgs: []interface{}{"abc", "abc"},
215+
},
216+
}
217+
for _, tt := range cases {
218+
t.Run(tt.name, func(t *testing.T) {
219+
filter, err := NewSearchFilter(tt.fn, []string{"abc"})
220+
require.NoError(t, err)
221+
222+
expr, err := filter.QueryExpression("")
223+
require.NoError(t, err)
224+
assert.Equal(t, tt.query, expr.Query)
225+
assert.Equal(t, tt.wantArgs, expr.Args)
226+
})
227+
}
228+
}
229+
230+
// TestNewSearchFilter_WithTablePrefix verifies both columns are table-qualified when the
231+
// predicate is rendered for a query that aliases the actions table.
232+
func TestNewSearchFilter_WithTablePrefix(t *testing.T) {
233+
filter, err := NewSearchFilter(common.Filter_EQUAL, []string{"abc"})
234+
require.NoError(t, err)
235+
236+
expr, err := filter.QueryExpression("actions")
237+
require.NoError(t, err)
238+
assert.Equal(t, "(actions.run_name = ?) OR (actions.task_name = ?)", expr.Query)
239+
}
240+
241+
// TestNewSearchFilter_Errors verifies the search field rejects operators that make no sense
242+
// for a free-text search and malformed value lists, so a bad request fails loudly instead
243+
// of silently matching everything.
244+
func TestNewSearchFilter_Errors(t *testing.T) {
245+
cases := []struct {
246+
name string
247+
fn common.Filter_Function
248+
values []string
249+
}{
250+
{name: "unsupported function", fn: common.Filter_GREATER_THAN, values: []string{"abc"}},
251+
{name: "value in is not a search", fn: common.Filter_VALUE_IN, values: []string{"a", "b"}},
252+
{name: "no values", fn: common.Filter_CONTAINS_CASE_INSENSITIVE, values: nil},
253+
{name: "multiple values", fn: common.Filter_CONTAINS_CASE_INSENSITIVE, values: []string{"a", "b"}},
254+
{name: "empty term", fn: common.Filter_CONTAINS_CASE_INSENSITIVE, values: []string{""}},
255+
}
256+
for _, tt := range cases {
257+
t.Run(tt.name, func(t *testing.T) {
258+
filter, err := NewSearchFilter(tt.fn, tt.values)
259+
require.Error(t, err)
260+
assert.Nil(t, filter)
261+
})
262+
}
263+
}
264+
265+
// TestSplitSearchFilters verifies the "search" field is pulled out of the filter list and
266+
// resolved to the run-name-or-task-name predicate, while plain column filters pass through
267+
// for ConvertProtoFilters.
268+
func TestSplitSearchFilters(t *testing.T) {
269+
filters := []*common.Filter{
270+
{Function: common.Filter_EQUAL, Field: "phase", Values: []string{"2"}},
271+
{Function: common.Filter_CONTAINS_CASE_INSENSITIVE, Field: SearchField, Values: []string{"abc"}},
272+
}
273+
274+
search, remaining, err := SplitSearchFilters(filters)
275+
require.NoError(t, err)
276+
277+
require.Len(t, remaining, 1)
278+
assert.Equal(t, "phase", remaining[0].GetField())
279+
280+
require.NotNil(t, search)
281+
expr, err := search.QueryExpression("")
282+
require.NoError(t, err)
283+
assert.Equal(t, "(LOWER(run_name) LIKE LOWER(?)) OR (LOWER(task_name) LIKE LOWER(?))", expr.Query)
284+
assert.Equal(t, []interface{}{"%abc%", "%abc%"}, expr.Args)
285+
}
286+
287+
// TestSplitSearchFilters_NoSearch verifies a nil predicate is returned when there is no
288+
// search filter, so the caller skips the extra AND and leaves the request untouched.
289+
func TestSplitSearchFilters_NoSearch(t *testing.T) {
290+
filters := []*common.Filter{
291+
{Function: common.Filter_EQUAL, Field: "phase", Values: []string{"2"}},
292+
}
293+
294+
search, remaining, err := SplitSearchFilters(filters)
295+
require.NoError(t, err)
296+
assert.Nil(t, search)
297+
assert.Len(t, remaining, 1)
298+
}
299+
300+
// TestSplitSearchFilters_Error verifies a malformed search filter fails the whole split, so
301+
// the handler returns InvalidArgument instead of dropping the predicate.
302+
func TestSplitSearchFilters_Error(t *testing.T) {
303+
filters := []*common.Filter{
304+
{Function: common.Filter_GREATER_THAN, Field: SearchField, Values: []string{"abc"}},
305+
}
306+
307+
search, remaining, err := SplitSearchFilters(filters)
308+
require.Error(t, err)
309+
assert.Nil(t, search)
310+
assert.Nil(t, remaining)
311+
}

runs/service/run_service.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -947,8 +947,24 @@ func (s *RunService) ListRuns(
947947
scopeFilter = scopeFilter.And(impl.NewHasPausedActionFilter())
948948
}
949949

950+
// The console's run search box arrives as a filter on the synthetic "search" field
951+
// (run name OR task name). Resolve it here and AND it onto the scope filter: the
952+
// generic parser below validates fields against the action column allow-list and
953+
// would reject it.
954+
listReq := req.Msg.Request
955+
searchFilter, remainingFilters, err := impl.SplitSearchFilters(listReq.GetFilters())
956+
if err != nil {
957+
return nil, connect.NewError(connect.CodeInvalidArgument, err)
958+
}
959+
if searchFilter != nil {
960+
scopeFilter = scopeFilter.And(searchFilter)
961+
// Clone so the caller's request is not mutated when the search filter is stripped.
962+
listReq = proto.Clone(listReq).(*common.ListRequest)
963+
listReq.Filters = remainingFilters
964+
}
965+
950966
// Parse pagination, sort, and user-supplied filters from the common ListRequest.
951-
listInput, err := impl.NewListResourceInputFromProto(req.Msg.Request, models.ActionColumnsSet)
967+
listInput, err := impl.NewListResourceInputFromProto(listReq, models.ActionColumnsSet)
952968
if err != nil {
953969
return nil, connect.NewError(connect.CodeInvalidArgument, err)
954970
}

0 commit comments

Comments
 (0)