restore: harden validation and improve targeting UX
This commit is contained in:
parent
98f1166685
commit
a9f97aab6b
@ -17,6 +17,7 @@ import (
|
||||
"scm.bstein.dev/bstein/soteria/internal/longhorn"
|
||||
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
k8svalidation "k8s.io/apimachinery/pkg/util/validation"
|
||||
)
|
||||
|
||||
type kubeClient interface {
|
||||
@ -308,24 +309,54 @@ func (s *Server) handleRestore(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, http.StatusBadRequest, fmt.Sprintf("invalid JSON: %v", err))
|
||||
return
|
||||
}
|
||||
if strings.TrimSpace(req.Namespace) == "" {
|
||||
req.Namespace = strings.TrimSpace(req.Namespace)
|
||||
req.PVC = strings.TrimSpace(req.PVC)
|
||||
req.TargetPVC = strings.TrimSpace(req.TargetPVC)
|
||||
req.TargetNamespace = strings.TrimSpace(req.TargetNamespace)
|
||||
|
||||
if req.Namespace == "" {
|
||||
s.metrics.RecordRestoreRequest(s.cfg.BackupDriver, "validation_error")
|
||||
writeError(w, http.StatusBadRequest, "namespace is required")
|
||||
return
|
||||
}
|
||||
if strings.TrimSpace(req.PVC) == "" {
|
||||
if req.PVC == "" {
|
||||
s.metrics.RecordRestoreRequest(s.cfg.BackupDriver, "validation_error")
|
||||
writeError(w, http.StatusBadRequest, "pvc is required")
|
||||
return
|
||||
}
|
||||
if strings.TrimSpace(req.TargetPVC) == "" {
|
||||
if req.TargetPVC == "" {
|
||||
s.metrics.RecordRestoreRequest(s.cfg.BackupDriver, "validation_error")
|
||||
writeError(w, http.StatusBadRequest, "target_pvc is required")
|
||||
return
|
||||
}
|
||||
if strings.TrimSpace(req.TargetNamespace) == "" {
|
||||
if req.TargetNamespace == "" {
|
||||
req.TargetNamespace = req.Namespace
|
||||
}
|
||||
if err := validateKubernetesName("namespace", req.Namespace); err != nil {
|
||||
s.metrics.RecordRestoreRequest(s.cfg.BackupDriver, "validation_error")
|
||||
writeError(w, http.StatusBadRequest, err.Error())
|
||||
return
|
||||
}
|
||||
if err := validateKubernetesName("pvc", req.PVC); err != nil {
|
||||
s.metrics.RecordRestoreRequest(s.cfg.BackupDriver, "validation_error")
|
||||
writeError(w, http.StatusBadRequest, err.Error())
|
||||
return
|
||||
}
|
||||
if err := validateKubernetesName("target_namespace", req.TargetNamespace); err != nil {
|
||||
s.metrics.RecordRestoreRequest(s.cfg.BackupDriver, "validation_error")
|
||||
writeError(w, http.StatusBadRequest, err.Error())
|
||||
return
|
||||
}
|
||||
if err := validateKubernetesName("target_pvc", req.TargetPVC); err != nil {
|
||||
s.metrics.RecordRestoreRequest(s.cfg.BackupDriver, "validation_error")
|
||||
writeError(w, http.StatusBadRequest, err.Error())
|
||||
return
|
||||
}
|
||||
if req.Namespace == req.TargetNamespace && req.PVC == req.TargetPVC {
|
||||
s.metrics.RecordRestoreRequest(s.cfg.BackupDriver, "conflict")
|
||||
writeError(w, http.StatusConflict, "target namespace/pvc must differ from source")
|
||||
return
|
||||
}
|
||||
|
||||
requester := currentRequester(r.Context())
|
||||
|
||||
@ -658,6 +689,13 @@ func splitGroups(raw string) []string {
|
||||
})
|
||||
}
|
||||
|
||||
func validateKubernetesName(field, value string) error {
|
||||
if errs := k8svalidation.IsDNS1123Label(value); len(errs) > 0 {
|
||||
return fmt.Errorf("%s must be a valid Kubernetes DNS-1123 label", field)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func buildBackupRecords(backups []longhorn.Backup) []api.BackupRecord {
|
||||
records := make([]api.BackupRecord, 0, len(backups))
|
||||
latestName := ""
|
||||
|
||||
@ -159,6 +159,44 @@ func TestRestoreRejectsExistingTargetPVC(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRestoreRejectsInvalidTargetNamespace(t *testing.T) {
|
||||
srv := &Server{
|
||||
cfg: &config.Config{AuthRequired: false, BackupDriver: "longhorn"},
|
||||
client: &fakeKubeClient{},
|
||||
longhorn: &fakeLonghornClient{},
|
||||
metrics: newTelemetry(),
|
||||
}
|
||||
srv.handler = http.HandlerFunc(srv.route)
|
||||
|
||||
body := `{"namespace":"apps","pvc":"data","target_namespace":"Apps_Ns","target_pvc":"restore-data","backup_url":"s3://bucket/backup"}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/v1/restores", strings.NewReader(body))
|
||||
res := httptest.NewRecorder()
|
||||
srv.Handler().ServeHTTP(res, req)
|
||||
|
||||
if res.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", res.Code, res.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRestoreRejectsSameSourceAndTarget(t *testing.T) {
|
||||
srv := &Server{
|
||||
cfg: &config.Config{AuthRequired: false, BackupDriver: "longhorn"},
|
||||
client: &fakeKubeClient{},
|
||||
longhorn: &fakeLonghornClient{},
|
||||
metrics: newTelemetry(),
|
||||
}
|
||||
srv.handler = http.HandlerFunc(srv.route)
|
||||
|
||||
body := `{"namespace":"apps","pvc":"data","target_namespace":"apps","target_pvc":"data","backup_url":"s3://bucket/backup"}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/v1/restores", strings.NewReader(body))
|
||||
res := httptest.NewRecorder()
|
||||
srv.Handler().ServeHTTP(res, req)
|
||||
|
||||
if res.Code != http.StatusConflict {
|
||||
t.Fatalf("expected 409, got %d: %s", res.Code, res.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestMetricsStayPublic(t *testing.T) {
|
||||
srv := &Server{
|
||||
cfg: &config.Config{AuthRequired: true, AllowedGroups: []string{"admin"}},
|
||||
|
||||
@ -176,6 +176,7 @@
|
||||
const generatedAtEl = document.getElementById('generated-at');
|
||||
const authPillEl = document.getElementById('auth-pill');
|
||||
const refreshBtn = document.getElementById('refresh-btn');
|
||||
let latestInventory = null;
|
||||
|
||||
function escapeHtml(value) {
|
||||
return String(value || '')
|
||||
@ -190,6 +191,25 @@
|
||||
resultEl.textContent = typeof payload === 'string' ? payload : JSON.stringify(payload, null, 2);
|
||||
}
|
||||
|
||||
function suggestTargetPVCName(sourcePVC) {
|
||||
const now = new Date();
|
||||
const pad = (value) => String(value).padStart(2, '0');
|
||||
const suffix = [
|
||||
now.getUTCFullYear(),
|
||||
pad(now.getUTCMonth() + 1),
|
||||
pad(now.getUTCDate()),
|
||||
pad(now.getUTCHours()),
|
||||
pad(now.getUTCMinutes())
|
||||
].join('');
|
||||
const normalized = ('restore-' + sourcePVC + '-' + suffix)
|
||||
.toLowerCase()
|
||||
.replace(/[^a-z0-9-]/g, '-')
|
||||
.replace(/-+/g, '-')
|
||||
.replace(/^-|-$/g, '');
|
||||
const trimmed = normalized.length <= 63 ? normalized : normalized.slice(0, 63).replace(/-+$/g, '');
|
||||
return trimmed || 'restore-' + suffix;
|
||||
}
|
||||
|
||||
async function fetchJSON(url, options) {
|
||||
const response = await fetch(url, options);
|
||||
const text = await response.text();
|
||||
@ -234,6 +254,9 @@
|
||||
detailsEl.innerHTML = '<p class="muted">Loading backups...</p>';
|
||||
try {
|
||||
const payload = await fetchJSON('/v1/backups?namespace=' + encodeURIComponent(namespace) + '&pvc=' + encodeURIComponent(pvc));
|
||||
const namespaceOptions = (latestInventory && latestInventory.namespaces ? latestInventory.namespaces : [])
|
||||
.map((group) => '<option value="' + escapeHtml(group.name) + '"></option>')
|
||||
.join('');
|
||||
const options = payload.backups
|
||||
.filter((backup) => backup.state === 'Completed')
|
||||
.map((backup) => '<option value="' + escapeHtml(backup.url) + '">' + escapeHtml(backup.name) + ' | ' + escapeHtml(backup.created || 'unknown time') + '</option>')
|
||||
@ -242,14 +265,18 @@
|
||||
'<div class="stack">',
|
||||
'<div><h3 style="margin-bottom: 6px;">' + escapeHtml(namespace) + '/' + escapeHtml(pvc) + '</h3><p class="muted" style="margin-top: 0;">Source volume: ' + escapeHtml(payload.volume) + '</p></div>',
|
||||
'<label>Backup snapshot<select id="restore-backup">' + (options || '<option value="">No completed backups available</option>') + '</select></label>',
|
||||
'<label>Target namespace<input id="restore-namespace" value="' + escapeHtml(namespace) + '"></label>',
|
||||
'<label>Target PVC name<input id="restore-pvc" value="restore-' + escapeHtml(pvc) + '"></label>',
|
||||
'<label>Target namespace<input id="restore-namespace" list="restore-namespace-options" value="' + escapeHtml(namespace) + '"></label>',
|
||||
'<datalist id="restore-namespace-options">' + namespaceOptions + '</datalist>',
|
||||
'<label>Target PVC name<input id="restore-pvc" value="' + escapeHtml(suggestTargetPVCName(pvc)) + '"></label>',
|
||||
'<p class="muted">Tip: keep target PVC unique for each restore drill to avoid conflicts.</p>',
|
||||
'<div class="actions">',
|
||||
'<button id="restore-submit"' + (options ? '' : ' disabled') + '>Create restore PVC</button>',
|
||||
'<button id="restore-dry-run" class="secondary"' + (options ? '' : ' disabled') + '>Dry run restore</button>',
|
||||
'<button id="restore-view" class="secondary">Show backup JSON</button>',
|
||||
'</div></div>'
|
||||
].join('');
|
||||
document.getElementById('restore-submit').onclick = async () => {
|
||||
|
||||
const runRestore = async (dryRun) => {
|
||||
const backupURL = document.getElementById('restore-backup').value;
|
||||
const targetNamespace = document.getElementById('restore-namespace').value.trim();
|
||||
const targetPVC = document.getElementById('restore-pvc').value.trim();
|
||||
@ -263,15 +290,17 @@
|
||||
backup_url: backupURL,
|
||||
target_namespace: targetNamespace,
|
||||
target_pvc: targetPVC,
|
||||
dry_run: false
|
||||
dry_run: dryRun
|
||||
})
|
||||
});
|
||||
showResult(result);
|
||||
await loadInventory();
|
||||
} catch (error) {
|
||||
showResult({ error: error.message, namespace, pvc, target_namespace: targetNamespace, target_pvc: targetPVC });
|
||||
showResult({ error: error.message, namespace, pvc, target_namespace: targetNamespace, target_pvc: targetPVC, dry_run: dryRun });
|
||||
}
|
||||
};
|
||||
document.getElementById('restore-submit').onclick = () => runRestore(false);
|
||||
document.getElementById('restore-dry-run').onclick = () => runRestore(true);
|
||||
document.getElementById('restore-view').onclick = () => showResult(payload);
|
||||
} catch (error) {
|
||||
detailsEl.innerHTML = '<p class="error">' + escapeHtml(error.message) + '</p>';
|
||||
@ -279,6 +308,7 @@
|
||||
}
|
||||
|
||||
function renderInventory(payload) {
|
||||
latestInventory = payload;
|
||||
generatedAtEl.textContent = payload.generated_at ? 'Updated ' + payload.generated_at : '';
|
||||
if (!payload.namespaces || payload.namespaces.length === 0) {
|
||||
inventoryEl.innerHTML = '<p class="muted">No bound PVCs found.</p>';
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user