launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29459
[Merge] ~cjwatson/lp-archive:virtual-hosts into lp-archive:main
Colin Watson has proposed merging ~cjwatson/lp-archive:virtual-hosts into lp-archive:main.
Commit message:
Split primary and PPA snapshots into separate virtual hosts
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/lp-archive/+git/lp-archive/+merge/433939
This is controlled by the `LAYOUTS` array of tables in `config.toml` (capitalized due to Flask's requirements), which might look something like this:
[[LAYOUTS]]
host = "snapshot.ubuntu.test"
purpose = "primary"
[[LAYOUTS]]
host = "snapshot.ppa.test"
purpose = "ppa"
This lets us deploy something that looks a little more like the current layout for our archive servers.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lp-archive:virtual-hosts into lp-archive:main.
diff --git a/lp_archive/__init__.py b/lp_archive/__init__.py
index 0a2e695..393afdf 100644
--- a/lp_archive/__init__.py
+++ b/lp_archive/__init__.py
@@ -16,14 +16,15 @@ from lp_archive.cache import cache
def create_app(test_config: dict[str, Any] | None = None) -> Flask:
- app = Flask(__name__)
+ app = Flask(__name__, static_folder=None, host_matching=True)
if test_config is None:
with open("config.toml", "rb") as f:
app.config.from_mapping(tomllib.load(f))
else:
app.config.from_mapping(test_config)
- app.url_map.converters["archive"] = routing.ArchiveConverter
- app.register_blueprint(root.bp)
- app.register_blueprint(archive.bp)
+ app.url_map.converters["primary"] = routing.PrimaryArchiveConverter
+ app.url_map.converters["ppa"] = routing.PPAConverter
+ root.init_app(app)
+ archive.init_app(app)
cache.init_app(app)
return app
diff --git a/lp_archive/archive.py b/lp_archive/archive.py
index e3be81a..ab310d7 100644
--- a/lp_archive/archive.py
+++ b/lp_archive/archive.py
@@ -5,15 +5,13 @@
from xmlrpc.client import Fault, ServerProxy
-from flask import Blueprint, current_app, g, request
+from flask import Flask, current_app, g, request
from werkzeug.datastructures import WWWAuthenticate
from werkzeug.exceptions import Unauthorized
from werkzeug.wrappers import Response
from lp_archive.cache import cache
-bp = Blueprint("archive", __name__)
-
def get_archive_proxy() -> ServerProxy:
archive_proxy = getattr(g, "archive_proxy", None)
@@ -75,9 +73,6 @@ def check_auth(archive: str) -> None:
current_app.logger.info("%s: Authorized.", log_prefix)
-# The exact details of the URLs used here should be regarded as a proof of
-# concept for now.
-@bp.route("/<archive:archive>/<path:path>")
def translate(archive: str, path: str) -> tuple[str, int, dict[str, str]]:
check_auth(archive)
try:
@@ -91,7 +86,16 @@ def translate(archive: str, path: str) -> tuple[str, int, dict[str, str]]:
return "", 307, {"Location": url}
-@bp.after_request
def add_headers(response: Response) -> Response:
response.headers["Vary"] = "Authorization"
return response
+
+
+def init_app(app: Flask) -> None:
+ for layout in app.config.get("LAYOUTS", []):
+ app.add_url_rule(
+ f"/<{layout['purpose']}:archive>/<path:path>",
+ host=layout["host"],
+ view_func=translate,
+ )
+ app.after_request(add_headers)
diff --git a/lp_archive/root.py b/lp_archive/root.py
index 5716a6d..f0304b5 100644
--- a/lp_archive/root.py
+++ b/lp_archive/root.py
@@ -3,11 +3,13 @@
"""API views for the Launchpad archive service."""
-from flask import Blueprint
+from flask import Flask
-bp = Blueprint("root", __name__)
-
-@bp.route("/")
def root() -> tuple[str, dict[str, str]]:
return "Launchpad archive service.\n", {"Content-Type": "text/plain"}
+
+
+def init_app(app: Flask) -> None:
+ for layout in app.config.get("LAYOUTS", []):
+ app.add_url_rule("/", host=layout["host"], view_func=root)
diff --git a/lp_archive/routing.py b/lp_archive/routing.py
index a3be94d..0b140ce 100644
--- a/lp_archive/routing.py
+++ b/lp_archive/routing.py
@@ -6,18 +6,21 @@
from werkzeug.routing import BaseConverter
-class ArchiveConverter(BaseConverter):
- """Match an archive reference.
+class PrimaryArchiveConverter(BaseConverter):
+ """Match a primary archive reference.
See `lp.soyuz.model.archive.Archive.reference` in Launchpad.
"""
- # This doesn't currently support the partner/copy archive reference
- # syntax (distribution/archive), since it's hard to avoid that being
- # ambiguous when parsing URLs (compare with the primary archive
- # reference syntax).
- #
- # PPA: ~[^/]+/[^/]+/[^/]+ (~owner/distribution/archive)
- # Primary: [^~+][^/]* (distribution)
- regex = r"~[^/]+/[^/]+/[^/]+|[^~+][^/]*"
+ regex = r"[^~+/][^/]*"
+
+
+class PPAConverter(BaseConverter):
+ """Match a PPA reference.
+
+ See `lp.soyuz.model.archive.Archive.reference` in Launchpad.
+ """
+
+ # ~owner/distribution/archive
+ regex = r"~[^/]+/[^/]+/[^/]+"
part_isolating = False
diff --git a/tests/conftest.py b/tests/conftest.py
index 31e1267..f781440 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -8,7 +8,16 @@ from lp_archive import create_app
@pytest.fixture()
def app():
- app = create_app({"CACHE_TYPE": "SimpleCache", "TESTING": True})
+ app = create_app(
+ {
+ "CACHE_TYPE": "SimpleCache",
+ "TESTING": True,
+ "LAYOUTS": [
+ {"host": "snapshot.ubuntu.test", "purpose": "primary"},
+ {"host": "snapshot.ppa.test", "purpose": "ppa"},
+ ],
+ }
+ )
yield app
diff --git a/tests/test_archive.py b/tests/test_archive.py
index 7bdfccb..0ac6439 100644
--- a/tests/test_archive.py
+++ b/tests/test_archive.py
@@ -60,7 +60,9 @@ def archive_proxy(app):
def test_auth_failed(client, archive_proxy):
response = client.get(
- "/~user/ubuntu/private/dists/focal/InRelease", auth=("user", "secret")
+ "/~user/ubuntu/private/dists/focal/InRelease",
+ auth=("user", "secret"),
+ headers=[("Host", "snapshot.ppa.test")],
)
assert response.status_code == 401
assert (
@@ -77,6 +79,7 @@ def test_auth_not_found(client, archive_proxy):
response = client.get(
"/~user/ubuntu/nonexistent/dists/focal/InRelease",
auth=("user", "secret"),
+ headers=[("Host", "snapshot.ppa.test")],
)
assert response.status_code == 401
assert (
@@ -93,6 +96,7 @@ def test_auth_positive_cached(client, archive_proxy):
response = client.get(
"/~user/ubuntu/authorized/dists/focal/InRelease",
auth=("user", "secret"),
+ headers=[("Host", "snapshot.ppa.test")],
)
assert response.status_code == 307
assert archive_proxy.call_log == [
@@ -103,6 +107,7 @@ def test_auth_positive_cached(client, archive_proxy):
response = client.get(
"/~user/ubuntu/authorized/dists/focal/InRelease",
auth=("user", "secret"),
+ headers=[("Host", "snapshot.ppa.test")],
)
assert response.status_code == 307
assert archive_proxy.call_log == [
@@ -112,7 +117,9 @@ def test_auth_positive_cached(client, archive_proxy):
def test_auth_negative_not_cached(client, archive_proxy):
response = client.get(
- "/~user/ubuntu/private/dists/focal/InRelease", auth=("user", "secret")
+ "/~user/ubuntu/private/dists/focal/InRelease",
+ auth=("user", "secret"),
+ headers=[("Host", "snapshot.ppa.test")],
)
assert response.status_code == 401
assert archive_proxy.call_log == [
@@ -120,7 +127,9 @@ def test_auth_negative_not_cached(client, archive_proxy):
]
archive_proxy.call_log = []
response = client.get(
- "/~user/ubuntu/private/dists/focal/InRelease", auth=("user", "secret")
+ "/~user/ubuntu/private/dists/focal/InRelease",
+ auth=("user", "secret"),
+ headers=[("Host", "snapshot.ppa.test")],
)
assert response.status_code == 401
assert archive_proxy.call_log == [
@@ -129,7 +138,10 @@ def test_auth_negative_not_cached(client, archive_proxy):
def test_translate(client, archive_proxy):
- response = client.get("/ubuntu/dists/focal/InRelease")
+ response = client.get(
+ "/ubuntu/dists/focal/InRelease",
+ headers=[("Host", "snapshot.ubuntu.test")],
+ )
assert response.status_code == 307
assert response.headers["Location"] == "http://librarian.example.org/1";
assert response.headers["Vary"] == "Authorization"
@@ -140,7 +152,9 @@ def test_translate(client, archive_proxy):
def test_translate_not_found(client, archive_proxy):
- response = client.get("/ubuntu/nonexistent")
+ response = client.get(
+ "/ubuntu/nonexistent", headers=[("Host", "snapshot.ubuntu.test")]
+ )
assert response.status_code == 404
assert response.headers["Content-Type"] == "text/plain"
assert response.headers["Vary"] == "Authorization"
@@ -152,7 +166,9 @@ def test_translate_not_found(client, archive_proxy):
def test_translate_oops(client, archive_proxy):
- response = client.get("/ubuntu/oops")
+ response = client.get(
+ "/ubuntu/oops", headers=[("Host", "snapshot.ubuntu.test")]
+ )
assert response.status_code == 500
assert response.headers["Content-Type"] == "text/plain"
assert response.headers["Vary"] == "Authorization"
diff --git a/tests/test_root.py b/tests/test_root.py
index a9ded99..50656de 100644
--- a/tests/test_root.py
+++ b/tests/test_root.py
@@ -3,6 +3,6 @@
def test_root(client):
- response = client.get("/")
+ response = client.get("/", headers=[("Host", "snapshot.ubuntu.test")])
assert response.data == b"Launchpad archive service.\n"
assert response.headers["Content-Type"] == "text/plain"
diff --git a/tests/test_routing.py b/tests/test_routing.py
index e11ae4e..a97c789 100644
--- a/tests/test_routing.py
+++ b/tests/test_routing.py
@@ -5,37 +5,75 @@ from flask import url_for
def test_primary(app, client):
- @app.route("/+test/<archive:archive>")
+ @app.route("/+test/<primary:archive>", host="snapshot.ubuntu.test")
def index(archive):
return archive
- response = client.get("/+test/ubuntu")
+ response = client.get(
+ "/+test/ubuntu", headers=[("Host", "snapshot.ubuntu.test")]
+ )
assert response.status_code == 200
assert response.data == b"ubuntu"
with app.test_request_context():
- assert url_for("index", archive="ubuntu") == "/+test/ubuntu"
+ assert (
+ url_for("index", archive="ubuntu")
+ == "http://snapshot.ubuntu.test/+test/ubuntu";
+ )
+
+
+def test_primary_invalid(app, client):
+ @app.route("/+test/<primary:archive>", host="snapshot.ubuntu.test")
+ def index(archive): # pragma: no cover
+ return archive
+
+ assert (
+ client.get(
+ "/+test/~owner/ubuntu/ppa",
+ headers=[("Host", "snapshot.ubuntu.test")],
+ ).status_code
+ == 404
+ )
+ assert (
+ client.get(
+ "/+test/~owner/ubuntu", headers=[("Host", "snapshot.ubuntu.test")]
+ ).status_code
+ == 404
+ )
def test_ppa(app, client):
- @app.route("/+test/<archive:archive>")
+ @app.route("/+test/<ppa:archive>", host="ppa.ubuntu.test")
def index(archive):
return archive
- response = client.get("/+test/~owner/ubuntu/ppa")
+ response = client.get(
+ "/+test/~owner/ubuntu/ppa", headers=[("Host", "ppa.ubuntu.test")]
+ )
assert response.status_code == 200
assert response.data == b"~owner/ubuntu/ppa"
with app.test_request_context():
assert (
url_for("index", archive="~owner/ubuntu/ppa")
- == "/+test/~owner/ubuntu/ppa"
+ == "http://ppa.ubuntu.test/+test/~owner/ubuntu/ppa";
)
-def test_invalid_archive(app, client):
- @app.route("/+test/<archive:archive>")
+def test_ppa_invalid(app, client):
+ @app.route("/+test/<ppa:archive>", host="ppa.ubuntu.test")
def index(archive): # pragma: no cover
return archive
- assert client.get("/+test/~owner/ubuntu").status_code == 404
+ assert (
+ client.get(
+ "/+test/ubuntu", headers=[("Host", "ppa.ubuntu.test")]
+ ).status_code
+ == 404
+ )
+ assert (
+ client.get(
+ "/+test/~owner/ubuntu", headers=[("Host", "ppa.ubuntu.test")]
+ ).status_code
+ == 404
+ )
