don't duplicate pre-release charts in index.yaml

Prior to this commit chart-releaser would incorrectly parse pre-release
versions, e.g. v25.1.1-beta2, due to naively splitting on the last
index. This would result in calling `IndexFile.Get` with just `beta2`
and trigger a reinsertion of the pre-release version upon every call to
`cr index`.

This commit removes the file name based parsing altogether and instead
preemptively parses the chart metadata to avoid such complications all
together.

It should be noted this commit highlights a pre-existing issue more
clearly but intentionally does not fix it: chart-releaser can *not*
update an existing index.yaml entry.

Signed-off-by: Chris Seto <[email protected]>

Author: chrisseto

pkg/releaser/releaser.go

@@ -168,14 +168,13 @@ func (r *Releaser) UpdateIndexFile() (bool, error) {
 			if filepath.Ext(name) != chartAssetFileExtension {
 				continue
 			}
-			baseName := strings.TrimSuffix(name, filepath.Ext(name))
-			tagParts := r.splitPackageNameAndVersion(baseName)
-			packageName, packageVersion := tagParts[0], tagParts[1]
-			fmt.Printf("Found %s-%s.tgz\n", packageName, packageVersion)
-			if _, err := indexFile.Get(packageName, packageVersion); err != nil {
-				if err := r.addToIndexFile(indexFile, downloadURL.String()); err != nil {
-					return false, err
-				}
+			fmt.Printf("Found %s\n", name)
+			updated, err := r.maybeAddToIndexFile(indexFile, downloadURL.String())
+			if err != nil {
+				return false, err
+			}
+
+			if updated {
 				update = true
 				break
 			}
@@ -255,20 +254,26 @@ func (r *Releaser) splitPackageNameAndVersion(pkg string) []string {
 	return []string{pkg[0:delimIndex], pkg[delimIndex+1:]}
 }
 
-func (r *Releaser) addToIndexFile(indexFile *repo.IndexFile, url string) error {
+func (r *Releaser) maybeAddToIndexFile(indexFile *repo.IndexFile, url string) (bool, error) {
 	arch := filepath.Join(r.config.PackagePath, filepath.Base(url))
 
 	// extract chart metadata
 	fmt.Printf("Extracting chart metadata from %s\n", arch)
 	c, err := loader.LoadFile(arch)
 	if err != nil {
-		return errors.Wrapf(err, "%s is not a helm chart package", arch)
+		return false, errors.Wrapf(err, "%s is not a helm chart package", arch)
 	}
+
+	if _, err := indexFile.Get(c.Name(), c.Metadata.Version); err == nil {
+		fmt.Printf("version %s of %s already present in index file", c.Metadata.Version, c.Name())
+		return false, nil
+	}
+
 	// calculate hash
 	fmt.Printf("Calculating Hash for %s\n", arch)
 	hash, err := provenance.DigestFile(arch)
 	if err != nil {
-		return err
+		return false, err
 	}
 
 	// remove url name from url as helm's index library
@@ -284,7 +289,7 @@ func (r *Releaser) addToIndexFile(indexFile *repo.IndexFile, url string) error {
 	}
 
 	// Add to index
-	return indexFile.MustAdd(c.Metadata, filepath.Base(arch), strings.Join(s, "/"), hash)
+	return true, indexFile.MustAdd(c.Metadata, filepath.Base(arch), strings.Join(s, "/"), hash)
 }
 
 // CreateReleases finds and uploads Helm chart packages to GitHub

pkg/releaser/releaser_test.go

@@ -246,44 +246,7 @@ func TestReleaser_UpdateIndexFileGenerated(t *testing.T) {
 	}
 }
 
-func TestReleaser_splitPackageNameAndVersion(t *testing.T) {
-	tests := []struct {
-		name     string
-		pkg      string
-		expected []string
-	}{
-		{
-			"no-hyphen",
-			"foo",
-			nil,
-		},
-		{
-			"one-hyphen",
-			"foo-1.2.3",
-			[]string{"foo", "1.2.3"},
-		},
-		{
-			"two-hyphens",
-			"foo-bar-1.2.3",
-			[]string{"foo-bar", "1.2.3"},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			r := &Releaser{}
-			if tt.expected == nil {
-				assert.Panics(t, func() {
-					r.splitPackageNameAndVersion(tt.pkg)
-				}, "slice bounds out of range")
-			} else {
-				actual := r.splitPackageNameAndVersion(tt.pkg)
-				assert.Equal(t, tt.expected, actual)
-			}
-		})
-	}
-}
-
-func TestReleaser_addToIndexFile(t *testing.T) {
+func TestReleaser_maybeAddToIndexFile(t *testing.T) {
 	tests := []struct {
 		name              string
 		chart             string
@@ -336,20 +299,28 @@ func TestReleaser_addToIndexFile(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			indexFile := repo.NewIndexFile()
 			url := fmt.Sprintf("https://myrepo/charts/%s-%s.tgz", tt.chart, tt.version)
-			err := tt.releaser.addToIndexFile(indexFile, url)
+			updated, err := tt.releaser.maybeAddToIndexFile(indexFile, url)
 			if tt.error {
 				assert.Error(t, err)
+				assert.False(t, updated)
 				assert.False(t, indexFile.Has(tt.chart, tt.version))
-			} else {
-				assert.True(t, indexFile.Has(tt.chart, tt.version))
+				return
+			}
 
-				indexEntry, _ := indexFile.Get(tt.chart, tt.version)
-				if tt.packagesWithIndex {
-					assert.Equal(t, filepath.Base(url), indexEntry.URLs[0])
-				} else {
-					assert.Equal(t, url, indexEntry.URLs[0])
-				}
+			assert.True(t, updated)
+			assert.True(t, indexFile.Has(tt.chart, tt.version))
+
+			indexEntry, _ := indexFile.Get(tt.chart, tt.version)
+			if tt.packagesWithIndex {
+				assert.Equal(t, filepath.Base(url), indexEntry.URLs[0])
+			} else {
+				assert.Equal(t, url, indexEntry.URLs[0])
 			}
+
+			// Second time around is a no-op.
+			updated, err = tt.releaser.maybeAddToIndexFile(indexFile, url)
+			assert.NoError(t, err)
+			assert.False(t, updated)
 		})
 	}
 }