search

tektoncd / results

Long term storage of execution results.
Apache License 2.0
78 stars 74 forks source link

CEL filter with NOT operator with a group crashes the API server #498

Closed sayan-biswas closed 1 year ago

sayan-biswas commented 1 year ago

Expected Behaviour

The API server shouldn't panic. If the CEL expression is not valid it should throw an error in the response instead.

Actual Behaviour

The server panics with the following error. 

panic: runtime error: index out of range [1] with length 1

goroutine 60 [running]:
github.com/tektoncd/results/pkg/api/server/cel2sql.(*interpreter).interpretBinaryCallExpr(0x140002646f0, 0x14000116540)
        /Users/sayan/Code/github.com/tektoncd/results/pkg/api/server/cel2sql/interpreter.go:308 +0x6e0
github.com/tektoncd/results/pkg/api/server/cel2sql.(*interpreter).interpretCallExpr(0x30?, 0x14000076480?, 0x14000116540)
        /Users/sayan/Code/github.com/tektoncd/results/pkg/api/server/cel2sql/interpreter.go:279 +0x12c
github.com/tektoncd/results/pkg/api/server/cel2sql.(*interpreter).interpretExpr(0x14000760c01?, 0x14000116540?)
        /Users/sayan/Code/github.com/tektoncd/results/pkg/api/server/cel2sql/interpreter.go:77 +0xc0
github.com/tektoncd/results/pkg/api/server/cel2sql.(*interpreter).interpret(0x140002646f0)
        /Users/sayan/Code/github.com/tektoncd/results/pkg/api/server/cel2sql/interpreter.go:58 +0x28
github.com/tektoncd/results/pkg/api/server/cel2sql.Convert(0x14000202600?, {0x14000202600?, 0x1400051e300?})
        /Users/sayan/Code/github.com/tektoncd/results/pkg/api/server/cel2sql/convert.go:40 +0x16c
github.com/tektoncd/results/pkg/api/server/v1alpha2/lister.(*filter).build(0x1400052d650, 0x1400092b408?)
        /Users/sayan/Code/github.com/tektoncd/results/pkg/api/server/v1alpha2/lister/filter.go:60 +0x174
github.com/tektoncd/results/pkg/api/server/v1alpha2/lister.(*Lister[...]).buildQuery(0x1400052a3a0, {0x102719570?, 0x1400052d530?}, 0x10?)
        /Users/sayan/Code/github.com/tektoncd/results/pkg/api/server/v1alpha2/lister/lister.go:91 +0xd8
github.com/tektoncd/results/pkg/api/server/v1alpha2/lister.(*Lister[...]).List(0x1027238c0, {0x102719570, 0x1400052d530?}, 0x1?)
        /Users/sayan/Code/github.com/tektoncd/results/pkg/api/server/v1alpha2/lister/lister.go:104 +0x38
github.com/tektoncd/results/pkg/api/server/v1alpha2.(*Server).ListRecords(0x140005c7cc0, {0x102719570, 0x1400052d530}, 0x140000227e0)
        /Users/sayan/Code/github.com/tektoncd/results/pkg/api/server/v1alpha2/records.go:160 +0x104
github.com/tektoncd/results/proto/v1alpha2/results_go_proto._Results_ListRecords_Handler.func1({0x102719570, 0x1400052d530}, {0x102591ee0?, 0x140000227e0})
        /Users/sayan/Code/github.com/tektoncd/results/proto/v1alpha2/results_go_proto/api_grpc.pb.go:357 +0x74
github.com/grpc-ecosystem/go-grpc-prometheus.(*ServerMetrics).UnaryServerInterceptor.func1({0x102719570, 0x1400052d530}, {0x102591ee0, 0x140000227e0}, 0x14000350801?, 0x140008600d8)
        /Users/sayan/Code/github.com/tektoncd/results/vendor/github.com/grpc-ecosystem/go-grpc-prometheus/server_metrics.go:107 +0x74
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x102719570?, 0x1400052d530?}, {0x102591ee0?, 0x140000227e0?})
        /Users/sayan/Code/github.com/tektoncd/results/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:25 +0x3c
github.com/grpc-ecosystem/go-grpc-middleware/auth.UnaryServerInterceptor.func1({0x102719570, 0x1400052d530}, {0x102591ee0, 0x140000227e0}, 0x140002ae080, 0x140002ae0a0)
        /Users/sayan/Code/github.com/tektoncd/results/vendor/github.com/grpc-ecosystem/go-grpc-middleware/auth/auth.go:47 +0xbc
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x102719570?, 0x1400052d530?}, {0x102591ee0?, 0x140000227e0?})
        /Users/sayan/Code/github.com/tektoncd/results/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:25 +0x3c
github.com/grpc-ecosystem/go-grpc-middleware/logging/zap.UnaryServerInterceptor.func1({0x102719570, 0x1400052d4d0}, {0x102591ee0, 0x140000227e0}, 0x140002ae080, 0x140002ae0e0)
        /Users/sayan/Code/github.com/tektoncd/results/vendor/github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/server_interceptors.go:31 +0xbc
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x102719570?, 0x1400052d4d0?}, {0x102591ee0?, 0x140000227e0?})
        /Users/sayan/Code/github.com/tektoncd/results/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:25 +0x3c
github.com/grpc-ecosystem/go-grpc-middleware/tags.UnaryServerInterceptor.func1({0x102719570?, 0x1400052d470?}, {0x102591ee0, 0x140000227e0}, 0x140002ae080, 0x140002ae100)
        /Users/sayan/Code/github.com/tektoncd/results/vendor/github.com/grpc-ecosystem/go-grpc-middleware/tags/interceptors.go:23 +0x84
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x102719570?, 0x1400052d470?}, {0x102591ee0?, 0x140000227e0?})
        /Users/sayan/Code/github.com/tektoncd/results/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:25 +0x3c
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1({0x102719570, 0x1400052d470}, {0x102591ee0, 0x140000227e0}, 0x14000895a38?, 0x100aa6fa8?)
        /Users/sayan/Code/github.com/tektoncd/results/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:34 +0xb8
github.com/tektoncd/results/proto/v1alpha2/results_go_proto._Results_ListRecords_Handler({0x1026428e0?, 0x140005c7cc0}, {0x102719570, 0x1400052d470}, 0x14000022770, 0x1400083d0b0)
        /Users/sayan/Code/github.com/tektoncd/results/proto/v1alpha2/results_go_proto/api_grpc.pb.go:359 +0x138
google.golang.org/grpc.(*Server).processUnaryRPC(0x14000846000, {0x102723b60, 0x14000762000}, 0x140008526c0, 0x1400083d230, 0x103848980, 0x0)
        /Users/sayan/Code/github.com/tektoncd/results/vendor/google.golang.org/grpc/server.go:1337 +0xc90
google.golang.org/grpc.(*Server).handleStream(0x14000846000, {0x102723b60, 0x14000762000}, 0x140008526c0, 0x0)
        /Users/sayan/Code/github.com/tektoncd/results/vendor/google.golang.org/grpc/server.go:1714 +0x82c
google.golang.org/grpc.(*Server).serveStreams.func1.1()
        /Users/sayan/Code/github.com/tektoncd/results/vendor/google.golang.org/grpc/server.go:959 +0x84
created by google.golang.org/grpc.(*Server).serveStreams.func1
        /Users/sayan/Code/github.com/tektoncd/results/vendor/google.golang.org/grpc/server.go:957 +0x16c

Steps to Reproduce the Problem

Using the CEL expression with ! (NOT) operator with group ()

Example: !(data.metadata.annotations.contains('foo/bar'))

alan-ghelardi commented 1 year ago

Thank you for catching the issue @sayan-biswas. I'll look into that. /assign

avinal commented 1 year ago

Hello @alan-ghelardi, this issue has been open for quite some time. If you are not currently working on this issue, I would be happy to take over.

alan-ghelardi commented 1 year ago

@avinal I haven't yet managed to start working on that directly. I started adding support for the has macro, which indirectly provides support for verifying whether a map contains a key, but haven't finished yet. If you want to look into this issue, I'd be glad to review the pull request.

avinal commented 1 year ago

Thanks Alan, I will start working on it then. /assign