launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #27974
[Merge] ~cjwatson/launchpad:private-revision-status-artifacts into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:private-revision-status-artifacts into launchpad:master with ~cjwatson/launchpad:revision-status-permissions as a prerequisite.
Commit message:
Restrict artifacts for private repositories
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/414201
Revision status artifacts for private repositories must be stored in the restricted librarian, in order that they can only be fetched with a suitable token.
I found I needed to do a reasonable amount of test refactoring first, which is in a separate commit to make it easier to review.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:private-revision-status-artifacts into launchpad:master.
diff --git a/lib/lp/code/interfaces/revisionstatus.py b/lib/lp/code/interfaces/revisionstatus.py
index 8e4da9f..20c6a5a 100644
--- a/lib/lp/code/interfaces/revisionstatus.py
+++ b/lib/lp/code/interfaces/revisionstatus.py
@@ -46,7 +46,10 @@ from lp.code.enums import (
RevisionStatusResult,
)
from lp.services.auth.enums import AccessTokenScope
-from lp.services.fields import URIField
+from lp.services.fields import (
+ PublicPersonChoice,
+ URIField,
+ )
@error_status(http.client.UNAUTHORIZED)
@@ -63,6 +66,11 @@ class IRevisionStatusReportView(Interface):
id = Int(title=_("ID"), required=True, readonly=True)
+ creator = PublicPersonChoice(
+ title=_("Creator"), required=True, readonly=True,
+ vocabulary="ValidPersonOrTeam",
+ description=_("The person who created this report."))
+
date_created = exported(Datetime(
title=_("When the report was created."), required=True, readonly=True))
date_started = exported(Datetime(
diff --git a/lib/lp/code/model/revisionstatus.py b/lib/lp/code/model/revisionstatus.py
index 7d1f911..de9bfff 100644
--- a/lib/lp/code/model/revisionstatus.py
+++ b/lib/lp/code/model/revisionstatus.py
@@ -83,7 +83,8 @@ class RevisionStatusReport(StormBase):
lfa = getUtility(ILibraryFileAliasSet).create(
name=filename, size=len(log_data),
- file=io.BytesIO(log_data), contentType='text/plain')
+ file=io.BytesIO(log_data), contentType='text/plain',
+ restricted=self.git_repository.private)
getUtility(IRevisionStatusArtifactSet).new(
lfa, self, RevisionStatusArtifactType.LOG)
@@ -93,7 +94,8 @@ class RevisionStatusReport(StormBase):
"""See `IRevisionStatusReport`."""
lfa = getUtility(ILibraryFileAliasSet).create(
name=name, size=len(data), file=io.BytesIO(data),
- contentType='application/octet-stream')
+ contentType='application/octet-stream',
+ restricted=self.git_repository.private)
getUtility(IRevisionStatusArtifactSet).new(lfa, self, artifact_type)
def transitionToNewResult(self, result):
diff --git a/lib/lp/code/model/tests/test_revisionstatus.py b/lib/lp/code/model/tests/test_revisionstatus.py
index ac30636..6ce84c5 100644
--- a/lib/lp/code/model/tests/test_revisionstatus.py
+++ b/lib/lp/code/model/tests/test_revisionstatus.py
@@ -9,6 +9,8 @@ import io
from testtools.matchers import (
AnyMatch,
Equals,
+ GreaterThan,
+ Is,
MatchesSetwise,
MatchesStructure,
)
@@ -121,94 +123,119 @@ class TestRevisionStatusReport(TestCaseWithFactory):
class TestRevisionStatusReportWebservice(TestCaseWithFactory):
layer = LaunchpadFunctionalLayer
- def setUp(self):
- super().setUp()
- self.repository = self.factory.makeGitRepository()
- self.requester = self.repository.owner
- self.title = self.factory.getUniqueUnicode('report-title')
- self.commit_sha1 = hashlib.sha1(b"Some content").hexdigest()
- self.result_summary = "120/120 tests passed"
-
- self.report = self.factory.makeRevisionStatusReport(
- user=self.repository.owner, git_repository=self.repository,
- title=self.title, commit_sha1=self.commit_sha1,
- result_summary=self.result_summary,
- result=RevisionStatusResult.FAILED)
-
- with person_logged_in(self.requester):
- self.report_url = api_url(self.report)
-
+ def getWebservice(self, person, repository):
+ with person_logged_in(person):
secret, _ = self.factory.makeAccessToken(
- owner=self.requester, target=self.repository,
+ owner=person, target=repository,
scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS])
- self.webservice = webservice_for_person(
- self.requester, default_api_version="devel",
- access_token_secret=secret)
-
- def test_setLog(self):
+ return webservice_for_person(
+ person, default_api_version="devel", access_token_secret=secret)
+
+ def _test_setLog(self, private):
+ requester = self.factory.makePerson()
+ with person_logged_in(requester):
+ kwargs = {"owner": requester}
+ if private:
+ kwargs["information_type"] = InformationType.USERDATA
+ repository = self.factory.makeGitRepository(**kwargs)
+ report = self.factory.makeRevisionStatusReport(
+ git_repository=repository)
+ report_url = api_url(report)
+ webservice = self.getWebservice(requester, repository)
content = b'log_content_data'
- response = self.webservice.named_post(
- self.report_url, "setLog", log_data=io.BytesIO(content))
+ response = webservice.named_post(
+ report_url, "setLog", log_data=io.BytesIO(content))
self.assertEqual(200, response.status)
# A report may have multiple artifacts.
# We verify that the content we just submitted via API now
# matches one of the artifacts in the DB for the report.
- with person_logged_in(self.requester):
+ with person_logged_in(requester):
artifacts = list(getUtility(
- IRevisionStatusArtifactSet).findByReport(self.report))
+ IRevisionStatusArtifactSet).findByReport(report))
self.assertThat(artifacts, AnyMatch(
MatchesStructure(
- report=Equals(self.report),
+ report=Equals(report),
library_file=MatchesStructure(
content=MatchesStructure.byEquality(
sha256=hashlib.sha256(content).hexdigest()),
filename=Equals(
- "%s-%s.txt" % (self.title, self.commit_sha1)),
- mimetype=Equals("text/plain")),
+ "%s-%s.txt" % (report.title, report.commit_sha1)),
+ mimetype=Equals("text/plain"),
+ restricted=Is(private)),
artifact_type=Equals(RevisionStatusArtifactType.LOG))))
- def test_attach(self):
+ def test_setLog(self):
+ self._test_setLog(private=False)
+
+ def test_setLog_private(self):
+ self._test_setLog(private=True)
+
+ def _test_attach(self, private):
+ requester = self.factory.makePerson()
+ with person_logged_in(requester):
+ kwargs = {"owner": requester}
+ if private:
+ kwargs["information_type"] = InformationType.USERDATA
+ repository = self.factory.makeGitRepository(**kwargs)
+ report = self.factory.makeRevisionStatusReport(
+ git_repository=repository)
+ report_url = api_url(report)
+ webservice = self.getWebservice(requester, repository)
filenames = ["artifact-1", "artifact-2"]
contents = [b"artifact 1", b"artifact 2"]
for filename, content in zip(filenames, contents):
- response = self.webservice.named_post(
- self.report_url, "attach", name=filename,
- data=io.BytesIO(content))
+ response = webservice.named_post(
+ report_url, "attach", name=filename, data=io.BytesIO(content))
self.assertEqual(200, response.status)
- with person_logged_in(self.requester):
+ with person_logged_in(requester):
artifacts = list(getUtility(
- IRevisionStatusArtifactSet).findByReport(self.report))
+ IRevisionStatusArtifactSet).findByReport(report))
self.assertThat(artifacts, MatchesSetwise(*(
MatchesStructure(
- report=Equals(self.report),
+ report=Equals(report),
library_file=MatchesStructure(
content=MatchesStructure.byEquality(
sha256=hashlib.sha256(content).hexdigest()),
filename=Equals(filename),
- mimetype=Equals("application/octet-stream")),
+ mimetype=Equals("application/octet-stream"),
+ restricted=Is(private)),
artifact_type=Equals(RevisionStatusArtifactType.BINARY))
for filename, content in zip(filenames, contents))))
+ def test_attach(self):
+ self._test_attach(private=False)
+
+ def test_attach_private(self):
+ self._test_attach(private=True)
+
def test_update(self):
- response = self.webservice.named_post(
- self.report_url, "update", title="updated-report-title")
+ report = self.factory.makeRevisionStatusReport(
+ result=RevisionStatusResult.FAILED)
+ requester = report.creator
+ repository = report.git_repository
+ initial_commit_sha1 = report.commit_sha1
+ initial_result_summary = report.result_summary
+ report_url = api_url(report)
+ webservice = self.getWebservice(requester, repository)
+ response = webservice.named_post(
+ report_url, "update", title="updated-report-title")
self.assertEqual(200, response.status)
- with person_logged_in(self.requester):
- self.assertEqual('updated-report-title', self.report.title)
- self.assertEqual(self.commit_sha1, self.report.commit_sha1)
- self.assertEqual(self.result_summary, self.report.result_summary)
- self.assertEqual(RevisionStatusResult.FAILED, self.report.result)
- date_finished_before_update = self.report.date_finished
- response = self.webservice.named_post(
- self.report_url, "update", result="Succeeded")
+ with person_logged_in(requester):
+ self.assertThat(report, MatchesStructure.byEquality(
+ title="updated-report-title",
+ commit_sha1=initial_commit_sha1,
+ result_summary=initial_result_summary,
+ result=RevisionStatusResult.FAILED))
+ date_finished_before_update = report.date_finished
+ response = webservice.named_post(
+ report_url, "update", result="Succeeded")
self.assertEqual(200, response.status)
- with person_logged_in(self.requester):
- self.assertEqual('updated-report-title', self.report.title)
- self.assertEqual(self.commit_sha1, self.report.commit_sha1)
- self.assertEqual(self.result_summary, self.report.result_summary)
- self.assertEqual(RevisionStatusResult.SUCCEEDED,
- self.report.result)
- self.assertGreater(self.report.date_finished,
- date_finished_before_update)
+ with person_logged_in(requester):
+ self.assertThat(report, MatchesStructure(
+ title=Equals("updated-report-title"),
+ commit_sha1=Equals(initial_commit_sha1),
+ result_summary=Equals(initial_result_summary),
+ result=Equals(RevisionStatusResult.SUCCEEDED),
+ date_finished=GreaterThan(date_finished_before_update)))
