Revert "Enhancements to Release Bundle Create Command: Optional Flags and Fallback Support (#2772)" (#2788)

Author: eyalbe4

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 {

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")
-	})
-}

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...))
 }