Skip to content

Add URL rewriting in Rails#435

Closed
hunchr wants to merge 2 commits intomainfrom
feature/23056-add-url-rewriting-in-rails
Closed

Add URL rewriting in Rails#435
hunchr wants to merge 2 commits intomainfrom
feature/23056-add-url-rewriting-in-rails

Conversation

@hunchr
Copy link
Member

@hunchr hunchr commented Jan 15, 2026

TICKET-23056

Discussed with @sislr

@hunchr hunchr requested review from coorasse and sislr January 15, 2026 08:29
@hunchr hunchr self-assigned this Jan 15, 2026
Copy link
Member

@coorasse coorasse left a comment

Choose a reason for hiding this comment

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

Why do we do this on the app layer and not cloudflare?

@hunchr
Copy link
Member Author

hunchr commented Jan 15, 2026

Why do we do this on the app layer and not cloudflare?

I'd also prefer to configure it in Cloudflare tbh. But @sislr prefers it to be in the code rather than "hidden in Cloudflare"

@schmijos what's your opinion?

@hunchr hunchr requested a review from schmijos January 15, 2026 12:50
Comment on lines +49 to +52
if (app_host = ENV["APP_HOST"]).present?
match "*path", constraints: ->(req) { req.host != app_host && !req.path.start_with?("/.well-known/") },
to: redirect { |_params, req| "https://#{app_host}#{req.fullpath}" }, via: :all
end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (app_host = ENV["APP_HOST"]).present?
match "*path", constraints: ->(req) { req.host != app_host && !req.path.start_with?("/.well-known/") },
to: redirect { |_params, req| "https://#{app_host}#{req.fullpath}" }, via: :all
end
if (canonical_host = ENV["CANONICAL_HOST"]).present?
match "*path", constraints: ->(req) { req.host != canonical_host && !req.path.start_with?("/.well-known/") },
to: redirect { |_params, req| "https://#{canonical_host}#{req.fullpath}" }, via: :all
end

I'd rather use a separate ENV variable here. APP_HOST is in most cases also set in staging apps AFAIK

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't that what we want? If the env is set in staging/develop, then the deplo.io host redirects to renuoapp.

@hunchr hunchr requested a review from sislr January 19, 2026 08:19
@schmijos
Copy link
Member

I prefer to have this things in code as well if feasible.
The default should not be that we have proxy page rules in Cloudflare.
They are comfortable and fast to set up. But they are confusing to reason about. And we don't test them.

Consider the example here: https://renuo.slack.com/archives/C18D5LVDF/p1768293590120929

In order that a page rule works, you must have at least a record on that domain you want to be effected

Cloudflare needs a domain to point to somewhere for it to be proxyable.
So we end up with defunct DNS records.
In my understanding of the world DNS should come first.
Only after, a proxy should do the rewriting. Cloudflare inversed this:

  1. First it will answer with a Cloudflare IP
  2. Then it will apply the page rules
  3. And only afterwards it will look at your DNS records.

It seems that DNS is not powerful enough for what people want to do.
And I think it's fine to use Cloudflare as a proxy. But we're mixing things up.

@hunchr
Copy link
Member Author

hunchr commented Jan 20, 2026

Okay, should we merge it then? This PR received three comments, but no approvals or requested changes.

@hunchr hunchr requested a review from coorasse January 20, 2026 09:49
@hunchr hunchr force-pushed the feature/23056-add-url-rewriting-in-rails branch from fbc36c2 to 71b2e63 Compare January 23, 2026 15:43
@coorasse
Copy link
Member

I still think this causes unnecessary load on our app. it should rather be on ngnix, but having the app responsible of jiggling with the domains still does not seem right to me: the app should not really care about the domains it responds to.

@coorasse coorasse removed their request for review January 27, 2026 15:11
@hunchr hunchr closed this Jan 27, 2026
@hunchr hunchr removed request for schmijos and sislr January 27, 2026 16:11
@hunchr hunchr deleted the feature/23056-add-url-rewriting-in-rails branch January 27, 2026 16:11
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.

4 participants