Adjust behaviour to new decisions:

options default: {}
API:
 options is not nullable
 specific option is additive
 specific option is nullable
 empty hash does not change anything (additive)
 get empty options -> {}
manifest:
 options and specific option is nullable, but no-op

Author: a18e

app/actions/manifest_route_update.rb

@@ -92,16 +92,15 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info)
               domain: existing_domain,
               manifest_triggered: true
             )
+          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:)
-
-          elsif route.space.guid != app.space_guid
-            raise InvalidRoute.new('Routes cannot be mapped to destinations in different spaces')
           end
 
           return route

app/actions/route_create.rb

@@ -16,7 +16,7 @@ def create(message:, space:, domain:, manifest_triggered: false)
         port: port(message, domain),
         space: space,
         domain: domain,
-        options: message.options
+        options: message.options.nil? ? {} : message.options.compact
       )
 
       Route.db.transaction do

app/actions/route_update.rb

@@ -4,11 +4,9 @@ def update(route:, message:)
       Route.db.transaction do
         if message.requested?(:options)
           route.options = if message.options.nil?
-                            nil
-                          elsif route.options.nil?
-                            message.options
+                            {}
                           else
-                            route.options.merge(message.options)
+                            route.options.symbolize_keys.merge(message.options).compact
                           end
         end
 

app/messages/manifest_routes_update_message.rb

@@ -13,14 +13,8 @@ def validate(record)
           return
         end
 
-        if contains_invalid_route_options?(record.routes)
-          record.errors.add(:routes, message: 'contains invalid route options')
-          return
-        end
-
-        return unless contains_invalid_lb_algo?(record.routes)
-
-        record.errors.add(:routes, message: 'contains an invalid loadbalancing-algorithm option')
+        contains_invalid_route_options?(record)
+        contains_invalid_lb_algo?(record)
         nil
       end
 
@@ -32,23 +26,34 @@ def contains_non_route_hash_values?(routes)
         routes.any? { |r| !(r.is_a?(Hash) && r[:route].present?) }
       end
 
-      def contains_invalid_route_options?(routes)
+      def contains_invalid_route_options?(record)
+        routes = record.routes
         routes.any? do |r|
           next unless r[:options]
 
           return true unless r[:options].is_a?(Hash)
 
           return false if r[:options].empty?
 
-          return r[:options].keys.all? { |key| RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.exclude?(key) }
+          r[:options].each_key do |key|
+            RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.exclude?(key) &&
+              record.errors.add(:base,
+                                message: "Route '#{r[:route]}' contains invalid route option '#{key}'. \
+Valid keys: '#{RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.join(', ')}'")
+          end
         end
       end
 
-      def contains_invalid_lb_algo?(routes)
-        routes.any? do |r|
+      def contains_invalid_lb_algo?(record)
+        routes = record.routes
+        routes.each do |r|
           next unless r[:options] && r[:options][:'loadbalancing-algorithm']
 
-          return true if r[:options][:'loadbalancing-algorithm'] && RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(r[:options][:'loadbalancing-algorithm'])
+          lb_algo = r[:options][:'loadbalancing-algorithm']
+          RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(lb_algo) &&
+            record.errors.add(:base,
+                              message: "Route '#{r[:route]}' contains invalid load-balancing algorithm '#{lb_algo}'. \
+Valid algorithms: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'")
         end
       end
     end

app/messages/route_create_message.rb

@@ -14,6 +14,10 @@ class RouteCreateMessage < MetadataBaseMessage
       options
     ]
 
+    def self.options_requested?
+      @options_requested ||= proc { |a| a.requested?(:options) }
+    end
+
     validates :host,
               allow_nil: true,
               string: true,
@@ -58,7 +62,7 @@ class RouteCreateMessage < MetadataBaseMessage
 
     validates_with NoAdditionalKeysValidator
     validates_with RelationshipValidator
-    validates_with OptionsValidator
+    validates_with OptionsValidator, if: options_requested?
 
     delegate :space_guid, to: :relationships_message
     delegate :domain_guid, to: :relationships_message

app/messages/route_options_message.rb

@@ -6,7 +6,7 @@ class RouteOptionsMessage < BaseMessage
     VALID_ROUTE_OPTIONS = %i[lb_algo].freeze
     VALID_LOADBALANCING_ALGORITHMS = %w[round-robin least-connections].freeze
 
-    register_allowed_keys %i[lb_algo]
+    register_allowed_keys VALID_ROUTE_OPTIONS
     validates_with NoAdditionalKeysValidator
     validates :lb_algo,
               inclusion: { in: VALID_LOADBALANCING_ALGORITHMS, message: "'%<value>s' is not a supported load-balancing algorithm" },

app/messages/validators.rb

@@ -242,9 +242,6 @@ def validate(record)
 
   class OptionsValidator < ActiveModel::Validator
     def validate(record)
-      # Empty option hashes are allowed, so we skip further validation
-      record.options.blank? && return
-
       unless record.options.is_a?(Hash)
         record.errors.add(:options, message: "'options' is not a valid object")
         return

db/migrations/20241105000000_add_options_to_route.rb

@@ -1,9 +1,7 @@
 Sequel.migration do
   up do
     alter_table(:routes) do
-      # rubocop:disable Migration/IncludeStringSize
-      add_column :options, String, text: true, default: nil
-      # rubocop:enable Migration/IncludeStringSize
+      add_column :options, String, size: 4096, default: '{}'
     end
   end
   down do

lib/cloud_controller/app_manifest/manifest_route.rb

@@ -16,18 +16,16 @@ def self.parse(route, options=nil)
               end
 
       attrs[:full_route] = route
-
-      if options
-        attrs[:options] = {}
-        attrs[:options][:lb_algo] = options[:'loadbalancing-algorithm'] if options.key?(:'loadbalancing-algorithm')
-      end
+      attrs[:options] = {}
+      attrs[:options][:lb_algo] = options[:'loadbalancing-algorithm'] if options && options.key?(:'loadbalancing-algorithm')
 
       ManifestRoute.new(attrs)
     end
 
     def valid?
       if @attrs[:options] && !@attrs[:options].empty?
         return false if @attrs[:options].keys.any? { |key| RouteOptionsMessage::VALID_ROUTE_OPTIONS.exclude?(key) }
+        # validation for loadbalancing algorithm
         return false if @attrs[:options][:lb_algo] && RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(@attrs[:options][:lb_algo])
       end
 

spec/request/app_manifests_spec.rb

@@ -97,11 +97,13 @@
               'routes' => [
                 {
                   'route' => "#{route.host}.#{route.domain.name}",
-                  'protocol' => 'http1'
+                  'protocol' => 'http1',
+                  'options' => {}
                 },
                 {
                   'route' => "#{second_route.host}.#{second_route.domain.name}/path",
-                  'protocol' => 'http1'
+                  'protocol' => 'http1',
+                  'options' => {}
                 }
               ],
               'metadata' => { 'labels' => { 'potato' => 'idaho' }, 'annotations' => { 'style' => 'mashed' } },
@@ -201,11 +203,13 @@
               'routes' => [
                 {
                   'route' => "#{route.host}.#{route.domain.name}",
-                  'protocol' => 'http1'
+                  'protocol' => 'http1',
+                  'options' => {}
                 },
                 {
                   'route' => "#{second_route.host}.#{second_route.domain.name}/path",
-                  'protocol' => 'http1'
+                  'protocol' => 'http1',
+                  'options' => {}
                 }
               ],
               'metadata' => { 'labels' => { 'potato' => 'idaho' }, 'annotations' => { 'style' => 'mashed' } },
@@ -273,11 +277,13 @@
               'routes' => [
                 {
                   'route' => "#{route.host}.#{route.domain.name}",
-                  'protocol' => 'http2'
+                  'protocol' => 'http2',
+                  'options' => {}
                 },
                 {
                   'route' => "#{second_route.host}.#{second_route.domain.name}/path",
-                  'protocol' => 'http1'
+                  'protocol' => 'http1',
+                  'options' => {}
                 }
               ]
             }