diff --git a/pkg/generators/openapi.go b/pkg/generators/openapi.go index 65b392654..840fb5392 100644 --- a/pkg/generators/openapi.go +++ b/pkg/generators/openapi.go @@ -81,19 +81,24 @@ func hasOpenAPITagValue(comments []string, value string) bool { return false } -func hasRequiredTag(m *types.Member) bool { - return types.ExtractCommentTags( - "+", m.CommentLines)[tagRequired] != nil -} - // hasOptionalTag returns true if the member has +optional in its comments or // omitempty in its json tags. -func hasOptionalTag(m *types.Member) bool { +func hasOptionalTag(m *types.Member) (bool, error) { hasOptionalCommentTag := types.ExtractCommentTags( "+", m.CommentLines)[tagOptional] != nil - hasOptionalJsonTag := strings.Contains( - reflect.StructTag(m.Tags).Get("json"), "omitempty") - return hasOptionalCommentTag || hasOptionalJsonTag + hasRequiredCommentTag := types.ExtractCommentTags( + "+", m.CommentLines)[tagRequired] != nil + if hasOptionalCommentTag && hasRequiredCommentTag { + return false, fmt.Errorf("member %s cannot be both optional and required", m.Name) + } else if hasRequiredCommentTag { + return false, nil + } else if hasOptionalCommentTag { + return true, nil + } + + // If neither +optional nor +required is present in the comments, + // infer optional from the json tags. + return strings.Contains(reflect.StructTag(m.Tags).Get("json"), "omitempty"), nil } func apiTypeFilterFunc(c *generator.Context, t *types.Type) bool { @@ -323,10 +328,10 @@ func (g openAPITypeWriter) generateMembers(t *types.Type, required []string) ([] if name == "" { continue } - if isOptional, isRequired := hasOptionalTag(&m), hasRequiredTag(&m); isOptional && isRequired { + if isOptional, err := hasOptionalTag(&m); err != nil { klog.Errorf("Error when generating: %v, %v\n", name, m) - return required, fmt.Errorf("member %s of type %s cannot be both optional and required", m.Name, t.Name) - } else if !isOptional || isRequired { + return required, err + } else if !isOptional { required = append(required, name) } if err = g.generateProperty(&m, t); err != nil { diff --git a/pkg/generators/openapi_test.go b/pkg/generators/openapi_test.go index 207328014..87d490c62 100644 --- a/pkg/generators/openapi_test.go +++ b/pkg/generators/openapi_test.go @@ -2187,6 +2187,110 @@ func TestMultilineCELMarkerComments(t *testing.T) { } } +func TestRequired(t *testing.T) { + callErr, funcErr, assert, _, funcBuffer, imports := testOpenAPITypeWriter(t, ` + package foo + + // +k8s:openapi-gen=true + type Blah struct { + // +optional + OptionalField string + + // +required + RequiredField string + + // +required + RequiredPointerField *string `+"`json:\"requiredPointerField,omitempty\"`"+` + + // +optional + OptionalPointerField *string `+"`json:\"optionalPointerField,omitempty\"`"+` + + ImplicitlyRequiredField string + ImplicitlyOptionalField string `+"`json:\"implicitlyOptionalField,omitempty\"`"+` + } + `) + + assert.NoError(funcErr) + assert.NoError(callErr) + assert.ElementsMatch(imports, []string{`foo "base/foo"`, `common "k8s.io/kube-openapi/pkg/common"`, `spec "k8s.io/kube-openapi/pkg/validation/spec"`}) + + if formatted, err := format.Source(funcBuffer.Bytes()); err != nil { + t.Fatalf("%v\n%v", err, string(funcBuffer.Bytes())) + } else { + formatted_expected, ree := format.Source([]byte(`func schema_base_foo_Blah(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "OptionalField": { + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + "RequiredField": { + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + "requiredPointerField": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + "optionalPointerField": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + "ImplicitlyRequiredField": { + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + "implicitlyOptionalField": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + }, + Required: []string{"RequiredField", "requiredPointerField", "ImplicitlyRequiredField"}, + }, + }, + } + } + +`)) + if ree != nil { + t.Fatal(ree) + } + assert.Equal(string(formatted_expected), string(formatted)) + } + + // Show specifying both is an error + callErr, funcErr, assert, _, _, _ = testOpenAPITypeWriter(t, ` + package foo + + // +k8s:openapi-gen=true + type Blah struct { + // +optional + // +required + ConfusingField string + } +`) + assert.NoError(callErr) + assert.ErrorContains(funcErr, "cannot be both optional and required") +} + func TestMarkerCommentsCustomDefsV3(t *testing.T) { callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo diff --git a/pkg/generators/union.go b/pkg/generators/union.go index a0281fe47..487c2ca3f 100644 --- a/pkg/generators/union.go +++ b/pkg/generators/union.go @@ -163,7 +163,7 @@ func parseUnionStruct(t *types.Type) (*union, []error) { if types.ExtractCommentTags("+", m.CommentLines)[tagUnionDiscriminator] != nil { errors = append(errors, u.setDiscriminator(jsonName)...) } else { - if !hasOptionalTag(&m) { + if optional, err := hasOptionalTag(&m); !optional || err != nil { errors = append(errors, fmt.Errorf("union members must be optional: %v.%v", t.Name, m.Name)) } u.addMember(jsonName, m.Name) @@ -194,7 +194,7 @@ func parseUnionMembers(t *types.Type) (*union, []error) { continue } if types.ExtractCommentTags("+", m.CommentLines)[tagUnionDeprecated] != nil { - if !hasOptionalTag(&m) { + if optional, err := hasOptionalTag(&m); !optional || err != nil { errors = append(errors, fmt.Errorf("union members must be optional: %v.%v", t.Name, m.Name)) } u.addMember(jsonName, m.Name)