Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apply finalizers to Secret for MariaDBAccount #210

Merged
merged 1 commit into from
Mar 13, 2024
+189 −20
Merged

apply finalizers to Secret for MariaDBAccount #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions api/test/helpers/harnesses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ func (harness *MariaDBTestHarness) RunBasicSuite() {
return mariadbAccount.Finalizers
}, timeout, interval).Should(ContainElement(harness.finalizerName))

// as well as in the secret
Eventually(func() []string {
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: mariadbAccount.Spec.Secret, Namespace: mariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

// mariaDBDatabaseName is set
Expect(mariadbAccount.Labels["mariaDBDatabaseName"]).Should(Equal(harness.databaseName))

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

// as well as in the secret
Eventually(func() []string {
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: mariadbAccount.Spec.Secret, Namespace: mariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

// mariaDBDatabaseName is set
Expect(mariadbAccount.Labels["mariaDBDatabaseName"]).Should(Equal(harness.databaseName))

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

// as well as in the secret
Eventually(func() []string {
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: oldMariadbAccount.Spec.Secret, Namespace: oldMariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

})
It("should ensure a new MariaDBAccount exists when accountname is changed", func() {
mariaDBHelper, timeout, interval := harness.mariaDBHelper, harness.timeout, harness.interval
Expand Down Expand Up @@ -228,12 +247,27 @@ func (harness *MariaDBTestHarness) RunBasicSuite() {
return newMariadbAccount.Finalizers
}, timeout, interval).Should(ContainElement(harness.finalizerName))

// as well as in the secret
Eventually(func() []string {
newMariadbAccount := mariaDBHelper.GetMariaDBAccount(newAccountName)
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: newMariadbAccount.Spec.Secret, Namespace: newMariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

// old account retains the finalizer because we did not yet
// complete the new MariaDBAccount
Consistently(func() []string {
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
return oldMariadbAccount.Finalizers
}, timeout, interval).Should(ContainElement(harness.finalizerName))

// as well as in the secret
Eventually(func() []string {
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: oldMariadbAccount.Spec.Secret, Namespace: oldMariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

})

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

// as well as in the secret
Eventually(func() []string {
newMariadbAccount := mariaDBHelper.GetMariaDBAccount(newAccountName)
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: newMariadbAccount.Spec.Secret, Namespace: newMariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

// finalizer removed from old account
Eventually(func() []string {
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
return oldMariadbAccount.Finalizers
}, timeout, interval).ShouldNot(ContainElement(harness.finalizerName))

// as well as in the secret
Eventually(func() []string {
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: oldMariadbAccount.Spec.Secret, Namespace: oldMariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).ShouldNot(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

// CreateOrPatchDBByName will add a label referring to the database
Eventually(func() string {
mariadbAccount := mariaDBHelper.GetMariaDBAccount(newAccountName)
Expand Down Expand Up @@ -311,11 +359,25 @@ func (harness *MariaDBTestHarness) RunBasicSuite() {
return newMariadbAccount.Finalizers
}, timeout, interval).Should(ContainElement(harness.finalizerName))

// as well as in the secret
Eventually(func() []string {
newMariadbAccount := mariaDBHelper.GetMariaDBAccount(newAccountName)
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: newMariadbAccount.Spec.Secret, Namespace: newMariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

Eventually(func() []string {
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
return oldMariadbAccount.Finalizers
}, timeout, interval).Should(ContainElement(harness.finalizerName))

// as well as in the secret
Eventually(func() []string {
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: oldMariadbAccount.Spec.Secret, Namespace: oldMariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).Should(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

// now delete the CR
harness.DeleteCR()

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

// as well as in the secret
Eventually(func() []string {
oldMariadbAccount := mariaDBHelper.GetMariaDBAccount(oldAccountName)
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: oldMariadbAccount.Spec.Secret, Namespace: oldMariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).ShouldNot(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

Eventually(func() []string {
newMariadbAccount := mariaDBHelper.GetMariaDBAccount(newAccountName)
dbSecret := harness.mariaDBHelper.GetSecret(types.NamespacedName{Name: newMariadbAccount.Spec.Secret, Namespace: newMariadbAccount.Namespace})
return dbSecret.Finalizers
}, timeout, interval).ShouldNot(ContainElement(fmt.Sprintf("mariadb.openstack.org/%s", harness.finalizerName)))

})

})
Expand Down
134 changes: 114 additions & 20 deletions api/v1beta1/mariadbdatabase_funcs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,18 +356,18 @@ func (d *Database) CreateOrPatchAll(
return ctrl.Result{RequeueAfter: time.Second * 5}, nil
}

opAcc, errAcc := CreateOrPatchAccount(
ctx, h, mariaDBAccount,
opAcc, err := createOrPatchAccountAndSecret(
ctx, h, mariaDBAccount, nil,
map[string]string{
"mariaDBDatabaseName": d.name,
},
)

if errAcc != nil {
if err != nil {
return ctrl.Result{}, util.WrapErrorForObject(
fmt.Sprintf("Error creating or updating MariaDBAccount object %s", mariaDBAccount.Name),
mariaDBAccount,
errAcc,
err,
)
}

Expand Down Expand Up @@ -585,12 +585,21 @@ func (d *Database) DeleteFinalizer(
h *helper.Helper,
) error {

secretFinalizer := fmt.Sprintf("mariadb.openstack.org/%s", h.GetFinalizer())
if d.secretObj != nil && controllerutil.RemoveFinalizer(d.secretObj, secretFinalizer) {
err := h.GetClient().Update(ctx, d.secretObj)
if err != nil && !k8s_errors.IsNotFound(err) {
return err
}
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from Secret %s", secretFinalizer, d.secretObj.Name), d.secretObj)
}

if d.account != nil && controllerutil.RemoveFinalizer(d.account, h.GetFinalizer()) {
err := h.GetClient().Update(ctx, d.account)
if err != nil && !k8s_errors.IsNotFound(err) {
return err
}
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBAccount object", h.GetFinalizer()), d.account)
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBAccount %s", h.GetFinalizer(), d.account.Name), d.account)

// also do a delete for "unused" MariaDBAccounts, associated with
// this MariaDBDatabase.
Expand Down Expand Up @@ -678,21 +687,117 @@ func DeleteUnusedMariaDBAccountFinalizers(
continue
}

if mariaDBAccount.Spec.Secret != "" {
dbSecret, _, err := secret.GetSecret(ctx, h, mariaDBAccount.Spec.Secret, namespace)
if err != nil && !k8s_errors.IsNotFound(err) {
return err
}

secretFinalizer := fmt.Sprintf("mariadb.openstack.org/%s", h.GetFinalizer())
if dbSecret != nil && controllerutil.RemoveFinalizer(dbSecret, secretFinalizer) {
err := h.GetClient().Update(ctx, dbSecret)
if err != nil && !k8s_errors.IsNotFound(err) {
h.GetLogger().Error(
err,
fmt.Sprintf("Unable to remove finalizer %s from Secret %s", h.GetFinalizer(), dbSecret.Name))
return err
}
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from Secret %s", h.GetFinalizer(), dbSecret.Name), dbSecret)
}

}

if controllerutil.RemoveFinalizer(&mariaDBAccount, h.GetFinalizer()) {
err := h.GetClient().Update(ctx, &mariaDBAccount)
if err != nil && !k8s_errors.IsNotFound(err) {
h.GetLogger().Error(err, fmt.Sprintf("Unable to remove finalizer %s from MariaDBAccount %s", h.GetFinalizer(), mariaDBAccount.Name))
return err
}
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBAccount", h.GetFinalizer()), &mariaDBAccount)
util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBAccount %s", h.GetFinalizer(), mariaDBAccount.Name), &mariaDBAccount)
}

}
return nil

}

// createOrPatchAccountAndSecret creates/updates a given MariaDBAccount / Secret CR.
func createOrPatchAccountAndSecret(
ctx context.Context,
h *helper.Helper,
account *MariaDBAccount,
accountSecret *corev1.Secret,
labels map[string]string,
) (controllerutil.OperationResult, error) {
opAcc, errAcc := controllerutil.CreateOrPatch(ctx, h.GetClient(), account, func() error {
account.Labels = util.MergeStringMaps(
account.GetLabels(),
labels,
)

err := controllerutil.SetControllerReference(h.GetBeforeObject(), account, h.GetScheme())
if err != nil {
return err
}

if account.Spec.UserName == "" {
return fmt.Errorf("no UserName field in account %s", account.Name)
}
if account.Spec.Secret == "" {
return fmt.Errorf("no secret field in account %s", account.Name)
}

if accountSecret == nil {
accountSecret, _, err = secret.GetSecret(ctx, h, account.Spec.Secret, account.Namespace)

if err != nil {
return fmt.Errorf("error loading secret %s for account %s",
account.Spec.Secret,
account.Name)
}
}

_, errSecret := controllerutil.CreateOrPatch(ctx, h.GetClient(), accountSecret, func() error {
trueVal := true

_, ok1 := accountSecret.Data[DatabasePasswordSelector]
_, ok2 := accountSecret.StringData[DatabasePasswordSelector]
if !ok1 && !ok2 {
err := fmt.Errorf("field %s not found in Secret %s", DatabasePasswordSelector, accountSecret.Name)
return err
}

accountSecret.Immutable = &trueVal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: accountSecret.Immutable = ptr.To(true)


err := controllerutil.SetControllerReference(h.GetBeforeObject(), accountSecret, h.GetScheme())
if err != nil {
return err
}

// add calling CR finalizer to accountSecret first. controllers use
// GetDatabaseByNameAndAccount to locate the Database which is how
// they remove finalizers. this will return not found if secret
// is not present, so finalizer will keep it around
controllerutil.AddFinalizer(accountSecret, fmt.Sprintf("mariadb.openstack.org/%s", h.GetFinalizer()))

return nil
})

if errSecret != nil {
return errSecret
}

// then add calling CR finalizer to MariaDBAccount
controllerutil.AddFinalizer(account, h.GetFinalizer())

return nil
})

return opAcc, errAcc
}

// CreateOrPatchAccount creates/updates a given MariaDBAccount CR.
// deprecated; use CreateOrPatchAll
func CreateOrPatchAccount(
ctx context.Context,
h *helper.Helper,
Expand Down Expand Up @@ -844,22 +949,11 @@ func EnsureMariaDBAccount(ctx context.Context,
DatabasePasswordSelector: dbPassword,
},
}

_, _, errSecret := secret.CreateOrPatchSecret(
ctx,
helper,
helper.GetBeforeObject(),
dbSecret,
)
if errSecret != nil {
return nil, nil, errSecret
}

}

_, errAcc := CreateOrPatchAccount(ctx, helper, account, map[string]string{})
if errAcc != nil {
return nil, nil, errAcc
_, err = createOrPatchAccountAndSecret(ctx, helper, account, dbSecret, map[string]string{})
if err != nil {
return nil, nil, err
}

util.LogForObject(
Expand Down
Loading