Repository navigation
clickhouse: readonly = 2 for the read-only users - #540
Merged
Merged
Conversation
…ts, oonirun) #528 gave every SQL user readonly = 0. For users with only SELECT grants that changes nothing a query can do today (tested on 24.8 and 25.3: a readonly = 0 user with SELECT on ooni.* and oonitest.* is refused INSERT, ALTER, TRUNCATE, CREATE, DROP, temporary tables, url()/file(), SYSTEM and KILL with ACCESS_DENIED), but it drops the second lock: with readonly = 2 a grant added by mistake still can't make the user write. The ooniapi services that use these users never write to ClickHouse: oonimeasurements and oonirun call only query_click (ooni/backend ooniapi/services/*/src; oonirun reads url_priorities and runs a health check). Every ooniapi query carries a log_comment (common/dependencies.py, backend #1305), which readonly = 1 refuses (code 164); readonly = 2 allows it, and still refuses INSERT and SETTINGS readonly = 0. Production (SHOW ACCESS and system.settings_profile_elements on 2026-11-10): oonimeasurements data1: no profile, so readonly 2 from the default profile; data2: the same; data3: PROFILE readonly, so readonly 1, which refuses log_comment oonirun PROFILE write (readonly 0) on all three nodes So oonimeasurements gets what data1 and data2 have, and oonirun is tighter than production. Memory limits are unchanged (4 GiB and 61 GB). Tested with the role's tests (tests/run.sh, 24.8.6.70, nodes seeded like production): 1082 checks passed, among them, on every node, both users have readonly = 2, can set log_comment, can SELECT and cannot INSERT.
Contributor
Author
|
I double checked and oonirun does not insert into clickhouse, so this is correct. These default memory limits are too high, we should lower these after upgrading. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#528 gave every SQL user readonly = 0. For users with only SELECT grants that changes nothing a query can do today (tested on 24.8 and 25.3: a readonly = 0 user with SELECT on ooni.* and oonitest.* is refused INSERT, ALTER, TRUNCATE, CREATE, DROP, temporary tables, url()/file(), SYSTEM and KILL with ACCESS_DENIED), but it drops the second lock: with readonly = 2 a grant added by mistake still can't make the user write.
Every ooniapi query carries a log_comment (common/dependencies.py, backend #1305), which readonly = 1 refuses (code 164); readonly = 2 allows it, and still refuses INSERT and SETTINGS readonly = 0.
Production (SHOW ACCESS and system.settings_profile_elements on 2026-11-10):
oonimeasurements data1: no profile, so readonly 2 from the default
profile; data2: the same; data3: PROFILE readonly,
so readonly 1, which refuses log_comment
oonirun PROFILE write (readonly 0) on all three nodes
So oonimeasurements gets what data1 and data2 have, and oonirun is tighter than production. Memory limits are unchanged (4 GiB and 61 GB).
Tested with the role's tests (tests/run.sh, 24.8.6.70, nodes seeded like production): 1082 checks passed, among them, on every node, both users have readonly = 2, can set log_comment, can SELECT and cannot INSERT.