Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add reverse proxy setup documentation #4085

Open
wants to merge 14 commits into
base: latest
Choose a base branch
from

Conversation

mcollovati
Copy link
Contributor

Part of #1776

@github-actions github-actions bot added the Language unchecked English language and AsciiDoc formatting checks haven't been done label Jan 23, 2025
@russelljtdyer russelljtdyer added Language checking English language and AsciiDoc formatting checking is in progress and removed Language unchecked English language and AsciiDoc formatting checks haven't been done labels Jan 26, 2025
@russelljtdyer
Copy link
Collaborator

I'll edit this more when it's no longer in Draft mode.

@mshabarov mshabarov requested a review from tepi January 27, 2025 12:38
---


= Deploying Vaadin Applications Behind Reverse Proxies
Copy link
Contributor

Choose a reason for hiding this comment

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

Just an idea: what do you think about a paragraph or block with "Best Practice" or "Recommendation"? Like "Vaadin recommends to add a reverse proxy" or "it's best practice to add a reverse proxy"?

It should emphasis the importance of this topic and is hopefully (?) not controversial - at least I don't know anyone who would say: neh don't need it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an introduction paragraph about benefits of reverse proxying

Connector ajpConnector = new Connector(PROTOCOL);
ajpConnector.setPort(ajpPort);
AbstractAjpProtocol<?> ajpProtocol = (AbstractAjpProtocol<?>) ajpConnector.getProtocolHandler();
ajpProtocol.setSecretRequired(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: this line might be controversal

Copy link
Contributor Author

@mcollovati mcollovati Jan 29, 2025

Choose a reason for hiding this comment

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

Updated setting the secret and adding a note about its usage on Apache httpd

RewriteEngine on
RewriteCond %{HTTP:Upgrade} websocket [NC]
RewriteCond %{HTTP:Connection} upgrade [NC]
RewriteRule ^/?(.*) "ws://vaadin-app:8080/$1" [P,L]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be one or two comments: be careful about different ports for AJP and HTTP/WS - and - WS / Rewrite is only required if Push is used with websockets or xhr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good points.
Do you think it would be better to have the comments in the configuration snippets or as a note in the documentation narrative?

Copy link
Contributor

@knoobie knoobie Jan 28, 2025

Choose a reason for hiding this comment

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

I'm wondering if it would be enough to have a simple "What is web sockets" block that expains like the basic.. it's optional.. is used for asyncronous communication (pushes messages from the server to the client).. uses the http protocol.. ajp does not support it; it requires an http upgrade)

and afterwards add // -- Websockets only (begin) + // -- Websockets only (end) to one example below that.. maybe? I would not want to add this comment in every config tho

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be related: Do you think it's worth to have "so many" AJP configurations in the docs? Do we think it's a widespread combination or more like "legacy"? I had this discussion 2-3x in the past with different architects.. Especially since you need to manually configure it in spring boot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a paragraph with a short websockets introduction

proxy_read_timeout 90;
----
--

Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be a chapter about balancing? Or at least mention the pitfalls with stateful flow sessions? Sticky cookies as solution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am currently working on it. Will push changes tomorrow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

drafted something

@mcollovati
Copy link
Contributor Author

For reviewers that want to test the configuration, here's the repository I used https://github.com/mcollovati/vaadin-reverse-proxy-tests

@tepi
Copy link
Contributor

tepi commented Feb 3, 2025

Looks quite good to me, only commented on some typos etc I happened to notice.

@mcollovati mcollovati marked this pull request as ready for review February 3, 2025 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language checking English language and AsciiDoc formatting checking is in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants