🐛 OPRUN-4110: Restart when SystemCertPool should change

cmd/catalogd/main.go

@@ -60,6 +60,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/catalogd/webhook"
 	sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers"
 	fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs"
+	httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
 	imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
 	"github.com/operator-framework/operator-controller/internal/shared/util/pullsecretcache"
 	sautil "github.com/operator-framework/operator-controller/internal/shared/util/sa"
@@ -291,6 +292,18 @@ func run(ctx context.Context) error {
 		return err
 	}
 
+	// This watches the pullCasDir and the SSL_CERT_DIR, and SSL_CERT_FILE for changes
+	cpwPull, err := httputil.NewCertPoolWatcher(cfg.pullCasDir, ctrl.Log.WithName("pull-ca-pool"))
+	if err != nil {
+		setupLog.Error(err, "unable to create pull-ca-pool watcher")
+		return err
+	}
+	cpwPull.Restart(os.Exit)
+	if err = mgr.Add(cpwPull); err != nil {
+		setupLog.Error(err, "unable to add pull-ca-pool watcher to manager")
+		return err
+	}
+
 	if cfg.systemNamespace == "" {
 		cfg.systemNamespace = podNamespace()
 	}

cmd/operator-controller/main.go

@@ -319,9 +319,26 @@ func run() error {
 		return err
 	}
 
-	certPoolWatcher, err := httputil.NewCertPoolWatcher(cfg.catalogdCasDir, ctrl.Log.WithName("cert-pool"))
+	cpwCatalogd, err := httputil.NewCertPoolWatcher(cfg.catalogdCasDir, ctrl.Log.WithName("catalogd-ca-pool"))
 	if err != nil {
-		setupLog.Error(err, "unable to create CA certificate pool")
+		setupLog.Error(err, "unable to create catalogd-ca-pool watcher")
+		return err
+	}
+	cpwCatalogd.Restart(os.Exit)
+	if err = mgr.Add(cpwCatalogd); err != nil {
+		setupLog.Error(err, "unable to add catalogd-ca-pool watcher to manager")
+		return err
+	}
+
+	// This watches the pullCasDir and the SSL_CERT_DIR, and SSL_CERT_FILE for changes
+	cpwPull, err := httputil.NewCertPoolWatcher(cfg.pullCasDir, ctrl.Log.WithName("pull-ca-pool"))
+	if err != nil {
+		setupLog.Error(err, "unable to create pull-ca-pool watcher")
+		return err
+	}
+	cpwPull.Restart(os.Exit)
+	if err = mgr.Add(cpwPull); err != nil {
+		setupLog.Error(err, "unable to add pull-ca-pool watcher to manager")
 		return err
 	}
 
@@ -375,7 +392,7 @@ func run() error {
 	}
 	catalogClientBackend := cache.NewFilesystemCache(catalogsCachePath)
 	catalogClient := catalogclient.New(catalogClientBackend, func() (*http.Client, error) {
-		return httputil.BuildHTTPClient(certPoolWatcher)
+		return httputil.BuildHTTPClient(cpwCatalogd)
 	})
 
 	resolver := &resolve.CatalogResolver{

internal/shared/util/http/certlog.go

@@ -122,7 +122,16 @@ func logFile(filename, path, action string, log logr.Logger) {
 		log.Error(err, "error in os.ReadFile()", "file", filepath)
 		return
 	}
-	logPem(data, filename, path, action, log)
+	if len(data) > 0 {
+		logPem(data, filename, path, action, log)
+		return
+	}
+	// Indicate that the file is empty
+	args := []any{"file", filename, "empty", "true"}
+	if path != "" {
+		args = append(args, "directory", path)
+	}
+	log.V(defaultLogLevel).Info(action, args...)
 }
 
 func logPem(data []byte, filename, path, action string, log logr.Logger) {

internal/shared/util/http/certpoolwatcher.go

@@ -1,6 +1,7 @@
 package http
 
 import (
+	"context"
 	"crypto/x509"
 	"fmt"
 	"os"
@@ -14,13 +15,15 @@ import (
 )
 
 type CertPoolWatcher struct {
-	generation int
-	dir        string
-	mx         sync.RWMutex
-	pool       *x509.CertPool
-	log        logr.Logger
-	watcher    *fsnotify.Watcher
-	done       chan bool
+	generation   int
+	dir          string
+	sslCertPaths []string
+	mx           sync.RWMutex
+	pool         *x509.CertPool
+	log          logr.Logger
+	watcher      *fsnotify.Watcher
+	done         chan bool
+	restart      func(int)
 }
 
 // Returns the current CertPool and the generation number
@@ -33,77 +36,111 @@ func (cpw *CertPoolWatcher) Get() (*x509.CertPool, int, error) {
 	return cpw.pool.Clone(), cpw.generation, nil
 }
 
-func (cpw *CertPoolWatcher) Done() {
-	cpw.done <- true
+// Change the restart behavior
+func (cpw *CertPoolWatcher) Restart(f func(int)) {
+	cpw.restart = f
 }
 
-func NewCertPoolWatcher(caDir string, log logr.Logger) (*CertPoolWatcher, error) {
-	pool, err := NewCertPool(caDir, log)
-	if err != nil {
-		return nil, err
+// Indicate that you're done with the CertPoolWatcher so it can terminate
+// the watcher go func
+func (cpw *CertPoolWatcher) Done() {
+	if cpw.watcher != nil {
+		cpw.done <- true
 	}
-	watcher, err := fsnotify.NewWatcher()
+}
+
+func (cpw *CertPoolWatcher) Start(ctx context.Context) error {
+	var err error
+	cpw.pool, err = NewCertPool(cpw.dir, cpw.log)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	// If the SSL_CERT_DIR or SSL_CERT_FILE environment variables are
-	// specified, this means that we have some control over the system root
-	// location, thus they may change, thus we should watch those locations.
-	sslCertDir := os.Getenv("SSL_CERT_DIR")
-	sslCertFile := os.Getenv("SSL_CERT_FILE")
-	log.V(defaultLogLevel).Info("SSL environment", "SSL_CERT_DIR", sslCertDir, "SSL_CERT_FILE", sslCertFile)
+	watchPaths := append(cpw.sslCertPaths, cpw.dir)
+	watchPaths = slices.DeleteFunc(watchPaths, deleteEmptyStrings)
 
-	watchPaths := strings.Split(sslCertDir, ":")
-	watchPaths = append(watchPaths, caDir, sslCertFile)
-	watchPaths = slices.DeleteFunc(watchPaths, func(p string) bool {
-		if p == "" {
-			return true
-		}
-		if _, err := os.Stat(p); err != nil {
-			return true
-		}
-		return false
-	})
+	// Nothing was configured to be watched, which means this is
+	// using the SystemCertPool, so we still need to no error out
+	if len(watchPaths) == 0 {
+		cpw.log.Info("No paths to watch")
+		return nil
+	}
+
+	cpw.watcher, err = fsnotify.NewWatcher()
+	if err != nil {
+		return err
+	}
 
 	for _, p := range watchPaths {
-		if err := watcher.Add(p); err != nil {
-			return nil, err
+		if err := cpw.watcher.Add(p); err != nil {
+			cpw.watcher.Close()
+			cpw.watcher = nil
+			return err
 		}
-		logPath(p, "watching certificate", log)
+		logPath(p, "watching certificate", cpw.log)
 	}
 
-	cpw := &CertPoolWatcher{
-		generation: 1,
-		dir:        caDir,
-		pool:       pool,
-		log:        log,
-		watcher:    watcher,
-		done:       make(chan bool),
-	}
 	go func() {
 		for {
 			select {
-			case <-watcher.Events:
+			case e := <-cpw.watcher.Events:
+				cpw.checkForRestart(e.Name)
 				cpw.drainEvents()
-				cpw.update()
-			case err := <-watcher.Errors:
-				log.Error(err, "error watching certificate dir")
+				cpw.update(e.Name)
+			case err := <-cpw.watcher.Errors:
+				cpw.log.Error(err, "error watching certificate dir")
 				os.Exit(1)
+			case <-ctx.Done():
+				cpw.Done()
 			case <-cpw.done:
-				err := watcher.Close()
+				err := cpw.watcher.Close()
 				if err != nil {
-					log.Error(err, "error closing watcher")
+					cpw.log.Error(err, "error closing watcher")
 				}
 				return
 			}
 		}
 	}()
+	return nil
+}
+
+func NewCertPoolWatcher(caDir string, log logr.Logger) (*CertPoolWatcher, error) {
+	// If the SSL_CERT_DIR or SSL_CERT_FILE environment variables are
+	// specified, this means that we have some control over the system root
+	// location, thus they may change, thus we should watch those locations.
+	//
+	// BECAUSE THE SYSTEM POOL WILL NOT UPDATE, WE HAVE TO RESTART IF THERE
+	// CHANGES TO EITHER OF THESE LOCATIONS: SSL_CERT_DIR, SSL_CERT_FILE
+	//
+	sslCertDir := os.Getenv("SSL_CERT_DIR")
+	sslCertFile := os.Getenv("SSL_CERT_FILE")
+	log.V(defaultLogLevel).Info("SSL environment", "SSL_CERT_DIR", sslCertDir, "SSL_CERT_FILE", sslCertFile)
+
+	sslCertPaths := append(strings.Split(sslCertDir, ":"), sslCertFile)
+	sslCertPaths = slices.DeleteFunc(sslCertPaths, deleteEmptyStrings)
+
+	cpw := &CertPoolWatcher{
+		generation:   1,
+		dir:          caDir,
+		sslCertPaths: sslCertPaths,
+		log:          log,
+		done:         make(chan bool),
+	}
 	return cpw, nil
 }
 
-func (cpw *CertPoolWatcher) update() {
-	cpw.log.Info("updating certificate pool")
+func deleteEmptyStrings(p string) bool {
+	if p == "" {
+		return true
+	}
+	if _, err := os.Stat(p); err != nil {
+		return true
+	}
+	return false
+}
+
+func (cpw *CertPoolWatcher) update(name string) {
+	cpw.log.Info("updating certificate pool", "file", name)
 	pool, err := NewCertPool(cpw.dir, cpw.log)
 	if err != nil {
 		cpw.log.Error(err, "error updating certificate pool")
@@ -115,6 +152,17 @@ func (cpw *CertPoolWatcher) update() {
 	cpw.generation++
 }
 
+func (cpw *CertPoolWatcher) checkForRestart(name string) {
+	for _, p := range cpw.sslCertPaths {
+		if strings.Contains(name, p) {
+			cpw.log.Info("restarting due to file change", "file", name)
+			if cpw.restart != nil {
+				cpw.restart(0)
+			}
+		}
+	}
+}
+
 // 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
@@ -124,7 +172,8 @@ func (cpw *CertPoolWatcher) drainEvents() {
 		select {
 		case <-drainTimer.C:
 			return
-		case <-cpw.watcher.Events:
+		case e := <-cpw.watcher.Events:
+			cpw.checkForRestart(e.Name)
 		}
 		if !drainTimer.Stop() {
 			<-drainTimer.C