rackspace / gophercloud

A Go SDK for OpenStack. IN FEATURE FREEZE. See Issue #592
http://gophercloud.io
Other
456 stars 181 forks source link

Stack Overflow on Reauth to Rackspace Identity #558

Closed reaperhulk closed 8 years ago

reaperhulk commented 8 years ago

I have a system using gophercloud to upload/download files from Rackspace Cloud Files. The relevant code looks something like this:

var client *gophercloud.ServiceClient

func init() {
    username := os.Getenv("CLOUD_FILES_USERNAME")
    apiKey := os.Getenv("CLOUD_FILES_API_KEY")
    region := os.Getenv("CLOUD_FILES_REGION")

    if username == "" || apiKey == "" || region == "" {
        log.Println("No CloudFiles auth info")
        return
    }

    provider, err := rackspace.AuthenticatedClient(gophercloud.AuthOptions{
        Username:    username,
        APIKey:      apiKey,
        AllowReauth: true,
    })
    if err != nil {
        log.Println("Failed to connect to Rackspace:", err)
        return
    }

    client, err = rackspace.NewObjectStorageV1(provider, gophercloud.EndpointOpts{
        Region: region,
    })
    if err != nil {
        log.Println("Failed to connect to Rackspace:", err)
        return
    }
}

func Upload(content []byte) error {
    opts := objects.CreateOpts{}

    r := bytes.NewReader(content)
    res := objects.Create(client, containerName, name, r, opts)
    _, err := res.ExtractHeader()

    return err
}

Originally we didn't have Reauth enabled so when the identity token expired we'd see errors. However, once we turned on reauth we eventually see a stack overflow that kills the go process entirely. The (mildly redacted) trace can be seen here.

The trace seems to suggest that under certain conditions reauth may recurse until the stack limit is reached. I've looked a bit into it but haven't spotted what could cause that.

jrperritt commented 8 years ago

Have a look here: https://github.com/rackspace/gophercloud/issues/498 You'll want to limit the number of retries; reauth will definitely recurse to death if left unchecked. I should add a comment about that above the AllowReauth field.

reaperhulk commented 8 years ago

Ah, thank you. I will go ahead and implement those retry limits. A comment would definitely be helpful! :)

jrperritt commented 8 years ago

https://github.com/rackspace/gophercloud/pull/559

reaperhulk commented 8 years ago

👍 thanks for the quick response. I'll go ahead and close this

reaperhulk commented 8 years ago

@jrperritt Reflecting a bit more on this it seems like the set of circumstances that can lead to this crash are quite common so it feels a bit footgun-y to have it be the default reauth behavior. What do you think about having some retry limits be present by default?