ops: un deploiement qui refuse de partir casse, et qui se verifie
OPS-011, en partie. Ce que le workflow garantit maintenant : - rien ne part d un arbre casse. La suite, ruff check et ruff format tournent sur le runner de deploiement avant tout envoi. Une CI verte sur GitHub ne prouve rien ici : le deploiement se declenche a la main, sur ce que la branche contient a cet instant ; - seuls les fichiers nommes partent. La charge est une liste blanche — app/, wsgi.py, requirements.txt — et non l arbre de travail moins neuf exclusions. C est par cette porte que clear_db.py, la suite de tests et les definitions de CI se sont retrouves sur le noeud de production ; - le deploiement est verifie. /health est interroge pendant deux minutes apres l envoi et le job echoue s il ne repond jamais « healthy ». Avant, un arbre a moitie televerse etait un deploiement vert. Ce qui n est pas garanti, et c est ecrit dans le fichier : la bascule n est pas atomique. Le miroir se fait sur place, donc pendant le transfert la production execute un melange de deux versions. En cherchant a fermer ce point, un defaut a part entiere est apparu. Les contrats etaient ranges a os.getcwd()/documents et leur chemin absolu ecrit en base. La racine de stockage suivait donc le repertoire depuis lequel le processus avait ete lance : redemarrer le serveur ailleurs envoie les nouveaux contrats dans un nouvel arbre et rend les anciens illisibles — la base continuant d affirmer qu ils sont la, la panne se manifeste par un 500 au telechargement, pas par quelque chose d actionnable. app/storage.py fixe la racine et DOCUMENTS_ROOT la deplace. Les nouvelles lignes gardent un chemin relatif, les anciennes gardent leur chemin absolu et continuent de resoudre : aucune migration de donnees n est necessaire, donc ce changement n attend pas Alembic. C etait aussi le troisieme pre-requis de la bascule par repertoires de version. Les deux autres sont hors d atteinte d ici — la commande de demarrage Pterodactyl doit pointer sur current/, et les repertoires partages doivent etre installes sur le noeud. Les deux sont decrits dans docs/deployment.md, avec la procedure de retour arriere qui manquait. 511 tests.
This commit is contained in:
@@ -6,6 +6,34 @@ on:
|
|||||||
# branches:
|
# branches:
|
||||||
# - main # Optional: Run automatically on pushes to the main branch
|
# - main # Optional: Run automatically on pushes to the main branch
|
||||||
|
|
||||||
|
# OPS-011 — what this workflow now guarantees, and what it still does not.
|
||||||
|
#
|
||||||
|
# Guaranteed:
|
||||||
|
# - nothing is uploaded unless the test suite and the linters pass;
|
||||||
|
# - only files on an explicit allowlist are uploaded, so a new file at the
|
||||||
|
# repository root does not reach production by default. That is how
|
||||||
|
# clear_db.py — a script that DROPs every table and recreates
|
||||||
|
# admin/password — got there in the first place;
|
||||||
|
# - after the upload, /health is polled until it answers healthy, and the
|
||||||
|
# job fails loudly if it does not. Before, a half-uploaded tree was a
|
||||||
|
# green deployment.
|
||||||
|
#
|
||||||
|
# NOT guaranteed — the switch is not atomic. Files are mirrored in place, so
|
||||||
|
# for the length of the transfer production runs a mixture of two versions.
|
||||||
|
# Closing that needs a release-directory layout, which has three
|
||||||
|
# prerequisites, two of which cannot be done from here:
|
||||||
|
#
|
||||||
|
# 1. the Pterodactyl startup command must run the app from `current/`
|
||||||
|
# rather than from the server root, and the server must be restarted on
|
||||||
|
# switch — a panel change;
|
||||||
|
# 2. `documents/`, `logs/` and `.env` must live beside the releases, not
|
||||||
|
# inside one. DOCUMENTS_ROOT exists for this (app/storage.py);
|
||||||
|
# 3. contract paths must be relative to that root, so the switch does not
|
||||||
|
# strand them. Done: new rows are relative, old absolute ones still
|
||||||
|
# resolve.
|
||||||
|
#
|
||||||
|
# docs/deployment.md carries the design and the rollback procedure.
|
||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
deploy-to-sftp:
|
deploy-to-sftp:
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
@@ -16,8 +44,40 @@ jobs:
|
|||||||
- name: Checkout repository
|
- name: Checkout repository
|
||||||
uses: actions/checkout@v4
|
uses: actions/checkout@v4
|
||||||
|
|
||||||
- name: Install lftp and ssh
|
- name: Set up Python
|
||||||
run: sudo apt-get update && sudo apt-get install -y lftp openssh-client
|
uses: actions/setup-python@v5
|
||||||
|
with:
|
||||||
|
python-version: '3.12'
|
||||||
|
|
||||||
|
- name: Install dependencies
|
||||||
|
run: pip install -r requirements.txt -r requirements-dev.txt
|
||||||
|
|
||||||
|
# The gate. This runner is not the GitHub one, so a green CI over
|
||||||
|
# there proves nothing about what is about to be shipped from here:
|
||||||
|
# the deploy is triggered by hand, on whatever the branch holds.
|
||||||
|
- name: Refuse to deploy a broken tree
|
||||||
|
run: |
|
||||||
|
python -m pytest -q
|
||||||
|
python -m ruff check .
|
||||||
|
python -m ruff format --check .
|
||||||
|
|
||||||
|
# An allowlist, not a list of exclusions. The previous form mirrored
|
||||||
|
# the whole working tree minus nine globs, so every file added to the
|
||||||
|
# repository shipped to production unless someone remembered to
|
||||||
|
# exclude it. This inverts the default: a new top-level file has to be
|
||||||
|
# named here to reach the server.
|
||||||
|
- name: Assemble the release payload
|
||||||
|
run: |
|
||||||
|
set -euo pipefail
|
||||||
|
mkdir -p payload
|
||||||
|
cp -r app payload/
|
||||||
|
cp requirements.txt wsgi.py payload/
|
||||||
|
# Compiled catalogues are versioned deliberately: the deployment is
|
||||||
|
# a file mirror with no build step (docs/translations.md).
|
||||||
|
find payload -name '__pycache__' -type d -prune -exec rm -rf {} +
|
||||||
|
find payload -name '*.pyc' -delete
|
||||||
|
echo "Shipping $(find payload -type f | wc -l) files:"
|
||||||
|
find payload -maxdepth 2 -type d | sort
|
||||||
|
|
||||||
- name: Set up SSH Private Key
|
- name: Set up SSH Private Key
|
||||||
env:
|
env:
|
||||||
@@ -31,36 +91,51 @@ jobs:
|
|||||||
|
|
||||||
- name: Push files via SFTP with progress
|
- name: Push files via SFTP with progress
|
||||||
run: |
|
run: |
|
||||||
# The mirror command below uses the -R (reverse) flag
|
|
||||||
# to push from local './' to remote './'
|
|
||||||
# Connection is made using 'open' inside the execution block to enforce SSH key usage
|
|
||||||
#
|
|
||||||
# --exclude-glob entries: the previous command mirrored the entire
|
|
||||||
# working tree, so CI definitions, the test suite and clear_db.py --
|
|
||||||
# a script that DELETEs every table and recreates admin/password --
|
|
||||||
# were all shipped to the production node.
|
|
||||||
#
|
|
||||||
# --delete is deliberately NOT used. Uploaded contracts, logs and the
|
# --delete is deliberately NOT used. Uploaded contracts, logs and the
|
||||||
# server's own .env live under the deployment root and are absent
|
# server's own .env live under the deployment root and are absent
|
||||||
# from the repository; deleting anything not present locally would
|
# from the repository; deleting anything not present locally would
|
||||||
# destroy them. Stale files therefore accumulate: switching to an
|
# destroy them. Stale files therefore still accumulate — that is the
|
||||||
# atomic timestamped-directory deploy is tracked as OPS-011.
|
# other half of what the release-directory layout would fix.
|
||||||
lftp -e "set sftp:connect-program 'ssh -a -x -i ~/.ssh/id_rsa -o StrictHostKeyChecking=no -o BatchMode=yes -o PasswordAuthentication=no'; \
|
lftp -e "set sftp:connect-program 'ssh -a -x -i ~/.ssh/id_rsa -o StrictHostKeyChecking=no -o BatchMode=yes -o PasswordAuthentication=no'; \
|
||||||
set sftp:auto-confirm yes; \
|
set sftp:auto-confirm yes; \
|
||||||
set net:max-retries 5; \
|
set net:max-retries 5; \
|
||||||
set net:timeout 30; \
|
set net:timeout 30; \
|
||||||
set cmd:fail-exit yes; \
|
set cmd:fail-exit yes; \
|
||||||
open -u ${{ secrets.SSH_USER }}, sftp://sftp.node4.immortal.host:2022; \
|
open -u ${{ secrets.SSH_USER }}, sftp://sftp.node4.immortal.host:2022; \
|
||||||
mirror -R --verbose --parallel=4 \
|
mirror -R --verbose --parallel=4 ./payload/ ./; \
|
||||||
--exclude-glob .git/ \
|
|
||||||
--exclude-glob .github/ \
|
|
||||||
--exclude-glob .gitea/ \
|
|
||||||
--exclude-glob .venv/ \
|
|
||||||
--exclude-glob venv/ \
|
|
||||||
--exclude-glob tests/ \
|
|
||||||
--exclude-glob audit/ \
|
|
||||||
--exclude-glob .ai/ \
|
|
||||||
--exclude-glob __pycache__/ \
|
|
||||||
--exclude-glob clear_db.py \
|
|
||||||
./ ./; \
|
|
||||||
quit"
|
quit"
|
||||||
|
|
||||||
|
# Without this a deployment that left the site 500ing reported success,
|
||||||
|
# and the first person to hear about it was a user. /health checks the
|
||||||
|
# database connection and reports whether the Discord bot thread is
|
||||||
|
# alive (OPS-012).
|
||||||
|
# HEALTH_URL is carried as a secret rather than as a variable. It is
|
||||||
|
# not secret — it is the public site — but `secrets` is the context
|
||||||
|
# this runner is already known to support, and a smoke test that fails
|
||||||
|
# to run because of an unsupported expression is worse than none.
|
||||||
|
- name: Smoke test
|
||||||
|
if: ${{ secrets.HEALTH_URL != '' }}
|
||||||
|
env:
|
||||||
|
HEALTH_URL: ${{ secrets.HEALTH_URL }}
|
||||||
|
run: |
|
||||||
|
set -euo pipefail
|
||||||
|
# The app is restarted by the panel, not by this workflow, so the
|
||||||
|
# first few probes are expected to fail or answer from the old
|
||||||
|
# process. Two minutes, then give up.
|
||||||
|
for attempt in $(seq 1 24); do
|
||||||
|
body=$(curl -fsS --max-time 10 "$HEALTH_URL" 2>/dev/null) || body=''
|
||||||
|
if echo "$body" | grep -q '"status": *"healthy"'; then
|
||||||
|
echo "Healthy after ${attempt} attempt(s):"
|
||||||
|
echo "$body"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
echo "attempt ${attempt}: not healthy yet"
|
||||||
|
sleep 5
|
||||||
|
done
|
||||||
|
echo "::error::/health never reported healthy. The deployment is live and may be broken — see the rollback procedure in docs/deployment.md."
|
||||||
|
exit 1
|
||||||
|
|
||||||
|
- name: Warn when no health check is configured
|
||||||
|
if: ${{ secrets.HEALTH_URL == '' }}
|
||||||
|
run: |
|
||||||
|
echo "::warning::HEALTH_URL is not set, so this deployment was not verified. Set it to https://<host>/health in the repository secrets."
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ from app.routes.users._shared import (
|
|||||||
pdf_upload_error,
|
pdf_upload_error,
|
||||||
)
|
)
|
||||||
from app.routes.users.blueprint import users_bp
|
from app.routes.users.blueprint import users_bp
|
||||||
|
from app.storage import CONTRACTS_DIR, document_path
|
||||||
from app.validators import UploadContractSchema
|
from app.validators import UploadContractSchema
|
||||||
|
|
||||||
|
|
||||||
@@ -108,25 +109,24 @@ def upload_contract():
|
|||||||
flash(error, 'danger')
|
flash(error, 'danger')
|
||||||
return redirect(url_for('users.upload_contract'))
|
return redirect(url_for('users.upload_contract'))
|
||||||
|
|
||||||
upload_dir = os.path.join(os.getcwd(), 'documents', 'contrats signés')
|
|
||||||
os.makedirs(upload_dir, exist_ok=True)
|
|
||||||
|
|
||||||
player = User.query.get_or_404(player_id)
|
player = User.query.get_or_404(player_id)
|
||||||
player_teams = player.get_org_teams()
|
player_teams = player.get_org_teams()
|
||||||
team = player_teams[0] if player_teams else None
|
team = player_teams[0] if player_teams else None
|
||||||
|
|
||||||
if team:
|
|
||||||
team_folder = os.path.join(upload_dir, secure_filename(team.name))
|
|
||||||
os.makedirs(team_folder, exist_ok=True)
|
|
||||||
final_dir = team_folder
|
|
||||||
else:
|
|
||||||
final_dir = upload_dir
|
|
||||||
|
|
||||||
original_filename = secure_filename(file.filename)
|
original_filename = secure_filename(file.filename)
|
||||||
file_uuid = str(uuid.uuid4())
|
stored_filename = f"{uuid.uuid4()}.pdf"
|
||||||
stored_filename = f"{file_uuid}.pdf"
|
|
||||||
file_path = os.path.join(final_dir, stored_filename)
|
# Kept relative to the document root, not absolute (see app/storage.py):
|
||||||
file.save(file_path)
|
# an absolute path pins the file to the directory the process was
|
||||||
|
# started from, which is the one thing a release-directory deploy
|
||||||
|
# changes.
|
||||||
|
relative_path = os.path.join(CONTRACTS_DIR, stored_filename)
|
||||||
|
if team:
|
||||||
|
relative_path = os.path.join(CONTRACTS_DIR, secure_filename(team.name), stored_filename)
|
||||||
|
|
||||||
|
absolute_path = document_path(relative_path)
|
||||||
|
os.makedirs(os.path.dirname(absolute_path), exist_ok=True)
|
||||||
|
file.save(absolute_path)
|
||||||
|
|
||||||
contract = Contract(
|
contract = Contract(
|
||||||
player_id=player_id,
|
player_id=player_id,
|
||||||
@@ -134,7 +134,7 @@ def upload_contract():
|
|||||||
uploaded_by_id=current_user.id,
|
uploaded_by_id=current_user.id,
|
||||||
original_filename=original_filename,
|
original_filename=original_filename,
|
||||||
stored_filename=stored_filename,
|
stored_filename=stored_filename,
|
||||||
file_path=file_path,
|
file_path=relative_path,
|
||||||
notes=notes if notes else None,
|
notes=notes if notes else None,
|
||||||
)
|
)
|
||||||
db.session.add(contract)
|
db.session.add(contract)
|
||||||
@@ -164,12 +164,11 @@ def upload_signed_contract(contract_id):
|
|||||||
return redirect(url_for('users.list_contracts'))
|
return redirect(url_for('users.list_contracts'))
|
||||||
|
|
||||||
signed_filename = f"signed_{contract.stored_filename}"
|
signed_filename = f"signed_{contract.stored_filename}"
|
||||||
file.save(contract.file_path.replace(contract.stored_filename, signed_filename))
|
signed_path = contract.file_path.replace(contract.stored_filename, signed_filename)
|
||||||
|
file.save(document_path(signed_path))
|
||||||
|
|
||||||
contract.signed_filename = signed_filename
|
contract.signed_filename = signed_filename
|
||||||
contract.signed_file_path = contract.file_path.replace(
|
contract.signed_file_path = signed_path
|
||||||
contract.stored_filename, signed_filename
|
|
||||||
)
|
|
||||||
contract.status = 'signed'
|
contract.status = 'signed'
|
||||||
contract.signed_at = datetime.utcnow()
|
contract.signed_at = datetime.utcnow()
|
||||||
db.session.commit()
|
db.session.commit()
|
||||||
@@ -186,7 +185,9 @@ def download_contract(contract_id):
|
|||||||
flash(_('You do not have permission to download this contract.'), 'danger')
|
flash(_('You do not have permission to download this contract.'), 'danger')
|
||||||
return redirect(url_for('users.list_contracts'))
|
return redirect(url_for('users.list_contracts'))
|
||||||
return send_file(
|
return send_file(
|
||||||
contract.file_path, as_attachment=True, download_name=contract.original_filename
|
document_path(contract.file_path),
|
||||||
|
as_attachment=True,
|
||||||
|
download_name=contract.original_filename,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -202,5 +203,7 @@ def download_signed_contract(contract_id):
|
|||||||
flash(_('No signed contract available.'), 'danger')
|
flash(_('No signed contract available.'), 'danger')
|
||||||
return redirect(url_for('users.list_contracts'))
|
return redirect(url_for('users.list_contracts'))
|
||||||
return send_file(
|
return send_file(
|
||||||
contract.signed_file_path, as_attachment=True, download_name=contract.signed_filename
|
document_path(contract.signed_file_path),
|
||||||
|
as_attachment=True,
|
||||||
|
download_name=contract.signed_filename,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -0,0 +1,59 @@
|
|||||||
|
"""Where uploaded documents live, and how the database refers to them.
|
||||||
|
|
||||||
|
Contracts were stored at `os.path.join(os.getcwd(), 'documents', …)`,
|
||||||
|
evaluated at upload time, and the resulting absolute path was written into
|
||||||
|
`Contract.file_path`. The storage root therefore moved with whatever
|
||||||
|
directory the process happened to be started from. Two consequences:
|
||||||
|
|
||||||
|
- one latent: start the server from elsewhere and new contracts land in a
|
||||||
|
new tree while the old ones become unreadable — with the database still
|
||||||
|
saying they are there, so the failure surfaces as a 500 on download
|
||||||
|
rather than as anything a person could act on;
|
||||||
|
- one blocking: it rules out a release-directory deployment (OPS-011)
|
||||||
|
outright. Every stored path would point inside a release that is about
|
||||||
|
to be replaced, so the first switch would take every contract ever
|
||||||
|
uploaded with it.
|
||||||
|
|
||||||
|
New rows keep a path *relative* to the document root. Old rows keep their
|
||||||
|
absolute path and are returned untouched, so this change needs no data
|
||||||
|
migration and can ship before Alembic does (DB-002).
|
||||||
|
"""
|
||||||
|
|
||||||
|
import os
|
||||||
|
|
||||||
|
#: Environment override for the document root. What a release-directory
|
||||||
|
#: deployment sets, to a path outside the releases — alongside them, not
|
||||||
|
#: inside whichever one is current.
|
||||||
|
DOCUMENTS_ROOT_ENV = 'DOCUMENTS_ROOT'
|
||||||
|
|
||||||
|
#: Sub-directory holding uploaded contracts, under the document root.
|
||||||
|
CONTRACTS_DIR = 'contrats signés'
|
||||||
|
|
||||||
|
|
||||||
|
def documents_root():
|
||||||
|
"""Absolute path of the document store.
|
||||||
|
|
||||||
|
Falls back to `documents/` beside this package — the project root
|
||||||
|
wherever it is installed, rather than wherever the process was launched.
|
||||||
|
"""
|
||||||
|
configured = os.getenv(DOCUMENTS_ROOT_ENV)
|
||||||
|
if configured:
|
||||||
|
return os.path.abspath(configured)
|
||||||
|
package_dir = os.path.dirname(os.path.abspath(__file__))
|
||||||
|
return os.path.join(os.path.dirname(package_dir), 'documents')
|
||||||
|
|
||||||
|
|
||||||
|
def document_path(stored_path):
|
||||||
|
"""Absolute path of a document, from what the database holds.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
stored_path: The value of Contract.file_path or signed_file_path.
|
||||||
|
Relative for rows written since this module existed, absolute
|
||||||
|
for the ones written before.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
str: An absolute path.
|
||||||
|
"""
|
||||||
|
if os.path.isabs(stored_path):
|
||||||
|
return stored_path
|
||||||
|
return os.path.join(documents_root(), stored_path)
|
||||||
@@ -145,6 +145,93 @@ python security_scan.py --url https://yourdomain.com
|
|||||||
|
|
||||||
All checks must pass before deployment.
|
All checks must pass before deployment.
|
||||||
|
|
||||||
|
## Deploying, and undoing a deployment (OPS-011)
|
||||||
|
|
||||||
|
Deployment is the Gitea workflow `.gitea/workflows/git-to-ptero.yaml`, run by
|
||||||
|
hand (`workflow_dispatch`). It mirrors files over SFTP to the Pterodactyl
|
||||||
|
node.
|
||||||
|
|
||||||
|
### What the workflow guarantees
|
||||||
|
|
||||||
|
1. **Nothing ships from a broken tree.** The suite, `ruff check` and
|
||||||
|
`ruff format --check` all run on the deploy runner first. A green CI on
|
||||||
|
GitHub proves nothing here: the deploy is triggered by hand, on whatever
|
||||||
|
the branch currently holds.
|
||||||
|
2. **Only named files ship.** The payload is an allowlist — `app/`,
|
||||||
|
`wsgi.py`, `requirements.txt` — not the working tree minus exclusions.
|
||||||
|
A new file at the repository root does not reach production unless
|
||||||
|
somebody adds it. The old form is how `clear_db.py`, the test suite and
|
||||||
|
the CI definitions got onto the production node.
|
||||||
|
3. **The deployment is verified.** `/health` is polled for two minutes after
|
||||||
|
the upload and the job fails if it never reports healthy. Set the
|
||||||
|
`HEALTH_URL` repository secret to `https://<host>/health`; without it the
|
||||||
|
workflow warns that the deployment went out unverified.
|
||||||
|
|
||||||
|
Deliberately *not* shipped: `run.py` (development entry point), `tests/`,
|
||||||
|
`migrations/add_tryout_coaches.py` (a one-off, already applied — run it by
|
||||||
|
hand if a fresh database ever needs it), `docs/`, `audit/`, `clear_db.py`.
|
||||||
|
|
||||||
|
### What it does not guarantee
|
||||||
|
|
||||||
|
**The switch is not atomic.** Files are mirrored in place, so for the length
|
||||||
|
of the transfer the node runs a mixture of two versions. And because
|
||||||
|
`--delete` is off — uploaded contracts, logs and the server's `.env` live
|
||||||
|
under the deployment root and are absent from the repository — a file
|
||||||
|
removed from the repository stays on the server for ever.
|
||||||
|
|
||||||
|
### Rolling back
|
||||||
|
|
||||||
|
There is no previous release on the node to switch back to, so a rollback is
|
||||||
|
a forward deployment of a known-good commit:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# 1. Find the last deployment that was verified healthy — the workflow run
|
||||||
|
# log names the commit.
|
||||||
|
git log --oneline
|
||||||
|
|
||||||
|
# 2. Deploy that commit. In the Gitea UI, run the "Push to SFTP" workflow
|
||||||
|
# against the tag or branch pointing at it. Tag known-good releases so
|
||||||
|
# this step does not depend on reading a log:
|
||||||
|
git tag -a deploy-2026-08-11 -m "verified healthy" <commit>
|
||||||
|
git push origin deploy-2026-08-11 # requires the push freeze to be lifted
|
||||||
|
|
||||||
|
# 3. Confirm.
|
||||||
|
curl -fsS https://<host>/health
|
||||||
|
```
|
||||||
|
|
||||||
|
A rollback does **not** undo a database migration. If the deployment that
|
||||||
|
broke production also changed the schema, restore from backup first —
|
||||||
|
`docs/database-restore.md`.
|
||||||
|
|
||||||
|
### Making the switch atomic
|
||||||
|
|
||||||
|
The remaining work, and why it is not done here. A release-directory layout
|
||||||
|
looks like this on the node:
|
||||||
|
|
||||||
|
```
|
||||||
|
/home/container/
|
||||||
|
├── releases/
|
||||||
|
│ ├── 2026-08-11-a1b2c3/
|
||||||
|
│ └── 2026-08-10-9f8e7d/
|
||||||
|
├── current -> releases/2026-08-11-a1b2c3
|
||||||
|
├── documents/ # shared, never inside a release
|
||||||
|
├── logs/ # shared
|
||||||
|
└── .env # shared
|
||||||
|
```
|
||||||
|
|
||||||
|
Three prerequisites. One is done, two are not:
|
||||||
|
|
||||||
|
| # | Prerequisite | State |
|
||||||
|
|---|---|---|
|
||||||
|
| 1 | The Pterodactyl startup command must run the app from `current/`, and the server must be restarted on switch | **Panel change.** Cannot be made or verified from the repository |
|
||||||
|
| 2 | `documents/`, `logs/` and `.env` must sit beside the releases, not inside one. `DOCUMENTS_ROOT` points the document store at a fixed path (`app/storage.py`) | Mechanism ready, **not yet configured on the node** |
|
||||||
|
| 3 | Contract paths must be relative to that root, or the first switch strands every contract ever uploaded | **Done.** New rows store a relative path; rows written earlier keep their absolute one and still resolve, so no data migration is needed |
|
||||||
|
|
||||||
|
Prerequisite 3 was a defect on its own, not just a blocker: paths were built
|
||||||
|
from `os.getcwd()`, so starting the server from a different directory would
|
||||||
|
have sent new contracts to a new tree and made the existing ones unreadable
|
||||||
|
— with the database still claiming they were there.
|
||||||
|
|
||||||
## Security Checklist
|
## Security Checklist
|
||||||
|
|
||||||
- [ ] `.env` is not committed to repository
|
- [ ] `.env` is not committed to repository
|
||||||
|
|||||||
@@ -70,6 +70,18 @@ def _base_config(db_path, csrf=False):
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def documents_in_a_throwaway_directory(tmp_path, monkeypatch):
|
||||||
|
"""Keep uploads out of the repository.
|
||||||
|
|
||||||
|
Contract uploads land under app.storage.documents_root(), which without
|
||||||
|
this is `documents/` beside the package — i.e. inside the checkout. No
|
||||||
|
test writes a file there today; this is so that the first one that does
|
||||||
|
cannot leave a PDF in someone's working tree.
|
||||||
|
"""
|
||||||
|
monkeypatch.setenv('DOCUMENTS_ROOT', str(tmp_path / 'documents'))
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def app():
|
def app():
|
||||||
"""A fully configured application backed by a throwaway SQLite file.
|
"""A fully configured application backed by a throwaway SQLite file.
|
||||||
|
|||||||
@@ -0,0 +1,146 @@
|
|||||||
|
"""Where uploaded documents live (OPS-011).
|
||||||
|
|
||||||
|
Contract paths were absolute and built from `os.getcwd()`, so the storage
|
||||||
|
root followed whatever directory the process was started from. That is a
|
||||||
|
defect on its own — a server restarted from elsewhere writes new contracts
|
||||||
|
into a new tree and cannot read the old ones, while the database goes on
|
||||||
|
saying they are there — and it is what made a release-directory deployment
|
||||||
|
impossible: every stored path would point inside a release about to be
|
||||||
|
replaced.
|
||||||
|
|
||||||
|
The compatibility case is the one that matters most here. Rows written
|
||||||
|
before this change hold absolute paths, and they have to keep working
|
||||||
|
without a data migration, because the migration tooling does not exist yet
|
||||||
|
(DB-002, blocked).
|
||||||
|
"""
|
||||||
|
|
||||||
|
import os
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from app.storage import CONTRACTS_DIR, document_path, documents_root
|
||||||
|
|
||||||
|
|
||||||
|
class TestDocumentsRoot:
|
||||||
|
def test_the_environment_wins(self, tmp_path, monkeypatch):
|
||||||
|
monkeypatch.setenv('DOCUMENTS_ROOT', str(tmp_path / 'elsewhere'))
|
||||||
|
|
||||||
|
assert documents_root() == str(tmp_path / 'elsewhere')
|
||||||
|
|
||||||
|
def test_a_relative_override_is_made_absolute(self, monkeypatch):
|
||||||
|
monkeypatch.setenv('DOCUMENTS_ROOT', 'docs-here')
|
||||||
|
|
||||||
|
assert os.path.isabs(documents_root())
|
||||||
|
|
||||||
|
def test_the_default_does_not_follow_the_working_directory(self, tmp_path, monkeypatch):
|
||||||
|
"""The whole point. `os.getcwd()` made this move; the package
|
||||||
|
location does not."""
|
||||||
|
monkeypatch.delenv('DOCUMENTS_ROOT', raising=False)
|
||||||
|
before = documents_root()
|
||||||
|
|
||||||
|
monkeypatch.chdir(tmp_path)
|
||||||
|
after = documents_root()
|
||||||
|
|
||||||
|
assert before == after
|
||||||
|
|
||||||
|
def test_the_default_sits_beside_the_package(self, monkeypatch):
|
||||||
|
monkeypatch.delenv('DOCUMENTS_ROOT', raising=False)
|
||||||
|
|
||||||
|
# From the module file, not from `app.__file__`: `app` has no
|
||||||
|
# __init__.py, so it is a namespace package and __file__ is None.
|
||||||
|
from app import storage
|
||||||
|
|
||||||
|
package_dir = os.path.dirname(os.path.abspath(storage.__file__))
|
||||||
|
expected = os.path.join(os.path.dirname(package_dir), 'documents')
|
||||||
|
assert documents_root() == expected
|
||||||
|
|
||||||
|
|
||||||
|
class TestDocumentPath:
|
||||||
|
def test_a_relative_path_is_resolved_against_the_root(self, tmp_path, monkeypatch):
|
||||||
|
monkeypatch.setenv('DOCUMENTS_ROOT', str(tmp_path))
|
||||||
|
|
||||||
|
resolved = document_path(os.path.join(CONTRACTS_DIR, 'abc.pdf'))
|
||||||
|
|
||||||
|
assert resolved == str(tmp_path / CONTRACTS_DIR / 'abc.pdf')
|
||||||
|
|
||||||
|
def test_an_absolute_path_is_left_alone(self, tmp_path, monkeypatch):
|
||||||
|
"""Rows written before this module existed. They must keep resolving
|
||||||
|
to where the file actually is, or every contract uploaded so far
|
||||||
|
becomes a 500 on download the day this ships."""
|
||||||
|
monkeypatch.setenv('DOCUMENTS_ROOT', str(tmp_path / 'new-root'))
|
||||||
|
legacy = os.path.abspath(os.path.join('C:' + os.sep, 'old', 'place', 'abc.pdf'))
|
||||||
|
|
||||||
|
assert document_path(legacy) == legacy
|
||||||
|
|
||||||
|
def test_moving_the_root_moves_new_documents_and_not_old_ones(self, tmp_path, monkeypatch):
|
||||||
|
relative = os.path.join(CONTRACTS_DIR, 'abc.pdf')
|
||||||
|
legacy = os.path.abspath(os.path.join(str(tmp_path), 'legacy', 'abc.pdf'))
|
||||||
|
|
||||||
|
monkeypatch.setenv('DOCUMENTS_ROOT', str(tmp_path / 'one'))
|
||||||
|
first_new, first_old = document_path(relative), document_path(legacy)
|
||||||
|
|
||||||
|
monkeypatch.setenv('DOCUMENTS_ROOT', str(tmp_path / 'two'))
|
||||||
|
second_new, second_old = document_path(relative), document_path(legacy)
|
||||||
|
|
||||||
|
assert first_new != second_new, 'a release switch has to move new documents'
|
||||||
|
assert first_old == second_old, 'and must not move the ones already filed'
|
||||||
|
|
||||||
|
|
||||||
|
class TestThroughTheUploadRoute:
|
||||||
|
@pytest.fixture
|
||||||
|
def uploaded(self, app, client, as_role, make_user):
|
||||||
|
"""A contract filed by an admin for a player."""
|
||||||
|
as_role('admin')
|
||||||
|
player_id = make_user('player')
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
'/users/contracts/upload',
|
||||||
|
data={
|
||||||
|
'player_id': str(player_id),
|
||||||
|
'notes': 'Season contract',
|
||||||
|
'contract_file': (_pdf(), 'contract.pdf'),
|
||||||
|
},
|
||||||
|
content_type='multipart/form-data',
|
||||||
|
follow_redirects=True,
|
||||||
|
)
|
||||||
|
assert response.status_code == 200
|
||||||
|
return player_id
|
||||||
|
|
||||||
|
def test_the_stored_path_is_relative(self, app, uploaded):
|
||||||
|
from app.models import Contract
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
contract = Contract.query.one()
|
||||||
|
assert not os.path.isabs(contract.file_path), (
|
||||||
|
'an absolute path pins the file to the directory the process '
|
||||||
|
'was started from — the one thing a release switch changes'
|
||||||
|
)
|
||||||
|
assert contract.file_path.startswith(CONTRACTS_DIR)
|
||||||
|
|
||||||
|
def test_the_file_lands_under_the_configured_root(self, app, uploaded):
|
||||||
|
from app.models import Contract
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
contract = Contract.query.one()
|
||||||
|
resolved = document_path(contract.file_path)
|
||||||
|
|
||||||
|
assert os.path.exists(resolved)
|
||||||
|
assert resolved.startswith(documents_root())
|
||||||
|
|
||||||
|
def test_it_can_be_downloaded_back(self, app, client, uploaded):
|
||||||
|
from app.models import Contract
|
||||||
|
|
||||||
|
with app.app_context():
|
||||||
|
contract_id = Contract.query.one().id
|
||||||
|
|
||||||
|
response = client.get(f'/users/contracts/{contract_id}/download')
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.data.startswith(b'%PDF-')
|
||||||
|
|
||||||
|
|
||||||
|
def _pdf():
|
||||||
|
"""The smallest thing pdf_upload_error accepts."""
|
||||||
|
import io
|
||||||
|
|
||||||
|
return io.BytesIO(b'%PDF-1.4\n%%EOF\n')
|
||||||
Reference in New Issue
Block a user