Skip to content

Commit

Permalink
Impersonation (#240)
Browse files Browse the repository at this point in the history
* Add SET ROLE tests for clusters
* Enable connection impersonation
* Fix tests after the driver upgrade
* Bump Docker and CI versions
* Split check and build finally
* Refactor version introspection, simplify the test setup, split check jobs
* Bump Metabase version
* Bump workflows versions
* Update memoization with Cal's code
* Update CHANGELOG, add 1.5.0 to the README
* Add a few more JDBC settings, remove HTTP dependency
* Bump JDBC, add impersonation tests on a cluster
  • Loading branch information
slvrtrn authored May 28, 2024
1 parent 47a64ec commit 08500c1
Show file tree
Hide file tree
Showing 17 changed files with 564 additions and 223 deletions.
2 changes: 1 addition & 1 deletion .docker/clickhouse/single_node_tls/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM clickhouse/clickhouse-server:24.3-alpine
FROM clickhouse/clickhouse-server:24.4-alpine
COPY .docker/clickhouse/single_node_tls/certificates /etc/clickhouse-server/certs
RUN chown clickhouse:clickhouse -R /etc/clickhouse-server/certs \
&& chmod 600 /etc/clickhouse-server/certs/* \
Expand Down
140 changes: 110 additions & 30 deletions .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,35 @@ on:
- "**.md"
pull_request:

env:
METABASE_VERSION: v0.49.12

jobs:
check:
check-local-current-version:
runs-on: ubuntu-latest
steps:
- name: Checkout Metabase Repo
uses: actions/checkout@v2
uses: actions/checkout@v4
with:
repository: metabase/metabase
ref: v0.49.11
ref: ${{ env.METABASE_VERSION }}

- name: Remove incompatible tests
# dataset-definition-test tests test data definition,
# and is currently failing for an unknown reason
# and is currently failing for an unknown reason.
# metabase.query-processor.middleware.permissions-test does not look like it is related to the driver at all,
# but it is failing on the CI only.
run: |
echo "(ns metabase.test.data.dataset-definition-test)" > test/metabase/test/data/dataset_definition_test.clj
echo "(ns metabase.query-processor.middleware.permissions-test)" > test/metabase/query_processor/middleware/permissions_test.clj
- name: Checkout Driver Repo
uses: actions/checkout@v2
uses: actions/checkout@v4
with:
path: modules/drivers/clickhouse

- name: Prepare JDK 17
uses: actions/setup-java@v2
uses: actions/setup-java@v4
with:
distribution: "temurin"
java-version: "17"
Expand All @@ -41,33 +47,36 @@ jobs:
sudo echo "127.0.0.1 server.clickhouseconnect.test" | sudo tee -a /etc/hosts
- name: Start ClickHouse in Docker
uses: isbang/compose-action@v1.4.1
uses: hoverkraft-tech/compose-action@v2.0.0
with:
compose-file: "modules/drivers/clickhouse/docker-compose.yml"
down-flags: "--volumes"
services: |
clickhouse
clickhouse_older_version
clickhouse_tls
clickhouse_older_version
clickhouse_cluster_node1
clickhouse_cluster_node2
nginx
- name: Install Clojure CLI
run: |
curl -O https://download.clojure.org/install/linux-install-1.11.1.1182.sh &&
sudo bash ./linux-install-1.11.1.1182.sh
- name: Setup Node
uses: actions/setup-node@v2
uses: actions/setup-node@v4
with:
node-version: "18"
node-version: "20"
cache: "yarn"

- name: Get M2 cache
uses: actions/cache@v2
with:
path: |
~/.m2
~/.gitlibs
key: ${{ runner.os }}-clickhouse-${{ hashFiles('**/deps.edn') }}
# - name: Get M2 cache
# uses: actions/cache@v4
# with:
# path: |
# ~/.m2
# ~/.gitlibs
# key: ${{ runner.os }}-clickhouse-${{ hashFiles('**/deps.edn') }}

- name: Prepare stuff for pulses
run: yarn build-static-viz
Expand All @@ -78,18 +87,96 @@ jobs:
mkdir -p /home/runner/.config/clojure
cat modules/drivers/clickhouse/.github/deps.edn | sed -e "s|PWD|$PWD|g" > /home/runner/.config/clojure/deps.edn
- name: Run ClickHouse driver tests with 23.3
- name: Run all tests with the latest ClickHouse version
env:
DRIVERS: clickhouse
MB_CLICKHOUSE_TEST_PORT: 8124
run: |
clojure -X:dev:drivers:drivers-dev:test:user/clickhouse :only metabase.driver.clickhouse-test
clojure -X:dev:ee:ee-dev:drivers:drivers-dev:test:user/clickhouse
- name: Run all tests with the latest ClickHouse version
check-local-older-version:
runs-on: ubuntu-latest
steps:
- name: Checkout Metabase Repo
uses: actions/checkout@v4
with:
repository: metabase/metabase
ref: ${{ env.METABASE_VERSION }}

- name: Checkout Driver Repo
uses: actions/checkout@v4
with:
path: modules/drivers/clickhouse

- name: Prepare JDK 17
uses: actions/setup-java@v4
with:
distribution: "temurin"
java-version: "17"

- name: Add ClickHouse TLS instance to /etc/hosts
run: |
sudo echo "127.0.0.1 server.clickhouseconnect.test" | sudo tee -a /etc/hosts
- name: Start ClickHouse in Docker
uses: hoverkraft-tech/compose-action@v2.0.0
with:
compose-file: "modules/drivers/clickhouse/docker-compose.yml"
down-flags: "--volumes"
# FIXME: We only need to run a few specific tests with `clickhouse_older_version`
# Currently, we run all tests with it, which is unnecessary.
services: |
clickhouse
clickhouse_tls
clickhouse_older_version
clickhouse_cluster_node1
clickhouse_cluster_node2
nginx
- name: Install Clojure CLI
run: |
curl -O https://download.clojure.org/install/linux-install-1.11.1.1182.sh &&
sudo bash ./linux-install-1.11.1.1182.sh
# - name: Get M2 cache
# uses: actions/cache@v4
# with:
# path: |
# ~/.m2
# ~/.gitlibs
# key: ${{ runner.os }}-clickhouse-${{ hashFiles('**/deps.edn') }}

# Use custom deps.edn containing "user/clickhouse" alias to include driver sources
- name: Prepare deps.edn
run: |
mkdir -p /home/runner/.config/clojure
cat modules/drivers/clickhouse/.github/deps.edn | sed -e "s|PWD|$PWD|g" > /home/runner/.config/clojure/deps.edn
- name: Run ClickHouse driver tests with 23.3
env:
DRIVERS: clickhouse
MB_CLICKHOUSE_TEST_PORT: 8124
run: |
clojure -X:dev:drivers:drivers-dev:test:user/clickhouse
clojure -X:dev:ee:ee-dev:drivers:drivers-dev:test:user/clickhouse :only metabase.driver.clickhouse-test
build-jar:
runs-on: ubuntu-latest
needs: [ 'check-local-current-version', 'check-local-older-version' ]
steps:
- name: Checkout Metabase Repo
uses: actions/checkout@v4
with:
repository: metabase/metabase
ref: ${{ env.METABASE_VERSION }}

- name: Checkout Driver Repo
uses: actions/checkout@v4
with:
path: modules/drivers/clickhouse

- name: Install Clojure CLI
run: |
curl -O https://download.clojure.org/install/linux-install-1.11.1.1182.sh &&
sudo bash ./linux-install-1.11.1.1182.sh
- name: Build ClickHouse driver
run: |
Expand All @@ -98,14 +185,7 @@ jobs:
ls -lah resources/modules
- name: Archive driver JAR
uses: actions/upload-artifact@v2
uses: actions/upload-artifact@v4
with:
name: clickhouse.metabase-driver.jar
path: resources/modules/clickhouse.metabase-driver.jar

- name: Report test results
uses: mikepenz/action-junit-report@v2.8.1
if: always()
with:
report_paths: "**/target/junit/*.xml"
github_token: ${{ secrets.GITHUB_TOKEN }}
14 changes: 13 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
# 1.4.1
# 1.5.0

Metabase 0.49.12+ only.

### New features

* Added [Metabase CSV Uploads feature](https://www.metabase.com/docs/latest/databases/uploads) support, which is currently enabled with ClickHouse Cloud only. On-premise deployments support will be added in the next release. ([calherries](https://github.com/calherries), [#236](https://github.com/ClickHouse/metabase-clickhouse-driver/pull/236), [#238](https://github.com/ClickHouse/metabase-clickhouse-driver/pull/238))
* Added [Metabase connection impersonation feature](https://www.metabase.com/learn/permissions/impersonation) support. This feature will be enabled by the driver only if ClickHouse version 24.4+ is detected. ([#219](https://github.com/ClickHouse/metabase-clickhouse-driver/issues/219))

### Improvements

* Proper role setting support on cluster deployments (related issue: [#192](https://github.com/ClickHouse/metabase-clickhouse-driver/issues/192)).
* Bump the JDBC driver to [0.6.0-patch5](https://github.com/ClickHouse/clickhouse-java/releases/tag/v0.6.0-patch5).

### Bug fixes

Expand Down
30 changes: 22 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Please refer to the extensive documentation available on the Metabase website: [

ClickHouse driver's code should be inside the main Metabase repository checkout in `modules/drivers/clickhouse` directory.

Additionally, you need to tweak Metabase `deps.edn` a bit.
Additionally, you need to tweak Metabase's `deps.edn` a bit.

The easiest way to set up a development environment is as follows (mostly the same as in the [CI](https://github.com/enqueue/metabase-clickhouse-driver/blob/master/.github/workflows/check.yml)):

Expand All @@ -41,7 +41,7 @@ mkdir -p ~/.clojure
cat modules/drivers/clickhouse/.github/deps.edn | sed -e "s|PWD|$PWD|g" > ~/.clojure/deps.edn
```

Modifying `~/.clojure/deps.edn` will create two useful profiles: `user/clickhouse` that adds driver's sources to the classpath, and `user/test` that includes all the Metabase tests that are guaranteed to work with the driver.
Modifying `~/.clojure/deps.edn` will create a new profile: `user/clickhouse`, that adds driver's sources to the class path, and includes all the Metabase tests that are guaranteed to work with the driver.

* Install the Metabase dependencies:

Expand All @@ -63,32 +63,46 @@ Required for TLS tests.
sudo -- sh -c "echo 127.0.0.1 server.clickhouseconnect.test >> /etc/hosts"
```

* Start ClickHouse as a Docker container
* Start Docker containers

```bash
docker compose -f modules/drivers/clickhouse/docker-compose.yml up -d clickhouse
docker compose -f modules/drivers/clickhouse/docker-compose.yml up -d
```

Here's an overview of the started containers, which have the ports exposed to the `localhost` (see [docker-compose.yml](./docker-compose.yml)):

- Metabase with the ClickHouse driver loaded from the JAR file (port: 3000)
- Current ClickHouse version (port: 8123) - the main instance for all tests.
- Current ClickHouse cluster with two nodes (+ nginx as an LB, port: 8127) - required for the set role tests (verifying that the role is set correctly via the query parameters).
- Current ClickHouse version with TLS support (port: 8443) - required for the TLS tests.
- Older ClickHouse version (port: 8124) - required for the string functions tests (switch between UTF8 (current) and non-UTF8 (pre-23.8) versions), as well as to verify that certain features, such as connection impersonation, are disabled on the older server versions.

Now, you should be able to run the tests:

```bash
DRIVERS=clickhouse clojure -X:dev:drivers:drivers-dev:test:user/clickhouse:user/test
DRIVERS=clickhouse clojure -X:dev:drivers:drivers-dev:test:user/clickhouse
```

you can see that we have our profiles `:user/clickhouse:user/test` added to the command above, and with `DRIVERS=clickhouse` we instruct Metabase to run the tests only for ClickHouse.
you can see that we have our `:user/clickhouse` profile added to the command above, and with `DRIVERS=clickhouse` we instruct Metabase to run the tests only for ClickHouse.

NB: Omitting `DRIVERS` will run the tests for all the built-in database drivers.

If you want to run tests for only a specific namespace:

```bash
DRIVERS=clickhouse clojure -X:dev:drivers:drivers-dev:test:user/clickhouse:user/test :only metabase.driver.clickhouse-test
DRIVERS=clickhouse clojure -X:dev:drivers:drivers-dev:test:user/clickhouse :only metabase.driver.clickhouse-test
```

or even a single test:

```bash
DRIVERS=clickhouse clojure -X:dev:drivers:drivers-dev:test:user/clickhouse:user/test :only metabase.driver.clickhouse-test/clickhouse-nullable-arrays
DRIVERS=clickhouse clojure -X:dev:drivers:drivers-dev:test:user/clickhouse :only metabase.driver.clickhouse-test/clickhouse-nullable-arrays
```

Testing the driver with the older ClickHouse version (see [docker-compose.yml](./docker-compose.yml)):

```bash
MB_CLICKHOUSE_TEST_PORT=8124 DRIVERS=clickhouse clojure -X:dev:drivers:drivers-dev:test:user/clickhouse :only metabase.driver.clickhouse-test
```

## Building a jar
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ docker run -d -p 3000:3000 \
| 0.47.7+ | 1.2.5 |
| 0.48.x | 1.3.4 |
| 0.49.x | 1.4.0 |
| 0.49.12+ | 1.5.0 |

## Creating a Metabase Docker image with ClickHouse driver

Expand Down
2 changes: 1 addition & 1 deletion deps.edn
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

:deps
{com.clickhouse/clickhouse-jdbc$http
{:mvn/version "0.4.6"
{:mvn/version "0.6.0-patch5"
:exclusions [com.clickhouse/clickhouse-cli-client$shaded
com.clickhouse/clickhouse-grpc-client$shaded]}
com.widdindustries/cljc.java-time {:mvn/version "0.1.21"}}}
58 changes: 0 additions & 58 deletions docker-compose.cluster.yml

This file was deleted.

Loading

0 comments on commit 08500c1

Please sign in to comment.