docs: Add naming strategy for multi-tenant isolation #41
Conversation
|
Warning Rate limit exceeded@rafabene has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
WalkthroughA new Sentinel naming strategy document was added describing topic naming, tenant isolation, container image naming, tagging conventions, and retention. sentinel.md and sentinel-versioning.md were updated to reference the naming strategy and adjusted example image tag formats. AWS SQS configuration was removed from sentinel-broker-config.yaml and sentinel-configmap-template.yaml; supported broker types are now Google Pub/Sub and RabbitMQ. sentinel-deployment.md was updated to align publisher fields, remove SQS-specific fields, and change the Pub/Sub broker_type label value to "pubsub"; notes about the naming strategy were added. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
hyperfleet/components/sentinel/sentinel-naming-strategy.md (2)
354-365: Image retention policy is practical but lacks implementation guidance.The retention policy table (lines 358-365) provides useful guidelines:
- Release tags (
v*): Keep forever ✓- Main branch (
main-*): Keep last 10 ✓- Staging (
staging-*): Keep last 20 ✓- Dev tags (
dev-*): Keep last 5 per developer ✓- PR/branch tags: Delete after PR/branch + 7 days ✓
However, the guidance is missing:
- Implementation details: Which registry platforms support these policies? (Quay, ECR, GCR, Harbor all support different retention mechanisms)
- Cost implications: Quantify the impact mentioned in Trade-offs (line 251: "may increase cloud costs")
- Cleanup triggers: Should cleanup be automated (lifecycle policies) or manual?
- Validation: How to verify retention policies are applied correctly?
Consider adding a brief note on implementation tooling (e.g., Quay's repository settings, ECR lifecycle policies, etc.).
369-380: Dependencies section is comprehensive but could clarify adapter-side requirements.The dependencies section identifies key components needing changes:
- Sentinel: Topic prefix logic ✓
- Adapters: Subscribe to prefixed topics ✓
- Helm Charts: Expose topicPrefix configuration ✓
- Message Broker: Support topic names with hyphens ✓
However, missing clarification on adapter behavior:
- What if Sentinel and adapter prefixes mismatch? Should adapters fail loudly or log warnings?
- Is prefix validation needed in adapters? (same DNS subdomain constraints as Sentinel)
- Are adapters responsible for deriving prefix the same way (BROKER_TOPIC_PREFIX → NAMESPACE)?
These details matter for operational reliability. Consider adding a note under adapter dependencies or linking to adapter-specific configuration guidance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
hyperfleet/components/sentinel/sentinel-naming-strategy.md(1 hunks)hyperfleet/components/sentinel/sentinel-versioning.md(1 hunks)hyperfleet/components/sentinel/sentinel.md(1 hunks)
🔇 Additional comments (7)
hyperfleet/components/sentinel/sentinel.md (1)
327-327: LGTM on editorial note placement.The added reference to the Naming Strategy document is well-positioned after the broker configuration examples and before adapter status requirements. The note is concise and directive.
hyperfleet/components/sentinel/sentinel-naming-strategy.md (5)
54-119: Clear problem statement with effective visual aid.The "Why" section effectively illustrates the naming collision problem through a clear scenario table (lines 75-79) and an architecture diagram (lines 83-119). The mermaid diagram clearly shows the current problematic behavior vs. the proposed solution. This helps readers quickly understand the motivation.
1-50: Strong introduction and clear structure.The document is well-organized with a clear table of contents (lines 10-37), metadata (lines 3-6), and a concise "What" and "Why" section. The problem statement is concrete with specific impact examples.
125-143: Verify topic naming examples align with configuration guidance.The topic naming format
{prefix}-{resource.Kind}is clear, and the examples table shows the expected output. However, confirm that:
- The "Custom prefix" example (
team-a-Cluster) is consistent with validation rules (lines 226-228 reference DNS subdomain labels)- All examples shown are covered by the configuration methods (Helm values, environment variables, ConfigMaps)
- The examples match configuration examples in the ConfigMap sections (lines 179-205)
The format appears correct but cross-validation with configuration examples is needed.
221-234: Prefix resolution order is clear but missing error context.The prefix resolution order (lines 221-224) is logical:
- Use
BROKER_TOPIC_PREFIXif set- Fall back to
NAMESPACE- Fail with error if both empty
However, the implementation guidance at line 217 shows a Go function that only checks
if prefix == ""and falls back toNAMESPACE. Clarify whether the code also checks ifNAMESPACEis empty before returning, or if a missing namespace is considered a startup failure. The "fail with clear error message" requirement needs explicit validation in the implementation requirements section.The implementation requirement at line 217 should explicitly state the validation logic to ensure no silent fallback to bare
resource.Kind.
284-302: Image tag format tables require manual cross-validation due to repository access limitations.The tagging conventions are well-documented across two tables (lines 284-302):
- Primary conventions: environment-based tags (dev, staging, prod)
- Branch-based tags: feature branches, PRs, main branch
However, verification of the following concerns requires direct repository access:
- Consistency of
v{semver}format with sentinel-versioning.md (line 181)- Conformance of CI/CD pipeline examples (lines 322-339) to the tagging conventions
- Actual format accuracy of all tag examples
Manual review of these cross-references is needed to confirm the conventions are correctly applied throughout the documentation.
hyperfleet/components/sentinel/sentinel-versioning.md (1)
181-186: Verify tag format consistency with naming-strategy document.The changes update image tag examples from
sentinel:1.2.3andsentinel:a1b2c3dtosentinel:v1.2.3andsentinel:main-a1b2c3d, which aligns with the new naming conventions. Confirm that these formats are explicitly documented in the referenced naming-strategy document and that tag format examples are consistent across all related documentation in the PR.
rafabene
force-pushed
the
HYPERFLEET-283-naming-strategy
branch
from
a3a09d4 to
48068c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
hyperfleet/components/sentinel/sentinel-naming-strategy.md (1)
35-40: Clarify the distinction between message-flow isolation and environment isolation.Line 37 states "multiple developers testing simultaneously can cause message interference if all Sentinels publish to the same topics" and attributes this to environment isolation. This framing could be misleading—this is about message-flow isolation at the topic level within a shared namespace, not environment-level infrastructure isolation. The document should clarify that:
- The prefix enables logical message isolation (different topics) for concurrent developers
- Infrastructure isolation (dev vs prod environments) typically uses separate GCP projects, namespaces, or clusters (as noted in past feedback)
Consider revising the "Why" section for clarity:
-In shared development environments (e.g., hyperfleet-dev cluster), multiple developers testing simultaneously can cause message interference if all Sentinels publish to the same topics. A configurable topic prefix allows each developer or team to have isolated message flows. +In shared development environments (e.g., hyperfleet-dev cluster), multiple developers testing simultaneously can cause message interference if all Sentinels publish to the same topics. A configurable topic prefix allows each developer or team to isolate their message flows at the topic level, preventing events from one developer's Sentinel from being consumed by another's adapters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
hyperfleet/components/sentinel/sentinel-broker-config.yaml(1 hunks)hyperfleet/components/sentinel/sentinel-configmap-template.yaml(1 hunks)hyperfleet/components/sentinel/sentinel-deployment.md(2 hunks)hyperfleet/components/sentinel/sentinel-naming-strategy.md(1 hunks)hyperfleet/components/sentinel/sentinel-versioning.md(1 hunks)hyperfleet/components/sentinel/sentinel.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- hyperfleet/components/sentinel/sentinel.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
hyperfleet/components/sentinel/sentinel-naming-strategy.md
3-3: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
69-69: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (4)
hyperfleet/components/sentinel/sentinel-broker-config.yaml (1)
24-37: AWS SQS removal properly reflected in option numbering.The RabbitMQ option header has been correctly updated from "Option 3" to "Option 2" following AWS SQS removal. Configuration content is intact.
hyperfleet/components/sentinel/sentinel-versioning.md (1)
180-185: Image tag examples properly aligned with new naming conventions.The semantic versioning prefix (
v1.2.3) and branch-based SHA format (main-a1b2c3d) are now consistent with the new naming strategy. The reference link is appropriately positioned.hyperfleet/components/sentinel/sentinel-configmap-template.yaml (1)
110-110: BROKER_TYPE options correctly updated after AWS SQS removal.Field reference documentation now accurately reflects supported broker types: pubsub and rabbitmq.
hyperfleet/components/sentinel/sentinel-deployment.md (1)
185-189: Broker configuration documentation and metrics labels properly aligned.The clarification of BROKER_TOPIC/BROKER_EXCHANGE usage, removal of AWS SQS references, and metrics label standardization (
gcp-pubsub→pubsub) are all consistent with the AWS SQS removal. The naming strategy reference provides helpful context.
rafabene
force-pushed
the
HYPERFLEET-283-naming-strategy
branch
2 times, most recently
from
9e4f653 to
dd96ad4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hyperfleet/components/sentinel/sentinel-configmap-template.yaml (1)
90-93: Remove stale AWS configuration reference.Line 92 references AWS configuration (
BROKER_REGIONandBROKER_QUEUE_URL) that no longer exists after AWS SQS was removed from supported broker types. This creates confusion for operators.Apply this diff to remove the orphaned AWS scenario:
# Scenario 2: Multi-region deployment # - For GCP: Update BROKER_PROJECT_ID for each region's project -# - For AWS: Update BROKER_REGION and BROKER_QUEUE_URL for regional queues # - For RabbitMQ: Update BROKER_HOST to point to regional RabbitMQ cluster
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
hyperfleet/components/sentinel/sentinel-broker-config.yaml(1 hunks)hyperfleet/components/sentinel/sentinel-configmap-template.yaml(1 hunks)hyperfleet/components/sentinel/sentinel-deployment.md(2 hunks)hyperfleet/components/sentinel/sentinel-naming-strategy.md(1 hunks)hyperfleet/components/sentinel/sentinel-versioning.md(1 hunks)hyperfleet/components/sentinel/sentinel.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- hyperfleet/components/sentinel/sentinel.md
- hyperfleet/components/sentinel/sentinel-deployment.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
hyperfleet/components/sentinel/sentinel-naming-strategy.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (5)
hyperfleet/components/sentinel/sentinel-broker-config.yaml (1)
24-37: LGTM!The renumbering from "Option 3" to "Option 2" for RabbitMQ is correct and consistent with the AWS SQS removal. Configuration comments are clear and the examples are complete.
hyperfleet/components/sentinel/sentinel-configmap-template.yaml (1)
110-110: Good: BROKER_TYPE options updated correctly.The field reference now correctly reflects the two supported broker types (pubsub, rabbitmq), aligning with AWS SQS removal.
hyperfleet/components/sentinel/sentinel-versioning.md (1)
179-186: Good alignment with the new naming strategy documentation.The image tag examples are clear and the reference to the Naming Strategy document appropriately delegates detailed tagging conventions. This update successfully links the versioning strategy to the broader naming framework.
hyperfleet/components/sentinel/sentinel-naming-strategy.md (2)
25-62: Clear and concise topic naming strategy.The topic naming section effectively explains the rationale, format, and examples without verbosity. The configurable prefix approach with Kubernetes namespace default provides good flexibility for multi-tenant scenarios. Examples table clearly illustrates prefix variations for different isolation contexts.
64-91: Well-structured container image naming and retention policy.The image naming strategy logically separates tag format, tagging conventions by environment, and retention policy. The distinction between development (short SHA) and production (semantic version) tagging aligns with industry best practices. Retention rules are practical and clear.
rafabene
force-pushed
the
HYPERFLEET-283-naming-strategy
branch
from
dd96ad4 to
9343d30
Compare
Add documentation for Pub/Sub topic naming and container image naming
conventions to enable multi-tenant and isolated deployments.
Topic Naming:
- Configurable topic prefix via Helm/environment variables
- Default prefix uses Kubernetes namespace
- Format: {prefix}-{resource.Kind}
Image Naming:
- Environment-based tagging conventions (dev, staging, prod)
- Branch-based tags for CI/CD
- Image retention policy guidelines
Also updates sentinel.md and sentinel-versioning.md with references
to the new naming strategy document.
Closes: HYPERFLEET-283
rafabene
force-pushed
the
HYPERFLEET-283-naming-strategy
branch
from
9343d30 to
982b7b6
Compare
2 participants
Summary
Documents Pub/Sub topic naming and container image naming conventions to enable multi-tenant and isolated deployments (HYPERFLEET-283).
Topic Naming Strategy
{prefix}-{resource.Kind}(e.g.,hyperfleet-dev-Cluster)Container Image Naming Strategy
dev-{sha},staging-{sha},v{semver}main-{sha},pr-{number}-{sha}Changes
sentinel-naming-strategy.mdsentinel.mdwith reference to naming strategysentinel-versioning.mdwith aligned image tag format and referenceRelated
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.