Merge lp:~xtoddx/nova/disable_creds into lp:~hudson-openstack/nova/trunk

[Todd Willey]

In the case of compromised vpn credentials, we need to be able to terminate the affected vpn and revoke the credentials. This adds that functionality to the ec2 admin api.

[John Dewey (retr0h)]

I do not see tests for adminclient.py, but maybe now is a good time to add them?

I have a hard time approving a feature that lacks tests, even if those before us didn't add them.

review: Needs Fixing

[Thierry Carrez (ttx)]

We just hit feature freeze, if you want this into Cactus rather than wait for Diablo, could you provide a quick benefit vs. risk of regression rationale, then request a specific FFe review from me ?

[Thierry Carrez (ttx)]

review: Needs Information (ffe)

[termie (termie)]

diffline 33: please put StatusResponse on the next line, indented with the other args.

should we be internationalizing the response in the disable project creds call?

also looks like you'll need a newline before the StatusResponse class def.

Other than that, looking good. Other folks's comments still applicable, though I think tests will need to be in smoketests if you write them.

review: Needs Fixing

[lp:~xtoddx/nova/disable_creds]

847. By Todd Willey on 2011-03-29

merge trunk

848. By Todd Willey on 2011-03-29

Fix style issues and add i18n.

[Todd Willey (xtoddx)]

Merged trunk and fixed style.

Decided against i18n as clients may be different language than api server.

[lp:~xtoddx/nova/disable_creds]

849. By Todd Willey on 2011-03-29

Revert i18n on server-side.

[termie (termie)]

style-wise looks good, smoketests would be great

review: Approve

[Matt Dietz (cerberus)]

Looks good to me, though I agree that tests would be lovely.

[Christopher MacGown (0x44)]

Per the new testing policy decided at the Diablo summit, this needs tests before it can be merged.

review: Needs Fixing

[Christopher MacGown (0x44)]

> Per the new testing policy decided at the Diablo summit, this needs tests
> before it can be merged.

Otherwise, it looks good.

Unmerged revisions

849. By Todd Willey on 2011-03-29

Revert i18n on server-side.

848. By Todd Willey on 2011-03-29

Fix style issues and add i18n.

847. By Todd Willey on 2011-03-29

merge trunk

846. By Todd Willey on 2011-03-23

Admin api methods for disabling credentials & terminating vpns.

=== modified file 'nova/adminclient.py'
--- nova/adminclient.py	2011-03-16 04:04:38 +0000
+++ nova/adminclient.py	2011-03-30 00:01:11 +0000
@@ -265,6 +265,21 @@
             setattr(self, name, str(value))
 
 
+class StatusResponse(object):
+    def __init__(self, connection=None):
+        self.connection = connection
+        self.status = None
+
+    def __repr__(self):
+        return 'Status:%s' % self.status
+
+    def startElement(self, name, attrs, connection):
+        return None
+
+    def endElement(self, name, value, connection):
+        setattr(self, name, str(value))
+
+
 class NovaAdminClient(object):
 
     def __init__(
@@ -471,3 +486,9 @@
         """Grabs the list of all users."""
         return self.apiconn.get_list('DescribeInstanceTypes', {},
                                      [('item', InstanceType)])
+
+    def disable_project_credentials(self, project):
+        """Revoke project credentials and kill the cloudpipe/vpn instance."""
+        return self.apiconn.get_object('DisableProjectCredentials',
+                                       {'Project': project},
+                                       StatusResponse)

=== modified file 'nova/api/ec2/admin.py'
--- nova/api/ec2/admin.py	2011-03-24 18:02:04 +0000
+++ nova/api/ec2/admin.py	2011-03-30 00:01:11 +0000
@@ -23,6 +23,7 @@
 import base64
 import datetime
 
+from nova import crypto
 from nova import db
 from nova import exception
 from nova import flags
@@ -329,3 +330,8 @@
     def describe_host(self, _context, name, **_kwargs):
         """Returns status info for single node."""
         return host_dict(db.host_get(name))
+
+    def disable_project_credentials(self, context, project):
+        """Revoke credentials and stop the vpn instance."""
+        crypto.revoke_certs_by_project(project)
+        return {'status': 'OK', 'message': 'Credentials Revoked'}