Skip to content

Commit 65450e9

Browse files
committed
chore(api): add finalizers to critical secrets and add recommended labels to secrets
1 parent dfcb023 commit 65450e9

7 files changed

Lines changed: 133 additions & 19 deletions

File tree

config/crd/bases/s3.bedag.ch_globals3policies.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ spec:
198198
renderedPolicy:
199199
description: |-
200200
Rendered policy json document that is applied to the s3access.
201-
Resource is ommited as this is automatically set to the bucket referenced in the s3access.
201+
Resource is omitted as this is automatically set to the bucket referenced in the s3access.
202202
type: string
203203
type: object
204204
type: object

config/crd/bases/s3.bedag.ch_s3accesses.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,9 @@ spec:
101101
rule: self == oldSelf
102102
secretRef:
103103
description: |-
104-
Optionally specify the secret for the access and secret key
105-
if not specified, a secret will be automatically generated and managed by the operator.
104+
Optionally specify the secret name for the access credentials.
105+
If not specified, a secret will be automatically generated and managed by the operator.
106+
Changing this after creation will rename the secret (data is moved, old secret is deleted).
106107
properties:
107108
name:
108109
default: ""

config/crd/bases/s3.bedag.ch_s3policies.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ spec:
200200
renderedPolicy:
201201
description: |-
202202
Rendered policy json document that is applied to the s3access.
203-
Resource is ommited as this is automatically set to the bucket referenced in the s3access.
203+
Resource is omitted as this is automatically set to the bucket referenced in the s3access.
204204
type: string
205205
type: object
206206
type: object

internal/controller/s3access/s3access_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ func (r *S3AccessReconciler) createAccessKeypair(ctx context.Context, rctx *s3Ac
606606
}
607607
}
608608

609-
if err := kube.CreateKeyPairSecret(ctx, r.Client, rctx.S3Access.Namespace, secretName, accessKey, secretKey, rctx.S3Access); err != nil {
609+
if err := kube.CreateKeyPairSecret(ctx, r.Client, rctx.S3Access.Namespace, secretName, accessKey, secretKey, rctx.S3Access, rctx.S3Access.Kind); err != nil {
610610
return fmt.Errorf("failed to create credentials secret: %w", err)
611611
}
612612

@@ -662,7 +662,7 @@ func (r *S3AccessReconciler) moveSecret(ctx context.Context, rctx *s3AccessRecon
662662
}
663663

664664
// Create the new secret with the same data.
665-
if err := kube.CreateKeyPairSecret(ctx, r.Client, rctx.S3Access.Namespace, newName, accessKey, secretKey, rctx.S3Access); err != nil {
665+
if err := kube.CreateKeyPairSecret(ctx, r.Client, rctx.S3Access.Namespace, newName, accessKey, secretKey, rctx.S3Access, rctx.S3Access.Kind); err != nil {
666666
return fmt.Errorf("failed to create new secret %s: %w", newName, err)
667667
}
668668
log.V(1).Info("New secret created", "secretName", newName)

internal/controller/s3bucket_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,7 @@ func (r *S3BucketReconciler) createS3AdminKeypair(ctx context.Context, rctx *buc
808808

809809
// store s3 keys in a secret.
810810
// Update secret with new credentials.
811-
err = kube.CreateKeyPairSecret(ctx, r.Client, rctx.Bucket.Namespace, rctx.Bucket.Status.S3AdminKeysSecretRef.Name, accessKey, secretKey, rctx.Bucket)
811+
err = kube.CreateKeyPairSecret(ctx, r.Client, rctx.Bucket.Namespace, rctx.Bucket.Status.S3AdminKeysSecretRef.Name, accessKey, secretKey, rctx.Bucket, rctx.Bucket.Kind)
812812
if err != nil {
813813
return fmt.Errorf("failed to update credential secret: %w", err)
814814
}

internal/controller/s3tenantaccount_controller.go

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,12 +1071,18 @@ func (r *S3TenantAccountReconciler) reconcileCreate(ctx context.Context, rctx *a
10711071
fmt.Sprintf("Successfully created tenant with ID %s", tenantID))
10721072

10731073
// store credentials in a secret.
1074-
err = kube.CreateCredentialSecret(ctx, r.Client, rctx.Account.Status.RootSecretRef.Namespace, rctx.Account.Status.RootSecretRef.Name, "root", password, rctx.Account)
1074+
err = kube.CreateCredentialSecret(ctx, r.Client, rctx.Account.Status.RootSecretRef.Namespace, rctx.Account.Status.RootSecretRef.Name, "root", password, rctx.Account, rctx.Account.Kind)
10751075
if err != nil {
10761076
log.Error(err, "Failed to create secret")
10771077
return err
10781078
}
10791079

1080+
// Add protective finalizer to root secret to prevent premature deletion during namespace teardown.
1081+
if err := kube.AddSecretFinalizer(ctx, r.Client, rctx.Account.Status.RootSecretRef.Namespace, rctx.Account.Status.RootSecretRef.Name); err != nil {
1082+
log.Error(err, "Failed to add finalizer to root secret")
1083+
return err
1084+
}
1085+
10801086
log.Info(fmt.Sprintf("Tenant created successfully, ID: %s", tenantID))
10811087

10821088
// update status as needed.
@@ -1492,13 +1498,15 @@ func (r *S3TenantAccountReconciler) createTenantAdminCredentials(ctx context.Con
14921498
// Always use S3Tenant as owner if it exists (user-facing secret).
14931499
// Otherwise fall back to Account (platform-managed scenario).
14941500
var owner metav1.Object
1501+
ownerKind := "S3TenantAccount"
14951502
if rctx.Account.Status.S3TenantRef != nil {
14961503
owner = rctx.S3Tenant
1504+
ownerKind = "S3Tenant"
14971505
} else {
14981506
owner = rctx.Account
14991507
}
15001508

1501-
err = kube.CreateCredentialSecret(ctx, r.Client, rctx.Account.Status.AdminSecretRef.Namespace, rctx.Account.Status.AdminSecretRef.Name, username, password, owner)
1509+
err = kube.CreateCredentialSecret(ctx, r.Client, rctx.Account.Status.AdminSecretRef.Namespace, rctx.Account.Status.AdminSecretRef.Name, username, password, owner, ownerKind)
15021510
if err != nil {
15031511
log.Error(err, "Failed to create secret")
15041512
return err
@@ -1575,18 +1583,26 @@ func (r *S3TenantAccountReconciler) createS3AdminKeypair(ctx context.Context, rc
15751583

15761584
// if tenantref is set use this as owner instead.
15771585
var owner metav1.Object
1586+
ownerKind := rctx.Account.Kind
15781587
if rctx.Account.Status.S3TenantRef != nil {
15791588
owner = rctx.S3Tenant
1589+
ownerKind = rctx.S3Tenant.Kind
15801590
} else {
15811591
owner = rctx.Account
15821592
}
15831593
// store s3 keys in a secret.
1584-
err = kube.CreateKeyPairSecret(ctx, r.Client, rctx.Account.Status.S3AdminKeysSecretRef.Namespace, rctx.Account.Status.S3AdminKeysSecretRef.Name, accessKey, secretKey, owner)
1594+
err = kube.CreateKeyPairSecret(ctx, r.Client, rctx.Account.Status.S3AdminKeysSecretRef.Namespace, rctx.Account.Status.S3AdminKeysSecretRef.Name, accessKey, secretKey, owner, ownerKind)
15851595
if err != nil {
15861596
log.Error(err, "Failed to create secret")
15871597
return err
15881598
}
15891599

1600+
// Add protective finalizer to S3 admin keys secret to prevent premature deletion during namespace teardown.
1601+
if err := kube.AddSecretFinalizer(ctx, r.Client, rctx.Account.Status.S3AdminKeysSecretRef.Namespace, rctx.Account.Status.S3AdminKeysSecretRef.Name); err != nil {
1602+
log.Error(err, "Failed to add finalizer to S3 admin keys secret")
1603+
return err
1604+
}
1605+
15901606
rctx.Account.Status.S3AdminAccessKeyId = accessKeyId
15911607

15921608
return nil
@@ -1724,8 +1740,12 @@ func (r *S3TenantAccountReconciler) mapTenantClassToAccounts(ctx context.Context
17241740
func (r *S3TenantAccountReconciler) deleteOperatorSecrets(ctx context.Context, rctx *accountReconcileContext) error {
17251741
log := log.FromContext(ctx)
17261742

1727-
// Delete root secret if present
1743+
// Delete root secret if present (remove protective finalizer first)
17281744
if rctx.Account.Status.RootSecretRef != nil && rctx.Account.Status.RootSecretRef.Name != "" {
1745+
if err := kube.RemoveSecretFinalizer(ctx, r.Client, rctx.Account.Status.RootSecretRef.Namespace, rctx.Account.Status.RootSecretRef.Name); err != nil {
1746+
log.Error(err, "Failed to remove finalizer from root secret")
1747+
return err
1748+
}
17291749
if err := kube.DeleteSecret(ctx, r.Client, rctx.Account.Status.RootSecretRef.Namespace, rctx.Account.Status.RootSecretRef.Name); err != nil {
17301750
log.Error(err, "Failed to delete root secret")
17311751
return err
@@ -1740,8 +1760,12 @@ func (r *S3TenantAccountReconciler) deleteOperatorSecrets(ctx context.Context, r
17401760
}
17411761
}
17421762

1743-
// Delete S3 admin keys secret if present
1763+
// Delete S3 admin keys secret if present (remove protective finalizer first)
17441764
if rctx.Account.Status.S3AdminKeysSecretRef != nil && rctx.Account.Status.S3AdminKeysSecretRef.Name != "" {
1765+
if err := kube.RemoveSecretFinalizer(ctx, r.Client, rctx.Account.Status.S3AdminKeysSecretRef.Namespace, rctx.Account.Status.S3AdminKeysSecretRef.Name); err != nil {
1766+
log.Error(err, "Failed to remove finalizer from S3 admin keys secret")
1767+
return err
1768+
}
17451769
if err := kube.DeleteSecret(ctx, r.Client, rctx.Account.Status.S3AdminKeysSecretRef.Namespace, rctx.Account.Status.S3AdminKeysSecretRef.Name); err != nil {
17461770
log.Error(err, "Failed to delete S3 admin keys secret")
17471771
return err

pkg/kube/secrets.go

Lines changed: 96 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"reflect"
23+
"strings"
2324

2425
"sigs.k8s.io/controller-runtime/pkg/log"
2526

@@ -30,6 +31,11 @@ import (
3031
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
3132
)
3233

34+
const (
35+
// SecretFinalizer is added to critical secrets to prevent premature deletion during namespace teardown.
36+
SecretFinalizer = "secret.s3.bedag.ch/finalizer"
37+
)
38+
3339
// FetchCredentialsFromSecret fetches a Secret and returns the credentials as strings.
3440
// assumes that the keys are always "username" and "password".
3541
func FetchCredentialsFromSecret(ctx context.Context, k8sClient client.Client, namespace string, secretName string) (username string, password string, err error) {
@@ -57,33 +63,36 @@ func FetchKeyPairFromSecret(ctx context.Context, k8sClient client.Client, namesp
5763
}
5864

5965
// creates a secret with keys "username" and "password" in the specified namespace.
60-
func CreateCredentialSecret(ctx context.Context, k8sClient client.Client, namespace string, secretName string, username string, password string, owner metav1.Object) error {
66+
func CreateCredentialSecret(ctx context.Context, k8sClient client.Client, namespace string, secretName string, username string, password string, owner metav1.Object, ownerKind string) error {
6167
data := map[string][]byte{
6268
"username": []byte(username),
6369
"password": []byte(password),
6470
}
6571

66-
return createSecret(ctx, k8sClient, secretName, namespace, data, owner)
72+
return createSecret(ctx, k8sClient, secretName, namespace, data, owner, ownerKind)
6773
}
6874

6975
// creates a secret with keys "accessKeyId" and "secretAccessKey" in the specified namespace.
70-
func CreateKeyPairSecret(ctx context.Context, k8sClient client.Client, namespace string, secretName string, accessKeyId string, secretAccessKey string, owner metav1.Object) error {
76+
func CreateKeyPairSecret(ctx context.Context, k8sClient client.Client, namespace string, secretName string, accessKeyId string, secretAccessKey string, owner metav1.Object, ownerKind string) error {
7177
data := map[string][]byte{
7278
"accessKey": []byte(accessKeyId),
7379
"secretKey": []byte(secretAccessKey),
7480
}
7581

76-
return createSecret(ctx, k8sClient, secretName, namespace, data, owner)
82+
return createSecret(ctx, k8sClient, secretName, namespace, data, owner, ownerKind)
7783
}
7884

79-
func createSecret(ctx context.Context, k8sClient client.Client, secretName string, secretNamespace string, data map[string][]byte, owner metav1.Object) error {
85+
func createSecret(ctx context.Context, k8sClient client.Client, secretName string, secretNamespace string, data map[string][]byte, owner metav1.Object, ownerKind string) error {
8086
log := log.FromContext(ctx).WithValues("func", "createSecret")
8187
log.V(1).Info("Creating or updating secret", "name", secretName, "namespace", secretNamespace)
8288

89+
labels := secretLabels(ownerKind, owner.GetName())
90+
8391
desired := &corev1.Secret{
8492
ObjectMeta: metav1.ObjectMeta{
8593
Name: secretName,
8694
Namespace: secretNamespace,
95+
Labels: labels,
8796
},
8897
Data: data,
8998
}
@@ -116,9 +125,15 @@ func createSecret(ctx context.Context, k8sClient client.Client, secretName strin
116125
)
117126
}
118127

119-
// Update existing if necessary.
120-
if !reflect.DeepEqual(existing.Data, desired.Data) {
128+
// Reconcile labels and data on existing secret.
129+
labelsChanged := mergeLabels(existing, labels)
130+
dataChanged := !reflect.DeepEqual(existing.Data, desired.Data)
131+
132+
if dataChanged {
121133
existing.Data = desired.Data
134+
}
135+
136+
if labelsChanged || dataChanged {
122137
if err := k8sClient.Update(ctx, existing); err != nil {
123138
return err
124139
}
@@ -129,6 +144,32 @@ func createSecret(ctx context.Context, k8sClient client.Client, secretName strin
129144
return nil
130145
}
131146

147+
// secretLabels returns the standard labels for operator-managed secrets.
148+
func secretLabels(ownerKind string, ownerName string) map[string]string {
149+
return map[string]string{
150+
"app.kubernetes.io/managed-by": "storagegrid-operator",
151+
"app.kubernetes.io/part-of": strings.ToLower(ownerKind),
152+
"app.kubernetes.io/instance": ownerName,
153+
}
154+
}
155+
156+
// mergeLabels adds missing labels to the existing object. Returns true if any labels were added or changed.
157+
func mergeLabels(obj metav1.Object, desired map[string]string) bool {
158+
existing := obj.GetLabels()
159+
if existing == nil {
160+
obj.SetLabels(desired)
161+
return true
162+
}
163+
changed := false
164+
for k, v := range desired {
165+
if existing[k] != v {
166+
existing[k] = v
167+
changed = true
168+
}
169+
}
170+
return changed
171+
}
172+
132173
// DeleteSecret deletes a secret from the specified namespace.
133174
func DeleteSecret(ctx context.Context, k8sClient client.Client, namespace string, secretName string) error {
134175
log := log.FromContext(ctx).WithValues("func", "DeleteSecret")
@@ -153,3 +194,51 @@ func DeleteSecret(ctx context.Context, k8sClient client.Client, namespace string
153194
log.V(1).Info("Secret deleted successfully", "name", secretName, "namespace", namespace)
154195
return nil
155196
}
197+
198+
// AddSecretFinalizer adds the protective finalizer to a secret to prevent premature deletion during namespace teardown.
199+
func AddSecretFinalizer(ctx context.Context, k8sClient client.Client, namespace string, secretName string) error {
200+
log := log.FromContext(ctx).WithValues("func", "AddSecretFinalizer")
201+
202+
secret := &corev1.Secret{}
203+
if err := k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: secretName}, secret); err != nil {
204+
return fmt.Errorf("failed to get secret %s/%s for adding finalizer: %w", namespace, secretName, err)
205+
}
206+
207+
if controllerutil.ContainsFinalizer(secret, SecretFinalizer) {
208+
return nil
209+
}
210+
211+
controllerutil.AddFinalizer(secret, SecretFinalizer)
212+
if err := k8sClient.Update(ctx, secret); err != nil {
213+
return fmt.Errorf("failed to add finalizer to secret %s/%s: %w", namespace, secretName, err)
214+
}
215+
216+
log.V(1).Info("Added finalizer to secret", "name", secretName, "namespace", namespace)
217+
return nil
218+
}
219+
220+
// RemoveSecretFinalizer removes the protective finalizer from a secret to allow deletion.
221+
func RemoveSecretFinalizer(ctx context.Context, k8sClient client.Client, namespace string, secretName string) error {
222+
log := log.FromContext(ctx).WithValues("func", "RemoveSecretFinalizer")
223+
224+
secret := &corev1.Secret{}
225+
if err := k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: secretName}, secret); err != nil {
226+
if apierrors.IsNotFound(err) {
227+
log.V(1).Info("Secret already deleted, no finalizer to remove", "name", secretName, "namespace", namespace)
228+
return nil
229+
}
230+
return fmt.Errorf("failed to get secret %s/%s for removing finalizer: %w", namespace, secretName, err)
231+
}
232+
233+
if !controllerutil.ContainsFinalizer(secret, SecretFinalizer) {
234+
return nil
235+
}
236+
237+
controllerutil.RemoveFinalizer(secret, SecretFinalizer)
238+
if err := k8sClient.Update(ctx, secret); err != nil {
239+
return fmt.Errorf("failed to remove finalizer from secret %s/%s: %w", namespace, secretName, err)
240+
}
241+
242+
log.V(1).Info("Removed finalizer from secret", "name", secretName, "namespace", namespace)
243+
return nil
244+
}

0 commit comments

Comments
 (0)