Karaf configuration options are sometimes ignored [PAXEXAM-929]

[ops4j-issues]

Amichai Rothman created PAXEXAM-929

Karaf configuration options are sometimes ignored, like the extend option to an existing property, or a put following a replace. The order of precedence is unclear, and requires a lot of trial and error and workarounds (like copying the full configuration in code multiple times, or alternatively using only files), which makes the whole configuration process unnecessarily cumbersome.

The KarafConfigurationFile implementations also have a couple of bugs, like not clearing previous properties when loading a new config file, or leaving the in-memory map empty when a configuration file is replaced instead of loading the new properties. In addition, the configuration handling code in KarafTestContainer is quite old and very convoluted, and also seems to include a workaround for a featureScanner that hasn't existed since the code moved from the Karaf project to Pax Exam a couple years ago.

I'll soon open a PR that greatly simplifies the config option handling code, fixes the issues and adds tests. Note that the new code has the very simple and intuitive logic of applying all configuration options to each file in order, so later options override earlier ones but nothing is ignored or skipped (e.g. if two options set the value for the same key, or there is a put or replace from file followed by a dynamic put or extend, the last set values will prevail). While this is technically a change in behavior that is not entirely backward-compatible, it seems that most of the option sequences whose behavior is changed were broken and unused for years anyway (since nobody said anything) so it likely will improve the situation going forward without harming the existing straightforward use cases which continue to work.

If there are any issues caused by the changes let me know and I'll try to accommodate the new code, but hopefully it can stay straightforward and flexible as it is now.

Also, I worked on this on master (5.0.0-SNAPSHOT), but would really like to see this backported to 4.x and released soon, unless there's a plan to release 5 real soon as well :smile:

---

Affects: 4.13.1 Votes: 0, Watches: 2

[ops4j-issues]

Jean-Baptiste Onofre commented

Let me take a look, but I'm a bit surprised as the options are evaluated before the container actually starts.

[ops4j-issues]

Amichai Rothman commented

Yes, I was surprised too :smile:

Specifically, I expected all the scenarios in the ValidateConfigurationOptions test I added to work, but most of them originally failed.

You are correct, the problems are all in the configuration options pre-processing in KarafTestContainer.updateUserSetProperties and the underlying KarafConfigurationFile implementations it uses. In several different ways they disposes of unprocessed options, whether by overwriting the per-key option lists when a new option for that key is encountered (so extend doesn't work), aborting processing of all file options after a replace is encountered, copying files without ever updating the in-memory state so further operations can't work properly, etc.

[ops4j-issues]

Amichai Rothman commented

I now realize that KarafDistributionConfigurationFileReplacementOption is used also to copy non-configuration files, so the config loading fix above might interfere with that. I would recommend adding a separate KarafDistributionFileReplacementOption that just does an arbitrary binary file copy (replace) operation, e.g. copying a jar to system folder.