Refactor k8s provider to use shared git client instead of initializing a new one

pkg/app/piped/cloudprovider/kubernetes/kubernetes.go

@@ -79,18 +79,13 @@ type gitClient interface {
 	Clone(ctx context.Context, repoID, remote, branch, destination string) (git.Repo, error)
 }
 
-var (
-	// shared gitClient used inside this package for downloading dependencies.
-	sharedGitClient         gitClient
-	initSharedGitClientOnce sync.Once
-)
-
 type provider struct {
 	appName        string
 	appDir         string
 	repoDir        string
 	configFileName string
 	input          config.KubernetesDeploymentInput
+	gc             gitClient
 	logger         *zap.Logger
 
 	kubectl          *Kubectl
@@ -101,35 +96,35 @@ type provider struct {
 	initErr          error
 }
 
-func initSharedGitClient(logger *zap.Logger) error {
-	var err error
-	initSharedGitClientOnce.Do(func() {
-		sharedGitClient, err = git.NewClient(git.WithLogger(logger))
-	})
-	return err
-}
+func NewProvider(
+	appName, appDir, repoDir, configFileName string,
+	input config.KubernetesDeploymentInput,
+	gc gitClient,
+	logger *zap.Logger,
+) Provider {
 
-func NewProvider(appName, appDir, repoDir, configFileName string, input config.KubernetesDeploymentInput, logger *zap.Logger) Provider {
 	return &provider{
 		appName:        appName,
 		appDir:         appDir,
 		repoDir:        repoDir,
 		configFileName: configFileName,
 		input:          input,
+		gc:             gc,
 		logger:         logger.Named("kubernetes-provider"),
 	}
 }
 
-func NewManifestLoader(appName, appDir, repoDir, configFileName string, input config.KubernetesDeploymentInput, logger *zap.Logger) ManifestLoader {
-	return NewProvider(appName, appDir, repoDir, configFileName, input, logger)
+func NewManifestLoader(
+	appName, appDir, repoDir, configFileName string,
+	input config.KubernetesDeploymentInput,
+	gc gitClient,
+	logger *zap.Logger,
+) ManifestLoader {
+
+	return NewProvider(appName, appDir, repoDir, configFileName, input, gc, logger)
 }
 
 func (p *provider) init(ctx context.Context) {
-	if err := initSharedGitClient(p.logger); err != nil {
-		p.initErr = err
-		return
-	}
-
 	p.templatingMethod = determineTemplatingMethod(p.input, p.appDir)
 
 	// We need kubectl for all templating methods.
@@ -169,7 +164,7 @@ func (p *provider) LoadManifests(ctx context.Context) (manifests []Manifest, err
 				p.appDir,
 				p.input.Namespace,
 				chart,
-				sharedGitClient,
+				p.gc,
 				p.input.HelmOptions)
 
 		case p.input.HelmChart.Repository != "":

pkg/app/piped/controller/planner.go

@@ -165,6 +165,7 @@ func (p *planner) Run(ctx context.Context) error {
 		PipedConfig:                    p.pipedConfig,
 		AppManifestsCache:              p.appManifestsCache,
 		RegexPool:                      regexpool.DefaultPool(),
+		GitClient:                      p.gitClient,
 		Logger:                         p.logger,
 	}
 

pkg/app/piped/driftdetector/kubernetes/detector.go

@@ -247,7 +247,7 @@ func (d *detector) loadHeadManifests(ctx context.Context, app *model.Application
 			}
 		}
 
-		loader := provider.NewManifestLoader(app.Name, appDir, repoDir, app.GitPath.ConfigFilename, cfg.KubernetesDeploymentSpec.Input, d.logger)
+		loader := provider.NewManifestLoader(app.Name, appDir, repoDir, app.GitPath.ConfigFilename, cfg.KubernetesDeploymentSpec.Input, d.gitClient, d.logger)
 		manifests, err = loader.LoadManifests(ctx)
 		if err != nil {
 			err = fmt.Errorf("failed to load new manifests: %w", err)

pkg/app/piped/executor/kubernetes/kubernetes.go

@@ -96,7 +96,7 @@ func (e *deployExecutor) Execute(sig executor.StopSignal) model.StageStatus {
 		}
 	}
 
-	e.provider = provider.NewProvider(e.Deployment.ApplicationName, ds.AppDir, ds.RepoDir, e.Deployment.GitPath.ConfigFilename, e.deployCfg.Input, e.Logger)
+	e.provider = provider.NewProvider(e.Deployment.ApplicationName, ds.AppDir, ds.RepoDir, e.Deployment.GitPath.ConfigFilename, e.deployCfg.Input, e.GitClient, e.Logger)
 	e.Logger.Info("start executing kubernetes stage",
 		zap.String("stage-name", e.Stage.Name),
 		zap.String("app-dir", ds.AppDir),
@@ -157,6 +157,7 @@ func (e *deployExecutor) loadRunningManifests(ctx context.Context) (manifests []
 				ds.RepoDir,
 				e.Deployment.GitPath.ConfigFilename,
 				e.deployCfg.Input,
+				e.GitClient,
 				e.Logger,
 			)
 			return loader.LoadManifests(ctx)

pkg/app/piped/executor/kubernetes/rollback.go

@@ -74,7 +74,7 @@ func (e *rollbackExecutor) ensureRollback(ctx context.Context) model.StageStatus
 		}
 	}
 
-	p := provider.NewProvider(e.Deployment.ApplicationName, ds.AppDir, ds.RepoDir, e.Deployment.GitPath.ConfigFilename, deployCfg.Input, e.Logger)
+	p := provider.NewProvider(e.Deployment.ApplicationName, ds.AppDir, ds.RepoDir, e.Deployment.GitPath.ConfigFilename, deployCfg.Input, e.GitClient, e.Logger)
 	e.Logger.Info("start executing kubernetes stage",
 		zap.String("stage-name", e.Stage.Name),
 		zap.String("app-dir", ds.AppDir),

pkg/app/piped/planner/BUILD.bazel

@@ -12,6 +12,7 @@ go_library(
         "//pkg/app/piped/deploysource:go_default_library",
         "//pkg/cache:go_default_library",
         "//pkg/config:go_default_library",
+        "//pkg/git:go_default_library",
         "//pkg/model:go_default_library",
         "//pkg/regexpool:go_default_library",
         "@org_uber_go_zap//:go_default_library",

pkg/app/piped/planner/kubernetes/kubernetes.go

@@ -81,7 +81,7 @@ func (p *Planner) Plan(ctx context.Context, in planner.Input) (out planner.Outpu
 	newManifests, ok := manifestCache.Get(in.Trigger.Commit.Hash)
 	if !ok {
 		// When the manifests were not in the cache we have to load them.
-		loader := provider.NewManifestLoader(in.ApplicationName, ds.AppDir, ds.RepoDir, in.GitPath.ConfigFilename, cfg.Input, in.Logger)
+		loader := provider.NewManifestLoader(in.ApplicationName, ds.AppDir, ds.RepoDir, in.GitPath.ConfigFilename, cfg.Input, in.GitClient, in.Logger)
 		newManifests, err = loader.LoadManifests(ctx)
 		if err != nil {
 			return
@@ -188,7 +188,7 @@ func (p *Planner) Plan(ctx context.Context, in planner.Input) (out planner.Outpu
 			return
 		}
 
-		loader := provider.NewManifestLoader(in.ApplicationName, runningDs.AppDir, runningDs.RepoDir, in.GitPath.ConfigFilename, cfg.Input, in.Logger)
+		loader := provider.NewManifestLoader(in.ApplicationName, runningDs.AppDir, runningDs.RepoDir, in.GitPath.ConfigFilename, cfg.Input, in.GitClient, in.Logger)
 		oldManifests, err = loader.LoadManifests(ctx)
 		if err != nil {
 			err = fmt.Errorf("failed to load previously deployed manifests: %w", err)

pkg/app/piped/planner/planner.go

@@ -28,6 +28,7 @@ import (
 	"github.com/pipe-cd/pipe/pkg/app/piped/deploysource"
 	"github.com/pipe-cd/pipe/pkg/cache"
 	"github.com/pipe-cd/pipe/pkg/config"
+	"github.com/pipe-cd/pipe/pkg/git"
 	"github.com/pipe-cd/pipe/pkg/model"
 	"github.com/pipe-cd/pipe/pkg/regexpool"
 )
@@ -36,6 +37,10 @@ type Planner interface {
 	Plan(ctx context.Context, in Input) (Output, error)
 }
 
+type gitClient interface {
+	Clone(ctx context.Context, repoID, remote, branch, destination string) (git.Repo, error)
+}
+
 type Input struct {
 	ApplicationID                  string
 	ApplicationName                string
@@ -47,6 +52,7 @@ type Input struct {
 	RunningDSP                     deploysource.Provider
 	AppManifestsCache              cache.Cache
 	RegexPool                      *regexpool.Pool
+	GitClient                      gitClient
 	Logger                         *zap.Logger
 }
 

pkg/app/piped/planpreview/kubernetesdiff.go

@@ -40,7 +40,7 @@ func (b *builder) kubernetesDiff(
 	var oldManifests, newManifests []provider.Manifest
 	var err error
 
-	newManifests, err = loadKubernetesManifests(ctx, *app, targetDSP, b.appManifestsCache, b.logger)
+	newManifests, err = loadKubernetesManifests(ctx, *app, targetDSP, b.appManifestsCache, b.gitClient, b.logger)
 	if err != nil {
 		fmt.Fprintf(buf, "failed to load kubernetes manifests at the head commit (%v)\n", err)
 		return nil, err
@@ -53,7 +53,7 @@ func (b *builder) kubernetesDiff(
 			*app.GitPath,
 			b.secretDecrypter,
 		)
-		oldManifests, err = loadKubernetesManifests(ctx, *app, runningDSP, b.appManifestsCache, b.logger)
+		oldManifests, err = loadKubernetesManifests(ctx, *app, runningDSP, b.appManifestsCache, b.gitClient, b.logger)
 		if err != nil {
 			fmt.Fprintf(buf, "failed to load kubernetes manifests at the running commit (%v)\n", err)
 			return nil, err
@@ -91,7 +91,7 @@ func (b *builder) kubernetesDiff(
 	}, nil
 }
 
-func loadKubernetesManifests(ctx context.Context, app model.Application, dsp deploysource.Provider, manifestsCache cache.Cache, logger *zap.Logger) (manifests []provider.Manifest, err error) {
+func loadKubernetesManifests(ctx context.Context, app model.Application, dsp deploysource.Provider, manifestsCache cache.Cache, gc gitClient, logger *zap.Logger) (manifests []provider.Manifest, err error) {
 	commit := dsp.Revision()
 	cache := provider.AppManifestsCache{
 		AppID:  app.Id,
@@ -121,6 +121,7 @@ func loadKubernetesManifests(ctx context.Context, app model.Application, dsp dep
 		ds.RepoDir,
 		app.GitPath.ConfigFilename,
 		deployCfg.Input,
+		gc,
 		logger,
 	)
 	manifests, err = loader.LoadManifests(ctx)