From d46f3208bc9f3921596b1bab468757580dd1802f Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Fri, 22 May 2020 12:55:06 -0700 Subject: [PATCH 1/6] generate: add `packagemanifests` subcommand for new project layouts --- .../fragments/generate-packagemanifests.yaml | 3 + cmd/operator-sdk/generate/cmd.go | 2 + .../generate/packagemanifests/cmd.go | 138 +++++++++ .../packagemanifests/packagemanifests.go | 216 ++++++++++++++ .../clusterserviceversion.go | 19 +- .../clusterserviceversion_test.go | 185 ++++++------ .../clusterserviceversion_updaters.go | 3 +- internal/generate/collector/collect.go | 10 +- .../packagemanifest/bases/packagemanifest.go | 67 +++++ .../packagemanifest/packagemanifest.go | 230 +++++++++++++++ .../packagemanifest/packagemanifest_test.go | 274 ++++++++++++++++++ .../testdata/memcached-operator.package.yaml | 5 + test/e2e-new/e2e_suite.go | 25 +- 13 files changed, 1078 insertions(+), 99 deletions(-) create mode 100644 changelog/fragments/generate-packagemanifests.yaml create mode 100644 cmd/operator-sdk/generate/packagemanifests/cmd.go create mode 100644 cmd/operator-sdk/generate/packagemanifests/packagemanifests.go create mode 100644 internal/generate/packagemanifest/bases/packagemanifest.go create mode 100644 internal/generate/packagemanifest/packagemanifest.go create mode 100644 internal/generate/packagemanifest/packagemanifest_test.go create mode 100644 internal/generate/testdata/memcached-operator.package.yaml diff --git a/changelog/fragments/generate-packagemanifests.yaml b/changelog/fragments/generate-packagemanifests.yaml new file mode 100644 index 0000000000..d20f1720c3 --- /dev/null +++ b/changelog/fragments/generate-packagemanifests.yaml @@ -0,0 +1,3 @@ +entries: + - description: add `generate packagemanifests` subcommand for new project layouts + kind: addition diff --git a/cmd/operator-sdk/generate/cmd.go b/cmd/operator-sdk/generate/cmd.go index 694dae99e8..2dc10f6891 100644 --- a/cmd/operator-sdk/generate/cmd.go +++ b/cmd/operator-sdk/generate/cmd.go @@ -18,6 +18,7 @@ import ( "github.com/spf13/cobra" "github.com/operator-framework/operator-sdk/cmd/operator-sdk/generate/bundle" + "github.com/operator-framework/operator-sdk/cmd/operator-sdk/generate/packagemanifests" ) func newCmd() *cobra.Command { @@ -34,6 +35,7 @@ func NewCmd() *cobra.Command { cmd := newCmd() cmd.AddCommand( bundle.NewCmd(), + packagemanifests.NewCmd(), ) return cmd } diff --git a/cmd/operator-sdk/generate/packagemanifests/cmd.go b/cmd/operator-sdk/generate/packagemanifests/cmd.go new file mode 100644 index 0000000000..118045ce0e --- /dev/null +++ b/cmd/operator-sdk/generate/packagemanifests/cmd.go @@ -0,0 +1,138 @@ +// 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" +) + +//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", + RunE: func(cmd *cobra.Command, args []string) error { + 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") + 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", false, "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") +} diff --git a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go new file mode 100644 index 0000000000..151d9a3401 --- /dev/null +++ b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go @@ -0,0 +1,216 @@ +// 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 ( + "errors" + "fmt" + "os" + "path/filepath" + + "sigs.k8s.io/kubebuilder/pkg/model/config" + + genutil "github.com/operator-framework/operator-sdk/cmd/operator-sdk/generate/internal" + gencsv "github.com/operator-framework/operator-sdk/internal/generate/clusterserviceversion" + "github.com/operator-framework/operator-sdk/internal/generate/collector" + genpkg "github.com/operator-framework/operator-sdk/internal/generate/packagemanifest" +) + +// setCommonDefaults sets defaults useful to all modes of this subcommand. +func (c *packagemanifestsCmd) setCommonDefaults(cfg *config.Config) { + if c.operatorName == "" { + c.operatorName = filepath.Base(cfg.Repo) + } +} + +// runKustomize generates kustomize package bases. +func (c packagemanifestsCmd) runKustomize(cfg *config.Config) error { + + if !c.quiet { + fmt.Println("Generating package manifests kustomize bases") + } + + defaultDir := filepath.Join("config", "packages") + if c.inputDir == "" { + c.inputDir = defaultDir + } + if c.outputDir == "" { + c.outputDir = defaultDir + } + if c.apisDir == "" { + if cfg.MultiGroup { + c.apisDir = "apis" + } else { + c.apisDir = "api" + } + } + + csvGen := gencsv.Generator{ + OperatorName: c.operatorName, + OperatorType: genutil.PluginKeyToOperatorType(cfg.Layout), + } + opts := []gencsv.Option{ + gencsv.WithBase(c.inputDir, c.apisDir, c.interactiveLevel), + gencsv.WithBaseWriter(c.outputDir), + } + if err := csvGen.Generate(cfg, opts...); err != nil { + return fmt.Errorf("error generating ClusterServiceVersion: %v", err) + } + + if !c.quiet { + fmt.Println("Bases generated successfully in", c.outputDir) + } + + return nil +} + +// validateManifests validates c for package manifests generation. +func (c packagemanifestsCmd) validateManifests() error { + + if err := genutil.ValidateVersion(c.version); err != nil { + return err + } + + if c.fromVersion != "" { + return errors.New("--from-version cannot be set for PROJECT configured projects") + } + + if !genutil.IsPipeReader() { + if c.deployDir == "" { + return errors.New("--deploy-dir must be set if not reading from stdin") + } + if c.crdsDir == "" { + return errors.New("--crd-dir must be set if not reading from stdin") + } + } + + if c.stdout { + if c.outputDir != "" { + return errors.New("--output-dir cannot be set if writing to stdout") + } + } + + if c.isDefaultChannel && c.channelName == "" { + return fmt.Errorf("--default-channel can only be set if --channel is set") + } + + return nil +} + +// runManifests generates package manifests. +func (c packagemanifestsCmd) runManifests(cfg *config.Config) error { + + if !c.quiet && !c.stdout { + fmt.Println("Generating package manifests version", c.version) + } + + defaultDir := filepath.Join("config", "packages") + if c.inputDir == "" { + c.inputDir = defaultDir + } + if !c.stdout { + if c.outputDir == "" { + c.outputDir = defaultDir + } + } + // Only regenerate API definitions once. + if c.apisDir == "" && !c.kustomize { + if cfg.MultiGroup { + c.apisDir = "apis" + } else { + c.apisDir = "api" + } + } + + if err := c.generatePackageManifest(); err != nil { + return err + } + + col := &collector.Manifests{} + if genutil.IsPipeReader() { + if err := col.UpdateFromReader(os.Stdin); err != nil { + return err + } + } + if c.deployDir != "" { + if err := col.UpdateFromDirs(c.deployDir, c.crdsDir); err != nil { + return err + } + } + + csvGen := gencsv.Generator{ + OperatorName: c.operatorName, + OperatorType: genutil.PluginKeyToOperatorType(cfg.Layout), + Version: c.version, + Collector: col, + } + + stdout := genutil.NewMultiManifestWriter(os.Stdout) + opts := []gencsv.Option{ + gencsv.WithBase(c.inputDir, c.apisDir, c.interactiveLevel), + } + if c.stdout { + opts = append(opts, gencsv.WithWriter(stdout)) + } else { + opts = append(opts, gencsv.WithPackageWriter(c.outputDir)) + } + + if err := csvGen.Generate(cfg, opts...); err != nil { + return fmt.Errorf("error generating ClusterServiceVersion: %v", err) + } + + if c.updateCRDs { + var objs []interface{} + for _, crd := range col.V1CustomResourceDefinitions { + objs = append(objs, crd) + } + for _, crd := range col.V1beta1CustomResourceDefinitions { + objs = append(objs, crd) + } + if c.stdout { + if err := genutil.WriteObjects(stdout, objs...); err != nil { + return err + } + } else { + dir := filepath.Join(c.outputDir, c.version) + if err := genutil.WriteObjectsToFiles(dir, objs...); err != nil { + return err + } + } + } + + if !c.quiet && !c.stdout { + fmt.Println("Package manifests generated successfully in", c.outputDir) + } + + return nil +} + +func (c packagemanifestsCmd) generatePackageManifest() error { + pkgGen := genpkg.Generator{ + OperatorName: c.operatorName, + Version: c.version, + ChannelName: c.channelName, + IsDefaultChannel: c.isDefaultChannel, + } + opts := []genpkg.Option{ + genpkg.WithBase(c.inputDir), + genpkg.WithFileWriter(c.outputDir), + } + if err := pkgGen.Generate(opts...); err != nil { + return err + } + return nil +} diff --git a/internal/generate/clusterserviceversion/clusterserviceversion.go b/internal/generate/clusterserviceversion/clusterserviceversion.go index a261fb24eb..be5bb5c445 100644 --- a/internal/generate/clusterserviceversion/clusterserviceversion.go +++ b/internal/generate/clusterserviceversion/clusterserviceversion.go @@ -48,12 +48,14 @@ var ( // ClusterServiceVersion configures ClusterServiceVersion manifest generation. type Generator struct { - // OperatorName is the operator's name, ex. app-operator + // OperatorName is the operator's name, ex. app-operator. OperatorName string // OperatorType determines what code API types are written in for getBase. OperatorType projutil.OperatorType // Version is the CSV current version. Version string + // FromVersion is the version of a previous CSV to upgrade from from. + FromVersion string // Collector holds all manifests relevant to the Generator. Collector *collector.Manifests @@ -125,6 +127,21 @@ func WithBundleWriter(dir string) Option { } } +// WithPackageWriter sets a Generator's writer to a package CSV file under +// /. +func WithPackageWriter(dir string) Option { + return func(g *Generator) error { + fileName := makeCSVFileName(g.OperatorName) + if g.FromVersion != "" { + g.bundledPath = filepath.Join(dir, g.FromVersion, fileName) + } + g.getWriter = func() (io.Writer, error) { + return genutil.Open(filepath.Join(dir, g.Version), fileName) + } + return nil + } +} + // Generate configures the generator with cfg and opts then runs it. func (g *Generator) Generate(cfg *config.Config, opts ...Option) (err error) { g.config = cfg diff --git a/internal/generate/clusterserviceversion/clusterserviceversion_test.go b/internal/generate/clusterserviceversion/clusterserviceversion_test.go index d08da052c1..a8101e007d 100644 --- a/internal/generate/clusterserviceversion/clusterserviceversion_test.go +++ b/internal/generate/clusterserviceversion/clusterserviceversion_test.go @@ -48,10 +48,10 @@ var ( // dir and a "deploy" dir that contains `kustomize build config/default` // output to simulate actual manifest collection behavior. Using "config" // directly is not standard behavior. - goTestDataDir = filepath.Join(testDataDir, "non-standard-layout") - goAPIsDir = filepath.Join(goTestDataDir, "api") - goManifestRootDir = filepath.Join(goTestDataDir, "config") - goCRDsDir = filepath.Join(goManifestRootDir, "crds") + goTestDataDir = filepath.Join(testDataDir, "non-standard-layout") + goAPIsDir = filepath.Join(goTestDataDir, "api") + goConfigDir = filepath.Join(goTestDataDir, "config") + goCRDsDir = filepath.Join(goConfigDir, "crds") ) var ( @@ -66,7 +66,7 @@ var ( var _ = BeforeSuite(func() { col = &collector.Manifests{} - Expect(col.UpdateFromDirs(goManifestRootDir, goCRDsDir)).ToNot(HaveOccurred()) + Expect(col.UpdateFromDirs(goConfigDir, goCRDsDir)).ToNot(HaveOccurred()) cfg = readConfigHelper(goTestDataDir) @@ -100,7 +100,7 @@ var _ = Describe("Generating a ClusterServiceVersion", func() { BeforeEach(func() { tmp, err = ioutil.TempDir(".", "") - ExpectWithOffset(1, err).ToNot(HaveOccurred()) + Expect(err).ToNot(HaveOccurred()) }) AfterEach(func() { @@ -153,8 +153,24 @@ var _ = Describe("Generating a ClusterServiceVersion", func() { Expect(outputFile).To(BeAnExistingFile()) Expect(string(readFileHelper(outputFile))).To(MatchYAML(newCSVStr)) }) + It("should write a ClusterServiceVersion manifest to a package file", func() { + g = Generator{ + OperatorName: operatorName, + OperatorType: operatorType, + Version: version, + Collector: col, + } + opts := []Option{ + WithBase(csvBasesDir, goAPIsDir, projutil.InteractiveHardOff), + WithPackageWriter(tmp), + } + Expect(g.Generate(cfg, opts...)).ToNot(HaveOccurred()) + outputFile := filepath.Join(tmp, g.Version, makeCSVFileName(operatorName)) + Expect(outputFile).To(BeAnExistingFile()) + Expect(string(readFileHelper(outputFile))).To(MatchYAML(newCSVStr)) + }) - It("should write a ClusterServiceVersion manifest to a legacy base/bundle file", func() { + It("should write a ClusterServiceVersion manifest to a legacy bundle file", func() { g = Generator{ OperatorName: operatorName, OperatorType: operatorType, @@ -218,98 +234,87 @@ var _ = Describe("Generating a ClusterServiceVersion", func() { }) }) - Context("to create a new", func() { - - Context("bundle base", func() { - It("should return the default base object", func() { - g = Generator{ - OperatorName: operatorName, - OperatorType: operatorType, - config: cfg, - getBase: makeBaseGetter(baseCSV), - } - csv, err := g.generate() - Expect(err).ToNot(HaveOccurred()) - Expect(csv).To(Equal(baseCSV)) - }) - It("should return a base object with customresourcedefinitions", func() { - g = Generator{ - OperatorName: operatorName, - OperatorType: operatorType, - config: cfg, - getBase: makeBaseGetter(baseCSVUIMeta), - } - csv, err := g.generate() - Expect(err).ToNot(HaveOccurred()) - Expect(csv).To(Equal(baseCSVUIMeta)) - }) + Context("to create a new base ClusterServiceVersion", func() { + It("should return the default base object", func() { + g = Generator{ + OperatorName: operatorName, + OperatorType: operatorType, + config: cfg, + getBase: makeBaseGetter(baseCSV), + } + csv, err := g.generate() + Expect(err).ToNot(HaveOccurred()) + Expect(csv).To(Equal(baseCSV)) }) - - Context("bundle", func() { - It("should return the expected object", func() { - g = Generator{ - OperatorName: operatorName, - OperatorType: operatorType, - Version: version, - Collector: col, - config: cfg, - getBase: makeBaseGetter(baseCSVUIMeta), - } - csv, err := g.generate() - Expect(err).ToNot(HaveOccurred()) - Expect(csv).To(Equal(newCSV)) - }) + It("should return a base object with customresourcedefinitions", func() { + g = Generator{ + OperatorName: operatorName, + OperatorType: operatorType, + config: cfg, + getBase: makeBaseGetter(baseCSVUIMeta), + } + csv, err := g.generate() + Expect(err).ToNot(HaveOccurred()) + Expect(csv).To(Equal(baseCSVUIMeta)) }) - }) - Context("to update an existing", func() { - Context("bundle", func() { - It("should return the expected object", func() { - g = Generator{ - OperatorName: operatorName, - OperatorType: operatorType, - Version: version, - Collector: &collector.Manifests{}, - config: cfg, - getBase: makeBaseGetter(newCSV), - } - // Update the input's and expected CSV's Deployment image. - Expect(g.Collector.UpdateFromDirs(goManifestRootDir, goCRDsDir)).ToNot(HaveOccurred()) - Expect(len(g.Collector.Deployments)).To(BeNumerically(">=", 1)) - imageTag := "controller:v" + g.Version - modifyDepImageHelper(&g.Collector.Deployments[0].Spec, imageTag) - updatedCSV := updateCSV(newCSV, modifyCSVDepImageHelper(imageTag)) - - csv, err := g.generate() - Expect(err).ToNot(HaveOccurred()) - Expect(csv).To(Equal(updatedCSV)) - }) + Context("to create a new ClusterServiceVersion", func() { + It("should return a new object", func() { + g = Generator{ + OperatorName: operatorName, + OperatorType: operatorType, + Version: version, + Collector: col, + config: cfg, + getBase: makeBaseGetter(baseCSVUIMeta), + } + csv, err := g.generate() + Expect(err).ToNot(HaveOccurred()) + Expect(csv).To(Equal(newCSV)) }) - }) - Context("to upgrade an existing", func() { - - Context("bundle", func() { - It("should return the expected manifest", func() { - g = Generator{ - OperatorName: operatorName, - OperatorType: operatorType, - Version: "0.0.2", - Collector: col, - config: cfg, - getBase: makeBaseGetter(newCSV), - // Bundles need a path, usually set by an Option, to an existing - // CSV manifest so "replaces" can be set correctly. - bundledPath: filepath.Join(csvNewLayoutBundleDir, "memcached-operator.clusterserviceversion.yaml"), - } - csv, err := g.generate() - Expect(err).ToNot(HaveOccurred()) - Expect(csv).To(Equal(upgradeCSV(newCSV, g.OperatorName, g.Version))) - }) + Context("to update an existing ClusterServiceVersion", func() { + It("should return an updated object", func() { + g = Generator{ + OperatorName: operatorName, + OperatorType: operatorType, + Version: version, + Collector: &collector.Manifests{}, + config: cfg, + getBase: makeBaseGetter(newCSV), + } + // Update the input's and expected CSV's Deployment image. + Expect(g.Collector.UpdateFromDirs(goConfigDir, goCRDsDir)).ToNot(HaveOccurred()) + Expect(len(g.Collector.Deployments)).To(BeNumerically(">=", 1)) + imageTag := "controller:v" + g.Version + modifyDepImageHelper(&g.Collector.Deployments[0].Spec, imageTag) + updatedCSV := updateCSV(newCSV, modifyCSVDepImageHelper(imageTag)) + + csv, err := g.generate() + Expect(err).ToNot(HaveOccurred()) + Expect(csv).To(Equal(updatedCSV)) }) + }) + Context("to upgrade an existing ClusterServiceVersion", func() { + It("should return an upgraded object", func() { + g = Generator{ + OperatorName: operatorName, + OperatorType: operatorType, + Version: "0.0.2", + Collector: col, + config: cfg, + getBase: makeBaseGetter(newCSV), + // Bundles need a path, usually set by an Option, to an existing + // CSV manifest so "replaces" can be set correctly. + bundledPath: filepath.Join(csvNewLayoutBundleDir, "memcached-operator.clusterserviceversion.yaml"), + } + csv, err := g.generate() + Expect(err).ToNot(HaveOccurred()) + Expect(csv).To(Equal(upgradeCSV(newCSV, g.OperatorName, g.Version))) + }) }) }) diff --git a/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go b/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go index 3cd193b22e..73bd0a65be 100644 --- a/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go +++ b/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go @@ -328,9 +328,8 @@ func validate(csv *operatorsv1alpha1.ClusterServiceVersion) error { hasErrors = true } } - if hasErrors { - return errors.New("generated ClusterServiceVersion is invalid") + return errors.New("invalid generated ClusterServiceVersion") } return nil diff --git a/internal/generate/collector/collect.go b/internal/generate/collector/collect.go index a861a5841d..715695b69d 100644 --- a/internal/generate/collector/collect.go +++ b/internal/generate/collector/collect.go @@ -47,13 +47,13 @@ type Manifests struct { Others []unstructured.Unstructured } -// UpdateFromDirs adds Roles, ClusterRoles, Deployments, and Custom Resources -// found in manifestRoot, and CustomResourceDefinitions found in crdsDir, +// UpdateFromDirs adds Roles, ClusterRoles, Deployments, and Custom Resource examples +// found in deployDir, and CustomResourceDefinitions found in crdsDir, // to their respective fields in a Manifests, then filters and deduplicates them. // All other objects are added to Manifests.Others. -func (c *Manifests) UpdateFromDirs(manifestRoot, crdsDir string) error { +func (c *Manifests) UpdateFromDirs(deployDir, crdsDir string) error { // Collect all manifests in paths. - err := filepath.Walk(manifestRoot, func(path string, info os.FileInfo, err error) error { + err := filepath.Walk(deployDir, func(path string, info os.FileInfo, err error) error { if err != nil || info.IsDir() { return err } @@ -95,7 +95,7 @@ func (c *Manifests) UpdateFromDirs(manifestRoot, crdsDir string) error { return scanner.Err() }) if err != nil { - return fmt.Errorf("error collecting manifests from directory %s: %v", manifestRoot, err) + return fmt.Errorf("error collecting manifests from directory %s: %v", deployDir, err) } // Add CRDs from input. diff --git a/internal/generate/packagemanifest/bases/packagemanifest.go b/internal/generate/packagemanifest/bases/packagemanifest.go new file mode 100644 index 0000000000..0a598837b0 --- /dev/null +++ b/internal/generate/packagemanifest/bases/packagemanifest.go @@ -0,0 +1,67 @@ +// 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 bases + +import ( + "fmt" + "io/ioutil" + + apimanifests "github.com/operator-framework/api/pkg/manifests" + "sigs.k8s.io/yaml" +) + +// PackageManifest configures the PackageManifest that GetBase() returns. +type PackageManifest struct { + PackageName string + BasePath string +} + +// GetBase returns a base PackageManifest, populated either with default +// values or, if b.BasePath is set, bytes from disk. +func (b PackageManifest) GetBase() (base *apimanifests.PackageManifest, err error) { + if b.BasePath != "" { + if base, err = readPackageManifestBase(b.BasePath); err != nil { + return nil, fmt.Errorf("error reading existing PackageManifest base %s: %v", b.BasePath, err) + } + } else { + base = b.makeNewBase() + } + + return base, nil +} + +// makeNewBase returns a base makeNewBase to modify. +func (b PackageManifest) makeNewBase() *apimanifests.PackageManifest { + return &apimanifests.PackageManifest{ + PackageName: b.PackageName, + } +} + +// readPackageManifestBase returns the PackageManifest base at path. +// If no base is found, readPackageManifestBase returns an error. +func readPackageManifestBase(path string) (*apimanifests.PackageManifest, error) { + b, err := ioutil.ReadFile(path) + if err != nil { + return nil, err + } + pkg := &apimanifests.PackageManifest{} + if err := yaml.Unmarshal(b, pkg); err != nil { + return nil, fmt.Errorf("error unmarshalling PackageManifest from %s: %w", path, err) + } + if pkg.PackageName == "" { + return nil, fmt.Errorf("no PackageManifest in %s", path) + } + return pkg, nil +} diff --git a/internal/generate/packagemanifest/packagemanifest.go b/internal/generate/packagemanifest/packagemanifest.go new file mode 100644 index 0000000000..87c8486fa1 --- /dev/null +++ b/internal/generate/packagemanifest/packagemanifest.go @@ -0,0 +1,230 @@ +// 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 packagemanifest + +import ( + "errors" + "fmt" + "io" + "path/filepath" + "sort" + "strings" + + apimanifests "github.com/operator-framework/api/pkg/manifests" + "github.com/operator-framework/api/pkg/validation" + log "github.com/sirupsen/logrus" + + genutil "github.com/operator-framework/operator-sdk/internal/generate/internal" + "github.com/operator-framework/operator-sdk/internal/generate/packagemanifest/bases" +) + +const ( + // File extension for all PackageManifests written by Generator. + packageManifestFileExt = ".package.yaml" +) + +var ( + // User-facing errors. + errNoVersion = errors.New("version must be set") + + // Internal errors. + errNoGetBase = genutil.InternalError("getBase must be set") + errNoGetWriter = genutil.InternalError("getWriter must be set") +) + +type Generator struct { + // OperatorName is the operator's name, ex. app-operator. + OperatorName string + // Version is the version of the operator being updated. + Version string + // ChannelName is operator's PackageManifest channel. If a new PackageManifest is generated + // or ChannelName is the only channel in the generated PackageManifest, + // this channel will be set to the PackageManifest's default. + ChannelName string + // IsDefaultChannel determines whether ChannelName should be the default channel in the + // generated PackageManifest. If true, ChannelName will be the PackageManifest's default channel. + // Setting this field is only necessary when more than one channel exists. + IsDefaultChannel bool + + // Func that returns a base PackageManifest. + getBase getBaseFunc + // Func that returns the writer the generated PackageManifest's bytes are written to. + getWriter func() (io.Writer, error) +} + +// Type of Generator.getBase. +type getBaseFunc func() (*apimanifests.PackageManifest, error) + +// Option is a function that modifies a Generator. +type Option func(*Generator) error + +// WithBase sets a Generator's base PackageManifest to one that either exists or a default. +func WithBase(inputDir string) Option { + return func(g *Generator) error { + g.getBase = g.makeBaseGetter(inputDir) + return nil + } +} + +// WithWriter sets a Generator's writer to w. +func WithWriter(w io.Writer) Option { + return func(g *Generator) error { + g.getWriter = func() (io.Writer, error) { + return w, nil + } + return nil + } +} + +// WithFileWriter sets a Generator's writer to a PackageManifest file under . +func WithFileWriter(dir string) Option { + return func(g *Generator) (err error) { + g.getWriter = func() (io.Writer, error) { + return genutil.Open(dir, makePkgManFileName(g.OperatorName)) + } + return nil + } +} + +// Generate configures the Generator with opts then runs it. +func (g *Generator) Generate(opts ...Option) error { + for _, opt := range opts { + if err := opt(g); err != nil { + return err + } + } + + if g.getWriter == nil { + return errNoGetWriter + } + + pkg, err := g.generate() + if err != nil { + return err + } + + w, err := g.getWriter() + if err != nil { + return err + } + return genutil.WriteYAML(w, pkg) +} + +// generate runs a configured Generator. +func (g *Generator) generate() (*apimanifests.PackageManifest, error) { + if g.getBase == nil { + return nil, errNoGetBase + } + if g.Version == "" { + return nil, errNoVersion + } + + base, err := g.getBase() + if err != nil { + return nil, fmt.Errorf("error getting PackageManifest base: %v", err) + } + + csvName := genutil.MakeCSVName(g.OperatorName, g.Version) + if g.ChannelName != "" { + setChannels(base, g.ChannelName, csvName) + sortChannelsByName(base) + if g.IsDefaultChannel || len(base.Channels) == 1 { + base.DefaultChannelName = g.ChannelName + } + } else if len(base.Channels) == 0 { + setChannels(base, "alpha", csvName) + base.DefaultChannelName = "alpha" + } + + if err = validatePackageManifest(base); err != nil { + return nil, err + } + + return base, nil +} + +// makeBaseGetter returns a function that gets a base from inputDir. +func (g Generator) makeBaseGetter(inputDir string) func() (*apimanifests.PackageManifest, error) { + basePath := filepath.Join(inputDir, makePkgManFileName(g.OperatorName)) + if genutil.IsNotExist(basePath) { + basePath = "" + } + + return func() (*apimanifests.PackageManifest, error) { + b := bases.PackageManifest{ + PackageName: g.OperatorName, + BasePath: basePath, + } + return b.GetBase() + } +} + +// makePkgManFileName will return the file name of a PackageManifest. +func makePkgManFileName(operatorName string) string { + return strings.ToLower(operatorName) + packageManifestFileExt +} + +// sortChannelsByName sorts pkg.Channels by each element's name. +func sortChannelsByName(pkg *apimanifests.PackageManifest) { + sort.Slice(pkg.Channels, func(i int, j int) bool { + return pkg.Channels[i].Name < pkg.Channels[j].Name + }) +} + +// validatePackageManifest will validate pkg and log warnings and errors. +// If a validation error is encountered, an error is returned. +func validatePackageManifest(pkg *apimanifests.PackageManifest) error { + if pkg == nil { + return errors.New("empty PackageManifest") + } + + hasErrors := false + results := validation.PackageManifestValidator.Validate(pkg) + for _, r := range results { + for _, e := range r.Errors { + log.Errorf("PackageManifest validation: [%s] %s", e.Type, e.Detail) + } + for _, w := range r.Warnings { + log.Warnf("PackageManifest validation: [%s] %s", w.Type, w.Detail) + } + if r.HasError() { + hasErrors = true + } + } + + if hasErrors { + return errors.New("invalid generated PackageManifest") + } + + return nil +} + +// setChannels checks for duplicate channels in pkg and sets the default channel if possible. +func setChannels(pkg *apimanifests.PackageManifest, channelName, csvName string) { + channelIdx := -1 + for i, channel := range pkg.Channels { + if channel.Name == channelName { + pkg.Channels[i].CurrentCSVName = csvName + channelIdx = i + break + } + } + if channelIdx == -1 { + pkg.Channels = append(pkg.Channels, apimanifests.PackageChannel{ + Name: channelName, + CurrentCSVName: csvName, + }) + } +} diff --git a/internal/generate/packagemanifest/packagemanifest_test.go b/internal/generate/packagemanifest/packagemanifest_test.go new file mode 100644 index 0000000000..d617292c67 --- /dev/null +++ b/internal/generate/packagemanifest/packagemanifest_test.go @@ -0,0 +1,274 @@ +// 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 packagemanifest + +import ( + "bytes" + "io/ioutil" + "os" + "path/filepath" + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" + + apimanifests "github.com/operator-framework/api/pkg/manifests" + "sigs.k8s.io/yaml" + + genutil "github.com/operator-framework/operator-sdk/internal/generate/internal" +) + +func TestGenerator(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Generator Suite") +} + +const ( + pkgManDefaultContent = `channels: + - currentCSV: memcached-operator.v0.0.1 + name: alpha +defaultChannel: alpha +packageName: memcached-operator +` + + pkgManSingleChannelContent = `channels: + - currentCSV: memcached-operator.v0.0.1 + name: stable +defaultChannel: stable +packageName: memcached-operator +` +) + +var ( + testDataDir = filepath.Join("..", "testdata") + + pkgManDefault, pkgManSingleChannel *apimanifests.PackageManifest +) + +var _ = BeforeSuite(func() { + initTestPackageManifestsHelper() +}) + +var _ = Describe("Generating a PackageManifest", func() { + format.UseStringerRepresentation = true + + var ( + g Generator + buf *bytes.Buffer + operatorName = "memcached-operator" + version = "0.0.1" + ) + + BeforeEach(func() { + buf = &bytes.Buffer{} + }) + + Describe("for the new Go project layout", func() { + + Context("with correct Options", func() { + + var ( + tmp string + err error + ) + + BeforeEach(func() { + tmp, err = ioutil.TempDir(".", "") + Expect(err).ToNot(HaveOccurred()) + }) + + AfterEach(func() { + if tmp != "" { + os.RemoveAll(tmp) + } + }) + + It("should write a PackageManifest to an io.Writer", func() { + g = Generator{ + OperatorName: operatorName, + Version: version, + } + opts := []Option{ + WithBase(testDataDir), + WithWriter(buf), + } + Expect(g.Generate(opts...)).ToNot(HaveOccurred()) + Expect(buf.String()).To(MatchYAML(pkgManDefaultContent)) + }) + It("should write a PackageManifest to disk", func() { + g = Generator{ + OperatorName: operatorName, + Version: version, + } + opts := []Option{ + WithBase(testDataDir), + WithFileWriter(tmp), + } + Expect(g.Generate(opts...)).ToNot(HaveOccurred()) + outputFile := filepath.Join(tmp, makePkgManFileName(operatorName)) + Expect(outputFile).To(BeAnExistingFile()) + Expect(string(readFileHelper(outputFile))).To(MatchYAML(pkgManDefaultContent)) + }) + }) + + Context("with incorrect configuration", func() { + + BeforeEach(func() { + g = Generator{ + OperatorName: operatorName, + Version: version, + } + }) + + It("should return an error without any Options", func() { + opts := []Option{} + Expect(g.Generate(opts...)).To(MatchError(errNoGetWriter)) + }) + It("should return an error without a getWriter", func() { + opts := []Option{ + WithBase(testDataDir), + } + Expect(g.Generate(opts...)).To(MatchError(errNoGetWriter)) + }) + It("should return an error without a getBase", func() { + opts := []Option{ + WithWriter(&bytes.Buffer{}), + } + Expect(g.Generate(opts...)).To(MatchError(errNoGetBase)) + }) + It("should return an error without a Version", func() { + g.Version = "" + opts := []Option{ + WithBase(testDataDir), + WithWriter(&bytes.Buffer{}), + } + Expect(g.Generate(opts...)).To(MatchError(errNoVersion)) + }) + }) + + Context("to create a new PackageManifest", func() { + It("should return the default file", func() { + g = Generator{ + OperatorName: operatorName, + Version: version, + getBase: makeBaseGetter(pkgManDefaultContent), + } + pkg, err := g.generate() + Expect(err).ToNot(HaveOccurred()) + Expect(pkg).To(Equal(pkgManDefault)) + }) + It("should return a PackageManifest with a non-default channel", func() { + g = Generator{ + OperatorName: operatorName, + Version: version, + ChannelName: "stable", + getBase: makeBaseGetter(pkgManSingleChannelContent), + } + pkg, err := g.generate() + Expect(err).ToNot(HaveOccurred()) + Expect(pkg).To(Equal(pkgManSingleChannel)) + }) + }) + + Context("to update an existing PackageManifest file", func() { + It("should return a PackageManifest with one updated channel CSV name", func() { + g = Generator{ + OperatorName: operatorName, + Version: "0.0.2", + ChannelName: "alpha", + getBase: makeBaseGetter(pkgManDefaultContent), + } + pkg, err := g.generate() + Expect(err).ToNot(HaveOccurred()) + Expect(pkg).To(Equal(&apimanifests.PackageManifest{ + Channels: []apimanifests.PackageChannel{ + {Name: "alpha", CurrentCSVName: genutil.MakeCSVName(operatorName, "0.0.2")}, + }, + DefaultChannelName: "alpha", + PackageName: operatorName, + })) + }) + It("should return a PackageManifest with two channels", func() { + g = Generator{ + OperatorName: operatorName, + Version: "0.0.2", + ChannelName: "stable", + getBase: makeBaseGetter(pkgManDefaultContent), + } + pkg, err := g.generate() + Expect(err).ToNot(HaveOccurred()) + Expect(pkg).To(Equal(&apimanifests.PackageManifest{ + Channels: []apimanifests.PackageChannel{ + {Name: "alpha", CurrentCSVName: genutil.MakeCSVName(operatorName, version)}, + {Name: "stable", CurrentCSVName: genutil.MakeCSVName(operatorName, "0.0.2")}, + }, + DefaultChannelName: "alpha", + PackageName: operatorName, + })) + }) + It("should return a PackageManifest with two channels and an updated default channel", func() { + g = Generator{ + OperatorName: operatorName, + Version: "0.0.2", + ChannelName: "stable", + IsDefaultChannel: true, + getBase: makeBaseGetter(pkgManDefaultContent), + } + pkg, err := g.generate() + Expect(err).ToNot(HaveOccurred()) + Expect(pkg).To(Equal(&apimanifests.PackageManifest{ + Channels: []apimanifests.PackageChannel{ + {Name: "alpha", CurrentCSVName: genutil.MakeCSVName(operatorName, version)}, + {Name: "stable", CurrentCSVName: genutil.MakeCSVName(operatorName, "0.0.2")}, + }, + DefaultChannelName: "stable", + PackageName: operatorName, + })) + }) + }) + + }) + +}) + +func makeBaseGetter(content string) getBaseFunc { + return func() (*apimanifests.PackageManifest, error) { + return marshalContent(content) + } +} + +func initTestPackageManifestsHelper() { + var err error + pkgManDefault, err = marshalContent(pkgManDefaultContent) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + pkgManSingleChannel, err = marshalContent(pkgManSingleChannelContent) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) +} + +func readFileHelper(path string) []byte { + b, err := ioutil.ReadFile(path) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + return b +} + +func marshalContent(content string) (*apimanifests.PackageManifest, error) { + base := &apimanifests.PackageManifest{} + if content == "" { + return base, nil + } + err := yaml.Unmarshal([]byte(content), base) + return base, err +} diff --git a/internal/generate/testdata/memcached-operator.package.yaml b/internal/generate/testdata/memcached-operator.package.yaml new file mode 100644 index 0000000000..646eb1b4a9 --- /dev/null +++ b/internal/generate/testdata/memcached-operator.package.yaml @@ -0,0 +1,5 @@ +channels: +- currentCSV: memcached-operator.v0.0.1 + name: alpha +defaultChannel: alpha +packageName: memcached-operator diff --git a/test/e2e-new/e2e_suite.go b/test/e2e-new/e2e_suite.go index 23280722b7..4daa8550a2 100644 --- a/test/e2e-new/e2e_suite.go +++ b/test/e2e-new/e2e_suite.go @@ -17,9 +17,11 @@ package e2e import ( + "bytes" "fmt" "io/ioutil" "os" + "os/exec" "path" "path/filepath" "strings" @@ -33,7 +35,11 @@ import ( var _ = Describe("operator-sdk", func() { Context("with the new project layout", func() { - var tc *utils.TestContext + var ( + tc *utils.TestContext + projectName string + ) + BeforeEach(func() { By("creating a new test context") @@ -41,6 +47,8 @@ var _ = Describe("operator-sdk", func() { tc, err = utils.NewTestContext("operator-sdk", "GO111MODULE=on") Expect(err).NotTo(HaveOccurred()) Expect(tc.Prepare()).To(Succeed()) + + projectName = filepath.Base(tc.Dir) }) AfterEach(func() { @@ -56,6 +64,7 @@ var _ = Describe("operator-sdk", func() { By("initializing a project") err := tc.Init( "--project-version", "3-alpha", + "--repo", path.Join("github.com", "example", projectName), "--domain", tc.Domain, "--fetch-deps=false") Expect(err).Should(Succeed()) @@ -154,6 +163,20 @@ var _ = Describe("operator-sdk", func() { } err = tc.Make("bundle-build", "BUNDLE_IMG="+bundleImage) Expect(err).Should(Succeed()) + + By("generating the operator package") + bundlePath := filepath.Join("config", "bundle") + kustomizeOutput, err := tc.Run(exec.Command("kustomize", "build", bundlePath)) + Expect(err).Should(Succeed()) + genPkgManCmd := exec.Command(tc.BinaryName, "generate", "packagemanifests", + "--manifests", + "--update-crds", + "--input-dir", bundlePath, + "--version", "0.0.1") + Expect(err).Should(Succeed()) + tc.Stdin = bytes.NewBuffer(kustomizeOutput) + _, err = tc.Run(genPkgManCmd) + Expect(err).Should(Succeed()) }) }) }) From 284902a2bf01ec8bc7fa54075a3494abae5e706b Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 11 Jun 2020 14:11:14 -0700 Subject: [PATCH 2/6] write config/packagemanifests/kustomization.yaml on 'generate packagemanifests --kustomize' --- .../packagemanifests/packagemanifests.go | 17 +++++++++++++++-- test/e2e-new/e2e_suite.go | 15 ++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go index 151d9a3401..bfee333358 100644 --- a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go +++ b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go @@ -26,8 +26,18 @@ import ( gencsv "github.com/operator-framework/operator-sdk/internal/generate/clusterserviceversion" "github.com/operator-framework/operator-sdk/internal/generate/collector" genpkg "github.com/operator-framework/operator-sdk/internal/generate/packagemanifest" + "github.com/operator-framework/operator-sdk/internal/scaffold/kustomize" ) +// kustomization.yaml file contents for manifests. This should always be written to +// config/packagemanifests/kustomization.yaml since it only references files in config. +const manifestsKustomization = `resources: +- ../default +- ../samples +` + +var defaultDir = filepath.Join("config", "packagemanifests") + // setCommonDefaults sets defaults useful to all modes of this subcommand. func (c *packagemanifestsCmd) setCommonDefaults(cfg *config.Config) { if c.operatorName == "" { @@ -42,7 +52,6 @@ func (c packagemanifestsCmd) runKustomize(cfg *config.Config) error { fmt.Println("Generating package manifests kustomize bases") } - defaultDir := filepath.Join("config", "packages") if c.inputDir == "" { c.inputDir = defaultDir } @@ -69,6 +78,11 @@ func (c packagemanifestsCmd) runKustomize(cfg *config.Config) error { return fmt.Errorf("error generating ClusterServiceVersion: %v", err) } + // Write a kustomization.yaml to the config directory. + if err := kustomize.WriteIfNotExist(defaultDir, manifestsKustomization); err != nil { + return err + } + if !c.quiet { fmt.Println("Bases generated successfully in", c.outputDir) } @@ -116,7 +130,6 @@ func (c packagemanifestsCmd) runManifests(cfg *config.Config) error { fmt.Println("Generating package manifests version", c.version) } - defaultDir := filepath.Join("config", "packages") if c.inputDir == "" { c.inputDir = defaultDir } diff --git a/test/e2e-new/e2e_suite.go b/test/e2e-new/e2e_suite.go index 4daa8550a2..1ea95d63d7 100644 --- a/test/e2e-new/e2e_suite.go +++ b/test/e2e-new/e2e_suite.go @@ -164,14 +164,19 @@ var _ = Describe("operator-sdk", func() { err = tc.Make("bundle-build", "BUNDLE_IMG="+bundleImage) Expect(err).Should(Succeed()) - By("generating the operator package") - bundlePath := filepath.Join("config", "bundle") - kustomizeOutput, err := tc.Run(exec.Command("kustomize", "build", bundlePath)) + By("generating the operator package manifests") + var genPkgManCmd *exec.Cmd + Expect(tc.Make("manifests")).Should(Succeed()) + genPkgManCmd = exec.Command(tc.BinaryName, "generate", "packagemanifests", + "--kustomize", + "--interactive=false") + _, err = tc.Run(genPkgManCmd) + Expect(err).Should(Succeed()) + kustomizeOutput, err := tc.Run(exec.Command("kustomize", "build", filepath.Join("config", "packagemanifests"))) Expect(err).Should(Succeed()) - genPkgManCmd := exec.Command(tc.BinaryName, "generate", "packagemanifests", + genPkgManCmd = exec.Command(tc.BinaryName, "generate", "packagemanifests", "--manifests", "--update-crds", - "--input-dir", bundlePath, "--version", "0.0.1") Expect(err).Should(Succeed()) tc.Stdin = bytes.NewBuffer(kustomizeOutput) From 018bbc65db950945a968bb79eca15ede65af78ac Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 11 Jun 2020 14:41:10 -0700 Subject: [PATCH 3/6] check if --version is set --- .../generate/packagemanifests/packagemanifests.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go index bfee333358..c6dfdcdd07 100644 --- a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go +++ b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go @@ -93,12 +93,16 @@ func (c packagemanifestsCmd) runKustomize(cfg *config.Config) error { // validateManifests validates c for package manifests generation. func (c packagemanifestsCmd) validateManifests() error { - if err := genutil.ValidateVersion(c.version); err != nil { - return err + if c.version != "" { + if err := genutil.ValidateVersion(c.version); err != nil { + return err + } + } else { + return errors.New("--version must be set") } if c.fromVersion != "" { - return errors.New("--from-version cannot be set for PROJECT configured projects") + return errors.New("--from-version cannot be set for PROJECT-configured projects") } if !genutil.IsPipeReader() { From f3242b2477347ff67869d161b5b16022f582c7cc Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 11 Jun 2020 19:16:09 -0700 Subject: [PATCH 4/6] write CRDs when generating a new package --- cmd/operator-sdk/generate/packagemanifests/cmd.go | 10 +++++++--- .../generate/packagemanifests/packagemanifests.go | 7 +++++-- test/e2e-new/e2e_suite.go | 1 - 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/cmd/operator-sdk/generate/packagemanifests/cmd.go b/cmd/operator-sdk/generate/packagemanifests/cmd.go index 118045ce0e..f81a14d214 100644 --- a/cmd/operator-sdk/generate/packagemanifests/cmd.go +++ b/cmd/operator-sdk/generate/packagemanifests/cmd.go @@ -40,10 +40,13 @@ type packagemanifestsCmd struct { deployDir string apisDir string crdsDir string - updateCRDs bool stdout bool quiet bool + // CRD update options + updateCRDs bool + updateCRDsFlag *pflag.Flag + // Interactive options. interactiveLevel projutil.InteractiveLevel interactive bool @@ -130,8 +133,9 @@ func (c *packagemanifestsCmd) addCommonFlagsTo(fs *pflag.FlagSet) { 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", false, "Update CustomResoureDefinition manifests "+ - "in this package") + fs.BoolVar(&c.updateCRDs, "update-crds", false, "Update CustomResoureDefinition manifests in this package. "+ + "This will occur automatically when a new package is generated") + c.updateCRDsFlag = fs.Lookup("update-crds") 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") diff --git a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go index c6dfdcdd07..802387e4ab 100644 --- a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go +++ b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go @@ -128,7 +128,7 @@ func (c packagemanifestsCmd) validateManifests() error { } // runManifests generates package manifests. -func (c packagemanifestsCmd) runManifests(cfg *config.Config) error { +func (c packagemanifestsCmd) runManifests(cfg *config.Config) (err error) { if !c.quiet && !c.stdout { fmt.Println("Generating package manifests version", c.version) @@ -150,6 +150,9 @@ func (c packagemanifestsCmd) runManifests(cfg *config.Config) error { c.apisDir = "api" } } + // When generating a new package, CRDs should be written unless --update-crds has been explicitly set to false. + updateCRDsOff := c.updateCRDsFlag.Changed && !c.updateCRDs + writeNewPackageCRDs := !updateCRDsOff && genutil.IsNotExist(filepath.Join(c.outputDir, c.version)) if err := c.generatePackageManifest(); err != nil { return err @@ -188,7 +191,7 @@ func (c packagemanifestsCmd) runManifests(cfg *config.Config) error { return fmt.Errorf("error generating ClusterServiceVersion: %v", err) } - if c.updateCRDs { + if c.updateCRDs || writeNewPackageCRDs { var objs []interface{} for _, crd := range col.V1CustomResourceDefinitions { objs = append(objs, crd) diff --git a/test/e2e-new/e2e_suite.go b/test/e2e-new/e2e_suite.go index 1ea95d63d7..b744a69650 100644 --- a/test/e2e-new/e2e_suite.go +++ b/test/e2e-new/e2e_suite.go @@ -176,7 +176,6 @@ var _ = Describe("operator-sdk", func() { Expect(err).Should(Succeed()) genPkgManCmd = exec.Command(tc.BinaryName, "generate", "packagemanifests", "--manifests", - "--update-crds", "--version", "0.0.1") Expect(err).Should(Succeed()) tc.Stdin = bytes.NewBuffer(kustomizeOutput) From dfd161e1be44463a1564328851bbe38d9d4e54f3 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 11 Jun 2020 19:41:10 -0700 Subject: [PATCH 5/6] add help text and examples --- .../generate/packagemanifests/cmd.go | 23 +++++++++++-- .../packagemanifests/packagemanifests.go | 33 +++++++++++++++++-- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/cmd/operator-sdk/generate/packagemanifests/cmd.go b/cmd/operator-sdk/generate/packagemanifests/cmd.go index f81a14d214..5134cfff3a 100644 --- a/cmd/operator-sdk/generate/packagemanifests/cmd.go +++ b/cmd/operator-sdk/generate/packagemanifests/cmd.go @@ -25,6 +25,23 @@ import ( "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. @@ -61,8 +78,10 @@ func NewCmd() *cobra.Command { c := &packagemanifestsCmd{} cmd := &cobra.Command{ - Use: "packagemanifests", - Short: "Generates a package manifests format", + Use: "packagemanifests", + Short: "Generates a package manifests format", + Long: longHelp, + Example: examples, RunE: func(cmd *cobra.Command, args []string) error { if len(args) != 0 { return fmt.Errorf("command %s doesn't accept any arguments", cmd.CommandPath()) diff --git a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go index 802387e4ab..9839b3b0c7 100644 --- a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go +++ b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go @@ -29,6 +29,33 @@ import ( "github.com/operator-framework/operator-sdk/internal/scaffold/kustomize" ) +//nolint:lll +const examples = ` + # Generate manifests then create the package manifests base: + $ make manifests + /home/user/go/bin/controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases + $ operator-sdk generate packagemanifests -q --kustomize + + Display name for the operator (required): + > memcached-operator + ... + + $ kustomize build config/packagemanifests | operator-sdk generate packagemanifests --manifests --metadata --overwrite --version 0.0.1 + Generating package manifests version 0.0.1 + ... + + # After running the above commands, you should see: + $ tree config/packages + config/packages + ├── bases + │   └── memcached-operator.clusterserviceversion.yaml + ├── kustomization.yaml + ├── 0.0.1 + │   ├── cache.my.domain_memcacheds.yaml + │   └── memcached-operator.clusterserviceversion.yaml + └── memcached-operator.package.yaml +` + // kustomization.yaml file contents for manifests. This should always be written to // config/packagemanifests/kustomization.yaml since it only references files in config. const manifestsKustomization = `resources: @@ -150,9 +177,10 @@ func (c packagemanifestsCmd) runManifests(cfg *config.Config) (err error) { c.apisDir = "api" } } + packageDir := filepath.Join(c.outputDir, c.version) // When generating a new package, CRDs should be written unless --update-crds has been explicitly set to false. updateCRDsOff := c.updateCRDsFlag.Changed && !c.updateCRDs - writeNewPackageCRDs := !updateCRDsOff && genutil.IsNotExist(filepath.Join(c.outputDir, c.version)) + writeNewPackageCRDs := !updateCRDsOff && genutil.IsNotExist(packageDir) if err := c.generatePackageManifest(); err != nil { return err @@ -204,8 +232,7 @@ func (c packagemanifestsCmd) runManifests(cfg *config.Config) (err error) { return err } } else { - dir := filepath.Join(c.outputDir, c.version) - if err := genutil.WriteObjectsToFiles(dir, objs...); err != nil { + if err := genutil.WriteObjectsToFiles(packageDir, objs...); err != nil { return err } } From 598931b27acdf439d9514d7e01c798d46e078000 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Tue, 16 Jun 2020 09:08:06 -0700 Subject: [PATCH 6/6] --update-crds=true by default --- cmd/operator-sdk/generate/packagemanifests/cmd.go | 9 ++------- .../generate/packagemanifests/packagemanifests.go | 9 +++------ .../clusterserviceversion/clusterserviceversion.go | 2 +- test/e2e-new/e2e_suite.go | 1 + 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/cmd/operator-sdk/generate/packagemanifests/cmd.go b/cmd/operator-sdk/generate/packagemanifests/cmd.go index 5134cfff3a..863f13865d 100644 --- a/cmd/operator-sdk/generate/packagemanifests/cmd.go +++ b/cmd/operator-sdk/generate/packagemanifests/cmd.go @@ -57,13 +57,10 @@ type packagemanifestsCmd struct { deployDir string apisDir string crdsDir string + updateCRDs bool stdout bool quiet bool - // CRD update options - updateCRDs bool - updateCRDsFlag *pflag.Flag - // Interactive options. interactiveLevel projutil.InteractiveLevel interactive bool @@ -152,9 +149,7 @@ func (c *packagemanifestsCmd) addCommonFlagsTo(fs *pflag.FlagSet) { 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", false, "Update CustomResoureDefinition manifests in this package. "+ - "This will occur automatically when a new package is generated") - c.updateCRDsFlag = fs.Lookup("update-crds") + 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") diff --git a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go index 9839b3b0c7..ef68400d0a 100644 --- a/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go +++ b/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go @@ -40,7 +40,7 @@ const examples = ` > memcached-operator ... - $ kustomize build config/packagemanifests | operator-sdk generate packagemanifests --manifests --metadata --overwrite --version 0.0.1 + $ kustomize build config/packagemanifests | operator-sdk generate packagemanifests --manifests --version 0.0.1 Generating package manifests version 0.0.1 ... @@ -155,7 +155,7 @@ func (c packagemanifestsCmd) validateManifests() error { } // runManifests generates package manifests. -func (c packagemanifestsCmd) runManifests(cfg *config.Config) (err error) { +func (c packagemanifestsCmd) runManifests(cfg *config.Config) error { if !c.quiet && !c.stdout { fmt.Println("Generating package manifests version", c.version) @@ -178,9 +178,6 @@ func (c packagemanifestsCmd) runManifests(cfg *config.Config) (err error) { } } packageDir := filepath.Join(c.outputDir, c.version) - // When generating a new package, CRDs should be written unless --update-crds has been explicitly set to false. - updateCRDsOff := c.updateCRDsFlag.Changed && !c.updateCRDs - writeNewPackageCRDs := !updateCRDsOff && genutil.IsNotExist(packageDir) if err := c.generatePackageManifest(); err != nil { return err @@ -219,7 +216,7 @@ func (c packagemanifestsCmd) runManifests(cfg *config.Config) (err error) { return fmt.Errorf("error generating ClusterServiceVersion: %v", err) } - if c.updateCRDs || writeNewPackageCRDs { + if c.updateCRDs { var objs []interface{} for _, crd := range col.V1CustomResourceDefinitions { objs = append(objs, crd) diff --git a/internal/generate/clusterserviceversion/clusterserviceversion.go b/internal/generate/clusterserviceversion/clusterserviceversion.go index be5bb5c445..45292e6af6 100644 --- a/internal/generate/clusterserviceversion/clusterserviceversion.go +++ b/internal/generate/clusterserviceversion/clusterserviceversion.go @@ -54,7 +54,7 @@ type Generator struct { OperatorType projutil.OperatorType // Version is the CSV current version. Version string - // FromVersion is the version of a previous CSV to upgrade from from. + // FromVersion is the version of a previous CSV to upgrade from. FromVersion string // Collector holds all manifests relevant to the Generator. Collector *collector.Manifests diff --git a/test/e2e-new/e2e_suite.go b/test/e2e-new/e2e_suite.go index b744a69650..1ea95d63d7 100644 --- a/test/e2e-new/e2e_suite.go +++ b/test/e2e-new/e2e_suite.go @@ -176,6 +176,7 @@ var _ = Describe("operator-sdk", func() { Expect(err).Should(Succeed()) genPkgManCmd = exec.Command(tc.BinaryName, "generate", "packagemanifests", "--manifests", + "--update-crds", "--version", "0.0.1") Expect(err).Should(Succeed()) tc.Stdin = bytes.NewBuffer(kustomizeOutput)