Resolve "Requesting a specific config path of an OND returns always the whole config"

See merge request !266

controller/northbound/server/pnd.go

@@ -12,7 +12,9 @@ import (
 	"code.fbi.h-da.de/danet/gosdn/controller/nucleus"
 	"code.fbi.h-da.de/danet/gosdn/controller/nucleus/errors"
 	"github.com/google/uuid"
+	"github.com/openconfig/gnmi/proto/gnmi"
 	"github.com/openconfig/ygot/ygot"
+	"github.com/openconfig/ygot/ytypes"
 	"github.com/prometheus/client_golang/prometheus"
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"
@@ -139,6 +141,100 @@ func fillOnds(pnd networkdomain.NetworkDomain, all bool, did ...string) ([]*ppb.
 	return onds, nil
 }
 
+func fillOndBySpecificPath(pnd networkdomain.NetworkDomain, did string, path string) (*ppb.OrchestratedNetworkingDevice, error) {
+	d, err := pnd.GetDevice(did)
+	if err != nil {
+		log.Error(err)
+		return nil, status.Errorf(codes.Aborted, "%v", err)
+	}
+
+	gnmiPath, err := ygot.StringToPath(path, ygot.StructuredPath)
+	if err != nil {
+		log.Error(err)
+		return nil, status.Errorf(codes.Aborted, "%v", err)
+	}
+
+	opts := []ytypes.GetNodeOpt{&ytypes.GetHandleWildcards{}}
+	nodes, err := ytypes.GetNode(d.SBI().Schema().RootSchema(), d.Model(), gnmiPath, opts...)
+	if err != nil {
+		log.Error(err)
+		return nil, status.Errorf(codes.Aborted, "%v", err)
+	}
+
+	// we should be fine to access nodes[0] here, since ytypes.GetNode() throws
+	// an error if there are no matches for the path.
+	nodeEntry := nodes[0].Schema
+	node := nodes[0].Data
+
+	var dev []*gnmi.Notification
+	if nodeEntry.IsDir() {
+		validatedNode, ok := nodes[0].Data.(ygot.ValidatedGoStruct)
+		if !ok {
+			return nil, status.Errorf(codes.Aborted, "%v", errors.ErrInvalidTypeAssertion{
+				Value: node,
+				Type:  (ygot.ValidatedGoStruct)(nil),
+			})
+		}
+		cfg := ygot.GNMINotificationsConfig{}
+		dev, err = ygot.TogNMINotifications(validatedNode, time.Now().UnixNano(), cfg)
+		if err != nil {
+			log.Error(err)
+			return nil, status.Errorf(codes.Aborted, "%v", err)
+		}
+	} else if nodeEntry.IsLeaf() {
+		dev, err = genGnmiNotificationForLeaf(gnmiPath, node)
+		if err != nil {
+			log.Error(err)
+			return nil, status.Errorf(codes.Aborted, "%v", err)
+		}
+	}
+
+	sbi := spb.SouthboundInterface{}
+	if d.SBI() != nil {
+		sbi.Id = d.SBI().ID().String()
+		sbi.Type = d.SBI().Type()
+	}
+
+	ond := &ppb.OrchestratedNetworkingDevice{
+		Id:     d.ID().String(),
+		Name:   d.Name(),
+		Device: dev,
+		Sbi:    &sbi,
+	}
+
+	return ond, nil
+}
+
+func genGnmiNotificationForLeaf(path *gnmi.Path, val any) ([]*gnmi.Notification, error) {
+	typedVal, err := ygot.EncodeTypedValue(val, gnmi.Encoding_PROTO)
+	if err != nil {
+		return nil, err
+	}
+	pathElem := path.GetElem()[len(path.GetElem())-1].GetName()
+	ppath := []string{pathElem}
+	return []*gnmi.Notification{
+		{
+			Timestamp: time.Now().UnixNano(),
+			Update: []*gnmi.Update{
+				{
+					Path: &gnmi.Path{
+						// NOTE: This is dirty and should be changed as soon as
+						// ygot.TogNMINotifications does not return
+						// notifications with `Element`!
+						// `Element` is marked as deprecated, but since
+						// ygot.TogNMINotifications returns `Element`, we will
+						// keep using it for now and to maintain consistency.
+						// Only returning the last element here, since we
+						// request a path node.
+						Element: ppath,
+					},
+					Val: typedVal,
+				},
+			},
+		},
+	}, nil
+}
+
 func (p pndServer) GetSbi(ctx context.Context, request *ppb.GetSbiRequest) (*ppb.GetSbiResponse, error) {
 	labels := prometheus.Labels{"service": "pnd", "rpc": "get"}
 	start := metrics.StartHook(labels, grpcRequestsTotal)
@@ -276,11 +372,13 @@ func (p pndServer) GetPath(ctx context.Context, request *ppb.GetPathRequest) (*p
 		log.Error(err)
 		return nil, status.Errorf(codes.Aborted, "%v", err)
 	}
-	ond, err := fillOnds(pnd, false, request.Did)
+
+	ond, err := fillOndBySpecificPath(pnd, request.Did, path)
 	if err != nil {
 		log.Error(err)
 		return nil, status.Errorf(codes.Aborted, "%v", err)
 	}
+
 	return &ppb.GetPathResponse{
 		Timestamp: time.Now().UnixNano(),
 		Pnd: &ppb.PrincipalNetworkDomain{
@@ -288,7 +386,7 @@ func (p pndServer) GetPath(ctx context.Context, request *ppb.GetPathRequest) (*p
 			Name:        pnd.GetName(),
 			Description: pnd.GetDescription(),
 		},
-		Device: ond[0].Device,
+		Device: ond.Device,
 	}, nil
 }
 

controller/northbound/server/pnd_test.go

@@ -17,7 +17,10 @@ import (
 	"code.fbi.h-da.de/danet/gosdn/controller/nucleus"
 	"code.fbi.h-da.de/danet/gosdn/controller/store"
 	"code.fbi.h-da.de/danet/yang-models/generated/openconfig"
+	"github.com/golang/protobuf/proto"
+	"github.com/google/go-cmp/cmp"
 	"github.com/google/uuid"
+	"github.com/openconfig/gnmi/proto/gnmi"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/mock"
 	"google.golang.org/grpc"
@@ -33,6 +36,7 @@ const sbiID = "f6fd4b35-f039-4111-9156-5e4501bb8a5a"
 const ondID = "7e0ed8cc-ebf5-46fa-9794-741494914883"
 
 var hostname = "manfred"
+var domainname = "uwe"
 var pndUUID uuid.UUID
 var sbiUUID uuid.UUID
 var pendingChangeUUID uuid.UUID
@@ -122,7 +126,8 @@ func TestMain(m *testing.M) {
 		GoStruct: &openconfig.Device{
 			System: &openconfig.OpenconfigSystem_System{
 				Config: &openconfig.OpenconfigSystem_System_Config{
-					Hostname: &hostname,
+					Hostname:   &hostname,
+					DomainName: &domainname,
 				},
 			},
 		},
@@ -159,11 +164,16 @@ func TestMain(m *testing.M) {
 	mockPnd.On("Commit", mock.Anything).Return(nil)
 	mockPnd.On("Confirm", mock.Anything).Return(nil)
 	mockPnd.On("ChangeOND", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(uuid.Nil, nil)
+	mockPnd.On("Request", mock.Anything, mock.Anything).Return(nil, nil)
 
-	newPnd := getMockPND()
+	//newPnd := getMockPND()
 
 	pndc = store.NewPndStore()
-	if err := pndc.Add(newPnd); err != nil {
+	//if err := pndc.Add(newPnd); err != nil {
+	//	log.Fatal(err)
+	//}
+
+	if err := pndc.Add(mockPnd); err != nil {
 		log.Fatal(err)
 	}
 
@@ -224,6 +234,124 @@ func Test_pnd_Get(t *testing.T) {
 	}
 }
 
+func Test_pnd_GetPath(t *testing.T) {
+	removeExistingStores()
+	// options to apply to cmp.Equal
+	type args struct {
+		ctx     context.Context
+		request *ppb.GetPathRequest
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    []*gnmi.Notification
+		wantErr bool
+	}{
+		{
+			name: "get path: system/config/hostname",
+			args: args{
+				ctx: context.Background(),
+				request: &ppb.GetPathRequest{
+					Timestamp: 1648488947324854250,
+					Did:       mockDevice.ID().String(),
+					Path:      "system/config/hostname",
+					Pid:       mockPnd.ID().String(),
+				},
+			},
+			want: []*gnmi.Notification{
+				{
+					Update: []*gnmi.Update{
+						{
+							Path: &gnmi.Path{Element: []string{"hostname"}},
+							Val: &gnmi.TypedValue{
+								Value: &gnmi.TypedValue_StringVal{
+									StringVal: "manfred",
+								},
+							},
+						},
+					}},
+			},
+			wantErr: false,
+		},
+		{
+			name: "get path: system",
+			args: args{
+				ctx: context.Background(),
+				request: &ppb.GetPathRequest{
+					Timestamp: 1648488947324854250,
+					Did:       mockDevice.ID().String(),
+					Path:      "system",
+					Pid:       mockPnd.ID().String(),
+				},
+			},
+			want: []*gnmi.Notification{
+				{
+					Update: []*gnmi.Update{
+						{
+							Path: &gnmi.Path{Element: []string{"config", "domain-name"}},
+							Val: &gnmi.TypedValue{
+								Value: &gnmi.TypedValue_StringVal{
+									StringVal: "uwe",
+								},
+							},
+						},
+						{
+							Path: &gnmi.Path{Element: []string{"config", "hostname"}},
+							Val: &gnmi.TypedValue{
+								Value: &gnmi.TypedValue_StringVal{
+									StringVal: "manfred",
+								},
+							},
+						},
+					}},
+			},
+			wantErr: false,
+		},
+		{
+			name: "get path: this/path/is/not/valid",
+			args: args{
+				ctx: context.Background(),
+				request: &ppb.GetPathRequest{
+					Timestamp: 1648488947324854250,
+					Did:       mockDevice.ID().String(),
+					Path:      "this/path/is/not/valid",
+					Pid:       mockPnd.ID().String(),
+				},
+			},
+			want:    []*gnmi.Notification{},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s := pndServer{
+				UnimplementedPndServiceServer: ppb.UnimplementedPndServiceServer{},
+			}
+			resp, err := s.GetPath(tt.args.ctx, tt.args.request)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("GetPath() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			got := resp.GetDevice()
+
+			// with the current implementation we should only get a
+			// notification set with length one. For the case that we do not
+			// get anything this makes sure we do not trigger a panic.
+			if len(got) >= 1 {
+				// set timestamp for equality; not the prettiest solution
+				tt.want[0].Timestamp = got[0].Timestamp
+			}
+
+			for i, n := range got {
+				if diff := cmp.Diff(n, tt.want[i], cmp.Comparer(proto.Equal)); diff != "" {
+					t.Errorf("GetPath() diff in the received notification %d: \n%s", i+1, diff)
+				}
+			}
+		})
+	}
+}
+
 func Test_pnd_Set(t *testing.T) {
 	removeExistingStores()