pkg/transport: reload TLS certificates for every client requests #7829
Conversation
clientv3/integration/dial_test.go
Outdated
| @@ -66,6 +70,89 @@ func TestDialTLSExpired(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| // TestDialTLSExpiredReload ensures server reloads expired certs, | |||
| // rejecting client requests, and vice versa. | |||
| func TestDialTLSExpiredReload(t *testing.T) { | |||
There was a problem hiding this comment.
integration/ instead of clientv3/integration since it's testing server behavior that's client independent?
clientv3/integration/dial_test.go
Outdated
| @@ -173,3 +260,52 @@ func TestDialForeignEndpoint(t *testing.T) { | |||
| t.Fatal(err) | |||
| } | |||
| } | |||
|
|
|||
| // copyTLSFiles clones certs files to temp directory. | |||
| func copyTLSFiles(ti transport.TLSInfo) (transport.TLSInfo, error) { | |||
There was a problem hiding this comment.
func copyTLSFiles(ti transport.TLSInfo, dst string) (transport.TLSInfo, error) instead of bothering with tempdirs here; can be passed in as an argument
clientv3/integration/dial_test.go
Outdated
| if err = w.Sync(); err != nil { | ||
| return err | ||
| } | ||
| if _, err = w.Seek(0, io.SeekStart); err != nil { |
clientv3/integration/dial_test.go
Outdated
| defer clus.Terminate(t) | ||
|
|
||
| // replace certs directory with expired ones | ||
| if err = os.Rename(certsDir, tmpDir); err != nil { |
There was a problem hiding this comment.
Two separate tests: atomic directory rename and copy-over. The copy-over case would cause connection failures during cert update (or might take down the server in some way).
clientv3/integration/dial_test.go
Outdated
|
|
||
| // now server expects 'tls: bad certificate' | ||
| // on incoming client requests | ||
| tls, err := ts.ClientConfig() |
There was a problem hiding this comment.
Some concurrency would be good here. Boot a cluster, start a goroutine that spams it with connections, manipulate the certs while connections concurrently hit the server. The connection goroutine would complete when it transitions from good connections to tls rejects.
gyuho
force-pushed
the
certs
branch
2 times, most recently
from
c2f7574 to
8020d42
Compare
This changes the baseConfig used when creating tls Configs to utilize the GetCertificate and GetClientCertificate functions to always reload the certificates from disk whenever they are needed. Always reloading the certificates allows changing the certificates via an external process without interrupting etcd. Fixes etcd-io#7576 Cherry-picked by Gyu-Ho Lee <[email protected]> Original commit can be found at etcd-io#7784
|
@heyitsanthony PTAL. Thanks. |
| } | ||
| }() | ||
|
|
||
| // overwrite valid certs with expired ones |
There was a problem hiding this comment.
copyTLSFiles to replace the copyFiles?
integration/v3_grpc_test.go
Outdated
| t.Fatal("expected dial timeout in 3 seconds, but never got it") | ||
| } | ||
|
|
||
| // now, replace expired certs back with valid ones |
There was a problem hiding this comment.
copyTLSFiles to replace the copyFiles?
integration/v3_grpc_test.go
Outdated
| defer clus.Terminate(t) | ||
|
|
||
| // concurrent client dialing while certs transition from valid to expired | ||
| errc := make(chan error) |
integration/v3_grpc_test.go
Outdated
| } | ||
| cli, cerr := clientv3.New(clientv3.Config{ | ||
| Endpoints: []string{clus.Members[0].GRPCAddr()}, | ||
| DialTimeout: 3 * time.Second, |
integration/v3_grpc_test.go
Outdated
| } | ||
| cli, cerr := clientv3.New(clientv3.Config{ | ||
| Endpoints: []string{clus.Members[0].GRPCAddr()}, | ||
| DialTimeout: 3 * time.Second, |
integration/cluster.go
Outdated
| @@ -929,3 +938,42 @@ type grpcAPI struct { | |||
| // Auth is the authentication API for the client's connection. | |||
| Auth pb.AuthClient | |||
| } | |||
|
|
|||
| // copyTLSFiles clones certs files to dst directory. | |||
There was a problem hiding this comment.
util_test.go or some other test file for these copy functions since they're not used by the integration package outside of tests?
gyuho
force-pushed
the
certs
branch
3 times, most recently
from
059044f to
4aa95cd
Compare
Signed-off-by: Gyu-Ho Lee <[email protected]>
|
lgtm |
No description provided.