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

cel: Add canonical CEL (dev.cel.expr) fields #89

Merged
merged 3 commits into from
Mar 29, 2024
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
4 changes: 4 additions & 0 deletions bazel/external_proto_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,14 @@ EXTERNAL_PROTO_GO_BAZEL_DEP_MAP = {
# go_googleapis in https://github.com/bazelbuild/rules_go/blob/master/go/dependencies.rst#overriding-dependencies
"@com_google_googleapis//google/api/expr/v1alpha1:checked_proto": "@go_googleapis//google/api/expr/v1alpha1:expr_go_proto",
"@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto": "@go_googleapis//google/api/expr/v1alpha1:expr_go_proto",
"@dev_cel//proto/cel/expr:checked_proto": "@dev_cel//proto/cel/expr:checked_go_proto",

Choose a reason for hiding this comment

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

IIUC these are introduced in addition to the com_googleapis variant to avoid introducing a breaking API change. However, as this is a superset of the com_googleapis project, then I think that conceptually it needs to replace it completely (to avoid violating ODR).
I'm a bit concerned what will happen if these two repos diverge. but as we don't have versioning in place, I don't think we can do much about it.
cc @htuch if there are other ways here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adisuissa Canonical CEL (cel.dev) is meant to replace v1alpha1 CEL from google APIs package. Canonical CEL will always be extended in a backward-compatible way (as protos normally should be), so there isn't a divergence problem. I'm CCing CEL folks here for more context: @l46kok, @TristonianJones.
Also, @tyxia has more context from the Envoy side.

Copy link
Contributor

@tyxia tyxia Mar 29, 2024

Choose a reason for hiding this comment

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

From Envoy perspective, cel.dev and v1alpha1 will co-exist. They are guaranteed to be in sync by CEL team.

New users in Envoy are recommended to use cel.dev while existing users can still use v1alpha1

Thus, v1alpha1 is deprecated but can not be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this makes sense. When the Envoy PR lands, which shoudl be adding implementation level support (not just API import), it might make sense to audit any remaining references to CEL that goes direct to google.api package namespace in the API protos in envoyproxy/envoy, e.g. RBAC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like nothing is blocking this PR from being merged, right?

"@dev_cel//proto/cel/expr:syntax_proto": "@dev_cel//proto/cel/expr:syntax_go_proto",
}

# This maps from the Bazel proto_library target to the C++ language binding target for external dependencies.
EXTERNAL_PROTO_CC_BAZEL_DEP_MAP = {
"@com_google_googleapis//google/api/expr/v1alpha1:checked_proto": "@com_google_googleapis//google/api/expr/v1alpha1:checked_cc_proto",
"@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto": "@com_google_googleapis//google/api/expr/v1alpha1:syntax_cc_proto",
"@dev_cel//proto/cel/expr:checked_proto": "@dev_cel//proto/cel/expr:checked_cc_proto",
"@dev_cel//proto/cel/expr:syntax_proto": "@dev_cel//proto/cel/expr:syntax_cc_proto",
}
4 changes: 4 additions & 0 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ def xds_api_dependencies():
"com_google_protobuf",
locations = REPOSITORY_LOCATIONS,
)
xds_http_archive(
name = "dev_cel",
locations = REPOSITORY_LOCATIONS,
)
xds_http_archive(
"io_bazel_rules_go",
locations = REPOSITORY_LOCATIONS,
Expand Down
5 changes: 5 additions & 0 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ REPOSITORY_LOCATIONS = dict(
strip_prefix = "protobuf-3.21.5",
urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.21.5.zip"],
),
dev_cel = dict(
sha256 = "3ee09eb69dbe77722e9dee23dc48dc2cd9f765869fcf5ffb1226587c81791a0b",
strip_prefix = "cel-spec-0.15.0",
urls = ["https://github.com/google/cel-spec/archive/refs/tags/v0.15.0.tar.gz"],
),
io_bazel_rules_go = dict(
sha256 = "6dc2da7ab4cf5d7bfc7c949776b1b7c733f05e56edc4bcd9022bb249d2e2a996",
urls = [
Expand Down
5 changes: 3 additions & 2 deletions go/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@ module github.com/cncf/xds/go
go 1.19

require (
cel.dev/expr v0.15.0
github.com/envoyproxy/protoc-gen-validate v1.0.4
google.golang.org/genproto/googleapis/api v0.0.0-20230822172742-b8732ec3820d
google.golang.org/grpc v1.59.0
google.golang.org/protobuf v1.32.0
google.golang.org/protobuf v1.33.0
)

require (
github.com/golang/protobuf v1.5.3 // indirect
golang.org/x/net v0.20.0 // indirect
golang.org/x/sys v0.16.0 // indirect
golang.org/x/text v0.14.0 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240318140521-94a12d6c2237 // indirect
)
10 changes: 6 additions & 4 deletions go/go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
cel.dev/expr v0.15.0 h1:O1jzfJCQBfL5BFoYktaxwIhuttaQPsVWerH9/EEKx0w=
cel.dev/expr v0.15.0/go.mod h1:TRSuuV7DlVCE/uwv5QbAiW/v8l5O8C4eEPHeu7gf7Sg=
github.com/envoyproxy/protoc-gen-validate v1.0.4 h1:gVPz/FMfvh57HdSJQyvBtF00j8JU4zdyUgIUNhlgg0A=
github.com/envoyproxy/protoc-gen-validate v1.0.4/go.mod h1:qys6tmnRsYrQqIhm2bvKZH4Blx/1gTIZ2UKVY1M+Yew=
github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
Expand All @@ -14,11 +16,11 @@ golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
google.golang.org/genproto/googleapis/api v0.0.0-20230822172742-b8732ec3820d h1:DoPTO70H+bcDXcd39vOqb2viZxgqeBeSGtZ55yZU4/Q=
google.golang.org/genproto/googleapis/api v0.0.0-20230822172742-b8732ec3820d/go.mod h1:KjSP20unUpOx5kyQUFa7k4OJg0qeJ7DEZflGDu2p6Bk=
google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d h1:uvYuEyMHKNt+lT4K3bN6fGswmK8qSvcreM3BwjDh+y4=
google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d/go.mod h1:+Bk1OCOj40wS2hwAMA+aCW9ypzm63QTBBHp6lQ3p+9M=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240318140521-94a12d6c2237 h1:NnYq6UN9ReLM9/Y01KWNOWyI5xQ9kbIms5GGJVwS/Yc=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240318140521-94a12d6c2237/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY=
google.golang.org/grpc v1.59.0 h1:Z5Iec2pjwb+LEOqzpB2MR12/eKFhDPhuqW91O+4bwUk=
google.golang.org/grpc v1.59.0/go.mod h1:aUPDwccQo6OTjy7Hct4AfBPD1GptF4fyUjIkQ9YtF98=
google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.32.0 h1:pPC6BG5ex8PDFnkbrGU3EixyhKcQ2aDuBS36lqK/C7I=
google.golang.org/protobuf v1.32.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI=
google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
122 changes: 79 additions & 43 deletions go/xds/type/v3/cel.pb.go

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

71 changes: 58 additions & 13 deletions go/xds/type/v3/cel.pb.validate.go

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

2 changes: 2 additions & 0 deletions xds/type/v3/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,7 @@ xds_proto_package(
"//xds/annotations/v3:pkg",
"@com_google_googleapis//google/api/expr/v1alpha1:checked_proto",
"@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto",
"@dev_cel//proto/cel/expr:checked_proto",
"@dev_cel//proto/cel/expr:syntax_proto",
],
)
Loading