Skip to content

Commit 2c01ee5

Browse files
rogpepperogpeppe
committed
encoding/jsonschema: implement x-kubernetes-group-version-kind
The `x-kubernetes-group-version-kind` keyword, although lightly documented, appears to specify the required values of the `apiVersion` and `kind` fields for Kubernetes resources. When there's only a single value, we turn that into a regular field. This isn't entirely desirable because it means the generated schemas are no longer "pure" but it makes for a better UX. See #3871 for a proposal that would mean we could still use required fields while avoiding the need for users to manually specify the `apiVersion` and `kind` fields. We also specify that various Kubernetes-specific keywords are ignored rather than TODOs because they don't affect current semantics and we don't want them to cause failure when StrictKeywords is enabled. Signed-off-by: Roger Peppe <[email protected]> Change-Id: Ib79957a2dd74f9b43a26416691760274dfa4379b Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1214704 TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Daniel Martí <[email protected]>
1 parent a918c8b commit 2c01ee5

File tree

7 files changed

+280
-17
lines changed

7 files changed

+280
-17
lines changed

‎encoding/jsonschema/constraints.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,13 @@ var constraints = []*constraint{
128128
px("writeOnly", constraintTODO, vfrom(VersionDraft7)|openAPI),
129129
px("xml", constraintTODO, openAPI),
130130
p1("x-kubernetes-embedded-resource", constraintEmbeddedResource, k8s),
131+
p1("x-kubernetes-group-version-kind", constraintGroupVersionKind, k8sAPI),
131132
p2("x-kubernetes-int-or-string", constraintIntOrString, k8s),
132-
px("x-kubernetes-list-map-keys", constraintTODO, k8s),
133-
px("x-kubernetes-list-type", constraintTODO, k8s),
134-
px("x-kubernetes-map-type", constraintTODO, k8s),
135-
px("x-kubernetes-patch-merge-key", constraintTODO, k8s),
136-
px("x-kubernetes-patch-strategy", constraintTODO, k8s),
133+
px("x-kubernetes-list-map-keys", constraintIgnore, k8s),
134+
px("x-kubernetes-list-type", constraintIgnore, k8s),
135+
px("x-kubernetes-map-type", constraintIgnore, k8s),
136+
px("x-kubernetes-patch-merge-key", constraintIgnore, k8s),
137+
px("x-kubernetes-patch-strategy", constraintIgnore, k8s),
137138
p2("x-kubernetes-preserve-unknown-fields", constraintPreserveUnknownFields, k8s),
138139
px("x-kubernetes-validations", constraintTODO, k8s),
139140
}

‎encoding/jsonschema/constraints_meta.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,10 @@ func constraintTODO(key string, n cue.Value, s *state) {
7474
s.errf(n, `keyword %q not yet implemented`, key)
7575
}
7676
}
77+
78+
// constraintIgnore represents a constraint that we're deliberately
79+
// ignoring, by contrast with [constraintTODO] that represents
80+
// a constraint that we're definitely intending to implement
81+
// at some point.
82+
func constraintIgnore(key string, b cue.Value, s *state) {
83+
}

‎encoding/jsonschema/constraints_object.go

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
package jsonschema
1616

1717
import (
18+
"strings"
19+
1820
"cuelang.org/go/cue"
1921
"cuelang.org/go/cue/ast"
2022
"cuelang.org/go/cue/token"
@@ -44,6 +46,38 @@ func constraintPreserveUnknownFields(key string, n cue.Value, s *state) {
4446
s.preserveUnknownFields = true
4547
}
4648

49+
func constraintGroupVersionKind(key string, n cue.Value, s *state) {
50+
// x-kubernetes-group-version-kind is used by Kubernetes schemas
51+
// to indicate the required values of the apiVersion and kind fields.
52+
items := s.listItems(key, n, false)
53+
if len(items) != 1 {
54+
// When there's more than one item, we _could_ generate
55+
// a disjunction over apiVersion and kind but for now, we'll
56+
// just ignore it.
57+
// TODO implement support for multiple items
58+
return
59+
}
60+
s.processMap(items[0], func(key string, n cue.Value) {
61+
if strings.HasPrefix(key, "x-") {
62+
// TODO are x- extension properties actually allowed in this context?
63+
return
64+
}
65+
switch key {
66+
case "group":
67+
return
68+
case "kind":
69+
s.k8sResourceKind, _ = s.strValue(n)
70+
case "version":
71+
s.k8sAPIVersion, _ = s.strValue(n)
72+
default:
73+
s.errf(n, "unknown field %q in x-kubernetes-group-version-kind item", key)
74+
}
75+
})
76+
if s.k8sResourceKind == "" || s.k8sAPIVersion == "" {
77+
s.errf(n, "x-kubernetes-group-version-kind needs both kind and version fields")
78+
}
79+
}
80+
4781
func constraintAdditionalProperties(key string, n cue.Value, s *state) {
4882
switch n.Kind() {
4983
case cue.BoolKind:
@@ -178,6 +212,8 @@ func constraintProperties(key string, n cue.Value, s *state) {
178212
if n.Kind() != cue.StructKind {
179213
s.errf(n, `"properties" expected an object, found %v`, n.Kind())
180214
}
215+
hasKind := false
216+
hasAPIVersion := false
181217
s.processMap(n, func(key string, n cue.Value) {
182218
// property?: value
183219
name := ast.NewString(key)
@@ -188,7 +224,19 @@ func constraintProperties(key string, n cue.Value, s *state) {
188224
if doc := state.comment(); doc != nil {
189225
ast.SetComments(f, []*ast.CommentGroup{doc})
190226
}
191-
f.Optional = token.Blank.Pos()
227+
f.Constraint = token.OPTION
228+
if s.k8sResourceKind != "" && key == "kind" {
229+
// Define a regular field with the specified kind value.
230+
f.Constraint = token.ILLEGAL
231+
f.Value = ast.NewString(s.k8sResourceKind)
232+
hasKind = true
233+
}
234+
if s.k8sAPIVersion != "" && key == "apiVersion" {
235+
// Define a regular field with the specified value.
236+
f.Constraint = token.ILLEGAL
237+
f.Value = ast.NewString(s.k8sAPIVersion)
238+
hasAPIVersion = true
239+
}
192240
if len(obj.Elts) > 0 && len(f.Comments()) > 0 {
193241
// TODO: change formatter such that either a NewSection on the
194242
// field or doc comment will cause a new section.
@@ -204,6 +252,21 @@ func constraintProperties(key string, n cue.Value, s *state) {
204252
}
205253
obj.Elts = append(obj.Elts, f)
206254
})
255+
// It's not entirely clear whether it's OK to have an x-kubernetes-group-version-kind
256+
// keyword without the kind and apiVersion properties but be defensive
257+
// and add them anyway even if they're not there already.
258+
if s.k8sAPIVersion != "" && !hasAPIVersion {
259+
obj.Elts = append(obj.Elts, &ast.Field{
260+
Label: ast.NewString("apiVersion"),
261+
Value: ast.NewString(s.k8sAPIVersion),
262+
})
263+
}
264+
if s.k8sResourceKind != "" && !hasKind {
265+
obj.Elts = append(obj.Elts, &ast.Field{
266+
Label: ast.NewString("kind"),
267+
Value: ast.NewString(s.k8sResourceKind),
268+
})
269+
}
207270
s.hasProperties = true
208271
}
209272

@@ -249,10 +312,9 @@ func constraintRequired(key string, n cue.Value, s *state) {
249312
obj.Elts = append(obj.Elts, f)
250313
continue
251314
}
252-
if f.Optional == token.NoPos {
315+
if f.Constraint == token.NOT {
253316
s.errf(n, "duplicate required field %q", str)
254317
}
255318
f.Constraint = token.NOT
256-
f.Optional = token.NoPos
257319
}
258320
}

‎encoding/jsonschema/decode.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,12 @@ type state struct {
530530
// reset within properties or additionalProperties.
531531
preserveUnknownFields bool
532532

533+
// k8sResourceKind and k8sAPIVersion record values from the
534+
// x-kubernetes-group-version-kind keyword
535+
// for the kind and apiVersion properties respectively.
536+
k8sResourceKind string
537+
k8sAPIVersion string
538+
533539
// Keep track of whether the object has been explicitly
534540
// closed or opened (see [Config.OpenOnlyWhenExplicit]).
535541
openness openness

‎encoding/jsonschema/decode_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,18 @@ func TestDecode(t *testing.T) {
8383
// We should probably change most of the tests to use an explicit $schema
8484
// field apart from when we're explicitly testing the default version logic.
8585
switch versStr {
86-
case "openapi", "k8sAPI":
86+
case "openapi":
8787
cfg.DefaultVersion = jsonschema.VersionOpenAPI
88-
if versStr == "k8sAPI" {
89-
cfg.DefaultVersion = jsonschema.VersionKubernetesAPI
90-
} else {
91-
cfg.Map = func(p token.Pos, a []string) ([]ast.Label, error) {
92-
// Just for testing: does not validate the path.
93-
return []ast.Label{ast.NewIdent("#" + a[len(a)-1])}, nil
94-
}
88+
cfg.Map = func(p token.Pos, a []string) ([]ast.Label, error) {
89+
// Just for testing: does not validate the path.
90+
return []ast.Label{ast.NewIdent("#" + a[len(a)-1])}, nil
9591
}
9692
cfg.Root = "#/components/schemas/"
97-
cfg.StrictKeywords = true // OpenAPI always uses strict keywords
93+
cfg.StrictKeywords = true // encoding/openapi always uses strict keywords
94+
case "k8sAPI":
95+
cfg.DefaultVersion = jsonschema.VersionKubernetesAPI
96+
cfg.Root = "#/components/schemas/"
97+
cfg.StrictKeywords = true
9898
case "k8sCRD":
9999
cfg.DefaultVersion = jsonschema.VersionKubernetesCRD
100100
// Default to the first version; can be overridden with #root.

‎encoding/jsonschema/testdata/txtar/k8s_group_kind_version.txtar

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
Note: the schema.json file here is derived from the
2+
file api/openapi-spec/v3/api_openapi.json in
3+
the Kubernetes repository.
4+
5+
#version: k8sAPI
6+
7+
-- schema.json --
8+
{
9+
"components": {
10+
"schemas": {
11+
"io.k8s.apimachinery.pkg.apis.meta.v1.APIVersions": {
12+
"properties": {
13+
"apiVersion": {
14+
"type": "string"
15+
},
16+
"kind": {
17+
"type": "string"
18+
},
19+
"serverAddressByClientCIDRs": {
20+
"items": {
21+
"allOf": [
22+
{
23+
"$ref": "#/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.ServerAddressByClientCIDR"
24+
}
25+
],
26+
"default": {}
27+
},
28+
"type": "array",
29+
"x-kubernetes-list-type": "atomic"
30+
},
31+
"versions": {
32+
"items": {
33+
"default": "",
34+
"type": "string"
35+
},
36+
"type": "array",
37+
"x-kubernetes-list-type": "atomic"
38+
}
39+
},
40+
"required": [
41+
"versions",
42+
"serverAddressByClientCIDRs"
43+
],
44+
"type": "object",
45+
"x-kubernetes-group-version-kind": [
46+
{
47+
"group": "",
48+
"kind": "APIVersions",
49+
"version": "v1"
50+
}
51+
]
52+
},
53+
"io.k8s.apimachinery.pkg.apis.meta.v1.ServerAddressByClientCIDR": {
54+
"properties": {
55+
"clientCIDR": {
56+
"default": "",
57+
"type": "string"
58+
},
59+
"serverAddress": {
60+
"default": "",
61+
"type": "string"
62+
}
63+
},
64+
"required": [
65+
"clientCIDR",
66+
"serverAddress"
67+
],
68+
"type": "object"
69+
}
70+
},
71+
"securitySchemes": {
72+
"BearerToken": {
73+
"in": "header",
74+
"name": "authorization",
75+
"type": "apiKey"
76+
}
77+
}
78+
},
79+
"info": {
80+
"title": "Kubernetes",
81+
"version": "unversioned"
82+
},
83+
"openapi": "3.0.0",
84+
"paths": {}
85+
}
86+
-- out/decode/extract --
87+
_#defs: "/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.APIVersions": {
88+
apiVersion: "v1"
89+
kind: "APIVersions"
90+
serverAddressByClientCIDRs!: [..._#defs."/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.ServerAddressByClientCIDR"]
91+
versions!: [...string]
92+
}
93+
94+
_#defs: "/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.ServerAddressByClientCIDR": {
95+
clientCIDR!: string
96+
serverAddress!: string
97+
}

‎encoding/jsonschema/testdata/txtar/k8s_group_kind_version_without_fields.txtar

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
Check that the x-kubernetes-group-version-kind keyword works
2+
even when the kind and apiVersion properties aren't defined.
3+
4+
#version: k8sAPI
5+
6+
-- schema.json --
7+
{
8+
"components": {
9+
"schemas": {
10+
"io.k8s.apimachinery.pkg.apis.meta.v1.APIVersions": {
11+
"properties": {
12+
"serverAddressByClientCIDRs": {
13+
"items": {
14+
"allOf": [
15+
{
16+
"$ref": "#/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.ServerAddressByClientCIDR"
17+
}
18+
],
19+
"default": {}
20+
},
21+
"type": "array",
22+
"x-kubernetes-list-type": "atomic"
23+
},
24+
"versions": {
25+
"items": {
26+
"default": "",
27+
"type": "string"
28+
},
29+
"type": "array",
30+
"x-kubernetes-list-type": "atomic"
31+
}
32+
},
33+
"required": [
34+
"versions",
35+
"serverAddressByClientCIDRs"
36+
],
37+
"type": "object",
38+
"x-kubernetes-group-version-kind": [
39+
{
40+
"group": "",
41+
"kind": "APIVersions",
42+
"version": "v1"
43+
}
44+
]
45+
},
46+
"io.k8s.apimachinery.pkg.apis.meta.v1.ServerAddressByClientCIDR": {
47+
"properties": {
48+
"clientCIDR": {
49+
"default": "",
50+
"type": "string"
51+
},
52+
"serverAddress": {
53+
"default": "",
54+
"type": "string"
55+
}
56+
},
57+
"required": [
58+
"clientCIDR",
59+
"serverAddress"
60+
],
61+
"type": "object"
62+
}
63+
},
64+
"securitySchemes": {
65+
"BearerToken": {
66+
"in": "header",
67+
"name": "authorization",
68+
"type": "apiKey"
69+
}
70+
}
71+
},
72+
"info": {
73+
"title": "Kubernetes",
74+
"version": "unversioned"
75+
},
76+
"openapi": "3.0.0",
77+
"paths": {}
78+
}
79+
-- out/decode/extract --
80+
_#defs: "/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.APIVersions": {
81+
serverAddressByClientCIDRs!: [..._#defs."/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.ServerAddressByClientCIDR"]
82+
versions!: [...string]
83+
apiVersion: "v1"
84+
kind: "APIVersions"
85+
}
86+
87+
_#defs: "/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.ServerAddressByClientCIDR": {
88+
clientCIDR!: string
89+
serverAddress!: string
90+
}

0 commit comments

Comments
 (0)