goose: all current code

identity/identity.go

+package identity
+
+import (
+        "os"
+)
+
+const (
+        AUTH_LEGACY = iota

[rog, 2012/11/26 12:51:38]

authLegacy ?

[jameinel, 2012/11/26 16:30:58]

This one is actually part of the public api. Though AuthLegacy would be ok.

[rog, 2012/11/26 18:27:36]

yeah, i saw that. although, given that the auth method is an argument to
NewOpenStackClient, perhaps it wouldn't be unreasonable to ask clients to pass
in an auth method value themselves, rather than a constant that implies one.
alternatively, you could make these constants of a type that has a Method()
Authenticator method.

requiring external clients to enumerate all the possible auth methods when we
can easily make the linkage here seems somewhat unnecessary.

i'm probably missing something crucial though.

+        AUTH_USERPASS
+)
+
+type AuthDetails struct {

[rog, 2012/11/26 12:51:38]

doc comment

+        TokenId     string
+        TenantId    string
+        UserId      string
+        ServiceURLs map[string]string
+}
+
+type Credentials struct {

[rog, 2012/11/26 12:51:38]

doc comment

+        URL        string // The URL to authenticate against
+        User       string // The username to authenticate as
+        Secrets    string // The secrets to pass
+        Region     string // Region to send requests to
+        TenantName string // The tenant information for this connection
+}
+
+type Authenticator interface {

[rog, 2012/11/26 12:51:38]

doc comment - particularly crucial here.

+        Auth(creds *Credentials) (*AuthDetails, error)
+}
+
+func getConfig(envVars ...string) (value string) {
+        value = ""
+        for _, v := range envVars {
+                value = os.Getenv(v)

[rog, 2012/11/26 12:51:38]

if value := os.Getenv(v); value != "" {
    return value
}

then return "" at the end and lose the named return variable.

+                if value != "" {
+                        break
+                }
+        }
+        return
+}
+
+func CredentialsFromEnv() *Credentials {

[rog, 2012/11/26 12:51:38]

doc comment.

are some of these credentials optional? ISTM that some reflection code in
environ/openstack could be avoided if this function returned an error.

+        return &Credentials{
+                URL:        getConfig("OS_AUTH_URL"),
+                User:       getConfig("OS_USERNAME", "NOVA_USERNAME"),
+                Secrets:    getConfig("OS_PASSWORD", "NOVA_PASSWORD"),
+                Region:     getConfig("OS_REGION_NAME", "NOVA_REGION"),
+                TenantName: getConfig("OS_TENANT_NAME", "NOVA_PROJECT_ID"),
+        }
+}