ytt icon indicating copy to clipboard operation
ytt copied to clipboard

[lang] support emitting comments in resulting YAML

Open cppforlife opened this issue 6 years ago • 39 comments

originally @patricknelson added as a comment to related issue: https://github.com/k14s/ytt/issues/14#issuecomment-542965903:

It's not always essential for the resulting YAML to have comments for me, but it certainly would be very helpful!

What if we were to use a double # to work as an escape method when at the beginning of a line, so that the resulting YAML will contain a standard comment? For example, taken from the main demo at https://get-ytt.io:

#@ def labels():
#! This is a ytt comment
## This is a yaml comment
app: echo
org: test
#@ end

kind: Pod
apiVersion: v1
metadata:
  name: test-comments
  labels: #@ labels()

With the output resulting in:

kind: Pod
apiVersion: v1
metadata:
  name: echo-app
  labels:
    # This is a yaml comment
    app: echo
    org: test

I'm brand new to ytt (just doing research right now), so I'm asking just in case this isn't already available with first class syntax support without any special flags.

cppforlife avatar Oct 17 '19 15:10 cppforlife

It's not always essential for the resulting YAML to have comments for me, but it certainly would be very helpful!

that would be helpful in certain cases for sure.

What if we were to use a double # to work as an escape method when at the beginning of a line

that would work from a syntax perspective. minor concern i would have is if someone accidentally throws in annotations like ##@something thinking it's an annotation, but really would be annotation in a resulting YAML.

we currently use plain YAML library to implement serialization and it doesnt support emitting YAML comments; however, i was also playing around with ytt fmt command (experimental feature; https://github.com/k14s/ytt/issues/13) which has a custom YAML serializer that knows how to deal with comments. i did have some plans to switch to that one as it gets more mature.

cppforlife avatar Oct 17 '19 15:10 cppforlife

That's entirely valid, especially since that's an easy typo to make. For me it'd be fine to emit a highly visible notice (or fatal parse error?) to STDERR when that special combination is encountered. Specifically, this notice would be triggered if the parser sees ##@ at the start of a new line.

I understand this is due to the ambiguity introduced from the [albeit positive] fact that YAML comments are used for ytt's DSL. Can you think of any other edge cases where this syntax could cause issues? Or alternatively, any better syntaxes? Perhaps this should be converted to an RFC for others to discuss (as I'm not the only user).

patricknelson avatar Oct 18 '19 00:10 patricknelson

Or alternatively, any better syntaxes?

currently #! foo is a shorthand to #@comment foo. we could use something like #@yaml/comment foo to emit resulting yaml comments. this will fit nicely with other yaml/... annotations we already support: https://github.com/k14s/ytt/blob/develop/docs/lang-ref-annotation.md#yaml-templating-annotations

Perhaps this should be converted to an RFC for others to discuss (as I'm not the only user).

we currently dont have more advanced proposal process beyond dedicated github issues+comments (and slack for live discussion). imho it works ok for now.

cppforlife avatar Oct 18 '19 17:10 cppforlife

Using #@yaml/comment foo sounds good to me from a structural/organizational standpoint. To make it more accessible (since that's still pretty verbose) can ## foo still be a shorthand for it? 😄 That's of course taking into account the validation against the potential ##@ typo.

patricknelson avatar Oct 19 '19 02:10 patricknelson

can ## foo still be a shorthand for it?

i would imagine it would be not be used that often, hence shorthand would not be necessary. how frequently do you think you'll be adding comments to resulting YAML?

cppforlife avatar Oct 21 '19 13:10 cppforlife

Maybe not that often. My latest workflow is primarily focused on the ytt template files and not so much on the yaml that they generate. There may be others with workflows where they commit or otherwise retain long term the generated yaml with the intent for it to be readable later and, if that's the case, having comments will make sense.

So I cannot argue for it strongly since I may not use it much now, myself. Being a new user, I thought I might need it, but it turns out that it'll be of fairly low importance (and wouldn't use it at all if it had that verbose of syntax, actually).

patricknelson avatar Oct 22 '19 03:10 patricknelson

👍 let keep the issue open for now, and let's see if anyone else has a use case for this feature.

cppforlife avatar Oct 22 '19 16:10 cppforlife

Hi, I'm trying to generate a cloud config file: https://cloudinit.readthedocs.io/en/latest/topics/format.html#cloud-config-data Its first line needs to be #cloud-config. Is there a way to achieve this now?

smg8fe avatar Nov 07 '19 08:11 smg8fe

@smg8fe for your use case you can use something like this for now, echo '#cloud-conifg'; ytt -f .

cppforlife avatar Nov 07 '19 16:11 cppforlife

Guys, yaml is yaml. IMO it would be best if ytt default behaviour does not fail when there is a comment in the input yaml file:

#@ def labels():
#! This is a ytt comment
## This is a comment in the yaml output
# And this is just a comment in this file

PyMeH avatar Nov 27 '19 11:11 PyMeH

@PyMeH our thinking was to help catch mistakes by authors if they forget to add @ after '#' (since IDEs for example dont help to add it). there is currently --ignore-unknown-comments to allow anything to pass through. do you think opting in for a flag is unnecessary?

cppforlife avatar Nov 27 '19 16:11 cppforlife

This makes me wonder @cppforlife, is there the ability to flag the file (maybe in the header at the very top) to enable this functionality? That or maybe some code-based configuration so that we can keep it "git ops". Since the code is already ytt capable (i.e. intentionally passed into ytt and likely including #@ directives and etc) then presumably someone who uses that special header/flag at the top of the file, and of course the --ignore-unknown-comments flag, will expect that functionality.

That way we can keep it all in-code and clearly defined as a preference somewhere.

patricknelson avatar Nov 27 '19 21:11 patricknelson

Yes. In my head ytt extends yaml. Think of OOP - ytt should follow the yaml contract. Talking about polymorphism- ytt format should be processible by every yaml tool. An analogy - json and json-ld formats.

Regarding —ignore-unknown-comments - it should either be reversed, e.g —fail-on-yaml-comments or (IMO the better approach) should go away.

My 2c

On 27 Nov 2019, at 18:32, Dmitriy Kalinin [email protected] wrote:

 @PyMeH our thinking was to help catch mistakes by authors if they forget to add @ after '#' (since IDEs for example dont help to add it). there is currently --ignore-unknown-comments to allow anything to pass through. do you think opting in for a flag is unnecessary?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

PyMeH avatar Nov 29 '19 09:11 PyMeH

@smg8fe for your use case you can use something like this for now, echo '#cloud-conifg'; ytt -f .

Please don't suggest hackish workarounds like this. I, too, am trying to do something more robust that Jinja templating to get values from external variables into a YAML file, in this case also a cloud-init that expects #cloud-config at the start of the file.

In this use case, I am trying to make it easier for someone with little programming experience to preconfigure a Raspberry Pi using a cloud-init file. I began using Jinja templating, which you yourself pointed out was inadequate in your IBM tech talk (where I learned about ytt in the first place.) Others are even more vocal about not using Jinja to template YAML. (https://leebriggs.co.uk/blog/2019/02/07/why-are-we-templating-yaml.html)

The concept of templating is hard enough, so new users insert hard-coded passwords into a YAML file, or worse, just use the default password that came in the cloned repo. It is already going to be a hard sell for me to get them to use ytt templating. Requiring additional scaffolding in a Makefile or extra scripts or complicated command lines to force an initial line in a file complicates the pipeline and the additional friction may scare them off completely. To me, that's a lose/lose situation.

I've been trying to find a supported way to include the string using existing ytt features (like text templating, etc.) While I think the text templating might be working, all attempts fail with the error message:

ytt: Error: Unmarshaling YAML template 'rpialgo.yml': yaml: line 4: did not find expected <document start>

I can only make ytt more strict with -s, but not less strict. Is it really necessary to take such a strongly opinionated view of what a "valid" YAML file is?

Not providing a means of working around this issue (besides concatenation at the shell) does not seem to be user-friendly or robust.

Can you just add an option to select what the "expected document start" string should be? I would be happy to just add --document-start '#cloud-config' to work around this.

Thanks!

davedittrich avatar Apr 18 '20 00:04 davedittrich

This would be super useful! We're starting to explore using ytt to process and create yaml documents that are meant for human consumption. Being able to pass comments through via ## would be fantastic so we can have inline documentation for the files created.

This exact use case is having a bunch of shorter yaml docs in source that can be composed together to create a valid values.yaml file for a specific environment for documentation purposes.

paulczar avatar Feb 01 '21 19:02 paulczar

The use case of using ytt for something like a cloud-config is especially challenging here. What is the appropriate way to engage here to get some focus on this use case from the maintainers here? I think there are valid use cases that would be very helpful for the project to consider here.

jmcshane avatar Apr 12 '21 14:04 jmcshane

see if anyone else has a use case for this feature

I found this issue looking for a way to tag my generated files with headers like

# generated by ytt, do not modify
# author: 
...

dmarmugi avatar Jul 21 '21 19:07 dmarmugi

I don't know if this belongs in here or could be a separate issue, but we're finding ourselves doing more and more stuff like this:

apiVersion: v1
kind: Secret
metadata:
  name: tbs-values
  namespace: kapp-controller
stringData:
  overlay-options.yaml: |
    #@ load("@ytt:overlay", "overlay")
    #@ load("@ytt:data", "data")

    #@ def sync_canonical_secret():
    apiVersion: v1
    kind: Secret
    metadata:
      name:  canonical-registry-secret
    #@ end

    #@overlay/match by=overlay.subset(sync_canonical_secret()),when="1+"
    ---
    metadata:
      #@overlay/match missing_ok=True
      labels:
        #@overlay/match missing_ok=True
        com.vmware.tanzu.buildservice.sync: "true"

Which is cutting down on readability pretty significantly, it would be really nice to be able to pull this out into a yaml fragment like we do with non-ytt overlay config files:

#@ def dex_config():
issuer: #@ "https://login.sso.{}".format(config.domain)
frontend:
  theme: tkg
web:
  http: 0.0.0.0:5556
...
#@ end

stringData:
  config.yaml: #@ yaml.encode(dex_config())

voor avatar Jul 31 '21 12:07 voor

If I understand this correctly, there is currently no way to force ytt to emit yaml # comments into the output yaml. For the use case where ytt is being used to generate new human-readable yaml this is a problem.

E.g. App Accelerator templates can invoke ytt on input yaml files, to generate modified output yaml files for a new project. The value of those output files is significantly impacted by a lack of comments.

jldec avatar Sep 23 '21 14:09 jldec

This would be a huge benefit for us to have this option. We have a few use cases currently impacted by this. The main one is using ytt in app accelerator and we need comments which isnt possible. The other is a bit more advanced but is using ytt to generate ytt templates. This also may be relevant down the road in a system like app accelerator but currently i use a bunch of sed and awk stuff to generate ytt templates that could much more easily and cleanly be done via ytt generating them. As ytt is # based we need to be able to print out ytt directives in the output of the original ytt

vrabbi avatar Sep 23 '21 15:09 vrabbi

We try to bootstrap new repositories with templates that follow common patterns as "getting started" -- we do this mostly with sed scripts now, but it comes up extremely often it seems silly to use sed to create ytt templates when ytt would excel in that use case.

voor avatar Sep 23 '21 16:09 voor

It stinks that we're still stuck at not being able to simply emit comments in ytt output.

@pivotaljohn what do you think: Could we at the very least have some kind of esoteric syntax like @cppforlife's suggestion above https://github.com/vmware-tanzu/carvel-ytt/issues/63#issuecomment-543839685 just to make it possible? e.g. #@yaml/comment foo. Presumably this is fully backward compatible first class ytt syntax that shouldn't be a breaking change and could even be a minor/dot release.

... and then maybe discuss having a more intuitive shorthand syntax like ## foo (hash-hash-space)? 😉😉 nudge nudge...

patricknelson avatar Sep 23 '21 21:09 patricknelson

This may be sacrilege, but: I’m actually dropping ytt entirely now in favor of kustomize since I’m finding that it’s actually much better suited for my use case anyway (building complex Kubernetes manifests). If anybody here is encountering issues while using ytt and happen to have a similar use case, I’d encourage you to give kustomize a shot as well. It’s just a completely different approach, but potentially much simpler (i.e. use of declarative overlays/components/generators instead of templates with parameterization and side-effects).

Ironically, it doesn’t include comments in resulting YAML, but it’s generally not necessary at all. 😊

patricknelson avatar Oct 03 '21 04:10 patricknelson

Hey @patricknelson. Sorry for the delay. Thanks for letting us know where you're at.

It's unfortunate that we haven't been able to make more progress on this issue. We're constantly prioritizing where we invest and haven't had the bandwidth to include this (albeit useful) suggestion.

We're really big on folks using the right tool for the job. 👍🏻 Kustomize can be great for straight-forward patching and is well integrated into the Kubernetes tooling.

ytt itself specifically includes both templating and overlaying/patching capabilities deliberately to cover a wider range of use-cases:

  • overlays are great for aspects of configuration (e.g. ensure all resources have a particular namespace; all containers have certain resource limits; etc.)
  • overlays allow the cluster operator to make "last minute" adjustments to k8s manifests
  • ytt overlays are written in the same structure as the documents they target (other tools use JSON Patch syntax)
  • templates tend to be easier to reason about since they explicitly indicate where values are being inserted
  • data values can be thought of as a formal API of anticipated configuration values.

Keeping this open as we haven't decided not to implement this feature. In the meantime, we would absolutely welcome a contribution for implementing the @yaml/comment annotation.

pivotaljohn avatar Oct 05 '21 17:10 pivotaljohn

@vrabbi noted, earlier:

This would be a huge benefit for us to have this option. We have a few use cases ... using ytt to generate ytt templates. ... currently i use a bunch of sed and awk stuff to generate ytt templates that could much more easily and cleanly be done via ytt generating them.

Another concrete example of this arose, recently: where a template evaluated at build time...

A Carvel user wanted to template their PackageInstall CR so that they could include it in a collection of packages to be installed as a larger system:

Build Step: during "build time", populate some fields with values determined by the published of the package:

telemetry-package-install.yml

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

#@ #@ load("@ytt:data", "data")
#@ #@ load("@ytt:yaml", "yaml")
#@ #@ load("_profiles.star", "profiles")

#@ #@ if profiles.is_any_enabled([profiles.full]):
#@ #@ if profiles.is_pkg_enabled("telemetry.platform.com"):

---
apiVersion: packaging.carvel.dev/v1alpha1
kind: PackageInstall
metadata:
  name: #@ data.values.name
  namespace: platform-install
spec:
  serviceAccountName: #@ data.values.serviceAccount
  packageRef:
    refName: #@ data.values.name + "." + data.values.domain
    versionSelection:
      constraints: #@ data.values.version
      prereleases: {}
---
apiVersion: v1
kind: Secret
metadata:
  name: telemetry-values
  namespace: platform-install
stringData:
  values.yml: #@ #@ yaml.encode(data.values.telemetry)

#@ #@ end
#@ #@ end

... such that the lines that start with #@ #@ ... are desired to pass through to the output:

#@ load("@ytt:data", "data")
#@ load("@ytt:yaml", "yaml")
#@ load("_profiles.star", "profiles")

#@ if profiles.is_any_enabled([profiles.full]):
#@ if profiles.is_pkg_enabled("telemetry.platform.com"):

---
apiVersion: packaging.carvel.dev/v1alpha1
kind: PackageInstall
metadata:
  name: telemetry
  namespace: platform-install
spec:
  serviceAccountName: telemetry-acct
  packageRef:
    refName: telemetry.platform.com
    versionSelection:
      constraints: 1.1.0
      prereleases: {}
---
apiVersion: v1
kind: Secret
metadata:
  name: telemetry-values
  namespace: platform-install
stringData:
  values.yml: #@ yaml.encode(data.values.telemetry)

#@ end
#@ end

Install Step: while the system is being installed, the system operator provides installation values to complete the PackageInstall CR:

---
apiVersion: packaging.carvel.dev/v1alpha1
kind: PackageInstall
metadata:
  name: telemetry
  namespace: platform-install
spec:
  serviceAccountName: telemetry-acct
  packageRef:
    refName: telemetry.platform.com
    versionSelection:
      constraints: 1.1.0
      prereleases: {}
---
apiVersion: v1
kind: Secret
metadata:
  name: telemetry-values
  namespace: platform-install
stringData:
  values.yml: |
    apiVersion: observability/v1alpha1
    kind: TelemetryConfig
    spec: 
      ...

pivotaljohn avatar Dec 08 '21 00:12 pivotaljohn

A potentially related example: the ability to — from within a template — take a YAML Fragment and encode it (complete with its annotations) ... Being able to do so means that such values (e.g. a k8s Secret who's data is an overlay) could be closer to being editable (overlay-able) as well... 🤔

https://kubernetes.slack.com/archives/CH8KCCKA5/p1652468285086519?thread_ts=1652465480.622579&cid=CH8KCCKA5

pivotaljohn avatar May 13 '22 19:05 pivotaljohn

Friendly bump, this still is strongly desired.

voor avatar Nov 03 '22 20:11 voor

i would vote for simply allowing # as comment see my comment here https://github.com/carvel-dev/ytt/issues/14#issuecomment-1407066137

c33s avatar Jan 27 '23 20:01 c33s

i would vote for simply allowing # as comment see my comment here #14 (comment)

While this is a good point, the problem with the above examples is that we don't want things to go through as:

# #@ data.values.hello

and come out as:

# #@ data.values.hello

but instead to come out as:

#@ data.values.hello

in the output.

voor avatar Feb 02 '23 19:02 voor

Just calling this out in abstract, but: What if we resolve some of this ambiguity issues by making a decision and then issuing warnings to the console when the application detects possible edge cases one the ones being called out for each various approach? Then maybe just add a flag maybe in the file to ignore that warning (e.g. sort of like what we already do for code linters).

Just a thought.

patricknelson avatar Feb 02 '23 20:02 patricknelson