Added optional signing_key_name to msg signer messages

Signed-off-by: Jindrich Luza <[email protected]>

Author: midnightercz

src/pubtools/sign/clients/msg.py

@@ -30,9 +30,8 @@ def on_error(self, event: proton.Event, source: Any = None) -> bool:
             return False
         self.errors.append(
             MsgError(
-                name=event,
-                description=getattr(source, "condition", None)
-                or getattr(source, "remote_condition", None),
+                name=condition.name,
+                description=condition.description,
                 source=source,
             )
         )

src/pubtools/sign/clients/msg_recv_client.py

@@ -98,8 +98,10 @@ def on_message(self, event: proton.Event) -> None:
 
         if self.recv_ids.values() and all(self.recv_ids.values()):
             self.timer_task.cancel()
-            event.receiver.close()
-            event.connection.close()
+            if event.receiver:
+                event.receiver.close()
+            if event.connection:
+                event.connection.close()
             LOG.info(self._format_log_msg("RECEIVER:All messages received", event=event))
 
     def on_timer_task(self, event: proton.Event) -> None:

src/pubtools/sign/clients/msg_send_client.py

@@ -55,7 +55,7 @@ def on_connection_opened(self, event: proton.Event) -> None:
 
     @tw.instrument_func()
     def on_sendable(self, event: proton.Event) -> None:
-        if event.sender.credit and self.sent < self.total:
+        if event.sender and event.sender.credit and self.sent < self.total:
             message = self.messages[self.sent]
             # Inject trace context to message properties
             propagator.inject(carrier=message.headers)
@@ -66,21 +66,23 @@ def on_sendable(self, event: proton.Event) -> None:
                     f"{json.dumps(message.headers)}",
                 )
             )
-            event.sender.send(
-                proton.Message(
-                    properties=message.headers,
-                    address=message.address,
-                    body=json.dumps(message.body),
+            if event.sender:
+                event.sender.send(
+                    proton.Message(
+                        properties=message.headers,
+                        address=message.address,
+                        body=json.dumps(message.body),
+                    )
                 )
-            )
             self.sent += 1
 
     def on_accepted(self, event: proton.Event) -> None:
         # LOG.info("Sender accepted")
         self.confirmed += 1
         if self.confirmed == self.total:
             LOG.info("SENDER: closing")
-            event.connection.close()
+            if event.connection:
+                event.connection.close()
             self.sender.close()
 
     def on_disconnected(self, event: proton.Event) -> None:  # pragma: no cover

src/pubtools/sign/operations/base.py

@@ -2,7 +2,7 @@
 
 from abc import ABC, abstractmethod
 
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 from typing import ClassVar, Dict, Any, Type
 from typing_extensions import Self
 
@@ -15,6 +15,13 @@ class SignOperation(ABC):
 
     ResultType: ClassVar[OperationResult]
     signing_key: str
+    signing_key_name: str = field(
+        default="",
+        metadata={
+            "description": "Signing key name which should be used for signing",
+            "sample": "key1",
+        },
+    )
 
     @classmethod
     def doc_arguments(cls: Type[Self]) -> Dict[str, Any]:

src/pubtools/sign/operations/clearsign.py

@@ -20,31 +20,35 @@ class ClearSignOperation(SignOperation):
             "description": "Signing data",
             "required": "true",
             "sample": ["input1", "input2"],
-        }
+        },
+        default_factory=list,
     )
     signing_key: str = field(
         metadata={
             "type": "str",
             "description": "Signing key short id which should be used for signing",
             "required": "true",
             "sample": "123",
-        }
+        },
+        default="",
     )
     task_id: str = field(
         metadata={
             "type": "str",
             "description": "Usually pub task id, serves as identifier for in signing request",
             "required": "true",
             "sample": "1",
-        }
+        },
+        default="",
     )
     repo: str = field(
         metadata={
             "type": "str",
             "description": "Repository name",
             "required": "true",
             "sample": "repo",
-        }
+        },
+        default="",
     )
     requester: str = field(
         metadata={"description": "Requester of the signing operation"}, default=""

src/pubtools/sign/operations/containersign.py

@@ -14,13 +14,21 @@ class ContainerSignOperation(SignOperation):
     """ContainersSignOperation model class."""
 
     ResultType: ClassVar[OperationResult]
-    digests: List[str] = field(metadata={"description": "List of digest to sign"})
-    references: List[str] = field(metadata={"description": "List of references to sign"})
+    digests: List[str] = field(
+        metadata={"description": "List of digest to sign"}, default_factory=list
+    )
+    references: List[str] = field(
+        metadata={"description": "List of references to sign"}, default_factory=list
+    )
     signing_key: str = field(
-        metadata={"description": "Signing key short id which should be used for signing"}
+        metadata={"description": "Signing key short id which should be used for signing"},
+        default="",
     )
     task_id: str = field(
-        metadata={"description": "Usually pub task id, serves as identifier for in signing request"}
+        metadata={
+            "description": "Usually pub task id, serves as identifier for in signing request"
+        },
+        default="",
     )
     identity_references: List[str] = field(
         metadata={"description": "List of references to sign"}, default_factory=list
@@ -34,6 +42,7 @@ def to_dict(self) -> dict[str, Any]:
         return dict(
             digests=self.digests,
             references=self.references,
+            signing_key_name=self.signing_key_name,
             signing_key=self.signing_key,
             task_id=self.task_id,
             requester=self.requester,

src/pubtools/sign/signers/msgsigner.py

@@ -36,6 +36,7 @@
 
 
 LOG = logging.getLogger("pubtools.sign.signers.msgsigner")
+LOG.setLevel(logging.INFO)
 
 
 class SignRequestType(str, enum.Enum):
@@ -169,6 +170,7 @@ def _construct_signing_message(
         claim: str,
         signing_key: str,
         repo: str,
+        signing_key_name: str = "",
         extra_attrs: Optional[Dict[str, Any]] = None,
         sig_type: str = SignRequestType.CONTAINER,
     ) -> dict[str, Any]:
@@ -182,6 +184,8 @@ def _construct_signing_message(
             "requested_by": self.creator,
             "repo": repo,
         }
+        if signing_key_name:
+            message["sig_keyname"] = signing_key_name
         message.update(_extra_attrs)
         return message
 
@@ -217,6 +221,7 @@ def _create_msg_message(
                 data,
                 signing_key,
                 repo,
+                signing_key_name=operation.signing_key_name,
                 extra_attrs=extra_attrs,
                 sig_type=sig_type.value,
             ),
@@ -625,6 +630,7 @@ def msg_clear_sign(
 
 def msg_container_sign(
     signing_key: str = "",
+    signing_key_name: str = "",
     task_id: str = "",
     config_file: str = "",
     digest: list[str] = [],
@@ -642,6 +648,7 @@ def msg_container_sign(
         digests=digest,
         references=reference,
         signing_key=signing_key,
+        signing_key_name=signing_key_name,
         task_id=task_id,
         requester=requester,
     )
@@ -727,6 +734,11 @@ def msg_clear_sign_main(
     required=True,
     help="8 characters key fingerprint of key which should be used for signing",
 )
+@click.option(
+    "--signing-key-name",
+    required=False,
+    help="signing key name",
+)
 @click.option("--task-id", required=True, help="Task id identifier (usually pub task-id)")
 @click.option("--config-file", default=CONFIG_PATHS[0], help="path to the config file")
 @click.option(
@@ -759,6 +771,7 @@ def msg_clear_sign_main(
 )
 def msg_container_sign_main(
     signing_key: str = "",
+    signing_key_name: str = "",
     task_id: str = "",
     config_file: str = "",
     digest: List[str] = [],
@@ -775,6 +788,7 @@ def msg_container_sign_main(
 
     ret = msg_container_sign(
         signing_key=signing_key,
+        signing_key_name=signing_key_name,
         task_id=task_id,
         config_file=config_file,
         digest=digest,

tests/test_msg_handle.py

@@ -14,15 +14,21 @@ def test_msg_handler_errors():
     msgsc = _MsgClient(errors)
     msgsc.on_link_error(mock_error)
     assert errors == [
-        MsgError(name=mock_error, description=mock_error.link.condition, source=mock_error.link)
+        MsgError(
+            name=mock_error.link.condition.name,
+            description=mock_error.link.condition.description,
+            source=mock_error.link,
+        )
     ]
 
     errors = []
     msgsc = _MsgClient(errors)
     msgsc.on_session_error(mock_error)
     assert errors == [
         MsgError(
-            name=mock_error, description=mock_error.session.condition, source=mock_error.session
+            name=mock_error.session.condition.name,
+            description=mock_error.session.condition.description,
+            source=mock_error.session,
         )
     ]
 
@@ -31,8 +37,8 @@ def test_msg_handler_errors():
     msgsc.on_connection_error(mock_error)
     assert errors == [
         MsgError(
-            name=mock_error,
-            description=mock_error.connection.condition,
+            name=mock_error.connection.condition.name,
+            description=mock_error.connection.condition.description,
             source=mock_error.connection,
         )
     ]

tests/test_msg_send_client.py

@@ -123,6 +123,8 @@ def test_non_ingored_error(
     msgsc.on_transport_error(mock_error)
     assert errors == [
         MsgError(
-            name=mock_error, description=mock_error.transport.condition, source=mock_error.transport
+            name="amqp:simulated-error",
+            description="",
+            source=mock_error.transport,
         )
     ]

tests/test_msg_signer.py

@@ -53,6 +53,34 @@ def test_msg_container_sign(f_msg_signer, f_config_msg_signer_ok):
     assert result.exit_code == 0, result.output
 
 
+def test_msg_container_sign_keyname(f_msg_signer, f_config_msg_signer_ok):
+    f_msg_signer.return_value.sign.return_value.signer_results.to_dict.return_value = {
+        "status": "ok"
+    }
+    f_msg_signer.return_value.sign.return_value.operation_result.results = []
+    f_msg_signer.return_value.sign.return_value.operation_result.signing_key = ""
+    f_msg_signer.return_value.sign.return_value.operation.to_dict.return_value = {}
+    result = CliRunner().invoke(
+        msg_container_sign_main,
+        [
+            "--signing-key",
+            "test-signing-key",
+            "--signing-key-name",
+            "test-signing-key-name",
+            "--digest",
+            "some-digest",
+            "--reference",
+            "some-reference",
+            "--task-id",
+            "1",
+            "--config-file",
+            f_config_msg_signer_ok,
+        ],
+    )
+    print(result.stdout)
+    assert result.exit_code == 0, result.output
+
+
 def test_msg_container_sign_requester(f_msg_signer, f_config_msg_signer_ok):
     f_msg_signer.return_value.sign.return_value.signer_results.to_dict.return_value = {
         "status": "ok"
@@ -408,9 +436,35 @@ def test__construct_signing_message(f_config_msg_signer_ok):
     with patch("uuid.uuid4", return_value="1234-5678-abcd-efgh"):
         with patch("pubtools.sign.signers.msgsigner.isodate_now") as patched_date:
             patched_date.return_value = "created-date-Z"
-            ret = signer._construct_signing_message("some-claim", "some-signing-key", "repo", {})
+            ret = signer._construct_signing_message(
+                "some-claim", "some-signing-key", "repo", extra_attrs={}, signing_key_name=""
+            )
+            assert ret == {
+                "sig_key_id": "ning-key",
+                "claim_file": "some-claim",
+                "request_id": "1234-5678-abcd-efgh",
+                "created": "created-date-Z",
+                "requested_by": "pubtools-sign-test",
+                "repo": "repo",
+            }
+
+
+def test__construct_signing_message_signing_key_name(f_config_msg_signer_ok):
+    signer = MsgSigner()
+    signer.load_config(load_config(f_config_msg_signer_ok))
+    with patch("uuid.uuid4", return_value="1234-5678-abcd-efgh"):
+        with patch("pubtools.sign.signers.msgsigner.isodate_now") as patched_date:
+            patched_date.return_value = "created-date-Z"
+            ret = signer._construct_signing_message(
+                "some-claim",
+                "some-signing-key",
+                "repo",
+                extra_attrs={},
+                signing_key_name="test-signing-key-name",
+            )
             assert ret == {
                 "sig_key_id": "ning-key",
+                "sig_keyname": "test-signing-key-name",
                 "claim_file": "some-claim",
                 "request_id": "1234-5678-abcd-efgh",
                 "created": "created-date-Z",
@@ -595,6 +649,7 @@ def test_clear_sign_aliases(patched_uuid, f_config_msg_signer_aliases):
                     "repo",
                     extra_attrs={"pub_task_id": "1"},
                     sig_type="clearsign_signature",
+                    signing_key_name="",
                 )
 
             assert res == SigningResults(
@@ -755,6 +810,7 @@ def test_container_sign_alias(patched_uuid, f_config_msg_signer_aliases, f_clien
                     "abcde1245",
                     "namespace/repo",
                     extra_attrs={"pub_task_id": "1", "manifest_digest": "sha256:abcdefg"},
+                    signing_key_name="",
                     sig_type="container_signature",
                 )