From 5ec83622b23a9c311a88acca2ef23c48c09366d4 Mon Sep 17 00:00:00 2001 From: Justin Gallardo Date: Wed, 8 Oct 2025 14:39:34 -0700 Subject: [PATCH 01/10] Check and remove grants and role memberships before removing a role --- pkg/postgres/roles.go | 209 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 205 insertions(+), 4 deletions(-) diff --git a/pkg/postgres/roles.go b/pkg/postgres/roles.go index 45f25a79..07088695 100644 --- a/pkg/postgres/roles.go +++ b/pkg/postgres/roles.go @@ -157,19 +157,220 @@ func (c *Client) CreateRole(ctx context.Context, roleName string) error { return err } -func (c *Client) DeleteRole(ctx context.Context, roleName string) error { +// RoleOwnsObjects checks if a role owns any database objects. +func (c *Client) RoleOwnsObjects(ctx context.Context, roleName string) (bool, error) { + l := ctxzap.Extract(ctx) + + query := ` + SELECT EXISTS( + SELECT 1 FROM ( + -- Check for owned schemas + SELECT 1 FROM pg_namespace WHERE nspowner = (SELECT oid FROM pg_roles WHERE rolname = $1) + UNION ALL + -- Check for owned tables + SELECT 1 FROM pg_class WHERE relowner = (SELECT oid FROM pg_roles WHERE rolname = $1) + UNION ALL + -- Check for owned functions + SELECT 1 FROM pg_proc WHERE proowner = (SELECT oid FROM pg_roles WHERE rolname = $1) + UNION ALL + -- Check for owned sequences + SELECT 1 FROM pg_class WHERE relowner = (SELECT oid FROM pg_roles WHERE rolname = $1) AND relkind = 'S' + UNION ALL + -- Check for owned views + SELECT 1 FROM pg_class WHERE relowner = (SELECT oid FROM pg_roles WHERE rolname = $1) AND relkind = 'v' + UNION ALL + -- Check for owned types + SELECT 1 FROM pg_type WHERE typowner = (SELECT oid FROM pg_roles WHERE rolname = $1) + UNION ALL + -- Check for owned databases + SELECT 1 FROM pg_database WHERE datdba = (SELECT oid FROM pg_roles WHERE rolname = $1) + ) owned_objects + )` + + var ownsObjects bool + err := c.db.QueryRow(ctx, query, roleName).Scan(&ownsObjects) + if err != nil { + l.Error("error checking if role owns objects", zap.Error(err)) + return false, err + } + + return ownsObjects, nil +} + +// RevokeAllGrantsFromRole revokes all grants from a role across all schemas. +func (c *Client) RevokeAllGrantsFromRole(ctx context.Context, roleName string) error { + l := ctxzap.Extract(ctx) + + sanitizedRoleName := pgx.Identifier{roleName}.Sanitize() + + schemasQuery := ` + SELECT nspname + FROM pg_namespace + WHERE nspname NOT LIKE 'pg_%' + AND nspname != 'information_schema' + ORDER BY nspname` + + rows, err := c.db.Query(ctx, schemasQuery) + if err != nil { + l.Error("error querying schemas", zap.Error(err)) + return err + } + defer rows.Close() + + var schemas []string + for rows.Next() { + var schemaName string + if err := rows.Scan(&schemaName); err != nil { + l.Error("error scanning schema name", zap.Error(err)) + return err + } + schemas = append(schemas, schemaName) + } + + if err := rows.Err(); err != nil { + l.Error("error iterating schemas", zap.Error(err)) + return err + } + + for _, schema := range schemas { + sanitizedSchema := pgx.Identifier{schema}.Sanitize() + + revokeTablesQuery := fmt.Sprintf("REVOKE ALL ON ALL TABLES IN SCHEMA %s FROM %s", sanitizedSchema, sanitizedRoleName) + l.Debug("revoking table grants", zap.String("query", revokeTablesQuery)) + if _, err := c.db.Exec(ctx, revokeTablesQuery); err != nil { + l.Warn("error revoking table grants", zap.String("schema", schema), zap.Error(err)) + } + + revokeSequencesQuery := fmt.Sprintf("REVOKE ALL ON ALL SEQUENCES IN SCHEMA %s FROM %s", sanitizedSchema, sanitizedRoleName) + l.Debug("revoking sequence grants", zap.String("query", revokeSequencesQuery)) + if _, err := c.db.Exec(ctx, revokeSequencesQuery); err != nil { + l.Warn("error revoking sequence grants", zap.String("schema", schema), zap.Error(err)) + } + + revokeFunctionsQuery := fmt.Sprintf("REVOKE ALL ON ALL FUNCTIONS IN SCHEMA %s FROM %s", sanitizedSchema, sanitizedRoleName) + l.Debug("revoking function grants", zap.String("query", revokeFunctionsQuery)) + if _, err := c.db.Exec(ctx, revokeFunctionsQuery); err != nil { + l.Warn("error revoking function grants", zap.String("schema", schema), zap.Error(err)) + } + + revokeTypesQuery := fmt.Sprintf("REVOKE ALL ON ALL TYPES IN SCHEMA %s FROM %s", sanitizedSchema, sanitizedRoleName) + l.Debug("revoking type grants", zap.String("query", revokeTypesQuery)) + if _, err := c.db.Exec(ctx, revokeTypesQuery); err != nil { + l.Warn("error revoking type grants", zap.String("schema", schema), zap.Error(err)) + } + + revokeSchemaQuery := fmt.Sprintf("REVOKE ALL ON SCHEMA %s FROM %s", sanitizedSchema, sanitizedRoleName) + l.Debug("revoking schema grants", zap.String("query", revokeSchemaQuery)) + if _, err := c.db.Exec(ctx, revokeSchemaQuery); err != nil { + l.Warn("error revoking schema grants", zap.String("schema", schema), zap.Error(err)) + } + } + + revokeDbQuery := fmt.Sprintf("REVOKE ALL ON DATABASE %s FROM %s", pgx.Identifier{c.DatabaseName()}.Sanitize(), sanitizedRoleName) + l.Debug("revoking database grants", zap.String("query", revokeDbQuery)) + if _, err := c.db.Exec(ctx, revokeDbQuery); err != nil { + l.Warn("error revoking database grants", zap.Error(err)) + } + + return nil +} + +// RemoveRoleFromAllRoles removes a role from all other roles +func (c *Client) RemoveRoleFromAllRoles(ctx context.Context, roleName string) error { + l := ctxzap.Extract(ctx) + + sanitizedRoleName := pgx.Identifier{roleName}.Sanitize() + + // Get all roles that have this role as a member + query := ` + SELECT r.rolname + FROM pg_roles r + JOIN pg_auth_members am ON r.oid = am.roleid + JOIN pg_roles member ON am.member = member.oid + WHERE member.rolname = $1` + + rows, err := c.db.Query(ctx, query, roleName) + if err != nil { + l.Error("error querying role memberships", zap.Error(err)) + return err + } + defer rows.Close() + + var parentRoles []string + for rows.Next() { + var parentRole string + if err := rows.Scan(&parentRole); err != nil { + l.Error("error scanning parent role", zap.Error(err)) + return err + } + parentRoles = append(parentRoles, parentRole) + } + + if err := rows.Err(); err != nil { + l.Error("error iterating parent roles", zap.Error(err)) + return err + } + + // Remove the role from each parent role + for _, parentRole := range parentRoles { + sanitizedParentRole := pgx.Identifier{parentRole}.Sanitize() + revokeQuery := fmt.Sprintf("REVOKE %s FROM %s", sanitizedParentRole, sanitizedRoleName) + + l.Debug("removing role from parent role", zap.String("query", revokeQuery)) + if _, err := c.db.Exec(ctx, revokeQuery); err != nil { + l.Error("error removing role from parent role", zap.String("parent_role", parentRole), zap.Error(err)) + return err + } + } + + return nil +} + +// SafeDeleteRole safely deletes a role by first revoking grants and removing memberships. +func (c *Client) SafeDeleteRole(ctx context.Context, roleName string) error { l := ctxzap.Extract(ctx) if roleName == "" { return errors.New("role name cannot be empty") } + ownsObjects, err := c.RoleOwnsObjects(ctx, roleName) + if err != nil { + l.Error("error checking if role owns objects", zap.Error(err)) + return err + } + + if ownsObjects { + return fmt.Errorf("cannot delete role '%s': role owns database objects (tables, schemas, functions, etc.). Please transfer ownership or drop objects first", roleName) + } + + l.Debug("revoking all grants from role", zap.String("role", roleName)) + if err := c.RevokeAllGrantsFromRole(ctx, roleName); err != nil { + l.Error("error revoking grants from role", zap.Error(err)) + return err + } + + l.Debug("removing role from all parent roles", zap.String("role", roleName)) + if err := c.RemoveRoleFromAllRoles(ctx, roleName); err != nil { + l.Error("error removing role from parent roles", zap.Error(err)) + return err + } + sanitizedRoleName := pgx.Identifier{roleName}.Sanitize() query := "DROP ROLE " + sanitizedRoleName + l.Debug("dropping role", zap.String("query", query)) + _, err = c.db.Exec(ctx, query) + if err != nil { + l.Error("error dropping role", zap.Error(err)) + return err + } - l.Debug("deleting role", zap.String("query", query)) - _, err := c.db.Exec(ctx, query) - return err + l.Info("successfully deleted role", zap.String("role", roleName)) + return nil +} + +func (c *Client) DeleteRole(ctx context.Context, roleName string) error { + return c.SafeDeleteRole(ctx, roleName) } func (c *Client) CreateUser(ctx context.Context, login string, password string) (*RoleModel, error) { From 7d39cc95474a9aa1ea204845059fe71e4d87fcc8 Mon Sep 17 00:00:00 2001 From: Justin Gallardo Date: Wed, 8 Oct 2025 15:00:47 -0700 Subject: [PATCH 02/10] Remove type grants explicitly since postgres doesn't support revoking all types at once. --- pkg/postgres/roles.go | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/pkg/postgres/roles.go b/pkg/postgres/roles.go index 07088695..2ce82921 100644 --- a/pkg/postgres/roles.go +++ b/pkg/postgres/roles.go @@ -253,10 +253,33 @@ func (c *Client) RevokeAllGrantsFromRole(ctx context.Context, roleName string) e l.Warn("error revoking function grants", zap.String("schema", schema), zap.Error(err)) } - revokeTypesQuery := fmt.Sprintf("REVOKE ALL ON ALL TYPES IN SCHEMA %s FROM %s", sanitizedSchema, sanitizedRoleName) - l.Debug("revoking type grants", zap.String("query", revokeTypesQuery)) - if _, err := c.db.Exec(ctx, revokeTypesQuery); err != nil { - l.Warn("error revoking type grants", zap.String("schema", schema), zap.Error(err)) + typesQuery := ` + SELECT typname + FROM pg_type t + JOIN pg_namespace n ON t.typnamespace = n.oid + WHERE n.nspname = $1 + AND t.typtype = 'c'` + + typeRows, err := c.db.Query(ctx, typesQuery, schema) + if err != nil { + l.Warn("error querying types", zap.String("schema", schema), zap.Error(err)) + } else { + defer typeRows.Close() + + for typeRows.Next() { + var typeName string + if err := typeRows.Scan(&typeName); err != nil { + l.Warn("error scanning type name", zap.String("schema", schema), zap.Error(err)) + continue + } + + sanitizedTypeName := pgx.Identifier{schema, typeName}.Sanitize() + revokeTypeQuery := fmt.Sprintf("REVOKE ALL ON TYPE %s FROM %s", sanitizedTypeName, sanitizedRoleName) + l.Debug("revoking type grants", zap.String("query", revokeTypeQuery)) + if _, err := c.db.Exec(ctx, revokeTypeQuery); err != nil { + l.Warn("error revoking type grants", zap.String("schema", schema), zap.String("type", typeName), zap.Error(err)) + } + } } revokeSchemaQuery := fmt.Sprintf("REVOKE ALL ON SCHEMA %s FROM %s", sanitizedSchema, sanitizedRoleName) From 26590037df88ac6447b52ff4d1377431929a50e6 Mon Sep 17 00:00:00 2001 From: Lauren Leach Date: Fri, 10 Oct 2025 17:44:02 -0700 Subject: [PATCH 03/10] add period to comment --- pkg/postgres/roles.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/postgres/roles.go b/pkg/postgres/roles.go index 2ce82921..7f7fffd1 100644 --- a/pkg/postgres/roles.go +++ b/pkg/postgres/roles.go @@ -298,7 +298,7 @@ func (c *Client) RevokeAllGrantsFromRole(ctx context.Context, roleName string) e return nil } -// RemoveRoleFromAllRoles removes a role from all other roles +// RemoveRoleFromAllRoles removes a role from all other roles. func (c *Client) RemoveRoleFromAllRoles(ctx context.Context, roleName string) error { l := ctxzap.Extract(ctx) From 63a79a994b39843b8fddc57c1808065bf8733a41 Mon Sep 17 00:00:00 2001 From: Justin Gallardo Date: Tue, 14 Oct 2025 12:07:00 -0700 Subject: [PATCH 04/10] Better error handling --- pkg/postgres/roles.go | 44 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/pkg/postgres/roles.go b/pkg/postgres/roles.go index 7f7fffd1..10fa5f44 100644 --- a/pkg/postgres/roles.go +++ b/pkg/postgres/roles.go @@ -14,6 +14,9 @@ import ( "go.uber.org/zap" ) +var errRevokeGrantsFromRole = errors.New("error revoking grants from role") +var errRevokeParentRolesFromRole = errors.New("error revoking parent roles from role") + type RoleModel struct { ID int64 `db:"oid"` Name string `db:"rolname"` @@ -232,6 +235,7 @@ func (c *Client) RevokeAllGrantsFromRole(ctx context.Context, roleName string) e return err } + var revokeError error for _, schema := range schemas { sanitizedSchema := pgx.Identifier{schema}.Sanitize() @@ -239,18 +243,21 @@ func (c *Client) RevokeAllGrantsFromRole(ctx context.Context, roleName string) e l.Debug("revoking table grants", zap.String("query", revokeTablesQuery)) if _, err := c.db.Exec(ctx, revokeTablesQuery); err != nil { l.Warn("error revoking table grants", zap.String("schema", schema), zap.Error(err)) + revokeError = errors.Join(revokeError, err) } revokeSequencesQuery := fmt.Sprintf("REVOKE ALL ON ALL SEQUENCES IN SCHEMA %s FROM %s", sanitizedSchema, sanitizedRoleName) l.Debug("revoking sequence grants", zap.String("query", revokeSequencesQuery)) if _, err := c.db.Exec(ctx, revokeSequencesQuery); err != nil { l.Warn("error revoking sequence grants", zap.String("schema", schema), zap.Error(err)) + revokeError = errors.Join(revokeError, err) } revokeFunctionsQuery := fmt.Sprintf("REVOKE ALL ON ALL FUNCTIONS IN SCHEMA %s FROM %s", sanitizedSchema, sanitizedRoleName) l.Debug("revoking function grants", zap.String("query", revokeFunctionsQuery)) if _, err := c.db.Exec(ctx, revokeFunctionsQuery); err != nil { l.Warn("error revoking function grants", zap.String("schema", schema), zap.Error(err)) + revokeError = errors.Join(revokeError, err) } typesQuery := ` @@ -263,6 +270,7 @@ func (c *Client) RevokeAllGrantsFromRole(ctx context.Context, roleName string) e typeRows, err := c.db.Query(ctx, typesQuery, schema) if err != nil { l.Warn("error querying types", zap.String("schema", schema), zap.Error(err)) + revokeError = errors.Join(revokeError, err) } else { defer typeRows.Close() @@ -270,6 +278,7 @@ func (c *Client) RevokeAllGrantsFromRole(ctx context.Context, roleName string) e var typeName string if err := typeRows.Scan(&typeName); err != nil { l.Warn("error scanning type name", zap.String("schema", schema), zap.Error(err)) + revokeError = errors.Join(revokeError, err) continue } @@ -278,6 +287,7 @@ func (c *Client) RevokeAllGrantsFromRole(ctx context.Context, roleName string) e l.Debug("revoking type grants", zap.String("query", revokeTypeQuery)) if _, err := c.db.Exec(ctx, revokeTypeQuery); err != nil { l.Warn("error revoking type grants", zap.String("schema", schema), zap.String("type", typeName), zap.Error(err)) + revokeError = errors.Join(revokeError, err) } } } @@ -286,6 +296,7 @@ func (c *Client) RevokeAllGrantsFromRole(ctx context.Context, roleName string) e l.Debug("revoking schema grants", zap.String("query", revokeSchemaQuery)) if _, err := c.db.Exec(ctx, revokeSchemaQuery); err != nil { l.Warn("error revoking schema grants", zap.String("schema", schema), zap.Error(err)) + revokeError = errors.Join(revokeError, err) } } @@ -293,6 +304,11 @@ func (c *Client) RevokeAllGrantsFromRole(ctx context.Context, roleName string) e l.Debug("revoking database grants", zap.String("query", revokeDbQuery)) if _, err := c.db.Exec(ctx, revokeDbQuery); err != nil { l.Warn("error revoking database grants", zap.Error(err)) + revokeError = errors.Join(revokeError, err) + } + + if revokeError != nil { + return errors.Join(errRevokeGrantsFromRole, revokeError) } return nil @@ -334,6 +350,7 @@ func (c *Client) RemoveRoleFromAllRoles(ctx context.Context, roleName string) er return err } + var revokeError error // Remove the role from each parent role for _, parentRole := range parentRoles { sanitizedParentRole := pgx.Identifier{parentRole}.Sanitize() @@ -342,10 +359,14 @@ func (c *Client) RemoveRoleFromAllRoles(ctx context.Context, roleName string) er l.Debug("removing role from parent role", zap.String("query", revokeQuery)) if _, err := c.db.Exec(ctx, revokeQuery); err != nil { l.Error("error removing role from parent role", zap.String("parent_role", parentRole), zap.Error(err)) - return err + revokeError = errors.Join(revokeError, fmt.Errorf("error removing role from %s role: %w", parentRole, err)) } } + if revokeError != nil { + return errors.Join(errRevokeParentRolesFromRole, revokeError) + } + return nil } @@ -368,15 +389,21 @@ func (c *Client) SafeDeleteRole(ctx context.Context, roleName string) error { } l.Debug("revoking all grants from role", zap.String("role", roleName)) - if err := c.RevokeAllGrantsFromRole(ctx, roleName); err != nil { - l.Error("error revoking grants from role", zap.Error(err)) - return err + grantsRevokeError := c.RevokeAllGrantsFromRole(ctx, roleName) + if grantsRevokeError != nil { + l.Error("error revoking grants from role", zap.Error(grantsRevokeError)) + if !errors.Is(grantsRevokeError, errRevokeGrantsFromRole) { + return fmt.Errorf("error revoking existing grants from role: %w", err) + } } l.Debug("removing role from all parent roles", zap.String("role", roleName)) - if err := c.RemoveRoleFromAllRoles(ctx, roleName); err != nil { - l.Error("error removing role from parent roles", zap.Error(err)) - return err + roleRevokeError := c.RemoveRoleFromAllRoles(ctx, roleName) + if roleRevokeError != nil { + l.Error("error removing role from parent roles", zap.Error(roleRevokeError)) + if !errors.Is(roleRevokeError, errRevokeParentRolesFromRole) { + return fmt.Errorf("error removing role from parent roles: %w", roleRevokeError) + } } sanitizedRoleName := pgx.Identifier{roleName}.Sanitize() @@ -385,7 +412,8 @@ func (c *Client) SafeDeleteRole(ctx context.Context, roleName string) error { _, err = c.db.Exec(ctx, query) if err != nil { l.Error("error dropping role", zap.Error(err)) - return err + finalError := errors.Join(err, roleRevokeError, grantsRevokeError) + return fmt.Errorf("error dropping role(%s): %w", roleName, finalError) } l.Info("successfully deleted role", zap.String("role", roleName)) From 104813d363d5ac3e2a4d9ed8d7b8e158fbbeaae7 Mon Sep 17 00:00:00 2001 From: Justin Gallardo Date: Tue, 14 Oct 2025 13:49:24 -0700 Subject: [PATCH 05/10] Attempt at tests for creating and deleting user --- .github/workflows/ci.yaml | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index f7ebb363..746b0739 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -47,11 +47,12 @@ jobs: POSTGRES_PASSWORD: secretpassword env: BATON_LOG_LEVEL: debug - BATON_DSN: 'postgres://postgres:secretpassword@localhost:5432/postgres' - CONNECTOR_GRANT: 'grant:entitlement:role:3375:member:role:10' - CONNECTOR_ENTITLEMENT: 'entitlement:role:3375:member' - CONNECTOR_PRINCIPAL: 'role:10' - CONNECTOR_PRINCIPAL_TYPE: 'role' + BATON_DSN: "postgres://postgres:secretpassword@localhost:5432/postgres" + CONNECTOR_GRANT: "grant:entitlement:role:3375:member:role:10" + CONNECTOR_ENTITLEMENT: "entitlement:role:3375:member" + CONNECTOR_PRINCIPAL: "role:10" + CONNECTOR_PRINCIPAL_TYPE: "role" + CONNECTOR_NEW_USER: "testuser" steps: - name: Install Go uses: actions/setup-go@v5 @@ -63,7 +64,7 @@ jobs: run: sudo apt install postgresql-client # - name: Import sql into postgres # env: - # PGPASSWORD: secretpassword + # PGPASSWORD: secretpassword # run: psql -h localhost --user postgres -f test/ci.sql - name: Install baton run: ./scripts/get-baton.sh && mv baton /usr/local/bin @@ -91,7 +92,30 @@ jobs: run: ./baton-postgresql && baton grants --entitlement "${{ env.CONNECTOR_ENTITLEMENT }}" --output-format=json | jq --exit-status ".grants[].principal.id.resource == \"${{ env.CONNECTOR_PRINCIPAL }}\"" - name: Create user - run: ./baton-postgresql --create-account-login 'testuser' + run: ./baton-postgresql --create-account-login "${{ env.CONNECTOR_NEW_USER }}" + + - name: Check user was created + run: ./baton-postgresql && baton resources -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.resources[].resource.annotations[]?;.["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login)' + + - name: Fetch user id + shell: bash + run: | + set -eub pipefail + NEW_USER_ID="$(baton resources -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.resources[].resource.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login)')" + echo "NEW_USER_ID=$NEW_USER_ID" >> "$GITHUB_ENV" + + - name: Grant role to user + run: ./baton-postgresql --grant-entitlement "${{ env.CONNECTOR_ENTITLEMENT }}" --grant-principal "${{ env.NEW_USER_ID }}" --grant-principal-type "${{ env.CONNECTOR_PRINCIPAL_TYPE }}" + + - name: Check role was granted + run: ./baton-postgresql && baton grants -e entitlement:role:16390:member -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.grants[]?; any(.principal.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login) or any(.grant.principal.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login))' + + - name: Delete user + run: ./baton-postgresql --delete-resource "${{ env.NEW_USER_ID }}" --resource-type "${{ env.CONNECTOR_PRINCIPAL_TYPE }}" + + - name: Check user was deleted + run: ./baton-postgresql && baton resources -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.resources[].resource.annotations[]?;.["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login) | not' + # TODO: get correct role id using baton CLI # - name: Rotate credentials for user # run: ./baton-postgresql --rotate-credentials 'role:16384' --rotate-credentials-type 'role' From 9990ace447f02936ff84d0fbc9a6454cec6e8ae4 Mon Sep 17 00:00:00 2001 From: Justin Gallardo Date: Tue, 14 Oct 2025 13:54:14 -0700 Subject: [PATCH 06/10] Don't hardcode entitlement in test --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 746b0739..fe629d0a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -108,7 +108,7 @@ jobs: run: ./baton-postgresql --grant-entitlement "${{ env.CONNECTOR_ENTITLEMENT }}" --grant-principal "${{ env.NEW_USER_ID }}" --grant-principal-type "${{ env.CONNECTOR_PRINCIPAL_TYPE }}" - name: Check role was granted - run: ./baton-postgresql && baton grants -e entitlement:role:16390:member -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.grants[]?; any(.principal.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login) or any(.grant.principal.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login))' + run: ./baton-postgresql && baton grants --entitlement "${{ env.CONNECTOR_ENTITLEMENT }}" -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.grants[]?; any(.principal.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login) or any(.grant.principal.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login))' - name: Delete user run: ./baton-postgresql --delete-resource "${{ env.NEW_USER_ID }}" --resource-type "${{ env.CONNECTOR_PRINCIPAL_TYPE }}" From 657a821c3696a33cecae8ef570d01b1e6d4f48b8 Mon Sep 17 00:00:00 2001 From: Justin Gallardo Date: Tue, 14 Oct 2025 13:58:27 -0700 Subject: [PATCH 07/10] Pull resource id when fetching the new user id --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index fe629d0a..b5ab72fd 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -101,7 +101,7 @@ jobs: shell: bash run: | set -eub pipefail - NEW_USER_ID="$(baton resources -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.resources[].resource.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login)')" + NEW_USER_ID="$(baton resources -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.resources[].resource.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login) | .id.resource')" echo "NEW_USER_ID=$NEW_USER_ID" >> "$GITHUB_ENV" - name: Grant role to user From a16d446d9c3146a7f27af13dea916bedd162cc61 Mon Sep 17 00:00:00 2001 From: Justin Gallardo Date: Tue, 14 Oct 2025 15:31:56 -0700 Subject: [PATCH 08/10] Fix fetching user id --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b5ab72fd..97d96e87 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -101,7 +101,7 @@ jobs: shell: bash run: | set -eub pipefail - NEW_USER_ID="$(baton resources -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.resources[].resource.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login) | .id.resource')" + NEW_USER_ID=$(baton resources -t role -o json | jq -r --arg login "${{ env.CONNECTOR_NEW_USER }}" '.resources[].resource | select(any(.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login)) | .id.resource') echo "NEW_USER_ID=$NEW_USER_ID" >> "$GITHUB_ENV" - name: Grant role to user From d0bf5461191387548dd97d67576c4206a3834058 Mon Sep 17 00:00:00 2001 From: Justin Gallardo Date: Tue, 14 Oct 2025 15:35:02 -0700 Subject: [PATCH 09/10] Fix flag in delete user command. --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 97d96e87..0fb59ef1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -111,7 +111,7 @@ jobs: run: ./baton-postgresql && baton grants --entitlement "${{ env.CONNECTOR_ENTITLEMENT }}" -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.grants[]?; any(.principal.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login) or any(.grant.principal.annotations[]?; .["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login))' - name: Delete user - run: ./baton-postgresql --delete-resource "${{ env.NEW_USER_ID }}" --resource-type "${{ env.CONNECTOR_PRINCIPAL_TYPE }}" + run: ./baton-postgresql --delete-resource "${{ env.NEW_USER_ID }}" --delete-resource-type "${{ env.CONNECTOR_PRINCIPAL_TYPE }}" - name: Check user was deleted run: ./baton-postgresql && baton resources -o json | jq -e --arg login "${{ env.CONNECTOR_NEW_USER }}" 'any(.resources[].resource.annotations[]?;.["@type"]=="type.googleapis.com/c1.connector.v2.UserTrait" and .login==$login) | not' From 7c016d282af93541cbdb39781a4b0b4d6e84fd81 Mon Sep 17 00:00:00 2001 From: Justin Gallardo Date: Tue, 14 Oct 2025 15:43:27 -0700 Subject: [PATCH 10/10] Return correct error --- pkg/postgres/roles.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/postgres/roles.go b/pkg/postgres/roles.go index 10fa5f44..25088073 100644 --- a/pkg/postgres/roles.go +++ b/pkg/postgres/roles.go @@ -393,7 +393,7 @@ func (c *Client) SafeDeleteRole(ctx context.Context, roleName string) error { if grantsRevokeError != nil { l.Error("error revoking grants from role", zap.Error(grantsRevokeError)) if !errors.Is(grantsRevokeError, errRevokeGrantsFromRole) { - return fmt.Errorf("error revoking existing grants from role: %w", err) + return fmt.Errorf("error revoking existing grants from role: %w", grantsRevokeError) } }