From 61da8a4a00ee1e80f06fbdc088fc2a836bf55acd Mon Sep 17 00:00:00 2001 From: leandrorichardtoledo <169929015+leandrorichardtoledo@users.noreply.github.com> Date: Sat, 26 Oct 2024 00:20:51 -0300 Subject: [PATCH] PTEUDO-1691: remove database name from dbclient (#345) * PTEUDO-1691: remove database name from dbclient * PTEUDO-1691: checking whether the db exists or not * PTEUDO-1691: removed old function * PTEUDO-1691: renamed function name to a go standard * PTEUDO-1691: renamed receivers to pc * PTEUDO-1691: rollback, renamed receivers to pc * PTEUDO-1691: added admin db connection * PTEUDO-1691: fix client test * fix bugs in mutating webhooks --------- Co-authored-by: Drew Wells --- cmd/main.go | 2 +- helm/db-controller/minikube.yaml | 2 - .../mutatingwebhookconfiguration.yaml | 2 + pkg/dbclient/client.go | 90 ++++++++++++------- pkg/dbclient/client_test.go | 9 +- 5 files changed, 68 insertions(+), 37 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 273ca02..84b508d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -103,7 +103,7 @@ func main() { flag.StringVar(&metricsDepYamlPath, "metrics-dep-yaml", "/config/postgres-exporter/deployment.yaml", "path to the metrics deployment yaml") flag.StringVar(&metricsConfigYamlPath, "metrics-config-yaml", "/config/postgres-exporter/config.yaml", "path to the metrics config yaml") flag.BoolVar(&enableDBProxyWebhook, "enable-db-proxy", false, "Enable DB Proxy webhook. See docs for usage: https://infobloxopen.github.io/db-controller/#quick-start") - flag.BoolVar(&enableDBProxyWebhook, "enable-deprecation-conversion-webhook", false, "Enable conversion of deprecated pods using dbproxy and/or dsnexec annotations") + flag.BoolVar(&enableDeprecatedConversionWebhook, "enable-deprecation-conversion-webhook", false, "Enable conversion of deprecated pods using dbproxy and/or dsnexec annotations") opts := zap.Options{ Development: true, diff --git a/helm/db-controller/minikube.yaml b/helm/db-controller/minikube.yaml index 0a3178a..9f31ab1 100644 --- a/helm/db-controller/minikube.yaml +++ b/helm/db-controller/minikube.yaml @@ -9,8 +9,6 @@ controllerConfig: zapLogger: develMode: true level: debug -dbproxy: - enabled: true # Block XRDs and XRs in manual deploys xrd: diff --git a/helm/db-controller/templates/mutatingwebhookconfiguration.yaml b/helm/db-controller/templates/mutatingwebhookconfiguration.yaml index dd6bfd9..31c39e7 100644 --- a/helm/db-controller/templates/mutatingwebhookconfiguration.yaml +++ b/helm/db-controller/templates/mutatingwebhookconfiguration.yaml @@ -6,6 +6,7 @@ metadata: annotations: cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ include "db-controller.fullname" . }}-webhook webhooks: + {{- if .Values.deprecationConversionWebhook.enabled }} - clientConfig: service: name: {{ include "db-controller.fullname" . }} @@ -34,6 +35,7 @@ webhooks: scope: "Namespaced" sideEffects: None timeoutSeconds: 10 +{{- end }} - clientConfig: service: name: {{ include "db-controller.fullname" . }} diff --git a/pkg/dbclient/client.go b/pkg/dbclient/client.go index 0afcf34..a6644d8 100644 --- a/pkg/dbclient/client.go +++ b/pkg/dbclient/client.go @@ -3,7 +3,6 @@ package dbclient import ( "context" "database/sql" - "errors" "fmt" "net/url" "strings" @@ -27,17 +26,18 @@ var defaultExtensions = []string{"citext", "uuid-ossp", "plpgsql", "hll"} var specialExtensionsMap = map[string]func(*client, string, string) error{ - "pg_partman": (*client).pg_partman, - "pg_cron": (*client).pg_cron, + "pg_partman": (*client).CreatePgPartmanExtension, + "pg_cron": (*client).CreatePgCronExtension, } type client struct { // cloud is `aws` or `gcp` - cloud string - dbType string - dbURL string - DB *sql.DB - log logr.Logger + cloud string + dbType string + dbURL string + DB *sql.DB + adminDB *sql.DB + log logr.Logger } func (p *client) GetDB() *sql.DB { @@ -78,7 +78,7 @@ func New(cfg Config) (*client, error) { return newPostgresClient(context.TODO(), cfg) } -// creates postgres client +// newPostgresClient creates a new client for postgres. func newPostgresClient(ctx context.Context, cfg Config) (*client, error) { if cfg.Cloud == "" { @@ -98,12 +98,32 @@ func newPostgresClient(ctx context.Context, cfg Config) (*client, error) { if err != nil { return nil, err } + + // create a new connection to the database to run admin commands. + + dsnURL, err := url.Parse(authedDSN) + if err != nil { + return nil, fmt.Errorf("could not parse DSN: %w", err) + } + dsnURL.Path = "/postgres" + + adminDB, err := sql.Open(PostgresType, dsnURL.String()) + if err != nil { + return nil, err + } + + if err := adminDB.PingContext(ctx); err != nil { + adminDB.Close() + return nil, fmt.Errorf("could not connect to admin database: %w", err) + } + return &client{ - cloud: cfg.Cloud, - dbType: PostgresType, - DB: db, - log: cfg.Log, - dbURL: cfg.DSN, + cloud: cfg.Cloud, + dbType: PostgresType, + DB: db, + adminDB: adminDB, + log: cfg.Log, + dbURL: cfg.DSN, }, nil } @@ -126,40 +146,50 @@ func (pc *client) SanitizeDSN() string { return u.Redacted() } -// CreateDataBase implements typo func name incase anybody is using it -func (pc *client) CreateDataBase(name string) (bool, error) { - pc.log.Error(errors.New("CreateDataBase called, use CreateDatabase"), "old_interface_called") - return pc.CreateDatabase(name) -} - // unit test override var getDefaulExtensions = func() []string { return defaultExtensions } +// CreateDatabase creates a database if it does not exist. func (pc *client) CreateDatabase(dbName string) (bool, error) { var exists bool - db := pc.DB log := pc.log.WithValues("database", dbName, "dsn", pc.SanitizeDSN()) - log.Info("pinging database", "ping", db.Ping()) - - err := db.QueryRow(`SELECT EXISTS(SELECT datname FROM pg_catalog.pg_database WHERE datname = $1)`, dbName).Scan(&exists) + query := `SELECT EXISTS(SELECT datname FROM pg_catalog.pg_database WHERE datname = $1)` + err := pc.adminDB.QueryRow(query, dbName).Scan(&exists) if err != nil { + // TODO: use error codes provided by the pq driver. + if strings.Contains(err.Error(), "does not exist") { + return pc.createDatabase(dbName, log) + } + log.Error(err, "could not query for database name", "query", fmt.Sprintf(`SELECT EXISTS(SELECT datname FROM pg_catalog.pg_database WHERE datname = '%s')`, dbName)) - metrics.DBProvisioningErrors.WithLabelValues("read error") + metrics.DBProvisioningErrors.WithLabelValues("read error").Inc() return false, err } + if exists { return false, nil } - // create the database - if _, err := db.Exec(fmt.Sprintf("create database %s", pq.QuoteIdentifier(dbName))); err != nil { + return pc.createDatabase(dbName, log) +} + +func (pc *client) createDatabase(dbName string, log logr.Logger) (bool, error) { + _, err := pc.adminDB.Exec(fmt.Sprintf("CREATE DATABASE %s", pq.QuoteIdentifier(dbName))) + if err != nil { log.Error(err, "could not create database") - metrics.DBProvisioningErrors.WithLabelValues("create error") + metrics.DBProvisioningErrors.WithLabelValues("create error").Inc() + return false, err + } + + if err := pc.DB.Ping(); err != nil { + log.Error(err, "could not connect to database") + metrics.DBProvisioningErrors.WithLabelValues("ping error").Inc() return false, err } + metrics.DBCreated.Inc() return true, nil } @@ -218,7 +248,7 @@ func (pc *client) CreateSpecialExtensions(dbName string, role string) error { return nil } -func (pc *client) pg_cron(dbName string, role string) error { +func (pc *client) CreatePgCronExtension(dbName string, role string) error { // create extension pg_cron and grant usage to public db, err := pc.getDB(dbName) if err != nil { @@ -243,7 +273,7 @@ func (pc *client) pg_cron(dbName string, role string) error { return nil } -func (pc *client) pg_partman(dbName string, role string) error { +func (pc *client) CreatePgPartmanExtension(dbName string, role string) error { db, err := pc.getDB(dbName) if err != nil { diff --git a/pkg/dbclient/client_test.go b/pkg/dbclient/client_test.go index bd47755..4527683 100644 --- a/pkg/dbclient/client_test.go +++ b/pkg/dbclient/client_test.go @@ -71,10 +71,11 @@ func TestPostgresClientOperations(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pc := &client{ - dbType: "postgres", - dbURL: dsn, - DB: db, - log: NewTestLogger(t), + dbType: "postgres", + dbURL: dsn, + DB: db, + adminDB: db, + log: NewTestLogger(t), } got, err := pc.CreateDatabase(tt.args.dbName)