diff --git a/internal/controller/databaserequest_controller.go b/internal/controller/databaserequest_controller.go index e64be93..0fc2c9c 100644 --- a/internal/controller/databaserequest_controller.go +++ b/internal/controller/databaserequest_controller.go @@ -298,7 +298,7 @@ func (r *DatabaseRequestReconciler) handleSeed( // if we got a provider connection and db info we set the database info. if err = r.RelationalDatabaseClient.SetDatabaseInfo( ctx, - seedInfo.conn.getDSN(), + seedInfo.conn.getDSN(false), databaseRequest.Name, databaseRequest.Namespace, databaseRequest.Spec.Type, @@ -450,7 +450,9 @@ func (r *DatabaseRequestReconciler) deleteDatabase( Namespace: databaseRequest.Namespace, }, }); err != nil { - return r.handleError(ctx, databaseRequest, "delete-service", err) + if !apierrors.IsNotFound(err) { + return r.handleError(ctx, databaseRequest, "delete-service", err) + } } if err := r.Delete(ctx, &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -458,7 +460,9 @@ func (r *DatabaseRequestReconciler) deleteDatabase( Namespace: databaseRequest.Namespace, }, }); err != nil { - return r.handleError(ctx, databaseRequest, "delete-secret", err) + if !apierrors.IsNotFound(err) { + return r.handleError(ctx, databaseRequest, "delete-secret", err) + } } if controllerutil.RemoveFinalizer(databaseRequest, databaseRequestFinalizer) { if err := r.Update(ctx, databaseRequest); err != nil { @@ -705,7 +709,7 @@ func (r *DatabaseRequestReconciler) relationalDatabaseOperation( log.FromContext(ctx).Info("Creating relational database", "database", databaseRequest.Name) info, err := r.RelationalDatabaseClient.CreateDatabase( ctx, - conn.getDSN(), + conn.getDSN(false), databaseRequest.Name, databaseRequest.Namespace, databaseRequest.Spec.Type, @@ -736,7 +740,7 @@ func (r *DatabaseRequestReconciler) relationalDatabaseOperation( case drop: if err := r.RelationalDatabaseClient.DropDatabase( ctx, - conn.getDSN(), + conn.getDSN(false), databaseRequest.Name, databaseRequest.Namespace, databaseRequest.Spec.Type, @@ -758,7 +762,7 @@ func (r *DatabaseRequestReconciler) relationalDatabaseOperation( // get the database information info, err := r.RelationalDatabaseClient.GetDatabaseInfo( ctx, - conn.getDSN(), + conn.getDSN(false), databaseRequest.Name, databaseRequest.Namespace, databaseRequest.Spec.Type, @@ -798,7 +802,7 @@ func (r *DatabaseRequestReconciler) relationalDatabaseInfoFromSeed( } // test if the connection works with the seed - if err := r.relDBTestConnection(ctx, dbInfo, dbType); err != nil { + if err := r.relDBTestSeedConnection(ctx, dbInfo, dbType); err != nil { return nil, fmt.Errorf("%s db find connection from seed failed to test connection: %w", dbType, err) } @@ -915,7 +919,7 @@ func (r *DatabaseRequestReconciler) findRelationalDatabaseProvider( // check the load of the provider connection // we select the provider with the lower load log.FromContext(ctx).Info("Checking provider database connection", "connection", dbConnection.Name) - dbLoad, err := r.RelationalDatabaseClient.Load(ctx, conn.getDSN(), databaseRequest.Spec.Type) + dbLoad, err := r.RelationalDatabaseClient.Load(ctx, conn.getDSN(false), databaseRequest.Spec.Type) if err != nil { return nil, "", fmt.Errorf("%s db find provider failed to get load: %w", databaseRequest.Spec.Type, err) } @@ -966,8 +970,8 @@ func (r *DatabaseRequestReconciler) relDBInfo( return &dbInfo, nil } -// relDBTestConnection tests a mysql or postgres connection -func (r *DatabaseRequestReconciler) relDBTestConnection( +// relDBTestSeedConnection tests a mysql or postgres connection +func (r *DatabaseRequestReconciler) relDBTestSeedConnection( ctx context.Context, dbi *dbInfo, dbType string, @@ -979,9 +983,10 @@ func (r *DatabaseRequestReconciler) relDBTestConnection( username: dbi.userName, password: dbi.password, port: dbi.port, + name: dbi.database, } - if err := r.RelationalDatabaseClient.Ping(ctx, conn.getDSN(), dbType); err != nil { - return fmt.Errorf("relation database test connection failed: %w", err) + if err := r.RelationalDatabaseClient.Ping(ctx, conn.getDSN(true), dbType); err != nil { + return fmt.Errorf("relational database test connection failed: %w", err) } return nil } diff --git a/internal/controller/relationaldatabaseprovider_controller.go b/internal/controller/relationaldatabaseprovider_controller.go index c3034a3..773b0ee 100644 --- a/internal/controller/relationaldatabaseprovider_controller.go +++ b/internal/controller/relationaldatabaseprovider_controller.go @@ -184,7 +184,7 @@ func (r *RelationalDatabaseProviderReconciler) Reconcile(ctx context.Context, re // make a ping to the database to check if it's up and running and we can connect to it // if not, we should return an error and set the status to 0 // Note we could periodically check the status of the database and update the status accordingly... - if err := r.RelDBClient.Ping(ctx, conn.getDSN(), instance.Spec.Type); err != nil { + if err := r.RelDBClient.Ping(ctx, conn.getDSN(false), instance.Spec.Type); err != nil { errors = append(errors, err) dbStatus = append(dbStatus, crdv1alpha1.ConnectionStatus{ Name: conn.name, @@ -197,7 +197,7 @@ func (r *RelationalDatabaseProviderReconciler) Reconcile(ctx context.Context, re logger.Error(err, "Failed to ping the database", "hostname", conn.hostname) continue } - version, err := r.RelDBClient.Version(ctx, conn.getDSN(), instance.Spec.Type) + version, err := r.RelDBClient.Version(ctx, conn.getDSN(false), instance.Spec.Type) if err != nil { errors = append(errors, err) dbStatus = append(dbStatus, crdv1alpha1.ConnectionStatus{ @@ -213,7 +213,7 @@ func (r *RelationalDatabaseProviderReconciler) Reconcile(ctx context.Context, re } // check if the database is initialized - err = r.RelDBClient.Initialize(ctx, conn.getDSN(), instance.Spec.Type) + err = r.RelDBClient.Initialize(ctx, conn.getDSN(false), instance.Spec.Type) if err != nil { errors = append(errors, err) dbStatus = append(dbStatus, crdv1alpha1.ConnectionStatus{ @@ -327,17 +327,23 @@ type reldbConn struct { } // getDSN constructs the DSN string for the MySQL or PostgreSQL connection. -func (rc *reldbConn) getDSN() string { +func (rc *reldbConn) getDSN(useDatabase bool) string { + dsn := "" if rc.dbType == "mysql" { - return fmt.Sprintf("%s:%s@tcp(%s:%d)/", rc.username, rc.password, rc.hostname, rc.port) + dsn = fmt.Sprintf("%s:%s@tcp(%s:%d)/", rc.username, rc.password, rc.hostname, rc.port) + if useDatabase { + dsn += rc.name + } } else if rc.dbType == "postgres" { - return fmt.Sprintf( + dsn = fmt.Sprintf( "host=%s port=%d user=%s password=%s sslmode=disable", rc.hostname, rc.port, rc.username, rc.password, ) - } else { - return "" + if useDatabase { + dsn += fmt.Sprintf(" dbname=%s", rc.name) + } } + return dsn } // SetupWithManager sets up the controller with the Manager. diff --git a/internal/database/database.go b/internal/database/database.go index ae5359b..e49d846 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -3,10 +3,12 @@ package database import ( "context" "database/sql" + "errors" "fmt" "math/rand" _ "github.com/go-sql-driver/mysql" + md "github.com/go-sql-driver/mysql" "github.com/lib/pq" _ "github.com/lib/pq" @@ -115,6 +117,32 @@ func (ri *RelationalDatabaseImpl) Ping(ctx context.Context, dsn string, dbType s } if err := db.PingContext(ctx); err != nil { + if dbType == mysql { + log.FromContext(ctx).Error(err, "Failed to ping MMMMMySQL database") + var driverErr *md.MySQLError + if errors.As(err, &driverErr) { + switch driverErr.Number { + case 1044, 1045: + return fmt.Errorf("failed to ping %s database due to invalid credentials: %w", dbType, err) + case 1049: + return fmt.Errorf("failed to ping %s database due to database does not exist: %w", dbType, err) + default: + return fmt.Errorf("failed to ping driveErr %s database: %w", dbType, err) + } + } + } else if dbType == postgres { + var driverErr *pq.Error + if errors.As(err, &driverErr) { + switch driverErr.Code { + case "28P01": + return fmt.Errorf("failed to ping %s database due to invalid credentials: %w", dbType, err) + case "3D000": + return fmt.Errorf("failed to ping %s database due to database does not exist: %w", dbType, err) + default: + return fmt.Errorf("failed to ping %s database: %w", dbType, err) + } + } + } return fmt.Errorf("failed to ping %s database: %w", dbType, err) } diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index d95e0f7..c93ec00 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -19,6 +19,7 @@ package e2e import ( "fmt" "os/exec" + "strings" "time" . "github.com/onsi/ginkgo/v2" @@ -176,6 +177,7 @@ var _ = Describe("controller", Ordered, func() { } EventuallyWithOffset(1, verifyControllerUp, time.Minute, time.Second).Should(Succeed()) + By("validating that all database providers and database requests are working") for _, name := range []string{"mysql", "postgres", "seed"} { if name != "seed" { By("creating a RelationalDatabaseProvider resource") @@ -306,8 +308,94 @@ var _ = Describe("controller", Ordered, func() { ExpectWithOffset(1, secretNames).Should(HaveLen(1)) } - // uncomment to debug ... - //time.Sleep(15 * time.Minute) + By("validating that broken seed database request are failing in exptected way") + for _, name := range []string{"credential-broken-seed", "non-existing-database-seed"} { + By("creating seed secret for the DatabaseRequest resource") + cmd = exec.Command("kubectl", "apply", "-f", fmt.Sprintf("test/e2e/testdata/%s-secret.yaml", name)) + _, err = utils.Run(cmd) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + By("creating a DatabaseRequest resource") + // replace - with _ + dbrName := strings.ReplaceAll(name, "-", "_") + cmd = exec.Command( + "kubectl", + "apply", + "-f", + fmt.Sprintf("test/e2e/testdata/crd_v1alpha1_%s_databaserequest.yaml", dbrName), + ) + _, err = utils.Run(cmd) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + By("validating that the DatabaseRequest resource is created but fails") + cmd = exec.Command( + "kubectl", + "get", + "databaserequest", + fmt.Sprintf("%s-databaserequest-sample", name), + "-o=jsonpath={.status.conditions[?(@.type=='Ready')].status}", + ) + for i := 0; i < 3; i++ { + output, err := utils.Run(cmd) + if err != nil { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } + if strings.TrimSpace(string(output)) == "False" { + break + } else if i == 2 { + Expect(strings.TrimSpace(string(output))).To(Equal("False")) + } + // give it a bit of time to fail + time.Sleep(time.Second) + } + + // verify that the service and secret got created + By("validating that the service is not created") + cmd = exec.Command( + "kubectl", + "get", + "service", + "-n", "default", + "-l", fmt.Sprintf("app.kubernetes.io/instance=%s-databaserequest-sample", name), + ) + output, err := utils.Run(cmd) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + Expect(strings.TrimSpace(string(output))).To(Equal("No resources found in default namespace.")) + + By("validating that the secret is not created") + cmd = exec.Command( + "kubectl", + "get", + "secret", + "-n", "default", + "-l", fmt.Sprintf("app.kubernetes.io/instance=%s-databaserequest-sample", name), + ) + serviceOutput, err := utils.Run(cmd) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + Expect(strings.TrimSpace(string(serviceOutput))).To(Equal("No resources found in default namespace.")) + + By("validating that the seed secret is not deleted") + cmd = exec.Command("kubectl", "get", "secret", fmt.Sprintf("%s-secret", name)) + _, err = utils.Run(cmd) + // expect no error to have occurred because the secret should not get deleted + // if the database request is not successfully created + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + By("deleting the DatabaseRequest resource the database is getting deprovisioned") + cmd = exec.Command( + "kubectl", + "delete", + "databaserequest", + fmt.Sprintf("%s-databaserequest-sample", name), + ) + _, err = utils.Run(cmd) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } }) + + // uncomment to debug ... + //time.Sleep(15 * time.Minute) + }) + }) diff --git a/test/e2e/testdata/crd_v1alpha1_credential_broken_seed_databaserequest.yaml b/test/e2e/testdata/crd_v1alpha1_credential_broken_seed_databaserequest.yaml new file mode 100644 index 0000000..959dec4 --- /dev/null +++ b/test/e2e/testdata/crd_v1alpha1_credential_broken_seed_databaserequest.yaml @@ -0,0 +1,17 @@ +apiVersion: crd.lagoon.sh/v1alpha1 +kind: DatabaseRequest +metadata: + labels: + app.kubernetes.io/name: credential-broken-seed-databaserequest + app.kubernetes.io/instance: credential-broken-seed-databaserequest-sample + app.kubernetes.io/part-of: dbaas-controller + app.kubernetes.io/managed-by: kustomize + app.kubernetes.io/created-by: dbaas-controller + name: credential-broken-seed-databaserequest-sample +spec: + seed: + name: credential-broken-seed-secret + namespace: default + name: credential-broken-seed-mysql-db + scope: development + type: mysql \ No newline at end of file diff --git a/test/e2e/testdata/crd_v1alpha1_non_existing_database_seed_databaserequest.yaml b/test/e2e/testdata/crd_v1alpha1_non_existing_database_seed_databaserequest.yaml new file mode 100644 index 0000000..5c551c6 --- /dev/null +++ b/test/e2e/testdata/crd_v1alpha1_non_existing_database_seed_databaserequest.yaml @@ -0,0 +1,17 @@ +apiVersion: crd.lagoon.sh/v1alpha1 +kind: DatabaseRequest +metadata: + labels: + app.kubernetes.io/name: non-existing-database-seed-databaserequest + app.kubernetes.io/instance: non-existing-database-seed-databaserequest-sample + app.kubernetes.io/part-of: dbaas-controller + app.kubernetes.io/managed-by: kustomize + app.kubernetes.io/created-by: dbaas-controller + name: non-existing-database-seed-databaserequest-sample +spec: + seed: + name: non-existing-database-seed-secret + namespace: default + name: non-existing-database-seed-mysql-db + scope: development + type: mysql \ No newline at end of file diff --git a/test/e2e/testdata/credential-broken-seed-secret.yaml b/test/e2e/testdata/credential-broken-seed-secret.yaml new file mode 100644 index 0000000..d718ffa --- /dev/null +++ b/test/e2e/testdata/credential-broken-seed-secret.yaml @@ -0,0 +1,18 @@ +apiVersion: v1 +kind: Secret +metadata: + name: credential-broken-seed-secret + namespace: default +type: Opaque +data: + #database: seed-database + database: c2VlZC1kYXRhYmFzZQ== + #password: invalid-seed-password + password: aW52YWxpZC1zZWVkLXBhc3N3b3Jk + #username: seed-username + username: c2VlZC11c2VybmFtZQ== + #hostname mysql-service.mysql + hostname: bXlzcWwtc2VydmljZS5teXNxbA== + #port 3306 + port: MzMwNg== + diff --git a/test/e2e/testdata/non-existing-database-seed-secret.yaml b/test/e2e/testdata/non-existing-database-seed-secret.yaml new file mode 100644 index 0000000..bf74eec --- /dev/null +++ b/test/e2e/testdata/non-existing-database-seed-secret.yaml @@ -0,0 +1,18 @@ +apiVersion: v1 +kind: Secret +metadata: + name: non-existing-database-seed-secret + namespace: default +type: Opaque +data: + #database: non-existing-seed-database + database: bm9uLWV4aXN0aW5pbmctc2VlZC1kYXRhYmFzZQ== + #password: seed-password + password: c2VlZC1wYXNzd29yZA== + #username: seed-username + username: c2VlZC11c2VybmFtZQ== + #hostname mysql-service.mysql + hostname: bXlzcWwtc2VydmljZS5teXNxbA== + #port 3306 + port: MzMwNg== +