Conversation
tis24dev
commented
Feb 9, 2026
tis24dev
commented
- Add config var expansion, docs and roadmap
- Treat PVE ACLs in user.cfg; remove roadmap
Implement ${VAR}/$VAR expansion for scalar config values (config keys first, then environment) by adding a configVarExpander with caching and cycle protection; preserve historical BASE_DIR behavior. Update getString/getStringWithFallback to use the new expander and add a unit test to validate resolving config-key references. Update templates/backup.env and docs/CONFIGURATION.md to document the new expansion behavior and clarify that backup.env values are resolved without exporting. Add a new RESTORE_ROADMAP.md documenting restore coverage, and update internal/orchestrator/.backup.lock timestamps.
Document and enforce that PVE stores ACLs inside user.cfg: update CONFIGURATION note, change collector to record only user.cfg (adjust description), remove acl.cfg from directory excludes, and update tests accordingly.
There was a problem hiding this comment.
Pull request overview
This PR syncs dev changes into main by improving configuration variable expansion behavior, updating documentation/templates to reflect it, and aligning PVE access-control collection with how ACLs are represented.
Changes:
- Add config-file-aware
$VAR/${VAR}expansion for scalar string config values (with tests). - Update docs and the
backup.envtemplate to document the new expansion behavior. - Treat PVE ACLs as stored in
user.cfg(stop handlingacl.cfgfor PVE; adjust exclusions and tests).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/orchestrator/.backup.lock | Updates a lockfile’s runtime contents (PID/time/host). |
| internal/config/templates/backup.env | Documents that variable expansion can reference keys within the same file. |
| internal/config/config_test.go | Adds a unit test ensuring config-key references expand correctly. |
| internal/config/config.go | Implements recursive config-key expansion (config first, then environment). |
| internal/backup/collector_pve_test.go | Updates PVE collector test expectations to no longer involve acl.cfg. |
| internal/backup/collector_pve.go | Treats PVE ACLs as part of user.cfg and updates snapshot exclusions accordingly. |
| docs/CONFIGURATION.md | Documents scalar string expansion behavior and clarifies PVE ACL snapshot behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pid=29780 | ||
| host=pve | ||
| time=2026-02-05T20:31:58+01:00 | ||
| time=2026-02-09T14:30:21+01:00 |
There was a problem hiding this comment.
This appears to be a runtime lock file (PID/host/timestamp) that was accidentally committed. Keeping it in the repo can confuse users and may cause spurious diffs; it should be removed from version control and added to .gitignore (or generated at runtime under BACKUP_PATH/LOCK_PATH instead).
| if envVal, ok := os.LookupEnv(upperKey); ok { | ||
| e.cache[upperKey] = envVal | ||
| return envVal | ||
| } |
There was a problem hiding this comment.
configVarExpander uppercases variable names before looking them up in the process environment. This prevents expansion of commonly-lowercase env vars (e.g. http_proxy/no_proxy) when referenced as ${http_proxy}. Consider checking the original key with os.LookupEnv(key) as a fallback (while still using uppercased keys for config-file lookups).
| } | |
| } | |
| // Fallback: check the original (non-uppercased) key in the environment to | |
| // support commonly-lowercase variables like http_proxy and no_proxy. | |
| if key != upperKey { | |
| if envVal, ok := os.LookupEnv(key); ok { | |
| e.cache[upperKey] = envVal | |
| return envVal | |
| } | |
| } |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
1 participant
