You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Henry Robinson (Code Review)" <ge...@cloudera.org> on 2017/08/19 00:12:17 UTC

[Impala-ASF-CR] IMPALA-5775: (Addendum) Make SSL cluster actually come up in test client ssl.py

Henry Robinson has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7732

Change subject: IMPALA-5775: (Addendum) Make SSL cluster actually come up in test_client_ssl.py
......................................................................

IMPALA-5775: (Addendum) Make SSL cluster actually come up in test_client_ssl.py

The non-wildcard certs in test_client_ssl.py require that the hostname
of the process is 'localhost' for clients to validate them. This wasn't
the case for one test, and so the cluster wouldn't actually
start. Although the test would still pass (because the shell wasn't
actually checking the certificate), it's better hygiene to have the
cluster correctly configured to make sure we're testing what we think we
are.

Testing: test continues to pass

Change-Id: Idad8bbf3b8be853d3406bcbaed24909501500ea9
---
M tests/custom_cluster/test_client_ssl.py
1 file changed, 8 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/7732/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idad8bbf3b8be853d3406bcbaed24909501500ea9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>

[Impala-ASF-CR] IMPALA-5775: (Addendum) Make SSL cluster actually come up in test client ssl.py

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5775: (Addendum) Make SSL cluster actually come up in test_client_ssl.py
......................................................................


Patch Set 1: Code-Review+2

I agree with the fix, however, we don't have bi-directional hostname verification turned on (as that's the default in the Thrift SSL transport), so this was already a known issue. That's something we might want to expose as an option later to catch these kind of bad configurations.

-- 
To view, visit http://gerrit.cloudera.org:8080/7732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idad8bbf3b8be853d3406bcbaed24909501500ea9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5775: (Addendum) Make SSL cluster actually come up in test client ssl.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5775: (Addendum) Make SSL cluster actually come up in test_client_ssl.py
......................................................................


IMPALA-5775: (Addendum) Make SSL cluster actually come up in test_client_ssl.py

The non-wildcard certs in test_client_ssl.py require that the hostname
of the process is 'localhost' for clients to validate them. This wasn't
the case for one test, and so the cluster wouldn't actually
start. Although the test would still pass (because the shell wasn't
actually checking the certificate), it's better hygiene to have the
cluster correctly configured to make sure we're testing what we think we
are.

Testing: test continues to pass

Change-Id: Idad8bbf3b8be853d3406bcbaed24909501500ea9
Reviewed-on: http://gerrit.cloudera.org:8080/7732
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M tests/custom_cluster/test_client_ssl.py
1 file changed, 8 insertions(+), 3 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Henry Robinson: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/7732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Idad8bbf3b8be853d3406bcbaed24909501500ea9
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-5775: (Addendum) Make SSL cluster actually come up in test client ssl.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5775: (Addendum) Make SSL cluster actually come up in test_client_ssl.py
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1137/

-- 
To view, visit http://gerrit.cloudera.org:8080/7732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idad8bbf3b8be853d3406bcbaed24909501500ea9
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5775: (Addendum) Make SSL cluster actually come up in test client ssl.py

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-5775: (Addendum) Make SSL cluster actually come up in test_client_ssl.py
......................................................................


Patch Set 2: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/7732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idad8bbf3b8be853d3406bcbaed24909501500ea9
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5775: (Addendum) Make SSL cluster actually come up in test client ssl.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5775: (Addendum) Make SSL cluster actually come up in test_client_ssl.py
......................................................................


Patch Set 2: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/7732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idad8bbf3b8be853d3406bcbaed24909501500ea9
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No