Skip to content

Commit ee1a9f7

Browse files
fix: Fix Bearer authentication with Nessie catalog (prestodb#26512)
## Description Bearer authentication for Nessie catalog is not working ## Motivation and Context Fixes the problem with Bearer authentication for Nessie catalog ## Impact Is not possible to connect to Nessie catalog using Bearer authentication ## Test Plan A new TestIcebergSystemTablesNessieWithBearerAuth class has been added. It extends TestIcebergSystemTablesNessie and re-runs those tests using Bearer authentication. The other Nessie tests are not duplicated, as this class sufficiently validates the authentication mechanism. Test adapted from trinodb/trino#17725 ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == Iceberg Connector Changes * Fix Bearer authentication with Nessie catalog.
1 parent a87fdb9 commit ee1a9f7

File tree

5 files changed

+278
-1
lines changed

5 files changed

+278
-1
lines changed

presto-iceberg/src/main/java/com/facebook/presto/iceberg/nessie/IcebergNessieCatalogFactory.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ protected Map<String, String> getCatalogProperties(ConnectorSession session)
5959
if (hash != null) {
6060
properties.put("ref.hash", hash);
6161
}
62+
catalogConfig.getAuthenticationType().ifPresent(val -> properties.put("nessie.authentication.type", val.toString()));
6263
catalogConfig.getReadTimeoutMillis().ifPresent(val -> properties.put("transport.read-timeout", val.toString()));
6364
catalogConfig.getConnectTimeoutMillis().ifPresent(val -> properties.put("transport.connect-timeout", val.toString()));
6465
catalogConfig.getClientBuilderImpl().ifPresent(val -> properties.put("client-builder-impl", val));
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package com.facebook.presto.iceberg.nessie;
15+
16+
import com.facebook.presto.Session;
17+
import com.facebook.presto.iceberg.IcebergConfig;
18+
import com.facebook.presto.iceberg.IcebergPlugin;
19+
import com.facebook.presto.testing.QueryRunner;
20+
import com.facebook.presto.testing.containers.KeycloakContainer;
21+
import com.facebook.presto.testing.containers.NessieContainer;
22+
import com.facebook.presto.tests.DistributedQueryRunner;
23+
import com.google.common.collect.ImmutableMap;
24+
import org.testcontainers.containers.Network;
25+
import org.testng.annotations.AfterClass;
26+
import org.testng.annotations.BeforeClass;
27+
import org.testng.annotations.Test;
28+
29+
import java.nio.file.Path;
30+
import java.util.Map;
31+
32+
import static com.facebook.presto.iceberg.CatalogType.NESSIE;
33+
import static com.facebook.presto.iceberg.IcebergQueryRunner.ICEBERG_CATALOG;
34+
import static com.facebook.presto.iceberg.IcebergQueryRunner.getIcebergDataDirectoryPath;
35+
import static com.facebook.presto.iceberg.nessie.NessieTestUtil.nessieConnectorProperties;
36+
import static com.facebook.presto.testing.TestingSession.testSessionBuilder;
37+
38+
public class TestIcebergSystemTablesNessieWithBearerAuth
39+
extends TestIcebergSystemTablesNessie
40+
{
41+
private NessieContainer nessieContainer;
42+
private KeycloakContainer keycloakContainer;
43+
44+
@BeforeClass
45+
@Override
46+
public void init()
47+
throws Exception
48+
{
49+
Map<String, String> envVars = ImmutableMap.<String, String>builder()
50+
.putAll(NessieContainer.DEFAULT_ENV_VARS)
51+
.put("QUARKUS_OIDC_AUTH_SERVER_URL", KeycloakContainer.SERVER_URL + "/realms/" + KeycloakContainer.MASTER_REALM)
52+
.put("QUARKUS_OIDC_CLIENT_ID", "nessie")
53+
.put("NESSIE_SERVER_AUTHENTICATION_ENABLED", "true")
54+
.buildOrThrow();
55+
56+
Network network = Network.newNetwork();
57+
58+
nessieContainer = NessieContainer.builder().withEnvVars(envVars).withNetwork(network).build();
59+
nessieContainer.start();
60+
keycloakContainer = KeycloakContainer.builder().withNetwork(network).build();
61+
keycloakContainer.start();
62+
63+
super.init();
64+
}
65+
66+
@AfterClass(alwaysRun = true)
67+
@Override
68+
public void tearDown()
69+
{
70+
super.tearDown();
71+
if (nessieContainer != null) {
72+
nessieContainer.stop();
73+
}
74+
if (keycloakContainer != null) {
75+
keycloakContainer.stop();
76+
}
77+
}
78+
79+
@Override
80+
protected QueryRunner createQueryRunner()
81+
throws Exception
82+
{
83+
Session session = testSessionBuilder()
84+
.setCatalog(ICEBERG_CATALOG)
85+
.build();
86+
87+
DistributedQueryRunner queryRunner = DistributedQueryRunner.builder(session).build();
88+
89+
Path dataDirectory = queryRunner.getCoordinator().getDataDirectory();
90+
Path catalogDirectory = getIcebergDataDirectoryPath(dataDirectory, "NESSIE", new IcebergConfig().getFileFormat(), false);
91+
92+
queryRunner.installPlugin(new IcebergPlugin());
93+
Map<String, String> icebergProperties = ImmutableMap.<String, String>builder()
94+
.put("iceberg.catalog.type", String.valueOf(NESSIE))
95+
.putAll(nessieConnectorProperties(nessieContainer.getRestApiUri()))
96+
.put("iceberg.catalog.warehouse", catalogDirectory.getParent().toFile().toURI().toString())
97+
.put("iceberg.nessie.auth.type", "BEARER")
98+
.put("iceberg.nessie.auth.bearer.token", keycloakContainer.getAccessToken())
99+
.build();
100+
101+
queryRunner.createCatalog(ICEBERG_CATALOG, "iceberg", icebergProperties);
102+
103+
icebergProperties = ImmutableMap.<String, String>builder()
104+
.put("iceberg.catalog.type", String.valueOf(NESSIE))
105+
.putAll(nessieConnectorProperties(nessieContainer.getRestApiUri()))
106+
.put("iceberg.catalog.warehouse", catalogDirectory.getParent().toFile().toURI().toString())
107+
.put("iceberg.nessie.auth.type", "BEARER")
108+
.put("iceberg.nessie.auth.bearer.token", "invalid_token")
109+
.build();
110+
111+
queryRunner.createCatalog("iceberg_invalid_credentials", "iceberg", icebergProperties);
112+
113+
return queryRunner;
114+
}
115+
116+
@Test
117+
public void testInvalidBearerToken()
118+
{
119+
assertQueryFails("CREATE SCHEMA iceberg_invalid_credentials.test_schema", "Unauthorized \\(HTTP/401\\).*", true);
120+
}
121+
}

presto-testing-docker/pom.xml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
<description>presto-testing-docker</description>
1414

1515
<properties>
16+
<dep.keycloak.client.version>26.0.7</dep.keycloak.client.version>
1617
<air.main.basedir>${project.parent.basedir}</air.main.basedir>
1718
<air.check.skip-modernizer>true</air.check.skip-modernizer>
1819
</properties>
@@ -64,6 +65,28 @@
6465
<groupId>net.jodah</groupId>
6566
<artifactId>failsafe</artifactId>
6667
</dependency>
68+
69+
<dependency>
70+
<groupId>org.keycloak</groupId>
71+
<artifactId>keycloak-admin-client</artifactId>
72+
<version>${dep.keycloak.client.version}</version>
73+
<exclusions>
74+
<exclusion>
75+
<groupId>org.jboss.resteasy</groupId>
76+
<artifactId>resteasy-jaxb-provider</artifactId>
77+
</exclusion>
78+
<exclusion>
79+
<groupId>org.eclipse.angus</groupId>
80+
<artifactId>angus-mail</artifactId>
81+
</exclusion>
82+
</exclusions>
83+
</dependency>
84+
85+
<dependency>
86+
<groupId>org.keycloak</groupId>
87+
<artifactId>keycloak-client-common-synced</artifactId>
88+
<version>${dep.keycloak.client.version}</version>
89+
</dependency>
6790
</dependencies>
6891

6992
<build>
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package com.facebook.presto.testing.containers;
15+
16+
import com.facebook.airlift.log.Logger;
17+
import com.google.common.collect.ImmutableList;
18+
import com.google.common.collect.ImmutableMap;
19+
import com.google.common.collect.ImmutableSet;
20+
import org.keycloak.admin.client.Keycloak;
21+
import org.keycloak.admin.client.KeycloakBuilder;
22+
import org.keycloak.admin.client.resource.RealmResource;
23+
import org.keycloak.representations.idm.RealmRepresentation;
24+
import org.testcontainers.containers.Network;
25+
26+
import java.util.Map;
27+
import java.util.Optional;
28+
import java.util.Set;
29+
30+
public class KeycloakContainer
31+
extends BaseTestContainer
32+
{
33+
private static final Logger log = Logger.get(KeycloakContainer.class);
34+
35+
public static final String DEFAULT_IMAGE = "quay.io/keycloak/keycloak:26.4.2";
36+
public static final String DEFAULT_HOST_NAME = "keycloak";
37+
38+
public static final String DEFAULT_USER_NAME = "admin";
39+
public static final String DEFAULT_PASSWORD = "admin";
40+
41+
public static final String MASTER_REALM = "master";
42+
public static final String ADMIN_CLI_CLIENT = "admin-cli";
43+
44+
public static final int PORT = 8080;
45+
public static final String SERVER_URL = "http://" + DEFAULT_HOST_NAME + ":" + PORT;
46+
47+
public static Builder builder()
48+
{
49+
return new Builder();
50+
}
51+
52+
protected KeycloakContainer(String image,
53+
String hostName,
54+
Set<Integer> exposePorts,
55+
Map<String, String> filesToMount,
56+
Map<String, String> envVars,
57+
Optional<Network> network,
58+
int retryLimit)
59+
{
60+
super(
61+
image,
62+
hostName,
63+
exposePorts,
64+
filesToMount,
65+
envVars,
66+
network,
67+
retryLimit);
68+
}
69+
70+
@Override
71+
protected void setupContainer()
72+
{
73+
super.setupContainer();
74+
withRunCommand(ImmutableList.of("start-dev"));
75+
}
76+
77+
@Override
78+
public void start()
79+
{
80+
super.start();
81+
log.info("Keycloak container started with URL: %s", getUrl());
82+
}
83+
84+
public String getUrl()
85+
{
86+
return "http://" + getMappedHostAndPortForExposedPort(PORT);
87+
}
88+
89+
public String getAccessToken()
90+
{
91+
try (Keycloak keycloak = KeycloakBuilder.builder()
92+
.serverUrl(getUrl())
93+
.realm(MASTER_REALM)
94+
.clientId(ADMIN_CLI_CLIENT)
95+
.username(DEFAULT_USER_NAME)
96+
.password(DEFAULT_PASSWORD)
97+
.build()) {
98+
RealmResource master = keycloak.realm(MASTER_REALM);
99+
RealmRepresentation masterRep = master.toRepresentation();
100+
// change access token lifespan from 1 minute (default) to 1 hour
101+
// to keep the token alive in case testcase takes more than a minute to finish execution.
102+
masterRep.setAccessTokenLifespan(3600);
103+
master.update(masterRep);
104+
return keycloak.tokenManager().getAccessTokenString();
105+
}
106+
}
107+
108+
public static class Builder
109+
extends BaseTestContainer.Builder<KeycloakContainer.Builder, KeycloakContainer>
110+
{
111+
private Builder()
112+
{
113+
this.image = DEFAULT_IMAGE;
114+
this.hostName = DEFAULT_HOST_NAME;
115+
this.exposePorts = ImmutableSet.of(PORT);
116+
this.envVars = ImmutableMap.of(
117+
"KC_BOOTSTRAP_ADMIN_USERNAME", DEFAULT_USER_NAME,
118+
"KC_BOOTSTRAP_ADMIN_PASSWORD", DEFAULT_PASSWORD,
119+
"KC_HOSTNAME", SERVER_URL);
120+
}
121+
122+
@Override
123+
public KeycloakContainer build()
124+
{
125+
return new KeycloakContainer(image, hostName, exposePorts, filesToMount, envVars, network, startupRetryLimit);
126+
}
127+
}
128+
}

presto-testing-docker/src/main/java/com/facebook/presto/testing/containers/NessieContainer.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ public class NessieContainer
3333

3434
public static final int PORT = 19121;
3535

36+
public static final ImmutableMap<String, String> DEFAULT_ENV_VARS = ImmutableMap.of(
37+
"QUARKUS_HTTP_PORT", String.valueOf(PORT),
38+
"NESSIE_VERSION_STORE_TYPE", VERSION_STORE_TYPE);
39+
3640
public static Builder builder()
3741
{
3842
return new Builder();
@@ -63,7 +67,7 @@ private Builder()
6367
this.image = DEFAULT_IMAGE;
6468
this.hostName = DEFAULT_HOST_NAME;
6569
this.exposePorts = ImmutableSet.of(PORT);
66-
this.envVars = ImmutableMap.of("QUARKUS_HTTP_PORT", String.valueOf(PORT), "NESSIE_VERSION_STORE_TYPE", VERSION_STORE_TYPE);
70+
this.envVars = DEFAULT_ENV_VARS;
6771
}
6872

6973
@Override

0 commit comments

Comments
 (0)