carvel-dev / ytt

YAML templating tool that works on YAML structure instead of text
https://carvel.dev/ytt
Apache License 2.0
1.66k stars 136 forks source link

overlay/match's `by` does not accept a string when applied to a document #303

Open pivotaljohn opened 3 years ago

pivotaljohn commented 3 years ago

What steps did you take:

#@ load("@ytt:overlay", "overlay")
#@ load("@ytt:data", "data")
#@overlay/match by="kind"
---
kind: Deployment
spec:
  template:
    spec:
      containers:
      #@overlay/match by="name"
        - name: frontend
          image: #@ data.values.frontend.image

What happened:

Expected 'overlay/match' annotation keyword argument 'by' to be function, but was starlark.String

https://kubernetes.slack.com/archives/CH8KCCKA5/p1613490488283300?thread_ts=1613471663.281200&cid=CH8KCCKA5

What did you expect:

To behave identically to:

#@ load("@ytt:overlay", "overlay")
#@ load("@ytt:data", "data")
#@overlay/match by=overlay.map_key("kind")
---
kind: Deployment
spec:
  template:
    spec:
      containers:
      #@overlay/match by="name"
        - name: frontend
          image: #@ data.values.frontend.image

It's particularly surprising that this works for the array, but not the document.

Anything else you would like to add:

As a matter of practical use, there's a risk that the user beginning with this short-hand will select more documents than they intend. Seems like it would be worth noting this in documentation/examples that illustrate use of this type of parameter on documents.

Environment:

joaopapereira commented 3 years ago

As per the documentation in https://carvel.dev/ytt/docs/latest/lang-ref-ytt-overlay/#overlaymatch ytt should behave the same way for documents and arrays

aaronshurley commented 2 years ago

Labeling this issue as important-longterm as it's a bug that currently provides an unexpected behavior.