Client can specify **destination ports**

This also enables support for Docker apps not listening on port 8080

This affects two 2 endpoints:
- POST `/v3/routes/route_GUID/destinations`
- PATCH `/v3/routes/route_GUID/destinations`

|**field**| **required**|**default** | **validations**|
|---|---|---|
|port|no| 8080 | integer, min 1024, max 65535 |

**Example Object**

```json
{
  "guid": "c87ebef2-fd8d-462d-9825-228f3055ddf2",
  "app": {
    "guid": "first-app-guid",
    "process": { "type": "web" }
  },
  “weight”: 30,
  "port": 1234
}
```

When _no_ port is specified on the destination, we adhere to the
following behavior:

| User Set Port	| App Lifecycle | Staged   | Display             |
|---------------|---------------|----------|---------------------|
| true          | anything      | anything | user-specified port |
| false         | buildpack     | anything | 8080 |
| false         | docker        | false    | 8080 |
| false         | docker        | true     | execution metadata 1st port OR 8080 |

Per Greg Cobb:

> In short: the displayed port should be the same as the port that will be used

When a destination is created to a certain port on a process, we notify
Diego that this process should have that port open; in V2 we didn't.

[finishes #166091491]
[fixes #167149569]

Co-authored-by: Reid Mitchell <[email protected]>
Co-authored-by: Eli Wrenn <[email protected]>
Co-authored-by: Supraja Narasimhan <[email protected]>
Co-authored-by: Piyali Banerjee <[email protected]>
Co-authored-by: Brian Cunnie <[email protected]>
Co-authored-by: Luan Santos <[email protected]>

app/actions/update_route_destinations.rb

@@ -1,23 +1,26 @@
 module VCAP::CloudController
   class UpdateRouteDestinations
-    class Error < StandardError; end
+    class Error < StandardError
+    end
 
     class << self
-      def add(message, route, user_audit_info)
+      def add(new_route_mappings, route, apps_hash, user_audit_info)
         existing_route_mappings = route_to_mapping_hashes(route)
+        new_route_mappings = update_port(new_route_mappings, apps_hash)
+        new_route_mappings = add_route(new_route_mappings, route)
         if existing_route_mappings.any? { |rm| rm[:weight] }
           raise Error.new('Destinations cannot be inserted when there are weighted destinations already configured.')
         end
 
-        new_route_mappings = message_to_mapping_hashes(message, route)
         to_add = new_route_mappings - existing_route_mappings
 
         update(route, to_add, [], user_audit_info)
       end
 
-      def replace(message, route, user_audit_info)
+      def replace(new_route_mappings, route, apps_hash, user_audit_info)
         existing_route_mappings = route_to_mapping_hashes(route)
-        new_route_mappings = message_to_mapping_hashes(message, route)
+        new_route_mappings = update_port(new_route_mappings, apps_hash)
+        new_route_mappings = add_route(new_route_mappings, route)
         to_add = new_route_mappings - existing_route_mappings
         to_delete = existing_route_mappings - new_route_mappings
 
@@ -38,12 +41,19 @@ def delete(destination, route, user_audit_info)
 
       def update(route, to_add, to_delete, user_audit_info)
         RouteMappingModel.db.transaction do
+          processes_to_ports_map = {}
+
           to_delete.each do |rm|
             route_mapping = RouteMappingModel.find(rm)
             route_mapping.destroy
 
             Copilot::Adapter.unmap_route(route_mapping)
-            update_route_information(route_mapping)
+            route_mapping.processes.each do |process|
+              processes_to_ports_map[process] ||= { to_add: [], to_delete: [] }
+              processes_to_ports_map[process][:to_delete] << route_mapping.app_port unless process.route_mappings.any? do |process_route_mapping|
+                process_route_mapping.guid != route_mapping.guid && process_route_mapping.app_port == route_mapping.app_port
+              end
+            end
 
             Repositories::RouteEventRepository.new.record_route_unmap(route_mapping, user_audit_info)
           end
@@ -53,39 +63,57 @@ def update(route, to_add, to_delete, user_audit_info)
             route_mapping.save
 
             Copilot::Adapter.map_route(route_mapping)
-            update_route_information(route_mapping)
+            route_mapping.processes.each do |process|
+              processes_to_ports_map[process] ||= { to_add: [], to_delete: [] }
+              processes_to_ports_map[process][:to_add] << route_mapping.app_port
+            end
 
             Repositories::RouteEventRepository.new.record_route_map(route_mapping, user_audit_info)
           end
+
+          update_processes(processes_to_ports_map)
         end
 
         route.reload
       end
 
-      def update_route_information(route_mapping)
-        route_mapping.processes.each do |process|
-          ProcessRouteHandler.new(process).update_route_information(perform_validation: false)
+      def update_processes(processes_to_ports_map)
+        processes_to_ports_map.each do |process, ports_hash|
+          ports_to_add = ports_hash[:to_add].uniq.reject { |port| port == ProcessModel::NO_APP_PORT_SPECIFIED }
+          ports_to_delete = ports_hash[:to_delete].uniq.reject { |port| port == ProcessModel::NO_APP_PORT_SPECIFIED }
+
+          updated_ports = ((process.ports || []) + ports_to_add - ports_to_delete).uniq
+          if process.ports.nil? && updated_ports.empty?
+            updated_ports = nil
+          end
+
+          ProcessRouteHandler.new(process).update_route_information(
+            perform_validation: false,
+            updated_ports: updated_ports
+          )
         end
+      rescue Sequel::ValidationFailed => e
+        raise Error.new(e.message)
       end
 
-      def message_to_mapping_hashes(message, route)
-        new_route_mappings = []
-        message.destinations.each do |dst|
-          app_guid = HashUtils.dig(dst, :app, :guid)
-          process_type = HashUtils.dig(dst, :app, :process, :type) || 'web'
-          weight = HashUtils.dig(dst, :weight)
-
-          new_route_mappings << {
-            app_guid: app_guid,
-            route_guid: route.guid,
-            route: route,
-            process_type: process_type,
-            app_port: ProcessModel::DEFAULT_HTTP_PORT,
-            weight: weight
-          }
+      def add_route(destinations, route)
+        destinations.map do |dst|
+          dst.merge({ route: route, route_guid: route.guid })
         end
+      end
+
+      def update_port(destinations, apps_hash)
+        destinations.each do |dst|
+          if dst[:app_port].nil?
+            app = apps_hash[dst[:app_guid]]
 
-        new_route_mappings
+            dst[:app_port] = if app.buildpack?
+                               ProcessModel::DEFAULT_HTTP_PORT
+                             else
+                               ProcessModel::NO_APP_PORT_SPECIFIED
+                             end
+          end
+        end
       end
 
       def route_to_mapping_hashes(route)

app/controllers/v3/routes_controller.rb

@@ -115,7 +115,7 @@ def insert_destinations
     validate_app_guids!(apps_hash, desired_app_guids)
     validate_app_spaces!(apps_hash, route)
 
-    route = UpdateRouteDestinations.add(message, route, user_audit_info)
+    route = UpdateRouteDestinations.add(message.destinations_array, route, apps_hash, user_audit_info)
 
     render status: :ok, json: Presenters::V3::RouteDestinationsPresenter.new(route)
   rescue UpdateRouteDestinations::Error => e
@@ -137,7 +137,7 @@ def replace_destinations
     validate_app_guids!(apps_hash, desired_app_guids)
     validate_app_spaces!(apps_hash, route)
 
-    route = UpdateRouteDestinations.replace(message, route, user_audit_info)
+    route = UpdateRouteDestinations.replace(message.destinations_array, route, apps_hash, user_audit_info)
 
     render status: :ok, json: Presenters::V3::RouteDestinationsPresenter.new(route)
   end

app/messages/route_update_destinations_message.rb

@@ -11,14 +11,32 @@ def initialize(params, replace: false)
 
     validate :destinations_valid?
 
+    def destinations_array
+      new_route_mappings = []
+      destinations.each do |dst|
+        app_guid = HashUtils.dig(dst, :app, :guid)
+        process_type = HashUtils.dig(dst, :app, :process, :type) || 'web'
+        weight = HashUtils.dig(dst, :weight)
+
+        new_route_mappings << {
+          app_guid: app_guid,
+          process_type: process_type,
+          app_port: dst[:port],
+          weight: weight
+        }
+      end
+
+      new_route_mappings
+    end
+
     private
 
     ERROR_MESSAGE = 'Destinations must have the structure "destinations": [{"app": {"guid": "app_guid"}}]'.freeze
 
     def destinations_valid?
       minimum = @replace ? 0 : 1
 
-      unless destinations.is_a?(Array) && (minimum...100).cover?(destinations.length)
+      unless destinations.is_a?(Array) && (minimum..100).cover?(destinations.length)
         errors.add(:destinations, "must be an array containing between #{minimum} and 100 destination objects.")
         return
       end
@@ -27,27 +45,40 @@ def destinations_valid?
     end
 
     def validate_destination_contents
+      app_to_ports_hash = {}
+
       destinations.each_with_index do |dst, index|
         unless dst.is_a?(Hash)
           add_destination_error(index, 'must be a hash.')
           next
         end
 
-        if dst.is_a?(Hash) && !dst.key?(:app)
+        unless dst.key?(:app)
           add_destination_error(index, 'must have an "app".')
           next
         end
 
-        if dst.is_a?(Hash) && !(dst.keys - [:app, :weight]).empty?
-          add_destination_error(index, 'must have only "app" and "weight".')
+        unless (dst.keys - [:app, :weight, :port]).empty?
+          add_destination_error(index, 'must have only "app" and optionally "weight" and "port".')
           next
         end
 
         validate_app(index, dst[:app])
         validate_weight(index, dst[:weight])
+        validate_port(index, dst[:port])
+
+        app_to_ports_hash[dst[:app]] ||= []
+        app_to_ports_hash[dst[:app]] << dst[:port]
       end
 
-      return if !errors.empty?
+      app_to_ports_hash.each do |_, port_array|
+        if port_array.length > 10
+          errors.add(:process, 'must have at most 10 exposed ports.')
+          break
+        end
+      end
+
+      return unless errors.empty?
 
       validate_weights(destinations)
     end
@@ -65,6 +96,14 @@ def validate_weight(destination_index, weight)
       end
     end
 
+    def validate_port(destination_index, port)
+      return unless port
+
+      unless port.is_a?(Integer) && port >= 1024 && port <= 65535
+        add_destination_error(destination_index, 'port must be a positive integer between 1024 and 65535 inclusive.')
+      end
+    end
+
     def validate_weights(destinations)
       weights = destinations.map { |d| d.is_a?(Hash) && d[:weight] }
 

app/models/runtime/droplet_model.rb

@@ -3,11 +3,11 @@ class DropletModel < Sequel::Model(:droplets)
     include Serializer
 
     DROPLET_STATES = [
-      STAGING_STATE         = 'STAGING'.freeze,
-      COPYING_STATE         = 'COPYING'.freeze,
-      FAILED_STATE          = 'FAILED'.freeze,
-      STAGED_STATE          = 'STAGED'.freeze,
-      EXPIRED_STATE         = 'EXPIRED'.freeze,
+      STAGING_STATE = 'STAGING'.freeze,
+      COPYING_STATE = 'COPYING'.freeze,
+      FAILED_STATE = 'FAILED'.freeze,
+      STAGED_STATE = 'STAGED'.freeze,
+      EXPIRED_STATE = 'EXPIRED'.freeze,
       AWAITING_UPLOAD_STATE = 'AWAITING_UPLOAD'.freeze,
       PROCESSING_UPLOAD_STATE = 'PROCESSING_UPLOAD'.freeze,
     ].freeze
@@ -28,8 +28,8 @@ class DropletModel < Sequel::Model(:droplets)
       without_guid_generation: true
     one_through_one :space, join_table: AppModel.table_name, left_key: :guid, left_primary_key: :app_guid, right_primary_key: :guid, right_key: :space_guid
     one_to_one :buildpack_lifecycle_data,
-      class:       'VCAP::CloudController::BuildpackLifecycleDataModel',
-      key:         :droplet_guid,
+      class: 'VCAP::CloudController::BuildpackLifecycleDataModel',
+      key: :droplet_guid,
       primary_key: :guid
     one_to_many :labels, class: 'VCAP::CloudController::DropletLabelModel', key: :resource_guid, primary_key: :guid
     one_to_many :annotations, class: 'VCAP::CloudController::DropletAnnotationModel', key: :resource_guid, primary_key: :guid
@@ -54,7 +54,7 @@ def set_buildpack_receipt(buildpack_key:, detect_output:, requested_buildpack:,
 
       if buildpack_key.present? && (admin_buildpack = Buildpack.find(key: buildpack_key))
         self.buildpack_receipt_buildpack_guid = admin_buildpack.guid
-        self.buildpack_receipt_buildpack      = admin_buildpack.name
+        self.buildpack_receipt_buildpack = admin_buildpack.name
       elsif buildpack_url.present?
         self.buildpack_receipt_buildpack = buildpack_url
       else
@@ -81,6 +81,24 @@ def docker?
       lifecycle_type == DockerLifecycleDataModel::LIFECYCLE_TYPE
     end
 
+    def docker_ports
+      exposed_ports = []
+      if self.execution_metadata.present?
+        begin
+          metadata = JSON.parse(self.execution_metadata)
+          unless metadata['ports'].nil?
+            metadata['ports'].each { |port|
+              if port['Protocol'] == 'tcp'
+                exposed_ports << port['Port']
+              end
+            }
+          end
+        rescue JSON::ParserError
+        end
+      end
+      exposed_ports
+    end
+
     def staging?
       self.state == STAGING_STATE
     end
@@ -108,8 +126,8 @@ def mark_as_staged
     def fail_to_stage!(reason='StagingError', details='staging failed')
       reason = 'StagingError' unless STAGING_FAILED_REASONS.include?(reason)
 
-      self.state             = FAILED_STATE
-      self.error_id          = reason
+      self.state = FAILED_STATE
+      self.error_id = reason
       self.error_description = CloudController::Errors::ApiError.new_from_details(reason, details).message
       save_changes(raise_on_save_failure: true)
     end

app/models/runtime/process_model.rb

@@ -538,21 +538,11 @@ def to_hash(opts={})
     end
 
     def docker_ports
-      exposed_ports = []
-      if !self.needs_staging? && desired_droplet.present? && self.execution_metadata.present?
-        begin
-          metadata = JSON.parse(self.execution_metadata)
-          unless metadata['ports'].nil?
-            metadata['ports'].each { |port|
-              if port['Protocol'] == 'tcp'
-                exposed_ports << port['Port']
-              end
-            }
-          end
-        rescue JSON::ParserError
-        end
+      if !self.needs_staging? && desired_droplet.present?
+        return desired_droplet.docker_ports
       end
-      exposed_ports
+
+      []
     end
 
     def web?
@@ -577,17 +567,6 @@ def changed_from_default_ports?
       @ports_changed_by_user && (initial_value(:ports).nil? || initial_value(:ports) == [DEFAULT_HTTP_PORT])
     end
 
-    # HACK: We manually call the Serializer here because the plugin uses the
-    # _before_validation method to serialize ports. This is called before
-    # validations and we want to set the default ports after validations.
-    #
-    # See:
-    # https://github.com/jeremyevans/sequel/blob/7d6753da53196884e218a59a7dcd9a7803881b68/lib/sequel/model/base.rb#L1772-L1779
-    def update_ports(new_ports)
-      self.ports   = new_ports
-      self[:ports] = IntegerArraySerializer.serializer.call(self.ports)
-    end
-
     def metadata_deserialized
       deserialized_values[:metadata]
     end

app/presenters/v3/route_destinations_presenter.rb

@@ -25,7 +25,8 @@ def build_destinations
               type: route_mapping.process_type
             }
           },
-          weight: route_mapping.weight
+          weight: route_mapping.weight,
+          port: build_port(route_mapping)
         }
       end
     end
@@ -45,5 +46,20 @@ def build_links
 
       links
     end
+
+    def build_port(route_mapping)
+      rm_port = route_mapping.app_port
+
+      if rm_port != VCAP::CloudController::ProcessModel::NO_APP_PORT_SPECIFIED
+        return rm_port
+      end
+
+      app_droplet = route_mapping.app.droplet
+      if app_droplet && !app_droplet.docker_ports.empty?
+        return app_droplet.docker_ports.first
+      end
+
+      VCAP::CloudController::ProcessModel::DEFAULT_HTTP_PORT
+    end
   end
 end