Skip to content

Commit f152bee

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #210 from zzzeek/no_really_add_finalizer_to_secret
apply finalizers to Secret for MariaDBAccount
2 parents 6b04e3e + cd96520 commit f152bee

File tree

2 files changed

+189
-20
lines changed

2 files changed

+189
-20
lines changed

‎api/test/helpers/harnesses.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,12 @@ func (harness *MariaDBTestHarness) RunBasicSuite() {
124124
return mariadbAccount.Finalizers
125125
}, timeout, interval).Should(ContainElement(harness.finalizerName))
126126

127+
// as well as in the secret
128+
Eventually(func() []string {
129+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: mariadbAccount.Spec.Secret, Namespace: mariadbAccount.Namespace})
130+
return dbSecret.Finalizers
131+
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
132+
127133
// mariaDBDatabaseName is set
128134
Expect(mariadbAccount.Labels["mariaDBDatabaseName"]).Should(Equal(harness.databaseName))
129135

@@ -160,6 +166,12 @@ func (harness *MariaDBTestHarness) RunBasicSuite() {
160166
return mariadbAccount.Finalizers
161167
}, timeout, interval).Should(ContainElement(harness.finalizerName))
162168

169+
// as well as in the secret
170+
Eventually(func() []string {
171+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: mariadbAccount.Spec.Secret, Namespace: mariadbAccount.Namespace})
172+
return dbSecret.Finalizers
173+
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
174+
163175
// mariaDBDatabaseName is set
164176
Expect(mariadbAccount.Labels["mariaDBDatabaseName"]).Should(Equal(harness.databaseName))
165177

@@ -194,6 +206,13 @@ func (harness *MariaDBTestHarness) RunBasicSuite() {
194206
return oldMariadbAccount.Finalizers
195207
}, timeout, interval).Should(ContainElement(harness.finalizerName))
196208

209+
// as well as in the secret
210+
Eventually(func() []string {
211+
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
212+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: oldMariadbAccount.Spec.Secret, Namespace: oldMariadbAccount.Namespace})
213+
return dbSecret.Finalizers
214+
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
215+
197216
})
198217
It("should ensure a new MariaDBAccount exists when accountname is changed", func() {
199218
mariaDBHelper, timeout, interval := harness.mariaDBHelper, harness.timeout, harness.interval
@@ -228,12 +247,27 @@ func (harness *MariaDBTestHarness) RunBasicSuite() {
228247
return newMariadbAccount.Finalizers
229248
}, timeout, interval).Should(ContainElement(harness.finalizerName))
230249

250+
// as well as in the secret
251+
Eventually(func() []string {
252+
newMariadbAccount := mariaDBHelper.GetMariaDBAccount(newAccountName)
253+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: newMariadbAccount.Spec.Secret, Namespace: newMariadbAccount.Namespace})
254+
return dbSecret.Finalizers
255+
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
256+
231257
// old account retains the finalizer because we did not yet
232258
// complete the new MariaDBAccount
233259
Consistently(func() []string {
234260
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
235261
return oldMariadbAccount.Finalizers
236262
}, timeout, interval).Should(ContainElement(harness.finalizerName))
263+
264+
// as well as in the secret
265+
Eventually(func() []string {
266+
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
267+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: oldMariadbAccount.Spec.Secret, Namespace: oldMariadbAccount.Namespace})
268+
return dbSecret.Finalizers
269+
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
270+
237271
})
238272

239273
It("should move the finalizer to a new MariaDBAccount when create is complete", func() {
@@ -263,12 +297,26 @@ func (harness *MariaDBTestHarness) RunBasicSuite() {
263297
return newMariadbAccount.Finalizers
264298
}, timeout, interval).Should(ContainElement(harness.finalizerName))
265299

300+
// as well as in the secret
301+
Eventually(func() []string {
302+
newMariadbAccount := mariaDBHelper.GetMariaDBAccount(newAccountName)
303+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: newMariadbAccount.Spec.Secret, Namespace: newMariadbAccount.Namespace})
304+
return dbSecret.Finalizers
305+
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
306+
266307
// finalizer removed from old account
267308
Eventually(func() []string {
268309
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
269310
return oldMariadbAccount.Finalizers
270311
}, timeout, interval).ShouldNot(ContainElement(harness.finalizerName))
271312

313+
// as well as in the secret
314+
Eventually(func() []string {
315+
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
316+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: oldMariadbAccount.Spec.Secret, Namespace: oldMariadbAccount.Namespace})
317+
return dbSecret.Finalizers
318+
}, timeout, interval).ShouldNot(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
319+
272320
// CreateOrPatchDBByName will add a label referring to the database
273321
Eventually(func() string {
274322
mariadbAccount := mariaDBHelper.GetMariaDBAccount(newAccountName)
@@ -311,11 +359,25 @@ func (harness *MariaDBTestHarness) RunBasicSuite() {
311359
return newMariadbAccount.Finalizers
312360
}, timeout, interval).Should(ContainElement(harness.finalizerName))
313361

362+
// as well as in the secret
363+
Eventually(func() []string {
364+
newMariadbAccount := mariaDBHelper.GetMariaDBAccount(newAccountName)
365+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: newMariadbAccount.Spec.Secret, Namespace: newMariadbAccount.Namespace})
366+
return dbSecret.Finalizers
367+
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
368+
314369
Eventually(func() []string {
315370
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
316371
return oldMariadbAccount.Finalizers
317372
}, timeout, interval).Should(ContainElement(harness.finalizerName))
318373

374+
// as well as in the secret
375+
Eventually(func() []string {
376+
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
377+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: oldMariadbAccount.Spec.Secret, Namespace: oldMariadbAccount.Namespace})
378+
return dbSecret.Finalizers
379+
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
380+
319381
// now delete the CR
320382
harness.DeleteCR()
321383

@@ -331,6 +393,19 @@ func (harness *MariaDBTestHarness) RunBasicSuite() {
331393
return oldMariadbAccount.Finalizers
332394
}, timeout, interval).ShouldNot(ContainElement(harness.finalizerName))
333395

396+
// as well as in the secret
397+
Eventually(func() []string {
398+
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
399+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: oldMariadbAccount.Spec.Secret, Namespace: oldMariadbAccount.Namespace})
400+
return dbSecret.Finalizers
401+
}, timeout, interval).ShouldNot(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
402+
403+
Eventually(func() []string {
404+
newMariadbAccount := mariaDBHelper.GetMariaDBAccount(newAccountName)
405+
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: newMariadbAccount.Spec.Secret, Namespace: newMariadbAccount.Namespace})
406+
return dbSecret.Finalizers
407+
}, timeout, interval).ShouldNot(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))
408+
334409
})
335410

336411
})

‎api/v1beta1/mariadbdatabase_funcs.go

Lines changed: 114 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -356,18 +356,18 @@ func (d *Database) CreateOrPatchAll(
356356
return ctrl.Result{RequeueAfter: time.Second * 5}, nil
357357
}
358358

359-
opAcc, errAcc := CreateOrPatchAccount(
360-
ctx, h, mariaDBAccount,
359+
opAcc, err := createOrPatchAccountAndSecret(
360+
ctx, h, mariaDBAccount, nil,
361361
map[string]string{
362362
"mariaDBDatabaseName": d.name,
363363
},
364364
)
365365

366-
if errAcc != nil {
366+
if err != nil {
367367
return ctrl.Result{}, util.WrapErrorForObject(
368368
fmt.Sprintf("Error creating or updating MariaDBAccount object %s", mariaDBAccount.Name),
369369
mariaDBAccount,
370-
errAcc,
370+
err,
371371
)
372372
}
373373

@@ -585,12 +585,21 @@ func (d *Database) DeleteFinalizer(
585585
h *helper.Helper,
586586
) error {
587587

588+
secretFinalizer := fmt.Sprintf("mariadb.openstack.org/%s", h.GetFinalizer())
589+
if d.secretObj != nil && controllerutil.RemoveFinalizer(d.secretObj, secretFinalizer) {
590+
err := h.GetClient().Update(ctx, d.secretObj)
591+
if err != nil && !k8s_errors.IsNotFound(err) {
592+
return err
593+
}
594+
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from Secret %s", secretFinalizer, d.secretObj.Name), d.secretObj)
595+
}
596+
588597
if d.account != nil && controllerutil.RemoveFinalizer(d.account, h.GetFinalizer()) {
589598
err := h.GetClient().Update(ctx, d.account)
590599
if err != nil && !k8s_errors.IsNotFound(err) {
591600
return err
592601
}
593-
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBAccount object", h.GetFinalizer()), d.account)
602+
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBAccount %s", h.GetFinalizer(), d.account.Name), d.account)
594603

595604
// also do a delete for "unused" MariaDBAccounts, associated with
596605
// this MariaDBDatabase.
@@ -678,21 +687,117 @@ func DeleteUnusedMariaDBAccountFinalizers(
678687
continue
679688
}
680689

690+
if mariaDBAccount.Spec.Secret != "" {
691+
dbSecret, _, err := secret.GetSecret(ctx, h, mariaDBAccount.Spec.Secret, namespace)
692+
if err != nil && !k8s_errors.IsNotFound(err) {
693+
return err
694+
}
695+
696+
secretFinalizer := fmt.Sprintf("mariadb.openstack.org/%s", h.GetFinalizer())
697+
if dbSecret != nil && controllerutil.RemoveFinalizer(dbSecret, secretFinalizer) {
698+
err := h.GetClient().Update(ctx, dbSecret)
699+
if err != nil && !k8s_errors.IsNotFound(err) {
700+
h.GetLogger().Error(
701+
err,
702+
fmt.Sprintf("Unable to remove finalizer %s from Secret %s", h.GetFinalizer(), dbSecret.Name))
703+
return err
704+
}
705+
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from Secret %s", h.GetFinalizer(), dbSecret.Name), dbSecret)
706+
}
707+
708+
}
709+
681710
if controllerutil.RemoveFinalizer(&mariaDBAccount, h.GetFinalizer()) {
682711
err := h.GetClient().Update(ctx, &mariaDBAccount)
683712
if err != nil && !k8s_errors.IsNotFound(err) {
684713
h.GetLogger().Error(err, fmt.Sprintf("Unable to remove finalizer %s from MariaDBAccount %s", h.GetFinalizer(), mariaDBAccount.Name))
685714
return err
686715
}
687-
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBAccount", h.GetFinalizer()), &mariaDBAccount)
716+
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBAccount %s", h.GetFinalizer(), mariaDBAccount.Name), &mariaDBAccount)
688717
}
689718

690719
}
691720
return nil
692721

693722
}
694723

724+
// createOrPatchAccountAndSecret creates/updates a given MariaDBAccount / Secret CR.
725+
func createOrPatchAccountAndSecret(
726+
ctx context.Context,
727+
h *helper.Helper,
728+
account *MariaDBAccount,
729+
accountSecret *corev1.Secret,
730+
labels map[string]string,
731+
) (controllerutil.OperationResult, error) {
732+
opAcc, errAcc := controllerutil.CreateOrPatch(ctx, h.GetClient(), account, func() error {
733+
account.Labels = util.MergeStringMaps(
734+
account.GetLabels(),
735+
labels,
736+
)
737+
738+
err := controllerutil.SetControllerReference(h.GetBeforeObject(), account, h.GetScheme())
739+
if err != nil {
740+
return err
741+
}
742+
743+
if account.Spec.UserName == "" {
744+
return fmt.Errorf("no UserName field in account %s", account.Name)
745+
}
746+
if account.Spec.Secret == "" {
747+
return fmt.Errorf("no secret field in account %s", account.Name)
748+
}
749+
750+
if accountSecret == nil {
751+
accountSecret, _, err = secret.GetSecret(ctx, h, account.Spec.Secret, account.Namespace)
752+
753+
if err != nil {
754+
return fmt.Errorf("error loading secret %s for account %s",
755+
account.Spec.Secret,
756+
account.Name)
757+
}
758+
}
759+
760+
_, errSecret := controllerutil.CreateOrPatch(ctx, h.GetClient(), accountSecret, func() error {
761+
trueVal := true
762+
763+
_, ok1 := accountSecret.Data[DatabasePasswordSelector]
764+
_, ok2 := accountSecret.StringData[DatabasePasswordSelector]
765+
if !ok1 && !ok2 {
766+
err := fmt.Errorf("field %s not found in Secret %s", DatabasePasswordSelector, accountSecret.Name)
767+
return err
768+
}
769+
770+
accountSecret.Immutable = &trueVal
771+
772+
err := controllerutil.SetControllerReference(h.GetBeforeObject(), accountSecret, h.GetScheme())
773+
if err != nil {
774+
return err
775+
}
776+
777+
// add calling CR finalizer to accountSecret first. controllers use
778+
// GetDatabaseByNameAndAccount to locate the Database which is how
779+
// they remove finalizers. this will return not found if secret
780+
// is not present, so finalizer will keep it around
781+
controllerutil.AddFinalizer(accountSecret, fmt.Sprintf("mariadb.openstack.org/%s", h.GetFinalizer()))
782+
783+
return nil
784+
})
785+
786+
if errSecret != nil {
787+
return errSecret
788+
}
789+
790+
// then add calling CR finalizer to MariaDBAccount
791+
controllerutil.AddFinalizer(account, h.GetFinalizer())
792+
793+
return nil
794+
})
795+
796+
return opAcc, errAcc
797+
}
798+
695799
// CreateOrPatchAccount creates/updates a given MariaDBAccount CR.
800+
// deprecated; use CreateOrPatchAll
696801
func CreateOrPatchAccount(
697802
ctx context.Context,
698803
h *helper.Helper,
@@ -844,22 +949,11 @@ func EnsureMariaDBAccount(ctx context.Context,
844949
DatabasePasswordSelector: dbPassword,
845950
},
846951
}
847-
848-
_, _, errSecret := secret.CreateOrPatchSecret(
849-
ctx,
850-
helper,
851-
helper.GetBeforeObject(),
852-
dbSecret,
853-
)
854-
if errSecret != nil {
855-
return nil, nil, errSecret
856-
}
857-
858952
}
859953

860-
_, errAcc := CreateOrPatchAccount(ctx, helper, account, map[string]string{})
861-
if errAcc != nil {
862-
return nil, nil, errAcc
954+
_, err = createOrPatchAccountAndSecret(ctx, helper, account, dbSecret, map[string]string{})
955+
if err != nil {
956+
return nil, nil, err
863957
}
864958

865959
util.LogForObject(

0 commit comments

Comments
 (0)