From c926e4295ad716f5ff1fe668c11faec81c09dd16 Mon Sep 17 00:00:00 2001 From: oshratz Date: Sun, 17 Nov 2024 16:35:29 +0200 Subject: [PATCH 1/4] Turn the signing key to be optional in RBC --- lifecycle/cli.go | 4 ---- lifecycle/cli_test.go | 4 ++-- lifecycle_test.go | 31 +++++++++++++++++++------------ 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/lifecycle/cli.go b/lifecycle/cli.go index b47731934..366ba25cf 100644 --- a/lifecycle/cli.go +++ b/lifecycle/cli.go @@ -127,10 +127,6 @@ func validateCreateReleaseBundleContext(c *cli.Context) error { return cliutils.WrongNumberOfArgumentsHandler(c) } - if err := assertSigningKeyProvided(c); err != nil { - return err - } - return assertValidCreationMethod(c) } diff --git a/lifecycle/cli_test.go b/lifecycle/cli_test.go index 65b911958..8f5aefe8b 100644 --- a/lifecycle/cli_test.go +++ b/lifecycle/cli_test.go @@ -21,10 +21,10 @@ func TestValidateCreateReleaseBundleContext(t *testing.T) { {"extraArgs", []string{"one", "two", "three", "four"}, []string{}, true}, {"bothSources", []string{"one", "two", "three"}, []string{cliutils.Builds + "=/path/to/file", cliutils.ReleaseBundles + "=/path/to/file"}, true}, {"noSources", []string{"one", "two", "three"}, []string{}, true}, - {"builds without signing key", []string{"name", "version"}, []string{cliutils.Builds + "=/path/to/file"}, true}, + {"builds without signing key", []string{"name", "version"}, []string{cliutils.Builds + "=/path/to/file"}, false}, {"builds correct", []string{"name", "version"}, []string{ cliutils.Builds + "=/path/to/file", cliutils.SigningKey + "=key"}, false}, - {"releaseBundles without signing key", []string{"name", "version", "env"}, []string{cliutils.ReleaseBundles + "=/path/to/file"}, true}, + {"releaseBundles without signing key", []string{"name", "version"}, []string{cliutils.ReleaseBundles + "=/path/to/file"}, false}, {"releaseBundles correct", []string{"name", "version"}, []string{ cliutils.ReleaseBundles + "=/path/to/file", cliutils.SigningKey + "=key"}, false}, {"spec without signing key", []string{"name", "version", "env"}, []string{"spec=/path/to/file"}, true}, diff --git a/lifecycle_test.go b/lifecycle_test.go index 455f035d8..b8e35d578 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -87,14 +87,18 @@ func compareRbArtifacts(t *testing.T, actual services.ReleaseBundleSpecResponse, } func TestReleaseBundleCreationFromAql(t *testing.T) { - testReleaseBundleCreation(t, tests.UploadDevSpecA, tests.LifecycleAql, tests.GetExpectedLifecycleCreationByAql()) + testReleaseBundleCreation(t, tests.UploadDevSpecA, tests.LifecycleAql, tests.GetExpectedLifecycleCreationByAql(), false) } func TestReleaseBundleCreationFromArtifacts(t *testing.T) { - testReleaseBundleCreation(t, tests.UploadDevSpec, tests.LifecycleArtifacts, tests.GetExpectedLifecycleCreationByArtifacts()) + testReleaseBundleCreation(t, tests.UploadDevSpec, tests.LifecycleArtifacts, tests.GetExpectedLifecycleCreationByArtifacts(), false) } -func testReleaseBundleCreation(t *testing.T, uploadSpec, creationSpec string, expected []string) { +func TestReleaseBundleCreationFromArtifactsWithoutSigningKey(t *testing.T) { + testReleaseBundleCreation(t, tests.UploadDevSpec, tests.LifecycleArtifacts, tests.GetExpectedLifecycleCreationByArtifacts(), true) +} + +func testReleaseBundleCreation(t *testing.T, uploadSpec, creationSpec string, expected []string, withoutKey bool) { cleanCallback := initLifecycleTest(t) defer cleanCallback() lcManager := getLcServiceManager(t) @@ -103,7 +107,7 @@ func testReleaseBundleCreation(t *testing.T, uploadSpec, creationSpec string, ex assert.NoError(t, err) runRt(t, "upload", "--spec="+specFile) - createRbFromSpec(t, creationSpec, tests.LcRbName1, number1, true) + createRbFromSpec(t, creationSpec, tests.LcRbName1, number1, true, withoutKey) defer deleteReleaseBundle(t, lcManager, tests.LcRbName1, number1) assertRbArtifacts(t, lcManager, tests.LcRbName1, number1, expected) @@ -119,17 +123,17 @@ func TestLifecycleFullFlow(t *testing.T) { defer deleteBuilds() // Create release bundle from builds synchronously. - createRbFromSpec(t, tests.LifecycleBuilds12, tests.LcRbName1, number1, true) + createRbFromSpec(t, tests.LifecycleBuilds12, tests.LcRbName1, number1, true, false) defer deleteReleaseBundle(t, lcManager, tests.LcRbName1, number1) // Create release bundle from a build asynchronously and assert status. // This build has dependencies which are included in the release bundle. - createRbFromSpec(t, tests.LifecycleBuilds3, tests.LcRbName2, number2, false) + createRbFromSpec(t, tests.LifecycleBuilds3, tests.LcRbName2, number2, false, false) defer deleteReleaseBundle(t, lcManager, tests.LcRbName2, number2) assertStatusCompleted(t, lcManager, tests.LcRbName2, number2, "") // Create a combined release bundle from the two previous release bundle. - createRbFromSpec(t, tests.LifecycleReleaseBundles, tests.LcRbName3, number3, true) + createRbFromSpec(t, tests.LifecycleReleaseBundles, tests.LcRbName3, number3, true, false) defer deleteReleaseBundle(t, lcManager, tests.LcRbName3, number3) // Promote the last release bundle to prod repo 1. @@ -195,22 +199,25 @@ 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) + createRb(t, specFile, sourceOption, rbName, rbVersion, sync, false) } -func createRbFromSpec(t *testing.T, specName, rbName, rbVersion string, sync bool) { +func createRbFromSpec(t *testing.T, specName, rbName, rbVersion string, sync bool, withoutKey bool) { specFile, err := tests.CreateSpec(specName) assert.NoError(t, err) - createRb(t, specFile, "spec", rbName, rbVersion, sync) + createRb(t, specFile, "spec", rbName, rbVersion, sync, withoutKey) } -func createRb(t *testing.T, specFilePath, sourceOption, rbName, rbVersion string, sync bool) { +func createRb(t *testing.T, specFilePath, sourceOption, rbName, rbVersion string, sync bool, withoutKey bool) { argsAndOptions := []string{ "rbc", rbName, rbVersion, getOption(sourceOption, specFilePath), - getOption(cliutils.SigningKey, gpgKeyPairName), + } + + if !withoutKey { + argsAndOptions = append(argsAndOptions, getOption(cliutils.SigningKey, gpgKeyPairName)) } // Add the --sync option only if requested, to test the default value. if sync { From 8aded40d7104aa5e1ceccbb00683d55ab585b8df Mon Sep 17 00:00:00 2001 From: oshratz Date: Mon, 18 Nov 2024 08:57:10 +0200 Subject: [PATCH 2/4] Turn the signing key to be optional in RBP --- lifecycle/cli.go | 11 ----------- utils/cliutils/commandsflags.go | 2 +- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/lifecycle/cli.go b/lifecycle/cli.go index 366ba25cf..da6c3d7d7 100644 --- a/lifecycle/cli.go +++ b/lifecycle/cli.go @@ -184,10 +184,6 @@ func promote(c *cli.Context) error { return cliutils.WrongNumberOfArgumentsHandler(c) } - if err := assertSigningKeyProvided(c); err != nil { - return err - } - lcDetails, err := createLifecycleDetailsByFlags(c) if err != nil { return err @@ -354,13 +350,6 @@ func validateDistributeCommand(c *cli.Context) error { return nil } -func assertSigningKeyProvided(c *cli.Context) error { - if c.String(cliutils.SigningKey) == "" { - return errorutils.CheckErrorf("the --%s option is mandatory", cliutils.SigningKey) - } - return nil -} - func createLifecycleDetailsByFlags(c *cli.Context) (*coreConfig.ServerDetails, error) { lcDetails, err := cliutils.CreateServerDetailsWithConfigOffer(c, true, commonCliUtils.Platform) if err != nil { diff --git a/utils/cliutils/commandsflags.go b/utils/cliutils/commandsflags.go index 446f84eff..c4060f8f5 100644 --- a/utils/cliutils/commandsflags.go +++ b/utils/cliutils/commandsflags.go @@ -1650,7 +1650,7 @@ var flagsMap = map[string]cli.Flag{ }, lcSigningKey: cli.StringFlag{ Name: SigningKey, - Usage: "[Mandatory] The GPG/RSA key-pair name given in Artifactory.` `", + Usage: "[Optional] The GPG/RSA key-pair name given in Artifactory. If not provided will create/use the default one` `", }, lcPathMappingPattern: cli.StringFlag{ Name: PathMappingPattern, From 60968299d24148ac172ddfe5ba5456a1d1020dab Mon Sep 17 00:00:00 2001 From: oshratz Date: Mon, 18 Nov 2024 12:30:29 +0200 Subject: [PATCH 3/4] Fix integration tests --- lifecycle_test.go | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/lifecycle_test.go b/lifecycle_test.go index b8e35d578..7444a82d0 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -29,14 +29,16 @@ import ( ) const ( - artifactoryLifecycleMinVersion = "7.68.3" - gpgKeyPairName = "lc-tests-key-pair" - lcTestdataPath = "lifecycle" - releaseBundlesSpec = "release-bundles-spec.json" - buildsSpec12 = "builds-spec-1-2.json" - buildsSpec3 = "builds-spec-3.json" - prodEnvironment = "PROD" - number1, number2, number3 = "111", "222", "333" + artifactoryLifecycleMinVersion = "7.68.3" + signingKeyOptionalArtifactoryMinVersion = "7.101.1" + gpgKeyPairName = "lc-tests-key-pair" + lcTestdataPath = "lifecycle" + releaseBundlesSpec = "release-bundles-spec.json" + buildsSpec12 = "builds-spec-1-2.json" + buildsSpec3 = "builds-spec-3.json" + prodEnvironment = "PROD" + number1, number2, number3 = "111", "222", "333" + withoutSigningKey = true ) var ( @@ -45,7 +47,7 @@ var ( ) func TestBackwardCompatibleReleaseBundleCreation(t *testing.T) { - cleanCallback := initLifecycleTest(t) + cleanCallback := initLifecycleTest(t, artifactoryLifecycleMinVersion) defer cleanCallback() lcManager := getLcServiceManager(t) @@ -95,14 +97,19 @@ func TestReleaseBundleCreationFromArtifacts(t *testing.T) { } func TestReleaseBundleCreationFromArtifactsWithoutSigningKey(t *testing.T) { - testReleaseBundleCreation(t, tests.UploadDevSpec, tests.LifecycleArtifacts, tests.GetExpectedLifecycleCreationByArtifacts(), true) + testReleaseBundleCreation(t, tests.UploadDevSpec, tests.LifecycleArtifacts, tests.GetExpectedLifecycleCreationByArtifacts(), withoutSigningKey) } func testReleaseBundleCreation(t *testing.T, uploadSpec, creationSpec string, expected []string, withoutKey bool) { - cleanCallback := initLifecycleTest(t) - defer cleanCallback() - lcManager := getLcServiceManager(t) + if withoutKey { + cleanCallback := initLifecycleTest(t, signingKeyOptionalArtifactoryMinVersion) + defer cleanCallback() + } else { + cleanCallback := initLifecycleTest(t, artifactoryLifecycleMinVersion) + defer cleanCallback() + } + lcManager := getLcServiceManager(t) specFile, err := tests.CreateSpec(uploadSpec) assert.NoError(t, err) runRt(t, "upload", "--spec="+specFile) @@ -114,7 +121,7 @@ func testReleaseBundleCreation(t *testing.T, uploadSpec, creationSpec string, ex } func TestLifecycleFullFlow(t *testing.T) { - cleanCallback := initLifecycleTest(t) + cleanCallback := initLifecycleTest(t, signingKeyOptionalArtifactoryMinVersion) defer cleanCallback() lcManager := getLcServiceManager(t) @@ -165,7 +172,7 @@ func TestLifecycleFullFlow(t *testing.T) { // Import bundles only work on onPerm platforms func TestImportReleaseBundle(t *testing.T) { - cleanCallback := initLifecycleTest(t) + cleanCallback := initLifecycleTest(t, artifactoryLifecycleMinVersion) defer cleanCallback() wd, err := os.Getwd() assert.NoError(t, err) @@ -370,11 +377,12 @@ func uploadBuildWithDeps(t *testing.T, buildName, buildNumber string) { runRt(t, "build-publish", buildName, buildNumber) } -func initLifecycleTest(t *testing.T) (cleanCallback func()) { +func initLifecycleTest(t *testing.T, minVersion string) (cleanCallback func()) { if !*tests.TestLifecycle { t.Skip("Skipping lifecycle test. To run release bundle test add the '-test.lc=true' option.") } - validateArtifactoryVersion(t, artifactoryLifecycleMinVersion) + + validateArtifactoryVersion(t, minVersion) if !isLifecycleSupported(t) { t.Skip("Skipping lifecycle test because the functionality is not enabled on the provided JPD.") From 7df8c5cebc7f89c3c0f00e49753016bd598a5fa9 Mon Sep 17 00:00:00 2001 From: oshratz Date: Wed, 20 Nov 2024 10:22:51 +0200 Subject: [PATCH 4/4] PR Fixes --- lifecycle_test.go | 14 +++++++------- utils/cliutils/commandsflags.go | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lifecycle_test.go b/lifecycle_test.go index 7444a82d0..ac66bcec5 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -100,8 +100,8 @@ func TestReleaseBundleCreationFromArtifactsWithoutSigningKey(t *testing.T) { testReleaseBundleCreation(t, tests.UploadDevSpec, tests.LifecycleArtifacts, tests.GetExpectedLifecycleCreationByArtifacts(), withoutSigningKey) } -func testReleaseBundleCreation(t *testing.T, uploadSpec, creationSpec string, expected []string, withoutKey bool) { - if withoutKey { +func testReleaseBundleCreation(t *testing.T, uploadSpec, creationSpec string, expected []string, withoutSigningKey bool) { + if withoutSigningKey { cleanCallback := initLifecycleTest(t, signingKeyOptionalArtifactoryMinVersion) defer cleanCallback() } else { @@ -114,7 +114,7 @@ func testReleaseBundleCreation(t *testing.T, uploadSpec, creationSpec string, ex assert.NoError(t, err) runRt(t, "upload", "--spec="+specFile) - createRbFromSpec(t, creationSpec, tests.LcRbName1, number1, true, withoutKey) + createRbFromSpec(t, creationSpec, tests.LcRbName1, number1, true, withoutSigningKey) defer deleteReleaseBundle(t, lcManager, tests.LcRbName1, number1) assertRbArtifacts(t, lcManager, tests.LcRbName1, number1, expected) @@ -209,13 +209,13 @@ func createRbBackwardCompatible(t *testing.T, specName, sourceOption, rbName, rb createRb(t, specFile, sourceOption, rbName, rbVersion, sync, false) } -func createRbFromSpec(t *testing.T, specName, rbName, rbVersion string, sync bool, withoutKey bool) { +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, withoutKey) + createRb(t, specFile, "spec", rbName, rbVersion, sync, withoutSigningKey) } -func createRb(t *testing.T, specFilePath, sourceOption, rbName, rbVersion string, sync bool, withoutKey bool) { +func createRb(t *testing.T, specFilePath, sourceOption, rbName, rbVersion string, sync bool, withoutSigningKey bool) { argsAndOptions := []string{ "rbc", rbName, @@ -223,7 +223,7 @@ func createRb(t *testing.T, specFilePath, sourceOption, rbName, rbVersion string getOption(sourceOption, specFilePath), } - if !withoutKey { + if !withoutSigningKey { argsAndOptions = append(argsAndOptions, getOption(cliutils.SigningKey, gpgKeyPairName)) } // Add the --sync option only if requested, to test the default value. diff --git a/utils/cliutils/commandsflags.go b/utils/cliutils/commandsflags.go index c4060f8f5..0157f6c13 100644 --- a/utils/cliutils/commandsflags.go +++ b/utils/cliutils/commandsflags.go @@ -1650,7 +1650,7 @@ var flagsMap = map[string]cli.Flag{ }, lcSigningKey: cli.StringFlag{ Name: SigningKey, - Usage: "[Optional] The GPG/RSA key-pair name given in Artifactory. If not provided will create/use the default one` `", + Usage: "[Optional] The GPG/RSA key-pair name given in Artifactory. If the key isn't provided, the command creates or uses the default key.` `", }, lcPathMappingPattern: cli.StringFlag{ Name: PathMappingPattern,