Add TLS support to socket_writer and socket_listener plugins #4021
Conversation
internal/internal.go
Outdated
| // for use with a client. | ||
| // The full path to each file must be provided. | ||
| // Returns a nil pointer if all files are blank and InsecureSkipVerify=false. | ||
| func GetClientTLSConfig( |
There was a problem hiding this comment.
Just leave this as GetTLSConfig for now.
| ## Enable and require client certificate authentication. | ||
| # ssl_client_auth = false | ||
| ## CAs used to verify client certificates. | ||
| # ssl_ca = ["/etc/telegraf/ca.pem"] |
There was a problem hiding this comment.
We have a little bit of prior art on the server side: #3191, I would prefer if we use the same variable names, but let me know if you think that is a problem. The biggest difference is that client_auth is enabled automatically if there are cacerts specified.
There was a problem hiding this comment.
Makes sense -- definitely want to stay consistent. The config now matches those variable names.
bobmshannon
force-pushed
the
bs/socket_security
branch
5 times, most recently
from
e264ea6
to
474e9f3
Compare
|
I've addressed the comments, however, I still need to investigate why the tests are failing. |
bobmshannon
force-pushed
the
bs/socket_security
branch
from
474e9f3
to
7dbc96b
Compare
bobmshannon
force-pushed
the
bs/socket_security
branch
from
a1701ba
to
8e1c686
Compare
|
OK, tests should be sorted now. |
bobmshannon
force-pushed
the
bs/socket_security
branch
from
406a096
to
ff3d682
Compare
danielnelson
added
the
feat
|
Thanks! |
| @@ -313,6 +338,14 @@ func (uc unixCloser) Close() error { | |||
| return err | |||
| } | |||
|
|
|||
| func (uc unixCloser) Accept() (net.Conn, error) { | |||
| return uc.closer.(net.Listener).Accept() | |||
There was a problem hiding this comment.
if we're assuming that uc.closer is always going to be a net.Listener, you can avoid the type assertion by redefining uc.closer to be a net.Listener.
This is good practice anyway as then you get compile time verification that you never assign anything other than a net.Listener to uc.closer, as if you do otherwise this code will panic.
There was a problem hiding this comment.
I don't see why we need these functions at all...
There was a problem hiding this comment.
if you embed net.Listener directly within unixCloser so that unixCloser picks up net.Listener's methods, then yeah, you shouldn't.
There was a problem hiding this comment.
Oh, you mean they aren't needed because they're not even called. yeah... :-)
| var ( | ||
| c net.Conn | ||
| err error | ||
| ) |
There was a problem hiding this comment.
nit: it's idiomatic go to declare variables right before they're used, not at the top of the function.
Also err doesn't need to be declared at all. It'll get declared on line 83.
|
Thanks, I'll get these. |
Fixes #2866.
Required for all PRs: