fix(sources): invalidate cache when modifying sources [BE-13146] (#3050)

api/http/handler/gitops/sources/cache_test.go

@@ -0,0 +1,172 @@
+package sources
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	portainer "github.com/portainer/portainer/api"
+	"github.com/portainer/portainer/api/dataservices"
+	"github.com/portainer/portainer/api/datastore"
+	gittypes "github.com/portainer/portainer/api/git/types"
+
+	"github.com/segmentio/encoding/json"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// Each test below primes the source cache with a read before mutating, then
+// reads again and asserts the change is reflected. The handler is built with a
+// non-expiring cache (newTestHandlerNoCacheExpiry), so a fresh result can only
+// come from invalidation on the write — not from the TTL elapsing.
+
+func TestGitSourceCreate_InvalidatesCache(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	user := &portainer.User{ID: 1, Role: portainer.AdministratorRole}
+	require.NoError(t, store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		return tx.User().Create(user)
+	}))
+
+	h := newTestHandlerNoCacheExpiry(t, store)
+
+	// prime the cache with the (empty) list
+	rr := httptest.NewRecorder()
+	h.ServeHTTP(rr, buildListReq(t, user.ID, ""))
+	require.Empty(t, decodeSources(t, rr))
+
+	body, err := json.Marshal(GitSourceCreatePayload{
+		URL:  "https://github.com/org/repo.git",
+		Name: "my-source",
+	})
+	require.NoError(t, err)
+
+	rr = httptest.NewRecorder()
+	h.ServeHTTP(rr, buildCreateReq(t, user.ID, body))
+	require.Equal(t, http.StatusCreated, rr.Code)
+
+	// the new source must appear despite the primed empty cache
+	rr = httptest.NewRecorder()
+	h.ServeHTTP(rr, buildListReq(t, user.ID, ""))
+	got := decodeSources(t, rr)
+	require.Len(t, got, 1)
+	require.Equal(t, "my-source", got[0].Name)
+}
+
+func TestSourceDelete_InvalidatesCache(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	var srcID portainer.SourceID
+	user := &portainer.User{ID: 1, Role: portainer.AdministratorRole}
+	require.NoError(t, store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		src := &portainer.Source{Name: "to-delete", Type: portainer.SourceTypeGit, Git: &gittypes.GitSource{URL: "https://github.com/org/repo"}}
+		require.NoError(t, tx.Source().Create(adminUserContext, src))
+		srcID = src.ID
+
+		return tx.User().Create(user)
+	}))
+
+	h := newTestHandlerNoCacheExpiry(t, store)
+
+	// prime the cache with the source present
+	rr := httptest.NewRecorder()
+	h.ServeHTTP(rr, buildListReq(t, user.ID, ""))
+	require.Len(t, decodeSources(t, rr), 1)
+
+	rr = httptest.NewRecorder()
+	h.ServeHTTP(rr, buildDeleteReq(t, user.ID, int(srcID)))
+	require.Equal(t, http.StatusNoContent, rr.Code)
+
+	// the deleted source must be gone despite the primed cache
+	rr = httptest.NewRecorder()
+	h.ServeHTTP(rr, buildListReq(t, user.ID, ""))
+	require.Empty(t, decodeSources(t, rr))
+}
+
+func TestGitSourceUpdate_InvalidatesCache(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	var srcID portainer.SourceID
+	user := &portainer.User{ID: 1, Role: portainer.AdministratorRole}
+	require.NoError(t, store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		src := &portainer.Source{Name: "old-name", Type: portainer.SourceTypeGit, Git: &gittypes.GitSource{URL: "https://github.com/org/repo"}}
+		require.NoError(t, tx.Source().Create(adminUserContext, src))
+		srcID = src.ID
+
+		return tx.User().Create(user)
+	}))
+
+	h := newTestHandlerNoCacheExpiry(t, store)
+
+	// prime the cache with the original name
+	rr := httptest.NewRecorder()
+	h.ServeHTTP(rr, buildListReq(t, user.ID, ""))
+	primed := decodeSources(t, rr)
+	require.Len(t, primed, 1)
+	require.Equal(t, "old-name", primed[0].Name)
+
+	body, err := json.Marshal(GitSourceUpdatePayload{Name: new("new-name")})
+	require.NoError(t, err)
+
+	rr = httptest.NewRecorder()
+	h.ServeHTTP(rr, buildUpdateReq(t, user.ID, int(srcID), body))
+	require.Equal(t, http.StatusOK, rr.Code)
+
+	// the updated name must be reflected despite the primed cache
+	rr = httptest.NewRecorder()
+	h.ServeHTTP(rr, buildListReq(t, user.ID, ""))
+	got := decodeSources(t, rr)
+	require.Len(t, got, 1)
+	require.Equal(t, "new-name", got[0].Name)
+}
+
+// TestSourceMutation_InvalidatesSummaryCache verifies the summary endpoint, which
+// shares the cache entry with the list via getSources, also sees fresh data.
+func TestSourceMutation_InvalidatesSummaryCache(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	user := &portainer.User{ID: 1, Role: portainer.AdministratorRole}
+	require.NoError(t, store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		return tx.User().Create(user)
+	}))
+
+	h := newTestHandlerNoCacheExpiry(t, store)
+
+	// prime the cache with the empty summary
+	rr := httptest.NewRecorder()
+	h.ServeHTTP(rr, buildSummaryReq(t, user.ID))
+	require.Equal(t, 0, summaryTotal(t, rr))
+
+	body, err := json.Marshal(GitSourceCreatePayload{
+		URL:  "https://github.com/org/repo.git",
+		Name: "my-source",
+	})
+	require.NoError(t, err)
+
+	rr = httptest.NewRecorder()
+	h.ServeHTTP(rr, buildCreateReq(t, user.ID, body))
+	require.Equal(t, http.StatusCreated, rr.Code)
+
+	// the summary count must reflect the new source despite the primed cache
+	rr = httptest.NewRecorder()
+	h.ServeHTTP(rr, buildSummaryReq(t, user.ID))
+	assert.Equal(t, 1, summaryTotal(t, rr))
+}
+
+func summaryTotal(t *testing.T, rr *httptest.ResponseRecorder) int {
+	t.Helper()
+	require.Equal(t, http.StatusOK, rr.Code, "unexpected status: %s", rr.Body.String())
+
+	var summary map[string]int
+	require.NoError(t, json.NewDecoder(rr.Body).Decode(&summary))
+
+	total := 0
+	for _, count := range summary {
+		total += count
+	}
+	return total
+}

api/http/handler/gitops/sources/create_git.go

@@ -89,6 +89,8 @@ func (h *Handler) gitSourceCreate(w http.ResponseWriter, r *http.Request) *httpe
 		return httperror.InternalServerError("Unable to create source", err)
 	}
 
+	h.invalidateCache()
+
 	src.Git = gittypes.SanitizeGitSource(src.Git)
 
 	return response.JSONWithStatus(w, src, http.StatusCreated)

api/http/handler/gitops/sources/delete.go

@@ -91,5 +91,7 @@ func (h *Handler) sourceDelete(w http.ResponseWriter, r *http.Request) *httperro
 		return httperror.InternalServerError("Unable to delete source", err)
 	}
 
+	h.invalidateCache()
+
 	return response.Empty(w)
 }

api/http/handler/gitops/sources/handler.go

@@ -54,3 +54,9 @@ func NewHandler(bouncer security.BouncerService, dataStore dataservices.DataStor
 
 	return h
 }
+
+// invalidateCache clears the cached source lists so the next read reflects
+// the latest datastore state. Called after any mutating operation.
+func (h *Handler) invalidateCache() {
+	h.cache.Flush()
+}

api/http/handler/gitops/sources/helpers_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/portainer/portainer/api/http/security"
 	"github.com/portainer/portainer/api/internal/testhelpers"
 
+	gocache "github.com/patrickmn/go-cache"
 	"github.com/segmentio/encoding/json"
 	"github.com/stretchr/testify/require"
 )
@@ -50,6 +51,16 @@ func newTestHandler(t *testing.T, store dataservices.DataStore) *Handler {
 	return NewHandler(testhelpers.NewTestRequestBouncer(), store, nil, nil)
 }
 
+// newTestHandlerNoCacheExpiry returns a handler whose source cache never expires,
+// so cache-invalidation tests can prove a write clears the cache rather than the
+// TTL simply elapsing between requests.
+func newTestHandlerNoCacheExpiry(t *testing.T, store dataservices.DataStore) *Handler {
+	t.Helper()
+	h := newTestHandler(t, store)
+	h.cache = gocache.New(gocache.NoExpiration, cacheCleanupInterval)
+	return h
+}
+
 func adminRestrictedContext(userID portainer.UserID) *security.RestrictedRequestContext {
 	return &security.RestrictedRequestContext{
 		UserID:  userID,
@@ -58,20 +69,25 @@ func adminRestrictedContext(userID portainer.UserID) *security.RestrictedRequest
 	}
 }
 
-func buildListReq(t *testing.T, userID portainer.UserID, query string) *http.Request {
-	t.Helper()
-	req := httptest.NewRequest(http.MethodGet, "/gitops/sources?"+query, nil)
+// withSecurityContext attaches the admin token data and restricted request
+// context for userID to req, mirroring what the auth middleware sets up in
+// production.
+func withSecurityContext(req *http.Request, userID portainer.UserID) *http.Request {
 	req = req.WithContext(security.StoreTokenData(req, &portainer.TokenData{ID: userID}))
 	req = req.WithContext(security.StoreRestrictedRequestContext(req, adminRestrictedContext(userID)))
 	return req
 }
 
+func buildListReq(t *testing.T, userID portainer.UserID, query string) *http.Request {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodGet, "/gitops/sources?"+query, nil)
+	return withSecurityContext(req, userID)
+}
+
 func buildGetReq(t *testing.T, userID portainer.UserID, id string) *http.Request {
 	t.Helper()
 	req := httptest.NewRequest(http.MethodGet, "/gitops/sources/"+id, nil)
-	req = req.WithContext(security.StoreTokenData(req, &portainer.TokenData{ID: userID}))
-	req = req.WithContext(security.StoreRestrictedRequestContext(req, adminRestrictedContext(userID)))
-	return req
+	return withSecurityContext(req, userID)
 }
 
 func decodeSources(t *testing.T, rr *httptest.ResponseRecorder) []Source {
@@ -98,49 +114,37 @@ func buildCreateReq(t *testing.T, userID portainer.UserID, body []byte) *http.Re
 	t.Helper()
 	req := httptest.NewRequest(http.MethodPost, "/gitops/sources/git", bytes.NewReader(body))
 	req.Header.Set("Content-Type", "application/json")
-	req = req.WithContext(security.StoreTokenData(req, &portainer.TokenData{ID: userID}))
-	req = req.WithContext(security.StoreRestrictedRequestContext(req, adminRestrictedContext(userID)))
-	return req
+	return withSecurityContext(req, userID)
 }
 
 func buildUpdateReq(t *testing.T, userID portainer.UserID, id int, body []byte) *http.Request {
 	t.Helper()
 	req := httptest.NewRequest(http.MethodPut, fmt.Sprintf("/gitops/sources/%d", id), bytes.NewReader(body))
 	req.Header.Set("Content-Type", "application/json")
-	req = req.WithContext(security.StoreTokenData(req, &portainer.TokenData{ID: userID}))
-	req = req.WithContext(security.StoreRestrictedRequestContext(req, adminRestrictedContext(userID)))
-	return req
+	return withSecurityContext(req, userID)
 }
 
 func buildDeleteReq(t *testing.T, userID portainer.UserID, id int) *http.Request {
 	t.Helper()
 	req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/gitops/sources/%d", id), nil)
-	req = req.WithContext(security.StoreTokenData(req, &portainer.TokenData{ID: userID}))
-	req = req.WithContext(security.StoreRestrictedRequestContext(req, adminRestrictedContext(userID)))
-	return req
+	return withSecurityContext(req, userID)
 }
 
 func buildSummaryReq(t *testing.T, userID portainer.UserID) *http.Request {
 	t.Helper()
 	req := httptest.NewRequest(http.MethodGet, "/gitops/sources/summary", nil)
-	req = req.WithContext(security.StoreTokenData(req, &portainer.TokenData{ID: userID}))
-	req = req.WithContext(security.StoreRestrictedRequestContext(req, adminRestrictedContext(userID)))
-	return req
+	return withSecurityContext(req, userID)
 }
 
 func buildUpdateReqWithRawID(t *testing.T, userID portainer.UserID, id string, body []byte) *http.Request {
 	t.Helper()
 	req := httptest.NewRequest(http.MethodPut, "/gitops/sources/"+id, bytes.NewReader(body))
 	req.Header.Set("Content-Type", "application/json")
-	req = req.WithContext(security.StoreTokenData(req, &portainer.TokenData{ID: userID}))
-	req = req.WithContext(security.StoreRestrictedRequestContext(req, adminRestrictedContext(userID)))
-	return req
+	return withSecurityContext(req, userID)
 }
 
 func buildDeleteReqWithRawID(t *testing.T, userID portainer.UserID, id string) *http.Request {
 	t.Helper()
 	req := httptest.NewRequest(http.MethodDelete, "/gitops/sources/"+id, nil)
-	req = req.WithContext(security.StoreTokenData(req, &portainer.TokenData{ID: userID}))
-	req = req.WithContext(security.StoreRestrictedRequestContext(req, adminRestrictedContext(userID)))
-	return req
+	return withSecurityContext(req, userID)
 }

api/http/handler/gitops/sources/update_access.go

@@ -73,6 +73,8 @@ func (h *Handler) gitSourceUpdateAccess(w http.ResponseWriter, r *http.Request)
 		return httperror.InternalServerError("Unable to update source access", err)
 	}
 
+	h.invalidateCache()
+
 	return response.JSON(w, src)
 }
 

api/http/handler/gitops/sources/update_git.go

@@ -106,6 +106,8 @@ func (h *Handler) gitSourceUpdate(w http.ResponseWriter, r *http.Request) *httpe
 		return httperror.InternalServerError("Unable to update source", err)
 	}
 
+	h.invalidateCache()
+
 	src.Git = gittypes.SanitizeGitSource(src.Git)
 
 	return response.JSON(w, src)