🐛 fix: eliminate race condition in ISO 27001 parallel cluster audit (#3968)#3985
🐛 fix: eliminate race condition in ISO 27001 parallel cluster audit (#3968)#3985clubanderson merged 1 commit intomainfrom
Conversation
…3968) Replace shared mutable `accumulated` array with per-task return values. Each parallel task now returns its own findings array independently. Results are collected from settled promises via flatMap after all tasks complete, eliminating the possibility of interleaved push operations corrupting the array when cluster tasks resolve concurrently. Closes #3968 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
kubestellar-prow
bot
added
the
dco-signoff: yes
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubestellar-prow
bot
added
the
size/S
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
There was a problem hiding this comment.
Pull request overview
Fixes a concurrency-related correctness issue in the ISO 27001 audit fetcher by removing shared accumulation of findings across parallel cluster tasks.
Changes:
- Refactors
fetchISO27001AuditViaKubectlso each parallel cluster task returns its ownISO27001Finding[]rather than mutating a sharedaccumulatedarray. - Collects final findings by flattening fulfilled results from
settledWithConcurrency. - Keeps progressive
onProgressupdates by aggregating completed cluster results.
| // Each task returns its own findings array to avoid shared-state mutation. | ||
| // Progressive updates are built by collecting all fulfilled results so far. |
There was a problem hiding this comment.
The comment says this avoids shared-state mutation, but completed is still shared mutable state that every parallel task mutates via completed.push(...). Either update the comment to match what the code is actually doing, or switch to an immutable accumulator approach for progress (e.g., reassign a partial array with concat / spread into a new array each time) so there’s no shared in-place mutation at all.
| // Each task returns its own findings array to avoid shared-state mutation. | |
| // Progressive updates are built by collecting all fulfilled results so far. | |
| // Each task returns its own findings array; we accumulate results in a shared | |
| // `completed` array and build progressive updates by collecting all fulfilled | |
| // results so far. |
| const clusterFindings = await runISO27001ChecksForCluster(name, context || name) | ||
| accumulated.push(...clusterFindings) | ||
| onProgress?.([...accumulated]) | ||
| completed.push(clusterFindings) | ||
| onProgress?.(completed.flat()) | ||
| return clusterFindings |
There was a problem hiding this comment.
onProgress?.(completed.flat()) emits findings in completion order (because of completed.push(...)), but the final return value is flattened in cluster/task order via settledWithConcurrency’s indexed results. That means the UI can reorder/shuffle findings when the final result replaces the progressive data. Consider storing per-task results by index (e.g., completed[idx] = clusterFindings) and building progress by flattening indices in order, so progressive updates and the final payload have consistent ordering.
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 1 code suggestion(s) and 1 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
Summary
accumulated) being mutated concurrently from parallel cluster tasks infetchISO27001AuditViaKubectlflatMap, eliminating interleavedpushcorruptiononProgressupdates still work by flattening all completed results so farTest plan
Closes #3968