Reorder config loading to load config and *then* fill in missing defaults

[chelog]

Quoting the original PR:

This PR changes the order of operations when loading the config.yml

Currently when wings starts up, it starts with an empty Configuration struct that is then filled with the defaults and THEN we overlay the data from the config.yml using yaml.Unmarshal(). This overwrites any values in the struct with any values that exist in the config.yml

The issue is when encountering an array/list/map yaml.Unmarshal() will merge the values from the config.yml with the defaults. Such as shown in pterodactyl/panel#5008

By changing the order so that defaults.Set() is called after the struct has been filled with the data from config.yml, only missing keys will be filled

This would resolve pterodactyl/panel#5008

Summary by CodeRabbit

• Bug Fixes

  • Default configuration values are now applied when loading from a file, ensuring consistent behavior for file-based configurations.
  • Newly created configurations no longer auto-populate defaults at creation; defaults are applied when a config is loaded from disk, which may affect workflows relying on immediate defaults.

• Documentation

  • Clarified when configuration defaults are applied to align expectations for file-loaded vs. newly created configurations.

[coderabbitai]

Walkthrough

NewAtPath no longer applies default values when creating a config; FromFile now applies defaults after unmarshalling YAML. Comments were updated to indicate defaults are applied on file load rather than at construction time.

Changes

Cohort / File(s)	Summary of Changes
Config defaults application shift config/config.go	Removed defaults.Set(&c) from NewAtPath. Added defaults.Set(c) after YAML unmarshal in FromFile. Updated comment blocks to indicate defaults are applied during file loading, not during construction.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant Config as config.NewAtPath
  Note over Config: NewAtPath (after change)
  Caller->>Config: NewAtPath(path)
  activate Config
  Config-->>Caller: Config{Path: path} (no defaults applied)
  deactivate Config

Loading

sequenceDiagram
  autonumber
  participant Caller
  participant Loader as config.FromFile
  participant FS as Filesystem
  Note over Loader: FromFile (after change)
  Caller->>Loader: FromFile(path)
  activate Loader
  Loader->>FS: Read file
  FS-->>Loader: YAML bytes
  Loader->>Loader: Unmarshal YAML into Config
  Loader->>Loader: Apply defaults.Set(c)
  Loader-->>Caller: Config with defaults applied
  deactivate Loader

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibble at lines where defaults once lay,
Now born from files, the rules hold sway.
I hop and set what YAML leaves behind,
A tidy config, gentle and kind. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly and accurately describes the main change of loading the configuration first and then applying missing defaults, directly reflecting the core intent of the pull request without unnecessary detail or ambiguity.
Linked Issues Check	✅ Passed	The pull request flips the default application to occur after unmarshalling the YAML, which prevents default docker log driver options from being merged into a user-provided fluentd configuration and thus directly resolves the behavior described in issue #5008.
Out of Scope Changes Check	✅ Passed	All changes are confined to the configuration loading logic in config/config.go and pertain solely to the order of default application, with no modifications outside the scope of addressing the linked issue’s objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d2901f and 284d7a7.

📒 Files selected for processing (1)

• config/config.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• config/config.go

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 48951de and 1d2901f.

📒 Files selected for processing (1)

• config/config.go (1 hunks)

🔇 Additional comments (1)

config/config.go (1)

368-373: LGTM!

The removal of defaults.Set() from NewAtPath is correct. The constructor now only sets the path, and defaults are applied later in FromFile after YAML unmarshaling. This prevents unintended merging of arrays/maps from the config file with default values.

[chelog]

[QuintenQVD0]

You know they reverted it as it broke config generation:
pterodactyl/wings@7bb7696

[chelog]

Sorry, haven't followed that thread. I am still having a problem with setting log_config. Guess I'll have to open an issue as I have no experience in Go to make a PR myself. Thank you for the heads up!