Merge pull request #1155 from bryan-cox/HOSTEDCP-1994

HOSTEDCP-1994: Add filewatcher for Azure client certificate authentication

Author: openshift-merge-bot[bot]

go.mod

@@ -50,7 +50,6 @@ require (
 	k8s.io/apimachinery v0.30.1
 	k8s.io/client-go v0.30.1
 	k8s.io/klog/v2 v2.120.1
-	k8s.io/kubernetes v1.30.2
 	k8s.io/utils v0.0.0-20230726121419-3b25d923346b
 )
 

go.sum

@@ -1721,8 +1721,6 @@ k8s.io/kube-aggregator v0.30.1 h1:ymR2BsxDacTKwzKTuNhGZttuk009c+oZbSeD+IPX5q4=
 k8s.io/kube-aggregator v0.30.1/go.mod h1:SFbqWsM6ea8dHd3mPLsZFzJHbjBOS5ykIgJh4znZ5iQ=
 k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag=
 k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98=
-k8s.io/kubernetes v1.30.2 h1:11WhS78OYX/lnSy6TXxPO6Hk+E5K9ZNrEsk9JgMSX8I=
-k8s.io/kubernetes v1.30.2/go.mod h1:yPbIk3MhmhGigX62FLJm+CphNtjxqCvAIFQXup6RKS0=
 k8s.io/utils v0.0.0-20230726121419-3b25d923346b h1:sgn3ZU783SCgtaSJjpcVVlRqd6GSnlTLKgpAAttJvpI=
 k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 lukechampine.com/uint128 v1.1.1/go.mod h1:c4eWIwlEGaxC/+H1VguhU4PHXNWDCDMUlWdIWl2j1gk=

pkg/filewatcher/filewatcher.go

@@ -0,0 +1,67 @@
+package filewatcher
+
+import (
+	"os"
+	"path/filepath"
+	"sync"
+
+	"github.com/fsnotify/fsnotify"
+	"k8s.io/klog/v2"
+)
+
+var watchCertificateFileOnce sync.Once
+
+// WatchFileForChanges watches the file, fileToWatch, for changes. If the file contents have changed, the pod this
+// function is running on will be restarted.
+func WatchFileForChanges(fileToWatch string) error {
+	var err error
+
+	// This starts only one occurrence of the file watcher, which watches the file, fileToWatch.
+	watchCertificateFileOnce.Do(func() {
+		klog.Infof("Starting the file change watcher on file, %s", fileToWatch)
+
+		// Update the file path to watch in case this is a symlink
+		fileToWatch, err = filepath.EvalSymlinks(fileToWatch)
+		if err != nil {
+			return
+		}
+		klog.Infof("Watching file, %s", fileToWatch)
+
+		// Start the file watcher to monitor file changes
+		go func() {
+			err = checkForFileChanges(fileToWatch)
+		}()
+	})
+	return err
+}
+
+// checkForFileChanges starts a new file watcher. If the file is changed, the pod running this function will exit.
+func checkForFileChanges(path string) error {
+	watcher, err := fsnotify.NewWatcher()
+	if err != nil {
+		return err
+	}
+
+	go func() {
+		for {
+			select {
+			case event, ok := <-watcher.Events:
+				if ok && (event.Has(fsnotify.Write) || event.Has(fsnotify.Chmod) || event.Has(fsnotify.Remove)) {
+					klog.Infof("file, %s, was modified, exiting...", event.Name)
+					os.Exit(0)
+				}
+			case err, ok := <-watcher.Errors:
+				if ok {
+					klog.Errorf("file watcher error: %v", err)
+				}
+			}
+		}
+	}()
+
+	err = watcher.Add(path)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}

pkg/storage/azure/azure.go

@@ -29,7 +29,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/rand"
 	kcorelisters "k8s.io/client-go/listers/core/v1"
 	"k8s.io/klog/v2"
-	"k8s.io/kubernetes/pkg/util/interrupt"
 
 	configv1 "github.com/openshift/api/config/v1"
 	imageregistryv1 "github.com/openshift/api/imageregistry/v1"
@@ -39,6 +38,7 @@ import (
 	regopclient "github.com/openshift/cluster-image-registry-operator/pkg/client"
 	"github.com/openshift/cluster-image-registry-operator/pkg/defaults"
 	"github.com/openshift/cluster-image-registry-operator/pkg/envvar"
+	"github.com/openshift/cluster-image-registry-operator/pkg/filewatcher"
 	"github.com/openshift/cluster-image-registry-operator/pkg/storage/azure/azureclient"
 	"github.com/openshift/cluster-image-registry-operator/pkg/storage/util"
 )
@@ -395,18 +395,8 @@ func (d *driver) storageAccountsClient(cfg *Azure, environment autorestazure.Env
 			return storage.AccountsClient{}, fmt.Errorf(`failed to parse certificate data "%s": %v`, certPath, err)
 		}
 
-		// Set up a watch on our config file; if it changes, we should exit -
-		// (we don't have the ability to dynamically reload config changes).
-		stopCh := make(chan struct{})
-		err = interrupt.New(func(s os.Signal) {
-			_, _ = fmt.Fprintf(os.Stderr, "interrupt: Gracefully shutting down ...\n")
-			close(stopCh)
-		}).Run(func() error {
-			if err := azureclient.WatchForChanges(certPath, stopCh); err != nil {
-				return err
-			}
-			return nil
-		})
+		// Watch the certificate for changes; if the certificate changes, the pod will be restarted
+		err = filewatcher.WatchFileForChanges(certPath)
 		if err != nil {
 			return storage.AccountsClient{}, err
 		}

pkg/storage/azure/azureclient/azureclient.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"net/http"
 	"os"
-	"path/filepath"
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
@@ -22,9 +21,8 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/container"
 	autorestazure "github.com/Azure/go-autorest/autorest/azure"
 	"github.com/Azure/go-autorest/autorest/to"
-	"github.com/fsnotify/fsnotify"
+	"github.com/openshift/cluster-image-registry-operator/pkg/filewatcher"
 	"k8s.io/klog/v2"
-	"k8s.io/kubernetes/pkg/util/interrupt"
 )
 
 const (
@@ -130,18 +128,8 @@ func (c *Client) getCreds() (azcore.TokenCredential, error) {
 			return nil, fmt.Errorf(`failed to parse certificate data "%s": %v`, certPath, err)
 		}
 
-		// Set up a watch on our config file; if it changes, we should exit -
-		// (we don't have the ability to dynamically reload config changes).
-		stopCh := make(chan struct{})
-		err = interrupt.New(func(s os.Signal) {
-			_, _ = fmt.Fprintf(os.Stderr, "interrupt: Gracefully shutting down ...\n")
-			close(stopCh)
-		}).Run(func() error {
-			if err := WatchForChanges(certPath, stopCh); err != nil {
-				return err
-			}
-			return nil
-		})
+		// Watch the certificate for changes; if the certificate changes, the pod will be restarted
+		err = filewatcher.WatchFileForChanges(certPath)
 		if err != nil {
 			return nil, err
 		}
@@ -939,59 +927,3 @@ func (client *BlobClient) DeleteStorageContainer(ctx context.Context, containerN
 	_, err := client.client.DeleteContainer(ctx, containerName, &azblob.DeleteContainerOptions{})
 	return err
 }
-
-// WatchForChanges closes stopCh if the configuration file changed.
-func WatchForChanges(configPath string, stopCh chan struct{}) error {
-	configPath, err := filepath.Abs(configPath)
-	if err != nil {
-		return err
-	}
-	watcher, err := fsnotify.NewWatcher()
-	if err != nil {
-		return err
-	}
-	// Watch all symlinks for changes
-	p := configPath
-	maxDepth := 100
-	for depth := 0; depth < maxDepth; depth++ {
-		if err := watcher.Add(p); err != nil {
-			return err
-		}
-		klog.V(2).Infof("Watching config file %s for changes", p)
-		stat, err := os.Lstat(p)
-		if err != nil {
-			return err
-		}
-		// configmaps are usually symlinks
-		if stat.Mode()&os.ModeSymlink > 0 {
-			p, err = filepath.EvalSymlinks(p)
-			if err != nil {
-				return err
-			}
-		} else {
-			break
-		}
-	}
-	go func() {
-		for {
-			select {
-			case <-stopCh:
-			case event, ok := <-watcher.Events:
-				if !ok {
-					klog.Errorf("failed to watch config file %s", p)
-					return
-				}
-				klog.V(2).Infof("Configuration file %s changed, exiting...", event.Name)
-				close(stopCh)
-				return
-			case err, ok := <-watcher.Errors:
-				if !ok {
-					klog.Errorf("failed to watch config file %s", p)
-					return
-				}
-				klog.Errorf("fsnotify error: %v", err)
-			}
-		}
-	}()
-	return nil
-}