packages: move coursier init side effects to coursier handle constructor (#49157)

The side-effects of importing the `coursier` package can be unintended
effects when accidentally imported into an unsuspecting binary besides
repo-updater. To avoid this, we wrap these side-effects in a handle
constructor, so that they will only occur when the handle is explicitly
constructed (so far, this only happens in gitserver in `GetVCSSyncer`)

## Test plan

Only gitserver imports NewJVMPackageSyncer, while migrator imports
internal/repos which imports cmd/gitserver/server (shouldnt do that but
here we are), hence the side effects occurred here when they shouldnt

cmd/gitserver/server/vcs_syncer_jvm_packages.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/sourcegraph/log"
 
+	"github.com/sourcegraph/sourcegraph/internal/observation"
 	"github.com/sourcegraph/sourcegraph/internal/unpack"
 
 	"github.com/sourcegraph/sourcegraph/internal/api"
@@ -46,6 +47,8 @@ func NewJVMPackagesSyncer(connection *schema.JVMPackagesConnection, svc *depende
 		configDeps = connection.Maven.Dependencies
 	}
 
+	chandle := coursier.NewCoursierHandle(observation.NewContext(log.Scoped("gitserver.jvmsyncer", "")))
+
 	return &vcsPackagesSyncer{
 		logger:      log.Scoped("JVMPackagesSyncer", "sync JVM packages"),
 		typ:         "jvm_packages",
@@ -53,13 +56,18 @@ func NewJVMPackagesSyncer(connection *schema.JVMPackagesConnection, svc *depende
 		placeholder: placeholder,
 		svc:         svc,
 		configDeps:  configDeps,
-		source:      &jvmPackagesSyncer{config: connection, fetch: coursier.FetchSources},
+		source: &jvmPackagesSyncer{
+			coursier: chandle,
+			config:   connection,
+			fetch:    chandle.FetchSources,
+		},
 	}
 }
 
 type jvmPackagesSyncer struct {
-	config *schema.JVMPackagesConnection
-	fetch  func(ctx context.Context, config *schema.JVMPackagesConnection, dependency *reposource.MavenVersionedPackage) (sourceCodeJarPath string, err error)
+	coursier *coursier.CoursierHandle
+	config   *schema.JVMPackagesConnection
+	fetch    func(ctx context.Context, config *schema.JVMPackagesConnection, dependency *reposource.MavenVersionedPackage) (sourceCodeJarPath string, err error)
 }
 
 func (jvmPackagesSyncer) ParseVersionedPackageFromNameAndVersion(name reposource.PackageName, version string) (reposource.VersionedPackage, error) {
@@ -169,7 +177,7 @@ func (s *jvmPackagesSyncer) inferJVMVersionFromByteCode(ctx context.Context,
 		return dependency.Version, nil
 	}
 
-	byteCodeJarPath, err := coursier.FetchByteCode(ctx, s.config, dependency)
+	byteCodeJarPath, err := s.coursier.FetchByteCode(ctx, s.config, dependency)
 	if err != nil {
 		return "", err
 	}

cmd/gitserver/server/vcs_syncer_jvm_packages_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/sourcegraph/sourcegraph/internal/database"
 	"github.com/sourcegraph/sourcegraph/internal/database/dbtest"
 	"github.com/sourcegraph/sourcegraph/internal/extsvc/jvmpackages/coursier"
+	"github.com/sourcegraph/sourcegraph/internal/observation"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 	"github.com/sourcegraph/sourcegraph/schema"
 )
@@ -78,7 +79,7 @@ func assertCommandOutput(t *testing.T, cmd *exec.Cmd, workingDir, expectedOut st
 }
 
 func coursierScript(t *testing.T, dir string) string {
-	coursierPath, err := os.OpenFile(path.Join(dir, "coursier"), os.O_CREATE|os.O_RDWR, 07777)
+	coursierPath, err := os.OpenFile(path.Join(dir, "coursier"), os.O_CREATE|os.O_RDWR, 0o7777)
 	assert.Nil(t, err)
 	defer coursierPath.Close()
 	script := fmt.Sprintf(`#!/usr/bin/env bash
@@ -129,7 +130,8 @@ func TestNoMaliciousFiles(t *testing.T) {
 	assert.Nil(t, os.Mkdir(extractPath, os.ModePerm))
 
 	s := jvmPackagesSyncer{
-		config: &schema.JVMPackagesConnection{Maven: &schema.Maven{Dependencies: []string{}}},
+		coursier: coursier.NewCoursierHandle(&observation.TestContext),
+		config:   &schema.JVMPackagesConnection{Maven: &schema.Maven{Dependencies: []string{}}},
 		fetch: func(ctx context.Context, config *schema.JVMPackagesConnection, dependency *reposource.MavenVersionedPackage) (sourceCodeJarPath string, err error) {
 			jarPath := path.Join(dir, "sampletext.zip")
 			createMaliciousJar(t, jarPath)

internal/extsvc/jvmpackages/coursier/coursier.go

@@ -9,6 +9,7 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
+	"sync"
 	"time"
 
 	otlog "github.com/opentracing/opentracing-go/log"
@@ -24,31 +25,43 @@ import (
 var CoursierBinary = "coursier"
 
 var (
-	coursierCacheDir string
-	invocTimeout, _  = time.ParseDuration(env.Get("SRC_COURSIER_TIMEOUT", "2m", "Time limit per Coursier invocation, which is used to resolve JVM/Java dependencies."))
-)
-
-func init() {
+	invocTimeout, _ = time.ParseDuration(env.Get("SRC_COURSIER_TIMEOUT", "2m", "Time limit per Coursier invocation, which is used to resolve JVM/Java dependencies."))
 	// if COURSIER_CACHE_DIR is set, try create that dir and use it. If not set, use the SRC_REPOS_DIR value (or default).
 	// This is expected to only be used in gitserver, if this assumption changes, please revisit this due to the failability
 	// of this on read-only filesystems.
 	coursierCacheDir = env.Get("COURSIER_CACHE_DIR", "", "Directory in which coursier data is cached for JVM package repos.")
-	srcReposDir := env.Get("SRC_REPOS_DIR", "/data/repos", "Root dir containing repos.")
-	if coursierCacheDir == "" && srcReposDir != "" {
-		coursierCacheDir = filepath.Join(srcReposDir, "coursier")
+	srcReposDir      = env.Get("SRC_REPOS_DIR", "/data/repos", "Root dir containing repos.")
+	mkdirOnce        sync.Once
+)
+
+type CoursierHandle struct {
+	operations *operations
+}
+
+func NewCoursierHandle(obsctx *observation.Context) *CoursierHandle {
+	mkdirOnce.Do(func() {
+		if coursierCacheDir == "" && srcReposDir != "" {
+			coursierCacheDir = filepath.Join(srcReposDir, "coursier")
+		}
+		if coursierCacheDir != "" {
+			if err := os.MkdirAll(coursierCacheDir, os.ModePerm); err != nil {
+				panic(fmt.Sprintf("failed to create coursier cache dir in %q: %s\n", coursierCacheDir, err))
+			}
+		}
+	})
+	return &CoursierHandle{
+		operations: newOperations(obsctx),
 	}
 }
 
-func FetchSources(ctx context.Context, config *schema.JVMPackagesConnection, dependency *reposource.MavenVersionedPackage) (sourceCodeJarPath string, err error) {
-	operations := getOperations()
-
-	ctx, _, endObservation := operations.fetchSources.With(ctx, &err, observation.Args{LogFields: []otlog.Field{
+func (c *CoursierHandle) FetchSources(ctx context.Context, config *schema.JVMPackagesConnection, dependency *reposource.MavenVersionedPackage) (sourceCodeJarPath string, err error) {
+	ctx, _, endObservation := c.operations.fetchSources.With(ctx, &err, observation.Args{LogFields: []otlog.Field{
 		otlog.String("dependency", dependency.VersionedPackageSyntax()),
 	}})
 	defer endObservation(1, observation.Args{})
 
 	if dependency.IsJDK() {
-		output, err := runCoursierCommand(
+		output, err := c.runCoursierCommand(
 			ctx,
 			config,
 			"java-home", "--jvm",
@@ -70,7 +83,7 @@ func FetchSources(ctx context.Context, config *schema.JVMPackagesConnection, dep
 		}
 		return "", errors.Errorf("failed to find src.zip for JVM dependency %s", dependency)
 	}
-	paths, err := runCoursierCommand(
+	paths, err := c.runCoursierCommand(
 		ctx,
 		config,
 		// NOTE: make sure to update the method `coursierScript` in
@@ -94,13 +107,11 @@ func FetchSources(ctx context.Context, config *schema.JVMPackagesConnection, dep
 	return paths[0], nil
 }
 
-func FetchByteCode(ctx context.Context, config *schema.JVMPackagesConnection, dependency *reposource.MavenVersionedPackage) (byteCodeJarPath string, err error) {
-	operations := getOperations()
-
-	ctx, _, endObservation := operations.fetchByteCode.With(ctx, &err, observation.Args{})
+func (c *CoursierHandle) FetchByteCode(ctx context.Context, config *schema.JVMPackagesConnection, dependency *reposource.MavenVersionedPackage) (byteCodeJarPath string, err error) {
+	ctx, _, endObservation := c.operations.fetchByteCode.With(ctx, &err, observation.Args{})
 	defer endObservation(1, observation.Args{})
 
-	paths, err := runCoursierCommand(
+	paths, err := c.runCoursierCommand(
 		ctx,
 		config,
 		// NOTE: make sure to update the method `coursierScript` in
@@ -123,18 +134,16 @@ func FetchByteCode(ctx context.Context, config *schema.JVMPackagesConnection, de
 	return paths[0], nil
 }
 
-func Exists(ctx context.Context, config *schema.JVMPackagesConnection, dependency *reposource.MavenVersionedPackage) (err error) {
-	operations := getOperations()
-
-	ctx, _, endObservation := operations.exists.With(ctx, &err, observation.Args{LogFields: []otlog.Field{
+func (c *CoursierHandle) Exists(ctx context.Context, config *schema.JVMPackagesConnection, dependency *reposource.MavenVersionedPackage) (err error) {
+	ctx, _, endObservation := c.operations.exists.With(ctx, &err, observation.Args{LogFields: []otlog.Field{
 		otlog.String("dependency", dependency.VersionedPackageSyntax()),
 	}})
 	defer endObservation(1, observation.Args{})
 
 	if dependency.IsJDK() {
-		_, err = FetchSources(ctx, config, dependency)
+		_, err = c.FetchSources(ctx, config, dependency)
 	} else {
-		_, err = runCoursierCommand(
+		_, err = c.runCoursierCommand(
 			ctx,
 			config,
 			"resolve",
@@ -154,13 +163,11 @@ func (e coursierError) NotFound() bool {
 	return true
 }
 
-func runCoursierCommand(ctx context.Context, config *schema.JVMPackagesConnection, args ...string) (stdoutLines []string, err error) {
-	operations := getOperations()
-
+func (c *CoursierHandle) runCoursierCommand(ctx context.Context, config *schema.JVMPackagesConnection, args ...string) (stdoutLines []string, err error) {
 	ctx, cancel := context.WithTimeout(ctx, invocTimeout)
 	defer cancel()
 
-	ctx, trace, endObservation := operations.runCommand.With(ctx, &err, observation.Args{LogFields: []otlog.Field{
+	ctx, trace, endObservation := c.operations.runCommand.With(ctx, &err, observation.Args{LogFields: []otlog.Field{
 		otlog.String("repositories", strings.Join(config.Maven.Repositories, "|")),
 		otlog.String("args", strings.Join(args, ", ")),
 	}})
@@ -177,11 +184,6 @@ func runCoursierCommand(ctx context.Context, config *schema.JVMPackagesConnectio
 		)
 	}
 	if coursierCacheDir != "" {
-		// TODO: Don't run this every time we run a coursier command. Hotfix to fix
-		// production.
-		if err := os.MkdirAll(coursierCacheDir, os.ModePerm); err != nil {
-			return nil, errors.Wrapf(err, "failed to create coursier cache dir in %q", coursierCacheDir)
-		}
 		cmd.Env = append(cmd.Env, "COURSIER_CACHE="+coursierCacheDir)
 	}