[misc] Installation fallback for when host is missing openssl dependency#6431
[misc] Installation fallback for when host is missing openssl dependency#6431adikavemen wants to merge 3 commits into
Conversation
Checks if openssl dependency is met, then either operates as before or spins up temporary alpine container to generate keys. Tries to use pre-existing openssl image if present.
📝 WalkthroughWalkthrough
ChangesConditional Secret Generation with Docker Fallback
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@infrastructure_files/getting-started.sh`:
- Around line 352-356: The openssl detection using `if openssl &> /dev/null` is
broken because running openssl without arguments returns exit code 1, making
this condition always false and the subsequent code for setting
NETBIRD_RELAY_AUTH_SECRET and DATASTORE_ENCRYPTION_KEY unreachable. Replace the
condition check with `if command -v openssl &> /dev/null` to properly verify
that the openssl command is available on the system.
- Around line 361-372: The variable OPENSSL_IMAGE needs to be double-quoted in
all usages throughout the script to prevent word splitting and globbing issues.
Add double quotes around $OPENSSL_IMAGE in three locations: the two docker run
commands that use it to generate secrets (NETBIRD_RELAY_AUTH_SECRET and
DATASTORE_ENCRYPTION_KEY), and the docker rmi command that removes the image.
This ensures the variable is treated as a single argument even if it contains
spaces or special characters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6d4d97a-a229-432d-b0d3-8a7d9e01a381
📒 Files selected for processing (1)
infrastructure_files/getting-started.sh
|
|
Hi @pappz & @riccardomanfrin sorry for the ping. Just wanted to get some visibility on this PR. |
|
@adikavemen why not use |
|
@champtar I was unaware of /dev/urandom and how openssl often uses it. Openssl was what was previously used in the script so I was using the same key generation method. Openssl remains the better option as it would use available hardware in keygen where possible. It also uses more bits in encryption which makes openssl more secure than using /dev/urandom this way |



Describe your changes
Added checks for openssl dependency being met in the installation script. When the dependency is not met the script spins up temporary alpine openssl containers to generate keys. Tries to use pre-existing openssl image if present. Handles clean up if images are retrieved.
Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
This PR does not change how the script already functions. It provides a fallback for when openssl is not available on the host.
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here: N/A
Summary by CodeRabbit
Summary by CodeRabbit