Fixed Lagoon deployment to download CLI client without rate limiting. #1815
Conversation
WalkthroughThe deployment script for installing the Lagoon CLI was updated to construct the download URL directly based on the system's platform and architecture, instead of querying the GitHub API for release information. Informational notes were added, and the process for downloading and installing the binary was streamlined. Changes
Estimated code review effort1 (~4 minutes) Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
scripts/vortex/deploy-lagoon.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-docs
| platform=$(uname -s | tr '[:upper:]' '[:lower:]') | ||
| arch_suffix=$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/') | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle unsupported architectures explicitly instead of silently producing a bad URL
If uname -m returns something other than x86_64 or aarch64 (e.g. armv7l, i686, ppc64le), the sed substitution leaves the value untouched, which leads to a 404 download URL and the script will keep going.
Refactor to fail fast (or extend the mapping) so users immediately see a clear error.
-arch_suffix=$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/')
+case "$(uname -m)" in
+ x86_64) arch_suffix="amd64" ;;
+ aarch64|arm64) arch_suffix="arm64" ;;
+ *)
+ fail "Unsupported architecture: $(uname -m)"
+ exit 1
+ ;;
+esacCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/vortex/deploy-lagoon.sh around lines 92 to 94, the architecture
detection uses sed to map uname -m output but does not handle unsupported
architectures, causing invalid URLs and silent failures. Modify the script to
explicitly check if the architecture is one of the supported values (amd64 or
arm64) after substitution, and if not, print a clear error message and exit the
script immediately to fail fast and avoid continuing with a bad URL.
| download_url="https://github.com/uselagoon/lagoon-cli/releases/latest/download/lagoon-cli-${platform}-${arch_suffix}" | ||
| if [ "${VORTEX_LAGOONCLI_VERSION}" != "latest" ]; then | ||
| lagooncli_download_url="https://api.github.com/repos/uselagoon/lagoon-cli/releases/tags/${VORTEX_LAGOONCLI_VERSION}" | ||
| download_url="https://github.com/uselagoon/lagoon-cli/releases/download/${VORTEX_LAGOONCLI_VERSION}/lagoon-cli-${platform}-${arch_suffix}" | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
VORTEX_LAGOONCLI_VERSION may need normalisation
The releases are tagged with a leading v (e.g. v2.20.0).
If a user sets VORTEX_LAGOONCLI_VERSION=2.20.0 the constructed URL will 404.
Consider normalising or at least validating the value and warning the user.
🤖 Prompt for AI Agents
In scripts/vortex/deploy-lagoon.sh around lines 95 to 98, the
VORTEX_LAGOONCLI_VERSION variable may lack the required leading 'v' in version
tags, causing the download URL to 404. Add logic to check if
VORTEX_LAGOONCLI_VERSION starts with 'v'; if not, prepend 'v' to normalize it
before constructing the download URL. Optionally, add a warning message if
normalization occurs to inform the user.
| note "Downloading Lagoon CLI from ${download_url}." | ||
| curl -L -o "${VORTEX_LAGOONCLI_PATH}/lagoon" "${download_url}" | ||
|
|
||
| note "Installing Lagoon CLI to ${VORTEX_LAGOONCLI_PATH}/lagoon." | ||
| chmod +x "${VORTEX_LAGOONCLI_PATH}/lagoon" |
There was a problem hiding this comment.
Add --fail (or --fail-with-body) to curl to abort on HTTP errors
curl -L returns 0 even for 404/403 responses, so the script can mark an HTML error page executable and continue, ultimately breaking later Lagoon calls. Ensure the download actually succeeds:
-note "Downloading Lagoon CLI from ${download_url}."
-curl -L -o "${VORTEX_LAGOONCLI_PATH}/lagoon" "${download_url}"
+note "Downloading Lagoon CLI from ${download_url}."
+# --fail : non-zero exit on HTTP errors; --silent/--show-error for CI cleanliness
+curl --fail --silent --show-error -L -o "${VORTEX_LAGOONCLI_PATH}/lagoon" "${download_url}"Optionally verify the file is non-empty or checksum-verified before chmod.
📝 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.
| note "Downloading Lagoon CLI from ${download_url}." | |
| curl -L -o "${VORTEX_LAGOONCLI_PATH}/lagoon" "${download_url}" | |
| note "Installing Lagoon CLI to ${VORTEX_LAGOONCLI_PATH}/lagoon." | |
| chmod +x "${VORTEX_LAGOONCLI_PATH}/lagoon" | |
| note "Downloading Lagoon CLI from ${download_url}." | |
| # --fail : non-zero exit on HTTP errors; --silent/--show-error for CI cleanliness | |
| curl --fail --silent --show-error -L -o "${VORTEX_LAGOONCLI_PATH}/lagoon" "${download_url}" | |
| note "Installing Lagoon CLI to ${VORTEX_LAGOONCLI_PATH}/lagoon." | |
| chmod +x "${VORTEX_LAGOONCLI_PATH}/lagoon" |
🤖 Prompt for AI Agents
In scripts/vortex/deploy-lagoon.sh around lines 100 to 104, the curl command
used to download the Lagoon CLI lacks the --fail or --fail-with-body option,
causing it to return success even on HTTP errors like 404 or 403. Modify the
curl command to include --fail or --fail-with-body to ensure it aborts on HTTP
errors. Additionally, add a check after the download to verify the file is
non-empty or matches a checksum before running chmod to avoid marking an invalid
file executable.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1815 +/- ##
===========================================
+ Coverage 70.15% 72.61% +2.45%
===========================================
Files 84 84
Lines 4725 4725
Branches 35 35
===========================================
+ Hits 3315 3431 +116
+ Misses 1410 1294 -116 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit