Skip to content

Commit 2d53f85

Browse files
committed
fixup! Add support for CA/certificate rotation
Signed-off-by: Todd Short <[email protected]>
1 parent 5e7beee commit 2d53f85

File tree

8 files changed

+160
-90
lines changed

8 files changed

+160
-90
lines changed

.github/workflows/e2e.yaml

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,3 @@ jobs:
5959

6060
- name: Run the upgrade e2e test
6161
run: make test-upgrade-e2e
62-
63-
cert-e2e:
64-
runs-on: ubuntu-latest
65-
steps:
66-
- uses: actions/checkout@v4
67-
68-
- uses: actions/setup-go@v5
69-
with:
70-
go-version-file: go.mod
71-
72-
- name: Run the cert e2e test
73-
run: make test-cert-e2e

Makefile

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,14 +188,6 @@ test-upgrade-e2e: export TEST_CLUSTER_CATALOG_NAME := test-catalog
188188
test-upgrade-e2e: export TEST_CLUSTER_EXTENSION_NAME := test-package
189189
test-upgrade-e2e: kind-cluster run-latest-release image-registry build-push-e2e-catalog registry-load-bundles pre-upgrade-setup docker-build kind-load kind-deploy post-upgrade-checks kind-clean #HELP Run upgrade e2e tests on a local kind cluster
190190

191-
.PHONY: run-cert-e2e
192-
run-cert-e2e:
193-
./hack/test/cert-e2e.sh
194-
195-
.PHONY: test-cert-e2e
196-
test-cert-e2e: KIND_CLUSTER_NAME := operator-controller-cert-e2e
197-
test-cert-e2e: run run-cert-e2e kind-clean #HELP Run certificate e2e tests on a local kind cluster
198-
199191
.PHONY: e2e-coverage
200192
e2e-coverage:
201193
COVERAGE_OUTPUT=./coverage/e2e.out ./hack/test/e2e-coverage.sh

hack/test/cert-e2e.sh

Lines changed: 0 additions & 26 deletions
This file was deleted.

internal/httputil/certpoolwatcher.go

Lines changed: 58 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -12,47 +12,27 @@ import (
1212
)
1313

1414
type CertPoolWatcher struct {
15-
dir string
16-
mx sync.RWMutex
17-
pool *x509.CertPool
18-
log logr.Logger
19-
watcher *fsnotify.Watcher
15+
generation int
16+
dir string
17+
mx sync.RWMutex
18+
pool *x509.CertPool
19+
log logr.Logger
20+
watcher *fsnotify.Watcher
21+
done chan bool
2022
}
2123

22-
func (cpw *CertPoolWatcher) Get() (*x509.CertPool, error) {
23-
if cpw.pool == nil {
24-
return nil, fmt.Errorf("no certificate pool available")
25-
}
24+
// Returns the current CertPool and the generation number
25+
func (cpw *CertPoolWatcher) Get() (*x509.CertPool, int, error) {
2626
cpw.mx.RLock()
2727
defer cpw.mx.RUnlock()
28-
return cpw.pool.Clone(), nil
29-
}
30-
31-
func (cpw *CertPoolWatcher) update() {
32-
cpw.log.Info("updating certificate pool")
33-
pool, err := NewCertPool(cpw.dir, cpw.log)
34-
if err != nil {
35-
cpw.log.Error(err, "error updating certificate pool")
36-
os.Exit(1)
28+
if cpw.pool == nil {
29+
return nil, 0, fmt.Errorf("no certificate pool available")
3730
}
38-
cpw.mx.Lock()
39-
defer cpw.mx.Unlock()
40-
cpw.pool = pool
31+
return cpw.pool.Clone(), cpw.generation, nil
4132
}
4233

43-
// Drain as many events as possible before doing anything
44-
// Otherwise, we will be hit with an event for _every_ entry in the
45-
// directory, and end up doing an update for each one
46-
func (cpw *CertPoolWatcher) drainEvents() {
47-
for {
48-
// sleep to let events accumulate
49-
time.Sleep(time.Millisecond * 50)
50-
select {
51-
case <-cpw.watcher.Events:
52-
default:
53-
return
54-
}
55-
}
34+
func (cpw *CertPoolWatcher) Done() {
35+
cpw.done <- true
5636
}
5737

5838
func NewCertPoolWatcher(caDir string, log logr.Logger) (*CertPoolWatcher, error) {
@@ -67,23 +47,62 @@ func NewCertPoolWatcher(caDir string, log logr.Logger) (*CertPoolWatcher, error)
6747
if err = watcher.Add(caDir); err != nil {
6848
return nil, err
6949
}
50+
7051
cpw := &CertPoolWatcher{
71-
dir: caDir,
72-
pool: pool,
73-
log: log,
74-
watcher: watcher,
52+
generation: 1,
53+
dir: caDir,
54+
pool: pool,
55+
log: log,
56+
watcher: watcher,
57+
done: make(chan bool),
7558
}
7659
go func() {
7760
for {
7861
select {
7962
case <-watcher.Events:
8063
cpw.drainEvents()
8164
cpw.update()
82-
case err = <-watcher.Errors:
65+
case err := <-watcher.Errors:
8366
log.Error(err, "error watching certificate dir")
8467
os.Exit(1)
68+
case <-cpw.done:
69+
err := watcher.Close()
70+
if err != nil {
71+
log.Error(err, "error closing watcher")
72+
}
73+
return
8574
}
8675
}
8776
}()
8877
return cpw, nil
8978
}
79+
80+
func (cpw *CertPoolWatcher) update() {
81+
cpw.log.Info("updating certificate pool")
82+
pool, err := NewCertPool(cpw.dir, cpw.log)
83+
if err != nil {
84+
cpw.log.Error(err, "error updating certificate pool")
85+
os.Exit(1)
86+
}
87+
cpw.mx.Lock()
88+
defer cpw.mx.Unlock()
89+
cpw.pool = pool
90+
cpw.generation++
91+
}
92+
93+
// Drain as many events as possible before doing anything
94+
// Otherwise, we will be hit with an event for _every_ entry in the
95+
// directory, and end up doing an update for each one
96+
func (cpw *CertPoolWatcher) drainEvents() {
97+
for {
98+
drainTimer := time.NewTimer(time.Millisecond * 50)
99+
select {
100+
case <-drainTimer.C:
101+
return
102+
case <-cpw.watcher.Events:
103+
}
104+
if !drainTimer.Stop() {
105+
<-drainTimer.C
106+
}
107+
}
108+
}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
package httputil_test
2+
3+
import (
4+
"context"
5+
"crypto/ecdsa"
6+
"crypto/elliptic"
7+
"crypto/rand"
8+
"crypto/x509"
9+
"crypto/x509/pkix"
10+
"encoding/pem"
11+
"math/big"
12+
"os"
13+
"path/filepath"
14+
"testing"
15+
"time"
16+
17+
"github.com/stretchr/testify/require"
18+
"sigs.k8s.io/controller-runtime/pkg/log"
19+
20+
"github.com/operator-framework/operator-controller/internal/httputil"
21+
)
22+
23+
func createCert(t *testing.T, name string) error {
24+
priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
25+
require.NoError(t, err)
26+
27+
notBefore := time.Now()
28+
notAfter := notBefore.Add(time.Hour)
29+
serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
30+
serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
31+
require.NoError(t, err)
32+
33+
template := x509.Certificate{
34+
SerialNumber: serialNumber,
35+
Subject: pkix.Name{
36+
Organization: []string{name},
37+
},
38+
NotBefore: notBefore,
39+
NotAfter: notAfter,
40+
41+
IsCA: true,
42+
43+
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
44+
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
45+
46+
BasicConstraintsValid: true,
47+
}
48+
49+
derBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
50+
require.NoError(t, err)
51+
52+
certOut, err := os.Create(name)
53+
require.NoError(t, err)
54+
55+
err = pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes})
56+
require.NoError(t, err)
57+
58+
err = certOut.Close()
59+
require.NoError(t, err)
60+
61+
// ignore the key
62+
return nil
63+
}
64+
65+
func TestCertPoolWatcher(t *testing.T) {
66+
// create a temporary directory
67+
tmpDir, err := os.MkdirTemp("", "cert-pool")
68+
require.NoError(t, err)
69+
defer os.RemoveAll(tmpDir)
70+
71+
// create the first cert
72+
certName := filepath.Join(tmpDir, "test1.pem")
73+
t.Logf("Create cert file at %q\n", certName)
74+
createCert(t, certName)
75+
76+
// Create the cert pool watcher
77+
cpw, err := httputil.NewCertPoolWatcher(tmpDir, log.FromContext(context.Background()))
78+
require.NoError(t, err)
79+
defer cpw.Done()
80+
81+
// Get the original pool
82+
firstPool, firstGen, err := cpw.Get()
83+
require.NoError(t, err)
84+
require.NotNil(t, firstPool)
85+
86+
// Create a second cert
87+
certName = filepath.Join(tmpDir, "test2.pem")
88+
t.Logf("Create cert file at %q\n", certName)
89+
createCert(t, certName)
90+
91+
require.Eventually(t, func() bool {
92+
secondPool, secondGen, err := cpw.Get()
93+
if err != nil {
94+
return false
95+
}
96+
return secondGen != firstGen && !firstPool.Equal(secondPool)
97+
}, 30*time.Second, time.Second)
98+
}

internal/httputil/certutil.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,9 @@ func appendCertsFromPEM(s *x509.CertPool, pemCerts []byte, firstExpiration *time
190190
return fmt.Errorf("not yet valid cert %d: %q", n, cert.NotBefore.Format(time.RFC3339))
191191
} else if now.After(cert.NotAfter) {
192192
return fmt.Errorf("expired cert %d: %q", n, cert.NotAfter.Format(time.RFC3339))
193-
} else {
194-
// no return values - panics or always succeeds
195-
s.AddCert(cert)
196193
}
194+
// no return values - panics or always succeeds
195+
s.AddCert(cert)
197196
n++
198197
}
199198

internal/httputil/httputil.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
func BuildHTTPClient(cpw *CertPoolWatcher) (*http.Client, error) {
1010
httpClient := &http.Client{Timeout: 10 * time.Second}
1111

12-
pool, err := cpw.Get()
12+
pool, _, err := cpw.Get()
1313
if err != nil {
1414
return nil, err
1515
}

internal/rukpak/source/image_registry.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func (i *ImageRegistry) Unpack(ctx context.Context, bundle *BundleSource) (*Resu
101101
transport.TLSClientConfig.InsecureSkipVerify = true // nolint:gosec
102102
}
103103
if i.CertPoolWatcher != nil {
104-
pool, err := i.CertPoolWatcher.Get()
104+
pool, _, err := i.CertPoolWatcher.Get()
105105
if err != nil {
106106
return nil, err
107107
}

0 commit comments

Comments
 (0)