From cf07082371c04ba1f051947623b1e9f3052cf32e Mon Sep 17 00:00:00 2001 From: asafambar Date: Wed, 13 Sep 2023 00:29:26 +0300 Subject: [PATCH 1/8] Add support to calculate npm tree only by package-lock.json file --- build/npm.go | 3 ++- build/utils/npm.go | 51 +++++++++++++++++++++++++++++++---------- build/utils/npm_test.go | 12 +++++----- 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/build/npm.go b/build/npm.go index 66d52e04..10d658f3 100644 --- a/build/npm.go +++ b/build/npm.go @@ -74,7 +74,8 @@ func (nm *NpmModule) CalcDependencies() error { if !nm.containingBuild.buildNameAndNumberProvided() { return errors.New("a build name must be provided in order to collect the project's dependencies") } - buildInfoDependencies, err := buildutils.CalculateNpmDependenciesList(nm.executablePath, nm.srcPath, nm.name, nm.npmArgs, true, nm.containingBuild.logger) + buildInfoDependencies, err := buildutils.CalculateNpmDependenciesList(nm.executablePath, nm.srcPath, nm.name, + buildutils.NpmTreeDepListParam{Args: nm.npmArgs}, true, nm.containingBuild.logger) if err != nil { return err } diff --git a/build/utils/npm.go b/build/utils/npm.go index 2d43e1e2..ee0f7444 100644 --- a/build/utils/npm.go +++ b/build/utils/npm.go @@ -17,19 +17,19 @@ import ( ) // CalculateNpmDependenciesList gets an npm project's dependencies. -func CalculateNpmDependenciesList(executablePath, srcPath, moduleId string, npmArgs []string, calculateChecksums bool, log utils.Log) ([]entities.Dependency, error) { +func CalculateNpmDependenciesList(executablePath, srcPath, moduleId string, npmParams NpmTreeDepListParam, calculateChecksums bool, log utils.Log) ([]entities.Dependency, error) { if log == nil { log = &utils.NullLog{} } // Calculate npm dependency tree using 'npm ls...'. - dependenciesMap, err := CalculateDependenciesMap(executablePath, srcPath, moduleId, npmArgs, log) + dependenciesMap, err := CalculateDependenciesMap(executablePath, srcPath, moduleId, npmParams, log) if err != nil { return nil, err } var cacache *cacache if calculateChecksums { // Get local npm cache. - cacheLocation, err := GetNpmConfigCache(srcPath, executablePath, npmArgs, log) + cacheLocation, err := GetNpmConfigCache(srcPath, executablePath, npmParams.Args, log) if err != nil { return nil, err } @@ -87,7 +87,7 @@ type dependencyInfo struct { // Run 'npm list ...' command and parse the returned result to create a dependencies map of. // The dependencies map looks like name:version -> entities.Dependency. -func CalculateDependenciesMap(executablePath, srcPath, moduleId string, npmArgs []string, log utils.Log) (map[string]*dependencyInfo, error) { +func CalculateDependenciesMap(executablePath, srcPath, moduleId string, npmListParams NpmTreeDepListParam, log utils.Log) (map[string]*dependencyInfo, error) { dependenciesMap := make(map[string]*dependencyInfo) // These arguments must be added at the end of the command, to override their other values (if existed in nm.npmArgs). npmVersion, err := GetNpmVersion(executablePath, log) @@ -100,10 +100,10 @@ func CalculateDependenciesMap(executablePath, srcPath, moduleId string, npmArgs } var data []byte // If we don't have node_modules, the function will use the package-lock dependencies. - if nodeModulesExist { - data = runNpmLsWithNodeModules(executablePath, srcPath, npmArgs, log) + if nodeModulesExist && !npmListParams.IgnoreNodeModules { + data = runNpmLsWithNodeModules(executablePath, srcPath, npmListParams.Args, log) } else { - data, err = runNpmLsWithoutNodeModules(executablePath, srcPath, npmArgs, log, npmVersion) + data, err = runNpmLsWithoutNodeModules(executablePath, srcPath, npmListParams, log, npmVersion) if err != nil { return nil, err } @@ -130,19 +130,19 @@ func runNpmLsWithNodeModules(executablePath, srcPath string, npmArgs []string, l return } -func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmArgs []string, log utils.Log, npmVersion *version.Version) ([]byte, error) { +func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams NpmTreeDepListParam, log utils.Log, npmVersion *version.Version) ([]byte, error) { isPackageLockExist, isDirExistsErr := utils.IsFileExists(filepath.Join(srcPath, "package-lock.json"), false) if isDirExistsErr != nil { return nil, isDirExistsErr } - if !isPackageLockExist { - err := installPackageLock(executablePath, srcPath, npmArgs, log, npmVersion) + if !isPackageLockExist || (npmListParams.OverWritePackageLock && checkIfLockFileShouldBeUpdated(srcPath, log)) { + err := installPackageLock(executablePath, srcPath, npmListParams.Args, log, npmVersion) if err != nil { return nil, err } } - npmArgs = append(npmArgs, "--json", "--all", "--long", "--package-lock-only") - data, errData, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmArgs, "ls"), log) + npmListParams.Args = append(npmListParams.Args, "--json", "--all", "--long", "--package-lock-only") + data, errData, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmListParams.Args, "ls"), log) if err != nil { log.Warn(err.Error()) } else if len(errData) > 0 { @@ -164,6 +164,25 @@ func installPackageLock(executablePath, srcPath string, npmArgs []string, log ut return errors.New("it looks like you’re using version " + npmVersion.GetVersion() + " of the npm client. Versions below 6.0.0 require running `npm install` before running this command") } +// check package.json modified, +// this can hint us new packages added to package.json which were not updated in package-lock.json.. +func checkIfLockFileShouldBeUpdated(srcPath string, log utils.Log) bool { + packageJsonInfo, err := os.Stat(filepath.Join(srcPath, "package.json")) + if err != nil { + log.Warn("Failed to get file info for package.json, err: %v", err) + return false + } + + packageJsonInfoModTime := packageJsonInfo.ModTime() + packageLockInfo, err := os.Stat(filepath.Join(srcPath, "package-lock.json")) + if err != nil { + log.Warn("Failed to get file info for package-lock.json, err: %v", err) + return false + } + packageLockInfoModTime := packageLockInfo.ModTime() + return packageJsonInfoModTime.After(packageLockInfoModTime) +} + func GetNpmVersion(executablePath string, log utils.Log) (*version.Version, error) { versionData, _, err := RunNpmCmd(executablePath, "", []string{"--version"}, log) if err != nil { @@ -172,6 +191,14 @@ func GetNpmVersion(executablePath string, log utils.Log) (*version.Version, erro return version.NewVersion(string(versionData)), nil } +type NpmTreeDepListParam struct { + Args []string + // ignore node modules folder if exists. + IgnoreNodeModules bool + // rewrite package lock json if exists. + OverWritePackageLock bool +} + // npm >=7 ls results for a single dependency type npmLsDependency struct { Name string diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index fc995b30..83178e28 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -192,7 +192,7 @@ func TestDependencyWithNoIntegrity(t *testing.T) { assert.NoError(t, err) // Calculate dependencies. - dependencies, err := CalculateNpmDependenciesList("npm", projectPath, "jfrogtest", npmArgs, true, logger) + dependencies, err := CalculateNpmDependenciesList("npm", projectPath, "jfrogtest", NpmTreeDepListParam{Args: npmArgs}, true, logger) assert.NoError(t, err) assert.Greaterf(t, len(dependencies), 0, "Error: dependencies are not found!") @@ -214,7 +214,7 @@ func TestDependenciesTreeDifferentBetweenOKs(t *testing.T) { assert.NoError(t, err) // Calculate dependencies. - dependencies, err := CalculateNpmDependenciesList("npm", projectPath, "bundle-dependencies", npmArgs, true, logger) + dependencies, err := CalculateNpmDependenciesList("npm", projectPath, "bundle-dependencies", NpmTreeDepListParam{Args: npmArgs}, true, logger) assert.NoError(t, err) assert.Greaterf(t, len(dependencies), 0, "Error: dependencies are not found!") @@ -222,7 +222,7 @@ func TestDependenciesTreeDifferentBetweenOKs(t *testing.T) { // Remove node_modules directory, then calculate dependencies by package-lock. assert.NoError(t, utils.RemoveTempDir(filepath.Join(projectPath, "node_modules"))) - dependencies, err = CalculateNpmDependenciesList("npm", projectPath, "build-info-go-tests", npmArgs, true, logger) + dependencies, err = CalculateNpmDependenciesList("npm", projectPath, "build-info-go-tests", NpmTreeDepListParam{Args: npmArgs}, true, logger) assert.NoError(t, err) // Asserting there is at least one dependency. @@ -253,7 +253,7 @@ func TestNpmProdFlag(t *testing.T) { assert.NoError(t, err) // Calculate dependencies with scope. - dependencies, err := CalculateNpmDependenciesList("npm", projectPath, "build-info-go-tests", npmArgs, true, logger) + dependencies, err := CalculateNpmDependenciesList("npm", projectPath, "build-info-go-tests", NpmTreeDepListParam{Args: npmArgs}, true, logger) assert.NoError(t, err) assert.Len(t, dependencies, entry.totalDeps) }() @@ -302,7 +302,7 @@ func validateDependencies(t *testing.T, projectPath string, npmArgs []string) { assert.NoError(t, err) // Calculate dependencies. - dependencies, err := CalculateNpmDependenciesList("npm", projectPath, "build-info-go-tests", npmArgs, true, logger) + dependencies, err := CalculateNpmDependenciesList("npm", projectPath, "build-info-go-tests", NpmTreeDepListParam{Args: npmArgs}, true, logger) assert.NoError(t, err) assert.Greaterf(t, len(dependencies), 0, "Error: dependencies are not found!") @@ -310,7 +310,7 @@ func validateDependencies(t *testing.T, projectPath string, npmArgs []string) { // Remove node_modules directory, then calculate dependencies by package-lock. assert.NoError(t, utils.RemoveTempDir(filepath.Join(projectPath, "node_modules"))) - dependencies, err = CalculateNpmDependenciesList("npm", projectPath, "build-info-go-tests", npmArgs, true, logger) + dependencies, err = CalculateNpmDependenciesList("npm", projectPath, "build-info-go-tests", NpmTreeDepListParam{Args: npmArgs}, true, logger) assert.NoError(t, err) // Asserting there is at least one dependency. From 87d5bff8b949dc0ac1ce43091cb5877d4939988a Mon Sep 17 00:00:00 2001 From: asafambar Date: Wed, 13 Sep 2023 16:54:43 +0300 Subject: [PATCH 2/8] add test --- .../npm/project6/node_modules/dummy-file | 0 .../npm/project6/package-lock_test.json | 36 +++++++++ build/testdata/npm/project6/package.json | 17 ++++ build/utils/npm_test.go | 77 +++++++++++++++++++ 4 files changed, 130 insertions(+) create mode 100644 build/testdata/npm/project6/node_modules/dummy-file create mode 100644 build/testdata/npm/project6/package-lock_test.json create mode 100644 build/testdata/npm/project6/package.json diff --git a/build/testdata/npm/project6/node_modules/dummy-file b/build/testdata/npm/project6/node_modules/dummy-file new file mode 100644 index 00000000..e69de29b diff --git a/build/testdata/npm/project6/package-lock_test.json b/build/testdata/npm/project6/package-lock_test.json new file mode 100644 index 00000000..28c7bf77 --- /dev/null +++ b/build/testdata/npm/project6/package-lock_test.json @@ -0,0 +1,36 @@ +{ + "name": "project6", + "version": "1.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "project6", + "version": "1.0.0", + "license": "ISC", + "dependencies": { + "lightweight": "^0.1.0", + "minimist": "^0.1.0", + "underscore": "^1.13.6" + } + }, + "node_modules/lightweight": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/lightweight/-/lightweight-0.1.0.tgz", + "integrity": "sha512-10pYSQA9EJqZZnXDR0urhg8Z0Y1XnRfi41ZFj3ZFTKJ5PjRq82HzT7LKlPyxewy3w2WA2POfi3jQQn7Y53oPcQ==", + "bin": { + "lwt": "bin/lwt.js" + } + }, + "node_modules/minimist": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.1.0.tgz", + "integrity": "sha512-wR5Ipl99t0mTGwLjQJnBjrP/O7zBbLZqvA3aw32DmLx+nXHfWctUjzDjnDx09pX1Po86WFQazF9xUzfMea3Cnw==" + }, + "node_modules/underscore": { + "version": "1.13.6", + "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.13.6.tgz", + "integrity": "sha512-+A5Sja4HP1M08MaXya7p5LvjuM7K6q/2EaC0+iovj/wOcMsTzMvDFbasi/oSapiwOlt252IqsKqPjCl7huKS0A==" + } + } +} diff --git a/build/testdata/npm/project6/package.json b/build/testdata/npm/project6/package.json new file mode 100644 index 00000000..9a4a6cbb --- /dev/null +++ b/build/testdata/npm/project6/package.json @@ -0,0 +1,17 @@ +{ + "name": "npm_test2", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "lightweight": "^0.1.0", + "minimist": "^0.1.0", + "underscore": "^1.13.6", + "cors.js": "0.0.1-security" + } +} diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index 83178e28..7727f8c2 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -1,10 +1,14 @@ package utils import ( + "github.com/jfrog/build-info-go/entities" + "github.com/stretchr/testify/require" "os" "path/filepath" "reflect" + "sort" "testing" + "time" testdatautils "github.com/jfrog/build-info-go/build/testdata" "github.com/jfrog/build-info-go/utils" @@ -198,6 +202,79 @@ func TestDependencyWithNoIntegrity(t *testing.T) { assert.Greaterf(t, len(dependencies), 0, "Error: dependencies are not found!") } +// This test case check that CalculateNpmDependenciesList ignore node_modules and update package-lock.json when needed, +// this according to the params 'IgnoreNodeModules' and 'OverWritePackageLock'. +func TestDependencyPackageLockOnly(t *testing.T) { + path, err := filepath.Abs(filepath.Join("..", "testdata/npm/project6")) + assert.NoError(t, err) + data, err := os.ReadFile(filepath.Join(path, "package-lock_test.json")) + require.NoError(t, err) + info, err := os.Stat(filepath.Join(path, "package-lock_test.json")) + require.NoError(t, err) + os.WriteFile(filepath.Join(path, "package-lock.json"), data, info.Mode().Perm()) + defer func() { + assert.NoError(t, os.Remove(filepath.Join(path, "package-lock.json"))) + assert.NoError(t, os.Remove(filepath.Join(path, "node_modules", ".package-lock.json"))) + }() + // sleep so the package.json modified time will be bigger than the package-lock.json, this make sure it will recalculate lock file. + time.Sleep(time.Millisecond * 5) + require.NoError(t, os.Chtimes(filepath.Join(path, "package.json"), time.Now(), time.Now())) + + // Calculate dependencies. + dependencies, err := CalculateNpmDependenciesList("npm", path, "jfrogtest", + NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: true, OverWritePackageLock: true}, true, logger) + assert.NoError(t, err) + expectedDeps := []entities.Dependency{{ + Id: "underscore:1.13.6", + Scopes: []string{"prod"}, + RequestedBy: [][]string{{"jfrogtest"}}, + Checksum: entities.Checksum{ + Sha1: "04786a1f589dc6c09f761fc5f45b89e935136441", + Md5: "945e1ea169a281c296b82ad2dd5466f6", + Sha256: "aef5a43ac7f903136a93e75a274e3a7b50de1a92277e1666457cabf62eeb0140", + }, + }, + { + Id: "cors.js:0.0.1-security", + Scopes: []string{"prod"}, + RequestedBy: [][]string{{"jfrogtest"}}, + Checksum: entities.Checksum{ + Sha1: "a1304531e44d11f4406b424b8377c3f3f1d3a934", + Md5: "f798d8a0d5e59e0d1b10a8fdc7660df0", + Sha256: "e2352450325dba7f38c45ec43ca77eab2cdba66fdb232061045e7039ada1da7e", + }, + }, + { + Id: "lightweight:0.1.0", + Scopes: []string{"prod"}, + RequestedBy: [][]string{{"jfrogtest"}}, + Checksum: entities.Checksum{ + Sha1: "5e154f8080f0e07a3a28950a5e5ee563df625ed3", + Md5: "8a0ac99046e2c9c962aee498633eccc3", + Sha256: "4119c009fa51fba45331235f00908ab77f2a402ee37e47dfc2dd8d422faa160f", + }, + }, + { + Id: "minimist:0.1.0", + Type: "", + Scopes: []string{"prod"}, + RequestedBy: [][]string{{"jfrogtest"}}, + Checksum: entities.Checksum{ + Sha1: "99df657a52574c21c9057497df742790b2b4c0de", + Md5: "0c9e3002c2af447fcf831fe3f751b2d8", + Sha256: "d8d08725641599bd538ef91f6e77109fec81f74aecaa994d568d61b44d06df6d", + }, + }, + } + sort.Slice(expectedDeps, func(i, j int) bool { + return expectedDeps[i].Id > expectedDeps[j].Id + }) + sort.Slice(dependencies, func(i, j int) bool { + return dependencies[i].Id > dependencies[j].Id + }) + assert.Equal(t, expectedDeps, dependencies) +} + // A project built differently for each operating system. func TestDependenciesTreeDifferentBetweenOKs(t *testing.T) { npmVersion, _, err := GetNpmVersionAndExecPath(logger) From a999bd70be0aad73afa51d8f1985dddd6f7a264a Mon Sep 17 00:00:00 2001 From: asafambar Date: Wed, 13 Sep 2023 18:12:16 +0300 Subject: [PATCH 3/8] fix test --- build/utils/npm_test.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index 7727f8c2..46b7b92f 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -205,17 +205,13 @@ func TestDependencyWithNoIntegrity(t *testing.T) { // This test case check that CalculateNpmDependenciesList ignore node_modules and update package-lock.json when needed, // this according to the params 'IgnoreNodeModules' and 'OverWritePackageLock'. func TestDependencyPackageLockOnly(t *testing.T) { - path, err := filepath.Abs(filepath.Join("..", "testdata/npm/project6")) - assert.NoError(t, err) + path, cleanup := testdatautils.CreateTestProject(t, filepath.Join("..", "testdata/npm/project6")) + defer cleanup() data, err := os.ReadFile(filepath.Join(path, "package-lock_test.json")) require.NoError(t, err) info, err := os.Stat(filepath.Join(path, "package-lock_test.json")) require.NoError(t, err) os.WriteFile(filepath.Join(path, "package-lock.json"), data, info.Mode().Perm()) - defer func() { - assert.NoError(t, os.Remove(filepath.Join(path, "package-lock.json"))) - assert.NoError(t, os.Remove(filepath.Join(path, "node_modules", ".package-lock.json"))) - }() // sleep so the package.json modified time will be bigger than the package-lock.json, this make sure it will recalculate lock file. time.Sleep(time.Millisecond * 5) require.NoError(t, os.Chtimes(filepath.Join(path, "package.json"), time.Now(), time.Now())) From 96c8b19a97c49e7679f6b891146f0d05a37705e9 Mon Sep 17 00:00:00 2001 From: asafambar Date: Wed, 13 Sep 2023 21:58:52 +0300 Subject: [PATCH 4/8] fix test pt.2 --- build/utils/npm_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index 46b7b92f..5c06e04e 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -211,9 +211,9 @@ func TestDependencyPackageLockOnly(t *testing.T) { require.NoError(t, err) info, err := os.Stat(filepath.Join(path, "package-lock_test.json")) require.NoError(t, err) - os.WriteFile(filepath.Join(path, "package-lock.json"), data, info.Mode().Perm()) + require.NoError(t, os.WriteFile(filepath.Join(path, "package-lock.json"), data, info.Mode().Perm())) // sleep so the package.json modified time will be bigger than the package-lock.json, this make sure it will recalculate lock file. - time.Sleep(time.Millisecond * 5) + time.Sleep(time.Millisecond * 1000) require.NoError(t, os.Chtimes(filepath.Join(path, "package.json"), time.Now(), time.Now())) // Calculate dependencies. From 94aa17c16cd08f3002112f9bd4ef28d5be198ea8 Mon Sep 17 00:00:00 2001 From: asafambar Date: Wed, 13 Sep 2023 22:43:35 +0300 Subject: [PATCH 5/8] fix test pt.3 --- build/utils/npm_test.go | 95 ++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 45 deletions(-) diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index 5c06e04e..5d237177 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -6,7 +6,6 @@ import ( "os" "path/filepath" "reflect" - "sort" "testing" "time" @@ -213,62 +212,68 @@ func TestDependencyPackageLockOnly(t *testing.T) { require.NoError(t, err) require.NoError(t, os.WriteFile(filepath.Join(path, "package-lock.json"), data, info.Mode().Perm())) // sleep so the package.json modified time will be bigger than the package-lock.json, this make sure it will recalculate lock file. - time.Sleep(time.Millisecond * 1000) + time.Sleep(time.Millisecond * 20) require.NoError(t, os.Chtimes(filepath.Join(path, "package.json"), time.Now(), time.Now())) // Calculate dependencies. - dependencies, err := CalculateNpmDependenciesList("npm", path, "jfrogtest", - NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: true, OverWritePackageLock: true}, true, logger) + dependencies, err := CalculateDependenciesMap("npm", path, "jfrogtest", + NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: true, OverWritePackageLock: true}, logger) assert.NoError(t, err) - expectedDeps := []entities.Dependency{{ - Id: "underscore:1.13.6", - Scopes: []string{"prod"}, - RequestedBy: [][]string{{"jfrogtest"}}, - Checksum: entities.Checksum{ - Sha1: "04786a1f589dc6c09f761fc5f45b89e935136441", - Md5: "945e1ea169a281c296b82ad2dd5466f6", - Sha256: "aef5a43ac7f903136a93e75a274e3a7b50de1a92277e1666457cabf62eeb0140", + expectedRes := map[string]*dependencyInfo{ + "underscore:1.13.6": { + Dependency: entities.Dependency{ + Id: "underscore:1.13.6", + Scopes: []string{"prod"}, + RequestedBy: [][]string{{"jfrogtest"}}, + Checksum: entities.Checksum{}, + }, + npmLsDependency: &npmLsDependency{ + Name: "underscore", + Version: "1.13.6", + Integrity: "sha512-+A5Sja4HP1M08MaXya7p5LvjuM7K6q/2EaC0+iovj/wOcMsTzMvDFbasi/oSapiwOlt252IqsKqPjCl7huKS0A==", + }, }, - }, - { - Id: "cors.js:0.0.1-security", - Scopes: []string{"prod"}, - RequestedBy: [][]string{{"jfrogtest"}}, - Checksum: entities.Checksum{ - Sha1: "a1304531e44d11f4406b424b8377c3f3f1d3a934", - Md5: "f798d8a0d5e59e0d1b10a8fdc7660df0", - Sha256: "e2352450325dba7f38c45ec43ca77eab2cdba66fdb232061045e7039ada1da7e", + "cors.js:0.0.1-security": { + Dependency: entities.Dependency{ + Id: "cors.js:0.0.1-security", + Scopes: []string{"prod"}, + RequestedBy: [][]string{{"jfrogtest"}}, + Checksum: entities.Checksum{}, + }, + npmLsDependency: &npmLsDependency{ + Name: "cors.js", + Version: "0.0.1-security", + Integrity: "sha512-Cu4D8imt82jd/AuMBwTpjrXiULhaMdig2MD2NBhRKbbcuCTWeyN2070SCEDaJuI/4kA1J9Nnvj6/cBe/zfnrrw==", }, }, - { - Id: "lightweight:0.1.0", - Scopes: []string{"prod"}, - RequestedBy: [][]string{{"jfrogtest"}}, - Checksum: entities.Checksum{ - Sha1: "5e154f8080f0e07a3a28950a5e5ee563df625ed3", - Md5: "8a0ac99046e2c9c962aee498633eccc3", - Sha256: "4119c009fa51fba45331235f00908ab77f2a402ee37e47dfc2dd8d422faa160f", + "lightweight:0.1.0": { + Dependency: entities.Dependency{ + Id: "lightweight:0.1.0", + Scopes: []string{"prod"}, + RequestedBy: [][]string{{"jfrogtest"}}, + Checksum: entities.Checksum{}, + }, + npmLsDependency: &npmLsDependency{ + Name: "lightweight", + Version: "0.1.0", + Integrity: "sha512-10pYSQA9EJqZZnXDR0urhg8Z0Y1XnRfi41ZFj3ZFTKJ5PjRq82HzT7LKlPyxewy3w2WA2POfi3jQQn7Y53oPcQ==", }, }, - { - Id: "minimist:0.1.0", - Type: "", - Scopes: []string{"prod"}, - RequestedBy: [][]string{{"jfrogtest"}}, - Checksum: entities.Checksum{ - Sha1: "99df657a52574c21c9057497df742790b2b4c0de", - Md5: "0c9e3002c2af447fcf831fe3f751b2d8", - Sha256: "d8d08725641599bd538ef91f6e77109fec81f74aecaa994d568d61b44d06df6d", + "minimist:0.1.0": { + Dependency: entities.Dependency{ + Id: "minimist:0.1.0", + Scopes: []string{"prod"}, + RequestedBy: [][]string{{"jfrogtest"}}, + Checksum: entities.Checksum{}, + }, + npmLsDependency: &npmLsDependency{ + Name: "minimist", + Version: "0.1.0", + Integrity: "sha512-wR5Ipl99t0mTGwLjQJnBjrP/O7zBbLZqvA3aw32DmLx+nXHfWctUjzDjnDx09pX1Po86WFQazF9xUzfMea3Cnw==", }, }, } - sort.Slice(expectedDeps, func(i, j int) bool { - return expectedDeps[i].Id > expectedDeps[j].Id - }) - sort.Slice(dependencies, func(i, j int) bool { - return dependencies[i].Id > dependencies[j].Id - }) - assert.Equal(t, expectedDeps, dependencies) + assert.Equal(t, expectedRes, dependencies) } // A project built differently for each operating system. From b6bffbdb56395871035f2feb29d615b5236f28a5 Mon Sep 17 00:00:00 2001 From: asafambar Date: Thu, 14 Sep 2023 15:27:10 +0300 Subject: [PATCH 6/8] fix test by limit to run only on npm v7 and above. --- build/utils/npm_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index 5d237177..03d9bea5 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -204,6 +204,10 @@ func TestDependencyWithNoIntegrity(t *testing.T) { // This test case check that CalculateNpmDependenciesList ignore node_modules and update package-lock.json when needed, // this according to the params 'IgnoreNodeModules' and 'OverWritePackageLock'. func TestDependencyPackageLockOnly(t *testing.T) { + npmVersion, _, err := GetNpmVersionAndExecPath(logger) + if !npmVersion.AtLeast("7.0.0") { + t.Skip("Running on npm v7 and above only, skipping...") + } path, cleanup := testdatautils.CreateTestProject(t, filepath.Join("..", "testdata/npm/project6")) defer cleanup() data, err := os.ReadFile(filepath.Join(path, "package-lock_test.json")) From 5f4b478be9fcdc99cac9f9ce3083592f84e59142 Mon Sep 17 00:00:00 2001 From: asafambar Date: Mon, 18 Sep 2023 11:07:24 +0300 Subject: [PATCH 7/8] fix test --- build/utils/npm_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index 03d9bea5..6b44e9fe 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -205,6 +205,7 @@ func TestDependencyWithNoIntegrity(t *testing.T) { // this according to the params 'IgnoreNodeModules' and 'OverWritePackageLock'. func TestDependencyPackageLockOnly(t *testing.T) { npmVersion, _, err := GetNpmVersionAndExecPath(logger) + require.NoError(t, err) if !npmVersion.AtLeast("7.0.0") { t.Skip("Running on npm v7 and above only, skipping...") } From 9caa065a068e7b49562a6975c9e518d7334eb40a Mon Sep 17 00:00:00 2001 From: asafambar Date: Wed, 27 Sep 2023 20:40:03 +0300 Subject: [PATCH 8/8] Fix CR. --- build/utils/npm.go | 12 ++++++------ build/utils/npm_test.go | 23 +++++++++++------------ 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/build/utils/npm.go b/build/utils/npm.go index ee0f7444..205a1c15 100644 --- a/build/utils/npm.go +++ b/build/utils/npm.go @@ -135,7 +135,7 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams Np if isDirExistsErr != nil { return nil, isDirExistsErr } - if !isPackageLockExist || (npmListParams.OverWritePackageLock && checkIfLockFileShouldBeUpdated(srcPath, log)) { + if !isPackageLockExist || (npmListParams.OverwritePackageLock && checkIfLockFileShouldBeUpdated(srcPath, log)) { err := installPackageLock(executablePath, srcPath, npmListParams.Args, log, npmVersion) if err != nil { return nil, err @@ -164,8 +164,8 @@ func installPackageLock(executablePath, srcPath string, npmArgs []string, log ut return errors.New("it looks like you’re using version " + npmVersion.GetVersion() + " of the npm client. Versions below 6.0.0 require running `npm install` before running this command") } -// check package.json modified, -// this can hint us new packages added to package.json which were not updated in package-lock.json.. +// Check if package.json has been modified. +// This might indicate the addition of new packages to package.json that haven't been reflected in package-lock.json. func checkIfLockFileShouldBeUpdated(srcPath string, log utils.Log) bool { packageJsonInfo, err := os.Stat(filepath.Join(srcPath, "package.json")) if err != nil { @@ -193,10 +193,10 @@ func GetNpmVersion(executablePath string, log utils.Log) (*version.Version, erro type NpmTreeDepListParam struct { Args []string - // ignore node modules folder if exists. + // Ignore the node_modules folder if exists, using the '--package-lock-only' flag IgnoreNodeModules bool - // rewrite package lock json if exists. - OverWritePackageLock bool + // Rewrite package-lock.json, if exists. + OverwritePackageLock bool } // npm >=7 ls results for a single dependency diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index 6b44e9fe..23509790 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -201,8 +201,8 @@ func TestDependencyWithNoIntegrity(t *testing.T) { assert.Greaterf(t, len(dependencies), 0, "Error: dependencies are not found!") } -// This test case check that CalculateNpmDependenciesList ignore node_modules and update package-lock.json when needed, -// this according to the params 'IgnoreNodeModules' and 'OverWritePackageLock'. +// This test case verifies that CalculateNpmDependenciesList correctly handles the exclusion of 'node_modules' +// and updates 'package-lock.json' as required, based on the 'IgnoreNodeModules' and 'OverwritePackageLock' parameters. func TestDependencyPackageLockOnly(t *testing.T) { npmVersion, _, err := GetNpmVersionAndExecPath(logger) require.NoError(t, err) @@ -211,20 +211,20 @@ func TestDependencyPackageLockOnly(t *testing.T) { } path, cleanup := testdatautils.CreateTestProject(t, filepath.Join("..", "testdata/npm/project6")) defer cleanup() - data, err := os.ReadFile(filepath.Join(path, "package-lock_test.json")) - require.NoError(t, err) - info, err := os.Stat(filepath.Join(path, "package-lock_test.json")) - require.NoError(t, err) - require.NoError(t, os.WriteFile(filepath.Join(path, "package-lock.json"), data, info.Mode().Perm())) + assert.NoError(t, utils.MoveFile(filepath.Join(path, "package-lock_test.json"), filepath.Join(path, "package-lock.json"))) // sleep so the package.json modified time will be bigger than the package-lock.json, this make sure it will recalculate lock file. - time.Sleep(time.Millisecond * 20) - require.NoError(t, os.Chtimes(filepath.Join(path, "package.json"), time.Now(), time.Now())) + require.NoError(t, os.Chtimes(filepath.Join(path, "package.json"), time.Now(), time.Now().Add(time.Millisecond*20))) // Calculate dependencies. dependencies, err := CalculateDependenciesMap("npm", path, "jfrogtest", - NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: true, OverWritePackageLock: true}, logger) + NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: true, OverwritePackageLock: true}, logger) assert.NoError(t, err) - expectedRes := map[string]*dependencyInfo{ + var expectedRes = getExpectedRespForTestDependencyPackageLockOnly() + assert.Equal(t, expectedRes, dependencies) +} + +func getExpectedRespForTestDependencyPackageLockOnly() map[string]*dependencyInfo { + return map[string]*dependencyInfo{ "underscore:1.13.6": { Dependency: entities.Dependency{ Id: "underscore:1.13.6", @@ -278,7 +278,6 @@ func TestDependencyPackageLockOnly(t *testing.T) { }, }, } - assert.Equal(t, expectedRes, dependencies) } // A project built differently for each operating system.