Implement TLS support #44
Conversation
| coordinator = new RequestCoordinator(srvServiceUrl); | ||
| } else if (addresses != null) { | ||
| coordinator = createCoordinator(srvServiceUrl); | ||
| } else { // addresses != null |
There was a problem hiding this comment.
sonalint recommends removing this unnecessary conditional statement
| String caRootCert = null; | ||
|
|
||
| if (caRootCertPem != null) { | ||
| caRootCert = caRootCertPem.replace("\\n", System.lineSeparator()); |
There was a problem hiding this comment.
in case the user pass PEM formatted file in one line
There was a problem hiding this comment.
@kota2and3kan
FYI. The error we met yesterday was because picocli takes \n as a plaintext but not a newline character. So we need to convert it ourselves.
There was a problem hiding this comment.
@supl
Ah, I see. I understood.
Thank you for your investigation!
There was a problem hiding this comment.
@supl
Can you make sure if it is really picocli thing?
I'm not sure, but it might be shell thing.
I haven't run this command, but did a quick check with zsh and bash as follows. (Do you use bash?)
# zsh
$ echo "aaa\nbbb"
aaa
bbb
# bash
bash-3.2$ echo "aaa\nbbb"
aaa\nbbb
There was a problem hiding this comment.
Thanks for your concern.
Yes, I used bash to run the command.
I did a survey for this.
It seems neither bash nor zsh expand newline's escape sequence (\n) to newline.
The different results you got on zsh and bash should be because of echo.
It seems echo (as a built-in command) in zsh uses -e option by default while bash's echo doesn't.
If we use this command
printf "aaa\nbbb"
we should get the same result
aaa
bbb
It's not even because of using single quote or double quote.
On zsh,
% echo 'aaa\nbbb'
aaa
bbb
Echo in zsh by default expands newline escape sequence.
It equals we run
> echo -e "aaa\nbbb"
aaa
bbb
on bash.
To confirm this again, I run picocli on zsh
% ./scalar-admin --ca-root-cert-pem "aaa\nccc" -c pause
aaa\nccc
(I revised this PR's call to just print out the argument)
So, I think this is not shell's issue.
Picocli just doesn't expand newline escape sequence either.
There was a problem hiding this comment.
@feeblefakie Just to be precise, the example you provided is caused not by the difference of the shells but by the difference in the echo commands. Both in bash and zsh, echo is a built-in command (you can see type echo). And, only the echo of zsh interpreters backslashed escape characters, like "\n". We need the -e flag for bash's echo. See man of bash and zsh.
So, I suppose when passing string parameters to this command, the difference between bash and zsh does not affect.
There was a problem hiding this comment.
You can see that the strings passed to the programs are the same:
# zsh
❯ ruby -e "puts ARGV" "foo\nbar"
foo\nbar
# bash
❯ bash
bash-5.2$ ruby -e "puts ARGV" "foo\nbar"
foo\nbar
There was a problem hiding this comment.
@supl Sorry that I found that you already replied after reloading
There was a problem hiding this comment.
@choplin
No problem at all.
Thank you very much for clarifying this question 🙇
| @@ -22,6 +27,30 @@ public AdminClient(String host, int port) { | |||
| this.blockingStub = AdminGrpc.newBlockingStub(channel); | |||
| } | |||
|
|
|||
| public AdminClient( | |||
There was a problem hiding this comment.
Co-authored-by: Toshihiro Suzuki <brfrn169@gmail.com>
There was a problem hiding this comment.
LGTM! Thank you!
FYI: I was able to run the scalar-admin command with TLS in my local environment as follows.
$ build/install/scalar-admin/bin/scalar-admin --tls -c pause --ca-root-cert-path ~/cert/ca.pem --override-authority envoy.scalar.example.com -a 172.20.255.200:60053
Pause completed at 2024-03-07T06:36:42.004Z UTC (1709793402004)
| String caRootCert = null; | ||
|
|
||
| if (caRootCertPem != null) { | ||
| caRootCert = caRootCertPem.replace("\\n", System.lineSeparator()); |
Description
This PR implements the TLS support so that user can pass CA root certificate and override authority to build TLS gRPC connection to targets.
The command-line tool has three more options
--tls--ca-root-cert-pathand--ca-root-cert-pem--override-authorityRelated issues and/or PRs
N/A
Changes made
TlsRequestCoordinatorChecklist
Additional notes