cue icon indicating copy to clipboard operation
cue copied to clipboard

cue.Value.Patch() and cue.Value.Set()

Open myitcv opened this issue 3 years ago • 12 comments

Is your feature request related to a problem? Please describe.

One of the core premises of CUE evaluation is that a value cannot be overridden. Hence the following is an error:

x: 5
x: 4

This is an important invariant of evaluation in CUE. From The Logic of CUE:

With approaches that allow overrides, whether it be the complex inheritance used in languages like GCL and Jsonnet or the much simpler file-based approaches as used in HCL and Kustomize, finding a declaration for a concrete field value does not guarantee a final answer, because another concrete value that occurs elsewhere can override it.

Note the focus here is on evaluation of CUE code and on protecting the author of that CUE code. The lack of override support is a language feature, enforced by the core evaluator.

Therefore, this does not preclude workflows where CUE evaluation forms one (or more) part of a wider workflow. In such workflows, patching or overrides are sometimes required.

For example, users of the CUE Go API can consume some CUE, parse to an AST, "patch" the AST to override a value, then re-evaluate. CUE cannot and should not look to prevent such manipulation.

(Whether such workflows could better be supported by CUE via defaults and restructuring of CUE, inputs etc is beyond this proposal).

The KubeVela project have such a patching requirement. Right now, they resort of converting a cue.Value to an AST, patching, then re-evaluating.

Describe the solution you'd like

The following configuration, referred to as the variable v, helps make our explanation of those semantics more concrete:

a: [1, 2, 3]
y: a          // assume we want to "patch" this field with [4, 5, 6]
z: y

The two proposed methods are:

cue.Value.Patch(cue.Path, interface{}) cue.Value

Calling:

v.Patch(cue.ParsePath("y"), []int{4,5,6})

and making concrete the result would result in:

a: [1, 2, 3]
y: [4, 5, 6]
z: [1, 2, 3]

i.e. z's reference to y retains the original value for y.

cue.Value.Set(cue.Path, interface{}) cue.Value

Calling:

v.Set(cue.ParsePath("y"), []int{4,5,6})

and making concrete the result would result in:

a: [1, 2, 3]
y: [4, 5, 6]
z: [4, 5, 6]

i.e. z's reference to y reflects the new value for y.

Describe alternatives you've considered

AST manipulation and other related awkward workarounds.

Note that cue.Value.FillPath() is insufficient: it can only make a value more specific.

Additional context

In discussion with @FogDong, @wonderflow and the KubeVela team, two of the conclusions we reached (for now) are:

  • KubeVela will be patching concrete values.
  • KubeVela are therefore comfortable with the proposed semantics for cue.Value.Patch().

myitcv avatar Dec 04 '22 20:12 myitcv

Following previous discussions with @FogDong and @mpvl, we are looking to implement cue.Value.Patch() in v0.6.0.

myitcv avatar Dec 04 '22 20:12 myitcv

It is hard to understand when one would want the Patch behavior, such that field "z" referring to field "y"'s value does not wind up holding true in the final concrete value. How would you document the behavior of Patch?

Also, did you consider functions like these that would accept transforming callback functions, where Patch and Set would call the supplied function with the current value and take that called function's return value as the desired value. That way, a caller could express something like "append another element to the current list." You could argue that the caller could first read the current value, compute the desired value, and only then call on Patch or Set, but it would be more convenient to express the desire as a transformation. That makes more sense for a function named Patch, which implies the possibility of considering and preserving aspects of the current value.

seh avatar Dec 05 '22 14:12 seh

This one's priority is medium currently but in the long term it's high priority. Currently, our workaround is to unify and patch in the Syntax level, but the community has deprecated some capabilities like resolve reference. We're still using it, though. We need this Value based patch to get rid of those deprecated capabilities. Not a user-facing issue and no time constraint but hope to be fixed in v0.6.0.

FogDong avatar Feb 09 '23 02:02 FogDong

@seh Patch could be explained as an overlay. Everything in the original value remains as is, and the patched value would specify another CUE value that replaces the semantics in the overlay. Instead of a function, I can imagine it would allow an ast.Expr that could be interpreted in the context of the current node.

mpvl avatar Apr 04 '23 17:04 mpvl

After the hypothetical call to Patch in the original example, if you were to serialize the result to CUE code (a la cue def), would field "z"'s value still be y (a reference to a sibling field named "y")?

seh avatar Apr 04 '23 18:04 seh

Pushing this back to v0.6.x so as not to delay the release of required fields. The reason for pushing back is that there have been some queries on the cost/benefit of introducing Patch(). @FogDong and team, we will be in touch to discuss in case our thinking changes on introducing this.

myitcv avatar Jun 14 '23 12:06 myitcv

I need this functionality. Trying to do a straight value.FillPath() will fail when attempting to override a concrete value. I've resorted to an extremely esoteric hack of converting it to JSON and then overriding it. I just started looking at the API today, so I could be overlooking more sane alternatives.

I noticed we're on v0.7.0 and this didn't seem to get implemented. Is there a timeline for when it will appear?

jmgilman avatar Jan 19 '24 01:01 jmgilman

@jmgilman if you want you could try my cue-helper: https://github.com/mheers/cue-helper/blob/main/pkg/cue/cue.go This offers some functions where you can modify the cuelang document as a string. It leverages https://github.com/kubevela/workflow to merge the data.

mheers avatar Jan 19 '24 10:01 mheers

Thanks @mheers, that did the trick nicely. Do you by chance know how to get around the function stripping comments? Was hoping to keep the source file mostly intact when replacing the value.

I peeked through the kubevela code and it looks like it should preserve comments, but for some reason the cue.Value being returned no longer has any comments (i.e. they were there before, but afterwards calling .Doc() returns nothing).

Here is an excerpt:

fmt.Printf("%v", v.LookupPath(cue.ParsePath("bundle.apiVersion")).Doc())
v, err = ch.Replace(v, cli.Path, cli.Value)
ctx.FatalIfErrorf(err)
fmt.Printf("%v", v.LookupPath(cue.ParsePath("bundle.apiVersion")).Doc())

After using your Replace function that particular field no longer has comments associated with it.

jmgilman avatar Jan 26 '24 02:01 jmgilman

bumping.

My use case: we want to programmatically validate and update values in cue files. That's insane that we need to use JSON etc conversions to achieve it when it all can be done using standard methods which are hidden deep inside the code base..

dyatlov avatar Sep 05 '24 15:09 dyatlov

Bumping this too, we'd love to see this feature

alecholmez avatar Oct 08 '24 13:10 alecholmez

Hi all,

I have a intermediary solution based off some prototyping done by @myitcv. It's embedded in a work-related project, but it's OSS and you're free to incorporate it into your own flows: https://github.com/input-output-hk/catalyst-forge/blob/master/lib/tools/cue/mutate.go.

jmgilman avatar Oct 08 '24 14:10 jmgilman

Would this proposal benefit from splitting into two, one for cue.Value.Set which doesn't seem to be have any arguments against and one for cue.Value.Patch which seems like it might need more deliberation?

Getting cue.Value.Set would be enough to solve most simple use cases I presume.

DavidGamba avatar Feb 24 '25 15:02 DavidGamba

Thanks for the interest here folks. This is a tricky area and whilst the main reason for our lack of focus here is best summarised in the most recent community call (i.e. incredibly focussed on other higher-priority items), another key reason we have been "slow" to move here is that the answer is not clear.

Getting cue.Value.Set would be enough to solve most simple use cases I presume.

@DavidGamba - just to make sure you intended to say cue.Value.Set here? If so, I would actually argue (based on my experimentation elsewhere, which fed into https://github.com/cue-lang/cue/issues/2170#issuecomment-2399945467), actually cue.Value.Patch is more clearly "correct" in this case, under the constraints that the input is concrete (one of the original conditions in this issue).

But this just goes to highlight the lack of an obvious "single answer" here.

(There is also the tension that exists between the desire to mutate/patch/etc something vs "fixing" it upstream by re-organising a configuration... This very much flows into the conversations we had recently in the #deployment-patterns conversation on Slack)

myitcv avatar Mar 08 '25 12:03 myitcv