Skip to content

Enhance Terminus Installation Reliability #32

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.

Sign up for GitHub

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Enhance Terminus Installation Reliability #32

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a2cebb
Improve version detection and input parameters
lcatlett Apr 1, 2025
436002c
Add download validation, checksum verification, and installation veri…
lcatlett Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 58 additions & 8 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ inputs:
terminus-version:
description: |
The full version of Terminus to install. If omitted, the latest version is used.
For deterministic builds, specifying a version is recommended.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For deterministic builds, specifying a version is recommended.
Specifying a version provides deterministic results in exchange for not always having the latest version, with the caveat that deprecated versions will eventually be rejected by the the back-end.

I like the idea of calling out specifying a version for deterministic results builds, but I don't think we want to make that a blanket recommendation for all users. Terminus evolves over time and we plan to begin hard-refusing requests from deprecated versions server-side in the near future.

Hard-coding a version would mean the customer would need to update their CI configuration from time to time. For larger enterprise customers, that's a reasonable tradeoff, but it's not a good tradeoff for everyone.

required: false
disable-cache:
description: Disable session cache and force a new session to be initiated.
required: false
default: false
default: "false"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for switching this from boolean to string?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe GHA really handle bool properly in these configs. IIRC, false is already a string, ^ just makes it more explicit.


runs:
using: composite
Expand All @@ -23,23 +24,72 @@ runs:
if: ${{ ! inputs.terminus-version }}
shell: bash
run: |
TERMINUS_RELEASE=$(
curl --silent \
echo "Attempting to find latest Terminus version..."
# Try to get the latest release from GitHub API with proper error handling
if ! TERMINUS_RELEASE=$(curl --silent --fail --max-time 10 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why constrain max-time to 10 seconds for everyone? Could this be made a configuration option rather than hard-coded?

--header 'authorization: Bearer ${{ github.token }}' \
"https://api.github.com/repos/pantheon-systems/terminus/releases/latest" \
| perl -nle'print $& while m#"tag_name": "\K[^"]*#g'
)
| perl -nle'print $& while m#"tag_name": "\K[^"]*#g'); then
echo "::error::Failed to connect to GitHub API to determine latest Terminus version."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This request could fail for a lot of reasons that weren't a connection failure; if there is any http response code at all then the connection was successful.

echo "::error::Please specify a terminus-version input parameter for deterministic builds."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again I don't think we should assert that all users should hard-code their terminus version. Some customers will want deterministic builds, other users will want to stay on the latest version.

exit 1
fi

# Validate we got a proper version string (should be semver format)
if [[ -z "$TERMINUS_RELEASE" || ! "$TERMINUS_RELEASE" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
echo "::error::Unable to determine a valid Terminus version from GitHub API."
echo "::error::Please specify a terminus-version input parameter for deterministic builds."
Comment on lines +40 to +41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current error handling buries any indication of what exactly went wrong. Yes, the current code suffers from that same problem, but while we're improving error handling we should also improve visibility into the details of the error. At minimum, if this condition fails, the value of $TERMINUS_RELEASE should be printed in the error message. Depending on what has gone wrong, $TERMINUS_RELEASE may be empty, and there are probably ways to get more detailed information fro curl about the nature of the problem.

exit 1
fi

echo "Latest Terminus version detected: $TERMINUS_RELEASE"
echo "TERMINUS_RELEASE=$TERMINUS_RELEASE" >> $GITHUB_ENV

- name: Install Terminus
shell: bash
run: |
mkdir $HOME/terminus && cd $HOME/terminus
mkdir -p $HOME/terminus && cd $HOME/terminus
echo "Installing Terminus v$TERMINUS_RELEASE"
curl -L https://github.com/pantheon-systems/terminus/releases/download/$TERMINUS_RELEASE/terminus.phar --output terminus

# Download Terminus with validation
if ! curl --fail -L https://github.com/pantheon-systems/terminus/releases/download/$TERMINUS_RELEASE/terminus.phar --output terminus; then
echo "::error::Failed to download Terminus v$TERMINUS_RELEASE"
exit 1
fi

# Download checksum for verification
if curl --fail -L https://github.com/pantheon-systems/terminus/releases/download/$TERMINUS_RELEASE/terminus.phar.sha256 --output terminus.sha256; then
echo "Verifying download integrity..."
# Fix the checksum file format if needed (ensure it only contains hash and filename)
sed -i.bak 's/^.*\([0-9a-f]\{64\}\).*$/\1 terminus/g' terminus.sha256
if ! sha256sum -c terminus.sha256; then
echo "::error::Checksum verification failed! The downloaded file may be corrupted."
exit 1
fi
echo "Integrity verification passed."
else
echo "::warning::Could not download checksum file. Skipping integrity check."
fi
Comment on lines +60 to +72
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This checksum check should be removed, at least for now. That file doesn't exist, so currently this is just adding another request which is guaranteed to fail.


# Basic file type verification
file_info=$(file terminus)
if ! echo "$file_info" | grep -q -E 'PHP script|executable|.+ASCII.+PHP'; then
echo "::error::Downloaded file is not a PHP script or executable. Got: $file_info"
exit 1
fi
Comment on lines +74 to +79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File type check should be removed. The output of the file command is not consistent across systems, and it's clear you've already encountered this since the grep is attempting to match 3 different possible responses.

It is receiving a different response in Github Actions than what the pattern match currently covers. Adding more conditions to account for this additional format will still not provide full coverage. Additionally it assumes that the file command exists, which will not always be the case.

This check is also made redundant by the terminus --version test at the end. That test will not succeed if what's downloaded isn't the correct type.


# Make executable and create symlink
chmod +x terminus
sudo ln -s $HOME/terminus/terminus /usr/local/bin/terminus
sudo ln -sf $HOME/terminus/terminus /usr/local/bin/terminus
mkdir -p $HOME/.terminus/{cache,plugins}

# Verify installation
echo "Verifying Terminus installation..."
if ! terminus --version; then
echo "::error::Terminus installation verification failed."
exit 1
fi
echo "Terminus installation successful."
env:
TERMINUS_RELEASE: ${{ inputs.terminus-version || env.TERMINUS_RELEASE }}

Expand Down
Loading