Fixes #30129 - Improve experience around expired and deleted manifests

app/controllers/katello/api/v2/api_controller.rb

@@ -211,10 +211,9 @@ def find_host_with_subscriptions(id, permission)
       fail HttpErrors::BadRequest, _("Host has not been registered with subscription-manager") if @host.subscription_facet.nil?
     end
 
-    def check_disconnected
-      msg = "You are currently operating in disconnected mode where access to Red Hat Subcription Management " \
-            "is prohibited. If you would like to change this, please update the content setting 'Disconnected mode'."
-      fail HttpErrors::BadRequest, _(msg) if Setting[:content_disconnected]
+    def check_upstream_connection
+      checker = Katello::UpstreamConnectionChecker.new(@organization)
+      checker.assert_connection
     end
   end
 end

app/controllers/katello/api/v2/subscriptions_controller.rb

@@ -7,7 +7,7 @@ class Api::V2::SubscriptionsController < Api::V2::ApiController
     before_action :find_optional_organization, :only => [:index, :available, :show]
     before_action :find_organization, :only => [:upload, :delete_manifest,
                                                 :refresh_manifest, :manifest_history]
-    before_action :check_disconnected, only: [:refresh_manifest]
+    before_action :check_upstream_connection, only: [:refresh_manifest]
     before_action :find_provider
 
     skip_before_action :check_content_type, :only => [:upload]

app/controllers/katello/api/v2/upstream_subscriptions_controller.rb

@@ -1,6 +1,7 @@
 module Katello
   class Api::V2::UpstreamSubscriptionsController < Api::V2::ApiController
-    before_action :check_disconnected
+    before_action :find_organization
+    before_action :check_upstream_connection
 
     resource_description do
       description "Red Hat subscriptions management platform."
@@ -65,6 +66,17 @@ def create
       respond_for_async resource: task
     end
 
+    api :GET, "/organizations/:organization_id/upstream_subscriptions/ping",
+      N_("Check if a connection can be made to Red Hat Subscription Management.")
+    def ping
+      # This API raises an error if:
+      # - Katello is in disconnected mode
+      # - There is no manifest imported
+      # - The local manifest identity certs have expired
+      # - The manifest has been deleted upstream
+      render json: { status: 'OK' }
+    end
+
     private
 
     def update_params

app/lib/katello/errors.rb

@@ -154,5 +154,18 @@ def message
         _("No URL found for a container registry. Please check the configuration.")
       end
     end
+
+    class DisconnectedMode < StandardError
+      def message
+        _("You are currently operating in disconnected mode where access to Red Hat Subcription Management " \
+        "is prohibited. If you would like to change this, please update the content setting 'Disconnected mode'.")
+      end
+    end
+
+    class ManifestExpired < StandardError
+      def message
+        _("This Organization's subscription manifest has expired. Please import a new manifest.")
+      end
+    end
   end
 end

app/lib/katello/resources/candlepin/upstream_consumer.rb

@@ -9,6 +9,12 @@ def path(id = upstream_consumer_id)
             super(id)
           end
 
+          def ping
+            resource.head
+          rescue RestClient::Unauthorized, RestClient::Gone
+            raise ::Katello::Errors::UpstreamConsumerGone
+          end
+
           # Overrides the HttpResource get method to check if the upstream
           # consumer exists.
           def get(params)

app/models/katello/concerns/organization_extensions.rb

@@ -82,6 +82,16 @@ def anonymous_provider
           self.providers.anonymous.first
         end
 
+        def manifest_expired?
+          manifest_expiry = owner_details.dig(:upstreamConsumer, :idCert, :serial, :expiration)
+
+          if manifest_expiry
+            DateTime.parse(manifest_expiry) < DateTime.now
+          else
+            false
+          end
+        end
+
         def manifest_history
           imports.map { |i| OpenStruct.new(i) }
         end

app/services/katello/ui_notifications/subscriptions/manifest_expired_warning.rb

@@ -7,14 +7,11 @@ class ManifestExpiredWarning < ::UINotifications::Base
         CDN_PATH = '/content/dist/rhel/server/7/listing'.freeze
 
         def self.deliver!(orgs = ::Organization.all)
+          return if Setting[:content_disconnected]
+
           orgs.each do |org|
-            next unless redhat_connected?(org)
-            content = org.contents.find_by(:label => CONTENT_LABEL)
-            product = content&.products&.find { |p| p.key }
-            if content && product && product.pools.any?
-              if got_403? { product.cdn_resource.get(CDN_PATH) }
-                new(org).deliver!
-              end
+            if cdn_inaccessible?(org) || upstream_inaccessible?(org)
+              new(org).deliver!
             end
           end
         rescue StandardError => e
@@ -47,15 +44,31 @@ def blueprint
           @blueprint ||= NotificationBlueprint.find_by(name: 'manifest_expired_warning')
         end
 
+        def self.cdn_inaccessible?(org)
+          content = org.contents.find_by(:label => CONTENT_LABEL)
+          product = content&.products&.find { |p| p.key }
+          if content && product && product.pools.any?
+            return got_403? { product.cdn_resource.get(CDN_PATH) }
+          end
+        end
+
         def self.got_403?
           yield
           false
         rescue Katello::Errors::SecurityViolation
           true
         end
 
+        def self.upstream_inaccessible?(org)
+          checker = Katello::UpstreamConnectionChecker.new(org)
+          checker.assert_connection
+          false
+        rescue Katello::Errors::ManifestExpired, Katello::Errors::UpstreamConsumerGone
+          true
+        end
+
         def self.redhat_connected?(org)
-          org.redhat_provider.repository_url.include?(CDN_HOSTNAME) && !Setting[:content_disconnected]
+          org.redhat_provider.repository_url.include?(CDN_HOSTNAME)
         end
       end
     end

app/services/katello/upstream_connection_checker.rb

@@ -0,0 +1,29 @@
+module Katello
+  class UpstreamConnectionChecker
+    def initialize(organization)
+      @organization = organization
+    end
+
+    def assert_connection
+      assert_connected
+      assert_unexpired_manifest
+      assert_can_upstream_ping
+
+      true
+    end
+
+    private
+
+    def assert_connected
+      fail Katello::Errors::DisconnectedMode if Setting[:content_disconnected]
+    end
+
+    def assert_can_upstream_ping
+      Katello::Resources::Candlepin::UpstreamConsumer.ping
+    end
+
+    def assert_unexpired_manifest
+      fail Katello::Errors::ManifestExpired if @organization.manifest_expired?
+    end
+  end
+end

config/routes/api/v2.rb

@@ -349,10 +349,12 @@ class ActionDispatch::Routing::Mapper
               put :refresh_manifest
             end
           end
+
           api_resources :upstream_subscriptions, only: [:index, :create] do
             collection do
               delete :destroy
               put :update
+              get :ping
             end
           end
         end

db/seeds.d/109-katello-notification-blueprints.rb

@@ -22,7 +22,7 @@
   {
     group: N_('Subscriptions'),
     name: 'manifest_expired_warning',
-    message: N_('The manifest imported within Organization %{subject} is no longer valid.  Attempted CDN access returned Forbidden.  Refreshing the manifest may resolve this issue.'),
+    message: N_('The manifest imported within Organization %{subject} is no longer valid. Please import a new manifest.'),
     level: 'warning'
   },
   {

lib/katello/permission_creator.rb

@@ -367,7 +367,7 @@ def subscription_permissions # rubocop:disable Metrics/MethodLength
                          :resource_type => 'Katello::Subscription'
       @plugin.permission :manage_subscription_allocations,
                          {
-                           'katello/api/v2/upstream_subscriptions' => [:index, :create, :destroy, :update]
+                           'katello/api/v2/upstream_subscriptions' => [:index, :create, :destroy, :update, :ping]
                          },
                          :resource_type => 'Katello::Subscription'
     end

test/controllers/api/v2/subscriptions_controller_test.rb

@@ -171,7 +171,8 @@ def test_upload_protected
       end
     end
 
-    def test_refresh_manfiest
+    def test_refresh_manifest
+      Katello::UpstreamConnectionChecker.any_instance.expects(:assert_connection)
       assert_async_task(::Actions::Katello::Organization::ManifestRefresh) do |organization|
         assert_equal(@organization, organization)
       end
@@ -180,6 +181,7 @@ def test_refresh_manfiest
     end
 
     def test_refresh_protected
+      Katello::UpstreamConnectionChecker.any_instance.expects(:assert_connection)
       allowed_perms = [@import_permission]
       denied_perms = [@attach_permission, @unattach_permission, @delete_permission, @read_permission]
 
@@ -188,12 +190,6 @@ def test_refresh_protected
       end
     end
 
-    def test_refresh_disconnected
-      Setting["content_disconnected"] = true
-      put :refresh_manifest, params: { :organization_id => @organization.id }
-      assert_response :bad_request
-    end
-
     def test_delete_manifest
       assert_async_task(::Actions::Katello::Organization::ManifestDelete) do |org|
         assert_equal @organization, org

test/controllers/api/v2/upstream_subscriptions_controller_test.rb

@@ -18,9 +18,7 @@ def setup
       setup_controller_defaults_api
       login_user(User.find(users(:admin).id))
       models
-
-      @organization.stubs(:owner_details)
-        .returns("upstreamConsumer" => {'uuid' => '', 'idCert' => {'key' => '', 'cert' => ''}})
+      Katello::UpstreamConnectionChecker.any_instance.expects(:assert_connection)
     end
 
     def test_index
@@ -69,12 +67,6 @@ def test_index_protected
       end
     end
 
-    def test_index_disconnected
-      Setting["content_disconnected"] = true
-      get :index, params: { organization_id: @organization.id }
-      assert_response :bad_request
-    end
-
     def test_destroy
       params = { pool_ids: %w(1 2 3), organization_id: @organization.id }
       assert_async_task ::Actions::Katello::UpstreamSubscriptions::RemoveEntitlements do |entitlement_ids|

test/services/katello/ui_notifications/subscriptions/manifest_expired_warning_test.rb

@@ -17,6 +17,7 @@ def setup
 
           cert = File.read(CERT_FIXTURE)
 
+          ::Organization.any_instance.stubs(:manifest_expired?).returns(false)
           Product.any_instance.stubs(:key).returns(cert)
           Product.any_instance.stubs(:certificate).returns(cert)
           FactoryBot.create(
@@ -31,6 +32,15 @@ def clear_notifications
           @product.organization.clear_manifest_expired_notifications
         end
 
+        def test_manifest_expired
+          ::Organization.any_instance.stubs(:manifest_expired?).returns(true)
+          @class.stubs(:cdn_inaccessible?).returns(false)
+
+          @class.deliver!([@product.organization])
+
+          assert_equal 1, NotificationBlueprint.find_by(name: 'manifest_expired_warning').notifications.count
+        end
+
         def test_with_failure
           Net::HTTP.any_instance.stubs(:request).raises(RestClient::Forbidden)
           @class.deliver!([@product.organization])

test/services/katello/upstream_connection_checker_test.rb

@@ -0,0 +1,44 @@
+require 'katello_test_helper'
+
+module Katello
+  class UpstreamConnectionCheckerTest < ActiveSupport::TestCase
+    def setup
+      @organization = get_organization
+      @checker = Katello::UpstreamConnectionChecker.new(@organization)
+    end
+
+    def test_all_good
+      Katello::Resources::Candlepin::UpstreamConsumer.expects(:ping).returns
+      @organization.expects(:manifest_expired?).returns(false)
+
+      assert @checker.assert_connection
+    end
+
+    def test_disconnected
+      Setting[:content_disconnected] = true
+
+      assert_raises Katello::Errors::DisconnectedMode do
+        @checker.assert_connection
+      end
+    ensure
+      Setting[:content_disconnected] = false
+    end
+
+    def test_manifest_expired
+      @organization.expects(:manifest_expired?).returns(true)
+
+      assert_raises Katello::Errors::ManifestExpired do
+        @checker.assert_connection
+      end
+    end
+
+    def test_upstream_ping
+      @organization.expects(:manifest_expired?).returns(false)
+      Katello::Resources::Candlepin::UpstreamConsumer.expects(:ping).raises(Katello::Errors::UpstreamConsumerGone)
+
+      assert_raises Katello::Errors::UpstreamConsumerGone do
+        @checker.assert_connection
+      end
+    end
+  end
+end

webpack/scenes/Subscriptions/SubscriptionActions.js

@@ -28,8 +28,9 @@ import {
   SUBSCRIPTIONS_ENABLE_DELETE_BUTTON,
   BLOCKING_FOREMAN_TASK_TYPES,
   SUBSCRIPTIONS_RESET_TASKS,
+  MANIFEST_DELETE_TASK_LABEL,
 } from './SubscriptionConstants';
-import { filterRHSubscriptions, selectSubscriptionsQuantitiesFromResponse } from './SubscriptionHelpers.js';
+import { selectSubscriptionsQuantitiesFromResponse } from './SubscriptionHelpers.js';
 import { apiError } from '../../move_to_foreman/common/helpers.js';
 import {
   startPollingTask,
@@ -39,6 +40,7 @@ import {
   toastTaskFinished,
 } from '../Tasks/TaskActions';
 import { selectIsPollingTasks } from '../Tasks/TaskSelectors';
+import { pingUpstreamSubscriptions } from './UpstreamSubscriptions/UpstreamSubscriptionsActions';
 
 export const createSubscriptionParams = (extendedParams = {}) => ({
   ...{
@@ -51,10 +53,8 @@ export const createSubscriptionParams = (extendedParams = {}) => ({
 export const loadAvailableQuantities = (extendedParams = {}) => async (dispatch) => {
   dispatch({ type: SUBSCRIPTIONS_QUANTITIES_REQUEST });
 
-  const params = createSubscriptionParams(extendedParams);
-
   try {
-    const { data } = await api.get(`/organizations/${orgId()}/upstream_subscriptions`, {}, params);
+    const { data } = await api.get(`/organizations/${orgId()}/upstream_subscriptions`, {}, propsToSnakeCase(extendedParams));
     return dispatch({
       type: SUBSCRIPTIONS_QUANTITIES_SUCCESS,
       payload: selectSubscriptionsQuantitiesFromResponse(data),
@@ -76,10 +76,6 @@ export const loadSubscriptions = (extendedParams = {}) => async (dispatch) => {
       response: data,
       search: extendedParams.search,
     });
-    const poolIds = filterRHSubscriptions(data.results).map(subs => subs.id);
-    if (poolIds.length > 0) {
-      dispatch(loadAvailableQuantities({ poolIds }));
-    }
     return result;
   } catch (error) {
     return dispatch(apiError(SUBSCRIPTIONS_FAILURE, error));
@@ -110,6 +106,10 @@ export const handleFinishedTask = task => (dispatch) => {
   dispatch(resetTasks());
   dispatch(pollTasks());
   dispatch(loadSubscriptions());
+
+  if (task.label !== MANIFEST_DELETE_TASK_LABEL) {
+    dispatch(pingUpstreamSubscriptions());
+  }
 };
 
 export const handleStartTask = task => (dispatch) => {