launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+427608@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/launchpad-buildd:avoid-leaking-credentials into launchpad-buildd:master.

Commit message:
Pass secrets in a YAML file

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jugmac00/launchpad-buildd/+git/launchpad-buildd/+merge/427608
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad-buildd:avoid-leaking-credentials into launchpad-buildd:master.

diff --git a/lpbuildd/ci.py b/lpbuildd/ci.py
index 8ffd506..ac6c585 100644
--- a/lpbuildd/ci.py
+++ b/lpbuildd/ci.py
@@ -2,6 +2,8 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import os
+import tempfile
+import yaml
 
 from six.moves.configparser import (
     NoOptionError,
@@ -151,8 +153,18 @@ class CIBuildManager(BuildManagerProxyMixin, DebianBuildManager):
                 args.extend(
                     ["--plugin-setting", f"{key}={value}"])
         if self.secrets is not None:
-            for key, value in self.secrets.items():
-                args.extend(["--secret", f"{key}={value}"])
+            text = yaml.dump(self.secrets)
+            with tempfile.NamedTemporaryFile(mode="w") as f:
+                f.write(text)
+                f.flush()
+                path_to_secrets = f.name
+                self.backend.copy_in(
+                    source_path=path_to_secrets,
+                    target_path="/build/.launchpad-secrets.yaml"
+                )
+            args.extend(
+                ["--secrets", "/build/.launchpad-secrets.yaml"])
+
         job_name, job_index = self.current_job
         self.current_job_id = _make_job_id(job_name, job_index)
         args.extend([job_name, str(job_index)])
diff --git a/lpbuildd/target/run_ci.py b/lpbuildd/target/run_ci.py
index 82f9740..75deaf9 100644
--- a/lpbuildd/target/run_ci.py
+++ b/lpbuildd/target/run_ci.py
@@ -3,9 +3,6 @@
 
 import logging
 import os
-import tempfile
-
-import yaml
 
 from lpbuildd.target.build_snap import SnapChannelsAction
 from lpbuildd.target.operation import Operation
@@ -120,11 +117,8 @@ class RunCI(BuilderProxyOperationMixin, Operation):
             help="plugin setting where the key and value are separated by =",
         )
         parser.add_argument(
-            "--secret",
-            dest="secrets",
+            "--secrets",
             type=str,
-            action="append",
-            default=[],
             help="secrets where the key and the value are separated by =",
         )
 
@@ -161,22 +155,9 @@ class RunCI(BuilderProxyOperationMixin, Operation):
         for key, value in plugin_settings.items():
             lpcraft_args.extend(["--plugin-setting", f"{key}={value}"])
 
-        secrets = dict(
-            pair.split("=", maxsplit=1)
-            for pair in self.args.secrets
-        )
-        if secrets:
-            text = yaml.dump(secrets)
-            with tempfile.NamedTemporaryFile(mode="w") as f:
-                f.write(text)
-                f.flush()
-                path_to_secrets = f.name
-                self.backend.copy_in(
-                    source_path=path_to_secrets,
-                    target_path="/build/.launchpad-secrets.yaml"
-                )
+        if self.args.secrets:
             lpcraft_args.extend(
-                ["--secrets", "/build/.launchpad-secrets.yaml"])
+                ["--secrets", self.args.secrets])
 
         escaped_lpcraft_args = (
             " ".join(shell_escape(arg) for arg in lpcraft_args))
diff --git a/lpbuildd/target/tests/test_run_ci.py b/lpbuildd/target/tests/test_run_ci.py
index 1ca31dc..2f18bd5 100644
--- a/lpbuildd/target/tests/test_run_ci.py
+++ b/lpbuildd/target/tests/test_run_ci.py
@@ -423,8 +423,7 @@ class TestRunCI(TestCase):
         args = [
             "run-ci",
             "--backend=fake", "--series=focal", "--arch=amd64", "1",
-            "--secret", "soss=user:pass",
-            "--secret", "another_project=token:123",
+            "--secrets", "/build/.launchpad-secrets.yaml",
             "test", "0",
             ]
         run_ci = parse_args(args=args).operation
@@ -440,11 +439,6 @@ class TestRunCI(TestCase):
                 "| tee /build/output/test:0.log",
                 ], cwd="/build/tree"),
             ]))
-        content, _ = run_ci.backend.backend_fs[
-            "/build/.launchpad-secrets.yaml"]
-        self.assertEqual(
-            "another_project: token:123\nsoss: user:pass\n", content.decode()
-        )
 
     def test_run_succeeds(self):
         args = [
diff --git a/lpbuildd/tests/test_ci.py b/lpbuildd/tests/test_ci.py
index ab397e8..f67300d 100644
--- a/lpbuildd/tests/test_ci.py
+++ b/lpbuildd/tests/test_ci.py
@@ -126,6 +126,9 @@ class TestCIBuildManagerIteration(TestCase):
                 "miniconda_conda_channel": "https://user:pass@xxxxxxxxxxxxxxxxxxxxx/artifactory/soss-conda-stable-local/";,  # noqa: E501
                 "foo": "bar",
             },
+            "secrets": {
+                "auth": "user:pass",
+            }
         }
         expected_prepare_options = [
             "--git-repository", "https://git.launchpad.test/~example/+git/ci";,
@@ -141,6 +144,7 @@ class TestCIBuildManagerIteration(TestCase):
             "--environment-variable", "PATH=foo",
             "--plugin-setting", "miniconda_conda_channel=https://user:pass@xxxxxxxxxxxxxxxxxxxxx/artifactory/soss-conda-stable-local/";,  # noqa: E501
             "--plugin-setting", "foo=bar",
+            "--secrets", "/build/.launchpad-secrets.yaml",
             ]
         yield self.expectRunJob("build", "0", options=expected_job_options)
         self.buildmanager.backend.add_file(
@@ -275,7 +279,7 @@ class TestCIBuildManagerIteration(TestCase):
             "--environment-variable", "PATH=foo",
             "--plugin-setting", "miniconda_conda_channel=https://user:pass@xxxxxxxxxxxxxxxxxxxxx/artifactory/soss-conda-stable-local/";,  # noqa: E501
             "--plugin-setting", "foo=bar",
-            "--secret", "auth=user:pass"
+            "--secrets", "/build/.launchpad-secrets.yaml",
             ]
         yield self.expectRunJob("lint", "0", options=expected_job_options)
         self.buildmanager.backend.add_file(