Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ (go/v3-alpha) default to v1 CRDs and webhooks #1644

Merged
merged 2 commits into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions docs/book/src/reference/makefile-helpers.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Makefile Helpers

By default, the projects are scaffolded with a `Makefile`. You can customize and update this file as please you. Here, you will find some helpers that can be useful.
By default, the projects are scaffolded with a `Makefile`. You can customize and update this file as please you. Here, you will find some helpers that can be useful.

## To debug with go-delve

Expand All @@ -14,28 +14,24 @@ run-delve: generate fmt vet manifests
dlv --listen=:2345 --headless=true --api-version=2 --accept-multiclient exec ./bin/manager
```

## To change the version of CRDs
## To change the version of CRDs

The tool generate the CRDs by using [controller-tools](https://github.com/kubernetes-sigs/controller-tools), see in the manifests target:
The `controller-gen` program (from [controller-tools](https://github.com/kubernetes-sigs/controller-tools))
generates CRDs for kubebuilder projects, wrapped in the following `make` rule:

```sh
# Generate manifests e.g. CRD, RBAC etc.
manifests: controller-gen
$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
```

In this way, update `CRD_OPTIONS` to define the version of the CRDs manifests which will be generated in the `config/crd/bases` directory:
`controller-gen` lets you specify what CRD API version to generate (either "v1", the default, or "v1beta1").
You can direct it to generate a specific version by adding `crd:crdVersions={<version>}` to your `CRD_OPTIONS`,
found at the top of your Makefile:

```sh
# Produce CRDs that work back to Kubernetes 1.11 (no version conversion)
CRD_OPTIONS ?= "crd:trivialVersions=true"
CRD_OPTIONS ?= "crd:crdVersions={v1beta1},trivialVersions=true,preserveUnknownFields=false"
```

| CRD_OPTIONS | API version |
|--- |--- |
| `"crd:trivialVersions=true"` | `apiextensions.k8s.io/v1beta1` |
| `"crd:crdVersions=v1"` | `apiextensions.k8s.io/v1` |

## To get all the manifests without deploying

By adding `make dry-run` you can get the patched manifests in the dry-run folder, unlike `make depĺoy` which runs `kustomize` and `kubectl apply`.
Expand Down
62 changes: 54 additions & 8 deletions pkg/model/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,19 @@ func (c Config) HasResource(target GVK) bool {
return false
}
estroz marked this conversation as resolved.
Show resolved Hide resolved

// AddResource appends the provided resource to the tracked ones
// It returns if the configuration was modified
func (c *Config) AddResource(gvk GVK) bool {
// No-op if the resource was already tracked, return false
if c.HasResource(gvk) {
return false
// UpdateResources either adds gvk to the tracked set or, if the resource already exists,
// updates the the equivalent resource in the set.
func (c *Config) UpdateResources(gvk GVK) {
Copy link
Member

@camilamacedo86 camilamacedo86 Oct 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 great. IMO it makes more sense to be void and update

// If the resource already exists, update it.
for i, r := range c.Resources {
if r.isEqualTo(gvk) {
c.Resources[i].merge(gvk)
return
}
}

// Append the resource to the tracked ones, return true
// The resource does not exist, append the resource to the tracked ones.
c.Resources = append(c.Resources, gvk)
return true
}

// HasGroup returns true if group is already tracked
Expand All @@ -113,11 +115,44 @@ func (c Config) HasGroup(group string) bool {
return false
}

// IsCRDVersionCompatible returns true if crdVersion can be added to the existing set of CRD versions.
func (c Config) IsCRDVersionCompatible(crdVersion string) bool {
return c.resourceAPIVersionCompatible("crd", crdVersion)
}

// IsWebhookVersionCompatible returns true if webhookVersion can be added to the existing set of Webhook versions.
func (c Config) IsWebhookVersionCompatible(webhookVersion string) bool {
return c.resourceAPIVersionCompatible("webhook", webhookVersion)
}

// resourceAPIVersionCompatible returns true if version can be added to the existing set of versions
// for a given verType.
func (c Config) resourceAPIVersionCompatible(verType, version string) bool {
for _, res := range c.Resources {
var currVersion string
switch verType {
case "crd":
currVersion = res.CRDVersion
case "webhook":
currVersion = res.WebhookVersion
}
if currVersion != "" && version != currVersion {
return false
}
}
return true
}

// GVK contains information about scaffolded resources
type GVK struct {
Group string `json:"group,omitempty"`
Version string `json:"version,omitempty"`
Kind string `json:"kind,omitempty"`

// CRDVersion holds the CustomResourceDefinition API version used for the GVK.
CRDVersion string `json:"crdVersion,omitempty"`
// WebhookVersion holds the {Validating,Mutating}WebhookConfiguration API version used for the GVK.
WebhookVersion string `json:"webhookVersion,omitempty"`
}

// isEqualTo compares it with another resource
Expand All @@ -127,6 +162,17 @@ func (r GVK) isEqualTo(other GVK) bool {
r.Kind == other.Kind
}

// merge combines fields of two GVKs that have matching group, version, and kind,
// favoring the receiver's values.
estroz marked this conversation as resolved.
Show resolved Hide resolved
estroz marked this conversation as resolved.
Show resolved Hide resolved
func (r *GVK) merge(other GVK) {
if r.CRDVersion == "" && other.CRDVersion != "" {
r.CRDVersion = other.CRDVersion
}
if r.WebhookVersion == "" && other.WebhookVersion != "" {
r.WebhookVersion = other.WebhookVersion
}
}

// Marshal returns the bytes of c.
func (c Config) Marshal() ([]byte, error) {
// Ignore extra fields at first.
Expand Down
95 changes: 94 additions & 1 deletion pkg/model/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import (
. "github.com/onsi/gomega"
)

var _ = Describe("Config", func() {
const v1beta1 = "v1beta1"

var _ = Describe("PluginConfig", func() {
// Test plugin config. Don't want to export this config, but need it to
// be accessible by test.
type PluginConfig struct {
Expand Down Expand Up @@ -146,3 +148,94 @@ var _ = Describe("Config", func() {
Expect(pluginConfig).To(Equal(expectedPluginConfig))
})
})

var _ = Describe("Resource Version Compatibility", func() {

var (
c *Config
gvk1, gvk2 GVK

defaultVersion = "v1"
)

BeforeEach(func() {
c = &Config{}
gvk1 = GVK{Group: "example", Version: "v1", Kind: "TestKind"}
gvk2 = GVK{Group: "example", Version: "v1", Kind: "TestKind2"}
})

Context("resourceAPIVersionCompatible", func() {
It("returns true for a list of empty resources", func() {
Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeTrue())
})
It("returns true for one resource with an empty version", func() {
c.Resources = []GVK{gvk1}
Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeTrue())
})
It("returns true for one resource with matching version", func() {
gvk1.CRDVersion = defaultVersion
c.Resources = []GVK{gvk1}
Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeTrue())
})
It("returns true for two resources with matching versions", func() {
gvk1.CRDVersion = defaultVersion
gvk2.CRDVersion = defaultVersion
c.Resources = []GVK{gvk1, gvk2}
Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeTrue())
})
It("returns false for one resource with a non-matching version", func() {
gvk1.CRDVersion = v1beta1
c.Resources = []GVK{gvk1}
Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeFalse())
})
It("returns false for two resources containing a non-matching version", func() {
gvk1.CRDVersion = v1beta1
gvk2.CRDVersion = defaultVersion
c.Resources = []GVK{gvk1, gvk2}
Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeFalse())
})

It("returns false for two resources containing a non-matching version (webhooks)", func() {
gvk1.WebhookVersion = v1beta1
gvk2.WebhookVersion = defaultVersion
c.Resources = []GVK{gvk1, gvk2}
Expect(c.resourceAPIVersionCompatible("webhook", defaultVersion)).To(BeFalse())
})
})
})

var _ = Describe("Config", func() {
var (
c *Config
gvk1, gvk2 GVK
)

BeforeEach(func() {
c = &Config{}
gvk1 = GVK{Group: "example", Version: "v1", Kind: "TestKind"}
gvk2 = GVK{Group: "example", Version: "v1", Kind: "TestKind2"}
})

Context("UpdateResource", func() {
It("Adds a non-existing resource", func() {
c.UpdateResources(gvk1)
Expect(c.Resources).To(Equal([]GVK{gvk1}))
// Update again to ensure idempotency.
c.UpdateResources(gvk1)
Expect(c.Resources).To(Equal([]GVK{gvk1}))
})
It("Updates an existing resource", func() {
c.UpdateResources(gvk1)
gvk := GVK{Group: gvk1.Group, Version: gvk1.Version, Kind: gvk1.Kind, CRDVersion: "v1"}
c.UpdateResources(gvk)
Expect(c.Resources).To(Equal([]GVK{gvk}))
})
It("Updates an existing resource with more than one resource present", func() {
c.UpdateResources(gvk1)
c.UpdateResources(gvk2)
gvk := GVK{Group: gvk1.Group, Version: gvk1.Version, Kind: gvk1.Kind, CRDVersion: "v1"}
c.UpdateResources(gvk)
Expect(c.Resources).To(Equal([]GVK{gvk, gvk2}))
})
})
})
27 changes: 24 additions & 3 deletions pkg/model/resource/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ type Options struct {

// Namespaced is true if the resource is namespaced.
Namespaced bool

// CRDVersion holds the CustomResourceDefinition API version used for the Options.
CRDVersion string
// WebhookVersion holds the {Validating,Mutating}WebhookConfiguration API version used for the Options.
WebhookVersion string
estroz marked this conversation as resolved.
Show resolved Hide resolved
}

// ValidateV2 verifies that V2 project has all the fields have valid values
Expand Down Expand Up @@ -183,6 +188,18 @@ func (opts *Options) Validate() error {
return fmt.Errorf("invalid Kind: %#v", validationErrors)
}

// Ensure apiVersions for k8s types are empty or valid.
for typ, apiVersion := range map[string]string{
"CRD": opts.CRDVersion,
"Webhook": opts.WebhookVersion,
} {
switch apiVersion {
case "", "v1", "v1beta1":
default:
return fmt.Errorf("%s version must be one of: v1, v1beta1", typ)
}
}

// TODO: validate plural strings if provided

return nil
Expand All @@ -191,9 +208,11 @@ func (opts *Options) Validate() error {
// 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{
Group: opts.Group,
Version: opts.Version,
Kind: opts.Kind,
Group: opts.Group,
Version: opts.Version,
Kind: opts.Kind,
CRDVersion: opts.CRDVersion,
WebhookVersion: opts.WebhookVersion,
}
}

Expand Down Expand Up @@ -269,5 +288,7 @@ func (opts *Options) newResource() *Resource {
Kind: opts.Kind,
Plural: plural,
ImportAlias: opts.safeImport(opts.Group + opts.Version),
CRDVersion: opts.CRDVersion,
WebhookVersion: opts.WebhookVersion,
}
}
13 changes: 10 additions & 3 deletions pkg/model/resource/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,21 @@ type Resource struct {

// Namespaced is true if the resource is namespaced.
Namespaced bool `json:"namespaced,omitempty"`

// CRDVersion holds the CustomResourceDefinition API version used for the Resource.
CRDVersion string `json:"crdVersion,omitempty"`
// WebhookVersion holds the {Validating,Mutating}WebhookConfiguration API version used for the Resource.
WebhookVersion string `json:"webhookVersion,omitempty"`
}

// GVK returns the group-version-kind information to check against tracked resources in the configuration file
func (r *Resource) GVK() config.GVK {
return config.GVK{
Group: r.Group,
Version: r.Version,
Kind: r.Kind,
Group: r.Group,
Version: r.Version,
Kind: r.Kind,
CRDVersion: r.CRDVersion,
WebhookVersion: r.WebhookVersion,
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/plugin/v2/scaffolds/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (s *apiScaffolder) newUniverse() *model.Universe {

func (s *apiScaffolder) scaffold() error {
if s.doResource {
s.config.AddResource(s.resource.GVK())
s.config.UpdateResources(s.resource.GVK())

if err := machinery.NewScaffold(s.plugins...).Execute(
s.newUniverse(),
Expand Down
21 changes: 17 additions & 4 deletions pkg/plugin/v3/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,15 @@ import (
"sigs.k8s.io/kubebuilder/v2/plugins/addon"
)

// KbDeclarativePatternVersion is the sigs.k8s.io/kubebuilder-declarative-pattern version
// (used only to gen api with --pattern=addon)
// TODO: remove this when a better solution for using addons is implemented.
const KbDeclarativePatternVersion = "1cbf859290cab81ae8e73fc5caebe792280175d1"
const (
// KbDeclarativePatternVersion is the sigs.k8s.io/kubebuilder-declarative-pattern version
// (used only to gen api with --pattern=addon)
// TODO: remove this when a better solution for using addons is implemented.
KbDeclarativePatternVersion = "1cbf859290cab81ae8e73fc5caebe792280175d1"

// defaultCRDVersion is the default CRD API version to scaffold.
defaultCRDVersion = "v1"
)

// DefaultMainPath is default file path of main.go
const DefaultMainPath = "main.go"
Expand Down Expand Up @@ -124,6 +129,8 @@ func (p *createAPISubcommand) BindFlags(fs *pflag.FlagSet) {
fs.StringVar(&p.resource.Group, "group", "", "resource Group")
fs.StringVar(&p.resource.Version, "version", "", "resource Version")
fs.BoolVar(&p.resource.Namespaced, "namespaced", true, "resource is namespaced")
fs.StringVar(&p.resource.CRDVersion, "crd-version", defaultCRDVersion,
"version of CustomResourceDefinition to scaffold. Options: [v1, v1beta1]")
camilamacedo86 marked this conversation as resolved.
Show resolved Hide resolved
}

func (p *createAPISubcommand) InjectConfig(c *config.Config) {
Expand Down Expand Up @@ -172,6 +179,12 @@ func (p *createAPISubcommand) Validate() error {
return fmt.Errorf("multiple groups are not allowed by default, " +
"to enable multi-group visit kubebuilder.io/migration/multi-group.html")
}

// Check CRDVersion against all other CRDVersions in p.config for compatibility.
if !p.config.IsCRDVersionCompatible(p.resource.CRDVersion) {
return fmt.Errorf("only one CRD version can be used for all resources, cannot add %q",
camilamacedo86 marked this conversation as resolved.
Show resolved Hide resolved
p.resource.CRDVersion)
}
}
estroz marked this conversation as resolved.
Show resolved Hide resolved

return nil
Expand Down
Loading