goutamadwant commented on code in PR #19202:
URL: https://github.com/apache/pinot/pull/19202#discussion_r3755320649
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/ConfigUtils.java:
##########
@@ -86,13 +96,17 @@ private static JsonNode
applyConfigWithEnvVariablesAndSystemProperties(Map<Strin
ArrayNode arrayNode = (ArrayNode) jsonNode;
for (int i = 0; i < arrayNode.size(); i++) {
JsonNode arrayElement = arrayNode.get(i);
- arrayNode.set(i,
applyConfigWithEnvVariablesAndSystemProperties(configValues, arrayElement));
+ arrayNode.set(i,
+ applyConfigWithEnvVariablesAndSystemProperties(configValues,
arrayElement, inheritedConfigProviders));
}
}
break;
case STRING:
final String field = jsonNode.asText();
- if (field.startsWith("${") && field.endsWith("}")) {
+ // Kafka ConfigProvider references share Pinot's ${name:value} syntax.
Keep references for providers declared
+ // in the containing config object so Kafka can resolve them when
constructing the client.
+ if (field.startsWith("${") && field.endsWith("}")
+ && !isConfigProviderReference(field, inheritedConfigProviders)) {
Review Comment:
Addressed in the shared Kafka helper. After Pinot unescapes `$${...}`,
client property preparation validates every Kafka provider reference and throws
`ConfigException` if the alias is not listed in `config.providers` or its class
is missing. I added negative tests for both cases, so an escaped but incomplete
provider setup fails during client creation instead of becoming a path-like
password.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
