From 24d823d4d4c82037546e7b3849146702be98246d Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Thu, 19 Dec 2019 17:42:37 -0800 Subject: [PATCH 1/5] Support explicitly setting field tags The YANG to Protobuf specification mentions using extensions to explicitly set the field tag of generated Protobuf fields. Add support for annotating field tags. * Support setting the field number with occodegenext:field-number. * Support offsetting the field number with occodegenext:field-number-offset. * Reject field number annotations that result in a value in the Protobuf reserved range. --- ygen/protogen.go | 33 +++++++++++++++ ygen/protogen_test.go | 96 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+) diff --git a/ygen/protogen.go b/ygen/protogen.go index 3ab451d26..315958017 100644 --- a/ygen/protogen.go +++ b/ygen/protogen.go @@ -21,6 +21,7 @@ import ( "path/filepath" "regexp" "sort" + "strconv" "strings" "text/template" @@ -1094,6 +1095,38 @@ func safeProtoIdentifierName(name string) string { // protoTagForEntry returns a protobuf tag value for the entry e. func protoTagForEntry(e *yang.Entry) (uint32, error) { + var offset uint32 + for _, ext := range e.Exts { + if ext.Keyword == "occodegenext:field-number-offset" { + off, err := strconv.ParseUint(ext.Argument, 10, 32) + if err != nil { + return 0, fmt.Errorf("could not parse %s %s: %s", ext.Keyword, ext.Argument, err) + } + offset = uint32(off) + if offset == 0 { + return 0, fmt.Errorf("%s cannot be 0", ext.Keyword) + } + break + } + } + for _, ext := range e.Exts { + if ext.Keyword == "occodegenext:field-number" { + fn, err := strconv.ParseUint(ext.Argument, 10, 32) + if err != nil { + return 0, fmt.Errorf("could not parse %s %s: %s", ext.Keyword, ext.Argument, err) + } + if fn == 0 { + return 0, fmt.Errorf("%s cannot be 0", ext.Keyword) + } + annotatedFieldNumber := uint32(fn) + offset + if annotatedFieldNumber >= 19000 && annotatedFieldNumber <= 19999 { + return 0, fmt.Errorf("%s field number %d in Protobuf reserved range", + e.Name, annotatedFieldNumber) + } + return annotatedFieldNumber, nil + } + } + return fieldTag(e.Path()) } diff --git a/ygen/protogen_test.go b/ygen/protogen_test.go index 2645b5f0b..ffec533b9 100644 --- a/ygen/protogen_test.go +++ b/ygen/protogen_test.go @@ -654,6 +654,102 @@ func TestGenProto3Msg(t *testing.T) { }}, }, }, + }, { + name: "simple message with only scalar fields and field number extensions", + inMsg: &Directory{ + Name: "MessageName", + Entry: &yang.Entry{ + Name: "message-name", + Dir: map[string]*yang.Entry{}, + Kind: yang.DirectoryEntry, + }, + Fields: map[string]*yang.Entry{ + "field-one": { + Name: "field-one", + Type: &yang.YangType{Kind: yang.Ystring}, + Exts: []*yang.Statement{ + &yang.Statement{ + Keyword: "occodegenext:field-number-offset", + Argument: "100", + }, + }, + }, + "field-two": { + Name: "field-two", + Type: &yang.YangType{Kind: yang.Yint8}, + Exts: []*yang.Statement{ + &yang.Statement{ + Keyword: "occodegenext:field-number", + Argument: "1", + }, + }, + }, + "field-three": { + Name: "field-three", + Type: &yang.YangType{Kind: yang.Yint8}, + Exts: []*yang.Statement{ + &yang.Statement{ + Keyword: "occodegenext:field-number", + Argument: "1", + }, + &yang.Statement{ + Keyword: "occodegenext:field-number-offset", + Argument: "100", + }, + }, + }, + }, + Path: []string{"", "root", "message-name"}, + }, + inBasePackage: "base", + inEnumPackage: "enums", + wantMsgs: map[string]*protoMsg{ + "MessageName": { + Name: "MessageName", + YANGPath: "/root/message-name", + Fields: []*protoMsgField{{ + Tag: 410095931, + Name: "field_one", + Type: "ywrapper.StringValue", + }, { + Tag: 1, + Name: "field_two", + Type: "ywrapper.IntValue", + }, { + Tag: 101, + Name: "field_three", + Type: "ywrapper.IntValue", + }}, + }, + }, + }, { + name: "message with field number in reserved range", + inMsg: &Directory{ + Name: "MessageName", + Entry: &yang.Entry{ + Name: "message-name", + Dir: map[string]*yang.Entry{}, + Kind: yang.DirectoryEntry, + }, + Fields: map[string]*yang.Entry{ + "field-one": { + Name: "field-one", + Type: &yang.YangType{Kind: yang.Ystring}, + Exts: []*yang.Statement{ + &yang.Statement{ + Keyword: "occodegenext:field-number", + Argument: "1", + }, + &yang.Statement{ + Keyword: "occodegenext:field-number-offset", + Argument: "18999", + }, + }, + }, + }, + Path: []string{"", "root", "message-name"}, + }, + wantErr: true, }} for _, tt := range tests { From a332dd69c4834560d8719c30ebbdc2e3d943eaa3 Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Fri, 20 Dec 2019 12:20:08 -0800 Subject: [PATCH 2/5] Fix gofmt issues --- ygen/protogen_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ygen/protogen_test.go b/ygen/protogen_test.go index ffec533b9..2869b9a4a 100644 --- a/ygen/protogen_test.go +++ b/ygen/protogen_test.go @@ -668,7 +668,7 @@ func TestGenProto3Msg(t *testing.T) { Name: "field-one", Type: &yang.YangType{Kind: yang.Ystring}, Exts: []*yang.Statement{ - &yang.Statement{ + { Keyword: "occodegenext:field-number-offset", Argument: "100", }, @@ -678,7 +678,7 @@ func TestGenProto3Msg(t *testing.T) { Name: "field-two", Type: &yang.YangType{Kind: yang.Yint8}, Exts: []*yang.Statement{ - &yang.Statement{ + { Keyword: "occodegenext:field-number", Argument: "1", }, @@ -688,11 +688,11 @@ func TestGenProto3Msg(t *testing.T) { Name: "field-three", Type: &yang.YangType{Kind: yang.Yint8}, Exts: []*yang.Statement{ - &yang.Statement{ + { Keyword: "occodegenext:field-number", Argument: "1", }, - &yang.Statement{ + { Keyword: "occodegenext:field-number-offset", Argument: "100", }, @@ -736,11 +736,11 @@ func TestGenProto3Msg(t *testing.T) { Name: "field-one", Type: &yang.YangType{Kind: yang.Ystring}, Exts: []*yang.Statement{ - &yang.Statement{ + { Keyword: "occodegenext:field-number", Argument: "1", }, - &yang.Statement{ + { Keyword: "occodegenext:field-number-offset", Argument: "18999", }, From ed1ea13ff9b5ca9887ef000f1acf5951ef81823e Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Mon, 13 Jan 2020 14:44:49 -0800 Subject: [PATCH 3/5] Add integration test for field number setting --- .../proto/proto-test-a.compress.parent.child.formatted-txt | 2 +- .../proto-test-a.nocompress.parent.child.formatted-txt | 2 +- ygen/testdata/proto/proto-test-a.yang | 6 +++++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ygen/testdata/proto/proto-test-a.compress.parent.child.formatted-txt b/ygen/testdata/proto/proto-test-a.compress.parent.child.formatted-txt index bad4886df..656aa627d 100644 --- a/ygen/testdata/proto/proto-test-a.compress.parent.child.formatted-txt +++ b/ygen/testdata/proto/proto-test-a.compress.parent.child.formatted-txt @@ -12,7 +12,7 @@ import "github.com/openconfig/ygot/proto/yext/yext.proto"; // Child represents the /proto-test-a/parent/child YANG schema element. message Child { - ywrapper.BoolValue boolean = 135159880; + ywrapper.BoolValue boolean = 101; ywrapper.IntValue integer = 367917455; repeated ywrapper.StringValue leaf_list = 370551192; ywrapper.StringValue leaf_with_dashes = 503746721; diff --git a/ygen/testdata/proto/proto-test-a.nocompress.parent.child.formatted-txt b/ygen/testdata/proto/proto-test-a.nocompress.parent.child.formatted-txt index 7bc35cd4c..f44df7f48 100644 --- a/ygen/testdata/proto/proto-test-a.nocompress.parent.child.formatted-txt +++ b/ygen/testdata/proto/proto-test-a.nocompress.parent.child.formatted-txt @@ -25,7 +25,7 @@ message Config { // State represents the /proto-test-a/parent/child/state YANG schema element. message State { - ywrapper.BoolValue boolean = 135159880; + ywrapper.BoolValue boolean = 101; ywrapper.IntValue integer = 486380674; repeated ywrapper.StringValue leaf_list = 256667601; ywrapper.StringValue leaf_with_dashes = 475722830; diff --git a/ygen/testdata/proto/proto-test-a.yang b/ygen/testdata/proto/proto-test-a.yang index 1ea4dbb72..6dfc43ee4 100644 --- a/ygen/testdata/proto/proto-test-a.yang +++ b/ygen/testdata/proto/proto-test-a.yang @@ -22,7 +22,11 @@ module proto-test-a { } grouping child-state { - leaf boolean { type boolean; } + leaf boolean { + type boolean; + occodegenext:field-number 1; + occodegenext:field-number-offset 100; + } } container parent { From 5ff177f4ad9d7adb9b731bdd4f8e97b22d718600 Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Mon, 13 Jan 2020 14:52:15 -0800 Subject: [PATCH 4/5] Restrict annotated field numbers to 1-1000 --- ygen/protogen.go | 4 ++-- ygen/protogen_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ygen/protogen.go b/ygen/protogen.go index 315958017..d284b7a94 100644 --- a/ygen/protogen.go +++ b/ygen/protogen.go @@ -1119,8 +1119,8 @@ func protoTagForEntry(e *yang.Entry) (uint32, error) { return 0, fmt.Errorf("%s cannot be 0", ext.Keyword) } annotatedFieldNumber := uint32(fn) + offset - if annotatedFieldNumber >= 19000 && annotatedFieldNumber <= 19999 { - return 0, fmt.Errorf("%s field number %d in Protobuf reserved range", + if annotatedFieldNumber > 1000 || annotatedFieldNumber < 1 { + return 0, fmt.Errorf("%s field number %d not in annotation reserved range of 1-1000", e.Name, annotatedFieldNumber) } return annotatedFieldNumber, nil diff --git a/ygen/protogen_test.go b/ygen/protogen_test.go index 2869b9a4a..315455c3b 100644 --- a/ygen/protogen_test.go +++ b/ygen/protogen_test.go @@ -723,7 +723,7 @@ func TestGenProto3Msg(t *testing.T) { }, }, }, { - name: "message with field number in reserved range", + name: "message with field number outside of annotation range", inMsg: &Directory{ Name: "MessageName", Entry: &yang.Entry{ @@ -742,7 +742,7 @@ func TestGenProto3Msg(t *testing.T) { }, { Keyword: "occodegenext:field-number-offset", - Argument: "18999", + Argument: "1000", }, }, }, From 179dfb678798518a6c00071881f535ce66fb7a31 Mon Sep 17 00:00:00 2001 From: wenovus Date: Mon, 21 Jun 2021 15:32:49 -0700 Subject: [PATCH 5/5] Support explicitly setting proto field tags. Credits to @jasonewang for posting the first implementation at #339. I've modified this a bit to read the extension values from the new openconfig-codegen-extensions.yang model that's now in openconfig/public, and also to address some of the corner cases that were discussed in the PR. Spec: https://github.com/openconfig/ygot/blob/master/docs/yang-to-protobuf-transformations-spec.md#field-numbering Corner cases not mentioned in the spec: 1. multiple `field-number` extensions. - Error out. 1. multiple `field-number-offset` extensions. - Add them up. 1. `field-number` or `field-number-offset` extensions on non-leaves. - Silently ignore. 1. `field-number-offset` on non-uses statements, including leafs. - Allow. 1. Conflicts. - Allow. The proto compilation will of course fail. Automated checking can be added here if we'd prefer. I made some simple choices for the above items. Erroring out for 3 and 4 would likely require changing `goyang` to specifically support checking these cases, which I'm not sure is worth it here. --- go.mod | 2 +- go.sum | 4 +- ygen/codegen_test.go | 7 +- ygen/protogen.go | 67 +++--- ygen/protogen_test.go | 194 +++++++++++++++++- .../proto/openconfig-codegen-extensions.yang | 80 ++++++++ ...test-a.compress.parent.child.formatted-txt | 3 +- ...proto-test-a.compress.parent.formatted-txt | 1 + ...st-a.nocompress.parent.child.formatted-txt | 2 +- ygen/testdata/proto/proto-test-a.yang | 6 +- 10 files changed, 323 insertions(+), 43 deletions(-) create mode 100644 ygen/testdata/proto/openconfig-codegen-extensions.yang diff --git a/go.mod b/go.mod index 71d7d6f38..98dbdf026 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/google/go-cmp v0.5.5 github.com/kylelemons/godebug v1.1.0 github.com/openconfig/gnmi v0.0.0-20200508230933-d19cebf5e7be - github.com/openconfig/goyang v0.2.5 + github.com/openconfig/goyang v0.2.6 github.com/openconfig/gribi v0.1.1-0.20210423184541-ce37eb4ba92f github.com/pmezard/go-difflib v1.0.0 google.golang.org/genproto v0.0.0-20201214200347-8c77b98c765d diff --git a/go.sum b/go.sum index 423902501..705900b8a 100644 --- a/go.sum +++ b/go.sum @@ -44,8 +44,8 @@ github.com/openconfig/gnmi v0.0.0-20200508230933-d19cebf5e7be h1:VEK8utxoyZu/hkp github.com/openconfig/gnmi v0.0.0-20200508230933-d19cebf5e7be/go.mod h1:M/EcuapNQgvzxo1DDXHK4tx3QpYM/uG4l591v33jG2A= github.com/openconfig/goyang v0.0.0-20200115183954-d0a48929f0ea/go.mod h1:dhXaV0JgHJzdrHi2l+w0fZrwArtXL7jEFoiqLEdmkvU= github.com/openconfig/goyang v0.2.2/go.mod h1:vX61x01Q46AzbZUzG617vWqh/cB+aisc+RrNkXRd3W8= -github.com/openconfig/goyang v0.2.5 h1:ZvV+5cF5thPFun1H6/Itt/Jbwb/bKmjS0o8imN+7ddw= -github.com/openconfig/goyang v0.2.5/go.mod h1:vX61x01Q46AzbZUzG617vWqh/cB+aisc+RrNkXRd3W8= +github.com/openconfig/goyang v0.2.6 h1:uJlr2bOUe/9mTtnIXgWG9VBqZd/BeCKur4+wAU2jJPc= +github.com/openconfig/goyang v0.2.6/go.mod h1:vX61x01Q46AzbZUzG617vWqh/cB+aisc+RrNkXRd3W8= github.com/openconfig/gribi v0.1.1-0.20210423184541-ce37eb4ba92f h1:8vRtC+y0xh9BYPrEGf/jG/paYXiDUJ6P8iYt5rCVols= github.com/openconfig/gribi v0.1.1-0.20210423184541-ce37eb4ba92f/go.mod h1:OoH46A2kV42cIXGyviYmAlGmn6cHjGduyC2+I9d/iVs= github.com/openconfig/ygot v0.6.0/go.mod h1:o30svNf7O0xK+R35tlx95odkDmZWS9JyWWQSmIhqwAs= diff --git a/ygen/codegen_test.go b/ygen/codegen_test.go index 2cf694643..ccdabf549 100644 --- a/ygen/codegen_test.go +++ b/ygen/codegen_test.go @@ -1967,8 +1967,11 @@ func TestGenerateProto3(t *testing.T) { wantOutputFiles map[string]string wantErr bool }{{ - name: "simple protobuf test with compression", - inFiles: []string{filepath.Join(TestRoot, "testdata", "proto", "proto-test-a.yang")}, + name: "simple protobuf test with compression", + inFiles: []string{ + filepath.Join(TestRoot, "testdata", "proto", "proto-test-a.yang"), + filepath.Join(TestRoot, "testdata", "proto", "openconfig-codegen-extensions.yang"), + }, inConfig: GeneratorConfig{ TransformationOptions: TransformationOpts{ CompressBehaviour: genutil.PreferIntendedConfig, diff --git a/ygen/protogen.go b/ygen/protogen.go index deae832d6..e34351f79 100644 --- a/ygen/protogen.go +++ b/ygen/protogen.go @@ -93,6 +93,13 @@ const ( protoMatchingListNameKeySuffix = "key" ) +// Names from https://github.com/openconfig/public/blob/master/release/models/extensions/openconfig-codegen-extensions.yang +const ( + codegenExtModuleName = "openconfig-codegen-extensions" + fieldNumExtName = "field-number" + offsetExtName = "field-number-offset" +) + // protoMsgField describes a field of a protobuf message. // Note, throughout this package private structs that have public fields are used // in text/template which cannot refer to unexported fields. @@ -1048,40 +1055,44 @@ func safeProtoIdentifierName(name string) string { } // protoTagForEntry returns a protobuf tag value for the entry e. +// If the entry has user-specified field numberings, then use that instead: +// https://github.com/openconfig/ygot/blob/master/docs/yang-to-protobuf-transformations-spec.md#field-numbering func protoTagForEntry(e *yang.Entry) (uint32, error) { - var offset uint32 - for _, ext := range e.Exts { - if ext.Keyword == "occodegenext:field-number-offset" { - off, err := strconv.ParseUint(ext.Argument, 10, 32) - if err != nil { - return 0, fmt.Errorf("could not parse %s %s: %s", ext.Keyword, ext.Argument, err) - } - offset = uint32(off) - if offset == 0 { - return 0, fmt.Errorf("%s cannot be 0", ext.Keyword) - } - break + var fieldNum uint32 + exts, err := yang.MatchingEntryExtensions(e, codegenExtModuleName, fieldNumExtName) + if err != nil { + return 0, err + } + switch len(exts) { + case 0: + return fieldTag(e.Path()) + case 1: + num, err := strconv.ParseUint(exts[0].Argument, 10, 32) + if err != nil || num == 0 { + return 0, fmt.Errorf("could not parse %s:%s %q as a non-zero integer: %s", codegenExtModuleName, fieldNumExtName, exts[0].Argument, err) } + fieldNum = uint32(num) + default: + return 0, fmt.Errorf("more than one %s:%s defined", codegenExtModuleName, fieldNumExtName) } - for _, ext := range e.Exts { - if ext.Keyword == "occodegenext:field-number" { - fn, err := strconv.ParseUint(ext.Argument, 10, 32) - if err != nil { - return 0, fmt.Errorf("could not parse %s %s: %s", ext.Keyword, ext.Argument, err) - } - if fn == 0 { - return 0, fmt.Errorf("%s cannot be 0", ext.Keyword) - } - annotatedFieldNumber := uint32(fn) + offset - if annotatedFieldNumber > 1000 || annotatedFieldNumber < 1 { - return 0, fmt.Errorf("%s field number %d not in annotation reserved range of 1-1000", - e.Name, annotatedFieldNumber) - } - return annotatedFieldNumber, nil + + exts, err = yang.MatchingEntryExtensions(e, codegenExtModuleName, offsetExtName) + if err != nil { + return 0, err + } + for _, ext := range exts { + num, err := strconv.ParseUint(ext.Argument, 10, 32) + if err != nil { + return 0, fmt.Errorf("could not parse %s:%s %q as an uint: %s", codegenExtModuleName, offsetExtName, ext.Argument, err) } + // Allow for multiple offsets to accumulate. + fieldNum += uint32(num) } - return fieldTag(e.Path()) + if fieldNum > 1000 || fieldNum < 1 { + return 0, fmt.Errorf("%s: %d not in reserved range of 1-1000 for %s:%s", e.Path(), fieldNum, codegenExtModuleName, offsetExtName) + } + return fieldNum, nil } // fieldTag takes an input string and calculates a FNV hash for the value. If the diff --git a/ygen/protogen_test.go b/ygen/protogen_test.go index 31d6a1fec..f649cf250 100644 --- a/ygen/protogen_test.go +++ b/ygen/protogen_test.go @@ -20,6 +20,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/kylelemons/godebug/pretty" + "github.com/openconfig/gnmi/errdiff" "github.com/openconfig/goyang/pkg/yang" "github.com/openconfig/ygot/testutil" ) @@ -63,7 +64,7 @@ func TestGenProto3Msg(t *testing.T) { inParentPackage string inChildMsgs []*generatedProto3Message wantMsgs map[string]*protoMsg - wantErr bool + wantErrSubstring string }{{ name: "simple message with only scalar fields", inMsg: &Directory{ @@ -539,7 +540,7 @@ func TestGenProto3Msg(t *testing.T) { }, }, }, - wantErr: true, + wantErrSubstring: "could not resolve", }, { name: "message with an unimplemented mapping", inMsg: &Directory{ @@ -565,7 +566,7 @@ func TestGenProto3Msg(t *testing.T) { }, Path: []string{"", "messasge-with-invalid-contents", "unimplemented"}, }, - wantErr: true, + wantErrSubstring: "unimplemented type", }, { name: "message with any anydata field", inMsg: &Directory{ @@ -673,6 +674,26 @@ func TestGenProto3Msg(t *testing.T) { Argument: "100", }, }, + Node: &yang.Leaf{ + Name: "field-one", + Parent: &yang.Container{ + Name: "message-name", + Parent: &yang.Module{ + Name: "module", + Prefix: &yang.Value{ + Name: "foo", + }, + Import: []*yang.Import{{ + Prefix: &yang.Value{ + Name: "occodegenext", + }, + Module: &yang.Module{ + Name: codegenExtModuleName, + }, + }}, + }, + }, + }, }, "field-two": { Name: "field-two", @@ -683,6 +704,26 @@ func TestGenProto3Msg(t *testing.T) { Argument: "1", }, }, + Node: &yang.Leaf{ + Name: "field-two", + Parent: &yang.Container{ + Name: "message-name", + Parent: &yang.Module{ + Name: "module", + Prefix: &yang.Value{ + Name: "foo", + }, + Import: []*yang.Import{{ + Prefix: &yang.Value{ + Name: "occodegenext", + }, + Module: &yang.Module{ + Name: codegenExtModuleName, + }, + }}, + }, + }, + }, }, "field-three": { Name: "field-three", @@ -697,6 +738,64 @@ func TestGenProto3Msg(t *testing.T) { Argument: "100", }, }, + Node: &yang.Leaf{ + Name: "field-three", + Parent: &yang.Container{ + Name: "message-name", + Parent: &yang.Module{ + Name: "module", + Prefix: &yang.Value{ + Name: "foo", + }, + Import: []*yang.Import{{ + Prefix: &yang.Value{ + Name: "occodegenext", + }, + Module: &yang.Module{ + Name: codegenExtModuleName, + }, + }}, + }, + }, + }, + }, + "field-four": { + Name: "field-four", + Type: &yang.YangType{Kind: yang.Yint8}, + Exts: []*yang.Statement{ + { + Keyword: "occodegenext:field-number", + Argument: "1", + }, + { + Keyword: "occodegenext:field-number-offset", + Argument: "100", + }, + { + Keyword: "occodegenext:field-number-offset", + Argument: "200", + }, + }, + Node: &yang.Leaf{ + Name: "field-four", + Parent: &yang.Container{ + Name: "message-name", + Parent: &yang.Module{ + Name: "module", + Prefix: &yang.Value{ + Name: "foo", + }, + Import: []*yang.Import{{ + Prefix: &yang.Value{ + Name: "occodegenext", + }, + Module: &yang.Module{ + Name: codegenExtModuleName, + }, + }}, + }, + }, + }, }, }, Path: []string{"", "root", "message-name"}, @@ -719,6 +818,10 @@ func TestGenProto3Msg(t *testing.T) { Tag: 101, Name: "field_three", Type: "ywrapper.IntValue", + }, { + Tag: 301, + Name: "field_four", + Type: "ywrapper.IntValue", }}, }, }, @@ -745,11 +848,79 @@ func TestGenProto3Msg(t *testing.T) { Argument: "1000", }, }, + Node: &yang.Leaf{ + Name: "field-one", + Parent: &yang.Container{ + Name: "message-name", + Parent: &yang.Module{ + Name: "module", + Prefix: &yang.Value{ + Name: "foo", + }, + Import: []*yang.Import{{ + Prefix: &yang.Value{ + Name: "occodegenext", + }, + Module: &yang.Module{ + Name: codegenExtModuleName, + }, + }}, + }, + }, + }, }, }, Path: []string{"", "root", "message-name"}, }, - wantErr: true, + wantErrSubstring: "not in reserved range", + }, { + name: "message with duplicate field numbers", + inMsg: &Directory{ + Name: "MessageName", + Entry: &yang.Entry{ + Name: "message-name", + Dir: map[string]*yang.Entry{}, + Kind: yang.DirectoryEntry, + }, + Fields: map[string]*yang.Entry{ + "field-one": { + Name: "field-one", + Type: &yang.YangType{Kind: yang.Ystring}, + Exts: []*yang.Statement{ + { + Keyword: "occodegenext:field-number", + Argument: "1", + }, + { + Keyword: "occodegenext:field-number", + Argument: "1000", + }, + }, + Node: &yang.Leaf{ + Name: "field-one", + Parent: &yang.Container{ + Name: "message-name", + Parent: &yang.Module{ + Name: "module", + Prefix: &yang.Value{ + Name: "foo", + }, + Import: []*yang.Import{{ + Prefix: &yang.Value{ + Name: "occodegenext", + }, + Module: &yang.Module{ + Name: codegenExtModuleName, + }, + }}, + }, + }, + }, + }, + }, + Path: []string{"", "root", "message-name"}, + }, + wantErrSubstring: "more than one", }} for _, tt := range tests { @@ -771,11 +942,20 @@ func TestGenProto3Msg(t *testing.T) { annotateSchemaPaths: tt.inAnnotateSchemaPaths, }, tt.inParentPackage, tt.inChildMsgs, true, true) - if (errs != nil) != tt.wantErr { - t.Errorf("s: genProtoMsg(%#v, %#v, *genState, %v, %v, %s, %s): did not get expected error status, got: %v, wanted err: %v", tt.name, tt.inMsg, tt.inMsgs, tt.inCompressPaths, tt.inBasePackage, tt.inEnumPackage, errs, tt.wantErr) + var err error + switch len(errs) { + case 1: + err = errs[0] + fallthrough + case 0: + if diff := errdiff.Substring(err, tt.wantErrSubstring); diff != "" { + t.Fatalf("s: genProtoMsg(%#v, %#v, *genState, %v, %v, %s, %s): did not get expected error: %s", tt.name, tt.inMsg, tt.inMsgs, tt.inCompressPaths, tt.inBasePackage, tt.inEnumPackage, diff) + } + default: + t.Fatalf("got more than 1 error: %v", errs) } - if tt.wantErr { + if tt.wantErrSubstring != "" { return } diff --git a/ygen/testdata/proto/openconfig-codegen-extensions.yang b/ygen/testdata/proto/openconfig-codegen-extensions.yang new file mode 100644 index 000000000..da7631e6c --- /dev/null +++ b/ygen/testdata/proto/openconfig-codegen-extensions.yang @@ -0,0 +1,80 @@ +module openconfig-codegen-extensions { + + yang-version "1"; + + // namespace + namespace "http://openconfig.net/yang/openconfig-codegen-ext"; + + prefix "oc-codegen-ext"; + + // import some basic types + import openconfig-extensions { prefix oc-ext; } + + // meta + organization "OpenConfig working group"; + + contact + "OpenConfig working group + www.openconfig.net"; + + description + "This module provides OpenConfig-specific code generation extensions to the + YANG language."; + + oc-ext:openconfig-version "0.1.0"; + + revision "2020-10-05" { + description + "Initial commit of code generation extensions."; + reference "0.1.0"; + } + + // extension statements + + extension camelcase-name { + argument "name" { + yin-element false; + } + description + "When specified, this extension indicates a specific CamelCase name that + is to be used for the field, for example, this can allow for more natural + capitalisation than can be achieved algorithmically (e.g., IPv4Address rather + than Ipv4Address)."; + } + + extension field-number { + argument "number" { + yin-element false; + } + description + "field-number is used to explicitly specify the field number used in the + protocol buffer generated code instead of auto-generating them. + + Only 1-1000 are valid numbers. The rest is either reserved for Protobuf + internal usage or for use by the generated code when generating field + numbers automatically. + + Specification at + https://github.com/openconfig/ygot/blob/master/docs/yang-to-protobuf-transformations-spec.md#field-numbering"; + } + + extension field-number-offset { + argument "offset" { + yin-element false; + } + description + "field-number-offset is used within a uses statement to specify the + offset that is added to every explicit field-number for fields directly + within the grouping. + + When field-number is used to explicitly specify Protobuf field numbers + used in the protocol buffer generated code, it's possible that different + fields having the same field number could collide when multiple logical + groupings are imported into the same schema tree location. + field-number-offset helps resolve this by adding a different offset to + each grouping. + + Specification at + https://github.com/openconfig/ygot/blob/master/docs/yang-to-protobuf-transformations-spec.md#field-numbering"; + } +} diff --git a/ygen/testdata/proto/proto-test-a.compress.parent.child.formatted-txt b/ygen/testdata/proto/proto-test-a.compress.parent.child.formatted-txt index 656aa627d..a49c5387d 100644 --- a/ygen/testdata/proto/proto-test-a.compress.parent.child.formatted-txt +++ b/ygen/testdata/proto/proto-test-a.compress.parent.child.formatted-txt @@ -3,6 +3,7 @@ // // Input schema modules: // - testdata/proto/proto-test-a.yang +// - testdata/proto/openconfig-codegen-extensions.yang syntax = "proto3"; package openconfig.parent; @@ -12,7 +13,7 @@ import "github.com/openconfig/ygot/proto/yext/yext.proto"; // Child represents the /proto-test-a/parent/child YANG schema element. message Child { - ywrapper.BoolValue boolean = 101; + ywrapper.BoolValue boolean = 201; ywrapper.IntValue integer = 367917455; repeated ywrapper.StringValue leaf_list = 370551192; ywrapper.StringValue leaf_with_dashes = 503746721; diff --git a/ygen/testdata/proto/proto-test-a.compress.parent.formatted-txt b/ygen/testdata/proto/proto-test-a.compress.parent.formatted-txt index bcb9c5e86..56c64ccbb 100644 --- a/ygen/testdata/proto/proto-test-a.compress.parent.formatted-txt +++ b/ygen/testdata/proto/proto-test-a.compress.parent.formatted-txt @@ -3,6 +3,7 @@ // // Input schema modules: // - testdata/proto/proto-test-a.yang +// - testdata/proto/openconfig-codegen-extensions.yang syntax = "proto3"; package openconfig; diff --git a/ygen/testdata/proto/proto-test-a.nocompress.parent.child.formatted-txt b/ygen/testdata/proto/proto-test-a.nocompress.parent.child.formatted-txt index f44df7f48..9a9912f03 100644 --- a/ygen/testdata/proto/proto-test-a.nocompress.parent.child.formatted-txt +++ b/ygen/testdata/proto/proto-test-a.nocompress.parent.child.formatted-txt @@ -25,7 +25,7 @@ message Config { // State represents the /proto-test-a/parent/child/state YANG schema element. message State { - ywrapper.BoolValue boolean = 101; + ywrapper.BoolValue boolean = 201; ywrapper.IntValue integer = 486380674; repeated ywrapper.StringValue leaf_list = 256667601; ywrapper.StringValue leaf_with_dashes = 475722830; diff --git a/ygen/testdata/proto/proto-test-a.yang b/ygen/testdata/proto/proto-test-a.yang index 6dfc43ee4..b6b93780e 100644 --- a/ygen/testdata/proto/proto-test-a.yang +++ b/ygen/testdata/proto/proto-test-a.yang @@ -6,6 +6,8 @@ module proto-test-a { "Test YANG simple scheam for protobuf translation."; + import openconfig-codegen-extensions { prefix occodegenext; } + grouping child-cfg { leaf string { type string; } leaf integer { type int64; } @@ -38,7 +40,9 @@ module proto-test-a { container state { config false; uses child-cfg; - uses child-state; + uses child-state { + occodegenext:field-number-offset 100; + } } } }