FIX port setting

2026-08-08 06:24:44 +08:00 · 2026-07-24 18:51:52 +08:00
parent f2fa2449e9
commit 6c9f24bb24
13 changed files with 892 additions and 189 deletions
@@ -247,6 +247,8 @@ type ContainerConfig struct {
 	IOReadMBps           int                        `json:"io_read_mbps"`
 	IOWriteMBps          int                        `json:"io_write_mbps"`
 	ExtraPorts           []int                      `json:"extra_ports"`
+	NATPortMappings      []config.PortMapping       `json:"nat_port_mappings,omitempty"`
+	ManagementPort       int                        `json:"management_port,omitempty"`
 	PortMappingCount     int                        `json:"port_mapping_count"`
 	AssignNAT            *bool                      `json:"assign_nat,omitempty"`
 	LANIPv4Mode          string                     `json:"lan_ipv4_mode,omitempty"`
@@ -319,6 +321,120 @@ func (cfg ContainerConfig) WantsNAT() bool {
 	return cfg.AssignNAT == nil || *cfg.AssignNAT
 }

+func (cfg *ContainerConfig) NormalizeCreateNATMappings() error {
+	if cfg == nil {
+		return nil
+	}
+	if !cfg.WantsNAT() {
+		cfg.ExtraPorts = nil
+		cfg.NATPortMappings = nil
+		cfg.ManagementPort = 0
+		cfg.PortMappingCount = 0
+		return nil
+	}
+	if cfg.ManagementPort < 0 || cfg.ManagementPort > 65535 {
+		return fmt.Errorf("management_port must be 1-65535 or 0 for automatic allocation")
+	}
+	if cfg.ManagementPort > 0 && !config.NATPortInRange(cfg.ManagementPort) {
+		start, end := config.NATPortRange()
+		return fmt.Errorf("management_port must be within configured NAT4 range %d-%d", start, end)
+	}
+
+	mappings := append([]config.PortMapping(nil), cfg.NATPortMappings...)
+	if len(mappings) == 0 && len(cfg.ExtraPorts) > 0 {
+		mappings = make([]config.PortMapping, 0, len(cfg.ExtraPorts))
+		for _, port := range cfg.ExtraPorts {
+			mappings = append(mappings, config.PortMapping{
+				HostPort:      port,
+				ContainerPort: port,
+				Protocol:      "tcp",
+			})
+		}
+	}
+	if len(mappings) == 0 {
+		cfg.ExtraPorts = nil
+		if cfg.PortMappingCount < 2 {
+			cfg.PortMappingCount = 2
+		}
+		return nil
+	}
+	if len(mappings) > 63 {
+		return fmt.Errorf("custom NAT port mappings cannot exceed 63")
+	}
+
+	seen := map[string]bool{}
+	if cfg.ManagementPort > 0 {
+		seen[fmt.Sprintf("%d/tcp", cfg.ManagementPort)] = true
+	}
+	for i := range mappings {
+		pm := &mappings[i]
+		pm.HostIP = strings.TrimSpace(pm.HostIP)
+		if pm.HostIP != "" {
+			return fmt.Errorf("nat_port_mappings[%d].host_ip is not supported during creation", i)
+		}
+		if pm.HostPort < 1 || pm.HostPort > 65535 {
+			return fmt.Errorf("nat_port_mappings[%d].host_port must be 1-65535", i)
+		}
+		if !config.NATPortInRange(pm.HostPort) {
+			start, end := config.NATPortRange()
+			return fmt.Errorf("nat_port_mappings[%d].host_port must be within configured NAT4 range %d-%d", i, start, end)
+		}
+		if pm.ContainerPort < 1 || pm.ContainerPort > 65535 {
+			return fmt.Errorf("nat_port_mappings[%d].container_port must be 1-65535", i)
+		}
+		pm.Protocol = strings.ToLower(strings.TrimSpace(pm.Protocol))
+		if pm.Protocol == "" {
+			pm.Protocol = "tcp"
+		}
+		if pm.Protocol != "tcp" && pm.Protocol != "udp" {
+			return fmt.Errorf("nat_port_mappings[%d].protocol must be tcp or udp", i)
+		}
+		key := fmt.Sprintf("%d/%s", pm.HostPort, pm.Protocol)
+		if seen[key] {
+			if pm.HostPort == cfg.ManagementPort && pm.Protocol == "tcp" {
+				return fmt.Errorf("NAT host port mapping %s conflicts with management_port", key)
+			}
+			return fmt.Errorf("duplicate NAT host port mapping: %s", key)
+		}
+		seen[key] = true
+		pm.Description = strings.TrimSpace(pm.Description)
+		if pm.Description == "" {
+			pm.Description = fmt.Sprintf("Port-%d", pm.ContainerPort)
+		}
+	}
+
+	cfg.NATPortMappings = mappings
+	cfg.ExtraPorts = nil
+	cfg.PortMappingCount = len(mappings) + 1
+	return nil
+}
+
+func (cfg ContainerConfig) RequestedNATHostPorts() []int {
+	ports := make([]int, 0, len(cfg.NATPortMappings)+1)
+	if cfg.ManagementPort > 0 {
+		ports = append(ports, cfg.ManagementPort)
+	}
+	for _, pm := range cfg.NATPortMappings {
+		if pm.HostPort > 0 {
+			ports = append(ports, pm.HostPort)
+		}
+	}
+	return ports
+}
+
+func ValidateCreateNATPortAvailability(cfg ContainerConfig) error {
+	candidate := &config.Container{ID: -1}
+	if cfg.ManagementPort > 0 && !HostPortAvailable(candidate, "", cfg.ManagementPort, "tcp") {
+		return fmt.Errorf("NAT management port %d/tcp is already in use", cfg.ManagementPort)
+	}
+	for _, pm := range cfg.NATPortMappings {
+		if !HostPortAvailable(candidate, "", pm.HostPort, pm.Protocol) {
+			return fmt.Errorf("NAT host port %d/%s is already in use", pm.HostPort, pm.Protocol)
+		}
+	}
+	return nil
+}
+
 func (cfg ContainerConfig) WantsLANDHCP() bool {
 	return strings.EqualFold(strings.TrimSpace(cfg.LANIPv4Mode), config.LANIPv4ModeDHCP)
 }
@@ -339,11 +455,8 @@ func (m *Manager) CreateContainer(cfg ContainerConfig) error {
 	if tmpl == nil {
 		return fmt.Errorf("template not found: %s", cfg.TemplateID)
 	}
-	if cfg.WantsNAT() && cfg.PortMappingCount < 2 {
-		cfg.PortMappingCount = 2
-	} else if !cfg.WantsNAT() {
-		cfg.PortMappingCount = 0
-		cfg.ExtraPorts = nil
+	if err := cfg.NormalizeCreateNATMappings(); err != nil {
+		return err
 	}
 	if cfg.SnapshotLimit <= 0 {
 		cfg.SnapshotLimit = config.DefaultSnapshotLimit
@@ -363,6 +476,15 @@ func (m *Manager) CreateContainer(cfg ContainerConfig) error {
 	if err != nil {
 		return err
 	}
+	sshPort := 0
+	releaseNATReservation := func() {}
+	if cfg.WantsNAT() {
+		sshPort, releaseNATReservation, err = ReserveCreateNATPorts(cfg)
+		if err != nil {
+			return err
+		}
+		defer releaseNATReservation()
+	}

 	// Allocate ID and build LXC name
 	id := config.AllocateContainerID()
@@ -443,40 +565,17 @@ func (m *Manager) CreateContainer(cfg ContainerConfig) error {

 	sshPassword := sshAccess.Password

-	sshPort := 0
 	portMappings := []config.PortMapping{}
 	if cfg.WantsNAT() {
-		sshPort, err = config.AllocateSSHPort()
-		if err != nil {
-			_ = m.cleanupContainerStorage(lxcName)
-			return err
-		}
-
 		// Setup default port mappings (SSH only)
 		portMappings = SetupDefaultPortMappings(sshPort)
 		// NAT4 port mappings should bind to the host IP, not the container's independent public IPv4.
 		tempC := &config.Container{ID: id, PublicIPv4s: publicIPv4s, PortMappings: portMappings}

-		extraPorts := cfg.ExtraPorts
-		if len(extraPorts) == 0 && cfg.PortMappingCount > 1 {
-			extraPorts = allocateDefaultEqualPorts(tempC, cfg.PortMappingCount-1)
-		}
-		for _, containerPort := range extraPorts {
-			if containerPort <= 0 {
-				continue
-			}
-			pm, err := normalizePortMapping(tempC, -1, config.PortMapping{
-				ContainerPort: containerPort,
-				HostPort:      containerPort,
-				HostIP:        "",
-				Protocol:      "tcp",
-				Description:   fmt.Sprintf("Port-%d", containerPort),
-			})
-			if err != nil {
-				continue
-			}
-			tempC.PortMappings = append(tempC.PortMappings, pm)
-			portMappings = tempC.PortMappings
+		portMappings, err = SetupCreatePortMappings(tempC, cfg)
+		if err != nil {
+			_ = m.cleanupContainerStorage(lxcName)
+			return err
 		}
 	}

@@ -6,6 +6,8 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+
+	"clicd/internal/config"
 )

 func TestRootfsCommandAddsSeparatorForAllowedCommand(t *testing.T) {
@@ -27,6 +29,158 @@ func TestRootfsCommandAddsSeparatorForAllowedCommand(t *testing.T) {
 	}
 }

+func TestNormalizeCreateNATMappingsSupportsDifferentHostAndContainerPorts(t *testing.T) {
+	previous := config.AppConfig
+	t.Cleanup(func() { config.AppConfig = previous })
+	config.AppConfig = &config.ClicdConfig{NATPortStart: 20000, NATPortEnd: 65535}
+
+	cfg := ContainerConfig{
+		PortMappingCount: 2,
+		NATPortMappings: []config.PortMapping{{
+			HostPort:      30080,
+			ContainerPort: 80,
+			Protocol:      "TCP",
+		}},
+	}
+	if err := cfg.NormalizeCreateNATMappings(); err != nil {
+		t.Fatal(err)
+	}
+	if cfg.PortMappingCount != 2 || len(cfg.NATPortMappings) != 1 {
+		t.Fatalf("normalized config = %+v", cfg)
+	}
+	mapping := cfg.NATPortMappings[0]
+	if mapping.HostPort != 30080 || mapping.ContainerPort != 80 || mapping.Protocol != "tcp" {
+		t.Fatalf("normalized mapping = %+v", mapping)
+	}
+
+	container := &config.Container{
+		ID: -1,
+		PortMappings: []config.PortMapping{{
+			HostPort:      22000,
+			ContainerPort: 22,
+			Protocol:      "tcp",
+			Description:   "SSH",
+		}},
+	}
+	mappings, err := SetupCreatePortMappings(container, cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(mappings) != 2 || mappings[1].HostPort != 30080 || mappings[1].ContainerPort != 80 {
+		t.Fatalf("created mappings = %+v", mappings)
+	}
+}
+
+func TestNormalizeCreateNATMappingsKeepsLegacyExtraPortsCompatible(t *testing.T) {
+	previous := config.AppConfig
+	t.Cleanup(func() { config.AppConfig = previous })
+	config.AppConfig = &config.ClicdConfig{NATPortStart: 20000, NATPortEnd: 65535}
+
+	cfg := ContainerConfig{ExtraPorts: []int{30080, 30443}}
+	if err := cfg.NormalizeCreateNATMappings(); err != nil {
+		t.Fatal(err)
+	}
+	if len(cfg.ExtraPorts) != 0 || len(cfg.NATPortMappings) != 2 {
+		t.Fatalf("legacy ports were not converted: %+v", cfg)
+	}
+	for _, mapping := range cfg.NATPortMappings {
+		if mapping.HostPort != mapping.ContainerPort {
+			t.Fatalf("legacy mapping changed semantics: %+v", mapping)
+		}
+	}
+}
+
+func TestNormalizeCreateNATMappingsRejectsDuplicateHostPort(t *testing.T) {
+	previous := config.AppConfig
+	t.Cleanup(func() { config.AppConfig = previous })
+	config.AppConfig = &config.ClicdConfig{NATPortStart: 20000, NATPortEnd: 65535}
+
+	cfg := ContainerConfig{NATPortMappings: []config.PortMapping{
+		{HostPort: 30080, ContainerPort: 80, Protocol: "tcp"},
+		{HostPort: 30080, ContainerPort: 8080, Protocol: "tcp"},
+	}}
+	if err := cfg.NormalizeCreateNATMappings(); err == nil {
+		t.Fatal("duplicate host port was accepted")
+	}
+}
+
+func TestNormalizeCreateNATMappingsRejectsManagementPortConflict(t *testing.T) {
+	previous := config.AppConfig
+	t.Cleanup(func() { config.AppConfig = previous })
+	config.AppConfig = &config.ClicdConfig{NATPortStart: 20000, NATPortEnd: 65535}
+
+	cfg := ContainerConfig{
+		ManagementPort: 30022,
+		NATPortMappings: []config.PortMapping{{
+			HostPort:      30022,
+			ContainerPort: 8080,
+			Protocol:      "tcp",
+		}},
+	}
+	if err := cfg.NormalizeCreateNATMappings(); err == nil || !strings.Contains(err.Error(), "management_port") {
+		t.Fatalf("management port conflict returned %v", err)
+	}
+}
+
+func TestReserveCreateNATPortsProtectsConcurrentTasks(t *testing.T) {
+	previous := config.AppConfig
+	t.Cleanup(func() { config.AppConfig = previous })
+	config.AppConfig = &config.ClicdConfig{
+		NATPortStart: 20000,
+		NATPortEnd:   65535,
+		NextSSHPort:  22000,
+	}
+
+	createNATReservationMu.Lock()
+	createNATReservations = map[uint64][]config.PortMapping{}
+	createNATReservationMu.Unlock()
+	t.Cleanup(func() {
+		createNATReservationMu.Lock()
+		createNATReservations = map[uint64][]config.PortMapping{}
+		createNATReservationMu.Unlock()
+	})
+
+	cfg := ContainerConfig{NATPortMappings: []config.PortMapping{{
+		HostPort:      22000,
+		ContainerPort: 80,
+		Protocol:      "tcp",
+	}}}
+	if err := cfg.NormalizeCreateNATMappings(); err != nil {
+		t.Fatal(err)
+	}
+
+	managementPort, release, err := ReserveCreateNATPorts(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if managementPort == 22000 {
+		t.Fatal("management port collided with the requested custom host port")
+	}
+	if _, _, err := ReserveCreateNATPorts(cfg); err == nil {
+		t.Fatal("concurrent task reserved an already reserved custom host port")
+	}
+
+	release()
+	if _, releaseAgain, err := ReserveCreateNATPorts(cfg); err != nil {
+		t.Fatalf("released custom host port remained reserved: %v", err)
+	} else {
+		releaseAgain()
+	}
+
+	explicit := ContainerConfig{ManagementPort: 30022}
+	if err := explicit.NormalizeCreateNATMappings(); err != nil {
+		t.Fatal(err)
+	}
+	if port, releaseExplicit, err := ReserveCreateNATPorts(explicit); err != nil {
+		t.Fatal(err)
+	} else {
+		defer releaseExplicit()
+		if port != explicit.ManagementPort {
+			t.Fatalf("reserved management port = %d, want %d", port, explicit.ManagementPort)
+		}
+	}
+}
+
 func TestRootfsCommandRejectsUnmanagedCommand(t *testing.T) {
 	base := t.TempDir()
 	rootfs := filepath.Join(base, "ct-1", "rootfs")
@@ -6,10 +6,17 @@ import (
 	"os/exec"
 	"strconv"
 	"strings"
+	"sync"

 	"clicd/internal/config"
 )

+var (
+	createNATReservationMu     sync.Mutex
+	createNATReservationNextID uint64
+	createNATReservations      = map[uint64][]config.PortMapping{}
+)
+
 // ApplyPortMappings applies iptables DNAT rules for a container's port mappings
 func (m *Manager) ApplyPortMappings(id int) error {
 	c := config.FindContainer(id)
@@ -509,6 +516,96 @@ func normalizePortMapping(c *config.Container, skipIndex int, pm config.PortMapp
 	return pm, nil
 }

+// SetupCreatePortMappings appends validated custom or automatically allocated
+// mappings to a container's management port mapping.
+func SetupCreatePortMappings(c *config.Container, cfg ContainerConfig) ([]config.PortMapping, error) {
+	if c == nil {
+		return nil, fmt.Errorf("container is required")
+	}
+	requested := append([]config.PortMapping(nil), cfg.NATPortMappings...)
+	if len(requested) == 0 && cfg.PortMappingCount > 1 {
+		for _, port := range allocateDefaultEqualPorts(c, cfg.PortMappingCount-1) {
+			requested = append(requested, config.PortMapping{
+				ContainerPort: port,
+				HostPort:      port,
+				Protocol:      "tcp",
+				Description:   fmt.Sprintf("Port-%d", port),
+			})
+		}
+	}
+	for _, mapping := range requested {
+		pm, err := normalizePortMapping(c, -1, mapping)
+		if err != nil {
+			return nil, err
+		}
+		c.PortMappings = append(c.PortMappings, pm)
+	}
+	return c.PortMappings, nil
+}
+
+// ReserveCreateNATPorts keeps concurrent create tasks from selecting each
+// other's custom or management ports before their containers are persisted.
+func ReserveCreateNATPorts(cfg ContainerConfig) (int, func(), error) {
+	if !cfg.WantsNAT() {
+		return 0, func() {}, nil
+	}
+
+	createNATReservationMu.Lock()
+	defer createNATReservationMu.Unlock()
+
+	if err := ValidateCreateNATPortAvailability(cfg); err != nil {
+		return 0, nil, err
+	}
+	requestedReservations := append([]config.PortMapping(nil), cfg.NATPortMappings...)
+	if cfg.ManagementPort > 0 {
+		requestedReservations = append(requestedReservations, config.PortMapping{
+			HostPort: cfg.ManagementPort,
+			Protocol: "tcp",
+		})
+	}
+	for _, requested := range requestedReservations {
+		for _, reservations := range createNATReservations {
+			for _, reserved := range reservations {
+				if requested.HostPort == reserved.HostPort && protocolsOverlap(requested.Protocol, reserved.Protocol) {
+					return 0, nil, fmt.Errorf("NAT host port %d/%s is reserved by another create task", requested.HostPort, requested.Protocol)
+				}
+			}
+		}
+	}
+
+	excluded := cfg.RequestedNATHostPorts()
+	for _, reservations := range createNATReservations {
+		for _, reserved := range reservations {
+			excluded = append(excluded, reserved.HostPort)
+		}
+	}
+	managementPort := cfg.ManagementPort
+	if managementPort == 0 {
+		var err error
+		managementPort, err = config.AllocateSSHPortExcluding(excluded)
+		if err != nil {
+			return 0, nil, err
+		}
+	}
+
+	createNATReservationNextID++
+	reservationID := createNATReservationNextID
+	reservations := make([]config.PortMapping, 0, len(cfg.NATPortMappings)+1)
+	reservations = append(reservations, config.PortMapping{HostPort: managementPort, Protocol: "tcp"})
+	reservations = append(reservations, cfg.NATPortMappings...)
+	createNATReservations[reservationID] = reservations
+
+	var once sync.Once
+	release := func() {
+		once.Do(func() {
+			createNATReservationMu.Lock()
+			delete(createNATReservations, reservationID)
+			createNATReservationMu.Unlock()
+		})
+	}
+	return managementPort, release, nil
+}
+
 func allocateDefaultEqualPorts(c *config.Container, count int) []int {
 	if count <= 0 {
 		return nil