Skip to content

Omit alldbs rule in ACL SAVE/LIST and CONFIG REWRITE for compatibility#3964

Merged
madolson merged 1 commit into
valkey-io:unstablefrom
dvkashapov:implicit-alldbs
Jun 10, 2026
Merged

Omit alldbs rule in ACL SAVE/LIST and CONFIG REWRITE for compatibility#3964
madolson merged 1 commit into
valkey-io:unstablefrom
dvkashapov:implicit-alldbs

Conversation

@dvkashapov

@dvkashapov dvkashapov commented Jun 10, 2026

Copy link
Copy Markdown
Member

Database-level ACL #2309 introduced alldbs rule that was explicit for all users and because of that previous versions no longer had the ability to parse ACL strings produced by later versions.
Omit alldbs in ACLDescribeSelector(), that is used in ACL SAVE/LOAD and CONFIG REWRITE command paths so that downgrades would be possible if new feature was not used (db= and resetdbs rules).
Keep ACL GETUSER command's output as is and return alldbs in databases field because of command's field-value format.
Add test to check that ACL LIST omits implicit alldbs and add check to existing ACL SAVE and CONFIG REWRITE tests.

Fixes #3915

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR modifies how ACL (Access Control List) selector descriptions represent database permissions. The core implementation change stops explicitly emitting the alldbs token when a selector has the SELECTOR_FLAG_ALLDBS flag, treating it as an implicit default. Tests across multiple files are updated to verify and reflect this new behavior in ACL LIST output, persistence, and module API expectations.

Changes

ACL implicit alldbs representation

Layer / File(s) Summary
Implementation and core ACL behavior verification
src/acl.c, tests/unit/acl-v2.tcl, tests/unit/acl.tcl
ACLDescribeSelector() stops emitting the alldbs token for selectors with SELECTOR_FLAG_ALLDBS; new test verifies ACL LIST omits implicit alldbs; existing tests verify ACL SAVE and config rewrite exclude implicit alldbs from persisted output.
Module API test expectation updates
tests/unit/moduleapi/usercall.tcl
Module user ACL assertions updated to expect new ACL string format without the implicit alldbs token across three test scenarios.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • zuiderkwast
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the rationale, implementation approach, and testing strategy related to the alldbs rule changes across ACL serialization paths.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately describes the main change: omitting the alldbs rule from ACL SAVE/LIST and CONFIG REWRITE for backward compatibility.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dvkashapov dvkashapov changed the title Omit alldbs rule in ACL SAVE/LOAD and CONFIG REWRITE for compatibility Omit alldbs rule in ACL SAVE/LIST and CONFIG REWRITE for compatibility Jun 10, 2026
@madolson madolson merged commit f3bdf50 into valkey-io:unstable Jun 10, 2026
62 checks passed
@github-project-automation github-project-automation Bot moved this from Needs Review to To be backported in Valkey 9.1 Jun 10, 2026
@dvkashapov dvkashapov deleted the implicit-alldbs branch June 10, 2026 21:46
zuiderkwast pushed a commit to valkey-io/valkey-doc that referenced this pull request Jun 18, 2026
As per valkey-io/valkey#3721 and
valkey-io/valkey#3964, omit `alldbs` rules and
drop mentions of `sanitize-payload` and `skip-sanitize-payload` user
flags.

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To be backported

Development

Successfully merging this pull request may close these issues.

[Compatibility] ACL LIST emits new "alldbs" selector for the default user in 9.1 (absent in 9.0) - intended? backward-compat impact for ACL parsers

3 participants