Handle "any value" openapi definitions properly #25

luksa · 2017-11-27T17:45:35Z

This will allow us to support the k8s RawExtension object

luksa · 2017-11-27T17:54:50Z

mengqiy · 2017-11-29T07:22:57Z

apelisse · 2017-11-29T18:54:53Z

OK, I've read the other pull-request that you've created. I might have misunderstood the intent here. Looking at it again.

apelisse · 2017-11-29T19:04:19Z

OK I think this is it: https://tools.ietf.org/html/draft-zyp-json-schema-04#section-5.1

apelisse · 2017-11-29T19:08:00Z

pkg/util/proto/openapi.go

@@ -242,6 +242,21 @@ func (p *Primitive) GetName() string {
 	return fmt.Sprintf("%s (%s)", p.Type, p.Format)
 }

+// AnyValue is an arbitrary value (object, array or primitive)


Can we figure out a better name? My initial understanding was that this was "AnyOf" (from openapi 3.0).

apelisse · 2017-11-29T19:08:53Z

pkg/util/proto/openapi.go

+var _ Schema = &AnyValue{}
+
+func (m *AnyValue) Accept(v SchemaVisitor) {
+	// A AnyValue accepts any type of value, so it's not necessary to call the SchemaVisitor


I think that decision should be taken by each individual visitor, what do you think?

@pwittrock Do you have an opinion?

Yes, it should call a schema visitor. The visitor may want to do something.

OK, that was my initial implementation. Then, since all the visitors' methods were empty, I thought I could remove it, since all visitors should always accept any value.

I'll add the methods again and also rename this to ArbitraryValue or whatever you guys think is a better name.

apelisse · 2017-11-29T21:17:47Z

pkg/util/proto/openapi.go

@@ -55,6 +55,7 @@ type SchemaVisitor interface {
 	VisitMap(*Map)
 	VisitPrimitive(*Primitive)
 	VisitKind(*Kind)
+	VisitArbitraryValue(*ArbitraryValue)


I'm happy you thought of that word Arbitrary. Can I nit and ask you to just keep Arbitrary without the Value?

apelisse · 2017-11-29T22:12:05Z

That looks good to me. I have no right here, but lgtm. @mbohlool

ericchiang · 2017-12-11T23:22:00Z

On master I'm getting this when trying to generate the openapi schema with this change.

$ ./hack/update-openapi-spec.sh
make: Entering directory '/home/eric/src/k8s.io/kubernetes'                                                                                                                                                        
make[1]: Entering directory '/home/eric/src/k8s.io/kubernetes' 
# ...
    cmd/kube-apiserver                              
# k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/strategicpatch                                   
vendor/k8s.io/apimachinery/pkg/util/strategicpatch/meta.go:166:17: cannot use kindItem (type *kindItem) as type "k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/util/proto".SchemaVisitor in argument to s.Schema
.Accept:                                            
        *kindItem does not implement "k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/util/proto".SchemaVisitor (missing VisitArbitrary method)                                                                   
vendor/k8s.io/apimachinery/pkg/util/strategicpatch/meta.go:181:17: cannot use sliceItem (type *sliceItem) as type "k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/util/proto".SchemaVisitor in argument to s.Sche
ma.Accept:                                          
        *sliceItem does not implement "k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/util/proto".SchemaVisitor (missing VisitArbitrary method)                                                                  
vendor/k8s.io/apimachinery/pkg/util/strategicpatch/types.go:55:5: cannot use kindItem literal (type *kindItem) as type LookupPatchItem in assignment:                                                              
        *kindItem does not implement LookupPatchItem (missing VisitArbitrary method)                     
vendor/k8s.io/apimachinery/pkg/util/strategicpatch/types.go:79:28: cannot use item (type *kindItem) as type "k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/util/proto".SchemaVisitor in argument to schema.SubSc
hema().Accept:                                      
        *kindItem does not implement "k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/util/proto".SchemaVisitor (missing VisitArbitrary method)                                                                   
vendor/k8s.io/apimachinery/pkg/util/strategicpatch/types.go:118:5: cannot use sliceItem literal (type *sliceItem) as type LookupPatchItem in assignment:                                                           
        *sliceItem does not implement LookupPatchItem (missing VisitArbitrary method)                    
vendor/k8s.io/apimachinery/pkg/util/strategicpatch/types.go:146:28: cannot use item (type *sliceItem) as type "k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/util/proto".SchemaVisitor in argument to schema.Sub
Schema().Accept:                                    
        *sliceItem does not implement "k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/util/proto".SchemaVisitor (missing VisitArbitrary method)                                                                  
vendor/k8s.io/apimachinery/pkg/util/strategicpatch/types.go:169:18: cannot use item (type *sliceItem) as type "k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/util/proto".SchemaVisitor in argument to subschema.
Accept:                                             
        *sliceItem does not implement "k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/util/proto".SchemaVisitor (missing VisitArbitrary method)

ericchiang · 2017-12-11T23:23:02Z

~~Actually sorry false alarm. I don't think this is related to this change. sorry for the noise.~~

edit: actually see my comment below.

ericchiang · 2017-12-11T23:55:47Z

pkg/util/proto/openapi.go

@@ -55,6 +55,7 @@ type SchemaVisitor interface {
 	VisitMap(*Map)
 	VisitPrimitive(*Primitive)
 	VisitKind(*Kind)
+	VisitArbitrary(*Arbitrary)


Expanding this interface causes a ton of code to break in kubernetes/kubernetes...

kubernetes/kubernetes#57059

Fix str->bson function usages

Handle "any value" openapi definitions properly

94206b9

This will allow us to support the k8s RawExtension object

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 27, 2017

luksa mentioned this pull request Nov 27, 2017

WIP: Fix definition of RawExtension in OpenAPI schema kubernetes/kubernetes#56434

Closed

luksa mentioned this pull request Nov 27, 2017

make apiserver serve openapi kubernetes-retired/service-catalog#1208

Closed

k8s-ci-robot assigned apelisse Nov 29, 2017

apelisse reviewed Nov 29, 2017

View reviewed changes

Rename AnyValue to ArbitraryValue & make it call SchemaVisitor

2d9ea0c

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 29, 2017

apelisse reviewed Nov 29, 2017

View reviewed changes

Rename ArbitraryValue to Arbitrary

34c97a3

mbohlool merged commit fff8c87 into kubernetes:master Dec 5, 2017

ericchiang reviewed Dec 11, 2017

View reviewed changes

ericchiang mentioned this pull request Dec 12, 2017

remove VisitArbitrary from SchemaVisitor, make different interface #28

Merged

luksa mentioned this pull request Dec 12, 2017

Implement VisitArbitrary in schemavisitors kubernetes/kubernetes#57104

Closed

pmorie mentioned this pull request Jan 4, 2018

Improve kubectl get for the service-catalog resources kubernetes-retired/service-catalog#1634

Closed

seans3 mentioned this pull request Jun 1, 2018

kubectl edit machine fails validations kubernetes-sigs/cluster-api#137

Closed

sttts pushed a commit to sttts/kube-openapi that referenced this pull request Nov 5, 2020

Merge pull request kubernetes#25 from outdoorsy/master

6ba3155

Fix str->bson function usages

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle "any value" openapi definitions properly #25

Handle "any value" openapi definitions properly #25

luksa commented Nov 27, 2017

luksa commented Nov 27, 2017

mengqiy commented Nov 29, 2017

apelisse commented Nov 29, 2017

apelisse commented Nov 29, 2017

apelisse Nov 29, 2017

apelisse Nov 29, 2017

apelisse Nov 29, 2017

pwittrock Nov 29, 2017

luksa Nov 29, 2017

apelisse Nov 29, 2017

apelisse commented Nov 29, 2017

ericchiang commented Dec 11, 2017

ericchiang commented Dec 11, 2017 •

edited

Loading

ericchiang Dec 11, 2017

Handle "any value" openapi definitions properly #25

Handle "any value" openapi definitions properly #25

Conversation

luksa commented Nov 27, 2017

luksa commented Nov 27, 2017

mengqiy commented Nov 29, 2017

apelisse commented Nov 29, 2017

apelisse commented Nov 29, 2017

apelisse Nov 29, 2017

Choose a reason for hiding this comment

apelisse Nov 29, 2017

Choose a reason for hiding this comment

apelisse Nov 29, 2017

Choose a reason for hiding this comment

pwittrock Nov 29, 2017

Choose a reason for hiding this comment

luksa Nov 29, 2017

Choose a reason for hiding this comment

apelisse Nov 29, 2017

Choose a reason for hiding this comment

apelisse commented Nov 29, 2017

ericchiang commented Dec 11, 2017

ericchiang commented Dec 11, 2017 • edited Loading

ericchiang Dec 11, 2017

Choose a reason for hiding this comment

ericchiang commented Dec 11, 2017 •

edited

Loading