servala/servala-portal
Watch
10
0
Fork
You've already forked servala-portal
 0
Code Issues 44 Pull requests 6 Projects 2 Releases 30 Packages 2 Activity Actions

Self-Service Restore into new Instance #678

Merged
tobru merged 20 commits from feat/self-service-restore into main 2026-09-18 12:08:42 +00:00
Conversation 0 Commits 20 Files changed 29 +7112 -60
tobru commented 2026-09-17 04:56:39 +00:00
Owner
Copy link
No description provided. 
Adds the `restore` block to `ServiceDefinition.backup_config` and
`core/restore.py`, which reads it. The mode-dependent requirements live
in `validate_restore_config` rather than in the JSON schema, because
django-jsonform's editor rejects allOf/if/then.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The restore block's mode used django-jsonform's 'choices', which the admin
widget renders as an enum but jsonschema ignores, so a typo'd mode saved
cleanly and then read as "restore not configured" with nothing anywhere
saying otherwise. It now uses 'enum', and a block with something in it but
no valid mode is rejected outright, the way the sibling access block's
half-finished configs already are.

get_restore_config also drops blank entries inside lists, not just at the
top level: a blank exclude_paths row is an empty prefix, and an empty
prefix matches every spec path. It tolerates a non-dict backup_config
too, since JSONField validators only run under full_clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add a self-FK `restored_from` and a `restore_point` CharField to
ServiceInstance. SET_NULL because deleting the source is the documented
end of a successful cutover and must never cascade into the instance now
serving production.

The self-FK joins the admin's autocomplete fields so the form does not
render a select over every instance.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three gaps from the review of the restore relationship fields:

- `restored_from_name` denormalises the source's resource name, so the
  relationship stays readable after the documented end of a successful
  restore: deleting the source. Folded into migration 0068, which had
  not been applied anywhere yet.
- `ServiceInstance.clean()` rejects a self-reference and a source in
  another organization or on another control plane CRD. Claim names only
  resolve inside one namespace, and a cross-namespace claim makes the
  restore hang rather than fail.
- `sync_instances` reads `servala.com/restoredFrom` and
  `servala.com/restorePoint` back on import, so the annotations really do
  survive a portal-side rebuild. The name is kept even when no matching
  source exists to point the foreign key at.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`create_instance` gains `restored_from` and `restore_point`. The restore
block is written by `apply_restore_to_spec` as the last spec mutation, so
neither a hardcoded param nor a compute plan value can land on a restore
path and point the new instance at another claim. `claimName` always comes
from the source instance, never from user input.

The source is mirrored into `servala.com/restoredFrom` and
`servala.com/restorePoint` annotations, merged in next to the billing
annotations rather than built by `_build_billing_annotations`, which
`resync_billing_annotations` re-applies with a merge patch. "Latest
available state" goes on the wire as the literal `latest`, since an
annotation cannot hold "unset" meaningfully.

`restored_from_name` is filled in here too: `clean()` does not run on
`objects.create()`, and the column exists to outlive the SET_NULL FK when
the source is deleted after a successful cutover.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
create_instance wrote a restore block only if the service was configured
for it, but recorded the source, the annotations and the detail page
either way - so a service without a restore config produced an empty
instance that the UI, and the documented cutover, present as a copy of a
healthy one. Guard at the top of the method instead: same-namespace
source, a restore config that exists, and a restore point in snapshot
mode, where "latest" means nothing. Nothing is created and no namespace
is touched when it refuses.

Also hide spec.parameters.restore in the expert form, so the block the
portal owns cannot be added or repointed by hand; name the annotation
keys and the `latest` sentinel once in core.restore; and name a missing
configured path in apply_restore_to_spec instead of letting a KeyError
surface as "Error creating instance".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A restore doubles the bill for a service and starts a cutover somebody
has to finish, and for Keycloak the restored instance inherits the
source's admin password.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The button, the restore form's GET and its POST all ask the same
function, so a stale page or a crafted request cannot reach a restore
the detail page refused to offer. The permission check runs before the
first Kubernetes call, so a denied user costs no control plane round
trip.

A backup listing that could not be read is kept apart from an empty one:
claiming 'no backups' on a transport error would hide the only restore
point a snapshot-mode service has, so that blocks with a retry message,
while point-in-time mode proceeds with the window flagged as unknown.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A dependent's backup group is labelled with the dependent's service_label,
and validate_backup_config only checks snapshot_source against the own
definition's source labels - so a definition combining snapshot restore
with dependent_labels could offer a snapshot from another namespace, which
would then be written verbatim into the source claim's backupName. Filter
every group to the instance's own namespace in both modes, and sort the
combined rows newest first: list_backups only sorts within a group, and
callers read the last row as the oldest restorable point.

get_restore_availability now takes an optional backup_data and returns the
filtered rows under "backups", so the restore page and the detail page list
the backups once per render instead of two or three times.

It also refuses an instance with no status.instanceNamespace: the claim is
still being provisioned, so there is nothing to restore from, and without
the guard point-in-time mode offered the restore with a "window unknown"
warning while snapshot mode asked the user to try again forever.

The unavailable-versus-empty message is now decided by whether the group
being filtered to is present at all, rather than by the global unavailable
flag any partial failure raises.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The restore point is one string in every case, because create_instance
writes it verbatim into a Kubernetes spec and into a CharField: a
datetime or a None leaking through would be written raw.

The timestamp is read as UTC, as decided while planning - a
datetime-local input carries no zone, and a wrong guess recovers to the
wrong hour with no error anywhere. Microseconds are dropped by the
format, which the CRD's recoveryTimeStamp pattern rejects, and a
timestamp left behind by a switch back to "latest" is dropped rather
than silently restored to.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The field was labelled UTC but parsed in the server's timezone: naive input
goes through from_current_timezone(), so it was only correct because
settings.TIME_ZONE happens to be "UTC". Under an activated or configured
Europe/Zurich, a typed 10:30 became 08:30Z - a valid form, no error, and a
point-in-time restore to the wrong hour. UTCDateTimeField pins both
directions, prepare_value included, so a prefilled picker does not shift back
either.

While in there:

- restore_point is a property that refuses an unvalidated or invalid form. An
  empty string is a real answer here ("the latest available state"), so a
  caller skipping is_valid() would otherwise silently restore to latest.
- Constructing the form without a usable restore mode raises ValueError
  instead of quietly offering the point-in-time shape for a service
  create_instance will refuse.
- Both radio values are local widget constants now, with a note that sharing a
  value with core.restore's mode and wire sentinel is a coincidence.
- The restore_point tests assert the expected value per row, split unbound
  from validated, and check the CRD's timestamp pattern rather than str-ness.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A restore is a create: the new ServiceInstanceRestoreView renders the same
CRD form machinery as the create view, prefilled from the source instance's
live spec with the control plane and offering fixed, plus the restore point
picker and the compute plan form. get_restore_availability guards both GET
and POST, so a page left open while the instance was paused or while the
organization filled its last slot is refused on submit.

`?backup=` and `?at=` preselect a restore point as initial data only: the
snapshot choices are rebuilt from the listing this request made, so a name
that is no longer there renders as nothing selected and the ChoiceField
rejects it.

ServiceInstance.__getstate__ drops the cached `spec_object` on the way into
a pickle. It is an instance of a model class generated at runtime and never
added to the app registry, and auditlog deepcopies every instance it
serialises - which a restore reaches through `restored_from`. Without it,
rendering the form once made the next save of that row fail.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The curated form is the default mode, and it only submits the fields it
exposes - `majorVersion` is in no `form_config` in the repo, so a restore
through it dropped the source's major version, along with the backup,
security, monitoring and scheduling blocks. For the CNPG path that reads
as "no backups found" hours into a restore, because the archive is
resolved from the *new* instance's major version.

The restored spec is now the source's live spec with the submitted form
merged on top, in both modes so the two cannot drift. The base is
stripped of every `exclude_paths` prefix first: `_blank_excluded` only
clears the form's *initial*, and a form skips empty values, so a straight
merge would have handed the still-running source's FQDN to its own
replacement. `spec.parameters.restore` is stripped unconditionally, the
way `CrdModelFormMixin.HIDDEN_FIELDS` already treats it, so a restore of
a restore cannot inherit stale keys.

Also:

- Collect the `supporting_plan_*` inputs the page renders and prices.
  Without them `create_instance` fell back to the component's default
  plan, overwrote the size block with it and annotated it for billing, so
  the user was quoted one price and sold another.
- Move the virtual-model copy fix from `ServiceInstance.__getstate__` to
  `CRDModel.__reduce__`, where it protects every holder of a virtual
  instance rather than the one model that happens to hold one today. The
  update path is covered too: it is where the bug bites in production,
  and it only fires when a tracked field changed.
- Extract `ServiceInstanceUpdateView._deep_merge` to a module-level
  helper, now that two views need it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The restore spec is the source's spec with the submitted form merged onto
it, and a form skips empty values - so a user who deliberately emptied a
field got the source's old value handed back and clearing silently did
nothing. Half of "a full clone of the source, editable" was missing.

Any path the form exposes is now the form's to own, emptied included: it
is deleted from the merge base before the merge, so what the form does
not show stays cloned and what it shows is what was submitted. "Emptied"
is exactly what `get_nested_data` skips, so a meaningful `0`, `False` or
`[]` is left to the merge rather than deleted, and a disabled field -
expert mode's hidden fields, which fall back to their initial - is never
the user's to clear.

The field-to-path mapping now lives in one place, `get_field_path`, which
`get_nested_data` and the deletion both use, so the two cannot disagree
about a property name that contains dots.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A restore branches production data into a second instance, so it gets a
log entry of its own, written by the view on the success path: the entry
records the actor, the source by primary key *and* by name, and the
restore point. Name as well as key because deleting the source is the
documented end of a successful cutover, after which the foreign key is
null and this row is the only remaining record of what came from what.

The organization audit log renders it as "restored X from Y (point)"
rather than as a second, indistinguishable "created X": the instance's
own creation entry already says it was created and cannot say where its
contents came from.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`value is None or value == ""` was written out three times: in both
`get_nested_data` implementations and in the restore view's
`_cleared_paths`. The copies agreed, but nothing held them together -
narrowing one of them to match `prune_empty_data` left the whole suite
green while an emptied list quietly stopped clearing on the restore
form. `FormGeneratorMixin.is_unset` is now the single definition, the
way `get_field_path` is the single definition of where a field writes,
and a test empties the one array field the form both renders and lets
the user edit.

The audit entry gains what the design asked for and the first cut
dropped. The source's display name, because after a cutover the log is
the only record left and a resource name is not what an investigator
recognises; the resource name stays, since that is what the claim and
the annotation used. The restore mode, because `restore_point` does not
read on its own - empty means "the latest available state" in one mode
and cannot happen in the other, and the configuration that would say
which is staff-editable. And whether the actor was staff, as the
sibling reveal audit already records.

The rendered line now goes through gettext as a whole and says "latest
available state" where it means it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The backup summary card gains a Restore button, and the backup table a
"Restore from here" on every row a restore point can actually be picked
from - deep-linked as `?backup=` in snapshot mode and `?at=` in
point-in-time mode. Which groups those are is the rule
`_restore_source_groups` already owns, marked onto the groups the
fragment renders rather than repeated in the template: a row from a
dependent's namespace, or from a group the configuration does not name
as the snapshot source, deep-links a value the form would refuse.

Availability is handed the listing the fragment already made.
`get_backup_data` is not memoised, so deciding it independently would
cost a second listing - two to four more custom-object calls for a
service with dependents - on every summary render.

A member sees the button disabled with the reason. Hiding it instead
would read as "this service has no restore", which is a different and
wrong answer to a question they are entitled to ask.

The instance page now also shows where an instance came from and what
came from it. The source is linked while it exists and named in plain
text once it does not, which is the steady state rather than the edge
case: the foreign key is SET_NULL and deleting the source is the
documented end of a successful cutover. Both directions are filtered by
organization in the view. ServiceInstance is not under django-scopes, so
nothing filters the reverse relation at the ORM layer, and a foreign key
set wrongly in the admin would otherwise render another organization's
instance names and links onto the page. `clean()` rejects a
cross-organization source; this is the second layer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(restore): configure what the restore feature assumed
All checks were successful
Tests / test (push) Successful in 1m28s
Details
5d08a312cf 
The whole-feature review found six places where restore trusted a literal
or a convention instead of the configuration it is supposed to read.

Keycloak could neither be configured nor used. Its PostgreSQL is a
dependent service, so its definition carries `dependent_labels` and no
sources of its own: `validate_restore_config` refused to save it, and the
own-namespace rule filtered every backup it could restore from out of the
listing, leaving "the list of backups could not be read, please try
again" forever. The namespace rule belongs to snapshot mode, where a
backup name is a K8up Snapshot object resolved in the source claim's own
namespace; in point-in-time mode the rows only establish the window, and
for Keycloak that window is the dependent's.

`apply_restore_to_spec` now deletes the mode's path when there is no
restore point instead of leaving the source's. A restore of a restore
would otherwise recover to its predecessor's moment while the database,
the annotations, the audit entry and the detail card all said "latest
available state".

The configured paths are checked against the CRD schema before the form
is offered. The API server prunes unknown fields silently, so a typo in
`claim_name_path` produced a brand-new empty instance that everything
else described as a restore - and whose recommended next step is deleting
the instance that still holds the data. A schema that could not be
fetched is not a wrong configuration and does not refuse.

Which spec paths a restore owns is now derived from `claim_name_path` and
friends, in `restore_owned_paths`: the restore form excludes them, and
all three CRD form call sites hide and disable them. The
`spec.parameters.restore` entry in `HIDDEN_FIELDS` stays as the AppCat
default, which also covers CRDs that declare a restore block nobody
configured.

Also: `_restore_source_backups` had no production caller and is gone, its
tests now going through `get_restore_availability` as the view does; the
snapshot picker's unreachable empty-choices placeholder is gone, since
availability refuses that case first and says why; a failing
`audit_restore` logs instead of 500ing on a restore that already created
a billing instance; and "a backup's time" is one rule, `backup_time`,
shared by the listing order, the picker labels, the window's lower bound
and the row deep link - completion first, since that is the moment a
restore point means.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tobru force-pushed feat/self-service-restore from 5d08a312cf
All checks were successful
Tests / test (push) Successful in 1m28s
Details
to 557f16f158
All checks were successful
Tests / test (push) Successful in 1m30s
Details
2026-09-18 11:43:11 +00:00 Compare
fix(restore): show restore where it is still actionable
All checks were successful
Tests / test (push) Successful in 1m39s
Details
61298e0fd8 
The Backup Summary card offered a Restore button even when the restore
source held no backups: enabled with a warning in point-in-time mode,
disabled in snapshot mode. Neither reads as "there is nothing to restore
from yet" - which the card already says, two lines above. Hide the block
instead, and keep the disabled-with-a-reason button for the cases a user
can do something about, the member's "ask an organization admin" included.

Deciding that from `get_restore_availability` would have hidden it from
members too: that function reports no backups for every refusal, a
permission refusal among them. `mark_restore_sources` already walks the
groups the rule selects, so it answers the question instead.

Once the source instance is gone the cutover is over, and the "Restored
from" card is advice nobody can still act on. Demote it to Metadata, next
to Created At - the same kind of fact - and drop the next steps with it.
Metadata is server-rendered from the database, so provenance survives a
control plane the Backups accordion could not reach.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tobru changed title from WIP: Self-Service Restore into new Instance to Self-Service Restore into new Instance 2026-09-18 12:08:23 +00:00
tobru merged commit b590d9d11c into main 2026-09-18 12:08:42 +00:00
tobru deleted branch feat/self-service-restore 2026-09-18 12:08:42 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
API
Relates to API

  
Billing
Billing and invoicing

  
UI/UX
User interface and user experience

  
dependencies
Dependencies Update

  
bug
Something is not working

  
change
Change an existing feature

  
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
help wanted
Need some help

  
invalid
Something is wrong

  
question
More information is needed

  
wontfix
This won't be fixed

No labels
API
Billing
UI/UX
dependencies
bug
change
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
servala/servala-portal!678
No description provided.