# Design: CRRF Backup/Restore — Full coreCli Elimination via `GetAnyDedicatedClusterBasicByID`

**Date:** 2026-05-29  
**PR:** #3415 (`fix/crrf-backup-list-mgmt-basic-view`)  
**Author:** ideascf  
**Status:** ✅ Implemented (all callsites migrated, tests passing, lint clean)

---

## Background

CRRF (Cross-Region Replication based Failover) requires that backup listing, PITR time-range queries,
and restore initiation remain functional when the upstream cluster's regional control-plane is
unavailable. Diagnosis revealed that several backup/restore code paths call `coreCli` (which routes
to the regional K8s API), causing silent failures during a regional outage.

PR #3415 introduced `MgmtSvcGrpcClient.GetAnyDedicatedClusterBasicByID` — a `CLUSTER_VIEW_BASIC` variant
that is served entirely from the tidb-management-service database, bypassing the regional control-plane.
The PR fixes `checkClusterPermission` and `validateClusterAccess`, but several `coreCli` callsites
remain uncovered.

This spec extends PR #3415 to achieve complete elimination of `coreCli` from backup/restore
cluster-lookup paths.

---

## Root Cause Summary

| Symptom | Root Cause |
|---------|-----------|
| Backup list empty during CRRF | `checkClusterPermission` used `coreCli` → now fixed in PR #3415 |
| PITR time range empty during CRRF | `canClusterVersionEnableLogBackup` uses `coreCli.GetAvailableClusterByID` — silently returns `false` on error |
| Restore fail during CRRF | `inner/backup.go:Restore` uses `coreCli.GetAvailableClusterByID` on target cluster |
| List restore jobs broken | `open/restore.go` uses `coreCli` for cluster info enrichment |

---

## Decision

Use `mgmtSvcGrpcClient.GetAnyDedicatedClusterBasicByID` (and `mgmtSvcGrpcClient.ListClusters` with
`CLUSTER_VIEW_BASIC`) for all cluster lookups in backup/restore code paths.

**Rationale:** `MgmtSvcGrpcClient` goes through tidb-management-service, which acts as the authoritative
global cluster registry and applies proper access-control and business logic. Bypassing it (e.g., calling
`dedicated-global-service` directly) would skip that layer and is not approved by the owning team.

---

## Scope

### All Fixed in PR #3415

| File | Function | Change |
|------|----------|--------|
| `console/backup.go` | `checkClusterPermission` | `coreCli` → `mgmtCli.GetAnyDedicatedClusterBasicByID` |
| `console/backup.go` | `recordAuditLogForSnapshotRestore` | `coreCli` → `mgmtCli.GetAnyDedicatedClusterBasicByID` |
| `console/backup.go` | `doBillingActionCheckForBackupDeletion` | `coreCli` → `mgmtCli.GetAnyDedicatedClusterBasicByID` |
| `console/backup.go` | `canClusterVersionEnableLogBackup` | `coreCli.GetAvailableClusterByID` → `mgmtCli.GetDedicatedClusterBasicByID` |
| `v1beta1/backup.go` | `validateClusterAccess` | `coreCli` → `mgmtCli.GetAnyDedicatedClusterBasicByID` |
| `v1beta1/backup.go` | `validateClusterAccessForRestore` | `coreCli` → `mgmtCli.GetAnyDedicatedClusterBasicByID` |
| `v1beta1/backup.go` | `recordDeleteBackupAuditLog` | `coreCli` → `mgmtCli.GetAnyDedicatedClusterBasicByID` |
| `v1beta1/backup.go` | `validateClusterAccessForModifyBackupSetting` | `coreCli` → `mgmtCli.GetAnyDedicatedClusterBasicByID` |
| `inner/backup.go` | `Restore` | `coreCli.GetAvailableClusterByID` → `mgmtCli.GetDedicatedClusterBasicByID` |
| `inner/backup.go` | `DeleteAllManualBackups` | `coreCli.ListClusters` → `mgmtCli.ListClusters` with `CLUSTER_VIEW_BASIC` |
| `open/restore.go` | `convertGetRestoreResp` / `GetRestore` / `GetRestoreV2` | `coreCli.GetAnyClusterByID` → `mgmtCli.GetAnyDedicatedClusterBasicByID` |
| `open/restore.go` | `CreateRestore` | `coreCli.GetAnyClusterByID` → `mgmtCli.GetAnyDedicatedClusterBasicByID` |
| `open/restore.go` | `queryRestoringClusters` | `coreCli.ListClusters` → `mgmtCli.ListClusters` with `CLUSTER_VIEW_BASIC` |
| `open/cvt.go` | — | Added `DedicatedClusterState2OpenAPIList` conversion helper |
| `models/backup/impl/snapshot_backup.go` | `checkCluster` | Added `DeployType == DEPLOY_TYPE_DEDICATED` guard |

### Out of Scope

- Serverless cluster paths — not affected by CRRF.
- Non-backup controllers (changefeed, interceptor) — separate concern.
- `backupv2.go` audit log path — serverless path, out of scope.

---

## Interface Changes

### `MgmtSvcGrpcClient` — Methods (implemented)

Two methods were added; existing `GetAnyClusterBasicByID` / `GetClusterBasicByID` (introduced early in PR #3415) were renamed for clarity:

| Method | Semantics | Used by |
|--------|-----------|---------|
| `GetAnyDedicatedClusterBasicByID` | Returns any-state cluster, BASIC view | Permission checks, audit logs, restore enrichment |
| `GetDedicatedClusterBasicByID` | Returns non-deleted cluster only, BASIC view | `canClusterVersionEnableLogBackup`, `inner/backup.go:Restore` |

> **Naming:** The `Dedicated` prefix was added (vs the original `GetAnyClusterBasicByID` / `GetClusterBasicByID`) to make it explicit that these methods only serve Dedicated (non-serverless) clusters, matching the naming convention used elsewhere in the codebase.

`ListClusters` is already on the interface. `inner/backup.go:DeleteAllManualBackups` and `open/restore.go:queryRestoringClusters` use it directly with `CLUSTER_VIEW_BASIC` in the filter.

### `snapshot_backup.go:checkCluster` — `DeployType` guard

Added an explicit `DeployType == DEPLOY_TYPE_DEDICATED` check after fetching the cluster. This prevents dedicated-only backup logic from running on serverless clusters if a serverless cluster ID is somehow passed in. A matching test fix was needed: `snapshot_backup_test.go` mocked cluster now includes `DeployType: mgmt.DeployType_DEPLOY_TYPE_DEDICATED`.

---

## Field Access Mapping

When replacing `*core.Cluster` with `*mgmt.Cluster` returned by `GetAnyDedicatedClusterBasicByID`:

| `core.Cluster` field | `mgmt.Cluster` equivalent | Available in BASIC view |
|---------------------|--------------------------|------------------------|
| `cluster.Name` | `cluster.GetClusterName()` | ✅ Yes |
| `cluster.RegionName` | `cluster.GetRegion()` | ✅ Yes |
| `cluster.Version` | `cluster.GetDedicatedCluster().GetVersion()` | ✅ Yes |
| `cluster.Status == DELETED` | `cluster.GetDedicatedCluster().GetState() == Cluster_DELETED` | ✅ Yes |
| `cluster.Status == DELETECOMPLETE` | No direct equivalent | `TODO(ideascf): pr #3415` |
| `grpcli.IsClusterDeleted(cluster.Status)` | `cluster.GetDedicatedCluster().GetState() == Cluster_DELETED` | ✅ Yes |

> **DELETECOMPLETE gap**: `core.ClusterStatus_DELETECOMPLETE` has no equivalent in `mgmt.Cluster`.
> In `DeleteAllManualBackups`, the original filter was `[DELETED, DELETECOMPLETE]`. The replacement
> filters only by the `DELETED` state equivalent. Mark with `TODO(ideascf): pr #3415` and confirm
> with the cluster-service team whether DELETECOMPLETE clusters appear as DELETED in mgmt view.

---

## Structural Changes

### `inner/backup.go` — Add `mgmtCli`

`BackupInnerController` currently has no `mgmtCli` field. It must be added:

```go
type BackupInnerController struct {
    // ...existing fields...
    mgmtCli pkggrpcli.MgmtSvcGrpcClient  // added
}
```

Constructor updated to accept `mgmtCli pkggrpcli.MgmtSvcGrpcClient`, wired from `container.go`.

After the change, `coreCli` may be removable from `BackupInnerController` if no other usages remain
(verify during implementation).

### `open/restore.go` — Replace `coreCli` with `mgmtCli`

`restoreControllerImpl.coreCli` is used only for cluster lookups in this file. Replace with
`mgmtCli grpcli.MgmtSvcGrpcClient`. Remove `coreCli` field if no other usages remain.

### `console/backup.go` — `mgmtCli` already present (PR #3415)

No structural change needed; `canClusterVersionEnableLogBackup` just gains a new `mgmtCli` call.

---

## Error Handling

`GetAvailableClusterByID` differs from `GetAnyDedicatedClusterBasicByID` in one key way: the former only
returns available (non-deleted) clusters. For `canClusterVersionEnableLogBackup`, the caller
(`ListPiTRTimeRanges`) already holds a valid, accessible cluster ID — so using `GetAnyDedicatedClusterBasicByID`
is safe. On error, the existing behavior (log + return `false`) is preserved.

For `inner/backup.go:Restore`, the target cluster must exist and be in AVAILABLE state. Using
`GetAnyDedicatedClusterBasicByID` (which returns any state) means the caller should not rely on the method
to enforce state; the state check already happens upstream in the restore workflow.

---

## Testing

- All unit tests updated to use `*mgmt.Cluster` mocks.
- `snapshot_backup_test.go`: mocked cluster required `DeployType: DEPLOY_TYPE_DEDICATED` due to the new guard in `checkCluster` — this was a test-only gap, not a production issue.
- Integration/E2E: covered by existing CRRF integration tests.

---

## Known Gaps / TODOs

| Location | TODO | Owner |
|----------|------|-------|
| `inner/backup.go:DeleteAllManualBackups` | `mgmt.ClusterFilter` has no `DELETECOMPLETE` equivalent; `Deleted:true` covers `DELETED` state only | Confirm with cluster-service team whether DELETECOMPLETE clusters appear as DELETED in mgmt view |