openstack/python-openstackclient
Code Issues Proposed changes

identity: Remove unnecessary helper

Browse Source 
Make better use of argparse and eliminate the need for a helper in the
process.

Change-Id: Ibdc9b4bfbb4d532ddb05bce9b49bcf0580cce76d
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane
2025-05-26 16:02:21 +01:00
parent 88b59d8975
commit 2f03c3ea3c
7 changed files with 79 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
openstackclient/identity/common.py
View File

@@ -348,15 +348,6 @@ def _find_sdk_id(
return resource.id return resource.id
def get_immutable_options(parsed_args):
options = {}
if parsed_args.immutable:
options['immutable'] = True
if parsed_args.no_immutable:
options['immutable'] = False
return options
def add_user_domain_option_to_parser(parser): def add_user_domain_option_to_parser(parser):
parser.add_argument( parser.add_argument(
'--user-domain', '--user-domain',
@@ -419,17 +410,21 @@ def add_inherited_option_to_parser(parser):
def add_resource_option_to_parser(parser): def add_resource_option_to_parser(parser):
enable_group = parser.add_mutually_exclusive_group() immutable_group = parser.add_mutually_exclusive_group()
enable_group.add_argument( immutable_group.add_argument(
'--immutable', '--immutable',
action='store_true', action='store_true',
dest='immutable',
default=None,
help=_( help=_(
'Make resource immutable. An immutable project may not ' 'Make resource immutable. An immutable project may not '
'be deleted or modified except to remove the immutable flag' 'be deleted or modified except to remove the immutable flag'
), ),
) )
enable_group.add_argument( immutable_group.add_argument(
'--no-immutable', '--no-immutable',
action='store_true', action='store_false',
dest='immutable',
default=None,
help=_('Make resource mutable (default)'), help=_('Make resource mutable (default)'),
) )

11
openstackclient/identity/v3/domain.py
View File

@@ -94,7 +94,9 @@ class CreateDomain(command.ShowOne):
def take_action(self, parsed_args): def take_action(self, parsed_args):
identity_client = self.app.client_manager.sdk_connection.identity identity_client = self.app.client_manager.sdk_connection.identity
options = common.get_immutable_options(parsed_args) options = {}
if parsed_args.immutable is not None:
options['immutable'] = parsed_args.immutable
try: try:
domain = identity_client.create_domain( domain = identity_client.create_domain(
@@ -242,13 +244,10 @@ class SetDomain(command.Command):
kwargs['name'] = parsed_args.name kwargs['name'] = parsed_args.name
if parsed_args.description: if parsed_args.description:
kwargs['description'] = parsed_args.description kwargs['description'] = parsed_args.description
if parsed_args.is_enabled is not None: if parsed_args.is_enabled is not None:
kwargs['is_enabled'] = parsed_args.is_enabled kwargs['is_enabled'] = parsed_args.is_enabled
if parsed_args.immutable is not None:
options = common.get_immutable_options(parsed_args) kwargs['options'] = {'immutable': parsed_args.immutable}
if options:
kwargs['options'] = options
identity_client.update_domain(domain.id, **kwargs) identity_client.update_domain(domain.id, **kwargs)

45
openstackclient/identity/v3/project.py
View File

@@ -59,17 +59,22 @@ class CreateProject(command.ShowOne):
enable_group.add_argument( enable_group.add_argument(
'--enable', '--enable',
action='store_true', action='store_true',
dest='enabled',
default=True,
help=_('Enable project'), help=_('Enable project'),
) )
enable_group.add_argument( enable_group.add_argument(
'--disable', '--disable',
action='store_true', action='store_false',
dest='enabled',
default=True,
help=_('Disable project'), help=_('Disable project'),
) )
parser.add_argument( parser.add_argument(
'--property', '--property',
metavar='<key=value>', metavar='<key=value>',
action=parseractions.KeyValueAction, action=parseractions.KeyValueAction,
dest='properties',
help=_( help=_(
'Add a property to <name> ' 'Add a property to <name> '
'(repeat option to set multiple properties)' '(repeat option to set multiple properties)'
@@ -98,15 +103,9 @@ class CreateProject(command.ShowOne):
parsed_args.parent, parsed_args.parent,
).id ).id
enabled = True
if parsed_args.disable:
enabled = False
options = common.get_immutable_options(parsed_args)
kwargs = {} kwargs = {}
if parsed_args.property: if parsed_args.properties:
kwargs = parsed_args.property.copy() kwargs = parsed_args.properties.copy()
if 'is_domain' in kwargs.keys(): if 'is_domain' in kwargs.keys():
if kwargs['is_domain'].lower() == "true": if kwargs['is_domain'].lower() == "true":
kwargs['is_domain'] = True kwargs['is_domain'] = True
@@ -117,13 +116,17 @@ class CreateProject(command.ShowOne):
kwargs['tags'] = list(set(parsed_args.tags)) kwargs['tags'] = list(set(parsed_args.tags))
options = {}
if parsed_args.immutable is not None:
options['immutable'] = parsed_args.immutable
try: try:
project = identity_client.projects.create( project = identity_client.projects.create(
name=parsed_args.name, name=parsed_args.name,
domain=domain, domain=domain,
parent=parent, parent=parent,
description=parsed_args.description, description=parsed_args.description,
enabled=enabled, enabled=parsed_args.enabled,
options=options, options=options,
**kwargs, **kwargs,
) )
@@ -356,16 +359,21 @@ class SetProject(command.Command):
enable_group.add_argument( enable_group.add_argument(
'--enable', '--enable',
action='store_true', action='store_true',
dest='enabled',
default=None,
help=_('Enable project'), help=_('Enable project'),
) )
enable_group.add_argument( enable_group.add_argument(
'--disable', '--disable',
action='store_true', action='store_false',
dest='enabled',
default=None,
help=_('Disable project'), help=_('Disable project'),
) )
parser.add_argument( parser.add_argument(
'--property', '--property',
metavar='<key=value>', metavar='<key=value>',
dest='properties',
action=parseractions.KeyValueAction, action=parseractions.KeyValueAction,
help=_( help=_(
'Set a property on <project> ' 'Set a property on <project> '
@@ -388,15 +396,12 @@ class SetProject(command.Command):
kwargs['name'] = parsed_args.name kwargs['name'] = parsed_args.name
if parsed_args.description: if parsed_args.description:
kwargs['description'] = parsed_args.description kwargs['description'] = parsed_args.description
if parsed_args.enable: if parsed_args.enabled is not None:
kwargs['enabled'] = True kwargs['enabled'] = parsed_args.enabled
if parsed_args.disable: if parsed_args.immutable is not None:
kwargs['enabled'] = False kwargs['options'] = {'immutable': parsed_args.immutable}
options = common.get_immutable_options(parsed_args) if parsed_args.properties:
if options: kwargs.update(parsed_args.properties)
kwargs['options'] = options
if parsed_args.property:
kwargs.update(parsed_args.property)
tag.update_tags_in_args(parsed_args, project, kwargs) tag.update_tags_in_args(parsed_args, project, kwargs)
identity_client.projects.update(project.id, **kwargs) identity_client.projects.update(project.id, **kwargs)

9
openstackclient/identity/v3/role.py
View File

@@ -334,9 +334,12 @@ class CreateRole(command.ShowOne):
if parsed_args.name: if parsed_args.name:
create_kwargs['name'] = parsed_args.name create_kwargs['name'] = parsed_args.name
if parsed_args.description: if parsed_args.description:
create_kwargs['description'] = parsed_args.description create_kwargs['description'] = parsed_args.description
create_kwargs['options'] = common.get_immutable_options(parsed_args)
if parsed_args.immutable is not None:
create_kwargs['options'] = {"immutable": parsed_args.immutable}
try: try:
role = identity_client.create_role(**create_kwargs) role = identity_client.create_role(**create_kwargs)
@@ -585,7 +588,9 @@ class SetRole(command.Command):
) )
update_kwargs["domain_id"] = domain_id update_kwargs["domain_id"] = domain_id
update_kwargs["options"] = common.get_immutable_options(parsed_args) if parsed_args.immutable is not None:
update_kwargs["options"] = {"immutable": parsed_args.immutable}
role = _find_sdk_id( role = _find_sdk_id(
identity_client.find_role, identity_client.find_role,
name_or_id=parsed_args.role, name_or_id=parsed_args.role,

4
openstackclient/tests/unit/identity/v3/test_domain.py
View File

@@ -186,7 +186,7 @@ class TestDomainCreate(identity_fakes.TestIdentityv3):
self.domain.name, self.domain.name,
] ]
verifylist = [ verifylist = [
('no_immutable', True), ('immutable', False),
('name', self.domain.name), ('name', self.domain.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -461,7 +461,7 @@ class TestDomainSet(identity_fakes.TestIdentityv3):
self.domain.id, self.domain.id,
] ]
verifylist = [ verifylist = [
('no_immutable', True), ('immutable', False),
('domain', self.domain.id), ('domain', self.domain.id),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)

83
openstackclient/tests/unit/identity/v3/test_project.py
View File

@@ -77,8 +77,7 @@ class TestProjectCreate(TestProject):
] ]
verifylist = [ verifylist = [
('parent', None), ('parent', None),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('tags', []), ('tags', []),
] ]
@@ -134,8 +133,7 @@ class TestProjectCreate(TestProject):
] ]
verifylist = [ verifylist = [
('description', 'new desc'), ('description', 'new desc'),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('parent', None), ('parent', None),
('tags', []), ('tags', []),
@@ -172,8 +170,7 @@ class TestProjectCreate(TestProject):
] ]
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('parent', None), ('parent', None),
('tags', []), ('tags', []),
@@ -210,8 +207,7 @@ class TestProjectCreate(TestProject):
] ]
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('parent', None), ('parent', None),
('tags', []), ('tags', []),
@@ -243,8 +239,7 @@ class TestProjectCreate(TestProject):
self.project.name, self.project.name,
] ]
verifylist = [ verifylist = [
('enable', True), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('parent', None), ('parent', None),
('tags', []), ('tags', []),
@@ -279,8 +274,7 @@ class TestProjectCreate(TestProject):
self.project.name, self.project.name,
] ]
verifylist = [ verifylist = [
('enable', False), ('enabled', False),
('disable', True),
('name', self.project.name), ('name', self.project.name),
('parent', None), ('parent', None),
] ]
@@ -317,7 +311,7 @@ class TestProjectCreate(TestProject):
self.project.name, self.project.name,
] ]
verifylist = [ verifylist = [
('property', {'fee': 'fi', 'fo': 'fum'}), ('properties', {'fee': 'fi', 'fo': 'fum'}),
('name', self.project.name), ('name', self.project.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -354,11 +348,10 @@ class TestProjectCreate(TestProject):
] ]
verifylist = [ verifylist = [
('parent', None), ('parent', None),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('tags', []), ('tags', []),
('property', {'is_domain': 'false'}), ('properties', {'is_domain': 'false'}),
('name', self.project.name), ('name', self.project.name),
] ]
@@ -393,11 +386,10 @@ class TestProjectCreate(TestProject):
] ]
verifylist = [ verifylist = [
('parent', None), ('parent', None),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('tags', []), ('tags', []),
('property', {'is_domain': 'true'}), ('properties', {'is_domain': 'true'}),
('name', self.project.name), ('name', self.project.name),
] ]
@@ -432,11 +424,10 @@ class TestProjectCreate(TestProject):
] ]
verifylist = [ verifylist = [
('parent', None), ('parent', None),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('tags', []), ('tags', []),
('property', {'is_domain': 'none'}), ('properties', {'is_domain': 'none'}),
('name', self.project.name), ('name', self.project.name),
] ]
@@ -481,8 +472,7 @@ class TestProjectCreate(TestProject):
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('parent', self.parent.name), ('parent', self.parent.name),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('tags', []), ('tags', []),
] ]
@@ -544,8 +534,7 @@ class TestProjectCreate(TestProject):
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('parent', 'invalid'), ('parent', 'invalid'),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -566,8 +555,7 @@ class TestProjectCreate(TestProject):
] ]
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('parent', None), ('parent', None),
('tags', ['foo']), ('tags', ['foo']),
@@ -602,8 +590,7 @@ class TestProjectCreate(TestProject):
verifylist = [ verifylist = [
('immutable', True), ('immutable', True),
('description', None), ('description', None),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('parent', None), ('parent', None),
('tags', []), ('tags', []),
@@ -638,10 +625,9 @@ class TestProjectCreate(TestProject):
self.project.name, self.project.name,
] ]
verifylist = [ verifylist = [
('no_immutable', True), ('immutable', False),
('description', None), ('description', None),
('enable', False), ('enabled', True),
('disable', False),
('name', self.project.name), ('name', self.project.name),
('parent', None), ('parent', None),
('tags', []), ('tags', []),
@@ -981,8 +967,7 @@ class TestProjectSet(TestProject):
] ]
verifylist = [ verifylist = [
('project', self.project.name), ('project', self.project.name),
('enable', False), ('enabled', None),
('disable', False),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -1001,8 +986,7 @@ class TestProjectSet(TestProject):
verifylist = [ verifylist = [
('name', 'qwerty'), ('name', 'qwerty'),
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('enable', False), ('enabled', None),
('disable', False),
('project', self.project.name), ('project', self.project.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -1029,8 +1013,7 @@ class TestProjectSet(TestProject):
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('description', 'new desc'), ('description', 'new desc'),
('enable', False), ('enabled', None),
('disable', False),
('project', self.project.name), ('project', self.project.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -1053,8 +1036,7 @@ class TestProjectSet(TestProject):
] ]
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('enable', True), ('enabled', True),
('disable', False),
('project', self.project.name), ('project', self.project.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -1077,8 +1059,7 @@ class TestProjectSet(TestProject):
] ]
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('enable', False), ('enabled', False),
('disable', True),
('project', self.project.name), ('project', self.project.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -1104,7 +1085,7 @@ class TestProjectSet(TestProject):
] ]
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('property', {'fee': 'fi', 'fo': 'fum'}), ('properties', {'fee': 'fi', 'fo': 'fum'}),
('project', self.project.name), ('project', self.project.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -1132,8 +1113,7 @@ class TestProjectSet(TestProject):
verifylist = [ verifylist = [
('name', 'qwerty'), ('name', 'qwerty'),
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('enable', False), ('enabled', None),
('disable', False),
('project', self.project.name), ('project', self.project.name),
('tags', ['foo']), ('tags', ['foo']),
] ]
@@ -1160,8 +1140,7 @@ class TestProjectSet(TestProject):
self.project.name, self.project.name,
] ]
verifylist = [ verifylist = [
('enable', False), ('enabled', None),
('disable', False),
('project', self.project.name), ('project', self.project.name),
('remove_tag', ['tag1', 'tag2']), ('remove_tag', ['tag1', 'tag2']),
] ]
@@ -1183,8 +1162,7 @@ class TestProjectSet(TestProject):
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('immutable', True), ('immutable', True),
('enable', False), ('enabled', None),
('disable', False),
('project', self.project.name), ('project', self.project.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -1207,9 +1185,8 @@ class TestProjectSet(TestProject):
] ]
verifylist = [ verifylist = [
('domain', self.project.domain_id), ('domain', self.project.domain_id),
('no_immutable', True), ('immutable', False),
('enable', False), ('enabled', None),
('disable', False),
('project', self.project.name), ('project', self.project.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)

10
openstackclient/tests/unit/identity/v3/test_role.py
View File

@@ -494,7 +494,6 @@ class TestRoleCreate(identity_fakes.TestIdentityv3):
# Set expected values # Set expected values
kwargs = { kwargs = {
'name': self.role.name, 'name': self.role.name,
'options': {},
} }
self.identity_sdk_client.create_role.assert_called_with(**kwargs) self.identity_sdk_client.create_role.assert_called_with(**kwargs)
@@ -533,7 +532,6 @@ class TestRoleCreate(identity_fakes.TestIdentityv3):
kwargs = { kwargs = {
'domain_id': self.domain.id, 'domain_id': self.domain.id,
'name': self.role_with_domain.name, 'name': self.role_with_domain.name,
'options': {},
} }
self.identity_sdk_client.create_role.assert_called_with(**kwargs) self.identity_sdk_client.create_role.assert_called_with(**kwargs)
@@ -572,7 +570,6 @@ class TestRoleCreate(identity_fakes.TestIdentityv3):
kwargs = { kwargs = {
'name': self.role_with_description.name, 'name': self.role_with_description.name,
'description': self.role_with_description.description, 'description': self.role_with_description.description,
'options': {},
} }
self.identity_sdk_client.create_role.assert_called_with(**kwargs) self.identity_sdk_client.create_role.assert_called_with(**kwargs)
@@ -629,7 +626,7 @@ class TestRoleCreate(identity_fakes.TestIdentityv3):
self.role.name, self.role.name,
] ]
verifylist = [ verifylist = [
('no_immutable', True), ('immutable', False),
('name', self.role.name), ('name', self.role.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -1437,7 +1434,6 @@ class TestRoleSet(identity_fakes.TestIdentityv3):
kwargs = { kwargs = {
'name': 'over', 'name': 'over',
'role': self.role.id, 'role': self.role.id,
'options': {},
} }
self.identity_sdk_client.update_role.assert_called_with(**kwargs) self.identity_sdk_client.update_role.assert_called_with(**kwargs)
self.assertIsNone(result) self.assertIsNone(result)
@@ -1472,7 +1468,6 @@ class TestRoleSet(identity_fakes.TestIdentityv3):
'name': 'over', 'name': 'over',
'role': self.role_with_domain.id, 'role': self.role_with_domain.id,
'domain_id': self.domain2.id, 'domain_id': self.domain2.id,
'options': {},
} }
self.identity_sdk_client.update_role.assert_called_with(**kwargs) self.identity_sdk_client.update_role.assert_called_with(**kwargs)
self.assertIsNone(result) self.assertIsNone(result)
@@ -1501,7 +1496,6 @@ class TestRoleSet(identity_fakes.TestIdentityv3):
'name': 'over', 'name': 'over',
'description': 'role description', 'description': 'role description',
'role': self.role_with_domain.id, 'role': self.role_with_domain.id,
'options': {},
} }
self.identity_sdk_client.update_role.assert_called_with(**kwargs) self.identity_sdk_client.update_role.assert_called_with(**kwargs)
self.assertIsNone(result) self.assertIsNone(result)
@@ -1544,7 +1538,7 @@ class TestRoleSet(identity_fakes.TestIdentityv3):
] ]
verifylist = [ verifylist = [
('name', 'over'), ('name', 'over'),
('no_immutable', True), ('immutable', False),
('role', self.role_with_domain.name), ('role', self.role_with_domain.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)