diff --git a/CHANGELOG.md b/CHANGELOG.md index 33d55c03..259dfd2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# Unreleased + +* Fix proxy name conflicts leaking an open port #69 + # 1.2.0 * Add a Toxic and Toxics type for the Go client diff --git a/api.go b/api.go index 97ca5aa1..83823814 100644 --- a/api.go +++ b/api.go @@ -117,15 +117,8 @@ func (server *server) ProxyCreate(response http.ResponseWriter, request *http.Re proxy.Name = input.Name proxy.Listen = input.Listen proxy.Upstream = input.Upstream - if input.Enabled { - err = proxy.Start() - if err != nil { - http.Error(response, server.apiError(err, http.StatusConflict), http.StatusConflict) - return - } - } - err = server.collection.Add(proxy) + err = server.collection.Add(proxy, input.Enabled) if err != nil { http.Error(response, server.apiError(err, http.StatusConflict), http.StatusConflict) return diff --git a/api_test.go b/api_test.go index db3e8517..ccb5357f 100644 --- a/api_test.go +++ b/api_test.go @@ -231,16 +231,56 @@ func TestDeleteProxy(t *testing.T) { }) } -func TestCreateProxyTwice(t *testing.T) { +func TestCreateProxyPortConflict(t *testing.T) { WithServer(t, func(addr string) { err := testProxy.Create() if err != nil { t.Fatal("Unable to create proxy") } - err = testProxy.Create() + testProxy2 := *testProxy + testProxy2.Name = "test" + err = testProxy2.Create() if err == nil { - t.Fatal("Expected error when creating same proxy twice") + t.Fatal("Proxy did not result in conflict.") + } else if err.Error() != "Create: HTTP 409: listen tcp 127.0.0.1:3310: bind: address already in use" { + t.Fatal("Incorrect error adding proxy:", err) + } + + err = testProxy.Delete() + if err != nil { + t.Fatal("Unable to delete proxy: ", err) + } + err = testProxy2.Create() + if err != nil { + t.Fatal("Unable to create proxy: ", err) + } + }) +} + +func TestCreateProxyNameConflict(t *testing.T) { + WithServer(t, func(addr string) { + err := testProxy.Create() + if err != nil { + t.Fatal("Unable to create proxy: ", err) + } + + testProxy2 := *testProxy + testProxy2.Listen = "localhost:3311" + err = testProxy2.Create() + if err == nil { + t.Fatal("Proxy did not result in conflict.") + } else if err.Error() != "Create: HTTP 409: Proxy with name mysql_master already exists" { + t.Fatal("Incorrect error adding proxy:", err) + } + + err = testProxy.Delete() + if err != nil { + t.Fatal("Unable to delete proxy: ", err) + } + err = testProxy2.Create() + if err != nil { + t.Fatal("Unable to create proxy: ", err) } }) } diff --git a/proxy_collection.go b/proxy_collection.go index 27a4ff9e..c8975d9a 100644 --- a/proxy_collection.go +++ b/proxy_collection.go @@ -21,7 +21,7 @@ func NewProxyCollection() *ProxyCollection { } } -func (collection *ProxyCollection) Add(proxy *Proxy) error { +func (collection *ProxyCollection) Add(proxy *Proxy, start bool) error { collection.Lock() defer collection.Unlock() @@ -29,6 +29,13 @@ func (collection *ProxyCollection) Add(proxy *Proxy) error { return fmt.Errorf("Proxy with name %s already exists", proxy.Name) } + if start { + err := proxy.Start() + if err != nil { + return err + } + } + collection.proxies[proxy.Name] = proxy return nil diff --git a/proxy_collection_test.go b/proxy_collection_test.go index 648777ca..90690468 100644 --- a/proxy_collection_test.go +++ b/proxy_collection_test.go @@ -14,7 +14,7 @@ func TestAddProxyToCollection(t *testing.T) { t.Error("Expected proxies to be empty") } - err := collection.Add(proxy) + err := collection.Add(proxy, false) if err != nil { t.Error("Expected to be able to add first proxy to collection") } @@ -28,12 +28,12 @@ func TestAddTwoProxiesToCollection(t *testing.T) { collection := NewProxyCollection() proxy := NewTestProxy("test", "localhost:20000") - err := collection.Add(proxy) + err := collection.Add(proxy, false) if err != nil { t.Error("Expected to be able to add first proxy to collection") } - err = collection.Add(proxy) + err = collection.Add(proxy, false) if err == nil { t.Error("Expected to not be able to add proxy with same name") } @@ -43,14 +43,35 @@ func TestListProxies(t *testing.T) { collection := NewProxyCollection() proxy := NewTestProxy("test", "localhost:20000") - err := collection.Add(proxy) + err := collection.Add(proxy, false) if err != nil { t.Error("Expected to be able to add first proxy to collection") } proxies := collection.Proxies() - if _, ok := proxies[proxy.Name]; !ok { + proxy, ok := proxies[proxy.Name] + if !ok { t.Error("Expected to be able to see existing proxy") + } else if proxy.Enabled { + t.Error("Expected proxy not to be running") + } +} + +func TestAddProxyAndStart(t *testing.T) { + collection := NewProxyCollection() + proxy := NewTestProxy("test", "localhost:20000") + + err := collection.Add(proxy, true) + if err != nil { + t.Error("Expected to be able to add proxy to collection:", err) + } + + proxies := collection.Proxies() + proxy, ok := proxies[proxy.Name] + if !ok { + t.Error("Expected to be able to see existing proxy") + } else if !proxy.Enabled { + t.Error("Expected proxy to be running") } } @@ -62,7 +83,7 @@ func TestAddAndRemoveProxyFromCollection(t *testing.T) { t.Error("Expected proxies to be empty") } - err := collection.Add(proxy) + err := collection.Add(proxy, false) if err != nil { t.Error("Expected to be able to add first proxy to collection") }