Verify sigs and attestations in parallel

[priyawadhwa]

This should significantly speed up verification time for images that have a lot of signature or attestations.

[cpanato]

nice

[codecov]

Codecov Report

Merging #3066 (41db33e) into main (14b7674) will decrease coverage by 0.12%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #3066      +/-   ##
==========================================
- Coverage   30.84%   30.72%   -0.12%     
==========================================
  Files         155      155              
  Lines        9726     9763      +37     
==========================================
  Hits         3000     3000              
- Misses       6275     6312      +37     
  Partials      451      451              

Impacted Files	Coverage Δ
pkg/cosign/verify.go	35.62% <0.00%> (-1.46%)	⬇️

[vaikas]

same here as below.

[vaikas]

I seem to recall (maybe I'm forgetting some details and have something wrong), but for loops where you're then launching go funcs, you needed to make a local copy, so something like this:
for i, att := range sl {
i := i
att := att
wg.Add(1)

[priyawadhwa]

ah yah i've seen that before! i think that passing in the values to the go function, like go func(att oci.Signature, index int), copies the values as well and accomplishes the same thing

I looked into it just to make sure i hadn't mixed up different things and this post says you can do it either way :)

[vaikas]

lol, yeah, thanks for looking. I couldn't recall all the details, but remember there be dragons!!

[imjasonh]

It might be worth capping the parallelism here, possibly even making it configurable. If I have 1000 signatures on an image, this will unleash 1000 concurrent requests to the registry, which will not be fun for anybody. 😓

Instead, we can spin up N concurrent workers, and feed them verification tasks:

errCh := make(chan error)
ch := make(chan foo)
for i := 0; i < 10; i++ {
  go func() {
    select {
      case <-ctx.Done():
        return
      case f := <- ch:
        // verify(f)
    }
  }()
}
for _, f := range foos {
  ch <- f
}

You can wrap this up in errgroup too, which now has SetLimit for exactly this.

[cpanato]

should we also have a flag to set the number of workers in case someone needs to configure to a higher or lower number?

@priyawadhwa are you working on this? otherwise I am happy to do

[priyawadhwa]

@cpanato go for it, I haven't started yet! i think a flag would be great. We can wait to get the flag merged before we release 2.1.0

[cpanato]

@cpanato go for it, I haven't started yet! i think a flag would be great. We can wait to get the flag merged before we release 2.1.0

ok, will do that tomorrow my morning