kube-openapi icon indicating copy to clipboard operation
kube-openapi copied to clipboard

Published 20 hours ago • verified publisher icon kubernetes

Generated enum fails validation when used for CRD creation

Open jingyuanliang opened this issue 1 year ago • 15 comments

https://github.com/kubernetes/kube-openapi/blob/78281498afbbfb6fdabe43b8b6a7898f75892c44/pkg/generators/openapi_test.go#L1618

This shows an example of generated code:

https://github.com/kubernetes/kube-openapi/blob/78281498afbbfb6fdabe43b8b6a7898f75892c44/pkg/generators/openapi_test.go#L1655-L1662

However if we use this code to generate CRD it fails validation. The error message is like:

CustomResourceDefinition.apiextensions.k8s.io "..." is invalid: spec.validation.openAPIV3Schema...default: Unsupported value: "": supported values: "a", "b"

jingyuanliang avatar May 24 '23 19:05 jingyuanliang

/cc @apelisse @jiahuif @alexzielenski

Jefftree avatar May 26 '23 20:05 Jefftree

So you're using the test code to make a CRD and the CRD is invalid? From a very cursory look, that seems normal and expected, but the examples included in the tests should probably be valid. Alternative, we could also detect if the default is not part of the enum at generation time, and fail at that time, that'd probably be better.

apelisse avatar May 30 '23 16:05 apelisse

I'm using code generated by kube-openapi with an enum to make a CRD and the CRD can't be made for having an invalid default value, because the default zero value is not one of the enum values. I didn't provide the default value and the generator chose a zero value based on datatype. The example included in tests is not valid for the same reason, and that's why I showcased it using the example in tests.

jingyuanliang avatar May 30 '23 17:05 jingyuanliang

Looked at it again. It looks like the value is REQUIRED anyway, the default is kind of forced because the field is not a pointer, so I would say this works as expected. There's not really any other great way to do that.

apelisse avatar Jun 07 '23 20:06 apelisse

Uhm I worked around it by removing the Default: "", line and it worked. Is this "REQUIRED" not enforced?

jingyuanliang avatar Jun 07 '23 21:06 jingyuanliang

Yeah, that's a little weird I agree. Also not entirely sure depending on your exact situation. (though required should ALWAYS be respected tbh).

apelisse avatar Jun 07 '23 21:06 apelisse

Can we have the default value use the first enum value then?

jingyuanliang avatar Jun 07 '23 22:06 jingyuanliang

Looked at it again. It looks like the value is REQUIRED anyway, the default is kind of forced because the field is not a pointer, so I would say this works as expected. There's not really any other great way to do that.

IMO the schema is invalid if the default value wouldn’t pass validation itself. The schema generator should throw an error for that

alexzielenski avatar Jun 07 '23 23:06 alexzielenski

yeah, I don't think that's wrong. I'm not exactly sure why we have a default here, I need to double check.

apelisse avatar Jun 08 '23 22:06 apelisse

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Jan 22 '24 04:01 k8s-triage-robot

/remove-lifecycle stale

jingyuanliang avatar Jan 22 '24 04:01 jingyuanliang

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Apr 21 '24 05:04 k8s-triage-robot

/remove-lifecycle stale

jingyuanliang avatar Apr 22 '24 20:04 jingyuanliang

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Jul 21 '24 20:07 k8s-triage-robot

/remove-lifecycle stale

jingyuanliang avatar Jul 22 '24 01:07 jingyuanliang