Skip to content

Treat an unspecified extension version as version 0 in cel::Config and the env YAML parser, consistent with cel-go and cel-java. #2440

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.

Sign up for GitHub

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

Merged
copybara-service merged 1 commit into from
Nov 11, 2026
Merged

Treat an unspecified extension version as version 0 in cel::Config and the env YAML parser, consistent with cel-go and cel-java. #2440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions env/config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ namespace cel {

namespace {

std::string VersionToString(int version) {
return version == Config::ExtensionConfig::kLatest ? "'latest'"
: absl::StrCat(version);
}

const char* ConstantKindToTypeName(const ConstantKind& kind) {
return std::visit(absl::Overload{
[](const std::monostate& arg) { return "dyn"; },
Expand All @@ -58,15 +63,11 @@ absl::Status Config::AddExtensionConfig(std::string name, int version) {
if (extension_config.version == version) {
return absl::OkStatus();
}
std::string version_str;
if (version == ExtensionConfig::kLatest) {
version_str = "'latest'";
} else {
version_str = absl::StrCat(version);
}
return absl::AlreadyExistsError(absl::StrCat(
"Extension '", name, "' version ", extension_config.version,
" is already included. Cannot also include version ", version_str));
return absl::AlreadyExistsError(
absl::StrCat("Extension '", name, "' version ",
VersionToString(extension_config.version),
" is already included. Cannot also include version ",
VersionToString(version)));
}
}
extension_configs_.push_back(
Expand Down
9 changes: 6 additions & 3 deletions env/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,17 @@ class Config {
}

struct ExtensionConfig {
// Sentinel requesting the newest registered version of the extension.
static constexpr int kLatest = std::numeric_limits<int>::max();

std::string name;
int version = kLatest;
// Defaults to version 0, consistent with cel-go and cel-java. kLatest
// requests the newest registered version.
int version = 0;
};

absl::Status AddExtensionConfig(std::string name,
int version = ExtensionConfig::kLatest);
// Adds an extension config. If version is omitted, defaults to version 0.
absl::Status AddExtensionConfig(std::string name, int version = 0);

const std::vector<ExtensionConfig>& GetExtensionConfigs() const {
return extension_configs_;
Expand Down
29 changes: 25 additions & 4 deletions env/config_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,37 @@ TEST(EnvConfigTest, ExtensionConfigs) {
AllOf(Field(&Config::ExtensionConfig::name, "optional"),
Field(&Config::ExtensionConfig::version, 2)),
AllOf(Field(&Config::ExtensionConfig::name, "strings"),
Field(&Config::ExtensionConfig::version,
Config::ExtensionConfig::kLatest))));
Field(&Config::ExtensionConfig::version, 0))));
}

TEST(EnvConfigTest, ExtensionConfigConflict) {
Config config;
ASSERT_THAT(config.AddExtensionConfig("math", 2), IsOk());
ASSERT_THAT(config.AddExtensionConfig("math", 2), IsOk());
ASSERT_THAT(config.AddExtensionConfig("math", 3),
StatusIs(absl::StatusCode::kAlreadyExists));
EXPECT_THAT(config.AddExtensionConfig("math", 3),
StatusIs(absl::StatusCode::kAlreadyExists,
HasSubstr("Extension 'math' version 2 is already "
"included. Cannot also include version 3")));

Config config2;
ASSERT_THAT(
config2.AddExtensionConfig("strings", Config::ExtensionConfig::kLatest),
IsOk());
EXPECT_THAT(
config2.AddExtensionConfig("strings", 0),
StatusIs(
absl::StatusCode::kAlreadyExists,
HasSubstr("Extension 'strings' version 'latest' is already included. "
"Cannot also include version 0")));

Config config3;
ASSERT_THAT(config3.AddExtensionConfig("strings", 0), IsOk());
EXPECT_THAT(
config3.AddExtensionConfig("strings", Config::ExtensionConfig::kLatest),
StatusIs(
absl::StatusCode::kAlreadyExists,
HasSubstr("Extension 'strings' version 0 is already included. Cannot "
"also include version 'latest'")));
}

struct StandardLibraryConfigTestCase {
Expand Down
6 changes: 5 additions & 1 deletion env/env_std_extensions_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ TEST_P(EnvStdExtensions, RegistrationTest) {
env.SetDescriptorPool(internal::GetSharedTestingDescriptorPool());

Config config;
ASSERT_THAT(config.AddExtensionConfig(param.extension), IsOk());
// These test cases exercise features from the newest version of each
// extension.
ASSERT_THAT(config.AddExtensionConfig(param.extension,
Config::ExtensionConfig::kLatest),
IsOk());
env.SetConfig(config);

ASSERT_OK_AND_ASSIGN(std::unique_ptr<Compiler> compiler, env.NewCompiler());
Expand Down
48 changes: 42 additions & 6 deletions env/env_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ absl::StatusOr<bool> CompileAndEvalBooleanExpr(
class LibraryConfigTest : public testing::Test {
protected:
void SetUp() override {
env_.RegisterCompilerLibrary("testlib", "ml", 0,
[]() { return TestLibrary(0); });
env_.RegisterCompilerLibrary("testlib", "ml", 1,
[]() { return TestLibrary(1); });
env_.RegisterCompilerLibrary("testlib", "ml", 2,
Expand All @@ -160,24 +162,58 @@ class LibraryConfigTest : public testing::Test {
Env env_;
};

TEST_F(LibraryConfigTest, DefaultVersion) {
TEST_F(LibraryConfigTest, LatestVersion) {
Config config;
ASSERT_THAT(config.AddExtensionConfig("testlib"), IsOk());
ASSERT_THAT(
config.AddExtensionConfig("testlib", Config::ExtensionConfig::kLatest),
IsOk());

env_.SetConfig(config);

ASSERT_OK_AND_ASSIGN(std::unique_ptr<Compiler> compiler, env_.NewCompiler());
ASSERT_OK_AND_ASSIGN(auto result1, compiler->Compile("testMacro1()"));
ASSERT_OK_AND_ASSIGN(auto result2, compiler->Compile("testFunc1()"));
ASSERT_OK_AND_ASSIGN(auto result3, compiler->Compile("testMacro2()"));
ASSERT_OK_AND_ASSIGN(auto result4, compiler->Compile("testFunc2()"));
ASSERT_OK_AND_ASSIGN(ValidationResult result1,
compiler->Compile("testMacro1()"));
ASSERT_OK_AND_ASSIGN(ValidationResult result2,
compiler->Compile("testFunc1()"));
ASSERT_OK_AND_ASSIGN(ValidationResult result3,
compiler->Compile("testMacro2()"));
ASSERT_OK_AND_ASSIGN(ValidationResult result4,
compiler->Compile("testFunc2()"));

EXPECT_THAT(result1.GetIssues(), IsEmpty());
EXPECT_THAT(result2.GetIssues(), IsEmpty());
EXPECT_THAT(result3.GetIssues(), IsEmpty());
EXPECT_THAT(result4.GetIssues(), IsEmpty());
}

TEST_F(LibraryConfigTest, DefaultVersionIsZero) {
Config config;
ASSERT_THAT(config.AddExtensionConfig("testlib"), IsOk());

env_.SetConfig(config);

ASSERT_OK_AND_ASSIGN(std::unique_ptr<Compiler> compiler, env_.NewCompiler());
ASSERT_OK_AND_ASSIGN(ValidationResult result1,
compiler->Compile("testMacro1()"));
ASSERT_OK_AND_ASSIGN(ValidationResult result2,
compiler->Compile("testFunc1()"));
ASSERT_OK_AND_ASSIGN(ValidationResult result3,
compiler->Compile("testMacro2()"));
ASSERT_OK_AND_ASSIGN(ValidationResult result4,
compiler->Compile("testFunc2()"));

EXPECT_THAT(result1.GetIssues(), IsEmpty());
EXPECT_THAT(result2.GetIssues(), IsEmpty());
EXPECT_THAT(result3.GetIssues(),
UnorderedElementsAre(
Property(&TypeCheckIssue::message,
HasSubstr("undeclared reference to 'testMacro2'"))));
EXPECT_THAT(result4.GetIssues(),
UnorderedElementsAre(
Property(&TypeCheckIssue::message,
HasSubstr("undeclared reference to 'testFunc2'"))));
}

TEST_F(LibraryConfigTest, SpecificVersion) {
Config config;
ASSERT_THAT(config.AddExtensionConfig("testlib", 1), IsOk());
Expand Down
31 changes: 21 additions & 10 deletions env/env_yaml.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,8 @@ absl::Status ParseContainerConfig(Config& config, absl::string_view yaml,
}

absl::Status ParseExtensionConfigs(Config& config, absl::string_view yaml,
const YAML::Node& root) {
const YAML::Node& root,
const EnvConfigFromYamlOptions& options) {
const YAML::Node extensions = root["extensions"];
if (!extensions.IsDefined()) {
return absl::OkStatus();
Expand All @@ -256,9 +257,11 @@ absl::Status ParseExtensionConfigs(Config& config, absl::string_view yaml,
std::string name_str = GetString(yaml, name);

const YAML::Node version = extension["version"];
std::string version_str = GetString(yaml, version);
int extension_version;
int extension_version = options.legacy_default_extension_version_latest
? Config::ExtensionConfig::kLatest
: 0;
if (version.IsDefined()) {
std::string version_str = GetString(yaml, version);
bool is_valid_version = false;
if (version.IsScalar()) {
if (version_str == "latest") {
Expand All @@ -277,8 +280,6 @@ absl::Status ParseExtensionConfigs(Config& config, absl::string_view yaml,
absl::StrCat("Extension '", name_str,
"' version is not a valid number or 'latest'"));
}
} else {
extension_version = Config::ExtensionConfig::kLatest;
}
absl::Status add_status =
config.AddExtensionConfig(name_str, extension_version);
Expand Down Expand Up @@ -876,7 +877,8 @@ void EmitContainerConfig(const Config& env_config, YAML::Emitter& out) {
}
}

void EmitExtensionConfigs(const Config& env_config, YAML::Emitter& out) {
void EmitExtensionConfigs(const Config& env_config, YAML::Emitter& out,
const EnvConfigToYamlOptions& options) {
if (env_config.GetExtensionConfigs().empty()) {
return;
}
Expand All @@ -894,7 +896,15 @@ void EmitExtensionConfigs(const Config& env_config, YAML::Emitter& out) {
out << YAML::BeginMap;
out << YAML::Key << "name";
out << YAML::Value << YAML::DoubleQuoted << extension_config.name;
if (extension_config.version != Config::ExtensionConfig::kLatest) {
if (options.legacy_omit_latest_extension_version) {
if (extension_config.version != Config::ExtensionConfig::kLatest) {
out << YAML::Key << "version";
out << YAML::Value << extension_config.version;
}
} else if (extension_config.version == Config::ExtensionConfig::kLatest) {
out << YAML::Key << "version";
out << YAML::Value << "latest";
} else if (extension_config.version != 0) {
out << YAML::Key << "version";
out << YAML::Value << extension_config.version;
}
Expand Down Expand Up @@ -1280,7 +1290,8 @@ absl::Status ParseContextVariableConfig(Config& config, absl::string_view yaml,

} // namespace

absl::StatusOr<Config> EnvConfigFromYaml(const std::string& yaml) {
absl::StatusOr<Config> EnvConfigFromYaml(
const std::string& yaml, const EnvConfigFromYamlOptions& options) {
Config config;
CEL_ASSIGN_OR_RETURN(YAML::Node root, LoadYaml(yaml));
if (!root.IsDefined() || root.IsNull()) {
Expand All @@ -1294,7 +1305,7 @@ absl::StatusOr<Config> EnvConfigFromYaml(const std::string& yaml) {

CEL_RETURN_IF_ERROR(ParseName(config, yaml, root));
CEL_RETURN_IF_ERROR(ParseContainerConfig(config, yaml, root));
CEL_RETURN_IF_ERROR(ParseExtensionConfigs(config, yaml, root));
CEL_RETURN_IF_ERROR(ParseExtensionConfigs(config, yaml, root, options));
CEL_RETURN_IF_ERROR(ParseStandardLibraryConfig(config, yaml, root));
CEL_RETURN_IF_ERROR(ParseContextVariableConfig(config, yaml, root));
CEL_RETURN_IF_ERROR(ParseVariableConfigs(config, yaml, root));
Expand All @@ -1312,7 +1323,7 @@ void EnvConfigToYaml(const Config& env_config, std::ostream& os,
out << YAML::Value << YAML::DoubleQuoted << env_config.GetName();
}
EmitContainerConfig(env_config, out);
EmitExtensionConfigs(env_config, out);
EmitExtensionConfigs(env_config, out, options);
EmitStandardLibraryConfig(env_config, out);
EmitVariableConfigs(env_config, out, options);
EmitFunctionConfigs(env_config, out, options);
Expand Down
16 changes: 15 additions & 1 deletion env/env_yaml.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,21 @@

namespace cel {

struct EnvConfigFromYamlOptions {
// Whether to treat an omitted extension `version` as
// `Config::ExtensionConfig::kLatest` instead of version 0.
// Temporary rollout option; will be removed once callers migrate.
bool legacy_default_extension_version_latest = false;
};

// EnvConfigFromYaml creates an environment configuration from a YAML string.
//
// To ensure safety, only pass trusted YAML input. yaml-cpp has some fuzz
// coverage, but its security model is unclear. Additionally, callers should be
// aware that improper CEL configuration can lead to unsafe or unpredictably
// expensive expressions.
absl::StatusOr<Config> EnvConfigFromYaml(const std::string& yaml);
absl::StatusOr<Config> EnvConfigFromYaml(
const std::string& yaml, const EnvConfigFromYamlOptions& options = {});

struct EnvConfigToYamlOptions {
// Whether to use type and overload signatures instead of arg/return types in
Expand Down Expand Up @@ -63,6 +71,12 @@ struct EnvConfigToYamlOptions {
// type_name: "int"
// TODO(uncreated-issue/91): default to true after all dependencies are updated
bool use_type_signatures = false;

// Whether to omit `version` when it is `Config::ExtensionConfig::kLatest` and
// emit `version: 0` explicitly instead of emitting `version: latest` and
// omitting version 0.
// Temporary rollout option; will be removed once callers migrate.
bool legacy_omit_latest_extension_version = false;
};

// EnvConfigToYaml serializes an environment configuration as a YAML string.
Expand Down
Loading
Loading