Skip to content

Fix data races in built-in BundlePublisher plugins on dynamic reconfiguration#7082

Merged
sorindumitru merged 1 commit into
spiffe:mainfrom
amartinezfayo:fix-bundlepublisher-client-race
Jun 19, 2026
Merged

Fix data races in built-in BundlePublisher plugins on dynamic reconfiguration#7082
sorindumitru merged 1 commit into
spiffe:mainfrom
amartinezfayo:fix-bundlepublisher-client-race

Conversation

@amartinezfayo

@amartinezfayo amartinezfayo commented Jun 18, 2026

Copy link
Copy Markdown
Member

The built-in BundlePublisher plugins assign their storage client in Configure and read it in PublishBundle without any synchronization. Several of them also read the cached bundle in getBundle while holding configMtx, even though setBundle writes it under a separate bundleMtx, so the two never exclude each other.

These were harmless while Configure only ran once during plugin initialization, but dynamic reconfiguration now calls Configure again at runtime, so it can run concurrently with PublishBundle. That produces data races on both the client field and the cached bundle, and a concurrent reconfiguration could also leave PublishBundle using a client that no longer matches the active configuration.

This PR fixes the aws_s3, gcp_cloudstorage, and aws_rolesanywhere_trustanchor plugins by storing the configuration and its client together and updating them atomically under configMtx, with getConfig returning both so that PublishBundle always observes a matching pair. It also corrects getBundle in those plugins to read under bundleMtx, matching setBundle. The k8s_configmap plugin already stores its clients inside the configuration and reads the bundle under the correct lock, so it needs no change. The azure_blob plugin under review in #7030 has the same structure and adopts this pattern there.

Changes:

  • Store the config and storage client as a pair under configMtx in the aws_s3, gcp_cloudstorage, and aws_rolesanywhere_trustanchor plugins, returned together from getConfig.
  • Read the cached bundle under bundleMtx in getBundle to match setBundle in those plugins.
  • Guard the client read in the gcp_cloudstorage Close method.
  • Add a TestConcurrentConfigureAndPublish regression test to each fixed plugin that runs Configure and PublishBundle concurrently and fails under -race without the fix.

@amartinezfayo amartinezfayo added this to the 1.15.2 milestone Jun 18, 2026
@amartinezfayo amartinezfayo force-pushed the fix-bundlepublisher-client-race branch from c259bb1 to 368812d Compare June 18, 2026 14:28
…guration

Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
@amartinezfayo amartinezfayo changed the title Fix data races in the aws_s3 and gcp_cloudstorage BundlePublisher plugins on dynamic reconfiguration Fix data races in built-in BundlePublisher plugins on dynamic reconfiguration Jun 18, 2026
@amartinezfayo amartinezfayo force-pushed the fix-bundlepublisher-client-race branch from 368812d to 3b7a874 Compare June 18, 2026 14:44
@amartinezfayo amartinezfayo requested a review from Copilot June 18, 2026 15:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes data races in built-in BundlePublisher plugins during dynamic reconfiguration by ensuring configuration/client are updated and read atomically, and by using the correct mutex for cached bundle access.

Changes:

  • Atomically store and retrieve (config, client) under configMtx in aws_s3, gcp_cloudstorage, and aws_rolesanywhere_trustanchor.
  • Fix getBundle to read under bundleMtx (matching setBundle) in the affected plugins.
  • Add concurrent Configure vs PublishBundle regression tests intended to fail under -race without the fix.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/server/plugin/bundlepublisher/gcpcloudstorage/gcpcloudstorage.go Atomically pairs config+client under configMtx; fixes bundle locking; guards client read in Close.
pkg/server/plugin/bundlepublisher/gcpcloudstorage/gcpcloudstorage_test.go Adds concurrent Configure/Publish regression test.
pkg/server/plugin/bundlepublisher/awss3/awss3.go Atomically pairs config+client under configMtx; fixes bundle locking.
pkg/server/plugin/bundlepublisher/awss3/awss3_test.go Adds concurrent Configure/Publish regression test.
pkg/server/plugin/bundlepublisher/awsrolesanywhere/awsrolesanywhere.go Atomically pairs config+client under configMtx; fixes bundle locking; introduces setConfig.
pkg/server/plugin/bundlepublisher/awsrolesanywhere/awsrolesanywhere_test.go Adds concurrent Configure/Publish regression test.

Comment thread pkg/server/plugin/bundlepublisher/awss3/awss3_test.go
@amartinezfayo amartinezfayo marked this pull request as ready for review June 18, 2026 18:39
@amartinezfayo amartinezfayo removed this from the 1.15.2 milestone Jun 18, 2026
@github-actions github-actions Bot assigned sorindumitru and unassigned MarcosDY Jun 18, 2026
@sorindumitru sorindumitru enabled auto-merge June 18, 2026 20:13
@sorindumitru sorindumitru added this pull request to the merge queue Jun 18, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 18, 2026
@sorindumitru sorindumitru added this to the 1.15.2 milestone Jun 19, 2026
@sorindumitru sorindumitru added this pull request to the merge queue Jun 19, 2026
Merged via the queue into spiffe:main with commit 06cf96c Jun 19, 2026
157 of 161 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants