Skip to content

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

Merged
adisuissa merged 1 commit intocncf:mainfrom
sergiitk:revert-75-canonical-cel
Nov 21, 2023
Merged

Revert "cel: Add canonical CEL (dev.cel.expr) fields"#79
adisuissa merged 1 commit intocncf:mainfrom
sergiitk:revert-75-canonical-cel

Conversation

@sergiitk
Copy link
Copy Markdown
Contributor

Reverts #75

Temporarily reverting the PR so that folks aren't affected by #76. Will be brought back once CEL folks solve the domain issue.

This reverts commit 0f5e0d9.
Temporarily reverting the PR so that folks aren't affected by
cncf#76.
Will be brought back once CEL folks solve the domain issue.

Signed-off-by: Sergii Tkachenko <sergiitk@google.com>
@sergiitk sergiitk force-pushed the revert-75-canonical-cel branch from 858b6db to 7cf295f Compare November 21, 2023 17:53
Copy link
Copy Markdown

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!
For the record, the plan is to: rollback #75, add a CI check that executes Go, fix the cel.dev/expr redirect link, re-add the canonical CEL fields.

@adisuissa adisuissa merged commit 5b9bca5 into cncf:main Nov 21, 2023
@sergiitk sergiitk deleted the revert-75-canonical-cel branch November 21, 2023 19:38
sergiitk added a commit to sergiitk/xds that referenced this pull request Nov 27, 2023
sergiitk added a commit to sergiitk/xds that referenced this pull request Nov 27, 2023
…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
---

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>
sergiitk added a commit to sergiitk/xds that referenced this pull request Jan 24, 2024
…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>
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.

2 participants