Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SQL: Added SSL configuration options tests #37875

Merged
merged 7 commits into from
Jan 31, 2019
Merged

SQL: Added SSL configuration options tests #37875

merged 7 commits into from
Jan 31, 2019

Conversation

astefan
Copy link
Contributor

@astefan astefan commented Jan 25, 2019

This PR adds tests for the SSL configuration options and checks they can be specified in both a Properties object and in the URL of a JDBC connection. They were already supported in the URL, this PR only adds tests specifically for them.

Also, the allow.self.signed option from the documentation is removed to keep the settings consistent with the rest of projects around Elasticsearch (which don't have such an option) and because it's somewhat redundant to allow this option, as it's somehow obvious that if an user added the server's self-signed certificate to the keystore, that user is aware what type the respective certificate is.

Fixes #37711.

@astefan
Added SSL configuration options tests
db72a30 
Removed the allow.self.signed option from the documentation since we allow
by default self signed certificates as well.
@astefan astefan requested review from costin and matriv January 25, 2019 15:56
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

matriv
matriv approved these changes Jan 25, 2019
View reviewed changes
Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

costin
costin approved these changes Jan 25, 2019
View reviewed changes
Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a test where the driver itself is initialized either through the DataSource or the DriverManager.getDriver api.
Also one with typos to the correct SSL configuration is being shown?

@astefan
Copy link
Contributor Author

astefan commented Jan 28, 2019

@elasticmachine run elasticsearch-ci/1
@elasticmachine run elasticsearch-ci/2
@elasticmachine run elasticsearch-ci/default-distro

@astefan
Copy link
Contributor Author

astefan commented Jan 28, 2019

@elasticmachine run elasticsearch-ci/2

3 similar comments
@astefan
Copy link
Contributor Author

astefan commented Jan 28, 2019

@elasticmachine run elasticsearch-ci/2

@astefan
Copy link
Contributor Author

astefan commented Jan 28, 2019

@elasticmachine run elasticsearch-ci/2

@astefan
Copy link
Contributor Author

astefan commented Jan 28, 2019

@elasticmachine run elasticsearch-ci/2

@astefan
Copy link
Contributor Author

astefan commented Jan 28, 2019

@elasticmachine run elasticsearch-ci/default-distro

@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/1
@elasticmachine run elasticsearch-ci/2

@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/default-distro

@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/2

@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/1

@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/1
@elasticmachine run elasticsearch-ci/2

@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/1

1 similar comment
@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/1

@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/2

@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/1

@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/2

@astefan
Copy link
Contributor Author

astefan commented Jan 29, 2019

@elasticmachine run elasticsearch-ci/1

@astefan
Copy link
Contributor Author

astefan commented Jan 30, 2019

@elasticmachine run elasticsearch-ci/docs-check

@astefan
Copy link
Contributor Author

astefan commented Jan 30, 2019

@elasticmachine run elasticsearch-ci/docs-check
@elasticmachine run elasticsearch-ci/1

1 similar comment
@costin
Copy link
Member

costin commented Jan 30, 2019

@elasticmachine run elasticsearch-ci/docs-check
@elasticmachine run elasticsearch-ci/1

@astefan astefan merged commit 22d3290 into elastic:master Jan 31, 2019
astefan added a commit that referenced this pull request Jan 31, 2019
@astefan
SQL: Added SSL configuration options tests (#37875)
455b6c8 
* Removed the allow.self.signed option from the documentation since we allow
by default self signed certificates as well.

(cherry picked from commit 22d3290)
astefan added a commit that referenced this pull request Jan 31, 2019
@astefan
SQL: Added SSL configuration options tests (#37875)
59ff166 
* Removed the allow.self.signed option from the documentation since we allow
by default self signed certificates as well.

(cherry picked from commit 22d3290)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 31, 2019
@jasontedor
Merge branch 'master' into retention-leases-version
4aa6b77 
* master: (100 commits)
  Push primary term to replication tracker (elastic#38044)
  Introduce ability to minimize round-trips in CCS (elastic#37828)
  Don't Assert Ack on when Publish Timeout is 0 in Test (elastic#38077)
  Reduce object creation in Rounding class (elastic#38061)
  Treat put-mapping calls with `_doc` as a top-level key as typed calls. (elastic#38032)
  Fix test bug when testing the merging of mappings and templates. (elastic#38021)
  spelling: java script -- not JavaScript (elastic#37057)
  Enable SSL in reindex with security QA tests (elastic#37600)
  Disable BWC tests during backport (elastic#38074)
  SQL: Added SSL configuration options tests (elastic#37875)
  Minor fixes in the release notes script. (elastic#37967)
  Fix typo in docs. (elastic#38018)
  Update Lucene repo for 7.0.0-alpha2 (elastic#37985)
  Fix size of rolling-upgrade bootstrap config (elastic#38031)
  fix DateIndexNameProcessorTests offset pattern (elastic#38069)
  Speed up converting of temporal accessor to zoned date time (elastic#37915)
  Work around JDK8 timezone bug in tests (elastic#37968)
  Correct arg names when update mapping/settings from leader (elastic#38063)
  Introduce ssl settings to reindex from remote (elastic#37527)
  Mute testRetentionLeasesSyncOnExpiration
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@costin costin costin approved these changes

@matriv matriv matriv approved these changes

Assignees
No one assigned
Labels
:Analytics/SQL SQL querying >bug v6.6.1 v6.7.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@astefan @elasticmachine @costin @matriv @colings86