Closed josh-padnick closed 6 years ago
A bummer, I just filed this as a GitHub issue with the "newbie" label. It would make for a good trial project :)
Will review shortly.
Ah, it did occur to me that it felt a little trial-projectish, but I wanted to actually use it in my Confluent setup. Thanks for the forthcoming review!
All feedback responded to and tests passing, so merging now.
Fetch is used to download both GitHub files and Release Assets. When downloading a Release Asset, it's helpful to verify the SHA256 or SHA512 hash of the release asset (the "checksum") so that you can confirm the release asset hasn't been changed from the time you first downloaded it. This PR adds optional support for doing that.
This use case came up for me when I was downloading a third-party health checker tool using Fetch. Without verifying the checksum, the downloaded tool could be changed without anyone knowing, a potential security issue given that we do not control the third-party repo. Using bash to validate the checksum is an option, but fetch support leads to slightly cleaner code and avoids platform compatibility issues.
In addition, I opted to make a backwards-incompatible change to simplify the UX. Previously, fetch allowed users to download more than one Release Asset in a single call. Once I implemented this PR, the fetch interface quickly became unwieldy with so many flags to pass in when using multiple release assets.
It occurred to me that (a) downloading multiple release assets is itself probably a less likely use case, and (b) users can simply call fetch multiple times, once for each release asset they want to download. Therefore, I opted for the UX simplicity with no loss of functionality but at the cost of backwards-incompatible change.
Other updates: