From a37eb1011d45471146227d0b0561a441c2ba2f7d Mon Sep 17 00:00:00 2001 From: Shashank Varma <324153016+shashankvarma499@users.noreply.github.com> Date: Sun, 13 Sep 2026 07:09:09 +0000 Subject: [PATCH] Fix DefaultPrivileges Observe to filter by target role and schema The Observe SELECT query only filtered by object type and grantee role, so default privileges for a different target role or schema that happen to grant the same privileges to the same grantee were mistaken for this resource's grants. Observe then reported the resource as up-to-date and Create (which issues the ALTER DEFAULT PRIVILEGES) was never called, leaving the privileges unapplied while the resource showed Ready/Synced. Filter by defaclrole (FOR ROLE target) and defaclnamespace (IN SCHEMA, or the database-wide namespace oid 0 when objectType is schema). Signed-off-by: Shashank Varma <324153016+shashankvarma499@users.noreply.github.com> --- .../default_privileges/reconciler.go | 31 +++++++--- .../default_privileges/reconciler_test.go | 62 +++++++++++++++++++ .../default_privileges/reconciler.go | 31 +++++++--- .../default_privileges/reconciler_test.go | 62 +++++++++++++++++++ 4 files changed, 172 insertions(+), 14 deletions(-) diff --git a/pkg/controller/cluster/postgresql/default_privileges/reconciler.go b/pkg/controller/cluster/postgresql/default_privileges/reconciler.go index 410c385d..004ab438 100644 --- a/pkg/controller/cluster/postgresql/default_privileges/reconciler.go +++ b/pkg/controller/cluster/postgresql/default_privileges/reconciler.go @@ -153,20 +153,37 @@ var ( ) func selectDefaultPrivilegesQuery(gp v1alpha1.DefaultPrivilegesParameters, q *xsql.Query) { + // Filter by object type, grantee (TO role), target role (FOR ROLE), and + // schema (IN SCHEMA). Without the target-role and schema filters, default + // privileges for a different target role or schema that happen to grant the + // same privileges to the same grantee are mistaken for this resource's + // grants, so Observe reports the resource as up-to-date and Create is never + // called. sqlString := ` select distinct(default_acl.privilege_type) - from pg_roles r - join (SELECT defaclnamespace, (aclexplode(defaclacl)).* FROM pg_default_acl + from pg_roles grantee + join (SELECT defaclrole, defaclnamespace, (aclexplode(defaclacl)).* FROM pg_default_acl WHERE defaclobjtype = $1) default_acl - on r.oid = default_acl.grantee - where r.rolname = $2; + on grantee.oid = default_acl.grantee + join pg_roles target_role + on target_role.oid = default_acl.defaclrole + where grantee.rolname = $2 + and target_role.rolname = $3 ` - q.String = sqlString - q.Parameters = []interface{}{ + params := []interface{}{ objectTypes[*gp.ObjectType], *gp.Role, + *gp.TargetRole, } - + if gp.Schema != nil { + sqlString += ` and default_acl.defaclnamespace = (select oid from pg_namespace where nspname = $4)` + params = append(params, *gp.Schema) + } else { + sqlString += ` and default_acl.defaclnamespace = 0` + } + sqlString += `;` + q.String = sqlString + q.Parameters = params } func withOption(option *v1alpha1.GrantOption) string { diff --git a/pkg/controller/cluster/postgresql/default_privileges/reconciler_test.go b/pkg/controller/cluster/postgresql/default_privileges/reconciler_test.go index 93f61906..8f828ddc 100644 --- a/pkg/controller/cluster/postgresql/default_privileges/reconciler_test.go +++ b/pkg/controller/cluster/postgresql/default_privileges/reconciler_test.go @@ -958,3 +958,65 @@ func TestDelete(t *testing.T) { }) } } + +func TestSelectDefaultPrivilegesQuery(t *testing.T) { + type args struct { + gp v1alpha1.DefaultPrivilegesParameters + } + + tests := map[string]struct { + reason string + args args + want func(t *testing.T, q xsql.Query) + }{ + "FiltersByTargetRoleAndSchema": { + reason: "The SELECT query must filter by target role and schema so unrelated default privileges are not matched", + args: args{gp: v1alpha1.DefaultPrivilegesParameters{ + Role: ptr.To("grantee-role"), + TargetRole: ptr.To("target-role"), + ObjectType: ptr.To("table"), + Schema: ptr.To("myschema"), + }}, + want: func(t *testing.T, q xsql.Query) { + for _, want := range []string{ + "target_role.rolname = $3", + "default_acl.defaclnamespace = (select oid from pg_namespace where nspname = $4)", + } { + if !strings.Contains(q.String, want) { + t.Errorf("query should contain %q, got:\n%s", want, q.String) + } + } + if diff := cmp.Diff([]interface{}{"r", "grantee-role", "target-role", "myschema"}, q.Parameters); diff != "" { + t.Errorf("unexpected parameters (-want +got):\n%s", diff) + } + }, + }, + "SchemaObjectTypeUsesDatabaseWideNamespace": { + reason: "For objectType schema there is no IN SCHEMA, so the query must filter the database-wide namespace (oid 0)", + args: args{gp: v1alpha1.DefaultPrivilegesParameters{ + Role: ptr.To("grantee-role"), + TargetRole: ptr.To("target-role"), + ObjectType: ptr.To("schema"), + }}, + want: func(t *testing.T, q xsql.Query) { + if !strings.Contains(q.String, "default_acl.defaclnamespace = 0") { + t.Errorf("query should filter the database-wide namespace, got:\n%s", q.String) + } + if strings.Contains(q.String, "pg_namespace") { + t.Errorf("query should not filter by schema when objectType is schema, got:\n%s", q.String) + } + if diff := cmp.Diff([]interface{}{"n", "grantee-role", "target-role"}, q.Parameters); diff != "" { + t.Errorf("unexpected parameters (-want +got):\n%s", diff) + } + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + var q xsql.Query + selectDefaultPrivilegesQuery(tc.args.gp, &q) + tc.want(t, q) + }) + } +} diff --git a/pkg/controller/namespaced/postgresql/default_privileges/reconciler.go b/pkg/controller/namespaced/postgresql/default_privileges/reconciler.go index 637fb72b..aea771e7 100644 --- a/pkg/controller/namespaced/postgresql/default_privileges/reconciler.go +++ b/pkg/controller/namespaced/postgresql/default_privileges/reconciler.go @@ -139,20 +139,37 @@ var ( ) func selectDefaultPrivilegesQuery(gp v1alpha1.DefaultPrivilegesParameters, q *xsql.Query) { + // Filter by object type, grantee (TO role), target role (FOR ROLE), and + // schema (IN SCHEMA). Without the target-role and schema filters, default + // privileges for a different target role or schema that happen to grant the + // same privileges to the same grantee are mistaken for this resource's + // grants, so Observe reports the resource as up-to-date and Create is never + // called. sqlString := ` select distinct(default_acl.privilege_type) - from pg_roles r - join (SELECT defaclnamespace, (aclexplode(defaclacl)).* FROM pg_default_acl + from pg_roles grantee + join (SELECT defaclrole, defaclnamespace, (aclexplode(defaclacl)).* FROM pg_default_acl WHERE defaclobjtype = $1) default_acl - on r.oid = default_acl.grantee - where r.rolname = $2; + on grantee.oid = default_acl.grantee + join pg_roles target_role + on target_role.oid = default_acl.defaclrole + where grantee.rolname = $2 + and target_role.rolname = $3 ` - q.String = sqlString - q.Parameters = []interface{}{ + params := []interface{}{ objectTypes[*gp.ObjectType], *gp.Role, + *gp.TargetRole, } - + if gp.Schema != nil { + sqlString += ` and default_acl.defaclnamespace = (select oid from pg_namespace where nspname = $4)` + params = append(params, *gp.Schema) + } else { + sqlString += ` and default_acl.defaclnamespace = 0` + } + sqlString += `;` + q.String = sqlString + q.Parameters = params } func withOption(option *v1alpha1.GrantOption) string { diff --git a/pkg/controller/namespaced/postgresql/default_privileges/reconciler_test.go b/pkg/controller/namespaced/postgresql/default_privileges/reconciler_test.go index f5ee9885..6f4a0844 100644 --- a/pkg/controller/namespaced/postgresql/default_privileges/reconciler_test.go +++ b/pkg/controller/namespaced/postgresql/default_privileges/reconciler_test.go @@ -1006,3 +1006,65 @@ func TestDelete(t *testing.T) { }) } } + +func TestSelectDefaultPrivilegesQuery(t *testing.T) { + type args struct { + gp v1alpha1.DefaultPrivilegesParameters + } + + tests := map[string]struct { + reason string + args args + want func(t *testing.T, q xsql.Query) + }{ + "FiltersByTargetRoleAndSchema": { + reason: "The SELECT query must filter by target role and schema so unrelated default privileges are not matched", + args: args{gp: v1alpha1.DefaultPrivilegesParameters{ + Role: ptr.To("grantee-role"), + TargetRole: ptr.To("target-role"), + ObjectType: ptr.To("table"), + Schema: ptr.To("myschema"), + }}, + want: func(t *testing.T, q xsql.Query) { + for _, want := range []string{ + "target_role.rolname = $3", + "default_acl.defaclnamespace = (select oid from pg_namespace where nspname = $4)", + } { + if !strings.Contains(q.String, want) { + t.Errorf("query should contain %q, got:\n%s", want, q.String) + } + } + if diff := cmp.Diff([]interface{}{"r", "grantee-role", "target-role", "myschema"}, q.Parameters); diff != "" { + t.Errorf("unexpected parameters (-want +got):\n%s", diff) + } + }, + }, + "SchemaObjectTypeUsesDatabaseWideNamespace": { + reason: "For objectType schema there is no IN SCHEMA, so the query must filter the database-wide namespace (oid 0)", + args: args{gp: v1alpha1.DefaultPrivilegesParameters{ + Role: ptr.To("grantee-role"), + TargetRole: ptr.To("target-role"), + ObjectType: ptr.To("schema"), + }}, + want: func(t *testing.T, q xsql.Query) { + if !strings.Contains(q.String, "default_acl.defaclnamespace = 0") { + t.Errorf("query should filter the database-wide namespace, got:\n%s", q.String) + } + if strings.Contains(q.String, "pg_namespace") { + t.Errorf("query should not filter by schema when objectType is schema, got:\n%s", q.String) + } + if diff := cmp.Diff([]interface{}{"n", "grantee-role", "target-role"}, q.Parameters); diff != "" { + t.Errorf("unexpected parameters (-want +got):\n%s", diff) + } + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + var q xsql.Query + selectDefaultPrivilegesQuery(tc.args.gp, &q) + tc.want(t, q) + }) + } +}