diff --git a/Gemfile b/Gemfile index 22f3852df78..0e42695c52e 100644 --- a/Gemfile +++ b/Gemfile @@ -36,7 +36,7 @@ gem 'rfc822' gem 'rubyzip', '>= 1.3.0' gem 'sequel', '~> 5.53' gem 'sequel_pg', require: 'sequel' -gem 'sinatra', '~> 2.1' +gem 'sinatra', '~> 2.2' gem 'sinatra-contrib' gem 'statsd-ruby', '~> 1.4.0' gem 'steno' diff --git a/Gemfile.lock b/Gemfile.lock index 5992f8ef933..1d6ec48270f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -313,7 +313,7 @@ GEM mime-types-data (~> 3.2015) mime-types-data (3.2022.0105) mini_mime (1.1.2) - mini_portile2 (2.7.1) + mini_portile2 (2.8.0) minitest (5.15.0) ms_rest (0.6.4) concurrent-ruby (~> 1.0) @@ -336,12 +336,12 @@ GEM netaddr (2.0.5) netrc (0.11.0) newrelic_rpm (8.4.0) - nokogiri (1.13.1) - mini_portile2 (~> 2.7.0) + nokogiri (1.13.3) + mini_portile2 (~> 2.8.0) racc (~> 1.4) - nokogiri (1.13.1-x86_64-darwin) + nokogiri (1.13.3-x86_64-darwin) racc (~> 1.4) - nokogiri (1.13.1-x86_64-linux) + nokogiri (1.13.3-x86_64-linux) racc (~> 1.4) oj (3.13.11) os (1.1.4) @@ -367,7 +367,7 @@ GEM public_suffix (4.0.6) racc (1.6.0) rack (2.2.3) - rack-protection (2.1.0) + rack-protection (2.2.0) rack rack-test (1.1.0) rack (>= 1.0, < 3) @@ -473,16 +473,16 @@ GEM json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.2) - sinatra (2.1.0) + sinatra (2.2.0) mustermann (~> 1.0) rack (~> 2.2) - rack-protection (= 2.1.0) + rack-protection (= 2.2.0) tilt (~> 2.0) - sinatra-contrib (2.1.0) + sinatra-contrib (2.2.0) multi_json mustermann (~> 1.0) - rack-protection (= 2.1.0) - sinatra (= 2.1.0) + rack-protection (= 2.2.0) + sinatra (= 2.2.0) tilt (~> 2.0) solargraph (0.44.3) backport (~> 1.2) @@ -621,7 +621,7 @@ DEPENDENCIES rubyzip (>= 1.3.0) sequel (~> 5.53) sequel_pg - sinatra (~> 2.1) + sinatra (~> 2.2) sinatra-contrib solargraph spork! diff --git a/app/actions/route_destination_update.rb b/app/actions/route_destination_update.rb new file mode 100644 index 00000000000..ac308cbe64d --- /dev/null +++ b/app/actions/route_destination_update.rb @@ -0,0 +1,32 @@ +module VCAP::CloudController + class RouteDestinationUpdate + class Error < StandardError + end + + class << self + def update(destination, message) + validate_protocol_matches_route!(destination, message) + + destination.db.transaction do + destination.lock! + + destination.protocol = message.protocol if message.requested? :protocol + + destination.save + end + end + + private + + def validate_protocol_matches_route!(destination, message) + if destination.route&.protocol == 'tcp' + unless message.protocol == 'tcp' + raise Error.new("Destination protocol must be 'tcp' if the parent route's protocol is 'tcp'") + end + elsif message.protocol == 'tcp' + raise Error.new("Destination protocol must be 'http1' or 'http2' if the parent route's protocol is 'http'") + end + end + end + end +end diff --git a/app/actions/service_credential_binding_app_create.rb b/app/actions/service_credential_binding_app_create.rb index 1bdbc2a97b6..00202e93a81 100644 --- a/app/actions/service_credential_binding_app_create.rb +++ b/app/actions/service_credential_binding_app_create.rb @@ -33,13 +33,13 @@ def precursor(service_instance, app: nil, volume_mount_services_enabled: false, credentials: {} } - (binding || ServiceBinding.new).tap do |b| + ServiceBinding.new.tap do |b| ServiceBinding.db.transaction do + binding.destroy if binding b.save_with_attributes_and_new_operation( binding_details, CREATE_IN_PROGRESS_OPERATION ) - MetadataUpdate.update(b, message) end end @@ -62,7 +62,7 @@ def validate_service_instance!(app, service_instance, volume_mount_services_enab def validate_binding!(binding) if binding - already_bound! if binding.create_succeeded? || binding.create_in_progress? || binding.last_operation.nil? + already_bound! if binding.create_succeeded? || binding.create_in_progress? incomplete_deletion! if binding.delete_failed? || binding.delete_in_progress? end end diff --git a/app/actions/service_credential_binding_key_create.rb b/app/actions/service_credential_binding_key_create.rb index cfaca6e3fa8..bf03c7674f8 100644 --- a/app/actions/service_credential_binding_key_create.rb +++ b/app/actions/service_credential_binding_key_create.rb @@ -28,8 +28,9 @@ def precursor(service_instance, message:) credentials: {} } - (key || ServiceKey.new).tap do |b| + ServiceKey.new.tap do |b| ServiceKey.db.transaction do + key.destroy if key b.save_with_attributes_and_new_operation( binding_details, CREATE_IN_PROGRESS_OPERATION @@ -59,7 +60,7 @@ def validate_service_instance!(service_instance) def validate_key!(key, message_name) if key - key_already_exists!(message_name) if key.create_succeeded? || key.create_in_progress? || key.last_operation.nil? + key_already_exists!(message_name) if key.create_succeeded? || key.create_in_progress? key_incomplete_deletion!(message_name) if key.delete_failed? || key.delete_in_progress? end end diff --git a/app/actions/update_route_destinations.rb b/app/actions/update_route_destinations.rb index fcf83dca374..58a3427dd0a 100644 --- a/app/actions/update_route_destinations.rb +++ b/app/actions/update_route_destinations.rb @@ -79,7 +79,7 @@ def update(route, to_add, to_delete, user_audit_info, manifest_triggered) end to_add.each do |rm| - validate_protocol_matches(rm) + validate_protocol_matches!(rm) route_mapping = RouteMappingModel.new(rm) route_mapping.save @@ -204,7 +204,7 @@ def validate_unique!(new_route_mappings) raise DuplicateDestinationError.new('Destinations cannot contain duplicate entries') if new_route_mappings.any? { |rm| new_route_mappings.count(rm) > 1 } end - def validate_protocol_matches(route_destination) + def validate_protocol_matches!(route_destination) destination_protocol = route_destination[:protocol] return unless destination_protocol diff --git a/app/controllers/v3/routes_controller.rb b/app/controllers/v3/routes_controller.rb index c216fd74403..e09d573cbff 100644 --- a/app/controllers/v3/routes_controller.rb +++ b/app/controllers/v3/routes_controller.rb @@ -1,5 +1,6 @@ -require 'messages/route_create_message' require 'messages/route_destinations_list_message' +require 'messages/route_create_message' +require 'messages/route_destination_update_message' require 'messages/routes_list_message' require 'messages/route_show_message' require 'messages/route_update_message' @@ -9,6 +10,7 @@ require 'presenters/v3/route_presenter' require 'presenters/v3/route_destinations_presenter' require 'presenters/v3/paginated_list_presenter' +require 'actions/route_destination_update' require 'actions/route_create' require 'actions/route_delete' require 'actions/route_update' @@ -144,6 +146,24 @@ def replace_destinations unprocessable!(e.message) end + def update_destination + message = RouteDestinationUpdateMessage.new(hashed_params[:body]) + unprocessable!(message.errors.full_messages) unless message.valid? + + route = Route.find(guid: hashed_params[:guid]) + route_not_found! unless route && permission_queryer.can_read_route?(route.space.guid, route.organization.guid) + unauthorized! unless permission_queryer.can_manage_apps_in_space?(route.space.guid) + + destination = RouteMappingModel.find(guid: hashed_params[:destination_guid]) + unprocessable_destination! unless destination + + RouteDestinationUpdate.update(destination, message) + + render status: :ok, json: Presenters::V3::RouteDestinationPresenter.new(destination) + rescue RouteDestinationUpdate::Error => e + unprocessable!(e.message) + end + def route @route || begin @route = Route.find(guid: hashed_params[:guid]) diff --git a/app/controllers/v3/service_credential_bindings_controller.rb b/app/controllers/v3/service_credential_bindings_controller.rb index 8e9993036a3..7a5dcf98da0 100644 --- a/app/controllers/v3/service_credential_bindings_controller.rb +++ b/app/controllers/v3/service_credential_bindings_controller.rb @@ -121,9 +121,7 @@ def destroy def details ensure_service_credential_binding_is_accessible! not_found! unless can_read_secrets_in_the_binding_space? - if service_credential_binding.create_in_progress? || service_credential_binding.create_failed? - not_found_with_message!(service_credential_binding) - end + not_found_with_message!(service_credential_binding) unless service_credential_binding.create_succeeded? credentials = if service_credential_binding[:type] == 'key' && service_credential_binding.credhub_reference? fetch_credentials_value(service_credential_binding.credhub_reference) @@ -391,8 +389,9 @@ def binding_operation_in_progress! def not_found_with_message!(service_credential_binding) type = service_credential_binding.is_a?(ServiceKey) ? 'key' : 'binding' + operation = service_credential_binding.last_operation.type == 'create' ? 'Creation' : 'Deletion' state = service_credential_binding.last_operation.state - resource_not_found_with_message!("Creation of service #{type} #{state}") + resource_not_found_with_message!("#{operation} of service #{type} #{state}") end def not_found! diff --git a/app/jobs/reoccurring_job.rb b/app/jobs/reoccurring_job.rb index ef1259b73ef..2d6ce7f974d 100644 --- a/app/jobs/reoccurring_job.rb +++ b/app/jobs/reoccurring_job.rb @@ -3,7 +3,7 @@ module VCAP::CloudController module Jobs class ReoccurringJob < VCAP::CloudController::Jobs::CCJob - attr_reader :finished, :start_time + attr_reader :finished, :start_time, :retry_number def success(current_delayed_job) pollable_job = PollableJobModel.find_by_delayed_job(current_delayed_job) @@ -49,6 +49,7 @@ def polling_interval_seconds=(interval) def initialize @start_time = Time.now @finished = false + @retry_number = 0 end def default_maximum_duration_seconds @@ -59,8 +60,16 @@ def default_polling_interval_seconds Config.config.get(:broker_client_default_async_poll_interval_seconds) end + def default_polling_exponential_backoff + Config.config.get(:broker_client_async_poll_exponential_backoff_rate) + end + + def next_execution_in + polling_interval_seconds * default_polling_exponential_backoff**retry_number + end + def next_enqueue_would_exceed_maximum_duration? - Time.now + polling_interval_seconds > start_time + maximum_duration_seconds + Time.now + next_execution_in > start_time + maximum_duration_seconds end def finish @@ -75,9 +84,10 @@ def expire! def enqueue_next_job(pollable_job) opts = { queue: Jobs::Queues.generic, - run_at: Delayed::Job.db_time_now + polling_interval_seconds + run_at: Delayed::Job.db_time_now + next_execution_in } + @retry_number += 1 Jobs::Enqueuer.new(self, opts).enqueue_pollable(existing_guid: pollable_job.guid) end end diff --git a/app/messages/route_destination_update_message.rb b/app/messages/route_destination_update_message.rb new file mode 100644 index 00000000000..bab049edc20 --- /dev/null +++ b/app/messages/route_destination_update_message.rb @@ -0,0 +1,25 @@ +module VCAP::CloudController + class RouteDestinationUpdateMessage < BaseMessage + def initialize(params) + super(params) + end + + def self.key_requested?(key) + proc { |a| a.requested?(key) } + end + + register_allowed_keys [:protocol] + + validates_with NoAdditionalKeysValidator + + validate :validate_protocol?, if: key_requested?(:protocol) + + private + + def validate_protocol? + unless protocol.is_a?(String) && RouteMappingModel::VALID_PROTOCOLS.include?(protocol) + errors.add(:destination, "protocol must be 'http1', 'http2' or 'tcp'.") + end + end + end +end diff --git a/app/models/runtime/helpers/service_credential_binding_mixin.rb b/app/models/runtime/helpers/service_credential_binding_mixin.rb index 0c5c9fee112..ef4f1b6848d 100644 --- a/app/models/runtime/helpers/service_credential_binding_mixin.rb +++ b/app/models/runtime/helpers/service_credential_binding_mixin.rb @@ -9,6 +9,7 @@ def operation_in_progress? end def create_succeeded? + return true unless last_operation return true if last_operation&.type == 'create' && last_operation.state == 'succeeded' false diff --git a/app/models/services/service_binding.rb b/app/models/services/service_binding.rb index 5c2c0b059af..42ff1fca2c1 100644 --- a/app/models/services/service_binding.rb +++ b/app/models/services/service_binding.rb @@ -119,20 +119,6 @@ def last_operation service_binding_operation end - def is_created? - return true unless last_operation - - if last_operation.type == 'create' && last_operation.state != 'succeeded' - return false - end - - if last_operation.type == 'delete' && last_operation.state == 'succeeded' - return false - end - - true - end - def save_with_attributes_and_new_operation(attributes, operation) save_with_new_operation(operation, attributes: attributes) self diff --git a/app/presenters/system_environment/system_env_presenter.rb b/app/presenters/system_environment/system_env_presenter.rb index 5f669b2d214..7fbab00f106 100644 --- a/app/presenters/system_environment/system_env_presenter.rb +++ b/app/presenters/system_environment/system_env_presenter.rb @@ -14,7 +14,7 @@ def system_env def service_binding_env_variables services_hash = {} - @service_bindings.select(&:is_created?).each do |service_binding| + @service_bindings.select(&:create_succeeded?).each do |service_binding| service_name = service_binding_label(service_binding) services_hash[service_name.to_sym] ||= [] services_hash[service_name.to_sym] << service_binding_env_values(service_binding) diff --git a/app/presenters/v3/route_destination_presenter.rb b/app/presenters/v3/route_destination_presenter.rb index de36a4f1ec7..38482c08d8e 100644 --- a/app/presenters/v3/route_destination_presenter.rb +++ b/app/presenters/v3/route_destination_presenter.rb @@ -2,7 +2,21 @@ module VCAP::CloudController::Presenters::V3 class RouteDestinationPresenter < BasePresenter + def initialize( + resource, + show_secrets: false, + censored_message: VCAP::CloudController::Presenters::Censorship::REDACTED_CREDENTIAL + ) + super(resource, show_secrets: show_secrets, censored_message: censored_message, decorators: []) + end + def to_hash + hash = destination_hash + hash[:links] = build_links + hash + end + + def destination_hash { guid: destination.guid, app: { @@ -13,12 +27,26 @@ def to_hash }, weight: destination.weight, port: destination.presented_port, - protocol: destination.protocol + protocol: destination.protocol, } end private + def build_links + links = { + destintions: { + href: url_builder.build_url(path: "/v3/routes/#{destination.route_guid}/destinations") + }, + } + + links[:route] = { + href: url_builder.build_url(path: "/v3/routes/#{destination.route_guid}") + } + + links + end + def destination @resource end diff --git a/app/presenters/v3/route_destinations_presenter.rb b/app/presenters/v3/route_destinations_presenter.rb index 14bc1f129c3..5faf89ab258 100644 --- a/app/presenters/v3/route_destinations_presenter.rb +++ b/app/presenters/v3/route_destinations_presenter.rb @@ -20,6 +20,12 @@ def to_hash } end + def presented_destinations + destinations.map do |route_mapping| + RouteDestinationPresenter.new(route_mapping).destination_hash + end + end + private attr_reader :route @@ -28,12 +34,6 @@ def destinations @resource end - def presented_destinations - destinations.map do |route_mapping| - RouteDestinationPresenter.new(route_mapping).to_hash - end - end - def build_links links = { self: { diff --git a/app/presenters/v3/route_presenter.rb b/app/presenters/v3/route_presenter.rb index 79b64351646..40df122c7f1 100644 --- a/app/presenters/v3/route_presenter.rb +++ b/app/presenters/v3/route_presenter.rb @@ -33,7 +33,7 @@ def to_hash path: route.path, port: route.port, url: build_url, - destinations: route.route_mappings.map { |rm| RouteDestinationPresenter.new(rm).to_hash }, + destinations: RouteDestinationsPresenter.new(route.route_mappings, route: route).presented_destinations, metadata: { labels: hashified_labels(route.labels), annotations: hashified_annotations(route.annotations), diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml index c92e8a709ed..2efd274a1fa 100644 --- a/config/cloud_controller.yml +++ b/config/cloud_controller.yml @@ -60,6 +60,7 @@ max_retained_revisions_per_app: 100 broker_client_default_async_poll_interval_seconds: 60 broker_client_max_async_poll_duration_minutes: 10080 +broker_client_async_poll_exponential_backoff_rate: 1.0 shared_isolation_segment_name: 'shared' diff --git a/config/routes.rb b/config/routes.rb index 67d62efcd34..7fff6026612 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -166,6 +166,7 @@ post '/routes/:guid/destinations', to: 'routes#insert_destinations' patch '/routes/:guid/destinations', to: 'routes#replace_destinations' delete '/routes/:guid/destinations/:destination_guid', to: 'routes#destroy_destination' + patch '/routes/:guid/destinations/:destination_guid', to: 'routes#update_destination' # security_groups post '/security_groups', to: 'security_groups#create' diff --git a/db/migrations/20220216124400_add_delayed_job_guid_index_to_jobs.rb b/db/migrations/20220216124400_add_delayed_job_guid_index_to_jobs.rb new file mode 100644 index 00000000000..5cbfeb4a19a --- /dev/null +++ b/db/migrations/20220216124400_add_delayed_job_guid_index_to_jobs.rb @@ -0,0 +1,13 @@ +Sequel.migration do + up do + alter_table :jobs do + add_index :delayed_job_guid, name: :jobs_delayed_job_guid_index + end + end + + down do + alter_table :jobs do + drop_index :delayed_job_guid, name: :jobs_delayed_job_guid_index + end + end +end diff --git a/docs/v3/Gemfile b/docs/v3/Gemfile index 2c6699b07eb..d66d8a3f336 100644 --- a/docs/v3/Gemfile +++ b/docs/v3/Gemfile @@ -8,7 +8,7 @@ gem 'middleman-sprockets', '~> 4.1' gem 'middleman-gh-pages', '>= 0.0.3' gem 'middleman-livereload', '>= 3.4.6' gem 'mini_racer' -gem 'nokogiri', '~> 1.13.1' +gem 'nokogiri', '~> 1.13.3' gem 'rake', '>= 12.3.3' gem 'redcarpet', '~> 3.5.1' gem 'sass' diff --git a/docs/v3/Gemfile.lock b/docs/v3/Gemfile.lock index 824416b7175..46cf4b81fe5 100644 --- a/docs/v3/Gemfile.lock +++ b/docs/v3/Gemfile.lock @@ -93,12 +93,12 @@ GEM middleman-syntax (2.1.0) middleman-core (>= 3.2) rouge (~> 1.0) - mini_portile2 (2.7.1) + mini_portile2 (2.8.0) mini_racer (0.4.0) libv8-node (~> 15.14.0.0) minitest (5.14.4) - nokogiri (1.13.1) - mini_portile2 (~> 2.7.0) + nokogiri (1.13.3) + mini_portile2 (~> 2.8.0) racc (~> 1.4) padrino-helpers (0.15.1) i18n (>= 0.6.7, < 2) @@ -154,7 +154,7 @@ DEPENDENCIES middleman-sprockets (~> 4.1) middleman-syntax (= 2.1) mini_racer - nokogiri (~> 1.13.1) + nokogiri (~> 1.13.3) rake (>= 12.3.3) redcarpet (~> 3.5.1) sass diff --git a/docs/v3/package-lock.json b/docs/v3/package-lock.json index ba4b0e764d3..fee52357f2a 100644 --- a/docs/v3/package-lock.json +++ b/docs/v3/package-lock.json @@ -11,7 +11,7 @@ "devDependencies": { "check-pages": "^0.10.0", "cheerio": "^1.0.0-rc.10", - "express": "^4.17.2", + "express": "^4.17.3", "glob": "^7.2.0", "gulp": "^4.0.2", "gulp-cli": "^2.3.0" @@ -25,13 +25,13 @@ "optional": true }, "node_modules/accepts": { - "version": "1.3.7", - "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.7.tgz", - "integrity": "sha512-Il80Qs2WjYlJIBNzNkK6KYqlVMTbZLXgHx2oT0pU/fjRHyEp+PEfEPY0R3WCwAGVOtauxh1hOxNgIf5bv7dQpA==", + "version": "1.3.8", + "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.8.tgz", + "integrity": "sha512-PYAthTa2m2VKxuvSD3DPC/Gy+U+sOA1LAuT8mkmRuvw+NACSaeXEQ+NHcVF7rONl6qcaxV3Uuemwawk+7+SJLw==", "dev": true, "dependencies": { - "mime-types": "~2.1.24", - "negotiator": "0.6.2" + "mime-types": "~2.1.34", + "negotiator": "0.6.3" }, "engines": { "node": ">= 0.6" @@ -524,20 +524,20 @@ } }, "node_modules/body-parser": { - "version": "1.19.1", - "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.19.1.tgz", - "integrity": "sha512-8ljfQi5eBk8EJfECMrgqNGWPEY5jWP+1IzkzkGdFFEwFQZZyaZ21UqdaHktgiMlH0xLHqIFtE/u2OYE5dOtViA==", + "version": "1.19.2", + "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.19.2.tgz", + "integrity": "sha512-SAAwOxgoCKMGs9uUAUFHygfLAyaniaoun6I8mFY9pRAJL9+Kec34aU+oIjDhTycub1jozEfEwx1W1IuOYxVSFw==", "dev": true, "dependencies": { - "bytes": "3.1.1", + "bytes": "3.1.2", "content-type": "~1.0.4", "debug": "2.6.9", "depd": "~1.1.2", "http-errors": "1.8.1", "iconv-lite": "0.4.24", "on-finished": "~2.3.0", - "qs": "6.9.6", - "raw-body": "2.4.2", + "qs": "6.9.7", + "raw-body": "2.4.3", "type-is": "~1.6.18" }, "engines": { @@ -545,9 +545,9 @@ } }, "node_modules/body-parser/node_modules/qs": { - "version": "6.9.6", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.9.6.tgz", - "integrity": "sha512-TIRk4aqYLNoJUbd+g2lEdz5kLWIuTMRagAXxl78Q0RiVjAOugHmeKNGdd3cwo/ktpf9aL9epCfFqWDEKysUlLQ==", + "version": "6.9.7", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.9.7.tgz", + "integrity": "sha512-IhMFgUmuNpyRfxA90umL7ByLlgRXu6tIfKPpF5TmcfRLlLCckfP/g3IQmju6jjpu+Hh8rA+2p6A27ZSPOOHdKw==", "dev": true, "engines": { "node": ">=0.6" @@ -631,9 +631,9 @@ "dev": true }, "node_modules/bytes": { - "version": "3.1.1", - "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.1.1.tgz", - "integrity": "sha512-dWe4nWO/ruEOY7HkUJ5gFt1DCFV9zPRoJr8pV0/ASQermOZjtq8jMjOprC0Kd10GLN+l7xaUPvxzJFWtxGu8Fg==", + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.1.2.tgz", + "integrity": "sha512-/Nf7TyzTx6S3yRJObOAV7956r8cr2+Oj8AC5dt8wSP3BQAoeX58NoHyCU8P8zGkNXStjTSi6fzO6F0pBdcYbEg==", "dev": true, "engines": { "node": ">= 0.8" @@ -1226,9 +1226,9 @@ } }, "node_modules/cookie": { - "version": "0.4.1", - "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.4.1.tgz", - "integrity": "sha512-ZwrFkGJxUR3EIoXtO+yVE69Eb7KlixbaeAWfBQB9vVsNn/o+Yw69gBWSSDK825hQNdN+wF8zELf3dFNl/kxkUA==", + "version": "0.4.2", + "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.4.2.tgz", + "integrity": "sha512-aSWTXFzaKWkvHO1Ny/s+ePFpvKsPnjc551iI41v3ny/ow6tBG5Vd+FuqGNhh1LxOmVzOlGUriIlOaokOvhaStA==", "dev": true, "engines": { "node": ">= 0.6" @@ -1829,17 +1829,17 @@ } }, "node_modules/express": { - "version": "4.17.2", - "resolved": "https://registry.npmjs.org/express/-/express-4.17.2.tgz", - "integrity": "sha512-oxlxJxcQlYwqPWKVJJtvQiwHgosH/LrLSPA+H4UxpyvSS6jC5aH+5MoHFM+KABgTOt0APue4w66Ha8jCUo9QGg==", + "version": "4.17.3", + "resolved": "https://registry.npmjs.org/express/-/express-4.17.3.tgz", + "integrity": "sha512-yuSQpz5I+Ch7gFrPCk4/c+dIBKlQUxtgwqzph132bsT6qhuzss6I8cLJQz7B3rFblzd6wtcI0ZbGltH/C4LjUg==", "dev": true, "dependencies": { - "accepts": "~1.3.7", + "accepts": "~1.3.8", "array-flatten": "1.1.1", - "body-parser": "1.19.1", + "body-parser": "1.19.2", "content-disposition": "0.5.4", "content-type": "~1.0.4", - "cookie": "0.4.1", + "cookie": "0.4.2", "cookie-signature": "1.0.6", "debug": "2.6.9", "depd": "~1.1.2", @@ -1854,7 +1854,7 @@ "parseurl": "~1.3.3", "path-to-regexp": "0.1.7", "proxy-addr": "~2.0.7", - "qs": "6.9.6", + "qs": "6.9.7", "range-parser": "~1.2.1", "safe-buffer": "5.2.1", "send": "0.17.2", @@ -1870,9 +1870,9 @@ } }, "node_modules/express/node_modules/qs": { - "version": "6.9.6", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.9.6.tgz", - "integrity": "sha512-TIRk4aqYLNoJUbd+g2lEdz5kLWIuTMRagAXxl78Q0RiVjAOugHmeKNGdd3cwo/ktpf9aL9epCfFqWDEKysUlLQ==", + "version": "6.9.7", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.9.7.tgz", + "integrity": "sha512-IhMFgUmuNpyRfxA90umL7ByLlgRXu6tIfKPpF5TmcfRLlLCckfP/g3IQmju6jjpu+Hh8rA+2p6A27ZSPOOHdKw==", "dev": true, "engines": { "node": ">=0.6" @@ -3548,9 +3548,9 @@ } }, "node_modules/negotiator": { - "version": "0.6.2", - "resolved": "https://registry.npmjs.org/negotiator/-/negotiator-0.6.2.tgz", - "integrity": "sha512-hZXc7K2e+PgeI1eDBe/10Ard4ekbfrrqG8Ep+8Jmf4JID2bNg7NvCPOZN+kfF574pFQI7mum2AUqDidoKqcTOw==", + "version": "0.6.3", + "resolved": "https://registry.npmjs.org/negotiator/-/negotiator-0.6.3.tgz", + "integrity": "sha512-+EUsqGPLsM+j/zdChZjsnX51g4XrHFOIXwfnCVPGlQk/k5giakcKsuxCObBRu6DSm9opw/O6slWbJdghQM4bBg==", "dev": true, "engines": { "node": ">= 0.6" @@ -4135,12 +4135,12 @@ } }, "node_modules/raw-body": { - "version": "2.4.2", - "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-2.4.2.tgz", - "integrity": "sha512-RPMAFUJP19WIet/99ngh6Iv8fzAbqum4Li7AD6DtGaW2RpMB/11xDoalPiJMTbu6I3hkbMVkATvZrqb9EEqeeQ==", + "version": "2.4.3", + "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-2.4.3.tgz", + "integrity": "sha512-UlTNLIcu0uzb4D2f4WltY6cVjLi+/jEN4lgEUj3E04tpMDpUlkBo/eSn6zou9hum2VMNpCCUone0O0WeJim07g==", "dev": true, "dependencies": { - "bytes": "3.1.1", + "bytes": "3.1.2", "http-errors": "1.8.1", "iconv-lite": "0.4.24", "unpipe": "1.0.0" @@ -5587,13 +5587,13 @@ "optional": true }, "accepts": { - "version": "1.3.7", - "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.7.tgz", - "integrity": "sha512-Il80Qs2WjYlJIBNzNkK6KYqlVMTbZLXgHx2oT0pU/fjRHyEp+PEfEPY0R3WCwAGVOtauxh1hOxNgIf5bv7dQpA==", + "version": "1.3.8", + "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.8.tgz", + "integrity": "sha512-PYAthTa2m2VKxuvSD3DPC/Gy+U+sOA1LAuT8mkmRuvw+NACSaeXEQ+NHcVF7rONl6qcaxV3Uuemwawk+7+SJLw==", "dev": true, "requires": { - "mime-types": "~2.1.24", - "negotiator": "0.6.2" + "mime-types": "~2.1.34", + "negotiator": "0.6.3" } }, "acorn": { @@ -5983,27 +5983,27 @@ } }, "body-parser": { - "version": "1.19.1", - "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.19.1.tgz", - "integrity": "sha512-8ljfQi5eBk8EJfECMrgqNGWPEY5jWP+1IzkzkGdFFEwFQZZyaZ21UqdaHktgiMlH0xLHqIFtE/u2OYE5dOtViA==", + "version": "1.19.2", + "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.19.2.tgz", + "integrity": "sha512-SAAwOxgoCKMGs9uUAUFHygfLAyaniaoun6I8mFY9pRAJL9+Kec34aU+oIjDhTycub1jozEfEwx1W1IuOYxVSFw==", "dev": true, "requires": { - "bytes": "3.1.1", + "bytes": "3.1.2", "content-type": "~1.0.4", "debug": "2.6.9", "depd": "~1.1.2", "http-errors": "1.8.1", "iconv-lite": "0.4.24", "on-finished": "~2.3.0", - "qs": "6.9.6", - "raw-body": "2.4.2", + "qs": "6.9.7", + "raw-body": "2.4.3", "type-is": "~1.6.18" }, "dependencies": { "qs": { - "version": "6.9.6", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.9.6.tgz", - "integrity": "sha512-TIRk4aqYLNoJUbd+g2lEdz5kLWIuTMRagAXxl78Q0RiVjAOugHmeKNGdd3cwo/ktpf9aL9epCfFqWDEKysUlLQ==", + "version": "6.9.7", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.9.7.tgz", + "integrity": "sha512-IhMFgUmuNpyRfxA90umL7ByLlgRXu6tIfKPpF5TmcfRLlLCckfP/g3IQmju6jjpu+Hh8rA+2p6A27ZSPOOHdKw==", "dev": true } } @@ -6076,9 +6076,9 @@ "dev": true }, "bytes": { - "version": "3.1.1", - "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.1.1.tgz", - "integrity": "sha512-dWe4nWO/ruEOY7HkUJ5gFt1DCFV9zPRoJr8pV0/ASQermOZjtq8jMjOprC0Kd10GLN+l7xaUPvxzJFWtxGu8Fg==", + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.1.2.tgz", + "integrity": "sha512-/Nf7TyzTx6S3yRJObOAV7956r8cr2+Oj8AC5dt8wSP3BQAoeX58NoHyCU8P8zGkNXStjTSi6fzO6F0pBdcYbEg==", "dev": true }, "cache-base": { @@ -6565,9 +6565,9 @@ } }, "cookie": { - "version": "0.4.1", - "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.4.1.tgz", - "integrity": "sha512-ZwrFkGJxUR3EIoXtO+yVE69Eb7KlixbaeAWfBQB9vVsNn/o+Yw69gBWSSDK825hQNdN+wF8zELf3dFNl/kxkUA==", + "version": "0.4.2", + "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.4.2.tgz", + "integrity": "sha512-aSWTXFzaKWkvHO1Ny/s+ePFpvKsPnjc551iI41v3ny/ow6tBG5Vd+FuqGNhh1LxOmVzOlGUriIlOaokOvhaStA==", "dev": true }, "cookie-signature": { @@ -7082,17 +7082,17 @@ } }, "express": { - "version": "4.17.2", - "resolved": "https://registry.npmjs.org/express/-/express-4.17.2.tgz", - "integrity": "sha512-oxlxJxcQlYwqPWKVJJtvQiwHgosH/LrLSPA+H4UxpyvSS6jC5aH+5MoHFM+KABgTOt0APue4w66Ha8jCUo9QGg==", + "version": "4.17.3", + "resolved": "https://registry.npmjs.org/express/-/express-4.17.3.tgz", + "integrity": "sha512-yuSQpz5I+Ch7gFrPCk4/c+dIBKlQUxtgwqzph132bsT6qhuzss6I8cLJQz7B3rFblzd6wtcI0ZbGltH/C4LjUg==", "dev": true, "requires": { - "accepts": "~1.3.7", + "accepts": "~1.3.8", "array-flatten": "1.1.1", - "body-parser": "1.19.1", + "body-parser": "1.19.2", "content-disposition": "0.5.4", "content-type": "~1.0.4", - "cookie": "0.4.1", + "cookie": "0.4.2", "cookie-signature": "1.0.6", "debug": "2.6.9", "depd": "~1.1.2", @@ -7107,7 +7107,7 @@ "parseurl": "~1.3.3", "path-to-regexp": "0.1.7", "proxy-addr": "~2.0.7", - "qs": "6.9.6", + "qs": "6.9.7", "range-parser": "~1.2.1", "safe-buffer": "5.2.1", "send": "0.17.2", @@ -7120,9 +7120,9 @@ }, "dependencies": { "qs": { - "version": "6.9.6", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.9.6.tgz", - "integrity": "sha512-TIRk4aqYLNoJUbd+g2lEdz5kLWIuTMRagAXxl78Q0RiVjAOugHmeKNGdd3cwo/ktpf9aL9epCfFqWDEKysUlLQ==", + "version": "6.9.7", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.9.7.tgz", + "integrity": "sha512-IhMFgUmuNpyRfxA90umL7ByLlgRXu6tIfKPpF5TmcfRLlLCckfP/g3IQmju6jjpu+Hh8rA+2p6A27ZSPOOHdKw==", "dev": true }, "safe-buffer": { @@ -8501,9 +8501,9 @@ } }, "negotiator": { - "version": "0.6.2", - "resolved": "https://registry.npmjs.org/negotiator/-/negotiator-0.6.2.tgz", - "integrity": "sha512-hZXc7K2e+PgeI1eDBe/10Ard4ekbfrrqG8Ep+8Jmf4JID2bNg7NvCPOZN+kfF574pFQI7mum2AUqDidoKqcTOw==", + "version": "0.6.3", + "resolved": "https://registry.npmjs.org/negotiator/-/negotiator-0.6.3.tgz", + "integrity": "sha512-+EUsqGPLsM+j/zdChZjsnX51g4XrHFOIXwfnCVPGlQk/k5giakcKsuxCObBRu6DSm9opw/O6slWbJdghQM4bBg==", "dev": true }, "next-tick": { @@ -8978,12 +8978,12 @@ "dev": true }, "raw-body": { - "version": "2.4.2", - "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-2.4.2.tgz", - "integrity": "sha512-RPMAFUJP19WIet/99ngh6Iv8fzAbqum4Li7AD6DtGaW2RpMB/11xDoalPiJMTbu6I3hkbMVkATvZrqb9EEqeeQ==", + "version": "2.4.3", + "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-2.4.3.tgz", + "integrity": "sha512-UlTNLIcu0uzb4D2f4WltY6cVjLi+/jEN4lgEUj3E04tpMDpUlkBo/eSn6zou9hum2VMNpCCUone0O0WeJim07g==", "dev": true, "requires": { - "bytes": "3.1.1", + "bytes": "3.1.2", "http-errors": "1.8.1", "iconv-lite": "0.4.24", "unpipe": "1.0.0" diff --git a/docs/v3/package.json b/docs/v3/package.json index 0c502044864..96baa205852 100644 --- a/docs/v3/package.json +++ b/docs/v3/package.json @@ -21,7 +21,7 @@ "devDependencies": { "check-pages": "^0.10.0", "cheerio": "^1.0.0-rc.10", - "express": "^4.17.2", + "express": "^4.17.3", "glob": "^7.2.0", "gulp": "^4.0.2", "gulp-cli": "^2.3.0" diff --git a/lib/cloud_controller/config_schemas/base/api_schema.rb b/lib/cloud_controller/config_schemas/base/api_schema.rb index dd297138462..e0b7575e7ed 100644 --- a/lib/cloud_controller/config_schemas/base/api_schema.rb +++ b/lib/cloud_controller/config_schemas/base/api_schema.rb @@ -184,6 +184,7 @@ class ApiSchema < VCAP::Config broker_client_timeout_seconds: Integer, broker_client_default_async_poll_interval_seconds: Integer, broker_client_max_async_poll_duration_minutes: Integer, + broker_client_async_poll_exponential_backoff_rate: Float, optional(:uaa_client_name) => String, optional(:uaa_client_secret) => String, optional(:uaa_client_scope) => String, diff --git a/lib/cloud_controller/config_schemas/base/worker_schema.rb b/lib/cloud_controller/config_schemas/base/worker_schema.rb index 12ee9f6724b..6249ebe7416 100644 --- a/lib/cloud_controller/config_schemas/base/worker_schema.rb +++ b/lib/cloud_controller/config_schemas/base/worker_schema.rb @@ -111,6 +111,7 @@ class WorkerSchema < VCAP::Config broker_client_timeout_seconds: Integer, broker_client_default_async_poll_interval_seconds: Integer, broker_client_max_async_poll_duration_minutes: Integer, + broker_client_async_poll_exponential_backoff_rate: Float, optional(:uaa_client_name) => String, optional(:uaa_client_secret) => String, optional(:uaa_client_scope) => String, diff --git a/spec/request/route_destinations_spec.rb b/spec/request/route_destinations_spec.rb index 886a56bd4da..d4084c6ac07 100644 --- a/spec/request/route_destinations_spec.rb +++ b/spec/request/route_destinations_spec.rb @@ -1142,6 +1142,118 @@ end end + describe 'UPDATE /v3/routes/:guid/destinations/:destination_guid' do + let(:user_header) { headers_for(user) } + let(:route) { VCAP::CloudController::Route.make(space: space) } + let(:app_model) { VCAP::CloudController::AppModel.make(space: space) } + + let!(:destination_to_preserve) do + VCAP::CloudController::RouteMappingModel.make( + app: app_model, + route: route, + process_type: 'web', + app_port: VCAP::CloudController::ProcessModel::DEFAULT_HTTP_PORT, + weight: nil + ) + end + + let!(:destination_to_update) do + VCAP::CloudController::RouteMappingModel.make( + app: app_model, + route: route, + process_type: 'worker', + app_port: VCAP::CloudController::ProcessModel::DEFAULT_HTTP_PORT, + weight: nil + ) + end + + context 'permissions' do + let(:api_call) { lambda { |user_headers| patch "/v3/routes/#{route.guid}/destinations/#{destination_to_update.guid}", { protocol: 'http1' }.to_json, user_headers } } + + let(:db_check) do + lambda do + get "/v3/routes/#{route.guid}/destinations", {}, admin_headers + parsed_response = MultiJson.load(last_response.body) + expect(parsed_response['destinations'].length).to eq(1) + expect(parsed_response['destinations'][0]['guid']).to eq(destination_to_preserve.guid) + end + end + + let(:expected_codes_and_responses) do + h = Hash.new( + code: 403, + ) + + h['admin'] = { code: 200 } + h['space_developer'] = { code: 200 } + h['space_supporter'] = { code: 200 } + h['org_billing_manager'] = { code: 404 } + h['no_role'] = { code: 404 } + h + end + end + + context 'when the route does not exist' do + it 'returns not found' do + patch "/v3/routes/does-not-exist/destinations/#{destination_to_update.guid}", { protocol: 'http1' }.to_json, admin_header + expect(last_response.status).to eq(404) + end + end + + context 'when the destination does not exist' do + it 'returns 422 with a helpful message' do + patch "/v3/routes/#{route.guid}/destinations/does-not-exist", { protocol: 'http1' }.to_json, admin_header + expect(last_response.status).to eq(422) + expect(last_response).to have_error_message('Unable to unmap route from destination. Ensure the route has a destination with this guid.') + end + end + + context 'when the route has a protocol of tcp' do + let(:routing_api_client) { double('routing_api_client', router_group: router_group) } + let(:router_group) { double('router_group', type: 'tcp', guid: 'router-group-guid') } + let(:tcp_route) do + UAARequests.stub_all + allow_any_instance_of(CloudController::DependencyLocator).to receive(:routing_api_client).and_return(routing_api_client) + allow_any_instance_of(VCAP::CloudController::RouteValidator).to receive(:validate) + + VCAP::CloudController::Route.make(:tcp, space: space) + end + let(:tcp_app) { VCAP::CloudController::AppModel.make(space: space) } + let!(:destination) do + VCAP::CloudController::RouteMappingModel.make( + app: tcp_app, + route: tcp_route, + process_type: 'worker', + app_port: VCAP::CloudController::ProcessModel::DEFAULT_HTTP_PORT, + weight: nil + ) + end + + context 'and the destination has a protocol of tcp' do + it 'succeeds' do + patch "/v3/routes/#{tcp_route.guid}/destinations/#{destination.guid}", { protocol: 'tcp' }.to_json, admin_header + expect(last_response.status).to eq(200) + end + end + + context 'and the destination has a protocol of http1' do + it 'it returns 422 with a helpful message' do + patch "/v3/routes/#{tcp_route.guid}/destinations/#{destination.guid}", { protocol: 'http1' }.to_json, admin_header + expect(last_response.status).to eq(422) + expect(last_response).to have_error_message("Destination protocol must be 'tcp' if the parent route's protocol is 'tcp'") + end + end + + context 'and the destination has a protocol of http2' do + it 'it returns 422 with a helpful message' do + patch "/v3/routes/#{tcp_route.guid}/destinations/#{destination.guid}", { protocol: 'http2' }.to_json, admin_header + expect(last_response.status).to eq(422) + expect(last_response).to have_error_message("Destination protocol must be 'tcp' if the parent route's protocol is 'tcp'") + end + end + end + end + describe 'DELETE /v3/routes/:guid/destinations/:destination_guid' do let(:user_header) { headers_for(user) } let(:route) { VCAP::CloudController::Route.make(space: space) } diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb index 885140cc7af..a1c4d5aa2aa 100644 --- a/spec/request/service_credential_bindings_spec.rb +++ b/spec/request/service_credential_bindings_spec.rb @@ -614,6 +614,17 @@ def check_filtered_bindings(*bindings) } } + context "last binding operation is in 'create succeeded' state" do + before do + app_binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' }) + end + + it 'returns details' do + api_call.call(admin_headers) + expect(last_response).to have_status_code(200) + end + end + context "last binding operation is in 'create in progress' state" do before do app_binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'in progress' }) @@ -646,6 +657,38 @@ def check_filtered_bindings(*bindings) end end + context "last binding operation is in 'delete in progress' state" do + before do + app_binding.save_with_attributes_and_new_operation({}, { type: 'delete', state: 'in progress' }) + end + + it 'returns an error' do + api_call.call(admin_headers) + expect(last_response).to have_status_code(404) + expect(parsed_response['errors']).to include(include({ + 'detail' => 'Deletion of service binding in progress', + 'title' => 'CF-ResourceNotFound', + 'code' => 10010, + })) + end + end + + context "last binding operation is in 'delete failed' state" do + before do + app_binding.save_with_attributes_and_new_operation({}, { type: 'delete', state: 'failed' }) + end + + it 'returns an error' do + api_call.call(admin_headers) + expect(last_response).to have_status_code(404) + expect(parsed_response['errors']).to include(include({ + 'detail' => 'Deletion of service binding failed', + 'title' => 'CF-ResourceNotFound', + 'code' => 10010, + })) + end + end + context 'permissions' do it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS do let(:expected_codes_and_responses) do diff --git a/spec/unit/actions/route_destination_update_spec.rb b/spec/unit/actions/route_destination_update_spec.rb new file mode 100644 index 00000000000..cad9020a01c --- /dev/null +++ b/spec/unit/actions/route_destination_update_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' +require 'actions/route_destination_update' + +module VCAP::CloudController + RSpec.describe RouteDestinationUpdate do + subject(:destination_update) { RouteDestinationUpdate } + + describe '#update' do + let!(:destination) { RouteMappingModel.make({ protocol: 'http1' }) } + + let(:message) do + VCAP::CloudController::RouteDestinationUpdateMessage.new({ + protocol: 'http2' + }) + end + + it 'updates the destination record' do + updated_destination = RouteDestinationUpdate.update(destination, message) + + expect(updated_destination.protocol).to eq 'http2' + end + + context 'when the given protocol is incompatible' do + context 'for tcp route' do + let(:routing_api_client) { double('routing_api_client', router_group: router_group) } + let(:router_group) { double('router_group', type: 'tcp', guid: 'router-group-guid') } + let(:tcp_route) do + UAARequests.stub_all + allow_any_instance_of(CloudController::DependencyLocator).to receive(:routing_api_client).and_return(routing_api_client) + allow_any_instance_of(VCAP::CloudController::RouteValidator).to receive(:validate) + + VCAP::CloudController::Route.make(:tcp) + end + let!(:tcp_destination) { RouteMappingModel.make({ route: tcp_route }) } + it 'does not update the destination record' do + expect { RouteDestinationUpdate.update(tcp_destination, message) }.to raise_error(StandardError) + end + end + + context 'for http route' do + it 'does not update the destination record' do + message.protocol = 'tcp' + expect(message).to be_valid + expect { RouteDestinationUpdate.update(destination, message) }.to raise_error(StandardError) + end + end + end + end + end +end diff --git a/spec/unit/actions/service_credential_binding_app_create_spec.rb b/spec/unit/actions/service_credential_binding_app_create_spec.rb index 1c3e7d1de51..08f0d052a0c 100644 --- a/spec/unit/actions/service_credential_binding_app_create_spec.rb +++ b/spec/unit/actions/service_credential_binding_app_create_spec.rb @@ -82,11 +82,12 @@ module V3 binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'failed' }) end - it 'updates and returns the existing binding' do + it 'deletes the existing binding and creates a new one' do b = action.precursor(service_instance, app: app, message: message) - expect(b.id).to eq(binding.id) + expect(b.guid).not_to eq(binding.guid) expect(b.create_in_progress?).to be_truthy + expect { binding.reload }.to raise_error Sequel::NoExistingObject end end diff --git a/spec/unit/actions/service_credential_binding_key_create_spec.rb b/spec/unit/actions/service_credential_binding_key_create_spec.rb index 3b8ae3527b7..c91763f5c1b 100644 --- a/spec/unit/actions/service_credential_binding_key_create_spec.rb +++ b/spec/unit/actions/service_credential_binding_key_create_spec.rb @@ -71,11 +71,12 @@ module V3 binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'failed' }) end - it 'updates and returns the existing key' do + it 'deletes the existing key and creates a new one' do b = action.precursor(service_instance, message: message) - expect(b.id).to eq(binding.id) + expect(b.guid).not_to eq(binding.guid) expect(b.create_in_progress?).to be_truthy + expect { binding.reload }.to raise_error Sequel::NoExistingObject end end diff --git a/spec/unit/jobs/reoccurring_job_spec.rb b/spec/unit/jobs/reoccurring_job_spec.rb index 44bd3302b87..424a2216514 100644 --- a/spec/unit/jobs/reoccurring_job_spec.rb +++ b/spec/unit/jobs/reoccurring_job_spec.rb @@ -75,10 +75,13 @@ def perform job.polling_interval_seconds = 95 expect(job.polling_interval_seconds).to eq(95) - enqueued_time = Time.now + enqueued_time = 0 - Jobs::Enqueuer.new(job, queue: Jobs::Queues.generic).enqueue_pollable - execute_all_jobs(expected_successes: 1, expected_failures: 0) + Timecop.freeze do + Jobs::Enqueuer.new(job, queue: Jobs::Queues.generic).enqueue_pollable + execute_all_jobs(expected_successes: 1, expected_failures: 0) + enqueued_time = Time.now + end Timecop.freeze(94.seconds.after(enqueued_time)) do execute_all_jobs(expected_successes: 0, expected_failures: 0) @@ -98,15 +101,123 @@ def perform expect(job.polling_interval_seconds).to eq(24.hours) end + context 'exponential backoff rate' do + context 'updates the polling interval' do + it 'when changing exponential backoff rate only' do + TestConfig.config[:broker_client_async_poll_exponential_backoff_rate] = 2.0 + + enqueued_time = 0 + + Timecop.freeze do + Jobs::Enqueuer.new(FakeJob.new, queue: Jobs::Queues.generic).enqueue_pollable + execute_all_jobs(expected_successes: 1, expected_failures: 0) + enqueued_time = Time.now + end + + [60, 180, 420, 900, 1860].each do |seconds| + Timecop.freeze((seconds - 1).seconds.after(enqueued_time)) do + execute_all_jobs(expected_successes: 0, expected_failures: 0) + end + + Timecop.freeze((seconds + 1).seconds.after(enqueued_time)) do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + end + end + end + it 'when changing exponential backoff rate and default polling interval' do + TestConfig.config[:broker_client_async_poll_exponential_backoff_rate] = 1.3 + TestConfig.config[:broker_client_default_async_poll_interval_seconds] = 10 + + enqueued_time = 0 + + Timecop.freeze do + Jobs::Enqueuer.new(FakeJob.new, queue: Jobs::Queues.generic).enqueue_pollable + execute_all_jobs(expected_successes: 1, expected_failures: 0) + enqueued_time = Time.now + end + + [10, 23, 39.9, 61.8, 90.4].each do |seconds| + Timecop.freeze((seconds - 1).seconds.after(enqueued_time)) do + execute_all_jobs(expected_successes: 0, expected_failures: 0) + end + + Timecop.freeze((seconds.ceil + 1).seconds.after(enqueued_time)) do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + end + end + end + it 'when changing exponential backoff rate and retry_after from the job' do + TestConfig.config[:broker_client_async_poll_exponential_backoff_rate] = 1.3 + TestConfig.config[:broker_client_default_async_poll_interval_seconds] = 10 + + enqueued_time = 0 + + Timecop.freeze do + Jobs::Enqueuer.new(FakeJob.new(retry_after: [20, 30]), queue: Jobs::Queues.generic).enqueue_pollable + execute_all_jobs(expected_successes: 1, expected_failures: 0) + enqueued_time = Time.now + end + + # the job should run after 20s * 1.3^0 = 20 seconds + Timecop.freeze(19.seconds.after(enqueued_time)) do + execute_all_jobs(expected_successes: 0, expected_failures: 0) + end + + Timecop.freeze(21.seconds.after(enqueued_time)) do + enqueued_time = Time.now + execute_all_jobs(expected_successes: 1, expected_failures: 0) + end + + # the job should run after 30s * 1.3^1 = 39 seconds + Timecop.freeze(38.seconds.after(enqueued_time)) do + execute_all_jobs(expected_successes: 0, expected_failures: 0) + end + + Timecop.freeze(40.seconds.after(enqueued_time)) do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + end + end + end + + it 'takes the exponential backoff into account when checking whether the next run would exceed the maximum duration' do + TestConfig.config[:broker_client_async_poll_exponential_backoff_rate] = 1.3 + TestConfig.config[:broker_client_max_async_poll_duration_minutes] = 60 + + job = FakeJob.new(iterations: 100) + # With a backoff rate of 1.3, 11 jobs could have been executed in 60 minutes (initial run + 10 retries). + job.instance_variable_set(:@retry_number, 10) + + enqueued_time = 0 + + Timecop.freeze do + Jobs::Enqueuer.new(job, queue: Jobs::Queues.generic).enqueue_pollable + enqueued_time = Time.now + end + + # The calculated backoff for the 11th retry would be 3384.321 seconds. + Timecop.freeze(enqueued_time + 3384.321.ceil.seconds) do + execute_all_jobs(expected_successes: 0, expected_failures: 1, jobs_to_execute: 1) + expect(PollableJobModel.first.state).to eq('FAILED') + expect(PollableJobModel.first.cf_api_error).not_to be_nil + error = YAML.safe_load(PollableJobModel.first.cf_api_error) + expect(error['errors'].first['code']).to eq(290006) + expect(error['errors'].first['detail']). + to eq('The job execution has timed out.') + end + end + end + context 'updates the polling interval if config changes' do it 'when changed from the job only' do - job = FakeJob.new(retry_after: ['20', '30']) TestConfig.config[:broker_client_default_async_poll_interval_seconds] = 10 - enqueued_time = Time.now + enqueued_time = 0 - Jobs::Enqueuer.new(job, queue: Jobs::Queues.generic).enqueue_pollable - execute_all_jobs(expected_successes: 1, expected_failures: 0) + Timecop.freeze do + Jobs::Enqueuer.new(FakeJob.new(retry_after: [20, 30]), queue: Jobs::Queues.generic).enqueue_pollable + execute_all_jobs(expected_successes: 1, expected_failures: 0) + enqueued_time = Time.now + end Timecop.freeze(19.seconds.after(enqueued_time)) do execute_all_jobs(expected_successes: 0, expected_failures: 0) @@ -127,13 +238,15 @@ def perform end it 'when default changed after changing from the job' do - job = FakeJob.new(retry_after: [20]) TestConfig.config[:broker_client_default_async_poll_interval_seconds] = 10 - enqueued_time = Time.now + enqueued_time = 0 - Jobs::Enqueuer.new(job, queue: Jobs::Queues.generic).enqueue_pollable - execute_all_jobs(expected_successes: 1, expected_failures: 0) + Timecop.freeze do + Jobs::Enqueuer.new(FakeJob.new(retry_after: [20]), queue: Jobs::Queues.generic).enqueue_pollable + execute_all_jobs(expected_successes: 1, expected_failures: 0) + enqueued_time = Time.now + end Timecop.freeze(19.seconds.after(enqueued_time)) do execute_all_jobs(expected_successes: 0, expected_failures: 0) @@ -155,13 +268,15 @@ def perform end it 'when changing default only' do - job = FakeJob.new TestConfig.config[:broker_client_default_async_poll_interval_seconds] = 10 - enqueued_time = Time.now + enqueued_time = 0 - Jobs::Enqueuer.new(job, queue: Jobs::Queues.generic).enqueue_pollable - execute_all_jobs(expected_successes: 1, expected_failures: 0) + Timecop.freeze do + Jobs::Enqueuer.new(FakeJob.new, queue: Jobs::Queues.generic).enqueue_pollable + execute_all_jobs(expected_successes: 1, expected_failures: 0) + enqueued_time = Time.now + end Timecop.freeze(9.seconds.after(enqueued_time)) do execute_all_jobs(expected_successes: 0, expected_failures: 0) diff --git a/spec/unit/messages/route_destination_update_message_spec.rb b/spec/unit/messages/route_destination_update_message_spec.rb new file mode 100644 index 00000000000..1fecaf03e31 --- /dev/null +++ b/spec/unit/messages/route_destination_update_message_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' +require 'messages/route_destination_update_message' + +module VCAP::CloudController + RSpec.describe RouteDestinationUpdateMessage do + describe 'protocol' do + it 'accepts a tcp protocol param' do + message = RouteDestinationUpdateMessage.new({ 'protocol' => 'tcp' }) + expect(message).to be_valid + end + + it 'accepts an http1 protocol param' do + message = RouteDestinationUpdateMessage.new({ 'protocol' => 'http1' }) + expect(message).to be_valid + end + + it 'accepts an http2 protocol param' do + message = RouteDestinationUpdateMessage.new({ 'protocol' => 'http2' }) + expect(message).to be_valid + end + + it 'accepts empty params' do + message = RouteDestinationUpdateMessage.new({}) + expect(message).to be_valid + end + + it 'does not accept any other strings' do + message = RouteDestinationUpdateMessage.new({ 'protocol' => 'my-cool-protocol' }) + expect(message).not_to be_valid + expect(message.errors[:destination]).to include("protocol must be 'http1', 'http2' or 'tcp'.") + end + + it 'does not accept protocol params of incorrect type' do + message = RouteDestinationUpdateMessage.new({ 'protocol' => 123 }) + expect(message).not_to be_valid + expect(message.errors[:destination]).to include("protocol must be 'http1', 'http2' or 'tcp'.") + end + + it 'does not accept any other params' do + message = RouteDestinationUpdateMessage.new({ 'foobar' => 'pants' }) + expect(message).not_to be_valid + expect(message.errors[:base]).to include("Unknown field(s): 'foobar'") + end + end + end +end diff --git a/spec/unit/models/services/service_binding_spec.rb b/spec/unit/models/services/service_binding_spec.rb index 824b1d04d01..592db0be274 100644 --- a/spec/unit/models/services/service_binding_spec.rb +++ b/spec/unit/models/services/service_binding_spec.rb @@ -357,83 +357,6 @@ def new_model end end - describe 'is_created?' do - let(:service_instance) { ManagedServiceInstance.make } - let(:service_binding) { ServiceBinding.make(service_instance: service_instance) } - - context 'when the service binding has been created synchronously' do - it 'returns true' do - expect(service_binding.is_created?).to be true - end - end - - context 'when the service binding is being created asynchronously' do - let(:state) {} - let(:operation) { ServiceBindingOperation.make(type: 'create', state: state) } - - before do - service_binding.service_binding_operation = operation - end - - context 'and the operation is in progress' do - let(:state) { 'in progress' } - - it 'returns false' do - expect(service_binding.is_created?).to be false - end - end - - context 'and the operation has failed' do - let(:state) { 'failed' } - - it 'returns false' do - expect(service_binding.is_created?).to be false - end - end - - context 'and the operation has succeeded' do - let(:state) { 'succeeded' } - - it 'returns true' do - expect(service_binding.is_created?).to be true - end - end - end - - context 'when the service binding is being deleted asynchronously' do - let(:state) {} - let(:operation) { ServiceBindingOperation.make(type: 'delete', state: state) } - - before do - service_binding.service_binding_operation = operation - end - - context 'and the operation is in progress' do - let(:state) { 'in progress' } - - it 'returns true' do - expect(service_binding.is_created?).to be true - end - end - - context 'and the operation has failed' do - let(:state) { 'failed' } - - it 'returns true' do - expect(service_binding.is_created?).to be true - end - end - - context 'and the operation has succeeded' do - let(:state) { 'succeeded' } - - it 'returns false' do - expect(service_binding.is_created?).to be false - end - end - end - end - describe '#save_with_new_operation' do let(:service_instance) { ServiceInstance.make } let(:app) { AppModel.make(space: service_instance.space) } diff --git a/spec/unit/models/services/service_credential_binding_shared.rb b/spec/unit/models/services/service_credential_binding_shared.rb index f1a85d4fac8..c8e28ee2b9f 100644 --- a/spec/unit/models/services/service_credential_binding_shared.rb +++ b/spec/unit/models/services/service_credential_binding_shared.rb @@ -76,6 +76,33 @@ end end + describe '#create_succeeded?' do + let(:operation) { nil } + + context 'when there is no binding operation' do + it 'returns true' do + expect(service_credential_binding.create_succeeded?).to be true + end + end + + context 'when there is a binding operation' do + it "returns true only for the 'create succeeded' state" do + [ + { type: 'create', state: 'failed', result: false }, + { type: 'create', state: 'in progress', result: false }, + { type: 'create', state: 'succeeded', result: true }, + { type: 'delete', state: 'in progress', result: false }, + { type: 'delete', state: 'failed', result: false }, + { type: 'delete', state: 'succeeded', result: false }, + ].each do |test| + service_credential_binding.save_with_attributes_and_new_operation({}, { type: test[:type], state: test[:state] }) + + expect(service_credential_binding.create_succeeded?).to be test[:result] + end + end + end + end + describe '#create_failed?' do let(:operation) { nil } diff --git a/spec/unit/presenters/system_environment/system_env_presenter_spec.rb b/spec/unit/presenters/system_environment/system_env_presenter_spec.rb index 94c63540bf2..fc31e045984 100644 --- a/spec/unit/presenters/system_environment/system_env_presenter_spec.rb +++ b/spec/unit/presenters/system_environment/system_env_presenter_spec.rb @@ -77,12 +77,21 @@ module VCAP::CloudController service_binding.service_binding_operation = service_binding_operation end - it 'includes service binding and instance information' do - expect(system_env_presenter.system_env[:VCAP_SERVICES][service.label.to_sym]).to have(1).items - binding = system_env_presenter.system_env[:VCAP_SERVICES][service.label.to_sym].first.to_hash + it 'does not include service binding and instance information' do + expect(system_env_presenter.system_env[:VCAP_SERVICES][service.label.to_sym]).to be_nil + end + end - expect(binding[:credentials]).to eq(service_binding.credentials) - expect(binding[:name]).to eq('elephantsql-vip-uat') + context 'when a delete service binding failed' do + let(:service_binding_operation) { ServiceBindingOperation.make(type: 'delete', state: 'failed') } + let!(:service_binding) { ServiceBinding.make(app: app, service_instance: service_instance) } + + before do + service_binding.service_binding_operation = service_binding_operation + end + + it 'does not include service binding and instance information' do + expect(system_env_presenter.system_env[:VCAP_SERVICES][service.label.to_sym]).to be_nil end end diff --git a/spec/unit/presenters/v3/route_destination_presenter_spec.rb b/spec/unit/presenters/v3/route_destination_presenter_spec.rb index 6bfc0c49605..66a91fc36e9 100644 --- a/spec/unit/presenters/v3/route_destination_presenter_spec.rb +++ b/spec/unit/presenters/v3/route_destination_presenter_spec.rb @@ -18,10 +18,33 @@ module VCAP::CloudController::Presenters::V3 end describe '#to_hash' do - let(:result) { presenter.to_hash } - it 'presents the route mapping as a destination' do expect(presenter.to_hash).to eq( + guid: route_mapping.guid, + app: { + guid: route_mapping.app_guid, + process: { + type: route_mapping.process_type + } + }, + weight: route_mapping.weight, + port: route_mapping.presented_port, + protocol: route_mapping.protocol, + links: { + destintions: { + href: "http://api2.vcap.me/v3/routes/#{route.guid}/destinations" + }, + route: { + href: "http://api2.vcap.me/v3/routes/#{route.guid}" + } + } + ) + end + end + + describe '#destination_hash' do + it 'presents the route mapping as a destination' do + expect(presenter.destination_hash).to eq( guid: route_mapping.guid, app: { guid: route_mapping.app_guid,