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.

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

(fix?) do not create a copy of helm index using index.Merge() #190

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

(fix?) do not create a copy of helm index using index.Merge() #190

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 14 additions & 12 deletions pkg/helm/helm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,28 +56,30 @@ func CreateOrUpdateHelmIndex(rootFs billy.Filesystem) error {
}

// UpdateIndex updates the original index with the new contents
func UpdateIndex(original, new *helmRepo.IndexFile) (*helmRepo.IndexFile, bool) {
// !! modifies newRepo in place and returns it
func UpdateIndex(origRepo, newRepo *helmRepo.IndexFile) (*helmRepo.IndexFile, bool) {
Copy link
Author

Choose a reason for hiding this comment

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

this does now modify newRepo in place which could cause side effects, but I vetted the current codebase and newRepo struct is never used after a call to UpdateIndex

// Create a copy of new to return
updatedIndex := helmRepo.NewIndexFile()
updatedIndex.Merge(new)
// !! This is not equivalent to creating a copy of the index file !!
// updatedIndex := helmRepo.NewIndexFile()
// updatedIndex.Merge(newRepo)
upToDate := true

// Preserve generated timestamp
updatedIndex.Generated = original.Generated
newRepo.Generated = origRepo.Generated

// Ensure newer version of chart is used if it has been updated
for chartName, chartVersions := range updatedIndex.Entries {
for chartName, chartVersions := range newRepo.Entries {
for i, chartVersion := range chartVersions {
version := chartVersion.Version
if !original.Has(chartName, version) {
if !origRepo.Has(chartName, version) {
// Keep the newly generated chart version as-is
upToDate = false
logrus.Debugf("Chart %s has introduced a new version %s: %v", chartName, chartVersion.Version, *chartVersion)
continue
}
// Get original chart version
var originalChartVersion *helmRepo.ChartVersion
for _, originalChartVersion = range original.Entries[chartName] {
for _, originalChartVersion = range origRepo.Entries[chartName] {
if originalChartVersion.Version == chartVersion.Version {
// found originalChartVersion, which must exist since we checked that the original has it
break
Expand All @@ -86,17 +88,17 @@ func UpdateIndex(original, new *helmRepo.IndexFile) (*helmRepo.IndexFile, bool)
// Try to preserve it only if nothing has changed.
if originalChartVersion.Digest == chartVersion.Digest {
// Don't modify created timestamp
updatedIndex.Entries[chartName][i].Created = originalChartVersion.Created
newRepo.Entries[chartName][i].Created = originalChartVersion.Created
} else {
upToDate = false
logrus.Debugf("Chart %s at version %s has been modified", chartName, originalChartVersion.Version)
}
}
}

for chartName, chartVersions := range original.Entries {
for chartName, chartVersions := range origRepo.Entries {
for _, chartVersion := range chartVersions {
if !updatedIndex.Has(chartName, chartVersion.Version) {
if !newRepo.Has(chartName, chartVersion.Version) {
Copy link
Author

@alexandreLamarre alexandreLamarre Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This still relies on the underling *helmrepo.Indexfile semver so the check of index.yaml being up to date doesn't work in the case where two versions 103.0.1 have different upstream tags

// Chart was removed
upToDate = false
logrus.Debugf("Chart %s at version %s has been removed: %v", chartName, chartVersion.Version, *chartVersion)
Expand All @@ -106,8 +108,8 @@ func UpdateIndex(original, new *helmRepo.IndexFile) (*helmRepo.IndexFile, bool)
}

// Sort and return entries
updatedIndex.SortEntries()
return updatedIndex, upToDate
newRepo.SortEntries()
return newRepo, upToDate
}

// OpenIndexYaml will check and open the index.yaml file in the local repository at the default file path
Expand Down
Loading