Merge pull request #86 from jajreidy/fix-url-bug

**feat(upload): per-arch RPM repos, signed RPM result URLs, clearer repo/distro logs**

Author: jajreidy

CHANGELOG.md

@@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Added
-- `upload --target-arch-repo`: RPM-only; per-architecture RPM repos/distributions (e.g. `{namespace}/{arch}/`); logs/SBOM/artifacts remain build-scoped; lazy repo creation at upload; supported with `--results-json`, `--signed-by`, and `--overwrite`
+- `upload --target-arch-repo`: per-architecture RPM repos/distributions (``{namespace}/{arch}/Packages/...``); logs/SBOM/artifacts stay build-scoped; lazy repo creation at upload; works with `--results-json`, `--signed-by`, and `--overwrite`; with `--signed-by`, same arch repo and `signed_by` is label-only
 - `upload --overwrite`: RPM-only; remove existing RPM package units in the target repo that match local file SHA256 (and `signed_by` when set) via `remove_content_units` before upload
 - `upload --results-json`: Upload artifacts from pulp_results.json; files resolved from JSON directory or --files-base-path; --build-id and --namespace optional (extracted from artifact labels)
 - DistributionClient username/password (Basic Auth) support; use `username` and `password` in config as alternative to cert/key for pull downloads
@@ -21,10 +21,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Username/password (Basic Auth) support for packages.redhat.com
 
 ### Changed
-- `upload --target-arch-repo` with `--signed-by`: RPM repo/distribution paths remain `{arch}/` only (no `{arch}/rpms-signed`); signing is via `signed_by` label on content
+- Repository setup logs use the concrete repo slug (e.g. ``rpms-signed``) instead of a generic ``Rpms`` label; distribution creation logs state that ``name`` and ``base_path`` match the repository name on one line
+- `upload --target-arch-repo` with `--signed-by`: RPM paths remain `{arch}/` only (no `{arch}/rpms-signed`); signing is via `signed_by` label on content
 - `pull`: use each artifact's ``url`` from pulp_results.json when present instead of synthesizing download URLs from distribution entries
 
 ### Fixed
+- Results JSON RPM URLs with `--signed-by`: use the `rpms-signed` distribution base (`distributions.rpms_signed` / correct artifact `url`) instead of the unsigned `rpms` path
 - RPM distribution URLs: ``Packages/<letter>/`` uses the lowercase first character of the RPM **basename** only (correct for paths like ``Packages/W/foo.rpm``, ``arch/pkg.rpm``, or plain ``foo.rpm``)
 - Clear error when no auth credentials provided (client_id/client_secret or username/password)
 

pulp_tool/api/pulp_client.py

@@ -1078,9 +1078,19 @@ def _rpm_distribution_base_url_from_labels(self, labels: Dict[str, str]) -> str:
         arch_seg = sanitize_build_id_for_repository(arch)
         if not validate_build_id(arch_seg):
             arch_seg = "noarch"
-        # Per-arch repos do not use a separate *-signed distribution; signed_by is label-only.
         return f"{pulp_content}{ns}/{arch_seg}/"
 
+    def _build_rpm_packages_url_for_arch(self, arch: str, relative_path: str) -> str:
+        """Build RPM content URL under per-arch distribution (namespace/arch/Packages/...)."""
+        base_url_str = str(self.config["base_url"]).rstrip("/")
+        pulp_content_base_url = f"{base_url_str}/api/pulp-content"
+        ns = self.namespace if isinstance(self.namespace, str) else ""
+        base = f"{pulp_content_base_url}/{ns}/{arch}/"
+        filename, first_letter = rpm_packages_letter_and_basename(relative_path)
+        if filename:
+            return f"{base}Packages/{first_letter}/{filename}"
+        return relative_path
+
     def _build_rpm_distribution_url(
         self,
         relative_path: str,
@@ -1091,11 +1101,22 @@ def _build_rpm_distribution_url(
     ) -> str:
         """Build distribution URL for RPM artifacts (Packages/<lowercase-first-of-basename>/<basename>)."""
         labels = labels or {}
-        rpms_url = ""
         if target_arch_repo:
+            arch = (labels.get("arch") or "").strip()
+            if arch and arch in SUPPORTED_ARCHITECTURES:
+                return self._build_rpm_packages_url_for_arch(arch, relative_path)
             rpms_url = self._rpm_distribution_base_url_from_labels(labels)
-        if not rpms_url:
-            rpms_url = distribution_urls.get("rpms", "")
+            filename, first_letter = rpm_packages_letter_and_basename(relative_path)
+            if filename:
+                return f"{rpms_url}Packages/{first_letter}/{filename}"
+            return relative_path
+        if labels.get("signed_by") and distribution_urls.get("rpms_signed"):
+            rpms_url = distribution_urls["rpms_signed"]
+            if rpms_url:
+                filename, first_letter = rpm_packages_letter_and_basename(relative_path)
+                if filename:
+                    return f"{rpms_url}Packages/{first_letter}/{filename}"
+        rpms_url = distribution_urls.get("rpms", "")
         if rpms_url:
             filename, first_letter = rpm_packages_letter_and_basename(relative_path)
             if filename:
@@ -1150,14 +1171,17 @@ def _build_artifact_distribution_url(
             is_rpm: Whether this is an RPM artifact
             labels: Labels from content (may contain arch, etc.)
             distribution_urls: Dictionary mapping repo_type to distribution base URL
-            target_arch_repo: When True, RPM base URLs are derived per architecture from labels
+            target_arch_repo: When True, RPM URLs use per-arch paths from labels (or sanitized fallback)
 
         Returns:
             Full distribution URL for the artifact
         """
         if is_rpm:
             return self._build_rpm_distribution_url(
-                relative_path, distribution_urls, labels, target_arch_repo=target_arch_repo
+                relative_path,
+                distribution_urls,
+                labels=labels,
+                target_arch_repo=target_arch_repo,
             )
         return PulpClient._build_file_distribution_url(relative_path, labels, distribution_urls)
 
@@ -1746,7 +1770,7 @@ def build_results_structure(
             content_results: Content data from Pulp
             file_info_map: Mapping of artifact hrefs to file info models
             distribution_urls: Optional dictionary mapping repo_type to distribution base URL
-            target_arch_repo: When True, RPM artifact URLs use per-arch distribution paths from labels
+            target_arch_repo: When True, RPM URLs use per-arch distribution paths from labels
 
         Returns:
             Populated PulpResultsModel

pulp_tool/cli/upload.py

@@ -110,7 +110,8 @@ def _extract_build_id_namespace_from_results_json(results_json_path: Path) -> Tu
     default=False,
     help=(
         "RPM only: use each architecture as the RPM repository/distribution name "
-        "(e.g. .../pulp-content/{namespace}/x86_64/) instead of {build}/rpms; logs/SBOM/artifacts stay build-scoped"
+        "(e.g. .../pulp-content/{namespace}/x86_64/Packages/...) instead of {build}/rpms; "
+        "logs/SBOM/artifacts stay build-scoped. With --signed-by, same arch repo; signed_by is label-only."
     ),
 )
 @click.pass_context
@@ -192,7 +193,7 @@ def upload(  # pylint: disable=too-many-arguments,too-many-positional-arguments
             build_id,
             signed_by=args.signed_by,
             skip_artifacts_repo=skip_artifacts,
-            target_arch_repo=target_arch_repo,
+            target_arch_repo=args.target_arch_repo,
         )
         logging.info("Repository setup completed")
 

pulp_tool/services/upload_service.py

@@ -163,6 +163,17 @@ def upload_sbom(
     return task_response.created_resources
 
 
+def _distribution_urls_for_context(helper: PulpHelper, build_id: str, context: UploadContext) -> Dict[str, str]:
+    """Resolve distribution URL map for results JSON (per-arch vs signed aggregate RPM base)."""
+    target_arch = bool(getattr(context, "target_arch_repo", False))
+    signed_by = getattr(context, "signed_by", None)
+    if target_arch:
+        return helper.get_distribution_urls(build_id, target_arch_repo=True)
+    if signed_by and str(signed_by).strip():
+        return helper.get_distribution_urls(build_id, include_signed_rpm_distro=True)
+    return helper.get_distribution_urls(build_id)
+
+
 def _classify_artifact_from_key(key: str) -> str:
     """
     Classify artifact type from key (path/filename).
@@ -200,7 +211,7 @@ def process_uploads_from_results_json(
         client: PulpClient instance
         context: UploadRpmContext with results_json, files_base_path, signed_by
         repositories: RepositoryRefs (with signed refs when signed_by set)
-        pulp_helper: PulpHelper for per-arch RPM repos when ``target_arch_repo`` is set
+        pulp_helper: Optional PulpHelper for per-arch RPM repos when ``target_arch_repo`` is set
 
     Returns:
         URL of the uploaded results JSON, or None if upload failed
@@ -228,16 +239,15 @@ def process_uploads_from_results_json(
     base_path = Path(context.files_base_path or os.path.dirname(context.results_json)).resolve()
     use_signed = bool(context.signed_by and context.signed_by.strip())
 
-    # Only RPMs use signed repos; logs and SBOMs are never signed
-    if context.target_arch_repo:
-        rpm_href = ""
-    else:
-        rpm_href = repositories.rpms_signed_href if use_signed else repositories.rpms_href
+    # Only RPMs use signed aggregate repo; with target_arch_repo, signed_by is label-only on arch repos
+    rpm_href = (
+        "" if context.target_arch_repo else (repositories.rpms_signed_href if use_signed else repositories.rpms_href)
+    )
     logs_prn = repositories.logs_prn
     sbom_prn = repositories.sbom_prn
     artifacts_prn = repositories.artifacts_prn
 
-    if not context.target_arch_repo and use_signed and not rpm_href and not repositories.rpms_signed_prn:
+    if use_signed and not context.target_arch_repo and not rpm_href and not repositories.rpms_signed_prn:
         logging.error("signed_by set but signed repositories not available")
         raise ValueError("signed_by requires signed repositories")
 
@@ -302,7 +312,9 @@ def process_uploads_from_results_json(
 
     # Upload RPMs
     for arch, rpm_list in rpms_by_arch.items():
-        arch_href = helper.ensure_rpm_repository_for_arch(arch) if context.target_arch_repo else rpm_href
+        arch_href = (
+            helper.ensure_rpm_repository_for_arch(context.build_id, arch) if context.target_arch_repo else rpm_href
+        )
         created_resources.extend(
             upload_rpms(
                 rpm_list,
@@ -583,11 +595,8 @@ def _populate_results_model(
     """
     # Get distribution URLs to construct proper artifact URLs
     repository_helper = PulpHelper(client, parent_package=context.parent_package)
-    target_arch_repo = getattr(context, "target_arch_repo", False)
-    if target_arch_repo:
-        distribution_urls = repository_helper.get_distribution_urls(context.build_id, target_arch_repo=True)
-    else:
-        distribution_urls = repository_helper.get_distribution_urls(context.build_id)
+    distribution_urls = _distribution_urls_for_context(repository_helper, context.build_id, context)
+    target_arch_repo = bool(getattr(context, "target_arch_repo", False))
 
     client.build_results_structure(
         results_model,
@@ -610,11 +619,7 @@ def _add_distributions_to_results(
         results_model: Model to add distributions to
     """
     repository_helper = PulpHelper(client, parent_package=context.parent_package)
-    target_arch_repo = getattr(context, "target_arch_repo", False)
-    if target_arch_repo:
-        distribution_urls = repository_helper.get_distribution_urls(context.build_id, target_arch_repo=True)
-    else:
-        distribution_urls = repository_helper.get_distribution_urls(context.build_id)
+    distribution_urls = _distribution_urls_for_context(repository_helper, context.build_id, context)
 
     if distribution_urls:
         for repo_type, url in distribution_urls.items():

pulp_tool/utils/distribution_manager.py

@@ -39,7 +39,13 @@ def __init__(
             distribution_cache if distribution_cache is not None else {}
         )
 
-    def get_distribution_urls(self, build_id: str, target_arch_repo: bool = False) -> Dict[str, str]:
+    def get_distribution_urls(
+        self,
+        build_id: str,
+        *,
+        target_arch_repo: bool = False,
+        include_signed_rpm_distro: bool = False,
+    ) -> Dict[str, str]:
         """
         Get distribution URLs for all repository types.
 
@@ -48,7 +54,8 @@ def get_distribution_urls(self, build_id: str, target_arch_repo: bool = False) -
 
         Args:
             build_id: Build ID for naming repositories and distributions
-            target_arch_repo: If True, omit the aggregate ``rpms`` URL (per-arch RPM URLs are built elsewhere)
+            target_arch_repo: If True, omit aggregate ``rpms`` URL (per-arch RPM URLs elsewhere)
+            include_signed_rpm_distro: If True, add ``rpms_signed`` URL for ``{build}/rpms-signed/``
 
         Returns:
             Dictionary mapping repo_type to distribution URL
@@ -69,7 +76,11 @@ def get_distribution_urls(self, build_id: str, target_arch_repo: bool = False) -
         logging.debug("Getting distribution URLs for build: %s", sanitized_build_id)
 
         # Get distribution URLs directly using the helper's own methods
-        distribution_urls = self._get_distribution_urls_impl(sanitized_build_id, target_arch_repo=target_arch_repo)
+        distribution_urls = self._get_distribution_urls_impl(
+            sanitized_build_id,
+            target_arch_repo=target_arch_repo,
+            include_signed_rpm_distro=include_signed_rpm_distro,
+        )
 
         logging.debug("Retrieved %d distribution URLs", len(distribution_urls))
         return distribution_urls
@@ -108,12 +119,20 @@ def _get_single_distribution_url(self, build_id: str, repo_type: str, base_url:
         )
         return distribution_url
 
-    def _get_distribution_urls_impl(self, build_id: str, target_arch_repo: bool = False) -> Dict[str, str]:
+    def _get_distribution_urls_impl(
+        self,
+        build_id: str,
+        *,
+        target_arch_repo: bool = False,
+        include_signed_rpm_distro: bool = False,
+    ) -> Dict[str, str]:
         """
         Get distribution URLs for all repository types.
 
         Args:
             build_id: Base name for the repositories (may include namespace prefix)
+            target_arch_repo: If True, omit aggregate ``rpms`` URL
+            include_signed_rpm_distro: If True, add ``rpms_signed`` for signed RPM distribution
 
         Returns:
             Dictionary mapping repo_type to distribution URL
@@ -132,6 +151,11 @@ def _get_distribution_urls_impl(self, build_id: str, target_arch_repo: bool = Fa
             if url:
                 distribution_urls[repo_type] = url
 
+        if include_signed_rpm_distro and not target_arch_repo:
+            signed_url = self._get_single_distribution_url(build_id, "rpms-signed", base_url)
+            if signed_url:
+                distribution_urls["rpms_signed"] = signed_url
+
         return distribution_urls
 
 

pulp_tool/utils/pulp_helper.py

@@ -66,9 +66,9 @@ def setup_repositories(
 
         Args:
             build_id: Build ID for naming repositories and distributions
-            signed_by: If set, also create signed repos (rpms-signed, etc.)
+            signed_by: If set, also create signed repos (rpms-signed, etc.) unless target_arch_repo
             skip_artifacts_repo: If True, do not create artifacts repo (e.g. when saving locally)
-            target_arch_repo: If True, skip bulk rpms/rpms-signed; use ensure_rpm_repository_for_arch per arch
+            target_arch_repo: If True, skip aggregate rpms/rpms-signed; per-arch RPM repos at upload time
 
         Returns:
             RepositoryRefs NamedTuple containing all repository PRNs and hrefs
@@ -80,16 +80,13 @@ def setup_repositories(
             target_arch_repo=target_arch_repo,
         )
 
-    def ensure_rpm_repository_for_arch(self, arch: str) -> str:
-        """
-        Create or get the RPM repository for an architecture (target-arch-repo mode).
-
-        Returns:
-            Repository pulp_href for modify/add_content
-        """
-        return self._repository_manager.ensure_rpm_repository_for_arch(arch)
-
-    def get_distribution_urls(self, build_id: str, target_arch_repo: bool = False) -> dict[str, str]:
+    def get_distribution_urls(
+        self,
+        build_id: str,
+        *,
+        target_arch_repo: bool = False,
+        include_signed_rpm_distro: bool = False,
+    ) -> dict[str, str]:
         """
         Get distribution URLs for all repository types.
 
@@ -98,11 +95,21 @@ def get_distribution_urls(self, build_id: str, target_arch_repo: bool = False) -
 
         Args:
             build_id: Build ID for naming repositories and distributions
+            target_arch_repo: If True, omit aggregate ``rpms`` distribution URL
+            include_signed_rpm_distro: If True, include ``rpms_signed`` URL
 
         Returns:
             Dictionary mapping repo_type to distribution URL
         """
-        return self._distribution_manager.get_distribution_urls(build_id)
+        return self._distribution_manager.get_distribution_urls(
+            build_id,
+            target_arch_repo=target_arch_repo,
+            include_signed_rpm_distro=include_signed_rpm_distro,
+        )
+
+    def ensure_rpm_repository_for_arch(self, build_id: str, arch: str) -> str:
+        """Create or get RPM repository for ``target_arch_repo`` mode (base_path = arch)."""
+        return self._repository_manager.ensure_rpm_repository_for_arch(build_id, arch)
 
     def create_or_get_repository(
         self,