diff --git a/MODULE.bazel b/MODULE.bazel index 703063c8d..970b719bd 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -98,9 +98,9 @@ bazel_dep( version = "0.9.0", ) -_CEL_POLICY_TAG = "ebfb2361f47080af643c14cf4da4c2b551a68740" +_CEL_POLICY_TAG = "4c2c2fd5b8f20da4eb34e3ee82a4ef1b7f023347" -_CEL_POLICY_SHA = "ea69e9c6b7bd5bc37d358148aebd2fcca38bc7c45a23feb635de72338e0327c1" +_CEL_POLICY_SHA = "e2bd943f89a3cab3b6508daf0bc5aa53d00e3e6ea52ef14a1d6f618a2b2909be" http_archive = use_repo_rule("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") diff --git a/conformance/policy/BUILD b/conformance/policy/BUILD index 29210e02d..e5d503d88 100644 --- a/conformance/policy/BUILD +++ b/conformance/policy/BUILD @@ -64,15 +64,39 @@ cc_library( alwayslink = True, ) +_ALL_TESTS = [ + "@cel_policy//conformance:k8s", + "@cel_policy//conformance:limits", + "@cel_policy//conformance:nested_rule", + "@cel_policy//conformance:nested_rule2", + "@cel_policy//conformance:nested_rule3", + "@cel_policy//conformance:nested_rule4", + "@cel_policy//conformance:nested_rule5", + "@cel_policy//conformance:nested_rule6", + "@cel_policy//conformance:nested_rule7", + "@cel_policy//conformance:nested_rules_variable_shadowing", + "@cel_policy//conformance:pb", + "@cel_policy//conformance:required_labels", + "@cel_policy//conformance:restricted_destinations", + "@cel_policy//conformance:unconditional_rules", + "@cel_policy//conformance:unnest", + "@cel_policy//conformance:variable_type_propagation", + "@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_duplicate_variable", + "@cel_policy//conformance:compile_error_import", + "@cel_policy//conformance:compile_error_incompatible_outputs", + "@cel_policy//conformance:compile_error_syntax", + "@cel_policy//conformance:compile_error_undeclared_reference", +] + cel_policy_conformance_test( name = "policy_conformance_test", - example = "@cel_policy//conformance:testdata/nested_rule/policy.yaml", skip_tests = [ # TODO(b/506179116): Fix these. # Need to add k8s custom yaml parser and mock runtime. "k8s", ], - test_files = [ - "@cel_policy//conformance:testdata", - ], + test_files = _ALL_TESTS, ) diff --git a/conformance/policy/policy_conformance_test.bzl b/conformance/policy/policy_conformance_test.bzl index 0b4d1a4c6..e57ac7be4 100644 --- a/conformance/policy/policy_conformance_test.bzl +++ b/conformance/policy/policy_conformance_test.bzl @@ -18,26 +18,25 @@ This module contains build rules for generating policy conformance test targets. load("@rules_cc//cc:cc_test.bzl", "cc_test") -def cel_policy_conformance_test(name, test_files, example, skip_tests = [], **kwargs): +def cel_policy_conformance_test(name, test_files, skip_tests = [], **kwargs): """Generates a policy conformance test target. Args: name: Name of the test target. test_files: List of targets or files representing the test data. - example: A specific example file from test_files used for runfiles resolution. skip_tests: List of test cases to skip. - testdata_dir: Path to testdata directory under runfiles. **kwargs: Additional arguments passed to the underlying cc_test. """ args = ["--gunit_fail_if_no_test_linked"] - args.append("--testdata_example='$(rlocationpath {})'".format(example)) + paths = ["$(rlocationpath {})".format(f) for f in test_files] + args.append("--test_bundles=\"{}\"".format(",".join(paths))) if skip_tests: args.append("--skip_tests=" + ",".join(skip_tests)) cc_test( name = name, - data = test_files + [example], + data = test_files, deps = [ "//conformance/policy:policy_conformance_test_lib", ], diff --git a/conformance/policy/policy_conformance_test.cc b/conformance/policy/policy_conformance_test.cc index 0d68f8abf..ef53ab34e 100644 --- a/conformance/policy/policy_conformance_test.cc +++ b/conformance/policy/policy_conformance_test.cc @@ -12,13 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include #include #include #include -// NOLINTNEXTLINE(build/c++17) for OSS compatibility -#include #include "cel/expr/eval.pb.h" #include "absl/flags/flag.h" @@ -28,8 +27,8 @@ #include "absl/status/statusor.h" #include "absl/strings/match.h" #include "absl/strings/str_cat.h" -#include "absl/strings/str_replace.h" #include "absl/strings/string_view.h" +#include "absl/strings/strip.h" #include "common/ast.h" #include "common/internal/value_conversion.h" #include "common/source.h" @@ -61,12 +60,9 @@ #include "google/protobuf/descriptor.h" #include "google/protobuf/dynamic_message.h" #include "google/protobuf/message.h" -#include "google/protobuf/text_format.h" -// Use a specific file to handle bazel runfiles resolution correctly. We find -// parent directory named 'testdata' to use as the root of the test cases. -ABSL_FLAG(std::string, testdata_example, "", - "Path to a specific example file."); +ABSL_FLAG(std::vector, test_bundles, {}, + "Space or comma separated list of test bundle runfiles paths."); ABSL_FLAG(std::vector, skip_tests, {}, "Comma-separated list of tests to skip."); @@ -78,6 +74,63 @@ using ::cel::expr::conformance::test::TestSuite; using ::cel::internal::GetSharedTestingDescriptorPool; using ::testing::HasSubstr; +struct BundleSections { + absl::string_view config_content; + absl::string_view policy_content; + absl::string_view tests_content; +}; + +absl::string_view TrimDoc(absl::string_view doc) { + absl::ConsumeSuffix(&doc, "\n"); + absl::ConsumeSuffix(&doc, "\r"); + return doc; +} + +absl::StatusOr ParseYamlBundle( + absl::string_view bundle_content) { + BundleSections sections; + std::vector docs; + absl::string_view remaining = bundle_content; + + size_t next_line = remaining.find('\n'); + while (next_line != absl::string_view::npos) { + if (absl::StartsWith(remaining.substr(next_line), "\n---\r\n")) { + docs.push_back(TrimDoc(remaining.substr(0, next_line))); + remaining = remaining.substr(next_line + 5); + next_line = remaining.find('\n'); + continue; + } + + if (absl::StartsWith(remaining.substr(next_line), "\n---\n")) { + docs.push_back(TrimDoc(remaining.substr(0, next_line))); + remaining = remaining.substr(next_line + 4); + next_line = remaining.find('\n'); + continue; + } + + next_line = remaining.find('\n', next_line + 1); + } + + if (remaining.empty()) { + return absl::InvalidArgumentError("Empty document in yaml bundle"); + } + docs.push_back(remaining); + + if (docs.size() == 3) { + sections.config_content = docs[0]; + sections.policy_content = docs[1]; + sections.tests_content = docs[2]; + } else if (docs.size() == 2) { + sections.policy_content = docs[0]; + sections.tests_content = docs[1]; + } else { + return absl::InvalidArgumentError( + absl::StrCat("Invalid number of sections: ", docs.size())); + } + + return sections; +} + // Implementations for extension functions referenced in conformance tests. cel::Value LocationCode(const cel::StringValue& ip, const google::protobuf::DescriptorPool* pool, @@ -447,9 +500,8 @@ class CelPolicyTest : public testing::Test { bool skip_; }; - absl::Status RegisterTestSuite( - const std::filesystem::path& dir_path, const std::string& suite_name, + absl::string_view suite_name, const BundleSections& sections, const std::shared_ptr& input_evaluator, const std::shared_ptr& pool, const std::shared_ptr& message_factory, @@ -463,28 +515,15 @@ absl::Status RegisterTestSuite( } } - std::filesystem::path policy_path = dir_path / "policy.yaml"; - std::filesystem::path tests_path = dir_path / "tests.yaml"; - bool is_yaml = true; - if (!std::filesystem::exists(tests_path)) { - tests_path = dir_path / "tests.textproto"; - is_yaml = false; - } - std::filesystem::path config_path = dir_path / "config.yaml"; - - if (!std::filesystem::exists(policy_path) || - !std::filesystem::exists(tests_path)) { - // Not a valid test suite, assume it's a directory we don't care about. + if (sections.policy_content.empty() || sections.tests_content.empty()) { return absl::OkStatus(); } // Parse Environment Config cel::Config config; - if (std::filesystem::exists(config_path)) { - std::string config_content; - CEL_RETURN_IF_ERROR( - cel::internal::GetFileContents(config_path.string(), &config_content)); - CEL_ASSIGN_OR_RETURN(config, cel::EnvConfigFromYaml(config_content)); + if (!sections.config_content.empty()) { + CEL_ASSIGN_OR_RETURN( + config, cel::EnvConfigFromYaml(std::string(sections.config_content))); } // Enable default extensions (optional, bindings) in the config @@ -537,11 +576,8 @@ absl::Status RegisterTestSuite( CEL_ASSIGN_OR_RETURN(auto runtime, std::move(runtime_builder).Build()); // Parse Policy - std::string policy_content; - CEL_RETURN_IF_ERROR( - cel::internal::GetFileContents(policy_path.string(), &policy_content)); CEL_ASSIGN_OR_RETURN(auto source, - cel::NewSource(policy_content, "policy.yaml")); + cel::NewSource(sections.policy_content, "policy.yaml")); auto policy_source = std::make_shared(std::move(source)); CEL_ASSIGN_OR_RETURN(CelPolicyParseResult parse_result, cel::ParseYamlCelPolicy(policy_source)); @@ -556,25 +592,16 @@ absl::Status RegisterTestSuite( CEL_ASSIGN_OR_RETURN(CelPolicyValidationResult compile_result, CompilePolicy(*compiler, *policy)); - std::string tests_content; - CEL_RETURN_IF_ERROR( - cel::internal::GetFileContents(tests_path.string(), &tests_content)); TestSuite test_suite; - if (is_yaml) { - CEL_ASSIGN_OR_RETURN(test_suite, - cel::test::ParsePolicyTestSuiteYaml(tests_content)); - } else { - if (!google::protobuf::TextFormat::ParseFromString(tests_content, &test_suite)) { - return absl::InvalidArgumentError( - absl::StrCat("Failed to parse text proto in ", tests_path.string())); - } - } + CEL_ASSIGN_OR_RETURN( + test_suite, cel::test::ParsePolicyTestSuiteYaml(sections.tests_content)); + std::string suite_name_str(suite_name); auto runner = std::make_shared( - suite_name, std::move(compiler), std::move(runtime), + suite_name_str, std::move(compiler), std::move(runtime), std::move(policy_source), std::move(compile_result), pool, message_factory, input_evaluator, config.GetContextType(), - /*expect_compile_fail=*/absl::StrContains(suite_name, "compile_errors")); + /*expect_compile_fail=*/absl::StrContains(suite_name, "compile_error")); for (const auto& section : test_suite.sections()) { std::string section_name = section.name(); @@ -586,7 +613,7 @@ absl::Status RegisterTestSuite( bool skip = !ShouldRunTest(full_test_name, skip_tests); testing::RegisterTest( - suite_name.c_str(), + suite_name_str.c_str(), absl::StrCat(section_name, "/", test_name).c_str(), nullptr, test_name.c_str(), __FILE__, __LINE__, [runner, test, full_test_name, skip]() -> CelPolicyTest* { @@ -598,14 +625,11 @@ absl::Status RegisterTestSuite( } void RegisterAllTests() { - // cel::google3-end - std::string testdata_example_flag = absl::GetFlag(FLAGS_testdata_example); + std::vector bundle_paths = absl::GetFlag(FLAGS_test_bundles); std::vector skip_tests = absl::GetFlag(FLAGS_skip_tests); - std::string abs_testdata_example = - cel::internal::ResolveRunfilesPath(testdata_example_flag); - ABSL_CHECK(!abs_testdata_example.empty()) - << "Could not find testdata directory: " << testdata_example_flag; + ABSL_CHECK(!bundle_paths.empty()) + << "No test bundles specified in --test_bundles flag."; std::shared_ptr pool = GetSharedTestingDescriptorPool(); @@ -616,35 +640,30 @@ void RegisterAllTests() { ABSL_CHECK_OK(evaluator_or.status()) << "Failed to create input evaluator"; std::shared_ptr evaluator = std::move(evaluator_or.value()); - std::filesystem::path testdata_path(abs_testdata_example); - ABSL_CHECK(std::filesystem::exists(testdata_path)) - << "Testdata path does not exist: " << testdata_path; - // walk up to find 'testdata' parent. A work around to portably - // get the expected directory from bazel. - while (!absl::EndsWith(testdata_path.string(), "testdata")) { - testdata_path = testdata_path.parent_path(); - ABSL_CHECK(testdata_path.string().size() > sizeof("testdata")) - << "could not resolve testdata directory"; - } + for (const std::string& bundle_path : bundle_paths) { + std::string abs_path = cel::internal::ResolveRunfilesPath(bundle_path); + ABSL_CHECK(!abs_path.empty()) + << "Could not resolve runfile path for test bundle: " << bundle_path; - for (const auto& entry : - std::filesystem::recursive_directory_iterator(testdata_path)) { - if (!entry.is_directory()) { - continue; - } - std::filesystem::path dir_path = entry.path(); - // Check if this directory has policy.yaml and tests.yaml (or - // tests.textproto) - if (std::filesystem::exists(dir_path / "policy.yaml") && - (std::filesystem::exists(dir_path / "tests.yaml") || - std::filesystem::exists(dir_path / "tests.textproto"))) { - std::string suite_name = absl::StrReplaceAll( - std::filesystem::relative(dir_path, testdata_path).string(), - {{"\\", "/"}}); - - ABSL_CHECK_OK(RegisterTestSuite(dir_path, suite_name, evaluator, pool, - message_factory, skip_tests)); + std::string bundle_content; + ABSL_CHECK_OK(cel::internal::GetFileContents(abs_path, &bundle_content)) + << "Failed to read bundle file: " << abs_path; + + auto sections_or = ParseYamlBundle(bundle_content); + ABSL_CHECK_OK(sections_or.status()) + << "Failed to parse bundle file: " << abs_path; + + absl::string_view filename = bundle_path; + size_t last_slash = filename.find_last_of("/\\"); + if (last_slash != absl::string_view::npos) { + filename = filename.substr(last_slash + 1); } + absl::string_view suite_view = filename; + absl::ConsumeSuffix(&suite_view, "_bundle.yaml"); + std::string suite_name = std::string(suite_view); + + ABSL_CHECK_OK(RegisterTestSuite(suite_name, sections_or.value(), evaluator, + pool, message_factory, skip_tests)); } }