Skip to content

Commit

Permalink
🐛 Envtest should enable conversion only for convertible GVKs
Browse files Browse the repository at this point in the history 
Signed-off-by: Vince Prignano <vincepri@vmware.com>
  • Loading branch information
@vincepri
vincepri committed May 19, 2021
1 parent d8dd31f commit fa5455e
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 4 deletions.
98 changes: 94 additions & 4 deletions pkg/envtest/crd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,31 @@ import (
"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
k8syaml "k8s.io/apimachinery/pkg/util/yaml"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"k8s.io/client-go/util/retry"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook/conversion"
"sigs.k8s.io/yaml"
)

// CRDInstallOptions are the options for installing CRDs
type CRDInstallOptions struct {
// Scheme is used to determine if conversion webhooks should be enabled
// for a particular CRD / object.
//
// Conversion webhooks are going to be enabled if an object in the scheme
// implements Hub and Spoke conversions.
//
// If nil, scheme.Scheme is used.
Scheme *runtime.Scheme

// Paths is a list of paths to the directories or files containing CRDs
Paths []string

Expand Down Expand Up @@ -86,7 +98,7 @@ func InstallCRDs(config *rest.Config, options CRDInstallOptions) ([]client.Objec
return nil, err
}

if err := modifyConversionWebhooks(options.CRDs, options.WebhookOptions); err != nil {
if err := modifyConversionWebhooks(options.CRDs, options.Scheme, options.WebhookOptions); err != nil {
return nil, err
}

Expand Down Expand Up @@ -118,6 +130,9 @@ func readCRDFiles(options *CRDInstallOptions) error {

// defaultCRDOptions sets the default values for CRDs
func defaultCRDOptions(o *CRDInstallOptions) {
if o.Scheme == nil {
o.Scheme = scheme.Scheme
}
if o.MaxTime == 0 {
o.MaxTime = defaultMaxWait
}
Expand Down Expand Up @@ -356,11 +371,23 @@ func renderCRDs(options *CRDInstallOptions) ([]client.Object, error) {
return res, nil
}

func modifyConversionWebhooks(crds []client.Object, webhookOptions WebhookInstallOptions) error {
func modifyConversionWebhooks(crds []client.Object, scheme *runtime.Scheme, webhookOptions WebhookInstallOptions) error {
if len(webhookOptions.LocalServingCAData) == 0 {
return nil
}

// Determine all registered convertible types.
convertibles := map[schema.GroupKind]struct{}{}
for gvk := range scheme.AllKnownTypes() {
obj, err := scheme.New(gvk)
if err != nil {
return err
}
if ok, err := conversion.IsConvertible(scheme, obj); ok && err == nil {
convertibles[gvk.GroupKind()] = struct{}{}
}
}

// generate host port.
hostPort, err := webhookOptions.generateHostPort()
if err != nil {
Expand All @@ -371,10 +398,19 @@ func modifyConversionWebhooks(crds []client.Object, webhookOptions WebhookInstal
for _, crd := range crds {
switch c := crd.(type) {
case *apiextensionsv1beta1.CustomResourceDefinition:
// Continue if we're preserving unknown fields.
//
// preserveUnknownFields defaults to true if `nil` in v1beta1.
if c.Spec.PreserveUnknownFields == nil || *c.Spec.PreserveUnknownFields {
continue
}
// Continue if the GroupKind isn't registered as being convertible.
if _, ok := convertibles[schema.GroupKind{
Group: c.Spec.Group,
Kind: c.Spec.Names.Kind,
}]; !ok {
continue
}
c.Spec.Conversion.Strategy = apiextensionsv1beta1.WebhookConverter
c.Spec.Conversion.WebhookClientConfig.Service = nil
c.Spec.Conversion.WebhookClientConfig = &apiextensionsv1beta1.WebhookClientConfig{
Expand All @@ -383,9 +419,17 @@ func modifyConversionWebhooks(crds []client.Object, webhookOptions WebhookInstal
CABundle: webhookOptions.LocalServingCAData,
}
case *apiextensionsv1.CustomResourceDefinition:
// Continue if we're preserving unknown fields.
if c.Spec.PreserveUnknownFields {
continue
}
// Continue if the GroupKind isn't registered as being convertible.
if _, ok := convertibles[schema.GroupKind{
Group: c.Spec.Group,
Kind: c.Spec.Names.Kind,
}]; !ok {
continue
}
c.Spec.Conversion.Strategy = apiextensionsv1.WebhookConverter
c.Spec.Conversion.Webhook.ClientConfig.Service = nil
c.Spec.Conversion.Webhook.ClientConfig = &apiextensionsv1.WebhookClientConfig{
Expand All @@ -401,8 +445,32 @@ func modifyConversionWebhooks(crds []client.Object, webhookOptions WebhookInstal

switch c.GroupVersionKind().Version {
case "v1beta1":
// Continue if we're preserving unknown fields.
//
// preserveUnknownFields defaults to true if `nil` in v1beta1.
if preserve, found, _ := unstructured.NestedBool(c.Object, "spec", "preserveUnknownFields"); preserve || !found {
if preserve, found, err := unstructured.NestedBool(c.Object, "spec", "preserveUnknownFields"); preserve || !found {
continue
} else if err != nil {
return err
}

// Continue if the GroupKind isn't registered as being convertible.
group, found, err := unstructured.NestedString(c.Object, "spec", "group")
if !found {
continue
} else if err != nil {
return err
}
kind, found, err := unstructured.NestedString(c.Object, "spec", "names", "kind")
if !found {
continue
} else if err != nil {
return err
}
if _, ok := convertibles[schema.GroupKind{
Group: group,
Kind: kind,
}]; !ok {
continue
}

Expand All @@ -428,7 +496,29 @@ func modifyConversionWebhooks(crds []client.Object, webhookOptions WebhookInstal
return err
}
case "v1":
if preserve, _, _ := unstructured.NestedBool(c.Object, "spec", "preserveUnknownFields"); preserve {
if preserve, _, err := unstructured.NestedBool(c.Object, "spec", "preserveUnknownFields"); preserve {
continue
} else if err != nil {
return err
}

// Continue if the GroupKind isn't registered as being convertible.
group, found, err := unstructured.NestedString(c.Object, "spec", "group")
if !found {
continue
} else if err != nil {
return err
}
kind, found, err := unstructured.NestedString(c.Object, "spec", "names", "kind")
if !found {
continue
} else if err != nil {
return err
}
if _, ok := convertibles[schema.GroupKind{
Group: group,
Kind: kind,
}]; !ok {
continue
}

Expand Down
16 changes: 16 additions & 0 deletions pkg/envtest/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
"strings"
"time"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/config"
Expand Down Expand Up @@ -95,6 +97,15 @@ type Environment struct {
// ControlPlane is the ControlPlane including the apiserver and etcd
ControlPlane integration.ControlPlane

// Scheme is used to determine if conversion webhooks should be enabled
// for a particular CRD / object.
//
// Conversion webhooks are going to be enabled if an object in the scheme
// implements Hub and Spoke conversions.
//
// If nil, scheme.Scheme is used.
Scheme *runtime.Scheme

// Config can be used to talk to the apiserver. It's automatically
// populated if not set using the standard controller-runtime config
// loading.
Expand Down Expand Up @@ -263,6 +274,11 @@ func (te *Environment) Start() (*rest.Config, error) {
}
}

// Set the default scheme if nil.
if te.Scheme == nil {
te.Scheme = scheme.Scheme
}

// Call PrepWithoutInstalling to setup certificates first
// and have them available to patch CRD conversion webhook as well.
if err := te.WebhookInstallOptions.PrepWithoutInstalling(); err != nil {
Expand Down

0 comments on commit fa5455e

Please sign in to comment.