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 135 forks source link

[data/values] Add support for omitting "data.values" when referencing values #125

Open jessehu opened 4 years ago

jessehu commented 4 years ago

ytt uses data.values.myKey to reference a value and data.values. is always the prefix for the keys. So we can let ytt treats data.values. as the default prefix, and instead uses .myKey to reference the value.

BTW the first line #@ load("@ytt:data", "data") (AFAIK is always the same in every template files) can also be omitted and make it default by ytt.

#@ load("@ytt:data", "data")

#@ some_list = [data.values.myKey, "item1"]
pivotaljohn commented 4 years ago

If I understand correctly, @jessehu, there are two suggestions:

  1. provide syntactic sugar such that . implies data.values. (since this variable is typically referenced so often)
  2. include data as a built-in (since data values are used in most files)

We see a lot of value in this proposal:

Given that each new language feature introduces risk of unintended side-effects / surprises and that once such optimizations are introduced they are very difficult to remove, it's worthwhile to explore the idea before implementing:

jessehu commented 4 years ago

Thanks @pivotaljohn a lot for the details. 1) I see that the usage of leading '.' depends on Starlark to support this syntax, and seems it's not easy to implement it. It is possibly doable via pre-processing of the source template files, e.g. replacing . and (. with "data.values.", but as you said there might be surprises. This idea comes from the Helm Chart template language :

apiVersion: v1
kind: ConfigMap
metadata:
  name: {{ .Release.Name }}-configmap
data:
  myvalue: "Hello World"
  drink: {{ .Values.favoriteDrink }}

Are there other fields in the data objects? If not, simply data.mykey or values.mykey should work as well.

2) I realized #@ load("@ytt:data", "data") is used to load the data object, while #@data/values is used to indicate this is a data value yaml file. In Helm, '-f' is used the specify the data value yaml, while the templates dir is passed in via another CLI option, so they are separated. Making data or values as a keyword might work.

pivotaljohn commented 4 years ago

Are there other fields in the data objects?

There are two methods on the data object: list() and read().


Continuing to think out loud...

pre-processing

alternative mechanisms I'm also continuing to think through other mechanisms to achieve (some of) the readability we're going for, here.

jessehu commented 4 years ago

@pivotaljohn as you described, the pre-process by replacing the raw source code after '#@' is not a good way. The solution #@ load("@ytt:data", "data"); d=data.values looks great , and ytt should be able to automatically add d=data.values or values=data.values for each #@ load("@ytt:data", "data").

pivotaljohn commented 4 years ago

The solution #@ load("@ytt:data", "data"); d=data.values looks great...

Excellent. And reducing that verbosity is the lion's share of need. 👍

... ytt should be able to automatically add d=data.values or values=data.values for each #@ load("@ytt:data", "data").

Source generation seems like a behavior I'd expect from my editor rather than my language. We're not tackling verbosity in this part: it's avoiding typing that one line for each new template that uses data values.

jessehu commented 4 years ago

My expectation would be: avoid writing the boilerplate line #@ load("@ytt:data", "data"); values=data.values in all template files, and writing the boilerplate line #@data/values in all value files. However currently ytt is designed to use these directives to detect whether it's a template file or a value file.

I made a related proposal in Slack https://kubernetes.slack.com/archives/CH8KCCKA5: /p1587524076448700:

since --data-value specifies a single key=value , how about using a dedicated --values or --data-values like Helm to specify the path to one or more values yaml files. Then we can explicitly specifying the values yaml files and also there is no need to have the directive #@data/values in the values yaml files, because ytt can inject it automatically if needed. This is a more straightforward and easy to understand UX (especially for Helm users).

This proposal had been submmited in https://github.com/k14s/ytt/issues/51 as well.

pivotaljohn commented 4 years ago

My expectation would be: avoid writing the boilerplate line #@ load("@ytt:data", "data"); values=data.values in all template files, and writing the boilerplate line #@data/values in all value files.

I see agreement in the desire to avoid boilerplate. The debate is about which class of tool should own that responsibility.

This proposal had been submitted in #51 as well.

I want to be careful not to conflate these two cases.

  1. the UX of loading and referencing data values
  2. detecting whether a given file is a data values file or template.

They may have common characteristics (e.g. reducing boilerplate, making ytt more familiar to Helm users, ...), but the design choices are somewhat orthogonal.

This issue is squarely in the first concern.

jessehu commented 4 years ago

Exactly!

pivotaljohn commented 4 years ago

Okay:

==> let's table this discussion until after that work is complete.