From ef161a0100f63f01dd0ce1791ced007567bdea72 Mon Sep 17 00:00:00 2001 From: Laura Barcziova Date: Mon, 10 Mar 2025 15:26:13 +0100 Subject: [PATCH 1/4] Switch to FastAPI + Gunicorn+Uvicorn, run Flask as hardcoded WSGI Introduce FastAPI running with Uvicorn (+Gunicorn in prod/stg), keep old Flask endpoints under /api hardcoded as WSGI. New endpoints will come under /v1, Swagger is at /docs and Redoc under /redoc. --- files/packit.wsgi | 1 - files/run_httpd.sh | 39 ++++--- files/tasks/httpd.yaml | 5 - packit_service/sentry_integration.py | 10 +- packit_service/service/api/__init__.py | 2 +- packit_service/service/api_v1/__init__.py | 4 + packit_service/service/app.py | 124 ++++++++++++++++------ 7 files changed, 129 insertions(+), 56 deletions(-) delete mode 100644 files/packit.wsgi create mode 100644 packit_service/service/api_v1/__init__.py diff --git a/files/packit.wsgi b/files/packit.wsgi deleted file mode 100644 index 28641e7be..000000000 --- a/files/packit.wsgi +++ /dev/null @@ -1 +0,0 @@ -from packit_service.service.app import application # noqa: F401 diff --git a/files/run_httpd.sh b/files/run_httpd.sh index 04a4a5f4e..53643b9e0 100755 --- a/files/run_httpd.sh +++ b/files/run_httpd.sh @@ -20,22 +20,27 @@ if [[ $n -eq $ATTEMPTS ]]; then fi export PACKIT_SERVICE_CONFIG="${HOME}/.config/packit-service.yaml" -SERVER_NAME=$(sed -nr 's/^server_name: ([^:]+)(:([0-9]+))?$/\1/p' "$PACKIT_SERVICE_CONFIG") HTTPS_PORT=$(sed -nr 's/^server_name: ([^:]+)(:([0-9]+))?$/\3/p' "$PACKIT_SERVICE_CONFIG") -# See "mod_wsgi-express-3 start-server --help" for details on -# these options, and the configuration documentation of mod_wsgi: -# https://modwsgi.readthedocs.io/en/master/configuration.html -exec mod_wsgi-express-3 start-server \ - --access-log \ - --log-to-terminal \ - --http2 \ - --https-port "${HTTPS_PORT:-8443}" \ - --ssl-certificate-file /secrets/fullchain.pem \ - --ssl-certificate-key-file /secrets/privkey.pem \ - --server-name "${SERVER_NAME}" \ - --processes 2 \ - --restart-interval 28800 \ - --graceful-timeout 15 \ - --locale "C.UTF-8" \ - /usr/share/packit/packit.wsgi +DEPLOYMENT=${DEPLOYMENT:-dev} + +# Gunicorn is recommended for prod deployment, because it's more robust and scalable +if [[ "${DEPLOYMENT}" == "prod" || "${DEPLOYMENT}" == "stg" ]]; then + echo "Running Gunicorn with Uvicorn workers" + exec gunicorn -k uvicorn.workers.UvicornWorker \ + -b 0.0.0.0:"${HTTPS_PORT:-8443}" \ + --access-logfile - \ + --log-level debug \ + --certfile /secrets/fullchain.pem \ + --keyfile /secrets/privkey.pem \ + "packit_service.service.app:app" +else + echo "Running Uvicorn in development mode" + exec uvicorn packit_service.service.app:app \ + --host 0.0.0.0 \ + --port "${HTTPS_PORT:-8443}" \ + --log-level debug \ + --ssl-certfile /secrets/fullchain.pem \ + --ssl-keyfile /secrets/privkey.pem \ + --reload +fi diff --git a/files/tasks/httpd.yaml b/files/tasks/httpd.yaml index e048ae712..a4dfb6031 100644 --- a/files/tasks/httpd.yaml +++ b/files/tasks/httpd.yaml @@ -1,11 +1,6 @@ # Copyright Contributors to the Packit project. # SPDX-License-Identifier: MIT -- name: Copy packit.wsgi file - ansible.builtin.copy: - src: packit.wsgi - dest: /usr/share/packit/packit.wsgi - mode: 0664 - name: Copy run_httpd.sh ansible.builtin.copy: src: run_httpd.sh diff --git a/packit_service/sentry_integration.py b/packit_service/sentry_integration.py index 83d795d29..fba875ca9 100644 --- a/packit_service/sentry_integration.py +++ b/packit_service/sentry_integration.py @@ -36,12 +36,14 @@ def configure_sentry( celery_integration: bool = False, flask_integration: bool = False, sqlalchemy_integration: bool = False, + fastapi_integration: bool = False, ) -> None: logger.debug( f"Setup sentry for {runner_type}: " f"celery_integration={celery_integration}, " f"flask_integration={flask_integration}, " - f"sqlalchemy_integration={sqlalchemy_integration}", + f"sqlalchemy_integration={sqlalchemy_integration}, " + f"fastapi_integration={fastapi_integration}", ) secret_key = getenv("SENTRY_SECRET") @@ -71,6 +73,12 @@ def configure_sentry( integrations.append(SqlalchemyIntegration()) + if fastapi_integration: + # https://docs.sentry.io/platforms/python/integrations/fastapi/ + from sentry_sdk.integrations.fastapi import FastApiIntegration + + integrations.append(FastApiIntegration()) + # https://docs.sentry.io/platforms/python/guides/logging/ sentry_logging = LoggingIntegration( level=logging.DEBUG, # Log everything, from DEBUG and above diff --git a/packit_service/service/api/__init__.py b/packit_service/service/api/__init__.py index b1e345e09..bf40bef6e 100644 --- a/packit_service/service/api/__init__.py +++ b/packit_service/service/api/__init__.py @@ -23,7 +23,7 @@ from packit_service.service.api.webhooks import ns as webhooks_ns # https://flask-restplus.readthedocs.io/en/stable/scaling.html -blueprint = Blueprint("api", __name__, url_prefix="/api") +blueprint = Blueprint("api", __name__) api = Api( app=blueprint, version="1.0", diff --git a/packit_service/service/api_v1/__init__.py b/packit_service/service/api_v1/__init__.py new file mode 100644 index 000000000..d3a3aa7c4 --- /dev/null +++ b/packit_service/service/api_v1/__init__.py @@ -0,0 +1,4 @@ +# Copyright Contributors to the Packit project. +# SPDX-License-Identifier: MIT + +routers = [] diff --git a/packit_service/service/app.py b/packit_service/service/app.py index c981b81e7..207473a78 100644 --- a/packit_service/service/app.py +++ b/packit_service/service/app.py @@ -2,9 +2,13 @@ # SPDX-License-Identifier: MIT import logging +import sys from os import getenv from socket import gaierror +from fastapi import FastAPI +from fastapi import __version__ as fastapi_version +from fastapi.middleware.wsgi import WSGIMiddleware from flask import Flask # Mypy errors out with Module 'flask' has no attribute '__version__'. @@ -14,40 +18,36 @@ from flask_cors import CORS from flask_restx import __version__ as restx_version from flask_talisman import Talisman -from lazy_object_proxy import Proxy from packit.utils import set_logging -from prometheus_client import make_wsgi_app as prometheus_app +from prometheus_client import make_asgi_app +from starlette.middleware.cors import CORSMiddleware +from starlette.responses import RedirectResponse from syslog_rfc5424_formatter import RFC5424Formatter -from werkzeug.middleware.dispatcher import DispatcherMiddleware from packit_service import __version__ as ps_version from packit_service.config import ServiceConfig from packit_service.sentry_integration import configure_sentry from packit_service.service.api import blueprint +from packit_service.service.api_v1 import routers from packit_service.utils import log_package_versions set_logging(logger_name="packit_service", level=logging.DEBUG) -def get_flask_application(): +def setup_logging_and_sentry(): configure_sentry( runner_type="packit-service", celery_integration=True, sqlalchemy_integration=True, flask_integration=True, + fastapi_integration=True, ) - app = Flask(__name__) - app.register_blueprint(blueprint) - service_config = ServiceConfig.get_service_config() - # https://flask.palletsprojects.com/en/1.1.x/config/#SERVER_NAME - # also needs to contain port if it's not 443 - app.config["SERVER_NAME"] = service_config.server_name - app.config["PREFERRED_URL_SCHEME"] = "https" - if getenv("DEPLOYMENT") in ("dev", "stg"): - app.config["DEBUG"] = True - - app.logger.setLevel(logging.DEBUG) logger = logging.getLogger("packit_service") + logger.setLevel(logging.DEBUG) + + console_handler = logging.StreamHandler(sys.stdout) + console_handler.setLevel(logging.DEBUG) + logger.addHandler(console_handler) syslog_host = getenv("SYSLOG_HOST", "fluentd") syslog_port = int(getenv("SYSLOG_PORT", 5140)) @@ -62,46 +62,108 @@ def get_flask_application(): handler.setFormatter(RFC5424Formatter(msgid=project)) logger.addHandler(handler) - logger.info( - f"server name = {service_config.server_name}, all HTTP requests need to use this URL!", - ) - package_versions = [ ("Flask", flask_version), ("Flask RestX", restx_version), + ("FastAPI", fastapi_version), ("Packit Service", ps_version), ] log_package_versions(package_versions) + return logger + + +def get_flask_application(): + app = Flask(__name__) + app.register_blueprint(blueprint) + service_config = ServiceConfig.get_service_config() + # https://flask.palletsprojects.com/en/1.1.x/config/#SERVER_NAME + # also needs to contain port if it's not 443 + # TODO local deployment fails without uncommenting + # app.config["SERVER_NAME"] = service_config.server_name + app.config["PREFERRED_URL_SCHEME"] = "https" + if getenv("DEPLOYMENT") in ("dev", "stg"): + app.config["DEBUG"] = True + + app.logger.setLevel(logging.DEBUG) + + logger.info( + f"server name = {service_config.server_name}, all HTTP requests need to use this URL!", + ) # no need to thank me, just buy me a beer logger.debug(f"URL map = {app.url_map}") return app -packit_as_a_service = Proxy(get_flask_application) +logger = setup_logging_and_sentry() -CORS(packit_as_a_service) +flask_app = get_flask_application() -INLINE = [ - "'unsafe-inline'", - "'self'", -] +CORS(flask_app) +INLINE = ["'unsafe-inline'", "'self'"] +# https://github.com/wntrblm/flask-talisman#options +# https://infosec.mozilla.org/guidelines/web_security#implementation-notes Talisman( - packit_as_a_service, - # https://github.com/wntrblm/flask-talisman#options - # https://infosec.mozilla.org/guidelines/web_security#implementation-notes + flask_app, content_security_policy={ "default-src": "'self'", "object-src": "'none'", "img-src": ["'self'", "data:"], - # https://github.com/python-restx/flask-restx/issues/252 "style-src": INLINE, "script-src": INLINE, }, ) -# Make Prometheus Client serve the /metrics endpoint -application = DispatcherMiddleware(packit_as_a_service, {"/metrics": prometheus_app()}) +app = FastAPI( + title="Packit Service API", + version="1.0.0", + docs_url="/v1/docs", + redoc_url="/v1/redoc", + openapi_url="/v1/openapi.json", +) + +# CORS settings +app.add_middleware( + CORSMiddleware, + allow_origins=["*"], + allow_credentials=True, + allow_methods=["*"], + allow_headers=["*"], +) + +# mount Flask inside FastAPI +# https://fastapi.tiangolo.com/advanced/wsgi/ +app.mount("/api", app=WSGIMiddleware(flask_app)) + + +# Prometheus metrics +# https://prometheus.github.io/client_python/exporting/http/fastapi-gunicorn/ +app.mount("/metrics", make_asgi_app()) + +# mount the endpoints +for router in routers: + app.include_router(router, prefix="/v1") + + +@app.middleware("http") +async def security_and_https_middleware(request, call_next): + if request.url.scheme != "https": + url = request.url.replace(scheme="https", port=443) + return RedirectResponse(url, status_code=301) + + response = await call_next(request) + response.headers["Content-Security-Policy"] = ( + "default-src 'self'; " + "object-src 'none'; " + "img-src 'self' data:; " + # Swagger and ReDoc don't work without these + "style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; " + "script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; " + "worker-src 'self' blob:" + ) + + return response + # With the code below, you can debug ALL requests coming to flask # @application.before_request From 826d5ccedb2cafe5ef6547a5e498bf2b57fc988a Mon Sep 17 00:00:00 2001 From: Laura Barcziova Date: Mon, 10 Mar 2025 15:27:24 +0100 Subject: [PATCH 2/4] Introduce FastAPI healthz and system endpoints --- packit_service/service/api_v1/__init__.py | 5 +- packit_service/service/api_v1/healthz.py | 12 +++++ packit_service/service/api_v1/system.py | 60 +++++++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 packit_service/service/api_v1/healthz.py create mode 100644 packit_service/service/api_v1/system.py diff --git a/packit_service/service/api_v1/__init__.py b/packit_service/service/api_v1/__init__.py index d3a3aa7c4..fb8a68bb7 100644 --- a/packit_service/service/api_v1/__init__.py +++ b/packit_service/service/api_v1/__init__.py @@ -1,4 +1,7 @@ # Copyright Contributors to the Packit project. # SPDX-License-Identifier: MIT -routers = [] +from .healthz import router as healthz_router +from .system import router as system_router + +routers = [healthz_router, system_router] diff --git a/packit_service/service/api_v1/healthz.py b/packit_service/service/api_v1/healthz.py new file mode 100644 index 000000000..cf6bf0897 --- /dev/null +++ b/packit_service/service/api_v1/healthz.py @@ -0,0 +1,12 @@ +# Copyright Contributors to the Packit project. +# SPDX-License-Identifier: MIT + +from fastapi import APIRouter + +router = APIRouter() + + +@router.get("/healthz") +async def health_check() -> dict: + """Health check""" + return {"status": "ok"} diff --git a/packit_service/service/api_v1/system.py b/packit_service/service/api_v1/system.py new file mode 100644 index 000000000..e304ecf96 --- /dev/null +++ b/packit_service/service/api_v1/system.py @@ -0,0 +1,60 @@ +# Copyright Contributors to the Packit project. +# SPDX-License-Identifier: MIT + +import re +from logging import getLogger +from typing import Optional + +import ogr +import packit +import specfile +from fastapi import APIRouter +from pydantic import BaseModel +from setuptools_scm import get_version + +import packit_service + +logger = getLogger("packit_service") + +router = APIRouter() + + +def get_commit_from_version(version) -> Optional[str]: + """ + Version can look like this: + 0.76.0.post18+g116edc5 + 0.1.dev1+gc03b1bd.d20230615 + 0.18.0.post4+g28cb117 + 0.45.1.dev2+g3b0fc3b + + The 7 characters after the "+g" is the short version of the git commit hash. + """ + if matches := re.search(r"\+g([A-Za-z0-9]{7})", version): + return matches.groups()[0] + return None + + +class PackageVersionResponse(BaseModel): + commit: Optional[str] + version: str + + +@router.get("/system") +async def get_system_information() -> dict[str, PackageVersionResponse]: + """System information""" + packages_and_versions = {project: project.__version__ for project in [ogr, specfile, packit]} + # packit_service might not be installed (i.e. when running locally) + # so it's treated differently + packages_and_versions[packit_service] = packit_service.__version__ or get_version( + root="..", + relative_to=packit_service.__file__, + ) + + return { + project.__name__: PackageVersionResponse( + commit=get_commit_from_version(version), + version=version, + ) + for project, version in packages_and_versions.items() + if version + } From 0ee44f8aa401f6bf7ac93fb62f8fca318ceb88db Mon Sep 17 00:00:00 2001 From: Laura Barcziova Date: Tue, 11 Mar 2025 09:06:12 +0100 Subject: [PATCH 3/4] Reflect FastAPI changes in tests --- tests/unit/test_views.py | 4 +-- tests_openshift/service/conftest.py | 2 +- tests_openshift/service/test_api_v1.py | 34 ++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 tests_openshift/service/test_api_v1.py diff --git a/tests/unit/test_views.py b/tests/unit/test_views.py index fc0280b67..95b9f2026 100644 --- a/tests/unit/test_views.py +++ b/tests/unit/test_views.py @@ -16,7 +16,7 @@ SRPMBuildModel, ) from packit_service.service import urls -from packit_service.service.app import packit_as_a_service as application +from packit_service.service.app import flask_app as application from packit_service.service.urls import ( get_copr_build_info_url, get_srpm_build_info_url, @@ -118,7 +118,7 @@ def test_get_srpm_logs(client): def test_system_api(client): - response = client.get("/api/system") + response = client.get("/system") assert response.status_code == 200 response_data = response.json for package in ["ogr", "packit", "specfile", "packit_service"]: diff --git a/tests_openshift/service/conftest.py b/tests_openshift/service/conftest.py index f67fec04f..58dccadae 100644 --- a/tests_openshift/service/conftest.py +++ b/tests_openshift/service/conftest.py @@ -3,7 +3,7 @@ import pytest -from packit_service.service.app import packit_as_a_service as application +from packit_service.service.app import flask_app as application @pytest.fixture diff --git a/tests_openshift/service/test_api_v1.py b/tests_openshift/service/test_api_v1.py new file mode 100644 index 000000000..a3dfa61ba --- /dev/null +++ b/tests_openshift/service/test_api_v1.py @@ -0,0 +1,34 @@ +# Copyright Contributors to the Packit project. +# SPDX-License-Identifier: MIT + +from fastapi.testclient import TestClient + +from packit_service.service.app import app + +client = TestClient(app) + + +def test_healthz(): + response = client.get("/v1/healthz") + assert response.status_code == 200 + assert response.json() == {"status": "ok"} + + +def test_system(): + response = client.get("/v1/system") + assert response.status_code == 200 + data = response.json() + + expected_keys = {"ogr", "specfile", "packit", "packit_service"} + assert set(data.keys()) == expected_keys + + for key in expected_keys: + assert isinstance(data[key], dict) + assert "commit" in data[key] + assert "version" in data[key] + + +def test_meta(): + """Test meta info like headers.""" + response = client.get("/v1/healthz") + assert "default-src 'self'" in response.headers["Content-Security-Policy"] From 5cd7aeb8e9ddeaf4322df9b26629be5bc5bee76a Mon Sep 17 00:00:00 2001 From: Laura Barcziova Date: Tue, 11 Mar 2025 11:10:13 +0100 Subject: [PATCH 4/4] Rename run_httpd.sh and occurences We no longer run httpd. --- CONTRIBUTING.md | 6 +++--- docker-compose.yml | 6 +++--- docs/database/README.md | 2 +- files/docker/Dockerfile | 4 ++-- files/recipe.yaml | 2 +- files/{run_httpd.sh => run_server.sh} | 0 files/tasks/{httpd.yaml => server.yaml} | 6 +++--- packit_service/models.py | 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) rename files/{run_httpd.sh => run_server.sh} (100%) rename files/tasks/{httpd.yaml => server.yaml} (58%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e8c30da3a..6ab2a1f26 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -47,11 +47,11 @@ This repository contains [docker-compose.yml](./docker-compose.yml) for (can be also [used with podman](https://fedoramagazine.org/use-docker-compose-with-podman-to-orchestrate-containers-on-fedora)). Before you run it, we suggest that you open the file and read all the comments. You can also run only certain pieces of packit-service for local development -(e.g. worker, database or service/httpd). +(e.g. worker, database or http service). You also need to populate `secrets/packit/dev/` manually, for instructions see [deployment repo](https://github.com/packit/deployment/tree/main/secrets). -When you are running service/httpd and making requests to it, +When you are running http service and making requests to it, make sure that `server_name` configuration file in `packit-service.yaml` is set. ### tokman @@ -84,7 +84,7 @@ For this reason `docker-compose` needs access to ports lower than 1024: ### binding on other hosts If you are not binding the service on `localhost` -then you **need** to make requests to httpd using the hostname +then you **need** to make requests to http service using the hostname (which can be done by creating a new entry in `/etc/hosts` on your laptop) and you have to provide a route to that host. diff --git a/docker-compose.yml b/docker-compose.yml index 7204e385d..05bdcbe3f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -130,7 +130,7 @@ services: args: SOURCE_BRANCH: main image: quay.io/packit/packit-service:dev - command: /usr/bin/run_httpd.sh + command: /usr/bin/run_server.sh depends_on: - redis - postgres @@ -157,7 +157,7 @@ services: - ./secrets/packit/dev/fedora.keytab:/secrets/fedora.keytab:ro,z - ./secrets/packit/dev/fullchain.pem:/secrets/fullchain.pem:ro,z - ./secrets/packit/dev/privkey.pem:/secrets/privkey.pem:ro,z - - ./files/run_httpd.sh:/usr/bin/run_httpd.sh:ro,z + - ./files/run_server.sh:/usr/bin/run_server.sh:ro,z user: "1024" dashboard: @@ -172,7 +172,7 @@ services: # server_name: service.localhost # dashboard_url: https://dashboard.localhost:8443 image: quay.io/packit/dashboard:stg - command: /usr/bin/run_httpd.sh + command: /usr/bin/run_server.sh depends_on: - service ports: diff --git a/docs/database/README.md b/docs/database/README.md index 938176809..dc2521eae 100644 --- a/docs/database/README.md +++ b/docs/database/README.md @@ -10,7 +10,7 @@ the project which handles migrations and schema versioning for SQLAlchemy. To generate a migration script for your recent change you can use docker or more easily, with rootless podman, you can use our make target. -Both expect that the `alembic upgrade head` is run in [run_httpd.sh](../../files/run_httpd.sh) +Both expect that the `alembic upgrade head` is run in [run_server.sh](../../files/run_server.sh) during (packit-)service pod/container start. When modifying the migration manually, do not forget to update the downgrade diff --git a/files/docker/Dockerfile b/files/docker/Dockerfile index 0470cc9a7..4b8b51d6a 100644 --- a/files/docker/Dockerfile +++ b/files/docker/Dockerfile @@ -1,4 +1,4 @@ -# Image for the web service (httpd), for celery worker see files/docker/Dockerfile.worker +# Image for the web service, for celery worker see files/docker/Dockerfile.worker FROM quay.io/packit/base:fedora @@ -34,4 +34,4 @@ COPY alembic/ ./alembic/ EXPOSE 8443 -CMD ["/usr/bin/run_httpd.sh"] +CMD ["/usr/bin/run_server.sh"] diff --git a/files/recipe.yaml b/files/recipe.yaml index 8ea6da948..6aa8c97de 100644 --- a/files/recipe.yaml +++ b/files/recipe.yaml @@ -9,4 +9,4 @@ packit_service_path: /src tasks: - import_tasks: tasks/common.yaml - - import_tasks: tasks/httpd.yaml + - import_tasks: tasks/server.yaml diff --git a/files/run_httpd.sh b/files/run_server.sh similarity index 100% rename from files/run_httpd.sh rename to files/run_server.sh diff --git a/files/tasks/httpd.yaml b/files/tasks/server.yaml similarity index 58% rename from files/tasks/httpd.yaml rename to files/tasks/server.yaml index a4dfb6031..8f5a6f05d 100644 --- a/files/tasks/httpd.yaml +++ b/files/tasks/server.yaml @@ -1,8 +1,8 @@ # Copyright Contributors to the Packit project. # SPDX-License-Identifier: MIT -- name: Copy run_httpd.sh +- name: Copy run_server.sh ansible.builtin.copy: - src: run_httpd.sh - dest: /usr/bin/run_httpd.sh + src: run_server.sh + dest: /usr/bin/run_server.sh mode: 0775 diff --git a/packit_service/models.py b/packit_service/models.py index b7ae69865..0b5b1dc5b 100644 --- a/packit_service/models.py +++ b/packit_service/models.py @@ -95,7 +95,7 @@ def is_multi_threaded() -> bool: # fails to rollback you have to restart the workers so that they pick another session. singleton_session = Session() logger.debug("Going to use a single SQLAlchemy session.") -else: # service/httpd +else: # http service Session = scoped_session(Session) singleton_session = None