From 8b7e731f9889cdb75d436c08e0ee67691e461a32 Mon Sep 17 00:00:00 2001 From: Adrian Orive Date: Thu, 4 Feb 2021 12:50:55 +0100 Subject: [PATCH] Store abstraction for persisting Config - Use the cli filesystem to persist the project config - Use PreRunE and PostRunE functions to handle configuration file loading and saving Signed-off-by: Adrian Orive --- pkg/cli/api.go | 17 +- pkg/cli/cli.go | 14 +- pkg/cli/cli_test.go | 2 + pkg/cli/cmd_helpers.go | 42 +++- pkg/cli/edit.go | 17 +- pkg/cli/init.go | 36 ++-- pkg/cli/internal/config/config.go | 196 ------------------ pkg/cli/internal/config/config_test.go | 71 ------- pkg/cli/webhook.go | 17 +- pkg/config/store/errors.go | 51 +++++ pkg/config/store/interface.go | 38 ++++ pkg/config/store/yaml/store.go | 147 +++++++++++++ pkg/config/store/yaml/store_test.go | 121 +++++++++++ .../store/yaml/suite_test.go} | 8 +- pkg/config/v3/config.go | 7 +- pkg/config/v3/config_test.go | 6 +- pkg/plugin/interfaces.go | 5 +- pkg/plugins/golang/v2/api.go | 3 +- pkg/plugins/golang/v2/init.go | 4 +- pkg/plugins/golang/v2/webhook.go | 9 +- pkg/plugins/golang/v3/api.go | 4 +- pkg/plugins/golang/v3/webhook.go | 4 +- 22 files changed, 465 insertions(+), 354 deletions(-) delete mode 100644 pkg/cli/internal/config/config.go delete mode 100644 pkg/cli/internal/config/config_test.go create mode 100644 pkg/config/store/errors.go create mode 100644 pkg/config/store/interface.go create mode 100644 pkg/config/store/yaml/store.go create mode 100644 pkg/config/store/yaml/store_test.go rename pkg/{cli/internal/config/config_suite_test.go => config/store/yaml/suite_test.go} (82%) diff --git a/pkg/cli/api.go b/pkg/cli/api.go index 7907b06edc5..c8d0fa29e08 100644 --- a/pkg/cli/api.go +++ b/pkg/cli/api.go @@ -21,7 +21,7 @@ import ( "github.com/spf13/cobra" - "sigs.k8s.io/kubebuilder/v3/pkg/cli/internal/config" + yamlstore "sigs.k8s.io/kubebuilder/v3/pkg/config/store/yaml" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" ) @@ -76,18 +76,15 @@ func (c cli) bindCreateAPI(ctx plugin.Context, cmd *cobra.Command) { return } - cfg, err := config.LoadInitialized() - if err != nil { - cmdErr(cmd, err) - return - } - subcommand := createAPIPlugin.GetCreateAPISubcommand() - subcommand.InjectConfig(cfg.Config) subcommand.BindFlags(cmd.Flags()) subcommand.UpdateContext(&ctx) cmd.Long = ctx.Description cmd.Example = ctx.Examples - cmd.RunE = runECmdFunc(c.fs, cfg, subcommand, - fmt.Sprintf("failed to create API with %q", plugin.KeyFor(createAPIPlugin))) + + cfg := yamlstore.New(c.fs) + msg := fmt.Sprintf("failed to create API with %q", plugin.KeyFor(createAPIPlugin)) + cmd.PreRunE = preRunECmdFunc(subcommand, cfg, msg) + cmd.RunE = runECmdFunc(c.fs, subcommand, msg) + cmd.PostRunE = postRunECmdFunc(cfg, msg) } diff --git a/pkg/cli/cli.go b/pkg/cli/cli.go index 22329b3794b..ecd2ef32f45 100644 --- a/pkg/cli/cli.go +++ b/pkg/cli/cli.go @@ -17,6 +17,7 @@ limitations under the License. package cli import ( + "errors" "fmt" "os" "strings" @@ -25,8 +26,8 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" - internalconfig "sigs.k8s.io/kubebuilder/v3/pkg/cli/internal/config" "sigs.k8s.io/kubebuilder/v3/pkg/config" + yamlstore "sigs.k8s.io/kubebuilder/v3/pkg/config/store/yaml" cfgv3 "sigs.k8s.io/kubebuilder/v3/pkg/config/v3" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" ) @@ -196,18 +197,19 @@ func (c *cli) getInfoFromFlags() (string, []string, error) { } // getInfoFromConfigFile obtains the project version and plugin keys from the project config file. -func getInfoFromConfigFile() (config.Version, []string, error) { +func (c cli) getInfoFromConfigFile() (config.Version, []string, error) { // Read the project configuration file - projectConfig, err := internalconfig.Read() + cfg := yamlstore.New(c.fs) + err := cfg.Load() switch { case err == nil: - case os.IsNotExist(err): + case errors.Is(err, os.ErrNotExist): return config.Version{}, nil, nil default: return config.Version{}, nil, err } - return getInfoFromConfig(projectConfig) + return getInfoFromConfig(cfg.Config()) } // getInfoFromConfig obtains the project version and plugin keys from the project config. @@ -302,7 +304,7 @@ func (c *cli) getInfo() error { return err } // Get project version and plugin info from project configuration file - cfgProjectVersion, cfgPlugins, _ := getInfoFromConfigFile() + cfgProjectVersion, cfgPlugins, _ := c.getInfoFromConfigFile() // We discard the error because not being able to read a project configuration file // is not fatal for some commands. The ones that require it need to check its existence. diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go index d9b7000e439..a7fe02cfd02 100644 --- a/pkg/cli/cli_test.go +++ b/pkg/cli/cli_test.go @@ -23,6 +23,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/spf13/afero" "github.com/spf13/cobra" "sigs.k8s.io/kubebuilder/v3/pkg/config" @@ -585,6 +586,7 @@ var _ = Describe("CLI", func() { defaultPlugins: map[config.Version][]string{ projectVersion: pluginKeys, }, + fs: afero.NewMemMapFs(), } c.cmd = c.newRootCmd() Expect(c.getInfo()).To(Succeed()) diff --git a/pkg/cli/cmd_helpers.go b/pkg/cli/cmd_helpers.go index 26a13440f1e..15ee41751aa 100644 --- a/pkg/cli/cmd_helpers.go +++ b/pkg/cli/cmd_helpers.go @@ -18,11 +18,12 @@ package cli import ( "fmt" + "os" "github.com/spf13/afero" "github.com/spf13/cobra" - "sigs.k8s.io/kubebuilder/v3/pkg/cli/internal/config" + "sigs.k8s.io/kubebuilder/v3/pkg/config/store" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" ) @@ -46,18 +47,39 @@ func errCmdFunc(err error) func(*cobra.Command, []string) error { } } -// runECmdFunc returns a cobra RunE function that runs subcommand and saves the -// config, which may have been modified by subcommand. -func runECmdFunc( - fs afero.Fs, - c *config.Config, - subcommand plugin.Subcommand, - msg string, -) func(*cobra.Command, []string) error { +// preRunECmdFunc returns a cobra PreRunE function that loads the configuration file +// and injects it into the subcommand +func preRunECmdFunc(subcmd plugin.Subcommand, cfg store.Store, msg string) func(*cobra.Command, []string) error { + return func(*cobra.Command, []string) error { + err := cfg.Load() + if os.IsNotExist(err) { + return fmt.Errorf("%s: unable to find configuration file, project must be initialized", msg) + } else if err != nil { + return fmt.Errorf("%s: unable to load configuration file: %w", msg, err) + } + + subcmd.InjectConfig(cfg.Config()) + return nil + } +} + +// runECmdFunc returns a cobra RunE function that runs subcommand +func runECmdFunc(fs afero.Fs, subcommand plugin.Subcommand, msg string) func(*cobra.Command, []string) error { return func(*cobra.Command, []string) error { if err := subcommand.Run(fs); err != nil { return fmt.Errorf("%s: %v", msg, err) } - return c.Save() + return nil + } +} + +// postRunECmdFunc returns a cobra PostRunE function that saves the configuration file +func postRunECmdFunc(cfg store.Store, msg string) func(*cobra.Command, []string) error { + return func(*cobra.Command, []string) error { + err := cfg.Save() + if err != nil { + return fmt.Errorf("%s: unable to save configuration file: %w", msg, err) + } + return nil } } diff --git a/pkg/cli/edit.go b/pkg/cli/edit.go index f22e0fcea99..202ca8a7022 100644 --- a/pkg/cli/edit.go +++ b/pkg/cli/edit.go @@ -21,7 +21,7 @@ import ( "github.com/spf13/cobra" - "sigs.k8s.io/kubebuilder/v3/pkg/cli/internal/config" + yamlstore "sigs.k8s.io/kubebuilder/v3/pkg/config/store/yaml" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" ) @@ -76,18 +76,15 @@ func (c cli) bindEdit(ctx plugin.Context, cmd *cobra.Command) { return } - cfg, err := config.LoadInitialized() - if err != nil { - cmdErr(cmd, err) - return - } - subcommand := editPlugin.GetEditSubcommand() - subcommand.InjectConfig(cfg.Config) subcommand.BindFlags(cmd.Flags()) subcommand.UpdateContext(&ctx) cmd.Long = ctx.Description cmd.Example = ctx.Examples - cmd.RunE = runECmdFunc(c.fs, cfg, subcommand, - fmt.Sprintf("failed to edit project with %q", plugin.KeyFor(editPlugin))) + + cfg := yamlstore.New(c.fs) + msg := fmt.Sprintf("failed to edit project with %q", plugin.KeyFor(editPlugin)) + cmd.PreRunE = preRunECmdFunc(subcommand, cfg, msg) + cmd.RunE = runECmdFunc(c.fs, subcommand, msg) + cmd.PostRunE = postRunECmdFunc(cfg, msg) } diff --git a/pkg/cli/init.go b/pkg/cli/init.go index 22f57a57606..4f4463b519d 100644 --- a/pkg/cli/init.go +++ b/pkg/cli/init.go @@ -17,8 +17,8 @@ limitations under the License. package cli import ( + "errors" "fmt" - "log" "os" "sort" "strconv" @@ -26,8 +26,8 @@ import ( "github.com/spf13/cobra" - internalconfig "sigs.k8s.io/kubebuilder/v3/pkg/cli/internal/config" "sigs.k8s.io/kubebuilder/v3/pkg/config" + yamlstore "sigs.k8s.io/kubebuilder/v3/pkg/config/store/yaml" cfgv2 "sigs.k8s.io/kubebuilder/v3/pkg/config/v2" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" ) @@ -136,28 +136,28 @@ func (c cli) bindInit(ctx plugin.Context, cmd *cobra.Command) { return } - cfg, err := internalconfig.New(c.projectVersion, internalconfig.DefaultPath) - if err != nil { - cmdErr(cmd, fmt.Errorf("unable to initialize the project configuration: %w", err)) - return - } - subcommand := initPlugin.GetInitSubcommand() - subcommand.InjectConfig(cfg.Config) subcommand.BindFlags(cmd.Flags()) subcommand.UpdateContext(&ctx) cmd.Long = ctx.Description cmd.Example = ctx.Examples - cmd.RunE = func(*cobra.Command, []string) error { - // Check if a config is initialized in the command runner so the check - // doesn't erroneously fail other commands used in initialized projects. - _, err := internalconfig.Read() - if err == nil || os.IsExist(err) { - log.Fatal("config already initialized") + + cfg := yamlstore.New(c.fs) + msg := fmt.Sprintf("failed to initialize project with %q", plugin.KeyFor(initPlugin)) + cmd.PreRunE = func(*cobra.Command, []string) error { + // Check if a config is initialized. + if err := cfg.Load(); err == nil || !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("%s: already initialized", msg) } - if err := subcommand.Run(c.fs); err != nil { - return fmt.Errorf("failed to initialize project with %q: %v", plugin.KeyFor(initPlugin), err) + + err := cfg.New(c.projectVersion) + if err != nil { + return fmt.Errorf("%s: error initializing project configuration: %w", msg, err) } - return cfg.Save() + + subcommand.InjectConfig(cfg.Config()) + return nil } + cmd.RunE = runECmdFunc(c.fs, subcommand, msg) + cmd.PostRunE = postRunECmdFunc(cfg, msg) } diff --git a/pkg/cli/internal/config/config.go b/pkg/cli/internal/config/config.go deleted file mode 100644 index 75adcec2269..00000000000 --- a/pkg/cli/internal/config/config.go +++ /dev/null @@ -1,196 +0,0 @@ -/* -Copyright 2020 The Kubernetes 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 config - -import ( - "errors" - "fmt" - "os" - - "github.com/spf13/afero" - "sigs.k8s.io/yaml" - - "sigs.k8s.io/kubebuilder/v3/pkg/config" -) - -const ( - // DefaultPath is the default path for the configuration file - DefaultPath = "PROJECT" -) - -// TODO: use cli.fs instead of creating a new afero.Fs for each config. For this purpose, we may want to turn this -// package's functions into methods of cli. - -func exists(fs afero.Fs, path string) (bool, error) { - // Look up the file - _, err := fs.Stat(path) - - // If we could find it the file exists - if err == nil || os.IsExist(err) { - return true, nil - } - - // Not existing and different errors are differentiated - if os.IsNotExist(err) { - err = nil - } - return false, err -} - -type versionedConfig struct { - Version config.Version -} - -func readFrom(fs afero.Fs, path string) (config.Config, error) { - // Read the file - in, err := afero.ReadFile(fs, path) //nolint:gosec - if err != nil { - return nil, err - } - - // Check the file version - var versioned versionedConfig - if err := yaml.Unmarshal(in, &versioned); err != nil { - return nil, err - } - - // Create the config object - var c config.Config - c, err = config.New(versioned.Version) - if err != nil { - return nil, err - } - - // Unmarshal the file content - if err := c.Unmarshal(in); err != nil { - return nil, err - } - - return c, nil -} - -// Read obtains the configuration from the default path but doesn't allow to persist changes -func Read() (config.Config, error) { - return ReadFrom(DefaultPath) -} - -// ReadFrom obtains the configuration from the provided path but doesn't allow to persist changes -func ReadFrom(path string) (config.Config, error) { - return readFrom(afero.NewOsFs(), path) -} - -// Config extends model/config.Config allowing to persist changes -// NOTE: the existence of Config structs in both model and internal packages is to guarantee that kubebuilder -// is the only project that can modify the file, while plugins can still receive the configuration -type Config struct { - config.Config - - // path stores where the config should be saved to - path string - // mustNotExist requires the file not to exist when saving it - mustNotExist bool - // fs is for testing. - fs afero.Fs -} - -// New creates a new configuration that will be stored at the provided path -func New(version config.Version, path string) (*Config, error) { - cfg, err := config.New(version) - if err != nil { - return nil, err - } - - return &Config{ - Config: cfg, - path: path, - mustNotExist: true, - fs: afero.NewOsFs(), - }, nil -} - -// Load obtains the configuration from the default path allowing to persist changes (Save method) -func Load() (*Config, error) { - return LoadFrom(DefaultPath) -} - -// LoadInitialized calls Load() but returns helpful error messages if the config -// does not exist. -func LoadInitialized() (*Config, error) { - c, err := Load() - if os.IsNotExist(err) { - return nil, errors.New("unable to find configuration file, project must be initialized") - } - return c, err -} - -// LoadFrom obtains the configuration from the provided path allowing to persist changes (Save method) -func LoadFrom(path string) (*Config, error) { - fs := afero.NewOsFs() - c, err := readFrom(fs, path) - return &Config{Config: c, path: path, fs: fs}, err -} - -// Save saves the configuration information -func (c Config) Save() error { - if c.fs == nil { - c.fs = afero.NewOsFs() - } - // If path is unset, it was created directly with `Config{}` - if c.path == "" { - return saveError{errors.New("no information where it should be stored, " + - "use one of the constructors (`New`, `Load` or `LoadFrom`) to create Config instances")} - } - - // If it is a new configuration, the path should not exist yet - if c.mustNotExist { - // Lets check that the file doesn't exist - alreadyExists, err := exists(c.fs, c.path) - if err != nil { - return saveError{err} - } - if alreadyExists { - return saveError{errors.New("configuration already exists in the provided path")} - } - } - - // Marshall into YAML - content, err := c.Marshal() - if err != nil { - return saveError{err} - } - - // Write the marshalled configuration - err = afero.WriteFile(c.fs, c.path, content, 0600) - if err != nil { - return saveError{fmt.Errorf("failed to save configuration to %s: %v", c.path, err)} - } - - return nil -} - -// Path returns the path for configuration file -func (c Config) Path() string { - return c.path -} - -type saveError struct { - err error -} - -func (e saveError) Error() string { - return fmt.Sprintf("unable to save the configuration: %v", e.err) -} diff --git a/pkg/cli/internal/config/config_test.go b/pkg/cli/internal/config/config_test.go deleted file mode 100644 index fe7d7ee4932..00000000000 --- a/pkg/cli/internal/config/config_test.go +++ /dev/null @@ -1,71 +0,0 @@ -/* -Copyright 2020 The Kubernetes 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 config - -import ( - "os" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - "github.com/spf13/afero" - - cfgv2 "sigs.k8s.io/kubebuilder/v3/pkg/config/v2" -) - -var _ = Describe("Config", func() { - Context("Save", func() { - It("should success for valid configs", func() { - cfg := Config{ - Config: cfgv2.New(), - fs: afero.NewMemMapFs(), - path: DefaultPath, - } - Expect(cfg.Save()).To(Succeed()) - - cfgBytes, err := afero.ReadFile(cfg.fs, DefaultPath) - Expect(err).NotTo(HaveOccurred()) - Expect(string(cfgBytes)).To(Equal(`version: "2" -`)) - }) - - It("should fail if path is not provided", func() { - cfg := Config{ - Config: cfgv2.New(), - fs: afero.NewMemMapFs(), - } - Expect(cfg.Save()).NotTo(Succeed()) - }) - }) - - Context("readFrom", func() { - It("should success for valid configs", func() { - configStr := `domain: example.com -repo: github.com/example/project -version: "2"` - expectedConfig := cfgv2.New() - _ = expectedConfig.SetDomain("example.com") - _ = expectedConfig.SetRepository("github.com/example/project") - - fs := afero.NewMemMapFs() - Expect(afero.WriteFile(fs, DefaultPath, []byte(configStr), os.ModePerm)).To(Succeed()) - - cfg, err := readFrom(fs, DefaultPath) - Expect(err).NotTo(HaveOccurred()) - Expect(cfg).To(Equal(expectedConfig)) - }) - }) -}) diff --git a/pkg/cli/webhook.go b/pkg/cli/webhook.go index 54fb19010ec..443edbf3585 100644 --- a/pkg/cli/webhook.go +++ b/pkg/cli/webhook.go @@ -21,7 +21,7 @@ import ( "github.com/spf13/cobra" - "sigs.k8s.io/kubebuilder/v3/pkg/cli/internal/config" + yamlstore "sigs.k8s.io/kubebuilder/v3/pkg/config/store/yaml" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" ) @@ -76,18 +76,15 @@ func (c cli) bindCreateWebhook(ctx plugin.Context, cmd *cobra.Command) { return } - cfg, err := config.LoadInitialized() - if err != nil { - cmdErr(cmd, err) - return - } - subcommand := createWebhookPlugin.GetCreateWebhookSubcommand() - subcommand.InjectConfig(cfg.Config) subcommand.BindFlags(cmd.Flags()) subcommand.UpdateContext(&ctx) cmd.Long = ctx.Description cmd.Example = ctx.Examples - cmd.RunE = runECmdFunc(c.fs, cfg, subcommand, - fmt.Sprintf("failed to create webhook with %q", plugin.KeyFor(createWebhookPlugin))) + + cfg := yamlstore.New(c.fs) + msg := fmt.Sprintf("failed to create webhook with %q", plugin.KeyFor(createWebhookPlugin)) + cmd.PreRunE = preRunECmdFunc(subcommand, cfg, msg) + cmd.RunE = runECmdFunc(c.fs, subcommand, msg) + cmd.PostRunE = postRunECmdFunc(cfg, msg) } diff --git a/pkg/config/store/errors.go b/pkg/config/store/errors.go new file mode 100644 index 00000000000..fd57aee0a9a --- /dev/null +++ b/pkg/config/store/errors.go @@ -0,0 +1,51 @@ +/* +Copyright 2021 The Kubernetes 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 store + +import ( + "fmt" +) + +// LoadError wraps errors yielded by Store.Load and Store.LoadFrom methods +type LoadError struct { + Err error +} + +// Error implements error interface +func (e LoadError) Error() string { + return fmt.Sprintf("unable to load the configuration: %v", e.Err) +} + +// Unwrap implements Wrapper interface +func (e LoadError) Unwrap() error { + return e.Err +} + +// SaveError wraps errors yielded by Store.Save and Store.SaveTo methods +type SaveError struct { + Err error +} + +// Error implements error interface +func (e SaveError) Error() string { + return fmt.Sprintf("unable to save the configuration: %v", e.Err) +} + +// Unwrap implements Wrapper interface +func (e SaveError) Unwrap() error { + return e.Err +} diff --git a/pkg/config/store/interface.go b/pkg/config/store/interface.go new file mode 100644 index 00000000000..2bb1a21cb01 --- /dev/null +++ b/pkg/config/store/interface.go @@ -0,0 +1,38 @@ +/* +Copyright 2021 The Kubernetes 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 store + +import ( + "sigs.k8s.io/kubebuilder/v3/pkg/config" +) + +// Store represents a persistence backend for config.Config +type Store interface { + // New creates a new config.Config to store + New(config.Version) error + // Load retrieves the config.Config from the persistence backend + Load() error + // LoadFrom retrieves the config.Config from the persistence backend at the specified key + LoadFrom(string) error + // Save stores the config.Config into the persistence backend + Save() error + // SaveTo stores the config.Config into the persistence backend at the specified key + SaveTo(string) error + + // Config returns the stored config.Config + Config() config.Config +} diff --git a/pkg/config/store/yaml/store.go b/pkg/config/store/yaml/store.go new file mode 100644 index 00000000000..7ca80c2a018 --- /dev/null +++ b/pkg/config/store/yaml/store.go @@ -0,0 +1,147 @@ +/* +Copyright 2021 The Kubernetes 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 yaml + +import ( + "fmt" + "os" + + "github.com/spf13/afero" + "sigs.k8s.io/yaml" + + "sigs.k8s.io/kubebuilder/v3/pkg/config" + "sigs.k8s.io/kubebuilder/v3/pkg/config/store" +) + +const ( + // DefaultPath is the default path for the configuration file + DefaultPath = "PROJECT" +) + +// yamlStore implements store.Store using a YAML file as the storage backend +// The key is translated into the YAML file path +type yamlStore struct { + // fs is the filesystem that will be used to store the config.Config + fs afero.Fs + // mustNotExist requires the file not to exist when saving it + mustNotExist bool + + cfg config.Config +} + +// New creates a new configuration that will be stored at the provided path +func New(fs afero.Fs) store.Store { + return &yamlStore{fs: fs} +} + +// New implements store.Store interface +func (s *yamlStore) New(version config.Version) error { + cfg, err := config.New(version) + if err != nil { + return err + } + + s.cfg = cfg + s.mustNotExist = true + return nil +} + +// Load implements store.Store interface +func (s *yamlStore) Load() error { + return s.LoadFrom(DefaultPath) +} + +type versionedConfig struct { + Version config.Version `json:"version"` +} + +// LoadFrom implements store.Store interface +func (s *yamlStore) LoadFrom(path string) error { + s.mustNotExist = false + + // Read the file + in, err := afero.ReadFile(s.fs, path) + if err != nil { + return store.LoadError{Err: fmt.Errorf("unable to read %q file: %w", path, err)} + } + + // Check the file version + var versioned versionedConfig + if err := yaml.Unmarshal(in, &versioned); err != nil { + return store.LoadError{Err: fmt.Errorf("unable to determine config version: %w", err)} + } + + // Create the config object + var cfg config.Config + cfg, err = config.New(versioned.Version) + if err != nil { + return store.LoadError{Err: fmt.Errorf("unable to create config for version %q: %w", versioned.Version, err)} + } + + // Unmarshal the file content + if err := cfg.Unmarshal(in); err != nil { + return store.LoadError{Err: fmt.Errorf("unable to unmarshal config at %q: %w", path, err)} + } + + s.cfg = cfg + return nil +} + +// Save implements store.Store interface +func (s yamlStore) Save() error { + return s.SaveTo(DefaultPath) +} + +// SaveTo implements store.Store interface +func (s yamlStore) SaveTo(path string) error { + // If yamlStore is unset, none of New, Load, or LoadFrom were called successfully + if s.cfg == nil { + return store.SaveError{Err: fmt.Errorf("undefined config, use one of the initializers: New, Load, LoadFrom")} + } + + // If it is a new configuration, the path should not exist yet + if s.mustNotExist { + // Lets check that the file doesn't exist + _, err := s.fs.Stat(path) + if os.IsNotExist(err) { + // This is exactly what we want + } else if err == nil || os.IsExist(err) { + return store.SaveError{Err: fmt.Errorf("configuration already exists in %q", path)} + } else { + return store.SaveError{Err: fmt.Errorf("unable to check for file prior existence: %w", err)} + } + } + + // Marshall into YAML + content, err := s.cfg.Marshal() + if err != nil { + return store.SaveError{Err: fmt.Errorf("unable to marshal to YAML: %w", err)} + } + + // Write the marshalled configuration + err = afero.WriteFile(s.fs, path, content, 0600) + if err != nil { + return store.SaveError{Err: fmt.Errorf("failed to save configuration to %q: %w", path, err)} + } + + return nil +} + +// Config implements store.Store interface +func (s yamlStore) Config() config.Config { + return s.cfg +} diff --git a/pkg/config/store/yaml/store_test.go b/pkg/config/store/yaml/store_test.go new file mode 100644 index 00000000000..4e7ec313a2a --- /dev/null +++ b/pkg/config/store/yaml/store_test.go @@ -0,0 +1,121 @@ +/* +Copyright 2021 The Kubernetes 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 yaml + +import ( + "os" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/spf13/afero" + + cfgv2 "sigs.k8s.io/kubebuilder/v3/pkg/config/v2" +) + +var _ = Describe("New", func() { + It("should return a new empty store", func() { + s := New(afero.NewMemMapFs()) + Expect(s.Config()).To(BeNil()) + + ys, ok := s.(*yamlStore) + Expect(ok).To(BeTrue()) + Expect(ys.fs).NotTo(BeNil()) + }) +}) + +var _ = Describe("yamlStore", func() { + var ( + s *yamlStore + ) + + BeforeEach(func() { + s = New(afero.NewMemMapFs()).(*yamlStore) + }) + + Context("New", func() { + It("should initialize a new Config backend for the provided version", func() { + Expect(s.New(cfgv2.Version)).To(Succeed()) + Expect(s.fs).NotTo(BeNil()) + Expect(s.mustNotExist).To(BeTrue()) + Expect(s.Config()).NotTo(BeNil()) + Expect(s.Config().GetVersion().Compare(cfgv2.Version)).To(Equal(0)) + }) + }) + + Context("Load", func() { + It("should load the Config from an existing file at the default path", func() { + configStr := `version: "2"` + + Expect(afero.WriteFile(s.fs, DefaultPath, []byte(configStr), os.ModePerm)).To(Succeed()) + + Expect(s.Load()).To(Succeed()) + Expect(s.fs).NotTo(BeNil()) + Expect(s.mustNotExist).To(BeFalse()) + Expect(s.Config()).NotTo(BeNil()) + Expect(s.Config().GetVersion().Compare(cfgv2.Version)).To(Equal(0)) + }) + }) + + Context("LoadFrom", func() { + It("should load the Config from an existing file from the specified path", func() { + configStr := `version: "2"` + path := DefaultPath + "2" + + Expect(afero.WriteFile(s.fs, path, []byte(configStr), os.ModePerm)).To(Succeed()) + + Expect(s.LoadFrom(path)).To(Succeed()) + Expect(s.fs).NotTo(BeNil()) + Expect(s.mustNotExist).To(BeFalse()) + Expect(s.Config()).NotTo(BeNil()) + Expect(s.Config().GetVersion().Compare(cfgv2.Version)).To(Equal(0)) + }) + }) + + Context("Save", func() { + It("should success for valid configs", func() { + s.cfg = cfgv2.New() + Expect(s.Save()).To(Succeed()) + + cfgBytes, err := afero.ReadFile(s.fs, DefaultPath) + Expect(err).NotTo(HaveOccurred()) + Expect(string(cfgBytes)).To(Equal(`version: "2" +`)) + }) + + It("should fail for an empty config", func() { + Expect(s.Save()).NotTo(Succeed()) + }) + }) + + Context("SaveTo", func() { + path := DefaultPath + "2" + + It("should success for valid configs", func() { + s.cfg = cfgv2.New() + Expect(s.SaveTo(path)).To(Succeed()) + + cfgBytes, err := afero.ReadFile(s.fs, path) + Expect(err).NotTo(HaveOccurred()) + Expect(string(cfgBytes)).To(Equal(`version: "2" +`)) + }) + + It("should fail for an empty config", func() { + Expect(s.SaveTo(path)).NotTo(Succeed()) + }) + }) +}) diff --git a/pkg/cli/internal/config/config_suite_test.go b/pkg/config/store/yaml/suite_test.go similarity index 82% rename from pkg/cli/internal/config/config_suite_test.go rename to pkg/config/store/yaml/suite_test.go index fffe2f8bb2f..65f6f8314c2 100644 --- a/pkg/cli/internal/config/config_suite_test.go +++ b/pkg/config/store/yaml/suite_test.go @@ -1,5 +1,5 @@ /* -Copyright 2020 The Kubernetes Authors. +Copyright 2021 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package config +package yaml import ( "testing" @@ -23,7 +23,7 @@ import ( . "github.com/onsi/gomega" ) -func TestCLI(t *testing.T) { +func TestConfigStoreYaml(t *testing.T) { RegisterFailHandler(Fail) - RunSpecs(t, "Config Suite") + RunSpecs(t, "Config Store YAML Suite") } diff --git a/pkg/config/v3/config.go b/pkg/config/v3/config.go index 144e9580d96..cbacc61521b 100644 --- a/pkg/config/v3/config.go +++ b/pkg/config/v3/config.go @@ -47,12 +47,11 @@ type cfg struct { Resources []resource.Resource `json:"resources,omitempty"` // Plugins - Plugins PluginConfigs `json:"plugins,omitempty"` + Plugins pluginConfigs `json:"plugins,omitempty"` } -// PluginConfigs holds a set of arbitrary plugin configuration objects mapped by plugin key. -// TODO: do not export this once internalconfig has merged with config -type PluginConfigs map[string]pluginConfig +// pluginConfigs holds a set of arbitrary plugin configuration objects mapped by plugin key. +type pluginConfigs map[string]pluginConfig // pluginConfig is an arbitrary plugin configuration object. type pluginConfig interface{} diff --git a/pkg/config/v3/config_test.go b/pkg/config/v3/config_test.go index 37ed9d864af..b42cc42cd26 100644 --- a/pkg/config/v3/config_test.go +++ b/pkg/config/v3/config_test.go @@ -367,7 +367,7 @@ var _ = Describe("cfg", func() { Repository: repo, Name: name, Layout: layout, - Plugins: PluginConfigs{ + Plugins: pluginConfigs{ "plugin-x": map[string]interface{}{ "data-1": "", }, @@ -379,7 +379,7 @@ var _ = Describe("cfg", func() { Repository: repo, Name: name, Layout: layout, - Plugins: PluginConfigs{ + Plugins: pluginConfigs{ "plugin-x": map[string]interface{}{ "data-1": "plugin value 1", "data-2": "plugin value 2", @@ -488,7 +488,7 @@ var _ = Describe("cfg", func() { }, }, }, - Plugins: PluginConfigs{ + Plugins: pluginConfigs{ "plugin-x": map[string]interface{}{ "data-1": "single plugin datum", }, diff --git a/pkg/plugin/interfaces.go b/pkg/plugin/interfaces.go index 1b050f36060..6957c104c1e 100644 --- a/pkg/plugin/interfaces.go +++ b/pkg/plugin/interfaces.go @@ -52,12 +52,13 @@ type Subcommand interface { UpdateContext(*Context) // BindFlags binds the subcommand's flags to the CLI. This allows each subcommand to define its own // command line flags. + // NOTE(Adirio): by the time we bind flags, the config hasn't been injected, trying to use it panics BindFlags(*pflag.FlagSet) - // Run runs the subcommand. - Run(fs afero.Fs) error // InjectConfig passes a config to a plugin. The plugin may modify the config. // Initializing, loading, and saving the config is managed by the cli package. InjectConfig(config.Config) + // Run runs the subcommand. + Run(fs afero.Fs) error } // Context is the runtime context for a subcommand. diff --git a/pkg/plugins/golang/v2/api.go b/pkg/plugins/golang/v2/api.go index 0c9a64c6c00..997c9af174f 100644 --- a/pkg/plugins/golang/v2/api.go +++ b/pkg/plugins/golang/v2/api.go @@ -105,7 +105,6 @@ func (p *createAPISubcommand) BindFlags(fs *pflag.FlagSet) { p.options = &Options{} fs.StringVar(&p.options.Group, "group", "", "resource Group") - p.options.Domain = p.config.GetDomain() fs.StringVar(&p.options.Version, "version", "", "resource Version") fs.StringVar(&p.options.Kind, "kind", "", "resource Kind") // p.options.Plural can be set to specify an irregular plural form @@ -123,6 +122,8 @@ func (p *createAPISubcommand) BindFlags(fs *pflag.FlagSet) { func (p *createAPISubcommand) InjectConfig(c config.Config) { p.config = c + + p.options.Domain = c.GetDomain() } func (p *createAPISubcommand) Run(fs afero.Fs) error { diff --git a/pkg/plugins/golang/v2/init.go b/pkg/plugins/golang/v2/init.go index 9ddfa2f8631..62fe6f97326 100644 --- a/pkg/plugins/golang/v2/init.go +++ b/pkg/plugins/golang/v2/init.go @@ -96,9 +96,7 @@ func (p *initSubcommand) BindFlags(fs *pflag.FlagSet) { fs.StringVar(&p.domain, "domain", "my.domain", "domain for groups") fs.StringVar(&p.repo, "repo", "", "name to use for go module (e.g., github.com/user/repo), "+ "defaults to the go package of the current working directory.") - if p.config.GetVersion().Compare(cfgv2.Version) > 0 { - fs.StringVar(&p.name, "project-name", "", "name of this project") - } + fs.StringVar(&p.name, "project-name", "", "name of this project") } func (p *initSubcommand) InjectConfig(c config.Config) { diff --git a/pkg/plugins/golang/v2/webhook.go b/pkg/plugins/golang/v2/webhook.go index 3048353756e..0ce7176f8ab 100644 --- a/pkg/plugins/golang/v2/webhook.go +++ b/pkg/plugins/golang/v2/webhook.go @@ -22,7 +22,7 @@ import ( "github.com/spf13/afero" "github.com/spf13/pflag" - newconfig "sigs.k8s.io/kubebuilder/v3/pkg/config" + "sigs.k8s.io/kubebuilder/v3/pkg/config" "sigs.k8s.io/kubebuilder/v3/pkg/model/resource" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v2/scaffolds" @@ -30,7 +30,7 @@ import ( ) type createWebhookSubcommand struct { - config newconfig.Config + config config.Config // For help text. commandName string @@ -63,7 +63,6 @@ validating and (or) conversion webhooks. func (p *createWebhookSubcommand) BindFlags(fs *pflag.FlagSet) { p.options = &Options{} fs.StringVar(&p.options.Group, "group", "", "resource Group") - p.options.Domain = p.config.GetDomain() fs.StringVar(&p.options.Version, "version", "", "resource Version") fs.StringVar(&p.options.Kind, "kind", "", "resource Kind") fs.StringVar(&p.options.Plural, "resource", "", "resource irregular plural form") @@ -77,8 +76,10 @@ func (p *createWebhookSubcommand) BindFlags(fs *pflag.FlagSet) { "if set, scaffold the conversion webhook") } -func (p *createWebhookSubcommand) InjectConfig(c newconfig.Config) { +func (p *createWebhookSubcommand) InjectConfig(c config.Config) { p.config = c + + p.options.Domain = c.GetDomain() } func (p *createWebhookSubcommand) Run(fs afero.Fs) error { diff --git a/pkg/plugins/golang/v3/api.go b/pkg/plugins/golang/v3/api.go index 97d3dfbda0d..48525c68699 100644 --- a/pkg/plugins/golang/v3/api.go +++ b/pkg/plugins/golang/v3/api.go @@ -120,7 +120,6 @@ func (p *createAPISubcommand) BindFlags(fs *pflag.FlagSet) { p.options = &goPlugin.Options{} fs.StringVar(&p.options.Group, "group", "", "resource Group") - p.options.Domain = p.config.GetDomain() fs.StringVar(&p.options.Version, "version", "", "resource Version") fs.StringVar(&p.options.Kind, "kind", "", "resource Kind") fs.StringVar(&p.options.Plural, "plural", "", "resource irregular plural form") @@ -139,6 +138,9 @@ func (p *createAPISubcommand) BindFlags(fs *pflag.FlagSet) { func (p *createAPISubcommand) InjectConfig(c config.Config) { p.config = c + + // TODO: offer a flag instead of hard-coding the project-wide domain + p.options.Domain = c.GetDomain() } func (p *createAPISubcommand) Run(fs afero.Fs) error { diff --git a/pkg/plugins/golang/v3/webhook.go b/pkg/plugins/golang/v3/webhook.go index e6b6b56d98b..931d2eca51b 100644 --- a/pkg/plugins/golang/v3/webhook.go +++ b/pkg/plugins/golang/v3/webhook.go @@ -71,7 +71,6 @@ validating and (or) conversion webhooks. func (p *createWebhookSubcommand) BindFlags(fs *pflag.FlagSet) { p.options = &goPlugin.Options{} fs.StringVar(&p.options.Group, "group", "", "resource Group") - p.options.Domain = p.config.GetDomain() fs.StringVar(&p.options.Version, "version", "", "resource Version") fs.StringVar(&p.options.Kind, "kind", "", "resource Kind") fs.StringVar(&p.options.Plural, "plural", "", "resource irregular plural form") @@ -91,6 +90,9 @@ func (p *createWebhookSubcommand) BindFlags(fs *pflag.FlagSet) { func (p *createWebhookSubcommand) InjectConfig(c config.Config) { p.config = c + + // TODO: offer a flag instead of hard-coding the project-wide domain + p.options.Domain = c.GetDomain() } func (p *createWebhookSubcommand) Run(fs afero.Fs) error {