fix: heap sizing and signing policy changes without a CA restart (#592, #588)

README.md

@@ -17,7 +17,7 @@ A Kubernetes Operator that maps [OpenVox Server](https://github.com/OpenVoxProje
 - 🔄 **Multi-Version Deployments** - Run different server versions side by side - canary deployments, rolling upgrades
 - 🔒 **Rootless & OpenShift Ready** - Random UID compatible, no root, no ezbake, no privilege escalation
 - 🪶 **Minimal Image** - UBI9-based, no agent Ruby, no ezbake packaging - smaller footprint, fewer updates
-- 🧠 **JVM sizing** - Set `javaArgs` per Server or Database; see [Server](docs/reference/server.md) for the current default
+- 🧠 **Auto-tuned JVM** - Heap derived from the pod memory limit (90%) unless `javaArgs` is set
 - 📦 **OCI Image Volumes** - Package Puppet code as OCI images, deploy immutably with automatic rollout (K8s 1.35+)
 - 🌐 **Gateway API** - SNI-based TLSRoute support - share a single LoadBalancer across environments via TLS passthrough
 - 🗄️ **Managed OpenVox DB** - Deploy OpenVox DB (PuppetDB) with external PostgreSQL - TLS, config, and credentials managed by the operator

api/v1alpha1/javaargs_default_test.go

@@ -0,0 +1,36 @@
+package v1alpha1
+
+import (
+	"context"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// TestServerJavaArgsHasNoDefault is the test that was missing while the bug
+// existed. The unit tests around resolveJavaArgs all call the function
+// directly and therefore pass whether or not the field can ever be empty; only
+// a round-trip through the API server shows that.
+//
+// A default here is not cosmetic: the controller derives the heap from the
+// pod's memory limit exactly when javaArgs is empty, so a default silently
+// pins every Server to the same heap.
+func TestServerJavaArgsHasNoDefault(t *testing.T) {
+	ctx := context.Background()
+
+	server := &Server{
+		ObjectMeta: metav1.ObjectMeta{GenerateName: "test-server-", Namespace: "default"},
+		Spec: ServerSpec{
+			ConfigRef:      "production",
+			CertificateRef: "production-cert",
+		},
+	}
+	if err := k8sClient.Create(ctx, server); err != nil {
+		t.Fatalf("creating Server: %v", err)
+	}
+	t.Cleanup(func() { _ = k8sClient.Delete(ctx, server) })
+
+	if server.Spec.JavaArgs != "" {
+		t.Errorf("javaArgs must come back empty so the heap can be derived, got %q", server.Spec.JavaArgs)
+	}
+}

api/v1alpha1/server_types.go

@@ -82,7 +82,11 @@ type ServerSpec struct {
 	Resources corev1.ResourceRequirements `json:"resources,omitempty"`
 
 	// JavaArgs defines the JVM arguments.
-	// +kubebuilder:default="-Xms512m -Xmx1024m"
+	//
+	// There is deliberately no default. A defaulted field is never empty, and
+	// the controller uses emptiness to decide whether to derive the heap from
+	// the pod's memory limit. With a default in place that derivation is dead
+	// code and every Server runs on the same heap regardless of its limit.
 	// +optional
 	JavaArgs string `json:"javaArgs,omitempty"`
 

charts/openvox-operator/crds/openvox.voxpupuli.org_servers.yaml

@@ -3428,8 +3428,13 @@ spec:
                     type: string
                 type: object
               javaArgs:
-                default: -Xms512m -Xmx1024m
-                description: JavaArgs defines the JVM arguments.
+                description: |-
+                  JavaArgs defines the JVM arguments.
+
+                  There is deliberately no default. A defaulted field is never empty, and
+                  the controller uses emptiness to decide whether to derive the heap from
+                  the pod's memory limit. With a default in place that derivation is dead
+                  code and every Server runs on the same heap regardless of its limit.
                 type: string
               maxActiveInstances:
                 default: 1

config/crd/bases/openvox.voxpupuli.org_servers.yaml

@@ -3428,8 +3428,13 @@ spec:
                     type: string
                 type: object
               javaArgs:
-                default: -Xms512m -Xmx1024m
-                description: JavaArgs defines the JVM arguments.
+                description: |-
+                  JavaArgs defines the JVM arguments.
+
+                  There is deliberately no default. A defaulted field is never empty, and
+                  the controller uses emptiness to decide whether to derive the heap from
+                  the pod's memory limit. With a default in place that derivation is dead
+                  code and every Server runs on the same heap regardless of its limit.
                 type: string
               maxActiveInstances:
                 default: 1

docs/_snippets/features.md

@@ -6,7 +6,7 @@
 - 🔄 **Multi-Version Deployments** - Run different server versions side by side - canary deployments, rolling upgrades
 - 🔒 **Rootless & OpenShift Ready** - Random UID compatible, no root, no ezbake, no privilege escalation
 - 🪶 **Minimal Image** - UBI9-based, no agent Ruby, no ezbake packaging - smaller footprint, fewer updates
-- 🧠 **JVM sizing** - Set `javaArgs` per Server or Database; see [Server](reference/server.md) for the current default
+- 🧠 **Auto-tuned JVM** - Heap derived from the pod memory limit (90%) unless `javaArgs` is set
 - 📦 **OCI Image Volumes** - Package Puppet code as OCI images, deploy immutably with automatic rollout (K8s 1.35+)
 - 🌐 **Gateway API** - SNI-based TLSRoute support - share a single LoadBalancer across environments via TLS passthrough
 - 🗄️ **Managed OpenVox DB** - Deploy OpenVox DB (PuppetDB) with external PostgreSQL - TLS, config, and credentials managed by the operator

docs/concepts/config-rollout.md

@@ -15,7 +15,6 @@ Tracked annotations:
 | `openvox.voxpupuli.org/ca-secret-hash` | CA Secret (`{ca}-ca`) | Yes |
 | `openvox.voxpupuli.org/enc-secret-hash` | ENC Secret (`{config}-enc`) | Yes |
 | `openvox.voxpupuli.org/report-webhook-secret-hash` | Report webhook Secret (`{config}-report-webhook`) | Yes |
-| `openvox.voxpupuli.org/autosign-policy-secret-hash` | Autosign policy Secret (`{ca}-autosign-policy`, CA pods only) | Yes |
 | `openvox.voxpupuli.org/code-image` | Code OCI image reference | Yes |
 
 ## What Triggers a Restart
@@ -88,8 +87,20 @@ Creating or updating a SigningPolicy:
 
 1. Config controller is triggered via SigningPolicy watcher
 2. Autosign policy Secret (`{ca}-autosign-policy`) is updated
-3. Server controller detects the updated `autosign-policy-secret-hash` annotation
-4. CA pod is recreated so `openvox-autosign` reads the new policy on the next CSR (no manual restart)
+3. The kubelet syncs the updated Secret into the CA pod, which mounts it as a
+   directory rather than through `subPath`
+4. `openvox-autosign` reads the file on the next CSR
+
+**No restart is involved.** The policy Secret is deliberately the one Secret
+that does not roll its pod. A `subPath` mount is never refreshed by the
+kubelet, which is why this used to need a restart - and because the CA
+Deployment uses the `Recreate` strategy, that restart meant a short outage with
+no signing and no CRL, for a change that alters no running state.
+
+The trade-off is timing: a policy edit takes effect within the kubelet sync
+period, up to about a minute, instead of immediately after a restart. For a
+rule that governs which CSRs get signed, waiting a minute is preferable to
+dropping the CA.
 
 ### Changing ENC Configuration
 

docs/reference/server.md

@@ -33,7 +33,7 @@ spec:
 | `replicas` | int32 | `1` | Number of pod replicas |
 | `autoscaling` | [AutoscalingSpec](#autoscalingspec) | - | HPA configuration |
 | `resources` | ResourceRequirements | - | CPU/memory requests and limits |
-| `javaArgs` | string | `-Xms512m -Xmx1024m` | JVM arguments. The controller can derive the heap from the memory limit, but the CRD default makes that path unreachable today - see [#592](https://github.com/slauger/openvox-operator/issues/592). Set this explicitly to size the heap |
+| `javaArgs` | string | *(derived)* | JVM arguments. Unset derives the heap from the pod's memory limit (90%), falling back to `-Xms512m -Xmx1024m` when no limit is set |
 | `maxActiveInstances` | int32 | `1` | Number of JRuby instances per pod |
 | `code` | [[]CodeSpec](index.md#codespec) | - | Override the Config's code sources (replace, not merge). A list; see [CodeSpec](index.md#codespec) |
 | `topologySpreadConstraints` | []TopologySpreadConstraint | - | Pod spread constraints across topology domains |

internal/controller/config_autosign.go

@@ -20,6 +20,15 @@ import (
 
 const autosignBinaryPath = "/usr/local/bin/openvox-autosign"
 
+// autosignPolicyDir is where the rendered policy Secret is mounted. It is a
+// directory so the kubelet keeps it in sync; see the mount in
+// server_deployment.go.
+const autosignPolicyDir = "/etc/puppetlabs/puppet/autosign-policy"
+
+// autosignPolicyPath is the file inside that directory, passed to the binary
+// with --config.
+const autosignPolicyPath = autosignPolicyDir + "/autosign-policy.yaml"
+
 // findSigningPolicies returns all SigningPolicies referencing the given CA.
 //
 // A list error is returned rather than swallowed: an empty policy set renders

internal/controller/config_controller_test.go

@@ -212,6 +212,12 @@ func TestConfigReconcile_PuppetConfWithCA(t *testing.T) {
 	if !strings.Contains(puppetConf, "autosign = ") {
 		t.Errorf("puppet.conf missing autosign\n---\n%s", puppetConf)
 	}
+	// The policy lives in a directory mount, so the binary has to be told where
+	// to look. A wrong path here denies every CSR and would otherwise surface
+	// only in an end-to-end run.
+	if !strings.Contains(puppetConf, "--config "+autosignPolicyPath) {
+		t.Errorf("puppet.conf must point the binary at %s\n---\n%s", autosignPolicyPath, puppetConf)
+	}
 }
 
 func TestConfigReconcile_PuppetConfWithENC(t *testing.T) {

internal/controller/config_rendering.go

@@ -76,12 +76,12 @@ func (r *ConfigReconciler) renderPuppetConf(ctx context.Context, cfg *openvoxv1a
 		}
 
 		// Autosign: by default point to the built-in binary, which reads the policy
-		// Secret (mounted by the server controller) and decides sign/deny. A policy
-		// change rewrites the Secret and the server controller rolls the CA pod via
-		// the autosign-policy-secret-hash annotation, so it applies without a manual
-		// restart. A custom autosignCommand replaces the built-in binary and disables
-		// the SigningPolicy-driven flow (the policy Secret is not mounted).
-		autosignCmd := autosignBinaryPath
+		// Secret (mounted by the server controller as a directory) and decides
+		// sign/deny. The binary re-reads the file on every CSR and the kubelet keeps
+		// the mount in sync, so a policy change applies without restarting the CA.
+		// A custom autosignCommand replaces the built-in binary and disables the
+		// SigningPolicy-driven flow (the policy Secret is not mounted).
+		autosignCmd := fmt.Sprintf("%s --config %s", autosignBinaryPath, autosignPolicyPath)
 		if cfg.Spec.Puppet.AutosignCommand != "" {
 			autosignCmd = cfg.Spec.Puppet.AutosignCommand
 		}

internal/controller/server_controller_test.go

@@ -6,6 +6,7 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
+	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	policyv1 "k8s.io/api/policy/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -180,7 +181,12 @@ func TestServerReconcile_AnnotationHashes(t *testing.T) {
 	}
 }
 
-func TestServerReconcile_AutosignPolicyHashAnnotation(t *testing.T) {
+// TestServerReconcile_AutosignPolicyIsLiveMounted replaces the former hash
+// annotation test. A policy edit must not roll the CA pod: the CA Deployment
+// uses the Recreate strategy, so a restart is a short outage with no signing
+// and no CRL - for a change that alters no running state. The binary re-reads
+// the file on every CSR, so nothing downstream needs the restart either.
+func TestServerReconcile_AutosignPolicyIsLiveMounted(t *testing.T) {
 	objs := append(serverPrereqs(),
 		newSecret("production-ca-autosign-policy", map[string][]byte{
 			"autosign-policy.yaml": []byte("policies:\n"),
@@ -199,9 +205,26 @@ func TestServerReconcile_AutosignPolicyHashAnnotation(t *testing.T) {
 		t.Fatalf("Deployment not found: %v", err)
 	}
 
-	// The CA pod must carry the autosign-policy hash so a SigningPolicy change rolls it.
-	if v, ok := deploy.Spec.Template.Annotations["openvox.voxpupuli.org/autosign-policy-secret-hash"]; !ok || v == "" {
-		t.Error("CA pod should carry the autosign-policy-secret-hash annotation")
+	if _, ok := deploy.Spec.Template.Annotations["openvox.voxpupuli.org/autosign-policy-secret-hash"]; ok {
+		t.Error("the policy hash must not be in the pod template any more, it would roll the CA on every edit")
+	}
+
+	// A SubPath mount is never refreshed by the kubelet, which is what forced
+	// the restart. The mount has to stay a directory for the sync to happen.
+	var mount *corev1.VolumeMount
+	for i := range deploy.Spec.Template.Spec.Containers[0].VolumeMounts {
+		if deploy.Spec.Template.Spec.Containers[0].VolumeMounts[i].Name == "autosign-policy" {
+			mount = &deploy.Spec.Template.Spec.Containers[0].VolumeMounts[i]
+		}
+	}
+	if mount == nil {
+		t.Fatal("the CA pod must mount the autosign policy")
+	}
+	if mount.SubPath != "" {
+		t.Errorf("the policy must not be mounted with SubPath, got %q", mount.SubPath)
+	}
+	if mount.MountPath != autosignPolicyDir {
+		t.Errorf("expected the policy directory %q, got %q", autosignPolicyDir, mount.MountPath)
 	}
 }
 

internal/controller/server_deployment.go

@@ -118,14 +118,6 @@ func (r *ServerReconciler) reconcileDeployment(ctx context.Context, server *open
 	// SigningPolicy changes. The policy Secret is subPath-mounted, so kubelet does not
 	// live-sync it; hashing it into the pod template rolls the CA pod when the rendered
 	// policy changes, so a SigningPolicy edit applies without a manual restart.
-	// Skipped when a custom autosignCommand disables the SigningPolicy-driven flow.
-	if server.Spec.CA && cfg.Spec.Puppet.AutosignCommand == "" {
-		autosignSecretName := fmt.Sprintf("%s-autosign-policy", ca.Name)
-		if autosignHash, err := r.secretHash(ctx, autosignSecretName, server.Namespace); err == nil {
-			annotations["openvox.voxpupuli.org/autosign-policy-secret-hash"] = autosignHash
-		}
-	}
-
 	deploy := &appsv1.Deployment{}
 	err = r.Get(ctx, types.NamespacedName{Name: deployName, Namespace: server.Namespace}, deploy)
 	if errors.IsNotFound(err) {
@@ -312,10 +304,15 @@ func (r *ServerReconciler) buildPodSpec(server *openvoxv1alpha1.Server, cfg *ope
 		// flow: the command is then responsible for its own signing decision.
 		if cfg.Spec.Puppet.AutosignCommand == "" {
 			autosignSecretName := fmt.Sprintf("%s-autosign-policy", ca.Name)
+			// Mounted as a directory rather than with SubPath: a SubPath mount is
+			// never refreshed by the kubelet, which is why this used to need a
+			// pod restart on every policy change. The CA Deployment uses the
+			// Recreate strategy, so that restart was a short CA outage - no
+			// signing, no CRL - for an edit that changes no running state. The
+			// CRL Secret is mounted the same way for the same reason.
 			volumeMounts = append(volumeMounts, corev1.VolumeMount{
 				Name:      "autosign-policy",
-				MountPath: "/etc/puppetlabs/puppet/autosign-policy.yaml",
-				SubPath:   "autosign-policy.yaml",
+				MountPath: autosignPolicyDir,
 				ReadOnly:  true,
 			})
 			volumes = append(volumes, corev1.Volume{