feat: S3 setup by spokonya · Pull Request #83 · GenerateNU/selfserve

spokonya · 2026-01-27T16:36:23Z

Description

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactoring (code improvement without changing functionality)
Documentation update
Configuration/infrastructure change
Performance improvement
Test coverage improvement

Related Issue(s)

Closes #95
Working towards #51

What Changed?

Added S3 configuration (config/s3.go) with environment variable support for AWS credentials, region, and bucket name
-Added S3 test handler
Integrated S3 storage into app initialization and route setup (server.go)
Made an s3 storage layer that initializes S3 client with credentials, generates time-limited upload URLs for direct client-to-S3 uploads, and allows for server-side file deletion from S3

Testing & Validation

How this was tested

started local server with s3 credentials configured
Created a call for a presigned URL
Used curl -X PUT with the presigned URL to upload a test file directly to s3
verified file appeared in my s3 dashboard

Screenshots/Recordings

Unfinished Work & Known Issues

None, this PR is complete and production-ready
The following items are intentionally deferred:
- user profile picture endpoints
- Frontend UI for uploads

Notes & Nuances

Pre-Merge Checklist

Code Quality

Code follows the project's style guidelines and conventions
Self-review completed (I've reviewed my own code for obvious issues)
No debugging code, console logs, or commented-out code left behind
No merge conflicts with the base branch
Meaningful commit messages that explain the "why"

Testing & CI

All CI checks are passing
All new and existing tests pass locally
Test coverage hasn't decreased (or decrease is justified)
Linting passes without errors

Documentation

Code is self-documenting or includes helpful comments for complex logic
API documentation updated (if backend endpoints changed)
Type definitions are accurate and up-to-date

Reviewer Notes

Areas needing extra attention: ...
Questions for reviewers: I need help with my .env variables. When I was testing I connected to local supabase but I couldn't get connecting to the remote server. It was either telling me that I was missing a required value (which I had) or that "Tenant or user not found"

danctila

nice implementation @spokonya 🔥🔥🔥

Some good things to note

presigned URL usage is great
good separation of concerns b/t config, storage and handler
like the swagger docs 🙏
context propagation is correct

Must fix

Missing an update to the .env.sample
- Make sure to update this as the ticket mentioned "you should probably update env.sample if you've updated your .env" just so it's easier to keep track over time for both engs and the people who will look at this code after us
Swagger import commented out
File location for the S3 storage file

Other suggestions are related to error context, input validation, and nits about code quality/cleanup but overall nothing major to change ✌️

danctila · 2026-02-03T00:19:41Z

backend/cmd/server/main.go


 	"github.com/generate/selfserve/config"
-	_ "github.com/generate/selfserve/docs"
+	//_ "github.com/generate/selfserve/docs"


not sure what happened here but i think this breaks swagger docs

Either uncomment the line or describe why it was removed if it was deliberate (might just be for debugging?)

uncommented

danctila · 2026-02-03T00:22:51Z

backend/internal/service/storage/postgres/s3/s3storage.go

I think the S3 storage location being under storage/postgres/s3/ might not make the most sense architecturally because S3 is not related to Postgres

I would recommend:

backend/internal/service/storage/ |-postgres/ | |----storage.go | |-s3/ |---s3storage.go

Make sure to:

move the file

update all imports necessary

danctila · 2026-02-03T00:27:38Z

backend/internal/service/storage/postgres/s3/s3storage.go

+	if err != nil {
+		return "", err
+	}


shoudn't return raw errors without context - will make debugging more difficult, etc. so we want to follow our err wrapping setup

suggestion for example:

Suggested change

if err != nil {

return "", err

}

if err != nil {

return "", fmt.Errorf("failed to generate presigned URL for key %s: %w", key, err)

}

Also apply similar wrapping to:

DeleteFile

NewS3Storage

danctila · 2026-02-03T00:30:16Z

backend/internal/service/storage/postgres/s3/s3storage.go

+func (s *Storage) GeneratePresignedURL(ctx context.Context, key string, expiration time.Duration) (string, error) {
+
+	presignedURL, err := s.URL.PresignPutObject(ctx, &s3.PutObjectInput{
+		Bucket: aws.String(s.BucketName),
+		Key:    aws.String(key),
+	}, func(opts *s3.PresignOptions) {
+		opts.Expires = expiration


make sure the functions have input validation

func (s *Storage) GeneratePresignedURL(ctx context.Context, key string, expiration time.Duration) (string, error) { if key == "" { return "", fmt.Errorf("key cannot be empty") } if expiration <= 0 { return "", fmt.Errorf("expiration must be positive") }

Also do the same for:

DeleteFile (validating that key is not empty)

danctila · 2026-02-03T00:32:13Z

backend/internal/handler/s3.go

+		return errs.BadRequest("key is required")
+	}
+
+	presignedURL, err := h.S3Storage.GeneratePresignedURL(c.Context(), key, 5*time.Minute)


would be good to have the 5-minute hardcoded expiration either configurable via environment variable or defined as a package level constant.

for example:

const DefaultPresignedURLExpiration = 5 * time.Minute ... presignedURL, err := h.S3Storage.GeneratePresignedURL(c.Context(), key, DefaultPresignedURLExpiration)

danctila · 2026-02-03T00:37:27Z

backend/internal/service/storage/postgres/s3/s3storage.go

+}
+
+func (s *Storage) GeneratePresignedURL(ctx context.Context, key string, expiration time.Duration) (string, error) {
+


Suggested change

danctila · 2026-02-03T00:37:46Z

backend/internal/handler/s3.go

+	return c.JSON(fiber.Map{
+		"presigned_url": presignedURL,
+	})
+}


Suggested change

}

}

go convention to end files with newline character

danctila · 2026-02-03T00:39:07Z

backend/internal/service/storage/postgres/s3/s3storage.go

+	}, nil
+}
+
+func (s *Storage) GeneratePresignedURL(ctx context.Context, key string, expiration time.Duration) (string, error) {


consider adding a contentType parameter to your function.

we want to make the s3 implementation as scalable as possible beyond profile pictures. for profile picture we will want to restrict file types to types like image/jpeg or image/png for example but other uses of s3 won't have the same restrictions

danctila · 2026-02-03T00:40:42Z

backend/internal/service/storage/postgres/s3/s3storage.go

+	URL        *s3.PresignClient
+}
+
+func NewS3Storage(cfg config.S3) (*Storage, error) {


we have upload (presigned PUT) and delete but looks like no way to retrieve files? we may want a GetFile URL generator similar to your other functions like:

func (s *Storage) GenerateGetURL(ctx context.Context, key string, expiration time.Duration) (string, error) { presignedURL, err := s.URL.PresignGetObject(ctx, &s3.GetObjectInput{ Bucket: aws.String(s.BucketName), Key: aws.String(key), }, func(opts *s3.PresignOptions) { opts.Expires = expiration }) ... }

danctila · 2026-02-03T00:42:12Z

backend/internal/handler/s3.go

+// @Accept       json
+// @Produce      json
+// @Param        key  path  string  true  "File key"
+// @Success      200  {string}  string  "Presigned URL"


swagger docs here show @success 200 {string} string but the actual response is a JSON object so it should be

Suggested change

// @Success 200 {string} string "Presigned URL"

// @Success 200 {object} map[string]string "Presigned URL response"

spokonya added 5 commits January 25, 2026 19:21

Add S3 storage configuration and service

f1d1744

finished s3 setup and added testing handler and endpoint

124ca04

chore: tidy go.mod dependencies

21ae351

go.mod tidy pt2

3871a29

github slack workflow

03b47ad

spokonya requested review from Dao-Ho and danctila January 30, 2026 15:42

danctila linked an issue Jan 31, 2026 that may be closed by this pull request

feat: profile picture #51

Open

danctila assigned spokonya Jan 31, 2026

danctila changed the title ~~S3 setup~~ feat: S3 setup Feb 1, 2026

Fix invalid timezone in users_tests.go

9f72dc0

danctila added the needs review label Feb 2, 2026

danctila requested changes Feb 3, 2026

View reviewed changes

danctila added changes requested and removed needs review labels Feb 3, 2026

		}

		func (s *Storage) GeneratePresignedURL(ctx context.Context, key string, expiration time.Duration) (string, error) {

	// @Success 200 {string} string "Presigned URL"
	// @Success 200 {object} map[string]string "Presigned URL response"

Conversation

spokonya commented Jan 27, 2026 • edited by danctila Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of Change

Related Issue(s)

What Changed?

Testing & Validation

How this was tested

Screenshots/Recordings

Unfinished Work & Known Issues

Notes & Nuances

Pre-Merge Checklist

Code Quality

Testing & CI

Documentation

Reviewer Notes

Uh oh!

danctila left a comment

Choose a reason for hiding this comment

Some good things to note

Must fix

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

spokonya commented Jan 27, 2026 •

edited by danctila

Loading