Fixes #29602 - Refactor repository handling

This removes the repo parameters from the main class, in favor of a
standalone class that can be included. It uses an anchor because that
can be collected in the main class to keep the correct dependency
chaining while using composition.

manifests/cli.pp

@@ -86,4 +86,6 @@
       content => template('foreman/hammer_root.yml.erb'),
     }
   }
+
+  Anchor <| title == 'foreman::repo' |> ~> Package <| tag == 'foreman::cli' |>
 }

manifests/init.pp

@@ -63,15 +63,6 @@
 #
 # $ssl::                          Enable and set require_ssl in Foreman settings (note: requires passenger, SSL does not apply to kickstarts)
 #
-# $repo::                         This can be a specific version or nightly
-#
-# $configure_epel_repo::          If disabled the EPEL repo will not be configured on Red Hat family systems.
-#
-# $configure_scl_repo::           If disabled the SCL repo will not be configured on Red Hat clone systems.
-#                                 (Currently only installs repos for CentOS and Scientific)
-#
-# $gpgcheck::                     Turn on/off gpg check in repo files (effective only on RedHat family systems)
-#
 # $version::                      Foreman package version, it's passed to ensure parameter of package resource
 #                                 can be set to specific version number, 'latest', 'present' etc.
 #
@@ -227,10 +218,6 @@
   Stdlib::Fqdn $servername = $foreman::params::servername,
   Array[Stdlib::Fqdn] $serveraliases = $foreman::params::serveraliases,
   Boolean $ssl = $foreman::params::ssl,
-  Optional[String] $repo = $foreman::params::repo,
-  Boolean $configure_epel_repo = $foreman::params::configure_epel_repo,
-  Boolean $configure_scl_repo = $foreman::params::configure_scl_repo,
-  Boolean $gpgcheck = $foreman::params::gpgcheck,
   String $version = $foreman::params::version,
   Enum['installed', 'present', 'latest'] $plugin_version = $foreman::params::plugin_version,
   Boolean $db_manage = $foreman::params::db_manage,
@@ -336,13 +323,12 @@
     $foreman_service_bind = '0.0.0.0'
   }
 
-  include foreman::repo
   include foreman::install
   include foreman::config
   include foreman::database
   contain foreman::service
 
-  Class['foreman::repo'] ~> Class['foreman::install']
+  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']
@@ -365,13 +351,8 @@
   ~> Package <| tag == 'foreman-compute' |>
   ~> Class['foreman::service']
 
-  Class['foreman::repo']
-  ~> Package <| tag == 'foreman::cli' |>
-  ~> Class['foreman']
-
-  Class['foreman::repo']
-  ~> Package <| tag == 'foreman::providers' |>
-  -> Class['foreman']
+  Package <| tag == 'foreman::cli' |> -> Class['foreman']
+  Package <| tag == 'foreman::providers' |> -> Class['foreman']
 
   contain 'foreman::settings' # lint:ignore:relative_classname_inclusion (PUP-1597)
   Class['foreman::database'] -> Class['foreman::settings']

manifests/params.pp

@@ -24,20 +24,14 @@
   $passenger_min_instances = 1
   $passenger_start_timeout = 90
 
-  # Additional software repos
-  $configure_epel_repo      = ($facts['os']['family'] == 'RedHat' and $facts['os']['name'] != 'Fedora')
-
   # Advanced configuration
-  # this can be a version or nightly
-  $repo              = undef
   $app_root          = '/usr/share/foreman'
   $plugin_config_dir = '/etc/foreman/plugins'
   $manage_user       = true
   $user              = 'foreman'
   $group             = 'foreman'
   $user_groups       = ['puppet']
   $rails_env         = 'production'
-  $gpgcheck          = true
   $version           = 'present'
   $plugin_version    = 'present'
 

manifests/providers.pp

@@ -17,5 +17,6 @@
 ) inherits foreman::providers::params {
   if $oauth {
     ensure_packages([$oauth_package])
+    Anchor <| title == 'foreman::repo' |> -> Package[$oauth_package]
   }
 }

manifests/repo.pp

@@ -1,22 +1,43 @@
-# @summary Configure the foreman repo
-# @api private
+# Configure the foreman repo
+#
+# @param repo
+#   The repository version to manage. This can be a specific version or nightly
+#
+# @param configure_scl_repo
+#   If disabled the SCL repo will not be configured on Red Hat clone systems.
+#   (Currently only installs repos for CentOS and Scientific)
+#
+# @param gpgcheck
+#   Turn on/off gpg check in repo files (effective only on RedHat family systems)
+#
+# @param scl_repo_ensure
+#   The ensure to set on the SCL repo package
+#
 class foreman::repo(
-  $repo                = $foreman::repo,
-  $gpgcheck            = $foreman::gpgcheck,
-  $configure_epel_repo = $foreman::configure_epel_repo,
-  $configure_scl_repo  = $foreman::configure_scl_repo,
+  Optional[Variant[Enum['nightly'], Pattern['^\d+\.\d+$']]] $repo = undef,
+  Boolean $gpgcheck = true,
+  Boolean $configure_scl_repo = $facts['os']['family'] == 'RedHat' and $facts['os']['release']['major'] == '7',
+  String $scl_repo_ensure = 'installed',
 ) {
   if $repo {
     foreman::repos { 'foreman':
       repo     => $repo,
       gpgcheck => $gpgcheck,
-      before   => Class['foreman::repos::extra'],
+      before   => Anchor['foreman::repo'],
+    }
+
+    if $configure_scl_repo {
+      Foreman::Repos['foreman'] -> Package['foreman-release-scl']
     }
   }
 
-  class { 'foreman::repos::extra':
-    configure_epel_repo => $configure_epel_repo,
-    configure_scl_repo  => $configure_scl_repo,
+  if $configure_scl_repo {
+    package {'foreman-release-scl':
+      ensure => $scl_repo_ensure,
+      before => Anchor['foreman::repo'],
+    }
   }
-  contain foreman::repos::extra
+
+  # An anchor is used because it can be collected
+  anchor { 'foreman::repo': } # lint:ignore:anchor_resource
 }

spec/classes/foreman_cli_spec.rb

@@ -64,6 +64,13 @@
         end
       end
 
+      context 'with repo included' do
+        let(:pre_condition) { 'include foreman::repo' }
+
+        it { is_expected.to compile.with_all_deps }
+        it { should contain_package('foreman-cli').that_subscribes_to('Anchor[foreman::repo]') }
+      end
+
       context 'with settings from foreman' do
         let :pre_condition do
           <<-PUPPET
@@ -80,7 +87,6 @@ class { 'foreman':
         it do
           should contain_package('foreman-cli')
             .with_ensure('installed')
-            .that_subscribes_to('Class[foreman::repo]')
         end
 
         it 'should contain settings in /etc from foreman' do

spec/classes/foreman_install_spec.rb

@@ -13,10 +13,9 @@
       end
 
       describe 'with repo' do
-        let(:params) { super().merge(repo: 'nightly') }
-        it { should contain_class('foreman::repo') }
-        it { should contain_foreman__repos('foreman') }
-        it { should contain_package('foreman-postgresql').that_requires('Class[foreman::repo]') }
+        let(:pre_condition) { 'include foreman::repo' }
+
+        it { should contain_package('foreman-postgresql').that_requires('Anchor[foreman::repo]') }
       end
 
       describe 'sidekiq jobs' do

spec/classes/foreman_providers_spec.rb

@@ -21,11 +21,18 @@
         it { should_not contain_package(oauth_os) }
       end
 
+      context 'with foreman::repo' do
+        let(:pre_condition) { 'include foreman::repo' }
+
+        it { is_expected.to compile.with_all_deps }
+        it { should contain_package(oauth_os).that_requires('Anchor[foreman::repo]') }
+      end
+
       context 'with foreman' do
         let(:pre_condition) { 'include foreman' }
 
         it { is_expected.to compile.with_all_deps }
-        it { should contain_package(oauth_os).that_subscribes_to('Class[foreman::repo]') }
+        it { should contain_package(oauth_os).that_comes_before('Class[foreman]') }
       end
     end
   end

spec/classes/foreman_repo_spec.rb

@@ -1,16 +1,43 @@
 require 'spec_helper'
 
 describe 'foreman::repo' do
-  on_supported_os.each do |os, facts|
+  on_supported_os.each do |os, os_facts|
     context "on #{os}" do
-      let(:facts) { facts }
+      let(:facts) { os_facts }
+
+      describe 'with default parameters' do
+        it { is_expected.to compile.with_all_deps }
+        it { is_expected.to contain_anchor('foreman::repo') }
+        it { is_expected.not_to contain_foreman__repos('foreman') }
+
+        it do
+          if facts[:osfamily] == 'RedHat' && facts[:operatingsystemmajrelease] == '7'
+            is_expected.to contain_package('foreman-release-scl')
+          else
+            is_expected.not_to contain_package('foreman-release-scl')
+          end
+        end
+      end
+
+      describe 'with minimal parameters' do
+        let(:params) { {repo: 'nightly'} }
+
+        it { is_expected.to compile.with_all_deps }
+        it { is_expected.to contain_foreman__repos('foreman').with_repo('nightly').with_gpgcheck(true) }
+
+        it do
+          if facts[:osfamily] == 'RedHat' && facts[:operatingsystemmajrelease] == '7'
+            is_expected.to contain_package('foreman-release-scl')
+          else
+            is_expected.not_to contain_package('foreman-release-scl')
+          end
+        end
+      end
 
       describe 'with explicit parameters' do
         let :params do
           {
             repo: '1.19',
-            gpgcheck: true,
-            configure_epel_repo: false,
             configure_scl_repo: false
           }
         end
@@ -23,11 +50,7 @@
             .with_gpgcheck(true)
         end
 
-        it 'should include extra repos' do
-          is_expected.to contain_class('foreman::repos::extra')
-            .with_configure_epel_repo(false)
-            .with_configure_scl_repo(false)
-        end
+        it { is_expected.not_to contain_package('foreman-release-scl') }
       end
     end
   end

spec/classes/foreman_spec.rb

@@ -9,29 +9,6 @@
       context 'with default parameters' do
         it { is_expected.to compile.with_all_deps }
 
-        # repo
-        it { should contain_class('foreman::repo').that_notifies('Class[foreman::install]') }
-        it { should_not contain_foreman__repos('foreman') }
-        case facts[:osfamily]
-        when 'RedHat'
-          configure_repo = facts[:operatingsystem] != 'Fedora'
-          it {
-            should contain_class('foreman::repos::extra')
-              .with_configure_scl_repo(configure_repo)
-              .with_configure_epel_repo(configure_repo)
-          }
-
-          if facts[:operatingsystem] != 'Fedora'
-            it { should_not contain_package('tfm-rubygem-passenger-native') }
-          end
-        when 'Debian'
-          it {
-            should contain_class('foreman::repos::extra')
-              .with_configure_scl_repo(false)
-              .with_configure_epel_repo(false)
-          }
-        end
-
         # install
         it { should contain_class('foreman::install') }
         it { should contain_package('foreman-postgresql').with_ensure('present') }
@@ -208,10 +185,6 @@
             servername: 'localhost',
             serveraliases: ['foreman'],
             ssl: true,
-            repo: 'nightly',
-            configure_epel_repo: true,
-            configure_scl_repo: false,
-            gpgcheck: true,
             version: '1.12',
             plugin_version: 'installed',
             db_manage: true,

spec/setup_acceptance_node.pp

@@ -1,14 +1,9 @@
-$configure_scl_repo = $facts['os']['family'] == 'RedHat' and $facts['os']['release']['major'] == '7'
-
 class { 'foreman::repo':
-  repo                => 'nightly',
-  gpgcheck            => false,
-  configure_epel_repo => false,
-  configure_scl_repo  => $configure_scl_repo,
+  repo => 'nightly',
 }
 
 # Needed for idempotency when SELinux is enabled
-if $configure_scl_repo {
+if $foreman::repo::configure_scl_repo {
   package { 'rh-redis5-redis':
     ensure  => installed,
     require => Class['foreman::repo'],