diff --git a/components/gitpod-protocol/go/gitpod-service.go b/components/gitpod-protocol/go/gitpod-service.go index 7cb8355f24a2e2..1b037f2a4cdc40 100644 --- a/components/gitpod-protocol/go/gitpod-service.go +++ b/components/gitpod-protocol/go/gitpod-service.go @@ -1756,7 +1756,6 @@ type PortConfig struct { Visibility string `json:"visibility,omitempty"` Description string `json:"description,omitempty"` Name string `json:"name,omitempty"` - Sort uint32 `json:"sort,omitempty"` } // TaskConfig is the TaskConfig message type diff --git a/components/supervisor/pkg/ports/ports-config.go b/components/supervisor/pkg/ports/ports-config.go index 9118e1e803dbb5..dd1028125f43db 100644 --- a/components/supervisor/pkg/ports/ports-config.go +++ b/components/supervisor/pkg/ports/ports-config.go @@ -16,28 +16,36 @@ import ( "github.com/gitpod-io/gitpod/supervisor/pkg/config" ) +const NON_CONFIGED_BASIC_SCORE = 100000 + // RangeConfig is a port range config. type RangeConfig struct { - *gitpod.PortsItems + gitpod.PortsItems Start uint32 End uint32 Sort uint32 } +// SortConfig is a port with a sort field +type SortConfig struct { + gitpod.PortConfig + Sort uint32 +} + // Configs provides access to port configurations. type Configs struct { - workspaceConfigs map[uint32]*gitpod.PortConfig - instancePortConfigs map[uint32]*gitpod.PortConfig + workspaceConfigs map[uint32]*SortConfig + instancePortConfigs map[uint32]*SortConfig instanceRangeConfigs []*RangeConfig } // ForEach iterates over all configured ports. -func (configs *Configs) ForEach(callback func(port uint32, config *gitpod.PortConfig)) { +func (configs *Configs) ForEach(callback func(port uint32, config *SortConfig)) { if configs == nil { return } visited := make(map[uint32]struct{}) - for _, configs := range []map[uint32]*gitpod.PortConfig{configs.instancePortConfigs, configs.workspaceConfigs} { + for _, configs := range []map[uint32]*SortConfig{configs.instancePortConfigs, configs.workspaceConfigs} { for port, config := range configs { _, exists := visited[port] if exists { @@ -60,7 +68,7 @@ var ( ) // Get returns the config for the give port. -func (configs *Configs) Get(port uint32) (*gitpod.PortConfig, ConfigKind, bool) { +func (configs *Configs) Get(port uint32) (*SortConfig, ConfigKind, bool) { if configs == nil { return nil, PortConfigKind, false } @@ -74,13 +82,15 @@ func (configs *Configs) Get(port uint32) (*gitpod.PortConfig, ConfigKind, bool) } for _, rangeConfig := range configs.instanceRangeConfigs { if rangeConfig.Start <= port && port <= rangeConfig.End { - return &gitpod.PortConfig{ - Port: float64(port), - OnOpen: rangeConfig.OnOpen, - Visibility: rangeConfig.Visibility, - Description: rangeConfig.Description, - Name: rangeConfig.Name, - Sort: rangeConfig.Sort, + return &SortConfig{ + PortConfig: gitpod.PortConfig{ + Port: float64(port), + OnOpen: rangeConfig.OnOpen, + Visibility: rangeConfig.Visibility, + Description: rangeConfig.Description, + Name: rangeConfig.Name, + }, + Sort: rangeConfig.Sort, }, RangeConfigKind, true } } @@ -170,22 +180,26 @@ func (service *ConfigService) update(config *gitpod.GitpodConfig, current *Confi var portRangeRegexp = regexp.MustCompile(`^(\d+)[-:](\d+)$`) -func parseWorkspaceConfigs(ports []*gitpod.PortConfig) (portConfigs map[uint32]*gitpod.PortConfig) { +func parseWorkspaceConfigs(ports []*gitpod.PortConfig) (portConfigs map[uint32]*SortConfig) { if len(ports) == 0 { return nil } - portConfigs = make(map[uint32]*gitpod.PortConfig) + portConfigs = make(map[uint32]*SortConfig) for _, config := range ports { port := uint32(config.Port) _, exists := portConfigs[port] if !exists { - portConfigs[port] = config + portConfigs[port] = &SortConfig{ + PortConfig: *config, + // We don't care about workspace configs but instance config + Sort: NON_CONFIGED_BASIC_SCORE - 1, + } } } return portConfigs } -func parseInstanceConfigs(ports []*gitpod.PortsItems) (portConfigs map[uint32]*gitpod.PortConfig, rangeConfigs []*RangeConfig) { +func parseInstanceConfigs(ports []*gitpod.PortsItems) (portConfigs map[uint32]*SortConfig, rangeConfigs []*RangeConfig) { for index, config := range ports { if config == nil { continue @@ -195,18 +209,20 @@ func parseInstanceConfigs(ports []*gitpod.PortsItems) (portConfigs map[uint32]*g Port, err := strconv.ParseUint(rawPort, 10, 16) if err == nil { if portConfigs == nil { - portConfigs = make(map[uint32]*gitpod.PortConfig) + portConfigs = make(map[uint32]*SortConfig) } port := uint32(Port) _, exists := portConfigs[port] if !exists { - portConfigs[port] = &gitpod.PortConfig{ - OnOpen: config.OnOpen, - Port: float64(Port), - Visibility: config.Visibility, - Description: config.Description, - Name: config.Name, - Sort: uint32(index), + portConfigs[port] = &SortConfig{ + PortConfig: gitpod.PortConfig{ + OnOpen: config.OnOpen, + Port: float64(Port), + Visibility: config.Visibility, + Description: config.Description, + Name: config.Name, + }, + Sort: uint32(index), } } continue @@ -224,7 +240,7 @@ func parseInstanceConfigs(ports []*gitpod.PortsItems) (portConfigs map[uint32]*g continue } rangeConfigs = append(rangeConfigs, &RangeConfig{ - PortsItems: config, + PortsItems: *config, Start: uint32(start), End: uint32(end), Sort: uint32(index), diff --git a/components/supervisor/pkg/ports/ports-config_test.go b/components/supervisor/pkg/ports/ports-config_test.go index 39c88d7490741d..c505995aec22ff 100644 --- a/components/supervisor/pkg/ports/ports-config_test.go +++ b/components/supervisor/pkg/ports/ports-config_test.go @@ -89,7 +89,7 @@ func TestPortsConfig(t *testing.T) { Expectation: &PortConfigTestExpectations{ InstanceRangeConfigs: []*RangeConfig{ { - PortsItems: &gitpod.PortsItems{ + PortsItems: gitpod.PortsItems{ Port: "9229-9339", OnOpen: "ignore", Visibility: "public", @@ -136,7 +136,7 @@ func TestPortsConfig(t *testing.T) { t.Fatal(err) case change := <-updates: for _, config := range change.workspaceConfigs { - actual.WorkspaceConfigs = append(actual.WorkspaceConfigs, config) + actual.WorkspaceConfigs = append(actual.WorkspaceConfigs, &config.PortConfig) } } @@ -150,7 +150,7 @@ func TestPortsConfig(t *testing.T) { case change := <-updates: actual.InstanceRangeConfigs = change.instanceRangeConfigs for _, config := range change.instancePortConfigs { - actual.InstancePortConfigs = append(actual.InstancePortConfigs, config) + actual.InstancePortConfigs = append(actual.InstancePortConfigs, &config.PortConfig) } } } diff --git a/components/supervisor/pkg/ports/ports.go b/components/supervisor/pkg/ports/ports.go index 1570f51af7eaae..d7bd4a96a3f643 100644 --- a/components/supervisor/pkg/ports/ports.go +++ b/components/supervisor/pkg/ports/ports.go @@ -291,7 +291,7 @@ func (pm *Manager) updateState(ctx context.Context, exposed []ExposedPort, serve stateChanged := !reflect.DeepEqual(newState, pm.state) pm.state = newState - if !stateChanged { + if !stateChanged && configured == nil { return } @@ -315,10 +315,14 @@ func (pm *Manager) nextState(ctx context.Context) map[uint32]*managedPort { return mp } config, _, exists := pm.configs.Get(port) + var portConfig *gitpod.PortConfig + if exists && config != nil { + portConfig = &config.PortConfig + } mp := &managedPort{ LocalhostPort: port, - OnExposed: getOnExposedAction(config, port), - OnOpen: getOnOpenAction(config, port), + OnExposed: getOnExposedAction(portConfig, port), + OnOpen: getOnOpenAction(portConfig, port), } if exists { mp.Name = config.Name @@ -358,7 +362,7 @@ func (pm *Manager) nextState(ctx context.Context) map[uint32]*managedPort { // 2. second capture configured since we don't want to auto expose already exposed ports if pm.configs != nil { - pm.configs.ForEach(func(port uint32, config *gitpod.PortConfig) { + pm.configs.ForEach(func(port uint32, config *SortConfig) { if pm.boundInternally(port) { return } @@ -740,17 +744,12 @@ func (pm *Manager) Subscribe() (*Subscription, error) { func (pm *Manager) getStatus() []*api.PortsStatus { res := make([]*api.PortsStatus, 0, len(pm.state)) for port := range pm.state { - portStatus := pm.getPortStatus(port) - // Filter out ports that not served and not inside `.gitpod.yml` - if _, _, ok := pm.configs.Get(port); !ok && !portStatus.Served { - continue - } - res = append(res, portStatus) + res = append(res, pm.getPortStatus(port)) } sort.SliceStable(res, func(i, j int) bool { // Max number of port 65536 - score1 := 100000 + res[i].LocalPort - score2 := 100000 + res[j].LocalPort + score1 := NON_CONFIGED_BASIC_SCORE + res[i].LocalPort + score2 := NON_CONFIGED_BASIC_SCORE + res[j].LocalPort if c, _, ok := pm.configs.Get(res[i].LocalPort); ok { score1 = c.Sort } diff --git a/components/supervisor/pkg/ports/ports_test.go b/components/supervisor/pkg/ports/ports_test.go index 7156b236ec8686..c14d883698be61 100644 --- a/components/supervisor/pkg/ports/ports_test.go +++ b/components/supervisor/pkg/ports/ports_test.go @@ -63,8 +63,8 @@ func TestPortsUpdateState(t *testing.T) { []*api.PortsStatus{{LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private}}, []*api.PortsStatus{{LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private, Url: "foobar"}}}, []*api.PortsStatus{{LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private, Url: "foobar"}}, {LocalPort: 60000, Served: true}}, - []*api.PortsStatus{{LocalPort: 60000, Served: true}}, - {}, + []*api.PortsStatus{{LocalPort: 8080, Served: false, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private, Url: "foobar"}}, {LocalPort: 60000, Served: true}}, + []*api.PortsStatus{{LocalPort: 8080, Served: false, OnOpen: api.PortsStatus_notify_private, Exposed: &api.ExposedPortInfo{OnExposed: api.OnPortExposedAction_notify_private, Visibility: api.PortVisibility_private, Url: "foobar"}}}, }, }, { @@ -168,6 +168,7 @@ func TestPortsUpdateState(t *testing.T) { {LocalPort: 60000}, }, ExpectedUpdates: UpdateExpectation{ + {}, {}, []*api.PortsStatus{{LocalPort: 4040, Served: true, OnOpen: api.PortsStatus_open_browser}}, []*api.PortsStatus{{LocalPort: 4040, Served: true, OnOpen: api.PortsStatus_open_browser, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_public, Url: "4040-foobar", OnExposed: api.OnPortExposedAction_open_browser}}}, @@ -234,8 +235,8 @@ func TestPortsUpdateState(t *testing.T) { ExpectedUpdates: UpdateExpectation{ {}, { - {LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private}, {LocalPort: 3000, Served: true, OnOpen: api.PortsStatus_notify_private}, + {LocalPort: 8080, Served: true, OnOpen: api.PortsStatus_notify_private}, }, }, }, @@ -496,6 +497,185 @@ func TestPortsUpdateState(t *testing.T) { {{LocalPort: 3000, Name: "react", Served: true, OnOpen: api.PortsStatus_notify, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_private, OnExposed: api.OnPortExposedAction_notify, Url: "foobar"}}}, }, }, + { + Desc: "change configed ports order", + Changes: []Change{ + { + Config: &ConfigChange{instance: []*gitpod.PortsItems{ + {Port: 3001, Visibility: "private", Name: "react"}, + {Port: 3000, Visibility: "private", Name: "react"}, + }}, + }, + { + Config: &ConfigChange{instance: []*gitpod.PortsItems{ + {Port: "5000-5999", Visibility: "private", Name: "react"}, + {Port: 3001, Visibility: "private", Name: "react"}, + {Port: 3000, Visibility: "private", Name: "react"}, + }}, + }, + { + Served: []ServedPort{{net.IPv4zero, 5002, false}}, + }, + { + Served: []ServedPort{{net.IPv4zero, 5002, false}, {net.IPv4zero, 5001, false}}, + }, + { + Config: &ConfigChange{instance: []*gitpod.PortsItems{ + {Port: 3000, Visibility: "private", Name: "react"}, + {Port: 3001, Visibility: "private", Name: "react"}, + }}, + }, + { + Served: []ServedPort{{net.IPv4zero, 5001, false}, {net.IPv4zero, 3000, false}}, + }, + { + Exposed: []ExposedPort{{LocalPort: 3000, Public: false, URL: "foobar"}}, + }, + }, + ExpectedExposure: []ExposedPort{ + {LocalPort: 5002}, + {LocalPort: 5001}, + {LocalPort: 3000}, + {LocalPort: 3001}, + }, + ExpectedUpdates: UpdateExpectation{ + {}, + { + {LocalPort: 3001, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3000, Name: "react", OnOpen: api.PortsStatus_notify}, + }, + { + {LocalPort: 3001, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3000, Name: "react", OnOpen: api.PortsStatus_notify}, + }, + { + {LocalPort: 5002, Name: "react", Served: true, OnOpen: api.PortsStatus_notify}, + {LocalPort: 3001, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3000, Name: "react", OnOpen: api.PortsStatus_notify}, + }, + { + {LocalPort: 5001, Name: "react", Served: true, OnOpen: api.PortsStatus_notify}, + {LocalPort: 5002, Name: "react", Served: true, OnOpen: api.PortsStatus_notify}, + {LocalPort: 3001, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3000, Name: "react", OnOpen: api.PortsStatus_notify}, + }, + { + {LocalPort: 3000, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3001, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 5001, Served: true, OnOpen: api.PortsStatus_notify_private}, + {LocalPort: 5002, Served: true, OnOpen: api.PortsStatus_notify_private}, + }, + { + {LocalPort: 3000, Name: "react", Served: true, OnOpen: api.PortsStatus_notify}, + {LocalPort: 3001, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 5001, Served: true, OnOpen: api.PortsStatus_notify_private}, + }, + { + {LocalPort: 3000, Name: "react", Served: true, OnOpen: api.PortsStatus_notify, Exposed: &api.ExposedPortInfo{Visibility: api.PortVisibility_private, OnExposed: api.OnPortExposedAction_notify, Url: "foobar"}}, + {LocalPort: 3001, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 5001, Served: true, OnOpen: api.PortsStatus_notify_private}, + }, + }, + }, + { + Desc: "change configed ports order with ranged covered not ranged", + Changes: []Change{ + { + Config: &ConfigChange{ + workspace: []*gitpod.PortConfig{ + {Port: 3000, Visibility: "private", Name: "react"}, + }, + instance: []*gitpod.PortsItems{ + {Port: 3001, Visibility: "private", Name: "react"}, + {Port: 3000, Visibility: "private", Name: "react"}, + }, + }, + }, + { + Config: &ConfigChange{ + workspace: []*gitpod.PortConfig{ + {Port: 3000, Visibility: "private", Name: "react"}, + }, + instance: []*gitpod.PortsItems{ + {Port: 3003, Visibility: "private", Name: "react"}, + {Port: 3001, Visibility: "private", Name: "react"}, + {Port: "3001-3005", Visibility: "private", Name: "react"}, + {Port: 3000, Visibility: "private", Name: "react"}, + }, + }, + }, + { + Served: []ServedPort{{net.IPv4zero, 3000, false}}, + }, + { + Served: []ServedPort{{net.IPv4zero, 3000, false}, {net.IPv4zero, 3001, false}, {net.IPv4zero, 3002, false}}, + }, + { + Config: &ConfigChange{ + workspace: []*gitpod.PortConfig{ + {Port: 3000, Visibility: "private", Name: "react"}, + }, + instance: []*gitpod.PortsItems{ + {Port: 3003, Visibility: "private", Name: "react"}, + {Port: 3000, Visibility: "private", Name: "react"}, + }, + }, + }, + { + Config: &ConfigChange{ + workspace: []*gitpod.PortConfig{ + {Port: 3000, Visibility: "private", Name: "react"}, + }, + instance: []*gitpod.PortsItems{ + {Port: "3001-3005", Visibility: "private", Name: "react"}, + {Port: 3003, Visibility: "private", Name: "react"}, + {Port: 3000, Visibility: "private", Name: "react"}, + }, + }, + }, + }, + ExpectedExposure: []ExposedPort{ + {LocalPort: 3000}, + {LocalPort: 3001}, + {LocalPort: 3002}, + {LocalPort: 3003}, + }, + ExpectedUpdates: UpdateExpectation{ + {}, + { + {LocalPort: 3001, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3000, Name: "react", OnOpen: api.PortsStatus_notify}, + }, + { + {LocalPort: 3003, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3001, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3000, Name: "react", OnOpen: api.PortsStatus_notify}, + }, + { + {LocalPort: 3003, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3001, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3000, Served: true, Name: "react", OnOpen: api.PortsStatus_notify}, + }, + { + {LocalPort: 3003, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3001, Served: true, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3002, Served: true, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3000, Served: true, Name: "react", OnOpen: api.PortsStatus_notify}, + }, + { + {LocalPort: 3003, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3000, Served: true, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3001, Served: true, OnOpen: api.PortsStatus_notify_private}, + {LocalPort: 3002, Served: true, OnOpen: api.PortsStatus_notify_private}, + }, + { + {LocalPort: 3001, Name: "react", Served: true, OnOpen: api.PortsStatus_notify}, + {LocalPort: 3002, Name: "react", Served: true, OnOpen: api.PortsStatus_notify}, + {LocalPort: 3003, Name: "react", OnOpen: api.PortsStatus_notify}, + {LocalPort: 3000, Served: true, Name: "react", OnOpen: api.PortsStatus_notify}, + }, + }, + }, } log.Log.Logger.SetLevel(logrus.FatalLevel) @@ -584,8 +764,6 @@ func TestPortsUpdateState(t *testing.T) { wg.Wait() var ( - sorPorts = cmpopts.SortSlices(func(x, y uint32) bool { return x < y }) - sortPortStatus = cmpopts.SortSlices(func(x, y *api.PortsStatus) bool { return x.LocalPort < y.LocalPort }) sortExposed = cmpopts.SortSlices(func(x, y ExposedPort) bool { return x.LocalPort < y.LocalPort }) ignoreUnexported = cmpopts.IgnoreUnexported( api.PortsStatus{}, @@ -596,7 +774,7 @@ func TestPortsUpdateState(t *testing.T) { t.Errorf("unexpected exposures (-want +got):\n%s", diff) } - if diff := cmp.Diff(test.ExpectedUpdates, UpdateExpectation(updts), sorPorts, sortPortStatus, ignoreUnexported); diff != "" { + if diff := cmp.Diff(test.ExpectedUpdates, UpdateExpectation(updts), ignoreUnexported); diff != "" { t.Errorf("unexpected updates (-want +got):\n%s", diff) } })