diff --git a/cmd/agent/container/credentials_server.go b/cmd/agent/container/credentials_server.go index 4f4c62ceb..f4ed1ff9d 100644 --- a/cmd/agent/container/credentials_server.go +++ b/cmd/agent/container/credentials_server.go @@ -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{ diff --git a/cmd/runusercommands.go b/cmd/runusercommands.go index 19aa85307..1ede89c47 100644 --- a/cmd/runusercommands.go +++ b/cmd/runusercommands.go @@ -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 @@ -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}, @@ -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 { @@ -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 { diff --git a/cmd/up/up.go b/cmd/up/up.go index 45192c327..0670b8f26 100644 --- a/cmd/up/up.go +++ b/cmd/up/up.go @@ -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, diff --git a/cmd/up/up_client.go b/cmd/up/up_client.go index 8bdba46bc..6bfa6f47a 100644 --- a/cmd/up/up_client.go +++ b/cmd/up/up_client.go @@ -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, diff --git a/e2e/tests/ssh/ports_attributes_test.go b/e2e/tests/ssh/ports_attributes_test.go index cc9944904..702c9f895 100644 --- a/e2e/tests/ssh/ports_attributes_test.go +++ b/e2e/tests/ssh/ports_attributes_test.go @@ -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", + ) + }, + ) }) diff --git a/e2e/tests/ssh/testdata/ports-attributes/.devcontainer.json b/e2e/tests/ssh/testdata/ports-attributes/.devcontainer.json index bddd1c120..70913fa70 100644 --- a/e2e/tests/ssh/testdata/ports-attributes/.devcontainer.json +++ b/e2e/tests/ssh/testdata/ports-attributes/.devcontainer.json @@ -9,6 +9,10 @@ }, "9501": { "onAutoForward": "ignore" + }, + "9503": { + "requireLocalPort": true, + "onAutoForward": "silent" } }, "otherPortsAttributes": { diff --git a/pkg/netstat/watcher.go b/pkg/netstat/watcher.go index c14c687d3..552cc16ad 100644 --- a/pkg/netstat/watcher.go +++ b/pkg/netstat/watcher.go @@ -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 { @@ -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) diff --git a/pkg/netstat/watcher_test.go b/pkg/netstat/watcher_test.go index 8e6ce5340..dee88c579 100644 --- a/pkg/netstat/watcher_test.go +++ b/pkg/netstat/watcher_test.go @@ -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} @@ -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) @@ -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)) @@ -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") +} diff --git a/pkg/tunnel/forwarder.go b/pkg/tunnel/forwarder.go index 56e5f23db..54c538b45 100644 --- a/pkg/tunnel/forwarder.go +++ b/pkg/tunnel/forwarder.go @@ -3,8 +3,10 @@ package tunnel import ( "context" "fmt" + "runtime" "slices" "strconv" + "strings" "sync" config2 "github.com/devsy-org/devsy/pkg/devcontainer/config" @@ -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) }