snazy commented on code in PR #469: URL: https://github.com/apache/polaris/pull/469#discussion_r1889227724
########## dropwizard/service/README-quarkus.md: ########## @@ -0,0 +1,98 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you 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. +--> + +This module contains the Polaris Service powered by Quarkus (instead of Dropwizard). + +# Main differences + +* Bean injection (CDI) is made using `@ApplicationScoped` annotation on class and injected in other classes using `@Inject` annotation (https://quarkus.io/guides/cdi-reference). +* Codehale metrics registry and opentelemetry boilerplate (prometheus exporter included) are not needed anymore: Quarkus provides it "out of the box" (https://quarkus.io/guides/opentelemetry) +* `PolarisHealthCheck` is not needed anymore: Quarkus provides it "out of the box" (https://quarkus.io/guides/smallrye-health) +* `TimedApplicationEventListener` and the `@TimedApi` annotation are replaced by Quarkus (micrometer) `@Timed` annotation (https://quarkus.io/guides/telemetry-micrometer) +* `PolarisJsonLayoutFactory` is not needed anymore: Quarkus provides it by configuration (using `quarkus.log.*` configuration) +* `PolarisApplication` is not needed, Quarkus provide a "main" application out of the box (it's possible to provide `QuarkusApplication` for control the startup and also using `@Startup` annotation) +* CORS boilerplate is not needed anymore: Quarkus supports it via configuration (using `quarkus.http.cors.*` configuration) +* CLI is not part of `polaris-service` anymore, we have (will have) a dedicated module (`polaris-cli`) + +# Build and run + +To build `polaris-service` you simply do: + +``` +./gradlew :polaris-service:build +``` + +The build creates ready to run package: +* in the `build/quarkus-app` folder, you can run with `java -jar quarkus-run.jar` Review Comment: For a follow-up, IIRC it's possible to have different names than `quarkus-appl` and `quarkus-run`. ########## dropwizard/service/README-quarkus.md: ########## @@ -0,0 +1,98 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you 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. +--> + +This module contains the Polaris Service powered by Quarkus (instead of Dropwizard). + +# Main differences + +* Bean injection (CDI) is made using `@ApplicationScoped` annotation on class and injected in other classes using `@Inject` annotation (https://quarkus.io/guides/cdi-reference). +* Codehale metrics registry and opentelemetry boilerplate (prometheus exporter included) are not needed anymore: Quarkus provides it "out of the box" (https://quarkus.io/guides/opentelemetry) +* `PolarisHealthCheck` is not needed anymore: Quarkus provides it "out of the box" (https://quarkus.io/guides/smallrye-health) +* `TimedApplicationEventListener` and the `@TimedApi` annotation are replaced by Quarkus (micrometer) `@Timed` annotation (https://quarkus.io/guides/telemetry-micrometer) +* `PolarisJsonLayoutFactory` is not needed anymore: Quarkus provides it by configuration (using `quarkus.log.*` configuration) +* `PolarisApplication` is not needed, Quarkus provide a "main" application out of the box (it's possible to provide `QuarkusApplication` for control the startup and also using `@Startup` annotation) +* CORS boilerplate is not needed anymore: Quarkus supports it via configuration (using `quarkus.http.cors.*` configuration) +* CLI is not part of `polaris-service` anymore, we have (will have) a dedicated module (`polaris-cli`) + +# Build and run + +To build `polaris-service` you simply do: + +``` +./gradlew :polaris-service:build +``` + +The build creates ready to run package: +* in the `build/quarkus-app` folder, you can run with `java -jar quarkus-run.jar` +* the `build/distributions` folder contains tar/zip distributions you can extract + +You can directly run Polaris service (in the build scope) using: + +``` +./gradlew :polaris-service:quarkusRun +``` + +You can run in Dev mode as well: + +``` +./gradlew --console=plain :polaris-service:quarkusDev +``` + +You can directly build a Docker image using: + +``` +./gradlew :polaris-service:imageBuild +``` + +# Configuration + +The main configuration file is not the `application.properties`. The default configuration is +package as part of the `polaris-service`. `polaris-service` uses several configuration sources (in +this order): +* system properties +* environment variables +* `.env` file in the current working directory +* `$PWD/config/application.properties` file +* the `application.properties` packaged in the `polaris-service` application + +It means you can override some configuration property using environment variables for example. + +By default, `polaris-service` uses 8181 as the HTTP port (defined in the `quarkus.http.port` +configuration property) and 8182 as the management port (defined in the `quarkus.management.port` +configuration property). + +You can find more details here: https://quarkus.io/guides/config + +# TODO + +* Modify `CallContext` and remove all usages of `ThreadLocal`, replace with proper context propagation. +* Remove `PolarisCallContext` – it's just an aggregation of CDI beans +* Complete utests/itests in `polaris-service` +* Use `@QuarkustIntegrationTest` for integration tests +* Remove `OAuthCredentialAuthFilter` and replace with Quarkus OIDC security +* Create `polaris-cli` module, add Bootstrap and Purge commands +* Adapt Helm charts, Dockerfiles, K8s examples +* Fix intermittent Gradle build error : SRCFG00011: Could not expand value + platform.quarkus.native.builder-image in property quarkus.native.builder-image + (https://github.com/quarkusio/quarkus/issues/19139) – due to Spark integration tests and their + dependencies. + +* Update documentation/README/... + +* Do we want to support existing json configuration file as configuration source ? Review Comment: I'd say no. It's not common in Quarkus and not "IDE friendly" (IJ has auto-completion and code-navigation for application.properties). ########## dropwizard/service/src/main/resources/application.properties: ########## @@ -0,0 +1,158 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. +# + +quarkus.application.name=Polaris +quarkus.banner.path=/org/apache/polaris/service/banner.txt + +quarkus.config.mapping.validate-unknown=true + +quarkus.container-image.build=false +quarkus.container-image.push=false +# quarkus.container-image.registry=ghcr.io +quarkus.container-image.group=apache +quarkus.container-image.name=polaris +# quarkus.container-image.tag=latest + +# multi-architecture builds require pushing to a registry +# quarkus.docker.buildx.platform=linux/amd64,linux/arm64 + +quarkus.http.auth.basic=false +quarkus.http.access-log.enabled=true +# quarkus.http.access-log.pattern=common +quarkus.http.enable-compression=true +quarkus.http.enable-decompression=true +quarkus.http.body.handle-file-uploads=false +quarkus.http.limits.max-body-size=10240K +quarkus.http.compress-media-types=application/json,text/html,text/plain + +quarkus.http.cors.origins=http://localhost:8080 +quarkus.http.cors.methods=PATCH, POST, DELETE, GET, PUT +quarkus.http.cors.headers=* +quarkus.http.cors.exposed-headers=* +quarkus.http.cors.access-control-max-age=PT10M +quarkus.http.cors.access-control-allow-credentials=true + +quarkus.http.port=8181 +quarkus.http.test-port=0 + +quarkus.log.level=INFO +quarkus.log.console.enable=true +quarkus.log.console.level=ALL +quarkus.log.console.json=false +quarkus.log.console.format=%d{yyyy-MM-dd HH:mm:ss,SSS} %-5p [%c{3.}] [%X{requestId},%X{realmId}] [%X{traceId},%X{parentId},%X{spanId},%X{sampled}] (%t) %s%e%n +quarkus.log.file.enable=true +quarkus.log.file.level=ALL +quarkus.log.file.json=false +quarkus.log.file.format=%d{yyyy-MM-dd HH:mm:ss,SSS} %-5p [%c{3.}] [%X{requestId},%X{realmId}] [%X{traceId},%X{parentId},%X{spanId},%X{sampled}] (%t) %s%e%n +quarkus.log.file.path=./logs/polaris.log +quarkus.log.file.rotation.file-suffix=.yyyy-MM-dd.gz +quarkus.log.file.rotation.max-file-size=10M +quarkus.log.file.rotation.max-backup-index=14 +quarkus.log.category."org.apache.polaris".level=INFO +quarkus.log.category."org.apache.iceberg.rest".level=INFO +quarkus.log.category."io.smallrye.config".level=INFO + +quarkus.management.enabled=true +quarkus.management.port=8182 +quarkus.management.test-port=0 + +quarkus.micrometer.enabled=true +quarkus.micrometer.export.prometheus.enabled=true + +quarkus.otel.enabled=true +quarkus.otel.sdk.disabled=false +# quarkus.otel.exporter.otlp.endpoint=http://otlp-collector:4317 +# quarkus.otel.resource.attributes=deployment.environment=dev +# quarkus.otel.service.name=polaris +# quarkus.otel.traces.sampler=parentbased_always_on +# quarkus.otel.traces.sampler.arg=1.0d + +polaris.context.realm-context-resolver.default-realm=default-realm +polaris.context.realm-context-resolver.type=default + +polaris.config.defaults.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING=false +polaris.config.defaults.SUPPORTED_CATALOG_STORAGE_TYPES=["S3","GCS","AZURE","FILE"] +# realm overrides +# polaris.config.realm-overrides."my-realm".INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST=true +# polaris.config.realm-overrides."my-realm".SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION=true + +polaris.persistence.metastore-manager.type=in-memory + +polaris.io.file-io-factory.type=default + +polaris.log.request-id-header-name=request_id +# polaris.log.mdc.aid=polaris +# polaris.log.mdc.sid=polaris-service + +polaris.metrics.tags.application=Polaris +# polaris.metrics.tags.service=polaris +# polaris.metrics.tags.environment=prod +# polaris.metrics.tags.region=us-west-2 + +# polaris.otel.span-attributes.service=polaris +# polaris.otel.span-attributes.environment=prod +# polaris.otel.span-attributes.region=us-west-2 + +# polaris.tasks.max-concurrent-tasks=100 +# polaris.tasks.max-queued-tasks=1000 + +polaris.rate-limiter.type=default +polaris.rate-limiter.token-bucket.type=default +polaris.rate-limiter.token-bucket.requests-per-second=9999 +polaris.rate-limiter.token-bucket.window=PT10S Review Comment: IIRC there was a native Quarkus extension for this - but not sure about the state/quality of it. ########## gradle/libs.versions.toml: ########## @@ -20,11 +20,13 @@ [versions] hadoop = "3.4.0" iceberg = "1.6.1" -dropwizard = "4.0.8" +junit = "5.10.3" +quarkus = "3.17.0" Review Comment: ```suggestion quarkus = "3.17.4" ``` ########## dropwizard/service/build.gradle.kts: ########## @@ -109,28 +95,41 @@ dependencies { exclude("org.apache.logging.log4j", "log4j-slf4j2-impl") exclude("org.apache.logging.log4j", "log4j-api") exclude("org.apache.logging.log4j", "log4j-1.2-api") + exclude("org.slf4j", "jul-to-slf4j") } - testImplementation(platform(libs.awssdk.bom)) testImplementation("software.amazon.awssdk:glue") testImplementation("software.amazon.awssdk:kms") testImplementation("software.amazon.awssdk:dynamodb") - testImplementation(libs.auth0.jwt) + testImplementation(platform(libs.junit.bom)) + testImplementation(libs.bundles.junit.testing) - testCompileOnly(libs.smallrye.common.annotation) + testImplementation(platform(libs.quarkus.bom)) + testImplementation("io.quarkus:quarkus-junit5") + testImplementation("io.quarkus:quarkus-junit5-mockito") + testImplementation("io.quarkus:quarkus-rest-client") + testImplementation("io.quarkus:quarkus-rest-client-jackson") + testImplementation("io.rest-assured:rest-assured") - testImplementation(platform(libs.junit.bom)) - testImplementation("org.junit.jupiter:junit-jupiter") - testImplementation(libs.assertj.core) - testImplementation(libs.mockito.core) - testRuntimeOnly("org.junit.platform:junit-platform-launcher") + testImplementation(platform(libs.testcontainers.bom)) + testImplementation("org.testcontainers:testcontainers") + testImplementation(libs.s3mock.testcontainers) - testImplementation(project(":polaris-eclipselink")) + // required for PolarisSparkIntegrationTest Review Comment: Agree, those tests should be moved to a different Gradle project in a follow-up. We can likely port `nessie-apprunner` - maybe it's possible to already use nessie-apprunner w/ some configuration. ########## dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/config/QuarkusJacksonConfig.java: ########## @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.polaris.service.dropwizard.config; + +import com.fasterxml.jackson.annotation.JsonAutoDetect; +import com.fasterxml.jackson.annotation.PropertyAccessor; +import com.fasterxml.jackson.core.StreamReadConstraints; +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.PropertyNamingStrategies; +import io.quarkus.jackson.ObjectMapperCustomizer; +import io.quarkus.runtime.configuration.MemorySize; +import io.quarkus.runtime.configuration.MemorySizeConverter; +import io.smallrye.config.WithConverter; +import jakarta.inject.Inject; +import jakarta.inject.Singleton; +import org.apache.iceberg.rest.RESTSerializers; +import org.apache.polaris.service.config.Serializers; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Singleton +public class QuarkusJacksonConfig implements ObjectMapperCustomizer { + + private static final Logger LOGGER = LoggerFactory.getLogger(QuarkusJacksonConfig.class); + + private final long maxBodySize; + + @Inject + public QuarkusJacksonConfig( + @ConfigProperty(name = "quarkus.http.limits.max-body-size") + @WithConverter(MemorySizeConverter.class) + MemorySize maxBodySize) { + this.maxBodySize = maxBodySize.asLongValue(); + } + + @Override + public void customize(ObjectMapper objectMapper) { + objectMapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY); + objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + objectMapper.setPropertyNamingStrategy(new PropertyNamingStrategies.KebabCaseStrategy()); + RESTSerializers.registerAll(objectMapper); + Serializers.registerSerializers(objectMapper); + objectMapper + .getFactory() + .setStreamReadConstraints( + StreamReadConstraints.builder().maxDocumentLength(maxBodySize).build()); Review Comment: I think this is no longer needed w/ Quarkus ########## dropwizard/service/README-quarkus.md: ########## @@ -0,0 +1,98 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you 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. +--> + +This module contains the Polaris Service powered by Quarkus (instead of Dropwizard). + +# Main differences + +* Bean injection (CDI) is made using `@ApplicationScoped` annotation on class and injected in other classes using `@Inject` annotation (https://quarkus.io/guides/cdi-reference). +* Codehale metrics registry and opentelemetry boilerplate (prometheus exporter included) are not needed anymore: Quarkus provides it "out of the box" (https://quarkus.io/guides/opentelemetry) +* `PolarisHealthCheck` is not needed anymore: Quarkus provides it "out of the box" (https://quarkus.io/guides/smallrye-health) +* `TimedApplicationEventListener` and the `@TimedApi` annotation are replaced by Quarkus (micrometer) `@Timed` annotation (https://quarkus.io/guides/telemetry-micrometer) +* `PolarisJsonLayoutFactory` is not needed anymore: Quarkus provides it by configuration (using `quarkus.log.*` configuration) +* `PolarisApplication` is not needed, Quarkus provide a "main" application out of the box (it's possible to provide `QuarkusApplication` for control the startup and also using `@Startup` annotation) +* CORS boilerplate is not needed anymore: Quarkus supports it via configuration (using `quarkus.http.cors.*` configuration) +* CLI is not part of `polaris-service` anymore, we have (will have) a dedicated module (`polaris-cli`) + +# Build and run + +To build `polaris-service` you simply do: + +``` +./gradlew :polaris-service:build +``` + +The build creates ready to run package: +* in the `build/quarkus-app` folder, you can run with `java -jar quarkus-run.jar` +* the `build/distributions` folder contains tar/zip distributions you can extract + +You can directly run Polaris service (in the build scope) using: + +``` +./gradlew :polaris-service:quarkusRun +``` + +You can run in Dev mode as well: + +``` +./gradlew --console=plain :polaris-service:quarkusDev +``` + +You can directly build a Docker image using: + +``` +./gradlew :polaris-service:imageBuild Review Comment: Maybe we should remove this paragraph, thinking of adding multiplatform Docker images in a follow-up. ########## dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/metrics/QuarkusMeterFilterProducer.java: ########## @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.polaris.service.dropwizard.metrics; + +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.config.MeterFilter; +import jakarta.enterprise.inject.Produces; +import jakarta.inject.Inject; +import jakarta.inject.Singleton; +import java.util.Map; +import java.util.stream.Collectors; +import org.eclipse.microprofile.config.inject.ConfigProperty; + +public class QuarkusMeterFilterProducer { + + @Inject + @ConfigProperty(name = "polaris.metrics.tags") Review Comment: Huh? I thought there's a Quarkus config for this. ########## dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/config/QuarkusProducers.java: ########## @@ -0,0 +1,200 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.polaris.service.dropwizard.config; + +import io.smallrye.common.annotation.Identifier; +import io.smallrye.context.SmallRyeManagedExecutor; +import io.vertx.core.http.HttpServerRequest; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.context.RequestScoped; +import jakarta.enterprise.inject.Any; +import jakarta.enterprise.inject.Disposes; +import jakarta.enterprise.inject.Instance; +import jakarta.enterprise.inject.Produces; +import jakarta.inject.Singleton; +import jakarta.ws.rs.core.Context; +import java.time.Clock; +import java.util.HashMap; +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.PolarisConfigurationStore; +import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; +import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.auth.PolarisAuthorizerImpl; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.context.RealmContext; +import org.apache.polaris.core.persistence.MetaStoreManagerFactory; +import org.apache.polaris.core.persistence.PolarisMetaStoreSession; +import org.apache.polaris.core.storage.cache.StorageCredentialCache; +import org.apache.polaris.service.auth.Authenticator; +import org.apache.polaris.service.auth.TokenBrokerFactory; +import org.apache.polaris.service.catalog.api.IcebergRestOAuth2ApiService; +import org.apache.polaris.service.catalog.io.FileIOFactory; +import org.apache.polaris.service.context.RealmContextResolver; +import org.apache.polaris.service.ratelimiter.RateLimiter; +import org.apache.polaris.service.ratelimiter.TokenBucketFactory; +import org.apache.polaris.service.task.TaskHandlerConfiguration; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.eclipse.microprofile.context.ManagedExecutor; +import org.eclipse.microprofile.context.ThreadContext; + +public class QuarkusProducers { + + @Produces + @ApplicationScoped // cannot be singleton because it is mocked in tests + public Clock clock() { + return Clock.systemDefaultZone(); Review Comment: Hm - not systemUTC? ########## dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/QuarkusTokenBrokerFactoryConfiguration.java: ########## @@ -16,12 +16,10 @@ * specific language governing permissions and limitations * under the License. */ -[org.apache.polaris.service.dropwizard.catalog.io.TestFileIOFactory]S -contract={org.apache.polaris.service.catalog.io.FileIOFactory} -name=test -qualifier={io.smallrye.common.annotation.Identifier} +package org.apache.polaris.service.dropwizard.auth; -[org.apache.polaris.service.dropwizard.ratelimiter.MockTokenBucketFactory]S -contract={org.apache.polaris.service.ratelimiter.TokenBucketFactory} -name=mock -qualifier={io.smallrye.common.annotation.Identifier} +import io.smallrye.config.ConfigMapping; +import org.apache.polaris.service.auth.TokenBrokerFactoryConfiguration; + +@ConfigMapping(prefix = "polaris.authentication.token-broker-factory") Review Comment: I guess it's safe to add the annotation directly on `TokenBrokerFactoryConfiguration`. ########## dropwizard/service/build.gradle.kts: ########## @@ -17,13 +17,11 @@ * under the License. */ -import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar - plugins { + alias(libs.plugins.quarkus) Review Comment: I think, the `dropwizard` directory needs to be renamed ########## dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/QuarkusOAuthFilter.java: ########## @@ -0,0 +1,159 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.polaris.service.dropwizard.auth; + +import jakarta.inject.Inject; +import jakarta.ws.rs.NotAuthorizedException; +import jakarta.ws.rs.Priorities; +import jakarta.ws.rs.container.ContainerRequestContext; +import jakarta.ws.rs.core.HttpHeaders; +import jakarta.ws.rs.core.SecurityContext; +import java.security.Principal; +import java.util.Optional; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.service.auth.Authenticator; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.jboss.resteasy.reactive.server.ServerRequestFilter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +// TODO replace with OIDC Review Comment: ```suggestion // TODO replace with all authN with Quarkus native authN ``` ;) ########## dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/logging/QuarkusLoggingConfiguration.java: ########## @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.polaris.service.dropwizard.logging; + +import io.smallrye.config.ConfigMapping; +import java.util.Map; + +@ConfigMapping(prefix = "polaris.log") Review Comment: Remind me next year that we should auto-generate config-docs in this project as well ;) ########## dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/logging/QuarkusLoggingMDCFilter.java: ########## @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.polaris.service.dropwizard.logging; + +import io.quarkus.vertx.web.RouteFilter; +import io.vertx.ext.web.RoutingContext; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import org.apache.polaris.core.context.RealmContext; +import org.slf4j.MDC; + +@ApplicationScoped +public class QuarkusLoggingMDCFilter { + + public static final int PRIORITY = RouteFilter.DEFAULT_PRIORITY + 100; + + private static final String REQUEST_ID_KEY = "requestId"; + private static final String REALM_ID_KEY = "realmId"; + + @Inject RealmContext realmContext; + + @Inject QuarkusLoggingConfiguration loggingConfiguration; + + public static String requestId(RoutingContext rc) { + return rc.get(REQUEST_ID_KEY); + } + + public static String realmId(RoutingContext rc) { + return rc.get(REALM_ID_KEY); + } + + @RouteFilter(value = PRIORITY) + public void applyMDCContext(RoutingContext rc) { + // The request scope is active here, so any MDC values set here will be propagated to + // threads handling the request. + // Also put the MDC values in the request context for use by other filters and handlers + loggingConfiguration.mdc().forEach(MDC::put); + loggingConfiguration.mdc().forEach(rc::put); + var requestId = rc.request().getHeader(loggingConfiguration.requestIdHeaderName()); + if (requestId != null) { + MDC.put(REQUEST_ID_KEY, requestId); + rc.put(REQUEST_ID_KEY, requestId); + } + MDC.put(REALM_ID_KEY, realmContext.getRealmIdentifier()); + rc.put(REALM_ID_KEY, realmContext.getRealmIdentifier()); + // Do not explicitly remove the MDC values from the request context with an end handler, + // as this could remove MDC context still in use in TaskExecutor threads Review Comment: Hm. `MDC` seems to use `ThreadLocal`s. Maybe worth to clear all MDC values at the beginning of this function? ########## dropwizard/service/README-quarkus.md: ########## @@ -0,0 +1,98 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you 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. +--> + +This module contains the Polaris Service powered by Quarkus (instead of Dropwizard). + +# Main differences + +* Bean injection (CDI) is made using `@ApplicationScoped` annotation on class and injected in other classes using `@Inject` annotation (https://quarkus.io/guides/cdi-reference). +* Codehale metrics registry and opentelemetry boilerplate (prometheus exporter included) are not needed anymore: Quarkus provides it "out of the box" (https://quarkus.io/guides/opentelemetry) +* `PolarisHealthCheck` is not needed anymore: Quarkus provides it "out of the box" (https://quarkus.io/guides/smallrye-health) +* `TimedApplicationEventListener` and the `@TimedApi` annotation are replaced by Quarkus (micrometer) `@Timed` annotation (https://quarkus.io/guides/telemetry-micrometer) +* `PolarisJsonLayoutFactory` is not needed anymore: Quarkus provides it by configuration (using `quarkus.log.*` configuration) +* `PolarisApplication` is not needed, Quarkus provide a "main" application out of the box (it's possible to provide `QuarkusApplication` for control the startup and also using `@Startup` annotation) +* CORS boilerplate is not needed anymore: Quarkus supports it via configuration (using `quarkus.http.cors.*` configuration) +* CLI is not part of `polaris-service` anymore, we have (will have) a dedicated module (`polaris-cli`) + +# Build and run + +To build `polaris-service` you simply do: + +``` +./gradlew :polaris-service:build +``` + +The build creates ready to run package: +* in the `build/quarkus-app` folder, you can run with `java -jar quarkus-run.jar` +* the `build/distributions` folder contains tar/zip distributions you can extract + +You can directly run Polaris service (in the build scope) using: + +``` +./gradlew :polaris-service:quarkusRun +``` + +You can run in Dev mode as well: + +``` +./gradlew --console=plain :polaris-service:quarkusDev +``` + +You can directly build a Docker image using: + +``` +./gradlew :polaris-service:imageBuild +``` + +# Configuration + +The main configuration file is not the `application.properties`. The default configuration is +package as part of the `polaris-service`. `polaris-service` uses several configuration sources (in +this order): +* system properties +* environment variables +* `.env` file in the current working directory +* `$PWD/config/application.properties` file +* the `application.properties` packaged in the `polaris-service` application + +It means you can override some configuration property using environment variables for example. + +By default, `polaris-service` uses 8181 as the HTTP port (defined in the `quarkus.http.port` +configuration property) and 8182 as the management port (defined in the `quarkus.management.port` +configuration property). + +You can find more details here: https://quarkus.io/guides/config + +# TODO + +* Modify `CallContext` and remove all usages of `ThreadLocal`, replace with proper context propagation. +* Remove `PolarisCallContext` – it's just an aggregation of CDI beans +* Complete utests/itests in `polaris-service` +* Use `@QuarkustIntegrationTest` for integration tests +* Remove `OAuthCredentialAuthFilter` and replace with Quarkus OIDC security +* Create `polaris-cli` module, add Bootstrap and Purge commands +* Adapt Helm charts, Dockerfiles, K8s examples +* Fix intermittent Gradle build error : SRCFG00011: Could not expand value + platform.quarkus.native.builder-image in property quarkus.native.builder-image + (https://github.com/quarkusio/quarkus/issues/19139) – due to Spark integration tests and their + dependencies. Review Comment: ```suggestion (https://github.com/quarkusio/quarkus/issues/19139). The Quarkus issue says that using the Quarkus-platform bom helps (it's really what we should depend on). But IIRC there were some dependency issues with Spark/Scala, which prevents us from using the Quarkus-platform bom. ``` ########## dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/storage/QuarkusStorageConfiguration.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.polaris.service.dropwizard.storage; + +import io.smallrye.config.ConfigMapping; +import io.smallrye.config.WithDefault; +import io.smallrye.config.WithName; +import java.time.Duration; +import java.util.*; +import org.apache.polaris.service.storage.StorageConfiguration; + +@ConfigMapping(prefix = "polaris.storage") +public interface QuarkusStorageConfiguration extends StorageConfiguration { + + @WithName("aws.awsAccessKey") Review Comment: Should we kind-of prepare for secrets managers in a follow-up to have the config keys ready? ########## dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/metrics/QuarkusValueExpressionResolver.java: ########## @@ -0,0 +1,38 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.polaris.service.dropwizard.metrics; + +import io.micrometer.common.annotation.ValueExpressionResolver; +import io.micrometer.common.lang.Nullable; +import jakarta.annotation.Nonnull; +import jakarta.enterprise.context.ApplicationScoped; +import org.apache.polaris.core.context.RealmContext; + +@ApplicationScoped +public class QuarkusValueExpressionResolver implements ValueExpressionResolver { + + @Override + public String resolve(@Nonnull String expression, @Nullable Object parameter) { + // TODO maybe replace with CEL of some expression engine and make this more generic Review Comment: :) ########## dropwizard/service/src/main/resources/application.properties: ########## @@ -0,0 +1,158 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. +# + +quarkus.application.name=Polaris Review Comment: ```suggestion quarkus.application.name=Apache Polaris (incubating) ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
