Reorder Registry Server Helm deploy guide - #1066
Merged
Merged
Conversation
The guide ran `helm upgrade -f values.yaml` before it introduced the contents of values.yaml, and database credentials appeared later still, so a first-time reader could not execute the steps in order. Reorder to prerequisites, create a values file, provide database credentials, install the chart, then optional umbrella-chart config. Add the kubectl commands to create the namespace and pgpass Secret, note that the pgpass entry must match the connection fields, and match the quickstart's `--wait` install and labeled pod check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Kubernetes Secrets tip sent readers to the deprecated operator guide, a bash block used a bare `deployment.mdx` filename in a comment, and the connection troubleshooting step named only the operator's `pgpassSecretRef` field. Point each at the Helm guide or quickstart instead, keeping the database page a deployment-agnostic reference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Helm guide ended at a ready pod with no way to reach the API. The chart ships no Ingress or HTTPRoute template, so readers have to bring their own routing and nothing said so. Describe the ClusterIP Service and its default `<RELEASE_NAME>- toolhive-registry-server` name, point readers at their existing ingress controller, Gateway API, or cloud load balancer, and keep port-forward as a pre-routing smoke test only. Verified against chart v1.5.0 via `helm template`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The YAML example pointed at `authentication.mdx` in a comment, which readers can't click and which duplicates the prose link further down the page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Reorders and fills gaps in the Registry Server Helm deployment guide so the steps are executable end-to-end (values file + DB credentials before install), and updates related Registry Server docs to remove stale or misleading pointers while adding an explicit “Expose the service” step.
Changes:
- Reordered
deploy-helm.mdxto: prerequisites → values file → DB credentials (pgpass Secret) → install → expose Service → optional umbrella-chart config. - Added missing Kubernetes commands to create the namespace and pgpass Secret, plus an explicit Service exposure section (Ingress/Gateway API/LoadBalancer + port-forward smoke test).
- Retargeted
database.mdx/configuration.mdxreferences to remove stale/bare-filename pointers and clarify credential mechanisms.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/toolhive/guides-registry/deploy-helm.mdx | Reorders the Helm deploy flow and adds missing namespace/Secret creation plus explicit Service exposure guidance. |
| docs/toolhive/guides-registry/database.mdx | Updates Kubernetes credential guidance to point at the right docs and clarifies troubleshooting language. |
| docs/toolhive/guides-registry/configuration.mdx | Removes a stale bare-filename comment in a YAML example. |
Use placeholders for the whole pgpass entry instead of mixing sample values with <PASSWORD>. The positional format now doubles as documentation of the field order, and readers can't update values.yaml while leaving a stale hard-coded entry behind. Name both Helm credential mechanisms in the connection troubleshooting step, since the Secret lives in extraEnv or extraVolumes depending on which pattern was used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rdimitrov
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up on two items deferred from the #1050 review, plus two gaps that surfaced while fixing them.
1. The Helm guide was not executable in order.
deploy-helm.mdxranhelm upgrade ... -f values.yamlbefore the guide ever showed what goes invalues.yaml, and database credentials came later still. Reordered to: prerequisites → create a values file → provide database credentials → install the chart → expose the Service → optional umbrella-chart config.Gaps that became visible while reordering:
kubectl create namespaceandkubectl create secretcommands.--wait --timeout=3m,-l app.kubernetes.io/instance=registry-server). Dropped--create-namespace, since the namespace is created explicitly a step earlier.2. Database and deployment pointed readers in opposite directions. Resolved in favor of self-contained deployment guides, with
database.mdxas the deeper operational reference owning production hardening, migrations, pooling, and alternative credential mechanisms. Most of this landed in #1050 already, so only three stale back-pointers needed retargeting:secretKeyRefpattern it actually describes, with the pgpass section as the alternative.# See deployment.mdx for examplesas a comment, a bare filename rather than a link, pointing at a page with no such example.pgpassSecretRef, an operator-only CRD field, on a page that serves all deployment methods. Now names both mechanisms.3. The guide ended at a ready pod with no way to reach the API. The chart ships no Ingress or HTTPRoute template (verified against v1.5.0: the only templates are
_helpers,clusterrole,configmap,deployment,leader-election-role,service,serviceaccount), so routing is bring-your-own and nothing said so. New "Expose the Registry Server" section documents the ClusterIP Service and its default<RELEASE_NAME>-toolhive-registry-servername, points readers at their existing ingress controller, Gateway API, or cloud load balancer, and mentionsfullnameOverridefor a shorter name to reference from routing resources. Port-forward is kept only as a pre-routing smoke test, not as an exposure mechanism.4.
configuration.mdxhad the same bare-filename-in-a-YAML-comment pattern, pointing atauthentication.mdx. Dropped; the page already links it properly in prose at line 659.Service naming was verified with
helm templateagainst chart v1.5.0. Worth noting for reviewers thatquickstart.mdxandmigrate-to-helm.mdxappear to disagree on the Service name but don't: the quickstart setsfullnameOverride: registry-server, whilemigrate-to-helm.mdxdocuments the default. This PR follows the default.Type of change
Related issues/PRs
Follow-up to #1050.
Screenshots
N/A
Submitter checklist
Content and formatting
Navigation
No pages added, deleted, renamed, or reordered. Section ordering within
deploy-helm.mdxchanged, but the#provide-database-credentialsanchor is preserved (inbound links fromquickstart.mdx,migrate-to-helm.mdx, anddatabase.mdxall resolve).Reviewer checklist
Content
🤖 Generated with Claude Code