Skip to content

Commit

Permalink
Add Ports to lib/config and lib/service/servicecfg
Browse files Browse the repository at this point in the history
  • Loading branch information
ravicious committed Oct 18, 2024
1 parent 8b2b96a commit a801fc8
Show file tree
Hide file tree
Showing 4 changed files with 225 additions and 0 deletions.
11 changes: 11 additions & 0 deletions lib/config/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -2056,6 +2056,17 @@ func applyAppsConfig(fc *FileConfig, cfg *servicecfg.Config) error {
ExternalID: application.AWS.ExternalID,
}
}

if len(application.Ports) != 0 {
app.Ports = make([]servicecfg.PortRange, 0, len(application.Ports))
for _, portRange := range application.Ports {
app.Ports = append(app.Ports, servicecfg.PortRange{
Port: portRange.Port,
EndPort: portRange.EndPort,
})
}
}

if err := app.CheckAndSetDefaults(); err != nil {
return trace.Wrap(err)
}
Expand Down
17 changes: 17 additions & 0 deletions lib/config/fileconf.go
Original file line number Diff line number Diff line change
Expand Up @@ -2025,6 +2025,12 @@ type App struct {
// CORS defines the Cross-Origin Resource Sharing configuration for the app,
// controlling how resources are shared across different origins.
CORS *CORS `yaml:"cors,omitempty"`

// Ports is a list of ports and port ranges that an app agent can forward connections to.
// Only applicable to TCP App Access.
// If this field is not empty, URI is expected to contain no port number and start with the tcp
// protocol.
Ports []PortRange `yaml:"ports,omitempty"`
}

// CORS represents the configuration for Cross-Origin Resource Sharing (CORS)
Expand Down Expand Up @@ -2071,6 +2077,17 @@ type AppAWS struct {
ExternalID string `yaml:"external_id,omitempty"`
}

// PortRange describes a port range for TCP apps. The range starts with Port and ends with EndPort.
// PortRange can be used to describe a single port in which case the Port field is the port and the
// EndPort field is 0.
type PortRange struct {
// Port describes the start of the range. It must be between 1-65535.
Port uint32 `yaml:"port"`
// EndPort describes the end of the range, inclusive. It must be between 2-65535 and be greater
// than Port when describing a port range. When describing a single port, it must be set to 0.
EndPort uint32 `yaml:"end_port,omitempty"`
}

// Proxy is a `proxy_service` section of the config file:
type Proxy struct {
// Service is a generic service configuration section
Expand Down
66 changes: 66 additions & 0 deletions lib/service/servicecfg/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ type App struct {
// CORS defines the Cross-Origin Resource Sharing configuration for the app,
// controlling how resources are shared across different origins.
CORS *CORS

// Ports is a list of ports and port ranges that an app agent can forward connections to.
// Only applicable to TCP App Access.
// If this field is not empty, URI is expected to contain no port number and start with the tcp
// protocol.
Ports []PortRange
}

// CORS represents the configuration for Cross-Origin Resource Sharing (CORS)
Expand Down Expand Up @@ -125,6 +131,17 @@ type CORS struct {
MaxAge uint `yaml:"max_age"`
}

// PortRange describes a port range for TCP apps. The range starts with Port and ends with EndPort.
// PortRange can be used to describe a single port in which case the Port field is the port and the
// EndPort field is 0.
type PortRange struct {
// Port describes the start of the range. It must be between 1-65535.
Port uint32
// EndPort describes the end of the range, inclusive. It must be between 2-65535 and be greater
// than Port when describing a port range. When describing a single port, it must be set to 0.
EndPort uint32
}

// CheckAndSetDefaults validates an application.
func (a *App) CheckAndSetDefaults() error {
if a.Name == "" {
Expand Down Expand Up @@ -173,6 +190,55 @@ func (a *App) CheckAndSetDefaults() error {
}
}
}

if len(a.Ports) != 0 {
if err := a.checkPorts(); err != nil {
return trace.Wrap(err)
}
}

return nil
}

func (a *App) checkPorts() error {
// Parsing the URI here does not break compatibility. The URI is parsed only if Ports are present.
// This means that old apps that do have invalid URIs but don't use Ports can continue existing.
uri, err := url.Parse(a.URI)
if err != nil {
return trace.BadParameter("invalid app URI format: %v", err)
}

// The scheme of URI is not validated to be "tcp" on purpose. This way in the future we can add
// multi-port support to web apps without throwing hard errors when a cluster with a multi-port
// web app gets downgraded to a version which supports multi-port only for TCP apps.
//
// For now, we simply ignore the Ports field set on non-TCP apps.
if uri.Scheme != "tcp" {
return nil
}

if uri.Port() != "" {
return trace.BadParameter("app URI %q must not include a port number when the app spec defines a list of ports", a.URI)
}

const minPort = 1
const maxPort = 65535
for _, portRange := range a.Ports {
if portRange.Port < minPort || portRange.Port > maxPort {
return trace.BadParameter("app port must be between %d and %d, but got %d", minPort, maxPort, portRange.Port)
}

if portRange.EndPort != 0 {
if portRange.EndPort < minPort+1 || portRange.EndPort > maxPort {
return trace.BadParameter("app end port must be between %d and %d, but got %d", minPort+1, maxPort, portRange.EndPort)
}

if portRange.EndPort <= portRange.Port {
return trace.BadParameter("app end port must be greater than port (%d vs %d)", portRange.EndPort, portRange.Port)
}
}
}

return nil
}

Expand Down
131 changes: 131 additions & 0 deletions lib/service/servicecfg/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,137 @@ func TestCheckApp(t *testing.T) {
}
}

func TestCheckAppPorts(t *testing.T) {
type check func(t *testing.T, err error)

hasNoErr := func() check {
return func(t *testing.T, err error) {
require.NoError(t, err)
}
}
hasErrTypeBadParameter := func() check {
return func(t *testing.T, err error) {
require.True(t, trace.IsBadParameter(err))
}
}
hasErrTypeBadParameterAndContains := func(msg string) check {
return func(t *testing.T, err error) {
require.True(t, trace.IsBadParameter(err), "err should be trace.BadParameter")
require.ErrorContains(t, err, msg)
}
}
hasErrAndContains := func(msg string) check {
return func(t *testing.T, err error) {
require.ErrorContains(t, err, msg)
}
}

tests := []struct {
name string
ports []PortRange
uri string
check check
}{
{
name: "valid ranges and single ports",
ports: []PortRange{
PortRange{Port: 22, EndPort: 25},
PortRange{Port: 26},
PortRange{Port: 65535},
},
check: hasNoErr(),
},
{
name: "valid overlapping ranges",
ports: []PortRange{
PortRange{Port: 100, EndPort: 200},
PortRange{Port: 150, EndPort: 175},
PortRange{Port: 111},
PortRange{Port: 150, EndPort: 210},
PortRange{Port: 1, EndPort: 65535},
},
check: hasNoErr(),
},
{
name: "valid non-TCP app with ports ignored",
uri: "http://localhost:8000",
ports: []PortRange{
PortRange{Port: 123456789},
PortRange{Port: 10, EndPort: 2},
},
check: hasNoErr(),
},
// Test cases for invalid ports.
{
name: "port smaller than 1",
ports: []PortRange{
PortRange{Port: 0},
},
check: hasErrTypeBadParameter(),
},
{
name: "port bigger than 65535",
ports: []PortRange{
PortRange{Port: 78787},
},
check: hasErrTypeBadParameter(),
},
{
name: "end port smaller than 2",
ports: []PortRange{
PortRange{Port: 5, EndPort: 1},
},
check: hasErrTypeBadParameterAndContains("end port must be between"),
},
{
name: "end port bigger than 65535",
ports: []PortRange{
PortRange{Port: 1, EndPort: 78787},
},
check: hasErrTypeBadParameter(),
},
{
name: "end port smaller than port",
ports: []PortRange{
PortRange{Port: 10, EndPort: 5},
},
check: hasErrTypeBadParameterAndContains("end port must be greater than port"),
},
{
name: "uri specifies port",
uri: "tcp://localhost:1234",
ports: []PortRange{
PortRange{Port: 1000, EndPort: 1500},
},
check: hasErrTypeBadParameterAndContains("must not include a port number"),
},
{
name: "invalid uri",
uri: "%",
ports: []PortRange{
PortRange{Port: 1000, EndPort: 1500},
},
check: hasErrAndContains("invalid URL escape"),
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
app := App{
Name: "foo",
URI: "tcp://localhost",
Ports: tc.ports,
}
if tc.uri != "" {
app.URI = tc.uri
}

err := app.CheckAndSetDefaults()
tc.check(t, err)
})
}
}

// TestDatabaseStaticLabels ensures the static labels are set.
func TestDatabaseStaticLabels(t *testing.T) {
db := Database{
Expand Down

0 comments on commit a801fc8

Please sign in to comment.