Merge pull request #1679 from prafull01/empty-group

✨ Added support for empty group in kubebuilder api creation with v3+ #1404

generate_testdata.sh

@@ -79,7 +79,11 @@ scaffold_test_project() {
             $kb create api --group sea-creatures --version v1beta1 --kind Kraken --controller=true --resource=true --make=false
             $kb create api --group sea-creatures --version v1beta2 --kind Leviathan --controller=true --resource=true --make=false
             $kb create api --group foo.policy --version v1 --kind HealthCheckPolicy --controller=true --resource=true --make=false
-          elif [ $project == "project-v2-addon" ] || [ $project == "project-v3-addon" ]; then
+            if [ $project == "project-v3-multigroup" ]; then
+              $kb create api --version v1 --kind Lakers --controller=true --resource=true --make=false
+              $kb create webhook --version v1 --kind Lakers --defaulting --programmatic-validation
+            fi
+        elif [ $project == "project-v2-addon" ] || [ $project == "project-v3-addon" ]; then
             header_text 'enabling --pattern flag ...'
             export KUBEBUILDER_ENABLE_PLUGINS=1
             header_text 'Creating APIs ...'

pkg/model/resource/options.go

@@ -29,8 +29,7 @@ import (
 )
 
 const (
-	versionPattern = "^v\\d+(alpha\\d+|beta\\d+)?$"
-
+	versionPattern  = "^v\\d+(alpha\\d+|beta\\d+)?$"
 	groupRequired   = "group cannot be empty"
 	versionRequired = "version cannot be empty"
 	kindRequired    = "kind cannot be empty"
@@ -85,8 +84,8 @@ type Options struct {
 	Namespaced bool
 }
 
-// Validate verifies that all the fields have valid values
-func (opts *Options) Validate() error {
+// ValidateV2 verifies that V2 project has all the fields have valid values
+func (opts *Options) ValidateV2() error {
 	// Check that the required flags did not get a flag as their value
 	// We can safely look for a '-' as the first char as none of the fields accepts it
 	// NOTE: We must do this for all the required flags first or we may output the wrong
@@ -139,6 +138,56 @@ func (opts *Options) Validate() error {
 	return nil
 }
 
+// Validate verifies that all the fields have valid values
+func (opts *Options) Validate() error {
+	// Check that the required flags did not get a flag as their value
+	// We can safely look for a '-' as the first char as none of the fields accepts it
+	// NOTE: We must do this for all the required flags first or we may output the wrong
+	// error as flags may seem to be missing because Cobra assigned them to another flag.
+	if strings.HasPrefix(opts.Version, "-") {
+		return fmt.Errorf(versionRequired)
+	}
+	if strings.HasPrefix(opts.Kind, "-") {
+		return fmt.Errorf(kindRequired)
+	}
+	// Now we can check that all the required flags are not empty
+	if len(opts.Version) == 0 {
+		return fmt.Errorf(versionRequired)
+	}
+	if len(opts.Kind) == 0 {
+		return fmt.Errorf(kindRequired)
+	}
+
+	// Check if the Group has a valid DNS1123 subdomain value
+	if len(opts.Group) != 0 {
+		if err := validation.IsDNS1123Subdomain(opts.Group); err != nil {
+			return fmt.Errorf("group name is invalid: (%v)", err)
+		}
+	}
+
+	// Check if the version follows the valid pattern
+	if !versionRegex.MatchString(opts.Version) {
+		return fmt.Errorf("version must match %s (was %s)", versionPattern, opts.Version)
+	}
+
+	validationErrors := []string{}
+
+	// require Kind to start with an uppercase character
+	if string(opts.Kind[0]) == strings.ToLower(string(opts.Kind[0])) {
+		validationErrors = append(validationErrors, "kind must start with an uppercase character")
+	}
+
+	validationErrors = append(validationErrors, validation.IsDNS1035Label(strings.ToLower(opts.Kind))...)
+
+	if len(validationErrors) != 0 {
+		return fmt.Errorf("invalid Kind: %#v", validationErrors)
+	}
+
+	// TODO: validate plural strings if provided
+
+	return nil
+}
+
 // GVK returns the group-version-kind information to check against tracked resources in the configuration file
 func (opts *Options) GVK() config.GVK {
 	return config.GVK{
@@ -167,7 +216,11 @@ func (opts *Options) NewResource(c *config.Config, doResource bool) *Resource {
 
 	pkg := replacer.Replace(path.Join(c.Repo, "api", "%[version]"))
 	if c.MultiGroup {
-		pkg = replacer.Replace(path.Join(c.Repo, "apis", "%[group]", "%[version]"))
+		if opts.Group != "" {
+			pkg = replacer.Replace(path.Join(c.Repo, "apis", "%[group]", "%[version]"))
+		} else {
+			pkg = replacer.Replace(path.Join(c.Repo, "apis", "%[version]"))
+		}
 	}
 	domain := c.Domain
 
@@ -188,8 +241,14 @@ func (opts *Options) NewResource(c *config.Config, doResource bool) *Resource {
 
 	res.Package = pkg
 	res.Domain = opts.Group
-	if domain != "" {
+	if domain != "" && opts.Group != "" {
 		res.Domain += "." + domain
+	} else if opts.Group == "" && !c.IsV2() {
+		// Empty group overrides the default values provided by newResource().
+		// GroupPackageName and ImportAlias includes domain instead of group name as user provided group is empty.
+		res.Domain = domain
+		res.GroupPackageName = opts.safeImport(domain)
+		res.ImportAlias = opts.safeImport(domain + opts.Version)
 	}
 
 	return res

pkg/model/resource/options_test.go

@@ -17,10 +17,9 @@ var _ = Describe("Resource Options", func() {
 			Expect(options.Validate()).To(Succeed())
 		})
 
-		It("should fail if the Group is not specified", func() {
+		It("should succeed if the Group is not specified for V3", func() {
 			options := &Options{Version: "v1", Kind: "FirstMate"}
-			Expect(options.Validate()).NotTo(Succeed())
-			Expect(options.Validate().Error()).To(ContainSubstring("group cannot be empty"))
+			Expect(options.Validate()).To(Succeed())
 		})
 
 		It("should fail if the Group is not all lowercase", func() {
@@ -43,6 +42,7 @@ var _ = Describe("Resource Options", func() {
 			Expect(options.Validate().Error()).To(ContainSubstring("version cannot be empty"))
 		})
 
+		//nolint:dupl
 		It("should fail if the Version does not match the version format", func() {
 			options := &Options{Group: "crew", Version: "1", Kind: "FirstMate"}
 			Expect(options.Validate()).NotTo(Succeed())
@@ -110,4 +110,106 @@ var _ = Describe("Resource Options", func() {
 			Expect(err).To(MatchError(ContainSubstring("kind must start with an uppercase character")))
 		})
 	})
+
+	// We are duplicating the test cases for ValidateV2 with the Validate(). This test cases will be removed when
+	// the V2 will no longer be supported.
+	Describe("scaffolding an API for V2", func() {
+		It("should succeed if the Options is valid for V2", func() {
+			options := &Options{Group: "crew", Version: "v1", Kind: "FirstMate"}
+			Expect(options.ValidateV2()).To(Succeed())
+		})
+
+		It("should not succeed if the Group is not specified for V2", func() {
+			options := &Options{Version: "v1", Kind: "FirstMate"}
+			Expect(options.ValidateV2()).NotTo(Succeed())
+			Expect(options.ValidateV2().Error()).To(ContainSubstring("group cannot be empty"))
+		})
+
+		It("should fail if the Group is not all lowercase for V2", func() {
+			options := &Options{Group: "Crew", Version: "v1", Kind: "FirstMate"}
+			Expect(options.ValidateV2()).NotTo(Succeed())
+			Expect(options.ValidateV2().Error()).To(ContainSubstring("group name is invalid: " +
+				"([a DNS-1123 subdomain must consist of lower case alphanumeric characters"))
+		})
+
+		It("should fail if the Group contains non-alpha characters for V2", func() {
+			options := &Options{Group: "crew1*?", Version: "v1", Kind: "FirstMate"}
+			Expect(options.ValidateV2()).NotTo(Succeed())
+			Expect(options.ValidateV2().Error()).To(ContainSubstring("group name is invalid: " +
+				"([a DNS-1123 subdomain must consist of lower case alphanumeric characters"))
+		})
+
+		It("should fail if the Version is not specified for V2", func() {
+			options := &Options{Group: "crew", Kind: "FirstMate"}
+			Expect(options.ValidateV2()).NotTo(Succeed())
+			Expect(options.ValidateV2().Error()).To(ContainSubstring("version cannot be empty"))
+		})
+		//nolint:dupl
+		It("should fail if the Version does not match the version format for V2", func() {
+			options := &Options{Group: "crew", Version: "1", Kind: "FirstMate"}
+			Expect(options.ValidateV2()).NotTo(Succeed())
+			Expect(options.ValidateV2().Error()).To(ContainSubstring(
+				`version must match ^v\d+(alpha\d+|beta\d+)?$ (was 1)`))
+
+			options = &Options{Group: "crew", Version: "1beta1", Kind: "FirstMate"}
+			Expect(options.ValidateV2()).NotTo(Succeed())
+			Expect(options.ValidateV2().Error()).To(ContainSubstring(
+				`version must match ^v\d+(alpha\d+|beta\d+)?$ (was 1beta1)`))
+
+			options = &Options{Group: "crew", Version: "a1beta1", Kind: "FirstMate"}
+			Expect(options.ValidateV2()).NotTo(Succeed())
+			Expect(options.ValidateV2().Error()).To(ContainSubstring(
+				`version must match ^v\d+(alpha\d+|beta\d+)?$ (was a1beta1)`))
+
+			options = &Options{Group: "crew", Version: "v1beta", Kind: "FirstMate"}
+			Expect(options.ValidateV2()).NotTo(Succeed())
+			Expect(options.ValidateV2().Error()).To(ContainSubstring(
+				`version must match ^v\d+(alpha\d+|beta\d+)?$ (was v1beta)`))
+
+			options = &Options{Group: "crew", Version: "v1beta1alpha1", Kind: "FirstMate"}
+			Expect(options.ValidateV2()).NotTo(Succeed())
+			Expect(options.ValidateV2().Error()).To(ContainSubstring(
+				`version must match ^v\d+(alpha\d+|beta\d+)?$ (was v1beta1alpha1)`))
+		})
+
+		It("should fail if the Kind is not specified for V2", func() {
+			options := &Options{Group: "crew", Version: "v1"}
+			Expect(options.ValidateV2()).NotTo(Succeed())
+			Expect(options.ValidateV2().Error()).To(ContainSubstring("kind cannot be empty"))
+		})
+
+		DescribeTable("valid Kind values-according to core Kubernetes for V2",
+			func(kind string) {
+				options := &Options{Group: "crew", Kind: kind, Version: "v1"}
+				Expect(options.ValidateV2()).To(Succeed())
+			},
+			Entry("should pass validation if Kind is camelcase", "FirstMate"),
+			Entry("should pass validation if Kind has more than one caps at the start", "FIRSTMate"),
+		)
+
+		It("should fail if Kind is too long for V2", func() {
+			kind := strings.Repeat("a", 64)
+
+			options := &Options{Group: "crew", Kind: kind, Version: "v1"}
+			err := options.ValidateV2()
+			Expect(err).To(MatchError(ContainSubstring("must be no more than 63 characters")))
+		})
+
+		DescribeTable("invalid Kind values-according to core Kubernetes for V2",
+			func(kind string) {
+				options := &Options{Group: "crew", Kind: kind, Version: "v1"}
+				Expect(options.ValidateV2()).To(MatchError(
+					ContainSubstring("a DNS-1035 label must consist of lower case alphanumeric characters")))
+			},
+			Entry("should fail validation if Kind contains whitespaces", "Something withSpaces"),
+			Entry("should fail validation if Kind ends in -", "KindEndingIn-"),
+			Entry("should fail validation if Kind starts with number", "0ValidityKind"),
+		)
+
+		It("should fail if Kind starts with a lowercase character for V2", func() {
+			options := &Options{Group: "crew", Kind: "lOWERCASESTART", Version: "v1"}
+			err := options.ValidateV2()
+			Expect(err).To(MatchError(ContainSubstring("kind must start with an uppercase character")))
+		})
+	})
 })

pkg/model/resource/resource_test.go

@@ -146,6 +146,7 @@ var _ = Describe("Resource", func() {
 			Expect(options.Validate()).To(Succeed())
 
 			resource := options.NewResource(singleGroupConfig, true)
+
 			Expect(resource.Group).To(Equal(options.Group))
 			Expect(resource.GroupPackageName).To(Equal("myproject"))
 			Expect(resource.ImportAlias).To(Equal("myprojectv1"))
@@ -234,5 +235,27 @@ var _ = Describe("Resource", func() {
 			Expect(resource.Package).To(Equal(path.Join("k8s.io", "api", options.Group, options.Version)))
 			Expect(resource.Domain).To(Equal("authentication.k8s.io"))
 		})
+		It("should use domain if the group is empty for version v3", func() {
+			options := &Options{Version: "v1", Kind: "FirstMate"}
+			Expect(options.Validate()).To(Succeed())
+
+			resource := options.NewResource(
+				&config.Config{
+					Version: config.Version3Alpha,
+					Domain:  "test.io",
+					Repo:    "test",
+				},
+				true,
+			)
+			Expect(resource.Namespaced).To(Equal(options.Namespaced))
+			Expect(resource.Group).To(Equal(""))
+			Expect(resource.GroupPackageName).To(Equal("testio"))
+			Expect(resource.Version).To(Equal(options.Version))
+			Expect(resource.Kind).To(Equal(options.Kind))
+			Expect(resource.Plural).To(Equal("firstmates"))
+			Expect(resource.ImportAlias).To(Equal("testiov1"))
+			Expect(resource.Package).To(Equal(path.Join("test", "api", "v1")))
+			Expect(resource.Domain).To(Equal("test.io"))
+		})
 	})
 })

pkg/plugin/v2/api.go

@@ -127,7 +127,7 @@ func (p *createAPIPlugin) Run() error {
 }
 
 func (p *createAPIPlugin) Validate() error {
-	if err := p.resource.Validate(); err != nil {
+	if err := p.resource.ValidateV2(); err != nil {
 		return err
 	}
 

pkg/plugin/v3/api.go

@@ -137,6 +137,10 @@ func (p *createAPIPlugin) Validate() error {
 		return err
 	}
 
+	if p.resource.Group == "" && p.config.Domain == "" {
+		return fmt.Errorf("can not have group and domain both empty")
+	}
+
 	// TODO: re-evaluate whether y/n input still makes sense. We should probably always
 	// scaffold the resource and controller.
 	reader := bufio.NewReader(os.Stdin)

pkg/plugin/v3/scaffolds/internal/templates/config/api/group.go

@@ -36,7 +36,11 @@ type Group struct {
 func (f *Group) SetTemplateDefaults() error {
 	if f.Path == "" {
 		if f.MultiGroup {
-			f.Path = filepath.Join("apis", "%[group]", "%[version]", "groupversion_info.go")
+			if f.Resource.Group != "" {
+				f.Path = filepath.Join("apis", "%[group]", "%[version]", "groupversion_info.go")
+			} else {
+				f.Path = filepath.Join("apis", "%[version]", "groupversion_info.go")
+			}
 		} else {
 			f.Path = filepath.Join("api", "%[version]", "groupversion_info.go")
 		}

pkg/plugin/v3/scaffolds/internal/templates/config/api/types.go

@@ -37,7 +37,11 @@ type Types struct {
 func (f *Types) SetTemplateDefaults() error {
 	if f.Path == "" {
 		if f.MultiGroup {
-			f.Path = filepath.Join("apis", "%[group]", "%[version]", "%[kind]_types.go")
+			if f.Resource.Group != "" {
+				f.Path = filepath.Join("apis", "%[group]", "%[version]", "%[kind]_types.go")
+			} else {
+				f.Path = filepath.Join("apis", "%[version]", "%[kind]_types.go")
+			}
 		} else {
 			f.Path = filepath.Join("api", "%[version]", "%[kind]_types.go")
 		}

pkg/plugin/v3/scaffolds/internal/templates/config/api/webhook.go

@@ -46,7 +46,11 @@ type Webhook struct { // nolint:maligned
 func (f *Webhook) SetTemplateDefaults() error {
 	if f.Path == "" {
 		if f.MultiGroup {
-			f.Path = filepath.Join("apis", "%[group]", "%[version]", "%[kind]_webhook.go")
+			if f.Resource.Group != "" {
+				f.Path = filepath.Join("apis", "%[group]", "%[version]", "%[kind]_webhook.go")
+			} else {
+				f.Path = filepath.Join("apis", "%[version]", "%[kind]_webhook.go")
+			}
 		} else {
 			f.Path = filepath.Join("api", "%[version]", "%[kind]_webhook.go")
 		}

pkg/plugin/v3/scaffolds/internal/templates/config/controller/controller.go

@@ -36,7 +36,7 @@ type Controller struct {
 // SetTemplateDefaults implements input.Template
 func (f *Controller) SetTemplateDefaults() error {
 	if f.Path == "" {
-		if f.MultiGroup {
+		if f.MultiGroup && f.Resource.Group != "" {
 			f.Path = filepath.Join("controllers", "%[group]", "%[kind]_controller.go")
 		} else {
 			f.Path = filepath.Join("controllers", "%[kind]_controller.go")

pkg/plugin/v3/scaffolds/internal/templates/config/controller/controller_suitetest.go

@@ -41,7 +41,7 @@ type SuiteTest struct {
 // SetTemplateDefaults implements file.Template
 func (f *SuiteTest) SetTemplateDefaults() error {
 	if f.Path == "" {
-		if f.MultiGroup {
+		if f.MultiGroup && f.Resource.Group != "" {
 			f.Path = filepath.Join("controllers", "%[group]", "suite_test.go")
 		} else {
 			f.Path = filepath.Join("controllers", "suite_test.go")
@@ -57,7 +57,7 @@ func (f *SuiteTest) SetTemplateDefaults() error {
 	// If is multigroup the path needs to be ../../ since it has
 	// the group dir.
 	f.CRDDirectoryRelativePath = `".."`
-	if f.MultiGroup {
+	if f.MultiGroup && f.Resource.Group != "" {
 		f.CRDDirectoryRelativePath = `"..", ".."`
 	}
 

pkg/plugin/v3/scaffolds/internal/templates/main.go

@@ -150,7 +150,7 @@ func (f *MainUpdater) GetCodeFragments() file.CodeFragmentsMap {
 	imports := make([]string, 0)
 	imports = append(imports, fmt.Sprintf(apiImportCodeFragment, f.Resource.ImportAlias, f.Resource.Package))
 	if f.WireController {
-		if !f.MultiGroup {
+		if !f.MultiGroup || f.Resource.Group == "" {
 			imports = append(imports, fmt.Sprintf(controllerImportCodeFragment, f.Repo))
 		} else {
 			imports = append(imports, fmt.Sprintf(multiGroupControllerImportCodeFragment,
@@ -165,7 +165,7 @@ func (f *MainUpdater) GetCodeFragments() file.CodeFragmentsMap {
 	// Generate setup code fragments
 	setup := make([]string, 0)
 	if f.WireController {
-		if !f.MultiGroup {
+		if !f.MultiGroup || f.Resource.Group == "" {
 			setup = append(setup, fmt.Sprintf(reconcilerSetupCodeFragment,
 				f.Resource.Kind, f.Resource.Kind, f.Resource.Kind))
 		} else {

testdata/project-v3-multigroup/PROJECT

@@ -25,4 +25,6 @@ resources:
 - group: foo.policy
   kind: HealthCheckPolicy
   version: v1
+- kind: Lakers
+  version: v1
 version: 3-alpha