Identity 0.2.0 refactor

10 views
Skip to first unread message

Jon Perritt

unread,
Apr 23, 2014, 2:10:23 PM4/23/14
to gopherc...@googlegroups.com
I spent some time refactoring the Identity v2 API to be more in line with the Gophercloud 0.2.0 format. The code can be found here, and the usage is here.

A few things worth mentioning:

1. I've broken out tokens, service catalogs, and endpoints into their own packages. This makes the Identity API more consistent with Compute and Object Storage.
Usage would now look like the following:
ao, err := utils.AuthOptions()
gr, err := token.Get(ao)
token, err = token.Extract(gr)
sc, err := serviceCatalog.Extract(gr)
sce, err = serviceCatalog.Entries(sc)

2. I've used common verbs to refer to HTTP requests. When getting data out of a data structure (such as a map[string]interface{}) I've used the verb 'Extract'.
I think this is more intuitive to understand what's happening; Namely, you make a request to get a token with token.Get and you then extract the token object with token.Extract.

The current OpenStack Identity, OpenStack Compute and Rackspace Monitoring acceptance tests are successfully running for me.

This code was written as an attempt to increase usability and coherence with the other services. I'd appreciate feedback as to whether or not this has been met.

Thoughts?

Samuel Falvo II

unread,
May 6, 2014, 3:19:50 PM5/6/14
to Jon Perritt, gophercloud-dev
Sorry for my tardiness. Work, life, then more work.

On Wed, Apr 23, 2014 at 11:10 AM, Jon Perritt <jrpe...@gmail.com> wrote:
> 1. I've broken out tokens, service catalogs, and endpoints into their own
> packages. This makes the Identity API more consistent with Compute and
> Object Storage.
> Usage would now look like the following:
>
> ao, err := utils.AuthOptions()
> gr, err := token.Get(ao)
> token, err = token.Extract(gr)
> sc, err := serviceCatalog.Extract(gr)
> sce, err = serviceCatalog.Entries(sc)

This is a lot of steps, and I'm afraid that someone new to Gophercloud
won't be able to divine the correct sequence very easily from reading
a godoc. Would it be possible to add this to the utils package to
help automate the process for the common case? Also, might be worth
putting code examples in the doc.go file of the Identity package.

> 2. I've used common verbs to refer to HTTP requests. When getting data out
> of a data structure (such as a map[string]interface{}) I've used the verb
> 'Extract'.
> I think this is more intuitive to understand what's happening; Namely, you
> make a request to get a token with token.Get and you then extract the token
> object with token.Extract.

I like this, but not for the same reason. One of the things that
always irked me was the discrepency between those who prefer GetFoo()
and just plain Foo(). Idiomatically, Go prefers Foo(), without any
special verb prefix. However, it's sometimes more convenient to use a
verb to distinguish a getter from a simple member field. I like the
use of Extract, because it's more explicit, while at the same time not
lying to the user about possible idempotency.

> Thoughts?

Overall, I like it; I'm just worried about possible explosion of steps
needed to just get something done. Authentication, arguably, should
be the simplest possible thing to get done.

Also, can this be easily wrapped up to support auto-reauth going forward?

--
Samuel A. Falvo II

Jon Perritt

unread,
May 8, 2014, 11:40:36 PM5/8/14
to gophercloud-dev
On Tue, May 6, 2014 at 2:19 PM, Samuel Falvo II <sam....@gmail.com> wrote:
Sorry for my tardiness.  Work, life, then more work.

No worries. I have plenty to keep me busy  :)
 

On Wed, Apr 23, 2014 at 11:10 AM, Jon Perritt <jrpe...@gmail.com> wrote:
> 1. I've broken out tokens, service catalogs, and endpoints into their own
> packages. This makes the Identity API more consistent with Compute and
> Object Storage.
> Usage would now look like the following:
>
> ao, err := utils.AuthOptions()
> gr, err := token.Get(ao)
> token, err = token.Extract(gr)
> sc, err := serviceCatalog.Extract(gr)
> sce, err = serviceCatalog.Entries(sc)

This is a lot of steps, and I'm afraid that someone new to Gophercloud
won't be able to divine the correct sequence very easily from reading
a godoc.  Would it be possible to add this to the utils package to
help automate the process for the common case?  Also, might be worth
putting code examples in the doc.go file of the Identity package.

Aha, I'm glad you asked! I have 2 ideas on this:

1) Have an "Auth" function in the utils package that takes no parameters (environment variables are used) and returns the values that get passed to a "NewClient" method (namely, endpoint, token response, and auth options). Then, the user simply passes these to the appropriate "NewClient" method to create a client. It would look something like below.

func Auth() (string, identity.AuthResults, identity.AuthOptions, error) {
        ao, err := utils.AuthOptions()
        if err != nil {
                return "", nil, nil, err
        }

        r, err := identity.Authenticate(ao)
        if err != nil {
                return "", nil, nil, err
        }

        sc, err := identity.GetServiceCatalog(r)
        if err != nil {
                return "", nil, nil, err
        }

        ces, err := sc.CatalogEntries()
        if err != nil {
                return "", nil, nil, err
        }

        var eps []identity.Endpoint
        for _, ce := range ces {
                if ce.Type == "volume" {
                        eps = ce.Endpoints
                }
        }

        region := os.Getenv("OS_REGION_NAME")
        rep := ""
        for _, ep := range eps {
                if ep.Region == region {
                        rep = ep.PublicURL
                }
        }

        return rep, r, ao, nil
}


2) Have a "GetClient" function in the utils package that accepts a string (of the service type). The function would be identical to (1) with the addition of a switch statement that creates a client based on the string parameter. Of course, this would also require having an interface that all the Clients implement, which is what would get returned from this function.

I agree. When an API is finalized, examples should be placed in a doc.go file for that service.
 

> 2. I've used common verbs to refer to HTTP requests. When getting data out
> of a data structure (such as a map[string]interface{}) I've used the verb
> 'Extract'.
> I think this is more intuitive to understand what's happening; Namely, you
> make a request to get a token with token.Get and you then extract the token
> object with token.Extract.

I like this, but not for the same reason.  One of the things that
always irked me was the discrepency between those who prefer GetFoo()
and just plain Foo().  Idiomatically, Go prefers Foo(), without any
special verb prefix.  However, it's sometimes more convenient to use a
verb to distinguish a getter from a simple member field.  I like the
use of Extract, because it's more explicit, while at the same time not
lying to the user about possible idempotency.

That too  :)
 

> Thoughts?

Overall, I like it; I'm just worried about possible explosion of steps
needed to just get something done.  

That's understandable. The problem arises because this version of the OpenStack Identity API returns many possibly-desirable pieces of information in the Token response. It's purely a convenience to users who want that possibly-desirable information but don't want to muck with the Token response.
 
Authentication, arguably, should
be the simplest possible thing to get done.


Completely agree.
 
Also, can this be easily wrapped up to support auto-reauth going forward?

Off the top of my head, I can think of 2 different ways to handle auto-reauth:

1) Check that the token's expire time is still fresh before every HTTP request.
2) Assume the token is fresh and reauth on the appropriate response code.

Either of those could be implemented with this API (or the current one), but I think that logic belongs in the HTTP request (or response) handler. There may be something I'm missing there though.
Reply all
Reply to author
Forward
0 new messages