Skip to content

Commit

Permalink
Fix Multi Repository Scanning (#510)
Browse files Browse the repository at this point in the history
  • Loading branch information
@EyalDelarea
EyalDelarea authored Sep 20, 2023
1 parent 31d0207 commit e8ac541
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 59 deletions.
2 changes: 1 addition & 1 deletion scanpullrequest/scanpullrequest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ func prepareConfigAndClient(t *testing.T, configPath string, server *httptest.Se

configData, err := utils.ReadConfigFromFileSystem(configPath)
assert.NoError(t, err)
configAggregator, err := utils.BuildRepoAggregator(configData, gitTestParams, &serverParams)
configAggregator, err := utils.BuildRepoAggregator(configData, gitTestParams, &serverParams, utils.ScanPullRequest)
assert.NoError(t, err)

client, err := vcsclient.NewClientBuilder(vcsutils.GitLab).ApiEndpoint(server.URL).Token("123456").Build()
Expand Down
2 changes: 1 addition & 1 deletion scanrepository/scanmultiplerepositories_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestScanAndFixRepos(t *testing.T) {
}()

utils.CreateDotGitWithCommit(t, testDir, port, testRepositories...)
configAggregator, err := utils.BuildRepoAggregator(configData, &gitTestParams, &serverParams)
configAggregator, err := utils.BuildRepoAggregator(configData, &gitTestParams, &serverParams, utils.ScanMultipleRepositories)
assert.NoError(t, err)

var cmd = ScanMultipleRepositories{dryRun: true, dryRunRepoPath: testDir}
Expand Down
4 changes: 2 additions & 2 deletions scanrepository/scanrepository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func TestScanRepositoryCmd_Run(t *testing.T) {
}

utils.CreateDotGitWithCommit(t, testDir, port, test.testName)
configAggregator, err := utils.BuildRepoAggregator(configData, &gitTestParams, &serverParams)
configAggregator, err := utils.BuildRepoAggregator(configData, &gitTestParams, &serverParams, utils.ScanRepository)
assert.NoError(t, err)
// Run
var cmd = ScanRepositoryCmd{dryRun: true, dryRunRepoPath: testDir}
Expand Down Expand Up @@ -289,7 +289,7 @@ pr body
// Load default configurations
var configData []byte
gitTestParams.Branches = []string{"master"}
configAggregator, err := utils.BuildRepoAggregator(configData, gitTestParams, &serverParams)
configAggregator, err := utils.BuildRepoAggregator(configData, gitTestParams, &serverParams, utils.ScanRepository)
assert.NoError(t, err)
// Run
var cmd = ScanRepositoryCmd{dryRun: true, dryRunRepoPath: testDir}
Expand Down
36 changes: 22 additions & 14 deletions utils/params.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ type Params struct {
JFrogPlatform `yaml:"jfrogPlatform,omitempty"`
}

func (p *Params) setDefaultsIfNeeded(gitParamsFromEnv *Git) error {
if err := p.Git.setDefaultsIfNeeded(gitParamsFromEnv); err != nil {
func (p *Params) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) error {
if err := p.Git.setDefaultsIfNeeded(gitParamsFromEnv, commandName); err != nil {
return err
}
if err := p.JFrogPlatform.setDefaultsIfNeeded(); err != nil {
Expand Down Expand Up @@ -235,7 +235,7 @@ type Git struct {
RepositoryCloneUrl string
}

func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git) (err error) {
func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git, commandName string) (err error) {
g.RepoOwner = gitParamsFromEnv.RepoOwner
g.GitProvider = gitParamsFromEnv.GitProvider
g.VcsInfo = gitParamsFromEnv.VcsInfo
Expand All @@ -251,11 +251,19 @@ func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git) (err error) {
g.EmailAuthor = frogbotAuthorEmail
}
}
// When pull request ID is provided, no need to continue and extract unrelated env params.
isPullRequestContext := gitParamsFromEnv.PullRequestDetails.ID != 0
if isPullRequestContext {
return
// Check that PR ID was provided when scanning specific pull request.
if commandName == ScanPullRequest && gitParamsFromEnv.PullRequestDetails.ID == 0 {
return fmt.Errorf("no pull request ID was provided. Please configure it using the 'JF_GIT_PULL_REQUEST_ID' environment variable")
}
if commandName == ScanRepository || commandName == ScanMultipleRepositories {
if err = g.extractScanRepositoryEnvParams(gitParamsFromEnv); err != nil {
return
}
}
return
}

func (g *Git) extractScanRepositoryEnvParams(gitParamsFromEnv *Git) (err error) {
// Continue to extract ScanRepository related env params
noBranchesProvidedViaConfig := len(g.Branches) == 0
noBranchesProvidedViaEnv := len(gitParamsFromEnv.Branches) == 0
Expand Down Expand Up @@ -299,7 +307,7 @@ func GetFrogbotDetails(commandName string) (frogbotDetails *FrogbotDetails, err
if err != nil {
return
}
gitParamsFromEnv, err := extractGitParamsFromEnvs()
gitParamsFromEnv, err := extractGitParamsFromEnvs(commandName)
if err != nil {
return
}
Expand Down Expand Up @@ -337,7 +345,7 @@ func getConfigAggregator(gitClient vcsclient.VcsClient, gitParamsFromEnv *Git, j
if !errors.As(err, &errMissingConfig) && len(configFileContent) == 0 {
return nil, err
}
return BuildRepoAggregator(configFileContent, gitParamsFromEnv, jfrogServer)
return BuildRepoAggregator(configFileContent, gitParamsFromEnv, jfrogServer, commandName)
}

// The getConfigFileContent function retrieves the frogbot-config.yml file content.
Expand All @@ -353,7 +361,7 @@ func getConfigFileContent(gitClient vcsclient.VcsClient, gitParamsFromEnv *Git,

// BuildRepoAggregator receives the content of a frogbot-config.yml file, along with the Git (built from environment variables) and ServerDetails parameters.
// Returns a RepoAggregator instance with all the defaults and necessary fields.
func BuildRepoAggregator(configFileContent []byte, gitParamsFromEnv *Git, server *coreconfig.ServerDetails) (resultAggregator RepoAggregator, err error) {
func BuildRepoAggregator(configFileContent []byte, gitParamsFromEnv *Git, server *coreconfig.ServerDetails, commandName string) (resultAggregator RepoAggregator, err error) {
var cleanAggregator RepoAggregator
// Unmarshal the frogbot-config.yml file if exists
if cleanAggregator, err = unmarshalFrogbotConfigYaml(configFileContent); err != nil {
Expand All @@ -362,7 +370,7 @@ func BuildRepoAggregator(configFileContent []byte, gitParamsFromEnv *Git, server
for _, repository := range cleanAggregator {
repository.Server = *server
repository.OutputWriter = outputwriter.GetCompatibleOutputWriter(gitParamsFromEnv.GitProvider)
if err = repository.Params.setDefaultsIfNeeded(gitParamsFromEnv); err != nil {
if err = repository.Params.setDefaultsIfNeeded(gitParamsFromEnv, commandName); err != nil {
return
}
resultAggregator = append(resultAggregator, repository)
Expand Down Expand Up @@ -412,7 +420,7 @@ func extractJFrogCredentialsFromEnvs() (*coreconfig.ServerDetails, error) {
return &server, nil
}

func extractGitParamsFromEnvs() (*Git, error) {
func extractGitParamsFromEnvs(commandName string) (*Git, error) {
e := &ErrMissingEnv{}
var err error
gitEnvParams := &Git{}
Expand Down Expand Up @@ -447,8 +455,8 @@ func extractGitParamsFromEnvs() (*Git, error) {
return nil, err
}

// [Mandatory] Set the repository name
if err = readParamFromEnv(GitRepoEnv, &gitEnvParams.RepoName); err != nil {
// [Mandatory] Set the repository name, except for multi repository.
if err = readParamFromEnv(GitRepoEnv, &gitEnvParams.RepoName); err != nil && commandName != ScanMultipleRepositories {
return nil, err
}

Expand Down
76 changes: 35 additions & 41 deletions utils/params_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,25 +46,23 @@ func TestExtractParamsFromEnvPlatformScanPullRequest(t *testing.T) {
GitTokenEnv: "123456789",
GitPullRequestIDEnv: "1",
})
extractAndAssertParamsFromEnv(t, true, true)
extractAndAssertParamsFromEnv(t, true, true, ScanPullRequest)
}

// Test extraction in ScanRepository command
// Pull request ID's default is 0, which means we will have branches related variables.
func TestExtractParamsFromEnvPlatformScanRepository(t *testing.T) {
SetEnvAndAssert(t, map[string]string{
JFrogUrlEnv: "http://127.0.0.1:8081",
JFrogUserEnv: "admin",
JFrogPasswordEnv: "password",
GitProvider: string(BitbucketServer),
GitRepoOwnerEnv: "jfrog",
GitRepoEnv: "frogbot",
GitTokenEnv: "123456789",
CommitMessageTemplateEnv: "my-custom-commit-template",
GitBaseBranchEnv: "dev",
GitPullRequestIDEnv: "0",
JFrogUrlEnv: "http://127.0.0.1:8081",
JFrogUserEnv: "admin",
JFrogPasswordEnv: "password",
GitProvider: string(BitbucketServer),
GitRepoOwnerEnv: "jfrog",
GitRepoEnv: "frogbot",
GitTokenEnv: "123456789",
GitBaseBranchEnv: "dev",
})
extractAndAssertParamsFromEnv(t, true, true)
extractAndAssertParamsFromEnv(t, true, true, ScanRepository)
}

func TestExtractParamsFromEnvArtifactoryXray(t *testing.T) {
Expand All @@ -79,25 +77,23 @@ func TestExtractParamsFromEnvArtifactoryXray(t *testing.T) {
GitRepoEnv: "frogbot",
GitTokenEnv: "123456789",
GitBaseBranchEnv: "dev",
GitPullRequestIDEnv: "1",
})
extractAndAssertParamsFromEnv(t, false, true)
extractAndAssertParamsFromEnv(t, false, true, ScanRepository)
}

func TestExtractParamsFromEnvToken(t *testing.T) {
SetEnvAndAssert(t, map[string]string{
JFrogUrlEnv: "http://127.0.0.1:8081",
JFrogUserEnv: "",
JFrogPasswordEnv: "",
JFrogTokenEnv: "token",
GitProvider: string(BitbucketServer),
GitRepoOwnerEnv: "jfrog",
GitRepoEnv: "frogbot",
GitTokenEnv: "123456789",
GitBaseBranchEnv: "dev",
GitPullRequestIDEnv: "1",
JFrogUrlEnv: "http://127.0.0.1:8081",
JFrogUserEnv: "",
JFrogPasswordEnv: "",
JFrogTokenEnv: "token",
GitProvider: string(BitbucketServer),
GitRepoOwnerEnv: "jfrog",
GitRepoEnv: "frogbot",
GitTokenEnv: "123456789",
GitBaseBranchEnv: "dev",
})
extractAndAssertParamsFromEnv(t, true, false)
extractAndAssertParamsFromEnv(t, true, false, ScanRepository)
}

func TestExtractVcsProviderFromEnv(t *testing.T) {
Expand Down Expand Up @@ -133,19 +129,19 @@ func TestExtractClientInfo(t *testing.T) {
assert.NoError(t, SanitizeEnv())
}()

_, err := extractGitParamsFromEnvs()
_, err := extractGitParamsFromEnvs(ScanRepository)
assert.EqualError(t, err, "JF_GIT_PROVIDER should be one of: 'github', 'gitlab', 'bitbucketServer' or 'azureRepos'")

SetEnvAndAssert(t, map[string]string{GitProvider: "github"})
_, err = extractGitParamsFromEnvs()
_, err = extractGitParamsFromEnvs(ScanRepository)
assert.EqualError(t, err, "'JF_GIT_OWNER' environment variable is missing")

SetEnvAndAssert(t, map[string]string{GitRepoOwnerEnv: "jfrog"})
_, err = extractGitParamsFromEnvs()
_, err = extractGitParamsFromEnvs(ScanRepository)
assert.EqualError(t, err, "'JF_GIT_TOKEN' environment variable is missing")

SetEnvAndAssert(t, map[string]string{GitTokenEnv: "token"})
_, err = extractGitParamsFromEnvs()
_, err = extractGitParamsFromEnvs(ScanRepository)
assert.EqualError(t, err, "'JF_GIT_REPO' environment variable is missing")
}

Expand Down Expand Up @@ -173,11 +169,11 @@ func TestExtractAndAssertRepoParams(t *testing.T) {

server, err := extractJFrogCredentialsFromEnvs()
assert.NoError(t, err)
gitParams, err := extractGitParamsFromEnvs()
gitParams, err := extractGitParamsFromEnvs(ScanRepository)
assert.NoError(t, err)
configFileContent, err := ReadConfigFromFileSystem(configParamsTestFile)
assert.NoError(t, err)
configAggregator, err := BuildRepoAggregator(configFileContent, gitParams, server)
configAggregator, err := BuildRepoAggregator(configFileContent, gitParams, server, ScanRepository)
assert.NoError(t, err)
for _, repo := range configAggregator {
for projectI, project := range repo.Projects {
Expand Down Expand Up @@ -217,11 +213,11 @@ func TestBuildRepoAggregatorWithEmptyScan(t *testing.T) {
}()
server, err := extractJFrogCredentialsFromEnvs()
assert.NoError(t, err)
gitParams, err := extractGitParamsFromEnvs()
gitParams, err := extractGitParamsFromEnvs(ScanRepository)
assert.NoError(t, err)
configFileContent, err := ReadConfigFromFileSystem(configEmptyScanParamsTestFile)
assert.NoError(t, err)
configAggregator, err := BuildRepoAggregator(configFileContent, gitParams, server)
configAggregator, err := BuildRepoAggregator(configFileContent, gitParams, server, ScanRepository)
assert.NoError(t, err)
assert.Len(t, configAggregator, 1)
assert.Equal(t, frogbotAuthorEmail, configAggregator[0].EmailAuthor)
Expand Down Expand Up @@ -250,12 +246,12 @@ func testExtractAndAssertProjectParams(t *testing.T, project Project) {
assert.Equal(t, "", project.PipRequirementsFile)
}

func extractAndAssertParamsFromEnv(t *testing.T, platformUrl, basicAuth bool) {
func extractAndAssertParamsFromEnv(t *testing.T, platformUrl, basicAuth bool, commandName string) {
server, err := extractJFrogCredentialsFromEnvs()
assert.NoError(t, err)
gitParams, err := extractGitParamsFromEnvs()
gitParams, err := extractGitParamsFromEnvs(commandName)
assert.NoError(t, err)
configFile, err := BuildRepoAggregator(nil, gitParams, server)
configFile, err := BuildRepoAggregator(nil, gitParams, server, commandName)
assert.NoError(t, err)
err = SanitizeEnv()
assert.NoError(t, err)
Expand All @@ -279,10 +275,9 @@ func extractAndAssertParamsFromEnv(t *testing.T, platformUrl, basicAuth bool) {
assert.Equal(t, "frogbot", configParams.RepoName)
assert.Equal(t, "123456789", configParams.Token)
// ScanRepository command context
if len(configParams.Branches) != 0 {
if commandName == ScanRepository {
assert.Equal(t, "dev", configParams.Branches[0])
assert.Equal(t, int64(0), configParams.PullRequestDetails.ID)
assert.Equal(t, "my-custom-commit-template", configParams.Git.CommitMessageTemplate)
} else {
// ScanPullRequest context
assert.Equal(t, int64(1), configParams.PullRequestDetails.ID)
Expand Down Expand Up @@ -345,7 +340,6 @@ func TestGenerateConfigAggregatorFromEnv(t *testing.T) {
MinSeverityEnv: "medium",
FixableOnlyEnv: "true",
AllowedLicensesEnv: "MIT, Apache-2.0",
GitPullRequestIDEnv: "0",
})
defer func() {
assert.NoError(t, SanitizeEnv())
Expand All @@ -367,7 +361,7 @@ func TestGenerateConfigAggregatorFromEnv(t *testing.T) {
User: "admin",
Password: "password",
}
repoAggregator, err := BuildRepoAggregator(nil, &gitParams, &server)
repoAggregator, err := BuildRepoAggregator(nil, &gitParams, &server, ScanRepository)
assert.NoError(t, err)
repo := repoAggregator[0]
assert.Equal(t, "repoName", repo.RepoName)
Expand Down Expand Up @@ -518,7 +512,7 @@ func TestBuildMergedRepoAggregator(t *testing.T) {
User: "admin",
Password: "password",
}
repoAggregator, err := BuildRepoAggregator(fileContent, gitParams, &server)
repoAggregator, err := BuildRepoAggregator(fileContent, gitParams, &server, ScanRepository)
assert.NoError(t, err)

repo := repoAggregator[0]
Expand Down

0 comments on commit e8ac541

Please sign in to comment.