diff --git a/app/actions/manifest_route_update.rb b/app/actions/manifest_route_update.rb index e00182e0295..59fab7b4e6f 100644 --- a/app/actions/manifest_route_update.rb +++ b/app/actions/manifest_route_update.rb @@ -52,7 +52,7 @@ def update(app_guid, message, user_audit_info) ) end end - rescue Sequel::ValidationFailed, RouteCreate::Error => e + rescue Sequel::ValidationFailed, RouteCreate::Error, RouteUpdate::Error => e raise InvalidRoute.new(e.message) end @@ -94,9 +94,7 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info) ) elsif !route.available_in_space?(app.space) 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 + elsif manifest_route[:options] message = RouteUpdateMessage.new({ 'options' => manifest_route[:options] }) diff --git a/app/actions/route_create.rb b/app/actions/route_create.rb index 15be9568456..5a494b798e1 100644 --- a/app/actions/route_create.rb +++ b/app/actions/route_create.rb @@ -68,22 +68,34 @@ def route_resource_manager end def validation_error!(error, host, path, port, space, domain) - error!("Invalid domain. Domain '#{domain.name}' is not available in organization '#{space.organization.name}'.") if error.errors.on(:domain)&.include?(:invalid_relation) + validation_error_domain!(error, space, domain) + validation_error_quota!(error, space) + validation_error_route!(error) + validation_error_routing_api!(error) + validation_error_host!(error, host, domain) + validation_error_path!(error, host, path, domain) + validation_error_port!(error, host, port, domain) - error!("Routes quota exceeded for space '#{space.name}'.") if error.errors.on(:space)&.include?(:total_routes_exceeded) + error!(error.message) + end - error!("Reserved route ports quota exceeded for space '#{space.name}'.") if error.errors.on(:space)&.include?(:total_reserved_route_ports_exceeded) + def validation_error_domain!(error, space, domain) + return unless error.errors.on(:domain)&.include?(:invalid_relation) - error!("Routes quota exceeded for organization '#{space.organization.name}'.") if error.errors.on(:organization)&.include?(:total_routes_exceeded) + error!("Invalid domain. Domain '#{domain.name}' is not available in organization '#{space.organization.name}'.") + end + def validation_error_quota!(error, space) + error!("Routes quota exceeded for space '#{space.name}'.") if error.errors.on(:space)&.include?(:total_routes_exceeded) + error!("Reserved route ports quota exceeded for space '#{space.name}'.") if error.errors.on(:space)&.include?(:total_reserved_route_ports_exceeded) + error!("Routes quota exceeded for organization '#{space.organization.name}'.") if error.errors.on(:organization)&.include?(:total_routes_exceeded) error!("Reserved route ports quota exceeded for organization '#{space.organization.name}'.") if error.errors.on(:organization)&.include?(:total_reserved_route_ports_exceeded) + end - validation_error_routing_api!(error) - validation_error_host!(error, host, domain) - validation_error_path!(error, host, path, domain) - validation_error_port!(error, host, port, domain) + def validation_error_route!(error) + return unless error.errors.on(:route)&.include?(:hash_header_missing) - error!(error.message) + error!('Hash header must be present when loadbalancing is set to hash.') end def validation_error_routing_api!(error) diff --git a/app/actions/route_update.rb b/app/actions/route_update.rb index 3520ba17877..df3a325f378 100644 --- a/app/actions/route_update.rb +++ b/app/actions/route_update.rb @@ -1,8 +1,11 @@ module VCAP::CloudController class RouteUpdate + class Error < StandardError + end + def update(route:, message:) Route.db.transaction do - route.options = route.options.symbolize_keys.merge(message.options).compact if message.requested?(:options) + route.options = route.options.symbolize_keys.merge(message.options) if message.requested?(:options) route.save MetadataUpdate.update(route, message) end @@ -13,6 +16,18 @@ def update(route:, message:) end end route + rescue Sequel::ValidationFailed => e + validation_error!(e) + end + + private + + def validation_error!(error) + # Handle hash_header validation error for hash loadbalancing + raise Error.new('Hash header must be present when loadbalancing is set to hash.') if error.errors.on(:route)&.include?(:hash_header_missing) + + # Fallback for any other validation errors + raise Error.new(error.message) end end end diff --git a/app/controllers/v3/routes_controller.rb b/app/controllers/v3/routes_controller.rb index 86f042dc3a7..7c7e48ce212 100644 --- a/app/controllers/v3/routes_controller.rb +++ b/app/controllers/v3/routes_controller.rb @@ -108,6 +108,8 @@ def update VCAP::CloudController::RouteUpdate.new.update(route:, message:) render status: :ok, json: Presenters::V3::RoutePresenter.new(route) + rescue RouteUpdate::Error => e + unprocessable!(e.message) end def destroy diff --git a/app/messages/manifest_routes_update_message.rb b/app/messages/manifest_routes_update_message.rb index bc4c6b9cb5e..f00003ac550 100644 --- a/app/messages/manifest_routes_update_message.rb +++ b/app/messages/manifest_routes_update_message.rb @@ -28,6 +28,7 @@ def contains_non_route_hash_values?(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 :hash_options_are_valid, if: proc { |record| record.requested?(:routes) } validate :no_route_is_boolean validate :default_route_is_boolean validate :random_route_is_boolean @@ -58,10 +59,10 @@ def route_options_are_valid end r[:options].each_key do |key| - RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.exclude?(key) && + RouteOptionsMessage.valid_route_options.exclude?(key) && errors.add(:base, message: "Route '#{r[:route]}' contains invalid route option '#{key}'. \ -Valid keys: '#{RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.join(', ')}'") +Valid keys: '#{RouteOptionsMessage.valid_route_options.join(', ')}'") end end end @@ -76,16 +77,81 @@ def loadbalancings_are_valid 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(', ')}'") +Valid values are: '#{RouteOptionsMessage.valid_loadbalancing_algorithms.join(', ')}'") next end - RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(loadbalancing) && + 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(', ')}'") +Valid values are: '#{RouteOptionsMessage.valid_loadbalancing_algorithms.join(', ')}'") end end + def hash_options_are_valid + return if errors[:routes].present? + + # Only validate hash-specific options when the feature flag is enabled + # If disabled, route_options_are_valid will already report them as invalid + return unless VCAP::CloudController::FeatureFlag.enabled?(:hash_based_routing) + + # NOTE: route_options_are_valid already validates that hash_header and hash_balance + # are only allowed when the feature flag is enabled (via valid_route_options). + + routes.each do |r| + next unless r.keys.include?(:options) && r[:options].is_a?(Hash) + + validate_route_hash_options(r) + end + end + + def validate_route_hash_options(route) + options = route[:options] + loadbalancing = options[:loadbalancing] + hash_header = options[:hash_header] + hash_balance = options[:hash_balance] + + validate_route_hash_header(route, hash_header) + validate_route_hash_balance(route, hash_balance) + + validate_route_hash_options_with_loadbalancing(route, loadbalancing, hash_header, hash_balance) + end + + def validate_route_hash_header(route, hash_header) + return unless hash_header.present? && (hash_header.to_s.length > 128) + + errors.add(:base, message: "Route '#{route[:route]}': Hash header must be at most 128 characters") + end + + def validate_route_hash_balance(route, hash_balance) + return if hash_balance.blank? + + if hash_balance.to_s.length > 16 + errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be at most 16 characters") + return + end + + validate_route_hash_balance_numeric(route, hash_balance) + end + + def validate_route_hash_balance_numeric(route, hash_balance) + balance_float = Float(hash_balance) + # Must be either 0 or >= 1.1 and <= 10.0 + errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be either 0 or between 1.1 and 10.0") unless balance_float == 0 || balance_float.between?(1.1, 10) + rescue ArgumentError, TypeError + errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be a numeric value") + end + + def validate_route_hash_options_with_loadbalancing(route, loadbalancing, hash_header, hash_balance) + # When loadbalancing is explicitly set to non-hash value, hash options are not allowed + if hash_header.present? && loadbalancing.present? && loadbalancing != 'hash' + errors.add(:base, message: "Route '#{route[:route]}': Hash header can only be set when loadbalancing is hash") + end + + return unless hash_balance.present? && loadbalancing.present? && loadbalancing != 'hash' + + errors.add(:base, message: "Route '#{route[:route]}': Hash balance can only be set when loadbalancing is hash") + end + def routes_are_uris return if errors[:routes].present? diff --git a/app/messages/route_options_message.rb b/app/messages/route_options_message.rb index d2fd43e28a6..f79a2bb6a0c 100644 --- a/app/messages/route_options_message.rb +++ b/app/messages/route_options_message.rb @@ -2,15 +2,85 @@ 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-connection].freeze + # Register all possible keys upfront so attr_accessors are created + register_allowed_keys %i[loadbalancing hash_header hash_balance] + + def self.valid_route_options + options = %i[loadbalancing] + options += %i[hash_header hash_balance] if VCAP::CloudController::FeatureFlag.enabled?(:hash_based_routing) + options.freeze + end + + def self.valid_loadbalancing_algorithms + algorithms = %w[round-robin least-connection] + algorithms << 'hash' if VCAP::CloudController::FeatureFlag.enabled?(:hash_based_routing) + algorithms.freeze + end - 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 + validate :loadbalancing_algorithm_is_valid + validate :route_options_are_valid + validate :hash_options_are_valid + + def loadbalancing_algorithm_is_valid + return if loadbalancing.blank? + return if self.class.valid_loadbalancing_algorithms.include?(loadbalancing) + + errors.add(:loadbalancing, "must be one of '#{self.class.valid_loadbalancing_algorithms.join(', ')}' if present") + end + + def route_options_are_valid + valid_options = self.class.valid_route_options + + # Check if any requested options are not in valid_route_options + # Check needs to be done manually, as the set of allowed options may change during runtime (feature flag) + invalid_keys = requested_keys.select do |key| + value = public_send(key) if respond_to?(key) + value.present? && valid_options.exclude?(key) + end + + errors.add(:base, "Unknown field(s): '#{invalid_keys.join("', '")}'") if invalid_keys.any? + end + + def hash_options_are_valid + # Only validate hash-specific options when the feature flag is enabled + # If disabled, route_options_are_valid will already report them as unknown fields + return unless VCAP::CloudController::FeatureFlag.enabled?(:hash_based_routing) + + validate_hash_header_length + validate_hash_balance_value + validate_hash_options_with_loadbalancing + end + + def validate_hash_header_length + return unless hash_header.present? && (hash_header.to_s.length > 128) + + errors.add(:hash_header, 'must be at most 128 characters') + end + + def validate_hash_balance_value + return if hash_balance.blank? + + if hash_balance.to_s.length > 16 + errors.add(:hash_balance, 'must be at most 16 characters') + return + end + + validate_hash_balance_numeric + end + + def validate_hash_balance_numeric + balance_float = Float(hash_balance) + # Must be either 0 or >= 1.1 and <= 10.0 + errors.add(:hash_balance, 'must be either 0 or between 1.1 and 10.0') unless balance_float == 0 || balance_float.between?(1.1, 10) + rescue ArgumentError, TypeError + errors.add(:hash_balance, 'must be a numeric value') + end + + def validate_hash_options_with_loadbalancing + # When loadbalancing is explicitly set to non-hash value, hash options are not allowed + errors.add(:base, 'Hash header can only be set when loadbalancing is hash') if hash_header.present? && loadbalancing.present? && loadbalancing != 'hash' + errors.add(:base, 'Hash balance can only be set when loadbalancing is hash') if hash_balance.present? && loadbalancing.present? && loadbalancing != 'hash' + end end end diff --git a/app/models/runtime/feature_flag.rb b/app/models/runtime/feature_flag.rb index 88a5c24a353..df991d89f7e 100644 --- a/app/models/runtime/feature_flag.rb +++ b/app/models/runtime/feature_flag.rb @@ -23,7 +23,8 @@ class UndefinedFeatureFlagError < StandardError service_instance_sharing: false, hide_marketplace_from_unauthenticated_users: false, resource_matching: true, - route_sharing: false + route_sharing: false, + hash_based_routing: false }.freeze ADMIN_SKIPPABLE = %i[ diff --git a/app/models/runtime/route.rb b/app/models/runtime/route.rb index 70fd484ca42..1a54bd72b44 100644 --- a/app/models/runtime/route.rb +++ b/app/models/runtime/route.rb @@ -71,7 +71,12 @@ def as_summary_json end def options_with_serialization=(opts) - self.options_without_serialization = Oj.dump(opts) + cleaned_opts = remove_hash_options_for_non_hash_loadbalancing(opts) + rounded_opts = round_hash_balance_to_one_decimal(cleaned_opts) + normalized_opts = normalize_hash_balance_to_string(rounded_opts) + # Remove nil values after all processing + normalized_opts = normalized_opts.compact if normalized_opts.is_a?(Hash) + self.options_without_serialization = Oj.dump(normalized_opts) end alias_method :options_without_serialization=, :options= @@ -125,6 +130,7 @@ def validate validate_total_routes validate_ports validate_total_reserved_route_ports if port && port > 0 + validate_route_options RouteValidator.new(self).validate rescue RoutingApi::UaaUnavailable @@ -317,6 +323,64 @@ def validate_total_reserved_route_ports errors.add(:organization, :total_reserved_route_ports_exceeded) end + def validate_route_options + return if options.blank? + + route_options = options.is_a?(Hash) ? options : options.symbolize_keys + loadbalancing = route_options[:loadbalancing] || route_options['loadbalancing'] + + return if loadbalancing != 'hash' + + hash_header = route_options[:hash_header] || route_options['hash_header'] + + return if hash_header.present? + + errors.add(:route, :hash_header_missing) + end + + def round_hash_balance_to_one_decimal(opts) + return opts unless opts.is_a?(Hash) + + opts_symbolized = opts.symbolize_keys + hash_balance = opts_symbolized[:hash_balance] + + if hash_balance.present? + begin + balance_float = Float(hash_balance) + # Round to at most 1 decimal place + opts_symbolized[:hash_balance] = (balance_float * 10).round / 10.0 + rescue ArgumentError, TypeError + # If conversion fails, leave it as is - validation will catch it + end + end + + opts_symbolized + end + + def normalize_hash_balance_to_string(opts) + return opts unless opts.is_a?(Hash) + + # We have a flat structure on options, so no deep_symbolize required + normalized = opts.symbolize_keys + normalized[:hash_balance] = normalized[:hash_balance].to_s if normalized[:hash_balance].present? + normalized + end + + def remove_hash_options_for_non_hash_loadbalancing(opts) + return opts unless opts.is_a?(Hash) + + opts_symbolized = opts.symbolize_keys + loadbalancing = opts_symbolized[:loadbalancing] + + # Remove hash-specific options if loadbalancing is set to non-hash value + if loadbalancing != 'hash' + opts_symbolized.delete(:hash_header) + opts_symbolized.delete(:hash_balance) + end + + opts_symbolized + end + def validate_uniqueness_on_host_and_domain validates_unique [:host, :domain_id] do |ds| ds.where(path: '', port: 0) 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 d58c52c3003..824567e1b52 100644 --- a/app/presenters/v3/app_manifest_presenters/route_properties_presenter.rb +++ b/app/presenters/v3/app_manifest_presenters/route_properties_presenter.rb @@ -11,10 +11,13 @@ def to_hash(route_mappings:, app:, **_) } if route_mapping.route.options + opts = route_mapping.route.options + route_hash[:options] = {} - route_hash[:options][:loadbalancing] = route_mapping.route.options[:loadbalancing] if route_mapping.route.options[:loadbalancing] + route_hash[:options][:loadbalancing] = opts['loadbalancing'] if opts.key?('loadbalancing') + route_hash[:options][:hash_header] = opts['hash_header'] if opts.key?('hash_header') + route_hash[:options][:hash_balance] = opts['hash_balance'] if opts.key?('hash_balance') end - route_hash end diff --git a/app/repositories/app_event_repository.rb b/app/repositories/app_event_repository.rb index 84e3e4023e5..4b1c99c14a7 100644 --- a/app/repositories/app_event_repository.rb +++ b/app/repositories/app_event_repository.rb @@ -110,6 +110,7 @@ def record_map_route(user_audit_info, route_mapping, manifest_triggered: false) weight: route_mapping.weight, protocol: route_mapping.protocol }) + metadata[:route_options] = route.options if route.options.present? create_app_audit_event(EventTypes::APP_MAP_ROUTE, app, app.space, actor_hash, metadata) end @@ -126,6 +127,7 @@ def record_unmap_route(user_audit_info, route_mapping, manifest_triggered: false weight: route_mapping.weight, protocol: route_mapping.protocol }) + metadata[:route_options] = route.options if route.options.present? create_app_audit_event(EventTypes::APP_UNMAP_ROUTE, app, app.space, actor_hash, metadata) end diff --git a/docs/v3/source/includes/resources/routes/_create.md.erb b/docs/v3/source/includes/resources/routes/_create.md.erb index 6ad5b93a9f9..702ce526d7c 100644 --- a/docs/v3/source/includes/resources/routes/_create.md.erb +++ b/docs/v3/source/includes/resources/routes/_create.md.erb @@ -23,7 +23,7 @@ curl "https://api.example.org/v3/routes" \ }, "options": { "loadbalancing": "round-robin" - } + }, "metadata": { "labels": { "key": "value" }, "annotations": { "note": "detailed information"} @@ -71,3 +71,30 @@ Admin | Space Developer | Space Supporter | +#### Example with hash-based routing + +```shell +curl "https://api.example.org/v3/routes" \ + -X POST \ + -H "Authorization: bearer [token]" \ + -H "Content-type: application/json" \ + -d '{ + "host": "user-specific-app", + "relationships": { + "domain": { + "data": { "guid": "domain-guid" } + }, + "space": { + "data": { "guid": "space-guid" } + } + }, + "options": { + "loadbalancing": "hash", + "hash_header": "X-User-ID", + "hash_balance": "50.0" + } + }' +``` + +This creates a route that uses hash-based routing on the `X-User-ID` header with a load balance factor of 50.0. + 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 index 01fc9c82b56..53729e872fa 100644 --- a/docs/v3/source/includes/resources/routes/_route_options_object.md.erb +++ b/docs/v3/source/includes/resources/routes/_route_options_object.md.erb @@ -7,6 +7,8 @@ Example Route-Options object <%= yield_content :route_options %> ``` -| Name | Type | Description | -|-------------------|----------|------------------------------------------------------------------------------------------------------| -| **loadbalancing** | _string_ | The load-balancer associated with this route. Valid values are 'round-robin' and 'least-connection' | +| Name | Type | Description | +|-------------------|----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| **loadbalancing** | _string_ | The load-balancer associated with this route. Valid values are 'round-robin', 'least-connection', and 'hash' | +| **hash_header** | _string_ | HTTP header name to hash for routing (e.g., 'X-User-ID', 'Cookie'). Required when loadbalancing is 'hash'. Cannot be set when loadbalancing is not 'hash'. | +| **hash_balance** | _string_ | Weight factor for load balancing (1.1 - 10, or 0 for disabling balancing). Higher values consider load more. Optional when loadbalancing is 'hash'. Cannot be set when loadbalancing is not 'hash'. | diff --git a/lib/cloud_controller/app_manifest/manifest_route.rb b/lib/cloud_controller/app_manifest/manifest_route.rb index 113dfe99b9b..3c68ffc9fe0 100644 --- a/lib/cloud_controller/app_manifest/manifest_route.rb +++ b/lib/cloud_controller/app_manifest/manifest_route.rb @@ -18,6 +18,8 @@ def self.parse(route, options=nil) attrs[:full_route] = route attrs[:options] = {} attrs[:options][:loadbalancing] = options[:loadbalancing] if options && options.key?(:loadbalancing) + attrs[:options][:hash_header] = options[:hash_header] if options && options.key?(:hash_header) + attrs[:options][:hash_balance] = options[:hash_balance] if options && options.key?(:hash_balance) ManifestRoute.new(attrs) end diff --git a/spec/api/documentation/feature_flags_api_spec.rb b/spec/api/documentation/feature_flags_api_spec.rb index ce55fbd2c6b..5fe6740eb30 100644 --- a/spec/api/documentation/feature_flags_api_spec.rb +++ b/spec/api/documentation/feature_flags_api_spec.rb @@ -18,7 +18,7 @@ client.get '/v2/config/feature_flags', {}, headers expect(status).to eq(200) - expect(parsed_response.length).to eq(18) + expect(parsed_response.length).to eq(19) expect(parsed_response).to include( { 'name' => 'user_org_creation', diff --git a/spec/unit/actions/manifest_route_update_spec.rb b/spec/unit/actions/manifest_route_update_spec.rb index 623aa6384b6..35d6e8a5e19 100644 --- a/spec/unit/actions/manifest_route_update_spec.rb +++ b/spec/unit/actions/manifest_route_update_spec.rb @@ -393,6 +393,270 @@ module VCAP::CloudController end.to raise_error(ManifestRouteUpdate::InvalidRoute, /Host format is invalid/) end end + + context 'when route options are provided' do + let!(:domain) { VCAP::CloudController::SharedDomain.make(name: 'tomato.avocado-toast.com') } + + before do + VCAP::CloudController::FeatureFlag.make(name: 'hash_based_routing', enabled: true) + end + + context 'when creating a new route with loadbalancing options' do + let(:message) do + ManifestRoutesUpdateMessage.new( + routes: [ + { + route: 'http://potato.tomato.avocado-toast.com/some-path', + options: { loadbalancing: 'round-robin' } + } + ] + ) + end + + it 'creates the route with the specified loadbalancing option' do + ManifestRouteUpdate.update(app.guid, message, user_audit_info) + + routes = app.reload.routes + expect(routes.length).to eq(1) + + route = routes.first + expect(route.options).to include({ 'loadbalancing' => 'round-robin' }) + end + end + + context 'when creating a new route with hash loadbalancing and hash_header' do + let(:message) do + ManifestRoutesUpdateMessage.new( + routes: [ + { + route: 'http://potato.tomato.avocado-toast.com/some-path', + options: { + loadbalancing: 'hash', + hash_header: 'X-User-ID' + } + } + ] + ) + end + + it 'creates the route with hash loadbalancing options' do + ManifestRouteUpdate.update(app.guid, message, user_audit_info) + + routes = app.reload.routes + expect(routes.length).to eq(1) + + route = routes.first + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-User-ID' }) + end + end + + context 'when creating a new route with hash loadbalancing, hash_header, and hash_balance' do + let(:message) do + ManifestRoutesUpdateMessage.new( + routes: [ + { + route: 'http://potato.tomato.avocado-toast.com/some-path', + options: { + loadbalancing: 'hash', + hash_header: 'X-Session-ID', + hash_balance: '2.5' + } + } + ] + ) + end + + it 'creates the route with all hash loadbalancing options' do + ManifestRouteUpdate.update(app.guid, message, user_audit_info) + + routes = app.reload.routes + expect(routes.length).to eq(1) + + route = routes.first + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-Session-ID', 'hash_balance' => '2.5' }) + end + end + + context 'when creating a new route with hash loadbalancing but missing hash_header' do + let(:message) do + ManifestRoutesUpdateMessage.new( + routes: [ + { + route: 'http://potato.tomato.avocado-toast.com/some-path', + options: { loadbalancing: 'hash' } + } + ] + ) + end + + it 'raises an error indicating hash_header is required' do + expect do + ManifestRouteUpdate.update(app.guid, message, user_audit_info) + end.to raise_error(ManifestRouteUpdate::InvalidRoute, /Hash header must be present when loadbalancing is set to hash./) + end + end + + context 'when updating an existing route with new loadbalancing options' do + let!(:route) { Route.make(host: 'potato', domain: domain, path: '/some-path', space: app.space) } + let(:message) do + ManifestRoutesUpdateMessage.new( + routes: [ + { + route: 'http://potato.tomato.avocado-toast.com/some-path', + options: { loadbalancing: 'least-connection' } + } + ] + ) + end + + it 'updates the existing route with the new loadbalancing option' do + expect do + ManifestRouteUpdate.update(app.guid, message, user_audit_info) + end.not_to(change(Route, :count)) + + route.reload + expect(route.options).to include({ 'loadbalancing' => 'least-connection' }) + end + end + + context 'when updating an existing route from round-robin to hash' do + let!(:route) do + Route.make( + host: 'potato', + domain: domain, + path: '/some-path', + space: app.space, + options: { loadbalancing: 'round-robin' } + ) + end + + let(:message) do + ManifestRoutesUpdateMessage.new( + routes: [ + { + route: 'http://potato.tomato.avocado-toast.com/some-path', + options: { + loadbalancing: 'hash', + hash_header: 'X-User-ID' + } + } + ] + ) + end + + it 'updates the route to hash loadbalancing with hash_header' do + ManifestRouteUpdate.update(app.guid, message, user_audit_info) + + route.reload + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-User-ID' }) + end + end + + context 'when updating an existing hash route with new hash_header' do + let!(:route) do + Route.make( + host: 'potato', + domain: domain, + path: '/some-path', + space: app.space, + options: { + loadbalancing: 'hash', + hash_header: 'X-Old-Header', + hash_balance: '2.0' + } + ) + end + + let(:message) do + ManifestRoutesUpdateMessage.new( + routes: [ + { + route: 'http://potato.tomato.avocado-toast.com/some-path', + options: { hash_header: 'X-New-Header' } + } + ] + ) + end + + it 'updates only the hash_header while keeping other options' do + ManifestRouteUpdate.update(app.guid, message, user_audit_info) + + route.reload + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-New-Header', 'hash_balance' => '2.0' }) + end + end + + context 'when updating an existing hash route with new hash_balance' do + let!(:route) do + Route.make( + host: 'potato', + domain: domain, + path: '/some-path', + space: app.space, + options: { + loadbalancing: 'hash', + hash_header: 'X-User-ID', + hash_balance: '2.0' + } + ) + end + + let(:message) do + ManifestRoutesUpdateMessage.new( + routes: [ + { + route: 'http://potato.tomato.avocado-toast.com/some-path', + options: { hash_balance: '5.0' } + } + ] + ) + end + + it 'updates only the hash_balance while keeping other options' do + ManifestRouteUpdate.update(app.guid, message, user_audit_info) + + route.reload + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-User-ID', 'hash_balance' => '5.0' }) + end + end + + context 'when updating an existing hash route to remove loadbalancing' do + let!(:route) do + Route.make( + host: 'potato', + domain: domain, + path: '/some-path', + space: app.space, + options: { + loadbalancing: 'hash', + hash_header: 'X-User-ID', + hash_balance: '2.0' + } + ) + end + + let(:message) do + ManifestRoutesUpdateMessage.new( + routes: [ + { + route: 'http://potato.tomato.avocado-toast.com/some-path', + options: { loadbalancing: nil } + } + ] + ) + end + + it 'removes loadbalancing and hash options' do + ManifestRouteUpdate.update(app.guid, message, user_audit_info) + + route.reload + expect(route.options).to eq({}) + expect(route.options).not_to have_key('loadbalancing') + expect(route.options).not_to have_key('hash_header') + expect(route.options).not_to have_key('hash_balance') + end + end + end end end end diff --git a/spec/unit/actions/route_create_spec.rb b/spec/unit/actions/route_create_spec.rb index fe40a2b50a4..86fdca59888 100644 --- a/spec/unit/actions/route_create_spec.rb +++ b/spec/unit/actions/route_create_spec.rb @@ -108,6 +108,118 @@ module VCAP::CloudController end end + context 'when given route options' do + context 'when creating a route with loadbalancing=hash' do + context 'with hash_header but without hash_balance' do + let(:message_with_hash_options) do + RouteCreateMessage.new({ + relationships: { + space: { + data: { guid: space.guid } + }, + domain: { + data: { guid: domain.guid } + } + }, + options: { + loadbalancing: 'hash', + hash_header: 'X-User-ID' + } + }) + end + + it 'creates a route with hash loadbalancing and hash_header options' do + expect do + subject.create(message: message_with_hash_options, space: space, domain: domain) + end.to change(Route, :count).by(1) + + route = Route.last + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-User-ID' }) + end + end + + context 'with both hash_header and hash_balance' do + let(:message_with_hash_options) do + RouteCreateMessage.new({ + relationships: { + space: { + data: { guid: space.guid } + }, + domain: { + data: { guid: domain.guid } + } + }, + options: { + loadbalancing: 'hash', + hash_header: 'X-Session-ID', + hash_balance: '2' + } + }) + end + + it 'creates a route with hash loadbalancing, hash_header, and hash_balance options' do + expect do + subject.create(message: message_with_hash_options, space: space, domain: domain) + end.to change(Route, :count).by(1) + + route = Route.last + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-Session-ID', 'hash_balance' => '2.0' }) + end + end + + context 'without hash_header (required)' do + let(:message_without_hash_header) do + RouteCreateMessage.new({ + relationships: { + space: { + data: { guid: space.guid } + }, + domain: { + data: { guid: domain.guid } + } + }, + options: { + loadbalancing: 'hash' + } + }) + end + + it 'raises an error indicating hash_header is required' do + expect do + subject.create(message: message_without_hash_header, space: space, domain: domain) + end.to raise_error(RouteCreate::Error, 'Hash header must be present when loadbalancing is set to hash.') + end + end + end + + context 'when creating a route with other loadbalancing options' do + let(:message_with_round_robin) do + RouteCreateMessage.new({ + relationships: { + space: { + data: { guid: space.guid } + }, + domain: { + data: { guid: domain.guid } + } + }, + options: { + loadbalancing: 'round-robin' + } + }) + end + + it 'creates a route with the specified loadbalancing option' do + expect do + subject.create(message: message_with_round_robin, space: space, domain: domain) + end.to change(Route, :count).by(1) + + route = Route.last + expect(route.options).to include({ 'loadbalancing' => 'round-robin' }) + end + end + end + context 'when the domain has an owning org that is different from the space\'s parent org' do let(:other_org) { VCAP::CloudController::Organization.make } let(:inaccessible_domain) { VCAP::CloudController::PrivateDomain.make(owning_organization: other_org) } diff --git a/spec/unit/actions/route_update_spec.rb b/spec/unit/actions/route_update_spec.rb index 472714d2cad..be966d96fe2 100644 --- a/spec/unit/actions/route_update_spec.rb +++ b/spec/unit/actions/route_update_spec.rb @@ -190,7 +190,191 @@ module VCAP::CloudController end end - context 'when the route has existing options' do + context 'when the route has existing options for loadbalancing=hash' do + before do + VCAP::CloudController::FeatureFlag.make(name: 'hash_based_routing', enabled: true) + route[:options] = '{"loadbalancing": "hash", "hash_header": "foobar", "hash_balance": "2"}' + end + + context 'when the loadbalancing option value is set to null' do + let(:body) do + { + options: { + loadbalancing: nil + } + } + end + + it 'removes this option and hash options' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to eq({}) + end + + it 'notifies the backend' do + expect(fake_route_handler).to receive(:notify_backend_of_route_update) + subject.update(route:, message:) + end + end + + context 'when updating only hash_header' do + let(:body) do + { + options: { + hash_header: 'X-New-Header' + } + } + end + + it 'updates hash_header while keeping other options' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-New-Header', 'hash_balance' => '2.0' }) + end + + it 'notifies the backend' do + expect(fake_route_handler).to receive(:notify_backend_of_route_update) + subject.update(route:, message:) + end + end + + context 'when updating only hash_balance' do + let(:body) do + { + options: { + hash_balance: '3.5' + } + } + end + + it 'updates hash_balance while keeping other options' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'foobar', 'hash_balance' => '3.5' }) + end + + it 'notifies the backend' do + expect(fake_route_handler).to receive(:notify_backend_of_route_update) + subject.update(route:, message:) + end + end + + context 'when updating both hash_header and hash_balance' do + let(:body) do + { + options: { + hash_header: 'X-Updated-Header', + hash_balance: '5.0' + } + } + end + + it 'updates both hash options while keeping loadbalancing' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-Updated-Header', 'hash_balance' => '5.0' }) + end + + it 'notifies the backend' do + expect(fake_route_handler).to receive(:notify_backend_of_route_update) + subject.update(route:, message:) + end + end + + context 'when setting hash_balance to null' do + let(:body) do + { + options: { + hash_balance: nil + } + } + end + + it 'removes hash_balance while keeping other options' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'foobar' }) + expect(route.options).not_to have_key('hash_balance') + end + + it 'notifies the backend' do + expect(fake_route_handler).to receive(:notify_backend_of_route_update) + subject.update(route:, message:) + end + end + + context 'when updating from hash to round-robin' do + let(:body) do + { + options: { + loadbalancing: 'round-robin' + } + } + end + + it 'updates to round-robin and removes hash options' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to eq({ 'loadbalancing' => 'round-robin' }) + expect(route.options).not_to have_key('hash_header') + expect(route.options).not_to have_key('hash_balance') + end + + it 'notifies the backend' do + expect(fake_route_handler).to receive(:notify_backend_of_route_update) + subject.update(route:, message:) + end + end + + context 'when updating from hash to least-connection' do + let(:body) do + { + options: { + loadbalancing: 'least-connection' + } + } + end + + it 'updates to least-connection and removes hash options' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to eq({ 'loadbalancing' => 'least-connection' }) + expect(route.options).not_to have_key('hash_header') + expect(route.options).not_to have_key('hash_balance') + end + + it 'notifies the backend' do + expect(fake_route_handler).to receive(:notify_backend_of_route_update) + subject.update(route:, message:) + end + end + + context 'when setting hash_header to null' do + let(:body) do + { + options: { + hash_header: nil + } + } + end + + it 'raises an error because hash_header is required for hash loadbalancing' do + expect(message).to be_valid + expect do + subject.update(route:, message:) + end.to raise_error(RouteUpdate::Error, 'Hash header must be present when loadbalancing is set to hash.') + end + end + end + + context 'when the route has existing option loadbalancing=round-robin' do before do route[:options] = '{"loadbalancing": "round-robin"}' end @@ -213,6 +397,76 @@ module VCAP::CloudController end end + context 'when hash_based_routing feature flag is enabled' do + before do + VCAP::CloudController::FeatureFlag.make(name: 'hash_based_routing', enabled: true) + end + + context 'when updating to hash loadbalancing without hash_header' do + let(:body) do + { + options: { + loadbalancing: 'hash' + } + } + end + + it 'raises an error' do + expect(message).to be_valid + expect do + subject.update(route:, message:) + end.to raise_error(RouteUpdate::Error, 'Hash header must be present when loadbalancing is set to hash.') + end + end + + context 'when updating to hash loadbalancing with hash_header' do + let(:body) do + { + options: { + loadbalancing: 'hash', + hash_header: 'X-User-ID' + } + } + end + + it 'successfully updates to hash loadbalancing' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-User-ID' }) + end + + it 'notifies the backend' do + expect(fake_route_handler).to receive(:notify_backend_of_route_update) + subject.update(route:, message:) + end + end + + context 'when updating to hash loadbalancing with hash_header and hash_balance' do + let(:body) do + { + options: { + loadbalancing: 'hash', + hash_header: 'X-Session-ID', + hash_balance: '2.5' + } + } + end + + it 'successfully updates to hash loadbalancing with all options' do + expect(message).to be_valid + subject.update(route:, message:) + route.reload + expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'X-Session-ID', 'hash_balance' => '2.5' }) + end + + it 'notifies the backend' do + expect(fake_route_handler).to receive(:notify_backend_of_route_update) + subject.update(route:, message:) + end + end + end + context 'when an option is specified' do let(:body) do { diff --git a/spec/unit/messages/manifest_routes_update_message_spec.rb b/spec/unit/messages/manifest_routes_update_message_spec.rb index f10b4050303..11d719fd686 100644 --- a/spec/unit/messages/manifest_routes_update_message_spec.rb +++ b/spec/unit/messages/manifest_routes_update_message_spec.rb @@ -212,10 +212,10 @@ module VCAP::CloudController context 'when a route contains empty route options' do let(:body) do { 'routes' => - [ - { 'route' => 'existing.example.com', - 'options' => {} } - ] } + [ + { 'route' => 'existing.example.com', + 'options' => {} } + ] } end it 'returns true' do @@ -245,10 +245,10 @@ module VCAP::CloudController context 'when a route contains invalid route options' do let(:body) do { 'routes' => - [ - { 'route' => 'existing.example.com', - 'options' => { 'invalid' => 'invalid' } } - ] } + [ + { 'route' => 'existing.example.com', + 'options' => { 'invalid' => 'invalid' } } + ] } end it 'returns true' do @@ -263,12 +263,12 @@ module VCAP::CloudController context 'when a route contains a valid value for loadbalancing' do let(:body) do { 'routes' => - [ - { 'route' => 'existing.example.com', - 'options' => { - 'loadbalancing' => 'round-robin' - } } - ] } + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'round-robin' + } } + ] } end it 'returns true' do @@ -316,6 +316,532 @@ module VCAP::CloudController expect(msg.errors.full_messages).to include("Cannot use loadbalancing value 'sushi' for Route 'existing.example.com'; Valid values are: 'round-robin, least-connection'") end end + + context 'hash options validation' do + context 'when hash_based_routing feature flag is disabled' do + before do + VCAP::CloudController::FeatureFlag.make(name: 'hash_based_routing', enabled: false) + end + + context 'when a route contains loadbalancing=hash' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash' + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include( + "Cannot use loadbalancing value 'hash' for Route 'existing.example.com'; Valid values are: 'round-robin, least-connection'" + ) + end + end + + context 'when a route contains hash_header' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'hash_header' => 'X-User-ID' + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include( + "Route 'existing.example.com' contains invalid route option 'hash_header'. Valid keys: 'loadbalancing'" + ) + end + end + + context 'when a route contains hash_balance' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'hash_balance' => '1.5' + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com' contains invalid route option 'hash_balance'. Valid keys: 'loadbalancing'") + end + end + + context 'when a route contains hash_balance with round-robin loadbalancing' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'round-robin', + 'hash_balance' => '1.2' + } } + ] } + end + + it 'returns false with only one error (invalid option), and skips hash-based routing specific error messages' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com' contains invalid route option 'hash_balance'. Valid keys: 'loadbalancing'") + expect(msg.errors.full_messages).not_to include("Route 'existing.example.com': Hash balance can only be set when loadbalancing is hash") + expect(msg.errors.full_messages.length).to eq(1) + end + end + + context 'when a route contains both hash_header and hash_balance' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'hash_header' => 'X-User-ID', + 'hash_balance' => '1.5' + } } + ] } + end + + it 'returns false with appropriate error' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + # Should get error for invalid route options (both hash_header and hash_balance are invalid) + expect(msg.errors.full_messages.any? { |m| m.include?('contains invalid route option') }).to be(true) + end + end + end + + context 'when hash_based_routing feature flag is enabled' do + before do + VCAP::CloudController::FeatureFlag.make(name: 'hash_based_routing', enabled: true) + end + + context 'when a route contains hash_header with hash loadbalancing' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID' + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains hash_balance with hash loadbalancing' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID', + 'hash_balance' => '1.5' + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains hash_header without loadbalancing' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'hash_header' => 'X-User-ID' + } } + ] } + end + + it 'returns true (loadbalancing is omitted)' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains hash_header longer than 128 characters' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X' * 129 + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash header must be at most 128 characters") + end + end + + context 'when a route contains hash_header exactly 128 characters' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X' * 128 + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains hash_balance without loadbalancing' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'hash_header' => 'X-User-ID', + 'hash_balance' => '2.0' + } } + ] } + end + + it 'returns true (loadbalancing is omitted)' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains hash_header with non-hash loadbalancing' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'round-robin', + 'hash_header' => 'X-User-ID' + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash header can only be set when loadbalancing is hash") + end + end + + context 'when a route contains hash_balance with non-hash loadbalancing' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'least-connection', + 'hash_balance' => '1.5' + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance can only be set when loadbalancing is hash") + end + end + + context 'when a route contains non-numeric hash_balance' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'hash_balance' => 'not-a-number' + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance must be a numeric value") + end + end + + context 'when a route contains hash_balance of 0' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID', + 'hash_balance' => 0 + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains hash_balance between 0 and 1.1' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID', + 'hash_balance' => 0.5 + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance must be either 0 or between 1.1 and 10.0") + end + end + + context 'when a route contains hash_balance of 1.1' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID', + 'hash_balance' => 1.1 + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains hash_balance greater than 10.0' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID', + 'hash_balance' => 10.1 + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance must be either 0 or between 1.1 and 10.0") + end + end + + context 'when a route contains hash_balance exactly 10.0' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID', + 'hash_balance' => 10.0 + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains numeric string hash_balance' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID', + 'hash_balance' => '2.5' + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains float hash_balance' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID', + 'hash_balance' => 1.5 + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains hash_balance longer than 16 characters' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID', + 'hash_balance' => '2.' + ('1' * 15) + } } + ] } + end + + it 'returns false' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance must be at most 16 characters") + end + end + + context 'when a route contains hash_balance exactly 16 characters' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID', + 'hash_balance' => '2.' + ('1' * 14) + } } + ] } + end + + it 'returns true if the value is numeric and within range' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains multiple issues with hash options' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X' * 129, + 'hash_balance' => '2.' + ('1' * 15) + } } + ] } + end + + it 'returns false and prints all errors' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash header must be at most 128 characters") + expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance must be at most 16 characters") + end + end + + context 'when multiple routes have mixed valid and invalid hash options' do + let(:body) do + { 'routes' => + [ + { 'route' => 'valid1.example.com', + 'options' => { + 'loadbalancing' => 'hash', + 'hash_header' => 'X-User-ID' + } }, + { 'route' => 'invalid.example.com', + 'options' => { + 'loadbalancing' => 'round-robin', + 'hash_header' => 'X-User-ID' + } }, + { 'route' => 'valid2.example.com', + 'options' => { + 'hash_header' => 'X-Session-ID' + } } + ] } + end + + it 'returns false and reports the invalid route' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + expect(msg.errors.full_messages).to include("Route 'invalid.example.com': Hash header can only be set when loadbalancing is hash") + end + end + 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 cbfe96b8b3d..c77b9c6d15e 100644 --- a/spec/unit/messages/route_create_message_spec.rb +++ b/spec/unit/messages/route_create_message_spec.rb @@ -496,6 +496,139 @@ module VCAP::CloudController end end end + + context 'when hash_based_routing feature flag is enabled' do + before do + VCAP::CloudController::FeatureFlag.make(name: 'hash_based_routing', enabled: true) + end + + context 'when hash_header is longer than 128 characters' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { + loadbalancing: 'hash', + hash_header: 'X' * 129 + } + } + end + + it 'is not valid' do + expect(subject).not_to be_valid + expect(subject.errors[:options]).to include('Hash header must be at most 128 characters') + end + end + + context 'when hash_header is exactly 128 characters' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { + loadbalancing: 'hash', + hash_header: 'X' * 128 + } + } + end + + it 'is valid' do + expect(subject).to be_valid + end + end + + context 'when hash_balance is longer than 16 characters' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { + loadbalancing: 'hash', + hash_header: 'X-User-ID', + hash_balance: '12345678901234567' + } + } + end + + it 'is not valid' do + expect(subject).not_to be_valid + expect(subject.errors[:options]).to include('Hash balance must be at most 16 characters') + end + end + + context 'when hash_balance is exactly 16 characters' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { + loadbalancing: 'hash', + hash_header: 'X-User-ID', + hash_balance: '9.9' + } + } + end + + it 'is valid' do + expect(subject).to be_valid + end + end + + context 'when hash_balance is greater than 10.0' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { + loadbalancing: 'hash', + hash_header: 'X-User-ID', + hash_balance: 10.1 + } + } + end + + it 'is not valid' do + expect(subject).not_to be_valid + expect(subject.errors[:options]).to include('Hash balance must be either 0 or between 1.1 and 10.0') + end + end + + context 'when hash_balance is exactly 10.0' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { + loadbalancing: 'hash', + hash_header: 'X-User-ID', + hash_balance: 10.0 + } + } + end + + it 'is valid' do + expect(subject).to be_valid + end + end + end end end diff --git a/spec/unit/messages/route_options_message_spec.rb b/spec/unit/messages/route_options_message_spec.rb new file mode 100644 index 00000000000..57646d21950 --- /dev/null +++ b/spec/unit/messages/route_options_message_spec.rb @@ -0,0 +1,212 @@ +require 'spec_helper' +require 'messages/route_options_message' + +module VCAP::CloudController + RSpec.describe RouteOptionsMessage do + describe 'basic validations' do + it 'successfully validates round-robin load-balancing algorithm' do + message = RouteOptionsMessage.new({ loadbalancing: 'round-robin' }) + expect(message).to be_valid + end + + it 'successfully validates least-connection load-balancing algorithm' do + message = RouteOptionsMessage.new({ loadbalancing: 'least-connection' }) + expect(message).to be_valid + end + + it 'successfully validates empty options' do + message = RouteOptionsMessage.new({}) + expect(message).to be_valid + end + + it 'successfully validates empty load balancer' do + message = RouteOptionsMessage.new({ loadbalancing: nil }) + expect(message).to be_valid + end + + it 'adds invalid load balancer error message' do + message = RouteOptionsMessage.new({ loadbalancing: 'donuts' }) + expect(message).not_to be_valid + expect(message.errors_on(:loadbalancing)).to include("must be one of 'round-robin, least-connection' if present") + end + + it 'adds invalid field error message' do + message = RouteOptionsMessage.new({ cookies: 'round-robin' }) + expect(message).not_to be_valid + expect(message.errors_on(:base)).to include("Unknown field(s): 'cookies'") + end + end + + describe 'hash-based routing validations' do + context 'when hash_based_routing feature flag is disabled' do + it 'does not allow hash_header option' do + message = RouteOptionsMessage.new({ hash_header: 'X-User-ID' }) + expect(message).not_to be_valid + expect(message.errors_on(:base)).to include("Unknown field(s): 'hash_header'") + end + + it 'does not allow hash_balance option' do + message = RouteOptionsMessage.new({ hash_balance: '1.5' }) + expect(message).not_to be_valid + expect(message.errors_on(:base)).to include("Unknown field(s): 'hash_balance'") + end + + it 'reports multiple invalid keys together' do + message = RouteOptionsMessage.new({ hash_header: 'X-User-ID', hash_balance: '1.5' }) + expect(message).not_to be_valid + expect(message.errors_on(:base)).to include("Unknown field(s): 'hash_header', 'hash_balance'") + end + + it 'does not allow hash load-balancing algorithm' do + message = RouteOptionsMessage.new({ loadbalancing: 'hash' }) + expect(message).not_to be_valid + expect(message.errors_on(:loadbalancing)).to include("must be one of 'round-robin, least-connection' if present") + end + end + + context 'when hash_based_routing feature flag is enabled' do + before do + VCAP::CloudController::FeatureFlag.make(name: 'hash_based_routing', enabled: true) + end + + describe 'loadbalancing algorithm' do + it 'allows hash loadbalancing option' do + message = RouteOptionsMessage.new({ loadbalancing: 'hash', hash_header: 'X-User-ID' }) + expect(message).to be_valid + end + + it 'allows round-robin loadbalancing' do + message = RouteOptionsMessage.new({ loadbalancing: 'round-robin' }) + expect(message).to be_valid + end + + it 'allows least-connection loadbalancing' do + message = RouteOptionsMessage.new({ loadbalancing: 'least-connection' }) + expect(message).to be_valid + end + end + + describe 'hash_header validation' do + it 'allows hash_header option' do + message = RouteOptionsMessage.new({ hash_header: 'X-User-ID' }) + expect(message).to be_valid + end + + it 'does not allow hash_header without hash load-balancing' do + message = RouteOptionsMessage.new({ loadbalancing: 'round-robin', hash_header: 'X-User-ID' }) + expect(message).not_to be_valid + expect(message.errors_on(:base)).to include('Hash header can only be set when loadbalancing is hash') + end + + context 'hash_header length validation' do + it 'does not accept hash_header longer than 128 characters' do + message = RouteOptionsMessage.new({ loadbalancing: 'hash', hash_header: 'X' * 129 }) + expect(message).not_to be_valid + expect(message.errors_on(:hash_header)).to include('must be at most 128 characters') + end + + it 'accepts hash_header exactly 128 characters' do + message = RouteOptionsMessage.new({ loadbalancing: 'hash', hash_header: 'X' * 128 }) + expect(message).to be_valid + end + end + end + + describe 'hash_balance validation' do + it 'allows hash_balance option' do + message = RouteOptionsMessage.new({ hash_balance: '1.5' }) + expect(message).to be_valid + end + + it 'does not allow hash_balance without hash load-balancing' do + message = RouteOptionsMessage.new({ loadbalancing: 'round-robin', hash_balance: '1.5' }) + expect(message).not_to be_valid + expect(message.errors_on(:base)).to include('Hash balance can only be set when loadbalancing is hash') + end + + context 'numeric validation' do + it 'does not allow non-numeric hash_balance' do + message = RouteOptionsMessage.new({ hash_balance: 'not-a-number' }) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Hash balance must be a numeric value') + end + + it 'allows hash_balance of 0' do + message = RouteOptionsMessage.new({ hash_balance: 0 }) + expect(message).to be_valid + end + + it 'allows hash_balance of 1.1' do + message = RouteOptionsMessage.new({ hash_balance: 1.1 }) + expect(message).to be_valid + end + + it 'allows hash_balance greater than 1.1' do + message = RouteOptionsMessage.new({ hash_balance: 2.5 }) + expect(message).to be_valid + end + + it 'does not allow hash_balance between 0 and 1.1' do + message = RouteOptionsMessage.new({ hash_balance: 0.5 }) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Hash balance must be either 0 or between 1.1 and 10.0') + end + + it 'allows numeric string hash_balance' do + message = RouteOptionsMessage.new({ hash_balance: '2.5' }) + expect(message).to be_valid + end + + it 'allows integer string hash_balance' do + message = RouteOptionsMessage.new({ hash_balance: '3' }) + expect(message).to be_valid + end + + it 'allows float hash_balance' do + message = RouteOptionsMessage.new({ hash_balance: 1.5 }) + expect(message).to be_valid + end + end + + context 'length validation' do + it 'does not accept hash_balance longer than 16 characters' do + message = RouteOptionsMessage.new({ loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '12345678901234567' }) + expect(message).not_to be_valid + expect(message.errors_on(:hash_balance)).to include('must be at most 16 characters') + end + + it 'accepts hash_balance exactly 16 characters' do + message = RouteOptionsMessage.new({ loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '9.9' }) + expect(message).to be_valid + end + end + + context 'range validation' do + it 'does not accept hash_balance greater than 10.0' do + message = RouteOptionsMessage.new({ loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: 10.1 }) + expect(message).not_to be_valid + expect(message.errors_on(:hash_balance)).to include('must be either 0 or between 1.1 and 10.0') + end + + it 'accepts hash_balance exactly 10.0' do + message = RouteOptionsMessage.new({ loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: 10.0 }) + expect(message).to be_valid + end + end + end + + describe 'combined hash options' do + it 'allows hash loadbalancing with hash_header and hash_balance' do + message = RouteOptionsMessage.new({ loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '2.5' }) + expect(message).to be_valid + end + + it 'allows hash loadbalancing with only hash_header' do + message = RouteOptionsMessage.new({ loadbalancing: 'hash', hash_header: 'X-User-ID' }) + expect(message).to be_valid + end + end + end + end + end +end diff --git a/spec/unit/messages/route_update_message_spec.rb b/spec/unit/messages/route_update_message_spec.rb index 926e8289ead..6c7b1e3f65a 100644 --- a/spec/unit/messages/route_update_message_spec.rb +++ b/spec/unit/messages/route_update_message_spec.rb @@ -61,6 +61,45 @@ module VCAP::CloudController expect(message).not_to be_valid expect(message.errors.full_messages[0]).to include("Options Unknown field(s): 'gorgonzola'") end + + context 'when hash_based_routing feature flag is enabled' do + before do + VCAP::CloudController::FeatureFlag.make(name: 'hash_based_routing', enabled: true) + end + + it 'does not accept hash_header longer than 128 characters' do + message = RouteUpdateMessage.new(params.merge(options: { loadbalancing: 'hash', hash_header: 'X' * 129 })) + expect(message).not_to be_valid + expect(message.errors.full_messages[0]).to include('Hash header must be at most 128 characters') + end + + it 'accepts hash_header exactly 128 characters' do + message = RouteUpdateMessage.new(params.merge(options: { loadbalancing: 'hash', hash_header: 'X' * 128 })) + expect(message).to be_valid + end + + it 'does not accept hash_balance longer than 16 characters' do + message = RouteUpdateMessage.new(params.merge(options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '12345678901234567' })) + expect(message).not_to be_valid + expect(message.errors.full_messages[0]).to include('Hash balance must be at most 16 characters') + end + + it 'accepts hash_balance exactly 16 characters' do + message = RouteUpdateMessage.new(params.merge(options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '9.9' })) + expect(message).to be_valid + end + + it 'does not accept hash_balance greater than 10.0' do + message = RouteUpdateMessage.new(params.merge(options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: 10.1 })) + expect(message).not_to be_valid + expect(message.errors.full_messages[0]).to include('Hash balance must be either 0 or between 1.1 and 10.0') + end + + it 'accepts hash_balance exactly 10.0' do + message = RouteUpdateMessage.new(params.merge(options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: 10.0 })) + expect(message).to be_valid + end + end end end end diff --git a/spec/unit/messages/validators_spec.rb b/spec/unit/messages/validators_spec.rb index a92ac3ced52..03e31652f02 100644 --- a/spec/unit/messages/validators_spec.rb +++ b/spec/unit/messages/validators_spec.rb @@ -529,62 +529,6 @@ 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-connection load-balancing algorithm' do - message = OptionsMessage.new({ options: { loadbalancing: 'least-connection' } }) - 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-connection' 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 8e04a79e87b..c6554939adc 100644 --- a/spec/unit/models/runtime/route_spec.rb +++ b/spec/unit/models/runtime/route_spec.rb @@ -1055,6 +1055,490 @@ module VCAP::CloudController it { is_expected.to import_attributes :host, :domain_guid, :space_guid, :app_guids, :path, :port, :options } end + describe 'options normalization' do + let(:space) { Space.make } + let(:domain) { PrivateDomain.make(owning_organization: space.organization) } + + context 'when hash_balance is provided as a float' do + it 'stores hash_balance as a string in the database' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: 1.5 } + ) + + route.reload + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options['hash_balance']).to be_a(String) + expect(parsed_options['hash_balance']).to eq('1.5') + end + end + + context 'when hash_balance is provided as an integer' do + it 'stores hash_balance as a string in the database' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: 2 } + ) + + route.reload + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options['hash_balance']).to be_a(String) + expect(parsed_options['hash_balance']).to eq('2.0') + end + end + + context 'when hash_balance is provided as a string' do + it 'keeps hash_balance as a string in the database' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '1.25' } + ) + + route.reload + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options['hash_balance']).to be_a(String) + expect(parsed_options['hash_balance']).to eq('1.3') + end + end + + context 'when hash_balance is 0' do + it 'stores hash_balance as a string "0" in the database' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: 0 } + ) + + route.reload + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options['hash_balance']).to be_a(String) + expect(parsed_options['hash_balance']).to eq('0.0') + end + end + + context 'when options do not include hash_balance' do + it 'does not add hash_balance to the options' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'round-robin' } + ) + + route.reload + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options).not_to have_key('hash_balance') + end + end + + context 'when options are nil' do + it 'handles nil options gracefully' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: nil + ) + + route.reload + expect(route.options).to be_nil + end + end + end + + describe 'normalize_hash_balance_to_string' do + let(:space) { Space.make } + let(:domain) { PrivateDomain.make(owning_organization: space.organization) } + let(:route) { Route.new(host: 'test', domain: domain, space: space) } + + context 'when hash_balance is provided as a float' do + it 'converts it to a string' do + result = route.send(:normalize_hash_balance_to_string, { hash_balance: 1.5 }) + expect(result[:hash_balance]).to eq('1.5') + expect(result[:hash_balance]).to be_a(String) + end + end + + context 'when hash_balance is provided as an integer' do + it 'converts it to a string' do + result = route.send(:normalize_hash_balance_to_string, { hash_balance: 2 }) + expect(result[:hash_balance]).to eq('2') + expect(result[:hash_balance]).to be_a(String) + end + end + + context 'when hash_balance is provided as 0' do + it 'converts it to string "0"' do + result = route.send(:normalize_hash_balance_to_string, { hash_balance: 0 }) + expect(result[:hash_balance]).to eq('0') + expect(result[:hash_balance]).to be_a(String) + end + end + + context 'when hash_balance is already a string' do + it 'keeps it as a string' do + result = route.send(:normalize_hash_balance_to_string, { hash_balance: '1.25' }) + expect(result[:hash_balance]).to eq('1.25') + expect(result[:hash_balance]).to be_a(String) + end + end + + context 'when hash_balance is provided with string key' do + it 'converts it to a string with symbol key' do + result = route.send(:normalize_hash_balance_to_string, { 'hash_balance' => 2.5 }) + expect(result[:hash_balance]).to eq('2.5') + expect(result[:hash_balance]).to be_a(String) + end + end + + context 'when hash_balance is not present' do + it 'returns the hash unchanged' do + result = route.send(:normalize_hash_balance_to_string, { loadbalancing: 'hash', hash_header: 'X-User-ID' }) + expect(result[:loadbalancing]).to eq('hash') + expect(result[:hash_header]).to eq('X-User-ID') + expect(result).not_to have_key(:hash_balance) + end + end + + context 'when hash_balance is nil' do + it 'does not convert nil to string' do + result = route.send(:normalize_hash_balance_to_string, { hash_balance: nil }) + expect(result[:hash_balance]).to be_nil + end + end + + context 'when hash_balance is an empty string' do + it 'does not convert empty string' do + result = route.send(:normalize_hash_balance_to_string, { hash_balance: '' }) + expect(result[:hash_balance]).to eq('') + end + end + + context 'when options is not a hash' do + it 'returns the input unchanged' do + result = route.send(:normalize_hash_balance_to_string, nil) + expect(result).to be_nil + end + end + + context 'when options is an empty hash' do + it 'returns an empty hash' do + result = route.send(:normalize_hash_balance_to_string, {}) + expect(result).to eq({}) + end + end + + context 'with complete options hash' do + it 'normalizes hash_balance while preserving other options' do + result = route.send(:normalize_hash_balance_to_string, { + loadbalancing: 'hash', + hash_header: 'X-User-ID', + hash_balance: 3.14159 + }) + expect(result[:loadbalancing]).to eq('hash') + expect(result[:hash_header]).to eq('X-User-ID') + expect(result[:hash_balance]).to eq('3.14159') + expect(result[:hash_balance]).to be_a(String) + end + end + end + + describe 'hash options cleanup for non-hash loadbalancing' do + let(:space) { Space.make } + let(:domain) { PrivateDomain.make(owning_organization: space.organization) } + + context 'when creating a route with hash loadbalancing' do + it 'keeps hash_header and hash_balance' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '1.5' } + ) + + route.reload + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options['loadbalancing']).to eq('hash') + expect(parsed_options['hash_header']).to eq('X-User-ID') + expect(parsed_options['hash_balance']).to eq('1.5') + end + end + + context 'when updating a route from hash to round-robin' do + it 'removes hash_header and hash_balance' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '1.5' } + ) + + route.update(options: { loadbalancing: 'round-robin' }) + route.reload + + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options['loadbalancing']).to eq('round-robin') + expect(parsed_options).not_to have_key('hash_header') + expect(parsed_options).not_to have_key('hash_balance') + end + end + + context 'when updating a route from hash to least-connection' do + it 'removes hash_header and hash_balance' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: 'X-Request-ID', hash_balance: '2.5' } + ) + + route.update(options: { loadbalancing: 'least-connection' }) + route.reload + + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options['loadbalancing']).to eq('least-connection') + expect(parsed_options).not_to have_key('hash_header') + expect(parsed_options).not_to have_key('hash_balance') + end + end + + context 'when updating a route from round-robin to hash' do + it 'keeps hash_header and hash_balance if provided' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'round-robin' } + ) + + route.update(options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '1.5' }) + route.reload + + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options['loadbalancing']).to eq('hash') + expect(parsed_options['hash_header']).to eq('X-User-ID') + expect(parsed_options['hash_balance']).to eq('1.5') + end + end + + context 'when removing hash loadbalancing option' do + it 'deletes hash_header and hash_balance when present' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '1.5' } + ) + + route.update(options: { loadbalancing: nil }) + route.reload + + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options).not_to have_key('hash_header') + expect(parsed_options).not_to have_key('hash_balance') + end + end + + context 'when using string keys instead of symbols' do + it 'still removes hash options for non-hash loadbalancing' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { 'loadbalancing' => 'round-robin', 'hash_header' => 'X-User-ID', 'hash_balance' => '1.5' } + ) + + route.reload + parsed_options = Oj.load(route.options_without_serialization) + expect(parsed_options['loadbalancing']).to eq('round-robin') + expect(parsed_options).not_to have_key('hash_header') + expect(parsed_options).not_to have_key('hash_balance') + end + end + end + + describe 'route options validation' do + let(:space) { Space.make } + let(:domain) { PrivateDomain.make(owning_organization: space.organization) } + + context 'when loadbalancing is hash' do + context 'and hash_header is present' do + it 'is valid' do + route = Route.new( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: 'X-User-ID' } + ) + + expect(route).to be_valid + end + end + + context 'and hash_header is missing' do + it 'is invalid and adds an error' do + route = Route.new( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash' } + ) + + expect(route).not_to be_valid + expect(route.errors[:route]).to include :hash_header_missing + end + end + + context 'and hash_header is blank string' do + it 'is invalid and adds an error' do + route = Route.new( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: '' } + ) + + expect(route).not_to be_valid + expect(route.errors[:route]).to include :hash_header_missing + end + end + + context 'and hash_header and hash_balance are both present' do + it 'is valid' do + route = Route.new( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'hash', hash_header: 'X-User-ID', hash_balance: '1.5' } + ) + + expect(route).to be_valid + end + end + end + + context 'when loadbalancing is round-robin' do + it 'is valid' do + route = Route.new( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'round-robin' } + ) + + expect(route).to be_valid + end + end + + context 'when loadbalancing is least-connection' do + it 'is valid' do + route = Route.new( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'least-connection' } + ) + + expect(route).to be_valid + end + end + + context 'when options are nil' do + it 'is valid' do + route = Route.new( + host: 'test-route', + domain: domain, + space: space, + options: nil + ) + + expect(route).to be_valid + end + end + + context 'when options are empty hash' do + it 'is valid' do + route = Route.new( + host: 'test-route', + domain: domain, + space: space, + options: {} + ) + + expect(route).to be_valid + end + end + + context 'when updating an existing route' do + context 'changing to hash loadbalancing without hash_header' do + it 'is invalid' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'round-robin' } + ) + + route.options = { loadbalancing: 'hash' } + + expect(route).not_to be_valid + expect(route.errors[:route]).to include :hash_header_missing + end + end + + context 'changing to hash loadbalancing with hash_header' do + it 'is valid' do + route = Route.make( + host: 'test-route', + domain: domain, + space: space, + options: { loadbalancing: 'round-robin' } + ) + + route.options = { loadbalancing: 'hash', hash_header: 'X-Request-ID' } + + expect(route).to be_valid + end + end + end + + context 'when options use string keys instead of symbols' do + context 'and hash_header is present' do + it 'is valid' do + route = Route.new( + host: 'test-route', + domain: domain, + space: space, + options: { 'loadbalancing' => 'hash', 'hash_header' => 'X-User-ID' } + ) + + expect(route).to be_valid + end + end + + context 'and hash_header is missing' do + it 'is invalid and adds an error' do + route = Route.new( + host: 'test-route', + domain: domain, + space: space, + options: { 'loadbalancing' => 'hash' } + ) + + expect(route).not_to be_valid + expect(route.errors[:route]).to include :hash_header_missing + end + end + end + end + describe 'instance methods' do let(:space) { Space.make }