cue: ResolveReferences option not work in method "Syntax"

[cueckoo]

Originally opened by @leejanee in https://github.com/cuelang/cue/issues/867

What version of CUE are you using (cue version)?

$ cue version
v0.3.0-beta.8

What did you do?

The code is as follows

       var src =`
t: {name: string}
output: [...t]
`
	var r cue.Runtime
	inst,_:=r.Compile("-",src)
	ct,_:=format.Node(inst.Value().Lookup("output").Syntax(cue.ResolveReferences(true)))
	fmt.Println(string(ct))

After running the code,I get

[...t]

But import the v0.2.2 version, The result is as follows

[...{
        name: string
}]

and, This is what i expected

[cueckoo]

Original reply by @myitcv in https://github.com/cuelang/cue/issues/867#issuecomment-811931709

@leejanee - thanks for raising this.

Just as an FYI, there are some tips on how to create clean reproducers in https://github.com/cuelang/cue/wiki/Creating-test-or-performance-reproducers. The txtar format is particularly good when it comes to multi-file repros, as is the case here.

For example, here is my reproducer based on the above:

exec go mod tidy
exec go run main.go
cmp stdout stdout.golden

-- go.mod --
module mod.com

go 1.16

require cuelang.org/go v0.3.0-beta.8
-- main.go --
package main

import (
 "fmt"

 "cuelang.org/go/cue"
 "cuelang.org/go/cue/format"
)

func main() {
 var src = `
t: {name: string}
output: [...t]
`
 var r cue.Runtime
 inst, _ := r.Compile("-", src)
 ct, _ := format.Node(inst.Value().Lookup("output").Syntax(cue.ResolveReferences(true)))
 fmt.Println(string(ct))
}
-- stdout.golden --
[...{
	name: string
}]

What did you expect to see?

The test pass.

What did you see instead?

> exec go run main.go
[stdout]
[...t]
> cmp stdout stdout.golden
[diff -stdout +stdout.golden]
-[...t]
+[...{
+       name: string
+}]

Creating such repros makes it much easier to confirm the issue, and indeed prepare a fix (because we have code/an example that can largely be copy-pasted).

That said, this does appear to be a regression compared to v0.2.2 (confirmed a problem at tip, 792da39a7835f963f8c2b43d7e55c8eb5a951822), bisected to 845df056ca4facf04a7a844238d3f3da0d4dbe99. Therefore I suspect this will be fixed after the planned v0.3.0 release.

[cueckoo]

Original reply by @wonderflow in https://github.com/cuelang/cue/issues/867#issuecomment-820013044

@mpvl Hi, any progress about this issue, we're really want to upgrade to 0.3 while this issue blocks our needs.

[cueckoo]

Original reply by @myitcv in https://github.com/cuelang/cue/issues/867#issuecomment-849776633

@wonderflow @leejanee - just to check, this remains a priority for you?

[cueckoo]

Original reply by @wonderflow in https://github.com/cuelang/cue/issues/867#issuecomment-850071014

@myitcv yes, our project KubeVela highly relies on this feature, and we love to use CUE very much.

[cueckoo]

Original reply by @wonderflow in https://github.com/cuelang/cue/issues/867#issuecomment-850071707

Now we are still using version 0.2.2 because of this block https://github.com/oam-dev/kubevela/blob/master/go.mod#L6

And our users love to use the feature sets in CUE >= 0.3, so we also very hope to upgrade. But one of our feature relies on it.

[cueckoo]

Original reply by @myitcv in https://github.com/cuelang/cue/issues/867#issuecomment-850110774

Ok, thanks for the update @wonderflow

[cueckoo]

Original reply by @myitcv in https://github.com/cuelang/cue/issues/867#issuecomment-850257273

@wonderflow I've tentatively marked this for v0.5.0.

On a related subject, I would be keen to understand if/how we can add KubeVela to our regression testing setup, unity. Would you be happy for me to email you offline to discuss further?

[cueckoo]

Original reply by @wonderflow in https://github.com/cuelang/cue/issues/867#issuecomment-850263067

Sure, very glad ! My email: [email protected]

[cueckoo]

Original reply by @myitcv in https://github.com/cuelang/cue/issues/867#issuecomment-852197396

@wonderflow @leejanee - when we come to look at this it would be good to understand exactly what semantics you expect from cue.ResolveReferences. My understanding from the example above is that given a CUE Instance/Value, you want to resolve all references to their literal equivalents, whether from within the package or across a package boundary. Does that match your understanding?

[cueckoo]

Original reply by @leejanee in https://github.com/cuelang/cue/issues/867#issuecomment-852779987

@wonderflow @leejanee - when we come to look at this it would be good to understand exactly what semantics you expect from cue.ResolveReferences. My understanding from the example above is that given a CUE Instance/Value, you want to resolve all references to their literal equivalents, whether from within the package or across a package boundary. Does that match your understanding?

Yes it is, I expect that the result returned by the Syntax(cue.All(),cue.ResolveReferences(true)) method can be recompiled

[hprotzek]

Morning, can we expect this fix in the next version? or is this blocked somehow? A new version of culang would improve the writing of templates in Kubevela very much :-)

[myitcv]

@hprotzek thanks for the reminder. We have this issue on our radar for an upcoming release, as discussed with @wonderflow. As we discussed recently in the first CUE community town hall we are in the process of getting back to the project in earnest. So hope to share details of upcoming releases very soon.

[mpvl]

One problem with arbitrarily expanding references is that it can lead to a combinatorial explosion.

@wonderflow, @leejanee: It would therefore be good to understand what the objective is. Can you share some more detail on the use case?

For instance, if the objective is to create a self-contained snippet of CUE, then a different solution would be better.

In either case, to fully fix any such solution we need to have a different internal representation for let expressions. We are working towards this.

If there are simple use cases you would like to see covered in the mean time, please let us know.

[leejanee]

We use cuelang to render kubernetes resources in kubevela. This process is divided into two stages:

1. First, perform user-side rendering, the result is transmitted to the server in ast.Node format.
2. Server-side rendering.

For example:

User 1 and User 2 cooperate to complete resource definition. User 1 define the resource type and default values, and user 2 focus on replicas

First of all, the template on the user side has a consistent format, for example as follows:

parameter: {
    ...
}

resource: {
   ...
}

the parameter is user input, and the resource is the desired resource definition.

1. The template and input of user 1 is as follows:

----- template.file --------
import "k8s.io/api/core/v1"
parameter: {
    image: string
    replicas: int
}

resource: v1.#Deployment{
    spec: {
       replicas: parameter.replicas
       template: spec: containers: [{ 
                image: parameter.image
                name: "main"
                envs: [...]
        }]
    }
}

----- input.file --------
// set parameter
   image: *"myimage" | string
   // limit replicas scope 
   replicas: *2 ｜ >=1 & <5

2. The template and input of user 2（focus on replicas） is as follows:

----- template.file --------
parameter: {
    replicas: int
}

// not care about specific resource types here.
resource: {
   spec: replicas: parameter.replicas
}

----- input.file --------
// set parameter
replicas: 3

3. on the server-side, process like as follows:

resource:  v1.#Deployment & { // user 1
     spec: replicas: *2 ｜ >=1 & <5
     ...
} & {  // user 2
     spec: replicas: 3
}

I expect that the cue execution result on the user side can be recompiled on the server side again.

[mpvl]

I see. So what I observe going on here is that the input of the two users is resolved to be self-contained without references (like an OpenAPI equivalent) to be recombined later.

What I believe would suffice here, though, is to be able to produce a self-contained CUE file, which still may maintain references within the file. IOW, remove the imports.

Is this correct?

IOW, would it suffice to have a def command that just expands all imports, but not necessarily all references?

Or would do you still need all references to be expanded?

Note that references may act as constraints themselves.

Right now, evaluating references may result in an exponential blowup, analogous to the "billion laughs" in YAML. So if the goal here is to put the onus of this on the user, this is understandable. Once we introduce structure sharing, though, such blowup can be avoided if the depth of the evaluation is limited.

[verdverm]

I could imagine import inlining could have unexpected blowup in size. Even with some tree shaking, the file could get quite large with k8s. We'd probably have to account for the cue.mod/{gen,pkg,usr} folders together. Maybe this has already been handled at the point of rendering?

For cue def it would seem like the less often used output. So would this be enabled with a flag?

[leejanee]

1. @mpvl Yes, it is correct that expand v1.#Deployment and remove imports on user 1 side. In other words, process on the server-side should be as follows:

resource:  { // user 1
     ...
     metadata: kind: "Deployment"
     spec: replicas: *2 ｜ >=1 & <5
     ...
} & {  // user 2
     spec: replicas: 3
}

2. But, I sincerely hope that all references to be expanded (though ,the depth of the evaluation is limited). And If the referenced object cannot be found, it can be kept. as follows:

---raw----
x: {}
t: {name: string}
output: [ ... {t & x.value}]

---lookUp(output)---
[... {{name: string} & x.value}]

Although evaluating references may result in an exponential blowup, But if don‘t try to expand the reference, it mean that the reference definition will be lost

[mpvl]

@leejanee: CL https://review.gerrithub.io/c/cue-lang/cue/+/528214 hints at a solution where we could do an expansion where we include definitions outside of scope (e.g. from imports) as top-level let expression in the evaluation until all references are closed. This would create a self-contained file that is guaranteed to not have cycles.

An alternative would be to expand until we have a structural cycle.

I guess either of these would work for you. Will give it some more thought, but it seems that one of these, as least, is doable.

[mpvl]

And just to confirm: it is not sufficient for you to use, for instance, the cue.Final() option. This would also simplify defaults and close lists.

[myitcv]

@leejanee @wonderflow please see v0.4.1-beta.6 (release notes to follow) as a first cut of a solution to resolve this issue. Please let us know what works/doesn't work for you.

[myitcv]

Note: I've just updated my previous comment to reference v0.4.1-beta.6. We had some issues with GoReleaser creating GitHub and Docker release artefacts so we've removed the GitHub releases (the tags remain, as do any modules cached in the Go proxy).

[leejanee]

v0.4.1-beta.6

Very Good, It seems to satisfy our usage, next I will try to upgrade the cue version used in our project KubeVela. Also very much looking forward to the GA version.

[mpvl]

I'm making good progress with the SelfContained option. It's quite involved but getting there. :)

A few more edge cases to be handled, like aliases and references to parent nodes of the tree that needs to be made self-contained. I'll post a WIP CL chain soonish.

[mpvl]

Here you can find a first implementation: https://review.gerrithub.io/c/cue-lang/cue/+/541464/3.

There is a new test set where you can see some of the logic behind it.

Note that the current implementation assumes that whenever you pass a struct that is not the root, this behavior is desirable. I still need to distinguish between also closing over import references or not. So my current thinking is that there will be SelfContained option that is needed to expand references to imported packages (maybe ExpandImports is a better name). But that otherwise the use of the option is not necessary.

[mpvl]

BTW, one edge case that this implementation does not yet support is if the value for which Syntax is used contains references to ancestor nodes (thus recursively referring to that node again). To resolve this we need support for aliases to embedded expressions ({ X=_ }).

[mpvl]

Also, referenced values that cannot be inlined are currently represented as hidden fields. These really should be let expressions. I may still do that.

[wonderflow]

Hi @myitcv @mpvl We really appreciate to see the progress. Our team is trying to upgrade and there're some more compatible issues:

• #1815 (This affect the most cause all our CUE files need to change, they're almost break change for our users.)
• #1813 (Maybe this one is related with this issue)
• #1806 (This not affect our upgrade for now, we can silence the go lint check)

[myitcv]

Our team is trying to upgrade and there're some more compatible issues

Thanks. Will take a look through these today.

[wonderflow]

https://github.com/cue-lang/cue/issues/1816 one more case, maybe it's also related with the selfcontained and can be resolved together.

[mpvl]

#1816 one more case, maybe it's also related with the selfcontained and can be resolved together.

It indeed solve it, to some extent, but please see my answer there. It may not solve it how you want it, but at least in principle it is able to solve it so it will not be hard to iterate towards something workable.