feat: add digital ocean snapshot builder - #2345
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis pull request adds DigitalOcean deployment infrastructure for Alby Hub, including Packer snapshot creation, Ubuntu provisioning, Docker Compose runtime setup, boot messaging, image cleanup, Marketplace validation, and Marketplace update submission. ChangesDigitalOcean Droplet Deployment Infrastructure
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Operator
participant build.sh
participant Packer
participant DigitalOcean
participant Marketplace
Operator->>build.sh: Set version and API token
build.sh->>Packer: Build template.json
Packer->>DigitalOcean: Create and provision snapshot
DigitalOcean-->>Packer: Write manifest.json
Operator->>Marketplace: Run marketplace-submit.sh
Marketplace->>DigitalOcean: PATCH Marketplace update
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 3
🧹 Nitpick comments (2)
deploy/digitalocean/scripts/012-grub-opts.sh (1)
3-4: ⚡ Quick winMake the GRUB update idempotent.
Line 3 can append duplicate
cgroup_enable=memory swapaccount=1values if rerun. Add a guard so the flags are only inserted once.Suggested patch
-sed -e 's|GRUB_CMDLINE_LINUX="|GRUB_CMDLINE_LINUX="cgroup_enable=memory swapaccount=1|g' \ - -i /etc/default/grub +if ! grep -q 'cgroup_enable=memory swapaccount=1' /etc/default/grub; then + sed -e 's|GRUB_CMDLINE_LINUX="|GRUB_CMDLINE_LINUX="cgroup_enable=memory swapaccount=1 |g' \ + -i /etc/default/grub +fi🤖 Prompt for 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. In `@deploy/digitalocean/scripts/012-grub-opts.sh` around lines 3 - 4, The current sed invocation can duplicate the flags because it blindly replaces the GRUB_CMDLINE_LINUX value; change the script to first check /etc/default/grub for the presence of cgroup_enable=memory and swapaccount=1 in the GRUB_CMDLINE_LINUX line and only perform the modification if those flags are missing. Concretely, locate the GRUB_CMDLINE_LINUX line in /etc/default/grub (reference symbol: GRUB_CMDLINE_LINUX) and if the flags are absent, update that line to include them (and then run update-grub); otherwise leave the file unchanged to make the operation idempotent.deploy/digitalocean/template.json (1)
5-5: ⚡ Quick winUse
linux-image-extra-virtual-hwe-24.04for optimal Noble compatibility.Line 5 includes
linux-image-extra-virtual, which exists on Ubuntu 24.04 but is not the recommended variant. Replace withlinux-image-extra-virtual-hwe-24.04to ensure supported Docker host/kernel compatibility with the HWE (Hardware Enablement) kernel stream tracking on Noble.🤖 Prompt for 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. In `@deploy/digitalocean/template.json` at line 5, The apt_packages entry currently lists "linux-image-extra-virtual"; update the apt_packages string to replace linux-image-extra-virtual with linux-image-extra-virtual-hwe-24.04 so the template uses the HWE kernel variant for Noble compatibility (modify the "apt_packages" value in the JSON to include linux-image-extra-virtual-hwe-24.04 instead of linux-image-extra-virtual).
🤖 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 `@deploy/digitalocean/files/etc/update-motd.d/99-one-click`:
- Line 3: Replace the unreliable hostname-based IP lookup used to set the myip
variable with a call to the DigitalOcean metadata service; specifically, set
myip by querying the metadata endpoint for the droplet's public IPv4 (e.g.,
metadata/v1/interfaces/public/0/ipv4/address) with a short curl timeout and a
sensible fallback (keep the existing hostname-based command as fallback) so the
onboarding URL always uses the droplet's public IP; update the assignment to
myip in the 99-one-click script accordingly.
In `@deploy/digitalocean/scripts/014-ufw-albyhub.sh`:
- Around line 3-4: The ufw forward policy is set to a global ACCEPT which is too
permissive for a hardened image; change the value of DEFAULT_FORWARD_POLICY in
/etc/default/ufw from "ACCEPT" to "DROP" (i.e., ensure the sed replacement sets
DEFAULT_FORWARD_POLICY="DROP") and instead add explicit ufw route/forward rules
only for the specific services or subnets that require forwarding; locate the
occurrence of DEFAULT_FORWARD_POLICY in the sed command and update it, and
document or add corresponding ufw allow/route rules for any required forwarded
traffic.
In `@deploy/digitalocean/scripts/900-cleanup.sh`:
- Line 45: The purge command in 900-cleanup.sh uses sudo and an unquoted glob
which can lead to unintended shell expansion; change the invocation to run as
root (remove sudo) and quote the package pattern (e.g., "droplet-agent*") so the
apt-get --yes purge call in the script invokes apt-get as root and treats the
pattern literally.
---
Nitpick comments:
In `@deploy/digitalocean/scripts/012-grub-opts.sh`:
- Around line 3-4: The current sed invocation can duplicate the flags because it
blindly replaces the GRUB_CMDLINE_LINUX value; change the script to first check
/etc/default/grub for the presence of cgroup_enable=memory and swapaccount=1 in
the GRUB_CMDLINE_LINUX line and only perform the modification if those flags are
missing. Concretely, locate the GRUB_CMDLINE_LINUX line in /etc/default/grub
(reference symbol: GRUB_CMDLINE_LINUX) and if the flags are absent, update that
line to include them (and then run update-grub); otherwise leave the file
unchanged to make the operation idempotent.
In `@deploy/digitalocean/template.json`:
- Line 5: The apt_packages entry currently lists "linux-image-extra-virtual";
update the apt_packages string to replace linux-image-extra-virtual with
linux-image-extra-virtual-hwe-24.04 so the template uses the HWE kernel variant
for Noble compatibility (modify the "apt_packages" value in the JSON to include
linux-image-extra-virtual-hwe-24.04 instead of linux-image-extra-virtual).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3841a427-3041-43f6-bab3-b2a0bbf94aa5
📒 Files selected for processing (13)
deploy/digitalocean/README.mddeploy/digitalocean/build.shdeploy/digitalocean/files/etc/update-motd.d/99-one-clickdeploy/digitalocean/files/opt/albyhub/docker-compose.ymldeploy/digitalocean/files/var/lib/cloud/scripts/per-instance/001_onbootdeploy/digitalocean/files/var/lib/digitalocean/application.infodeploy/digitalocean/scripts/010-docker.shdeploy/digitalocean/scripts/012-grub-opts.shdeploy/digitalocean/scripts/014-ufw-albyhub.shdeploy/digitalocean/scripts/015-albyhub.shdeploy/digitalocean/scripts/020-application-tag.shdeploy/digitalocean/scripts/900-cleanup.shdeploy/digitalocean/template.json
| sed -e 's|DEFAULT_FORWARD_POLICY=.*|DEFAULT_FORWARD_POLICY="ACCEPT"|g' \ | ||
| -i /etc/default/ufw |
There was a problem hiding this comment.
Forward policy is overly permissive for a hardened base image.
Line 3 sets routed/forwarded traffic to global ACCEPT, which broadens network exposure beyond required ingress rules. Prefer default deny for forwarding and explicit route allowances only where needed.
🤖 Prompt for 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.
In `@deploy/digitalocean/scripts/014-ufw-albyhub.sh` around lines 3 - 4, The ufw
forward policy is set to a global ACCEPT which is too permissive for a hardened
image; change the value of DEFAULT_FORWARD_POLICY in /etc/default/ufw from
"ACCEPT" to "DROP" (i.e., ensure the sed replacement sets
DEFAULT_FORWARD_POLICY="DROP") and instead add explicit ufw route/forward rules
only for the specific services or subnets that require forwarding; locate the
occurrence of DEFAULT_FORWARD_POLICY in the sed command and update it, and
document or add corresponding ufw allow/route rules for any required forwarded
traffic.
|
fly and render files are in the root, I wonder if it's possible / makes sense to move them so all the third party deployment options are in this deploy folder |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
deploy/digitalocean/scripts/900-cleanup.sh (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove
--force-yes. It’s deprecated inapt-get, and this upgrade path doesn’t appear to need the broad--allow-*equivalents.🤖 Prompt for 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. In `@deploy/digitalocean/scripts/900-cleanup.sh` at line 24, Update the apt-get upgrade command in the cleanup script to remove the deprecated --force-yes option, while preserving the existing configuration and noninteractive upgrade flags.deploy/digitalocean/scripts/999-img-check.sh (2)
1-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNote the canonical upstream source for future syncing.
This is a verbatim copy of DigitalOcean's official
99-img-check.sh(v1.8.1) fromdigitalocean/marketplace-partners. Other marketplace forks explicitly document this and provide an update mechanism, e.g. one repo notes: "The image validation script in common/scripts/999-img_check.sh is copied from the marketplace-partners repo. The marketplace-partners repo is the script's canonical source, so make sure you're using the latest version from there." Consider adding a similar note/link here so future maintainers know to pull upstream security/bugfixes rather than patch this copy in place.🤖 Prompt for 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. In `@deploy/digitalocean/scripts/999-img-check.sh` around lines 1 - 7, Add a concise maintenance comment near the header of the 999-img-check.sh copy identifying DigitalOcean's marketplace-partners repository as the canonical upstream source and instructing maintainers to sync future security and bug fixes from that source rather than patching this copy independently.
624-629: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the dead GPU compatibility guard at
deploy/digitalocean/scripts/999-img-check.sh:624-629. This branch only prints the skip message; if GPU support is planned, add the missing script instead.🤖 Prompt for 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. In `@deploy/digitalocean/scripts/999-img-check.sh` around lines 624 - 629, Remove the entire GPU compatibility guard around the check_gpu_support.sh source in the image-check script, including its fallback skip message. Do not replace it with another guard; the planned GPU support should be handled by adding the missing check_gpu_support.sh script separately.
🤖 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 `@deploy/digitalocean/scripts/900-cleanup.sh`:
- Line 50: Update the cleanup command around the dd operation so it runs sync
before deleting /zerofile, and use rm -f for the removal while preserving the
existing failure flow.
In `@deploy/digitalocean/scripts/999-img-check.sh`:
- Around line 163-204: Remove the raw file-content printing from both the
checkRoot and checkUsers SSH-key validation branches: eliminate the cat/akey and
“File Contents” output for populated authorized_keys and id_rsa files. Preserve
the existing FAIL messages, paths, counters, and status updates so failures
remain clearly reported without exposing credential material in logs.
---
Nitpick comments:
In `@deploy/digitalocean/scripts/900-cleanup.sh`:
- Line 24: Update the apt-get upgrade command in the cleanup script to remove
the deprecated --force-yes option, while preserving the existing configuration
and noninteractive upgrade flags.
In `@deploy/digitalocean/scripts/999-img-check.sh`:
- Around line 1-7: Add a concise maintenance comment near the header of the
999-img-check.sh copy identifying DigitalOcean's marketplace-partners repository
as the canonical upstream source and instructing maintainers to sync future
security and bug fixes from that source rather than patching this copy
independently.
- Around line 624-629: Remove the entire GPU compatibility guard around the
check_gpu_support.sh source in the image-check script, including its fallback
skip message. Do not replace it with another guard; the planned GPU support
should be handled by adding the missing check_gpu_support.sh script separately.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5dbcba02-4af5-4dd0-80b3-bd0cefcd7c48
📒 Files selected for processing (3)
deploy/digitalocean/scripts/900-cleanup.shdeploy/digitalocean/scripts/999-img-check.shdeploy/digitalocean/template.json
🚧 Files skipped from review as they are similar to previous changes (1)
- deploy/digitalocean/template.json
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
deploy/digitalocean/README.md (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the Packer plugin version used for builds.
Without a version argument,
packer plugins installinstalls the latest plugin, so future upstream releases can change snapshot builds unexpectedly. Pin the tested version here and in the build configuration. (developer.hashicorp.com)Suggested change
-packer plugins install github.com/digitalocean/digitalocean +packer plugins install github.com/digitalocean/digitalocean <tested-version>🤖 Prompt for 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. In `@deploy/digitalocean/README.md` at line 17, Pin the DigitalOcean Packer plugin to the tested version by adding an explicit version constraint to the packer plugins install command in the README, and apply the same version in the corresponding build configuration.deploy/digitalocean/scripts/marketplace-submit.sh (1)
22-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueQuote variables to prevent globbing and word splitting.
Ensure variable expansions are double-quoted to comply with Shellcheck and avoid potential formatting issues.
♻️ Proposed fix
-echo ${ALBYHUB_VERSION} - -echo https://api.digitalocean.com/api/v1/vendor-portal/apps/${DIGITALOCEAN_APP_ID} +echo "${ALBYHUB_VERSION}" + +echo "https://api.digitalocean.com/api/v1/vendor-portal/apps/${DIGITALOCEAN_APP_ID}"🤖 Prompt for 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. In `@deploy/digitalocean/scripts/marketplace-submit.sh` around lines 22 - 24, Update the ALBYHUB_VERSION expansion in the marketplace submission script to use double quotes, preventing globbing and word splitting while preserving the existing output.Source: Linters/SAST tools
🤖 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 `@deploy/digitalocean/scripts/marketplace-submit.sh`:
- Line 1: Remove the -x execution-tracing flag from the script shebang to
prevent DIGITALOCEAN_API_TOKEN leakage in CI logs. Add strict shell handling
with set -euo pipefail so missing manifest.json files or unset variables such as
ALBYHUB_VERSION fail safely, and ensure the existing curl failure handling
remains reachable under set -e.
- Around line 26-36: Update the curl handling in the marketplace update flow to
evaluate the curl command directly as the if condition, preserving the existing
success and failure branches without checking $? afterward. Double-quote the
DigitalOcean API URL, including the DIGITALOCEAN_APP_ID expansion, to prevent
word splitting.
---
Nitpick comments:
In `@deploy/digitalocean/README.md`:
- Line 17: Pin the DigitalOcean Packer plugin to the tested version by adding an
explicit version constraint to the packer plugins install command in the README,
and apply the same version in the corresponding build configuration.
In `@deploy/digitalocean/scripts/marketplace-submit.sh`:
- Around line 22-24: Update the ALBYHUB_VERSION expansion in the marketplace
submission script to use double quotes, preventing globbing and word splitting
while preserving the existing output.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e5f4514-3335-4b15-beda-b98d80756b29
📒 Files selected for processing (3)
deploy/digitalocean/README.mddeploy/digitalocean/scripts/marketplace-submit.shdeploy/digitalocean/template.json
🚧 Files skipped from review as they are similar to previous changes (1)
- deploy/digitalocean/template.json
| curl --fail-with-body -X PATCH -H "Content-Type: application/json" -H "Authorization: Bearer ${DIGITALOCEAN_API_TOKEN}" \ | ||
| -d @update.json https://api.digitalocean.com/api/v1/vendor-portal/apps/${DIGITALOCEAN_APP_ID} | ||
|
|
||
| if [ $? -eq 0 ] | ||
| then | ||
| echo "Digital Ocean Market Place update complete" | ||
| rm update.json | ||
| else | ||
| echo "Digital Ocean Market Place update failed" | ||
| exit 1 | ||
| fi No newline at end of file |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Evaluate the curl command directly for robust error handling.
If set -e is enabled, a failing curl command will cause the script to exit immediately before evaluating if [ $? -eq 0 ], completely bypassing the failure logging. Evaluating the command directly inside the if statement resolves this and complies with Shellcheck.
Also, the URL parameter should be double-quoted to prevent word splitting.
🛠️ Proposed fix
-curl --fail-with-body -X PATCH -H "Content-Type: application/json" -H "Authorization: Bearer ${DIGITALOCEAN_API_TOKEN}" \
- -d `@update.json` https://api.digitalocean.com/api/v1/vendor-portal/apps/${DIGITALOCEAN_APP_ID}
-
-if [ $? -eq 0 ]
-then
+if curl --fail-with-body -X PATCH -H "Content-Type: application/json" -H "Authorization: Bearer ${DIGITALOCEAN_API_TOKEN}" \
+ -d `@update.json` "https://api.digitalocean.com/api/v1/vendor-portal/apps/${DIGITALOCEAN_APP_ID}"; then
echo "Digital Ocean Market Place update complete"
rm update.json
else📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| curl --fail-with-body -X PATCH -H "Content-Type: application/json" -H "Authorization: Bearer ${DIGITALOCEAN_API_TOKEN}" \ | |
| -d @update.json https://api.digitalocean.com/api/v1/vendor-portal/apps/${DIGITALOCEAN_APP_ID} | |
| if [ $? -eq 0 ] | |
| then | |
| echo "Digital Ocean Market Place update complete" | |
| rm update.json | |
| else | |
| echo "Digital Ocean Market Place update failed" | |
| exit 1 | |
| fi | |
| if curl --fail-with-body -X PATCH -H "Content-Type: application/json" -H "Authorization: Bearer ${DIGITALOCEAN_API_TOKEN}" \ | |
| -d `@update.json` "https://api.digitalocean.com/api/v1/vendor-portal/apps/${DIGITALOCEAN_APP_ID}"; then | |
| echo "Digital Ocean Market Place update complete" | |
| rm update.json | |
| else | |
| echo "Digital Ocean Market Place update failed" | |
| exit 1 | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[style] 29-29: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
(SC2181)
🤖 Prompt for 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.
In `@deploy/digitalocean/scripts/marketplace-submit.sh` around lines 26 - 36,
Update the curl handling in the marketplace update flow to evaluate the curl
command directly as the if condition, preserving the existing success and
failure branches without checking $? afterward. Double-quote the DigitalOcean
API URL, including the DIGITALOCEAN_APP_ID expansion, to prevent word splitting.
Source: Linters/SAST tools
Adds a snapshot builder which we can use to publish on DigitalOcean marketplace
Next steps:
But this PR is fully functional and can also be merged as-is
Summary by CodeRabbit