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

[alexandreLamarre]

Fixes #100

Why doesn't the current iteration support multiple upstream chart rcs?

Let's use these to example versions:

• 106.0.0+up5.0.0-rc.1
• 106.0.0+up5.0.0-rc.2

Semantically for rancher these would indicate that we want to release a chart for the 2.11 line, with the chart patches being the same (i.e.) 106.0.0, but with different upstream charts, which technically makes them different charts.

However using semver, these are actually treated as the same version, but with different build metadata : up5.0.0-rc.1 versus : up5.0.0-rc.2

---

When we generate the new index file from assets path :

// Generate the current index file from the assets/ directory
	newHelmIndexFile, err := helmRepo.IndexDirectory(absRepositoryAssetsDir, path.RepositoryAssetsDir)
	if err != nil {
		return fmt.Errorf("encountered error while trying to generate new Helm index: %s", err)
	}

we do correctly pickup both 106.0.0+up5.0.0-rc.1 106.0.0+up5.0.0-rc.2

so what happens next? When it comes time to update the index, we call :

// Update index
	helmIndexFile, upToDate := UpdateIndex(helmIndexFile, newHelmIndexFile)

which in turn calls (which is equivalent to the upstream helm command helm index merge ...):

	updatedIndex := helmRepo.NewIndexFile()
	updatedIndex.Merge(newRepo)

to use as the new index. BUT as we iterate and merge into the empty index, we check that there are no equivalent semver versions. So what ends up happening is 106.0.0+up5.0.0-rc.1 gets add to the update index but 106.0.0+up5.0.0-rc.2 correctly does not get added to the update index because both 106.0.0+up5.0.0-rc.1,106.0.0+up5.0.0-rc.2 are semantically equivalent (as expected):

package main

import (
	"fmt"

	"github.com/Masterminds/semver/v3"
)

func main() {
	constraint, err := semver.NewConstraint("106.0.0+up5.0.0-rc.1")
	if err != nil {
		panic(err)
	}
	fmt.Println(constraint.Check(semver.MustParse("106.0.0+up5.0.0-rc.2")))
}

true

--

I guess the question becomes should we even allow 106.0.0+up5.0.0-rc.1,106.0.0+up5.0.0-rc.2 to be released at the same time?

If we don't it makes it impossible to release two charts with the same base version like 106.0.0, which could be relevant for charts like rancher-monitoring and prometheus-federator where we may want to release two upstream versions using the same set of patches.

In the existing codebase, we can work around that problem like:

106.0.0+up5.0.0
106.0.1+up5.1.0

but we can get into weird versioning scenarios where if we want to update the set of patches to particular upstream charts

106.0.0+up5.0.0
106.0.1+up5.1.0
106.0.2+up5.0.0
106.0.3+up5.1.0

which I'm not against at all, but we should document. That way of doing things also becomes VERY time consuming and can be confusing as multiple set of patches from different people come in.

---

If this is determined a won't-fix as it likely should, I'd at least like to add some validation logic to check that all entries in assets path are actually in index.yaml to make PRs that do updates like 106.0.0+up5.0.0-rc.1->106.0.0+up5.0.0-rc.2 don't accidentally end up with a missing index.yaml entry if we don't remove 106.0.0+up5.0.0-rc.1

[alexandreLamarre]

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

[nicholasSUSE]

Ok, this is an incredible work.

Before merging, I would like to test some stuff on this branch.

[alexandreLamarre]

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