Skip to content

Commit a6e941f

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 c020dd5 commit a6e941f

File tree

12 files changed

+158
-22
lines changed

12 files changed

+158
-22
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
@@ -129,15 +129,15 @@ func (r *MariaDBAccountReconciler) Reconcile(ctx context.Context, req ctrl.Reque
129129
instance.Status.Conditions.Init(&cl)
130130

131131
if instance.DeletionTimestamp.IsZero() || isNewInstance { //revive:disable:indent-error-flow
132-
return r.reconcileCreate(ctx, log, helper, instance)
132+
return r.reconcileCreateOrUpdate(ctx, log, helper, instance)
133133
} else {
134134
return r.reconcileDelete(ctx, log, helper, instance)
135135
}
136136

137137
}
138138

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

@@ -190,18 +190,27 @@ func (r *MariaDBAccountReconciler) reconcileCreate(
190190

191191
var jobDef *batchv1.Job
192192

193+
var createOrUpdate string
194+
if instance.Status.CurrentSecret != "" {
195+
createOrUpdate = "update"
196+
} else {
197+
createOrUpdate = "create"
198+
}
199+
193200
if instance.IsUserAccount() {
194-
log.Info(fmt.Sprintf("Running account create '%s' MariaDBDatabase '%s'",
201+
log.Info(fmt.Sprintf("Checking %s account job '%s' MariaDBDatabase '%s'",
202+
createOrUpdate,
195203
instance.Name, mariadbDatabase.Spec.Name))
196-
jobDef, err = mariadb.CreateDbAccountJob(
204+
jobDef, err = mariadb.CreateOrUpdateDbAccountJob(
197205
dbGalera, instance, mariadbDatabase.Spec.Name, dbHostname,
198206
dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)
199207
if err != nil {
200208
return ctrl.Result{}, err
201209
}
202210
} else {
203-
log.Info(fmt.Sprintf("Running system account create '%s'", instance.Name))
204-
jobDef, err = mariadb.CreateDbAccountJob(
211+
log.Info(fmt.Sprintf("Checking %s system account job '%s'", createOrUpdate,
212+
instance.Name))
213+
jobDef, err = mariadb.CreateOrUpdateDbAccountJob(
205214
dbGalera, instance, "", dbHostname,
206215
dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)
207216
if err != nil {
@@ -226,11 +235,24 @@ func (r *MariaDBAccountReconciler) reconcileCreate(
226235
}
227236

228237
if accountCreateJob.HasChanged() {
238+
229239
if instance.Status.Hash == nil {
230240
instance.Status.Hash = make(map[string]string)
231241
}
232242
instance.Status.Hash[databasev1beta1.AccountCreateHash] = accountCreateJob.GetHash()
233-
log.Info(fmt.Sprintf("Job %s hash added - %s", jobDef.Name, instance.Status.Hash[databasev1beta1.AccountCreateHash]))
243+
log.Info(fmt.Sprintf("Job %s hash created or updated - %s", jobDef.Name, instance.Status.Hash[databasev1beta1.AccountCreateHash]))
244+
245+
// set up new Secret and remove finalizer from old secret
246+
if instance.Status.CurrentSecret != instance.Spec.Secret {
247+
currentSecret := instance.Status.CurrentSecret
248+
err = r.removeSecretFinalizer(ctx, helper, currentSecret, instance.Namespace)
249+
if err == nil {
250+
instance.Status.CurrentSecret = instance.Spec.Secret
251+
} else {
252+
return ctrl.Result{}, err
253+
}
254+
}
255+
234256
}
235257

236258
// database creation finished
@@ -711,7 +733,22 @@ func (r *MariaDBAccountReconciler) ensureAccountSecret(
711733
func (r *MariaDBAccountReconciler) removeAccountAndSecretFinalizer(ctx context.Context,
712734
helper *helper.Helper, instance *databasev1beta1.MariaDBAccount) error {
713735

714-
accountSecret, _, err := secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace)
736+
err := r.removeSecretFinalizer(
737+
ctx, helper, instance.Spec.Secret, instance.Namespace,
738+
)
739+
if err != nil && !k8s_errors.IsNotFound(err) {
740+
return err
741+
}
742+
743+
// remove mariadbaccount finalizer which will update at end of reconcile
744+
controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
745+
746+
return nil
747+
}
748+
749+
func (r *MariaDBAccountReconciler) removeSecretFinalizer(ctx context.Context,
750+
helper *helper.Helper, secretName string, namespace string) error {
751+
accountSecret, _, err := secret.GetSecret(ctx, helper, secretName, namespace)
715752

716753
if err == nil {
717754
if controllerutil.RemoveFinalizer(accountSecret, helper.GetFinalizer()) {
@@ -724,9 +761,6 @@ func (r *MariaDBAccountReconciler) removeAccountAndSecretFinalizer(ctx context.C
724761
return err
725762
}
726763

727-
// will take effect when reconcile ends
728-
controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
729-
730764
return nil
731765

732766
}

‎pkg/mariadb/account.go‎

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

22-
// CreateDbAccountJob creates a Kubernetes job for creating a MariaDB database account
23-
func CreateDbAccountJob(galera *mariadbv1.Galera, account *mariadbv1.MariaDBAccount, databaseName string, databaseHostName string, containerImage string, serviceAccountName string, nodeSelector *map[string]string) (*batchv1.Job, error) {
22+
// CreateOrUpdateDbAccountJob creates or updates a Kubernetes job for creating a MariaDB database account
23+
func CreateOrUpdateDbAccountJob(galera *mariadbv1.Galera, account *mariadbv1.MariaDBAccount, databaseName string, databaseHostName string, containerImage string, serviceAccountName string, nodeSelector *map[string]string) (*batchv1.Job, error) {
2424
var tlsStatement string
2525
if account.Spec.RequireTLS {
2626
tlsStatement = " REQUIRE SSL"
@@ -47,7 +47,7 @@ func CreateDbAccountJob(galera *mariadbv1.Galera, account *mariadbv1.MariaDBAcco
4747
// provided db name is used as metadata name where underscore is a not allowed
4848
// character. Lets replace all underscores with hypen. Underscores in the db name are
4949
// possible.
50-
Name: strings.ReplaceAll(account.Spec.UserName, "_", "-") + "-account-create",
50+
Name: strings.ReplaceAll(account.Spec.UserName, "_", "-") + "-account-create-update",
5151
Namespace: account.Namespace,
5252
Labels: labels,
5353
},
@@ -58,7 +58,7 @@ func CreateDbAccountJob(galera *mariadbv1.Galera, account *mariadbv1.MariaDBAcco
5858
ServiceAccountName: serviceAccountName,
5959
Containers: []corev1.Container{
6060
{
61-
Name: "mariadb-account-create",
61+
Name: "mariadb-account-create-update",
6262
Image: containerImage,
6363
Command: []string{"/bin/sh", "-c", dbCmd},
6464
Env: []corev1.EnvVar{

‎templates/account.sh‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,35 @@ MYSQL_REMOTE_HOST={{.DatabaseHostname}} source /var/lib/operator-scripts/mysql_r
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/chainsaw/tests/create_system_account/chainsaw-test.yaml‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,20 @@ spec:
4949
check:
5050
($error): ~
5151

52+
- name: update secret in place
53+
description: change the secret to a new one and verify password is updated in database
54+
skipDelete: true
55+
try:
56+
- apply:
57+
file: system-account-update-secret.yaml
58+
- assert:
59+
file: system-account-update-assert.yaml
60+
- script:
61+
content: |
62+
../../scripts/check_db_account.sh openstack-galera-0 '' systemuser dbsecret2
63+
check:
64+
($error): ~
65+
5266
- name: drop system account
5367
description: delete the MariaDBAccount CR and verify account is removed from database
5468
try:
@@ -59,9 +73,11 @@ spec:
5973
name: chainsawdb-some-system-db-account
6074
- script:
6175
content: |
62-
../../scripts/check_db_account.sh openstack-galera-0 "" systemuser dbsecret1 --reverse
76+
../../scripts/check_db_account.sh openstack-galera-0 "" systemuser dbsecret2 --reverse
6377
check:
6478
($error): ~
6579
finally:
6680
- delete:
6781
file: system-account-secret.yaml
82+
- delete:
83+
file: system-account-update-secret.yaml

‎tests/chainsaw/tests/create_system_account/system-account-assert.yaml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ metadata:
66
dbName: openstack
77
name: chainsawdb-some-system-db-account
88
status:
9+
currentSecret: some-system-db-secret
910
conditions:
1011
- message: Setup complete
1112
reason: Ready
@@ -34,7 +35,7 @@ type: Opaque
3435
apiVersion: batch/v1
3536
kind: Job
3637
metadata:
37-
name: systemuser-account-create
38+
name: systemuser-account-create-update
3839
spec:
3940
template:
4041
spec: {}

‎tests/chainsaw/tests/create_system_account/system-account-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/chainsaw/tests/create_system_account/system-account-update-assert.yaml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
apiVersion: mariadb.openstack.org/v1beta1
3+
kind: MariaDBAccount
4+
metadata:
5+
labels:
6+
dbName: openstack
7+
name: chainsawdb-some-system-db-account
8+
status:
9+
currentSecret: some-new-system-db-secret
10+
conditions:
11+
- message: Setup complete
12+
reason: Ready
13+
status: "True"
14+
type: Ready
15+
- message: MariaDBAccount creation complete
16+
reason: Ready
17+
status: "True"
18+
type: MariaDBAccountReady
19+
- message: MariaDB / Galera server ready
20+
reason: Ready
21+
status: "True"
22+
type: MariaDBServerReady

0 commit comments

Comments
 (0)