ahmido
2bf5de4663
Summary Consolidates the “Tenant Operate Hub” work (Spec 085) and the follow-up adjustments from the 086 session merge into a single branch ready to merge into dev. Primary focus: stabilize Ops/Operate Hub UX flows, tighten/align authorization semantics, and make the full Sail test suite green. Key Changes Ops UX / Verification Readonly members can view verification operation runs (reports) while starting verification remains restricted. Normalized failure reason-code handling and aligned UX expectations with the provider reason-code taxonomy. Onboarding wizard UX “Start verification” CTA is hidden while a verification run is active; “Refresh” is shown during in-progress runs. Treats provider_permission_denied as a blocking reason (while keeping legacy compatibility). Test + fixture hardening Standardized use of default provider connection fixtures in tests where sync/restore flows require it. Fixed multiple Filament URL/tenant-context test cases to avoid 404s and reduce tenancy routing brittleness. Policy sync / restore safety Enrollment configuration type collision classification tests now exercise the real sync path (with required provider connection present). Restore edge-case safety tests updated to reflect current provider-connection requirements. Testing vendor/bin/sail artisan test --compact (green) vendor/bin/sail bin pint --dirty (green) Notes Includes merged 086 session work already (no separate PR needed). Co-authored-by: Ahmed Darrazi <ahmeddarrazi@ebc83aaa-d947-4a08-b88e-bd72ac9645f7.fritz.box> Co-authored-by: Ahmed Darrazi <ahmeddarrazi@MacBookPro.fritz.box> Co-authored-by: Ahmed Darrazi <ahmeddarrazi@adsmac.fritz.box> Reviewed-on: #103
88 lines
6.9 KiB
Markdown
88 lines
6.9 KiB
Markdown
# Research (Spec 086)
|
|
|
|
This document resolves the unknowns needed to write an implementation plan for “Retire Legacy Runs Into Operation Runs”. It is based on repository inspection (no new external dependencies).
|
|
|
|
## Decisions
|
|
|
|
### 1) Canonical run viewer is already implemented; keep the route shape
|
|
|
|
- **Decision:** Use the existing tenantless canonical viewer route `admin.operations.view` (path pattern `/admin/operations/{run}`) implemented by the Filament page `TenantlessOperationRunViewer`.
|
|
- **Rationale:** This already enforces “tenantless deep link” while still doing workspace / tenant entitlement checks server-side through `Gate::authorize('view', $run)`.
|
|
- **Alternatives considered:** Create a second viewer page or route. Rejected because it would introduce duplicate UX and increase the chance of policy drift.
|
|
|
|
Repository anchors:
|
|
- Canonical viewer page: `app/Filament/Pages/Operations/TenantlessOperationRunViewer.php`
|
|
- Link helper: `app/Support/OperationRunLinks.php`
|
|
- Workspace selection middleware explicitly treats `/admin/operations/{id}` as workspace-optional: `app/Http/Middleware/EnsureWorkspaceSelected.php`
|
|
|
|
### 2) OperationRun is persisted and DB-rendered; schema supports workspace-tenant and workspace-only runs
|
|
|
|
- **Decision:** Treat `operation_runs` as the canonical persistence format for status/progress/results.
|
|
- **Rationale:** The schema already includes `workspace_id` (required) and `tenant_id` (nullable), enabling both tenant-plane and workspace-plane operations.
|
|
- **Alternatives considered:** Separate tables per operation family. Rejected because it breaks the Monitoring → Operations single source of truth principle.
|
|
|
|
Repository anchors:
|
|
- Migrations: `database/migrations/2026_01_16_180642_create_operation_runs_table.php`, `database/migrations/2026_02_04_090030_add_workspace_id_to_operation_runs_table.php`
|
|
- Model: `app/Models/OperationRun.php`
|
|
|
|
### 3) View authorization must be capability-gated per operation type (in addition to membership)
|
|
|
|
- **Decision:** Extend run viewing authorization to require the same capability used to start the operation type.
|
|
- **Rationale:** Spec 086 clarifications require: non-members get 404; members without capability get 403; and the “view” capability equals the “start” capability.
|
|
- **Implementation approach (planned):** Update `OperationRunPolicy::view()` to:
|
|
1) Keep existing workspace membership and tenant entitlement checks (deny-as-not-found).
|
|
2) Resolve required capability from `OperationRun->type` using a centralized mapping helper.
|
|
3) If capability is known and tenant-scoped, enforce `403` when the member lacks it.
|
|
|
|
Repository anchors:
|
|
- Current policy (membership + tenant entitlement only): `app/Policies/OperationRunPolicy.php`
|
|
- Existing capability enforcement in start surfaces (examples):
|
|
- Inventory sync start: `Capabilities::TENANT_INVENTORY_SYNC_RUN` in `app/Filament/Resources/InventoryItemResource/Pages/ListInventoryItems.php`
|
|
- Directory groups sync start: `Capabilities::TENANT_SYNC` in `app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php`
|
|
- Backup schedule run/retry: `Capabilities::TENANT_BACKUP_SCHEDULES_RUN` in `app/Filament/Resources/BackupScheduleResource.php`
|
|
|
|
### 4) Run identity / dedupe strategy varies by operation type
|
|
|
|
- **Decision:** Use existing `OperationRunService` helpers but apply type-specific identity rules:
|
|
- `inventory.sync` and `directory_groups.sync`: **while-active dedupe** based on deterministic inputs (continue using `ensureRun(...)`-style identity).
|
|
- `backup_schedule.run_now` and `backup_schedule.retry`: **unique per click** (no dedupe). Create a new run each time by including a nonce in identity inputs (e.g., UUID).
|
|
- `backup_schedule.scheduled`: **strict dedupe** per `(backup_schedule_id, scheduled_for)`; create a new operation type `backup_schedule.scheduled` and use `ensureRunWithIdentity(...)` keyed by schedule + intended fire-time.
|
|
- **Rationale:** Matches explicit spec clarifications and protects against scheduler double-fire.
|
|
- **Alternatives considered:**
|
|
- Keep using `ensureRun(...)` for manual runs → rejected (dedupes while active).
|
|
- Use legacy table unique constraints as idempotency → rejected (spec requires OperationRun is canonical).
|
|
|
|
Repository anchors:
|
|
- `ensureRun(...)` and `ensureRunWithIdentity(...)`: `app/Services/OperationRunService.php`
|
|
- Existing partial unique index for active runs: `operation_runs_active_unique_*` in the migrations above.
|
|
|
|
### 5) Legacy run tables are real and currently written to; deterministic redirect requires an explicit mapping field
|
|
|
|
- **Decision:** Legacy tables remain viewable and read-only, but should not be relied on for current execution tracking.
|
|
- **Rationale:** Spec requires “no new legacy rows” for in-scope operations. Today, some start surfaces still create legacy rows (e.g., inventory/group sync, backup schedule runs).
|
|
- **Planned design:**
|
|
- Stop creating new legacy rows as part of the cutover PRs.
|
|
- Implement legacy “view” redirect behavior only when a record has a canonical mapping.
|
|
- To make redirects deterministic without a backfill, add an optional `operation_run_id` FK column to legacy tables that we intend to redirect (only populated for rows created after the migration; older rows remain legacy-only view).
|
|
- **Alternatives considered:** Derive mapping by recomputing hashes and searching by time window. Rejected as non-deterministic and likely to pick the wrong run when identities collide historically.
|
|
|
|
Repository anchors (legacy tables):
|
|
- Inventory sync runs: `database/migrations/2026_01_07_142719_create_inventory_sync_runs_table.php`
|
|
- Directory group sync runs: `database/migrations/2026_01_11_120004_create_entra_group_sync_runs_table.php`
|
|
- Backup schedule runs: `database/migrations/**create_backup_schedule_runs**` (used in `BackupScheduleResource`)
|
|
- Restore runs (domain): `database/migrations/2025_12_10_000150_create_restore_runs_table.php`
|
|
|
|
### 6) DB-only rendering constraint is already enforced in Monitoring pages, but Tenant configuration forms still call Graph
|
|
|
|
- **Decision:** Remove outbound Graph calls from configuration-form search/labels by introducing cached directory role definitions and using cached directory groups.
|
|
- **Rationale:** Constitution OPS-EX-AUTH-001 + Spec 086 FR-006/FR-015 require render/search/label resolution to be DB-only.
|
|
- **Repository finding:** `TenantResource` currently queries Graph for role definitions in selector callbacks.
|
|
|
|
Repository anchors:
|
|
- Graph call sites inside UI callbacks: `app/Filament/Resources/TenantResource.php` (roleDefinitions search/label methods)
|
|
|
|
## Open items (resolved enough for planning)
|
|
|
|
- Exact schema for the new role definition cache tables and the sync job contract will be specified in `data-model.md` and implemented in Phase PR(s).
|
|
- The capability mapping for run viewing will be implemented via a centralized helper; the plan will enumerate required capabilities per in-scope operation type.
|