From da28046944aaa5b6068d2cc8f14e72ef1de6c012 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Tue, 4 Feb 2020 14:06:41 -0800 Subject: [PATCH] Default to bootstrapping roles as immutable In the previous cycle, the ``--immutable-roles`` option was added to the bootstrap command as an optional way to opt-in to making the default roles immutable. Following step 4 of the spec[1], we now make that behavior the default and additionally offer a way to opt out of it. [1] http://specs.openstack.org/openstack/keystone-specs/specs/keystone/train/immutable-resources.html#proposed-change Change-Id: I6b680efb2c87c1d7559ddcc989bbce68456b9a5f Closes-Bug: #1823258 --- keystone/cmd/bootstrap.py | 11 ++---- keystone/cmd/cli.py | 17 +++++++-- keystone/tests/unit/test_cli.py | 37 +++++++++++++++++-- .../notes/bug-1823258-9649b56a440b5ae1.yaml | 10 +++++ 4 files changed, 60 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/bug-1823258-9649b56a440b5ae1.yaml diff --git a/keystone/cmd/bootstrap.py b/keystone/cmd/bootstrap.py index 350f0de49a..bf9d960cff 100644 --- a/keystone/cmd/bootstrap.py +++ b/keystone/cmd/bootstrap.py @@ -122,13 +122,10 @@ class Bootstrapper(object): LOG.info('Created role %s', role_name) if not self.immutable_roles: LOG.warning("Role %(role)s was created as a mutable role. It " - "is recommended to make this role immutable, " - "which will become the default behavior of the " - "bootstrap command in the future.You can opt into " - "this behavior by using the --immutable-role " - "flag, or update role %(role)s with the " - "'immutable' resource option.", - {'role': role_name}) + "is recommended to make this role immutable by " + "adding the 'immutable' resource option to this " + "role, or re-running this command without " + "--no-immutable-role.", {'role': role_name}) return role except exception.Conflict: LOG.info('Role %s exists, skipping creation.', role_name) diff --git a/keystone/cmd/cli.py b/keystone/cmd/cli.py index daf876e24f..acfffa3449 100644 --- a/keystone/cmd/cli.py +++ b/keystone/cmd/cli.py @@ -113,13 +113,19 @@ class BootStrap(BaseApp): 'placed in during the keystone bootstrap ' 'process.')) parser.add_argument('--immutable-roles', + default=True, + action='store_true', + help=('Whether default roles (admin, member, and ' + 'reader) should be immutable. This is the ' + 'default.')) + parser.add_argument('--no-immutable-roles', default=False, action='store_true', help=('Whether default roles (admin, member, and ' 'reader) should be immutable. Immutable ' - 'default roles is currently an opt-in ' - 'behavior, but will become the default in ' - 'future releases.')) + 'default roles is the default, use this ' + 'flag to opt out of immutable default ' + 'roles.')) return parser def do_bootstrap(self): @@ -175,7 +181,10 @@ class BootStrap(BaseApp): self.bootstrapper.public_url = self.public_url self.bootstrapper.internal_url = self.internal_url self.bootstrapper.region_id = self.region_id - self.bootstrapper.immutable_roles = CONF.command.immutable_roles + if CONF.command.no_immutable_roles: + self.bootstrapper.immutable_roles = False + else: + self.bootstrapper.immutable_roles = True self.bootstrapper.bootstrap() self.reader_role_id = self.bootstrapper.reader_role_id diff --git a/keystone/tests/unit/test_cli.py b/keystone/tests/unit/test_cli.py index e8c73e300b..db160457b5 100644 --- a/keystone/tests/unit/test_cli.py +++ b/keystone/tests/unit/test_cli.py @@ -224,9 +224,9 @@ class CliBootStrapTestCase(unit.SQLDriverOverrides, unit.TestCase): self.bootstrap.reader_role_id) member_role = PROVIDERS.role_api.get_role( self.bootstrap.member_role_id) - self.assertEqual(admin_role['options'], {}) - self.assertEqual(member_role['options'], {}) - self.assertEqual(reader_role['options'], {}) + self.assertEqual(admin_role['options'], {'immutable': True}) + self.assertEqual(member_role['options'], {'immutable': True}) + self.assertEqual(reader_role['options'], {'immutable': True}) def test_bootstrap_is_not_idempotent_when_password_does_change(self): # NOTE(lbragstad): Ensure bootstrap isn't idempotent when run with @@ -300,7 +300,7 @@ class CliBootStrapTestCase(unit.SQLDriverOverrides, unit.TestCase): user_id, self.bootstrap.password) - def test_bootstrap_with_immutable_roles(self): + def test_bootstrap_with_explicit_immutable_roles(self): CONF(args=['bootstrap', '--bootstrap-password', uuid.uuid4().hex, '--immutable-roles'], @@ -315,6 +315,35 @@ class CliBootStrapTestCase(unit.SQLDriverOverrides, unit.TestCase): self.assertTrue(member_role['options']['immutable']) self.assertTrue(reader_role['options']['immutable']) + def test_bootstrap_with_default_immutable_roles(self): + CONF(args=['bootstrap', + '--bootstrap-password', uuid.uuid4().hex], + project='keystone') + self._do_test_bootstrap(self.bootstrap) + admin_role = PROVIDERS.role_api.get_role(self.bootstrap.role_id) + reader_role = PROVIDERS.role_api.get_role( + self.bootstrap.reader_role_id) + member_role = PROVIDERS.role_api.get_role( + self.bootstrap.member_role_id) + self.assertTrue(admin_role['options']['immutable']) + self.assertTrue(member_role['options']['immutable']) + self.assertTrue(reader_role['options']['immutable']) + + def test_bootstrap_with_no_immutable_roles(self): + CONF(args=['bootstrap', + '--bootstrap-password', uuid.uuid4().hex, + '--no-immutable-roles'], + project='keystone') + self._do_test_bootstrap(self.bootstrap) + admin_role = PROVIDERS.role_api.get_role(self.bootstrap.role_id) + reader_role = PROVIDERS.role_api.get_role( + self.bootstrap.reader_role_id) + member_role = PROVIDERS.role_api.get_role( + self.bootstrap.member_role_id) + self.assertNotIn('immutable', admin_role['options']) + self.assertNotIn('immutable', member_role['options']) + self.assertNotIn('immutable', reader_role['options']) + def test_bootstrap_with_ambiguous_role_names(self): # bootstrap system to create the default admin role self._do_test_bootstrap(self.bootstrap) diff --git a/releasenotes/notes/bug-1823258-9649b56a440b5ae1.yaml b/releasenotes/notes/bug-1823258-9649b56a440b5ae1.yaml new file mode 100644 index 0000000000..d703007c2b --- /dev/null +++ b/releasenotes/notes/bug-1823258-9649b56a440b5ae1.yaml @@ -0,0 +1,10 @@ +--- +upgrade: + - | + [`bug 1823258 `_] + The ``keystone-manage bootstrap`` command now defaults to making the + default roles (`admin`, `member`, and `reader`) immutable. This has the + consequence that if the bootstrap command is re-run on an existing + deployment, those roles will become immutable if they were not before. To + opt out of this behavior, add the ``--no-immutable-roles`` flag to the + bootstrap command.