Take mode into account when filtering for allowed_resources

Also don't check size limits in resource_known? as it is already done
before.

lib/cloud_controller/resource_match.rb

@@ -68,7 +68,9 @@ def after_match_log(time_by_bucket)
     end
 
     def allowed_resources
-      @allowed_resources ||= descriptors.select { |descriptor| resource_pool.size_allowed?(descriptor['size']) }
+      @allowed_resources ||= descriptors.select do |descriptor|
+        resource_pool.size_allowed?(descriptor['size']) && resource_pool.mode_allowed?(descriptor['mode'])
+      end
     end
 
     def logger

lib/cloud_controller/resource_pool.rb

@@ -92,9 +92,8 @@ def copy(descriptor, destination)
   end
 
   def resource_known?(descriptor)
-    size = descriptor['size']
     sha1 = descriptor['sha1']
-    if size_allowed?(size) && valid_sha?(sha1)
+    if valid_sha?(sha1)
       blobstore.exists?(sha1)
     end
   rescue => e
@@ -106,6 +105,10 @@ def size_allowed?(size)
     size && size > minimum_size && size < maximum_size
   end
 
+  def mode_allowed?(raw_mode)
+    raw_mode && raw_mode.to_i(8) >= 0600
+  end
+
   private
 
   def valid_sha?(sha1)

spec/support/shared_examples/controllers/resource_pool.rb

@@ -8,7 +8,7 @@
     @total_allowed_files =
       num_dirs * num_unique_allowed_files_per_dir * file_duplication_factor
 
-    @nonexisting_descriptor = { 'sha1' => Digester.new.digest('abc'), 'size' => 1 }
+    @nonexisting_descriptor = { 'sha1' => Digester.new.digest('abc'), 'size' => 1, 'mode' => '666' }
     @tmpdir = Dir.mktmpdir
 
     @descriptors = []
@@ -22,7 +22,8 @@
 
         descriptor = {
             'sha1' => Digester.new.digest(contents),
-            'size' => contents.length
+            'size' => contents.length,
+            'mode' => '666'
         }
         @descriptors << descriptor
 

spec/unit/lib/cloud_controller/resource_match_spec.rb

@@ -4,6 +4,20 @@ module VCAP::CloudController
   RSpec.describe ResourceMatch do
     include_context 'resource pool'
 
+    let(:descriptors) do
+      [
+        { 'sha1' => 0, 'size' => 500, 'mode' => '666' },
+        { 'sha1' => 1, 'size' => 1024, 'mode' => '666' },
+        { 'sha1' => 2, 'size' => 10_011, 'mode' => '666' },
+        { 'sha1' => 3, 'size' => 50_011, 'mode' => '666' },
+        { 'sha1' => 4, 'size' => 90_011, 'mode' => '666' },
+        { 'sha1' => 5, 'size' => 11_100_000, 'mode' => '666' },
+        { 'sha1' => 6, 'size' => 22_200_000, 'mode' => '666' },
+        { 'sha1' => 7, 'size' => 32_200_000, 'mode' => '666' },
+        { 'sha1' => 8, 'size' => 1_110_000_000, 'mode' => '666' },
+      ]
+    end
+
     describe '#match_resources' do
       before do
         @resource_pool.add_directory(@tmpdir)
@@ -24,33 +38,29 @@ module VCAP::CloudController
         expect(res).to eq(@descriptors)
       end
 
+      context 'one resource with a not allowed size and one with a not allowed mode' do
+        before do
+          # 'size_allowed?' and 'mode_allowed?' return 'false' once
+          allow(@resource_pool).to receive(:size_allowed?).and_return(false, true)
+          allow(@resource_pool).to receive(:mode_allowed?).and_return(false, true)
+        end
+
+        it 'returns two resources less' do
+          res = ResourceMatch.new(@descriptors).match_resources
+          expect(res.length).to eq(@descriptors.length - 2)
+        end
+      end
+
       it 'does not break when the sha1 is not long enough to generate a key' do
         expect do
-          ResourceMatch.new([{ 'sha1' => 0, 'size' => 123 }]).match_resources
-          ResourceMatch.new([{ 'sha1' => 'abc', 'size' => 234 }]).match_resources
+          ResourceMatch.new([{ 'sha1' => 0, 'size' => 123, 'mode' => '666' }]).match_resources
+          ResourceMatch.new([{ 'sha1' => 'abc', 'size' => 234, 'mode' => '666' }]).match_resources
         end.not_to raise_error
       end
 
       describe 'logging' do
         let(:maximum_file_size) { 500.megabytes } # this is arbitrary
         let(:minimum_file_size) { 3.kilobytes }
-        let(:descriptors) do
-          [
-            { 'sha1' => 0, 'size' => 500 },
-            { 'sha1' => 1, 'size' => 1024 },
-            { 'sha1' => 2, 'size' => 10_011 },
-            { 'sha1' => 3, 'size' => 50_011 },
-            { 'sha1' => 4, 'size' => 90_011 },
-            { 'sha1' => 5, 'size' => 11_100_000 },
-            { 'sha1' => 6, 'size' => 22_200_000 },
-            { 'sha1' => 7, 'size' => 32_200_000 },
-            { 'sha1' => 8, 'size' => 1_110_000_000 },
-          ]
-        end
-
-        it 'logs prior to matching all resources' do
-          ResourceMatch.new(descriptors).match_resources
-        end
 
         it 'correctly calculates and logs the time for each file size range' do
           Timecop.freeze
@@ -99,20 +109,6 @@ module VCAP::CloudController
       let(:maximum_file_size) { 500.megabytes } # this is arbitrary
       let(:minimum_file_size) { 3.kilobytes }
 
-      let(:descriptors) do
-        [
-          { 'sha1' => 0, 'size' => 500 },
-          { 'sha1' => 1, 'size' => 1024 },
-          { 'sha1' => 2, 'size' => 10_011 },
-          { 'sha1' => 3, 'size' => 50_011 },
-          { 'sha1' => 4, 'size' => 90_011 },
-          { 'sha1' => 5, 'size' => 11_100_000 },
-          { 'sha1' => 6, 'size' => 22_200_000 },
-          { 'sha1' => 7, 'size' => 32_200_000 },
-          { 'sha1' => 8, 'size' => 1_110_000_000 },
-        ]
-      end
-
       it 'correctly calculates the quantity of each file size range' do
         histogram = ResourceMatch.new(descriptors).resource_count_by_filesize
         # Filters out descriptors outside of range

spec/unit/lib/cloud_controller/resource_pool_spec.rb

@@ -24,7 +24,7 @@ module VCAP::CloudController
         @resource_pool.add_directory(@tmpdir)
 
         without_sizes = @descriptors.map do |d|
-          { 'sha1' => d['sha1'] }
+          { 'sha1' => d['sha1'], 'mode' => '666' }
         end
 
         res = @resource_pool.resource_sizes(without_sizes)
@@ -48,19 +48,29 @@ module VCAP::CloudController
       end
 
       it 'should return true for a size between min and max size' do
-        expect(@resource_pool.send(:size_allowed?, @minimum_size + 1)).to be true
+        expect(@resource_pool.size_allowed?(@minimum_size + 1)).to be true
       end
 
       it 'should return false for a size < min size' do
-        expect(@resource_pool.send(:size_allowed?, @minimum_size - 1)).to be false
+        expect(@resource_pool.size_allowed?(@minimum_size - 1)).to be false
       end
 
       it 'should return false for a size > max size' do
-        expect(@resource_pool.send(:size_allowed?, @maximum_size + 1)).to be false
+        expect(@resource_pool.size_allowed?(@maximum_size + 1)).to be false
       end
 
       it 'should return false for a nil size' do
-        expect(@resource_pool.send(:size_allowed?, nil)).to be nil
+        expect(@resource_pool.size_allowed?(nil)).to be nil
+      end
+    end
+
+    describe '#mode_allowed?' do
+      it 'should return true for a mode >= 600' do
+        expect(@resource_pool.mode_allowed?('666')).to be true
+      end
+
+      it 'should return false for a mode < 600' do
+        expect(@resource_pool.mode_allowed?('444')).to be false
       end
     end