Skip to content

Commit 5b5c7b2

Browse files
fix: enforce existence for explicit templates
Updates `ResolveCoreTemplate`, `ResolveCellTemplate`, and `ResolveShardTemplate` to return an error if a specific template name (explicitly requested or cluster default) is not found. Maintains the "safe fallback" behavior only for the implicit "default" template lookup, ensuring that user typos or missing critical config fail-fast while allowing Level 4 defaults to apply for standard setups.
1 parent e4ad00d commit 5b5c7b2

File tree

2 files changed

+107
-9
lines changed

2 files changed

+107
-9
lines changed

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

Lines changed: 80 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,26 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
317317
}
318318
},
319319
},
320+
{
321+
name: "Create: MultiAdmin with ImagePullSecrets",
322+
cluster: func() *multigresv1alpha1.MultigresCluster {
323+
c := baseCluster.DeepCopy()
324+
c.Spec.Images.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "my-secret"}}
325+
return c
326+
}(),
327+
existingObjects: []client.Object{coreTpl, cellTpl, shardTpl},
328+
expectError: false,
329+
validate: func(t *testing.T, c client.Client) {
330+
ctx := context.Background()
331+
deploy := &appsv1.Deployment{}
332+
if err := c.Get(ctx, types.NamespacedName{Name: clusterName + "-multiadmin", Namespace: namespace}, deploy); err != nil {
333+
t.Fatal("MultiAdmin not created")
334+
}
335+
if len(deploy.Spec.Template.Spec.ImagePullSecrets) != 1 || deploy.Spec.Template.Spec.ImagePullSecrets[0].Name != "my-secret" {
336+
t.Errorf("ImagePullSecrets not propagated. Got %v", deploy.Spec.Template.Spec.ImagePullSecrets)
337+
}
338+
},
339+
},
320340
{
321341
name: "Create: Inline Etcd",
322342
cluster: func() *multigresv1alpha1.MultigresCluster {
@@ -475,6 +495,26 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
475495
expectError: true, // Updated to true because controller now enforces 50 char limit
476496
validate: func(t *testing.T, c client.Client) {},
477497
},
498+
{
499+
name: "Create: No Global Topo Config",
500+
cluster: func() *multigresv1alpha1.MultigresCluster {
501+
c := baseCluster.DeepCopy()
502+
c.Spec.GlobalTopoServer = multigresv1alpha1.GlobalTopoServerSpec{} // Empty
503+
c.Spec.TemplateDefaults = multigresv1alpha1.TemplateDefaults{} // Empty
504+
c.Spec.MultiAdmin = multigresv1alpha1.MultiAdminConfig{} // Empty
505+
return c
506+
}(),
507+
existingObjects: []client.Object{cellTpl, shardTpl}, // No Core Template
508+
expectError: false,
509+
validate: func(t *testing.T, c client.Client) {
510+
// Verify Cell got empty topo address
511+
cell := &multigresv1alpha1.Cell{}
512+
_ = c.Get(context.Background(), types.NamespacedName{Name: clusterName + "-zone-a", Namespace: namespace}, cell)
513+
if cell.Spec.GlobalTopoServer.Address != "" {
514+
t.Errorf("Expected empty topo address, got %q", cell.Spec.GlobalTopoServer.Address)
515+
}
516+
},
517+
},
478518
{
479519
name: "Status: Aggregation Logic",
480520
cluster: func() *multigresv1alpha1.MultigresCluster {
@@ -602,6 +642,37 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
602642
// ---------------------------------------------------------------------
603643
// Error Injection Scenarios
604644
// ---------------------------------------------------------------------
645+
{
646+
name: "Error: Explicit Template Missing (Should Fail)",
647+
cluster: func() *multigresv1alpha1.MultigresCluster {
648+
c := baseCluster.DeepCopy()
649+
c.Spec.TemplateDefaults.CoreTemplate = "non-existent-template"
650+
return c
651+
}(),
652+
existingObjects: []client.Object{}, // No templates exist
653+
failureConfig: nil, // No API failure, just logical failure
654+
expectError: true, // MUST ERROR NOW
655+
},
656+
{
657+
name: "Error: Explicit Cell Template Missing",
658+
cluster: func() *multigresv1alpha1.MultigresCluster {
659+
c := baseCluster.DeepCopy()
660+
c.Spec.Cells[0].CellTemplate = "missing-cell-tpl"
661+
return c
662+
}(),
663+
existingObjects: []client.Object{coreTpl, shardTpl}, // Missing cellTpl
664+
expectError: true,
665+
},
666+
{
667+
name: "Error: Explicit Shard Template Missing",
668+
cluster: func() *multigresv1alpha1.MultigresCluster {
669+
c := baseCluster.DeepCopy()
670+
c.Spec.Databases[0].TableGroups[0].Shards[0].ShardTemplate = "missing-shard-tpl"
671+
return c
672+
}(),
673+
existingObjects: []client.Object{coreTpl, cellTpl}, // Missing shardTpl
674+
expectError: true,
675+
},
605676
{
606677
name: "Error: Object Not Found (Clean Exit)",
607678
cluster: baseCluster.DeepCopy(),
@@ -913,6 +984,8 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
913984
c := baseCluster.DeepCopy()
914985
c.Spec.TemplateDefaults.CoreTemplate = ""
915986
c.Spec.GlobalTopoServer.TemplateRef = "topo-fail-cells"
987+
// Clear MultiAdmin to ensure predictable call counts
988+
c.Spec.MultiAdmin = multigresv1alpha1.MultiAdminConfig{}
916989
return c
917990
}(),
918991
existingObjects: []client.Object{
@@ -928,8 +1001,8 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
9281001
return func(key client.ObjectKey) error {
9291002
if key.Name == "topo-fail-cells" {
9301003
count++
931-
// Call 1: reconcileGlobalComponents (Succeed)
932-
// Call 2: reconcileCells (Fail)
1004+
// Call 1: reconcileGlobalComponents -> ResolveCoreTemplate (Succeeds to proceed)
1005+
// Call 2: reconcileCells -> getGlobalTopoRef -> ResolveCoreTemplate (Fails)
9331006
if count == 2 {
9341007
return errBoom
9351008
}
@@ -946,6 +1019,8 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
9461019
c := baseCluster.DeepCopy()
9471020
c.Spec.TemplateDefaults.CoreTemplate = ""
9481021
c.Spec.GlobalTopoServer.TemplateRef = "topo-fail-db"
1022+
// Clear MultiAdmin to ensure predictable call counts
1023+
c.Spec.MultiAdmin = multigresv1alpha1.MultiAdminConfig{}
9491024
return c
9501025
}(),
9511026
existingObjects: []client.Object{
@@ -961,9 +1036,9 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
9611036
return func(key client.ObjectKey) error {
9621037
if key.Name == "topo-fail-db" {
9631038
count++
964-
// Call 1: reconcileGlobalComponents (Succeed)
965-
// Call 2: reconcileCells (Succeed)
966-
// Call 3: reconcileDatabases (Fail)
1039+
// Call 1: reconcileGlobalComponents (Succeeds)
1040+
// Call 2: reconcileCells (Succeeds)
1041+
// Call 3: reconcileDatabases -> getGlobalTopoRef (Fails)
9671042
if count == 3 {
9681043
return errBoom
9691044
}

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

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package multigrescluster
22

33
import (
44
"context"
5+
"fmt"
56
"reflect"
67

78
multigresv1alpha1 "github.com/numtide/multigres-operator/api/v1alpha1"
@@ -27,21 +28,31 @@ type TemplateResolver struct {
2728
// 1. The cluster-level default defined in TemplateDefaults.
2829
// 2. A CoreTemplate named "default" found in the same namespace where MultigresCluster is deployed.
2930
//
30-
// If the resolved template is not found, it returns an empty CoreTemplate object and nil error.
31+
// If an explicit template (param or cluster default) is not found, it returns an error.
32+
// If the implicit "default" template is not found, it returns an empty object (safe fallback).
33+
// In this case the default would be applied by the operator via mutating webhook.
3134
func (r *TemplateResolver) ResolveCoreTemplate(ctx context.Context, templateName string) (*multigresv1alpha1.CoreTemplate, error) {
3235
name := templateName
36+
isImplicitFallback := false
37+
3338
if name == "" {
3439
name = r.Defaults.CoreTemplate
3540
}
3641
if name == "" {
3742
name = "default"
43+
isImplicitFallback = true
3844
}
3945

4046
tpl := &multigresv1alpha1.CoreTemplate{}
4147
err := r.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: r.Namespace}, tpl)
4248
if err != nil {
4349
if errors.IsNotFound(err) {
44-
return &multigresv1alpha1.CoreTemplate{}, nil
50+
if isImplicitFallback {
51+
// It is safe to ignore a missing "default" template
52+
return &multigresv1alpha1.CoreTemplate{}, nil
53+
}
54+
// It is NOT safe to ignore a missing template requested by the user
55+
return nil, fmt.Errorf("referenced CoreTemplate '%s' not found", name)
4556
}
4657
return nil, err
4758
}
@@ -51,18 +62,24 @@ func (r *TemplateResolver) ResolveCoreTemplate(ctx context.Context, templateName
5162
// ResolveCellTemplate fetches and resolves a CellTemplate by name, handling defaults.
5263
func (r *TemplateResolver) ResolveCellTemplate(ctx context.Context, templateName string) (*multigresv1alpha1.CellTemplate, error) {
5364
name := templateName
65+
isImplicitFallback := false
66+
5467
if name == "" {
5568
name = r.Defaults.CellTemplate
5669
}
5770
if name == "" {
5871
name = "default"
72+
isImplicitFallback = true
5973
}
6074

6175
tpl := &multigresv1alpha1.CellTemplate{}
6276
err := r.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: r.Namespace}, tpl)
6377
if err != nil {
6478
if errors.IsNotFound(err) {
65-
return &multigresv1alpha1.CellTemplate{}, nil
79+
if isImplicitFallback {
80+
return &multigresv1alpha1.CellTemplate{}, nil
81+
}
82+
return nil, fmt.Errorf("referenced CellTemplate '%s' not found", name)
6683
}
6784
return nil, err
6885
}
@@ -72,18 +89,24 @@ func (r *TemplateResolver) ResolveCellTemplate(ctx context.Context, templateName
7289
// ResolveShardTemplate fetches and resolves a ShardTemplate by name, handling defaults.
7390
func (r *TemplateResolver) ResolveShardTemplate(ctx context.Context, templateName string) (*multigresv1alpha1.ShardTemplate, error) {
7491
name := templateName
92+
isImplicitFallback := false
93+
7594
if name == "" {
7695
name = r.Defaults.ShardTemplate
7796
}
7897
if name == "" {
7998
name = "default"
99+
isImplicitFallback = true
80100
}
81101

82102
tpl := &multigresv1alpha1.ShardTemplate{}
83103
err := r.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: r.Namespace}, tpl)
84104
if err != nil {
85105
if errors.IsNotFound(err) {
86-
return &multigresv1alpha1.ShardTemplate{}, nil
106+
if isImplicitFallback {
107+
return &multigresv1alpha1.ShardTemplate{}, nil
108+
}
109+
return nil, fmt.Errorf("referenced ShardTemplate '%s' not found", name)
87110
}
88111
return nil, err
89112
}

0 commit comments

Comments
 (0)