Skip to content

Commit 06467ea

Browse files
zzzeekzzzeek
committed
support in-place secret changes
The existing mariadb operator in fact already "supports" in-place change of secret, since if you change Spec.Secret to a new secret name, that would imply a new job hash, and the account.sh script running using only GRANT statements would update the password per mariadb. This already works for flipping the TLS flag on and off too. So in this patch, we clean this up and add a test to include: * a new field Status.CurrentSecret, which is used to indicate the previous secret from which the finalizer should be removed. This will also be used when we migrate the root password to use MariaDBAccount by providing the "current" root password when changing to a new root password * improved messaging in log messages, name of job. This changes the job hash for mariadbaccount which will incur a run on existing environments, however the job hashes are already changing on existing environments due to the change in how the mysql root password is sent, i.e. via volume mounted script rather than env var secret * update account.sh to use modern idiomatic patterns for user create /alter, while mariadb is fine with the legacy style of using only GRANT statements, MySQL 8 no longer allows this statement to proceed without a CREATE USER, so formalize the commands used here to use distinct CREATE USER, ALTER USER, GRANT statements and clarify the script is good for all create/update user operations.
1 parent 7eb7a65 commit 06467ea

File tree

15 files changed

+151
-21
lines changed

15 files changed

+151
-21
lines changed

‎api/bases/mariadb.openstack.org_mariadbaccounts.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,14 @@ spec:
114114
- type
115115
type: object
116116
type: array
117+
currentSecret:
118+
description: |-
119+
the Secret that's currently in use for the account.
120+
keeping a handle to this secret allows us to remove its finalizer
121+
when it's replaced with a new one. It also is useful for storing
122+
the current "root" secret separate from a newly proposed one which is
123+
needed when changing the database root password.
124+
type: string
117125
hash:
118126
additionalProperties:
119127
type: string

‎api/v1beta1/mariadbaccount_types.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ const (
6363

6464
// MariaDBAccountStatus defines the observed state of MariaDBAccount
6565
type MariaDBAccountStatus struct {
66+
// the Secret that's currently in use for the account.
67+
// keeping a handle to this secret allows us to remove its finalizer
68+
// when it's replaced with a new one. It also is useful for storing
69+
// the current "root" secret separate from a newly proposed one which is
70+
// needed when changing the database root password.
71+
CurrentSecret string `json:"currentSecret,omitempty"`
72+
6673
// Deployment Conditions
6774
Conditions condition.Conditions `json:"conditions,omitempty" optional:"true"`
6875

‎config/crd/bases/mariadb.openstack.org_mariadbaccounts.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,14 @@ spec:
114114
- type
115115
type: object
116116
type: array
117+
currentSecret:
118+
description: |-
119+
the Secret that's currently in use for the account.
120+
keeping a handle to this secret allows us to remove its finalizer
121+
when it's replaced with a new one. It also is useful for storing
122+
the current "root" secret separate from a newly proposed one which is
123+
needed when changing the database root password.
124+
type: string
117125
hash:
118126
additionalProperties:
119127
type: string

‎controllers/mariadbaccount_controller.go‎

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,15 @@ func (r *MariaDBAccountReconciler) Reconcile(ctx context.Context, req ctrl.Reque
133133
instance.Status.Conditions.Init(&cl)
134134

135135
if instance.DeletionTimestamp.IsZero() || isNewInstance { //revive:disable:indent-error-flow
136-
return r.reconcileCreate(ctx, log, helper, instance)
136+
return r.reconcileCreateOrUpdate(ctx, log, helper, instance)
137137
} else {
138138
return r.reconcileDelete(ctx, log, helper, instance)
139139
}
140140

141141
}
142142

143-
// reconcileDelete - run reconcile for case where delete timestamp is zero
144-
func (r *MariaDBAccountReconciler) reconcileCreate(
143+
// reconcileCreateOrUpdate - run reconcile for case where delete timestamp is zero
144+
func (r *MariaDBAccountReconciler) reconcileCreateOrUpdate(
145145
ctx context.Context, log logr.Logger,
146146
helper *helper.Helper, instance *databasev1beta1.MariaDBAccount) (result ctrl.Result, _err error) {
147147

@@ -194,18 +194,27 @@ func (r *MariaDBAccountReconciler) reconcileCreate(
194194

195195
var jobDef *batchv1.Job
196196

197+
var createOrUpdate string
198+
if instance.Status.CurrentSecret != "" {
199+
createOrUpdate = "update"
200+
} else {
201+
createOrUpdate = "create"
202+
}
203+
197204
if instance.IsUserAccount() {
198-
log.Info(fmt.Sprintf("Running account create '%s' MariaDBDatabase '%s'",
205+
log.Info(fmt.Sprintf("Checking %s account job '%s' MariaDBDatabase '%s'",
206+
createOrUpdate,
199207
instance.Name, mariadbDatabase.Spec.Name))
200-
jobDef, err = mariadb.CreateDbAccountJob(
208+
jobDef, err = mariadb.CreateOrUpdateDbAccountJob(
201209
dbGalera, instance, mariadbDatabase.Spec.Name, dbHostname,
202210
dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)
203211
if err != nil {
204212
return ctrl.Result{}, err
205213
}
206214
} else {
207-
log.Info(fmt.Sprintf("Running system account create '%s'", instance.Name))
208-
jobDef, err = mariadb.CreateDbAccountJob(
215+
log.Info(fmt.Sprintf("Checking %s system account job '%s'", createOrUpdate,
216+
instance.Name))
217+
jobDef, err = mariadb.CreateOrUpdateDbAccountJob(
209218
dbGalera, instance, "", dbHostname,
210219
dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)
211220
if err != nil {
@@ -230,11 +239,24 @@ func (r *MariaDBAccountReconciler) reconcileCreate(
230239
}
231240

232241
if accountCreateJob.HasChanged() {
242+
233243
if instance.Status.Hash == nil {
234244
instance.Status.Hash = make(map[string]string)
235245
}
236246
instance.Status.Hash[databasev1beta1.AccountCreateHash] = accountCreateJob.GetHash()
237-
log.Info(fmt.Sprintf("Job %s hash added - %s", jobDef.Name, instance.Status.Hash[databasev1beta1.AccountCreateHash]))
247+
log.Info(fmt.Sprintf("Job %s hash created or updated - %s", jobDef.Name, instance.Status.Hash[databasev1beta1.AccountCreateHash]))
248+
249+
// set up new Secret and remove finalizer from old secret
250+
if instance.Status.CurrentSecret != instance.Spec.Secret {
251+
currentSecret := instance.Status.CurrentSecret
252+
err = r.removeSecretFinalizer(ctx, helper, currentSecret, instance.Namespace)
253+
if err == nil {
254+
instance.Status.CurrentSecret = instance.Spec.Secret
255+
} else {
256+
return ctrl.Result{}, err
257+
}
258+
}
259+
238260
}
239261

240262
// database creation finished
@@ -671,7 +693,22 @@ func (r *MariaDBAccountReconciler) ensureAccountSecret(
671693
func (r *MariaDBAccountReconciler) removeAccountAndSecretFinalizer(ctx context.Context,
672694
helper *helper.Helper, instance *databasev1beta1.MariaDBAccount) error {
673695

674-
accountSecret, _, err := secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace)
696+
err := r.removeSecretFinalizer(
697+
ctx, helper, instance.Spec.Secret, instance.Namespace,
698+
)
699+
if err != nil && !k8s_errors.IsNotFound(err) {
700+
return err
701+
}
702+
703+
// remove mariadbaccount finalizer which will update at end of reconcile
704+
controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
705+
706+
return nil
707+
}
708+
709+
func (r *MariaDBAccountReconciler) removeSecretFinalizer(ctx context.Context,
710+
helper *helper.Helper, secretName string, namespace string) error {
711+
accountSecret, _, err := secret.GetSecret(ctx, helper, secretName, namespace)
675712

676713
if err == nil {
677714
if controllerutil.RemoveFinalizer(accountSecret, helper.GetFinalizer()) {
@@ -684,9 +721,6 @@ func (r *MariaDBAccountReconciler) removeAccountAndSecretFinalizer(ctx context.C
684721
return err
685722
}
686723

687-
// will take effect when reconcile ends
688-
controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
689-
690724
return nil
691725

692726
}

‎pkg/mariadb/account.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ type accountCreateOrDeleteOptions struct {
1919
RequireTLS string
2020
}
2121

22-
func CreateDbAccountJob(galera *mariadbv1.Galera, account *databasev1beta1.MariaDBAccount, databaseName string, databaseHostName string, containerImage string, serviceAccountName string, nodeSelector *map[string]string) (*batchv1.Job, error) {
22+
func CreateOrUpdateDbAccountJob(galera *mariadbv1.Galera, account *databasev1beta1.MariaDBAccount, databaseName string, databaseHostName string, containerImage string, serviceAccountName string, nodeSelector *map[string]string) (*batchv1.Job, error) {
2323
var tlsStatement string
2424
if account.Spec.RequireTLS {
2525
tlsStatement = " REQUIRE SSL"
@@ -46,7 +46,7 @@ func CreateDbAccountJob(galera *mariadbv1.Galera, account *databasev1beta1.Maria
4646
// provided db name is used as metadata name where underscore is a not allowed
4747
// character. Lets replace all underscores with hypen. Underscores in the db name are
4848
// possible.
49-
Name: strings.Replace(account.Spec.UserName, "_", "-", -1) + "-account-create",
49+
Name: strings.Replace(account.Spec.UserName, "_", "-", -1) + "-account-create-update",
5050
Namespace: account.Namespace,
5151
Labels: labels,
5252
},
@@ -57,7 +57,7 @@ func CreateDbAccountJob(galera *mariadbv1.Galera, account *databasev1beta1.Maria
5757
ServiceAccountName: serviceAccountName,
5858
Containers: []corev1.Container{
5959
{
60-
Name: "mariadb-account-create",
60+
Name: "mariadb-account-create-update",
6161
Image: containerImage,
6262
Command: []string{"/bin/sh", "-c", dbCmd},
6363
Env: []corev1.EnvVar{

‎templates/account.sh‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,35 @@ source /var/lib/operator-scripts/root_auth.sh
44

55
export DatabasePassword=${DatabasePassword:?"Please specify a DatabasePassword variable."}
66

7+
MYSQL_CMD="mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306"
8+
79
if [ -n "{{.DatabaseName}}" ]; then
8-
mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.UserName}}'@'localhost' IDENTIFIED BY '$DatabasePassword'{{.RequireTLS}};GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.UserName}}'@'%' IDENTIFIED BY '$DatabasePassword'{{.RequireTLS}};"
10+
GRANT_DATABASE="{{.DatabaseName}}"
911
else
10-
mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "GRANT ALL PRIVILEGES ON *.* TO '{{.UserName}}'@'localhost' IDENTIFIED BY '$DatabasePassword'{{.RequireTLS}};GRANT ALL PRIVILEGES ON *.* TO '{{.UserName}}'@'%' IDENTIFIED BY '$DatabasePassword'{{.RequireTLS}};"
12+
GRANT_DATABASE="*"
1113
fi
1214

15+
# going for maximum compatibility here:
16+
# 1. MySQL 8 no longer allows implicit create user when GRANT is used
17+
# 2. MariaDB has "CREATE OR REPLACE", but MySQL does not
18+
# 3. create user with CREATE but then do all password and TLS with ALTER to
19+
# support updates
20+
21+
$MYSQL_CMD <<EOF
22+
CREATE USER IF NOT EXISTS '{{.UserName}}'@'localhost';
23+
CREATE USER IF NOT EXISTS '{{.UserName}}'@'%';
24+
25+
ALTER USER '{{.UserName}}'@'localhost' IDENTIFIED BY '$DatabasePassword'{{.RequireTLS}};
26+
ALTER USER '{{.UserName}}'@'%' IDENTIFIED BY '$DatabasePassword'{{.RequireTLS}};
27+
28+
GRANT ALL PRIVILEGES ON ${GRANT_DATABASE}.* TO '{{.UserName}}'@'localhost';
29+
GRANT ALL PRIVILEGES ON ${GRANT_DATABASE}.* TO '{{.UserName}}'@'%';
30+
EOF
31+
32+
1333
# search for the account. not using SHOW CREATE USER to avoid displaying
1434
# password hash
15-
username=$(mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -NB -e "select user from mysql.user where user='{{.UserName}}' and host='localhost';" )
35+
username=$($MYSQL_CMD -NB -e "select user from mysql.user where user='{{.UserName}}' and host='localhost';" )
1636

1737
if [[ ${username} != "{{.UserName}}" ]]; then
1838
exit 1

‎tests/kuttl/tests/account_create/05-assert.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ commands:
1111
apiVersion: batch/v1
1212
kind: Job
1313
metadata:
14-
name: someuser-account-create
14+
name: someuser-account-create-update
1515
spec:
1616
template:
1717
spec:

‎tests/kuttl/tests/create_system_account/03-assert.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ metadata:
66
dbName: openstack
77
name: kuttldb-some-system-db-account
88
status:
9+
currentSecret: some-system-db-secret
910
conditions:
1011
- message: Setup complete
1112
reason: Ready

‎tests/kuttl/tests/create_system_account/03-create-secret.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
apiVersion: v1
22
data:
3+
# dbsecret1
34
DatabasePassword: ZGJzZWNyZXQx
45
kind: Secret
56
metadata:

‎tests/kuttl/tests/create_system_account/04-assert.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ commands:
1111
apiVersion: batch/v1
1212
kind: Job
1313
metadata:
14-
name: systemuser-account-create
14+
name: systemuser-account-create-update
1515
spec:
1616
template:
1717
spec:

0 commit comments

Comments
 (0)