Skip to content

Commit 55c3a27

Browse files
refactor(resolver): make default resource variables immutable functions
Refactors `DefaultResourcesAdmin` and `DefaultResourcesEtcd` in the `resolver` package from exported package-level variables to functions. This change prevents consumers of the package from accidentally mutating the global default state, which could have side effects on other parts of the application. Updates `resolver` logic and tests, as well as `cluster-handler` integration tests, to invoke these functions instead of accessing variables directly.
1 parent 5e233b8 commit 55c3a27

File tree

4 files changed

+29
-29
lines changed

4 files changed

+29
-29
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ func TestMultigresClusterReconciliation(t *testing.T) {
128128
Image: "etcd:latest",
129129
Replicas: ptr.To(int32(3)), // Default from logic
130130
Storage: multigresv1alpha1.StorageSpec{Size: resolver.DefaultEtcdStorageSize},
131-
Resources: resolver.DefaultResourcesEtcd,
131+
Resources: resolver.DefaultResourcesEtcd(),
132132
},
133133
},
134134
},
@@ -154,7 +154,7 @@ func TestMultigresClusterReconciliation(t *testing.T) {
154154
{
155155
Name: "multiadmin",
156156
Image: "admin:latest",
157-
Resources: resolver.DefaultResourcesAdmin,
157+
Resources: resolver.DefaultResourcesAdmin(),
158158
},
159159
},
160160
},

pkg/resolver/cluster.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func (r *Resolver) PopulateClusterDefaults(cluster *multigresv1alpha1.MultigresC
6060
if cluster.Spec.MultiAdmin.Spec != nil {
6161
defaultStatelessSpec(
6262
cluster.Spec.MultiAdmin.Spec,
63-
DefaultResourcesAdmin,
63+
DefaultResourcesAdmin(),
6464
DefaultAdminReplicas,
6565
)
6666
}
@@ -93,8 +93,8 @@ func defaultEtcdSpec(spec *multigresv1alpha1.EtcdSpec) {
9393
spec.Replicas = &r
9494
}
9595
if isResourcesEmpty(spec.Resources) {
96-
// Safety: Must DeepCopy global default to avoid shared mutable state
97-
spec.Resources = *DefaultResourcesEtcd.DeepCopy()
96+
// Safety: DefaultResourcesEtcd() returns a fresh struct, so no DeepCopy needed.
97+
spec.Resources = DefaultResourcesEtcd()
9898
}
9999
}
100100

@@ -108,7 +108,8 @@ func defaultStatelessSpec(
108108
spec.Replicas = &defaultReplicas
109109
}
110110
if isResourcesEmpty(spec.Resources) {
111-
// Safety: Must DeepCopy global default to avoid shared mutable state
111+
// Safety: We assume defaultRes is passed by value (a fresh copy from the default function).
112+
// We perform a DeepCopy to ensure spec.Resources owns its own maps, independent of the input defaultRes.
112113
spec.Resources = *defaultRes.DeepCopy()
113114
}
114115
}
@@ -208,7 +209,7 @@ func ResolveMultiAdmin(
208209
}
209210

210211
// 2. Apply Deep Defaults
211-
defaultStatelessSpec(finalSpec, DefaultResourcesAdmin, DefaultAdminReplicas)
212+
defaultStatelessSpec(finalSpec, DefaultResourcesAdmin(), DefaultAdminReplicas)
212213

213214
return finalSpec
214215
}

pkg/resolver/cluster_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,14 @@ func TestResolver_PopulateClusterDefaults(t *testing.T) {
115115
Etcd: &multigresv1alpha1.EtcdSpec{
116116
Image: DefaultEtcdImage,
117117
Replicas: ptr.To(DefaultEtcdReplicas),
118-
Resources: DefaultResourcesEtcd,
118+
Resources: DefaultResourcesEtcd(),
119119
Storage: multigresv1alpha1.StorageSpec{Size: DefaultEtcdStorageSize},
120120
},
121121
},
122122
MultiAdmin: multigresv1alpha1.MultiAdminConfig{
123123
Spec: &multigresv1alpha1.StatelessSpec{
124124
Replicas: ptr.To(DefaultAdminReplicas),
125-
Resources: DefaultResourcesAdmin,
125+
Resources: DefaultResourcesAdmin(),
126126
},
127127
},
128128
},
@@ -199,7 +199,7 @@ func TestResolver_PopulateClusterDefaults(t *testing.T) {
199199
Etcd: &multigresv1alpha1.EtcdSpec{
200200
Image: "custom-etcd",
201201
Replicas: ptr.To(DefaultEtcdReplicas),
202-
Resources: DefaultResourcesEtcd,
202+
Resources: DefaultResourcesEtcd(),
203203
Storage: multigresv1alpha1.StorageSpec{Size: DefaultEtcdStorageSize},
204204
},
205205
},
@@ -344,7 +344,7 @@ func TestResolveGlobalTopo(t *testing.T) {
344344
Etcd: &multigresv1alpha1.EtcdSpec{
345345
Image: "inline",
346346
Replicas: ptr.To(DefaultEtcdReplicas),
347-
Resources: DefaultResourcesEtcd,
347+
Resources: DefaultResourcesEtcd(),
348348
Storage: multigresv1alpha1.StorageSpec{Size: DefaultEtcdStorageSize},
349349
},
350350
},
@@ -375,7 +375,7 @@ func TestResolveGlobalTopo(t *testing.T) {
375375
Etcd: &multigresv1alpha1.EtcdSpec{
376376
Image: "template",
377377
Replicas: ptr.To(DefaultEtcdReplicas),
378-
Resources: DefaultResourcesEtcd,
378+
Resources: DefaultResourcesEtcd(),
379379
Storage: multigresv1alpha1.StorageSpec{Size: DefaultEtcdStorageSize},
380380
},
381381
},
@@ -391,7 +391,7 @@ func TestResolveGlobalTopo(t *testing.T) {
391391
Etcd: &multigresv1alpha1.EtcdSpec{
392392
Image: DefaultEtcdImage,
393393
Replicas: ptr.To(DefaultEtcdReplicas),
394-
Resources: DefaultResourcesEtcd,
394+
Resources: DefaultResourcesEtcd(),
395395
Storage: multigresv1alpha1.StorageSpec{Size: DefaultEtcdStorageSize},
396396
},
397397
},
@@ -403,7 +403,7 @@ func TestResolveGlobalTopo(t *testing.T) {
403403
Etcd: &multigresv1alpha1.EtcdSpec{
404404
Image: DefaultEtcdImage,
405405
Replicas: ptr.To(DefaultEtcdReplicas),
406-
Resources: DefaultResourcesEtcd,
406+
Resources: DefaultResourcesEtcd(),
407407
Storage: multigresv1alpha1.StorageSpec{Size: DefaultEtcdStorageSize},
408408
},
409409
},
@@ -436,7 +436,7 @@ func TestResolveMultiAdmin(t *testing.T) {
436436
tpl: nil,
437437
want: &multigresv1alpha1.StatelessSpec{
438438
Replicas: ptr.To(int32(5)),
439-
Resources: DefaultResourcesAdmin,
439+
Resources: DefaultResourcesAdmin(),
440440
},
441441
},
442442
"Template Fallback": {
@@ -448,7 +448,7 @@ func TestResolveMultiAdmin(t *testing.T) {
448448
},
449449
want: &multigresv1alpha1.StatelessSpec{
450450
Replicas: ptr.To(int32(3)),
451-
Resources: DefaultResourcesAdmin,
451+
Resources: DefaultResourcesAdmin(),
452452
},
453453
},
454454
"Template Found but Nil Content": {
@@ -460,15 +460,15 @@ func TestResolveMultiAdmin(t *testing.T) {
460460
},
461461
want: &multigresv1alpha1.StatelessSpec{
462462
Replicas: ptr.To(DefaultAdminReplicas),
463-
Resources: DefaultResourcesAdmin,
463+
Resources: DefaultResourcesAdmin(),
464464
},
465465
},
466466
"No-op (Nil Template)": {
467467
spec: &multigresv1alpha1.MultiAdminConfig{},
468468
tpl: nil,
469469
want: &multigresv1alpha1.StatelessSpec{
470470
Replicas: ptr.To(DefaultAdminReplicas),
471-
Resources: DefaultResourcesAdmin,
471+
Resources: DefaultResourcesAdmin(),
472472
},
473473
},
474474
}

pkg/resolver/defaults.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,10 @@ const (
4646
DefaultEtcdStorageSize = "1Gi"
4747
)
4848

49-
// Resource Defaults (Computed variables for reuse).
50-
// Note: These are declared as variables because Go does not support const structs,
51-
// but they should be treated as immutable.
52-
var (
53-
// DefaultResourcesAdmin defines the default resource requests and limits for the MultiAdmin deployment.
54-
// It requests 100m CPU and 128Mi memory, with a limit of 256Mi memory.
55-
DefaultResourcesAdmin = corev1.ResourceRequirements{
49+
// DefaultResourcesAdmin returns the default resource requests and limits for the MultiAdmin deployment.
50+
// It requests 100m CPU and 128Mi memory, with a limit of 256Mi memory.
51+
func DefaultResourcesAdmin() corev1.ResourceRequirements {
52+
return corev1.ResourceRequirements{
5653
Requests: corev1.ResourceList{
5754
corev1.ResourceCPU: resource.MustParse("100m"),
5855
corev1.ResourceMemory: resource.MustParse("128Mi"),
@@ -61,10 +58,12 @@ var (
6158
corev1.ResourceMemory: resource.MustParse("256Mi"),
6259
},
6360
}
61+
}
6462

65-
// DefaultResourcesEtcd defines the default resource requests and limits for the managed Etcd cluster.
66-
// It requests 100m CPU and 256Mi memory, with a limit of 512Mi memory.
67-
DefaultResourcesEtcd = corev1.ResourceRequirements{
63+
// DefaultResourcesEtcd returns the default resource requests and limits for the managed Etcd cluster.
64+
// It requests 100m CPU and 256Mi memory, with a limit of 512Mi memory.
65+
func DefaultResourcesEtcd() corev1.ResourceRequirements {
66+
return corev1.ResourceRequirements{
6867
Requests: corev1.ResourceList{
6968
corev1.ResourceCPU: resource.MustParse("100m"),
7069
corev1.ResourceMemory: resource.MustParse("256Mi"),
@@ -73,4 +72,4 @@ var (
7372
corev1.ResourceMemory: resource.MustParse("512Mi"),
7473
},
7574
}
76-
)
75+
}

0 commit comments

Comments
 (0)