Skip to content
This repository has been archived by the owner on Aug 14, 2018. It is now read-only.

Credentials as a parameter to sign. #16

Merged
merged 3 commits into from
Jun 2, 2014
Merged

Credentials as a parameter to sign. #16

merged 3 commits into from
Jun 2, 2014

Conversation

daneuriona
Copy link

Adding a variadic option to sign calls so that we can provide credentials directly if we like instead of using ENV variables.

…ials directly if we like instead of using ENV variables.
signMutex.Lock()
defer signMutex.Unlock()
checkKeys()
if cred == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make all of these a len check like above

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b7d78fd shows up in the Pull Request, but the diff still shows the old check.

@peterbourgon
Copy link

Am I reading this correctly that you're using variadic function parameters as a sort of poor-man's function overloading?

@daneuriona
Copy link
Author

Got rid of global Keys to try and make awsauth reenterant via 3c21e95. If user doesn't specifically pass Credentials they will fall back to env definitions. Not sure about what a rich man's function overloading in Go would look like, I'm new to the language. If there is a better way of going about it, I'd settle for upper middle class. My bad.

mholt added a commit that referenced this pull request Jun 2, 2014
Credentials as a parameter to all Sign functions
@mholt mholt merged commit 41cdffe into master Jun 2, 2014
@mholt mholt deleted the variadic-credentials branch June 2, 2014 21:17
@peterbourgon
Copy link

Not sure about what a rich man's function overloading in Go would look like . . . If there is a better way of going about it, I'd settle for upper middle class.

func Sign(req *http.Request, creds Credentials) *http.Request {
    // ...
}

func SignFromEnv(req *http.Request) *http.Request {
    return Sign(req, Credentials{
        os.Getenv(...),
        // ...
    })
}

Or, better yet, avoid magic altogether.

func Sign(req *http.Request, creds Credentials) *http.Request {
    // ...
}

func MakeCredentialsFromEnv() (Credentials, error) {
    key := os.Getenv(...)
    if key == "" {
        return Credentials{}, fmt.Errorf("AWSKEY not in env (or whatever)")
    }
    // ...
   return Credentials{key, ...}, nil
}

creds, err := awsauth.MakeCredentialsFromEnv()
if err != nil {
    log.Fatal(err)
}
req = awsauth.Sign(req, creds)

@daneuriona
Copy link
Author

So no optional params then. Are variadic params dangerous in some way I'm not realizing? Maybe opening the door to pass huge objects, like buffer overrun or something of that nature?

@peterbourgon
Copy link

Variadic parameters are designed for use in e.g. printf-style functions, where some amount of reflection over the parameter list is unavoidable.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants