From d6505de3c7b88595565f40a8645c0275949e0533 Mon Sep 17 00:00:00 2001 From: Alex Jurkiewicz Date: Fri, 30 Oct 2026 13:32:07 +0800 Subject: [PATCH] feat(mysql): support role membership grants Add memberOf to cluster-scoped and namespaced MySQL Grants, following the PostgreSQL membership model. Observe direct memberships through MariaDB or MySQL catalogs and grant or revoke only the requested relationship. Leave existing privilege grants unchanged. Include API validation, generated manifests, examples, unit tests, opt-in database integration tests, and MariaDB controller E2E coverage. Signed-off-by: Alex Jurkiewicz --- apis/cluster/mysql/v1alpha1/grant_types.go | 17 +- .../mysql/v1alpha1/zz_generated.deepcopy.go | 5 + apis/namespaced/mysql/v1alpha1/grant_types.go | 17 +- .../mysql/v1alpha1/zz_generated.deepcopy.go | 5 + cluster/local/integration_tests.sh | 59 ++++++ examples/cluster/mysql/grant_member_of.yaml | 19 ++ .../namespaced/mysql/grant_member_of.yaml | 23 +++ .../crds/mysql.sql.crossplane.io_grants.yaml | 24 ++- .../mysql.sql.m.crossplane.io_grants.yaml | 24 ++- pkg/clients/mysql/role_grants.go | 106 ++++++++++ .../mysql/role_grants_integration_test.go | 127 ++++++++++++ pkg/clients/mysql/role_grants_test.go | 191 ++++++++++++++++++ .../cluster/mysql/grant/reconciler.go | 26 +++ .../cluster/mysql/grant/reconciler_test.go | 137 +++++++++++++ .../namespaced/mysql/grant/reconciler.go | 26 +++ .../namespaced/mysql/grant/reconciler_test.go | 137 +++++++++++++ 16 files changed, 937 insertions(+), 6 deletions(-) create mode 100644 examples/cluster/mysql/grant_member_of.yaml create mode 100644 examples/namespaced/mysql/grant_member_of.yaml create mode 100644 pkg/clients/mysql/role_grants.go create mode 100644 pkg/clients/mysql/role_grants_integration_test.go create mode 100644 pkg/clients/mysql/role_grants_test.go diff --git a/apis/cluster/mysql/v1alpha1/grant_types.go b/apis/cluster/mysql/v1alpha1/grant_types.go index 2c0c4a34..7e9ab96a 100644 --- a/apis/cluster/mysql/v1alpha1/grant_types.go +++ b/apis/cluster/mysql/v1alpha1/grant_types.go @@ -51,10 +51,24 @@ func (gp *GrantPrivileges) ToStringSlice() []string { } // GrantParameters define the desired state of a MySQL grant instance. +// +kubebuilder:validation:XValidation:rule="has(self.memberOf) != has(self.privileges)",message="specify either memberOf or privileges" +// +kubebuilder:validation:XValidation:rule="!has(self.memberOf) || !(has(self.database) || has(self.databaseRef) || has(self.databaseSelector) || has(self.table))",message="memberOf cannot be combined with database or table targeting" +// +kubebuilder:validation:XValidation:rule="!has(self.memberOf) || has(self.user) || has(self.userRef) || has(self.userSelector)",message="memberOf requires a user, userRef, or userSelector" type GrantParameters struct { // Privileges to be granted. // See https://mariadb.com/kb/en/grant/#database-privileges for available privileges. - Privileges GrantPrivileges `json:"privileges"` + // Cannot be combined with MemberOf. + // +optional + Privileges GrantPrivileges `json:"privileges,omitempty"` + + // MemberOf is an existing role to grant to the user. + // Use name@host to specify a MySQL role host; the default host is %. + // Cannot be combined with privileges, database, databaseRef, databaseSelector, or table. + // This Grant does not configure default roles or session activation. + // Deleting the Grant revokes the membership without deleting the user or role. + // +kubebuilder:validation:MinLength=1 + // +optional + MemberOf *string `json:"memberOf,omitempty"` // User this grant is for. // +optional @@ -116,6 +130,7 @@ type GrantObservation struct { // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp" // +kubebuilder:printcolumn:name="ROLE",type="string",JSONPath=".spec.forProvider.user" // +kubebuilder:printcolumn:name="DATABASE",type="string",JSONPath=".spec.forProvider.database" +// +kubebuilder:printcolumn:name="MEMBER OF",type="string",JSONPath=".spec.forProvider.memberOf" // +kubebuilder:printcolumn:name="PRIVILEGES",type="string",JSONPath=".spec.forProvider.privileges" // +kubebuilder:resource:scope=Cluster,categories={crossplane,managed,sql} type Grant struct { diff --git a/apis/cluster/mysql/v1alpha1/zz_generated.deepcopy.go b/apis/cluster/mysql/v1alpha1/zz_generated.deepcopy.go index d7b2096f..33b25c8e 100644 --- a/apis/cluster/mysql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/cluster/mysql/v1alpha1/zz_generated.deepcopy.go @@ -242,6 +242,11 @@ func (in *GrantParameters) DeepCopyInto(out *GrantParameters) { *out = make(GrantPrivileges, len(*in)) copy(*out, *in) } + if in.MemberOf != nil { + in, out := &in.MemberOf, &out.MemberOf + *out = new(string) + **out = **in + } if in.User != nil { in, out := &in.User, &out.User *out = new(string) diff --git a/apis/namespaced/mysql/v1alpha1/grant_types.go b/apis/namespaced/mysql/v1alpha1/grant_types.go index f513ca65..a1940cd8 100644 --- a/apis/namespaced/mysql/v1alpha1/grant_types.go +++ b/apis/namespaced/mysql/v1alpha1/grant_types.go @@ -52,10 +52,24 @@ func (gp *GrantPrivileges) ToStringSlice() []string { } // GrantParameters define the desired state of a MySQL grant instance. +// +kubebuilder:validation:XValidation:rule="has(self.memberOf) != has(self.privileges)",message="specify either memberOf or privileges" +// +kubebuilder:validation:XValidation:rule="!has(self.memberOf) || !(has(self.database) || has(self.databaseRef) || has(self.databaseSelector) || has(self.table))",message="memberOf cannot be combined with database or table targeting" +// +kubebuilder:validation:XValidation:rule="!has(self.memberOf) || has(self.user) || has(self.userRef) || has(self.userSelector)",message="memberOf requires a user, userRef, or userSelector" type GrantParameters struct { // Privileges to be granted. // See https://mariadb.com/kb/en/grant/#database-privileges for available privileges. - Privileges GrantPrivileges `json:"privileges"` + // Cannot be combined with MemberOf. + // +optional + Privileges GrantPrivileges `json:"privileges,omitempty"` + + // MemberOf is an existing role to grant to the user. + // Use name@host to specify a MySQL role host; the default host is %. + // Cannot be combined with privileges, database, databaseRef, databaseSelector, or table. + // This Grant does not configure default roles or session activation. + // Deleting the Grant revokes the membership without deleting the user or role. + // +kubebuilder:validation:MinLength=1 + // +optional + MemberOf *string `json:"memberOf,omitempty"` // User this grant is for. // +optional @@ -117,6 +131,7 @@ type GrantObservation struct { // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp" // +kubebuilder:printcolumn:name="ROLE",type="string",JSONPath=".spec.forProvider.user" // +kubebuilder:printcolumn:name="DATABASE",type="string",JSONPath=".spec.forProvider.database" +// +kubebuilder:printcolumn:name="MEMBER OF",type="string",JSONPath=".spec.forProvider.memberOf" // +kubebuilder:printcolumn:name="PRIVILEGES",type="string",JSONPath=".spec.forProvider.privileges" // +kubebuilder:resource:scope=Namespaced,categories={crossplane,managed,sql} type Grant struct { diff --git a/apis/namespaced/mysql/v1alpha1/zz_generated.deepcopy.go b/apis/namespaced/mysql/v1alpha1/zz_generated.deepcopy.go index 1eb25e8c..ac56b1b0 100644 --- a/apis/namespaced/mysql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/namespaced/mysql/v1alpha1/zz_generated.deepcopy.go @@ -364,6 +364,11 @@ func (in *GrantParameters) DeepCopyInto(out *GrantParameters) { *out = make(GrantPrivileges, len(*in)) copy(*out, *in) } + if in.MemberOf != nil { + in, out := &in.MemberOf, &out.MemberOf + *out = new(string) + **out = **in + } if in.User != nil { in, out := &in.User, &out.User *out = new(string) diff --git a/cluster/local/integration_tests.sh b/cluster/local/integration_tests.sh index 903eb896..614133bd 100755 --- a/cluster/local/integration_tests.sh +++ b/cluster/local/integration_tests.sh @@ -385,6 +385,64 @@ test_create_grant() { echo_step_completed } +mariadb_exec() { + "${KUBECTL}" exec mariadb-0 -- bash -c \ + 'mariadb -uroot -p"${MARIADB_ROOT_PASSWORD}" -N -e "$1"' -- "$1" +} + +test_role_membership() { + echo_step "test MySQL Grant role membership" + mariadb_exec "CREATE ROLE reporting_reader; CREATE ROLE unrelated_reader; + GRANT unrelated_reader TO 'example-user'@'%' WITH ADMIN OPTION;" + local provider_user="root" + if [[ "${TLS}" == "true" ]]; then + provider_user="test" + fi + mariadb_exec "GRANT reporting_reader TO '${provider_user}'@'%' WITH ADMIN OPTION;" + + local grants_before + grants_before=$(mariadb_exec "SHOW GRANTS FOR 'example-user'@'%';" | sort) + local membership_query="SELECT COUNT(*) FROM mysql.roles_mapping + WHERE \`Role\` = 'reporting_reader' AND \`User\` = 'example-user' AND \`Host\` = '%';" + + "${KUBECTL}" apply -f ${projectdir}/examples/${API_TYPE}/mysql/grant_member_of.yaml + "${KUBECTL}" wait --timeout 2m --for condition=Ready -f ${projectdir}/examples/${API_TYPE}/mysql/grant_member_of.yaml + [[ $(mariadb_exec "${membership_query}") == "1" ]] + + echo_info "check that membership and privileges cannot be combined" + local admission_error + if admission_error=$("${KUBECTL}" patch --dry-run=server --type merge \ + -f ${projectdir}/examples/${API_TYPE}/mysql/grant_member_of.yaml \ + -p '{"spec":{"forProvider":{"privileges":["SELECT"]}}}' 2>&1); then + echo_error "expected memberOf and privileges to be rejected" + fi + if [[ "${admission_error}" != *"specify either memberOf or privileges"* ]]; then + echo_error "unexpected admission error: ${admission_error}" + fi + echo_step_completed + + echo_info "check if reconciliation restores a revoked membership" + mariadb_exec "REVOKE reporting_reader FROM 'example-user'@'%';" + "${KUBECTL}" annotate -f ${projectdir}/examples/${API_TYPE}/mysql/grant_member_of.yaml reconcile=role-membership --overwrite + for _ in $(seq 1 30); do + if [[ $(mariadb_exec "${membership_query}") == "1" ]]; then + break + fi + sleep 2 + done + [[ $(mariadb_exec "${membership_query}") == "1" ]] + + echo_info "check if deletion removes only the managed membership" + "${KUBECTL}" delete --timeout 2m -f ${projectdir}/examples/${API_TYPE}/mysql/grant_member_of.yaml + [[ $(mariadb_exec "${membership_query}") == "0" ]] + local grants_after + grants_after=$(mariadb_exec "SHOW GRANTS FOR 'example-user'@'%';" | sort) + [[ "${grants_before}" == "${grants_after}" ]] + + mariadb_exec "DROP ROLE reporting_reader; DROP ROLE unrelated_reader;" + echo_step_completed +} + test_all() { test_create_database test_database_charset @@ -393,6 +451,7 @@ test_all() { test_create_user test_update_user_password test_create_grant + test_role_membership } cleanup_test_resources() { diff --git a/examples/cluster/mysql/grant_member_of.yaml b/examples/cluster/mysql/grant_member_of.yaml new file mode 100644 index 00000000..8e5e4f61 --- /dev/null +++ b/examples/cluster/mysql/grant_member_of.yaml @@ -0,0 +1,19 @@ +# Grant an existing role to a user. +# Create reporting_reader before applying this example. +# Activate the role separately, for example with SET ROLE or a default role. +# The provider needs WITH ADMIN OPTION for the role (or equivalent admin rights) +# and SELECT on mysql.roles_mapping (MariaDB) or mysql.role_edges (MySQL). +# Deleting this Grant removes the role from this user without deleting either account. +# To change roles without keeping the old role assigned, delete and recreate the Grant. +apiVersion: mysql.sql.crossplane.io/v1alpha1 +kind: Grant +metadata: + name: example-grant-member-of +spec: + forProvider: + userRef: + name: example-user + # MySQL roles may use role@host; the default host is %. Use bare roles on MariaDB. + # For Aurora S3 exports, use AWS_SELECT_S3_ACCESS instead. + # Set up IAM/S3 access separately. + memberOf: reporting_reader diff --git a/examples/namespaced/mysql/grant_member_of.yaml b/examples/namespaced/mysql/grant_member_of.yaml new file mode 100644 index 00000000..c3349f24 --- /dev/null +++ b/examples/namespaced/mysql/grant_member_of.yaml @@ -0,0 +1,23 @@ +# Grant an existing role to a user. +# Create reporting_reader before applying this example. +# Activate the role separately, for example with SET ROLE or a default role. +# The provider needs WITH ADMIN OPTION for the role (or equivalent admin rights) +# and SELECT on mysql.roles_mapping (MariaDB) or mysql.role_edges (MySQL). +# Deleting this Grant removes the role from this user without deleting either account. +# To change roles without keeping the old role assigned, delete and recreate the Grant. +apiVersion: mysql.sql.m.crossplane.io/v1alpha1 +kind: Grant +metadata: + name: example-grant-member-of + namespace: default +spec: + forProvider: + userRef: + name: example-user + # MySQL roles may use role@host; the default host is %. Use bare roles on MariaDB. + # For Aurora S3 exports, use AWS_SELECT_S3_ACCESS instead. + # Set up IAM/S3 access separately. + memberOf: reporting_reader + providerConfigRef: + kind: ProviderConfig + name: default diff --git a/package/crds/mysql.sql.crossplane.io_grants.yaml b/package/crds/mysql.sql.crossplane.io_grants.yaml index 98e14f7e..2e377e94 100644 --- a/package/crds/mysql.sql.crossplane.io_grants.yaml +++ b/package/crds/mysql.sql.crossplane.io_grants.yaml @@ -34,6 +34,9 @@ spec: - jsonPath: .spec.forProvider.database name: DATABASE type: string + - jsonPath: .spec.forProvider.memberOf + name: MEMBER OF + type: string - jsonPath: .spec.forProvider.privileges name: PRIVILEGES type: string @@ -164,10 +167,20 @@ spec: type: string type: object type: object + memberOf: + description: |- + MemberOf is an existing role to grant to the user. + Use name@host to specify a MySQL role host; the default host is %. + Cannot be combined with privileges, database, databaseRef, databaseSelector, or table. + This Grant does not configure default roles or session activation. + Deleting the Grant revokes the membership without deleting the user or role. + minLength: 1 + type: string privileges: description: |- Privileges to be granted. See https://mariadb.com/kb/en/grant/#database-privileges for available privileges. + Cannot be combined with MemberOf. items: description: GrantPrivilege represents a privilege to be granted pattern: ^([A-Z0-9_ ]+|[A-Z_ ]+ [(]`[^`]+`(, `[^`]+`)*[)])$ @@ -256,9 +269,16 @@ spec: type: string type: object type: object - required: - - privileges type: object + x-kubernetes-validations: + - message: specify either memberOf or privileges + rule: has(self.memberOf) != has(self.privileges) + - message: memberOf cannot be combined with database or table targeting + rule: '!has(self.memberOf) || !(has(self.database) || has(self.databaseRef) + || has(self.databaseSelector) || has(self.table))' + - message: memberOf requires a user, userRef, or userSelector + rule: '!has(self.memberOf) || has(self.user) || has(self.userRef) + || has(self.userSelector)' managementPolicies: default: - '*' diff --git a/package/crds/mysql.sql.m.crossplane.io_grants.yaml b/package/crds/mysql.sql.m.crossplane.io_grants.yaml index 8975b824..bb449cb1 100644 --- a/package/crds/mysql.sql.m.crossplane.io_grants.yaml +++ b/package/crds/mysql.sql.m.crossplane.io_grants.yaml @@ -34,6 +34,9 @@ spec: - jsonPath: .spec.forProvider.database name: DATABASE type: string + - jsonPath: .spec.forProvider.memberOf + name: MEMBER OF + type: string - jsonPath: .spec.forProvider.privileges name: PRIVILEGES type: string @@ -156,10 +159,20 @@ spec: type: string type: object type: object + memberOf: + description: |- + MemberOf is an existing role to grant to the user. + Use name@host to specify a MySQL role host; the default host is %. + Cannot be combined with privileges, database, databaseRef, databaseSelector, or table. + This Grant does not configure default roles or session activation. + Deleting the Grant revokes the membership without deleting the user or role. + minLength: 1 + type: string privileges: description: |- Privileges to be granted. See https://mariadb.com/kb/en/grant/#database-privileges for available privileges. + Cannot be combined with MemberOf. items: description: GrantPrivilege represents a privilege to be granted pattern: ^([A-Z0-9_ ]+|[A-Z_ ]+ [(]`[^`]+`(, `[^`]+`)*[)])$ @@ -254,9 +267,16 @@ spec: type: string type: object type: object - required: - - privileges type: object + x-kubernetes-validations: + - message: specify either memberOf or privileges + rule: has(self.memberOf) != has(self.privileges) + - message: memberOf cannot be combined with database or table targeting + rule: '!has(self.memberOf) || !(has(self.database) || has(self.databaseRef) + || has(self.databaseSelector) || has(self.table))' + - message: memberOf requires a user, userRef, or userSelector + rule: '!has(self.memberOf) || has(self.user) || has(self.userRef) + || has(self.userSelector)' managementPolicies: default: - '*' diff --git a/pkg/clients/mysql/role_grants.go b/pkg/clients/mysql/role_grants.go new file mode 100644 index 00000000..e0e26a4a --- /dev/null +++ b/pkg/clients/mysql/role_grants.go @@ -0,0 +1,106 @@ +// SPDX-FileCopyrightText: 2026 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package mysql + +import ( + "context" + "fmt" + + mysqldriver "github.com/go-sql-driver/mysql" + "github.com/pkg/errors" + + "github.com/crossplane-contrib/provider-sql/pkg/clients/xsql" +) + +const ( + // MariaDB roles have no host, so only the implicit default role host matches. + selectRolesMapping = "SELECT EXISTS(SELECT 1 FROM mysql.roles_mapping " + + "WHERE `Role` = ? AND `User` = ? AND `Host` = ? AND ? = '%')" + selectRoleEdges = "SELECT EXISTS(SELECT 1 FROM mysql.role_edges " + + "WHERE FROM_USER = ? AND FROM_HOST = ? AND TO_USER = ? AND TO_HOST = ?)" +) + +type roleAccount struct { + name string + host string +} + +func (a roleAccount) sql() string { + return QuoteIdentifier(a.name) + "@" + QuoteIdentifier(a.host) +} + +func (a roleAccount) roleSQL() string { + if a.host == "%" { + return QuoteIdentifier(a.name) + } + return a.sql() +} + +func membershipAccounts(user, memberOf string) (roleAccount, roleAccount, error) { + if user == "" || memberOf == "" { + return roleAccount{}, roleAccount{}, errors.New("role membership requires a user and a role") + } + username, userhost := SplitUserHost(user) + rolename, rolehost := SplitUserHost(memberOf) + return roleAccount{name: username, host: userhost}, roleAccount{name: rolename, host: rolehost}, nil +} + +// RoleMembershipExists checks one direct membership in mysql.roles_mapping or +// mysql.role_edges. Missing users or roles have no matching membership. +// Administrative options are not managed here. +func RoleMembershipExists(ctx context.Context, db xsql.DB, user, memberOf string) (bool, error) { + grantee, role, err := membershipAccounts(user, memberOf) + if err != nil { + return false, err + } + exists := false + mappingErr := db.Scan(ctx, xsql.Query{ + String: selectRolesMapping, + Parameters: []interface{}{role.name, grantee.name, grantee.host, role.host}, + }, &exists) + if mappingErr == nil { + return exists, nil + } + + // A missing catalog can also return SELECT denied, so try the other catalog + // on any query error. + if err := db.Scan(ctx, xsql.Query{ + String: selectRoleEdges, + Parameters: []interface{}{role.name, role.host, grantee.name, grantee.host}, + }, &exists); err != nil { + return false, fmt.Errorf("cannot read role membership: mysql.roles_mapping: %w; mysql.role_edges: %w", mappingErr, err) + } + return exists, nil +} + +// GrantRoleMembership grants only the requested membership. It neither creates +// the role nor changes its activation or administrative options. +func GrantRoleMembership(ctx context.Context, db xsql.DB, user, memberOf string) error { + grantee, role, err := membershipAccounts(user, memberOf) + if err != nil { + return err + } + return db.Exec(ctx, xsql.Query{String: "GRANT " + role.roleSQL() + " TO " + grantee.sql()}) +} + +// RevokeRoleMembership removes only the requested membership, not the user or +// role. Dropping either account also removes its memberships. +func RevokeRoleMembership(ctx context.Context, db xsql.DB, user, memberOf string) error { + grantee, role, err := membershipAccounts(user, memberOf) + if err != nil { + return err + } + err = db.Exec(ctx, xsql.Query{String: "REVOKE " + role.roleSQL() + " FROM " + grantee.sql()}) + var myErr *mysqldriver.MySQLError + if errors.As(err, &myErr) { + switch myErr.Number { + case 1141, 3523: // MySQL: no such grant or unknown user/role. + return nil + case 1959, 1962: // MariaDB: invalid role or no such membership/recipient. + return nil + } + } + return err +} diff --git a/pkg/clients/mysql/role_grants_integration_test.go b/pkg/clients/mysql/role_grants_integration_test.go new file mode 100644 index 00000000..1366f7c4 --- /dev/null +++ b/pkg/clients/mysql/role_grants_integration_test.go @@ -0,0 +1,127 @@ +// SPDX-FileCopyrightText: 2026 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package mysql + +import ( + "database/sql" + "os" + "sort" + "strconv" + "testing" + "time" + + "github.com/google/go-cmp/cmp" +) + +// TestRoleMembershipIntegration runs against an explicitly supplied disposable +// server. It creates and removes its own users, roles, and database. +func TestRoleMembershipIntegration(t *testing.T) { + dsn := os.Getenv("MYSQL_TEST_DSN") + if dsn == "" { + t.Skip("set MYSQL_TEST_DSN to run against a disposable server") + } + admin, err := sql.Open("mysql", dsn) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = admin.Close() }) + ctx := t.Context() + + suffix := strconv.FormatInt(time.Now().UnixNano(), 36) + user := "mysql_user_" + suffix + role := "mysql_role_" + suffix + other := "mysql_other_" + suffix + database := "mysql_db_" + suffix + grantee := roleAccount{name: user, host: "%"} + roleSQL := QuoteIdentifier(role) + otherSQL := QuoteIdentifier(other) + exec := func(query string) { + t.Helper() + if _, err := admin.ExecContext(ctx, query); err != nil { + t.Fatal(err) + } + } + t.Cleanup(func() { + // Cleanup also runs after a failed assertion; individual accounts may have + // already been removed by the test. + _, _ = admin.Exec("DROP USER IF EXISTS " + grantee.sql()) + _, _ = admin.Exec("DROP ROLE IF EXISTS " + roleSQL) + _, _ = admin.Exec("DROP ROLE IF EXISTS " + otherSQL) + _, _ = admin.Exec("DROP DATABASE IF EXISTS " + QuoteIdentifier(database)) + }) + exec("CREATE USER " + grantee.sql()) + exec("CREATE ROLE " + roleSQL) + exec("CREATE ROLE " + otherSQL) + exec("CREATE DATABASE " + QuoteIdentifier(database)) + exec("GRANT SELECT ON " + QuoteIdentifier(database) + ".* TO " + grantee.sql()) + exec("GRANT " + otherSQL + " TO " + grantee.sql() + " WITH ADMIN OPTION") + + db := mySQLDB{dsn: dsn} + grants := func() []string { + t.Helper() + rows, err := admin.QueryContext(ctx, "SHOW GRANTS FOR "+grantee.sql()) + if err != nil { + t.Fatal(err) + } + defer rows.Close() + var got []string + for rows.Next() { + var grant string + if err := rows.Scan(&grant); err != nil { + t.Fatal(err) + } + got = append(got, grant) + } + if err := rows.Err(); err != nil { + t.Fatal(err) + } + sort.Strings(got) + return got + } + assertMembership := func(role string, want bool) { + t.Helper() + got, err := RoleMembershipExists(ctx, db, user, role) + if err != nil { + t.Fatal(err) + } + if got != want { + t.Fatalf("membership in %q = %v, want %v", role, got, want) + } + } + + before := grants() + assertMembership(role, false) + for range 2 { + if err := GrantRoleMembership(ctx, db, user, role); err != nil { + t.Fatal(err) + } + assertMembership(role, true) + } + for range 2 { + if err := RevokeRoleMembership(ctx, db, user, role); err != nil { + t.Fatal(err) + } + assertMembership(role, false) + } + if diff := cmp.Diff(before, grants()); diff != "" { + t.Fatalf("unrelated grants changed (-want, +got):\n%s", diff) + } + + // Dropping either account removes the membership. Observation and deletion + // should not require the now-missing account to be recreated. + if err := GrantRoleMembership(ctx, db, user, role); err != nil { + t.Fatal(err) + } + exec("DROP ROLE " + roleSQL) + assertMembership(role, false) + if err := RevokeRoleMembership(ctx, db, user, role); err != nil { + t.Fatal(err) + } + exec("DROP USER " + grantee.sql()) + assertMembership(other, false) + if err := RevokeRoleMembership(ctx, db, user, other); err != nil { + t.Fatal(err) + } +} diff --git a/pkg/clients/mysql/role_grants_test.go b/pkg/clients/mysql/role_grants_test.go new file mode 100644 index 00000000..0c26b264 --- /dev/null +++ b/pkg/clients/mysql/role_grants_test.go @@ -0,0 +1,191 @@ +// SPDX-FileCopyrightText: 2026 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package mysql + +import ( + "context" + "database/sql" + "errors" + "testing" + + "github.com/DATA-DOG/go-sqlmock" + mysqldriver "github.com/go-sql-driver/mysql" + + "github.com/crossplane-contrib/provider-sql/pkg/clients/xsql" +) + +const ( + readerGrant = "GRANT `reader` TO `app`@`%`" + readerRevoke = "REVOKE `reader` FROM `app`@`%`" +) + +// roleTestDB adapts sqlmock without implementing unrelated xsql operations. +type roleTestDB struct { + xsql.DB + db *sql.DB +} + +func (d roleTestDB) Scan(ctx context.Context, q xsql.Query, dest ...interface{}) error { + return d.db.QueryRowContext(ctx, q.String, q.Parameters...).Scan(dest...) +} + +func (d roleTestDB) Exec(ctx context.Context, q xsql.Query) error { + _, err := d.db.ExecContext(ctx, q.String, q.Parameters...) + return err +} + +func newRoleTestDB(t *testing.T) (roleTestDB, sqlmock.Sqlmock) { + t.Helper() + db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } + _ = db.Close() + }) + return roleTestDB{db: db}, mock +} + +func TestRoleMembershipExists(t *testing.T) { + missingMapping := &mysqldriver.MySQLError{Number: 1146, Message: "mysql.roles_mapping does not exist"} + denied := &mysqldriver.MySQLError{Number: 1142, Message: "SELECT denied"} + mappingErr := errors.New("roles_mapping unavailable") + edgesErr := errors.New("role_edges unavailable") + + cases := map[string]struct { + user string + role string + exists bool + mappingErr error + edgesErr error + wantErr bool + }{ + "MariaDBPresent": {user: "app", role: "reader", exists: true}, + "MariaDBAbsent": {user: "app", role: "reader"}, + "MariaDBExplicitUserHost": {user: "app@10.%", role: "reader", exists: true}, + "MariaDBNonDefaultRoleHost": {user: "app", role: "reader@localhost"}, + "UppercaseRoleName": {user: "app", role: "AWS_SELECT_S3_ACCESS", exists: true}, + "MySQLPresent": {user: "app", role: "reader", exists: true, mappingErr: missingMapping}, + "MySQLAbsent": {user: "app", role: "reader", mappingErr: missingMapping}, + "MySQLExplicitHosts": {user: "app@10.%", role: "reader@localhost", exists: true, mappingErr: missingMapping}, + "PermissionDeniedFallback": {user: "app", role: "reader", exists: true, mappingErr: denied}, + "OtherQueryErrorFallback": {user: "app", role: "reader", exists: true, mappingErr: mappingErr}, + "BothQueriesUnavailable": {user: "app", role: "reader", mappingErr: mappingErr, edgesErr: edgesErr, wantErr: true}, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + db, mock := newRoleTestDB(t) + username, userhost := SplitUserHost(tc.user) + rolename, rolehost := SplitUserHost(tc.role) + mapping := mock.ExpectQuery(selectRolesMapping). + WithArgs(rolename, username, userhost, rolehost) + if tc.mappingErr == nil { + mapping.WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(tc.exists)) + } else { + mapping.WillReturnError(tc.mappingErr) + edges := mock.ExpectQuery(selectRoleEdges). + WithArgs(rolename, rolehost, username, userhost) + if tc.edgesErr == nil { + edges.WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(tc.exists)) + } else { + edges.WillReturnError(tc.edgesErr) + } + } + + exists, err := RoleMembershipExists(t.Context(), db, tc.user, tc.role) + if tc.wantErr { + for _, cause := range []error{tc.mappingErr, tc.edgesErr} { + if !errors.Is(err, cause) { + t.Errorf("RoleMembershipExists() error = %v, want cause %v", err, cause) + } + } + } else if err != nil { + t.Fatal(err) + } + if exists != tc.exists { + t.Errorf("RoleMembershipExists() = %v, want %v", exists, tc.exists) + } + }) + } +} + +func TestGrantRoleMembership(t *testing.T) { + for name, tc := range map[string]struct { + user string + role string + sql string + }{ + "DefaultHosts": {user: "app", role: "reader", sql: readerGrant}, + "ExplicitDefaultHost": {user: "app", role: "reader@%", sql: readerGrant}, + "ExplicitHosts": {user: "app@10.%", role: "reader@localhost", sql: "GRANT `reader`@`localhost` TO `app`@`10.%`"}, + "Escaping": {user: "a`p'p\\", role: "r`e'a\\", sql: "GRANT `r``e'a\\` TO `a``p'p\\`@`%`"}, + } { + t.Run(name, func(t *testing.T) { + db, mock := newRoleTestDB(t) + mock.ExpectExec(tc.sql).WillReturnResult(sqlmock.NewResult(0, 0)) + if err := GrantRoleMembership(t.Context(), db, tc.user, tc.role); err != nil { + t.Fatal(err) + } + }) + } + t.Run("ExecError", func(t *testing.T) { + db, mock := newRoleTestDB(t) + want := errors.New("grant denied") + mock.ExpectExec(readerGrant).WillReturnError(want) + if err := GrantRoleMembership(t.Context(), db, "app", "reader"); !errors.Is(err, want) { + t.Fatalf("error = %v, want %v", err, want) + } + }) +} + +func TestRevokeRoleMembership(t *testing.T) { + for name, tc := range map[string]struct { + execErr error + wantErr bool + }{ + "SuccessOrAlreadyAbsent": {}, + "NoGrant": {execErr: &mysqldriver.MySQLError{Number: 1141}}, + "UnknownUserOrRole": {execErr: &mysqldriver.MySQLError{Number: 3523}}, + "MariaDBUnknownRole": {execErr: &mysqldriver.MySQLError{Number: 1959}}, + "MariaDBNoMembership": {execErr: &mysqldriver.MySQLError{Number: 1962}}, + "MariaDBPermissionError": {execErr: &mysqldriver.MySQLError{Number: 1698}, wantErr: true}, + "Unavailable": {execErr: errors.New("unavailable"), wantErr: true}, + } { + t.Run(name, func(t *testing.T) { + db, mock := newRoleTestDB(t) + exec := mock.ExpectExec(readerRevoke) + if tc.execErr != nil { + exec.WillReturnError(tc.execErr) + } else { + exec.WillReturnResult(sqlmock.NewResult(0, 0)) + } + err := RevokeRoleMembership(t.Context(), db, "app", "reader") + if tc.wantErr && !errors.Is(err, tc.execErr) { + t.Fatalf("error = %v, want %v", err, tc.execErr) + } + if !tc.wantErr && err != nil { + t.Fatal(err) + } + }) + } +} + +func TestMissingMembershipUser(t *testing.T) { + db, _ := newRoleTestDB(t) + const want = "role membership requires a user and a role" + if _, err := RoleMembershipExists(t.Context(), db, "", "reader"); err == nil || err.Error() != want { + t.Errorf("RoleMembershipExists() error = %v, want %q", err, want) + } + if err := GrantRoleMembership(t.Context(), db, "", "reader"); err == nil || err.Error() != want { + t.Errorf("GrantRoleMembership() error = %v, want %q", err, want) + } + if err := RevokeRoleMembership(t.Context(), db, "", "reader"); err == nil || err.Error() != want { + t.Errorf("RevokeRoleMembership() error = %v, want %q", err, want) + } +} diff --git a/pkg/controller/cluster/mysql/grant/reconciler.go b/pkg/controller/cluster/mysql/grant/reconciler.go index 9c358aa4..3ead5314 100644 --- a/pkg/controller/cluster/mysql/grant/reconciler.go +++ b/pkg/controller/cluster/mysql/grant/reconciler.go @@ -28,6 +28,7 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -147,6 +148,18 @@ type external struct{ db xsql.DB } var _ managed.TypedExternalClient[*v1alpha1.Grant] = &external{} func (c *external) Observe(ctx context.Context, mg *v1alpha1.Grant) (managed.ExternalObservation, error) { + if mg.Spec.ForProvider.MemberOf != nil { + exists, err := mysql.RoleMembershipExists(ctx, c.db, ptr.Deref(mg.Spec.ForProvider.User, ""), *mg.Spec.ForProvider.MemberOf) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, errCurrentGrant) + } + mg.Status.AtProvider.Privileges = nil + if exists { + mg.SetConditions(xpv2.Available()) + } + return managed.ExternalObservation{ResourceExists: exists, ResourceUpToDate: exists}, nil + } + username, host := mysql.SplitUserHost(*mg.Spec.ForProvider.User) dbname := defaultIdentifier(mg.Spec.ForProvider.Database) table := defaultIdentifier(mg.Spec.ForProvider.Table) @@ -288,6 +301,10 @@ func (c *external) parseGrantRows(ctx context.Context, username, host, dbname, t } func (c *external) Create(ctx context.Context, mg *v1alpha1.Grant) (managed.ExternalCreation, error) { + if mg.Spec.ForProvider.MemberOf != nil { + return managed.ExternalCreation{}, errors.Wrap(mysql.GrantRoleMembership(ctx, c.db, ptr.Deref(mg.Spec.ForProvider.User, ""), *mg.Spec.ForProvider.MemberOf), errCreateGrant) + } + username, host := mysql.SplitUserHost(*mg.Spec.ForProvider.User) dbname := defaultIdentifier(mg.Spec.ForProvider.Database) table := defaultIdentifier(mg.Spec.ForProvider.Table) @@ -302,6 +319,11 @@ func (c *external) Create(ctx context.Context, mg *v1alpha1.Grant) (managed.Exte } func (c *external) Update(ctx context.Context, mg *v1alpha1.Grant) (managed.ExternalUpdate, error) { + if mg.Spec.ForProvider.MemberOf != nil { + // Observe reports missing memberships as nonexistent, so Create applies them. + return managed.ExternalUpdate{}, nil + } + username, host := mysql.SplitUserHost(*mg.Spec.ForProvider.User) dbname := defaultIdentifier(mg.Spec.ForProvider.Database) table := defaultIdentifier(mg.Spec.ForProvider.Table) @@ -388,6 +410,10 @@ func (c *external) Disconnect(ctx context.Context) error { } func (c *external) Delete(ctx context.Context, mg *v1alpha1.Grant) (managed.ExternalDelete, error) { + if mg.Spec.ForProvider.MemberOf != nil { + return managed.ExternalDelete{}, errors.Wrap(mysql.RevokeRoleMembership(ctx, c.db, ptr.Deref(mg.Spec.ForProvider.User, ""), *mg.Spec.ForProvider.MemberOf), errRevokeGrant) + } + username, host := mysql.SplitUserHost(*mg.Spec.ForProvider.User) dbname := defaultIdentifier(mg.Spec.ForProvider.Database) table := defaultIdentifier(mg.Spec.ForProvider.Table) diff --git a/pkg/controller/cluster/mysql/grant/reconciler_test.go b/pkg/controller/cluster/mysql/grant/reconciler_test.go index ccddbcda..1fe99959 100644 --- a/pkg/controller/cluster/mysql/grant/reconciler_test.go +++ b/pkg/controller/cluster/mysql/grant/reconciler_test.go @@ -19,6 +19,7 @@ package grant import ( "context" "database/sql" + "fmt" "strings" "testing" @@ -208,6 +209,75 @@ func TestObserve(t *testing.T) { args args want want }{ + "MembershipPresent": { + reason: "An existing membership should be available and clear stale privilege status", + fields: fields{ + db: &mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + if diff := cmp.Diff([]interface{}{"reader", "app", "10.%", "%"}, q.Parameters); diff != "" { + return errors.Errorf("query parameters (-want, +got):\n%s", diff) + } + *dest[0].(*bool) = true + return nil + }, + }, + }, + args: args{mg: membershipGrant("SELECT")}, + want: want{ + o: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, + }, + }, + "MembershipAbsent": { + reason: "A missing membership should be reported as nonexistent and clear stale privilege status", + fields: fields{ + db: &mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + *dest[0].(*bool) = false + return nil + }, + }, + }, + args: args{mg: membershipGrant("SELECT")}, + }, + "ErrObserveMembership": { + reason: "Errors reading role membership should be returned without changing privilege status", + fields: fields{ + db: &mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + return errBoom + }, + }, + }, + args: args{mg: membershipGrant("SELECT")}, + want: want{ + err: errors.Wrap(fmt.Errorf("cannot read role membership: mysql.roles_mapping: %w; mysql.role_edges: %w", errBoom, errBoom), errCurrentGrant), + observedPrivileges: []string{"SELECT"}, + }, + }, + "PrivilegeGrantWithRoleMembership": { + reason: "Privilege grants should ignore role memberships without reading the role catalog", + fields: fields{ + db: &mockDB{ + MockQuery: func(ctx context.Context, q xsql.Query) (*sql.Rows, error) { + if want := "SHOW GRANTS FOR 'app'@'%'"; q.String != want { + return nil, errors.Errorf("query = %q, want %q", q.String, want) + } + return mockRowsToSQLRows(sqlmock.NewRows([]string{"Grants"}). + AddRow("GRANT `reader`@`%` TO `app`@`%`"). + AddRow("GRANT SELECT ON *.* TO `app`@`%`")), nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{Spec: v1alpha1.GrantSpec{ForProvider: v1alpha1.GrantParameters{ + User: ptr.To("app"), Privileges: v1alpha1.GrantPrivileges{"SELECT"}, + }}}, + }, + want: want{ + o: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, + observedPrivileges: []string{"SELECT"}, + }, + }, "SuccessNoGrant": { reason: "We should return ResourceExists: false when no grant is found, being privileges result empty", fields: fields{ @@ -626,6 +696,9 @@ func TestObserve(t *testing.T) { if diff := cmp.Diff(tc.want.observedPrivileges, cr.Status.AtProvider.Privileges, equateSlices()...); diff != "" { t.Errorf("\n%s\ne.Observe(...): -want, +got:\n%s\n", tc.reason, diff) } + if cr.Spec.ForProvider.MemberOf != nil && tc.want.o.ResourceExists && !cr.GetCondition(xpv2.TypeReady).Equal(xpv2.Available()) { + t.Errorf("\n%s\ne.Observe(...): expected available condition, got %+v\n", tc.reason, cr.GetCondition(xpv2.TypeReady)) + } } }) } @@ -654,6 +727,30 @@ func TestCreate(t *testing.T) { args args want want }{ + "SuccessMembership": { + reason: "Creating a membership should grant only the requested role", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { + if want := "GRANT `reader` TO `app`@`10.%`"; q.String != want { + return errors.Errorf("query = %q, want %q", q.String, want) + } + return nil + }, + }, + }, + args: args{mg: membershipGrant()}, + }, + "ErrCreateMembership": { + reason: "Errors granting a membership should be returned with context", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return errBoom }, + }, + }, + args: args{mg: membershipGrant()}, + want: want{err: errors.Wrap(errBoom, errCreateGrant)}, + }, "ErrExec": { reason: "Any errors encountered while creating the grant should be returned", fields: fields{ @@ -792,6 +889,11 @@ func TestUpdate(t *testing.T) { args args want want }{ + "Membership": { + reason: "Membership updates should execute no SQL because Observe reports missing memberships for Create", + fields: fields{db: &mockDB{}}, + args: args{mg: membershipGrant()}, + }, "ErrExecRevokeNotRequired": { reason: "Any errors encountered while revoking a not required privilege from the desired ones should be returned", fields: fields{ @@ -1021,6 +1123,30 @@ func TestDelete(t *testing.T) { args args want error }{ + "SuccessMembership": { + reason: "Deleting a membership should revoke only the requested role", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { + if want := "REVOKE `reader` FROM `app`@`10.%`"; q.String != want { + return errors.Errorf("query = %q, want %q", q.String, want) + } + return nil + }, + }, + }, + args: args{mg: membershipGrant()}, + }, + "ErrRevokeMembership": { + reason: "Errors revoking a membership should be returned with context", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return errBoom }, + }, + }, + args: args{mg: membershipGrant()}, + want: errors.Wrap(errBoom, errRevokeGrant), + }, "ErrDropGrant": { reason: "Errors dropping a grant should be returned", fields: fields{ @@ -1103,6 +1229,17 @@ func TestDelete(t *testing.T) { } } +func membershipGrant(privileges ...string) *v1alpha1.Grant { + return &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ForProvider: v1alpha1.GrantParameters{ + User: ptr.To("app@10.%"), MemberOf: ptr.To("reader"), + }}, + Status: v1alpha1.GrantStatus{ + AtProvider: v1alpha1.GrantObservation{Privileges: privileges}, + }, + } +} + func mockRowsToSQLRows(mockRows *sqlmock.Rows) *sql.Rows { db, mock, _ := sqlmock.New() mock.ExpectQuery("select").WillReturnRows(mockRows) diff --git a/pkg/controller/namespaced/mysql/grant/reconciler.go b/pkg/controller/namespaced/mysql/grant/reconciler.go index 38f892c2..9d23fd7b 100644 --- a/pkg/controller/namespaced/mysql/grant/reconciler.go +++ b/pkg/controller/namespaced/mysql/grant/reconciler.go @@ -26,6 +26,7 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/statemetrics" mysqldriver "github.com/go-sql-driver/mysql" "github.com/pkg/errors" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -129,6 +130,18 @@ type external struct{ db xsql.DB } var _ managed.TypedExternalClient[*namespacedv1alpha1.Grant] = &external{} func (c *external) Observe(ctx context.Context, mg *namespacedv1alpha1.Grant) (managed.ExternalObservation, error) { + if mg.Spec.ForProvider.MemberOf != nil { + exists, err := mysql.RoleMembershipExists(ctx, c.db, ptr.Deref(mg.Spec.ForProvider.User, ""), *mg.Spec.ForProvider.MemberOf) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, errCurrentGrant) + } + mg.Status.AtProvider.Privileges = nil + if exists { + mg.SetConditions(xpv2.Available()) + } + return managed.ExternalObservation{ResourceExists: exists, ResourceUpToDate: exists}, nil + } + username, host := mysql.SplitUserHost(*mg.Spec.ForProvider.User) dbname := defaultIdentifier(mg.Spec.ForProvider.Database) table := defaultIdentifier(mg.Spec.ForProvider.Table) @@ -270,6 +283,10 @@ func (c *external) parseGrantRows(ctx context.Context, username, host, dbname, t } func (c *external) Create(ctx context.Context, mg *namespacedv1alpha1.Grant) (managed.ExternalCreation, error) { + if mg.Spec.ForProvider.MemberOf != nil { + return managed.ExternalCreation{}, errors.Wrap(mysql.GrantRoleMembership(ctx, c.db, ptr.Deref(mg.Spec.ForProvider.User, ""), *mg.Spec.ForProvider.MemberOf), errCreateGrant) + } + username, host := mysql.SplitUserHost(*mg.Spec.ForProvider.User) dbname := defaultIdentifier(mg.Spec.ForProvider.Database) table := defaultIdentifier(mg.Spec.ForProvider.Table) @@ -284,6 +301,11 @@ func (c *external) Create(ctx context.Context, mg *namespacedv1alpha1.Grant) (ma } func (c *external) Update(ctx context.Context, mg *namespacedv1alpha1.Grant) (managed.ExternalUpdate, error) { + if mg.Spec.ForProvider.MemberOf != nil { + // Observe reports missing memberships as nonexistent, so Create applies them. + return managed.ExternalUpdate{}, nil + } + username, host := mysql.SplitUserHost(*mg.Spec.ForProvider.User) dbname := defaultIdentifier(mg.Spec.ForProvider.Database) table := defaultIdentifier(mg.Spec.ForProvider.Table) @@ -370,6 +392,10 @@ func (c *external) Disconnect(ctx context.Context) error { } func (c *external) Delete(ctx context.Context, mg *namespacedv1alpha1.Grant) (managed.ExternalDelete, error) { + if mg.Spec.ForProvider.MemberOf != nil { + return managed.ExternalDelete{}, errors.Wrap(mysql.RevokeRoleMembership(ctx, c.db, ptr.Deref(mg.Spec.ForProvider.User, ""), *mg.Spec.ForProvider.MemberOf), errRevokeGrant) + } + username, host := mysql.SplitUserHost(*mg.Spec.ForProvider.User) dbname := defaultIdentifier(mg.Spec.ForProvider.Database) table := defaultIdentifier(mg.Spec.ForProvider.Table) diff --git a/pkg/controller/namespaced/mysql/grant/reconciler_test.go b/pkg/controller/namespaced/mysql/grant/reconciler_test.go index 774cb182..a6f9bf80 100644 --- a/pkg/controller/namespaced/mysql/grant/reconciler_test.go +++ b/pkg/controller/namespaced/mysql/grant/reconciler_test.go @@ -19,6 +19,7 @@ package grant import ( "context" "database/sql" + "fmt" "strings" "testing" @@ -258,6 +259,75 @@ func TestObserve(t *testing.T) { args args want want }{ + "MembershipPresent": { + reason: "An existing membership should be available and clear stale privilege status", + fields: fields{ + db: &mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + if diff := cmp.Diff([]interface{}{"reader", "app", "10.%", "%"}, q.Parameters); diff != "" { + return errors.Errorf("query parameters (-want, +got):\n%s", diff) + } + *dest[0].(*bool) = true + return nil + }, + }, + }, + args: args{mg: membershipGrant("SELECT")}, + want: want{ + o: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, + }, + }, + "MembershipAbsent": { + reason: "A missing membership should be reported as nonexistent and clear stale privilege status", + fields: fields{ + db: &mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + *dest[0].(*bool) = false + return nil + }, + }, + }, + args: args{mg: membershipGrant("SELECT")}, + }, + "ErrObserveMembership": { + reason: "Errors reading role membership should be returned without changing privilege status", + fields: fields{ + db: &mockDB{ + MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error { + return errBoom + }, + }, + }, + args: args{mg: membershipGrant("SELECT")}, + want: want{ + err: errors.Wrap(fmt.Errorf("cannot read role membership: mysql.roles_mapping: %w; mysql.role_edges: %w", errBoom, errBoom), errCurrentGrant), + observedPrivileges: []string{"SELECT"}, + }, + }, + "PrivilegeGrantWithRoleMembership": { + reason: "Privilege grants should ignore role memberships without reading the role catalog", + fields: fields{ + db: &mockDB{ + MockQuery: func(ctx context.Context, q xsql.Query) (*sql.Rows, error) { + if want := "SHOW GRANTS FOR 'app'@'%'"; q.String != want { + return nil, errors.Errorf("query = %q, want %q", q.String, want) + } + return mockRowsToSQLRows(sqlmock.NewRows([]string{"Grants"}). + AddRow("GRANT `reader`@`%` TO `app`@`%`"). + AddRow("GRANT SELECT ON *.* TO `app`@`%`")), nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{Spec: v1alpha1.GrantSpec{ForProvider: v1alpha1.GrantParameters{ + User: ptr.To("app"), Privileges: v1alpha1.GrantPrivileges{"SELECT"}, + }}}, + }, + want: want{ + o: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, + observedPrivileges: []string{"SELECT"}, + }, + }, "SuccessNoGrant": { reason: "We should return ResourceExists: false when no grant is found, being privileges result empty", fields: fields{ @@ -676,6 +746,9 @@ func TestObserve(t *testing.T) { if diff := cmp.Diff(tc.want.observedPrivileges, cr.Status.AtProvider.Privileges, equateSlices()...); diff != "" { t.Errorf("\n%s\ne.Observe(...): -want, +got:\n%s\n", tc.reason, diff) } + if cr.Spec.ForProvider.MemberOf != nil && tc.want.o.ResourceExists && !cr.GetCondition(xpv2.TypeReady).Equal(xpv2.Available()) { + t.Errorf("\n%s\ne.Observe(...): expected available condition, got %+v\n", tc.reason, cr.GetCondition(xpv2.TypeReady)) + } } }) } @@ -704,6 +777,30 @@ func TestCreate(t *testing.T) { args args want want }{ + "SuccessMembership": { + reason: "Creating a membership should grant only the requested role", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { + if want := "GRANT `reader` TO `app`@`10.%`"; q.String != want { + return errors.Errorf("query = %q, want %q", q.String, want) + } + return nil + }, + }, + }, + args: args{mg: membershipGrant()}, + }, + "ErrCreateMembership": { + reason: "Errors granting a membership should be returned with context", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return errBoom }, + }, + }, + args: args{mg: membershipGrant()}, + want: want{err: errors.Wrap(errBoom, errCreateGrant)}, + }, "ErrExec": { reason: "Any errors encountered while creating the grant should be returned", fields: fields{ @@ -842,6 +939,11 @@ func TestUpdate(t *testing.T) { args args want want }{ + "Membership": { + reason: "Membership updates should execute no SQL because Observe reports missing memberships for Create", + fields: fields{db: &mockDB{}}, + args: args{mg: membershipGrant()}, + }, "ErrExecRevokeNotRequired": { reason: "Any errors encountered while revoking a not required privilege from the desired ones should be returned", fields: fields{ @@ -1071,6 +1173,30 @@ func TestDelete(t *testing.T) { args args want error }{ + "SuccessMembership": { + reason: "Deleting a membership should revoke only the requested role", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { + if want := "REVOKE `reader` FROM `app`@`10.%`"; q.String != want { + return errors.Errorf("query = %q, want %q", q.String, want) + } + return nil + }, + }, + }, + args: args{mg: membershipGrant()}, + }, + "ErrRevokeMembership": { + reason: "Errors revoking a membership should be returned with context", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return errBoom }, + }, + }, + args: args{mg: membershipGrant()}, + want: errors.Wrap(errBoom, errRevokeGrant), + }, "ErrDropGrant": { reason: "Errors dropping a grant should be returned", fields: fields{ @@ -1153,6 +1279,17 @@ func TestDelete(t *testing.T) { } } +func membershipGrant(privileges ...string) *v1alpha1.Grant { + return &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ForProvider: v1alpha1.GrantParameters{ + User: ptr.To("app@10.%"), MemberOf: ptr.To("reader"), + }}, + Status: v1alpha1.GrantStatus{ + AtProvider: v1alpha1.GrantObservation{Privileges: privileges}, + }, + } +} + func mockRowsToSQLRows(mockRows *sqlmock.Rows) *sql.Rows { db, mock, _ := sqlmock.New() mock.ExpectQuery("select").WillReturnRows(mockRows)