Skip to content
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

runtime: load rtds bool correctly as true/false instead of 1/0 #36044

Merged
merged 10 commits into from
Sep 11, 2024
5 changes: 5 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ bug_fixes:
the number of requests per I/O cycle is configured and an HTTP decoder filter that pauses filter chain is present. This behavior
can be reverted by setting the runtime guard ``envoy.reloadable_features.use_filter_manager_state_for_downstream_end_stream``
to false.
- area: runtime
change: |
Fixed an inconsistency in how boolean values are loaded in RTDS, where they were previously converted to "1"/"0"
instead of "true"/"false". The correct string representation ("true"/"false") will now be used. This change can be
reverted by setting the runtime guard ``envoy.reloadable_features.boolean_to_string_fix`` to false.

removed_config_or_runtime:
# *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
Expand Down
1 change: 1 addition & 0 deletions source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
// ASAP by filing a bug on github. Overriding non-buggy code is strongly discouraged to avoid the
// problem of the bugs being found after the old code path has been removed.
RUNTIME_GUARD(envoy_reloadable_features_allow_alt_svc_for_ips);
RUNTIME_GUARD(envoy_reloadable_features_boolean_to_string_fix);
RUNTIME_GUARD(envoy_reloadable_features_check_switch_protocol_websocket_handshake);
RUNTIME_GUARD(envoy_reloadable_features_conn_pool_delete_when_idle);
RUNTIME_GUARD(envoy_reloadable_features_consistent_header_validation);
Expand Down
8 changes: 7 additions & 1 deletion source/common/runtime/runtime_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,13 @@ SnapshotImpl::Entry SnapshotImpl::createEntry(const ProtobufWkt::Value& value,
case ProtobufWkt::Value::kBoolValue:
entry.bool_value_ = value.bool_value();
if (entry.raw_string_value_.empty()) {
entry.raw_string_value_ = absl::StrCat(value.bool_value());
if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.boolean_to_string_fix")) {
// Convert boolean to "true"/"false"
entry.raw_string_value_ = value.bool_value() ? "true" : "false";
} else {
// Use absl::StrCat for backward compatibility, which converts to "1"/"0"
entry.raw_string_value_ = absl::StrCat(value.bool_value());
}
}
break;
case ProtobufWkt::Value::kStructValue:
Expand Down
35 changes: 35 additions & 0 deletions test/common/runtime/runtime_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#ifdef ENVOY_ENABLE_QUIC
#include "quiche/common/platform/api/quiche_flags.h"
#endif
ABSL_DECLARE_FLAG(bool, envoy_reloadable_features_boolean_to_string_fix);
ABSL_DECLARE_FLAG(bool, envoy_reloadable_features_reject_invalid_yaml);

using testing::_;
Expand Down Expand Up @@ -1314,6 +1315,40 @@ TEST_F(RtdsLoaderImplTest, BadConfigSource) {
"bad config");
}

TEST_F(RtdsLoaderImplTest, BooleanToStringConversionWhenFlagEnabled) {
setup();

absl::SetFlag(&FLAGS_envoy_reloadable_features_boolean_to_string_fix, true);

auto runtime = TestUtility::parseYaml<envoy::service::runtime::v3::Runtime>(R"EOF(
name: some_resource
layer:
toggle: true
)EOF");

EXPECT_CALL(rtds_init_callback_, Call());
doOnConfigUpdateVerifyNoThrow(runtime);

EXPECT_EQ("true", loader_->snapshot().get("toggle").value().get());
}

TEST_F(RtdsLoaderImplTest, BooleanToStringConversionWhenFlagDisabled) {
setup();

absl::SetFlag(&FLAGS_envoy_reloadable_features_boolean_to_string_fix, false);

auto runtime = TestUtility::parseYaml<envoy::service::runtime::v3::Runtime>(R"EOF(
name: some_resource
layer:
toggle: true
)EOF");

EXPECT_CALL(rtds_init_callback_, Call());
doOnConfigUpdateVerifyNoThrow(runtime);

EXPECT_EQ("1", loader_->snapshot().get("toggle").value().get()); // Assuming previous behavior
}

} // namespace
} // namespace Runtime
} // namespace Envoy