Replace ParseCiliumVersion with semver.ParseTolerant

ParseCiliumVersion uses versioncheck.Version from Cilium's versioncheck package ([1]), which ignores certain pre-release identifiers. Silently ignoring certain pre-release identifiers can be confusing since you might end up installing a different version of Cilium than what you specified. This commit replaces ParseCiliumVersion with semver.ParseTolerant ([2]) so that cilium-cli interprets the version provided via the --version flag as is. [1]: https://pkg.go.dev/github.com/cilium/cilium/pkg/versioncheck#Version [2]: https://pkg.go.dev/github.com/blang/semver#ParseTolerant Signed-off-by: Michi Mutsuzaki <[email protected]>
cilium · May 1, 2023 · d4fa6a9 · d4fa6a9
1 parent 3d28eba
commit d4fa6a9
Show file tree

Hide file tree

Showing 8 changed files with 9 additions and 58 deletions.
diff --git a/clustermesh/clustermesh.go b/clustermesh/clustermesh.go
@@ -1653,7 +1653,7 @@ func (k *K8sClusterMesh) WriteExternalWorkloadInstallScript(ctx context.Context,
 		if err != nil {
 			return err
 		}
-		ciliumVer, err = utils.ParseCiliumVersion(v)
+		ciliumVer, err = semver.ParseTolerant(v)
 		if err != nil {
 			return fmt.Errorf("failed to parse Cilium version %s: %w", v, err)
 		}

diff --git a/connectivity/check/delete.go b/connectivity/check/delete.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/blang/semver/v4"
 	"golang.org/x/exp/slices"
 	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/cli/values"
@@ -183,7 +184,7 @@ func (ct *ConnectivityTest) generateDefaultHelmState(ctx context.Context, client
 	if version == "" || err != nil {
 		return nil, fmt.Errorf("unable to obtain cilium version, no Cilium pods found in namespace %q", namespace)
 	}
-	semVer, err := utils.ParseCiliumVersion(version)
+	semVer, err := semver.ParseTolerant(version)
 	if err != nil {
 		return nil, fmt.Errorf("unable to parse cilium version %s: %w", version, err)
 	}

diff --git a/connectivity/check/features.go b/connectivity/check/features.go
@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/blang/semver/v4"
 	"github.com/cilium/cilium/api/v1/models"
 	"github.com/cilium/cilium/pkg/option"
 	"github.com/cilium/cilium/pkg/versioncheck"
@@ -18,7 +19,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/cilium/cilium-cli/defaults"
-	"github.com/cilium/cilium-cli/internal/utils"
 	"github.com/cilium/cilium-cli/k8s"
 )
 
@@ -448,13 +448,13 @@ func (ct *ConnectivityTest) detectCiliumVersion(ctx context.Context) error {
 	if assumeCiliumVersion := ct.Params().AssumeCiliumVersion; assumeCiliumVersion != "" {
 		ct.Warnf("Assuming Cilium version %s for connectivity tests", assumeCiliumVersion)
 		var err error
-		ct.CiliumVersion, err = utils.ParseCiliumVersion(assumeCiliumVersion)
+		ct.CiliumVersion, err = semver.ParseTolerant(assumeCiliumVersion)
 		if err != nil {
 			return err
 		}
 	} else if minVersion, err := ct.DetectMinimumCiliumVersion(ctx); err != nil {
 		ct.Warnf("Unable to detect Cilium version, assuming %v for connectivity tests: %s", defaults.Version, err)
-		ct.CiliumVersion, err = utils.ParseCiliumVersion(defaults.Version)
+		ct.CiliumVersion, err = semver.ParseTolerant(defaults.Version)
 		if err != nil {
 			return err
 		}

diff --git a/hubble/hubble.go b/hubble/hubble.go
@@ -166,7 +166,7 @@ func (k *K8sHubble) generateDefaultHelmState(ctx context.Context, client k8sHubb
 	if version == "" || err != nil {
 		return nil, fmt.Errorf("unable to obtain cilium version, no cilium pods found in namespace %q", namespace)
 	}
-	semVer, err := utils.ParseCiliumVersion(version)
+	semVer, err := semver.ParseTolerant(version)
 	if err != nil {
 		return nil, fmt.Errorf("unable to parse cilium version %s: %w", version, err)
 	}

diff --git a/internal/helm/helm.go b/internal/helm/helm.go
@@ -436,7 +436,7 @@ func ResolveHelmChartVersion(versionFlag, chartDirectoryFlag string) (semver2.Ve
 }
 
 func resolveChartVersion(versionFlag string) (semver2.Version, *chart.Chart, error) {
-	version, err := utils.ParseCiliumVersion(versionFlag)
+	version, err := semver2.ParseTolerant(versionFlag)
 	if err != nil {
 		return semver2.Version{}, nil, err
 	}

diff --git a/internal/utils/utils.go b/internal/utils/utils.go
@@ -10,9 +10,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/blang/semver/v4"
-	"github.com/cilium/cilium/pkg/versioncheck"
-
 	"github.com/cilium/cilium-cli/defaults"
 )
 
@@ -25,11 +22,6 @@ func CheckVersion(version string) error {
 	return nil
 }
 
-func ParseCiliumVersion(version string) (semver.Version, error) {
-	ersion := strings.TrimPrefix(version, "v")
-	return versioncheck.Version(ersion)
-}
-
 type ImagePathMode int
 
 const (

diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go
@@ -5,10 +5,7 @@ package utils
 
 import (
 	"fmt"
-	"reflect"
 	"testing"
-
-	"github.com/blang/semver/v4"
 )
 
 func TestCheckVersion(t *testing.T) {
@@ -55,44 +52,6 @@ func TestCheckVersion(t *testing.T) {
 	}
 }
 
-func TestParseCiliumVersion(t *testing.T) {
-	tests := []struct {
-		name    string
-		version string
-		want    semver.Version
-		wantErr bool
-	}{
-		{
-			name:    "empty",
-			wantErr: true,
-		},
-		{
-			name:    "invalid-version",
-			version: "invalid",
-			wantErr: true,
-		},
-		{
-			name:    "valid-version",
-			version: "v1.9.99",
-			want:    semver.Version{Major: 1, Minor: 9, Patch: 99},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := ParseCiliumVersion(tt.version)
-			if tt.wantErr && err == nil {
-				t.Errorf("ParseCiliumVersion(%q) got nil, want error", tt.version)
-			} else if !tt.wantErr && err != nil {
-				t.Errorf("ParseCiliumVersion(%q) got error, want nil", tt.version)
-			}
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("PetCiliumVersion(%q) = %v, want %v", tt.version, got, tt.want)
-			}
-		})
-	}
-}
-
 func TestBuildImagePath(t *testing.T) {
 	tests := []struct {
 		userImage      string

diff --git a/k8s/client.go b/k8s/client.go
@@ -51,7 +51,6 @@ import (
 
 	"github.com/cilium/cilium-cli/defaults"
 	"github.com/cilium/cilium-cli/internal/helm"
-	"github.com/cilium/cilium-cli/internal/utils"
 )
 
 type Client struct {
@@ -1003,7 +1002,7 @@ func (c *Client) GetHelmState(ctx context.Context, namespace string, secretName
 		return nil, fmt.Errorf("unable to retrieve helm chart version from secret %s/%s: %w", namespace, secretName, err)
 	}
 	versionString := string(versionBytes)
-	version, err := utils.ParseCiliumVersion(versionString)
+	version, err := semver.ParseTolerant(versionString)
 	if err != nil {
 		return nil, fmt.Errorf("unable to parse helm chart version from secret %s/%s: %s %w", namespace, secretName, versionString, err)
 	}