Use address of ssh daemon when creating a tunnel

Change-Id: Ie66daa17104be1ea06849eccaaee78b2b4353c2e
Signed-off-by: Aleksander Mistewicz <a.mistewicz@samsung.com>

Use net.TCPAddr instead of net.IP as it contains Port field.

Some methods were renamed to reflect migration from net.IP to
net.TCPAddr. It allows multiple dryads to coexist behind NAT or on
a single host. Currently, it is possible to provide network addresses to
non-existent address or to a different dryad than expected. Addresses in
Register() call are checked only for resolvability.

Change-Id: I46cecb8ad6f06fabb88f2262d95c0678329915ab
Signed-off-by: Aleksander Mistewicz <a.mistewicz@samsung.com>

4 files changed, 88 insertions, 59 deletions

diff --git a/workers/dryadclientmanager_mock_test.go b/workers/dryadclientmanager_mock_test.go
index 1d46791..8db3a0d 100644
--- a/workers/dryadclientmanager_mock_test.go
+++ b/workers/dryadclientmanager_mock_test.go
@@ -46,15 +46,15 @@ func (mr *MockDryadClientManagerMockRecorder) Close() *gomock.Call {
 }
 
 // Create mocks base method
-func (m *MockDryadClientManager) Create(arg0 net.IP, arg1 int) error {
-	ret := m.ctrl.Call(m, "Create", arg0, arg1)
+func (m *MockDryadClientManager) Create(arg0 *net.TCPAddr) error {
+	ret := m.ctrl.Call(m, "Create", arg0)
 	ret0, _ := ret[0].(error)
 	return ret0
 }
 
 // Create indicates an expected call of Create
-func (mr *MockDryadClientManagerMockRecorder) Create(arg0, arg1 interface{}) *gomock.Call {
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Create", reflect.TypeOf((*MockDryadClientManager)(nil).Create), arg0, arg1)
+func (mr *MockDryadClientManagerMockRecorder) Create(arg0 interface{}) *gomock.Call {
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Create", reflect.TypeOf((*MockDryadClientManager)(nil).Create), arg0)
 }
 
 // Healthcheck mocks base method
diff --git a/workers/error.go b/workers/error.go
index 6cf60bd..455ab1c 100644
--- a/workers/error.go
+++ b/workers/error.go
@@ -44,4 +44,10 @@ var (
 	// ErrNoMatchingWorker is returned when there is no worker matching groups nor
 	// capabilities required by request.
 	ErrNoMatchingWorker = errors.New("No matching worker")
+	// ErrMissingIP is returned when Register is called with either dryad or sshd
+	// address missing IP value.
+	ErrMissingIP = errors.New("IP address is missing from address")
+	// ErrMissingPort is returned when Register is called with either dryad or sshd
+	// address missing Port value.
+	ErrMissingPort = errors.New("Port is missing from address")
 )
diff --git a/workers/worker_list_test.go b/workers/worker_list_test.go
index 1c85098..6c17bc2 100644
--- a/workers/worker_list_test.go
+++ b/workers/worker_list_test.go
@@ -24,26 +24,29 @@ import (
 	"net"
 
 	. "git.tizen.org/tools/boruta"
-	"git.tizen.org/tools/boruta/dryad/conf"
 	"git.tizen.org/tools/boruta/rpc/dryad"
 
 	gomock "github.com/golang/mock/gomock"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/ginkgo/extensions/table"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/types"
 	"github.com/satori/go.uuid"
 )
 
 var _ = Describe("WorkerList", func() {
 	var wl *WorkerList
-	dryadAddr := net.TCPAddr{
+	dryadAddr := &net.TCPAddr{
 		IP:   net.IPv4(127, 0, 0, 1),
 		Port: 7175,
 	}
-	sshdAddr := net.TCPAddr{
+	sshdAddr := &net.TCPAddr{
 		IP:   net.IPv4(127, 0, 0, 1),
 		Port: 22,
 	}
+	missingPort := &net.TCPAddr{
+		IP: dryadAddr.IP,
+	}
 	BeforeEach(func() {
 		wl = NewWorkerList()
 	})
@@ -62,6 +65,7 @@ var _ = Describe("WorkerList", func() {
 
 	Describe("Register", func() {
 		var registeredWorkers []string
+		invalidAddr := "addr.invalid"
 
 		BeforeEach(func() {
 			registeredWorkers = make([]string, 0)
@@ -99,6 +103,23 @@ var _ = Describe("WorkerList", func() {
 			}
 		}
 
+		DescribeTable("dryad and sshd addresses",
+			func(dryadAddress, sshAddress string, errMatcher types.GomegaMatcher) {
+				caps := getRandomCaps()
+				err := wl.Register(caps, dryadAddress, sshAddress)
+				Expect(err).To(errMatcher)
+			},
+			Entry("both addresses missing", "", "", Equal(ErrMissingIP)),
+			Entry("sshd address missing", dryadAddr.String(), "", Equal(ErrMissingIP)),
+			Entry("dryad address missing", "", sshdAddr.String(), Equal(ErrMissingIP)),
+			Entry("dryad port missing", missingPort.String(), sshdAddr.String(), Equal(ErrMissingPort)),
+			Entry("sshd port missing", dryadAddr.String(), missingPort.String(), Equal(ErrMissingPort)),
+			Entry("both ports missing", missingPort.String(), missingPort.String(), Equal(ErrMissingPort)),
+			Entry("both invalid", invalidAddr, invalidAddr, HaveOccurred()),
+			Entry("dryad invalid", invalidAddr, sshdAddr.String(), HaveOccurred()),
+			Entry("sshd invalid", dryadAddr.String(), invalidAddr, HaveOccurred()),
+		)
+
 		It("should add Worker in MAINTENANCE state", func() {
 			caps := getRandomCaps()
 			err := wl.Register(caps, dryadAddr.String(), sshdAddr.String())
@@ -368,7 +389,7 @@ var _ = Describe("WorkerList", func() {
 
 					It("should work to SetState", func() {
 						gomock.InOrder(
-							dcm.EXPECT().Create(ip, conf.DefaultRPCPort),
+							dcm.EXPECT().Create(info.dryad),
 							dcm.EXPECT().Prepare().Return(key, nil),
 							dcm.EXPECT().Close(),
 						)
@@ -381,7 +402,7 @@ var _ = Describe("WorkerList", func() {
 
 					It("should fail to SetState if dryadClientManager fails to prepare client", func() {
 						gomock.InOrder(
-							dcm.EXPECT().Create(ip, conf.DefaultRPCPort),
+							dcm.EXPECT().Create(info.dryad),
 							dcm.EXPECT().Prepare().Return(nil, testerr),
 							dcm.EXPECT().Close(),
 						)
@@ -393,7 +414,7 @@ var _ = Describe("WorkerList", func() {
 					})
 
 					It("should fail to SetState if dryadClientManager fails to create client", func() {
-						dcm.EXPECT().Create(ip, conf.DefaultRPCPort).Return(testerr)
+						dcm.EXPECT().Create(info.dryad).Return(testerr)
 
 						err := wl.SetState(worker, IDLE)
 						Expect(err).ToNot(HaveOccurred())
@@ -422,7 +443,7 @@ var _ = Describe("WorkerList", func() {
 
 						It("should work to SetState", func() {
 							gomock.InOrder(
-								dcm.EXPECT().Create(ip, conf.DefaultRPCPort),
+								dcm.EXPECT().Create(info.dryad),
 								dcm.EXPECT().PutInMaintenance(putStr),
 								dcm.EXPECT().Close(),
 							)
@@ -434,7 +455,7 @@ var _ = Describe("WorkerList", func() {
 
 						It("should fail to SetState if dryadClientManager fails to put dryad in maintenance state", func() {
 							gomock.InOrder(
-								dcm.EXPECT().Create(ip, conf.DefaultRPCPort),
+								dcm.EXPECT().Create(info.dryad),
 								dcm.EXPECT().PutInMaintenance(putStr).Return(testerr),
 								dcm.EXPECT().Close().Do(func() {
 									wl.mutex.Lock()
@@ -451,7 +472,7 @@ var _ = Describe("WorkerList", func() {
 						})
 
 						It("should fail to SetState if dryadClientManager fails to create client", func() {
-							dcm.EXPECT().Create(ip, conf.DefaultRPCPort).Return(testerr).Do(func(net.IP, int) {
+							dcm.EXPECT().Create(info.dryad).Return(testerr).Do(func(*net.TCPAddr) {
 								wl.mutex.Lock()
 								info.State = WorkerState("TEST")
 								wl.mutex.Unlock()
@@ -468,7 +489,7 @@ var _ = Describe("WorkerList", func() {
 				Describe("putInMaintenance", func() {
 					It("should work", func() {
 						gomock.InOrder(
-							dcm.EXPECT().Create(ip, conf.DefaultRPCPort),
+							dcm.EXPECT().Create(info.dryad),
 							dcm.EXPECT().PutInMaintenance(putStr),
 							dcm.EXPECT().Close(),
 						)
@@ -479,7 +500,7 @@ var _ = Describe("WorkerList", func() {
 
 					It("should fail if dryadClientManager fails to put dryad in maintenance state", func() {
 						gomock.InOrder(
-							dcm.EXPECT().Create(ip, conf.DefaultRPCPort),
+							dcm.EXPECT().Create(info.dryad),
 							dcm.EXPECT().PutInMaintenance(putStr).Return(testerr),
 							dcm.EXPECT().Close(),
 						)
@@ -489,7 +510,7 @@ var _ = Describe("WorkerList", func() {
 					})
 
 					It("should fail if dryadClientManager fails to create client", func() {
-						dcm.EXPECT().Create(ip, conf.DefaultRPCPort).Return(testerr)
+						dcm.EXPECT().Create(info.dryad).Return(testerr)
 
 						err := wl.putInMaintenance(worker)
 						Expect(err).To(Equal(testerr))
@@ -702,38 +723,38 @@ var _ = Describe("WorkerList", func() {
 
 		Describe("Setters and Getters", func() {
 			type genericGet func(wl *WorkerList, uuid WorkerUUID, expectedItem interface{}, expectedErr error)
-			getIP := genericGet(func(wl *WorkerList, uuid WorkerUUID, expectedItem interface{}, expectedErr error) {
-				item, err := wl.GetWorkerIP(uuid)
+			getDryad := genericGet(func(wl *WorkerList, uuid WorkerUUID, expectedItem interface{}, expectedErr error) {
+				item, err := wl.GetWorkerAddr(uuid)
 				if expectedErr != nil {
-					Expect(item).To(BeNil())
+					Expect(item).To(Equal(net.TCPAddr{}))
 					Expect(err).To(Equal(expectedErr))
 					return
 				}
 				Expect(err).ToNot(HaveOccurred())
-				Expect(item).To(Equal(expectedItem.(net.IP)))
+				Expect(item).To(Equal(expectedItem.(net.TCPAddr)))
 			})
-			getKey := genericGet(func(wl *WorkerList, uuid WorkerUUID, expectedItem interface{}, expectedErr error) {
-				item, err := wl.GetWorkerKey(uuid)
+			getSSH := genericGet(func(wl *WorkerList, uuid WorkerUUID, expectedItem interface{}, expectedErr error) {
+				item, err := wl.GetWorkerSSHAddr(uuid)
 				if expectedErr != nil {
+					Expect(item).To(Equal(net.TCPAddr{}))
 					Expect(err).To(Equal(expectedErr))
 					return
 				}
 				Expect(err).ToNot(HaveOccurred())
-				Expect(&item).To(Equal(expectedItem.(*rsa.PrivateKey)))
+				Expect(item).To(Equal(expectedItem.(net.TCPAddr)))
 			})
-			getters := []genericGet{getIP, getKey}
-
-			type genericSet func(wl *WorkerList, uuid WorkerUUID, expectedErr error) interface{}
-			setIP := genericSet(func(wl *WorkerList, uuid WorkerUUID, expectedErr error) interface{} {
-				ip := net.IP{255, 255, 255, 255}
-				err := wl.SetWorkerIP(uuid, ip)
+			getKey := genericGet(func(wl *WorkerList, uuid WorkerUUID, expectedItem interface{}, expectedErr error) {
+				item, err := wl.GetWorkerKey(uuid)
 				if expectedErr != nil {
 					Expect(err).To(Equal(expectedErr))
-					return nil
+					return
 				}
 				Expect(err).ToNot(HaveOccurred())
-				return ip
+				Expect(&item).To(Equal(expectedItem.(*rsa.PrivateKey)))
 			})
+			getters := []genericGet{getKey, getDryad, getSSH}
+
+			type genericSet func(wl *WorkerList, uuid WorkerUUID, expectedErr error) interface{}
 			setKey := genericSet(func(wl *WorkerList, uuid WorkerUUID, expectedErr error) interface{} {
 				key, err := rsa.GenerateKey(rand.Reader, 128)
 				Expect(err).ToNot(HaveOccurred())
@@ -745,7 +766,7 @@ var _ = Describe("WorkerList", func() {
 				Expect(err).ToNot(HaveOccurred())
 				return key
 			})
-			setters := []genericSet{setIP, setKey}
+			setters := []genericSet{setKey}
 
 			It("should fail to get information of nonexistent worker", func() {
 				uuid := randomUUID()
@@ -832,7 +853,7 @@ var _ = Describe("WorkerList", func() {
 				})
 				It("should set worker into IDLE state and prepare a key", func() {
 					gomock.InOrder(
-						dcm.EXPECT().Create(ip, conf.DefaultRPCPort),
+						dcm.EXPECT().Create(info.dryad),
 						dcm.EXPECT().Prepare().Return(key, nil),
 						dcm.EXPECT().Close(),
 					)
@@ -845,7 +866,7 @@ var _ = Describe("WorkerList", func() {
 				})
 				It("should fail to prepare worker if dryadClientManager fails to prepare client", func() {
 					gomock.InOrder(
-						dcm.EXPECT().Create(ip, conf.DefaultRPCPort),
+						dcm.EXPECT().Create(info.dryad),
 						dcm.EXPECT().Prepare().Return(nil, testerr),
 						dcm.EXPECT().Close(),
 					)
@@ -857,7 +878,7 @@ var _ = Describe("WorkerList", func() {
 					Expect(info.key).To(BeNil())
 				})
 				It("should fail to prepare worker if dryadClientManager fails to create client", func() {
-					dcm.EXPECT().Create(ip, conf.DefaultRPCPort).Return(testerr)
+					dcm.EXPECT().Create(info.dryad).Return(testerr)
 
 					err := wl.PrepareWorker(worker, true)
 					Expect(err).NotTo(HaveOccurred())
diff --git a/workers/workers.go b/workers/workers.go
index 6e3dd7c..da22336 100644
--- a/workers/workers.go
+++ b/workers/workers.go
@@ -19,13 +19,12 @@ package workers
 
 import (
 	"crypto/rsa"
-	"errors"
+	"fmt"
 	"math"
 	"net"
 	"sync"
 
 	. "git.tizen.org/tools/boruta"
-	"git.tizen.org/tools/boruta/dryad/conf"
 	"git.tizen.org/tools/boruta/rpc/dryad"
 )
 
@@ -88,19 +87,26 @@ func (wl *WorkerList) Register(caps Capabilities, dryadAddress string, sshAddres
 
 	dryad, err := net.ResolveTCPAddr("tcp", dryadAddress)
 	if err != nil {
-		return err
+		return fmt.Errorf("invalid dryad address: %s", err)
 	}
 	// dryad.IP is empty if dryadAddress provided port number only.
 	if dryad.IP == nil {
-		return errors.New("missing IP in dryad address")
+		return ErrMissingIP
+	}
+	if dryad.Port == 0 {
+		return ErrMissingPort
 	}
+
 	sshd, err := net.ResolveTCPAddr("tcp", sshAddress)
 	if err != nil {
-		return err
+		return fmt.Errorf("invalid sshd address: %s", err)
 	}
 	// same as with dryad.IP
 	if sshd.IP == nil {
-		return errors.New("missing IP in ssh address")
+		return ErrMissingIP
+	}
+	if sshd.Port == 0 {
+		return ErrMissingPort
 	}
 
 	wl.mutex.Lock()
@@ -283,30 +289,26 @@ func (wl *WorkerList) GetWorkerInfo(uuid WorkerUUID) (WorkerInfo, error) {
 	return worker.WorkerInfo, nil
 }
 
-// SetWorkerIP stores ip in the worker structure referenced by uuid.
-// It should be called after Register by function which is aware of
-// the source of the connection and therefore its IP address.
-func (wl *WorkerList) SetWorkerIP(uuid WorkerUUID, ip net.IP) error {
-	wl.mutex.Lock()
-	defer wl.mutex.Unlock()
+// GetWorkerAddr retrieves IP address from the internal structure.
+func (wl *WorkerList) GetWorkerAddr(uuid WorkerUUID) (net.TCPAddr, error) {
+	wl.mutex.RLock()
+	defer wl.mutex.RUnlock()
 	worker, ok := wl.workers[uuid]
 	if !ok {
-		return ErrWorkerNotFound
+		return net.TCPAddr{}, ErrWorkerNotFound
 	}
-	// FIXME
-	worker.dryad.IP = ip
-	return nil
+	return *worker.dryad, nil
 }
 
-// GetWorkerIP retrieves IP address from the internal structure.
-func (wl *WorkerList) GetWorkerIP(uuid WorkerUUID) (net.IP, error) {
+// GetWorkerSSHAddr retrieves address of worker's ssh daemon from the internal structure.
+func (wl *WorkerList) GetWorkerSSHAddr(uuid WorkerUUID) (net.TCPAddr, error) {
 	wl.mutex.RLock()
 	defer wl.mutex.RUnlock()
 	worker, ok := wl.workers[uuid]
 	if !ok {
-		return nil, ErrWorkerNotFound
+		return net.TCPAddr{}, ErrWorkerNotFound
 	}
-	return worker.dryad.IP, nil
+	return *worker.sshd, nil
 }
 
 // SetWorkerKey stores private key in the worker structure referenced by uuid.
@@ -441,12 +443,12 @@ func (wl *WorkerList) setState(worker WorkerUUID, state WorkerState) error {
 // prepareKey delegates key generation to Dryad and sets up generated key in the
 // worker. In case of any failure it returns an error.
 func (wl *WorkerList) prepareKey(worker WorkerUUID) error {
-	ip, err := wl.GetWorkerIP(worker)
+	addr, err := wl.GetWorkerAddr(worker)
 	if err != nil {
 		return err
 	}
 	client := wl.newDryadClient()
-	err = client.Create(ip, conf.DefaultRPCPort)
+	err = client.Create(&addr)
 	if err != nil {
 		return err
 	}
@@ -461,12 +463,12 @@ func (wl *WorkerList) prepareKey(worker WorkerUUID) error {
 
 // putInMaintenance orders Dryad to enter maintenance mode.
 func (wl *WorkerList) putInMaintenance(worker WorkerUUID) error {
-	ip, err := wl.GetWorkerIP(worker)
+	addr, err := wl.GetWorkerAddr(worker)
 	if err != nil {
 		return err
 	}
 	client := wl.newDryadClient()
-	err = client.Create(ip, conf.DefaultRPCPort)
+	err = client.Create(&addr)
 	if err != nil {
 		return err
 	}