diff --git a/api/test/helpers/crd.go b/api/test/helpers/crd.go index 092670fb..2bd41fe8 100644 --- a/api/test/helpers/crd.go +++ b/api/test/helpers/crd.go @@ -15,6 +15,8 @@ package helpers import ( "context" + "fmt" + "strings" "time" "github.com/go-logr/logr" @@ -266,6 +268,38 @@ func (tc *TestHelper) CreateMariaDBAccount(namespace string, acctName string, sp return name } +// CreateMariaDBAccountAndSecret creates a new MariaDBAccount and Secret with the specified namespace in the Kubernetes cluster. +func (tc *TestHelper) CreateMariaDBAccountAndSecret(name types.NamespacedName, spec mariadbv1.MariaDBAccountSpec) (*mariadbv1.MariaDBAccount, *corev1.Secret) { + secretName := fmt.Sprintf("%s-db-secret", name.Name) + secret := tc.CreateSecret( + types.NamespacedName{Namespace: name.Namespace, Name: secretName}, + map[string][]byte{ + "DatabasePassword": []byte(fmt.Sprintf("%s123", name.Name)), + }, + ) + + if spec.UserName == "" { + spec.UserName = fmt.Sprintf("%s_account", strings.Replace(name.Name, "-", "_", -1)) + } + spec.Secret = secretName + + instance := &mariadbv1.MariaDBAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: name.Name, + Namespace: name.Namespace, + }, + Spec: spec, + } + + gomega.Eventually(func(g gomega.Gomega) { + g.Expect(tc.K8sClient.Create(tc.Ctx, instance)).Should(gomega.Succeed()) + }, tc.Timeout, tc.Interval).Should(gomega.Succeed()) + + tc.Logger.Info(fmt.Sprintf("Created MariaDBAccount %s, username %s, secret %s", name.Name, instance.Spec.UserName, instance.Spec.Secret)) + + return instance, secret +} + // GetMariaDBAccount waits for and retrieves a MariaDBAccount resource from the Kubernetes cluster func (tc *TestHelper) GetMariaDBAccount(name types.NamespacedName) *mariadbv1.MariaDBAccount { instance := &mariadbv1.MariaDBAccount{} diff --git a/api/test/helpers/harnesses.go b/api/test/helpers/harnesses.go new file mode 100644 index 00000000..7ff429bb --- /dev/null +++ b/api/test/helpers/harnesses.go @@ -0,0 +1,330 @@ +/* +Copyright 2023 Red Hat +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package helpers + +import ( + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" + "k8s.io/apimachinery/pkg/types" +) + +// populateHarness describes a function that will insert suite-appropriate +// data into a MariaDBTestHarness instance +type populateHarness func(*MariaDBTestHarness) + +// establishesCR describes a test function that can fully set up a particular +// controller's "Reconciliation Successful" state for a given kind of CR. +type establishesCR func(types.NamespacedName) + +// updatesAccountName describes a test function that can change the +// "databaseAccount" or similar member of an already-reconciled CR to a new +// one, which is expected to kick off a username/password rotation sequence. +type updatesAccountName func(types.NamespacedName) + +// deletesCr describes a test function that will delete the CR that was +// created by an establishesCR function +type deletesCR func() + +// MariaDBTestHarness describes the parameters for running a series +// of Ginkgo tests which exercise a controller's ability to correctly +// work with MariaDBDatabase / MariaDBAccount APIs. +type MariaDBTestHarness struct { + Namespace string + DatabaseName string + FinalizerName string + MariaDBHelper *TestHelper + Timeout time.Duration + Interval time.Duration +} + +// PopulateHarness receives named arguments to place within a +// MariaDBTestHarness instance. +func (harness *MariaDBTestHarness) PopulateHarness( + namespace string, + databaseName string, + finalizerName string, + mariadb *TestHelper, + timeout time.Duration, + interval time.Duration, +) { + harness.Namespace = namespace + harness.DatabaseName = databaseName + harness.FinalizerName = finalizerName + harness.MariaDBHelper = mariadb + harness.Timeout = timeout + harness.Interval = interval +} + +// MariaDBAccountSuiteTests runs MariaDBAccount suite tests. these are +// pre-packaged ginkgo tests that exercise standard account create / update +// patterns that should be common to all controllers that work with +// MariaDBDatabase and MariaDBAccount CRs. +func MariaDBAccountSuiteTests( + description string, + provideHarness populateHarness, + crSetup establishesCR, + updateAccount updatesAccountName, + crDelete deletesCR, +) { + + var harness *MariaDBTestHarness + + BeforeEach(func() { + harness = &MariaDBTestHarness{} + provideHarness(harness) + }) + + When(fmt.Sprintf("The %s service is being configured to run", description), func() { + It("Uses a pre-existing MariaDBAccount and sets a finalizer", func() { + + mariadb := harness.MariaDBHelper + k8sClient := mariadb.K8sClient + + accountName := types.NamespacedName{ + Name: "some-mariadb-account", + Namespace: harness.Namespace, + } + + // create MariaDBAccount first + acc, accSecret := mariadb.CreateMariaDBAccountAndSecret(accountName, mariadbv1.MariaDBAccountSpec{}) + DeferCleanup(k8sClient.Delete, mariadb.Ctx, accSecret) + DeferCleanup(k8sClient.Delete, mariadb.Ctx, acc) + + // then create the CR + crSetup(accountName) + + mariadb.Logger.Info(fmt.Sprintf("Service should fully configure on MariaDBAccount %s", accountName)) + + // now wait for the account to have the finalizer and the + // database name + + Eventually(func() []string { + mariadbAccount := mariadb.GetMariaDBAccount(accountName) + return mariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).Should(ContainElement(harness.FinalizerName)) + + // CreateOrPatchDBByName will add a label referring to the database + Eventually(func() string { + mariadbAccount := mariadb.GetMariaDBAccount(accountName) + return mariadbAccount.Labels["mariaDBDatabaseName"] + }, harness.Timeout, harness.Interval).Should(Equal(harness.DatabaseName)) + + }) + + It("Ensures a MariaDBAccount is created if not present and sets a finalizer", func() { + mariadb := harness.MariaDBHelper + + accountName := types.NamespacedName{ + Name: "some-mariadb-account", + Namespace: harness.Namespace, + } + + // here, dont create a mariadbaccount. right now CRs should + // generate this if not exists using EnsureMariaDBAccount + + // then create the CR + crSetup(accountName) + + mariadb.Logger.Info(fmt.Sprintf("Service should fully configure on MariaDBAccount %s", accountName)) + + // now wait for the account to have the finalizer and the + // database name + + Eventually(func() []string { + mariadbAccount := mariadb.GetMariaDBAccount(accountName) + return mariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).Should(ContainElement(harness.FinalizerName)) + + // CreateOrPatchDBByName will add a label referring to the database + Eventually(func() string { + mariadbAccount := mariadb.GetMariaDBAccount(accountName) + return mariadbAccount.Labels["mariaDBDatabaseName"] + }, harness.Timeout, harness.Interval).Should(Equal(harness.DatabaseName)) + + }) + }) + + When(fmt.Sprintf("The %s service is fully running", description), func() { + // get service fully complete with a mariadbaccount + BeforeEach(func() { + mariadb := harness.MariaDBHelper + + oldAccountName := types.NamespacedName{ + Name: "some-old-account", + Namespace: harness.Namespace, + } + + // create the CR with old account + crSetup(oldAccountName) + + // also simulate that it got completed + mariadb.SimulateMariaDBAccountCompleted(oldAccountName) + + mariadb.Logger.Info(fmt.Sprintf("Service should fully configure on MariaDBAccount %s", oldAccountName)) + + // finalizer is attached to old account + Eventually(func() []string { + oldMariadbAccount := mariadb.GetMariaDBAccount(oldAccountName) + return oldMariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).Should(ContainElement(harness.FinalizerName)) + + }) + It("should ensure a new MariaDBAccount exists when accountname is changed", func() { + mariadb := harness.MariaDBHelper + + oldAccountName := types.NamespacedName{ + Name: "some-old-account", + Namespace: harness.Namespace, + } + + newAccountName := types.NamespacedName{ + Name: "some-new-account", + Namespace: harness.Namespace, + } + + mariadb.Logger.Info("About to update account from some-old-account to some-new-account") + + updateAccount(newAccountName) + + // new account is (eventually) created + _ = mariadb.GetMariaDBAccount(newAccountName) + + // dont simuluate MariaDBAccount being created. it's not done yet + + mariadb.Logger.Info( + fmt.Sprintf("Service should have ensured MariaDBAccount %s exists but should remain running on %s", + newAccountName, oldAccountName), + ) + + // finalizer is attached to new account + Eventually(func() []string { + newMariadbAccount := mariadb.GetMariaDBAccount(newAccountName) + return newMariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).Should(ContainElement(harness.FinalizerName)) + + // old account retains the finalizer because we did not yet + // complete the new MariaDBAccount + Eventually(func() []string { + oldMariadbAccount := mariadb.GetMariaDBAccount(oldAccountName) + return oldMariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).Should(ContainElement(harness.FinalizerName)) + }) + + It("should move the finalizer to a new MariaDBAccount when create is complete", func() { + mariadb := harness.MariaDBHelper + + oldAccountName := types.NamespacedName{ + Name: "some-old-account", + Namespace: harness.Namespace, + } + + newAccountName := types.NamespacedName{ + Name: "some-new-account", + Namespace: harness.Namespace, + } + + updateAccount(newAccountName) + + // new account is (eventually) created + _ = mariadb.GetMariaDBAccount(newAccountName) + + // also simulate that it got completed + mariadb.SimulateMariaDBAccountCompleted(newAccountName) + + mariadb.Logger.Info( + fmt.Sprintf("Service should move to run fully off MariaDBAccount %s and remove finalizer from %s", + newAccountName, oldAccountName), + ) + + // finalizer is attached to new account + Eventually(func() []string { + newMariadbAccount := mariadb.GetMariaDBAccount(newAccountName) + return newMariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).Should(ContainElement(harness.FinalizerName)) + + // CreateOrPatchDBByName will add a label referring to the database + Eventually(func() string { + mariadbAccount := mariadb.GetMariaDBAccount(newAccountName) + return mariadbAccount.Labels["mariaDBDatabaseName"] + }, harness.Timeout, harness.Interval).Should(Equal(harness.DatabaseName)) + + // finalizer removed from old account + Eventually(func() []string { + oldMariadbAccount := mariadb.GetMariaDBAccount(oldAccountName) + return oldMariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).ShouldNot(ContainElement(harness.FinalizerName)) + }) + + It("should remove the finalizer from all associated MariaDBAccount objects regardless of status when deleted", func() { + mariadb := harness.MariaDBHelper + + oldAccountName := types.NamespacedName{ + Name: "some-old-account", + Namespace: harness.Namespace, + } + + newAccountName := types.NamespacedName{ + Name: "some-new-account", + Namespace: harness.Namespace, + } + + mariadb.Logger.Info("About to update account from some-old-account to some-new-account") + + updateAccount(newAccountName) + + // new account is (eventually) created + _ = mariadb.GetMariaDBAccount(newAccountName) + + // dont simuluate MariaDBAccount being created, so that finalizer is + // on both + + mariadb.Logger.Info( + fmt.Sprintf("Service should have ensured MariaDBAccount %s exists but should remain running on %s", + newAccountName, oldAccountName), + ) + + // as before, both accounts have a finalizer + Eventually(func() []string { + newMariadbAccount := mariadb.GetMariaDBAccount(newAccountName) + return newMariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).Should(ContainElement(harness.FinalizerName)) + + Eventually(func() []string { + oldMariadbAccount := mariadb.GetMariaDBAccount(oldAccountName) + return oldMariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).Should(ContainElement(harness.FinalizerName)) + + // now delete the CR + crDelete() + + // finalizer is removed from both as part of the delete + // process + Eventually(func() []string { + newMariadbAccount := mariadb.GetMariaDBAccount(newAccountName) + return newMariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).ShouldNot(ContainElement(harness.FinalizerName)) + + Eventually(func() []string { + oldMariadbAccount := mariadb.GetMariaDBAccount(oldAccountName) + return oldMariadbAccount.Finalizers + }, harness.Timeout, harness.Interval).ShouldNot(ContainElement(harness.FinalizerName)) + + }) + }) + +} diff --git a/api/v1beta1/conditions.go b/api/v1beta1/conditions.go index 79e5cbfb..5565a2af 100644 --- a/api/v1beta1/conditions.go +++ b/api/v1beta1/conditions.go @@ -86,9 +86,15 @@ const ( MariaDBAccountReadyMessage = "MariaDBAccount creation complete" + MariaDBAccountNotReadyMessage = "MariaDBAccount is not present: %s" + MariaDBAccountSecretNotReadyMessage = "MariaDBAccount secret is missing or incomplete: %s" MariaDBErrorRetrievingMariaDBDatabaseMessage = "Error retrieving MariaDBDatabase instance %s" MariaDBErrorRetrievingMariaDBGaleraMessage = "Error retrieving MariaDB/Galera instance %s" + + MariaDBAccountFinalizersRemainMessage = "Waiting for finalizers %s to be removed before dropping username" + + MariaDBAccountReadyForDeleteMessage = "MariaDBAccount ready for delete" ) diff --git a/api/v1beta1/mariadbaccount_types.go b/api/v1beta1/mariadbaccount_types.go index f261d992..31da0b3c 100644 --- a/api/v1beta1/mariadbaccount_types.go +++ b/api/v1beta1/mariadbaccount_types.go @@ -27,6 +27,12 @@ const ( // AccountDeleteHash hash AccountDeleteHash = "accountdelete" + + // DbRootPassword selector for galera root account + DbRootPasswordSelector = "DbRootPassword" + + // DatabasePassword selector for MariaDBAccount->Secret + DatabasePasswordSelector = "DatabasePassword" ) // MariaDBAccountSpec defines the desired state of MariaDBAccount diff --git a/api/v1beta1/mariadbdatabase_funcs.go b/api/v1beta1/mariadbdatabase_funcs.go index d8f71dc6..b3c74f48 100644 --- a/api/v1beta1/mariadbdatabase_funcs.go +++ b/api/v1beta1/mariadbdatabase_funcs.go @@ -18,11 +18,16 @@ package v1beta1 import ( "context" + "crypto/rand" + "encoding/hex" "fmt" "strings" "time" + corev1 "k8s.io/api/core/v1" + "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/service" "github.com/openstack-k8s-operators/lib-common/modules/common/tls" "github.com/openstack-k8s-operators/lib-common/modules/common/util" @@ -31,10 +36,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) -// NewDatabase returns an initialized DB. +// NewDatabase returns a partially-initialized Database struct. +// Deprecated; use NewDatabaseForAccount func NewDatabase( databaseName string, databaseUser string, @@ -47,11 +54,13 @@ func NewDatabase( secret: secret, labels: labels, name: "", + accountName: "", namespace: "", } } -// NewDatabaseWithNamespace returns an initialized DB. +// NewDatabase returns a partially-initialized Database struct. +// Deprecated; use NewDatabaseForAccount func NewDatabaseWithNamespace( databaseName string, databaseUser string, @@ -66,17 +75,37 @@ func NewDatabaseWithNamespace( secret: secret, labels: labels, name: name, + accountName: "", + namespace: namespace, + } +} + +// NewDatabaseForAccount returns an initialized Database struct. +// the stucture has all pre-requisite fields filled in, however has not +// yet populated its object parameters .database and .account +func NewDatabaseForAccount( + databaseInstanceName string, + databaseName string, + name string, + accountName string, + namespace string, +) *Database { + return &Database{ + databaseName: databaseName, + mariadbName: databaseInstanceName, + name: name, + accountName: accountName, namespace: namespace, } } // setDatabaseHostname - set the service name of the DB as the databaseHostname // by looking up the Service via the name of the MariaDB CR which provides it. -func (d *Database) setDatabaseHostname( - ctx context.Context, - h *helper.Helper, - name string, -) error { +func (d *Database) setDatabaseHostname(ctx context.Context, h *helper.Helper) error { + + if d.mariadbName == "" { + return fmt.Errorf("MariaDB CR name mariadbName field is blank") + } // When the MariaDB CR provides the Service it sets the "cr" label of the // Service to "mariadb-". So we use this label @@ -85,16 +114,29 @@ func (d *Database) setDatabaseHostname( // https://github.com/openstack-k8s-operators/mariadb-operator/blob/590ffdc5ad86fe653f9cd8a7102bb76dfe2e36d1/pkg/utils.go#L4 selector := map[string]string{ "app": "mariadb", - "cr": fmt.Sprintf("mariadb-%s", name), + "cr": fmt.Sprintf("mariadb-%s", d.mariadbName), + } + + // assert that Database has the correct namespace. This code + // previously used h.GetBeforeObject().GetNamespace() for the + // namespace, so this assertion allows us to use d.namespace directly + // as we know it is the same value. We basically want to stop relying + // on h.GetBeforeObject() + if h.GetBeforeObject().GetNamespace() != d.namespace { + return fmt.Errorf( + "helper namespace does not match the Database namespace %s != %s", + h.GetBeforeObject().GetNamespace(), d.namespace, + ) } + serviceList, err := service.GetServicesListWithLabel( ctx, h, - h.GetBeforeObject().GetNamespace(), + d.namespace, selector, ) if err != nil || len(serviceList.Items) == 0 { - return fmt.Errorf("Error getting the DB service using label %v: %w", + return fmt.Errorf("error getting the DB service using label %v: %w", selector, err) } @@ -109,7 +151,9 @@ func (d *Database) setDatabaseHostname( ) } svc := serviceList.Items[0] + d.databaseHostname = svc.GetName() + "." + svc.GetNamespace() + ".svc" + h.GetLogger().Info(fmt.Sprintf("Applied new databasehostname %s to MariaDBDatabase %s", d.databaseHostname, d.name)) return nil } @@ -124,20 +168,23 @@ func (d *Database) GetDatabaseHostname() string { return d.databaseHostname } -// GetDatabase - returns the DB +// GetDatabase - returns the MariaDBDatabase object, if one has been loaded. +// may be nil if CreateOrPatchAll or GetDatabaseByNameAndAccount +// have not been called func (d *Database) GetDatabase() *MariaDBDatabase { return d.database } -// GetAccount - returns the account +// GetAccount - returns the MariaDBAccount object, if one has been loaded. +// may be nil if CreateOrPatchAll or GetDatabaseByNameAndAccount +// have not been called func (d *Database) GetAccount() *MariaDBAccount { return d.account } // CreateOrPatchDB - create or patch the service DB instance -// Deprecated. Use CreateOrPatchDBByName instead. If you want to use the -// default the DB service instance of the deployment then pass "openstack" as -// the name. +// Deprecated. Use CreateOrPatchAll() after calling +// NewDatabaseForAccount. func (d *Database) CreateOrPatchDB( ctx context.Context, h *helper.Helper, @@ -145,15 +192,28 @@ func (d *Database) CreateOrPatchDB( return d.CreateOrPatchDBByName(ctx, h, "openstack") } -// CreateOrPatchDBByName - create or patch the service DB instance on -// the DB service. The DB service is selected by the name of the MariaDB CR -// providing the service. +// CreateOrPatchDBByName - create or patch the service DB instance +// Deprecated. Use CreateOrPatchAll() after calling +// NewDatabaseForAccount. func (d *Database) CreateOrPatchDBByName( ctx context.Context, h *helper.Helper, name string, ) (ctrl.Result, error) { + if d.mariadbName == "" { + d.mariadbName = name + } else if d.mariadbName != name { + return ctrl.Result{}, fmt.Errorf( + "the given mariadbname and name sent to CreateOrPatchDBByName "+ + "do not match; %s != %s. Use CreateOrPatchAll() for new code", + d.mariadbName, + name, + ) + } + + // legacy h.GetBeforeObject() stuff. we'd like the Database object + // to be given all correct information up front by the caller. if d.name == "" { d.name = h.GetBeforeObject().GetName() } @@ -161,114 +221,159 @@ func (d *Database) CreateOrPatchDBByName( d.namespace = h.GetBeforeObject().GetNamespace() } - db := d.database - if db == nil { - db = &MariaDBDatabase{ + return d.CreateOrPatchAll(ctx, h) +} + +// CreateOrPatchAll - create or patch the MariaDBDatabase and +// MariaDBAccount. +func (d *Database) CreateOrPatchAll( + ctx context.Context, + h *helper.Helper, +) (ctrl.Result, error) { + + if d.mariadbName == "" { + return ctrl.Result{}, fmt.Errorf( + "MariaDB CR name is not present", + ) + } + if d.name == "" { + return ctrl.Result{}, fmt.Errorf( + "MariaDBDatabase CR name is not present", + ) + + } + if d.accountName == "" { + // no accountName at all. this indicates this Database came about + // using either NewDatabase or NewDatabaseWithNamespace; both + // legacy and both pass along a databaseUser and secret. + + // so for forwards compatibility, + // make a name and a MariaDBAccount for it. name it the same as + // the MariaDBDatabase so we can get it back + // again based on that name alone (also for backwards compatibility). + + h.GetLogger().Info( + fmt.Sprintf( + "Database object for MariaDBDatabase %s does not have a MariaDBAccount CR name configured. "+ + "Assuming legacy use of the API, will use the same name for the MariaDBAccount.", d.name, + ), + ) + + d.accountName = d.name + } + + mariaDBDatabase := d.database + if mariaDBDatabase == nil { + // MariaDBDatabase not present; create one to be patched/created + + mariaDBDatabase = &MariaDBDatabase{ ObjectMeta: metav1.ObjectMeta{ Name: d.name, Namespace: d.namespace, }, Spec: MariaDBDatabaseSpec{ - // the DB name must not change, therefore specify it outside the mutuate function + // the DB name must not change, therefore specify it outside the mutate function Name: d.databaseName, }, } } - account := d.account - if account == nil { - // no account is present in this Database, so for forwards compatibility, - // make one. name it the same as the MariaDBDatabase so we can get it back - // again based on that name alone. - account = &MariaDBAccount{ + mariaDBAccount := d.account + + if mariaDBAccount == nil { + // MariaDBAccount not present. + + mariaDBAccount = &MariaDBAccount{ ObjectMeta: metav1.ObjectMeta{ - Name: d.name, + Name: d.accountName, Namespace: d.namespace, - Labels: map[string]string{ - "mariaDBDatabaseName": d.name, - }, - }, - Spec: MariaDBAccountSpec{ - UserName: d.databaseUser, - Secret: d.secret, }, } + + // databaseUser was given, this is from legacy mode. populate it + // into the account + if d.databaseUser != "" { + mariaDBAccount.Spec.UserName = d.databaseUser + } + + // secret was given, this is also from legacy mode. populate it + // into the account. note here that this is osp-secret, which has + // many PW fields in it. By setting it here, as was the case when + // osp-secret was associated directly with MariaDBDatabase, the + // mariadb-controller is going to use the DatabasePassword value + // for the password, and **not** any of the controller-specific + // passwords. + if d.secret != "" { + mariaDBAccount.Spec.Secret = d.secret + } } + // set the database hostname on the db instance - err := d.setDatabaseHostname(ctx, h, name) + err := d.setDatabaseHostname(ctx, h) if err != nil { return ctrl.Result{}, err } - op, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), db, func() error { - db.Labels = util.MergeStringMaps( - db.GetLabels(), + op, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), mariaDBDatabase, func() error { + mariaDBDatabase.Labels = util.MergeStringMaps( + mariaDBDatabase.GetLabels(), d.labels, + map[string]string{"dbName": d.mariadbName}, ) - err := controllerutil.SetControllerReference(h.GetBeforeObject(), db, h.GetScheme()) + err := controllerutil.SetControllerReference(h.GetBeforeObject(), mariaDBDatabase, h.GetScheme()) if err != nil { return err } // If the service object doesn't have our finalizer, add it. - controllerutil.AddFinalizer(db, h.GetFinalizer()) + controllerutil.AddFinalizer(mariaDBDatabase, h.GetFinalizer()) return nil }) + if d.databaseHostname == "" { + return ctrl.Result{}, fmt.Errorf("Database hostname is blank") + } if err != nil && !k8s_errors.IsNotFound(err) { return ctrl.Result{}, util.WrapErrorForObject( - fmt.Sprintf("Error create or update DB object %s", db.Name), - db, + fmt.Sprintf("Error create or update DB object %s", mariaDBDatabase.Name), + mariaDBDatabase, err, ) } if op != controllerutil.OperationResultNone { - util.LogForObject(h, fmt.Sprintf("DB object %s created or patched", db.Name), db) + util.LogForObject(h, fmt.Sprintf("MariaDBDatabase object %s created or patched", mariaDBDatabase.Name), mariaDBDatabase) return ctrl.Result{RequeueAfter: time.Second * 5}, nil } - opAcc, errAacc := controllerutil.CreateOrPatch(ctx, h.GetClient(), account, func() error { - account.Labels = util.MergeStringMaps( - account.GetLabels(), - d.labels, - ) - - err := controllerutil.SetControllerReference(h.GetBeforeObject(), account, h.GetScheme()) - if err != nil { - return err - } - - // If the service object doesn't have our finalizer, add it. - controllerutil.AddFinalizer(account, h.GetFinalizer()) - - return nil - }) + opAcc, errAcc := CreateOrPatchAccount( + ctx, h, mariaDBAccount, + map[string]string{ + "mariaDBDatabaseName": d.name, + }, + ) - if errAacc != nil && !k8s_errors.IsNotFound(errAacc) { + if errAcc != nil { return ctrl.Result{}, util.WrapErrorForObject( - fmt.Sprintf("Error create or update account object %s", account.Name), - account, - errAacc, + fmt.Sprintf("Error creating or updating MariaDBAccount object %s", mariaDBAccount.Name), + mariaDBAccount, + errAcc, ) } if opAcc != controllerutil.OperationResultNone { - util.LogForObject(h, fmt.Sprintf("Account object %s created or patched", account.Name), account) + util.LogForObject(h, fmt.Sprintf("MariaDBAccount object %s created or patched", mariaDBAccount.Name), mariaDBAccount) return ctrl.Result{RequeueAfter: time.Second * 5}, nil } - err = d.getDBWithName( - ctx, - h, - ) + err = d.loadDatabaseAndAccountCRs(ctx, h) if err != nil { return ctrl.Result{}, err } - d.tlsSupport = db.Status.TLSSupport + d.tlsSupport = mariaDBDatabase.Status.TLSSupport return ctrl.Result{}, nil } @@ -282,10 +387,7 @@ func (d *Database) WaitForDBCreatedWithTimeout( requeueAfter time.Duration, ) (ctrl.Result, error) { - err := d.getDBWithName( - ctx, - h, - ) + err := d.loadDatabaseAndAccountCRs(ctx, h) if err != nil && !k8s_errors.IsNotFound(err) { return ctrl.Result{}, err } @@ -293,7 +395,7 @@ func (d *Database) WaitForDBCreatedWithTimeout( if !d.database.Status.Conditions.IsTrue(MariaDBDatabaseReadyCondition) { util.LogForObject( h, - fmt.Sprintf("Waiting for service DB %s to be created", d.database.Name), + fmt.Sprintf("Waiting for MariaDBDatabase %s to be fully reconciled", d.database.Name), d.database, ) @@ -303,7 +405,7 @@ func (d *Database) WaitForDBCreatedWithTimeout( if d.account != nil && !d.account.Status.Conditions.IsTrue(MariaDBAccountReadyCondition) { util.LogForObject( h, - fmt.Sprintf("Waiting for service account %s to be created", d.account.Name), + fmt.Sprintf("Waiting for MariaDBAccount %s to be fully reconciled", d.account.Name), d.account, ) @@ -313,7 +415,11 @@ func (d *Database) WaitForDBCreatedWithTimeout( if k8s_errors.IsNotFound(err) { util.LogForObject( h, - fmt.Sprintf("DB or account objects not yet found %s", d.database.Name), + fmt.Sprintf( + "MariaDBDatabase %s and/or MariaDBAccount %s not yet found", + d.database.Name, + d.accountName, + ), d.database, ) @@ -332,22 +438,24 @@ func (d *Database) WaitForDBCreated( return d.WaitForDBCreatedWithTimeout(ctx, h, time.Second*5) } -// getDBWithName - get DB object with name in namespace -// note this is legacy as a new function will be added that allows for -// lookup of Database based on mariadbdatabase name and mariadbaccount name -// individually -func (d *Database) getDBWithName( +// loadDatabaseAndAccountCRs - populate Database.database and Database.account +func (d *Database) loadDatabaseAndAccountCRs( ctx context.Context, h *helper.Helper, ) error { - db := &MariaDBDatabase{} + mariaDBDatabase := &MariaDBDatabase{} name := d.name namespace := d.namespace - if name == "" { - name = h.GetBeforeObject().GetName() + + if d.name == "" { + return fmt.Errorf( + "MariaDBDatabase CR name is not present", + ) } - if namespace == "" { - namespace = h.GetBeforeObject().GetNamespace() + if d.namespace == "" { + return fmt.Errorf( + "MariaDBDatabase CR namespace is not present", + ) } err := h.GetClient().Get( @@ -356,7 +464,7 @@ func (d *Database) getDBWithName( Name: name, Namespace: namespace, }, - db) + mariaDBDatabase) if err != nil { if k8s_errors.IsNotFound(err) { @@ -374,76 +482,95 @@ func (d *Database) getDBWithName( ) } - d.database = db - d.tlsSupport = db.Status.TLSSupport + d.database = mariaDBDatabase + d.tlsSupport = mariaDBDatabase.Status.TLSSupport + d.mariadbName = mariaDBDatabase.Labels["dbName"] + accountName := d.accountName - account := &MariaDBAccount{} - username := d.databaseUser + legacyAccount := false - if username == "" { - // no username, so this is a legacy lookup. locate MariaDBAccount + if accountName == "" { + // no account name, so this is a legacy lookup. locate MariaDBAccount // based on the same name as that of the MariaDBDatabase - err = h.GetClient().Get( - ctx, - types.NamespacedName{ - Name: d.name, - Namespace: namespace, - }, - account) - } else { - // username is given. locate MariaDBAccount based on that given - // username. this is also legacy and in practice should not occur - // for any current controller. - err = h.GetClient().Get( - ctx, - types.NamespacedName{ - Name: strings.Replace(username, "_", "-", -1), - Namespace: namespace, - }, - account) + accountName = d.name + legacyAccount = true } + mariaDBAccount, err := GetAccount(ctx, h, accountName, namespace) + if err != nil { - if k8s_errors.IsNotFound(err) { + if legacyAccount && k8s_errors.IsNotFound(err) { // if account can't be found, log it, but don't quit, still // return the Database with MariaDBDatabase h.GetLogger().Info( - fmt.Sprintf("Could not find account %s for Database named %s", username, namespace), + fmt.Sprintf("Could not find account %s for Database named %s", accountName, namespace), ) // note that d.account remains nil in this case + } else { + // only if not legacy account, or other kind of error, do we + // bail out + return util.WrapErrorForObject( + fmt.Sprintf("account error %s %s ", accountName, namespace), + h.GetBeforeObject(), + err, + ) } - return util.WrapErrorForObject( - fmt.Sprintf("account error %s %s ", username, namespace), - h.GetBeforeObject(), - err, - ) } else { - d.account = account + d.account = mariaDBAccount + d.databaseUser = mariaDBAccount.Spec.UserName + d.secret = mariaDBAccount.Spec.Secret } return nil } // GetDatabaseByName returns a *Database object with specified name and namespace +// deprecated; this needs to have the account name given as well for it to work +// completely func GetDatabaseByName( ctx context.Context, h *helper.Helper, name string, ) (*Database, error) { - // create a Database by suppplying a resource name db := &Database{ - name: name, + name: name, + namespace: h.GetBeforeObject().GetNamespace(), + } + + // then querying the MariaDBDatabase and store it in db by calling + if err := db.loadDatabaseAndAccountCRs(ctx, h); err != nil { + return db, err + } + return db, nil +} + +func GetDatabaseByNameAndAccount( + ctx context.Context, + h *helper.Helper, + name string, + accountName string, + namespace string, +) (*Database, error) { + db := &Database{ + name: name, + accountName: accountName, + namespace: namespace, } // then querying the MariaDBDatabase and store it in db by calling - if err := db.getDBWithName(ctx, h); err != nil { + if err := db.loadDatabaseAndAccountCRs(ctx, h); err != nil { return db, err } return db, nil } -// DeleteFinalizer deletes a finalizer by its object +// DeleteFinalizer deletes a finalizer by its object from both +// MariaDBDatabase as well as all associated MariaDBAccount objects. +// if the Database object does not refer to any named MariaDBAccount, this +// is assumed to be legacy and the MariaDBAccount record step is skipped. +// note however this is not expected as MariaDBAccount creation is included +// with all the create functions in this module. func (d *Database) DeleteFinalizer( ctx context.Context, h *helper.Helper, @@ -455,14 +582,30 @@ func (d *Database) DeleteFinalizer( return err } util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBAccount object", h.GetFinalizer()), d.account) - } + // also do a delete for "unused" MariaDBAccounts, associated with + // this MariaDBDatabase. + DeleteUnusedMariaDBAccountFinalizers( + ctx, h, d.database.Name, d.account.Name, d.account.Namespace, + ) + } if controllerutil.RemoveFinalizer(d.database, h.GetFinalizer()) { err := h.GetClient().Update(ctx, d.database) if err != nil && !k8s_errors.IsNotFound(err) { return err } - util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBDatabase object", h.GetFinalizer()), d.database) + util.LogForObject(h, fmt.Sprintf("Removed finalizer %s from MariaDBDatabase %s", h.GetFinalizer(), d.database.Spec.Name), d.database) + + if d.account == nil { + util.LogForObject( + h, + fmt.Sprintf( + "Warning: No MariaDBAccount CR was included when finalizer was removed from MariaDBDatabase %s", + d.database.Spec.Name, + ), + d.database, + ) + } } return nil } @@ -496,3 +639,249 @@ func (d *Database) GetDatabaseClientConfig(s *tls.Service) string { return strings.Join(conn, "\n") } + +// DeleteUnusedMariaDBAccountFinalizers searches for all MariaDBAccounts +// associated with the given MariaDBDatabase name and removes the finalizer for all +// of them except for the given named account. +func DeleteUnusedMariaDBAccountFinalizers( + ctx context.Context, + h *helper.Helper, + mariaDBDatabaseName string, + mariaDBAccountName string, + namespace string, +) error { + + accountList := &MariaDBAccountList{} + + opts := []client.ListOption{ + client.InNamespace(namespace), + client.MatchingLabels{"mariaDBDatabaseName": mariaDBDatabaseName}, + } + + if err := h.GetClient().List(ctx, accountList, opts...); err != nil { + h.GetLogger().Error(err, "Unable to retrieve MariaDBAccountList") + return nil + } + + for _, mariaDBAccount := range accountList.Items { + + if mariaDBAccount.Name == mariaDBAccountName { + continue + } + + 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) + } + + } + return nil + +} + +// CreateOrPatchAccount creates/updates a given MariaDBAccount CR. +func CreateOrPatchAccount( + ctx context.Context, + h *helper.Helper, + account *MariaDBAccount, + 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 the service object doesn't have our finalizer, add it. + controllerutil.AddFinalizer(account, h.GetFinalizer()) + + 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) + } + + return nil + }) + + return opAcc, errAcc +} + +// GetAccount returns an existing MariaDBAccount object from the cluster +func GetAccount(ctx context.Context, + h *helper.Helper, + accountName string, namespace string, +) (*MariaDBAccount, error) { + databaseAccount := &MariaDBAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: accountName, + Namespace: namespace, + }, + } + objectKey := client.ObjectKeyFromObject(databaseAccount) + + err := h.GetClient().Get(ctx, objectKey, databaseAccount) + if err != nil { + return nil, err + } + return databaseAccount, err +} + +// GetAccount returns an existing MariaDBAccount object and its associated +// Secret object from the cluster +func GetAccountAndSecret(ctx context.Context, + h *helper.Helper, + accountName string, namespace string, +) (*MariaDBAccount, *corev1.Secret, error) { + + databaseAccount, err := GetAccount(ctx, h, accountName, namespace) + if err != nil { + return nil, nil, err + } + + if databaseAccount.Spec.Secret == "" { + return nil, nil, fmt.Errorf("no secret field present in MariaDBAccount %s", accountName) + } + + dbSecret, _, err := secret.GetSecret(ctx, h, databaseAccount.Spec.Secret, namespace) + if err != nil { + return nil, nil, err + } + + return databaseAccount, dbSecret, nil +} + +// EnsureMariaDBAccount ensures a MariaDBAccount has been created for a given +// operator calling the function, and returns the MariaDBAccount and its +// Secret for use in consumption into a configuration. +// The current version of the function creates the objects if they don't +// exist; a later version of this can be set to only ensure that the objects +// were already created by an external actor such as openstack-operator up +// front. +func EnsureMariaDBAccount(ctx context.Context, + helper *helper.Helper, + accountName string, namespace string, requireTLS bool, +) (*MariaDBAccount, *corev1.Secret, error) { + + account, err := GetAccount(ctx, helper, accountName, namespace) + + if err != nil { + if !k8s_errors.IsNotFound(err) { + return nil, nil, err + } + + username, err := generateUniqueUsername(helper.GetBeforeObject().GetName()) + if err != nil { + return nil, nil, err + } + + account = &MariaDBAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: accountName, + Namespace: namespace, + // note no labels yet; the account will not have a + // mariadbdatabase yet so the controller will not + // try to create a DB; it instead will respond again to the + // MariaDBAccount once this is filled in + }, + Spec: MariaDBAccountSpec{ + UserName: username, + Secret: fmt.Sprintf("%s-db-secret", accountName), + RequireTLS: requireTLS, + }, + } + + } else { + account.Spec.RequireTLS = requireTLS + + if account.Spec.Secret == "" { + account.Spec.Secret = fmt.Sprintf("%s-db-secret", accountName) + } + } + + dbSecret, _, err := secret.GetSecret(ctx, helper, account.Spec.Secret, namespace) + + if err != nil { + if !k8s_errors.IsNotFound(err) { + return nil, nil, err + } + + dbPassword, err := generateDBPassword() + if err != nil { + return nil, nil, err + } + + dbSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: account.Spec.Secret, + Namespace: namespace, + }, + StringData: map[string]string{ + 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 + } + + util.LogForObject( + helper, + fmt.Sprintf( + "Successfully ensured MariaDBAccount %s exists; database username is %s", + accountName, + account.Spec.UserName, + ), + account, + ) + + return account, dbSecret, nil +} + +// generateUniqueUsername creates a MySQL-compliant database username based on +// a prefix and a 4 character hex string generated randomly +func generateUniqueUsername(usernamePrefix string) (string, error) { + b := make([]byte, 2) + if _, err := rand.Read(b); err != nil { + return "", err + } + + return fmt.Sprintf( + "%s_%s", + strings.Replace(usernamePrefix, "-", "_", -1), + hex.EncodeToString(b)), nil + +} + +// generateDBPassword produces a hex string from a cryptographically secure +// random number generator +func generateDBPassword() (string, error) { + b := make([]byte, 16) + if _, err := rand.Read(b); err != nil { + return "", err + } + return hex.EncodeToString(b), nil +} diff --git a/api/v1beta1/mariadbdatabase_types.go b/api/v1beta1/mariadbdatabase_types.go index 9789213e..72b472b1 100644 --- a/api/v1beta1/mariadbdatabase_types.go +++ b/api/v1beta1/mariadbdatabase_types.go @@ -88,16 +88,19 @@ const ( DatabaseAdminPasswordKey = "AdminPassword" ) -// Database - +// Database - a facade on top of the combination of a MariaDBDatabase +// and MariaDBAccount pair type Database struct { database *MariaDBDatabase account *MariaDBAccount - databaseHostname string - databaseName string - databaseUser string - secret string - labels map[string]string - name string + databaseHostname string // string hostname of database + databaseName string // string name used in CREATE DATABASE statement + databaseUser string // legacy; will go away when operators fully integrate MariaDBAccount + secret string // legacy; will go away when operators fully integrate MariaDBAccount + labels map[string]string // labels to add to the MariaDBDatabase object + name string // CR name for the MariaDBDatabase object + accountName string // CR name for the MariaDBAccount object + mariadbName string // CR name for the MariaDB object namespace string tlsSupport bool } diff --git a/config/manifests/bases/mariadb-operator.clusterserviceversion.yaml b/config/manifests/bases/mariadb-operator.clusterserviceversion.yaml index fa2eaec2..8db3b7bf 100644 --- a/config/manifests/bases/mariadb-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/mariadb-operator.clusterserviceversion.yaml @@ -19,6 +19,10 @@ spec: displayName: Galera kind: Galera name: galeras.mariadb.openstack.org + specDescriptors: + - description: TLS settings for MySQL service and internal Galera replication + displayName: TLS + path: tls version: v1beta1 - description: MariaDBAccount is the Schema for the mariadbaccounts API displayName: Maria DBAccount diff --git a/controllers/mariadbaccount_controller.go b/controllers/mariadbaccount_controller.go index 83e1e827..1330bb8c 100644 --- a/controllers/mariadbaccount_controller.go +++ b/controllers/mariadbaccount_controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "strings" "time" "github.com/go-logr/logr" @@ -260,7 +261,7 @@ func (r *MariaDBAccountReconciler) reconcileCreate( _, secret_result, err := secret.VerifySecret( ctx, types.NamespacedName{Name: instance.Spec.Secret, Namespace: instance.Namespace}, - []string{"DatabasePassword"}, + []string{databasev1beta1.DatabasePasswordSelector}, r.Client, time.Duration(30)*time.Second, ) @@ -389,6 +390,32 @@ func (r *MariaDBAccountReconciler) reconcileDelete( return ctrl.Result{}, err } + // dont do actual DROP USER until finalizers from downstream controllers + // are removed from the CR. + finalizers := instance.GetFinalizers() + finalizersWeCareAbout := []string{} + + for _, f := range finalizers { + if f != helper.GetFinalizer() { + finalizersWeCareAbout = append(finalizersWeCareAbout, f) + } + } + if len(finalizersWeCareAbout) > 0 { + instance.Status.Conditions.MarkFalse( + databasev1beta1.MariaDBAccountReadyCondition, + condition.DeletingReason, + condition.SeverityInfo, + databasev1beta1.MariaDBAccountFinalizersRemainMessage, + strings.Join(finalizersWeCareAbout, ", "), + ) + return ctrl.Result{}, err + } else { + instance.Status.Conditions.MarkTrue( + databasev1beta1.MariaDBAccountReadyCondition, + databasev1beta1.MariaDBAccountReadyForDeleteMessage, + ) + } + instance.Status.Conditions.MarkTrue( databasev1beta1.MariaDBDatabaseReadyCondition, databasev1beta1.MariaDBDatabaseReadyMessage, diff --git a/pkg/mariadb/account.go b/pkg/mariadb/account.go index 1c17ed62..e72d85ff 100644 --- a/pkg/mariadb/account.go +++ b/pkg/mariadb/account.go @@ -67,7 +67,7 @@ func CreateDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName st LocalObjectReference: corev1.LocalObjectReference{ Name: databaseSecret, }, - Key: "DbRootPassword", + Key: databasev1beta1.DbRootPasswordSelector, }, }, }, @@ -78,7 +78,7 @@ func CreateDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName st LocalObjectReference: corev1.LocalObjectReference{ Name: account.Spec.Secret, }, - Key: "DatabasePassword", + Key: databasev1beta1.DatabasePasswordSelector, }, }, }, @@ -128,7 +128,7 @@ func DeleteDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName st LocalObjectReference: corev1.LocalObjectReference{ Name: databaseSecret, }, - Key: "DbRootPassword", + Key: databasev1beta1.DbRootPasswordSelector, }, }, }, diff --git a/pkg/mariadb/database.go b/pkg/mariadb/database.go index 42168f79..791b2b75 100644 --- a/pkg/mariadb/database.go +++ b/pkg/mariadb/database.go @@ -148,7 +148,7 @@ func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHost LocalObjectReference: corev1.LocalObjectReference{ Name: databaseSecret, }, - Key: "DbRootPassword", + Key: databasev1beta1.DbRootPasswordSelector, }, }, }, @@ -161,7 +161,7 @@ func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHost LocalObjectReference: corev1.LocalObjectReference{ Name: *database.Spec.Secret, }, - Key: "DatabasePassword", + Key: databasev1beta1.DatabasePasswordSelector, }, }, }, @@ -175,7 +175,7 @@ func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHost LocalObjectReference: corev1.LocalObjectReference{ Name: databaseSecret, }, - Key: "DbRootPassword", + Key: databasev1beta1.DbRootPasswordSelector, }, }, },