Repository: openconfig/gnmic (language: go)

Issue:
using target name with http target loader 

Hi, I am using dynamic HTTP target loader and run into issue where gnmic continuously adds the same device.

For example, my http url can return this data to gnmic:

```
{
  "pan1:50051": {
    "name": "bob",
    "username": "admin",
    "password": "asdf",
    "subscriptions": [
      "pan-interface"
    ]
  }
}
```

in the gnmic logs, i continuously see this message every watch interval:

```
gnmic1        | 2023/06/03 03:23:00.146635 [gnmic] target "bob" deleted from config
gnmic1        | 2023/06/03 03:23:00.146779 [gnmic] target "bob": listener stopped
gnmic1        | 2023/06/03 03:23:00.146805 [gnmic] adding target {"name":"bob","address":"pan1:50051","username":"admin","password":"****","timeout":10000000000,"insecure":true,"skip-verify":false,"subscriptions":["pan-interface"],"buffer-size":100,"retry-timer":10000000000,"log-tls-secret":false,"gzip":false,"token":""}
gnmic1        | 2023/06/03 03:23:00.146851 [gnmic] queuing target "bob"
gnmic1        | 2023/06/03 03:23:00.146869 [gnmic] subscribing to target: "bob"

```

On the target device, I can see that the number of persistently established tcp connections from gnmic continuously increases.  If i remove the "name" key/value pair from the above json, then the problem goes away.

I can supply simple docker-compose to reproduce if needed.

loader config:
```
loader:
  type: http
  url: "http://10.196.21.67:9999/devices"
  interval: 30s
  start-delay: 0s
  enable-metrics: true
```

thank you for continuous development!



Discussion / hints:
Good catch, I see where the issue is, I will try to fix it ASAP. 

Fix patch:
diff --git a/loaders/consul_loader/consul_loader.go b/loaders/consul_loader/consul_loader.go
index fbd9a513..e3ecfa86 100644
--- a/loaders/consul_loader/consul_loader.go
+++ b/loaders/consul_loader/consul_loader.go
@@ -485,7 +485,7 @@ func (c *consulLoader) runActions(ctx context.Context, tcs map[string]*types.Tar
 	opChan := make(chan *loaders.TargetOperation)
 	doneCh := make(chan struct{})
 	result := &loaders.TargetOperation{
-		Add: make([]*types.TargetConfig, 0, len(targetOp.Add)),
+		Add: make(map[string]*types.TargetConfig, len(targetOp.Add)),
 		Del: make([]string, 0, len(targetOp.Del)),
 	}
 	ctx, cancel := context.WithTimeout(ctx, c.cfg.ActionsTimeout)
@@ -501,7 +501,9 @@ func (c *consulLoader) runActions(ctx context.Context, tcs map[string]*types.Tar
 					close(doneCh)
 					return
 				}
-				result.Add = append(result.Add, op.Add...)
+				for n, t := range op.Add {
+					result.Add[n] = t
+				}
 				result.Del = append(result.Del, op.Del...)
 			}
 		}
@@ -510,16 +512,16 @@ func (c *consulLoader) runActions(ctx context.Context, tcs map[string]*types.Tar
 	wg := new(sync.WaitGroup)
 	wg.Add(len(targetOp.Add) + len(targetOp.Del))
 	// run OnAdd actions
-	for _, tAdd := range targetOp.Add {
-		go func(tc *types.TargetConfig) {
+	for n, tAdd := range targetOp.Add {
+		go func(n string, tc *types.TargetConfig) {
 			defer wg.Done()
 			err := c.runOnAddActions(ctx, tc.Name, tcs)
 			if err != nil {
 				c.logger.Printf("failed running OnAdd actions: %v", err)
 				return
 			}
-			opChan <- &loaders.TargetOperation{Add: []*types.TargetConfig{tc}}
-		}(tAdd)
+			opChan <- &loaders.TargetOperation{Add: map[string]*types.TargetConfig{n:tc}}
+		}(n, tAdd)
 	}
 	// run OnDelete actions
 	for _, tDel := range targetOp.Del {
diff --git a/loaders/docker_loader/docker_loader.go b/loaders/docker_loader/docker_loader.go
index 486b15e8..0aed2d0e 100644
--- a/loaders/docker_loader/docker_loader.go
+++ b/loaders/docker_loader/docker_loader.go
@@ -574,7 +574,7 @@ func (d *dockerLoader) runActions(ctx context.Context, tcs map[string]*types.Tar
 	// some actions are defined,
 	doneCh := make(chan struct{})
 	result := &loaders.TargetOperation{
-		Add: make([]*types.TargetConfig, 0, len(targetOp.Add)),
+		Add: make(map[string]*types.TargetConfig, len(targetOp.Add)),
 		Del: make([]string, 0, len(targetOp.Del)),
 	}
 	ctx, cancel := context.WithTimeout(ctx, d.cfg.Interval)
@@ -591,7 +591,9 @@ func (d *dockerLoader) runActions(ctx context.Context, tcs map[string]*types.Tar
 					close(doneCh)
 					return
 				}
-				result.Add = append(result.Add, op.Add...)
+				for n, t := range op.Add {
+					result.Add[n] = t
+				}
 				result.Del = append(result.Del, op.Del...)
 			}
 		}
@@ -600,16 +602,16 @@ func (d *dockerLoader) runActions(ctx context.Context, tcs map[string]*types.Tar
 	wg := new(sync.WaitGroup)
 	wg.Add(len(targetOp.Add) + len(targetOp.Del))
 	// run OnAdd actions
-	for _, tAdd := range targetOp.Add {
-		go func(tc *types.TargetConfig) {
+	for n, tAdd := range targetOp.Add {
+		go func(n string, tc *types.TargetConfig) {
 			defer wg.Done()
 			err := d.runOnAddActions(ctx, tc.Name, tcs)
 			if err != nil {
 				d.logger.Printf("failed running OnAdd actions: %v", err)
 				return
 			}
-			opChan <- &loaders.TargetOperation{Add: []*types.TargetConfig{tc}}
-		}(tAdd)
+			opChan <- &loaders.TargetOperation{Add: map[string]*types.TargetConfig{n: tc}}
+		}(n, tAdd)
 	}
 	// run OnDelete actions
 	for _, tDel := range targetOp.Del {
diff --git a/loaders/file_loader/file_loader.go b/loaders/file_loader/file_loader.go
index e2ac26ed..f558be10 100644
--- a/loaders/file_loader/file_loader.go
+++ b/loaders/file_loader/file_loader.go
@@ -348,7 +348,7 @@ func (f *fileLoader) runActions(ctx context.Context, tcs map[string]*types.Targe
 	// some actions are defined,
 	doneCh := make(chan struct{})
 	result := &loaders.TargetOperation{
-		Add: make([]*types.TargetConfig, 0, len(targetOp.Add)),
+		Add: make(map[string]*types.TargetConfig, len(targetOp.Add)),
 		Del: make([]string, 0, len(targetOp.Del)),
 	}
 	ctx, cancel := context.WithTimeout(ctx, f.cfg.Interval)
@@ -365,7 +365,9 @@ func (f *fileLoader) runActions(ctx context.Context, tcs map[string]*types.Targe
 					close(doneCh)
 					return
 				}
-				result.Add = append(result.Add, op.Add...)
+				for n, t := range op.Add {
+					result.Add[n] = t
+				}
 				result.Del = append(result.Del, op.Del...)
 			}
 		}
@@ -374,16 +376,16 @@ func (f *fileLoader) runActions(ctx context.Context, tcs map[string]*types.Targe
 	wg := new(sync.WaitGroup)
 	wg.Add(len(targetOp.Add) + len(targetOp.Del))
 	// run OnAdd actions
-	for _, tAdd := range targetOp.Add {
-		go func(tc *types.TargetConfig) {
+	for n, tAdd := range targetOp.Add {
+		go func(n string, tc *types.TargetConfig) {
 			defer wg.Done()
 			err := f.runOnAddActions(ctx, tc.Name, tcs)
 			if err != nil {
 				f.logger.Printf("failed running OnAdd actions: %v", err)
 				return
 			}
-			opChan <- &loaders.TargetOperation{Add: []*types.TargetConfig{tc}}
-		}(tAdd)
+			opChan <- &loaders.TargetOperation{Add: map[string]*types.TargetConfig{n: tc}}
+		}(n, tAdd)
 	}
 	// run OnDelete actions
 	for _, tDel := range targetOp.Del {
diff --git a/loaders/http_loader/http_loader.go b/loaders/http_loader/http_loader.go
index 2af5e409..514d2e31 100644
--- a/loaders/http_loader/http_loader.go
+++ b/loaders/http_loader/http_loader.go
@@ -325,9 +325,9 @@ func (h *httpLoader) updateTargets(ctx context.Context, tcs map[string]*types.Ta
 		return
 	}
 	h.m.Lock()
-	for _, t := range targetOp.Add {
-		if _, ok := h.lastTargets[t.Name]; !ok {
-			h.lastTargets[t.Name] = t
+	for n, t := range targetOp.Add {
+		if _, ok := h.lastTargets[n]; !ok {
+			h.lastTargets[n] = t
 		}
 	}
 	for _, n := range targetOp.Del {
@@ -389,7 +389,7 @@ func (f *httpLoader) runActions(ctx context.Context, tcs map[string]*types.Targe
 	// some actions are defined,
 	doneCh := make(chan struct{})
 	result := &loaders.TargetOperation{
-		Add: make([]*types.TargetConfig, 0, len(targetOp.Add)),
+		Add: make(map[string]*types.TargetConfig, len(targetOp.Add)),
 		Del: make([]string, 0, len(targetOp.Del)),
 	}
 	ctx, cancel := context.WithTimeout(ctx, f.cfg.Interval)
@@ -405,7 +405,9 @@ func (f *httpLoader) runActions(ctx context.Context, tcs map[string]*types.Targe
 					close(doneCh)
 					return
 				}
-				result.Add = append(result.Add, op.Add...)
+				for n, t := range op.Add {
+					result.Add[n] = t
+				}
 				result.Del = append(result.Del, op.Del...)
 			}
 		}
@@ -414,16 +416,16 @@ func (f *httpLoader) runActions(ctx context.Context, tcs map[string]*types.Targe
 	wg := new(sync.WaitGroup)
 	wg.Add(len(targetOp.Add) + len(targetOp.Del))
 	// run OnAdd actions
-	for _, tAdd := range targetOp.Add {
-		go func(tc *types.TargetConfig) {
+	for n, tAdd := range targetOp.Add {
+		go func(n string, tc *types.TargetConfig) {
 			defer wg.Done()
 			err := f.runOnAddActions(ctx, tc.Name, tcs)
 			if err != nil {
 				f.logger.Printf("failed running OnAdd actions: %v", err)
 				return
 			}
-			opChan <- &loaders.TargetOperation{Add: []*types.TargetConfig{tc}}
-		}(tAdd)
+			opChan <- &loaders.TargetOperation{Add: map[string]*types.TargetConfig{n: tc}}
+		}(n, tAdd)
 	}
 	// run OnDelete actions
 	for _, tDel := range targetOp.Del {
diff --git a/loaders/loaders.go b/loaders/loaders.go
index ae3acadb..1553c677 100644
--- a/loaders/loaders.go
+++ b/loaders/loaders.go
@@ -54,7 +54,7 @@ func Register(name string, initFn Initializer) {
 }
 
 type TargetOperation struct {
-	Add []*types.TargetConfig
+	Add map[string]*types.TargetConfig
 	Del []string
 }
 
@@ -73,12 +73,12 @@ func DecodeConfig(src, dst interface{}) error {
 
 func Diff(m1, m2 map[string]*types.TargetConfig) *TargetOperation {
 	result := &TargetOperation{
-		Add: make([]*types.TargetConfig, 0),
+		Add: make(map[string]*types.TargetConfig, 0),
 		Del: make([]string, 0),
 	}
 	if len(m1) == 0 {
-		for _, t := range m2 {
-			result.Add = append(result.Add, t)
+		for n, t := range m2 {
+			result.Add[n] = t
 		}
 		return result
 	}
@@ -90,7 +90,7 @@ func Diff(m1, m2 map[string]*types.TargetConfig) *TargetOperation {
 	}
 	for n, t := range m2 {
 		if _, ok := m1[n]; !ok {
-			result.Add = append(result.Add, t)
+			result.Add[n] = t
 		}
 	}
 	for n := range m1 {


Test patch:
diff --git a/loaders/loaders_test.go b/loaders/loaders_test.go
index 595996df..9dbcee29 100644
--- a/loaders/loaders_test.go
+++ b/loaders/loaders_test.go
@@ -9,7 +9,6 @@
 package loaders
 
 import (
-	"sort"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -24,7 +23,7 @@ var testSet = map[string]struct {
 		m1: nil,
 		m2: nil,
 		output: &TargetOperation{
-			Add: make([]*types.TargetConfig, 0),
+			Add: make(map[string]*types.TargetConfig, 0),
 			Del: make([]string, 0),
 		},
 	},
@@ -34,8 +33,8 @@ var testSet = map[string]struct {
 			"target1": {Name: "target1"},
 		},
 		output: &TargetOperation{
-			Add: []*types.TargetConfig{
-				{
+			Add: map[string]*types.TargetConfig{
+				"target1": {
 					Name: "target1",
 				},
 			},
@@ -50,7 +49,7 @@ var testSet = map[string]struct {
 			"target1": {Name: "target1"},
 		},
 		output: &TargetOperation{
-			Add: make([]*types.TargetConfig, 0),
+			Add: make(map[string]*types.TargetConfig, 0),
 			Del: make([]string, 0),
 		},
 	},
@@ -64,7 +63,7 @@ var testSet = map[string]struct {
 			"target2": {Name: "target2"},
 		},
 		output: &TargetOperation{
-			Add: make([]*types.TargetConfig, 0),
+			Add: make(map[string]*types.TargetConfig, 0),
 			Del: make([]string, 0),
 		},
 	},
@@ -74,7 +73,7 @@ var testSet = map[string]struct {
 		},
 		m2: nil,
 		output: &TargetOperation{
-			Add: make([]*types.TargetConfig, 0),
+			Add: make(map[string]*types.TargetConfig, 0),
 			Del: []string{"target1"},
 		},
 	},
@@ -87,8 +86,8 @@ var testSet = map[string]struct {
 			"target2": {Name: "target2"},
 		},
 		output: &TargetOperation{
-			Add: []*types.TargetConfig{
-				{
+			Add: map[string]*types.TargetConfig{
+				"target2": {
 					Name: "target2",
 				},
 			},
@@ -103,8 +102,8 @@ var testSet = map[string]struct {
 			"target2": {Name: "target2"},
 		},
 		output: &TargetOperation{
-			Add: []*types.TargetConfig{
-				{
+			Add: map[string]*types.TargetConfig{
+				"target2": {
 					Name: "target2",
 				},
 			},
@@ -120,11 +119,11 @@ var testSet = map[string]struct {
 			"target3": {Name: "target3"},
 		},
 		output: &TargetOperation{
-			Add: []*types.TargetConfig{
-				{
+			Add: map[string]*types.TargetConfig{
+				"target2": {
 					Name: "target2",
 				},
-				{
+				"target3": {
 					Name: "target3",
 				},
 			},
@@ -141,8 +140,8 @@ var testSet = map[string]struct {
 			"target3": {Name: "target3"},
 		},
 		output: &TargetOperation{
-			Add: []*types.TargetConfig{
-				{
+			Add: map[string]*types.TargetConfig{
+				"target3": {
 					Name: "target3",
 				},
 			},
@@ -155,12 +154,6 @@ func TestGetInstancesTagsMatches(t *testing.T) {
 	for name, item := range testSet {
 		t.Run(name, func(t *testing.T) {
 			res := Diff(item.m1, item.m2)
-			sort.Slice(res.Add, func(i, j int) bool {
-				return res.Add[i].Name < res.Add[j].Name
-			})
-			sort.Slice(item.output.Add, func(i, j int) bool {
-				return item.output.Add[i].Name < item.output.Add[j].Name
-			})
 			t.Logf("exp value: %+v", item.output)
 			t.Logf("got value: %+v", res)
 			if !cmp.Equal(item.output, res) {