Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source ClickHouse: add custom jdbc params #17031

Merged
merged 12 commits into from
Sep 27, 2022
Merged

Source ClickHouse: add custom jdbc params #17031

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
80f25ed
Source clickhouse: add jdbc params
sashaNeshcheret Sep 22, 2022
35748b6
Source clickhouse: add jdbc params
sashaNeshcheret Sep 22, 2022
97e5b41
Merge branch 'master' into omneshcheret/source-clickhouse-custom-para…
sashaNeshcheret Sep 22, 2022
ab75820
Source clickhouse: add jdbc params
sashaNeshcheret Sep 23, 2022
ba94e0f
Source clickhouse: updated tests
sashaNeshcheret Sep 23, 2022
7a193f9
Source clickhouse: format
sashaNeshcheret Sep 23, 2022
827d059
Source clickhouse: format
sashaNeshcheret Sep 23, 2022
2ae34db
Source clickhouse: format
sashaNeshcheret Sep 26, 2022
76f828a
Source clickhouse: format
sashaNeshcheret Sep 26, 2022
646f505
Merge remote-tracking branch 'origin/master' into omneshcheret/source…
sashaNeshcheret Sep 27, 2022
13295da
Source clickhouse: bump version
sashaNeshcheret Sep 27, 2022
8956887
auto-bump connector version [ci skip]
octavia-squidington-iii Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions ...ectors/source-clickhouse-strict-encrypt/src/test-integration/resources/expected_spec.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"host": {
"description": "The host endpoint of the Clickhouse cluster.",
"title": "Host",
"type": "string"
"type": "string",
"order": 0
},
"port": {
"description": "The port of the database.",
Expand All @@ -18,24 +19,34 @@
"minimum": 0,
"maximum": 65536,
"default": 8123,
"examples": ["8123"]
"examples": ["8123"],
"order": 1
},
"database": {
"description": "The name of the database.",
"title": "Database",
"type": "string",
"examples": ["default"]
"examples": ["default"],
"order": 2
},
"username": {
"description": "The username which is used to access the database.",
"title": "Username",
"type": "string"
"type": "string",
"order": 3
},
"password": {
"description": "The password associated with this username.",
"title": "Password",
"type": "string",
"airbyte_secret": true
"airbyte_secret": true,
"order": 4
},
"jdbc_url_params": {
"description": "Additional properties to pass to the JDBC URL string when connecting to the database formatted as 'key=value' pairs separated by the symbol '&'. (Eg. key1=value1&key2=value2&key3=value3). For more information read about <a href=\"https://jdbc.postgresql.org/documentation/head/connect.html\">JDBC URL parameters</a>.",
"title": "JDBC URL Parameters (Advanced)",
"type": "string",
"order": 5
}
}
}
Expand Down
18 changes: 14 additions & 4 deletions ...-clickhouse/src/main/java/io/airbyte/integrations/source/clickhouse/ClickHouseSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
import java.sql.JDBCType;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please replace star import

import java.util.stream.Collectors;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static io.airbyte.db.jdbc.JdbcUtils.AMPERSAND;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this import is not used in this class


public class ClickHouseSource extends AbstractJdbcSource<JDBCType> implements Source {

/**
Expand Down Expand Up @@ -90,9 +90,19 @@ public JsonNode toDatabaseConfig(final JsonNode config) {
config.get(JdbcUtils.PORT_KEY).asText(),
config.get(JdbcUtils.DATABASE_KEY).asText()));

boolean isAdditionalParamsExists = config.get(JdbcUtils.JDBC_URL_PARAMS_KEY) != null && !config.get(JdbcUtils.JDBC_URL_PARAMS_KEY).asText().isEmpty();
List<String> params = new ArrayList<>();
// assume ssl if not explicitly mentioned.
if (isSsl) {
jdbcUrl.append("?").append(SSL_MODE);
params.add(SSL_MODE);
}
if (isAdditionalParamsExists) {
params.add(config.get(JdbcUtils.JDBC_URL_PARAMS_KEY).asText());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what will happen if the user puts "sslmode=strict" into jdbc_url_params field? Specifically, will the connection continue to work and what the effective SSL mode of the connection will be in this case?

Copy link
Contributor Author

@sashaNeshcheret sashaNeshcheret Sep 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems parameters on clickhouse side puts in map and second parameter with the same key will replace first parameter value, so param passed via jdbc_param_url will have higher priority as far as we firstly set sslmode=none implicitly in code. As far as clickhouse source supports ssl just with none mode and no certificates are created on server, if user puts "sslmode=strict", the following error will be shown: "unable to find valid certification path to requested target", as far as user didn't specify any certificate. @grishick

}

if (isSsl || isAdditionalParamsExists) {
jdbcUrl.append("?");
jdbcUrl.append(String.join("&", params));
}

final ImmutableMap.Builder<Object, Object> configBuilder = ImmutableMap.builder()
Expand Down
24 changes: 18 additions & 6 deletions airbyte-integrations/connectors/source-clickhouse/src/main/resources/spec.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"host": {
"description": "The host endpoint of the Clickhouse cluster.",
"title": "Host",
"type": "string"
"type": "string",
"order": 0
},
"port": {
"description": "The port of the database.",
Expand All @@ -18,30 +19,41 @@
"minimum": 0,
"maximum": 65536,
"default": 8123,
"examples": ["8123"]
"examples": ["8123"],
"order": 1
},
"database": {
"description": "The name of the database.",
"title": "Database",
"type": "string",
"examples": ["default"]
"examples": ["default"],
"order": 2
},
"username": {
"description": "The username which is used to access the database.",
"title": "Username",
"type": "string"
"type": "string",
"order": 3
},
"password": {
"description": "The password associated with this username.",
"title": "Password",
"type": "string",
"airbyte_secret": true
"airbyte_secret": true,
"order": 4
},
"jdbc_url_params": {
"description": "Additional properties to pass to the JDBC URL string when connecting to the database formatted as 'key=value' pairs separated by the symbol '&'. (Eg. key1=value1&key2=value2&key3=value3). For more information read about <a href=\"https://jdbc.postgresql.org/documentation/head/connect.html\">JDBC URL parameters</a>.",
"title": "JDBC URL Parameters (Advanced)",
"type": "string",
"order": 5
},
"ssl": {
"title": "SSL Connection",
"description": "Encrypt data using SSL.",
"type": "boolean",
"default": true
"default": true,
"order": 6
}
}
}
Expand Down
67 changes: 67 additions & 0 deletions ...integrations/io/airbyte/integration_tests/sources/ClickHouseJdbcSourceAcceptanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@

package io.airbyte.integrations.io.airbyte.integration_tests.sources;

import static io.airbyte.db.jdbc.JdbcUtils.JDBC_URL_KEY;
import static io.airbyte.integrations.source.clickhouse.ClickHouseSource.SSL_MODE;
import static java.time.temporal.ChronoUnit.SECONDS;
import static org.junit.jupiter.api.Assertions.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please replace star import


import com.fasterxml.jackson.databind.JsonNode;
import com.google.common.collect.ImmutableMap;
Expand All @@ -18,6 +21,9 @@
import java.sql.SQLException;
import java.time.Duration;
import java.util.List;

import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -114,4 +120,65 @@ public AbstractJdbcSource<JDBCType> getJdbcSource() {
return new ClickHouseSource();
}

@Test
public void testEmptyExtraParamsWithSsl() {
final String extraParam = "";
JsonNode config = buildConfigWithExtraJdbcParameters(extraParam, true);
final JsonNode jdbcConfig = new ClickHouseSource().toDatabaseConfig(config);
JsonNode jdbcUrlNode = jdbcConfig.get(JDBC_URL_KEY);
assertNotNull(jdbcUrlNode);
String actualJdbcUrl = jdbcUrlNode.asText();
assertTrue(actualJdbcUrl.endsWith("?" + SSL_MODE));
}

@Test
public void testEmptyExtraParamsWithoutSsl() {
final String extraParam = "";
JsonNode config = buildConfigWithExtraJdbcParameters(extraParam, false);
final JsonNode jdbcConfig = new ClickHouseSource().toDatabaseConfig(config);
JsonNode jdbcUrlNode = jdbcConfig.get(JDBC_URL_KEY);
assertNotNull(jdbcUrlNode);
String actualJdbcUrl = jdbcUrlNode.asText();
assertTrue(actualJdbcUrl.endsWith(config.get("database").asText()));
}

@Test
public void testExtraParamsWithSsl() {
final String extraParam = "key1=value1&key2=value2&key3=value3";
JsonNode config = buildConfigWithExtraJdbcParameters(extraParam, true);
final JsonNode jdbcConfig = new ClickHouseSource().toDatabaseConfig(config);
JsonNode jdbcUrlNode = jdbcConfig.get(JDBC_URL_KEY);
assertNotNull(jdbcUrlNode);
String actualJdbcUrl = jdbcUrlNode.asText();
assertTrue(actualJdbcUrl.endsWith(getFullExpectedValue(extraParam, SSL_MODE)));
}

@Test
public void testExtraParamsWithoutSsl() {
final String extraParam = "key1=value1&key2=value2&key3=value3";
JsonNode config = buildConfigWithExtraJdbcParameters(extraParam, false);
final JsonNode jdbcConfig = new ClickHouseSource().toDatabaseConfig(config);
JsonNode jdbcUrlNode = jdbcConfig.get(JDBC_URL_KEY);
assertNotNull(jdbcUrlNode);
String actualJdbcUrl = jdbcUrlNode.asText();
assertTrue(actualJdbcUrl.endsWith("?" + extraParam));
}

private String getFullExpectedValue(String extraParam, String sslMode) {
StringBuilder expected = new StringBuilder();
return expected.append("?").append(sslMode).append("&").append(extraParam).toString();
}

private JsonNode buildConfigWithExtraJdbcParameters(String extraParam, boolean isSsl) {

return Jsons.jsonNode(com.google.common.collect.ImmutableMap.of(
"host", "localhost",
"port", 8123,
"database", "db",
"username", "username",
"password", "verysecure",
"jdbc_url_params", extraParam,
"ssl", isSsl));
}

}