DE-2923: Enhance YAML support by allowing hydration of sequences (arrays)#6
DE-2923: Enhance YAML support by allowing hydration of sequences (arrays)#6suraj-swaroop-alida wants to merge 1 commit intomasterfrom
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Pull request overview
Enhances YAML hydration so secrets nested within YAML sequences (arrays) can be resolved during recursive traversal.
Changes:
- Add
[]interface{}handling to recurse into array elements that are maps. - Convert YAML-decoded
map[interface{}]interface{}elements inside arrays intomap[string]interface{}before hydration.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case []interface{}: | ||
| for i, item := range v { | ||
| switch elem := item.(type) { | ||
| case map[string]interface{}: | ||
| if err := ps.hydrateMapRecursively(elem, append(path, key)); err != nil { | ||
| return err | ||
| } | ||
| case map[interface{}]interface{}: | ||
| ee := map[string]interface{}{} | ||
| for k, val := range elem { | ||
| ee[k.(string)] = val | ||
| } | ||
| v[i] = ee | ||
| if err := ps.hydrateMapRecursively(ee, append(path, key)); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This sequence handling only recurses into elements that are maps; scalar elements (e.g., a YAML list of strings like ["$SECRET:foo"]) will be left unhydrated even though the PR goal is “hydration of sequences”. If you intend to support secrets in arrays, add handling for string elements (at least for the $SECRET:<param> form, since $SECRET/$$ have no per-item key to infer).
| ee := map[string]interface{}{} | ||
| for k, val := range elem { | ||
| ee[k.(string)] = val | ||
| } |
There was a problem hiding this comment.
The conversion of YAML-decoded maps inside sequences uses k.(string) for each key. YAML allows non-string keys, so this can panic at runtime if a key is not a string. Consider using a checked type assertion and returning a descriptive error (including the full path), or converting keys via fmt.Sprint(k) if non-string keys are acceptable here.
| case map[string]interface{}: | ||
| if err := ps.hydrateMapRecursively(elem, append(path, key)); err != nil { | ||
| return err | ||
| } | ||
| case map[interface{}]interface{}: | ||
| ee := map[string]interface{}{} | ||
| for k, val := range elem { | ||
| ee[k.(string)] = val | ||
| } | ||
| v[i] = ee | ||
| if err := ps.hydrateMapRecursively(ee, append(path, key)); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
When recursing into sequence elements, the path passed to hydrateMapRecursively does not include the element index. If hydration fails, the wrapped error will point to ...<key>.<field> without identifying which array item caused it. Consider including the index in the path element (e.g., fmt.Sprintf("%s[%d]", key, i)) to make errors actionable.
VojtechVitek
left a comment
VojtechVitek
left a comment
There was a problem hiding this comment.
LGTM.
Thanks for the nice throwback to my old gig. Good luck! 🎉
No description provided.