testutils: roachtest log import linter

Previously, it was possible to add the CRDB log or the standard golang log as an
import in roachtests. This adds a new linter that prevents using incorrect
logging in roachtests. The logger supplied by the test framework to a test
should be used in all circumstances.

Epic: None
Release note: None

pkg/testutils/lint/lint_test.go

@@ -198,6 +198,31 @@ func TestLint(t *testing.T) {
 		t.Error(err)
 	}
 
+	// Things that are package scoped are below here.
+	pkgScope := pkgVar
+	if !pkgSpecified {
+		pkgScope = "./pkg/..."
+	}
+
+	// Load packages for top-level forbidden import tests.
+	pkgPath := filepath.Join(cockroachDB, pkgScope)
+	pkgs, err := packages.Load(
+		&packages.Config{
+			Mode: packages.NeedImports | packages.NeedName,
+			Dir:  crdbDir,
+		},
+		pkgPath,
+	)
+	if err != nil {
+		t.Fatal(errors.Wrapf(err, "error loading package %s", pkgPath))
+	}
+	// NB: if no packages were found, this API confusingly
+	// returns no error, so we need to explicitly check that
+	// something was returned.
+	if len(pkgs) == 0 {
+		t.Fatalf("could not list packages under %s", pkgPath)
+	}
+
 	t.Run("TestLowercaseFunctionNames", func(t *testing.T) {
 		skip.UnderShort(t)
 		t.Parallel()
@@ -1804,12 +1829,6 @@ func TestLint(t *testing.T) {
 		}
 	})
 
-	// Things that are packaged scoped are below here.
-	pkgScope := pkgVar
-	if !pkgSpecified {
-		pkgScope = "./pkg/..."
-	}
-
 	t.Run("TestForbiddenImports", func(t *testing.T) {
 		t.Parallel()
 
@@ -1850,23 +1869,6 @@ func TestLint(t *testing.T) {
 		grepBuf.WriteString(")$")
 
 		filter := stream.FilterFunc(func(arg stream.Arg) error {
-			pkgPath := filepath.Join(cockroachDB, pkgScope)
-			pkgs, err := packages.Load(
-				&packages.Config{
-					Mode: packages.NeedImports | packages.NeedName,
-					Dir:  crdbDir,
-				},
-				pkgPath,
-			)
-			if err != nil {
-				return errors.Wrapf(err, "error loading package %s", pkgPath)
-			}
-			// NB: if no packages were found, this API confusingly
-			// returns no error, so we need to explicitly check that
-			// something was returned.
-			if len(pkgs) == 0 {
-				return errors.Newf("could not list packages under %s", pkgPath)
-			}
 			for _, pkg := range pkgs {
 				for _, s := range pkg.Imports {
 					arg.Out <- pkg.PkgPath + ": " + s.PkgPath
@@ -2794,4 +2796,56 @@ func TestLint(t *testing.T) {
 			}
 		}
 	})
+
+	// Test forbidden roachtest imports.
+	t.Run("TestRoachtestForbiddenImports", func(t *testing.T) {
+		t.Parallel()
+
+		roachprodLoggerPkg := "github.com/cockroachdb/cockroach/pkg/roachprod/logger"
+		// forbiddenImportPkg -> permittedReplacementPkg
+		forbiddenImports := map[string]string{
+			"github.com/cockroachdb/cockroach/pkg/util/log": roachprodLoggerPkg,
+			"log": roachprodLoggerPkg,
+		}
+
+		// grepBuf creates a grep string that matches any forbidden import pkgs.
+		var grepBuf bytes.Buffer
+		grepBuf.WriteByte('(')
+		for forbiddenPkg := range forbiddenImports {
+			grepBuf.WriteByte('|')
+			grepBuf.WriteString(regexp.QuoteMeta(forbiddenPkg))
+		}
+		grepBuf.WriteString(")$")
+
+		filter := stream.FilterFunc(func(arg stream.Arg) error {
+			for _, pkg := range pkgs {
+				for _, s := range pkg.Imports {
+					arg.Out <- pkg.PkgPath + ": " + s.PkgPath
+				}
+			}
+			return nil
+		})
+		numAnalyzed := 0
+		if err := stream.ForEach(stream.Sequence(
+			filter,
+			stream.Sort(),
+			stream.Uniq(),
+			stream.Grep(`cockroach/pkg/cmd/roachtest/(tests|operations): `),
+		), func(s string) {
+			pkgStr := strings.Split(s, ": ")
+			_, importedPkg := pkgStr[0], pkgStr[1]
+			numAnalyzed++
+
+			// Test that a disallowed package is not imported.
+			if replPkg, ok := forbiddenImports[importedPkg]; ok {
+				t.Errorf("\n%s <- please use %q instead of %q", s, replPkg, importedPkg)
+			}
+		}); err != nil {
+			t.Error(err)
+		}
+		if numAnalyzed == 0 {
+			t.Errorf("Empty input! Please check the linter.")
+		}
+	})
+
 }