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" 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..9e04da23c 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,72 @@ 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 + } + + 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, end, err := parsePortRange(port) + if err != nil { + return nil, err + } + + 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..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} } @@ -224,3 +227,118 @@ func TestMaxByteString(t *testing.T) { }) } } + +func TestExpandPortRange(t *testing.T) { + tests := []struct { + name string + input string + want []string + wantErr bool + }{ + {"single port", "8080", []string{"8080"}, false}, + {"host:port passthrough", "localhost:3000", []string{"localhost:3000"}, false}, + { + "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) { + 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 !slices.Equal(got, tt.want) { + t.Errorf("expandPortRange(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + +func TestMergeForwardPorts_RangeExpansion(t *testing.T) { + entries := []*ImageMetadata{ + {DevContainerConfigBase: DevContainerConfigBase{ + ForwardPorts: []string{"8080", testPortRange}, + }}, + } + 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", testPortRange, "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{testPortRange}, + }}, + {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]) + } + } +}