From 106b498c46293789634c50379dc1a5511879b77a Mon Sep 17 00:00:00 2001 From: Brian Mendoza Date: Fri, 23 Aug 2024 15:15:01 -0400 Subject: [PATCH] [feat] Add OBJKey validating webhook to check for non-empty secret data format with cluster resourceset --- .../linodeobjectstoragekey_webhook.go | 83 ++++++++++++++++++ .../linodeobjectstoragekey_webhook_test.go | 86 +++++++++++++++++++ cloud/scope/object_storage_key.go | 4 - cloud/scope/object_storage_key_test.go | 26 ------ cmd/main.go | 4 + ...ainjection_in_linodeobjectstoragekeys.yaml | 7 ++ .../webhook_in_linodeobjectstoragekeys.yaml | 16 ++++ config/webhook/manifests.yaml | 20 +++++ .../linodeobjectstoragekey_controller.go | 27 +++--- 9 files changed, 227 insertions(+), 46 deletions(-) create mode 100644 api/v1alpha2/linodeobjectstoragekey_webhook.go create mode 100644 api/v1alpha2/linodeobjectstoragekey_webhook_test.go create mode 100644 config/crd/patches/cainjection_in_linodeobjectstoragekeys.yaml create mode 100644 config/crd/patches/webhook_in_linodeobjectstoragekeys.yaml diff --git a/api/v1alpha2/linodeobjectstoragekey_webhook.go b/api/v1alpha2/linodeobjectstoragekey_webhook.go new file mode 100644 index 000000000..a85cbe9f9 --- /dev/null +++ b/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 +} diff --git a/api/v1alpha2/linodeobjectstoragekey_webhook_test.go b/api/v1alpha2/linodeobjectstoragekey_webhook_test.go new file mode 100644 index 000000000..ac6f028b1 --- /dev/null +++ b/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") + } + }) + } +} diff --git a/cloud/scope/object_storage_key.go b/cloud/scope/object_storage_key.go index bb358a8f2..d0895ca0d 100644 --- a/cloud/scope/object_storage_key.go +++ b/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) diff --git a/cloud/scope/object_storage_key_test.go b/cloud/scope/object_storage_key_test.go index 9afa0deaa..a908a58df 100644 --- a/cloud/scope/object_storage_key_test.go +++ b/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{ diff --git a/cmd/main.go b/cmd/main.go index c0e70fbf3..9df7ace92 100644 --- a/cmd/main.go +++ b/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() { diff --git a/config/crd/patches/cainjection_in_linodeobjectstoragekeys.yaml b/config/crd/patches/cainjection_in_linodeobjectstoragekeys.yaml new file mode 100644 index 000000000..d407225ca --- /dev/null +++ b/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 diff --git a/config/crd/patches/webhook_in_linodeobjectstoragekeys.yaml b/config/crd/patches/webhook_in_linodeobjectstoragekeys.yaml new file mode 100644 index 000000000..8b4faf327 --- /dev/null +++ b/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 diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 5fa6fc077..276f29157 100644 --- a/config/webhook/manifests.yaml +++ b/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 diff --git a/controller/linodeobjectstoragekey_controller.go b/controller/linodeobjectstoragekey_controller.go index 09fd94ce5..24281c4d5 100644 --- a/controller/linodeobjectstoragekey_controller.go +++ b/controller/linodeobjectstoragekey_controller.go @@ -180,10 +180,19 @@ 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)) @@ -191,20 +200,6 @@ func (r *LinodeObjectStorageKeyReconciler) reconcileApply(ctx context.Context, k 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 {