Uh oh!
There was an error while loading. Please reload this page.
🌱 Add support for CA/certificate rotation#1062
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| package httputil | ||
| import ( | ||
| "crypto/x509" | ||
| "fmt" | ||
| "os" | ||
| "sync" | ||
| "time" | ||
| "github.com/fsnotify/fsnotify" | ||
| "github.com/go-logr/logr" | ||
| ) | ||
| type CertPoolWatcher struct { | ||
| generation int | ||
| dir string | ||
| mx sync.RWMutex | ||
| pool *x509.CertPool | ||
| log logr.Logger | ||
| watcher *fsnotify.Watcher | ||
| done chan bool | ||
| } | ||
| // Returns the current CertPool and the generation number | ||
| func (cpw *CertPoolWatcher) Get() (*x509.CertPool, int, error) { | ||
| cpw.mx.RLock() | ||
| defer cpw.mx.RUnlock() | ||
| if cpw.pool == nil { | ||
| return nil, 0, fmt.Errorf("no certificate pool available") | ||
| } | ||
| return cpw.pool.Clone(), cpw.generation, nil | ||
| } | ||
| func (cpw *CertPoolWatcher) Done() { | ||
| cpw.done <- true | ||
| } | ||
| func NewCertPoolWatcher(caDir string, log logr.Logger) (*CertPoolWatcher, error) { | ||
| pool, err := NewCertPool(caDir, log) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| watcher, err := fsnotify.NewWatcher() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if err = watcher.Add(caDir); err != nil { | ||
| return nil, err | ||
| } | ||
| cpw := &CertPoolWatcher{ | ||
| generation: 1, | ||
| dir: caDir, | ||
| pool: pool, | ||
| log: log, | ||
| watcher: watcher, | ||
| done: make(chan bool), | ||
| } | ||
| go func() { | ||
| for { | ||
| select { | ||
| case <-watcher.Events: | ||
bentito marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| cpw.drainEvents() | ||
Contributor There was a problem hiding this comment. Do you know if any events include only the directory being watched as the event.Name value? If so, I wonder if instead of performing this drainEvents action we could do some event filtering similar to https://github.com/fsnotify/fsnotify/blob/c1467c02fba575afdb5f4201072ab8403bbf00f4/cmd/fsnotify/file.go#L66-L78 I won't block the PR merging on this, but something that could make it so we don't have any "sleep" actions if it is possible ContributorAuthor There was a problem hiding this comment. Some filtering might be useful. The only time this path should be updated is when a Secret is updated. The directory is read-only within the pod. ContributorAuthor There was a problem hiding this comment. Note: the filter will not work if new files are added, as they will be filtered out. We need to recognize new files, deleted files, updated files, etc. Contributor There was a problem hiding this comment.
If we only react to "directory has been updated" type events wouldn't we catch these events as well? ContributorAuthor There was a problem hiding this comment. But it wouldn't catch updates to files within, as that's a change to the file, not the directory. Contributor There was a problem hiding this comment. I thought the reasoning for the drain events operation was because when we receive updates we get mass events on everything when something changed. Maybe I misunderstood, which led me to thinking that if any change happened in the directory (including an individual file), it would trigger an event for the directory as well. ContributorAuthor There was a problem hiding this comment. The Watch is on the directory, and that includes the contents. It also depends on how things are mounted. A change to a file within a directory does not necessarily indicate a change to the directory. ContributorAuthor There was a problem hiding this comment. I added debug output to the cert watcher test for every event in the unit test: So, there's an event for the create of the new PEM, and one for the write, but nothing on the directory itself. There are two events, and that would cause two reloads without the drain mechanism in place. | ||
| cpw.update() | ||
| case err := <-watcher.Errors: | ||
| log.Error(err, "error watching certificate dir") | ||
| os.Exit(1) | ||
| case <-cpw.done: | ||
| err := watcher.Close() | ||
| if err != nil { | ||
| log.Error(err, "error closing watcher") | ||
everettraven marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| return | ||
| } | ||
| } | ||
| }() | ||
| return cpw, nil | ||
| } | ||
| func (cpw *CertPoolWatcher) update() { | ||
| cpw.log.Info("updating certificate pool") | ||
| pool, err := NewCertPool(cpw.dir, cpw.log) | ||
| if err != nil { | ||
| cpw.log.Error(err, "error updating certificate pool") | ||
| os.Exit(1) | ||
everettraven marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| cpw.mx.Lock() | ||
| defer cpw.mx.Unlock() | ||
| cpw.pool = pool | ||
| cpw.generation++ | ||
| } | ||
| // Drain as many events as possible before doing anything | ||
| // Otherwise, we will be hit with an event for _every_ entry in the | ||
| // directory, and end up doing an update for each one | ||
| func (cpw *CertPoolWatcher) drainEvents() { | ||
| for { | ||
| drainTimer := time.NewTimer(time.Millisecond * 50) | ||
| select { | ||
| case <-drainTimer.C: | ||
| return | ||
| case <-cpw.watcher.Events: | ||
| } | ||
| if !drainTimer.Stop() { | ||
| <-drainTimer.C | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package httputil_test | ||
| import ( | ||
| "context" | ||
| "crypto/ecdsa" | ||
| "crypto/elliptic" | ||
| "crypto/rand" | ||
| "crypto/x509" | ||
| "crypto/x509/pkix" | ||
| "encoding/pem" | ||
| "math/big" | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
| "time" | ||
| "github.com/stretchr/testify/require" | ||
| "sigs.k8s.io/controller-runtime/pkg/log" | ||
| "github.com/operator-framework/operator-controller/internal/httputil" | ||
| ) | ||
| func createCert(t *testing.T, name string) { | ||
| priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) | ||
| require.NoError(t, err) | ||
| notBefore := time.Now() | ||
| notAfter := notBefore.Add(time.Hour) | ||
| serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) | ||
| serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) | ||
| require.NoError(t, err) | ||
| template := x509.Certificate{ | ||
| SerialNumber: serialNumber, | ||
| Subject: pkix.Name{ | ||
| Organization: []string{name}, | ||
| }, | ||
| NotBefore: notBefore, | ||
| NotAfter: notAfter, | ||
| IsCA: true, | ||
| KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, | ||
| ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, | ||
| BasicConstraintsValid: true, | ||
| } | ||
| derBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv) | ||
| require.NoError(t, err) | ||
| certOut, err := os.Create(name) | ||
| require.NoError(t, err) | ||
| err = pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes}) | ||
| require.NoError(t, err) | ||
| err = certOut.Close() | ||
| require.NoError(t, err) | ||
| // ignore the key | ||
| } | ||
| func TestCertPoolWatcher(t *testing.T) { | ||
| // create a temporary directory | ||
| tmpDir, err := os.MkdirTemp("", "cert-pool") | ||
| require.NoError(t, err) | ||
| defer os.RemoveAll(tmpDir) | ||
| // create the first cert | ||
| certName := filepath.Join(tmpDir, "test1.pem") | ||
| t.Logf("Create cert file at %q\n", certName) | ||
| createCert(t, certName) | ||
| // Create the cert pool watcher | ||
| cpw, err := httputil.NewCertPoolWatcher(tmpDir, log.FromContext(context.Background())) | ||
| require.NoError(t, err) | ||
| defer cpw.Done() | ||
| // Get the original pool | ||
| firstPool, firstGen, err := cpw.Get() | ||
| require.NoError(t, err) | ||
| require.NotNil(t, firstPool) | ||
| // Create a second cert | ||
| certName = filepath.Join(tmpDir, "test2.pem") | ||
| t.Logf("Create cert file at %q\n", certName) | ||
| createCert(t, certName) | ||
| require.Eventually(t, func() bool { | ||
| secondPool, secondGen, err := cpw.Get() | ||
| if err != nil { | ||
| return false | ||
| } | ||
| return secondGen != firstGen && !firstPool.Equal(secondPool) | ||
| }, 30*time.Second, time.Second) | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.