fix(config/clouds): find the clouds config file independently of each… - #3983
fix(config/clouds): find the clouds config file independently of each…#3983AntoninRuan wants to merge 1 commit into
Conversation
github-actions
Bot
added
edit:config
|
coverage: 63.585% (-0.003%) from 63.588% — AntoninRuan:indepedent_find into gophercloud:main |
Sharpz7
left a comment
Sharpz7
left a comment
There was a problem hiding this comment.
I found two issues with the new independent file lookup behavior.
Assisted-By: Codex
|
|
||
| if options.secureyamlReader != nil { | ||
| var secureClouds Clouds | ||
| if err := yaml.NewDecoder(options.secureyamlReader).Decode(&secureClouds); err != nil { |
There was a problem hiding this comment.
WithCloudsYAML only sets the default reader, so this call falls through to filesystem discovery for secure.yaml. That means an in-memory config can unexpectedly pick up a same-named entry from the current directory or XDG config. Can we avoid filesystem lookup unless a secure reader/location was explicitly provided?
There was a problem hiding this comment.
I am adding a test to detect it and I'll see if I have time to change it in the week
| if options.cloudsPublicyamlReader == nil { | ||
| if len(options.publicLocations) < 1 { | ||
| func readClouds(options *cloudOpts, ctype CloudsType) (map[string]Cloud, error) { | ||
| // Set the defaults and open the files for reading. This code only runs |
There was a problem hiding this comment.
WithLocations only sets locations[Default], so the secure lookup does not check beside the selected clouds.yaml. Existing callers using /custom/clouds.yaml and /custom/secure.yaml will silently lose the secure values. Can we preserve the companion lookup, or add a separate secure-file location option and update the docs?
There was a problem hiding this comment.
Maybe we could make WithLocation also update the secure.yaml location file to keep old behavior. And have independent find only when no WithLocation is set.
That would mean we match openstack-cli when no options are passed. When only WithLocation is given we preserve old behavior and when both WithLocation and WithSecureLocation are given WithSecureLocation overrides the secure location.
Would you be ok with that?
c68af9b to
5e4f1c3
Compare
5e4f1c3 to
eb568a6
Compare
3 participants
Opening the PR as a follow up of this comment: #3949 (comment). (@stephenfin)
Change should make config files loading independent of each others as openstacksdk does