Support GET-only HTTP servers for image sources #472
Conversation
libvirt/resource_libvirt_volume.go
Outdated
| @@ -143,12 +143,11 @@ func resourceLibvirtVolumeCreate(d *schema.ResourceData, meta interface{}) error | |||
|
|
|||
| // update the image in the description, even if the file has not changed | |||
| size, err := img.Size() | |||
| if err != nil { | |||
| return err | |||
| if err == nil { | |||
There was a problem hiding this comment.
if we do so, we are not checking and returning the error of the above function
or i am missing something here? 🤔 ( i think in case we need to handle a speicif error, we should do it instead of bypassing the whole errors)
There was a problem hiding this comment.
At the moment there's not many other failure scenarios but I see the general point. If this is the only thing holding back merging just +1 this and I will fix it of course.
There was a problem hiding this comment.
@moio thanks, yop otherwise looks clean to me 🎸
There was a problem hiding this comment.
I like how you've pushed the size information down into the copier (most of a2e28a3). But I'd rather leave this particular hunk as it stands. To have httpImage.Size() support endpoints that 403 HEAD:
$ curl -sLI https://github.com/moio/sumaform-images/releases/download/4.0.0/centos7.qcow2 | grep 'HTTP\|Location'
HTTP/1.1 302 Found
Location: https://github-production-release-asset-2e65be.s3.amazonaws.com/...
HTTP/1.1 403 Forbiddenhow about a "you can't make me read the body" GET ;) :
func (i *httpImage) Size() (uint64, error) {
response, err := http.Head(i.url.String())
if err != nil {
return 0, err
}
if response.StatusCode == 403 {
response, err = http.Get(i.url.String())
if err != nil {
return 0, err
}
response.Body.Close()
}
if response.StatusCode != 200 {
...
}
...
}
|
Oooh how much more I like my patch now! Thank you very much @wking! |
|
For curiosity how @wking helped here? :) i am walking at moment i will merge all pr after including the wking one. Thx for prs |
Some HTTP services, notably GitHub Releases, do not support HEAD requests to their endpoints.
terraform-provider-libvirtwill thus fail on something along the lines of:With:
I observed this phenomenon in sumaform when trying to use images built with Packer in CircleCI (sumaform-images project).
This PR removes the assumption a HEAD request will succeed. If it doesn't, it will defer getting the
Content-Lengthwe need for the libvirt volume definition at GET time.Test code has been adapted too, namely, the internal Web Server also sets
Content-Lengthnow.