Refactor permissions functions

.rubocop.yml

@@ -196,6 +196,9 @@ Style/BlockDelimiters:
 Style/ClassAndModuleChildren:
   Enabled: false
 
+Style/ConditionalAssignment:
+  Enabled: false
+
 Style/Documentation:
   Enabled: false
 

Gemfile

@@ -85,7 +85,7 @@ group :test do
   gem 'rspec-rails', '~> 5.0.2'
   gem 'rspec-wait'
   gem 'rspec_api_documentation', '>= 6.1.0'
-  gem 'rubocop', '~> 1.18.3'
+  gem 'rubocop', '~> 1.19.1'
   gem 'timecop'
   gem 'webmock', '> 2.3.1'
 end

Gemfile.lock

@@ -262,7 +262,7 @@ GEM
       addressable (~> 2.0)
       excon
       http (>= 2.0, < 5.0)
-    listen (3.6.0)
+    listen (3.7.0)
       rb-fsevent (~> 0.10, >= 0.10.3)
       rb-inotify (~> 0.9, >= 0.9.10)
     loggregator_emitter (5.2.0)
@@ -404,13 +404,13 @@ GEM
       activesupport (>= 3.0.0)
       mustache (~> 1.0, >= 0.99.4)
       rspec (~> 3.0)
-    rubocop (1.18.3)
+    rubocop (1.19.1)
       parallel (~> 1.10)
       parser (>= 3.0.0.0)
       rainbow (>= 2.2.2, < 4.0)
       regexp_parser (>= 1.8, < 3.0)
       rexml
-      rubocop-ast (>= 1.7.0, < 2.0)
+      rubocop-ast (>= 1.9.1, < 2.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (>= 1.4.0, < 3.0)
     rubocop-ast (1.10.0)
@@ -572,7 +572,7 @@ DEPENDENCIES
   rspec-rails (~> 5.0.2)
   rspec-wait
   rspec_api_documentation (>= 6.1.0)
-  rubocop (~> 1.18.3)
+  rubocop (~> 1.19.1)
   ruby-debug-ide (>= 0.7.0.beta4)
   rubyzip (>= 1.3.0)
   sequel (~> 5.47)

app/actions/app_delete.rb

@@ -38,11 +38,7 @@ def delete(apps, record_event: true)
       apps.each do |app|
         logger.info("Deleting app: #{app.guid}")
 
-        errs = delete_non_transactional_subresources(app)
-
-        if errs&.any?
-          raise SubResourceError.new(errs)
-        end
+        delete_non_transactional_subresources(app)
 
         app.db.transaction do
           app.lock!
@@ -104,10 +100,7 @@ def delete_subresources(app)
 
     def delete_non_transactional_subresources(app)
       errors = delete_bindings(app.service_bindings, user_audit_info: @user_audit_info)
-      non_async_errors = errors.reject { |e| e.is_a?(AsyncBindingDeletionsTriggered) }
-      raise non_async_errors.first unless non_async_errors.empty?
-
-      errors
+      raise SubResourceError.new(errors) if errors.any?
     end
 
     def stagers

app/actions/update_route_destinations.rb

@@ -12,6 +12,7 @@ class << self
       def add(new_route_mappings, route, apps_hash, user_audit_info, manifest_triggered: false)
         existing_route_mappings = route_to_mapping_hashes(route)
         new_route_mappings = update_port(new_route_mappings, apps_hash)
+        new_route_mappings = update_protocol(new_route_mappings, route)
         new_route_mappings = add_route(new_route_mappings, route)
 
         validate_max_destinations!(existing_route_mappings, new_route_mappings)
@@ -21,14 +22,15 @@ def add(new_route_mappings, route, apps_hash, user_audit_info, manifest_triggere
           raise Error.new('Destinations cannot be inserted when there are weighted destinations already configured.')
         end
 
-        to_add = new_route_mappings - existing_route_mappings
+        to_add = compare_destination_hashes(new_route_mappings, existing_route_mappings)
 
         update(route, to_add, [], user_audit_info, manifest_triggered)
       end
 
       def replace(new_route_mappings, route, apps_hash, user_audit_info, manifest_triggered: false)
         existing_route_mappings = route_to_mapping_hashes(route)
         new_route_mappings = update_port(new_route_mappings, apps_hash)
+        new_route_mappings = update_protocol(new_route_mappings, route)
         new_route_mappings = add_route(new_route_mappings, route)
 
         validate_max_destinations!([], new_route_mappings)
@@ -57,7 +59,8 @@ def update(route, to_add, to_delete, user_audit_info, manifest_triggered)
           processes_to_ports_map = {}
 
           to_delete.each do |rm|
-            route_mapping = RouteMappingModel.find(rm)
+            rm.delete(:protocol)
+            route_mapping = RouteMappingModel[rm]
             route_mapping.destroy
 
             Copilot::Adapter.unmap_route(route_mapping)
@@ -76,6 +79,8 @@ def update(route, to_add, to_delete, user_audit_info, manifest_triggered)
           end
 
           to_add.each do |rm|
+            validate_protocol_matches(rm)
+
             route_mapping = RouteMappingModel.new(rm)
             route_mapping.save
 
@@ -126,6 +131,23 @@ def update_processes(processes_to_ports_map)
         raise Error.new(e.message)
       end
 
+      def compare_destination_hashes(new_route_mappings, existing_route_mappings)
+        new_route_mappings.reject do |new|
+          matching_route_mapping = existing_route_mappings.find do |existing|
+            new[:app_guid] == existing[:app_guid] &&
+              new[:process_type] == existing[:process_type] &&
+              new[:route_guid] == existing[:route_guid] &&
+              new[:app_port] == existing[:app_port]
+          end
+
+          if matching_route_mapping && matching_route_mapping[:protocol] != new[:protocol]
+            raise Error.new('Destination exists with conflicting protocol')
+          end
+
+          matching_route_mapping
+        end
+      end
+
       def add_route(destinations, route)
         destinations.map do |dst|
           dst.merge({ route: route, route_guid: route.guid })
@@ -142,6 +164,14 @@ def update_port(destinations, apps_hash)
         end
       end
 
+      def update_protocol(destinations, route)
+        destinations.each do |dst|
+          if dst[:protocol].nil?
+            dst[:protocol] = RouteMappingModel::DEFAULT_PROTOCOL_MAPPING[route.protocol]
+          end
+        end
+      end
+
       def default_port(app)
         if app.buildpack?
           ProcessModel::DEFAULT_HTTP_PORT
@@ -163,7 +193,8 @@ def destination_to_mapping_hash(route, destination)
           process_type: destination.process_type,
           app_port: destination.app_port,
           route: route,
-          weight: destination.weight
+          weight: destination.weight,
+          protocol: destination.protocol_without_defaults
         }
       end
 
@@ -175,6 +206,21 @@ def validate_unique!(new_route_mappings)
         raise DuplicateDestinationError.new('Destinations cannot contain duplicate entries') if new_route_mappings.any? { |rm| new_route_mappings.count(rm) > 1 }
       end
 
+      def validate_protocol_matches(route_destination)
+        destination_protocol = route_destination[:protocol]
+        return unless destination_protocol
+
+        route_protocol = route_destination[:route].protocol
+        dest_to_route_map = { 'tcp' => 'tcp', 'http1' => 'http', 'http2' => 'http' }
+
+        raise protocol_mismatch_error(route_protocol, destination_protocol) if route_protocol != dest_to_route_map[destination_protocol]
+      end
+
+      def protocol_mismatch_error(route_protocol, destination_protocol)
+        valid_options = { 'tcp' => 'tcp', 'http' => 'http1, http2' }
+        Error.new("Cannot use '#{destination_protocol}' protocol for #{route_protocol} routes; valid options are: [#{valid_options[route_protocol]}].")
+      end
+
       def validate_max_destinations!(existing_route_mappings, new_route_mappings)
         total_route_mapping_count = existing_route_mappings.count + new_route_mappings.count
 

app/controllers/internal/app_crashed_controller.rb

@@ -19,7 +19,7 @@ def crashed(process_guid)
       crash_payload['version'] = Diego::ProcessGuid.cc_process_version(process_guid)
 
       Repositories::ProcessEventRepository.record_crash(process, crash_payload)
-      Repositories::AppEventRepository.new.create_app_exit_event(process, crash_payload)
+      Repositories::AppEventRepository.new.create_app_crash_event(process, crash_payload)
     end
 
     private

app/controllers/internal/app_rescheduling_controller.rb

@@ -0,0 +1,39 @@
+require 'sinatra'
+require 'controllers/base/base_controller'
+require 'cloud_controller/internal_api'
+
+module VCAP::CloudController
+  class AppReschedulingController < RestController::BaseController
+    # Endpoint does its own (non-standard) auth
+    allow_unauthenticated_access
+
+    post '/internal/v4/apps/:process_guid/rescheduling', :rescheduling
+    def rescheduling(process_guid)
+      rescheduling_payload = rescheduling_request
+
+      app_guid = Diego::ProcessGuid.cc_process_guid(process_guid)
+
+      process = ProcessModel.find(guid: app_guid)
+      raise CloudController::Errors::NotFound.new_from_details('ProcessNotFound', app_guid) unless process
+
+      rescheduling_payload['version'] = Diego::ProcessGuid.cc_process_version(process_guid)
+
+      Repositories::ProcessEventRepository.record_rescheduling(process, rescheduling_payload)
+    end
+
+    private
+
+    def rescheduling_request
+      rescheduling = {}
+      begin
+        payload = body.read
+        rescheduling = MultiJson.load(payload)
+      rescue MultiJson::ParseError => pe
+        logger.error('diego.app_rescheduling.parse-error', payload: payload, error: pe.to_s)
+        raise CloudController::Errors::ApiError.new_from_details('MessageParseError', payload)
+      end
+
+      rescheduling
+    end
+  end
+end

app/controllers/v3/app_features_controller.rb

@@ -17,7 +17,7 @@ class AppFeaturesController < ApplicationController
 
   def index
     app, space, org = AppFetcher.new.fetch(hashed_params[:app_guid])
-    app_not_found! unless app && permission_queryer.untrusted_can_read_from_space?(space.guid, org.guid)
+    app_not_found! unless app && permission_queryer.can_read_from_space?(space.guid, org.guid)
     resources = presented_app_features(app)
 
     render status: :ok, json: {
@@ -28,7 +28,7 @@ def index
 
   def show
     app, space, org = AppFetcher.new.fetch(hashed_params[:app_guid])
-    app_not_found! unless app && permission_queryer.untrusted_can_read_from_space?(space.guid, org.guid)
+    app_not_found! unless app && permission_queryer.can_read_from_space?(space.guid, org.guid)
     resource_not_found!(:feature) unless APP_FEATURES.include?(hashed_params[:name])
 
     render status: :ok, json: feature_presenter_for(hashed_params[:name], app)
@@ -37,12 +37,12 @@ def show
   def update
     app, space, org = AppFetcher.new.fetch(hashed_params[:app_guid])
 
-    app_not_found! unless app && permission_queryer.untrusted_can_read_from_space?(space.guid, org.guid)
+    app_not_found! unless app && permission_queryer.can_read_from_space?(space.guid, org.guid)
 
     name = hashed_params[:name]
     resource_not_found!(:feature) unless APP_FEATURES.include?(name)
     if UNTRUSTED_APP_FEATURES.include?(name)
-      unauthorized! unless permission_queryer.untrusted_can_write_to_space?(space.guid)
+      unauthorized! unless permission_queryer.can_manage_apps_in_space?(space.guid)
     else
       unauthorized! unless permission_queryer.can_write_to_space?(space.guid)
     end
@@ -57,7 +57,7 @@ def update
   def ssh_enabled
     app, space, org = AppFetcher.new.fetch(hashed_params[:guid])
 
-    app_not_found! unless app && permission_queryer.untrusted_can_read_from_space?(space.guid, org.guid)
+    app_not_found! unless app && permission_queryer.can_read_from_space?(space.guid, org.guid)
 
     render status: :ok, json: Presenters::V3::AppSshStatusPresenter.new(app, Config.config.get(:allow_app_ssh_access))
   end

app/controllers/v3/app_manifests_controller.rb

@@ -8,7 +8,7 @@ class AppManifestsController < ApplicationController
   def show
     app, space, org = AppFetcher.new.fetch(hashed_params[:guid])
 
-    app_not_found! unless app && permission_queryer.untrusted_can_read_from_space?(space.guid, org.guid)
+    app_not_found! unless app && permission_queryer.can_read_from_space?(space.guid, org.guid)
     unauthorized! unless permission_queryer.can_read_secrets_in_space?(space.guid, org.guid)
 
     manifest_presenter = Presenters::V3::AppManifestPresenter.new(app, app.service_bindings, app.routes)

app/controllers/v3/app_revisions_controller.rb

@@ -14,7 +14,7 @@ def index
     invalid_param!(message.errors.full_messages) unless message.valid?
 
     app, space, org = AppFetcher.new.fetch(hashed_params[:guid])
-    app_not_found! unless app && permission_queryer.untrusted_can_read_from_space?(space.guid, org.guid)
+    app_not_found! unless app && permission_queryer.can_read_from_space?(space.guid, org.guid)
 
     dataset = AppRevisionsListFetcher.fetch(app, message)
 
@@ -31,7 +31,7 @@ def deployed
     invalid_param!(message.errors.full_messages) unless message.valid?
 
     app, space, org = AppFetcher.new.fetch(hashed_params[:guid])
-    app_not_found! unless app && permission_queryer.untrusted_can_read_from_space?(space.guid, org.guid)
+    app_not_found! unless app && permission_queryer.can_read_from_space?(space.guid, org.guid)
 
     dataset = AppRevisionsListFetcher.fetch_deployed(app)