fix(config): improve config override behavior

[nhedger]

Note

Disclosure:

• AI assistance was used to implement this feature
• Manual review and testing was done

Description

Fixes #737

This updates config resolution so environment variables override account credentials without discarding the rest of the selected profile.

Previously, when EXOSCALE_API_KEY and EXOSCALE_API_SECRET were set, the CLI returned early during config initialization and skipped loading the config file entirely. As a result, profile values such as defaultZone were ignored and commands relying on the account default zone could fall back to the built-in default instead of the zone configured in exoscale.toml.

This change now:

• loads and selects the config profile first when a config file is available
• applies environment overrides afterwards for credentials and related override fields
• preserves profile settings such as defaultZone and other account defaults
• keeps the existing env-only behavior when no config file is present
  This fixes the override precedence so env credentials replace authentication settings, not the whole profile.

Checklist

(For exoscale contributors)

• Changelog updated (under Unreleased block, and add the Pull Request #number for each bit you add to the CHANGELOG.md)
• Testing

Testing

• Ran go test ./...
• Ran targeted e2e coverage for the config/env precedence:
  • go test -run 'TestScriptsLocal/(show_with_env_credentials_keeps_config_defaults|show_with_env_credentials_without_config)$' -count=1 ./... from tests/e2e
• Verified that:
  • env credentials still work without a config file
  • defaultZone from the config is preserved when env credentials are set
• Confirmed that [Bug]: Default zone is not respected #737 cannot be reproduced anymore:

  export EXOSCALE_API_KEY="..."
  export EXOSCALE_API_SECRET="..."
  VOL_ID=$(./bin/exo --output-format json compute block-storage create repro-737-$(date +%s) --zone ch-gva-2 --size 10 | jq -r '.id')
  ./bin/exo compute block-storage list
  ./bin/exo compute block-storage show "$VOL_ID"
  ./bin/exo compute block-storage show "$VOL_ID" --zone ch-gva-2
  ./bin/exo compute block-storage delete "$VOL_ID" --zone ch-gva-2 -f

[imiric]

Hi, thanks for the stream of improvements and fixes you've contributed! It's much appreciated 🙇 I also appreciate the additional tests, and disclosure of "AI" use. 👍

I confirmed that this PR fixes the issue described in #737, which is great. However, I'm not sure if this is the right approach. 🤔

I don't think implementing manual overrides like this is a robust or maintainable solution. The canonical way to ensure that values from CLI flags, env vars, config files, and other sources, have a correct and predictable order of precedence within a Cobra application is to use the related Viper library, which has a commonly accepted precedence order.

We currently only use it for reading the config file, and not much else. So the previous code wasn't great either, but I think we should leverage Viper more instead of doing this overriding ourselves. This way we can align with a common convention, it can work for other values besides the ones we specified here (e.g. there is interest in supporting EXOSCALE_ZONE), and it should likely be less code for us to maintain, and therefore hopefully less chances for surprising behavior and bugs.

For example, even on this branch, the EXOSCALE_CONFIG env var takes precedence over the --config flag, which shouldn't be the case.

I'm not sure how difficult it would be to partially improve our usage of Viper to fix this particular issue without having to refactor the rest of the code base. Ideally that could get us on the right path to expand the improvement for other commands and values later.

So I don't want to block this from merging, but would strongly suggest revisiting this fix with the above in mind. I'll let others from @exoscale/giza chime in.

[nhedger]

Thanks for your review. I wasn't sure if you'd want such a refactor to begin with, so I went for the quick win, but I agree it would probably be more robust to use Viper.

I'm happy to explore an alternative way using Viper. Just let me know if you you still want to merge this in the meantime, in which case I'd create another PR for te Viper refactor

Cheers

[natalie-o-perret]

Hey there,

@imiric, you're right that the manual override pattern isn't great long-term, and the EXOSCALE_CONFIG vs --config catch is a good one.

@nhedger, before going down the Viper path for the follow-up, worth noting that (based on my understanding of) Viper's precedence model maps flat key-value pairs. It doesn't natively handle our "select a profile from a list, then overlay env vars on top of that profile's fields" pattern. Imho, we might still need custom merge logic for the multi-account model; the question is just whether Viper or plain os.Getenv feeds into it.

What I think could be a wee more sustainable (in a dedicated follow-up PR) is separating source loading from resolution: read env vars, config file, and flags into discrete structs independently, then merge them with a single explicit-precedence function. That would make the rules visible as code and easy to unit-test. Adding something like EXOSCALE_ZONE later becomes one new field instead of scattered if-chains.

The envAccountOverrides struct in this PR is actually a decent first step toward that pattern.

---

Good to merge for the #737 fix, it still fixes a real bug.

That said, the config resolution logic could really use a rethink / revamp.

Happy to help shape that.