From 7f93a2a394863950aeb416cbb78c99ce1e958f3c Mon Sep 17 00:00:00 2001 From: Anton Kosyakov Date: Thu, 3 Dec 2020 17:22:45 +0000 Subject: [PATCH] [supervisor] avoid auto exposing the same port multiple times --- components/supervisor/pkg/ports/ports.go | 46 ++++++++++++------- components/supervisor/pkg/ports/ports_test.go | 41 +++++++++++++---- 2 files changed, 62 insertions(+), 25 deletions(-) diff --git a/components/supervisor/pkg/ports/ports.go b/components/supervisor/pkg/ports/ports.go index 1954a3db9aa8eb..dec10c07864a03 100644 --- a/components/supervisor/pkg/ports/ports.go +++ b/components/supervisor/pkg/ports/ports.go @@ -43,8 +43,9 @@ func NewManager(exposed ExposedPortsInterface, served ServedPortsObserver, confi S: served, C: config, - internal: internal, - proxies: make(map[uint32]*localhostProxy), + internal: internal, + proxies: make(map[uint32]*localhostProxy), + autoExposed: make(map[uint32]uint32), state: state, subscriptions: make(map[*Subscription]struct{}), @@ -66,6 +67,7 @@ type Manager struct { internal map[uint32]struct{} proxies map[uint32]*localhostProxy proxyStarter func(LocalhostPort uint32, GlobalPort uint32) (proxy io.Closer, err error) + autoExposed map[uint32]uint32 configs *Configs exposed []ExposedPort @@ -263,17 +265,18 @@ func (pm *Manager) nextState() map[uint32]*managedPort { return } mp.OnExposed = getOnExposedAction(config, port) + + _, autoExposed := pm.autoExposed[port] + if autoExposed { + return + } + mp.GlobalPort = port mp.Visibility = api.PortVisibility_public if config.Visibility == "private" { mp.Visibility = api.PortVisibility_private } public := mp.Visibility == api.PortVisibility_public - err := pm.E.Expose(ctx, mp.LocalhostPort, mp.GlobalPort, public) - if err != nil { - log.WithError(err).WithField("port", *mp).Warn("cannot auto-expose port") - return - } - log.WithField("port", *mp).Warn("auto-expose port") + pm.autoExpose(ctx, mp, public) }) } @@ -295,7 +298,11 @@ func (pm *Manager) nextState() map[uint32]*managedPort { mp.LocalhostPort = port mp.Served = true - exposedGlobalPort := mp.GlobalPort + exposedGlobalPort, autoExposed := pm.autoExposed[port] + if mp.Exposed { + exposedGlobalPort = mp.GlobalPort + } + if served.BoundToLocalhost { proxy, exists := pm.proxies[port] if exists { @@ -308,7 +315,7 @@ func (pm *Manager) nextState() map[uint32]*managedPort { mp.GlobalPort = port } - if mp.GlobalPort == 0 || (mp.Exposed && mp.GlobalPort == exposedGlobalPort) { + if mp.GlobalPort == 0 || ((mp.Exposed || autoExposed) && mp.GlobalPort == exposedGlobalPort) { continue } @@ -321,16 +328,21 @@ func (pm *Manager) nextState() map[uint32]*managedPort { public = exists && config.Visibility != "private" } - err := pm.E.Expose(ctx, mp.LocalhostPort, mp.GlobalPort, public) - if err != nil { - log.WithError(err).WithField("port", *mp).Warn("cannot auto-expose port") - continue - } - log.WithField("port", *mp).Warn("auto-expose port") + pm.autoExpose(ctx, mp, public) } return state } +func (pm *Manager) autoExpose(ctx context.Context, mp *managedPort, public bool) { + err := pm.E.Expose(ctx, mp.LocalhostPort, mp.GlobalPort, public) + if err != nil { + log.WithError(err).WithField("port", *mp).Warn("cannot auto-expose port") + return + } + pm.autoExposed[mp.LocalhostPort] = mp.GlobalPort + log.WithField("port", *mp).Info("auto-expose port") +} + func (pm *Manager) updateProxies() { opened := make(map[uint32]struct{}, len(pm.served)) for _, p := range pm.served { @@ -527,7 +539,7 @@ func (pm *Manager) getPortStatus(port uint32) *api.PortsStatus { LocalPort: mp.LocalhostPort, Served: mp.Served, } - if mp.Exposed { + if mp.Exposed && mp.URL != "" { ps.Exposed = &api.ExposedPortInfo{ Visibility: mp.Visibility, Url: mp.URL, diff --git a/components/supervisor/pkg/ports/ports_test.go b/components/supervisor/pkg/ports/ports_test.go index a137390480e2c6..79467b7c17a3ee 100644 --- a/components/supervisor/pkg/ports/ports_test.go +++ b/components/supervisor/pkg/ports/ports_test.go @@ -47,7 +47,7 @@ func TestPortsUpdateState(t *testing.T) { Desc: "basic locally served", Changes: []Change{ {Served: []ServedPort{{8080, true}}}, - {Exposed: []ExposedPort{{LocalPort: 8080, GlobalPort: 60000}}}, + {Exposed: []ExposedPort{{LocalPort: 8080, GlobalPort: 60000, URL: "foobar"}}}, {Served: []ServedPort{{8080, true}, {60000, false}}}, {Served: []ServedPort{{60000, false}}}, {Served: []ServedPort{}}, @@ -58,8 +58,8 @@ func TestPortsUpdateState(t *testing.T) { ExpectedUpdates: UpdateExpectation{ {}, []*api.PortsStatus{{LocalPort: 8080, GlobalPort: 60000, Served: true}}, - []*api.PortsStatus{{LocalPort: 8080, GlobalPort: 60000, Served: true, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private}}}, - []*api.PortsStatus{{LocalPort: 8080, GlobalPort: 60000, Served: false, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private}}}, + []*api.PortsStatus{{LocalPort: 8080, GlobalPort: 60000, Served: true, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private, Url: "foobar"}}}, + []*api.PortsStatus{{LocalPort: 8080, GlobalPort: 60000, Served: false, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private, Url: "foobar"}}}, }, }, { @@ -125,13 +125,13 @@ func TestPortsUpdateState(t *testing.T) { }, }, ExpectedExposure: []ExposedPort{ - {LocalPort: 8080, Public: true}, - {LocalPort: 9229}, + {LocalPort: 8080, GlobalPort: 8080, Public: true}, + {LocalPort: 9229, GlobalPort: 9229}, {LocalPort: 9229, GlobalPort: 60000}, }, ExpectedUpdates: UpdateExpectation{ {}, - []*api.PortsStatus{{LocalPort: 8080}, {LocalPort: 9229}}, + []*api.PortsStatus{{LocalPort: 8080, GlobalPort: 8080}, {LocalPort: 9229, GlobalPort: 9229}}, []*api.PortsStatus{ {LocalPort: 8080, GlobalPort: 8080, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_public, Url: "8080-foobar", OnExposed: api.OnPortExposedAction_open_browser}}, {LocalPort: 9229, GlobalPort: 9229, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_private, Url: "9229-foobar", OnExposed: api.OnPortExposedAction_ignore}}, @@ -200,13 +200,13 @@ func TestPortsUpdateState(t *testing.T) { }, }, ExpectedExposure: []ExposedPort{ - {LocalPort: 8080, Public: false}, + {LocalPort: 8080, GlobalPort: 8080, Public: false}, {LocalPort: 8080, GlobalPort: 60000, Public: true}, {LocalPort: 8080, GlobalPort: 8080, Public: true}, }, ExpectedUpdates: UpdateExpectation{ {}, - []*api.PortsStatus{{LocalPort: 8080}}, + []*api.PortsStatus{{LocalPort: 8080, GlobalPort: 8080}}, []*api.PortsStatus{{LocalPort: 8080, GlobalPort: 8080, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_private, OnExposed: api.OnPortExposedAction_notify, Url: "foobar"}}}, []*api.PortsStatus{{LocalPort: 8080, GlobalPort: 8080, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_public, OnExposed: api.OnPortExposedAction_notify, Url: "foobar"}}}, []*api.PortsStatus{{LocalPort: 8080, GlobalPort: 60000, Served: true, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_public, OnExposed: api.OnPortExposedAction_notify, Url: "foobar"}}}, @@ -233,6 +233,31 @@ func TestPortsUpdateState(t *testing.T) { }, }, }, + { + Desc: "served between auto exposing configured and exposed update", + Changes: []Change{ + { + Config: &ConfigChange{workspace: []*gitpod.PortConfig{ + {Port: 8080, Visibility: "private"}, + }}, + }, + { + Served: []ServedPort{{8080, false}}, + }, + { + Exposed: []ExposedPort{{LocalPort: 8080, GlobalPort: 8080, Public: false, URL: "foobar"}}, + }, + }, + ExpectedExposure: []ExposedPort{ + {LocalPort: 8080, GlobalPort: 8080}, + }, + ExpectedUpdates: UpdateExpectation{ + {}, + {{LocalPort: 8080, GlobalPort: 8080}}, + {{LocalPort: 8080, GlobalPort: 8080, Served: true}}, + {{LocalPort: 8080, GlobalPort: 8080, Served: true, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_private, OnExposed: api.OnPortExposedAction_notify, Url: "foobar"}}}, + }, + }, } log.Log.Logger.SetLevel(logrus.FatalLevel)