feat: Migrate fully to protov2 (#1833)

.github/workflows/ci.yml

@@ -32,7 +32,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions/setup-go@v1
         with:
-          go-version: 1.13
+          go-version: 1.17
       - run: go mod tidy -v
       - name: Check for changes in go.mod
         run: |
@@ -44,32 +44,16 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions/setup-go@v1
         with:
-          go-version: 1.13
+          go-version: 1.17
       - run: |
           go build -v ./...
-
-#  shellcheck:
-#    name: shellcheck
-#    runs-on: ubuntu-latest
-#    steps:
-#      - uses: actions/checkout@v2
-#      - name: shellcheck
-#        uses: reviewdog/action-shellcheck@v1
-#        with:
-#          github_token: ${{ secrets.github_token }}
-#          reporter: github-check
-#  test:
-#    name: test
-#    runs-on: ubuntu-latest
-#    steps:
-#      - uses: actions/checkout@v2
-#      - uses: actions/setup-go@v1
-#        with:
-#          go-version: 1.13
-#      - name: Install gotestsum
-#        run: go get gotest.tools/gotestsum@v0.4.0
-#      - name: Run tests
-#        run: |
-#         eval $(go env)
-#         mkdir -p ~/junit/
-#         ${GOPATH}/bin/gotestsum --junitfile ~/junit/unit-tests.xml -- -short $(go list ./...)
+  unittest:
+    name: unit test
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-go@v1
+        with:
+          go-version: 1.17
+      - run: |
+          go test -v ./...

client/client_api.go

@@ -17,7 +17,8 @@ package client
 import (
 	"context"
 
-	"github.com/golang/protobuf/proto"
+	"google.golang.org/protobuf/proto"
+
 	"go.ligato.io/vpp-agent/v3/pkg/models"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/generic"
 )

client/dynamic_config.go

@@ -4,19 +4,19 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/go-errors/errors"
 	"github.com/goccy/go-yaml"
 	"go.ligato.io/cn-infra/v2/logging/logrus"
-	"go.ligato.io/vpp-agent/v3/pkg/models"
-	"go.ligato.io/vpp-agent/v3/pkg/util"
-	"go.ligato.io/vpp-agent/v3/proto/ligato/generic"
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protodesc"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/reflect/protoregistry"
 	"google.golang.org/protobuf/types/descriptorpb"
 	"google.golang.org/protobuf/types/dynamicpb"
+
+	"go.ligato.io/vpp-agent/v3/pkg/models"
+	"go.ligato.io/vpp-agent/v3/pkg/util"
+	"go.ligato.io/vpp-agent/v3/proto/ligato/generic"
 )
 
 // field proto name/json name constants (can't be changes to not break json/yaml compatibility with configurator.Config)
@@ -92,19 +92,19 @@ func NewDynamicConfig(knownModels []*models.ModelInfo) (*dynamicpb.Message, erro
 	// create dependency registry
 	dependencyRegistry, err := createFileDescRegistry(knownModels)
 	if err != nil {
-		return nil, errors.Errorf("can't create dependency file descriptor registry due to: %v", err)
+		return nil, fmt.Errorf("cannot create dependency file descriptor registry due to: %w", err)
 	}
 
 	// get file descriptor proto for give known models
 	fileDP, rootMsgName, err := createDynamicConfigDescriptorProto(knownModels, dependencyRegistry)
 	if err != nil {
-		return nil, errors.Errorf("can't create descriptor proto for dynamic config due to: %v", err)
+		return nil, fmt.Errorf("cannot create descriptor proto for dynamic config due to: %v", err)
 	}
 
 	// convert file descriptor proto to file descriptor
 	fd, err := protodesc.NewFile(fileDP, dependencyRegistry)
 	if err != nil {
-		return nil, errors.Errorf("can't convert file descriptor proto to file descriptor due to: %v", err)
+		return nil, fmt.Errorf("cannot convert file descriptor proto to file descriptor due to: %v", err)
 	}
 
 	// get descriptor for config root message
@@ -200,28 +200,28 @@ func createDynamicConfigDescriptorProto(knownModels []*ModelInfo, dependencyRegi
 			TypeName: proto.String(fmt.Sprintf(".%v", modelDetail.ProtoName)),
 		})
 
-		//add proto file dependency for this known model (+ check that it is in dependency file descriptor registry)
+		// add proto file dependency for this known model (+ check that it is in dependency file descriptor registry)
 		protoFile, err := ModelOptionFor("protoFile", modelDetail.Options)
 		if err != nil {
-			error = errors.Errorf("can't retrieve protoFile from model options "+
+			error = fmt.Errorf("cannot retrieve protoFile from model options "+
 				"from model %v due to: %v", modelDetail.ProtoName, err)
 			return
 		}
 		if _, found := importedDependency[protoFile]; !found {
 			importedDependency[protoFile] = struct{}{}
 
-			//add proto file dependency for this known model
+			// add proto file dependency for this known model
 			fileDP.Dependency = append(fileDP.Dependency, protoFile)
 
 			// checking dependency registry that should already contain the linked dependency
 			if _, err := dependencyRegistry.FindFileByPath(protoFile); err != nil {
 				if err == protoregistry.NotFound {
-					error = errors.Errorf("proto file %v need to be referenced in dynamic config, but it "+
+					error = fmt.Errorf("proto file %v need to be referenced in dynamic config, but it "+
 						"is not in dependency registry that was created from file descriptor proto input "+
 						"(missing in input? check debug output from creating dependency registry) ", protoFile)
 					return
 				}
-				error = errors.Errorf("can't verify that proto file %v is in "+
+				error = fmt.Errorf("cannot verify that proto file %v is in "+
 					"dependency registry, it is due to: %v", protoFile, err)
 				return
 			}
@@ -265,7 +265,7 @@ func DynamicConfigKnownModelFieldNaming(modelDetail *models.ModelInfo) (protoNam
 // dynamic config (i.e. after json/yaml loading into dynamic config).
 func DynamicConfigExport(dynamicConfig *dynamicpb.Message) ([]proto.Message, error) {
 	if dynamicConfig == nil {
-		return nil, errors.Errorf("dynamic config can't be nil")
+		return nil, fmt.Errorf("dynamic config cannot be nil")
 	}
 
 	// iterate over config group messages and extract proto message from them
@@ -300,17 +300,17 @@ func ExportDynamicConfigStructure(dynamicConfig proto.Message) (string, error) {
 	}
 	b, err := m.Marshal(dynamicConfig)
 	if err != nil {
-		return "", errors.Errorf("can't marshal dynamic config to json due to: %v", err)
+		return "", fmt.Errorf("cannot marshal dynamic config to json due to: %v", err)
 	}
 	var jsonObj interface{}
 	err = yaml.UnmarshalWithOptions(b, &jsonObj, yaml.UseOrderedMap())
 	if err != nil {
-		return "", errors.Errorf("can't convert dynamic config's json bytes to "+
+		return "", fmt.Errorf("cannot convert dynamic config's json bytes to "+
 			"json struct for yaml marshalling due to: %v", err)
 	}
 	bb, err := yaml.Marshal(jsonObj)
 	if err != nil {
-		return "", errors.Errorf("can't marshal dynamic config from json to yaml due to: %v", err)
+		return "", fmt.Errorf("cannot marshal dynamic config from json to yaml due to: %v", err)
 	}
 	return string(bb), nil
 }
@@ -348,16 +348,16 @@ func ModelOptionFor(key string, options []*generic.ModelDetail_Option) (string,
 	for _, option := range options {
 		if option.Key == key {
 			if len(option.Values) == 0 {
-				return "", errors.Errorf("there is no value for key %v in model options", key)
+				return "", fmt.Errorf("there is no value for key %v in model options", key)
 			}
 			if strings.TrimSpace(option.Values[0]) == "" {
-				return "", errors.Errorf("there is no value(only empty string "+
+				return "", fmt.Errorf("there is no value(only empty string "+
 					"after trimming) for key %v in model options", key)
 			}
 			return option.Values[0], nil
 		}
 	}
-	return "", errors.Errorf("there is no model option with key %v (model options=%+v))", key, options)
+	return "", fmt.Errorf("there is no model option with key %v (model options=%+v))", key, options)
 }
 
 func existsModelOptionFor(key string, options []*generic.ModelDetail_Option) bool {

client/dynamic_config_test.go

@@ -19,21 +19,20 @@ import (
 	"encoding/json"
 	"testing"
 
-	testmodel "go.ligato.io/vpp-agent/v3/pkg/models/testdata/proto"
-
 	yaml2 "github.com/ghodss/yaml"
 	"github.com/go-errors/errors"
 	"github.com/goccy/go-yaml"
-	protoV1 "github.com/golang/protobuf/proto"
 	. "github.com/onsi/gomega"
+	"google.golang.org/protobuf/encoding/protojson"
+	"google.golang.org/protobuf/proto"
+
 	"go.ligato.io/vpp-agent/v3/client"
 	"go.ligato.io/vpp-agent/v3/pkg/models"
+	testmodel "go.ligato.io/vpp-agent/v3/pkg/models/testdata/proto"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/configurator"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/vpp"
 	interfaces "go.ligato.io/vpp-agent/v3/proto/ligato/vpp/interfaces"
 	vpp_srv6 "go.ligato.io/vpp-agent/v3/proto/ligato/vpp/srv6"
-	"google.golang.org/protobuf/encoding/protojson"
-	"google.golang.org/protobuf/proto"
 )
 
 // TestYamlCompatibility test dynamically generated all-in-one configuration proto message to be compatible
@@ -60,8 +59,8 @@ func TestYamlCompatibility(t *testing.T) {
 	for _, model := range models.RegisteredModels() {
 		if model.Spec().Class == "config" {
 			knownModels = append(knownModels, &models.ModelInfo{
-				ModelDetail:       *model.ModelDetail(),
-				MessageDescriptor: protoV1.MessageV2(model.NewInstance()).ProtoReflect().Descriptor(),
+				ModelDetail:       model.ModelDetail(),
+				MessageDescriptor: model.NewInstance().ProtoReflect().Descriptor(),
 			})
 		}
 	}
@@ -107,8 +106,8 @@ func TestDynamicConfigWithThirdPartyModel(t *testing.T) {
 	for _, model := range models.RegisteredModels() {
 		if model.Spec().Class == "config" && model.Spec().Module == "model" {
 			knownModels = append(knownModels, &models.ModelInfo{
-				ModelDetail:       *model.ModelDetail(),
-				MessageDescriptor: protoV1.MessageV2(model.NewInstance()).ProtoReflect().Descriptor(),
+				ModelDetail:       model.ModelDetail(),
+				MessageDescriptor: model.NewInstance().ProtoReflect().Descriptor(),
 			})
 		}
 	}

client/local_client.go

@@ -17,16 +17,17 @@ package client
 import (
 	"context"
 
-	"github.com/golang/protobuf/proto"
+	"github.com/sirupsen/logrus"
 	"go.ligato.io/cn-infra/v2/datasync/kvdbsync/local"
 	"go.ligato.io/cn-infra/v2/datasync/syncbase"
 	"go.ligato.io/cn-infra/v2/db/keyval"
+	"google.golang.org/protobuf/proto"
+
 	"go.ligato.io/vpp-agent/v3/pkg/models"
 	"go.ligato.io/vpp-agent/v3/pkg/util"
 	"go.ligato.io/vpp-agent/v3/plugins/orchestrator"
 	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/contextdecorator"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/generic"
-	protoV2 "google.golang.org/protobuf/proto"
 )
 
 // LocalClient is global client for direct local access.
@@ -53,8 +54,8 @@ func (c *client) KnownModels(class string) ([]*ModelInfo, error) {
 	for _, model := range models.RegisteredModels() {
 		if class == "" || model.Spec().Class == class {
 			modules = append(modules, &models.ModelInfo{
-				ModelDetail:       *model.ModelDetail(),
-				MessageDescriptor: proto.MessageV2(model.NewInstance()).ProtoReflect().Descriptor(),
+				ModelDetail:       model.ModelDetail(),
+				MessageDescriptor: model.NewInstance().ProtoReflect().Descriptor(),
 			})
 		}
 	}
@@ -84,7 +85,7 @@ func (c *client) GetConfig(dsts ...interface{}) error {
 		// TODO the clearIgnoreLayerCount function argument should be a option of generic.Client
 		//  (the value 1 generates from dynamic config the same json/yaml output as the hardcoded
 		//  configurator.Config and therefore serves for backward compatibility)
-		util.PlaceProtosIntoProtos(convertToProtoV2(protos), 1, protoDsts...)
+		util.PlaceProtosIntoProtos(protoMapToList(protos), 1, protoDsts...)
 	} else {
 		util.PlaceProtos(protos, dsts...)
 	}
@@ -162,28 +163,24 @@ func (p *txnFactory) NewTxn(resync bool) keyval.ProtoTxn {
 	return local.NewProtoTxn(p.registry.PropagateChanges)
 }
 
-func extractProtoMessages(dsts []interface{}) []protoV2.Message {
-	protoDsts := make([]protoV2.Message, 0)
+func extractProtoMessages(dsts []interface{}) []proto.Message {
+	msgs := make([]proto.Message, 0)
 	for _, dst := range dsts {
-		protoV1Dst, isProtoV1 := dst.(proto.Message)
-		if isProtoV1 {
-			protoDsts = append(protoDsts, proto.MessageV2(protoV1Dst))
+		msg, ok := dst.(proto.Message)
+		if ok {
+			msgs = append(msgs, msg)
 		} else {
-			protoV2Dst, isProtoV2 := dst.(protoV2.Message)
-			if isProtoV2 {
-				protoDsts = append(protoDsts, protoV2Dst)
-			} else {
-				break
-			}
+			logrus.Debugf("at least one of the %d items is not proto message, but: %#v", len(dsts), dst)
+			break
 		}
 	}
-	return protoDsts
+	return msgs
 }
 
-func convertToProtoV2(protoMap map[string]proto.Message) []protoV2.Message {
-	result := make([]protoV2.Message, 0, len(protoMap))
+func protoMapToList(protoMap map[string]proto.Message) []proto.Message {
+	result := make([]proto.Message, 0, len(protoMap))
 	for _, msg := range protoMap {
-		result = append(result, proto.MessageV2(msg))
+		result = append(result, msg)
 	}
 	return result
 }