Merge pull request #1278 from SUSE/cache-activate-product

Add cache check when activating a product with SCC

engines/instance_verification/config/environments/test.rb

@@ -5,6 +5,12 @@
 
 Rails.application.configure do
   config.cache_config_file = Rails.root.join('engines/registry/spec/data/rmt-cache-trim.sh')
-  config.repo_cache_dir = 'repo/cache'
+  config.repo_payg_cache_dir = 'repo/payg/cache'
+  config.repo_byos_cache_dir = 'repo/byos/cache'
+  config.repo_hybrid_cache_dir = 'repo/hybrid/cache'
   config.registry_cache_dir = 'registry/cache'
+  config.expire_repo_payg_cache = 20
+  config.expire_repo_byos_cache = 1440 # 24h in minutes
+  config.expire_repo_hybrid_cache = 1440 # 24h in minutes
+  config.expire_registry_cache = 840 # 8h in minutes
 end

engines/instance_verification/lib/instance_verification/engine.rb

@@ -1,25 +1,75 @@
+require 'base64'
 require 'fileutils'
 
 module InstanceVerification
-  def self.update_cache(remote_ip, system_login, product_id, registry: false)
-    # TODO: BYOS scenario
-    # to be addressed on a different PR
+  def self.update_cache(cache_entry, mode, registry: false)
     unless registry
-      InstanceVerification.write_cache_file(
-        Rails.application.config.repo_cache_dir,
-        [remote_ip, system_login, product_id].join('-')
-      )
+      cache_path = InstanceVerification.get_cache_path(mode)
+      InstanceVerification.write_cache_file(cache_path, cache_entry)
     end
 
+    # update the registry cache every time
     InstanceVerification.write_cache_file(
-      Rails.application.config.registry_cache_dir,
-      [remote_ip, system_login].join('-')
+      InstanceVerification.get_cache_path('registry'),
+      cache_entry
     )
   end
 
+  def self.build_cache_entry(remote_ip, system_login, encoded_reg_code, mode, product)
+    if mode == 'payg'
+      [remote_ip, system_login, product.id].join('-')
+    elsif mode == 'registry'
+      [remote_ip, system_login].join('-')
+    else
+      # for byos or hybrid cache
+      product_hash = product.attributes.symbolize_keys.slice(:identifier, :version, :arch)
+      product_triplet = "#{product_hash[:identifier]}_#{product_hash[:version]}_#{product_hash[:arch]}"
+      "#{encoded_reg_code}-#{product_triplet}-active"
+    end
+  end
+
   def self.write_cache_file(cache_dir, cache_key)
     FileUtils.mkdir_p(cache_dir)
     FileUtils.touch(File.join(cache_dir, cache_key))
+    Rails.logger.info "#{cache_dir} updated for #{cache_key}"
+  end
+
+  def self.get_cache_path(mode)
+    if mode == 'byos'
+      Rails.application.config.repo_byos_cache_dir
+    elsif mode == 'hybrid'
+      Rails.application.config.repo_hybrid_cache_dir
+    elsif mode == 'payg'
+      Rails.application.config.repo_payg_cache_dir
+    else
+      Rails.application.config.registry_cache_dir
+    end
+  end
+
+  def self.get_cache_entries(mode)
+    cache_path = InstanceVerification.get_cache_path(mode)
+    Dir.children(cache_path)
+  rescue SystemCallError
+    Rails.logger.info "#{cache_path} does not exist"
+    []
+  end
+
+  def self.reg_code_in_cache?(cache_key, mode)
+    cache_entries = InstanceVerification.get_cache_entries(mode)
+    cache_entries.find { |cache_entry| cache_entry.include?(cache_key) }
+  end
+
+  def self.remove_entry_from_cache(cache_key, mode)
+    cache_path = InstanceVerification.get_cache_path(mode)
+    full_path_cache_key = File.join(cache_path, cache_key)
+    File.unlink(full_path_cache_key) if File.exist?(full_path_cache_key)
+  end
+
+  def self.set_cache_inactive(cache_key, mode)
+    InstanceVerification.remove_entry_from_cache(cache_key, mode)
+    *all, _ = cache_key.split('-')
+    cache_key = [all, 'inactive'].join('-')
+    InstanceVerification.update_cache(cache_key, mode)
   end
 
   class Engine < ::Rails::Engine
@@ -124,7 +174,8 @@ def verify_payg_extension_activation!(product)
             )
           end
           logger.info "Product #{@product.product_string} available for this instance"
-          InstanceVerification.update_cache(request.remote_ip, @system.login, product.id)
+          cache_key = InstanceVerification.build_cache_entry(request.remote_ip, @system.login, nil, 'payg', product)
+          InstanceVerification.update_cache(cache_key, 'payg')
         end
 
         def verify_base_product_activation(product)
@@ -136,7 +187,16 @@ def verify_base_product_activation(product)
           )
 
           raise 'Unspecified error' unless verification_provider.instance_valid?
-          InstanceVerification.update_cache(request.remote_ip, @system.login, product.id)
+
+          encoded_reg_code = @system.pubcloud_reg_code
+          # we use the token sent from the client if present
+          # instead of the value stored in the DB
+          encoded_reg_code = Base64.strict_encode64(params[:token]) if params[:token].present?
+
+          cache_key = InstanceVerification.build_cache_entry(
+            request.remote_ip, @system.login, encoded_reg_code, @system.proxy_byos_mode, product
+          )
+          InstanceVerification.update_cache(cache_key, @system.proxy_byos_mode)
         end
 
         # Verify that the base product doesn't change in the offline migration

engines/instance_verification/spec/requests/api/connect/v3/systems/products_controller_spec.rb

@@ -1,3 +1,4 @@
+require 'base64'
 require 'rails_helper'
 
 # rubocop:disable RSpec/NestedGroups
@@ -10,13 +11,21 @@
   let(:product) { FactoryBot.create(:product, :product_sles, :with_mirrored_repositories, :with_mirrored_extensions) }
   let(:product_sap) { FactoryBot.create(:product, :product_sles_sap, :with_mirrored_repositories, :with_mirrored_extensions) }
   let(:scc_activate_url) { 'https://scc.suse.com/connect/systems/products' }
-  let(:payload) do
+  let(:expected_payload) do
     {
       identifier: product.identifier,
       version: product.version,
       arch: product.arch
     }
   end
+  let(:payload) do
+    {
+      identifier: product.identifier,
+      version: product.version,
+      arch: product.arch,
+      token: 'super_reg_code'
+    }
+  end
 
   describe '#activate' do
     let(:plugin_double) { instance_double('InstanceVerification::Providers::Example') }
@@ -38,11 +47,38 @@
 
         it 'class instance verification provider' do
           expect(InstanceVerification::Providers::Example).to receive(:new)
-            .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), payload, nil).and_call_original.at_least(:once)
+            .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), expected_payload, nil).and_call_original.at_least(:once)
+          allow(Dir).to receive(:mkdir)
+          allow(FileUtils).to receive(:touch)
+          post url, params: payload, headers: headers
+        end
+      end
+
+      context "when system doesn't have hw_info and cache is inactive" do
+        let(:system) { FactoryBot.create(:system, :byos, pubcloud_reg_code: Base64.strict_encode64('super_token')) }
+
+        before do
+          stub_request(:post, 'https://scc.suse.com/connect/systems/products')
+            .to_return(
+              status: 201,
+              body: { ok: 'ok' }.to_json,
+              headers: {}
+            )
+        end
+
+        it 'class instance verification provider' do
+          expect(InstanceVerification::Providers::Example).to receive(:new)
+            .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), expected_payload, nil).and_call_original.at_least(:once)
           allow(File).to receive(:directory?)
           allow(Dir).to receive(:mkdir)
+          allow(Dir).to receive(:children)
           allow(FileUtils).to receive(:touch)
+          allow(InstanceVerification).to receive(:reg_code_in_cache?).and_return(
+            "#{system.pubcloud_reg_code}-inactive"
+          )
           post url, params: payload, headers: headers
+          expect(response.body).to include('Subscription inactive')
+          expect(response).to have_http_status(403)
         end
       end
 
@@ -94,6 +130,10 @@
 
         context 'when verification provider raises an unhandled exception' do
           before do
+            expect(InstanceVerification::Providers::Example).to receive(:new).and_return(plugin_double)
+            allow(plugin_double).to receive(:allowed_extension?).and_return(true)
+            allow(plugin_double).to receive(:instance_valid?).and_raise('not expected')
+            allow(InstanceVerification).to receive(:set_cache_inactive).and_return(nil)
             stub_request(:post, scc_activate_url)
             .to_return(
               status: 422,
@@ -118,7 +158,7 @@
 
         it 'class instance verification provider' do
           expect(InstanceVerification::Providers::Example).to receive(:new)
-            .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), payload, nil).and_call_original.at_least(:once)
+            .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), expected_payload, nil).and_call_original.at_least(:once)
           allow(File).to receive(:directory?)
           allow(Dir).to receive(:mkdir)
           allow(FileUtils).to receive(:touch)
@@ -146,7 +186,7 @@
         context 'when verification provider returns false' do
           before do
             expect(InstanceVerification::Providers::Example).to receive(:new)
-              .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), payload, instance_data).and_return(plugin_double).at_least(:once)
+              .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), expected_payload, instance_data).and_return(plugin_double).at_least(:once)
             expect(plugin_double).to receive(:instance_valid?).and_return(false)
             allow(plugin_double).to receive(:allowed_extension?).and_return(true)
             post url, params: payload, headers: headers
@@ -161,7 +201,7 @@
         context 'when verification provider raises an unhandled exception' do
           before do
             expect(InstanceVerification::Providers::Example).to receive(:new)
-              .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), payload, instance_data).and_return(plugin_double).at_least(:once)
+              .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), expected_payload, instance_data).and_return(plugin_double).at_least(:once)
             expect(plugin_double).to receive(:instance_valid?).and_raise('Custom plugin error')
             allow(plugin_double).to receive(:allowed_extension?).and_return(true)
             post url, params: payload, headers: headers
@@ -178,7 +218,7 @@
 
           before do
             expect(InstanceVerification::Providers::Example).to receive(:new)
-              .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), payload, instance_data).and_return(plugin_double).at_least(:once)
+              .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), expected_payload, instance_data).and_return(plugin_double).at_least(:once)
             expect(plugin_double).to receive(:instance_valid?).and_raise(InstanceVerification::Exception, 'Custom plugin error')
             allow(plugin_double).to receive(:allowed_extension?).and_return(true)
             post url, params: payload, headers: headers
@@ -277,7 +317,13 @@
         let(:instance_data) { 'dummy_instance_data' }
         let(:system) do
           FactoryBot.create(
-            :system, :payg, :with_system_information, :with_activated_product, product: base_product, instance_data: instance_data
+            :system,
+            :payg,
+            :with_system_information,
+            :with_activated_product,
+            product: base_product,
+            instance_data: instance_data,
+            pubcloud_reg_code: Base64.strict_encode64('super_token_different')
           )
         end
         let(:serialized_service_json) do
@@ -288,7 +334,7 @@
         end
         let(:scc_activate_url) { 'https://scc.suse.com/connect/systems/products' }
         let(:plugin_double) { instance_double('InstanceVerification::Providers::Example') }
-        let(:payload_no_token) do
+        let(:payload_token) do
           {
             identifier: product.identifier,
             version: product.version,
@@ -345,14 +391,22 @@
               'User-Agent' => 'Ruby'
             }
           end
+          let(:cache_entry) do
+            product_hash = product.attributes.symbolize_keys.slice(:identifier, :version, :arch)
+            product_triplet = "#{product_hash[:identifier]}_#{product_hash[:version]}_#{product_hash[:arch]}"
+            "#{Base64.strict_encode64(payload_token[:token])}-#{product_triplet}-active"
+          end
 
           before do
             allow(InstanceVerification::Providers::Example).to receive(:new).and_return(plugin_double)
             allow(plugin_double).to receive(:instance_identifier).and_return('foo')
             allow(plugin_double).to receive(:parse_instance_data).and_return({ InstanceId: 'foo' })
             allow(plugin_double).to receive(:allowed_extension?).and_return(true)
 
-            allow(InstanceVerification).to receive(:update_cache).with('127.0.0.1', system.login, product.id)
+            allow(InstanceVerification).to receive(:update_cache).with("127.0.0.1-#{system.login}-#{product.id}", 'payg')
+            allow(InstanceVerification).to receive(:get_cache_entries).and_return(
+              [File.join(Rails.application.config.repo_hybrid_cache_dir, cache_entry)]
+            )
             FactoryBot.create(:subscription, product_classes: product_classes)
             stub_request(:post, scc_activate_url)
               .to_return(
@@ -366,14 +420,18 @@
               .with({ headers: scc_announce_headers, body: scc_annouce_body.to_json })
               .to_return(status: 201, body: scc_response_body, headers: {})
 
-            expect(InstanceVerification).to receive(:update_cache).with('127.0.0.1', system.login, product.id)
+            expect(InstanceVerification).to receive(:update_cache).with(cache_entry, 'hybrid')
 
-            post url, params: payload_no_token, headers: headers
+            post url, params: payload_token, headers: headers
           end
 
           context 'when regcode is provided' do
             it 'returns service JSON' do
               expect(response.body).to eq(serialized_service_json)
+              updated_system = System.find_by(login: system.login)
+              expect(updated_system.pubcloud_reg_code).to include(',')
+              expect(updated_system.pubcloud_reg_code).to include(Base64.strict_encode64(payload_token[:token]).to_s)
+              expect(updated_system.pubcloud_reg_code).to include(system.pubcloud_reg_code)
             end
           end
         end
@@ -393,7 +451,7 @@
             allow(plugin_double).to receive(:parse_instance_data).and_return({ InstanceId: 'foo' })
             allow(plugin_double).to receive(:allowed_extension?).and_return(true)
 
-            allow(InstanceVerification).to receive(:update_cache).with('127.0.0.1', system.login, product.id)
+            allow(InstanceVerification).to receive(:update_cache).with("127.0.0.1-#{system.login}-#{product.id}", 'foo')
             FactoryBot.create(:subscription, product_classes: product_classes)
             stub_request(:post, scc_activate_url)
               .to_return(
@@ -406,9 +464,9 @@
             stub_request(:post, 'https://scc.suse.com/connect/subscriptions/systems')
               .to_return(status: 201, body: scc_response_body, headers: {})
 
-            expect(InstanceVerification).not_to receive(:update_cache).with('127.0.0.1', system.login, product.id)
+            expect(InstanceVerification).not_to receive(:update_cache) # .with('127.0.0.1', system.login, product.id)
             allow(plugin_double).to receive(:allowed_extension?).and_return(true)
-            post url, params: payload_no_token, headers: headers
+            post url, params: payload_token, headers: headers
           end
 
           it 'returns service JSON' do
@@ -456,7 +514,7 @@
         context 'when verification provider returns false' do
           before do
             expect(InstanceVerification::Providers::Example).to receive(:new)
-              .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), payload, instance_data).and_return(plugin_double).at_least(:once)
+              .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), expected_payload, instance_data).and_return(plugin_double).at_least(:once)
             allow(plugin_double).to receive(:allowed_extension?).and_return(true)
             expect(plugin_double).to receive(:instance_valid?).and_return(false)
             post url, params: payload, headers: headers
@@ -471,7 +529,7 @@
         context 'when verification provider raises an unhandled exception' do
           before do
             expect(InstanceVerification::Providers::Example).to receive(:new)
-              .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), payload, instance_data).and_return(plugin_double).at_least(:once)
+              .with(be_a(ActiveSupport::Logger), be_a(ActionDispatch::Request), expected_payload, instance_data).and_return(plugin_double).at_least(:once)
             allow(plugin_double).to receive(:allowed_extension?).and_return(true)
             expect(plugin_double).to receive(:instance_valid?).and_raise('Custom plugin error')
             post url, params: payload, headers: headers

engines/registry/lib/registry/engine.rb

@@ -1,10 +1,8 @@
 module Registry
+  # rubocop:disable Lint/EmptyClass
   class << self
-    def remove_auth_cache(registry_cache_key)
-      cache_path = File.join(Rails.application.config.registry_cache_dir, registry_cache_key)
-      File.unlink(cache_path) if File.exist?(cache_path)
-    end
   end
+  # rubocop:enable Lint/EmptyClass
 
   class Engine < ::Rails::Engine
     isolate_namespace Registry
@@ -27,8 +25,8 @@ def refresh_auth_cache
         before_action :remove_auth_cache, only: %w[deregister]
 
         def remove_auth_cache
-          registry_cache_key = [request.remote_ip, @system.login].join('-')
-          Registry.remove_auth_cache(registry_cache_key)
+          registry_cache_key = InstanceVerification.build_cache_entry(request.remote_ip, @system.login, @system.pubcloud_reg_code, 'registry', nil)
+          InstanceVerification.remove_entry_from_cache(registry_cache_key, 'registry')
         end
       end
     end