Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Implement per-route options in Cloud Controller #4080

Merged
merged 7 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion app/actions/manifest_route_update.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'cloud_controller/app_manifest/manifest_route'
require 'actions/route_create'
require 'actions/route_update'

module VCAP::CloudController
class ManifestRouteUpdate
Expand Down Expand Up @@ -81,7 +82,8 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info)
message = RouteCreateMessage.new({
'host' => host,
'path' => manifest_route[:path],
'port' => manifest_route[:port]
'port' => manifest_route[:port],
'options' => manifest_route[:options]
})

route = RouteCreate.new(user_audit_info).create(
Expand All @@ -92,6 +94,13 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info)
)
elsif route.space.guid != app.space_guid
raise InvalidRoute.new('Routes cannot be mapped to destinations in different spaces')
elsif manifest_route[:options] && route[:options] != manifest_route[:options]
# remove nil values from options
manifest_route[:options] = manifest_route[:options].compact
message = RouteUpdateMessage.new({
'options' => manifest_route[:options]
})
route = RouteUpdate.new.update(route:, message:)
end

return route
Expand Down
3 changes: 2 additions & 1 deletion app/actions/route_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ def create(message:, space:, domain:, manifest_triggered: false)
path: message.path || '',
port: port(message, domain),
space: space,
domain: domain
domain: domain,
options: message.options.nil? ? {} : message.options.compact
)

Route.db.transaction do
Expand Down
3 changes: 3 additions & 0 deletions app/actions/route_update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ module VCAP::CloudController
class RouteUpdate
def update(route:, message:)
Route.db.transaction do
route.options = route.options.symbolize_keys.merge(message.options).compact if message.requested?(:options)

route.save
MetadataUpdate.update(route, message)
end

Expand Down
49 changes: 47 additions & 2 deletions app/messages/manifest_routes_update_message.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'messages/base_message'
require 'messages/route_options_message'
require 'cloud_controller/app_manifest/manifest_route'

module VCAP::CloudController
Expand All @@ -25,22 +26,66 @@ def contains_non_route_hash_values?(routes)
validates_with ManifestRoutesYAMLValidator, if: proc { |record| record.requested?(:routes) }
validate :routes_are_uris, if: proc { |record| record.requested?(:routes) }
validate :route_protocols_are_valid, if: proc { |record| record.requested?(:routes) }
validate :route_options_are_valid, if: proc { |record| record.requested?(:routes) }
validate :loadbalancings_are_valid, if: proc { |record| record.requested?(:routes) }
validate :no_route_is_boolean
validate :default_route_is_boolean
validate :random_route_is_boolean
validate :random_route_and_default_route_conflict

def manifest_route_mappings
@manifest_route_mappings ||= routes.map do |route|
{
route: ManifestRoute.parse(route[:route]),
r = {
route: ManifestRoute.parse(route[:route], route[:options]),
protocol: route[:protocol]
}
r[:options] = route[:options] unless route[:options].nil?
r
end
end

private

def route_options_are_valid
return if errors[:routes].present?

routes.any? do |r|
next unless r.keys.include?(:options)

unless r[:options].is_a?(Hash)
errors.add(:base, message: "Route '#{r[:route]}': options must be an object")
next
end

r[:options].each_key do |key|
RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.exclude?(key) &&
errors.add(:base,
message: "Route '#{r[:route]}' contains invalid route option '#{key}'. \
Valid keys: '#{RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.join(', ')}'")
end
end
end

def loadbalancings_are_valid
return if errors[:routes].present?

routes.each do |r|
next unless r.keys.include?(:options) && r[:options].is_a?(Hash) && r[:options].keys.include?(:loadbalancing)

loadbalancing = r[:options][:loadbalancing]
unless loadbalancing.is_a?(String)
errors.add(:base,
message: "Invalid value for 'loadbalancing' for Route '#{r[:route]}'; \
Valid values are: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'")
next
end
RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(loadbalancing) &&
errors.add(:base,
message: "Cannot use loadbalancing value '#{loadbalancing}' for Route '#{r[:route]}'; \
Valid values are: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'")
end
end

def routes_are_uris
return if errors[:routes].present?

Expand Down
11 changes: 11 additions & 0 deletions app/messages/route_create_message.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'messages/metadata_base_message'
require 'messages/route_options_message'

module VCAP::CloudController
class RouteCreateMessage < MetadataBaseMessage
Expand All @@ -10,8 +11,13 @@ class RouteCreateMessage < MetadataBaseMessage
path
port
relationships
options
]

def self.options_requested?
@options_requested ||= proc { |a| a.requested?(:options) }
end

validates :host,
allow_nil: true,
string: true,
Expand Down Expand Up @@ -56,6 +62,7 @@ class RouteCreateMessage < MetadataBaseMessage

validates_with NoAdditionalKeysValidator
validates_with RelationshipValidator
validates_with OptionsValidator, if: options_requested?

delegate :space_guid, to: :relationships_message
delegate :domain_guid, to: :relationships_message
Expand All @@ -65,6 +72,10 @@ def relationships_message
@relationships_message ||= Relationships.new(relationships&.deep_symbolize_keys)
end

def options_message
@options_message ||= RouteOptionsMessage.new(options&.deep_symbolize_keys)
end

def wildcard?
host == '*'
end
Expand Down
16 changes: 16 additions & 0 deletions app/messages/route_options_message.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
require 'messages/metadata_base_message'

module VCAP::CloudController
class RouteOptionsMessage < BaseMessage
VALID_MANIFEST_ROUTE_OPTIONS = %i[loadbalancing].freeze
VALID_ROUTE_OPTIONS = %i[loadbalancing].freeze
VALID_LOADBALANCING_ALGORITHMS = %w[round-robin least-connections].freeze

register_allowed_keys VALID_ROUTE_OPTIONS
validates_with NoAdditionalKeysValidator
validates :loadbalancing,
inclusion: { in: VALID_LOADBALANCING_ALGORITHMS, message: "must be one of '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}' if present" },
presence: true,
allow_nil: true
end
end
13 changes: 12 additions & 1 deletion app/messages/route_update_message.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
require 'messages/metadata_base_message'
require 'messages/route_options_message'

module VCAP::CloudController
class RouteUpdateMessage < MetadataBaseMessage
register_allowed_keys []
register_allowed_keys %i[options]

def self.options_requested?
@options_requested ||= proc { |a| a.requested?(:options) }
end

def options_message
@options_message ||= RouteOptionsMessage.new(options&.deep_symbolize_keys)
end

validates_with OptionsValidator, if: options_requested?

validates_with NoAdditionalKeysValidator
end
Expand Down
17 changes: 17 additions & 0 deletions app/messages/validators.rb
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,23 @@ def validate(record)
end
end

class OptionsValidator < ActiveModel::Validator
def validate(record)
unless record.options.is_a?(Hash)
record.errors.add(:options, message: "'options' is not a valid object")
return
end

opt = record.options_message

return if opt.valid?

opt.errors.full_messages.each do |message|
record.errors.add(:options, message:)
end
end
end

class ToOneRelationshipValidator < ActiveModel::EachValidator
def validate_each(record, attribute, relationship)
if has_correct_structure?(relationship)
Expand Down
21 changes: 19 additions & 2 deletions app/models/runtime/route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ class OutOfVIPException < CloudController::Errors::InvalidRelation; end

add_association_dependencies route_mappings: :destroy

export_attributes :host, :path, :domain_guid, :space_guid, :service_instance_guid, :port
import_attributes :host, :path, :domain_guid, :space_guid, :app_guids, :port
export_attributes :host, :path, :domain_guid, :space_guid, :service_instance_guid, :port, :options
import_attributes :host, :path, :domain_guid, :space_guid, :app_guids, :port, :options

add_association_dependencies labels: :destroy
add_association_dependencies annotations: :destroy
Expand Down Expand Up @@ -71,6 +71,23 @@ def as_summary_json
}
end

def options_with_serialization=(opts)
self.options_without_serialization = Oj.dump(opts)
end

alias_method :options_without_serialization=, :options=
alias_method :options=, :options_with_serialization=

def options_with_serialization
string = options_without_serialization
return nil if string.blank?

Oj.load(string)
end

alias_method :options_without_serialization, :options
alias_method :options, :options_with_serialization

alias_method :old_path, :path
def path
old_path.nil? ? '' : old_path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,17 @@ module AppManifestPresenters
class RoutePropertiesPresenter
def to_hash(route_mappings:, app:, **_)
route_hashes = route_mappings.map do |route_mapping|
{
route_hash = {
route: route_mapping.route.uri,
protocol: route_mapping.protocol
}

if route_mapping.route.options
route_hash[:options] = {}
route_hash[:options][:loadbalancing] = route_mapping.route.options[:loadbalancing] if route_mapping.route.options[:loadbalancing]
end

route_hash
end

{ routes: alphabetize(route_hashes).presence }
Expand Down
1 change: 1 addition & 0 deletions app/presenters/v3/route_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def to_hash
},
links: build_links
}
hash.merge!(options: route.options) unless route.options.nil?

@decorators.reduce(hash) { |memo, d| d.decorate(memo, [route]) }
end
Expand Down
12 changes: 12 additions & 0 deletions db/migrations/20241105000000_add_options_to_route.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Sequel.migration do
up do
alter_table(:routes) do
add_column :options, String, size: 4096, default: '{}'
end
end
down do
alter_table(:routes) do
drop_column :options
end
end
end
12 changes: 12 additions & 0 deletions docs/v3/source/includes/api_resources/_routes.erb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@
"protocol": "tcp"
}
],
"options": {
"loadbalancing": "round-robin"
},
"metadata": {
"labels": <%= metadata.fetch(:labels, {}).to_json(space: ' ', object_nl: ' ')%>,
"annotations": <%= metadata.fetch(:annotations, {}).to_json(space: ' ', object_nl: ' ')%>
Expand Down Expand Up @@ -124,6 +127,9 @@
"protocol": "http1"
}
],
"options": {
"loadbalancing": "round-robin"
},
"metadata": {
"labels": {},
"annotations": {}
Expand Down Expand Up @@ -212,3 +218,9 @@
"protocol": "http2"
}
<% end %>

<% content_for :route_options do %>
{
"loadbalancing": "round-robin"
}
<% end %>
26 changes: 15 additions & 11 deletions docs/v3/source/includes/resources/routes/_create.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ curl "https://api.example.org/v3/routes" \
"data": { "guid": "space-guid" }
}
},
"options": {
"loadbalancing": "round-robin"
}
"metadata": {
"labels": { "key": "value" },
"annotations": { "note": "detailed information"}
Expand All @@ -44,20 +47,21 @@ Content-Type: application/json

#### Required parameters

| Name | Type | Description |
| ----------------------------------------- | ---------------------------------------------- | ---------------------------------------------------------------------------------------------------------- |
| **relationships.space** | [_to-one relationship_](#to-one-relationships) | A relationship to the space containing the route; routes can only be mapped to destinations in that space |
| **relationships.domain** | [_to-one relationship_](#to-one-relationships) | A relationship to the domain of the route |
| Name | Type | Description |
|--------------------------|------------------------------------------------|-----------------------------------------------------------------------------------------------------------|
| **relationships.space** | [_to-one relationship_](#to-one-relationships) | A relationship to the space containing the route; routes can only be mapped to destinations in that space |
| **relationships.domain** | [_to-one relationship_](#to-one-relationships) | A relationship to the domain of the route |

#### Optional parameters

| Name | Type | Description |
| ------------------------ | ----------------------------------- | --------------------------------- |
| **host** | _string_ | The host component for the route; not compatible with routes specifying the `tcp` protocol |
| **path** | _string_ | The path component for the route; should begin with a `/` and not compatible with routes specifying the `tcp` protocol |
| **port** | _integer_ | The port the route will listen on; only compatible with routes leveraging a domain that supports the `tcp` protocol. For `tcp` domains, a port will be randomly assigned if not specified
| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the route |
| **metadata.labels** | [_label object_](#labels) | Labels applied to the route |
| Name | Type | Description |
|--------------------------|----------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| **host** | _string_ | The host component for the route; not compatible with routes specifying the `tcp` protocol |
| **path** | _string_ | The path component for the route; should begin with a `/` and not compatible with routes specifying the `tcp` protocol |
| **port** | _integer_ | The port the route will listen on; only compatible with routes leveraging a domain that supports the `tcp` protocol. For `tcp` domains, a port will be randomly assigned if not specified |
| **options** | [_route option object_](#the-route-options-object) | Options applied to the route |
| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the route |
| **metadata.labels** | [_label object_](#labels) | Labels applied to the route |

#### Permitted roles

Expand Down
Loading
Loading