Merge lp:~jameinel/goose/handle-missing-endpoint into lp:goose

[John A Meinel]

identity: Handle when a service is available, but only in a region we aren't currently using.

Add -live tests for the identity suite. Refactor a bit and test the parsing logic to ensure we don't get panic() when talking to canonistack.

With the recent changes to canonistack, we now have a service that is only exposed on lcy02. (fakecinder, I believe).

This was causing failures in the nova test suite (it panic'd because of accessing the 0th item of an empty array).

While doing this, I realized we didn't have any live tests focusing on the identity level, so I added some. This also was easier to test by refactoring it out into smaller helper functions, rather than having everything done inside the request processing.

Because the UserPass object doesn't have much state (none that I needed), I just made them free functions, but I would consider having them as struct methods if people like that more.

=== modified file 'identity/live_test.go'
--- identity/live_test.go	2013-01-27 13:37:53 +0000
+++ identity/live_test.go	2013-01-29 04:11:22 +0000
@@ -34,7 +34,7 @@
 
 func (s *LiveTests) TestAuth(c *C) {
 	s.client.Authenticate()
-        url, err := s.client.MakeServiceURL("compute", []string{})
-        c.Assert(err, IsNil)
-        c.Assert(url[:len(s.cred.URL)], Equals, s.cred.URL)
+	url, err := s.client.MakeServiceURL("compute", []string{})
+	c.Assert(err, IsNil)
+	c.Assert(url, Not(Equals), "")
 }

=== modified file 'identity/userpass.go'
--- identity/userpass.go	2013-01-27 13:37:53 +0000
+++ identity/userpass.go	2013-01-29 04:11:22 +0000
@@ -4,7 +4,6 @@
 	"fmt"
 	goosehttp "launchpad.net/goose/http"
 	"net/http"
-	"os"
 )
 
 type passwordCredentials struct {
@@ -71,6 +70,43 @@
 	client *goosehttp.Client
 }
 
+func findServiceEndpoint(service serviceResponse, region string) string {
+	possibleURLs := make([]string, 0, len(service.Endpoints))
+	for _, e := range service.Endpoints {
+		if e.Region == region {
+			possibleURLs = append(possibleURLs, e.PublicURL)
+		}
+	}
+	if len(possibleURLs) == 0 {
+		// TODO: Add a log entry:
+		// fmt.Fprintf(os.Stderr, "Found a service Type: %v but no endpoints in region: %v\n",
+		// service.Type, region)
+		// fmt.Fprintf(os.Stderr, "Found no endpoints for %v\n", service.Type)
+		return ""
+	}
+	return possibleURLs[0]
+}
+
+func parseAuthResponse(access *accessResponse, region string) (*AuthDetails, error) {
+	details := &AuthDetails{}
+	respToken := access.Token
+	if respToken.Id == "" {
+		return nil, fmt.Errorf("Did not get valid Token from auth request")
+	}
+	details.Token = respToken.Id
+	details.TenantId = respToken.Tenant.Id
+	details.UserId = access.User.Id
+	details.ServiceURLs = make(map[string]string, len(access.ServiceCatalog))
+	for _, service := range access.ServiceCatalog {
+		url := findServiceEndpoint(service, region)
+		if url == "" {
+			continue
+		}
+		details.ServiceURLs[service.Type] = url
+	}
+	return details, nil
+}
+
 func (u *UserPass) Auth(creds *Credentials) (*AuthDetails, error) {
 	if u.client == nil {
 		u.client = goosehttp.New(http.Client{CheckRedirect: nil}, nil, "")
@@ -88,28 +124,9 @@
 	if err != nil {
 		return nil, err
 	}
-
-	details := &AuthDetails{}
-	access := accessWrapper.Access
-	respToken := access.Token
-	if respToken.Id == "" {
-		return nil, fmt.Errorf("Did not get valid Token from auth request")
-	}
-	details.Token = respToken.Id
-	details.TenantId = respToken.Tenant.Id
-	details.UserId = access.User.Id
-	details.ServiceURLs = make(map[string]string, len(access.ServiceCatalog))
-	for _, service := range access.ServiceCatalog {
-		for i, e := range service.Endpoints {
-			if e.Region != creds.Region {
-				service.Endpoints = append(service.Endpoints[:i], service.Endpoints[i+1:]...)
-			}
-		}
-		if len(service.Endpoints) == 0 {
-			fmt.Fprintf(os.Stderr, "Found no endpoints for %v\n", service.Type)
-		}
-		details.ServiceURLs[service.Type] = service.Endpoints[0].PublicURL
-	}
-
+	details, err := parseAuthResponse(&accessWrapper.Access, creds.Region)
+	if err != nil {
+		return nil, err
+	}
 	return details, nil
 }

=== modified file 'identity/userpass_test.go'
--- identity/userpass_test.go	2013-01-24 01:01:35 +0000
+++ identity/userpass_test.go	2013-01-29 04:11:22 +0000
@@ -23,3 +23,42 @@
 	c.Assert(auth.Token, Equals, userInfo.Token)
 	c.Assert(auth.TenantId, Equals, userInfo.TenantId)
 }
+
+func (s *UserPassTestSuite) TestParseResponseWithoutEndpoint(c *C) {
+	// It is possible to get an endpoint which is available in some regions,
+	// but not in the region we are currently working in.
+	myRegion := "region01"
+	otherRegion := "region02"
+	otherEndpoint := endpoint{PublicURL: "http://example.invalid", Region: otherRegion}
+	service := serviceResponse{Name: "nova", Type: "compute",
+		Endpoints: []endpoint{otherEndpoint}}
+	response := &accessResponse{
+		ServiceCatalog: []serviceResponse{service},
+		Token:          tokenResponse{Id: "token"},
+		User:           userResponse{},
+	}
+	authDetails, err := parseAuthResponse(response, myRegion)
+	c.Assert(err, IsNil)
+	c.Assert(authDetails, NotNil)
+	// There should be no entries in the ServiceURLs map, because there is
+	// no URL for the service in this region
+	c.Assert(authDetails.ServiceURLs, DeepEquals, map[string]string{})
+}
+
+func (s *UserPassTestSuite) TestFindServiceEndpointWrongRegion(c *C) {
+	myRegion := "region01"
+	otherRegion := "region02"
+	otherEndpoint := endpoint{PublicURL: "http://example.invalid", Region: otherRegion}
+	service := serviceResponse{Name: "nova", Type: "compute",
+		Endpoints: []endpoint{otherEndpoint}}
+	c.Assert(findServiceEndpoint(service, myRegion), Equals, "")
+}
+
+func (s *UserPassTestSuite) TestFindServiceMultipleEndpoints(c *C) {
+	myRegion := "region01"
+	endpoint1 := endpoint{PublicURL: "http://example.invalid", Region: myRegion}
+	endpoint2 := endpoint{PublicURL: "http://other.invalid", Region: myRegion}
+	service := serviceResponse{Name: "nova", Type: "compute",
+		Endpoints: []endpoint{endpoint1, endpoint2}}
+	c.Assert(findServiceEndpoint(service, myRegion), Equals, endpoint1.PublicURL)
+}