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

Use ClusterImagePolicy with Keyless + e2e tests for CIP with kind #1650

Merged
merged 8 commits into from
Mar 25, 2022

Conversation

vaikas
Copy link
Contributor

@vaikas vaikas commented Mar 23, 2022

Summary

  • Plumb through the Keyless validation from ClusterImagePolicy

Ticket Link

Fixes

Release Note

 * Start enforcing ClusterImagePolicy. Supports both Key, Keyless Authorities.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
@vaikas vaikas force-pushed the scaffolding branch 5 times, most recently from 8cbd24c to 3c3903f Compare March 23, 2022 23:22
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
we need to make sure that all policies have at least one validating
authority.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
understand how to test fully with both rekor/fulcio the good case.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2022

Codecov Report

Merging #1650 (58ff276) into main (b102404) will increase coverage by 0.87%.
The diff coverage is 78.57%.

@@            Coverage Diff             @@
##             main    #1650      +/-   ##
==========================================
+ Coverage   27.89%   28.77%   +0.87%     
==========================================
  Files         139      139              
  Lines        7972     8139     +167     
==========================================
+ Hits         2224     2342     +118     
- Misses       5512     5544      +32     
- Partials      236      253      +17     
Impacted Files Coverage Δ
pkg/cosign/kubernetes/webhook/validator.go 80.49% <76.19%> (-1.22%) ⬇️
pkg/apis/config/image_policies.go 69.38% <100.00%> (ø)
pkg/cosign/kubernetes/webhook/validation.go 80.21% <100.00%> (+1.64%) ⬆️
pkg/cosign/tuf/client.go 62.34% <0.00%> (-0.95%) ⬇️
pkg/cosign/common.go 0.00% <0.00%> (ø)
cmd/cosign/cli/clean.go 0.00% <0.00%> (ø)
cmd/cosign/cli/options/clean.go 0.00% <0.00%> (ø)
pkg/cosign/verify.go 16.32% <0.00%> (+4.65%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b102404...58ff276. Read the comment docs.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Comment on lines +149 to +211
# These set up the env variables so that
- name: Set the endpoints on the cluster and grab secrets
run: |
REKOR_URL=`kubectl -n rekor-system get --no-headers ksvc rekor | cut -d ' ' -f 4`
echo "REKOR_URL=$REKOR_URL" >> $GITHUB_ENV
curl -s $REKOR_URL/api/v1/log/publicKey > ./rekor-public.pem

FULCIO_URL=`kubectl -n fulcio-system get --no-headers ksvc fulcio | cut -d ' ' -f 4`
echo "FULCIO_URL=$FULCIO_URL" >> $GITHUB_ENV
CTLOG_URL=`kubectl -n ctlog-system get --no-headers ksvc ctlog | cut -d ' ' -f 4`
echo "CTLOG_URL=$CTLOG_URL" >> $GITHUB_ENV

ISSUER_URL=`kubectl get --no-headers ksvc gettoken | cut -d ' ' -f 4`
echo "ISSUER_URL=$ISSUER_URL" >> $GITHUB_ENV
OIDC_TOKEN=`curl -s $ISSUER_URL`
echo "OIDC_TOKEN=$OIDC_TOKEN" >> $GITHUB_ENV

kubectl -n ctlog-system get secrets ctlog-public-key -o=jsonpath='{.data.public}' | base64 -d > ./ctlog-public.pem
echo "SIGSTORE_CT_LOG_PUBLIC_KEY_FILE=./ctlog-public.pem" >> $GITHUB_ENV

kubectl -n fulcio-system get secrets fulcio-secret -ojsonpath='{.data.cert}' | base64 -d > ./fulcio-root.pem
echo "SIGSTORE_ROOT_FILE=./fulcio-root.pem" >> $GITHUB_ENV

- name: Deploy ClusterImagePolicy
run: |
kubectl apply -f ./test/testdata/cosigned/e2e/cip.yaml

- name: build cosign
run: |
make cosign

- name: Sign demoimage with cosign
run: |
./cosign sign --rekor-url ${{ env.REKOR_URL }} --fulcio-url ${{ env.FULCIO_URL }} --force --allow-insecure-registry ${{ env.demoimage }} --identity-token ${{ env.OIDC_TOKEN }}

- name: Verify with cosign
run: |
SIGSTORE_TRUST_REKOR_API_PUBLIC_KEY=1 COSIGN_EXPERIMENTAL=1 ./cosign verify --rekor-url ${{ env.REKOR_URL }} --allow-insecure-registry ${{ env.demoimage }}

- name: Deploy jobs and verify signed works, unsigned fails
run: |
kubectl create namespace demo
kubectl label namespace demo cosigned.sigstore.dev/include=true

echo '::group:: test job success'
# We signed this above, this should work
if ! kubectl create -n demo job demo --image=${{ env.demoimage }} ; then
echo Failed to create Job in namespace without label!
exit 1
else
echo Succcessfully created Job with signed image
fi
echo '::endgroup:: test job success'

echo '::group:: test job rejection'
# We did not sign this, should fail
if kubectl create -n demo job demo2 --image=${{ env.demoimage2 }} ; then
echo Failed to block unsigned Job creation!
exit 1
else
echo Successfully blocked Job creation with unsigned image
fi
echo '::endgroup::'
Copy link
Member

Choose a reason for hiding this comment

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

Move this into scripts like these?

- name: Run Insecure Registry Tests
run: |
go install github.com/google/go-containerregistry/cmd/crane
./test/e2e_test_insecure_registry.sh
- name: Run Image Policy Tests
run: |
./test/e2e_test_policy_crd.sh
- name: Run Cosigned Tests
run: |
./test/e2e_test_cosigned.sh

Copy link
Member

Choose a reason for hiding this comment

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

Then you can avoid some of the GITHUB_ENV bits for passing env vars across steps, and folks can more easily run this locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah! I was planning on doing that as a follow on. Part of the original reasoning for the test/testadata/cosigned was to dump things into there so that we can then easier pick and choose what we run and automate that. I wasn't quite sure how to structure that :)

There's some stuff that I'd like to pass from the cluster (like the oidc_token, or a way to fetch it, or fulcio URL), etc. but I reckon when I call from actions to shell, env persists?

pkg/cosign/kubernetes/webhook/validator.go Show resolved Hide resolved
pkg/cosign/kubernetes/webhook/validator.go Show resolved Hide resolved
- glob: registry.local:5000/knative/demo*
authorities:
- keyless:
url: http://fulcio.fulcio-system.svc/api/v1/rootCert
Copy link
Member

Choose a reason for hiding this comment

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

Can we drop /api/v1/rootCert everywhere now?

Copy link
Member

Choose a reason for hiding this comment

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

(per my comment above)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it actually doesn't work with this anymore, I had fixed it already 👍

pkg/cosign/kubernetes/webhook/validator.go Show resolved Hide resolved
pkg/cosign/kubernetes/webhook/validator.go Show resolved Hide resolved
Copy link
Member

@mattmoor mattmoor left a comment

Choose a reason for hiding this comment

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

All of my comments are nits, so feel free to iterate.

@vaikas
Copy link
Contributor Author

vaikas commented Mar 25, 2022

Thanks for the reviews / feedbacks. Since you were both in the approval mood, I'll merge this now and then I'll address / refactor in a follow up PRs. I think it's good to get started testing the pieces and now we also have e2e tests to start building more complex policy validations.

@vaikas vaikas merged commit 340b6c6 into sigstore:main Mar 25, 2022
@vaikas vaikas deleted the scaffolding branch March 25, 2022 00:24
@github-actions github-actions bot added this to the v1.7.0 milestone Mar 25, 2022
vaikas added a commit to vaikas/cosign that referenced this pull request Mar 25, 2022
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
vaikas added a commit that referenced this pull request Mar 25, 2022
* First batch of followups to #1650

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Fix few typos.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* remove some unnecessary cruft since the actions handle them.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* kubectl apply vs. create
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* try harder
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* registry before installations.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Remove trying with 1.23 which might have different job retry behaviour.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
vaikas added a commit to vaikas/cosign that referenced this pull request Mar 28, 2022
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
dlorenc pushed a commit that referenced this pull request Mar 28, 2022
* Refactor based on discussions in #1650

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Simplify return signature.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* pr feedback.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* remove now unnecessary var.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
mlieberman85 pushed a commit to mlieberman85/cosign that referenced this pull request May 6, 2022
…gstore#1650)

* Just setting up the cluster tests.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Sign/verify a simple container.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Plumb Keyless with Fulcio / Rekor through.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Start wiring tests together. Not complete.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Ok, now the test should work, last run should've failed, it did. woot!

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Return policies instead of all the authorities in one fell swoop since
we need to make sure that all policies have at least one validating
authority.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Cleanup, add keyless unit tests. For now just failure cases, need to
understand how to test fully with both rekor/fulcio the good case.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Removed the API path portion of the test CIP as no longer necessary.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
mlieberman85 pushed a commit to mlieberman85/cosign that referenced this pull request May 6, 2022
* First batch of followups to sigstore#1650

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Fix few typos.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* remove some unnecessary cruft since the actions handle them.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* kubectl apply vs. create
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* try harder
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* registry before installations.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Remove trying with 1.23 which might have different job retry behaviour.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
mlieberman85 pushed a commit to mlieberman85/cosign that referenced this pull request May 6, 2022
* Refactor based on discussions in sigstore#1650

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Simplify return signature.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* pr feedback.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* remove now unnecessary var.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
@vaikas vaikas restored the scaffolding branch September 24, 2022 22:08
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.

None yet

4 participants