feat(RELEASE-1387): use trusted artifacts #784

scoheb · 2025-01-23T19:20:09Z

Describe your changes

in an effort to move away from PVCs, we are trying
trusted artifacts based on https://github.com/konflux-ci/build-trusted-artifacts/tree/main
tasks that have been updated now support 2 modes:
- PVC based workspaces
- Trusted artifacts with emptyDir workspaces

Summary:

collect-data, reduce-snapshot, apply-mapping now support trusted
artifacts mode
pipelines that use apply-mapping have had the workspace used for
the apply-mapping task renamed to be in line with other tasks.
CI changes:
- it now attempts to detect if a Trusted Artifacts based task
  has been changes and runs the tests with trusted artifacts enabled.
- tests now create an on-local-cluster registry.

requires konflux-ci/release-service#666 for use on real clusters

Relevant Jira

RELEASE-1387

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

openshift-ci · 2025-01-23T19:20:14Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

tasks/managed/apply-mapping/README.md

tasks/managed/apply-mapping/apply-mapping.yaml

johnbieren · 2025-02-18T18:47:16Z

tasks/managed/apply-mapping/apply-mapping.yaml

@@ -353,3 +391,29 @@ spec:
              "${SNAPSHOT_SPEC_FILE}" > /tmp/temp && mv /tmp/temp "${SNAPSHOT_SPEC_FILE}"
            fi
        done
+    - name: create-trusted-artifact


The first step says we will skip trusted-artifacts steps if the param is empty, but I don't see any when condition on this step. Is that intended?

seems you cannot guard Steps from execution ... only Tasks in a Pipeline

the mechanism for skipping involves creating a skip file in the extract or create directory.

Gotcha. I think it may be helpful to add like a # this step does nothing is a .skip-trusted-artifacts file exists or something (assuming that is accurate) as that is not at all obvious to me

tasks/managed/apply-mapping/tests/test-apply-mapping-content-gateway-disabled.yaml

.github/scripts/deploy_registry.sh

johnbieren · 2025-02-26T19:11:33Z

.github/scripts/deploy_registry.sh

+
+prepare_docker_config() {
+  authString=$(echo -n "root:root" | base64 -w0)
+  cat > "/tmp/.dockerconfig.json" <<EOF


Any consideration of using a mktemp here instead of a static file?

well, I need to know where the file is for this operation: https://github.com/konflux-ci/release-service-catalog/pull/784/files#diff-d7a3587abb00052131d9b5ae6602c6786f2189d7daa363e550859f89ab620bf3R35

Yeah, but (and this relates to my now resolved comment) couldn't you just call the export in the deploy_registry.sh script instead of test_tekton_tasks.sh? Then you wouldn't need to know where the file is in test_tekton_tasks.sh. It seems like it would be a bit more self contained that way, but it is mostly a nit pick anyways

pipelines/managed/rh-advisories/rh-advisories-oci-ta.yaml

pipelines/managed/rh-advisories/rh-advisories-pvc.yaml

- in an effort to move away from PVCs, we are trying trusted artifacts based on: https://github.com/konflux-ci/build-trusted-artifacts/tree/main - tasks that have been updated now support 2 modes: - PVC based workspaces - Trusted artifacts with emptyDir workspaces Summary: - collect-data, reduce-snapshot, apply-mapping now support trusted artifacts mode - pipelines that use apply-mapping have had the workspace used for the apply-mapping task renamed to be in line with other tasks. - CI changes: - it now attempts to detect if a Trusted Artifacts based task has been changes and runs the tests with trusted artifacts enabled. - tests now create an on-local-cluster registry. Signed-off-by: Scott Hebert <scoheb@gmail.com>

johnbieren · 2025-02-27T19:36:33Z

tasks/managed/apply-mapping/apply-mapping.yaml

@@ -22,22 +22,62 @@ spec:
      type: string
      description: Fail the task if the resulting snapshot contains 0 components
      default: "false"
+    - name: ociStorage
+      description: The OCI repository where the Trusted Artifacts are stored.


nit - period at end (already removed in README after other comment)

johnbieren · 2025-02-27T19:36:41Z

tasks/managed/apply-mapping/apply-mapping.yaml

+      default: "empty"
+    - name: ociArtifactExpiresAfter
+      description: Expiration date for the trusted artifacts created in the
+        OCI repository. An empty string means the artifacts do not expire.


nit - period at end (already removed in README after other comment)

johnbieren · 2025-02-27T19:39:30Z

.github/scripts/deploy_registry.sh

+
+prepare_docker_config() {
+  authString=$(echo -n "root:root" | base64 -w0)
+  cat > "/tmp/.dockerconfig.json" <<EOF


Yeah, but (and this relates to my now resolved comment) couldn't you just call the export in the deploy_registry.sh script instead of test_tekton_tasks.sh? Then you wouldn't need to know where the file is in test_tekton_tasks.sh. It seems like it would be a bit more self contained that way, but it is mostly a nit pick anyways

johnbieren · 2025-02-27T19:42:29Z

tasks/managed/apply-mapping/apply-mapping.yaml

@@ -353,3 +391,29 @@ spec:
              "${SNAPSHOT_SPEC_FILE}" > /tmp/temp && mv /tmp/temp "${SNAPSHOT_SPEC_FILE}"
            fi
        done
+    - name: create-trusted-artifact


Gotcha. I think it may be helpful to add like a # this step does nothing is a .skip-trusted-artifacts file exists or something (assuming that is accurate) as that is not at all obvious to me

johnbieren · 2025-02-27T19:49:51Z

tasks/managed/apply-mapping/tests/test-apply-mapping-content-gateway-disabled.yaml

@@ -10,22 +10,36 @@ spec:
    for the component.
  workspaces:
    - name: tests-workspace
+  params:
+    - name: ociStorage


So the way this works is the test script file injects this parameter based on what mode it is being called in, right? Pretty clever

johnbieren · 2025-02-27T20:05:39Z

tasks/managed/collect-data/collect-data.yaml

Why doesn't this task need

- name: patch-source-data-artifact-result image: quay.io/konflux-ci/release-service-utils:e633d51cd41d73e4b3310face21bb980af7a662f script: | #!/usr/bin/env bash set -eu # this is needed to skip trusted-artifacts tasks # when using PVC based workspaces. if [ "$(params.ociStorage)" == "empty" ]; then echo -n "$(params.ociStorage)" > "$(results.sourceDataArtifact.path)" fi

?

johnbieren · 2025-02-27T20:10:29Z

tasks/managed/apply-mapping/apply-mapping.yaml

+    - name: sourceDataArtifact
+      type: string
+      default: ""
+    - name: subdirectory


Can you remind me why this task needs this param when collect-data already outputs each file in a subdir?

jinqi7 · 2025-03-04T07:08:46Z

tasks/managed/collect-data/collect-data.yaml

@@ -72,6 +81,12 @@ spec:
    - name: releasePipelineMetadata
      type: string
      description: json object containing git resolver metadata about the running release pipeline
+    - description: Produced trusted data artifact


It's better to use the same attribute order with the previous params.

openshift-ci bot added the do-not-merge/work-in-progress label Jan 23, 2025

scoheb force-pushed the use-trusted-artifacts branch 28 times, most recently from c5831b5 to 92310a3 Compare January 28, 2025 18:28

scoheb force-pushed the use-trusted-artifacts branch 4 times, most recently from 7764841 to b8b89bc Compare February 3, 2025 18:40

scoheb changed the title ~~feat: use trusted artifacts~~ feat(RELEASE-1291): use trusted artifacts Feb 3, 2025

scoheb marked this pull request as ready for review February 3, 2025 18:43

scoheb requested a review from a team as a code owner February 3, 2025 18:43

openshift-ci bot removed the do-not-merge/work-in-progress label Feb 3, 2025

scoheb requested a review from mmalina February 3, 2025 18:48

scoheb changed the title ~~feat(RELEASE-1291): use trusted artifacts~~ feat(RELEASE-1387): use trusted artifacts Feb 4, 2025

scoheb force-pushed the use-trusted-artifacts branch 4 times, most recently from 4adff68 to 8779560 Compare February 12, 2025 01:35

scoheb force-pushed the use-trusted-artifacts branch from 8779560 to 09144e8 Compare February 18, 2025 01:28

johnbieren reviewed Feb 18, 2025

View reviewed changes

scoheb force-pushed the use-trusted-artifacts branch 3 times, most recently from e43cbd8 to 918991a Compare February 25, 2025 15:16

johnbieren reviewed Feb 26, 2025

View reviewed changes

scoheb force-pushed the use-trusted-artifacts branch 5 times, most recently from 4aa7a92 to 52074e0 Compare February 26, 2025 21:28

scoheb requested a review from johnbieren February 26, 2025 21:48

scoheb force-pushed the use-trusted-artifacts branch from 52074e0 to a9f87cc Compare February 27, 2025 14:17

johnbieren reviewed Feb 27, 2025

View reviewed changes

jinqi7 reviewed Mar 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(RELEASE-1387): use trusted artifacts #784

feat(RELEASE-1387): use trusted artifacts #784

scoheb commented Jan 23, 2025 •

edited

Loading

openshift-ci bot commented Jan 23, 2025

johnbieren Feb 18, 2025

scoheb Feb 26, 2025

johnbieren Feb 27, 2025

johnbieren Feb 26, 2025

scoheb Feb 26, 2025

johnbieren Feb 27, 2025

johnbieren Feb 27, 2025

johnbieren Feb 27, 2025

johnbieren Feb 27, 2025

johnbieren Feb 27, 2025

johnbieren Feb 27, 2025

johnbieren Feb 27, 2025

johnbieren Feb 27, 2025

jinqi7 Mar 4, 2025

feat(RELEASE-1387): use trusted artifacts #784

Are you sure you want to change the base?

feat(RELEASE-1387): use trusted artifacts #784

Conversation

scoheb commented Jan 23, 2025 • edited Loading

Describe your changes

Relevant Jira

Checklist before requesting a review

openshift-ci bot commented Jan 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scoheb commented Jan 23, 2025 •

edited

Loading