Add role query parameter to the HTTP interface

[slvrtrn]

Changelog category (leave one):

• New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Added role query parameter to the HTTP interface. It works similarly to SET ROLE x, applying the role before the statement is executed. This allows for overcoming the limitation of the HTTP interface, as multiple statements are not allowed, and it is not possible to send both SET ROLE x and the statement itself at the same time. It is possible to set multiple roles that way, e.g., ?role=x&role=y, which will be an equivalent of SET ROLE x, y.

Documentation entry for user-facing changes

• Documentation is written (mandatory for new features)

[robot-clickhouse-ci-2]

This is an automated comment for commit d9fd79e with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Check name	Description	Status
CI running	A meta-check that indicates the running CI. Normally, it's in success or pending state. The failed status indicates some problems with the PR	⏳ pending
Stress test	Runs stateless functional tests concurrently from several clients to detect concurrency-related errors	❌ failure

Successful checks

Check name	Description	Status
A Sync	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
AST fuzzer	Runs randomly generated queries to catch program errors. The build type is optionally given in parenthesis. If it fails, ask a maintainer for help	✅ success
ClickBench	Runs [ClickBench](https://github.com/ClickHouse/ClickBench/) with instant-attach table	✅ success
ClickHouse build check	Builds ClickHouse in various configurations for use in further steps. You have to fix the builds that fail. Build logs often has enough information to fix the error, but you might have to reproduce the failure locally. The cmake options can be found in the build log, grepping for cmake. Use these options and follow the general build process	✅ success
Compatibility check	Checks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help	✅ success
Docker keeper image	The check to build and optionally push the mentioned image to docker hub	✅ success
Docker server image	The check to build and optionally push the mentioned image to docker hub	✅ success
Docs check	Builds and tests the documentation	✅ success
Fast test	Normally this is the first check that is ran for a PR. It builds ClickHouse and runs most of stateless functional tests, omitting some. If it fails, further checks are not started until it is fixed. Look at the report to see which tests fail, then reproduce the failure locally as described here	✅ success
Flaky tests	Checks if new added or modified tests are flaky by running them repeatedly, in parallel, with more randomization. Functional tests are run 100 times with address sanitizer, and additional randomization of thread scheduling. Integrational tests are run up to 10 times. If at least once a new test has failed, or was too long, this check will be red. We don't allow flaky tests, read the doc	✅ success
Install packages	Checks that the built packages are installable in a clear environment	✅ success
Integration tests	The integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests	✅ success
Mergeable Check	Checks if all other necessary checks are successful	✅ success
PR Check	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
Performance Comparison	Measure changes in query performance. The performance test report is described in detail here. In square brackets are the optional part/total tests	✅ success
Stateful tests	Runs stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	✅ success
Stateless tests	Runs stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	✅ success
Style check	Runs a set of checks to keep the code style clean. If some of tests failed, see the related log from the report	✅ success
Unit tests	Runs the unit tests for different release types	✅ success
Upgrade check	Runs stress tests on server version from last release and then tries to upgrade it to the version from the PR. It checks if the new server can successfully startup without any errors, crashes or sanitizer asserts	✅ success

[vitlibar]

If the role doesn't exist then getID<Role>() will throw before this line. So this error message should be probably only about grants.

[slvrtrn]

Yes, you are right; this one probably should be ACCESS_DENIED then.

A question: If we call context->setCurrentRoles with a non-granted role, it is no-op without a failure; the error message is then not very specific, as it does not set the role, and just skips it. Then, if we are querying a table for which you need a specific grant, the query results in an error without a hint that the role is incorrect, saying that you need a grant instead (a standard one, as if you did not provide the role at all).

Is it intended behavior? That's why I do this isGranted check here, so the error message is a bit clearer.

[vitlibar]

Actually it should be rather ErrorCodes::SET_NON_GRANTED_ROLE - see

ClickHouse/src/Interpreters/Access/InterpreterSetRoleQuery.cpp

Line 88 in 6794f99

throw Exception(ErrorCodes::SET_NON_GRANTED_ROLE, "Role should be granted to set default");

.

Sometimes it's ok for Context::setCurrentRoles() to be no-op for a non-granted role, and sometimes it's not ok.
Probably we could extend Context::setCurrentRoles() function and add an argument check_roles_are_granted to control if the function should check if some roles are not granted and throw SET_NON_GRANTED_ROLE in that case.

[slvrtrn]

Thanks for pointing this out. I pushed a fix to use SET_NON_GRANTED_ROLE instead.

[slvrtrn]

@vitlibar, thanks for the review!

I noticed that the test that I added is, for some reason, flaky, as reported here: https://s3.amazonaws.com/clickhouse-test-reports/62669/f4af7e14ec580f65bdb4bf42672817b74a705428/stateless_tests_flaky_check__asan_.html

I checked it locally; running it just as a shell script (via sh) executes in less than a second. However, executing it via the clickhouse-test takes a very long time, and I suspect the same happened on the CI (everything timed out).

Edit: the long execution time was resolved after reducing the number of CLICKHOUSE_CLIENT calls. Still not sure what that was.

[vitlibar]

Let's move this algorithm extracting multiple values of a parameter to the class of params.

[slvrtrn]

@vitlibar, this is coming from Poco, namely, Poco::ListMap, if I am not mistaken; is it OK to add it there? I assume it is since it is included in the CH source code and not as a contrib module.

[slvrtrn]

I added a new NameValueCollection::getAll method to extract all the parameters. It might be useful in the future.

[vitlibar]

Well, I thought about modifying DB::HTMLForm actually, but probably modifying NameValueCollection is also ok.

[slvrtrn]

@vitlibar, I addressed the feedback; please have a look when you have time.

The test is also simplified. I think there is no need to create the tables because we only want to check if the correct role was assigned.

[slvrtrn]

@vitlibar, thanks for the review. If the failed test runs are unrelated, shall we merge?

[vitlibar]

shall we merge?

yes, why not? I've approved your PR.

[slvrtrn]

@vitlibar, I don't seem to have the merge privileges. Can you please merge it?