quarkus: CORSConfig is not representable as YAML

As seen during #4525 development, CORSConfig cannot be represented as YAML because we’d need the quarkus.http.cors key to be both the key of a key/value pair and the key of a dictionary, which is not possible.

A properties-based CORS filter configuration currently looks like this:

quarkus.http.cors=true
quarkus.http.cors.origins=http://foo.com,http://www.bar.io
quarkus.http.cors.methods=GET,PUT,POST
quarkus.http.cors.headers=X-Custom
quarkus.http.cors.exposed-headers=Content-Disposition
quarkus.http.cors.access-control-max-age=24H

We only need to change the quarkus.http.cors=true property to quarkus.http.cors.enabled=true to make it convertible to YAML.

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 25 (24 by maintainers)

Commits related to this issue

Most upvoted comments

Here’s the complete list of config keys that are currently not representable as YAML, based on the ALL CONFIGURATION OPTIONS doc page (I’m glad it existed to do this check!):

quarkus.hibernate-orm.database.generation
quarkus.hibernate-orm.database.generation.halt-on-error

^ quarkus.hibernate-orm.database.generation could be replaced with quarkus.hibernate-orm.database.generation.auto to match the original Hibernate property. @gsmet WDYT?

quarkus.hibernate-orm.dialect
quarkus.hibernate-orm.dialect.storage-engine

^ quarkus.hibernate-orm.dialect could be replaced with quarkus.hibernate-orm.dialect.name but it differs from the original Hibernate property. @gsmet WDYT?

quarkus.http.cors
quarkus.http.cors.origins
quarkus.http.cors.methods
quarkus.http.cors.headers
quarkus.http.cors.exposed-headers
quarkus.http.cors.access-control-max-age

^ quarkus.http.cors will become quarkus.http.cors.enabled

quarkus.log.console.async
quarkus.log.console.async.overflow
quarkus.log.console.async.queue-length

^ quarkus.log.console.async could be replaced with quarkus.log.console.async.enabled

quarkus.log.file.async
quarkus.log.file.async.overflow
quarkus.log.file.async.queue-length

^ quarkus.log.file.async could be replaced with quarkus.log.file.async.enabled

quarkus.log.syslog.async
quarkus.log.syslog.async.overflow
quarkus.log.syslog.async.queue-length

^ quarkus.log.syslog.async could be replaced with quarkus.log.syslog.async.enabled

I also noticed several configuration keys being used by quarkus-agroal, quarkus-reactive-mysql-client and quarkus-reactive-pg-client. I’m not sure that’s an issue though, but I’d like your opinion about it:

quarkus.datasource.max-size
quarkus.datasource.password
quarkus.datasource.url
quarkus.datasource.username

I’ll see if that kind of issue can be detected at build time, but I’m not sure it’ll be easy. Half of these cases were related to the use of @ConfigItem(name = ConfigItem.PARENT) and the other half was the consequence of hardcoded config keys names using @ConfigItem(name = "foo").

I like the idea. I guess this would have to be done in the YamlConfigSource that is currently being donated from micrprofile-extensions to smallrye-config, as discussed in #4525.

I’ll give empty YAML keys a try with #4525 as it is currently using the same YAML parser (snakeyaml) than the future smallrye-config version. It might already support empty keys.

Regarding _ or *, are you suggesting this could be used in a YAML file as the key of a config value?

Right. Rather than upend the entire configuration system to accommodate YAML, maybe we can hack YAML a bit to accommodate the config system. So if I have foo.bar and foo.bar.baz, then in the YAML maybe in addition to looking for a bar inside of a foo, we should look for a * within a bar within a foo.

There’s something about empty keys in the YAML spec, but I’ve never tested it with Java.

Ah well empty keys are probably the exact right thing to use in this case since it’s in the spec, presumably for this exact purpose.

Hardcoded config keys are a problem because they can lead to the same key base being shared by config attributes that were originally named differently. There’s a good example here.

This would be solved as well by allowing an empty key within dialect to be used for dialect.

Great, thanks @gwenneg

We have the test-extension with possible configuration values - https://github.com/quarkusio/quarkus/tree/master/core/test-extension/runtime/src/main/java/io/quarkus/extest/runtime/config with a rather large test suites here - https://github.com/quarkusio/quarkus/tree/master/core/test-extension/deployment/src/test/java/io/quarkus/extest

not sure if we must have the same coverage with a yaml file, but the extension is handy when you want to test various things, config among others 😃

HTH.

Yes, that’s a good point @machi1990.

I was planning on checking all config classes while fixing CORSConfig 😃

I think the issue is related to having something like: quarkus.extension.foo = … quarkus.extension.foo.bar = …

On Tue, Oct 15, 2019 at 6:20 PM Manyanda Chitimbo notifications@github.com wrote:

@gwenneg https://github.com/gwenneg Hi, is this issue only for the CORSConfig? or for all config items inheriting parent’s name with

@ConfigItem(name = ConfigItem.PARENT)

?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/quarkusio/quarkus/issues/4568?email_source=notifications&email_token=AAJYOBOPSU2KHLHPIDNFFLDQOXUVHA5CNFSM4JA3ALIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBJMAWY#issuecomment-542294107, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJYOBNI2XKZ5ZL5V33Z6P3QOXUVHANCNFSM4JA3ALIA .