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.

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

Enable SSL in reindex with security QA tests #37600

Merged
merged 5 commits into from
Jan 31, 2019

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Jan 18, 2019

Updates the x-pack/qa/reindex-tests-with-security integration tests to
run with TLS enabled on the Rest interface.

Relates: #37527

Update the x-pack/qa/reindex-tests-with-security integration tests to
run with TLS enabled on the Rest interface.
@tvernum tvernum added >test Issues or PRs that are addressing/adding tests v7.0.0 :Security/TLS SSL/TLS, Certificates v6.7.0 labels Jan 18, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

@tvernum
Copy link
Contributor Author

tvernum commented Jan 18, 2019

This test will fail until #37527 is merged, as reindex doesn't support SSL yet.
But I wanted to keep that PR as small as possible and limit it to changes on the reindex/ module.

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

LGTM

// Check whether the cluster has started
URL url = new URL("https://${node.httpUri()}/_cluster/health?wait_for_nodes=${numNodes}&wait_for_status=yellow");
for (int i = 0; i < 20; i++) {
// we use custom wait logic here for HTTPS
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be awesome to unify this wait logic with that in smoke-test-plugins-ssl, potentially as a followup. Maybe @atorok has some guidance on the best way to do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think we probably want to replace the existing Ant task we use for this with our own task that can read from a custom truststore (or perhaps PEM so it can run on a FIPS JVM).

Copy link
Member

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

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

LGTM

logger.info("HTTP response was [{}]", httpURLConnection.getResponseCode());
}
} catch (IOException e) {
logger.info("Failed to call cluster health - " + e)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will be too verbose in CI , would only log the last one.

@tvernum tvernum merged commit cde126d into elastic:master Jan 31, 2019
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 31, 2019
* 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
  ...
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Feb 4, 2019
Update the x-pack/qa/reindex-tests-with-security integration tests to
run with TLS enabled on the Rest interface.

Backport of: elastic#37600
Relates: elastic#37527
tvernum added a commit that referenced this pull request Feb 4, 2019
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Security/TLS SSL/TLS, Certificates >test Issues or PRs that are addressing/adding tests v6.7.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants