Skip to content

Conversation

@pjungkamp
Copy link
Contributor

description

This replaces the Config::load functions with an nlohmann::json-based implementation aiming for correctness and ease of use.

motivation

The final goal is to port VILLASnode configuration handling top-down to nlohmann::json's idiomatic C++ API. This would then enable us to validate configurations and API requests against our JSON schema using json-schema-validator inside the villas-node process.

This should improve the state of our schemas, as they become a requirement for new features instead of being an afterthought in the development cycle.

backwards-compatibility

This rewrite does change parsing behavior for a few edge cases I considered bugs and isn't feature complete yet.

include directives

Our include directives currently do not work as documented here.

The documented syntax is { "@include": "some/glob*pattern" }. But the actual implementation parses special strings like "@include some/glob*pattern" as you can see in e.g. github.com/VILLASframework/node/blob/master/etc/tricks/tricks.json.

I've implemented something mirroring the documented behavior with the difference that I've chosen to use $include instead of the libconfig-inspired @include, because I consider this to be the more common style for special keys in JSON objects (like $schema or $comment).

environment variable substitutions

Our current environment variable substitution code is recursively expanding environment variables and does not specify any way to "escape" our variable substitution syntax.

This means we can make villas-node spin indefinitely on recursive substitutions like this:

ENV_A='${ENV_B}' ENV_B='${ENV_A}' villas-node - << EOF
{ "logging": { "level": "${ENV_A}" } }
EOF

And there is no way to have a string that contains ${...} that does not substitute environment variables.

environment variable substitutions inside include directives

Our current implementation will expand environment variables before globbing without escaping special glob characters. This means that environment variables will be substituted as partial glob expressions. I don't think that this is desirable either.

open questions + edge cases

1. How should we include other files?

As documented, as currently implemented or another way?

2. How should we escape special values?

I'm personally in favor of just declaring $ as special, which then means that every literal dollar sign has to be escaped as $$.

Another option would be to use \$ which would need to be doubled to \\$ since it's also the escape for regular json strings. This would be consistent with the escaping of glob patterns in include directives (These are also escaped using \ which need to be doubled in JSON).

3. Should we escape substituted environment variables in include directives before globbing?

I'm personally not fond of allowing partial patterns to be substituted into the glob pattern.


@stv0g what do you think?

@pjungkamp
Copy link
Contributor Author

pjungkamp commented Feb 9, 2026

Some additional notes

Another option would be to restrict variable substitutions to "complete" values and remove the braces from the substitution syntax, i.e. { "$include": "$string:INCLUDE_PATTERN" }. I'd be fine with allowing glob patterns in the expanded environment variables in that case. This would mean that the dollar sign is only special and would need to be escapable when at the start of a string.

That could also be extended to something like an "$json:ENV" syntax that would parse the variable as JSON and substitute the parsed JSON instead.


I also want to add that the only place where I found "@include" syntax in a JSON configuration file is in the etc/tricks/tricks.json example configuration.


I'll add compatibility code for any breaking change with a warning that should notify any user of the upcoming change.

Signed-off-by: Philipp Jungkamp <philipp.jungkamp@rwth-aachen.de>
@pjungkamp pjungkamp force-pushed the configuration-overhaul branch from 6cbbd5c to 9932fa9 Compare February 10, 2026 00:05
Signed-off-by: Philipp Jungkamp <philipp.jungkamp@rwth-aachen.de>
@pjungkamp pjungkamp force-pushed the configuration-overhaul branch from 9932fa9 to 4037b5d Compare February 10, 2026 00:07
@pjungkamp
Copy link
Contributor Author

update

  • I've now added full backwards compatibility for the current parsing behavior.
  • I've also added deprecation warnings for when the current style of @include directive or ${ENV} substitution are encountered.
  • Environment variables can now be substituted as strings using "$string:ENV" or as JSON using "$json:ENV".
  • The special handling can be escaped by doubling the $ at the start of the string.

@stv0g
Copy link
Contributor

stv0g commented Feb 10, 2026

Hi @pjungkamp,

Thanks for pushing this forward!

I think we may be can drop support for @include and variable substitutions?
Its a quite rarely used feature anyways.

As we initially only supported libconfig's syntax, there where no tooling to easily pre-process a configuration. As we are now transitioning more to JSON, I believe its a better approach to use existing JSON tooling for such tasks (jq, simple Javascript code-snippet with NodeJS, or even Nix with toJSON).

I also thought about possibly replacing this feature, by allow passing configuration settings via CLI arguments:

villas node config1.json config2.json -o logging.level=debug

Here any file passed would be an overlay to the previous one. Any -o option would provide a JSONPath and value which would be applied as just another overlay.

What do you think?

@pjungkamp
Copy link
Contributor Author

pjungkamp commented Feb 11, 2026

I'm all in favor of removing the include and substitution mechanisms altogether.


The merging of configuration files is more tricky. How would you define "overlay" here? I'm concerned about the merging behavior for non-object types.

Let's assume the following two configurations:

{
  "nodes": {
    "signal": {
      "type": "signal.v2",
      "signal": "sine",
      "in": {
        "signals": [{ "name": "sine_signal" }]
      }
    }
  },
  "paths": [
    { "in": "signal", "hooks": ["print"] }
  ]
}
{
  "nodes": {
    "signal": {
      "type": "exec",
      "in": {
        "signals": []
      }
    }
  },
  "paths": []
}

This illustrates three problematic edge cases:

  1. Collision in nodes/signal/type string. I am not convinced that any merging logic we apply here will do us any good.
  2. Collision in nodes/signal/in/signals array. I'd argue that this should not concatenate both arrays, as the concatenated list is surely not what either configuration was intended to produce. The signal list is a definition of a sample's structure and should not be merged .
  3. Collision in paths array. The intuitive behavior for merging paths would arguably be concatenation, because it's more of a container rather than a structural definition.

I think that any attempt of us to define some behavior here would lead to a crude approximation of an RFC 7396 JSON Merge Patch.

# This makes it clear what kind of json document is passed here.
villas node config.json -merge-patch merge_patch.json

My first thought for the command-line override for values was something like jq's --arg name string and --argjson name json with RFC 6901 JSON pointers instead of a name.

villas node config.json --option-string nodes/signal/signal sine --option-json nodes/signal/rate 10

This would place three restrictions on our whole configuration:

  1. Object keys may not be numbers, or we'd have to consider an object with numeric keys to be equivalent to an array value.
  2. Empty objects and arrays are equivalent to null.
  3. No NUL characters in object keys.

The first and second restrictions are required to disambiguate a JSON pointer referring to an object key or an array index (see e.g. nlohmann/json#1575). The third is required because a process' arguments are passed as NUL-terminated strings, and there's no dedicated way to escape a NUL character in a JSON pointer.

I'm fine with introducing these restrictions, but I'd want to make this choice deliberately and not by accident.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants