diff --git a/MODULE.bazel b/MODULE.bazel index 825d893d4..d6ac0cb81 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -99,9 +99,9 @@ bazel_dep( version = "0.9.0", ) -_CEL_POLICY_TAG = "4c2c2fd5b8f20da4eb34e3ee82a4ef1b7f023347" +_CEL_POLICY_TAG = "8e4cf0378e3f0d3406b537afbcc6f59562652ad6" -_CEL_POLICY_SHA = "e2bd943f89a3cab3b6508daf0bc5aa53d00e3e6ea52ef14a1d6f618a2b2909be" +_CEL_POLICY_SHA = "c3aaed647f04efb7f77d67de21f21720d298938a066cec7f94dd9e24f9a8d88f" http_archive = use_repo_rule("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") diff --git a/checker/internal/type_inference_context.cc b/checker/internal/type_inference_context.cc index c1787c0fe..ed42ff0ff 100644 --- a/checker/internal/type_inference_context.cc +++ b/checker/internal/type_inference_context.cc @@ -15,6 +15,7 @@ #include "checker/internal/type_inference_context.h" #include +#include #include #include #include diff --git a/conformance/policy/BUILD b/conformance/policy/BUILD index 42adbcf90..50ea27534 100644 --- a/conformance/policy/BUILD +++ b/conformance/policy/BUILD @@ -65,6 +65,16 @@ cc_library( ) _ALL_TESTS = [ + "@cel_policy//conformance:agent_tool_execution_governance", + "@cel_policy//conformance:aggregate", + "@cel_policy//conformance:aggregate_explicit_list_output", + "@cel_policy//conformance:aggregate_explicit_optional_none", + "@cel_policy//conformance:aggregate_nested_all_rules_fall_through_to_empty", + "@cel_policy//conformance:aggregate_nested_explicit_list_double_wrapping", + "@cel_policy//conformance:aggregate_nested_multi_branch_match", + "@cel_policy//conformance:aggregate_nested_single_conditional_match", + "@cel_policy//conformance:aggregate_shadowed_variables", + "@cel_policy//conformance:first_match_nested_aggregate", "@cel_policy//conformance:k8s", "@cel_policy//conformance:limits", "@cel_policy//conformance:nested_rule", @@ -81,9 +91,15 @@ _ALL_TESTS = [ "@cel_policy//conformance:unconditional_rules", "@cel_policy//conformance:unnest", "@cel_policy//conformance:variable_type_propagation", + "@cel_policy//conformance:compile_error_aggregate_false_condition", + "@cel_policy//conformance:compile_error_aggregate_heterogeneous_outputs", + "@cel_policy//conformance:compile_error_aggregate_nested_mixed_semantics", + "@cel_policy//conformance:compile_error_aggregate_subrule_heterogeneous_outputs", + "@cel_policy//conformance:compile_error_aggregate_unreachable_in_nested_first_match", "@cel_policy//conformance:compile_error_compose_conflicting_output", "@cel_policy//conformance:compile_error_compose_conflicting_subrule", "@cel_policy//conformance:compile_error_unreachable", + "@cel_policy//conformance:compile_error_unreachable_under_unconditional_aggregate_subrule", "@cel_policy//conformance:compile_error_duplicate_variable", "@cel_policy//conformance:compile_error_import", "@cel_policy//conformance:compile_error_incompatible_outputs", @@ -97,6 +113,12 @@ cel_policy_conformance_test( # TODO(b/506179116): Fix these. # Need to add k8s custom yaml parser and mock runtime. "k8s", + # Expects Java reverse-order type mismatch error message. + "compile_error_aggregate_heterogeneous_outputs", + # Expects "rule creates unreachable outputs" on the exhaustive subrule, + # whereas C++ reports "match creates unreachable outputs" on the + # subsequent unreachable match. + "compile_error_unreachable_under_unconditional_aggregate_subrule", ], test_files = _ALL_TESTS, ) diff --git a/conformance/policy/policy_conformance_test.cc b/conformance/policy/policy_conformance_test.cc index c325dace8..2f05aed04 100644 --- a/conformance/policy/policy_conformance_test.cc +++ b/conformance/policy/policy_conformance_test.cc @@ -141,6 +141,43 @@ cel::Value LocationCode(const cel::StringValue& ip, return cel::StringValue::WrapUnsafe("ir"); } +bool MapContainsKey(const cel::MapValue& map_val, absl::string_view key, + const google::protobuf::DescriptorPool* pool, + google::protobuf::MessageFactory* factory, google::protobuf::Arena* arena) { + cel::Value has_val; + if (!map_val + .Has(cel::StringValue::WrapUnsafe(key), pool, factory, arena, + &has_val) + .ok()) { + return false; + } + return has_val.IsTrue(); +} + +cel::Value HasCreditCard(const cel::Value& arg, + const google::protobuf::DescriptorPool* pool, + google::protobuf::MessageFactory* factory, + google::protobuf::Arena* arena) { + if (!arg.IsMap()) { + return cel::BoolValue(false); + } + return cel::BoolValue( + MapContainsKey(arg.GetMap(), "cc", pool, factory, arena)); +} + +cel::Value HasEmailOrPhone(const cel::Value& arg, + const google::protobuf::DescriptorPool* pool, + google::protobuf::MessageFactory* factory, + google::protobuf::Arena* arena) { + if (!arg.IsMap()) { + return cel::BoolValue(false); + } + const cel::MapValue& map_val = arg.GetMap(); + return cel::BoolValue( + MapContainsKey(map_val, "email", pool, factory, arena) || + MapContainsKey(map_val, "phone", pool, factory, arena)); +} + // TODO(uncreated-issue/92): This should be migrated to use the testrunner utility // after adding support for reading the yaml specification for envs/tests. class InputEvaluator { @@ -568,11 +605,19 @@ absl::Status RegisterTestSuite( } } - // Register locationCode in runtime + // Register custom functions in runtime CEL_RETURN_IF_ERROR( (cel::UnaryFunctionAdapter:: RegisterGlobalOverload("locationCode", LocationCode, runtime_builder.function_registry()))); + CEL_RETURN_IF_ERROR( + (cel::UnaryFunctionAdapter:: + RegisterGlobalOverload("hasCreditCard", HasCreditCard, + runtime_builder.function_registry()))); + CEL_RETURN_IF_ERROR( + (cel::UnaryFunctionAdapter:: + RegisterGlobalOverload("hasEmailOrPhone", HasEmailOrPhone, + runtime_builder.function_registry()))); CEL_ASSIGN_OR_RETURN(auto runtime, std::move(runtime_builder).Build()); diff --git a/policy/BUILD b/policy/BUILD index 2c00dcf4d..f14d39f73 100644 --- a/policy/BUILD +++ b/policy/BUILD @@ -195,6 +195,7 @@ cc_test( "//common:ast", "//common:ast_proto", "//common:decl", + "//common:expr", "//common:navigable_ast", "//common:source", "//common:type", @@ -209,6 +210,7 @@ cc_test( "//internal:status_macros", "//internal:testing", "//internal:testing_descriptor_pool", + "//internal:testing_message_factory", "//runtime", "//runtime:activation", "//runtime:optional_types", diff --git a/policy/compiler.cc b/policy/compiler.cc index 0f3b7b20b..4a37a04d7 100644 --- a/policy/compiler.cc +++ b/policy/compiler.cc @@ -336,9 +336,12 @@ void AdaptTypeCheckIssues(CelPolicyElementId id, const ValidationResult& r, // as long as the output policies. class PolicyCompiler { public: - explicit PolicyCompiler(policy_internal::IssueReporter* issues, + explicit PolicyCompiler(google::protobuf::Arena* absl_nonnull arena, + policy_internal::IssueReporter* issues, std::unique_ptr base_compiler) - : issues_(*issues), base_compiler_(std::move(base_compiler)) {} + : arena_(arena), + issues_(*issues), + base_compiler_(std::move(base_compiler)) {} absl::string_view GetSourceDescription() const { if (src_ == nullptr) { @@ -366,7 +369,7 @@ class PolicyCompiler { CEL_ASSIGN_OR_RETURN( source, cel::NewSource(val, std::string(GetSourceDescription()))); } - auto result = env->Compile(*source, &arena_); + auto result = env->Compile(*source, arena_); if (!result.ok()) { return result; } @@ -522,8 +525,15 @@ class PolicyCompiler { } } } - out->result_type = overall_type; + if (overall_type.has_value() && + out->semantics == RuleSemantics::kAggregate) { + // Unlike first-match implicit optionalization (i.e. the user policy can't + // interact with the optional layer injected by the composer), aggregate + // rules are explicitly a list output and can intermingle with other rules + // producing compatible lists (aggregate or not). + out->result_type = cel::ListType(arena_, *overall_type); + } return absl::OkStatus(); } @@ -541,7 +551,7 @@ class PolicyCompiler { } private: - google::protobuf::Arena arena_; + google::protobuf::Arena* absl_nonnull arena_; const CelPolicySource* absl_nullable src_; policy_internal::IssueReporter& issues_; std::unique_ptr base_compiler_; @@ -551,10 +561,11 @@ bool IsExhaustive(const CompiledRule& rule); class PolicyComposer { public: - PolicyComposer(const IntermediateCompiledPolicy& icp, - const Compiler& compiler, const CompilePolicyOptions& options, + PolicyComposer(google::protobuf::Arena* absl_nonnull arena, + const IntermediateCompiledPolicy& icp, + const Compiler& compiler, policy_internal::IssueReporter& issues) - : issues_(issues), icp_(icp), compiler_(compiler), options_(options) {} + : arena_(arena), issues_(issues), icp_(icp), compiler_(compiler) {} absl::Status Compose(); @@ -605,11 +616,11 @@ class PolicyComposer { void ComposeRuleVariables(const CompiledRule& rule, Expr& init, Expr& insertion_expr); + google::protobuf::Arena* absl_nonnull arena_; policy_internal::IssueReporter& issues_; OptimizerExprFactory factory_; const IntermediateCompiledPolicy& icp_; const Compiler& compiler_; - const CompilePolicyOptions& options_; std::vector scopes_; bool optionalize_ = false; std::unique_ptr ast_; @@ -647,9 +658,9 @@ absl::Status PolicyComposer::Compose() { "composition failed to handle non-exhaustive rules"); } - google::protobuf::Arena* arena = nullptr; - CEL_ASSIGN_OR_RETURN(cel::ValidationResult result, - compiler_.GetTypeChecker().Check(factory_.ast(), arena)); + CEL_ASSIGN_OR_RETURN( + cel::ValidationResult result, + compiler_.GetTypeChecker().Check(factory_.ast(), arena_)); if (!result.IsValid()) { AdaptTypeCheckIssues(icp_.root_rule().id, result, icp_.policy_source(), issues_); @@ -800,7 +811,9 @@ class IndexRewrite : public AstRewriterBase { if (auto it = expr_id_to_index_.find(e.id()); it != expr_id_to_index_.end()) { e.mutable_ident_expr().set_name(absl::StrCat("@index", it->second)); - factory_.RecordReplacement(e.id(), e); + // Keep any metadata for the rewritten ident. This still replaces + // any references in the macro call map. + factory_.RecordReplacement(e.id(), e, /*keep_metadata=*/true); return true; } return false; @@ -897,8 +910,47 @@ absl::StatusOr PolicyComposer::ComposeFirstMatchRule( absl::Status PolicyComposer::ComposeAggregateRule(const CompiledRule& rule, Expr& init, Expr& insertion_expr) { - return absl::UnimplementedError( - "Aggregate rules are not supported in C++ policy engine."); + ABSL_DCHECK(rule.semantics == RuleSemantics::kAggregate); + scopes_.emplace_back(); + auto pop_scope = absl::MakeCleanup([this]() { scopes_.pop_back(); }); + ComposeRuleVariables(rule, init, insertion_expr); + + std::vector elements; + elements.reserve(rule.matches.size()); + + for (const auto& match : rule.matches) { + Expr production; + CEL_ASSIGN_OR_RETURN( + bool is_wrapped, + ComposeProduction(rule, match.production, init, production)); + + if (IsTriviallyTrueCondition(match)) { + elements.push_back( + factory_.NewListElement(std::move(production), is_wrapped)); + continue; + } + + if (!is_wrapped) { + production = factory_.NewCall("optional.of", std::move(production)); + } + + Ast condition = *match.condition->ast; + MapVariables(condition); + factory_.StartCopyContext(); + Expr cond_copy = factory_.Copy(condition.root_expr()); + SourceInfo source_info = factory_.RemapSourceInfo( + condition.source_info(), GetAstOffset(match.condition->id)); + factory_.MergeSourceInfo(source_info); + + Expr ternary = + factory_.NewCall("_?_:_", std::move(cond_copy), std::move(production), + factory_.NewCall("optional.none")); + elements.push_back( + factory_.NewListElement(std::move(ternary), /*optional=*/true)); + } + + insertion_expr = factory_.NewList(std::move(elements)); + return absl::OkStatus(); } absl::StatusOr PolicyComposer::ComposeProduction( @@ -947,8 +999,10 @@ absl::flat_hash_map PolicyComposer::ResolveBlockIndexes( void PolicyComposer::MapVariables(Ast& ast) { absl::flat_hash_map edit_map = ResolveBlockIndexes(ast); - IndexRewrite rewriter(std::move(edit_map), factory_); - AstRewrite(ast.mutable_root_expr(), rewriter); + OptimizerExprFactory ast_factory(std::move(ast)); + IndexRewrite rewriter(std::move(edit_map), ast_factory); + AstRewrite(ast_factory.mutable_ast().mutable_root_expr(), rewriter); + ast = std::move(ast_factory.mutable_ast()); } void PolicyComposer::ComposeRuleVariables(const CompiledRule& rule, Expr& init, @@ -983,10 +1037,11 @@ bool HasComprehensionParent(const NavigableAstNode& node) { // Unnester implementation. class Unnester { public: - Unnester(const TypeChecker& type_checker, Ast ast, - const CompilePolicyOptions& options, + Unnester(google::protobuf::Arena* absl_nonnull arena, const TypeChecker& type_checker, + Ast ast, const CompilePolicyOptions& options, policy_internal::IssueReporter& issues) - : type_checker_(type_checker), + : arena_(arena), + type_checker_(type_checker), factory_(std::move(ast)), options_(options), height_(options_.unnesting_height_limit), @@ -999,16 +1054,13 @@ class Unnester { CEL_RETURN_IF_ERROR(Slice()); } CEL_RETURN_IF_ERROR(Cleanup()); - if (modified_) { - CEL_ASSIGN_OR_RETURN( - ValidationResult result, - type_checker_.Check(std::move(factory_.mutable_ast()))); - ABSL_DCHECK(result.IsValid()); - CEL_ASSIGN_OR_RETURN(std::unique_ptr checked_ast, - result.ReleaseAst()); - return std::move(*checked_ast); - } - return std::move(factory_.mutable_ast()); + + CEL_ASSIGN_OR_RETURN( + ValidationResult result, + type_checker_.Check(std::move(factory_.mutable_ast()), arena_)); + ABSL_DCHECK(result.IsValid()); + CEL_ASSIGN_OR_RETURN(std::unique_ptr checked_ast, result.ReleaseAst()); + return std::move(*checked_ast); } private: @@ -1019,12 +1071,12 @@ class Unnester { void ReportErrorAtId(int64_t id, absl::string_view message); + google::protobuf::Arena* absl_nonnull arena_; const TypeChecker& type_checker_; OptimizerExprFactory factory_; const CompilePolicyOptions& options_; int height_; policy_internal::IssueReporter& issues_; - bool modified_ = false; }; class UnnestRewriter : public AstRewriterBase { @@ -1071,7 +1123,7 @@ absl::Status Unnester::Slice() { bool can_cut = true; std::vector cuts; for (const NavigableAstNode& node : nav_ast.Root().DescendantsPostorder()) { - // Subsequent cuts will be height_ + 1 in the block, indices. Within the + // Subsequent cuts will be height_ + 1 in the block indices. Within the // error margin we specified. if (node.height() % height_ == 0) { if (HasComprehensionParent(node)) { @@ -1102,7 +1154,6 @@ absl::Status Unnester::Slice() { UnnestRewriter rewriter(factory_, block_list_expr, cuts); AstRewrite(insertion_expr, rewriter); - modified_ = true; return absl::OkStatus(); } @@ -1128,7 +1179,6 @@ absl::Status Unnester::Cleanup() { factory_.PruneMetadata(block_id); factory_.PruneMetadata(list_id); - modified_ = true; } return absl::OkStatus(); @@ -1148,6 +1198,12 @@ void Unnester::ReportErrorAtId(int64_t id, absl::string_view message) { absl::StatusOr CompilePolicy( const Compiler& compiler, const CelPolicy& policy, const CompilePolicyOptions& options) { + google::protobuf::Arena* arena = options.arena; + std::optional local_arena; + if (arena == nullptr) { + local_arena.emplace(); + arena = &(*local_arena); + } policy_internal::IssueReporter issues; if (options.unnesting_height_limit != 0 && options.unnesting_height_limit < 2) { @@ -1168,7 +1224,7 @@ absl::StatusOr CompilePolicy( builder->GetCheckerBuilder().SetExpressionContainer(cont); CEL_ASSIGN_OR_RETURN(auto base_compiler, builder->Build()); - PolicyCompiler policy_compiler(&issues, std::move(base_compiler)); + PolicyCompiler policy_compiler(arena, &issues, std::move(base_compiler)); IntermediateCompiledPolicy icp; CEL_RETURN_IF_ERROR(policy_compiler.CompilePolicy(policy, &icp)); @@ -1179,7 +1235,7 @@ absl::StatusOr CompilePolicy( } CEL_ASSIGN_OR_RETURN(base_compiler, builder->Build()); - PolicyComposer composer(icp, *base_compiler, options, issues); + PolicyComposer composer(arena, icp, *base_compiler, issues); CEL_RETURN_IF_ERROR(composer.Compose()); if (!composer.success()) { return CelPolicyValidationResult(issues.ReleaseIssues(), @@ -1187,8 +1243,8 @@ absl::StatusOr CompilePolicy( } auto ast = composer.ReleaseAst(); - Unnester unnester(base_compiler->GetTypeChecker(), std::move(*ast), options, - issues); + Unnester unnester(arena, base_compiler->GetTypeChecker(), std::move(*ast), + options, issues); CEL_ASSIGN_OR_RETURN(Ast unnested_ast, unnester.Unnest()); if (HasErrors(issues)) { diff --git a/policy/compiler.h b/policy/compiler.h index df832112e..6f6db99b8 100644 --- a/policy/compiler.h +++ b/policy/compiler.h @@ -36,6 +36,8 @@ struct CompilePolicyOptions { // can cause problems in serialization/deserialization. This does not apply // if the AST is used directly by the runtime. int unnesting_height_limit = 0; + + google::protobuf::Arena* arena = nullptr; }; // Compiles a CEL policy using the provided CEL compiler as a base environment. diff --git a/policy/compiler_test.cc b/policy/compiler_test.cc index 1b75cbe11..f99cce5e9 100644 --- a/policy/compiler_test.cc +++ b/policy/compiler_test.cc @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -28,6 +29,7 @@ #include "common/ast.h" #include "common/ast_proto.h" #include "common/decl.h" +#include "common/expr.h" #include "common/navigable_ast.h" #include "common/source.h" #include "common/type.h" @@ -43,6 +45,7 @@ #include "internal/status_macros.h" #include "internal/testing.h" #include "internal/testing_descriptor_pool.h" +#include "internal/testing_message_factory.h" #include "policy/cel_policy.h" #include "policy/cel_policy_parse_result.h" #include "policy/cel_policy_validation_result.h" @@ -75,6 +78,7 @@ absl::StatusOr> BuildTestCompiler() { CompilerOptions opts; opts.adapt_parser_errors = true; opts.parser_options.enable_optional_syntax = true; + opts.parser_options.add_macro_calls = true; CEL_ASSIGN_OR_RETURN( auto builder, NewCompilerBuilder(internal::GetSharedTestingDescriptorPool(), opts)); @@ -133,6 +137,14 @@ MATCHER(MetadataMapValid, negation ? "has stale metadata map entries" for (const NavigableAstNode& node : nav_ast.Root().DescendantsPostorder()) { valid_ids.insert(node.expr()->id()); } + absl::flat_hash_set valid_position_ids = valid_ids; + for (const auto& [_, macro_expr] : ast->source_info().macro_calls()) { + NavigableAst macro_nav = cel::NavigableAst::Build(macro_expr); + for (const NavigableAstNode& node : + macro_nav.Root().DescendantsPostorder()) { + valid_position_ids.insert(node.expr()->id()); + } + } bool is_valid = true; for (const auto& [id, _] : ast->type_map()) { @@ -148,7 +160,7 @@ MATCHER(MetadataMapValid, negation ? "has stale metadata map entries" } } for (const auto& [id, _] : ast->source_info().positions()) { - if (!valid_ids.contains(id)) { + if (!valid_position_ids.contains(id)) { *result_listener << "stale source_info position for id " << id << "; "; is_valid = false; } @@ -399,6 +411,86 @@ TEST(CompilerTest, UnparseComposedAstWithMacros) { "optional.none())"); } +TEST(CompilerTest, RewrittenVariablesUpdatedInMacroCallsAndReferenceMap) { + absl::string_view yaml = R"yaml(name: macro_policy +rule: + variables: + - name: var_inner + expression: "[1, 2].all(i, i > 0)" + - name: var_outer + expression: "[3, 4].exists(j, j > 0 && variables.var_inner)" + match: + - condition: "[5, 6].all(k, k > 0 && variables.var_inner)" + output: "[1].map(m, m > 0 && variables.var_outer)" +)yaml"; + ASSERT_OK_AND_ASSIGN(std::unique_ptr policy, + ParsePolicyFromYaml(yaml)); + ASSERT_OK_AND_ASSIGN(std::unique_ptr compiler, BuildTestCompiler()); + ASSERT_OK_AND_ASSIGN(CelPolicyValidationResult result, + CompilePolicy(*compiler, *policy)); + ASSERT_TRUE(result.IsValid()) << result.FormatIssues(); + ASSERT_OK_AND_ASSIGN(std::unique_ptr ast, result.ReleaseAst()); + + EXPECT_THAT(ast->source_info().macro_calls(), testing::SizeIs(4)); + EXPECT_THAT(ast, MetadataMapValid()); + + absl::flat_hash_set macro_index_vars; + for (const auto& [macro_id, macro_expr] : ast->source_info().macro_calls()) { + NavigableAst macro_nav = cel::NavigableAst::Build(macro_expr); + for (const NavigableAstNode& node : + macro_nav.Root().DescendantsPostorder()) { + const Expr* expr = node.expr(); + if (expr->has_select_expr()) { + EXPECT_NE(expr->select_expr().field(), "var_inner"); + EXPECT_NE(expr->select_expr().field(), "var_outer"); + } + if (expr->has_ident_expr()) { + absl::string_view name = expr->ident_expr().name(); + EXPECT_FALSE(absl::StartsWith(name, "variables")); + if (absl::StartsWith(name, "@index")) { + macro_index_vars.insert(std::string(name)); + auto ref_it = ast->reference_map().find(expr->id()); + ASSERT_NE(ref_it, ast->reference_map().end()); + EXPECT_EQ(ref_it->second.name(), name); + } + } + } + } + EXPECT_THAT(macro_index_vars, + testing::UnorderedElementsAre("@index0", "@index1")); + + absl::flat_hash_set root_index_vars; + NavigableAst root_nav = cel::NavigableAst::Build(ast->root_expr()); + for (const NavigableAstNode& node : root_nav.Root().DescendantsPostorder()) { + const Expr* expr = node.expr(); + if (expr->has_ident_expr() && + absl::StartsWith(expr->ident_expr().name(), "@index")) { + absl::string_view name = expr->ident_expr().name(); + root_index_vars.insert(std::string(name)); + auto ref_it = ast->reference_map().find(expr->id()); + ASSERT_NE(ref_it, ast->reference_map().end()); + EXPECT_EQ(ref_it->second.name(), name); + EXPECT_TRUE(ast->type_map().contains(expr->id())); + } + } + EXPECT_THAT(root_index_vars, + testing::UnorderedElementsAre("@index0", "@index1")); + + for (const auto& [id, ref] : ast->reference_map()) { + EXPECT_FALSE(absl::StartsWith(ref.name(), "variables")); + } + + cel::expr::ParsedExpr parsed_expr; + ASSERT_THAT(cel::AstToParsedExpr(*ast, &parsed_expr), IsOk()); + ASSERT_OK_AND_ASSIGN(std::string unparsed, + google::api::expr::Unparse(parsed_expr)); + EXPECT_EQ( + unparsed, + "cel.@block([[1, 2].all(i, i > 0), [3, 4].exists(j, j > 0 && @index0)], " + "[5, 6].all(k, k > 0 && @index0) ? " + "optional.of([1].map(m, m > 0 && @index1)) : optional.none())"); +} + TEST(CompilerTest, UnreachableMatchAfterTriviallyTrueCondition) { absl::string_view yaml = R"yaml( name: cel_policy @@ -466,7 +558,7 @@ name: dead_branch_after_aggregate aggregate: - condition: x > 0 output: 1 - - output: 2 + - output: "[2]" )yaml"; ASSERT_OK_AND_ASSIGN(std::unique_ptr policy, ParsePolicyFromYaml(yaml)); @@ -1405,39 +1497,90 @@ name: cel_policy testing::HasSubstr("no matching overload for '_?_:_'")); } -TEST(CompilerTest, AggregateRuleReturnsUnimplementedError) { +TEST(CompilerTest, AggregateRuleComposedAstStructure) { absl::string_view yaml = R"yaml( name: aggregate_policy rule: + variables: + - name: v1 + expression: "'v'" aggregate: - - output: '"first"' - - condition: true - output: '"second"' - - condition: y > 5 - output: '"third"' + - condition: x > 10 + output: '"first"' + - output: variables.v1 + - rule: + match: + - condition: y > 5 + output: '"nested_opt"' )yaml"; ASSERT_OK_AND_ASSIGN(std::unique_ptr policy, ParsePolicyFromYaml(yaml)); ASSERT_OK_AND_ASSIGN(std::unique_ptr compiler, BuildTestCompiler()); - EXPECT_THAT(CompilePolicy(*compiler, *policy), - StatusIs(absl::StatusCode::kUnimplemented)); + ASSERT_OK_AND_ASSIGN(CelPolicyValidationResult result, + CompilePolicy(*compiler, *policy)); + ASSERT_TRUE(result.IsValid()) << result.FormatIssues(); + const cel::Ast* ast = result.GetAst(); + ASSERT_NE(ast, nullptr); + + cel::expr::ParsedExpr parsed_expr; + ASSERT_THAT(cel::AstToParsedExpr(*ast, &parsed_expr), IsOk()); + ASSERT_OK_AND_ASSIGN(std::string unparsed, + google::api::expr::Unparse(parsed_expr)); + + EXPECT_EQ(unparsed, + "cel.@block([\"v\"], [?(x > 10) ? optional.of(\"first\") : " + "optional.none(), @index0, ?(y > 5) ? optional.of(\"nested_opt\") " + ": optional.none()])"); } -TEST(CompilerTest, NestedAggregateRuleReturnsUnimplementedError) { - absl::string_view yaml = R"yaml( -name: nested_aggregate_policy +TEST(CompilerTest, AggregateRuleConditionVariablesAndMetadata) { + absl::string_view yaml = R"yaml(name: aggregate_condition_metadata_policy rule: - match: - - condition: x > 10 - rule: - aggregate: - - output: '"first"' + variables: + - name: v1 + expression: 10 + aggregate: + - condition: "[1, 2].all(i, i > variables.v1)" + output: "[3, 4].map(j, j + variables.v1)" )yaml"; ASSERT_OK_AND_ASSIGN(std::unique_ptr policy, ParsePolicyFromYaml(yaml)); ASSERT_OK_AND_ASSIGN(std::unique_ptr compiler, BuildTestCompiler()); - EXPECT_THAT(CompilePolicy(*compiler, *policy), - StatusIs(absl::StatusCode::kUnimplemented)); + ASSERT_OK_AND_ASSIGN(CelPolicyValidationResult result, + CompilePolicy(*compiler, *policy)); + ASSERT_TRUE(result.IsValid()) << result.FormatIssues(); + const cel::Source* source = result.GetSource()->content(); + ASSERT_NE(source, nullptr); + ASSERT_OK_AND_ASSIGN(std::unique_ptr ast, result.ReleaseAst()); + + EXPECT_THAT(ast, MetadataMapValid()); + EXPECT_THAT(ast->source_info().macro_calls(), testing::SizeIs(2)); + + cel::expr::ParsedExpr parsed_expr; + ASSERT_THAT(cel::AstToParsedExpr(*ast, &parsed_expr), IsOk()); + ASSERT_OK_AND_ASSIGN(std::string unparsed, + google::api::expr::Unparse(parsed_expr)); + EXPECT_EQ(unparsed, + "cel.@block([10], [?[1, 2].all(i, i > @index0) ? " + "optional.of([3, 4].map(j, j + @index0)) : optional.none()])"); + + NavigableAst nav_ast = cel::NavigableAst::Build(ast->root_expr()); + const NavigableAstNode* cond_gt_node = nullptr; + for (const NavigableAstNode& node : nav_ast.Root().DescendantsPostorder()) { + if (node.expr()->has_call_expr() && + node.expr()->call_expr().function() == "_>_") { + cond_gt_node = &node; + break; + } + } + ASSERT_NE(cond_gt_node, nullptr); + std::optional cond_loc = + ast->ComputeSourceLocation(cond_gt_node->expr()->id()); + ASSERT_TRUE(cond_loc.has_value()); + EXPECT_THAT(source->DisplayErrorLocation(*cond_loc), + testing::HasSubstr( + " - condition: \"[1, 2].all(i, i > variables.v1)\"\n | " + "..................................^")); } TEST(CompilerTest, NestedAggregateUnderAggregateReportsError) { @@ -1483,5 +1626,97 @@ name: nested_aggregate_mixed_policy testing::HasSubstr("nested aggregate rules are not allowed")); } +TEST(CompilerTest, AggregateRuleAllowsMultipleUnconditionalMatches) { + absl::string_view yaml = R"yaml( +name: aggregate_unconditional_policy +rule: + aggregate: + - output: '"first"' + - condition: true + output: '"second"' + - condition: x > 0 + output: '"third"' +)yaml"; + ASSERT_OK_AND_ASSIGN(std::unique_ptr policy, + ParsePolicyFromYaml(yaml)); + ASSERT_OK_AND_ASSIGN(std::unique_ptr compiler, BuildTestCompiler()); + ASSERT_OK_AND_ASSIGN(CelPolicyValidationResult result, + CompilePolicy(*compiler, *policy)); + ASSERT_TRUE(result.IsValid()) << result.FormatIssues(); +} + +TEST(CompilerTest, AggregateRuleIncompatibleOutputsReportsError) { + absl::string_view yaml = R"yaml( +name: aggregate_incompatible_policy +rule: + aggregate: + - condition: x > 10 + output: '"first"' + - condition: y > 5 + output: 42 +)yaml"; + ASSERT_OK_AND_ASSIGN(std::unique_ptr policy, + ParsePolicyFromYaml(yaml)); + ASSERT_OK_AND_ASSIGN(std::unique_ptr compiler, BuildTestCompiler()); + ASSERT_OK_AND_ASSIGN(CelPolicyValidationResult result, + CompilePolicy(*compiler, *policy)); + EXPECT_FALSE(result.IsValid()); + EXPECT_THAT( + result.FormatIssues(), + testing::HasSubstr("incompatible output types: block has output type " + "int, but previous outputs have type string")); +} + +TEST(CompilerTest, AggregatePolicyEvaluates) { + absl::string_view yaml = R"yaml( +name: aggregate_eval_policy +rule: + variables: + - name: base + expression: 100 + aggregate: + - condition: x > 0 + output: variables.base + x + - output: variables.base + - rule: + match: + - condition: y > 0 + output: variables.base + y +)yaml"; + ASSERT_OK_AND_ASSIGN(std::unique_ptr policy, + ParsePolicyFromYaml(yaml)); + ASSERT_OK_AND_ASSIGN(std::unique_ptr compiler, BuildTestCompiler()); + ASSERT_OK_AND_ASSIGN(CelPolicyValidationResult validation_result, + CompilePolicy(*compiler, *policy)); + ASSERT_TRUE(validation_result.IsValid()) << validation_result.FormatIssues(); + ASSERT_OK_AND_ASSIGN(std::unique_ptr ast, + validation_result.ReleaseAst()); + + cel::RuntimeOptions opts; + opts.enable_qualified_type_identifiers = true; + ASSERT_OK_AND_ASSIGN( + cel::RuntimeBuilder rt_builder, + cel::CreateStandardRuntimeBuilder( + cel::internal::GetSharedTestingDescriptorPool(), opts)); + ASSERT_THAT(cel::extensions::EnableOptionalTypes(rt_builder), IsOk()); + ASSERT_OK_AND_ASSIGN(std::unique_ptr runtime, + std::move(rt_builder).Build()); + ASSERT_OK_AND_ASSIGN(std::unique_ptr program, + runtime->CreateProgram(std::move(ast))); + + google::protobuf::Arena arena; + cel::Activation activation; + activation.InsertOrAssignValue("x", cel::IntValue(5)); + activation.InsertOrAssignValue("y", cel::IntValue(20)); + + ASSERT_OK_AND_ASSIGN(cel::Value result, + program->Evaluate(&arena, activation)); + EXPECT_THAT(result, cel::test::ListValueIs(cel::test::ListValueElements( + testing::ElementsAre(IntValueIs(105), IntValueIs(100), + IntValueIs(120)), + cel::internal::GetTestingDescriptorPool(), + cel::internal::GetTestingMessageFactory(), &arena))); +} + } // namespace } // namespace cel