xrd.webhooks: add unit tests

Signed-off-by: Muvaffak Onus <me@muvaf.com>

apis/apiextensions/v1/xrd_types.go

@@ -36,6 +36,7 @@ type CompositeResourceDefinitionSpec struct {
 
 	// Names specifies the resource and kind names of the defined composite
 	// resource.
+	// +immutable
 	Names extv1.CustomResourceDefinitionNames `json:"names"`
 
 	// ClaimNames specifies the names of an optional composite resource claim.
@@ -46,6 +47,7 @@ type CompositeResourceDefinitionSpec struct {
 	// create, update, or delete a corresponding composite resource. You may add
 	// claim names to an existing CompositeResourceDefinition, but they cannot
 	// be changed or removed once they have been set.
+	// +immutable
 	// +optional
 	ClaimNames *extv1.CustomResourceDefinitionNames `json:"claimNames,omitempty"`
 

apis/apiextensions/v1/xrd_webhooks.go

@@ -23,7 +23,17 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/errors"
 )
 
-// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-apiextensions-crossplane-io-v1-compositeresourcedefinition,mutating=false,failurePolicy=fail,groups=apiextensions.crossplane.io,resources=compositeresourcedefinitions,versions=v1,name=compositeresourcedefinitions.apiextensions.crossplane.io,sideEffects=None,admissionReviewVersions=v1
+const (
+	errUnexpectedType = "unexpected type"
+
+	errGroupImmutable       = "spec.group is immutable"
+	errPluralImmutable      = "spec.names.plural is immutable"
+	errKindImmutable        = "spec.names.kind is immutable"
+	errClaimPluralImmutable = "spec.claimNames.plural is immutable"
+	errClaimKindImmutable   = "spec.claimNames.kind is immutable"
+)
+
+// +kubebuilder:webhook:verbs=update,path=/validate-apiextensions-crossplane-io-v1-compositeresourcedefinition,mutating=false,failurePolicy=fail,groups=apiextensions.crossplane.io,resources=compositeresourcedefinitions,versions=v1,name=compositeresourcedefinitions.apiextensions.crossplane.io,sideEffects=None,admissionReviewVersions=v1
 
 // ValidateCreate is run for creation actions.
 func (in *CompositeResourceDefinition) ValidateCreate() error {
@@ -34,10 +44,23 @@ func (in *CompositeResourceDefinition) ValidateCreate() error {
 func (in *CompositeResourceDefinition) ValidateUpdate(old runtime.Object) error {
 	oldObj, ok := old.(*CompositeResourceDefinition)
 	if !ok {
-		return errors.Errorf("unexpected type")
+		return errors.New(errUnexpectedType)
 	}
-	if in.Spec.Group != oldObj.Spec.Group {
-		return errors.Errorf("spec.group is immutable")
+	switch {
+	case in.Spec.Group != oldObj.Spec.Group:
+		return errors.New(errGroupImmutable)
+	case in.Spec.Names.Plural != oldObj.Spec.Names.Plural:
+		return errors.New(errPluralImmutable)
+	case in.Spec.Names.Kind != oldObj.Spec.Names.Kind:
+		return errors.New(errKindImmutable)
+	}
+	if in.Spec.ClaimNames != nil && oldObj.Spec.ClaimNames != nil {
+		switch {
+		case in.Spec.ClaimNames.Plural != oldObj.Spec.ClaimNames.Plural:
+			return errors.New(errClaimPluralImmutable)
+		case in.Spec.ClaimNames.Kind != oldObj.Spec.ClaimNames.Kind:
+			return errors.New(errClaimKindImmutable)
+		}
 	}
 	return nil
 }

apis/apiextensions/v1/xrd_webhooks_test.go

@@ -0,0 +1,165 @@
+/*
+Copyright 2022 The Crossplane 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 (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	"github.com/crossplane/crossplane-runtime/pkg/errors"
+	"github.com/crossplane/crossplane-runtime/pkg/test"
+)
+
+func TestValidateUpdate(t *testing.T) {
+	type args struct {
+		old runtime.Object
+		new *CompositeResourceDefinition
+	}
+	cases := map[string]struct {
+		args
+		err error
+	}{
+		"UnexpectedType": {
+			args: args{
+				old: &extv1.CustomResourceDefinition{},
+				new: &CompositeResourceDefinition{},
+			},
+			err: errors.New(errUnexpectedType),
+		},
+		"GroupChanged": {
+			args: args{
+				old: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						Group: "a",
+					},
+				},
+				new: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						Group: "b",
+					},
+				},
+			},
+			err: errors.New(errGroupImmutable),
+		},
+		"PluralChanged": {
+			args: args{
+				old: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						Names: extv1.CustomResourceDefinitionNames{
+							Plural: "b",
+						},
+					},
+				},
+				new: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						Names: extv1.CustomResourceDefinitionNames{
+							Plural: "a",
+						},
+					},
+				},
+			},
+			err: errors.New(errPluralImmutable),
+		},
+		"KindChanged": {
+			args: args{
+				old: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						Names: extv1.CustomResourceDefinitionNames{
+							Kind: "b",
+						},
+					},
+				},
+				new: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						Names: extv1.CustomResourceDefinitionNames{
+							Kind: "a",
+						},
+					},
+				},
+			},
+			err: errors.New(errKindImmutable),
+		},
+		"ClaimPluralChanged": {
+			args: args{
+				old: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						ClaimNames: &extv1.CustomResourceDefinitionNames{
+							Plural: "b",
+						},
+					},
+				},
+				new: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						ClaimNames: &extv1.CustomResourceDefinitionNames{
+							Plural: "a",
+						},
+					},
+				},
+			},
+			err: errors.New(errClaimPluralImmutable),
+		},
+		"ClaimKindChanged": {
+			args: args{
+				old: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						ClaimNames: &extv1.CustomResourceDefinitionNames{
+							Kind: "b",
+						},
+					},
+				},
+				new: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						ClaimNames: &extv1.CustomResourceDefinitionNames{
+							Kind: "a",
+						},
+					},
+				},
+			},
+			err: errors.New(errClaimKindImmutable),
+		},
+		"Success": {
+			args: args{
+				old: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						Names: extv1.CustomResourceDefinitionNames{
+							Kind: "a",
+						},
+					},
+				},
+				new: &CompositeResourceDefinition{
+					Spec: CompositeResourceDefinitionSpec{
+						Names: extv1.CustomResourceDefinitionNames{
+							Kind: "a",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			err := tc.new.ValidateUpdate(tc.old)
+			if diff := cmp.Diff(tc.err, err, test.EquateErrors()); diff != "" {
+				t.Errorf("ValidateUpdate(): -want, +got:\n%s", diff)
+			}
+		})
+	}
+}

cluster/crds/apiextensions.crossplane.io_compositeresourcedefinitions.yaml

@@ -1,4 +1,3 @@
----
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:

cluster/crds/apiextensions.crossplane.io_compositionrevisions.yaml

@@ -1,4 +1,3 @@
----
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:

cluster/crds/apiextensions.crossplane.io_compositions.yaml

@@ -1,4 +1,3 @@
----
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:

cluster/crds/pkg.crossplane.io_configurationrevisions.yaml

@@ -1,4 +1,3 @@
----
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:

cluster/crds/pkg.crossplane.io_configurations.yaml

@@ -1,4 +1,3 @@
----
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:

cluster/crds/pkg.crossplane.io_controllerconfigs.yaml

@@ -1,4 +1,3 @@
----
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:

cluster/crds/pkg.crossplane.io_locks.yaml

@@ -1,4 +1,3 @@
----
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:

cluster/crds/pkg.crossplane.io_providerrevisions.yaml

@@ -1,4 +1,3 @@
----
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:

cluster/crds/pkg.crossplane.io_providers.yaml

@@ -1,4 +1,3 @@
----
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:

cluster/crds/secrets.crossplane.io_storeconfigs.yaml

@@ -1,4 +1,3 @@
----
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:

cluster/webhookconfigurations/manifests.yaml

@@ -20,9 +20,7 @@ webhooks:
     apiVersions:
     - v1
     operations:
-    - CREATE
     - UPDATE
-    - DELETE
     resources:
     - compositeresourcedefinitions
   sideEffects: None

cmd/crossplane/core/core.go

@@ -77,7 +77,7 @@ type startCommand struct {
 }
 
 // Run core Crossplane controllers.
-func (c *startCommand) Run(s *runtime.Scheme, log logging.Logger) error {
+func (c *startCommand) Run(s *runtime.Scheme, log logging.Logger) error { //nolint:gocyclo
 	cfg, err := ctrl.GetConfig()
 	if err != nil {
 		return errors.Wrap(err, "Cannot get config")

cmd/crossplane/core/init.go

@@ -37,7 +37,6 @@ type initCommand struct {
 	Configurations []string `name:"configuration" help:"Pre-install a Configuration by giving its image URI. This argument can be repeated."`
 	Namespace      string   `short:"n" help:"Namespace used to set as default scope in default secret store config." default:"crossplane-system" env:"POD_NAMESPACE"`
 
-	InstallationNamespace   string `help:"The namespace that Crossplane is installed." env:"POD_NAMESPACE"`
 	WebhookTLSSecretName    string `help:"The name of the Secret that the initializer will fill with webhook TLS certificate bundle." env:"WEBHOOK_TLS_SECRET_NAME"`
 	WebhookServiceName      string `help:"The name of the Service object that the webhook service will be run." env:"WEBHOOK_SERVICE_NAME"`
 	WebhookServiceNamespace string `help:"The namespace of the Service object that the webhook service will be run." env:"WEBHOOK_SERVICE_NAMESPACE"`
@@ -59,7 +58,7 @@ func (c *initCommand) Run(s *runtime.Scheme, log logging.Logger) error {
 	if c.WebhookTLSSecretName != "" {
 		nn := types.NamespacedName{
 			Name:      c.WebhookTLSSecretName,
-			Namespace: c.InstallationNamespace,
+			Namespace: c.Namespace,
 		}
 		svc := admv1.ServiceReference{
 			Name:      c.WebhookServiceName,
@@ -67,7 +66,7 @@ func (c *initCommand) Run(s *runtime.Scheme, log logging.Logger) error {
 			Port:      &c.WebhookServicePort,
 		}
 		steps = append(steps,
-			initializer.NewWebhookCertificateGenerator(nn, c.InstallationNamespace, log),
+			initializer.NewWebhookCertificateGenerator(nn, c.Namespace, log),
 			initializer.NewCoreCRDs("/crds", s, initializer.WithWebhookTLSSecretRef(nn)),
 			initializer.NewWebhookConfigurations("/webhookconfigurations", s, nn, svc))
 	} else {

go.mod

@@ -133,6 +133,7 @@ require (
 	github.com/ryanuber/go-glob v1.0.0 // indirect
 	github.com/spf13/cobra v1.3.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/objx v0.2.0 // indirect
 	github.com/vbatts/tar-split v0.11.2 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
 	go.uber.org/multierr v1.6.0 // indirect