Commit 4b953f7
committed
config: don't apply values that fail validation
Previously, config_store::read_yaml() would apply property values even
when custom validation failed, only recording the error. This meant
invalid configuration values would silently take effect despite being
reported as errors.
This behavior was also inconsistent: conversion exceptions (e.g.,
YAML::BadConversion) correctly prevented values from being set, but
custom validator errors did not.
Fix by moving set_value() inside the else branch so values are only
applied when validation succeeds. This has two effects:
1. Optional properties now correctly retain their default value when
validation fails.
2. Required properties now correctly throw std::invalid_argument when
validation fails. Previously, the throw logic was bypassed because
ok was set to true even after validation errors.
Note: It's unclear why we don't stop server startup on invalid optional
config values. To avoid breaking something I don't fully understand,
this change takes the conservative approach of retaining defaults.1 parent 4d63f53 commit 4b953f7
File tree
8 files changed
+37
-12
lines changed- src/v/config
- tests
- tests/rptest/tests
8 files changed
+37
-12
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
94 | 94 | | |
95 | 95 | | |
96 | 96 | | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
97 | 100 | | |
98 | | - | |
99 | | - | |
100 | 101 | | |
101 | 102 | | |
102 | 103 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
309 | 309 | | |
310 | 310 | | |
311 | 311 | | |
312 | | - | |
313 | | - | |
314 | | - | |
| 312 | + | |
315 | 313 | | |
316 | 314 | | |
317 | 315 | | |
| |||
346 | 344 | | |
347 | 345 | | |
348 | 346 | | |
349 | | - | |
350 | | - | |
351 | | - | |
352 | | - | |
353 | | - | |
| 347 | + | |
354 | 348 | | |
355 | 349 | | |
356 | 350 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
150 | 150 | | |
151 | 151 | | |
152 | 152 | | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
153 | 157 | | |
154 | 158 | | |
155 | 159 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1232 | 1232 | | |
1233 | 1233 | | |
1234 | 1234 | | |
| 1235 | + | |
| 1236 | + | |
| 1237 | + | |
| 1238 | + | |
1235 | 1239 | | |
1236 | 1240 | | |
1237 | 1241 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
70 | 70 | | |
71 | 71 | | |
72 | 72 | | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
73 | 77 | | |
74 | 78 | | |
75 | 79 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
63 | 67 | | |
64 | 68 | | |
65 | 69 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1025 | 1025 | | |
1026 | 1026 | | |
1027 | 1027 | | |
1028 | | - | |
| 1028 | + | |
| 1029 | + | |
| 1030 | + | |
| 1031 | + | |
1029 | 1032 | | |
1030 | 1033 | | |
1031 | 1034 | | |
| |||
1094 | 1097 | | |
1095 | 1098 | | |
1096 | 1099 | | |
1097 | | - | |
| 1100 | + | |
| 1101 | + | |
| 1102 | + | |
| 1103 | + | |
1098 | 1104 | | |
1099 | 1105 | | |
1100 | 1106 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
254 | 254 | | |
255 | 255 | | |
256 | 256 | | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
257 | 261 | | |
258 | 262 | | |
259 | 263 | | |
| |||
457 | 461 | | |
458 | 462 | | |
459 | 463 | | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
460 | 468 | | |
461 | 469 | | |
462 | 470 | | |
| |||
0 commit comments