Merge pull request #2096 from Adirio/config-chain

✨ Enable the storage of multiple plugins as layout

Author: estroz

pkg/cli/cli.go

@@ -220,15 +220,7 @@ func (c CLI) getInfoFromConfigFile() (config.Version, []string, error) {
 // getInfoFromConfig obtains the project version and plugin keys from the project config.
 // It is extracted from getInfoFromConfigFile for testing purposes.
 func getInfoFromConfig(projectConfig config.Config) (config.Version, []string, error) {
-	// Split the comma-separated plugins
-	var pluginSet []string
-	if projectConfig.GetLayout() != "" {
-		for _, p := range strings.Split(projectConfig.GetLayout(), ",") {
-			pluginSet = append(pluginSet, strings.TrimSpace(p))
-		}
-	}
-
-	return projectConfig.GetVersion(), pluginSet, nil
+	return projectConfig.GetVersion(), projectConfig.GetPluginChain(), nil
 }
 
 // resolveFlagsAndConfigFileConflicts checks if the provided combined input from flags and

pkg/cli/cli_test.go

@@ -191,6 +191,8 @@ var _ = Describe("CLI", func() {
 			projectVersion config.Version
 			plugins        []string
 			err            error
+
+			pluginChain = []string{"go.kubebuilder.io/v2"}
 		)
 
 		When("not having layout field", func() {
@@ -199,18 +201,18 @@ var _ = Describe("CLI", func() {
 				projectVersion, plugins, err = getInfoFromConfig(projectConfig)
 				Expect(err).NotTo(HaveOccurred())
 				Expect(projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0))
-				Expect(len(plugins)).To(Equal(0))
+				Expect(plugins).To(Equal(pluginChain))
 			})
 		})
 
 		When("having layout field", func() {
 			It("should succeed", func() {
 				projectConfig = cfgv3.New()
-				Expect(projectConfig.SetLayout("go.kubebuilder.io/v2")).To(Succeed())
+				Expect(projectConfig.SetPluginChain(pluginChain)).To(Succeed())
 				projectVersion, plugins, err = getInfoFromConfig(projectConfig)
 				Expect(err).NotTo(HaveOccurred())
 				Expect(projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0))
-				Expect(plugins).To(Equal([]string{projectConfig.GetLayout()}))
+				Expect(plugins).To(Equal(pluginChain))
 			})
 		})
 	})

pkg/config/interface.go

@@ -20,78 +20,78 @@ import (
 	"sigs.k8s.io/kubebuilder/v3/pkg/model/resource"
 )
 
-// Config defines the interface that project configuration types must follow
+// Config defines the interface that project configuration types must follow.
 type Config interface {
 	/* Version */
 
-	// GetVersion returns the current project version
+	// GetVersion returns the current project version.
 	GetVersion() Version
 
 	/* String fields */
 
-	// GetDomain returns the project domain
+	// GetDomain returns the project domain.
 	GetDomain() string
-	// SetDomain sets the project domain
+	// SetDomain sets the project domain.
 	SetDomain(domain string) error
 
 	// GetRepository returns the project repository.
 	GetRepository() string
-	// SetRepository sets the project repository
+	// SetRepository sets the project repository.
 	SetRepository(repository string) error
 
-	// GetProjectName returns the project name
+	// GetProjectName returns the project name.
 	// This method was introduced in project version 3.
 	GetProjectName() string
-	// SetProjectName sets the project name
+	// SetProjectName sets the project name.
 	// This method was introduced in project version 3.
 	SetProjectName(name string) error
 
-	// GetLayout returns the config layout
+	// GetPluginChain returns the plugin chain.
 	// This method was introduced in project version 3.
-	GetLayout() string
-	// SetLayout sets the Config layout
+	GetPluginChain() []string
+	// SetPluginChain sets the plugin chain.
 	// This method was introduced in project version 3.
-	SetLayout(layout string) error
+	SetPluginChain(pluginChain []string) error
 
 	/* Boolean fields */
 
-	// IsMultiGroup checks if multi-group is enabled
+	// IsMultiGroup checks if multi-group is enabled.
 	IsMultiGroup() bool
-	// SetMultiGroup enables multi-group
+	// SetMultiGroup enables multi-group.
 	SetMultiGroup() error
-	// ClearMultiGroup disables multi-group
+	// ClearMultiGroup disables multi-group.
 	ClearMultiGroup() error
 
-	// IsComponentConfig checks if component config is enabled
+	// IsComponentConfig checks if component config is enabled.
 	// This method was introduced in project version 3.
 	IsComponentConfig() bool
-	// SetComponentConfig enables component config
+	// SetComponentConfig enables component config.
 	// This method was introduced in project version 3.
 	SetComponentConfig() error
-	// ClearComponentConfig disables component config
+	// ClearComponentConfig disables component config.
 	// This method was introduced in project version 3.
 	ClearComponentConfig() error
 
 	/* Resources */
 
-	// ResourcesLength returns the number of tracked resources
+	// ResourcesLength returns the number of tracked resources.
 	ResourcesLength() int
-	// HasResource checks if the provided GVK is stored in the Config
+	// HasResource checks if the provided GVK is stored in the Config.
 	HasResource(gvk resource.GVK) bool
-	// GetResource returns the stored resource matching the provided GVK
+	// GetResource returns the stored resource matching the provided GVK.
 	GetResource(gvk resource.GVK) (resource.Resource, error)
-	// GetResources returns all the stored resources
+	// GetResources returns all the stored resources.
 	GetResources() ([]resource.Resource, error)
-	// AddResource adds the provided resource if it was not present, no-op if it was already present
+	// AddResource adds the provided resource if it was not present, no-op if it was already present.
 	AddResource(res resource.Resource) error
-	// UpdateResource adds the provided resource if it was not present, modifies it if it was already present
+	// UpdateResource adds the provided resource if it was not present, modifies it if it was already present.
 	UpdateResource(res resource.Resource) error
 
-	// HasGroup checks if the provided group is the same as any of the tracked resources
+	// HasGroup checks if the provided group is the same as any of the tracked resources.
 	HasGroup(group string) bool
-	// ListCRDVersions returns a list of the CRD versions in use by the tracked resources
+	// ListCRDVersions returns a list of the CRD versions in use by the tracked resources.
 	ListCRDVersions() []string
-	// ListWebhookVersions returns a list of the webhook versions in use by the tracked resources
+	// ListWebhookVersions returns a list of the webhook versions in use by the tracked resources.
 	ListWebhookVersions() []string
 
 	/* Plugins */
@@ -105,8 +105,8 @@ type Config interface {
 
 	/* Persistence */
 
-	// Marshal returns the YAML representation of the Config
-	Marshal() ([]byte, error)
-	// Unmarshal loads the Config fields from its YAML representation
-	Unmarshal([]byte) error
+	// Marshal returns the YAML representation of the Config.
+	MarshalYAML() ([]byte, error)
+	// Unmarshal loads the Config fields from its YAML representation.
+	UnmarshalYAML([]byte) error
 }

pkg/config/store/yaml/store.go

@@ -94,7 +94,7 @@ func (s *yamlStore) LoadFrom(path string) error {
 	}
 
 	// Unmarshal the file content
-	if err := cfg.Unmarshal(in); err != nil {
+	if err := cfg.UnmarshalYAML(in); err != nil {
 		return store.LoadError{Err: fmt.Errorf("unable to unmarshal config at %q: %w", path, err)}
 	}
 
@@ -128,7 +128,7 @@ func (s yamlStore) SaveTo(path string) error {
 	}
 
 	// Marshall into YAML
-	content, err := s.cfg.Marshal()
+	content, err := s.cfg.MarshalYAML()
 	if err != nil {
 		return store.SaveError{Err: fmt.Errorf("unable to marshal to YAML: %w", err)}
 	}

pkg/config/v2/config.go

@@ -92,16 +92,16 @@ func (c *cfg) SetProjectName(string) error {
 	}
 }
 
-// GetLayout implements config.Config
-func (c cfg) GetLayout() string {
-	return ""
+// GetPluginChain implements config.Config
+func (c cfg) GetPluginChain() []string {
+	return []string{"go.kubebuilder.io/v2"}
 }
 
-// SetLayout implements config.Config
-func (c *cfg) SetLayout(string) error {
+// SetPluginChain implements config.Config
+func (c *cfg) SetPluginChain([]string) error {
 	return config.UnsupportedFieldError{
 		Version: Version,
-		Field:   "layout",
+		Field:   "plugin chain",
 	}
 }
 
@@ -247,7 +247,7 @@ func (c cfg) EncodePluginConfig(string, interface{}) error {
 }
 
 // Marshal implements config.Config
-func (c cfg) Marshal() ([]byte, error) {
+func (c cfg) MarshalYAML() ([]byte, error) {
 	content, err := yaml.Marshal(c)
 	if err != nil {
 		return nil, config.MarshalError{Err: err}
@@ -257,7 +257,7 @@ func (c cfg) Marshal() ([]byte, error) {
 }
 
 // Unmarshal implements config.Config
-func (c *cfg) Unmarshal(b []byte) error {
+func (c *cfg) UnmarshalYAML(b []byte) error {
 	if err := yaml.UnmarshalStrict(b, c); err != nil {
 		return config.UnmarshalError{Err: err}
 	}

pkg/config/v2/config_test.go

@@ -78,7 +78,7 @@ var _ = Describe("cfg", func() {
 		})
 	})
 
-	Context("ProjectName", func() {
+	Context("Project name", func() {
 		It("GetProjectName should return an empty name", func() {
 			Expect(c.GetProjectName()).To(Equal(""))
 		})
@@ -88,13 +88,13 @@ var _ = Describe("cfg", func() {
 		})
 	})
 
-	Context("Layout", func() {
-		It("GetLayout should return an empty layout", func() {
-			Expect(c.GetLayout()).To(Equal(""))
+	Context("Plugin chain", func() {
+		It("GetPluginChain should return the only supported plugin", func() {
+			Expect(c.GetPluginChain()).To(Equal([]string{"go.kubebuilder.io/v2"}))
 		})
 
-		It("SetLayout should fail to set the layout", func() {
-			Expect(c.SetLayout("layout")).NotTo(Succeed())
+		It("SetPluginChain should fail to set the plugin chain", func() {
+			Expect(c.SetPluginChain([]string{})).NotTo(Succeed())
 		})
 	})
 
@@ -288,28 +288,28 @@ version: "2"
 `
 		)
 
-		DescribeTable("Marshal should succeed",
+		DescribeTable("MarshalYAML should succeed",
 			func(c cfg, content string) {
-				b, err := c.Marshal()
+				b, err := c.MarshalYAML()
 				Expect(err).NotTo(HaveOccurred())
 				Expect(string(b)).To(Equal(content))
 			},
 			Entry("for a basic configuration", c1, s1),
 			Entry("for a full configuration", c2, s2),
 		)
 
-		DescribeTable("Marshal should fail",
+		DescribeTable("MarshalYAML should fail",
 			func(c cfg) {
-				_, err := c.Marshal()
+				_, err := c.MarshalYAML()
 				Expect(err).To(HaveOccurred())
 			},
 			// TODO (coverage): add cases where yaml.Marshal returns an error
 		)
 
-		DescribeTable("Unmarshal should succeed",
+		DescribeTable("UnmarshalYAML should succeed",
 			func(content string, c cfg) {
 				var unmarshalled cfg
-				Expect(unmarshalled.Unmarshal([]byte(content))).To(Succeed())
+				Expect(unmarshalled.UnmarshalYAML([]byte(content))).To(Succeed())
 				Expect(unmarshalled.Version.Compare(c.Version)).To(Equal(0))
 				Expect(unmarshalled.Domain).To(Equal(c.Domain))
 				Expect(unmarshalled.Repository).To(Equal(c.Repository))
@@ -320,10 +320,10 @@ version: "2"
 			Entry("full", s2, c2),
 		)
 
-		DescribeTable("Unmarshal should fail",
+		DescribeTable("UnmarshalYAML should fail",
 			func(content string) {
 				var c cfg
-				Expect(c.Unmarshal([]byte(content))).NotTo(Succeed())
+				Expect(c.UnmarshalYAML([]byte(content))).NotTo(Succeed())
 			},
 			Entry("for unknown fields", `field: 1
 version: "2"`),

pkg/config/v3/config.go

@@ -29,15 +29,38 @@ import (
 // Version is the config.Version for project configuration 3
 var Version = config.Version{Number: 3}
 
+// stringSlice is a []string but that can also be unmarshalled from a single string,
+// which is introduced as the first and only element of the slice
+// It is used to offer backwards compatibility as the field used to be a string.
+type stringSlice []string
+
+func (ss *stringSlice) UnmarshalJSON(b []byte) error {
+	if b[0] == '[' {
+		var sl []string
+		if err := yaml.Unmarshal(b, &sl); err != nil {
+			return err
+		}
+		*ss = sl
+		return nil
+	}
+
+	var st string
+	if err := yaml.Unmarshal(b, &st); err != nil {
+		return err
+	}
+	*ss = stringSlice{st}
+	return nil
+}
+
 type cfg struct {
 	// Version
 	Version config.Version `json:"version"`
 
 	// String fields
-	Domain     string `json:"domain,omitempty"`
-	Repository string `json:"repo,omitempty"`
-	Name       string `json:"projectName,omitempty"`
-	Layout     string `json:"layout,omitempty"`
+	Domain      string      `json:"domain,omitempty"`
+	Repository  string      `json:"repo,omitempty"`
+	Name        string      `json:"projectName,omitempty"`
+	PluginChain stringSlice `json:"layout,omitempty"`
 
 	// Boolean fields
 	MultiGroup      bool `json:"multigroup,omitempty"`
@@ -104,13 +127,13 @@ func (c *cfg) SetProjectName(name string) error {
 }
 
 // GetLayout implements config.Config
-func (c cfg) GetLayout() string {
-	return c.Layout
+func (c cfg) GetPluginChain() []string {
+	return c.PluginChain
 }
 
 // SetLayout implements config.Config
-func (c *cfg) SetLayout(layout string) error {
-	c.Layout = layout
+func (c *cfg) SetPluginChain(pluginChain []string) error {
+	c.PluginChain = pluginChain
 	return nil
 }
 
@@ -324,7 +347,7 @@ func (c *cfg) EncodePluginConfig(key string, configObj interface{}) error {
 }
 
 // Marshal implements config.Config
-func (c cfg) Marshal() ([]byte, error) {
+func (c cfg) MarshalYAML() ([]byte, error) {
 	for i, r := range c.Resources {
 		// If API is empty, omit it (prevents `api: {}`).
 		if r.API != nil && r.API.IsEmpty() {
@@ -345,7 +368,7 @@ func (c cfg) Marshal() ([]byte, error) {
 }
 
 // Unmarshal implements config.Config
-func (c *cfg) Unmarshal(b []byte) error {
+func (c *cfg) UnmarshalYAML(b []byte) error {
 	if err := yaml.UnmarshalStrict(b, c); err != nil {
 		return config.UnmarshalError{Err: err}
 	}