From 011797428c5139ec6195f08a491e168fd43c9aa6 Mon Sep 17 00:00:00 2001 From: Vishakha7-Kumari Date: Tue, 1 Sep 2026 00:32:48 +0530 Subject: [PATCH] fix(plugin/kubernetes): prevent data race on applications map in livestate store Signed-off-by: Vishakha7-Kumari --- .../kubernetes/livestate/store/store.go | 11 +++- .../kubernetes/livestate/store/store_test.go | 65 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 pkg/app/pipedv1/plugin/kubernetes/livestate/store/store_test.go diff --git a/pkg/app/pipedv1/plugin/kubernetes/livestate/store/store.go b/pkg/app/pipedv1/plugin/kubernetes/livestate/store/store.go index 57b3423a4d..cf308a7326 100644 --- a/pkg/app/pipedv1/plugin/kubernetes/livestate/store/store.go +++ b/pkg/app/pipedv1/plugin/kubernetes/livestate/store/store.go @@ -223,9 +223,16 @@ func (s *deployTargetResources) initialize() { // getApplicationResources returns the application resources by the application ID. func (s *deployTargetResources) getApplicationResources(appID string) *applicationResources { s.mu.RLock() - defer s.mu.RUnlock() - app, ok := s.applications[appID] + s.mu.RUnlock() + if ok { + return app + } + + s.mu.Lock() + defer s.mu.Unlock() + + app, ok = s.applications[appID] if !ok { app = newApplicationResources(s.deployTarget) s.applications[appID] = app diff --git a/pkg/app/pipedv1/plugin/kubernetes/livestate/store/store_test.go b/pkg/app/pipedv1/plugin/kubernetes/livestate/store/store_test.go new file mode 100644 index 0000000000..e3c8173cbf --- /dev/null +++ b/pkg/app/pipedv1/plugin/kubernetes/livestate/store/store_test.go @@ -0,0 +1,65 @@ +// Copyright 2026 The PipeCD Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package store + +import ( + "fmt" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetApplicationResources(t *testing.T) { + dtr := newDeployTargetResources("target-1") + + app1 := dtr.getApplicationResources("app-1") + require.NotNil(t, app1) + assert.Equal(t, "target-1", app1.deployTarget) + + // Fetching the same appID returns the same instance + app1Again := dtr.getApplicationResources("app-1") + assert.Same(t, app1, app1Again) + + // Fetching a different appID returns a distinct instance + app2 := dtr.getApplicationResources("app-2") + require.NotNil(t, app2) + assert.NotSame(t, app1, app2) +} + +func TestGetApplicationResources_Concurrent(t *testing.T) { + dtr := newDeployTargetResources("target-concurrent") + + var wg sync.WaitGroup + workers := 100 + + for i := 0; i < workers; i++ { + wg.Add(1) + go func(workerID int) { + defer wg.Done() + appID := fmt.Sprintf("app-%d", workerID%10) + app := dtr.getApplicationResources(appID) + assert.NotNil(t, app) + }(i) + } + + wg.Wait() + + // Verify all 10 application entries exist in the map + dtr.mu.RLock() + defer dtr.mu.RUnlock() + assert.Len(t, dtr.applications, 10) +}