-
Notifications
You must be signed in to change notification settings - Fork 1.8k
generate: add packagemanifests subcommand for new project layouts
#3096
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
Changes from all commits
d46f320
284902a
018bbc6
f3242b2
dfd161e
598931b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| entries: | ||
| - description: add `generate packagemanifests` subcommand for new project layouts | ||
| kind: addition |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| // Copyright 2020 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 packagemanifests | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| log "github.com/sirupsen/logrus" | ||
| "github.com/spf13/cobra" | ||
| "github.com/spf13/pflag" | ||
|
|
||
| kbutil "github.com/operator-framework/operator-sdk/internal/util/kubebuilder" | ||
| "github.com/operator-framework/operator-sdk/internal/util/projutil" | ||
| ) | ||
|
|
||
| const longHelp = ` | ||
| Note: while the package manifests format is not yet deprecated, the operator-framework is migrated | ||
| towards using bundles by default. Run 'operator-sdk generate bundle -h' for more information. | ||
|
|
||
| Running 'generate packagemanifests' is the first step to publishing your operator to a catalog | ||
| and/or deploying it with OLM. This command generates a set of manifests in a versioned directory | ||
| and a package manifest file for your operator. It will interactively ask for UI metadata, | ||
| an important component of publishing your operator, by default unless a package for your | ||
| operator exists or you set '--interactive=false'. | ||
|
|
||
| Set '--version' to supply a semantic version for your new package. This is a required flag when running | ||
| 'generate packagemanifests --manifests'. | ||
|
|
||
| More information on the package manifests format: | ||
| https://github.com/operator-framework/operator-registry/#manifest-format | ||
| ` | ||
|
|
||
| //nolint:maligned | ||
| type packagemanifestsCmd struct { | ||
| // Options to turn on different parts of packaging. | ||
| kustomize bool | ||
| manifests bool | ||
|
|
||
| // Common options. | ||
| operatorName string | ||
| version string | ||
| fromVersion string | ||
| inputDir string | ||
| outputDir string | ||
| deployDir string | ||
| apisDir string | ||
| crdsDir string | ||
| updateCRDs bool | ||
| stdout bool | ||
| quiet bool | ||
|
|
||
| // Interactive options. | ||
| interactiveLevel projutil.InteractiveLevel | ||
| interactive bool | ||
|
|
||
| // Package manifest options. | ||
| channelName string | ||
| isDefaultChannel bool | ||
| } | ||
|
|
||
| // NewCmd returns the 'packagemanifests' command configured for the new project layout. | ||
| func NewCmd() *cobra.Command { | ||
| c := &packagemanifestsCmd{} | ||
|
|
||
| cmd := &cobra.Command{ | ||
| Use: "packagemanifests", | ||
| Short: "Generates a package manifests format", | ||
| Long: longHelp, | ||
| Example: examples, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it might be worth it to separate some of this out into functions instead of declaring it all inline. Maybe a validate() and a run() like in some of the other commands?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be better in a follow-up because both |
||
| if len(args) != 0 { | ||
| return fmt.Errorf("command %s doesn't accept any arguments", cmd.CommandPath()) | ||
| } | ||
|
|
||
| // Check if the user has any specific preference to enable/disable | ||
| // interactive prompts. Default behaviour is to disable the prompt | ||
| // unless a base package does not exist. | ||
| if cmd.Flags().Changed("interactive") { | ||
| if c.interactive { | ||
| c.interactiveLevel = projutil.InteractiveOnAll | ||
| } else { | ||
| c.interactiveLevel = projutil.InteractiveHardOff | ||
| } | ||
| } | ||
|
|
||
| // Generate kustomize bases and manifests by default if no flags are set | ||
| // so the default behavior is "do everything". | ||
| fs := cmd.Flags() | ||
| if !fs.Changed("kustomize") && !fs.Changed("manifests") { | ||
| c.kustomize = true | ||
| c.manifests = true | ||
| } | ||
|
|
||
| cfg, err := kbutil.ReadConfig() | ||
| if err != nil { | ||
| log.Fatal(fmt.Errorf("error reading configuration: %v", err)) | ||
| } | ||
| c.setCommonDefaults(cfg) | ||
|
|
||
| if c.kustomize { | ||
| if err = c.runKustomize(cfg); err != nil { | ||
| log.Fatalf("Error generating package bases: %v", err) | ||
| } | ||
| } | ||
| if c.manifests { | ||
| if err = c.validateManifests(); err != nil { | ||
| return fmt.Errorf("invalid command options: %v", err) | ||
| } | ||
| if err = c.runManifests(cfg); err != nil { | ||
| log.Fatalf("Error generating package manifests: %v", err) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| }, | ||
| } | ||
|
|
||
| cmd.Flags().BoolVar(&c.kustomize, "kustomize", false, "Generate kustomize bases") | ||
| cmd.Flags().BoolVar(&c.manifests, "manifests", false, "Generate package manifests") | ||
| cmd.Flags().BoolVar(&c.stdout, "stdout", false, "Write package to stdout") | ||
|
|
||
| c.addCommonFlagsTo(cmd.Flags()) | ||
|
|
||
| return cmd | ||
| } | ||
|
|
||
| func (c *packagemanifestsCmd) addCommonFlagsTo(fs *pflag.FlagSet) { | ||
| fs.StringVar(&c.operatorName, "operator-name", "", "Name of the packaged operator") | ||
| fs.StringVarP(&c.version, "version", "v", "", "Semantic version of the packaged operator") | ||
|
estroz marked this conversation as resolved.
|
||
| fs.StringVar(&c.inputDir, "input-dir", "", "Directory to read existing package manifests from. "+ | ||
| "This directory is the parent of individual versioned package directories, and different from --deploy-dir") | ||
| fs.StringVar(&c.outputDir, "output-dir", "", "Directory in which to write package manifests") | ||
| fs.StringVar(&c.deployDir, "deploy-dir", "", "Root directory for operator manifests such as "+ | ||
| "Deployments and RBAC, ex. 'deploy'. This directory is different from that passed to --input-dir") | ||
| fs.StringVar(&c.apisDir, "apis-dir", "", "Root directory for API type defintions") | ||
| fs.StringVar(&c.crdsDir, "crds-dir", "", "Root directory for CustomResoureDefinition manifests") | ||
| fs.StringVar(&c.channelName, "channel", "", "Channel name for the generated package") | ||
| fs.BoolVar(&c.isDefaultChannel, "default-channel", false, "Use the channel passed to --channel "+ | ||
| "as the package manifest file's default channel") | ||
| fs.BoolVar(&c.updateCRDs, "update-crds", true, "Update CustomResoureDefinition manifests in this package") | ||
| fs.BoolVarP(&c.quiet, "quiet", "q", false, "Run in quiet mode") | ||
| fs.BoolVar(&c.interactive, "interactive", false, "When set or no package base exists, an interactive "+ | ||
| "command prompt will be presented to accept package ClusterServiceVersion metadata") | ||
| } | ||
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.
It seems very little of the cmd layer of the CLI is tested. Do we have any goals to add tests for all of this? I'm nervous about this much untested code.
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.
We have some command-level tests here for legacy project commands, and will likely add similar tests for new project commands. The "happy path" for bew project commands is tested in these e2e tests, and we have plans to add more.
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.
do you think it would be worthwhile to add unit tests here?
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.
Most of these command files are just configuration for the underlying generator, so at this point unit tests would just be testing different inputs which is covered by the e2e and subcommand tests I linked above. I'd like to add subcommand tests in a follow-up. However I will add an e2e test here.
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'd be down to hash out some unit tests for these bits.