feat(admin): Extend /run_clickhouse_system_query to support custom SQL queries #2296
Conversation
This file contains 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
…m SQL queries You can now post custom SQL to /run_clickhouse_system_query in addition to requesting one of the predefined queries. This change includes some rudimentary validation to ensure that no attempt is made to insert, select or join on non system tables. Eventually this should be replaced by more robust parsing of the query into an AST. Depends on the API fixes included in #2294
Codecov Report
@@ Coverage Diff @@
## master #2296 +/- ##
==========================================
- Coverage 92.93% 92.83% -0.10%
==========================================
Files 556 557 +1
Lines 25494 25551 +57
==========================================
+ Hits 23692 23720 +28
- Misses 1802 1831 +29
Continue to review full report at Codecov.
|
fpacifici
approved these changes
Dec 17, 2021
Comment on lines
176
to
181
| # An incomplete list | ||
| VALID_SYSTEM_TABLES = [ | ||
| "clusters", | ||
| "merges", | ||
| "parts", | ||
| ] |
There was a problem hiding this comment.
You can allow all of them. There is nothing dangerous there.
But if you plan to keep it. Please add:
replication_queue
settings
metrics
columns
Comment on lines
212
to
228
| for kw in disallowed_keywords: | ||
| if kw in select_statement.lower(): | ||
| raise InvalidCustomQuery(f"{kw} is not allowed here") | ||
|
|
||
| system_table_name = match.group("system_table_name") | ||
|
|
||
| if system_table_name not in VALID_SYSTEM_TABLES: | ||
| raise InvalidCustomQuery("Invalid table") | ||
|
|
||
| extra = match.group("extra") | ||
|
|
||
| # Unfortunately "extra" is pretty permissive right now, just ensure | ||
| # there is no attempt to do a select, insert or join in there | ||
| if extra is not None: | ||
| for kw in disallowed_keywords: | ||
| if kw in extra.lower(): | ||
| raise InvalidCustomQuery(f"{kw} is not allowed here") |
There was a problem hiding this comment.
Let's have a proper AST here asap.
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.
You can now post custom SQL to /run_clickhouse_system_query in addition to requesting
one of the predefined ones. This change includes some rudimentary validation to
ensure that no attempt is made to insert, select or join on non system tables.
Eventually this should be replaced by more robust parsing of the query into an AST.
Depends on the API fixes included in #2294