From 7a6a6c36d11796b2048d90353f06d25013b132a8 Mon Sep 17 00:00:00 2001
From: Vishwas Siravara <siravara@amazon.com>
Date: Tue, 28 Feb 2023 08:39:04 -0800
Subject: [PATCH] refactor: Refactor -p/--publish flag test for run command
 (#39)

Issue #, if available:
https://github.com/runfinch/finch/issues/196

*Description of changes:*

Use nginx server for testing port publish(`-p/--publish`) flag in the
run command instead of [busybox
netcat](https://busybox.net/downloads/BusyBox.html#nc). This is due to
the fact that the cni gateway in rootful makes a request to the netcat
server running in the container and closes the connection which causes
netcat to exit before a client on the host can make a connection to it.

*Testing done:*
Yes.


- [X] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: Vishwas Siravara <vsiravara@gmail.com>
---
 fnet/dial.go   | 21 ++++++++-------------
 tests/run.go   | 15 +++++----------
 tests/tests.go |  1 +
 3 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/fnet/dial.go b/fnet/dial.go
index bb0b823..c299b99 100644
--- a/fnet/dial.go
+++ b/fnet/dial.go
@@ -4,31 +4,26 @@
 package fnet
 
 import (
-	"net"
+	"net/http"
 	"time"
 
 	"github.com/onsi/ginkgo/v2"
 	"github.com/onsi/gomega"
 )
 
-// DialAndRead dials the network address, reads the data from the established connection, and asserts it against want.
-func DialAndRead(network, address, want string, maxRetry int, retryInterval time.Duration) {
-	var (
-		conn net.Conn
-		err  error
-	)
+// HTTPGetAndAssert sends an HTTP GET request to the specified URL, asserts the response status code against want, and closes the response body.
+func HTTPGetAndAssert(url string, want int, maxRetry int, retryInterval time.Duration) {
+	var err error
 	for i := 0; i < maxRetry; i++ {
-		conn, err = net.Dial(network, address)
+		// #nosec G107 // it does not matter if url is not a constant for testing.
+		resp, err := http.Get(url)
 		if err != nil {
 			time.Sleep(retryInterval)
 			continue
 		}
 
-		b := make([]byte, len([]byte(want))) //nolint:makezero // The content of b does not matter,
-		// but len(b) must be equal to len([]byte(want)) so that conn.Read can read the whole thing.
-		gomega.Expect(conn.Read(b)).Error().ShouldNot(gomega.HaveOccurred())
-		gomega.Expect(b).To(gomega.Equal([]byte(want)))
-		gomega.Expect(conn.Close()).To(gomega.Succeed())
+		gomega.Expect(resp.StatusCode).To(gomega.Equal(want))
+		gomega.Expect(resp.Body.Close()).To(gomega.Succeed())
 		return
 	}
 	ginkgo.Fail(err.Error())
diff --git a/tests/run.go b/tests/run.go
index 8b96df2..c8f3d5b 100644
--- a/tests/run.go
+++ b/tests/run.go
@@ -360,17 +360,12 @@ func Run(o *RunOption) {
 			for _, publish := range []string{"-p", "--publish"} {
 				publish := publish
 				ginkgo.It(fmt.Sprintf("port of the container should be published to the host port with %s flag", publish), func() {
-					const (
-						containerPort = 80
-						strEchoed     = "hello"
-					)
+					const containerPort = 80
 					hostPort := fnet.GetFreePort()
-					// The nc version used by alpine image is https://busybox.net/downloads/BusyBox.html#nc,
-					// which is different from that in linux: https://linux.die.net/man/1/nc.
-					command.Run(o.BaseOpt, "run", "-d", publish, fmt.Sprintf("%d:%d", hostPort, containerPort), defaultImage,
-						"sh", "-c", fmt.Sprintf("echo %s | nc -l -p %d", strEchoed, containerPort))
-
-					fnet.DialAndRead("tcp", fmt.Sprintf("localhost:%d", hostPort), strEchoed, 20, 200*time.Millisecond)
+					// Start an Nginx container in detached mode with the specified publish flag and mapping the container port to
+					// a randomly selected host port.
+					command.Run(o.BaseOpt, "run", "-d", publish, fmt.Sprintf("%d:%d", hostPort, containerPort), nginxImage)
+					fnet.HTTPGetAndAssert(fmt.Sprintf("http://localhost:%d", hostPort), 200, 20, 200*time.Millisecond)
 				})
 			}
 		})
diff --git a/tests/tests.go b/tests/tests.go
index b0bfeeb..f1c2823 100644
--- a/tests/tests.go
+++ b/tests/tests.go
@@ -28,6 +28,7 @@ const (
 	alpineImage              = "public.ecr.aws/docker/library/alpine:latest"
 	olderAlpineImage         = "public.ecr.aws/docker/library/alpine:3.13"
 	amazonLinux2Image        = "public.ecr.aws/amazonlinux/amazonlinux:2"
+	nginxImage               = "public.ecr.aws/docker/library/nginx:latest"
 	testImageName            = "test:tag"
 	nonexistentImageName     = "ne-repo:ne-tag"
 	nonexistentContainerName = "ne-ctr"