Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ART-1618] [ART-2800] New signing mechanism #3817

Merged
merged 2 commits into from
Aug 3, 2023

Conversation

vfreex
Copy link
Contributor

@vfreex vfreex commented Jul 24, 2023

This PR adds a new signing mechanism to let promote-assembly job directly send signing requests through UMB.

This eliminates the intermediate step that pulling artifacts from the internet.

Basically this PR gives pyartcd the ability to sign things with pure Python implementation. So that we will not need a dedicated sign-artifiacts job and also eliminate the need to hand over artifacts between jobs. Concretely, it
comes with a class AsyncUMBClient aiming to simplify send and receive UMB messages using request/response model. (Existing python library is hard to use).

uri = "stomp+ssl://umb.stage.api.redhat.com:61612"
cert_file = "ssl/nonprod-openshift-art-bot.crt"
key_file = "ssl/nonprod-openshift-art-bot.key"  
async with AsyncUMBClient(uri, cert_file, key_file) as umb_client:
    # send a message
    await umb_client.send(topic, request_body)
    # receive messages
    receiver = await umb_client.subscribe(consumer_queue, subscription_name)
    async for message in receiver.iter_messages():
        print(message.headers["message-id"])

class AsyncSignatory sits on top of class AsyncUMBClient , which can be called from a python script to sign artifacts:

uri = "stomp+ssl://umb.stage.api.redhat.com:61612"
cert_file = "ssl/nonprod-openshift-art-bot.crt"
key_file = "ssl/nonprod-openshift-art-bot.key"
async with AsyncSignatory(uri, cert_file, key_file, sig_keyname="beta2", requestor="yuxzhu") as signatory:
    pullspec = "quay.io/openshift-release-dev/ocp-release:4.11.31-x86_64"
    digest = "sha256:cc10900ad98b44ba432bc0d99e7d4fffb5498fd6844fc3b6a0a3552ee6d64059"
    # sign a release payload
    with open("signature-1", "wb") as sig_file:
        await signatory.sign_json_digest("openshift", "4.11.31", pullspec, digest, sig_file)
    # sign a message digest
    with open("sha256sum.txt", "rb") as in_file, open("sha256sum.txt.gpg", "wb") as sig_file:
        await signatory.sign_message_digest("openshift", "4.11.31", in_file, sig_file)

promote-assembly is updated to use AsyncSignatory. sign-artifacts job is not used but I haven't removed it.

@vfreex vfreex force-pushed the new-signing branch 2 times, most recently from e595bce to b8904c2 Compare July 24, 2023 13:11
@@ -3,7 +3,7 @@
from pathlib import Path
from typing import Any, Dict, Optional

import toml
import tomli
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not related but let me explain why it is here.

When I attempted to store UMB brokers configuration in artcd.toml, I met some bugs in
toml library. It can't correctly parse configs with nested dicts and arrays.

Considering toml is not actively maintained and tomli has been adopted as a standard library in Python 3.11, I think we should move to tomli.

At the end, I decided to write a URI parser to configure UMB brokers using syntax like stomp+ssl://example.com:12345, so this became an irrelevant change.

pyartcd/pyartcd/signatory.py Outdated Show resolved Hide resolved
pyartcd/pyartcd/signatory.py Show resolved Hide resolved
pyartcd/pyartcd/pipelines/promote.py Show resolved Hide resolved
pyartcd/pyartcd/pipelines/promote.py Outdated Show resolved Hide resolved
pyartcd/pyartcd/pipelines/promote.py Outdated Show resolved Hide resolved
pyartcd/pyartcd/pipelines/promote.py Outdated Show resolved Hide resolved
self._logger.info("Signing message digest file %s...", input_path.absolute())
sig_path.parent.mkdir(parents=True, exist_ok=True)
with open(input_path, "rb") as in_file, open(sig_path, "wb") as sig_file:
if self.runtime.dry_run:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, I would rather move the dry_run check right at the beginning of the function

pyartcd/pyartcd/umb_client.py Outdated Show resolved Hide resolved
pyartcd/pyartcd/umb_client.py Outdated Show resolved Hide resolved
@vfreex vfreex force-pushed the new-signing branch 4 times, most recently from 946af17 to 9e5a955 Compare July 28, 2023 06:04
@vfreex
Copy link
Contributor Author

vfreex commented Jul 28, 2023

@vfreex vfreex changed the title New signing mechanism [ART-1618] [ART-1618] New signing mechanism Jul 28, 2023
@vfreex vfreex changed the title [ART-1618] [ART-1618] New signing mechanism [ART-1618] [ART-2800] New signing mechanism Jul 28, 2023
@locriandev
Copy link
Contributor

LGTM

This PR adds a new signing mechanism to let promote-assembly job
directly send signing requests through UMB.

This eliminates the intermediate step that pulling artifacts from the
internet.
@vfreex
Copy link
Contributor Author

vfreex commented Aug 3, 2023

Thanks for the review.
Rebased. Merging...

@vfreex vfreex merged commit 8a62c8e into openshift-eng:master Aug 3, 2023
1 check passed
vfreex added a commit to vfreex/aos-cd-jobs that referenced this pull request Aug 7, 2023
When signing a heterogeneous (multi) payload, manifests inside the
manifest list should also be signed. This is missed in
openshift-eng#3817.
vfreex added a commit to vfreex/aos-cd-jobs that referenced this pull request Aug 7, 2023
When signing a heterogeneous (multi) payload, manifests inside the
manifest list should also be signed. This is missed in
openshift-eng#3817.
ashwindasr pushed a commit to ashwindasr/pyartcd that referenced this pull request Aug 31, 2023
When signing a heterogeneous (multi) payload, manifests inside the
manifest list should also be signed. This is missed in
openshift-eng/aos-cd-jobs#3817.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants