pkg/transport: reload TLS certificates for every client requests#7829
Merged
Conversation
heyitsanthony
suggested changes
Apr 27, 2017
|
|
||
| // 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?
| } | ||
|
|
||
| // 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
| if err = w.Sync(); err != nil { | ||
| return err | ||
| } | ||
| if _, err = w.Seek(0, io.SeekStart); err != nil { |
| 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).
|
|
||
| // 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.
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 <gyuhox@gmail.com> Original commit can be found at etcd-io#7784
Contributor
Author
|
@heyitsanthony PTAL. Thanks. |
heyitsanthony
suggested changes
Apr 27, 2017
| } | ||
| }() | ||
|
|
||
| // overwrite valid certs with expired ones |
There was a problem hiding this comment.
copyTLSFiles to replace the copyFiles?
| 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?
| defer clus.Terminate(t) | ||
|
|
||
| // concurrent client dialing while certs transition from valid to expired | ||
| errc := make(chan error) |
| } | ||
| cli, cerr := clientv3.New(clientv3.Config{ | ||
| Endpoints: []string{clus.Members[0].GRPCAddr()}, | ||
| DialTimeout: 3 * time.Second, |
| } | ||
| cli, cerr := clientv3.New(clientv3.Config{ | ||
| Endpoints: []string{clus.Members[0].GRPCAddr()}, | ||
| DialTimeout: 3 * time.Second, |
| 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?
059044f to
4aa95cd
Compare
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
heyitsanthony
approved these changes
Apr 27, 2017
|
lgtm |
heyitsanthony
approved these changes
Apr 27, 2017
14 tasks
clwluvw
added a commit
to clwluvw/grafana-plugin-sdk-go
that referenced
this pull request
Nov 14, 2025
Setting golang's tls config.GetClientCertificate will let us pass a func that could read the certificate and key file on every req. This is useful for when using mTLS and the certificate is rotated so we can let the rotation be done without needing of restarting the app. e.g. etcd implementation: etcd-io/etcd#7829 Signed-off-by: Seena Fallah <seenafallah@gmail.com>
clwluvw
added a commit
to clwluvw/grafana-plugin-sdk-go
that referenced
this pull request
Nov 14, 2025
Setting golang's tls config.GetClientCertificate will let us pass a func that could read the certificate and key file on every req. This is useful for when using mTLS and the certificate is rotated so we can let the rotation be done without needing of restarting the app. e.g. etcd implementation: etcd-io/etcd#7829 Signed-off-by: Seena Fallah <seenafallah@gmail.com>
clwluvw
added a commit
to clwluvw/grafana-plugin-sdk-go
that referenced
this pull request
Nov 14, 2025
Expose tls.Config.GetClientCertificate to allow supplying a callback that loads the client certificate and key on each request. This enables mTLS setups where certificates are rotated automatically without restarting the application. Inspired by the etcd approach: etcd-io/etcd#7829 Signed-off-by: Seena Fallah <seenafallah@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.