From b69b69f1c25b6e36df1f4369c5731b089fad92fa Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Mon, 28 Aug 2023 14:46:30 -0700 Subject: [PATCH] :bug: Ignore missing tarballs for empty org .github repos (#3433) * Ignore tarball errors on org's .github folder Signed-off-by: Spencer Schrock * add test. Signed-off-by: Spencer Schrock --------- Signed-off-by: Spencer Schrock --- clients/githubrepo/tarball.go | 10 ++++- clients/githubrepo/tarball_test.go | 64 ++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/clients/githubrepo/tarball.go b/clients/githubrepo/tarball.go index 69bf8c59d04..3b3978e0a3d 100644 --- a/clients/githubrepo/tarball.go +++ b/clients/githubrepo/tarball.go @@ -35,8 +35,9 @@ import ( ) const ( - repoDir = "repo*" - repoFilename = "githubrepo*.tar.gz" + repoDir = "repo*" + repoFilename = "githubrepo*.tar.gz" + orgGithubRepo = ".github" ) var ( @@ -94,6 +95,11 @@ func (handler *tarballHandler) setup() error { // Setup temp dir/files and download repo tarball. if err := handler.getTarball(); errors.Is(err, errTarballNotFound) { + // don't warn for "someorg/.github" repos + // https://github.com/ossf/scorecard/issues/3076 + if handler.repo.GetName() == orgGithubRepo { + return + } log.Printf("unable to get tarball %v. Skipping...", err) return } else if err != nil { diff --git a/clients/githubrepo/tarball_test.go b/clients/githubrepo/tarball_test.go index f99b5ee7de3..c046675f11e 100644 --- a/clients/githubrepo/tarball_test.go +++ b/clients/githubrepo/tarball_test.go @@ -15,9 +15,14 @@ package githubrepo import ( + "bytes" + "context" "errors" "fmt" "io" + "log" + "net/http" + "net/http/httptest" "os" "strings" "sync" @@ -25,6 +30,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/google/go-github/v53/github" + + "github.com/ossf/scorecard/v4/clients" ) type listfileTest struct { @@ -175,3 +183,59 @@ func TestExtractTarball(t *testing.T) { }) } } + +// temporarily redirect default logger output somewhere else. +func setLogOutput(t *testing.T, w io.Writer) { + t.Helper() + old := log.Writer() + log.SetOutput(w) + t.Cleanup(func() { log.SetOutput(old) }) +} + +//nolint:paralleltest // modifying log output +func Test_setup_empty_repo(t *testing.T) { + tests := []struct { + name string + repo string + wantLog bool + }{ + { + name: "org .github has no log message", + repo: ".github", + wantLog: false, + }, + { + name: "non .github repo has log message", + repo: "foo", + wantLog: true, + }, + } + // server always responds with bad request to trigger errTarballNotFound + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + })) + t.Cleanup(ts.Close) + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + h := tarballHandler{ + httpClient: http.DefaultClient, + } + archiveURL := ts.URL + "/{archive_format}" + r := github.Repository{ + Name: &tt.repo, + ArchiveURL: &archiveURL, + } + var buf bytes.Buffer + setLogOutput(t, &buf) + h.init(context.Background(), &r, clients.HeadSHA) + err := h.setup() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if (tt.wantLog) != (buf.Len() > 0) { + t.Errorf("wanted log: %t, log: %q", tt.wantLog, buf.String()) + } + }) + } +}