Skip to content

Commit 790c220

Browse files
fix(api): Add omitzero and omitempty to optional fields (#546)
* fix(deps): Update Go version to 1.24 * fix(deps): Update go.mod golang version * fix(api): Add omitzero and omitempty to optional fields - Cluster is an optional field of type struct: omitzero - Subnets is a container type: omitempty * fix(lint): fix linting issues
1 parent fecc2f9 commit 790c220

File tree

8 files changed

+293
-34
lines changed

8 files changed

+293
-34
lines changed

api/v1beta1/nutanixmachine_types.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,12 @@ type NutanixMachineSpec struct {
132132
// The cluster identifier (uuid or name) can be obtained from the Prism Central console
133133
// or using the prism_central API.
134134
// +kubebuilder:validation:Optional
135-
Cluster NutanixResourceIdentifier `json:"cluster"`
135+
Cluster NutanixResourceIdentifier `json:"cluster,omitzero"`
136136
// subnet is to identify the cluster's network subnet to use for the Machine's VM
137137
// The cluster identifier (uuid or name) can be obtained from the Prism Central console
138138
// or using the prism_central API.
139139
// +kubebuilder:validation:Optional
140-
Subnets []NutanixResourceIdentifier `json:"subnet"`
140+
Subnets []NutanixResourceIdentifier `json:"subnet,omitempty"`
141141
// List of categories that need to be added to the machines. Categories must already exist in Prism Central
142142
// +kubebuilder:validation:Optional
143143
AdditionalCategories []NutanixCategoryIdentifier `json:"additionalCategories,omitempty"`
Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
/*
2+
Copyright 2025 Nutanix
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v1beta1
18+
19+
import (
20+
"encoding/json"
21+
"testing"
22+
23+
"github.com/stretchr/testify/assert"
24+
"github.com/stretchr/testify/require"
25+
"k8s.io/apimachinery/pkg/api/resource"
26+
"k8s.io/utils/ptr"
27+
)
28+
29+
func TestNutanixMachineSpec_JSONMarshalling(t *testing.T) {
30+
tests := []struct {
31+
name string
32+
spec NutanixMachineSpec
33+
expectedJSON string
34+
shouldContain []string
35+
shouldNotContain []string
36+
}{
37+
{
38+
name: "marshalling with zero-value cluster should omit cluster field",
39+
spec: NutanixMachineSpec{
40+
VCPUsPerSocket: 2,
41+
VCPUSockets: 1,
42+
MemorySize: resource.MustParse("4Gi"),
43+
SystemDiskSize: resource.MustParse("20Gi"),
44+
Subnets: []NutanixResourceIdentifier{
45+
{
46+
Type: NutanixIdentifierUUID,
47+
UUID: ptr.To("subnet-uuid"),
48+
},
49+
},
50+
// Cluster field is intentionally left as zero value
51+
},
52+
shouldNotContain: []string{`"cluster"`},
53+
shouldContain: []string{`"vcpusPerSocket"`, `"vcpuSockets"`, `"memorySize"`, `"systemDiskSize"`, `"subnet"`},
54+
},
55+
{
56+
name: "marshalling with populated cluster should include cluster field",
57+
spec: NutanixMachineSpec{
58+
VCPUsPerSocket: 2,
59+
VCPUSockets: 1,
60+
MemorySize: resource.MustParse("4Gi"),
61+
SystemDiskSize: resource.MustParse("20Gi"),
62+
Cluster: NutanixResourceIdentifier{
63+
Type: NutanixIdentifierUUID,
64+
UUID: ptr.To("cluster-uuid"),
65+
},
66+
Subnets: []NutanixResourceIdentifier{
67+
{
68+
Type: NutanixIdentifierUUID,
69+
UUID: ptr.To("subnet-uuid"),
70+
},
71+
},
72+
},
73+
shouldContain: []string{`"cluster"`, `"vcpusPerSocket"`, `"vcpuSockets"`, `"memorySize"`, `"systemDiskSize"`, `"subnet"`},
74+
},
75+
{
76+
name: "marshalling with cluster by name should include cluster field",
77+
spec: NutanixMachineSpec{
78+
VCPUsPerSocket: 2,
79+
VCPUSockets: 1,
80+
MemorySize: resource.MustParse("4Gi"),
81+
SystemDiskSize: resource.MustParse("20Gi"),
82+
Cluster: NutanixResourceIdentifier{
83+
Type: NutanixIdentifierName,
84+
Name: ptr.To("cluster-name"),
85+
},
86+
Subnets: []NutanixResourceIdentifier{
87+
{
88+
Type: NutanixIdentifierName,
89+
Name: ptr.To("subnet-name"),
90+
},
91+
},
92+
},
93+
shouldContain: []string{`"cluster"`, `"cluster-name"`, `"vcpusPerSocket"`, `"vcpuSockets"`, `"memorySize"`, `"systemDiskSize"`, `"subnet"`},
94+
},
95+
{
96+
name: "marshalling with explicitly empty cluster struct should omit cluster field",
97+
spec: NutanixMachineSpec{
98+
VCPUsPerSocket: 2,
99+
VCPUSockets: 1,
100+
MemorySize: resource.MustParse("4Gi"),
101+
SystemDiskSize: resource.MustParse("20Gi"),
102+
Cluster: NutanixResourceIdentifier{
103+
// Explicitly created empty struct with zero values
104+
Type: "",
105+
UUID: nil,
106+
Name: nil,
107+
},
108+
Subnets: []NutanixResourceIdentifier{
109+
{
110+
Type: NutanixIdentifierUUID,
111+
UUID: ptr.To("subnet-uuid"),
112+
},
113+
},
114+
},
115+
shouldNotContain: []string{`"cluster"`},
116+
shouldContain: []string{`"vcpusPerSocket"`, `"vcpuSockets"`, `"memorySize"`, `"systemDiskSize"`, `"subnet"`},
117+
},
118+
}
119+
120+
for _, tt := range tests {
121+
t.Run(tt.name, func(t *testing.T) {
122+
jsonBytes, err := json.Marshal(tt.spec)
123+
require.NoError(t, err, "Failed to marshal NutanixMachineSpec")
124+
125+
jsonString := string(jsonBytes)
126+
127+
// Check for strings that should be present
128+
for _, shouldContain := range tt.shouldContain {
129+
assert.Contains(t, jsonString, shouldContain, "Expected JSON to contain %q", shouldContain)
130+
}
131+
132+
// Check for strings that should not be present
133+
for _, shouldNotContain := range tt.shouldNotContain {
134+
assert.NotContains(t, jsonString, shouldNotContain, "Expected JSON to NOT contain %q", shouldNotContain)
135+
}
136+
137+
// Test that we can unmarshal back to the same structure
138+
var unmarshaledSpec NutanixMachineSpec
139+
require.NoError(t, json.Unmarshal(jsonBytes, &unmarshaledSpec), "Failed to unmarshal JSON back to NutanixMachineSpec")
140+
141+
// Verify critical fields are preserved
142+
assert.Equal(t, tt.spec.VCPUsPerSocket, unmarshaledSpec.VCPUsPerSocket, "VCPUsPerSocket mismatch")
143+
assert.Equal(t, tt.spec.VCPUSockets, unmarshaledSpec.VCPUSockets, "VCPUSockets mismatch")
144+
assert.True(t, unmarshaledSpec.MemorySize.Equal(tt.spec.MemorySize), "MemorySize mismatch: expected %v, got %v", tt.spec.MemorySize, unmarshaledSpec.MemorySize)
145+
assert.True(t, unmarshaledSpec.SystemDiskSize.Equal(tt.spec.SystemDiskSize), "SystemDiskSize mismatch: expected %v, got %v", tt.spec.SystemDiskSize, unmarshaledSpec.SystemDiskSize)
146+
147+
// Verify cluster field behavior
148+
if tt.spec.Cluster.Type != "" {
149+
// If original had a cluster, unmarshaled should too
150+
assert.Equal(t, tt.spec.Cluster.Type, unmarshaledSpec.Cluster.Type, "Cluster.Type mismatch")
151+
if tt.spec.Cluster.UUID != nil {
152+
require.NotNil(t, unmarshaledSpec.Cluster.UUID, "Expected Cluster.UUID to not be nil")
153+
assert.Equal(t, *tt.spec.Cluster.UUID, *unmarshaledSpec.Cluster.UUID, "Cluster.UUID mismatch")
154+
}
155+
if tt.spec.Cluster.Name != nil {
156+
require.NotNil(t, unmarshaledSpec.Cluster.Name, "Expected Cluster.Name to not be nil")
157+
assert.Equal(t, *tt.spec.Cluster.Name, *unmarshaledSpec.Cluster.Name, "Cluster.Name mismatch")
158+
}
159+
} else {
160+
// If original had zero cluster, unmarshaled should be zero too
161+
assert.Empty(t, unmarshaledSpec.Cluster.Type, "Expected unmarshaled cluster type to be empty")
162+
assert.Nil(t, unmarshaledSpec.Cluster.UUID, "Expected unmarshaled cluster UUID to be nil")
163+
assert.Nil(t, unmarshaledSpec.Cluster.Name, "Expected unmarshaled cluster Name to be nil")
164+
}
165+
})
166+
}
167+
}
168+
169+
func TestNutanixMachineSpec_JSONUnmarshalling(t *testing.T) {
170+
tests := []struct {
171+
name string
172+
jsonInput string
173+
expectError bool
174+
expectedSpec NutanixMachineSpec
175+
}{
176+
{
177+
name: "unmarshalling JSON without cluster field should result in zero-value cluster",
178+
jsonInput: `{
179+
"vcpusPerSocket": 2,
180+
"vcpuSockets": 1,
181+
"memorySize": "4Gi",
182+
"systemDiskSize": "20Gi",
183+
"subnet": [{"type": "uuid", "uuid": "subnet-uuid"}]
184+
}`,
185+
expectError: false,
186+
expectedSpec: NutanixMachineSpec{
187+
VCPUsPerSocket: 2,
188+
VCPUSockets: 1,
189+
MemorySize: resource.MustParse("4Gi"),
190+
SystemDiskSize: resource.MustParse("20Gi"),
191+
Subnets: []NutanixResourceIdentifier{
192+
{
193+
Type: NutanixIdentifierUUID,
194+
UUID: ptr.To("subnet-uuid"),
195+
},
196+
},
197+
// Cluster should be zero value
198+
},
199+
},
200+
{
201+
name: "unmarshalling JSON with cluster field should populate cluster",
202+
jsonInput: `{
203+
"vcpusPerSocket": 2,
204+
"vcpuSockets": 1,
205+
"memorySize": "4Gi",
206+
"systemDiskSize": "20Gi",
207+
"cluster": {"type": "uuid", "uuid": "cluster-uuid"},
208+
"subnet": [{"type": "uuid", "uuid": "subnet-uuid"}]
209+
}`,
210+
expectError: false,
211+
expectedSpec: NutanixMachineSpec{
212+
VCPUsPerSocket: 2,
213+
VCPUSockets: 1,
214+
MemorySize: resource.MustParse("4Gi"),
215+
SystemDiskSize: resource.MustParse("20Gi"),
216+
Cluster: NutanixResourceIdentifier{
217+
Type: NutanixIdentifierUUID,
218+
UUID: ptr.To("cluster-uuid"),
219+
},
220+
Subnets: []NutanixResourceIdentifier{
221+
{
222+
Type: NutanixIdentifierUUID,
223+
UUID: ptr.To("subnet-uuid"),
224+
},
225+
},
226+
},
227+
},
228+
}
229+
230+
for _, tt := range tests {
231+
t.Run(tt.name, func(t *testing.T) {
232+
var spec NutanixMachineSpec
233+
err := json.Unmarshal([]byte(tt.jsonInput), &spec)
234+
235+
if tt.expectError {
236+
assert.Error(t, err, "Expected an error but got none")
237+
return
238+
}
239+
240+
require.NoError(t, err, "Expected no error but got: %v", err)
241+
242+
// Verify the unmarshaled spec matches expectations
243+
assert.Equal(t, tt.expectedSpec.VCPUsPerSocket, spec.VCPUsPerSocket, "VCPUsPerSocket mismatch")
244+
assert.Equal(t, tt.expectedSpec.VCPUSockets, spec.VCPUSockets, "VCPUSockets mismatch")
245+
assert.True(t, spec.MemorySize.Equal(tt.expectedSpec.MemorySize), "MemorySize mismatch: expected %v, got %v", tt.expectedSpec.MemorySize, spec.MemorySize)
246+
assert.True(t, spec.SystemDiskSize.Equal(tt.expectedSpec.SystemDiskSize), "SystemDiskSize mismatch: expected %v, got %v", tt.expectedSpec.SystemDiskSize, spec.SystemDiskSize)
247+
248+
// Verify cluster field
249+
assert.Equal(t, tt.expectedSpec.Cluster.Type, spec.Cluster.Type, "Cluster.Type mismatch")
250+
if tt.expectedSpec.Cluster.UUID != nil {
251+
require.NotNil(t, spec.Cluster.UUID, "Expected Cluster.UUID to not be nil")
252+
assert.Equal(t, *tt.expectedSpec.Cluster.UUID, *spec.Cluster.UUID, "Cluster.UUID mismatch")
253+
} else {
254+
assert.Nil(t, spec.Cluster.UUID, "Expected Cluster.UUID to be nil")
255+
}
256+
})
257+
}
258+
}

controllers/helpers.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,7 +1093,7 @@ func getPrismCentralClientForCluster(ctx context.Context, cluster *infrav1.Nutan
10931093
managementEndpoint, err := clientHelper.BuildManagementEndpoint(ctx, cluster)
10941094
if err != nil {
10951095
log.Error(err, fmt.Sprintf("error occurred while getting management endpoint for cluster %q", cluster.GetNamespacedName()))
1096-
conditions.MarkFalse(cluster, infrav1.PrismCentralClientCondition, infrav1.PrismCentralClientInitializationFailed, capiv1.ConditionSeverityError, err.Error())
1096+
conditions.MarkFalse(cluster, infrav1.PrismCentralClientCondition, infrav1.PrismCentralClientInitializationFailed, capiv1.ConditionSeverityError, "%s", err.Error())
10971097
return nil, err
10981098
}
10991099

@@ -1104,7 +1104,7 @@ func getPrismCentralClientForCluster(ctx context.Context, cluster *infrav1.Nutan
11041104
})
11051105
if err != nil {
11061106
log.Error(err, "error occurred while getting nutanix prism v3 Client from cache")
1107-
conditions.MarkFalse(cluster, infrav1.PrismCentralClientCondition, infrav1.PrismCentralClientInitializationFailed, capiv1.ConditionSeverityError, err.Error())
1107+
conditions.MarkFalse(cluster, infrav1.PrismCentralClientCondition, infrav1.PrismCentralClientInitializationFailed, capiv1.ConditionSeverityError, "%s", err.Error())
11081108
return nil, fmt.Errorf("nutanix prism v3 Client error: %w", err)
11091109
}
11101110

@@ -1119,7 +1119,7 @@ func getPrismCentralV4ClientForCluster(ctx context.Context, cluster *infrav1.Nut
11191119
managementEndpoint, err := clientHelper.BuildManagementEndpoint(ctx, cluster)
11201120
if err != nil {
11211121
log.Error(err, fmt.Sprintf("error occurred while getting management endpoint for cluster %q", cluster.GetNamespacedName()))
1122-
conditions.MarkFalse(cluster, infrav1.PrismCentralV4ClientCondition, infrav1.PrismCentralV4ClientInitializationFailed, capiv1.ConditionSeverityError, err.Error())
1122+
conditions.MarkFalse(cluster, infrav1.PrismCentralV4ClientCondition, infrav1.PrismCentralV4ClientInitializationFailed, capiv1.ConditionSeverityError, "%s", err.Error())
11231123
return nil, err
11241124
}
11251125

@@ -1129,7 +1129,7 @@ func getPrismCentralV4ClientForCluster(ctx context.Context, cluster *infrav1.Nut
11291129
})
11301130
if err != nil {
11311131
log.Error(err, "error occurred while getting nutanix prism v4 client from cache")
1132-
conditions.MarkFalse(cluster, infrav1.PrismCentralV4ClientCondition, infrav1.PrismCentralV4ClientInitializationFailed, capiv1.ConditionSeverityError, err.Error())
1132+
conditions.MarkFalse(cluster, infrav1.PrismCentralV4ClientCondition, infrav1.PrismCentralV4ClientInitializationFailed, capiv1.ConditionSeverityError, "%s", err.Error())
11331133
return nil, fmt.Errorf("nutanix prism v4 client error: %w", err)
11341134
}
11351135

controllers/nutanixcluster_controller.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ func (r *NutanixClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
212212

213213
if err := r.reconcileCredentialRef(ctx, cluster); err != nil {
214214
log.Error(err, fmt.Sprintf("error occurred while reconciling credential ref for cluster %s", capiCluster.Name))
215-
conditions.MarkFalse(cluster, infrav1.CredentialRefSecretOwnerSetCondition, infrav1.CredentialRefSecretOwnerSetFailed, capiv1.ConditionSeverityError, err.Error())
215+
conditions.MarkFalse(cluster, infrav1.CredentialRefSecretOwnerSetCondition, infrav1.CredentialRefSecretOwnerSetFailed, capiv1.ConditionSeverityError, "%s", err.Error())
216216
return reconcile.Result{}, err
217217
}
218218
conditions.MarkTrue(cluster, infrav1.CredentialRefSecretOwnerSetCondition)
@@ -383,7 +383,7 @@ func (r *NutanixClusterReconciler) reconcileFailureDomains(rctx *nctx.ClusterCon
383383

384384
if len(validationErrs) != 0 {
385385
conditions.MarkFalse(rctx.NutanixCluster, infrav1.FailureDomainsValidatedCondition,
386-
infrav1.FailureDomainsMisconfiguredReason, capiv1.ConditionSeverityWarning, errors.Join(validationErrs...).Error())
386+
infrav1.FailureDomainsMisconfiguredReason, capiv1.ConditionSeverityWarning, "%s", errors.Join(validationErrs...).Error())
387387
return nil
388388
}
389389

@@ -415,7 +415,7 @@ func (r *NutanixClusterReconciler) reconcileCategories(rctx *nctx.ClusterContext
415415
defaultCategories := GetDefaultCAPICategoryIdentifiers(rctx.Cluster.Name)
416416
_, err := GetOrCreateCategories(rctx.Context, rctx.NutanixClient, defaultCategories)
417417
if err != nil {
418-
conditions.MarkFalse(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition, infrav1.ClusterCategoryCreationFailed, capiv1.ConditionSeverityError, err.Error())
418+
conditions.MarkFalse(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition, infrav1.ClusterCategoryCreationFailed, capiv1.ConditionSeverityError, "%s", err.Error())
419419
return err
420420
}
421421
conditions.MarkTrue(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition)
@@ -431,7 +431,7 @@ func (r *NutanixClusterReconciler) reconcileCategoriesDelete(rctx *nctx.ClusterC
431431
obsoleteCategories := GetObsoleteDefaultCAPICategoryIdentifiers(rctx.Cluster.Name)
432432
err := DeleteCategories(rctx.Context, rctx.NutanixClient, defaultCategories, obsoleteCategories)
433433
if err != nil {
434-
conditions.MarkFalse(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition, infrav1.DeletionFailed, capiv1.ConditionSeverityWarning, err.Error())
434+
conditions.MarkFalse(rctx.NutanixCluster, infrav1.ClusterCategoryCreatedCondition, infrav1.DeletionFailed, capiv1.ConditionSeverityWarning, "%s", err.Error())
435435
return err
436436
}
437437
} else {
@@ -500,7 +500,7 @@ func (r *NutanixClusterReconciler) reconcileTrustBundleRef(ctx context.Context,
500500
}
501501
if err := r.Client.Get(ctx, configMapKey, configMap); err != nil {
502502
log.Error(err, "error occurred while fetching trust bundle configmap", "nutanixCluster", nutanixCluster.Name)
503-
conditions.MarkFalse(nutanixCluster, infrav1.TrustBundleSecretOwnerSetCondition, infrav1.TrustBundleSecretOwnerSetFailed, capiv1.ConditionSeverityError, err.Error())
503+
conditions.MarkFalse(nutanixCluster, infrav1.TrustBundleSecretOwnerSetCondition, infrav1.TrustBundleSecretOwnerSetFailed, capiv1.ConditionSeverityError, "%s", err.Error())
504504
return err
505505
}
506506

@@ -525,7 +525,7 @@ func (r *NutanixClusterReconciler) reconcileTrustBundleRef(ctx context.Context,
525525

526526
if err := r.Client.Update(ctx, configMap); err != nil {
527527
log.Error(err, "error occurred while updating trust bundle configmap", "nutanixCluster", nutanixCluster)
528-
conditions.MarkFalse(nutanixCluster, infrav1.TrustBundleSecretOwnerSetCondition, infrav1.TrustBundleSecretOwnerSetFailed, capiv1.ConditionSeverityError, err.Error())
528+
conditions.MarkFalse(nutanixCluster, infrav1.TrustBundleSecretOwnerSetCondition, infrav1.TrustBundleSecretOwnerSetFailed, capiv1.ConditionSeverityError, "%s", err.Error())
529529
return err
530530
}
531531

controllers/nutanixfailuredomain_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ func (r *NutanixFailureDomainReconciler) reconcileDelete(ctx context.Context, fd
216216

217217
errMsg := fmt.Sprintf("The failure domain is used by machines: %v", ntxMachines)
218218
conditions.MarkFalse(fd, infrav1.FailureDomainSafeForDeletionCondition,
219-
infrav1.FailureDomainInUseReason, capiv1.ConditionSeverityError, errMsg)
219+
infrav1.FailureDomainInUseReason, capiv1.ConditionSeverityError, "%s", errMsg)
220220

221221
reterr := fmt.Errorf("the failure domain %q is not safe for deletion since it is in use", fd.Name)
222222
log.Error(reterr, errMsg)

0 commit comments

Comments
 (0)