Do not warn about property change when the property is mapped in build and runtime #42904
Conversation
|
I'm not sure TBH. I remember that we discussed whether to allow this, but we didn't come to a conclusion. I recall being against it, but that might just be my reactive nature rearing up. The issue would be that if I set a property to |
|
Yes, we talked about this without a final conclusion. It is not something I'm particularly fond of, but there are a few use cases that we either handle it like this, or we have to figure out something else: I was recently rewriting the logging configuration to use mappings and it was just full of hacks to work around this:
The other case is DevServices, where we have to check for specific configurations that is marked for runtime during the build. In reality, nothing stops you from querying a runtime property during build time programmatically, which allows you to skip all the validations we have in place for the config objects. However, it feels like it needs to be more consistent. We can certainly enforce the same policies for the programmatic API. We should at least discuss whether there is a better way to handle the Dev Services use case. |
Status for workflow
|
| Status | Name | Step | Failures | Logs | Raw logs | Build scan |
|---|---|---|---|---|---|---|
| ✖ | JVM Tests - JDK 17 | Build |
Failures | Logs | Raw logs | 🔍 |
| ✖ | JVM Tests - JDK 21 | Build |
Failures | Logs | Raw logs | 🔍 |
| ✖ | JVM Tests - JDK 17 Windows | Build |
Failures | Logs | Raw logs | 🔍 |
Full information is available in the Build summary check run.
You can consult the Develocity build scans.
Failures
⚙️ JVM Tests - JDK 17 #
- Failing: integration-tests/grpc-mutual-auth
📦 integration-tests/grpc-mutual-auth
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsEndpointTest.testRolesHelloWorldServiceUsingBlockingStub line 42 - History - More details - Source on GitHub
org.opentest4j.AssertionFailedError:
expected: "Hello neo from CN=testclient,O=Default Company Ltd,L=Default City,C=XX"
but was: "{"details":"Error id 9aa33dd1-4adc-4add-9a58-238ad819a22f-2, io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException","stack":"io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException\n\tat io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:268)\n\tat io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:249)\n\tat io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:167)\n\tat examples.GreeterGrpc$GreeterBlockingStub.sayHelloRoleUser(GreeterGrpc.java:224)\n\tat io.quarkus.grpc.examples.hello.HelloWorldTlsEndpoint.userRoleHelloBlocking(HelloWorldTlsEndpoint.java:52)\n\tat io.quarkus.grpc.examples.hello.HelloWorldTlsEndpoint$quarkusrestinvoker$userRoleHelloBlocking_c3f3bc53fca5e4f80b9ba2e1f1057bfb6c9d4f47.invoke(Unknown Source)\n\tat org.jboss.resteasy.reactive.server.handlers....
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsEndpointTest.testRolesHelloWorldServiceUsingMutinyStub line 65 - History - More details - Source on GitHub
org.opentest4j.AssertionFailedError:
expected: "Hello neo-mutiny from CN=testclient,O=Default Company Ltd,L=Default City,C=XX"
but was: "{"details":"Error id 9aa33dd1-4adc-4add-9a58-238ad819a22f-1, io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException","stack":"io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException\n\tat io.grpc.Status.asRuntimeException(Status.java:533)\n\tat io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onClose(ClientCalls.java:481)\n\tat io.grpc.PartialForwardingClientCallListener.onClose(PartialForwardingClientCallListener.java:39)\n\tat io.grpc.ForwardingClientCallListener.onClose(ForwardingClientCallListener.java:23)\n\tat io.grpc.ForwardingClientCallListener$SimpleForwardingClientCallListener.onClose(ForwardingClientCallListener.java:40)\n\tat io.quarkus.grpc.runtime.supports.IOThreadClientInterceptor$1$1.lambda$onClose$3(IOThreadClientInterceptor.java:70)\n\tat io.vertx.core.impl.Cont...
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsServiceTest.testRolesHelloWorldServiceUsingBlockingStub line 53 - History - More details - Source on GitHub
io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException
at io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:268)
at io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:249)
at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:167)
at examples.GreeterGrpc$GreeterBlockingStub.sayHelloRoleUser(GreeterGrpc.java:224)
at io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsServiceTest.testRolesHelloWorldServiceUsingBlockingStub(VertxHelloWorldMutualTlsServiceTest.java:53)
at java.base/java.lang.reflect.Method.invoke(Method.java:569)
at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:972)
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsServiceTest.testRolesHelloWorldServiceUsingMutinyStub - History - More details - Source on GitHub
io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException
at io.grpc.Status.asRuntimeException(Status.java:533)
at io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onClose(ClientCalls.java:481)
at io.vertx.grpc.client.VertxClientCall.lambda$doClose$4(VertxClientCall.java:138)
at io.vertx.grpc.client.VertxClientCall.doClose(VertxClientCall.java:141)
at io.vertx.grpc.client.VertxClientCall.lambda$null$1(VertxClientCall.java:112)
at io.vertx.core.impl.future.FutureImpl$4.onFailure(FutureImpl.java:188)
at io.vertx.core.impl.future.FutureBase.emitFailure(FutureBase.java:81)
⚙️ JVM Tests - JDK 21 #
- Failing: integration-tests/grpc-mutual-auth
📦 integration-tests/grpc-mutual-auth
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsEndpointTest.testRolesHelloWorldServiceUsingBlockingStub line 42 - History - More details - Source on GitHub
org.opentest4j.AssertionFailedError:
expected: "Hello neo from CN=testclient,O=Default Company Ltd,L=Default City,C=XX"
but was: "{"details":"Error id f40fbc17-2895-4565-9c53-4cbb83c26fa0-2, io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException","stack":"io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException\n\tat io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:268)\n\tat io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:249)\n\tat io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:167)\n\tat examples.GreeterGrpc$GreeterBlockingStub.sayHelloRoleUser(GreeterGrpc.java:224)\n\tat io.quarkus.grpc.examples.hello.HelloWorldTlsEndpoint.userRoleHelloBlocking(HelloWorldTlsEndpoint.java:52)\n\tat io.quarkus.grpc.examples.hello.HelloWorldTlsEndpoint$quarkusrestinvoker$userRoleHelloBlocking_c3f3bc53fca5e4f80b9ba2e1f1057bfb6c9d4f47.invoke(Unknown Source)\n\tat org.jboss.resteasy.reactive.server.handlers....
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsEndpointTest.testRolesHelloWorldServiceUsingMutinyStub line 65 - History - More details - Source on GitHub
org.opentest4j.AssertionFailedError:
expected: "Hello neo-mutiny from CN=testclient,O=Default Company Ltd,L=Default City,C=XX"
but was: "{"details":"Error id f40fbc17-2895-4565-9c53-4cbb83c26fa0-1, io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException","stack":"io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException\n\tat io.grpc.Status.asRuntimeException(Status.java:533)\n\tat io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onClose(ClientCalls.java:481)\n\tat io.grpc.PartialForwardingClientCallListener.onClose(PartialForwardingClientCallListener.java:39)\n\tat io.grpc.ForwardingClientCallListener.onClose(ForwardingClientCallListener.java:23)\n\tat io.grpc.ForwardingClientCallListener$SimpleForwardingClientCallListener.onClose(ForwardingClientCallListener.java:40)\n\tat io.quarkus.grpc.runtime.supports.IOThreadClientInterceptor$1$1.lambda$onClose$3(IOThreadClientInterceptor.java:70)\n\tat io.vertx.core.impl.Cont...
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsServiceTest.testRolesHelloWorldServiceUsingBlockingStub line 53 - History - More details - Source on GitHub
io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException
at io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:268)
at io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:249)
at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:167)
at examples.GreeterGrpc$GreeterBlockingStub.sayHelloRoleUser(GreeterGrpc.java:224)
at io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsServiceTest.testRolesHelloWorldServiceUsingBlockingStub(VertxHelloWorldMutualTlsServiceTest.java:53)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:972)
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsServiceTest.testRolesHelloWorldServiceUsingMutinyStub - History - More details - Source on GitHub
io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException
at io.grpc.Status.asRuntimeException(Status.java:533)
at io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onClose(ClientCalls.java:481)
at io.vertx.grpc.client.VertxClientCall.lambda$doClose$4(VertxClientCall.java:138)
at io.vertx.grpc.client.VertxClientCall.doClose(VertxClientCall.java:141)
at io.vertx.grpc.client.VertxClientCall.lambda$null$1(VertxClientCall.java:112)
at io.vertx.core.impl.future.FutureImpl$4.onFailure(FutureImpl.java:188)
at io.vertx.core.impl.future.FutureBase.emitFailure(FutureBase.java:81)
⚙️ JVM Tests - JDK 17 Windows #
- Failing: integration-tests/grpc-mutual-auth
📦 integration-tests/grpc-mutual-auth
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsEndpointTest.testRolesHelloWorldServiceUsingBlockingStub line 42 - History - More details - Source on GitHub
org.opentest4j.AssertionFailedError:
expected: "Hello neo from CN=testclient,O=Default Company Ltd,L=Default City,C=XX"
but was: "{"details":"Error id 8a361b90-e287-424d-949b-229630bc2267-2, io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException","stack":"io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException\r\n\tat io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:268)\r\n\tat io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:249)\r\n\tat io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:167)\r\n\tat examples.GreeterGrpc$GreeterBlockingStub.sayHelloRoleUser(GreeterGrpc.java:224)\r\n\tat io.quarkus.grpc.examples.hello.HelloWorldTlsEndpoint.userRoleHelloBlocking(HelloWorldTlsEndpoint.java:52)\r\n\tat io.quarkus.grpc.examples.hello.HelloWorldTlsEndpoint$quarkusrestinvoker$userRoleHelloBlocking_c3f3bc53fca5e4f80b9ba2e1f1057bfb6c9d4f47.invoke(Unknown Source)\r\n\tat org.jboss.resteasy.reactive.se...
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsEndpointTest.testRolesHelloWorldServiceUsingMutinyStub line 65 - History - More details - Source on GitHub
org.opentest4j.AssertionFailedError:
expected: "Hello neo-mutiny from CN=testclient,O=Default Company Ltd,L=Default City,C=XX"
but was: "{"details":"Error id 8a361b90-e287-424d-949b-229630bc2267-1, io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException","stack":"io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException\r\n\tat io.grpc.Status.asRuntimeException(Status.java:533)\r\n\tat io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onClose(ClientCalls.java:481)\r\n\tat io.grpc.PartialForwardingClientCallListener.onClose(PartialForwardingClientCallListener.java:39)\r\n\tat io.grpc.ForwardingClientCallListener.onClose(ForwardingClientCallListener.java:23)\r\n\tat io.grpc.ForwardingClientCallListener$SimpleForwardingClientCallListener.onClose(ForwardingClientCallListener.java:40)\r\n\tat io.quarkus.grpc.runtime.supports.IOThreadClientInterceptor$1$1.lambda$onClose$3(IOThreadClientInterceptor.java:70)\r\n\tat io.vertx....
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsServiceTest.testRolesHelloWorldServiceUsingBlockingStub line 53 - History - More details - Source on GitHub
io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException
at io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:268)
at io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:249)
at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:167)
at examples.GreeterGrpc$GreeterBlockingStub.sayHelloRoleUser(GreeterGrpc.java:224)
at io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsServiceTest.testRolesHelloWorldServiceUsingBlockingStub(VertxHelloWorldMutualTlsServiceTest.java:53)
at java.base/java.lang.reflect.Method.invoke(Method.java:569)
at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:972)
✖ io.quarkus.grpc.examples.hello.VertxHelloWorldMutualTlsServiceTest.testRolesHelloWorldServiceUsingMutinyStub - History - More details - Source on GitHub
io.grpc.StatusRuntimeException: UNKNOWN: io.quarkus.security.UnauthorizedException
at io.grpc.Status.asRuntimeException(Status.java:533)
at io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onClose(ClientCalls.java:481)
at io.vertx.grpc.client.VertxClientCall.lambda$doClose$4(VertxClientCall.java:138)
at io.vertx.grpc.client.VertxClientCall.doClose(VertxClientCall.java:141)
at io.vertx.grpc.client.VertxClientCall.lambda$null$1(VertxClientCall.java:112)
at io.vertx.core.impl.future.FutureImpl$4.onFailure(FutureImpl.java:188)
at io.vertx.core.impl.future.FutureBase.emitFailure(FutureBase.java:81)
Flaky tests - Develocity
⚙️ JVM Tests - JDK 17
📦 extensions/resteasy-reactive/rest-client/deployment
✖ io.quarkus.rest.client.reactive.stork.StorkResponseTimeLoadBalancerTest.shouldUseFasterService - History
expected: "hello, Alice" but was: "hello, I'm a slow server"-org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError:
expected: "hello, Alice"
but was: "hello, I'm a slow server"
at io.quarkus.rest.client.reactive.stork.StorkResponseTimeLoadBalancerTest.shouldUseFasterService(StorkResponseTimeLoadBalancerTest.java:58)
at java.base/java.lang.reflect.Method.invoke(Method.java:569)
at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:499)
at io.quarkus.test.QuarkusUnitTest.interceptTestMethod(QuarkusUnitTest.java:413)
Nothing stops us from erasing the hard drive either; at some point we just have to trust people to follow the rules. But the rules do have to make sense.
I think we don't have to do this, but we chose to do it. I don't think we should make exceptions for particular use cases, and I don't think we should change the rules to be inconsistent for that reason either. I maintain that the consistency of the rules and their enforcement is the key value proposition of Quarkus from which all others derive. I think that we need to review the dev services and dev mode use cases to fix the configurations and their usage to avoid these problems, rather than applying hack after hack. The amount of effort we've used working around these kinds of deficiencies seems like more effort than would have been required to fix them. |
Here is another issue that is bitting us: #42392 (comment) The easy solution is probably to dumb down Dev Services and always enable them during dev mode. If you want to connect to a specific service, you need to explicitly disable that Dev Service. It is going to make the experience a bit worse, but it is the easiest solution I can think of. Do you have any ideas for Dev Services? |
|
I’d like to understand better what’s going on here because if we end up having a warning for a perfectly valid usage of Quarkus, that’s going to be a problem. |
The warning system matches each property with its corresponding mapping/root class and config phase, so we have a list of properties and values that belong to build time. This list is recorded to compare with the configuration available at runtime. If, in the runtime configuration, we find a build-time property with a different value, we know that it has changed, and we report the warning. There are cases where we do query runtime configuration during build time, for instance a configuration URL to make decisions about DevServices, but this is done "off the grid", in a sense that such runtime configurations are not queried using a config object, but using the programmatic API and the full property name ( Now, coming back to the REST Client, we have such a case: So, how did we check the required configuration? The REST Client config has a specific implementation that other extensions do not follow: #21530, especially: These methods allowed you to query different combinations of how the REST Client properties could be found in the config system. When I moved the REST Client config in #42106, this created a problem when querying the possible runtime configuration for Dev Services. The current config system, cannot query for multiple variations of a key (except by using relocations / fallbacks). Right now, a REST Client may be configured with the following variations:
Plus, all of the Ok, so why is this a problem? As in the old implementation, we query the config mapping map in a certain order (FQN, simple name, etc.), which works fine. With the fallbacks and relocations rules, you get the expected configuration (with the improvements from #42932), in the mapping object But how do we query the runtime configuration Unfortunately, I forgot when adding a catch-all My suggestion to cover this case was to ignore it and not report it (implemented here), which followed my conversation with @dmlloyd in this PR. |
Mappings now allow us to map the same property in different config phases, which old config roots did not. That restriction means that a property mapped both at build time and runtime would receive a warning if modified by runtime.
The problem with the wrong warnings was introduced by #42106, namely:
https://github.com/quarkusio/quarkus/pull/42106/files#diff-481bcfd93c179379817ad3e7013830913c33b03317b7444c7ffa0a7bf2c58a5aR69-R71
Which is required by:
https://github.com/quarkusio/quarkus/pull/42106/files#diff-97429049f54dc587b3b05727330a73994cb216f10179439fd8f654e302bbb98dR82-R85
Previously, the code just checked the' ...url` property in build time directly without using a root/mapping, so we never noticed the issue.