[feat] Add OBJKey validating webhook to check for non-empty secret da…

…ta format with cluster resourceset

api/v1alpha2/linodeobjectstoragekey_webhook.go

@@ -0,0 +1,83 @@
+/*
+Copyright 2023 Akamai Technologies, Inc.
+
+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 v1alpha2
+
+import (
+	"fmt"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	clusteraddonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
+	ctrl "sigs.k8s.io/controller-runtime"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+// log is for logging in this package.
+var linodeobjectstoragekeylog = logf.Log.WithName("linodeobjectstoragekey-resource")
+
+// SetupWebhookWithManager will setup the manager to manage the webhooks
+func (r *LinodeObjectStorageKey) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(r).
+		Complete()
+}
+
+// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
+//+kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1alpha2-linodeobjectstoragekey,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=linodeobjectstoragekeys,verbs=create;update,versions=v1alpha2,name=vlinodeobjectstoragekey.kb.io,admissionReviewVersions=v1
+
+var _ webhook.Validator = &LinodeObjectStorageKey{}
+
+// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
+func (r *LinodeObjectStorageKey) ValidateCreate() (admission.Warnings, error) {
+	linodeobjectstoragekeylog.Info("validate create", "name", r.Name)
+
+	return r.validateLinodeObjectStorageKey()
+}
+
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
+func (r *LinodeObjectStorageKey) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
+	linodeobjectstoragekeylog.Info("validate update", "name", r.Name)
+
+	return r.validateLinodeObjectStorageKey()
+}
+
+// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
+func (r *LinodeObjectStorageKey) ValidateDelete() (admission.Warnings, error) {
+	return nil, nil
+}
+
+func (r *LinodeObjectStorageKey) validateLinodeObjectStorageKey() (admission.Warnings, error) {
+	var errs field.ErrorList
+
+	if r.Spec.SecretType == clusteraddonsv1.ClusterResourceSetSecretType && len(r.Spec.SecretDataFormat) == 0 {
+		errs = append(errs, field.Invalid(
+			field.NewPath("spec").Child("secretDataFormat"),
+			r.Spec.SecretDataFormat,
+			fmt.Sprintf("must not be empty with Secret type %s", clusteraddonsv1.ClusterResourceSetSecretType),
+		))
+	}
+
+	if len(errs) > 0 {
+		return nil, apierrors.NewInvalid(schema.GroupKind{Group: "infrastructure.cluster.x-k8s.io", Kind: "LinodeObjectStorageKey"}, r.Name, errs)
+	}
+
+	return nil, nil
+}

api/v1alpha2/linodeobjectstoragekey_webhook_test.go

@@ -0,0 +1,86 @@
+/*
+Copyright 2023 Akamai Technologies, Inc.
+
+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 v1alpha2
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	clusteraddonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
+)
+
+func TestValidateLinodeObjectStorageKey(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		spec LinodeObjectStorageKeySpec
+		err  error
+	}{
+		{
+			name: "opaque",
+			spec: LinodeObjectStorageKeySpec{
+				SecretType: corev1.SecretTypeOpaque,
+			},
+			err: nil,
+		},
+		{
+			name: "resourceset with empty secret data format",
+			spec: LinodeObjectStorageKeySpec{
+				SecretType:       clusteraddonsv1.ClusterResourceSetSecretType,
+				SecretDataFormat: map[string]string{},
+			},
+			err: errors.New("must not be empty with Secret type"),
+		},
+		{
+			name: "valid resourceset",
+			spec: LinodeObjectStorageKeySpec{
+				SecretType: clusteraddonsv1.ClusterResourceSetSecretType,
+				SecretDataFormat: map[string]string{
+					"file.yaml": "kind: Secret",
+				},
+			},
+			err: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		testcase := tt
+
+		t.Run(testcase.name, func(t *testing.T) {
+			t.Parallel()
+
+			key := LinodeObjectStorageKey{
+				Spec: testcase.spec,
+			}
+
+			_, err := key.validateLinodeObjectStorageKey()
+			if err != nil {
+				if testcase.err == nil {
+					t.Fatal(err)
+				}
+				if errStr := testcase.err.Error(); !strings.Contains(err.Error(), errStr) {
+					t.Errorf("error did not contain substring '%s'", errStr)
+				}
+			} else if testcase.err != nil {
+				t.Fatal("expected an error")
+			}
+		})
+	}
+}

cloud/scope/object_storage_key.go

@@ -119,10 +119,6 @@ func (s *ObjectStorageKeyScope) GenerateKeySecret(ctx context.Context, key *lino
 	// If the desired secret is of ClusterResourceSet type, encapsulate the secret.
 	// Bucket details are retrieved from the first referenced LinodeObjectStorageBucket in the access key.
 	if s.Key.Spec.SecretType == clusteraddonsv1.ClusterResourceSetSecretType {
-		if len(s.Key.Spec.SecretDataFormat) == 0 {
-			return nil, fmt.Errorf("unable to generate %s; spec.secretDataFormat must specify resources", clusteraddonsv1.ClusterResourceSetSecretType)
-		}
-
 		// This should never run since the CRD has a validation marker to ensure bucketAccess has at least one item.
 		if len(s.Key.Spec.BucketAccess) == 0 {
 			return nil, fmt.Errorf("unable to generate %s; spec.bucketAccess must not be empty", clusteraddonsv1.ClusterResourceSetSecretType)

cloud/scope/object_storage_key_test.go

@@ -461,32 +461,6 @@ func TestGenerateKeySecret(t *testing.T) {
 			},
 			expectedErr: errors.New("unable to generate addons.cluster.x-k8s.io/resource-set; failed to get bucket: api err"),
 		},
-		{
-			name: "cluster resource-set with empty data format",
-			Key: &infrav1alpha2.LinodeObjectStorageKey{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-key",
-					Namespace: "test-namespace",
-				},
-				Spec: infrav1alpha2.LinodeObjectStorageKeySpec{
-					SecretType: clusteraddonsv1.ClusterResourceSetSecretType,
-				},
-			},
-			key: &linodego.ObjectStorageKey{
-				ID:        1,
-				Label:     "test-key",
-				AccessKey: "access_key",
-				SecretKey: "secret_key",
-				BucketAccess: &[]linodego.ObjectStorageKeyBucketAccess{
-					{
-						BucketName:  "bucket",
-						Region:      "region",
-						Permissions: "read_write",
-					},
-				},
-			},
-			expectedErr: errors.New("unable to generate addons.cluster.x-k8s.io/resource-set; spec.secretDataFormat must specify resources"),
-		},
 		{
 			name: "cluster resource-set with empty buckets",
 			Key: &infrav1alpha2.LinodeObjectStorageKey{

cmd/main.go

@@ -325,6 +325,10 @@ func setupWebhooks(mgr manager.Manager) {
 		setupLog.Error(err, "unable to create webhook", "webhook", "LinodePlacementGroup")
 		os.Exit(1)
 	}
+	if err = (&infrastructurev1alpha2.LinodeObjectStorageKey{}).SetupWebhookWithManager(mgr); err != nil {
+		setupLog.Error(err, "unable to create webhook", "webhook", "LinodeObjectStorageKey")
+		os.Exit(1)
+	}
 }
 
 func setupObservabillity(ctx context.Context) func() {

config/crd/patches/cainjection_in_linodeobjectstoragekeys.yaml

@@ -0,0 +1,7 @@
+# The following patch adds a directive for certmanager to inject CA into the CRD
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    cert-manager.io/inject-ca-from: CERTIFICATE_NAMESPACE/CERTIFICATE_NAME
+  name: linodeobjectstoragekeys.infrastructure.cluster.x-k8s.io

config/crd/patches/webhook_in_linodeobjectstoragekeys.yaml

@@ -0,0 +1,16 @@
+# The following patch enables a conversion webhook for the CRD
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: linodeobjectstoragekeys.infrastructure.cluster.x-k8s.io
+spec:
+  conversion:
+    strategy: Webhook
+    webhook:
+      clientConfig:
+        service:
+          namespace: system
+          name: webhook-service
+          path: /convert
+      conversionReviewVersions:
+      - v1

config/webhook/manifests.yaml

@@ -108,6 +108,26 @@ webhooks:
     resources:
     - linodevpcs
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-infrastructure-cluster-x-k8s-io-v1alpha2-linodeobjectstoragekey
+  failurePolicy: Fail
+  name: validation.linodeobjectstoragekey.infrastructure.cluster.x-k8s.io
+  rules:
+  - apiGroups:
+    - infrastructure.cluster.x-k8s.io
+    apiVersions:
+    - v1alpha2
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - linodeobjectstoragekeys
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   - v1alpha1

controller/linodeobjectstoragekey_controller.go

@@ -180,31 +180,26 @@ func (r *LinodeObjectStorageKeyReconciler) reconcileApply(ctx context.Context, k
 			Name:      *keyScope.Key.Status.SecretName,
 		}
 
-		var shouldRestoreSecret bool
 		if err := keyScope.Client.Get(ctx, key, secret); err != nil {
 			if apierrors.IsNotFound(err) {
-				shouldRestoreSecret = true
+				key, err := services.GetObjectStorageKey(ctx, keyScope)
+				if err != nil {
+					keyScope.Logger.Error(err, "Failed to restore access key for modified/deleted secret")
+					r.setFailure(keyScope, err)
+
+					return err
+				}
+
+				keyForSecret = key
+
+				r.Recorder.Event(keyScope.Key, corev1.EventTypeNormal, "KeyRetrieved", "Object storage key retrieved")
 			} else {
 				keyScope.Logger.Error(err, "Failed check for access key secret")
 				r.setFailure(keyScope, fmt.Errorf("failed check for access key secret: %w", err))
 
 				return err
 			}
 		}
-
-		if shouldRestoreSecret {
-			key, err := services.GetObjectStorageKey(ctx, keyScope)
-			if err != nil {
-				keyScope.Logger.Error(err, "Failed to restore access key for modified/deleted secret")
-				r.setFailure(keyScope, err)
-
-				return err
-			}
-
-			keyForSecret = key
-
-			r.Recorder.Event(keyScope.Key, corev1.EventTypeNormal, "KeyRetrieved", "Object storage key retrieved")
-		}
 	}
 
 	if keyForSecret != nil {