fix(network): stop loading other yamls in .config on the same level #686
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
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.