Skip to content

fix(msk): isolate MSK resources by account and region - #3353

Merged
pgermosen merged 5 commits into
floci-io:mainfrom
nkootstra:fix/msk-resource-identity
Sep 10, 2026
Merged

fix(msk): isolate MSK resources by account and region#3353
pgermosen merged 5 commits into
floci-io:mainfrom
nkootstra:fix/msk-resource-identity

Conversation

@nkootstra

Copy link
Copy Markdown
Contributor

Summary

MSK broker logs and legacy host-persistent storage are now isolated by AWS account and region. This prevents resources with the same cluster name in different environments from sharing log streams or data directories.

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

AWS Compatibility

MSK cluster identity includes the account and region. Floci now uses that identity when attaching broker logs and constructing the legacy host storage path, while preserving the existing cluster name in user-facing log labels and container labels.

Checklist

  • ./mvnw test passes locally
  • New or updated integration test added
  • Commit messages follow Conventional Commits

@github-actions

Copy link
Copy Markdown

🎉 Thanks for your first pull request to Floci!

Your CI checks need a maintainer to approve them before they run. That is GitHub's standard gate on first-time contributors, not a problem with your PR — so if the checks look like they are doing nothing, that is why. Once a maintainer approves, CI and the compatibility suite start automatically. Nothing is needed from you in the meantime.

While you wait, a couple of things that make review faster:

  • Link the issue this fixes with Closes #N in the description
  • Commits follow Conventional Commits (feat(s3): ..., fix(dynamodb): ...)
  • Behaviour changes come with a test — see CONTRIBUTING.md

Come join us in Slack — it is the fastest way to reach maintainers if you get stuck, or want feedback on an approach before investing more time in it.

@nkootstra
nkootstra marked this pull request as ready for review September 10, 2026 06:32
@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR isolates MSK resources by AWS account and region while retaining compatibility with persisted legacy clusters.

  • Builds new cluster ARNs from the request region and persists that region with each cluster.
  • Limits cluster discovery and operations to the current region while keeping unmarked legacy records accessible.
  • Uses account-and-region-aware identities for broker logs, container labels, and host storage paths.
  • Preserves legacy host-path data only for clusters that predate the regional marker.
  • Adds regression coverage for regional isolation and legacy storage compatibility.

Confidence Score: 5/5

The PR appears safe to merge, with all previous findings resolved and no new actionable issues introduced since the previous review.

The current implementation preserves legacy cluster visibility and broker data while ensuring newly created clusters use account-and-region-scoped identities. The legacy fallback is limited to records without the persisted region marker, so new same-name clusters cannot reuse legacy storage across environments.

Important Files Changed

Filename Overview
src/main/java/io/github/hectorvent/floci/services/msk/MskService.java Persists request-region identity and applies compatible regional filtering to MSK operations.
src/main/java/io/github/hectorvent/floci/services/msk/RedpandaManager.java Isolates log streams, labels, and host storage by cluster account and region while preserving legacy data access.
src/main/java/io/github/hectorvent/floci/services/msk/model/MskCluster.java Adds the persisted resource-region marker used to distinguish scoped clusters from legacy records.
src/test/java/io/github/hectorvent/floci/services/msk/MskServiceTest.java Covers same-name regional clusters, cross-region access restrictions, and legacy record visibility.
src/test/java/io/github/hectorvent/floci/services/msk/RedpandaManagerTest.java Covers account-aware logging, scoped host paths, and guarded legacy path reuse.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Request[MSK request] --> Context[Resolve account and region]
    Context --> Cluster[Create or select cluster]
    Cluster --> ARN[Account and region scoped ARN]
    ARN --> Logs[Account and region scoped broker logs]
    ARN --> Storage{Legacy record?}
    Storage -->|No| Scoped[Scoped host data path]
    Storage -->|Yes and legacy path exists| Legacy[Legacy cluster-name path]
    Storage -->|Yes and legacy path absent| Scoped
Loading

Reviews (4): Last reviewed commit: "ci: retrigger PR checks after unrelated ..." | Re-trigger Greptile

Comment thread src/main/java/io/github/hectorvent/floci/services/msk/RedpandaManager.java Outdated
Comment thread src/main/java/io/github/hectorvent/floci/services/msk/MskService.java Outdated
Comment thread src/main/java/io/github/hectorvent/floci/services/msk/RedpandaManager.java Outdated
@hectorvent hectorvent added bug Something isn't working msk Amazon Managed Streaming for Apache Kafka (MSK) labels Sep 10, 2026
@nkootstra nkootstra changed the title fix: isolate MSK resources by account and region fix(msk): isolate MSK resources by account and region Sep 10, 2026

@pgermosen pgermosen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Traced this against the existing account-scoping mechanism rather than assuming it duplicates it: cluster metadata already gets that for free from AccountAwareStorageBackend, so the real gap closed here is region on the metadata side and both account+region on the RedpandaManager log/host-path side. Deriving region and account from the cluster's own ARN rather than live request context is the right call for a background lifecycle thread. The legacy-record fallback is explicitly reasoned in a comment, not an oversight. Approving.

@pgermosen
pgermosen merged commit 970cd30 into floci-io:main Sep 10, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working msk Amazon Managed Streaming for Apache Kafka (MSK)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants