launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #32667
[Merge] ~ilkeremrekoc/launchpad:cargo-maven-metadata-session into launchpad:master
İlker Emre Koç has proposed merging ~ilkeremrekoc/launchpad:cargo-maven-metadata-session into launchpad:master.
Commit message:
Add session to metadata publishing for artifactory
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~ilkeremrekoc/launchpad/+git/launchpad/+merge/488152
AQL (Artifactory Query Language needs a bearer token for
authentication. To add that and other session parameters to our
connection we used the previous artifactory publishing code.
Then we restored the commented out calls to property publishing
function and tests of it.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilkeremrekoc/launchpad:cargo-maven-metadata-session into launchpad:master.
diff --git a/lib/lp/crafts/model/craftrecipebuildjob.py b/lib/lp/crafts/model/craftrecipebuildjob.py
index c799b7f..eb2f835 100644
--- a/lib/lp/crafts/model/craftrecipebuildjob.py
+++ b/lib/lp/crafts/model/craftrecipebuildjob.py
@@ -17,6 +17,7 @@ import tempfile
from configparser import NoSectionError
from urllib.parse import urlparse
+import requests
import transaction
import yaml
from artifactory import ArtifactoryPath
@@ -416,11 +417,7 @@ class CraftPublishingJob(CraftRecipeBuildJobDerived):
if result.returncode != 0:
raise Exception(f"Failed to publish crate: {result.stderr}")
- # XXX ruinedyourlife 2025-06-06:
- # The publish_properties method is not working as expected.
- # Artifactory is giving us a 403.
- # We should fix it, but for now we'll skip it.
- # self._publish_properties(cargo_publish_url, artifact_name)
+ self._publish_properties(cargo_publish_url, artifact_name)
def _publish_maven_artifact(
self, work_dir, env_vars, artifact_name, jar_path=None, pom_path=None
@@ -484,11 +481,7 @@ class CraftPublishingJob(CraftRecipeBuildJobDerived):
f"Failed to publish Maven artifact: {result.stderr}"
)
- # XXX ruinedyourlife 2025-06-06:
- # The publish_properties method is not working as expected.
- # Artifactory is giving us a 403.
- # We should fix it, but for now we'll skip it.
- # self._publish_properties(maven_publish_url, artifact_name)
+ self._publish_properties(maven_publish_url, artifact_name)
def _get_maven_settings_xml(self, username, password):
"""Generate Maven settings.xml content.
@@ -582,6 +575,26 @@ class CraftPublishingJob(CraftRecipeBuildJobDerived):
# Combine all parts
return header + schema + servers + proxies + profiles + active_profiles
+ def _make_session(self) -> requests.Session:
+ """Make a suitable requests session for talking to Artifactory."""
+
+ session = requests.Session()
+ session.trust_env = False
+ if config.launchpad.http_proxy:
+ session.proxies = {
+ "http": config.launchpad.http_proxy,
+ "https": config.launchpad.http_proxy,
+ }
+ if config.launchpad.ca_certificates_path is not None:
+ session.verify = config.launchpad.ca_certificates_path
+ write_creds = config.artifactory.write_credentials
+
+ if write_creds is not None:
+ session.headers.update(
+ {"Authorization": f"Bearer {write_creds.split(':', 1)[1]}"}
+ )
+ return session
+
def _publish_properties(
self, publish_url: str, artifact_name: str
) -> None:
@@ -597,8 +610,10 @@ class CraftPublishingJob(CraftRecipeBuildJobDerived):
new_properties["soss.license"] = [self._get_license_metadata()]
# Repo name is derived from the URL
- # We assume the URL ends with the repository name
- repo_name = publish_url.rstrip("/").split("/")[-1]
+ # We assume the URL ends with "index" as both Cargo and Maven use such
+ # indexes to publish artifacts. Thus, we also assume that the
+ # repository name is the second last part of the URL.
+ repo_name = publish_url.rstrip("/").split("/")[-2]
root_path_str = self._extract_root_path(publish_url)
if not root_path_str:
@@ -608,6 +623,8 @@ class CraftPublishingJob(CraftRecipeBuildJobDerived):
# Search for the artifact in Artifactory using AQL
root_path = ArtifactoryPath(root_path_str)
+ root_path.session = self._make_session()
+
artifacts = root_path.aql(
"items.find",
{
@@ -621,8 +638,8 @@ class CraftPublishingJob(CraftRecipeBuildJobDerived):
if not artifacts:
raise NotFoundError(
- f"Artifact '{artifact_name}' not found in repository \
- '{repo_name}'"
+ f"Artifact '{artifact_name}' not found in repository: "
+ + f"'{repo_name}'"
)
if len(artifacts) > 1:
@@ -637,6 +654,7 @@ class CraftPublishingJob(CraftRecipeBuildJobDerived):
artifact_path = ArtifactoryPath(
root_path, artifact["repo"], artifact["path"], artifact["name"]
)
+ artifact_path.session = self._make_session()
artifact_path.set_properties(new_properties)
def _extract_root_path(self, publish_url: str) -> str:
@@ -658,7 +676,7 @@ class CraftPublishingJob(CraftRecipeBuildJobDerived):
craft_recipe = self.build.recipe
if craft_recipe.git_repository is not None:
- return craft_recipe.git_repository.git_https_url
+ return craft_recipe.git_repository.getCodebrowseUrl()
elif craft_recipe.git_repository_url is not None:
return craft_recipe.git_repository_url
else:
diff --git a/lib/lp/crafts/tests/test_craftrecipebuildjob.py b/lib/lp/crafts/tests/test_craftrecipebuildjob.py
index 0dc0462..3fe124b 100644
--- a/lib/lp/crafts/tests/test_craftrecipebuildjob.py
+++ b/lib/lp/crafts/tests/test_craftrecipebuildjob.py
@@ -86,6 +86,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
# Set up the Artifactory fixture
self.base_url = "https://example.com/artifactory";
self.repository_name = "repository"
+ self.publish_url = f"{self.base_url}/{self.repository_name}/index"
self.artifactory = self.useFixture(
FakeArtifactoryFixture(self.base_url, self.repository_name)
@@ -118,9 +119,9 @@ class TestCraftPublishingJob(TestCaseWithFactory):
else:
# Push with standard environment variables
env_vars = {
- "CARGO_PUBLISH_URL": f"{self.base_url}/repository",
+ "CARGO_PUBLISH_URL": self.publish_url,
"CARGO_PUBLISH_AUTH": "lp:token123",
- "MAVEN_PUBLISH_URL": f"{self.base_url}/repository",
+ "MAVEN_PUBLISH_URL": self.publish_url,
"MAVEN_PUBLISH_AUTH": "lp:token123",
}
self.pushConfig(
@@ -416,8 +417,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
self._setup_distribution("soss")
self._setup_config(with_env_vars=True, with_http_proxy=True)
- # lfa = self._create_crate_file()
- self._create_crate_file()
+ lfa = self._create_crate_file()
# Add a metadata file with license information
license_value = "Apache-2.0"
@@ -457,29 +457,25 @@ class TestCraftPublishingJob(TestCaseWithFactory):
self.patch(crbj_subprocess, "run", mock_run)
- # XXX ruinedyourlife 2025-06-06:
- # The publish_properties method is not working as expected.
- # Artifactory is giving us a 403.
- # We should fix it, but for now we'll skip it.
- # original_publish_properties = CraftPublishingJob._publish_properties
+ original_publish_properties = CraftPublishingJob._publish_properties
- # def mock_cargo_publish_properties(*args, **kwargs):
- # """Mock _publish_properties to deploy the crate to Artifactory
- # Fixture before testing.
+ def mock_cargo_publish_properties(*args, **kwargs):
+ """Mock _publish_properties to deploy the crate to Artifactory
+ Fixture before testing.
- # We need to do this in a nested function here because we need
- # to access the `lfa` variable which is created in the this test
- # setup above but the mocked function (and the lfa.open()) can
- # only be called inside the job's run method."""
+ We need to do this in a nested function here because we need
+ to access the `lfa` variable which is created in the this test
+ setup above but the mocked function (and the lfa.open()) can
+ only be called inside the job's run method."""
- # self._artifactory_put(args[1], "crates/test", args[2], lfa)
- # return original_publish_properties(*args, **kwargs)
+ self._artifactory_put(args[1], "crates/test", args[2], lfa)
+ return original_publish_properties(*args, **kwargs)
- # self.patch(
- # CraftPublishingJob,
- # "_publish_properties",
- # mock_cargo_publish_properties,
- # )
+ self.patch(
+ CraftPublishingJob,
+ "_publish_properties",
+ mock_cargo_publish_properties,
+ )
# Create and run the job
job = self.run_job(self.build)
@@ -521,30 +517,29 @@ class TestCraftPublishingJob(TestCaseWithFactory):
env = kwargs["env"]
self.assertIn("CARGO_HOME", env)
- # XXX ruinedyourlife 2025-06-06:
- # The publish_properties method is not working as expected.
- # Artifactory is giving us a 403.
- # We should fix it, but for now we'll skip it.
- # # Verify that the artifact's metadata were uploaded to Artifactory
- # artifact = self._artifactory_search("repository", lfa.filename)
-
- # self.assertIsNotNone(artifact, "Artifact not found in Artifactory")
-
- # self.assertEqual(artifact["repo"], "repository")
- # self.assertEqual(artifact["name"], lfa.filename)
- # self.assertEqual(artifact["path"], "crates/test")
- # self.assertEqual(
- # artifact["properties"]["soss.commit_id"], "random-revision-id"
- # )
- # self.assertEqual(
- # artifact["properties"]["soss.source_url"],
- # removeSecurityProxy(self.recipe).git_repository.git_https_url,
- # )
- # self.assertEqual(artifact["properties"]["soss.type"], "source")
- # self.assertEqual(
- # artifact["properties"]["soss.license"],
- # license_value
- # )
+ # Verify that the artifact's metadata were uploaded to Artifactory
+ artifact = self._artifactory_search("repository", lfa.filename)
+
+ self.assertIsNotNone(artifact, "Artifact not found in Artifactory")
+
+ self.assertEqual(artifact["repo"], "repository")
+ self.assertEqual(artifact["name"], lfa.filename)
+
+ # Our artifactory fixture preserves the "index/" path used during
+ # publishing. This is not the case in a real Artifactory
+ # instance, so we need to remove the "index/" prefix.
+ path_without_index = artifact["path"].replace("index/", "")
+ self.assertEqual(path_without_index, "crates/test")
+
+ self.assertEqual(
+ artifact["properties"]["soss.commit_id"], "random-revision-id"
+ )
+ self.assertEqual(
+ artifact["properties"]["soss.source_url"],
+ removeSecurityProxy(self.recipe).git_repository.git_https_url,
+ )
+ self.assertEqual(artifact["properties"]["soss.type"], "source")
+ self.assertEqual(artifact["properties"]["soss.license"], license_value)
def test_run_missing_maven_config(self):
"""
@@ -666,31 +661,27 @@ class TestCraftPublishingJob(TestCaseWithFactory):
self.patch(crbj_subprocess, "run", mock_run)
- # XXX ruinedyourlife 2025-06-06:
- # The publish_properties method is not working as expected.
- # Artifactory is giving us a 403.
- # We should fix it, but for now we'll skip it.
- # original_publish_properties = CraftPublishingJob._publish_properties
+ original_publish_properties = CraftPublishingJob._publish_properties
- # def mock_maven_publish_properties(*args, **kwargs):
- # """Mock _publish_properties to deploy the crate to Artifactory
- # Fixture before testing.
+ def mock_maven_publish_properties(*args, **kwargs):
+ """Mock _publish_properties to deploy the crate to Artifactory
+ Fixture before testing.
- # We need to do this in a nested function here because we need
- # to access the `lfa` variable which is created in the this test
- # setup above but the mocked function (and the lfa.open()) can
- # only be called inside the job's run method."""
+ We need to do this in a nested function here because we need
+ to access the `lfa` variable which is created in the this test
+ setup above but the mocked function (and the lfa.open()) can
+ only be called inside the job's run method."""
- # self._artifactory_put(
- # args[1], "com/example/test-artifact/0.1.0", args[2], jar_lfa
- # )
- # return original_publish_properties(*args, **kwargs)
+ self._artifactory_put(
+ args[1], "com/example/test-artifact/0.1.0", args[2], jar_lfa
+ )
+ return original_publish_properties(*args, **kwargs)
- # self.patch(
- # CraftPublishingJob,
- # "_publish_properties",
- # mock_maven_publish_properties,
- # )
+ self.patch(
+ CraftPublishingJob,
+ "_publish_properties",
+ mock_maven_publish_properties,
+ )
job = self.run_job(self.build)
@@ -750,30 +741,27 @@ class TestCraftPublishingJob(TestCaseWithFactory):
kwargs = mvn_call[1]
self.assertIn("cwd", kwargs)
- # XXX ruinedyourlife 2025-06-06:
- # The publish_properties method is not working as expected.
- # Artifactory is giving us a 403.
- # We should fix it, but for now we'll skip it.
- # artifact = self._artifactory_search("repository", jar_lfa.filename)
-
- # # Verify that the artifact's metadata were uploaded to Artifactory
- # self.assertIsNotNone(artifact, "Artifact not found in Artifactory")
-
- # self.assertEqual(artifact["repo"], "repository")
- # self.assertEqual(artifact["name"], jar_lfa.filename)
- # self.assertEqual(artifact["path"], "com/example/test-artifact/0.1.0")
- # self.assertEqual(
- # artifact["properties"]["soss.commit_id"], "random-revision-id"
- # )
- # self.assertEqual(
- # artifact["properties"]["soss.source_url"],
- # git_repository.git_https_url,
- # )
- # self.assertEqual(artifact["properties"]["soss.type"], "source")
- # self.assertEqual(
- # artifact["properties"]["soss.license"],
- # license_value
- # )
+ artifact = self._artifactory_search("repository", jar_lfa.filename)
+
+ # Verify that the artifact's metadata were uploaded to Artifactory
+ self.assertIsNotNone(artifact, "Artifact not found in Artifactory")
+ self.assertEqual(artifact["repo"], "repository")
+ self.assertEqual(artifact["name"], jar_lfa.filename)
+
+ # Our artifactory fixture preserves the "index/" path used during
+ # publishing. This is not the case in a real Artifactory
+ # instance, so we need to remove the "index/" prefix.
+ path_without_index = artifact["path"].replace("index/", "")
+ self.assertEqual(path_without_index, "com/example/test-artifact/0.1.0")
+ self.assertEqual(
+ artifact["properties"]["soss.commit_id"], "random-revision-id"
+ )
+ self.assertEqual(
+ artifact["properties"]["soss.source_url"],
+ removeSecurityProxy(self.recipe).git_repository.git_https_url,
+ )
+ self.assertEqual(artifact["properties"]["soss.type"], "source")
+ self.assertEqual(artifact["properties"]["soss.license"], license_value)
def test__publish_properties_sets_expected_properties(self):
"""Test that _publish_properties sets the correct properties in
@@ -796,12 +784,12 @@ class TestCraftPublishingJob(TestCaseWithFactory):
removeSecurityProxy(self.build).revision_id = "random-revision-id"
self._artifactory_put(
- f"{self.base_url}/repository",
+ f"{self.base_url}/{self.repository_name}",
"middle_folder",
"artifact.file",
MockBytesIO(b"dummy content"),
)
- job._publish_properties(f"{self.base_url}/repository", "artifact.file")
+ job._publish_properties(self.publish_url, "artifact.file")
artifact = self._artifactory_search("repository", "artifact.file")
self.assertIsNotNone(artifact, "Artifact not found in Artifactory")
@@ -829,7 +817,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
self.assertRaises(
NotFoundError,
job._publish_properties,
- f"{self.base_url}/repository",
+ self.publish_url,
"missing-artifact.file",
)
@@ -838,7 +826,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
metadata.yaml is present."""
self._artifactory_put(
- f"{self.base_url}/repository",
+ f"{self.base_url}/{self.repository_name}",
"some/path",
"artifact.file",
MockBytesIO(b"dummy content"),
@@ -847,7 +835,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
job = getUtility(ICraftPublishingJobSource).create(self.build)
job = removeSecurityProxy(job)
job.run = lambda: job._publish_properties(
- f"{self.base_url}/repository", "artifact.file"
+ self.publish_url, "artifact.file"
)
self.patch(
@@ -877,7 +865,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
removeSecurityProxy(self.build).addFile(metadata_lfa)
self._artifactory_put(
- f"{self.base_url}/repository",
+ f"{self.base_url}/{self.repository_name}",
"some/path",
"artifact.file",
MockBytesIO(b"dummy content"),
@@ -886,7 +874,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
job = getUtility(ICraftPublishingJobSource).create(self.build)
job = removeSecurityProxy(job)
job.run = lambda: job._publish_properties(
- f"{self.base_url}/repository", "artifact.file"
+ self.publish_url, "artifact.file"
)
self.patch(
@@ -917,7 +905,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
removeSecurityProxy(self.build).addFile(metadata_lfa)
self._artifactory_put(
- f"{self.base_url}/repository",
+ f"{self.base_url}/{self.repository_name}",
"some/path",
"artifact.file",
MockBytesIO(b"dummy content"),
@@ -926,7 +914,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
job = getUtility(ICraftPublishingJobSource).create(self.build)
job = removeSecurityProxy(job)
job.run = lambda: job._publish_properties(
- f"{self.base_url}/repository", "artifact.file"
+ self.publish_url, "artifact.file"
)
self.patch(
@@ -944,7 +932,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
"""Test that _publish_properties gets git_repository as source_url."""
self._artifactory_put(
- f"{self.base_url}/repository",
+ f"{self.base_url}/{self.repository_name}",
"some/path",
"artifact.file",
MockBytesIO(b"dummy content"),
@@ -960,7 +948,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
CraftPublishingJob, "_get_license_metadata", lambda self: "MIT"
)
- job._publish_properties(f"{self.base_url}/repository", "artifact.file")
+ job._publish_properties(self.publish_url, "artifact.file")
artifact = self._artifactory_search("repository", "artifact.file")
self.assertEqual(
@@ -973,7 +961,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
source_url."""
self._artifactory_put(
- f"{self.base_url}/repository",
+ f"{self.base_url}/{self.repository_name}",
"some/path",
"artifact.file",
MockBytesIO(b"dummy content"),
@@ -993,7 +981,7 @@ class TestCraftPublishingJob(TestCaseWithFactory):
CraftPublishingJob, "_get_license_metadata", lambda self: "MIT"
)
- job._publish_properties(f"{self.base_url}/repository", "artifact.file")
+ job._publish_properties(self.publish_url, "artifact.file")
artifact = self._artifactory_search("repository", "artifact.file")
self.assertEqual(
