From dff314ce9c8c38a3eff9bd059a3a48e564cd19b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Orive?= Date: Wed, 10 Mar 2021 13:38:12 +0100 Subject: [PATCH] Store abstraction for persisting Config - Store interface and YAML implementation for current usage - 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 | 10 +- pkg/cli/cmd_helpers.go | 42 +++- pkg/cli/edit.go | 17 +- pkg/cli/init.go | 36 +-- pkg/cli/internal/config/config.go | 189 -------------- pkg/cli/internal/config/config_suite_test.go | 29 --- pkg/cli/internal/config/config_test.go | 71 ------ pkg/cli/webhook.go | 17 +- pkg/config/store/errors.go | 51 ++++ pkg/config/store/errors_test.go | 68 +++++ pkg/config/store/interface.go | 38 +++ pkg/config/store/yaml/store.go | 147 +++++++++++ pkg/config/store/yaml/store_test.go | 245 +++++++++++++++++++ pkg/config/v3/config.go | 7 +- pkg/config/v3/config_test.go | 17 +- 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, 658 insertions(+), 372 deletions(-) delete mode 100644 pkg/cli/internal/config/config.go delete mode 100644 pkg/cli/internal/config/config_suite_test.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/errors_test.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 diff --git a/pkg/cli/api.go b/pkg/cli/api.go index 27bc4126f22..60cccb81198 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(c.fs) - 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 bf8b88c08ed..0f2ef53861e 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" ) @@ -195,16 +196,17 @@ func (c *CLI) getInfoFromFlags() (string, []string, error) { // getInfoFromConfigFile obtains the project version and plugin keys from the project config file. func (c CLI) getInfoFromConfigFile() (config.Version, []string, error) { // Read the project configuration file - projectConfig, err := internalconfig.Read(c.fs) + 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. 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 3d8b93192c4..07ff17a3c83 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(c.fs) - 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 e1a6e12dc31..04a38bdbd80 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.fs, 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(c.fs) - 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 31d8bb1781b..00000000000 --- a/pkg/cli/internal/config/config.go +++ /dev/null @@ -1,189 +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" -) - -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(fs afero.Fs) (config.Config, error) { - return ReadFrom(fs, DefaultPath) -} - -// ReadFrom obtains the configuration from the provided path but doesn't allow to persist changes -func ReadFrom(fs afero.Fs, path string) (config.Config, error) { - return readFrom(fs, 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(fs afero.Fs, 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: fs, - }, nil -} - -// Load obtains the configuration from the default path allowing to persist changes (Save method) -func Load(fs afero.Fs) (*Config, error) { - return LoadFrom(fs, DefaultPath) -} - -// LoadInitialized calls Load() but returns helpful error messages if the config -// does not exist. -func LoadInitialized(fs afero.Fs) (*Config, error) { - c, err := Load(fs) - 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(fs afero.Fs, path string) (*Config, error) { - 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 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_suite_test.go b/pkg/cli/internal/config/config_suite_test.go deleted file mode 100644 index fffe2f8bb2f..00000000000 --- a/pkg/cli/internal/config/config_suite_test.go +++ /dev/null @@ -1,29 +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 ( - "testing" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -func TestCLI(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Config Suite") -} 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 c078e234196..80d102d1bc8 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(c.fs) - 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/errors_test.go b/pkg/config/store/errors_test.go new file mode 100644 index 00000000000..acc9b445b1c --- /dev/null +++ b/pkg/config/store/errors_test.go @@ -0,0 +1,68 @@ +/* +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" + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestConfigStore(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Config Store Suite") +} + +var _ = Describe("LoadError", func() { + var ( + wrapped = fmt.Errorf("error message") + err = LoadError{Err: wrapped} + ) + + Context("Error", func() { + It("should return the correct error message", func() { + Expect(err.Error()).To(Equal(fmt.Sprintf("unable to load the configuration: %v", wrapped))) + }) + }) + + Context("Unwrap", func() { + It("should unwrap to the wrapped error", func() { + Expect(err.Unwrap()).To(Equal(wrapped)) + }) + }) +}) + +var _ = Describe("SaveError", func() { + var ( + wrapped = fmt.Errorf("error message") + err = SaveError{Err: wrapped} + ) + + Context("Error", func() { + It("should return the correct error message", func() { + Expect(err.Error()).To(Equal(fmt.Sprintf("unable to save the configuration: %v", wrapped))) + }) + }) + + Context("Unwrap", func() { + It("should unwrap to the wrapped error", func() { + Expect(err.Unwrap()).To(Equal(wrapped)) + }) + }) +}) 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..5d3521f13f5 --- /dev/null +++ b/pkg/config/store/yaml/store_test.go @@ -0,0 +1,245 @@ +/* +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 ( + "errors" + "os" + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/spf13/afero" + + "sigs.k8s.io/kubebuilder/v3/pkg/config" + "sigs.k8s.io/kubebuilder/v3/pkg/config/store" + cfgv2 "sigs.k8s.io/kubebuilder/v3/pkg/config/v2" +) + +func TestConfigStoreYaml(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Config Store YAML Suite") +} + +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() { + const ( + v2File = `version: "2" +` + unversionedFile = `version: +` + nonexistentVersionFile = `version: 1-alpha +` // v1-alpha never existed + wrongFile = `version: "2" +layout: "" +` // layout field does not exist in v2 + ) + + var ( + s *yamlStore + + path = DefaultPath + "2" + ) + + 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)) + }) + + It("should fail for an unregistered config version", func() { + Expect(s.New(config.Version{})).NotTo(Succeed()) + }) + }) + + Context("Load", func() { + It("should load the Config from an existing file at the default path", func() { + Expect(afero.WriteFile(s.fs, DefaultPath, []byte(v2File), 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)) + }) + + It("should fail if no file exists at the default path", func() { + err := s.Load() + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.LoadError{})).To(BeTrue()) + }) + + It("should fail if unable to identify the version of the file at the default path", func() { + Expect(afero.WriteFile(s.fs, DefaultPath, []byte(unversionedFile), os.ModePerm)).To(Succeed()) + + err := s.Load() + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.LoadError{})).To(BeTrue()) + }) + + It("should fail if unable to create a Config for the version of the file at the default path", func() { + Expect(afero.WriteFile(s.fs, DefaultPath, []byte(nonexistentVersionFile), os.ModePerm)).To(Succeed()) + + err := s.Load() + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.LoadError{})).To(BeTrue()) + }) + + It("should fail if unable to unmarshal the file at the default path", func() { + Expect(afero.WriteFile(s.fs, DefaultPath, []byte(wrongFile), os.ModePerm)).To(Succeed()) + + err := s.Load() + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.LoadError{})).To(BeTrue()) + }) + }) + + Context("LoadFrom", func() { + It("should load the Config from an existing file from the specified path", func() { + Expect(afero.WriteFile(s.fs, path, []byte(v2File), 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)) + }) + + It("should fail if no file exists at the specified path", func() { + err := s.LoadFrom(path) + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.LoadError{})).To(BeTrue()) + }) + + It("should fail if unable to identify the version of the file at the specified path", func() { + Expect(afero.WriteFile(s.fs, path, []byte(unversionedFile), os.ModePerm)).To(Succeed()) + + err := s.LoadFrom(path) + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.LoadError{})).To(BeTrue()) + }) + + It("should fail if unable to create a Config for the version of the file at the specified path", func() { + Expect(afero.WriteFile(s.fs, path, []byte(nonexistentVersionFile), os.ModePerm)).To(Succeed()) + + err := s.LoadFrom(path) + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.LoadError{})).To(BeTrue()) + }) + + It("should fail if unable to unmarshal the file at the specified path", func() { + Expect(afero.WriteFile(s.fs, path, []byte(wrongFile), os.ModePerm)).To(Succeed()) + + err := s.LoadFrom(path) + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.LoadError{})).To(BeTrue()) + }) + }) + + Context("Save", func() { + + It("should succeed for a valid config", 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(v2File)) + }) + + It("should succeed for a valid config that must not exist", func() { + s.cfg = cfgv2.New() + s.mustNotExist = true + Expect(s.Save()).To(Succeed()) + + cfgBytes, err := afero.ReadFile(s.fs, DefaultPath) + Expect(err).NotTo(HaveOccurred()) + Expect(string(cfgBytes)).To(Equal(v2File)) + }) + + It("should fail for an empty config", func() { + err := s.Save() + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.SaveError{})).To(BeTrue()) + }) + + It("should fail for a pre-existent file that must not exist", func() { + s.cfg = cfgv2.New() + s.mustNotExist = true + Expect(afero.WriteFile(s.fs, DefaultPath, []byte(v2File), os.ModePerm)).To(Succeed()) + + err := s.Save() + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.SaveError{})).To(BeTrue()) + }) + }) + + Context("SaveTo", func() { + 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 succeed for a valid config that must not exist", func() { + s.cfg = cfgv2.New() + s.mustNotExist = true + Expect(s.SaveTo(path)).To(Succeed()) + + cfgBytes, err := afero.ReadFile(s.fs, path) + Expect(err).NotTo(HaveOccurred()) + Expect(string(cfgBytes)).To(Equal(v2File)) + }) + + It("should fail for an empty config", func() { + err := s.SaveTo(path) + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.SaveError{})).To(BeTrue()) + }) + + It("should fail for a pre-existent file that must not exist", func() { + s.cfg = cfgv2.New() + s.mustNotExist = true + Expect(afero.WriteFile(s.fs, path, []byte(v2File), os.ModePerm)).To(Succeed()) + + err := s.SaveTo(path) + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &store.SaveError{})).To(BeTrue()) + }) + }) +}) diff --git a/pkg/config/v3/config.go b/pkg/config/v3/config.go index a462bbd5249..b2d2de713a8 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 cbcb5890f99..a8eb4af5939 100644 --- a/pkg/config/v3/config_test.go +++ b/pkg/config/v3/config_test.go @@ -386,8 +386,8 @@ var _ = Describe("cfg", func() { Repository: repo, Name: name, Layout: layout, - Plugins: PluginConfigs{ - "plugin-x": map[string]interface{}{ + Plugins: pluginConfigs{ + key: map[string]interface{}{ "data-1": "", }, }, @@ -398,8 +398,8 @@ var _ = Describe("cfg", func() { Repository: repo, Name: name, Layout: layout, - Plugins: PluginConfigs{ - "plugin-x": map[string]interface{}{ + Plugins: pluginConfigs{ + key: map[string]interface{}{ "data-1": "plugin value 1", "data-2": "plugin value 2", }, @@ -418,6 +418,13 @@ var _ = Describe("cfg", func() { Expect(errors.As(err, &config.PluginKeyNotFoundError{})).To(BeTrue()) }) + It("DecodePluginConfig should fail to retrieve data from a non-existent plugin", func() { + var pluginConfig PluginConfig + err := c1.DecodePluginConfig("plugin-y", &pluginConfig) + Expect(err).To(HaveOccurred()) + Expect(errors.As(err, &config.PluginKeyNotFoundError{})).To(BeTrue()) + }) + DescribeTable("DecodePluginConfig should retrieve the plugin data correctly", func(inputConfig cfg, expectedPluginConfig PluginConfig) { var pluginConfig PluginConfig @@ -507,7 +514,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 5b8346e7647..28db64803d7 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 dbaf1360b1f..d4c421f4b67 100644 --- a/pkg/plugins/golang/v2/init.go +++ b/pkg/plugins/golang/v2/init.go @@ -97,9 +97,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 36537b0961a..d472db1f99f 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" cfgv2 "sigs.k8s.io/kubebuilder/v3/pkg/config/v2" "sigs.k8s.io/kubebuilder/v3/pkg/model/resource" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" @@ -31,7 +31,7 @@ import ( ) type createWebhookSubcommand struct { - config newconfig.Config + config config.Config // For help text. commandName string @@ -64,7 +64,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") @@ -78,8 +77,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 d1deadac5a9..d36ed5a6250 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 7ee2234b983..3f66641811e 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 {