Ines Almeida has proposed merging
~ines-almeida/launchpad:fetch-service-redact-certificate-in-logs into
launchpad:master.
Commit message:
Redact `fetch_service_mitm_certificate` in build logs
When running a fetch service build, we send the certficate from buildd-manager
to buildd, and log it. This redacts the certificate.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/465326
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~ines-almeida/launchpad:fetch-service-redact-certificate-in-logs into
launchpad:master.
diff --git a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
index e8d82d2..4dd258a 100644
--- a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
+++ b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
@@ -12,6 +12,7 @@ import logging
import os
import tempfile
from collections import OrderedDict
+from copy import deepcopy
from datetime import datetime
import transaction
@@ -126,6 +127,14 @@ class BuildFarmJobBehaviourBase:
def redactXmlrpcArguments(self, args):
# we do not want to have secrets in logs
+
+ # we need to copy the input in order to avoid mutating `args` which
+ # will be passed to the builders
+ args = deepcopy(args)
+ if args["args"].get("secrets"):
+ for key in args["args"]["secrets"].keys():
+ args["args"]["secrets"][key] = "<redacted>"
+
return sanitise_urls(repr(args))
@defer.inlineCallbacks
diff --git a/lib/lp/code/model/cibuildbehaviour.py b/lib/lp/code/model/cibuildbehaviour.py
index 6cceb72..ba73848 100644
--- a/lib/lp/code/model/cibuildbehaviour.py
+++ b/lib/lp/code/model/cibuildbehaviour.py
@@ -9,7 +9,6 @@ __all__ = [
import json
from configparser import NoSectionError
-from copy import deepcopy
from typing import Any, Generator
from twisted.internet import defer
@@ -119,17 +118,6 @@ class CIBuildBehaviour(BuilderProxyMixin, BuildFarmJobBehaviourBase):
ALLOWED_STATUS_NOTIFICATIONS = ["PACKAGEFAIL"]
- def redactXmlrpcArguments(self, args):
- # we do not want to have secrets in logs
-
- # we need to copy the input in order to avoid mutating `args` which
- # will be passed to the builders
- args = deepcopy(args)
- if args["args"].get("secrets"):
- for key in args["args"]["secrets"].keys():
- args["args"]["secrets"][key] = "<redacted>"
- return super().redactXmlrpcArguments(args)
-
def getLogFileName(self):
return "buildlog_ci_%s_%s_%s.txt" % (
self.build.git_repository.name,
diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
index cb2269b..66fb638 100644
--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
@@ -419,6 +419,45 @@ class TestAsyncSnapBuildBehaviourFetchService(
self.assertEqual([], self.fetch_service_api.sessions.requests)
@defer.inlineCallbacks
+ def test_requestFetchServiceSession_mitm_certficate_redacted(self):
+ """The `fetch_service_mitm_certificate` field in the build arguments
+ is redacted in the build logs."""
+
+ self.useFixture(
+ FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: "on"})
+ )
+ snap = self.factory.makeSnap(use_fetch_service=True)
+ request = self.factory.makeSnapBuildRequest(snap=snap)
+ job = self.makeJob(snap=snap, build_request=request)
+ args = yield job.extraBuildArgs()
+
+ chroot_lfa = self.factory.makeLibraryFileAlias(db_only=True)
+ job.build.distro_arch_series.addOrUpdateChroot(
+ chroot_lfa, image_type=BuildBaseImageType.CHROOT
+ )
+ lxd_lfa = self.factory.makeLibraryFileAlias(db_only=True)
+ job.build.distro_arch_series.addOrUpdateChroot(
+ lxd_lfa, image_type=BuildBaseImageType.LXD
+ )
+ deferred = defer.Deferred()
+ deferred.callback(None)
+ job._worker.sendFileToWorker = MagicMock(return_value=deferred)
+ job._worker.build = MagicMock(return_value=(None, None))
+
+ logger = BufferLogger()
+ yield job.dispatchBuildToWorker(logger)
+
+ # Secrets exist within the arguments
+ self.assertIn(
+ "fake-cert", args["secrets"]["fetch_service_mitm_certificate"]
+ )
+ # But are redacted in the log output
+ self.assertIn(
+ "'fetch_service_mitm_certificate': '<redacted>'",
+ logger.getLogBuffer(),
+ )
+
+ @defer.inlineCallbacks
def test_endProxySession(self):
"""By ending a fetch service session, metadata is retrieved from the
fetch service and saved to a file; and call to end the session is made.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
