LocationResolver can leak goroutines

[wagoodman]

Today the LocationResolver returns a channel of locations:

type LocationResolver interface {
	AllLocations() <-chan Location
}

However, while iterating though the channel if you return early you will leak a go routine. This should probably be updated to allow for cancellation via context and follow an iterator pattern (soft suggestion):

type LocationResolver interface {
	AllLocations() LocationIterator
}

type LocationIterator interface {
	Next(context.Context) (Location, bool)
}

Where the caller would now be able to coordinate cancellation for correct usage:

ctx, cancel := context.WithCancel(ctx)
defer cancel()

iter := resolver.AllLocations()
for {
	location, ok := iter.Next(ctx)
	if !ok {
		break
	}
	// process the location...
}

Alternatively AllLocations can be a slice, which is simpler, but may cause memory pressures for images with many files:

type LocationResolver interface {
	AllLocations() [] Location
}

[willmurphyscode]

We should also pass context down via the cataloger interface: https://github.com/anchore/syft/blob/main/syft/pkg/cataloger.go#L15.

[willmurphyscode]

I have a question about the suggested implementation: Why are we making a separate iterator? Why no let AllLocations accept a context, like this (example from the directory iterator):

func (r *Directory) AllLocations(ctx context.Context) <-chan file.Location {
	results := make(chan file.Location)
	go func() {
		defer close(results)
		for _, ref := range r.tree.AllFiles(stereoscopeFile.AllTypes()...) {
			select {
			case <-ctx.Done():
				return
			case results <- file.NewLocationFromDirectory(r.responsePath(string(ref.RealPath)), ref):
				continue
			}
		}
	}()
	return results
}

Testing this with "go.uber.org/goleak" seems to work fine:

func TestAllLocationsDoesNotLeakGoRoutine(t *testing.T) {
	defer goleak.VerifyNone(t)
	resolver, err := NewFromDirectory("./test-fixtures/symlinks-from-image-symlinks-fixture", "")
	require.NoError(t, err)
	ctx, cancel := context.WithCancel(context.Background())
	for _ = range resolver.AllLocations(ctx) {
		break
	}
	cancel()
}

This seems simpler than having a struct carry around the state of the iteration and implement a new interface. What do you think @wagoodman?

[wagoodman]

I'm down with not needing to bring another interface along for the ride -- overall I think this looks good (and is a simpler change) 👍