Skip to content

Commit 22e4b06

Browse files
sbueringersbueringer
committed
Change Required fields in ClusterClass struct from bool to *bool
1 parent cf12569 commit 22e4b06

14 files changed

+277
-220
lines changed

‎api/core/v1beta1/conversion_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,8 @@ func spokeClusterVariable(in *ClusterVariable, c randfill.Continue) {
254254

255255
func ClusterClassFuncs(_ runtimeserializer.CodecFactory) []interface{} {
256256
return []interface{}{
257+
hubClusterClassVariable,
258+
hubClusterClassStatusVariableDefinition,
257259
hubClusterClassStatus,
258260
hubJSONPatch,
259261
hubJSONSchemaProps,
@@ -271,6 +273,22 @@ func ClusterClassFuncs(_ runtimeserializer.CodecFactory) []interface{} {
271273
}
272274
}
273275

276+
func hubClusterClassVariable(in *clusterv1.ClusterClassVariable, c randfill.Continue) {
277+
c.FillNoCustom(in)
278+
279+
if in.Required == nil {
280+
in.Required = ptr.To(false) // Required is a required field and nil does not round trip
281+
}
282+
}
283+
284+
func hubClusterClassStatusVariableDefinition(in *clusterv1.ClusterClassStatusVariableDefinition, c randfill.Continue) {
285+
c.FillNoCustom(in)
286+
287+
if in.Required == nil {
288+
in.Required = ptr.To(false) // Required is a required field and nil does not round trip
289+
}
290+
}
291+
274292
func hubClusterClassStatus(in *clusterv1.ClusterClassStatus, c randfill.Continue) {
275293
c.FillNoCustom(in)
276294
// Drop empty structs with only omit empty fields.

‎api/core/v1beta1/zz_generated.conversion.go

Lines changed: 12 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/core/v1beta2/clusterclass_types.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,7 @@ type ClusterClassVariable struct {
807807
// top-level object defined in the schema. If nested fields are
808808
// required, this will be specified inside the schema.
809809
// +required
810-
Required bool `json:"required"`
810+
Required *bool `json:"required,omitempty"`
811811

812812
// deprecatedV1Beta1Metadata is the metadata of a variable.
813813
// It can be used to add additional data for higher level tools to
@@ -1584,7 +1584,7 @@ type ClusterClassStatusVariableDefinition struct {
15841584
// top-level object defined in the schema. If nested fields are
15851585
// required, this will be specified inside the schema.
15861586
// +required
1587-
Required bool `json:"required"`
1587+
Required *bool `json:"required,omitempty"`
15881588

15891589
// deprecatedV1Beta1Metadata is the metadata of a variable.
15901590
// It can be used to add additional data for higher level tools to

‎api/core/v1beta2/zz_generated.deepcopy.go

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/core/v1beta2/zz_generated.openapi.go

Lines changed: 0 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/controllers/clusterclass/clusterclass_controller.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,9 @@ func (r *Reconciler) reconcileVariables(ctx context.Context, s *scope) (ctrl.Res
320320
v1beta2Variables := make([]clusterv1.ClusterClassVariable, 0, len(resp.Variables))
321321
for _, variable := range resp.Variables {
322322
v := clusterv1.ClusterClassVariable{}
323+
// Note: This conversion func converts Required == false to &false.
324+
// This is intended as the Required field is required (so nil would not be valid)
325+
// Accordingly we don't have to drop Required == &false in dropFalsePtrBool() below.
323326
if err := clusterv1beta1.Convert_v1beta1_ClusterClassVariable_To_v1beta2_ClusterClassVariable(&variable, &v, nil); err != nil {
324327
errs = append(errs, errors.Errorf("failed to convert variable %s to v1beta2", variable.Name))
325328
continue
@@ -411,7 +414,7 @@ func addDefinitionToExistingStatusVariable(variable clusterv1.ClusterClassVariab
411414
// If definitions already conflict, no need to check.
412415
if !ptr.Deref(combinedVariable.DefinitionsConflict, false) {
413416
currentDefinition := combinedVariable.Definitions[0]
414-
if currentDefinition.Required != newVariableDefinition.Required ||
417+
if ptr.Deref(currentDefinition.Required, false) != ptr.Deref(newVariableDefinition.Required, false) ||
415418
!reflect.DeepEqual(dropFalsePtrBool(&currentDefinition.Schema.OpenAPIV3Schema), dropFalsePtrBool(&newVariableDefinition.Schema.OpenAPIV3Schema)) ||
416419
!reflect.DeepEqual(currentDefinition.DeprecatedV1Beta1Metadata, newVariableDefinition.DeprecatedV1Beta1Metadata) {
417420
combinedVariable.DefinitionsConflict = ptr.To(true)

‎internal/controllers/clusterclass/clusterclass_controller_test.go

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,16 @@ func TestClusterClassReconciler_reconcile(t *testing.T) {
108108
WithVariables(
109109
clusterv1.ClusterClassVariable{
110110
Name: "hdd",
111-
Required: true,
111+
Required: ptr.To(true),
112112
Schema: clusterv1.VariableSchema{
113113
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
114114
Type: "string",
115115
},
116116
},
117117
},
118118
clusterv1.ClusterClassVariable{
119-
Name: "cpu",
119+
Name: "cpu",
120+
Required: ptr.To(false),
120121
Schema: clusterv1.VariableSchema{
121122
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
122123
Type: "integer",
@@ -198,8 +199,11 @@ func assertStatusVariables(actualClusterClass *clusterv1.ClusterClass) error {
198199
if statusVarDefinition.From != clusterv1.VariableDefinitionFromInline {
199200
return errors.Errorf("ClusterClass status variable %s from field does not match. Expected %s. Got %s", statusVar.Name, clusterv1.VariableDefinitionFromInline, statusVarDefinition.From)
200201
}
201-
if specVar.Required != statusVarDefinition.Required {
202-
return errors.Errorf("ClusterClass status variable %s required field does not match. Expecte %v. Got %v", specVar.Name, statusVarDefinition.Required, statusVarDefinition.Required)
202+
if specVar.Required == nil || statusVarDefinition.Required == nil {
203+
return errors.Errorf("ClusterClass spec or status variable %s is nil, expected both to be set", specVar.Name)
204+
}
205+
if *specVar.Required != *statusVarDefinition.Required {
206+
return errors.Errorf("ClusterClass status variable %s required field does not match. Expected %v. Got %v", specVar.Name, statusVarDefinition.Required, statusVarDefinition.Required)
203207
}
204208
if !cmp.Equal(specVar.Schema, statusVarDefinition.Schema) {
205209
return errors.Errorf("ClusterClass status variable %s schema does not match. Expected %v. Got %v", specVar.Name, specVar.Schema, statusVarDefinition.Schema)
@@ -421,7 +425,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
421425
WithVariables(
422426
[]clusterv1.ClusterClassVariable{
423427
{
424-
Name: "cpu",
428+
Name: "cpu",
429+
Required: ptr.To(true),
425430
Schema: clusterv1.VariableSchema{
426431
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
427432
Type: "integer",
@@ -450,7 +455,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
450455
},
451456
},
452457
{
453-
Name: "memory",
458+
Name: "memory",
459+
Required: ptr.To(false),
454460
Schema: clusterv1.VariableSchema{
455461
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
456462
Type: "string",
@@ -479,7 +485,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
479485
Name: "cpu",
480486
Definitions: []clusterv1.ClusterClassStatusVariableDefinition{
481487
{
482-
From: clusterv1.VariableDefinitionFromInline,
488+
From: clusterv1.VariableDefinitionFromInline,
489+
Required: ptr.To(true),
483490
Schema: clusterv1.VariableSchema{
484491
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
485492
Type: "integer",
@@ -514,7 +521,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
514521
Name: "memory",
515522
Definitions: []clusterv1.ClusterClassStatusVariableDefinition{
516523
{
517-
From: clusterv1.VariableDefinitionFromInline,
524+
From: clusterv1.VariableDefinitionFromInline,
525+
Required: ptr.To(false),
518526
Schema: clusterv1.VariableSchema{
519527
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
520528
Type: "string",
@@ -550,6 +558,7 @@ func TestReconciler_reconcileVariables(t *testing.T) {
550558
{
551559
Name: "cpu",
552560
// Note: This schema must be exactly equal to the one in clusterClassWithInlineVariables to avoid conflicts.
561+
Required: true,
553562
Schema: clusterv1beta1.VariableSchema{
554563
OpenAPIV3Schema: clusterv1beta1.JSONSchemaProps{
555564
Type: "integer",
@@ -578,7 +587,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
578587
},
579588
},
580589
{
581-
Name: "memory",
590+
Name: "memory",
591+
Required: false,
582592
Schema: clusterv1beta1.VariableSchema{
583593
OpenAPIV3Schema: clusterv1beta1.JSONSchemaProps{
584594
Type: "string",
@@ -621,7 +631,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
621631
DefinitionsConflict: ptr.To(false),
622632
Definitions: []clusterv1.ClusterClassStatusVariableDefinition{
623633
{
624-
From: clusterv1.VariableDefinitionFromInline,
634+
From: clusterv1.VariableDefinitionFromInline,
635+
Required: ptr.To(true),
625636
Schema: clusterv1.VariableSchema{
626637
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
627638
Type: "integer",
@@ -650,7 +661,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
650661
},
651662
},
652663
{
653-
From: "patch1",
664+
From: "patch1",
665+
Required: ptr.To(true),
654666
Schema: clusterv1.VariableSchema{
655667
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
656668
Type: "integer",
@@ -685,7 +697,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
685697
DefinitionsConflict: ptr.To(false),
686698
Definitions: []clusterv1.ClusterClassStatusVariableDefinition{
687699
{
688-
From: "patch1",
700+
From: "patch1",
701+
Required: ptr.To(false),
689702
Schema: clusterv1.VariableSchema{
690703
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
691704
Type: "string",
@@ -715,7 +728,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
715728
DefinitionsConflict: ptr.To(false),
716729
Definitions: []clusterv1.ClusterClassStatusVariableDefinition{
717730
{
718-
From: clusterv1.VariableDefinitionFromInline,
731+
From: clusterv1.VariableDefinitionFromInline,
732+
Required: ptr.To(false),
719733
Schema: clusterv1.VariableSchema{
720734
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
721735
Type: "string",
@@ -727,7 +741,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
727741
},
728742
},
729743
{
730-
From: "patch1",
744+
From: "patch1",
745+
Required: ptr.To(false),
731746
Schema: clusterv1.VariableSchema{
732747
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
733748
Type: "string",
@@ -861,7 +876,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
861876
DefinitionsConflict: ptr.To(false),
862877
Definitions: []clusterv1.ClusterClassStatusVariableDefinition{
863878
{
864-
From: "patch1",
879+
From: "patch1",
880+
Required: ptr.To(false),
865881
Schema: clusterv1.VariableSchema{
866882
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
867883
Type: "string",
@@ -875,7 +891,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
875891
DefinitionsConflict: ptr.To(false),
876892
Definitions: []clusterv1.ClusterClassStatusVariableDefinition{
877893
{
878-
From: "patch1",
894+
From: "patch1",
895+
Required: ptr.To(false),
879896
Schema: clusterv1.VariableSchema{
880897
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
881898
Type: "object",
@@ -899,7 +916,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
899916
DefinitionsConflict: ptr.To(false),
900917
Definitions: []clusterv1.ClusterClassStatusVariableDefinition{
901918
{
902-
From: "patch1",
919+
From: "patch1",
920+
Required: ptr.To(false),
903921
Schema: clusterv1.VariableSchema{
904922
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
905923
Type: "string",
@@ -929,7 +947,8 @@ func TestReconciler_reconcileVariables(t *testing.T) {
929947
DefinitionsConflict: ptr.To(false),
930948
Definitions: []clusterv1.ClusterClassStatusVariableDefinition{
931949
{
932-
From: "patch1",
950+
From: "patch1",
951+
Required: ptr.To(false),
933952
Schema: clusterv1.VariableSchema{
934953
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
935954
Type: "string",

0 commit comments

Comments
 (0)