Deprecate 'generate openapi', add 'generate crds' #2276
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // Copyright 2019 The Operator-SDK Authors | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package generate | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/operator-framework/operator-sdk/cmd/operator-sdk/internal/genutil" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| func newGenerateCRDsCmd() *cobra.Command { | ||
|
There was a problem hiding this comment. can we add a flag for passing in the location of the CRD's? There was a problem hiding this comment. Once this merges, I'll make an issue to discuss this idea for a follow-up. |
||
| return &cobra.Command{ | ||
| Use: "crds", | ||
| Short: "Generates CRDs for API's", | ||
| Long: `generate crds generates CRDs or updates them if they exist, | ||
| under deploy/crds/<full group>_<resource>_crd.yaml; OpenAPI | ||
| V3 validation YAML is generated as a 'validation' object. | ||
|
|
||
| Example: | ||
|
|
||
| $ operator-sdk generate crds | ||
| $ tree deploy/crds | ||
| ├── deploy/crds/app.example.com_v1alpha1_appservice_cr.yaml | ||
| ├── deploy/crds/app.example.com_appservices_crd.yaml | ||
| `, | ||
| RunE: crdsFunc, | ||
| } | ||
| } | ||
|
|
||
| func crdsFunc(cmd *cobra.Command, args []string) error { | ||
| if len(args) != 0 { | ||
| return fmt.Errorf("command %s doesn't accept any arguments", cmd.CommandPath()) | ||
| } | ||
|
|
||
| return genutil.CRDGen() | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| // Copyright 2018 The Operator-SDK Authors | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package genutil | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/operator-framework/operator-sdk/internal/scaffold" | ||
| "github.com/operator-framework/operator-sdk/internal/scaffold/input" | ||
| "github.com/operator-framework/operator-sdk/internal/util/k8sutil" | ||
| "github.com/operator-framework/operator-sdk/internal/util/projutil" | ||
|
|
||
| log "github.com/sirupsen/logrus" | ||
| ) | ||
|
|
||
| // CRDGen generates CRDs for all APIs in pkg/apis. | ||
| func CRDGen() error { | ||
| projutil.MustInProjectRoot() | ||
|
|
||
| absProjectPath := projutil.MustGetwd() | ||
| repoPkg := projutil.GetGoPkg() | ||
|
|
||
| gvMap, err := k8sutil.ParseGroupSubpackages(scaffold.ApisDir) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to parse group versions: (%v)", err) | ||
| } | ||
| gvb := &strings.Builder{} | ||
| for g, vs := range gvMap { | ||
| gvb.WriteString(fmt.Sprintf("%s:%v, ", g, vs)) | ||
| } | ||
|
|
||
| log.Infof("Running CRD generation for Custom Resource group versions: [%v]\n", gvb.String()) | ||
|
|
||
| s := &scaffold.Scaffold{} | ||
| cfg := &input.Config{ | ||
| Repo: repoPkg, | ||
| AbsProjectPath: absProjectPath, | ||
| ProjectName: filepath.Base(absProjectPath), | ||
| } | ||
| crds, err := k8sutil.GetCRDs(scaffold.CRDsDir) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| for _, crd := range crds { | ||
| g, v, k := crd.Spec.Group, crd.Spec.Version, crd.Spec.Names.Kind | ||
| if v == "" { | ||
| if len(crd.Spec.Versions) != 0 { | ||
| v = crd.Spec.Versions[0].Name | ||
| } else { | ||
| return fmt.Errorf("crd of group %s kind %s has no version", g, k) | ||
| } | ||
| } | ||
| r, err := scaffold.NewResource(g+"/"+v, k) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = s.Execute(cfg, | ||
| &scaffold.CRD{Resource: r, IsOperatorGo: projutil.IsOperatorGo()}, | ||
| ) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| log.Info("CRD generation complete.") | ||
| return nil | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| ## operator-sdk generate crds | ||
|
|
||
| Generates CRDs for API's | ||
|
|
||
| ### Synopsis | ||
|
|
||
| generate crds generates CRDs or updates them if they exist, | ||
| under deploy/crds/<full group>_<resource>_crd.yaml; OpenAPI | ||
| V3 validation YAML is generated as a 'validation' object. | ||
|
|
||
| Example: | ||
|
|
||
| $ operator-sdk generate crds | ||
| $ tree deploy/crds | ||
| ├── deploy/crds/app.example.com_v1alpha1_appservice_cr.yaml | ||
| ├── deploy/crds/app.example.com_appservices_crd.yaml | ||
|
|
||
|
|
||
| ``` | ||
| operator-sdk generate crds [flags] | ||
| ``` | ||
|
|
||
| ### Options | ||
|
|
||
| ``` | ||
| -h, --help help for crds | ||
| ``` | ||
|
|
||
| ### SEE ALSO | ||
|
|
||
| * [operator-sdk generate](operator-sdk_generate.md) - Invokes specific generator | ||
|
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if we should move forward with. See that kubebuilder is not supporting it and users are asking for here and a track issue from them here which in my understand shows that require it as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first issue is fair; based on feedback in #2042 some users do not use
zz_generated.openapi.go, which, on top of it not being necessary for API code unlikezz_generated.deepcopy.go, is why I decided to push for removing it. We should either:openapi-gento generate the files yourself, which is fairly straightforward.generate openapias a command but move CRD generation intogenerate crd.I vote for option 1 since we can offer Go OpenAPI code by wrapping
openapi-genin a Makefile recipe (in the future).The second issue describes something we do not currently support in API code right now since we use the same tool as kubebuilder to generate CRDs,
controller-genfrom controller-tools. Generatingzz_generated.openapi.godoes not change the state of supported annotations for CRD manifest validation in the SDK.We agreed to make this change because it de-couples CRD generation from OpenAPI spec generation, two different albeit related processes. This change will help users debug errors more easily as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option would be to add
generate crdsbut keepgenerate openapias is and deprecate it. We could hide it from the help output and log a deprecation message when it is called.Then in a later version it could be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And we could/should still do one of the options @estroz suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joelanford I like the combination of what you suggested and my first option. The docs referencing Go OpenAPI spec generation (I believe the only documentation doing so is the one being changed in this PR already) should warn that the subcommand is deprecated and users should switch to installing and running
openapi-genthemselves if they still want that spec file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea of phasing out the old command with a deprecation warning.
@estroz @joelanford we need to make it easy to download and use the openapi-gen command if that is the guidance, we either do it for the user and attempt to install the binary into a known location and give them the option to supply their own openapi-gen.
Could we vendor the openapi-gen command and use it in the operator-sdk command? was that an option already looked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shawn-hurley we currently vendor the exposed
openapi-gengenerator ingenerate openapi. If we want to install this binary for the user, I suggest this as the first candidate for a Makefile:Otherwise documentation to the effect of the above script should suffice. I'd rather have documentation than a make recipe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, our vendoring of the code generation libraries is what causes the GOROOT and panic issues described in this issue: #1854 (comment)
If we direct users to build
openapi-genfrom source like Eric suggests, we'll help them avoid that issue, at least for the openapi generation (I think it will still be an issue for deepcopy generation).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, updated the PR to keep
generate openapi, but to hide it from help and deprecate it.