Add suspended state for spaces and deleting state for spaces and orgs#5206
Open
johha wants to merge 5 commits into
Open
Add suspended state for spaces and deleting state for spaces and orgs#5206johha wants to merge 5 commits into
johha wants to merge 5 commits into
Conversation
Schema, model, and permission helpers underpinning the new
suspended/deleting lifecycle states. No controller, message, or
access-class wiring yet — those land in follow-up commits.
- Migration adds Space#status (default 'active'), mirroring the
existing Organization#status column.
- OrgSpaceStatus mixin gains DELETING constant, deleting? /
suspended_or_deleting? predicates; Space and Organization both
pick them up.
- Space gains in_suspended_or_deleting_org? convenience helper
that walks to the owning org. Domain (and via inheritance,
PrivateDomain) gains the same shape. ProcessModel, Route,
ServiceKey, ServiceBinding, ServiceInstance delegate it through
to their space (with safe-nav where the space association may
be nil during concurrent deletion).
- Permissions gains space_state, writable_space_state,
is_org_manager_for_org? helpers; org_state /
writable_org_state extended to surface :deleting alongside
:suspended / :active. Subquery-backed to dodge the Sequel
association include? quirks.
- The renamed in_suspended_or_deleting_org? replaces
in_suspended_org? — every existing v2 caller already paired
suspended with deleting, so the wider semantics match the
actual contract.
…ollers
Mechanical rename pass — every v3 controller previously called
'suspended! unless permission_queryer.is_space_active?(id)' (or the
org variant). This commit:
- Adds require_writable_space! and require_writable_org! to
application_controller.rb. They consult the permission helpers
introduced in the previous commit (writable_space_state /
writable_org_state) and raise the right error code per state:
OrgSuspended, SpaceSuspended, or ResourceBeingDeleted.
- Replaces every 'suspended! unless permission_queryer.is_*_active?'
call site with the new helper. Behavior is preserved for the
existing 'active' / 'suspended' branches; the new ':deleting'
branch is now reachable end-to-end.
- Two legacy v2 controllers (route_mappings_controller,
service_bindings_controller) consume Permissions directly and
can't use the v3 helpers; they're updated to call
'writable_space_state(id) == :active' to match the new
Permissions API.
This should be straightforward to review by scanning for the rename
pattern. The substantive feature wiring (suspended/deleting on the
spaces controller, error codes, messages, actions, docs) lands in
the next commit.
The substantive v3 feature work, on top of the controller helpers
adopted in the previous commit:
- SpaceCreateMessage / SpaceUpdateMessage accept 'suspended'.
- SpaceCreate / SpaceUpdate honor it; SpaceUpdate refuses to
reactivate a deleting space ('Space is being deleted and cannot
be reactivated').
- SpacesV3Controller restricts mutation of the 'suspended' field
to admins and org managers — space managers and below can no
longer toggle it. Mirrors the org-side fix from #5173/#5181 with
is_org_manager_for_org? from the Permissions helpers.
- OrganizationUpdate refuses to reactivate a deleting org with
the same message shape.
- SpacePresenter exposes 'suspended' in the response.
- errors/v2.yml registers SpaceSuspended (10019, 403) and
ResourceBeingDeleted (10020, 422) so the new states have stable
error codes for both v2 and v3 callers.
- Docs and request specs follow the controller wiring.
v2 is deprecated; the new suspended/deleting space feature is
v3-only. v2's role here is just safeguarding the lifecycle state so
parallel operations don't sneak through. Anyone (including org
managers) who needs to operate inside a non-active space goes
through v3.
Per access class: replace inline (suspended? || deleting?) at the
org and space level with the matching predicate from the model
layer:
return false if x.in_suspended_or_deleting_org?
return false if x.suspended_or_deleting? # space-level
(or org-level only for files that don't touch a space, e.g.
private_domain_access, space_quota_definition_access).
Also flagged: space_quota_definition_access previously only
blocked writes during 'suspended', not 'deleting' — a latent gap
from when the deleting state was added. Aligning it here.
Admins continue to bypass via the existing
'return true if admin_user?' / can_write_globally? short-circuits
that precede every state check.
Behavior: in v2, org managers can no longer write through a
suspended/deleting space (they could before). v3 behavior is
unchanged — org managers continue to be the 'admins of an org'
there.
philippthun
reviewed
Jun 26, 2026
| def create?(app, _params=nil) | ||
| return true if admin_user? | ||
| return false if app.in_suspended_org? | ||
| return false if app.in_suspended_or_deleting_org? || app.space&.suspended_or_deleting? |
Member
There was a problem hiding this comment.
Helper method? app.in_suspended_or_deleting_space?
| def can_write_to_route(route, is_create=false) | ||
| return true if context.queryer.can_write_globally? | ||
| return false if route.in_suspended_org? | ||
| return false if route.in_suspended_or_deleting_org? || route.space.suspended_or_deleting? |
Member
There was a problem hiding this comment.
Helper method? route.in_suspended_or_deleting_space?
Why not route.space&?
|
|
||
| FeatureFlag.raise_unless_enabled!(:service_instance_creation) | ||
| return false if service_instance.in_suspended_org? | ||
| return false if service_instance.in_suspended_or_deleting_org? || service_instance.space&.suspended_or_deleting? |
Member
There was a problem hiding this comment.
Helper method? service_instance.in_suspended_or_deleting_space?
|
|
||
| def require_writable_space!(space) | ||
| org_state = permission_queryer.writable_org_state(space.organization_id) | ||
| return resource_being_deleted!('organization') if org_state == :deleting |
Member
There was a problem hiding this comment.
These return statements are not needed. Do we use this pattern elsewhere? And without return we can skip the empty lines (maybe keep one before permission_queryer.writable_space_state).
Comment on lines
+166
to
+173
| case permission_queryer.writable_org_state(org_id) | ||
| when :active | ||
| nil | ||
| when :deleting | ||
| resource_being_deleted!('organization') | ||
| when :suspended | ||
| suspended! | ||
| end |
Member
There was a problem hiding this comment.
This would look more compact to me. Does Rubocop complain then?
org_state = permission_queryer.writable_org_state(org_id)
resource_being_deleted!('organization') if org_state == :deleting
suspended! if org_state == :suspended
| AnnotationsUpdate.update(org, message.annotations, OrganizationAnnotationModel) | ||
|
|
||
| if message.requested?(:suspended) | ||
| error!("Organization '#{org.name}' is being deleted and cannot be reactivated.") if org.deleting? && message.suspended == false |
Member
There was a problem hiding this comment.
There is no check for setting a DELETING org to SUSPENDED. I think this should also be forbidden.
| org_state = permission_queryer.writable_org_state(space.organization_id) | ||
| return resource_being_deleted!('organization') if org_state == :deleting | ||
|
|
||
| space_state = permission_queryer.writable_space_state(space.id) |
Member
There was a problem hiding this comment.
Pass org_state as optional parameter, so that writable_space_state does not fetch it again. This reduces the total number of DB statements issued by require_writable_space! from 3 to 2.
| Sequel.migration do | ||
| up do | ||
| alter_table :spaces do | ||
| add_column :status, String, null: false, default: 'active', size: 255 unless @db.schema(:spaces).map(&:first).include?(:status) |
Member
There was a problem hiding this comment.
Claude suggests to omit the null: false as this could be expensive on MySQL and the org.status field is also nullable at the moment.
At a later point in time we could (should) mark the column in both tables as NOT NULL and also set a size (255) for org.status.
| return true if can_write_globally? # admins can modify suspended orgs | ||
| def org_state(org_id) | ||
| status = VCAP::CloudController::Organization.where(id: org_id).get(:status) | ||
| return :active if status.nil? |
Member
There was a problem hiding this comment.
Does :active for a non-existing org (and space) make sense? What about returning nil instead?
| raise CloudController::Errors::ApiError.new_from_details('AppNotFound', request_attrs['app_guid']) unless process | ||
| raise CloudController::Errors::ApiError.new_from_details('NotAuthorized') unless permissions.can_write_to_active_space?(process.space.id) | ||
| raise CloudController::Errors::ApiError.new_from_details('OrgSuspended') unless permissions.is_space_active?(process.space.id) | ||
| raise CloudController::Errors::ApiError.new_from_details('OrgSuspended') unless permissions.writable_space_state(process.space.id) == :active |
Member
There was a problem hiding this comment.
This raises OrgSuspended also for a suspended space or a deleting org/space. Not sure if we want to fix this for the v2 controllers (app/controllers/runtime/route_mappings_controller.rb and app/controllers/services/service_bindings_controller.rb)?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A short explanation of the proposed change
Adds a suspended state for spaces (v3-only), mirroring the existing org-suspended feature. While the writes blocked by space-suspended are now well-defined, OrgManagers act as the "admins of an org" and can toggle space suspended; SpaceManagers and below cannot. Admins continue to bypass.
Also introduces internal deleting state plumbing for both spaces and orgs (no controller path flips it yet, it's prep for an upcoming async-delete flow - see Make recursive delete operations more intuitive #3589 ). Surfaces a new CF-ResourceBeingDeleted (10020) error so the rest of the system can react to it.
Branch is split into 4 small commits to make review easier:
An explanation of the use cases your change solves
Links to any other associated PRs:
Builds on OrgManager can suspend their own org but cannot un-suspend it #5173 / Restrict suspending orgs to admins (#5173) #5181 (org-side admin-only suspended fix). This PR mirrors that authority model on spaces, with OrgManager added as the space-scope equivalent of "org admin".
Pre-existing API quirks on the suspended field surfaced during black-box testing are tracked in #TODO (out of scope for this PR).
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the main branch
I have run all the unit tests using bundle exec rake
I have run CF Acceptance Tests