ContentSquare / chproxy

Open-Source ClickHouse http proxy and load balancer
https://www.chproxy.org/
MIT License
1.29k stars 259 forks source link

[BUG] Incompatibility with ClickHouse API #476

Open yudelevi opened 1 month ago

yudelevi commented 1 month ago

Describe the bug There are two issues here:

  1. Introduced in #456, the /ping endpoint depends on authentication. The original endpoint in ClickHouse protocol doesn't require authentication, and the official client doesn't provide it. As a result, /ping fails. To Reproduce On the CH machine directly (user foo doesn't exist)

    [dyudelevich@ch-01 ~]$ curl http://foo:bar@127.0.0.1:8123/ping
    Ok.

    On chproxy:

    root@chproxy-dev-6f4d78665b-k2vml:/# curl http://foo:bar@127.0.01:9001/ping
    "127.0.0.1:50460": invalid username or password for user "foo"; query: ""

    A workaround for now is to add default/"" user to chproxy.

  2. Headers are missing when using the query by default "Transfer-Encoding: chunked" is supplied, but it's not being sent via chproxy To Reproduce Issue 2: on the CH machine directly:

    *   Trying 127.0.0.1:8123...
    * Connected to 127.0.0.1 (127.0.0.1) port 8123 (#0)
    * Server auth using Basic with user 'foo'
    > GET /ping HTTP/1.1
    > Host: 127.0.0.1:8123
    > Authorization: Basic Zm9vOmJhcg==
    > User-Agent: curl/7.76.1
    > Accept: */*
    >
    * Mark bundle as not supporting multiuse
    < HTTP/1.1 200 OK
    < Date: Tue, 01 Oct 2024 02:16:31 GMT
    < Connection: Keep-Alive
    < Content-Type: text/html; charset=UTF-8
    < Transfer-Encoding: chunked
    < Keep-Alive: timeout=3, max=9999
    < X-ClickHouse-Summary: {"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0","result_rows":"0","result_bytes":"0","elapsed_ns":"33303"}
    <
    Ok.
    * Connection #0 to host 127.0.0.1 left intact 

on chproxy:

root@chproxy-dev-6f4d78665b-k2vml:/# curl -v http://127.0.01:9001/ping
*   Trying 127.0.0.1:9001...
* Connected to 127.0.0.1 (127.0.0.1) port 9001 (#0)
> GET /ping HTTP/1.1
> Host: 127.0.0.1:9001
> User-Agent: curl/7.88.1
> Accept: */*
> 
< HTTP/1.1 200 OK
< Content-Type: text/html; charset=UTF-8
< Date: Tue, 01 Oct 2024 02:18:51 GMT
< X-Clickhouse-Summary: {"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0","result_rows":"0","result_bytes":"0","elapsed_ns":"56796"}
< Content-Length: 4
< 
Ok.

This causes the following error on clickhouse-connect client

  File “/usr/local/lib/python3.11/site-packages/clickhouse_connect/driver/buffer.py”, line 65, in read_leb128
    b = self.read_byte()
        ^^^^^^^^^^^^^^^^
  File “/usr/local/lib/python3.11/site-packages/clickhouse_connect/driver/buffer.py”, line 51, in read_byte
    chunk = next(self.gen, None)
            ^^^^^^^^^^^^^^^^^^^^
  File “/usr/local/lib/python3.11/site-packages/clickhouse_connect/driver/httputil.py”, line 231, in buffered
    chunk = next(read_gen, None) # Always try to read at least one chunk if there are any left
            ^^^^^^^^^^^^^^^^^^^^
  File “/usr/local/lib/python3.11/site-packages/urllib3/response.py”, line 1179, in read_chunked
    raise ResponseNotChunked(
urllib3.exceptions.ResponseNotChunked: Response is not chunked. Header ‘transfer-encoding: chunked’ is missing.

Environment information Linux, chproxy 1.26.5, clickhouse_connect==0.8.1

yalattas commented 1 month ago

same here

ibakhsh4salla commented 1 month ago

addr = parseForwardedHeader(fwd)

here I don't see this being called at all if we set:

proxy:
    enable: true
    header: Transfer-Encoding

in proxy configuration because it will always execute line: 61-62 and then jump to: 70

yudelevi commented 1 month ago

@ibakhsh4salla I'm looking at the code you mentioned, looks like it's a function to set up the forwarded IP header. I don't believe that transfer encoding should be set there.

lqhl commented 3 weeks ago

clickhouse-connect 0.8.6 (https://github.com/ClickHouse/clickhouse-connect/pull/421) has fixed the urllib3.exceptions.ResponseNotChunked exception (https://github.com/ClickHouse/clickhouse-connect/issues/417).

yudelevi commented 1 week ago

Can confirm that second part of this is resolved by clickhouse-connect 0.8.6