Enhancements to Release Bundle Create Command. (#2789)

Author: oshratZairi

lifecycle/cli.go

@@ -2,9 +2,11 @@ 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"
@@ -24,6 +26,7 @@ import (
 	"github.com/jfrog/jfrog-client-go/utils"
 	"github.com/jfrog/jfrog-client-go/utils/errorutils"
 	"github.com/urfave/cli"
+	"os"
 	"strings"
 )
 
@@ -131,21 +134,56 @@ 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)}
-	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",
+		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,
 			"spec", cliutils.Builds, cliutils.ReleaseBundles,
-			"spec", cliutils.Builds, cliutils.ReleaseBundles)
+		)
 	}
-	// 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
+}
+
+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")
+		}
 	}
 	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
@@ -169,10 +207,34 @@ func create(c *cli.Context) (err error) {
 }
 
 func getReleaseBundleCreationSpec(c *cli.Context) (*spec.SpecFiles, error) {
+	// Checking 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)
 	}
-	return nil, nil
+
+	// 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)
 }
 
 func promote(c *cli.Context) error {

lifecycle/cli_test.go

@@ -1,8 +1,10 @@
 package lifecycle
 
 import (
+	"github.com/jfrog/jfrog-cli-core/v2/utils/coreutils"
 	"github.com/jfrog/jfrog-cli/utils/cliutils"
 	"github.com/jfrog/jfrog-cli/utils/tests"
+	clientTestUtils "github.com/jfrog/jfrog-client-go/utils/tests"
 	"github.com/stretchr/testify/assert"
 	"path/filepath"
 	"testing"
@@ -56,3 +58,74 @@ 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) {
+
+		setEnvBuildNameCallBack := clientTestUtils.SetEnvWithCallbackAndAssert(t, coreutils.BuildName, "Common-builds")
+		defer setEnvBuildNameCallBack()
+		setEnvBuildNumberCallBack := clientTestUtils.SetEnvWithCallbackAndAssert(t, coreutils.BuildNumber, "2.0.0")
+		defer setEnvBuildNumberCallBack()
+
+		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)
+	})
+
+	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{})
+		setEnvBuildNumberCallBack := clientTestUtils.SetEnvWithCallbackAndAssert(t, coreutils.BuildNumber, "2.0.0")
+		defer setEnvBuildNumberCallBack()
+
+		spec, err := getReleaseBundleCreationSpec(ctx)
+
+		assert.NoError(t, err)
+		assert.NotNil(t, spec)
+		assert.Equal(t, "Common-builds/2.0.0", spec.Files[0].Build)
+	})
+}

lifecycle_test.go

@@ -167,7 +167,6 @@ 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
@@ -206,30 +205,58 @@ 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)
-	createRb(t, specFile, sourceOption, rbName, rbVersion, sync, false)
+	createRbWithFlags(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)
-	createRb(t, specFile, "spec", rbName, rbVersion, sync, withoutSigningKey)
+	createRbWithFlags(t, specFile, "spec", "", "", rbName, rbVersion, sync, withoutSigningKey)
 }
 
-func createRb(t *testing.T, specFilePath, sourceOption, rbName, rbVersion string, sync bool, withoutSigningKey bool) {
+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) {
 	argsAndOptions := []string{
 		"rbc",
 		rbName,
 		rbVersion,
-		getOption(sourceOption, specFilePath),
+	}
+
+	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))
 	}
 
 	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...))
 }