sigstore / protobuf-specs

Protocol Buffer specifications
Apache License 2.0
23 stars 29 forks source link

Common test suite strategy #15

Open znewman01 opened 1 year ago

znewman01 commented 1 year ago

https://github.com/sigstore/cosign/issues/2434 proposes adding "conformance testing" to Cosign (previously implemented in sigstore-python).

I think this is a great idea and we've been thinking about it in the abstract in this repository. I'd like to use this issue to discuss strategy details. What kinds of tests should be be running across all Sigstore clients? How should we architect those?

CC @tetsuo-cpp @di @woodruffw

See also some context from the linked Cosign issue.


Hey @znewman01, thanks for the ideas and feedback.

Appreciate the additional context; I understand much better what you're trying to achieve here. I think overall our goals are aligned and I'm optimistic that we can come up with a plan we're both happy with (and again, I'm still open to merging the CLI tests as-written and even keeping them long-term, I just want to make sure that's part of a coherent conformance testing strategy).

Or were you thinking more that the conformance test suite should just provide a set of test inputs and expected outputs and leaving the actual test runner logic to each client's unit tests?

Precisely. IMO it's way more work to add new shell script-y test suites than to just have each client import standard test data. I also think the performance benefits of avoiding creating are im

It's also not clear to me that every language client should have a CLI.

I think that could be complicated because we'll want to test interactions with Fulcio, Rekor and eventually mock out those services to test scenarios like getting a broken inclusion proof, SCT, etc.

Hm...my imagination is that every CLI should be a pretty thin wrapper around a library interface, so anything you could do in a shell script (spin up a fake Fulcio server and point your tool at it) could happen inside a language-specific test harness just as easily (and integrate better with the repo's existing tests).

I guess this is mostly my personal biases—I worked full-time on a CLI for a while, and always found CLI-level testing to be a really blunt instrument, brittle, and slow. Further, architecting a CLI for testability without going through the CLI interface itself means that your library is much more usable as a library and that the tests are clearer (assertions don't require regexes and parsing STDOUT). Not sure—am I being at-all convincing here that CLI tests are something to avoid when possible?

I see value in using Protobuf messages but, the way I see it, they can complement the current approach. So at some point when these Protobuf definitions become standardised, we could simplify the CLI protocol and instead ask that clients expose some kind of --protobuf flag that takes an Input rather than the flags that we're asking for now.

Certainly the protobufs work really well for testing verification. I think it's doable to start expressing full sign+verification flows declaratively too, with faked responses—and in fact, I think a proto-centric approach makes the most sense for faking.

Maybe we could split the difference:

This may be overengineering for now, and it seems reasonable to start with everything in the "CLI harness" but perhaps with a longer-term vision of moving first the verification cases, then everything else, into the "declarative test specification" layer.

Regardless, I think the Cosign repo isn't quite the right place for this issue—would you want to open one up in https://github.com/sigstore/protobuf-specs which seems to be the home of most cross-client concerns?

This issue was specifically to add the existing GitHub Action to cosign's CI so that's why I've opened it here. At the moment, we're wanting to pilot it with clients other than sigstore-python (the client I usually work on).

In the medium-term, I'd expect "common test suite" to be tracked mostly over in the protobuf-specs repo, but it makes sense to have per-client repo issues as well. Maybe we can keep this issue open to track the integration for Golang, and I'll file another issue for the strategy/philosophy discussion.

(Part of the reason I want to move it out of this repo too is that at some point these tests would move over to https://github.com/sigstore/sigstore-go because that's where the core Sigstore logic for Go will live, and Cosign will mostly focus on OCI/container signing.)

Originally posted by @znewman01 in https://github.com/sigstore/cosign/issues/2434#issuecomment-1312060703

znewman01 commented 1 year ago

The above is very verbose; the current plan is: