Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cmd/agent/container/credentials_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,11 @@ func portOptionsFromResult() []netstat.WatcherOption {
}
attr := devconfig.ResolvePortAttribute(portNum, pa, opa)
return netstat.PortForwardAttribute{
Label: attr.Label,
Protocol: attr.Protocol,
OnAutoForward: attr.OnAutoForward,
Label: attr.Label,
Protocol: attr.Protocol,
OnAutoForward: attr.OnAutoForward,
RequireLocalPort: attr.RequireLocalPort,
ElevateIfNeeded: attr.ElevateIfNeeded,
}
}
return []netstat.WatcherOption{
Expand Down
10 changes: 6 additions & 4 deletions cmd/runusercommands.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ func NewRunUserCommandsCmdAlias(f *flags.GlobalFlags) *cobra.Command {
return primary
}

const updateContentCommand = "updateContentCommand"

type lifecycleExecParams struct {
ctx context.Context
helper *docker.DockerHelper
Expand Down Expand Up @@ -384,7 +386,7 @@ func (cmd *RunUserCommandsCmd) runLifecycleHooks(
skip bool
}{
{"onCreateCommand", result.MergedConfig.OnCreateCommands, cmd.SkipOnCreate},
{"updateContentCommand", result.MergedConfig.UpdateContentCommands, cmd.SkipUpdateContent},
{updateContentCommand, result.MergedConfig.UpdateContentCommands, cmd.SkipUpdateContent},
{"postCreateCommand", result.MergedConfig.PostCreateCommands, cmd.SkipPostCreate},
{"postStartCommand", result.MergedConfig.PostStartCommands, cmd.SkipPostStart},
{"postAttachCommand", result.MergedConfig.PostAttachCommands, cmd.SkipPostAttach},
Expand All @@ -394,7 +396,7 @@ func (cmd *RunUserCommandsCmd) runLifecycleHooks(

for i, hook := range hooks {
if cmd.Prebuild && i >= 2 {
log.Infof("stopping lifecycle execution (--prebuild: after updateContentCommand)")
log.Infof("stopping lifecycle execution (--prebuild: after %s)", updateContentCommand)
return nil
}
if cmd.SkipNonBlockingCommands && i > waitForBoundary {
Expand Down Expand Up @@ -424,14 +426,14 @@ func resolveWaitForBoundary(result *devcconfig.Result) int {
}
hookNames := []string{
"onCreateCommand",
"updateContentCommand",
updateContentCommand,
"postCreateCommand",
"postStartCommand",
"postAttachCommand",
}
waitFor := result.MergedConfig.WaitFor
if waitFor == "" {
waitFor = "updateContentCommand"
waitFor = updateContentCommand
}
for i, name := range hookNames {
if name == waitFor {
Expand Down
2 changes: 1 addition & 1 deletion cmd/up/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func NewUpCmd(f *flags.GlobalFlags) *cobra.Command {
}

// Run runs the command logic.
func (cmd *UpCmd) Run(
func (cmd *UpCmd) Run( //nolint:cyclop
ctx context.Context,
devsyConfig *config.Config,
client client2.BaseWorkspaceClient,
Expand Down
2 changes: 1 addition & 1 deletion cmd/up/up_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ var inheritedEnvironmentVariables = []string{
"GIT_COMMITTER_DATE",
}

func (cmd *UpCmd) prepareClient(
func (cmd *UpCmd) prepareClient( //nolint:funlen
ctx context.Context,
devsyConfig *config.Config,
args []string,
Expand Down
91 changes: 91 additions & 0 deletions e2e/tests/ssh/ports_attributes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,4 +204,95 @@ var _ = ginkgo.Describe("devsy portsAttributes e2e",
)
},
)

ginkgo.It(
"should skip forwarding when requireLocalPort=true and host port is occupied",
ginkgo.SpecTimeout(framework.TimeoutShort()),
func(ctx context.Context) {
if runtime.GOOS == "windows" {
ginkgo.Skip("skipping on windows")
}

requirePort := 9503

// Occupy the host port BEFORE starting the workspace
hostListener, err := net.Listen(
"tcp",
net.JoinHostPort("localhost", strconv.Itoa(requirePort)),
)
framework.ExpectNoError(err)
ginkgo.DeferCleanup(func() {
_ = hostListener.Close()
})

tempDir, err := framework.CopyToTempDir("tests/ssh/testdata/ports-attributes")
framework.ExpectNoError(err)

f := framework.NewDefaultFramework(initialDir + "/bin")
_ = f.DevsyProviderAdd(ctx, "docker")
err = f.DevsyProviderUse(ctx, "docker")
framework.ExpectNoError(err)

ginkgo.DeferCleanup(func(cleanupCtx context.Context) {
_ = f.DevsyWorkspaceDelete(cleanupCtx, tempDir)
framework.CleanupTempDir(initialDir, tempDir)
})

err = f.DevsyUp(ctx, tempDir)
framework.ExpectNoError(err)

serverCtx, serverCancel := context.WithCancel(ctx)
defer serverCancel()

workspaceName := filepath.Base(tempDir)

// Start server inside the container on port 9503
// #nosec G204 -- test command with controlled arguments
serverCmd := exec.CommandContext(serverCtx, f.DevsyBinDir+"/"+f.DevsyBinName,
"ssh", tempDir, "--command",
"go run /workspaces/"+workspaceName+"/server.go "+strconv.Itoa(requirePort),
)
err = serverCmd.Start()
framework.ExpectNoError(err)

var wg sync.WaitGroup
wg.Go(func() { _ = serverCmd.Wait() })

portForwardCtx, cancelPort := context.WithTimeout(ctx, 60*time.Second)
defer cancelPort()
wg.Go(func() {
_ = f.DevsyPortTest(portForwardCtx, strconv.Itoa(requirePort), tempDir)
})

ginkgo.DeferCleanup(func() {
serverCancel()
cancelPort()
wg.Wait()
})

// Port 9503 should NOT be reachable because the host port is occupied
// and requireLocalPort=true prevents fallback to another port
requireAddr := net.JoinHostPort("localhost", strconv.Itoa(requirePort))
gomega.Consistently(func() bool {
conn, dialErr := net.DialTimeout("tcp", requireAddr, 1*time.Second)
if dialErr != nil {
return false
}
// If we connect, check if it's the host listener (not the container server)
_ = conn.SetReadDeadline(time.Now().Add(1 * time.Second))
buf := make([]byte, 1024)
n, readErr := conn.Read(buf)
_ = conn.Close()
// The host listener doesn't write anything, so we should get
// a timeout or EOF — not "PONG\n" from the container
if readErr == nil && n > 0 && string(buf[:n]) == "PONG\n" {
return true
}
return false
}, 5*time.Second, 1*time.Second).Should(
gomega.BeFalse(),
"Port 9503 (requireLocalPort=true) should NOT be forwarded when host port is occupied",
)
},
)
})
4 changes: 4 additions & 0 deletions e2e/tests/ssh/testdata/ports-attributes/.devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
},
"9501": {
"onAutoForward": "ignore"
},
"9503": {
"requireLocalPort": true,
"onAutoForward": "silent"
}
},
"otherPortsAttributes": {
Expand Down
31 changes: 23 additions & 8 deletions pkg/netstat/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,24 @@ import (
"strconv"
"time"

"github.com/devsy-org/devsy/pkg/devcontainer/config"
"github.com/devsy-org/devsy/pkg/log"
)

const AutoForwardIgnore = "ignore"
const (
AutoForwardIgnore = config.AutoForwardIgnore
AutoForwardSilent = config.AutoForwardSilent
)

// PortForwardAttribute carries port metadata resolved from portsAttributes
// in the devcontainer config. Downstream forwarders use this to apply
// protocol, label, and forwarding-policy decisions.
type PortForwardAttribute struct {
Label string
Protocol string
OnAutoForward string
Label string
Protocol string
OnAutoForward string
RequireLocalPort bool
ElevateIfNeeded bool
}

type Forwarder interface {
Expand Down Expand Up @@ -113,10 +119,19 @@ func (w *Watcher) runOnce() error {
continue
}

if attr.Label != "" {
log.Debugf("Found open port %s (%s) ready to forward", port, attr.Label)
} else {
log.Debugf("Found open port %s ready to forward", port)
switch attr.OnAutoForward {
case AutoForwardSilent, "":
if attr.Label != "" {
log.Debugf("Found open port %s (%s) ready to forward", port, attr.Label)
} else {
log.Debugf("Found open port %s ready to forward", port)
}
default:
if attr.Label != "" {
log.Infof("Found open port %s (%s) ready to forward", port, attr.Label)
} else {
log.Infof("Found open port %s ready to forward", port)
}
}

err = w.forwarder.Forward(port, attr)
Expand Down
32 changes: 28 additions & 4 deletions pkg/netstat/watcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestWatcher_ResolveAttr_WithResolver(t *testing.T) {
return PortForwardAttribute{
Label: "Web App",
Protocol: "https",
OnAutoForward: "silent",
OnAutoForward: AutoForwardSilent,
}
}
return PortForwardAttribute{OnAutoForward: AutoForwardIgnore}
Expand All @@ -77,7 +77,7 @@ func TestWatcher_ResolveAttr_WithResolver(t *testing.T) {
attr := w.resolveAttr("3000")
assert.Equal(t, "Web App", attr.Label)
assert.Equal(t, "https", attr.Protocol)
assert.Equal(t, "silent", attr.OnAutoForward)
assert.Equal(t, AutoForwardSilent, attr.OnAutoForward)

attr = w.resolveAttr("9999")
assert.Equal(t, "ignore", attr.OnAutoForward)
Expand All @@ -88,7 +88,7 @@ func TestWatcher_PortAttributes_IgnoreSkipsForward(t *testing.T) {
if port == "9501" {
return PortForwardAttribute{OnAutoForward: AutoForwardIgnore}
}
return PortForwardAttribute{OnAutoForward: "silent", Label: "Allowed"}
return PortForwardAttribute{OnAutoForward: AutoForwardSilent, Label: "Allowed"}
}
w := NewWatcher(&mockForwarder{}, WithPortAttributes(resolver))

Expand All @@ -97,6 +97,30 @@ func TestWatcher_PortAttributes_IgnoreSkipsForward(t *testing.T) {
assert.Equal(t, "ignore", attr.OnAutoForward)

attr = w.resolveAttr("9500")
assert.Equal(t, "silent", attr.OnAutoForward)
assert.Equal(t, AutoForwardSilent, attr.OnAutoForward)
assert.Equal(t, "Allowed", attr.Label)
}

func TestWatcher_OnAutoForwardSilent_Forwards(t *testing.T) {
mf := &mockForwarder{}
resolver := func(port string) PortForwardAttribute {
return PortForwardAttribute{OnAutoForward: AutoForwardSilent, Label: "Silent Service"}
}
w := NewWatcher(mf, WithPortAttributes(resolver))

attr := w.resolveAttr("8080")
assert.Equal(t, AutoForwardSilent, attr.OnAutoForward)
assert.NotEqual(t, AutoForwardIgnore, attr.OnAutoForward, "silent should not skip")
}

func TestWatcher_OnAutoForwardNotify_Forwards(t *testing.T) {
mf := &mockForwarder{}
resolver := func(port string) PortForwardAttribute {
return PortForwardAttribute{OnAutoForward: "notify", Label: "Notify Service"}
}
w := NewWatcher(mf, WithPortAttributes(resolver))

attr := w.resolveAttr("8080")
assert.Equal(t, "notify", attr.OnAutoForward)
assert.NotEqual(t, AutoForwardIgnore, attr.OnAutoForward, "notify should not skip")
}
28 changes: 25 additions & 3 deletions pkg/tunnel/forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package tunnel
import (
"context"
"fmt"
"runtime"
"slices"
"strconv"
"strings"
"sync"

config2 "github.com/devsy-org/devsy/pkg/devcontainer/config"
Expand Down Expand Up @@ -67,12 +69,32 @@ func (f *forwarder) Forward(port string, _ netstat.PortForwardAttribute) error {
}
}

if attr.ElevateIfNeeded {
portNum, _ := strconv.Atoi(port)
if portNum < 1024 {
if runtime.GOOS == "linux" {
log.Warnf(
"Port %s requires elevation (elevateIfNeeded=true); privileged port binding not supported in tunnel mode",
port,
)
} else {
log.Warnf("Port %s: elevateIfNeeded is only applicable on Linux", port)
}
}
}

cancelCtx, cancel := context.WithCancel(context.Background())
f.portMap[port] = cancel

label := attr.Label
if label != "" {
log.Infof("Start port-forwarding on port %s (%s)", port, label)
parts := []string{}
if attr.Label != "" {
parts = append(parts, attr.Label)
}
if attr.Protocol != "" {
parts = append(parts, attr.Protocol)
}
if len(parts) > 0 {
log.Infof("Start port-forwarding on port %s (%s)", port, strings.Join(parts, ", "))
} else {
log.Infof("Start port-forwarding on port %s", port)
}
Expand Down
Loading