Skip to content

Commit acb2499

Browse files
committed
fix: handling of explicit configuration properties with @registryproperties
Test `io.apicurio.registry.utils.tests.KafkaSqlStartupVerificationIT#testBadSnapshotsTopicConfig2Override` in extra tests is failing because `apicurio.kafkasql.topic-configuration-verification-override-enabled` property has the prefix `apicurio.kafkasql.topic` which propagates it into topic configuration when creating the topic. The same issue is with `apicurio.kafkasql.topic.auto-create`, but we did not notice it because nobody has passed `-Dapicurio.kafkasql.topic.auto-create=true` to Registry before, since it's the default. This fixes the issue.
1 parent 3482721 commit acb2499

File tree

5 files changed

+71
-37
lines changed

5 files changed

+71
-37
lines changed

app/src/main/java/io/apicurio/registry/rules/RulesConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public class RulesConfiguration {
1212
@Produces
1313
@ApplicationScoped
1414
public RulesProperties rulesProperties(
15-
@RegistryProperties(value = { "apicurio.rules.global" }) Properties properties) {
15+
@RegistryProperties(prefixes = { "apicurio.rules.global" }) Properties properties) {
1616
return new RulesPropertiesImpl(properties);
1717
}
1818

app/src/main/java/io/apicurio/registry/storage/impl/kafkasql/KafkaSqlConfiguration.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public Duration getResponseTimeout() {
7676
String topic;
7777

7878
@Inject
79-
@RegistryProperties(value = "apicurio.kafkasql.topic")
79+
@RegistryProperties(prefixes = "apicurio.kafkasql.topic", excluded = {"auto-create", "configuration-verification-override-enabled"})
8080
@Info(category = CATEGORY_STORAGE, description = """
8181
Kafka sql storage topic properties. \
8282
There are two optional Registry-specific configuration properties: 'partitions' and 'replication.factor'.""")
@@ -105,7 +105,7 @@ public Map<String, String> getTopicProperties() {
105105
String snapshotsTopic;
106106

107107
@Inject
108-
@RegistryProperties(value = "apicurio.kafkasql.snapshots.topic")
108+
@RegistryProperties(prefixes = "apicurio.kafkasql.snapshots.topic")
109109
@Info(category = CATEGORY_STORAGE, description = """
110110
Kafka sql snapshots topic properties. \
111111
There are two optional Registry-specific configuration properties: 'partitions' and 'replication.factor'. \
@@ -148,7 +148,7 @@ public Map<String, String> getSnapshotTopicProperties() {
148148
String eventsTopic;
149149

150150
@Inject
151-
@RegistryProperties(value = "apicurio.events.kafka.topic")
151+
@RegistryProperties(prefixes = "apicurio.events.kafka.topic")
152152
@Info(category = CATEGORY_STORAGE, description = """
153153
Kafka sql events topic properties. \
154154
There is an optional Registry-specific configuration property: 'replication.factor'. \
@@ -189,7 +189,7 @@ public Duration getPollTimeout() {
189189
String groupPrefix;
190190

191191
@Inject
192-
@RegistryProperties(value = {"apicurio.kafka.common", "apicurio.kafkasql.consumer"}, empties = {"ssl.endpoint.identification.algorithm="})
192+
@RegistryProperties(prefixes = {"apicurio.kafka.common", "apicurio.kafkasql.consumer"}, defaults = {"ssl.endpoint.identification.algorithm="})
193193
Properties consumerProperties;
194194

195195
public Map<String, String> getConsumerProperties() {
@@ -213,7 +213,7 @@ public Map<String, String> getConsumerProperties() {
213213
// === Producer configurations ===
214214

215215
@Inject
216-
@RegistryProperties(value = {"apicurio.kafka.common", "apicurio.kafkasql.producer"}, empties = {"ssl.endpoint.identification.algorithm="})
216+
@RegistryProperties(prefixes = {"apicurio.kafka.common", "apicurio.kafkasql.producer"}, defaults = {"ssl.endpoint.identification.algorithm="})
217217
Properties producerProperties;
218218

219219
public Map<String, String> getProducerProperties() {
@@ -236,7 +236,7 @@ public Map<String, String> getProducerProperties() {
236236
// === Admin client configurations ===
237237

238238
@Inject
239-
@RegistryProperties(value = {"apicurio.kafka.common", "apicurio.kafkasql.admin"}, empties = {"ssl.endpoint.identification.algorithm="})
239+
@RegistryProperties(prefixes = {"apicurio.kafka.common", "apicurio.kafkasql.admin"}, defaults = {"ssl.endpoint.identification.algorithm="})
240240
Properties adminProperties;
241241

242242
public Map<String, String> getAdminProperties() {

common/src/main/java/io/apicurio/registry/utils/PropertiesUtil.java

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import java.util.Arrays;
1010
import java.util.HashMap;
11-
import java.util.Map;
11+
import java.util.HashSet;
1212
import java.util.Optional;
1313
import java.util.Properties;
1414
import java.util.stream.Collectors;
@@ -27,52 +27,68 @@ public class PropertiesUtil {
2727
* @return filtered/stripped properties
2828
*/
2929
public static Properties properties(InjectionPoint ip) {
30+
3031
RegistryProperties cp = ip.getAnnotated().getAnnotation(RegistryProperties.class);
3132
if (cp == null) {
32-
throw new IllegalArgumentException(ip.getMember() + " is not annotated with @RegistryProperties");
33+
throw new IllegalArgumentException(ip.getMember() + " must be annotated with @RegistryProperties.");
3334
}
34-
String[] prefixes = Stream.of(cp.value())
35+
36+
var prefixes = Stream.of(cp.prefixes())
3537
.map(pfx -> pfx.isEmpty() || pfx.endsWith(".") ? pfx : pfx + ".").distinct()
36-
.toArray(String[]::new);
37-
if (prefixes.length == 0) {
38+
.toList();
39+
40+
if (prefixes.isEmpty()) {
3841
throw new IllegalArgumentException("Annotation @RegistryProperties on " + ip.getMember()
39-
+ " is missing non-empty 'value' attribute");
42+
+ " must have a non-empty 'prefixes' attribute.");
4043
}
4144

4245
Properties properties = new Properties();
4346
Config config = ConfigProviderResolver.instance().getConfig();
4447

4548
if (debug && log.isDebugEnabled()) {
46-
String dump = StreamSupport.stream(config.getPropertyNames().spliterator(), false).sorted()
49+
String dump = StreamSupport.stream(config.getPropertyNames().spliterator(), false)
50+
.sorted()
4751
.map(key -> key + "=" + config.getOptionalValue(key, String.class).orElse(""))
4852
.collect(Collectors.joining("\n ", " ", "\n"));
4953
log.debug("Injecting config properties with prefixes {} into {} from the following...\n{}",
50-
Arrays.toString(prefixes), ip.getMember(), dump);
54+
prefixes, ip.getMember(), dump);
5155
}
5256

53-
// some security properties take empty value as config
54-
Map<String, String> defaults = new HashMap<>();
55-
if (cp != null) {
56-
String[] empties = cp.empties();
57-
for (String e : empties) {
58-
int p = e.indexOf("=");
59-
defaults.put(e.substring(0, p), e.substring(p + 1));
60-
}
57+
var defaults = new HashMap<String, String>();
58+
for (String d : cp.defaults()) {
59+
int p = d.indexOf("=");
60+
defaults.put(d.substring(0, p), d.substring(p + 1));
6161
}
6262

63-
// collect properties with specified prefixes in order of prefixes (later prefix overrides earlier)
63+
var excludedList = Arrays.asList(cp.excluded());
64+
var excluded = new HashSet<>();
65+
// When the property is passed as an env. variable, dashes are interpreted as dots,
66+
// so we need to exclude both forms.
67+
excluded.addAll(excludedList);
68+
excluded.addAll(excludedList.stream().map(s -> s.replace('-', '.')).toList());
69+
70+
// Collect properties with specified prefixes in order of prefixes (later prefix overrides earlier)
6471
for (String prefix : prefixes) {
6572
for (String key : config.getPropertyNames()) {
6673
if (key.startsWith(prefix)) {
67-
// property can exist with key, but no value ...
68-
Optional<String> value = config.getOptionalValue(key, String.class);
69-
if (value.isPresent()) {
70-
properties.put(key.substring(prefix.length()), value.get());
71-
} else if (defaults.size() > 0) {
72-
String sKey = key.substring(prefix.length());
73-
String defaultValue = defaults.get(sKey);
74-
if (defaultValue != null) {
75-
properties.put(sKey, defaultValue);
74+
var suffix = key.substring(prefix.length());
75+
if (!suffix.isEmpty()) {
76+
log.debug("Processing property '{}'", key);
77+
if (!excluded.contains(suffix)) {
78+
// Property can exist with a key but no value...
79+
Optional<String> value = config.getOptionalValue(key, String.class);
80+
if (value.isPresent()) {
81+
properties.put(suffix, value.get());
82+
} else {
83+
String defaultValue = defaults.get(suffix);
84+
if (defaultValue != null) {
85+
properties.put(suffix, defaultValue);
86+
} else {
87+
log.debug("Property '{}' has no value and no default, skipping.", key);
88+
}
89+
}
90+
} else {
91+
log.debug("Property '{}' is excluded, skipping.", key);
7692
}
7793
}
7894
}

common/src/main/java/io/apicurio/registry/utils/RegistryProperties.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,27 @@
77
import static java.lang.annotation.RetentionPolicy.RUNTIME;
88

99
@Retention(RUNTIME)
10-
@Target({ ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER })
10+
@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER})
1111
public @interface RegistryProperties {
12-
String[] value();
1312

14-
String[] empties() default {};
13+
/**
14+
* The prefixes of keys of configuration properties that should be included.
15+
* Keys with a later prefix in the array override the earlier ones.
16+
*/
17+
String[] prefixes();
18+
19+
/**
20+
* If a configuration property (excluding the prefix) key is found, but has no value,
21+
* the default value from this array is used.
22+
* This can be used to set empty string values for properties that require it.
23+
* Each entry must be of the form "key-prefix=value".
24+
*/
25+
String[] defaults() default {};
26+
27+
/**
28+
* Keys of configuration properties (excluding the prefix) that should be ignored.
29+
* This is useful, for example, if we have defined standard configuration property
30+
* in addition to a prefix-based set, and we don't want it to be included in the resulting Properties object.
31+
*/
32+
String[] excluded() default {};
1533
}

docs/config-generator/src/main/java/io/apicurio/registry/docs/GenerateAllConfigPartial.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ public static Map<String, Option> extractConfigurations(String jarFile, Map<Stri
269269
.orElse(""));
270270

271271
// Get the property prefixes from @RegistryProperties annotation
272-
var values = annotation.value("value").asStringArray();
272+
var values = annotation.value("prefixes").asStringArray();
273273

274274
for (String prefix : values) {
275275
prefix = prefix.replace("app.authn.", "apicurio.auth.");

0 commit comments

Comments
 (0)