Skip to content

Commit b602d50

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 df6ff54 commit b602d50

File tree

15 files changed

+144
-18
lines changed

15 files changed

+144
-18
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: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,15 @@ func (r *MariaDBAccountReconciler) Reconcile(ctx context.Context, req ctrl.Reque
128128
instance.Status.Conditions.Init(&cl)
129129

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

136136
}
137137

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

@@ -189,18 +189,27 @@ func (r *MariaDBAccountReconciler) reconcileCreate(
189189

190190
var jobDef *batchv1.Job
191191

192+
var createOrUpdate string
193+
if instance.Status.CurrentSecret != "" {
194+
createOrUpdate = "update"
195+
} else {
196+
createOrUpdate = "create"
197+
}
198+
192199
if instance.IsUserAccount() {
193-
log.Info(fmt.Sprintf("Running account create '%s' MariaDBDatabase '%s'",
200+
log.Info(fmt.Sprintf("Checking %s account job '%s' MariaDBDatabase '%s'",
201+
createOrUpdate,
194202
instance.Name, mariadbDatabase.Spec.Name))
195-
jobDef, err = mariadb.CreateDbAccountJob(
203+
jobDef, err = mariadb.CreateOrUpdateDbAccountJob(
196204
dbGalera, instance, mariadbDatabase.Spec.Name, dbHostname,
197205
dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)
198206
if err != nil {
199207
return ctrl.Result{}, err
200208
}
201209
} else {
202-
log.Info(fmt.Sprintf("Running system account create '%s'", instance.Name))
203-
jobDef, err = mariadb.CreateDbAccountJob(
210+
log.Info(fmt.Sprintf("Checking %s system account job '%s'", createOrUpdate,
211+
instance.Name))
212+
jobDef, err = mariadb.CreateOrUpdateDbAccountJob(
204213
dbGalera, instance, "", dbHostname,
205214
dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)
206215
if err != nil {
@@ -225,11 +234,24 @@ func (r *MariaDBAccountReconciler) reconcileCreate(
225234
}
226235

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

235257
// database creation finished
@@ -667,7 +689,15 @@ func (r *MariaDBAccountReconciler) removeAccountAndSecretFinalizer(ctx context.C
667689
helper *helper.Helper, instance *databasev1beta1.MariaDBAccount) error {
668690
controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
669691

670-
accountSecret, _, err := secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace)
692+
return r.removeSecretFinalizer(
693+
ctx, helper, instance.Spec.Secret, instance.Namespace,
694+
)
695+
696+
}
697+
698+
func (r *MariaDBAccountReconciler) removeSecretFinalizer(ctx context.Context,
699+
helper *helper.Helper, secretName string, namespace string) error {
700+
accountSecret, _, err := secret.GetSecret(ctx, helper, secretName, namespace)
671701

672702
if err == nil {
673703
if controllerutil.RemoveFinalizer(accountSecret, helper.GetFinalizer()) {

‎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)