diff --git a/Makefile b/Makefile index 07716b436795..b57e51d0efcf 100644 --- a/Makefile +++ b/Makefile @@ -236,6 +236,7 @@ generate_go: format_cpp $(GO) generate ./executor ./pkg/ifuzz ./pkg/build ./pkg/rpcserver $(GO) generate ./vm/proxyapp $(GO) generate ./pkg/coveragedb + $(GO) generate ./pkg/covermerger generate_rpc: flatc -o pkg/flatrpc --warnings-as-errors --gen-object-api --filename-suffix "" --go --gen-onefile --go-namespace flatrpc pkg/flatrpc/flatrpc.fbs diff --git a/dashboard/app/api.go b/dashboard/app/api.go index b06c5ac07833..e1269b3d087d 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -26,7 +26,6 @@ import ( "github.com/google/syzkaller/pkg/asset" "github.com/google/syzkaller/pkg/auth" "github.com/google/syzkaller/pkg/coveragedb" - "github.com/google/syzkaller/pkg/coveragedb/spannerclient" "github.com/google/syzkaller/pkg/debugtracer" "github.com/google/syzkaller/pkg/email" "github.com/google/syzkaller/pkg/gcs" @@ -105,6 +104,7 @@ var maxCrashes = func() int { func handleJSON(fn JSONHandler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { c := appengine.NewContext(r) + c = SetCoverageDBClient(c, coverageDBClient) reply, err := fn(c, r) if err != nil { status := logErrorPrepareStatus(c, err) @@ -1948,12 +1948,7 @@ func apiSaveCoverage(c context.Context, payload io.Reader) (interface{}, error) sss = service.List() log.Infof(c, "found %d subsystems for %s namespace", len(sss), descr.Namespace) } - client, err := spannerclient.NewClient(c, appengine.AppID(context.Background())) - if err != nil { - return 0, fmt.Errorf("coveragedb.NewClient() failed: %s", err.Error()) - } - defer client.Close() - rowsCreated, err := coveragedb.SaveMergeResult(c, client, descr, jsonDec, sss) + rowsCreated, err := coveragedb.SaveMergeResult(c, GetCoverageDBClient(c), descr, jsonDec, sss) if err != nil { log.Errorf(c, "error storing coverage for ns %s, date %s: %v", descr.Namespace, descr.DateTo.String(), err) diff --git a/dashboard/app/batch_coverage.go b/dashboard/app/batch_coverage.go index 6d53496a7087..aba85cf596da 100644 --- a/dashboard/app/batch_coverage.go +++ b/dashboard/app/batch_coverage.go @@ -43,7 +43,7 @@ func handleBatchCoverage(w http.ResponseWriter, r *http.Request) { if err != nil { log.Errorf(ctx, "failed nsDataAvailable(%s): %s", ns, err) } - periodsMerged, rowsMerged, err := coveragedb.NsDataMerged(ctx, "syzkaller", ns) + periodsMerged, rowsMerged, err := coveragedb.NsDataMerged(ctx, coverageDBClient, ns) if err != nil { log.Errorf(ctx, "failed coveragedb.NsDataMerged(%s): %s", ns, err) } @@ -154,7 +154,7 @@ func nsDataAvailable(ctx context.Context, ns string) ([]coveragedb.TimePeriod, [ func handleBatchCoverageClean(w http.ResponseWriter, r *http.Request) { ctx := context.Background() - totalDeleted, err := coveragedb.DeleteGarbage(ctx) + totalDeleted, err := coveragedb.DeleteGarbage(ctx, coverageDBClient) if err != nil { errMsg := fmt.Sprintf("failed to coveragedb.DeleteGarbage: %s", err.Error()) log.Errorf(ctx, "%s", errMsg) diff --git a/dashboard/app/config.go b/dashboard/app/config.go index 6d173cb19550..c71d09d8e88e 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -425,6 +425,7 @@ func installConfig(cfg *GlobalConfig) { initAPIHandlers() initKcidb() initBatchProcessors() + initCoverageDB() } var contextConfigKey = "Updated config (to be used during tests). Use only in tests!" diff --git a/dashboard/app/coverage.go b/dashboard/app/coverage.go index 7bc0d2a06660..abc75caab015 100644 --- a/dashboard/app/coverage.go +++ b/dashboard/app/coverage.go @@ -8,6 +8,7 @@ import ( "fmt" "html/template" "net/http" + "os" "slices" "strconv" @@ -19,6 +20,33 @@ import ( "github.com/google/syzkaller/pkg/validator" ) +var coverageDBClient spannerclient.SpannerClient + +func initCoverageDB() { + projectID := os.Getenv("GOOGLE_CLOUD_PROJECT") + if projectID == "" { + // It is a test environment. + // Use SetCoverageDBClient to specify the coveragedb mock or emulator in every test. + return + } + var err error + coverageDBClient, err = spannerclient.NewClient(context.Background(), projectID) + if err != nil { + panic("spanner.NewClient: " + err.Error()) + } +} + +var keyCoverageDBClient = "coveragedb client key" + +func SetCoverageDBClient(ctx context.Context, client spannerclient.SpannerClient) context.Context { + return context.WithValue(ctx, &keyCoverageDBClient, client) +} + +func GetCoverageDBClient(ctx context.Context) spannerclient.SpannerClient { + client, _ := ctx.Value(&keyCoverageDBClient).(spannerclient.SpannerClient) + return client +} + type funcStyleBodyJS func( ctx context.Context, client spannerclient.SpannerClient, scope *cover.SelectScope, onlyUnique bool, sss, managers []string, @@ -37,6 +65,10 @@ func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f if err != nil { return err } + nsConfig := getNsConfig(c, hdr.Namespace) + if nsConfig.Coverage == nil { + return ErrClientNotFound + } ss := r.FormValue("subsystem") manager := r.FormValue("manager") @@ -76,15 +108,9 @@ func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f onlyUnique := r.FormValue("unique-only") == "1" - spannerClient, err := spannerclient.NewClient(c, "syzkaller") - if err != nil { - return fmt.Errorf("spanner.NewClient: %s", err.Error()) - } - defer spannerClient.Close() - var style template.CSS var body, js template.HTML - if style, body, js, err = f(c, spannerClient, + if style, body, js, err = f(c, GetCoverageDBClient(c), &cover.SelectScope{ Ns: hdr.Namespace, Subsystem: ss, @@ -147,24 +173,37 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques } onlyUnique := r.FormValue("unique-only") == "1" mainNsRepo, _ := nsConfig.mainRepoBranch() - hitLines, hitCounts, err := coveragedb.ReadLinesHitCount(c, hdr.Namespace, targetCommit, manager, kernelFilePath, tp) + client := GetCoverageDBClient(c) + if client == nil { + return fmt.Errorf("spannerdb client is nil") + } + hitLines, hitCounts, err := coveragedb.ReadLinesHitCount( + c, client, hdr.Namespace, targetCommit, kernelFilePath, manager, tp) covMap := cover.MakeCovMap(hitLines, hitCounts) if err != nil { return fmt.Errorf("coveragedb.ReadLinesHitCount(%s): %w", manager, err) } if onlyUnique { - allHitLines, allHitCounts, err := coveragedb.ReadLinesHitCount(c, hdr.Namespace, targetCommit, manager, kernelFilePath, tp) + // This request is expected to be made second by tests. + // Moving it to goroutine don't forget to change multiManagerCovDBFixture. + allHitLines, allHitCounts, err := coveragedb.ReadLinesHitCount( + c, client, hdr.Namespace, targetCommit, kernelFilePath, "*", tp) if err != nil { return fmt.Errorf("coveragedb.ReadLinesHitCount(*): %w", err) } covMap = cover.UniqCoverage(cover.MakeCovMap(allHitLines, allHitCounts), covMap) } + webGit := getWebGit(c) // Get mock if available. + if webGit == nil { + webGit = covermerger.MakeWebGit(makeProxyURIProvider(nsConfig.Coverage.WebGitURI)) + } + content, err := cover.RendFileCoverage( mainNsRepo, targetCommit, kernelFilePath, - makeProxyURIProvider(nsConfig.Coverage.WebGitURI), + webGit, &covermerger.MergeResult{HitCounts: covMap}, cover.DefaultHTMLRenderConfig()) if err != nil { @@ -175,11 +214,26 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques return nil } +var keyWebGit = "file content provider" + +func setWebGit(ctx context.Context, provider covermerger.FileVersProvider) context.Context { + return context.WithValue(ctx, &keyWebGit, provider) +} + +func getWebGit(ctx context.Context) covermerger.FileVersProvider { + res, _ := ctx.Value(&keyWebGit).(covermerger.FileVersProvider) + return res +} + func handleCoverageGraph(c context.Context, w http.ResponseWriter, r *http.Request) error { hdr, err := commonHeader(c, r, w, "") if err != nil { return err } + nsConfig := getNsConfig(c, hdr.Namespace) + if nsConfig.Coverage == nil { + return ErrClientNotFound + } periodType := r.FormValue("period") if periodType == "" { periodType = coveragedb.QuarterPeriod @@ -187,7 +241,7 @@ func handleCoverageGraph(c context.Context, w http.ResponseWriter, r *http.Reque if periodType != coveragedb.QuarterPeriod && periodType != coveragedb.MonthPeriod { return fmt.Errorf("only quarter and month are allowed, but received %s instead", periodType) } - hist, err := MergedCoverage(c, hdr.Namespace, periodType) + hist, err := MergedCoverage(c, GetCoverageDBClient(c), hdr.Namespace, periodType) if err != nil { return err } diff --git a/dashboard/app/coverage_test.go b/dashboard/app/coverage_test.go new file mode 100644 index 000000000000..7965c378296d --- /dev/null +++ b/dashboard/app/coverage_test.go @@ -0,0 +1,171 @@ +// Copyright 2025 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +package main + +import ( + "strings" + "testing" + + "github.com/google/syzkaller/pkg/coveragedb" + "github.com/google/syzkaller/pkg/coveragedb/mocks" + "github.com/google/syzkaller/pkg/coveragedb/spannerclient" + "github.com/google/syzkaller/pkg/covermerger" + mergermocks "github.com/google/syzkaller/pkg/covermerger/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "google.golang.org/api/iterator" +) + +func TestFileCoverage(t *testing.T) { + tests := []struct { + name string + covDB func(t *testing.T) spannerclient.SpannerClient + fileProv covermerger.FileVersProvider + url string + wantInRes []string + }{ + { + name: "empty db", + covDB: func(t *testing.T) spannerclient.SpannerClient { return emptyCoverageDBFixture(t, 1) }, + fileProv: staticFileProvider(t), + url: "/test2/graph/coverage/file?dateto=2025-01-31&period=month" + + "&commit=c0e75905caf368e19aab585d20151500e750de89&filepath=virt/kvm/kvm_main.c", + wantInRes: []string{"1 line1"}, + }, + { + name: "regular db", + covDB: func(t *testing.T) spannerclient.SpannerClient { return coverageDBFixture(t) }, + fileProv: staticFileProvider(t), + url: "/test2/graph/coverage/file?dateto=2025-01-31&period=month" + + "&commit=c0e75905caf368e19aab585d20151500e750de89&filepath=virt/kvm/kvm_main.c", + wantInRes: []string{ + "4 1 line1", + "5 2 line2", + "6 3 line3"}, + }, + { + name: "multimanager db", + covDB: func(t *testing.T) spannerclient.SpannerClient { return multiManagerCovDBFixture(t) }, + fileProv: staticFileProvider(t), + url: "/test2/graph/coverage/file?dateto=2025-01-31&period=month" + + "&commit=c0e75905caf368e19aab585d20151500e750de89&filepath=virt/kvm/kvm_main.c" + + "&manager=special-cc-manager&unique-only=1", + wantInRes: []string{ + " 0 1 line1", // Covered, is not unique. + " 5 2 line2", // Covered and is unique. + " 3 line3", // Covered only by "*" managers. + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c := NewCtx(t) + defer c.Close() + c.setCoverageMocks("test2", test.covDB(t), test.fileProv) + fileCovPage, err := c.GET(test.url) + assert.NoError(t, err) + got := string(fileCovPage) + for _, want := range test.wantInRes { + if !strings.Contains(got, want) { + t.Errorf(`"%s" wasn't found in "%s"'`, want, got) + } + } + }) + } +} + +func staticFileProvider(t *testing.T) covermerger.FileVersProvider { + m := mergermocks.NewFileVersProvider(t) + m.On("GetFileVersions", mock.Anything, mock.Anything). + Return(func(targetFilePath string, repoCommits ...covermerger.RepoCommit, + ) covermerger.FileVersions { + res := covermerger.FileVersions{} + for _, rc := range repoCommits { + res[rc] = `line1 +line2 +line3` + } + return res + }, nil) + return m +} + +func emptyCoverageDBFixture(t *testing.T, times int) spannerclient.SpannerClient { + mRowIterator := mocks.NewRowIterator(t) + mRowIterator.On("Stop").Return().Times(times) + mRowIterator.On("Next"). + Return(nil, iterator.Done).Times(times) + + mTran := mocks.NewReadOnlyTransaction(t) + mTran.On("Query", mock.Anything, mock.Anything). + Return(mRowIterator).Times(times) + + m := mocks.NewSpannerClient(t) + m.On("Single"). + Return(mTran).Times(times) + return m +} + +func coverageDBFixture(t *testing.T) spannerclient.SpannerClient { + mRowIt := newRowIteratorMock(t, []*coveragedb.LinesCoverage{{ + LinesInstrumented: []int64{1, 2, 3}, + HitCounts: []int64{4, 5, 6}, + }}) + + mTran := mocks.NewReadOnlyTransaction(t) + mTran.On("Query", mock.Anything, mock.Anything). + Return(mRowIt).Once() + + m := mocks.NewSpannerClient(t) + m.On("Single"). + Return(mTran).Once() + return m +} + +func multiManagerCovDBFixture(t *testing.T) spannerclient.SpannerClient { + mReadFullCoverageTran := mocks.NewReadOnlyTransaction(t) + mReadFullCoverageTran.On("Query", mock.Anything, mock.Anything). + Return(newRowIteratorMock(t, []*coveragedb.LinesCoverage{{ + LinesInstrumented: []int64{1, 2, 3}, + HitCounts: []int64{4, 5, 6}, + }})).Once() + + mReadPartialCoverageTran := mocks.NewReadOnlyTransaction(t) + mReadPartialCoverageTran.On("Query", mock.Anything, mock.Anything). + Return(newRowIteratorMock(t, []*coveragedb.LinesCoverage{{ + LinesInstrumented: []int64{1, 2}, + HitCounts: []int64{3, 5}, + }})).Once() + + m := mocks.NewSpannerClient(t) + // The order matters. Full coverage is fetched second. + m.On("Single"). + Return(mReadPartialCoverageTran).Once() + m.On("Single"). + Return(mReadFullCoverageTran).Once() + + return m +} + +func newRowIteratorMock(t *testing.T, cov []*coveragedb.LinesCoverage, +) *mocks.RowIterator { + m := mocks.NewRowIterator(t) + m.On("Stop").Once().Return() + for _, item := range cov { + mRow := mocks.NewRow(t) + mRow.On("ToStruct", mock.Anything). + Run(func(args mock.Arguments) { + arg := args.Get(0).(*coveragedb.LinesCoverage) + *arg = *item + }). + Return(nil).Once() + + m.On("Next"). + Return(mRow, nil).Once() + } + + m.On("Next"). + Return(nil, iterator.Done).Once() + return m +} diff --git a/dashboard/app/entities_spanner.go b/dashboard/app/entities_spanner.go index e3f3bad9e23d..c550ef92e217 100644 --- a/dashboard/app/entities_spanner.go +++ b/dashboard/app/entities_spanner.go @@ -6,7 +6,6 @@ package main import ( "context" "fmt" - "os" "cloud.google.com/go/civil" "cloud.google.com/go/spanner" @@ -24,14 +23,8 @@ type CoverageHistory struct { } // MergedCoverage uses dates, not time. -func MergedCoverage(ctx context.Context, ns, periodType string) (*CoverageHistory, error) { - projectID := os.Getenv("GOOGLE_CLOUD_PROJECT") - client, err := spannerclient.NewClient(ctx, projectID) - if err != nil { - return nil, fmt.Errorf("spanner.NewClient() failed: %s", err.Error()) - } - defer client.Close() - +func MergedCoverage(ctx context.Context, client spannerclient.SpannerClient, ns, periodType string, +) (*CoverageHistory, error) { minDays, maxDays, err := coveragedb.MinMaxDays(periodType) if err != nil { return nil, fmt.Errorf("coveragedb.MinMaxDays: %w", err) diff --git a/dashboard/app/handler.go b/dashboard/app/handler.go index e737c2b0b4ce..27817ab8c82e 100644 --- a/dashboard/app/handler.go +++ b/dashboard/app/handler.go @@ -32,6 +32,9 @@ func handlerWrapper(fn contextHandler) http.Handler { func handleContext(fn contextHandler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { c := appengine.NewContext(r) + if coverageDBClient != nil { // Nil in prod. + c = SetCoverageDBClient(c, coverageDBClient) + } c = context.WithValue(c, ¤tURLKey, r.URL.RequestURI()) authorizedUser, _ := userAccessLevel(currentUser(c), "", getConfig(c)) if !authorizedUser { diff --git a/dashboard/app/main.go b/dashboard/app/main.go index d765c8df96a8..071dc854438f 100644 --- a/dashboard/app/main.go +++ b/dashboard/app/main.go @@ -65,13 +65,11 @@ func initHTTPHandlers() { http.Handle("/"+ns+"/graph/fuzzing", handlerWrapper(handleGraphFuzzing)) http.Handle("/"+ns+"/graph/crashes", handlerWrapper(handleGraphCrashes)) http.Handle("/"+ns+"/graph/found-bugs", handlerWrapper(handleFoundBugsGraph)) - if nsConfig.Coverage != nil { - http.Handle("/"+ns+"/graph/coverage/file", handlerWrapper(handleFileCoverage)) - http.Handle("/"+ns+"/graph/coverage", handlerWrapper(handleCoverageGraph)) - http.Handle("/"+ns+"/graph/coverage_heatmap", handlerWrapper(handleCoverageHeatmap)) - if nsConfig.Subsystems.Service != nil { - http.Handle("/"+ns+"/graph/coverage_subsystems_heatmap", handlerWrapper(handleSubsystemsCoverageHeatmap)) - } + http.Handle("/"+ns+"/graph/coverage/file", handlerWrapper(handleFileCoverage)) + http.Handle("/"+ns+"/graph/coverage", handlerWrapper(handleCoverageGraph)) + http.Handle("/"+ns+"/graph/coverage_heatmap", handlerWrapper(handleCoverageHeatmap)) + if nsConfig.Subsystems.Service != nil { + http.Handle("/"+ns+"/graph/coverage_subsystems_heatmap", handlerWrapper(handleSubsystemsCoverageHeatmap)) } http.Handle("/"+ns+"/repos", handlerWrapper(handleRepos)) http.Handle("/"+ns+"/bug-summaries", handlerWrapper(handleBugSummaries)) diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go index d2ed77e32a67..be7841b27fb0 100644 --- a/dashboard/app/util_test.go +++ b/dashboard/app/util_test.go @@ -28,6 +28,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/syzkaller/dashboard/api" "github.com/google/syzkaller/dashboard/dashapi" + "github.com/google/syzkaller/pkg/coveragedb/spannerclient" + "github.com/google/syzkaller/pkg/covermerger" "github.com/google/syzkaller/pkg/email" "github.com/google/syzkaller/pkg/subsystem" "google.golang.org/appengine/v2/aetest" @@ -226,6 +228,20 @@ func (c *Ctx) setSubsystems(ns string, list []*subsystem.Subsystem, rev int) { } } +func (c *Ctx) setCoverageMocks(ns string, dbClientMock spannerclient.SpannerClient, + fileProvMock covermerger.FileVersProvider) { + c.transformContext = func(ctx context.Context) context.Context { + newConfig := replaceNamespaceConfig(ctx, ns, func(cfg *Config) *Config { + ret := *cfg + ret.Coverage = &CoverageConfig{WebGitURI: "test-git"} + return &ret + }) + ctxWithSpanner := SetCoverageDBClient(ctx, dbClientMock) + ctxWithSpannerAndFileProvider := setWebGit(ctxWithSpanner, fileProvMock) + return contextWithConfig(ctxWithSpannerAndFileProvider, newConfig) + } +} + func (c *Ctx) setKernelRepos(ns string, list []KernelRepo) { c.transformContext = func(c context.Context) context.Context { newConfig := replaceNamespaceConfig(c, ns, func(cfg *Config) *Config { diff --git a/pkg/cover/file.go b/pkg/cover/file.go index c56f882045c6..ea6c326f0a05 100644 --- a/pkg/cover/file.go +++ b/pkg/cover/file.go @@ -40,10 +40,10 @@ func DefaultHTMLRenderConfig() *CoverageRenderConfig { } } -func RendFileCoverage(repo, forCommit, filePath string, proxy covermerger.FuncProxyURI, +func RendFileCoverage(repo, forCommit, filePath string, fileProvider covermerger.FileVersProvider, mr *covermerger.MergeResult, renderConfig *CoverageRenderConfig) (string, error) { repoCommit := covermerger.RepoCommit{Repo: repo, Commit: forCommit} - files, err := covermerger.MakeWebGit(proxy).GetFileVersions(filePath, repoCommit) + files, err := fileProvider.GetFileVersions(filePath, repoCommit) if err != nil { return "", fmt.Errorf("failed to GetFileVersions: %w", err) } diff --git a/pkg/cover/heatmap.go b/pkg/cover/heatmap.go index 04b7e6914b77..4cc212fadac6 100644 --- a/pkg/cover/heatmap.go +++ b/pkg/cover/heatmap.go @@ -116,13 +116,13 @@ type fileCoverageWithDetails struct { Subsystems []string } -type fileCoverageWithLineInfo struct { +type FileCoverageWithLineInfo struct { fileCoverageWithDetails LinesInstrumented []int64 HitCounts []int64 } -func (fc *fileCoverageWithLineInfo) CovMap() map[int]int64 { +func (fc *FileCoverageWithLineInfo) CovMap() map[int]int64 { return MakeCovMap(fc.LinesInstrumented, fc.HitCounts) } @@ -223,12 +223,12 @@ func readCoverage(iterManager spannerclient.RowIterator) ([]*fileCoverageWithDet func readCoverageUniq(full, mgr spannerclient.RowIterator, ) ([]*fileCoverageWithDetails, error) { eg, ctx := errgroup.WithContext(context.Background()) - fullCh := make(chan *fileCoverageWithLineInfo) + fullCh := make(chan *FileCoverageWithLineInfo) eg.Go(func() error { defer close(fullCh) return readIterToChan(ctx, full, fullCh) }) - partCh := make(chan *fileCoverageWithLineInfo) + partCh := make(chan *FileCoverageWithLineInfo) eg.Go(func() error { defer close(partCh) return readIterToChan(ctx, mgr, partCh) @@ -309,7 +309,7 @@ func UniqCoverage(fullCov, partCov map[int]int64) map[int]int64 { return res } -func readIterToChan[K fileCoverageWithLineInfo | fileCoverageWithDetails]( +func readIterToChan[K FileCoverageWithLineInfo | fileCoverageWithDetails]( ctx context.Context, iter spannerclient.RowIterator, ch chan<- *K) error { for { row, err := iter.Next() diff --git a/pkg/cover/heatmap_test.go b/pkg/cover/heatmap_test.go index ad1f9bf645ff..0f66256e4e8d 100644 --- a/pkg/cover/heatmap_test.go +++ b/pkg/cover/heatmap_test.go @@ -65,7 +65,7 @@ func TestFilesCoverageWithDetails(t *testing.T) { client: func() spannerclient.SpannerClient { return fullCoverageDBFixture( t, - []*fileCoverageWithLineInfo{ + []*FileCoverageWithLineInfo{ { fileCoverageWithDetails: fileCoverageWithDetails{ Filepath: "file1", @@ -98,7 +98,7 @@ func TestFilesCoverageWithDetails(t *testing.T) { client: func() spannerclient.SpannerClient { return fullCoverageDBFixture( t, - []*fileCoverageWithLineInfo{ + []*FileCoverageWithLineInfo{ { fileCoverageWithDetails: fileCoverageWithDetails{ Filepath: "file1", @@ -109,7 +109,7 @@ func TestFilesCoverageWithDetails(t *testing.T) { HitCounts: []int64{1, 1, 1}, }, }, - []*fileCoverageWithLineInfo{ + []*FileCoverageWithLineInfo{ { fileCoverageWithDetails: fileCoverageWithDetails{ Filepath: "file1", @@ -141,7 +141,7 @@ func TestFilesCoverageWithDetails(t *testing.T) { client: func() spannerclient.SpannerClient { return fullCoverageDBFixture( t, - []*fileCoverageWithLineInfo{ + []*FileCoverageWithLineInfo{ { fileCoverageWithDetails: fileCoverageWithDetails{ Filepath: "file1", @@ -152,7 +152,7 @@ func TestFilesCoverageWithDetails(t *testing.T) { HitCounts: []int64{3, 4, 5, 6, 7}, }, }, - []*fileCoverageWithLineInfo{ + []*FileCoverageWithLineInfo{ { fileCoverageWithDetails: fileCoverageWithDetails{ Filepath: "file1", @@ -212,7 +212,7 @@ func emptyCoverageDBFixture(t *testing.T, times int) spannerclient.SpannerClient } func fullCoverageDBFixture( - t *testing.T, full, partial []*fileCoverageWithLineInfo, + t *testing.T, full, partial []*FileCoverageWithLineInfo, ) spannerclient.SpannerClient { mPartialTran := mocks.NewReadOnlyTransaction(t) mPartialTran.On("Query", mock.Anything, mock.Anything). @@ -230,7 +230,7 @@ func fullCoverageDBFixture( return m } -func newRowIteratorMock(t *testing.T, events []*fileCoverageWithLineInfo, +func newRowIteratorMock(t *testing.T, events []*FileCoverageWithLineInfo, ) *mocks.RowIterator { m := mocks.NewRowIterator(t) m.On("Stop").Once().Return() @@ -238,7 +238,7 @@ func newRowIteratorMock(t *testing.T, events []*fileCoverageWithLineInfo, mRow := mocks.NewRow(t) mRow.On("ToStruct", mock.Anything). Run(func(args mock.Arguments) { - arg := args.Get(0).(*fileCoverageWithLineInfo) + arg := args.Get(0).(*FileCoverageWithLineInfo) *arg = *item }). Return(nil).Once() diff --git a/pkg/coveragedb/coveragedb.go b/pkg/coveragedb/coveragedb.go index 22e84d8ebd23..792d4ec79256 100644 --- a/pkg/coveragedb/coveragedb.go +++ b/pkg/coveragedb/coveragedb.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "io" - "os" "sync/atomic" "time" @@ -74,6 +73,9 @@ type MergedCoverageRecord struct { func SaveMergeResult(ctx context.Context, client spannerclient.SpannerClient, descr *HistoryRecord, dec *json.Decoder, sss []*subsystem.Subsystem) (int, error) { + if client == nil { + return 0, fmt.Errorf("nil spannerclient") + } var rowsCreated int ssMatcher := subsystem.MakePathMatcher(sss) ssCache := make(map[string][]string) @@ -117,7 +119,7 @@ func SaveMergeResult(ctx context.Context, client spannerclient.SpannerClient, de return rowsCreated, nil } -type linesCoverage struct { +type LinesCoverage struct { LinesInstrumented []int64 HitCounts []int64 } @@ -147,15 +149,8 @@ where } } -func ReadLinesHitCount(ctx context.Context, ns, commit, file, manager string, tp TimePeriod, +func ReadLinesHitCount(ctx context.Context, client spannerclient.SpannerClient, ns, commit, file, manager string, tp TimePeriod, ) ([]int64, []int64, error) { - projectID := os.Getenv("GOOGLE_CLOUD_PROJECT") - client, err := spannerclient.NewClient(ctx, projectID) - if err != nil { - return nil, nil, fmt.Errorf("spanner.NewClient: %w", err) - } - defer client.Close() - stmt := linesCoverageStmt(ns, file, commit, manager, tp) iter := client.Single().Query(ctx, stmt) defer iter.Stop() @@ -167,10 +162,13 @@ func ReadLinesHitCount(ctx context.Context, ns, commit, file, manager string, tp if err != nil { return nil, nil, fmt.Errorf("iter.Next: %w", err) } - var r linesCoverage + var r LinesCoverage if err = row.ToStruct(&r); err != nil { return nil, nil, fmt.Errorf("failed to row.ToStruct() spanner DB: %w", err) } + if _, err := iter.Next(); err != iterator.Done { + return nil, nil, fmt.Errorf("more than 1 line is available") + } return r.LinesInstrumented, r.HitCounts, nil } @@ -230,13 +228,11 @@ func fileSubsystems(filePath string, ssMatcher *subsystem.PathMatcher, ssCache m return sss } -func NsDataMerged(ctx context.Context, projectID, ns string) ([]TimePeriod, []int64, error) { - client, err := spannerclient.NewClient(ctx, projectID) - if err != nil { - return nil, nil, fmt.Errorf("spanner.NewClient() failed: %s", err.Error()) +func NsDataMerged(ctx context.Context, client spannerclient.SpannerClient, ns string, +) ([]TimePeriod, []int64, error) { + if client == nil { + return nil, nil, fmt.Errorf("nil spannerclient") } - defer client.Close() - stmt := spanner.Statement{ SQL: ` select @@ -285,13 +281,11 @@ func NsDataMerged(ctx context.Context, projectID, ns string) ([]TimePeriod, []in // Note that in case of an error during batch deletion, some files may be deleted but not counted in the total. // // Returns the number of orphaned file entries successfully deleted. -func DeleteGarbage(ctx context.Context) (int64, error) { +func DeleteGarbage(ctx context.Context, client spannerclient.SpannerClient) (int64, error) { batchSize := 10_000 - client, err := spannerclient.NewClient(ctx, os.Getenv("GOOGLE_CLOUD_PROJECT")) - if err != nil { - return 0, fmt.Errorf("coveragedb.NewClient: %w", err) + if client == nil { + return 0, fmt.Errorf("nil spannerclient") } - defer client.Close() iter := client.Single().Query(ctx, spanner.Statement{ SQL: `SELECT session, filepath @@ -328,7 +322,7 @@ func DeleteGarbage(ctx context.Context) (int64, error) { } } goSpannerDelete(ctx, batch, eg, client, &totalDeleted) - if err = eg.Wait(); err != nil { + if err := eg.Wait(); err != nil { return 0, fmt.Errorf("spanner.Delete: %w", err) } return totalDeleted.Load(), nil diff --git a/pkg/covermerger/covermerger_test.go b/pkg/covermerger/covermerger_test.go index 36f42fdc1bd2..ab2b1efc5b1d 100644 --- a/pkg/covermerger/covermerger_test.go +++ b/pkg/covermerger/covermerger_test.go @@ -95,7 +95,7 @@ func TestMergerdCoverageRecords(t *testing.T) { FilePath: "file.c", MergeResult: &MergeResult{ FileExists: true, - HitCounts: map[int]int{ + HitCounts: map[int]int64{ 1: 5, 2: 7, }, @@ -367,8 +367,8 @@ type fileVersProviderMock struct { } func (m *fileVersProviderMock) GetFileVersions(targetFilePath string, repoCommits ...RepoCommit, -) (fileVersions, error) { - res := make(fileVersions) +) (FileVersions, error) { + res := make(FileVersions) for _, repoCommit := range repoCommits { filePath := filepath.Join(m.Workdir, "repos", repoCommit.Commit, targetFilePath) if bytes, err := os.ReadFile(filePath); err == nil { diff --git a/pkg/covermerger/file_line_merger.go b/pkg/covermerger/file_line_merger.go index ebc747f47abc..817099a60786 100644 --- a/pkg/covermerger/file_line_merger.go +++ b/pkg/covermerger/file_line_merger.go @@ -5,7 +5,7 @@ package covermerger import "github.com/google/syzkaller/pkg/log" -func makeFileLineCoverMerger(fvs fileVersions, base RepoCommit) FileCoverageMerger { +func makeFileLineCoverMerger(fvs FileVersions, base RepoCommit) FileCoverageMerger { baseFile := "" baseFileExists := false for repoCommit, fv := range fvs { diff --git a/pkg/covermerger/mocks/FileVersProvider.go b/pkg/covermerger/mocks/FileVersProvider.go new file mode 100644 index 000000000000..69fadfa425c7 --- /dev/null +++ b/pkg/covermerger/mocks/FileVersProvider.go @@ -0,0 +1,64 @@ +// Code generated by mockery v2.45.1. DO NOT EDIT. + +package mocks + +import ( + covermerger "github.com/google/syzkaller/pkg/covermerger" + mock "github.com/stretchr/testify/mock" +) + +// FileVersProvider is an autogenerated mock type for the FileVersProvider type +type FileVersProvider struct { + mock.Mock +} + +// GetFileVersions provides a mock function with given fields: targetFilePath, repoCommits +func (_m *FileVersProvider) GetFileVersions(targetFilePath string, repoCommits ...covermerger.RepoCommit) (covermerger.FileVersions, error) { + _va := make([]interface{}, len(repoCommits)) + for _i := range repoCommits { + _va[_i] = repoCommits[_i] + } + var _ca []interface{} + _ca = append(_ca, targetFilePath) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + if len(ret) == 0 { + panic("no return value specified for GetFileVersions") + } + + var r0 covermerger.FileVersions + var r1 error + if rf, ok := ret.Get(0).(func(string, ...covermerger.RepoCommit) (covermerger.FileVersions, error)); ok { + return rf(targetFilePath, repoCommits...) + } + if rf, ok := ret.Get(0).(func(string, ...covermerger.RepoCommit) covermerger.FileVersions); ok { + r0 = rf(targetFilePath, repoCommits...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(covermerger.FileVersions) + } + } + + if rf, ok := ret.Get(1).(func(string, ...covermerger.RepoCommit) error); ok { + r1 = rf(targetFilePath, repoCommits...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewFileVersProvider creates a new instance of FileVersProvider. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewFileVersProvider(t interface { + mock.TestingT + Cleanup(func()) +}) *FileVersProvider { + mock := &FileVersProvider{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/covermerger/provider_monorepo.go b/pkg/covermerger/provider_monorepo.go index e6c67d1d1901..73a08786a1c6 100644 --- a/pkg/covermerger/provider_monorepo.go +++ b/pkg/covermerger/provider_monorepo.go @@ -3,6 +3,8 @@ package covermerger +//go:generate ../../tools/mockery.sh --name FileVersProvider -r + import ( "fmt" "path/filepath" @@ -15,7 +17,7 @@ import ( type FileVersProvider interface { GetFileVersions(targetFilePath string, repoCommits ...RepoCommit, - ) (fileVersions, error) + ) (FileVersions, error) } type monoRepo struct { @@ -24,10 +26,10 @@ type monoRepo struct { repo vcs.Repo } -type fileVersions map[RepoCommit]string +type FileVersions map[RepoCommit]string func (mr *monoRepo) GetFileVersions(targetFilePath string, repoCommits ...RepoCommit, -) (fileVersions, error) { +) (FileVersions, error) { mr.mu.RLock() if !mr.allRepoCommitsPresent(repoCommits) { mr.mu.RUnlock() @@ -35,7 +37,7 @@ func (mr *monoRepo) GetFileVersions(targetFilePath string, repoCommits ...RepoCo mr.mu.RLock() } defer mr.mu.RUnlock() - res := make(fileVersions) + res := make(FileVersions) for _, repoCommit := range repoCommits { fileBytes, err := mr.repo.Object(targetFilePath, repoCommit.Commit) // It is ok if some file doesn't exist. It means we have repo FS diff. diff --git a/pkg/covermerger/provider_web.go b/pkg/covermerger/provider_web.go index 43bfee7e0832..554ee3f97fa8 100644 --- a/pkg/covermerger/provider_web.go +++ b/pkg/covermerger/provider_web.go @@ -18,8 +18,8 @@ type webGit struct { } func (mr *webGit) GetFileVersions(targetFilePath string, repoCommits ...RepoCommit, -) (fileVersions, error) { - res := make(fileVersions) +) (FileVersions, error) { + res := make(FileVersions) for _, repoCommit := range repoCommits { fileBytes, err := mr.loadFile(targetFilePath, repoCommit.Repo, repoCommit.Commit) // It is ok if some file doesn't exist. It means we have repo FS diff. diff --git a/pkg/validator/validator.go b/pkg/validator/validator.go index b9031302aa7d..12224963ec59 100644 --- a/pkg/validator/validator.go +++ b/pkg/validator/validator.go @@ -74,7 +74,7 @@ var ( CommitHash = makeCombinedStrFunc("not a hash", AlphaNumeric, makeStrLenFunc("len is not 40", 40)) KernelFilePath = makeStrReFunc("not a kernel file path", "^[./_a-zA-Z0-9-]*$") NamespaceName = makeStrReFunc("not a namespace name", "^[a-zA-Z0-9_.-]{4,32}$") - ManagerName = makeStrReFunc("not a manager name", "^ci[a-z0-9-]*$") + ManagerName = makeStrReFunc("not a manager name", "^[a-z0-9-]*$") DashClientName = makeStrReFunc("not a dashboard client name", "^[a-zA-Z0-9_.-]{4,100}$") DashClientKey = makeStrReFunc("not a dashboard client key", "^([a-zA-Z0-9]{16,128})|("+regexp.QuoteMeta(auth.OauthMagic)+".*)$") diff --git a/tools/syz-cover/syz-cover.go b/tools/syz-cover/syz-cover.go index bbdb4bd96be4..4a72b9bf6479 100644 --- a/tools/syz-cover/syz-cover.go +++ b/tools/syz-cover/syz-cover.go @@ -38,6 +38,7 @@ import ( "github.com/google/syzkaller/pkg/cover" "github.com/google/syzkaller/pkg/cover/backend" "github.com/google/syzkaller/pkg/coveragedb" + "github.com/google/syzkaller/pkg/covermerger" "github.com/google/syzkaller/pkg/log" "github.com/google/syzkaller/pkg/mgrconfig" "github.com/google/syzkaller/pkg/osutil" @@ -92,7 +93,7 @@ func toolFileCover() { *flagRepo, *flagCommit, *flagForFile, - nil, // no proxy - get files directly from WebGits + covermerger.MakeWebGit(nil), // get files directly from WebGits mr, config, )