refactor: bake FreeRADIUS reload into all radius write functions

WriteRadiusConfig, WriteRadSecTLS, WriteRadSecServerCert, and
WriteStatusConfig now accept a deployment name and call Reload
internally whenever they change content. Callers no longer need
a separate Reload call, and the cert-watcher's redundant Reload
after WriteRadSecServerCert is removed.

Author: mbillow

cmd/pint/main.go

@@ -87,27 +87,13 @@ func main() {
 		log.Fatal("ensure status secret failed", zap.Error(err))
 	}
 	statusConf := radius.RenderStatusConfig(statusSecret, "0.0.0.0/0")
-	updated, err := radius.WriteStatusConfig(context.Background(), k8sClient, cfg.Namespace, cfg.ConfigSecret, statusConf)
-	if err != nil {
+	if err := radius.WriteStatusConfig(context.Background(), k8sClient, cfg.Namespace, cfg.ConfigSecret, cfg.FreeRADIUSDeployment, statusConf); err != nil {
 		log.Fatal("write status config failed", zap.Error(err))
 	}
-	if updated {
-		log.Info("updated FreeRADIUS status config, triggering rollout restart")
-		if err := radius.Reload(context.Background(), k8sClient, cfg.Namespace, cfg.FreeRADIUSDeployment); err != nil {
-			log.Warn("status config reload failed", zap.Error(err))
-		}
-	}
 
-	tlsUpdated, err := radius.WriteRadSecTLS(context.Background(), k8sClient, cfg.Namespace, cfg.ConfigSecret, cfg.RadSecCheckCRL, cfg.RadSecProxyProtocol)
-	if err != nil {
+	if err := radius.WriteRadSecTLS(context.Background(), k8sClient, cfg.Namespace, cfg.ConfigSecret, cfg.FreeRADIUSDeployment, cfg.RadSecCheckCRL, cfg.RadSecProxyProtocol); err != nil {
 		log.Fatal("write radsec-tls.conf failed", zap.Error(err))
 	}
-	if tlsUpdated {
-		log.Info("updated radsec-tls.conf, triggering rollout restart", zap.Bool("check_crl", cfg.RadSecCheckCRL))
-		if err := radius.Reload(context.Background(), k8sClient, cfg.Namespace, cfg.FreeRADIUSDeployment); err != nil {
-			log.Warn("radsec tls config reload failed", zap.Error(err))
-		}
-	}
 
 	// Fetch CA certs in parallel. Code-signing CA is fetched only when configured.
 	var (
@@ -328,13 +314,10 @@ func loadOrRenewRadSecServerCert(ctx context.Context, log *zap.Logger, k8sClient
 							zap.Int("stored_bytes", len(stored)),
 							zap.Int("expected_bytes", len(wifiCAPEM)),
 						)
-						if patchErr := radius.PatchSecretKey(ctx, k8sClient, cfg.Namespace, cfg.RadSecCertSecret, "wifi-ca.pem", wifiCAPEM); patchErr != nil {
-							log.Error("failed to update wifi-ca.pem in secret", zap.Error(patchErr))
+						if writeErr := radius.WriteRadSecServerCert(ctx, k8sClient, cfg.Namespace, cfg.RadSecCertSecret, cfg.FreeRADIUSDeployment, existing, key, caPEM, wifiCAPEM); writeErr != nil {
+							log.Error("failed to update wifi-ca.pem in secret", zap.Error(writeErr))
 						} else {
 							log.Info("updated wifi-ca.pem in secret, triggering FreeRADIUS rollout restart")
-							if reloadErr := radius.Reload(ctx, k8sClient, cfg.Namespace, cfg.FreeRADIUSDeployment); reloadErr != nil {
-								log.Warn("FreeRADIUS rollout restart failed after wifi-ca.pem update", zap.Error(reloadErr))
-							}
 						}
 					}
 
@@ -362,7 +345,7 @@ func loadOrRenewRadSecServerCert(ctx context.Context, log *zap.Logger, k8sClient
 	}
 	newKeyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: ecKeyBytes})
 
-	if writeErr := radius.WriteRadSecServerCert(ctx, k8sClient, cfg.Namespace, cfg.RadSecCertSecret, newCertPEM, newKeyPEM, caPEM, wifiCAPEM); writeErr != nil {
+	if writeErr := radius.WriteRadSecServerCert(ctx, k8sClient, cfg.Namespace, cfg.RadSecCertSecret, cfg.FreeRADIUSDeployment, newCertPEM, newKeyPEM, caPEM, wifiCAPEM); writeErr != nil {
 		return nil, nil, false, fmt.Errorf("write radsec cert: %w", writeErr)
 	}
 	log.Info("issued and stored new RadSec server cert")
@@ -383,11 +366,7 @@ func watchRadSecServerCert(log *zap.Logger, k8sClient kubernetes.Interface, ipaC
 			continue
 		}
 		if renewed {
-			if err := radius.Reload(ctx, k8sClient, cfg.Namespace, cfg.FreeRADIUSDeployment); err != nil {
-				log.Error("radsec cert watcher: freeradius reload failed", zap.Error(err))
-			} else {
-				log.Info("radsec cert watcher: renewed cert and reloaded freeradius")
-			}
+			log.Info("radsec cert watcher: renewed cert and reloaded freeradius")
 		}
 	}
 }

internal/handlers/radius.go

@@ -206,15 +206,11 @@ func commitStore(c *gin.Context, log *zap.Logger, store *radius.ClientStore, k8s
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return "", err
 	}
-	if err := radius.WriteRadiusConfig(ctx, k8s, cfg.Namespace, cfg.ConfigSecret, store.All()); err != nil {
-		log.Error("radius config write failed", zap.Error(err))
+	if err := radius.WriteRadiusConfig(ctx, k8s, cfg.Namespace, cfg.ConfigSecret, cfg.FreeRADIUSDeployment, store.All()); err != nil {
+		log.Error("radius config write/reload failed", zap.Error(err))
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return "", err
 	}
-	if err := radius.Reload(ctx, k8s, cfg.Namespace, cfg.FreeRADIUSDeployment); err != nil {
-		log.Warn("freeradius reload failed", zap.Error(err))
-		return err.Error(), nil
-	}
 	log.Debug("freeradius reloaded")
 	return "", nil
 }

internal/radius/reload.go

@@ -23,37 +23,47 @@ const (
 	KeyRadSecTLS    = "radsec-tls.conf"
 )
 
-// WriteRadiusConfig renders clients.conf from the given client list and patches
-// the clients.conf key in the named Kubernetes Secret.
-func WriteRadiusConfig(ctx context.Context, k8s kubernetes.Interface, namespace, secretName string, clients []RadiusClient) error {
-	return patchSecretKey(ctx, k8s, namespace, secretName, KeyClientsConf, []byte(RenderClientsConf(clients)))
+// WriteRadiusConfig renders clients.conf from the given client list, patches the
+// key in the named Kubernetes Secret, and triggers a FreeRADIUS rollout restart.
+func WriteRadiusConfig(ctx context.Context, k8s kubernetes.Interface, namespace, secretName, deployment string, clients []RadiusClient) error {
+	if err := patchSecretKey(ctx, k8s, namespace, secretName, KeyClientsConf, []byte(RenderClientsConf(clients))); err != nil {
+		return err
+	}
+	return Reload(ctx, k8s, namespace, deployment)
 }
 
 // WriteRadSecTLS renders and patches the radsec-tls.conf key in the named K8s Secret.
-// Returns (didUpdate, err). didUpdate is false when the existing value is identical.
-func WriteRadSecTLS(ctx context.Context, k8s kubernetes.Interface, namespace, secretName string, checkCRL, proxyProtocol bool) (bool, error) {
+// If the content changed, FreeRADIUS is reloaded. No-op (no reload) when unchanged.
+func WriteRadSecTLS(ctx context.Context, k8s kubernetes.Interface, namespace, secretName, deployment string, checkCRL, proxyProtocol bool) error {
 	rendered := RenderRadSecTLS(checkCRL, proxyProtocol)
 	existing, err := k8s.CoreV1().Secrets(namespace).Get(ctx, secretName, metav1.GetOptions{})
 	if err == nil && string(existing.Data[KeyRadSecTLS]) == rendered {
-		return false, nil
+		return nil
 	}
-	return true, patchSecretKey(ctx, k8s, namespace, secretName, KeyRadSecTLS, []byte(rendered))
+	if err := patchSecretKey(ctx, k8s, namespace, secretName, KeyRadSecTLS, []byte(rendered)); err != nil {
+		return err
+	}
+	return Reload(ctx, k8s, namespace, deployment)
 }
 
-// WriteRadSecServerCert writes all FreeRADIUS TLS material to the named K8s Secret:
+// WriteRadSecServerCert writes all FreeRADIUS TLS material to the named K8s Secret
+// and triggers a FreeRADIUS rollout restart:
 //   - tls.crt / tls.key: server cert presented to RadSec clients and EAP supplicants
 //   - ca.pem: RadSec CA chain; verifies connecting router client certificates
-//   - wifi-ca.pem: WiFi CA cert; verifies EAP-TLS user certificates
-func WriteRadSecServerCert(ctx context.Context, k8s kubernetes.Interface, namespace, secretName string, certPEM, keyPEM, caPEM, wifiCAPEM []byte) error {
-	return UpsertSecret(ctx, k8s, &corev1.Secret{
+//   - wifi-ca.pem: WiFi CA chain; verifies EAP-TLS user client certificates
+func WriteRadSecServerCert(ctx context.Context, k8s kubernetes.Interface, namespace, secretName, deployment string, certPEM, keyPEM, caPEM, wifiCAPEM []byte) error {
+	if err := UpsertSecret(ctx, k8s, &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: namespace},
 		Data: map[string][]byte{
 			"tls.crt":     certPEM,
 			"tls.key":     keyPEM,
 			"ca.pem":      caPEM,
 			"wifi-ca.pem": wifiCAPEM,
 		},
-	})
+	}); err != nil {
+		return err
+	}
+	return Reload(ctx, k8s, namespace, deployment)
 }
 
 // EnsureConfigSecret creates the combined PINT config secret with all keys
@@ -72,13 +82,6 @@ func EnsureConfigSecret(ctx context.Context, k8s kubernetes.Interface, namespace
 	})
 }
 
-// patchSecretKey updates a single key without touching the rest of the secret,
-// avoiding races with other components that may patch different keys concurrently.
-// PatchSecretKey updates a single key in an existing K8s Secret via a merge patch.
-func PatchSecretKey(ctx context.Context, k8s kubernetes.Interface, namespace, secretName, key string, value []byte) error {
-	return patchSecretKey(ctx, k8s, namespace, secretName, key, value)
-}
-
 func patchSecretKey(ctx context.Context, k8s kubernetes.Interface, namespace, secretName, key string, value []byte) error {
 	p, err := json.Marshal(map[string]interface{}{
 		"data": map[string][]byte{key: value},
@@ -110,7 +113,11 @@ func createIfAbsent(ctx context.Context, k8s kubernetes.Interface, secret *corev
 
 // Reload triggers a rollout restart of the FreeRADIUS deployment by patching
 // the pod template annotation, equivalent to kubectl rollout restart.
+// A no-op when deployment is empty (e.g. FreeRADIUS is disabled).
 func Reload(ctx context.Context, k8s kubernetes.Interface, namespace, deployment string) error {
+	if deployment == "" {
+		return nil
+	}
 	patch := fmt.Sprintf(
 		`{"spec":{"template":{"metadata":{"annotations":{"kubectl.kubernetes.io/restartedAt":%q}}}}}`,
 		time.Now().Format(time.RFC3339),

internal/radius/reload_test.go

@@ -34,7 +34,7 @@ func TestWriteRadiusConfig(t *testing.T) {
 		{Username: "mbillow", IPCIDR: nil},
 	}
 
-	if err := radius.WriteRadiusConfig(ctx, k8s, "default", "pint-config", clients); err != nil {
+	if err := radius.WriteRadiusConfig(ctx, k8s, "default", "pint-config", "", clients); err != nil {
 		t.Fatalf("WriteRadiusConfig() error: %v", err)
 	}
 
@@ -60,7 +60,7 @@ func TestWriteRadSecServerCert(t *testing.T) {
 	caPEM := []byte("-----BEGIN CERTIFICATE-----\nfake-ca\n-----END CERTIFICATE-----\n")
 	wifiCAPEM := []byte("-----BEGIN CERTIFICATE-----\nfake-wifi-ca\n-----END CERTIFICATE-----\n")
 
-	if err := radius.WriteRadSecServerCert(ctx, k8s, "default", "pint-radsec-server", certPEM, keyPEM, caPEM, wifiCAPEM); err != nil {
+	if err := radius.WriteRadSecServerCert(ctx, k8s, "default", "pint-radsec-server", "", certPEM, keyPEM, caPEM, wifiCAPEM); err != nil {
 		t.Fatalf("WriteRadSecServerCert() error: %v", err)
 	}
 
@@ -82,7 +82,7 @@ func TestWriteRadSecServerCert(t *testing.T) {
 	}
 
 	// Call again to exercise the update path
-	if err := radius.WriteRadSecServerCert(ctx, k8s, "default", "pint-radsec-server", certPEM, keyPEM, caPEM, wifiCAPEM); err != nil {
+	if err := radius.WriteRadSecServerCert(ctx, k8s, "default", "pint-radsec-server", "", certPEM, keyPEM, caPEM, wifiCAPEM); err != nil {
 		t.Fatalf("WriteRadSecServerCert() update error: %v", err)
 	}
 }
@@ -91,13 +91,9 @@ func TestWriteRadSecTLS_WritesAndDetectsChanges(t *testing.T) {
 	ctx := context.Background()
 	k8s := fake.NewSimpleClientset(newConfigSecret("default", "pint-config"))
 
-	updated, err := radius.WriteRadSecTLS(ctx, k8s, "default", "pint-config", false, false)
-	if err != nil {
+	if err := radius.WriteRadSecTLS(ctx, k8s, "default", "pint-config", "", false, false); err != nil {
 		t.Fatalf("WriteRadSecTLS() error: %v", err)
 	}
-	if !updated {
-		t.Error("expected updated=true on first write")
-	}
 
 	secret, err := k8s.CoreV1().Secrets("default").Get(ctx, "pint-config", metav1.GetOptions{})
 	if err != nil {
@@ -107,14 +103,10 @@ func TestWriteRadSecTLS_WritesAndDetectsChanges(t *testing.T) {
 		t.Error("expected check_crl = no in radsec-tls.conf")
 	}
 
-	// Second write with same value should not report updated.
-	updated, err = radius.WriteRadSecTLS(ctx, k8s, "default", "pint-config", false, false)
-	if err != nil {
+	// Second write with same value should be a no-op (no error).
+	if err := radius.WriteRadSecTLS(ctx, k8s, "default", "pint-config", "", false, false); err != nil {
 		t.Fatalf("WriteRadSecTLS() second call error: %v", err)
 	}
-	if updated {
-		t.Error("expected updated=false when config unchanged")
-	}
 }
 
 func TestReload_DeploymentNotFound(t *testing.T) {

internal/radius/statusconfig.go

@@ -43,11 +43,14 @@ func RenderStatusConfig(secret, cidr string) string {
 }
 
 // WriteStatusConfig patches the status key in the named Secret with the rendered config.
-// Returns (didUpdate, err). didUpdate is false when the existing value is identical.
-func WriteStatusConfig(ctx context.Context, k8s kubernetes.Interface, namespace, secretName, config string) (bool, error) {
+// If the content changed, FreeRADIUS is reloaded. No-op (no reload) when unchanged.
+func WriteStatusConfig(ctx context.Context, k8s kubernetes.Interface, namespace, secretName, deployment, config string) error {
 	existing, err := k8s.CoreV1().Secrets(namespace).Get(ctx, secretName, metav1.GetOptions{})
 	if err == nil && string(existing.Data[KeyStatus]) == config {
-		return false, nil
+		return nil
 	}
-	return true, patchSecretKey(ctx, k8s, namespace, secretName, KeyStatus, []byte(config))
+	if err := patchSecretKey(ctx, k8s, namespace, secretName, KeyStatus, []byte(config)); err != nil {
+		return err
+	}
+	return Reload(ctx, k8s, namespace, deployment)
 }