From c9e9df094479c6fffe747db63a2aee9abf7da52b Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 2 May 2026 23:07:08 -0500 Subject: [PATCH 1/3] feat(config): support forwardPorts range syntax expansion Parse port range notation like '3000-3005' in forwardPorts and expand to individual ports [3000, 3001, ..., 3005]. Mixed ranges and single ports work together; invalid ranges are rejected with clear errors. --- .../readconfiguration/readconfiguration.go | 42 +++++ .../.devcontainer/devcontainer.json | 5 + pkg/devcontainer/config/merge.go | 60 ++++++- pkg/devcontainer/config/merge_test.go | 154 ++++++++++++++++++ 4 files changed, 252 insertions(+), 9 deletions(-) create mode 100644 e2e/tests/readconfiguration/testdata-port-range/.devcontainer/devcontainer.json diff --git a/e2e/tests/readconfiguration/readconfiguration.go b/e2e/tests/readconfiguration/readconfiguration.go index 52ebb0b55..160477969 100644 --- a/e2e/tests/readconfiguration/readconfiguration.go +++ b/e2e/tests/readconfiguration/readconfiguration.go @@ -137,4 +137,46 @@ var _ = ginkgo.Describe("read-configuration command", ginkgo.Label("read-configu }) framework.ExpectError(err) }, ginkgo.SpecTimeout(framework.TimeoutShort())) + + ginkgo.It("expands forwardPorts range syntax in merged configuration", + func(ctx context.Context) { + f := framework.NewDefaultFramework(initialDir + "/bin") + tempDir, err := framework.CopyToTempDirWithoutChdir( + "tests/readconfiguration/testdata-port-range", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(func() { _ = os.RemoveAll(tempDir) }) + + stdout, _, err := f.ExecCommandCapture(ctx, []string{ + "read-configuration", + "--workspace-folder", tempDir, + "--include-merged-configuration", + }) + framework.ExpectNoError(err) + + var result map[string]any + err = json.Unmarshal([]byte(stdout), &result) + framework.ExpectNoError(err) + + merged, ok := result["mergedConfiguration"].(map[string]any) + gomega.Expect(ok).To(gomega.BeTrue()) + + portsRaw, ok := merged["forwardPorts"].([]any) + gomega.Expect(ok).To( + gomega.BeTrue(), + "forwardPorts should be an array", + ) + + var ports []string + for _, p := range portsRaw { + s, ok := p.(string) + gomega.Expect(ok).To(gomega.BeTrue()) + ports = append(ports, s) + } + + gomega.Expect(ports).To(gomega.ContainElement("8080")) + gomega.Expect(ports).To(gomega.ContainElement("3000")) + gomega.Expect(ports).To(gomega.ContainElement("3005")) + gomega.Expect(ports).To(gomega.HaveLen(7)) + }, ginkgo.SpecTimeout(framework.TimeoutShort())) }) diff --git a/e2e/tests/readconfiguration/testdata-port-range/.devcontainer/devcontainer.json b/e2e/tests/readconfiguration/testdata-port-range/.devcontainer/devcontainer.json new file mode 100644 index 000000000..d89862421 --- /dev/null +++ b/e2e/tests/readconfiguration/testdata-port-range/.devcontainer/devcontainer.json @@ -0,0 +1,5 @@ +{ + "name": "Port Range Test", + "image": "mcr.microsoft.com/devcontainers/base:ubuntu", + "forwardPorts": [8080, "3000-3005"] +} diff --git a/pkg/devcontainer/config/merge.go b/pkg/devcontainer/config/merge.go index 572b0a170..f9a585c5d 100644 --- a/pkg/devcontainer/config/merge.go +++ b/pkg/devcontainer/config/merge.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "maps" "strconv" "strings" @@ -253,22 +254,63 @@ func mergeGPU(a, b *GPURequirement) *GPURequirement { return a } +func expandPortRange(port string) ([]string, error) { + if strings.Contains(port, ":") { + return []string{port}, nil + } + + startStr, endStr, hasRange := strings.Cut(port, "-") + if !hasRange { + if _, err := strconv.Atoi(port); err != nil { + return nil, fmt.Errorf("invalid port %q: %w", port, err) + } + return []string{port}, nil + } + + start, err := strconv.Atoi(startStr) + if err != nil { + return nil, fmt.Errorf("invalid range start in %q: %w", port, err) + } + end, err := strconv.Atoi(endStr) + if err != nil { + return nil, fmt.Errorf("invalid range end in %q: %w", port, err) + } + if start < 0 || end < 0 { + return nil, fmt.Errorf("negative port in range %q", port) + } + if start > end { + return nil, fmt.Errorf("invalid port range %q: start (%d) > end (%d)", port, start, end) + } + + ports := make([]string, 0, end-start+1) + for p := start; p <= end; p++ { + ports = append(ports, strconv.Itoa(p)) + } + return ports, nil +} + func mergeForwardPorts(entries []*ImageMetadata) types.StrIntArray { portMap := map[string]bool{} var retPorts types.StrIntArray for _, entry := range entries { for _, port := range entry.ForwardPorts { - portString := port - _, err := strconv.Atoi(portString) - if err == nil { - portString = "localhost:" + portString - } - if portMap[portString] { + expanded, err := expandPortRange(port) + if err != nil { continue } - - portMap[portString] = true - retPorts = append(retPorts, port) + for _, p := range expanded { + portString := p + _, err := strconv.Atoi(portString) + if err == nil { + portString = "localhost:" + portString + } + if portMap[portString] { + continue + } + + portMap[portString] = true + retPorts = append(retPorts, p) + } } } diff --git a/pkg/devcontainer/config/merge_test.go b/pkg/devcontainer/config/merge_test.go index 6e50e33b4..cf35f18d2 100644 --- a/pkg/devcontainer/config/merge_test.go +++ b/pkg/devcontainer/config/merge_test.go @@ -224,3 +224,157 @@ func TestMaxByteString(t *testing.T) { }) } } + +func TestExpandPortRange(t *testing.T) { + tests := []struct { + name string + input string + want []string + wantErr bool + }{ + { + name: "single port", + input: "8080", + want: []string{"8080"}, + }, + { + name: "host:port passthrough", + input: "localhost:3000", + want: []string{"localhost:3000"}, + }, + { + name: "range expands", + input: "3000-3005", + want: []string{"3000", "3001", "3002", "3003", "3004", "3005"}, + }, + { + name: "single element range", + input: "8080-8080", + want: []string{"8080"}, + }, + { + name: "start greater than end", + input: "3005-3000", + wantErr: true, + }, + { + name: "negative start", + input: "-1-3000", + wantErr: true, + }, + { + name: "non-numeric start", + input: "abc-3000", + wantErr: true, + }, + { + name: "non-numeric end", + input: "3000-xyz", + wantErr: true, + }, + { + name: "non-numeric single port", + input: "abc", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := expandPortRange(tt.input) + if tt.wantErr { + if err == nil { + t.Errorf("expandPortRange(%q) expected error, got %v", tt.input, got) + } + return + } + if err != nil { + t.Fatalf("expandPortRange(%q) unexpected error: %v", tt.input, err) + } + if len(got) != len(tt.want) { + t.Fatalf("expandPortRange(%q) = %v (len %d), want %v (len %d)", + tt.input, got, len(got), tt.want, len(tt.want)) + } + for i := range got { + if got[i] != tt.want[i] { + t.Errorf("expandPortRange(%q)[%d] = %q, want %q", + tt.input, i, got[i], tt.want[i]) + } + } + }) + } +} + +func TestMergeForwardPorts_RangeExpansion(t *testing.T) { + entries := []*ImageMetadata{ + {DevContainerConfigBase: DevContainerConfigBase{ + ForwardPorts: []string{"8080", "3000-3002"}, + }}, + } + got := mergeForwardPorts(entries) + want := []string{"8080", "3000", "3001", "3002"} + if len(got) != len(want) { + t.Fatalf("mergeForwardPorts = %v, want %v", got, want) + } + for i := range got { + if got[i] != want[i] { + t.Errorf("mergeForwardPorts[%d] = %q, want %q", i, got[i], want[i]) + } + } +} + +func TestMergeForwardPorts_MixedRangesAndSinglePorts(t *testing.T) { + entries := []*ImageMetadata{ + {DevContainerConfigBase: DevContainerConfigBase{ + ForwardPorts: []string{"8080", "3000-3002", "localhost:9090"}, + }}, + } + got := mergeForwardPorts(entries) + want := []string{"8080", "3000", "3001", "3002", "localhost:9090"} + if len(got) != len(want) { + t.Fatalf("mergeForwardPorts = %v, want %v", got, want) + } + for i := range got { + if got[i] != want[i] { + t.Errorf("mergeForwardPorts[%d] = %q, want %q", i, got[i], want[i]) + } + } +} + +func TestMergeForwardPorts_DeduplicatesAcrossRanges(t *testing.T) { + entries := []*ImageMetadata{ + {DevContainerConfigBase: DevContainerConfigBase{ + ForwardPorts: []string{"3000-3002"}, + }}, + {DevContainerConfigBase: DevContainerConfigBase{ + ForwardPorts: []string{"3001-3003"}, + }}, + } + got := mergeForwardPorts(entries) + want := []string{"3000", "3001", "3002", "3003"} + if len(got) != len(want) { + t.Fatalf("mergeForwardPorts = %v, want %v", got, want) + } + for i := range got { + if got[i] != want[i] { + t.Errorf("mergeForwardPorts[%d] = %q, want %q", i, got[i], want[i]) + } + } +} + +func TestMergeForwardPorts_InvalidRangeSkipped(t *testing.T) { + entries := []*ImageMetadata{ + {DevContainerConfigBase: DevContainerConfigBase{ + ForwardPorts: []string{"8080", "5000-4000", "9090"}, + }}, + } + got := mergeForwardPorts(entries) + want := []string{"8080", "9090"} + if len(got) != len(want) { + t.Fatalf("mergeForwardPorts = %v, want %v", got, want) + } + for i := range got { + if got[i] != want[i] { + t.Errorf("mergeForwardPorts[%d] = %q, want %q", i, got[i], want[i]) + } + } +} From f0c20f288803c1d9d0eba8086c796e525ef0f8a6 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 2 May 2026 23:16:30 -0500 Subject: [PATCH 2/3] fix(config): resolve lint issues in forwardPorts range expansion Extract parsePortRange helper to reduce expandPortRange cyclomatic complexity (cyclop), compact TestExpandPortRange with slices.Equal and inline test cases (funlen), fix continuation-line indentation (gci/gofumpt), and extract repeated "3000-3002" to testPortRange const (goconst). --- pkg/devcontainer/config/merge.go | 37 +++++++++----- pkg/devcontainer/config/merge_test.go | 74 +++++++-------------------- 2 files changed, 42 insertions(+), 69 deletions(-) diff --git a/pkg/devcontainer/config/merge.go b/pkg/devcontainer/config/merge.go index f9a585c5d..9e04da23c 100644 --- a/pkg/devcontainer/config/merge.go +++ b/pkg/devcontainer/config/merge.go @@ -254,32 +254,41 @@ func mergeGPU(a, b *GPURequirement) *GPURequirement { return a } +func parsePortRange(port string) (int, int, error) { + startStr, endStr, _ := strings.Cut(port, "-") + + start, err := strconv.Atoi(startStr) + if err != nil { + return 0, 0, fmt.Errorf("invalid range start in %q: %w", port, err) + } + end, err := strconv.Atoi(endStr) + if err != nil { + return 0, 0, fmt.Errorf("invalid range end in %q: %w", port, err) + } + if start < 0 || end < 0 { + return 0, 0, fmt.Errorf("negative port in range %q", port) + } + if start > end { + return 0, 0, fmt.Errorf("invalid port range %q: start (%d) > end (%d)", port, start, end) + } + return start, end, nil +} + func expandPortRange(port string) ([]string, error) { if strings.Contains(port, ":") { return []string{port}, nil } - startStr, endStr, hasRange := strings.Cut(port, "-") - if !hasRange { + if !strings.Contains(port, "-") { if _, err := strconv.Atoi(port); err != nil { return nil, fmt.Errorf("invalid port %q: %w", port, err) } return []string{port}, nil } - start, err := strconv.Atoi(startStr) - if err != nil { - return nil, fmt.Errorf("invalid range start in %q: %w", port, err) - } - end, err := strconv.Atoi(endStr) + start, end, err := parsePortRange(port) if err != nil { - return nil, fmt.Errorf("invalid range end in %q: %w", port, err) - } - if start < 0 || end < 0 { - return nil, fmt.Errorf("negative port in range %q", port) - } - if start > end { - return nil, fmt.Errorf("invalid port range %q: start (%d) > end (%d)", port, start, end) + return nil, err } ports := make([]string, 0, end-start+1) diff --git a/pkg/devcontainer/config/merge_test.go b/pkg/devcontainer/config/merge_test.go index cf35f18d2..95038a77e 100644 --- a/pkg/devcontainer/config/merge_test.go +++ b/pkg/devcontainer/config/merge_test.go @@ -1,9 +1,12 @@ package config import ( + "slices" "testing" ) +const testPortRange = "3000-3002" + func gpu(val string) *GPURequirement { return &GPURequirement{Value: val} } @@ -232,51 +235,19 @@ func TestExpandPortRange(t *testing.T) { want []string wantErr bool }{ + {"single port", "8080", []string{"8080"}, false}, + {"host:port passthrough", "localhost:3000", []string{"localhost:3000"}, false}, { - name: "single port", - input: "8080", - want: []string{"8080"}, - }, - { - name: "host:port passthrough", - input: "localhost:3000", - want: []string{"localhost:3000"}, - }, - { - name: "range expands", - input: "3000-3005", - want: []string{"3000", "3001", "3002", "3003", "3004", "3005"}, - }, - { - name: "single element range", - input: "8080-8080", - want: []string{"8080"}, - }, - { - name: "start greater than end", - input: "3005-3000", - wantErr: true, - }, - { - name: "negative start", - input: "-1-3000", - wantErr: true, - }, - { - name: "non-numeric start", - input: "abc-3000", - wantErr: true, - }, - { - name: "non-numeric end", - input: "3000-xyz", - wantErr: true, - }, - { - name: "non-numeric single port", - input: "abc", - wantErr: true, + "range expands", "3000-3005", + []string{"3000", "3001", "3002", "3003", "3004", "3005"}, + false, }, + {"single element range", "8080-8080", []string{"8080"}, false}, + {"start greater than end", "3005-3000", nil, true}, + {"negative start", "-1-3000", nil, true}, + {"non-numeric start", "abc-3000", nil, true}, + {"non-numeric end", "3000-xyz", nil, true}, + {"non-numeric single port", "abc", nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -290,15 +261,8 @@ func TestExpandPortRange(t *testing.T) { if err != nil { t.Fatalf("expandPortRange(%q) unexpected error: %v", tt.input, err) } - if len(got) != len(tt.want) { - t.Fatalf("expandPortRange(%q) = %v (len %d), want %v (len %d)", - tt.input, got, len(got), tt.want, len(tt.want)) - } - for i := range got { - if got[i] != tt.want[i] { - t.Errorf("expandPortRange(%q)[%d] = %q, want %q", - tt.input, i, got[i], tt.want[i]) - } + if !slices.Equal(got, tt.want) { + t.Errorf("expandPortRange(%q) = %v, want %v", tt.input, got, tt.want) } }) } @@ -307,7 +271,7 @@ func TestExpandPortRange(t *testing.T) { func TestMergeForwardPorts_RangeExpansion(t *testing.T) { entries := []*ImageMetadata{ {DevContainerConfigBase: DevContainerConfigBase{ - ForwardPorts: []string{"8080", "3000-3002"}, + ForwardPorts: []string{"8080", testPortRange}, }}, } got := mergeForwardPorts(entries) @@ -325,7 +289,7 @@ func TestMergeForwardPorts_RangeExpansion(t *testing.T) { func TestMergeForwardPorts_MixedRangesAndSinglePorts(t *testing.T) { entries := []*ImageMetadata{ {DevContainerConfigBase: DevContainerConfigBase{ - ForwardPorts: []string{"8080", "3000-3002", "localhost:9090"}, + ForwardPorts: []string{"8080", testPortRange, "localhost:9090"}, }}, } got := mergeForwardPorts(entries) @@ -343,7 +307,7 @@ func TestMergeForwardPorts_MixedRangesAndSinglePorts(t *testing.T) { func TestMergeForwardPorts_DeduplicatesAcrossRanges(t *testing.T) { entries := []*ImageMetadata{ {DevContainerConfigBase: DevContainerConfigBase{ - ForwardPorts: []string{"3000-3002"}, + ForwardPorts: []string{testPortRange}, }}, {DevContainerConfigBase: DevContainerConfigBase{ ForwardPorts: []string{"3001-3003"}, From a064e1252a46b78e9021f73747b45f6b1729e7a6 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 2 May 2026 23:46:11 -0500 Subject: [PATCH 3/3] fix(e2e): wire readconfiguration test into E2E suite --- e2e/e2e_suite_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/e2e/e2e_suite_test.go b/e2e/e2e_suite_test.go index bcf7e5698..1745df044 100644 --- a/e2e/e2e_suite_test.go +++ b/e2e/e2e_suite_test.go @@ -19,6 +19,7 @@ import ( _ "github.com/devsy-org/devsy/e2e/tests/machine" _ "github.com/devsy-org/devsy/e2e/tests/machineprovider" _ "github.com/devsy-org/devsy/e2e/tests/provider" + _ "github.com/devsy-org/devsy/e2e/tests/readconfiguration" _ "github.com/devsy-org/devsy/e2e/tests/ssh" _ "github.com/devsy-org/devsy/e2e/tests/tunnel" _ "github.com/devsy-org/devsy/e2e/tests/up"