fix: support concurrent access in system report endpoint

Previously, concurrent access to the global system report endpoint
variable resulted in a data race and triggered the race detector.

This commit makes the variable safe for concurrent use.

Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>

system-reports/datastructures/basic_report.go

@@ -11,23 +11,73 @@ import (
 	"github.com/armosec/utils-go/httputils"
 )
 
-var (
-	systemReportEndpoint = ""
+const (
+	defaultSystemReportEndpoint = "/k8s/sysreport"
 )
 
-func SetSystemReportEndpoint(endpoint string) {
-	if endpoint == "" {
-		endpoint = "/k8s/sysreport"
+type sysEndpoint struct {
+	value string
+	mu    sync.RWMutex
+}
+
+func (e *sysEndpoint) IsEmpty() bool {
+	return e.Get() == ""
+}
+
+func (e *sysEndpoint) Set(value string) {
+	e.mu.Lock()
+	e.value = value
+	e.mu.Unlock()
+}
+
+func (e *sysEndpoint) Get() string {
+	e.mu.RLock()
+	defer e.mu.RUnlock()
+	return e.value
+}
+
+// SetOrDefault sets the system report endpoint to the provided value for valid
+// inputs, or to a default value on invalid ones
+//
+// An empty input is considered invalid, and would thus be set to a default endpoint
+func (e *sysEndpoint) SetOrDefault(value string) {
+	if value == "" {
+		value = defaultSystemReportEndpoint
 	}
-	systemReportEndpoint = endpoint
+	e.Set(value)
+}
 
+// GetOrDefault returns the value of the current system report endpoint if it
+// is set. If not set, it sets the value to a default and returns the newly set
+// value
+func (e *sysEndpoint) GetOrDefault() string {
+	current := e.Get()
+	if current == "" {
+		e.SetOrDefault("")
+	}
+	return e.Get()
 }
 
+var (
+	systemReportEndpoint = &sysEndpoint{}
+)
+
+// SetSystemReportEndpoint sets the system report endpoint to the provided
+// value or a default
+//
+// Deprecated: the SetSysetmReportEndpoint shouldn’t be the setter for an
+// unexported variable. Use the sysEndpoint.SetOrDefault() method instead.
+func SetSystemReportEndpoint(endpoint string) {
+	systemReportEndpoint.SetOrDefault(endpoint)
+}
+
+// GetSystemReportEndpoint gets the system report endpoint or sets a default
+// and returns it
+//
+// Deprecated: the GetSysetmReportEndpoint shouldn’t be the setter for an
+// unexported variable. Use the sysEndpoint.GetOrDefault() method instead.
 func GetSystemReportEndpoint() string {
-	if systemReportEndpoint == "" {
-		SetSystemReportEndpoint("")
-	}
-	return systemReportEndpoint
+	return systemReportEndpoint.GetOrDefault()
 }
 
 // JobsAnnotations job annotation

system-reports/datastructures/datastructures_test.go

@@ -16,6 +16,169 @@ import (
 	"github.com/francoispqt/gojay"
 )
 
+func TestSetSystemReportEndpoint(t *testing.T) {
+	tt := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{
+			name:  "valid value is set",
+			input: "/k8s/sysreport-test",
+			want:  "/k8s/sysreport-test",
+		},
+		{
+			name:  "empty value is set to default",
+			input: "",
+			want:  "/k8s/sysreport",
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			SetSystemReportEndpoint(tc.input)
+
+			got := systemReportEndpoint.Get()
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}
+
+func TestGetSystemReportEndpoint(t *testing.T) {
+	tt := []struct {
+		name          string
+		previousValue string
+		want          string
+		wantAfter     string
+	}{
+		{
+			name:          "previously set value is returned",
+			previousValue: "/k8s/sysreport-test",
+			want:          "/k8s/sysreport-test",
+			wantAfter:     "/k8s/sysreport-test",
+		},
+		{
+			name:          "no previous value returns default",
+			previousValue: "",
+			want:          "/k8s/sysreport",
+			wantAfter:     "/k8s/sysreport",
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			systemReportEndpoint.Set(tc.previousValue)
+
+			got := GetSystemReportEndpoint()
+			gotAfter := systemReportEndpoint.Get()
+			assert.Equal(t, tc.want, got)
+			assert.Equalf(t, tc.wantAfter, gotAfter, "default value has not been set after getting with default")
+		})
+	}
+}
+
+func TestSystemReportEndpointSetOrDefault(t *testing.T) {
+	tt := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{
+			name:  "valid value is set",
+			input: "/k8s/sysreport-test",
+			want:  "/k8s/sysreport-test",
+		},
+		{
+			name:  "empty value is set to default",
+			input: "",
+			want:  "/k8s/sysreport",
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			systemReportEndpoint.SetOrDefault(tc.input)
+
+			got := systemReportEndpoint.Get()
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}
+
+func TestSystemReportEndpointGetOrDefault(t *testing.T) {
+	tt := []struct {
+		name          string
+		previousValue string
+		want          string
+		wantAfter     string
+	}{
+		{
+			name:          "previously set value is returned",
+			previousValue: "/k8s/sysreport-test",
+			want:          "/k8s/sysreport-test",
+			wantAfter:     "/k8s/sysreport-test",
+		},
+		{
+			name:          "no previous value returns default",
+			previousValue: "",
+			want:          "/k8s/sysreport",
+			wantAfter:     "/k8s/sysreport",
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			systemReportEndpoint.Set(tc.previousValue)
+
+			got := systemReportEndpoint.GetOrDefault()
+			gotAfter := systemReportEndpoint.Get()
+			assert.Equal(t, tc.want, got)
+			assert.Equalf(t, tc.wantAfter, gotAfter, "default value has not been set after getting with default")
+		})
+	}
+}
+
+func TestSystemReportEndpointIsEmpty(t *testing.T) {
+	tt := []struct {
+		name  string
+		value string
+		want  bool
+	}{
+		{
+			name:  "empty string as endpoint value is considered empty",
+			value: "",
+			want:  true,
+		},
+		{
+			name:  "non-empty string as endpoint value is considered empty",
+			value: "/k8s/sysreport",
+			want:  false,
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			systemReportEndpoint.Set(tc.value)
+
+			got := systemReportEndpoint.IsEmpty()
+
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}
+
+// When run with `go test -race`, tests that concurrent access to the system report endpoint is safe
+func TestSetSystemReportEndpointConcurrent(t *testing.T) {
+	go SetSystemReportEndpoint("a")
+	go SetSystemReportEndpoint("b")
+}
+
+// When run with `go test -race`, tests that concurrent access to the system report endpoint is safe
+func TestGetSystemReportEndpointConcurrent(t *testing.T) {
+	go GetSystemReportEndpoint()
+	go GetSystemReportEndpoint()
+}
+
 func TestBaseReportStructure(t *testing.T) {
 	// a := BaseReport{Reporter: "unit-test", Target: "unit-test-framework", JobID: "id", ActionID: "id2"}
 	// timestamp := a.Timestamp

system-reports/datastructures/methods.go

@@ -117,7 +117,7 @@ func (report *BaseReport) GetReportID() string {
 
 // Send - send http request. returns-> http status code, return message (jobID/OK), http/go error
 func (report *BaseReport) Send() (int, string, error) {
-	url := report.eventReceiverUrl + GetSystemReportEndpoint()
+	url := report.eventReceiverUrl + systemReportEndpoint.GetOrDefault()
 	report.Timestamp = time.Now()
 	if report.ActionID == "" {
 		report.ActionID = "1"