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

Generated enum fails validation when used for CRD creation #395

Open
jingyuanliang opened this issue May 24, 2023 · 13 comments · May be fixed by #407
Open

Generated enum fails validation when used for CRD creation #395

jingyuanliang opened this issue May 24, 2023 · 13 comments · May be fixed by #407

Comments

@jingyuanliang
Copy link

jingyuanliang commented May 24, 2023

func TestEnum(t *testing.T) {

This shows an example of generated code:

"Value": {
SchemaProps: spec.SchemaProps{`+"\n"+
"Description: \"Value is the value.\\n\\nPossible enum values:\\n - `\\\"a\\\"` is a.\\n - `\\\"b\\\"` is b.\","+`
Default: "",
Type: []string{"string"},
Format: "",
Enum: []interface{}{"a", "b"},
},

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"
@Jefftree
Copy link
Member

/cc @apelisse @jiahuif @alexzielenski

@apelisse
Copy link
Member

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.

@jingyuanliang
Copy link
Author

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.

@apelisse
Copy link
Member

apelisse commented Jun 7, 2023

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.

@jingyuanliang
Copy link
Author

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

@apelisse
Copy link
Member

apelisse commented Jun 7, 2023

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

@jingyuanliang
Copy link
Author

jingyuanliang commented Jun 7, 2023

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

@alexzielenski
Copy link
Contributor

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

@apelisse
Copy link
Member

apelisse commented Jun 8, 2023

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 apelisse linked a pull request Jun 12, 2023 that will close this issue
@k8s-triage-robot
Copy link

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-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 Jan 22, 2024
@jingyuanliang
Copy link
Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 22, 2024
@k8s-triage-robot
Copy link

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-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 Apr 21, 2024
@jingyuanliang
Copy link
Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants