Skip to content

Commit

Permalink
Handle TLSOpts.GetCertificate in webhook
Browse files Browse the repository at this point in the history
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 <vincepri@redhat.com>
  • Loading branch information
vincepri committed May 2, 2023
1 parent 0ef0753 commit 1e52202
Showing 1 changed file with 37 additions and 22 deletions.
59 changes: 37 additions & 22 deletions pkg/webhook/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,13 @@ type Server struct {
CertDir string

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

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

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

certPath := filepath.Join(s.CertDir, s.CertName)
keyPath := filepath.Join(s.CertDir, s.KeyName)

certWatcher, err := certwatcher.New(certPath, keyPath)
tlsMinVersion, err := tlsVersion(s.TLSMinVersion)
if err != nil {
return err
}

go func() {
if err := certWatcher.Start(ctx); err != nil {
log.Error(err, "certificate watcher error")
cfg := &tls.Config{ //nolint:gosec
NextProtos: []string{"h2"},
MinVersion: tlsMinVersion,
}
// fallback TLS config ready, will now mutate if passer wants full control over it
for _, op := range s.TLSOpts {
op(cfg)
}

switch {
case cfg.GetCertificate != nil:
if s.CertName != "" {
return fmt.Errorf("cannot use GetCertificate and CertName at the same time")
}
}()
if s.KeyName != "" {
return fmt.Errorf("cannot use GetCertificate and KeyName at the same time")
}
default:
certPath := filepath.Join(s.CertDir, s.CertName)
keyPath := filepath.Join(s.CertDir, s.KeyName)

tlsMinVersion, err := tlsVersion(s.TLSMinVersion)
if err != nil {
return err
}
// Create the certificate watcher and
// set the config's GetCertificate on the TLSConfig
certWatcher, err := certwatcher.New(certPath, keyPath)
if err != nil {
return err
}
cfg.GetCertificate = certWatcher.GetCertificate

cfg := &tls.Config{ //nolint:gosec
NextProtos: []string{"h2"},
GetCertificate: certWatcher.GetCertificate,
MinVersion: tlsMinVersion,
go func() {
if err := certWatcher.Start(ctx); err != nil {
log.Error(err, "certificate watcher error")
}
}()
}

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

// fallback TLS config ready, will now mutate if passer wants full control over it
for _, op := range s.TLSOpts {
op(cfg)
}

listener, err := tls.Listen("tcp", net.JoinHostPort(s.Host, strconv.Itoa(s.Port)), cfg)
if err != nil {
return err
Expand Down

0 comments on commit 1e52202

Please sign in to comment.