Skip to content
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

Enhancements to Release Bundle Create Command: Optional Flags and Fallback Support #2772

Merged
merged 12 commits into from
Dec 5, 2024
Merged
80 changes: 71 additions & 9 deletions lifecycle/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
)

Expand Down Expand Up @@ -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 {
eyalbe4 marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -169,10 +207,34 @@ 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) {
eyalbe4 marked this conversation as resolved.
Show resolved Hide resolved
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)
eyalbe4 marked this conversation as resolved.
Show resolved Hide resolved

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 {
eyalbe4 marked this conversation as resolved.
Show resolved Hide resolved
if c.IsSet(flag) {
return c.String(flag)
}
return os.Getenv(envVar)
}

func promote(c *cli.Context) error {
Expand Down
71 changes: 71 additions & 0 deletions lifecycle/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/jfrog/jfrog-cli/utils/cliutils"
"github.com/jfrog/jfrog-cli/utils/tests"
"github.com/stretchr/testify/assert"
"os"
"path/filepath"
"testing"
)
Expand Down Expand Up @@ -56,3 +57,73 @@ 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")
Comment on lines +84 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When test functions set env vars, they must also remove them at the end. There's already a tulity function in the code that takes care of this. Check other tests asa reference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way you set the variables is unsafe. You should use the SetEnvWithCallbackAndAssert API.


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")
})
}
39 changes: 33 additions & 6 deletions lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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...))
}

Expand Down
Loading
Loading