You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "flyrain (via GitHub)" <gi...@apache.org> on 2026/06/26 21:51:12 UTC
[PR] Add readiness check for reflection-free serializers [polaris]
flyrain opened a new pull request, #4913:
URL: https://github.com/apache/polaris/pull/4913
## Why
Polaris should fail production readiness when Quarkus REST Jackson reflection-free serializers are enabled. Polaris REST JSON handling relies on registered `ObjectMapper` customizations for Iceberg request and response models, so allowing generated reflection-free serializers in production can bypass behavior Polaris depends on.
This follows the Jackson 3 / reflection-free serializer readiness discussion. The default runtime configuration already sets the Quarkus option to `false`; this PR adds a production-readiness guard so an explicit production override to `true` is caught before serving traffic.
## What changed
- Added a severe production readiness check for `quarkus.rest.jackson.optimization.enable-reflection-free-serializers=true`.
- Added unit coverage for unset, `false`, and `true` config values.
## Verification
Passed:
- `./gradlew :polaris-runtime-service:test --tests "org.apache.polaris.service.config.ProductionReadinessChecksTest"`
- `./gradlew format compileAll`
- `git diff --check`
- `./gradlew :polaris-runtime-service:check -x :polaris-runtime-service:test -x :polaris-runtime-service:intTest -x :polaris-runtime-service:cloudTest`
Attempted after Docker was enabled:
- `./gradlew :polaris-runtime-service:check`
That full module check still failed in existing integration resource setup for:
- `RestCatalogKeycloakFileIT`
- `RestCatalogMinIOSpecialIT`
- `RestCatalogRustFSSpecialIT`
A targeted rerun of those three integration tests failed the same way. The reports did not reference the new reflection-free serializer readiness check.
## Checklist
- [x] 🛡️ Don't disclose security issues! Security issues should be reported privately via security@apache.org.
- [x] 🔗 Clearly explained why these changes are needed. Related dev-list discussion; no GitHub issue linked.
- [x] 🧪 Added/updated tests, or explained why tests are not needed.
- [x] 💡 Added comments for complex logic, or no complex logic was added.
- [x] 🧾 Updated `CHANGELOG.md` if the change affects users. Not needed: this guards existing production configuration behavior.
- [x] 📚 Updated documentation for user-facing behavior/configuration, or explained why not needed. Not needed: the default property is already documented in runtime defaults.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add readiness check for reflection-free serializers [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain merged PR #4913:
URL: https://github.com/apache/polaris/pull/4913
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add readiness check for reflection-free serializers [polaris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4913:
URL: https://github.com/apache/polaris/pull/4913#discussion_r3484312074
##########
runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java:
##########
@@ -221,6 +224,18 @@ public ProductionReadinessCheck checkRealmResolver(Config config, RealmContextRe
return ProductionReadinessCheck.OK;
}
+ @Produces
+ public ProductionReadinessCheck checkReflectionFreeSerializers(Config config) {
+ ConfigValue configValue = config.getConfigValue(REFLECTION_FREE_SERIALIZERS_PROPERTY);
+ if (Boolean.parseBoolean(configValue.getValue())) {
+ return ProductionReadinessCheck.of(
+ Error.ofSevere(
+ "Quarkus REST Jackson reflection-free serializers must be disabled because Polaris REST JSON handling relies on registered ObjectMapper customizations for Iceberg request and response models.",
+ REFLECTION_FREE_SERIALIZERS_PROPERTY));
Review Comment:
This adds a new *severe* startup/readiness failure tied to the Quarkus config key `quarkus.rest.jackson.optimization.enable-reflection-free-serializers`. I couldn’t find any mention of this constraint in the production configuration docs, so operators who flip the Quarkus optimization may be surprised by the readiness failure. Please check whether the production configuration documentation (e.g., `site/content/in-dev/unreleased/configuration/configuring-polaris-for-production/_index.md`) should mention that this Quarkus option must remain disabled when running Polaris.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add readiness check for reflection-free serializers [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4913:
URL: https://github.com/apache/polaris/pull/4913#discussion_r3493481537
##########
runtime/service/src/test/java/org/apache/polaris/service/config/ProductionReadinessChecksTest.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.config;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.apache.polaris.core.config.ProductionReadinessCheck;
+import org.eclipse.microprofile.config.Config;
+import org.eclipse.microprofile.config.ConfigValue;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class ProductionReadinessChecksTest {
+
+ private static final String REFLECTION_FREE_SERIALIZERS_PROPERTY =
+ "quarkus.rest.jackson.optimization.enable-reflection-free-serializers";
+
+ private ProductionReadinessChecks checks;
+
+ @BeforeEach
+ void setUp() {
+ checks = new ProductionReadinessChecks();
+ }
+
+ @Test
+ void reflectionFreeSerializersDisabledReturnsOk() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers("false"));
+
+ assertThat(result.ready()).isTrue();
+ }
+
+ @Test
+ void reflectionFreeSerializersUnsetReturnsOk() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers(null));
+
+ assertThat(result.ready()).isTrue();
+ }
+
+ @Test
+ void reflectionFreeSerializersEnabledReturnsSevereError() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers("true"));
+
+ assertThat(result.ready()).isFalse();
+ assertThat(result.getErrors())
+ .singleElement()
+ .satisfies(
+ error -> {
+ assertThat(error.offendingProperty()).isEqualTo(REFLECTION_FREE_SERIALIZERS_PROPERTY);
Review Comment:
I'm fine with leaving it.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add readiness check for reflection-free serializers [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4913:
URL: https://github.com/apache/polaris/pull/4913#discussion_r3492678932
##########
runtime/service/src/test/java/org/apache/polaris/service/config/ProductionReadinessChecksTest.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.config;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.apache.polaris.core.config.ProductionReadinessCheck;
+import org.eclipse.microprofile.config.Config;
+import org.eclipse.microprofile.config.ConfigValue;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class ProductionReadinessChecksTest {
+
+ private static final String REFLECTION_FREE_SERIALIZERS_PROPERTY =
+ "quarkus.rest.jackson.optimization.enable-reflection-free-serializers";
+
+ private ProductionReadinessChecks checks;
+
+ @BeforeEach
+ void setUp() {
+ checks = new ProductionReadinessChecks();
+ }
+
+ @Test
+ void reflectionFreeSerializersDisabledReturnsOk() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers("false"));
+
+ assertThat(result.ready()).isTrue();
+ }
+
+ @Test
+ void reflectionFreeSerializersUnsetReturnsOk() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers(null));
+
+ assertThat(result.ready()).isTrue();
+ }
+
+ @Test
+ void reflectionFreeSerializersEnabledReturnsSevereError() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers("true"));
+
+ assertThat(result.ready()).isFalse();
+ assertThat(result.getErrors())
+ .singleElement()
+ .satisfies(
+ error -> {
+ assertThat(error.offendingProperty()).isEqualTo(REFLECTION_FREE_SERIALIZERS_PROPERTY);
Review Comment:
While the test may not capture runtime behavior, it still provides valuable coverage for the new readiness check. I vote for leaving the test as is.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add readiness check for reflection-free serializers [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4913:
URL: https://github.com/apache/polaris/pull/4913#discussion_r3484641853
##########
runtime/service/src/test/java/org/apache/polaris/service/config/ProductionReadinessChecksTest.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.config;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.apache.polaris.core.config.ProductionReadinessCheck;
+import org.eclipse.microprofile.config.Config;
+import org.eclipse.microprofile.config.ConfigValue;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class ProductionReadinessChecksTest {
+
+ private static final String REFLECTION_FREE_SERIALIZERS_PROPERTY =
+ "quarkus.rest.jackson.optimization.enable-reflection-free-serializers";
+
+ private ProductionReadinessChecks checks;
+
+ @BeforeEach
+ void setUp() {
+ checks = new ProductionReadinessChecks();
+ }
+
+ @Test
+ void reflectionFreeSerializersDisabledReturnsOk() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers("false"));
+
+ assertThat(result.ready()).isTrue();
+ }
+
+ @Test
+ void reflectionFreeSerializersUnsetReturnsOk() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers(null));
+
+ assertThat(result.ready()).isTrue();
+ }
+
+ @Test
+ void reflectionFreeSerializersEnabledReturnsSevereError() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers("true"));
+
+ assertThat(result.ready()).isFalse();
+ assertThat(result.getErrors())
+ .singleElement()
+ .satisfies(
+ error -> {
+ assertThat(error.offendingProperty()).isEqualTo(REFLECTION_FREE_SERIALIZERS_PROPERTY);
Review Comment:
nit: this test is a bit of an overkill IMHO. The condition under test is very simple. Plus mocked `Config` does not yield 100% guarantees that the readiness check actually captures runtime Quarkus behaviour.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add readiness check for reflection-free serializers [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4913:
URL: https://github.com/apache/polaris/pull/4913#discussion_r3486690704
##########
runtime/service/src/test/java/org/apache/polaris/service/config/ProductionReadinessChecksTest.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.config;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.apache.polaris.core.config.ProductionReadinessCheck;
+import org.eclipse.microprofile.config.Config;
+import org.eclipse.microprofile.config.ConfigValue;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class ProductionReadinessChecksTest {
+
+ private static final String REFLECTION_FREE_SERIALIZERS_PROPERTY =
+ "quarkus.rest.jackson.optimization.enable-reflection-free-serializers";
+
+ private ProductionReadinessChecks checks;
+
+ @BeforeEach
+ void setUp() {
+ checks = new ProductionReadinessChecks();
+ }
+
+ @Test
+ void reflectionFreeSerializersDisabledReturnsOk() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers("false"));
+
+ assertThat(result.ready()).isTrue();
+ }
+
+ @Test
+ void reflectionFreeSerializersUnsetReturnsOk() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers(null));
+
+ assertThat(result.ready()).isTrue();
+ }
+
+ @Test
+ void reflectionFreeSerializersEnabledReturnsSevereError() {
+ ProductionReadinessCheck result =
+ checks.checkReflectionFreeSerializers(configWithReflectionFreeSerializers("true"));
+
+ assertThat(result.ready()).isFalse();
+ assertThat(result.getErrors())
+ .singleElement()
+ .satisfies(
+ error -> {
+ assertThat(error.offendingProperty()).isEqualTo(REFLECTION_FREE_SERIALIZERS_PROPERTY);
Review Comment:
Should we just delete this test?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org