feat(webhook): immutability latches for restoreFrom/migration, mutual exclusion, exactly-one source

Extends HermesInstanceValidator with Plan 5 rules: reject spec.restoreFrom
changes after status.restoredFrom latches, reject spec.migration.fromOpenClaw
changes after status.migration.completed, reject both restoreFrom+migration
set simultaneously, enforce exactly-one of openclawInstanceRef/backupRef, and
emit warnings for unresolvable backup S3 credential secrets and autoUpdate+latest tag.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: stubbi

internal/webhook/webhook_hermesinstance_validate.go

@@ -2,13 +2,15 @@ package webhook
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -35,16 +37,24 @@ func (v *HermesInstanceValidator) ValidateCreate(ctx context.Context, obj runtim
 	if !ok {
 		return nil, fmt.Errorf("expected *HermesInstance, got %T", obj)
 	}
-	warns, err := validateCommon(inst)
+	errs := field.ErrorList{}
+	errs = append(errs, validateRestoreMigrationMutualExclusion(inst)...)
+	errs = append(errs, validateMigrationSourceExactlyOne(inst)...)
+	warnings := v.crossCheckSecrets(ctx, inst)
+	if len(errs) > 0 {
+		return warnings, errs.ToAggregate()
+	}
+	commonWarns, err := validateCommon(inst)
+	warnings = append(warnings, commonWarns...)
 	if err != nil {
-		return warns, err
+		return warnings, err
 	}
 	gwWarns, gwErr := v.validateGateways(ctx, inst)
-	warns = append(warns, gwWarns...)
+	warnings = append(warnings, gwWarns...)
 	if gwErr != nil {
-		return warns, gwErr
+		return warnings, gwErr
 	}
-	return warns, nil
+	return warnings, nil
 }
 
 // ValidateUpdate runs the create rules + immutability rules.
@@ -54,19 +64,28 @@ func (v *HermesInstanceValidator) ValidateUpdate(ctx context.Context, oldObj, ne
 	if !ok1 || !ok2 {
 		return nil, fmt.Errorf("ValidateUpdate types: old=%T new=%T", oldObj, newObj)
 	}
+	errs := field.ErrorList{}
+	errs = append(errs, validateImmutableTerminals(oldI, newI)...)
+	errs = append(errs, validateRestoreMigrationMutualExclusion(newI)...)
+	errs = append(errs, validateMigrationSourceExactlyOne(newI)...)
+	warnings := v.crossCheckSecrets(ctx, newI)
+	if len(errs) > 0 {
+		return warnings, errs.ToAggregate()
+	}
 	if err := validateImmutable(oldI, newI); err != nil {
-		return nil, err
+		return warnings, err
 	}
-	warns, err := validateCommon(newI)
+	commonWarns, err := validateCommon(newI)
+	warnings = append(warnings, commonWarns...)
 	if err != nil {
-		return warns, err
+		return warnings, err
 	}
 	gwWarns, gwErr := v.validateGateways(ctx, newI)
-	warns = append(warns, gwWarns...)
+	warnings = append(warnings, gwWarns...)
 	if gwErr != nil {
-		return warns, gwErr
+		return warnings, gwErr
 	}
-	return warns, nil
+	return warnings, nil
 }
 
 // ValidateDelete is a no-op.
@@ -208,4 +227,86 @@ func validateImmutable(oldI, newI *hermesv1.HermesInstance) error {
 	return nil
 }
 
+// validateImmutableTerminals checks restore + migration terminal latches.
+// `old` is the previous version (nil on create).
+func validateImmutableTerminals(old, updated *hermesv1.HermesInstance) field.ErrorList {
+	var errs field.ErrorList
+	if old == nil {
+		return errs
+	}
+	if old.Status.RestoredFrom != "" && old.Status.RestoredFrom == old.Spec.RestoreFrom &&
+		old.Spec.RestoreFrom != updated.Spec.RestoreFrom {
+		errs = append(errs, field.Forbidden(
+			field.NewPath("spec", "restoreFrom"),
+			fmt.Sprintf("spec.restoreFrom is immutable after status.restoredFrom is set (current: %q). This is intentional to prevent accidental re-restore on restart.", old.Status.RestoredFrom),
+		))
+	}
+	if old.Status.Migration.Completed {
+		if !equalMigration(old.Spec.Migration, updated.Spec.Migration) {
+			errs = append(errs, field.Forbidden(
+				field.NewPath("spec", "migration", "fromOpenClaw"),
+				"spec.migration.fromOpenClaw is immutable after status.migration.completed is true (one-shot migration).",
+			))
+		}
+	}
+	return errs
+}
+
+// validateRestoreMigrationMutualExclusion rejects setting both fields at once.
+func validateRestoreMigrationMutualExclusion(inst *hermesv1.HermesInstance) field.ErrorList {
+	if inst.Spec.RestoreFrom != "" && inst.Spec.Migration.FromOpenClaw != nil {
+		return field.ErrorList{field.Invalid(
+			field.NewPath("spec"),
+			"restoreFrom + migration.fromOpenClaw",
+			"set exactly one of spec.restoreFrom or spec.migration.fromOpenClaw — the combined order of operations is ambiguous (which source wins?). To both restore and migrate, do them as two separate instances.",
+		)}
+	}
+	return nil
+}
+
+// validateMigrationSourceExactlyOne enforces exactly-one of openclawInstanceRef
+// or backupRef under spec.migration.fromOpenClaw.source.
+func validateMigrationSourceExactlyOne(inst *hermesv1.HermesInstance) field.ErrorList {
+	fc := inst.Spec.Migration.FromOpenClaw
+	if fc == nil {
+		return nil
+	}
+	refSet := fc.Source.OpenClawInstanceRef != nil
+	backupSet := fc.Source.BackupRef != nil
+	if refSet == backupSet {
+		return field.ErrorList{field.Invalid(
+			field.NewPath("spec", "migration", "fromOpenClaw", "source"),
+			map[string]bool{"openclawInstanceRef": refSet, "backupRef": backupSet},
+			"set exactly one of source.openclawInstanceRef or source.backupRef",
+		)}
+	}
+	return nil
+}
+
+// equalMigration is a JSON-based structural compare for the migration sub-spec.
+func equalMigration(a, b hermesv1.MigrationSpec) bool {
+	aj, _ := json.Marshal(a)
+	bj, _ := json.Marshal(b)
+	return string(aj) == string(bj)
+}
+
+// crossCheckSecrets emits warnings (never denials) for resolvable references
+// that are likely typos or that signal coming pitfalls (autoUpdate + tag=latest).
+func (v *HermesInstanceValidator) crossCheckSecrets(ctx context.Context, inst *hermesv1.HermesInstance) admission.Warnings {
+	var warnings admission.Warnings
+	if inst.Spec.Backup.S3 != nil && v.Client != nil {
+		name := inst.Spec.Backup.S3.CredentialsSecretRef.Name
+		if name != "" {
+			secret := &corev1.Secret{}
+			if err := v.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: inst.Namespace}, secret); err != nil {
+				warnings = append(warnings, fmt.Sprintf("spec.backup.s3.credentialsSecretRef %q is not resolvable in namespace %q: %v", name, inst.Namespace, err))
+			}
+		}
+	}
+	if inst.Spec.AutoUpdate.Enabled && inst.Spec.Image.Tag == "latest" {
+		warnings = append(warnings, "spec.autoUpdate.enabled with spec.image.tag=\"latest\" — the operator will resolve to a concrete tag, but please pin spec.image.tag for GitOps deterministic apply")
+	}
+	return warnings
+}
+
 var _ = webhook.Admission{}

internal/webhook/webhook_hermesinstance_validate_test.go

@@ -231,3 +231,93 @@ func TestValidateSelfConfigure_UnknownActionDenied(t *testing.T) {
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "reboot-cluster")
 }
+
+func TestValidateRestoreFromImmutableAfterLatch(t *testing.T) {
+	old := &hermesv1.HermesInstance{
+		Spec:   hermesv1.HermesInstanceSpec{RestoreFrom: "k1"},
+		Status: hermesv1.HermesInstanceStatus{RestoredFrom: "k1"},
+	}
+	newer := old.DeepCopy()
+	newer.Spec.RestoreFrom = "k2"
+	errs := validateImmutableTerminals(old, newer)
+	assert.NotEmpty(t, errs)
+	assert.Contains(t, errs[0].Error(), "spec.restoreFrom")
+}
+
+func TestValidateMigrationImmutableAfterCompleted(t *testing.T) {
+	old := &hermesv1.HermesInstance{
+		Spec: hermesv1.HermesInstanceSpec{
+			Migration: hermesv1.MigrationSpec{
+				FromOpenClaw: &hermesv1.MigrationFromOpenClawSpec{
+					Mode: "copy",
+					Source: hermesv1.MigrationFromOpenClawSource{
+						OpenClawInstanceRef: &hermesv1.NamespacedObjectReference{Name: "x", Namespace: "y"},
+					},
+				},
+			},
+		},
+		Status: hermesv1.HermesInstanceStatus{Migration: hermesv1.MigrationStatus{Completed: true}},
+	}
+	newer := old.DeepCopy()
+	newer.Spec.Migration.FromOpenClaw.Mode = "move"
+	errs := validateImmutableTerminals(old, newer)
+	assert.NotEmpty(t, errs)
+	assert.Contains(t, errs[0].Error(), "migration")
+}
+
+func TestValidateMutualExclusion(t *testing.T) {
+	inst := &hermesv1.HermesInstance{
+		Spec: hermesv1.HermesInstanceSpec{
+			RestoreFrom: "k1",
+			Migration: hermesv1.MigrationSpec{
+				FromOpenClaw: &hermesv1.MigrationFromOpenClawSpec{
+					Source: hermesv1.MigrationFromOpenClawSource{
+						OpenClawInstanceRef: &hermesv1.NamespacedObjectReference{Name: "x", Namespace: "y"},
+					},
+				},
+			},
+		},
+	}
+	errs := validateRestoreMigrationMutualExclusion(inst)
+	assert.NotEmpty(t, errs)
+}
+
+func TestValidateMigrationSourceExactlyOne(t *testing.T) {
+	both := &hermesv1.HermesInstance{
+		Spec: hermesv1.HermesInstanceSpec{
+			Migration: hermesv1.MigrationSpec{
+				FromOpenClaw: &hermesv1.MigrationFromOpenClawSpec{
+					Source: hermesv1.MigrationFromOpenClawSource{
+						OpenClawInstanceRef: &hermesv1.NamespacedObjectReference{Name: "x", Namespace: "y"},
+						BackupRef:           &hermesv1.MigrationBackupRef{S3: hermesv1.MigrationBackupS3{Bucket: "b", Key: "k", Endpoint: "e", CredentialsSecretRef: hermesv1.LocalObjectReference{Name: "s"}}},
+					},
+				},
+			},
+		},
+	}
+	assert.NotEmpty(t, validateMigrationSourceExactlyOne(both))
+
+	neither := &hermesv1.HermesInstance{
+		Spec: hermesv1.HermesInstanceSpec{
+			Migration: hermesv1.MigrationSpec{
+				FromOpenClaw: &hermesv1.MigrationFromOpenClawSpec{
+					Source: hermesv1.MigrationFromOpenClawSource{},
+				},
+			},
+		},
+	}
+	assert.NotEmpty(t, validateMigrationSourceExactlyOne(neither))
+
+	one := &hermesv1.HermesInstance{
+		Spec: hermesv1.HermesInstanceSpec{
+			Migration: hermesv1.MigrationSpec{
+				FromOpenClaw: &hermesv1.MigrationFromOpenClawSpec{
+					Source: hermesv1.MigrationFromOpenClawSource{
+						OpenClawInstanceRef: &hermesv1.NamespacedObjectReference{Name: "x", Namespace: "y"},
+					},
+				},
+			},
+		},
+	}
+	assert.Empty(t, validateMigrationSourceExactlyOne(one))
+}