diff --git a/apis/cluster/postgresql/v1alpha1/grant_types.go b/apis/cluster/postgresql/v1alpha1/grant_types.go index 325701f3..21f542e6 100644 --- a/apis/cluster/postgresql/v1alpha1/grant_types.go +++ b/apis/cluster/postgresql/v1alpha1/grant_types.go @@ -302,6 +302,7 @@ type Routine struct { } // GrantParameters define the desired state of a PostgreSQL grant instance. +// +kubebuilder:validation:XValidation:rule="!has(self.withInherit) || has(self.memberOf) || has(self.memberOfRef) || has(self.memberOfSelector)",message="withInherit may only be set on memberOf grants" type GrantParameters struct { // Privileges to be granted. // See https://www.postgresql.org/docs/current/sql-grant.html for available privileges. @@ -406,6 +407,15 @@ type GrantParameters struct { // +optional // +kubebuilder:validation:items:Pattern:=^[a-zA-Z_][a-zA-Z0-9_$]*$ ForeignServers []string `json:"foreignServers,omitempty"` + + // WithInherit controls whether the grantee automatically inherits the privileges + // of the granted role. When set to false, emits WITH INHERIT FALSE (PostgreSQL 16+), + // granting membership without automatic privilege inheritance. Only valid when + // memberOf is set. When omitted, PostgreSQL's default behavior (inherit true) applies. + // Note: this field is only evaluated when non-nil. Removing it from the manifest + // does NOT revert the database-side setting; set withInherit: true explicitly to revert. + // +optional + WithInherit *bool `json:"withInherit,omitempty"` } // A GrantStatus represents the observed state of a Grant. diff --git a/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go index dedd46c2..05241d2e 100644 --- a/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -633,6 +633,11 @@ func (in *GrantParameters) DeepCopyInto(out *GrantParameters) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.WithInherit != nil { + in, out := &in.WithInherit, &out.WithInherit + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GrantParameters. diff --git a/apis/namespaced/postgresql/v1alpha1/grant_types.go b/apis/namespaced/postgresql/v1alpha1/grant_types.go index cbfe9f27..99004214 100644 --- a/apis/namespaced/postgresql/v1alpha1/grant_types.go +++ b/apis/namespaced/postgresql/v1alpha1/grant_types.go @@ -304,6 +304,7 @@ type Routine struct { } // GrantParameters define the desired state of a PostgreSQL grant instance. +// +kubebuilder:validation:XValidation:rule="!has(self.withInherit) || has(self.memberOf) || has(self.memberOfRef) || has(self.memberOfSelector)",message="withInherit may only be set on memberOf grants" type GrantParameters struct { // Privileges to be granted. // See https://www.postgresql.org/docs/current/sql-grant.html for available privileges. @@ -408,6 +409,15 @@ type GrantParameters struct { // +optional // +kubebuilder:validation:items:Pattern:=^[a-zA-Z_][a-zA-Z0-9_$]*$ ForeignServers []string `json:"foreignServers,omitempty"` + + // WithInherit controls whether the grantee automatically inherits the privileges + // of the granted role. When set to false, emits WITH INHERIT FALSE (PostgreSQL 16+), + // granting membership without automatic privilege inheritance. Only valid when + // memberOf is set. When omitted, PostgreSQL's default behavior (inherit true) applies. + // Note: this field is only evaluated when non-nil. Removing it from the manifest + // does NOT revert the database-side setting; set withInherit: true explicitly to revert. + // +optional + WithInherit *bool `json:"withInherit,omitempty"` } // A GrantStatus represents the observed state of a Grant. diff --git a/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go index 7a62990c..fa54e26b 100644 --- a/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -750,6 +750,11 @@ func (in *GrantParameters) DeepCopyInto(out *GrantParameters) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.WithInherit != nil { + in, out := &in.WithInherit, &out.WithInherit + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GrantParameters. diff --git a/build b/build index 65819b05..907604dd 160000 --- a/build +++ b/build @@ -1 +1 @@ -Subproject commit 65819b05239a0b22f1a98c3698a14acd22bc3cc0 +Subproject commit 907604dd4d57eba8bcf94699d7834f68389b4f96 diff --git a/examples/cluster/postgresql/grant-with-inherit-false.yaml b/examples/cluster/postgresql/grant-with-inherit-false.yaml new file mode 100644 index 00000000..24442840 --- /dev/null +++ b/examples/cluster/postgresql/grant-with-inherit-false.yaml @@ -0,0 +1,25 @@ +# Demonstrates WITH INHERIT FALSE on a role membership grant (PostgreSQL 16+). +# +# Use case: RDS IAM database authentication with a shared admin role. +# +# 1. An "admin" role holds rds_iam membership so that migration tools +# (Flyway, Liquibase, Atlas) can authenticate via IAM tokens. +# 2. The RDS master user needs membership in "admin" in order to run +# ALTER DEFAULT PRIVILEGES FOR ROLE admin — but must NOT inherit +# the rds_iam privilege, which would break its password authentication. +# +# GRANT admin TO master_user WITH INHERIT FALSE satisfies both requirements: +# the master user can SET ROLE admin (for ALTER DEFAULT PRIVILEGES) while +# retaining its own password-based login. +--- +apiVersion: postgresql.sql.crossplane.io/v1alpha1 +kind: Grant +metadata: + name: grant-master-user-membership-no-inherit +spec: + forProvider: + withInherit: false + roleRef: + name: master-user + memberOfRef: + name: admin-role diff --git a/package/crds/postgresql.sql.crossplane.io_grants.yaml b/package/crds/postgresql.sql.crossplane.io_grants.yaml index 516fb06a..810a7ec5 100644 --- a/package/crds/postgresql.sql.crossplane.io_grants.yaml +++ b/package/crds/postgresql.sql.crossplane.io_grants.yaml @@ -462,6 +462,15 @@ spec: pattern: ^[a-zA-Z_][a-zA-Z0-9_$]*$ type: string type: array + withInherit: + description: |- + WithInherit controls whether the grantee automatically inherits the privileges + of the granted role. When set to false, emits WITH INHERIT FALSE (PostgreSQL 16+), + granting membership without automatic privilege inheritance. Only valid when + memberOf is set. When omitted, PostgreSQL's default behavior (inherit true) applies. + Note: this field is only evaluated when non-nil. Removing it from the manifest + does NOT revert the database-side setting; set withInherit: true explicitly to revert. + type: boolean withOption: description: |- WithOption allows an option to be set on the grant. @@ -472,6 +481,10 @@ spec: - GRANT type: string type: object + x-kubernetes-validations: + - message: withInherit may only be set on memberOf grants + rule: '!has(self.withInherit) || has(self.memberOf) || has(self.memberOfRef) + || has(self.memberOfSelector)' managementPolicies: default: - '*' diff --git a/package/crds/postgresql.sql.m.crossplane.io_grants.yaml b/package/crds/postgresql.sql.m.crossplane.io_grants.yaml index 7dc8c04a..6d140dc4 100644 --- a/package/crds/postgresql.sql.m.crossplane.io_grants.yaml +++ b/package/crds/postgresql.sql.m.crossplane.io_grants.yaml @@ -472,6 +472,15 @@ spec: pattern: ^[a-zA-Z_][a-zA-Z0-9_$]*$ type: string type: array + withInherit: + description: |- + WithInherit controls whether the grantee automatically inherits the privileges + of the granted role. When set to false, emits WITH INHERIT FALSE (PostgreSQL 16+), + granting membership without automatic privilege inheritance. Only valid when + memberOf is set. When omitted, PostgreSQL's default behavior (inherit true) applies. + Note: this field is only evaluated when non-nil. Removing it from the manifest + does NOT revert the database-side setting; set withInherit: true explicitly to revert. + type: boolean withOption: description: |- WithOption allows an option to be set on the grant. @@ -482,6 +491,10 @@ spec: - GRANT type: string type: object + x-kubernetes-validations: + - message: withInherit may only be set on memberOf grants + rule: '!has(self.withInherit) || has(self.memberOf) || has(self.memberOfRef) + || has(self.memberOfSelector)' managementPolicies: default: - '*' diff --git a/pkg/controller/cluster/postgresql/grant/reconciler.go b/pkg/controller/cluster/postgresql/grant/reconciler.go index 81c716b1..802bc3e9 100644 --- a/pkg/controller/cluster/postgresql/grant/reconciler.go +++ b/pkg/controller/cluster/postgresql/grant/reconciler.go @@ -61,6 +61,8 @@ const ( errInvalidParams = "invalid parameters for grant type %s" errGetServerVersion = "cannot get server version" errMemberOfWithDatabaseOrPrivileges = "cannot set privileges or database in the same grant as memberOf" + errWithInheritOnlyForMemberOf = "withInherit is only valid for memberOf grants" + errInheritRequiresPG16 = "withInherit requires PostgreSQL 16 or later (server version %d)" maxConcurrency = 5 ) @@ -280,6 +282,9 @@ func createGrantQueriesWithVersion(gp v1alpha1.GrantParameters, ql *[]xsql.Query case v1alpha1.RoleForeignServer: return createForeignServerGrantQueries(gp, ql, ro) case v1alpha1.RoleMember: + if gp.WithInherit != nil && serverVersion < 160000 { + return errors.Errorf(errInheritRequiresPG16, serverVersion) + } return createMemberGrantQueries(gp, ql, ro) case v1alpha1.RoleRoutine: return createRoutineGrantQueries(gp, ql, ro) @@ -303,12 +308,33 @@ func createMemberGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro *ql = append(*ql, xsql.Query{String: fmt.Sprintf("REVOKE %s FROM %s", mo, ro)}, xsql.Query{String: fmt.Sprintf("GRANT %s TO %s %s", mo, ro, - withOption(gp.WithOption), + membershipWithClauses(gp.WithOption, gp.WithInherit), )}, ) return nil } +// membershipWithClauses builds the WITH clause for role membership GRANTs, +// combining the optional ADMIN/SET option and the optional INHERIT flag. +// On PostgreSQL 16+, multiple options are comma-separated: WITH ADMIN OPTION, INHERIT FALSE. +func membershipWithClauses(option *v1alpha1.GrantOption, inherit *bool) string { + var parts []string + if option != nil { + parts = append(parts, fmt.Sprintf("%s OPTION", string(*option))) + } + if inherit != nil { + if *inherit { + parts = append(parts, "INHERIT TRUE") + } else { + parts = append(parts, "INHERIT FALSE") + } + } + if len(parts) == 0 { + return "" + } + return "WITH " + strings.Join(parts, ", ") +} + func createRoutineGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || gp.Schema == nil || len(gp.Routines) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleRoutine) @@ -603,6 +629,10 @@ func resolveGrantType(gp v1alpha1.GrantParameters) (v1alpha1.GrantType, error) { return v1alpha1.RoleMember, nil } + if gp.WithInherit != nil { + return "", errors.New(errWithInheritOnlyForMemberOf) + } + return gp.IdentifyGrantType() } @@ -771,6 +801,9 @@ func selectGrantQueryWithVersion(gp v1alpha1.GrantParameters, q *xsql.Query, ser case v1alpha1.RoleForeignServer: return selectForeignServerGrantQuery(gp, q) case v1alpha1.RoleMember: + if gp.WithInherit != nil && serverVersion < 160000 { + return errors.Errorf(errInheritRequiresPG16, serverVersion) + } return selectMemberGrantQuery(gp, q) case v1alpha1.RoleRoutine: return selectRoutineGrantQuery(gp, q) @@ -791,17 +824,27 @@ func selectMemberGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { // A simpler query would use ::regrole to cast the roleid and member oids // to their role names, but that throws an error for nonexistent roles // rather than returning false. - q.String = "SELECT EXISTS(SELECT 1 FROM pg_auth_members m " + - "INNER JOIN pg_roles mo ON m.roleid = mo.oid " + - "INNER JOIN pg_roles r ON m.member = r.oid " + - "WHERE r.rolname=$1 AND mo.rolname=$2 AND " + - "m.admin_option = $3)" - q.Parameters = []interface{}{ gp.Role, gp.MemberOf, ao, } + + if gp.WithInherit != nil { + // inherit_option is a pg_auth_members column added in PostgreSQL 16. + q.String = "SELECT EXISTS(SELECT 1 FROM pg_auth_members m " + + "INNER JOIN pg_roles mo ON m.roleid = mo.oid " + + "INNER JOIN pg_roles r ON m.member = r.oid " + + "WHERE r.rolname=$1 AND mo.rolname=$2 AND " + + "m.admin_option = $3 AND m.inherit_option = $4)" + q.Parameters = append(q.Parameters, *gp.WithInherit) + } else { + q.String = "SELECT EXISTS(SELECT 1 FROM pg_auth_members m " + + "INNER JOIN pg_roles mo ON m.roleid = mo.oid " + + "INNER JOIN pg_roles r ON m.member = r.oid " + + "WHERE r.rolname=$1 AND mo.rolname=$2 AND " + + "m.admin_option = $3)" + } return nil } diff --git a/pkg/controller/cluster/postgresql/grant/reconciler_test.go b/pkg/controller/cluster/postgresql/grant/reconciler_test.go index 1ccabe70..e4329a6a 100644 --- a/pkg/controller/cluster/postgresql/grant/reconciler_test.go +++ b/pkg/controller/cluster/postgresql/grant/reconciler_test.go @@ -195,7 +195,8 @@ func TestObserve(t *testing.T) { gog := v1alpha1.GrantOptionGrant type fields struct { - db xsql.DB + db xsql.DB + serverVersion int } type args struct { @@ -632,6 +633,115 @@ func TestObserve(t *testing.T) { err: nil, }, }, + "SuccessRoleMembershipWithInheritNil": { + reason: "WithInherit nil should produce a 3-parameter query (no inherit_option filter)", + fields: fields{ + serverVersion: 160000, + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + if len(q.Parameters) != 3 { + return fmt.Errorf("expected 3 query parameters, got %d", len(q.Parameters)) + } + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, + "SuccessRoleMembershipWithInheritFalse": { + reason: "WithInherit false should produce a 4-parameter query with $4 == false", + fields: fields{ + serverVersion: 160000, + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + if len(q.Parameters) != 4 { + return fmt.Errorf("expected 4 query parameters, got %d", len(q.Parameters)) + } + inheritParam, ok := q.Parameters[3].(bool) + if !ok || inheritParam != false { + return fmt.Errorf("expected $4 to be false, got %v", q.Parameters[3]) + } + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + WithInherit: ptr.To(false), + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, + "SuccessRoleMembershipWithInheritTrue": { + reason: "WithInherit true should produce a 4-parameter query with $4 == true", + fields: fields{ + serverVersion: 160000, + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + if len(q.Parameters) != 4 { + return fmt.Errorf("expected 4 query parameters, got %d", len(q.Parameters)) + } + inheritParam, ok := q.Parameters[3].(bool) + if !ok || inheritParam != true { + return fmt.Errorf("expected $4 to be true, got %v", q.Parameters[3]) + } + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + WithInherit: ptr.To(true), + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, } for name, tc := range cases { @@ -640,7 +750,7 @@ func TestObserve(t *testing.T) { if db == nil { db = mockDB{} } - e := external{db: db} + e := external{db: db, serverVersion: tc.fields.serverVersion} got, err := e.Observe(tc.args.ctx, tc.args.mg) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\ne.Observe(...): -want error, +got error:\n%s\n", tc.reason, diff) @@ -657,7 +767,8 @@ func TestCreate(t *testing.T) { goa := v1alpha1.GrantOptionAdmin type fields struct { - db xsql.DB + db xsql.DB + serverVersion int } type args struct { @@ -935,6 +1046,102 @@ func TestCreate(t *testing.T) { err: nil, }, }, + "RoleMembershipWithInheritNil": { + reason: "WithInherit nil should produce a GRANT with no WITH clause", + fields: fields{ + serverVersion: 160000, + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { + if len(ql) != 2 { + return fmt.Errorf("expected 2 queries, got %d", len(ql)) + } + grantSQL := ql[1].String + if strings.Contains(grantSQL, "INHERIT") { + return fmt.Errorf("expected no INHERIT clause, got: %s", grantSQL) + } + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, + "RoleMembershipWithInheritFalse": { + reason: "WithInherit false should produce a GRANT with WITH INHERIT FALSE", + fields: fields{ + serverVersion: 160000, + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { + if len(ql) != 2 { + return fmt.Errorf("expected 2 queries, got %d", len(ql)) + } + grantSQL := ql[1].String + if !strings.Contains(grantSQL, "WITH INHERIT FALSE") { + return fmt.Errorf("expected WITH INHERIT FALSE in grant SQL, got: %s", grantSQL) + } + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + WithInherit: ptr.To(false), + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, + "RoleMembershipWithInheritFalseAndAdminOption": { + reason: "WithInherit false and WithOption ADMIN should produce WITH ADMIN OPTION, INHERIT FALSE", + fields: fields{ + serverVersion: 160000, + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { + if len(ql) != 2 { + return fmt.Errorf("expected 2 queries, got %d", len(ql)) + } + grantSQL := ql[1].String + if !strings.Contains(grantSQL, "WITH ADMIN OPTION, INHERIT FALSE") { + return fmt.Errorf("expected WITH ADMIN OPTION, INHERIT FALSE in grant SQL, got: %s", grantSQL) + } + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + WithOption: &goa, + WithInherit: ptr.To(false), + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, } for name, tc := range cases { @@ -943,7 +1150,7 @@ func TestCreate(t *testing.T) { if db == nil { db = mockDB{} } - e := external{db: db} + e := external{db: db, serverVersion: tc.fields.serverVersion} got, err := e.Create(tc.args.ctx, tc.args.mg) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\ne.Create(...): -want error, +got error:\n%s\n", tc.reason, diff) diff --git a/pkg/controller/namespaced/postgresql/grant/reconciler.go b/pkg/controller/namespaced/postgresql/grant/reconciler.go index b1908f22..28205462 100644 --- a/pkg/controller/namespaced/postgresql/grant/reconciler.go +++ b/pkg/controller/namespaced/postgresql/grant/reconciler.go @@ -56,6 +56,8 @@ const ( errInvalidParams = "invalid parameters for grant type %s" errGetServerVersion = "cannot get server version" errMemberOfWithDatabaseOrPrivileges = "cannot set privileges or database in the same grant as memberOf" + errWithInheritOnlyForMemberOf = "withInherit is only valid for memberOf grants" + errInheritRequiresPG16 = "withInherit requires PostgreSQL 16 or later (server version %d)" maxConcurrency = 5 ) @@ -238,6 +240,9 @@ func createGrantQueriesWithVersion(gp v1alpha1.GrantParameters, ql *[]xsql.Query case v1alpha1.RoleForeignServer: return createForeignServerGrantQueries(gp, ql, ro) case v1alpha1.RoleMember: + if gp.WithInherit != nil && serverVersion < 160000 { + return errors.Errorf(errInheritRequiresPG16, serverVersion) + } return createMemberGrantQueries(gp, ql, ro) case v1alpha1.RoleRoutine: return createRoutineGrantQueries(gp, ql, ro) @@ -261,12 +266,33 @@ func createMemberGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro *ql = append(*ql, xsql.Query{String: fmt.Sprintf("REVOKE %s FROM %s", mo, ro)}, xsql.Query{String: fmt.Sprintf("GRANT %s TO %s %s", mo, ro, - withOption(gp.WithOption), + membershipWithClauses(gp.WithOption, gp.WithInherit), )}, ) return nil } +// membershipWithClauses builds the WITH clause for role membership GRANTs, +// combining the optional ADMIN/SET option and the optional INHERIT flag. +// On PostgreSQL 16+, multiple options are comma-separated: WITH ADMIN OPTION, INHERIT FALSE. +func membershipWithClauses(option *v1alpha1.GrantOption, inherit *bool) string { + var parts []string + if option != nil { + parts = append(parts, fmt.Sprintf("%s OPTION", string(*option))) + } + if inherit != nil { + if *inherit { + parts = append(parts, "INHERIT TRUE") + } else { + parts = append(parts, "INHERIT FALSE") + } + } + if len(parts) == 0 { + return "" + } + return "WITH " + strings.Join(parts, ", ") +} + func createRoutineGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || gp.Schema == nil || len(gp.Routines) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleRoutine) @@ -561,6 +587,10 @@ func resolveGrantType(gp v1alpha1.GrantParameters) (v1alpha1.GrantType, error) { return v1alpha1.RoleMember, nil } + if gp.WithInherit != nil { + return "", errors.New(errWithInheritOnlyForMemberOf) + } + return gp.IdentifyGrantType() } @@ -729,6 +759,9 @@ func selectGrantQueryWithVersion(gp v1alpha1.GrantParameters, q *xsql.Query, ser case v1alpha1.RoleForeignServer: return selectForeignServerGrantQuery(gp, q) case v1alpha1.RoleMember: + if gp.WithInherit != nil && serverVersion < 160000 { + return errors.Errorf(errInheritRequiresPG16, serverVersion) + } return selectMemberGrantQuery(gp, q) case v1alpha1.RoleRoutine: return selectRoutineGrantQuery(gp, q) @@ -749,17 +782,27 @@ func selectMemberGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { // A simpler query would use ::regrole to cast the roleid and member oids // to their role names, but that throws an error for nonexistent roles // rather than returning false. - q.String = "SELECT EXISTS(SELECT 1 FROM pg_auth_members m " + - "INNER JOIN pg_roles mo ON m.roleid = mo.oid " + - "INNER JOIN pg_roles r ON m.member = r.oid " + - "WHERE r.rolname=$1 AND mo.rolname=$2 AND " + - "m.admin_option = $3)" - q.Parameters = []interface{}{ gp.Role, gp.MemberOf, ao, } + + if gp.WithInherit != nil { + // inherit_option is a pg_auth_members column added in PostgreSQL 16. + q.String = "SELECT EXISTS(SELECT 1 FROM pg_auth_members m " + + "INNER JOIN pg_roles mo ON m.roleid = mo.oid " + + "INNER JOIN pg_roles r ON m.member = r.oid " + + "WHERE r.rolname=$1 AND mo.rolname=$2 AND " + + "m.admin_option = $3 AND m.inherit_option = $4)" + q.Parameters = append(q.Parameters, *gp.WithInherit) + } else { + q.String = "SELECT EXISTS(SELECT 1 FROM pg_auth_members m " + + "INNER JOIN pg_roles mo ON m.roleid = mo.oid " + + "INNER JOIN pg_roles r ON m.member = r.oid " + + "WHERE r.rolname=$1 AND mo.rolname=$2 AND " + + "m.admin_option = $3)" + } return nil } diff --git a/pkg/controller/namespaced/postgresql/grant/reconciler_test.go b/pkg/controller/namespaced/postgresql/grant/reconciler_test.go index 92857b2c..add7099a 100644 --- a/pkg/controller/namespaced/postgresql/grant/reconciler_test.go +++ b/pkg/controller/namespaced/postgresql/grant/reconciler_test.go @@ -21,6 +21,7 @@ import ( "database/sql" "fmt" "sort" + "strings" "testing" "github.com/crossplane-contrib/provider-sql/apis/namespaced/postgresql/v1alpha1" @@ -253,7 +254,8 @@ func TestObserve(t *testing.T) { gog := v1alpha1.GrantOptionGrant type fields struct { - db xsql.DB + db xsql.DB + serverVersion int } type args struct { @@ -681,6 +683,115 @@ func TestObserve(t *testing.T) { err: nil, }, }, + "SuccessRoleMembershipWithInheritNil": { + reason: "WithInherit nil should produce a 3-parameter query (no inherit_option filter)", + fields: fields{ + serverVersion: 160000, + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + if len(q.Parameters) != 3 { + return fmt.Errorf("expected 3 query parameters, got %d", len(q.Parameters)) + } + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, + "SuccessRoleMembershipWithInheritFalse": { + reason: "WithInherit false should produce a 4-parameter query with $4 == false", + fields: fields{ + serverVersion: 160000, + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + if len(q.Parameters) != 4 { + return fmt.Errorf("expected 4 query parameters, got %d", len(q.Parameters)) + } + inheritParam, ok := q.Parameters[3].(bool) + if !ok || inheritParam != false { + return fmt.Errorf("expected $4 to be false, got %v", q.Parameters[3]) + } + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + WithInherit: ptr.To(false), + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, + "SuccessRoleMembershipWithInheritTrue": { + reason: "WithInherit true should produce a 4-parameter query with $4 == true", + fields: fields{ + serverVersion: 160000, + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + if len(q.Parameters) != 4 { + return fmt.Errorf("expected 4 query parameters, got %d", len(q.Parameters)) + } + inheritParam, ok := q.Parameters[3].(bool) + if !ok || inheritParam != true { + return fmt.Errorf("expected $4 to be true, got %v", q.Parameters[3]) + } + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + WithInherit: ptr.To(true), + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, } for name, tc := range cases { @@ -689,7 +800,7 @@ func TestObserve(t *testing.T) { if db == nil { db = mockDB{} } - e := external{db: db} + e := external{db: db, serverVersion: tc.fields.serverVersion} got, err := e.Observe(tc.args.ctx, tc.args.mg) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\ne.Observe(...): -want error, +got error:\n%s\n", tc.reason, diff) @@ -706,7 +817,8 @@ func TestCreate(t *testing.T) { goa := v1alpha1.GrantOptionAdmin type fields struct { - db xsql.DB + db xsql.DB + serverVersion int } type args struct { @@ -975,6 +1087,102 @@ func TestCreate(t *testing.T) { err: nil, }, }, + "RoleMembershipWithInheritNil": { + reason: "WithInherit nil should produce a GRANT with no WITH clause", + fields: fields{ + serverVersion: 160000, + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { + if len(ql) != 2 { + return fmt.Errorf("expected 2 queries, got %d", len(ql)) + } + grantSQL := ql[1].String + if strings.Contains(grantSQL, "INHERIT") { + return fmt.Errorf("expected no INHERIT clause, got: %s", grantSQL) + } + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, + "RoleMembershipWithInheritFalse": { + reason: "WithInherit false should produce a GRANT with WITH INHERIT FALSE", + fields: fields{ + serverVersion: 160000, + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { + if len(ql) != 2 { + return fmt.Errorf("expected 2 queries, got %d", len(ql)) + } + grantSQL := ql[1].String + if !strings.Contains(grantSQL, "WITH INHERIT FALSE") { + return fmt.Errorf("expected WITH INHERIT FALSE in grant SQL, got: %s", grantSQL) + } + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + WithInherit: ptr.To(false), + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, + "RoleMembershipWithInheritFalseAndAdminOption": { + reason: "WithInherit false and WithOption ADMIN should produce WITH ADMIN OPTION, INHERIT FALSE", + fields: fields{ + serverVersion: 160000, + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { + if len(ql) != 2 { + return fmt.Errorf("expected 2 queries, got %d", len(ql)) + } + grantSQL := ql[1].String + if !strings.Contains(grantSQL, "WITH ADMIN OPTION, INHERIT FALSE") { + return fmt.Errorf("expected WITH ADMIN OPTION, INHERIT FALSE in grant SQL, got: %s", grantSQL) + } + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + WithOption: &goa, + WithInherit: ptr.To(false), + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, } for name, tc := range cases { @@ -983,7 +1191,7 @@ func TestCreate(t *testing.T) { if db == nil { db = mockDB{} } - e := external{db: db} + e := external{db: db, serverVersion: tc.fields.serverVersion} got, err := e.Create(tc.args.ctx, tc.args.mg) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\ne.Create(...): -want error, +got error:\n%s\n", tc.reason, diff)