From b2b04f2660cfad3185d00cab465bd8ffb5fce0c8 Mon Sep 17 00:00:00 2001
From: Faisal Memon <fymemon@yahoo.com>
Date: Wed, 4 Dec 2024 17:38:50 -0800
Subject: [PATCH 1/3] Use mutex to ensure only one command is run

Signed-off-by: Faisal Memon <fymemon@yahoo.com>
---
 pkg/sidecar/sidecar.go      | 19 ++++++++++++-------
 pkg/sidecar/sidecar_test.go | 26 ++++++++++++++++++++++++++
 pkg/sidecar/sidecar_test.sh |  3 +++
 3 files changed, 41 insertions(+), 7 deletions(-)
 create mode 100755 pkg/sidecar/sidecar_test.sh

diff --git a/pkg/sidecar/sidecar.go b/pkg/sidecar/sidecar.go
index 891b79fb..a0fc16bd 100644
--- a/pkg/sidecar/sidecar.go
+++ b/pkg/sidecar/sidecar.go
@@ -10,7 +10,6 @@ import (
 	"strconv"
 	"strings"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/spiffe/go-spiffe/v2/bundle/jwtbundle"
@@ -27,9 +26,11 @@ type Sidecar struct {
 	config         *Config
 	client         *workloadapi.Client
 	jwtSource      *workloadapi.JWTSource
-	processRunning int32
+	processRunning bool
 	process        *os.Process
 	certReadyChan  chan struct{}
+
+	mu sync.Mutex
 }
 
 // New creates a new SPIFFE sidecar
@@ -200,7 +201,10 @@ func (s *Sidecar) updateCertificates(svidResponse *workloadapi.X509Context) {
 
 // signalProcessCMD sends the renew signal to the process or starts it if its first time
 func (s *Sidecar) signalProcess() error {
-	if atomic.LoadInt32(&s.processRunning) == 0 {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	if !s.processRunning {
 		cmdArgs, err := getCmdArgs(s.config.CmdArgs)
 		if err != nil {
 			return fmt.Errorf("error parsing cmd arguments: %w", err)
@@ -213,6 +217,7 @@ func (s *Sidecar) signalProcess() error {
 			return fmt.Errorf("error executing process \"%v\": %w", s.config.Cmd, err)
 		}
 		s.process = cmd.Process
+		s.processRunning = true
 		go s.checkProcessExit()
 	} else {
 		if err := SignalProcess(s.process, s.config.RenewSignal); err != nil {
@@ -244,13 +249,13 @@ func (s *Sidecar) signalPID() error {
 }
 
 func (s *Sidecar) checkProcessExit() {
-	atomic.StoreInt32(&s.processRunning, 1)
-	_, err := s.process.Wait()
-	if err != nil {
+	if _, err := s.process.Wait(); err != nil {
 		s.config.Log.Errorf("error waiting for process exit: %v", err)
 	}
 
-	atomic.StoreInt32(&s.processRunning, 0)
+	s.mu.Lock()
+	s.processRunning = false
+	s.mu.Unlock()
 }
 
 func (s *Sidecar) fetchJWTSVIDs(ctx context.Context, jwtAudience string, jwtExtraAudiences []string) (*jwtsvid.SVID, error) {
diff --git a/pkg/sidecar/sidecar_test.go b/pkg/sidecar/sidecar_test.go
index c462402b..361ada25 100644
--- a/pkg/sidecar/sidecar_test.go
+++ b/pkg/sidecar/sidecar_test.go
@@ -263,3 +263,29 @@ func TestGetCmdArgs(t *testing.T) {
 		})
 	}
 }
+
+// TestSignalProcess makes sure only one copy of the process is started. It uses a small script that creates a file
+// where the name is the process ID of the script. If more then one file exists, then multiple processes were started
+func TestSignalProcess(t *testing.T) {
+	tempDir := t.TempDir()
+	config := &Config{
+		Cmd:         "./sidecar_test.sh",
+		CmdArgs:     tempDir,
+		RenewSignal: "SIGWINCH",
+	}
+	sidecar := New(config)
+	require.NotNil(t, sidecar)
+
+	// Run signalProcess() twice. The second should only signal the process with SIGWINCH which is basically a no op.
+	err := sidecar.signalProcess()
+	require.NoError(t, err)
+	err = sidecar.signalProcess()
+	require.NoError(t, err)
+
+	// Give the script some time to run
+	time.Sleep(1 * time.Second)
+
+	files, err := os.ReadDir(tempDir)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(files))
+}
diff --git a/pkg/sidecar/sidecar_test.sh b/pkg/sidecar/sidecar_test.sh
new file mode 100755
index 00000000..374e7da0
--- /dev/null
+++ b/pkg/sidecar/sidecar_test.sh
@@ -0,0 +1,3 @@
+#!/bin/bash
+
+touch $1/$$

From 18e8cfb6cb550de753f4d3c2c0876505534d86be Mon Sep 17 00:00:00 2001
From: Faisal Memon <fymemon@yahoo.com>
Date: Mon, 3 Feb 2025 22:55:18 -0800
Subject: [PATCH 2/3] Fix merge error

---
 pkg/sidecar/sidecar.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/sidecar/sidecar.go b/pkg/sidecar/sidecar.go
index f53ea14b..b91eacb5 100644
--- a/pkg/sidecar/sidecar.go
+++ b/pkg/sidecar/sidecar.go
@@ -29,8 +29,8 @@ type Sidecar struct {
 	jwtSource      *workloadapi.JWTSource
 	processRunning bool
 	process        *os.Process
-  certReadyChan  chan struct{}
-  health         Health
+	certReadyChan  chan struct{}
+	health         Health
 
 	mu sync.Mutex
 

From 8b0cc24fa399532f1c853077a55deda99195d209 Mon Sep 17 00:00:00 2001
From: Faisal Memon <fymemon@yahoo.com>
Date: Mon, 3 Feb 2025 22:55:41 -0800
Subject: [PATCH 3/3] Fix merge error 2

---
 pkg/sidecar/sidecar.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pkg/sidecar/sidecar.go b/pkg/sidecar/sidecar.go
index b91eacb5..e6383698 100644
--- a/pkg/sidecar/sidecar.go
+++ b/pkg/sidecar/sidecar.go
@@ -33,7 +33,6 @@ type Sidecar struct {
 	health         Health
 
 	mu sync.Mutex
-
 }
 
 type Health struct {