[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..

java: improve error messages when tokens are not used

We recently had some trouble debugging Java client connection failures
where we expected it to authenticate by token but it did not. Previously
it just complained that it couldn't authenticate by SASL and didn't give
any clue as to why it didn't use tokens.

This patch makes the error more clear by explaining why a token was not
used to authenticate.

Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Reviewed-on: http://gerrit.cloudera.org:8080/9070
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
3 files changed, 83 insertions(+), 11 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] java: improve error messages when tokens are not used

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 05 Mar 2018 06:37:13 +
Gerrit-HasComments: No

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9070/2/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9070/2/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@161
PS2, Line 161: does
> does not
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 05 Mar 2018 06:34:46 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Hao Hao,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: java: improve error messages when tokens are not used
..

java: improve error messages when tokens are not used

We recently had some trouble debugging Java client connection failures
where we expected it to authenticate by token but it did not. Previously
it just complained that it couldn't authenticate by SASL and didn't give
any clue as to why it didn't use tokens.

This patch makes the error more clear by explaining why a token was not
used to authenticate.

Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
3 files changed, 83 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/9070/3
--
To view, visit http://gerrit.cloudera.org:8080/9070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] java: improve error messages when tokens are not used

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 2: Code-Review+2

Other than the nit pointed out by Alexey, LGTM.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sun, 04 Mar 2018 20:41:09 +
Gerrit-HasComments: No

[kudu-CR] java: improve error messages when tokens are not used

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9070/2/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9070/2/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@161
PS2, Line 161: does
does not



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sun, 04 Mar 2018 20:16:13 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Hao Hao,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: java: improve error messages when tokens are not used
..

java: improve error messages when tokens are not used

We recently had some trouble debugging Java client connection failures
where we expected it to authenticate by token but it did not. Previously
it just complained that it couldn't authenticate by SASL and didn't give
any clue as to why it didn't use tokens.

This patch makes the error more clear by explaining why a token was not
used to authenticate.

Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
3 files changed, 83 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/9070/2
--
To view, visit http://gerrit.cloudera.org:8080/9070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@135
PS1, Line 135: was
> s/was/is to keep the tense consistent
Done


http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@439
PS1, Line 439:   message = "server requires SASL authentication, but " +
> I don't think adding SASL makes it more correct here, the server may also a
Done


http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@57
PS1, Line 57: Assert.assertThat(e.getMessage(), 
CoreMatchers.containsString(
> Agree, maybe a simple verification after initiation of Negotiator object is
added a test case for the "no ca certs" case. Couldn't figure out how to get 
the NOT_CHOSEN_BY_SERVER case -- I think that one is pretty much impossible to 
hit in real life anyway.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sun, 04 Mar 2018 19:21:24 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

yep, this is on my list today to revise


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 23 Feb 2018 19:22:49 +
Gerrit-HasComments: No

[kudu-CR] java: improve error messages when tokens are not used

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

> > Would be great to get this in for 1.7.
 >
 > Overall, the change looks good to me. Can it make to 1.7?

There are some nits left (those that Dan pointed at), but otherwise I think it 
looks good.

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
> in the case that the server doesn't accept token, we'll end up in the SASL
sgtm



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 23 Feb 2018 19:15:02 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

> Would be great to get this in for 1.7.

Overall, the change looks good to me. Can it make to 1.7?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 21 Feb 2018 22:22:35 +
Gerrit-HasComments: No

[kudu-CR] java: improve error messages when tokens are not used

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

Would be great to get this in for 1.7.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 16 Feb 2018 19:01:21 +
Gerrit-HasComments: No

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
> Yes, I think it makes sense to expose that information as well  in case of
in the case that the server doesn't accept token, we'll end up in the SASL path 
down below and hit the error messages that I've added in this patch. ie these 
error messages I've added are _only_ in the case that it's SASL, so adding it 
would always result in saying "SASL was chosen".



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:42:08 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@135
PS1, Line 135: was
s/was/is to keep the tense consistent


http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@439
PS1, Line 439:   message = "server requires SASL authentication, but " +
I don't think adding SASL makes it more correct here, the server may also 
accept token or cert authn.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:39:01 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@57
PS1, Line 57: Assert.assertThat(e.getMessage(), 
CoreMatchers.containsString(
> I wasn't sure how to engineer those situations without having to do a bunch
Agree, maybe a simple verification after initiation of Negotiator object is 
enough? Though that may not work for NOT_CHOSEN_BY_SERVER case.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:25:21 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
> Isn't there only the choice of sasl and token? where are you suggesting we
Yes, I think it makes sense to expose that information as well  in case of 
negotiation error.  It could be useful if there is some bug on the server side 
so the server doesn't accept token even if the client is ready to send it in.


http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@57
PS1, Line 57: Assert.assertThat(e.getMessage(), 
CoreMatchers.containsString(
> I wasn't sure how to engineer those situations without having to do a bunch
Well, by my understanding this particular place is just for the specific case.  
Not sure we want to verify other cases right here.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:24:15 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@57
PS1, Line 57: Assert.assertThat(e.getMessage(), 
CoreMatchers.containsString(
> Do we also want to add test cases to verify the log message for other cases
I wasn't sure how to engineer those situations without having to do a bunch of 
gymnastics with the server. Any ideas?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:16:22 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
> I think it makes sense to report on the authentication type sent back by th
+1


http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@57
PS1, Line 57: Assert.assertThat(e.getMessage(), 
CoreMatchers.containsString(
Do we also want to add test cases to verify the log message for other cases 
authn token is not used?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:12:47 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
> I think it makes sense to report on the authentication type sent back by th
Isn't there only the choice of sasl and token? where are you suggesting we add 
this?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:12:02 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9070/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@337
PS1, Line 337: chosenAuthnType = chooseAuthenticationType(response);
I think it makes sense to report on the authentication type sent back by the 
server if connection negotiation failed.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:03:50 +
Gerrit-HasComments: Yes

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Todd Lipcon has removed Anonymous Coward #314 from this change.  ( 
http://gerrit.cloudera.org:8080/9070 )

Change subject: java: improve error messages when tokens are not used
..


Removed reviewer null.
--
To view, visit http://gerrit.cloudera.org:8080/9070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] java: improve error messages when tokens are not used

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Anonymous Coward #314,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: java: improve error messages when tokens are not used
..

java: improve error messages when tokens are not used

We recently had some trouble debugging Java client connection failures
where we expected it to authenticate by token but it did not. Previously
it just complained that it couldn't authenticate by SASL and didn't give
any clue as to why it didn't use tokens.

This patch makes the error more clear by explaining why a token was not
used to authenticate.

Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
2 files changed, 42 insertions(+), 9 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/9070/1
--
To view, visit http://gerrit.cloudera.org:8080/9070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7f5160a79e973c411c16a784dc105444f5dd2a6f
Gerrit-Change-Number: 9070
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314