Proxy ActiveStorage through Rails instead of redirecting to internal MinIO URL

ldraney commented

2026-05-29 11:51:39 +00:00

Owner

Summary

ActiveStorage's default redirect mode sends a 302 to minio.minio.svc.cluster.local:9000, which is a cluster-internal address the browser can't resolve. This makes all uploaded images appear broken.

Changes

Added config.active_storage.resolve_model_to_route = :rails_storage_proxy in config/environments/production.rb
Rails now streams files from MinIO to the browser instead of redirecting

Test Plan

Deploy to prod and verify images display on /uploads index (thumbnails)
Verify images display on /uploads/:id show page (full-size)
Upload a new photo and confirm it displays immediately

Review Checklist

One-line config change, no logic changes
Only affects production environment
No security implications — proxy runs through Rails auth stack

Issue #41
PR #36 introduced ActiveStorage + MinIO
PR #384 (pal-e-platform) fixed MinIO NetworkPolicy

Closes #41

## Summary ActiveStorage's default `redirect` mode sends a 302 to `minio.minio.svc.cluster.local:9000`, which is a cluster-internal address the browser can't resolve. This makes all uploaded images appear broken. ## Changes - Added `config.active_storage.resolve_model_to_route = :rails_storage_proxy` in `config/environments/production.rb` - Rails now streams files from MinIO to the browser instead of redirecting ## Test Plan - [ ] Deploy to prod and verify images display on `/uploads` index (thumbnails) - [ ] Verify images display on `/uploads/:id` show page (full-size) - [ ] Upload a new photo and confirm it displays immediately ## Review Checklist - [x] One-line config change, no logic changes - [x] Only affects production environment - [x] No security implications — proxy runs through Rails auth stack ## Related Notes - Issue #41 - PR #36 introduced ActiveStorage + MinIO - PR #384 (pal-e-platform) fixed MinIO NetworkPolicy Closes #41

ldraney added 1 commit

2026-05-29 11:51:39 +00:00

Proxy ActiveStorage through Rails instead of redirecting to internal MinIO URL

ci/woodpecker/push/woodpecker Pipeline was successful

Details

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

CI / scan_ruby (pull_request) Has been cancelled

Details

CI / scan_js (pull_request) Has been cancelled

Details

CI / lint (pull_request) Has been cancelled

Details

4646fbd988

Closes #41

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented

2026-05-29 11:52:41 +00:00

Author

Owner

PR #42 Review

DOMAIN REVIEW

Stack: Ruby on Rails (ActiveStorage configuration)

This is a single-line Rails config change in config/environments/production.rb:

config.active_storage.resolve_model_to_route = :rails_storage_proxy

Correctness: The setting is the correct Rails API for switching ActiveStorage from redirect mode (:rails_storage_redirect, the default) to proxy mode (:rails_storage_proxy). This is the documented fix for environments where the storage backend (MinIO in this case) is not directly reachable by the browser -- exactly the situation described in the issue where minio.minio.svc.cluster.local:9000 is a cluster-internal address.

Placement: The line is correctly placed in production.rb only, directly adjacent to the config.active_storage.service line (line 24). Development uses :local storage (no MinIO), and test uses :test storage, so neither environment needs this setting. Good scoping.

Performance consideration (non-blocking): Proxy mode means Rails will stream every file download through the application server instead of issuing a redirect. For a landscaping app with photo uploads, this adds load to the Rails process. This is a known and accepted tradeoff for cluster-internal object stores. If file sizes or traffic grow significantly, consider putting a CDN or nginx proxy in front, but that is a future concern, not a blocker for this PR.

Security: Proxy mode routes file serving through the full Rails middleware stack, including any authentication. This is actually slightly more secure than redirect mode, which hands off a presigned URL. No concerns here.

BLOCKERS

None.

This is a one-line config change using a well-documented Rails API. It introduces no new functionality requiring test coverage -- it changes the transport mechanism for an existing feature. The BLOCKER criteria (missing tests for new functionality, unvalidated input, secrets, DRY violations in auth) do not apply.

NITS

No automated test coverage for the config value itself: A config-level spec asserting Rails.application.config.active_storage.resolve_model_to_route == :rails_storage_proxy in production would catch accidental reverts. Very minor -- the test plan covers this through manual deployment verification, which is appropriate for a config-only change.

SOP COMPLIANCE

Branch named after issue: 41-fix-broken-images-proxy-activestorage-th references issue #41
PR body follows template: Summary, Changes, Test Plan, Related sections all present
Related references issue #41 and prior PRs (#36, #384) for context
Related does not reference a plan slug (noted: no plan exists for this hotfix, which is reasonable for a one-line config fix)
No secrets committed
No scope creep -- single file, single line, directly addressing the issue
Closes #41 present for auto-close on merge

PROCESS OBSERVATIONS

Change failure risk: Very low. This is a supported Rails configuration option with well-understood behavior. The only risk is performance under heavy file-serving load, which is not a concern at current scale.
Deployment frequency: This is a targeted hotfix for broken images introduced by PR #36. Quick turnaround is appropriate.
Documentation: The PR body provides excellent context linking back to the root cause (PR #36 introduced ActiveStorage + MinIO) and the related infrastructure fix (PR #384 for MinIO NetworkPolicy). Good traceability.

VERDICT: APPROVED

## PR #42 Review ### DOMAIN REVIEW **Stack**: Ruby on Rails (ActiveStorage configuration) This is a single-line Rails config change in `config/environments/production.rb`: ```ruby config.active_storage.resolve_model_to_route = :rails_storage_proxy ``` **Correctness**: The setting is the correct Rails API for switching ActiveStorage from redirect mode (`:rails_storage_redirect`, the default) to proxy mode (`:rails_storage_proxy`). This is the documented fix for environments where the storage backend (MinIO in this case) is not directly reachable by the browser -- exactly the situation described in the issue where `minio.minio.svc.cluster.local:9000` is a cluster-internal address. **Placement**: The line is correctly placed in `production.rb` only, directly adjacent to the `config.active_storage.service` line (line 24). Development uses `:local` storage (no MinIO), and test uses `:test` storage, so neither environment needs this setting. Good scoping. **Performance consideration (non-blocking)**: Proxy mode means Rails will stream every file download through the application server instead of issuing a redirect. For a landscaping app with photo uploads, this adds load to the Rails process. This is a known and accepted tradeoff for cluster-internal object stores. If file sizes or traffic grow significantly, consider putting a CDN or nginx proxy in front, but that is a future concern, not a blocker for this PR. **Security**: Proxy mode routes file serving through the full Rails middleware stack, including any authentication. This is actually slightly more secure than redirect mode, which hands off a presigned URL. No concerns here. ### BLOCKERS None. This is a one-line config change using a well-documented Rails API. It introduces no new functionality requiring test coverage -- it changes the transport mechanism for an existing feature. The BLOCKER criteria (missing tests for new functionality, unvalidated input, secrets, DRY violations in auth) do not apply. ### NITS 1. **No automated test coverage for the config value itself**: A config-level spec asserting `Rails.application.config.active_storage.resolve_model_to_route == :rails_storage_proxy` in production would catch accidental reverts. Very minor -- the test plan covers this through manual deployment verification, which is appropriate for a config-only change. ### SOP COMPLIANCE - [x] Branch named after issue: `41-fix-broken-images-proxy-activestorage-th` references issue #41 - [x] PR body follows template: Summary, Changes, Test Plan, Related sections all present - [x] Related references issue #41 and prior PRs (#36, #384) for context - [ ] Related does not reference a plan slug (noted: no plan exists for this hotfix, which is reasonable for a one-line config fix) - [x] No secrets committed - [x] No scope creep -- single file, single line, directly addressing the issue - [x] `Closes #41` present for auto-close on merge ### PROCESS OBSERVATIONS - **Change failure risk**: Very low. This is a supported Rails configuration option with well-understood behavior. The only risk is performance under heavy file-serving load, which is not a concern at current scale. - **Deployment frequency**: This is a targeted hotfix for broken images introduced by PR #36. Quick turnaround is appropriate. - **Documentation**: The PR body provides excellent context linking back to the root cause (PR #36 introduced ActiveStorage + MinIO) and the related infrastructure fix (PR #384 for MinIO NetworkPolicy). Good traceability. ### VERDICT: APPROVED