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: support cosign signatures / attestations and discover in zarf prepare find-images #2027

Merged
merged 32 commits into from
Oct 13, 2023

Conversation

mjnagel
Copy link
Contributor

@mjnagel mjnagel commented Sep 25, 2023

Description

Output from @rjferguson21 and my dash day's explorations of this.

This PR includes:

  • Logic pulled from cosign tree to detect and include signatures/attestations when using zarf prepare find-images
  • Skips SBOMs and caching for "non-images" by checking layer mediatypes at pull time

Several TODOs for follow-on work based on the issue/other needs:

  • Add in SBOM pulling when available, skip over syft creation of SBOM when applicable.
  • Mutating digests as needed - in order for tools to identify the signature the tag for it must be tagged <digest>.sig. When zarf does its AddImageAnnotation this could change the image digest, making it so that the signature is no longer at the correct tag. Images already having that annotation are unaffected (which is why this works OK for Ironbank).

Related Issue

Relates to #475

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@netlify
Copy link

netlify bot commented Sep 25, 2023

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit f61288f
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/65286f7857f27f0008d10342

Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

I know it is still draft but figured I would add some initial comments

examples/cosign-signatures/zarf.yaml Outdated Show resolved Hide resolved
src/cmd/common/viper.go Outdated Show resolved Hide resolved
src/pkg/packager/create.go Outdated Show resolved Hide resolved
src/internal/packager/images/pull.go Outdated Show resolved Hide resolved
src/internal/packager/sbom/catalog.go Outdated Show resolved Hide resolved
src/pkg/packager/create.go Outdated Show resolved Hide resolved
src/pkg/packager/create.go Outdated Show resolved Hide resolved
src/pkg/utils/cosign.go Show resolved Hide resolved
src/pkg/utils/cosign.go Show resolved Hide resolved
src/test/packages/10-cosign-lookup/zarf.yaml Outdated Show resolved Hide resolved
src/pkg/packager/create.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

It would be good to also update the fuzzy image regex to this while you touch the find images code:

["|=]([a-z0-9\-.\/:]+:[\w.\-]*[a-z\.\-][\w.\-]*)"

Welcome feedback on whether it makes sense and the full context is here: https://kubernetes.slack.com/archives/C03B6BJAUJ3/p1696639896292219?thread_ts=1696585484.114289&cid=C03B6BJAUJ3 (edited)

@Racer159 Racer159 changed the title Pull cosign signatures / attestations at create time feat: support cosign signatures / attestations and discover in zarf prepare find-images Oct 11, 2023
@Racer159 Racer159 changed the title feat: support cosign signatures / attestations and discover in zarf prepare find-images feat: support cosign signatures / attestations and discover in zarf prepare find-images Oct 11, 2023
src/test/packages/10-cosign-artifacts/zarf.yaml Outdated Show resolved Hide resolved
src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
src/internal/packager/images/pull.go Outdated Show resolved Hide resolved
@mjnagel mjnagel marked this pull request as ready for review October 12, 2023 21:39
src/pkg/utils/cosign.go Outdated Show resolved Hide resolved
Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com>
Copy link
Contributor

@Noxsios Noxsios left a comment

Choose a reason for hiding this comment

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

lgtm now the error was handled

@Racer159 Racer159 merged commit c46a4b9 into main Oct 13, 2023
22 checks passed
@Racer159 Racer159 deleted the signatures branch October 13, 2023 03:27
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.

4 participants