Fixes #24819 - Add support for hostgroup content facet

app/controllers/katello/concerns/api/v2/hostgroups_controller_extensions.rb

@@ -20,7 +20,7 @@ module ApiPieExtensions
         include ApiPieExtensions
 
         def create
-          @hostgroup = Hostgroup.new(hostgroup_params)
+          @hostgroup = ::Hostgroup.new(hostgroup_params)
           process_response @hostgroup.save
         end
 

app/helpers/katello/hosts_and_hostgroups_helper.rb

@@ -15,7 +15,7 @@ def blank_or_inherit_with_id(f, attr)
     end
 
     def organizations(host)
-      if host.is_a?(Hostgroup)
+      if host.is_a?(::Hostgroup)
         host.organizations
       else
         host.organization ? [host.organization] : []
@@ -29,8 +29,7 @@ def use_install_media(host, options = {})
 
     def host_hostgroup_kickstart_repository_id(host)
       return if host.blank?
-      return host.kickstart_repository_id if host.is_a?(Hostgroup)
-      host.content_facet.kickstart_repository_id if host.try(:content_facet).present?
+      host.content_facet&.kickstart_repository_id
     end
 
     def kickstart_repository_id(host, options = {})
@@ -205,7 +204,7 @@ def kickstart_repository_options(param_host, options = {})
 
         return [] unless new_host.operatingsystem.is_a?(Redhat)
 
-        if (host.is_a? Hostgroup)
+        if (host.is_a? ::Hostgroup)
           new_host.content_facet = ::Katello::Host::ContentFacet.new(:lifecycle_environment_id => host.inherited_lifecycle_environment_id,
                                                           :content_view_id => host.inherited_content_view_id,
                                                           :content_source_id => host.inherited_content_source_id)

app/lib/katello/validators/hostgroup_kickstart_repository_validator.rb

@@ -1,29 +1,29 @@
 module Katello
   module Validators
     class HostgroupKickstartRepositoryValidator < ActiveModel::Validator
-      def validate(hostgroup)
+      def validate(facet)
         # check content source first, otherwise it's meaningless to proceed
-        if hostgroup.content_source && hostgroup.lifecycle_environment
-          valid = hostgroup.content_source.lifecycle_environments.include?(hostgroup.lifecycle_environment)
-          hostgroup.errors.add(:base, _("The selected content source and lifecycle environment do not match")) && return unless valid
+        if facet.content_source && facet.lifecycle_environment
+          valid = facet.content_source.lifecycle_environments.include?(facet.lifecycle_environment)
+          facet.errors.add(:base, _("The selected content source and lifecycle environment do not match")) && return unless valid
         end
 
-        return unless hostgroup.kickstart_repository_id
+        return unless facet.kickstart_repository_id
 
-        msg = if hostgroup.content_source.blank?
+        msg = if facet.content_source.blank?
                 _("Please select a content source before assigning a kickstart repository")
-              elsif hostgroup.operatingsystem.blank?
+              elsif facet.hostgroup.operatingsystem.blank?
                 _("Please select an operating system before assigning a kickstart repository")
-              elsif !hostgroup.operatingsystem.is_a?(Redhat)
+              elsif !facet.hostgroup.operatingsystem.is_a?(Redhat)
                 _("Kickstart repositories can only be assigned to hosts in the Red Hat family")
-              elsif hostgroup.architecture.blank?
+              elsif facet.hostgroup.architecture.blank?
                 _("Please select an architecture before assigning a kickstart repository")
-              elsif !hostgroup.matching_kickstart_repository?
+              elsif !facet.hostgroup.matching_kickstart_repository?(facet)
                 _("The selected kickstart repository is not part of the assigned content view, lifecycle environment,
                   content source, operating system, and architecture")
               end
 
-        hostgroup.errors.add(:base, msg) if msg
+        facet.errors.add(:base, msg) if msg
       end
     end
   end

app/models/katello/concerns/hostgroup_extensions.rb

@@ -5,35 +5,36 @@ module HostgroupExtensions
 
       included do
         before_save :add_organization_for_environment
-        belongs_to :kickstart_repository, :class_name => "::Katello::Repository",
-                   :foreign_key => :kickstart_repository_id, :inverse_of => :kickstart_hostgroups
-        belongs_to :content_source, :class_name => "::SmartProxy", :foreign_key => :content_source_id, :inverse_of => :hostgroups
-        belongs_to :content_view, :inverse_of => :hostgroups, :class_name => "::Katello::ContentView"
-        belongs_to :lifecycle_environment, :inverse_of => :hostgroups, :class_name => "::Katello::KTEnvironment"
-
-        validates_with Katello::Validators::ContentViewEnvironmentValidator
-        validates_with Katello::Validators::HostgroupKickstartRepositoryValidator
-        validates_with ::AssociationExistsValidator, attributes: [:content_source]
+        has_one :kickstart_repository, :through => :content_facet
+        has_one :content_source, :through => :content_facet
+        has_one :content_view, :through => :content_facet
+        has_one :lifecycle_environment, :through => :content_facet
 
         scoped_search :relation => :content_source, :on => :name, :complete_value => true, :rename => :content_source, :only_explicit => true
         scoped_search :relation => :content_view, :on => :name, :complete_value => true, :rename => :content_view, :only_explicit => true
         scoped_search :relation => :lifecycle_environment, :on => :name, :complete_value => true, :rename => :lifecycle_environment, :only_explicit => true
 
         before_validation :correct_kickstart_repository
+
+        delegate :content_source_name, :content_view_name, :lifecycle_environment_name, to: :content_facet, allow_nil: true
+        delegate :content_source_id, :content_view_id, :lifecycle_environment_id, :kickstart_repository_id, to: :content_facet, allow_nil: true
+        delegate :'content_source_id=', :'content_view_id=', :'lifecycle_environment_id=', :'kickstart_repository_id=', to: :safe_content_facet, allow_nil: true
       end
 
       def correct_kickstart_repository
         # If switched from ks repo to install media:
-        if medium_id_changed? && medium && kickstart_repository_id
-          self.kickstart_repository_id = nil
+        if medium_id_changed? && medium && content_facet&.kickstart_repository_id
+          # since it's :through association, nullify both the actual data source and delegate
+          self.content_facet.kickstart_repository = nil
+          self.kickstart_repository = nil
         # If switched from install media to ks repo:
-        elsif kickstart_repository && medium
+        elsif content_facet&.kickstart_repository && medium
           self.medium = nil
         end
 
-        unless matching_kickstart_repository?
+        unless matching_kickstart_repository?(content_facet)
           if (equivalent = equivalent_kickstart_repository)
-            self.kickstart_repository_id = equivalent[:id]
+            self.content_facet.kickstart_repository_id = equivalent[:id]
           end
         end
       end
@@ -55,19 +56,19 @@ def content_source
       end
 
       def inherited_content_source_id
-        inherited_ancestry_attribute(:content_source_id)
+        inherited_ancestry_attribute(:content_source_id, :content_facet)
       end
 
       def inherited_content_view_id
-        inherited_ancestry_attribute(:content_view_id)
+        inherited_ancestry_attribute(:content_view_id, :content_facet)
       end
 
       def inherited_lifecycle_environment_id
-        inherited_ancestry_attribute(:lifecycle_environment_id)
+        inherited_ancestry_attribute(:lifecycle_environment_id, :content_facet)
       end
 
       def inherited_kickstart_repository_id
-        inherited_ancestry_attribute(:kickstart_repository_id)
+        inherited_ancestry_attribute(:kickstart_repository_id, :content_facet)
       end
 
       def rhsm_organization_label
@@ -95,22 +96,36 @@ def equivalent_kickstart_repository
         ks_repos.find { |repo| repo[:name] == kickstart_repository.label }
       end
 
-      def matching_kickstart_repository?
+      def matching_kickstart_repository?(content_facet)
         return true unless operatingsystem
 
         if operatingsystem.respond_to? :kickstart_repos
-          return operatingsystem.kickstart_repos(self).any? { |repo| repo[:id] == kickstart_repository_id }
+          return operatingsystem.kickstart_repos(self).any? { |repo| repo[:id] == (content_facet&.kickstart_repository_id || content_facet&.kickstart_repository&.id) }
         end
       end
 
       private
 
-      def inherited_ancestry_attribute(attribute)
-        if ancestry.present?
-          self[attribute] || self.class.sort_by_ancestry(ancestors.where("#{attribute.to_s} is not NULL")).last.try(attribute)
-        else
-          self.send(attribute)
+      def inherited_ancestry_attribute(attribute, facet)
+        value = self.send(facet)&.send(attribute)
+
+        if value.nil? && ancestry.present?
+          # take first non-null value for the attribute going up the ancestry tree.
+          # example: you have hg1 -> hg11 -> hg111 -> hg1111 hostgroups.
+          # given we are querying hg1111 (the leaf), and a value is set on:
+          # hg1: 1
+          # hg11: 2
+          # it will return the value 2.
+          facet_model = Facets.registered_facets[facet].hostgroup_configuration.model
+          value = facet_model.where.not(attribute => nil).joins(:hostgroup).merge(
+            ::Hostgroup.where(id: self.ancestor_ids).reorder(ancestry: :desc)
+          ).limit(1).pluck(attribute)
         end
+        value
+      end
+
+      def safe_content_facet
+        content_facet || build_content_facet
       end
     end
   end

app/models/katello/concerns/smart_proxy_extensions.rb

@@ -49,8 +49,9 @@ def refresh
         has_many :content_facets, :class_name => "::Katello::Host::ContentFacet", :foreign_key => :content_source_id,
                                   :inverse_of => :content_source, :dependent => :nullify
 
-        has_many :hostgroups, :class_name => "::Hostgroup", :foreign_key => :content_source_id,
+        has_many :hostgroup_content_facets, :class_name => "::Katello::Hostgroup::ContentFacet", :foreign_key => :content_source_id,
                               :inverse_of => :content_source, :dependent => :nullify
+        has_many :hostgroups, :class_name => "::Hostgroup", :through => :hostgroup_content_facets
 
         validates :download_policy, inclusion: {
           :in => DOWNLOAD_POLICIES,

app/models/katello/content_view.rb

@@ -46,8 +46,10 @@ class ContentView < Katello::Model
                           :inverse_of => :content_view, :dependent => :restrict_with_exception
     has_many :hosts,      :class_name => "::Host::Managed", :through => :content_facets,
                           :inverse_of => :content_view
-    has_many :hostgroups, :class_name => "::Hostgroup", :foreign_key => :content_view_id,
+    has_many :hostgroup_content_facets, :class_name => "Katello::Hostgroup::ContentFacet", :foreign_key => :content_view_id,
                           :inverse_of => :content_view, :dependent => :nullify
+    has_many :hostgroups, :class_name => "::Hostgroup", :through => :hostgroup_content_facets,
+                          :inverse_of => :content_view
 
     has_many :repository_references, :class_name => 'Katello::Pulp3::RepositoryReference', :foreign_key => :content_view_id,
              :dependent => :destroy, :inverse_of => :content_view

app/models/katello/hostgroup/content_facet.rb

@@ -0,0 +1,19 @@
+module Katello
+  module Hostgroup
+    class ContentFacet < Katello::Model
+      audited :associated_with => :lifecycle_environment
+      self.table_name = 'katello_hostgroup_content_facets'
+      include Facets::HostgroupFacet
+
+      belongs_to :kickstart_repository, :class_name => "::Katello::Repository", :foreign_key => :kickstart_repository_id, :inverse_of => :kickstart_hostgroup_content_facets
+
+      belongs_to :content_view, :inverse_of => :hostgroup_content_facets, :class_name => "Katello::ContentView"
+      belongs_to :lifecycle_environment, :inverse_of => :hostgroup_content_facets, :class_name => "Katello::KTEnvironment"
+      belongs_to :content_source, :class_name => "::SmartProxy", :foreign_key => :content_source_id, :inverse_of => :hostgroup_content_facets
+
+      validates_with Katello::Validators::ContentViewEnvironmentValidator
+      validates_with Katello::Validators::HostgroupKickstartRepositoryValidator
+      validates_with ::AssociationExistsValidator, attributes: [:content_source]
+    end
+  end
+end

app/models/katello/kt_environment.rb

@@ -31,8 +31,10 @@ class KTEnvironment < Katello::Model
                           :inverse_of => :lifecycle_environment, :dependent => :restrict_with_exception
     has_many :hosts,      :class_name => "::Host::Managed", :through => :content_facets,
                           :inverse_of => :lifecycle_environment
-    has_many :hostgroups, :class_name => "::Hostgroup",     :foreign_key => :lifecycle_environment_id,
+    has_many :hostgroup_content_facets, :class_name => "Katello::Hostgroup::ContentFacet", :foreign_key => :lifecycle_environment_id,
                           :inverse_of => :lifecycle_environment, :dependent => :restrict_with_exception
+    has_many :hostgroups, :class_name => "::Hostgroup", :through => :hostgroup_content_facets,
+                          :inverse_of => :lifecycle_environment
 
     scope :completer_scope, ->(options = nil) { where('organization_id = ?', options[:organization_id]) if options[:organization_id].present? }
     scope :non_library, -> { where(library: false) }

app/models/katello/repository.rb

@@ -92,9 +92,11 @@ class Repository < Katello::Model
     has_many :kickstart_content_facets, :class_name => "Katello::Host::ContentFacet", :foreign_key => :kickstart_repository_id,
                           :inverse_of => :kickstart_repository, :dependent => :nullify
 
-    has_many :kickstart_hostgroups, :class_name => "::Hostgroup", :foreign_key => :kickstart_repository_id,
+    has_many :kickstart_hostgroup_content_facets, :class_name => "Katello::Hostgroup::ContentFacet", :foreign_key => :kickstart_repository_id,
              :inverse_of => :kickstart_repository, :dependent => :nullify
 
+    has_many :kickstart_hostgroups, :class_name => "::Hostgroup", :through => :kickstart_hostgroup_content_facets
+
     has_many :repository_module_streams, class_name: "Katello::RepositoryModuleStream", dependent: :delete_all
     has_many :module_streams, through: :repository_module_streams
 

db/migrate/20141222151001_add_host_content_view_environment.rb

@@ -6,7 +6,7 @@ def up
     add_column :hostgroups, :content_view_id, :integer, :null => true
     add_column :hostgroups, :lifecycle_environment_id, :integer, :null => true
 
-    [Hostgroup, Host::Managed].each do |model|
+    [::Hostgroup, Host::Managed].each do |model|
       model.find_each do |host|
         lifecycle_environment = host.environment.try(:lifecycle_environment)
         content_view = host.environment.try(:content_view)

db/migrate/20180904122343_create_hostgroup_content_facet.katello.rb

@@ -0,0 +1,16 @@
+class CreateHostgroupContentFacet < ActiveRecord::Migration[5.1]
+  def change
+    create_table :katello_hostgroup_content_facets do |t|
+      t.column :hostgroup_id, :integer, :null => false
+      t.column :kickstart_repository_id, :integer, :null => true
+      t.column :content_source_id, :integer, :null => true
+      t.column :content_view_id, :integer, :null => true
+      t.column :lifecycle_environment_id, :integer, :null => true
+    end
+    add_foreign_key :katello_hostgroup_content_facets, :katello_repositories, :column => :kickstart_repository_id
+    add_foreign_key :katello_hostgroup_content_facets, :hostgroups, :column => :hostgroup_id
+    add_foreign_key :katello_hostgroup_content_facets, :katello_content_views, :column => :content_view_id
+    add_foreign_key :katello_hostgroup_content_facets, :smart_proxies, :name => "katello_hostgroup_content_facets_content_source_id_fk", :column => "content_source_id"
+    add_foreign_key :katello_hostgroup_content_facets, :katello_environments, :column => :lifecycle_environment_id
+  end
+end

db/migrate/20200514092553_move_katello_fields_from_hostgroups.katello.rb

@@ -0,0 +1,53 @@
+class MoveKatelloFieldsFromHostgroups < ActiveRecord::Migration[6.0]
+  def up
+    if User.where(login: User::ANONYMOUS_ADMIN).exists?
+      User.as_anonymous_admin do
+        copy_data_from_hostgroup
+      end
+    end
+
+    change_table :hostgroups do |table|
+      table.remove(
+        :content_source_id,
+        :kickstart_repository_id,
+        :content_view_id,
+        :lifecycle_environment_id
+      )
+    end
+  end
+
+  def down
+    change_table :hostgroups do |t|
+      t.column :kickstart_repository_id, :integer, :null => true
+      t.column :content_source_id, :integer, :null => true
+      t.column :content_view_id, :integer, :null => true
+      t.column :lifecycle_environment_id, :integer, :null => true
+    end
+  end
+
+  def copy_data_from_hostgroup
+    hg_table = ::Hostgroup.arel_table
+    hostgroups = ::Hostgroup.unscoped.where(
+           hg_table[:content_source_id].eq(nil)
+      .and(hg_table[:kickstart_repository_id].eq(nil))
+      .and(hg_table[:content_view_id].eq(nil))
+      .and(hg_table[:lifecycle_environment_id].eq(nil))
+      .not)
+    hostgroups.in_batches do |batch|
+      batch.pluck(
+        :id,
+        :content_source_id,
+        :kickstart_repository_id,
+        :content_view_id,
+        :lifecycle_environment_id
+      ).each do |hostgroup_id, content_source_id, kickstart_repository_id, content_view_id, lifecycle_environment_id|
+        content_facet = ::Katello::Hostgroup::ContentFacet.find_or_create_by(hostgroup_id: hostgroup_id)
+        content_facet.content_source_id = content_source_id
+        content_facet.kickstart_repository_id = kickstart_repository_id
+        content_facet.content_view_id = content_view_id
+        content_facet.lifecycle_environment_id = lifecycle_environment_id
+        content_facet.save!
+      end
+    end
+  end
+end

lib/katello/plugin.rb

@@ -233,7 +233,7 @@
                                   :host, :kickstart_repository_id],
     :subscription_facet_attributes => [:release_version, :autoheal, :purpose_usage, :purpose_role, :service_level, :host,
                                        {:installed_products => [:product_id, :product_name, :arch, :version]}, :facts, :hypervisor_guest_uuids => [], :purpose_addon_ids => []]
-  parameter_filter Hostgroup, :content_view_id, :lifecycle_environment_id, :content_source_id,
+  parameter_filter ::Hostgroup, :content_view_id, :lifecycle_environment_id, :content_source_id,
     :kickstart_repository_id
   parameter_filter Organization, :label, :service_level
   parameter_filter SmartProxy, :download_policy, :lifecycle_environment_ids => []
@@ -289,10 +289,14 @@
   end
 
   register_facet Katello::Host::ContentFacet, :content_facet do
-    api_view :list => 'katello/api/v2/content_facet/base_with_root', :single => 'katello/api/v2/content_facet/show'
-    api_docs :content_facet_attributes, ::Katello::Api::V2::HostContentsController
-    template_compatibility_properties :content_source_id, :content_source
-    extend_model ::Katello::Concerns::ContentFacetHostExtensions
+    configure_host do
+      api_view :list => 'katello/api/v2/content_facet/base_with_root', :single => 'katello/api/v2/content_facet/show'
+      api_docs :content_facet_attributes, ::Katello::Api::V2::HostContentsController
+      template_compatibility_properties :content_source_id, :content_source
+      extend_model ::Katello::Concerns::ContentFacetHostExtensions
+    end
+
+    configure_hostgroup(::Katello::Hostgroup::ContentFacet)
   end
 
   register_facet Katello::Host::SubscriptionFacet, :subscription_facet do

test/controllers/foreman/hostgroups_controller_test.rb

@@ -19,10 +19,17 @@ def test_new
   end
 
   def test_create
-    post :create, params: { :hostgroup => {:name => "foobar", :content_view_id => @library_view.id,
-                                           :lifecycle_environment_id => @library.id} }
+    post :create, params: {
+      :hostgroup => {
+        :name => "foobar",
+        :content_facet_attributes => {
+          :content_view_id => @library_view.id,
+          :lifecycle_environment_id => @library.id
+        }
+      }
+    }
 
-    assert_equal 1, Hostgroup.unscoped.where(:name => "foobar").count
+    assert_equal 1, ::Hostgroup.unscoped.where(:name => "foobar").count
     assert_response 302
   end
 end