Skip to content

fix(helm): separate build-info modules by type #427

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.

Sign up for GitHub

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

Open
vjda wants to merge 3 commits into
base: main
Choose a base branch
from
+348 −15
Open

fix(helm): separate build-info modules by type #427

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9b9c13
fix(helm): separate build-info modules by type
vjda Apr 16, 2026
46c8de6
fix(helm): ensure Helm module exists before appending artifacts
vjda Apr 21, 2026
63aa6f3
Merge branch 'main' into fix/jf-helm-oci-build-module-merge-fix
vjda Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion artifactory/commands/helm/push.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func handlePushCommand(buildInfo *entities.BuildInfo, helmArgs []string, service
for _, artLayer := range artifactsLayers {
artifacts = append(artifacts, artLayer.ToArtifact())
}
addArtifactsInBuildInfo(buildInfo, artifacts, chartName, chartVersion)
addArtifactsInBuildInfo(buildInfo, artifacts, chartName, chartVersion, entities.ModuleType("helm"))
removeDuplicateArtifacts(buildInfo)
Comment thread
vjda marked this conversation as resolved.
return saveBuildInfo(buildInfo, buildName, buildNumber, project)
}
Expand Down
6 changes: 3 additions & 3 deletions artifactory/commands/helm/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,13 @@ func removeDuplicateDependencies(buildInfo *entities.BuildInfo) {
}
}

func addArtifactsInBuildInfo(buildInfo *entities.BuildInfo, artifacts []entities.Artifact, chartName, chartVersion string) {
func addArtifactsInBuildInfo(buildInfo *entities.BuildInfo, artifacts []entities.Artifact, chartName, chartVersion string, moduleType entities.ModuleType) {
if buildInfo == nil {
return
}
moduleId := fmt.Sprintf("%s:%s", chartName, chartVersion)
for moduleIdx, module := range buildInfo.Modules {
if module.Id == moduleId {
if module.Type == moduleType && module.Id == moduleId {
module.Artifacts = append(module.Artifacts, artifacts...)
buildInfo.Modules[moduleIdx] = module
Comment thread
vjda marked this conversation as resolved.
}
Expand Down Expand Up @@ -192,7 +192,7 @@ func appendModuleInExistingBuildInfo(buildInfo *entities.BuildInfo, moduleToAdd
return
}
for moduleIdx, module := range buildInfo.Modules {
if module.Id == moduleToAdd.Id {
if module.Type == moduleToAdd.Type && module.Id == moduleToAdd.Id {
dependencies := moduleToAdd.Dependencies
if len(dependencies) > 0 {
buildInfo.Modules[moduleIdx].Dependencies = append(buildInfo.Modules[moduleIdx].Dependencies, dependencies...)
Expand Down
283 changes: 279 additions & 4 deletions artifactory/commands/helm/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ func TestRemoveDuplicateDependencies(t *testing.T) {

func TestAddArtifactsInBuildInfo(t *testing.T) {
t.Run("Nil build info", func(t *testing.T) {
addArtifactsInBuildInfo(nil, []entities.Artifact{}, "chart", "1.0.0")
addArtifactsInBuildInfo(nil, []entities.Artifact{}, "chart", "1.0.0", entities.ModuleType("helm"))
})

t.Run("Add artifacts to matching module", func(t *testing.T) {
Expand All @@ -379,7 +379,7 @@ func TestAddArtifactsInBuildInfo(t *testing.T) {
{Name: "artifact1", Checksum: entities.Checksum{Sha256: "sha1"}},
{Name: "artifact2", Checksum: entities.Checksum{Sha256: "sha2"}},
}
addArtifactsInBuildInfo(buildInfo, artifacts, "chart", "1.0.0")
addArtifactsInBuildInfo(buildInfo, artifacts, "chart", "1.0.0", entities.ModuleType("helm"))
assert.Len(t, buildInfo.Modules[0].Artifacts, 2)
assert.Equal(t, "artifact1", buildInfo.Modules[0].Artifacts[0].Name)
assert.Equal(t, "artifact2", buildInfo.Modules[0].Artifacts[1].Name)
Expand All @@ -398,7 +398,7 @@ func TestAddArtifactsInBuildInfo(t *testing.T) {
artifacts := []entities.Artifact{
{Name: "artifact1", Checksum: entities.Checksum{Sha256: "sha1"}},
}
addArtifactsInBuildInfo(buildInfo, artifacts, "chart", "1.0.0")
addArtifactsInBuildInfo(buildInfo, artifacts, "chart", "1.0.0", entities.ModuleType("helm"))
assert.Len(t, buildInfo.Modules[0].Artifacts, 0)
})

Expand All @@ -418,14 +418,76 @@ func TestAddArtifactsInBuildInfo(t *testing.T) {
{Name: "new1", Checksum: entities.Checksum{Sha256: "sha1"}},
{Name: "new2", Checksum: entities.Checksum{Sha256: "sha2"}},
}
addArtifactsInBuildInfo(buildInfo, artifacts, "chart", "1.0.0")
addArtifactsInBuildInfo(buildInfo, artifacts, "chart", "1.0.0", entities.ModuleType("helm"))
assert.Len(t, buildInfo.Modules[0].Artifacts, 3)
assert.Equal(t, "existing", buildInfo.Modules[0].Artifacts[0].Name)
assert.Equal(t, "new1", buildInfo.Modules[0].Artifacts[1].Name)
assert.Equal(t, "new2", buildInfo.Modules[0].Artifacts[2].Name)
})
}

func TestAddArtifactsInBuildInfoUsesModuleTypeIdentity(t *testing.T) {
tests := []struct {
name string
moduleType entities.ModuleType
buildInfo *entities.BuildInfo
expectedArtifacts map[string][]string
expectedModuleCount int
}{
{
name: "same id and same type appends artifacts",
moduleType: entities.ModuleType("helm"),
buildInfo: &entities.BuildInfo{Modules: []entities.Module{{
Id: "chart:1.0.0",
Type: entities.ModuleType("helm"),
Artifacts: []entities.Artifact{
{Name: "existing", Checksum: entities.Checksum{Sha256: "sha-existing"}},
},
}}},
expectedArtifacts: map[string][]string{
"helm|chart:1.0.0": {"existing", "new-artifact"},
},
expectedModuleCount: 1,
},
{
name: "same id and different type does not merge",
moduleType: entities.ModuleType("helm"),
buildInfo: &entities.BuildInfo{Modules: []entities.Module{
{
Id: "chart:1.0.0",
Type: entities.ModuleType("docker"),
Artifacts: []entities.Artifact{
{Name: "docker-artifact", Checksum: entities.Checksum{Sha256: "sha-docker"}},
},
},
{
Id: "chart:1.0.0",
Type: entities.ModuleType("helm"),
Artifacts: []entities.Artifact{
{Name: "helm-artifact", Checksum: entities.Checksum{Sha256: "sha-helm"}},
},
},
}},
expectedArtifacts: map[string][]string{
"docker|chart:1.0.0": {"docker-artifact"},
"helm|chart:1.0.0": {"helm-artifact", "new-artifact"},
},
expectedModuleCount: 2,
},
}

artifactsToAdd := []entities.Artifact{{Name: "new-artifact", Checksum: entities.Checksum{Sha256: "sha-new"}}}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
addArtifactsInBuildInfo(tt.buildInfo, artifactsToAdd, "chart", "1.0.0", tt.moduleType)

assert.Len(t, tt.buildInfo.Modules, tt.expectedModuleCount)
assertModuleArtifactsByTypeAndID(t, tt.buildInfo.Modules, tt.expectedArtifacts)
})
}
}

func TestRemoveDuplicateArtifacts(t *testing.T) {
t.Run("Nil build info", func(t *testing.T) {
removeDuplicateArtifacts(nil)
Expand Down Expand Up @@ -642,3 +704,216 @@ func TestAppendModuleInExistingBuildInfo(t *testing.T) {
assert.Equal(t, "new1", buildInfo.Modules[0].Artifacts[0].Name)
})
}

func TestAppendModuleInExistingBuildInfoUsesModuleTypeIdentity(t *testing.T) {
tests := []struct {
name string
initialModules []entities.Module
moduleToAdd entities.Module
expectedModules []moduleExpectation
expectedModuleCount int
}{
{
name: "same id and same type merges into existing module",
initialModules: []entities.Module{{
Id: "chart:1.0.0",
Type: entities.ModuleType("helm"),
Artifacts: []entities.Artifact{{Name: "old-artifact", Checksum: entities.Checksum{Sha256: "sha-old"}}},
Dependencies: []entities.Dependency{{Id: "dep-old", Checksum: entities.Checksum{Sha256: "dep-sha-old"}}},
}},
moduleToAdd: entities.Module{
Id: "chart:1.0.0",
Type: entities.ModuleType("helm"),
Artifacts: []entities.Artifact{{Name: "new-artifact", Checksum: entities.Checksum{Sha256: "sha-new"}}},
Dependencies: []entities.Dependency{{Id: "dep-new", Checksum: entities.Checksum{Sha256: "dep-sha-new"}}},
},
expectedModules: []moduleExpectation{{
Type: entities.ModuleType("helm"),
ID: "chart:1.0.0",
ArtifactNames: []string{"new-artifact"},
DependencyIDs: []string{"dep-old", "dep-new"},
}},
expectedModuleCount: 1,
},
{
name: "same id and different type stays separate docker to helm",
initialModules: []entities.Module{{
Id: "img:1.0",
Type: entities.ModuleType("docker"),
Artifacts: []entities.Artifact{{Name: "docker-artifact", Checksum: entities.Checksum{Sha256: "sha-docker"}}},
}},
moduleToAdd: entities.Module{
Id: "img:1.0",
Type: entities.ModuleType("helm"),
Artifacts: []entities.Artifact{{Name: "helm-artifact", Checksum: entities.Checksum{Sha256: "sha-helm"}}},
},
expectedModules: []moduleExpectation{
{Type: entities.ModuleType("docker"), ID: "img:1.0", ArtifactNames: []string{"docker-artifact"}},
{Type: entities.ModuleType("helm"), ID: "img:1.0", ArtifactNames: []string{"helm-artifact"}},
},
expectedModuleCount: 2,
},
{
name: "same id and different type stays separate helm to docker",
initialModules: []entities.Module{{
Id: "img:1.0",
Type: entities.ModuleType("helm"),
Artifacts: []entities.Artifact{{Name: "helm-artifact", Checksum: entities.Checksum{Sha256: "sha-helm"}}},
}},
moduleToAdd: entities.Module{
Id: "img:1.0",
Type: entities.ModuleType("docker"),
Artifacts: []entities.Artifact{{Name: "docker-artifact", Checksum: entities.Checksum{Sha256: "sha-docker"}}},
},
expectedModules: []moduleExpectation{
{Type: entities.ModuleType("helm"), ID: "img:1.0", ArtifactNames: []string{"helm-artifact"}},
{Type: entities.ModuleType("docker"), ID: "img:1.0", ArtifactNames: []string{"docker-artifact"}},
},
expectedModuleCount: 2,
},
{
name: "empty type matches empty type",
initialModules: []entities.Module{{
Id: "x:1",
Type: entities.ModuleType(""),
Artifacts: []entities.Artifact{{Name: "old-empty", Checksum: entities.Checksum{Sha256: "sha-empty-old"}}},
}},
moduleToAdd: entities.Module{
Id: "x:1",
Type: entities.ModuleType(""),
Artifacts: []entities.Artifact{{Name: "new-empty", Checksum: entities.Checksum{Sha256: "sha-empty-new"}}},
},
expectedModules: []moduleExpectation{{Type: entities.ModuleType(""), ID: "x:1", ArtifactNames: []string{"new-empty"}}},
expectedModuleCount: 1,
},
{
name: "empty type does not match non empty type",
initialModules: []entities.Module{{
Id: "x:1",
Type: entities.ModuleType("docker"),
Artifacts: []entities.Artifact{{Name: "docker-artifact", Checksum: entities.Checksum{Sha256: "sha-docker"}}},
}},
moduleToAdd: entities.Module{
Id: "x:1",
Type: entities.ModuleType(""),
Artifacts: []entities.Artifact{{Name: "empty-artifact", Checksum: entities.Checksum{Sha256: "sha-empty"}}},
},
expectedModules: []moduleExpectation{
{Type: entities.ModuleType("docker"), ID: "x:1", ArtifactNames: []string{"docker-artifact"}},
{Type: entities.ModuleType(""), ID: "x:1", ArtifactNames: []string{"empty-artifact"}},
},
expectedModuleCount: 2,
},
{
name: "order independence docker then helm",
initialModules: []entities.Module{{
Id: "img:1.0",
Type: entities.ModuleType("docker"),
Artifacts: []entities.Artifact{{Name: "docker-artifact", Checksum: entities.Checksum{Sha256: "sha-docker"}}},
}},
moduleToAdd: entities.Module{
Id: "img:1.0",
Type: entities.ModuleType("helm"),
Artifacts: []entities.Artifact{{Name: "helm-artifact", Checksum: entities.Checksum{Sha256: "sha-helm"}}},
},
expectedModules: []moduleExpectation{
{Type: entities.ModuleType("docker"), ID: "img:1.0", ArtifactNames: []string{"docker-artifact"}},
{Type: entities.ModuleType("helm"), ID: "img:1.0", ArtifactNames: []string{"helm-artifact"}},
},
expectedModuleCount: 2,
},
{
name: "order independence helm then docker",
initialModules: []entities.Module{{
Id: "img:1.0",
Type: entities.ModuleType("helm"),
Artifacts: []entities.Artifact{{Name: "helm-artifact", Checksum: entities.Checksum{Sha256: "sha-helm"}}},
}},
moduleToAdd: entities.Module{
Id: "img:1.0",
Type: entities.ModuleType("docker"),
Artifacts: []entities.Artifact{{Name: "docker-artifact", Checksum: entities.Checksum{Sha256: "sha-docker"}}},
},
expectedModules: []moduleExpectation{
{Type: entities.ModuleType("helm"), ID: "img:1.0", ArtifactNames: []string{"helm-artifact"}},
{Type: entities.ModuleType("docker"), ID: "img:1.0", ArtifactNames: []string{"docker-artifact"}},
},
expectedModuleCount: 2,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
buildInfo := &entities.BuildInfo{Modules: append([]entities.Module(nil), tt.initialModules...)}
moduleToAdd := tt.moduleToAdd

appendModuleInExistingBuildInfo(buildInfo, &moduleToAdd)

assert.Len(t, buildInfo.Modules, tt.expectedModuleCount)
assertModulesByTypeAndID(t, buildInfo.Modules, tt.expectedModules)
})
}
}

type moduleExpectation struct {
Type entities.ModuleType
ID string
ArtifactNames []string
DependencyIDs []string
}

func assertModulesByTypeAndID(t *testing.T, modules []entities.Module, expected []moduleExpectation) {
t.Helper()

assert.Len(t, modules, len(expected))
for _, expectedModule := range expected {
var matched *entities.Module
for i := range modules {
if modules[i].Type == expectedModule.Type && modules[i].Id == expectedModule.ID {
matched = &modules[i]
break
}
}
if assert.NotNil(t, matched, "module %s|%s should exist", expectedModule.Type, expectedModule.ID) {
assert.Equal(t, expectedModule.ArtifactNames, artifactNames(matched.Artifacts))
assert.Equal(t, expectedModule.DependencyIDs, dependencyIDs(matched.Dependencies))
}
}
}

func assertModuleArtifactsByTypeAndID(t *testing.T, modules []entities.Module, expected map[string][]string) {
t.Helper()

assert.Len(t, modules, len(expected))
for key, names := range expected {
matched := false
for i := range modules {
moduleKey := string(modules[i].Type) + "|" + modules[i].Id
if moduleKey == key {
assert.Equal(t, names, artifactNames(modules[i].Artifacts))
matched = true
break
}
}
assert.True(t, matched, "module %s should exist", key)
}
}

func artifactNames(artifacts []entities.Artifact) []string {
names := make([]string, 0, len(artifacts))
for _, artifact := range artifacts {
names = append(names, artifact.Name)
}
return names
}

func dependencyIDs(dependencies []entities.Dependency) []string {
if len(dependencies) == 0 {
return nil
}
ids := make([]string, 0, len(dependencies))
for _, dependency := range dependencies {
ids = append(ids, dependency.Id)
}
return ids
}
Loading