Merge pull request 'dev' (#39) from dev into main

Reviewed-on: #39

.dockerignore

@@ -26,8 +26,6 @@
 
 # CI and test configuration, not needed at runtime.
 **/Jenkinsfile
-**/pytest.ini
-**/tests/
 
 # Documentation and planning, not needed at runtime.
 **/*.md

Dockerfile

@@ -1,31 +1,37 @@
-FROM python:3.11-slim
+FROM python:3.11-slim AS runtime
 
-# Fail fast and log straight to stdout, so `docker-compose logs` is useful.
 ENV PYTHONDONTWRITEBYTECODE=1 \
     PYTHONUNBUFFERED=1
 
 WORKDIR /app
 
-# Dependencies first: this layer is cached and only rebuilt when
-# requirements.txt changes, not on every code change.
 COPY expenses_manager/requirements.txt .
 RUN pip install --no-cache-dir -r requirements.txt
 
-# Copy the Django project itself, not the repository root, so that /app is
-# the directory holding manage.py. The previous version copied the repo root
-# into /app/expenses_manager, which left manage.py and wsgi.py one level
-# deeper than the gunicorn command expected.
 COPY expenses_manager/ .
 
-# Collect static files at build time so the running container never needs to
-# write to its own filesystem. SECRET_KEY is required by settings.py when
-# DEBUG is False, but collectstatic never uses it, so a throwaway value is
-# enough — the real one comes from the environment at runtime.
 RUN SECRET_KEY=build-time-placeholder \
     DEBUG=False \
     ALLOWED_HOSTS=localhost \
     python manage.py collectstatic --noinput
 
+# Run as an unprivileged account. /app must belong to it: gunicorn writes its
+# control socket there, and pytest its cache in the test image.
+RUN useradd --create-home --uid 1000 appuser \
+    && chown -R appuser:appuser /app
+
+USER appuser
+
 EXPOSE 8000
 
 CMD ["gunicorn", "expenses_manager.wsgi:application", "--bind", "0.0.0.0:8000"]
+
+
+# The production image plus the test-only dependencies.
+FROM runtime AS test
+
+# pip writes to the system site-packages, so the install runs as root and the
+# image then drops back to the unprivileged account.
+USER root
+RUN pip install --no-cache-dir -r requirements-dev.txt
+USER appuser

expenses_manager/Jenkinsfile

@@ -4,13 +4,8 @@ pipeline {
     agent none
 
     environment {
-        DEBUG = 'True'
-        SECRET_KEY = 'ci-only-value-never-used-in-production'
-        DJANGO_SETTINGS_MODULE = 'expenses_manager.settings'
-        PYTHONUNBUFFERED = '1'
-
-        // Jenkins has no Docker daemon, so everything Docker-related happens on
-        // the NAS over SSH. Absolute paths are required: the remote
+        // Everything Docker-related happens on the NAS over SSH: Jenkins has no
+        // daemon of its own. Absolute paths are required — the remote
         // non-interactive PATH does not include /usr/local/bin.
         NAS_HOST = '192.168.1.140'
         NAS_USER = 'jenkins-ci'
@@ -31,81 +26,139 @@ pipeline {
 
     stages {
 
-        // Nested stages share one agent allocation, so the venv built in the
-        // first stage is still there for the ones that follow.
+        // Nested stages share one agent allocation.
         stage('CI') {
             agent any
-            options { timeout(time: 30, unit: 'MINUTES') }
+            options { timeout(time: 40, unit: 'MINUTES') }
             stages {
 
-                stage('Setup Python') {
-                    steps {
-                        sh '''
-                            cd expenses_manager
-                            python --version
-                            python -m venv venv
-                            venv/bin/pip install --upgrade pip
-                            venv/bin/pip install -r requirements.txt
-                        '''
-                    }
-                }
-
-                // Catches models changed without generating the migration.
-                stage('Check migrations') {
-                    steps {
-                        sh '''
-                            cd expenses_manager
-                            venv/bin/python manage.py makemigrations --check --dry-run
-                        '''
-                    }
-                }
-
-                stage('Run tests') {
-                    steps {
-                        sh '''
-                            cd expenses_manager
-                            venv/bin/pytest --cov --junitxml=junit.xml
-                        '''
-                    }
-                    post {
-                        always {
-                            // allowEmptyResults: false is deliberate. A build where
-                            // pytest died before collecting anything produces no XML,
-                            // and that must fail rather than pass quietly.
-                            junit allowEmptyResults: false,
-                                testResults: 'expenses_manager/junit.xml'
-                        }
-                    }
-                }
-
-                stage('Build image') {
+                stage('Build images') {
                     steps {
                         sshagent(credentials: ['nas-ssh']) {
                             sh '''
                                 SHA=$(echo "${GIT_COMMIT}" | cut -c1-7)
                                 TAG=$(echo "${BRANCH_NAME}" | tr '/' '-')
 
-                                # Ship the workspace that was just tested rather than
-                                # cloning again: what gets built is then provably the
-                                # same code the tests ran against.
+                                # Ship the workspace rather than cloning again on the
+                                # NAS: what gets built is then provably the same code
+                                # this build checked out.
                                 ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
                                     "rm -rf ${CI_DIR} && mkdir -p ${CI_DIR}"
 
-                                tar -czf - --exclude=./expenses_manager/venv \
-                                        --exclude=./.git . \
+                                tar -czf - --exclude=./.git . \
                                     | ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
                                         "tar -xzf - -C ${CI_DIR}"
 
+                                # The test image is the runtime image plus test
+                                # dependencies, so the second build reuses every layer
+                                # of the first.
                                 ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
                                     "cd ${CI_DIR} && sudo ${NAS_DOCKER} build \
+                                        --target runtime \
                                         -t ${IMAGE}:${SHA} -t ${IMAGE}:${TAG} ."
+
+                                ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
+                                    "cd ${CI_DIR} && sudo ${NAS_DOCKER} build \
+                                        --target test \
+                                        -t ${IMAGE}-test:${SHA} ."
                             '''
                         }
                     }
                 }
 
-                // A successful build only proves the Dockerfile is syntactically
-                // sound. Django's own checks prove the image can actually start.
+                // Catches models changed without generating the migration. This
+                // never touches a database, so SQLite is enough here.
+                stage('Check migrations') {
+                    steps {
+                        sshagent(credentials: ['nas-ssh']) {
+                            sh '''
+                                SHA=$(echo "${GIT_COMMIT}" | cut -c1-7)
+                                ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
+                                    "sudo ${NAS_DOCKER} run --rm \
+                                        -e SECRET_KEY=ci-only-value \
+                                        -e DEBUG=False \
+                                        -e ALLOWED_HOSTS=localhost,testserver \
+                                        -e DJANGO_SETTINGS_MODULE=expenses_manager.settings_test \
+                                        -e DB_ENGINE=sqlite3 \
+                                        ${IMAGE}-test:${SHA} \
+                                        python manage.py makemigrations --check --dry-run"
+                            '''
+                        }
+                    }
+                }
+
+                stage('Run tests') {
+                    steps {
+                        sshagent(credentials: ['nas-ssh']) {
+                            // Tests run against PostgreSQL, the engine production
+                            // uses. SQLite is only a local-development convenience
+                            // and the two have already differed once (default
+                            // ordering), so a green SQLite run proves little.
+                            sh '''
+                                ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
+                                    "sudo ${NAS_DOCKER} network create ci-net-${BUILD_NUMBER} \
+                                     && sudo ${NAS_DOCKER} run -d \
+                                            --name ci-db-${BUILD_NUMBER} \
+                                            --network ci-net-${BUILD_NUMBER} \
+                                            -e POSTGRES_DB=ci \
+                                            -e POSTGRES_USER=ci \
+                                            -e POSTGRES_PASSWORD=ci-only-password \
+                                            postgres:15"
+                            '''
+
+                            sh '''
+                                ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
+                                    "for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15; do \
+                                        sudo ${NAS_DOCKER} exec ci-db-${BUILD_NUMBER} \
+                                            pg_isready -U ci -d ci > /dev/null 2>&1 && exit 0; \
+                                        sleep 2; \
+                                     done; \
+                                     echo 'Postgres no arranco a tiempo'; exit 1"
+                            '''
+
+                            sh '''
+                                SHA=$(echo "${GIT_COMMIT}" | cut -c1-7)
+                                ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
+                                    "sudo ${NAS_DOCKER} run --rm \
+                                        --network ci-net-${BUILD_NUMBER} \
+                                        -e SECRET_KEY=ci-only-value \
+                                        -e DEBUG=False \
+                                        -e ALLOWED_HOSTS=localhost,testserver \
+                                        -e DJANGO_SETTINGS_MODULE=expenses_manager.settings_test \
+                                        -e DB_ENGINE=postgresql \
+                                        -e DB_NAME=ci \
+                                        -e DB_USER=ci \
+                                        -e DB_PASSWORD=ci-only-password \
+                                        -e DB_HOST=ci-db-${BUILD_NUMBER} \
+                                        -e DB_PORT=5432 \
+                                        ${IMAGE}-test:${SHA} \
+                                        sh -c 'pytest --cov --junitxml=/tmp/junit.xml >&2; \
+                                               rc=\\$?; cat /tmp/junit.xml; exit \\$rc'" \
+                                    > junit.xml
+                            '''
+                        }
+                    }
+                    post {
+                        always {
+                            // allowEmptyResults: false is deliberate. A build where
+                            // pytest died before collecting anything produces no XML,
+                            // and that must fail rather than pass quietly.
+                            junit allowEmptyResults: false, testResults: 'junit.xml'
+
+                            // The throwaway database goes away whatever happened, so a
+                            // failed build does not leave containers behind.
+                            sshagent(credentials: ['nas-ssh']) {
+                                sh '''
+                                    ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
+                                        "sudo ${NAS_DOCKER} rm -f ci-db-${BUILD_NUMBER} > /dev/null 2>&1; \
+                                         sudo ${NAS_DOCKER} network rm ci-net-${BUILD_NUMBER} > /dev/null 2>&1; \
+                                         exit 0"
+                                '''
+                            }
+                        }
+                    }
+                }
+
                 stage('Smoke test image') {
                     steps {
                         sshagent(credentials: ['nas-ssh']) {
@@ -121,10 +174,10 @@ pipeline {
                             '''
 
                             // A migrations directory without __init__.py is a namespace
-                        // package, and Django deliberately ignores those: the app then
-                        // appears to have no migrations at all, migrate --check always
-                        // reports nothing pending, and a real migration would silently
-                        // never be applied. This is what .gitignore line 6 caused.
+                            // package, and Django deliberately ignores those: the app
+                            // then appears to have no migrations at all, migrate --check
+                            // always reports nothing pending, and a real migration would
+                            // silently never be applied.
                             sh '''
                                 SHA=$(echo "${GIT_COMMIT}" | cut -c1-7)
                                 ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
@@ -139,6 +192,30 @@ pipeline {
                         }
                     }
                 }
+
+                stage('Prune old images') {
+                    steps {
+                        sshagent(credentials: ['nas-ssh']) {
+                            // docker images lists newest first, so anything past the
+                            // fifth is an older build. Branch tags are kept because
+                            // they always point at a recent image, and the image the
+                            // running container uses cannot be removed anyway.
+                            sh '''
+                                ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
+                                    "for repo in ${IMAGE} ${IMAGE}-test; do \
+                                        sudo ${NAS_DOCKER} images \\$repo --format '{{.Repository}}:{{.Tag}}' \
+                                            | grep -v ':dev' | grep -v ':main' | grep -v '<none>' \
+                                            | tail -n +6 \
+                                            | while read img; do \
+                                                sudo ${NAS_DOCKER} rmi \\$img > /dev/null 2>&1; \
+                                              done; \
+                                     done; \
+                                     sudo ${NAS_DOCKER} image prune -f > /dev/null; \
+                                     exit 0"
+                            '''
+                        }
+                    }
+                }
             }
         }
 
@@ -164,8 +241,8 @@ pipeline {
                         if (rc != 0) {
                             // Printing the plan also proves the database is reachable.
                             // If the non-zero status above was a connection failure
-                            // rather than pending migrations, this fails loudly
-                            // instead of asking for approval on a false premise.
+                            // rather than pending migrations, this fails loudly instead
+                            // of asking for approval on a false premise.
                             sh '''
                                 SHA=$(echo "${GIT_COMMIT}" | cut -c1-7)
                                 ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
@@ -189,6 +266,8 @@ pipeline {
                 }
             }
             steps {
+                notifyTelegram("⏸️ Esperando aprobacion",
+                               "Hay migraciones pendientes. Entra desde el enlace para revisar el plan y aprobar el despliegue. Si no respondes en 2 horas, el despliegue se cancela.")
                 timeout(time: 2, unit: 'HOURS') {
                     input message: 'Hay migraciones pendientes. Revisa el plan en el log del stage anterior.',
                           ok: 'Migrar y desplegar'
@@ -205,8 +284,8 @@ pipeline {
                     sh '''
                         SHA=$(echo "${GIT_COMMIT}" | cut -c1-7)
 
-                        # Always back up, even with no migrations pending: this is
-                        # the restore point for this particular deployment.
+                        # Always back up, even with no migrations pending: this is the
+                        # restore point for this particular deployment.
                         ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} "sudo ${NAS_BACKUP}"
 
                         # Migrate with the new image before the running container is
@@ -218,10 +297,9 @@ pipeline {
                                 --env-file ${APP_ENV} \
                                 ${IMAGE}:${SHA} python manage.py migrate --noinput"
 
-                        # Point compose at the image that was just tested, then swap
-                        # the container. Writing the tag to .env leaves a readable
-                        # record of what is deployed, and makes rollback a one-line
-                        # edit plus "docker-compose up -d web".
+                        # Point compose at the image that was just tested, then swap the
+                        # container. Writing the tag to .env leaves a readable record of
+                        # what is deployed and makes rollback a one-line edit.
                         ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
                             "echo WEB_IMAGE=${IMAGE}:${SHA} > ${COMPOSE_DIR}/.env \
                              && cd ${COMPOSE_DIR} \
@@ -229,10 +307,10 @@ pipeline {
                     '''
 
                     script {
-                        // The Host header must match ALLOWED_HOSTS or Django answers
-                        // 400 regardless of health. A 301 is the expected reply here:
-                        // SECURE_SSL_REDIRECT sends plain HTTP to HTTPS, and reaching
-                        // that redirect already proves the app processed the request.
+                        // The Host header must match ALLOWED_HOSTS or Django answers 400
+                        // regardless of health. A 301 is expected: SECURE_SSL_REDIRECT
+                        // sends plain HTTP to HTTPS, and reaching that redirect already
+                        // proves the app processed the request.
                         def code = sh(returnStdout: true, script: '''
                             ssh ${SSH_OPTS} ${NAS_USER}@${NAS_HOST} \
                                 "sleep 10 && curl -s -o /dev/null -w '%{http_code}' \
@@ -249,23 +327,24 @@ pipeline {
             }
             post {
                 success {
-                    notifyTelegram("🚀 Desplegado")
+                    notifyTelegram("🚀 Desplegado", "")
                 }
             }
         }
     }
 
     post {
-        failure { notifyTelegram("❌ FALLO") }
-        fixed   { notifyTelegram("✅ Recuperado") }
-        aborted { notifyTelegram("⚠️ Abortado") }
+        failure { notifyTelegram("❌ FALLO", "") }
+        fixed   { notifyTelegram("✅ Recuperado", "") }
+        aborted { notifyTelegram("⚠️ Abortado", "") }
     }
 }
 
-// Sends a one-line build summary to Telegram. The node block is required because
-// the pipeline declares "agent none", so post actions have no workspace of their
-// own. Credentials come from the Jenkins store, never from the repository.
-void notifyTelegram(String status) {
+// Sends a one-line build summary to Telegram, with an optional extra paragraph.
+// The node block is required because the pipeline declares "agent none", so post
+// actions have no workspace of their own. Both parameters are mandatory to avoid
+// Groovy default arguments, which Jenkins' CPS interpreter handles poorly.
+void notifyTelegram(String status, String extra) {
     node {
         withCredentials([
             string(credentialsId: 'telegram-bot-token', variable: 'TG_TOKEN'),
@@ -276,7 +355,8 @@ void notifyTelegram(String status) {
                     --data-urlencode "chat_id=\${TG_CHAT}" \
                     --data-urlencode "text=${status} ${env.JOB_NAME} #${env.BUILD_NUMBER}
 Rama: ${env.BRANCH_NAME ?: 'main'}
-${env.BUILD_URL}" > /dev/null
+${env.BUILD_URL}
+${extra}" > /dev/null
             """
         }
     }

expenses_manager/expenses_manager/settings_test.py

@@ -0,0 +1,11 @@
+"""Settings for the test suite.
+
+Inherits the production configuration so tests exercise what actually runs on
+the NAS — DEBUG off, secure cookies, WhiteNoise storage — and turns off only
+what makes Django's test client unusable: SECURE_SSL_REDIRECT answers every
+request with a 301 before the view is ever reached.
+"""
+
+from .settings import *  # noqa: F401,F403
+
+SECURE_SSL_REDIRECT = False

expenses_manager/pytest.ini

@@ -1,3 +1,3 @@
 [pytest]
-DJANGO_SETTINGS_MODULE = expenses_manager.settings
+DJANGO_SETTINGS_MODULE = expenses_manager.settings_test
 python_files = tests.py test_*.py *_tests.py

expenses_manager/requirements-dev.txt

@@ -0,0 +1,11 @@
+-r requirements.txt
+
+colorama==0.4.6
+coverage==7.13.1
+iniconfig==2.3.0
+pluggy==1.6.0
+Pygments==2.19.2
+pytest==9.0.2
+pytest-cov>=4.0
+pytest-django==4.11.1
+toml==0.10.2

expenses_manager/requirements.txt

@@ -1,21 +1,11 @@
 asgiref==3.11.0
-colorama==0.4.6
-coverage==7.13.1
 Django==5.2.10
 gunicorn==25.1.0
-iniconfig==2.3.0
 packaging==25.0
-pluggy==1.6.0
 psycopg==3.3.2
 psycopg-binary==3.3.2
-Pygments==2.19.2
-pytest==9.0.2
-pytest-cov>=4.0
-pytest-django==4.11.1
 python-dotenv==1.2.1
 sqlparse==0.5.5
-toml==0.10.2
 typing_extensions==4.15.0
 tzdata==2025.3
 whitenoise==6.12.0
-