Skip to content

Comments

Revert "Revert "cel: Add canonical CEL (dev.cel.expr) fields""#81

Closed
sergiitk wants to merge 2 commits intocncf:mainfrom
sergiitk:revert-79-revert-75-canonical-cel
Closed

Revert "Revert "cel: Add canonical CEL (dev.cel.expr) fields""#81
sergiitk wants to merge 2 commits intocncf:mainfrom
sergiitk:revert-79-revert-75-canonical-cel

Conversation

@sergiitk
Copy link
Contributor

@sergiitk sergiitk commented Nov 27, 2023

Reverts #79.
Now that #78 go build CI is in place; this PR brings back the CEL fields.

Once del.dev domain is fixed, we'll see the CI passing and will be able to merge this PR.
Ref #76.

Original comment

This PR adds canonical CEL (https://github.com/google/cel-spec/tree/master/proto/cel/expr) fields to xds.type.v3.CelExpression. Canonical CEL cel.expr was created identical to the google.api.expr.v1alpha1, but may be extended in a backward-compatible way.

Nuances:

  1. The new fields cel_expr_parsed and cel_expr_checked are added outside of oneof expr_specifier per updated policy change: API: change style guide to discourage use of "oneof" envoyproxy/envoy#30851
  2. option (validate.required) = true is removed from the oneof expr_specifier, so the users may not presume one of the parsed_expr, checked_expr will be set.

@sergiitk sergiitk force-pushed the revert-79-revert-75-canonical-cel branch from f8769ec to b322c8a Compare November 27, 2023 18:36
sergiitk and others added 2 commits January 24, 2024 00:21
…ncf#79)"

This reverts commit 5b9bca5.

Reverts cncf#79.
Now that cncf#78 `go build` CI is in place; this PR brings back the
CEL fields.

Once `del.dev` domain is fixed, we'll see the CI passing and will be
able to merge this PR.

Original comment

Signed-off-by: Sergii Tkachenko <sergiitk@google.com>
---

This PR adds canonical CEL
(https://github.com/google/cel-spec/tree/master/proto/cel/expr) fields
to `xds.type.v3.CelExpression`. Canonical CEL `cel.expr` was created
identical to the `google.api.expr.v1alpha1`, but may be extended in a
backward-compatible way.

Nuances: 1. The new fields `cel_expr_parsed` and `cel_expr_checked` are
added outside of `oneof expr_specifier` per updated policy change:
envoyproxy/envoy#30851 2. `option
(validate.required) = true` is removed from the `oneof expr_specifier`,
so the users may not presume one of the `parsed_expr`, `checked_expr`
will be set.

---

Signed-off-by: Sergii Tkachenko <sergiitk@google.com>
Signed-off-by: Sergii Tkachenko <sergiitk@google.com>
@sergiitk sergiitk force-pushed the revert-79-revert-75-canonical-cel branch from 2fa09ca to 205f35c Compare January 24, 2024 08:23
@sergiitk
Copy link
Contributor Author

Superseded by #89.

@sergiitk sergiitk closed this Mar 28, 2024
@sergiitk sergiitk deleted the revert-79-revert-75-canonical-cel branch March 28, 2024 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant