Merge lp:~dan-prince/nova/bug723897 into lp:~hudson-openstack/nova/trunk

[Dan Prince]

Revert commit 709. This fixes issues with the Openstack API causing 'No user for access key admin' errors.

[justinsb (justin-fathomdb)]

As I posed on the bug report, I think this is just that the credentials in novarc are wrong. I'm working on a bugfix right now, should just be a few minutes.

[Rick Harris (rconradharris)]

Since r709 is a serious blocker at the moment, I'm voting to revert this immediately so that we can have a good back-and-forth debate on The Right Way without holding up continuing development against trunk.

review: Approve

[justinsb (justin-fathomdb)]

The OpenStack API and the EC2 API should have the same set of credentials. Nothing else makes sense from a user point of view: users should not need to know which API an app is written to in order to pick the correct set of credentials to use. That's what r709 does.

There is a very easy workaround (just use the EC2 credentials, removing the project name from the EC2 username) When my novarc patch lands, it becomes trivial (use the specified credentials). The OpenStack API is still unstable, so it's okay to break things now. Reverting r709 just postpones the pain of fixing OpenStack authentication.

To interested parties: there's lots more discussion on the linked bug :-)

review: Disapprove

[Vish Ishaya (vishvananda)]

reverting looks good for the moment, since that seems to be the option favored by the guys working most frequently with the os api.

review: Approve

[Ed Leafe (ed-leafe)]

Fix trunk first; fix code logic second. Makes sense to me.

review: Approve

[justinsb (justin-fathomdb)]

Trunk is not broken. The credentials that you have to use changed in r709, for the better.

I haven't seen anyone explain why this is a "serious blocker"... No offense to both people using the OpenStack API ;-)

[Paul Voccio (pvo)]

I agree with reverting. if we're going to change auth, we need to discuss this on the ML or have a BP before making the change. Don't disagree that it can't be changed but at the moment, this breaks quite a few environments that are depending on this to work as is.

review: Approve

[justinsb (justin-fathomdb)]

Which environments is it breaking? Why can't they apply the 60 second workaround?

=== modified file 'nova/api/openstack/auth.py'
--- nova/api/openstack/auth.py	2011-02-22 09:59:53 +0000
+++ nova/api/openstack/auth.py	2011-02-23 20:04:29 +0000
@@ -120,8 +120,8 @@
         req - webob.Request object
         """
         ctxt = context.get_admin_context()
-        user = self.auth.get_user_from_access_key(username)
-        if user and user.secret == key:
+        user = self.auth.get_user_from_access_key(key)
+        if user and user.name == username:
             token_hash = hashlib.sha1('%s%s%f' % (username, key,
                 time.time())).hexdigest()
             token_dict = {}

=== modified file 'nova/tests/api/openstack/fakes.py'
--- nova/tests/api/openstack/fakes.py	2011-02-19 07:33:06 +0000
+++ nova/tests/api/openstack/fakes.py	2011-02-23 20:04:29 +0000
@@ -221,8 +221,7 @@
 class FakeAuthManager(object):
     auth_data = {}
 
-    def add_user(self, user):
-        key = user.id
+    def add_user(self, key, user):
         FakeAuthManager.auth_data[key] = user
 
     def get_user(self, uid):
@@ -235,10 +234,7 @@
         return None
 
     def get_user_from_access_key(self, key):
-        for k, v in FakeAuthManager.auth_data.iteritems():
-            if v.access == key:
-                return v
-        return None
+        return FakeAuthManager.auth_data.get(key, None)
 
 
 class FakeRateLimiter(object):

=== modified file 'nova/tests/api/openstack/test_auth.py'
--- nova/tests/api/openstack/test_auth.py	2011-02-19 07:33:06 +0000
+++ nova/tests/api/openstack/test_auth.py	2011-02-23 20:04:29 +0000
@@ -48,7 +48,7 @@
 
     def test_authorize_user(self):
         f = fakes.FakeAuthManager()
-        f.add_user(nova.auth.manager.User(1, 'herp', 'herp', 'derp', None))
+        f.add_user('derp', nova.auth.manager.User(1, 'herp', None, None, None))
 
         req = webob.Request.blank('/v1.0/')
         req.headers['X-Auth-User'] = 'herp'
@@ -62,7 +62,7 @@
 
     def test_authorize_token(self):
         f = fakes.FakeAuthManager()
-        f.add_user(nova.auth.manager.User(1, 'herp', 'herp', 'derp', None))
+        f.add_user('derp', nova.auth.manager.User(1, 'herp', None, None, None))
 
         req = webob.Request.blank('/v1.0/', {'HTTP_HOST': 'foo'})
         req.headers['X-Auth-User'] = 'herp'
@@ -144,7 +144,7 @@
 
     def test_authorize_token(self):
         f = fakes.FakeAuthManager()
-        f.add_user(nova.auth.manager.User(1, 'herp', 'herp', 'derp', None))
+        f.add_user('derp', nova.auth.manager.User(1, 'herp', None, None, None))
 
         req = webob.Request.blank('/v1.0/')
         req.headers['X-Auth-User'] = 'herp'