Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PEP 740: add provenance to simple API #16801

Merged
merged 20 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion requirements/main.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1988,7 +1988,6 @@ rfc8785==0.1.4 \
--hash=sha256:520d690b448ecf0703691c76e1a34a24ddcd4fc5bc41d589cb7c58ec651bcd48 \
--hash=sha256:e545841329fe0eee4f6a3b44e7034343100c12b4ec566dc06ca9735681deb4da
# via
# -r requirements/main.in
# sigstore
rich==13.9.1 \
--hash=sha256:097cffdf85db1babe30cc7deba5ab3a29e1b9885047dab24c57e9a7f8a9c1466 \
Expand Down
29 changes: 28 additions & 1 deletion tests/functional/api/test_simple.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@

from http import HTTPStatus

from ...common.db.packaging import FileFactory, ProjectFactory, ReleaseFactory
from ...common.db.packaging import (
FileFactory,
ProjectFactory,
ProvenanceFactory,
ReleaseFactory,
)


def test_simple_api_html(webtest):
Expand All @@ -34,3 +39,25 @@ def test_simple_api_detail(webtest):
assert resp.html.h1.string == f"Links for {project.normalized_name}"
# There should be a link for every file
assert len(resp.html.find_all("a")) == 2


def test_simple_api_has_provenance(webtest):
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project)
files = FileFactory.create_batch(2, release=release, packagetype="bdist_wheel")

for file in files:
ProvenanceFactory.create(file=file)

resp = webtest.get(f"/simple/{project.normalized_name}/", status=HTTPStatus.OK)
links = resp.html.find_all("a")

for file in files:
link = next(link for link in links if link.text == file.filename)
provenance_url = link.get("data-provenance")
assert provenance_url is not None
woodruffw marked this conversation as resolved.
Show resolved Hide resolved

assert provenance_url == (
f"http://localhost/integrity/{file.release.project.normalized_name}/"
f"{file.release.version}/{file.filename}/provenance"
)
11 changes: 11 additions & 0 deletions tests/functional/forklift/test_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,3 +329,14 @@ def test_provenance_upload(webtest):
assert len(attestations) == 1
attestation = attestations[0]
assert attestation == json.loads(attestation_contents)

# While we needed to be authenticated to upload a project, this is no longer
# required to view it.
webtest.authorization = None
expected_filename = "sampleproject-3.0.0.tar.gz"

response = webtest.get(
f"/integrity/{project.name}/3.0.0/{expected_filename}/provenance",
status=HTTPStatus.OK,
)
assert response.json == project.releases[0].files[0].provenance.provenance
101 changes: 101 additions & 0 deletions tests/unit/api/test_simple.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from pyramid.httpexceptions import HTTPMovedPermanently
from pyramid.testing import DummyRequest

from tests.common.db.oidc import GitHubPublisherFactory
from warehouse.api import simple
from warehouse.packaging.utils import API_VERSION, _valid_simple_detail_context

Expand All @@ -26,6 +27,7 @@
FileFactory,
JournalEntryFactory,
ProjectFactory,
ProvenanceFactory,
ReleaseFactory,
)

Expand Down Expand Up @@ -298,6 +300,7 @@ def test_with_files_no_serial(self, db_request, content_type, renderer_override)
"upload-time": f.upload_time.isoformat() + "Z",
"data-dist-info-metadata": False,
"core-metadata": False,
"provenance": None,
}
for f in files
],
Expand Down Expand Up @@ -349,6 +352,7 @@ def test_with_files_with_serial(self, db_request, content_type, renderer_overrid
"upload-time": f.upload_time.isoformat() + "Z",
"data-dist-info-metadata": False,
"core-metadata": False,
"provenance": None,
}
for f in files
],
Expand Down Expand Up @@ -445,6 +449,7 @@ def test_with_files_with_version_multi_digit(
if f.metadata_file_sha256_digest is not None
else False
),
"provenance": None,
}
for f in files
],
Expand Down Expand Up @@ -489,6 +494,102 @@ def test_with_files_quarantined_omitted_from_index(
if renderer_override is not None:
assert db_request.override_renderer == renderer_override

@pytest.mark.parametrize(
("content_type", "renderer_override"),
CONTENT_TYPE_PARAMS,
)
def test_with_files_varying_provenance(
self,
db_request,
integrity_service,
dummy_attestation,
content_type,
renderer_override,
):
db_request.accept = content_type
db_request.oidc_publisher = GitHubPublisherFactory.create()

project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0.0")

# wheel with provenance, sdist with no provenance
wheel = FileFactory.create(
release=release,
filename=f"{project.name}-1.0.0.whl",
packagetype="bdist_wheel",
metadata_file_sha256_digest="deadbeefdeadbeefdeadbeefdeadbeef",
)

provenance = ProvenanceFactory.create(file=wheel)
assert wheel.provenance is not None
assert wheel.provenance == provenance
woodruffw marked this conversation as resolved.
Show resolved Hide resolved

sdist = FileFactory.create(
release=release,
filename=f"{project.name}-1.0.0.tar.gz",
packagetype="sdist",
)

files = [sdist, wheel]

urls_iter = (f"/file/{f.filename}" for f in files)
provenance_iter = (
(
f"/integrity/{f.release.project.normalized_name}/"
f"{f.release.version}/{f.filename}/provenance"
)
for f in [wheel]
)
db_request.matchdict["name"] = project.normalized_name
db_request.route_url = lambda r, **kw: (
next(urls_iter) if r == "packaging.file" else next(provenance_iter)
woodruffw marked this conversation as resolved.
Show resolved Hide resolved
)
user = UserFactory.create()
je = JournalEntryFactory.create(name=project.name, submitted_by=user)

context = {
"meta": {"_last-serial": je.id, "api-version": API_VERSION},
"name": project.normalized_name,
"versions": ["1.0.0"],
"files": [
{
"filename": f.filename,
"url": f"/file/{f.filename}",
"hashes": {"sha256": f.sha256_digest},
"requires-python": f.requires_python,
"yanked": False,
"size": f.size,
"upload-time": f.upload_time.isoformat() + "Z",
"data-dist-info-metadata": (
{"sha256": "deadbeefdeadbeefdeadbeefdeadbeef"}
if f.metadata_file_sha256_digest is not None
else False
),
"core-metadata": (
{"sha256": "deadbeefdeadbeefdeadbeefdeadbeef"}
if f.metadata_file_sha256_digest is not None
else False
),
"provenance": (
(
f"/integrity/{f.release.project.normalized_name}/"
f"{f.release.version}/{f.filename}/provenance"
)
if f.provenance is not None
else None
),
}
for f in files
],
"alternate-locations": [],
}
context = _update_context(context, content_type, renderer_override)

assert simple.simple_detail(project, db_request) == context

if renderer_override is not None:
assert db_request.override_renderer == renderer_override


def _update_context(context, content_type, renderer_override):
if renderer_override != "json" or content_type in [
Expand Down
12 changes: 11 additions & 1 deletion warehouse/packaging/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from warehouse.packaging.interfaces import ISimpleStorage
from warehouse.packaging.models import File, LifecycleStatus, Project, Release

API_VERSION = "1.2"
API_VERSION = "1.3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough yet about incrementing this value and what that does.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from the PEP itself:

The api-version SHALL specify version 1.3 or later.

(Via https://peps.python.org/pep-0740/#json-based-simple-api)

In this context, all incrementing it does is signal to a client that the index is now serving PEP 740 objects. Clients are expected to handle this gracefully, since api-version is intended to be a semantic version.

(I can find a cite for that last claim if you'd like, although I'd have to go digging. But for context, this was just bumped a few weeks ago from 1.1 to 1.2 with no client breakage due to PEP 708.)



def _simple_index(request, serial):
Expand Down Expand Up @@ -101,6 +101,16 @@ def _simple_detail(project, request):
if file.metadata_file_sha256_digest
else False
),
"provenance": (
request.route_url(
"integrity.provenance",
project_name=project.normalized_name,
release=file.release.version,
filename=file.filename,
)
if file.provenance
else None
),
}
for file in files
],
Expand Down
2 changes: 1 addition & 1 deletion warehouse/templates/api/simple/detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<body>
<h1>Links for {{ name }}</h1>
{% for file in files -%}
<a href="{{ file.url }}#sha256={{ file.hashes.sha256 }}" {% if file.get('requires-python') %}data-requires-python="{{ file['requires-python'] }}" {% endif %}{% if file.yanked %}data-yanked="{% if file.yanked is string %}{{ file.yanked }}{% endif %}" {% endif %}{% if file['core-metadata'] %}data-dist-info-metadata="sha256={{ file['core-metadata']['sha256'] }}" data-core-metadata="sha256={{ file['core-metadata']['sha256'] }}"{% endif %}>{{ file.filename }}</a><br />
<a href="{{ file.url }}#sha256={{ file.hashes.sha256 }}" {% if file.get('requires-python') %}data-requires-python="{{ file['requires-python'] }}" {% endif %}{% if file.yanked %}data-yanked="{% if file.yanked is string %}{{ file.yanked }}{% endif %}" {% endif %}{% if file['core-metadata'] %}data-dist-info-metadata="sha256={{ file['core-metadata']['sha256'] }}" data-core-metadata="sha256={{ file['core-metadata']['sha256'] }}"{% endif %}{% if file.get('provenance') %}data-provenance="{{ file['provenance'] }}" {% endif %}>{{ file.filename }}</a><br />
{% endfor -%}
</body>
</html>
Expand Down