Skip to content

[refactor] deprecate db/common/role.DatabaseRoleMatchers#34132

Merged
greedy52 merged 3 commits intomasterfrom
STeve/27323_refactor_role
Nov 9, 2023
Merged

[refactor] deprecate db/common/role.DatabaseRoleMatchers#34132
greedy52 merged 3 commits intomasterfrom
STeve/27323_refactor_role

Conversation

@greedy52
Copy link
Copy Markdown
Contributor

@greedy52 greedy52 commented Nov 1, 2023

pre-req for MongoDB auto-user provisioning change #33717

Changes:

  • Removed role.DatabaseRoleMatchers
  • Added a new DisableDatabaseNameMatcher flag to role.RoleMatchersConfig
  • Refactored the logic of role.GetDatabaseRoleMatchers a little bit and added UT

No backporting necessary.

@greedy52 greedy52 added refactoring no-changelog Indicates that a PR does not require a changelog entry labels Nov 1, 2023
@greedy52 greedy52 self-assigned this Nov 1, 2023
@github-actions github-actions Bot requested a review from AntonAM November 1, 2023 20:47
@github-actions github-actions Bot added database-access Database access related issues and PRs size/md labels Nov 1, 2023
@greedy52 greedy52 force-pushed the STeve/27323_refactor_role branch from 20916ce to 82e41b1 Compare November 1, 2023 20:48
Copy link
Copy Markdown
Contributor

@Tener Tener left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory one may forget to supply one of (db, db user, db name) as these are now optional fields in the config. In practice I think this is unlikely.

@Tener
Copy link
Copy Markdown
Contributor

Tener commented Nov 2, 2023

I think we should update EnumerateDatabaseUsers to be aware of auto-provisioning, but perhaps this can be a separate PR.

// EnumerateDatabaseUsers specializes EnumerateEntities to enumerate db_users.
func (a *accessChecker) EnumerateDatabaseUsers(database types.Database, extraUsers ...string) EnumerationResult {
listFn := func(role types.Role, condition types.RoleConditionType) []string {
return role.GetDatabaseUsers(condition)
}
newMatcher := func(user string) RoleMatcher {
return NewDatabaseUserMatcher(database, user)
}
return a.EnumerateEntities(database, listFn, newMatcher, extraUsers...)
}

@greedy52
Copy link
Copy Markdown
Contributor Author

greedy52 commented Nov 2, 2023

I think we should update EnumerateDatabaseUsers to be aware of auto-provisioning, but perhaps this can be a separate PR.

// EnumerateDatabaseUsers specializes EnumerateEntities to enumerate db_users.
func (a *accessChecker) EnumerateDatabaseUsers(database types.Database, extraUsers ...string) EnumerationResult {
listFn := func(role types.Role, condition types.RoleConditionType) []string {
return role.GetDatabaseUsers(condition)
}
newMatcher := func(user string) RoleMatcher {
return NewDatabaseUserMatcher(database, user)
}
return a.EnumerateEntities(database, listFn, newMatcher, extraUsers...)
}

Good call. Will do a separate fix.

@greedy52 greedy52 enabled auto-merge November 9, 2023 14:33
@greedy52 greedy52 added this pull request to the merge queue Nov 9, 2023
Merged via the queue into master with commit 105df98 Nov 9, 2023
@greedy52 greedy52 deleted the STeve/27323_refactor_role branch November 9, 2023 15:12
greedy52 added a commit that referenced this pull request Dec 3, 2023
* [refactor] deprecate db/common/role.DatabaseRoleMatchers

* fix typo

* fix a small typo
github-merge-queue Bot pushed a commit that referenced this pull request Dec 4, 2023
* [refactor] deprecate db/common/role.DatabaseRoleMatchers (#34132)

* [refactor] deprecate db/common/role.DatabaseRoleMatchers

* fix typo

* fix a small typo

* Restore DatabaseRoleMatchers to avoid e breakage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

database-access Database access related issues and PRs no-changelog Indicates that a PR does not require a changelog entry refactoring size/md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants