hanahmily commented on code in PR #882:
URL:
https://github.com/apache/skywalking-banyandb/pull/882#discussion_r2589597175
##########
banyand/queue/pub/pub.go:
##########
@@ -441,13 +486,210 @@ func isFailoverError(err error) bool {
}
func (p *pub) getClientTransportCredentials() ([]grpc.DialOption, error) {
+ if !p.tlsEnabled {
+ return grpchelper.SecureOptions(nil, false, false, "")
+ }
+
+ // Use reloader if available (for dynamic reloading)
+ if p.caCertReloader != nil {
+ // Extract server name from the connection (we'll use a default
for now)
+ // The actual server name will be validated by the TLS handshake
+ tlsConfig, err := p.caCertReloader.GetClientTLSConfig("")
+ if err != nil {
+ return nil, fmt.Errorf("failed to get TLS config from
reloader: %w", err)
+ }
+ creds := credentials.NewTLS(tlsConfig)
+ return []grpc.DialOption{grpc.WithTransportCredentials(creds)},
nil
+ }
+
+ // Fallback to static file reading if reloader is not available
opts, err := grpchelper.SecureOptions(nil, p.tlsEnabled, false,
p.caCertPath)
if err != nil {
return nil, fmt.Errorf("failed to load TLS config: %w", err)
}
return opts, nil
}
+// reconnectAllClients reconnects all active clients when CA certificate is
updated.
+func (p *pub) reconnectAllClients() {
Review Comment:
Use the lock to get the nodesToReconnct from p.register, not the p.active.
Implement the reconnection with `pub.OnDelete` and `pub.OnAddOrUpdate`, that
are more stable than the new logic you implemented.
##########
banyand/queue/pub/pub.go:
##########
Review Comment:
```suggestion
return fmt.Errorf("TLS is enabled (--data-client-tls), but no
CA certificate file was provided (--data-client-ca-cert is required)")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
