cue icon indicating copy to clipboard operation
cue copied to clipboard

internal/core/convert: does not handle k8s inline fields as expected

Open tkuchiki opened this issue 1 year ago • 1 comments
trafficstars

What version of CUE library

$ grep cuelang\.org go.mod
        cuelang.org/go v0.7.1 // indirect

$ go version
go version go1.21.6 darwin/amd64

Does this issue reproduce with the latest stable release?

Yes, 0.7.1

What did you do?

I tried decoding from Go struct data to json with Codec.Decode.

The code(main.go) is the following:

package main

import (
	"fmt"
	"log"

	"cuelang.org/go/cue/cuecontext"
	"cuelang.org/go/encoding/gocode/gocodec"
)

type A struct {
	B string
	C C
}

type CReference struct {
	Name string `json:"name,omitempty" protobuf:"bytes,1,opt,name=name"`
}

type C struct {
	CReference `json:",inline" protobuf:"bytes,1,opt,name=cReference"`
}

func main() {
	a := A{
		B: "bbb",
		C: C{
			CReference{
				Name: "alice",
			},
		},
	}

	ctx := cuecontext.New()
	codec := gocodec.New(ctx, nil)
	v, err := codec.Decode(a)
	if err != nil {
		log.Fatal(err)
	}

	b, err := v.MarshalJSON()
	if err != nil {
		log.Fatal(err)
	}

	fmt.Println(string(b))
}

I executed the following command:

go get "cuelang.org/go/cue/"
go run main.go | jq .

What did you expect to see?

The data structure is the following:

type A struct {
	B string
	C C
}

type CReference struct {
	Name string `json:"name,omitempty" protobuf:"bytes,1,opt,name=name"`
}

type C struct {
	CReference `json:",inline" protobuf:"bytes,1,opt,name=cReference"`
}

a := A{
	B: "bbb",
	C: C{
		CReference{
			Name: "alice",
		},
	},
}

So I expected the following result:

{
  "B": "bbb",
  "C": {
    "name": "alice"
  }
}

What did you see instead?

I got the following result:

{
  "B": "bbb",
  "C": {
    "bytes": {
      "name": "alice"
    }
  }
}

I read the code of the getName function in internal/core/convert/go.go. This function does not refer to inline struct tags, so if multiple struct tags such as json or protobuf are written and the name is omitted, it probably won’t work correctly in cases where it is expected to be inline.

type C struct {
	CReference `json:",inline" protobuf:"bytes,1,opt,name=cReference"`
	                 ^^                  ^^^^^
}

I encountered this issue when manipulating Kubernetes API Objects with Go and outputting them as CUE values. Therefore, I believe this issue could be a common occurrence.

I considered creating a patch myself, but since I was uncertain regarding what the specification should be, I created this issue.

tkuchiki avatar Mar 06 '24 12:03 tkuchiki

It's worth noting that, as Joe Tsai replied in https://github.com/golang/protobuf/issues/487, the following field is contradictory:

	CReference `json:",inline" protobuf:"bytes,1,opt,name=cReference"`

First, the Go embedding and JSON tag ,inline seem to imply that we should inline the field. However, the ProtoBuf tag name=cReference says the opposite - that the field is meant to have a name, and thus not be inlined.

However, we currently take bytes as a name from ProtoBuf, which is incorrect - that first part of the protobuf tag is not a name, but a type hint. So we should fix the code. Moreover, all protobuf generators seem to generate json field tags as well, so it should be strictly better to simply follow the json tag and ignore the protobuf tag in this code.

mvdan avatar Aug 13 '24 12:08 mvdan