fix: incorrect tag selection in arkade chart upgrade

Signed-off-by: Nana Kwadwo <agyemang.nana.b@gmail.com>

cmd/chart/chart.go

@@ -25,7 +25,7 @@ func MakeChart() *cobra.Command {
  }
 
  command.AddCommand(MakeVerify())
- command.AddCommand(MakeUpgrade())
+ command.AddCommand(MakeUpgrade(craneLister{}))
 
  return command
 }

cmd/chart/upgrade.go

@@ -15,7 +15,17 @@ import (
  "github.com/spf13/cobra"
 )
 
-func MakeUpgrade() *cobra.Command {
+type tagLister interface {
+ listTags(string) ([]string, error)
+}
+
+type craneLister struct{}
+
+func (c craneLister) listTags(image string) ([]string, error) {
+ return crane.ListTags(image)
+}
+
+func MakeUpgrade(lister tagLister) *cobra.Command {
  var command = &cobra.Command{
  Use: "upgrade",
  Short: "Upgrade all images in a values.yaml file to the latest version",
@@ -87,23 +97,16 @@ Otherwise, it returns a non-zero exit code and the updated values.yaml file.`,
  for k := range filtered {
 
  imageName, tag := splitImageName(k)
- ref, err := crane.ListTags(imageName)
+ ref, err := lister.listTags(imageName)
  if err != nil {
  return errors.New("unable to list tags for " + imageName)
  }
 
- var vs []*semver.Version
- for _, r := range ref {
- v, err := semver.NewVersion(r)
- if err == nil {
- vs = append(vs, v)
- }
+ latestTag, err := getLatestTag(tag, ref)
+ if err != nil {
+ return err
  }
 
- sort.Sort(sort.Reverse(semver.Collection(vs)))
-
- latestTag := vs[0].String()
-
  if latestTag != tag {
  updated++
  // Semver is "eating" the "v" prefix, so we need to add it back, if it was there in first place
@@ -144,3 +147,59 @@ func splitImageName(reposName string) (string, string) {
  nameParts := strings.SplitN(reposName, ":", 2)
  return nameParts[0], nameParts[1]
 }
+
+func getLatestTag(tag string, refs []string) (string, error) {
+ current, err := semver.NewVersion(tag)
+ if err != nil {
+ return "", err
+ }
+
+ hasRootlessSuffix := strings.Contains(current.Prerelease(), "rootless")
+
+ var vs []*semver.Version
+ for _, r := range refs {
+ // Strip out '-rootless' metadata from version string as semver treats it as prerelease
+ if hasRootlessSuffix {
+ r = strings.TrimSuffix(r, "-rootless")
+ r = r + "+rootless"
+ }
+
+ v, err := semver.NewVersion(r)
+ if err == nil && v.GreaterThan(current) {
+ vs = append(vs, v)
+ }
+ }
+
+ if len(vs) == 0 {
+ return tag, err
+ }
+
+ latestStableVersion := getLatestStableVersion(vs)
+
+ return latestStableVersion, nil
+}
+
+func getLatestStableVersion(vs []*semver.Version) string {
+ sort.Sort(sort.Reverse(semver.Collection(vs)))
+ latest := vs[0]
+
+ // Finding the latest stable version by selecting the first version without a prerelease tag
+ if latest.Prerelease() != "" {
+ for _, ver := range vs {
+ if ver.Prerelease() == "" {
+ latest = ver
+ break
+ }
+ }
+ }
+
+ latestStr := latest.String()
+
+ // Adding the '-rootless' build tag back
+ if strings.HasSuffix(latestStr, "+rootless") {
+ latestStr = strings.TrimSuffix(latestStr, "+rootless")
+ latestStr = latestStr + "-rootless"
+ }
+
+ return latestStr
+}

cmd/chart/upgrade_test.go

@@ -0,0 +1,155 @@
+package chart
+
+import (
+ "fmt"
+ "os"
+ "testing"
+
+ "gopkg.in/yaml.v2"
+)
+
+type test struct {
+ title string
+ current string
+ tags []string
+ want string
+}
+
+type mockLister struct {
+ imageTags map[string][]string
+}
+
+func (ml mockLister) listTags(image string) ([]string, error) {
+ tags, ok := ml.imageTags[image]
+ if !ok {
+ return nil, fmt.Errorf("Image %s not found", image)
+ }
+
+ return tags, nil
+}
+
+func newMockLister(t *testing.T, tests []test) *mockLister {
+ tagsMap := make(map[string][]string)
+ lister := mockLister{imageTags: tagsMap}
+
+ if len(tests) == 0 {
+ t.Fatal("Tests cannot be empty, please provide test cases")
+ }
+
+ for _, tt := range tests {
+ key, _ := splitImageName(tt.current)
+ // Ensuring that a test entry with the same key doesn't already exist to prevent overwriting
+ if _, ok := lister.imageTags[key]; !ok {
+ lister.imageTags[key] = tt.tags
+ } else {
+ t.Fatalf("Duplicate image name found which might break tests: %s", tt.current)
+ }
+ }
+
+ return &lister
+}
+
+func Test_ChartUpgrade(t *testing.T) {
+ tests := []test{
+ {
+ title: "Promote from a rootless build to the latest rootless build",
+ current: "moby/buildkit:v0.11.5-rootless",
+ want: "moby/buildkit:v0.11.6-rootless",
+ tags: []string{"0.10.6-rootless", "v0.11.5", "0.11.6", "0.11.6-rootless", "0.12.0-rc1", "0.12.0-rc1-rootless"},
+ },
+ {
+ title: "Promote from a rootless build to the latest rootless build",
+ current: "moby1/buildkit:v0.10.0-rootless",
+ want: "moby1/buildkit:v0.11.6-rootless",
+ tags: []string{"0.10.0", "0.10.6", "0.10.6-rootless", "0.11.6", "0.11.6-rootless", "0.12.0-rc1", "0.12.0-rc1-rootless"},
+ },
+ {
+ title: "Promote from non-rootless build to the latest stable build",
+ current: "moby2/buildkit:v0.8.0",
+ want: "moby2/buildkit:v0.11.6",
+ tags: []string{"v0.8.0", "0.10.6-rootless", "0.11.6", "0.11.6-rootless", "0.12.0-rc1", "0.12.0-rc1-rootless"},
+ },
+ {
+ title: "Promote from stable release to the latest stable release",
+ current: "prom/prometheus:v2.42.0",
+ want: "prom/prometheus:v2.44.0",
+ tags: []string{"2.41.5", "2.42.0", "2.43.0", "2.44.0", "2.45.0-rc.0", "2.45.0-rc.1"},
+ },
+ {
+ title: "Promote from stable release to the latest stable release",
+ current: "prom1/prometheus:v2.2.0",
+ want: "prom1/prometheus:v2.44.0",
+ tags: []string{"v2.1.0", "v2.2.0", "2.41.5", "2.42.0", "2.43.0", "2.44.0", "2.45.0-rc.0", "2.45.0-rc.1"},
+ },
+ {
+ title: "Promote from release condidate to the latest stable release",
+ current: "bitnami/postgresql:v2.42.0-rc.2",
+ want: "bitnami/postgresql:v2.44.0",
+ tags: []string{"v2.42.0-rc.5", "2.41.5", "2.42.0", "2.43.0", "2.44.0", "2.45.0-rc.0", "2.45.0-rc.1"},
+ },
+ {
+ title: "Promote from prerelease version to the latest prerelease version",
+ current: "bitnami/rabbitmq:v2.22.0-rc1",
+ want: "bitnami/rabbitmq:v2.22.0-rc3",
+ tags: []string{"v2.21.0", "v2.22.0-rc1", "v2.22.0-rc2", "v2.22.0-rc3"},
+ },
+ {
+ title: "Remain on current version when there isn't a later version",
+ current: "openfaas/openfaas:v2.23.3",
+ want: "openfaas/openfaas:v2.23.3",
+ tags: []string{"v2.23.3", "v2.21.0", "v2.15.6"},
+ },
+ }
+
+ testFile, err := os.CreateTemp(os.TempDir(), "arkade_*.yml")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ defer os.Remove(testFile.Name())
+
+ testData := make(map[string]map[string]string)
+ for i, t := range tests {
+ title := fmt.Sprintf("test%d", i)
+ testData[title] = map[string]string{"image": t.current}
+ }
+
+ yamlBytes, err := yaml.Marshal(testData)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := testFile.Write(yamlBytes); err != nil {
+ t.Fatal(err)
+ }
+
+ lister := newMockLister(t, tests)
+
+ cmd := MakeUpgrade(lister)
+ cmd.SetArgs([]string{"--write", "--file", testFile.Name()})
+
+ if err := cmd.Execute(); err != nil {
+ t.Fatal(err)
+ }
+
+ yamlData, err := os.ReadFile(testFile.Name())
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ var final map[string]map[string]string
+ err = yaml.Unmarshal(yamlData, &final)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ for i, tc := range tests {
+ t.Run(tc.title, func(t *testing.T) {
+ title := fmt.Sprintf("test%d", i)
+ got := final[title]["image"]
+ if got != tc.want {
+ t.Fatalf("want: %s, got: %s", tc.want, got)
+ }
+ })
+ }
+}

pkg/helm/io.go

@@ -1,10 +1,12 @@
 package helm
 
 import (
+ "bufio"
+ "bytes"
  "fmt"
  "os"
  "reflect"
- "strings"
+ "regexp"
 
  "gopkg.in/yaml.v2"
 )
@@ -38,10 +40,26 @@ func ReplaceValuesInHelmValuesFile(values map[string]string, yamlPath string) (s
  return "", err
  }
 
- fileContent := string(readFile)
- for k, v := range values {
- fileContent = strings.ReplaceAll(fileContent, k, v)
+ var buffer bytes.Buffer
+
+ scanner := bufio.NewScanner(bytes.NewReader(readFile))
+ for scanner.Scan() {
+ line := scanner.Text()
+
+ for k, v := range values {
+ re := regexp.MustCompile(`\b` + regexp.QuoteMeta(k) + `$`)
+ line = re.ReplaceAllString(line, v)
+ }
+
+ buffer.WriteString(line)
+ buffer.WriteByte('\n')
  }
+
+ if err := scanner.Err(); err != nil {
+ return "", err
+ }
+
+ fileContent := buffer.String()
  return fileContent, nil
 }