-
Notifications
You must be signed in to change notification settings - Fork 255
Support npm publish using native npm client with .npmrc via --use-npmrc flag #2952
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.
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
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3,6 +3,7 @@ package main | |||||
import ( | ||||||
"fmt" | ||||||
"github.com/jfrog/jfrog-cli-artifactory/artifactory/commands/generic" | ||||||
utils2 "github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/utils" | ||||||
"github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" | ||||||
"github.com/jfrog/jfrog-client-go/http/httpclient" | ||||||
"github.com/stretchr/testify/require" | ||||||
|
@@ -148,12 +149,119 @@ func testNpm(t *testing.T, isLegacy bool) { | |||||
inttestutils.DeleteBuild(serverDetails.ArtifactoryUrl, tests.NpmBuildName, artHttpDetails) | ||||||
} | ||||||
|
||||||
func TestNpmPublishWithNpmrc(t *testing.T) { | ||||||
testNpmPublishWithNpmrc(t, validateNpmPublish, "npmpublishrcproject", tests.NpmRepo, false) | ||||||
} | ||||||
|
||||||
func TestNpmPublishWithNpmrcScoped(t *testing.T) { | ||||||
testNpmPublishWithNpmrc(t, validateNpmScopedPublish, "npmpublishrcscopedproject", tests.NpmScopedRepo, true) | ||||||
} | ||||||
|
||||||
func testNpmPublishWithNpmrc(t *testing.T, validationFunc func(t *testing.T, npmTest npmTestParams, isNpm7 bool), projectName string, repoName string, isScoped bool) { | ||||||
initNpmTest(t) | ||||||
defer cleanNpmTest(t) | ||||||
wd, err := os.Getwd() | ||||||
assert.NoError(t, err, "Failed to get current dir") | ||||||
defer clientTestUtils.ChangeDirAndAssert(t, wd) | ||||||
buildNumber := strconv.Itoa(1) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
just wondering why not directly to string? |
||||||
npmVersion, _, err := buildutils.GetNpmVersionAndExecPath(log.Logger) | ||||||
if err != nil { | ||||||
assert.NoError(t, err) | ||||||
return | ||||||
} | ||||||
|
||||||
// Init npm project & npmp command for testing | ||||||
npmProjectPath := initNpmPublishRcProjectTest(t, projectName) | ||||||
configFilePath := filepath.Join(npmProjectPath, ".jfrog", "projects", "npm.yaml") | ||||||
|
||||||
// fetch module id | ||||||
packageJsonPath := npmProjectPath + "/package.json" | ||||||
moduleName := readModuleId(t, packageJsonPath, npmVersion) | ||||||
|
||||||
err = createNpmrcForTesting(configFilePath) | ||||||
assert.NoError(t, err) | ||||||
|
||||||
if isScoped { | ||||||
addNpmScopeRegistryToNpmRc(t, npmProjectPath, packageJsonPath, npmVersion) | ||||||
} | ||||||
|
||||||
npmpCmd, err := publishUsingNpmrc(configFilePath, buildNumber) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is it |
||||||
assert.NoError(t, err) | ||||||
|
||||||
result := npmpCmd.Result() | ||||||
assert.NotNil(t, result) | ||||||
|
||||||
validateNpmLocalBuildInfo(t, tests.NpmBuildName, buildNumber, moduleName) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we creating build info as well with |
||||||
assert.NoError(t, artifactoryCli.Exec("bp", tests.NpmBuildName, buildNumber)) | ||||||
|
||||||
// validation | ||||||
testParams := npmTestParams{testName: "npm p", | ||||||
nativeCommand: "npm publish", | ||||||
legacyCommand: "rt npm-publish", | ||||||
repo: repoName, | ||||||
wd: npmProjectPath, | ||||||
validationFunc: validateNpmPublish, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
buildNumber: buildNumber, | ||||||
moduleName: moduleName, | ||||||
} | ||||||
validationFunc(t, testParams, false) | ||||||
} | ||||||
|
||||||
func createNpmrcForTesting(configFilePath string) (err error) { | ||||||
//Creation of npmrc - npmCommand.CreateTempNpmrc() function is used to create a npmrc file used for uploading | ||||||
npmCommand := npm.NewNpmCommand("install", true) | ||||||
npmCommand.SetConfigFilePath(configFilePath) | ||||||
npmCommand.SetServerDetails(serverDetails) | ||||||
err = npmCommand.PreparePrerequisites(tests.NpmRepo) | ||||||
if err != nil { | ||||||
return | ||||||
} | ||||||
err = npmCommand.CreateTempNpmrc() | ||||||
return | ||||||
} | ||||||
|
||||||
func publishUsingNpmrc(configFilePath string, buildNumber string) (npm.NpmPublishCommand, error) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does sample output looks with |
||||||
args := []string{"--use-npmrc=true", "--build-name=" + tests.NpmBuildName, "--build-number=" + buildNumber} | ||||||
npmpCmd := npm.NewNpmPublishCommand() | ||||||
npmpCmd.SetConfigFilePath(configFilePath).SetArgs(args) | ||||||
err := npmpCmd.Init() | ||||||
if err != nil { | ||||||
return *npmpCmd, err | ||||||
} | ||||||
err = commands.Exec(npmpCmd) | ||||||
if err != nil { | ||||||
return *npmpCmd, err | ||||||
} | ||||||
return *npmpCmd, err | ||||||
} | ||||||
|
||||||
func readModuleId(t *testing.T, wd string, npmVersion *version.Version) string { | ||||||
packageInfo, err := buildutils.ReadPackageInfoFromPackageJsonIfExists(filepath.Dir(wd), npmVersion) | ||||||
assert.NoError(t, err) | ||||||
return packageInfo.BuildInfoModuleId() | ||||||
} | ||||||
|
||||||
func addNpmScopeRegistryToNpmRc(t *testing.T, projectPath string, packageJsonPath string, npmVersion *version.Version) { | ||||||
scope := getScopeFromPackageJson(t, packageJsonPath, npmVersion) | ||||||
authConfig, err := serverDetails.CreateArtAuthConfig() | ||||||
assert.NoError(t, err) | ||||||
_, registry, err := utils2.GetArtifactoryNpmRepoDetails(tests.NpmScopedRepo, authConfig, false) | ||||||
assert.NoError(t, err) | ||||||
scopedRegistry := scope + ":registry=" + registry | ||||||
npmrcFilePath := filepath.Join(projectPath, ".npmrc") | ||||||
npmrcFile, err := os.OpenFile(npmrcFilePath, os.O_APPEND|os.O_WRONLY, 0644) | ||||||
assert.NoError(t, err) | ||||||
defer npmrcFile.Close() | ||||||
_, err = npmrcFile.WriteString(scopedRegistry) | ||||||
assert.NoError(t, err) | ||||||
} | ||||||
|
||||||
func getScopeFromPackageJson(t *testing.T, wd string, npmVersion *version.Version) string { | ||||||
packageInfo, err := buildutils.ReadPackageInfoFromPackageJsonIfExists(filepath.Dir(wd), npmVersion) | ||||||
assert.NoError(t, err) | ||||||
return packageInfo.Scope | ||||||
} | ||||||
|
||||||
func TestNpmWithGlobalConfig(t *testing.T) { | ||||||
initNpmTest(t) | ||||||
defer cleanNpmTest(t) | ||||||
|
@@ -285,6 +393,14 @@ func initNpmProjectTest(t *testing.T) (npmProjectPath string) { | |||||
return | ||||||
} | ||||||
|
||||||
func initNpmPublishRcProjectTest(t *testing.T, projectName string) (npmProjectPath string) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
npmProjectPath = filepath.Dir(createNpmProject(t, projectName)) | ||||||
err := createConfigFileForTest([]string{npmProjectPath}, tests.NpmRemoteRepo, tests.NpmRepo, t, project.Npm, false) | ||||||
assert.NoError(t, err) | ||||||
prepareArtifactoryForNpmBuild(t, npmProjectPath) | ||||||
return | ||||||
} | ||||||
|
||||||
func initNpmWorkspacesProjectTest(t *testing.T) (npmProjectPath string) { | ||||||
npmProjectPath = filepath.Dir(createNpmProject(t, "npmworkspaces")) | ||||||
err := createConfigFileForTest([]string{npmProjectPath}, tests.NpmRemoteRepo, tests.NpmRepo, t, project.Npm, false) | ||||||
|
@@ -361,8 +477,8 @@ func validateNpmPublish(t *testing.T, npmTestParams npmTestParams, isNpm7 bool) | |||||
} | ||||||
|
||||||
func validateNpmScopedPublish(t *testing.T, npmTestParams npmTestParams, isNpm7 bool) { | ||||||
verifyExistInArtifactoryByProps(tests.GetNpmDeployedScopedArtifacts(isNpm7), | ||||||
tests.NpmRepo+"/*", | ||||||
verifyExistInArtifactoryByProps(tests.GetNpmDeployedScopedArtifacts(npmTestParams.repo, isNpm7), | ||||||
npmTestParams.repo+"/*", | ||||||
fmt.Sprintf("build.name=%v;build.number=%v;build.timestamp=*", tests.NpmBuildName, npmTestParams.buildNumber), t) | ||||||
validateNpmCommonPublish(t, npmTestParams, isNpm7, true) | ||||||
} | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
{ | ||
"name": "jfrog-cli-tests", | ||
"version": "1.0.0", | ||
"description": "test package", | ||
"main": "index.js", | ||
"scripts": { | ||
"test": "echo \"Error: no test specified\" && exit 1" | ||
}, | ||
"author": "", | ||
"license": "ISC", | ||
"dependencies": { | ||
"xml": "1.0.1" | ||
}, | ||
"devDependencies": { | ||
"json": "9.0.6" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
{ | ||
"name": "@jscope/jfrog-cli-tests", | ||
"version": "1.0.0", | ||
"description": "test package", | ||
"main": "index.js", | ||
"scripts": { | ||
"test": "echo \"Error: no test specified\" && exit 1" | ||
}, | ||
"author": "", | ||
"license": "ISC", | ||
"dependencies": { | ||
"xml": "1.0.1" | ||
}, | ||
"devDependencies": { | ||
"json": "9.0.6" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"key": "${NPM_SCOPED_REPO}", | ||
"rclass": "local", | ||
"packageType": "npm" | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -363,6 +363,7 @@ const ( | |||||||||||||||||
// Unique npm flags | ||||||||||||||||||
npmPrefix = "npm-" | ||||||||||||||||||
npmDetailedSummary = npmPrefix + detailedSummary | ||||||||||||||||||
useNpmRc = "use-npmrc" | ||||||||||||||||||
|
||||||||||||||||||
// Unique nuget/dotnet config flags | ||||||||||||||||||
nugetV2 = "nuget-v2" | ||||||||||||||||||
|
@@ -1704,6 +1705,10 @@ var flagsMap = map[string]cli.Flag{ | |||||||||||||||||
Name: ApplicationKey, | ||||||||||||||||||
Usage: "[Optional] JFrog ApplicationKey Key` ` ", | ||||||||||||||||||
}, | ||||||||||||||||||
useNpmRc: cli.BoolFlag{ | ||||||||||||||||||
Name: useNpmRc, | ||||||||||||||||||
Usage: "[Default: false] Set to true if you'd like to use the .npmrc file for configurations. Note: This flag would invoke npm native client behind the scenes, has performance implications and does not support deployment view and detailed summary` `", | ||||||||||||||||||
}, | ||||||||||||||||||
Comment on lines
+1708
to
+1711
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
var commandFlags = map[string][]string{ | ||||||||||||||||||
|
@@ -1853,7 +1858,7 @@ var commandFlags = map[string][]string{ | |||||||||||||||||
BuildName, BuildNumber, module, Project, | ||||||||||||||||||
}, | ||||||||||||||||||
NpmPublish: { | ||||||||||||||||||
BuildName, BuildNumber, module, Project, npmDetailedSummary, xrayScan, xrOutput, | ||||||||||||||||||
BuildName, BuildNumber, module, Project, npmDetailedSummary, xrayScan, xrOutput, useNpmRc, | ||||||||||||||||||
}, | ||||||||||||||||||
PnpmConfig: { | ||||||||||||||||||
global, serverIdResolve, repoResolve, | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,6 +88,7 @@ const ( | |
MavenWithoutDeployerConfig = "maven_without_deployer.yaml" | ||
MoveCopySpecExclusions = "move_copy_spec_exclusions.json" | ||
NpmLocalRepositoryConfig = "npm_local_repository_config.json" | ||
NpmLocalScopedRespositoryConfig = "npm_local_scoped_repository_config.json" | ||
NpmRemoteRepositoryConfig = "npm_remote_repository_config.json" | ||
NugetRemoteRepositoryConfig = "nuget_remote_repository_config.json" | ||
Out = "out" | ||
|
@@ -165,6 +166,7 @@ const ( | |
XrayEndpoint = "xray/" | ||
YarnRemoteRepositoryConfig = "yarn_remote_repository_config.json" | ||
ReleaseLifecycleImportDependencySpec = "import_bundle_repo_dependency.json" | ||
UseNpmRcFlag = "--use-npmrc" | ||
) | ||
|
||
var ( | ||
|
@@ -181,6 +183,7 @@ var ( | |
MvnRepo1 = "cli-mvn1" | ||
MvnRepo2 = "cli-mvn2" | ||
NpmRepo = "cli-npm" | ||
NpmScopedRepo = "cli-npm-scoped" | ||
NpmRemoteRepo = "cli-npm-remote" | ||
NugetRemoteRepo = "cli-nuget-remote" | ||
YarnRemoteRepo = "cli-yarn-remote" | ||
|
@@ -1797,8 +1800,8 @@ func GetGradleDeployedArtifacts() []string { | |
} | ||
} | ||
|
||
func GetNpmDeployedScopedArtifacts(isNpm7 bool) []string { | ||
path := NpmRepo + "/@jscope/jfrog-cli-tests/-/@jscope/" | ||
func GetNpmDeployedScopedArtifacts(npmRepo string, isNpm7 bool) []string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this change required? |
||
path := npmRepo + "/@jscope/jfrog-cli-tests/-/@jscope/" | ||
path += GetNpmArtifactName(isNpm7, true) | ||
return []string{ | ||
path, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.