From 7ab4d47fe9370e6cb70a59165f72947f10b47e58 Mon Sep 17 00:00:00 2001
From: Shriram Sharma <shriram_sharma@intuit.com>
Date: Fri, 27 May 2022 09:10:42 -0700
Subject: [PATCH] Added mutex to sync access to the SidecarEgressMap (#220)

---
 admiral/pkg/controller/common/types.go      | 14 +++++
 admiral/pkg/controller/common/types_test.go | 57 +++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/admiral/pkg/controller/common/types.go b/admiral/pkg/controller/common/types.go
index 7a51f7ab..91e20a26 100644
--- a/admiral/pkg/controller/common/types.go
+++ b/admiral/pkg/controller/common/types.go
@@ -177,6 +177,8 @@ func (s *SidecarEgressMap) Put(identity string, namespace string, fqdn string, c
 }
 
 func (s *SidecarEgressMap) Get(key string) map[string]SidecarEgress {
+	defer s.mutex.Unlock()
+	s.mutex.Lock()
 	return s.cache[key]
 }
 
@@ -186,6 +188,18 @@ func (s *SidecarEgressMap) Delete(key string) {
 	delete(s.cache, key)
 }
 
+// Map func returns a map of identity to namespace:SidecarEgress map
+// Iterating through the returned map is not implicitly thread safe,
+// use (s *SidecarEgressMap) Range() func instead.
 func (s *SidecarEgressMap) Map() map[string]map[string]SidecarEgress {
 	return s.cache
 }
+
+// Range is a thread safe iterator to iterate through the SidecarEgress map
+func (s *SidecarEgressMap) Range(fn func(k string, v map[string]SidecarEgress)) {
+	defer s.mutex.Unlock()
+	s.mutex.Lock()
+	for k, v := range s.cache {
+		fn(k, v)
+	}
+}
diff --git a/admiral/pkg/controller/common/types_test.go b/admiral/pkg/controller/common/types_test.go
index ebb33d2c..741a0d41 100644
--- a/admiral/pkg/controller/common/types_test.go
+++ b/admiral/pkg/controller/common/types_test.go
@@ -3,6 +3,7 @@ package common
 import (
 	"context"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -191,3 +192,59 @@ func TestMapRange(t *testing.T) {
 	assert.Equal(t, 3, numOfIter)
 
 }
+
+func TestSidecarEgressGet(t *testing.T) {
+
+	egressMap := NewSidecarEgressMap()
+	egressMap.Put("pkey1", "pkey2", "fqdn", map[string]string{"pkey2": "pkey2"})
+
+	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(3*time.Second))
+	defer cancel()
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+	// Producer go routine
+	go func(ctx context.Context) {
+		defer wg.Done()
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			default:
+				egressMap.Put("pkey1", string(uuid.NewUUID()), "fqdn", map[string]string{"pkey2": "pkey2"})
+			}
+		}
+	}(ctx)
+
+	// Consumer go routine
+	go func(ctx context.Context) {
+		defer wg.Done()
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			default:
+				assert.NotNil(t, egressMap.Get("pkey1"))
+			}
+		}
+	}(ctx)
+
+	wg.Wait()
+}
+
+func TestSidecarEgressRange(t *testing.T) {
+
+	egressMap := NewSidecarEgressMap()
+	egressMap.Put("pkey1", "pkey2", "fqdn", map[string]string{"pkey2": "pkey2"})
+	egressMap.Put("pkey2", "pkey2", "fqdn", map[string]string{"pkey2": "pkey2"})
+	egressMap.Put("pkey3", "pkey2", "fqdn", map[string]string{"pkey2": "pkey2"})
+
+	numOfIter := 0
+	egressMap.Range(func(k string, v map[string]SidecarEgress) {
+		assert.NotNil(t, v)
+		numOfIter++
+	})
+
+	assert.Equal(t, 3, numOfIter)
+
+}