Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support both "type":"any" and "type":"object" in kubectl validation #24309

Closed
nikhiljindal opened this issue Apr 15, 2016 · 10 comments
Closed

Support both "type":"any" and "type":"object" in kubectl validation #24309

nikhiljindal opened this issue Apr 15, 2016 · 10 comments
Assignees
Labels
area/swagger lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@nikhiljindal
Copy link
Contributor

nikhiljindal commented Apr 15, 2016

We used to generate our swagger spec with "type":"any" for any type that wasnt supported in swagger (for ex: maps).

It was later pointed out that "type":"object" is the correct representation: #4700 (comment).

We have now moved to using "type":"object" in our swagger spec (which will be part of release 1.3), but for kubectl to work with old apiservers, we allow both "type":"object" and "type":"any" in kubectl validation code (#24054).
We cherrypicked that in release 1.2 (so that 1.2 kubectl can work with 1.3 apiserver).

We can stop supporting "type":"any" in release 1.4 (1.4 kubectl will not be able to work with 1.2 apiserver, which is fine since we support version skew of only 1).

Filing this issue to keep track.

cc @bgrant0607 @kubernetes/sig-api-machinery

@nikhiljindal nikhiljindal added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. area/swagger kind/workaround-removal labels Apr 15, 2016
@nikhiljindal nikhiljindal self-assigned this Apr 15, 2016
@maisem
Copy link

maisem commented Apr 18, 2016

This seems to have broken backwards compat.
1.2.2 kubectl cannot validate responses from the server.
If you don't have the labels fields set, it works fine.

kubectl create --v=7 --validate -f node.yaml
I0418 13:30:58.159142 18784 loader.go:229] Config loaded from file ~/.kube/config
I0418 13:30:58.266280 18784 round_trippers.go:264] GET https:///api
I0418 13:30:58.266303 18784 round_trippers.go:271] Request Headers:
I0418 13:30:58.266310 18784 round_trippers.go:274] Accept: application/json, /
I0418 13:30:58.266315 18784 round_trippers.go:274] User-Agent: kubectl/v1.2.2 (linux/amd64) kubernetes/528f879
I0418 13:30:58.266320 18784 round_trippers.go:274] Authorization: Basic
I0418 13:30:58.451798 18784 round_trippers.go:289] Response Status: 200 OK in 185 milliseconds
I0418 13:30:58.452142 18784 round_trippers.go:264] GET https:///apis
I0418 13:30:58.452166 18784 round_trippers.go:271] Request Headers:
I0418 13:30:58.452176 18784 round_trippers.go:274] Accept: application/json, /
I0418 13:30:58.452186 18784 round_trippers.go:274] User-Agent: kubectl/v1.2.2 (linux/amd64) kubernetes/528f879
I0418 13:30:58.452197 18784 round_trippers.go:274] Authorization: Basic
I0418 13:30:58.491680 18784 round_trippers.go:289] Response Status: 200 OK in 39 milliseconds
I0418 13:30:58.493672 18784 round_trippers.go:264] GET https:///version
I0418 13:30:58.493711 18784 round_trippers.go:271] Request Headers:
I0418 13:30:58.493727 18784 round_trippers.go:274] Accept: application/json, /
I0418 13:30:58.493743 18784 round_trippers.go:274] User-Agent: kubectl/v1.2.2 (linux/amd64) kubernetes/528f879
I0418 13:30:58.493759 18784 round_trippers.go:274] Authorization: Basic
I0418 13:30:58.533184 18784 round_trippers.go:289] Response Status: 200 OK in 39 milliseconds
I0418 13:30:58.534088 18784 decoder.go:144] decoding stream as YAML
F0418 13:30:58.566577 18784 helpers.go:107] error validating "node.yaml": error validating data: unexpected type: object; if you choose to ignore these errors, turn validation off with --validate=false

node.yaml

apiVersion:
v1
kind: Pod
metadata:
name: pause-test
labels:
name: pause-test
spec:
containers:
- name: pause
image: gcr.io/google-containers/pause:1.0

kubectl version

Client Version: version.Info{Major:"1", Minor:"2", GitVersion:"v1.2.2", GitCommit:"528f879e7d3790ea4287687ef0ab3f2a01cc2718", GitTreeState:"clean"}
Server Version: version.Info{Major:"1", Minor:"3+", GitVersion:"v1.3.0-alpha.2.308+616af686cbb50d", GitCommit:"616af686cbb50dc42434d7a9864e2cf465cb9ab9", GitTreeState:"clean"}

@maisem maisem added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Apr 18, 2016
@nikhiljindal
Copy link
Contributor Author

Yes that should be fixed once we release 1.2.3 with #24104.
cc @roberthbailey

Do you want us to rollback the change on HEAD until 1.2.3 is rolled out?

@janetkuo
Copy link
Member

cc @mikedanese

@janetkuo
Copy link
Member

cc @gmarek

@gmarek
Copy link
Contributor

gmarek commented Apr 26, 2016

@nikhiljindal - what's the status of this? It seems that it blocks at least two other things.

@nikhiljindal
Copy link
Contributor Author

nikhiljindal commented Apr 26, 2016

1.2.3 is released already so this should not be blocking anymore.
Please let me know if it still blocks you.

Am keeping the issue open to track removing the workaround.

@whitlockjc
Copy link
Contributor

Is there ever a time when type: any would be expected to permit an array or a primitive to pass validation? The reason I ask is because to me, type: any would allow any value to pass validation, including primitives, whereas type: object would allow any object value to pass validation (regardless of its structure) but would fail on any type that is not an object, including arrays and primitives.

If the answer is "yes", then we should be using {} as the replacement for type: any instead of type: object. In JSON Schema, the wildcard schema is the empty schema.

@fejta-bot
Copy link

Issues go stale after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 19, 2017
@nikhiljindal
Copy link
Contributor Author

I think this is obsolete now with our move to OpenAPI (swagger 2.0).
Assigning to @mbohlool to confirm

@mbohlool
Copy link
Contributor

agreed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/swagger lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Development

No branches or pull requests

9 participants