-
Notifications
You must be signed in to change notification settings - Fork 4
Update gcp-private-service-connect.adoc - Add more steps for BYOC #220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
👷 Deploy request for rp-cloud pending review.Visit the deploys page to approve it
|
|
@david-yu reminder about this! |
PR Change SummaryUpdated the GCP Private Service Connect documentation to enhance the BYOC setup process with additional steps and improved variable handling.
Modified Files
How can I customize these reviews?Check out the Hyperlint AI Reviewer docs for more information on how to customize the review. If you just want to ignore it on this PR, you can add the Note specifically for link checks, we only check the first 30 links in a file and we cache the results for several hours (for instance, if you just added a page, you might experience this). Our recommendation is to add |
1 similar comment
PR Change SummaryUpdated the GCP Private Service Connect documentation to enhance the BYOC setup process with additional steps and improved variable handling.
Modified Files
How can I customize these reviews?Check out the Hyperlint AI Reviewer docs for more information on how to customize the review. If you just want to ignore it on this PR, you can add the Note specifically for link checks, we only check the first 30 links in a file and we cache the results for several hours (for instance, if you just added a page, you might experience this). Our recommendation is to add |
|
Added some suggestions and resolved conflict from latest changes. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe documentation for enabling Private Service Connect (PSC) on an existing BYOC cluster was revised for clarity. The process is now split into two explicit steps: running the Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
modules/networking/pages/gcp-private-service-connect.adoc (1)
223-233: 🛠️ Refactor suggestionClarify BYOVPC cluster follow-up steps
The new BYOVPC snippet drops context on IAM/firewall and NAT setup, which could confuse readers. Link back to the “Create a new BYOVPC cluster” section to avoid repetition:.For a *BYOVPC cluster*: * Follow the steps in <<create-a-new-byovpc-cluster-with-private-service-connect,Create a new BYOVPC cluster>> to configure IAM, create a NAT subnet, and set up firewall rules. * Then run: ---- rpk cloud byoc gcp apply --redpanda-id="${CLUSTER_ID}" --project-id="<service-project-id>" ----This ensures readers have the full context without duplicating instructions.
🧹 Nitpick comments (2)
modules/networking/pages/gcp-private-service-connect.adoc (2)
52-68: Ensure list syntax consistency
Great addition of placeholder explanations. Earlier in this document we use*for bullet lists—consider switching from-to*here to maintain visual and semantic consistency.
176-197: Update BYOVPC placeholders list
The new BYOVPC placeholder list is comprehensive. For consistent AsciiDoc styling, you may want to:
- Use
*for bullets instead of-.- Ensure a blank line before/after the list so it renders cleanly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/networking/pages/gcp-private-service-connect.adoc(2 hunks)
🔇 Additional comments (3)
modules/networking/pages/gcp-private-service-connect.adoc (3)
40-46: Clearer resource group ID handling
Adding an explicit step to copy the resource group UUID from the UI URL improves usability by removing ambiguity about where to find the value.
113-122: Old placeholder list removal
Removing the outdated placeholder bullets is correct since they’ve been superseded by the new list later in the flow.
243-246: Shell variable interpolation corrected
Switching to${PSC_NAT_SUBNET_NAME}ensures proper expansion even if the variable contains special characters.
|
@micheleRP Can you provide a status update? |
Co-authored-by: Michele Cyran <[email protected]>
Co-authored-by: Michele Cyran <[email protected]>
Co-authored-by: Michele Cyran <[email protected]>
micheleRP
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@david-yu is this good to merge?
Description
Still testing BYOC, and will need to test BYOVPC as well. In general, we should try to use curly braces around SHELL variables as described here: https://stackoverflow.com/questions/8748831/when-do-we-need-curly-braces-around-shell-variables. Removed one env variable in the subnet configure step, as the accept list is in the next step.
Page previews
Checks