From efc88f944b6ff58bb6843d0c005ccd7d810d8358 Mon Sep 17 00:00:00 2001 From: Sachin Holla Date: Wed, 24 May 2023 10:15:09 +0530 Subject: [PATCH 1/2] TranslClient: use PathValidator to sanitize the request paths TranslClient now uses the PathValidator utility to preprocess get, set and subscribe request paths before calling translib APIs. PathValidator is setup to performs following actions: * Verify the get/set/subscribe paths are known to translib * Insert missing module name prefixes in get/set/subscribe paths. gNMI allows the clients to omit them but translib APIs need them * Insert missing wildcard keys in subscribe paths Signed-off-by: Sachin Holla --- gnmi_server/client_subscribe.go | 2 +- sonic_data_client/transl_data_client.go | 33 +++--- transl_utils/transl_utils.go | 129 +++++++++++------------- 3 files changed, 80 insertions(+), 84 deletions(-) diff --git a/gnmi_server/client_subscribe.go b/gnmi_server/client_subscribe.go index 85d0d1933..bea9ca50e 100644 --- a/gnmi_server/client_subscribe.go +++ b/gnmi_server/client_subscribe.go @@ -163,7 +163,7 @@ func (c *Client) Run(stream gnmipb.GNMI_SubscribeServer) (err error) { dc, err = sdc.NewDbClient(paths, prefix) } else { /* For any other target or no target create new Transl Client. */ - dc, err = sdc.NewTranslClient(prefix, paths, ctx, extensions) + dc, err = sdc.NewTranslClient(prefix, paths, ctx, extensions, sdc.TranslWildcardOption{}) } if err != nil { diff --git a/sonic_data_client/transl_data_client.go b/sonic_data_client/transl_data_client.go index 08208523a..b850d0cb5 100644 --- a/sonic_data_client/transl_data_client.go +++ b/sonic_data_client/transl_data_client.go @@ -40,16 +40,24 @@ type TranslClient struct { extensions []*gnmi_extpb.Extension } -func NewTranslClient(prefix *gnmipb.Path, getpaths []*gnmipb.Path, ctx context.Context, extensions []*gnmi_extpb.Extension) (Client, error) { +func NewTranslClient(prefix *gnmipb.Path, getpaths []*gnmipb.Path, ctx context.Context, extensions []*gnmi_extpb.Extension, opts ...TranslClientOption) (Client, error) { var client TranslClient var err error client.ctx = ctx client.prefix = prefix client.extensions = extensions + if getpaths != nil { + var addWildcardKeys bool + for _, o := range opts { + if _, ok := o.(TranslWildcardOption); ok { + addWildcardKeys = true + } + } + client.path2URI = make(map[*gnmipb.Path]string) /* Populate GNMI path to REST URL map. */ - err = transutil.PopulateClientPaths(prefix, getpaths, &client.path2URI) + err = transutil.PopulateClientPaths(prefix, getpaths, &client.path2URI, addWildcardKeys) } if err != nil { @@ -99,7 +107,6 @@ func (c *TranslClient) Get(w *sync.WaitGroup) ([]*spb.Value, error) { func (c *TranslClient) Set(delete []*gnmipb.Path, replace []*gnmipb.Update, update []*gnmipb.Update) error { rc, ctx := common_utils.GetContext(c.ctx) c.ctx = ctx - var uri string version := getBundleVersion(c.extensions) if version != nil { rc.BundleVersion = version @@ -109,19 +116,13 @@ func (c *TranslClient) Set(delete []*gnmipb.Path, replace []*gnmipb.Update, upda return transutil.TranslProcessBulk(delete, replace, update, c.prefix, c.ctx) } else { if len(delete) == 1 { - /* Convert the GNMI Path to URI. */ - transutil.ConvertToURI(c.prefix, delete[0], &uri) - return transutil.TranslProcessDelete(uri, c.ctx) + return transutil.TranslProcessDelete(c.prefix, delete[0], c.ctx) } if len(replace) == 1 { - /* Convert the GNMI Path to URI. */ - transutil.ConvertToURI(c.prefix, replace[0].GetPath(), &uri) - return transutil.TranslProcessReplace(uri, replace[0].GetVal(), c.ctx) + return transutil.TranslProcessReplace(c.prefix, replace[0], c.ctx) } if len(update) == 1 { - /* Convert the GNMI Path to URI. */ - transutil.ConvertToURI(c.prefix, update[0].GetPath(), &uri) - return transutil.TranslProcessUpdate(uri, update[0].GetVal(), c.ctx) + return transutil.TranslProcessUpdate(c.prefix, update[0], c.ctx) } } return nil @@ -541,3 +542,11 @@ func getBundleVersion(extensions []*gnmi_extpb.Extension) *string { } return nil } + +type TranslClientOption interface { + IsTranslClientOption() +} + +type TranslWildcardOption struct{} + +func (t TranslWildcardOption) IsTranslClientOption() {} diff --git a/transl_utils/transl_utils.go b/transl_utils/transl_utils.go index b6b33857b..2160fdbf1 100644 --- a/transl_utils/transl_utils.go +++ b/transl_utils/transl_utils.go @@ -2,17 +2,19 @@ package transl_utils import ( "bytes" + "context" "encoding/json" - "strings" "fmt" + "log/syslog" + "strings" + + "github.com/Azure/sonic-mgmt-common/translib" + pathutil "github.com/Azure/sonic-mgmt-common/translib/path" + "github.com/Azure/sonic-mgmt-common/translib/tlerr" log "github.com/golang/glog" gnmipb "github.com/openconfig/gnmi/proto/gnmi" - "github.com/Azure/sonic-mgmt-common/translib" + "github.com/openconfig/ygot/ygot" "github.com/sonic-net/sonic-gnmi/common_utils" - "context" - "log/syslog" - "github.com/Azure/sonic-mgmt-common/translib/tlerr" - ) var ( @@ -58,55 +60,40 @@ func GnmiTranslFullPath(prefix, path *gnmipb.Path) *gnmipb.Path { } /* Populate the URI path corresponding GNMI paths. */ -func PopulateClientPaths(prefix *gnmipb.Path, paths []*gnmipb.Path, path2URI *map[*gnmipb.Path]string) error { - var req string - - /* Fetch the URI for each GET URI. */ +func PopulateClientPaths(prefix *gnmipb.Path, paths []*gnmipb.Path, path2URI *map[*gnmipb.Path]string, addWildcardKeys bool) error { + opts := []pathutil.PathValidatorOpt{ + &pathutil.AppendModulePrefix{}, + } + if addWildcardKeys { + opts = append(opts, &pathutil.AddWildcardKeys{}) + } for _, path := range paths { - ConvertToURI(prefix, path, &req) + req, err := ConvertToURI(prefix, path, opts...) + if err != nil { + return err + } (*path2URI)[path] = req } return nil } -/* Populate the URI path corresponding each GNMI paths. */ -func ConvertToURI(prefix *gnmipb.Path, path *gnmipb.Path, req *string) error { +// ConvertToURI returns translib path for a gnmi Path +func ConvertToURI(prefix, path *gnmipb.Path, opts ...pathutil.PathValidatorOpt) (string, error) { fullPath := path if prefix != nil { fullPath = GnmiTranslFullPath(prefix, path) } - elems := fullPath.GetElem() - *req = "/" - - if elems != nil { - /* Iterate through elements. */ - for i, elem := range elems { - log.V(6).Infof("index %d elem : %#v %#v", i, elem.GetName(), elem.GetKey()) - *req += elem.GetName() - key := elem.GetKey() - /* If no keys are present end the element with "/" */ - if key == nil { - *req += "/" - } - - /* If keys are present , process the keys. */ - if key != nil { - for k, v := range key { - log.V(6).Infof("elem : %#v %#v", k, v) - *req += "[" + k + "=" + v + "]" - } - - /* Append "/" after all keys are processed. */ - *req += "/" - } - } + if len(opts) == 0 { + opts = append(opts, &pathutil.AppendModulePrefix{}) + } + pv := pathutil.NewPathValidator(opts...) + if err := pv.Validate(fullPath); err != nil { + return "", err } - /* Trim the "/" at the end which is not required. */ - *req = strings.TrimSuffix(*req, "/") - return nil + return ygot.PathToString(fullPath) } /* Fill the values from TransLib. */ @@ -150,11 +137,14 @@ func TranslProcessGet(uriPath string, op *string, ctx context.Context) (*gnmipb. } /* Delete request handling. */ -func TranslProcessDelete(uri string, ctx context.Context) error { - var str3 string - payload := []byte(str3) +func TranslProcessDelete(prefix, delPath *gnmipb.Path, ctx context.Context) error { + uri, err := ConvertToURI(prefix, delPath) + if err != nil { + return err + } + rc, _ := common_utils.GetContext(ctx) - req := translib.SetRequest{Path:uri, Payload:payload, User: translib.UserRoles{Name: rc.Auth.User, Roles: rc.Auth.Roles}} + req := translib.SetRequest{Path:uri, User: translib.UserRoles{Name: rc.Auth.User, Roles: rc.Auth.Roles}} if rc.BundleVersion != nil { nver, err := translib.NewVersion(*rc.BundleVersion) if err != nil { @@ -176,13 +166,13 @@ func TranslProcessDelete(uri string, ctx context.Context) error { } /* Replace request handling. */ -func TranslProcessReplace(uri string, t *gnmipb.TypedValue, ctx context.Context) error { - /* Form the CURL request and send to client . */ - str := string(t.GetJsonIetfVal()) - str3 := strings.Replace(str, "\n", "", -1) - log.V(2).Info("Incoming JSON body is", str) +func TranslProcessReplace(prefix *gnmipb.Path, entry *gnmipb.Update, ctx context.Context) error { + uri, err := ConvertToURI(prefix, entry.GetPath()) + if err != nil { + return err + } - payload := []byte(str3) + payload := entry.GetVal().GetJsonIetfVal() rc, _ := common_utils.GetContext(ctx) req := translib.SetRequest{Path:uri, Payload:payload, User: translib.UserRoles{Name: rc.Auth.User, Roles: rc.Auth.Roles}} if rc.BundleVersion != nil { @@ -208,13 +198,13 @@ func TranslProcessReplace(uri string, t *gnmipb.TypedValue, ctx context.Context) } /* Update request handling. */ -func TranslProcessUpdate(uri string, t *gnmipb.TypedValue, ctx context.Context) error { - /* Form the CURL request and send to client . */ - str := string(t.GetJsonIetfVal()) - str3 := strings.Replace(str, "\n", "", -1) - log.V(2).Info("Incoming JSON body is", str) +func TranslProcessUpdate(prefix *gnmipb.Path, entry *gnmipb.Update, ctx context.Context) error { + uri, err := ConvertToURI(prefix, entry.GetPath()) + if err != nil { + return err + } - payload := []byte(str3) + payload := entry.GetVal().GetJsonIetfVal() rc, _ := common_utils.GetContext(ctx) req := translib.SetRequest{Path:uri, Payload:payload, User: translib.UserRoles{Name: rc.Auth.User, Roles: rc.Auth.Roles}} if rc.BundleVersion != nil { @@ -266,12 +256,11 @@ func TranslProcessBulk(delete []*gnmipb.Path, replace []*gnmipb.Update, update [ } } for _,d := range delete { - ConvertToURI(prefix, d, &uri) - var str3 string - payload := []byte(str3) + if uri, err = ConvertToURI(prefix, d); err != nil { + return err + } req := translib.SetRequest{ Path: uri, - Payload: payload, User: translib.UserRoles{Name: rc.Auth.User, Roles: rc.Auth.Roles}, } if rc.BundleVersion != nil { @@ -284,11 +273,10 @@ func TranslProcessBulk(delete []*gnmipb.Path, replace []*gnmipb.Update, update [ deleteUri = append(deleteUri, uri) } for _,r := range replace { - ConvertToURI(prefix, r.GetPath(), &uri) - str := string(r.GetVal().GetJsonIetfVal()) - str3 := strings.Replace(str, "\n", "", -1) - log.V(2).Info("Incoming JSON body is", str) - payload := []byte(str3) + if uri, err = ConvertToURI(prefix, r.GetPath()); err != nil { + return err + } + payload := r.GetVal().GetJsonIetfVal() req := translib.SetRequest{ Path: uri, Payload: payload, @@ -304,11 +292,10 @@ func TranslProcessBulk(delete []*gnmipb.Path, replace []*gnmipb.Update, update [ replaceUri = append(replaceUri, uri) } for _,u := range update { - ConvertToURI(prefix, u.GetPath(), &uri) - str := string(u.GetVal().GetJsonIetfVal()) - str3 := strings.Replace(str, "\n", "", -1) - log.V(2).Info("Incoming JSON body is", str) - payload := []byte(str3) + if uri, err = ConvertToURI(prefix, u.GetPath()); err != nil { + return err + } + payload := u.GetVal().GetJsonIetfVal() req := translib.SetRequest{ Path: uri, Payload: payload, From 796a459a646bd4dbb0225bffac8e612817501ef4 Mon Sep 17 00:00:00 2001 From: Sachin Holla Date: Fri, 2 Jun 2023 10:31:58 +0530 Subject: [PATCH 2/2] Gotests for TranslClient based write cases - Set ENABLE_TRANSLIB_WRITE=y during pipeline tests to enable all TranslClient based write tests. They were skipped earlier - Setup correct translib test env in makefile 'check_gotest' target - Fixed path & payload of existing TranslClient based write tests - Added more get and set tests to cover path validator calls with prefixed and unprefixed paths - Skip unsupported gNOI tests --- Makefile | 20 +++- azure-pipelines.yml | 2 +- gnmi_server/server_test.go | 230 ++++++++++++++++++++++++++++++------- tools/test/env.sh | 4 +- 4 files changed, 208 insertions(+), 48 deletions(-) diff --git a/Makefile b/Makefile index b5a66ccd6..aef43bc96 100644 --- a/Makefile +++ b/Makefile @@ -82,17 +82,25 @@ endif swsscommon_wrap: make -C swsscommon -check_gotest: +DBCONFG = $(DBDIR)/database_config.json +ENVFILE = build/test/env.txt +TESTENV = $(shell cat $(ENVFILE)) + +$(DBCONFG): testdata/database_config.json sudo mkdir -p ${DBDIR} sudo cp ./testdata/database_config.json ${DBDIR} - sudo mkdir -p /usr/models/yang || true - sudo find $(MGMT_COMMON_DIR)/models -name '*.yang' -exec cp {} /usr/models/yang/ \; + +$(ENVFILE): + mkdir -p $(@D) + tools/test/env.sh | grep -v DB_CONFIG_PATH | tee $@ + +check_gotest: $(DBCONFG) $(ENVFILE) sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-config.txt -covermode=atomic -v github.com/sonic-net/sonic-gnmi/sonic_db_config - sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -coverprofile=coverage-gnmi.txt -covermode=atomic -mod=vendor $(BLD_FLAGS) -v github.com/sonic-net/sonic-gnmi/gnmi_server -coverpkg ../... - sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -coverprofile=coverage-dialcout.txt -covermode=atomic -mod=vendor $(BLD_FLAGS) -v github.com/sonic-net/sonic-gnmi/dialout/dialout_client + sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -coverprofile=coverage-gnmi.txt -covermode=atomic -mod=vendor $(BLD_FLAGS) -v github.com/sonic-net/sonic-gnmi/gnmi_server -coverpkg ../... + sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -coverprofile=coverage-dialcout.txt -covermode=atomic -mod=vendor $(BLD_FLAGS) -v github.com/sonic-net/sonic-gnmi/dialout/dialout_client sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-data.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/sonic_data_client sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-dbus.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/sonic_service_client - sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -coverprofile=coverage-translutils.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/transl_utils + sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -coverprofile=coverage-translutils.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/transl_utils $(GO) get github.com/axw/gocov/... $(GO) get github.com/AlekSi/gocov-xml $(GO) mod vendor diff --git a/azure-pipelines.yml b/azure-pipelines.yml index afe10128e..1fabbae87 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -154,7 +154,7 @@ stages: - script: | pushd sonic-gnmi - make check_gotest + make check_gotest ENABLE_TRANSLIB_WRITE=y displayName: "Test" - publish: $(Build.ArtifactStagingDirectory)/ diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 9ff3821fb..662f079c0 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -228,7 +228,7 @@ func runTestGet(t *testing.T, ctx context.Context, gClient pb.GNMIClient, pathTa textPbPath string, wantRetCode codes.Code, wantRespVal interface{}, valTest bool) { //var retCodeOk bool // Send request - + t.Helper() var pbPath pb.Path if err := proto.UnmarshalText(textPbPath, &pbPath); err != nil { t.Fatalf("error in unmarshaling path: %v %v", textPbPath, err) @@ -276,6 +276,9 @@ func runTestGet(t *testing.T, ctx context.Context, gClient pb.GNMIClient, pathTa t.Fatalf("error in unmarshaling IETF JSON data to json container: %v", err) } var wantJSONStruct interface{} + if v, ok := wantRespVal.(string); ok { + wantRespVal = []byte(v) + } if err := json.Unmarshal(wantRespVal.([]byte), &wantJSONStruct); err != nil { t.Fatalf("error in unmarshaling IETF JSON data to json container: %v", err) } @@ -302,10 +305,12 @@ type op_t int const ( Delete op_t = 1 Replace op_t = 2 + Update op_t = 3 ) func runTestSet(t *testing.T, ctx context.Context, gClient pb.GNMIClient, pathTarget string, textPbPath string, wantRetCode codes.Code, wantRespVal interface{}, attributeData string, op op_t) { + t.Helper() // Send request var pbPath pb.Path if err := proto.UnmarshalText(textPbPath, &pbPath); err != nil { @@ -313,21 +318,34 @@ func runTestSet(t *testing.T, ctx context.Context, gClient pb.GNMIClient, pathTa } req := &pb.SetRequest{} switch op { - case Replace: + case Replace, Update: prefix := pb.Path{Target: pathTarget} var v *pb.TypedValue v = &pb.TypedValue{ Value: &pb.TypedValue_JsonIetfVal{JsonIetfVal: extractJSON(attributeData)}} + data := []*pb.Update{{Path: &pbPath, Val: v}} req = &pb.SetRequest{ Prefix: &prefix, - Replace: []*pb.Update{&pb.Update{Path: &pbPath, Val: v}}, + } + if op == Replace { + req.Replace = data + } else { + req.Update = data } case Delete: req = &pb.SetRequest{ Delete: []*pb.Path{&pbPath}, } } + + runTestSetRaw(t, ctx, gClient, req, wantRetCode) +} + +func runTestSetRaw(t *testing.T, ctx context.Context, gClient pb.GNMIClient, req *pb.SetRequest, + wantRetCode codes.Code) { + t.Helper() + _, err := gClient.Set(ctx, req) gotRetStatus, ok := status.FromError(err) if !ok { @@ -340,6 +358,26 @@ func runTestSet(t *testing.T, ctx context.Context, gClient pb.GNMIClient, pathTa } } +// pathToPb converts string representation of gnmi path to protobuf format +func pathToPb(s string) string { + p, _ := ygot.StringToStructuredPath(s) + return proto.MarshalTextString(p) +} + +func removeModulePrefixFromPathPb(t *testing.T, s string) string { + t.Helper() + var p pb.Path + if err := proto.UnmarshalText(s, &p); err != nil { + t.Fatalf("error unmarshaling path: %v %v", s, err) + } + for _, ele := range p.Elem { + if k := strings.IndexByte(ele.Name, ':'); k != -1 { + ele.Name = ele.Name[k+1:] + } + } + return proto.MarshalTextString(&p) +} + func runServer(t *testing.T, s *Server) { //t.Log("Starting RPC server on address:", s.Address()) err := s.Serve() // blocks until close @@ -753,8 +791,6 @@ func TestGnmiSet(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - var emptyRespVal interface{} - tds := []struct { desc string pathTarget string @@ -765,29 +801,28 @@ func TestGnmiSet(t *testing.T) { operation op_t valTest bool }{ + { + desc: "Invalid path", + pathTarget: "OC_YANG", + textPbPath: pathToPb("/openconfig-interfaces:interfaces/interface[name=Ethernet4]/unknown"), + wantRetCode: codes.Unknown, + operation: Delete, + }, { desc: "Set OC Interface MTU", pathTarget: "OC_YANG", - textPbPath: ` - elem: elem: > - `, + textPbPath: pathToPb("openconfig-interfaces:interfaces/interface[name=Ethernet4]/config"), attributeData: "../testdata/set_interface_mtu.json", wantRetCode: codes.OK, - wantRespVal: emptyRespVal, - operation: Replace, - valTest: false, + operation: Update, }, { desc: "Set OC Interface IP", pathTarget: "OC_YANG", - textPbPath: ` - elem: elem: > elem: elem: > - `, + textPbPath: pathToPb("/openconfig-interfaces:interfaces/interface[name=Ethernet4]/subinterfaces/subinterface[index=0]/openconfig-if-ip:ipv4"), attributeData: "../testdata/set_interface_ipv4.json", wantRetCode: codes.OK, - wantRespVal: emptyRespVal, - operation: Replace, - valTest: false, + operation: Update, }, // { // desc: "Check OC Interface values set", @@ -807,19 +842,82 @@ func TestGnmiSet(t *testing.T) { `, attributeData: "", wantRetCode: codes.OK, - wantRespVal: emptyRespVal, operation: Delete, valTest: false, }, + { + desc: "Set OC Interface IPv6 (unprefixed path)", + pathTarget: "OC_YANG", + textPbPath: pathToPb("/interfaces/interface[name=Ethernet0]/subinterfaces/subinterface[index=0]/ipv6/addresses/address"), + attributeData: `{"address": [{"ip": "150::1","config": {"ip": "150::1","prefix-length": 80}}]}`, + wantRetCode: codes.OK, + operation: Update, + }, + { + desc: "Delete OC Interface IPv6 (unprefixed path)", + pathTarget: "OC_YANG", + textPbPath: pathToPb("/interfaces/interface[name=Ethernet0]/subinterfaces/subinterface[index=0]/ipv6/addresses/address[ip=150::1]"), + wantRetCode: codes.OK, + operation: Delete, + }, + { + desc: "Create ACL (unprefixed path)", + pathTarget: "OC_YANG", + textPbPath: pathToPb("/acl/acl-sets/acl-set"), + attributeData: `{"acl-set": [{"name": "A001", "type": "ACL_IPV4", + "config": {"name": "A001", "type": "ACL_IPV4", "description": "hello, world!"}}]}`, + wantRetCode: codes.OK, + operation: Update, + }, + { + desc: "Verify Create ACL", + pathTarget: "OC_YANG", + textPbPath: pathToPb("/openconfig-acl:acl/acl-sets/acl-set[name=A001][type=ACL_IPV4]/config/description"), + wantRespVal: `{"openconfig-acl:description": "hello, world!"}`, + wantRetCode: codes.OK, + valTest: true, + }, + { + desc: "Replace ACL Description (unprefixed path)", + pathTarget: "OC_YANG", + textPbPath: pathToPb("/acl/acl-sets/acl-set[name=A001][type=ACL_IPV4]/config/description"), + attributeData: `{"description": "dummy"}`, + wantRetCode: codes.OK, + operation: Replace, + }, + { + desc: "Verify Replace ACL Description", + pathTarget: "OC_YANG", + textPbPath: pathToPb("/openconfig-acl:acl/acl-sets/acl-set[name=A001][type=ACL_IPV4]/config/description"), + wantRespVal: `{"openconfig-acl:description": "dummy"}`, + wantRetCode: codes.OK, + valTest: true, + }, + { + desc: "Delete ACL", + pathTarget: "OC_YANG", + textPbPath: pathToPb("/openconfig-acl:acl/acl-sets/acl-set[name=A001][type=ACL_IPV4]"), + wantRetCode: codes.OK, + operation: Delete, + }, + { + desc: "Verify Delete ACL", + pathTarget: "OC_YANG", + textPbPath: pathToPb("/openconfig-acl:acl/acl-sets/acl-set[name=A001][type=ACL_IPV4]"), + wantRetCode: codes.NotFound, + valTest: true, + }, } for _, td := range tds { if td.valTest == true { - // wait for 2 seconds for change to sync - time.Sleep(2 * time.Second) t.Run(td.desc, func(t *testing.T) { runTestGet(t, ctx, gClient, td.pathTarget, td.textPbPath, td.wantRetCode, td.wantRespVal, td.valTest) }) + t.Run(td.desc + " (unprefixed path)", func(t *testing.T) { + p := removeModulePrefixFromPathPb(t, td.textPbPath) + runTestGet(t, ctx, gClient, td.pathTarget, p, td.wantRetCode, td.wantRespVal, td.valTest) + }) } else { t.Run(td.desc, func(t *testing.T) { runTestSet(t, ctx, gClient, td.pathTarget, td.textPbPath, td.wantRetCode, td.wantRespVal, td.attributeData, td.operation) @@ -2578,7 +2676,9 @@ func TestGNOI(t *testing.T) { t.Fatalf("Invalid System Time %d", resp.Time) } }) + t.Run("SonicShowTechsupport", func(t *testing.T) { + t.Skip("Not supported yet") sc := sgpb.NewSonicServiceClient(conn) rtime := time.Now().AddDate(0, -1, 0) req := &sgpb.TechsupportRequest{ @@ -2613,6 +2713,7 @@ func TestGNOI(t *testing.T) { for _, v := range cfg_data { t.Run("SonicCopyConfig", func(t *testing.T) { + t.Skip("Not supported yet") sc := sgpb.NewSonicServiceClient(conn) req := &sgpb.CopyConfigRequest{ Input: &sgpb.CopyConfigRequest_Input{ @@ -2698,7 +2799,7 @@ func TestBulkSet(t *testing.T) { go runServer(t, s) defer s.s.Stop() - // prepareDb(t) + prepareDbTranslib(t) //t.Log("Start gNMI client") tlsConfig := &tls.Config{InsecureSkipVerify: true} @@ -2715,32 +2816,81 @@ func TestBulkSet(t *testing.T) { gClient := pb.NewGNMIClient(conn) ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() + t.Run("Set Multiple mtu", func(t *testing.T) { - pbPath1, _ := xpath.ToGNMIPath("openconfig-interfaces:interfaces/interface[name=Ethernet0]/config/mtu") - v := &pb.TypedValue{ - Value: &pb.TypedValue_JsonIetfVal{JsonIetfVal: []byte("{\"mtu\": 9104}")}} - update1 := &pb.Update{ - Path: pbPath1, - Val: v, - } - pbPath2, _ := xpath.ToGNMIPath("openconfig-interfaces:interfaces/interface[name=Ethernet4]/config/mtu") - v2 := &pb.TypedValue{ - Value: &pb.TypedValue_JsonIetfVal{JsonIetfVal: []byte("{\"mtu\": 9105}")}} - update2 := &pb.Update{ - Path: pbPath2, - Val: v2, - } + req := &pb.SetRequest{ + Prefix: &pb.Path{Elem: []*pb.PathElem{{Name: "interfaces"}}}, + Update: []*pb.Update{ + newPbUpdate("interface[name=Ethernet0]/config/mtu", `{"mtu": 9104}`), + newPbUpdate("interface[name=Ethernet4]/config/mtu", `{"mtu": 9105}`), + }} + runTestSetRaw(t, ctx, gClient, req, codes.OK) + }) + + t.Run("Update and Replace", func(t *testing.T) { + aclKeys := `"name": "A002", "type": "ACL_IPV4"` + req := &pb.SetRequest{ + Replace: []*pb.Update{ + newPbUpdate( + "openconfig-acl:acl/acl-sets/acl-set", + `{"acl-set": [{`+aclKeys+`, "config":{`+aclKeys+`}}]}`), + }, + Update: []*pb.Update{ + newPbUpdate( + "interfaces/interface[name=Ethernet0]/config/description", + `{"description": "Bulk update 1"}`), + newPbUpdate( + "openconfig-interfaces:interfaces/interface[name=Ethernet4]/config/description", + `{"description": "Bulk update 2"}`), + }} + runTestSetRaw(t, ctx, gClient, req, codes.OK) + }) + + aclPath1, _ := ygot.StringToStructuredPath("/acl/acl-sets") + aclPath2, _ := ygot.StringToStructuredPath("/openconfig-acl:acl/acl-sets") + t.Run("Multiple deletes", func(t *testing.T) { req := &pb.SetRequest{ - Update: []*pb.Update{update1, update2}, + Delete: []*pb.Path{aclPath1, aclPath2}, } + runTestSetRaw(t, ctx, gClient, req, codes.OK) + }) - _, err = gClient.Set(ctx, req) - _, ok := status.FromError(err) - if !ok { - t.Fatal("got a non-grpc error from grpc call") + t.Run("Invalid Update Path", func(t *testing.T) { + req := &pb.SetRequest{ + Delete: []*pb.Path{aclPath1, aclPath2}, + Update: []*pb.Update{ + newPbUpdate("interface[name=Ethernet0]/config/mtu", `{"mtu": 9104}`), + }} + runTestSetRaw(t, ctx, gClient, req, codes.Unknown) + }) + + t.Run("Invalid Replace Path", func(t *testing.T) { + req := &pb.SetRequest{ + Delete: []*pb.Path{aclPath1, aclPath2}, + Replace: []*pb.Update{ + newPbUpdate("interface[name=Ethernet0]/config/mtu", `{"mtu": 9104}`), + }} + runTestSetRaw(t, ctx, gClient, req, codes.Unknown) + }) + + t.Run("Invalid Delete Path", func(t *testing.T) { + req := &pb.SetRequest{ + Prefix: &pb.Path{Elem: []*pb.PathElem{{Name: "interfaces"}}}, + Delete: []*pb.Path{aclPath1, aclPath2}, } + runTestSetRaw(t, ctx, gClient, req, codes.Unknown) }) + +} + +func newPbUpdate(path, value string) *pb.Update { + p, _ := ygot.StringToStructuredPath(path) + v := &pb.TypedValue_JsonIetfVal{JsonIetfVal: extractJSON(value)} + return &pb.Update{ + Path: p, + Val: &pb.TypedValue{Value: v}, + } } type loginCreds struct { diff --git a/tools/test/env.sh b/tools/test/env.sh index 46209f14d..a60b947e7 100755 --- a/tools/test/env.sh +++ b/tools/test/env.sh @@ -2,6 +2,8 @@ set -e -. $(dirname ${BASH_SOURCE})/../../../sonic-mgmt-common/tools/test/env.sh \ +TOPDIR=$(realpath $(dirname ${BASH_SOURCE})/../..) + +. ${TOPDIR}/../sonic-mgmt-common/tools/test/env.sh \ --dest=${TOPDIR}/build/test \ --dbconfig-in=${TOPDIR}/testdata/database_config.json \