From 0d06d2ee48ed6bc5d9e8696a053403d27e5218d0 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Tue, 6 Apr 2021 17:28:16 +0200 Subject: [PATCH 1/2] Fixes #29649 - Drop default_server argument in IPA ipa-getkeytab can figure out the default server on its own[1]. There is no need to specify it and can even break things. For example, DNS can be used to detect servers. Then the fact is empty and it fails while the command would actually pass. The foreman_ipa fact is removed since it's a major version bump anyway and nothing else should use our foreman_ipa fact. [1] https://github.com/theforeman/puppet-foreman/pull/880#issuecomment-683902223 --- lib/facter/sssd.rb | 13 ------------- lib/facter/util/sssd.rb | 4 ---- manifests/config.pp | 8 ++------ spec/classes/foreman_config_ipa_spec.rb | 14 -------------- 4 files changed, 2 insertions(+), 37 deletions(-) diff --git a/lib/facter/sssd.rb b/lib/facter/sssd.rb index 48a6903c8..52c02e4c8 100644 --- a/lib/facter/sssd.rb +++ b/lib/facter/sssd.rb @@ -1,19 +1,6 @@ require 'facter/util/sssd' if defined? Facter::Util::Sssd - # == Fact: foreman_ipa - Facter.add(:foreman_ipa, :type => :aggregate) do - { - :default_realm => 'global/realm', - :default_server => 'global/server', - }.each do |key, path| - chunk(key) do - val = Facter::Util::Sssd.ipa_value(path) - {key => val} if val - end - end - end - # == Fact: foreman_sssd Facter.add(:foreman_sssd, :type => :aggregate) do { diff --git a/lib/facter/util/sssd.rb b/lib/facter/util/sssd.rb index 6f9a7df26..fd3f89e88 100644 --- a/lib/facter/util/sssd.rb +++ b/lib/facter/util/sssd.rb @@ -11,10 +11,6 @@ def self.aug_value(lens, file, path) end end - def self.ipa_value(path) - aug_value('Puppet.lns', '/etc/ipa/default.conf', path) - end - def self.sssd_value(path) val = aug_value('Sssd.lns', '/etc/sssd/sssd.conf', path) val.split(',').map(&:strip) if val diff --git a/manifests/config.pp b/manifests/config.pp index 7edee7f61..2df77fb19 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -126,10 +126,6 @@ $foreman_socket_override = template('foreman/foreman.socket-overrides.erb') if $foreman::ipa_authentication { - unless fact('foreman_ipa.default_server') { - fail("${facts['networking']['hostname']}: The system does not seem to be IPA-enrolled") - } - if $facts['os']['selinux']['enabled'] { selboolean { ['allow_httpd_mod_auth_pam', 'httpd_dbus_sssd']: persistent => true, @@ -158,7 +154,7 @@ exec { 'ipa-getkeytab': command => "/bin/echo Get keytab \ && KRB5CCNAME=KEYRING:session:get-http-service-keytab kinit -k \ - && KRB5CCNAME=KEYRING:session:get-http-service-keytab /usr/sbin/ipa-getkeytab -s ${facts['foreman_ipa']['default_server']} -k ${http_keytab} -p HTTP/${facts['networking']['fqdn']} \ + && KRB5CCNAME=KEYRING:session:get-http-service-keytab /usr/sbin/ipa-getkeytab -k ${http_keytab} -p HTTP/${facts['networking']['fqdn']} \ && kdestroy -c KEYRING:session:get-http-service-keytab", creates => $http_keytab, } @@ -182,7 +178,7 @@ if $foreman::ipa_manage_sssd { - $sssd = $facts['foreman_sssd'] + $sssd = pick(fact('foreman_sssd'), {}) $sssd_services = join(unique(pick($sssd['services'], []) + ['ifp']), ', ') $sssd_ldap_user_extra_attrs = join(unique(pick($sssd['ldap_user_extra_attrs'], []) + ['email:mail', 'lastname:sn', 'firstname:givenname']), ', ') $sssd_allowed_uids = join(unique(pick($sssd['allowed_uids'], []) + [$apache::user, 'root']), ', ') diff --git a/spec/classes/foreman_config_ipa_spec.rb b/spec/classes/foreman_config_ipa_spec.rb index 2bb2724f3..f864375e4 100644 --- a/spec/classes/foreman_config_ipa_spec.rb +++ b/spec/classes/foreman_config_ipa_spec.rb @@ -16,23 +16,9 @@ context 'with apache' do let(:params) { super().merge(apache: true) } - describe 'not IPA-enrolled system' do - describe 'ipa_server fact missing' do - it { should raise_error(Puppet::Error, /The system does not seem to be IPA-enrolled/) } - end - - describe 'default_ipa_realm fact missing' do - it { should raise_error(Puppet::Error, /The system does not seem to be IPA-enrolled/) } - end - end - describe 'enrolled system' do let(:facts) do super().merge( - foreman_ipa: { - default_server: 'ipa.example.com', - default_realm: 'REALM' - }, foreman_sssd: { services: ['ifp'] } From 8d9a6f160298e8ee62b7bde56d87bc994ec477d6 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Sat, 10 Oct 2020 15:14:13 +0200 Subject: [PATCH 2/2] Factor out Apache to its own class Prior to this, when the Apache config was modified a full database refresh was triggered. There's no need for that and this makes applying those changes faster. --- manifests/apache.pp | 82 ++++++++++++++++++++ manifests/config.pp | 102 +------------------------ manifests/init.pp | 24 +++++- manifests/service.pp | 10 --- templates/foreman.socket-overrides.erb | 2 +- 5 files changed, 105 insertions(+), 115 deletions(-) create mode 100644 manifests/apache.pp diff --git a/manifests/apache.pp b/manifests/apache.pp new file mode 100644 index 000000000..ef4a821e6 --- /dev/null +++ b/manifests/apache.pp @@ -0,0 +1,82 @@ +# @summary The apache configuration for Foreman +# @api private +class foreman::apache { + class { 'foreman::config::apache': + app_root => $foreman::app_root, + priority => $foreman::vhost_priority, + servername => $foreman::servername, + serveraliases => $foreman::serveraliases, + server_port => $foreman::server_port, + server_ssl_port => $foreman::server_ssl_port, + proxy_backend => "unix://${foreman::listen_socket}", + ssl => $foreman::ssl, + ssl_ca => $foreman::server_ssl_ca, + ssl_chain => $foreman::server_ssl_chain, + ssl_cert => $foreman::server_ssl_cert, + ssl_certs_dir => $foreman::server_ssl_certs_dir, + ssl_key => $foreman::server_ssl_key, + ssl_crl => $foreman::server_ssl_crl, + ssl_protocol => $foreman::server_ssl_protocol, + ssl_verify_client => $foreman::server_ssl_verify_client, + user => $foreman::user, + foreman_url => $foreman::foreman_url, + ipa_authentication => $foreman::ipa_authentication, + keycloak => $foreman::keycloak, + keycloak_app_name => $foreman::keycloak_app_name, + keycloak_realm => $foreman::keycloak_realm, + } + + contain foreman::config::apache + + if $foreman::ipa_authentication { + if $facts['os']['selinux']['enabled'] { + selboolean { ['allow_httpd_mod_auth_pam', 'httpd_dbus_sssd']: + persistent => true, + value => 'on', + } + } + + if $foreman::ipa_manage_sssd { + service { 'sssd': + ensure => running, + enable => true, + require => Package['sssd-dbus'], + } + } + + file { "/etc/pam.d/${foreman::pam_service}": + ensure => file, + owner => root, + group => root, + mode => '0644', + content => template('foreman/pam_service.erb'), + } + + $http_keytab = pick($foreman::http_keytab, "${apache::conf_dir}/http.keytab") + + exec { 'ipa-getkeytab': + command => "/bin/echo Get keytab \ + && KRB5CCNAME=KEYRING:session:get-http-service-keytab kinit -k \ + && KRB5CCNAME=KEYRING:session:get-http-service-keytab /usr/sbin/ipa-getkeytab -k ${http_keytab} -p HTTP/${facts['networking']['fqdn']} \ + && kdestroy -c KEYRING:session:get-http-service-keytab", + creates => $http_keytab, + } + -> file { $http_keytab: + ensure => file, + owner => $apache::user, + mode => '0600', + } + + foreman::config::apache::fragment { 'intercept_form_submit': + ssl_content => template('foreman/intercept_form_submit.conf.erb'), + } + + foreman::config::apache::fragment { 'lookup_identity': + ssl_content => template('foreman/lookup_identity.conf.erb'), + } + + foreman::config::apache::fragment { 'auth_gssapi': + ssl_content => template('foreman/auth_gssapi.conf.erb'), + } + } +} diff --git a/manifests/config.pp b/manifests/config.pp index 2df77fb19..f2e4a5c0c 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -94,110 +94,10 @@ } if $foreman::apache { - $listen_socket = '/run/foreman.sock' - - class { 'foreman::config::apache': - app_root => $foreman::app_root, - priority => $foreman::vhost_priority, - servername => $foreman::servername, - serveraliases => $foreman::serveraliases, - server_port => $foreman::server_port, - server_ssl_port => $foreman::server_ssl_port, - proxy_backend => "unix://${listen_socket}", - ssl => $foreman::ssl, - ssl_ca => $foreman::server_ssl_ca, - ssl_chain => $foreman::server_ssl_chain, - ssl_cert => $foreman::server_ssl_cert, - ssl_certs_dir => $foreman::server_ssl_certs_dir, - ssl_key => $foreman::server_ssl_key, - ssl_crl => $foreman::server_ssl_crl, - ssl_protocol => $foreman::server_ssl_protocol, - ssl_verify_client => $foreman::server_ssl_verify_client, - user => $foreman::user, - foreman_url => $foreman::foreman_url, - ipa_authentication => $foreman::ipa_authentication, - keycloak => $foreman::keycloak, - keycloak_app_name => $foreman::keycloak_app_name, - keycloak_realm => $foreman::keycloak_realm, - } - - contain foreman::config::apache - $foreman_socket_override = template('foreman/foreman.socket-overrides.erb') if $foreman::ipa_authentication { - if $facts['os']['selinux']['enabled'] { - selboolean { ['allow_httpd_mod_auth_pam', 'httpd_dbus_sssd']: - persistent => true, - value => 'on', - } - } - - if $foreman::ipa_manage_sssd { - service { 'sssd': - ensure => running, - enable => true, - require => Package['sssd-dbus'], - } - } - - file { "/etc/pam.d/${foreman::pam_service}": - ensure => file, - owner => root, - group => root, - mode => '0644', - content => template('foreman/pam_service.erb'), - } - - $http_keytab = pick($foreman::http_keytab, "${apache::conf_dir}/http.keytab") - - exec { 'ipa-getkeytab': - command => "/bin/echo Get keytab \ - && KRB5CCNAME=KEYRING:session:get-http-service-keytab kinit -k \ - && KRB5CCNAME=KEYRING:session:get-http-service-keytab /usr/sbin/ipa-getkeytab -k ${http_keytab} -p HTTP/${facts['networking']['fqdn']} \ - && kdestroy -c KEYRING:session:get-http-service-keytab", - creates => $http_keytab, - } - -> file { $http_keytab: - ensure => file, - owner => $apache::user, - mode => '0600', - } - - foreman::config::apache::fragment { 'intercept_form_submit': - ssl_content => template('foreman/intercept_form_submit.conf.erb'), - } - - foreman::config::apache::fragment { 'lookup_identity': - ssl_content => template('foreman/lookup_identity.conf.erb'), - } - - foreman::config::apache::fragment { 'auth_gssapi': - ssl_content => template('foreman/auth_gssapi.conf.erb'), - } - - - if $foreman::ipa_manage_sssd { - $sssd = pick(fact('foreman_sssd'), {}) - $sssd_services = join(unique(pick($sssd['services'], []) + ['ifp']), ', ') - $sssd_ldap_user_extra_attrs = join(unique(pick($sssd['ldap_user_extra_attrs'], []) + ['email:mail', 'lastname:sn', 'firstname:givenname']), ', ') - $sssd_allowed_uids = join(unique(pick($sssd['allowed_uids'], []) + [$apache::user, 'root']), ', ') - $sssd_user_attributes = join(unique(pick($sssd['user_attributes'], []) + ['+email', '+firstname', '+lastname']), ', ') - - augeas { 'sssd-ifp-extra-attributes': - context => '/files/etc/sssd/sssd.conf', - changes => [ - "set target[.=~regexp('domain/.*')]/ldap_user_extra_attrs '${sssd_ldap_user_extra_attrs}'", - "set target[.='sssd']/services '${sssd_services}'", - 'set target[.=\'ifp\'] \'ifp\'', - "set target[.='ifp']/allowed_uids '${sssd_allowed_uids}'", - "set target[.='ifp']/user_attributes '${sssd_user_attributes}'", - ], - notify => Service['sssd'], - } - } - - concat::fragment {'foreman_settings+02-authorize_login_delegation.yaml': + concat::fragment { 'foreman_settings+02-authorize_login_delegation.yaml': target => '/etc/foreman/settings.yaml', content => template('foreman/settings-external-auth.yaml.erb'), order => '02', diff --git a/manifests/init.pp b/manifests/init.pp index d8c264aed..3462c1ef8 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -312,19 +312,28 @@ timeout => 0, } + $listen_socket = '/run/foreman.sock' + include foreman::install include foreman::config include foreman::database - contain foreman::service + include foreman::service + + anchor { 'foreman::running': # lint:ignore:anchor_resource + } Anchor <| title == 'foreman::repo' |> ~> Class['foreman::install'] Class['foreman::install'] ~> Class['foreman::config', 'foreman::service'] Class['foreman::config'] ~> Class['foreman::database', 'foreman::service'] Class['foreman::database'] ~> Class['foreman::service'] - Class['foreman::service'] -> Foreman_smartproxy <| base_url == $foreman_url |> + Class['foreman::service'] -> Anchor['foreman::running'] + Anchor['foreman::running'] -> Foreman_smartproxy <| base_url == $foreman_url |> if $apache { - Class['foreman::database'] -> Class['apache::service'] + include foreman::apache + + Class['foreman::config', 'foreman::database'] -> Class['foreman::apache'] + Class['foreman::apache', 'apache::service'] -> Anchor['foreman::running'] if $ipa_authentication and $keycloak { fail("${facts['networking']['hostname']}: External authentication via IPA and Keycloak are mutually exclusive.") } @@ -334,6 +343,15 @@ fail("${facts['networking']['hostname']}: External authentication via Keycloak can only be enabled when Apache is used.") } + # Ensure SSL certs from the puppetmaster are available + # Relationship is duplicated there as defined() is parse-order dependent + if $ssl and defined(Class['puppet::server::config']) { + Class['puppet::server::config'] -> Class['foreman::service'] + if $apache { + Class['puppet::server::config'] -> Class['foreman::apache'] + } + } + # Anchor these separately so as not to break # the notify between main classes Class['foreman::install'] diff --git a/manifests/service.pp b/manifests/service.pp index b1639c1cd..94ff206b4 100644 --- a/manifests/service.pp +++ b/manifests/service.pp @@ -25,16 +25,6 @@ } } - if $apache { - Class['apache::service'] -> Class['foreman::service'] - - # Ensure SSL certs from the puppetmaster are available - # Relationship is duplicated there as defined() is parse-order dependent - if $ssl and defined(Class['puppet::server::config']) { - Class['puppet::server::config'] -> Class['foreman::service'] - } - } - service { "${foreman_service}.socket": ensure => $foreman_service_ensure, enable => $foreman_service_enable, diff --git a/templates/foreman.socket-overrides.erb b/templates/foreman.socket-overrides.erb index 33d1f369f..01f7823cb 100644 --- a/templates/foreman.socket-overrides.erb +++ b/templates/foreman.socket-overrides.erb @@ -1,5 +1,5 @@ [Socket] ListenStream= -ListenStream=<%= @listen_socket %> +ListenStream=<%= scope['foreman::listen_socket'] %> SocketUser=<%= scope['apache::user'] %> SocketMode=0600