-
Notifications
You must be signed in to change notification settings - Fork 368
Support Hash-Based Routing #4746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
2c8e854
9d751e4
a8e3b88
1e63c79
0371984
afd8569
41fb78a
16e66df
a7a8862
42b01f3
dfde6ec
0710ef1
6676cf0
bc90e06
3edb661
9bf2395
9d60a3e
1a4b35b
0dddc1d
7b0f56c
981e130
308173e
c93cbaa
ed35a67
e3cffab
7379cff
9eec93c
4eb3b36
69a3ffe
b3f631f
cde7879
3e414e9
7373fe2
335b910
0c7c747
e293c12
9589e16
d39f0a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
| check_domain_errors!(error, space, domain) | ||
| check_quota_errors!(error, space) | ||
| check_route_errors!(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 check_domain_errors!(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 check_quota_errors!(error, space) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be consistent with the other methods, this should be |
||
| 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 check_route_errors!(error) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be consistent with the other methods, this should be |
||
| 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') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All other error messages have a |
||
| end | ||
|
|
||
| def validation_error_routing_api!(error) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also a |
||
|
|
||
| # Fallback for any other validation errors | ||
| raise Error.new(error.message) | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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,84 @@ 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] | ||||||
|
|
||||||
| return if validate_route_hash_header(route, hash_header) | ||||||
| return if 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 false 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") | ||||||
| true | ||||||
| end | ||||||
|
|
||||||
| def validate_route_hash_balance(route, hash_balance) | ||||||
| return false 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 true | ||||||
| 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 to 1.1 and 10.0") unless balance_float == 0 || balance_float.between?(1.1, 10) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| false | ||||||
| rescue ArgumentError, TypeError | ||||||
| errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be a numeric value") | ||||||
| false | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be |
||||||
| 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") | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add a |
||||||
| 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? | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| 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 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be consistent with the other methods, this should be
validation_error_domain!.