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

Prevent deletion of current droplet #3960

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
4 changes: 4 additions & 0 deletions .rubocop_cc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ Metrics/CyclomaticComplexity:
Max: 12
Metrics/MethodLength:
Max: 60
Metrics/ModuleLength:
Max: 200
Exclude:
- spec/**/*
Migration/AddConstraintName: # Exclude for old Migrations
Include:
- 'db/migrations/**/*'
Expand Down
76 changes: 34 additions & 42 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2024-07-25 15:02:19 UTC using RuboCop version 1.65.0.
# on 2024-09-11 09:08:10 UTC using RuboCop version 1.66.1.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
Expand All @@ -14,13 +14,13 @@ Lint/AmbiguousRange:
- 'lib/cloud_controller/resource_match.rb'
- 'spec/unit/messages/validators/metadata_validator_spec.rb'

# Offense count: 78
# Offense count: 77
# Configuration parameters: AllowedMethods.
# AllowedMethods: enums
Lint/ConstantDefinitionInBlock:
Enabled: false

# Offense count: 82
# Offense count: 83
# Configuration parameters: AllowComments, AllowEmptyLambdas.
Lint/EmptyBlock:
Enabled: false
Expand Down Expand Up @@ -80,7 +80,7 @@ Lint/UnusedMethodArgument:
- 'spec/support/fakes/fake_service_broker_v2_client.rb'

# Offense count: 1
# This cop supports unsafe autocorrection (--autocorrect-all).
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AutoCorrect.
Lint/UselessAssignment:
Exclude:
Expand All @@ -101,27 +101,22 @@ Lint/UselessRescue:
Exclude:
- 'app/jobs/runtime/update_buildpack_installer.rb'

# Offense count: 782
# Offense count: 790
# Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
Metrics/AbcSize:
Max: 99

# Offense count: 113
# Offense count: 114
# Configuration parameters: CountComments, CountAsOne.
Metrics/ClassLength:
Max: 447

# Offense count: 435
# Configuration parameters: CountComments, CountAsOne.
Metrics/ModuleLength:
Max: 2227

# Offense count: 61
# Offense count: 62
# Configuration parameters: CountKeywordArgs, MaxOptionalParameters.
Metrics/ParameterLists:
Max: 9

# Offense count: 162
# Offense count: 163
# Configuration parameters: AllowedMethods, AllowedPatterns.
Metrics/PerceivedComplexity:
Max: 30
Expand Down Expand Up @@ -156,11 +151,11 @@ Naming/PredicateName:
Naming/VariableNumber:
Enabled: false

# Offense count: 388
# Offense count: 407
RSpec/AnyInstance:
Enabled: false

# Offense count: 163
# Offense count: 164
RSpec/Be:
Enabled: false

Expand All @@ -179,13 +174,13 @@ RSpec/ChangeByZero:
Exclude:
- 'spec/unit/actions/update_route_destinations_spec.rb'

# Offense count: 3462
# Offense count: 3478
# Configuration parameters: Prefixes, AllowedPatterns.
# Prefixes: when, with, without
RSpec/ContextWording:
Enabled: false

# Offense count: 95
# Offense count: 98
# Configuration parameters: IgnoredMetadata.
RSpec/DescribeClass:
Enabled: false
Expand All @@ -195,7 +190,7 @@ RSpec/DescribeMethod:
Exclude:
- 'spec/api/internal/log_access_api_spec.rb'

# Offense count: 4383
# Offense count: 4428
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: SkipBlocks, EnforcedStyle, OnlyStaticConstants.
# SupportedStyles: described_class, explicit
Expand All @@ -208,7 +203,7 @@ RSpec/DescribedClass:
RSpec/EmptyExampleGroup:
Enabled: false

# Offense count: 4255
# Offense count: 4293
# Configuration parameters: CountAsOne.
RSpec/ExampleLength:
Max: 158
Expand All @@ -232,7 +227,7 @@ RSpec/ExpectActual:
RSpec/ExpectInHook:
Enabled: false

# Offense count: 77
# Offense count: 79
RSpec/ExpectInLet:
Enabled: false

Expand All @@ -241,7 +236,7 @@ RSpec/IdenticalEqualityAssertion:
Exclude:
- 'spec/unit/lib/cloud_controller/dependency_locator_spec.rb'

# Offense count: 1496
# Offense count: 1499
# Configuration parameters: Max, AllowedIdentifiers, AllowedPatterns.
RSpec/IndexedLet:
Enabled: false
Expand Down Expand Up @@ -273,7 +268,7 @@ RSpec/MessageChain:
- 'spec/unit/messages/space_quota_update_message_spec.rb'
- 'spec/unit/messages/space_quotas_create_message_spec.rb'

# Offense count: 766
# Offense count: 785
# Configuration parameters: EnforcedStyle.
# SupportedStyles: have_received, receive
RSpec/MessageSpies:
Expand All @@ -290,22 +285,22 @@ RSpec/MultipleDescribes:
- 'spec/request/knowledge_bombs/verify_old_lrps_can_download_assets_spec.rb'
- 'spec/unit/lib/vcap/json_message_spec.rb'

# Offense count: 8030
# Offense count: 8089
RSpec/MultipleExpectations:
Max: 48

# Offense count: 8862
# Offense count: 8923
# Configuration parameters: AllowSubject.
RSpec/MultipleMemoizedHelpers:
Max: 36

# Offense count: 2521
# Offense count: 2549
# Configuration parameters: EnforcedStyle, IgnoreSharedExamples.
# SupportedStyles: always, named_only
RSpec/NamedSubject:
Enabled: false

# Offense count: 1589
# Offense count: 1598
# Configuration parameters: AllowedGroups.
RSpec/NestedGroups:
Max: 8
Expand Down Expand Up @@ -407,19 +402,19 @@ RSpec/ReturnFromStub:
Exclude:
- 'spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb'

# Offense count: 202
# Offense count: 204
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AutoCorrect.
RSpec/ScatteredLet:
Enabled: false

# Offense count: 32
# Offense count: 33
# Configuration parameters: Include, CustomTransform, IgnoreMethods, IgnoreMetadata.
# Include: **/*_spec.rb
RSpec/SpecFilePathFormat:
Enabled: false

# Offense count: 201
# Offense count: 203
RSpec/StubbedMock:
Enabled: false

Expand Down Expand Up @@ -462,7 +457,7 @@ RSpec/VariableName:
- 'spec/request/revisions_spec.rb'
- 'spec/request/spaces_spec.rb'

# Offense count: 530
# Offense count: 531
# Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
RSpec/VerifiedDoubles:
Enabled: false
Expand All @@ -475,7 +470,7 @@ RSpec/VoidExpect:
- 'spec/unit/jobs/runtime/prune_excess_app_revisions_spec.rb'
- 'spec/unit/lib/cloud_controller/integer_array_serializer_spec.rb'

# Offense count: 1590
# Offense count: 1591
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: ResponseMethods.
# ResponseMethods: response, last_response
Expand Down Expand Up @@ -508,14 +503,14 @@ Rails/ThreeStateBooleanColumn:
- 'db/migrations/20141022211551_add_updateable_column_to_services.rb'
- 'db/migrations/20151217235335_remove_unused_package_cols.rb'

# Offense count: 253
# Offense count: 256
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: strict, flexible
Rails/TimeZone:
Enabled: false

# Offense count: 7
# Offense count: 8
# Configuration parameters: TransactionMethods.
Rails/TransactionExitStatement:
Exclude:
Expand Down Expand Up @@ -554,7 +549,7 @@ Style/CaseLikeIf:
- 'lib/vcap/host_system.rb'
- 'spec/support/shared_examples/jobs/delayed_job.rb'

# Offense count: 2009
# Offense count: 2014
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: nested, compact
Expand All @@ -564,6 +559,7 @@ Style/ClassAndModuleChildren:
# Offense count: 3
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: AllowedReceivers.
# AllowedReceivers: params
Style/CollectionCompact:
Exclude:
- 'app/presenters/v3/app_presenter.rb'
Expand All @@ -586,7 +582,7 @@ Style/ConcatArrayLiterals:
Exclude:
- 'spec/unit/lib/cloud_controller/install_buildpacks_spec.rb'

# Offense count: 1483
# Offense count: 1486
# Configuration parameters: AllowedConstants.
Style/Documentation:
Enabled: false
Expand All @@ -611,7 +607,7 @@ Style/FloatDivision:
Exclude:
- 'app/presenters/v3/paginated_list_presenter.rb'

# Offense count: 3263
# Offense count: 3279
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, always_true, never
Expand All @@ -634,16 +630,12 @@ Style/HashEachMethods:
Exclude:
- 'app/messages/validators/label_selector_requirement_validator.rb'

# Offense count: 9
# Offense count: 5
# This cop supports unsafe autocorrection (--autocorrect-all).
Style/MapIntoArray:
Exclude:
- 'app/actions/space_delete.rb'
- 'app/presenters/v3/relationship_presenter.rb'
- 'app/presenters/v3/to_many_relationship_presenter.rb'
- 'docs/v3/helpers/docs_helpers.rb'
- 'lib/cloud_controller/install_buildpacks.rb'
- 'lib/cloud_controller/metrics/periodic_updater.rb'
- 'spec/unit/controllers/v3/isolation_segments_controller_spec.rb'
- 'spec/unit/presenters/v3/relationship_presenter_spec.rb'
- 'spec/unit/presenters/v3/to_many_relationship_presenter_spec.rb'
Expand All @@ -656,7 +648,7 @@ Style/MapToHash:
- 'app/models/runtime/app_model.rb'
- 'lib/cloud_controller/diego/reporters/instances_stats_reporter.rb'

# Offense count: 75
# Offense count: 76
Style/MultilineBlockChain:
Enabled: false

Expand Down
1 change: 1 addition & 0 deletions app/actions/app_assign_droplet.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def assign(app, droplet)
SidecarSynchronizeFromAppDroplet::ConflictingSidecarsError => e
raise InvalidDroplet.new(e.message)
rescue ProcessCreate::SidecarMemoryLessThanProcessMemory, Sequel::ValidationFailed => e
unable_to_assign! if e.is_a?(Sequel::ValidationFailed) && e.message.include?('droplet presence')
raise InvalidApp.new(e.message)
end

Expand Down
1 change: 1 addition & 0 deletions app/actions/app_delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def delete(apps, record_event: true)
app.db.transaction do
app.lock!

app.update(droplet_guid: nil)
delete_subresources(app)

record_audit_event(app) if record_event
Expand Down
8 changes: 4 additions & 4 deletions app/actions/droplet_delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ def delete(droplets)
droplets = Array(droplets)

droplets.each do |droplet|
DropletModel.db.transaction do
droplet.destroy
end

if droplet.blobstore_key
blobstore_delete = Jobs::Runtime::BlobstoreDelete.new(droplet.blobstore_key, :droplet_blobstore)
Jobs::Enqueuer.new(blobstore_delete, queue: Jobs::Queues.generic).enqueue
Expand All @@ -22,10 +26,6 @@ def delete(droplets)
droplet.app.space_guid,
droplet.app.space.organization_guid
)

DropletModel.db.transaction do
droplet.destroy
end
end

[]
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/v3/droplets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ def destroy

unauthorized! unless permission_queryer.can_write_to_active_space?(space.id)
suspended! unless permission_queryer.is_space_active?(space.id)
in_use!(droplet) if droplet.current?

delete_action = DropletDelete.new(user_audit_info)
deletion_job = VCAP::CloudController::Jobs::DeleteActionJob.new(DropletModel, droplet.guid, delete_action)
Expand Down Expand Up @@ -190,6 +191,10 @@ def unprocessable_app!(app_guid)
unprocessable!("App with guid \"#{app_guid}\" does not exist, or you do not have access to it.")
end

def in_use!(droplet)
raise droplet.in_use_error
end

def droplet_not_found!
resource_not_found!(:droplet)
end
Expand Down
5 changes: 5 additions & 0 deletions app/models/runtime/app_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ def around_save

errors.add(%i[space_guid name], :unique)
raise validation_failed_error
rescue Sequel::ForeignKeyConstraintViolation => e
raise e unless e.message.include?('fk_apps_droplet_guid')

errors.add(:droplet, :presence)
raise validation_failed_error
end

def validate
Expand Down
16 changes: 16 additions & 0 deletions app/models/runtime/droplet_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ class DropletModel < Sequel::Model(:droplets)
serializes_via_json :process_types
serializes_via_json :sidecars

def around_destroy
yield
rescue Sequel::ForeignKeyConstraintViolation => e
raise e unless e.message.include?('fk_apps_droplet_guid')

raise in_use_error
end

def error
e = [error_id, error_description].compact.join(' - ')
e.presence
Expand Down Expand Up @@ -172,6 +180,14 @@ def process_start_command(process_type)
process_types.try(:[], process_type) || ''
end

def current?
app.droplet_guid == guid
end

def in_use_error
CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', "The droplet is currently used by app with guid \"#{app_guid}\".")
end

private

def app_usage_event_repository
Expand Down
Loading