From f9e802efc7e5bafd8a0e2522693a6979d1f87c38 Mon Sep 17 00:00:00 2001 From: bcm820 Date: Fri, 23 Aug 2024 15:38:12 -0400 Subject: [PATCH] [feat] Support key secret templating (#476) --- api/v1alpha2/linodeobjectstoragekey_types.go | 9 + .../linodeobjectstoragekey_webhook.go | 83 +++++ .../linodeobjectstoragekey_webhook_test.go | 86 +++++ api/v1alpha2/zz_generated.deepcopy.go | 7 + cloud/scope/object_storage_key.go | 94 ++--- cloud/scope/object_storage_key_test.go | 348 ++++++------------ cmd/main.go | 4 + ...ster.x-k8s.io_linodeobjectstoragekeys.yaml | 15 + ...ainjection_in_linodeobjectstoragekeys.yaml | 7 + .../webhook_in_linodeobjectstoragekeys.yaml | 16 + config/webhook/manifests.yaml | 20 + .../linodeobjectstoragekey_controller.go | 37 +- .../linodeobjectstoragekey_controller_test.go | 180 +++++---- .../etcd-backup-restore/linode-obj.yaml | 15 +- 14 files changed, 519 insertions(+), 402 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_types.go b/api/v1alpha2/linodeobjectstoragekey_types.go index 83473120d..f02ef691a 100644 --- a/api/v1alpha2/linodeobjectstoragekey_types.go +++ b/api/v1alpha2/linodeobjectstoragekey_types.go @@ -52,8 +52,17 @@ type LinodeObjectStorageKeySpec struct { // SecretType instructs the controller what type of secret to generate containing access key details. // +kubebuilder:validation:Enum=Opaque;addons.cluster.x-k8s.io/resource-set // +kubebuilder:default=Opaque + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable" // +optional SecretType corev1.SecretType `json:"secretType,omitempty"` + + // SecretDataFormat instructs the controller how to format the data stored in the secret containing access key details. + // It supports Go template syntax and interpolating the following values: .AccessKey, .SecretKey. + // If no format is supplied then a generic one is used containing the values specified. + // When SecretType is set to addons.cluster.x-k8s.io/resource-set, a .BucketEndpoint value is also available pointing to the location of the first bucket specified in BucketAccess. + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable" + // +optional + SecretDataFormat map[string]string `json:"secretDataFormat,omitempty"` } // LinodeObjectStorageKeyStatus defines the observed state of LinodeObjectStorageKey diff --git a/api/v1alpha2/linodeobjectstoragekey_webhook.go b/api/v1alpha2/linodeobjectstoragekey_webhook.go new file mode 100644 index 000000000..89539cb7b --- /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=validation.linodeobjectstoragekey.infrastructure.cluster.x-k8s.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/api/v1alpha2/zz_generated.deepcopy.go b/api/v1alpha2/zz_generated.deepcopy.go index c16a8c698..747433c46 100644 --- a/api/v1alpha2/zz_generated.deepcopy.go +++ b/api/v1alpha2/zz_generated.deepcopy.go @@ -968,6 +968,13 @@ func (in *LinodeObjectStorageKeySpec) DeepCopyInto(out *LinodeObjectStorageKeySp *out = new(v1.SecretReference) **out = **in } + if in.SecretDataFormat != nil { + in, out := &in.SecretDataFormat, &out.SecretDataFormat + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LinodeObjectStorageKeySpec. diff --git a/cloud/scope/object_storage_key.go b/cloud/scope/object_storage_key.go index 201da7560..d0895ca0d 100644 --- a/cloud/scope/object_storage_key.go +++ b/cloud/scope/object_storage_key.go @@ -1,18 +1,18 @@ package scope import ( + "bytes" "context" "errors" "fmt" + "text/template" "github.com/go-logr/logr" "github.com/linode/linodego" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusteraddonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1" "sigs.k8s.io/cluster-api/util/patch" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2" @@ -99,27 +99,7 @@ func (s *ObjectStorageKeyScope) AddFinalizer(ctx context.Context) error { return nil } -const ( - accessKeySecretNameTemplate = "%s-obj-key" - - ClusterResourceSetSecretFilename = "etcd-backup.yaml" - BucketKeySecret = `kind: Secret -apiVersion: v1 -metadata: - name: %s - namespace: kube-system -stringData: - bucket_name: %s - bucket_region: %s - bucket_endpoint: %s - access_key: %s - secret_key: %s` -) - -var secretTypeExpectedKey = map[corev1.SecretType]string{ - corev1.SecretTypeOpaque: "access_key", - clusteraddonsv1.ClusterResourceSetSecretType: ClusterResourceSetSecretFilename, -} +const accessKeySecretNameTemplate = "%s-obj-key" // GenerateKeySecret returns a secret suitable for submission to the Kubernetes API. // The secret is expected to contain keys for accessing the bucket, as well as owner and controller references. @@ -128,9 +108,13 @@ func (s *ObjectStorageKeyScope) GenerateKeySecret(ctx context.Context, key *lino return nil, errors.New("expected non-nil object storage key") } - var secretStringData map[string]string - secretName := fmt.Sprintf(accessKeySecretNameTemplate, s.Key.Name) + secretStringData := make(map[string]string) + + tmplData := map[string]string{ + "AccessKey": key.AccessKey, + "SecretKey": key.SecretKey, + } // If the desired secret is of ClusterResourceSet type, encapsulate the secret. // Bucket details are retrieved from the first referenced LinodeObjectStorageBucket in the access key. @@ -146,24 +130,28 @@ func (s *ObjectStorageKeyScope) GenerateKeySecret(ctx context.Context, key *lino return nil, fmt.Errorf("unable to generate %s; failed to get bucket: %w", clusteraddonsv1.ClusterResourceSetSecretType, err) } - secretStringData = map[string]string{ - ClusterResourceSetSecretFilename: fmt.Sprintf( - BucketKeySecret, - secretName, - bucket.Label, - bucket.Region, - bucket.Hostname, - key.AccessKey, - key.SecretKey, - ), - } - } else { + tmplData["BucketEndpoint"] = bucket.Hostname + } else if len(s.Key.Spec.SecretDataFormat) == 0 { secretStringData = map[string]string{ "access_key": key.AccessKey, "secret_key": key.SecretKey, } } + for key, tmpl := range s.Key.Spec.SecretDataFormat { + goTmpl, err := template.New(key).Parse(tmpl) + if err != nil { + return nil, fmt.Errorf("unable to generate secret; failed to parse template in secret data format for key %s: %w", key, err) + } + + var output bytes.Buffer + if err := goTmpl.Execute(&output, tmplData); err != nil { + return nil, fmt.Errorf("unable to generate secret; failed to exec template in secret data format for key %s: %w", key, err) + } + + secretStringData[key] = output.String() + } + secret := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: secretName, @@ -192,35 +180,3 @@ func (s *ObjectStorageKeyScope) ShouldRotateKey() bool { return s.Key.Status.LastKeyGeneration != nil && s.Key.Spec.KeyGeneration != *s.Key.Status.LastKeyGeneration } - -func (s *ObjectStorageKeyScope) ShouldReconcileKeySecret(ctx context.Context) (bool, error) { - if s.Key.Status.SecretName == nil { - return false, nil - } - - secret := &corev1.Secret{} - key := client.ObjectKey{Namespace: s.Key.Namespace, Name: *s.Key.Status.SecretName} - err := s.Client.Get(ctx, key, secret) - if apierrors.IsNotFound(err) { - return true, nil - } - if err != nil { - return false, err - } - - // Identify an expected key in secret.Data for the desired secret type. - // If it is missing, we must recreate the secret since the secret.type field is immutable. - expectedKey, ok := secretTypeExpectedKey[s.Key.Spec.SecretType] - if !ok { - return false, errors.New("unsupported secret type configured in LinodeObjectStorageKey") - } - if _, ok := secret.Data[expectedKey]; !ok { - if err := s.Client.Delete(ctx, secret); err != nil { - return false, err - } - - return true, nil - } - - return false, nil -} diff --git a/cloud/scope/object_storage_key_test.go b/cloud/scope/object_storage_key_test.go index 12fedc5b1..a908a58df 100644 --- a/cloud/scope/object_storage_key_test.go +++ b/cloud/scope/object_storage_key_test.go @@ -12,10 +12,8 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" clusteraddonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1" @@ -304,31 +302,28 @@ func TestGenerateKeySecret(t *testing.T) { name string Key *infrav1alpha2.LinodeObjectStorageKey key *linodego.ObjectStorageKey - expectedErr error expectK8s func(*mock.MockK8sClient) expectLinode func(*mock.MockLinodeClient) + expectedData map[string]string + expectedErr error }{ { name: "opaque secret", Key: &infrav1alpha2.LinodeObjectStorageKey{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-bucket", + Name: "test-key", Namespace: "test-namespace", }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("test-bucket-obj-key"), - }, }, key: &linodego.ObjectStorageKey{ ID: 1, - Label: "read_write", - SecretKey: "read_write_key", - AccessKey: "read_write_access_key", - Limited: false, + Label: "test-key", + AccessKey: "access_key", + SecretKey: "secret_key", BucketAccess: &[]linodego.ObjectStorageKeyBucketAccess{ { BucketName: "bucket", - Region: "test-bucket", + Region: "region", Permissions: "read_write", }, }, @@ -340,39 +335,70 @@ func TestGenerateKeySecret(t *testing.T) { return s }).Times(1) }, + expectedData: map[string]string{ + "access_key": "access_key", + "secret_key": "secret_key", + }, expectedErr: nil, }, + { + name: "invalid template", + Key: &infrav1alpha2.LinodeObjectStorageKey{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-key", + Namespace: "test-namespace", + }, + Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ + SecretDataFormat: map[string]string{ + "key": "{{ .AccessKey", + }, + }, + }, + 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 secret; failed to parse template in secret data format for key"), + }, { name: "cluster resource-set", Key: &infrav1alpha2.LinodeObjectStorageKey{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-bucket", + Name: "test-key", Namespace: "test-namespace", }, Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ BucketAccess: []infrav1alpha2.BucketAccessRef{ { BucketName: "bucket", - Region: "test-bucket", + Region: "region", Permissions: "read_write", }, }, SecretType: clusteraddonsv1.ClusterResourceSetSecretType, - }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("test-bucket-obj-key"), + SecretDataFormat: map[string]string{ + "key": "{{ .AccessKey }},{{ .SecretKey }},{{ .BucketEndpoint }}", + }, }, }, key: &linodego.ObjectStorageKey{ ID: 1, - Label: "read_write", - SecretKey: "read_write_key", - AccessKey: "read_write_access_key", - Limited: false, + Label: "test-key", + AccessKey: "access_key", + SecretKey: "secret_key", BucketAccess: &[]linodego.ObjectStorageKeyBucketAccess{ { BucketName: "bucket", - Region: "test-bucket", + Region: "region", Permissions: "read_write", }, }, @@ -385,38 +411,79 @@ func TestGenerateKeySecret(t *testing.T) { }).Times(1) }, expectLinode: func(mck *mock.MockLinodeClient) { - mck.EXPECT().GetObjectStorageBucket(gomock.Any(), "test-bucket", "bucket").Return(&linodego.ObjectStorageBucket{ + mck.EXPECT().GetObjectStorageBucket(gomock.Any(), "region", "bucket").Return(&linodego.ObjectStorageBucket{ Label: "bucket", - Region: "us-ord", + Region: "region", Hostname: "hostname", }, nil) }, + expectedData: map[string]string{ + "key": "access_key,secret_key,hostname", + }, expectedErr: nil, }, { - name: "cluster resource-set with empty buckets", + name: "cluster resource-set get bucket fail", Key: &infrav1alpha2.LinodeObjectStorageKey{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-bucket", + Name: "test-key", Namespace: "test-namespace", }, Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ + BucketAccess: []infrav1alpha2.BucketAccessRef{ + { + BucketName: "bucket", + Region: "region", + Permissions: "read_write", + }, + }, SecretType: clusteraddonsv1.ClusterResourceSetSecretType, + SecretDataFormat: map[string]string{ + "key": "{{ .AccessKey }},{{ .SecretKey }},{{ .BucketEndpoint }}", + }, }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("test-bucket-obj-key"), + }, + key: &linodego.ObjectStorageKey{ + ID: 1, + Label: "test-key", + AccessKey: "access_key", + SecretKey: "secret_key", + BucketAccess: &[]linodego.ObjectStorageKeyBucketAccess{ + { + BucketName: "bucket", + Region: "region", + Permissions: "read_write", + }, + }, + }, + expectLinode: func(mck *mock.MockLinodeClient) { + mck.EXPECT().GetObjectStorageBucket(gomock.Any(), "region", "bucket").Return(nil, errors.New("api err")) + }, + 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 buckets", + Key: &infrav1alpha2.LinodeObjectStorageKey{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-key", + Namespace: "test-namespace", + }, + Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ + SecretType: clusteraddonsv1.ClusterResourceSetSecretType, + SecretDataFormat: map[string]string{ + "key": "{{ .AccessKey }},{{ .SecretKey }},{{ .BucketEndpoint }}", + }, }, }, key: &linodego.ObjectStorageKey{ ID: 1, - Label: "read_write", - SecretKey: "read_write_key", - AccessKey: "read_write_access_key", - Limited: false, + Label: "test-key", + AccessKey: "access_key", + SecretKey: "secret_key", BucketAccess: &[]linodego.ObjectStorageKeyBucketAccess{ { BucketName: "bucket", - Region: "test-bucket", + Region: "region", Permissions: "read_write", }, }, @@ -427,12 +494,9 @@ func TestGenerateKeySecret(t *testing.T) { name: "missing key", Key: &infrav1alpha2.LinodeObjectStorageKey{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-bucket", + Name: "test-key", Namespace: "test-namespace", }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("test-bucket-obj-key"), - }, }, expectedErr: errors.New("expected non-nil object storage key"), }, @@ -440,23 +504,19 @@ func TestGenerateKeySecret(t *testing.T) { name: "client scheme does not have infrav1alpha2", Key: &infrav1alpha2.LinodeObjectStorageKey{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-bucket", + Name: "test-key", Namespace: "test-namespace", }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("test-bucket-obj-key"), - }, }, key: &linodego.ObjectStorageKey{ ID: 1, - Label: "read_write", - SecretKey: "read_write_key", - AccessKey: "read_write_access_key", - Limited: false, + Label: "test-key", + AccessKey: "access_key", + SecretKey: "secret_key", BucketAccess: &[]linodego.ObjectStorageKeyBucketAccess{ { BucketName: "bucket", - Region: "test-bucket", + Region: "region", Permissions: "read_write", }, }, @@ -499,6 +559,7 @@ func TestGenerateKeySecret(t *testing.T) { t.Fatal(err) } + assert.Equal(t, testcase.expectedData, secret.StringData) assert.Equal(t, "LinodeObjectStorageKey", secret.OwnerReferences[0].Kind) assert.True(t, *secret.OwnerReferences[0].Controller) }) @@ -550,198 +611,3 @@ func TestShouldRotateKey(t *testing.T) { }, }).ShouldRotateKey()) } - -func TestShouldReconcileKeySecret(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - key *infrav1alpha2.LinodeObjectStorageKey - expects func(k8s *mock.MockK8sClient) - want bool - expectedErr error - }{ - { - name: "status has no secret name", - key: &infrav1alpha2.LinodeObjectStorageKey{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "ns", - }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: nil, - }, - }, - want: false, - }, - { - name: "secret has expected key", - key: &infrav1alpha2.LinodeObjectStorageKey{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "ns", - }, - Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ - SecretType: corev1.SecretTypeOpaque, - }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("secret"), - }, - }, - expects: func(k8s *mock.MockK8sClient) { - k8s.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj *corev1.Secret, opts ...client.GetOption) error { - *obj = corev1.Secret{ - Data: map[string][]byte{ - "access_key": {}, - }, - } - return nil - }).AnyTimes() - }, - want: false, - }, - { - name: "secret is missing expected key", - key: &infrav1alpha2.LinodeObjectStorageKey{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "ns", - }, - Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ - SecretType: corev1.SecretTypeOpaque, - }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("secret"), - }, - }, - expects: func(k8s *mock.MockK8sClient) { - k8s.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj *corev1.Secret, opts ...client.GetOption) error { - *obj = corev1.Secret{ - Data: map[string][]byte{ - "not_access_key": {}, - }, - } - return nil - }).AnyTimes() - k8s.EXPECT().Delete(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) - }, - want: true, - }, - { - name: "secret is missing", - key: &infrav1alpha2.LinodeObjectStorageKey{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "ns", - }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("secret"), - }, - }, - expects: func(k8s *mock.MockK8sClient) { - k8s.EXPECT(). - Get(gomock.Any(), client.ObjectKey{Namespace: "ns", Name: "secret"}, gomock.Any()). - Return(apierrors.NewNotFound(schema.GroupResource{Resource: "Secret"}, "secret")) - }, - want: true, - }, - { - name: "non-404 api error", - key: &infrav1alpha2.LinodeObjectStorageKey{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "ns", - }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("secret"), - }, - }, - expects: func(k8s *mock.MockK8sClient) { - k8s.EXPECT(). - Get(gomock.Any(), client.ObjectKey{Namespace: "ns", Name: "secret"}, gomock.Any()). - Return(errors.New("unexpected error")) - }, - want: false, - expectedErr: errors.New("unexpected error"), - }, - { - name: "unsupported secret type", - key: &infrav1alpha2.LinodeObjectStorageKey{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "ns", - }, - Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ - SecretType: "unsupported secret type", - }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("secret"), - }, - }, - expects: func(k8s *mock.MockK8sClient) { - k8s.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj *corev1.Secret, opts ...client.GetOption) error { - *obj = corev1.Secret{ - Data: map[string][]byte{ - "not_access_key": {}, - }, - } - return nil - }).AnyTimes() - }, - want: false, - expectedErr: errors.New("unsupported secret type configured in LinodeObjectStorageKey"), - }, - { - name: "failed delete", - key: &infrav1alpha2.LinodeObjectStorageKey{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "ns", - }, - Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ - SecretType: corev1.SecretTypeOpaque, - }, - Status: infrav1alpha2.LinodeObjectStorageKeyStatus{ - SecretName: ptr.To("secret"), - }, - }, - expects: func(k8s *mock.MockK8sClient) { - k8s.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj *corev1.Secret, opts ...client.GetOption) error { - *obj = corev1.Secret{ - Data: map[string][]byte{ - "not_access_key": {}, - }, - } - return nil - }).AnyTimes() - k8s.EXPECT().Delete(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("failed delete")) - }, - want: false, - expectedErr: errors.New("failed delete"), - }, - } - for _, tt := range tests { - testcase := tt - t.Run(testcase.name, func(t *testing.T) { - t.Parallel() - - var mockClient *mock.MockK8sClient - if testcase.expects != nil { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - mockClient = mock.NewMockK8sClient(ctrl) - testcase.expects(mockClient) - } - - keyScope := &ObjectStorageKeyScope{ - Client: mockClient, - Key: testcase.key, - } - - restore, err := keyScope.ShouldReconcileKeySecret(context.TODO()) - if testcase.expectedErr != nil { - require.ErrorContains(t, err, testcase.expectedErr.Error()) - } - - assert.Equal(t, testcase.want, restore) - }) - } -} 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/bases/infrastructure.cluster.x-k8s.io_linodeobjectstoragekeys.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeobjectstoragekeys.yaml index b7851fcbd..53b010214 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeobjectstoragekeys.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeobjectstoragekeys.yaml @@ -97,6 +97,18 @@ spec: description: KeyGeneration may be modified to trigger a rotation of the access key. type: integer + secretDataFormat: + additionalProperties: + type: string + description: |- + SecretDataFormat instructs the controller how to format the data stored in the secret containing access key details. + It supports Go template syntax and interpolating the following values: .AccessKey, .SecretKey. + If no format is supplied then a generic one is used containing the values specified. + When SecretType is set to addons.cluster.x-k8s.io/resource-set, a .BucketEndpoint value is also available pointing to the location of the first bucket specified in BucketAccess. + type: object + x-kubernetes-validations: + - message: Value is immutable + rule: self == oldSelf secretType: default: Opaque description: SecretType instructs the controller what type of secret @@ -105,6 +117,9 @@ spec: - Opaque - addons.cluster.x-k8s.io/resource-set type: string + x-kubernetes-validations: + - message: Value is immutable + rule: self == oldSelf required: - bucketAccess - keyGeneration 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..14d2b934c 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -67,6 +67,26 @@ webhooks: resources: - linodeobjectstoragebuckets 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 - v1alpha2 diff --git a/controller/linodeobjectstoragekey_controller.go b/controller/linodeobjectstoragekey_controller.go index cca2c8482..24281c4d5 100644 --- a/controller/linodeobjectstoragekey_controller.go +++ b/controller/linodeobjectstoragekey_controller.go @@ -173,27 +173,32 @@ func (r *LinodeObjectStorageKeyReconciler) reconcileApply(ctx context.Context, k r.Recorder.Event(keyScope.Key, corev1.EventTypeNormal, "KeyAssigned", "Object storage key assigned") // Ensure the generated secret still exists - case keyScope.Key.Status.AccessKeyRef != nil: - ok, err := keyScope.ShouldReconcileKeySecret(ctx) - if err != nil { - keyScope.Logger.Error(err, "Failed check for access key secret") - r.setFailure(keyScope, err) - - return err + case keyScope.Key.Status.AccessKeyRef != nil && keyScope.Key.Status.SecretName != nil: + secret := &corev1.Secret{} + key := client.ObjectKey{ + Namespace: keyScope.Key.Namespace, + Name: *keyScope.Key.Status.SecretName, } - if ok { - 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) + if err := keyScope.Client.Get(ctx, key, secret); err != nil { + if apierrors.IsNotFound(err) { + 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 - } + return err + } + + keyForSecret = key - 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)) - r.Recorder.Event(keyScope.Key, corev1.EventTypeNormal, "KeyRetrieved", "Object storage key retrieved") + return err + } } } diff --git a/controller/linodeobjectstoragekey_controller_test.go b/controller/linodeobjectstoragekey_controller_test.go index b4867ea1e..ebecea9ac 100644 --- a/controller/linodeobjectstoragekey_controller_test.go +++ b/controller/linodeobjectstoragekey_controller_test.go @@ -19,7 +19,6 @@ package controller import ( "context" "errors" - "fmt" "github.com/linode/linodego" "go.uber.org/mock/gomock" @@ -45,7 +44,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("lifecycle", Ordered, Label("key", "lifecycle"), func() { +var _ = Describe("lifecycle", Ordered, Label("key", "key-lifecycle"), func() { suite := NewControllerSuite(GinkgoT(), mock.MockLinodeClient{}) key := infrav1.LinodeObjectStorageKey{ @@ -250,80 +249,10 @@ var _ = Describe("lifecycle", Ordered, Label("key", "lifecycle"), func() { }), ), ), - Once("secretType set to cluster resource set", func(ctx context.Context, _ Mock) { + Once("secretType set to cluster resource set fails", func(ctx context.Context, _ Mock) { key.Spec.SecretType = clusteraddonsv1.ClusterResourceSetSecretType - Expect(k8sClient.Update(ctx, &key)).To(Succeed()) + Expect(k8sClient.Update(ctx, &key)).NotTo(Succeed()) }), - OneOf( - Path( - Call("(secretType set to cluster resource set) > key is not retrieved", func(ctx context.Context, mck Mock) { - mck.LinodeClient.EXPECT().GetObjectStorageKey(gomock.Any(), 2).Return(nil, errors.New("get key error")) - }), - Result("error", func(ctx context.Context, mck Mock) { - keyScope.LinodeClient = mck.LinodeClient - _, err := reconciler.reconcile(ctx, &keyScope) - Expect(err.Error()).To(ContainSubstring("get key error")) - }), - ), - Path( - Call("(secretType set to cluster resource set) > key is retrieved", func(ctx context.Context, mck Mock) { - mck.LinodeClient.EXPECT().GetObjectStorageKey(gomock.Any(), 2). - Return(&linodego.ObjectStorageKey{ - ID: 2, - AccessKey: "access-key-2", - SecretKey: "secret-key-2", - }, nil) - }), - OneOf( - Path( - Call("bucket is not retrieved", func(ctx context.Context, mck Mock) { - mck.LinodeClient.EXPECT().GetObjectStorageBucket(gomock.Any(), "us-ord", "mybucket").Return(nil, errors.New("get bucket error")) - }), - Result("error", func(ctx context.Context, mck Mock) { - keyScope.LinodeClient = mck.LinodeClient - _, err := reconciler.reconcile(ctx, &keyScope) - Expect(err.Error()).To(ContainSubstring("get bucket error")) - }), - ), - Path( - Call("bucket is retrieved", func(ctx context.Context, mck Mock) { - mck.LinodeClient.EXPECT().GetObjectStorageBucket(gomock.Any(), "us-ord", "mybucket").Return(&linodego.ObjectStorageBucket{ - Label: "mybucket", - Region: "us-ord", - Hostname: "mybucket.us-ord-1.linodeobjects.com", - }, nil) - }), - Result("secret is recreated as cluster resource set type", func(ctx context.Context, mck Mock) { - keyScope.LinodeClient = mck.LinodeClient - _, err := reconciler.reconcile(ctx, &keyScope) - Expect(err).NotTo(HaveOccurred()) - - var secret corev1.Secret - secretKey := client.ObjectKey{Namespace: "default", Name: *key.Status.SecretName} - Expect(k8sClient.Get(ctx, secretKey, &secret)).To(Succeed()) - Expect(secret.Data).To(HaveLen(1)) - Expect(string(secret.Data[scope.ClusterResourceSetSecretFilename])).To(Equal(fmt.Sprintf(scope.BucketKeySecret, - *key.Status.SecretName, - "mybucket", - "us-ord", - "mybucket.us-ord-1.linodeobjects.com", - "access-key-2", - "secret-key-2", - ))) - - events := mck.Events() - Expect(events).To(ContainSubstring("Object storage key retrieved")) - Expect(events).To(ContainSubstring("Object storage key stored in secret")) - Expect(events).To(ContainSubstring("Object storage key synced")) - - logOutput := mck.Logs() - Expect(logOutput).To(ContainSubstring("Reconciling apply")) - Expect(logOutput).To(ContainSubstring("Secret %s was created with access key", *key.Status.SecretName)) - }), - ), - ), - ), - ), Once("resource is deleted", func(ctx context.Context, _ Mock) { // nb: client.Delete does not set DeletionTimestamp on the object, so re-fetch from the apiserver. objectKey := client.ObjectKeyFromObject(&key) @@ -364,7 +293,108 @@ var _ = Describe("lifecycle", Ordered, Label("key", "lifecycle"), func() { ) }) -var _ = Describe("errors", Label("key", "errors"), func() { +var _ = Describe("secret-template", Label("key", "key-secret-template"), func() { + suite := NewControllerSuite(GinkgoT(), mock.MockLinodeClient{}) + + reconciler := LinodeObjectStorageKeyReconciler{} + keyScope := scope.ObjectStorageKeyScope{} + + suite.BeforeEach(func(_ context.Context, mck Mock) { + reconciler.Recorder = mck.Recorder() + keyScope.Logger = mck.Logger() + + keyScope.Client = k8sClient + keyScope.Key = &infrav1.LinodeObjectStorageKey{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + }, + Spec: infrav1.LinodeObjectStorageKeySpec{ + BucketAccess: []infrav1.BucketAccessRef{ + { + BucketName: "mybucket", + Permissions: "read_only", + Region: "us-ord", + }, + }, + }, + } + }) + + suite.Run( + Call("key created", func(ctx context.Context, mck Mock) { + mck.LinodeClient.EXPECT().CreateObjectStorageKey(gomock.Any(), gomock.Any()).Return(&linodego.ObjectStorageKey{ + ID: 1, + AccessKey: "access-key", + SecretKey: "secret-key", + BucketAccess: &[]linodego.ObjectStorageKeyBucketAccess{ + { + BucketName: "mybucket", + Permissions: "read_only", + Region: "us-ord", + }, + }, + }, nil) + }), + OneOf( + Path( + Call("with opaque secret", func(ctx context.Context, mck Mock) { + keyScope.LinodeClient = mck.LinodeClient + keyScope.Key.ObjectMeta.Name = "opaque" + keyScope.Key.Spec.SecretType = corev1.SecretTypeOpaque + keyScope.Key.Spec.SecretDataFormat = map[string]string{ + "data": "{{ .AccessKey }}-{{ .SecretKey }}", + } + + Expect(k8sClient.Create(ctx, keyScope.Key)).To(Succeed()) + patchHelper, err := patch.NewHelper(keyScope.Key, k8sClient) + Expect(err).NotTo(HaveOccurred()) + keyScope.PatchHelper = patchHelper + }), + Result("generates opaque secret with templated data", func(ctx context.Context, mck Mock) { + _, err := reconciler.reconcile(ctx, &keyScope) + Expect(err).NotTo(HaveOccurred()) + + var secret corev1.Secret + secretKey := client.ObjectKey{Namespace: "default", Name: "opaque-obj-key"} + Expect(k8sClient.Get(ctx, secretKey, &secret)).To(Succeed()) + Expect(secret.Data).To(HaveLen(1)) + Expect(string(secret.Data["data"])).To(Equal("access-key-secret-key")) + }), + ), + Path( + Call("with cluster-resource-set secret", func(ctx context.Context, mck Mock) { + keyScope.LinodeClient = mck.LinodeClient + keyScope.Key.ObjectMeta.Name = "cluster-resource-set" + keyScope.Key.Spec.SecretType = clusteraddonsv1.ClusterResourceSetSecretType + keyScope.Key.Spec.SecretDataFormat = map[string]string{ + "data": "{{ .AccessKey }}-{{ .SecretKey }}-{{ .BucketEndpoint }}", + } + + Expect(k8sClient.Create(ctx, keyScope.Key)).To(Succeed()) + patchHelper, err := patch.NewHelper(keyScope.Key, k8sClient) + Expect(err).NotTo(HaveOccurred()) + keyScope.PatchHelper = patchHelper + + mck.LinodeClient.EXPECT().GetObjectStorageBucket(gomock.Any(), "us-ord", "mybucket").Return(&linodego.ObjectStorageBucket{ + Hostname: "hostname", + }, nil) + }), + Result("generates cluster-resource-set secret with templated data", func(ctx context.Context, mck Mock) { + _, err := reconciler.reconcile(ctx, &keyScope) + Expect(err).NotTo(HaveOccurred()) + + var secret corev1.Secret + secretKey := client.ObjectKey{Namespace: "default", Name: "cluster-resource-set-obj-key"} + Expect(k8sClient.Get(ctx, secretKey, &secret)).To(Succeed()) + Expect(secret.Data).To(HaveLen(1)) + Expect(string(secret.Data["data"])).To(Equal("access-key-secret-key-hostname")) + }), + ), + ), + ) +}) + +var _ = Describe("errors", Label("key", "key-errors"), func() { suite := NewControllerSuite( GinkgoT(), mock.MockLinodeClient{}, diff --git a/templates/addons/etcd-backup-restore/linode-obj.yaml b/templates/addons/etcd-backup-restore/linode-obj.yaml index 9a6107f8c..bed9918bf 100644 --- a/templates/addons/etcd-backup-restore/linode-obj.yaml +++ b/templates/addons/etcd-backup-restore/linode-obj.yaml @@ -25,11 +25,24 @@ metadata: cluster.x-k8s.io/cluster-name: ${CLUSTER_NAME} name: ${CLUSTER_NAME}-etcd-backup spec: - secretType: addons.cluster.x-k8s.io/resource-set bucketAccess: - bucketName: ${CLUSTER_NAME}-etcd-backup permissions: read_write region: ${OBJ_BUCKET_REGION:=${LINODE_REGION}} + secretType: addons.cluster.x-k8s.io/resource-set + secretDataFormat: + etcd-backup.yaml: | + apiVersion: v1 + kind: Secret + metadata: + name: ${CLUSTER_NAME}-etcd-backup-obj-key + namespace: kube-system + stringData: + bucket_name: ${CLUSTER_NAME}-etcd-backup + bucket_region: ${OBJ_BUCKET_REGION:=${LINODE_REGION}} + bucket_endpoint: {{ .BucketEndpoint }} + access_key: {{ .AccessKey }} + secret_key: {{ .SecretKey }} --- apiVersion: addons.cluster.x-k8s.io/v1beta1 kind: ClusterResourceSet