-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add ingress #322
Add ingress #322
Conversation
WalkthroughThe configuration file for Vaultwarden (Bitwarden) has been updated to include new ingress-related settings. The additions consist of enabling ingress, specifying the ingress port as 7277, and supporting streaming over ingress. The existing configuration structure remains unchanged, with other parameters intact. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Looks like job failed because I can't add enhancement label |
We cannot add ingress to this add-on, as it requires the use of SSL, which cannot be enforced via this route. Nevertheless, thanks for being willing to contribute 👍 ../Frenck |
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: 0
🧹 Outside diff range and nitpick comments (2)
bitwarden/config.yaml (2)
14-16
: LGTM! Consider adding a comment for clarity.The addition of ingress configuration looks good and aligns well with the PR objectives. The ingress port (7277) matches the existing Web interface port, which is great for consistency.
Consider adding a brief comment above these new lines to explain the purpose of ingress for future maintainers. For example:
# Enable ingress for easier initial setup and access ingress: true ingress_port: 7277 ingress_stream: true
Update the addon's documentation to include ingress configuration
The ingress settings have been successfully added to
bitwarden/config.yaml
. However, there are no mentions of ingress in the documentation. Please update the addon's documentation or README to reflect the new ingress functionality and provide guidance on its usage.🔗 Analysis chain
Line range hint
1-37
: Verify documentation and code changes for ingress supportThe ingress configuration has been added successfully and is consistent with the existing settings. To ensure a smooth integration, please consider the following:
- Update the addon's documentation or README to reflect the new ingress functionality and how users can benefit from it.
- Verify if any changes are needed in the addon's code to fully support ingress. This might include adjusting internal routing or handling ingress-specific requests.
To help with the verification process, you can run the following script to check for any mentions of ingress in the addon's documentation and code:
This script will help identify areas where documentation updates or code adjustments might be necessary to fully support the new ingress functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for ingress-related content in documentation and code # Search for ingress mentions in markdown files (documentation) echo "Searching for ingress mentions in documentation:" rg --type md 'ingress' -C 2 echo -e "\nSearching for ingress-related code:" # Search for ingress-related code in Python files rg --type python '(ingress|request\.headers\.get\(["'']X-Ingress-Path["''])' -C 2 # Search for ingress-related code in JavaScript files rg --type js '(ingress|window\.ingress)' -C 2Length of output: 529
Script:
#!/bin/bash # Description: Corrected search for ingress-related content in documentation and code echo "Searching for ingress mentions in markdown files:" rg --type markdown 'ingress' -C 2 echo -e "\nSearching for ingress-related code in Python files:" rg --type py '(ingress|request\.headers\.get\(["'']X-Ingress-Path["''])' -C 2 echo -e "\nSearching for ingress-related code in JavaScript files:" rg --type js '(ingress|window\.ingress)' -C 2 echo -e "\nSearching for ingress-related code in YAML files:" rg --type yaml 'ingress' -C 2Length of output: 1207
Wouldn't it be the same experience with or without ingress? Or is there some conditions I'm not understanding? |
Proposed Changes
Related Issues
Summary by CodeRabbit