feat(defaults): implement hybrid defaulting strategy with deep resource validation

This commit introduces a robust defaulting mechanism that strictly separates
Admission (Webhook) concerns from Reconciliation (Controller) concerns, aligned
with the v1alpha1 design.

Key Changes:
- **pkg/defaults**:
  - Added `constants.go` with hardcoded safety defaults for Resources (CPU/Mem), Storage (1Gi), and Images (using GHCR main).
  - Implemented `PopulateClusterDefaults` in `resolver.go` for the Webhook. This applies strictly *inline* defaults and static values without fetching external templates, adhering to the design's "Non-Goal".
  - Implemented `Resolve...` methods (e.g., `ResolveGlobalTopo`) for the Controller. These actively fetch templates, merge them with inline specs, and apply "Level 4" operator defaults to guarantee valid, runnable configs (preventing nil-pointer crashes).
  - Added `resolver_test.go` with 100% coverage, using table-driven tests and `go-cmp` to verify deep defaulting logic and error wrapping.

- **pkg/cluster-handler**:
  - Updated `multigrescluster_controller.go` to call `PopulateClusterDefaults` at the start of reconciliation. This ensures the controller operates on a valid state even if the admission webhook is disabled.
  - Simplified reconciliation logic by relying on the Resolver's guarantee of non-nil resource specs.
  - Updated all tests to ensure they are 100% coverage.

Author: fernando-villalba

pkg/cluster-handler/controller/multigrescluster/integration_test.go

@@ -19,6 +19,7 @@ import (
 
 	multigresv1alpha1 "github.com/numtide/multigres-operator/api/v1alpha1"
 	"github.com/numtide/multigres-operator/pkg/cluster-handler/controller/multigrescluster"
+	"github.com/numtide/multigres-operator/pkg/defaults"
 	"github.com/numtide/multigres-operator/pkg/testutil"
 )
 
@@ -124,8 +125,10 @@ func TestMultigresClusterReconciliation(t *testing.T) {
 					},
 					Spec: multigresv1alpha1.TopoServerSpec{
 						Etcd: &multigresv1alpha1.EtcdSpec{
-							Image:    "etcd:latest",
-							Replicas: ptr.To(int32(3)), // Default from logic
+							Image:     "etcd:latest",
+							Replicas:  ptr.To(int32(3)), // Default from logic
+							Storage:   multigresv1alpha1.StorageSpec{Size: defaults.DefaultEtcdStorageSize},
+							Resources: defaults.DefaultResourcesEtcd,
 						},
 					},
 				},
@@ -149,8 +152,9 @@ func TestMultigresClusterReconciliation(t *testing.T) {
 							Spec: corev1.PodSpec{
 								Containers: []corev1.Container{
 									{
-										Name:  "multiadmin",
-										Image: "admin:latest",
+										Name:      "multiadmin",
+										Image:     "admin:latest",
+										Resources: defaults.DefaultResourcesAdmin,
 									},
 								},
 							},
@@ -278,18 +282,45 @@ func TestMultigresClusterReconciliation(t *testing.T) {
 				t.Fatalf("Failed to create controller, %v", err)
 			}
 
-			// 4. Create the Input
+			// 4. Create Defaults (Templates)
+			// The controller expects "default" templates to exist when TemplateDefaults are not specified.
+			// We create empty templates so the resolution succeeds and falls back to hardcoded defaults or inline specs.
+			emptyCore := &multigresv1alpha1.CoreTemplate{
+				ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: namespace},
+				Spec:       multigresv1alpha1.CoreTemplateSpec{},
+			}
+			if err := k8sClient.Create(ctx, emptyCore); client.IgnoreAlreadyExists(err) != nil {
+				t.Fatalf("Failed to create default core template: %v", err)
+			}
+
+			emptyCell := &multigresv1alpha1.CellTemplate{
+				ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: namespace},
+				Spec:       multigresv1alpha1.CellTemplateSpec{},
+			}
+			if err := k8sClient.Create(ctx, emptyCell); client.IgnoreAlreadyExists(err) != nil {
+				t.Fatalf("Failed to create default cell template: %v", err)
+			}
+
+			emptyShard := &multigresv1alpha1.ShardTemplate{
+				ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: namespace},
+				Spec:       multigresv1alpha1.ShardTemplateSpec{},
+			}
+			if err := k8sClient.Create(ctx, emptyShard); client.IgnoreAlreadyExists(err) != nil {
+				t.Fatalf("Failed to create default shard template: %v", err)
+			}
+
+			// 5. Create the Input
 			if err := k8sClient.Create(ctx, tc.cluster); err != nil {
 				t.Fatalf("Failed to create the initial cluster, %v", err)
 			}
 
-			// 5. Assert Logic: Wait for Children
+			// 6. Assert Logic: Wait for Children
 			// This ensures the controller has run and reconciled at least once successfully
 			if err := watcher.WaitForMatch(tc.wantResources...); err != nil {
 				t.Errorf("Resources mismatch:\n%v", err)
 			}
 
-			// 6. Verify Parent Finalizer (Manual Check)
+			// 7. Verify Parent Finalizer (Manual Check)
 			// We check this manually to avoid fighting with status/spec diffs in the watcher
 			fetchedCluster := &multigresv1alpha1.MultigresCluster{}
 			if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(tc.cluster), fetchedCluster); err != nil {

pkg/cluster-handler/controller/multigrescluster/multigrescluster_controller.go

@@ -71,6 +71,9 @@ func (r *MultigresClusterReconciler) Reconcile(
 	// This now returns *defaults.Resolver
 	resolver := defaults.NewResolver(r.Client, cluster.Namespace, cluster.Spec.TemplateDefaults)
 
+	// Apply defaults (in-memory) to ensure we have images/configs even if webhook didn't run
+	resolver.PopulateClusterDefaults(cluster)
+
 	if err := r.reconcileGlobalComponents(ctx, cluster, resolver); err != nil {
 		l.Error(err, "Failed to reconcile global components")
 		return ctrl.Result{}, err
@@ -171,6 +174,8 @@ func (r *MultigresClusterReconciler) reconcileGlobalTopoServer(
 	}
 
 	spec := defaults.ResolveGlobalTopo(&cluster.Spec.GlobalTopoServer, tpl)
+	// If Etcd is nil, it means we are using External topology (or invalid config handled by validations).
+	// We only create a TopoServer CR if we are managing Etcd.
 	if spec.Etcd != nil {
 		ts := &multigresv1alpha1.TopoServer{
 			ObjectMeta: metav1.ObjectMeta{
@@ -180,14 +185,10 @@ func (r *MultigresClusterReconciler) reconcileGlobalTopoServer(
 			},
 		}
 		if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, ts, func() error {
-			replicas := defaults.DefaultEtcdReplicas
-			if spec.Etcd.Replicas != nil {
-				replicas = *spec.Etcd.Replicas
-			}
-
+			// defaults.ResolveGlobalTopo guarantees spec.Etcd.Replicas is set
 			ts.Spec.Etcd = &multigresv1alpha1.EtcdSpec{
 				Image:     spec.Etcd.Image,
-				Replicas:  &replicas,
+				Replicas:  spec.Etcd.Replicas,
 				Storage:   spec.Etcd.Storage,
 				Resources: spec.Etcd.Resources,
 			}
@@ -214,48 +215,46 @@ func (r *MultigresClusterReconciler) reconcileMultiAdmin(
 		return fmt.Errorf("failed to resolve admin template: %w", err)
 	}
 
+	// defaults.ResolveMultiAdmin guarantees a non-nil spec with defaults applied
 	spec := defaults.ResolveMultiAdmin(&cluster.Spec.MultiAdmin, tpl)
-	if spec != nil {
-		deploy := &appsv1.Deployment{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      cluster.Name + "-multiadmin",
-				Namespace: cluster.Namespace,
-				Labels: map[string]string{
-					"multigres.com/cluster": cluster.Name,
-					"app":                   "multiadmin",
-				},
+
+	deploy := &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      cluster.Name + "-multiadmin",
+			Namespace: cluster.Namespace,
+			Labels: map[string]string{
+				"multigres.com/cluster": cluster.Name,
+				"app":                   "multiadmin",
 			},
+		},
+	}
+	if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, deploy, func() error {
+		// defaults.ResolveMultiAdmin guarantees Replicas is set
+		deploy.Spec.Replicas = spec.Replicas
+		deploy.Spec.Selector = &metav1.LabelSelector{
+			MatchLabels: map[string]string{"app": "multiadmin", "multigres.com/cluster": cluster.Name},
 		}
-		if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, deploy, func() error {
-			replicas := defaults.DefaultAdminReplicas
-			if spec.Replicas != nil {
-				replicas = *spec.Replicas
-			}
-			deploy.Spec.Replicas = &replicas
-			deploy.Spec.Selector = &metav1.LabelSelector{
-				MatchLabels: map[string]string{"app": "multiadmin", "multigres.com/cluster": cluster.Name},
-			}
-			deploy.Spec.Template = corev1.PodTemplateSpec{
-				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{"app": "multiadmin", "multigres.com/cluster": cluster.Name},
-				},
-				Spec: corev1.PodSpec{
-					ImagePullSecrets: cluster.Spec.Images.ImagePullSecrets,
-					Containers: []corev1.Container{
-						{
-							Name:      "multiadmin",
-							Image:     cluster.Spec.Images.MultiAdmin,
-							Resources: spec.Resources,
-						},
+		deploy.Spec.Template = corev1.PodTemplateSpec{
+			ObjectMeta: metav1.ObjectMeta{
+				Labels: map[string]string{"app": "multiadmin", "multigres.com/cluster": cluster.Name},
+			},
+			Spec: corev1.PodSpec{
+				ImagePullSecrets: cluster.Spec.Images.ImagePullSecrets,
+				Containers: []corev1.Container{
+					{
+						Name:      "multiadmin",
+						Image:     cluster.Spec.Images.MultiAdmin,
+						Resources: spec.Resources,
 					},
-					Affinity: spec.Affinity,
 				},
-			}
-			return controllerutil.SetControllerReference(cluster, deploy, r.Scheme)
-		}); err != nil {
-			return fmt.Errorf("failed to create/update multiadmin: %w", err)
+				Affinity: spec.Affinity,
+			},
 		}
+		return controllerutil.SetControllerReference(cluster, deploy, r.Scheme)
+	}); err != nil {
+		return fmt.Errorf("failed to create/update multiadmin: %w", err)
 	}
+
 	return nil
 }