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

celmatcher.go: cannot use func #5797

Closed
greenpau opened this issue Sep 4, 2023 · 12 comments
Closed

celmatcher.go: cannot use func #5797

greenpau opened this issue Sep 4, 2023 · 12 comments
Labels
invalid ❓ This doesn't seem right

Comments

@greenpau
Copy link

greenpau commented Sep 4, 2023

@mholt , upgrading to 2.7.4. Please advise.

go.mod follows:

module github.com/greenpau/caddy-trace

go 1.20

require (
    github.com/caddyserver/caddy/v2 v2.7.4
    github.com/google/uuid v1.3.1
    go.uber.org/zap v1.25.0
)
$ go version
go version go1.20.7 linux/amd64

The test fail with the following errors:

BUILD| github.com/caddyserver/caddy/v2/modules/caddyhttp
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:462:9: cannot use func(eh parser.ExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {…} (value of type func(eh parser.ExprHelper, target *expr.Expr, args []*expr.Expr) (*expr.Expr, *common.Error)) as parser.MacroExpander value in return statement
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:479:13: eh.GlobalCall undefined (type parser.ExprHelper has no field or method GlobalCall)
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:479:37: eh.Ident undefined (type parser.ExprHelper has no field or method Ident)
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:479:66: cannot use matchArgs (variable of type []*expr.Expr) as []ast.Expr value in argument to eh.NewList
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:488:9: cannot use func(eh parser.ExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {…} (value of type func(eh parser.ExprHelper, target *expr.Expr, args []*expr.Expr) (*expr.Expr, *common.Error)) as parser.MacroExpander value in return statement
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:495:14: eh.GlobalCall undefined (type parser.ExprHelper has no field or method GlobalCall)
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:495:38: eh.Ident undefined (type parser.ExprHelper has no field or method Ident)
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:509:9: cannot use func(eh parser.ExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {…} (value of type func(eh parser.ExprHelper, target *expr.Expr, args []*expr.Expr) (*expr.Expr, *common.Error)) as parser.MacroExpander value in return statement
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:545:14: eh.GlobalCall undefined (type parser.ExprHelper has no field or method GlobalCall)
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:545:38: eh.Ident undefined (type parser.ExprHelper has no field or method Ident)
     | ../../../../pkg/mod/github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/celmatcher.go:545:38: too many errors
@mholt
Copy link
Member

mholt commented Sep 4, 2023

Am mobile currently, but what happens if you run go mod tidy and then run the tests?

@greenpau
Copy link
Author

greenpau commented Sep 4, 2023

@mholt , the cleaning of cache and tidying does not help.

@mohammed90
Copy link
Member

mohammed90 commented Sep 4, 2023

This commit from google/cel-go broke it:

google/cel-go@0b8fcf3

The signature of MacroExpander changed, which is why the compiler doesn't see them as the same.

@francislavoie
Copy link
Member

We didn't bump the dependency though did we? I'm confused.

@mohammed90
Copy link
Member

We didn't bump the dependency though did we? I'm confused.

Not an issue with Caddy nor the Go dep management. I am not able to replicate it. I've successfully built Caddy v2.7.4 with the caddy-trace module without issues.

@greenpau, make sure you don't upgrade cel-go accidentally to HEAD.

@greenpau
Copy link
Author

greenpau commented Sep 4, 2023

@mohammed90 , thank you for looking into it! Yes, that was it. the cel-go v1.18 has the issue. I downgraded it to v1.17 and it is ok now. It was not at the HEAD, but rather latest release. The changes in v1.18 contain the changes to the signature you mentioned.

@mohammed90
Copy link
Member

@mohammed90 , thank you for looking into it! Yes, that was it. the cel-go v1.18 has the issue. I downgraded it to v1.17 and it is ok now. It was not at the HEAD, but rather latest release. The changes in v1.18 contain the changes to the signature you mentioned.

Thanks for the confirmation!

@mohammed90 mohammed90 added the invalid ❓ This doesn't seem right label Sep 4, 2023
@mohammed90 mohammed90 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2023
@francislavoie
Copy link
Member

I don't understand how you upgraded CEL in the first place.

@greenpau
Copy link
Author

greenpau commented Sep 4, 2023

I don't understand how you upgraded CEL in the first place.

@francislavoie ,

go get -u

and it picked the latest cel-go version with the func signature changes.

@francislavoie
Copy link
Member

Oh, I suggest you always use go mod tidy, after manually editing go.mod, don't use go get -u.

@mohammed90
Copy link
Member

I'd go as far as saying only touch your direct deps; and let them (upstream) manage their deps (your transitive deps) because they know how those will impact the code, unless you really have to go out of your way to upgrade the transitive dependency because of an urgent need.

@mholt
Copy link
Member

mholt commented Sep 5, 2023

Thanks for investigating this, @mohammed90! And for the tip @francislavoie

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid ❓ This doesn't seem right
Projects
None yet
Development

No branches or pull requests

4 participants