Skip to content

Add Redis Sentinel session storage tutorial and TLS docs#606

Merged
jhrozek merged 2 commits intomainfrom
authserver-redis-storage
Mar 16, 2026
Merged

Add Redis Sentinel session storage tutorial and TLS docs#606
jhrozek merged 2 commits intomainfrom
authserver-redis-storage

Conversation

@jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Mar 12, 2026

Description

Add a new tutorial at docs/toolhive/integrations/redis-session-storage.mdx that walks through deploying Redis Sentinel as the session storage backend for the embedded authorization server. The tutorial uses self-contained Kubernetes manifests instead of the Bitnami Helm chart, which Broadcom moved behind a paid subscription in August 2025. No maintained, freely available Helm chart with Sentinel support exists as a replacement, so the manifests are provided inline.

Also adds TLS configuration documentation for Redis connections (both master and Sentinel), updates the auth-k8s guide's session storage snippet to match, and resolves a merge conflict in backend-auth.mdx.

Type of change

  • New documentation

Related issues/PRs

stacklok/toolhive#3899

Screenshots

Submitter checklist

Content and formatting

  • I have reviewed the content for technical accuracy
  • I have reviewed the content for spelling, grammar, and style

Navigation

  • New pages include a frontmatter section with title and description at a minimum
  • Sidebar navigation (sidebars.ts) updated for added, deleted, reordered, or renamed files
  • Redirects added to vercel.json for moved, renamed, or deleted pages (i.e., if the URL slug changed)

Reviewer checklist

Content

  • I have reviewed the content for technical accuracy
  • I have reviewed the content for spelling, grammar, and style

Copilot AI review requested due to automatic review settings March 12, 2026 13:28
@vercel
Copy link

vercel bot commented Mar 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs-website Ready Ready Preview, Comment Mar 12, 2026 11:08pm

Request Review

@jhrozek
Copy link
Contributor Author

jhrozek commented Mar 12, 2026

@danbarr I was reviewing @aron-muon's patches and created a bunch of test manifests. I thought I'd reuse them to document the Redis integration.

I wasn't sure if an integration is the right place at all though.

Also, given the bitnami charts deprecation, there is a bunch of manifests which make the page longer..we could put them into examples in the toolhive repo as an option, but then the tutorial wouldn't be self-contained.

I'll be happy to redo the docs of course, but wanted to get something out there for discussion.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds documentation for using Redis Sentinel as persistent session storage for ToolHive’s embedded authorization server, including a new end-to-end Kubernetes-manifest-based tutorial and related guide cross-links.

Changes:

  • Adds a new Redis Sentinel session storage tutorial under ToolHive Integrations, including TLS configuration guidance.
  • Updates the embedded auth Kubernetes guide with a Redis Sentinel session storage snippet and links to the full tutorial.
  • Updates backend auth concept doc to point to the new session storage docs and resolves a small merge conflict.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
sidebars.ts Adds the new Redis session storage doc to the Integrations sidebar.
docs/toolhive/integrations/redis-session-storage.mdx New tutorial covering Redis Sentinel deployment/manifests, ToolHive storage config, and TLS guidance.
docs/toolhive/guides-k8s/auth-k8s.mdx Adds a “Configure session storage” section with a Redis Sentinel configuration snippet and link to the full tutorial.
docs/toolhive/concepts/backend-auth.mdx Updates session storage warning to link to the new guide/tutorial.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +69 to +71
users.acl:
'user toolhive-auth on ><REDIS_ACL_PASSWORD> ~thv:auth:* &* +GET +SET +SETNX
+DEL +EXISTS +EXPIRE +SADD +SREM +SMEMBERS +EVAL +MULTI +EXEC +EVALSHA +PING'
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The users.acl value is wrapped onto a second line inside single quotes. In YAML this introduces a newline into the Secret value, which contradicts the comment “the ACL entry is a single line” and can break Redis ACL parsing. Consider keeping it on one line, or using a folded scalar (>-) so line breaks render as spaces.

Suggested change
users.acl:
'user toolhive-auth on ><REDIS_ACL_PASSWORD> ~thv:auth:* &* +GET +SET +SETNX
+DEL +EXISTS +EXPIRE +SADD +SREM +SMEMBERS +EVAL +MULTI +EXEC +EVALSHA +PING'
users.acl: >-
user toolhive-auth on ><REDIS_ACL_PASSWORD> ~thv:auth:* &* +GET +SET +SETNX +DEL +EXISTS +EXPIRE +SADD +SREM +SMEMBERS +EVAL +MULTI +EXEC +EVALSHA +PING

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

In these particular cases, I wonder if the <PLACEHOLDER> convention is too confusing because of the need for the preceding literal > character?

Comment on lines +94 to +96
serviceName: redis
replicas: 1
selector:
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

This manifest deploys only a single Redis pod (replicas: 1). With no Redis replicas configured, Sentinel cannot perform an automatic failover (there’s nothing to promote), so the tutorial’s HA/failover claims won’t hold as written. Either adjust the text to reflect this limitation or extend the manifests to include Redis replicas configured for replication so Sentinel can fail over.

Copilot uses AI. Check for mistakes.
Comment on lines +270 to +277
The manifests above don't disable the Redis default user, which has full access
with no password. For production deployments, add `user default off` to the
`users.acl` entry in the `redis-acl` Secret:

```text
user default off
user toolhive-auth on ><PASSWORD> ~thv:auth:* &* +GET +SET +SETNX ...
```
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The tutorial recommends disabling the default Redis user for production, but neither the Sentinel config nor the probes are set up to authenticate to Redis. If user default off is applied, Sentinel monitoring and commands like redis-cli PING/ACL LIST will fail unless you configure sentinel auth-user/sentinel auth-pass (or equivalent) and update the probe/troubleshooting commands to authenticate.

Copilot uses AI. Check for mistakes.
Comment on lines +439 to +444
tls:
enabled: true
caCertSecretRef:
name: redis-ca-cert
key: ca.crt
# highlight-end
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The examples introduce redis.tls / redis.sentinelTls, but the linked CRD reference currently documents RedisStorageConfig without any TLS-related fields (only sentinelConfig/aclUserConfig/timeouts). If TLS is not actually supported by the CRD, these examples will not apply; if it is supported, the CRD reference doc likely needs to be updated alongside this tutorial so users can validate the schema and available fields.

Copilot uses AI. Check for mistakes.
Comment on lines +573 to +575
- If using self-signed certificates for testing, you can set
`insecureSkipVerify: true` to bypass verification (not recommended for
production)
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

insecureSkipVerify: true is mentioned as a supported knob, but it is not shown in the TLS examples and it does not appear in the CRD reference doc. If this field isn’t supported, the troubleshooting guidance should be removed; if it is supported, document it in the TLS config examples and ensure it’s reflected in the CRD reference.

Suggested change
- If using self-signed certificates for testing, you can set
`insecureSkipVerify: true` to bypass verification (not recommended for
production)

Copilot uses AI. Check for mistakes.
kubectl create secret generic redis-acl-secret \
--namespace toolhive-system \
--from-literal=username=toolhive-auth \
--from-literal=password="<YOUR_REDIS_PASSWORD>"
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The placeholder password here (<YOUR_REDIS_PASSWORD>) is inconsistent with the rest of the Redis tutorial/examples (<REDIS_ACL_PASSWORD>). Using one consistent placeholder name across docs helps avoid users wiring the wrong Secret value.

Suggested change
--from-literal=password="<YOUR_REDIS_PASSWORD>"
--from-literal=password="<REDIS_ACL_PASSWORD>"

Copilot uses AI. Check for mistakes.
@danbarr
Copy link
Collaborator

danbarr commented Mar 12, 2026

@jhrozek Hmm yeah, I'm also not sure Integrations is the right spot for this since it's not so much about integrating with existing enterprise systems; feels more like a how-to guide in the category of "enterprise deployment patterns" -- which is something I think we'll be having more of going forward (platform-wide architectures, HA configs, prod-ready examples etc.), but we don't really have enough of those to get their own section just yet.

For now maybe it just lands in the K8s Operator section below the other auth topics, and is slightly repositioned as a how-to instead of a tutorial?

Re: the long manifests, in some other spots we've used the approach of kubectl apply -f <raw github url> but I don't think that fits this case since there are values that need to be updated? A while ago I came across a Docusaurus plugin to source a file in GitHub for a code block which would solve the double-maintenance burden but I hadn't had a chance to actually try that out yet.

@danbarr
Copy link
Collaborator

danbarr commented Mar 12, 2026

@claude Use the docs-review skill to perform an editorial review of this new content. Take existing review comments into account.

Also, consider the discussion about placement - given the current information architecture and user journey, where might this best fit?

@claude
Copy link
Contributor

claude bot commented Mar 12, 2026

Claude finished @danbarr's task in 2m 46s —— View job


Editorial review

Summary

The content is well-structured and fills a genuine documentation gap—the deprecation of the Bitnami chart means users have no existing path to production-ready Redis Sentinel on Kubernetes, and the inline manifests solve that directly. The writing quality is good, voice and style are consistent, and the cross-file changes (auth-k8s.mdx, backend-auth.mdx) are appropriate. Several technical accuracy issues surfaced in the Copilot review need to be resolved before merge, and there's a placement question worth settling. The document also straddles the tutorial/how-to boundary in a way that's worth clarifying.


Primary issues

1. Placement: Integrations vs. K8s operator how-to guides

I agree with @danbarr's instinct here. The integrations/ section contains genuinely third-party enterprise system integrations: Vault, AWS STS, Okta, OpenTelemetry, ngrok. Redis Sentinel in this context is different—it's infrastructure you're deploying yourself specifically for ToolHive, not a pre-existing enterprise system you're integrating with.

More importantly, this content's prerequisites section explicitly requires "a Kubernetes cluster with the ToolHive Operator installed" and frames itself as a follow-on to the embedded auth server setup. That makes it contextually part of the K8s operator how-to guides, not the integrations catalog.

Recommendation: Move to docs/toolhive/guides-k8s/ alongside the other auth topics (auth-k8s.mdx, token-exchange-k8s.mdx). A title like "Configure persistent session storage" or "Set up Redis Sentinel session storage" fits cleanly as a how-to. Update the sidebar, the cross-references in auth-k8s.mdx and backend-auth.mdx, and add a redirect in vercel.json if you want to commit the integrations URL now. Alternatively, stage this in integrations as-is and do a restructure later when more deployment pattern content justifies its own section—the cross-references will need updating either way.

2. Diataxis classification drift: tutorial vs. how-to

The front matter description says "How to deploy Redis Sentinel…", the prerequisites admonition assumes an existing cluster setup, and the content is task-oriented—all of which align with a how-to guide, not a tutorial. But the document is placed under a Tutorials-style structure (numbered "Step N" headings, numbered progression). This isn't a blocking issue on its own, but the mismatch causes an internal tension:

  • "Step 4: Enable TLS" is numbered as a required step but is actually optional for non-production scenarios. In a how-to, this would be cleaner as a separate ## Enable TLS section that clearly reads as optional, not step 4 of 5.

Recommendation: Reframe as a how-to. Drop the "Step N:" prefix or reserve the step numbering only for the core required path and separate optional extensions (TLS, disabling default user) as standalone sections.

3. Technical accuracy issues from existing review comments

These were flagged by Copilot and need author/SME resolution before merge—editorial cannot resolve them, but they affect documentation accuracy:

  • Line 71 (ACL YAML multi-line value): The users.acl value wraps across lines inside single quotes, which introduces a literal newline into the Secret. This would break Redis ACL parsing. The Copilot suggestion (>- folded scalar) is correct. @danbarr's point about ><REDIS_ACL_PASSWORD> being confusing with the preceding literal > is also valid—consider a note or different placeholder styling here since this is a Redis-specific ACL syntax (> is the password marker, not a redirect operator).

  • Line 96 (single Redis replica): replicas: 1 means Sentinel monitors a master with no replicas—there's nothing to promote on failover. The "highly available storage" claim in the introduction (line 12) and the Sessions lost after failover troubleshooting section (line 582) are misleading as written. Either extend the manifests to include a replica, or qualify the HA claim to say Sentinel provides automatic master detection and reconnection but failover requires a replica to be available.

  • Line 277 (disable default user without Sentinel auth): The warning correctly flags that the manifests don't disable the default user. But the advice to add user default off without also configuring sentinel auth-user/sentinel auth-pass will break Sentinel's own PING health checks. The warning needs to include the corresponding Sentinel configuration changes, or it should not recommend disabling the default user in this section.

  • Lines 444/575 (TLS fields and insecureSkipVerify): If redis.tls, redis.sentinelTls, and insecureSkipVerify are not yet implemented in the CRD, these sections are documenting a future state and will actively mislead users. If they are implemented, the CRD reference doc should be updated to match. This needs author confirmation.

4. Inconsistent placeholder names across files

auth-k8s.mdx line 562 uses <YOUR_REDIS_PASSWORD>, while redis-session-storage.mdx uses <REDIS_ACL_PASSWORD> throughout. Users following the auth-k8s guide and then the full tutorial will see different placeholder names for the same credential. Standardize on <REDIS_ACL_PASSWORD> (the more descriptive name) in both files.

Fix this →


Secondary issues

Location Issue Recommendation
Line 3–5 (front matter) description: value is a multi-line implicit YAML block; works but inconsistent with other pages in the repo that use single-line descriptions Flatten to one line or use >-
Line 29 "Deploy a Redis master with a three-node Sentinel cluster" — the manifests create one Redis master + three Sentinels, not three Redis nodes. The phrasing is accurate but "three-node Sentinel cluster" could be read as three Redis nodes. "Deploy a Redis master and a three-node Sentinel cluster"
Line 41 "Save the following manifests to a file called redis-sentinel.yaml" followed by a second block at line 160 with "(append to same file)" — split manifests across two code blocks is confusing Either combine into one code block with sections delimited by comments, or explain up front that the full manifest is split across two blocks for readability
Line 157–159 Transitional sentence between the two code blocks ("The next section deploys a three-node Sentinel cluster…") is a bit awkward as a standalone paragraph Fold into the block's title attribute or remove—the code block title already explains it
Line 295–298 "following example shows a complete configuration" — it's not complete (it omits tokenLifespans), compared to the full example in auth-k8s.mdx Change "complete" to "full" or "working" to avoid a literal accuracy issue
Lines 353–355 "sentinelAddrs and sentinelService are mutually exclusive" note Move this note before the explicit addresses example so users see the constraint before deciding which approach to follow
Line 391–393 "Without TLS, Redis credentials and session tokens travel in plaintext… You should enable TLS for any deployment beyond local development." Strong agree with the sentiment, but this is currently a flat statement with no guidance on how to generate/obtain the CA cert and Redis server cert. The <PATH_TO_CA_CERTIFICATE> placeholder at line 415 assumes users already have a cert. Consider a brief note pointing to Redis TLS docs for generating certs, or at minimum acknowledge that cert provisioning is out of scope.
Line 511 "The examples below use weather-server-embedded as the MCPServer name — substitute your own." Style: em dash should not have spaces around it per US style (or use an en dash with spaces). Minor, but consistent with CLAUDE.md using straight punctuation.
Lines 519–525 Restart steps use weather-server-embedded-proxy as deployment name but the MCPServer resource is weather-server-embedded—confirm these are consistent with how the operator names the proxy deployment If operator-generated names differ, update to match
auth-k8s.mdx lines 531–535 The intro to the session storage section says "Upstream tokens are lost when pods restart, requiring users to re-authenticate"—this is correct context, but this text is already in backend-auth.mdx and is about to appear there again via a link. Slight duplication, acceptable given the guide needs to be self-contained. Keep as-is, but trim to one sentence if possible

What works well

  • The inline manifests approach is the right call given the Bitnami chart situation, and the explanation of why (lines 30–46) is well done.
  • The ACL user restriction to ~thv:auth:* key pattern is a good security practice and the doc explains it clearly (lines 43–46).
  • The sentinelService vs sentinelAddrs dual approach is well-documented with clear use cases.
  • The troubleshooting section uses collapsible <details> blocks appropriately—content is genuinely optional depth.
  • The cross-file changes (backend-auth.mdx warning update, auth-k8s.mdx summary section) are well-scoped: brief summaries with links to the full content rather than duplicating it.
  • The session storage section added to auth-k8s.mdx is the right length—enough to configure the common case, with a clear "full walkthrough" pointer.

Add a new how-to guide at docs/toolhive/guides-k8s/redis-session-storage.mdx
that walks through deploying Redis Sentinel as the session storage backend
for the embedded authorization server. The guide uses self-contained
Kubernetes manifests instead of the Bitnami Helm chart, which Broadcom
moved behind a paid subscription in August 2025. No maintained, freely
available Helm chart with Sentinel support exists as a replacement, so
the manifests are provided inline.

Also adds TLS configuration documentation for Redis connections (both
master and Sentinel), updates the auth-k8s guide's session storage
snippet for consistency, and resolves a merge conflict in backend-auth.mdx.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jhrozek
Copy link
Contributor Author

jhrozek commented Mar 12, 2026

Updated based on all review feedback:

  • Moved doc to guides-k8s/ — reframed as a how-to guide (not an integration), added permanent redirect in vercel.json
  • Removed "Step N:" heading prefixes — headings are now action-oriented per Diataxis how-to conventions
  • Qualified HA claims — single-replica Redis master can't actually failover; intro now says "optional failover when replicas are configured"
  • Expanded default user warning — disabling the default user also requires configuring Sentinel auth (sentinel auth-user/sentinel auth-pass) and updating readiness probes
  • Removed enabled: true from TLS blocks — cross-checked with deployed cluster; the field isn't needed, just providing caCertSecretRef is sufficient
  • Removed port: 26379 from sentinelService — the controller defaults it server-side
  • Added > prefix explanation — clarified that > before the password in the ACL entry is Redis syntax, not part of the placeholder
  • Standardized credentials — username toolhive-auth and placeholder <REDIS_ACL_PASSWORD> consistent across redis-session-storage.mdx and auth-k8s.mdx
  • Removed configuration reference table — prone to drift as CRDs change; linked to the CRD reference page instead
  • Strengthened TLS recommendation — reworded to make it clear TLS should be enabled for anything beyond local dev
  • Updated all cross-linksbackend-auth.mdx and auth-k8s.mdx now point to the new location

Replace <REDIS_ACL_PASSWORD> with YOUR_REDIS_ACL_PASSWORD (no angle
brackets) to avoid confusing ><PLACEHOLDER> syntax in Redis ACL entries.
Remove unused vercel.json redirect since the page was never published at
the old integrations/ path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jhrozek jhrozek merged commit b187f44 into main Mar 16, 2026
4 checks passed
@jhrozek jhrozek deleted the authserver-redis-storage branch March 16, 2026 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants