From 6fb07db4b2309691ade28f01c4b2fb1ce3975148 Mon Sep 17 00:00:00 2001 From: Pasha Sviderski Date: Mon, 21 Jul 2025 16:56:52 +1000 Subject: [PATCH] feat: follow-up compose 'ports' support: use ingress mode by default (closes #81) --- Makefile | 2 +- pkg/api/port.go | 19 +----------------- pkg/client/compose/port.go | 29 +++++++++++++++++++++------- pkg/client/compose/port_test.go | 34 ++++++++++++++++++++------------- 4 files changed, 45 insertions(+), 39 deletions(-) diff --git a/Makefile b/Makefile index 99ff1318..f12e9788 100644 --- a/Makefile +++ b/Makefile @@ -102,5 +102,5 @@ _lint: golangci-lint run $(ARGS) .PHONY: docs-image-push -docs-image: +docs-image-push: docker buildx build --push --platform linux/amd64,linux/arm64 -t "$(DOCS_IMAGE)" ./docs diff --git a/pkg/api/port.go b/pkg/api/port.go index 8fb7fd2f..b552b88e 100644 --- a/pkg/api/port.go +++ b/pkg/api/port.go @@ -34,23 +34,6 @@ type PortSpec struct { Mode string } -func (p *PortSpec) isHTTP() bool { - return p.Protocol == ProtocolHTTP || p.Protocol == ProtocolHTTPS -} - -// AdjustUncloudMode makes adjustments for uncloud compatibility -func (p *PortSpec) AdjustUncloudMode() { - if p.Protocol == "" { - p.Protocol = "tcp" - } - if p.Mode == "" { - p.Mode = PortModeIngress - } - if p.Mode == PortModeIngress && !p.isHTTP() && p.PublishedPort != 0 { - p.Mode = PortModeHost - } -} - func (p *PortSpec) Validate() error { if p.ContainerPort == 0 { return fmt.Errorf("container port must be non-zero") @@ -73,7 +56,7 @@ func (p *PortSpec) Validate() error { return fmt.Errorf("host IP cannot be specified in %s mode", PortModeIngress) } if p.Hostname != "" { - if !p.isHTTP() { + if p.Protocol != ProtocolHTTP && p.Protocol != ProtocolHTTPS { return fmt.Errorf("hostname is only valid with '%s' or '%s' protocols", ProtocolHTTP, ProtocolHTTPS) } if err := validateHostname(p.Hostname); err != nil { diff --git a/pkg/client/compose/port.go b/pkg/client/compose/port.go index 2008b7fb..dc998058 100644 --- a/pkg/client/compose/port.go +++ b/pkg/client/compose/port.go @@ -2,10 +2,12 @@ package compose import ( "fmt" - "github.com/compose-spec/compose-go/v2/types" - "github.com/psviderski/uncloud/pkg/api" "net/netip" "strconv" + "strings" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/psviderski/uncloud/pkg/api" ) const PortsExtensionKey = "x-ports" @@ -20,7 +22,8 @@ func transformServicesPortsExtension(project *types.Project) (*types.Project, er hasXPorts := service.Extensions[PortsExtensionKey] != nil if hasStandardPorts && hasXPorts { - return service, fmt.Errorf("service %q cannot specify both 'ports' and 'x-ports' directives, use only one", name) + return service, fmt.Errorf("service %q cannot specify both 'ports' and 'x-ports' directives, use only one", + name) } var ( @@ -32,7 +35,7 @@ func transformServicesPortsExtension(project *types.Project) (*types.Project, er // Convert standard ports directly to api.PortSpec specs, err = convertStandardPortsToPortSpecs(service.Ports) if err != nil { - return service, fmt.Errorf("convert standard ports for service %q: %w", name, err) + return service, fmt.Errorf("convert standard 'ports' for service '%s': %w", name, err) } } else if hasXPorts { // Use existing x-ports string-based processing for backward compatibility @@ -82,8 +85,23 @@ func convertServicePortConfigToPortSpec(port types.ServicePortConfig) (api.PortS Protocol: port.Protocol, Mode: port.Mode, } + // Compose parser sets the default protocol to "tcp" and mode to "ingress". We still explicitly set these values + // to avoid relying on implicit behavior and improve code robustness. + if spec.Protocol == "" { + spec.Protocol = api.ProtocolTCP + } + if spec.Mode == "" { + spec.Mode = api.PortModeIngress + } + // Set published port if specified if port.Published != "" { + if strings.Contains(port.Published, "-") { + // 'a-b:x' format is not automatically expanded by the compose parser and our PortSpec does not support port + // ranges for now. + return spec, fmt.Errorf("port range '%s' for published port is not supported, use a single port", + port.Published) + } publishedPort, err := strconv.ParseUint(port.Published, 10, 16) if err != nil { return spec, fmt.Errorf("invalid published port %q: %w", port.Published, err) @@ -100,9 +118,6 @@ func convertServicePortConfigToPortSpec(port types.ServicePortConfig) (api.PortS spec.HostIP = hostIP } - // Apply defaults according to uncloud - spec.AdjustUncloudMode() - // Validate the resulting spec if err := spec.Validate(); err != nil { return spec, fmt.Errorf("invalid port configuration: %w", err) diff --git a/pkg/client/compose/port_test.go b/pkg/client/compose/port_test.go index 49738e91..5712a8d0 100644 --- a/pkg/client/compose/port_test.go +++ b/pkg/client/compose/port_test.go @@ -22,14 +22,16 @@ func TestConvertStandardPortsToPortSpecs(t *testing.T) { { name: "multiple ports", ports: []types.ServicePortConfig{ - {Target: 8080, Published: "80", Protocol: "tcp"}, + {Target: 8080, Published: "80", Protocol: "tcp", Mode: "ingress"}, {Target: 8443, Published: "443", Protocol: "tcp", Mode: "host"}, {Target: 5353, Published: "53", Protocol: "udp"}, + {Target: 2222, Published: "22"}, }, expected: []api.PortSpec{ - {ContainerPort: 8080, PublishedPort: 80, Protocol: "tcp", Mode: "host"}, + {ContainerPort: 8080, PublishedPort: 80, Protocol: "tcp", Mode: "ingress"}, {ContainerPort: 8443, PublishedPort: 443, Protocol: "tcp", Mode: "host"}, - {ContainerPort: 5353, PublishedPort: 53, Protocol: "udp", Mode: "host"}, + {ContainerPort: 5353, PublishedPort: 53, Protocol: "udp", Mode: "ingress"}, + {ContainerPort: 2222, PublishedPort: 22, Protocol: "tcp", Mode: "ingress"}, }, }, { @@ -103,7 +105,7 @@ func TestConvertServicePortConfigToPortSpec(t *testing.T) { ContainerPort: 8080, PublishedPort: 80, Protocol: "tcp", - Mode: "host", + Mode: "ingress", }, }, { @@ -146,7 +148,7 @@ func TestConvertServicePortConfigToPortSpec(t *testing.T) { ContainerPort: 5353, PublishedPort: 53, Protocol: "udp", - Mode: "host", + Mode: "ingress", }, }, { @@ -167,7 +169,7 @@ func TestConvertServicePortConfigToPortSpec(t *testing.T) { }, }, { - name: "HTTP protocol stays in ingress mode", + name: "HTTP protocol", port: types.ServicePortConfig{ Target: 8080, Published: "80", @@ -181,7 +183,7 @@ func TestConvertServicePortConfigToPortSpec(t *testing.T) { }, }, { - name: "HTTPS protocol stays in ingress mode", + name: "HTTPS protocol", port: types.ServicePortConfig{ Target: 8080, Published: "443", @@ -219,6 +221,13 @@ func TestConvertServicePortConfigToPortSpec(t *testing.T) { }, wantErr: "container port must be non-zero", }, + { + name: "missing container port", + port: types.ServicePortConfig{ + Published: "8000-9000", + }, + wantErr: "port range '8000-9000' for published port is not supported", + }, } for _, tt := range tests { @@ -227,8 +236,7 @@ func TestConvertServicePortConfigToPortSpec(t *testing.T) { result, err := convertServicePortConfigToPortSpec(tt.port) if tt.wantErr != "" { - require.Error(t, err) - assert.Contains(t, err.Error(), tt.wantErr) + assert.ErrorContains(t, err, tt.wantErr) return } @@ -335,9 +343,9 @@ services: - "53:5353/udp" `, expected: []api.PortSpec{ - {ContainerPort: 8080, PublishedPort: 80, Protocol: "tcp", Mode: "host"}, - {ContainerPort: 8443, PublishedPort: 443, Protocol: "tcp", Mode: "host"}, - {ContainerPort: 5353, PublishedPort: 53, Protocol: "udp", Mode: "host"}, + {ContainerPort: 8080, PublishedPort: 80, Protocol: "tcp", Mode: "ingress"}, + {ContainerPort: 8443, PublishedPort: 443, Protocol: "tcp", Mode: "ingress"}, + {ContainerPort: 5353, PublishedPort: 53, Protocol: "udp", Mode: "ingress"}, }, }, { @@ -357,7 +365,7 @@ services: mode: host `, expected: []api.PortSpec{ - {ContainerPort: 8080, PublishedPort: 80, Protocol: "tcp", Mode: "host"}, + {ContainerPort: 8080, PublishedPort: 80, Protocol: "tcp", Mode: "ingress"}, {ContainerPort: 8443, PublishedPort: 443, Protocol: "tcp", Mode: "host"}, }, },