Conversation
tis24dev
commented
Feb 4, 2026
tis24dev
commented
- Log updated missing config keys after upgrade
- Insert missing template keys respecting user order
- Group unanchored missing keys into upgrade section
- Improve casing conflict warning wording
After running the configuration upgrade, log an informational message when missing keys were added. Adds a conditional that checks sessionLogger and cfgUpgradeResult and logs the count and comma-separated list of cfgUpgradeResult.MissingKeys to help surface which keys were introduced during the upgrade.
Improve config upgrade logic to preserve user key positions and insert missing template entries near related user keys. parseEnvValues now records key ranges (keyRange) for each user entry and returns them; computeConfigUpgrade collects template entries, computes missing entries, and builds ordered insert operations (using sort) to merge template lines into the original file without unnecessary rewrites. Also adjust handling for block (multi-line) values, preserve extra keys and casing warnings, and update tests (rename and expectations) to reflect preserved extra keys and original casing for blocks.
Add an index field to templateEntry to track template order and use it for insert ordering. When missing template entries have no nearby anchor, collect them and append a single "# Added by upgrade" section (with a separating blank line if appropriate) at the append index instead of trying to insert them in-place. Adjust insertOp ordering accordingly. Add a test (TestUpgradeConfigAddsMissingKeysUnderUpgradeSectionWhenNoAnchor) to verify legacy keys are preserved and missing keys are added under the upgrade section.
Update the warning emitted in computeConfigUpgrade when a config key differs only by case from a template key. The message now says the entry is "preserved with original casing" instead of "preserved as custom entry" to more accurately describe the behavior and reduce confusion. Affects internal/config/upgrade.go.
There was a problem hiding this comment.
Pull request overview
Updates the configuration upgrade flow to minimize diffs in user configs by inserting only missing template keys (while preserving existing user ordering/values), and improves upgrade visibility in logs/CLI messaging.
Changes:
- Reworked config upgrade logic to insert missing template keys into the existing file (with a dedicated “Added by upgrade” section for unanchored keys).
- Improved handling/messaging around casing conflicts and preserved key casing.
- Added/updated tests for the new insertion behavior and casing preservation; added upgrade logging of missing keys.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
internal/config/upgrade.go |
Replaces template-driven re-rendering with an insertion-based strategy; adds anchoring/index tracking for where to insert missing keys. |
internal/config/upgrade_test.go |
Updates expectations and adds coverage for “unanchored missing keys” being grouped under an upgrade section; adjusts casing expectations. |
cmd/proxsave/upgrade.go |
Logs which missing keys were added during an upgrade session. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/config/upgrade.go
Outdated
| // If exact match was in template keys (should have been processed above), skip | ||
| if templateKeys[key] { | ||
| if caseConflicts[upperKey] && caseMap[upperKey] != key { | ||
| extraKeys = append(extraKeys, key) |
There was a problem hiding this comment.
ExtraKeys is being populated with case-collision duplicates (caseConflicts[upperKey] && caseMap[upperKey] != key), even though it’s used/documented elsewhere as “keys not present in the template” (e.g., cmd/proxsave/main.go prints “Custom keys … not present in template”). This will cause misleading output for users (a differently-cased template key will be reported as a custom key). Consider keeping ExtraKeys strictly for non-template keys and tracking case-collision “extra” entries separately (e.g., a dedicated field or warnings-only), and update the UpgradeResult docs accordingly.
| extraKeys = append(extraKeys, key) | |
| // This key collides by case with another key; it is not a true "extra" key | |
| // (the template does contain a corresponding key), so do not report it in extraKeys. | |
| if templateKey, ok := templateKeyByUpper[upperKey]; ok { | |
| warnings = append(warnings, fmt.Sprintf("Key %q differs only by case from template key %q; preserved with original casing", key, templateKey)) | |
| } else { | |
| warnings = append(warnings, fmt.Sprintf("Key %q has a casing conflict with another key; preserved with original casing", key)) | |
| } |
| result.Changed = false | ||
| result.Warnings = warnings | ||
| result.ExtraKeys = extraKeys | ||
| result.PreservedValues = preserved |
There was a problem hiding this comment.
When len(missingKeys) == 0 the function returns without ever setting result.MissingKeys. Because UpgradeResult is emitted as JSON (--upgrade-config-json), this will serialize MissingKeys as null rather than [], which can be an avoidable API inconsistency. Consider assigning result.MissingKeys = missingKeys (even when empty) before returning (same for any other early-return paths if needed).
| result.PreservedValues = preserved | |
| result.PreservedValues = preserved | |
| result.MissingKeys = missingKeys |
| for i := 0; i < len(templateLines); i++ { | ||
| line := templateLines[i] | ||
| trimmed := strings.TrimSpace(line) | ||
| if utils.IsComment(trimmed) { | ||
| newLines = append(newLines, line) | ||
| continue | ||
| } | ||
|
|
||
| key, _, _, ok := splitKeyValueRaw(line) | ||
| if !ok || key == "" { | ||
| newLines = append(newLines, line) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Template scanning currently skips all comment/blank lines (utils.IsComment(trimmed) / non-KEY lines) and only records the key’s line(s). As a result, when a key is missing and gets inserted, it will often be added without the surrounding section headers and explanatory comments from the template (see internal/config/templates/backup.env). If the upgrade experience is supposed to preserve template guidance, consider capturing the immediately preceding contiguous comment block as part of each templateEntry so inserted keys carry their documentation context.
Add handling for keys that differ only by case from template keys. Introduce CaseConflictKeys on UpgradeResult and populate it in computeConfigUpgrade, update warnings to note case-only differences, and preserve those keys in-place. Update CLI output (cmd/proxsave) to print case-conflict summaries for dry-run and upgrade flows. Adjust tests to assert CaseConflictKeys instead of treating them as ExtraKeys. Also clarify ExtraKeys are preserved in-place in comments.
Ensure UpgradeResult.MissingKeys is assigned in the branch where no missing keys are detected. Previously MissingKeys was left unset when len(missingKeys) == 0, which could lead to inconsistent result state for callers; this change sets it to the empty slice alongside other result fields in computeConfigUpgrade (internal/config/upgrade.go).
Translate Italian comments and user-facing text to English across the codebase and docs. Changes touch docs/RESTORE_GUIDE.md, internal/* files (backup/archiver.go, config/config.go, config/templates/backup.env, orchestrator/deps_test.go, orchestrator/extensions.go, pbs/namespaces.go, types/exit_codes.go). These are comment/text updates and template comment translations only; no functional logic was modified.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
1 participant
