feat: replace afero-s3 with minimal in-tree S3 implementation#2348
Open
maggch97 wants to merge 3 commits intogo-vikunja:mainfrom
Open
feat: replace afero-s3 with minimal in-tree S3 implementation#2348maggch97 wants to merge 3 commits intogo-vikunja:mainfrom
maggch97 wants to merge 3 commits intogo-vikunja:mainfrom
Conversation
Replace the third-party afero-s3 library (and its temporary fork) with a minimal in-tree afero.Fs implementation (~200 lines) that only supports the three S3 operations Vikunja actually uses: Open (GetObject), Remove (DeleteObject), and Stat (HeadObject). The s3File implementation lazily opens a GetObject stream on first Read and supports Seek by closing and re-opening the stream from the new offset, matching the behavior of the fixed afero-s3 fork. All other afero.Fs/File methods return ErrS3NotSupported since they are never called in the S3 code path (writes already use direct PutObject). This removes the fclairamb/afero-s3 dependency and the temporary replace directive in go.mod entirely. Closes go-vikunja#2347 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
http.ServeContent requires io.ReadSeeker which S3 files don't support. Add S3 branch using io.Copy with explicit headers, matching the pattern already used in task attachment downloads. Refs: go-vikunja#2347 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kolaente
requested changes
Mar 3, 2026
| ) | ||
|
|
||
| // s3Fs is a minimal afero.Fs implementation backed by S3. | ||
| // It only supports Open (read), Remove, and Stat — the operations |
Member
There was a problem hiding this comment.
No create or write? how exactly does that work then to create upload attachments?
Member
There was a problem hiding this comment.
Okay, but if we're bypassing the s3 fs there already, what advantage do we have to reimplement it? (albeit partially)
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.
Summary
Replaces the third-party
fclairamb/afero-s3library (and its temporary fork) with a minimal in-treeafero.Fsimplementation (~200 lines) that only supports the three S3 operations Vikunja actually uses:HeadObject(existence check) + lazyGetObjecton firstReadHeadObject+DeleteObjectHeadObjectAll other
afero.Fs/afero.Filemethods returnErrS3NotSupportedsince they are never called in the S3 code path (writes already use directPutObject).Changes
pkg/files/s3fs.go(new) — Minimal S3afero.Fs+afero.File+os.FileInfoimplementationpkg/files/filehandling.go— Replaceaferos3.NewFsFromClient()withnewS3Fs()pkg/routes/api/v1/user_export.go— Add S3 branch for export download (io.Copyinstead ofhttp.ServeContentwhich requiresio.ReadSeeker)go.mod/go.sum— Removefclairamb/afero-s3dependency andreplacedirectiveWhy
replacedirectiveCloses #2347