launchpad-reviewers team mailing list archive

[Quentin Debhi <mp+488754@xxxxxxxxxxxxxxxxxx>]

Quentin Debhi has proposed merging ~ruinedyourlife/launchpad-buildd:fix-parse-env-vars into launchpad-buildd:master.

Commit message:
Fix prefix and suffix removal for env vars

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ruinedyourlife/launchpad-buildd/+git/launchpad-buildd/+merge/488754
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad-buildd:fix-parse-env-vars into launchpad-buildd:master.

diff --git a/lpbuildd/target/build_craft.py b/lpbuildd/target/build_craft.py
index 2bba28f..69106a8 100644
--- a/lpbuildd/target/build_craft.py
+++ b/lpbuildd/target/build_craft.py
@@ -184,13 +184,16 @@ class BuildCraft(
             if not key.startswith("CARGO_"):
                 continue
             
-            # Extract name (e.g., CARGO_ARTIFACTORY1_URL -> ARTIFACTORY1)
-            name = key[len("CARGO_"):].split("_")[0]
-            
+            # Extract name by removing CARGO_ prefix
+            # and the suffix(_URL or _READ_AUTH)
             if key.endswith("_URL"):
+                # Remove CARGO_ prefix and _URL suffix
+                name = key[len("CARGO_"):-len("_URL")]
                 # Convert URL to registry index
                 cargo_vars[f"CARGO_REGISTRIES_{name}_INDEX"] = value
             elif key.endswith("_READ_AUTH"):
+                # Remove CARGO_ prefix and _READ_AUTH suffix
+                name = key[len("CARGO_"):-len("_READ_AUTH")]
                 # Extract token from user:token format
                 token = value.split(":", 1)[1]
                 cargo_vars[f"CARGO_REGISTRIES_{name}_TOKEN"] = f"Bearer {token}"
diff --git a/lpbuildd/target/tests/test_build_craft.py b/lpbuildd/target/tests/test_build_craft.py
index 45b69ac..b750588 100644
--- a/lpbuildd/target/tests/test_build_craft.py
+++ b/lpbuildd/target/tests/test_build_craft.py
@@ -1059,7 +1059,38 @@ class TestBuildCraft(TestCase):
         build_craft = parse_args(args=args).operation
         build_craft.backend.build_path = self.useFixture(TempDir()).path
         build_craft.backend.run = FakeRevisionID("42")
+        
+        # Create directories that mimic the real structure
+        os.makedirs(os.path.join(build_craft.backend.build_path, "test-image"), exist_ok=True)
+        
+        yaml_content = (
+            "name: test-package\n"
+            "license: BSD-3-Clause\n"
+            "version: 1.0.0\n"
+        )
+        
+        # The path in the backend's filesystem
+        project_path = "/home/buildd/test-image"
+        build_craft.backend.backend_fs[os.path.join(project_path, "sourcecraft.yaml")] = (
+            yaml_content.encode("UTF-8"), 
+            0o644  # File permissions
+        )
+        
+        # Run the build process
         self.assertEqual(0, build_craft.run())
+        
+        # Check the metadata was created correctly
+        metadata_path = os.path.join(project_path, "extra-metadata.yaml")
+        self.assertTrue(
+            build_craft.backend.path_exists(metadata_path),
+            "extra-metadata.yaml file wasn't created"
+        )
+        
+        # Read the metadata file to verify its contents
+        metadata_content = build_craft.backend.backend_fs.get(metadata_path, (b"", 0))[0].decode("utf-8")
+        self.assertIn("license: BSD-3-Clause", metadata_content)
+        
+        # Check that the rest of the build process ran correctly
         self.assertThat(
             build_craft.backend.run.calls,
             MatchesAll(
@@ -1244,6 +1275,48 @@ class TestBuildCraft(TestCase):
             ),
         )
 
+    def test_build_with_cargo_credentials_complex_names(self):
+        args = [
+            "build-craft",
+            "--backend=fake",
+            "--series=xenial",
+            "--arch=amd64",
+            "1",
+            "--branch",
+            "lp:foo",
+            "--environment-variable",
+            "CARGO_ARTIFACTORY1_TEST_TEST_URL=sparse+https://canonical.example.com/artifactory/api/cargo/cargo-upstream1/index/";,
+            "--environment-variable",
+            "CARGO_ARTIFACTORY1_TEST_TEST_READ_AUTH=user:token1",
+            "--environment-variable",
+            "CARGO_MY_SPECIAL_REGISTRY_URL=sparse+https://my.special.registry/index/";,
+            "--environment-variable",
+            "CARGO_MY_SPECIAL_REGISTRY_READ_AUTH=user:token2",
+            "--environment-variable",
+            "CARGO_REGISTRY_GLOBAL_CREDENTIAL_PROVIDERS=cargo:token",
+            "test-image",
+        ]
+        build_craft = parse_args(args=args).operation
+        build_craft.build()
+
+        # Verify the build command was run with correct environment
+        self.assertThat(
+            build_craft.backend.run.calls,
+            MatchesListwise(
+                [
+                    RanBuildCommand(
+                        ["sourcecraft", "pack", "-v", "--destructive-mode"],
+                        cwd="/home/buildd/test-image/.",
+                        CARGO_REGISTRIES_ARTIFACTORY1_TEST_TEST_INDEX="sparse+https://canonical.example.com/artifactory/api/cargo/cargo-upstream1/index/";,
+                        CARGO_REGISTRIES_ARTIFACTORY1_TEST_TEST_TOKEN="Bearer token1",
+                        CARGO_REGISTRIES_MY_SPECIAL_REGISTRY_INDEX="sparse+https://my.special.registry/index/";,
+                        CARGO_REGISTRIES_MY_SPECIAL_REGISTRY_TOKEN="Bearer token2",
+                        CARGO_REGISTRY_GLOBAL_CREDENTIAL_PROVIDERS="cargo:token",
+                    ),
+                ]
+            ),
+        )
+
     def test_build_with_maven_credentials(self):
         args = [
             "build-craft",