fix(network): stop loading other yamls in .config on the same level#686
fix(network): stop loading other yamls in .config on the same level#686graham-chainlink merged 1 commit intomainfrom
Conversation
There is a bug where loading the network configs will load all yamls under `.config` instead of `.config/networks`.
🦋 Changeset detectedLatest commit: 73fadc4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where network configuration loading incorrectly scanned all YAML files under .config instead of limiting the scan to .config/networks. The fix ensures network configs are loaded only from the intended subdirectory.
Changes:
- Updated
loadNetworkConfigfunction to use.config/networksdirectory instead of.config - Removed recursive subdirectory scanning (
**glob pattern) to prevent loading unrelated YAML files - Updated comments and error messages to reflect the corrected directory path
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| engine/cld/config/networks.go | Changed directory path from .config to .config/networks and removed recursive glob patterns |
| .changeset/fuzzy-mice-arrive.md | Added changelog entry documenting the bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
| return nil, fmt.Errorf("cannot find config directory: %w", err) | ||
| } | ||
|
|
||
| // Find all yaml config files in the .config directory and any subdirectories |
There was a problem hiding this comment.
Should we still retain the subdirectories here? Not sure if this will affect any domains
There was a problem hiding this comment.
So what i learnt is that ** only matches 1 directory. Nested recursion never worked. ** is behaves exactly as *
If i have these files under .config/
[0 level(s)] direct.yaml
[1 level(s)] local/local.yaml
[1 level(s)] networks/networks.yaml
[3 level(s)] networks/sub1/sub2/deep.yaml
[2 level(s)] networks/subdir/nested.yaml
Total: 5 files
Files matched by ymlFiles, err := filepath.Glob(filepath.Join(configDir, "**", "*.yaml"))
✅ [1 level(s)] local/local.yaml
✅ [1 level(s)] networks/networks.yaml
Total: 2 files
❌ [0 level(s)] direct.yaml
❌ [3 level(s)] networks/sub1/sub2/deep.yaml
❌ [2 level(s)] networks/subdir/nested.yaml
Total: 3 files
My change basically just ensure instead of ** , we match networks
There was a problem hiding this comment.
playground: https://go.dev/play/p/nCsuVhH_Xzo
There was a problem hiding this comment.
Great thanks!I didn't know that
graham-chainlink
changed the title
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## chainlink-deployments-framework@0.76.1 ### Patch Changes - [#686](#686) [`1c0ef79`](1c0ef79) Thanks [@graham-chainlink](https://github.com/graham-chainlink)! - fix(network): stop loading all yamls in .config - [#687](#687) [`4f51470`](4f51470) Thanks [@friedemannf](https://github.com/friedemannf)! - Bump CTF to v0.13.9 --------- Co-authored-by: app-token-issuer-engops[bot] <144731339+app-token-issuer-engops[bot]@users.noreply.github.com>
…686) There is a bug where loading the network configs will load other yamls under `.config/XXX` instead of just `.config/networks`. There are already tests that checks reading from `.config/networks`.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## chainlink-deployments-framework@0.76.1 ### Patch Changes - [#686](#686) [`1c0ef79`](1c0ef79) Thanks [@graham-chainlink](https://github.com/graham-chainlink)! - fix(network): stop loading all yamls in .config - [#687](#687) [`4f51470`](4f51470) Thanks [@friedemannf](https://github.com/friedemannf)! - Bump CTF to v0.13.9 --------- Co-authored-by: app-token-issuer-engops[bot] <144731339+app-token-issuer-engops[bot]@users.noreply.github.com>
3 participants
There is a bug where loading the network configs will load other yamls under
.config/XXXinstead of just.config/networks.There are already tests that checks reading from
.config/networks.