Skip to content

feat(seaweedfs): expose JWT expires_after_seconds via values - #2911

Closed
IvanHunters wants to merge 1 commit into
cozystack:mainfrom
IvanHunters:feat/seaweedfs-jwt-ttl
Closed

feat(seaweedfs): expose JWT expires_after_seconds via values#2911
IvanHunters wants to merge 1 commit into
cozystack:mainfrom
IvanHunters:feat/seaweedfs-jwt-ttl

Conversation

@IvanHunters

@IvanHunters IvanHunters commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

The SeaweedFS upstream default JWT TTL is 10 seconds. The current Cozystack chart hardcodes the absence of expires_after_seconds in security.toml, so operators get the 10-second behaviour with no way to override.

For larger backends — most visibly Harbor's S3 backend (the s3aws registry driver against SeaweedFS' S3 frontend) — this default is too short. Blob uploads that take longer than ~10 seconds, or volume → master heartbeat / inter-process traffic on a slow link, end up seeing token has invalid claims: token is expired and the write fails. Symptom from Harbor's perspective:

POST /v2/<project>/<repo>/blobs/uploads/:
UNKNOWN: unknown error;
map[DriverName:s3aws Enclosed:map[RequestFailure:map[]]]

The corresponding SeaweedFS-side error in volume / s3 logs is wrong jwt with reason token is expired. Restarting the volume server forces re-issuing of tokens (with the same 10-second TTL), so the situation recurs.

Change

Add an optional expiresAfterSeconds block under global.seaweedfs.securityConfig.jwtSigning:

global:
  seaweedfs:
    securityConfig:
      jwtSigning:
        volumeWrite: true
        volumeRead: false
        filerWrite: false
        filerRead: false
        expiresAfterSeconds:
          volumeWrite: 3600   # 1 hour, or any positive integer
          volumeRead: 3600
          filerWrite: 3600
          filerRead: 3600

When the value is 0 (the default) the expires_after_seconds line is omitted from security.toml, preserving SeaweedFS' built-in 10-second default for backwards compatibility. When set to a positive integer, the value is emitted under the matching [jwt.*] section.

Test plan

  • helm template with default values — no expires_after_seconds line emitted (current behaviour preserved).
  • helm template with expiresAfterSeconds.volumeWrite: 3600expires_after_seconds = 3600 appears under [jwt.signing], nothing under other sections.
  • Live cluster: bump to 3600, restart master + volume pods, confirm large Harbor uploads succeed and token is expired errors stop appearing in volume logs.

Reference

SeaweedFS source: weed/security/jwt.go reads expiresAfterSec and only sets the exp claim when value is positive. Scaffold (security.toml) documents the 10-second default but doesn't include a way to override via Helm-style config — that's what this PR adds at the Cozystack chart level.

Summary by CodeRabbit

  • New Features
    • JWT token expiration times are now independently configurable for volume and filer operations (read/write). Default settings preserve SeaweedFS's built-in behavior.

SeaweedFS scaffold sets a default JWT TTL of 10 seconds, hardcoded in
the security.toml template — no values override available. Under heavy
load (e.g. large image blob uploads via Harbor's S3 backend) volume
servers report `token has invalid claims: token is expired` and the
write fails.

Add an optional knob:

    global:
      seaweedfs:
        securityConfig:
          jwtSigning:
            expiresAfterSeconds:
              volumeWrite: 3600
              volumeRead: 3600
              filerWrite: 3600
              filerRead: 3600

When the value is 0 (default) the field is omitted, preserving the
existing SeaweedFS default of 10 seconds. Otherwise the value is
emitted as `expires_after_seconds = N` under the matching jwt section.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@github-actions github-actions Bot added size/XS This PR changes 0-9 lines, ignoring generated files area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) kind/feature Categorizes issue or PR as related to a new feature labels Jun 12, 2026
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This pull request adds configurable JWT expiration times to the SeaweedFS Helm chart. The chart values define per-operation lifetime settings (volume write/read, filer write/read), and the security ConfigMap template conditionally renders expiration parameters in JWT sections based on these values, falling back to defaults when set to 0.

Changes

JWT Expiration Configuration

Layer / File(s) Summary
Chart values for JWT expiration configuration
packages/system/seaweedfs/charts/seaweedfs/values.yaml
New global.seaweedfs.securityConfig.jwtSigning.expiresAfterSeconds configuration block with per-operation lifetimes (volumeWrite, volumeRead, filerWrite, filerRead), each defaulting to 0 to preserve SeaweedFS built-in defaults when omitted.
Template implementation for configurable JWT expiration
packages/system/seaweedfs/charts/seaweedfs/templates/shared/security-configmap.yaml
Security ConfigMap template conditionally renders expires_after_seconds in four JWT sections (jwt.signing for volume write, jwt.signing.read for volume read, jwt.filer_signing for filer write, jwt.filer_signing.read for filer read), computing effective TTL per section and emitting the block only when greater than 0.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 In SeaweedFS realms where tokens fly,
Now admins set when JWTs die—
Per role, per read, per write they choose,
Or let the defaults loosely loose!
A configurable Helm chart, clever and spry! 🔐

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: exposing JWT expiration time configuration via Helm chart values, which is the primary objective of this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/system/seaweedfs/charts/seaweedfs/templates/shared/security-configmap.yaml (1)

1-119: ⚠️ Potential issue | 🟠 Major

Align SeaweedFS vendoring workflow for security-configmap changes
packages/system/seaweedfs/Makefile’s update target wipes charts/, re-downloads the upstream SeaweedFS chart, then re-applies only patches/resize-api-server-annotation.diff and patches/disable-ca-key-rotation.patch. The change in packages/system/seaweedfs/charts/seaweedfs/templates/shared/security-configmap.yaml will be lost on the next make update unless it’s already present upstream or added to the patches/ set and applied in update.

  • Should this be upstreamed to github.com/seaweedfs/seaweedfs first, then re-vendored?
  • If not upstreamed, add a new patch for this template/config and apply it in make update.
🤖 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
`@packages/system/seaweedfs/charts/seaweedfs/templates/shared/security-configmap.yaml`
around lines 1 - 119, The change to templates/shared/security-configmap.yaml
will be lost by the packages/system/seaweedfs Makefile's update target because
it re-downloads charts and reapplies only existing patches
(patches/resize-api-server-annotation.diff and
patches/disable-ca-key-rotation.patch); fix by either (A) upstreaming the
template change to github.com/seaweedfs/seaweedfs and then re-running the
vendoring to pick it up, or (B) add a new patch file (e.g.,
patches/security-configmap.patch) that contains the diff for
templates/shared/security-configmap.yaml and update the Makefile's update flow
to apply that patch alongside the existing ones so the change survives make
update; locate references via security-configmap.yaml, the Makefile update
target, and the patches/ directory to implement the chosen approach.

Source: Learnings

🤖 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.

Outside diff comments:
In
`@packages/system/seaweedfs/charts/seaweedfs/templates/shared/security-configmap.yaml`:
- Around line 1-119: The change to templates/shared/security-configmap.yaml will
be lost by the packages/system/seaweedfs Makefile's update target because it
re-downloads charts and reapplies only existing patches
(patches/resize-api-server-annotation.diff and
patches/disable-ca-key-rotation.patch); fix by either (A) upstreaming the
template change to github.com/seaweedfs/seaweedfs and then re-running the
vendoring to pick it up, or (B) add a new patch file (e.g.,
patches/security-configmap.patch) that contains the diff for
templates/shared/security-configmap.yaml and update the Makefile's update flow
to apply that patch alongside the existing ones so the change survives make
update; locate references via security-configmap.yaml, the Makefile update
target, and the patches/ directory to implement the chosen approach.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e0c0dc09-82fb-4f05-9756-3b77e768e763

📥 Commits

Reviewing files that changed from the base of the PR and between 25a6b56 and c335fbe.

📒 Files selected for processing (2)
  • packages/system/seaweedfs/charts/seaweedfs/templates/shared/security-configmap.yaml
  • packages/system/seaweedfs/charts/seaweedfs/values.yaml

@IvanHunters

Copy link
Copy Markdown
Collaborator Author

Closing in favour of upstreaming this. The patch as written modifies files under packages/system/seaweedfs/charts/seaweedfs/ (the vendored chart subtree), which gets wiped on every make update — so the fix wouldn't survive a future chart bump anyway.

Filed upstream as seaweedfs/seaweedfs#9937 asking for the values-driven expires_after_seconds knob in the bundled chart. Once that lands, the next make update in this repo will pick it up automatically, no downstream patch needed.

If the upstream change stalls, a follow-up PR here will reintroduce this as a proper entry under packages/system/seaweedfs/patches/ so make update reapplies it.

IvanHunters added a commit to IvanHunters/cozystack that referenced this pull request Jun 15, 2026
Pulls in upstream SeaweedFS 4.34, which includes:
- SIGHUP-based hot-reload of JWT signing keys
  (seaweedfs/seaweedfs#9826, fixing seaweedfs/seaweedfs#9823).
  Operators recovering from key rotation or mismatch no longer
  need to restart 'weed' processes — kill -HUP is enough.
- Native 'expires_after_seconds' values for jwt.signing /
  jwt.signing.read / jwt.filer_signing / jwt.filer_signing.read,
  exposed under global.seaweedfs.securityConfig.jwtSigning.expiresAfterSeconds.
  This supersedes the patch in the now-closed cozystack#2911.

'make update' driven; patches in 'patches/' reapplied cleanly.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
IvanHunters added a commit to IvanHunters/cozystack that referenced this pull request Jul 22, 2026
Pulls in upstream SeaweedFS 4.34, which includes:
- SIGHUP-based hot-reload of JWT signing keys
  (seaweedfs/seaweedfs#9826, fixing seaweedfs/seaweedfs#9823).
  Operators recovering from key rotation or mismatch no longer
  need to restart 'weed' processes — kill -HUP is enough.
- Native 'expires_after_seconds' values for jwt.signing /
  jwt.signing.read / jwt.filer_signing / jwt.filer_signing.read,
  exposed under global.seaweedfs.securityConfig.jwtSigning.expiresAfterSeconds.
  This supersedes the patch in the now-closed cozystack#2911.

'make update' driven; patches in 'patches/' reapplied cleanly.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) kind/feature Categorizes issue or PR as related to a new feature size/XS This PR changes 0-9 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant