Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ webhook: Handle TLS certificate rotation #421

Merged
merged 1 commit into from
May 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

162 changes: 162 additions & 0 deletions pkg/webhook/internal/certwatcher/certwatcher.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
/*
Copyright 2019 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package certwatcher

import (
"crypto/tls"
"sync"

"gopkg.in/fsnotify.v1"
mengqiy marked this conversation as resolved.
Show resolved Hide resolved
logf "sigs.k8s.io/controller-runtime/pkg/internal/log"
)

var log = logf.RuntimeLog.WithName("certwatcher")

// CertWatcher watches certificate and key files for changes. When either file
// changes, it reads and parses both and calls an optional callback with the new
// certificate.
type CertWatcher struct {
sync.Mutex

currentCert *tls.Certificate
watcher *fsnotify.Watcher

certPath string
keyPath string
}

// New returns a new CertWatcher watching the given certificate and key.
func New(certPath, keyPath string) (*CertWatcher, error) {
var err error

cw := &CertWatcher{
certPath: certPath,
keyPath: keyPath,
}

// Initial read of certificate and key.
if err := cw.ReadCertificate(); err != nil {
return nil, err
}

cw.watcher, err = fsnotify.NewWatcher()
if err != nil {
return nil, err
}

return cw, nil
}

// GetCertificate fetches the currently loaded certificate, which may be nil.
func (cw *CertWatcher) GetCertificate(_ *tls.ClientHelloInfo) (*tls.Certificate, error) {
cw.Lock()
defer cw.Unlock()
return cw.currentCert, nil
}

// Start starts the watch on the certificate and key files.
func (cw *CertWatcher) Start(stopCh <-chan struct{}) error {
files := []string{cw.certPath, cw.keyPath}

for _, f := range files {
if err := cw.watcher.Add(f); err != nil {
return err
}
}

go cw.Watch()

log.Info("Starting certificate watcher")

// Block until the stop channel is closed.
<-stopCh

return cw.watcher.Close()
}

// Watch reads events from the watcher's channel and reacts to changes.
func (cw *CertWatcher) Watch() {
for {
select {
case event, ok := <-cw.watcher.Events:
// Channel is closed.
if !ok {
return
}

cw.handleEvent(event)

case err, ok := <-cw.watcher.Errors:
// Channel is closed.
if !ok {
return
}

log.Error(err, "certificate watch error")
}
}
}

// ReadCertificate reads the certificate and key files from disk, parses them,
// and updates the current certificate on the watcher. If a callback is set, it
// is invoked with the new certificate.
func (cw *CertWatcher) ReadCertificate() error {
cert, err := tls.LoadX509KeyPair(cw.certPath, cw.keyPath)
if err != nil {
return err
}

cw.Lock()
cw.currentCert = &cert
cw.Unlock()

log.Info("Updated current TLS certiface")

return nil
}

func (cw *CertWatcher) handleEvent(event fsnotify.Event) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a secret get updated it actually triggers a rename event. Ref: algorithm of volume writer.
I'd like to know what testing has been done to verify if it work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this is a bit tricky. So, fsnotify always fully resolves symlinks. In the case of a Kubernetes volume mount, the certificate and key files in the current data directory are watched. On an update, it's actually the remove event after the rename that is picked up here, then the watch is re-added following the new symlink.

I've tested this manually pretty extensively. My use-case is that I have another controller in the cluster generating and rotating short-lived TLS certificates for my controller-runtime based project. With this patch, webhook requests have continued to succeed after the periodic refresh of the TLS assets.

// Only care about events which may modify the contents of the file.
if !(isWrite(event) || isRemove(event) || isCreate(event)) {
return
}

log.V(1).Info("certificate event", "event", event)

// If the file was removed, re-add the watch.
if isRemove(event) {
if err := cw.watcher.Add(event.Name); err != nil {
log.Error(err, "error re-watching file")
}
}

if err := cw.ReadCertificate(); err != nil {
log.Error(err, "error re-reading certificate")
}
}

func isWrite(event fsnotify.Event) bool {
return event.Op&fsnotify.Write == fsnotify.Write
}

func isCreate(event fsnotify.Event) bool {
return event.Op&fsnotify.Create == fsnotify.Create
}

func isRemove(event fsnotify.Event) bool {
return event.Op&fsnotify.Remove == fsnotify.Remove
}
18 changes: 14 additions & 4 deletions pkg/webhook/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ import (
"net"
"net/http"
"path"
"path/filepath"
"strconv"
"sync"
"time"

"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
"sigs.k8s.io/controller-runtime/pkg/webhook/internal/certwatcher"
"sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics"
)

Expand Down Expand Up @@ -132,15 +134,23 @@ func (s *Server) Start(stop <-chan struct{}) error {
}
}

// TODO: watch the cert dir. Reload the cert if it changes
cert, err := tls.LoadX509KeyPair(path.Join(s.CertDir, certName), path.Join(s.CertDir, keyName))
certPath := filepath.Join(s.CertDir, certName)
keyPath := filepath.Join(s.CertDir, keyName)

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

go func() {
if err := certWatcher.Start(stop); err != nil {
log.Error(err, "certificate watcher error")
}
}()

cfg := &tls.Config{
Certificates: []tls.Certificate{cert},
NextProtos: []string{"h2"},
NextProtos: []string{"h2"},
GetCertificate: certWatcher.GetCertificate,
}

listener, err := tls.Listen("tcp", net.JoinHostPort(s.Host, strconv.Itoa(int(s.Port))), cfg)
Expand Down