Repository navigation
feat(config): merge per-provider YAML into a single onctl.yaml - #930
Merged
Merged
Conversation
Replace gcp.yaml/azure.yaml (and the previously-removed hetzner.yaml/ aws.yaml/fc.yaml/onctl.yaml) with one onctl.yaml written by `onctl init`, holding every provider's settings pre-filled with working defaults under its own commented section. CLI flags still override it. Drop the ~45-line viper.SetDefault() block in cmd/common.go now that the YAML file is the single source of truth for defaults; `onctl init` is required again (initState() is fatal if onctl.yaml can't be loaded, for every provider, matching pre-zero-yaml-flags behavior).
![chatgpt-codex-connector[bot]](/p/https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5dd9b82e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The test called initializeOnctlEnv() without overriding HOME, so on a clean runner it created a real ~/.onctl directory. Under Go's test shuffling that leaked into later tests (e.g. TestReadConfig_NoConfigDirectory) which assert no config directory exists, causing flaky CI failures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RpWaEWJM7WzPajfLGjjQSu
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #930 +/- ##
==========================================
- Coverage 22.98% 22.58% -0.41%
==========================================
Files 42 42
Lines 4194 4176 -18
==========================================
- Hits 964 943 -21
+ Misses 3115 3114 -1
- Partials 115 119 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… files initializeOnctlEnv treated any existing .onctl directory as fully initialized, even one created before the single-onctl.yaml config that only had per-provider yaml files. That left ReadConfig permanently failing with "no configuration directory found" since onctl.yaml was never written. Now both init and every ReadConfig call populate a missing onctl.yaml and print a warning when legacy per-provider files (aws.yaml, azure.yaml, fc.yaml, gcp.yaml, hetzner.yaml) are present, since their settings are silently ignored by the new single-file config. Also isolates HOME in TestInitializeOnctlEnv_Coverage, a second instance of the same un-isolated-HOME bug fixed for TestInitializeOnctlEnv_NewDirectory in the previous commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
gcp.yaml/azure.yaml(and the previously-removedhetzner.yaml/aws.yaml/fc.yaml/onctl.yaml) with a singleonctl.yamlwritten byonctl init, holding every provider's settings pre-filled with working defaults under its own section. CLI flags still override it.viper.SetDefault()block incmd/common.gonow that the YAML file is the single source of truth for defaults.onctl initis required again:initState()is fatal ifonctl.yamlcan't be loaded, for every provider, matching pre-zero-yaml-flags behavior.Test plan
go build ./...go vet ./...golangci-lint run ./...(0 issues)go test ./...(all pass)onctl init" erroronctl init,onctl listworks against real Hetzner credentials with no other config--type/--location/--username(shared flags backing different per-provider defaults, e.g.hetzner.vm.type=cpx21vsaws.vm.type=t2.micro) still resolve correctly purely from the YAML's provider sectionshttps://claude.ai/code/session_01PTkspGQd3qXJbJaxqaoMSu