Merge pull request #1073 from xmidt-org/denopink/patch/service/consul…

…-forloopRefVar-bug

patch: remove forloop var pass by reference

service/consul/environment.go

@@ -167,7 +167,7 @@ func newRegistrars(l *adapter.Logger, registrationScheme string, c gokitconsul.C
 		rid := zap.String("id", registration.ID)
 		in := zap.String("instance", instance)
 		l.Logger = l.Logger.With(rid, in)
-		consulRegistrar, err = NewRegistrar(c, u, &registration, l)
+		consulRegistrar, err = NewRegistrar(c, u, registration, l)
 		if err != nil {
 			return
 		}

service/consul/environment_test.go

@@ -1,13 +1,15 @@
 package consul
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/hashicorp/consul/api"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"github.com/xmidt-org/webpa-common/v2/adapter"
+
 	"github.com/xmidt-org/webpa-common/v2/service"
 )
 
@@ -136,8 +138,66 @@ func testNewEnvironmentFull(t *testing.T) {
 	ttlUpdater.AssertExpectations(t)
 }
 
+func testVerifyEnviromentRegistrars(t *testing.T) {
+	var (
+		require       = require.New(t)
+		logger        = adapter.DefaultLogger()
+		clientFactory = prepareMockClientFactory()
+		client        = new(mockClient)
+		ttlUpdater    = new(mockTTLUpdater)
+		registrations = []api.AgentServiceRegistration{
+			{
+				ID:      "deadbeef",
+				Name:    "deadbeef",
+				Tags:    []string{"role=deadbeef, region=1"},
+				Address: "deadbeef.com",
+				Port:    8080,
+			},
+			{
+				ID:      "api:deadbeef",
+				Name:    "deadbeef-api",
+				Tags:    []string{"role=deadbeef, region=1"},
+				Address: "deadbeef.com",
+				Port:    443,
+			},
+		}
+		co = Options{
+			Client: &api.Config{
+				Address: "localhost:8500",
+				Scheme:  "https",
+			},
+			Registrations: registrations,
+		}
+	)
+
+	clientFactory.On("NewClient", mock.MatchedBy(func(*api.Client) bool { return true })).Return(client, ttlUpdater).Once()
+
+	client.On("Register", mock.MatchedBy(func(r *api.AgentServiceRegistration) bool {
+		return reflect.DeepEqual(registrations[0], *r)
+	})).Return(error(nil)).Once()
+	client.On("Register", mock.MatchedBy(func(r *api.AgentServiceRegistration) bool {
+		return reflect.DeepEqual(registrations[1], *r)
+	})).Return(error(nil)).Once()
+
+	client.On("Deregister", mock.MatchedBy(func(r *api.AgentServiceRegistration) bool {
+		return reflect.DeepEqual(registrations[0], *r)
+	})).Return(error(nil)).Once()
+	client.On("Deregister", mock.MatchedBy(func(r *api.AgentServiceRegistration) bool {
+		return reflect.DeepEqual(registrations[1], *r)
+	})).Return(error(nil)).Once()
+	consulEnv, err := NewEnvironment(logger, service.DefaultScheme, co)
+	require.NoError(err)
+
+	consulEnv.Register()
+	consulEnv.Deregister()
+	clientFactory.AssertExpectations(t)
+	ttlUpdater.AssertExpectations(t)
+	client.AssertExpectations(t)
+}
+
 func TestNewEnvironment(t *testing.T) {
 	t.Run("Empty", testNewEnvironmentEmpty)
 	t.Run("ClientError", testNewEnvironmentClientError)
 	t.Run("Full", testNewEnvironmentFull)
+	t.Run("Verify Multi Registrar Environment", testVerifyEnviromentRegistrars)
 }

service/consul/registrar.go

@@ -130,7 +130,7 @@ type ttlRegistrar struct {
 }
 
 // NewRegistrar creates an sd.Registrar, binding any TTL checks to the Register/Deregister lifecycle as needed.
-func NewRegistrar(c gokitconsul.Client, u ttlUpdater, r *api.AgentServiceRegistration, logger *adapter.Logger) (sd.Registrar, error) {
+func NewRegistrar(c gokitconsul.Client, u ttlUpdater, r api.AgentServiceRegistration, logger *adapter.Logger) (sd.Registrar, error) {
 	var (
 		ttlChecks []ttlCheck
 		err       error
@@ -148,7 +148,7 @@ func NewRegistrar(c gokitconsul.Client, u ttlUpdater, r *api.AgentServiceRegistr
 		}
 	}
 
-	var registrar sd.Registrar = gokitconsul.NewRegistrar(c, r, logger)
+	var registrar sd.Registrar = gokitconsul.NewRegistrar(c, &r, logger)
 
 	// decorate the given registrar if we have any TTL checks
 	if len(ttlChecks) > 0 {

service/consul/registrar_test.go

@@ -39,7 +39,7 @@ func testNewRegistrarNoChecks(t *testing.T) {
 		ttlUpdater    = new(mockTTLUpdater)
 		tickerFactory = prepareMockTickerFactory()
 
-		registration = &api.AgentServiceRegistration{
+		registration = api.AgentServiceRegistration{
 			ID:      "service1",
 			Address: "somehost.com",
 			Port:    1111,
@@ -80,7 +80,7 @@ func testNewRegistrarNoTTL(t *testing.T) {
 		ttlUpdater    = new(mockTTLUpdater)
 		tickerFactory = prepareMockTickerFactory()
 
-		registration = &api.AgentServiceRegistration{
+		registration = api.AgentServiceRegistration{
 			ID:      "service1",
 			Address: "somehost.com",
 			Port:    1111,
@@ -131,7 +131,7 @@ func testNewRegistrarCheckMalformedTTL(t *testing.T) {
 		ttlUpdater    = new(mockTTLUpdater)
 		tickerFactory = prepareMockTickerFactory()
 
-		registration = &api.AgentServiceRegistration{
+		registration = api.AgentServiceRegistration{
 			ID:      "service1",
 			Address: "somehost.com",
 			Port:    1111,
@@ -161,7 +161,7 @@ func testNewRegistrarCheckTTLTooSmall(t *testing.T) {
 		ttlUpdater    = new(mockTTLUpdater)
 		tickerFactory = prepareMockTickerFactory()
 
-		registration = &api.AgentServiceRegistration{
+		registration = api.AgentServiceRegistration{
 			ID:      "service1",
 			Address: "somehost.com",
 			Port:    1111,
@@ -191,7 +191,7 @@ func testNewRegistrarChecksMalformedTTL(t *testing.T) {
 		ttlUpdater    = new(mockTTLUpdater)
 		tickerFactory = prepareMockTickerFactory()
 
-		registration = &api.AgentServiceRegistration{
+		registration = api.AgentServiceRegistration{
 			ID:      "service1",
 			Address: "somehost.com",
 			Port:    1111,
@@ -223,7 +223,7 @@ func testNewRegistrarChecksTTLTooSmall(t *testing.T) {
 		ttlUpdater    = new(mockTTLUpdater)
 		tickerFactory = prepareMockTickerFactory()
 
-		registration = &api.AgentServiceRegistration{
+		registration = api.AgentServiceRegistration{
 			ID:      "service1",
 			Address: "somehost.com",
 			Port:    1111,
@@ -272,7 +272,7 @@ func testNewRegistrarTTL(t *testing.T) {
 			close(update2Done)
 		}
 
-		registration = &api.AgentServiceRegistration{
+		registration = api.AgentServiceRegistration{
 			ID:      "service1",
 			Address: "somehost.com",
 			Port:    1111,