clintropolis commented on a change in pull request #10314:
URL: https://github.com/apache/druid/pull/10314#discussion_r533817780
##########
File path:
extensions-core/avro-extensions/src/main/java/org/apache/druid/data/input/avro/SchemaRegistryBasedAvroBytesDecoder.java
##########
@@ -63,7 +73,7 @@ public GenericRecord parse(ByteBuffer bytes)
int id = bytes.getInt(); // extract schema registry id
int length = bytes.limit() - 1 - 4;
int offset = bytes.position() + bytes.arrayOffset();
- Schema schema = registry.getByID(id);
+ Schema schema = registry.getById(id);
Review comment:
it seems like this method is deprecated as well, but it doesn't really
seem like there is a better replacement since the underlying method is doing
this
```
ParsedSchema schema = this.getSchemaById(id);
return schema instanceof AvroSchema ? ((AvroSchema)schema).rawSchema() :
null;
```
##########
File path: docs/ingestion/data-formats.md
##########
@@ -1020,6 +1026,27 @@ For details, see the Schema Registry
[documentation](http://docs.confluent.io/cu
...
```
+Multiple Instances:
+```json
+...
+"avroBytesDecoder" : {
+ "type" : "schema_registry",
+ "urls" : [<schema-registry-url-1>, <schema-registry-url-2>, ...],
+ "config" : {
+ "schema.registry.basic.auth.credentials.source" : "USER_INFO",
+ "schema.registry.basic.auth.user.info" :
"fred:letmein",
+ ...
+ },
+ "headers": {
+ "traceID" : "b29c5de2-0db4-490b-b421",
+ "timeStamp" : "1577191871865",
+ ...
+
+ }
Review comment:
The basic auth example I don't think works anymore, looking at the code
underneath it looks like the `schema.registry` prefix is only for the ssl config
```
if (configs != null && !configs.isEmpty()) {
restService.configure(configs);
Map<String, Object> sslConfigs =
(Map)configs.entrySet().stream().filter((e) -> {
return ((String)e.getKey()).startsWith("schema.registry.");
}).collect(Collectors.toMap((e) -> {
return ((String)e.getKey()).substring("schema.registry.".length());
}, Entry::getValue));
SslFactory sslFactory = new SslFactory(sslConfigs);
if (sslFactory != null && sslFactory.sslContext() != null) {
restService.setSslSocketFactory(sslFactory.sslContext().getSocketFactory());
}
}
```
where `restService` is not considering the prefix:
```
String basicCredentialsSource =
(String)configs.get("basic.auth.credentials.source");
String bearerCredentialsSource =
(String)configs.get("bearer.auth.credentials.source");
...
```
After reading through
https://docs.confluent.io/platform/current/schema-registry/security/index.html#additional-configurations-for-https,
should we also include the SSL config since it seems relevant if connecting to
https?
Also, formatting seems a bit off here compared to the other JSON example:
```suggestion
"type" : "schema_registry",
"urls" : [<schema-registry-url-1>, <schema-registry-url-2>, ...],
"config" : {
"basic.auth.credentials.source": "USER_INFO",
"basic.auth.user.info": "fred:letmein",
"schema.registry.ssl.truststore.location":
"/some/secrets/kafka.client.truststore.jks",
"schema.registry.ssl.truststore.password": "<password>",
"schema.registry.ssl.keystore.location":
"/some/secrets/kafka.client.keystore.jks",
"schema.registry.ssl.keystore.password": "<password>",
"schema.registry.ssl.key.password": "<password>"
...
},
"headers": {
"traceID" : "b29c5de2-0db4-490b-b421",
"timeStamp" : "1577191871865",
...
}
```
##########
File path: licenses.yaml
##########
@@ -3081,13 +3081,105 @@ notices:
---
name: Kafka Schema Registry Client
-version: 3.0.1
+version: 5.5.1
license_category: binary
module: extensions/druid-avro-extensions
license_name: Apache License version 2.0
libraries:
- io.confluent: kafka-schema-registry-client
+ - io.confluent: common-config
+ - io.confluent: common-utils
+
+---
+
+name: com.101tec zkclient
+license_category: binary
+version: '0.10'
+module: druid-ranger-security
Review comment:
nit: did this get tagged with an incorrect module or was it just missing
from the file? (it doesn't seem like there were any changes to that extension)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
