Merge pull request #582 from suonto/fix-identity-base

Fix openstack NewClient to not remove relative paths from IdentityBase

Author: jrperritt

openstack/client.go

@@ -3,6 +3,7 @@ package openstack
 import (
 	"fmt"
 	"net/url"
+	"strconv"
 	"strings"
 
 	"github.com/rackspace/gophercloud"
@@ -25,18 +26,40 @@ func NewClient(endpoint string) (*gophercloud.ProviderClient, error) {
 	if err != nil {
 		return nil, err
 	}
-	hadPath := u.Path != ""
-	u.Path, u.RawQuery, u.Fragment = "", "", ""
-	base := u.String()
 
-	endpoint = gophercloud.NormalizeURL(endpoint)
-	base = gophercloud.NormalizeURL(base)
+	u.RawQuery, u.Fragment = "", ""
 
-	if hadPath {
-		return &gophercloud.ProviderClient{
-			IdentityBase:     base,
-			IdentityEndpoint: endpoint,
-		}, nil
+	// Base is url with path
+	endpoint = gophercloud.NormalizeURL(endpoint)
+	base := gophercloud.NormalizeURL(u.String())
+
+	path := u.Path
+	if !strings.HasSuffix(path, "/") {
+		path = path + "/"
+	}
+
+	parts := strings.Split(path[0:len(path)-1], "/")
+	for index,version := range(parts) {
+		if 2 <= len(version) && len(version) <= 4 && strings.HasPrefix(version, "v") {
+			_, err := strconv.ParseFloat(version[1:], 64)
+			if err == nil {
+				// post version suffixes in path are not supported
+				// version must be on the last index
+				if index < len(parts) - 1 {
+					return nil, fmt.Errorf("Path suffixes (after version) are not supported.")
+				}
+				switch version {
+				case "v2.0", "v3":
+					// valid version found, strip from base
+					return &gophercloud.ProviderClient{
+						IdentityBase:     base[0:len(base)-len(version)-1],
+						IdentityEndpoint: endpoint,
+					}, nil
+				default:
+					return nil, fmt.Errorf("Invalid identity endpoint version %v. Supported versions: v2.0, v3", version)
+				}
+			}
+		}
 	}
 
 	return &gophercloud.ProviderClient{

openstack/client_test.go

@@ -159,3 +159,95 @@ func TestAuthenticatedClientV2(t *testing.T) {
 	th.AssertNoErr(t, err)
 	th.CheckEquals(t, "01234567890", client.TokenID)
 }
+
+func TestNewClient(t *testing.T) {
+	client, err := NewClient("https://example.com")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/v2.0")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://example.com/v2.0/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/v2.0/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://example.com/v2.0/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/foo/bar")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/foo/bar/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/foo/bar/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/foo/bar/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/foo/bar/v2.0")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://example.com/foo/bar/v2.0/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/foo/bar/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/foo/bar/v2.0/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://example.com/foo/bar/v2.0/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/foo/bar/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/foo/bar/v3/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://example.com/foo/bar/v3/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/foo/bar/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/v3")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://example.com/v3/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/", client.IdentityBase)
+
+	client, err = NewClient("https://example.com/v3/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://example.com/v3/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/", client.IdentityBase)
+
+	_, err = NewClient("https://example.com/v2.3/")
+	th.AssertErr(t, err)
+	expected := fmt.Errorf("Invalid identity endpoint version %v. Supported versions: v2.0, v3", "v2.3")
+	th.AssertEquals(t, expected.Error(), err.Error())
+
+	_, err = NewClient("https://example.com/v2.0/foo")
+	th.AssertErr(t, err)
+	expected = fmt.Errorf("Path suffixes (after version) are not supported.")
+	th.AssertEquals(t, expected.Error(), err.Error())
+
+	// Does not match regexp, include to base
+	client, err = NewClient("https://example.com/v2a0/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/v2a0/", client.IdentityBase)
+
+	// Does not match regexp, include to base
+	client, err = NewClient("https://example.com/v3api")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://example.com/v3api/", client.IdentityBase)
+
+	// Matches regexp, invalid version v20.
+	_, err = NewClient("https://example.com/v20./")
+	th.AssertErr(t, err)
+	expected = fmt.Errorf("Invalid identity endpoint version %v. Supported versions: v2.0, v3", "v20.")
+	th.AssertEquals(t, expected.Error(), err.Error())
+
+	// domain contains version
+	client, err = NewClient("https://v3.v2.0example.com/v3/")
+	th.AssertNoErr(t, err)
+	th.AssertEquals(t, "https://v3.v2.0example.com/v3/", client.IdentityEndpoint)
+	th.AssertEquals(t, "https://v3.v2.0example.com/", client.IdentityBase)
+
+}

testhelper/convenience.go

@@ -224,6 +224,21 @@ func CheckEquals(t *testing.T, expected, actual interface{}) {
 	}
 }
 
+// AssertErr is a convenience function for checking that an error occurred
+func AssertErr(t *testing.T, e error) {
+	if e == nil {
+		logFatal(t, fmt.Sprintf("expected an error but none occurred"))
+	}
+}
+
+// CheckErr is a convenience function for checking that an error occurred,
+// except with a non-fatal error
+func CheckErr(t *testing.T, e error) {
+	if e == nil {
+		logError(t, fmt.Sprintf("expected an error but none occurred"))
+	}
+}
+
 // AssertDeepEquals - like Equals - performs a comparison - but on more complex
 // structures that requires deeper inspection
 func AssertDeepEquals(t *testing.T, expected, actual interface{}) {