Fixes #36897 - Hosts now consistently follow rules when inheriting at…

…tributes from their hostgroups

app/models/katello/concerns/host_managed_extensions.rb

@@ -48,28 +48,51 @@ def inherited_attributes
           inherited_attrs
         end
 
-        def apply_inherited_attributes(attributes, initialized = true)
-          attributes = super(attributes, initialized) || {}
-          facet_attrs = attributes&.[]('content_facet_attributes')
-          return attributes if facet_attrs.blank?
-          cv_id = facet_attrs['content_view_id']
-          lce_id = facet_attrs['lifecycle_environment_id']
-          if initialized && (cv_id.blank? || lce_id.blank?)
-            if cv_id.blank?
-              Rails.logger.info "Hostgroup has no content view assigned; using host's existing content view"
-              facet_attrs['content_view_id'] = content_facet&.single_content_view&.id
-            end
-            if lce_id.blank?
-              Rails.logger.info "Hostgroup has no lifecycle environment assigned; using host's existing lifecycle environment"
-              facet_attrs['lifecycle_environment_id'] = content_facet&.single_lifecycle_environment&.id
-            end
-            attributes['content_facet_attributes'] = facet_attrs
-          else
-            Rails.logger.debug "Hostgroup has content view and lifecycle environment assigned; using those"
+        def apply_inherited_attributes(input_attributes, initialized = true)
+          attributes = super(input_attributes.clone, initialized) || {}
+
+          # When the input explicitly sets content_facet_attributes, keep them
+          # regardless of what any higher function calls added
+          if initialized && input_attributes.key?(:content_facet_attributes) &&
+            attributes.key?(:content_facet_attributes)
+            attributes[:content_facet_attributes] = input_attributes[:content_facet_attributes]
+          end
+
+          # Inherit hostgroup's content facet attributes. Skip if we are
+          # initialized or if the call contains a 'type' key.
+          #     initialized = new host, inheritance needed
+          #     type key = first stage host creation, skip inheritance here
+          unless initialized || attributes.key?(:type)
+            attributes = apply_inherited_content_view(attributes)
+            attributes = apply_inherited_lifecycle_environment(attributes)
           end
           attributes
         end
 
+        def apply_inherited_content_view(attributes)
+          facet_cv_id = content_facet&.single_content_view&.id
+          is_missing_cv = !attributes.key?(:content_facet_attributes) ||
+              attributes[:content_facet_attributes][:content_view_id].blank?
+          if is_missing_cv && facet_cv_id.present?
+            Rails.logger.info "Host has no content view assigned; using hostgroups's existing content view"
+            attributes[:content_facet_attributes] = {} unless attributes.key?(:content_facet_attributes)
+            attributes[:content_facet_attributes][:content_view_id] = facet_cv_id
+          end
+          return attributes
+        end
+
+        def apply_inherited_lifecycle_environment(attributes)
+          facet_le_id = content_facet&.single_lifecycle_environment&.id
+          is_missing_le = !attributes.key?(:content_facet_attributes) ||
+            attributes[:content_facet_attributes][:lifecycle_environment_id].blank?
+          if is_missing_le && facet_le_id.present?
+            Rails.logger.info "Host has no lifecycle environment assigned; using hostgroups's existing lifecycle environment"
+            attributes[:content_facet_attributes] = {} unless attributes.key?(:content_facet_attributes)
+            attributes[:content_facet_attributes][:lifecycle_environment_id] = facet_le_id
+          end
+          return attributes
+        end
+
         def smart_proxy_ids
           ids = super
           ids << content_source_id

test/models/concerns/host_managed_extensions_test.rb

@@ -266,6 +266,272 @@ def test_host_update_with_overridden_dmi_uuid
       override = host.subscription_facet.dmi_uuid_override
       assert_equal override.value, params[:facts]['dmi.system.uuid']
     end
+
+    def test_create_apply_inherited_attributes_without_hostgroup
+      host = FactoryBot.create(:host, :with_content, :with_subscription, content_view: @library.content_views[0], lifecycle_environment: @library)
+      new_cv_id = @library.content_views[1].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 2, result.length
+      assert_equal params[:comment], result[:comment]
+      assert result.key?(:content_facet_attributes)
+      assert_equal 2, result[:content_facet_attributes].length
+      assert_equal @library.content_views[0].id, result[:content_facet_attributes][:content_view_id]
+      assert_equal @library.id, result[:content_facet_attributes][:lifecycle_environment_id]
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 2, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+      assert_equal @library.id, result[:content_facet_attributes][:lifecycle_environment_id]
+    end
+
+    def test_update_apply_inherited_attributes_without_hostgroup
+      host = FactoryBot.create(:host, :with_content, :with_subscription, content_view: @library.content_views[0], lifecycle_environment: @library)
+      new_cv_id = @library.content_views[1].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert_equal params[:comment], result[:comment]
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 1, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+    end
+
+    def test_create_apply_inherited_attributes_without_hostgroup_no_cv
+      host = FactoryBot.create(:host, :with_content, :with_subscription, lifecycle_environment: @library)
+      new_cv_id = @library.content_views[1].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 1, result.length
+      assert_equal params[:comment], result[:comment]
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 1, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+    end
+
+    def test_update_apply_inherited_attributes_without_hostgroup_no_cv
+      host = FactoryBot.create(:host, :with_content, :with_subscription, lifecycle_environment: @library)
+      new_cv_id = @library.content_views[1].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert_equal params[:comment], result[:comment]
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 1, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+    end
+
+    def test_create_apply_inherited_attributes_with_same_attributes_as_hostgroup
+      hostgroup = FactoryBot.create(:hostgroup, content_view: @library.content_views[0], lifecycle_environment: @library)
+      host = FactoryBot.create(:host, :with_content, :with_subscription, content_view: @library.content_views[0], lifecycle_environment: @library, hostgroup: hostgroup)
+      new_cv_id = @library.content_views[1].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 2, result.length
+      assert_equal params[:comment], result[:comment]
+      assert result.key?(:content_facet_attributes)
+      assert_equal 2, result[:content_facet_attributes].length
+      assert_equal @library.content_views[0].id, result[:content_facet_attributes][:content_view_id]
+      assert_equal @library.id, result[:content_facet_attributes][:lifecycle_environment_id]
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 2, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+      assert_equal @library.id, result[:content_facet_attributes][:lifecycle_environment_id]
+    end
+
+    def test_update_apply_inherited_attributes_with_same_attributes_as_hostgroup
+      hostgroup = FactoryBot.create(:hostgroup, content_view: @library.content_views[0], lifecycle_environment: @library)
+      host = FactoryBot.create(:host, :with_content, :with_subscription, content_view: @library.content_views[0], lifecycle_environment: @library, hostgroup: hostgroup)
+      new_cv_id = @library.content_views[1].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert_equal params[:comment], result[:comment]
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 1, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+    end
+
+    def test_create_apply_inherited_attributes_with_same_attributes_as_hostgroup_no_cv
+      hostgroup = FactoryBot.create(:hostgroup, lifecycle_environment: @library)
+      host = FactoryBot.create(:host, :with_content, :with_subscription, content_view: @library.content_views[0], lifecycle_environment: @library, hostgroup: hostgroup)
+      new_cv_id = @library.content_views[1].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 2, result.length
+      assert_equal params[:comment], result[:comment]
+      assert result.key?(:content_facet_attributes)
+      assert_equal 2, result[:content_facet_attributes].length
+      assert_equal @library.content_views[0].id, result[:content_facet_attributes][:content_view_id]
+      assert_equal @library.id, result[:content_facet_attributes][:lifecycle_environment_id]
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 2, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+      assert_equal @library.id, result[:content_facet_attributes][:lifecycle_environment_id]
+    end
+
+    def test_update_apply_inherited_attributes_with_same_attributes_as_hostgroup_no_cv
+      hostgroup = FactoryBot.create(:hostgroup, lifecycle_environment: @library)
+      host = FactoryBot.create(:host, :with_content, :with_subscription, content_view: @library.content_views[0], lifecycle_environment: @library, hostgroup: hostgroup)
+      new_cv_id = @library.content_views[1].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert_equal params[:comment], result[:comment]
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 1, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+    end
+
+    def test_create_apply_inherited_attributes_with_different_attributes_as_hostgroup
+      hostgroup = FactoryBot.create(:hostgroup, content_view: @library.content_views[0], lifecycle_environment: @library)
+      host = FactoryBot.create(:host, :with_content, :with_subscription, content_view: @library.content_views[1], lifecycle_environment: @library, hostgroup: hostgroup)
+      new_cv_id = @library.content_views[2].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 2, result.length
+      assert_equal params[:comment], result[:comment]
+      assert result.key?(:content_facet_attributes)
+      assert_equal 2, result[:content_facet_attributes].length
+      assert_equal @library.content_views[1].id, result[:content_facet_attributes][:content_view_id]
+      assert_equal @library.id, result[:content_facet_attributes][:lifecycle_environment_id]
+      # Note: we're returning the host's CV on this call. That *technically* shouldn't happen becuase this
+      # is a non-attribute call, but it doesn't actually change anything when the host is updated down the
+      # road. I'm keeping this behavior for now.
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 2, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+      assert_equal @library.id, result[:content_facet_attributes][:lifecycle_environment_id]
+    end
+
+    def test_update_inherited_attributes_with_different_attributes_as_hostgroup
+      hostgroup = FactoryBot.create(:hostgroup, content_view: @library.content_views[0], lifecycle_environment: @library)
+      host = FactoryBot.create(:host, :with_content, :with_subscription, content_view: @library.content_views[1], lifecycle_environment: @library, hostgroup: hostgroup)
+      new_cv_id = @library.content_views[2].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert_equal params[:comment], result[:comment]
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 1, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+    end
+
+    def test_create_apply_inherited_attributes_with_different_attributes_as_hostgroup_no_cv
+      hostgroup = FactoryBot.create(:hostgroup, lifecycle_environment: @library)
+      host = FactoryBot.create(:host, :with_content, :with_subscription, content_view: @library.content_views[1], lifecycle_environment: @library, hostgroup: hostgroup)
+      new_cv_id = @library.content_views[2].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 2, result.length
+      assert_equal params[:comment], result[:comment]
+      assert result.key?(:content_facet_attributes)
+      assert_equal 2, result[:content_facet_attributes].length
+      assert_equal @library.content_views[1].id, result[:content_facet_attributes][:content_view_id]
+      assert_equal @library.id, result[:content_facet_attributes][:lifecycle_environment_id]
+      # Note: like the above test, we're returning the host's CV on this call. That's technically an
+      # error but there are no side effects since the future host update changes no data.
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params, false)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 2, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+      assert_equal @library.id, result[:content_facet_attributes][:lifecycle_environment_id]
+    end
+
+    def test_update_apply_inherited_attributes_with_different_attributes_as_hostgroup_no_cv
+      hostgroup = FactoryBot.create(:hostgroup, lifecycle_environment: @library)
+      host = FactoryBot.create(:host, :with_content, :with_subscription, content_view: @library.content_views[1], lifecycle_environment: @library, hostgroup: hostgroup)
+      new_cv_id = @library.content_views[2].id
+
+      # no attributes
+      params = {"comment": "test comment"}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert_equal params[:comment], result[:comment]
+
+      # with attributes
+      params = {"content_facet_attributes": {"content_view_id": new_cv_id}}
+      result = host.apply_inherited_attributes(params)
+      assert_equal 1, result.length
+      assert result.key?(:content_facet_attributes)
+      assert_equal 1, result[:content_facet_attributes].length
+      assert_equal new_cv_id, result[:content_facet_attributes][:content_view_id]
+    end
   end
 
   class HostInstalledPackagesTest < HostManagedExtensionsTestBase