diff --git a/lifecycle/cli.go b/lifecycle/cli.go index 246c2e524..da6c3d7d7 100644 --- a/lifecycle/cli.go +++ b/lifecycle/cli.go @@ -2,11 +2,9 @@ package lifecycle import ( "errors" - "fmt" commonCliUtils "github.com/jfrog/jfrog-cli-core/v2/common/cliutils" "github.com/jfrog/jfrog-cli-core/v2/common/commands" "github.com/jfrog/jfrog-cli-core/v2/common/spec" - speccore "github.com/jfrog/jfrog-cli-core/v2/common/spec" coreCommon "github.com/jfrog/jfrog-cli-core/v2/docs/common" "github.com/jfrog/jfrog-cli-core/v2/lifecycle" coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" @@ -26,7 +24,6 @@ import ( "github.com/jfrog/jfrog-client-go/utils" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/urfave/cli" - "os" "strings" ) @@ -134,56 +131,21 @@ func validateCreateReleaseBundleContext(c *cli.Context) error { } func assertValidCreationMethod(c *cli.Context) error { - // Determine the methods provided methods := []bool{ - c.IsSet("spec"), - c.IsSet(cliutils.Builds), - c.IsSet(cliutils.ReleaseBundles), - } - methodCount := coreutils.SumTrueValues(methods) - - // Validate that only one creation method is provided - if err := validateSingleCreationMethod(methodCount); err != nil { - return err - } - - if err := validateCreationValuesPresence(c, methodCount); err != nil { - return err - } - return nil -} - -func validateSingleCreationMethod(methodCount int) error { - if methodCount > 1 { - return errorutils.CheckErrorf( - "exactly one creation source must be supplied: --%s, --%s, or --%s.\n"+ - "Opt to use the --%s option as the --%s and --%s are deprecated", - "spec", cliutils.Builds, cliutils.ReleaseBundles, + c.IsSet("spec"), c.IsSet(cliutils.Builds), c.IsSet(cliutils.ReleaseBundles)} + if coreutils.SumTrueValues(methods) > 1 { + return errorutils.CheckErrorf("exactly one creation source must be supplied: --%s, --%s or --%s.\n"+ + "Opt to use the --%s option as the --%s and --%s are deprecated", "spec", cliutils.Builds, cliutils.ReleaseBundles, - ) + "spec", cliutils.Builds, cliutils.ReleaseBundles) } - return nil -} - -func validateCreationValuesPresence(c *cli.Context, methodCount int) error { - if methodCount == 0 { - if !areBuildFlagsSet(c) && !areBuildEnvVarsSet() { - return errorutils.CheckErrorf("Either --build-name or JFROG_CLI_BUILD_NAME, and --build-number or JFROG_CLI_BUILD_NUMBER must be defined") - } + // If the user did not provide a source, we suggest only the recommended spec approach. + if coreutils.SumTrueValues(methods) == 0 { + return errorutils.CheckErrorf("the --spec option is mandatory") } return nil } -// areBuildFlagsSet checks if build-name or build-number flags are set. -func areBuildFlagsSet(c *cli.Context) bool { - return c.IsSet(cliutils.BuildName) || c.IsSet(cliutils.BuildNumber) -} - -// areBuildEnvVarsSet checks if build environment variables are set. -func areBuildEnvVarsSet() bool { - return os.Getenv("JFROG_CLI_BUILD_NUMBER") != "" && os.Getenv("JFROG_CLI_BUILD_NAME") != "" -} - func create(c *cli.Context) (err error) { if err = validateCreateReleaseBundleContext(c); err != nil { return err @@ -207,34 +169,10 @@ func create(c *cli.Context) (err error) { } func getReleaseBundleCreationSpec(c *cli.Context) (*spec.SpecFiles, error) { - // לֹhecking if the "builds" or "release-bundles" flags are set - if so, the spec flag should be ignored - if c.IsSet(cliutils.Builds) || c.IsSet(cliutils.ReleaseBundles) { - return nil, nil - } - - // Check if the "spec" flag is set - if so, return the spec if c.IsSet("spec") { return cliutils.GetSpec(c, true, false) } - - // Else - create a spec from the buildName and buildnumber flags or env vars - buildName := getStringFlagOrEnv(c, cliutils.BuildName, coreutils.BuildName) - buildNumber := getStringFlagOrEnv(c, cliutils.BuildNumber, coreutils.BuildNumber) - - if buildName != "" && buildNumber != "" { - return speccore.CreateSpecFromBuildNameAndNumber(buildName, buildNumber) - } - - return nil, fmt.Errorf("either the --spec flag must be provided, " + - "or both --build-name and --build-number flags (or their corresponding environment variables " + - "JFROG_CLI_BUILD_NAME and JFROG_CLI_BUILD_NUMBER) must be set") -} - -func getStringFlagOrEnv(c *cli.Context, flag string, envVar string) string { - if c.IsSet(flag) { - return c.String(flag) - } - return os.Getenv(envVar) + return nil, nil } func promote(c *cli.Context) error { diff --git a/lifecycle/cli_test.go b/lifecycle/cli_test.go index 55317a63a..8f5aefe8b 100644 --- a/lifecycle/cli_test.go +++ b/lifecycle/cli_test.go @@ -4,7 +4,6 @@ import ( "github.com/jfrog/jfrog-cli/utils/cliutils" "github.com/jfrog/jfrog-cli/utils/tests" "github.com/stretchr/testify/assert" - "os" "path/filepath" "testing" ) @@ -57,73 +56,3 @@ func TestCreateReleaseBundleSpecWithProject(t *testing.T) { creationSpec.Get(0).Project = "" assert.Equal(t, projectKey, cliutils.GetProject(context)) } - -func TestGetReleaseBundleCreationSpec(t *testing.T) { - - t.Run("Spec Flag Set", func(t *testing.T) { - specFile := filepath.Join("testdata", "specfile.json") - ctx, _ := tests.CreateContext(t, []string{"spec=" + specFile}, []string{}) - - spec, err := getReleaseBundleCreationSpec(ctx) - - assert.NoError(t, err) - assert.NotNil(t, spec) - }) - - t.Run("Build Name and Number Set via Flags", func(t *testing.T) { - ctx, _ := tests.CreateContext(t, []string{"build-name=Common-builds", "build-number=1.0.0"}, []string{}) - - spec, err := getReleaseBundleCreationSpec(ctx) - - assert.NoError(t, err) - assert.NotNil(t, spec) - assert.Equal(t, "Common-builds/1.0.0", spec.Files[0].Build) - }) - - t.Run("Build Name and Number Set via Env Variables", func(t *testing.T) { - t.Setenv("JFROG_CLI_BUILD_NAME", "Common-builds") - t.Setenv("JFROG_CLI_BUILD_NUMBER", "2.0.0") - - ctx, _ := tests.CreateContext(t, []string{}, []string{}) - - spec, err := getReleaseBundleCreationSpec(ctx) - - assert.NoError(t, err) - assert.NotNil(t, spec) - assert.Equal(t, "Common-builds/2.0.0", spec.Files[0].Build) - os.Unsetenv("JFROG_CLI_BUILD_NAME") - os.Unsetenv("JFROG_CLI_BUILD_NUMBER") - }) - - t.Run("Missing Build Name and Number", func(t *testing.T) { - ctx, _ := tests.CreateContext(t, []string{}, []string{}) - - spec, err := getReleaseBundleCreationSpec(ctx) - - assert.Error(t, err) - assert.Nil(t, spec) - assert.EqualError(t, err, "either the --spec flag must be provided, or both --build-name and --build-number flags (or their corresponding environment variables JFROG_CLI_BUILD_NAME and JFROG_CLI_BUILD_NUMBER) must be set") - }) - - t.Run("Only One Build Variable Set", func(t *testing.T) { - ctx, _ := tests.CreateContext(t, []string{"build-name=Common-builds"}, []string{}) - - spec, err := getReleaseBundleCreationSpec(ctx) - - assert.Error(t, err) - assert.Nil(t, spec) - assert.EqualError(t, err, "either the --spec flag must be provided, or both --build-name and --build-number flags (or their corresponding environment variables JFROG_CLI_BUILD_NAME and JFROG_CLI_BUILD_NUMBER) must be set") - }) - - t.Run("One Env Variable One Flag", func(t *testing.T) { - ctx, _ := tests.CreateContext(t, []string{"build-name=Common-builds"}, []string{}) - t.Setenv("JFROG_CLI_BUILD_NUMBER", "2.0.0") - - spec, err := getReleaseBundleCreationSpec(ctx) - - assert.NoError(t, err) - assert.NotNil(t, spec) - assert.Equal(t, "Common-builds/2.0.0", spec.Files[0].Build) - os.Unsetenv("JFROG_CLI_BUILD_NUMBER") - }) -} diff --git a/lifecycle_test.go b/lifecycle_test.go index 4913d5f91..ac66bcec5 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -167,6 +167,7 @@ func TestLifecycleFullFlow(t *testing.T) { // Verify the artifacts were distributed correctly by the provided path mappings. assertExpectedArtifacts(t, tests.SearchAllDevRepo, tests.GetExpectedLifecycleDistributedArtifacts()) */ + } // Import bundles only work on onPerm platforms @@ -205,58 +206,30 @@ func uploadBuilds(t *testing.T) func() { func createRbBackwardCompatible(t *testing.T, specName, sourceOption, rbName, rbVersion string, sync bool) { specFile, err := getSpecFile(specName) assert.NoError(t, err) - createRbWithFlags(t, specFile, sourceOption, "", "", rbName, rbVersion, sync, false) + createRb(t, specFile, sourceOption, rbName, rbVersion, sync, false) } func createRbFromSpec(t *testing.T, specName, rbName, rbVersion string, sync bool, withoutSigningKey bool) { specFile, err := tests.CreateSpec(specName) assert.NoError(t, err) - createRbWithFlags(t, specFile, "spec", "", "", rbName, rbVersion, sync, withoutSigningKey) + createRb(t, specFile, "spec", rbName, rbVersion, sync, withoutSigningKey) } -func TestCreateBundleWithoutSpec(t *testing.T) { - cleanCallback := initLifecycleTest(t, signingKeyOptionalArtifactoryMinVersion) - defer cleanCallback() - - lcManager := getLcServiceManager(t) - - deleteBuilds := uploadBuilds(t) - defer deleteBuilds() - - createRbWithFlags(t, "", "", tests.LcBuildName1, number1, tests.LcRbName1, number1, false, false) - assertStatusCompleted(t, lcManager, tests.LcRbName1, number1, "") - defer deleteReleaseBundle(t, lcManager, tests.LcRbName1, number1) - - createRbWithFlags(t, "", "", tests.LcBuildName2, number2, tests.LcRbName2, number2, false, true) - assertStatusCompleted(t, lcManager, tests.LcRbName2, number2, "") - defer deleteReleaseBundle(t, lcManager, tests.LcRbName2, number2) -} - -func createRbWithFlags(t *testing.T, specFilePath, sourceOption, buildName, buildNumber, rbName, rbVersion string, - sync, withoutSigningKey bool) { +func createRb(t *testing.T, specFilePath, sourceOption, rbName, rbVersion string, sync bool, withoutSigningKey bool) { argsAndOptions := []string{ "rbc", rbName, rbVersion, - } - - if specFilePath != "" { - argsAndOptions = append(argsAndOptions, getOption(sourceOption, specFilePath)) - } - - if buildName != "" && buildNumber != "" { - argsAndOptions = append(argsAndOptions, getOption(cliutils.BuildName, buildName)) - argsAndOptions = append(argsAndOptions, getOption(cliutils.BuildNumber, buildNumber)) + getOption(sourceOption, specFilePath), } if !withoutSigningKey { argsAndOptions = append(argsAndOptions, getOption(cliutils.SigningKey, gpgKeyPairName)) } - + // Add the --sync option only if requested, to test the default value. if sync { argsAndOptions = append(argsAndOptions, getOption(cliutils.Sync, "true")) } - assert.NoError(t, lcCli.Exec(argsAndOptions...)) } diff --git a/utils/cliutils/commandsflags.go b/utils/cliutils/commandsflags.go index 8005a28a7..1ad558d3c 100644 --- a/utils/cliutils/commandsflags.go +++ b/utils/cliutils/commandsflags.go @@ -164,8 +164,8 @@ const ( specVars = "spec-vars" // Build info flags - BuildName = "build-name" - BuildNumber = "build-number" + buildName = "build-name" + buildNumber = "build-number" module = "module" // Generic commands flags @@ -664,12 +664,12 @@ var flagsMap = map[string]cli.Flag{ Name: specVars, Usage: "[Optional] List of semicolon-separated(;) variables in the form of \"key1=value1;key2=value2;...\" (wrapped by quotes) to be replaced in the File Spec. In the File Spec, the variables should be used as follows: ${key1}.` `", }, - BuildName: cli.StringFlag{ - Name: BuildName, + buildName: cli.StringFlag{ + Name: buildName, Usage: "[Optional] Providing this option will collect and record build info for this build name. Build number option is mandatory when this option is provided.` `", }, - BuildNumber: cli.StringFlag{ - Name: BuildNumber, + buildNumber: cli.StringFlag{ + Name: buildNumber, Usage: "[Optional] Providing this option will collect and record build info for this build number. Build name option is mandatory when this option is provided.` `", }, module: cli.StringFlag{ @@ -1739,14 +1739,14 @@ var commandFlags = map[string][]string{ }, Upload: { url, user, password, accessToken, sshPassphrase, sshKeyPath, serverId, ClientCertPath, uploadTargetProps, - ClientCertKeyPath, specFlag, specVars, BuildName, BuildNumber, module, uploadExclusions, deb, + ClientCertKeyPath, specFlag, specVars, buildName, buildNumber, module, uploadExclusions, deb, uploadRecursive, uploadFlat, uploadRegexp, retries, retryWaitTime, dryRun, uploadExplode, symlinks, includeDirs, failNoOp, threads, uploadSyncDeletes, syncDeletesQuiet, InsecureTls, detailedSummary, Project, uploadAnt, uploadArchive, uploadMinSplit, uploadSplitCount, ChunkSize, }, Download: { url, user, password, accessToken, sshPassphrase, sshKeyPath, serverId, ClientCertPath, - ClientCertKeyPath, specFlag, specVars, BuildName, BuildNumber, module, exclusions, sortBy, + ClientCertKeyPath, specFlag, specVars, buildName, buildNumber, module, exclusions, sortBy, sortOrder, limit, offset, downloadRecursive, downloadFlat, build, includeDeps, excludeArtifacts, downloadMinSplit, downloadSplitCount, retries, retryWaitTime, dryRun, downloadExplode, bypassArchiveInspection, validateSymlinks, bundle, publicGpgKey, includeDirs, downloadProps, downloadExcludeProps, failNoOp, threads, archiveEntries, downloadSyncDeletes, syncDeletesQuiet, InsecureTls, detailedSummary, Project, @@ -1800,11 +1800,11 @@ var commandFlags = map[string][]string{ Project, }, BuildDockerCreate: { - BuildName, BuildNumber, module, url, user, password, accessToken, sshPassphrase, sshKeyPath, + buildName, buildNumber, module, url, user, password, accessToken, sshPassphrase, sshKeyPath, serverId, imageFile, Project, }, OcStartBuild: { - BuildName, BuildNumber, module, Project, serverId, ocStartBuildRepo, + buildName, buildNumber, module, Project, serverId, ocStartBuildRepo, }, BuildScanLegacy: { url, user, password, accessToken, sshPassphrase, sshKeyPath, serverId, fail, InsecureTls, @@ -1836,21 +1836,21 @@ var commandFlags = map[string][]string{ deployIvyDesc, ivyDescPattern, ivyArtifactsPattern, }, Mvn: { - BuildName, BuildNumber, deploymentThreads, InsecureTls, Project, detailedSummary, xrayScan, xrOutput, + buildName, buildNumber, deploymentThreads, InsecureTls, Project, detailedSummary, xrayScan, xrOutput, }, Gradle: { - BuildName, BuildNumber, deploymentThreads, Project, detailedSummary, xrayScan, xrOutput, + buildName, buildNumber, deploymentThreads, Project, detailedSummary, xrayScan, xrOutput, }, Docker: { - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, serverId, skipLogin, threads, detailedSummary, watches, repoPath, licenses, xrOutput, fail, ExtendedTable, BypassArchiveLimits, MinSeverity, FixableOnly, vuln, }, DockerPush: { - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, serverId, skipLogin, threads, detailedSummary, }, DockerPull: { - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, serverId, skipLogin, }, DockerPromote: { @@ -1858,21 +1858,21 @@ var commandFlags = map[string][]string{ serverId, }, ContainerPush: { - BuildName, BuildNumber, module, url, user, password, accessToken, sshPassphrase, sshKeyPath, + buildName, buildNumber, module, url, user, password, accessToken, sshPassphrase, sshKeyPath, serverId, skipLogin, threads, Project, detailedSummary, }, ContainerPull: { - BuildName, BuildNumber, module, url, user, password, accessToken, sshPassphrase, sshKeyPath, + buildName, buildNumber, module, url, user, password, accessToken, sshPassphrase, sshKeyPath, serverId, skipLogin, Project, }, NpmConfig: { global, serverIdResolve, serverIdDeploy, repoResolve, repoDeploy, }, NpmInstallCi: { - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, }, NpmPublish: { - BuildName, BuildNumber, module, Project, npmDetailedSummary, xrayScan, xrOutput, + buildName, buildNumber, module, Project, npmDetailedSummary, xrayScan, xrOutput, }, PnpmConfig: { global, serverIdResolve, repoResolve, @@ -1881,38 +1881,38 @@ var commandFlags = map[string][]string{ global, serverIdResolve, repoResolve, }, Yarn: { - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, }, NugetConfig: { global, serverIdResolve, repoResolve, nugetV2, }, Nuget: { - BuildName, BuildNumber, module, Project, allowInsecureConnections, + buildName, buildNumber, module, Project, allowInsecureConnections, }, DotnetConfig: { global, serverIdResolve, repoResolve, nugetV2, }, Dotnet: { - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, }, GoConfig: { global, serverIdResolve, serverIdDeploy, repoResolve, repoDeploy, }, GoPublish: { - url, user, password, accessToken, BuildName, BuildNumber, module, Project, detailedSummary, goPublishExclusions, + url, user, password, accessToken, buildName, buildNumber, module, Project, detailedSummary, goPublishExclusions, }, Go: { - BuildName, BuildNumber, module, Project, noFallback, + buildName, buildNumber, module, Project, noFallback, }, TerraformConfig: { global, serverIdDeploy, repoDeploy, }, Terraform: { namespace, provider, tag, exclusions, - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, }, Twine: { - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, }, TransferConfig: { Force, Verbose, IncludeRepos, ExcludeRepos, SourceWorkingDir, TargetWorkingDir, PreChecks, @@ -1931,19 +1931,19 @@ var commandFlags = map[string][]string{ global, serverIdResolve, serverIdDeploy, repoResolve, repoDeploy, }, PipInstall: { - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, }, PipenvConfig: { global, serverIdResolve, serverIdDeploy, repoResolve, repoDeploy, }, PipenvInstall: { - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, }, PoetryConfig: { global, serverIdResolve, repoResolve, }, Poetry: { - BuildName, BuildNumber, module, Project, + buildName, buildNumber, module, Project, }, ReleaseBundleV1Create: { distUrl, user, password, accessToken, serverId, specFlag, specVars, targetProps, @@ -2020,7 +2020,7 @@ var commandFlags = map[string][]string{ }, ReleaseBundleCreate: { platformUrl, user, password, accessToken, serverId, lcSigningKey, lcSync, lcProject, lcBuilds, lcReleaseBundles, - specFlag, specVars, BuildName, BuildNumber, + specFlag, specVars, }, ReleaseBundlePromote: { platformUrl, user, password, accessToken, serverId, lcSigningKey, lcSync, lcProject, lcIncludeRepos, lcExcludeRepos,