Skip to content

Commit 1f03e85

Browse files
author
Niki Driessen
committed
Fix support for GCP for both CloudSQL and AlloyDB.
The current logic didn't support AlloyDB postgres deletions. Deleting postgres CR caused various errors, regarding missing permissions and dependent object which caused failures to delete roles. Followed a similar logic as AWS RDS, where the user which the operator is running with is first made member of the database specific roles, so statements like REASSIGN OWNED can complete successfully. Tested with CloudSQL postgres (v 17) and AlloyDB.
1 parent 68eeeed commit 1f03e85

File tree

1 file changed

+24
-46
lines changed

1 file changed

+24
-46
lines changed

pkg/postgres/gcp.go

Lines changed: 24 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -17,30 +17,6 @@ func newGCPPG(postgres *pg) PG {
1717
}
1818
}
1919

20-
func (c *gcppg) DropDatabase(database string, logger logr.Logger) error {
21-
22-
_, err := c.db.Exec(fmt.Sprintf(REVOKE_CONNECT, database))
23-
// Error code 3D000 is returned if database doesn't exist
24-
if err != nil && err.(*pq.Error).Code != "3D000" {
25-
return err
26-
}
27-
28-
_, err = c.db.Exec(fmt.Sprintf(TERMINATE_BACKEND, database))
29-
// Error code 3D000 is returned if database doesn't exist
30-
if err != nil && err.(*pq.Error).Code != "3D000" {
31-
return err
32-
}
33-
_, err = c.db.Exec(fmt.Sprintf(DROP_DATABASE, database))
34-
// Error code 3D000 is returned if database doesn't exist
35-
if err != nil && err.(*pq.Error).Code != "3D000" {
36-
return err
37-
}
38-
39-
logger.Info(fmt.Sprintf("Dropped database %s", database))
40-
41-
return nil
42-
}
43-
4420
func (c *gcppg) CreateDB(dbname, role string) error {
4521

4622
err := c.GrantRole(role, c.user)
@@ -55,31 +31,33 @@ func (c *gcppg) CreateDB(dbname, role string) error {
5531
}
5632

5733
func (c *gcppg) DropRole(role, newOwner, database string, logger logr.Logger) error {
58-
59-
tmpDb, err := GetConnection(c.user, c.pass, c.host, database, c.args, logger)
60-
q := fmt.Sprintf(GET_DB_OWNER, database)
61-
logger.Info("Checking master role: " + q)
62-
rows, err := tmpDb.Query(q)
63-
if err != nil {
64-
return err
34+
if role == "alloydbsuperuser" || role == "postgres" {
35+
logger.Info(fmt.Sprintf("not dropping %s as it is a reserved AlloyDB role", role))
36+
return nil
6537
}
66-
var masterRole string
67-
for rows.Next() {
68-
rows.Scan(&masterRole)
38+
// On AlloyDB the postgres or other alloydbsuperuser members, aren't really superusers so they don't have permissions
39+
// to REASSIGN OWNED BY unless he belongs to both roles
40+
err := c.GrantRole(role, c.user)
41+
if err != nil && err.(*pq.Error).Code != "0LP01" {
42+
if err.(*pq.Error).Code == "42704" {
43+
// The group role does not exist, no point in continuing
44+
return nil
45+
}
46+
return err
6947
}
70-
71-
if role != masterRole {
72-
q = fmt.Sprintf(DROP_ROLE, role)
73-
logger.Info("GCP Drop Role: " + q)
74-
_, err = tmpDb.Exec(q)
75-
// Check if error exists and if different from "ROLE NOT FOUND" => 42704
76-
if err != nil && err.(*pq.Error).Code != "42704" {
48+
defer c.RevokeRole(role, c.user)
49+
if newOwner != c.user {
50+
err = c.GrantRole(newOwner, c.user)
51+
if err != nil && err.(*pq.Error).Code != "0LP01" {
52+
if err.(*pq.Error).Code == "42704" {
53+
// The group role does not exist, no point of granting roles
54+
logger.Info(fmt.Sprintf("not granting %s to %s as %s does not exist", role, newOwner, newOwner))
55+
return nil
56+
}
7757
return err
7858
}
79-
80-
defer tmpDb.Close()
81-
} else {
82-
logger.Info(fmt.Sprintf("GCP refusing DropRole on master role: %s", masterRole))
59+
defer c.RevokeRole(newOwner, c.user)
8360
}
84-
return nil
61+
62+
return c.pg.DropRole(role, newOwner, database, logger)
8563
}

0 commit comments

Comments
 (0)