Skip to content

Commit a2ab99b

Browse files
authored
fix: Deleting Postgres CR should delete PostgresUsers (movetokube#45)
Fixes broken `OwnerRef`
1 parent 2bbe76e commit a2ab99b

File tree

4 files changed

+63
-27
lines changed

4 files changed

+63
-27
lines changed

pkg/controller/postgresuser/postgresuser_controller.go

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,21 @@ func (r *ReconcilePostgresUser) Reconcile(request reconcile.Request) (reconcile.
120120
// Deletion logic
121121
if instance.GetDeletionTimestamp() != nil {
122122
if instance.Status.Succeeded && instance.Status.PostgresRole != "" {
123-
err := r.pg.DropRole(instance.Status.PostgresRole, instance.Status.PostgresGroup,
124-
instance.Status.DatabaseName, reqLogger)
123+
// Initialize database name for connection with default database
124+
// in case postgres cr isn't here anymore
125+
db := r.pg.GetDefaultDatabase()
126+
// Search Postgres CR
127+
postgres, err := r.getPostgresCR(instance)
128+
// Check if error exists and not a not found error
129+
if err != nil && !errors.IsNotFound(err) {
130+
return reconcile.Result{}, err
131+
}
132+
// Check if postgres cr is found and not in deletion state
133+
if postgres != nil && !postgres.GetDeletionTimestamp().IsZero() {
134+
db = instance.Status.DatabaseName
135+
}
136+
err = r.pg.DropRole(instance.Status.PostgresRole, instance.Status.PostgresGroup,
137+
db, reqLogger)
125138
if err != nil {
126139
return reconcile.Result{}, err
127140
}
@@ -194,7 +207,7 @@ func (r *ReconcilePostgresUser) Reconcile(request reconcile.Request) (reconcile.
194207
if err != nil {
195208
return r.requeue(instance, err)
196209
}
197-
err = r.addOwnerRef(instance)
210+
err = r.addOwnerRef(reqLogger, instance)
198211
if err != nil {
199212
return r.requeue(instance, err)
200213
}
@@ -241,7 +254,7 @@ func (r *ReconcilePostgresUser) addFinalizer(reqLogger logr.Logger, m *dbv1alpha
241254
// Update CR
242255
err := r.client.Update(context.TODO(), m)
243256
if err != nil {
244-
reqLogger.Error(err, "failed to update Posgres with finalizer")
257+
reqLogger.Error(err, "failed to update PosgresUser with finalizer")
245258
return err
246259
}
247260
}
@@ -300,19 +313,18 @@ func (r *ReconcilePostgresUser) getPostgresCR(instance *dbv1alpha1.PostgresUser)
300313
return &database, nil
301314
}
302315

303-
func (r *ReconcilePostgresUser) addOwnerRef(instance *dbv1alpha1.PostgresUser) error {
316+
func (r *ReconcilePostgresUser) addOwnerRef(reqLogger logr.Logger, instance *dbv1alpha1.PostgresUser) error {
317+
// Search postgres database CR
304318
pg, err := r.getPostgresCR(instance)
305319
if err != nil {
306320
return err
307321
}
308-
isTrue := true
309-
instance.OwnerReferences = append(instance.OwnerReferences, metav1.OwnerReference{
310-
APIVersion: pg.APIVersion,
311-
Kind: pg.Kind,
312-
Name: pg.Name,
313-
UID: pg.UID,
314-
Controller: &isTrue,
315-
BlockOwnerDeletion: &isTrue,
316-
})
317-
return nil
322+
// Update owners
323+
err = controllerutil.SetControllerReference(pg, instance, r.scheme)
324+
if err != nil {
325+
return err
326+
}
327+
// Update CR
328+
err = r.client.Update(context.TODO(), instance)
329+
return err
318330
}

pkg/postgres/mock/postgres.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/postgres/postgres.go

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,28 @@ type PG interface {
2222
DropDatabase(db string, logger logr.Logger) error
2323
DropRole(role, newOwner, database string, logger logr.Logger) error
2424
GetUser() string
25+
GetDefaultDatabase() string
2526
}
2627

2728
type pg struct {
28-
db *sql.DB
29-
log logr.Logger
30-
host string
31-
user string
32-
pass string
33-
args string
29+
db *sql.DB
30+
log logr.Logger
31+
host string
32+
user string
33+
pass string
34+
args string
35+
default_database string
3436
}
3537

3638
func NewPG(host, user, password, uri_args, default_database, cloud_type string, logger logr.Logger) (PG, error) {
3739
postgres := &pg{
38-
db: GetConnection(user, password, host, default_database, uri_args, logger),
39-
log: logger,
40-
host: host,
41-
user: user,
42-
pass: password,
43-
args: uri_args,
40+
db: GetConnection(user, password, host, default_database, uri_args, logger),
41+
log: logger,
42+
host: host,
43+
user: user,
44+
pass: password,
45+
args: uri_args,
46+
default_database: default_database,
4447
}
4548

4649
switch cloud_type {
@@ -57,6 +60,10 @@ func (c *pg) GetUser() string {
5760
return c.user
5861
}
5962

63+
func (c *pg) GetDefaultDatabase() string {
64+
return c.default_database
65+
}
66+
6067
func GetConnection(user, password, host, database, uri_args string, logger logr.Logger) *sql.DB {
6168
db, err := sql.Open("postgres", fmt.Sprintf("postgresql://%s:%s@%s/%s?%s", user, password, host, database, uri_args))
6269
if err != nil {

pkg/postgres/role.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,17 +65,20 @@ func (c *pg) DropRole(role, newOwner, database string, logger logr.Logger) error
6565
tmpDb := GetConnection(c.user, c.pass, c.host, database, c.args, logger)
6666
_, err := tmpDb.Exec(fmt.Sprintf(REASIGN_OBJECTS, role, newOwner))
6767
defer tmpDb.Close()
68+
// Check if error exists and if different from "ROLE NOT FOUND" => 42704
6869
if err != nil && err.(*pq.Error).Code != "42704" {
6970
return err
7071
}
7172

7273
// We previously assigned all objects to the operator's role so DROP OWNED BY will drop privileges of role
7374
_, err = tmpDb.Exec(fmt.Sprintf(DROP_OWNED_BY, role))
75+
// Check if error exists and if different from "ROLE NOT FOUND" => 42704
7476
if err != nil && err.(*pq.Error).Code != "42704" {
7577
return err
7678
}
7779

7880
_, err = c.db.Exec(fmt.Sprintf(DROP_ROLE, role))
81+
// Check if error exists and if different from "ROLE NOT FOUND" => 42704
7982
if err != nil && err.(*pq.Error).Code != "42704" {
8083
return err
8184
}

0 commit comments

Comments
 (0)