Add nested archive cataloging (#5260)

Signed-off-by: Kudryavcev Nikolay <kydry.nikolau@gmail.com>
Signed-off-by: Thomas Bechtold <thomasbechtold@jpberlin.de>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Co-authored-by: Kudryavcev Nikolay <kydry.nikolau@gmail.com>
Co-authored-by: Thomas Bechtold <thomasbechtold@jpberlin.de>
Co-authored-by: Alex Goodman <wagoodman@users.noreply.github.com>

internal/bus/helpers.go

@@ -1,6 +1,9 @@
 package bus
 
 import (
+	"context"
+	"sync"
+
 	"github.com/wagoodman/go-partybus"
 	"github.com/wagoodman/go-progress"
 
@@ -36,7 +39,82 @@ func Notify(message string) {
 	})
 }
 
-func StartCatalogerTask(info monitor.GenericTask, size int64, initialStage string) *monitor.TaskProgress {
+type catalogerTaskRegistryKey struct{}
+
+// catalogerTaskRegistry remembers the progress row each cataloger publishes. A cataloger runs once
+// over the scan root and again inside every archive the archive cataloger walks; a re-run reports
+// into the row it already owns rather than publishing another, and republishing an ID would replace
+// the live row with one the UI never sees completed.
+type catalogerTaskRegistry struct {
+	mu   sync.Mutex
+	rows map[string]*monitor.TaskProgress
+}
+
+// WithCatalogerTaskRegistry returns a context under which each cataloger publishes one progress row
+// however many times it runs. A context that already carries a registry is returned unchanged.
+func WithCatalogerTaskRegistry(ctx context.Context) context.Context {
+	if catalogerTaskRegistryFromContext(ctx) != nil {
+		return ctx
+	}
+	return context.WithValue(ctx, catalogerTaskRegistryKey{}, &catalogerTaskRegistry{
+		rows: make(map[string]*monitor.TaskProgress),
+	})
+}
+
+func catalogerTaskRegistryFromContext(ctx context.Context) *catalogerTaskRegistry {
+	reg, _ := ctx.Value(catalogerTaskRegistryKey{}).(*catalogerTaskRegistry)
+	return reg
+}
+
+// catalogerTaskKey identifies the row a task owns: its ID when it has one, otherwise its parent and
+// title (file catalogers publish no ID).
+func catalogerTaskKey(info monitor.GenericTask) string {
+	if info.ID != "" {
+		return "id:" + info.ID
+	}
+	return "title:" + info.ParentID + "/" + info.Title.Default
+}
+
+func StartCatalogerTask(ctx context.Context, info monitor.GenericTask, size int64, initialStage string) *monitor.TaskProgress {
+	reg := catalogerTaskRegistryFromContext(ctx)
+	if reg == nil {
+		return publishCatalogerTask(info, size, initialStage)
+	}
+
+	reg.mu.Lock()
+	defer reg.mu.Unlock()
+
+	key := catalogerTaskKey(info)
+	if row, ok := reg.rows[key]; ok {
+		resumeCatalogerTask(row, size, initialStage)
+		return row
+	}
+
+	t := publishCatalogerTask(info, size, initialStage)
+	reg.rows[key] = t
+
+	return t
+}
+
+// resumeCatalogerTask reopens a row an earlier run completed and grows its total by what this run
+// adds, so the row does not read as finished mid-walk. A row carrying a real failure keeps it.
+func resumeCatalogerTask(row *monitor.TaskProgress, size int64, initialStage string) {
+	if size < 0 || row.Size() < 0 {
+		row.SetTotal(-1)
+	} else {
+		row.SetTotal(row.Size() + size)
+	}
+
+	if progress.IsErrCompleted(row.Error()) {
+		row.SetError(nil)
+	}
+
+	if initialStage != "" {
+		row.AtomicStage.Set(initialStage)
+	}
+}
+
+func publishCatalogerTask(info monitor.GenericTask, size int64, initialStage string) *monitor.TaskProgress {
 	t := &monitor.TaskProgress{
 		AtomicStage: progress.NewAtomicStage(initialStage),
 		Manual:      progress.NewManual(size),

internal/bus/helpers_test.go

@@ -0,0 +1,152 @@
+package bus
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/wagoodman/go-progress"
+
+	"github.com/anchore/syft/syft/event/monitor"
+)
+
+func digestsTask() monitor.GenericTask {
+	return monitor.GenericTask{
+		Title:    monitor.Title{Default: "File digests"},
+		ParentID: monitor.TopLevelCatalogingTaskID,
+	}
+}
+
+func TestStartCatalogerTask_withoutARegistryEveryRunPublishes(t *testing.T) {
+	// the right answer for a caller driving a single pass: nothing re-runs, so nothing should be reused
+	ctx := context.Background()
+
+	first := StartCatalogerTask(ctx, digestsTask(), 2, "")
+	second := StartCatalogerTask(ctx, digestsTask(), 3, "")
+
+	assert.NotSame(t, first, second)
+	assert.Equal(t, int64(2), first.Size())
+	assert.Equal(t, int64(3), second.Size())
+}
+
+func TestStartCatalogerTask_reRunReportsIntoTheRowItAlreadyOwns(t *testing.T) {
+	ctx := WithCatalogerTaskRegistry(context.Background())
+
+	first := StartCatalogerTask(ctx, digestsTask(), 2, "")
+	first.Increment()
+	first.Increment()
+	first.SetCompleted()
+
+	// the archive walk reaches the same cataloger again, over an archive's contents this time
+	second := StartCatalogerTask(ctx, digestsTask(), 3, "")
+	require.Same(t, first, second, "a re-run must find the row it already published")
+
+	assert.Equal(t, int64(5), second.Size(), "the total must cover both runs, not just the first")
+	assert.False(t, progress.IsCompleted(second), "a row with work still to do must not read as complete")
+
+	for i := 0; i < 3; i++ {
+		second.Increment()
+	}
+	second.SetCompleted()
+
+	assert.Equal(t, int64(5), second.Current())
+	assert.Equal(t, int64(5), second.Size(), "the count must not run past the total")
+	assert.True(t, progress.IsCompleted(second))
+}
+
+func TestStartCatalogerTask_aTotalResolvedOnCompletionIsAddable(t *testing.T) {
+	// a run that could not say how much it would do says so when it signs off (Manual.SetCompleted
+	// backfills the total from the count), so by the time a re-run arrives there is a real number to
+	// add to. This is the file metadata cataloger's shape: it publishes -1 every run.
+	ctx := WithCatalogerTaskRegistry(context.Background())
+
+	row := StartCatalogerTask(ctx, digestsTask(), -1, "")
+	row.Increment()
+	row.SetCompleted()
+	require.Equal(t, int64(1), row.Size(), "signing off must resolve the total to what the run did")
+
+	require.Same(t, row, StartCatalogerTask(ctx, digestsTask(), -1, ""))
+	assert.Equal(t, int64(-1), row.Size(), "a re-run that cannot say its own size makes the row indeterminate again")
+
+	row.Increment()
+	row.SetCompleted()
+	assert.Equal(t, int64(2), row.Current())
+	assert.Equal(t, int64(2), row.Size(), "and signing off resolves it again, over both runs")
+}
+
+func TestStartCatalogerTask_indeterminateTotalsStayIndeterminate(t *testing.T) {
+	// a run whose size is unknown cannot be added to one that is known
+	tests := []struct {
+		name        string
+		first, next int64
+		complete    func(row *monitor.TaskProgress)
+	}{
+		{
+			name:  "the re-run cannot say its own size",
+			first: 2, next: -1,
+			complete: func(row *monitor.TaskProgress) { row.Increment(); row.SetCompleted() },
+		},
+		{
+			name:  "the first run did nothing, so completing resolved nothing",
+			first: -1, next: 3,
+			complete: func(row *monitor.TaskProgress) { row.SetCompleted() },
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := WithCatalogerTaskRegistry(context.Background())
+
+			row := StartCatalogerTask(ctx, digestsTask(), tt.first, "")
+			tt.complete(row)
+
+			require.Same(t, row, StartCatalogerTask(ctx, digestsTask(), tt.next, ""))
+			assert.Equal(t, int64(-1), row.Size())
+			assert.False(t, progress.IsCompleted(row), "an indeterminate row is not complete until it says so")
+		})
+	}
+}
+
+func TestStartCatalogerTask_aFailedRowKeepsItsFailure(t *testing.T) {
+	// a later run finding nothing wrong does not mean the earlier one succeeded
+	ctx := WithCatalogerTaskRegistry(context.Background())
+	failure := errors.New("unable to read the thing")
+
+	row := StartCatalogerTask(ctx, digestsTask(), 2, "")
+	row.SetError(failure)
+
+	require.Same(t, row, StartCatalogerTask(ctx, digestsTask(), 3, ""))
+	assert.ErrorIs(t, row.Error(), failure)
+}
+
+func TestStartCatalogerTask_rowsAreKeyedByTaskIdentity(t *testing.T) {
+	ctx := WithCatalogerTaskRegistry(context.Background())
+
+	byID := func(id string) monitor.GenericTask {
+		return monitor.GenericTask{ID: id, Title: monitor.Title{Default: "shared title"}}
+	}
+
+	assert.Same(t, StartCatalogerTask(ctx, byID("a"), -1, ""), StartCatalogerTask(ctx, byID("a"), -1, ""))
+	assert.NotSame(t, StartCatalogerTask(ctx, byID("a"), -1, ""), StartCatalogerTask(ctx, byID("b"), -1, ""),
+		"an ID is the identity where there is one, even when two tasks share a title")
+
+	// a task publishing without an ID, as every file cataloger does, is keyed by where it sits and what
+	// it is called
+	underCataloging := monitor.GenericTask{Title: monitor.Title{Default: "Files"}, ParentID: "cataloging"}
+	underSomethingElse := monitor.GenericTask{Title: monitor.Title{Default: "Files"}, ParentID: "other"}
+
+	assert.Same(t, StartCatalogerTask(ctx, underCataloging, -1, ""), StartCatalogerTask(ctx, underCataloging, -1, ""))
+	assert.NotSame(t, StartCatalogerTask(ctx, underCataloging, -1, ""), StartCatalogerTask(ctx, underSomethingElse, -1, ""))
+}
+
+func TestWithCatalogerTaskRegistry_isIdempotent(t *testing.T) {
+	// anything that re-runs catalogers may ask for a registry; a second one would hold none of the rows
+	// already published and every re-run would publish again
+	ctx := WithCatalogerTaskRegistry(context.Background())
+	row := StartCatalogerTask(ctx, digestsTask(), 2, "")
+
+	nested := WithCatalogerTaskRegistry(ctx)
+	assert.Same(t, row, StartCatalogerTask(nested, digestsTask(), 2, ""))
+}