Add lookup function #32
Conversation
pkg/util/template.go
Outdated
|
|
||
| // lookup does a dot-separated path lookup on the input struct or map. If the input or any of | ||
| // its children on the targeted path are not a map or struct, it returns nil. | ||
| func lookup(path string, input interface{}) interface{} { |
There was a problem hiding this comment.
The get function of Sprig receives the input as first argument I believe, which reads with the more naturally as get <object> <field>.
There are pros and cons to either ways, taking the input as last argument helps in the pipeline form where the output of the previous command is passed as last argument. I'm a bit concerned that we may trick everyone into getting it wrong if get and lookup have swapped signatures tho.
How about offering both maybe?
func lookup(input interface{}, path string) interface{} {
...
}// naming is hard
func select(path string, input interface{}) interface{} {
return lookup(input, path)
}There was a problem hiding this comment.
Oh, interesting, for some reason I thought I was following the get convention. Will fix it up.
There was a problem hiding this comment.
Fixed. For the alternative, named it pathLookup since select is a golang keyword.
| for i := 0; i < len(components); { | ||
| component := components[i] | ||
|
|
||
| switch obj.Kind() { |
There was a problem hiding this comment.
It looks like Sprig made the opinionated choice to use map[string]interface{} only https://github.com/Masterminds/sprig/blob/39e4d5d0e0d566a256746e59749821155f209d11/dict.go#L8
Do you know if we may ever run into cases where a struct is passed to this function?
There was a problem hiding this comment.
Map only is fine. I was trying to be super-flexible but probably unnecessary :-).
pkg/util/template.go
Outdated
| "urlEncode": url.QueryEscape, | ||
| } | ||
|
|
||
| zeroValue = reflect.Value{} |
There was a problem hiding this comment.
Using reflect.Value.IsValid is usually the preferred way to check I believe.
There was a problem hiding this comment.
Oh, nice. Fixed.
| obj = obj.MapIndex(reflect.ValueOf(component)) | ||
| i++ | ||
| case reflect.Ptr, reflect.Interface: | ||
| // Get the thing being pointed to or interfaced, don't advance index |
There was a problem hiding this comment.
I'm under the impression that we may have to check reflect.Value.IsNil before we dereference the pointer, otherwise the code could panic.
There was a problem hiding this comment.
Added a check here.
pkg/util/template.go
Outdated
| obj = obj.Elem() | ||
| default: | ||
| // Got an unexpected type | ||
| return nil |
There was a problem hiding this comment.
Would a panic be more appropriate here? It seems like we'd be using the function really wrong if we run into this case.
Alternatively, we could return the value itself.
There was a problem hiding this comment.
Yeah, so there are two cases here- one that's an error and one that isn't. Separated them and updated the function to return an error if the user tries to traverse a non-map type.
|
Thanks for adding this! |
Description
This change adds a new kubeapply template function,
lookup, that can be used to look up a dot-delimited path in a map or struct. If the path exists, it returns the value, otherwise it returnsnil. Adding this because the spriggetfunction doesn't support looking more than one level down.