Address code review findings for backup verification

Fix 4 critical issues:
- Add filter/priority/deleteMarkerReplication to S3 CRR rule (deploy would fail without)
- Add stack dependency so replica deploys before main stack
- Fix DB file extension matching (.sqlite3/.sql instead of .db)
- Replace nonexistent `forgejo restore` command with actual restore steps in README

Fix 4 moderate issues:
- Add timeout=10 to Slack webhook urlopen call
- Add filter='data' to tarfile.extract for PEP 706 compliance
- Add explicit ValueError for unknown handler mode
- Use date-scoped S3/GCS prefix instead of unbounded listing
This commit is contained in:
Adam Moussa 2026-05-13 18:29:28 -04:00
parent 17179c84b2
commit fa51085578
4 changed files with 46 additions and 21 deletions

View file

@ -68,18 +68,23 @@ sudo /usr/local/bin/forgejo-backup.sh
```bash
aws s3 cp s3://forgejo-backups-328440206208/archive/<date>/forgejo-<date>.tar.gz /tmp/
systemctl stop forgejo
cd /tmp && tar xzf forgejo-<date>.tar.gz
forgejo restore --config /etc/forgejo/app.ini --from /tmp/forgejo-dump-*
chown -R forgejo:forgejo /var/lib/forgejo
mkdir -p /tmp/forgejo-restore && tar -xzf /tmp/forgejo-<date>.tar.gz -C /tmp/forgejo-restore
cd /tmp/forgejo-restore
cp app.ini /etc/forgejo/app.ini
cp gitea-db.sqlite3 /var/lib/forgejo/data/forgejo.db
rm -rf /var/lib/forgejo/data/repositories
cp -a repos /var/lib/forgejo/data/repositories
chown -R forgejo:forgejo /var/lib/forgejo /etc/forgejo/app.ini
systemctl start forgejo
rm -rf /tmp/forgejo-restore /tmp/forgejo-<date>.tar.gz
```
### Restore from GCS (disaster recovery)
```bash
gcloud config set project seahaven-backups
gcloud config set project sea-haven-backups
gsutil cp gs://forgejo-backups-offsite-seahaven/archive/<date>/forgejo-<date>.tar.gz /tmp/
# Then follow the same restore steps as S3
# Then follow the same restore steps as S3 above
```
To test the backup manually:
@ -167,7 +172,7 @@ Run the setup script to create the GCS offsite bucket, service account, and stor
./scripts/gcp-setup.sh
```
This creates the `seahaven-backups` GCP project with a locked-retention GCS bucket. After running, configure the Storage Transfer job in the GCP Console using the AWS credentials from `forgejo/gcs-transfer-credentials`.
This creates the `sea-haven-backups` GCP project with a locked-retention GCS bucket. After running, configure the Storage Transfer job in the GCP Console using the AWS credentials from `forgejo/gcs-transfer-credentials`.
## Deployment

View file

@ -6,12 +6,14 @@ import { ForgejoReplicaStack } from "../lib/forgejo-replica-stack";
const app = new cdk.App();
new ForgejoReplicaStack(app, "forgejo-replica", {
const replicaStack = new ForgejoReplicaStack(app, "forgejo-replica", {
stackName: "forgejo-replica",
env: { account: "328440206208", region: "us-west-2" },
});
new ForgejoStack(app, "forgejo", {
const forgejoStack = new ForgejoStack(app, "forgejo", {
stackName: "forgejo",
env: { account: "328440206208", region: "us-east-1" },
});
forgejoStack.addDependency(replicaStack);

View file

@ -38,10 +38,14 @@ def _check_s3_bucket(client, bucket, label):
now = datetime.now(timezone.utc)
cutoff = now - timedelta(hours=48)
try:
resp = client.list_objects_v2(Bucket=bucket, Prefix="archive/", MaxKeys=1000)
contents = resp.get("Contents", [])
today = now.strftime("%Y-%m-%d")
yesterday = (now - timedelta(days=1)).strftime("%Y-%m-%d")
contents = []
for date_prefix in [today, yesterday]:
resp = client.list_objects_v2(Bucket=bucket, Prefix=f"archive/{date_prefix}/")
contents.extend(resp.get("Contents", []))
if not contents:
return False, f"{label}: No objects found under archive/"
return False, f"{label}: No objects found under archive/ for last 2 days"
latest = max(contents, key=lambda o: o["LastModified"])
if latest["LastModified"] < cutoff:
age = (now - latest["LastModified"]).total_seconds() / 3600
@ -57,10 +61,14 @@ def _check_gcs():
try:
client = _get_gcs_client()
bucket = client.bucket(GCS_BUCKET)
blobs = list(bucket.list_blobs(prefix="archive/", max_results=1000))
if not blobs:
return False, "GCS Offsite: No objects found under archive/"
now = datetime.now(timezone.utc)
today = now.strftime("%Y-%m-%d")
yesterday = (now - timedelta(days=1)).strftime("%Y-%m-%d")
blobs = []
for date_prefix in [today, yesterday]:
blobs.extend(list(bucket.list_blobs(prefix=f"archive/{date_prefix}/")))
if not blobs:
return False, "GCS Offsite: No objects found under archive/ for last 2 days"
cutoff = now - timedelta(hours=72)
latest = max(blobs, key=lambda b: b.updated)
if latest.updated < cutoff:
@ -95,10 +103,14 @@ def _check_ebs_snapshots():
def _restore_test():
results = []
try:
resp = s3.list_objects_v2(Bucket=SOURCE_BUCKET, Prefix="archive/", MaxKeys=1000)
contents = resp.get("Contents", [])
now = datetime.now(timezone.utc)
contents = []
for days_ago in range(7):
date_prefix = (now - timedelta(days=days_ago)).strftime("%Y-%m-%d")
resp = s3.list_objects_v2(Bucket=SOURCE_BUCKET, Prefix=f"archive/{date_prefix}/")
contents.extend(resp.get("Contents", []))
if not contents:
return [{"pass": False, "msg": "Restore test: No dumps found in source bucket"}]
return [{"pass": False, "msg": "Restore test: No dumps found in source bucket (last 7 days)"}]
latest = max(contents, key=lambda o: o["LastModified"])
results.append({"pass": True, "msg": f"Restore test: Using {latest['Key']} ({latest['Size'] / 1_000_000:.1f} MB)"})
@ -112,10 +124,10 @@ def _restore_test():
names = tf.getnames()
results.append({"pass": True, "msg": f"Restore test: Archive OK — {len(names)} entries"})
db_entries = [n for n in names if n.endswith(".db") or n.endswith("forgejo.db")]
db_entries = [n for n in names if n.endswith(".sqlite3") or n.endswith(".sql")]
if db_entries:
import sqlite3 as sqlite_mod
tf.extract(db_entries[0], path=tmpdir)
tf.extract(db_entries[0], path=tmpdir, filter="data")
db_path = os.path.join(tmpdir, db_entries[0])
conn = sqlite_mod.connect(db_path)
result = conn.execute("PRAGMA integrity_check").fetchone()
@ -125,7 +137,7 @@ def _restore_test():
else:
results.append({"pass": False, "msg": f"Restore test: SQLite integrity FAILED — {result[0]}"})
else:
results.append({"pass": True, "msg": "Restore test: No .db file found in archive (may use different format)"})
results.append({"pass": False, "msg": "Restore test: No SQLite DB file found in archive"})
except tarfile.TarError as e:
results.append({"pass": False, "msg": f"Restore test: Archive extraction FAILED — {e}"})
except Exception as e:
@ -143,7 +155,7 @@ def _post_slack(blocks):
headers={"Content-Type": "application/json"},
method="POST",
)
urllib.request.urlopen(req)
urllib.request.urlopen(req, timeout=10)
def handler(event, context):
@ -186,6 +198,9 @@ def handler(event, context):
overall = ":white_check_mark: Restore test passed" if all_pass else ":rotating_light: Restore test failed"
blocks.append({"type": "section", "text": {"type": "mrkdwn", "text": f"*Overall:* {overall}"}})
else:
raise ValueError(f"Unknown mode: {mode!r} (expected 'daily' or 'restore-test')")
_post_slack(blocks)
return {

View file

@ -122,6 +122,9 @@ export class ForgejoStack extends cdk.Stack {
rules: [{
id: "replicate-to-west",
status: "Enabled",
priority: 1,
filter: { prefix: "" },
deleteMarkerReplication: { status: "Disabled" },
destination: {
bucket: replicaBucketArn,
storageClass: "STANDARD",