diff --git a/encoding/jsonschema/constraints.go b/encoding/jsonschema/constraints.go index ceb1b3a0e..d57f54413 100644 --- a/encoding/jsonschema/constraints.go +++ b/encoding/jsonschema/constraints.go @@ -71,62 +71,69 @@ var constraints = []*constraint{ p0("$schema", constraintSchema, allVersions), px("$vocabulary", constraintTODO, vfrom(VersionDraft2019_09)), p4("additionalItems", constraintAdditionalItems, vto(VersionDraft2019_09)), - p4("additionalProperties", constraintAdditionalProperties, allVersions|openAPI), - p3("allOf", constraintAllOf, allVersions|openAPI), - p3("anyOf", constraintAnyOf, allVersions|openAPI), + p4("additionalProperties", constraintAdditionalProperties, allVersions|openAPI|k8sCRD), + p3("allOf", constraintAllOf, allVersions|openAPI|k8sCRD), + p3("anyOf", constraintAnyOf, allVersions|openAPI|k8sCRD), p2("const", constraintConst, vfrom(VersionDraft6)), p2("contains", constraintContains, vfrom(VersionDraft6)), p2("contentEncoding", constraintContentEncoding, vfrom(VersionDraft7)), p2("contentMediaType", constraintContentMediaType, vfrom(VersionDraft7)), px("contentSchema", constraintTODO, vfrom(VersionDraft2019_09)), - p2("default", constraintDefault, allVersions|openAPI), + p2("default", constraintDefault, allVersions|openAPI|k8sCRD), p2("definitions", constraintAddDefinitions, allVersions), p2("dependencies", constraintDependencies, allVersions), px("dependentRequired", constraintTODO, vfrom(VersionDraft2019_09)), px("dependentSchemas", constraintTODO, vfrom(VersionDraft2019_09)), p2("deprecated", constraintDeprecated, vfrom(VersionDraft2019_09)|openAPI), - p2("description", constraintDescription, allVersions|openAPI), + p2("description", constraintDescription, allVersions|openAPI|k8sCRD), px("discriminator", constraintTODO, openAPI), p1("else", constraintElse, vfrom(VersionDraft7)), - p2("enum", constraintEnum, allVersions|openAPI), - px("example", constraintTODO, openAPI), + p2("enum", constraintEnum, allVersions|openAPI|k8sCRD), + px("example", constraintTODO, openAPI|k8sCRD), p2("examples", constraintExamples, vfrom(VersionDraft6)), - p2("exclusiveMaximum", constraintExclusiveMaximum, allVersions|openAPI), - p2("exclusiveMinimum", constraintExclusiveMinimum, allVersions|openAPI), - px("externalDocs", constraintTODO, openAPI), - p1("format", constraintFormat, allVersions|openAPI), + p2("exclusiveMaximum", constraintExclusiveMaximum, allVersions|openAPI|k8sCRD), + p2("exclusiveMinimum", constraintExclusiveMinimum, allVersions|openAPI|k8sCRD), + px("externalDocs", constraintTODO, openAPI|k8sCRD), + p1("format", constraintFormat, allVersions|openAPI|k8sCRD), p1("id", constraintID, vto(VersionDraft4)), p1("if", constraintIf, vfrom(VersionDraft7)), - p2("items", constraintItems, allVersions|openAPI), + p2("items", constraintItems, allVersions|openAPI|k8sCRD), p1("maxContains", constraintMaxContains, vfrom(VersionDraft2019_09)), - p2("maxItems", constraintMaxItems, allVersions|openAPI), - p2("maxLength", constraintMaxLength, allVersions|openAPI), - p2("maxProperties", constraintMaxProperties, allVersions|openAPI), - p3("maximum", constraintMaximum, allVersions|openAPI), + p2("maxItems", constraintMaxItems, allVersions|openAPI|k8sCRD), + p2("maxLength", constraintMaxLength, allVersions|openAPI|k8sCRD), + p2("maxProperties", constraintMaxProperties, allVersions|openAPI|k8sCRD), + p3("maximum", constraintMaximum, allVersions|openAPI|k8sCRD), p1("minContains", constraintMinContains, vfrom(VersionDraft2019_09)), - p2("minItems", constraintMinItems, allVersions|openAPI), - p2("minLength", constraintMinLength, allVersions|openAPI), - p1("minProperties", constraintMinProperties, allVersions|openAPI), - p3("minimum", constraintMinimum, allVersions|openAPI), - p2("multipleOf", constraintMultipleOf, allVersions|openAPI), - p3("not", constraintNot, allVersions|openAPI), - p2("nullable", constraintNullable, openAPI), - p3("oneOf", constraintOneOf, allVersions|openAPI), - p2("pattern", constraintPattern, allVersions|openAPI), + p2("minItems", constraintMinItems, allVersions|openAPI|k8sCRD), + p2("minLength", constraintMinLength, allVersions|openAPI|k8sCRD), + p1("minProperties", constraintMinProperties, allVersions|openAPI|k8sCRD), + p3("minimum", constraintMinimum, allVersions|openAPI|k8sCRD), + p2("multipleOf", constraintMultipleOf, allVersions|openAPI|k8sCRD), + p3("not", constraintNot, allVersions|openAPI|k8sCRD), + p2("nullable", constraintNullable, openAPI|k8sCRD), + p3("oneOf", constraintOneOf, allVersions|openAPI|k8sCRD), + p2("pattern", constraintPattern, allVersions|openAPI|k8sCRD), p3("patternProperties", constraintPatternProperties, allVersions), p2("prefixItems", constraintPrefixItems, vfrom(VersionDraft2020_12)), - p2("properties", constraintProperties, allVersions|openAPI), + p2("properties", constraintProperties, allVersions|openAPI|k8sCRD), p2("propertyNames", constraintPropertyNames, vfrom(VersionDraft6)), px("readOnly", constraintTODO, vfrom(VersionDraft7)|openAPI), - p3("required", constraintRequired, allVersions|openAPI), + p3("required", constraintRequired, allVersions|openAPI|k8sCRD), p1("then", constraintThen, vfrom(VersionDraft7)), - p2("title", constraintTitle, allVersions|openAPI), - p2("type", constraintType, allVersions|openAPI), + p2("title", constraintTitle, allVersions|openAPI|k8sCRD), + p2("type", constraintType, allVersions|openAPI|k8sCRD), px("unevaluatedItems", constraintTODO, vfrom(VersionDraft2019_09)), px("unevaluatedProperties", constraintTODO, vfrom(VersionDraft2019_09)), - p2("uniqueItems", constraintUniqueItems, allVersions|openAPI), + p2("uniqueItems", constraintUniqueItems, allVersions|openAPI|k8sCRD), px("writeOnly", constraintTODO, vfrom(VersionDraft7)|openAPI), px("xml", constraintTODO, openAPI), + px("x-kubernetes-embedded-resource", constraintTODO, k8sCRD), + p2("x-kubernetes-int-or-string", constraintIntOrString, k8sCRD), + px("x-kubernetes-list-map-keys", constraintTODO, k8sCRD), + px("x-kubernetes-list-type", constraintTODO, k8sCRD), + px("x-kubernetes-map-type", constraintTODO, k8sCRD), + p2("x-kubernetes-preserve-unknown-fields", constraintPreserveUnknownFields, k8sCRD), + px("x-kubernetes-validations", constraintTODO, k8sCRD), } // px represents a TODO constraint that we haven't decided on a phase for yet. diff --git a/encoding/jsonschema/constraints_array.go b/encoding/jsonschema/constraints_array.go index 785adbc2a..ae83b902e 100644 --- a/encoding/jsonschema/constraints_array.go +++ b/encoding/jsonschema/constraints_array.go @@ -106,6 +106,7 @@ func constraintItems(key string, n cue.Value, s *state) { elem := s.schema(n) ast.SetRelPos(elem, token.NoRelPos) s.add(n, arrayType, ast.NewList(&ast.Ellipsis{Type: elem})) + s.hasItems = true case cue.ListKind: if !vto(VersionDraft2019_09).contains(s.schemaVersion) { @@ -157,6 +158,10 @@ func constraintMinItems(key string, n cue.Value, s *state) { func constraintUniqueItems(key string, n cue.Value, s *state) { if s.boolValue(n) { + if s.schemaVersion == VersionKubernetesCRD { + s.errf(n, "cannot set uniqueItems to true in a CRD schema") + return + } list := s.addImport(n, "list") s.add(n, arrayType, ast.NewCall(ast.NewSel(list, "UniqueItems"))) } diff --git a/encoding/jsonschema/constraints_combinator.go b/encoding/jsonschema/constraints_combinator.go index 07ece912a..087ac6b78 100644 --- a/encoding/jsonschema/constraints_combinator.go +++ b/encoding/jsonschema/constraints_combinator.go @@ -33,7 +33,7 @@ func constraintAllOf(key string, n cue.Value, s *state) { } a := make([]ast.Expr, 0, len(items)) for _, v := range items { - x, sub := s.schemaState(v, s.allowedTypes) + x, sub := s.schemaState(v, s.allowedTypes, nil) s.allowedTypes &= sub.allowedTypes if sub.hasConstraints { // This might seem a little odd, since the actual @@ -79,7 +79,7 @@ func constraintAnyOf(key string, n cue.Value, s *state) { } a := make([]ast.Expr, 0, len(items)) for _, v := range items { - x, sub := s.schemaState(v, s.allowedTypes) + x, sub := s.schemaState(v, s.allowedTypes, nil) if sub.allowedTypes == 0 { // Nothing is allowed; omit. continue @@ -123,7 +123,7 @@ func constraintOneOf(key string, n cue.Value, s *state) { } a := make([]ast.Expr, 0, len(items)) for _, v := range items { - x, sub := s.schemaState(v, s.allowedTypes) + x, sub := s.schemaState(v, s.allowedTypes, nil) if sub.allowedTypes == 0 { // Nothing is allowed; omit continue @@ -198,14 +198,14 @@ func constraintIfThenElse(s *state) { return } var ifExpr, thenExpr, elseExpr ast.Expr - ifExpr, ifSub := s.schemaState(s.ifConstraint, s.allowedTypes) + ifExpr, ifSub := s.schemaState(s.ifConstraint, s.allowedTypes, nil) if hasThen { // The allowed types of the "then" constraint are constrained both // by the current constraints and the "if" constraint. - thenExpr, _ = s.schemaState(s.thenConstraint, s.allowedTypes&ifSub.allowedTypes) + thenExpr, _ = s.schemaState(s.thenConstraint, s.allowedTypes&ifSub.allowedTypes, nil) } if hasElse { - elseExpr, _ = s.schemaState(s.elseConstraint, s.allowedTypes) + elseExpr, _ = s.schemaState(s.elseConstraint, s.allowedTypes, nil) } if thenExpr == nil { thenExpr = top() diff --git a/encoding/jsonschema/constraints_format.go b/encoding/jsonschema/constraints_format.go index c6c578c9a..8fda962fa 100644 --- a/encoding/jsonschema/constraints_format.go +++ b/encoding/jsonschema/constraints_format.go @@ -26,37 +26,55 @@ type formatFuncInfo struct { f func(n cue.Value, s *state) } +// For reference, the Kubernetes-related format strings +// are defined here: +// https://github.com/kubernetes/apiextensions-apiserver/blob/aca9073a80bee92a0b77741b9c7ad444c49fe6be/pkg/apis/apiextensions/v1beta1/types_jsonschema.go#L73 + var formatFuncs = sync.OnceValue(func() map[string]formatFuncInfo { return map[string]formatFuncInfo{ "binary": {openAPI, formatTODO}, - "byte": {openAPI, formatTODO}, + "bsonobjectid": {k8sCRD, formatTODO}, + "byte|k8sCRD": {openAPI, formatTODO}, + "cidr": {k8sCRD, formatTODO}, + "creditcard": {k8sCRD, formatTODO}, "data": {openAPI, formatTODO}, - "date": {vfrom(VersionDraft7) | openAPI, formatDate}, + "date": {vfrom(VersionDraft7) | openAPI | k8sCRD, formatDate}, "date-time": {allVersions | openAPI, formatDateTime}, + "datetime": {k8sCRD, formatDateTime}, "double": {openAPI, formatTODO}, - "duration": {vfrom(VersionDraft2019_09), formatTODO}, - "email": {allVersions | openAPI, formatTODO}, + "duration": {vfrom(VersionDraft2019_09) | k8sCRD, formatTODO}, + "email": {allVersions | openAPI | k8sCRD, formatTODO}, "float": {openAPI, formatTODO}, - "hostname": {allVersions | openAPI, formatTODO}, + "hexcolor": {k8sCRD, formatTODO}, + "hostname": {allVersions | openAPI | k8sCRD, formatTODO}, "idn-email": {vfrom(VersionDraft7), formatTODO}, "idn-hostname": {vfrom(VersionDraft7), formatTODO}, "int32": {openAPI, formatInt32}, "int64": {openAPI, formatInt64}, - "ipv4": {allVersions | openAPI, formatTODO}, - "ipv6": {allVersions | openAPI, formatTODO}, + "ipv4": {allVersions | openAPI | k8sCRD, formatTODO}, + "ipv6": {allVersions | openAPI | k8sCRD, formatTODO}, "iri": {vfrom(VersionDraft7), formatURI}, "iri-reference": {vfrom(VersionDraft7), formatURIReference}, + "isbn": {k8sCRD, formatTODO}, + "isbn10": {k8sCRD, formatTODO}, + "isbn13": {k8sCRD, formatTODO}, "json-pointer": {vfrom(VersionDraft6), formatTODO}, - "password": {openAPI, formatTODO}, + "mac": {k8sCRD, formatTODO}, + "password": {openAPI | k8sCRD, formatTODO}, "regex": {vfrom(VersionDraft7), formatRegex}, "relative-json-pointer": {vfrom(VersionDraft7), formatTODO}, + "rgbcolor": {k8sCRD, formatTODO}, + "ssn": {k8sCRD, formatTODO}, "time": {vfrom(VersionDraft7), formatTODO}, // TODO we should probably disallow non-ASCII URIs (IRIs) but // this is good enough for now. - "uri": {allVersions | openAPI, formatURI}, + "uri": {allVersions | openAPI | k8sCRD, formatURI}, "uri-reference": {vfrom(VersionDraft6), formatURIReference}, "uri-template": {vfrom(VersionDraft6), formatTODO}, - "uuid": {vfrom(VersionDraft2019_09), formatTODO}, + "uuid": {vfrom(VersionDraft2019_09) | k8sCRD, formatTODO}, + "uuid3": {k8sCRD, formatTODO}, + "uuid4": {k8sCRD, formatTODO}, + "uuid5": {k8sCRD, formatTODO}, } }) @@ -77,13 +95,13 @@ func constraintFormat(key string, n cue.Value, s *state) { // we want unknown formats to be ignored even when StrictFeatures // is enabled, and StrictKeywords is closest to what we want. // Perhaps we should have a "lint" mode? - if s.cfg.StrictKeywords && s.schemaVersion != VersionOpenAPI { + if s.cfg.StrictKeywords && !openAPILike.contains(s.schemaVersion) { s.errf(n, "unknown format %q", formatStr) } return } if !finfo.versions.contains(s.schemaVersion) { - if s.cfg.StrictKeywords && s.schemaVersion != VersionOpenAPI { + if s.cfg.StrictKeywords && !openAPILike.contains(s.schemaVersion) { s.errf(n, "format %q is not recognized in schema version %v", formatStr, s.schemaVersion) } return diff --git a/encoding/jsonschema/constraints_generic.go b/encoding/jsonschema/constraints_generic.go index e220c81f2..f7f5852cb 100644 --- a/encoding/jsonschema/constraints_generic.go +++ b/encoding/jsonschema/constraints_generic.go @@ -169,6 +169,15 @@ func constraintTitle(key string, n cue.Value, s *state) { s.title, _ = s.strValue(n) } +func constraintIntOrString(key string, n cue.Value, s *state) { + // See x-kubernetes-int-or-string in + // https://kubernetes.io/docs/reference/kubernetes-api/extend-resources/custom-resource-definition-v1/#JSONSchemaProps. + s.setTypeUsed(n, stringType) + s.setTypeUsed(n, numType) + s.add(n, numType, ast.NewIdent("int")) + s.allowedTypes &= cue.StringKind | cue.IntKind +} + func constraintType(key string, n cue.Value, s *state) { var types cue.Kind set := func(n cue.Value) { @@ -197,6 +206,9 @@ func constraintType(key string, n cue.Value, s *state) { case "array": types |= cue.ListKind s.setTypeUsed(n, arrayType) + // For OpenAPI, specifically keep track of whether type is array + // so we can mandate the "items" keyword. + s.isArray = true case "object": types |= cue.StructKind s.setTypeUsed(n, objectType) @@ -210,6 +222,12 @@ func constraintType(key string, n cue.Value, s *state) { case cue.StringKind: set(n) case cue.ListKind: + if openAPILike.contains(s.schemaVersion) { + // From https://spec.openapis.org/oas/v3.0.3.html#properties: + // "Value MUST be a string. Multiple types via an array are not supported." + s.errf(n, `value of "type" must be a string in %v`, s.schemaVersion) + return + } for i, _ := n.List(); i.Next(); { set(i.Value()) } diff --git a/encoding/jsonschema/constraints_object.go b/encoding/jsonschema/constraints_object.go index c5a692129..f8f2814be 100644 --- a/encoding/jsonschema/constraints_object.go +++ b/encoding/jsonschema/constraints_object.go @@ -23,9 +23,35 @@ import ( // Object constraints +func constraintPreserveUnknownFields(key string, n cue.Value, s *state) { + // x-kubernetes-preserve-unknown-fields stops the API server decoding + // step from pruning fields which are not specified in the validation + // schema. This affects fields recursively, but switches back to normal + // pruning behaviour if nested properties or additionalProperties are + // specified in the schema. This can either be true or undefined. False + // is forbidden. + // Note: by experimentation, "nested properties" means "within a schema + // within a nested property" not "within a schema that has the properties keyword". + if !s.boolValue(n) { + s.errf(n, "x-kubernetes-preserve-unknown-fields value may not be false") + return + } + // TODO check that it's specified on an object type. This requires + // either setting a bool (hasPreserveUnknownFields?) and checking + // later or making a new phase and placing this after "type" but + // before "allOf", because it's important that this value be + // passed down recursively to allOf and friends. + s.preserveUnknownFields = true +} + func constraintAdditionalProperties(key string, n cue.Value, s *state) { switch n.Kind() { case cue.BoolKind: + closeStruct := !s.boolValue(n) + if s.schemaVersion == VersionKubernetesCRD && closeStruct { + s.errf(n, "additionalProperties may not be set to false in a CRD schema") + return + } s.closeStruct = !s.boolValue(n) _ = s.object(n) @@ -41,15 +67,20 @@ func constraintAdditionalProperties(key string, n cue.Value, s *state) { } // [!~(properties|patternProperties)]: schema existing := append(s.patterns, excludeFields(obj.Elts)...) + expr, _ := s.schemaState(n, allTypes, func(s *state) { + s.preserveUnknownFields = false + }) f := internal.EmbedStruct(ast.NewStruct(&ast.Field{ Label: ast.NewList(ast.NewBinExpr(token.AND, existing...)), - Value: s.schema(n), + Value: expr, })) obj.Elts = append(obj.Elts, f) default: s.errf(n, `value of "additionalProperties" must be an object or boolean`) + return } + s.hasAdditionalProperties = true } func constraintDependencies(key string, n cue.Value, s *state) { @@ -118,7 +149,9 @@ func constraintProperties(key string, n cue.Value, s *state) { s.processMap(n, func(key string, n cue.Value) { // property?: value name := ast.NewString(key) - expr, state := s.schemaState(n, allTypes) + expr, state := s.schemaState(n, allTypes, func(s *state) { + s.preserveUnknownFields = false + }) f := &ast.Field{Label: name, Value: expr} if doc := state.comment(); doc != nil { ast.SetComments(f, []*ast.CommentGroup{doc}) @@ -139,11 +172,12 @@ func constraintProperties(key string, n cue.Value, s *state) { } obj.Elts = append(obj.Elts, f) }) + s.hasProperties = true } func constraintPropertyNames(key string, n cue.Value, s *state) { // [=~pattern]: _ - if names, _ := s.schemaState(n, cue.StringKind); !isTop(names) { + if names, _ := s.schemaState(n, cue.StringKind, nil); !isTop(names) { x := ast.NewStruct(ast.NewList(names), top()) s.add(n, objectType, x) } diff --git a/encoding/jsonschema/decode.go b/encoding/jsonschema/decode.go index e23cd3382..3a6a7d0fc 100644 --- a/encoding/jsonschema/decode.go +++ b/encoding/jsonschema/decode.go @@ -182,7 +182,7 @@ func (d *decoder) decode(v cue.Value) *ast.File { // containing a field for each definition. constraintAddDefinitions("schemas", defsRoot, root) } else { - expr, state := root.schemaState(v, allTypes) + expr, state := root.schemaState(v, allTypes, nil) if state.allowedTypes == 0 { root.errf(v, "constraints are not possible to satisfy") return nil @@ -470,11 +470,6 @@ type state struct { exclusiveMin bool // For OpenAPI and legacy support. exclusiveMax bool // For OpenAPI and legacy support. - // Keep track of whether a $ref keyword is present, - // because pre-2019-09 schemas ignore sibling keywords - // to $ref. - hasRefKeyword bool - // isRoot holds whether this state is at the root // of the schema. isRoot bool @@ -507,6 +502,27 @@ type state struct { // // "items": [] listItemsIsArray bool + + // The following fields are used when the version is + // [VersionKubernetesCRD] to check that "properties" and + // "additionalProperties" may not be specified together. + hasProperties bool + hasAdditionalProperties bool + + // Keep track of whether "items" and "type": "array" have been specified, because + // in OpenAPI it's mandatory when "type" is "array". + hasItems bool + isArray bool + + // Keep track of whether a $ref keyword is present, + // because pre-2019-09 schemas ignore sibling keywords + // to $ref. + hasRefKeyword bool + + // Keep track of whether we're preserving existing fields, + // which is preserved recursively by default, and is + // reset within properties or additionalProperties. + preserveUnknownFields bool } // schemaInfo holds information about a schema @@ -549,12 +565,24 @@ func (s *state) object(n cue.Value) *ast.StructLit { } func (s *state) finalizeObject() { + if s.obj == nil && s.schemaVersion == VersionKubernetesCRD && (s.allowedTypes&cue.StructKind) != 0 && !s.preserveUnknownFields { + // With regular JSON Schema, we'll never get a closed struct + // unless additionalProperties is specified, which invokes + // s.object. However, that's not true of CRDs which are closed + // (or at least non-preserving) by default, so make sure there's + // an object. + _ = s.object(s.pos) + } if s.obj == nil { return } - var e ast.Expr - if s.closeStruct { + if s.closeStruct || (s.schemaVersion == VersionKubernetesCRD && !s.preserveUnknownFields) { + // TODO in CRDs, this isn't quite right, as the + // structs aren't _really_ closed: it's just that + // unknown fields are discarded when persisting + // data. But we don't really have a way of representing + // that in CUE yet, so close will have to do. e = ast.NewCall(ast.NewIdent("close"), s.obj) } else { s.obj.Elts = append(s.obj.Elts, &ast.Ellipsis{}) @@ -759,14 +787,17 @@ func (s schemaInfo) comment() *ast.CommentGroup { } func (s *state) schema(n cue.Value) ast.Expr { - expr, _ := s.schemaState(n, allTypes) + expr, _ := s.schemaState(n, allTypes, nil) return expr } // schemaState returns a new state value derived from s. // n holds the JSONSchema node to translate to a schema. // types holds the set of possible types that the value can hold. -func (s0 *state) schemaState(n cue.Value, types cue.Kind) (expr ast.Expr, info schemaInfo) { +// +// If init is not nil, it is called on the newly created state value +// before doing anything else. +func (s0 *state) schemaState(n cue.Value, types cue.Kind, init func(*state)) (expr ast.Expr, info schemaInfo) { s := &state{ up: s0, schemaInfo: schemaInfo{ @@ -774,9 +805,13 @@ func (s0 *state) schemaState(n cue.Value, types cue.Kind) (expr ast.Expr, info s allowedTypes: types, knownTypes: allTypes, }, - decoder: s0.decoder, - pos: n, - isRoot: s0.isRoot && n == s0.pos, + decoder: s0.decoder, + pos: n, + isRoot: s0.isRoot && n == s0.pos, + preserveUnknownFields: s0.preserveUnknownFields, + } + if init != nil { + init(s) } defer func() { // Perhaps replace the schema expression with a reference. @@ -804,19 +839,19 @@ func (s0 *state) schemaState(n cue.Value, types cue.Kind) (expr ast.Expr, info s // is >=2019-19 because $schema could be used to change the version. s.hasRefKeyword = true } - if strings.HasPrefix(key, "x-") { - // A keyword starting with a leading x- is clearly - // not intended to be a valid keyword, and is explicitly - // allowed by OpenAPI. It seems reasonable that - // this is not an error even with StrictKeywords enabled. - return - } // Convert each constraint into a either a value or a functor. c := constraintMap[key] if c == nil { + if strings.HasPrefix(key, "x-") { + // A keyword starting with a leading x- is clearly + // not intended to be a valid keyword, and is explicitly + // allowed by OpenAPI. It seems reasonable that + // this is not an error even with StrictKeywords enabled. + return + } if pass == 0 && s.cfg.StrictKeywords { // TODO: value is not the correct position, albeit close. Fix this. - s.warnf(value.Pos(), "unknown keyword %q", key) + s.warnUnrecognizedKeyword(key, value, "unknown keyword %q", key) } return } @@ -824,9 +859,7 @@ func (s0 *state) schemaState(n cue.Value, types cue.Kind) (expr ast.Expr, info s return } if !c.versions.contains(s.schemaVersion) { - if s.cfg.StrictKeywords { - s.warnf(value.Pos(), "keyword %q is not supported in JSON schema version %v", key, s.schemaVersion) - } + s.warnUnrecognizedKeyword(key, value, "keyword %q is not supported in JSON schema version %v", key, s.schemaVersion) return } if pass > 0 && !vfrom(VersionDraft2019_09).contains(s.schemaVersion) && s.hasRefKeyword && key != "$ref" { @@ -838,9 +871,7 @@ func (s0 *state) schemaState(n cue.Value, types cue.Kind) (expr ast.Expr, info s // ignore keywords alongside $ref, but $ref says we should ignore the $schema // keyword itself). We could make that situation an explicit error, but other // implementations don't, and it would require an entire extra pass just to do so. - if s.cfg.StrictKeywords { - s.warnf(value.Pos(), "ignoring keyword %q alongside $ref", key) - } + s.warnUnrecognizedKeyword(key, value, "ignoring keyword %q alongside $ref", key) return } c.fn(key, value, s) @@ -851,12 +882,37 @@ func (s0 *state) schemaState(n cue.Value, types cue.Kind) (expr ast.Expr, info s s.ensureDefinition(s.pos) } constraintIfThenElse(s) + if s.schemaVersion == VersionKubernetesCRD { + if s.hasProperties && s.hasAdditionalProperties { + s.errf(n, "additionalProperties may not be combined with properties in %v", s.schemaVersion) + } + } + if openAPILike.contains(s.schemaVersion) { + if s.isArray && !s.hasItems { + // From https://github.com/OAI/OpenAPI-Specification/blob/3.0.0/versions/3.0.0.md#schema-object + // "`items` MUST be present if the `type` is `array`." + s.errf(n, `"items" must be present when the "type" is "array" in %v`, s.schemaVersion) + } + } schemaExpr := s.finalize() s.schemaInfo.hasConstraints = s.hasConstraints() return schemaExpr, s.schemaInfo } +func (s *state) warnUnrecognizedKeyword(key string, n cue.Value, msg string, args ...any) { + if !s.cfg.StrictKeywords { + return + } + if openAPILike.contains(s.schemaVersion) && strings.HasPrefix(key, "x-") { + // Unimplemented x- keywords are allowed even with strict keywords + // under OpenAPI-like versions, because those versions enable + // strict keywords by default. + return + } + s.errf(n, msg, args...) +} + // maybeDefine checks whether we might need a definition // for n given its actual schema syntax expression. If // it does, it creates the definition as appropriate and returns diff --git a/encoding/jsonschema/decode_test.go b/encoding/jsonschema/decode_test.go index 3b8cdd515..e1a90c6bf 100644 --- a/encoding/jsonschema/decode_test.go +++ b/encoding/jsonschema/decode_test.go @@ -83,7 +83,8 @@ func TestDecode(t *testing.T) { // tag, so when we update the default version, they could break. // We should probably change most of the tests to use an explicit $schema // field apart from when we're explicitly testing the default version logic. - if versStr == "openapi" { + switch versStr { + case "openapi": // OpenAPI doesn't have a JSON Schema URI so it gets a special case. cfg.DefaultVersion = jsonschema.VersionOpenAPI cfg.Root = "#/components/schemas/" @@ -92,7 +93,14 @@ func TestDecode(t *testing.T) { // Just for testing: does not validate the path. return []ast.Label{ast.NewIdent("#" + a[len(a)-1])}, nil } - } else { + case "crd": + // Similar for CRDs as OpenAPI. + cfg.DefaultVersion = jsonschema.VersionKubernetesCRD + // Default to the first version; can be overridden with #root. + cfg.Root = "#/spec/versions/0/schema/openAPIV3Schema" + cfg.StrictKeywords = true // CRDs always use strict keywords + cfg.SingleRoot = true + default: vers, err := jsonschema.ParseVersion(versStr) qt.Assert(t, qt.IsNil(err)) cfg.DefaultVersion = vers @@ -105,7 +113,9 @@ func TestDecode(t *testing.T) { cfg.StrictKeywords = cfg.StrictKeywords || t.HasTag("strictKeywords") cfg.AllowNonExistentRoot = t.HasTag("allowNonExistentRoot") cfg.StrictFeatures = t.HasTag("strictFeatures") - cfg.SingleRoot = t.HasTag("singleRoot") + if t.HasTag("singleRoot") { + cfg.SingleRoot = true + } cfg.PkgName, _ = t.Value("pkgName") ctx := t.CueContext() diff --git a/encoding/jsonschema/testdata/txtar/crd_intorstring.txtar b/encoding/jsonschema/testdata/txtar/crd_intorstring.txtar new file mode 100644 index 000000000..460c9a677 --- /dev/null +++ b/encoding/jsonschema/testdata/txtar/crd_intorstring.txtar @@ -0,0 +1,34 @@ +#version: crd + +-- schema.yaml -- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + # name must be in the form: . + name: myapps.example.com +spec: + # group name to use for REST API: /apis// + group: example.com + scope: Namespaced + names: + # kind is normally the CamelCased singular type. + kind: MyApp + # singular name to be used as an alias on the CLI + singular: myapp + # plural name in the URL: /apis/// + plural: myapps + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + x-kubernetes-int-or-string: true + +-- out/decode/extract -- +close({ + spec?: int | string +}) diff --git a/encoding/jsonschema/testdata/txtar/crd_intorstring_with_anyof.txtar b/encoding/jsonschema/testdata/txtar/crd_intorstring_with_anyof.txtar new file mode 100644 index 000000000..eb5bd30ee --- /dev/null +++ b/encoding/jsonschema/testdata/txtar/crd_intorstring_with_anyof.txtar @@ -0,0 +1,40 @@ +x-kubernetes-int-or-string can be combined with an anyOf +TODO the CUE for this, though technically correct, could use improvement. + +#version: crd + +-- schema.yaml -- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + # name must be in the form: . + name: myapps.example.com +spec: + # group name to use for REST API: /apis// + group: example.com + scope: Namespaced + names: + # kind is normally the CamelCased singular type. + kind: MyApp + # singular name to be used as an alias on the CLI + singular: myapp + # plural name in the URL: /apis/// + plural: myapps + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + x-kubernetes-int-or-string: true + anyOf: + - type: integer + - type: string + +-- out/decode/extract -- +close({ + spec?: matchN(>=1, [int, string]) & (int | string) +}) diff --git a/encoding/jsonschema/testdata/txtar/crd_preserve_unknown.txtar b/encoding/jsonschema/testdata/txtar/crd_preserve_unknown.txtar new file mode 100644 index 000000000..0914532e9 --- /dev/null +++ b/encoding/jsonschema/testdata/txtar/crd_preserve_unknown.txtar @@ -0,0 +1,99 @@ +#version: crd + +-- schema.json -- +{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "myapps.example.com" + }, + "spec": { + "group": "example.com", + "scope": "Namespaced", + "names": { + "kind": "MyApp", + "singular": "myapp", + "plural": "myapps" + }, + "versions": [ + { + "name": "v1", + "served": true, + "storage": true, + "schema": { + "openAPIV3Schema": { + "type": "object", + "properties": { + "alpha": { + "type": "string" + }, + "beta": { + "type": "number" + }, + "preserving": { + "type": "object", + "x-kubernetes-preserve-unknown-fields": true, + "properties": { + "preserving": { + "type": "object", + "x-kubernetes-preserve-unknown-fields": true + }, + "pruning": { + "type": "object" + }, + "pruning2": { + "type": "object", + "additionalProperties": { + "type": "object", + "properties": { + "preserved": { + "type": "string" + } + } + } + } + } + }, + "pruning": { + "type": "object", + "properties": { + "preserving": { + "type": "object", + "x-kubernetes-preserve-unknown-fields": true + }, + "pruning": { + "type": "object" + } + } + } + }, + "x-kubernetes-preserve-unknown-fields": true + } + } + } + ] + } +} + +-- out/decode/extract -- +alpha?: string +beta?: number +preserving?: { + preserving?: { + ... + } + pruning?: close({}) + pruning2?: close({ + [string]: close({ + preserved?: string + }) + }) + ... +} +pruning?: close({ + preserving?: { + ... + } + pruning?: close({}) +}) +... diff --git a/encoding/jsonschema/testdata/txtar/crd_properties_with_additionalproperties.txtar b/encoding/jsonschema/testdata/txtar/crd_properties_with_additionalproperties.txtar new file mode 100644 index 000000000..d49322840 --- /dev/null +++ b/encoding/jsonschema/testdata/txtar/crd_properties_with_additionalproperties.txtar @@ -0,0 +1,45 @@ +#version: crd + +-- schema.json -- +{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "myapps.example.com" + }, + "spec": { + "group": "example.com", + "scope": "Namespaced", + "names": { + "kind": "MyApp", + "singular": "myapp", + "plural": "myapps" + }, + "versions": [ + { + "name": "v1", + "served": true, + "storage": true, + "schema": { + "openAPIV3Schema": { + "type": "object", + "properties": { + "spec": { + "type": "object", + "x-kubernetes-preserve-unknown-fields": true + } + }, + "additionalProperties": { + "type": "string" + } + } + } + } + ] + } +} + +-- out/decode/extract -- +ERROR: +additionalProperties may not be combined with properties in Kubernetes CRD: + schema.json:21:21 diff --git a/encoding/jsonschema/testdata/txtar/crd_ref.txtar b/encoding/jsonschema/testdata/txtar/crd_ref.txtar new file mode 100644 index 000000000..cf2abc6b8 --- /dev/null +++ b/encoding/jsonschema/testdata/txtar/crd_ref.txtar @@ -0,0 +1,36 @@ +CRDs do not allow $ref, so test for that. + +#version: crd + +-- schema.yaml -- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + # name must be in the form: . + name: myapps.example.com +spec: + # group name to use for REST API: /apis// + group: example.com + scope: Namespaced + names: + # kind is normally the CamelCased singular type. + kind: MyApp + # singular name to be used as an alias on the CLI + singular: myapp + # plural name in the URL: /apis/// + plural: myapps + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + $ref: "#/something" + +-- out/decode/extract -- +ERROR: +keyword "$ref" is not supported in JSON schema version Kubernetes CRD: + schema.yaml:26:13 diff --git a/encoding/jsonschema/testdata/txtar/crd_simple.txtar b/encoding/jsonschema/testdata/txtar/crd_simple.txtar new file mode 100644 index 000000000..7a65c0bef --- /dev/null +++ b/encoding/jsonschema/testdata/txtar/crd_simple.txtar @@ -0,0 +1,37 @@ +#version: crd + +-- schema.yaml -- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + # name must be in the form: . + name: myapps.example.com +spec: + # group name to use for REST API: /apis// + group: example.com + scope: Namespaced + names: + # kind is normally the CamelCased singular type. + kind: MyApp + # singular name to be used as an alias on the CLI + singular: myapp + # plural name in the URL: /apis/// + plural: myapps + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + x-kubernetes-preserve-unknown-fields: true + +-- out/decode/extract -- +close({ + spec?: { + ... + } +}) diff --git a/encoding/jsonschema/testdata/txtar/openapi_array-with-no-items.txtar b/encoding/jsonschema/testdata/txtar/openapi_array-with-no-items.txtar new file mode 100644 index 000000000..3889b103f --- /dev/null +++ b/encoding/jsonschema/testdata/txtar/openapi_array-with-no-items.txtar @@ -0,0 +1,13 @@ +#version: openapi + +-- schema.yaml -- +components: + schemas: + BadArray: + description: "A User uses something." + type: array + +-- out/decode/extract -- +ERROR: +"items" must be present when the "type" is "array" in OpenAPI 3.0: + schema.yaml:3:5 diff --git a/encoding/jsonschema/testdata/txtar/openapi_vs_crd.txtar b/encoding/jsonschema/testdata/txtar/openapi_vs_crd.txtar new file mode 100644 index 000000000..789e3b3aa --- /dev/null +++ b/encoding/jsonschema/testdata/txtar/openapi_vs_crd.txtar @@ -0,0 +1,18 @@ +Make sure that OpenAPI still works when there are unimplemented +CRD keywords present. + +#version: openapi + +-- schema.yaml -- +components: + schemas: + User: + description: "A User uses something." + type: array + items: + type: string + x-kubernetes-list-type: set + +-- out/decode/extract -- +// A User uses something. +#User: [...string] diff --git a/encoding/jsonschema/version.go b/encoding/jsonschema/version.go index d6dc76720..f6ec92a81 100644 --- a/encoding/jsonschema/version.go +++ b/encoding/jsonschema/version.go @@ -34,11 +34,16 @@ const ( numJSONSchemaVersions // unknown - // Note: OpenAPI stands alone: it's not in the regular JSON Schema lineage. - VersionOpenAPI // OpenAPI 3.0 + // Note: The following versions stand alone: they're not in the regular JSON Schema lineage. + VersionOpenAPI // OpenAPI 3.0 + VersionKubernetesCRD // Kubernetes CRD ) -const openAPI = versionSet(1 << VersionOpenAPI) +const ( + openAPI = versionSet(1 << VersionOpenAPI) + k8sCRD = versionSet(1 << VersionKubernetesCRD) + openAPILike = openAPI | k8sCRD +) type versionSet int diff --git a/encoding/jsonschema/version_string.go b/encoding/jsonschema/version_string.go index ff122a6ba..0816b504d 100644 --- a/encoding/jsonschema/version_string.go +++ b/encoding/jsonschema/version_string.go @@ -16,11 +16,12 @@ func _() { _ = x[VersionDraft2020_12-5] _ = x[numJSONSchemaVersions-6] _ = x[VersionOpenAPI-7] + _ = x[VersionKubernetesCRD-8] } -const _Version_name = "unknownhttp://json-schema.org/draft-04/schema#http://json-schema.org/draft-06/schema#http://json-schema.org/draft-07/schema#https://json-schema.org/draft/2019-09/schemahttps://json-schema.org/draft/2020-12/schemaunknownOpenAPI 3.0" +const _Version_name = "unknownhttp://json-schema.org/draft-04/schema#http://json-schema.org/draft-06/schema#http://json-schema.org/draft-07/schema#https://json-schema.org/draft/2019-09/schemahttps://json-schema.org/draft/2020-12/schemaunknownOpenAPI 3.0Kubernetes CRD" -var _Version_index = [...]uint8{0, 7, 46, 85, 124, 168, 212, 219, 230} +var _Version_index = [...]uint8{0, 7, 46, 85, 124, 168, 212, 219, 230, 244} func (i Version) String() string { if i < 0 || i >= Version(len(_Version_index)-1) {