Skip to content

Commit 1e52202

Browse files
committed
Handle TLSOpts.GetCertificate in webhook
This change rewrites some of the webhook server logic to better handle the user setting a custom configuration for the TLS options by providing a custom GetCertificate function. When that's present, we won't start a certwatcher routine. The change also updates the documentation to better clarify when each of the options are effective. Signed-off-by: Vince Prignano <[email protected]>
1 parent 0ef0753 commit 1e52202

File tree

1 file changed

+37
-22
lines changed

1 file changed

+37
-22
lines changed

pkg/webhook/server.go

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,13 @@ type Server struct {
6060
CertDir string
6161

6262
// CertName is the server certificate name. Defaults to tls.crt.
63+
//
64+
// Note: This option should only be set when TLSOpts does not override GetCertificate.
6365
CertName string
6466

6567
// KeyName is the server key name. Defaults to tls.key.
68+
//
69+
// Note: This option should only be set when TLSOpts does not override GetCertificate.
6670
KeyName string
6771

6872
// ClientCAName is the CA certificate name which server used to verify remote(client)'s certificate.
@@ -169,32 +173,48 @@ func (s *Server) Start(ctx context.Context) error {
169173
baseHookLog := log.WithName("webhooks")
170174
baseHookLog.Info("Starting webhook server")
171175

172-
certPath := filepath.Join(s.CertDir, s.CertName)
173-
keyPath := filepath.Join(s.CertDir, s.KeyName)
174-
175-
certWatcher, err := certwatcher.New(certPath, keyPath)
176+
tlsMinVersion, err := tlsVersion(s.TLSMinVersion)
176177
if err != nil {
177178
return err
178179
}
179180

180-
go func() {
181-
if err := certWatcher.Start(ctx); err != nil {
182-
log.Error(err, "certificate watcher error")
181+
cfg := &tls.Config{ //nolint:gosec
182+
NextProtos: []string{"h2"},
183+
MinVersion: tlsMinVersion,
184+
}
185+
// fallback TLS config ready, will now mutate if passer wants full control over it
186+
for _, op := range s.TLSOpts {
187+
op(cfg)
188+
}
189+
190+
switch {
191+
case cfg.GetCertificate != nil:
192+
if s.CertName != "" {
193+
return fmt.Errorf("cannot use GetCertificate and CertName at the same time")
183194
}
184-
}()
195+
if s.KeyName != "" {
196+
return fmt.Errorf("cannot use GetCertificate and KeyName at the same time")
197+
}
198+
default:
199+
certPath := filepath.Join(s.CertDir, s.CertName)
200+
keyPath := filepath.Join(s.CertDir, s.KeyName)
185201

186-
tlsMinVersion, err := tlsVersion(s.TLSMinVersion)
187-
if err != nil {
188-
return err
189-
}
202+
// Create the certificate watcher and
203+
// set the config's GetCertificate on the TLSConfig
204+
certWatcher, err := certwatcher.New(certPath, keyPath)
205+
if err != nil {
206+
return err
207+
}
208+
cfg.GetCertificate = certWatcher.GetCertificate
190209

191-
cfg := &tls.Config{ //nolint:gosec
192-
NextProtos: []string{"h2"},
193-
GetCertificate: certWatcher.GetCertificate,
194-
MinVersion: tlsMinVersion,
210+
go func() {
211+
if err := certWatcher.Start(ctx); err != nil {
212+
log.Error(err, "certificate watcher error")
213+
}
214+
}()
195215
}
196216

197-
// load CA to verify client certificate
217+
// Load CA to verify client certificate, if configured.
198218
if s.ClientCAName != "" {
199219
certPool := x509.NewCertPool()
200220
clientCABytes, err := os.ReadFile(filepath.Join(s.CertDir, s.ClientCAName))
@@ -211,11 +231,6 @@ func (s *Server) Start(ctx context.Context) error {
211231
cfg.ClientAuth = tls.RequireAndVerifyClientCert
212232
}
213233

214-
// fallback TLS config ready, will now mutate if passer wants full control over it
215-
for _, op := range s.TLSOpts {
216-
op(cfg)
217-
}
218-
219234
listener, err := tls.Listen("tcp", net.JoinHostPort(s.Host, strconv.Itoa(s.Port)), cfg)
220235
if err != nil {
221236
return err

0 commit comments

Comments
 (0)