From bea6e6ac23e893a85f4b0a49bab52934aca86726 Mon Sep 17 00:00:00 2001
From: Terry Howe <terrylhowe@gmail.com>
Date: Wed, 12 Mar 2014 11:51:17 -0600
Subject: [PATCH] Make endpoint commands more consistent

Make endpoints more consistent across create, show, etc
* Make the name option required for create
* Use a common function to fetch services by id, name or type
* Have show work by endpoint id or by service id, type or name
* Have show display all the fields by default
* Remove capability to filter queries by attribute value pairs

Change-Id: Idaa4b8d930ba859fd62de777e44a10b1ed58c79b
Partial-Bug: #1184012
---
 openstackclient/identity/common.py        | 38 +++++++++
 openstackclient/identity/v2_0/endpoint.py | 97 ++++++-----------------
 openstackclient/identity/v2_0/service.py  | 28 +------
 openstackclient/identity/v3/endpoint.py   | 13 ++-
 openstackclient/identity/v3/service.py    | 16 +---
 5 files changed, 74 insertions(+), 118 deletions(-)
 create mode 100644 openstackclient/identity/common.py

diff --git a/openstackclient/identity/common.py b/openstackclient/identity/common.py
new file mode 100644
index 0000000000..6aeaa3c387
--- /dev/null
+++ b/openstackclient/identity/common.py
@@ -0,0 +1,38 @@
+#   Copyright 2012-2013 OpenStack Foundation
+#
+#   Licensed under the Apache License, Version 2.0 (the "License"); you may
+#   not use this file except in compliance with the License. You may obtain
+#   a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#   Unless required by applicable law or agreed to in writing, software
+#   distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#   WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#   License for the specific language governing permissions and limitations
+#   under the License.
+#
+
+"""Common identity code"""
+
+from keystoneclient import exceptions as identity_exc
+from openstackclient.common import exceptions
+from openstackclient.common import utils
+
+
+def find_service(identity_client, name_type_or_id):
+    """Find a service by id, name or type."""
+
+    try:
+        # search for the usual ID or name
+        return utils.find_resource(identity_client.services, name_type_or_id)
+    except exceptions.CommandError:
+        try:
+            # search for service type
+            return identity_client.services.find(type=name_type_or_id)
+        # FIXME(dtroyer): This exception should eventually come from
+        #                 common client exceptions
+        except identity_exc.NotFound:
+            msg = ("No service with a type, name or ID of '%s' exists."
+                   % name_type_or_id)
+            raise exceptions.CommandError(msg)
diff --git a/openstackclient/identity/v2_0/endpoint.py b/openstackclient/identity/v2_0/endpoint.py
index 0319c2680a..4ee1636ff0 100644
--- a/openstackclient/identity/v2_0/endpoint.py
+++ b/openstackclient/identity/v2_0/endpoint.py
@@ -22,9 +22,8 @@ from cliff import command
 from cliff import lister
 from cliff import show
 
-from keystoneclient import exceptions as identity_exc
-from openstackclient.common import exceptions
 from openstackclient.common import utils
+from openstackclient.identity import common
 
 
 class CreateEndpoint(show.ShowOne):
@@ -45,6 +44,7 @@ class CreateEndpoint(show.ShowOne):
         parser.add_argument(
             '--publicurl',
             metavar='<public-url>',
+            required=True,
             help='New endpoint public URL')
         parser.add_argument(
             '--adminurl',
@@ -59,8 +59,7 @@ class CreateEndpoint(show.ShowOne):
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)' % parsed_args)
         identity_client = self.app.client_manager.identity
-        service = utils.find_resource(identity_client.services,
-                                      parsed_args.service)
+        service = common.find_service(identity_client, parsed_args.service)
         endpoint = identity_client.endpoints.create(
             parsed_args.region,
             service.id,
@@ -120,8 +119,7 @@ class ListEndpoint(lister.Lister):
         data = identity_client.endpoints.list()
 
         for ep in data:
-            service = utils.find_resource(
-                identity_client.services, ep.service_id)
+            service = common.find_service(identity_client, ep.service_id)
             ep.service_name = service.name
             ep.service_type = service.type
         return (columns,
@@ -139,77 +137,30 @@ class ShowEndpoint(show.ShowOne):
     def get_parser(self, prog_name):
         parser = super(ShowEndpoint, self).get_parser(prog_name)
         parser.add_argument(
-            'service',
-            metavar='<service>',
-            help='Name or ID of service endpoint to display')
-        parser.add_argument(
-            '--type',
-            metavar='<endpoint-type>',
-            default='publicURL',
-            help='Endpoint type: publicURL, internalURL, adminURL ' +
-                 '(default publicURL)')
-        parser.add_argument(
-            '--attr',
-            metavar='<endpoint-attribute>',
-            help='Endpoint attribute to use for selection')
-        parser.add_argument(
-            '--value',
-            metavar='<endpoint-value>',
-            help='Value of endpoint attribute to use for selection')
-        parser.add_argument(
-            '--all',
-            action='store_true',
-            default=False,
-            help='Show all endpoints for this service')
+            'endpoint_or_service',
+            metavar='<endpoint_or_service>',
+            help='Endpoint ID or name, type or ID of service to display')
         return parser
 
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)' % parsed_args)
         identity_client = self.app.client_manager.identity
-
-        if not parsed_args.all:
-            # Find endpoint filtered by a specific attribute or service type
-            kwargs = {
-                'service_type': parsed_args.service,
-                'endpoint_type': parsed_args.type,
-            }
-            if parsed_args.attr and parsed_args.value:
-                kwargs.update({
-                    'attr': parsed_args.attr,
-                    'filter_value': parsed_args.value,
-                })
-            elif parsed_args.attr or parsed_args.value:
-                msg = 'Both --attr and --value required'
-                raise exceptions.CommandError(msg)
-
-            url = identity_client.service_catalog.url_for(**kwargs)
-            info = {'%s.%s' % (parsed_args.service, parsed_args.type): url}
-            return zip(*sorted(six.iteritems(info)))
-        else:
-            # The Identity 2.0 API doesn't support retrieving a single
-            # endpoint so we have to do this ourselves
-            try:
-                service = utils.find_resource(identity_client.services,
-                                              parsed_args.service)
-            except exceptions.CommandError:
-                try:
-                    # search for service type
-                    service = identity_client.services.find(
-                        type=parsed_args.service)
-                # FIXME(dtroyer): This exception should eventually come from
-                #                 common client exceptions
-                except identity_exc.NotFound:
-                    msg = "No service with a type, name or ID of '%s' exists" \
-                        % parsed_args.service
-                    raise exceptions.CommandError(msg)
-
-            data = identity_client.endpoints.list()
+        data = identity_client.endpoints.list()
+        match = None
+        for ep in data:
+            if ep.id == parsed_args.endpoint_or_service:
+                match = ep
+                service = common.find_service(identity_client, ep.service_id)
+        if match is None:
+            service = common.find_service(identity_client,
+                                          parsed_args.endpoint_or_service)
             for ep in data:
                 if ep.service_id == service.id:
-                    info = {}
-                    info.update(ep._info)
-                    service = utils.find_resource(identity_client.services,
-                                                  ep.service_id)
-                    info['service_name'] = service.name
-                    info['service_type'] = service.type
-                    return zip(*sorted(six.iteritems(info)))
+                    match = ep
+        if match is None:
+            return None
+        info = {}
+        info.update(match._info)
+        info['service_name'] = service.name
+        info['service_type'] = service.type
+        return zip(*sorted(six.iteritems(info)))
diff --git a/openstackclient/identity/v2_0/service.py b/openstackclient/identity/v2_0/service.py
index ea45f63431..d61804c80e 100644
--- a/openstackclient/identity/v2_0/service.py
+++ b/openstackclient/identity/v2_0/service.py
@@ -22,9 +22,9 @@ from cliff import command
 from cliff import lister
 from cliff import show
 
-from keystoneclient import exceptions as identity_exc
 from openstackclient.common import exceptions
 from openstackclient.common import utils
+from openstackclient.identity import common
 
 
 class CreateService(show.ShowOne):
@@ -83,12 +83,7 @@ class DeleteService(command.Command):
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)' % parsed_args)
         identity_client = self.app.client_manager.identity
-
-        service = utils.find_resource(
-            identity_client.services,
-            parsed_args.service,
-        )
-
+        service = common.find_service(identity_client, parsed_args.service)
         identity_client.services.delete(service.id)
         return
 
@@ -159,24 +154,7 @@ class ShowService(show.ShowOne):
                    "exists." % (parsed_args.service))
             raise exceptions.CommandError(msg)
         else:
-            try:
-                # search for the usual ID or name
-                service = utils.find_resource(
-                    identity_client.services,
-                    parsed_args.service,
-                )
-            except exceptions.CommandError:
-                try:
-                    # search for service type
-                    service = identity_client.services.find(
-                        type=parsed_args.service)
-                # FIXME(dtroyer): This exception should eventually come from
-                #                 common client exceptions
-                except identity_exc.NotFound:
-                    msg = ("No service with a type, name or ID of '%s' exists."
-                           % parsed_args.service)
-                    raise exceptions.CommandError(msg)
-
+            service = common.find_service(identity_client, parsed_args.service)
             info = {}
             info.update(service._info)
             return zip(*sorted(six.iteritems(info)))
diff --git a/openstackclient/identity/v3/endpoint.py b/openstackclient/identity/v3/endpoint.py
index 055f9fe2ee..a51383ea54 100644
--- a/openstackclient/identity/v3/endpoint.py
+++ b/openstackclient/identity/v3/endpoint.py
@@ -24,6 +24,7 @@ from cliff import lister
 from cliff import show
 
 from openstackclient.common import utils
+from openstackclient.identity import common
 
 
 class CreateEndpoint(show.ShowOne):
@@ -69,8 +70,7 @@ class CreateEndpoint(show.ShowOne):
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)' % parsed_args)
         identity_client = self.app.client_manager.identity
-        service = utils.find_resource(identity_client.services,
-                                      parsed_args.service)
+        service = common.find_service(identity_client, parsed_args.service)
 
         endpoint = identity_client.endpoints.create(
             service.id,
@@ -122,8 +122,7 @@ class ListEndpoint(lister.Lister):
         data = identity_client.endpoints.list()
 
         for ep in data:
-            service = utils.find_resource(
-                identity_client.services, ep.service_id)
+            service = common.find_service(identity_client, ep.service_id)
             ep.service_name = service.name
             ep.service_type = service.type
         return (columns,
@@ -182,8 +181,7 @@ class SetEndpoint(command.Command):
         identity_client = self.app.client_manager.identity
         endpoint = utils.find_resource(identity_client.endpoints,
                                        parsed_args.endpoint)
-        service = utils.find_resource(identity_client.services,
-                                      parsed_args.service)
+        service = common.find_service(identity_client, parsed_args.service)
 
         if (not parsed_args.interface and not parsed_args.url
                 and not parsed_args.service and not parsed_args.region):
@@ -221,8 +219,7 @@ class ShowEndpoint(show.ShowOne):
         endpoint = utils.find_resource(identity_client.endpoints,
                                        parsed_args.endpoint)
 
-        service = utils.find_resource(identity_client.services,
-                                      endpoint.service_id)
+        service = common.find_service(identity_client, endpoint.service_id)
 
         info = {}
         info.update(endpoint._info)
diff --git a/openstackclient/identity/v3/service.py b/openstackclient/identity/v3/service.py
index 7e3bfc6b2d..8576a6e6f9 100644
--- a/openstackclient/identity/v3/service.py
+++ b/openstackclient/identity/v3/service.py
@@ -23,6 +23,7 @@ from cliff import lister
 from cliff import show
 
 from openstackclient.common import utils
+from openstackclient.identity import common
 
 
 class CreateService(show.ShowOne):
@@ -90,10 +91,7 @@ class DeleteService(command.Command):
         self.log.debug('take_action(%s)' % parsed_args)
         identity_client = self.app.client_manager.identity
 
-        service = utils.find_resource(
-            identity_client.services,
-            parsed_args.service,
-        )
+        service = common.find_service(identity_client, parsed_args.service)
 
         identity_client.services.delete(service.id)
         return
@@ -161,10 +159,7 @@ class SetService(command.Command):
                 and not parsed_args.disable):
             return
 
-        service = utils.find_resource(
-            identity_client.services,
-            parsed_args.service,
-        )
+        service = common.find_service(identity_client, parsed_args.service)
 
         kwargs = service._info
         if parsed_args.type:
@@ -203,9 +198,6 @@ class ShowService(show.ShowOne):
         self.log.debug('take_action(%s)' % parsed_args)
         identity_client = self.app.client_manager.identity
 
-        service = utils.find_resource(
-            identity_client.services,
-            parsed_args.service,
-        )
+        service = common.find_service(identity_client, parsed_args.service)
 
         return zip(*sorted(six.iteritems(service._info)))