feat(CLOUDDST-25034): run rh-sign-image-cosign signing in parallel

rh-sign-image-cosign now checks for existing signatures and sign only when signature for given identity and digest is not found. Also whole procedure now runs in parallel. Signed-off-by: Jindrich Luza <[email protected]>
konflux-ci · Dec 2, 2024 · 94e8c27 · 94e8c27
1 parent b6ec0c7
commit 94e8c27
Show file tree

Hide file tree

Showing 7 changed files with 204 additions and 38 deletions.
diff --git a/tasks/rh-sign-image-cosign/README.md b/tasks/rh-sign-image-cosign/README.md
@@ -10,6 +10,11 @@ Tekton task to sign container images in snapshot by cosign.
 | secretName             | Name of secret containing needed credentials                                                                                                                                                                                                      | No       | -             |
 | signRegistryAccessPath | The relative path in the workspace to a text file that contains a list of repositories that needs registry.access.redhat.com image references to be signed (i.e. requires_terms=true), one repository string per line, e.g. "rhtas/cosign-rhel9". | No       | -             |
 | retries                | Retry cosign N times                                                                                                                                                                                                                              | Yes      | 3             |
+| concurrentLimit       | Number of concurrent cosign operations                                                                                                                                                                                                            | Yes      | 5             |
+
+## Changes in 1.3.0
+* Containers are signed only if the signature doesn't exist in the destination image
+* Existing signature validation and signing are done in parallel now controlled by concurrencyLimit paremeter
 
 ## Changes in 1.2.0
 * Retry failed cosign

diff --git a/tasks/rh-sign-image-cosign/rh-sign-image-cosign.yaml b/tasks/rh-sign-image-cosign/rh-sign-image-cosign.yaml
@@ -4,7 +4,7 @@ kind: Task
 metadata:
   name: rh-sign-image-cosign
   labels:
-    app.kubernetes.io/version: "1.2.0"
+    app.kubernetes.io/version: "1.3.0"
   annotations:
     tekton.dev/pipelines.minVersion: "0.12.1"
     tekton.dev/tags: release
@@ -28,6 +28,10 @@ spec:
       description: Retry cosign N times.
       type: string
       default: "3"
+    - name: concurrentLimit
+      type: string
+      default: 5
+      description: The maximum number of concurrent cosign signing jobs
   workspaces:
     - name: data
       description: Workspace to read and save files
@@ -61,6 +65,11 @@ spec:
               name: $(params.secretName)
               key: REKOR_URL
               optional: true
+        - name: PUBLIC_KEY
+          valueFrom:
+            secretKeyRef:
+              name: $(params.secretName)
+              key: PUBLIC_KEY
       script: |
         #!/usr/bin/env bash
         set -eux
@@ -73,28 +82,79 @@ spec:
             echo "No valid file was provided as signRegistryAccessPath."
             exit 1
         fi
+        PUBLIC_KEY_FILE=$(mktemp)
+        echo -n "$PUBLIC_KEY" > "$PUBLIC_KEY_FILE"
+        RUNNING_JOBS="\j" # Bash parameter for number of jobs currently running
 
+        jobpid(){
+            pid=$(cut -d' ' -f4 < /proc/self/stat)
+            echo "$pid"
+        }
+        echopid(){
+            pid=$(jobpid)
+            echo "${pid}: $*"
+        }
         run_cosign () { # Expected arguments are [digest_reference, tag_reference]
-            # Upload transparency log when rekor url is specified
-            if [ -v REKOR_URL ]; then
-                COSIGN_COMMON_ARGS="-y --rekor-url=$REKOR_URL --key $SIGN_KEY"
-            else
-                COSIGN_COMMON_ARGS="--tlog-upload=false --key $SIGN_KEY"
-            fi
-            echo "Signing manifest $1 ($2)"
             attempt=0
+            backoff1=2
+            backoff2=3
             until [ "$attempt" -gt "$(params.retries)" ] ; do # 3 retries by default
-                cosign -t 3m0s sign\
-                ${COSIGN_COMMON_ARGS}\
-                --sign-container-identity "$2"\
-                "$1" && break
+                cosign "$@" && break
+                sleep $backoff2
+
+                # Fibbonaci backoff
+                old_backoff1=$backoff1
+                backoff1=$backoff2
+                backoff2=$((old_backoff1 + backoff2))
+
                 attempt=$((attempt+1))
             done
             if [ "$attempt" -gt "$(params.retries)" ] ; then
-              echo "Max retries exceeded."
+              echopid "Max retries exceeded."
               exit 1
             fi
         }
+        function check_existing_signatures() {
+          local identity=$1
+          local reference=$2
+          local digest=$3
+          if [ -v REKOR_URL ]; then
+              COSIGN_REKOR_ARGS="--rekor-url=$REKOR_URL"
+          else
+              COSIGN_REKOR_ARGS="--insecure-ignore-tlog=true"
+          fi
+          verify_output=$(run_cosign verify "$COSIGN_REKOR_ARGS" "$reference")
+          found_signatures=$(echo "$verify_output" | jq -j '['\
+        '.[]|select(.critical.image."docker-manifest-digest"| contains("'"$digest"'"))'\
+        '|select(.critical.identity."docker-reference"| contains("'"$identity"'"))'\
+        ']|length')
+          echo "$found_signatures"
+        }
+        function check_and_sign() {
+          local identity=$1
+          local reference=$2
+          local tag=$3
+          local digest=$4
+          found_signatures=$(check_existing_signatures "$identity" "$reference:$tag" "$digest")
+          if [ -z "$found_signatures" ]; then
+            found_signatures=0
+          fi
+          echopid "FOUND SIGNATURES for ${identity} ${digest}: $found_signatures"
+
+          if [ -v REKOR_URL ]; then
+              COSIGN_REKOR_ARGS="-y --rekor-url=$REKOR_URL"
+          else
+              COSIGN_REKOR_ARGS="--tlog-upload=false"
+          fi
+
+          if [ "$found_signatures" -eq 0 ]; then
+            run_cosign -t 3m0s sign "$COSIGN_REKOR_ARGS" \
+              --key "$SIGN_KEY" \
+              --sign-container-identity "$identity" "$reference@$digest"
+          else
+            echopid "Skip signing ${identity} (${digest})"
+          fi
+        }
 
         for (( COMPONENTS_INDEX=0; COMPONENTS_INDEX<COMPONENTS_LENGTH; COMPONENTS_INDEX++ )); do
             COMPONENT_NAME=$(jq -r ".components[${COMPONENTS_INDEX}].name" "${SNAPSHOT_PATH}")
@@ -129,7 +189,10 @@ spec:
                 for REGISTRY_REF in "${REGISTRY_REFERENCES[@]}"; do
                     for MDIGEST in $(echo "$IMAGE" | jq -r '.manifests[]|.digest'); do
                         for TAG in $TAGS; do
-                            run_cosign "${INTERNAL_CONTAINER_REF}@${MDIGEST}" "${REGISTRY_REF}:${TAG}"
+                            while (( ${RUNNING_JOBS@P} >= $(params.concurrentLimit) )); do
+                                wait -n
+                            done
+                            check_and_sign "${REGISTRY_REF}:${TAG}" "${INTERNAL_CONTAINER_REF}" "${TAG}" "${MDIGEST}" &
                         done
                     done
                 done
@@ -138,8 +201,14 @@ spec:
             # Sign manifest list itself or manifest if it's not list
             for REGISTRY_REF in "${REGISTRY_REFERENCES[@]}"; do
                 for TAG in $TAGS; do
-                    run_cosign "${INTERNAL_CONTAINER_REF}@${DIGEST}" "${REGISTRY_REF}:${TAG}"
+                    while (( ${RUNNING_JOBS@P} >= $(params.concurrentLimit) )); do
+                        wait -n
+                    done
+                    check_and_sign "${REGISTRY_REF}:${TAG}" "${INTERNAL_CONTAINER_REF}" "${TAG}" "${DIGEST}" &
                 done
             done
         done
+        while (( ${RUNNING_JOBS@P} > 0 )); do
+            wait -n
+        done
         echo "done"
diff --git a/tasks/rh-sign-image-cosign/tests/mocks.sh b/tasks/rh-sign-image-cosign/tests/mocks.sh
@@ -137,14 +137,28 @@ function skopeo() {
   fi
 }
 function cosign () {
-  echo "$@" >> $(workspaces.data.path)/mock_cosign_calls
-  echo "running cosign: $@"
-  # mock cosign failing the first 3 calls for the retry test
-  if [[ "$@" == *":retry-tag"* ]]
-  then
-    if [[ $(cat $(workspaces.data.path)/mock_cosign_calls | wc -l) -le 3 ]]
-    then
-      echo "expected cosign call failure for retry test"
+  # check if call should end successfully
+  # mock_cosign_success_calls file is expected to contain lines with "1" or "0" where
+  # "1" means that the call should end successfully and "0" means that the call should end with an error
+  # following command pops the first line from the file and stores it in successfull_run variable
+  successfull_run=$(sed -n '1p' $(workspaces.data.path)/mock_cosign_success_calls && \
+    sed -i '1d' $(workspaces.data.path)/mock_cosign_success_calls)
+
+  if [ "$1" = "verify" ]; then
+    mock_existing_sig_file=$(echo "${*: -1}" | tr "/" "-")
+    echo "$@" >> $(workspaces.data.path)/mock_cosign_verify_calls
+    # if the call shouldn't end successfully, exit with error
+    if [ "$successfull_run" != "1" ]; then
+      return 1
+    fi
+    cat "$(workspaces.data.path)/$mock_existing_sig_file"
+  else
+    echo "running cosign: $@"
+    echo "$@" >> "$(workspaces.data.path)/mock_cosign_sign_calls"
+    # if the call shouldn't end successfully, exit with error
+    if [ "$successfull_run" != "1" ]; then
+      >&2 echo "- SIMULATED ERROR -"
+      echo "- SIMULATED ERROR -" >> "$(workspaces.data.path)/mock_cosign_sign_calls"
       return 1
     fi
   fi

diff --git a/tasks/rh-sign-image-cosign/tests/pre-apply-task-hook.sh b/tasks/rh-sign-image-cosign/tests/pre-apply-task-hook.sh
@@ -7,14 +7,16 @@ kubectl create secret generic test-cosign-secret\
   --from-literal=AWS_DEFAULT_REGION=us-test-1\
   --from-literal=AWS_ACCESS_KEY_ID=test-access-key\
   --from-literal=AWS_SECRET_ACCESS_KEY=test-secret-access-key\
-  --from-literal=SIGN_KEY=aws://arn:mykey
+  --from-literal=SIGN_KEY=aws://arn:mykey\
+  --from-literal=PUBLIC_KEY=public_key
 
 kubectl create secret generic test-cosign-secret-rekor\
   --from-literal=AWS_DEFAULT_REGION=us-test-1\
   --from-literal=AWS_ACCESS_KEY_ID=test-access-key\
   --from-literal=AWS_SECRET_ACCESS_KEY=test-secret-access-key\
   --from-literal=SIGN_KEY=aws://arn:mykey\
-  --from-literal=REKOR_URL=https://fake-rekor-server
+  --from-literal=REKOR_URL=https://fake-rekor-server\
+  --from-literal=PUBLIC_KEY=public_key
 
 # Add mocks to the beginning of task step script
 TASK_PATH="$1"

diff --git a/tasks/rh-sign-image-cosign/tests/test-rh-sign-image-cosign-multiple-components.yaml b/tasks/rh-sign-image-cosign/tests/test-rh-sign-image-cosign-multiple-components.yaml
@@ -48,6 +48,21 @@ spec:
                 ]
               }
               EOF
+              REF11="quay.io/redhat-pending/test-product----test-image0:t1"
+              REF12="quay.io/redhat-pending/test-product----test-image0:t2"
+              REF21="quay.io/redhat-pending/test-product----test-image1:t1"
+              REF22="quay.io/redhat-pending/test-product----test-image1:t2"
+
+              # create empty cosign verify mock files
+              touch "$(workspaces.data.path)/$(echo $REF11 | tr '/' '-')"
+              touch "$(workspaces.data.path)/$(echo $REF12 | tr '/' '-')"
+              touch "$(workspaces.data.path)/$(echo $REF21 | tr '/' '-')"
+              touch "$(workspaces.data.path)/$(echo $REF22 | tr '/' '-')"
+
+              # setup cosign success calls - all calls should pass
+              for _ in $(seq 1 48); do
+                  echo "1" >> "$(workspaces.data.path)/mock_cosign_success_calls"
+              done
 
               cat > "$(workspaces.data.path)/signRegistryAccess.txt" << EOF
               test-product/test-image0
@@ -62,6 +77,8 @@ spec:
           value: 'test-cosign-secret-rekor'
         - name: signRegistryAccessPath
           value: signRegistryAccess.txt
+        - name: concurrentLimit
+          value: 1
       workspaces:
         - name: data
           workspace: tests-workspace
@@ -93,7 +110,7 @@ spec:
               CALLS=$(cat "$(workspaces.data.path)/mock_skopeo_calls")
               test "$CALLS" = "$EXPECTED"
 
-              CALLS=$(cat "$(workspaces.data.path)/mock_cosign_calls")
+              CALLS=$(cat "$(workspaces.data.path)/mock_cosign_sign_calls")
               COSIGN_COMMON="-t 3m0s sign -y --rekor-url=https://fake-rekor-server --key aws://arn:mykey \
               --sign-container-identity"
               EXPECTED=$(cat <<EOF
@@ -123,6 +140,15 @@ spec:
               $COSIGN_COMMON ${_TEST_PUB_REPO3}:t2 ${_TEST_REPO2}@sha256:1111
               EOF
               )
-              test "$CALLS" = "$EXPECTED"
+              echo "TESTING"
+              if [ "$CALLS" != "$EXPECTED" ]; then
+                echo "Diff:"
+                CALLS_FILE=$(mktemp XXXXX.calls)
+                EXPECTED_FILE=$(mktemp XXXXX.expected)
+                echo "$CALLS" > "$CALLS_FILE"
+                echo "$EXPECTED" > "$EXPECTED_FILE"
+                diff -Naur "$EXPECTED_FILE" "$CALLS_FILE"
+                exit 1
+              fi
       runAfter:
         - run-task
diff --git a/tasks/rh-sign-image-cosign/tests/test-rh-sign-image-cosign-retries.yaml b/tasks/rh-sign-image-cosign/tests/test-rh-sign-image-cosign-retries.yaml
@@ -31,12 +31,28 @@ spec:
                     "repository": "quay.io/redhat-pending/test-product----test-image2",
                     "rh-registry-repo": "registry.stage.redhat.io/test-product/test-image2",
                     "registry-access-repo": "registry.access.stage.redhat.com/test-product/test-image2",
-                    "tags": ["retry-tag"]
+                    "tags": ["t1", "t2"]
                   }
                 ]
               }
               EOF
 
+              # create empty cosign verify mock files
+              REF1="quay.io/redhat-pending/test-product----test-image2:t1"
+              touch "$(workspaces.data.path)/$(echo $REF1 | tr '/' '-')"
+              REF2="quay.io/redhat-pending/test-product----test-image2:t2"
+              touch "$(workspaces.data.path)/$(echo $REF2 | tr '/' '-')"
+
+              # first 3 cosign calls should end with success
+              for _ in $(seq 1 3); do
+                  echo "1" >> "$(workspaces.data.path)/mock_cosign_success_calls"
+              done
+              # simulate cosign failure on 6th call
+              echo "0" >> "$(workspaces.data.path)/mock_cosign_success_calls"
+
+              # after retrying, it should pass
+              echo "1" >> "$(workspaces.data.path)/mock_cosign_success_calls"
+
               cat > "$(workspaces.data.path)/signRegistryAccess.txt" << EOF
               test-product/test-image0
               EOF
@@ -52,6 +68,8 @@ spec:
           value: signRegistryAccess.txt
         - name: retries
           value: 3
+        - name: concurrentLimit
+          value: 1
       workspaces:
         - name: data
           workspace: tests-workspace
@@ -72,15 +90,24 @@ spec:
               _TEST_PUB_REPO="registry.stage.redhat.io/test-product/test-image2"
               _TEST_REPO="quay.io/redhat-pending/test-product----test-image2"
 
-              CALLS=$(cat "$(workspaces.data.path)/mock_cosign_calls")
+              CALLS=$(cat "$(workspaces.data.path)/mock_cosign_sign_calls")
               COSIGN_COMMON="-t 3m0s sign --tlog-upload=false --key aws://arn:mykey --sign-container-identity"
               EXPECTED=$(cat <<EOF
-              $COSIGN_COMMON ${_TEST_PUB_REPO}:retry-tag ${_TEST_REPO}@sha256:2222
-              $COSIGN_COMMON ${_TEST_PUB_REPO}:retry-tag ${_TEST_REPO}@sha256:2222
-              $COSIGN_COMMON ${_TEST_PUB_REPO}:retry-tag ${_TEST_REPO}@sha256:2222
-              $COSIGN_COMMON ${_TEST_PUB_REPO}:retry-tag ${_TEST_REPO}@sha256:2222
+              $COSIGN_COMMON ${_TEST_PUB_REPO}:t1 ${_TEST_REPO}@sha256:2222
+              $COSIGN_COMMON ${_TEST_PUB_REPO}:t2 ${_TEST_REPO}@sha256:2222
+              - SIMULATED ERROR -
+              $COSIGN_COMMON ${_TEST_PUB_REPO}:t2 ${_TEST_REPO}@sha256:2222
               EOF
               )
-              test "$CALLS" = "$EXPECTED"
+              echo "TESTING"
+              if [ "$CALLS" != "$EXPECTED" ]; then
+                echo "Diff:"
+                CALLS_FILE=$(mktemp XXXXX.calls)
+                EXPECTED_FILE=$(mktemp XXXXX.expected)
+                echo "$CALLS" > "$CALLS_FILE"
+                echo "$EXPECTED" > "$EXPECTED_FILE"
+                diff -Naur "$EXPECTED_FILE" "$CALLS_FILE"
+                exit 1
+              fi
       runAfter:
         - run-task