New CLI Flag --devel To Include Development/Prerelease Versions of Charts (#139)

* Include devel Flag for Toggling Dev Chart Versions The flag `--devel` for enabling/disabling dev versions of charts in following endpoints: 1. /api/helm/repositories/kafka-operator 2. /api/helm/repositories/versions 3. /api/helm/repositories/latestver Signed-off-by: Bhargav Ravuri <bhargav.ravuri@infracloud.io> * Run Tests on Devel Flag Related Changes Signed-off-by: Bhargav Ravuri <bhargav.ravuri@infracloud.io> --------- Signed-off-by: Bhargav Ravuri <bhargav.ravuri@infracloud.io>
2026-03-24 11:48:04 +00:00 · 2023-02-11 21:32:01 +05:30
parent 61b67f8bed
commit 6a4ca793c9
12 changed files with 348 additions and 55 deletions
--- a/pkg/dashboard/objects/app.go
+++ b/pkg/dashboard/objects/app.go
@@ -4,6 +4,7 @@ import (
 	"github.com/joomcode/errorx"
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/cli"
+
 	// Import to initialize client auth plugins.
 	// From https://github.com/kubernetes/client-go/issues/242
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
@@ -22,7 +23,7 @@ type Application struct {
 	Repositories *Repositories
 }

-func NewApplication(settings *cli.EnvSettings, helmConfig HelmNSConfigGetter, namespaces []string) (*Application, error) {
+func NewApplication(settings *cli.EnvSettings, helmConfig HelmNSConfigGetter, namespaces []string, devel bool) (*Application, error) {
 	hc, err := helmConfig(settings.Namespace())
 	if err != nil {
 		return nil, errorx.Decorate(err, "failed to get helm config for namespace '%s'", "")
@@ -33,6 +34,11 @@ func NewApplication(settings *cli.EnvSettings, helmConfig HelmNSConfigGetter, na
 		return nil, errorx.Decorate(err, "failed to get k8s client")
 	}

+	semVerConstraint, err := versionConstaint(devel)
+	if err != nil {
+		return nil, errorx.Decorate(err, "failed to create semantic version constraint")
+	}
+
 	return &Application{
 		HelmConfig: helmConfig,
 		K8s:        k8s,
@@ -42,8 +48,9 @@ func NewApplication(settings *cli.EnvSettings, helmConfig HelmNSConfigGetter, na
 			HelmConfig: helmConfig,
 		},
 		Repositories: &Repositories{
-			Settings:   settings,
-			HelmConfig: hc,
+			Settings:          settings,
+			HelmConfig:        hc,
+			versionConstraint: semVerConstraint,
 		},
 	}, nil
 }
--- a/pkg/dashboard/objects/data.go
+++ b/pkg/dashboard/objects/data.go
@@ -7,6 +7,8 @@ import (
 	"sync"
 	"time"

+	"io"
+
 	"github.com/joomcode/errorx"
 	"github.com/komodorio/helm-dashboard/pkg/dashboard/subproc"
 	"github.com/pkg/errors"
@@ -15,7 +17,6 @@ import (
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/release"
-	"io"
 	v1 "k8s.io/apimachinery/pkg/apis/testapigroup/v1"
 	"k8s.io/client-go/tools/clientcmd"
 )
@@ -30,6 +31,7 @@ type DataLayer struct {
 	ConfGen         HelmConfigGetter
 	appPerContext   map[string]*Application
 	appPerContextMx *sync.Mutex
+	devel           bool
 }

 type StatusInfo struct {
@@ -40,7 +42,7 @@ type StatusInfo struct {
 	ClusterMode   bool
 }

-func NewDataLayer(ns []string, ver string, cg HelmConfigGetter) (*DataLayer, error) {
+func NewDataLayer(ns []string, ver string, cg HelmConfigGetter, devel bool) (*DataLayer, error) {
 	if cg == nil {
 		return nil, errors.New("HelmConfigGetter can't be nil")
 	}
@@ -56,6 +58,7 @@ func NewDataLayer(ns []string, ver string, cg HelmConfigGetter) (*DataLayer, err
 		ConfGen:         cg,
 		appPerContext:   map[string]*Application{},
 		appPerContextMx: new(sync.Mutex),
+		devel:           devel,
 	}, nil
 }

@@ -162,7 +165,7 @@ func (d *DataLayer) AppForCtx(ctx string) (*Application, error) {
 			return d.ConfGen(settings, ns)
 		}

-		a, err := NewApplication(settings, cfgGetter, d.Namespaces)
+		a, err := NewApplication(settings, cfgGetter, d.Namespaces, d.devel)
 		if err != nil {
 			return nil, errorx.Decorate(err, "Failed to create application for context '%s'", ctx)
 		}
--- a/pkg/dashboard/objects/data_test.go
+++ b/pkg/dashboard/objects/data_test.go
@@ -15,6 +15,7 @@ func TestNewDataLayer(t *testing.T) {
 		namespaces    []string
 		version       string
 		helmConfig    HelmConfigGetter
+		devel         bool
 		errorExpected bool
 	}{
 		{
@@ -22,6 +23,7 @@ func TestNewDataLayer(t *testing.T) {
 			namespaces:    []string{"namespace1", "namespace2"},
 			version:       "1.0.0",
 			helmConfig:    nil,
+			devel:         false,
 			errorExpected: true,
 		},
 		{
@@ -34,12 +36,13 @@ func TestNewDataLayer(t *testing.T) {
 			helmConfig: func(sett *cli.EnvSettings, ns string) (*action.Configuration, error) {
 				return &action.Configuration{}, nil
 			},
+			devel:         false,
 			errorExpected: false,
 		},
 	}
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
-			dl, err := NewDataLayer(tt.namespaces, tt.version, tt.helmConfig)
+			dl, err := NewDataLayer(tt.namespaces, tt.version, tt.helmConfig, tt.devel)
 			if tt.errorExpected {
 				assert.Error(t, err, "Expected error but got nil")
 			} else {
--- a/pkg/dashboard/objects/repos.go
+++ b/pkg/dashboard/objects/repos.go
@@ -1,6 +1,12 @@
 package objects

 import (
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+
+	"github.com/Masterminds/semver/v3"
 	"github.com/joomcode/errorx"
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
@@ -11,18 +17,15 @@ import (
 	"helm.sh/helm/v3/pkg/getter"
 	"helm.sh/helm/v3/pkg/helmpath"
 	"helm.sh/helm/v3/pkg/repo"
-	"os"
-	"path/filepath"
-	"strings"
-	"sync"
 )

 const AnnRepo = "helm-dashboard/repository-name"

 type Repositories struct {
-	Settings   *cli.EnvSettings
-	HelmConfig *action.Configuration
-	mx         sync.Mutex
+	Settings          *cli.EnvSettings
+	HelmConfig        *action.Configuration
+	mx                sync.Mutex
+	versionConstraint *semver.Constraints
 }

 func (r *Repositories) Load() (*repo.File, error) {
@@ -142,8 +145,9 @@ func (r *Repositories) Get(name string) (*Repository, error) {
 	for _, entry := range f.Repositories {
 		if entry.Name == name {
 			return &Repository{
-				Settings: r.Settings,
-				Orig:     entry,
+				Settings:          r.Settings,
+				Orig:              entry,
+				versionConstraint: r.versionConstraint,
 			}, nil
 		}
 	}
@@ -166,6 +170,7 @@ func (r *Repositories) Containing(name string) (repo.ChartVersions, error) {
 			continue
 		}

+		var updatedChartVersions repo.ChartVersions
 		for _, v := range vers {
 			// just using annotations here to attach a bit of information to the object
 			// it has nothing to do with k8s annotations and should not get into manifests
@@ -174,9 +179,22 @@ func (r *Repositories) Containing(name string) (repo.ChartVersions, error) {
 			}

 			v.Annotations[AnnRepo] = rep.Orig.Name
+
+			// Validate the versions against semantic version constraints and filter
+			version, err := semver.NewVersion(v.Version)
+			if err != nil {
+				// Ignored if version string is not parsable
+				log.Debugf("failed to parse version string %q: %v", v.Version, err)
+				continue
+			}
+
+			if r.versionConstraint.Check(version) {
+				// Add only versions that satisfy the semantic version constraint
+				updatedChartVersions = append(updatedChartVersions, v)
+			}
 		}

-		res = append(res, vers...) // TODO filter dev versions here, relates to #139
+		res = append(res, updatedChartVersions...)
 	}
 	return res, nil
 }
@@ -220,6 +238,8 @@ type Repository struct {
 	Settings *cli.EnvSettings
 	Orig     *repo.Entry
 	mx       sync.Mutex
+
+	versionConstraint *semver.Constraints
 }

 func (r *Repository) indexFileName() string {
@@ -247,9 +267,25 @@ func (r *Repository) Charts() ([]*repo.ChartVersion, error) {
 	}

 	res := []*repo.ChartVersion{}
-	for _, v := range ind.Entries {
-		if len(v) > 0 { // TODO filter dev versions here, relates to #139
-			res = append(res, v[0])
+	for _, cv := range ind.Entries {
+		for _, v := range cv {
+			version, err := semver.NewVersion(v.Version)
+			if err != nil {
+				// Ignored if version string is not parsable
+				log.Debugf("failed to parse version string %q: %v", v.Version, err)
+				continue
+			}
+
+			if r.versionConstraint.Check(version) {
+				// Add only versions that satisfy the semantic version constraint
+				res = append(res, v)
+
+				// Only the highest version satisfying the constraint is required. Hence, break.
+				// The constraint here is (only stable versions) vs (stable + dev/prerelease).
+				// If dev versions are disabled and chart only has dev versions,
+				// chart is excluded from the result.
+				break
+			}
 		}
 	}

@@ -310,3 +346,23 @@ func removeRepoCache(root, name string) error {
 	}
 	return os.Remove(idx)
 }
+
+// versionConstaint returns semantic version constraint instance that can be used to
+// validate the version of repositories. The flag isDevelEnabled is used to configure
+// enabling/disabling of development/prerelease versions of charts.
+func versionConstaint(isDevelEnabled bool) (*semver.Constraints, error) {
+	// When devel flag is disabled. i.e., Only stable releases are included.
+	version := ">0.0.0"
+
+	if isDevelEnabled {
+		// When devel flag is enabled. i.e., Prereleases (alpha, beta, release candidate, etc.) are included.
+		version = ">0.0.0-0"
+	}
+
+	constraint, err := semver.NewConstraint(version)
+	if err != nil {
+		return nil, errors.Wrapf(err, "invalid version constraint format %q", version)
+	}
+
+	return constraint, nil
+}
--- a/pkg/dashboard/objects/repos_test.go
+++ b/pkg/dashboard/objects/repos_test.go
@@ -1,19 +1,23 @@
 package objects

 import (
-	"helm.sh/helm/v3/pkg/action"
 	"io/ioutil"
 	"os"
 	"path"
 	"testing"

 	"gotest.tools/v3/assert"
+	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/cli"
 )

-var filePath = "./testdata/repositories.yaml"
+const (
+	validRepositoryConfigPath                    = "./testdata/repositories.yaml"
+	invalidCacheFileRepositoryConfigPath         = "./testdata/repositories-invalid-cache-file.yaml"
+	invalidMalformedManifestRepositoryConfigPath = "./testdata/repositories-malformed-manifest.yaml"
+)

-func initRepository(t *testing.T) *Repositories {
+func initRepository(t *testing.T, filePath string, devel bool) *Repositories {
 	t.Helper()

 	settings := cli.New()
@@ -40,20 +44,26 @@ func initRepository(t *testing.T) *Repositories {
 		}
 	})

+	vc, err := versionConstaint(devel)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	// Sets the repository file path
 	settings.RepositoryConfig = fname.Name()
 	settings.RepositoryCache = path.Dir(filePath)

 	testRepository := &Repositories{
-		Settings:   settings,
-		HelmConfig: &action.Configuration{}, // maybe use copy of getFakeHelmConfig from api_test.go
+		Settings:          settings,
+		HelmConfig:        &action.Configuration{}, // maybe use copy of getFakeHelmConfig from api_test.go
+		versionConstraint: vc,
 	}

 	return testRepository
 }

 func TestList(t *testing.T) {
-	testRepository := initRepository(t)
+	testRepository := initRepository(t, validRepositoryConfigPath, false)

 	repos, err := testRepository.List()
 	if err != nil {
@@ -67,7 +77,7 @@ func TestAdd(t *testing.T) {
 	testRepoName := "TEST"
 	testRepoUrl := "https://helm.github.io/examples"

-	testRepository := initRepository(t)
+	testRepository := initRepository(t, validRepositoryConfigPath, false)
 	err := testRepository.Add(testRepoName, testRepoUrl)
 	if err != nil {
 		t.Fatal(err, "Failed to add repo")
@@ -82,7 +92,7 @@ func TestAdd(t *testing.T) {
 }

 func TestDelete(t *testing.T) {
-	testRepository := initRepository(t)
+	testRepository := initRepository(t, validRepositoryConfigPath, false)

 	testRepoName := "charts" // don't ever delete 'testing'!
 	err := testRepository.Delete(testRepoName)
@@ -100,7 +110,7 @@ func TestGet(t *testing.T) {
 	// Initial repositiry name in test file
 	repoName := "charts"

-	testRepository := initRepository(t)
+	testRepository := initRepository(t, validRepositoryConfigPath, false)

 	repo, err := testRepository.Get(repoName)
 	if err != nil {
@@ -110,8 +120,8 @@ func TestGet(t *testing.T) {
 	assert.Equal(t, repo.Orig.Name, repoName)
 }

-func TestCharts(t *testing.T) {
-	testRepository := initRepository(t)
+func TestRepository_Charts_DevelDisabled(t *testing.T) {
+	testRepository := initRepository(t, validRepositoryConfigPath, false)

 	r, err := testRepository.Get("testing")
 	if err != nil {
@@ -123,7 +133,164 @@ func TestCharts(t *testing.T) {
 		t.Fatal(err)
 	}

-	if len(charts) != 2 {
-		t.Fatalf("Wrong charts len: %d", len(charts))
+	// Total charts in ./testdata/testing-index.yaml = 4
+	// Excluded charts = 2 (1 has invalid version, 1 has only dev version)
+	// Included charts = 2 (2 stable versions)
+	expectedCount := 2
+	if len(charts) != expectedCount {
+		t.Fatalf("Wrong charts count: %d, expected: %d", len(charts), expectedCount)
+	}
+}
+
+func TestRepository_Charts_DevelEnabled(t *testing.T) {
+	testRepository := initRepository(t, validRepositoryConfigPath, true)
+
+	r, err := testRepository.Get("testing")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	charts, err := r.Charts()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Total charts in ./testdata/testing-index.yaml = 4
+	// Excluded charts = 1 (1 has invalid version)
+	// Included charts = 3 (2 stable versions, 1 has only dev version)
+	expectedCount := 3
+	if len(charts) != expectedCount {
+		t.Fatalf("Wrong charts count: %d, expected: %d", len(charts), expectedCount)
+	}
+}
+
+func TestRepository_Charts_InvalidCacheFile(t *testing.T) {
+	testRepository := initRepository(t, invalidCacheFileRepositoryConfigPath, false)
+
+	r, err := testRepository.Get("non-existing")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = r.Charts()
+	if err == nil {
+		t.Fatalf("Expected error for invalid cache file path, got nil")
+	}
+}
+
+func TestRepositories_Containing_DevelDisable(t *testing.T) {
+	testRepository := initRepository(t, validRepositoryConfigPath, false)
+
+	chartVersions, err := testRepository.Containing("alpine")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Total versions of chart alpine in ./testdata/testing-index.yaml = 3
+	// Excluded charts = 1 (1 dev version)
+	// Included charts = 2 (2 stable versions)
+	expectedCount := 2
+	if len(chartVersions) != expectedCount {
+		t.Fatalf("Wrong charts versions count: %d, expected: %d", len(chartVersions), expectedCount)
+	}
+
+}
+
+func TestRepositories_Containing_DevelEnabled(t *testing.T) {
+	testRepository := initRepository(t, validRepositoryConfigPath, true)
+
+	chartVersions, err := testRepository.Containing("alpine")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Total versions of chart alpine in ./testdata/testing-index.yaml = 3
+	// Excluded charts = 0
+	// Included charts = 3 (2 stable versions, 1 dev version)
+	expectedCount := 3
+	if len(chartVersions) != expectedCount {
+		t.Fatalf("Wrong charts versions count: %d, expected: %d", len(chartVersions), expectedCount)
+	}
+
+}
+
+func TestRepositories_Containing_DevelDisable_OnlyDevVersionsOfChartAvailable(t *testing.T) {
+	testRepository := initRepository(t, validRepositoryConfigPath, false)
+
+	chartVersions, err := testRepository.Containing("traefik")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Total versions of chart traefik in ./testdata/testing-index.yaml = 1
+	// Excluded charts = 1 (1 dev version)
+	// Included charts = 0
+	expectedCount := 0
+	if len(chartVersions) != expectedCount {
+		t.Fatalf("Wrong charts versions count: %d, expected: %d", len(chartVersions), expectedCount)
+	}
+
+}
+
+func TestRepositories_Containing_DevelEnabled_OnlyDevVersionsOfChartAvailable(t *testing.T) {
+	testRepository := initRepository(t, validRepositoryConfigPath, true)
+
+	chartVersions, err := testRepository.Containing("traefik")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Total versions of chart traefik in ./testdata/testing-index.yaml = 1
+	// Excluded charts = 0
+	// Included charts = 1 (1 dev version)
+	expectedCount := 1
+	if len(chartVersions) != expectedCount {
+		t.Fatalf("Wrong charts versions count: %d, expected: %d", len(chartVersions), expectedCount)
+	}
+
+}
+
+func TestRepositories_Containing_DevelDisable_InvalidChartVersion(t *testing.T) {
+	testRepository := initRepository(t, validRepositoryConfigPath, false)
+
+	chartVersions, err := testRepository.Containing("rabbitmq")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Total versions of chart rabbitmq in ./testdata/testing-index.yaml = 1
+	// Excluded charts = 1 (1 invalid version)
+	// Included charts = 0
+	expectedCount := 0
+	if len(chartVersions) != expectedCount {
+		t.Fatalf("Wrong charts versions count: %d, expected: %d", len(chartVersions), expectedCount)
+	}
+
+}
+
+func TestRepositories_Containing_DevelEnabled_InvalidChartVersion(t *testing.T) {
+	testRepository := initRepository(t, validRepositoryConfigPath, true)
+
+	chartVersions, err := testRepository.Containing("rabbitmq")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Total versions of chart rabbitmq in ./testdata/testing-index.yaml = 1
+	// Excluded charts = 1 (1 invalid version)
+	// Included charts = 0
+	expectedCount := 0
+	if len(chartVersions) != expectedCount {
+		t.Fatalf("Wrong charts versions count: %d, expected: %d", len(chartVersions), expectedCount)
+	}
+
+}
+
+func TestRepositories_Containing_MalformedRepositoryConfigFile(t *testing.T) {
+	testRepository := initRepository(t, invalidMalformedManifestRepositoryConfigPath, false)
+
+	_, err := testRepository.Containing("alpine")
+	if err == nil {
+		t.Fatalf("Expected error for malformed RepositoryConfig file, got nil")
 	}
 }
--- a/pkg/dashboard/objects/testdata/repositories-invalid-cache-file.yaml
+++ b/pkg/dashboard/objects/testdata/repositories-invalid-cache-file.yaml
@@ -0,0 +1,6 @@
+apiVersion: ""
+generated: "0001-01-01T00:00:00Z"
+repositories:
+- cache: non-existing-index.yaml
+  name: non-existing
+  url: http://example.com/charts
--- a/pkg/dashboard/objects/testdata/repositories-malformed-manifest.yaml
+++ b/pkg/dashboard/objects/testdata/repositories-malformed-manifest.yaml
@@ -0,0 +1,12 @@
+apiVersion: ""
+generated: "0001-01-01T00:00:00Z"
+- repositories:
+- caFile: ""
+  certFile: ""
+  insecure_skip_tls_verify: false
+  keyFile: ""
+  name: charts
+  pass_credentials_all: false
+  password: ""
+  url: https://charts.helm.sh/stable
+  username: ""
--- a/pkg/dashboard/objects/testdata/testing-index.yaml
+++ b/pkg/dashboard/objects/testdata/testing-index.yaml
@@ -64,3 +64,37 @@ entries:
          email: containers@bitnami.com
      icon: ""
      apiVersion: v2
+  traefik:
+    - apiVersion: v1
+      appVersion: 1.7.26
+      deprecated: true
+      description: A Traefik based Kubernetes ingress controller with Let's
+        Encrypt support
+      home: https://traefik.io/
+      icon: https://docs.traefik.io/assets/img/traefik.logo.png
+      keywords:
+      - traefik
+      - ingress
+      - acme
+      - letsencrypt
+      name: traefik
+      sources:
+      - https://github.com/containous/traefik
+      - https://github.com/helm/charts/tree/master/stable/traefik
+      version: 1.87.7-rc1
+  rabbitmq:
+    - apiVersion: v1
+      appVersion: 3.8.2
+      deprecated: true
+      description: DEPRECATED Open source message broker software that implements the Advanced
+        Message Queuing Protocol (AMQP)
+      home: https://www.rabbitmq.com
+      icon: https://bitnami.com/assets/stacks/rabbitmq/img/rabbitmq-stack-220x234.png
+      keywords:
+      - rabbitmq
+      - message queue
+      - AMQP
+      name: rabbitmq
+      sources:
+      - https://github.com/bitnami/bitnami-docker-rabbitmq
+      version: invalid-version