add e2e test for pkcs11 token signing #3495

viveksahu26 · 2024-01-23T06:45:38Z

Summary

Earlier e2e test doesn't have facility to test PKCS11, but this PR has the support for end-to-end pkcs11 token signing
Following commands being run to automate the e2e test for PKCS11:

sudo docker run -dit --name softhsm4 -p 2348:2345 vegardit/softhsm2-pkcs11-proxy

cd $HOME
apk update
# add git
apk add git
# clone cosign
git clone https://github.com/sigstore/cosign.git
# cd to cosign
cd cosign/
# add make pcsc-lite-libs go command
apk add make build-base go
# test
softhsm2-util --init-token --free --label "My Token" --pin 1234 --so-pin 1234
go test -v -cover -coverprofile=./cover.out -tags=softhsm,pkcs11key -coverpkg github.com/sigstore/cosign/v2/pkg/cosign/pkcs11key test/pkcs11_test.go

Release Note

Documentation

codecov · 2024-01-23T06:47:39Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (628df78) 40.10% compared to head (71eec70) 40.10%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3495   +/-   ##
=======================================
  Coverage   40.10%   40.10%           
=======================================
  Files         155      155           
  Lines       10044    10044           
=======================================
  Hits         4028     4028           
  Misses       5530     5530           
  Partials      486      486

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hectorj2f · 2024-01-23T09:39:57Z

test/e2e_test_pkcs11.sh

@@ -0,0 +1,32 @@
+#!/bin/bash
+
+# Copyright 2021 The Sigstore Authors.


This should be 2024

also update the line 1 to #!/usr/bin/env bash

thanks

haydentherapper · 2024-01-23T20:53:12Z

test/e2e_test_pkcs11.sh

+# add git
+apk add git
+# clone cosign
+git clone https://github.com/sigstore/cosign.git


This would test Cosign at HEAD rather than on the PR.

haydentherapper · 2024-01-23T20:53:46Z

test/e2e_test.sh

+# Test pkcs11 token signing
+echo "testing pkcs11 token signing"
+CONTAINER_ID=$(sudo docker run -dit --name softhsm -p 2345:2345 vegardit/softhsm2-pkcs11-proxy)
+sudo docker exec -i  $CONTAINER_ID /bin/bash < ./test/e2e_test_pkcs11.sh


Can this be run without sudo? I'd prefer that the tests could be run locally without needed this.

Yeah, sure.

haydentherapper · 2024-01-23T21:00:43Z

test/e2e_test.sh

@@ -79,6 +79,11 @@ docker run -d -p 5000:5000 --restart always -e REGISTRY_STORAGE_DELETE_ENABLED=t
 export COSIGN_TEST_REPO=localhost:5000
 go test -tags=e2e -v ./test/... -run TestSignVerifyClean

+# Test pkcs11 token signing
+echo "testing pkcs11 token signing"
+CONTAINER_ID=$(sudo docker run -dit --name softhsm -p 2345:2345 vegardit/softhsm2-pkcs11-proxy)


Need to cleanup container after the test completed

haydentherapper · 2024-01-31T03:31:33Z

test/e2e_test.sh

@@ -79,6 +79,15 @@ docker run -d -p 5000:5000 --restart always -e REGISTRY_STORAGE_DELETE_ENABLED=t
 export COSIGN_TEST_REPO=localhost:5000
 go test -tags=e2e -v ./test/... -run TestSignVerifyClean

+# Test pkcs11 token signing


Can we put this in its own shell script and workflow, following the pattern of other e2e tests, so it runs in parallel?

haydentherapper · 2024-01-31T03:31:45Z

test/e2e_test.sh

@@ -79,6 +79,15 @@ docker run -d -p 5000:5000 --restart always -e REGISTRY_STORAGE_DELETE_ENABLED=t
 export COSIGN_TEST_REPO=localhost:5000
 go test -tags=e2e -v ./test/... -run TestSignVerifyClean

+# Test pkcs11 token signing
+echo "testing pkcs11 token signing"


Remove testing line

haydentherapper · 2024-01-31T03:32:10Z

test/e2e_test.sh

@@ -79,6 +79,15 @@ docker run -d -p 5000:5000 --restart always -e REGISTRY_STORAGE_DELETE_ENABLED=t
 export COSIGN_TEST_REPO=localhost:5000
 go test -tags=e2e -v ./test/... -run TestSignVerifyClean

+# Test pkcs11 token signing
+echo "testing pkcs11 token signing"
+CONTAINER_ID=$(docker run -dit --name softhsm -v $(pwd):/root/cosign -p 2345:2345 vegardit/softhsm2-pkcs11-proxy)


Can we pin the docker container by hash?

haydentherapper

Just one small thing, moving the test to a different GHA. Thanks!

cc @cpanato if you any other comments

haydentherapper · 2024-01-31T18:54:58Z

.github/workflows/tests.yaml

@@ -119,6 +119,9 @@ jobs:
      - name: Run end-to-end tests
        run: ./test/e2e_test.sh

+      - name: Run pkcs11 end-to-end tests


Can you add this as a dedicated job in https://github.com/sigstore/cosign/blob/main/.github/workflows/e2e-tests.yml?

haydentherapper · 2024-01-31T18:57:04Z

test/e2e_test_pkcs11.sh

+# Test pkcs11 token signing
+CONTAINER_ID=$(docker run -dit --name softhsm -v $(pwd):/root/cosign -p 2345:2345 vegardit/softhsm2-pkcs11-proxy@sha256:557a65d2a14e3986f2389d36ddce75609cbd8fb7ee6cf08a78adcc8236c2a80e)
+
+docker exec -i  $CONTAINER_ID /bin/bash << 'EOF'


Suggested change

docker exec -i $CONTAINER_ID /bin/bash << 'EOF'

docker exec -i $CONTAINER_ID /bin/bash << 'EOF'

cpanato

thanks for this pr a few changes

cpanato · 2024-02-02T07:24:26Z

test/e2e_test_pkcs11.sh

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -ex


Can you make this explicit?

like

set -o errexit set -o nounset set -o pipefail

just need to update this

cpanato · 2024-02-02T07:27:15Z

.github/workflows/e2e-tests.yml

@@ -73,3 +76,17 @@ jobs:
      - name: Run e2e_signblob_tsa_mtls.sh
        shell: bash
        run: make && PATH="$PWD:$PATH" ./test/e2e_signblob_tsa_mtls.sh
+
+  e2e-test-pkcs11:
+    name: Run pkcs11 e2e tests


you can drop this

cpanato · 2024-02-02T07:27:34Z

.github/workflows/e2e-tests.yml

+          go-version: ${{ env.GO_VERSION }}
+          check-latest: true
+
+      - name: Run pkcs11 end-to-end tests


set the shell to bash like the others

Signed-off-by: Vivek Kumar Sahu <[email protected]> add license Signed-off-by: Vivek Kumar Sahu <[email protected]> small fix Signed-off-by: Vivek Kumar Sahu <[email protected]> update shebang portable with cross platform Signed-off-by: Vivek Kumar Sahu <[email protected]> enable exit on error and xtrace mode Signed-off-by: Vivek Kumar Sahu <[email protected]> cleanup container Signed-off-by: Vivek Kumar Sahu <[email protected]> pkcs11 test with upcoming changes Signed-off-by: Vivek Kumar Sahu <[email protected]> run pkcs11 e2e test in a separate workflow Signed-off-by: Vivek Kumar Sahu <[email protected]> add pkcs11 test in separate workflow Signed-off-by: Vivek Kumar Sahu <[email protected]>

Signed-off-by: Vivek Kumar Sahu <[email protected]>

haydentherapper · 2024-02-05T06:30:09Z

@cpanato PTAL

cpanato

i will do any adjust in a followup

cpanato · 2024-02-05T15:34:27Z

thanks for this pr!

hectorj2f reviewed Jan 23, 2024

View reviewed changes

haydentherapper requested changes Jan 23, 2024

View reviewed changes

viveksahu26 requested a review from haydentherapper January 25, 2024 10:22

haydentherapper requested changes Jan 31, 2024

View reviewed changes

viveksahu26 force-pushed the e2e_test_pkcs11 branch from 35352ee to 1b311d2 Compare January 31, 2024 11:02

viveksahu26 requested a review from haydentherapper January 31, 2024 18:41

haydentherapper reviewed Jan 31, 2024

View reviewed changes

haydentherapper previously approved these changes Feb 1, 2024

View reviewed changes

viveksahu26 force-pushed the e2e_test_pkcs11 branch from 86b4fdf to 4f20f72 Compare February 2, 2024 03:33

cpanato requested changes Feb 2, 2024

View reviewed changes

viveksahu26 added 2 commits February 2, 2024 15:37

set shell to bash

5ed1fab

Signed-off-by: Vivek Kumar Sahu <[email protected]>

viveksahu26 dismissed haydentherapper’s stale review via 5ed1fab February 2, 2024 10:08

viveksahu26 force-pushed the e2e_test_pkcs11 branch from 4f20f72 to 5ed1fab Compare February 2, 2024 10:08

set shell options

71eec70

Signed-off-by: Vivek Kumar Sahu <[email protected]>

haydentherapper approved these changes Feb 2, 2024

View reviewed changes

cpanato approved these changes Feb 5, 2024

View reviewed changes

cpanato merged commit 7951940 into sigstore:main Feb 5, 2024

github-actions bot added this to the v2.3.0 milestone Feb 5, 2024

		@@ -0,0 +1,32 @@
		#!/bin/bash

		# Copyright 2021 The Sigstore Authors.

	docker exec -i $CONTAINER_ID /bin/bash << 'EOF'
	docker exec -i $CONTAINER_ID /bin/bash << 'EOF'

add e2e test for pkcs11 token signing #3495

add e2e test for pkcs11 token signing #3495

Uh oh!

Conversation

viveksahu26 commented Jan 23, 2024

Summary

Release Note

Documentation

Uh oh!

codecov bot commented Jan 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haydentherapper left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpanato left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haydentherapper commented Feb 5, 2024

Uh oh!

cpanato left a comment

Choose a reason for hiding this comment

Uh oh!

cpanato commented Feb 5, 2024

Uh oh!

Uh oh!

codecov bot commented Jan 23, 2024 •

edited

Loading