HYPERFLEET-839 - fix: update additional docs/scripts to explicitly se…#93
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis pull request updates documentation and comments across two files to clarify Pub/Sub namespace configuration requirements. In Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/runbook.md`:
- Around line 158-162: Update the NAMESPACE example to produce DNS-1123
compliant names by normalizing the USER-derived portion: convert to lowercase,
replace any non-alphanumeric characters with hyphens, collapse repeated hyphens,
trim leading/trailing non-alphanumeric/hyphen characters and optionally limit
the length; update the .env.example export of NAMESPACE to describe this
sanitization and provide an example command that uses USER-derived input and
these normalization steps so generated namespaces are valid for GCP Pub/Sub;
reference the NAMESPACE variable and the .env.example export line in the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: dde21e62-7243-44b7-a169-9295903eed82
📒 Files selected for processing (2)
deploy-scripts/lib/gcp.shdocs/runbook.md
| # NAMESPACE must be unique to prevent GCP Pub/Sub topic/subscription collisions. | ||
| # Set in the .env.example file as: | ||
| export NAMESPACE="${NAMESPACE:-hyperfleet-e2e-$(echo ${USER:-default} | tr '[:upper:]' '[:lower:]')}" | ||
| # Or can manually set it with as the namespace is DNS-1123 compliant | ||
| export NAMESPACE=<unique_namespace> |
There was a problem hiding this comment.
DNS-1123 guidance is incomplete; current example can still generate invalid namespaces.
$(echo ${USER:-default} | tr '[:upper:]' '[:lower:]') only lowercases; it does not remove/normalize invalid DNS-1123 chars (e.g., _, .). That can still produce invalid namespace/topic/subscription names and break deploy flows.
Proposed doc fix
# NAMESPACE must be unique to prevent GCP Pub/Sub topic/subscription collisions.
# Set in the .env.example file as:
-export NAMESPACE="${NAMESPACE:-hyperfleet-e2e-$(echo ${USER:-default} | tr '[:upper:]' '[:lower:]')}"
-# Or can manually set it with as the namespace is DNS-1123 compliant
-export NAMESPACE=<unique_namespace>
+export NAMESPACE="${NAMESPACE:-hyperfleet-e2e-$(echo "${USER:-default}" | tr '[:upper:]' '[:lower:]' | sed -E 's/[^a-z0-9-]+/-/g; s/^-+//; s/-+$//')}"
+# Or set it manually to a unique DNS-1123 value (example):
+export NAMESPACE="hyperfleet-e2e-jdoe"As per coding guidelines, “Focus on major issues impacting performance, readability, maintainability and security… Validate changes against HyperFleet architecture standards,” and the referenced runbook content requires NAMESPACE to be unique and DNS-1123 compliant.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # NAMESPACE must be unique to prevent GCP Pub/Sub topic/subscription collisions. | |
| # Set in the .env.example file as: | |
| export NAMESPACE="${NAMESPACE:-hyperfleet-e2e-$(echo ${USER:-default} | tr '[:upper:]' '[:lower:]')}" | |
| # Or can manually set it with as the namespace is DNS-1123 compliant | |
| export NAMESPACE=<unique_namespace> | |
| # NAMESPACE must be unique to prevent GCP Pub/Sub topic/subscription collisions. | |
| # Set in the .env.example file as: | |
| export NAMESPACE="${NAMESPACE:-hyperfleet-e2e-$(echo "${USER:-default}" | tr '[:upper:]' '[:lower:]' | sed -E 's/[^a-z0-9-]+/-/g; s/^-+//; s/-+$//')}" | |
| # Or set it manually to a unique DNS-1123 value (example): | |
| export NAMESPACE="hyperfleet-e2e-jdoe" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/runbook.md` around lines 158 - 162, Update the NAMESPACE example to
produce DNS-1123 compliant names by normalizing the USER-derived portion:
convert to lowercase, replace any non-alphanumeric characters with hyphens,
collapse repeated hyphens, trim leading/trailing non-alphanumeric/hyphen
characters and optionally limit the length; update the .env.example export of
NAMESPACE to describe this sanitization and provide an example command that uses
USER-derived input and these normalization steps so generated namespaces are
valid for GCP Pub/Sub; reference the NAMESPACE variable and the .env.example
export line in the change.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rh-amarin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit d3a9552
into
openshift-hyperfleet:main
2 participants
Summary
Follow up PR to: #86
Update runbook.md and gcp.sh to clearly indicate that unique namespaces must be provided when using the deploy scripts to avoid Pub Sub collisions.
Jira Issue
HYPERFLEET-839
Test Plan