Skip to content

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

5 participants