From b643c5f620fbf5aa3f9d398cfcd4f3e69ef6008f Mon Sep 17 00:00:00 2001 From: Lance Albertson Date: Tue, 14 Jan 2020 16:43:53 -0800 Subject: [PATCH] Update to apache2 ~> 8.0 cookbook This brings us up to date with the latest apache2 cookbook which included a major refactor in 6.0.0 removing all of the definitions and recipe with proper resources. Instead of using the apache2_default_site resource, directly use a template and then enable the config file using the apache2_site resource. This gives us the most flexibility. Additional fixes: - Install mod_wsgi as a package on RHEL since there is no built-in resource for it. - Convert web_app to template and subscribe to restarting apache. - Remove erroneous restart for ironic-api service on packages - Properly set service password and auth URL - Improve tests for ironic.conf - Add missing apache2 depend - Add missing api RHEL ChefSpec tests - Include additional cookbooks in Berksfile required for CI Depends-On: https://review.opendev.org/702772 Depends-On: https://review.opendev.org/701824 Change-Id: I198e2c211630e190bf2a992b3dc6b6c5afaf54e8 --- Berksfile | 4 + README.rst | 1 + attributes/default.rb | 2 +- metadata.rb | 1 + recipes/api.rb | 69 ++++++++---- recipes/ironic-common.rb | 10 ++ spec/api-rhel_spec.rb | 23 ++++ spec/api_spec.rb | 131 ++++++++++++++++++++++- spec/ironic-common_spec.rb | 33 ++++++ spec/spec_helper.rb | 32 ++++++ templates/default/wsgi-template.conf.erb | 40 +++---- 11 files changed, 296 insertions(+), 50 deletions(-) create mode 100644 spec/api-rhel_spec.rb diff --git a/Berksfile b/Berksfile index f08f6b3..0a71871 100644 --- a/Berksfile +++ b/Berksfile @@ -5,9 +5,13 @@ solver :ruby, :required %w( client -common + -dns -identity -image + -integration-test -network + -ops-database + -ops-messaging ).each do |cookbook| if Dir.exist?("../cookbook-openstack#{cookbook}") cookbook "openstack#{cookbook}", path: "../cookbook-openstack#{cookbook}" diff --git a/README.rst b/README.rst index d000452..52cfffd 100644 --- a/README.rst +++ b/README.rst @@ -36,6 +36,7 @@ Cookbooks The following cookbooks are dependencies: +- 'apache2', '~> 8.0' - 'openstack-common', '>= 18.0.0' - 'openstack-identity', '>= 18.0.0' - 'openstack-image', '>= 18.0.0' diff --git a/attributes/default.rb b/attributes/default.rb index c117a8c..551885a 100644 --- a/attributes/default.rb +++ b/attributes/default.rb @@ -104,7 +104,7 @@ default['openstack']['bare_metal']['ssl']['ciphers'] = '' case node['platform_family'] when 'fedora', 'rhel' default['openstack']['bare_metal']['platform'] = { - 'ironic_api_packages' => ['openstack-ironic-api'], + 'ironic_api_packages' => ['openstack-ironic-api', 'mod_wsgi'], 'ironic_api_service' => 'openstack-ironic-api', 'ironic_conductor_packages' => ['openstack-ironic-conductor', 'ipmitool'], 'ironic_conductor_service' => 'openstack-ironic-conductor', diff --git a/metadata.rb b/metadata.rb index f98a8d5..be5a686 100644 --- a/metadata.rb +++ b/metadata.rb @@ -18,6 +18,7 @@ recipe 'openstack-bare-metal::ironic-common', 'Defines the common pieces of repe supports os end +depends 'apache2', '~> 8.0' depends 'openstack-common', '>= 18.0.0' depends 'openstack-identity', '>= 18.0.0' depends 'openstack-image', '>= 18.0.0' diff --git a/recipes/api.rb b/recipes/api.rb index 0af32ac..e4813c8 100644 --- a/recipes/api.rb +++ b/recipes/api.rb @@ -20,6 +20,7 @@ class ::Chef::Recipe include ::Openstack + include Apache2::Cookbook::Helpers end include_recipe 'openstack-bare-metal::ironic-common' @@ -29,8 +30,6 @@ platform_options = node['openstack']['bare_metal']['platform'] platform_options['ironic_api_packages'].each do |pkg| package pkg do action :upgrade - - notifies :restart, 'service[ironic-api]', :delayed end end @@ -43,32 +42,56 @@ execute 'ironic db sync' do command 'ironic-dbsync --config-file /etc/ironic/ironic.conf upgrade' user 'root' group 'root' - action :run end # remove the ironic-wsgi.conf automatically generated from package -apache_config 'ironic-wsgi' do - enable false +apache2_conf 'ironic-wsgi' do + action :disable end bind_service = node['openstack']['bind_service']['all']['bare_metal'] -web_app 'ironic-api' do - template 'wsgi-template.conf.erb' - daemon_process 'ironic-wsgi' - server_host bind_service['host'] - server_port bind_service['port'] - server_entry '/usr/bin/ironic-api-wsgi' - log_dir node['apache']['log_dir'] - run_dir node['apache']['run_dir'] - user node['openstack']['bare_metal']['user'] - group node['openstack']['bare_metal']['group'] - use_ssl node['openstack']['bare_metal']['ssl']['enabled'] - cert_file node['openstack']['bare_metal']['ssl']['certfile'] - chain_file node['openstack']['bare_metal']['ssl']['chainfile'] - key_file node['openstack']['bare_metal']['ssl']['keyfile'] - ca_certs_path node['openstack']['bare_metal']['ssl']['ca_certs_path'] - cert_required node['openstack']['bare_metal']['ssl']['cert_required'] - protocol node['openstack']['bare_metal']['ssl']['protocol'] - ciphers node['openstack']['bare_metal']['ssl']['ciphers'] +# Finds and appends the listen port to the apache2_install[openstack] +# resource which is defined in openstack-identity::server-apache. +apache_resource = find_resource(:apache2_install, 'openstack') + +if apache_resource + apache_resource.listen = [apache_resource.listen, "#{bind_service['host']}:#{bind_service['port']}"].flatten +else + apache2_install 'openstack' do + listen "#{bind_service['host']}:#{bind_service['port']}" + end +end + +# service['apache2'] is defined in the apache2_default_install resource +# but other resources are currently unable to reference it. To work +# around this issue, define the following helper in your cookbook: +service 'apache2' do + extend Apache2::Cookbook::Helpers + service_name lazy { apache_platform_service_name } + supports restart: true, status: true, reload: true + action :nothing +end + +apache2_module 'wsgi' +apache2_module 'ssl' if node['openstack']['bare_metal']['ssl']['enabled'] + +template "#{apache_dir}/sites-available/ironic-api.conf" do + extend Apache2::Cookbook::Helpers + source 'wsgi-template.conf.erb' + variables( + daemon_process: 'ironic-wsgi', + server_host: bind_service['host'], + server_port: bind_service['port'], + server_entry: '/usr/bin/ironic-api-wsgi', + log_dir: default_log_dir, + run_dir: lock_dir, + user: node['openstack']['bare_metal']['user'], + group: node['openstack']['bare_metal']['group'] + ) + notifies :restart, 'service[apache2]' +end + +apache2_site 'ironic-api' do + notifies :restart, 'service[apache2]', :immediately end diff --git a/recipes/ironic-common.rb b/recipes/ironic-common.rb index 1cde845..4428038 100644 --- a/recipes/ironic-common.rb +++ b/recipes/ironic-common.rb @@ -57,6 +57,16 @@ if node['openstack']['mq']['service_type'] == 'rabbit' node.default['openstack']['bare_metal']['conf_secrets']['DEFAULT']['transport_url'] = rabbit_transport_url 'bare_metal' end +identity_endpoint = internal_endpoint 'identity' +node.default['openstack']['bare_metal']['conf_secrets'] + .[]('keystone_authtoken')['password'] = + get_password 'service', 'openstack-bare-metal' +auth_url = ::URI.decode identity_endpoint.to_s + +node.default['openstack']['bare_metal']['conf'].tap do |conf| + conf['keystone_authtoken']['auth_url'] = auth_url +end + # merge all config options and secrets to be used in ironic.conf ironic_conf_options = merge_config_options 'bare_metal' diff --git a/spec/api-rhel_spec.rb b/spec/api-rhel_spec.rb new file mode 100644 index 0000000..38f6c15 --- /dev/null +++ b/spec/api-rhel_spec.rb @@ -0,0 +1,23 @@ +# Encoding: utf-8 + +require_relative 'spec_helper' + +describe 'openstack-bare-metal::api' do + describe 'redhat' do + let(:runner) { ChefSpec::SoloRunner.new(REDHAT_OPTS) } + let(:node) { runner.node } + cached(:chef_run) { runner.converge(described_recipe) } + + include_context 'bare-metal-stubs' + + it do + expect(chef_run).to upgrade_package('openstack-ironic-api') + expect(chef_run).to upgrade_package('mod_wsgi') + end + + it do + expect(chef_run).to disable_service('ironic-api').with(service_name: 'openstack-ironic-api') + expect(chef_run).to stop_service('ironic-api').with(service_name: 'openstack-ironic-api') + end + end +end diff --git a/spec/api_spec.rb b/spec/api_spec.rb index 737c43b..fc8d76b 100644 --- a/spec/api_spec.rb +++ b/spec/api_spec.rb @@ -32,16 +32,141 @@ describe 'openstack-bare-metal::api' do expect(chef_run).to include_recipe('openstack-bare-metal::ironic-common') end - it 'upgrades ironic api packages' do + it do expect(chef_run).to upgrade_package('ironic-api') end - it 'disables ironic api on boot' do - expect(chef_run).to disable_service('ironic-api') + it do + expect(chef_run).to disable_service('ironic-api').with(service_name: 'ironic-api') + expect(chef_run).to stop_service('ironic-api').with(service_name: 'ironic-api') end it 'runs db migrations' do expect(chef_run).to run_execute('ironic db sync').with(user: 'root', group: 'root') end + + it do + expect(chef_run).to install_apache2_install('openstack').with(listen: '127.0.0.1:6385') + end + + it do + expect(chef_run).to enable_apache2_module('wsgi') + end + + it do + expect(chef_run).to_not enable_apache2_module('ssl') + end + + it do + expect(chef_run).to create_template('/etc/apache2/sites-available/ironic-api.conf').with( + source: 'wsgi-template.conf.erb', + variables: { + daemon_process: 'ironic-wsgi', + group: 'ironic', + log_dir: '/var/log/apache2', + run_dir: '/var/lock/apache2', + server_entry: '/usr/bin/ironic-api-wsgi', + server_host: '127.0.0.1', + server_port: '6385', + user: 'ironic', + } + ) + end + [ + /$/, + /WSGIDaemonProcess ironic-wsgi processes=2 threads=10 user=ironic group=ironic display-name=%{GROUP}$/, + /WSGIProcessGroup ironic-wsgi$/, + %r{WSGIScriptAlias / /usr/bin/ironic-api-wsgi$}, + /WSGIApplicationGroup %{GLOBAL}$/, + %r{ErrorLog /var/log/apache2/ironic-wsgi_error.log$}, + %r{CustomLog /var/log/apache2/ironic-wsgi_access.log combined$}, + %r{WSGISocketPrefix /var/lock/apache2$}, + ].each do |line| + it do + expect(chef_run).to render_file('/etc/apache2/sites-available/ironic-api.conf').with_content(line) + end + end + + [ + /SSLEngine On$/, + /SSLCertificateFile/, + /SSLCertificateKeyFile/, + /SSLCACertificatePath/, + /SSLCertificateChainFile/, + /SSLProtocol/, + /SSLCipherSuite/, + /SSLVerifyClient require/, + ].each do |line| + it do + expect(chef_run).to_not render_file('/etc/apache2/sites-available/ironic-api.conf').with_content(line) + end + end + + context 'Enable SSL' do + cached(:chef_run) do + node.override['openstack']['bare_metal']['ssl']['enabled'] = true + node.override['openstack']['bare_metal']['ssl']['certfile'] = 'ssl.cert' + node.override['openstack']['bare_metal']['ssl']['keyfile'] = 'ssl.key' + node.override['openstack']['bare_metal']['ssl']['ca_certs_path'] = 'ca_certs_path' + node.override['openstack']['bare_metal']['ssl']['protocol'] = 'ssl_protocol_value' + runner.converge(described_recipe) + end + + it do + expect(chef_run).to enable_apache2_module('ssl') + end + + [ + /SSLEngine On$/, + /SSLCertificateFile ssl.cert$/, + /SSLCertificateKeyFile ssl.key$/, + /SSLCACertificatePath ca_certs_path$/, + /SSLProtocol ssl_protocol_value$/, + ].each do |line| + it do + expect(chef_run).to render_file('/etc/apache2/sites-available/ironic-api.conf').with_content(line) + end + end + [ + /SSLCipherSuite/, + /SSLCertificateChainFile/, + /SSLVerifyClient require/, + ].each do |line| + it do + expect(chef_run).to_not render_file('/etc/apache2/sites-available/ironic-api.conf').with_content(line) + end + end + context 'Enable chainfile, ciphers & cert_required' do + cached(:chef_run) do + node.override['openstack']['bare_metal']['ssl']['enabled'] = true + node.override['openstack']['bare_metal']['ssl']['ciphers'] = 'ssl_ciphers_value' + node.override['openstack']['bare_metal']['ssl']['chainfile'] = 'chainfile' + node.override['openstack']['bare_metal']['ssl']['cert_required'] = true + runner.converge(described_recipe) + end + [ + /SSLCipherSuite ssl_ciphers_value$/, + /SSLCertificateChainFile chainfile$/, + /SSLVerifyClient require/, + ].each do |line| + it do + expect(chef_run).to render_file('/etc/apache2/sites-available/ironic-api.conf').with_content(line) + end + end + end + end + + it do + expect(chef_run.template('/etc/apache2/sites-available/ironic-api.conf')).to \ + notify('service[apache2]').to(:restart) + end + + it do + expect(chef_run).to enable_apache2_site('ironic-api') + end + + it do + expect(chef_run.apache2_site('ironic-api')).to notify('service[apache2]').to(:restart).immediately + end end end diff --git a/spec/ironic-common_spec.rb b/spec/ironic-common_spec.rb index fc52cf0..ed49356 100644 --- a/spec/ironic-common_spec.rb +++ b/spec/ironic-common_spec.rb @@ -64,6 +64,39 @@ describe 'openstack-bare-metal::ironic-common' do ) end + [ + /^auth_strategy = keystone$/, + /^control_exchange = ironic$/, + /^glance_api_version = 2$/, + %r{^state_path = /var/lib/ironic$}, + %r{^transport_url = rabbit://guest:mypass@127.0.0.1:5672$}, + ].each do |line| + it do + expect(chef_run).to render_config_file(file.name).with_section_content('DEFAULT', line) + end + end + [ + /^auth_type = password$/, + /^region_name = RegionOne$/, + /^username = ironic$/, + /^project_name = service$/, + /^user_domain_name = Default$/, + /^project_domain_name = Default$/, + %r{^auth_url = http://127.0.0.1:5000/v3$}, + /^password = ironic_pass$/, + ].each do |line| + it do + expect(chef_run).to render_config_file(file.name).with_section_content('keystone_authtoken', line) + end + end + [ + %r{^lock_path = /var/lib/cinder/tmp$}, + ].each do |line| + it do + expect(chef_run).to render_config_file(file.name).with_section_content('oslo_concurrency', line) + end + end + context 'template contents' do cached(:chef_run) do node.override['openstack']['bare_metal']['syslog']['use'] = true diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 995b764..2fe4451 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -45,6 +45,38 @@ shared_context 'bare-metal-stubs' do stub_command('/usr/sbin/apache2 -t').and_return(true) allow_any_instance_of(Chef::Recipe).to receive(:memcached_servers).and_return [] allow(Chef::Application).to receive(:fatal!) + # identity stubs + allow_any_instance_of(Chef::Recipe).to receive(:secret) + .with('secrets', 'credential_key0') + .and_return('thisiscredentialkey0') + allow_any_instance_of(Chef::Recipe).to receive(:secret) + .with('secrets', 'credential_key1') + .and_return('thisiscredentialkey1') + allow_any_instance_of(Chef::Recipe).to receive(:secret) + .with('secrets', 'fernet_key0') + .and_return('thisisfernetkey0') + allow_any_instance_of(Chef::Recipe).to receive(:secret) + .with('secrets', 'fernet_key1') + .and_return('thisisfernetkey1') + allow_any_instance_of(Chef::Recipe).to receive(:search_for) + .with('os-identity').and_return( + [{ + 'openstack' => { + 'identity' => { + 'admin_tenant_name' => 'admin', + 'admin_user' => 'admin', + }, + }, + }] + ) + allow_any_instance_of(Chef::Recipe).to receive(:memcached_servers) + .and_return([]) + allow_any_instance_of(Chef::Recipe).to receive(:rabbit_transport_url) + .with('identity') + .and_return('rabbit://openstack:mypass@127.0.0.1:5672') + allow_any_instance_of(Chef::Recipe).to receive(:get_password) + .with('db', 'keystone') + .and_return('test-passes') end end diff --git a/templates/default/wsgi-template.conf.erb b/templates/default/wsgi-template.conf.erb index 82bd10c..3726877 100644 --- a/templates/default/wsgi-template.conf.erb +++ b/templates/default/wsgi-template.conf.erb @@ -1,11 +1,9 @@ <%= node['openstack']['bare_metal']['custom_template_banner'] %> -Listen <%= @params[:server_host] %>:<%= @params[:server_port] %> - -:<%= @params[:server_port] %>> - WSGIDaemonProcess <%= @params[:daemon_process] %> processes=2 threads=10 user=<%= @params[:user] %> group=<%= @params[:group] %> display-name=%{GROUP} - WSGIProcessGroup <%= @params[:daemon_process] %> - WSGIScriptAlias / <%= @params[:server_entry] %> +:<%= @server_port %>> + WSGIDaemonProcess <%= @daemon_process %> processes=2 threads=10 user=<%= @user %> group=<%= @group %> display-name=%{GROUP} + WSGIProcessGroup <%= @daemon_process %> + WSGIScriptAlias / <%= @server_entry %> WSGIApplicationGroup %{GLOBAL} WSGIPassAuthorization On @@ -14,29 +12,25 @@ Listen <%= @params[:server_host] %>:<%= @params[:server_port] %> ErrorLogFormat "%{cu}t %M" - ErrorLog <%= @params[:log_dir] %>/<%= @params[:daemon_process] %>_error.log - CustomLog <%= @params[:log_dir] %>/<%= @params[:daemon_process] %>_access.log combined -<% if [true, 'true', 'True'].include?(@params[:log_debug]) -%> - LogLevel debug -<% end -%> + ErrorLog <%= @log_dir %>/<%= @daemon_process %>_error.log + CustomLog <%= @log_dir %>/<%= @daemon_process %>_access.log combined +<% if node['openstack']['bare_metal']['ssl']['enabled'] -%> -<% if @params[:use_ssl] -%> SSLEngine On - SSLCertificateFile <%= @params[:cert_file] %> - SSLCertificateKeyFile <%= @params[:key_file] %> - SSLCACertificatePath <%= @params[:ca_certs_path] %> -<% if @params[:chain_file] %> - SSLCertificateChainFile <%= @params[:chain_file] %> + SSLCertificateFile <%= node['openstack']['bare_metal']['ssl']['certfile'] %> + SSLCertificateKeyFile <%= node['openstack']['bare_metal']['ssl']['keyfile'] %> + SSLCACertificatePath <%= node['openstack']['bare_metal']['ssl']['ca_certs_path'] %> +<% unless node['openstack']['bare_metal']['ssl']['chainfile'].empty? %> + SSLCertificateChainFile <%= node['openstack']['bare_metal']['ssl']['chainfile'] %> <% end -%> - SSLProtocol <%= @params[:protocol] %> -<% if @params[:ciphers] -%> - SSLCipherSuite <%= @params[:ciphers] %> + SSLProtocol <%= node['openstack']['bare_metal']['ssl']['protocol'] %> +<% unless node['openstack']['bare_metal']['ssl']['ciphers'].empty? -%> + SSLCipherSuite <%= node['openstack']['bare_metal']['ssl']['ciphers'] %> <% end -%> -<% if @params[:cert_required] -%> +<% if node['openstack']['bare_metal']['ssl']['cert_required'] -%> SSLVerifyClient require <% end -%> <% end -%> -WSGISocketPrefix <%= @params[:run_dir] -%> - +WSGISocketPrefix <%= @run_dir -%>