You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "cgivre (via GitHub)" <gi...@apache.org> on 2023/04/23 19:06:51 UTC

[GitHub] [calcite] cgivre opened a new pull request, #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

cgivre opened a new pull request, #3174:
URL: https://github.com/apache/calcite/pull/3174

   For many development (and sometimes production) instances of ElasticSearch, the instance is configured using a self-signed SSL certificate or otherwise cannot be validated.  
   
   This PR adds a configuration option `disableSSLVerification` which, when set to `true`, disables SSL verification.  The default behavior is not changed.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ElasticSearch Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1529747852

   @libenchao Will this get merged?  I saw the PR was closed.  Thanks!


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1523584945

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![33.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/25-16px.png '33.3%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [33.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1175281819


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -131,10 +135,20 @@ public ElasticsearchSchemaFactory() {
         ("Both 'coordinates' and 'hosts' is missing in configuration. Provide one of them.");
       }
       final String pathPrefix = (String) map.get("pathPrefix");
+
+      // Add or Disable SSL Verification
+      boolean disableSSLVerification;
+      if (map.containsKey("disableSSLVerification")) {
+        disableSSLVerification = (Boolean) map.get("disableSSLVerification");

Review Comment:
   Fixed.  I broke this into two steps to avoid a possible NPE.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1523592377

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![33.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/25-16px.png '33.3%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [33.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1520551672

   @libenchao @zinking 
   Thanks for your comments.  I addressed your comments and added a unit test.  I'm using this in Drill, so once this is merged, and [DRILL-8385](https://github.com/apache/drill/pull/2795) is merged, there will be end-to-end tests for this capability.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1519141419

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![D](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D-16px.png 'D')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [7 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] github-code-scanning[bot] commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "github-code-scanning[bot] (via GitHub)" <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1174633639


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ *
+ *
+ * <p>This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates, String authType) {

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevnJF5Hs0MTalI-jiR-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevnJF5Hs0MTalI-jiR&open=AYevnJF5Hs0MTalI-jiR&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/12)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ *
+ *
+ * <p>This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates,
+      String authType, Socket socket) {
+    // No op
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates,
+      String authType, SSLEngine sslEngine) {
+    // No op
+  }
+
+  @Override public void checkServerTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override public void checkServerTrusted(X509Certificate[] certificates,

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevpel6oMuZgdLV3OOz-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevpel6oMuZgdLV3OOz&open=AYevpel6oMuZgdLV3OOz&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/19)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ *
+ *
+ * <p>This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates,
+      String authType, Socket socket) {
+    // No op
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates,

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevpel6oMuZgdLV3OOy-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevpel6oMuZgdLV3OOy&open=AYevpel6oMuZgdLV3OOy&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/18)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ *
+ *
+ * <p>This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates,
+      String authType, Socket socket) {
+    // No op
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates,
+      String authType, SSLEngine sslEngine) {
+    // No op
+  }
+
+  @Override public void checkServerTrusted(X509Certificate[] certificates, String authType) {

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevnJF5Hs0MTalI-jiU-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevnJF5Hs0MTalI-jiU&open=AYevnJF5Hs0MTalI-jiU&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/15)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ *
+ *
+ * <p>This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates,
+      String authType, Socket socket) {
+    // No op
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates,
+      String authType, SSLEngine sslEngine) {
+    // No op
+  }
+
+  @Override public void checkServerTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override public void checkServerTrusted(X509Certificate[] certificates,
+      String authType, Socket socket) {
+    // No op
+  }
+
+  @Override public void checkServerTrusted(X509Certificate[] certificates,

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevpel6oMuZgdLV3OO0-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevpel6oMuZgdLV3OO0&open=AYevpel6oMuZgdLV3OO0&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/17)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ *
+ *
+ * <p>This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override public void checkClientTrusted(X509Certificate[] certificates,

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevnJF5Hs0MTalI-jiS-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevnJF5Hs0MTalI-jiS&open=AYevnJF5Hs0MTalI-jiS&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/13)



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1179878144


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not. This should <b>not</b> be
+ * used in production environments.
+ */
+@SuppressWarnings("java:S4830")
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {

Review Comment:
   @libenchao Thanks for the comment.  I updated the comment to reflect that this could be used for any calcite adapter that makes http calls. 



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1177173873


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -173,6 +189,16 @@ private static RestClient connect(List<HttpHost> hosts, String pathPrefix,
                 httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider));
           }
 
+          if (disableSSLVerification) {

Review Comment:
   Done!



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1177173369


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -93,6 +98,27 @@ public class ElasticsearchSchemaFactory implements SchemaFactory {
   public ElasticsearchSchemaFactory() {
   }
 
+  /**
+   * Create an ElasticSearch {@link Schema}.

Review Comment:
   YW!



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] libenchao commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1521568037

   > I'm using this in Drill, so once this is merged, and https://github.com/apache/drill/pull/2795 is merged, there will be end-to-end tests for this capability.
   
   This sounds great!  +1 for merging.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1520437685

   > For future reference I believe you can run the gradle target "autostyleApply" to hopefully avoid having to manually fix some of the linting errors. Hope this helps!
   
   Thanks @tanclary Still getting used to Calcite and gradle.  


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1522565801

   @libenchao Thanks for the review.  This is my first contribution to Calcite, so is there anything left for me to do to get this merged?


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1519139690

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![D](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D-16px.png 'D')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [7 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] jnturton commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1177993034


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not. This should <b>not</b> be
+ * used in production environments.
+ */
+@SuppressWarnings("java:S4830")
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {

Review Comment:
   @cgivre there is an org.apache.calcite.util package in the core module that looks like it would work.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1520551541

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![33.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/25-16px.png '33.3%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [33.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1177173476


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -151,18 +188,20 @@ public ElasticsearchSchemaFactory() {
    * @param password the password of ES
    * @return new or cached low-level rest http client for ES
    */
+  @SuppressWarnings({"java:S4830", "java:S5527"})
   private static RestClient connect(List<HttpHost> hosts, String pathPrefix,
-                                    String username, String password) {
+                                    String username, String password,
+                                    boolean disableSSLVerification) {
 
     Objects.requireNonNull(hosts, "hosts or coordinates");
     Preconditions.checkArgument(!hosts.isEmpty(), "no ES hosts specified");
     // Two lists are considered equal when all of their corresponding elements are equal
-    // making a list of RestClient parms a suitable cache key.
+    // making a list of RestClient params a suitable cache key.

Review Comment:
   :-p



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1526942938

   @libenchao Thanks for the review!


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1526913652

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![33.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/25-16px.png '33.3%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [33.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1519144916

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![D](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D-16px.png 'D')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [7 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] zinking commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "zinking (via GitHub)" <gi...@apache.org>.
zinking commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1175172824


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -131,10 +135,20 @@ public ElasticsearchSchemaFactory() {
         ("Both 'coordinates' and 'hosts' is missing in configuration. Provide one of them.");
       }
       final String pathPrefix = (String) map.get("pathPrefix");
+
+      // Add or Disable SSL Verification
+      boolean disableSSLVerification;
+      if (map.containsKey("disableSSLVerification")) {
+        disableSSLVerification = (Boolean) map.get("disableSSLVerification");

Review Comment:
   cast string directly to boolean ? maybe guarding against cast error ?



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] zinking commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "zinking (via GitHub)" <gi...@apache.org>.
zinking commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1520004808

   looks good overall, could you actually try to come up with some test?


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1519135094

   @jnturton FYSA


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] libenchao commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ElasticSearch Adapter

Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1529750846

   @cgivre This has been merged in https://github.com/apache/calcite/commit/dd8fa24a6a662e7e82780a940af4b3036c2adb23, you can see it in git history: https://github.com/apache/calcite/commits


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1177999232


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not. This should <b>not</b> be
+ * used in production environments.
+ */
+@SuppressWarnings("java:S4830")
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {

Review Comment:
   @jnturton Good call.   Done!



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1520519667

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![33.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/25-16px.png '33.3%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [33.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1520556938

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![33.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/25-16px.png '33.3%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [33.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1519137175

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![D](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D-16px.png 'D')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [7 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] jnturton commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "jnturton (via GitHub)" <gi...@apache.org>.
jnturton commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1176608199


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -93,6 +98,27 @@ public class ElasticsearchSchemaFactory implements SchemaFactory {
   public ElasticsearchSchemaFactory() {
   }
 
+  /**
+   * Create an ElasticSearch {@link Schema}.

Review Comment:
   Thank you for this documentation.



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not. This should <b>not</b> be
+ * used in production environments.
+ */
+@SuppressWarnings("java:S4830")
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {

Review Comment:
   I don't know the Calcite code layout well but this trust manager is a generic utility that could easily be useful to other adapters so I'd personally look for a utils package to put it into.



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -151,18 +188,20 @@ public ElasticsearchSchemaFactory() {
    * @param password the password of ES
    * @return new or cached low-level rest http client for ES
    */
+  @SuppressWarnings({"java:S4830", "java:S5527"})
   private static RestClient connect(List<HttpHost> hosts, String pathPrefix,
-                                    String username, String password) {
+                                    String username, String password,
+                                    boolean disableSSLVerification) {
 
     Objects.requireNonNull(hosts, "hosts or coordinates");
     Preconditions.checkArgument(!hosts.isEmpty(), "no ES hosts specified");
     // Two lists are considered equal when all of their corresponding elements are equal
-    // making a list of RestClient parms a suitable cache key.
+    // making a list of RestClient params a suitable cache key.

Review Comment:
   I'm not suggesting any reversion here but "parm" is also a common abbreviation of parameter :)



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1177173654


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not. This should <b>not</b> be
+ * used in production environments.
+ */
+@SuppressWarnings("java:S4830")
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {

Review Comment:
   I was thinking the same thing, but I wasn't sure where to put it.  If anyone has any suggestions, I'm all ears.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] tanclary commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1520431397

   For future reference I believe you can run the gradle target "autostyleApply" to hopefully avoid having to manually fix some of the linting errors. Hope this helps!


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1175280102


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ *
+ *

Review Comment:
   Fixed



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ *
+ *
+ * <p>This should <b>not</b> be used in production environments.
+ * </p>

Review Comment:
   Fixed 



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust

Review Comment:
   Fixed



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] github-code-scanning[bot] commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "github-code-scanning[bot] (via GitHub)" <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1174630880


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.calcite.adapter.elasticsearch;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ * <p>
+ * This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType, Socket socket) {
+    // No op
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType, SSLEngine sslEngine) {
+    // No op
+  }
+
+  @Override
+  public void checkServerTrusted(X509Certificate[] certificates, String authType) {

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevjzkg2F6dJxSmTb8y-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevjzkg2F6dJxSmTb8y&open=AYevjzkg2F6dJxSmTb8y&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/9)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.calcite.adapter.elasticsearch;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ * <p>
+ * This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType, Socket socket) {
+    // No op
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType, SSLEngine sslEngine) {
+    // No op
+  }
+
+  @Override
+  public void checkServerTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override
+  public void checkServerTrusted(X509Certificate[] certificates, String authType, Socket socket) {
+    // No op
+  }
+
+  @Override
+  public void checkServerTrusted(X509Certificate[] certificates, String authType, SSLEngine sslEngine) {

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevjzkg2F6dJxSmTb80-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevjzkg2F6dJxSmTb80&open=AYevjzkg2F6dJxSmTb80&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/6)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.calcite.adapter.elasticsearch;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ * <p>
+ * This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType, Socket socket) {
+    // No op
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType, SSLEngine sslEngine) {

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevjzkg2F6dJxSmTb8x-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevjzkg2F6dJxSmTb8x&open=AYevjzkg2F6dJxSmTb8x&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/8)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.calcite.adapter.elasticsearch;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ * <p>
+ * This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType, Socket socket) {
+    // No op
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType, SSLEngine sslEngine) {
+    // No op
+  }
+
+  @Override
+  public void checkServerTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override
+  public void checkServerTrusted(X509Certificate[] certificates, String authType, Socket socket) {

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevjzkg2F6dJxSmTb8z-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevjzkg2F6dJxSmTb8z&open=AYevjzkg2F6dJxSmTb8z&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/10)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -173,6 +187,16 @@
                 httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider));
           }
 
+          if (disableSSLVerification) {
+            SSLContext sslContext = SSLContext.getInstance("TLS");
+            sslContext.init(null, new TrustManager[] { UnsafeX509ExtendedTrustManager.INSTANCE },
+                null);
+
+            builder.setHttpClientConfigCallback(httpClientBuilder ->
+                httpClientBuilder.setSSLContext(sslContext)
+                    .setSSLHostnameVerifier((host, session) -> true));

Review Comment:
   ## Server hostnames should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevjzgB2F6dJxSmTb8u-->Enable server hostname verification on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevjzgB2F6dJxSmTb8u&open=AYevjzgB2F6dJxSmTb8u&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/4)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.calcite.adapter.elasticsearch;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ * <p>
+ * This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType) {
+    // No op
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType, Socket socket) {

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevjzkg2F6dJxSmTb8w-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevjzkg2F6dJxSmTb8w&open=AYevjzkg2F6dJxSmTb8w&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/7)



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.calcite.adapter.elasticsearch;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ * <p>
+ * This should <b>not</b> be used in production environments.
+ * </p>
+ */
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {
+
+  /**
+   * Return a new instance of the unsafe, all-trusting trust manager.
+   */
+  static final X509ExtendedTrustManager INSTANCE = new UnsafeX509ExtendedTrustManager();
+  private static final X509Certificate[] EMPTY_CERTIFICATES = new X509Certificate[0];
+
+  private UnsafeX509ExtendedTrustManager() {}
+
+  public static X509ExtendedTrustManager getInstance() {
+    return INSTANCE;
+  }
+
+  @Override
+  public void checkClientTrusted(X509Certificate[] certificates, String authType) {

Review Comment:
   ## Server certificates should be verified during SSL/TLS connections
   
   <!--SONAR_ISSUE_KEY:AYevjzkf2F6dJxSmTb8v-->Enable server certificate validation on this SSL/TLS connection. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_calcite&issues=AYevjzkf2F6dJxSmTb8v&open=AYevjzkf2F6dJxSmTb8v&pullRequest=3174">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/calcite/security/code-scanning/5)



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1519165693

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] libenchao closed pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao closed pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter
URL: https://github.com/apache/calcite/pull/3174


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ElasticSearch Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1529751442

   > @cgivre This has been merged in [dd8fa24](https://github.com/apache/calcite/commit/dd8fa24a6a662e7e82780a940af4b3036c2adb23), you can see it in git history: https://github.com/apache/calcite/commits
   
   Thx!


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] cgivre commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "cgivre (via GitHub)" <gi...@apache.org>.
cgivre commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1174631103


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -173,6 +187,16 @@
                 httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider));
           }
 
+          if (disableSSLVerification) {
+            SSLContext sslContext = SSLContext.getInstance("TLS");
+            sslContext.init(null, new TrustManager[] { UnsafeX509ExtendedTrustManager.INSTANCE },
+                null);
+
+            builder.setHttpClientConfigCallback(httpClientBuilder ->
+                httpClientBuilder.setSSLContext(sslContext)
+                    .setSSLHostnameVerifier((host, session) -> true));

Review Comment:
   This is a deliberate option for development servers.  



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] libenchao commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1178635964


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not. This should <b>not</b> be
+ * used in production environments.
+ */
+@SuppressWarnings("java:S4830")
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {

Review Comment:
   I don't have a strong preference of this, currently it seems that only ES Adapter needs this, and I'm not sure if there will be more adapters that need this, so putting it in the util of core seems a little over-designed for me, we may could defer it to the next time that another adapter calls for it. (Please note that calcite is highly customizable, and calcite-core is widely used for many projects, it's not only used by calcite adapters)
   
   Of course if somebody strongly supports this, and I'm fine with it. Then the class comment for `UnsafeX509ExtendedTrustManager` should be adapted to not only mention about ES.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] libenchao commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1178635964


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not. This should <b>not</b> be
+ * used in production environments.
+ */
+@SuppressWarnings("java:S4830")
+public final class UnsafeX509ExtendedTrustManager extends X509ExtendedTrustManager {

Review Comment:
   I don't have a strong preference of this, currently it seems that only ES Adapter needs this, and I'm not sure if there will be more adapters that need this, so putting it in the util of core seems a little over-designed for me, we may could defer it to the next time that another adapter calls for it.
   
   Of course if somebody strongly supports this, and I'm fine with it. Then the class comment for `UnsafeX509ExtendedTrustManager` should be adapted to not only mention about ES.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3174:
URL: https://github.com/apache/calcite/pull/3174#issuecomment-1519150980

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3174)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [![D](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D-16px.png 'D')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY) [1 Vulnerability](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3174&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3174&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3174&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] zinking commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "zinking (via GitHub)" <gi...@apache.org>.
zinking commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1175172824


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -131,10 +135,20 @@ public ElasticsearchSchemaFactory() {
         ("Both 'coordinates' and 'hosts' is missing in configuration. Provide one of them.");
       }
       final String pathPrefix = (String) map.get("pathPrefix");
+
+      // Add or Disable SSL Verification
+      boolean disableSSLVerification;
+      if (map.containsKey("disableSSLVerification")) {
+        disableSSLVerification = (Boolean) map.get("disableSSLVerification");

Review Comment:
   cast string directly to boolean ?



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [calcite] libenchao commented on a diff in pull request #3174: CALCITE-5671: Add Option to Disable SSL Certificate Validation to ES Adapter

Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao commented on code in PR #3174:
URL: https://github.com/apache/calcite/pull/3174#discussion_r1175136488


##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ *
+ *

Review Comment:
   Nit: we usually only use 1 empty line.



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust

Review Comment:
   Nit:
   ```suggestion
    * This class is used to disable SSL Certificate Verification in ElasticSearch. This trust
   ```



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/UnsafeX509ExtendedTrustManager.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.elasticsearch;
+
+import java.net.Socket;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedTrustManager;
+
+/**
+ * This class is used to disable SSL Certificate Verification in ElasticSearch.  This trust
+ * manager will validate any SSL certificate, whether valid or not.
+ *
+ *
+ * <p>This should <b>not</b> be used in production environments.
+ * </p>

Review Comment:
   Nit: we usually not add `</p>`



##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchSchemaFactory.java:
##########
@@ -173,6 +189,16 @@ private static RestClient connect(List<HttpHost> hosts, String pathPrefix,
                 httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider));
           }
 
+          if (disableSSLVerification) {

Review Comment:
   I'm wondering that if it's possible to add some tests to verify this feature, and warrant it from future changes.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org