Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Multi Repository Scanning #510

Merged
merged 3 commits into from
Sep 20, 2023
Merged

Fix Multi Repository Scanning #510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
60698df
Fix Mandatory git repo param for multi repo scanning
EyalDelarea Sep 20, 2023
2d9bf41
pass command name to tests
EyalDelarea Sep 20, 2023
6e8a199
typo
EyalDelarea Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scanpullrequest/scanpullrequest_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed template check as this is tested in other function and this is not generic to the test

} 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
Loading