fix(operator): propagate webhook cert disk write errors

operator/internal/controller/cert_utils.go

@@ -30,6 +30,7 @@ import (
 	"math/big"
 	"os"
 	"path/filepath"
+	"sync"
 	"time"
 
 	"github.com/NVIDIA/nodewright/operator/api/v1alpha1"
@@ -243,7 +244,13 @@ func ensureDummyCert(certDir string) error {
 	return nil
 }
 
+// certWriteMu serializes writeCertAndKey. It is package-level because the writers
+// (webhook bootstrap reconcile and SecretCertWatcher) share no struct, only the
+// process and the cert directory.
+var certWriteMu sync.Mutex
+
 // Write cert and key to disk if CertDir is set, make sure the directory exists, and the files are 600. Will also sync from the secret to the disk.
+// Writes are serialized across goroutines and each file is replaced atomically (temp file + rename).
 func writeCertAndKey(certPEM, keyPEM []byte, certDir string) error {
 	if certDir == "" {
 		return fmt.Errorf("certDir is required")
@@ -254,15 +261,59 @@ func writeCertAndKey(certPEM, keyPEM []byte, certDir string) error {
 
 	certPath := filepath.Join(certDir, "tls.crt")
 	keyPath := filepath.Join(certDir, "tls.key")
-	if err := os.WriteFile(certPath, certPEM, 0o600); err != nil {
+
+	// Serialize writers so a cert write and a key write from two goroutines
+	// (bootstrap reconcile and SecretCertWatcher) can never interleave and
+	// leave tls.crt from one cert next to tls.key from another.
+	certWriteMu.Lock()
+	defer certWriteMu.Unlock()
+
+	if err := writeFileAtomic(certPath, certPEM, 0o600); err != nil {
 		return fmt.Errorf("failed to write cert: %w", err)
 	}
-	if err := os.WriteFile(keyPath, keyPEM, 0o600); err != nil {
+	if err := writeFileAtomic(keyPath, keyPEM, 0o600); err != nil {
 		return fmt.Errorf("failed to write key: %w", err)
 	}
 	return nil
 }
 
+// writeFileAtomic writes data to a temp file in the same directory as path and
+// renames it into place, so readers see either the old file or the new one in
+// full, never a partially written file. The temp file lives next to path so the
+// rename stays on one filesystem (rename is only atomic within a filesystem).
+func writeFileAtomic(path string, data []byte, perm os.FileMode) error {
+	tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".tmp-*")
+	if err != nil {
+		return err
+	}
+	tmpName := tmp.Name()
+
+	if _, err := tmp.Write(data); err != nil {
+		_ = tmp.Close()
+		_ = os.Remove(tmpName)
+		return err
+	}
+	if err := tmp.Chmod(perm); err != nil {
+		_ = tmp.Close()
+		_ = os.Remove(tmpName)
+		return err
+	}
+	if err := tmp.Sync(); err != nil {
+		_ = tmp.Close()
+		_ = os.Remove(tmpName)
+		return err
+	}
+	if err := tmp.Close(); err != nil {
+		_ = os.Remove(tmpName)
+		return err
+	}
+	if err := os.Rename(tmpName, path); err != nil {
+		_ = os.Remove(tmpName)
+		return err
+	}
+	return nil
+}
+
 // certExistsOnDisk returns an error if the cert or key is not present on disk
 func certExistsOnDisk(certDir string) error {
 	certPath := filepath.Join(certDir, "tls.crt")

operator/internal/controller/webhook_controller_test.go

@@ -933,6 +933,33 @@ var _ = Describe("WebhookController", Ordered, func() {
 			Expect(err).NotTo(HaveOccurred())
 			Expect(updated).To(BeFalse())
 		})
+
+		It("should return an error and leave the Secret untouched when the cert cannot be written to disk", func() {
+			diskCert, err := generateCert(serviceName, namespace, certValidityDurationYear)
+			Expect(err).NotTo(HaveOccurred())
+			secretCert, err := generateCert(serviceName, namespace, certValidityDurationYear)
+			Expect(err).NotTo(HaveOccurred())
+
+			secret := secretCert.ToSecret(secretName, namespace, serviceName)
+			controller.Client = fake.NewClientBuilder().WithObjects(secret).Build()
+
+			// Disk holds a different cert than the Secret, so rotation is triggered.
+			err = writeCertAndKey([]byte(diskCert.TLSCert), []byte(diskCert.TLSKey), tmpDir)
+			Expect(err).NotTo(HaveOccurred())
+
+			// Turn tls.key into a directory so the write fails even when tests run as root.
+			keyPath := filepath.Join(tmpDir, "tls.key")
+			Expect(os.Remove(keyPath)).To(Succeed())
+			Expect(os.Mkdir(keyPath, 0o700)).To(Succeed())
+
+			origCert := append([]byte(nil), secret.Data["tls.crt"]...)
+
+			updated, err := controller.CheckOrUpdateWebhookCertSecret(context.Background(), secret)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("writing rotated webhook cert"))
+			Expect(updated).To(BeFalse())
+			Expect(secret.Data["tls.crt"]).To(Equal(origCert))
+		})
 	})
 })