Introduce ssl settings to reindex from remote by tvernum · Pull Request #37527 · elastic/elasticsearch

tvernum · 2019-01-16T11:38:23Z

Adds reindex.ssl.* settings for reindex from remote.

This uses the ssl-config/ internal library to parse and load SSL
configuration and files. This SSL configuration is applied to the
low level rest client when connecting to a remote ES node.

Relates: #37287
Resolves: #29755

This uses the ssl-config/ internal library to parse and load SSL configuration and files. This is applied when using the low level rest client to connect to a remove ES node

elasticmachine · 2019-01-16T11:38:25Z

Pinging @elastic/es-security

elasticmachine · 2019-01-16T11:38:26Z

Pinging @elastic/es-distributed

tvernum · 2019-01-16T11:43:05Z

@elastic/es-distributed I'm not sure who is the reindex from remote expert these days. Can someone volunteer to review?

tvernum · 2019-01-16T11:43:23Z

I have 2 follow up PRs after this one

To switch the reindex-with-security QA tests to use SSL
Another to update docs.

tvernum · 2019-01-16T11:55:13Z

...s/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollAction.java

 */
-public abstract class AbstractAsyncBulkByScrollAction<Request extends AbstractBulkByScrollRequest<Request>> {
+public abstract class AbstractAsyncBulkByScrollAction<Request extends AbstractBulkByScrollRequest<Request>,
+    Action extends TransportAction<Request, ?>> {


The introduction of the Action as a generic type parameter and field (mainAction below) is an attempt to solve the problems that come from the way AbstractAsyncBulkByScrollAction calls buildScrollableResultSource from its constructor.
Because the call is made from the superclass's constructor, it means that TransportReindexAction.AsyncIndexBySearchAction.buildScrollableResultSource (and by extension buildRestClient) can't make use of any fields in the subclass.
But buildRestClient needs access to the ReindexSslConfig instance, so we either need to push that up into the base class, or come up with a different approach.
By pushing a generic mainAction up into the base class, we can access fields from the Action without needing to duplicate them in AbstractAsyncBulkByScrollAction. This allows us to also remove scriptService from this class.

ywelsch

Thanks @tvernum. I've left two initial comments. I'm not super familiar with the SSL work though.

ywelsch · 2019-01-17T14:02:17Z

modules/reindex/build.gradle


 dependencies {
  compile "org.elasticsearch.client:elasticsearch-rest-client:${version}"
+  compile project(':libs:elasticsearch-ssl-config')


register the ssl-config project in the top-level build.gradle under projectSubstitutions, and then you can use compile "org.elasticsearch:elasticsearch-ssl-config:${version}"
Also check if the ssl-config is properly published through the release manager

Thanks @ywelsch, will do.

The build.gradle change has been pushed, and there is an open PR for release manager.

Actually @ywelsch Is there a reason you think this should be in release-manager? I don't think we publish any reindex jars that would need this.
As an example we don't publish the grok libs that are used in ingest modules.

you're right. No need for that.

ywelsch · 2019-01-17T14:24:58Z

modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexRestClientSslTests.java

+import static org.mockito.Mockito.mock;
+
+/**
+ * Because core ES doesn't have SSL available, this test uses a mock webserver


In addition to this test here, can you also add a reindex test to x-pack? This will provide us a full end-to-end test.

There's an existing reindex-with-security test in x-pack/qa.
I have a change prepared to switch it to use SSL. I left it out of this change so that this PR could be limited to the parts that needed reviews from the distributed team.
I'll open that PR now (the test will fail until this is PR is merged, but it will be available for review)

I opened #37600 with that test change.

tvernum · 2019-01-24T13:25:47Z

Ping @jkakavas

ywelsch

LGTM

jkakavas

LGTM Tim, sorry for the delay

tvernum · 2019-01-29T02:21:49Z

@elasticmachine test this please

tvernum · 2019-01-29T04:10:55Z

02:45:53   1> [2019-01-29T08:45:53,368][INFO ][o.e.u.FullClusterRestartIT] [testRecovery] after test
02:45:53 FAILURE 0.09s | FullClusterRestartIT.testRecovery <<< FAILURES!
02:45:53    > Throwable #1: java.lang.AssertionError: expected version to be one of [8.0.0,7.6.0] but was p 0 testrecovery 7.7.0
02:45:53    > 	at __randomizedtesting.SeedInfo.seed([AAC6A07A621C65D0:6B36D9D64F4CAF77]:0)
02:45:53    > 	at org.elasticsearch.upgrades.FullClusterRestartIT.testRecovery(FullClusterRestartIT.java:842)
02:45:53    > 	at java.lang.Thread.run(Thread.java:748)
02:45:53 Completed [3/3] in 5.04s, 13 tests, 1 failure, 1 skipped <<< FAILURES!

@elasticmachine run elasticsearch-ci/default-distro

tvernum · 2019-01-29T05:04:08Z

@elasticmachine run elasticsearch-ci/1

tvernum · 2019-01-29T05:53:37Z

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

Update the x-pack/qa/reindex-tests-with-security integration tests to run with TLS enabled on the Rest interface. Backport of: #37600 Relates: #37527

Reindex from remote now supports configurable SSL/TLS (node level) settings. This change adds documentation relating to those settings Relates: elastic#37527

Reindex from remote now supports configurable SSL/TLS (node level) settings. This change adds documentation relating to those settings Relates: #37527

Reindex from remote now supports configurable SSL/TLS (node level) settings. This change adds documentation relating to those settings Relates: elastic#37527 Backport of: elastic#38486

Reindex from remote now supports configurable SSL/TLS (node level) settings. This change adds documentation relating to those settings Relates: #37527 Backport of: #38486

Reindex from remote now supports configurable SSL/TLS (node level) settings. This change adds documentation relating to those settings Relates: elastic#37527 Backport of: elastic#38486

Reindex from remote now supports configurable SSL/TLS (node level) settings. This change adds documentation relating to those settings Relates: #37527 Backport of: #38486

This is used by the reindex-client library which is published to maven Relates: elastic#37287, elastic#37527 Closes: elastic#38944

This is used by the reindex-client library which is published to maven Relates: #37287, #37527 Closes: #38944

This is used by the reindex-client library which is published to maven Relates: elastic#37287, elastic#37527 Backport of: elastic#39019

This is used by the reindex-client library which is published to maven Relates: #37287, #37527 Backport of: #39019

This is used by the reindex-client library which is published to maven Relates: elastic#37287, elastic#37527 Closes: elastic#38944 Backport of: elastic#39019

This is used by the reindex-client library which is published to maven Relates: #37287, #37527 Backport of: #39019

Introduce ssl settings to reindex from remote

4a5781c

This uses the ssl-config/ internal library to parse and load SSL configuration and files. This is applied when using the low level rest client to connect to a remove ES node

tvernum added >enhancement :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v7.0.0 :Security/TLS SSL/TLS, Certificates v6.7.0 labels Jan 16, 2019

tvernum requested a review from jkakavas January 16, 2019 11:38

tvernum commented Jan 16, 2019

View reviewed changes

tvernum added 2 commits January 16, 2019 23:04

Make field final

bffcdde

Revert whitespace changes

7ab750b

ywelsch self-requested a review January 16, 2019 12:44

ywelsch suggested changes Jan 17, 2019

View reviewed changes

tvernum mentioned this pull request Jan 18, 2019

Enable SSL in reindex with security QA tests #37600

Merged

ywelsch self-requested a review January 21, 2019 22:36

Switch to use project substitutioni

9418c68

tvernum added 2 commits January 25, 2019 00:27

Merge branch 'master' into reindex-ssl-lib/2

00ce76e

Merge branch 'master' into reindex-ssl-lib/2

370ec35

ywelsch approved these changes Jan 25, 2019

View reviewed changes

jkakavas approved these changes Jan 28, 2019

View reviewed changes

Merge branch 'master' into reindex-ssl-lib/2

d50ea50

Merge branch 'master' into reindex-ssl-lib/2

475394c

tvernum mentioned this pull request Feb 4, 2019

[BACKPORT] Enable SSL in reindex with security QA tests #38293

Merged

tvernum added a commit that referenced this pull request Feb 4, 2019

Enable SSL in reindex with security QA tests (#38293)

3e2da3c

Update the x-pack/qa/reindex-tests-with-security integration tests to run with TLS enabled on the Rest interface. Backport of: #37600 Relates: #37527

tvernum mentioned this pull request Feb 6, 2019

Add docs for new reindex.ssl settings #38486

Merged

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

tvernum added a commit that referenced this pull request Feb 11, 2019

Add docs for new reindex.ssl settings (#38486)

4f68a4a

Reindex from remote now supports configurable SSL/TLS (node level) settings. This change adds documentation relating to those settings Relates: #37527

tvernum mentioned this pull request Feb 11, 2019

[BACKPORT 7.x] Add docs for new reindex.ssl settings #38686

Merged

tvernum mentioned this pull request Feb 11, 2019

[BACKPORT 7.0] Add docs for new reindex.ssl settings #38688

Merged

tvernum mentioned this pull request Feb 11, 2019

[BACKPORT 6.7] Add docs for new reindex.ssl settings #38689

Merged

jaymode mentioned this pull request Feb 15, 2019

Remote _reindex fail for HTTPS remotes #27267

Closed

tvernum added a commit to tvernum/elasticsearch that referenced this pull request Feb 18, 2019

Generate mvn pom for ssl-config library

478d6b7

This is used by the reindex-client library which is published to maven Relates: elastic#37287, elastic#37527 Closes: elastic#38944

tvernum mentioned this pull request Feb 18, 2019

Generate mvn pom for ssl-config library #39019

Merged

tvernum added a commit that referenced this pull request Feb 18, 2019

Generate mvn pom for ssl-config library (#39019)

cbdda71

This is used by the reindex-client library which is published to maven Relates: #37287, #37527 Closes: #38944

tvernum added a commit to tvernum/elasticsearch that referenced this pull request Feb 18, 2019

Generate mvn pom for ssl-config library

86c7e40

This is used by the reindex-client library which is published to maven Relates: elastic#37287, elastic#37527 Backport of: elastic#39019

tvernum mentioned this pull request Feb 18, 2019

[BACKPORT 7.x] Generate mvn pom for ssl-config library #39026

Merged

tvernum added a commit to tvernum/elasticsearch that referenced this pull request Feb 18, 2019

Generate mvn pom for ssl-config library

8bb9a63

This is used by the reindex-client library which is published to maven Relates: elastic#37287, elastic#37527 Backport of: elastic#39019

tvernum mentioned this pull request Feb 18, 2019

[BACKPORT 7.0] Generate mvn pom for ssl-config library #39027

Merged

tvernum added a commit that referenced this pull request Feb 18, 2019

Generate mvn pom for ssl-config library (#39026)

8895bef

This is used by the reindex-client library which is published to maven Relates: #37287, #37527 Backport of: #39019

tvernum added a commit that referenced this pull request Feb 18, 2019

Generate mvn pom for ssl-config library (#39027)

9a50845

This is used by the reindex-client library which is published to maven Relates: #37287, #37527 Backport of: #39019

tvernum mentioned this pull request Feb 18, 2019

Generate mvn pom for ssl-config library #39033

Merged

tvernum added a commit that referenced this pull request Feb 18, 2019

Generate mvn pom for ssl-config library (#39033)

63baa61

This is used by the reindex-client library which is published to maven Relates: #37287, #37527 Backport of: #39019

Conversation

tvernum commented Jan 16, 2019

Uh oh!

elasticmachine commented Jan 16, 2019

Uh oh!

elasticmachine commented Jan 16, 2019

Uh oh!

tvernum commented Jan 16, 2019

Uh oh!

tvernum commented Jan 16, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ywelsch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvernum commented Jan 24, 2019

Uh oh!

ywelsch left a comment

Choose a reason for hiding this comment

Uh oh!

jkakavas left a comment

Choose a reason for hiding this comment

Uh oh!

tvernum commented Jan 29, 2019

Uh oh!

tvernum commented Jan 29, 2019

Uh oh!

tvernum commented Jan 29, 2019

Uh oh!

tvernum commented Jan 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants