Mark arguments for 'credential' commands as required

According to the [1], 'user_id', 'type', and 'blob' are all required
arguments for 'credential set' but the code treats them as optional. Set the
'required' flag and remove logic supporting missing arguments.

[1]: https://github.com/openstack/keystone-specs/blob/master/api/v3/identity-api-v3.rst#credentials-v3credentials "spec"

Change-Id: I597c9616ad744385fc6dd92379feb03daec54458
Closes-Bug: #1418837

Author: Sean Perry

openstackclient/identity/v3/credential.py

@@ -17,7 +17,6 @@
 
 import logging
 import six
-import sys
 
 from cliff import command
 from cliff import lister
@@ -130,17 +129,20 @@ def get_parser(self, prog_name):
         parser.add_argument(
             '--user',
             metavar='<user>',
+            required=True,
             help='Name or ID of user that owns the credential',
         )
         parser.add_argument(
             '--type',
             metavar='<type>',
             choices=['ec2', 'cert'],
+            required=True,
             help='New credential type',
         )
         parser.add_argument(
             '--data',
             metavar='<data>',
+            required=True,
             help='New credential data',
         )
         parser.add_argument(
@@ -153,25 +155,22 @@ def get_parser(self, prog_name):
     @utils.log_method(log)
     def take_action(self, parsed_args):
         identity_client = self.app.client_manager.identity
-        kwargs = {}
-        if parsed_args.user:
-            user_id = utils.find_resource(identity_client.users,
-                                          parsed_args.user).id
-            if user_id:
-                kwargs['user'] = user_id
-        if parsed_args.type:
-            kwargs['type'] = parsed_args.type
-        if parsed_args.data:
-            kwargs['data'] = parsed_args.data
+
+        user_id = utils.find_resource(identity_client.users,
+                                      parsed_args.user).id
+
         if parsed_args.project:
             project = utils.find_resource(identity_client.projects,
                                           parsed_args.project).id
-            kwargs['project'] = project
+        else:
+            project = None
+
+        identity_client.credentials.update(parsed_args.credential,
+                                           user=user_id,
+                                           type=parsed_args.type,
+                                           blob=parsed_args.data,
+                                           project=project)
 
-        if not kwargs:
-            sys.stdout.write("Credential not updated, no arguments present")
-            return
-        identity_client.credentials.update(parsed_args.credential, **kwargs)
         return
 
 

openstackclient/tests/identity/v3/fakes.py

@@ -195,6 +195,8 @@
     'links': base_url + 'services/' + service_id,
 }
 
+credential_id = 'c-123'
+
 endpoint_id = 'e-123'
 endpoint_url = 'http://127.0.0.1:35357'
 endpoint_region = 'RegionOne'
@@ -400,6 +402,8 @@ class FakeIdentityv3Client(object):
     def __init__(self, **kwargs):
         self.domains = mock.Mock()
         self.domains.resource_class = fakes.FakeResource(None, {})
+        self.credentials = mock.Mock()
+        self.credentials.resource_class = fakes.FakeResource(None, {})
         self.endpoints = mock.Mock()
         self.endpoints.resource_class = fakes.FakeResource(None, {})
         self.groups = mock.Mock()

openstackclient/tests/identity/v3/test_credential.py

@@ -0,0 +1,112 @@
+#   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.
+
+import json
+
+from openstackclient.identity.v3 import credential
+from openstackclient.tests.identity.v3 import fakes as identity_fakes
+from openstackclient.tests.utils import ParserException
+
+
+class TestCredential(identity_fakes.TestIdentityv3):
+    data = {
+        "access": "abc123",
+        "secret": "hidden-message",
+        "trust_id": None
+    }
+
+    def __init__(self, *args):
+        super(TestCredential, self).__init__(*args)
+
+        self.json_data = json.dumps(self.data)
+
+    def setUp(self):
+        super(TestCredential, self).setUp()
+
+        # Get a shortcut to the CredentialManager Mock
+        self.credentials_mock = self.app.client_manager.identity.credentials
+        self.credentials_mock.reset_mock()
+
+        # Get a shortcut to the UserManager Mock
+        self.users_mock = self.app.client_manager.identity.users
+        self.users_mock.reset_mock()
+
+        # Get a shortcut to the ProjectManager Mock
+        self.projects_mock = self.app.client_manager.identity.projects
+        self.projects_mock.reset_mock()
+
+
+class TestCredentialSet(TestCredential):
+    def setUp(self):
+        super(TestCredentialSet, self).setUp()
+        self.cmd = credential.SetCredential(self.app, None)
+
+    def test_credential_set_no_options(self):
+        arglist = [
+            identity_fakes.credential_id,
+        ]
+
+        self.assertRaises(ParserException,
+                          self.check_parser, self.cmd, arglist, [])
+
+    def test_credential_set_missing_user(self):
+        arglist = [
+            '--type', 'ec2',
+            '--data', self.json_data,
+            identity_fakes.credential_id,
+        ]
+
+        self.assertRaises(ParserException,
+                          self.check_parser, self.cmd, arglist, [])
+
+    def test_credential_set_missing_type(self):
+        arglist = [
+            '--user', identity_fakes.user_name,
+            '--data', self.json_data,
+            identity_fakes.credential_id,
+        ]
+
+        self.assertRaises(ParserException,
+                          self.check_parser, self.cmd, arglist, [])
+
+    def test_credential_set_missing_data(self):
+        arglist = [
+            '--user', identity_fakes.user_name,
+            '--type', 'ec2',
+            identity_fakes.credential_id,
+        ]
+
+        self.assertRaises(ParserException,
+                          self.check_parser, self.cmd, arglist, [])
+
+    def test_credential_set_valid(self):
+        arglist = [
+            '--user', identity_fakes.user_name,
+            '--type', 'ec2',
+            '--data', self.json_data,
+            identity_fakes.credential_id,
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, [])
+        self.cmd.take_action(parsed_args)
+
+    def test_credential_set_valid_with_project(self):
+        arglist = [
+            '--user', identity_fakes.user_name,
+            '--type', 'ec2',
+            '--data', self.json_data,
+            '--project', identity_fakes.project_name,
+            identity_fakes.credential_id,
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, [])
+        self.cmd.take_action(parsed_args)