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

Move "verify-manifest" into "verify". #527

Closed
wants to merge 1 commit into from
Closed

Conversation

dlorenc
Copy link
Member

@dlorenc dlorenc commented Aug 9, 2021

We'll keep both commands for now, but deprecate verify-manifest.

Signed-off-by: Dan Lorenc [email protected]

We'll keep both commands for now, but deprecate verify-manifest.

Signed-off-by: Dan Lorenc <[email protected]>
@dlorenc
Copy link
Member Author

dlorenc commented Aug 9, 2021

Ref #437

@cpanato cpanato added this to the v1.1.0 milestone Aug 9, 2021
@dekkagaijin
Copy link
Member

Dan and I chatted about things offline, but I think we probably don't want to be adding any additional file scanning capabilities to cosign. It's probably regrettable that we had it in the first place, instead of creating an independent utility which could be composed with cosign to achieve the desired functionality.

Going forward, we want to prioritize ease of use for signature and attestation validation over all

Copy link
Contributor

@hectorj2f hectorj2f left a comment

Choose a reason for hiding this comment

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

lgtm, I just saw the previous comment 👍🏻 .

@hectorj2f
Copy link
Contributor

hectorj2f commented Aug 16, 2021

@dekkagaijin Please, could you disclosure more details about this new approach creation of an independent utility which could be composed with cosign to achieve the desired functionality ?

How would you verify Dockerfiles in the future using cosign ?

allImgs = append(allImgs, args...)

for _, f := range c.Files.slice {
err := isExtensionAllowed(f)
Copy link
Member

Choose a reason for hiding this comment

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

Will we distinguish the given file whether Dockerfile in this function? Currently, isExtensionAllowed only returns true in case given file is .yml or .yaml. In the flag description, since we pointed out that -f flag accepts Dockerfile, we actually do not check here.

flagset.Var(&cmd.Files, "f", "files to validate (kubernetes manifests or Dockerfiles)")

Will we move the verify-dockerfile into verify command in the long term?

@dekkagaijin
Copy link
Member

@hectorj2f I'm thinking that one would chain a utility to extract images from a dockerfile and feed them to cosign, e.g. dockerfile extract images $DOCKERFILE | xargs cosign verify

@cpanato cpanato modified the milestones: v1.1.0, v1.2.0 Aug 26, 2021
@hectorj2f
Copy link
Contributor

@dekkagaijin Thanks for the explanation :).

@dlorenc
Copy link
Member Author

dlorenc commented Sep 18, 2021

Closing, we went a different way here!

@dlorenc dlorenc closed this Sep 18, 2021
@cpanato cpanato removed this from the v1.3.0 milestone Sep 18, 2021
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.

5 participants