From 2c549b2b395b04b342e9add46dc4aa8a00391233 Mon Sep 17 00:00:00 2001 From: Bastien CERIANI Date: Thu, 16 May 2024 17:31:26 +0200 Subject: [PATCH 01/12] feat: add grant specs schema, objects, objectType Signed-off-by: Bastien CERIANI Signed-off-by: Julien Christophe --- .gitignore | 1 + apis/postgresql/v1alpha1/grant_types.go | 45 +++++++ .../v1alpha1/zz_generated.deepcopy.go | 25 ++++ .../postgresql.sql.crossplane.io_grants.yaml | 112 ++++++++++++++++++ pkg/controller/postgresql/grant/reconciler.go | 85 ++++++++++--- 5 files changed, 252 insertions(+), 16 deletions(-) diff --git a/.gitignore b/.gitignore index d3788258..960b3c64 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ .work _output __debug_bin +.tool-versions \ No newline at end of file diff --git a/apis/postgresql/v1alpha1/grant_types.go b/apis/postgresql/v1alpha1/grant_types.go index 1bf6fc7c..560ec2e2 100644 --- a/apis/postgresql/v1alpha1/grant_types.go +++ b/apis/postgresql/v1alpha1/grant_types.go @@ -47,6 +47,7 @@ type GrantPrivileges []GrantPrivilege // happen internally inside postgresql when making grants. When we query the // privileges back, we need to look for the expanded set. // https://www.postgresql.org/docs/15/ddl-priv.html +// TODO: Grand ALL ON SCHEMA should be expanded to GRANT USAGE, CREATE ON SCHEMA var grantReplacements = map[GrantPrivilege]GrantPrivileges{ "ALL": {"CREATE", "TEMPORARY", "CONNECT"}, "ALL PRIVILEGES": {"CREATE", "TEMPORARY", "CONNECT"}, @@ -141,6 +142,20 @@ type GrantParameters struct { // +optional DatabaseSelector *xpv1.Selector `json:"databaseSelector,omitempty"` + // Schema this grant is for. + // +optional + Schema *string `json:"schema,omitempty"` + + // SchemaRef references the schema object this grant it for. + // +immutable + // +optional + SchemaRef *xpv1.Reference `json:"schemaRef,omitempty"` + + // SchemaSelector selects a reference to a Schema this grant is for. + // +immutable + // +optional + SchemaSelector *xpv1.Selector `json:"schemaSelector,omitempty"` + // MemberOf is the Role that this grant makes Role a member of. // +optional MemberOf *string `json:"memberOf,omitempty"` @@ -158,6 +173,22 @@ type GrantParameters struct { // RevokePublicOnDb apply the statement "REVOKE ALL ON DATABASE %s FROM PUBLIC" to make database unreachable from public // +optional RevokePublicOnDb *bool `json:"revokePublicOnDb,omitempty" default:"false"` + + // ObjectType is the PostgreSQL object type to grant the privileges on. + // +kubebuilder:validation:Enum=database;schema;table;sequence;function;procedure;routine;foreign_data_wrapper;foreign_server;column + ObjectType string `json:"objectType" default:"database"` + + // Objects are the objects upon which to grant the privileges. + // An empty list (the default) means to grant permissions on all objects of the specified type. + // You cannot specify this option if the objectType is database or schema. + // When objectType is column, only one value is allowed. + // +optional + Objects []string `json:"objects,omitempty"` + + // The columns upon which to grant the privileges. + // Required when object_type is column. You cannot specify this option if the object_type is not column + // +optional + Columns []string `json:"columns,omitempty"` } // A GrantStatus represents the observed state of a Grant. @@ -212,6 +243,20 @@ func (mg *Grant) ResolveReferences(ctx context.Context, c client.Reader) error { mg.Spec.ForProvider.Database = reference.ToPtrValue(rsp.ResolvedValue) mg.Spec.ForProvider.DatabaseRef = rsp.ResolvedReference + // Resolve spec.forProvider.schema + rsp, err = r.Resolve(ctx, reference.ResolutionRequest{ + CurrentValue: reference.FromPtrValue(mg.Spec.ForProvider.Schema), + Reference: mg.Spec.ForProvider.SchemaRef, + Selector: mg.Spec.ForProvider.SchemaSelector, + To: reference.To{Managed: &Schema{}, List: &SchemaList{}}, + Extract: reference.ExternalName(), + }) + if err != nil { + return errors.Wrap(err, "spec.forProvider.schema") + } + mg.Spec.ForProvider.Schema = reference.ToPtrValue(rsp.ResolvedValue) + mg.Spec.ForProvider.SchemaRef = rsp.ResolvedReference + // Resolve spec.forProvider.role rsp, err = r.Resolve(ctx, reference.ResolutionRequest{ CurrentValue: reference.FromPtrValue(mg.Spec.ForProvider.Role), diff --git a/apis/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/postgresql/v1alpha1/zz_generated.deepcopy.go index 70d15187..05838181 100644 --- a/apis/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -411,6 +411,21 @@ func (in *GrantParameters) DeepCopyInto(out *GrantParameters) { *out = new(v1.Selector) (*in).DeepCopyInto(*out) } + if in.Schema != nil { + in, out := &in.Schema, &out.Schema + *out = new(string) + **out = **in + } + if in.SchemaRef != nil { + in, out := &in.SchemaRef, &out.SchemaRef + *out = new(v1.Reference) + (*in).DeepCopyInto(*out) + } + if in.SchemaSelector != nil { + in, out := &in.SchemaSelector, &out.SchemaSelector + *out = new(v1.Selector) + (*in).DeepCopyInto(*out) + } if in.MemberOf != nil { in, out := &in.MemberOf, &out.MemberOf *out = new(string) @@ -431,6 +446,16 @@ func (in *GrantParameters) DeepCopyInto(out *GrantParameters) { *out = new(bool) **out = **in } + if in.Objects != nil { + in, out := &in.Objects, &out.Objects + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Columns != nil { + in, out := &in.Columns, &out.Columns + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GrantParameters. diff --git a/package/crds/postgresql.sql.crossplane.io_grants.yaml b/package/crds/postgresql.sql.crossplane.io_grants.yaml index f9849f71..a95a3a8e 100644 --- a/package/crds/postgresql.sql.crossplane.io_grants.yaml +++ b/package/crds/postgresql.sql.crossplane.io_grants.yaml @@ -83,6 +83,13 @@ spec: description: GrantParameters define the desired state of a PostgreSQL grant instance. properties: + columns: + description: |- + The columns upon which to grant the privileges. + Required when object_type is column. You cannot specify this option if the object_type is not column + items: + type: string + type: array database: description: Database this grant is for. type: string @@ -242,6 +249,30 @@ spec: type: string type: object type: object + objectType: + description: ObjectType is the PostgreSQL object type to grant + the privileges on. + enum: + - database + - schema + - table + - sequence + - function + - procedure + - routine + - foreign_data_wrapper + - foreign_server + - column + type: string + objects: + description: |- + Objects are the objects upon which to grant the privileges. + An empty list (the default) means to grant permissions on all objects of the specified type. + You cannot specify this option if the objectType is database or schema. + When objectType is column, only one value is allowed. + items: + type: string + type: array privileges: description: |- Privileges to be granted. @@ -336,6 +367,85 @@ spec: type: string type: object type: object + schema: + description: Schema this grant is for. + type: string + schemaRef: + description: SchemaRef references the schema object this grant + it for. + properties: + name: + description: Name of the referenced object. + type: string + policy: + description: Policies for referencing. + properties: + resolution: + default: Required + description: |- + Resolution specifies whether resolution of this reference is required. + The default is 'Required', which means the reconcile will fail if the + reference cannot be resolved. 'Optional' means this reference will be + a no-op if it cannot be resolved. + enum: + - Required + - Optional + type: string + resolve: + description: |- + Resolve specifies when this reference should be resolved. The default + is 'IfNotPresent', which will attempt to resolve the reference only when + the corresponding field is not present. Use 'Always' to resolve the + reference on every reconcile. + enum: + - Always + - IfNotPresent + type: string + type: object + required: + - name + type: object + schemaSelector: + description: SchemaSelector selects a reference to a Schema this + grant is for. + properties: + matchControllerRef: + description: |- + MatchControllerRef ensures an object with the same controller reference + as the selecting object is selected. + type: boolean + matchLabels: + additionalProperties: + type: string + description: MatchLabels ensures an object with matching labels + is selected. + type: object + policy: + description: Policies for selection. + properties: + resolution: + default: Required + description: |- + Resolution specifies whether resolution of this reference is required. + The default is 'Required', which means the reconcile will fail if the + reference cannot be resolved. 'Optional' means this reference will be + a no-op if it cannot be resolved. + enum: + - Required + - Optional + type: string + resolve: + description: |- + Resolve specifies when this reference should be resolved. The default + is 'IfNotPresent', which will attempt to resolve the reference only when + the corresponding field is not present. Use 'Always' to resolve the + reference on every reconcile. + enum: + - Always + - IfNotPresent + type: string + type: object + type: object withOption: description: |- WithOption allows an option to be set on the grant. @@ -345,6 +455,8 @@ spec: - ADMIN - GRANT type: string + required: + - objectType type: object managementPolicies: default: diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index 7a236c09..766ca8a5 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -52,6 +52,7 @@ const ( errSelectGrant = "cannot select grant" errCreateGrant = "cannot create grant" errRevokeGrant = "cannot revoke grant" + errNoSchema = "schema not passed or could not be resolved" errNoRole = "role not passed or could not be resolved" errNoDatabase = "database not passed or could not be resolved" errNoPrivileges = "privileges not passed" @@ -144,6 +145,41 @@ const ( roleDatabase grantType = "ROLE_DATABASE" ) +// sliceContainsStr checks if a slice contains a specific string. +func sliceContainsStr(haystack []string, needle string) bool { + for _, s := range haystack { + if s == needle { + return true + } + } + return false +} + +func validateGrantParams(gp v1alpha1.GrantParameters) error { + if gp.Schema == nil && !sliceContainsStr([]string{"database", "foreign_data_wrapper", "foreign_server"}, gp.ObjectType) { + return fmt.Errorf("parameter 'schema' is mandatory for grant resource") + } + if len(gp.Objects) > 0 && (gp.ObjectType == "database" || gp.ObjectType == "schema") { + return fmt.Errorf("cannot specify `objects` when `object_type` is `database` or `schema`") + } + if len(gp.Columns) > 0 && gp.ObjectType != "column" { + return fmt.Errorf("cannot specify `columns` when `object_type` is not `column`") + } + if len(gp.Columns) == 0 && gp.ObjectType == "column" { + return fmt.Errorf("must specify `columns` when `object_type` is `column`") + } + if len(gp.Privileges) != 1 && gp.ObjectType == "column" { + return fmt.Errorf("must specify exactly 1 `privileges` when `object_type` is `column`") + } + if len(gp.Objects) != 1 && gp.ObjectType == "column" { + return fmt.Errorf("must specify exactly 1 table in the `objects` field when `object_type` is `column`") + } + if len(gp.Objects) != 1 && (gp.ObjectType == "foreign_data_wrapper" || gp.ObjectType == "foreign_server") { + return fmt.Errorf("one element must be specified in `objects` when `object_type` is `foreign_data_wrapper` or `foreign_server`") + } + return nil +} + func identifyGrantType(gp v1alpha1.GrantParameters) (grantType, error) { pc := len(gp.Privileges) @@ -201,34 +237,43 @@ func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { ep := gp.Privileges.ExpandPrivileges() sp := ep.ToStringSlice() + // Join grantee. Filter by database name and grantee name. // Finally, perform a permission comparison against expected // permissions. q.String = "SELECT EXISTS(SELECT 1 " + - "FROM pg_database db, " + - "aclexplode(datacl) as acl " + - "INNER JOIN pg_roles s ON acl.grantee = s.oid " + - // Filter by database, role and grantable setting - "WHERE db.datname=$1 " + - "AND s.rolname=$2 " + - "AND acl.is_grantable=$3 " + - "GROUP BY db.datname, s.rolname, acl.is_grantable " + - // Check privileges match. Convoluted right-hand-side is necessary to - // ensure identical sort order of the input permissions. - "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + - "= (SELECT array(SELECT unnest($4::text[]) as perms ORDER BY perms ASC)))" + "FROM pg_database db " + + "JOIN pg_namespace nsp ON db.datname = $1 " + + "JOIN LATERAL aclexplode(nsp.nspacl) acl ON true " + + "JOIN pg_roles s ON acl.grantee = s.oid " + + // Filter by role, schema and grantable setting + "WHERE nsp.nspname = $2 " + + "AND s.rolname = $3 " + + "AND acl.is_grantable = $6 " + + "GROUP BY db.datname, nsp.nspname, s.rolname, acl.is_grantable " + + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) = " + + "(SELECT array(SELECT unnest($7::text[]) ORDER BY 1)))" q.Parameters = []interface{}{ gp.Database, + gp.Schema, gp.Role, + gp.ObjectType, + gp.Objects, gro, pq.Array(sp), } - return nil } return errors.New(errUnknownGrant) } +func withSchema(schema *string) string { + if schema != nil { + return fmt.Sprintf("IN SCHEMA %s", *schema) + } + return "" +} + func withOption(option *v1alpha1.GrantOption) string { if option != nil { return fmt.Sprintf("WITH %s OPTION", string(*option)) @@ -269,17 +314,19 @@ func createGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query) error { / *ql = append(*ql, // REVOKE ANY MATCHING EXISTING PERMISSIONS - xsql.Query{String: fmt.Sprintf("REVOKE %s ON DATABASE %s FROM %s", + xsql.Query{String: fmt.Sprintf("REVOKE %s ON DATABASE %s %s FROM %s", sp, db, + withSchema(gp.Schema), ro, )}, // GRANT REQUESTED PERMISSIONS - xsql.Query{String: fmt.Sprintf("GRANT %s ON DATABASE %s TO %s %s", + xsql.Query{String: fmt.Sprintf("GRANT %s ON DATABASE %s TO %s %s %s", sp, db, ro, + withSchema(gp.Schema), withOption(gp.WithOption), )}, ) @@ -312,9 +359,10 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { ) return nil case roleDatabase: - q.String = fmt.Sprintf("REVOKE %s ON DATABASE %s FROM %s", + q.String = fmt.Sprintf("REVOKE %s ON DATABASE %s %s FROM %s", strings.Join(gp.Privileges.ToStringSlice(), ","), pq.QuoteIdentifier(*gp.Database), + withSchema(gp.Schema), ro, ) return nil @@ -364,6 +412,11 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext return managed.ExternalCreation{}, errors.New(errNotGrant) } + // Validate grant specs + if err := validateGrantParams(cr.Spec.ForProvider); err != nil { + return managed.ExternalCreation{}, errors.Wrap(err, errInvalidParams) + } + var queries []xsql.Query cr.SetConditions(xpv1.Creating()) From c2772548d258286fd4f568aaf31fe82d44f472a1 Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Tue, 29 Apr 2025 10:26:52 +0200 Subject: [PATCH 02/12] feat: support full schema grants Signed-off-by: Julien Christophe --- apis/postgresql/v1alpha1/grant_types.go | 187 ++++++++-- .../v1alpha1/zz_generated.deepcopy.go | 38 ++- cluster/local/postgresdb_functions.sh | 58 +++- examples/postgresql/grant.yaml | 16 + examples/postgresql/role.yaml | 14 +- .../postgresql.sql.crossplane.io_grants.yaml | 66 ++-- pkg/controller/postgresql/grant/reconciler.go | 322 +++++++++--------- .../postgresql/grant/reconciler_test.go | 136 +++++++- 8 files changed, 598 insertions(+), 239 deletions(-) diff --git a/apis/postgresql/v1alpha1/grant_types.go b/apis/postgresql/v1alpha1/grant_types.go index 560ec2e2..cb7a4ef1 100644 --- a/apis/postgresql/v1alpha1/grant_types.go +++ b/apis/postgresql/v1alpha1/grant_types.go @@ -27,6 +27,12 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/reference" ) +const ( + errNoPrivileges = "privileges not passed" + errUnknownGrant = "cannot identify grant type based on passed params" + errMemberOfWithPrivileges = "cannot set privileges in the same grant as memberOf" +) + // A GrantSpec defines the desired state of a Grant. type GrantSpec struct { xpv1.ResourceSpec `json:",inline"` @@ -43,25 +49,152 @@ type GrantPrivilege string // +kubebuilder:validation:MinItems:=1 type GrantPrivileges []GrantPrivilege +type GrantType string + +// GrantType is the list of the possible grant types represented by a GrantParameters +const ( + RoleMember GrantType = "ROLE_MEMBER" + RoleDatabase GrantType = "ROLE_DATABASE" + RoleSchema GrantType = "ROLE_SCHEMA" + RoleTable GrantType = "ROLE_TABLE" + RoleSequence GrantType = "ROLE_SEQUENCE" + RoleFunction GrantType = "ROLE_FUNCTION" + RoleProcedure GrantType = "ROLE_PROCEDURE" + RoleRoutine GrantType = "ROLE_ROUTE" + RoleColumn GrantType = "ROLE_COLUMN" + RoleForeignDataWrapper GrantType = "ROLE_FOREIGN_DATA_WRAPPER" + RoleForeignServer GrantType = "ROLE_FOREIGN_SERVER" +) + +type marker struct{} +type stringSet struct { + elements map[string]marker +} + +func newStringSet() *stringSet { + return &stringSet{ + elements: make(map[string]marker), + } +} + +func (s *stringSet) add(element string) { + s.elements[element] = marker{} +} + +func (s *stringSet) contains(element string) bool { + _, exists := s.elements[element] + return exists +} + +func (s *stringSet) containsExactly(elements ...string) bool { + if len(s.elements) != len(elements) { + return false + } + for _, elem := range elements { + if !s.contains(elem) { + return false + } + } + return true +} + +func (gp *GrantParameters) filledInFields() *stringSet { + fields := map[string]bool{ + "MemberOf": gp.MemberOf != nil, + "Database": gp.Database != nil, + "Schema": gp.Schema != nil, + "Tables": len(gp.Tables) > 0, + "Columns": len(gp.Columns) > 0, + "Sequences": len(gp.Sequences) > 0, + "Functions": len(gp.Functions) > 0, + "Procedures": len(gp.Procedures) > 0, + "Routines": len(gp.Routines) > 0, + "ForeignServers": len(gp.ForeignServers) > 0, + "ForeignDataWrappers": len(gp.ForeignDataWrappers) > 0, + } + set := newStringSet() + + for key, hasField := range fields { + if hasField { + set.add(key) + } + } + return set +} + +// IdentifyGrantType return the deduced GrantType from the filled in fields. +func (gp *GrantParameters) IdentifyGrantType() (GrantType, error) { + fields := gp.filledInFields() + pc := len(gp.Privileges) + + grantTypeFields := map[GrantType][]string{ + RoleMember: {"MemberOf"}, + RoleDatabase: {"Database"}, + RoleSchema: {"Database", "Schema"}, + RoleTable: {"Database", "Schema", "Tables"}, + RoleColumn: {"Database", "Schema", "Tables", "Columns"}, + RoleSequence: {"Database", "Schema", "Sequences"}, + RoleFunction: {"Database", "Schema", "Functions"}, + RoleProcedure: {"Database", "Schema", "Procedures"}, + RoleRoutine: {"Database", "Schema", "Routines"}, + RoleForeignServer: {"Database", "ForeignServers"}, + RoleForeignDataWrapper: {"Database", "ForeignDataWrappers"}, + } + + var role *GrantType + + for key, expectedFields := range grantTypeFields { + if fields.containsExactly(expectedFields...) { + role = &key + break + } + } + if role == nil { + return "", errors.New(errUnknownGrant) + } + if *role == RoleMember && pc > 0 { + return "", errors.New(errMemberOfWithPrivileges) + } + if *role != RoleMember && pc < 1 { + return "", errors.New(errNoPrivileges) + } + return *role, nil +} + // Some privileges are shorthands for multiple privileges. These translations // happen internally inside postgresql when making grants. When we query the // privileges back, we need to look for the expanded set. // https://www.postgresql.org/docs/15/ddl-priv.html // TODO: Grand ALL ON SCHEMA should be expanded to GRANT USAGE, CREATE ON SCHEMA -var grantReplacements = map[GrantPrivilege]GrantPrivileges{ - "ALL": {"CREATE", "TEMPORARY", "CONNECT"}, - "ALL PRIVILEGES": {"CREATE", "TEMPORARY", "CONNECT"}, - "TEMP": {"TEMPORARY"}, +var grantReplacements = map[GrantType]map[GrantPrivilege]GrantPrivileges{ + RoleDatabase: { + "ALL": {"CREATE", "TEMPORARY", "CONNECT"}, + "ALL PRIVILEGES": {"CREATE", "TEMPORARY", "CONNECT"}, + "TEMP": {"TEMPORARY"}, + }, + RoleSchema: { + "ALL": {"CREATE", "USAGE"}, + "ALL PRIVILEGES": {"CREATE", "USAGE"}, + }, } // ExpandPrivileges expands any shorthand privileges to their full equivalents. -func (gp *GrantPrivileges) ExpandPrivileges() GrantPrivileges { +func (gp *GrantParameters) ExpandPrivileges() GrantPrivileges { + grantType, err := gp.IdentifyGrantType() + if err != nil { + return gp.Privileges + } + replacements, hasReplacements := grantReplacements[grantType] + if !hasReplacements { + return gp.Privileges + } + privilegeSet := make(map[GrantPrivilege]struct{}) // Replace any shorthand privileges with their full equivalents - for _, p := range *gp { - if _, ok := grantReplacements[p]; ok { - for _, rp := range grantReplacements[p] { + for _, p := range gp.Privileges { + if _, ok := replacements[p]; ok { + for _, rp := range replacements[p] { privilegeSet[rp] = struct{}{} } } else { @@ -174,21 +307,37 @@ type GrantParameters struct { // +optional RevokePublicOnDb *bool `json:"revokePublicOnDb,omitempty" default:"false"` - // ObjectType is the PostgreSQL object type to grant the privileges on. - // +kubebuilder:validation:Enum=database;schema;table;sequence;function;procedure;routine;foreign_data_wrapper;foreign_server;column - ObjectType string `json:"objectType" default:"database"` + // The columns upon which to grant the privileges. + // +optional + Columns []string `json:"columns,omitempty"` + + // The tables upon which to grant the privileges. + // +optional + Tables []string `json:"tables,omitempty"` - // Objects are the objects upon which to grant the privileges. - // An empty list (the default) means to grant permissions on all objects of the specified type. - // You cannot specify this option if the objectType is database or schema. - // When objectType is column, only one value is allowed. + // The sequences upon which to grant the privileges. // +optional - Objects []string `json:"objects,omitempty"` + Sequences []string `json:"sequences,omitempty"` - // The columns upon which to grant the privileges. - // Required when object_type is column. You cannot specify this option if the object_type is not column + // The functions upon which to grant the privileges. // +optional - Columns []string `json:"columns,omitempty"` + Functions []string `json:"functions,omitempty"` + + // The procedures upon which to grant the privileges. + // +optional + Procedures []string `json:"procedures,omitempty"` + + // The routines upon which to grant the privileges. + // +optional + Routines []string `json:"routines,omitempty"` + + // The foreign data wrappers upon which to grant the privileges. + // +optional + ForeignDataWrappers []string `json:"foreignDataWrappers,omitempty"` + + // The foreign servers upon which to grant the privileges. + // +optional + ForeignServers []string `json:"foreignServers,omitempty"` } // A GrantStatus represents the observed state of a Grant. diff --git a/apis/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/postgresql/v1alpha1/zz_generated.deepcopy.go index 05838181..a0cdd16e 100644 --- a/apis/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -446,13 +446,43 @@ func (in *GrantParameters) DeepCopyInto(out *GrantParameters) { *out = new(bool) **out = **in } - if in.Objects != nil { - in, out := &in.Objects, &out.Objects + if in.Columns != nil { + in, out := &in.Columns, &out.Columns *out = make([]string, len(*in)) copy(*out, *in) } - if in.Columns != nil { - in, out := &in.Columns, &out.Columns + if in.Tables != nil { + in, out := &in.Tables, &out.Tables + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Sequences != nil { + in, out := &in.Sequences, &out.Sequences + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Functions != nil { + in, out := &in.Functions, &out.Functions + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Procedures != nil { + in, out := &in.Procedures, &out.Procedures + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Routines != nil { + in, out := &in.Routines, &out.Routines + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.ForeignDataWrappers != nil { + in, out := &in.ForeignDataWrappers, &out.ForeignDataWrappers + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.ForeignServers != nil { + in, out := &in.ForeignServers, &out.ForeignServers *out = make([]string, len(*in)) copy(*out, *in) } diff --git a/cluster/local/postgresdb_functions.sh b/cluster/local/postgresdb_functions.sh index 4a64c7e2..82384b94 100644 --- a/cluster/local/postgresdb_functions.sh +++ b/cluster/local/postgresdb_functions.sh @@ -66,13 +66,13 @@ echo_step "check if database is ready" "${KUBECTL}" wait --timeout 2m --for condition=Ready -f ${projectdir}/examples/postgresql/database.yaml echo_step_completed -echo_step "check if grant is ready" -"${KUBECTL}" wait --timeout 2m --for condition=Ready -f ${projectdir}/examples/postgresql/grant.yaml -echo_step_completed - echo_step "check if schema is ready" "${KUBECTL}" wait --timeout 2m --for condition=Ready -f ${projectdir}/examples/postgresql/schema.yaml echo_step_completed + +echo_step "check if grant is ready" +"${KUBECTL}" wait --timeout 2m --for condition=Ready -f ${projectdir}/examples/postgresql/grant.yaml +echo_step_completed } check_all_roles_privileges() { @@ -116,26 +116,52 @@ check_role_privileges() { fi } -check_schema_privileges(){ +check_all_schema_privileges() { # check if schema privileges are set properly echo_step "check if schema privileges are set properly" - TARGET_DB='db1' + OWNER_ROLE='ownerrole' + USER_ROLE='no-grants-role' - nspacl=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "SELECT nspacl FROM pg_namespace WHERE nspname = 'public';") - nspacl=$(echo "$nspacl" | xargs) + # Define roles and their expected privileges + roles="$OWNER_ROLE $USER_ROLE" + dbs="db1 example" + schemas="public my-schema" + privileges="USAGE|f,CREATE|f USAGE|t,CREATE|t" - if [[ "$nspacl" == "{ownerrole=UC/ownerrole}" ]]; then - echo "Privileges on schema public are as expected: $nspacl" - echo_info "OK" - else - echo "Privileges on schema public are NOT as expected: $nspacl" - echo_error "Not OK" - fi + # Iterate over roles and expected privileges + role_index=1 + for role in $roles; do + expected_privileges=$(echo "$privileges" | cut -d ' ' -f $role_index) + target_db=$(echo "$dbs" | cut -d ' ' -f $role_index) + target_schema=$(echo "$schemas" | cut -d ' ' -f $role_index) + check_schema_privileges "$role" "$expected_privileges" "${postgres_root_pw}" "$target_db" "$target_schema" + role_index=$((role_index + 1)) + done echo_step_completed } +check_schema_privileges(){ + local role=$1 + local expected_privileges=$2 + local target_db=$4 + local target_schema=$5 + + request="select acl.privilege_type, acl.is_grantable from pg_namespace n, aclexplode(n.nspacl) acl INNER JOIN pg_roles s ON acl.grantee = s.oid where n.nspname = '$target_schema' and s.rolname='$role'" + + nspacl=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$target_db" -wtAc "$request") + nspacl=$(echo "$nspacl" | xargs | tr ' ' ',') + + if [[ "$nspacl" == "$expected_privileges" ]]; then + echo "Privileges on schema $target_db.$target_schema for role $role are as expected: $nspacl" + echo_info "OK" + else + echo "Privileges on schema $target_db.$target_schema for role $role are NOT as expected: $nspacl" + echo_error "Not OK" + fi +} + setup_observe_only_database(){ echo_step "create pre-existing database for observe only" @@ -196,6 +222,6 @@ integration_tests_postgres() { setup_postgresdb_tests check_observe_only_database check_all_roles_privileges - check_schema_privileges + check_all_schema_privileges delete_postgresdb_resources } \ No newline at end of file diff --git a/examples/postgresql/grant.yaml b/examples/postgresql/grant.yaml index b37be69c..76e3ef4d 100644 --- a/examples/postgresql/grant.yaml +++ b/examples/postgresql/grant.yaml @@ -64,3 +64,19 @@ spec: databaseRef: name: "db1" revokePublicOnDb: true +--- +apiVersion: postgresql.sql.crossplane.io/v1alpha1 +kind: Grant +metadata: + name: example-grant-role-1-on-schema +spec: + forProvider: + privileges: + - ALL + withOption: GRANT + roleRef: + name: no-grants-role + databaseRef: + name: example + schemaRef: + name: my-schema diff --git a/examples/postgresql/role.yaml b/examples/postgresql/role.yaml index 5f07914c..860de429 100644 --- a/examples/postgresql/role.yaml +++ b/examples/postgresql/role.yaml @@ -42,4 +42,16 @@ spec: createDb: true login: true createRole: true - inherit: true \ No newline at end of file + inherit: true +--- +apiVersion: postgresql.sql.crossplane.io/v1alpha1 +kind: Role +metadata: + name: no-grants-role +spec: + writeConnectionSecretToRef: + name: no-grants-role-secret + namespace: default + forProvider: + privileges: + login: true \ No newline at end of file diff --git a/package/crds/postgresql.sql.crossplane.io_grants.yaml b/package/crds/postgresql.sql.crossplane.io_grants.yaml index a95a3a8e..245979be 100644 --- a/package/crds/postgresql.sql.crossplane.io_grants.yaml +++ b/package/crds/postgresql.sql.crossplane.io_grants.yaml @@ -84,9 +84,7 @@ spec: grant instance. properties: columns: - description: |- - The columns upon which to grant the privileges. - Required when object_type is column. You cannot specify this option if the object_type is not column + description: The columns upon which to grant the privileges. items: type: string type: array @@ -169,6 +167,22 @@ spec: type: string type: object type: object + foreignDataWrappers: + description: The foreign data wrappers upon which to grant the + privileges. + items: + type: string + type: array + foreignServers: + description: The foreign servers upon which to grant the privileges. + items: + type: string + type: array + functions: + description: The functions upon which to grant the privileges. + items: + type: string + type: array memberOf: description: MemberOf is the Role that this grant makes Role a member of. @@ -249,30 +263,6 @@ spec: type: string type: object type: object - objectType: - description: ObjectType is the PostgreSQL object type to grant - the privileges on. - enum: - - database - - schema - - table - - sequence - - function - - procedure - - routine - - foreign_data_wrapper - - foreign_server - - column - type: string - objects: - description: |- - Objects are the objects upon which to grant the privileges. - An empty list (the default) means to grant permissions on all objects of the specified type. - You cannot specify this option if the objectType is database or schema. - When objectType is column, only one value is allowed. - items: - type: string - type: array privileges: description: |- Privileges to be granted. @@ -283,6 +273,11 @@ spec: type: string minItems: 1 type: array + procedures: + description: The procedures upon which to grant the privileges. + items: + type: string + type: array revokePublicOnDb: description: RevokePublicOnDb apply the statement "REVOKE ALL ON DATABASE %s FROM PUBLIC" to make database unreachable from @@ -367,6 +362,11 @@ spec: type: string type: object type: object + routines: + description: The routines upon which to grant the privileges. + items: + type: string + type: array schema: description: Schema this grant is for. type: string @@ -446,6 +446,16 @@ spec: type: string type: object type: object + sequences: + description: The sequences upon which to grant the privileges. + items: + type: string + type: array + tables: + description: The tables upon which to grant the privileges. + items: + type: string + type: array withOption: description: |- WithOption allows an option to be set on the grant. @@ -455,8 +465,6 @@ spec: - ADMIN - GRANT type: string - required: - - objectType type: object managementPolicies: default: diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index 766ca8a5..d3898213 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -34,6 +34,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/event" "github.com/crossplane/crossplane-runtime/pkg/feature" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" + "github.com/crossplane/crossplane-runtime/pkg/reference" "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/crossplane-contrib/provider-sql/apis/postgresql/v1alpha1" @@ -48,19 +49,15 @@ const ( errNoSecretRef = "ProviderConfig does not reference a credentials Secret" errGetSecret = "cannot get credentials Secret" - errNotGrant = "managed resource is not a Grant custom resource" - errSelectGrant = "cannot select grant" - errCreateGrant = "cannot create grant" - errRevokeGrant = "cannot revoke grant" - errNoSchema = "schema not passed or could not be resolved" - errNoRole = "role not passed or could not be resolved" - errNoDatabase = "database not passed or could not be resolved" - errNoPrivileges = "privileges not passed" - errUnknownGrant = "cannot identify grant type based on passed params" + errNotGrant = "managed resource is not a Grant custom resource" + errSelectGrant = "cannot select grant" + errCreateGrant = "cannot create grant" + errRevokeGrant = "cannot revoke grant" + errNoRole = "role not passed or could not be resolved" - errInvalidParams = "invalid parameters for grant type %s" - - errMemberOfWithDatabaseOrPrivileges = "cannot set privileges or database in the same grant as memberOf" + errUnknownGrant = "cannot identify grant type based on passed params" + errUnsupportedGrant = "identified grant type is not supported" + errInvalidParams = "invalid parameters for grant type %s" maxConcurrency = 5 ) @@ -127,8 +124,12 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E if err := c.kube.Get(ctx, types.NamespacedName{Namespace: ref.Namespace, Name: ref.Name}, s); err != nil { return nil, errors.Wrap(err, errGetSecret) } + database := reference.FromPtrValue(cr.Spec.ForProvider.Database) + if database == "" { + database = pc.Spec.DefaultDatabase + } return &external{ - db: c.newDB(s.Data, pc.Spec.DefaultDatabase, clients.ToString(pc.Spec.SSLMode)), + db: c.newDB(s.Data, database, clients.ToString(pc.Spec.SSLMode)), kube: c.kube, }, nil } @@ -138,81 +139,14 @@ type external struct { kube client.Client } -type grantType string - -const ( - roleMember grantType = "ROLE_MEMBER" - roleDatabase grantType = "ROLE_DATABASE" -) - -// sliceContainsStr checks if a slice contains a specific string. -func sliceContainsStr(haystack []string, needle string) bool { - for _, s := range haystack { - if s == needle { - return true - } - } - return false -} - -func validateGrantParams(gp v1alpha1.GrantParameters) error { - if gp.Schema == nil && !sliceContainsStr([]string{"database", "foreign_data_wrapper", "foreign_server"}, gp.ObjectType) { - return fmt.Errorf("parameter 'schema' is mandatory for grant resource") - } - if len(gp.Objects) > 0 && (gp.ObjectType == "database" || gp.ObjectType == "schema") { - return fmt.Errorf("cannot specify `objects` when `object_type` is `database` or `schema`") - } - if len(gp.Columns) > 0 && gp.ObjectType != "column" { - return fmt.Errorf("cannot specify `columns` when `object_type` is not `column`") - } - if len(gp.Columns) == 0 && gp.ObjectType == "column" { - return fmt.Errorf("must specify `columns` when `object_type` is `column`") - } - if len(gp.Privileges) != 1 && gp.ObjectType == "column" { - return fmt.Errorf("must specify exactly 1 `privileges` when `object_type` is `column`") - } - if len(gp.Objects) != 1 && gp.ObjectType == "column" { - return fmt.Errorf("must specify exactly 1 table in the `objects` field when `object_type` is `column`") - } - if len(gp.Objects) != 1 && (gp.ObjectType == "foreign_data_wrapper" || gp.ObjectType == "foreign_server") { - return fmt.Errorf("one element must be specified in `objects` when `object_type` is `foreign_data_wrapper` or `foreign_server`") - } - return nil -} - -func identifyGrantType(gp v1alpha1.GrantParameters) (grantType, error) { - pc := len(gp.Privileges) - - // If memberOf is specified, this is ROLE_MEMBER - // NOTE: If any of these are set, even if the lookup by ref or selector fails, - // then this is still a roleMember grant type. - if gp.MemberOfRef != nil || gp.MemberOfSelector != nil || gp.MemberOf != nil { - if gp.Database != nil || pc > 0 { - return "", errors.New(errMemberOfWithDatabaseOrPrivileges) - } - return roleMember, nil - } - - if gp.Database == nil { - return "", errors.New(errNoDatabase) - } - - if pc < 1 { - return "", errors.New(errNoPrivileges) - } - - // This is ROLE_DATABASE - return roleDatabase, nil -} - func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { - gt, err := identifyGrantType(gp) + gt, err := gp.IdentifyGrantType() if err != nil { return err } switch gt { - case roleMember: + case v1alpha1.RoleMember: ao := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionAdmin // Always returns a row with a true or false value @@ -232,46 +166,65 @@ func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { ao, } return nil - case roleDatabase: + case v1alpha1.RoleDatabase: gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant - ep := gp.Privileges.ExpandPrivileges() + ep := gp.ExpandPrivileges() sp := ep.ToStringSlice() - // Join grantee. Filter by database name and grantee name. // Finally, perform a permission comparison against expected // permissions. q.String = "SELECT EXISTS(SELECT 1 " + - "FROM pg_database db " + - "JOIN pg_namespace nsp ON db.datname = $1 " + - "JOIN LATERAL aclexplode(nsp.nspacl) acl ON true " + - "JOIN pg_roles s ON acl.grantee = s.oid " + - // Filter by role, schema and grantable setting - "WHERE nsp.nspname = $2 " + - "AND s.rolname = $3 " + - "AND acl.is_grantable = $6 " + - "GROUP BY db.datname, nsp.nspname, s.rolname, acl.is_grantable " + - "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) = " + - "(SELECT array(SELECT unnest($7::text[]) ORDER BY 1)))" + "FROM pg_database db, " + + "aclexplode(datacl) as acl " + + "INNER JOIN pg_roles s ON acl.grantee = s.oid " + + // Filter by database, role and grantable setting + "WHERE db.datname=$1 " + + "AND s.rolname=$2 " + + "AND acl.is_grantable=$3 " + + "GROUP BY db.datname, s.rolname, acl.is_grantable " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($4::text[]) as perms ORDER BY perms ASC)))" q.Parameters = []interface{}{ gp.Database, - gp.Schema, gp.Role, - gp.ObjectType, - gp.Objects, gro, pq.Array(sp), } - } - return errors.New(errUnknownGrant) -} + return nil + case v1alpha1.RoleSchema: + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant -func withSchema(schema *string) string { - if schema != nil { - return fmt.Sprintf("IN SCHEMA %s", *schema) + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + // Join grantee. Filter by schema name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT EXISTS(SELECT 1 " + + "FROM pg_namespace n, " + + "aclexplode(nspacl) as acl " + + "INNER JOIN pg_roles s ON acl.grantee = s.oid " + + // Filter by schema, role and grantable setting + "WHERE n.nspname=$1 " + + "AND s.rolname=$2 " + + "AND acl.is_grantable=$3 " + + "GROUP BY n.nspname, s.rolname, acl.is_grantable " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($4::text[]) as perms ORDER BY perms ASC)))" + q.Parameters = []interface{}{ + gp.Schema, + gp.Role, + gro, + pq.Array(sp), + } + return nil } - return "" + return errors.New(errUnsupportedGrant) } func withOption(option *v1alpha1.GrantOption) string { @@ -282,7 +235,7 @@ func withOption(option *v1alpha1.GrantOption) string { } func createGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query) error { // nolint: gocyclo - gt, err := identifyGrantType(gp) + gt, err := gp.IdentifyGrantType() if err != nil { return err } @@ -290,61 +243,96 @@ func createGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query) error { / ro := pq.QuoteIdentifier(*gp.Role) switch gt { - case roleMember: - if gp.MemberOf == nil || gp.Role == nil { - return errors.Errorf(errInvalidParams, roleMember) - } + case v1alpha1.RoleMember: + return createRoleMember(gp, ql, ro) + case v1alpha1.RoleDatabase: + return createRoleDatabase(gp, ql, ro) + case v1alpha1.RoleSchema: + return createRoleSchema(gp, ql, ro) + } + return errors.New(errUnsupportedGrant) +} - mo := pq.QuoteIdentifier(*gp.MemberOf) +func createRoleMember(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { + if gp.MemberOf == nil || gp.Role == nil { + return errors.Errorf(errInvalidParams, v1alpha1.RoleMember) + } - *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), - )}, - ) - return nil - case roleDatabase: - if gp.Database == nil || gp.Role == nil || len(gp.Privileges) < 1 { - return errors.Errorf(errInvalidParams, roleDatabase) - } + mo := pq.QuoteIdentifier(*gp.MemberOf) + + *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), + )}, + ) + return nil +} - db := pq.QuoteIdentifier(*gp.Database) - sp := strings.Join(gp.Privileges.ToStringSlice(), ",") +func createRoleDatabase(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { + if gp.Database == nil || gp.Role == nil || len(gp.Privileges) < 1 { + return errors.Errorf(errInvalidParams, v1alpha1.RoleDatabase) + } - *ql = append(*ql, - // REVOKE ANY MATCHING EXISTING PERMISSIONS - xsql.Query{String: fmt.Sprintf("REVOKE %s ON DATABASE %s %s FROM %s", - sp, - db, - withSchema(gp.Schema), - ro, - )}, + db := pq.QuoteIdentifier(*gp.Database) + sp := strings.Join(gp.Privileges.ToStringSlice(), ",") + + *ql = append(*ql, + // REVOKE ANY MATCHING EXISTING PERMISSIONS + xsql.Query{String: fmt.Sprintf("REVOKE %s ON DATABASE %s FROM %s", + sp, + db, + ro, + )}, - // GRANT REQUESTED PERMISSIONS - xsql.Query{String: fmt.Sprintf("GRANT %s ON DATABASE %s TO %s %s %s", - sp, + // GRANT REQUESTED PERMISSIONS + xsql.Query{String: fmt.Sprintf("GRANT %s ON DATABASE %s TO %s %s", + sp, + db, + ro, + withOption(gp.WithOption), + )}, + ) + if gp.RevokePublicOnDb != nil && *gp.RevokePublicOnDb { + *ql = append(*ql, + // REVOKE FROM PUBLIC + xsql.Query{String: fmt.Sprintf("REVOKE ALL ON DATABASE %s FROM PUBLIC", db, - ro, - withSchema(gp.Schema), - withOption(gp.WithOption), )}, ) - if gp.RevokePublicOnDb != nil && *gp.RevokePublicOnDb { - *ql = append(*ql, - // REVOKE FROM PUBLIC - xsql.Query{String: fmt.Sprintf("REVOKE ALL ON DATABASE %s FROM PUBLIC", - db, - )}, - ) - } - return nil } - return errors.New(errUnknownGrant) + return nil +} + +func createRoleSchema(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { + if gp.Database == nil || gp.Schema == nil || gp.Role == nil || len(gp.Privileges) < 1 { + return errors.Errorf(errInvalidParams, v1alpha1.RoleDatabase) + } + + sh := pq.QuoteIdentifier(*gp.Schema) + sp := strings.Join(gp.Privileges.ToStringSlice(), ",") + + *ql = append(*ql, + // REVOKE ANY MATCHING EXISTING PERMISSIONS + xsql.Query{String: fmt.Sprintf("REVOKE %s ON SCHEMA %s FROM %s", + sp, + sh, + ro, + )}, + + // GRANT REQUESTED PERMISSIONS + xsql.Query{String: fmt.Sprintf("GRANT %s ON SCHEMA %s TO %s %s", + sp, + sh, + ro, + withOption(gp.WithOption), + )}, + ) + return nil } func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { - gt, err := identifyGrantType(gp) + gt, err := gp.IdentifyGrantType() if err != nil { return err } @@ -352,22 +340,28 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { ro := pq.QuoteIdentifier(*gp.Role) switch gt { - case roleMember: + case v1alpha1.RoleMember: q.String = fmt.Sprintf("REVOKE %s FROM %s", pq.QuoteIdentifier(*gp.MemberOf), ro, ) return nil - case roleDatabase: - q.String = fmt.Sprintf("REVOKE %s ON DATABASE %s %s FROM %s", + case v1alpha1.RoleDatabase: + q.String = fmt.Sprintf("REVOKE %s ON DATABASE %s FROM %s", strings.Join(gp.Privileges.ToStringSlice(), ","), pq.QuoteIdentifier(*gp.Database), - withSchema(gp.Schema), + ro, + ) + return nil + case v1alpha1.RoleSchema: + q.String = fmt.Sprintf("REVOKE %s ON SCHEMA %s FROM %s", + strings.Join(gp.Privileges.ToStringSlice(), ","), + pq.QuoteIdentifier(*gp.Schema), ro, ) return nil } - return errors.New(errUnknownGrant) + return errors.New(errUnsupportedGrant) } func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) { @@ -382,7 +376,10 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex gp := cr.Spec.ForProvider var query xsql.Query - if err := selectGrantQuery(gp, &query); err != nil { + + err := selectGrantQuery(gp, &query) + + if err != nil { return managed.ExternalObservation{}, err } @@ -412,11 +409,6 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext return managed.ExternalCreation{}, errors.New(errNotGrant) } - // Validate grant specs - if err := validateGrantParams(cr.Spec.ForProvider); err != nil { - return managed.ExternalCreation{}, errors.Wrap(err, errInvalidParams) - } - var queries []xsql.Query cr.SetConditions(xpv1.Creating()) @@ -424,9 +416,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext if err := createGrantQueries(cr.Spec.ForProvider, &queries); err != nil { return managed.ExternalCreation{}, errors.Wrap(err, errCreateGrant) } - - err := c.db.ExecTx(ctx, queries) - return managed.ExternalCreation{}, errors.Wrap(err, errCreateGrant) + return managed.ExternalCreation{}, errors.Wrap(c.db.ExecTx(ctx, queries), errCreateGrant) } func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) { @@ -440,12 +430,12 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) error { if !ok { return errors.New(errNotGrant) } + var query xsql.Query cr.SetConditions(xpv1.Deleting()) - err := deleteGrantQuery(cr.Spec.ForProvider, &query) - if err != nil { + if err := deleteGrantQuery(cr.Spec.ForProvider, &query); err != nil { return errors.Wrap(err, errRevokeGrant) } diff --git a/pkg/controller/postgresql/grant/reconciler_test.go b/pkg/controller/postgresql/grant/reconciler_test.go index 8ab046db..0f3c95ab 100644 --- a/pkg/controller/postgresql/grant/reconciler_test.go +++ b/pkg/controller/postgresql/grant/reconciler_test.go @@ -220,6 +220,24 @@ func TestObserve(t *testing.T) { err: errors.New(errNotGrant), }, }, + "ErrBadGrant": { + reason: "An error should be returned if the managed resource has no identifiable grant type", + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Tables: []string{"test-example"}, + Role: ptr.To("test-example"), + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: want{ + err: errors.New(errUnknownGrant), + }, + }, "SuccessNoGrant": { reason: "We should return ResourceExists: false when no grant is found", fields: fields{ @@ -382,6 +400,38 @@ func TestObserve(t *testing.T) { err: nil, }, }, + "SuccessRoleSchema": { + reason: "We should return no error if we can find our role-schema grant", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("testdb"), + Role: ptr.To("testrole"), + Schema: ptr.To("testschema"), + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + WithOption: &gog, + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, } for name, tc := range cases { @@ -430,6 +480,24 @@ func TestCreate(t *testing.T) { err: errors.New(errNotGrant), }, }, + "ErrBadGrant": { + reason: "An error should be returned if the managed resource has no identifiable grant type", + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Tables: []string{"test-example"}, + Role: ptr.To("test-example"), + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: want{ + err: errors.Wrap(errors.New(errUnknownGrant), errCreateGrant), + }, + }, "ErrExec": { reason: "Any errors encountered while creating the grant should be returned", fields: fields{ @@ -452,8 +520,8 @@ func TestCreate(t *testing.T) { err: errors.Wrap(errBoom, errCreateGrant), }, }, - "Success": { - reason: "No error should be returned when we successfully create a grant", + "RoleDatabaseSuccess": { + reason: "No error should be returned when we successfully create a role-database grant", fields: fields{ db: &mockDB{ MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil }, @@ -474,6 +542,29 @@ func TestCreate(t *testing.T) { err: nil, }, }, + "RoleSchemaSuccess": { + reason: "No error should be returned when we successfully create a role-schema grant", + fields: fields{ + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + Schema: ptr.To("test-example"), + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, } for name, tc := range cases { @@ -571,6 +662,22 @@ func TestDelete(t *testing.T) { }, want: errors.New(errNotGrant), }, + "ErrBadGrant": { + reason: "An error should be returned if the managed resource has no identifiable grant type", + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Tables: []string{"test-example"}, + Role: ptr.To("test-example"), + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: errors.Wrap(errors.New(errUnknownGrant), errRevokeGrant), + }, "ErrDropGrant": { reason: "Errors dropping a grant should be returned", fields: fields{ @@ -593,14 +700,35 @@ func TestDelete(t *testing.T) { }, want: errors.Wrap(errBoom, errRevokeGrant), }, - "Success": { - reason: "No error should be returned if the grant was revoked", + "RoleDatabaseSuccess": { + reason: "No error should be returned if the role-database grant was revoked", + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + }, + }, + want: nil, + }, + "RoleSchemaSuccess": { + reason: "No error should be returned if the role-schema grant was revoked", args: args{ mg: &v1alpha1.Grant{ Spec: v1alpha1.GrantSpec{ ForProvider: v1alpha1.GrantParameters{ Database: ptr.To("test-example"), Role: ptr.To("test-example"), + Schema: ptr.To("test-example"), Privileges: v1alpha1.GrantPrivileges{"ALL"}, }, }, From 2920f03d787a8f57362bf9b7eef34df5b0d4e373 Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Tue, 13 May 2025 11:12:46 +0200 Subject: [PATCH 03/12] feat: support table grants Signed-off-by: Julien Christophe --- apis/postgresql/v1alpha1/grant_types.go | 8 ++ cluster/local/postgresdb_functions.sh | 36 ++++++ examples/postgresql/grant.yaml | 18 +++ pkg/controller/postgresql/grant/reconciler.go | 122 +++++++++++++++++- 4 files changed, 179 insertions(+), 5 deletions(-) diff --git a/apis/postgresql/v1alpha1/grant_types.go b/apis/postgresql/v1alpha1/grant_types.go index cb7a4ef1..da111380 100644 --- a/apis/postgresql/v1alpha1/grant_types.go +++ b/apis/postgresql/v1alpha1/grant_types.go @@ -176,6 +176,14 @@ var grantReplacements = map[GrantType]map[GrantPrivilege]GrantPrivileges{ "ALL": {"CREATE", "USAGE"}, "ALL PRIVILEGES": {"CREATE", "USAGE"}, }, + RoleTable: { + "ALL": {"SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", "MAINTAIN"}, + "ALL PRIVILEGES": {"SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", "MAINTAIN"}, + }, + RoleSequence: { + "ALL": {"USAGE", "SELECT", "UPDATE"}, + "ALL PRIVILEGES": {"USAGE", "SELECT", "UPDATE"}, + }, } // ExpandPrivileges expands any shorthand privileges to their full equivalents. diff --git a/cluster/local/postgresdb_functions.sh b/cluster/local/postgresdb_functions.sh index 82384b94..762fa193 100644 --- a/cluster/local/postgresdb_functions.sh +++ b/cluster/local/postgresdb_functions.sh @@ -40,6 +40,30 @@ EOF echo "${yaml}" | "${KUBECTL}" apply -f - } +create_grantable_objects() { + TARGET_DB='db1' + TARGE_SCHEMA='public' + request="CREATE TABLE \"$TARGE_SCHEMA\".test_table(column1 INT NULL)" + create_table=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") + if [ $? -eq 0 ]; then + echo_info "PostgresDB test_table created in schema public" + else + echo_error "ERROR: could not create grantable objects: $create_table" + fi +} + +delete_grantable_objects() { + TARGET_DB='db1' + TARGE_SCHEMA='public' + request="DROP TABLE \"$TARGE_SCHEMA\".test_table" + drop_table=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") + if [ $? -eq 0 ]; then + echo_info "PostgresDB test_table dropped from schema public" + else + echo_error "ERROR: could not delete grantable objects: $drop_table" + fi +} + setup_postgresdb_tests(){ # install provider resources echo_step "creating PostgresDB Database resource" @@ -70,6 +94,10 @@ echo_step "check if schema is ready" "${KUBECTL}" wait --timeout 2m --for condition=Ready -f ${projectdir}/examples/postgresql/schema.yaml echo_step_completed +echo_step "create grantable objects" +create_grantable_objects +echo_step_completed + echo_step "check if grant is ready" "${KUBECTL}" wait --timeout 2m --for condition=Ready -f ${projectdir}/examples/postgresql/grant.yaml echo_step_completed @@ -194,7 +222,14 @@ check_observe_only_database(){ echo_step_completed } +check_custom_object_privileges(){ + echo_info "nop" +} + delete_postgresdb_resources(){ + echo_step "deleting grantable resources" + delete_grantable_objects + # uninstall echo_step "uninstalling ${PROJECT_NAME}" "${KUBECTL}" delete -f "${projectdir}/examples/postgresql/grant.yaml" @@ -223,5 +258,6 @@ integration_tests_postgres() { check_observe_only_database check_all_roles_privileges check_all_schema_privileges + check_custom_object_privileges delete_postgresdb_resources } \ No newline at end of file diff --git a/examples/postgresql/grant.yaml b/examples/postgresql/grant.yaml index 76e3ef4d..beb61d03 100644 --- a/examples/postgresql/grant.yaml +++ b/examples/postgresql/grant.yaml @@ -80,3 +80,21 @@ spec: name: example schemaRef: name: my-schema +--- +apiVersion: postgresql.sql.crossplane.io/v1alpha1 +kind: Grant +metadata: + name: example-grant-role-1-on-table +spec: + forProvider: + privileges: + - ALL + roleRef: + name: no-grants-role + databaseRef: + name: db1 + schemaRef: + name: public + tables: + - test_table + diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index d3898213..569ae8bb 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -56,7 +56,7 @@ const ( errNoRole = "role not passed or could not be resolved" errUnknownGrant = "cannot identify grant type based on passed params" - errUnsupportedGrant = "identified grant type is not supported" + errUnsupportedGrant = "grant type not supported: %s" errInvalidParams = "invalid parameters for grant type %s" maxConcurrency = 5 @@ -223,8 +223,40 @@ func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { pq.Array(sp), } return nil + case v1alpha1.RoleTable: + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + + // Join grantee. Filter by schema name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) = $1 AS ct " + + "FROM (SELECT 1 " + + "FROM information_schema.role_table_grants " + + // Filter by table, schema, role and grantable setting + "WHERE table_schema=$2 " + + "AND table_name = ANY($3) " + + "AND grantee=$4 " + + "AND is_grantable=$5 " + + "GROUP BY table_name " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($6::text[]) as perms ORDER BY perms ASC))" + + ") sub" + q.Parameters = []interface{}{ + len(gp.Tables), + gp.Schema, + pq.Array(gp.Tables), + gp.Role, + gro, + pq.Array(sp), + } + return nil } - return errors.New(errUnsupportedGrant) + return errors.Errorf(errUnsupportedGrant, gt) } func withOption(option *v1alpha1.GrantOption) string { @@ -249,8 +281,12 @@ func createGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query) error { / return createRoleDatabase(gp, ql, ro) case v1alpha1.RoleSchema: return createRoleSchema(gp, ql, ro) + case v1alpha1.RoleTable: + return createRoleTable(gp, ql, ro) + case v1alpha1.RoleSequence: + return createRoleSequence(gp, ql, ro) } - return errors.New(errUnsupportedGrant) + return errors.Errorf(errUnsupportedGrant, gt) } func createRoleMember(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { @@ -306,7 +342,7 @@ func createRoleDatabase(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string func createRoleSchema(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || gp.Schema == nil || gp.Role == nil || len(gp.Privileges) < 1 { - return errors.Errorf(errInvalidParams, v1alpha1.RoleDatabase) + return errors.Errorf(errInvalidParams, v1alpha1.RoleSchema) } sh := pq.QuoteIdentifier(*gp.Schema) @@ -331,6 +367,66 @@ func createRoleSchema(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) return nil } +func createRoleTable(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { + if gp.Database == nil || gp.Schema == nil || len(gp.Tables) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { + return errors.Errorf(errInvalidParams, v1alpha1.RoleTable) + } + + sh := pq.QuoteIdentifier(*gp.Schema) + tb := strings.Join(gp.Tables, ",") + sp := strings.Join(gp.Privileges.ToStringSlice(), ",") + + *ql = append(*ql, + // REVOKE ANY MATCHING EXISTING PERMISSIONS + xsql.Query{String: fmt.Sprintf("REVOKE %s ON TABLE %s IN SCHEMA %s FROM %s", + sp, + tb, + sh, + ro, + )}, + + // GRANT REQUESTED PERMISSIONS + xsql.Query{String: fmt.Sprintf("GRANT %s ON TABLE %s IN SCHEMA %s TO %s %s", + sp, + tb, + sh, + ro, + withOption(gp.WithOption), + )}, + ) + return nil +} + +func createRoleSequence(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { + if gp.Database == nil || gp.Schema == nil || len(gp.Sequences) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { + return errors.Errorf(errInvalidParams, v1alpha1.RoleTable) + } + + sh := pq.QuoteIdentifier(*gp.Schema) + sq := strings.Join(gp.Sequences, ",") + sp := strings.Join(gp.Privileges.ToStringSlice(), ",") + + *ql = append(*ql, + // REVOKE ANY MATCHING EXISTING PERMISSIONS + xsql.Query{String: fmt.Sprintf("REVOKE %s ON SEQUENCE %s IN SCHEMA %s FROM %s", + sp, + sq, + sh, + ro, + )}, + + // GRANT REQUESTED PERMISSIONS + xsql.Query{String: fmt.Sprintf("GRANT %s ON SEQUENCE %s IN SCHEMA %s TO %s %s", + sp, + sq, + sh, + ro, + withOption(gp.WithOption), + )}, + ) + return nil +} + func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { gt, err := gp.IdentifyGrantType() if err != nil { @@ -360,8 +456,24 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { ro, ) return nil + case v1alpha1.RoleTable: + q.String = fmt.Sprintf("REVOKE %s ON TABLE %s IN SCHEMA %s FROM %s", + strings.Join(gp.Privileges.ToStringSlice(), ","), + strings.Join(gp.Tables, ","), + pq.QuoteIdentifier(*gp.Schema), + ro, + ) + return nil + case v1alpha1.RoleSequence: + q.String = fmt.Sprintf("REVOKE %s ON SEQUENCE %s IN SCHEMA %s FROM %s", + strings.Join(gp.Privileges.ToStringSlice(), ","), + strings.Join(gp.Sequences, ","), + pq.QuoteIdentifier(*gp.Schema), + ro, + ) + return nil } - return errors.New(errUnsupportedGrant) + return errors.Errorf(errUnsupportedGrant, gt) } func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) { From a2ea718c52f68647f92b29bc9f369c7efe1f86f6 Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Tue, 13 May 2025 17:11:58 +0200 Subject: [PATCH 04/12] feat: support grants on sequences Signed-off-by: Julien Christophe --- cluster/local/postgresdb_functions.sh | 109 ++++++++++++------ examples/postgresql/grant.yaml | 21 +++- pkg/controller/postgresql/grant/reconciler.go | 83 ++++++++++--- 3 files changed, 159 insertions(+), 54 deletions(-) diff --git a/cluster/local/postgresdb_functions.sh b/cluster/local/postgresdb_functions.sh index 762fa193..baadb70e 100644 --- a/cluster/local/postgresdb_functions.sh +++ b/cluster/local/postgresdb_functions.sh @@ -17,7 +17,7 @@ setup_postgresdb_no_tls() { --from-literal endpoint="postgresdb-postgresql.default.svc.cluster.local" \ --from-literal port="5432" - "${KUBECTL}" port-forward --namespace default svc/postgresdb-postgresql 5432:5432 & + "${KUBECTL}" port-forward --namespace default svc/postgresdb-postgresql 5432:5432 | grep -v "Handling connection for" & PORT_FORWARD_PID=$! } @@ -43,24 +43,26 @@ EOF create_grantable_objects() { TARGET_DB='db1' TARGE_SCHEMA='public' - request="CREATE TABLE \"$TARGE_SCHEMA\".test_table(column1 INT NULL)" - create_table=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") + request="CREATE TABLE \"$TARGE_SCHEMA\".test_table(column1 INT NULL); + CREATE SEQUENCE \"$TARGE_SCHEMA\".test_sequence START WITH 1000 INCREMENT BY 1;" + create_objects=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") if [ $? -eq 0 ]; then - echo_info "PostgresDB test_table created in schema public" + echo_info "PostgresDB objects created in schema public" else - echo_error "ERROR: could not create grantable objects: $create_table" + echo_error "ERROR: could not create grantable objects: $create_objects" fi } delete_grantable_objects() { TARGET_DB='db1' TARGE_SCHEMA='public' - request="DROP TABLE \"$TARGE_SCHEMA\".test_table" - drop_table=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") + request="DROP TABLE \"$TARGE_SCHEMA\".test_table; + DROP SEQUENCE \"$TARGE_SCHEMA\".test_sequence;" + drop_objects=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") if [ $? -eq 0 ]; then - echo_info "PostgresDB test_table dropped from schema public" + echo_info "PostgresDB objects dropped from schema public" else - echo_error "ERROR: could not delete grantable objects: $drop_table" + echo_error "ERROR: could not delete grantable objects: $drop_objects" fi } @@ -131,14 +133,14 @@ check_role_privileges() { local expected_privileges=$2 local target_db=$4 - echo_info "Checking privileges for role: $role (expected: $expected_privileges)" - echo "" + echo -n "Privileges for role: $role (expected: $expected_privileges)" + result=$(PGPASSWORD="$3" psql -h localhost -p 5432 -U postgres -d postgres -wtAc" SELECT CASE WHEN has_database_privilege('$role', '$target_db', 'CONNECT') THEN 'CONNECT' ELSE NULL END, CASE WHEN has_database_privilege('$role', '$target_db', 'CREATE') THEN 'CREATE' ELSE NULL END, CASE WHEN has_database_privilege('$role', '$target_db', 'TEMP') THEN 'TEMP' ELSE NULL END " | tr '\n' ',' | sed 's/,$//') if [ "$result" = "$expected_privileges" ]; then - echo_info "Privileges for $role are as expected: $result" - echo "" + echo " condition met" else + echo "" echo_error "ERROR: Privileges for $role do not match expected. Found: $result, Expected: $expected_privileges" echo "" fi @@ -160,36 +162,72 @@ check_all_schema_privileges() { # Iterate over roles and expected privileges role_index=1 for role in $roles; do - expected_privileges=$(echo "$privileges" | cut -d ' ' -f $role_index) - target_db=$(echo "$dbs" | cut -d ' ' -f $role_index) - target_schema=$(echo "$schemas" | cut -d ' ' -f $role_index) - check_schema_privileges "$role" "$expected_privileges" "${postgres_root_pw}" "$target_db" "$target_schema" - role_index=$((role_index + 1)) + expected_privileges=$(echo "$privileges" | cut -d ' ' -f $role_index) + target_db=$(echo "$dbs" | cut -d ' ' -f $role_index) + target_schema=$(echo "$schemas" | cut -d ' ' -f $role_index) + check_schema_privileges "$role" "$expected_privileges" "${postgres_root_pw}" "$target_db" "$target_schema" + role_index=$((role_index + 1)) done echo_step_completed } +check_privileges(){ + local target_db=$1 + local object=$2 + local role=$3 + local expected=$4 + local request=$5 + echo -n "Privileges on $object for role: $role (expected: $expected)" + + response=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$target_db" -wtAc "$request") + response=$(echo "$response" | xargs | tr ' ' ',') + + if [[ "$response" == "$expected" ]]; then + echo " condition met" + else + echo "" + echo_error "Found unexpected privileges: $response" + echo "" + fi +} + check_schema_privileges(){ - local role=$1 - local expected_privileges=$2 - local target_db=$4 - local target_schema=$5 + local role=$1 + local expected_privileges=$2 + local target_db=$4 + local target_schema=$5 - request="select acl.privilege_type, acl.is_grantable from pg_namespace n, aclexplode(n.nspacl) acl INNER JOIN pg_roles s ON acl.grantee = s.oid where n.nspname = '$target_schema' and s.rolname='$role'" + request="select acl.privilege_type, acl.is_grantable from pg_namespace n, aclexplode(n.nspacl) acl INNER JOIN pg_roles s ON acl.grantee = s.oid where n.nspname = '$target_schema' and s.rolname='$role'" - nspacl=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$target_db" -wtAc "$request") - nspacl=$(echo "$nspacl" | xargs | tr ' ' ',') + check_privileges $target_db "schema $target_db.$target_schema" $role $expected_privileges "$request" +} - if [[ "$nspacl" == "$expected_privileges" ]]; then - echo "Privileges on schema $target_db.$target_schema for role $role are as expected: $nspacl" - echo_info "OK" - else - echo "Privileges on schema $target_db.$target_schema for role $role are NOT as expected: $nspacl" - echo_error "Not OK" - fi +check_table_privileges(){ + target_db="db1" + schema="public" + table="test_table" + role='no-grants-role' + expected_privileges='INSERT|NO,SELECT|NO' + + request="select privilege_type, is_grantable from information_schema.role_table_grants where grantee = '$role' and table_schema = '$schema' and table_name='$table' order by privilege_type asc" + + check_privileges $target_db "table $schema.$table" $role $expected_privileges "$request" } +check_sequence_privileges(){ + target_db="db1" + schema="public" + sequence="test_sequence" + role='no-grants-role' + expected_privileges='SELECT|f,UPDATE|f,USAGE|f' + + request="select acl.privilege_type, acl.is_grantable from pg_class c inner join pg_namespace n on c.relnamespace = n.oid, aclexplode(c.relacl) as acl inner join pg_roles s on acl.grantee = s.oid where c.relkind = 'S' and n.nspname = '$schema' and s.rolname='$role' and c.relname = '$sequence'" + + check_privileges $target_db "sequence $schema.$sequence" $role $expected_privileges "$request" +} + + setup_observe_only_database(){ echo_step "create pre-existing database for observe only" @@ -223,7 +261,12 @@ check_observe_only_database(){ } check_custom_object_privileges(){ - echo_info "nop" + echo_step "check if custom_object_privileges privileges are set properly" + + check_table_privileges + check_sequence_privileges + + echo_step_completed } delete_postgresdb_resources(){ diff --git a/examples/postgresql/grant.yaml b/examples/postgresql/grant.yaml index beb61d03..396f9300 100644 --- a/examples/postgresql/grant.yaml +++ b/examples/postgresql/grant.yaml @@ -88,7 +88,8 @@ metadata: spec: forProvider: privileges: - - ALL + - SELECT + - INSERT roleRef: name: no-grants-role databaseRef: @@ -97,4 +98,20 @@ spec: name: public tables: - test_table - +--- +apiVersion: postgresql.sql.crossplane.io/v1alpha1 +kind: Grant +metadata: + name: example-grant-role-1-on-sequence +spec: + forProvider: + privileges: + - ALL + roleRef: + name: no-grants-role + databaseRef: + name: db1 + schemaRef: + name: public + sequences: + - test_sequence diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index 569ae8bb..0dff4b95 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -139,6 +139,14 @@ type external struct { kube client.Client } +func yesOrNo(b bool) string { + if b { + return "YES" + } else { + return "NO" + } +} + func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { gt, err := gp.IdentifyGrantType() if err != nil { @@ -229,7 +237,7 @@ func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { ep := gp.ExpandPrivileges() sp := ep.ToStringSlice() - // Join grantee. Filter by schema name and grantee name. + // Join grantee. Filter by schema name, table name and grantee name. // Finally, perform a permission comparison against expected // permissions. q.String = "SELECT COUNT(*) = $1 AS ct " + @@ -251,9 +259,45 @@ func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { gp.Schema, pq.Array(gp.Tables), gp.Role, + yesOrNo(gro), + pq.Array(sp), + } + + return nil + case v1alpha1.RoleSequence: + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + // Join grantee. Filter by sequence name, schema name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) = $1 AS ct " + + "FROM (SELECT 1 FROM pg_class c " + + "INNER JOIN pg_namespace n ON c.relnamespace = n.oid, " + + "aclexplode(c.relacl) as acl " + + "INNER JOIN pg_roles s ON acl.grantee = s.oid " + + "WHERE c.relkind = 'S' " + + // Filter by sequence, schema, role and grantable setting + "AND n.nspname=$2 " + + "AND s.rolname=$3 " + + "AND c.relname = ANY($4) " + + "AND acl.is_grantable=$5 " + + "GROUP BY n.nspname, s.rolname, acl.is_grantable " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($6::text[]) as perms ORDER BY perms ASC))" + + ") sub" + q.Parameters = []interface{}{ + len(gp.Sequences), + gp.Schema, + gp.Role, + pq.Array(gp.Sequences), gro, pq.Array(sp), } + return nil } return errors.Errorf(errUnsupportedGrant, gt) @@ -372,24 +416,21 @@ func createRoleTable(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) e return errors.Errorf(errInvalidParams, v1alpha1.RoleTable) } - sh := pq.QuoteIdentifier(*gp.Schema) - tb := strings.Join(gp.Tables, ",") + tb := strings.Join(prefixWithSchema(*gp.Schema, gp.Tables), ",") sp := strings.Join(gp.Privileges.ToStringSlice(), ",") *ql = append(*ql, // REVOKE ANY MATCHING EXISTING PERMISSIONS - xsql.Query{String: fmt.Sprintf("REVOKE %s ON TABLE %s IN SCHEMA %s FROM %s", + xsql.Query{String: fmt.Sprintf("REVOKE %s ON TABLE %s FROM %s", sp, tb, - sh, ro, )}, // GRANT REQUESTED PERMISSIONS - xsql.Query{String: fmt.Sprintf("GRANT %s ON TABLE %s IN SCHEMA %s TO %s %s", + xsql.Query{String: fmt.Sprintf("GRANT %s ON TABLE %s TO %s %s", sp, tb, - sh, ro, withOption(gp.WithOption), )}, @@ -402,24 +443,21 @@ func createRoleSequence(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string return errors.Errorf(errInvalidParams, v1alpha1.RoleTable) } - sh := pq.QuoteIdentifier(*gp.Schema) - sq := strings.Join(gp.Sequences, ",") + sq := strings.Join(prefixWithSchema(*gp.Schema, gp.Sequences), ",") sp := strings.Join(gp.Privileges.ToStringSlice(), ",") *ql = append(*ql, // REVOKE ANY MATCHING EXISTING PERMISSIONS - xsql.Query{String: fmt.Sprintf("REVOKE %s ON SEQUENCE %s IN SCHEMA %s FROM %s", + xsql.Query{String: fmt.Sprintf("REVOKE %s ON SEQUENCE %s FROM %s", sp, sq, - sh, ro, )}, // GRANT REQUESTED PERMISSIONS - xsql.Query{String: fmt.Sprintf("GRANT %s ON SEQUENCE %s IN SCHEMA %s TO %s %s", + xsql.Query{String: fmt.Sprintf("GRANT %s ON SEQUENCE %s TO %s %s", sp, sq, - sh, ro, withOption(gp.WithOption), )}, @@ -457,18 +495,16 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { ) return nil case v1alpha1.RoleTable: - q.String = fmt.Sprintf("REVOKE %s ON TABLE %s IN SCHEMA %s FROM %s", + q.String = fmt.Sprintf("REVOKE %s ON TABLE %s FROM %s", strings.Join(gp.Privileges.ToStringSlice(), ","), - strings.Join(gp.Tables, ","), - pq.QuoteIdentifier(*gp.Schema), + strings.Join(prefixWithSchema(*gp.Schema, gp.Tables), ","), ro, ) return nil case v1alpha1.RoleSequence: - q.String = fmt.Sprintf("REVOKE %s ON SEQUENCE %s IN SCHEMA %s FROM %s", + q.String = fmt.Sprintf("REVOKE %s ON SEQUENCE %s FROM %s", strings.Join(gp.Privileges.ToStringSlice(), ","), - strings.Join(gp.Sequences, ","), - pq.QuoteIdentifier(*gp.Schema), + strings.Join(prefixWithSchema(*gp.Schema, gp.Sequences), ","), ro, ) return nil @@ -476,6 +512,15 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { return errors.Errorf(errUnsupportedGrant, gt) } +func prefixWithSchema(schema string, objects []string) []string { + sh := pq.QuoteIdentifier(schema) + objectsWithSchema := make([]string, len(objects)) + for i, object := range objects { + objectsWithSchema[i] = sh + "." + pq.QuoteIdentifier(object) + } + return objectsWithSchema +} + func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) { cr, ok := mg.(*v1alpha1.Grant) if !ok { From ae2621ca9aa8a9bad2db23865ed61f79c80ab74f Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Tue, 13 May 2025 19:51:56 +0200 Subject: [PATCH 05/12] feat: support grant on routines Signed-off-by: Julien Christophe --- apis/postgresql/v1alpha1/grant_types.go | 43 ++++--- .../v1alpha1/zz_generated.deepcopy.go | 36 ++++-- cluster/local/postgresdb_functions.sh | 27 ++++- examples/postgresql/grant.yaml | 19 +++ .../postgresql.sql.crossplane.io_grants.yaml | 21 ++-- pkg/controller/postgresql/grant/reconciler.go | 108 +++++++++++++++++- 6 files changed, 209 insertions(+), 45 deletions(-) diff --git a/apis/postgresql/v1alpha1/grant_types.go b/apis/postgresql/v1alpha1/grant_types.go index da111380..ab0688e3 100644 --- a/apis/postgresql/v1alpha1/grant_types.go +++ b/apis/postgresql/v1alpha1/grant_types.go @@ -18,7 +18,6 @@ package v1alpha1 import ( "context" - "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -58,8 +57,6 @@ const ( RoleSchema GrantType = "ROLE_SCHEMA" RoleTable GrantType = "ROLE_TABLE" RoleSequence GrantType = "ROLE_SEQUENCE" - RoleFunction GrantType = "ROLE_FUNCTION" - RoleProcedure GrantType = "ROLE_PROCEDURE" RoleRoutine GrantType = "ROLE_ROUTE" RoleColumn GrantType = "ROLE_COLUMN" RoleForeignDataWrapper GrantType = "ROLE_FOREIGN_DATA_WRAPPER" @@ -106,8 +103,6 @@ func (gp *GrantParameters) filledInFields() *stringSet { "Tables": len(gp.Tables) > 0, "Columns": len(gp.Columns) > 0, "Sequences": len(gp.Sequences) > 0, - "Functions": len(gp.Functions) > 0, - "Procedures": len(gp.Procedures) > 0, "Routines": len(gp.Routines) > 0, "ForeignServers": len(gp.ForeignServers) > 0, "ForeignDataWrappers": len(gp.ForeignDataWrappers) > 0, @@ -134,8 +129,6 @@ func (gp *GrantParameters) IdentifyGrantType() (GrantType, error) { RoleTable: {"Database", "Schema", "Tables"}, RoleColumn: {"Database", "Schema", "Tables", "Columns"}, RoleSequence: {"Database", "Schema", "Sequences"}, - RoleFunction: {"Database", "Schema", "Functions"}, - RoleProcedure: {"Database", "Schema", "Procedures"}, RoleRoutine: {"Database", "Schema", "Routines"}, RoleForeignServer: {"Database", "ForeignServers"}, RoleForeignDataWrapper: {"Database", "ForeignDataWrappers"}, @@ -165,7 +158,6 @@ func (gp *GrantParameters) IdentifyGrantType() (GrantType, error) { // happen internally inside postgresql when making grants. When we query the // privileges back, we need to look for the expanded set. // https://www.postgresql.org/docs/15/ddl-priv.html -// TODO: Grand ALL ON SCHEMA should be expanded to GRANT USAGE, CREATE ON SCHEMA var grantReplacements = map[GrantType]map[GrantPrivilege]GrantPrivileges{ RoleDatabase: { "ALL": {"CREATE", "TEMPORARY", "CONNECT"}, @@ -180,10 +172,26 @@ var grantReplacements = map[GrantType]map[GrantPrivilege]GrantPrivileges{ "ALL": {"SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", "MAINTAIN"}, "ALL PRIVILEGES": {"SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", "MAINTAIN"}, }, + RoleColumn: { + "ALL": {"SELECT", "INSERT", "UPDATE", "REFERENCES"}, + "ALL PRIVILEGES": {"SELECT", "INSERT", "UPDATE", "REFERENCES"}, + }, RoleSequence: { "ALL": {"USAGE", "SELECT", "UPDATE"}, "ALL PRIVILEGES": {"USAGE", "SELECT", "UPDATE"}, }, + RoleRoutine: { + "ALL": {"EXECUTE"}, + "ALL PRIVILEGES": {"EXECUTE"}, + }, + RoleForeignDataWrapper: { + "ALL": {"USAGE"}, + "ALL PRIVILEGES": {"USAGE"}, + }, + RoleForeignServer: { + "ALL": {"USAGE"}, + "ALL PRIVILEGES": {"USAGE"}, + }, } // ExpandPrivileges expands any shorthand privileges to their full equivalents. @@ -241,6 +249,15 @@ const ( GrantOptionGrant GrantOption = "GRANT" ) +type Routine struct { + // The name of the routine. + Name string `json:"name,omitempty"` + + // The arguments of the routine. + // +optional + Arguments []string `json:"args,omitempty"` +} + // GrantParameters define the desired state of a PostgreSQL grant instance. type GrantParameters struct { // Privileges to be granted. @@ -327,17 +344,9 @@ type GrantParameters struct { // +optional Sequences []string `json:"sequences,omitempty"` - // The functions upon which to grant the privileges. - // +optional - Functions []string `json:"functions,omitempty"` - - // The procedures upon which to grant the privileges. - // +optional - Procedures []string `json:"procedures,omitempty"` - // The routines upon which to grant the privileges. // +optional - Routines []string `json:"routines,omitempty"` + Routines []Routine `json:"routines,omitempty"` // The foreign data wrappers upon which to grant the privileges. // +optional diff --git a/apis/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/postgresql/v1alpha1/zz_generated.deepcopy.go index a0cdd16e..0761c4e6 100644 --- a/apis/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -461,20 +461,12 @@ func (in *GrantParameters) DeepCopyInto(out *GrantParameters) { *out = make([]string, len(*in)) copy(*out, *in) } - if in.Functions != nil { - in, out := &in.Functions, &out.Functions - *out = make([]string, len(*in)) - copy(*out, *in) - } - if in.Procedures != nil { - in, out := &in.Procedures, &out.Procedures - *out = make([]string, len(*in)) - copy(*out, *in) - } if in.Routines != nil { in, out := &in.Routines, &out.Routines - *out = make([]string, len(*in)) - copy(*out, *in) + *out = make([]Routine, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.ForeignDataWrappers != nil { in, out := &in.ForeignDataWrappers, &out.ForeignDataWrappers @@ -946,6 +938,26 @@ func (in *RoleStatus) DeepCopy() *RoleStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Routine) DeepCopyInto(out *Routine) { + *out = *in + if in.Arguments != nil { + in, out := &in.Arguments, &out.Arguments + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Routine. +func (in *Routine) DeepCopy() *Routine { + if in == nil { + return nil + } + out := new(Routine) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Schema) DeepCopyInto(out *Schema) { *out = *in diff --git a/cluster/local/postgresdb_functions.sh b/cluster/local/postgresdb_functions.sh index baadb70e..2dd41b37 100644 --- a/cluster/local/postgresdb_functions.sh +++ b/cluster/local/postgresdb_functions.sh @@ -43,8 +43,11 @@ EOF create_grantable_objects() { TARGET_DB='db1' TARGE_SCHEMA='public' - request="CREATE TABLE \"$TARGE_SCHEMA\".test_table(column1 INT NULL); - CREATE SEQUENCE \"$TARGE_SCHEMA\".test_sequence START WITH 1000 INCREMENT BY 1;" + request=" + CREATE TABLE \"$TARGE_SCHEMA\".test_table(column1 INT NULL); + CREATE SEQUENCE \"$TARGE_SCHEMA\".test_sequence START WITH 1000 INCREMENT BY 1; + CREATE PROCEDURE \"$TARGE_SCHEMA\".test_procedure(arg TEXT) LANGUAGE plpgsql AS \$\$ BEGIN END; \$\$; + " create_objects=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") if [ $? -eq 0 ]; then echo_info "PostgresDB objects created in schema public" @@ -56,8 +59,11 @@ create_grantable_objects() { delete_grantable_objects() { TARGET_DB='db1' TARGE_SCHEMA='public' - request="DROP TABLE \"$TARGE_SCHEMA\".test_table; - DROP SEQUENCE \"$TARGE_SCHEMA\".test_sequence;" + request=" + DROP TABLE \"$TARGE_SCHEMA\".test_table; + DROP SEQUENCE \"$TARGE_SCHEMA\".test_sequence; + DROP PROCEDURE \"$TARGE_SCHEMA\".test_procedure(TEXT); + " drop_objects=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") if [ $? -eq 0 ]; then echo_info "PostgresDB objects dropped from schema public" @@ -227,6 +233,18 @@ check_sequence_privileges(){ check_privileges $target_db "sequence $schema.$sequence" $role $expected_privileges "$request" } +check_routine_privileges(){ + target_db="db1" + schema="public" + routine="test_procedure" + role='no-grants-role' + expected_privileges='EXECUTE|NO' + + request="select privilege_type, is_grantable from information_schema.role_routine_grants where grantee = '$role' and routine_schema = '$schema' and routine_name='$routine' order by privilege_type asc" + + check_privileges $target_db "routine $schema.$routine" $role $expected_privileges "$request" +} + setup_observe_only_database(){ echo_step "create pre-existing database for observe only" @@ -265,6 +283,7 @@ check_custom_object_privileges(){ check_table_privileges check_sequence_privileges + check_routine_privileges echo_step_completed } diff --git a/examples/postgresql/grant.yaml b/examples/postgresql/grant.yaml index 396f9300..a17c6d40 100644 --- a/examples/postgresql/grant.yaml +++ b/examples/postgresql/grant.yaml @@ -115,3 +115,22 @@ spec: name: public sequences: - test_sequence +--- +apiVersion: postgresql.sql.crossplane.io/v1alpha1 +kind: Grant +metadata: + name: example-grant-role-1-on-routine +spec: + forProvider: + privileges: + - ALL + roleRef: + name: no-grants-role + databaseRef: + name: db1 + schemaRef: + name: public + routines: + - name: test_procedure + args: + - TEXT diff --git a/package/crds/postgresql.sql.crossplane.io_grants.yaml b/package/crds/postgresql.sql.crossplane.io_grants.yaml index 245979be..e16a70a0 100644 --- a/package/crds/postgresql.sql.crossplane.io_grants.yaml +++ b/package/crds/postgresql.sql.crossplane.io_grants.yaml @@ -178,11 +178,6 @@ spec: items: type: string type: array - functions: - description: The functions upon which to grant the privileges. - items: - type: string - type: array memberOf: description: MemberOf is the Role that this grant makes Role a member of. @@ -273,11 +268,6 @@ spec: type: string minItems: 1 type: array - procedures: - description: The procedures upon which to grant the privileges. - items: - type: string - type: array revokePublicOnDb: description: RevokePublicOnDb apply the statement "REVOKE ALL ON DATABASE %s FROM PUBLIC" to make database unreachable from @@ -365,7 +355,16 @@ spec: routines: description: The routines upon which to grant the privileges. items: - type: string + properties: + args: + description: The arguments of the routine. + items: + type: string + type: array + name: + description: The name of the routine. + type: string + type: object type: array schema: description: Schema this grant is for. diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index 0dff4b95..a855dc2d 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -298,6 +298,51 @@ func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { pq.Array(sp), } + return nil + case v1alpha1.RoleRoutine: + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + + routinesSignatures := make([]string, len(gp.Routines)) + for i, routine := range gp.Routines { + routinesSignatures[i] = signature(routine) + } + + // Join grantee. Filter by routine name and signature, schema name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) = $1 AS ct " + + "FROM (SELECT " + + // format routine args + "p.proname || '(' || coalesce(array_to_string(array_agg(pg_catalog.format_type(t, NULL) ORDER BY args.ord), ',')) || ')' " + + "AS signature " + + "FROM pg_proc p " + + "LEFT JOIN unnest(p.proargtypes) WITH ORDINALITY AS args(t, ord) on true " + + "INNER JOIN pg_namespace n ON p.pronamespace = n.oid, " + + "aclexplode(p.proacl) as acl " + + "INNER JOIN pg_roles s ON acl.grantee = s.oid " + + // Filter by sequence, schema, role and grantable setting + "WHERE n.nspname=$2 " + + "AND s.rolname=$3 " + + "AND acl.is_grantable=$4 " + + "GROUP BY n.nspname, s.rolname, acl.is_grantable, p.oid " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($5::text[]) as perms ORDER BY perms ASC))" + + ") sub " + + "WHERE sub.signature = ANY($6)" + q.Parameters = []interface{}{ + len(gp.Routines), + gp.Schema, + gp.Role, + gro, + pq.Array(sp), + pq.Array(routinesSignatures), + } + return nil } return errors.Errorf(errUnsupportedGrant, gt) @@ -329,6 +374,8 @@ func createGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query) error { / return createRoleTable(gp, ql, ro) case v1alpha1.RoleSequence: return createRoleSequence(gp, ql, ro) + case v1alpha1.RoleRoutine: + return createRoleRoutine(gp, ql, ro) } return errors.Errorf(errUnsupportedGrant, gt) } @@ -440,7 +487,7 @@ func createRoleTable(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) e func createRoleSequence(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || gp.Schema == nil || len(gp.Sequences) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { - return errors.Errorf(errInvalidParams, v1alpha1.RoleTable) + return errors.Errorf(errInvalidParams, v1alpha1.RoleSequence) } sq := strings.Join(prefixWithSchema(*gp.Schema, gp.Sequences), ",") @@ -465,6 +512,33 @@ func createRoleSequence(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string return nil } +func createRoleRoutine(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) + } + + rt := strings.Join(quotedSignaturesWithSchema(*gp.Schema, gp.Routines), ",") + sp := strings.Join(gp.Privileges.ToStringSlice(), ",") + + *ql = append(*ql, + // REVOKE ANY MATCHING EXISTING PERMISSIONS + xsql.Query{String: fmt.Sprintf("REVOKE %s ON ROUTINE %s FROM %s", + sp, + rt, + ro, + )}, + + // GRANT REQUESTED PERMISSIONS + xsql.Query{String: fmt.Sprintf("GRANT %s ON ROUTINE %s TO %s %s", + sp, + rt, + ro, + withOption(gp.WithOption), + )}, + ) + return nil +} + func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { gt, err := gp.IdentifyGrantType() if err != nil { @@ -508,10 +582,42 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { ro, ) return nil + case v1alpha1.RoleRoutine: + q.String = fmt.Sprintf("REVOKE %s ON ROUTINE %s FROM %s", + strings.Join(gp.Privileges.ToStringSlice(), ","), + strings.Join(quotedSignaturesWithSchema(*gp.Schema, gp.Routines), ","), + ro, + ) + return nil } return errors.Errorf(errUnsupportedGrant, gt) } +// signature returns routines with the same format as the select query +func signature(r v1alpha1.Routine) string { + args := make([]string, len(r.Arguments)) + for j, arg := range r.Arguments { + args[j] = strings.ToLower(arg) + } + return r.Name + "(" + strings.Join(args, ",") + ")" +} + +// signature returns routines with grantable format +func quotedSignaturesWithSchema(schema string, routines []v1alpha1.Routine) []string { + sh := pq.QuoteIdentifier(schema) + routinesWithSchema := make([]string, len(routines)) + + for i, routine := range routines { + args := make([]string, len(routine.Arguments)) + for j, arg := range routine.Arguments { + args[j] = strings.ToUpper(arg) + } + + routinesWithSchema[i] = sh + "." + pq.QuoteIdentifier(routine.Name) + "(" + strings.Join(args, ",") + ")" + } + return routinesWithSchema +} + func prefixWithSchema(schema string, objects []string) []string { sh := pq.QuoteIdentifier(schema) objectsWithSchema := make([]string, len(objects)) From 5fc07e49aa89e7559a4719f943ab4f0f806a60b2 Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Wed, 14 May 2025 11:43:29 +0200 Subject: [PATCH 06/12] feat: grant on columns Signed-off-by: Julien Christophe --- cluster/local/postgresdb_functions.sh | 26 +++++-- examples/postgresql/grant.yaml | 19 +++++ pkg/controller/postgresql/grant/reconciler.go | 75 +++++++++++++++++++ 3 files changed, 115 insertions(+), 5 deletions(-) diff --git a/cluster/local/postgresdb_functions.sh b/cluster/local/postgresdb_functions.sh index 2dd41b37..f44a0277 100644 --- a/cluster/local/postgresdb_functions.sh +++ b/cluster/local/postgresdb_functions.sh @@ -44,9 +44,10 @@ create_grantable_objects() { TARGET_DB='db1' TARGE_SCHEMA='public' request=" - CREATE TABLE \"$TARGE_SCHEMA\".test_table(column1 INT NULL); + CREATE TABLE \"$TARGE_SCHEMA\".test_table(col1 INT NULL); CREATE SEQUENCE \"$TARGE_SCHEMA\".test_sequence START WITH 1000 INCREMENT BY 1; CREATE PROCEDURE \"$TARGE_SCHEMA\".test_procedure(arg TEXT) LANGUAGE plpgsql AS \$\$ BEGIN END; \$\$; + CREATE TABLE \"$TARGE_SCHEMA\".test_table_column(test_column INT NULL); " create_objects=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") if [ $? -eq 0 ]; then @@ -63,6 +64,7 @@ delete_grantable_objects() { DROP TABLE \"$TARGE_SCHEMA\".test_table; DROP SEQUENCE \"$TARGE_SCHEMA\".test_sequence; DROP PROCEDURE \"$TARGE_SCHEMA\".test_procedure(TEXT); + DROP TABLE \"$TARGE_SCHEMA\".test_table_column; " drop_objects=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") if [ $? -eq 0 ]; then @@ -82,10 +84,6 @@ echo_step "creating PostgresDB Role resource" # create grant "${KUBECTL}" apply -f ${projectdir}/examples/postgresql/role.yaml -echo_step "creating PostgresDB Grant resource" -# create grant -"${KUBECTL}" apply -f ${projectdir}/examples/postgresql/grant.yaml - echo_step "creating PostgresDB Schema resources" # create grant "${KUBECTL}" apply -f ${projectdir}/examples/postgresql/schema.yaml @@ -106,6 +104,10 @@ echo_step "create grantable objects" create_grantable_objects echo_step_completed +echo_step "creating PostgresDB Grant resource" +# create grant +"${KUBECTL}" apply -f ${projectdir}/examples/postgresql/grant.yaml + echo_step "check if grant is ready" "${KUBECTL}" wait --timeout 2m --for condition=Ready -f ${projectdir}/examples/postgresql/grant.yaml echo_step_completed @@ -245,6 +247,19 @@ check_routine_privileges(){ check_privileges $target_db "routine $schema.$routine" $role $expected_privileges "$request" } +check_column_privileges(){ + target_db="db1" + schema="public" + table="test_table_column" + column="test_column" + role='no-grants-role' + expected_privileges='UPDATE|NO' + + request="select privilege_type, is_grantable from information_schema.role_column_grants where grantee = '$role' and table_schema = '$schema' and table_name='$table' and column_name='$column' order by privilege_type asc" + + check_privileges $target_db "column $column on table $schema.$table" $role $expected_privileges "$request" +} + setup_observe_only_database(){ echo_step "create pre-existing database for observe only" @@ -284,6 +299,7 @@ check_custom_object_privileges(){ check_table_privileges check_sequence_privileges check_routine_privileges + check_column_privileges echo_step_completed } diff --git a/examples/postgresql/grant.yaml b/examples/postgresql/grant.yaml index a17c6d40..1e06be7b 100644 --- a/examples/postgresql/grant.yaml +++ b/examples/postgresql/grant.yaml @@ -134,3 +134,22 @@ spec: - name: test_procedure args: - TEXT +--- +apiVersion: postgresql.sql.crossplane.io/v1alpha1 +kind: Grant +metadata: + name: example-grant-role-1-on-column +spec: + forProvider: + privileges: + - UPDATE + roleRef: + name: no-grants-role + databaseRef: + name: db1 + schemaRef: + name: public + tables: + - test_table_column + columns: + - test_column diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index a855dc2d..b1bf7563 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -343,6 +343,41 @@ func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { pq.Array(routinesSignatures), } + return nil + case v1alpha1.RoleColumn: + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + + // Join grantee. Filter by schema name, table name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) >= $1 AS ct " + + "FROM (SELECT 1 " + + "FROM information_schema.role_column_grants " + + // Filter by column, table, schema, role and grantable setting + "WHERE table_schema=$2 " + + "AND table_name = ANY($3) " + + "AND column_name = ANY($4) " + + "AND grantee=$5 " + + "AND is_grantable=$6 " + + "GROUP BY table_name, column_name " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($7::text[]) as perms ORDER BY perms ASC))" + + ") sub" + q.Parameters = []interface{}{ + len(gp.Tables) * len(gp.Columns), + gp.Schema, + pq.Array(gp.Tables), + pq.Array(gp.Columns), + gp.Role, + yesOrNo(gro), + pq.Array(sp), + } + return nil } return errors.Errorf(errUnsupportedGrant, gt) @@ -376,6 +411,8 @@ func createGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query) error { / return createRoleSequence(gp, ql, ro) case v1alpha1.RoleRoutine: return createRoleRoutine(gp, ql, ro) + case v1alpha1.RoleColumn: + return createRoleColumn(gp, ql, ro) } return errors.Errorf(errUnsupportedGrant, gt) } @@ -485,6 +522,36 @@ func createRoleTable(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) e return nil } +func createRoleColumn(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { + if gp.Database == nil || gp.Schema == nil || len(gp.Tables) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { + return errors.Errorf(errInvalidParams, v1alpha1.RoleColumn) + } + + co := strings.Join(gp.Columns, ",") + tb := strings.Join(prefixWithSchema(*gp.Schema, gp.Tables), ",") + sp := strings.Join(gp.Privileges.ToStringSlice(), ",") + + *ql = append(*ql, + // REVOKE ANY MATCHING EXISTING PERMISSIONS + xsql.Query{String: fmt.Sprintf("REVOKE %s (%s) ON TABLE %s FROM %s", + sp, + co, + tb, + ro, + )}, + + // GRANT REQUESTED PERMISSIONS + xsql.Query{String: fmt.Sprintf("GRANT %s (%s) ON TABLE %s TO %s %s", + sp, + co, + tb, + ro, + withOption(gp.WithOption), + )}, + ) + return nil +} + func createRoleSequence(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || gp.Schema == nil || len(gp.Sequences) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleSequence) @@ -589,6 +656,14 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { ro, ) return nil + case v1alpha1.RoleColumn: + q.String = fmt.Sprintf("REVOKE %s (%s) ON TABLE %s FROM %s", + strings.Join(gp.Privileges.ToStringSlice(), ","), + strings.Join(gp.Columns, ","), + strings.Join(prefixWithSchema(*gp.Schema, gp.Tables), ","), + ro, + ) + return nil } return errors.Errorf(errUnsupportedGrant, gt) } From 841c3855a37c7c75d5c2012d7a0d6be381b87734 Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Wed, 14 May 2025 13:27:35 +0200 Subject: [PATCH 07/12] feat: grants on foreign data wrappers and foreign servers Signed-off-by: Julien Christophe --- cluster/local/postgresdb_functions.sh | 33 ++++- examples/postgresql/grant.yaml | 30 ++++ pkg/controller/postgresql/grant/reconciler.go | 136 +++++++++++++++++- 3 files changed, 195 insertions(+), 4 deletions(-) diff --git a/cluster/local/postgresdb_functions.sh b/cluster/local/postgresdb_functions.sh index f44a0277..b909f8b4 100644 --- a/cluster/local/postgresdb_functions.sh +++ b/cluster/local/postgresdb_functions.sh @@ -48,6 +48,8 @@ create_grantable_objects() { CREATE SEQUENCE \"$TARGE_SCHEMA\".test_sequence START WITH 1000 INCREMENT BY 1; CREATE PROCEDURE \"$TARGE_SCHEMA\".test_procedure(arg TEXT) LANGUAGE plpgsql AS \$\$ BEGIN END; \$\$; CREATE TABLE \"$TARGE_SCHEMA\".test_table_column(test_column INT NULL); + CREATE FOREIGN DATA WRAPPER test_foreign_data_wrapper; + CREATE SERVER test_foreign_server FOREIGN DATA WRAPPER test_foreign_data_wrapper; " create_objects=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") if [ $? -eq 0 ]; then @@ -61,10 +63,12 @@ delete_grantable_objects() { TARGET_DB='db1' TARGE_SCHEMA='public' request=" - DROP TABLE \"$TARGE_SCHEMA\".test_table; - DROP SEQUENCE \"$TARGE_SCHEMA\".test_sequence; - DROP PROCEDURE \"$TARGE_SCHEMA\".test_procedure(TEXT); + DROP SERVER test_foreign_server; + DROP FOREIGN DATA WRAPPER test_foreign_data_wrapper; DROP TABLE \"$TARGE_SCHEMA\".test_table_column; + DROP PROCEDURE \"$TARGE_SCHEMA\".test_procedure(TEXT); + DROP SEQUENCE \"$TARGE_SCHEMA\".test_sequence; + DROP TABLE \"$TARGE_SCHEMA\".test_table; " drop_objects=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") if [ $? -eq 0 ]; then @@ -260,6 +264,27 @@ check_column_privileges(){ check_privileges $target_db "column $column on table $schema.$table" $role $expected_privileges "$request" } +check_foreign_data_wrapper_privileges(){ + target_db="db1" + foreign_data_wrapper="test_foreign_data_wrapper" + role='no-grants-role' + expected_privileges='USAGE|NO' + + request="select privilege_type, is_grantable from information_schema.role_usage_grants where grantee = '$role' and object_type = 'FOREIGN DATA WRAPPER' and object_name='$foreign_data_wrapper' order by privilege_type asc" + + check_privileges $target_db "foreign data wrapper $foreign_data_wrapper" $role $expected_privileges "$request" +} + +check_foreign_server_privileges(){ + target_db="db1" + foreign_server="test_foreign_server" + role='no-grants-role' + expected_privileges='USAGE|NO' + + request="select privilege_type, is_grantable from information_schema.role_usage_grants where grantee = '$role' and object_type = 'FOREIGN SERVER' and object_name='$foreign_server' order by privilege_type asc" + + check_privileges $target_db "foreign server $foreign_server" $role $expected_privileges "$request" +} setup_observe_only_database(){ echo_step "create pre-existing database for observe only" @@ -300,6 +325,8 @@ check_custom_object_privileges(){ check_sequence_privileges check_routine_privileges check_column_privileges + check_foreign_data_wrapper_privileges + check_foreign_server_privileges echo_step_completed } diff --git a/examples/postgresql/grant.yaml b/examples/postgresql/grant.yaml index 1e06be7b..ff5b1734 100644 --- a/examples/postgresql/grant.yaml +++ b/examples/postgresql/grant.yaml @@ -153,3 +153,33 @@ spec: - test_table_column columns: - test_column +--- +apiVersion: postgresql.sql.crossplane.io/v1alpha1 +kind: Grant +metadata: + name: example-grant-role-1-on-foreign-data-wrapper +spec: + forProvider: + privileges: + - USAGE + roleRef: + name: no-grants-role + databaseRef: + name: db1 + foreignDataWrappers: + - test_foreign_data_wrapper +--- +apiVersion: postgresql.sql.crossplane.io/v1alpha1 +kind: Grant +metadata: + name: example-grant-role-1-on-foreign-server +spec: + forProvider: + privileges: + - USAGE + roleRef: + name: no-grants-role + databaseRef: + name: db1 + foreignServers: + - test_foreign_server \ No newline at end of file diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index b1bf7563..c31840cf 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -378,6 +378,70 @@ func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { pq.Array(sp), } + return nil + case v1alpha1.RoleForeignDataWrapper: + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + + // Join grantee. Filter by schema name, table name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) >= $1 AS ct " + + "FROM (SELECT 1 " + + "FROM information_schema.role_usage_grants " + + // Filter by column, table, schema, role and grantable setting + "WHERE grantee=$2 " + + "AND object_type = 'FOREIGN DATA WRAPPER' " + + "AND object_name = ANY($3) " + + "AND is_grantable=$4 " + + "GROUP BY object_name " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($5::text[]) as perms ORDER BY perms ASC))" + + ") sub" + q.Parameters = []interface{}{ + len(gp.ForeignDataWrappers), + gp.Role, + pq.Array(gp.ForeignDataWrappers), + yesOrNo(gro), + pq.Array(sp), + } + + return nil + case v1alpha1.RoleForeignServer: + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + + // Join grantee. Filter by schema name, table name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) >= $1 AS ct " + + "FROM (SELECT 1 " + + "FROM information_schema.role_usage_grants " + + // Filter by column, table, schema, role and grantable setting + "WHERE grantee=$2 " + + "AND object_type = 'FOREIGN SERVER' " + + "AND object_name = ANY($3) " + + "AND is_grantable=$4 " + + "GROUP BY object_name " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($5::text[]) as perms ORDER BY perms ASC))" + + ") sub" + q.Parameters = []interface{}{ + len(gp.ForeignServers), + gp.Role, + pq.Array(gp.ForeignServers), + yesOrNo(gro), + pq.Array(sp), + } + return nil } return errors.Errorf(errUnsupportedGrant, gt) @@ -413,6 +477,10 @@ func createGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query) error { / return createRoleRoutine(gp, ql, ro) case v1alpha1.RoleColumn: return createRoleColumn(gp, ql, ro) + case v1alpha1.RoleForeignDataWrapper: + return createRoleForeignDataWrapper(gp, ql, ro) + case v1alpha1.RoleForeignServer: + return createRoleForeignServer(gp, ql, ro) } return errors.Errorf(errUnsupportedGrant, gt) } @@ -523,7 +591,7 @@ func createRoleTable(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) e } func createRoleColumn(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { - if gp.Database == nil || gp.Schema == nil || len(gp.Tables) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { + if gp.Database == nil || gp.Schema == nil || len(gp.Tables) < 1 || len(gp.Columns) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleColumn) } @@ -606,6 +674,58 @@ func createRoleRoutine(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) return nil } +func createRoleForeignDataWrapper(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { + if gp.Database == nil || len(gp.ForeignDataWrappers) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { + return errors.Errorf(errInvalidParams, v1alpha1.RoleForeignDataWrapper) + } + + sp := strings.Join(gp.Privileges.ToStringSlice(), ",") + + *ql = append(*ql, + // REVOKE ANY MATCHING EXISTING PERMISSIONS + xsql.Query{String: fmt.Sprintf("REVOKE %s ON FOREIGN DATA WRAPPER %s FROM %s", + sp, + strings.Join(gp.ForeignDataWrappers, ","), + ro, + )}, + + // GRANT REQUESTED PERMISSIONS + xsql.Query{String: fmt.Sprintf("GRANT %s ON FOREIGN DATA WRAPPER %s TO %s %s", + sp, + strings.Join(gp.ForeignDataWrappers, ","), + ro, + withOption(gp.WithOption), + )}, + ) + return nil +} + +func createRoleForeignServer(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { + if gp.Database == nil || len(gp.ForeignServers) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { + return errors.Errorf(errInvalidParams, v1alpha1.RoleForeignServer) + } + + sp := strings.Join(gp.Privileges.ToStringSlice(), ",") + + *ql = append(*ql, + // REVOKE ANY MATCHING EXISTING PERMISSIONS + xsql.Query{String: fmt.Sprintf("REVOKE %s ON FOREIGN SERVER %s FROM %s", + sp, + strings.Join(gp.ForeignServers, ","), + ro, + )}, + + // GRANT REQUESTED PERMISSIONS + xsql.Query{String: fmt.Sprintf("GRANT %s ON FOREIGN SERVER %s TO %s %s", + sp, + strings.Join(gp.ForeignServers, ","), + ro, + withOption(gp.WithOption), + )}, + ) + return nil +} + func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { gt, err := gp.IdentifyGrantType() if err != nil { @@ -664,6 +784,20 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { ro, ) return nil + case v1alpha1.RoleForeignDataWrapper: + q.String = fmt.Sprintf("REVOKE %s ON FOREIGN DATA WRAPPER %s FROM %s", + strings.Join(gp.Privileges.ToStringSlice(), ","), + strings.Join(gp.ForeignDataWrappers, ","), + ro, + ) + return nil + case v1alpha1.RoleForeignServer: + q.String = fmt.Sprintf("REVOKE %s ON FOREIGN SERVER %s FROM %s", + strings.Join(gp.Privileges.ToStringSlice(), ","), + strings.Join(gp.ForeignServers, ","), + ro, + ) + return nil } return errors.Errorf(errUnsupportedGrant, gt) } From e0f9a4b7b2440d8b2c7a1da5e63ae5d49d906dfc Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Wed, 14 May 2025 14:29:23 +0200 Subject: [PATCH 08/12] ci: make reviewable Signed-off-by: Julien Christophe --- README.md | 2 +- apis/postgresql/v1alpha1/grant_types.go | 55 +- pkg/controller/postgresql/grant/reconciler.go | 710 +++++++++--------- .../postgresql/grant/reconciler_test.go | 494 ++++++++++++ 4 files changed, 894 insertions(+), 367 deletions(-) diff --git a/README.md b/README.md index 1c67f598..2a53e79f 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ Check the example: 2. Create managed resources for your SQL server flavor: - **MySQL**: `Database`, `Grant`, `User` (See [the examples](examples/mysql)) - - **PostgreSQL**: `Database`, `Grant`, `Extension`, `Role` (See [the examples](examples/postgresql)) + - **PostgreSQL**: `Database`, `Schema`, `Grant`, `Extension`, `Role` (See [the examples](examples/postgresql)) - **MSSQL**: `Database`, `Grant`, `User` (See [the examples](examples/mssql)) [crossplane]: https://crossplane.io diff --git a/apis/postgresql/v1alpha1/grant_types.go b/apis/postgresql/v1alpha1/grant_types.go index ab0688e3..16c3a7a8 100644 --- a/apis/postgresql/v1alpha1/grant_types.go +++ b/apis/postgresql/v1alpha1/grant_types.go @@ -18,12 +18,13 @@ package v1alpha1 import ( "context" - "github.com/pkg/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/reference" + "github.com/pkg/errors" ) const ( @@ -117,41 +118,41 @@ func (gp *GrantParameters) filledInFields() *stringSet { return set } +var grantTypeFields = map[GrantType][]string{ + RoleMember: {"MemberOf"}, + RoleDatabase: {"Database"}, + RoleSchema: {"Database", "Schema"}, + RoleTable: {"Database", "Schema", "Tables"}, + RoleColumn: {"Database", "Schema", "Tables", "Columns"}, + RoleSequence: {"Database", "Schema", "Sequences"}, + RoleRoutine: {"Database", "Schema", "Routines"}, + RoleForeignServer: {"Database", "ForeignServers"}, + RoleForeignDataWrapper: {"Database", "ForeignDataWrappers"}, +} + // IdentifyGrantType return the deduced GrantType from the filled in fields. func (gp *GrantParameters) IdentifyGrantType() (GrantType, error) { - fields := gp.filledInFields() + ff := gp.filledInFields() pc := len(gp.Privileges) - grantTypeFields := map[GrantType][]string{ - RoleMember: {"MemberOf"}, - RoleDatabase: {"Database"}, - RoleSchema: {"Database", "Schema"}, - RoleTable: {"Database", "Schema", "Tables"}, - RoleColumn: {"Database", "Schema", "Tables", "Columns"}, - RoleSequence: {"Database", "Schema", "Sequences"}, - RoleRoutine: {"Database", "Schema", "Routines"}, - RoleForeignServer: {"Database", "ForeignServers"}, - RoleForeignDataWrapper: {"Database", "ForeignDataWrappers"}, - } - - var role *GrantType + var gt *GrantType - for key, expectedFields := range grantTypeFields { - if fields.containsExactly(expectedFields...) { - role = &key + for k, v := range grantTypeFields { + if ff.containsExactly(v...) { + gt = &k break } } - if role == nil { + if gt == nil { return "", errors.New(errUnknownGrant) } - if *role == RoleMember && pc > 0 { + if *gt == RoleMember && pc > 0 { return "", errors.New(errMemberOfWithPrivileges) } - if *role != RoleMember && pc < 1 { + if *gt != RoleMember && pc < 1 { return "", errors.New(errNoPrivileges) } - return *role, nil + return *gt, nil } // Some privileges are shorthands for multiple privileges. These translations @@ -196,12 +197,12 @@ var grantReplacements = map[GrantType]map[GrantPrivilege]GrantPrivileges{ // ExpandPrivileges expands any shorthand privileges to their full equivalents. func (gp *GrantParameters) ExpandPrivileges() GrantPrivileges { - grantType, err := gp.IdentifyGrantType() + gt, err := gp.IdentifyGrantType() if err != nil { return gp.Privileges } - replacements, hasReplacements := grantReplacements[grantType] - if !hasReplacements { + gr, ex := grantReplacements[gt] + if !ex { return gp.Privileges } @@ -209,8 +210,8 @@ func (gp *GrantParameters) ExpandPrivileges() GrantPrivileges { // Replace any shorthand privileges with their full equivalents for _, p := range gp.Privileges { - if _, ok := replacements[p]; ok { - for _, rp := range replacements[p] { + if _, ok := gr[p]; ok { + for _, rp := range gr[p] { privilegeSet[rp] = struct{}{} } } else { diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index c31840cf..2a2c1ad1 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -124,12 +124,12 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E if err := c.kube.Get(ctx, types.NamespacedName{Namespace: ref.Namespace, Name: ref.Name}, s); err != nil { return nil, errors.Wrap(err, errGetSecret) } - database := reference.FromPtrValue(cr.Spec.ForProvider.Database) - if database == "" { - database = pc.Spec.DefaultDatabase + db := reference.FromPtrValue(cr.Spec.ForProvider.Database) + if db == "" { + db = pc.Spec.DefaultDatabase } return &external{ - db: c.newDB(s.Data, database, clients.ToString(pc.Spec.SSLMode)), + db: c.newDB(s.Data, db, clients.ToString(pc.Spec.SSLMode)), kube: c.kube, }, nil } @@ -147,7 +147,321 @@ func yesOrNo(b bool) string { } } -func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { +func selectForeignServerGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + + // Join grantee. Filter by schema name, table name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) >= $1 AS ct " + + "FROM (SELECT 1 " + + "FROM information_schema.role_usage_grants " + + // Filter by column, table, schema, role and grantable setting + "WHERE grantee=$2 " + + "AND object_type = 'FOREIGN SERVER' " + + "AND object_name = ANY($3) " + + "AND is_grantable=$4 " + + "GROUP BY object_name " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($5::text[]) as perms ORDER BY perms ASC))" + + ") sub" + q.Parameters = []interface{}{ + len(gp.ForeignServers), + gp.Role, + pq.Array(gp.ForeignServers), + yesOrNo(gro), + pq.Array(sp), + } + + return nil +} + +func selectForeignDataWrapperGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + + // Join grantee. Filter by schema name, table name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) >= $1 AS ct " + + "FROM (SELECT 1 " + + "FROM information_schema.role_usage_grants " + + // Filter by column, table, schema, role and grantable setting + "WHERE grantee=$2 " + + "AND object_type = 'FOREIGN DATA WRAPPER' " + + "AND object_name = ANY($3) " + + "AND is_grantable=$4 " + + "GROUP BY object_name " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($5::text[]) as perms ORDER BY perms ASC))" + + ") sub" + q.Parameters = []interface{}{ + len(gp.ForeignDataWrappers), + gp.Role, + pq.Array(gp.ForeignDataWrappers), + yesOrNo(gro), + pq.Array(sp), + } + + return nil +} + +func selectColumnGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + + // Join grantee. Filter by schema name, table name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) >= $1 AS ct " + + "FROM (SELECT 1 " + + "FROM information_schema.role_column_grants " + + // Filter by column, table, schema, role and grantable setting + "WHERE table_schema=$2 " + + "AND table_name = ANY($3) " + + "AND column_name = ANY($4) " + + "AND grantee=$5 " + + "AND is_grantable=$6 " + + "GROUP BY table_name, column_name " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($7::text[]) as perms ORDER BY perms ASC))" + + ") sub" + q.Parameters = []interface{}{ + len(gp.Tables) * len(gp.Columns), + gp.Schema, + pq.Array(gp.Tables), + pq.Array(gp.Columns), + gp.Role, + yesOrNo(gro), + pq.Array(sp), + } + + return nil +} + +func selectRoutineGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + + routinesSignatures := make([]string, len(gp.Routines)) + for i, routine := range gp.Routines { + routinesSignatures[i] = signature(routine) + } + + // Join grantee. Filter by routine name and signature, schema name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) = $1 AS ct " + + "FROM (SELECT " + + // format routine args + "p.proname || '(' || coalesce(array_to_string(array_agg(pg_catalog.format_type(t, NULL) ORDER BY args.ord), ',')) || ')' " + + "AS signature " + + "FROM pg_proc p " + + "LEFT JOIN unnest(p.proargtypes) WITH ORDINALITY AS args(t, ord) on true " + + "INNER JOIN pg_namespace n ON p.pronamespace = n.oid, " + + "aclexplode(p.proacl) as acl " + + "INNER JOIN pg_roles s ON acl.grantee = s.oid " + + // Filter by sequence, schema, role and grantable setting + "WHERE n.nspname=$2 " + + "AND s.rolname=$3 " + + "AND acl.is_grantable=$4 " + + "GROUP BY n.nspname, s.rolname, acl.is_grantable, p.oid " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($5::text[]) as perms ORDER BY perms ASC))" + + ") sub " + + "WHERE sub.signature = ANY($6)" + q.Parameters = []interface{}{ + len(gp.Routines), + gp.Schema, + gp.Role, + gro, + pq.Array(sp), + pq.Array(routinesSignatures), + } + + return nil +} + +func selectSequenceGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + // Join grantee. Filter by sequence name, schema name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) = $1 AS ct " + + "FROM (SELECT 1 FROM pg_class c " + + "INNER JOIN pg_namespace n ON c.relnamespace = n.oid, " + + "aclexplode(c.relacl) as acl " + + "INNER JOIN pg_roles s ON acl.grantee = s.oid " + + "WHERE c.relkind = 'S' " + + // Filter by sequence, schema, role and grantable setting + "AND n.nspname=$2 " + + "AND s.rolname=$3 " + + "AND c.relname = ANY($4) " + + "AND acl.is_grantable=$5 " + + "GROUP BY n.nspname, s.rolname, acl.is_grantable " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($6::text[]) as perms ORDER BY perms ASC))" + + ") sub" + q.Parameters = []interface{}{ + len(gp.Sequences), + gp.Schema, + gp.Role, + pq.Array(gp.Sequences), + gro, + pq.Array(sp), + } + + return nil +} + +func selectTableGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + + // Join grantee. Filter by schema name, table name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT COUNT(*) = $1 AS ct " + + "FROM (SELECT 1 " + + "FROM information_schema.role_table_grants " + + // Filter by table, schema, role and grantable setting + "WHERE table_schema=$2 " + + "AND table_name = ANY($3) " + + "AND grantee=$4 " + + "AND is_grantable=$5 " + + "GROUP BY table_name " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($6::text[]) as perms ORDER BY perms ASC))" + + ") sub" + q.Parameters = []interface{}{ + len(gp.Tables), + gp.Schema, + pq.Array(gp.Tables), + gp.Role, + yesOrNo(gro), + pq.Array(sp), + } + + return nil +} + +func selectSchemaGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + // Join grantee. Filter by schema name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT EXISTS(SELECT 1 " + + "FROM pg_namespace n, " + + "aclexplode(nspacl) as acl " + + "INNER JOIN pg_roles s ON acl.grantee = s.oid " + + // Filter by schema, role and grantable setting + "WHERE n.nspname=$1 " + + "AND s.rolname=$2 " + + "AND acl.is_grantable=$3 " + + "GROUP BY n.nspname, s.rolname, acl.is_grantable " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($4::text[]) as perms ORDER BY perms ASC)))" + q.Parameters = []interface{}{ + gp.Schema, + gp.Role, + gro, + pq.Array(sp), + } + return nil +} + +func selectDatabaseGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { + gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant + + ep := gp.ExpandPrivileges() + sp := ep.ToStringSlice() + // Join grantee. Filter by database name and grantee name. + // Finally, perform a permission comparison against expected + // permissions. + q.String = "SELECT EXISTS(SELECT 1 " + + "FROM pg_database db, " + + "aclexplode(datacl) as acl " + + "INNER JOIN pg_roles s ON acl.grantee = s.oid " + + // Filter by database, role and grantable setting + "WHERE db.datname=$1 " + + "AND s.rolname=$2 " + + "AND acl.is_grantable=$3 " + + "GROUP BY db.datname, s.rolname, acl.is_grantable " + + // Check privileges match. Convoluted right-hand-side is necessary to + // ensure identical sort order of the input permissions. + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + + "= (SELECT array(SELECT unnest($4::text[]) as perms ORDER BY perms ASC)))" + + q.Parameters = []interface{}{ + gp.Database, + gp.Role, + gro, + pq.Array(sp), + } + return nil +} + +func selectMemberGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { + ao := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionAdmin + + // Always returns a row with a true or false value + // A simpler query would use ::regrol to cast the + // roleid and member oids to their role names, but + // if this is used with a nonexistent role name it will + // throw an error rather than return 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, + } + return nil +} + +func withOption(option *v1alpha1.GrantOption) string { + if option != nil { + return fmt.Sprintf("WITH %s OPTION", string(*option)) + } + return "" +} + +func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { // nolint: gocyclo gt, err := gp.IdentifyGrantType() if err != nil { return err @@ -155,305 +469,27 @@ func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { switch gt { case v1alpha1.RoleMember: - ao := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionAdmin - - // Always returns a row with a true or false value - // A simpler query would use ::regrol to cast the - // roleid and member oids to their role names, but - // if this is used with a nonexistent role name it will - // throw an error rather than return 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, - } - return nil + return selectMemberGrantQuery(gp, q) case v1alpha1.RoleDatabase: - gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant - - ep := gp.ExpandPrivileges() - sp := ep.ToStringSlice() - // Join grantee. Filter by database name and grantee name. - // Finally, perform a permission comparison against expected - // permissions. - q.String = "SELECT EXISTS(SELECT 1 " + - "FROM pg_database db, " + - "aclexplode(datacl) as acl " + - "INNER JOIN pg_roles s ON acl.grantee = s.oid " + - // Filter by database, role and grantable setting - "WHERE db.datname=$1 " + - "AND s.rolname=$2 " + - "AND acl.is_grantable=$3 " + - "GROUP BY db.datname, s.rolname, acl.is_grantable " + - // Check privileges match. Convoluted right-hand-side is necessary to - // ensure identical sort order of the input permissions. - "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + - "= (SELECT array(SELECT unnest($4::text[]) as perms ORDER BY perms ASC)))" - - q.Parameters = []interface{}{ - gp.Database, - gp.Role, - gro, - pq.Array(sp), - } - return nil + return selectDatabaseGrantQuery(gp, q) case v1alpha1.RoleSchema: - gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant - - ep := gp.ExpandPrivileges() - sp := ep.ToStringSlice() - // Join grantee. Filter by schema name and grantee name. - // Finally, perform a permission comparison against expected - // permissions. - q.String = "SELECT EXISTS(SELECT 1 " + - "FROM pg_namespace n, " + - "aclexplode(nspacl) as acl " + - "INNER JOIN pg_roles s ON acl.grantee = s.oid " + - // Filter by schema, role and grantable setting - "WHERE n.nspname=$1 " + - "AND s.rolname=$2 " + - "AND acl.is_grantable=$3 " + - "GROUP BY n.nspname, s.rolname, acl.is_grantable " + - // Check privileges match. Convoluted right-hand-side is necessary to - // ensure identical sort order of the input permissions. - "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + - "= (SELECT array(SELECT unnest($4::text[]) as perms ORDER BY perms ASC)))" - q.Parameters = []interface{}{ - gp.Schema, - gp.Role, - gro, - pq.Array(sp), - } - return nil + return selectSchemaGrantQuery(gp, q) case v1alpha1.RoleTable: - gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant - - ep := gp.ExpandPrivileges() - sp := ep.ToStringSlice() - - // Join grantee. Filter by schema name, table name and grantee name. - // Finally, perform a permission comparison against expected - // permissions. - q.String = "SELECT COUNT(*) = $1 AS ct " + - "FROM (SELECT 1 " + - "FROM information_schema.role_table_grants " + - // Filter by table, schema, role and grantable setting - "WHERE table_schema=$2 " + - "AND table_name = ANY($3) " + - "AND grantee=$4 " + - "AND is_grantable=$5 " + - "GROUP BY table_name " + - // Check privileges match. Convoluted right-hand-side is necessary to - // ensure identical sort order of the input permissions. - "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + - "= (SELECT array(SELECT unnest($6::text[]) as perms ORDER BY perms ASC))" + - ") sub" - q.Parameters = []interface{}{ - len(gp.Tables), - gp.Schema, - pq.Array(gp.Tables), - gp.Role, - yesOrNo(gro), - pq.Array(sp), - } - - return nil + return selectTableGrantQuery(gp, q) case v1alpha1.RoleSequence: - gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant - - ep := gp.ExpandPrivileges() - sp := ep.ToStringSlice() - // Join grantee. Filter by sequence name, schema name and grantee name. - // Finally, perform a permission comparison against expected - // permissions. - q.String = "SELECT COUNT(*) = $1 AS ct " + - "FROM (SELECT 1 FROM pg_class c " + - "INNER JOIN pg_namespace n ON c.relnamespace = n.oid, " + - "aclexplode(c.relacl) as acl " + - "INNER JOIN pg_roles s ON acl.grantee = s.oid " + - "WHERE c.relkind = 'S' " + - // Filter by sequence, schema, role and grantable setting - "AND n.nspname=$2 " + - "AND s.rolname=$3 " + - "AND c.relname = ANY($4) " + - "AND acl.is_grantable=$5 " + - "GROUP BY n.nspname, s.rolname, acl.is_grantable " + - // Check privileges match. Convoluted right-hand-side is necessary to - // ensure identical sort order of the input permissions. - "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + - "= (SELECT array(SELECT unnest($6::text[]) as perms ORDER BY perms ASC))" + - ") sub" - q.Parameters = []interface{}{ - len(gp.Sequences), - gp.Schema, - gp.Role, - pq.Array(gp.Sequences), - gro, - pq.Array(sp), - } - - return nil + return selectSequenceGrantQuery(gp, q) case v1alpha1.RoleRoutine: - gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant - - ep := gp.ExpandPrivileges() - sp := ep.ToStringSlice() - - routinesSignatures := make([]string, len(gp.Routines)) - for i, routine := range gp.Routines { - routinesSignatures[i] = signature(routine) - } - - // Join grantee. Filter by routine name and signature, schema name and grantee name. - // Finally, perform a permission comparison against expected - // permissions. - q.String = "SELECT COUNT(*) = $1 AS ct " + - "FROM (SELECT " + - // format routine args - "p.proname || '(' || coalesce(array_to_string(array_agg(pg_catalog.format_type(t, NULL) ORDER BY args.ord), ',')) || ')' " + - "AS signature " + - "FROM pg_proc p " + - "LEFT JOIN unnest(p.proargtypes) WITH ORDINALITY AS args(t, ord) on true " + - "INNER JOIN pg_namespace n ON p.pronamespace = n.oid, " + - "aclexplode(p.proacl) as acl " + - "INNER JOIN pg_roles s ON acl.grantee = s.oid " + - // Filter by sequence, schema, role and grantable setting - "WHERE n.nspname=$2 " + - "AND s.rolname=$3 " + - "AND acl.is_grantable=$4 " + - "GROUP BY n.nspname, s.rolname, acl.is_grantable, p.oid " + - // Check privileges match. Convoluted right-hand-side is necessary to - // ensure identical sort order of the input permissions. - "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + - "= (SELECT array(SELECT unnest($5::text[]) as perms ORDER BY perms ASC))" + - ") sub " + - "WHERE sub.signature = ANY($6)" - q.Parameters = []interface{}{ - len(gp.Routines), - gp.Schema, - gp.Role, - gro, - pq.Array(sp), - pq.Array(routinesSignatures), - } - - return nil + return selectRoutineGrantQuery(gp, q) case v1alpha1.RoleColumn: - gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant - - ep := gp.ExpandPrivileges() - sp := ep.ToStringSlice() - - // Join grantee. Filter by schema name, table name and grantee name. - // Finally, perform a permission comparison against expected - // permissions. - q.String = "SELECT COUNT(*) >= $1 AS ct " + - "FROM (SELECT 1 " + - "FROM information_schema.role_column_grants " + - // Filter by column, table, schema, role and grantable setting - "WHERE table_schema=$2 " + - "AND table_name = ANY($3) " + - "AND column_name = ANY($4) " + - "AND grantee=$5 " + - "AND is_grantable=$6 " + - "GROUP BY table_name, column_name " + - // Check privileges match. Convoluted right-hand-side is necessary to - // ensure identical sort order of the input permissions. - "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + - "= (SELECT array(SELECT unnest($7::text[]) as perms ORDER BY perms ASC))" + - ") sub" - q.Parameters = []interface{}{ - len(gp.Tables) * len(gp.Columns), - gp.Schema, - pq.Array(gp.Tables), - pq.Array(gp.Columns), - gp.Role, - yesOrNo(gro), - pq.Array(sp), - } - - return nil + return selectColumnGrantQuery(gp, q) case v1alpha1.RoleForeignDataWrapper: - gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant - - ep := gp.ExpandPrivileges() - sp := ep.ToStringSlice() - - // Join grantee. Filter by schema name, table name and grantee name. - // Finally, perform a permission comparison against expected - // permissions. - q.String = "SELECT COUNT(*) >= $1 AS ct " + - "FROM (SELECT 1 " + - "FROM information_schema.role_usage_grants " + - // Filter by column, table, schema, role and grantable setting - "WHERE grantee=$2 " + - "AND object_type = 'FOREIGN DATA WRAPPER' " + - "AND object_name = ANY($3) " + - "AND is_grantable=$4 " + - "GROUP BY object_name " + - // Check privileges match. Convoluted right-hand-side is necessary to - // ensure identical sort order of the input permissions. - "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + - "= (SELECT array(SELECT unnest($5::text[]) as perms ORDER BY perms ASC))" + - ") sub" - q.Parameters = []interface{}{ - len(gp.ForeignDataWrappers), - gp.Role, - pq.Array(gp.ForeignDataWrappers), - yesOrNo(gro), - pq.Array(sp), - } - - return nil + return selectForeignDataWrapperGrantQuery(gp, q) case v1alpha1.RoleForeignServer: - gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant - - ep := gp.ExpandPrivileges() - sp := ep.ToStringSlice() - - // Join grantee. Filter by schema name, table name and grantee name. - // Finally, perform a permission comparison against expected - // permissions. - q.String = "SELECT COUNT(*) >= $1 AS ct " + - "FROM (SELECT 1 " + - "FROM information_schema.role_usage_grants " + - // Filter by column, table, schema, role and grantable setting - "WHERE grantee=$2 " + - "AND object_type = 'FOREIGN SERVER' " + - "AND object_name = ANY($3) " + - "AND is_grantable=$4 " + - "GROUP BY object_name " + - // Check privileges match. Convoluted right-hand-side is necessary to - // ensure identical sort order of the input permissions. - "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + - "= (SELECT array(SELECT unnest($5::text[]) as perms ORDER BY perms ASC))" + - ") sub" - q.Parameters = []interface{}{ - len(gp.ForeignServers), - gp.Role, - pq.Array(gp.ForeignServers), - yesOrNo(gro), - pq.Array(sp), - } - - return nil + return selectForeignServerGrantQuery(gp, q) } return errors.Errorf(errUnsupportedGrant, gt) } -func withOption(option *v1alpha1.GrantOption) string { - if option != nil { - return fmt.Sprintf("WITH %s OPTION", string(*option)) - } - return "" -} - func createGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query) error { // nolint: gocyclo gt, err := gp.IdentifyGrantType() if err != nil { @@ -464,28 +500,28 @@ func createGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query) error { / switch gt { case v1alpha1.RoleMember: - return createRoleMember(gp, ql, ro) + return createMemberGrantQueries(gp, ql, ro) case v1alpha1.RoleDatabase: - return createRoleDatabase(gp, ql, ro) + return createDatabaseGrantQueries(gp, ql, ro) case v1alpha1.RoleSchema: - return createRoleSchema(gp, ql, ro) + return createSchemaGrantQueries(gp, ql, ro) case v1alpha1.RoleTable: - return createRoleTable(gp, ql, ro) + return createTableGrantQueries(gp, ql, ro) case v1alpha1.RoleSequence: - return createRoleSequence(gp, ql, ro) + return createSequenceGrantQueries(gp, ql, ro) case v1alpha1.RoleRoutine: - return createRoleRoutine(gp, ql, ro) + return createRoutineGrantQueries(gp, ql, ro) case v1alpha1.RoleColumn: - return createRoleColumn(gp, ql, ro) + return createColumnGrantQueries(gp, ql, ro) case v1alpha1.RoleForeignDataWrapper: - return createRoleForeignDataWrapper(gp, ql, ro) + return createForeignDataWrapperGrantQueries(gp, ql, ro) case v1alpha1.RoleForeignServer: - return createRoleForeignServer(gp, ql, ro) + return createForeignServerGrantQueries(gp, ql, ro) } return errors.Errorf(errUnsupportedGrant, gt) } -func createRoleMember(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { +func createMemberGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.MemberOf == nil || gp.Role == nil { return errors.Errorf(errInvalidParams, v1alpha1.RoleMember) } @@ -501,7 +537,7 @@ func createRoleMember(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) return nil } -func createRoleDatabase(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { +func createDatabaseGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleDatabase) } @@ -536,7 +572,7 @@ func createRoleDatabase(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string return nil } -func createRoleSchema(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { +func createSchemaGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || gp.Schema == nil || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleSchema) } @@ -563,12 +599,12 @@ func createRoleSchema(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) return nil } -func createRoleTable(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { +func createTableGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || gp.Schema == nil || len(gp.Tables) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleTable) } - tb := strings.Join(prefixWithSchema(*gp.Schema, gp.Tables), ",") + tb := strings.Join(prefixAndQuote(*gp.Schema, gp.Tables), ",") sp := strings.Join(gp.Privileges.ToStringSlice(), ",") *ql = append(*ql, @@ -590,13 +626,13 @@ func createRoleTable(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) e return nil } -func createRoleColumn(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { +func createColumnGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || gp.Schema == nil || len(gp.Tables) < 1 || len(gp.Columns) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleColumn) } co := strings.Join(gp.Columns, ",") - tb := strings.Join(prefixWithSchema(*gp.Schema, gp.Tables), ",") + tb := strings.Join(prefixAndQuote(*gp.Schema, gp.Tables), ",") sp := strings.Join(gp.Privileges.ToStringSlice(), ",") *ql = append(*ql, @@ -620,12 +656,12 @@ func createRoleColumn(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) return nil } -func createRoleSequence(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { +func createSequenceGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || gp.Schema == nil || len(gp.Sequences) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleSequence) } - sq := strings.Join(prefixWithSchema(*gp.Schema, gp.Sequences), ",") + sq := strings.Join(prefixAndQuote(*gp.Schema, gp.Sequences), ",") sp := strings.Join(gp.Privileges.ToStringSlice(), ",") *ql = append(*ql, @@ -647,12 +683,12 @@ func createRoleSequence(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string return nil } -func createRoleRoutine(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { +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) } - rt := strings.Join(quotedSignaturesWithSchema(*gp.Schema, gp.Routines), ",") + rt := strings.Join(quotedSignatures(*gp.Schema, gp.Routines), ",") sp := strings.Join(gp.Privileges.ToStringSlice(), ",") *ql = append(*ql, @@ -674,7 +710,7 @@ func createRoleRoutine(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) return nil } -func createRoleForeignDataWrapper(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { +func createForeignDataWrapperGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || len(gp.ForeignDataWrappers) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleForeignDataWrapper) } @@ -700,7 +736,7 @@ func createRoleForeignDataWrapper(gp v1alpha1.GrantParameters, ql *[]xsql.Query, return nil } -func createRoleForeignServer(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { +func createForeignServerGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro string) error { if gp.Database == nil || len(gp.ForeignServers) < 1 || gp.Role == nil || len(gp.Privileges) < 1 { return errors.Errorf(errInvalidParams, v1alpha1.RoleForeignServer) } @@ -726,7 +762,7 @@ func createRoleForeignServer(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro s return nil } -func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { +func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { // nolint: gocyclo gt, err := gp.IdentifyGrantType() if err != nil { return err @@ -758,21 +794,21 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { case v1alpha1.RoleTable: q.String = fmt.Sprintf("REVOKE %s ON TABLE %s FROM %s", strings.Join(gp.Privileges.ToStringSlice(), ","), - strings.Join(prefixWithSchema(*gp.Schema, gp.Tables), ","), + strings.Join(prefixAndQuote(*gp.Schema, gp.Tables), ","), ro, ) return nil case v1alpha1.RoleSequence: q.String = fmt.Sprintf("REVOKE %s ON SEQUENCE %s FROM %s", strings.Join(gp.Privileges.ToStringSlice(), ","), - strings.Join(prefixWithSchema(*gp.Schema, gp.Sequences), ","), + strings.Join(prefixAndQuote(*gp.Schema, gp.Sequences), ","), ro, ) return nil case v1alpha1.RoleRoutine: q.String = fmt.Sprintf("REVOKE %s ON ROUTINE %s FROM %s", strings.Join(gp.Privileges.ToStringSlice(), ","), - strings.Join(quotedSignaturesWithSchema(*gp.Schema, gp.Routines), ","), + strings.Join(quotedSignatures(*gp.Schema, gp.Routines), ","), ro, ) return nil @@ -780,7 +816,7 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { q.String = fmt.Sprintf("REVOKE %s (%s) ON TABLE %s FROM %s", strings.Join(gp.Privileges.ToStringSlice(), ","), strings.Join(gp.Columns, ","), - strings.Join(prefixWithSchema(*gp.Schema, gp.Tables), ","), + strings.Join(prefixAndQuote(*gp.Schema, gp.Tables), ","), ro, ) return nil @@ -805,35 +841,31 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { // signature returns routines with the same format as the select query func signature(r v1alpha1.Routine) string { args := make([]string, len(r.Arguments)) - for j, arg := range r.Arguments { - args[j] = strings.ToLower(arg) + for i, v := range r.Arguments { + args[i] = strings.ToLower(v) } return r.Name + "(" + strings.Join(args, ",") + ")" } -// signature returns routines with grantable format -func quotedSignaturesWithSchema(schema string, routines []v1alpha1.Routine) []string { - sh := pq.QuoteIdentifier(schema) - routinesWithSchema := make([]string, len(routines)) - - for i, routine := range routines { - args := make([]string, len(routine.Arguments)) - for j, arg := range routine.Arguments { - args[j] = strings.ToUpper(arg) - } +// quotedSignatures returns routines in a quoted grantable format, prefixed with the schema +func quotedSignatures(sc string, rs []v1alpha1.Routine) []string { + qsc := pq.QuoteIdentifier(sc) + sigs := make([]string, len(rs)) - routinesWithSchema[i] = sh + "." + pq.QuoteIdentifier(routine.Name) + "(" + strings.Join(args, ",") + ")" + for i, r := range rs { + sigs[i] = qsc + "." + pq.QuoteIdentifier(r.Name) + "(" + strings.Join(r.Arguments, ",") + ")" } - return routinesWithSchema + return sigs } -func prefixWithSchema(schema string, objects []string) []string { - sh := pq.QuoteIdentifier(schema) - objectsWithSchema := make([]string, len(objects)) - for i, object := range objects { - objectsWithSchema[i] = sh + "." + pq.QuoteIdentifier(object) +// prefixAndQuote returns objects in a quoted grantable format, prefixed with the schema +func prefixAndQuote(sc string, obj []string) []string { + qsc := pq.QuoteIdentifier(sc) + ret := make([]string, len(obj)) + for i, v := range obj { + ret[i] = qsc + "." + pq.QuoteIdentifier(v) } - return objectsWithSchema + return ret } func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) { diff --git a/pkg/controller/postgresql/grant/reconciler_test.go b/pkg/controller/postgresql/grant/reconciler_test.go index 0f3c95ab..e2634488 100644 --- a/pkg/controller/postgresql/grant/reconciler_test.go +++ b/pkg/controller/postgresql/grant/reconciler_test.go @@ -432,6 +432,203 @@ func TestObserve(t *testing.T) { err: nil, }, }, + "SuccessRoleTable": { + reason: "We should return no error if we can find our role-table grant", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("testdb"), + Role: ptr.To("testrole"), + Schema: ptr.To("testschema"), + Tables: []string{"testtable"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + WithOption: &gog, + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, + "SuccessRoleColumn": { + reason: "We should return no error if we can find our role-column grant", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("testdb"), + Role: ptr.To("testrole"), + Schema: ptr.To("testschema"), + Tables: []string{"testtable"}, + Columns: []string{"testcolumn"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + WithOption: &gog, + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, + "SuccessRoleSequence": { + reason: "We should return no error if we can find our role-sequence grant", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("testdb"), + Role: ptr.To("testrole"), + Schema: ptr.To("testschema"), + Sequences: []string{"testsequence"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + WithOption: &gog, + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, + "SuccessRoleRoutine": { + reason: "We should return no error if we can find our role-routine grant", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("testdb"), + Role: ptr.To("testrole"), + Schema: ptr.To("testschema"), + Routines: []v1alpha1.Routine{{Name: "testroutine", Arguments: []string{"text"}}}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + WithOption: &gog, + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, + "SuccessRoleForeingDataWrapper": { + reason: "We should return no error if we can find our role-foreign-data-wrapper grant", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("testdb"), + Role: ptr.To("testrole"), + ForeignDataWrappers: []string{"testforeigndatawrapper"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + WithOption: &gog, + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, + "SuccessRoleForeignServer": { + reason: "We should return no error if we can find our role-foreign-server grant", + fields: fields{ + db: mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + bv := dest[0].(*bool) + *bv = true + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("testdb"), + Role: ptr.To("testrole"), + ForeignServers: []string{"testforeignserver"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + WithOption: &gog, + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, } for name, tc := range cases { @@ -450,6 +647,7 @@ func TestObserve(t *testing.T) { func TestCreate(t *testing.T) { errBoom := errors.New("boom") + goa := v1alpha1.GrantOptionAdmin type fields struct { db xsql.DB @@ -520,6 +718,28 @@ func TestCreate(t *testing.T) { err: errors.Wrap(errBoom, errCreateGrant), }, }, + "RoleMembershipSuccess": { + reason: "No error should be returned when we successfully create a role-membership grant", + fields: fields{ + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Role: ptr.To("testrole"), + MemberOf: ptr.To("parentrole"), + WithOption: &goa, + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, "RoleDatabaseSuccess": { reason: "No error should be returned when we successfully create a role-database grant", fields: fields{ @@ -565,6 +785,149 @@ func TestCreate(t *testing.T) { err: nil, }, }, + "RoleTableSuccess": { + reason: "No error should be returned when we successfully create a role-table grant", + fields: fields{ + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + Schema: ptr.To("test-example"), + Tables: []string{"test-example"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, + "RoleColumnSuccess": { + reason: "No error should be returned when we successfully create a role-column grant", + fields: fields{ + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + Schema: ptr.To("test-example"), + Tables: []string{"test-example"}, + Columns: []string{"test-example"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, + "RoleSequenceSuccess": { + reason: "No error should be returned when we successfully create a role-sequence grant", + fields: fields{ + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + Schema: ptr.To("test-example"), + Sequences: []string{"test-example"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, + "RoleRoutineSuccess": { + reason: "No error should be returned when we successfully create a role-routine grant", + fields: fields{ + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + Schema: ptr.To("test-example"), + Routines: []v1alpha1.Routine{{Name: "test-example", Arguments: []string{"test-example"}}}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, + "RoleForeignDataWrapperSuccess": { + reason: "No error should be returned when we successfully create a role-foreign-data-wrapper grant", + fields: fields{ + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + ForeignDataWrappers: []string{"test-example"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, + "RoleForeignServerSuccess": { + reason: "No error should be returned when we successfully create a role-foreign-server grant", + fields: fields{ + db: &mockDB{ + MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + ForeignServers: []string{"test-example"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, } for name, tc := range cases { @@ -741,6 +1104,137 @@ func TestDelete(t *testing.T) { }, want: nil, }, + "RoleTableSuccess": { + reason: "No error should be returned if the role-table grant was revoked", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + Schema: ptr.To("test-example"), + Tables: []string{"test-example"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: nil, + }, + "RoleColumnSuccess": { + reason: "No error should be returned if the role-column grant was revoked", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + Schema: ptr.To("test-example"), + Tables: []string{"test-example"}, + Columns: []string{"test-example"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: nil, + }, + "RoleSequenceSuccess": { + reason: "No error should be returned if the role-sequence grant was revoked", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + Schema: ptr.To("test-example"), + Sequences: []string{"test-example"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: nil, + }, + "RoleRoutineSuccess": { + reason: "No error should be returned if the role-routine grant was revoked", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + Schema: ptr.To("test-example"), + Routines: []v1alpha1.Routine{{Name: "test-example", Arguments: []string{"test-example"}}}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: nil, + }, + "RoleForeignDataWrapperSuccess": { + reason: "No error should be returned if the role-foreign-data-wrapper grant was revoked", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + ForeignDataWrappers: []string{"test-example"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: nil, + }, + "RoleForeignServerSuccess": { + reason: "No error should be returned if the role-foreign-server grant was revoked", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + Role: ptr.To("test-example"), + ForeignServers: []string{"test-example"}, + Privileges: v1alpha1.GrantPrivileges{"ALL"}, + }, + }, + }, + }, + want: nil, + }, } for name, tc := range cases { From a18171dca4c58ea49c10232e73c86cc3378c149c Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Tue, 23 Sep 2025 11:51:34 +0200 Subject: [PATCH 09/12] fix: wrong sequence grants count --- pkg/controller/postgresql/grant/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index 2a2c1ad1..3ef18968 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -318,7 +318,7 @@ func selectSequenceGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error "AND s.rolname=$3 " + "AND c.relname = ANY($4) " + "AND acl.is_grantable=$5 " + - "GROUP BY n.nspname, s.rolname, acl.is_grantable " + + "GROUP BY c.relname, n.nspname, s.rolname, acl.is_grantable " + // Check privileges match. Convoluted right-hand-side is necessary to // ensure identical sort order of the input permissions. "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + From fc5a8aa925c7ccfdfde128d07eee92abeab4d3fc Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Tue, 23 Sep 2025 14:21:29 +0200 Subject: [PATCH 10/12] fix: table grants --- pkg/controller/postgresql/grant/reconciler.go | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index 3ef18968..80957c1f 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -346,25 +346,28 @@ func selectTableGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { // Finally, perform a permission comparison against expected // permissions. q.String = "SELECT COUNT(*) = $1 AS ct " + - "FROM (SELECT 1 " + - "FROM information_schema.role_table_grants " + + "FROM (SELECT 1 FROM pg_class c " + + "INNER JOIN pg_namespace n ON c.relnamespace = n.oid, " + + "aclexplode(c.relacl) as acl " + + "INNER JOIN pg_roles s ON acl.grantee = s.oid " + + "WHERE c.relkind = 'r' " + // Filter by table, schema, role and grantable setting - "WHERE table_schema=$2 " + - "AND table_name = ANY($3) " + - "AND grantee=$4 " + - "AND is_grantable=$5 " + - "GROUP BY table_name " + + "AND n.nspname=$2 " + + "AND s.rolname=$3 " + + "AND c.relname = ANY($4) " + + "AND acl.is_grantable=$5 " + + "GROUP BY c.relname, n.nspname, s.rolname, acl.is_grantable " + // Check privileges match. Convoluted right-hand-side is necessary to // ensure identical sort order of the input permissions. - "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + "= (SELECT array(SELECT unnest($6::text[]) as perms ORDER BY perms ASC))" + ") sub" q.Parameters = []interface{}{ len(gp.Tables), gp.Schema, - pq.Array(gp.Tables), gp.Role, - yesOrNo(gro), + pq.Array(gp.Tables), + gro, pq.Array(sp), } From 08a31a30cf1d6151fa05f3b4c4b80503ed3b3432 Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Tue, 23 Sep 2025 15:25:23 +0200 Subject: [PATCH 11/12] fix: grant columns privileges --- pkg/controller/postgresql/grant/reconciler.go | 58 +++++++++++-------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go index 80957c1f..3b0519e9 100644 --- a/pkg/controller/postgresql/grant/reconciler.go +++ b/pkg/controller/postgresql/grant/reconciler.go @@ -224,28 +224,32 @@ func selectColumnGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { // Join grantee. Filter by schema name, table name and grantee name. // Finally, perform a permission comparison against expected // permissions. - q.String = "SELECT COUNT(*) >= $1 AS ct " + - "FROM (SELECT 1 " + - "FROM information_schema.role_column_grants " + - // Filter by column, table, schema, role and grantable setting - "WHERE table_schema=$2 " + - "AND table_name = ANY($3) " + - "AND column_name = ANY($4) " + - "AND grantee=$5 " + - "AND is_grantable=$6 " + - "GROUP BY table_name, column_name " + + q.String = "SELECT COUNT(*) = $1 AS ct " + + "FROM (SELECT 1 FROM pg_class c " + + "INNER JOIN pg_namespace n ON c.relnamespace = n.oid " + + "INNER JOIN pg_attribute attr on c.oid = attr.attrelid, " + + "aclexplode(attr.attacl) as acl " + + "INNER JOIN pg_roles s ON acl.grantee = s.oid " + + "WHERE c.relkind = 'r' " + + // Filter by table, schema, role and grantable setting + "AND n.nspname=$2 " + + "AND s.rolname=$3 " + + "AND c.relname = ANY($4) " + + "AND attr.attname = ANY($5) " + + "AND acl.is_grantable=$6 " + + "GROUP BY c.relname, n.nspname, s.rolname, attr.attname, acl.is_grantable " + // Check privileges match. Convoluted right-hand-side is necessary to // ensure identical sort order of the input permissions. - "HAVING array_agg(TEXT(privilege_type) ORDER BY privilege_type ASC) " + + "HAVING array_agg(acl.privilege_type ORDER BY privilege_type ASC) " + "= (SELECT array(SELECT unnest($7::text[]) as perms ORDER BY perms ASC))" + ") sub" q.Parameters = []interface{}{ len(gp.Tables) * len(gp.Columns), gp.Schema, + gp.Role, pq.Array(gp.Tables), pq.Array(gp.Columns), - gp.Role, - yesOrNo(gro), + gro, pq.Array(sp), } @@ -635,22 +639,20 @@ func createColumnGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query, ro } co := strings.Join(gp.Columns, ",") + cp := columnsPrivileges(gp.Privileges.ToStringSlice(), co) tb := strings.Join(prefixAndQuote(*gp.Schema, gp.Tables), ",") - sp := strings.Join(gp.Privileges.ToStringSlice(), ",") *ql = append(*ql, // REVOKE ANY MATCHING EXISTING PERMISSIONS - xsql.Query{String: fmt.Sprintf("REVOKE %s (%s) ON TABLE %s FROM %s", - sp, - co, + xsql.Query{String: fmt.Sprintf("REVOKE %s ON TABLE %s FROM %s", + cp, tb, ro, )}, // GRANT REQUESTED PERMISSIONS - xsql.Query{String: fmt.Sprintf("GRANT %s (%s) ON TABLE %s TO %s %s", - sp, - co, + xsql.Query{String: fmt.Sprintf("GRANT %s ON TABLE %s TO %s %s", + cp, tb, ro, withOption(gp.WithOption), @@ -816,9 +818,10 @@ func deleteGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error { // nol ) return nil case v1alpha1.RoleColumn: - q.String = fmt.Sprintf("REVOKE %s (%s) ON TABLE %s FROM %s", - strings.Join(gp.Privileges.ToStringSlice(), ","), - strings.Join(gp.Columns, ","), + co := strings.Join(gp.Columns, ",") + cp := columnsPrivileges(gp.Privileges.ToStringSlice(), co) + q.String = fmt.Sprintf("REVOKE %s ON TABLE %s FROM %s", + cp, strings.Join(prefixAndQuote(*gp.Schema, gp.Tables), ","), ro, ) @@ -871,6 +874,15 @@ func prefixAndQuote(sc string, obj []string) []string { return ret } +// columnsPrivileges returns the privileges for columns in grant format +func columnsPrivileges(priv []string, cols string) string { + ret := make([]string, len(priv)) + for i, v := range priv { + ret[i] = v + "(" + cols + ")" + } + return strings.Join(ret, ",") +} + func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) { cr, ok := mg.(*v1alpha1.Grant) if !ok { From a2ecd7ef1752c097a586aee66465d669cf3f8b3d Mon Sep 17 00:00:00 2001 From: Julien Christophe Date: Tue, 23 Sep 2025 16:18:07 +0200 Subject: [PATCH 12/12] feat: adds a different case in tests --- cluster/local/postgresdb_functions.sh | 11 ++++++++--- examples/postgresql/grant.yaml | 3 ++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cluster/local/postgresdb_functions.sh b/cluster/local/postgresdb_functions.sh index b909f8b4..0217c2c9 100644 --- a/cluster/local/postgresdb_functions.sh +++ b/cluster/local/postgresdb_functions.sh @@ -45,7 +45,8 @@ create_grantable_objects() { TARGE_SCHEMA='public' request=" CREATE TABLE \"$TARGE_SCHEMA\".test_table(col1 INT NULL); - CREATE SEQUENCE \"$TARGE_SCHEMA\".test_sequence START WITH 1000 INCREMENT BY 1; + CREATE SEQUENCE \"$TARGE_SCHEMA\".test_sequence_1 START WITH 1000 INCREMENT BY 1; + CREATE SEQUENCE \"$TARGE_SCHEMA\".test_sequence_2 START WITH 1000 INCREMENT BY 1; CREATE PROCEDURE \"$TARGE_SCHEMA\".test_procedure(arg TEXT) LANGUAGE plpgsql AS \$\$ BEGIN END; \$\$; CREATE TABLE \"$TARGE_SCHEMA\".test_table_column(test_column INT NULL); CREATE FOREIGN DATA WRAPPER test_foreign_data_wrapper; @@ -67,7 +68,8 @@ delete_grantable_objects() { DROP FOREIGN DATA WRAPPER test_foreign_data_wrapper; DROP TABLE \"$TARGE_SCHEMA\".test_table_column; DROP PROCEDURE \"$TARGE_SCHEMA\".test_procedure(TEXT); - DROP SEQUENCE \"$TARGE_SCHEMA\".test_sequence; + DROP SEQUENCE \"$TARGE_SCHEMA\".test_sequence_1; + DROP SEQUENCE \"$TARGE_SCHEMA\".test_sequence_2; DROP TABLE \"$TARGE_SCHEMA\".test_table; " drop_objects=$(PGPASSWORD="${postgres_root_pw}" psql -h localhost -p 5432 -U postgres -d "$TARGET_DB" -wtAc "$request") @@ -230,12 +232,15 @@ check_table_privileges(){ check_sequence_privileges(){ target_db="db1" schema="public" - sequence="test_sequence" role='no-grants-role' expected_privileges='SELECT|f,UPDATE|f,USAGE|f' + sequence="test_sequence_1" request="select acl.privilege_type, acl.is_grantable from pg_class c inner join pg_namespace n on c.relnamespace = n.oid, aclexplode(c.relacl) as acl inner join pg_roles s on acl.grantee = s.oid where c.relkind = 'S' and n.nspname = '$schema' and s.rolname='$role' and c.relname = '$sequence'" + check_privileges $target_db "sequence $schema.$sequence" $role $expected_privileges "$request" + sequence="test_sequence_2" + request="select acl.privilege_type, acl.is_grantable from pg_class c inner join pg_namespace n on c.relnamespace = n.oid, aclexplode(c.relacl) as acl inner join pg_roles s on acl.grantee = s.oid where c.relkind = 'S' and n.nspname = '$schema' and s.rolname='$role' and c.relname = '$sequence'" check_privileges $target_db "sequence $schema.$sequence" $role $expected_privileges "$request" } diff --git a/examples/postgresql/grant.yaml b/examples/postgresql/grant.yaml index ff5b1734..cbe7e82a 100644 --- a/examples/postgresql/grant.yaml +++ b/examples/postgresql/grant.yaml @@ -114,7 +114,8 @@ spec: schemaRef: name: public sequences: - - test_sequence + - test_sequence_1 + - test_sequence_2 --- apiVersion: postgresql.sql.crossplane.io/v1alpha1 kind: Grant