Fixes #36310 - Migrate off ContentViews generated for repository export

[wbclark]

Ensures no Hosts remain on ContentViews generated for repository export. If doing so would leave the Host without a valid ContentView, the Default Organization View is assigned to it.

Also implements additional filtering in the candlepin proxies controller, and several additional tests.

What considerations are taken in implementing this pull request?

At least some validation was already done with c33da7a

What are the testing steps for this pull request?

1. Export a Content View

2. Try assigning some hosts to the content view created for export

3. Apply patch

4. Run migration

[wbclark]

My understanding of c33da7a is that multi-CV assignment is possible for Hosts but NOT for Activation Keys -- a Host performs multi-CV registration using multiple Activation Keys, never via a single Activation Key with multiple CVs assigned to it.

In addition to the fact that Activation Keys may have only a single CV assigned, validations already prevent CVs generated for repository export from being associated with AKs.

Based on these facts, I've removed the (harmless, but also unnecessary) Activation Key portion of the migration logic.

[wbclark]

The test failures in the RPM build pipeline are unrelated to this change.

[chris1984]

Getting the following error on a stream install:

2024-08-27T15:28:59 [E|kat|dba292f5] NameError: undefined local variable or method `cve' for #<Katello::Api::Rhsm::CandlepinProxiesController:0x000055cfe8863c70>
 dba292f5 | /usr/share/gems/gems/katello-4.14.0.pre.master/app/controllers/katello/api/rhsm/candlepin_proxies_controller.rb:421:in `block in update_environments_from_facts'
 dba292f5 | /usr/share/gems/gems/katello-4.14.0.pre.master/app/controllers/katello/api/rhsm/candlepin_proxies_controller.rb:419:in `map'
 dba292f5 | /usr/share/gems/gems/katello-4.14.0.pre.master/app/controllers/katello/api/rhsm/candlepin_proxies_controller.rb:419:in `update_environments_from_facts'
 dba292f5 | /usr/share/gems/gems/katello-4.14.0.pre.master/app/controllers/katello/api/rhsm/candlepin_proxies_controller.rb:271:in `facts'
 dba292f5 | /usr/share/gems/gems/actionpack-6.1.7.8/lib/action_controller/metal/basic_implicit_render.rb:6:in `send_action'

[wbclark]

Thanks for the review @chris1984

Huh, I didn't change that in my prior update so I should have hit that issue before. Maybe the test coverage for the facts method in the candlepin proxies controller could be improved so that gets caught before a fresh install

Anyway, I just needed to set cve = get_content_view_environment("cp_id", env['id']) and it should be working with the latest change I pushed

[wbclark]

Hey @chris1984 thanks for the initial review. Do you have some time to take a fresh look?

[chris1984]

Hey @chris1984 thanks for the initial review. Do you have some time to take a fresh look?

Will get to it this week :) wrapping up a few last minute 6.16 things

[wbclark]

no worries... thanks!

[chris1984]

@parthaa did you still want to look at this one, since you did ISS

[parthaa]

Nice work. LGTM

[parthaa]

Dont think this is necessary because of https://github.com/Katello/katello/blob/master/app/models/katello/host/content_facet.rb#L47 that checks for .generated_for_repository_export?

[parthaa]

Would recommend the following patch for db/migrate/20240815080259_migrate_off_generated_content_views.rb

diff --git a/db/migrate/20240815080259_migrate_off_generated_content_views.rb b/db/migrate/20240815080259_migrate_off_generated_content_views.rb
index 9df225aeb4..6e542787fc 100644
--- a/db/migrate/20240815080259_migrate_off_generated_content_views.rb
+++ b/db/migrate/20240815080259_migrate_off_generated_content_views.rb
@@ -1,4 +1,8 @@
 class MigrateOffGeneratedContentViews < ActiveRecord::Migration[6.1]
+  class FakeCVECF < ApplicationRecord
+    self.table_name = 'katello_content_view_environment_content_facets'
+  end
+
   def up
     say_with_time "Migrating hosts off generated content views" do
       migrate_hosts
@@ -13,25 +17,28 @@ class MigrateOffGeneratedContentViews < ActiveRecord::Migration[6.1]
 
   def migrate_hosts
     say_with_time "Migrating hosts..." do
-      generated_content_views = Katello::ContentView.where(generated_for: 'repository_export')
-
-      facets = Katello::Host::ContentFacet.joins(:content_view_environments)
-      facets = facets.where(katello_content_view_environments: { content_view_id: generated_content_views })
-      facets.find_each do |content_facet|
-        offending_cves = content_facet.content_view_environments.select { |cve| generated_content_views.include?(cve.content_view) }
-        valid_cves = content_facet.content_view_environments - offending_cves
+      generated_content_views = Katello::ContentView.generated_for_repository
 
+      facets = Katello::Host::ContentFacet.joins(:content_view_environments).
+                where(content_view_environments: { content_view: generated_content_views })
+      facets.all.each do |content_facet|
+        valid_cves = content_facet.content_view_environments.where.not(content_view: generated_content_views)
         if valid_cves.empty?
-          default_view = Katello::ContentView.find_by(name: "Default Organization View", organization: content_facet.host.organization)
-          if default_view
-            default_cve = default_view.content_view_environments.find_by(lifecycle_environment: content_facet.lifecycle_environments.first)
-            content_facet.update!(content_view_environments: [default_cve])
+          organization = content_facet.host.organization
+          default_cve = organization.content_view_environments.find_by(lifecycle_environment: organization.library,
+                                                                       content_view: organization.default_content_view)
+          if default_cve
+            FakeCVECF.where(content_facet_id: content_facet).delete_all
+            FakeCVECF.create!(content_facet_id: content_facet.id,
+                              content_view_environment_id: default_cve.id)
             say "Replaced all content views with Default Organization View for host #{content_facet.host.name}", true
           else
             say "No Default Organization View found for host #{content_facet.host.name}. Skipping.", true
           end
         else
-          content_facet.update!(content_view_environments: valid_cves)
+          FakeCVECF.where(content_facet_id: content_facet).
+                    where.not(content_view_environment_id: valid_cves).
+                    delete_all
           say "Removed offending content views for host #{content_facet.host.name}", true
         end
       end

[parthaa]

Closing this for #11144 (basically @wbclark 's PR with my changes)