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

feat(CLOUDDST-25758): use pubtools-sign and pubtools-pyxis to sign and upload images #808

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

midnightercz
Copy link
Contributor

Internal request request-and-upload-signature was transfered to use pubtools-sign and pubtools-pyxis to sign and upload signatures. With that, internal requests are now called with multiple references and digests. Batch size is limited by params string length which is set by default to 4096. Retry mechanism in signing was also removed as it's already included in pubtools-sign

Describe your changes

Relevant Jira

Checklist before requesting a review

  • I have marked as draft or added do not merge label if there's a dependency PR
    • If you want reviews on your draft PR, you can add reviewers or add the release-service-maintainers handle if you are unsure who to tag
  • My commit message includes Signed-off-by: My name <email>
  • I have bumped the task/pipeline version string and updated changelog in the relevant README
  • I read CONTRIBUTING.MD and commit formatting

@midnightercz midnightercz requested a review from a team as a code owner February 6, 2025 14:30
Copy link

openshift-ci bot commented Feb 6, 2025

Hi @midnightercz. Thanks for your PR.

I'm waiting for a konflux-ci member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@midnightercz midnightercz force-pushed the pubtool-sign-integration branch 6 times, most recently from 5b8a3f0 to bb05b9c Compare February 7, 2025 09:43
Copy link
Collaborator

@johnbieren johnbieren left a comment

Choose a reason for hiding this comment

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

Can you update the version strings in the yaml files and update the READMEs? Should be a Changes in entry and also the parameters were changed. It isn't clear to me (and not mentioned on the pr) why requester is being removed - I am pretty sure we need that for tracking purposes

@midnightercz midnightercz force-pushed the pubtool-sign-integration branch 4 times, most recently from 4147777 to 057e003 Compare February 17, 2025 09:01
@midnightercz midnightercz force-pushed the pubtool-sign-integration branch from 12d9ec6 to ea282f6 Compare February 20, 2025 10:02
internal request request-and-upload-signature was transfered to
use pubtools-sign and pubtools-pyxis to sign and upload signatures.
With that, internal requests are now called with multiple references and
digests. Batch size is limited by params string length which is set by
default to 4096. Retry mechanism in signing was also removed as it's
already included in pubtools-sign

Signed-off-by: Jindrich Luza <[email protected]>
- Updated readmes
- fixed pubtools-sign-msg-container-sign mock

Signed-off-by: Jindrich Luza <[email protected]>
@midnightercz midnightercz force-pushed the pubtool-sign-integration branch from ea282f6 to f08081e Compare February 20, 2025 10:04
@johnbieren
Copy link
Collaborator

/retest

Copy link
Collaborator

@johnbieren johnbieren left a comment

Choose a reason for hiding this comment

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

One small comment but otherwise lgtm. The PR is large, so I am going to ask for someone else on my team to review it too

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see

          retries: 1
          send_retries: 2

in the new task definition. So why is it no longer worth testing that the retries work?

@konflux-ci-qe-bot
Copy link

@midnightercz: The following test has Failed, say /retest to rerun failed tests.

PipelineRun Name Status Rerun command Build Log Test Log
konflux-e2e-tests-catalog-j8dd9 Failed /retest View Pipeline Log View Test Logs

Inspecting Test Artifacts

To inspect your test artifacts, follow these steps:

  1. Install ORAS (see the ORAS installation guide).
  2. Download artifacts with the following commands:
mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-test-storage/konflux-team/release-service-catalog:konflux-e2e-tests-catalog-j8dd9

Test results analysis

🚨 Error occurred while running the E2E tests, list of failed Spec(s):

➡️ [failed] [It] [release-pipelines-suite FBC e2e-tests] with FBC hotfix process FBC hotfix post-release verification verifies the fbc release pipelinerun is running and succeeds [release-pipelines, fbc-tests, fbcHotfix]

Click to view logs

PipelineRun managed-2b94v failed
Expected
    : Pipelinerun 'managed-2b94v' didn't succeed\nLogs from failed container 'managed-2b94v-sign-index-image/step-sign-index-image': \nCreating internal-request to sign image:\n- reference=registry.redhat.io/redhat/preview-operator-index:v4.12-bz12345-1740084634\n- manifest_digest=sha256:ce95f780ccf8cb0b2495a9cd35905e0c7d2d8d37b8d1450e689fb65a0540eb15\n- requester=jinqi-1\nInternalRequest 'simple-signing-pipeline-vlvpc' created.\nSync flag set to true. Waiting for the InternalRequest to be completed.\nChecking IR statuses...\nFound 1 InternalRequests matching the name or label\nConditions:\n  simple-signing-pipeline-vlvpc: running\nChecking IR statuses...\nFound 1 InternalRequests matching the name or label\nConditions:\n  simple-signing-pipeline-vlvpc: Failed\nAll InternalRequests have been completed\nERROR: At least one InternalRequest failed\nConditions:\n  simple-signing-pipeline-vlvpc: [{\"lastTransitionTime\":\"2025-02-20T20:57:12Z\",\"message\":\"[User error] Validation failed for pipelinerun internalrequest-b5m9g with error invalid input params for task request-and-upload-signature: missing values for these params which have no default values: [manifest_digests references]\",\"reason\":\"Failed\",\"status\":\"False\",\"type\":\"Succeeded\"}]\nResult: failure\n
to equal
    : 

➡️ [failed] [It] [release-pipelines-suite e2e tests for rh-advisories pipeline] Rh-advisories happy path Post-release verification verifies the advs release pipelinerun is running and succeeds [release-pipelines, rh-advisories, rhAdvisories]

Click to view logs

Timed out after 3600.001s.
timed out when waiting for the release PipelineRun to be finished for the release snapshot-sample-dekh-hhrq9/dev-release-team-tenant
Expected success, but got an error:
    <*errors.errorString | 0xc001d06fa0>: 
    PipelineRun managed-zw5d9 has still not finished yet
    {
        s: "PipelineRun managed-zw5d9 has still not finished yet",
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants