openshift / kube-compare

A Kubectl plugin to allow to compare a known valid reference configuration and a set of specific cluster configuration CRs.
Apache License 2.0
13 stars 17 forks source link

Shorthand for "current yaml path" in templating #25

Open crwr45 opened 5 months ago

crwr45 commented 5 months ago

A very common pattern with this approach is to have an entry in the template that resolves to the value at the same YAML path in the document being compared. This is repetitive and therefore a potential source of error, particularly with longer paths or more complex documents.

Example:

For the document:

spec:
  template:
    metadata:
      name: ThisNameCanBeAnything

the current template would have to look like:

spec:
  template:
    metadata:
      name: {{ .spec.template.metadata.name }}

If there were to be a shorthand for this operation it could look like:

spec:
  template:
    metadata:
      name: {{ anything() }}

Consider this a discussion of:

crwr45 commented 5 months ago

My initial thought is that this would be too complex if all cases are supported. In order to reliably figure out the yaml path from the template the rest of the template would have to have been rendered, necessitating a two-stage rendering process and/or some form of lazy evaluation in the helper. This may not be possible where loops and conditionals are concerned without invasive changes in the templating engine. However, I do think this would be a considerable improvement to readability and usability of the templates if this is possible, even if only for the cases where embedded logic does not affect the yaml path.

pixelsoccupied commented 5 months ago

too complex if all cases are supported

Yep agreed. Unless you're talking about specifically the name? i.e a template function that would map to .spec.template.metadata.name --> getname() implemented with go (.eg see /compare/funcmap.go).

But generally we should try to be a little prudent when comes to adding custom functions in the code base and when we do...it should super general purpose IMO (of course exception apply but hopefully not many)

And tbh .spec.template.metadata.name and it's just cleaner to understand + for user...one less thing to remember and decide whether or not to use getname() when initially creating the references with the template rules?

nocturnalastro commented 5 months ago

I think using the merges would make this no longer required as you could just leave it out completely.

openshift-bot commented 2 months ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

nocturnalastro commented 1 month ago

/lifecycle frozen

lack commented 4 days ago

The new regex inline diff functionality may make this easier to represent in templates:

spec:
  template:
    metadata:
      name: ".*"

The additional cost of this approach is that you would need to mark that field as a regex in the metadata.yaml:

apiVersion: v2
parts:
- name: ExamplePart
  components:
  - name: Example
    allOf:
    - path: that-template.yaml
      config:
        perField:
        - pathToKey: spec.template.metadata.name
          inlineDiffFunc: regex