Merge branch 'master' into cache-activate-product

Gemfile.lock

@@ -38,7 +38,7 @@ GEM
     awesome_print (1.9.2)
     base32 (0.3.4)
     base64 (0.2.0)
-    bigdecimal (3.1.8)
+    bigdecimal (3.1.9)
     builder (3.3.0)
     byebug (11.1.3)
     bzip2-ffi (1.1.1)
@@ -138,7 +138,7 @@ GEM
       guard (~> 2.1)
       guard-compat (~> 1.1)
       rspec (>= 2.99.0, < 4.0)
-    hashdiff (1.1.1)
+    hashdiff (1.1.2)
     hpricot (0.8.6)
     i18n (1.14.6)
       concurrent-ruby (~> 1.0)
@@ -213,7 +213,7 @@ GEM
     responders (3.1.1)
       actionpack (>= 5.2)
       railties (>= 5.2)
-    rexml (3.3.9)
+    rexml (3.4.0)
     ronn (0.7.3)
       hpricot (>= 0.8.2)
       mustache (>= 0.7.0)
@@ -320,7 +320,7 @@ GEM
       activesupport (>= 3)
       railties (>= 3)
       yard (~> 0.9.20)
-    webmock (3.24.0)
+    webmock (3.25.0)
       addressable (>= 2.8.0)
       crack (>= 0.3.2)
       hashdiff (>= 0.4.0, < 2.0.0)

engines/instance_verification/lib/instance_verification/engine.rb

@@ -103,6 +103,35 @@ def find_product
           product
         end
 
+        def find_subscription(base_product, logger, request)
+          # this method is needed because
+          # https://bugzilla.suse.com/show_bug.cgi?id=1236816
+          # https://bugzilla.suse.com/show_bug.cgi?id=1236836
+          product_hash = {
+            identifier: base_product.identifier,
+            version: base_product.version,
+            arch: base_product.arch,
+            release_type: base_product.release_type
+          }
+          add_on_product_class = InstanceVerification.provider.new(
+            logger,
+            request,
+            product_hash,
+            @system.instance_data
+          ).add_on
+          # add_on_product_class, if present, is the real product class
+          # i.e. in the case of SUMA, it would be SUMA product class
+          # not the SUMA base product's product class (Micro)
+          product_class = add_on_product_class.presence || base_product.product_class
+          # it returns the first subscription that matches
+          # even if there are more subscriptions that match
+          Subscription.joins(:product_classes).find_by(
+            subscription_product_classes: {
+              product_class: product_class
+            }
+          )
+        end
+
         def verify_product_activation
           product = find_product
 
@@ -133,12 +162,7 @@ def verify_payg_extension_activation!(product)
           return if product.free?
 
           base_product = @system.products.find_by(product_type: :base)
-          subscription = Subscription.joins(:product_classes).find_by(
-            subscription_product_classes: {
-              product_class: base_product.product_class
-            }
-          )
-
+          subscription = find_subscription(base_product, logger, request)
           # This error would occur only if there's a problem with subscription setup on SCC side
           raise InstanceVerification::Exception, "Can't find a subscription for base product #{base_product.product_string}" unless subscription
 
@@ -182,7 +206,8 @@ def verify_base_product_upgrade
 
           activated_bases = @system.products.where(product_type: 'base')
           activated_bases.each do |base_product|
-            return true if (base_product.identifier == upgrade_product.identifier)
+            base_product_subscription = find_subscription(base_product, logger, request)
+            return true if base_product_subscription && base_product_subscription.products.include?(upgrade_product)
           end
 
           raise ActionController::TranslatedError.new('Migration target not allowed on this instance type')

engines/instance_verification/lib/instance_verification/providers/example.rb

@@ -55,4 +55,10 @@ def allowed_extension?
     # to be activated on SCC or not, i.e. LTSS in Azure Basic VM
     true
   end
+
+  def add_on
+    # method to check if a system has an add on product
+    # based on the system metadata
+    # and if so, it returns its real product class
+  end
 end

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

@@ -277,6 +277,7 @@
           allow(InstanceVerification::Providers::Example).to receive(:new).and_return(plugin_double)
           allow(plugin_double).to receive(:parse_instance_data).and_return({ InstanceId: 'foo' })
           allow(plugin_double).to receive(:allowed_extension?).and_return(true)
+          allow(plugin_double).to receive(:add_on).and_return(nil)
 
           FactoryBot.create(:subscription, product_classes: product_classes)
           stub_request(:post, scc_activate_url)
@@ -590,6 +591,7 @@
           .and_return(plugin_double)
         allow(plugin_double).to receive(:parse_instance_data).and_return({ InstanceId: 'foo' })
         allow(plugin_double).to receive(:allowed_extension?).and_return(true)
+        allow(plugin_double).to receive(:add_on).and_return(nil)
 
         FactoryBot.create(:subscription, product_classes: product_classes)
         stub_request(:post, scc_activate_url)
@@ -656,18 +658,24 @@
           'User-Agent' => 'Ruby'
         }
       end
+      let(:plugin_double) { instance_double('InstanceVerification::Providers::Example') }
 
       context 'when SCC upgrade success' do
-        before do
-          stub_request(:put, scc_systems_products_url)
-            .with({ headers: scc_headers, body: payload.merge({ byos_mode: 'byos' }) })
-            .and_return(status: 201, body: '', headers: {})
-          request
-        end
+        let(:fake_subscription) { instance_double(Subscription, id: 1, products: [new_product]) }
 
         context "when migration target base product doesn't have an activated successor/predecessor" do
           let(:new_product) { FactoryBot.create(:product, :with_mirrored_repositories) }
 
+          before do
+            allow(InstanceVerification::Providers::Example).to receive(:new)
+              .and_return(plugin_double)
+            allow(plugin_double).to receive(:add_on).and_return('foo')
+            stub_request(:put, scc_systems_products_url)
+              .with({ headers: scc_headers, body: payload.merge({ byos_mode: 'byos' }) })
+              .and_return(status: 201, body: '', headers: {})
+            request
+          end
+
           it 'HTTP response code is 422' do
             expect(response).to have_http_status(422)
           end
@@ -686,6 +694,17 @@
               )
           end
 
+          before do
+            allow(InstanceVerification::Providers::Example).to receive(:new)
+              .and_return(plugin_double)
+            allow(plugin_double).to receive(:add_on).and_return('foo')
+            allow_any_instance_of(described_class).to receive(:find_subscription).and_return(fake_subscription)
+            stub_request(:put, scc_systems_products_url)
+              .with({ headers: scc_headers, body: payload.merge({ byos_mode: 'byos' }) })
+              .and_return(status: 201, body: '', headers: {})
+            request
+          end
+
           it 'HTTP response code is 201' do
             expect(response).to have_http_status(201)
           end
@@ -706,12 +725,13 @@
               body: 'Migration target not allowed on this instance type',
               headers: {}
             )
-          request
         end
 
         context "when migration target base product doesn't have an activated successor/predecessor" do
           let(:new_product) { FactoryBot.create(:product, :with_mirrored_repositories) }
 
+          before { request }
+
           it 'HTTP response code is 422' do
             expect(response).to have_http_status(422)
           end
@@ -729,8 +749,15 @@
               version: '999', predecessors: [ old_product ]
               )
           end
+          let(:fake_subscription) { instance_double(Subscription, id: 1, products: [new_product]) }
+
+          before do
+            allow_any_instance_of(described_class).to receive(:find_subscription).and_return(fake_subscription)
+            request
+          end
 
           it 'HTTP response code is 422' do
+            # problem here
             expect(response).to have_http_status(422)
           end
 
@@ -753,11 +780,11 @@
         }
       end
 
-      before { request }
-
       context "when migration target base product doesn't have an activated successor/predecessor" do
         let(:new_product) { FactoryBot.create(:product, :with_mirrored_repositories) }
 
+        before { request }
+
         it 'HTTP response code is 422' do
           expect(response).to have_http_status(422)
         end
@@ -776,6 +803,8 @@
             )
         end
 
+        before { request }
+
         it 'HTTP response code is 422' do
           expect(response).to have_http_status(422)
         end
@@ -793,6 +822,13 @@
             version: '999', predecessors: [ old_product ]
             )
         end
+        let(:fake_subscription) { instance_double(Subscription, id: 1, products: [new_product]) }
+
+        before do
+          allow_any_instance_of(described_class).to receive(:find_subscription).and_return(fake_subscription)
+
+          request
+        end
 
         it 'HTTP response code is 201' do
           expect(response).to have_http_status(201)