Work around deletion of _member_ role assignments on upgrade

TL;DR:
To support upgrades, we need to make sure _member_ role is still
assigned to the admin user if that role assignment existed before
the undercloud was upgraded.

Background:
Until fairly recently, keystone created a _member_ role and this was
applied to all users, including the admin role used for overcloud
deployments.

This creates a problem on upgrade however, because puppet isn't
aware of any existing role assignments, hence it deletes them,
and heat contains a reference to a keystone trust, which expects
to delegate this role (by default all roles are delegated, so
on an older deployment the trust will delegate _member_ and admin).

The _member_ role is not needed anymore by new deployments, and is
no longer created, so we need to detect the existence of this legacy
role, and apply it only for environments where it exists - we can
safely ignore connection errors trying to connect to keystone and
read the roles during an intial deployment, because no new deployments
should ever contain the _member_ role, it's only an upgrade issue.

This patch is a Puppet collector, that will collect the admin role
assignement resource, and makes sure we assign both admin and _member_
based on a flag set by undercloud.py that is passed via hiera.

Closes-Bug: #1571708
Co-Authored-By: Steven Hardy <shardy@redhat.com>
Co-Authored-By: Juan Antonio Osorio Robles <jaosorior@redhat.com>
Change-Id: I06ccdeb01e0aa69754855a0dcae3087725399583
This commit is contained in:
Emilien Macchi 2016-04-18 12:09:37 -04:00 committed by Juan Antonio Osorio Robles
parent 5902936ecc
commit f623801860
6 changed files with 120 additions and 1 deletions

View File

@ -36,6 +36,7 @@ context = {
os.environ['TRIPLEO_UNDERCLOUD_CONF_FILE'],
'TRIPLEO_UNDERCLOUD_PASSWORD_FILE':
os.environ['TRIPLEO_UNDERCLOUD_PASSWORD_FILE'],
'MEMBER_ROLE_EXISTS': os.environ.get('MEMBER_ROLE_EXISTS', 'false'),
}
# Include all config opts in the context

View File

@ -259,6 +259,15 @@ keystone_config {
'ec2/driver': value => 'keystone.contrib.ec2.backends.sql.Ec2';
}
if str2bool(hiera('member_role_exists', false)) {
# Old deployments where assigning _member_ role to admin user.
# The _member_ role is needed because it's delegated via heat trusts in
# existing deployments, hence existing role assignments can't just be
# deleted. This Puppet Collector will allow to update deployments with
# admin role managed by Puppet.
Keystone_user_role<| title == 'admin@admin' |> { roles +> ['_member_'] }
}
# TODO: notifications, scrubber, etc.
class { '::glance::api':
enable_proxy_headers_parsing => $enable_proxy_headers_parsing,

View File

@ -31,6 +31,10 @@ tripleo::profile::base::haproxy::certificates_specs:
# CA defaults
certmonger_ca: {{CERTIFICATE_GENERATION_CA}}
# Workaround for puppet deleting _member_ role assignment on old deployments
member_role_exists: {{MEMBER_ROLE_EXISTS}}
# Common Hiera data gets applied to all nodes
ssh::server::storeconfigs_enabled: false

View File

@ -200,6 +200,11 @@ class TestGenerateEnvironment(BaseTestCase):
self.useFixture(self.mock_distro)
self.mock_distro.mock.return_value = [
'Red Hat Enterprise Linux Server 7.1']
stackrc_exists_patcher = mock.patch(
'instack_undercloud.undercloud._stackrc_exists',
return_value=False)
stackrc_exists_patcher.start()
self.addCleanup(stackrc_exists_patcher.stop)
@mock.patch('socket.gethostname')
def test_hostname_set(self, mock_gethostname):
@ -546,6 +551,53 @@ class TestPostConfig(base.BaseTestCase):
]
mock_run.assert_has_calls(calls)
@mock.patch('instack_undercloud.undercloud._stackrc_exists')
def test_member_role_exists_nostackrc(self, mock_stackrc_exists):
mock_stackrc_exists.return_value = False
instack_env = {}
undercloud._member_role_exists(instack_env)
self.assertEqual({'MEMBER_ROLE_EXISTS': 'False'}, instack_env)
def _mock_ksclient_roles(self, mock_auth_values, mock_ksdiscover, roles):
mock_auth_values.return_value = ('user', 'password',
'tenant', 'http://test:123')
mock_discover = mock.Mock()
mock_ksdiscover.return_value = mock_discover
mock_client = mock.Mock()
mock_roles = mock.Mock()
mock_role_list = []
for role in roles:
mock_role = mock.Mock()
mock_role.name = role
mock_role_list.append(mock_role)
mock_roles.list.return_value = mock_role_list
mock_client.roles = mock_roles
mock_discover.create_client.return_value = mock_client
@mock.patch('keystoneclient.discover.Discover')
@mock.patch('instack_undercloud.undercloud._get_auth_values')
@mock.patch('instack_undercloud.undercloud._stackrc_exists')
def test_member_role_exists(self, mock_stackrc_exists, mock_auth_values,
mock_ksdiscover):
mock_stackrc_exists.return_value = True
self._mock_ksclient_roles(mock_auth_values, mock_ksdiscover,
['admin'])
instack_env = {}
undercloud._member_role_exists(instack_env)
self.assertEqual({'MEMBER_ROLE_EXISTS': 'False'}, instack_env)
@mock.patch('keystoneclient.discover.Discover')
@mock.patch('instack_undercloud.undercloud._get_auth_values')
@mock.patch('instack_undercloud.undercloud._stackrc_exists')
def test_member_role_exists_true(self, mock_stackrc_exists,
mock_auth_values, mock_ksdiscover):
mock_stackrc_exists.return_value = True
self._mock_ksclient_roles(mock_auth_values, mock_ksdiscover,
['admin', '_member_'])
instack_env = {}
undercloud._member_role_exists(instack_env)
self.assertEqual({'MEMBER_ROLE_EXISTS': 'True'}, instack_env)
def _create_flavor_mocks(self):
mock_nova = mock.Mock()
mock_nova.flavors.create = mock.Mock()

View File

@ -28,6 +28,10 @@ import sys
import tempfile
import uuid
from keystoneclient import exceptions as ks_exceptions
from keystoneclient import auth
from keystoneclient import session
from keystoneclient import discover
from mistralclient.api import client as mistralclient
from novaclient import client as novaclient
from novaclient import exceptions
@ -743,6 +747,41 @@ def _write_password_file(instack_env):
password_file.write('%s=%s\n' % (opt.name, value))
def _member_role_exists(instack_env):
# This is a workaround for puppet removing the deprecated _member_
# role on upgrade - if it exists we must not remove role assignments
# or trusts stored in the undercloud heat will break
if not _stackrc_exists():
instack_env['MEMBER_ROLE_EXISTS'] = 'False'
return
user, password, tenant, auth_url = _get_auth_values()
role_exists = False
try:
# Note this is made somewhat verbose due to trying to handle
# any format auth_url (versionless, v2,0/v3 suffix)
auth_plugin_class = auth.get_plugin_class('password')
auth_kwargs = {
'auth_url': auth_url,
'username': user,
'password': password,
'project_name': tenant}
if 'v2.0' not in auth_url:
auth_kwargs.update({
'project_domain_name': 'default',
'user_domain_name': 'default'})
auth_plugin = auth_plugin_class(**auth_kwargs)
sess = session.Session(auth=auth_plugin)
disc = discover.Discover(session=sess)
c = disc.create_client()
role_names = [r.name for r in c.roles.list()]
role_exists = '_member_' in role_names
except ks_exceptions.ConnectionError:
# This will happen on initial deployment, assume False
# as no new deployments should have _member_
role_exists = False
instack_env['MEMBER_ROLE_EXISTS'] = six.text_type(role_exists)
def _generate_environment(instack_root):
"""Generate an environment dict for instack
@ -846,6 +885,8 @@ def _generate_environment(instack_root):
instack_env['UNDERCLOUD_SERVICE_CERTIFICATE'] = (
'/etc/pki/tls/certs/undercloud-%s.pem' % public_vip)
_member_role_exists(instack_env)
return instack_env
@ -995,9 +1036,20 @@ def _ensure_flavor(nova, name, profile=None):
LOG.info(message, name, profile)
def _stackrc_exists():
user_stackrc = os.path.expanduser('~/stackrc')
# We gotta check if the copying of stackrc has already been done.
if os.path.isfile('/root/stackrc') and os.path.isfile(user_stackrc):
return True
return False
def _copy_stackrc():
args = ['sudo', 'cp', '/root/stackrc', os.path.expanduser('~')]
_run_command(args, name='Copy stackrc')
try:
_run_command(args, name='Copy stackrc')
except subprocess.CalledProcessError:
LOG.info("/root/stackrc not found, this is OK on initial deploy")
args = ['sudo', 'chown', getpass.getuser() + ':',
os.path.expanduser('~/stackrc')]
_run_command(args, name='Chown stackrc')

View File

@ -1,4 +1,5 @@
six>=1.9.0
python-keystoneclient>=2.0.0,!=2.1.0 # Apache-2.0
python-novaclient
python-mistralclient>=2.0.0 # Apache-2.0
oslo.config