Fix tests

Co-authored-by: Pavel Busko <[email protected]>

Author: modulo11

app/controllers/runtime/buildpacks_controller.rb

@@ -7,12 +7,13 @@ def self.dependencies
     define_attributes do
       attribute :name, String
       attribute :stack, String, default: nil
+      attribute :lifecycle, String, default: Lifecycles::BUILDPACK, exclude_in: :update
       attribute :position, Integer, default: 0
       attribute :enabled, Message::Boolean, default: true
       attribute :locked, Message::Boolean, default: false
     end
 
-    query_parameters :name, :stack
+    query_parameters :name, :stack, :lifecycle
 
     def self.translate_validation_exception(e, attributes)
       buildpack_errors = e.errors.on(%i[name stack])

lib/cloud_controller/install_buildpacks.rb

@@ -12,8 +12,6 @@ def install(buildpacks)
       CloudController::DependencyLocator.instance.buildpack_blobstore.ensure_bucket_exists
       job_factory = VCAP::CloudController::Jobs::Runtime::BuildpackInstallerFactory.new
 
-      buildpack_install_jobs = []
-
       factory_options = []
       buildpacks.each do |bpack|
         buildpack_opts = bpack.deep_symbolize_keys
@@ -41,19 +39,10 @@ def install(buildpacks)
           logger.error "File not found: #{buildpack_file}, for the buildpack_opts: #{bpack}"
           next
         end
-
-        detected_stack = VCAP::CloudController::Buildpacks::StackNameExtractor.extract_from_file(buildpack_file) if buildpack_opts[:lifecycle] == Lifecycles::BUILDPACK
-        detected_stack = buildpack_opts[:stack] if buildpack_opts[:lifecycle] == Lifecycles::CNB
-
-        factory_options << { name: buildpack_name, file: buildpack_file, options: buildpack_opts, stack: detected_stack }
+        factory_options << { name: buildpack_name, file: buildpack_file, options: buildpack_opts, stack: detected_stack(buildpack_file, buildpack_opts) }
       end
 
-      buildpacks_by_lifecycle = factory_options.group_by { |options| options[:options][:lifecycle] }
-      buildpacks_by_lifecycle.each_value do |options|
-        options.group_by { |opts| opts[:name] }.each do |name, buildpack_options|
-          buildpack_install_jobs << job_factory.plan(name, buildpack_options)
-        end
-      end
+      buildpack_install_jobs = generate_install_jobs(factory_options, job_factory)
 
       buildpack_install_jobs.flatten!
       run_canary(buildpack_install_jobs)
@@ -66,6 +55,23 @@ def logger
 
     private
 
+    def generate_install_jobs(factory_options, job_factory)
+      buildpack_install_jobs = []
+      buildpacks_by_lifecycle = factory_options.group_by { |options| options[:options][:lifecycle] }
+      buildpacks_by_lifecycle.each_value do |options|
+        options.group_by { |opts| opts[:name] }.each do |name, buildpack_options|
+          buildpack_install_jobs << job_factory.plan(name, buildpack_options)
+        end
+      end
+      buildpack_install_jobs
+    end
+
+    def detected_stack(file, opts)
+      return opts[:stack] if opts[:lifecycle] == Lifecycles::CNB
+
+      VCAP::CloudController::Buildpacks::StackNameExtractor.extract_from_file(file)
+    end
+
     def buildpack_zip(package, zipfile)
       return zipfile if zipfile
 

spec/unit/controllers/runtime/buildpacks_controller_spec.rb

@@ -25,7 +25,8 @@ def ordered_buildpacks
                                                                                            stack: { type: 'string' },
                                                                                            position: { type: 'integer', default: 0 },
                                                                                            enabled: { type: 'bool', default: true },
-                                                                                           locked: { type: 'bool', default: false }
+                                                                                           locked: { type: 'bool', default: false },
+                                                                                           lifecycle: { type: 'string', default: 'buildpack' }
                                                                                          })
       end
 

spec/unit/controllers/v3/buildpacks_controller_spec.rb

@@ -512,8 +512,10 @@
     let(:buildpack_bits_name) { 'buildpack.zip' }
 
     before do
-      allow(File).to receive(:stat).and_return(stat_double)
-      # allow(VCAP::CloudController::BuildpackUpload).to receive(:new).and_return(uploader)
+      allow(File).to receive_messages(
+        stat: stat_double,
+        read: "PK\x03\x04".force_encoding('binary')
+      )
     end
 
     describe 'permissions' do

spec/unit/jobs/deserialization_spec.rb

@@ -178,6 +178,7 @@ module Jobs
                       - :lifecycle
                       extra_keys: []
                       lifecycle:
+                        :type: buildpack
                         :data:
                           :buildpacks:
                           - ruby

spec/unit/lib/cloud_controller/install_buildpacks_spec.rb

@@ -46,13 +46,13 @@ module VCAP::CloudController
           let(:buildpack2_file) { 'abuildpack2.zip' }
 
           let(:buildpack1a_fields) do
-            { name: 'buildpack1', file: buildpack1a_file, stack: 'cflinuxfs11', options: {} }
+            { name: 'buildpack1', file: buildpack1a_file, stack: 'cflinuxfs11', options: { lifecycle: Lifecycles::BUILDPACK } }
           end
           let(:buildpack1b_fields) do
-            { name: 'buildpack1', file: buildpack1b_file, stack: 'cflinuxfs12', options: {} }
+            { name: 'buildpack1', file: buildpack1b_file, stack: 'cflinuxfs12', options: { lifecycle: Lifecycles::BUILDPACK } }
           end
           let(:buildpack2_fields) do
-            { name: 'buildpack2', file: buildpack2_file, stack: nil, options: {} }
+            { name: 'buildpack2', file: buildpack2_file, stack: nil, options: { lifecycle: Lifecycles::BUILDPACK } }
           end
 
           before do
@@ -152,7 +152,7 @@ module VCAP::CloudController
           # call install
           # verify that job_factory.plan was called with the right file
           expect(File).to receive(:file?).with('another.zip').and_return(true)
-          expect(job_factory).to receive(:plan).with('buildpack1', [{ name: 'buildpack1', file: 'another.zip', stack: nil, options: {} }])
+          expect(job_factory).to receive(:plan).with('buildpack1', [{ name: 'buildpack1', file: 'another.zip', stack: nil, options: { lifecycle: Lifecycles::BUILDPACK } }])
 
           installer.install(TestConfig.config_instance.get(:install_buildpacks))
         end
@@ -166,7 +166,7 @@ module VCAP::CloudController
         it 'succeeds when no package is specified' do
           TestConfig.config[:install_buildpacks][0].delete('package')
           expect(File).to receive(:file?).with('another.zip').and_return(true)
-          expect(job_factory).to receive(:plan).with('buildpack1', [{ name: 'buildpack1', file: 'another.zip', stack: nil, options: {} }])
+          expect(job_factory).to receive(:plan).with('buildpack1', [{ name: 'buildpack1', file: 'another.zip', stack: nil, options: { lifecycle: Lifecycles::BUILDPACK } }])
 
           installer.install(TestConfig.config_instance.get(:install_buildpacks))
         end
@@ -224,6 +224,7 @@ module VCAP::CloudController
                     stack: nil,
                     options: {
                       enabled: true,
+                      lifecycle: Lifecycles::BUILDPACK,
                       locked: false,
                       position: 5
                     } }])

spec/unit/messages/buildpack_upload_message_spec.rb

@@ -9,7 +9,10 @@ module VCAP::CloudController
       let(:stat_double) { instance_double(File::Stat, size: 2) }
 
       before do
-        allow(File).to receive(:stat).and_return(stat_double)
+        allow(File).to receive_messages(
+          stat: stat_double,
+          read: "PK\x03\x04".force_encoding('binary')
+        )
       end
 
       context 'when the <ngnix_upload_module_dummy> param is set' do
@@ -93,12 +96,23 @@ module VCAP::CloudController
       end
 
       context 'when the file is not a zip' do
-        let(:opts) { { bits_path: '/tmp/bar', bits_name: 'buildpack.tgz' } }
+        let(:opts) { { bits_path: '/tmp/bar', bits_name: 'buildpack.abcd' } }
 
         it 'is not valid' do
+          allow(File).to receive(:read).and_return("PX\x03\x04".force_encoding('binary'))
           upload_message = BuildpackUploadMessage.new(opts)
           expect(upload_message).not_to be_valid
-          expect(upload_message.errors.full_messages[0]).to include('buildpack.tgz is not a zip')
+          expect(upload_message.errors.full_messages[0]).to include('buildpack.abcd is not a zip or gzip')
+        end
+      end
+
+      context 'when the file is a tgz' do
+        let(:opts) { { bits_path: '/tmp/bar', bits_name: 'buildpack.tgz' } }
+
+        it 'is not valid' do
+          allow(File).to receive(:read).and_return("\x1F\x8B\x08".force_encoding('binary'))
+          upload_message = BuildpackUploadMessage.new(opts)
+          expect(upload_message).to be_valid
         end
       end