launchpad-reviewers team mailing list archive

[Simone Pelosi <mp+473393@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/rocks/tests/test_rockrecipebuildbehaviour.py b/lib/lp/rocks/tests/test_rockrecipebuildbehaviour.py
> index 88cea3a..3dcd1db 100644
> --- a/lib/lp/rocks/tests/test_rockrecipebuildbehaviour.py
> +++ b/lib/lp/rocks/tests/test_rockrecipebuildbehaviour.py
> @@ -425,6 +438,151 @@ class TestAsyncRockRecipeBuildBehaviour(
>          )
>  
>  
> +class TestAsyncSnapBuildBehaviourFetchService(

Good catch! I forgot to rename the class here. Moreover, I think you are right, and we should avoid duplicating code, I will leave a comment as you suggested.

> +    StatsMixin, TestRockRecipeBuildBehaviourBase
> +):
> +    run_tests_with = AsynchronousDeferredRunTestForBrokenTwisted.make_factory(
> +        timeout=30
> +    )
> +
> +    @defer.inlineCallbacks
> +    def setUp(self):
> +        super().setUp()
> +        self.session = {
> +            "id": "1",
> +            "token": uuid.uuid4().hex,
> +        }
> +        self.fetch_service_url = (
> +            "http://{session_id}:{token}@{host}:{port}".format(
> +                session_id=self.session["id"],
> +                token=self.session["token"],
> +                host=config.builddmaster.fetch_service_host,
> +                port=config.builddmaster.fetch_service_port,
> +            )
> +        )
> +        self.fetch_service_api = self.useFixture(
> +            InProcessFetchServiceAuthAPIFixture()
> +        )
> +        yield self.fetch_service_api.start()
> +        self.now = time.time()
> +        self.useFixture(fixtures.MockPatch("time.time", return_value=self.now))
> +        self.addCleanup(shut_down_default_process_pool)
> +        self.setUpStats()
> +
> +    def makeJob(self, **kwargs):
> +        # We need a builder worker in these tests, in order that requesting
> +        # a proxy token can piggyback on its reactor and pool.
> +        job = super().makeJob(**kwargs)
> +        builder = MockBuilder()
> +        builder.processor = job.build.processor
> +        worker = self.useFixture(WorkerTestHelpers()).getClientWorker()
> +        job.setBuilder(builder, worker)
> +        self.addCleanup(worker.pool.closeCachedConnections)
> +        return job
> +
> +    @defer.inlineCallbacks
> +    def test_requestFetchServiceSession_unconfigured(self):
> +        """Create a rock recipe build request with an incomplete fetch service
> +        configuration.
> +
> +        If `fetch_service_host` is not provided the function will return
> +        without populating `proxy_url` and `revocation_endpoint`.
> +        """
> +        self.pushConfig("builddmaster", fetch_service_host=None)
> +        job = self.makeJob(use_fetch_service=True)
> +        with dbuser(config.builddmaster.dbuser):
> +            args = yield job.extraBuildArgs()
> +        self.assertEqual([], self.fetch_service_api.sessions.requests)
> +        self.assertNotIn("proxy_url", args)
> +        self.assertNotIn("revocation_endpoint", args)
> +
> +    @defer.inlineCallbacks
> +    def test_requestFetchServiceSession_no_certificate(self):
> +        """Create a rock recipe build request with an incomplete fetch service
> +        configuration.
> +
> +        If `fetch_service_mitm_certificate` is not provided
> +        the function raises a `CannotBuild` error.
> +        """
> +        self.pushConfig("builddmaster", fetch_service_mitm_certificate=None)
> +        job = self.makeJob(use_fetch_service=True)
> +        expected_exception_msg = (
> +            "fetch_service_mitm_certificate is not configured."
> +        )
> +        with ExpectedException(CannotBuild, expected_exception_msg):
> +            yield job.extraBuildArgs()
> +
> +    @defer.inlineCallbacks
> +    def test_requestFetchServiceSession_no_secret(self):
> +        """Create a rock recipe build request with an incomplete fetch service
> +        configuration.
> +
> +        If `fetch_service_control_admin_secret` is not provided
> +        the function raises a `CannotBuild` error.
> +        """
> +        self.pushConfig(
> +            "builddmaster", fetch_service_control_admin_secret=None
> +        )
> +        job = self.makeJob(use_fetch_service=True)
> +        expected_exception_msg = (
> +            "fetch_service_control_admin_secret is not configured."
> +        )
> +        with ExpectedException(CannotBuild, expected_exception_msg):
> +            yield job.extraBuildArgs()
> +
> +    @defer.inlineCallbacks
> +    def test_requestFetchServiceSession(self):
> +        """Create a rock recipe build request with a successful fetch service
> +        configuration.
> +
> +        `proxy_url` and `revocation_endpoint` are correctly populated.
> +        """
> +        job = self.makeJob(use_fetch_service=True)
> +        args = yield job.extraBuildArgs()
> +        request_matcher = MatchesDict(
> +            {
> +                "method": Equals(b"POST"),
> +                "uri": Equals(b"/session"),
> +                "headers": ContainsDict(
> +                    {
> +                        b"Authorization": MatchesListwise(
> +                            [
> +                                Equals(
> +                                    b"Basic "
> +                                    + base64.b64encode(
> +                                        b"admin-launchpad.test:admin-secret"
> +                                    )
> +                                )
> +                            ]
> +                        ),
> +                        b"Content-Type": MatchesListwise(
> +                            [
> +                                Equals(b"application/json"),
> +                            ]
> +                        ),
> +                    }
> +                ),
> +                "json": MatchesDict(
> +                    {
> +                        "policy": Equals("permissive"),
> +                    }
> +                ),
> +            }
> +        )
> +        self.assertThat(
> +            self.fetch_service_api.sessions.requests,
> +            MatchesListwise([request_matcher]),
> +        )
> +        self.assertIn("proxy_url", args)
> +        self.assertIn("revocation_endpoint", args)
> +        self.assertTrue(args["use_fetch_service"])
> +        self.assertIn("secrets", args)
> +        self.assertIn("fetch_service_mitm_certificate", args["secrets"])
> +        self.assertIn(
> +            "fake-cert", args["secrets"]["fetch_service_mitm_certificate"]
> +        )
> +
> +
>  class MakeRockRecipeBuildMixin:
>      """Provide the common makeBuild method returning a queued build."""
>  


-- 
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/473393
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad:fetch-service-configuration-for-rock-builds into launchpad:master.