-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
filter: add conditions to access control filter #7716
Changes from 38 commits
3d4f7aa
beb197c
e914f46
ec74571
97efd6d
f3668b2
dca0933
8e80999
970b361
ebf4c4a
d1fd462
1235c7d
41220da
982e3fd
c31b4a7
e309dd8
6cdbe8e
f934dda
bf99900
6861b98
b27790a
9393ca9
5d44fea
784a970
c081695
9f812b5
184fe6b
811fba6
64c9201
684d473
08fe702
580a79f
8f72a50
a2a9a7f
f86eadf
b9c755c
532ed67
c1f1890
aed67b6
76b6788
a0daefa
753b352
8df3414
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
diff --git a/eval/public/cel_function_adapter.h b/eval/public/cel_function_adapter.h | ||
index d99239c..4dc8cae 100644 | ||
--- a/eval/public/cel_function_adapter.h | ||
+++ b/eval/public/cel_function_adapter.h | ||
@@ -118,6 +118,34 @@ class FunctionAdapter : public CelFunction { | ||
return registry->Register(std::move(status.ValueOrDie())); | ||
} | ||
|
||
+#if !defined(__clang_major_version__) || (defined(__clang_major_version__) && __clang_major_version__ < 8) || defined(__APPLE__) | ||
+ inline cel_base::Status RunWrap(std::function<ReturnType()> func, | ||
+ const absl::Span<const CelValue> argset, | ||
+ ::google::protobuf::Arena* arena, CelValue* result, | ||
+ int arg_index) const { | ||
+ return CreateReturnValue(func(), arena, result); | ||
+ } | ||
+ | ||
+ template <typename Arg, typename... Args> | ||
+ inline cel_base::Status RunWrap(std::function<ReturnType(Arg, Args...)> func, | ||
+ const absl::Span<const CelValue> argset, | ||
+ ::google::protobuf::Arena* arena, CelValue* result, | ||
+ int arg_index) const { | ||
+ Arg argument; | ||
+ if (!ConvertFromValue(argset[arg_index], &argument)) { | ||
+ return cel_base::Status(cel_base::StatusCode::kInvalidArgument, | ||
+ "Type conversion failed"); | ||
+ } | ||
+ | ||
+ std::function<ReturnType(Args...)> wrapped_func = | ||
+ [func, argument](Args... args) -> ReturnType { | ||
+ return func(argument, args...); | ||
+ }; | ||
+ | ||
+ return RunWrap(std::move(wrapped_func), argset, arena, result, | ||
+ arg_index + 1); | ||
+ } | ||
+#else | ||
template <int arg_index> | ||
inline cel_base::Status RunWrap(absl::Span<const CelValue> arguments, | ||
std::tuple<::google::protobuf::Arena*, Arguments...> input, | ||
@@ -137,6 +165,7 @@ class FunctionAdapter : public CelFunction { | ||
::google::protobuf::Arena* arena) const { | ||
return CreateReturnValue(absl::apply(handler_, input), arena, result); | ||
} | ||
+#endif | ||
|
||
::cel_base::Status Evaluate(absl::Span<const CelValue> arguments, | ||
CelValue* result, | ||
@@ -146,9 +175,19 @@ class FunctionAdapter : public CelFunction { | ||
"Argument number mismatch"); | ||
} | ||
|
||
+ | ||
+#if !defined(__clang_major_version__) || (defined(__clang_major_version__) && __clang_major_version__ < 8) || defined(__APPLE__) | ||
+ const auto* handler = &handler_; | ||
+ std::function<ReturnType(Arguments...)> wrapped_handler = | ||
+ [handler, arena](Arguments... args) -> ReturnType { | ||
+ return (*handler)(arena, args...); | ||
+ }; | ||
+ return RunWrap(std::move(wrapped_handler), arguments, arena, result, 0); | ||
+#else | ||
std::tuple<::google::protobuf::Arena*, Arguments...> input; | ||
std::get<0>(input) = arena; | ||
return RunWrap<0>(arguments, input, result, arena); | ||
+#endif | ||
} | ||
|
||
private: |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -248,4 +248,14 @@ REPOSITORY_LOCATIONS = dict( | |
sha256 = "fcdebf54c89d839ffa7eefae166c8e4b551c765559db13ff15bff98047f344fb", | ||
urls = ["https://storage.googleapis.com/quiche-envoy-integration/2a930469533c3b541443488a629fe25cd8ff53d0.tar.gz"], | ||
), | ||
com_google_cel_cpp = dict( | ||
sha256 = "82186be314a2a9c6b9eb2477f15c4f3704b5ac9b4b26bf65694e231a48f4c1f1", | ||
strip_prefix = "cel-cpp-71fb0562a59c05239f92025d3e7beb63169c3923", | ||
urls = ["https://github.com/google/cel-cpp/archive/71fb0562a59c05239f92025d3e7beb63169c3923.tar.gz"], | ||
), | ||
com_googlesource_code_re2 = dict( | ||
sha256 = "f31db9cd224d018a7e4fe88ef84aaa874b0b3ed91d4d98ee5a1531101d3fdc64", | ||
strip_prefix = "re2-87e2ad45e7b18738e1551474f7ee5886ff572059", | ||
urls = ["https://github.com/google/re2/archive/87e2ad45e7b18738e1551474f7ee5886ff572059.tar.gz"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kyessenov qq: does cel-cpp rely on specific commit of re2? Asking because it might conflict with #7878, or latest release (2019-08-01) is fine? cc @mattklein123 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be no difference between which version is used. I think I chose the latest version which I started this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK that's fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was going to ask the same question. I'll switch this back a release version of re2 on a subsequent dependency PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will just fix this when I merge master. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. Happy to help if necessary. Google3 doesn't really have versions for its repositories, and the upstream cel-cpp is continuously tested against head. |
||
), | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
licenses(["notice"]) # Apache 2 | ||
|
||
load( | ||
"//bazel:envoy_build_system.bzl", | ||
"envoy_cc_library", | ||
"envoy_package", | ||
) | ||
|
||
envoy_package() | ||
|
||
envoy_cc_library( | ||
name = "evaluator_lib", | ||
srcs = ["evaluator.cc"], | ||
hdrs = ["evaluator.h"], | ||
deps = [ | ||
":context_lib", | ||
"//source/common/http:utility_lib", | ||
"//source/common/protobuf", | ||
"@com_google_cel_cpp//eval/public:builtin_func_registrar", | ||
"@com_google_cel_cpp//eval/public:cel_expr_builder_factory", | ||
"@com_google_cel_cpp//eval/public:cel_expression", | ||
"@com_google_cel_cpp//eval/public:cel_value", | ||
], | ||
) | ||
|
||
envoy_cc_library( | ||
name = "context_lib", | ||
srcs = ["context.cc"], | ||
hdrs = ["context.h"], | ||
deps = [ | ||
"//source/common/http:utility_lib", | ||
"@com_google_cel_cpp//eval/public:cel_value", | ||
], | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not keen to keep this in Envoy repo, as this seems a general open-source issue of cel-cpp. I understand that this is not an issue in google3 but as a open-sourced project it is general enough to upstreamed. I'm fine if other (possibly Google) maintainer is up for this. @htuch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's be ideal to use
absl::bind_front
but absl keeps delaying the release of important libraries. I'm happy to save this workaround in google3 once bind_front comes out.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, can we upstream this patch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not happy about the patch since it's almost 2x slower than the fast path. Can we postpone it till we get bind_front in absl (any day now TM)? I'm going to continue to maintain the integration, and it should be easier to reconcile the differences.