make harbor script more robust to error response returned by api #1094
Conversation
Reviewer's Guide by SourceryThe changes improve error handling in a shell script that interacts with Harbor's API to count artifacts. The implementation adds specific handling for the 'NOT_FOUND' error case when an image hasn't been uploaded yet, returning 0 instead of failing. The script now properly parses the error response JSON before making assumptions about the response structure. Sequence diagram for improved error handling in Harbor scriptsequenceDiagram
participant Script
participant HarborAPI
Script->>HarborAPI: GET /api/v2.0/projects/{project}/repositories/{repository}/artifacts
alt Image not uploaded
HarborAPI-->>Script: { "errors": [{ "code": "NOT_FOUND", "message": "..." }] }
Script->>Script: Check error code
Script->>Script: Return 0
else Image uploaded
HarborAPI-->>Script: List of artifacts
Script->>Script: Count artifacts
Script->>Script: Log number of artifacts
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @JoshuaSBrown - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -67,6 +81,26 @@ then | |||
| echo "$data" | jq >> "$LOG_FILE" | |||
| fi | |||
| number_of_artifacts=$(echo "$data" | jq ' . | length') | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Not quite sure what is being proposed here? The error check is dependent on the response being correctly formatted.
There was a problem hiding this comment.
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.
| @@ -44,6 +44,20 @@ | |||
| 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" ) | |||
There was a problem hiding this comment.
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
| # ] | ||
| # } | ||
|
|
||
|
|
||
| error_code=$(echo $?) |
There was a problem hiding this comment.
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
| 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 |
Summary by Sourcery
Enhancements: