Skip to content

Commit 5cfa708

Browse files
committed
Improve Azure implmentation
1 parent 035c13a commit 5cfa708

File tree

2 files changed

+46
-38
lines changed

2 files changed

+46
-38
lines changed

pkg/postgres/azure.go

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,36 @@ import (
55
"strings"
66

77
"github.com/go-logr/logr"
8-
"github.com/lib/pq"
8+
)
9+
10+
type AzureType string
11+
12+
const (
13+
// Azure Database for PostgreSQL Flexible Server uses default convention for login
14+
FLEXIBLE AzureType = "flexible"
15+
// Azure Database for PostgreSQL Single Server uses <username>@<servername> convention
16+
SINGLE AzureType = "single"
917
)
1018

1119
type azurepg struct {
1220
serverName string
21+
azureType AzureType
1322
pg
1423
}
1524

1625
func newAzurePG(postgres *pg) PG {
1726
splitUser := strings.Split(postgres.user, "@")
1827
serverName := ""
19-
// We need to know the server name for Azure Database for PostgreSQL Single Server
28+
azureType := FLEXIBLE
2029
if len(splitUser) > 1 {
30+
// If a servername is found, we are using Azure Database for PostgreSQL Single Server
2131
serverName = splitUser[1]
32+
azureType = SINGLE
2233
}
2334
return &azurepg{
24-
serverName,
25-
*postgres,
35+
serverName: serverName,
36+
azureType: azureType,
37+
pg: *postgres,
2638
}
2739
}
2840

@@ -31,24 +43,31 @@ func (azpg *azurepg) CreateUserRole(role, password string) (string, error) {
3143
if err != nil {
3244
return "", err
3345
}
34-
if azpg.serverName == "" {
46+
47+
// For Flexible Server, just return the role name as-is
48+
if azpg.azureType == FLEXIBLE {
3549
return returnedRole, nil
3650
}
37-
// Azure Database for PostgreSQL Single Server offering uses <username>@<servername> convention
51+
52+
// For Single Server, format as <username>@<servername>
3853
return fmt.Sprintf("%s@%s", returnedRole, azpg.serverName), nil
3954
}
4055

4156
func (azpg *azurepg) GetRoleForLogin(login string) string {
42-
splitUser := strings.Split(azpg.user, "@")
43-
if len(splitUser) > 1 {
44-
return splitUser[0]
57+
// For Azure Flexible Server, the login name is the same as the role name
58+
if azpg.azureType == FLEXIBLE {
59+
return login
4560
}
46-
return login
61+
62+
// For Azure Single Server, extract the username part before the '@' symbol
63+
splitUser := strings.Split(azpg.user, "@")
64+
return splitUser[0]
4765
}
4866

4967
func (azpg *azurepg) CreateDB(dbname, role string) error {
50-
// Have to add the master role to the group role before we can transfer the database owner
51-
err := azpg.GrantRole(role, azpg.GetRoleForLogin(azpg.user))
68+
// Grant admin privileges to the group role to enable database ownership transfer
69+
// This step is necessary before we can set the specified role as the database owner
70+
err := azpg.grantAdminRole(role, azpg.GetRoleForLogin(azpg.user))
5271
if err != nil {
5372
return err
5473
}
@@ -57,32 +76,12 @@ func (azpg *azurepg) CreateDB(dbname, role string) error {
5776
}
5877

5978
func (azpg *azurepg) DropRole(role, newOwner, database string, logger logr.Logger) error {
60-
if azpg.serverName != "" {
61-
// Logic for Single Server
62-
azNewOwner := azpg.GetRoleForLogin(newOwner)
63-
return azpg.pg.DropRole(role, azNewOwner, database, logger)
64-
} else {
65-
// Logic for Flexible Server (same as AWS)
66-
// to REASSIGN OWNED BY unless he belongs to both roles
67-
err := azpg.pg.GrantRole(role, azpg.user)
68-
if err != nil && err.(*pq.Error).Code != "0LP01" {
69-
if err.(*pq.Error).Code == "42704" {
70-
// The group role does not exist, no point in continuing
71-
return nil
72-
}
73-
return err
74-
}
75-
err = azpg.pg.GrantRole(newOwner, azpg.user)
76-
if err != nil && err.(*pq.Error).Code != "0LP01" {
77-
if err.(*pq.Error).Code == "42704" {
78-
// The group role does not exist, no point of granting roles
79-
logger.Info(fmt.Sprintf("not granting %s to %s as %s does not exist", role, newOwner, newOwner))
80-
return nil
81-
}
82-
return err
83-
}
84-
defer azpg.pg.RevokeRole(newOwner, azpg.pg.user)
85-
79+
// For Azure Flexible Server, we can use the standard pg implementation
80+
if azpg.azureType == FLEXIBLE {
8681
return azpg.pg.DropRole(role, newOwner, database, logger)
8782
}
83+
84+
// For Azure Single Server, we need to get the correct format for the newOwner
85+
azNewOwner := azpg.GetRoleForLogin(newOwner)
86+
return azpg.pg.DropRole(role, azNewOwner, database, logger)
8887
}

pkg/postgres/role.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const (
1111
CREATE_GROUP_ROLE = `CREATE ROLE "%s"`
1212
CREATE_USER_ROLE = `CREATE ROLE "%s" WITH LOGIN PASSWORD '%s'`
1313
GRANT_ROLE = `GRANT "%s" TO "%s"`
14+
GRANT_ADMIN_ROLE = `GRANT "%s" TO "%s" WITH ADMIN OPTION`
1415
ALTER_USER_SET_ROLE = `ALTER USER "%s" SET ROLE "%s"`
1516
REVOKE_ROLE = `REVOKE "%s" FROM "%s"`
1617
UPDATE_PASSWORD = `ALTER ROLE "%s" WITH PASSWORD '%s'`
@@ -44,6 +45,14 @@ func (c *pg) GrantRole(role, grantee string) error {
4445
return nil
4546
}
4647

48+
func (c *pg) grantAdminRole(role, grantee string) error {
49+
_, err := c.db.Exec(fmt.Sprintf(GRANT_ADMIN_ROLE, role, grantee))
50+
if err != nil {
51+
return err
52+
}
53+
return nil
54+
}
55+
4756
func (c *pg) AlterDefaultLoginRole(role, setRole string) error {
4857
_, err := c.db.Exec(fmt.Sprintf(ALTER_USER_SET_ROLE, role, setRole))
4958
if err != nil {

0 commit comments

Comments
 (0)