Skip to content

Commit

Permalink
Add support for compressed schemas/fix basePath trimming. (#540)
Browse files Browse the repository at this point in the history 
* Handle compressed schemas, fix base path trimming, test coverage.

  * (M) protomap/proto.go
  * (M) protomap/proto_test.go
    - handle the case where there is a schemapath of the form
      {config,state}/field - i.e., a compressed schema.
    - fix trimming of base path and add coverage for it within
      tets.
  * (A) testdata/
    - Update example protobufs for testing.
  • Loading branch information
@robshakir
robshakir authored Jun 14, 2021
1 parent 7889ea7 commit dcfe4c7
Show file tree
Hide file tree
Showing 4 changed files with 465 additions and 190 deletions.
157 changes: 145 additions & 12 deletions protomap/proto.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"google.golang.org/protobuf/types/descriptorpb"

"github.com/openconfig/gnmi/value"
"github.com/openconfig/ygot/util"
"github.com/openconfig/ygot/ygot"

gpb "github.com/openconfig/gnmi/proto/gnmi"
Expand Down Expand Up @@ -361,20 +362,106 @@ func resolvedPath(basePath, annotatedPath *gpb.Path) *gpb.Path {
return np
}

// ProtoFromPaths takes an input ygot-generated protobuf and unmarshals the values that are specified in the map
// vals into it, using prefix as the prefix to any paths within the vals map. The message, p, is modified in place.
// The map, vals, must be keyed by the gNMI path to the field which is annotated in the ygot generated protobuf,
// the complete path is taken to be prefix + the key found in the map.
func ProtoFromPaths(p proto.Message, vals map[*gpb.Path]interface{}, prefix *gpb.Path) error {
// UnmapOpt marks that a particular option can be supplied as an argument
// to the ProtoFromPaths function.
type UnmapOpt interface {
isUnmapOpt()
}

// IgnoreExtraPaths indicates that unmapping should ignore any additional
// paths that are found in the gNMI Notifications that do not have corresponding
// fields in the protobuf.
//
// This option is typically used in conjunction with path compression where there
// are some leaves that do not have corresponding fields.
func IgnoreExtraPaths() *ignoreExtraPaths { return &ignoreExtraPaths{} }

type ignoreExtraPaths struct{}

// isUnmapOpt marks ignoreExtraPaths as an unmap option.
func (*ignoreExtraPaths) isUnmapOpt() {}

// ValuePathPrefix indicates that the values in the supplied map have a prefix which
// is equal to the supplied path. The prefix plus each path in the vals map must be
// equal to the absolute path for the supplied values.
func ValuePathPrefix(path *gpb.Path) *valuePathPrefix {
return &valuePathPrefix{p: path}
}

type valuePathPrefix struct{ p *gpb.Path }

// isUnmapOpt marks valuePathPrefix as an unmap option.
func (*valuePathPrefix) isUnmapOpt() {}

// ProtobufMessagePrefix specifies the path that the protobuf message supplied to ProtoFromPaths
// makes up. This is used in cases where the message itself is not the root - and hence unmarshalling
// should look for paths relative to the specified path in the vals map.
func ProtobufMessagePrefix(path *gpb.Path) *protoMsgPrefix {
return &protoMsgPrefix{p: path}
}

type protoMsgPrefix struct{ p *gpb.Path }

// isUnmapOpt marks protoMsgPrefix as an unmap option.
func (*protoMsgPrefix) isUnmapOpt() {}

// ProtoFromPaths takes an input ygot-generated protobuf and unmarshals the values provided in vals into the map.
// The vals map must be keyed by the gNMI path to the leaf, with the interface{} value being the value that the
// leaf at the field should be set to.
//
// The protobuf p is modified in place to add the values.
//
// The set of UnmapOpts that are provided (opt) are used to control the behaviour of unmarshalling the specified data.
//
// ProtoFromPaths returns an error if the data cannot be unmarshalled.
func ProtoFromPaths(p proto.Message, vals map[*gpb.Path]interface{}, opt ...UnmapOpt) error {
if p == nil {
return errors.New("nil protobuf supplied")
}

valPrefix, err := hasValuePathPrefix(opt)
if err != nil {
return fmt.Errorf("invalid value prefix supplied, %v", err)
}

protoPrefix, err := hasProtoMsgPrefix(opt)
if err != nil {
return fmt.Errorf("invalid protobuf message prefix supplied in options, %v", err)
}

schemaPath := func(p *gpb.Path) *gpb.Path {
np := proto.Clone(p).(*gpb.Path)
for _, e := range np.Elem {
e.Key = nil
}
return np
}

// directCh is a map between the absolute schema path for a particular value, and
// the value specified.
directCh := map[*gpb.Path]interface{}{}
for p, v := range vals {
// TODO(robjs): needs fixing for compressed schemas.
if len(p.GetElem()) == len(prefix.GetElem())+1 {
directCh[p] = v
absPath := &gpb.Path{
Elem: append(append([]*gpb.PathElem{}, schemaPath(valPrefix).Elem...), p.Elem...),
}

if !util.PathMatchesPathElemPrefix(absPath, protoPrefix) {
return fmt.Errorf("invalid path provided, absolute paths must be used, %s does not have prefix %s", absPath, protoPrefix)
}

// make the path absolute, and a schema path.
pp := util.TrimGNMIPathElemPrefix(absPath, protoPrefix)

if len(pp.GetElem()) == 1 {
directCh[pp] = v
}
// TODO(robjs): it'd be good to have something here that tells us whether we are in
// a compressed schema. Potentially we should add something to the generated protobuf
// as a fileoption that would give us this indication.
if len(pp.Elem) == 2 {
if pp.Elem[len(pp.Elem)-2].Name == "config" || pp.Elem[len(pp.Elem)-2].Name == "state" {
directCh[pp] = v
}
}
}

Expand All @@ -389,8 +476,13 @@ func ProtoFromPaths(p proto.Message, vals map[*gpb.Path]interface{}, prefix *gpb
}

for _, ap := range annotatedPath {
if !util.PathMatchesPathElemPrefix(ap, protoPrefix) {
rangeErr = fmt.Errorf("annotation %s does not match the supplied prefix %s", ap, protoPrefix)
return false
}
trimmedAP := util.TrimGNMIPathElemPrefix(ap, protoPrefix)
for chp, chv := range directCh {
if proto.Equal(ap, chp) {
if proto.Equal(trimmedAP, chp) {
switch fd.Kind() {
case protoreflect.MessageKind:
v, isWrap, err := makeWrapper(m, fd, chv)
Expand Down Expand Up @@ -428,15 +520,56 @@ func ProtoFromPaths(p proto.Message, vals map[*gpb.Path]interface{}, prefix *gpb
return rangeErr
}

for chp := range directCh {
if !mapped[chp] {
return fmt.Errorf("did not map path %s to a proto field", chp)
if !hasIgnoreExtraPaths(opt) {
for chp := range directCh {
if !mapped[chp] {
return fmt.Errorf("did not map path %s to a proto field", chp)
}
}
}

return nil
}

// hasIgnoreExtraPaths checks whether the supplied opts slice contains the
// ignoreExtraPaths option.
func hasIgnoreExtraPaths(opts []UnmapOpt) bool {
for _, o := range opts {
if _, ok := o.(*ignoreExtraPaths); ok {
return true
}
}
return false
}

// hasProtoMsgPrefix checks whether the supplied opts slice contains the
// protoMsgPrefix option, and validates and returns the path it contains.
func hasProtoMsgPrefix(opts []UnmapOpt) (*gpb.Path, error) {
for _, o := range opts {
if v, ok := o.(*protoMsgPrefix); ok {
if v.p == nil {
return nil, fmt.Errorf("invalid protobuf prefix supplied, %+v", v)
}
return v.p, nil
}
}
return &gpb.Path{}, nil
}

// hasValuePathPrefix checks whether the supplied opts slice contains
// the valuePathPrefix option, and validates and returns the apth it contains.
func hasValuePathPrefix(opts []UnmapOpt) (*gpb.Path, error) {
for _, o := range opts {
if v, ok := o.(*valuePathPrefix); ok {
if v.p == nil {
return nil, fmt.Errorf("invalid protobuf prefix supplied, %+v", v)
}
return v.p, nil
}
}
return &gpb.Path{}, nil
}

// makeWrapper generates a new message for field fd of the proto message msg with the value set to val.
// The field fd must describe a field that has a message type. An error is returned if the wrong
// type of payload is provided for the message. The second, boolean, return argument specifies whether
Expand Down
131 changes: 129 additions & 2 deletions protomap/proto_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ func TestProtoFromPaths(t *testing.T) {
desc string
inProto proto.Message
inVals map[*gpb.Path]interface{}
inPrefix *gpb.Path
inOpt []UnmapOpt
wantProto proto.Message
wantErrSubstring string
}{{
Expand Down Expand Up @@ -441,18 +441,145 @@ func TestProtoFromPaths(t *testing.T) {
mustPath("/bytes"): 42,
},
wantErrSubstring: "got non-byte slice value for bytes field",
}, {
desc: "compressed schema",
inProto: &epb.ExampleMessage{},
inVals: map[*gpb.Path]interface{}{
mustPath("/state/compress"): "hello-world",
},
wantProto: &epb.ExampleMessage{
Compress: &wpb.StringValue{Value: "hello-world"},
},
}, {
desc: "trim prefix",
inProto: &epb.Interface{},
inVals: map[*gpb.Path]interface{}{
mustPath("/interfaces/interface/config/description"): "interface-42",
},
inOpt: []UnmapOpt{
ProtobufMessagePrefix(mustPath("/interfaces/interface")),
},
wantProto: &epb.Interface{
Description: &wpb.StringValue{Value: "interface-42"},
},
}, {
desc: "trim prefix with valPrefix",
inProto: &epb.Interface{},
inVals: map[*gpb.Path]interface{}{
mustPath("description"): "interface-42",
},
inOpt: []UnmapOpt{
ProtobufMessagePrefix(mustPath("/interfaces/interface")),
ValuePathPrefix(mustPath("/interfaces/interface/config")),
},
wantProto: &epb.Interface{
Description: &wpb.StringValue{Value: "interface-42"},
},
}, {
desc: "invalid message with no annotation on one of its other fields",
inProto: &epb.InvalidMessage{},
inVals: map[*gpb.Path]interface{}{
mustPath("three"): "str",
},
wantErrSubstring: "received field with invalid annotation",
}, {
desc: "invalid message with bad field type",
inProto: &epb.BadMessageKeyTwo{},
inVals: map[*gpb.Path]interface{}{
mustPath("one"): "42",
},
wantErrSubstring: "unknown field kind",
}, {
desc: "extra paths, not ignored",
inProto: &epb.Interface{},
inVals: map[*gpb.Path]interface{}{
mustPath("config/name"): "interface-42",
mustPath("config/description"): "portal-to-wonderland",
},
inOpt: []UnmapOpt{
ProtobufMessagePrefix(mustPath("/interfaces/interface")),
ValuePathPrefix(mustPath("/interfaces/interface")),
},
wantProto: &epb.Interface{
Description: &wpb.StringValue{Value: "interface-42"},
},
wantErrSubstring: `did not map path elem`,
}, {
desc: "extra paths, ignored",
inProto: &epb.Interface{},
inVals: map[*gpb.Path]interface{}{
mustPath("config/name"): "interface-42",
mustPath("config/description"): "portal-to-wonderland",
},
inOpt: []UnmapOpt{
ProtobufMessagePrefix(mustPath("/interfaces/interface")),
IgnoreExtraPaths(),
ValuePathPrefix(mustPath("/interfaces/interface")),
},
wantProto: &epb.Interface{
Description: &wpb.StringValue{Value: "portal-to-wonderland"},
},
}, {
desc: "field that is not directly a child",
inProto: &epb.ExampleMessage{},
inVals: map[*gpb.Path]interface{}{
mustPath("/one/two/three"): "ignored",
},
wantProto: &epb.ExampleMessage{},
}, {
desc: "value prefix specified - schema path",
inProto: &epb.Interface{},
inVals: map[*gpb.Path]interface{}{
mustPath("description"): "interface-42",
},
inOpt: []UnmapOpt{
ProtobufMessagePrefix(mustPath("/interfaces/interface")),
ValuePathPrefix(mustPath("/interfaces/interface/config")),
},
wantProto: &epb.Interface{
Description: &wpb.StringValue{Value: "interface-42"},
},
}, {
desc: "value prefix specified - data tree path",
inProto: &epb.Interface{},
inVals: map[*gpb.Path]interface{}{
mustPath("description"): "interface-42",
},
inOpt: []UnmapOpt{
ProtobufMessagePrefix(mustPath("/interfaces/interface")),
ValuePathPrefix(mustPath("/interfaces/interface[name=ethernet42]/config")),
},
wantProto: &epb.Interface{
Description: &wpb.StringValue{Value: "interface-42"},
},
}, {
desc: "bad trimmed value",
inProto: &epb.Interface{},
inVals: map[*gpb.Path]interface{}{
mustPath("config/description"): "interface-84",
},
inOpt: []UnmapOpt{
ProtobufMessagePrefix(mustPath("/interfaces/fish")),
},
wantErrSubstring: "invalid path provided, absolute paths must be used",
}, {
desc: "relative paths to protobuf prefix",
inProto: &epb.Interface{},
inVals: map[*gpb.Path]interface{}{
mustPath("config/description"): "value",
},
inOpt: []UnmapOpt{
ProtobufMessagePrefix(mustPath("/interfaces/interface")),
ValuePathPrefix(mustPath("/interfaces/interface")),
},
wantProto: &epb.Interface{
Description: &wpb.StringValue{Value: "value"},
},
}}

for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
err := ProtoFromPaths(tt.inProto, tt.inVals, tt.inPrefix)
err := ProtoFromPaths(tt.inProto, tt.inVals, tt.inOpt...)
if err != nil {
if diff := errdiff.Substring(err, tt.wantErrSubstring); diff != "" {
t.Fatalf("did not get expected error, %s", diff)
Expand Down
Loading

0 comments on commit dcfe4c7

Please sign in to comment.