From 0355eaf218f2670fe9429b09290756f45340560b Mon Sep 17 00:00:00 2001 From: Bhanu Reddy Date: Mon, 2 Nov 2026 14:54:45 +0530 Subject: [PATCH 1/4] RTECO-2742 - Add Helm push E2E tests for chart versions with SemVer build metadata Helm pushes chart X.Y.Z+meta under the OCI tag X.Y.Z_meta. The tests check that jf helm push collects build info for such a chart: the module id keeps the real version, the artifacts and the build properties are in the '_' folder. Push errors are never skipped for these tests. Needs the jfrog-cli-artifactory fix from jfrog/jfrog-cli-artifactory#581. --- helm_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 7 deletions(-) diff --git a/helm_test.go b/helm_test.go index aa051ba33..bb51d0f2b 100644 --- a/helm_test.go +++ b/helm_test.go @@ -1090,22 +1090,34 @@ func TestHelmPushWithMultiLevelSubpath(t *testing.T) { } func runHelmSubpathPushTest(t *testing.T, subpath, buildName, chartName, chartVersion string) { + buildInfo := pushHelmChartAndGetBuildInfo(t, subpath, buildName, chartName, chartVersion, true) + + validateHelmBuildInfo(t, buildInfo, buildName, true, false) + assertOCIArtifactsUnderSubpath(t, buildInfo, + subpath+"/"+chartName+"/"+chartVersion) +} + +// pushHelmChartAndGetBuildInfo packages a test chart, pushes it with 'jf helm push' and returns the published build info. +// With tolerateRegistryFlakes, push errors that look like an unavailable OCI registry skip the test instead of failing it. +func pushHelmChartAndGetBuildInfo(t *testing.T, subpath, buildName, chartName, chartVersion string, tolerateRegistryFlakes bool) buildinfo.BuildInfo { const buildNumber = "1" chartFile, chartDir, restoreCwd := packageHelmChartForPush(t, chartName, chartVersion) - defer restoreCwd() + // Deferred calls run last-in first-out: restore the working directory before removing the chart directory, + // since Windows cannot remove the current working directory. defer func() { if err := os.RemoveAll(chartDir); err != nil { t.Logf("Warning: Failed to remove test chart directory %s: %v", chartDir, err) } }() + defer restoreCwd() registryHost, registryURL := helmSubpathRegistryURL(t, subpath) if !isRepoExist(tests.HelmLocalRepo) { t.Skipf("Repository %s does not exist. Skipping test.", tests.HelmLocalRepo) } if !loginHelmRegistryOrSkip(t, registryHost) { - return + t.SkipNow() } jfrogCli := coreTests.NewJfrogCli(execMain, "jfrog", "") @@ -1115,8 +1127,10 @@ func runHelmSubpathPushTest(t *testing.T, subpath, buildName, chartName, chartVe "--build-number=" + buildNumber, } if err := jfrogCli.Exec(pushArgs...); err != nil { - maybeSkipHelmPushFlake(t, err) - require.NoError(t, err, "helm push to subpath %q should succeed", subpath) + if tolerateRegistryFlakes { + maybeSkipHelmPushFlake(t, err) + } + require.NoError(t, err, "helm push of chart %s:%s to %q should succeed", chartName, chartVersion, subpath) } assert.NoError(t, artifactoryCli.Exec("bp", buildName, buildNumber)) @@ -1124,10 +1138,47 @@ func runHelmSubpathPushTest(t *testing.T, subpath, buildName, chartName, chartVe publishedBuildInfo, found, err := tests.GetBuildInfo(serverDetails, buildName, buildNumber) require.NoError(t, err, "Failed to get build info") require.True(t, found, "build info should be found") + return publishedBuildInfo.BuildInfo +} - validateHelmBuildInfo(t, publishedBuildInfo.BuildInfo, buildName, true, false) - assertOCIArtifactsUnderSubpath(t, publishedBuildInfo.BuildInfo, - subpath+"/"+chartName+"/"+chartVersion) +// TestHelmPushWithSemVerBuildMetadata verifies that 'jf helm push' collects build info for a chart version with +// SemVer build metadata (RTECO-2742). Helm stores such a version under the OCI tag with '+' replaced by '_', +// so the pushed artifacts must be found in the '_' folder, while the module id keeps the real version. +// Push errors are never skipped here: the regression surfaces as a push command failure. +func TestHelmPushWithSemVerBuildMetadata(t *testing.T) { + initHelmTest(t) + defer cleanHelmTest(t) + + testCases := []struct { + name string + subpath string + chartName string + chartVersion string + expectedFolder string + }{ + {"no subpath", "", "semver-meta-chart", "1.2.3+abc1234", "semver-meta-chart/1.2.3_abc1234"}, + {"with subpath", "org/team-delta", "semver-meta-sub-chart", "103.287.0+0b2041e", "org/team-delta/semver-meta-sub-chart/103.287.0_0b2041e"}, + } + for i, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + buildName := fmt.Sprintf("%s-push-semver-meta-%d", tests.HelmBuildName, i) + buildInfo := pushHelmChartAndGetBuildInfo(t, testCase.subpath, buildName, testCase.chartName, testCase.chartVersion, false) + + validateHelmBuildInfo(t, buildInfo, buildName, true, false) + module := buildInfo.Modules[0] + assert.Equal(t, testCase.chartName+":"+testCase.chartVersion, module.Id, "module id must keep the original chart version") + require.NotEmpty(t, module.Artifacts, "helm module must contain the pushed OCI artifacts") + assertOCIArtifactsUnderSubpath(t, buildInfo, testCase.expectedFolder) + + // The build properties are set on the same folder, so they must be found on its manifest. + serviceManager, err := utils.CreateServiceManager(serverDetails, 3, 1000, false) + require.NoError(t, err) + props, err := serviceManager.GetItemProps(tests.HelmLocalRepo + "/" + testCase.expectedFolder + "/manifest.json") + require.NoError(t, err) + require.NotNil(t, props, "manifest.json should exist under the '_' tag folder") + assert.Contains(t, props.Properties["build.name"], buildName) + }) + } } func TestHelmPushModuleCoexistenceAfterExistingModule(t *testing.T) { From 56c206cff98a14acab0fb6720a192155ccbfaa7e Mon Sep 17 00:00:00 2001 From: Bhanu Reddy Date: Mon, 2 Nov 2026 15:16:36 +0530 Subject: [PATCH 2/4] RTECO-2742 - Address review: clearer E2E checks, clean up builds, fix cleanup order Check build.number as well as build.name, delete the test builds, restore the working directory before removing the chart directory in the coexistence test, and make loginHelmRegistryOrSkip return nothing (it never returned false). --- helm_test.go | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/helm_test.go b/helm_test.go index bb51d0f2b..f596ceb74 100644 --- a/helm_test.go +++ b/helm_test.go @@ -1103,8 +1103,7 @@ func pushHelmChartAndGetBuildInfo(t *testing.T, subpath, buildName, chartName, c const buildNumber = "1" chartFile, chartDir, restoreCwd := packageHelmChartForPush(t, chartName, chartVersion) - // Deferred calls run last-in first-out: restore the working directory before removing the chart directory, - // since Windows cannot remove the current working directory. + // Restore the working directory before removing the chart directory: Windows cannot remove the current directory. defer func() { if err := os.RemoveAll(chartDir); err != nil { t.Logf("Warning: Failed to remove test chart directory %s: %v", chartDir, err) @@ -1116,9 +1115,7 @@ func pushHelmChartAndGetBuildInfo(t *testing.T, subpath, buildName, chartName, c if !isRepoExist(tests.HelmLocalRepo) { t.Skipf("Repository %s does not exist. Skipping test.", tests.HelmLocalRepo) } - if !loginHelmRegistryOrSkip(t, registryHost) { - t.SkipNow() - } + loginHelmRegistryOrSkip(t, registryHost) jfrogCli := coreTests.NewJfrogCli(execMain, "jfrog", "") pushArgs := []string{ @@ -1142,9 +1139,10 @@ func pushHelmChartAndGetBuildInfo(t *testing.T, subpath, buildName, chartName, c } // TestHelmPushWithSemVerBuildMetadata verifies that 'jf helm push' collects build info for a chart version with -// SemVer build metadata (RTECO-2742). Helm stores such a version under the OCI tag with '+' replaced by '_', +// SemVer build metadata. Helm stores such a version under the OCI tag with '+' replaced by '_', // so the pushed artifacts must be found in the '_' folder, while the module id keeps the real version. -// Push errors are never skipped here: the regression surfaces as a push command failure. +// A failed push is never skipped here, so the regression cannot pass as a skipped test. +// The usual skips for a missing repository or an unusable registry login still apply. func TestHelmPushWithSemVerBuildMetadata(t *testing.T) { initHelmTest(t) defer cleanHelmTest(t) @@ -1159,9 +1157,11 @@ func TestHelmPushWithSemVerBuildMetadata(t *testing.T) { {"no subpath", "", "semver-meta-chart", "1.2.3+abc1234", "semver-meta-chart/1.2.3_abc1234"}, {"with subpath", "org/team-delta", "semver-meta-sub-chart", "103.287.0+0b2041e", "org/team-delta/semver-meta-sub-chart/103.287.0_0b2041e"}, } - for i, testCase := range testCases { + for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - buildName := fmt.Sprintf("%s-push-semver-meta-%d", tests.HelmBuildName, i) + buildName := tests.HelmBuildName + "-push-" + testCase.chartName + inttestutils.DeleteBuild(serverDetails.ArtifactoryUrl, buildName, artHttpDetails) + defer inttestutils.DeleteBuild(serverDetails.ArtifactoryUrl, buildName, artHttpDetails) buildInfo := pushHelmChartAndGetBuildInfo(t, testCase.subpath, buildName, testCase.chartName, testCase.chartVersion, false) validateHelmBuildInfo(t, buildInfo, buildName, true, false) @@ -1175,8 +1175,9 @@ func TestHelmPushWithSemVerBuildMetadata(t *testing.T) { require.NoError(t, err) props, err := serviceManager.GetItemProps(tests.HelmLocalRepo + "/" + testCase.expectedFolder + "/manifest.json") require.NoError(t, err) - require.NotNil(t, props, "manifest.json should exist under the '_' tag folder") + require.NotNil(t, props, "manifest.json under the '_' tag folder has no properties") assert.Contains(t, props.Properties["build.name"], buildName) + assert.Contains(t, props.Properties["build.number"], "1") }) } } @@ -1203,20 +1204,18 @@ func TestHelmPushModuleCoexistenceAfterExistingModule(t *testing.T) { ), "rt build-add-dependencies should seed the build with a non-helm module") chartFile, chartDir, restoreCwd := packageHelmChartForPush(t, chartName, chartVersion) - defer restoreCwd() defer func() { if err := os.RemoveAll(chartDir); err != nil { t.Logf("Warning: Failed to remove test chart directory %s: %v", chartDir, err) } }() + defer restoreCwd() registryHost, registryURL := helmSubpathRegistryURL(t, "") if !isRepoExist(tests.HelmLocalRepo) { t.Skipf("Repository %s does not exist. Skipping test.", tests.HelmLocalRepo) } - if !loginHelmRegistryOrSkip(t, registryHost) { - return - } + loginHelmRegistryOrSkip(t, registryHost) jfrogCli := coreTests.NewJfrogCli(execMain, "jfrog", "") pushArgs := []string{ @@ -1293,14 +1292,13 @@ func helmSubpathRegistryURL(t *testing.T, subpath string) (registryHost, registr return registryHost, registryURL } -func loginHelmRegistryOrSkip(t *testing.T, registryHost string) bool { +func loginHelmRegistryOrSkip(t *testing.T, registryHost string) { err := loginHelmRegistry(t, registryHost) if err == nil { - return true + return } maybeSkipHelmLoginFlake(t, err) require.NoError(t, err, "helm registry login should succeed") - return false } func maybeSkipHelmLoginFlake(t *testing.T, err error) { From ffb526a87af22d9cdca9b70b07dad5688bed63bd Mon Sep 17 00:00:00 2001 From: Bhanu Reddy Date: Mon, 2 Nov 2026 15:43:06 +0530 Subject: [PATCH 3/4] RTECO-2742 - Name the flake-policy arguments of the helm push helper --- helm_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/helm_test.go b/helm_test.go index f596ceb74..4e30557bb 100644 --- a/helm_test.go +++ b/helm_test.go @@ -1090,13 +1090,19 @@ func TestHelmPushWithMultiLevelSubpath(t *testing.T) { } func runHelmSubpathPushTest(t *testing.T, subpath, buildName, chartName, chartVersion string) { - buildInfo := pushHelmChartAndGetBuildInfo(t, subpath, buildName, chartName, chartVersion, true) + buildInfo := pushHelmChartAndGetBuildInfo(t, subpath, buildName, chartName, chartVersion, skipOnRegistryFlake) validateHelmBuildInfo(t, buildInfo, buildName, true, false) assertOCIArtifactsUnderSubpath(t, buildInfo, subpath+"/"+chartName+"/"+chartVersion) } +// Values of the tolerateRegistryFlakes argument of pushHelmChartAndGetBuildInfo. +const ( + skipOnRegistryFlake = true + failOnAnyPushError = false +) + // pushHelmChartAndGetBuildInfo packages a test chart, pushes it with 'jf helm push' and returns the published build info. // With tolerateRegistryFlakes, push errors that look like an unavailable OCI registry skip the test instead of failing it. func pushHelmChartAndGetBuildInfo(t *testing.T, subpath, buildName, chartName, chartVersion string, tolerateRegistryFlakes bool) buildinfo.BuildInfo { @@ -1162,7 +1168,7 @@ func TestHelmPushWithSemVerBuildMetadata(t *testing.T) { buildName := tests.HelmBuildName + "-push-" + testCase.chartName inttestutils.DeleteBuild(serverDetails.ArtifactoryUrl, buildName, artHttpDetails) defer inttestutils.DeleteBuild(serverDetails.ArtifactoryUrl, buildName, artHttpDetails) - buildInfo := pushHelmChartAndGetBuildInfo(t, testCase.subpath, buildName, testCase.chartName, testCase.chartVersion, false) + buildInfo := pushHelmChartAndGetBuildInfo(t, testCase.subpath, buildName, testCase.chartName, testCase.chartVersion, failOnAnyPushError) validateHelmBuildInfo(t, buildInfo, buildName, true, false) module := buildInfo.Modules[0] From 00023d6cce8048e1cc32f82385f370a6d5f71bb7 Mon Sep 17 00:00:00 2001 From: Bhanu Reddy Date: Tue, 3 Nov 2026 12:05:05 +0530 Subject: [PATCH 4/4] RTECO-2742 - Update jfrog-cli-artifactory to the commit with the SemVer build metadata fix jfrog-cli-artifactory main now has the fix (jfrog/jfrog-cli-artifactory#584), so TestHelmPushWithSemVerBuildMetadata can pass. --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 651c0f30d..ae4b72103 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/jfrog/build-info-go v1.13.1-0.20261028123549-f0cd26bfc2de github.com/jfrog/gofrog v1.7.7 github.com/jfrog/jfrog-cli-application v1.0.2-0.20260917110731-d7783acedd8c - github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20261101122816-c0098229a376 + github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20261103060251-30c80a7deec4 github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20261102064148-e4c77ed13cc6 github.com/jfrog/jfrog-cli-evidence v0.11.1-0.20261026075204-fdf3635939c5 github.com/jfrog/jfrog-cli-platform-services v1.10.1-0.20261030162302-b4948e3a2cb1 diff --git a/go.sum b/go.sum index ecd1a0d27..830ae1c34 100644 --- a/go.sum +++ b/go.sum @@ -402,8 +402,8 @@ github.com/jfrog/jfrog-apps-config v1.0.1 h1:mtv6k7g8A8BVhlHGlSveapqf4mJfonwvXYL github.com/jfrog/jfrog-apps-config v1.0.1/go.mod h1:8AIIr1oY9JuH5dylz2S6f8Ym2MaadPLR6noCBO4C22w= github.com/jfrog/jfrog-cli-application v1.0.2-0.20260917110731-d7783acedd8c h1:B/svbJcwCMHjeC+HqpzH9yEC7GEvOj3HPotYpG4Ga7A= github.com/jfrog/jfrog-cli-application v1.0.2-0.20260917110731-d7783acedd8c/go.mod h1:RHjDVsgfIE+6N49R1t1nBjbK2Je4VTN7CQm9OlwtFnQ= -github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20261101122816-c0098229a376 h1:KSmFFkmKixtromy54jI0MM5dZ8NIxGi+0sm/he2YXA4= -github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20261101122816-c0098229a376/go.mod h1:94/aLloMSK6avu4umcJS3DJ6IuGM5K10PwZ6kDqgzSo= +github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20261103060251-30c80a7deec4 h1:Z4WqOlZZe3NA02ekqFrbSNlO8hkdGPHn/PuWq1sb+CA= +github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20261103060251-30c80a7deec4/go.mod h1:94/aLloMSK6avu4umcJS3DJ6IuGM5K10PwZ6kDqgzSo= github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20261102064148-e4c77ed13cc6 h1:7Jdj+FiaMN5jiQ94+Xlehf9OSLqSbAqAvO44XB9d7E4= github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20261102064148-e4c77ed13cc6/go.mod h1:dxy3GflCOX9kGxcdEkCXWhV+EMUAGBWO+CY5fGjWcCQ= github.com/jfrog/jfrog-cli-evidence v0.11.1-0.20261026075204-fdf3635939c5 h1:PpGuelKEDO9ktDr90lCTDpg5jMMBuG1lS8Q4mhgegY0=