[odysseymorphey]

Fixes #435

ReferencedSecrets is written by Reconcile and read by the Secret watch map function, which run in different goroutines. A Secret event that arrives while a reconcile is updating the map makes the Go runtime abort the operator with fatal error: concurrent map read and map write.

Changes

• Guard ReferencedSecrets with a sync.RWMutex: write lock in Reconcile, read lock in the map function.
• Move the map function out of SetupWithManager into the mapSecretToNBSetupKey method so it can be tested.
• Add a test that runs Reconcile and mapSecretToNBSetupKey concurrently.

Testing

• Without the mutex the new test fails: plain go test hits the same fatal error as in the issue in 4 of 5 runs, and go test -race reports the data race every time.
• With the mutex: 10 of 10 plain runs and 3 of 3 -race runs of the NBSetupKey specs pass.
• make lint, make test-unit, go mod tidy and make -B generate pass with no extra changes.

Could a maintainer add the changelog/fixed label?

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability when setup-key updates and Secret events are processed at the same time, helping ensure the correct setup-key request is associated with a Secret.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Enterprise
• Run ID: eeeaac0c-7999-4719-96de-0a842ebc80a9

📥 Commits

Reviewing files that changed from the base of the PR and between f0694a6 and de8d45d.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 91784cd2-f6e8-4aab-b72b-3bcc88eb117e

📥 Commits

Reviewing files that changed from the base of the PR and between 627e388 and f0694a6.

📒 Files selected for processing (2)

• internal/controller/nbsetupkey_controller.go
• internal/controller/nbsetupkey_controller_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The NBSetupKey controller now protects its referenced Secret map with a read-write mutex. A concurrency test runs reconciliation alongside Secret event mapping and checks the final mapping.

Changes: Secret mapping synchronization

Layer / File(s)	Summary
Synchronize Secret mapping access internal/controller/nbsetupkey_controller.go, internal/controller/nbsetupkey_controller_test.go	Reconciliation locks the removal and update of Secret mappings. The Secret watch callback uses a read lock and returns the mapped setup-key request when present. A concurrency test checks mapping during repeated reconciliation and verifies the final request.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to f0694

The changed controller paths synchronize Secret mapping access. No issue requiring a fix before merge was identified.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The change satisfies issue [#435]. Reconcile protects deletion and insertion in ReferencedSecrets with referencedSecretsMu.Lock. mapSecretToNBSetupKey protects lookup with RLock. `SetupWithM…
Out of Scope Changes check	✅ Passed	The reviewed changes stay within issue [#435]. They add synchronization for ReferencedSecrets, extract the Secret mapping callback for use and testing, and add the related concurrency regression tes…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: fixing concurrent map access in the NBSetupKey controller.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

_{A rabbit watched the secrets flow,
While locks kept reads and writes in tow.
Reconcile ran, events came by,
The map stayed safe as time went by.
Then hopped the rabbit, pleased to see.}

_{Comment @coderabbitai help to get the list of available commands.}