diff --git a/app/actions/manifest_route_update.rb b/app/actions/manifest_route_update.rb index d5fef841e5d..a6fbc2af9fa 100644 --- a/app/actions/manifest_route_update.rb +++ b/app/actions/manifest_route_update.rb @@ -1,5 +1,6 @@ require 'cloud_controller/app_manifest/manifest_route' require 'actions/route_create' +require 'actions/route_update' module VCAP::CloudController class ManifestRouteUpdate @@ -81,7 +82,8 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info) message = RouteCreateMessage.new({ 'host' => host, 'path' => manifest_route[:path], - 'port' => manifest_route[:port] + 'port' => manifest_route[:port], + 'options' => manifest_route[:options] }) route = RouteCreate.new(user_audit_info).create( @@ -92,6 +94,13 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info) ) elsif route.space.guid != app.space_guid raise InvalidRoute.new('Routes cannot be mapped to destinations in different spaces') + elsif manifest_route[:options] && route[:options] != manifest_route[:options] + # remove nil values from options + manifest_route[:options] = manifest_route[:options].compact + message = RouteUpdateMessage.new({ + 'options' => manifest_route[:options] + }) + route = RouteUpdate.new.update(route:, message:) end return route diff --git a/app/actions/route_create.rb b/app/actions/route_create.rb index 21d08fd8fdd..15be9568456 100644 --- a/app/actions/route_create.rb +++ b/app/actions/route_create.rb @@ -15,7 +15,8 @@ def create(message:, space:, domain:, manifest_triggered: false) path: message.path || '', port: port(message, domain), space: space, - domain: domain + domain: domain, + options: message.options.nil? ? {} : message.options.compact ) Route.db.transaction do diff --git a/app/actions/route_update.rb b/app/actions/route_update.rb index d5254a824b7..746dcfcd8ef 100644 --- a/app/actions/route_update.rb +++ b/app/actions/route_update.rb @@ -2,6 +2,9 @@ module VCAP::CloudController class RouteUpdate def update(route:, message:) Route.db.transaction do + route.options = route.options.symbolize_keys.merge(message.options).compact if message.requested?(:options) + + route.save MetadataUpdate.update(route, message) end diff --git a/app/messages/manifest_routes_update_message.rb b/app/messages/manifest_routes_update_message.rb index 74fa2a31e8d..bc4c6b9cb5e 100644 --- a/app/messages/manifest_routes_update_message.rb +++ b/app/messages/manifest_routes_update_message.rb @@ -1,4 +1,5 @@ require 'messages/base_message' +require 'messages/route_options_message' require 'cloud_controller/app_manifest/manifest_route' module VCAP::CloudController @@ -25,6 +26,8 @@ def contains_non_route_hash_values?(routes) validates_with ManifestRoutesYAMLValidator, if: proc { |record| record.requested?(:routes) } validate :routes_are_uris, if: proc { |record| record.requested?(:routes) } validate :route_protocols_are_valid, if: proc { |record| record.requested?(:routes) } + validate :route_options_are_valid, if: proc { |record| record.requested?(:routes) } + validate :loadbalancings_are_valid, if: proc { |record| record.requested?(:routes) } validate :no_route_is_boolean validate :default_route_is_boolean validate :random_route_is_boolean @@ -32,15 +35,57 @@ def contains_non_route_hash_values?(routes) def manifest_route_mappings @manifest_route_mappings ||= routes.map do |route| - { - route: ManifestRoute.parse(route[:route]), + r = { + route: ManifestRoute.parse(route[:route], route[:options]), protocol: route[:protocol] } + r[:options] = route[:options] unless route[:options].nil? + r end end private + def route_options_are_valid + return if errors[:routes].present? + + routes.any? do |r| + next unless r.keys.include?(:options) + + unless r[:options].is_a?(Hash) + errors.add(:base, message: "Route '#{r[:route]}': options must be an object") + next + end + + r[:options].each_key do |key| + RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.exclude?(key) && + errors.add(:base, + message: "Route '#{r[:route]}' contains invalid route option '#{key}'. \ +Valid keys: '#{RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.join(', ')}'") + end + end + end + + def loadbalancings_are_valid + return if errors[:routes].present? + + routes.each do |r| + next unless r.keys.include?(:options) && r[:options].is_a?(Hash) && r[:options].keys.include?(:loadbalancing) + + loadbalancing = r[:options][:loadbalancing] + unless loadbalancing.is_a?(String) + errors.add(:base, + message: "Invalid value for 'loadbalancing' for Route '#{r[:route]}'; \ +Valid values are: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'") + next + end + RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(loadbalancing) && + errors.add(:base, + message: "Cannot use loadbalancing value '#{loadbalancing}' for Route '#{r[:route]}'; \ +Valid values are: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'") + end + end + def routes_are_uris return if errors[:routes].present? diff --git a/app/messages/route_create_message.rb b/app/messages/route_create_message.rb index 1eeac2c2e46..6605ded1a3c 100644 --- a/app/messages/route_create_message.rb +++ b/app/messages/route_create_message.rb @@ -1,4 +1,5 @@ require 'messages/metadata_base_message' +require 'messages/route_options_message' module VCAP::CloudController class RouteCreateMessage < MetadataBaseMessage @@ -10,8 +11,13 @@ class RouteCreateMessage < MetadataBaseMessage path port relationships + options ] + def self.options_requested? + @options_requested ||= proc { |a| a.requested?(:options) } + end + validates :host, allow_nil: true, string: true, @@ -56,6 +62,7 @@ class RouteCreateMessage < MetadataBaseMessage validates_with NoAdditionalKeysValidator validates_with RelationshipValidator + validates_with OptionsValidator, if: options_requested? delegate :space_guid, to: :relationships_message delegate :domain_guid, to: :relationships_message @@ -65,6 +72,10 @@ def relationships_message @relationships_message ||= Relationships.new(relationships&.deep_symbolize_keys) end + def options_message + @options_message ||= RouteOptionsMessage.new(options&.deep_symbolize_keys) + end + def wildcard? host == '*' end diff --git a/app/messages/route_options_message.rb b/app/messages/route_options_message.rb new file mode 100644 index 00000000000..818eb039ddf --- /dev/null +++ b/app/messages/route_options_message.rb @@ -0,0 +1,16 @@ +require 'messages/metadata_base_message' + +module VCAP::CloudController + class RouteOptionsMessage < BaseMessage + VALID_MANIFEST_ROUTE_OPTIONS = %i[loadbalancing].freeze + VALID_ROUTE_OPTIONS = %i[loadbalancing].freeze + VALID_LOADBALANCING_ALGORITHMS = %w[round-robin least-connections].freeze + + register_allowed_keys VALID_ROUTE_OPTIONS + validates_with NoAdditionalKeysValidator + validates :loadbalancing, + inclusion: { in: VALID_LOADBALANCING_ALGORITHMS, message: "must be one of '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}' if present" }, + presence: true, + allow_nil: true + end +end diff --git a/app/messages/route_update_message.rb b/app/messages/route_update_message.rb index 84365bdb3d2..3e567404690 100644 --- a/app/messages/route_update_message.rb +++ b/app/messages/route_update_message.rb @@ -1,8 +1,19 @@ require 'messages/metadata_base_message' +require 'messages/route_options_message' module VCAP::CloudController class RouteUpdateMessage < MetadataBaseMessage - register_allowed_keys [] + register_allowed_keys %i[options] + + def self.options_requested? + @options_requested ||= proc { |a| a.requested?(:options) } + end + + def options_message + @options_message ||= RouteOptionsMessage.new(options&.deep_symbolize_keys) + end + + validates_with OptionsValidator, if: options_requested? validates_with NoAdditionalKeysValidator end diff --git a/app/messages/validators.rb b/app/messages/validators.rb index c5555856c33..48d4d3bdf61 100644 --- a/app/messages/validators.rb +++ b/app/messages/validators.rb @@ -240,6 +240,23 @@ def validate(record) end end + class OptionsValidator < ActiveModel::Validator + def validate(record) + unless record.options.is_a?(Hash) + record.errors.add(:options, message: "'options' is not a valid object") + return + end + + opt = record.options_message + + return if opt.valid? + + opt.errors.full_messages.each do |message| + record.errors.add(:options, message:) + end + end + end + class ToOneRelationshipValidator < ActiveModel::EachValidator def validate_each(record, attribute, relationship) if has_correct_structure?(relationship) diff --git a/app/models/runtime/route.rb b/app/models/runtime/route.rb index 3f10c89dd67..7ec7a0b2264 100644 --- a/app/models/runtime/route.rb +++ b/app/models/runtime/route.rb @@ -40,8 +40,8 @@ class OutOfVIPException < CloudController::Errors::InvalidRelation; end add_association_dependencies route_mappings: :destroy - export_attributes :host, :path, :domain_guid, :space_guid, :service_instance_guid, :port - import_attributes :host, :path, :domain_guid, :space_guid, :app_guids, :port + export_attributes :host, :path, :domain_guid, :space_guid, :service_instance_guid, :port, :options + import_attributes :host, :path, :domain_guid, :space_guid, :app_guids, :port, :options add_association_dependencies labels: :destroy add_association_dependencies annotations: :destroy @@ -71,6 +71,23 @@ def as_summary_json } end + def options_with_serialization=(opts) + self.options_without_serialization = Oj.dump(opts) + end + + alias_method :options_without_serialization=, :options= + alias_method :options=, :options_with_serialization= + + def options_with_serialization + string = options_without_serialization + return nil if string.blank? + + Oj.load(string) + end + + alias_method :options_without_serialization, :options + alias_method :options, :options_with_serialization + alias_method :old_path, :path def path old_path.nil? ? '' : old_path diff --git a/app/presenters/v3/app_manifest_presenters/route_properties_presenter.rb b/app/presenters/v3/app_manifest_presenters/route_properties_presenter.rb index d3f43f40d31..d58c52c3003 100644 --- a/app/presenters/v3/app_manifest_presenters/route_properties_presenter.rb +++ b/app/presenters/v3/app_manifest_presenters/route_properties_presenter.rb @@ -5,10 +5,17 @@ module AppManifestPresenters class RoutePropertiesPresenter def to_hash(route_mappings:, app:, **_) route_hashes = route_mappings.map do |route_mapping| - { + route_hash = { route: route_mapping.route.uri, protocol: route_mapping.protocol } + + if route_mapping.route.options + route_hash[:options] = {} + route_hash[:options][:loadbalancing] = route_mapping.route.options[:loadbalancing] if route_mapping.route.options[:loadbalancing] + end + + route_hash end { routes: alphabetize(route_hashes).presence } diff --git a/app/presenters/v3/route_presenter.rb b/app/presenters/v3/route_presenter.rb index 9715e3c09c9..c090fafae5b 100644 --- a/app/presenters/v3/route_presenter.rb +++ b/app/presenters/v3/route_presenter.rb @@ -48,6 +48,7 @@ def to_hash }, links: build_links } + hash.merge!(options: route.options) unless route.options.nil? @decorators.reduce(hash) { |memo, d| d.decorate(memo, [route]) } end diff --git a/db/migrations/20241105000000_add_options_to_route.rb b/db/migrations/20241105000000_add_options_to_route.rb new file mode 100644 index 00000000000..c9e21eee170 --- /dev/null +++ b/db/migrations/20241105000000_add_options_to_route.rb @@ -0,0 +1,12 @@ +Sequel.migration do + up do + alter_table(:routes) do + add_column :options, String, size: 4096, default: '{}' + end + end + down do + alter_table(:routes) do + drop_column :options + end + end +end diff --git a/docs/v3/source/includes/api_resources/_routes.erb b/docs/v3/source/includes/api_resources/_routes.erb index 3782b4a124f..a1f664a76e5 100644 --- a/docs/v3/source/includes/api_resources/_routes.erb +++ b/docs/v3/source/includes/api_resources/_routes.erb @@ -34,6 +34,9 @@ "protocol": "tcp" } ], + "options": { + "loadbalancing": "round-robin" + }, "metadata": { "labels": <%= metadata.fetch(:labels, {}).to_json(space: ' ', object_nl: ' ')%>, "annotations": <%= metadata.fetch(:annotations, {}).to_json(space: ' ', object_nl: ' ')%> @@ -124,6 +127,9 @@ "protocol": "http1" } ], + "options": { + "loadbalancing": "round-robin" + }, "metadata": { "labels": {}, "annotations": {} @@ -212,3 +218,9 @@ "protocol": "http2" } <% end %> + +<% content_for :route_options do %> + { + "loadbalancing": "round-robin" + } +<% end %> diff --git a/docs/v3/source/includes/resources/routes/_create.md.erb b/docs/v3/source/includes/resources/routes/_create.md.erb index e6e28d87019..6ad5b93a9f9 100644 --- a/docs/v3/source/includes/resources/routes/_create.md.erb +++ b/docs/v3/source/includes/resources/routes/_create.md.erb @@ -21,6 +21,9 @@ curl "https://api.example.org/v3/routes" \ "data": { "guid": "space-guid" } } }, + "options": { + "loadbalancing": "round-robin" + } "metadata": { "labels": { "key": "value" }, "annotations": { "note": "detailed information"} @@ -44,20 +47,21 @@ Content-Type: application/json #### Required parameters -| Name | Type | Description | -| ----------------------------------------- | ---------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | -| **relationships.space** | [_to-one relationship_](#to-one-relationships) | A relationship to the space containing the route; routes can only be mapped to destinations in that space | -| **relationships.domain** | [_to-one relationship_](#to-one-relationships) | A relationship to the domain of the route | +| Name | Type | Description | +|--------------------------|------------------------------------------------|-----------------------------------------------------------------------------------------------------------| +| **relationships.space** | [_to-one relationship_](#to-one-relationships) | A relationship to the space containing the route; routes can only be mapped to destinations in that space | +| **relationships.domain** | [_to-one relationship_](#to-one-relationships) | A relationship to the domain of the route | #### Optional parameters -| Name | Type | Description | -| ------------------------ | ----------------------------------- | --------------------------------- | -| **host** | _string_ | The host component for the route; not compatible with routes specifying the `tcp` protocol | -| **path** | _string_ | The path component for the route; should begin with a `/` and not compatible with routes specifying the `tcp` protocol | -| **port** | _integer_ | The port the route will listen on; only compatible with routes leveraging a domain that supports the `tcp` protocol. For `tcp` domains, a port will be randomly assigned if not specified -| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the route | -| **metadata.labels** | [_label object_](#labels) | Labels applied to the route | +| Name | Type | Description | +|--------------------------|----------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| **host** | _string_ | The host component for the route; not compatible with routes specifying the `tcp` protocol | +| **path** | _string_ | The path component for the route; should begin with a `/` and not compatible with routes specifying the `tcp` protocol | +| **port** | _integer_ | The port the route will listen on; only compatible with routes leveraging a domain that supports the `tcp` protocol. For `tcp` domains, a port will be randomly assigned if not specified | +| **options** | [_route option object_](#the-route-options-object) | Options applied to the route | +| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the route | +| **metadata.labels** | [_label object_](#labels) | Labels applied to the route | #### Permitted roles diff --git a/docs/v3/source/includes/resources/routes/_object.md.erb b/docs/v3/source/includes/resources/routes/_object.md.erb index 4809097f531..a0de69a1675 100644 --- a/docs/v3/source/includes/resources/routes/_object.md.erb +++ b/docs/v3/source/includes/resources/routes/_object.md.erb @@ -8,19 +8,20 @@ Example Route object <%= yield_content :single_route %> ``` -| Name | Type | Description -| -------------- | ------------------------ | ------------------------------------------------------ -| **guid** | _uuid_ | Unique identifier for the route -| **created_at** | _[timestamp](#timestamps)_ | The time with zone when the object was created -| **updated_at** | _[timestamp](#timestamps)_ | The time with zone when the object was last updated -| **protocol** | _string_ | The protocol supported by the route, based on the route's domain configuration. Valid protocols are `http` and `tcp` -| **host** | _string_ | The hostname for the route; not compatible with routes specifying the `tcp` protocol;
must be either a wildcard (*) or be under 63 characters long and only contain letters, numbers, dashes (-) or underscores(_) -| **path** | _string_ | The path for the route; not compatible with routes specifying the `tcp` protocol;
must be under 128 characters long and not contain question marks (?), begin with a slash (/) and not be exactly a slash (/). Must conform to path components from [RFC 2396](https://tools.ietf.org/html/rfc2396) -| **port** | _integer or `null`_ | The port that the route listens on. Only compatible with routes specifying the `tcp` protocol -| **url** | _string_ | The URL for the route; URLs of protocol `http` are a combination of host, domain name, and path; URLs of protocol `tcp` are a combination of domain name and port -| **destinations** | array of [_destination objects_](#the-destination-object) | List of destinations for the route -| **relationships.space** | [_to-one relationship_](#to-one-relationships) | A relationship to the space containing the route. Routes can only be mapped to destinations in that space -| **relationships.domain** | [_to-one relationship_](#to-one-relationships) | A relationship to the domain of the route -| **metadata.labels** | [_label object_](#labels) | Labels applied to the route -| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the route -| **links** | [_links object_](#links) | Links to related resources +| Name | Type | Description | +|--------------------------|-----------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| **guid** | _uuid_ | Unique identifier for the route | +| **created_at** | _[timestamp](#timestamps)_ | The time with zone when the object was created | +| **updated_at** | _[timestamp](#timestamps)_ | The time with zone when the object was last updated | +| **protocol** | _string_ | The protocol supported by the route, based on the route's domain configuration. Valid protocols are `http` and `tcp` | +| **host** | _string_ | The hostname for the route; not compatible with routes specifying the `tcp` protocol;
must be either a wildcard (*) or be under 63 characters long and only contain letters, numbers, dashes (-) or underscores(_) | +| **path** | _string_ | The path for the route; not compatible with routes specifying the `tcp` protocol;
must be under 128 characters long and not contain question marks (?), begin with a slash (/) and not be exactly a slash (/). Must conform to path components from [RFC 2396](https://tools.ietf.org/html/rfc2396) | +| **port** | _integer or `null`_ | The port that the route listens on. Only compatible with routes specifying the `tcp` protocol | +| **url** | _string_ | The URL for the route; URLs of protocol `http` are a combination of host, domain name, and path; URLs of protocol `tcp` are a combination of domain name and port | +| **destinations** | array of [_destination objects_](#the-destination-object) | List of destinations for the route | +| **relationships.space** | [_to-one relationship_](#to-one-relationships) | A relationship to the space containing the route. Routes can only be mapped to destinations in that space | +| **relationships.domain** | [_to-one relationship_](#to-one-relationships) | A relationship to the domain of the route | +| **options** | [_route option object_](#the-route-options-object) | Options applied to the route | +| **metadata.labels** | [_label object_](#labels) | Labels applied to the route | +| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the route | +| **links** | [_links object_](#links) | Links to related resources | diff --git a/docs/v3/source/includes/resources/routes/_route_options_object.md.erb b/docs/v3/source/includes/resources/routes/_route_options_object.md.erb new file mode 100644 index 00000000000..48de982ed2f --- /dev/null +++ b/docs/v3/source/includes/resources/routes/_route_options_object.md.erb @@ -0,0 +1,12 @@ +### The route options object + +``` +Example Route-Options object +``` +```json +<%= yield_content :route_options %> +``` + +| Name | Type | Description | +|-------------------|----------|------------------------------------------------------------------------------------------------------| +| **loadbalancing** | _string_ | The load-balancer associated with this route. Valid values are 'round-robin' and 'least-connections' | diff --git a/docs/v3/source/includes/resources/routes/_update.md.erb b/docs/v3/source/includes/resources/routes/_update.md.erb index d1fa9d5ea79..60dcbf9be77 100644 --- a/docs/v3/source/includes/resources/routes/_update.md.erb +++ b/docs/v3/source/includes/resources/routes/_update.md.erb @@ -10,6 +10,9 @@ curl "https://api.example.org/v3/routes/[guid]" \ -H "Authorization: bearer [token]" \ -H "Content-type: application/json" \ -d '{ + "options": { + "loadbalancing": "round-robin" + } "metadata": { "labels": {"key": "value"}, "annotations": {"note": "detailed information"} @@ -33,14 +36,15 @@ Content-Type: application/json #### Optional parameters -| Name | Type | Description | Default | -| ----------- | -------- | ------------------------------------------------------------------------ | ------- | -| **metadata.labels** | [_label object_](#labels) | Labels applied to the route | | -| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the route | | +| Name | Type | Description | Default | +|----------------------------|-----------------------------------------------------|----------------------------------|---------| +| **options** | [_route option object_](#the-route-options-object) | Options applied to the route | | +| **metadata.labels** | [_label object_](#labels) | Labels applied to the route | | +| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the route | | #### Permitted roles - | +| ----- | Admin | Space Developer | diff --git a/docs/v3/source/index.html.md b/docs/v3/source/index.html.md index cc60bfea116..03c63aecaca 100644 --- a/docs/v3/source/index.html.md +++ b/docs/v3/source/index.html.md @@ -243,6 +243,7 @@ includes: - resources/routes/header - resources/routes/object - resources/routes/destination_object + - resources/routes/route_options_object - resources/routes/create - resources/routes/get - resources/routes/list diff --git a/lib/cloud_controller/app_manifest/manifest_route.rb b/lib/cloud_controller/app_manifest/manifest_route.rb index 74ad1493e72..113dfe99b9b 100644 --- a/lib/cloud_controller/app_manifest/manifest_route.rb +++ b/lib/cloud_controller/app_manifest/manifest_route.rb @@ -6,8 +6,8 @@ class ManifestRoute SUPPORTED_TCP_SCHEMES = %w[tcp unspecified].freeze WILDCARD_HOST = '*'.freeze - def self.parse(string) - parsed_uri = Addressable::URI.heuristic_parse(string, scheme: 'unspecified') + def self.parse(route, options=nil) + parsed_uri = Addressable::URI.heuristic_parse(route, scheme: 'unspecified') attrs = if parsed_uri.nil? {} @@ -15,7 +15,10 @@ def self.parse(string) parsed_uri.to_hash end - attrs[:full_route] = string + attrs[:full_route] = route + attrs[:options] = {} + attrs[:options][:loadbalancing] = options[:loadbalancing] if options && options.key?(:loadbalancing) + ManifestRoute.new(attrs) end @@ -43,7 +46,8 @@ def to_hash { candidate_host_domain_pairs: pairs, port: @attrs[:port], - path: @attrs[:path] + path: @attrs[:path], + options: @attrs[:options] } end diff --git a/lib/cloud_controller/diego/app_recipe_builder.rb b/lib/cloud_controller/diego/app_recipe_builder.rb index 46ce619baa6..f206e901349 100644 --- a/lib/cloud_controller/diego/app_recipe_builder.rb +++ b/lib/cloud_controller/diego/app_recipe_builder.rb @@ -151,13 +151,15 @@ def health_check_timeout_in_seconds def generate_routes(info) http_routes = (info['http_routes'] || []).map do |i| - { + http_route = { hostnames: [i['hostname']], port: i['port'], route_service_url: i['route_service_url'], isolation_segment: IsolationSegmentSelector.for_space(process.space), protocol: i['protocol'] } + http_route[:options] = i['options'] if i['options'] + http_route end { diff --git a/lib/cloud_controller/diego/protocol/routing_info.rb b/lib/cloud_controller/diego/protocol/routing_info.rb index 278f587f347..e85c061a4fd 100644 --- a/lib/cloud_controller/diego/protocol/routing_info.rb +++ b/lib/cloud_controller/diego/protocol/routing_info.rb @@ -43,6 +43,7 @@ def http_info(process_eager) info['router_group_guid'] = r.domain.router_group_guid if r.domain.is_a?(SharedDomain) && !r.domain.router_group_guid.nil? info['port'] = get_port_to_use(route_mapping) info['protocol'] = route_mapping.protocol + info['options'] = r.options if r.options info end end diff --git a/lib/cloud_controller/route_validator.rb b/lib/cloud_controller/route_validator.rb index 978fda7c02d..637a883ab38 100644 --- a/lib/cloud_controller/route_validator.rb +++ b/lib/cloud_controller/route_validator.rb @@ -4,6 +4,7 @@ class ValidationError < StandardError; end class DomainInvalid < ValidationError; end class RouteInvalid < ValidationError; end class RoutePortTaken < ValidationError; end + class InvalidRouteOptions < ValidationError; end attr_reader :route diff --git a/spec/request/app_manifests_spec.rb b/spec/request/app_manifests_spec.rb index 76c3d7ffca5..de384cb3208 100644 --- a/spec/request/app_manifests_spec.rb +++ b/spec/request/app_manifests_spec.rb @@ -97,11 +97,13 @@ 'routes' => [ { 'route' => "#{route.host}.#{route.domain.name}", - 'protocol' => 'http1' + 'protocol' => 'http1', + 'options' => {} }, { 'route' => "#{second_route.host}.#{second_route.domain.name}/path", - 'protocol' => 'http1' + 'protocol' => 'http1', + 'options' => {} } ], 'metadata' => { 'labels' => { 'potato' => 'idaho' }, 'annotations' => { 'style' => 'mashed' } }, @@ -201,11 +203,13 @@ 'routes' => [ { 'route' => "#{route.host}.#{route.domain.name}", - 'protocol' => 'http1' + 'protocol' => 'http1', + 'options' => {} }, { 'route' => "#{second_route.host}.#{second_route.domain.name}/path", - 'protocol' => 'http1' + 'protocol' => 'http1', + 'options' => {} } ], 'metadata' => { 'labels' => { 'potato' => 'idaho' }, 'annotations' => { 'style' => 'mashed' } }, @@ -273,11 +277,13 @@ 'routes' => [ { 'route' => "#{route.host}.#{route.domain.name}", - 'protocol' => 'http2' + 'protocol' => 'http2', + 'options' => {} }, { 'route' => "#{second_route.host}.#{second_route.domain.name}/path", - 'protocol' => 'http1' + 'protocol' => 'http1', + 'options' => {} } ] } diff --git a/spec/request/routes_spec.rb b/spec/request/routes_spec.rb index 82d154160e8..5a22276e405 100644 --- a/spec/request/routes_spec.rb +++ b/spec/request/routes_spec.rb @@ -89,6 +89,7 @@ labels: {}, annotations: {} }, + options: {}, links: { self: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{route_in_org.guid}} }, space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route_in_org.space.guid}} }, @@ -121,6 +122,7 @@ labels: {}, annotations: {} }, + options: {}, links: { self: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{route_in_other_org.guid}} }, space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route_in_other_org.space.guid}} }, @@ -305,6 +307,7 @@ } } }, + options: {}, links: { self: { href: "http://api2.vcap.me/v3/routes/#{route1_domain1.guid}" }, space: { href: "http://api2.vcap.me/v3/spaces/#{space.guid}" }, @@ -416,6 +419,7 @@ } } }, + options: {}, links: { self: { href: 'http://api2.vcap.me/v3/routes/route-without-host' }, space: { href: "http://api2.vcap.me/v3/spaces/#{space.guid}" }, @@ -451,6 +455,7 @@ } } }, + options: {}, links: { self: { href: 'http://api2.vcap.me/v3/routes/route-without-host2' }, space: { href: "http://api2.vcap.me/v3/spaces/#{space.guid}" }, @@ -489,6 +494,7 @@ } } }, + options: {}, links: { self: { href: 'http://api2.vcap.me/v3/routes/route-without-path' }, space: { href: "http://api2.vcap.me/v3/spaces/#{space.guid}" }, @@ -977,6 +983,7 @@ labels: {}, annotations: {} }, + options: {}, links: { self: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{UUID_REGEX}} }, space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route.space.guid}} }, @@ -1062,6 +1069,7 @@ labels: {}, annotations: {} }, + options: {}, links: { self: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{UUID_REGEX}} }, space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route.space.guid}} }, @@ -1213,7 +1221,8 @@ metadata: { labels: {}, annotations: {} - } + }, + options: {} } end @@ -1259,7 +1268,8 @@ metadata: { labels: { potato: 'yam' }, annotations: { style: 'mashed' } - } + }, + options: {} } end @@ -1291,7 +1301,8 @@ metadata: { labels: { potato: 'yam' }, annotations: { style: 'mashed' } - } + }, + options: {} } end @@ -1376,7 +1387,8 @@ metadata: { labels: {}, annotations: {} - } + }, + options: {} } end @@ -1474,7 +1486,8 @@ metadata: { labels: {}, annotations: {} - } + }, + options: {} } end @@ -1547,7 +1560,8 @@ metadata: { labels: {}, annotations: {} - } + }, + options: {} } end @@ -1634,7 +1648,8 @@ metadata: { labels: {}, annotations: {} - } + }, + options: {} } end @@ -1774,7 +1789,8 @@ space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{space.guid}} }, destinations: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{UUID_REGEX}/destinations} }, domain: { href: %r{#{Regexp.escape(link_prefix)}/v3/domains/#{domain.guid}} } - } + }, + options: {} } end @@ -1886,7 +1902,8 @@ metadata: { labels: {}, annotations: {} - } + }, + options: {} } end @@ -2119,7 +2136,8 @@ space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{space.guid}} }, destinations: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{UUID_REGEX}/destinations} }, domain: { href: %r{#{Regexp.escape(link_prefix)}/v3/domains/#{domain.guid}} } - } + }, + options: {} } end @@ -2435,7 +2453,8 @@ potato: 'russet', style: 'fried' } - } + }, + options: {} } end @@ -2506,7 +2525,8 @@ potato: 'russet', style: 'fried' } - } + }, + options: {} } end let(:expected_codes_and_responses) do @@ -3606,7 +3626,8 @@ space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route1.space.guid}} }, destinations: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{route1.guid}/destinations} }, domain: { href: %r{#{Regexp.escape(link_prefix)}/v3/domains/#{route1.domain.guid}} } - } + }, + options: {} } end @@ -3649,7 +3670,8 @@ space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route2.space.guid}} }, destinations: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{route2.guid}/destinations} }, domain: { href: %r{#{Regexp.escape(link_prefix)}/v3/domains/#{route2.domain.guid}} } - } + }, + options: {} } end diff --git a/spec/request/space_manifests_spec.rb b/spec/request/space_manifests_spec.rb index 74b77103bd0..55d3ca92fcd 100644 --- a/spec/request/space_manifests_spec.rb +++ b/spec/request/space_manifests_spec.rb @@ -606,7 +606,7 @@ before do app1_model.web_processes.first.update(state: VCAP::CloudController::ProcessModel::STARTED, instances: 4) space.update(space_quota_definition: - VCAP::CloudController::SpaceQuotaDefinition.make(organization: space.organization, log_rate_limit: 0)) + VCAP::CloudController::SpaceQuotaDefinition.make(organization: space.organization, log_rate_limit: 0)) end it 'successfully applies the manifest' do @@ -624,6 +624,234 @@ end end + describe 'route options' do + context 'when an invalid route option is provided' do + let(:yml_manifest) do + { + 'applications' => [ + { + 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{route.domain.name}", + 'options' => { + 'doesnt-exist' => 'doesnt-exist' + } } + ] + } + ] + }.to_yaml + end + + it 'returns a 422' do + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response).to have_status_code(422) + expect(last_response).to have_error_message("For application '#{app1_model.name}': \ +Route 'https://#{route.host}.#{route.domain.name}' contains invalid route option 'doesnt-exist'. Valid keys: 'loadbalancing'") + end + end + + context 'updating existing route options' do + # using loadbalancing as an example since it is the only route option currently supported + before do + yml_manifest = { + 'applications' => [ + { 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{shared_domain.name}", + 'options' => { + 'loadbalancing' => 'round-robin' + } } + ] } + ] + }.to_yaml + + # apply the manifest with the route option + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response.status).to eq(202) + job_guid = VCAP::CloudController::PollableJobModel.last.guid + + Delayed::Worker.new.work_off + expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error + app1_model.reload + expect(app1_model.routes.first.options).to eq({ 'loadbalancing' => 'round-robin' }) + end + + it 'updates the route option when a new value is provided' do + yml_manifest = { + 'applications' => [ + { 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{shared_domain.name}", + 'options' => { + 'loadbalancing' => 'least-connections' + } } + ] } + ] + }.to_yaml + + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response.status).to eq(202) + job_guid = VCAP::CloudController::PollableJobModel.last.guid + + Delayed::Worker.new.work_off + expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error + app1_model.reload + expect(app1_model.routes.first.options).to eq({ 'loadbalancing' => 'least-connections' }) + end + + it 'does not modify any route options when the options hash is not provided' do + yml_manifest = { + 'applications' => [ + { 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{shared_domain.name}" } + ] } + ] + }.to_yaml + + # apply the manifest with the route option + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response.status).to eq(202) + job_guid = VCAP::CloudController::PollableJobModel.last.guid + + Delayed::Worker.new.work_off + expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error + app1_model.reload + expect(app1_model.routes.first.options).to eq({ 'loadbalancing' => 'round-robin' }) + end + + it 'returns 422 when options: null is provided' do + yml_manifest = { + 'applications' => [ + { 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{shared_domain.name}", + 'options' => nil } + ] } + ] + }.to_yaml + + # apply the manifest with the route option + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response.status).to eq(422) + expect(last_response).to have_error_message("For application '#{app1_model.name}': \ +Route 'https://#{route.host}.#{route.domain.name}': options must be an object") + + job_guid = VCAP::CloudController::PollableJobModel.last.guid + Delayed::Worker.new.work_off + expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error + app1_model.reload + expect(app1_model.routes.first.options).to eq({ 'loadbalancing' => 'round-robin' }) + end + + it 'does not modify any route options if an empty options hash is provided' do + yml_manifest = { + 'applications' => [ + { 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{shared_domain.name}", + 'options' => {} } + ] } + ] + }.to_yaml + + # apply the manifest with the route option + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response.status).to eq(202) + + app1_model.reload + expect(app1_model.routes.first.options).to eq({ 'loadbalancing' => 'round-robin' }) + end + + it 'returns 422 when { loadbalancing: null } is provided' do + yml_manifest = { + 'applications' => [ + { 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{shared_domain.name}", + 'options' => { + 'loadbalancing' => nil + } } + ] } + ] + }.to_yaml + + # apply the manifest with the route option + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response.status).to eq(422) + expect(last_response).to have_error_message("For application '#{app1_model.name}': \ +Invalid value for 'loadbalancing' for Route 'https://#{route.host}.#{route.domain.name}'; Valid values are: 'round-robin, least-connections'") + + app1_model.reload + expect(app1_model.routes.first.options).to eq({ 'loadbalancing' => 'round-robin' }) + end + end + + context 'route-option: loadbalancing' do + context 'when the loadbalancing is not supported' do + let(:yml_manifest) do + { + 'applications' => [ + { + 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{route.domain.name}", + 'options' => { + 'loadbalancing' => 'unsupported-lb-algorithm' + } } + ] + } + ] + }.to_yaml + end + + it 'returns a 422' do + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response).to have_status_code(422) + expect(last_response).to have_error_message("For application '#{app1_model.name}': \ +Cannot use loadbalancing value 'unsupported-lb-algorithm' for Route 'https://#{route.host}.#{route.domain.name}'; Valid values are: 'round-robin, least-connections'") + end + end + + context 'when the loadbalancing is supported' do + let(:yml_manifest) do + { + 'applications' => [ + { 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{shared_domain.name}", + 'options' => { + 'loadbalancing' => 'round-robin' + } } + ] } + ] + }.to_yaml + end + + it 'adds the loadbalancing' do + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response.status).to eq(202) + job_guid = VCAP::CloudController::PollableJobModel.last.guid + + Delayed::Worker.new.work_off + expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error + + app1_model.reload + expect(app1_model.routes.first.options).to eq({ 'loadbalancing' => 'round-robin' }) + end + end + end + end + describe 'audit events' do let!(:process1) { nil } diff --git a/spec/support/legacy_api_dsl.rb b/spec/support/legacy_api_dsl.rb index c5c9c888291..54ab3deb48c 100644 --- a/spec/support/legacy_api_dsl.rb +++ b/spec/support/legacy_api_dsl.rb @@ -4,7 +4,7 @@ module LegacyApiDsl extend ActiveSupport::Concern def validate_response(model, json, expected_values: {}, ignored_attributes: [], expected_attributes: nil) - ignored_attributes.push :guid + ignored_attributes.push :guid, :options (expected_attributes || expected_attributes_for_model(model)).each do |expected_attribute| # refactor: pass exclusions, and figure out which are valid to not be there next if ignored_attributes.include? expected_attribute diff --git a/spec/unit/actions/route_update_spec.rb b/spec/unit/actions/route_update_spec.rb index 0e6464518c4..d9cbb94bd51 100644 --- a/spec/unit/actions/route_update_spec.rb +++ b/spec/unit/actions/route_update_spec.rb @@ -16,6 +16,9 @@ module VCAP::CloudController beet: 'formanova' } end + let(:old_options) do + '{"loadbalancing": "round-robin"}' + end let(:new_labels) do { cuisine: 'thai', @@ -41,7 +44,7 @@ module VCAP::CloudController let(:route) { Route.make } subject { RouteUpdate.new } - describe '#update' do + describe '#update metadata' do context 'when the route has no existing metadata' do context 'when no metadata is specified' do let(:body) do @@ -130,5 +133,94 @@ module VCAP::CloudController end end end + + describe '#update options' do + context 'when the route has no existing options' do + context 'when no options are specified' do + let(:body) do + {} + end + + it 'adds no options' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to eq({}) + end + end + + context 'when an option is specified' do + let(:body) do + { + options: { + loadbalancing: 'round-robin' + } + } + end + + it 'adds the route option' do + expect(message).to be_valid + subject.update(route:, message:) + + route.reload + expect(route[:options]).to eq('{"loadbalancing":"round-robin"}') + end + end + end + + context 'when the route has existing options' do + before do + route[:options] = '{"loadbalancing": "round-robin"}' + end + + context 'when no options are specified' do + let(:body) do + {} + end + + it 'modifies nothing' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to include({ 'loadbalancing' => 'round-robin' }) + end + end + + context 'when an option is specified' do + let(:body) do + { + options: { + loadbalancing: 'least-connections' + } + } + end + + it 'updates the option' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + + expect(route.options).to include({ 'loadbalancing' => 'least-connections' }) + end + end + + context 'when the option value is set to null' do + let(:body) do + { + options: { + loadbalancing: nil + } + } + end + + it 'removes this option' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to eq({}) + end + end + end + end end end diff --git a/spec/unit/jobs/deserialization_spec.rb b/spec/unit/jobs/deserialization_spec.rb index 604127c5920..5e3185b2ab6 100644 --- a/spec/unit/jobs/deserialization_spec.rb +++ b/spec/unit/jobs/deserialization_spec.rb @@ -217,6 +217,7 @@ module Jobs :query:#{' '} :fragment:#{' '} :full_route: app.bommel + :options: {} :protocol:#{' '} apply_manifest_action: !ruby/object:VCAP::CloudController::AppApplyManifest user_audit_info: &6 !ruby/object:VCAP::CloudController::UserAuditInfo diff --git a/spec/unit/lib/cloud_controller/app_manifest/manifest_route_spec.rb b/spec/unit/lib/cloud_controller/app_manifest/manifest_route_spec.rb index 29d4005a06d..6fa32f816aa 100644 --- a/spec/unit/lib/cloud_controller/app_manifest/manifest_route_spec.rb +++ b/spec/unit/lib/cloud_controller/app_manifest/manifest_route_spec.rb @@ -58,6 +58,16 @@ module VCAP::CloudController expect(manifest_route.valid?).to be(true) end end + + context 'when there is a valid loadbalancing' do + let(:route) { 'http://example.com' } + let(:options) { { 'loadbalancing' => 'round-robin' } } + + it 'is valid' do + manifest_route = ManifestRoute.parse(route, options) + expect(manifest_route.valid?).to be(true) + end + end end describe 'invalid routes' do @@ -119,7 +129,8 @@ module VCAP::CloudController { host: 'host', domain: 'sub.some-domain.com' } ], port: nil, - path: '/path' + path: '/path', + options: {} }) end @@ -131,7 +142,8 @@ module VCAP::CloudController { host: '*', domain: 'sub.some-domain.com' } ], port: nil, - path: '/path' + path: '/path', + options: {} }) end @@ -144,7 +156,8 @@ module VCAP::CloudController { host: 'potato', domain: 'sub.some-domain.com' } ], port: nil, - path: '/path' + path: '/path', + options: {} }) end @@ -157,7 +170,22 @@ module VCAP::CloudController { host: 'potato', domain: 'sub.some-domain.com' } ], port: 1234, - path: '' + path: '', + options: {} + }) + end + + it 'parses a route with a loadbalancing into route components' do + route = ManifestRoute.parse('http://potato.sub.some-domain.com', { loadbalancing: 'round-robin' }) + + expect(route.to_hash).to eq({ + candidate_host_domain_pairs: [ + { host: '', domain: 'potato.sub.some-domain.com' }, + { host: 'potato', domain: 'sub.some-domain.com' } + ], + port: nil, + path: '', + options: { loadbalancing: 'round-robin' } }) end end diff --git a/spec/unit/lib/cloud_controller/diego/protocol/routing_info_spec.rb b/spec/unit/lib/cloud_controller/diego/protocol/routing_info_spec.rb index 37e920291db..d74502cf615 100644 --- a/spec/unit/lib/cloud_controller/diego/protocol/routing_info_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/protocol/routing_info_spec.rb @@ -48,8 +48,8 @@ class Protocol it 'returns the mapped http routes associated with the app with protocol' do expected_http = [ - { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http2' }, - { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1' } + { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http2', 'options' => {} }, + { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1', 'options' => {} } ] expect(ri.keys).to match_array %w[http_routes internal_routes] @@ -66,8 +66,8 @@ class Protocol context 'and app has no ports' do it 'returns the mapped http routes associated with the app with a default of port 8080' do expected_http = [ - { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1' }, - { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1' } + { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1', 'options' => {} }, + { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1', 'options' => {} } ] expect(ri.keys).to match_array %w[http_routes internal_routes] @@ -80,8 +80,8 @@ class Protocol it 'uses the first port available on the app' do expected_http = [ - { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 7890, 'protocol' => 'http1' }, - { 'hostname' => route_without_service.uri, 'port' => 7890, 'protocol' => 'http1' } + { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 7890, 'protocol' => 'http1', 'options' => {} }, + { 'hostname' => route_without_service.uri, 'port' => 7890, 'protocol' => 'http1', 'options' => {} } ] expect(ri.keys).to match_array %w[http_routes internal_routes] @@ -113,8 +113,8 @@ class Protocol it 'uses 8080 as a default' do expected_http = [ - { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1' }, - { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1' } + { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1', 'options' => {} }, + { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1', 'options' => {} } ] expect(ri.keys).to match_array %w[http_routes internal_routes] @@ -127,8 +127,8 @@ class Protocol it 'uses the first docker port available on the app' do expected_http = [ - { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 1024, 'protocol' => 'http1' }, - { 'hostname' => route_without_service.uri, 'port' => 1024, 'protocol' => 'http1' } + { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 1024, 'protocol' => 'http1', 'options' => {} }, + { 'hostname' => route_without_service.uri, 'port' => 1024, 'protocol' => 'http1', 'options' => {} } ] expect(ri.keys).to match_array %w[http_routes internal_routes] @@ -154,7 +154,7 @@ class Protocol it 'returns the app port in routing info' do expected_http = [ - { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1' } + { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1', 'options' => {} } ] expect(ri.keys).to match_array %w[http_routes internal_routes] @@ -169,8 +169,8 @@ class Protocol it 'returns the app port in routing info' do expected_http = [ - { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1' }, - { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1' } + { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1', 'options' => {} }, + { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1', 'options' => {} } ] expect(ri.keys).to match_array %w[http_routes internal_routes] @@ -185,8 +185,8 @@ class Protocol it 'returns the app port in routing info' do expected_http = [ - { 'hostname' => route_without_service.uri, 'port' => 9090, 'protocol' => 'http1' }, - { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1' } + { 'hostname' => route_without_service.uri, 'port' => 9090, 'protocol' => 'http1', 'options' => {} }, + { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1', 'options' => {} } ] expect(ri.keys).to match_array %w[http_routes internal_routes] @@ -201,8 +201,8 @@ class Protocol it 'returns the app port in routing info' do expected_http = [ - { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1' }, - { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1' } + { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1', 'options' => {} }, + { 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1', 'options' => {} } ] expect(ri.keys).to match_array %w[http_routes internal_routes] expect(ri['http_routes']).to match_array expected_http @@ -216,7 +216,7 @@ class Protocol let!(:route_mapping2) { RouteMappingModel.make(app: process2.app, route: route_without_service) } it 'returns only one route without duplications' do - expected_http = { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1' } + expected_http = { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1', 'options' => {} } expect(ri.keys).to match_array %w[http_routes internal_routes] expect(ri['http_routes']).to contain_exactly expected_http @@ -232,7 +232,7 @@ class Protocol it 'returns the router group guid in the http routing info' do expect(ri.keys).to contain_exactly('http_routes', 'internal_routes') hr = ri['http_routes'][0] - expect(hr.keys).to contain_exactly('router_group_guid', 'port', 'hostname', 'protocol') + expect(hr.keys).to contain_exactly('router_group_guid', 'port', 'hostname', 'protocol', 'options') expect(hr['router_group_guid']).to eql(domain.router_group_guid) expect(hr['port']).to eql(http_route.port) expect(hr['hostname']).to match(/host-[0-9]+\.#{domain.name}/) diff --git a/spec/unit/messages/manifest_routes_update_message_spec.rb b/spec/unit/messages/manifest_routes_update_message_spec.rb index 2f894a81c71..60bab7e9639 100644 --- a/spec/unit/messages/manifest_routes_update_message_spec.rb +++ b/spec/unit/messages/manifest_routes_update_message_spec.rb @@ -208,6 +208,114 @@ module VCAP::CloudController expect(msg.errors.full_messages).to include("Route protocol must be 'http1', 'http2' or 'tcp'.") end end + + context 'when a route contains empty route options' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => {} } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains nil route option' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => nil } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com': options must be an object") + end + end + + context 'when a route contains invalid route options' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { 'invalid' => 'invalid' } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.errors.length).to eq(1) + expect(msg.errors.full_messages).to include("Route 'existing.example.com' contains invalid route option 'invalid'. Valid keys: 'loadbalancing'") + end + end + + context 'when a route contains a valid value for loadbalancing' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'round-robin' + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains null as a value for loadbalancing' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => nil + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Invalid value for 'loadbalancing' for Route 'existing.example.com'; Valid values are: 'round-robin, least-connections'") + end + end + + context 'when a route contains an invalid value for loadbalancing' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'sushi' + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.errors.length).to eq(1) + expect(msg.errors.full_messages).to include("Cannot use loadbalancing value 'sushi' for Route 'existing.example.com'; Valid values are: 'round-robin, least-connections'") + end + end end end end diff --git a/spec/unit/messages/route_create_message_spec.rb b/spec/unit/messages/route_create_message_spec.rb index 66bb5c6fb26..3fa936da526 100644 --- a/spec/unit/messages/route_create_message_spec.rb +++ b/spec/unit/messages/route_create_message_spec.rb @@ -19,6 +19,9 @@ module VCAP::CloudController metadata: { labels: { potato: 'yam' }, annotations: { style: 'mashed' } + }, + options: { + loadbalancing: 'round-robin' } } end @@ -354,6 +357,146 @@ module VCAP::CloudController end end end + + context 'options' do + context 'when not provided' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + } + } + end + + it 'is valid' do + expect(subject).to be_valid + end + end + + context 'when empty' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: {} + } + end + + it 'is valid' do + expect(subject).to be_valid + end + end + + context 'when nil' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: nil + } + end + + it 'is not valid' do + expect(subject).not_to be_valid + expect(subject.errors[:options]).to include("'options' is not a valid object") + end + end + + context 'when options has invalid key' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { foobar: 'baz' } + } + end + + it 'is not valid' do + expect(subject).not_to be_valid + expect(subject.errors[:options]).to include("Unknown field(s): 'foobar'") + end + end + + context 'when loadbalancing has value round-robin' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { loadbalancing: 'round-robin' } + } + end + + it 'is valid' do + expect(subject).to be_valid + end + + context 'when loadbalancing has value least-connections' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { loadbalancing: 'least-connections' } + } + end + + it 'is valid' do + expect(subject).to be_valid + end + end + + context 'when loadbalancing is nil' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { loadbalancing: nil } + } + end + + it 'is valid' do + expect(subject).to be_valid + end + end + + context 'when loadbalancing has invalid value' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { loadbalancing: 'random' } + } + end + + it 'is not valid' do + expect(subject).not_to be_valid + expect(subject.errors[:options]).to include("Loadbalancing must be one of 'round-robin, least-connections' if present") + end + end + end + end end describe 'accessor methods' do diff --git a/spec/unit/messages/route_update_message_spec.rb b/spec/unit/messages/route_update_message_spec.rb index f3a9c5a66f3..730b87d2ea0 100644 --- a/spec/unit/messages/route_update_message_spec.rb +++ b/spec/unit/messages/route_update_message_spec.rb @@ -18,11 +18,49 @@ module VCAP::CloudController expect(message).to be_valid end + it 'accepts options: {}' do + message = RouteUpdateMessage.new(params.merge(options: {})) + expect(message).to be_valid + end + + it 'accepts options params with round-robin load-balancing algorithm' do + message = RouteUpdateMessage.new(params.merge(options: { loadbalancing: 'round-robin' })) + expect(message).to be_valid + end + + it 'accepts options params with least-connections load-balancing algorithm' do + message = RouteUpdateMessage.new(params.merge(options: { loadbalancing: 'least-connections' })) + expect(message).to be_valid + end + + it 'does not accept options: nil' do + message = RouteUpdateMessage.new(params.merge(options: nil)) + expect(message).not_to be_valid + expect(message.errors.full_messages[0]).to include("Options 'options' is not a valid object") + end + + it 'accepts loadbalancing: nil to unset load-balancing algorithm' do + message = RouteUpdateMessage.new(params.merge(options: { loadbalancing: nil })) + expect(message).to be_valid + end + it 'does not accept any other params' do message = RouteUpdateMessage.new(params.merge(unexpected: 'unexpected_value')) expect(message).not_to be_valid expect(message.errors.full_messages[0]).to include("Unknown field(s): 'unexpected'") end + + it 'does not accept unknown load-balancing algorithm' do + message = RouteUpdateMessage.new(params.merge(options: { loadbalancing: 'cheesecake' })) + expect(message).not_to be_valid + expect(message.errors.full_messages[0]).to include("Options Loadbalancing must be one of 'round-robin, least-connections' if present") + end + + it 'does not accept unknown option' do + message = RouteUpdateMessage.new(params.merge(options: { gorgonzola: 'gouda' })) + expect(message).not_to be_valid + expect(message.errors.full_messages[0]).to include("Options Unknown field(s): 'gorgonzola'") + end end end end diff --git a/spec/unit/messages/validators_spec.rb b/spec/unit/messages/validators_spec.rb index 303a5f823c0..af49e16dc24 100644 --- a/spec/unit/messages/validators_spec.rb +++ b/spec/unit/messages/validators_spec.rb @@ -529,6 +529,62 @@ class Relationships < VCAP::CloudController::BaseMessage end end + describe 'OptionsValidator' do + class OptionsMessage < VCAP::CloudController::BaseMessage + register_allowed_keys [:options] + + def options_message + VCAP::CloudController::RouteOptionsMessage.new(options&.deep_symbolize_keys) + end + + validates_with OptionsValidator + end + + it 'successfully validates round-robin load-balancing algorithm' do + message = OptionsMessage.new({ options: { loadbalancing: 'round-robin' } }) + expect(message).to be_valid + end + + it 'successfully validates least-connections load-balancing algorithm' do + message = OptionsMessage.new({ options: { loadbalancing: 'least-connections' } }) + expect(message).to be_valid + end + + it 'successfully validates empty options' do + message = OptionsMessage.new({ options: {} }) + expect(message).to be_valid + end + + it 'adds invalid options message when options is null' do + message = OptionsMessage.new({ options: nil }) + expect(message).not_to be_valid + expect(message.errors_on(:options)).to include("'options' is not a valid object") + end + + it 'successfully validates empty load balancer' do + message = OptionsMessage.new({ options: { loadbalancing: nil } }) + expect(message).to be_valid + end + + it 'adds invalid object error message when options is not an object' do + message = OptionsMessage.new({ options: 'cheesecake' }) + expect(message).not_to be_valid + expect(message.errors_on(:options)).to include("'options' is not a valid object") + end + + it 'adds invalid load balancer error message to the base class' do + message = OptionsMessage.new({ options: { loadbalancing: 'donuts' } }) + expect(message).not_to be_valid + expect(message.errors_on(:options)).to include("Loadbalancing must be one of 'round-robin, least-connections' if present") + end + + it 'adds invalid field error message to the base class' do + message = OptionsMessage.new({ options: { cookies: 'round-robin' } }) + expect(message).not_to be_valid + expect(message.errors_on(:options)).to include('Unknown field(s): \'cookies\'') + end + end + describe 'ToOneRelationshipValidator' do let(:to_one_class) do Class.new(fake_class) do diff --git a/spec/unit/models/runtime/route_spec.rb b/spec/unit/models/runtime/route_spec.rb index 50e493ee45d..173d4cf1234 100644 --- a/spec/unit/models/runtime/route_spec.rb +++ b/spec/unit/models/runtime/route_spec.rb @@ -1049,8 +1049,8 @@ module VCAP::CloudController end describe 'Serialization' do - it { is_expected.to export_attributes :host, :domain_guid, :space_guid, :path, :service_instance_guid, :port } - it { is_expected.to import_attributes :host, :domain_guid, :space_guid, :app_guids, :path, :port } + it { is_expected.to export_attributes :host, :domain_guid, :space_guid, :path, :service_instance_guid, :port, :options } + it { is_expected.to import_attributes :host, :domain_guid, :space_guid, :app_guids, :path, :port, :options } end describe 'instance methods' do diff --git a/spec/unit/presenters/v3/app_manifest_presenter_spec.rb b/spec/unit/presenters/v3/app_manifest_presenter_spec.rb index c4b0a14de91..161f26171c5 100644 --- a/spec/unit/presenters/v3/app_manifest_presenter_spec.rb +++ b/spec/unit/presenters/v3/app_manifest_presenter_spec.rb @@ -111,8 +111,8 @@ module VCAP::CloudController::Presenters::V3 service_instance2.name ]) expect(application[:routes]).to eq([ - { route: route.uri, protocol: 'http1' }, - { route: route2.uri, protocol: 'http1' } + { route: route.uri, protocol: 'http1', options: {} }, + { route: route2.uri, protocol: 'http1', options: {} } ]) expect(application[:env]).to match({ 'one' => 'potato', 'two' => 'tomato' }) expect(application[:metadata]).to match({ labels: { 'potato' => 'idaho' }, annotations: { 'style' => 'mashed' } }) diff --git a/spec/unit/presenters/v3/route_presenter_spec.rb b/spec/unit/presenters/v3/route_presenter_spec.rb index c7c91dceea1..423e07c6eb5 100644 --- a/spec/unit/presenters/v3/route_presenter_spec.rb +++ b/spec/unit/presenters/v3/route_presenter_spec.rb @@ -5,6 +5,7 @@ module VCAP::CloudController::Presenters::V3 RSpec.describe RoutePresenter do let!(:app) { VCAP::CloudController::AppModel.make } let(:space) { VCAP::CloudController::Space.make } + let(:options) { { loadbalancing: 'round-robin' } } let(:org) { space.organization } let(:route_host) { 'host' } let(:path) { '/path' } @@ -20,7 +21,8 @@ module VCAP::CloudController::Presenters::V3 host: route_host, path: path, space: space, - domain: domain + domain: domain, + options: options ) end @@ -70,6 +72,7 @@ module VCAP::CloudController::Presenters::V3 expect(subject[:updated_at]).to be_a(Time) expect(subject[:host]).to eq(route_host) expect(subject[:path]).to eq(path) + expect(subject[:options]).to eq('loadbalancing' => 'round-robin') expect(subject[:url]).to eq("#{route.host}.#{domain.name}#{route.path}") expected_destinations = [ @@ -125,6 +128,21 @@ module VCAP::CloudController::Presenters::V3 end end + context 'when options is empty' do + let(:route) do + VCAP::CloudController::Route.make( + host: 'foobar', + path: path, + space: space, + domain: domain + ) + end + + it 'outputs empty options hash' do + expect(subject[:options]).to eq({}) + end + end + context 'when there are decorators' do let(:banana_decorator) do Class.new do