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.

Sign up for GitHub

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

Jump to bottom

Create CRD validation specs in CRD manifests and as Go code #869

Merged
merged 49 commits into from
Jan 31, 2019
Merged

Create CRD validation specs in CRD manifests and as Go code #869

merged 49 commits into from
Jan 31, 2019

Conversation

estroz
Copy link
Member

@estroz estroz commented Dec 19, 2018 •
edited
Loading

Description of the change: operator-sdk generate openapi generates OpenAPI validation specs for CRD manifests and as Go code under pkg/apis/<group>/<version>/zz_generated.openapi.go. operator-sdk add api also calls the openapi generator. The CRD scaffold now implements CustomRenderer to generate a CRD using controller-tools' CRD generator by default; if some CRD manifest exists, CustomRender will only set the validation spec of that CRD.

Motivation for the change: validation specs should be present for all CRD's, yaml or otherwise. Users might want to use the Go specs for Go-defined CRD's.

Note: we can remove Go spec generator, as there haven't been requests for one. I thought it would be a nice addition but not necessary. I'm also open to not having a separate generate command, and having all generation under generate k8s.

I'll add documentation in another PR since this one is already large.

Closes #256

@openshift-ci-robot openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Dec 19, 2018
@estroz estroz force-pushed the crd-validation branch 2 times, most recently from 646711a to 6e3f3e1 Compare December 20, 2018 05:44
@estroz
commands/.../generate/*: build and call openapi-gen, and re-scaffold …
c8cccc8 
…CRD manifests

commands/.../add/api.go: call OpenAPI generator function

pkg/scaffold/crd*: use CustomRenderer interface to write CRD manifests
with validation spec instead of a template

pkg/scaffold/gopkgtoml*: include openapi-gen deps

pkg/scaffold/types*: add openapi-gen directives
@estroz
fix comment
0e5fb86
@estroz
only use controller-tools CRD generator if in a Go project
7db89b6
@estroz
change dir in unit test so Go project is detected
82c09e2
@estroz
add names and only overwrite dstCrd when a file at path was generated
cdb791c 
remove status field from resulting struct
@estroz estroz added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 20, 2018
@estroz estroz mentioned this pull request Dec 20, 2018
Merged
@solcates
Copy link

solcates commented Jan 3, 2019

Any update on this PR? Seems pretty solid, have been testing it with some basic types to CRD generation, and so far so good

hasbro17
hasbro17 approved these changes Jan 28, 2019
View reviewed changes
Copy link
Contributor

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after nits.

@estroz As we discussed offline I'm fine with having users using the +kubebuilder:validation tags to add the custom validation directives.

But as a follow up to this PR we should try to look into not stomping out any validation directives that are added directly to the CRD yaml manifest on running generate openapi.

@hasbro17
Copy link
Contributor

Also the CHANGELOG and cli-reference guide need to be updated but it's fine if you have that lined up in follow up PR to add the docs.

@estroz estroz force-pushed the crd-validation branch 2 times, most recently from 0f5a186 to 613ac72 Compare January 30, 2019 16:07
@estroz
Copy link
Member Author

estroz commented Jan 30, 2019

The additions in this PR to the generated Gopkg.toml are breaking dep. I am in the process of figuring out which packages are in conflict.

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 31, 2019
@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 31, 2019
@estroz estroz merged commit c81f98c into operator-framework:master Jan 31, 2019
@estroz estroz deleted the crd-validation branch January 31, 2019 20:06
@estroz estroz mentioned this pull request Jan 31, 2019
Merged
@estroz estroz mentioned this pull request Mar 1, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lilic lilic lilic approved these changes

@joelanford joelanford joelanford approved these changes

@shawn-hurley shawn-hurley shawn-hurley approved these changes

@AlexNPavel AlexNPavel AlexNPavel approved these changes

@hasbro17 hasbro17 hasbro17 approved these changes

@mhrivnak mhrivnak Awaiting requested review from mhrivnak

Assignees
No one assigned
Labels
kind/feature Categorizes issue or PR as related to a new feature. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@estroz @solcates @joelanford @hasbro17 @shawn-hurley @AlexNPavel @lilic @openshift-ci-robot