diff --git a/cmd/agent/container/credentials_server.go b/cmd/agent/container/credentials_server.go index 72d89bcfe..4f4c62ceb 100644 --- a/cmd/agent/container/credentials_server.go +++ b/cmd/agent/container/credentials_server.go @@ -203,11 +203,11 @@ func configureGitUserLocally( } func forwardPorts(ctx context.Context, client tunnel.TunnelClient) error { - opts := portFilterFromResult() + opts := portOptionsFromResult() return netstat.NewWatcher(&forwarder{ctx: ctx, client: client}, opts...).Run(ctx) } -func portFilterFromResult() []netstat.WatcherOption { +func portOptionsFromResult() []netstat.WatcherOption { raw, err := os.ReadFile(config.DevContainerResultPath) if err != nil { log.Debugf("Could not read result for port attributes: %v", err) @@ -223,14 +223,20 @@ func portFilterFromResult() []netstat.WatcherOption { return nil } pa, opa := mc.PortsAttributes, mc.OtherPortsAttributes + resolver := func(port string) netstat.PortForwardAttribute { + portNum, err := strconv.Atoi(port) + if err != nil { + return netstat.PortForwardAttribute{} + } + attr := devconfig.ResolvePortAttribute(portNum, pa, opa) + return netstat.PortForwardAttribute{ + Label: attr.Label, + Protocol: attr.Protocol, + OnAutoForward: attr.OnAutoForward, + } + } return []netstat.WatcherOption{ - netstat.WithPortFilter(func(port string) bool { - portNum, err := strconv.Atoi(port) - if err != nil { - return true - } - return devconfig.ResolvePortAttribute(portNum, pa, opa).ShouldAutoForward() - }), + netstat.WithPortAttributes(resolver), } } @@ -240,7 +246,10 @@ type forwarder struct { client tunnel.TunnelClient } -func (f *forwarder) Forward(port string) error { +func (f *forwarder) Forward(port string, attr netstat.PortForwardAttribute) error { + if attr.Label != "" { + log.Debugf("Forwarding port %s (%s, protocol=%s)", port, attr.Label, attr.Protocol) + } _, err := f.client.ForwardPort(f.ctx, &tunnel.ForwardPortRequest{Port: port}) return err } diff --git a/e2e/tests/ssh/ports_attributes_test.go b/e2e/tests/ssh/ports_attributes_test.go index ae91c3250..cc9944904 100644 --- a/e2e/tests/ssh/ports_attributes_test.go +++ b/e2e/tests/ssh/ports_attributes_test.go @@ -58,7 +58,7 @@ var _ = ginkgo.Describe("devsy portsAttributes e2e", workspaceName := filepath.Base(tempDir) - // Start server on the allowed port (9500) + // Start server on the allowed port (9500, label=TestService, protocol=http) // #nosec G204 -- test command with controlled arguments serverCmd := exec.CommandContext(serverCtx, f.DevsyBinDir+"/"+f.DevsyBinName, "ssh", tempDir, "--command", @@ -93,7 +93,7 @@ var _ = ginkgo.Describe("devsy portsAttributes e2e", wg.Wait() }) - // Port 9500 should be reachable + // Port 9500 should be reachable (onAutoForward=silent, label=TestService) address := net.JoinHostPort("localhost", strconv.Itoa(allowedPort)) gomega.Eventually(func() string { conn, err := net.DialTimeout("tcp", address, 3*time.Second) @@ -109,7 +109,7 @@ var _ = ginkgo.Describe("devsy portsAttributes e2e", return "" }, 60*time.Second, 2*time.Second).Should( gomega.Equal("PONG\n"), - "Port 9500 (onAutoForward=silent) should be forwarded", + "Port 9500 (onAutoForward=silent, label=TestService) should be forwarded", ) // Port 9501 should NOT be reachable locally (onAutoForward=ignore) @@ -127,4 +127,81 @@ var _ = ginkgo.Describe("devsy portsAttributes e2e", ) }, ) + + ginkgo.It( + "should forward port with notify policy and apply label metadata", + ginkgo.SpecTimeout(framework.TimeoutShort()), + func(ctx context.Context) { + if runtime.GOOS == "windows" { + ginkgo.Skip("skipping on windows") + } + + 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) + + // Port 9502 has otherPortsAttributes with onAutoForward=notify (should forward) + notifyPort := 9502 + + serverCtx, serverCancel := context.WithCancel(ctx) + defer serverCancel() + + workspaceName := filepath.Base(tempDir) + + // #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(notifyPort), + ) + 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(notifyPort), tempDir) + }) + + ginkgo.DeferCleanup(func() { + serverCancel() + cancelPort() + wg.Wait() + }) + + // Port 9502 falls through to otherPortsAttributes (onAutoForward=notify) + // and should be forwarded + address := net.JoinHostPort("localhost", strconv.Itoa(notifyPort)) + gomega.Eventually(func() string { + conn, err := net.DialTimeout("tcp", address, 3*time.Second) + if err == nil { + _ = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) + buf := make([]byte, 1024) + n, readErr := conn.Read(buf) + _ = conn.Close() + if readErr == nil && n > 0 { + return string(buf[:n]) + } + } + return "" + }, 60*time.Second, 2*time.Second).Should( + gomega.Equal("PONG\n"), + "Port 9502 (otherPortsAttributes onAutoForward=notify) should be forwarded", + ) + }, + ) }) diff --git a/pkg/agent/tunnelserver/tunnelserver.go b/pkg/agent/tunnelserver/tunnelserver.go index 39b24e9df..0f23511b2 100644 --- a/pkg/agent/tunnelserver/tunnelserver.go +++ b/pkg/agent/tunnelserver/tunnelserver.go @@ -160,7 +160,7 @@ func (t *tunnelServer) ForwardPort( return nil, fmt.Errorf("cannot forward ports") } - err := t.forwarder.Forward(portRequest.Port) + err := t.forwarder.Forward(portRequest.Port, netstat.PortForwardAttribute{}) if err != nil { return nil, fmt.Errorf("error forwarding port %s: %w", portRequest.Port, err) } diff --git a/pkg/netstat/watcher.go b/pkg/netstat/watcher.go index 957328ea7..c14c687d3 100644 --- a/pkg/netstat/watcher.go +++ b/pkg/netstat/watcher.go @@ -9,11 +9,25 @@ import ( "github.com/devsy-org/devsy/pkg/log" ) +const AutoForwardIgnore = "ignore" + +// 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 +} + type Forwarder interface { - Forward(port string) error + Forward(port string, attr PortForwardAttribute) error StopForward(port string) error } +// PortAttributeResolver resolves port forwarding attributes for a given port. +type PortAttributeResolver func(port string) PortForwardAttribute + // PortFilter decides whether a discovered port should be auto-forwarded. // Return true to forward, false to skip. type PortFilter func(port string) bool @@ -40,10 +54,18 @@ func WithPortFilter(f PortFilter) WatcherOption { return func(w *Watcher) { w.portFilter = f } } +// WithPortAttributes configures the watcher with a port attribute resolver. +// When set, the watcher resolves attributes for each discovered port and +// passes them to the Forwarder. Ports with onAutoForward=ignore are skipped. +func WithPortAttributes(resolver PortAttributeResolver) WatcherOption { + return func(w *Watcher) { w.attrResolver = resolver } +} + type Watcher struct { forwarder Forwarder forwardedPorts map[string]bool portFilter PortFilter + attrResolver PortAttributeResolver } func (w *Watcher) Run(ctx context.Context) error { @@ -84,8 +106,20 @@ func (w *Watcher) runOnce() error { log.Debugf("Skipping port %s (filtered)", port) continue } - log.Debugf("Found open port %s ready to forward", port) - err = w.forwarder.Forward(port) + + attr := w.resolveAttr(port) + if attr.OnAutoForward == AutoForwardIgnore { + log.Debugf("Skipping port %s (onAutoForward=ignore)", port) + 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) + } + + err = w.forwarder.Forward(port, attr) if err != nil { return fmt.Errorf("error forwarding port %s: %w", port, err) } @@ -96,6 +130,13 @@ func (w *Watcher) runOnce() error { return nil } +func (w *Watcher) resolveAttr(port string) PortForwardAttribute { + if w.attrResolver == nil { + return PortForwardAttribute{} + } + return w.attrResolver(port) +} + func (w *Watcher) findPorts() (map[string]bool, error) { tcpSocks, err := TCPSocks(func(s *SockTabEntry) bool { return s.State == Listen diff --git a/pkg/netstat/watcher_test.go b/pkg/netstat/watcher_test.go index 43ff0ac67..8e6ce5340 100644 --- a/pkg/netstat/watcher_test.go +++ b/pkg/netstat/watcher_test.go @@ -7,12 +7,14 @@ import ( ) type mockForwarder struct { - forwarded []string - stopped []string + forwarded []string + forwardedAttr []PortForwardAttribute + stopped []string } -func (m *mockForwarder) Forward(port string) error { +func (m *mockForwarder) Forward(port string, attr PortForwardAttribute) error { m.forwarded = append(m.forwarded, port) + m.forwardedAttr = append(m.forwardedAttr, attr) return nil } @@ -24,6 +26,7 @@ func (m *mockForwarder) StopForward(port string) error { func TestNewWatcher_NilFilter(t *testing.T) { w := NewWatcher(&mockForwarder{}) assert.Nil(t, w.portFilter) + assert.Nil(t, w.attrResolver) } func TestNewWatcher_WithPortFilter(t *testing.T) { @@ -32,19 +35,68 @@ func TestNewWatcher_WithPortFilter(t *testing.T) { assert.NotNil(t, w.portFilter) } +func TestNewWatcher_WithPortAttributes(t *testing.T) { + resolver := func(port string) PortForwardAttribute { + return PortForwardAttribute{Label: "test", Protocol: "https"} + } + w := NewWatcher(&mockForwarder{}, WithPortAttributes(resolver)) + assert.NotNil(t, w.attrResolver) +} + func TestWatcher_PortFilterSkipsIgnored(t *testing.T) { mf := &mockForwarder{} w := NewWatcher(mf, WithPortFilter(func(port string) bool { return port != "9090" })) - // Simulate discovered ports by injecting into forwardedPorts after - // a hand-crafted runOnce cycle. Since findPorts reads /proc we - // drive the filter path by directly manipulating state. w.forwardedPorts = map[string]bool{} - // We can't call runOnce (it reads /proc), so verify the filter - // is wired correctly by checking the contract. assert.False(t, w.portFilter("9090"), "filter should reject 9090") assert.True(t, w.portFilter("8080"), "filter should accept 8080") } + +func TestWatcher_ResolveAttr_NilResolver(t *testing.T) { + w := NewWatcher(&mockForwarder{}) + attr := w.resolveAttr("3000") + assert.Equal(t, PortForwardAttribute{}, attr) +} + +func TestWatcher_ResolveAttr_WithResolver(t *testing.T) { + resolver := func(port string) PortForwardAttribute { + if port == "3000" { + return PortForwardAttribute{ + Label: "Web App", + Protocol: "https", + OnAutoForward: "silent", + } + } + return PortForwardAttribute{OnAutoForward: AutoForwardIgnore} + } + w := NewWatcher(&mockForwarder{}, WithPortAttributes(resolver)) + + attr := w.resolveAttr("3000") + assert.Equal(t, "Web App", attr.Label) + assert.Equal(t, "https", attr.Protocol) + assert.Equal(t, "silent", attr.OnAutoForward) + + attr = w.resolveAttr("9999") + assert.Equal(t, "ignore", attr.OnAutoForward) +} + +func TestWatcher_PortAttributes_IgnoreSkipsForward(t *testing.T) { + resolver := func(port string) PortForwardAttribute { + if port == "9501" { + return PortForwardAttribute{OnAutoForward: AutoForwardIgnore} + } + return PortForwardAttribute{OnAutoForward: "silent", Label: "Allowed"} + } + w := NewWatcher(&mockForwarder{}, WithPortAttributes(resolver)) + + // Verify that ignore causes skip in the resolver logic + attr := w.resolveAttr("9501") + assert.Equal(t, "ignore", attr.OnAutoForward) + + attr = w.resolveAttr("9500") + assert.Equal(t, "silent", attr.OnAutoForward) + assert.Equal(t, "Allowed", attr.Label) +} diff --git a/pkg/tunnel/forwarder.go b/pkg/tunnel/forwarder.go index d5edc62ea..56e5f23db 100644 --- a/pkg/tunnel/forwarder.go +++ b/pkg/tunnel/forwarder.go @@ -45,7 +45,7 @@ type forwarder struct { } // Forward opens an SSH channel in the existing connection with channel type "direct-tcpip" to forward the local port. -func (f *forwarder) Forward(port string) error { +func (f *forwarder) Forward(port string, _ netstat.PortForwardAttribute) error { f.Lock() defer f.Unlock()