Skip to content

Commit

Permalink
Merge pull request #24544 from WillemKauf/bounded_property_disable
Browse files Browse the repository at this point in the history
`config`: disable bounded property checks with environment variable
  • Loading branch information
dotnwat authored Dec 13, 2024
2 parents 32bab5c + a9ce489 commit 5060e91
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 2 deletions.
19 changes: 17 additions & 2 deletions src/v/config/bounded_property.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "config/base_property.h"
#include "config/property.h"

#include <cstdlib>
#include <optional>

namespace config {
Expand Down Expand Up @@ -84,6 +85,10 @@ concept bounds = requires(T bounds, const typename T::underlying_t& value) {
{ bounds.clamp(value) } -> std::same_as<typename T::underlying_t>;
};

inline bool bounds_checking_disabled() {
return std::getenv("__REDPANDA_TEST_DISABLE_BOUNDED_PROPERTY_CHECKS");
}

} // namespace detail

/**
Expand Down Expand Up @@ -197,6 +202,9 @@ class bounded_property : public property<T> {
meta,
def,
[this](T new_value) -> std::optional<ss::sstring> {
if (detail::bounds_checking_disabled()) {
return std::nullopt;
}
// Extract inner value if we are an optional<>,
// and pass through into numeric_bounds::validate
using outer_type = std::decay_t<T>;
Expand Down Expand Up @@ -235,6 +243,13 @@ class bounded_property : public property<T> {
}

private:
I clamp_with_bounds(I val) {
if (detail::bounds_checking_disabled()) {
return val;
}
return _bounds.clamp(val);
}

bool clamp_and_update(T val) {
using outer_type = std::decay_t<T>;

Expand All @@ -248,13 +263,13 @@ class bounded_property : public property<T> {
if constexpr (reflection::is_std_optional<outer_type>) {
if (val.has_value()) {
return property<T>::update_value(
std::move(_bounds.clamp(val.value())));
std::move(clamp_with_bounds(val.value())));
} else {
// nullopt is always valid, never clamped. Pass it through.
return property<T>::update_value(std::move(val));
}
} else {
return property<T>::update_value(std::move(_bounds.clamp(val)));
return property<T>::update_value(std::move(clamp_with_bounds(val)));
}
}

Expand Down
29 changes: 29 additions & 0 deletions tests/rptest/tests/cluster_config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1668,6 +1668,35 @@ def test_validate_cloud_storage_cache_size_config(self):
lambda e: e.response.status_code == 400):
self.admin.patch_cluster_config(upsert=upsert)

@cluster(num_nodes=1)
def test_disable_bounded_property_checks(self):
"""
Test that the environmental variable __REDPANDA_TEST_DISABLE_BOUNDED_PROPERTY_CHECKS
being set disables bounded property checks for cluster properties.
"""
out_of_bound_properties = {
"storage_compaction_key_map_memory": 1,
"log_segment_size": 2,
"log_segment_ms": 10
}

# Check that these out of bounds value updates for bounded properties are properly rejected
with expect_exception(requests.exceptions.HTTPError,
lambda e: e.response.status_code == 400):
self.redpanda.set_cluster_config(out_of_bound_properties,
expect_restart=True)

environment = {"__REDPANDA_TEST_DISABLE_BOUNDED_PROPERTY_CHECKS": "ON"}
self.redpanda.set_environment(environment)
self.redpanda.restart_nodes(self.redpanda.nodes)

# Expect these out of bound value updates to succeed.
# expect_restart=True due to some of the properties used.
self.redpanda.set_cluster_config(out_of_bound_properties,
expect_restart=True)
for prop, value in out_of_bound_properties.items():
self._check_value_everywhere(prop, value)


"""
PropertyAliasData:
Expand Down

0 comments on commit 5060e91

Please sign in to comment.