Skip to content

Commit 21db9c2

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#82653 from liggitt/skip-publish-nonstructural
Skip publishing OpenAPI for nonstructural schemas
2 parents 85827dc + 0f64ec9 commit 21db9c2

File tree

6 files changed

+54
-20
lines changed

6 files changed

+54
-20
lines changed

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1236,7 +1236,7 @@ func buildOpenAPIModelsForApply(staticOpenAPISpec *spec.Swagger, crd *apiextensi
12361236

12371237
specs := []*spec.Swagger{}
12381238
for _, v := range crd.Spec.Versions {
1239-
s, err := builder.BuildSwagger(crd, v.Name, builder.Options{V2: false, StripDefaults: true, StripValueValidation: true})
1239+
s, err := builder.BuildSwagger(crd, v.Name, builder.Options{V2: false, StripDefaults: true, StripValueValidation: true, AllowNonStructural: true})
12401240
if err != nil {
12411241
return nil, err
12421242
}

‎staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ go_test(
5151
"//vendor/github.com/go-openapi/spec:go_default_library",
5252
"//vendor/github.com/stretchr/testify/assert:go_default_library",
5353
"//vendor/github.com/stretchr/testify/require:go_default_library",
54+
"//vendor/k8s.io/utils/pointer:go_default_library",
5455
],
5556
)
5657

‎staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ type Options struct {
7575

7676
// Strip value validation.
7777
StripValueValidation bool
78+
79+
// AllowNonStructural indicates swagger should be built for a schema that fits into the structural type but does not meet all structural invariants
80+
AllowNonStructural bool
7881
}
7982

8083
// BuildSwagger builds swagger for the given crd in the given version
@@ -88,17 +91,19 @@ func BuildSwagger(crd *apiextensions.CustomResourceDefinition, version string, o
8891
if s != nil && s.OpenAPIV3Schema != nil {
8992
if !validation.SchemaHasInvalidTypes(s.OpenAPIV3Schema) {
9093
if ss, err := structuralschema.NewStructural(s.OpenAPIV3Schema); err == nil {
91-
// skip non-structural schemas
92-
schema = ss
93-
94-
if opts.StripDefaults {
95-
schema = schema.StripDefaults()
94+
// skip non-structural schemas unless explicitly asked to produce swagger from them
95+
if opts.AllowNonStructural || len(structuralschema.ValidateStructural(nil, ss)) == 0 {
96+
schema = ss
97+
98+
if opts.StripDefaults {
99+
schema = schema.StripDefaults()
100+
}
101+
if opts.StripValueValidation {
102+
schema = schema.StripValueValidations()
103+
}
104+
105+
schema = schema.Unfold()
96106
}
97-
if opts.StripValueValidation {
98-
schema = schema.StripValueValidations()
99-
}
100-
101-
schema = schema.Unfold()
102107
}
103108
}
104109
}
@@ -334,12 +339,12 @@ func (b *builder) buildRoute(root, path, httpMethod, actionVerb, operationVerb s
334339

335340
// buildKubeNative builds input schema with Kubernetes' native object meta, type meta and
336341
// extensions
337-
func (b *builder) buildKubeNative(schema *structuralschema.Structural, v2 bool) (ret *spec.Schema) {
342+
func (b *builder) buildKubeNative(schema *structuralschema.Structural, v2 bool, crdPreserveUnknownFields bool) (ret *spec.Schema) {
338343
// only add properties if we have a schema. Otherwise, kubectl would (wrongly) assume additionalProperties=false
339344
// and forbid anything outside of apiVersion, kind and metadata. We have to fix kubectl to stop doing this, e.g. by
340345
// adding additionalProperties=true support to explicitly allow additional fields.
341346
// TODO: fix kubectl to understand additionalProperties=true
342-
if schema == nil || (v2 && schema.XPreserveUnknownFields) {
347+
if schema == nil || (v2 && (schema.XPreserveUnknownFields || crdPreserveUnknownFields)) {
343348
ret = &spec.Schema{
344349
SchemaProps: spec.SchemaProps{Type: []string{"object"}},
345350
}
@@ -506,7 +511,8 @@ func newBuilder(crd *apiextensions.CustomResourceDefinition, version string, sch
506511
}
507512

508513
// Pre-build schema with Kubernetes native properties
509-
b.schema = b.buildKubeNative(schema, v2)
514+
preserveUnknownFields := crd.Spec.PreserveUnknownFields != nil && *crd.Spec.PreserveUnknownFields
515+
b.schema = b.buildKubeNative(schema, v2, preserveUnknownFields)
510516
b.listSchema = b.buildListSchema()
511517

512518
return b

‎staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder_test.go‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"k8s.io/apiserver/pkg/endpoints"
3434
"k8s.io/apiserver/pkg/features"
3535
utilfeature "k8s.io/apiserver/pkg/util/feature"
36+
utilpointer "k8s.io/utils/pointer"
3637
)
3738

3839
func TestNewBuilder(t *testing.T) {
@@ -554,50 +555,72 @@ func schemaDiff(a, b *spec.Schema) string {
554555

555556
func TestBuildSwagger(t *testing.T) {
556557
tests := []struct {
557-
name string
558-
schema string
559-
wantedSchema string
560-
opts Options
558+
name string
559+
schema string
560+
preserveUnknownFields *bool
561+
wantedSchema string
562+
opts Options
561563
}{
562564
{
563565
"nil",
564566
"",
567+
nil,
565568
`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
566569
Options{V2: true, StripDefaults: true},
567570
},
568571
{
569572
"with properties",
570573
`{"type":"object","properties":{"spec":{"type":"object"},"status":{"type":"object"}}}`,
574+
nil,
571575
`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"spec":{"type":"object"},"status":{"type":"object"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
572576
Options{V2: true, StripDefaults: true},
573577
},
574578
{
575579
"with invalid-typed properties",
576580
`{"type":"object","properties":{"spec":{"type":"bug"},"status":{"type":"object"}}}`,
581+
nil,
582+
`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
583+
Options{V2: true, StripDefaults: true},
584+
},
585+
{
586+
"with non-structural schema",
587+
`{"type":"object","properties":{"foo":{"type":"array"}}}`,
588+
nil,
589+
`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
590+
Options{V2: true, StripDefaults: true},
591+
},
592+
{
593+
"with spec.preseveUnknownFields=true",
594+
`{"type":"object","properties":{"foo":{"type":"string"}}}`,
595+
utilpointer.BoolPtr(true),
577596
`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
578597
Options{V2: true, StripDefaults: true},
579598
},
580599
{
581600
"with stripped defaults",
582601
`{"type":"object","properties":{"foo":{"type":"string","default":"bar"}}}`,
602+
nil,
583603
`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"foo":{"type":"string"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
584604
Options{V2: true, StripDefaults: true},
585605
},
586606
{
587607
"with stripped defaults",
588608
`{"type":"object","properties":{"foo":{"type":"string","default":"bar"}}}`,
609+
nil,
589610
`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"foo":{"type":"string"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
590611
Options{V2: true, StripDefaults: true},
591612
},
592613
{
593614
"v2",
594615
`{"type":"object","properties":{"foo":{"type":"string","oneOf":[{"pattern":"a"},{"pattern":"b"}]}}}`,
616+
nil,
595617
`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"foo":{"type":"string"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
596618
Options{V2: true, StripDefaults: true},
597619
},
598620
{
599621
"v3",
600622
`{"type":"object","properties":{"foo":{"type":"string","oneOf":[{"pattern":"a"},{"pattern":"b"}]}}}`,
623+
nil,
601624
`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"foo":{"type":"string","oneOf":[{"pattern":"a"},{"pattern":"b"}]}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
602625
Options{V2: false, StripDefaults: true},
603626
},
@@ -628,8 +651,9 @@ func TestBuildSwagger(t *testing.T) {
628651
Kind: "Foo",
629652
ListKind: "FooList",
630653
},
631-
Scope: apiextensions.NamespaceScoped,
632-
Validation: validation,
654+
Scope: apiextensions.NamespaceScoped,
655+
Validation: validation,
656+
PreserveUnknownFields: tt.preserveUnknownFields,
633657
},
634658
}, "v1", tt.opts)
635659
if err != nil {

‎test/integration/master/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ go_test(
7070
"//vendor/github.com/evanphx/json-patch:go_default_library",
7171
"//vendor/github.com/go-openapi/spec:go_default_library",
7272
"//vendor/github.com/stretchr/testify/require:go_default_library",
73+
"//vendor/k8s.io/utils/pointer:go_default_library",
7374
"//vendor/sigs.k8s.io/yaml:go_default_library",
7475
] + select({
7576
"@io_bazel_rules_go//go/platform:android": [

‎test/integration/master/crd_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
3838
"k8s.io/kubernetes/test/integration/etcd"
3939
"k8s.io/kubernetes/test/integration/framework"
40+
utilpointer "k8s.io/utils/pointer"
4041
)
4142

4243
func TestCRDShadowGroup(t *testing.T) {
@@ -172,6 +173,7 @@ func TestCRDOpenAPI(t *testing.T) {
172173
Plural: "foos",
173174
Kind: "Foo",
174175
},
176+
PreserveUnknownFields: utilpointer.BoolPtr(false),
175177
Validation: &apiextensionsv1beta1.CustomResourceValidation{
176178
OpenAPIV3Schema: &apiextensionsv1beta1.JSONSchemaProps{
177179
Type: "object",

0 commit comments

Comments
 (0)