Suggestion: expand ${NAME} environment variables in the config file

[haavar]

This is a suggestion rather than a finished proposal — happy to change the syntax, the undefined-variable behaviour, or drop it entirely if you'd prefer a different approach.

What it does

Reads the configuration file into memory and expands ${NAME} references from the process environment before handing the text to libconfig (which does no variable or include expansion of its own). Parsing then happens via config_read_string() instead of config_read_file().

Why

It lets a single shared configuration file serve many instances (each sets different environment variables), and lets secrets such as an MQTT password live in the environment instead of being rendered into the file at deploy time. This is an alternative to adding a per-setting command-line flag for the multi-instance use case discussed in #2266 — a single shared config file can drive every instance from the environment.

Syntax

• ${NAME} is replaced by the value of environment variable NAME, where NAME matches [A-Za-z_][A-Za-z0-9_]*. The braces are required, so a bare $NAME or a stray $ is never touched.
• $${ is replaced by a literal ${, so a config can still contain a literal ${.
• Referencing an undefined variable is a fatal error naming the variable, rather than a silent empty substitution (which is nasty to debug). This is a deliberate choice — an empty-string default would be easy to switch to if you'd prefer.

Backward compatibility

• A configuration file containing no ${ is passed through byte-for-byte and parses exactly as before.
• The file-not-found path and the parse-error reporting (line / text / filename) are preserved. Since config_read_string() records no filename, the parse-error message falls back to the resolved config path.

Documentation

scripts/shairport-sync.conf gains a short block describing the syntax.

Testing

Built the musl/Alpine binary and exercised it with -vv:

1. name = "${SP_NAME}" with SP_NAME set → the service name is the env value.
2. Undefined ${VAR} → dies naming the variable.
3. $${SP_NAME} → the literal ${SP_NAME} survives into the parsed value (no expansion).
4. A config with no ${...} → parses unchanged, and a bare $ in a string is left alone.
5. A deliberate syntax error → still reported as "Line N of the configuration file ...".

[mikebrady]

This is a nice idea, and seems well thought out.

My generalised worry, though, is that these things will need to be maintained. So -- real question here -- will it "earn its keep", so to speak.

[haavar]

Thanks — and honestly, I think you're right that it's hard to say up front whether it'll earn its keep. Let me give the context rather than oversell it.

It came out of the multi-instance case: running several AirPlay instances from one host. In our own setups that's 5 here and 11 for @jslove, and today each instance needs its own near-identical config file. The alternative people reach for is templating the config in their deployment tooling — which everyone ends up reinventing. A single shared config with a few per-instance variables (name, port) avoids that.

There's also a smaller benefit that stands on its own: it lets secrets — an MQTT password, say — come from the environment instead of being written into a config file on disk, which is generally good practice.

On the maintenance worry: the thing I tried to keep small is the blast radius. It's a text pre-processing pass that runs before libconfig sees the file, so it has no knowledge of any individual setting — adding, renaming or removing config options never touches it. It's self-contained in string_utilities.c with no libconfig coupling, so it shouldn't grow or need attention as the config evolves.

That said, I'm not attached to it landing as-is. The obvious alternative is command-line overrides for individual settings — for my own setup that would only mean adding the MQTT password as a flag, so I could get by that way. The reason the general approach appealed to me on the maintenance front is that each per-setting flag has to be added and maintained by hand, so that surface grows as settings are added, whereas this substitution pass is fixed no matter how the config evolves. But I genuinely don't know what other setups would need — and if the maintenance cost still outweighs the benefit for you, that's a fair call.

[haavar]

One thing I should raise honestly, since the recommended way to run this is in Docker: most of what this PR does could be handled in the image's entrypoint instead, with no change to the binary.

docker/run.sh already ends with exec /usr/local/bin/shairport-sync "$@", so a small envsubst step there could render a mounted config template from the environment before launch — the same pattern the official nginx image uses. It'd only need the gettext-envsubst subpackage, and using an explicit allowlist (envsubst '${SP_NAME} ${MQTT_PW}' < template > config) keeps it from touching anything else. That covers both things I was after — one shared config across instances, and keeping the MQTT password out of the file — and it lives where you already maintain the image rather than in the config parser.

The one gap is that it's Docker-only. I know you'd like to ship a Debian package at some point; depending on how that ends up being run it may not be Docker, and then there's no entrypoint to hook (though for a systemd setup, EnvironmentFile= would cover a lot of the same ground). So the in-binary version really only earns its keep for non-Docker installs.

Given that, I'm happy either way: land this, or drop it and I'll put together the entrypoint change for the image instead. No strong attachment to it being in the binary.

[noelhibbard]

I know I'd use this if it was available. I'd be able to supply settings right in my docker-compose and have one common sharport-sync.conf.

[mikebrady]

Thanks for your comments, @haavar and @noelhibbard!

[mikebrady]

Thanks for this!