Commit Graph

37 Commits

Author SHA1 Message Date
Azat Khuzhin
bdfaffa9d7 tests: make test_distributed_inter_server_secret idempotent
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2024-08-06 16:59:23 +02:00
Azat Khuzhin
0dccc34a20 Fix cluster() for inter-server secret (preserve initial user as before)
The behaviour of cluster() with inter-server secret had been changed in #63013,
after it started to use "default" user, and this introduces a
regression.

The intention of that patch was to adjust only remote(), since it only
it accept custom user, which should be ignored.

Fixes: https://github.com/ClickHouse/ClickHouse/issues/66287
Fixes: https://github.com/ClickHouse/ClickHouse/issues/66352
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2024-08-05 20:24:24 +02:00
Azat Khuzhin
9d7710684b tests/test_distributed_inter_server_secret: get_query_user_info return list
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2024-08-05 20:24:24 +02:00
Nikita Mikhaylov
fe6a875c74 Make the setting back IMPORTANT + fix build 2024-07-04 11:12:27 +00:00
Nikita Mikhaylov
d57375181d Better 2024-07-04 11:12:27 +00:00
robot-clickhouse
910065e427 Automatic style fix 2024-07-04 11:12:27 +00:00
Nikita Mikhaylov
6fcd0eed06 Remove tests which are no longer relevant 2024-07-04 11:12:27 +00:00
Nikita Mikhaylov
782115efea Very bad change 2024-07-04 11:12:27 +00:00
Azat Khuzhin
3e68103ac8 Fix interserver secret for Distributed over Distributed from remote()
Right if you are executing remote() and later the query will
go to the cluster with interserver secret, then you should have the same
user on the nodes from that cluster, otherwise the query will fail with:

    DB::NetException: Connection reset by peer

And on the remote node:

    <Debug> TCPHandler: User (initial, interserver mode): new_user (client: 172.16.1.5:40536)
    <Debug> TCP_INTERSERVER-Session: d29ecf7d-2c1c-44d2-8cc9-4ab08175bf05 Authentication failed with error: new_user: Authentication failed: password is incorrect, or there is no user with such name.
    <Error> ServerErrorHandler: Code: 516. DB::Exception: new_user: Authentication failed: password is incorrect, or there is no user with such name. (AUTHENTICATION_FAILED), Stack trace (when copying this message, always include the lines below):

The problem is that remote() will not use passed to it user in
any form, and instead, the initial user will be used, i.e. "cli_user"
not "query_user":

    chc --user cli_user -q "select * from remote(node, default, some_dist_table, 'query_user')"

Fix this by using the user from query for the remote().

Note, that the Distributed over Distributed in case of tables still wont
work, for this you have to have the same users on all nodes in all
clusters that are involved in case of interserver secret is enabled (see
also test).

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

v2: move client initial_user adjustment into ClusterProxy/executeQuery.cpp
v3: we cannot check for interserver_mode in
    updateSettingsAndClientInfoForCluster() since it is not yet
    interserver in remote() context
2024-04-29 07:12:17 +02:00
Azat Khuzhin
5facb1d099 Get back test for old inter-server mode (DBMS_MIN_REVISION_WITH_INTERSERVER_SECRET non-v2)
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2024-04-27 15:29:14 +02:00
Raúl Marín
98e68ccfb8 Remove old tags from integration tests 2024-02-26 13:45:20 +01:00
Alexey Milovidov
74319b5cbd
Merge pull request #56082 from Algunenano/less_diff_images
See what happens if we use less different docker images in integration tests
2023-11-18 17:24:28 +01:00
Alexey Milovidov
09bec3c754 Fix integration test 2023-11-16 00:16:02 +01:00
Azat Khuzhin
c25d6cd624
Rename directory monitor concept into background INSERT (#55978)
* Limit log frequence for "Skipping send data over distributed table" message

After SYSTEM STOP DISTRIBUTED SENDS it will constantly print this
message.

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

* Rename directory monitor concept into async INSERT

Rename the following query settings (with preserving backward
compatiblity, by keeping old name as an alias):
- distributed_directory_monitor_sleep_time_ms -> distributed_async_insert_sleep_time_ms
- distributed_directory_monitor_max_sleep_time_ms -> distributed_async_insert_max_sleep_time_ms
- distributed_directory_monitor_batch -> distributed_async_insert_batch_inserts
- distributed_directory_monitor_split_batch_on_failure -> distributed_async_insert_split_batch_on_failure

Rename the following table settings (with preserving backward
compatiblity, by keeping old name as an alias):
- monitor_batch_inserts -> async_insert_batch
- monitor_split_batch_on_failure -> async_insert_split_batch_on_failure
- directory_monitor_sleep_time_ms -> async_insert_sleep_time_ms
- directory_monitor_max_sleep_time_ms -> async_insert_max_sleep_time_ms

And also update all the references:

    $ gg -e directory_monitor_ -e monitor_ tests docs | cut -d: -f1 | sort -u | xargs sed -e 's/distributed_directory_monitor_sleep_time_ms/distributed_async_insert_sleep_time_ms/g' -e 's/distributed_directory_monitor_max_sleep_time_ms/distributed_async_insert_max_sleep_time_ms/g' -e 's/distributed_directory_monitor_batch_inserts/distributed_async_insert_batch/g' -e 's/distributed_directory_monitor_split_batch_on_failure/distributed_async_insert_split_batch_on_failure/g' -e 's/monitor_batch_inserts/async_insert_batch/g' -e 's/monitor_split_batch_on_failure/async_insert_split_batch_on_failure/g' -e 's/monitor_sleep_time_ms/async_insert_sleep_time_ms/g' -e 's/monitor_max_sleep_time_ms/async_insert_max_sleep_time_ms/g' -i

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

* Rename async_insert for Distributed into background_insert

This will avoid amigibuity between general async INSERT's and INSERT
into Distributed, which are indeed background, so new term express it
even better.

Mostly done with:

    $ git di HEAD^ --name-only | xargs sed -i -e 's/distributed_async_insert/distributed_background_insert/g' -e 's/async_insert_batch/background_insert_batch/g' -e 's/async_insert_split_batch_on_failure/background_insert_split_batch_on_failure/g' -e 's/async_insert_sleep_time_ms/background_insert_sleep_time_ms/g' -e 's/async_insert_max_sleep_time_ms/background_insert_max_sleep_time_ms/g'

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

* Mark 02417_opentelemetry_insert_on_distributed_table as long

CI: https://s3.amazonaws.com/clickhouse-test-reports/55978/7a6abb03a0b507e29e999cb7e04f246a119c6f28/stateless_tests_flaky_check__asan_.html
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

---------

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2023-11-01 15:09:39 +01:00
Raúl Marín
52a3d37ebe Try reducing number of different images 2023-10-31 17:20:07 +01:00
vdimir
f8b1d7474d
Update test_distributed_inter_server_secret to pass with analyzer 2023-08-14 12:46:23 +00:00
Dmitry Novik
02ed17dfa5 Analyzer: do not enable it for old servers in tests 2023-08-04 14:16:33 +00:00
Azat Khuzhin
bcf381c5ae Reimplement interserver mode to avoid replay attacks
Prevous implementation (DBMS_MIN_REVISION_WITH_INTERSERVER_SECRET)
accepts the salt from the client, which make it useless.

Reimplement the protocol to send the salt by the server and use it in
the client instead.

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2023-03-15 08:28:43 +01:00
Alexey Milovidov
b42d26acfe Remove one line from XML, because we do not care 2022-08-28 02:44:02 +02:00
Alexander Tokmakov
19afeda4b1 fix style 2022-05-18 14:23:52 +02:00
Alexander Tokmakov
dea39d8175 fix some trash 2022-05-17 18:22:52 +02:00
Alexey Milovidov
19a8207ab7 Debug integration tests 2022-04-18 00:16:53 +02:00
Mikhail f. Shiryaev
e6f5a3f98b
Apply black formatter to all *.py files in the repo 2022-03-22 17:39:58 +01:00
Azat Khuzhin
5472aef084 Fix current_user/current_address for interserver mode
Before this patch current_user/current_address will be preserved from
the previous query.

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2022-02-02 17:44:19 +03:00
Vitaly Baranov
27f6d5864d
Merge pull request #29060 from azat/inter-server-secret-auth-fix
Do not allow to reuse previous credentials in case of inter-server secret
2021-10-01 20:44:48 +03:00
Azat Khuzhin
1af02f02bd Add a test for INSERT w/o user in interserver mode
v2: ensure that the test fails with the version w/o fix
v3: force connect by modifying config and reload it
v4: add comments
2021-10-01 01:13:08 +03:00
Alexey Milovidov
e513a5db32 Change <yandex> to <clickhouse> in configs 2021-09-20 01:38:53 +03:00
alexey-milovidov
0c70b06960
Merge branch 'master' into system-querylog-map 2021-03-31 04:54:30 +03:00
alesapin
1b0a9461f0 Fix more tests 2021-03-26 18:30:35 +03:00
sundy-li
76da6014d2 Update more integration tests2 2021-01-21 16:36:13 +08:00
sundy-li
ca039f5219 Update more integration tests 2021-01-21 14:55:13 +08:00
sundy-li
3b62c5b50b Fix test 2021-01-06 07:26:16 +00:00
sundy-li
3dda607059 Update tests 2021-01-05 11:22:53 +00:00
Azat Khuzhin
84583faa43 Fix test_distributed_inter_server_secret under ASAN
And also cover both cases:
- settings from DDL
- settings from TCP protocol
2020-10-03 11:16:31 +03:00
Azat Khuzhin
f25c1742b8 Pass through *_for_user settings via Distributed with cluster-secure
In cluster-secure case the user on shards (remote) is equal to the user
on the initiator, so those settings can be safely applied.
2020-10-03 02:04:47 +03:00
Azat Khuzhin
9cb3c743bd
Convert to python3 (#15007) 2020-10-02 19:54:07 +03:00
Azat Khuzhin
0159c74f21 Secure inter-cluster query execution (with initial_user as current query user) [v3]
Add inter-server cluster secret, it is used for Distributed queries
inside cluster, you can configure in the configuration file:

  <remote_servers>
      <logs>
          <shard>
              <secret>foobar</secret> <!-- empty -- works as before -->
              ...
          </shard>
      </logs>
  </remote_servers>

And this will allow clickhouse to make sure that the query was not
faked, and was issued from the node that knows the secret. And since
trust appeared it can use initial_user for query execution, this will
apply correct *_for_user (since with inter-server secret enabled, the
query will be executed from the same user on the shards as on initator,
unlike "default" user w/o it).

v2: Change user to the initial_user for Distributed queries if secret match
v3: Add Protocol::Cluster package
v4: Drop Protocol::Cluster and use plain Protocol::Hello + user marker
v5: Do not use user from Hello for cluster-secure (superfluous)
2020-09-15 01:36:28 +03:00