Merge pull request #4019 from Horiodino/port-forwarding

AkihiroSuda · web-flow · commit 9e96fe10f0ef · 2025-09-19T17:14:25.000+09:00
bug fix: static port forward works in plain
diff --git a/hack/test-nonplain-static-port-forward.sh b/hack/test-nonplain-static-port-forward.sh
@@ -6,20 +6,20 @@
 set -euxo pipefail
 
 INSTANCE=nonplain-static-port-forward
-TEMPLATE=hack/test-templates/static-port-forward.yaml
+TEMPLATE="$(dirname "$0")/test-templates/test-misc.yaml"
 
-limactl delete -f $INSTANCE || true
+limactl delete -f "$INSTANCE" || true
 
-limactl start --name=$INSTANCE --tty=false $TEMPLATE
+limactl start --name="$INSTANCE" --tty=false "$TEMPLATE"
 
-limactl shell $INSTANCE -- bash -c 'until [ -e /run/nginx.pid ]; do sleep 1; done'
-limactl shell $INSTANCE -- bash -c 'until systemctl is-active --quiet test-server-9080; do sleep 1; done'
-limactl shell $INSTANCE -- bash -c 'until systemctl is-active --quiet test-server-9070; do sleep 1; done'
+limactl shell "$INSTANCE" -- bash -c 'until systemctl is-active --quiet nginx; do sleep 1; done'
+limactl shell "$INSTANCE" -- bash -c 'until systemctl is-active --quiet test-server-9080; do sleep 1; done'
+limactl shell "$INSTANCE" -- bash -c 'until systemctl is-active --quiet test-server-9070; do sleep 1; done'
 
 curl -sSf http://127.0.0.1:9090 | grep -i 'nginx' && echo 'Static port forwarding (9090) works in normal mode!'
-curl -sSf http://127.0.0.1:9080 | grep -i 'Dynamic port 9080' && echo 'Dynamic port forwarding (9080) works in normal mode!'
-curl -sSf http://127.0.0.1:9070 | grep -i 'Dynamic port 9070' && echo 'Dynamic port forwarding (9070) works in normal mode!'
+curl -sSf http://127.0.0.1:29080 | grep -i 'Dynamic port 9080' && echo 'Dynamic port forwarding (29080) works in normal mode!'
+curl -sSf http://127.0.0.1:29070 | grep -i 'Dynamic port 9070' && echo 'Dynamic port forwarding (29070) works in normal mode!'
 
-limactl delete -f $INSTANCE
+limactl delete -f "$INSTANCE"
 echo "All tests passed for normal mode - both static and dynamic ports work!"
 # EOF
diff --git a/hack/test-plain-static-port-forward.sh b/hack/test-plain-static-port-forward.sh
@@ -6,30 +6,34 @@
 set -euxo pipefail
 
 INSTANCE=plain-static-port-forward
-TEMPLATE=hack/test-templates/static-port-forward.yaml
+TEMPLATE="$(dirname "$0")/test-templates/test-misc.yaml"
 
-limactl delete -f $INSTANCE || true
+limactl delete -f "$INSTANCE" || true
 
-limactl start --name=$INSTANCE --plain=true --tty=false $TEMPLATE
+limactl start --name="$INSTANCE" --plain=true --tty=false "$TEMPLATE"
 
-limactl shell $INSTANCE -- bash -c 'until [ -e /run/nginx.pid ]; do sleep 1; done'
+limactl shell "$INSTANCE" -- bash -c 'until systemctl is-active --quiet nginx; do sleep 1; done'
 
-curl -sSf http://127.0.0.1:9090 | grep -i 'nginx' && echo 'Static port forwarding (9090) works in plain mode!'
+if ! curl -sSf http://127.0.0.1:9090 | grep -i 'nginx'; then
+	echo 'ERROR: Static port forwarding (9090) does not work in plain mode!'
+	exit 1
+fi
+echo 'Static port forwarding (9090) works in plain mode!'
 
-if curl -sSf http://127.0.0.1:9080 2>/dev/null; then
-	echo 'ERROR: Dynamic port 9080 should not be forwarded in plain mode!'
+if curl -sSf http://127.0.0.1:29080 2>/dev/null; then
+	echo 'ERROR: Dynamic port 29080 should not be forwarded in plain mode!'
 	exit 1
 else
-	echo 'Dynamic port 9080 is correctly NOT forwarded in plain mode.'
+	echo 'Dynamic port 29080 is correctly NOT forwarded in plain mode.'
 fi
 
-if curl -sSf http://127.0.0.1:9070 2>/dev/null; then
-	echo 'ERROR: Dynamic port 9070 should not be forwarded in plain mode!'
+if curl -sSf http://127.0.0.1:29070 2>/dev/null; then
+	echo 'ERROR: Dynamic port 29070 should not be forwarded in plain mode!'
 	exit 1
 else
-	echo 'Dynamic port 9070 is correctly NOT forwarded in plain mode.'
+	echo 'Dynamic port 29070 is correctly NOT forwarded in plain mode.'
 fi
 
-limactl delete -f $INSTANCE
+limactl delete -f "$INSTANCE"
 echo "All tests passed for plain mode - only static ports work!"
 # EOF
diff --git a/hack/test-templates.sh b/hack/test-templates.sh
@@ -51,7 +51,6 @@ declare -A CHECKS=(
 	["snapshot-offline"]=""
 	["clone"]=""
 	["port-forwards"]="1"
-	["static-port-forwards"]=""
 	["vmnet"]=""
 	["disk"]=""
 	["user-v2"]=""
@@ -61,6 +60,7 @@ declare -A CHECKS=(
 	["param-env-variables"]=""
 	["set-user"]=""
 	["preserve-env"]="1"
+	["static-port-forwards"]=""
 )
 
 case "$NAME" in
@@ -95,12 +95,7 @@ case "$NAME" in
 	CHECKS["provision-yq"]="1"
 	CHECKS["param-env-variables"]="1"
 	CHECKS["set-user"]="1"
-	;;
-"static-port-forward")
 	CHECKS["static-port-forwards"]="1"
-	CHECKS["port-forwards"]=""
-	CHECKS["container-engine"]=""
-	CHECKS["restart"]=""
 	;;
 "docker")
 	CONTAINER_ENGINE="docker"
@@ -463,13 +458,6 @@ if [[ -n ${CHECKS["port-forwards"]} ]]; then
 	set +x
 fi
 
-if [[ -n ${CHECKS["static-port-forwards"]} ]]; then
-	INFO "Testing static port forwarding functionality"
-	"${scriptdir}/test-plain-static-port-forward.sh" "$NAME"
-	"${scriptdir}/test-nonplain-static-port-forward.sh" "$NAME"
-	INFO "All static port forwarding tests passed!"
-fi
-
 if [[ -n ${CHECKS["vmnet"]} ]]; then
 	INFO "Testing vmnet functionality"
 	guestip="$(limactl shell "$NAME" ip -4 -j addr show dev lima0 | jq -r '.[0].addr_info[0].local')"
@@ -639,3 +627,10 @@ limactl delete "$NAME"
 if [[ -n ${CHECKS["mount-path-with-spaces"]} ]]; then
 	rm -rf "/tmp/lima test dir with spaces"
 fi
+
+if [[ -n ${CHECKS["static-port-forwards"]} ]]; then
+	INFO "Testing static port forwarding functionality"
+	"${scriptdir}/test-plain-static-port-forward.sh" "$NAME"
+	"${scriptdir}/test-nonplain-static-port-forward.sh" "$NAME"
+	INFO "All static port forwarding tests passed!"
+fi
diff --git a/hack/test-templates/static-port-forward.yaml b/hack/test-templates/static-port-forward.yaml
diff --git a/hack/test-templates/test-misc.yaml b/hack/test-templates/test-misc.yaml
@@ -30,7 +30,57 @@ provision:
 - mode: dependency
   script: "touch /tmp/param-$PARAM_DEPENDENCY"
 - mode: system
-  script: "touch /tmp/param-$PARAM_SYSTEM"
+  script: |
+    touch /tmp/param-$PARAM_SYSTEM
+
+    # port forwarding test setup
+    apt-get update
+    apt-get install -y nginx python3
+    systemctl enable nginx
+    systemctl start nginx
+
+    cat > /etc/systemd/system/test-server-9080.service << 'EOF'
+    [Unit]
+    Description=Test Server on Port 9080
+    After=network.target
+
+    [Service]
+    Type=simple
+    User=root
+    ExecStart=/usr/bin/python3 -m http.server 9080 --bind 127.0.0.1
+    Restart=always
+
+    [Install]
+    WantedBy=multi-user.target
+    EOF
+
+    cat > /etc/systemd/system/test-server-9070.service << 'EOF'
+    [Unit]
+    Description=Test Server on Port 9070
+    After=network.target
+
+    [Service]
+    Type=simple
+    User=root
+    ExecStart=/usr/bin/python3 -m http.server 9070 --bind 127.0.0.1
+    Restart=always
+
+    [Install]
+    WantedBy=multi-user.target
+    EOF
+
+    mkdir -p /var/www/html-9080
+    mkdir -p /var/www/html-9070
+
+    echo '<html><body><h1>Dynamic port 9080</h1></body></html>' > /var/www/html-9080/index.html
+    echo '<html><body><h1>Dynamic port 9070</h1></body></html>' > /var/www/html-9070/index.html
+
+    systemctl daemon-reload
+    systemctl enable test-server-9080
+    systemctl enable test-server-9070
+    systemctl start test-server-9080
+    systemctl start test-server-9070
+
 - mode: user
   script: "touch /tmp/param-$PARAM_USER"
 - mode: data
@@ -41,7 +91,6 @@ provision:
 - mode: yq
   path: "/tmp/param-{{.Param.YQ}}.json"
   expression: .YQ = "{{.Param.YQ}}"
-  user: "{{.User}}"
 
 probes:
 - mode: readiness
@@ -61,3 +110,14 @@ user:
   uid: 4711
   # Ubuntu has identical /bin/bash and /usr/bin/bash
   shell: /usr/bin/bash
+
+portForwards:
+- guestPort: 80
+  hostPort: 9090
+  static: true
+- guestPort: 9080
+  hostPort: 29080
+  static: false
+- guestPort: 9070
+  hostPort: 29070
+  static: false
diff --git a/pkg/hostagent/hostagent.go b/pkg/hostagent/hostagent.go
@@ -536,13 +536,10 @@ sudo chown -R "${USER}" /run/host-services`
 		})
 	}
 
-	if !*a.instConfig.Plain {
-		staticPortForwards, err := a.separateStaticPortForwards()
-		if err != nil {
-			errs = append(errs, err)
-		}
-		a.addStaticPortForwardsFromList(ctx, staticPortForwards)
+	staticPortForwards := a.separateStaticPortForwards()
+	a.addStaticPortForwardsFromList(ctx, staticPortForwards)
 
+	if !*a.instConfig.Plain {
 		go a.watchGuestAgentEvents(ctx)
 		if a.showProgress {
 			cloudInitDone := make(chan struct{})
@@ -704,7 +701,10 @@ func (a *HostAgent) addStaticPortForwardsFromList(ctx context.Context, staticPor
 	}
 }
 
-func (a *HostAgent) separateStaticPortForwards() ([]limatype.PortForward, error) {
+// separateStaticPortForwards separates static port forwards from a.instConfig.PortForwards,
+// updates a.instConfig.PortForwards to contain only non-static port forwards,
+// and returns the list of static port forwards.
+func (a *HostAgent) separateStaticPortForwards() []limatype.PortForward {
 	staticPortForwards := make([]limatype.PortForward, 0, len(a.instConfig.PortForwards))
 	nonStaticPortForwards := make([]limatype.PortForward, 0, len(a.instConfig.PortForwards))
 
@@ -722,8 +722,7 @@ func (a *HostAgent) separateStaticPortForwards() ([]limatype.PortForward, error)
 	logrus.Debugf("Static port forwards: %d, Non-static port forwards: %d", len(staticPortForwards), len(nonStaticPortForwards))
 
 	a.instConfig.PortForwards = nonStaticPortForwards
-
-	return staticPortForwards, nil
+	return staticPortForwards
 }
 
 func isGuestAgentSocketAccessible(ctx context.Context, client *guestagentclient.GuestAgentClient) bool {
