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

Validation fails for CustomResource with array of pointers #98079

Closed
caseydavenport opened this issue Jan 14, 2021 · 14 comments
Closed

Validation fails for CustomResource with array of pointers #98079

caseydavenport opened this issue Jan 14, 2021 · 14 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cli Categorizes an issue or PR as relevant to SIG CLI. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@caseydavenport
Copy link
Member

caseydavenport commented Jan 14, 2021

What happened:

I have a CustomResourceDefinition with a schema that includes an array property, where each element of the array is either an integer or null. This is implemented in the CRD like so:

spec:
  properties:
    allocations:
      items:
        nullable: true
        type: integer
      type: array

Using the golang client, I am able to properly write instances of the custom resource. However, kubectl fails validation with the following error when attempting to edit an instance of the custom resource:

ValidationError(IPAMBlock.spec.allocations): unknown object type "nil" in IPAMBlock.spec.allocations[3]

This error is duplicated for each "null" entry in the "allocations" array. This can be reproduced simply by using kubectl edit on a resource created by the golang client, and then saving the resource with no changes. It seems to be a problem with kubectl validation, since kubectl edit --validate=false successfully updates the resource (but hits this other bug instead: #98080).

What you expected to happen:

I can modify the custom resource using kubectl without errors or turning off validation.

How to reproduce it (as minimally and precisely as possible):

I encountered this in the context of Calico. If you have a cluster running Calico, you should be able to hit this with the following command:

kubectl edit ipamblocks

Then, simply save without changing anything.

Anything else we need to know?:

Environment:

  • Kubernetes version (use kubectl version):
Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.8", GitCommit:"9f2892aab98fe339f3bd70e3c470144299398ace", GitTreeState:"clean", BuildDate:"2020-08-13T16:12:48Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.2", GitCommit:"52c56ce7a8272c798dbc29846288d7cd9fbae032", GitTreeState:"clean", BuildDate:"2020-04-30T20:19:45Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}

@liggitt was also able to reproduce on master.

CC @sttts @soltysh

/sig api-machinery

@caseydavenport caseydavenport added the kind/bug Categorizes issue or PR as related to a bug. label Jan 14, 2021
@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 14, 2021
@apelisse
Copy link
Member

/triage accepted

I know what that happens, and I suspect I know how to fix it (though there are interfaces everywhere in this code so it's hard to modify without breaking compatibility): https://github.com/kubernetes/kube-openapi/blob/master/pkg/util/proto/openapi.go
There is no concept of null here.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 15, 2021
@apelisse
Copy link
Member

Or rather, I think that very specific todo very well highlights the problem:
https://github.com/kubernetes/kube-openapi/blob/master/pkg/util/proto/validation/types.go#L219

@sttts
Copy link
Contributor

sttts commented Jan 15, 2021

@apelisse I bet this is another case for k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2/conversion.go. Note that fixing kubectl is only a long-term fix. As long as old kubectls are around, the bug will show up.

@apelisse
Copy link
Member

Yeah, we can probably fix this one fairly easily (we have the same long-term and short-term goals ;-)

@apelisse
Copy link
Member

This is also sig-cli since the validation is performed by kubectl.

/sig cli

@k8s-ci-robot k8s-ci-robot added the sig/cli Categorizes an issue or PR as relevant to SIG CLI. label Jan 25, 2021
@chymy
Copy link
Contributor

chymy commented Feb 2, 2021

Hi, @caseydavenport, I can not reproduce it(k8s version 1.20)

# kubectl edit ipamblocks
Edit cancelled, no changes made.

Is it related to this PR #95423?

@apelisse
Copy link
Member

apelisse commented Feb 2, 2021

#95423 is a server-side change, this bug is because of a client-side bug. I don't think it can be related. Are you sure you're reproducing the exact same bug?

@chymy
Copy link
Contributor

chymy commented Feb 3, 2021

I reproduce it in the way given in How to reproduce it (as minimally and precisely as possible):, but I didn't reproduce it.

@fejta-bot
Copy link

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

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

Send feedback to sig-contributor-experience at kubernetes/community.
/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 May 4, 2021
@apelisse
Copy link
Member

apelisse commented May 4, 2021

We're starting to work on a plan to "address" this.
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 4, 2021
@k8s-triage-robot
Copy link

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Feb 8, 2023
@cici37
Copy link
Contributor

cici37 commented Feb 14, 2023

hi @apelisse , would you mind verifying if this is still valid? Thank you :)

@apelisse
Copy link
Member

Thanks, this is client-side validation, we can close this since it's addressed by kubernetes/enhancements#2885. (GA in 1.27).

@fedebongio
Copy link
Contributor

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cli Categorizes an issue or PR as relevant to SIG CLI. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

9 participants