Skip to content

Commit be4c91a

Browse files
AdirioAdirio
committed
Merge branch 'default-plugins' into plugin-resolution
2 parents d51f244 + a5d281c commit be4c91a

File tree

8 files changed

+45
-68
lines changed

8 files changed

+45
-68
lines changed

‎cmd/main.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"log"
2121

2222
"sigs.k8s.io/kubebuilder/v3/pkg/cli"
23-
cfgv2 "sigs.k8s.io/kubebuilder/v3/pkg/config/v2"
2423
cfgv3 "sigs.k8s.io/kubebuilder/v3/pkg/config/v3"
2524
declarativev1 "sigs.k8s.io/kubebuilder/v3/pkg/plugins/declarative/v1"
2625
pluginv2 "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v2"
@@ -37,8 +36,7 @@ func main() {
3736
&pluginv3.Plugin{},
3837
&declarativev1.Plugin{},
3938
),
40-
cli.WithDefaultPlugins(cfgv2.Version, &pluginv2.Plugin{}),
41-
cli.WithDefaultPlugins(cfgv3.Version, &pluginv3.Plugin{}),
39+
cli.WithDefaultPlugins(&pluginv3.Plugin{}),
4240
cli.WithCompletion(),
4341
)
4442
if err != nil {

‎pkg/cli/cli.go

+16-5
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@ import (
2828

2929
"sigs.k8s.io/kubebuilder/v3/pkg/config"
3030
yamlstore "sigs.k8s.io/kubebuilder/v3/pkg/config/store/yaml"
31+
cfgv2 "sigs.k8s.io/kubebuilder/v3/pkg/config/v2"
3132
cfgv3 "sigs.k8s.io/kubebuilder/v3/pkg/config/v3"
3233
"sigs.k8s.io/kubebuilder/v3/pkg/plugin"
34+
pluginv2 "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v2"
3335
)
3436

3537
const (
@@ -76,7 +78,7 @@ type cli struct { //nolint:maligned
7678
// Default project version in case none is provided and a config file can't be found.
7779
defaultProjectVersion config.Version
7880
// Default plugins in case none is provided and a config file can't be found.
79-
defaultPlugins map[config.Version][]string
81+
defaultPlugins []string
8082
// Plugins registered in the cli.
8183
plugins map[string]plugin.Plugin
8284
// Commands injected by options.
@@ -135,7 +137,7 @@ func newCLI(opts ...Option) (*cli, error) {
135137
c := &cli{
136138
commandName: "kubebuilder",
137139
defaultProjectVersion: cfgv3.Version,
138-
defaultPlugins: make(map[config.Version][]string),
140+
defaultPlugins: make([]string, 0),
139141
plugins: make(map[string]plugin.Plugin),
140142
fs: afero.NewOsFs(),
141143
}
@@ -191,6 +193,12 @@ func (c *cli) getInfoFromFlags() (string, []string, error) {
191193
plugins[i] = strings.TrimSpace(key)
192194
}
193195

196+
// For backwards compatibility reasons, defining the project version flag and not defining
197+
// the plugins flag should be interpreted as if the plugins flag was set to go/v2.
198+
if projectVersion == "2" && len(plugins) == 0 {
199+
plugins = append(plugins, plugin.KeyFor(pluginv2.Plugin{}))
200+
}
201+
194202
return projectVersion, plugins, nil
195203
}
196204

@@ -213,6 +221,11 @@ func (c cli) getInfoFromConfigFile() (config.Version, []string, error) {
213221
// getInfoFromConfig obtains the project version and plugin keys from the project config.
214222
// It is extracted from getInfoFromConfigFile for testing purposes.
215223
func getInfoFromConfig(projectConfig config.Config) (config.Version, []string, error) {
224+
// Project v2 did not store the layout field, so we set it to the only available plugin
225+
if projectConfig.GetVersion().Compare(cfgv2.Version) == 0 {
226+
return cfgv2.Version, []string{plugin.KeyFor(pluginv2.Plugin{})}, nil
227+
}
228+
216229
// Split the comma-separated plugins
217230
var pluginSet []string
218231
if projectConfig.GetLayout() != "" {
@@ -268,9 +281,7 @@ func (c cli) resolveFlagsAndConfigFileConflicts(
268281
switch {
269282
// If they are both empty, use the default
270283
case isFlagPluginsEmpty && isCfgPluginsEmpty:
271-
if defaults, hasDefaults := c.defaultPlugins[projectVersion]; hasDefaults {
272-
plugins = defaults
273-
}
284+
plugins = c.defaultPlugins
274285
// If any is empty, choose the other
275286
case isCfgPluginsEmpty:
276287
plugins = flagPlugins

‎pkg/cli/cli_test.go

+11-24
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ var _ = Describe("CLI", func() {
109109
projectVersion, plugins, err = c.getInfoFromFlags()
110110
Expect(err).NotTo(HaveOccurred())
111111
Expect(projectVersion).To(Equal("2"))
112-
Expect(len(plugins)).To(Equal(0))
112+
Expect(plugins).To(Equal([]string{"go.kubebuilder.io/v2"}))
113113
})
114114
})
115115

@@ -198,14 +198,14 @@ var _ = Describe("CLI", func() {
198198
projectVersion, plugins, err = getInfoFromConfig(projectConfig)
199199
Expect(err).NotTo(HaveOccurred())
200200
Expect(projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0))
201-
Expect(len(plugins)).To(Equal(0))
201+
Expect(plugins).To(Equal([]string{"go.kubebuilder.io/v2"}))
202202
})
203203
})
204204

205205
When("having layout field", func() {
206206
It("should succeed", func() {
207207
projectConfig = cfgv3.New()
208-
Expect(projectConfig.SetLayout("go.kubebuilder.io/v2")).To(Succeed())
208+
Expect(projectConfig.SetLayout("go.kubebuilder.io/v3")).To(Succeed())
209209
projectVersion, plugins, err = getInfoFromConfig(projectConfig)
210210
Expect(err).NotTo(HaveOccurred())
211211
Expect(projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0))
@@ -412,10 +412,7 @@ var _ = Describe("CLI", func() {
412412
It("should succeed", func() {
413413
c = &cli{
414414
defaultProjectVersion: projectVersion1,
415-
defaultPlugins: map[config.Version][]string{
416-
projectVersion1: {pluginKey1},
417-
projectVersion2: {pluginKey2},
418-
},
415+
defaultPlugins: []string{pluginKey1},
419416
}
420417
_, plugins, err = c.resolveFlagsAndConfigFileConflicts(
421418
"",
@@ -464,9 +461,7 @@ var _ = Describe("CLI", func() {
464461
When("having default plugin keys set and from flags", func() {
465462
It("should succeed", func() {
466463
c = &cli{
467-
defaultPlugins: map[config.Version][]string{
468-
{}: {pluginKey1},
469-
},
464+
defaultPlugins: []string{pluginKey1},
470465
}
471466
_, plugins, err = c.resolveFlagsAndConfigFileConflicts(
472467
"",
@@ -483,9 +478,7 @@ var _ = Describe("CLI", func() {
483478
When("having default plugin keys set and from config file", func() {
484479
It("should succeed", func() {
485480
c = &cli{
486-
defaultPlugins: map[config.Version][]string{
487-
{}: {pluginKey1},
488-
},
481+
defaultPlugins: []string{pluginKey1},
489482
}
490483
_, plugins, err = c.resolveFlagsAndConfigFileConflicts(
491484
"",
@@ -529,9 +522,7 @@ var _ = Describe("CLI", func() {
529522
When("having three plugin keys sources", func() {
530523
It("should succeed if plugin keys from flags and config file are the same", func() {
531524
c = &cli{
532-
defaultPlugins: map[config.Version][]string{
533-
{}: {pluginKey1},
534-
},
525+
defaultPlugins: []string{pluginKey1},
535526
}
536527
_, plugins, err = c.resolveFlagsAndConfigFileConflicts(
537528
"",
@@ -546,9 +537,7 @@ var _ = Describe("CLI", func() {
546537

547538
It("should fail if plugin keys from flags and config file are different", func() {
548539
c = &cli{
549-
defaultPlugins: map[config.Version][]string{
550-
{}: {pluginKey1},
551-
},
540+
defaultPlugins: []string{pluginKey1},
552541
}
553542
_, _, err = c.resolveFlagsAndConfigFileConflicts(
554543
"",
@@ -583,10 +572,8 @@ var _ = Describe("CLI", func() {
583572
pluginKeys := []string{"go.kubebuilder.io/v2"}
584573
c := &cli{
585574
defaultProjectVersion: projectVersion,
586-
defaultPlugins: map[config.Version][]string{
587-
projectVersion: pluginKeys,
588-
},
589-
fs: afero.NewMemMapFs(),
575+
defaultPlugins: pluginKeys,
576+
fs: afero.NewMemMapFs(),
590577
}
591578
c.cmd = c.newRootCmd()
592579
Expect(c.getInfo()).To(Succeed())
@@ -747,7 +734,7 @@ var _ = Describe("CLI", func() {
747734

748735
c, err = New(
749736
WithDefaultProjectVersion(projectVersion),
750-
WithDefaultPlugins(projectVersion, deprecatedPlugin),
737+
WithDefaultPlugins(deprecatedPlugin),
751738
WithPlugins(deprecatedPlugin),
752739
)
753740
_ = w.Close()

‎pkg/cli/options.go

+3-9
Original file line numberDiff line numberDiff line change
@@ -57,22 +57,16 @@ func WithDefaultProjectVersion(version config.Version) Option {
5757
}
5858

5959
// WithDefaultPlugins is an Option that sets the cli's default plugins.
60-
func WithDefaultPlugins(projectVersion config.Version, plugins ...plugin.Plugin) Option {
60+
func WithDefaultPlugins(plugins ...plugin.Plugin) Option {
6161
return func(c *cli) error {
62-
if err := projectVersion.Validate(); err != nil {
63-
return fmt.Errorf("broken pre-set project version %q for default plugins: %v", projectVersion, err)
64-
}
6562
if len(plugins) == 0 {
66-
return fmt.Errorf("empty set of plugins provided for project version %q", projectVersion)
63+
return fmt.Errorf("empty set of plugins provided")
6764
}
6865
for _, p := range plugins {
6966
if err := plugin.Validate(p); err != nil {
7067
return fmt.Errorf("broken pre-set default plugin %q: %v", plugin.KeyFor(p), err)
7168
}
72-
if !plugin.SupportsVersion(p, projectVersion) {
73-
return fmt.Errorf("default plugin %q doesn't support version %q", plugin.KeyFor(p), projectVersion)
74-
}
75-
c.defaultPlugins[projectVersion] = append(c.defaultPlugins[projectVersion], plugin.KeyFor(p))
69+
c.defaultPlugins = append(c.defaultPlugins, plugin.KeyFor(p))
7670
}
7771
return nil
7872
}

‎pkg/cli/options_test.go

+7-21
Original file line numberDiff line numberDiff line change
@@ -99,57 +99,43 @@ var _ = Describe("CLI options", func() {
9999

100100
Context("WithDefaultPlugins", func() {
101101
It("should return a valid CLI", func() {
102-
c, err = newCLI(WithDefaultPlugins(projectVersion, p))
102+
c, err = newCLI(WithDefaultPlugins(p))
103103
Expect(err).NotTo(HaveOccurred())
104104
Expect(c).NotTo(BeNil())
105-
Expect(c.defaultPlugins).To(Equal(map[config.Version][]string{projectVersion: {plugin.KeyFor(p)}}))
106-
})
107-
108-
When("providing an invalid project version", func() {
109-
It("should return an error", func() {
110-
_, err = newCLI(WithDefaultPlugins(config.Version{}, p))
111-
Expect(err).To(HaveOccurred())
112-
})
105+
Expect(c.defaultPlugins).To(Equal([]string{plugin.KeyFor(p)}))
113106
})
114107

115108
When("providing an empty set of plugins", func() {
116109
It("should return an error", func() {
117-
_, err = newCLI(WithDefaultPlugins(projectVersion))
110+
_, err = newCLI(WithDefaultPlugins())
118111
Expect(err).To(HaveOccurred())
119112
})
120113
})
121114

122115
When("providing a plugin with an invalid name", func() {
123116
It("should return an error", func() {
124-
_, err = newCLI(WithDefaultPlugins(projectVersion, np1))
117+
_, err = newCLI(WithDefaultPlugins(np1))
125118
Expect(err).To(HaveOccurred())
126119
})
127120
})
128121

129122
When("providing a plugin with an invalid version", func() {
130123
It("should return an error", func() {
131-
_, err = newCLI(WithDefaultPlugins(projectVersion, np2))
124+
_, err = newCLI(WithDefaultPlugins(np2))
132125
Expect(err).To(HaveOccurred())
133126
})
134127
})
135128

136129
When("providing a plugin with an empty list of supported versions", func() {
137130
It("should return an error", func() {
138-
_, err = newCLI(WithDefaultPlugins(projectVersion, np3))
131+
_, err = newCLI(WithDefaultPlugins(np3))
139132
Expect(err).To(HaveOccurred())
140133
})
141134
})
142135

143136
When("providing a plugin with an invalid list of supported versions", func() {
144137
It("should return an error", func() {
145-
_, err = newCLI(WithDefaultPlugins(projectVersion, np4))
146-
Expect(err).To(HaveOccurred())
147-
})
148-
})
149-
150-
When("providing a default plugin for an unsupported project version", func() {
151-
It("should return an error", func() {
152-
_, err = newCLI(WithDefaultPlugins(config.Version{Number: 2}, p))
138+
_, err = newCLI(WithDefaultPlugins(np4))
153139
Expect(err).To(HaveOccurred())
154140
})
155141
})

‎pkg/cli/root.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ configuration please run:
7474

7575
str += fmt.Sprintf("\nDefault project version: %s\n", c.defaultProjectVersion)
7676

77-
if defaultPlugins, hasDefaultPlugins := c.defaultPlugins[c.defaultProjectVersion]; hasDefaultPlugins {
78-
str += fmt.Sprintf("Default plugin keys: %q\n", strings.Join(defaultPlugins, ","))
77+
if len(c.defaultPlugins) != 0 {
78+
str += fmt.Sprintf("Default plugin keys: %q\n", strings.Join(c.defaultPlugins, ","))
7979
}
8080

8181
str += fmt.Sprintf(`

‎test/e2e/v2/plugin_cluster_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ var _ = Describe("kubebuilder", func() {
6767
By("init v2 project")
6868
err := kbc.Init(
6969
"--project-version", "2",
70+
"--plugins", "go/v2",
7071
"--domain", kbc.Domain,
7172
"--fetch-deps=false")
7273
Expect(err).Should(Succeed())

‎test/testdata/generate.sh

+4-4
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,10 @@ function scaffold_test_project {
110110

111111
build_kb
112112

113-
# Project version 2 uses plugin go/v2 (default).
114-
scaffold_test_project project-v2 --project-version=2
115-
scaffold_test_project project-v2-multigroup --project-version=2
116-
scaffold_test_project project-v2-addon --project-version=3 --plugins="go/v2,declarative"
113+
# Project version 2 uses plugin go/v2.
114+
scaffold_test_project project-v2 --plugins=go/v2 --project-version=2
115+
scaffold_test_project project-v2-multigroup --plugins=go/v2 --project-version=2
116+
scaffold_test_project project-v2-addon --plugins="go/v2,declarative" --project-version=3
117117
# Project version 3 (default) uses plugin go/v3 (default).
118118
scaffold_test_project project-v3
119119
scaffold_test_project project-v3-multigroup

0 commit comments

Comments
 (0)