refactor: improve cleanup

Author: evgeniy-scherbina

e2e_tests/boundary_integration_test.go

@@ -150,6 +150,11 @@ func TestBoundaryIntegration(t *testing.T) {
 		require.Contains(t, string(output), "Request Blocked by Boundary")
 	})
 
+	// Gracefully close process, call cleanup methods
+	err = boundaryCmd.Process.Signal(os.Interrupt)
+	require.NoError(t, err, "Failed to interrupt boundary process")
+	time.Sleep(time.Second * 1)
+
 	// Clean up
 	cancel()                 // This will terminate the boundary process
 	err = boundaryCmd.Wait() // Wait for process to finish

e2e_tests/iptables_cleanup_test.go

@@ -0,0 +1,76 @@
+package e2e_tests
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestIPTablesCleanup(t *testing.T) {
+	// Step 1: Capture initial iptables rules
+	initialCmd := exec.Command("sudo", "iptables", "-L", "-n")
+	initialOutput, err := initialCmd.Output()
+	require.NoError(t, err, "Failed to get initial iptables rules")
+	initialRules := string(initialOutput)
+	//fmt.Printf("Initial iptables rules:\n%s", initialRules)
+
+	// Step 2: Run Boundary
+	// Find project root by looking for go.mod file
+	projectRoot := findProjectRoot(t)
+
+	// Build the boundary binary
+	buildCmd := exec.Command("go", "build", "-o", "/tmp/boundary-test", "./cmd/...")
+	buildCmd.Dir = projectRoot
+	err = buildCmd.Run()
+	require.NoError(t, err, "Failed to build boundary binary")
+
+	// Create context for boundary process
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+
+	// Start boundary process with sudo
+	boundaryCmd := exec.CommandContext(ctx, "/tmp/boundary-test",
+		"--allow", "dev.coder.com",
+		"--allow", "jsonplaceholder.typicode.com",
+		"--log-level", "debug",
+		"--", "/bin/bash", "-c", "/usr/bin/sleep 10 && /usr/bin/echo 'Test completed'")
+
+	boundaryCmd.Stdin = os.Stdin
+	boundaryCmd.Stdout = os.Stdout
+	boundaryCmd.Stderr = os.Stderr
+
+	// Start the process
+	err = boundaryCmd.Start()
+	require.NoError(t, err, "Failed to start boundary process")
+
+	// Give boundary time to start
+	time.Sleep(2 * time.Second)
+
+	// Gracefully close process, call cleanup methods
+	err = boundaryCmd.Process.Signal(os.Interrupt)
+	require.NoError(t, err, "Failed to interrupt boundary process")
+	time.Sleep(time.Second * 1)
+
+	// Step 3: Clean up
+	cancel()                 // This will terminate the boundary process
+	err = boundaryCmd.Wait() // Wait for process to finish
+	if err != nil {
+		t.Logf("Boundary process finished with error: %v", err)
+	}
+
+	// Clean up binary
+	err = os.Remove("/tmp/boundary-test")
+	require.NoError(t, err, "Failed to remove /tmp/boundary-test")
+
+	// Step 4: Capture iptables rules after boundary has executed
+	iptablesCmd := exec.Command("sudo", "iptables", "-L", "-n")
+	iptablesOutput, err := iptablesCmd.Output()
+	require.NoError(t, err, "Failed to get iptables rules")
+	iptablesRules := string(iptablesOutput)
+
+	require.Equal(t, initialRules, iptablesRules)
+}

jail/linux.go

@@ -4,6 +4,7 @@ package jail
 
 import (
 	"fmt"
+	"log"
 	"log/slog"
 	"os"
 	"os/exec"
@@ -171,6 +172,22 @@ func (r *commandRunner) run() error {
 	return nil
 }
 
+func (r *commandRunner) runIgnoreErrors() error {
+	for _, command := range r.commands {
+		command.cmd.SysProcAttr = &syscall.SysProcAttr{
+			AmbientCaps: command.ambientCaps,
+		}
+
+		output, err := command.cmd.CombinedOutput()
+		if err != nil {
+			log.Printf("failed to %s: %v, output: %s", command.description, err, output)
+			continue
+		}
+	}
+
+	return nil
+}
+
 // configureHostNetworkBeforeCmdExec prepares host-side networking before the target
 // process is started. At this point the target process is not running, so its PID and network
 // namespace ID are not yet known.
@@ -283,7 +300,6 @@ func (l *LinuxJail) configureIptables() error {
 			exec.Command("iptables", "-t", "nat", "-A", "PREROUTING", "-i", l.vethHostName, "-p", "tcp", "-j", "REDIRECT", "--to-ports", fmt.Sprintf("%d", l.httpProxyPort)),
 			[]uintptr{uintptr(unix.CAP_NET_ADMIN)},
 		},
-		// TODO: clean up this rules
 		{
 			"iptables FORWARD -s",
 			exec.Command("iptables", "-A", "FORWARD", "-s", "192.168.100.0/24", "-j", "ACCEPT"),
@@ -314,12 +330,15 @@ func (l *LinuxJail) cleanupNetworking() error {
 		description string
 		command     *exec.Cmd
 	}{
-		{"delete veth pair", exec.Command("ip", "link", "del", vethHost)},
+		{
+			"delete veth pair",
+			exec.Command("ip", "link", "del", vethHost),
+		},
 	}
 
 	for _, command := range cleanupCmds {
 		if err := command.command.Run(); err != nil {
-			return fmt.Errorf("failed to %s: %v", command.description, err)
+			l.logger.Error("failed to execute command", "command", command.description, "error", err)
 		}
 	}
 
@@ -328,20 +347,30 @@ func (l *LinuxJail) cleanupNetworking() error {
 
 // cleanupIptables removes iptables rules
 func (l *LinuxJail) cleanupIptables() error {
-	// Remove comprehensive TCP redirect rule
-	cmd := exec.Command("iptables", "-t", "nat", "-D", "PREROUTING", "-i", l.vethHostName, "-p", "tcp", "-j", "REDIRECT", "--to-ports", fmt.Sprintf("%d", l.httpProxyPort))
-	err := cmd.Run()
-	if err != nil {
-		l.logger.Error("Failed to remove TCP redirect rule", "error", err)
-		// Continue with other cleanup even if this fails
-	}
-
-	// Remove NAT rule
-	cmd = exec.Command("iptables", "-t", "nat", "-D", "POSTROUTING", "-s", "192.168.100.0/24", "-j", "MASQUERADE")
-	err = cmd.Run()
-	if err != nil {
-		l.logger.Error("Failed to remove NAT rule", "error", err)
-		// Continue with other cleanup even if this fails
+	runner := newCommandRunner([]*command{
+		{
+			"Remove comprehensive TCP redirect rule",
+			exec.Command("iptables", "-t", "nat", "-D", "PREROUTING", "-i", l.vethHostName, "-p", "tcp", "-j", "REDIRECT", "--to-ports", fmt.Sprintf("%d", l.httpProxyPort)),
+			[]uintptr{uintptr(unix.CAP_NET_ADMIN)},
+		},
+		{
+			"Remove NAT rule",
+			exec.Command("iptables", "-t", "nat", "-D", "POSTROUTING", "-s", "192.168.100.0/24", "-j", "MASQUERADE"),
+			[]uintptr{uintptr(unix.CAP_NET_ADMIN)},
+		},
+		{
+			"Remove iptables FORWARD -s",
+			exec.Command("iptables", "-D", "FORWARD", "-s", "192.168.100.0/24", "-j", "ACCEPT"),
+			[]uintptr{uintptr(unix.CAP_NET_ADMIN)},
+		},
+		{
+			"Remove iptables FORWARD -d",
+			exec.Command("iptables", "-D", "FORWARD", "-d", "192.168.100.0/24", "-j", "ACCEPT"),
+			[]uintptr{uintptr(unix.CAP_NET_ADMIN)},
+		},
+	})
+	if err := runner.runIgnoreErrors(); err != nil {
+		return err
 	}
 
 	return nil