From 022353eb9a6ddf1ab7c6870e0052506a3b394454 Mon Sep 17 00:00:00 2001
From: Webb Scales <wscales@redhat.com>
Date: Wed, 27 Mar 2024 20:22:45 -0400
Subject: [PATCH] Review feedback

---
 connector_test.go | 32 +++++++++++++++++++++++---------
 tests/common.go   | 35 +++++++++++++++++------------------
 2 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/connector_test.go b/connector_test.go
index e7f8ed8..e3c2ac6 100644
--- a/connector_test.go
+++ b/connector_test.go
@@ -8,7 +8,6 @@ import (
 	"io"
 	"os"
 	"os/exec"
-	"runtime"
 	"strings"
 	"testing"
 	"time"
@@ -118,12 +117,23 @@ var volumeConfig = `
 `
 
 func bindMountHelper(t *testing.T, options string) {
-	t.Helper()
 	fileContent, err := os.ReadFile("./tests/volume/test_file.txt")
 	assert.NoError(t, err)
 
 	connector, _ := getConnector(t, fmt.Sprintf(volumeConfig, options))
 
+	if tests.IsRunningOnLinux() && options == "" {
+		// On Linux, when SELinux is enabled, then bind mounts without
+		// relabeling options will fail.  So, to test this case, disable
+		// SELinux on the test folder in order to make the file readable
+		// from within the container.
+		cmd := exec.Command("chcon", "-Rt", "svirt_sandbox_file_t", "./tests/volume")
+		err = cmd.Run()
+		if err != nil {
+			t.Fatalf("chcon error: %s", err.Error())
+		}
+	}
+
 	container, err := connector.Deploy(
 		context.Background(),
 		"quay.io/arcalot/podman-deployer-test-helper:0.1.0")
@@ -136,16 +146,21 @@ func bindMountHelper(t *testing.T, options string) {
 	// Note: If it ends up with length zero buffer, restarting the VM may help:
 	// https://stackoverflow.com/questions/71977532/podman-mount-host-volume-return-error-statfs-no-such-file-or-directory-in-ma
 	readBuffer := readOutputUntil(t, container, string(fileContent))
-	assert.GreaterThan(t, len(readBuffer), 0)
+	assert.Contains(t, string(readBuffer), string(fileContent))
 }
 
 func TestBindMount(t *testing.T) {
 	scenarios := map[string]string{
-		"No options": "",
-		"Private":    ":Z",
-		"Shared":     ":z",
 		"ReadOnly":   ":ro",
-		"Multiple":   ":z,ro,noexec",
+		"Multiple":   ":ro,noexec",
+		"No options": "",
+	}
+	//goland:noinspection GoBoolExpressions  // The linter cannot tell that this expression is not constant.
+	if tests.IsRunningOnLinux() {
+		// The SELinux options seem to cause problems on Mac OS X, so only test
+		// them on Linux.
+		scenarios["Private"] = ":Z"
+		scenarios["Shared"] = ":z"
 	}
 	for name, s := range scenarios {
 		options := s
@@ -306,9 +321,8 @@ func TestPrivateCgroupNs(t *testing.T) {
 
 func TestHostCgroupNs(t *testing.T) {
 	//goland:noinspection GoBoolExpressions  // The linter cannot tell that this expression is not constant.
-	if runtime.GOOS != "linux" {
+	if !tests.IsRunningOnLinux() {
 		t.Skipf("Not running on Linux. Skipping cgroup test.")
-		return
 	}
 	logger := log.NewTestLogger(t)
 
diff --git a/tests/common.go b/tests/common.go
index e897ef1..de6bf58 100644
--- a/tests/common.go
+++ b/tests/common.go
@@ -8,8 +8,8 @@ import (
 	"os"
 	"os/exec"
 	"regexp"
+	"runtime"
 	"strings"
-	"sync"
 	"time"
 )
 
@@ -84,7 +84,7 @@ func GetCommmandCgroupNs(logger log.Logger, command string, args []string) strin
 	}
 	// parse output from command
 	stdoutStr := stdout.String()
-	regex := regexp.MustCompile(`.*cgroup:\[(\d+)\]`)
+	regex := regexp.MustCompile(`.*cgroup:\[(\d+)]`)
 	userCgroupNs = regex.ReplaceAllString(stdoutStr, "$1")
 	userCgroupNs = strings.TrimSuffix(userCgroupNs, "\n")
 
@@ -97,22 +97,16 @@ func GetCommmandCgroupNs(logger log.Logger, command string, args []string) strin
 
 // GetPodmanCgroupNs  detects the running container cgroup namespace
 func GetPodmanCgroupNs(logger log.Logger, podmanPath string, containerName string) string {
-	var wg sync.WaitGroup
-	wg.Add(1)
-	var podmanCgroupNs string
-	go func() {
-		defer wg.Done()
-		var stdout bytes.Buffer
-		cmd := exec.Command(podmanPath, "ps", "--ns", "--filter", fmt.Sprintf("name=%s", containerName), "--format", "{{.CGROUPNS}}") //nolint:gosec
-		cmd.Stdout = &stdout
-		if err := cmd.Run(); err != nil {
-			logger.Errorf(err.Error())
-		}
-		podmanCgroupNs = stdout.String()
-	}()
-	wg.Wait()
-	podmanCgroupNs = strings.TrimSuffix(podmanCgroupNs, "\n")
-	return podmanCgroupNs
+	var stdout bytes.Buffer
+	cmd := exec.Command( //nolint:gosec
+		podmanPath, "ps", "--ns", "--filter",
+		fmt.Sprintf("name=%s", containerName),
+		"--format", "{{.CGROUPNS}}")
+	cmd.Stdout = &stdout
+	if err := cmd.Run(); err != nil {
+		logger.Errorf(err.Error())
+	}
+	return strings.TrimSuffix(stdout.String(), "\n")
 }
 
 func IsContainerRunning(logger log.Logger, podmanPath string, containerName string) bool {
@@ -140,3 +134,8 @@ func IsRunningOnGithub() bool {
 	githubEnv := os.Getenv("GITHUB_ACTION")
 	return githubEnv != ""
 }
+
+func IsRunningOnLinux() bool {
+	//goland:noinspection GoBoolExpressions  // The linter cannot tell that this expression is not constant.
+	return runtime.GOOS == "linux"
+}