diff --git a/internal/pkg/report/reporter.go b/internal/pkg/report/reporter.go index ec85de3..57c6a4f 100644 --- a/internal/pkg/report/reporter.go +++ b/internal/pkg/report/reporter.go @@ -5,6 +5,7 @@ package report import ( "context" + "errors" "fmt" "regexp" "strings" @@ -21,6 +22,11 @@ import ( "runner.forgejo.org/internal/pkg/client" ) +var ( + outputKeyMaxLength = 255 + outputValueMaxLength = 1024 * 1024 +) + type Reporter struct { ctx context.Context cancel context.CancelFunc @@ -199,24 +205,31 @@ func (r *Reporter) logf(format string, a ...interface{}) { } } -func (r *Reporter) SetOutputs(outputs map[string]string) { +func (r *Reporter) SetOutputs(outputs map[string]string) error { r.stateMu.Lock() defer r.stateMu.Unlock() + var errs []error + recordError := func(format string, a ...interface{}) { + r.logf(format, a...) + errs = append(errs, fmt.Errorf(format, a...)) + } for k, v := range outputs { - if len(k) > 255 { - r.logf("ignore output because the key is too long: %q", k) + if len(k) > outputKeyMaxLength { + recordError("ignore output because the key is longer than %d: %q", outputKeyMaxLength, k) continue } - if l := len(v); l > 1024*1024 { - log.Println("ignore output because the value is too long:", k, l) - r.logf("ignore output because the value %q is too long: %d", k, l) + if l := len(v); l > outputValueMaxLength { + recordError("ignore output because the length of the value for %q is %d (the maximum is %d)", k, l, outputValueMaxLength) + continue } if _, ok := r.outputs.Load(k); ok { + recordError("ignore output because a value already exists for the key %q", k) continue } r.outputs.Store(k, v) } + return errors.Join(errs...) } func (r *Reporter) Close(lastWords string) error { diff --git a/internal/pkg/report/reporter_test.go b/internal/pkg/report/reporter_test.go index d9cecaf..caba069 100644 --- a/internal/pkg/report/reporter_test.go +++ b/internal/pkg/report/reporter_test.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "strings" + "sync" "testing" "time" @@ -20,6 +21,7 @@ import ( "google.golang.org/protobuf/types/known/structpb" "runner.forgejo.org/internal/pkg/client/mocks" + "runner.forgejo.org/internal/pkg/testutils" ) func rowsToString(rows []*runnerv1.LogRow) string { @@ -58,6 +60,60 @@ func mockReporter(t *testing.T) (*Reporter, *mocks.Client, func()) { return reporter, client, close } +func TestReporterSetOutputs(t *testing.T) { + assertEqual := func(t *testing.T, expected map[string]string, actual *sync.Map) { + t.Helper() + actualMap := map[string]string{} + actual.Range(func(k, v interface{}) bool { + val, ok := v.(string) + require.True(t, ok) + actualMap[k.(string)] = val + return true + }) + assert.Equal(t, expected, actualMap) + } + + t.Run("All", func(t *testing.T) { + reporter, _, _ := mockReporter(t) + + expected := map[string]string{"a": "b", "c": "d"} + assert.NoError(t, reporter.SetOutputs(expected)) + assertEqual(t, expected, &reporter.outputs) + }) + + t.Run("IgnoreTooBig", func(t *testing.T) { + reporter, _, _ := mockReporter(t) + + testutils.MockVariable(&outputKeyMaxLength, 5) + testutils.MockVariable(&outputValueMaxLength, 5) + + in := map[string]string{ + "0123456": "b", // key too big + "c": "ABCDEFG", // value too big + "d": "e", + } + err := reporter.SetOutputs(in) + assert.ErrorContains(t, err, "ignore output because the length of the value for \"c\" is 7 (the maximum is 5)") + assert.ErrorContains(t, err, "ignore output because the key is longer than 5: \"0123456\"") + expected := map[string]string{"d": "e"} + assertEqual(t, expected, &reporter.outputs) + }) + + t.Run("IgnoreDuplicates", func(t *testing.T) { + reporter, _, _ := mockReporter(t) + + first := map[string]string{"a": "b", "c": "d"} + assert.NoError(t, reporter.SetOutputs(first)) + assertEqual(t, first, &reporter.outputs) + + second := map[string]string{"c": "d", "e": "f"} + assert.ErrorContains(t, reporter.SetOutputs(second), "ignore output because a value already exists for the key \"c\"") + + expected := map[string]string{"a": "b", "c": "d", "e": "f"} + assertEqual(t, expected, &reporter.outputs) + }) +} + func TestReporter_parseLogRow(t *testing.T) { tests := []struct { name string