Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for proto.to_any #46

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ name.

For projects that use `gogo_protobuf`, e.g. envoy, this can be very confusing.

To resolve this, use the package `github.com/stripe/skycfg/gogocopmat`
To resolve this, use the package `github.com/stripe/skycfg/gogocompat`
in your go code. This will allow skycfg code to load
`proto.package("gogo:google.protobuf") and the internal registry it
creates will handle the conversion of these types.
33 changes: 32 additions & 1 deletion internal/go/skycfg/proto_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ import (
"reflect"
"sort"

gogo_proto "github.com/gogo/protobuf/proto"
gogo_types "github.com/gogo/protobuf/types"
"github.com/golang/protobuf/jsonpb"
"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/ptypes"
"go.starlark.net/starlark"
yaml "gopkg.in/yaml.v2"
"gopkg.in/yaml.v2"
)

// UNSTABLE extension point for configuring how protobuf messages are loaded.
Expand All @@ -53,6 +56,7 @@ func NewProtoModule(registry ProtoRegistry) *ProtoModule {
"merge": starlark.NewBuiltin("proto.merge", fnProtoMerge),
"set_defaults": starlark.NewBuiltin("proto.set_defaults", fnProtoSetDefaults),
"to_json": starlark.NewBuiltin("proto.to_json", fnProtoToJson),
"to_any": starlark.NewBuiltin("proto.to_any", fnProtoToAny),
"to_text": starlark.NewBuiltin("proto.to_text", fnProtoToText),
"to_yaml": starlark.NewBuiltin("proto.to_yaml", fnProtoToYaml),
},
Expand Down Expand Up @@ -234,6 +238,33 @@ func fnProtoToJson(t *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple
return starlark.String(jsonData), nil
}

// Implementation of the `proto.to_any()` built-in function. Returns a
// skyProtoMessage with an `Any` proto.Message in it.
func fnProtoToAny(t *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
var msg *skyProtoMessage
err := wantSingleProtoMessage("proto.to_any", args, kwargs, &msg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
err := wantSingleProtoMessage("proto.to_any", args, kwargs, &msg)
err := wantSingleProtoMessage(fn.Name(), args, kwargs, &msg)

if err != nil {
return nil, err
}

// Disambiguate between golang and gogo encoded proto messages.
var any proto.Message
if "" != proto.MessageName(msg.msg) {
// Returns a golang any.Any type.
any, err = ptypes.MarshalAny(msg.msg)
} else if "" != gogo_proto.MessageName(msg.msg) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably only go here if msg's package has a gogo: prefix to be consistent with previous changes in gogocompat.

Previously @jmillikin-stripe also insisted on gogo dependencies going into separate package to avoid hard dependency within core implementation.

// Returns a gogo types.Any type.
any, err = gogo_types.MarshalAny(msg.msg)
} else {
return nil, fmt.Errorf("%s: could not get message name for %s", "proto.to_any", msg.Type())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, fmt.Errorf("%s: could not get message name for %s", "proto.to_any", msg.Type())
return nil, fmt.Errorf("%s: could not get message name for %s", fn.Name(), msg.Type())

}
if err != nil {
return nil, err
}

return NewSkyProtoMessage(any), nil
}

// Implementation of the `proto.to_yaml()` built-in function. Returns the
// YAML-formatted content of a protobuf message.
func fnProtoToYaml(t *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
Expand Down
72 changes: 72 additions & 0 deletions internal/go/skycfg/proto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ import (
"time"

gogo_proto "github.com/gogo/protobuf/proto"
gogo_types "github.com/gogo/protobuf/types"
"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/ptypes"
"github.com/golang/protobuf/ptypes/any"
"github.com/kylelemons/godebug/pretty"
"go.starlark.net/resolve"
"go.starlark.net/starlark"
Expand Down Expand Up @@ -288,6 +291,75 @@ func TestProtoToJsonFull(t *testing.T) {
}
}

func TestProtoToAnyGogo(t *testing.T) {
val := skyEval(t, `proto.to_any(gogo_proto.package("skycfg.test_proto").MessageGogo(
f_string = "some string",
))`)
myAny := val.(*skyProtoMessage).msg.(*gogo_types.Any)

want := "type.googleapis.com/skycfg.test_proto.MessageGogo"
if want != myAny.GetTypeUrl() {
t.Fatalf("to_any: wanted %q, got %q", want, myAny.GetTypeUrl())
}

msg := pb.MessageGogo{}
err := gogo_types.UnmarshalAny(myAny, &msg)
if err != nil {
t.Fatalf("to_any: could not unmarshal: %v", err)
}

want = "some string"
if want != msg.GetFString() {
t.Fatalf("to_any: wanted %q, got %q", want, msg.GetFString())
}
}

func TestProtoToAnyV2(t *testing.T) {
val := skyEval(t, `proto.to_any(proto.package("skycfg.test_proto").MessageV2(
f_string = "some string",
))`)
myAny := val.(*skyProtoMessage).msg.(*any.Any)

want := "type.googleapis.com/skycfg.test_proto.MessageV2"
if want != myAny.GetTypeUrl() {
t.Fatalf("to_any: wanted %q, got %q", want, myAny.GetTypeUrl())
}

msg := pb.MessageV2{}
err := ptypes.UnmarshalAny(myAny, &msg)
if err != nil {
t.Fatalf("to_any: could not unmarshal: %v", err)
}

want = "some string"
if want != msg.GetFString() {
t.Fatalf("to_any: wanted %q, got %q", want, msg.GetFString())
}
}

func TestProtoToAnyV3(t *testing.T) {
val := skyEval(t, `proto.to_any(proto.package("skycfg.test_proto").MessageV3(
f_string = "some string",
))`)
myAny := val.(*skyProtoMessage).msg.(*any.Any)

want := "type.googleapis.com/skycfg.test_proto.MessageV3"
if want != myAny.GetTypeUrl() {
t.Fatalf("to_any: wanted %q, got %q", want, myAny.GetTypeUrl())
}

msg := pb.MessageV3{}
err := ptypes.UnmarshalAny(myAny, &msg)
if err != nil {
t.Fatalf("to_any: could not unmarshal: %v", err)
}

want = "some string"
if want != msg.GetFString() {
t.Fatalf("to_any: wanted %q, got %q", want, msg.GetFString())
}
}

func TestProtoToYaml(t *testing.T) {
val := skyEval(t, `proto.to_yaml(proto.package("skycfg.test_proto").MessageV3(
f_string = "some string",
Expand Down