Merge pull request #297 from alexandernorth/fix/logging-and-getconfig-convert

Fix/logging and getconfig convert

Author: steiler

pkg/datastore/target/nc.go

@@ -100,7 +100,7 @@ func (t *ncTarget) Get(ctx context.Context, req *sdcpb.GetDataRequest) (*sdcpb.G
 		return nil, err
 	}
 
-	log.Debugf("netconf response:\n%s", ncResponse.DocAsString())
+	log.Debugf("%s: netconf response:\n%s", t.name, ncResponse.DocAsString())
 
 	// cmlImport := xml.NewXmlTreeImporter(ncResponse.Doc.Root())
 
@@ -203,7 +203,7 @@ func (t *ncTarget) internalSync(ctx context.Context, sc *config.SyncProtocol, fo
 	// execute netconf get
 	resp, err := t.Get(ctx, req)
 	if err != nil {
-		log.Errorf("failed getting config: %T | %v", err, err)
+		log.Errorf("failed getting config from target %v: %T | %v", t.name, err, err)
 		if strings.Contains(err.Error(), "EOF") {
 			t.Close()
 			go t.reconnect()

pkg/datastore/target/nc_test.go

@@ -91,6 +91,9 @@ func Test_ncTarget_Get(t *testing.T) {
 												Keys: []*sdcpb.LeafSchema{
 													{
 														Name: "name",
+														Type: &sdcpb.SchemaLeafType{
+															Type: "string",
+														},
 													},
 												},
 											},
@@ -110,6 +113,9 @@ func Test_ncTarget_Get(t *testing.T) {
 												Keys: []*sdcpb.LeafSchema{
 													{
 														Name: "name",
+														Type: &sdcpb.SchemaLeafType{
+															Type: "string",
+														},
 													},
 												},
 											},

pkg/datastore/target/netconf/xml2SchemapbAdapter.go

@@ -123,7 +123,12 @@ func (x *XML2sdcpbConfigAdapter) transformContainer(ctx context.Context, e *etre
 		if cPElem[len(cPElem)-1].Key == nil {
 			cPElem[len(cPElem)-1].Key = map[string]string{}
 		}
-		cPElem[len(cPElem)-1].Key[ls.Name] = e.FindElement("./" + ls.Name).Text()
+		tv, err := utils.Convert(e.FindElement("./"+ls.Name).Text(), ls.Type)
+		if err != nil {
+			return err
+		}
+
+		cPElem[len(cPElem)-1].Key[ls.Name] = tv.ToString()
 	}
 
 	ntc := NewTransformationContext(cPElem)
@@ -145,7 +150,8 @@ func (x *XML2sdcpbConfigAdapter) transformContainer(ctx context.Context, e *etre
 // transformField transforms an etree.element of a configuration as an update into the provided *sdcpb.Notification.
 func (x *XML2sdcpbConfigAdapter) transformField(ctx context.Context, e *etree.Element, pelems []*sdcpb.PathElem, ls *sdcpb.LeafSchema, result *sdcpb.Notification) error {
 	path := pelems
-	for ls.GetType().GetLeafref() != "" {
+	schemaLeafType := ls.GetType()
+	for schemaLeafType.GetLeafref() != "" {
 		path, err := utils.NormalizedAbsPath(ls.Type.Leafref, path)
 		if err != nil {
 			return err
@@ -156,18 +162,20 @@ func (x *XML2sdcpbConfigAdapter) transformField(ctx context.Context, e *etree.El
 			return err
 		}
 
-		var schemaElem *sdcpb.SchemaElem_Field
-		var ok bool
-		if schemaElem, ok = schema.GetSchema().GetSchema().(*sdcpb.SchemaElem_Field); !ok {
-			return fmt.Errorf("leafref resolved to non-field schema type")
+		switch se := schema.GetSchema().GetSchema().(type) {
+		case *sdcpb.SchemaElem_Leaflist:
+			schemaLeafType = se.Leaflist.GetType()
+		case *sdcpb.SchemaElem_Field:
+			schemaLeafType = se.Field.GetType()
+		default:
+			return fmt.Errorf("node [%s] with leafref [%s] has non-field or leaflist target type [%T]", e.GetPath(), ls.GetType().GetLeafref(), se)
 		}
-		ls = schemaElem.Field
 	}
 
 	// process terminal values
-	tv, err := StringElementToTypedValue(e.Text(), ls)
+	tv, err := utils.Convert(e.Text(), schemaLeafType)
 	if err != nil {
-		return err
+		return fmt.Errorf("unable to convert value [%s] at path [%s] according to SchemaLeafType [%+v]: %w", e.Text(), e.GetPath(), schemaLeafType, err)
 	}
 	// copy pathElems
 	npelem := make([]*sdcpb.PathElem, 0, len(pelems))

pkg/datastore/target/netconf/xml2SchemapbAdapter_test.go

@@ -90,6 +90,9 @@ func TestXML2sdcpbConfigAdapter_Transform(t *testing.T) {
 									Keys: []*sdcpb.LeafSchema{
 										{
 											Name: "name",
+											Type: &sdcpb.SchemaLeafType{
+												Type: "string",
+											},
 										},
 									},
 								},

pkg/server/server.go

@@ -275,6 +275,7 @@ func (s *Server) createInitialDatastores(ctx context.Context) {
 
 	for _, dsCfg := range s.config.Datastores {
 		log.Debugf("creating datastore %s", dsCfg.Name)
+		dsCfg.Validation = s.config.Validation.DeepCopy()
 		go func(dsCfg *config.DatastoreConfig) {
 			defer wg.Done()
 			// TODO: handle error

pkg/tree/leaf_variants.go

@@ -335,7 +335,7 @@ func (lv *LeafVariants) GetDeviations(ch chan<- *types.DeviationEntry, isActiveC
 	}
 
 	// get the path via the first LeafEntry
-	// is valida for all entries
+	// is valid for all entries
 	sdcpbPath, err := lv.parentEntry.SdcpbPath()
 	if err != nil {
 		log.Error(err)
@@ -375,7 +375,7 @@ func (lv *LeafVariants) GetDeviations(ch chan<- *types.DeviationEntry, isActiveC
 			overruled = append(overruled, de)
 			highest = le
 		}
-		// if precedence of actual (le) is lower then le needs to be adeded to overruled
+		// if precedence of actual (le) is lower then le needs to be added to overruled
 		if le.Priority() >= highest.Priority() {
 			de := types.NewDeviationEntry(le.Owner(), types.DeviationReasonOverruled, sdcpbPath).SetExpectedValue(le.Value())
 			overruled = append(overruled, de)
@@ -398,7 +398,7 @@ func (lv *LeafVariants) GetDeviations(ch chan<- *types.DeviationEntry, isActiveC
 		return
 	}
 
-	// if higeste exists but not running  OR   running != highest
+	// if highest exists but not running  OR   running != highest
 	if (running == nil && highest != nil) || running.Value().Cmp(highest.Value()) != 0 {
 		de := types.NewDeviationEntry(highest.Owner(), types.DeviationReasonNotApplied, sdcpbPath).SetExpectedValue(highest.Value())
 		if running != nil {

pkg/tree/sharedEntryAttributes.go

@@ -212,7 +212,9 @@ func (s *sharedEntryAttributes) checkAndCreateKeysAsLeafs(ctx context.Context, i
 	// if we're in the last level of keys, then we need to add the defaults
 	if len(ancestorContainerSchema.Keys) == levelsUp {
 		// iterate through the keys
-		for idx, k := range ancestor.GetSchemaKeys() {
+		schemaKeys := ancestor.GetSchemaKeys()
+		slices.Sort(schemaKeys)
+		for idx, k := range schemaKeys {
 			child, entryExists := s.childs.GetEntry(k)
 			// if the key Leaf exists continue with next key
 			if entryExists {
@@ -951,6 +953,9 @@ func (s *sharedEntryAttributes) Validate(ctx context.Context, resultChan chan<-
 
 	// validate the mandatory statement on this entry
 	if s.remainsToExist() {
+
+		// TODO: Validate Enums
+
 		if !vCfg.DisabledValidators.Mandatory {
 			s.validateMandatory(ctx, resultChan)
 		}
@@ -1137,13 +1142,13 @@ func (s *sharedEntryAttributes) ImportConfig(ctx context.Context, t importer.Imp
 			var exists bool
 			var actualEntry Entry = s
 			var keyChild Entry
-			for _, keySchema := range s.schema.GetContainer().GetKeys() {
+			schemaKeys := s.GetSchemaKeys()
+			slices.Sort(schemaKeys)
+			for _, schemaKey := range schemaKeys {
 
-				keyElemName := keySchema.Name
-
-				keyTransf := t.GetElement(keyElemName)
+				keyTransf := t.GetElement(schemaKey)
 				if keyTransf == nil {
-					return fmt.Errorf("unable to find key attribute %s under %s", keyElemName, s.Path())
+					return fmt.Errorf("unable to find key attribute %s under %s", schemaKey, s.Path())
 				}
 				keyElemValue, err := keyTransf.GetKeyValue()
 				if err != nil {
@@ -1654,18 +1659,18 @@ func (s *sharedEntryAttributes) getKeyName() (string, error) {
 		return "", fmt.Errorf("error %s is a schema element, can only get KeyNames for key element", strings.Join(s.Path(), " "))
 	}
 
-	// get ancestro schema
+	// get ancestor schema
 	ancestorWithSchema, levelUp := s.GetFirstAncestorWithSchema()
 
-	// only Contaieners have keys, so check for that
-	switch sch := ancestorWithSchema.GetSchema().GetSchema().(type) {
-	case *sdcpb.SchemaElem_Container:
-		// return the name of the levelUp-1 key
-		return sch.Container.GetKeys()[levelUp-1].Name, nil
+	// only Containers have keys, so check for that
+	schemaKeys := ancestorWithSchema.GetSchemaKeys()
+	if len(schemaKeys) == 0 {
+		// we probably called the function on a LeafList or LeafEntry which is not a valid call to be made.
+		return "", fmt.Errorf("error LeafList and Field should not have keys %s", strings.Join(s.Path(), " "))
 	}
 
-	// we probably called the function on a LeafList or LeafEntry which is not a valid call to be made.
-	return "", fmt.Errorf("error LeafList and Field should not have keys %s", strings.Join(s.Path(), " "))
+	slices.Sort(schemaKeys)
+	return schemaKeys[levelUp-1], nil
 }
 
 func (s *sharedEntryAttributes) getOrCreateChilds(ctx context.Context, path types.PathSlice) (Entry, error) {

pkg/tree/validation_entry_leafref.go

@@ -109,6 +109,18 @@ func (s *sharedEntryAttributes) BreadthSearch(ctx context.Context, path string)
 // NavigateLeafRef
 func (s *sharedEntryAttributes) NavigateLeafRef(ctx context.Context) ([]Entry, error) {
 
+	// leafref path takes as an argument a string that MUST refer to a leaf or leaf-list node.
+	// e.g.
+	// leaf foo {
+	//   type leafref {
+	//     path "/bar/baz";
+	//   }
+	// }
+	// the runtime semantics are:
+	// If /bar/baz resolves to a leaf, then foo must have exactly the same value as that leaf.
+	// If /bar/baz resolves to a leaf-list, then foo must have a value equal to one of the existing entries in that leaf-list.
+	// Thus, the "pointer" is not to the entire leaf-list node, but to one instance of it, selected by value.
+
 	var lref string
 	switch {
 	case s.GetSchema().GetField().GetType().GetLeafref() != "":
@@ -121,34 +133,34 @@ func (s *sharedEntryAttributes) NavigateLeafRef(ctx context.Context) ([]Entry, e
 
 	lv := s.leafVariants.GetHighestPrecedence(false, true)
 
+	// value of node with type leafref
 	tv := lv.Value()
-	var values []*sdcpb.TypedValue
-
-	switch ttv := tv.Value.(type) {
-	case *sdcpb.TypedValue_LeaflistVal:
-		values = append(values, ttv.LeaflistVal.GetElement()...)
-	default:
-		values = append(values, tv)
-	}
-
-	var resultEntries []Entry
 
 	foundEntries, err := s.BreadthSearch(ctx, lref)
 	if err != nil {
 		return nil, err
 	}
 
-	resultEntries = []Entry{}
+	var resultEntries []Entry
 
 	for _, e := range foundEntries {
-
 		r, err := e.getHighestPrecedenceLeafValue(ctx)
 		if err != nil {
 			return nil, err
 		}
-		val := r.Value()
-		for _, value := range values {
-			if utils.EqualTypedValues(val, value) {
+		// Value of the resolved leafref
+		refVal := r.Value()
+		var vals []*sdcpb.TypedValue
+
+		switch tVal := refVal.Value.(type) {
+		case *sdcpb.TypedValue_LeaflistVal:
+			vals = append(vals, tVal.LeaflistVal.GetElement()...)
+		default:
+			vals = append(vals, refVal)
+		}
+		// loop through possible values of found reference (leaf -> 1 value, leaf-list -> 1+ values)
+		for _, val := range vals {
+			if utils.EqualTypedValues(val, tv) {
 				resultEntries = append(resultEntries, e)
 				break
 			}

pkg/utils/converter.go

@@ -140,7 +140,8 @@ func (c *Converter) ExpandUpdate(ctx context.Context, upd *sdcpb.Update) ([]*sdc
 			}
 		}
 
-		if rsp.Field.GetType().Type == "identityref" {
+		// We expect that all identityrefs are sent by schema-server as a identityref type now, not string
+		if rsp.Field.GetType().Type == "identityref" && upd.GetValue().GetStringVal() != "" {
 			upd.Value, err = Convert(upd.GetValue().GetStringVal(), rsp.Field.GetType())
 			if err != nil {
 				return nil, err
@@ -421,6 +422,8 @@ func TypedValueToYANGType(tv *sdcpb.TypedValue, schemaObject *sdcpb.SchemaElem)
 		return tv, nil
 	case *sdcpb.TypedValue_AnyVal:
 		return tv, nil
+	case *sdcpb.TypedValue_IdentityrefVal:
+		return ConvertToTypedValue(schemaObject, tv.GetStringVal(), tv.GetTimestamp())
 	}
 	return tv, nil
 }

pkg/utils/leaf_convert.go

@@ -27,6 +27,9 @@ import (
 )
 
 func Convert(value string, lst *sdcpb.SchemaLeafType) (*sdcpb.TypedValue, error) {
+	if lst == nil {
+		return nil, fmt.Errorf("schemaleaftype cannot be nil")
+	}
 	switch lst.Type {
 	case "string":
 		return ConvertString(value, lst)
@@ -140,16 +143,26 @@ func ConvertSdcpbNumberToUint64(mm *sdcpb.Number) (uint64, error) {
 	return mm.Value, nil
 }
 
-func ConvertSdcpbNumberToInt64(mm *sdcpb.Number) (int64, error) {
-	if mm.Value > math.MaxInt64 {
-		return 0, fmt.Errorf("error converting %d to int64 overflow", mm.Value)
+func intAbs(x int64) uint64 {
+	ui := uint64(x)
+	if x < 0 {
+		return ^(ui) + 1
 	}
+	return ui
+}
 
+func ConvertSdcpbNumberToInt64(mm *sdcpb.Number) (int64, error) {
 	if mm.Negative {
+		if mm.Value > intAbs(math.MinInt64) {
+			return 0, fmt.Errorf("error converting -%d to int64: overflow", mm.Value)
+		}
 		return -int64(mm.Value), nil
-	} else {
-		return int64(mm.Value), nil
 	}
+
+	if mm.Value > math.MaxInt64 {
+		return 0, fmt.Errorf("error converting %d to int64 overflow", mm.Value)
+	}
+	return int64(mm.Value), nil
 }
 
 func convertUint(value string, minMaxs []*sdcpb.SchemaMinMaxType, ranges *URnges) (*sdcpb.TypedValue, error) {