You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/07/26 18:34:23 UTC

[GitHub] [incubator-pinot] pradeepgv42 opened a new pull request #5758: Pradeep/sr ssl fix

pradeepgv42 opened a new pull request #5758:
URL: https://github.com/apache/incubator-pinot/pull/5758


   Add ability to pass SSL certs info to schema registry.
   
   Example Config to pass SSL certs to schema registry in the table config follows:
         "stream.kafka.decoder.prop.schema.registry.ssl.truststore.location": "",
         "stream.kafka.decoder.prop.schema.registry.ssl.keystore.location": "",
         "stream.kafka.decoder.prop.schema.registry.ssl.truststore.password": "",
         "stream.kafka.decoder.prop.schema.registry.ssl.keystore.password": "",
         "stream.kafka.decoder.prop.schema.registry.ssl.keystore.type": "",
         "stream.kafka.decoder.prop.schema.registry.ssl.truststore.type": "",
         "stream.kafka.decoder.prop.schema.registry.ssl.key.password": "",
         "stream.kafka.decoder.prop.schema.registry.ssl.protocol": "SSL",


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] kishoreg commented on pull request #5758: Pradeep/sr ssl fix

Posted by GitBox <gi...@apache.org>.
kishoreg commented on pull request #5758:
URL: https://github.com/apache/incubator-pinot/pull/5758#issuecomment-665172447


   @elonazoulay @daniellavoie can you please review this?


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] elonazoulay commented on pull request #5758: Pradeep/sr ssl fix

Posted by GitBox <gi...@apache.org>.
elonazoulay commented on pull request #5758:
URL: https://github.com/apache/incubator-pinot/pull/5758#issuecomment-669427326


   Looks great, thanks @pradeepgv42 !


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] elonazoulay commented on a change in pull request #5758: Pradeep/sr ssl fix

Posted by GitBox <gi...@apache.org>.
elonazoulay commented on a change in pull request #5758:
URL: https://github.com/apache/incubator-pinot/pull/5758#discussion_r461731334



##########
File path: pinot-plugins/pinot-input-format/pinot-confluent-avro/src/main/java/org/apache/pinot/plugin/inputformat/avro/confluent/KafkaConfluentSchemaRegistryAvroMessageDecoder.java
##########
@@ -46,12 +54,50 @@
   private RecordExtractor<Record> _avroRecordExtractor;
   private String _topicName;
 
+  public RestService createRestService(String schemaRegistryUrl, Map<String, String> configs) {
+    RestService restService = new RestService(schemaRegistryUrl);
+    Map<String, Object> asslConfigs = configs.entrySet().stream()

Review comment:
       Why not rename to `sslConfigs`?

##########
File path: pinot-plugins/pinot-input-format/pinot-confluent-avro/src/main/java/org/apache/pinot/plugin/inputformat/avro/confluent/KafkaConfluentSchemaRegistryAvroMessageDecoder.java
##########
@@ -46,12 +54,50 @@
   private RecordExtractor<Record> _avroRecordExtractor;
   private String _topicName;
 
+  public RestService createRestService(String schemaRegistryUrl, Map<String, String> configs) {
+    RestService restService = new RestService(schemaRegistryUrl);
+    Map<String, Object> asslConfigs = configs.entrySet().stream()
+            .filter(e -> !e.equals(SCHEMA_REGISTRY_REST_URL))
+            .filter(e -> e.getKey().startsWith("schema.registry."))
+            .collect(Collectors.toMap(
+                    e -> e.getKey().substring("schema.registry.".length()),
+                    Map.Entry::getValue));
+
+    Map<String, Object> sslConfigs = new HashMap<>();
+    for (String key : configs.keySet()) {
+      if (!key.equals(SCHEMA_REGISTRY_REST_URL) && key.startsWith("schema.registry.")) {

Review comment:
       Can you use the Configuration library, instead of manually extracting the substring?

##########
File path: pinot-plugins/pinot-input-format/pinot-confluent-avro/src/main/java/org/apache/pinot/plugin/inputformat/avro/confluent/KafkaConfluentSchemaRegistryAvroMessageDecoder.java
##########
@@ -46,12 +54,50 @@
   private RecordExtractor<Record> _avroRecordExtractor;
   private String _topicName;
 
+  public RestService createRestService(String schemaRegistryUrl, Map<String, String> configs) {
+    RestService restService = new RestService(schemaRegistryUrl);
+    Map<String, Object> asslConfigs = configs.entrySet().stream()
+            .filter(e -> !e.equals(SCHEMA_REGISTRY_REST_URL))
+            .filter(e -> e.getKey().startsWith("schema.registry."))
+            .collect(Collectors.toMap(
+                    e -> e.getKey().substring("schema.registry.".length()),
+                    Map.Entry::getValue));
+
+    Map<String, Object> sslConfigs = new HashMap<>();
+    for (String key : configs.keySet()) {
+      if (!key.equals(SCHEMA_REGISTRY_REST_URL) && key.startsWith("schema.registry.")) {
+        String value = configs.get(key);
+        key = key.substring("schema.registry.".length());
+        if (key.contains("password")) {
+          sslConfigs.put(key, new Password(value));
+        } else {
+          sslConfigs.put(key, value);
+        }
+      }
+    }
+
+    System.out.println(configs.toString());

Review comment:
       Can you use a logger to output these?
   If this just for debugging you can just do log.debug()




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] kishoreg merged pull request #5758: Pradeep/sr ssl fix

Posted by GitBox <gi...@apache.org>.
kishoreg merged pull request #5758:
URL: https://github.com/apache/incubator-pinot/pull/5758


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] pradeepgv42 commented on a change in pull request #5758: Pradeep/sr ssl fix

Posted by GitBox <gi...@apache.org>.
pradeepgv42 commented on a change in pull request #5758:
URL: https://github.com/apache/incubator-pinot/pull/5758#discussion_r461904284



##########
File path: pinot-plugins/pinot-input-format/pinot-confluent-avro/src/main/java/org/apache/pinot/plugin/inputformat/avro/confluent/KafkaConfluentSchemaRegistryAvroMessageDecoder.java
##########
@@ -46,12 +54,50 @@
   private RecordExtractor<Record> _avroRecordExtractor;
   private String _topicName;
 
+  public RestService createRestService(String schemaRegistryUrl, Map<String, String> configs) {
+    RestService restService = new RestService(schemaRegistryUrl);
+    Map<String, Object> asslConfigs = configs.entrySet().stream()

Review comment:
       @elonazoulay This is an outdated copy of the code. Can you look at this
   https://github.com/apache/incubator-pinot/pull/5758/files

##########
File path: pinot-plugins/pinot-input-format/pinot-confluent-avro/src/main/java/org/apache/pinot/plugin/inputformat/avro/confluent/KafkaConfluentSchemaRegistryAvroMessageDecoder.java
##########
@@ -46,12 +54,50 @@
   private RecordExtractor<Record> _avroRecordExtractor;
   private String _topicName;
 
+  public RestService createRestService(String schemaRegistryUrl, Map<String, String> configs) {
+    RestService restService = new RestService(schemaRegistryUrl);
+    Map<String, Object> asslConfigs = configs.entrySet().stream()
+            .filter(e -> !e.equals(SCHEMA_REGISTRY_REST_URL))
+            .filter(e -> e.getKey().startsWith("schema.registry."))
+            .collect(Collectors.toMap(
+                    e -> e.getKey().substring("schema.registry.".length()),
+                    Map.Entry::getValue));
+
+    Map<String, Object> sslConfigs = new HashMap<>();
+    for (String key : configs.keySet()) {
+      if (!key.equals(SCHEMA_REGISTRY_REST_URL) && key.startsWith("schema.registry.")) {
+        String value = configs.get(key);
+        key = key.substring("schema.registry.".length());
+        if (key.contains("password")) {
+          sslConfigs.put(key, new Password(value));
+        } else {
+          sslConfigs.put(key, value);
+        }
+      }
+    }
+
+    System.out.println(configs.toString());

Review comment:
       @elonazoulay This is an outdated copy of the code. Can you look at this
   https://github.com/apache/incubator-pinot/pull/5758/files
   
   

##########
File path: pinot-plugins/pinot-input-format/pinot-confluent-avro/src/main/java/org/apache/pinot/plugin/inputformat/avro/confluent/KafkaConfluentSchemaRegistryAvroMessageDecoder.java
##########
@@ -46,12 +54,50 @@
   private RecordExtractor<Record> _avroRecordExtractor;
   private String _topicName;
 
+  public RestService createRestService(String schemaRegistryUrl, Map<String, String> configs) {
+    RestService restService = new RestService(schemaRegistryUrl);
+    Map<String, Object> asslConfigs = configs.entrySet().stream()
+            .filter(e -> !e.equals(SCHEMA_REGISTRY_REST_URL))
+            .filter(e -> e.getKey().startsWith("schema.registry."))
+            .collect(Collectors.toMap(
+                    e -> e.getKey().substring("schema.registry.".length()),
+                    Map.Entry::getValue));
+
+    Map<String, Object> sslConfigs = new HashMap<>();
+    for (String key : configs.keySet()) {
+      if (!key.equals(SCHEMA_REGISTRY_REST_URL) && key.startsWith("schema.registry.")) {

Review comment:
       Sorry not very familiar with this, do you have an example or give a bit more info? 




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] pradeepgv42 commented on pull request #5758: Pradeep/sr ssl fix

Posted by GitBox <gi...@apache.org>.
pradeepgv42 commented on pull request #5758:
URL: https://github.com/apache/incubator-pinot/pull/5758#issuecomment-668125822


   @elonazoulay could you review this when you get a chance? 


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org