Skip to content

Commit

Permalink
Added support for empty group in kubebuilder api creation with v3
Browse files Browse the repository at this point in the history
  • Loading branch information
prafull01 committed Sep 17, 2020
1 parent 39d833c commit 5653d66
Show file tree
Hide file tree
Showing 29 changed files with 759 additions and 19 deletions.
6 changes: 5 additions & 1 deletion generate_testdata.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 ...'
Expand Down
71 changes: 65 additions & 6 deletions pkg/model/resource/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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

Expand All @@ -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.IsV3() {
// 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
Expand Down
11 changes: 8 additions & 3 deletions pkg/model/resource/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,15 @@ 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 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", func() {
Expand Down
23 changes: 23 additions & 0 deletions pkg/model/resource/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down Expand Up @@ -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"))
})
})
})
2 changes: 1 addition & 1 deletion pkg/plugin/v2/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/plugin/v3/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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 = `"..", ".."`
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/plugin/v3/scaffolds/internal/templates/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions testdata/project-v3-multigroup/PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,6 @@ resources:
- group: foo.policy
kind: HealthCheckPolicy
version: v1
- kind: Lakers
version: v1
version: 3-alpha
36 changes: 36 additions & 0 deletions testdata/project-v3-multigroup/apis/v1/groupversion_info.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
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 v1 contains API Schema definitions for the v1 API group
// +kubebuilder:object:generate=true
// +groupName=testproject.org
package v1

import (
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/scheme"
)

var (
// GroupVersion is group version used to register these objects
GroupVersion = schema.GroupVersion{Group: "testproject.org", Version: "v1"}

// SchemeBuilder is used to add go types to the GroupVersionKind scheme
SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion}

// AddToScheme adds the types in this group-version to the given scheme.
AddToScheme = SchemeBuilder.AddToScheme
)
64 changes: 64 additions & 0 deletions testdata/project-v3-multigroup/apis/v1/lakers_types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
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 v1

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.

// LakersSpec defines the desired state of Lakers
type LakersSpec struct {
// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
// Important: Run "make" to regenerate code after modifying this file

// Foo is an example field of Lakers. Edit Lakers_types.go to remove/update
Foo string `json:"foo,omitempty"`
}

// LakersStatus defines the observed state of Lakers
type LakersStatus struct {
// INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
// Important: Run "make" to regenerate code after modifying this file
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status

// Lakers is the Schema for the lakers API
type Lakers struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec LakersSpec `json:"spec,omitempty"`
Status LakersStatus `json:"status,omitempty"`
}

// +kubebuilder:object:root=true

// LakersList contains a list of Lakers
type LakersList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []Lakers `json:"items"`
}

func init() {
SchemeBuilder.Register(&Lakers{}, &LakersList{})
}
Loading

0 comments on commit 5653d66

Please sign in to comment.