fix(DSXOSBRNG-1825): select promotion credentials by role

.github/actions/promote-rc/promotion.py

@@ -429,15 +429,18 @@ def __init__(self, source, destination=None):
         self.destination = destination or source
         self.tools = Path(__file__).resolve().parent.parent / "release-artifacts"
 
-    def docker_env(self, namespace):
-        key = "SOURCE_DOCKER_CONFIG" if namespace == self.source else "DESTINATION_DOCKER_CONFIG"
+    def docker_env(self, role):
+        require(role in {"source", "destination"}, "invalid registry credential role")
+        key = f"{role.upper()}_DOCKER_CONFIG"
         return {**os.environ, "DOCKER_CONFIG": os.environ[key]}
 
-    def image_digest(self, namespace, name, tag, absent=False):
+    def image_digest(self, role, name, tag, absent=False):
+        env = self.docker_env(role)
+        namespace = getattr(self, role)
         ref = f"nvcr.io/{namespace}/{name}:{tag}"
         output = command(
             ["docker", "buildx", "imagetools", "inspect", "--raw", ref],
-            env=self.docker_env(namespace),
+            env=env,
             missing=(lambda error: error.strip().removeprefix("ERROR: ") in {
                 f"{ref}: not found", f"{ref}: manifest unknown",
                 "manifest unknown", "manifest unknown: manifest unknown",
@@ -452,15 +455,15 @@ def image_digest(self, namespace, name, tag, absent=False):
         # not a hash of terminal formatting, as the immutable image identity.
         descriptor = json.loads(command(
             ["docker", "buildx", "imagetools", "inspect", "--format", "{{json .Manifest}}", ref],
-            env=self.docker_env(namespace),
+            env=env,
         ))
         require(matches(DIGEST, descriptor.get("digest")), "invalid registry manifest digest")
         return descriptor["digest"]
 
     def verify_image(self, name, digest, commit):
         command(["bash", str(self.tools / "verify-image.sh"),
                  f"nvcr.io/{self.source}/{name}@{digest}", commit],
-                env=self.docker_env(self.source))
+                env=self.docker_env("source"))
 
     def copy_image(self, name, digest, stable):
         command(["skopeo", "copy", "--all", "--preserve-digests",
@@ -469,8 +472,9 @@ def copy_image(self, name, digest, stable):
                  f"docker://nvcr.io/{self.source}/{name}@{digest}",
                  f"docker://nvcr.io/{self.destination}/{name}:{stable}"])
 
-    def chart(self, namespace, name, version, absent=False):
-        repo = "promotion-source" if namespace == self.source else "promotion-destination"
+    def chart(self, role, name, version, absent=False):
+        require(role in {"source", "destination"}, "invalid registry credential role")
+        repo = f"promotion-{role}"
         # Refresh errors never mean "version absent".
         command(["helm", "repo", "update", repo, "--fail-on-repo-update-fail"], clean_stderr=True)
         rows = json.loads(command([
@@ -511,24 +515,22 @@ def promote(config, evidence, github, registry, attempt, report_path=None):
     chart_targets = []
     # Complete the entire source and destination preflight before reserving a release.
     for item in evidence["images"]:
-        actual = registry.image_digest(config["source_ngc_path"], item["name"], rc)
+        actual = registry.image_digest("source", item["name"], rc)
         require(actual == item["digest"], "RC image tag changed after verification")
         registry.verify_image(item["name"], actual, commit)
-        existing = registry.image_digest(config["destination_ngc_path"], item["name"],
-                                         stable, absent=True)
+        existing = registry.image_digest("destination", item["name"], stable, absent=True)
         require(existing in {None, actual}, "stable image already has a different digest")
         image_targets.append({"name": item["name"], "digest": actual})
     for item in evidence["charts"]:
-        source = registry.chart(config["source_ngc_path"], item["name"], rc)
+        source = registry.chart("source", item["name"], rc)
         require(charts.digest(source) == item["digest"], "RC chart changed after verification")
         transformed = charts.promote(source, item["name"], rc, stable, commit,
                                      item["version_value_paths"], manifest_digest,
                                      {f"nvcr.io/{config['destination_ngc_path']}/{image['name']}:{stable}"
                                       for image in image_targets})
         prepared[item["name"]] = transformed
         registry.chart_destination(item["name"])
-        existing = registry.chart(config["destination_ngc_path"], item["name"],
-                                  stable, absent=True)
+        existing = registry.chart("destination", item["name"], stable, absent=True)
         require(existing is None or existing == transformed,
                 "stable chart already contains different files or provenance")
         chart_targets.append({"name": item["name"], "digest": charts.digest(transformed)})
@@ -551,23 +553,21 @@ def save():
     save()
     try:
         for item in image_targets:
-            current = registry.image_digest(config["destination_ngc_path"], item["name"],
-                                            stable, absent=True)
+            current = registry.image_digest("destination", item["name"], stable, absent=True)
             require(current in {None, item["digest"]}, "stable image changed during promotion")
             if current is None:
                 registry.copy_image(item["name"], item["digest"], stable)
-            require(registry.image_digest(config["destination_ngc_path"], item["name"], stable)
+            require(registry.image_digest("destination", item["name"], stable)
                     == item["digest"], "copied image digest does not match the RC")
             report["artifacts"].append({"kind": "image", **item, "verified": True})
             save()
         for item in chart_targets:
-            current = registry.chart(config["destination_ngc_path"], item["name"],
-                                     stable, absent=True)
+            current = registry.chart("destination", item["name"], stable, absent=True)
             require(current is None or current == prepared[item["name"]],
                     "stable chart changed during promotion")
             if current is None:
                 registry.publish_chart(item["name"], stable, prepared[item["name"]])
-            require(registry.chart(config["destination_ngc_path"], item["name"], stable)
+            require(registry.chart("destination", item["name"], stable)
                     == prepared[item["name"]], "published chart differs from the prepared package")
             report["artifacts"].append({"kind": "chart", **item, "verified": True})
             save()
@@ -596,12 +596,12 @@ def capture(config, registry, context):
         "images": [], "charts": [],
     }
     for name in sorted(config["images"]):
-        value = registry.image_digest(config["source_ngc_path"], name, rc)
+        value = registry.image_digest("source", name, rc)
         require(matches(DIGEST, value), "invalid image digest")
         registry.verify_image(name, value, context["commit"])
         result["images"].append({"name": name, "digest": value})
     for item in sorted(config["charts"], key=lambda item: item["name"]):
-        data = registry.chart(config["source_ngc_path"], item["name"], rc)
+        data = registry.chart("source", item["name"], rc)
         charts.validate(data, item["name"], rc, context["commit"])
         result["charts"].append({**item, "digest": charts.digest(data)})
     return result

.github/actions/promote-rc/tests/test_promotion.py

@@ -120,8 +120,11 @@ def __init__(self, data):
         self.fail_chart = False
         self.bad_copy = False
         self.fail_read = False
+        self.reads = []
 
-    def image_digest(self, namespace, name, version, absent=False):
+    def image_digest(self, role, name, version, absent=False):
+        p.require(role == ("source" if version == RC else "destination"), "wrong image read role")
+        self.reads.append(("image", role))
         if self.fail_read:
             raise p.PromotionError("registry unavailable")
         if version == RC:
@@ -138,7 +141,9 @@ def copy_image(self, name, digest, stable):
         self.writes.append(("image", name, digest, stable))
         self.images[name, stable] = "sha256:" + "c" * 64 if self.bad_copy else digest
 
-    def chart(self, namespace, name, version, absent=False):
+    def chart(self, role, name, version, absent=False):
+        p.require(role == ("source" if version == RC else "destination"), "wrong chart read role")
+        self.reads.append(("chart", role))
         if version == RC:
             return self.source_chart
         value = self.charts.get((name, version))
@@ -545,6 +550,36 @@ def test_complete_promotion_and_matching_rerun(self):
         self.assertEqual(self.promote("23-2")["intent"], first["intent"])
         self.assertEqual(len(self.registry.writes), 2)
 
+    def test_preflight_and_verification_reads_use_explicit_roles(self):
+        expected = [
+            ("image", "source"), ("image", "destination"),
+            ("chart", "source"), ("chart", "destination"),
+            ("image", "destination"), ("image", "destination"),
+            ("chart", "destination"), ("chart", "destination"),
+        ]
+        self.promote()
+        self.assertEqual(self.registry.reads, expected)
+        self.registry.reads.clear()
+        self.promote("23-2")
+        self.assertEqual(self.registry.reads, expected)
+
+    def test_destination_read_failure_stops_before_any_write(self):
+        for method in ("image_digest", "chart"):
+            with self.subTest(method=method):
+                self.setUp()
+                original = getattr(self.registry, method)
+
+                def read(role, *args, **kwargs):
+                    if role == "destination":
+                        raise p.PromotionError("destination read denied")
+                    return original(role, *args, **kwargs)
+
+                with patch.object(self.registry, method, side_effect=read):
+                    with self.assertRaisesRegex(p.PromotionError, "destination read denied"):
+                        self.promote()
+                self.assertIsNone(self.github.reservation)
+                self.assertEqual(self.registry.writes, [])
+
     def test_cross_namespace_charts_fail_before_any_io(self):
         candidate = config()
         candidate["destination_ngc_path"] = "example/components"
@@ -669,6 +704,7 @@ def test_captures_exact_inventory_without_publishing(self):
         registry = MemoryRegistry(data)
         self.assertEqual(p.capture(config(), registry, context), expected)
         self.assertEqual(registry.writes, [])
+        self.assertEqual(registry.reads, [("image", "source"), ("chart", "source")])
 
     def test_changed_chart_metadata_prevents_capture(self):
         context = {
@@ -774,6 +810,56 @@ def test_active_legacy_publisher_requires_common_guard_and_concurrency(self):
 
 
 class ToolTests(unittest.TestCase):
+    def test_image_reads_select_explicit_role_with_distinct_credentials(self):
+        source = "example/components-dev"
+        env = {"SOURCE_DOCKER_CONFIG": "/tmp/source-auth",
+               "DESTINATION_DOCKER_CONFIG": "/tmp/destination-auth"}
+        for destination in (source, "example/components"):
+            registry = p.Registry(source, destination)
+            for role, namespace in (("source", source), ("destination", destination)):
+                with self.subTest(destination=destination, role=role), \
+                        patch.dict(os.environ, env), patch.object(p, "command", side_effect=[
+                            b'{"schemaVersion":2}', p.canonical({"digest": IMAGE}),
+                        ]) as command:
+                    self.assertEqual(registry.image_digest(role, "service", RC), IMAGE)
+                    self.assertEqual(command.call_count, 2)
+                    for call in command.call_args_list:
+                        self.assertEqual(call.args[0][-1], f"nvcr.io/{namespace}/service:{RC}")
+                        self.assertEqual(call.kwargs["env"]["DOCKER_CONFIG"],
+                                         env[f"{role.upper()}_DOCKER_CONFIG"])
+            with patch.dict(os.environ, env), patch.object(p, "command") as command:
+                registry.verify_image("service", IMAGE, COMMIT)
+                self.assertEqual(command.call_args.kwargs["env"]["DOCKER_CONFIG"],
+                                 env["SOURCE_DOCKER_CONFIG"])
+
+    def test_chart_reads_select_explicit_role_with_equal_namespaces(self):
+        registry = p.Registry("example/components-dev", "example/components-dev")
+        payload = b"verified chart bytes"
+        for role in ("source", "destination"):
+            alias = f"promotion-{role}"
+
+            def response(args, **kwargs):
+                if args[:3] == ["helm", "repo", "update"]:
+                    self.assertEqual(args[3], alias)
+                    return b""
+                if args[:3] == ["helm", "search", "repo"]:
+                    self.assertEqual(args[3], f"{alias}/service")
+                    return p.canonical([{"name": f"{alias}/service", "version": RC}])
+                self.assertEqual(args[:3], ["helm", "pull", f"{alias}/service"])
+                (Path(args[-1]) / f"service-{RC}.tgz").write_bytes(payload)
+                return b""
+
+            with self.subTest(role=role), patch.object(p, "command", side_effect=response):
+                self.assertEqual(registry.chart(role, "service", RC), payload)
+
+    def test_registry_reads_reject_unknown_roles_before_io(self):
+        registry = p.Registry("example/components-dev")
+        with patch.object(p, "command") as command:
+            for operation in (registry.image_digest, registry.chart):
+                with self.subTest(operation=operation.__name__), self.assertRaises(p.PromotionError):
+                    operation("example/components-dev", "service", RC)
+            command.assert_not_called()
+
     def test_registry_authentication_writes_private_docker_credentials(self):
         for capture_only in (True, False):
             with self.subTest(capture_only=capture_only), tempfile.TemporaryDirectory() as directory:
@@ -785,8 +871,17 @@ def test_registry_authentication_writes_private_docker_credentials(self):
                 if not capture_only:
                     env["RC_PROMOTION_DESTINATION_NGC_KEY"] = "destination-test-key"
                     credentials["DESTINATION"] = "destination-test-key"
-                with patch.dict(os.environ, env, clear=True), patch.object(p, "command"):
+                with patch.dict(os.environ, env, clear=True), patch.object(p, "command") as command:
                     p.authenticate_registries(config(), capture_only=capture_only)
+                    if not capture_only:
+                        self.assertEqual(command.call_count, 2)
+                        for call, role in zip(command.call_args_list, ("SOURCE", "DESTINATION")):
+                            self.assertEqual(call.args[0], [
+                                "helm", "repo", "add", f"promotion-{role.lower()}",
+                                "https://helm.ngc.nvidia.com/example/components-dev",
+                                "--username", "$oauthtoken", "--password-stdin",
+                            ])
+                            self.assertEqual(call.kwargs["data"], (credentials[role] + "\n").encode())
                     for label, key in credentials.items():
                         path = Path(directory) / label.lower() / "config.json"
                         self.assertEqual(os.environ[f"{label}_DOCKER_CONFIG"], str(path.parent))
@@ -819,10 +914,10 @@ def test_registry_absence_is_narrow(self):
                 result = subprocess.CompletedProcess([], 1, b"", message.encode())
                 with patch("subprocess.run", return_value=result), self.subTest(message=message):
                     if absent:
-                        self.assertIsNone(registry.image_digest("example/components", "service", "1.2.3", True))
+                        self.assertIsNone(registry.image_digest("destination", "service", "1.2.3", True))
                     else:
                         with self.assertRaises(p.PromotionError):
-                            registry.image_digest("example/components", "service", "1.2.3", True)
+                            registry.image_digest("destination", "service", "1.2.3", True)
 
     def test_registry_digest_uses_descriptor_not_display_newline(self):
         with patch.dict(os.environ, {"SOURCE_DOCKER_CONFIG": "/tmp/no-credentials"}):
@@ -832,12 +927,12 @@ def test_registry_digest_uses_descriptor_not_display_newline(self):
             ]
             with patch("subprocess.run", side_effect=outputs):
                 self.assertEqual(p.Registry("example/components-dev").image_digest(
-                    "example/components-dev", "service", RC), IMAGE)
+                    "source", "service", RC), IMAGE)
 
     def test_warning_is_not_absence(self):
         result = subprocess.CompletedProcess([], 0, b"[]", b"cannot load index")
         with patch("subprocess.run", return_value=result), self.assertRaises(p.PromotionError):
-            p.Registry("example/components-dev").chart("example/components-dev", "service", RC, True)
+            p.Registry("example/components-dev").chart("source", "service", RC, True)
 
 
 if __name__ == "__main__":

docs/promote-rc.md

@@ -6,8 +6,9 @@ It does not build images or resolve chart dependencies.
 
 ## Product configuration
 
-Add this caller to the product's protected default branch. Pin both shared
-workflows to the same reviewed full commit SHA.
+Add this caller to the product's protected default branch. Pin each shared
+workflow to a reviewed full commit SHA. A compatible promoter update can use
+a newer revision without rebuilding the selected RC.
 
 ```yaml
 name: Promote RC
@@ -66,8 +67,8 @@ destination, even for disabled templates or values embedded in strings.
 Missing repositories and other version-value mapping shapes are unsupported and
 rejected before any publishing.
 
-Set `source_publisher_sha` to the reviewed full commit SHA pinned by the product's
-RC caller. The selected Actions attempt must reference
+Set `source_publisher_sha` to the approved full commit SHA used to publish the
+selected RC. Keep this value when updating only the promoter. The selected Actions attempt must reference
 `dsx-ai-factory/dsx-github-actions/.github/workflows/release-candidate-publish.yml`
 at exactly that immutable SHA. Missing provenance, mutable refs, different
 repositories, and different publisher revisions are rejected. The helper fetches
@@ -124,6 +125,9 @@ Before activation, repository administrators must configure:
 - Environment-only secrets `RC_PROMOTION_SOURCE_NGC_KEY` and
   `RC_PROMOTION_DESTINATION_NGC_KEY`. Use source read and destination publish
   permissions. Do not create repository or organization secrets with these names.
+  Reads select credentials by source or destination role, even when the NGC paths
+  are identical. The destination key must also read destination images and charts
+  for conflict detection and post-publication verification.
 - A GitHub App installed only on the product repository with Contents and
   Workflows write permissions. In `stable-release`, set variable
   `RC_PROMOTION_APP_ID` and secret `RC_PROMOTION_APP_PRIVATE_KEY`; do not place