-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
nginx_proxy: add use_ssl_backend option to support cases where backend is using SSL #4284
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
base: master
Are you sure you want to change the base?
Conversation
Add 'Use SSL Backend' option to Nginx configuration.
Updated instructions for configuring Home Assistant to use the 'use_ssl_backend' option. Added a new section for the 'use_ssl_backend' option.
Added 'use_ssl_backend' option for SSL configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
📝 WalkthroughWalkthroughThis pull request adds a boolean Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Nginx as "Nginx (proxy)"
participant Backend as "Home Assistant Backend"
Note over Nginx: Config: use_ssl_backend = true/false
Client->>Nginx: HTTP request to /
alt use_ssl_backend = false
Nginx->>Backend: proxy_pass http://homeassistant.local.hass.io:PORT
else use_ssl_backend = true
Nginx->>Backend: proxy_pass https://homeassistant.local.hass.io:PORT
end
Backend-->>Nginx: response
Nginx-->>Client: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @nginx_proxy/CHANGELOG.md:
- Line 5: Fix the double-space typo in the CHANGELOG entry sentence "add
use_ssl_backend option to support cases where the http section is using the
ssl_certificate, ssl_key options" by removing the extra space between "where"
and "the" so it reads "where the http section..."; update the line in
CHANGELOG.md accordingly.
In @nginx_proxy/DOCS.md:
- Line 84: Rewrite the sentence to use an active, authoritative tone and add the
missing period: change "If enabled, configure Nginx to connect with SSL to the
backend, needed if the `http` section is using the `ssl_certificate`, `ssl_key`
options" to a direct instruction that ends with a period and clearly ties the
condition to the `http` section using `ssl_certificate` or `ssl_key`.
In @nginx_proxy/translations/en.yaml:
- Around line 23-26: The description string for use_ssl_backend has an extra
trailing space after the final period; edit the use_ssl_backend -> description
value and remove the trailing space character so the line ends with the period
only (ensure the folded scalar >- value is updated accordingly).
🧹 Nitpick comments (1)
nginx_proxy/DOCS.md (1)
16-16: Front the goal and simplify the instruction.Line 16 is complex and doesn't front the action. According to the coding guidelines, instructions should front the goal and use a direct, authoritative tone.
🔎 Proposed improvement
-2. You must add the following section to your [Home Assistant configuration.yaml](https://www.home-assistant.io/docs/configuration/). If the `http` section is using the `ssl_certificate`, `ssl_key` or `server_port` keys, make sure to use the `use_ssl_backend` option. +2. Add the following section to your [Home Assistant configuration.yaml](https://www.home-assistant.io/docs/configuration/). If your `http` section uses the `ssl_certificate`, `ssl_key`, or `server_port` keys, enable the `use_ssl_backend` option in the nginx addon configuration.Changes:
- Removed "You must" (guideline: avoid politeness, be direct)
- Changed "make sure to use" to "enable" (more direct and actionable)
- Added "in the nginx addon configuration" for clarity on where to enable the option
As per coding guidelines for documentation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (5)
nginx_proxy/CHANGELOG.mdnginx_proxy/DOCS.mdnginx_proxy/config.yamlnginx_proxy/rootfs/etc/nginx/nginx.conf.gtplnginx_proxy/translations/en.yaml
🧰 Additional context used
📓 Path-based instructions (1)
*/**(html|markdown|md)
⚙️ CodeRabbit configuration file
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
*/**(html|markdown|md): - Use bold to mark UI strings.
If "" are used to mark UI strings, replace them by bold.
Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Use sentence-style capitalization also in headings.
do not comment on HTML used for icons
Avoid flagging inline HTML for embedding videos in future reviews for this repository.
Files:
nginx_proxy/CHANGELOG.mdnginx_proxy/DOCS.mdnginx_proxy/config.yaml
🔇 Additional comments (2)
nginx_proxy/config.yaml (1)
2-2: LGTM!The version bump and new configuration option are correctly implemented. The default value of
falseappropriately preserves backward compatibility.Also applies to: 25-25, 40-40
nginx_proxy/rootfs/etc/nginx/nginx.conf.gtpl (1)
93-97: Verify SSL backend certificate validation approach.When
use_ssl_backendis enabled, nginx will proxy to the backend using HTTPS. By default, nginx'sproxy_ssl_verifyis off, meaning it will accept any certificate from the backend (including self-signed or expired certificates).For internal Home Assistant communication on
homeassistant.local.hass.io, this may be intentional. However, confirm whether the current behavior (no explicit SSL verification directives) aligns with the intended security posture.Consider adding directives like
proxy_ssl_verify,proxy_ssl_trusted_certificate, orproxy_ssl_server_nameif stricter validation is needed.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @nginx_proxy/DOCS.md:
- Line 16: Update the sentence in DOCS.md to use a direct tone and remove the
inconsistent reference to server_port: replace "If the `http` section is using
the `ssl_certificate`, `ssl_key` or `server_port` keys, make sure to use the
`use_ssl_backend` option." with a direct authoritative phrasing that only
mentions ssl keys, e.g. "If the `http` section is using the `ssl_certificate` or
`ssl_key` keys, enable the `use_ssl_backend` option." Ensure the revised text
only references `ssl_certificate` and `ssl_key` and uses the word "enable"
instead of "make sure to use."
In @nginx_proxy/translations/en.yaml:
- Around line 23-26: The translation string for the key use_ssl_backend has
"Backend" capitalized in its description; change it to lowercase "backend" so it
reads "If enabled, configure Nginx to use SSL to connect to backend." Locate the
use_ssl_backend block (name and description) and update the description value to
use "backend" instead of "Backend".
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
nginx_proxy/DOCS.mdnginx_proxy/translations/en.yaml
🧰 Additional context used
📓 Path-based instructions (1)
*/**(html|markdown|md)
⚙️ CodeRabbit configuration file
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
*/**(html|markdown|md): - Use bold to mark UI strings.
If "" are used to mark UI strings, replace them by bold.
Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Use sentence-style capitalization also in headings.
do not comment on HTML used for icons
Avoid flagging inline HTML for embedding videos in future reviews for this repository.
Files:
nginx_proxy/DOCS.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Corrected capitalization of 'Backend' to 'backend' in SSL configuration description.
bertschedavid2005-debug
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Top
add use_ssl_backend option to support cases where the http section is using the ssl_certificate, ssl_key options
default to false, so will not affect existing installs, and will so work with default install
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.