diff --git a/docs/src/main/sphinx/connector/prometheus.rst b/docs/src/main/sphinx/connector/prometheus.rst index 8afa25fa1e36..604d61d8fa26 100644 --- a/docs/src/main/sphinx/connector/prometheus.rst +++ b/docs/src/main/sphinx/connector/prometheus.rst @@ -54,6 +54,8 @@ Property Name Description ``prometheus.query.chunk.size.duration`` The duration of each query to Prometheus ``prometheus.max.query.range.duration`` Width of overall query to Prometheus, will be divided into query-chunk-size-duration queries ``prometheus.cache.ttl`` How long values from this config file are cached +``prometheus.auth.user`` Username for basic authentication +``prometheus.auth.password`` Password for basic authentication ``prometheus.bearer.token.file`` File holding bearer token if needed for access to Prometheus ``prometheus.read-timeout`` How much time a query to Prometheus has before timing out ======================================== ============================================================================================ diff --git a/plugin/trino-prometheus/src/main/java/io/trino/plugin/prometheus/PrometheusClient.java b/plugin/trino-prometheus/src/main/java/io/trino/plugin/prometheus/PrometheusClient.java index 2bc109f44377..5b72dd9fa107 100644 --- a/plugin/trino-prometheus/src/main/java/io/trino/plugin/prometheus/PrometheusClient.java +++ b/plugin/trino-prometheus/src/main/java/io/trino/plugin/prometheus/PrometheusClient.java @@ -21,6 +21,8 @@ import io.trino.spi.type.DoubleType; import io.trino.spi.type.Type; import io.trino.spi.type.TypeManager; +import okhttp3.Credentials; +import okhttp3.Interceptor; import okhttp3.OkHttpClient; import okhttp3.OkHttpClient.Builder; import okhttp3.Request; @@ -39,8 +41,10 @@ import java.util.Set; import java.util.function.Supplier; +import static com.google.common.net.HttpHeaders.AUTHORIZATION; import static io.trino.plugin.prometheus.PrometheusErrorCode.PROMETHEUS_TABLES_METRICS_RETRIEVE_ERROR; import static io.trino.plugin.prometheus.PrometheusErrorCode.PROMETHEUS_UNKNOWN_ERROR; +import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR; import static io.trino.spi.type.TimestampWithTimeZoneType.createTimestampWithTimeZoneType; import static io.trino.spi.type.TypeSignature.mapType; import static io.trino.spi.type.VarcharType.VARCHAR; @@ -55,7 +59,6 @@ public class PrometheusClient static final String METRICS_ENDPOINT = "/api/v1/label/__name__/values"; private final OkHttpClient httpClient; - private final Optional bearerTokenFile; private final Supplier> tableSupplier; private final Type varcharMapType; @@ -66,9 +69,11 @@ public PrometheusClient(PrometheusConnectorConfig config, JsonCodec fetchMetrics(metricCodec, prometheusMetricsUri), @@ -137,8 +142,6 @@ private Map fetchMetrics(JsonCodec> metricsC public byte[] fetchUri(URI uri) { Request.Builder requestBuilder = new Request.Builder().url(uri.toString()); - getBearerAuthInfoFromFile().ifPresent(bearerToken -> requestBuilder.header("Authorization", "Bearer " + bearerToken)); - Response response; try { response = httpClient.newCall(requestBuilder.build()).execute(); @@ -150,10 +153,10 @@ public byte[] fetchUri(URI uri) throw new TrinoException(PROMETHEUS_UNKNOWN_ERROR, "Error reading metrics", e); } - throw new TrinoException(PROMETHEUS_UNKNOWN_ERROR, "Bad response " + response.code() + response.message()); + throw new TrinoException(PROMETHEUS_UNKNOWN_ERROR, "Bad response " + response.code() + " " + response.message()); } - private Optional getBearerAuthInfoFromFile() + private Optional getBearerAuthInfoFromFile(Optional bearerTokenFile) { return bearerTokenFile.map(tokenFileName -> { try { @@ -164,4 +167,38 @@ private Optional getBearerAuthInfoFromFile() } }); } + + private static void setupBasicAuth(OkHttpClient.Builder clientBuilder, Optional user, Optional password) + { + if (user.isPresent() && password.isPresent()) { + clientBuilder.addInterceptor(basicAuth(user.get(), password.get())); + } + } + + private static void setupTokenAuth(OkHttpClient.Builder clientBuilder, Optional accessToken) + { + accessToken.ifPresent(token -> clientBuilder.addInterceptor(tokenAuth(token))); + } + + private static Interceptor basicAuth(String user, String password) + { + requireNonNull(user, "user is null"); + requireNonNull(password, "password is null"); + if (user.contains(":")) { + throw new TrinoException(GENERIC_USER_ERROR, "Illegal character ':' found in username"); + } + + String credential = Credentials.basic(user, password); + return chain -> chain.proceed(chain.request().newBuilder() + .header(AUTHORIZATION, credential) + .build()); + } + + private static Interceptor tokenAuth(String accessToken) + { + requireNonNull(accessToken, "accessToken is null"); + return chain -> chain.proceed(chain.request().newBuilder() + .addHeader(AUTHORIZATION, "Bearer " + accessToken) + .build()); + } } diff --git a/plugin/trino-prometheus/src/main/java/io/trino/plugin/prometheus/PrometheusConnectorConfig.java b/plugin/trino-prometheus/src/main/java/io/trino/plugin/prometheus/PrometheusConnectorConfig.java index 1d09c91f5d19..9f217ed9d81b 100644 --- a/plugin/trino-prometheus/src/main/java/io/trino/plugin/prometheus/PrometheusConnectorConfig.java +++ b/plugin/trino-prometheus/src/main/java/io/trino/plugin/prometheus/PrometheusConnectorConfig.java @@ -18,6 +18,7 @@ import com.google.inject.spi.Message; import io.airlift.configuration.Config; import io.airlift.configuration.ConfigDescription; +import io.airlift.configuration.ConfigSecuritySensitive; import io.airlift.units.Duration; import io.airlift.units.MinDuration; @@ -37,6 +38,8 @@ public class PrometheusConnectorConfig private Duration cacheDuration = new Duration(30, TimeUnit.SECONDS); private Duration readTimeout = new Duration(10, TimeUnit.SECONDS); private File bearerTokenFile; + private String user; + private String password; @NotNull public URI getPrometheusURI() @@ -107,6 +110,33 @@ public PrometheusConnectorConfig setBearerTokenFile(File bearerTokenFile) return this; } + @NotNull + public Optional getUser() + { + return Optional.ofNullable(user); + } + + @Config("prometheus.auth.user") + public PrometheusConnectorConfig setUser(String user) + { + this.user = user; + return this; + } + + @NotNull + public Optional getPassword() + { + return Optional.ofNullable(password); + } + + @Config("prometheus.auth.password") + @ConfigSecuritySensitive + public PrometheusConnectorConfig setPassword(String password) + { + this.password = password; + return this; + } + @MinDuration("1s") public Duration getReadTimeout() { @@ -129,5 +159,11 @@ public void checkConfig() if (maxQueryRangeDuration < queryChunkSizeDuration) { throw new ConfigurationException(ImmutableList.of(new Message("prometheus.max.query.range.duration must be greater than prometheus.query.chunk.size.duration"))); } + if (getBearerTokenFile().isPresent() && (getUser().isPresent() || getPassword().isPresent())) { + throw new IllegalStateException("Either on of bearer token file or basic authentication should be used"); + } + if (getUser().isPresent() ^ getPassword().isPresent()) { + throw new IllegalStateException("Both username and password must be set when using basic authentication"); + } } } diff --git a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusQueryRunner.java b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusQueryRunner.java index 2cfc059e225b..0c4adc519401 100644 --- a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusQueryRunner.java +++ b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusQueryRunner.java @@ -19,6 +19,7 @@ import io.trino.Session; import io.trino.testing.DistributedQueryRunner; +import java.util.HashMap; import java.util.Map; import static io.airlift.testing.Closeables.closeAllSuppress; @@ -32,7 +33,7 @@ public final class PrometheusQueryRunner { private PrometheusQueryRunner() {} - public static DistributedQueryRunner createPrometheusQueryRunner(PrometheusServer server, Map extraProperties) + public static DistributedQueryRunner createPrometheusQueryRunner(PrometheusServer server, Map extraProperties, Map connectorProperties) throws Exception { DistributedQueryRunner queryRunner = null; @@ -40,9 +41,9 @@ public static DistributedQueryRunner createPrometheusQueryRunner(PrometheusServe queryRunner = DistributedQueryRunner.builder(createSession()).setExtraProperties(extraProperties).build(); queryRunner.installPlugin(new PrometheusPlugin()); - Map properties = ImmutableMap.of( - "prometheus.uri", server.getUri().toString()); - queryRunner.createCatalog("prometheus", "prometheus", properties); + connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties)); + connectorProperties.putIfAbsent("prometheus.uri", server.getUri().toString()); + queryRunner.createCatalog("prometheus", "prometheus", connectorProperties); return queryRunner; } catch (Throwable e) { @@ -73,7 +74,7 @@ public static PrometheusClient createPrometheusClient(PrometheusServer server) public static void main(String[] args) throws Exception { - DistributedQueryRunner queryRunner = createPrometheusQueryRunner(new PrometheusServer(), ImmutableMap.of("http-server.http.port", "8080")); + DistributedQueryRunner queryRunner = createPrometheusQueryRunner(new PrometheusServer(), ImmutableMap.of("http-server.http.port", "8080"), ImmutableMap.of()); Thread.sleep(10); Logger log = Logger.get(PrometheusQueryRunner.class); log.info("======== SERVER STARTED ========"); diff --git a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusServer.java b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusServer.java index 4a3e92820b24..17431a61138e 100644 --- a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusServer.java +++ b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/PrometheusServer.java @@ -20,24 +20,41 @@ import java.net.URI; import java.time.Duration; +import static org.testcontainers.utility.MountableFile.forClasspathResource; import static org.testng.Assert.fail; public class PrometheusServer implements Closeable { private static final int PROMETHEUS_PORT = 9090; - private static final String PROMETHEUS_DOCKER_IMAGE = "prom/prometheus:v2.15.1"; + private static final String DEFAULT_VERSION = "v2.15.1"; + public static final String LATEST_VERSION = "v2.35.0"; private static final Integer MAX_TRIES = 120; private static final Integer TIME_BETWEEN_TRIES_MILLIS = 1000; + public static final String USER = "admin"; + public static final String PASSWORD = "password"; + private final GenericContainer dockerContainer; public PrometheusServer() { - this.dockerContainer = new GenericContainer<>(PROMETHEUS_DOCKER_IMAGE) + this(DEFAULT_VERSION, false); + } + + public PrometheusServer(String version, boolean enableBasicAuth) + { + this.dockerContainer = new GenericContainer<>("prom/prometheus:" + version) .withExposedPorts(PROMETHEUS_PORT) .waitingFor(Wait.forHttp("/")) .withStartupTimeout(Duration.ofSeconds(120)); + // Basic authentication was introduced in v2.24.0 + if (enableBasicAuth) { + this.dockerContainer + .withCommand("--config.file=/etc/prometheus/prometheus.yml", "--web.config.file=/etc/prometheus/web.yml") + .withCopyFileToContainer(forClasspathResource("web.yml"), "/etc/prometheus/web.yml") + .waitingFor(Wait.forHttp("/").withBasicCredentials(USER, PASSWORD)); + } this.dockerContainer.start(); } diff --git a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusBasicAuth.java b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusBasicAuth.java new file mode 100644 index 000000000000..ce907436d263 --- /dev/null +++ b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusBasicAuth.java @@ -0,0 +1,78 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.prometheus; + +import com.google.common.collect.ImmutableMap; +import io.airlift.units.Duration; +import io.trino.testing.AbstractTestQueryFramework; +import io.trino.testing.QueryRunner; +import org.testng.annotations.Test; + +import static io.trino.plugin.prometheus.MetadataUtil.METRIC_CODEC; +import static io.trino.plugin.prometheus.PrometheusQueryRunner.createPrometheusQueryRunner; +import static io.trino.plugin.prometheus.PrometheusServer.LATEST_VERSION; +import static io.trino.plugin.prometheus.PrometheusServer.PASSWORD; +import static io.trino.plugin.prometheus.PrometheusServer.USER; +import static io.trino.testing.assertions.Assert.assertEventually; +import static io.trino.type.InternalTypeManager.TESTING_TYPE_MANAGER; +import static java.util.concurrent.TimeUnit.MINUTES; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class TestPrometheusBasicAuth + extends AbstractTestQueryFramework +{ + private PrometheusServer server; + + @Override + protected QueryRunner createQueryRunner() + throws Exception + { + server = closeAfterClass(new PrometheusServer(LATEST_VERSION, true)); + return createPrometheusQueryRunner(server, ImmutableMap.of(), ImmutableMap.of("prometheus.auth.user", USER, "prometheus.auth.password", PASSWORD)); + } + + @Test + public void testSelect() + { + assertEventually( + new Duration(1, MINUTES), + new Duration(1, SECONDS), + () -> assertQuery("SHOW TABLES IN prometheus.default LIKE 'up'", "VALUES 'up'")); + + assertQuery("SELECT labels['job'] FROM prometheus.default.up LIMIT 1", "VALUES 'prometheus'"); + } + + @Test + public void testInvalidCredential() + { + PrometheusConnectorConfig config = new PrometheusConnectorConfig(); + config.setPrometheusURI(server.getUri()); + config.setUser("invalid-user"); + config.setPassword("invalid-password"); + PrometheusClient client = new PrometheusClient(config, METRIC_CODEC, TESTING_TYPE_MANAGER); + assertThatThrownBy(() -> client.getTableNames("default")) + .hasMessage("Bad response 401 Unauthorized"); + } + + @Test + public void testMissingCredential() + { + PrometheusConnectorConfig config = new PrometheusConnectorConfig(); + config.setPrometheusURI(server.getUri()); + PrometheusClient client = new PrometheusClient(config, METRIC_CODEC, TESTING_TYPE_MANAGER); + assertThatThrownBy(() -> client.getTableNames("default")) + .hasMessage("Bad response 401 Unauthorized"); + } +} diff --git a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusConnectorConfig.java b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusConnectorConfig.java index 1dce5b9efb81..3dd66661f12d 100644 --- a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusConnectorConfig.java +++ b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusConnectorConfig.java @@ -42,6 +42,8 @@ public void testDefaults() .setMaxQueryRangeDuration(new Duration(21, DAYS)) .setCacheDuration(new Duration(30, SECONDS)) .setBearerTokenFile(null) + .setUser(null) + .setPassword(null) .setReadTimeout(new Duration(10, SECONDS))); } @@ -54,6 +56,8 @@ public void testExplicitPropertyMappings() .put("prometheus.max.query.range.duration", "1095d") .put("prometheus.cache.ttl", "60s") .put("prometheus.bearer.token.file", "/tmp/bearer_token.txt") + .put("prometheus.auth.user", "admin") + .put("prometheus.auth.password", "password") .put("prometheus.read-timeout", "30s") .buildOrThrow(); @@ -64,6 +68,8 @@ public void testExplicitPropertyMappings() expected.setMaxQueryRangeDuration(new Duration(1095, DAYS)); expected.setCacheDuration(new Duration(60, SECONDS)); expected.setBearerTokenFile(new File("/tmp/bearer_token.txt")); + expected.setUser("admin"); + expected.setPassword("password"); expected.setReadTimeout(new Duration(30, SECONDS)); assertFullMapping(properties, expected); @@ -82,4 +88,24 @@ public void testFailOnDurationLessThanQueryChunkConfig() .isInstanceOf(ConfigurationException.class) .hasMessageContaining("prometheus.max.query.range.duration must be greater than prometheus.query.chunk.size.duration"); } + + @Test + public void testInvalidAuth() + { + assertThatThrownBy(new PrometheusConnectorConfig().setBearerTokenFile(new File("/tmp/bearer_token.txt")).setUser("test")::checkConfig) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Either on of bearer token file or basic authentication should be used"); + + assertThatThrownBy(new PrometheusConnectorConfig().setBearerTokenFile(new File("/tmp/bearer_token.txt")).setPassword("test")::checkConfig) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Either on of bearer token file or basic authentication should be used"); + + assertThatThrownBy(new PrometheusConnectorConfig().setUser("test")::checkConfig) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Both username and password must be set when using basic authentication"); + + assertThatThrownBy(new PrometheusConnectorConfig().setPassword("test")::checkConfig) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Both username and password must be set when using basic authentication"); + } } diff --git a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusIntegrationStatus.java b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusIntegrationStatus.java index 3d5c886f8447..96ec110afc2f 100644 --- a/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusIntegrationStatus.java +++ b/plugin/trino-prometheus/src/test/java/io/trino/plugin/prometheus/TestPrometheusIntegrationStatus.java @@ -45,7 +45,7 @@ protected QueryRunner createQueryRunner() throws Exception { this.server = new PrometheusServer(); - return createPrometheusQueryRunner(server, ImmutableMap.of()); + return createPrometheusQueryRunner(server, ImmutableMap.of(), ImmutableMap.of()); } @Test diff --git a/plugin/trino-prometheus/src/test/resources/web.yml b/plugin/trino-prometheus/src/test/resources/web.yml new file mode 100644 index 000000000000..a12e881fb9e0 --- /dev/null +++ b/plugin/trino-prometheus/src/test/resources/web.yml @@ -0,0 +1,3 @@ +# Steps to generate this file: https://prometheus.io/docs/guides/basic-auth/ +basic_auth_users: + admin: $2b$12$4rM8ksUG5lzoe9mcfqlfWO2iD4e4wtWf/a/KEepDcd0nfqI6nsLlG diff --git a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvMultinodeAllConnectors.java b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvMultinodeAllConnectors.java index 576e048126b3..f0ae2e528040 100644 --- a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvMultinodeAllConnectors.java +++ b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvMultinodeAllConnectors.java @@ -87,6 +87,9 @@ public void extendEnvironment(Environment.Builder builder) container.withCopyFileToContainer( forHostPath(configDir.getPath("google-sheets-auth.json")), CONTAINER_PRESTO_ETC + "/catalog/google-sheets-auth.json"); + container.withCopyFileToContainer( + forHostPath(configDir.getPath("prometheus-bearer.txt")), + CONTAINER_PRESTO_ETC + "/catalog/prometheus-bearer.txt"); } }); } diff --git a/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/conf/environment/multinode-all/prometheus-bearer.txt b/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/conf/environment/multinode-all/prometheus-bearer.txt new file mode 100644 index 000000000000..9daeafb9864c --- /dev/null +++ b/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/conf/environment/multinode-all/prometheus-bearer.txt @@ -0,0 +1 @@ +test diff --git a/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/conf/environment/multinode-all/prometheus.properties b/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/conf/environment/multinode-all/prometheus.properties index 1eaf027eeb31..2ae2a5292106 100644 --- a/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/conf/environment/multinode-all/prometheus.properties +++ b/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/conf/environment/multinode-all/prometheus.properties @@ -3,5 +3,5 @@ prometheus.uri=http://host1.invalid:9090 prometheus.query.chunk.size.duration=1d prometheus.max.query.range.duration=21d prometheus.cache.ttl=30s -prometheus.bearer.token.file=/path/to/bearer/token/file +prometheus.bearer.token.file=/docker/presto-product-tests/conf/presto/etc/catalog/prometheus-bearer.txt prometheus.read-timeout=10s