Added querying for identity version. #19

Merged
merged 2 commits into from Mar 17, 2016

Conversation

Projects
None yet
4 participants
Contributor

perrito666 commented Mar 16, 2016

Openstack support for querying available keystone version through a GET to /, I added support for that.

client/client.go
@@ -108,7 +115,11 @@ var defaultRequiredServiceTypes = []string{"compute", "object-store"}
func newClient(creds *identity.Credentials, auth_method identity.AuthMode, httpClient *goosehttp.Client, logger *log.Logger) AuthenticatingClient {
client_creds := *creds
- client_creds.URL = client_creds.URL + apiTokens
+ if auth_method == identity.AuthUserPassV3 {
@wallyworld

wallyworld Mar 17, 2016

Owner

in cases like this, using switch is better. it explicitly enumerates the auth_methods supported and allows an error to be returned in case a new one is added and the code now updated properly

+ cl := client.NewClient(s.cred, s.authMode, nil)
+ options, err := cl.IdentityAuthOptions()
+ c.Assert(err, gc.IsNil)
+ optionsAvailable := len(options) > 0
@wallyworld

wallyworld Mar 17, 2016

Owner

please test that the options available values belong to the set of the ones we expect

identity/identity.go
+ Versions authInformationVersions `json:"versions"`
+}
+
+func FetchAuthOptions(url string, client *goosehttp.Client) (AuthOptions, error) {
@wallyworld

wallyworld Mar 17, 2016

Owner

even though goose lacks comments where it should have them, we should comment significant new work like this

identity/identity.go
+ opt = AuthOption{Mode: AuthUserPassV3, Endpoint: link}
+ case strings.HasPrefix(version.ID, "v2"):
+ opt = AuthOption{Mode: AuthUserPass, Endpoint: link}
+
@wallyworld

wallyworld Mar 17, 2016

Owner

do we want a warning logged if we get back a result we don't know about?

+
+}
+
+const authInformationBody = `{"versions": {"values": [{"status": "stable", "updated": "2015-03-30T00:00:00Z", "media-types": [{"base": "application/json", "type": "application/vnd.openstack.identity-v3+json"}], "id": "v3.4", "links": [{"href": "%s/v3/", "rel": "self"}]}, {"status": "stable", "updated": "2014-04-17T00:00:00Z", "media-types": [{"base": "application/json", "type": "application/vnd.openstack.identity-v2.0+json"}], "id": "v2.0", "links": [{"href": "%s/v2.0/", "rel": "self"}, {"href": "http://docs.openstack.org/", "type": "text/html", "rel": "describedby"}]}]}}`
@wallyworld

wallyworld Mar 17, 2016

Owner

such a long line, can we split it

@@ -86,4 +89,22 @@ func (openstack *Openstack) SetupHTTP(mux *http.ServeMux) {
openstack.Identity.SetupHTTP(mux)
openstack.Nova.SetupHTTP(mux)
openstack.Swift.SetupHTTP(mux)
+
+ mux.Handle("/", openstack)
@wallyworld

wallyworld Mar 17, 2016

Owner

can we add a comment that this is to handle identify auth requests

@@ -16,6 +17,7 @@ type Openstack struct {
Identity identityservice.IdentityService
Nova *novaservice.Nova
Swift *swiftservice.Swift
+ url string
@wallyworld

wallyworld Mar 17, 2016

Owner

comment what this url is used for

client/client.go
@@ -412,3 +423,31 @@ func (c *authenticatingClient) doAuthenticate() error {
c.tokenId = authDetails.Token
return nil
}
+
+func (c *authenticatingClient) IdentityAuthOptions() (identity.AuthOptions, error) {
+ return c.getAuthOptions()
@wallyworld

wallyworld Mar 17, 2016

Owner

why a separate function?

Owner

wallyworld commented Mar 17, 2016

A few small things.
LGTM

$$let-it-rip$$

Member

jujubot commented Mar 17, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-goose

jujubot added a commit that referenced this pull request Mar 17, 2016

Merge pull request #19 from perrito666/add_identity_version_query
Added querying for identity version.

Openstack support for querying available keystone version through a GET to /, I added support for that.

@jujubot jujubot merged commit 495e6fa into go-goose:v1 Mar 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment