From fd2bf0ac464690bdf7029975fb8cc997b8255374 Mon Sep 17 00:00:00 2001 From: Michael Polenchuk Date: Mon, 14 Dec 2015 17:15:53 +0300 Subject: [PATCH] Enable X-Forwarded-For for RadosGW service Enable insertion of the X-Forwarded-For header to requests sent to RadosGW servers. Added "httpclose" on server side to ensure that every request will be rewritten and not only the first one of each session. Co-authored-by: Matthew Mosesohn Change-Id: Ieed9d2cc7b06ba2b864b2751b76dce010178be65 Closes-Bug: #1523906 Closes-Bug: #1523895 --- deployment/puppet/ceph/manifests/params.pp | 2 + deployment/puppet/ceph/manifests/radosgw.pp | 8 +-- deployment/puppet/ceph/templates/rgw.conf.erb | 4 ++ .../puppet/openstack/manifests/ha/radosgw.pp | 2 +- .../spec/classes/openstack_ha_radosgw_spec.rb | 48 ++++++++------ deployment/puppet/osnailyfacter/.fixtures.yml | 6 ++ .../puppet/osnailyfacter/manifests/apache.pp | 27 +++++--- .../spec/classes/osnailyfacter_apache_spec.rb | 66 +++++++++++++++++++ tests/noop/spec/hosts/apache/apache_spec.rb | 7 ++ 9 files changed, 134 insertions(+), 36 deletions(-) create mode 100644 deployment/puppet/osnailyfacter/spec/classes/osnailyfacter_apache_spec.rb diff --git a/deployment/puppet/ceph/manifests/params.pp b/deployment/puppet/ceph/manifests/params.pp index 5d0ffa00c7..518d78d357 100644 --- a/deployment/puppet/ceph/manifests/params.pp +++ b/deployment/puppet/ceph/manifests/params.pp @@ -21,6 +21,7 @@ class ceph::params { $dir_httpd_conf = '/etc/httpd/conf/' $dir_httpd_sites = '/etc/httpd/conf.d/' $dir_httpd_ssl = '/etc/httpd/ssl/' + $dir_httpd_log = '/var/log/httpd/' package { ['ceph', 'redhat-lsb-core','ceph-deploy',]: ensure => installed, @@ -46,6 +47,7 @@ class ceph::params { $dir_httpd_conf = '/etc/httpd/conf/' $dir_httpd_sites = '/etc/apache2/sites-available/' $dir_httpd_ssl = '/etc/apache2/ssl/' + $dir_httpd_log = '/var/log/apache2/' package { ['ceph','ceph-deploy', ]: ensure => installed, diff --git a/deployment/puppet/ceph/manifests/radosgw.pp b/deployment/puppet/ceph/manifests/radosgw.pp index 3abdda5719..c0c32d2ced 100644 --- a/deployment/puppet/ceph/manifests/radosgw.pp +++ b/deployment/puppet/ceph/manifests/radosgw.pp @@ -1,10 +1,3 @@ -# enable an Apache module -define apache::loadmodule () { - exec { "/usr/sbin/a2enmod ${name}" : - unless => "/bin/readlink -e /etc/apache2/mods-enabled/${name}.load", - notify => Service['httpd'] - } -} # deploys Ceph radosgw as an Apache FastCGI application class ceph::radosgw ( @@ -49,6 +42,7 @@ class ceph::radosgw ( $keyring_path = "/etc/ceph/keyring.${rgw_id}" $radosgw_auth_key = "client.${rgw_id}" $dir_httpd_root = '/var/www/radosgw' + $dir_httpd_log = $::ceph::params::dir_httpd_log package { [$::ceph::params::package_radosgw, $::ceph::params::package_fastcgi, diff --git a/deployment/puppet/ceph/templates/rgw.conf.erb b/deployment/puppet/ceph/templates/rgw.conf.erb index 4766db0978..3920135054 100644 --- a/deployment/puppet/ceph/templates/rgw.conf.erb +++ b/deployment/puppet/ceph/templates/rgw.conf.erb @@ -19,6 +19,10 @@ FastCgiExternalServer <%= @dir_httpd_root %>/s3gw.fcgi -socket /tmp/radosgw.sock + ## Logging + ErrorLog "<%= @dir_httpd_log %>radosgw_error.log" + CustomLog "<%= @dir_httpd_log %>radosgw_access.log" forwarded + AllowEncodedSlashes On ServerSignature Off diff --git a/deployment/puppet/openstack/manifests/ha/radosgw.pp b/deployment/puppet/openstack/manifests/ha/radosgw.pp index 87f4598aed..5de417f16b 100644 --- a/deployment/puppet/openstack/manifests/ha/radosgw.pp +++ b/deployment/puppet/openstack/manifests/ha/radosgw.pp @@ -47,7 +47,7 @@ class openstack::ha::radosgw ( public_virtual_ip => $public_virtual_ip, server_names => $server_names, haproxy_config_options => { - 'option' => ['httplog', 'httpchk GET /'], + 'option' => ['httplog', 'httpchk HEAD /', 'http-server-close', 'forwardfor'], 'http-request' => 'set-header X-Forwarded-Proto https if { ssl_fc }', }, } diff --git a/deployment/puppet/openstack/spec/classes/openstack_ha_radosgw_spec.rb b/deployment/puppet/openstack/spec/classes/openstack_ha_radosgw_spec.rb index 114de41ffc..7f21e987af 100644 --- a/deployment/puppet/openstack/spec/classes/openstack_ha_radosgw_spec.rb +++ b/deployment/puppet/openstack/spec/classes/openstack_ha_radosgw_spec.rb @@ -1,18 +1,32 @@ require 'spec_helper' describe 'openstack::ha::radosgw' do - let(:params) { {:internal_virtual_ip => '127.0.0.1', - :ipaddresses => ['127.0.0.2', '127.0.0.3'], - :public_virtual_ip => '192.168.0.1', - :baremetal_virtual_ip => '192.168.0.2', - :server_names => ['node-1', 'node-2'], - :public_ssl => true, - :public_ssl_path => '/var/lib/fuel/haproxy/public_radosgw.pem', - } } - let(:facts) { {:kernel => 'Linux', - :concat_basedir => '/var/lib/puppet/concat', - :fqdn => 'some.host.tld' - } } + let :params do + { + :internal_virtual_ip => '127.0.0.1', + :ipaddresses => ['127.0.0.2', '127.0.0.3'], + :public_virtual_ip => '192.168.0.1', + :baremetal_virtual_ip => '192.168.0.2', + :server_names => ['node-1', 'node-2'], + :public_ssl => true, + :public_ssl_path => '/var/lib/fuel/haproxy/public_radosgw.pem', + } + end + + let :facts do + { + :kernel => 'Linux', + :concat_basedir => '/var/lib/puppet/concat', + :fqdn => 'some.host.tld' + } + end + + let :haproxy_config_opts do + { + 'option' => ['httplog', 'httpchk HEAD /', 'http-server-close', 'forwardfor'], + 'http-request' => 'set-header X-Forwarded-Proto https if { ssl_fc }', + } + end it "should properly configure radosgw haproxy based on ssl" do should contain_openstack__ha__haproxy_service('radosgw').with( @@ -22,10 +36,7 @@ require 'spec_helper' 'public' => true, 'public_ssl' => true, 'public_ssl_path' => '/var/lib/fuel/haproxy/public_radosgw.pem', - 'haproxy_config_options' => { - 'option' => ['httplog', 'httpchk GET /'], - 'http-request' => 'set-header X-Forwarded-Proto https if { ssl_fc }', - }, + 'haproxy_config_options' => haproxy_config_opts, ) end @@ -36,10 +47,7 @@ require 'spec_helper' 'balancermember_port' => 6780, 'public_virtual_ip' => false, 'internal_virtual_ip' => '192.168.0.2', - 'haproxy_config_options' => { - 'option' => ['httplog', 'httpchk GET /'], - 'http-request' => 'set-header X-Forwarded-Proto https if { ssl_fc }', - }, + 'haproxy_config_options' => haproxy_config_opts, ) end end diff --git a/deployment/puppet/osnailyfacter/.fixtures.yml b/deployment/puppet/osnailyfacter/.fixtures.yml index 90d6e70e39..0e96fdaed4 100644 --- a/deployment/puppet/osnailyfacter/.fixtures.yml +++ b/deployment/puppet/osnailyfacter/.fixtures.yml @@ -1,6 +1,12 @@ fixtures: repositories: 'stdlib': 'git://github.com/puppetlabs/puppetlabs-stdlib.git' + 'apache': + repo: 'https://review.fuel-infra.org/p/puppet-modules/puppetlabs-apache.git' + branch: '1.6.0' + 'concat': + repo: 'https://review.fuel-infra.org/p/puppet-modules/puppetlabs-concat.git' + branch: '1.2.3' symlinks: 'osnailyfacter': "#{source_dir}" 'l23network': "#{source_dir}/../l23network" diff --git a/deployment/puppet/osnailyfacter/manifests/apache.pp b/deployment/puppet/osnailyfacter/manifests/apache.pp index 79da1680c7..e7ae2728fd 100644 --- a/deployment/puppet/osnailyfacter/manifests/apache.pp +++ b/deployment/puppet/osnailyfacter/manifests/apache.pp @@ -18,12 +18,24 @@ # (optional) The number of times to be rotated before being removed. # Defaults to '52' # +# [*log_formats*] +# (optional) Hash w/ additional `LogFormat` directives. +# Defaults to {} +# class osnailyfacter::apache ( $purge_configs = false, $listen_ports = '80', $logrotate_rotate = '52', + $log_formats = {}, ) { + # define forwarded log format + $log_format_forwarded = { + 'forwarded' => '%{X-Forwarded-For}i %l %u %t \"%r\" %s %b \"%{Referer}i\" \"%{User-agent}i\"' + } + + $log_formats_mixed = merge($log_format_forwarded, $log_formats) + class { '::apache': mpm_module => false, default_vhost => false, @@ -32,17 +44,21 @@ class osnailyfacter::apache ( server_tokens => 'Prod', server_signature => 'Off', trace_enable => 'Off', + log_formats => $log_formats_mixed, } apache::listen { $listen_ports: } + File { + ensure => 'file', + owner => 'root', + group => 'root', + } + # we need to override the logrotate file provided by apache to work around # wsgi issues on the restart caused by logrotate. # LP#1491576 and https://github.com/GrahamDumpleton/mod_wsgi/issues/81 file { '/etc/logrotate.d/apache2': - ensure => 'file', - owner => 'root', - group => 'root', mode => '0644', content => template('osnailyfacter/apache2.logrotate.erb'), require => Package['httpd'] @@ -58,15 +74,10 @@ class osnailyfacter::apache ( file { '/etc/logrotate.d/httpd-prerotate': ensure => 'directory', - owner => 'root', - group => 'root', mode => '0755', } file { '/etc/logrotate.d/httpd-prerotate/apache2': - ensure => 'file', - owner => 'root', - group => 'root', mode => '0755', content => template('osnailyfacter/apache2.prerotate.erb'), } diff --git a/deployment/puppet/osnailyfacter/spec/classes/osnailyfacter_apache_spec.rb b/deployment/puppet/osnailyfacter/spec/classes/osnailyfacter_apache_spec.rb new file mode 100644 index 0000000000..999f684e55 --- /dev/null +++ b/deployment/puppet/osnailyfacter/spec/classes/osnailyfacter_apache_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe 'osnailyfacter::apache' do + let :facts do + { + :osfamily => 'Debian', + :operatingsystem => 'Ubuntu', + :operatingsystemrelease => '14.04', + :concat_basedir => '/var/lib/puppet/concat' + } + end + + let :params do + { + :log_formats => { + 'forwarded' => '%{X-Forwarded-For}i %l %u %t \"%r\" %s %b \"%{Referer}i\" \"%{User-agent}i\"' + } + } + end + + let :file_default_opts do + { + :ensure => 'file', + :owner => 'root', + :group => 'root', + :mode => '0755', + } + end + + it 'should configure apache to listen default 80 port' do + is_expected.to contain_apache__listen('80') + end + + it 'should have apache class' do + is_expected.to contain_class('apache').with( + :server_tokens => 'Prod', + :server_signature => 'Off', + :trace_enable => 'Off', + :log_formats => params[:log_formats], + ) + end + + it 'should have logrotate apache config' do + is_expected.to contain_file('/etc/logrotate.d/apache2').with( + file_default_opts.merge( + :mode => '0644', + :require => 'Package[httpd]', + ) + ) + end + + it 'should have a httpd prerotate folder' do + is_expected.to contain_file('/etc/logrotate.d/httpd-prerotate').with( + file_default_opts.merge( + :ensure => 'directory', + ) + ) + end + + it 'should have a httpd prerotate config' do + is_expected.to contain_file('/etc/logrotate.d/httpd-prerotate/apache2').with( + file_default_opts + ) + end +end + diff --git a/tests/noop/spec/hosts/apache/apache_spec.rb b/tests/noop/spec/hosts/apache/apache_spec.rb index 4deb2b017d..20b521369e 100644 --- a/tests/noop/spec/hosts/apache/apache_spec.rb +++ b/tests/noop/spec/hosts/apache/apache_spec.rb @@ -4,6 +4,13 @@ manifest = 'apache/apache.pp' describe manifest do shared_examples 'catalog' do + it 'should have osnailyfacter::apache class' do + should contain_class('osnailyfacter::apache').with( + :purge_configs => false, + :listen_ports => Noop.hiera_array('apache_ports', ['0.0.0.0:80']), + ) + end + it 'should execute apache class with given parameters' do should contain_class('apache').with( 'mpm_module' => 'false',