Skip to content

Commit 600518f

Browse files
gfreyGereon Freya-hilaly
authored
Fix required marker value handling (#517)
kro will now check the values and handle them accordingly. The truthy values are "true", "yes", and "1". The supported falsey values are "false" and "no". --------- Co-authored-by: Gereon Frey <[email protected]> Co-authored-by: Amine <[email protected]>
1 parent cb19c35 commit 600518f

File tree

2 files changed

+68
-1
lines changed

2 files changed

+68
-1
lines changed

pkg/simpleschema/transform.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,15 @@ func (tf *transformer) applyMarkers(schema *extv1.JSONSchemaProps, markers []*Ma
224224
for _, marker := range markers {
225225
switch marker.MarkerType {
226226
case MarkerTypeRequired:
227-
if parentSchema != nil {
227+
switch isRequired, err := strconv.ParseBool(marker.Value); {
228+
case err != nil:
229+
return fmt.Errorf("failed to parse required marker value: %w", err)
230+
case parentSchema == nil:
231+
return fmt.Errorf("required marker can't be applied; parent schema is nil")
232+
case isRequired:
228233
parentSchema.Required = append(parentSchema.Required, key)
234+
default:
235+
// ignore
229236
}
230237
case MarkerTypeDefault:
231238
var defaultValue []byte

pkg/simpleschema/transform_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@
1515
package simpleschema
1616

1717
import (
18+
"fmt"
1819
"reflect"
20+
"slices"
1921
"testing"
2022

23+
"github.com/stretchr/testify/assert"
2124
extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2225
)
2326

@@ -450,6 +453,28 @@ func TestBuildOpenAPISchema(t *testing.T) {
450453
},
451454
wantErr: false,
452455
},
456+
{
457+
name: "Invalid Required Marker value",
458+
obj: map[string]interface{}{
459+
"data": "string | required=invalid",
460+
},
461+
want: nil,
462+
wantErr: true,
463+
},
464+
{
465+
name: "Required Marker handling",
466+
obj: map[string]interface{}{
467+
"req_1": "string | required=true",
468+
},
469+
want: &extv1.JSONSchemaProps{
470+
Type: "object",
471+
Properties: map[string]extv1.JSONSchemaProps{
472+
"req_1": {Type: "string"},
473+
},
474+
Required: []string{"req_1"},
475+
},
476+
wantErr: false,
477+
},
453478
}
454479

455480
for _, tt := range tests {
@@ -459,13 +484,48 @@ func TestBuildOpenAPISchema(t *testing.T) {
459484
t.Errorf("BuildOpenAPISchema() error = %v, wantErr %v", err, tt.wantErr)
460485
return
461486
}
487+
assert.Equal(t, got, tt.want)
462488
if !reflect.DeepEqual(got, tt.want) {
463489
t.Errorf("BuildOpenAPISchema() = %+v, want %+v", got, tt.want)
464490
}
465491
})
466492
}
467493
}
468494

495+
func TestApplyMarkers_Required(t *testing.T) {
496+
transformer := newTransformer()
497+
498+
tests := []struct {
499+
value string
500+
required bool
501+
err error
502+
}{
503+
{"true", true, nil},
504+
{"True", true, nil},
505+
{"TRUE", true, nil},
506+
{"1", true, nil},
507+
{"false", false, nil},
508+
{"False", false, nil},
509+
{"FALSE", false, nil},
510+
{"invalid", false, fmt.Errorf("failed to parse required marker value: strconv.ParseBool: parsing \"invalid\": invalid syntax")},
511+
{"no", false, fmt.Errorf("failed to parse required marker value: strconv.ParseBool: parsing \"no\": invalid syntax")},
512+
}
513+
for _, tt := range tests {
514+
t.Run(fmt.Sprintf("Required Marker %s", tt.value), func(t *testing.T) {
515+
parentSchema := &extv1.JSONSchemaProps{}
516+
markers := []*Marker{{MarkerType: MarkerTypeRequired, Value: tt.value}}
517+
err := transformer.applyMarkers(nil, markers, "myFieldName", parentSchema)
518+
if err != nil && err.Error() != tt.err.Error() {
519+
t.Errorf("ApplyMarkers() error = %q, expected error %q", err, tt.err)
520+
}
521+
// If there was no error, check if the required field was added to the parent schema.
522+
if err == nil && tt.required != slices.Contains(parentSchema.Required, "myFieldName") {
523+
t.Errorf("ApplyMarkers() = %v, want %v", parentSchema.Required, tt.required)
524+
}
525+
})
526+
}
527+
}
528+
469529
func TestLoadPreDefinedTypes(t *testing.T) {
470530
transformer := newTransformer()
471531

0 commit comments

Comments
 (0)