Skip to content
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

Jump to bottom

make harbor script more robust to error response returned by api #1094

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from
Open

make harbor script more robust to error response returned by api #1094

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 34 additions & 0 deletions scripts/ci_harbor_artifact_count.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ local_DATAFED_HARBOR_URL="https://$local_DATAFED_HARBOR_REGISTRY"
URL="$local_DATAFED_HARBOR_URL/api/v2.0/projects/$local_DATAFED_HARBOR_PROJECT/repositories/$local_DATAFED_HARBOR_REPOSITORY/artifacts"
echo "$URL" >> "$LOG_FILE"
data=$(curl -u $local_DATAFED_HARBOR_USERNAME:$local_DATAFED_HARBOR_PASSWORD -s "${URL}?with_tag=$local_DATAFED_HARBOR_IMAGE_TAG" )
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): Variables should be quoted to prevent word splitting issues

Use "$local_DATAFED_HARBOR_USERNAME" and "$local_DATAFED_HARBOR_PASSWORD" to handle cases where these values contain spaces


# In the case that an image has not yet been uploaded the server will return
# a json object of the following format
#
# {
# "errors": [
# {
# "code": "NOT_FOUND",
# "message": "path /api/v2.0/projects/datafed/repositories/datafed/gcs-1086-bug-python-client-pypi-version/artifacts was not found"
# }
# ]
# }


error_code=$(echo $?)
Copy link

Choose a reason for hiding this comment

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

suggestion: Curl error handling could capture and log the actual error message

Consider capturing and logging curl's error output when the command fails to aid in debugging

Suggested change
error_code=$(echo $?)
error_message=$(curl -s -f "$url" 2>&1 || true)
error_code=$?
if [ $error_code -ne 0 ]; then
echo "Curl failed with error: $error_message"
fi

if [ "$error_code" != "0" ]
then
Expand All @@ -67,6 +81,26 @@ then
echo "$data" | jq >> "$LOG_FILE"
fi
number_of_artifacts=$(echo "$data" | jq ' . | length')
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): The error check should happen before getting the array length to avoid skipping error detection

If the response contains an error, jq length will return 0, causing the error check to be skipped. Move the error check before the length calculation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not quite sure what is being proposed here? The error check is dependent on the response being correctly formatted.

Copy link

Choose a reason for hiding this comment

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

The suggestion is to ensure that any error in the API response is detected before attempting to calculate the length of the data. If the response contains an error, using jq ' . | length' will return 0, which might incorrectly suggest that there are no artifacts, rather than indicating an error.

To address this, you could first check if the response contains an error field or status before proceeding to calculate the length. This way, you can handle any errors appropriately before processing the data. For example, you might want to check for an error message or status code in the response JSON and handle it accordingly before using jq to get the length of the artifacts array.


# Make sure the response doesn't contain an error code before assumptions are made
if [ "$number_of_artifacts" != "0" ]
then
ERROR_FOUND=$(echo "$data" | jq -r 'if .errors and (.errors | length > 0) then "TRUE" else "FALSE" end')
if [ "$ERROR_FOUND" == "TRUE" ]
then
ERROR_CODE=$(echo "$data" | jq -r '.errors[0].code')
if [ "$ERROR_CODE" == "NOT_FOUND" ]
then
echo "0"
exit 0
else
echo "Aborting unhandled error $ERROR_CODE" >> "$LOG_FILE"
exit 1
fi
fi
fi

echo "Number of artifacts found: $number_of_artifacts" >> "$LOG_FILE"

# Otherwise we assume the object contains the list of objects
echo "$number_of_artifacts"
Loading