From 0dcaff505e5937d41534d60b705a9c71efdcca1b Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Tue, 11 Mar 2025 13:58:13 +0000 Subject: [PATCH] encoding/jsonschema: initial support for Kubernetes CRDs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL introduces a new JSON Schema "version" dedicated to translating CRD schemas. We add all the relevant keywords but leave most of them as TODOs. We implement some of the easier checks but leave harder ones (structural schema checking, for example) for later. The main short-term TODO is support for `x-kubernetes-embedded-resource`, as most other things are less strict than needed currently, so can be punted for now. Signed-off-by: Roger Peppe Change-Id: I2a66ba0007a2401357dcae151dab15bd45f72b24 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1211200 TryBot-Result: CUEcueckoo Reviewed-by: Daniel Martí Unity-Result: CUE porcuepine --- encoding/jsonschema/constraints.go | 67 ++++++----- encoding/jsonschema/constraints_array.go | 5 + encoding/jsonschema/constraints_combinator.go | 12 +- encoding/jsonschema/constraints_format.go | 42 +++++-- encoding/jsonschema/constraints_generic.go | 18 +++ encoding/jsonschema/constraints_object.go | 40 ++++++- encoding/jsonschema/decode.go | 110 +++++++++++++----- encoding/jsonschema/decode_test.go | 16 ++- .../testdata/txtar/crd_intorstring.txtar | 34 ++++++ .../txtar/crd_intorstring_with_anyof.txtar | 40 +++++++ .../testdata/txtar/crd_preserve_unknown.txtar | 99 ++++++++++++++++ ...properties_with_additionalproperties.txtar | 45 +++++++ .../jsonschema/testdata/txtar/crd_ref.txtar | 36 ++++++ .../testdata/txtar/crd_simple.txtar | 37 ++++++ .../txtar/openapi_array-with-no-items.txtar | 13 +++ .../testdata/txtar/openapi_vs_crd.txtar | 18 +++ encoding/jsonschema/version.go | 11 +- encoding/jsonschema/version_string.go | 5 +- 18 files changed, 562 insertions(+), 86 deletions(-) create mode 100644 encoding/jsonschema/testdata/txtar/crd_intorstring.txtar create mode 100644 encoding/jsonschema/testdata/txtar/crd_intorstring_with_anyof.txtar create mode 100644 encoding/jsonschema/testdata/txtar/crd_preserve_unknown.txtar create mode 100644 encoding/jsonschema/testdata/txtar/crd_properties_with_additionalproperties.txtar create mode 100644 encoding/jsonschema/testdata/txtar/crd_ref.txtar create mode 100644 encoding/jsonschema/testdata/txtar/crd_simple.txtar create mode 100644 encoding/jsonschema/testdata/txtar/openapi_array-with-no-items.txtar create mode 100644 encoding/jsonschema/testdata/txtar/openapi_vs_crd.txtar 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) { -- 2.51.2