HewlettPackard / HewlettPackard/morpheus-cli

Upload returns success on HTTP errors: partial/failed uploads exit 0

Open
#58 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Ruby
Stars
20
Forks
8
Avg merge
57m
Merged PRs (30d)
7

Description

### Description

The `upload()` method in `VirtualImagesInterface` (`lib/morpheus/api/virtual_images_interface.rb:70-134`) does not check the HTTP response status code before returning. It unconditionally parses and returns the response body as JSON:

```ruby
response = http.post(url, http_opts)
# ...
return JSON.parse(response.body.to_s)
```

This means:
- Any HTTP error (4xx, 5xx) is silently ignored: the method returns whatever the server sent back (which may be an error HTML page or a partial JSON object), and the CLI reports success.
- Partial uploads (connection drops, timeouts, incomplete transfers) that result in an HTTP error are never detected: the CLI exits 0.

This is in contrast to all other API methods in the codebase (e.g., `create`, `update`, `destroy`, `upload_by_url`), which use `execute()` (`lib/morpheus/api/api_client.rb:152-274`). The `execute()` method delegates to `Morpheus::RestClient.execute()` which calls `::RestClient::Request.execute`. RestClient automatically raises `RestClient::Exception` for non-2xx responses, so those methods get proper error handling. The `upload()` method bypasses this entirely by using the `http.rb` gem directly with no status checking.

### Affected commands

- `morpheus virtual-images add` (`lib/morpheus/cli/commands/virtual_images.rb:477-676`): When a file is provided during virtual image creation, the upload response is captured at line 661 but never checked for errors:
```ruby
upload_json_response = @virtual_images_interface.upload(virtual_image['id'], image_file, file_name)
```
The method then proceeds to `print_green_success` and `return 0, nil` (line 674) regardless of what `upload_json_response` contains.

- `morpheus virtual-images add-file` (`lib/morpheus/cli/commands/virtual_images.rb:678-758`): The upload response is captured at line 745 but similarly never checked:
```ruby
json_response = @virtual_images_interface.upload(image['id'], image_file, file_name, do_gzip)
```
The method then prints "successfully updated" (line 749) regardless of the actual outcome.

Note: The `rescue RestClient::Exception` at line 754 never fires for upload failures because `upload()` uses `http.rb`, not RestClient. HTTP errors from the upload are silently eaten.

### How to reproduce

1. Start uploading a large file (e.g., 47 GB QCOW2 image) with `morpheus virtual-images add-file `
2. Interrupt the network connection or kill the server mid-transfer (or simply let a timeout occur)
3. The CLI still exits 0 and prints "successfully updated"
4. The file on the server is incomplete/corrupt

### Impact

This is a **critical bug for CI/CD pipeline automation**. We cannot rely on the exit code of the Morpheus CLI to determine whether an upload succeeded.
Without external verification, partial uploads would go undetected and result in corrupt virtual images being deployed to production VMs.

To make matters worse, the Morpheus API does not calculate or expose a checksum for uploaded files in the `GET /api/virtual-images/:id` response. The only verification available is `cloudFiles[].contentLength` (file size in bytes). This means there is no way to verify upload integrity through the API, making reliable exit codes from the CLI even more critical.

### Root cause analysis

The `upload()` method was rewritten to use the `http.rb` gem instead of RestClient (see the `# Using http.rb instead of RestClient` comment at line 78), likely to support streaming uploads of large files. However, the migration did not preserve RestClient's automatic status code validation. The `http.rb` gem does **not** raise exceptions on HTTP errors by default: it returns the response object and expects the caller to check `response.status`.

### Suggested fix

Add HTTP status code checking in `upload()` before the `JSON.parse` return, for example:

```ruby
response = http.post(url, http_opts)

if response.status >= 400
# Raise an error consistent with RestClient behavior
error_msg = "HTTP #{response.status}: #{response.body.to_s[0..500]}"
raise RuntimeError, error_msg
end

return JSON.parse(response.body.to_s)
```

Additionally, the callers (`add` and `add_file` commands) should check the returned JSON for error indicators (e.g., `{"success": false}` or `{"errors": {...}}`).

A more robust fix would also consider server-side checksum calculation and exposure via the API (`GET /api/virtual-images/:id` should return checksums in `cloudFiles[]`)

### Environment

- **morpheus-cli version:** 8.0.10.1 (discovered), also confirmed in latest source (9.0.0)
- **Affected file (latest source):** `lib/morpheus/api/virtual_images_interface.rb`, lines 70-134
- **Affected commands:** `virtual-images add`, `virtual-images add-file`

### References

- `upload()` method: [`lib/morpheus/api/virtual_images_interface.rb:70-134`](https://github.com/HewlettPackard/morpheus-cli/blob/master/lib/morpheus/api/virtual_images_interface.rb#L70-L134)
- `add` command handler: [`lib/morpheus/cli/commands/virtual_images.rb:637-674`](https://github.com/HewlettPackard/morpheus-cli/blob/master/lib/morpheus/cli/commands/virtual_images.rb#L637-L674)
- `add_file` command handler: [`lib/morpheus/cli/commands/virtual_images.rb:678-758`](https://github.com/HewlettPackard/morpheus-cli/blob/master/lib/morpheus/cli/commands/virtual_images.rb#L678-L758)
- `execute()` method (correctly handles errors via RestClient): [`lib/morpheus/api/api_client.rb:152-274`](https://github.com/HewlettPackard/morpheus-cli/blob/master/lib/morpheus/api/api_client.rb#L152-L274)

---

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.