Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add route destinations protocols #2439

Merged
merged 14 commits into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 47 additions & 3 deletions app/actions/update_route_destinations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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

Expand Down Expand Up @@ -126,6 +131,21 @@ 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.slice(:app_guid, :process_type, :route_guid, :app_port) == existing.slice(:app_guid, :process_type, :route_guid, :app_port)
end

if matching_route_mapping && matching_route_mapping[:protocol] != new[:protocol]
raise Error.new("Cannot add destination with protocol: #{new[:protocol]}. Destination already exists for route: #{new[:route].uri}, app: #{new[:app_guid]}, " \
"process: #{new[:process_type]}, and protocol: #{matching_route_mapping[:protocol]}.")
end

matching_route_mapping
end
end

def add_route(destinations, route)
destinations.map do |dst|
dst.merge({ route: route, route_guid: route.guid })
Expand All @@ -142,6 +162,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
Expand All @@ -163,7 +191,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
}
end

Expand All @@ -175,6 +204,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

Expand Down
59 changes: 24 additions & 35 deletions app/controllers/v3/routes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ def show
message = RouteShowMessage.from_params(query_params.merge(guid: hashed_params[:guid]))
unprocessable!(message.errors.full_messages) unless message.valid?

route = Route.find(guid: message.guid)
route_not_found! unless route && permission_queryer.untrusted_can_read_route?(route.space.guid, route.organization.guid)

decorators = []
decorators << IncludeRouteDomainDecorator if IncludeRouteDomainDecorator.match?(message.include)
decorators << IncludeSpaceDecorator if IncludeSpaceDecorator.match?(message.include)
Expand Down Expand Up @@ -87,16 +84,12 @@ def create
end

def update
route = Route.find(guid: hashed_params[:guid])
route_not_found! unless route

message = RouteUpdateMessage.new(hashed_params[:body])
unprocessable!(message.errors.full_messages) unless message.valid?

route_not_found! unless route && permission_queryer.untrusted_can_read_route?(route.space.guid, route.organization.guid)
unauthorized! unless permission_queryer.untrusted_can_write_to_space?(route.space.guid)

route = VCAP::CloudController::RouteUpdate.new.update(route: route, message: message)
VCAP::CloudController::RouteUpdate.new.update(route: route, message: message)

render status: :ok, json: Presenters::V3::RoutePresenter.new(route)
end
Expand All @@ -105,8 +98,6 @@ def destroy
message = RouteShowMessage.from_params({ guid: hashed_params['guid'] })
unprocessable!(message.errors.full_messages) unless message.valid?

route = Route.find(guid: message.guid)
route_not_found! unless route && permission_queryer.untrusted_can_read_route?(route.space.guid, route.organization.guid)
unauthorized! unless permission_queryer.untrusted_can_write_to_space?(route.space.guid)

delete_action = RouteDeleteAction.new(user_audit_info)
Expand All @@ -120,9 +111,6 @@ def index_destinations
message = RouteShowMessage.from_params({ guid: hashed_params['guid'] })
unprocessable!(message.errors.full_messages) unless message.valid?

route = Route.find(guid: message.guid)
route_not_found! unless route && permission_queryer.untrusted_can_read_route?(route.space.guid, route.organization.guid)

destinations_message = RouteDestinationsListMessage.from_params(query_params)
unprocessable!(destinations_message.errors.full_messages) unless destinations_message.valid?
route_mappings = RouteDestinationsListFetcher.new(message: destinations_message).fetch_for_route(route: route)
Expand All @@ -132,20 +120,11 @@ def index_destinations

def insert_destinations
message = RouteUpdateDestinationsMessage.new(hashed_params[:body])
unprocessable!(message.errors.full_messages) unless message.valid?

route = Route.find(guid: hashed_params[:guid])
route_not_found! unless route && permission_queryer.untrusted_can_read_route?(route.space.guid, route.organization.guid)

unprocessable!(message.errors.full_messages) unless message.valid?
unauthorized! unless permission_queryer.untrusted_can_write_to_space?(route.space.guid)

desired_app_guids = message.destinations.map { |dst| HashUtils.dig(dst, :app, :guid) }.compact

apps_hash = AppModel.where(guid: desired_app_guids).each_with_object({}) { |app, apps_hsh| apps_hsh[app.guid] = app; }
validate_app_guids!(apps_hash, desired_app_guids)
validate_app_spaces!(apps_hash, route)

route = UpdateRouteDestinations.add(message.destinations_array, route, apps_hash, user_audit_info)
UpdateRouteDestinations.add(message.destinations_array, route, apps_hash(message), user_audit_info)

render status: :ok, json: Presenters::V3::RouteDestinationsPresenter.new(route.route_mappings, route: route)
rescue UpdateRouteDestinations::Error => e
Expand All @@ -154,26 +133,36 @@ def insert_destinations

def replace_destinations
message = RouteUpdateDestinationsMessage.new(hashed_params[:body], replace: true)
unprocessable!(message.errors.full_messages) unless message.valid?

route = Route.find(guid: hashed_params[:guid])
route_not_found! unless route && permission_queryer.untrusted_can_read_route?(route.space.guid, route.organization.guid)

unprocessable!(message.errors.full_messages) unless message.valid?
unauthorized! unless permission_queryer.untrusted_can_write_to_space?(route.space.guid)

desired_app_guids = message.destinations.map { |dst| HashUtils.dig(dst, :app, :guid) }.compact

apps_hash = AppModel.where(guid: desired_app_guids).each_with_object({}) { |app, apps_hsh| apps_hsh[app.guid] = app; }
validate_app_guids!(apps_hash, desired_app_guids)
validate_app_spaces!(apps_hash, route)

route = UpdateRouteDestinations.replace(message.destinations_array, route, apps_hash, user_audit_info)
UpdateRouteDestinations.replace(message.destinations_array, route, apps_hash(message), user_audit_info)

render status: :ok, json: Presenters::V3::RouteDestinationsPresenter.new(route.route_mappings, route: route)
rescue UpdateRouteDestinations::DuplicateDestinationError => e
unprocessable!(e.message)
end

def route
@route || begin
@route = Route.find(guid: hashed_params[:guid])
route_not_found! unless @route && permission_queryer.untrusted_can_read_route?(@route.space.guid, @route.organization.guid)
@route
end
end

def apps_hash(update_message)
@apps_hash || begin
desired_app_guids = update_message.destinations.map { |dst| HashUtils.dig(dst, :app, :guid) }.compact

@apps_hash = AppModel.where(guid: desired_app_guids).each_with_object({}) { |app, apps_hsh| apps_hsh[app.guid] = app; }
validate_app_guids!(@apps_hash, desired_app_guids)
validate_app_spaces!(@apps_hash, route)
@apps_hash
end
end

def destroy_destination
route = Route.find(guid: hashed_params[:guid])
route_not_found! unless route && permission_queryer.untrusted_can_read_route?(route.space.guid, route.organization.guid)
Expand Down
17 changes: 14 additions & 3 deletions app/messages/route_update_destinations_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ def destinations_array
app_guid = HashUtils.dig(dst, :app, :guid)
process_type = HashUtils.dig(dst, :app, :process, :type) || 'web'
weight = HashUtils.dig(dst, :weight)
protocol = HashUtils.dig(dst, :protocol)

new_route_mappings << {
app_guid: app_guid,
process_type: process_type,
app_port: dst[:port],
weight: weight
weight: weight,
protocol: protocol,
}
end

Expand Down Expand Up @@ -58,14 +60,15 @@ def validate_destination_contents
next
end

unless (dst.keys - [:app, :weight, :port]).empty?
add_destination_error(index, 'must have only "app" and optionally "weight" and "port".')
unless (dst.keys - [:app, :weight, :port, :protocol]).empty?
add_destination_error(index, 'must have only "app" and optionally "weight", "port" or "protocol".')
next
end

validate_app(index, dst[:app])
validate_weight(index, dst[:weight])
validate_port(index, dst[:port])
validate_protocol(index, dst[:protocol])

app_to_ports_hash[dst[:app]] ||= []
app_to_ports_hash[dst[:app]] << dst[:port]
Expand Down Expand Up @@ -96,6 +99,14 @@ def validate_weight(destination_index, weight)
end
end

def validate_protocol(destination_index, protocol)
return unless protocol

unless protocol.is_a?(String) && ['http1', 'http2', 'tcp'].include?(protocol)
add_destination_error(destination_index, "protocol must be 'http1', 'http2' or 'tcp'.")
end
end

def validate_port(destination_index, port)
return unless port

Expand Down
4 changes: 4 additions & 0 deletions app/models/runtime/route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ def tcp?
domain.shared? && domain.tcp? && port.present? && port > 0
end

def protocol
self.domain.protocols.first
sethboyles marked this conversation as resolved.
Show resolved Hide resolved
end

def internal?
domain.internal
end
Expand Down
16 changes: 16 additions & 0 deletions app/models/runtime/route_mapping_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

module VCAP::CloudController
class RouteMappingModel < Sequel::Model(:route_mappings)
DEFAULT_PROTOCOL_MAPPING = { 'tcp' => 'tcp', 'http' => 'http1' }.freeze

many_to_one :app, class: 'VCAP::CloudController::AppModel', key: :app_guid,
primary_key: :guid, without_guid_generation: true
many_to_one :route, key: :route_guid, primary_key: :guid, without_guid_generation: true
Expand All @@ -17,6 +19,20 @@ class RouteMappingModel < Sequel::Model(:route_mappings)
one_to_many :processes, class: 'VCAP::CloudController::ProcessModel',
primary_key: [:app_guid, :process_type], key: [:app_guid, :type]

def protocol_with_defaults=(new_protocol)
self.protocol_without_defaults = (new_protocol == 'http2' ? new_protocol : nil)
end

alias_method :protocol_without_defaults=, :protocol=
alias_method :protocol=, :protocol_with_defaults=

def protocol_with_defaults
self.protocol_without_defaults == 'http2' ? 'http2' : DEFAULT_PROTOCOL_MAPPING[self.route&.protocol]
end

alias_method :protocol_without_defaults, :protocol
alias_method :protocol, :protocol_with_defaults

def validate
validates_presence [:app_port]
validates_unique [:app_guid, :route_guid, :process_type, :app_port]
Expand Down
3 changes: 2 additions & 1 deletion app/presenters/v3/route_destination_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ def to_hash
}
},
weight: destination.weight,
port: destination.presented_port
port: destination.presented_port,
protocol: destination.protocol
}
end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Sequel.migration do
change do
alter_table :route_mappings do
add_column :protocol, String, size: 255
end
end
end