From 5d8eba9b291e6f9f8ef1a2ecbcf688459115dfc0 Mon Sep 17 00:00:00 2001 From: Wi1dcard Date: Mon, 12 Oct 2020 08:20:55 +0800 Subject: [PATCH] Append --force-update for specific helm versions. (#1494) * Parse and process helm version using github.com/Masterminds/semver/v3. * Add --force-update only when Helm version >= 3.3.2, < 3.3.4. See: https://github.com/helm/helm/pull/8777. * Add test cases. --- go.mod | 2 +- go.sum | 2 + pkg/app/app_test.go | 2 +- pkg/app/mocks_test.go | 2 +- pkg/exectest/helm.go | 16 ++++---- pkg/helmexec/exec.go | 72 +++++++++++++++------------------ pkg/helmexec/exec_test.go | 75 +++++++++++------------------------ pkg/helmexec/helmexec.go | 2 +- pkg/state/chart_dependency.go | 13 +++--- pkg/state/state.go | 2 +- pkg/state/state_test.go | 33 ++++----------- 11 files changed, 87 insertions(+), 134 deletions(-) diff --git a/go.mod b/go.mod index 0876a198..ce8e5cdc 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.14 require ( github.com/Azure/azure-sdk-for-go v35.0.0+incompatible // indirect - github.com/Masterminds/semver v1.4.2 + github.com/Masterminds/semver/v3 v3.1.0 github.com/Masterminds/sprig/v3 v3.1.0 github.com/aryann/difflib v0.0.0-20170710044230-e206f873d14a github.com/go-test/deep v1.0.3 diff --git a/go.sum b/go.sum index 6ffec825..7af87159 100644 --- a/go.sum +++ b/go.sum @@ -181,6 +181,7 @@ github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d/go.mod h1:6QX/PXZ github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= github.com/bitly/go-hostpool v0.0.0-20171023180738-a3a6125de932 h1:mXoPYz/Ul5HYEDvkta6I8/rnYM5gSdSV2tJ6XbZuEtY= github.com/bitly/go-hostpool v0.0.0-20171023180738-a3a6125de932/go.mod h1:NOuUCSz6Q9T7+igc/hlvDOUdtWKryOrtFyIVABv/p7k= +github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ= github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 h1:DDGfHa7BWjL4YnC6+E63dPcxHo2sUxDIu8g3QgEJdRY= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= @@ -1108,6 +1109,7 @@ google.golang.org/api v0.21.0 h1:zS+Q/CJJnVlXpXQVIz+lH0ZT2lBuT2ac7XD8Y/3w6hY= google.golang.org/api v0.26.0 h1:VJZ8h6E8ip82FRpQl848c5vAadxlTXrUh8RzQzSRm08= google.golang.org/api v0.30.0 h1:yfrXXP61wVuLb0vBcG6qaOoIoqYEzOQS8jum51jkv2w= google.golang.org/api v0.31.0 h1:1w5Sz/puhxFo9lTtip2n47k7toB/U2nCqOKNHd3Yrbo= +google.golang.org/api v0.32.0 h1:Le77IccnTqEa8ryp9wIpX5W3zYm7Gf9LhOp9PHcwFts= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.2.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/appengine v1.3.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 22ac9b60..8c8e91d3 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2462,7 +2462,7 @@ func (helm *mockHelmExec) GetVersion() helmexec.Version { return helmexec.Version{} } -func (helm *mockHelmExec) IsVersionAtLeast(major int, minor int, patch int) bool { +func (helm *mockHelmExec) IsVersionAtLeast(versionStr string) bool { return false } diff --git a/pkg/app/mocks_test.go b/pkg/app/mocks_test.go index b2ce9674..aeca04b9 100644 --- a/pkg/app/mocks_test.go +++ b/pkg/app/mocks_test.go @@ -97,7 +97,7 @@ func (helm *noCallHelmExec) GetVersion() helmexec.Version { return helmexec.Version{} } -func (helm *noCallHelmExec) IsVersionAtLeast(major int, minor int, patch int) bool { +func (helm *noCallHelmExec) IsVersionAtLeast(versionStr string) bool { helm.doPanic() return false } diff --git a/pkg/exectest/helm.go b/pkg/exectest/helm.go index 658c312f..d535f5ed 100644 --- a/pkg/exectest/helm.go +++ b/pkg/exectest/helm.go @@ -6,6 +6,7 @@ import ( "strings" "sync" + "github.com/Masterminds/semver/v3" "github.com/roboll/helmfile/pkg/helmexec" ) @@ -30,7 +31,7 @@ type Helm struct { Diffed []Release FailOnUnexpectedDiff bool FailOnUnexpectedList bool - Version *helmexec.Version + Version *semver.Version UpdateDepsCallbacks map[string]func(string) error @@ -163,19 +164,20 @@ func (helm *Helm) IsHelm3() bool { } func (helm *Helm) GetVersion() helmexec.Version { - if helm.Version != nil { - return *helm.Version + return helmexec.Version{ + Major: int(helm.Version.Major()), + Minor: int(helm.Version.Minor()), + Patch: int(helm.Version.Patch()), } - - return helmexec.Version{} } -func (helm *Helm) IsVersionAtLeast(major int, minor int, patch int) bool { +func (helm *Helm) IsVersionAtLeast(versionStr string) bool { if helm.Version == nil { return false } - return helm.Version.Major >= major && minor >= helm.Version.Minor && patch >= helm.Version.Patch + ver := semver.MustParse(versionStr) + return helm.Version.Equal(ver) || helm.Version.GreaterThan(ver) } func (helm *Helm) sync(m *sync.Mutex, f func()) { diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go index 6c4e6854..47914dc8 100644 --- a/pkg/helmexec/exec.go +++ b/pkg/helmexec/exec.go @@ -6,11 +6,11 @@ import ( "io/ioutil" "os" "path/filepath" - "regexp" "strconv" "strings" "sync" + "github.com/Masterminds/semver/v3" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) @@ -22,7 +22,7 @@ type decryptedSecret struct { type execer struct { helmBinary string - version Version + version semver.Version runner Runner logger *zap.SugaredLogger kubeContext string @@ -48,51 +48,33 @@ func NewLogger(writer io.Writer, logLevel string) *zap.SugaredLogger { return zap.New(core).Sugar() } -// versionRegex matches versions like v1.1.1 and v1.1 -var versionRegex = regexp.MustCompile("v(?P\\d+)\\.(?P\\d+)(?:\\.(?P\\d+))?") - -func parseHelmVersion(versionStr string) (Version, error) { +func parseHelmVersion(versionStr string) (semver.Version, error) { if len(versionStr) == 0 { - return Version{}, nil + return semver.Version{}, nil } - matches := versionRegex.FindStringSubmatch(versionStr) - if len(matches) == 0 { - return Version{}, fmt.Errorf("error parsing helm verion '%s'", versionStr) - } - result := make(map[string]string) - for i, name := range versionRegex.SubexpNames() { - result[name] = matches[i] - } + versionStr = strings.TrimLeft(versionStr, "Client: ") + versionStr = strings.TrimRight(versionStr, "\n") - // We ignore errors because regex matches only integers - // If any of the parts does not exist - default "0" will be used - major, _ := strconv.Atoi(result["major"]) - minor, _ := strconv.Atoi(result["minor"]) - patch, _ := strconv.Atoi(result["patch"]) + ver, err := semver.NewVersion(versionStr) + if err != nil { + return semver.Version{}, fmt.Errorf("error parsing helm verion '%s'", versionStr) + } // Support explicit helm3 opt-in via environment variable - if os.Getenv("HELMFILE_HELM3") != "" && major < 3 { - return Version{ - Major: 3, - Minor: 0, - Patch: 0, - }, nil + if os.Getenv("HELMFILE_HELM3") != "" && ver.Major() < 3 { + return *semver.MustParse("v3.0.0"), nil } - return Version{ - Major: major, - Minor: minor, - Patch: patch, - }, nil + return *ver, nil } -func getHelmVersion(helmBinary string, runner Runner) (Version, error) { +func getHelmVersion(helmBinary string, runner Runner) (semver.Version, error) { // Autodetect from `helm verison` bytes, err := runner.Execute(helmBinary, []string{"version", "--client", "--short"}, nil) if err != nil { - return Version{}, fmt.Errorf("error determining helm version: %w", err) + return semver.Version{}, fmt.Errorf("error determining helm version: %w", err) } return parseHelmVersion(string(bytes)) @@ -130,9 +112,16 @@ func (helm *execer) AddRepo(name, repository, cafile, certfile, keyfile, usernam return fmt.Errorf("empty field name") } args = append(args, "repo", "add", name, repository) - if helm.IsHelm3() && helm.IsVersionAtLeast(3, 3, 2) { - args = append(args, "--force-update") + + // See https://github.com/helm/helm/pull/8777 + if cons, err := semver.NewConstraint(">= 3.3.2, < 3.3.4"); err == nil { + if cons.Check(&helm.version) { + args = append(args, "--force-update") + } + } else { + panic(err) } + if certfile != "" && keyfile != "" { args = append(args, "--cert-file", certfile, "--key-file", keyfile) } @@ -394,13 +383,18 @@ func (helm *execer) write(out []byte) { } func (helm *execer) IsHelm3() bool { - return helm.version.Major == 3 + return helm.version.Major() == 3 } func (helm *execer) GetVersion() Version { - return helm.version + return Version{ + Major: int(helm.version.Major()), + Minor: int(helm.version.Minor()), + Patch: int(helm.version.Patch()), + } } -func (helm *execer) IsVersionAtLeast(major int, minor int, patch int) bool { - return helm.version.Major >= major && helm.version.Minor >= minor && helm.version.Patch >= patch +func (helm *execer) IsVersionAtLeast(versionStr string) bool { + ver := semver.MustParse(versionStr) + return helm.version.Equal(ver) || helm.version.GreaterThan(ver) } diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go index 704153d2..71eba2fd 100644 --- a/pkg/helmexec/exec_test.go +++ b/pkg/helmexec/exec_test.go @@ -9,6 +9,7 @@ import ( "reflect" "testing" + "github.com/Masterminds/semver/v3" "go.uber.org/zap" ) @@ -71,6 +72,26 @@ func Test_SetHelmBinary(t *testing.T) { } } +func Test_AddRepo_Helm_3_3_2(t *testing.T) { + var buffer bytes.Buffer + logger := NewLogger(&buffer, "debug") + helm := &execer{ + helmBinary: "helm", + version: *semver.MustParse("3.3.2"), + logger: logger, + kubeContext: "dev", + runner: &mockRunner{}, + } + helm.AddRepo("myRepo", "https://repo.example.com/", "", "cert.pem", "key.pem", "", "") + expected := `Adding repo myRepo https://repo.example.com/ +exec: helm --kube-context dev repo add myRepo https://repo.example.com/ --force-update --cert-file cert.pem --key-file key.pem +exec: helm --kube-context dev repo add myRepo https://repo.example.com/ --force-update --cert-file cert.pem --key-file key.pem: +` + if buffer.String() != expected { + t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) + } +} + func Test_AddRepo(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") @@ -557,63 +578,15 @@ func Test_GetVersion(t *testing.T) { func Test_IsVersionAtLeast(t *testing.T) { helm2Runner := mockRunner{output: []byte("Client: v2.16.1+ge13bc94\n")} helm := New("helm", NewLogger(os.Stdout, "info"), "dev", &helm2Runner) - if !helm.IsVersionAtLeast(2, 1, 0) { + if !helm.IsVersionAtLeast("2.1.0") { t.Error("helmexec.IsVersionAtLeast - 2.16.1 not atleast 2.1") } - if helm.IsVersionAtLeast(2, 19, 0) { + if helm.IsVersionAtLeast("2.19.0") { t.Error("helmexec.IsVersionAtLeast - 2.16.1 is atleast 2.19") } - if helm.IsVersionAtLeast(3, 2, 0) { + if helm.IsVersionAtLeast("3.2.0") { t.Error("helmexec.IsVersionAtLeast - 2.16.1 is atleast 3.2") } - -} - -func Test_parseHelmVersion(t *testing.T) { - tests := []struct { - ver string - want Version - wantErr bool - }{ - { - ver: "v1.2.3", - want: Version{ - Major: 1, - Minor: 2, - Patch: 3, - }, - wantErr: false, - }, - { - ver: "v1.2", - want: Version{ - Major: 1, - Minor: 2, - Patch: 0, - }, - wantErr: false, - }, - { - ver: "v1", - wantErr: true, - }, - { - ver: "1.1.1", - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.ver, func(t *testing.T) { - got, err := parseHelmVersion(tt.ver) - if (err != nil) != tt.wantErr { - t.Errorf("parseHelmVersion() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("parseHelmVersion() got = %v, want %v", got, tt.want) - } - }) - } } diff --git a/pkg/helmexec/helmexec.go b/pkg/helmexec/helmexec.go index 9f50f3a0..53096380 100644 --- a/pkg/helmexec/helmexec.go +++ b/pkg/helmexec/helmexec.go @@ -28,7 +28,7 @@ type Interface interface { DecryptSecret(context HelmContext, name string, flags ...string) (string, error) IsHelm3() bool GetVersion() Version - IsVersionAtLeast(major int, minor int, patch int) bool + IsVersionAtLeast(versionStr string) bool } type DependencyUpdater interface { diff --git a/pkg/state/chart_dependency.go b/pkg/state/chart_dependency.go index f7e7ea2e..95d36887 100644 --- a/pkg/state/chart_dependency.go +++ b/pkg/state/chart_dependency.go @@ -2,18 +2,19 @@ package state import ( "fmt" - "github.com/Masterminds/semver" + "io/ioutil" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/Masterminds/semver/v3" goversion "github.com/hashicorp/go-version" "github.com/r3labs/diff" "github.com/roboll/helmfile/pkg/app/version" "github.com/roboll/helmfile/pkg/helmexec" "go.uber.org/zap" "gopkg.in/yaml.v2" - "io/ioutil" - "os" - "path/filepath" - "sort" - "strings" ) type ChartMeta struct { diff --git a/pkg/state/state.go b/pkg/state/state.go index 173ec597..9d60c2f5 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -2069,7 +2069,7 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp if release.CreateNamespace != nil && *release.CreateNamespace || release.CreateNamespace == nil && (st.HelmDefaults.CreateNamespace == nil || *st.HelmDefaults.CreateNamespace) { - if helm.IsVersionAtLeast(3, 2, 0) { + if helm.IsVersionAtLeast("3.2.0") { flags = append(flags, "--create-namespace") } else if release.CreateNamespace != nil || st.HelmDefaults.CreateNamespace != nil { // createNamespace was set explicitly, but not running supported version of helm - error diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 0b78da1c..c661c595 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -8,6 +8,7 @@ import ( "reflect" "testing" + "github.com/Masterminds/semver/v3" "github.com/roboll/helmfile/pkg/exectest" "github.com/roboll/helmfile/pkg/helmexec" "github.com/roboll/helmfile/pkg/testhelper" @@ -165,7 +166,7 @@ func TestHelmState_flagsForUpgrade(t *testing.T) { tests := []struct { name string - version *helmexec.Version + version *semver.Version defaults HelmSpec release *ReleaseSpec want []string @@ -582,11 +583,7 @@ func TestHelmState_flagsForUpgrade(t *testing.T) { defaults: HelmSpec{ Verify: false, }, - version: &helmexec.Version{ - Major: 3, - Minor: 2, - Patch: 0, - }, + version: semver.MustParse("3.2.0"), release: &ReleaseSpec{ Chart: "test/chart", Version: "0.1", @@ -606,11 +603,7 @@ func TestHelmState_flagsForUpgrade(t *testing.T) { Verify: false, CreateNamespace: &disable, }, - version: &helmexec.Version{ - Major: 3, - Minor: 2, - Patch: 0, - }, + version: semver.MustParse("3.2.0"), release: &ReleaseSpec{ Chart: "test/chart", Version: "0.1", @@ -629,11 +622,7 @@ func TestHelmState_flagsForUpgrade(t *testing.T) { Verify: false, CreateNamespace: &disable, }, - version: &helmexec.Version{ - Major: 3, - Minor: 2, - Patch: 0, - }, + version: semver.MustParse("3.2.0"), release: &ReleaseSpec{ Chart: "test/chart", Version: "0.1", @@ -654,11 +643,7 @@ func TestHelmState_flagsForUpgrade(t *testing.T) { Verify: false, CreateNamespace: &enable, }, - version: &helmexec.Version{ - Major: 3, - Minor: 2, - Patch: 0, - }, + version: semver.MustParse("3.2.0"), release: &ReleaseSpec{ Chart: "test/chart", Version: "0.1", @@ -678,11 +663,7 @@ func TestHelmState_flagsForUpgrade(t *testing.T) { Verify: false, CreateNamespace: &enable, }, - version: &helmexec.Version{ - Major: 2, - Minor: 16, - Patch: 0, - }, + version: semver.MustParse("2.16.0"), release: &ReleaseSpec{ Chart: "test/chart", Version: "0.1",