Config viewer as part of vertx-http in dev mode #7430

hpehl · 2020-02-26T09:46:51Z

Fixes: #1349.
Supersedes #5692.

Adds a route in dev mode which lists the configured values for MicroProfile Configuration. The route is registered using the path "/config" by default and lists all properties of all config sources. The config sources are sorted descending by ordinal, the properties by name. If no config is defined an empty JSON object is returned.

geoand · 2020-02-26T09:54:28Z

extensions/vertx-http/runtime/pom.xml

@@ -50,6 +50,10 @@
                </exclusion>
            </exclusions>
        </dependency>
+        <dependency>


Why is this needed? Isn't the io.vertx:vertx-web dependency added above enough?

I've tried w/o, but got a `java.lang.ClassNotFoundException:

[INFO] Running io.quarkus.vertx.http.configviewer.ShowConfigTest 2020-02-26 10:57:52,162 INFO [io.quarkus] (main) Quarkus 999-SNAPSHOT started in 1.513s. Listening on: http://0.0.0.0:8080 2020-02-26 10:57:52,162 INFO [io.quarkus] (main) Profile dev activated. Live Coding activated. 2020-02-26 10:57:52,162 INFO [io.quarkus] (main) Installed features: [cdi, security] 2020-02-26 10:57:53,319 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (vert.x-eventloop-thread-4) HTTP Request to /config failed, error id: f23efab7-ebff-4242-b10e-c4efa772f718-1: java.lang.NoClassDefFoundError: com/fasterxml/jackson/databind/JsonSerializer at io.vertx.core.json.jackson.JacksonCodec.toString(JacksonCodec.java:151) at io.vertx.core.json.JsonObject.encode(JsonObject.java:785) at io.quarkus.vertx.http.runtime.devmode.ConfigViewerHandler.handle(ConfigViewerHandler.java:19) at io.quarkus.vertx.http.runtime.devmode.ConfigViewerHandler.handle(ConfigViewerHandler.java:11) at io.vertx.ext.web.impl.BlockingHandlerDecorator.lambda$handle$0(BlockingHandlerDecorator.java:48) at io.vertx.core.impl.ContextImpl.lambda$executeBlocking$2(ContextImpl.java:316) at io.vertx.core.impl.TaskQueue.run(TaskQueue.java:76) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) at java.lang.Thread.run(Thread.java:748) Caused by: java.lang.ClassNotFoundException: com.fasterxml.jackson.databind.JsonSerializer at java.net.URLClassLoader.findClass(URLClassLoader.java:382) at java.lang.ClassLoader.loadClass(ClassLoader.java:424) at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:349) at java.lang.ClassLoader.loadClass(ClassLoader.java:357) at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:291) at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:251) ... 11 more

Ah yes, that is because we exclude Jackson databind - and we need to continue doing this, we can't let it sneak in.
Can the JSON part you PR just be based on Jackson core instead of relying on databind?

Yes, using JSON Object "encode" requires Jackson, which we want to avoid in vertx-http. It may require a bit of code, but in your case, Jackson should not be required if you just implement a simple "encode" method doing the ToJson transformation manually. As you know the structure of the result, you can just generate the JSON string.

Ok, I'm going to remove the dependency to Jackson and do the JSON encoding manually.

cescoffier · 2020-02-26T10:01:10Z

bom/runtime/pom.xml

+                <groupId>io.vertx</groupId>
+                <artifactId>vertx-core</artifactId>
+                <version>${vertx.version}</version>
+            </dependency>


This dependency is already defined in the vert.x bom. Isn't it?

cescoffier · 2020-02-26T10:04:48Z

extensions/vertx-http/runtime/pom.xml

@@ -50,6 +50,10 @@
                </exclusion>
            </exclusions>
        </dependency>
+        <dependency>


Yes, using JSON Object "encode" requires Jackson, which we want to avoid in vertx-http. It may require a bit of code, but in your case, Jackson should not be required if you just implement a simple "encode" method doing the ToJson transformation manually. As you know the structure of the result, you can just generate the JSON string.

dmlloyd · 2020-02-26T14:21:02Z

...ons/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/devmode/ConfigViewer.java

+                        JsonObject jsonProperties = new JsonObject();
+                        for (String propertyName : sortedPropertyNames) {
+                            try {
+                                jsonProperties.put(propertyName, source.getValue(propertyName));


It looks like this is going to print every config value from every config source (as opposed to printing only the effective values). I assume that this was your intent!

One concern I have is that this will expand property expressions. If you don't want that (and I suspect you don't since you're printing all the raw config keys rather than just the effective configuration contents), then the whole thing should be enclosed like this:

boolean old = ExpandingConfigSource.setExpanding(false); try { // ... do all the work here ... } finally { ExpandingConfigSource.setExpanding(old); }

Another concern I have is that this might expose secrets/passwords!

Thanks @dmlloyd
Right, I only want to print the effective values. Will wrap the code like you suggested.

I don't have a solution how to hide secrets / passwords.

Let me post something to the dev list on this. I have an idea... I think.

Actually I'll post an issue for it...

I've opened #7442 for this.

dmlloyd

I hate to do this but I think we should ⏸️ this until we solve #7442. If others vehemently disagree I'll lift this review but I think we should be security-minded here.

…urce.setExpanding(false)

hpehl · 2020-02-26T15:35:41Z

In any case I've updated the PR:

Remove dependency to Jackson data binding
wrap code in ExpandingConfigSource.setExpanding(false)

hpehl · 2020-02-26T15:41:33Z

The JSON code is based on https://github.com/stleary/JSON-java.
Third commit adds the relevant license headers.

gastaldi · 2020-02-27T13:45:59Z

One poor-man solution is to check if the key contains password and avoid showing it (or showing anyway, since you're in Dev mode, I don't think this is much of an issue)

gsmet

Added a concern about the license of the added JSON classes.

gsmet · 2020-04-22T16:32:54Z

...sions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/devmode/JsonObject.java

+ The above copyright notice and this permission notice shall be included in all
+ copies or substantial portions of the Software.
+
+ The Software shall be used for Good, not Evil.


Unfortunately, this makes it non-free software. We can't include it.

Better use Jackson here

Better use Jackson here

Ah wouldn't that meaning pulling Jackson to the core?

Ah I missed @cescoffier comments in #7430 (comment), Jackson is not an option here

mkouba · 2020-05-18T13:18:43Z

FYI in this PR there is a minimalistic JSON string generator (adapted from the Weld Probe) and I believe that it could be used in this PR as well.

hpehl · 2020-05-18T15:13:44Z

Thanks @mkouba. I'll have a look asap.

gastaldi · 2020-10-20T15:11:53Z

Closing as it needs a rebase and the PR needs to reuse the existing JSON libraries instead. Please reopen when done

Config viewer as part of vertx-http in dev mode

d7b736b

hpehl mentioned this pull request Feb 26, 2020

Extension to view MicroProfile config values in dev mode #5692

Closed

geoand reviewed Feb 26, 2020

View reviewed changes

cescoffier requested changes Feb 26, 2020

View reviewed changes

geoand added this to the 1.3.0 milestone Feb 26, 2020

dmlloyd reviewed Feb 26, 2020

View reviewed changes

dmlloyd mentioned this pull request Feb 26, 2020

Hide secrets in configuration #7442

Open

dmlloyd requested changes Feb 26, 2020

View reviewed changes

Remove Jackson databinding dependency; wrap code in ExpandingConfigSo…

0ce0aa3

…urce.setExpanding(false)

Add license header from https://github.com/stleary/JSON-java

c03633a

gsmet removed this from the 1.3.0.CR1 milestone Mar 5, 2020

ia3andy mentioned this pull request Mar 27, 2020

Add a way to log all/some configuration at startup #8218

Closed

gsmet requested changes Apr 22, 2020

View reviewed changes

gastaldi added the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Oct 20, 2020

gastaldi closed this Oct 20, 2020

gsmet added the triage/invalid This doesn't seem right label Oct 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Config viewer as part of vertx-http in dev mode #7430

Config viewer as part of vertx-http in dev mode #7430

hpehl commented Feb 26, 2020 •

edited by geoand

Loading

geoand Feb 26, 2020

hpehl Feb 26, 2020

geoand Feb 26, 2020

cescoffier Feb 26, 2020

hpehl Feb 26, 2020

cescoffier Feb 26, 2020

cescoffier Feb 26, 2020

dmlloyd Feb 26, 2020

dmlloyd Feb 26, 2020

hpehl Feb 26, 2020

dmlloyd Feb 26, 2020

dmlloyd Feb 26, 2020

dmlloyd Feb 26, 2020

dmlloyd left a comment

hpehl commented Feb 26, 2020

hpehl commented Feb 26, 2020

gastaldi commented Feb 27, 2020

gsmet left a comment

gsmet Apr 22, 2020

gastaldi Apr 22, 2020

machi1990 Apr 22, 2020

gastaldi Apr 22, 2020

mkouba commented May 18, 2020

hpehl commented May 18, 2020

gastaldi commented Oct 20, 2020

Config viewer as part of vertx-http in dev mode #7430

Config viewer as part of vertx-http in dev mode #7430

Conversation

hpehl commented Feb 26, 2020 • edited by geoand Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmlloyd left a comment

Choose a reason for hiding this comment

hpehl commented Feb 26, 2020

hpehl commented Feb 26, 2020

gastaldi commented Feb 27, 2020

gsmet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkouba commented May 18, 2020

hpehl commented May 18, 2020

gastaldi commented Oct 20, 2020

hpehl commented Feb 26, 2020 •

edited by geoand

Loading