From fea7edcbc5858c60458a5c2fbf52eaa197ab84e4 Mon Sep 17 00:00:00 2001 From: Evgenii Orlov <4623930+OrlovEvgeny@users.noreply.github.com> Date: Mon, 21 Jul 2025 07:24:37 +0200 Subject: [PATCH] feat: add support for standard compose ports directive (#95) --- pkg/api/port.go | 19 +- pkg/client/compose/port.go | 100 +++++++- pkg/client/compose/port_test.go | 408 ++++++++++++++++++++++++++++++++ 3 files changed, 518 insertions(+), 9 deletions(-) create mode 100644 pkg/client/compose/port_test.go diff --git a/pkg/api/port.go b/pkg/api/port.go index b552b88e..8fb7fd2f 100644 --- a/pkg/api/port.go +++ b/pkg/api/port.go @@ -34,6 +34,23 @@ 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") @@ -56,7 +73,7 @@ func (p *PortSpec) Validate() error { return fmt.Errorf("host IP cannot be specified in %s mode", PortModeIngress) } if p.Hostname != "" { - if p.Protocol != ProtocolHTTP && p.Protocol != ProtocolHTTPS { + if !p.isHTTP() { 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 79af6065..2008b7fb 100644 --- a/pkg/client/compose/port.go +++ b/pkg/client/compose/port.go @@ -4,26 +4,59 @@ import ( "fmt" "github.com/compose-spec/compose-go/v2/types" "github.com/psviderski/uncloud/pkg/api" + "net/netip" + "strconv" ) const PortsExtensionKey = "x-ports" type PortsSource []string -// TransformServicesPortsExtension transforms the ports extension of all services in the project by replacing a string -// representation of each port with a parsed PortSpec. +// transformServicesPortsExtension transforms both standard 'ports' and 'x-ports' to PortSpecs. func transformServicesPortsExtension(project *types.Project) (*types.Project, error) { return project.WithServicesTransform(func(name string, service types.ServiceConfig) (types.ServiceConfig, error) { - ports, ok := service.Extensions[PortsExtensionKey].(PortsSource) - if !ok { + // Check for mutual exclusivity + hasStandardPorts := len(service.Ports) > 0 + 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) + } + + var ( + specs []api.PortSpec + err error + ) + + if hasStandardPorts { + // 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) + } + } else if hasXPorts { + // Use existing x-ports string-based processing for backward compatibility + var portsSource PortsSource + var ok bool + portsSource, ok = service.Extensions[PortsExtensionKey].(PortsSource) + if !ok { + return service, nil + } + + // Parse the port strings using existing logic + specs, err = transformPortsExtension(portsSource) + if err != nil { + return service, err + } + } else { + // No ports specified return service, nil } - specs, err := transformPortsExtension(ports) - if err != nil { - return service, err + // Ensure extensions map exists before setting the port specs + if service.Extensions == nil { + service.Extensions = make(types.Extensions) } - service.Extensions[PortsExtensionKey] = specs return service, nil }) @@ -41,3 +74,54 @@ func transformPortsExtension(ports PortsSource) ([]api.PortSpec, error) { return specs, nil } + +// convertServicePortConfigToPortSpec converts types.ServicePortConfig directly to api.PortSpec +func convertServicePortConfigToPortSpec(port types.ServicePortConfig) (api.PortSpec, error) { + spec := api.PortSpec{ + ContainerPort: uint16(port.Target), + Protocol: port.Protocol, + Mode: port.Mode, + } + // Set published port if specified + if 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) + } + spec.PublishedPort = uint16(publishedPort) + } + + // Set host IP if specified + if port.HostIP != "" { + hostIP, err := netip.ParseAddr(port.HostIP) + if err != nil { + return spec, fmt.Errorf("invalid host IP %q: %w", port.HostIP, err) + } + 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) + } + + return spec, nil +} + +// convertStandardPortsToPortSpecs converts []types.ServicePortConfig directly to api.PortSpecs. +func convertStandardPortsToPortSpecs(ports []types.ServicePortConfig) ([]api.PortSpec, error) { + var specs = make([]api.PortSpec, 0, len(ports)) + + for _, port := range ports { + spec, err := convertServicePortConfigToPortSpec(port) + if err != nil { + return nil, err + } + specs = append(specs, spec) + } + + return specs, nil +} diff --git a/pkg/client/compose/port_test.go b/pkg/client/compose/port_test.go new file mode 100644 index 00000000..49738e91 --- /dev/null +++ b/pkg/client/compose/port_test.go @@ -0,0 +1,408 @@ +package compose + +import ( + "net/netip" + "testing" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/psviderski/uncloud/pkg/api" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConvertStandardPortsToPortSpecs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + ports []types.ServicePortConfig + expected []api.PortSpec + wantErr string + }{ + { + name: "multiple ports", + ports: []types.ServicePortConfig{ + {Target: 8080, Published: "80", Protocol: "tcp"}, + {Target: 8443, Published: "443", Protocol: "tcp", Mode: "host"}, + {Target: 5353, Published: "53", Protocol: "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"}, + }, + }, + { + name: "empty ports", + ports: []types.ServicePortConfig{}, + expected: make([]api.PortSpec, 0), + }, + { + name: "single port no published", + ports: []types.ServicePortConfig{ + {Target: 8080}, + }, + expected: []api.PortSpec{ + {ContainerPort: 8080, PublishedPort: 0, Protocol: "tcp", Mode: "ingress"}, + }, + }, + { + name: "IPv6 host IP", + ports: []types.ServicePortConfig{ + {Target: 8080, Published: "80", Protocol: "tcp", HostIP: "::1", Mode: "host"}, + }, + expected: []api.PortSpec{ + {ContainerPort: 8080, PublishedPort: 80, Protocol: "tcp", Mode: "host", HostIP: mustParseAddr("::1")}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + result, err := convertStandardPortsToPortSpecs(tt.ports) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } +} + +// mustParseAddr is a helper function for tests +func mustParseAddr(s string) netip.Addr { + addr, err := netip.ParseAddr(s) + if err != nil { + panic(err) + } + return addr +} + +func TestConvertServicePortConfigToPortSpec(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + port types.ServicePortConfig + expected api.PortSpec + wantErr string + }{ + { + name: "basic port", + port: types.ServicePortConfig{ + Target: 8080, + Published: "80", + Protocol: "tcp", + }, + expected: api.PortSpec{ + ContainerPort: 8080, + PublishedPort: 80, + Protocol: "tcp", + Mode: "host", + }, + }, + { + name: "port with defaults", + port: types.ServicePortConfig{ + Target: 8080, + }, + expected: api.PortSpec{ + ContainerPort: 8080, + PublishedPort: 0, + Protocol: "tcp", + Mode: "ingress", + }, + }, + { + name: "host mode with IP", + port: types.ServicePortConfig{ + Target: 8080, + Published: "80", + Protocol: "tcp", + Mode: "host", + HostIP: "127.0.0.1", + }, + expected: api.PortSpec{ + ContainerPort: 8080, + PublishedPort: 80, + Protocol: "tcp", + Mode: "host", + HostIP: mustParseAddr("127.0.0.1"), + }, + }, + { + name: "UDP protocol", + port: types.ServicePortConfig{ + Target: 5353, + Published: "53", + Protocol: "udp", + }, + expected: api.PortSpec{ + ContainerPort: 5353, + PublishedPort: 53, + Protocol: "udp", + Mode: "host", + }, + }, + { + name: "IPv6 host IP", + port: types.ServicePortConfig{ + Target: 8080, + Published: "80", + Protocol: "tcp", + Mode: "host", + HostIP: "::1", + }, + expected: api.PortSpec{ + ContainerPort: 8080, + PublishedPort: 80, + Protocol: "tcp", + Mode: "host", + HostIP: mustParseAddr("::1"), + }, + }, + { + name: "HTTP protocol stays in ingress mode", + port: types.ServicePortConfig{ + Target: 8080, + Published: "80", + Protocol: "http", + }, + expected: api.PortSpec{ + ContainerPort: 8080, + PublishedPort: 80, + Protocol: "http", + Mode: "ingress", + }, + }, + { + name: "HTTPS protocol stays in ingress mode", + port: types.ServicePortConfig{ + Target: 8080, + Published: "443", + Protocol: "https", + }, + expected: api.PortSpec{ + ContainerPort: 8080, + PublishedPort: 443, + Protocol: "https", + Mode: "ingress", + }, + }, + // Error cases + { + name: "invalid published port", + port: types.ServicePortConfig{ + Target: 8080, + Published: "invalid", + }, + wantErr: "invalid published port", + }, + { + name: "invalid host IP", + port: types.ServicePortConfig{ + Target: 8080, + Published: "80", + HostIP: "invalid", + }, + wantErr: "invalid host IP", + }, + { + name: "missing container port", + port: types.ServicePortConfig{ + Published: "80", + }, + wantErr: "container port must be non-zero", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + result, err := convertServicePortConfigToPortSpec(tt.port) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestTransformServicesPortsExtension_MutualExclusivity(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + content string + wantErr string + }{ + { + name: "both ports and x-ports specified", + content: ` +services: + web: + image: nginx + ports: + - "80:8080" + x-ports: + - "443:8443/https" +`, + wantErr: `service "web" cannot specify both 'ports' and 'x-ports' directives, use only one`, + }, + { + name: "only ports specified", + content: ` +services: + web: + image: nginx + ports: + - "80:8080" +`, + }, + { + name: "only x-ports specified", + content: ` +services: + web: + image: nginx + x-ports: + - "80:8080/tcp@host" +`, + }, + { + name: "no ports specified", + content: ` +services: + web: + image: nginx +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + project, err := loadProjectFromContent(t, tt.content) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + + // Verify service exists + service, err := project.GetService("web") + require.NoError(t, err) + + // Check that ports were processed correctly + if specs, ok := service.Extensions[PortsExtensionKey].([]api.PortSpec); ok { + // Should have specs if ports were specified + assert.NotEmpty(t, specs) + } + }) + } +} + +func TestTransformServicesPortsExtension_StandardPorts(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + content string + expected []api.PortSpec + }{ + { + name: "standard ports short syntax", + content: ` +services: + web: + image: nginx + ports: + - "80:8080" + - "443:8443/tcp" + - "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"}, + }, + }, + { + name: "standard ports long syntax", + content: ` +services: + web: + image: nginx + ports: + - target: 8080 + published: 80 + protocol: tcp + mode: ingress + - target: 8443 + published: 443 + protocol: tcp + mode: host +`, + expected: []api.PortSpec{ + {ContainerPort: 8080, PublishedPort: 80, Protocol: "tcp", Mode: "host"}, + {ContainerPort: 8443, PublishedPort: 443, Protocol: "tcp", Mode: "host"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + project, err := loadProjectFromContent(t, tt.content) + require.NoError(t, err) + + service, err := project.GetService("web") + require.NoError(t, err) + + specs, ok := service.Extensions[PortsExtensionKey].([]api.PortSpec) + require.True(t, ok, "Service should have port specs") + + assert.ElementsMatch(t, tt.expected, specs) + }) + } +} + +func TestTransformServicesPortsExtension_XPorts(t *testing.T) { + t.Parallel() + + content := ` +services: + web: + image: nginx + x-ports: + - "80:8080/tcp" + - "443:8443/https" + - "9090:9090/tcp@host" +` + + project, err := loadProjectFromContent(t, content) + require.NoError(t, err) + + service, err := project.GetService("web") + require.NoError(t, err) + + specs, ok := service.Extensions[PortsExtensionKey].([]api.PortSpec) + require.True(t, ok, "Service should have port specs") + + // Just verify that x-ports still work - don't check exact values as that's tested elsewhere + assert.Len(t, specs, 3) +}