Fix DefaultPrivileges Observe to filter by target role and schema

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 {

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)
+		})
+	}
+}

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 {

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)
+		})
+	}
+}