Skip to content

Commit bf6f3c5

Browse files
committed
config/tests: add tests for invalid yaml values
Test read_yaml behavior with validator errors, type mismatches, and throwing validators.
1 parent 7c33ebe commit bf6f3c5

File tree

2 files changed

+128
-6
lines changed

2 files changed

+128
-6
lines changed

src/v/config/tests/config_store_test.cc

Lines changed: 96 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,33 @@
1313
#include "json/writer.h"
1414

1515
#include <seastar/core/sstring.hh>
16-
#include <seastar/core/thread.hh>
1716
#include <seastar/testing/thread_test_case.hh>
1817
#include <seastar/util/log.hh>
1918

2019
#include <cstdint>
2120
#include <iostream>
2221
#include <iterator>
23-
#include <random>
2422
#include <string>
2523
#include <utility>
2624

2725
namespace {
2826

2927
ss::logger lg("config_test"); // NOLINT
3028

29+
std::optional<ss::sstring> validate_magic_prefix(const ss::sstring& value) {
30+
if (!value.starts_with("magic_")) {
31+
return fmt::format("value must start with 'magic_', got: {}", value);
32+
}
33+
return std::nullopt;
34+
}
35+
36+
std::optional<ss::sstring> validate_throwing(const ss::sstring& value) {
37+
if (value == "throw") {
38+
throw std::runtime_error("validator threw an exception");
39+
}
40+
return std::nullopt;
41+
}
42+
3143
struct test_config : public config::config_store {
3244
config::property<int> optional_int;
3345
config::property<ss::sstring> required_string;
@@ -44,6 +56,8 @@ struct test_config : public config::config_store {
4456
config::property<ss::sstring> default_secret_string;
4557
config::property<ss::sstring> secret_string;
4658
config::property<bool> aliased_bool;
59+
config::property<ss::sstring> validated_string;
60+
config::property<ss::sstring> throwing_validator_string;
4761

4862
test_config()
4963
: optional_int(
@@ -113,11 +127,38 @@ struct test_config : public config::config_store {
113127
"aliased_bool",
114128
"Property with a compat alias",
115129
{.aliases = {"aliased_bool_legacy"}},
116-
true) {}
130+
true)
131+
, validated_string(
132+
*this,
133+
"validated_string",
134+
"String that must start with magic_",
135+
{},
136+
"magic_foo",
137+
&validate_magic_prefix)
138+
, throwing_validator_string(
139+
*this,
140+
"throwing_validator_string",
141+
"String with a validator that throws",
142+
{},
143+
"safe",
144+
&validate_throwing) {}
117145
};
118146

119147
struct noop_config : public config::config_store {};
120148

149+
struct required_validated_config : public config::config_store {
150+
config::property<ss::sstring> required_validated_string;
151+
152+
required_validated_config()
153+
: required_validated_string(
154+
*this,
155+
"required_validated_string",
156+
"Required string that must start with magic_",
157+
{.required = config::required::yes},
158+
"magic_bar",
159+
&validate_magic_prefix) {}
160+
};
161+
121162
YAML::Node minimal_valid_configuration() {
122163
return YAML::Load(
123164
"required_string: test_value_1\n"
@@ -257,10 +298,59 @@ SEASTAR_THREAD_TEST_CASE(validate_valid_configuration) {
257298
BOOST_TEST(errors.size() == 0);
258299
}
259300

260-
SEASTAR_THREAD_TEST_CASE(validate_invalid_configuration) {
301+
SEASTAR_THREAD_TEST_CASE(validate_with_validator_error) {
261302
auto cfg = test_config();
262-
auto errors = cfg.read_yaml(valid_configuration());
263-
BOOST_TEST(errors.size() == 0);
303+
304+
auto invalid_yaml = YAML::Load("validated_string: invalid_value\n");
305+
306+
auto errors = cfg.read_yaml(invalid_yaml);
307+
308+
// Should have error from validator
309+
BOOST_TEST(errors.size() > 0);
310+
311+
// Property should retain default value
312+
// Surprising but the current behavior is that invalid values actually
313+
// update the property.
314+
BOOST_TEST(cfg.validated_string() == "invalid_value");
315+
}
316+
317+
SEASTAR_THREAD_TEST_CASE(validate_with_type_mismatch) {
318+
auto cfg = test_config();
319+
320+
// Provide string where int is expected
321+
auto invalid_yaml = YAML::Load("optional_int: not_an_int\n");
322+
323+
auto errors = cfg.read_yaml(invalid_yaml);
324+
325+
BOOST_REQUIRE(!errors.empty());
326+
auto& [key, msg] = *errors.begin();
327+
BOOST_TEST(key == "optional_int");
328+
BOOST_TEST_INFO(msg);
329+
BOOST_TEST(msg.contains("bad conversion"));
330+
331+
BOOST_TEST(cfg.optional_int() == 100); // expect default retained
332+
}
333+
334+
SEASTAR_THREAD_TEST_CASE(validate_with_throwing_validator) {
335+
auto cfg = test_config();
336+
337+
auto yaml = YAML::Load("throwing_validator_string: throw\n");
338+
339+
BOOST_CHECK_THROW(cfg.read_yaml(yaml), std::runtime_error);
340+
}
341+
342+
SEASTAR_THREAD_TEST_CASE(validate_required_with_validator_error) {
343+
auto cfg = required_validated_config();
344+
345+
auto invalid_yaml = YAML::Load(
346+
"required_validated_string: invalid_value\n");
347+
348+
// Required property with validation error throws std::invalid_argument
349+
// BOOST_CHECK_THROW(cfg.read_yaml(invalid_yaml), std::invalid_argument);
350+
351+
auto errors = cfg.read_yaml(invalid_yaml);
352+
BOOST_TEST(errors.size() > 0);
353+
BOOST_TEST(cfg.required_validated_string() == "invalid_value");
264354
}
265355

266356
SEASTAR_THREAD_TEST_CASE(config_json_serialization) {

tests/rptest/tests/cluster_config_test.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,38 @@ def test_unknown_redpanda_yaml(self):
205205
)
206206

207207

208+
class ClusterConfigBoundedPropertyTest(RedpandaTest):
209+
"""
210+
Test that bounded properties clamp out-of-range values from bootstrap config.
211+
"""
212+
213+
def __init__(self, *args, **kwargs):
214+
# Set log_segment_size to 100 bytes (below minimum of 1MB)
215+
super().__init__(
216+
*args,
217+
extra_rp_conf={"log_segment_size": 100},
218+
**kwargs,
219+
)
220+
221+
@cluster(num_nodes=1)
222+
def test_bounded_property_clamped_to_minimum(self):
223+
"""
224+
Verify that bounded properties are clamped to their minimum bound when
225+
an out-of-range value is provided in bootstrap config. Setting
226+
log_segment_size to 100 should result in the value being clamped to
227+
1MB (the minimum bound).
228+
"""
229+
admin = Admin(self.redpanda)
230+
config = admin.get_cluster_config()
231+
232+
# The value should be clamped to 1MB (1048576 bytes)
233+
min_segment_size = 1024 * 1024 # 1 MiB
234+
assert config["log_segment_size"] == min_segment_size, (
235+
f"Expected log_segment_size to be clamped to {min_segment_size} (1MB), "
236+
f"but got {config['log_segment_size']}"
237+
)
238+
239+
208240
class HasRedpandaAndAdmin(Protocol):
209241
redpanda: RedpandaService
210242
admin: Admin

0 commit comments

Comments
 (0)