refactor(orchestrator): Use singleton in orchestrator package and ren…

…ame methods (#4639) * rename interface, types and methods. some type changes and refactor * update dependent methods and variables * fix unit tests * a bit more refactor and fix * concurrent safe singleton * return old Options struct * refactor creating config provider and fix nil pointer derefernce * fix unit test and linter errors * introduce resetting config provider (for unit tests) * fix annoying error message when config provider is not configured --------- Co-authored-by: Gulom Alimov <[email protected]> Co-authored-by: Muhammadali Nazarov <[email protected]>
SAP · Jan 9, 2024 · ac5cf17 · ac5cf17
1 parent a5ea24d
commit ac5cf17
Show file tree

Hide file tree

Showing 27 changed files with 466 additions and 522 deletions.
diff --git a/.gitignore b/.gitignore
@@ -32,6 +32,7 @@ debug.test
 /piper_master.exe
 /jenkins-library
 /jenkins-library.exe
+node_modules/
 
 # piper binary outputs
 .pipeline/commonPipelineEnvironment/

diff --git a/cmd/artifactPrepareVersion.go b/cmd/artifactPrepareVersion.go
@@ -66,7 +66,7 @@ type artifactPrepareVersionUtils interface {
 	FileRead(path string) ([]byte, error)
 	FileRemove(path string) error
 
-	NewOrchestratorSpecificConfigProvider() (orchestrator.OrchestratorSpecificConfigProviding, error)
+	GetConfigProvider() (orchestrator.ConfigProvider, error)
 }
 
 type artifactPrepareVersionUtilsBundle struct {
@@ -75,8 +75,8 @@ type artifactPrepareVersionUtilsBundle struct {
 	*piperhttp.Client
 }
 
-func (a *artifactPrepareVersionUtilsBundle) NewOrchestratorSpecificConfigProvider() (orchestrator.OrchestratorSpecificConfigProviding, error) {
-	return orchestrator.NewOrchestratorSpecificConfigProvider()
+func (a *artifactPrepareVersionUtilsBundle) GetConfigProvider() (orchestrator.ConfigProvider, error) {
+	return orchestrator.GetOrchestratorConfigProvider(nil)
 }
 
 func newArtifactPrepareVersionUtilsBundle() artifactPrepareVersionUtils {
@@ -160,7 +160,7 @@ func runArtifactPrepareVersion(config *artifactPrepareVersionOptions, telemetryD
 	if config.VersioningType == "cloud" || config.VersioningType == "cloud_noTag" {
 		// make sure that versioning does not create tags (when set to "cloud")
 		// for PR pipelines, optimized pipelines (= no build)
-		provider, err := utils.NewOrchestratorSpecificConfigProvider()
+		provider, err := utils.GetConfigProvider()
 		if err != nil {
 			log.Entry().WithError(err).Warning("Cannot infer config from CI environment")
 		}

diff --git a/cmd/artifactPrepareVersion_test.go b/cmd/artifactPrepareVersion_test.go
@@ -192,7 +192,7 @@ func (a *artifactPrepareVersionMockUtils) DownloadFile(url, filename string, hea
 	return nil
 }
 
-func (a *artifactPrepareVersionMockUtils) NewOrchestratorSpecificConfigProvider() (orchestrator.OrchestratorSpecificConfigProviding, error) {
+func (a *artifactPrepareVersionMockUtils) GetConfigProvider() (orchestrator.ConfigProvider, error) {
 	return &orchestrator.UnknownOrchestratorConfigProvider{}, nil
 }
 

diff --git a/cmd/codeqlExecuteScan.go b/cmd/codeqlExecuteScan.go
@@ -130,20 +130,20 @@ func initGitInfo(config *codeqlExecuteScanOptions) (codeql.RepoInfo, error) {
 	repoInfo.Ref = config.AnalyzedRef
 	repoInfo.CommitId = config.CommitID
 
-	provider, err := orchestrator.NewOrchestratorSpecificConfigProvider()
+	provider, err := orchestrator.GetOrchestratorConfigProvider(nil)
 	if err != nil {
 		log.Entry().Warn("No orchestrator found. We assume piper is running locally.")
 	} else {
 		if repoInfo.Ref == "" {
-			repoInfo.Ref = provider.GetReference()
+			repoInfo.Ref = provider.GitReference()
 		}
 
 		if repoInfo.CommitId == "" || repoInfo.CommitId == "NA" {
-			repoInfo.CommitId = provider.GetCommit()
+			repoInfo.CommitId = provider.CommitSHA()
 		}
 
 		if repoInfo.ServerUrl == "" {
-			err = getGitRepoInfo(provider.GetRepoURL(), &repoInfo)
+			err = getGitRepoInfo(provider.RepoURL(), &repoInfo)
 			if err != nil {
 				log.Entry().Error(err)
 			}

diff --git a/cmd/codeqlExecuteScan_test.go b/cmd/codeqlExecuteScan_test.go
@@ -249,7 +249,7 @@ func TestInitGitInfo(t *testing.T) {
 		config := codeqlExecuteScanOptions{Repository: "https://github.hello.test", AnalyzedRef: "refs/head/branch", CommitID: "abcd1234"}
 		repoInfo, err := initGitInfo(&config)
 		assert.NoError(t, err)
-		_, err = orchestrator.NewOrchestratorSpecificConfigProvider()
+		_, err = orchestrator.GetOrchestratorConfigProvider(nil)
 		assert.Equal(t, "abcd1234", repoInfo.CommitId)
 		assert.Equal(t, "refs/head/branch", repoInfo.Ref)
 		if err != nil {

diff --git a/cmd/credentialdiggerScan.go b/cmd/credentialdiggerScan.go
@@ -42,14 +42,14 @@ func newCDUtils() credentialdiggerUtils {
 func credentialdiggerScan(config credentialdiggerScanOptions, telemetryData *telemetry.CustomData) error {
 	utils := newCDUtils()
 	// 0: Get attributes from orchestrator
-	provider, prov_err := orchestrator.NewOrchestratorSpecificConfigProvider()
+	provider, prov_err := orchestrator.GetOrchestratorConfigProvider(nil)
 	if prov_err != nil {
 		log.Entry().WithError(prov_err).Error(
 			"credentialdiggerScan: unable to load orchestrator specific configuration.")
 	}
 	if config.Repository == "" {
 		// Get current repository from orchestrator
-		repoUrlOrchestrator := provider.GetRepoURL()
+		repoUrlOrchestrator := provider.RepoURL()
 		if repoUrlOrchestrator == "n/a" {
 			// Jenkins configuration error
 			log.Entry().WithError(errors.New(
@@ -61,7 +61,7 @@ func credentialdiggerScan(config credentialdiggerScanOptions, telemetryData *tel
 	}
 	if provider.IsPullRequest() {
 		// set the pr number
-		config.PrNumber, _ = strconv.Atoi(provider.GetPullRequestConfig().Key)
+		config.PrNumber, _ = strconv.Atoi(provider.PullRequestConfig().Key)
 		log.Entry().Debug("Scan the current pull request: number ", config.PrNumber)
 	}
 

diff --git a/cmd/detectExecuteScan.go b/cmd/detectExecuteScan.go
@@ -46,7 +46,7 @@ type detectUtils interface {
 
 	GetIssueService() *github.IssuesService
 	GetSearchService() *github.SearchService
-	GetProvider() orchestrator.OrchestratorSpecificConfigProviding
+	GetProvider() orchestrator.ConfigProvider
 }
 
 type detectUtilsBundle struct {
@@ -55,7 +55,7 @@ type detectUtilsBundle struct {
 	*piperhttp.Client
 	issues   *github.IssuesService
 	search   *github.SearchService
-	provider orchestrator.OrchestratorSpecificConfigProviding
+	provider orchestrator.ConfigProvider
 }
 
 func (d *detectUtilsBundle) GetIssueService() *github.IssuesService {
@@ -66,7 +66,7 @@ func (d *detectUtilsBundle) GetSearchService() *github.SearchService {
 	return d.search
 }
 
-func (d *detectUtilsBundle) GetProvider() orchestrator.OrchestratorSpecificConfigProviding {
+func (d *detectUtilsBundle) GetProvider() orchestrator.ConfigProvider {
 	return d.provider
 }
 
@@ -112,7 +112,7 @@ func newDetectUtils(client *github.Client) detectUtils {
 	utils.Stdout(log.Writer())
 	utils.Stderr(log.Writer())
 
-	provider, err := orchestrator.NewOrchestratorSpecificConfigProvider()
+	provider, err := orchestrator.GetOrchestratorConfigProvider(nil)
 	if err != nil {
 		log.Entry().WithError(err).Warning(err)
 		provider = &orchestrator.UnknownOrchestratorConfigProvider{}
@@ -568,9 +568,9 @@ func isMajorVulnerability(v bd.Vulnerability) bool {
 }
 
 func postScanChecksAndReporting(ctx context.Context, config detectExecuteScanOptions, influx *detectExecuteScanInflux, utils detectUtils, sys *blackduckSystem) error {
-
-	if utils.GetProvider().IsPullRequest() {
-		issueNumber, err := strconv.Atoi(utils.GetProvider().GetPullRequestConfig().Key)
+	provider := utils.GetProvider()
+	if provider.IsPullRequest() {
+		issueNumber, err := strconv.Atoi(provider.PullRequestConfig().Key)
 		if err != nil {
 			log.Entry().Warning("Can not get issue number ", err)
 			return nil

diff --git a/cmd/detectExecuteScan_test.go b/cmd/detectExecuteScan_test.go
@@ -33,7 +33,7 @@ type detectTestUtilsBundle struct {
 	orchestrator *orchestratorConfigProviderMock
 }
 
-func (d *detectTestUtilsBundle) GetProvider() orchestrator.OrchestratorSpecificConfigProviding {
+func (d *detectTestUtilsBundle) GetProvider() orchestrator.ConfigProvider {
 	return d.orchestrator
 }
 

diff --git a/cmd/piper.go b/cmd/piper.go
@@ -212,16 +212,13 @@ func Execute() {
 }
 
 func addRootFlags(rootCmd *cobra.Command) {
-	var provider orchestrator.OrchestratorSpecificConfigProviding
-	var err error
-
-	provider, err = orchestrator.NewOrchestratorSpecificConfigProvider()
+	provider, err := orchestrator.GetOrchestratorConfigProvider(nil)
 	if err != nil {
 		log.Entry().Error(err)
 		provider = &orchestrator.UnknownOrchestratorConfigProvider{}
 	}
 
-	rootCmd.PersistentFlags().StringVar(&GeneralConfig.CorrelationID, "correlationID", provider.GetBuildURL(), "ID for unique identification of a pipeline run")
+	rootCmd.PersistentFlags().StringVar(&GeneralConfig.CorrelationID, "correlationID", provider.BuildURL(), "ID for unique identification of a pipeline run")
 	rootCmd.PersistentFlags().StringVar(&GeneralConfig.CustomConfig, "customConfig", ".pipeline/config.yml", "Path to the pipeline configuration file")
 	rootCmd.PersistentFlags().StringSliceVar(&GeneralConfig.GitHubTokens, "gitHubTokens", AccessTokensFromEnvJSON(os.Getenv("PIPER_gitHubTokens")), "List of entries in form of <hostname>:<token> to allow GitHub token authentication for downloading config / defaults")
 	rootCmd.PersistentFlags().StringSliceVar(&GeneralConfig.DefaultConfig, "defaultConfig", []string{".pipeline/defaults.yaml"}, "Default configurations, passed as path to yaml file")
@@ -290,12 +287,12 @@ func initStageName(outputToLog bool) {
 	}
 
 	// Use stageName from ENV as fall-back, for when extracting it from parametersJSON fails below
-	provider, err := orchestrator.NewOrchestratorSpecificConfigProvider()
+	provider, err := orchestrator.GetOrchestratorConfigProvider(nil)
 	if err != nil {
 		log.Entry().WithError(err).Warning("Cannot infer stage name from CI environment")
 	} else {
 		stageNameSource = "env variable"
-		GeneralConfig.StageName = provider.GetStageName()
+		GeneralConfig.StageName = provider.StageName()
 	}
 
 	if len(GeneralConfig.ParametersJSON) == 0 {

diff --git a/cmd/piper_test.go b/cmd/piper_test.go
@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"github.com/SAP/jenkins-library/pkg/orchestrator"
 	"os"
 	"path/filepath"
 	"strings"
@@ -62,6 +63,7 @@ func TestAdoptStageNameFromParametersJSON(t *testing.T) {
 			// init
 			defer resetEnv(os.Environ())
 			os.Clearenv()
+			orchestrator.ResetConfigProvider()
 
 			//mock Jenkins env
 			os.Setenv("JENKINS_HOME", "anything")

diff --git a/cmd/sonarExecuteScan.go b/cmd/sonarExecuteScan.go
@@ -477,14 +477,14 @@ func getTempDir() string {
 
 // Fetches parameters from environment variables and updates the options accordingly (only if not already set)
 func detectParametersFromCI(options *sonarExecuteScanOptions) {
-	provider, err := orchestrator.NewOrchestratorSpecificConfigProvider()
+	provider, err := orchestrator.GetOrchestratorConfigProvider(nil)
 	if err != nil {
 		log.Entry().WithError(err).Warning("Cannot infer config from CI environment")
 		return
 	}
 
 	if provider.IsPullRequest() {
-		config := provider.GetPullRequestConfig()
+		config := provider.PullRequestConfig()
 		if len(options.ChangeBranch) == 0 {
 			log.Entry().Info("Inferring parameter changeBranch from environment: " + config.Branch)
 			options.ChangeBranch = config.Branch
@@ -498,7 +498,7 @@ func detectParametersFromCI(options *sonarExecuteScanOptions) {
 			options.ChangeID = config.Key
 		}
 	} else {
-		branch := provider.GetBranch()
+		branch := provider.Branch()
 		if options.InferBranchName && len(options.BranchName) == 0 {
 			log.Entry().Info("Inferring parameter branchName from environment: " + branch)
 			options.BranchName = branch