Apply default env vars at configure-time rather than at save

[runleveldev]

Summary

Fixes #349.

Previously, admin-defined default environment variables were merged into a container's database record when the container was created. This had two undesirable effects:

1. The default env vars were revealed to the user in the edit UI.
2. The default values were frozen to whatever they were at creation time — later changes to the admin defaults never reached the container.

This PR changes the model so that:

1. The container record stores only user-defined environment variables.
2. Default env vars are merged with user-defined ones at configure-time (just before the Proxmox API call), with user-defined values taking precedence.

Design notes

There are three "default" sources; they are now handled at configure-time instead of being persisted:

• System defaults — admin-defined, stored in Settings.default_container_env_vars (the main subject of the issue).
• NVIDIA defaults (NVIDIA_VISIBLE_DEVICES, NVIDIA_DRIVER_CAPABILITIES) — derived from nvidiaRequested; previously baked into the saved record on create.
• Image-provided defaults — these are pulled into the create/edit form by the UI and submitted as part of the user-defined env vars, so they already live in container.environmentVars and need no special server-side handling.

Merge precedence (lowest → highest): system defaults < NVIDIA defaults < user-defined values.

Changes

• models/container.js
  • buildLxcEnvConfig(defaults = {}) now merges defaults (+ NVIDIA defaults) under the user-defined vars at configure-time.
  • New helpers: static getSystemDefaultEnvVars(), parseEnvironmentVars(), nvidiaDefaultEnvVars().
• bin/create-container.js
  • Merges system defaults at configure-time via the model helper.
  • Stops writing the merged env/entrypoint back into the DB record — the record keeps only user-supplied values.
• bin/reconfigure-container.js
  • Merges system defaults on edit, so newly added admin defaults now also reach existing containers (previously a gap — defaults were only ever applied at create).
• routers/api/v1/containers.js
  • Stops baking NVIDIA defaults into the saved record on create.

Verification

• node --check passes on all four modified files.
• Wrote and ran a standalone test of the merge semantics (precedence, NVIDIA override, delete handling, user-only persistence) — all assertions passed. (Temp file, not committed; no backend test framework exists in the repo.)

Notes / things to confirm in review

• There is no automated backend test suite, so this was validated by syntax checks + a throwaway logic test. Manual end-to-end testing against a real Proxmox node is recommended before merge.
• On the reconfigure path, env is fully replaced on each apply (via buildLxcEnvConfig's delete handling), so admin-removed system defaults now correctly disappear from existing containers on the next reconfigure.

[Copilot]

⚠️ Not ready to approve

There are correctness/safety edge cases in the new configure-time env/entrypoint handling (notably around input sanitization and entrypoint deletion semantics) that should be addressed before merging.

Pull request overview

This PR changes container environment variable handling so that only user-defined env vars are stored in the database, while admin/system defaults (plus NVIDIA-derived defaults) are merged at configure-time right before calling the Proxmox API—preventing default values from being revealed in the edit UI and ensuring default changes apply to existing containers on subsequent reconfigures.

Changes:

• Move default env-var merging into Container.buildLxcEnvConfig() with precedence: system defaults < NVIDIA defaults < user-defined.
• Stop persisting merged env/entrypoint values back into the container DB record during create.
• Ensure reconfigure path awaits the now-async env-config builder so existing containers pick up updated defaults.

File summaries

File	Description
create-a-container/routers/api/v1/containers.js	Stops baking NVIDIA defaults into persisted env vars on create.
create-a-container/models/container.js	Adds configure-time merging helpers and makes buildLxcEnvConfig() async with default precedence logic.
create-a-container/bin/reconfigure-container.js	Updates call site to await async env-config builder.
create-a-container/bin/create-container.js	Switches env/entrypoint application to model helper and stops writing merged values back into DB.

Copilot's findings

Comments suppressed due to low confidence (1)

create-a-container/models/container.js:130

• buildLxcEnvConfig() always adds entrypoint to the Proxmox delete list when container.entrypoint is unset. Since bin/create-container.js now always uses buildLxcEnvConfig(), creating (or later reconfiguring) a container with no user-provided entrypoint will actively remove any entrypoint already present in the cloned template/image config, which differs from the previous behavior (it preserved the existing entrypoint). Consider adding a way to preserve the current Proxmox entrypoint when the user hasn't explicitly overridden it (e.g., an option/parameter to buildLxcEnvConfig, or reading current config entrypoint in the job scripts and suppressing delete=entrypoint in that case).

      if (this.entrypoint && this.entrypoint.trim()) {
        config['entrypoint'] = this.entrypoint.trim();
      } else {
        deleteList.push('entrypoint');
      }

• Files reviewed: 2/4 changed files
• Comments generated: 3

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

[Copilot]

⚠️ Not ready to approve

create-time configuration can unintentionally wipe image/template-provided env/entrypoint by applying a delete-only LXC config when the DB record omits those fields.

Copilot's findings

• Files reviewed: 2/4 changed files
• Comments generated: 0 new

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

[runleveldev]

Addressed the create-time delete concern in 9135847.

buildLxcEnvConfig previously always added env/entrypoint to Proxmox's delete list when the container record had no value. Since the container is freshly cloned from a template on create, that delete actively wiped the template-provided env/entrypoint.

Fix: added a deleteMissing option (default false). Because Proxmox's config PUT is a partial update (omitted keys are left untouched, only delete-listed keys are removed):

• create (create-container.js) uses the default (false) — empty env/entrypoint are simply omitted, so whatever the cloned template provides is preserved. Per the discussion, the UI pre-fills env/entrypoint from the template anyway, so user-created containers still get explicit values; this only changes the "user supplied nothing" case to preserve-rather-than-wipe.
• reconfigure (reconfigure-container.js) passes deleteMissing: true so clearing the last env var or removing a custom entrypoint still unsets it on the existing container.

Per your call, I did not add a hard entrypoint-required guard on POST /containers — the deleteMissing default makes the omit-vs-delete behavior correct without forcing API clients to send an entrypoint.

Verified against the real model with a stubbed-Sequelize harness (18 assertions): create-with-empty now yields an empty config (no delete), reconfigure still deletes empties, and the existing restart-detection (which ignores the delete key) is unaffected.

[runleveldev]

Addressed the reconfigure-surprise concern in 086dfcc.

Problem: reconfigure uses deleteMissing: true, so env/entrypoint that lived only on the cloned template (and was never stored on the container record) would get unset on the first reconfigure — surprising the user, who saw those values in the edit UI. We can't recover them at reconfigure time: templates are mutable Docker refs that may change or disappear, and we don't cache them.

Compromise (per discussion): at creation, after the template is cloned, read its LXC config and fold its env/entrypoint into the container record as if the user had supplied them (precedence: template < user). This is essentially the old write-back, but scoped to template values only — the values we can't recompute later. It guarantees env/entrypoint stay stable across reconfigures without changing API-client expectations.

Still configure-time-only: system defaults (from Settings) and NVIDIA defaults are deliberately not persisted — buildLxcEnvConfig keeps merging them at configure-time. So those remain dynamic (admin changes reach existing containers) and are never frozen into the record or shown as user values in the edit UI. That preserves the core goal of #349.

Net effective precedence a container runs with: template ≤ persisted-record (template+user) then at configure-time system < NVIDIA < record. The only deviation from the ideal template < system < NVIDIA < user is that persisted template values now sit at the top alongside user values — an accepted trade-off given we can't reliably persist/requery templates.

Implementation:

• Container.parseLxcEnvString() — parse Proxmox's NUL-separated env into a normalized map.
• Container#persistTemplateDefaults(templateConfig) — merge template under user and persist.
• create-container.js calls it with the cloned template's config before applying the env config.

Verified against the real model with a stubbed-Sequelize harness (17 assertions): template-under-user precedence, entrypoint fallback to template, null handling, and that buildLxcEnvConfig after persist reflects the merged record with no delete.

[cmyers-mieweb]

Looks good for merge