Correct permissions on the nova logfiles

When you execute nova-manage commands, oslo logs to the following
location (file name is dynamically created based on command name).

    /var/log/nova/nova-manage.log

Because puppet-nova is executing these commands as root,
nova-manage.log is owned by root, preventing the 'nova-manage
db archive_deleted_rows' entry in nova's crontab from executing.

    Permission denied: '/var/log/nova/nova-manage.log'

This log file is also an outlier, as all other log files in
/var/log/nova/ are owned by nova:nova.

Similar issues are possible for other nova logs, if for example
a nova services is initially started manually as root, so the
ownership of all nova logs is corrected before configuring nova.

Co-Authored-By: Oliver Walsh <owalsh@redhat.com>
Co-Authored-By: Diana Clarke <diana.joan.clarke@gmail.com>
Co-Authored-By: Maciej Kucia <maciej@kucia.net>
Closes-Bug: #1671681
Change-Id: I0ca0110cbf9139c79074cf603dcab9135f96e765
This commit is contained in:
Diana Clarke 2017-03-07 14:07:16 -05:00 committed by Oliver Walsh
parent 9606525df2
commit e128ba6538
21 changed files with 126 additions and 6 deletions

View File

@ -46,6 +46,7 @@ class Puppet::Provider::Nova < Puppet::Provider::Openstack
# NOTE(mnaser): We pass the arguments as an array to avoid problems with
# symbols in the arguments breaking things.
Puppet::Util::Execution.execute(args, {
:uid => nova_user,
:failonfail => true,
:combine => false,
:custom_environment => {}
@ -56,6 +57,10 @@ class Puppet::Provider::Nova < Puppet::Provider::Openstack
self.class.nova_manage_request(args)
end
def self.nova_user
'nova'
end
def self.conf_filename
'/etc/nova/nova.conf'
end

View File

@ -13,10 +13,12 @@ class nova::cell_v2::discover_hosts (
) {
include ::nova::deps
include ::nova::params
exec { 'nova-cell_v2-discover_hosts':
path => ['/bin', '/usr/bin'],
command => "nova-manage ${extra_params} cell_v2 discover_hosts",
user => $::nova::params::nova_user,
refreshonly => true,
subscribe => Anchor['nova::service::end']
}

View File

@ -16,10 +16,12 @@ class nova::cell_v2::map_cell0 (
) {
include ::nova::deps
include ::nova::params
exec { 'nova-cell_v2-map_cell0':
path => ['/bin', '/usr/bin'],
command => "nova-manage ${extra_params} cell_v2 map_cell0",
user => $::nova::params::nova_user,
refreshonly => true,
logoutput => on_failure,
subscribe => Anchor['nova::cell_v2::begin'],

View File

@ -13,10 +13,12 @@ class nova::cell_v2::map_cell_and_hosts (
) {
include ::nova::deps
include ::nova::params
exec { 'nova-cell_v2-map_cell_and_hosts':
path => ['/bin', '/usr/bin'],
command => "nova-manage ${extra_params} cell_v2 map_cell_and_hosts",
user => $::nova::params::nova_user,
refreshonly => true,
}
}

View File

@ -26,6 +26,7 @@ class nova::cell_v2::map_instances (
) {
include ::nova::deps
include ::nova::params
if (!$cell_uuid and !$cell_name) {
fail('Either cell_uuid or cell_name must be provided')
@ -42,6 +43,7 @@ class nova::cell_v2::map_instances (
exec { 'nova-cell_v2-map_instances':
path => ['/bin', '/usr/bin'],
command => "nova-manage ${extra_params} cell_v2 map_instances --cell_uuid=${cell_uuid_real}",
user => $::nova::params::nova_user,
refreshonly => true,
}
}

View File

@ -43,7 +43,8 @@
#
# [*user*]
# (optional) User with access to nova files.
# Defaults to 'nova'.
# nova::params::nova_user will be used if this is undef.
# Defaults to undef.
#
# [*destination*]
# (optional) Path to file to which rows should be archived
@ -60,12 +61,13 @@ class nova::cron::archive_deleted_rows (
$month = '*',
$weekday = '*',
$max_rows = '100',
$user = 'nova',
$user = undef,
$destination = '/var/log/nova/nova-rowsflush.log',
$until_complete = false,
) {
include ::nova::deps
include ::nova::params
if $until_complete {
$until_complete_real = '--until_complete'
@ -74,7 +76,7 @@ class nova::cron::archive_deleted_rows (
cron { 'nova-manage db archive_deleted_rows':
command => "nova-manage db archive_deleted_rows --max_rows ${max_rows} ${until_complete_real} >>${destination} 2>&1",
environment => 'PATH=/bin:/usr/bin:/usr/sbin SHELL=/bin/sh',
user => $user,
user => pick($user, $::nova::params::nova_user),
minute => $minute,
hour => $hour,
monthday => $monthday,

View File

@ -23,6 +23,7 @@ class nova::db::online_data_migrations(
exec { 'nova-db-online-data-migrations':
command => "/usr/bin/nova-manage ${extra_params} db online_data_migrations",
user => $::nova::params::nova_user,
refreshonly => true,
try_sleep => 5,
tries => 10,

View File

@ -23,6 +23,7 @@ class nova::db::sync(
exec { 'nova-db-sync':
command => "/usr/bin/nova-manage ${extra_params} db sync",
user => $::nova::params::nova_user,
refreshonly => true,
try_sleep => 5,
tries => 10,

View File

@ -32,6 +32,7 @@ class nova::db::sync_api(
exec { 'nova-db-sync-api':
command => "/usr/bin/nova-manage ${extra_params} api_db sync",
user => $::nova::params::nova_user,
refreshonly => true,
try_sleep => 5,
tries => 10,

View File

@ -110,6 +110,7 @@ class nova::logging(
) {
include ::nova::deps
include ::nova::params
# NOTE(spredzy): In order to keep backward compatibility we rely on the pick function
# to use nova::<myparam> first then nova::logging::<myparam>.
@ -123,6 +124,46 @@ class nova::logging(
}
$debug_real = pick($::nova::debug,$debug)
if $log_dir_real != $::os_service_default {
# TODO: can probably remove this in Rocky once we've had it for 1 upgrade cycle
# Ensure ownership/permissions for any existing logfiles are correct before configuring nova
# This matches the rpm/deb logic:
# Ubuntu: /var/log/nova is nova:adm
# CentOS: /var/log/nova is nova:root
# Both: /var/log/nova/*.log is nova:nova
$log_dir_owner = $::nova::params::nova_user
$log_dir_group = $::nova::params::nova_log_group
$log_file_owner = $::nova::params::nova_user
$log_file_group = $::nova::params::nova_group
file { $log_dir_real:
ensure => directory,
owner => $log_dir_owner,
group => $log_dir_group,
require => Anchor['nova::install::end'],
before => Anchor['nova::config::begin']
}
# Can't tell File[$log_dir_real] to use a different user/group when recursing so resort to chown
exec { 'chown nova logfiles':
command => "chown ${log_file_owner}:${log_file_group} ${log_dir_real}/*.log",
onlyif => "test \"\$(stat -c '%U:%G' ${log_dir_real}/*.log | grep -v '${log_file_owner}:${log_file_group}')\" != ''",
path => ['/usr/bin', '/bin'],
require => File[$log_dir_real],
before => Anchor['nova::config::begin']
}
# END TODO, the following resource is likely to be necessary in Rocky and later
# This should force an update the selinux role if the logfile exists.
# It will be incorrect if the file was created by the dbsync exec resources.
file { "${log_dir_real}/nova-manage.log":
owner => $log_file_owner,
group => $log_file_group,
require => Anchor['nova::service::end']
}
}
oslo::log { 'nova_config':
debug => $debug_real,
use_stderr => $use_stderr_real,

View File

@ -45,7 +45,7 @@ class nova::params {
# redhat specific config defaults
$root_helper = 'sudo nova-rootwrap'
$lock_path = '/var/lib/nova/tmp'
$nova_log_group = 'nova'
$nova_log_group = 'root'
$nova_wsgi_script_path = '/var/www/cgi-bin/nova'
$nova_api_wsgi_script_source = '/usr/bin/nova-api-wsgi'
$placement_wsgi_script_source = '/usr/bin/nova-placement-api'
@ -70,6 +70,8 @@ class nova::params {
$messagebus_service_name = undef
}
}
$nova_user = 'nova'
$nova_group = 'nova'
}
'Debian': {
# package names
@ -132,6 +134,8 @@ class nova::params {
}
}
$libvirt_service_name = 'libvirtd'
$nova_user = 'nova'
$nova_group = 'nova'
}
default: {
fail("Unsupported osfamily: ${::osfamily} operatingsystem: ${::operatingsystem}, \

View File

@ -0,0 +1,17 @@
---
fixes:
- |
Correct permissions on the nova logfiles.
If the ``nova-manage`` commands (such as dbsync) were initially run as
root then subsequent runs as the nova user would fail as the logfile is
owned by root (see `bug 1671681`_).
The ownership of all nova logfiles is now checked before configuring
nova, as a similar issue could prevent a service starting, and the
nova-manage command is now run as the correct user.
Adds nova::params::nova_user and nova::params::nova_group.
nova::cron::archive_deleted_rows::user now defaults to
nova::params::nova_user instead of hardcoding 'nova'
.. _bug 1671681: https://bugs.launchpad.net/puppet-nova/+bug/1671681

View File

@ -9,6 +9,7 @@ describe 'nova::cell_v2::discover_hosts' do
is_expected.to contain_exec('nova-cell_v2-discover_hosts').with(
:path => ['/bin', '/usr/bin'],
:command => 'nova-manage cell_v2 discover_hosts',
:user => 'nova',
:refreshonly => true,
:subscribe => 'Anchor[nova::service::end]'
)
@ -26,6 +27,7 @@ describe 'nova::cell_v2::discover_hosts' do
is_expected.to contain_exec('nova-cell_v2-discover_hosts').with(
:path => ['/bin', '/usr/bin'],
:command => 'nova-manage --config-file /etc/nova/nova.conf cell_v2 discover_hosts',
:user => 'nova',
:refreshonly => true,
:subscribe => 'Anchor[nova::service::end]'
)

View File

@ -9,6 +9,7 @@ describe 'nova::cell_v2::map_cell0' do
is_expected.to contain_exec('nova-cell_v2-map_cell0').with(
:path => ['/bin', '/usr/bin'],
:command => 'nova-manage cell_v2 map_cell0',
:user => 'nova',
:refreshonly => 'true',
:logoutput => 'on_failure',
:subscribe => 'Anchor[nova::cell_v2::begin]',
@ -28,6 +29,7 @@ describe 'nova::cell_v2::map_cell0' do
is_expected.to contain_exec('nova-cell_v2-map_cell0').with(
:path => ['/bin', '/usr/bin'],
:command => 'nova-manage --config-file /etc/nova/nova.conf cell_v2 map_cell0',
:user => 'nova',
:refreshonly => 'true',
:logoutput => 'on_failure',
:subscribe => 'Anchor[nova::cell_v2::begin]',

View File

@ -9,6 +9,7 @@ describe 'nova::cell_v2::map_cell_and_hosts' do
is_expected.to contain_exec('nova-cell_v2-map_cell_and_hosts').with(
:path => ['/bin', '/usr/bin'],
:command => 'nova-manage cell_v2 map_cell_and_hosts',
:user => 'nova',
:refreshonly => true,
)
}
@ -25,6 +26,7 @@ describe 'nova::cell_v2::map_cell_and_hosts' do
is_expected.to contain_exec('nova-cell_v2-map_cell_and_hosts').with(
:path => ['/bin', '/usr/bin'],
:command => 'nova-manage --config-file /etc/nova/nova.conf cell_v2 map_cell_and_hosts',
:user => 'nova',
:refreshonly => true,
)
}

View File

@ -10,6 +10,7 @@ describe 'nova::cell_v2::map_instances' do
is_expected.to contain_exec('nova-cell_v2-map_instances').with(
:path => ['/bin', '/usr/bin'],
:command => 'nova-manage cell_v2 map_instances --cell_uuid=uuid',
:user => 'nova',
:refreshonly => true,
)
}
@ -27,6 +28,7 @@ describe 'nova::cell_v2::map_instances' do
is_expected.to contain_exec('nova-cell_v2-map_instances').with(
:path => ['/bin', '/usr/bin'],
:command => 'nova-manage --config-file /etc/nova/nova.conf cell_v2 map_instances --cell_uuid=uuid',
:user => 'nova',
:refreshonly => true,
)
}

View File

@ -22,6 +22,7 @@ describe 'nova::cron::archive_deleted_rows' do
it 'configures a cron without until_complete' do
is_expected.to contain_cron('nova-manage db archive_deleted_rows').with(
:command => "nova-manage db archive_deleted_rows --max_rows #{params[:max_rows]} >>#{params[:destination]} 2>&1",
:user => 'nova',
:environment => 'PATH=/bin:/usr/bin:/usr/sbin SHELL=/bin/sh',
:user => params[:user],
:minute => params[:minute],
@ -44,6 +45,7 @@ describe 'nova::cron::archive_deleted_rows' do
it 'configures a cron with until_complete' do
is_expected.to contain_cron('nova-manage db archive_deleted_rows').with(
:command => "nova-manage db archive_deleted_rows --max_rows #{params[:max_rows]} --until_complete >>#{params[:destination]} 2>&1",
:user => 'nova',
:environment => 'PATH=/bin:/usr/bin:/usr/sbin SHELL=/bin/sh',
:user => params[:user],
:minute => params[:minute],

View File

@ -7,6 +7,7 @@ describe 'nova::db::online_data_migrations' do
it 'runs nova-db-sync' do
is_expected.to contain_exec('nova-db-online-data-migrations').with(
:command => '/usr/bin/nova-manage db online_data_migrations',
:user => 'nova',
:refreshonly => 'true',
:try_sleep => 5,
:tries => 10,
@ -30,6 +31,7 @@ describe 'nova::db::online_data_migrations' do
it {
is_expected.to contain_exec('nova-db-online-data-migrations').with(
:command => '/usr/bin/nova-manage --config-file /etc/nova/nova.conf db online_data_migrations',
:user => 'nova',
:refreshonly => 'true',
:try_sleep => 5,
:tries => 10,
@ -54,6 +56,7 @@ describe 'nova::db::online_data_migrations' do
it {
is_expected.to contain_exec('nova-db-online-data-migrations').with(
:command => '/usr/bin/nova-manage db online_data_migrations',
:user => 'nova',
:refreshonly => 'true',
:try_sleep => 5,
:tries => 10,

View File

@ -7,6 +7,7 @@ describe 'nova::db::sync_api' do
it {
is_expected.to contain_exec('nova-db-sync-api').with(
:command => '/usr/bin/nova-manage api_db sync',
:user => 'nova',
:refreshonly => 'true',
:timeout => 300,
:logoutput => 'on_failure',
@ -31,6 +32,7 @@ describe 'nova::db::sync_api' do
it {
is_expected.to contain_exec('nova-db-sync-api').with(
:command => '/usr/bin/nova-manage --config-file /etc/nova/nova.conf api_db sync',
:user => 'nova',
:refreshonly => 'true',
:timeout => 300,
:logoutput => 'on_failure',
@ -54,6 +56,7 @@ describe 'nova::db::sync_api' do
it {
is_expected.to contain_exec('nova-db-sync-api').with(
:command => '/usr/bin/nova-manage api_db sync',
:user => 'nova',
:refreshonly => 'true',
:timeout => 750,
:logoutput => 'on_failure',

View File

@ -7,6 +7,7 @@ describe 'nova::db::sync' do
it 'runs nova-db-sync' do
is_expected.to contain_exec('nova-db-sync').with(
:command => '/usr/bin/nova-manage db sync',
:user => 'nova',
:refreshonly => 'true',
:try_sleep => 5,
:tries => 10,
@ -30,6 +31,7 @@ describe 'nova::db::sync' do
it {
is_expected.to contain_exec('nova-db-sync').with(
:command => '/usr/bin/nova-manage --config-file /etc/nova/nova.conf db sync',
:user => 'nova',
:refreshonly => 'true',
:try_sleep => 5,
:tries => 10,
@ -54,6 +56,7 @@ describe 'nova::db::sync' do
it {
is_expected.to contain_exec('nova-db-sync').with(
:command => '/usr/bin/nova-manage db sync',
:user => 'nova',
:refreshonly => 'true',
:try_sleep => 5,
:tries => 10,

View File

@ -27,7 +27,7 @@ describe 'nova::logging' do
:use_json => true,
:use_stderr => false,
:log_facility => 'LOG_FOO',
:log_dir => '/var/log',
:log_dir => '/var/log/foo',
:debug => true,
}
end
@ -63,6 +63,13 @@ describe 'nova::logging' do
:log_dir => '/var/log/nova',
:debug => '<SERVICE DEFAULT>',
)
is_expected.to contain_file('/var/log/nova').with(
:owner => 'nova',
)
is_expected.to contain_file('/var/log/nova/nova-manage.log').with(
:owner => 'nova',
)
is_expected.to contain_exec('chown nova logfiles')
end
end
@ -73,9 +80,16 @@ describe 'nova::logging' do
:use_json => true,
:use_stderr => false,
:syslog_log_facility => 'LOG_FOO',
:log_dir => '/var/log',
:log_dir => '/var/log/foo',
:debug => true,
)
is_expected.to contain_file('/var/log/foo').with(
:owner => 'nova',
)
is_expected.to contain_file('/var/log/foo/nova-manage.log').with(
:owner => 'nova',
)
is_expected.to contain_exec('chown nova logfiles')
end
end
@ -98,6 +112,13 @@ describe 'nova::logging' do
:instance_uuid_format => '[instance: %(uuid)s] ',
:log_date_format => '%Y-%m-%d %H:%M:%S',
)
is_expected.to contain_file('/var/log/foo').with(
:owner => 'nova',
)
is_expected.to contain_file('/var/log/foo/nova-manage.log').with(
:owner => 'nova',
)
is_expected.to contain_exec('chown nova logfiles')
end
end