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] @overlay/replace via=... with non-scalars (maps, arrays, yaml docs) #61

Closed guludo closed 2 years ago

guludo commented 4 years ago

Hi!

The current documentation on @overlay/replace says that it is "valid for both map and array items", however I was able to successfully apply it to a whole document as well. For example:

#@ load("@ytt:overlay", "overlay")
#@overlay/match by=overlay.subset({'foo': 'bar'}), expects="0+"
#@overlay/replace
---
foo: "baz"
note: "I am replacing the YAML document!"

The ability of replacing the YAML document is useful to me. Can I safely use it? If so, I think it would be nice to update the documentation.

guludo commented 4 years ago

UPDATE:

After some further testing, it looks that this isn't fully supported. I was really interested in updating a document (or subdocument) with an updated function (using the via keyword). It does not work when the letf or right value is not scalar.

Could this functionality be added to YTT?

cppforlife commented 4 years ago

After some further testing, it looks that this isn't fully supported. ... It does not work when the letf or right value is not scalar.

yup, i didnt finish up that part of via. im curious what kind of processing you are trying to do?

guludo commented 4 years ago

I wanted to make it possible in my project to provide override files that could add stuff to the YAML document in function of some data already it in. For example, an overlay that adds envFrom to deployments container sections, where the referenced configmap would be <name>-env and <name>, the name of the deployment (i.e. metadata.name). I had thought of accomplishing that by writing a function that gets the name and adds the necessary stuff.

But now I'm taking another approach: I defined a function to create the deployments. Most of the boilerplate and things that depend on some parameters (like the deployments name, for example) are done in that function.

Nevertheless, having a way to reference the matched node could be useful in some cases - maybe not necessarily via replace, for example:

#@overlay/match by=overlay.subset({'kind': 'Deployment'}), expects="0+", var="d"
---
spec:
  template:
    spec:
      containers:
        #@overlay/match by='name'
        - name: #@overlay/expr d.metadata.name
          envFrom:
          - configMapRef:
              name: #@overlay/expr d.metadata.name + '-env'

This example proposes two additions:

  1. Addition of the keyword var to @overlay/match. It receives a string to be the name of the variable with the reference to the matched node. That variable would only be available down the tree of the right side value via the @overlay/expr annotation.
  2. Addition of the annotation @overlay/expr, which is similar to #@, but the expression is only evaluated during the processing of the overlays, in which case the variables defined via the keyword var in upper-level @overlay/matchs would be available for use.

I'm not totally familiar with ytt's processing model, but I believe that implementing this wouldn't be so straightforward. However, I think it would bring a lot of value to the overlaying functionality.

nimakaviani commented 4 years ago

thats an interesting idea. An alternative would be

#@overlay/match by=overlay.subset({'kind': 'Deployment'}, as='d'), expects="0+"

and populating the reference through that way.

guludo commented 4 years ago

@nimakaviani I think making it as a keyword of #@overlay/match instead of overlay.subset would allow us to use it with other matchers instead of just the subset one.

guludo commented 4 years ago

But I like using as instead of var.

cppforlife commented 4 years ago

updated docs about overlay/replace working with documents: https://github.com/k14s/ytt/commit/3c3c5af90bf561972ba102215345ed0331cea427

cppforlife commented 4 years ago

@guludo regarding your proposal (https://github.com/k14s/ytt/issues/61#issuecomment-543287778): im not quite sold on this feature yet as it's relies on passing data on the node itself. it might be ok for super simple things, but as soon as you need to pass through just one more piece of data (that doesnt exist on the node), you would suddenly have to add and later remove that data from the node. that feels a bit hacky to me.

guludo commented 4 years ago

@cppforlife yep, the proposal would cover simple use cases where you can derive things from data you would know to be in the node. For things that are not in the node itself it would probably make more sense to use #@data/values.

pivotaljohn commented 2 years ago

This issue has been years stale. We'll close this for now. However, if there's a renewed interest, we can re-open.