From 10b44a114deef82be64195ab446519b76fd037fb Mon Sep 17 00:00:00 2001 From: Dao Cong Tien Date: Thu, 16 Jan 2020 18:40:24 +0700 Subject: [PATCH 01/22] Add unit test for Reverse() in pkg/releaseutil.go Signed-off-by: Dao Cong Tien --- pkg/releaseutil/sorter_test.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/pkg/releaseutil/sorter_test.go b/pkg/releaseutil/sorter_test.go index 69a6543ad..9544d2018 100644 --- a/pkg/releaseutil/sorter_test.go +++ b/pkg/releaseutil/sorter_test.go @@ -79,3 +79,30 @@ func TestSortByRevision(t *testing.T) { return vi < vj }) } + +func TestReverseSortByName(t *testing.T) { + Reverse(releases, SortByName) + check(t, "ByName", func(i, j int) bool { + ni := releases[i].Name + nj := releases[j].Name + return ni > nj + }) +} + +func TestReverseSortByDate(t *testing.T) { + Reverse(releases, SortByDate) + check(t, "ByDate", func(i, j int) bool { + ti := releases[i].Info.LastDeployed.Second() + tj := releases[j].Info.LastDeployed.Second() + return ti > tj + }) +} + +func TestReverseSortByRevision(t *testing.T) { + Reverse(releases, SortByRevision) + check(t, "ByRevision", func(i, j int) bool { + vi := releases[i].Version + vj := releases[j].Version + return vi > vj + }) +} From 9a2ff7802ff84fcc260c0eff940fa0af3ea91137 Mon Sep 17 00:00:00 2001 From: Daniel Strobusch <1847260+dastrobu@users.noreply.github.com> Date: Thu, 16 Jan 2020 21:43:17 +0100 Subject: [PATCH 02/22] fix(helm): sort hooks by kind for equal weight Use the same install order for hooks as for normal resources (non-hooks) for hooks with equal weight. This makes resource handling more consistent and helps, when there are hook consisting of several resources like e.g. a service account and a job using this service account. The sort functions are changed from an in place search to an out of place sort to avoid inout parameters. Closes #7416. Signed-off-by: Daniel Strobusch <1847260+dastrobu@users.noreply.github.com> --- pkg/action/hooks.go | 3 +- pkg/releaseutil/kind_sorter.go | 78 +++++++++++++++++++------ pkg/releaseutil/kind_sorter_test.go | 53 ++++++++++++++++- pkg/releaseutil/manifest_sorter.go | 2 +- pkg/releaseutil/manifest_sorter_test.go | 2 +- 5 files changed, 114 insertions(+), 24 deletions(-) diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index a161f9377..40c1ffdb6 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -38,7 +38,8 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, } } - sort.Sort(hookByWeight(executingHooks)) + // hooke are pre-ordered by kind, so keep order stable + sort.Stable(hookByWeight(executingHooks)) for _, h := range executingHooks { // Set default delete policy to before-hook-creation diff --git a/pkg/releaseutil/kind_sorter.go b/pkg/releaseutil/kind_sorter.go index 92ffa03f2..3f47ea55d 100644 --- a/pkg/releaseutil/kind_sorter.go +++ b/pkg/releaseutil/kind_sorter.go @@ -16,7 +16,11 @@ limitations under the License. package releaseutil -import "sort" +import ( + "sort" + + "helm.sh/helm/v3/pkg/release" +) // KindSortOrder is an ordering of Kinds. type KindSortOrder []string @@ -99,46 +103,84 @@ var UninstallOrder KindSortOrder = []string{ "Namespace", } -// sortByKind does an in-place sort of manifests by Kind. +// sort manifests by kind (out of place sort) // // Results are sorted by 'ordering', keeping order of items with equal kind/priority -func sortByKind(manifests []Manifest, ordering KindSortOrder) []Manifest { - ks := newKindSorter(manifests, ordering) +func manifestsSortedByKind(manifests []Manifest, ordering KindSortOrder) []Manifest { + k := make([]string, len(manifests)) + for i, h := range manifests { + k[i] = h.Head.Kind + } + ks := newKindSorter(k, ordering) sort.Stable(ks) - return ks.manifests + + // apply permutation + sorted := make([]Manifest, len(manifests)) + for i, p := range ks.permutation { + sorted[i] = manifests[p] + } + return sorted +} + +// sort hooks by kind (out of place sort) +// +// Results are sorted by 'ordering', keeping order of items with equal kind/priority +func hooksSortedByKind(hooks []*release.Hook, ordering KindSortOrder) []*release.Hook { + k := make([]string, len(hooks)) + for i, h := range hooks { + k[i] = h.Kind + } + + ks := newKindSorter(k, ordering) + sort.Stable(ks) + + // apply permutation + sorted := make([]*release.Hook, len(hooks)) + for i, p := range ks.permutation { + sorted[i] = hooks[p] + } + return sorted } type kindSorter struct { - ordering map[string]int - manifests []Manifest + permutation []int + ordering map[string]int + kinds []string } -func newKindSorter(m []Manifest, s KindSortOrder) *kindSorter { +func newKindSorter(kinds []string, s KindSortOrder) *kindSorter { o := make(map[string]int, len(s)) for v, k := range s { o[k] = v } + p := make([]int, len(kinds)) + for i := range p { + p[i] = i + } return &kindSorter{ - manifests: m, - ordering: o, + permutation: p, + kinds: kinds, + ordering: o, } } -func (k *kindSorter) Len() int { return len(k.manifests) } +func (k *kindSorter) Len() int { return len(k.kinds) } -func (k *kindSorter) Swap(i, j int) { k.manifests[i], k.manifests[j] = k.manifests[j], k.manifests[i] } +func (k *kindSorter) Swap(i, j int) { + k.permutation[i], k.permutation[j] = k.permutation[j], k.permutation[i] +} func (k *kindSorter) Less(i, j int) bool { - a := k.manifests[i] - b := k.manifests[j] - first, aok := k.ordering[a.Head.Kind] - second, bok := k.ordering[b.Head.Kind] + a := k.kinds[k.permutation[i]] + b := k.kinds[k.permutation[j]] + first, aok := k.ordering[a] + second, bok := k.ordering[b] if !aok && !bok { // if both are unknown then sort alphabetically by kind, keep original order if same kind - if a.Head.Kind != b.Head.Kind { - return a.Head.Kind < b.Head.Kind + if a != b { + return a < b } return first < second } diff --git a/pkg/releaseutil/kind_sorter_test.go b/pkg/releaseutil/kind_sorter_test.go index 4747e8252..777813667 100644 --- a/pkg/releaseutil/kind_sorter_test.go +++ b/pkg/releaseutil/kind_sorter_test.go @@ -19,6 +19,8 @@ package releaseutil import ( "bytes" "testing" + + "helm.sh/helm/v3/pkg/release" ) func TestKindSorter(t *testing.T) { @@ -175,7 +177,7 @@ func TestKindSorter(t *testing.T) { t.Fatalf("Expected %d names in order, got %d", want, got) } defer buf.Reset() - for _, r := range sortByKind(manifests, test.order) { + for _, r := range manifestsSortedByKind(manifests, test.order) { buf.WriteString(r.Name) } if got := buf.String(); got != test.expected { @@ -236,7 +238,7 @@ func TestKindSorterKeepOriginalOrder(t *testing.T) { var buf bytes.Buffer t.Run(test.description, func(t *testing.T) { defer buf.Reset() - for _, r := range sortByKind(manifests, test.order) { + for _, r := range manifestsSortedByKind(manifests, test.order) { buf.WriteString(r.Name) } if got := buf.String(); got != test.expected { @@ -257,7 +259,7 @@ func TestKindSorterNamespaceAgainstUnknown(t *testing.T) { } manifests := []Manifest{unknown, namespace} - sortByKind(manifests, InstallOrder) + manifests = manifestsSortedByKind(manifests, InstallOrder) expectedOrder := []Manifest{namespace, unknown} for i, manifest := range manifests { @@ -266,3 +268,48 @@ func TestKindSorterNamespaceAgainstUnknown(t *testing.T) { } } } + +// test hook sorting with a small subset of kinds, since it uses the same algorithm as manifestsSortedByKind +func TestKindSorterForHooks(t *testing.T) { + hooks := []*release.Hook{ + { + Name: "i", + Kind: "ClusterRole", + }, + { + Name: "j", + Kind: "ClusterRoleBinding", + }, + { + Name: "c", + Kind: "LimitRange", + }, + { + Name: "a", + Kind: "Namespace", + }, + } + + for _, test := range []struct { + description string + order KindSortOrder + expected string + }{ + {"install", InstallOrder, "acij"}, + {"uninstall", UninstallOrder, "jica"}, + } { + var buf bytes.Buffer + t.Run(test.description, func(t *testing.T) { + if got, want := len(test.expected), len(hooks); got != want { + t.Fatalf("Expected %d names in order, got %d", want, got) + } + defer buf.Reset() + for _, r := range hooksSortedByKind(hooks, test.order) { + buf.WriteString(r.Name) + } + if got := buf.String(); got != test.expected { + t.Errorf("Expected %q, got %q", test.expected, got) + } + }) + } +} diff --git a/pkg/releaseutil/manifest_sorter.go b/pkg/releaseutil/manifest_sorter.go index 24b0c3c95..454977b00 100644 --- a/pkg/releaseutil/manifest_sorter.go +++ b/pkg/releaseutil/manifest_sorter.go @@ -108,7 +108,7 @@ func SortManifests(files map[string]string, apis chartutil.VersionSet, ordering } } - return result.hooks, sortByKind(result.generic, ordering), nil + return hooksSortedByKind(result.hooks, ordering), manifestsSortedByKind(result.generic, ordering), nil } // sort takes a manifestFile object which may contain multiple resource definition diff --git a/pkg/releaseutil/manifest_sorter_test.go b/pkg/releaseutil/manifest_sorter_test.go index 0d2d6660a..29b0d85cf 100644 --- a/pkg/releaseutil/manifest_sorter_test.go +++ b/pkg/releaseutil/manifest_sorter_test.go @@ -219,7 +219,7 @@ metadata: } } - sorted = sortByKind(sorted, InstallOrder) + sorted = manifestsSortedByKind(sorted, InstallOrder) for i, m := range generic { if m.Content != sorted[i].Content { t.Errorf("Expected %q, got %q", m.Content, sorted[i].Content) From 671ceb5514900cbbd40fffc4691710f044aa73c4 Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Thu, 30 Jan 2020 15:12:16 -0800 Subject: [PATCH 03/22] ref(kind_sorter): use an in-place sort for sortManifestsByKind, reduce code duplication Signed-off-by: Matthew Fisher --- pkg/releaseutil/kind_sorter.go | 84 +++++++------------------ pkg/releaseutil/kind_sorter_test.go | 22 +++++-- pkg/releaseutil/manifest_sorter.go | 2 +- pkg/releaseutil/manifest_sorter_test.go | 2 +- 4 files changed, 40 insertions(+), 70 deletions(-) diff --git a/pkg/releaseutil/kind_sorter.go b/pkg/releaseutil/kind_sorter.go index 3f47ea55d..5b131b3b0 100644 --- a/pkg/releaseutil/kind_sorter.go +++ b/pkg/releaseutil/kind_sorter.go @@ -103,84 +103,42 @@ var UninstallOrder KindSortOrder = []string{ "Namespace", } -// sort manifests by kind (out of place sort) +// sort manifests by kind. // // Results are sorted by 'ordering', keeping order of items with equal kind/priority -func manifestsSortedByKind(manifests []Manifest, ordering KindSortOrder) []Manifest { - k := make([]string, len(manifests)) - for i, h := range manifests { - k[i] = h.Head.Kind - } - ks := newKindSorter(k, ordering) - sort.Stable(ks) +func sortManifestsByKind(manifests []Manifest, ordering KindSortOrder) []Manifest { + sort.SliceStable(manifests, func(i, j int) bool { + return lessByKind(manifests[i], manifests[j], manifests[i].Head.Kind, manifests[j].Head.Kind, ordering) + }) - // apply permutation - sorted := make([]Manifest, len(manifests)) - for i, p := range ks.permutation { - sorted[i] = manifests[p] - } - return sorted + return manifests } -// sort hooks by kind (out of place sort) +// sort hooks by kind, using an out-of-place sort to preserve the input parameters. // // Results are sorted by 'ordering', keeping order of items with equal kind/priority -func hooksSortedByKind(hooks []*release.Hook, ordering KindSortOrder) []*release.Hook { - k := make([]string, len(hooks)) - for i, h := range hooks { - k[i] = h.Kind - } - - ks := newKindSorter(k, ordering) - sort.Stable(ks) - - // apply permutation - sorted := make([]*release.Hook, len(hooks)) - for i, p := range ks.permutation { - sorted[i] = hooks[p] - } - return sorted -} +func sortHooksByKind(hooks []*release.Hook, ordering KindSortOrder) []*release.Hook { + h := hooks + sort.SliceStable(h, func(i, j int) bool { + return lessByKind(h[i], h[j], h[i].Kind, h[j].Kind, ordering) + }) -type kindSorter struct { - permutation []int - ordering map[string]int - kinds []string + return h } -func newKindSorter(kinds []string, s KindSortOrder) *kindSorter { - o := make(map[string]int, len(s)) - for v, k := range s { - o[k] = v +func lessByKind(a interface{}, b interface{}, kindA string, kindB string, o KindSortOrder) bool { + ordering := make(map[string]int, len(o)) + for v, k := range o { + ordering[k] = v } - p := make([]int, len(kinds)) - for i := range p { - p[i] = i - } - return &kindSorter{ - permutation: p, - kinds: kinds, - ordering: o, - } -} - -func (k *kindSorter) Len() int { return len(k.kinds) } - -func (k *kindSorter) Swap(i, j int) { - k.permutation[i], k.permutation[j] = k.permutation[j], k.permutation[i] -} - -func (k *kindSorter) Less(i, j int) bool { - a := k.kinds[k.permutation[i]] - b := k.kinds[k.permutation[j]] - first, aok := k.ordering[a] - second, bok := k.ordering[b] + first, aok := ordering[kindA] + second, bok := ordering[kindB] if !aok && !bok { // if both are unknown then sort alphabetically by kind, keep original order if same kind - if a != b { - return a < b + if kindA != kindB { + return kindA < kindB } return first < second } diff --git a/pkg/releaseutil/kind_sorter_test.go b/pkg/releaseutil/kind_sorter_test.go index 777813667..341f528a0 100644 --- a/pkg/releaseutil/kind_sorter_test.go +++ b/pkg/releaseutil/kind_sorter_test.go @@ -177,12 +177,18 @@ func TestKindSorter(t *testing.T) { t.Fatalf("Expected %d names in order, got %d", want, got) } defer buf.Reset() - for _, r := range manifestsSortedByKind(manifests, test.order) { + orig := manifests + for _, r := range sortManifestsByKind(manifests, test.order) { buf.WriteString(r.Name) } if got := buf.String(); got != test.expected { t.Errorf("Expected %q, got %q", test.expected, got) } + for i, manifest := range orig { + if manifest != manifests[i] { + t.Fatal("Expected input to sortManifestsByKind to stay the same") + } + } }) } } @@ -238,7 +244,7 @@ func TestKindSorterKeepOriginalOrder(t *testing.T) { var buf bytes.Buffer t.Run(test.description, func(t *testing.T) { defer buf.Reset() - for _, r := range manifestsSortedByKind(manifests, test.order) { + for _, r := range sortManifestsByKind(manifests, test.order) { buf.WriteString(r.Name) } if got := buf.String(); got != test.expected { @@ -259,7 +265,7 @@ func TestKindSorterNamespaceAgainstUnknown(t *testing.T) { } manifests := []Manifest{unknown, namespace} - manifests = manifestsSortedByKind(manifests, InstallOrder) + manifests = sortManifestsByKind(manifests, InstallOrder) expectedOrder := []Manifest{namespace, unknown} for i, manifest := range manifests { @@ -269,7 +275,7 @@ func TestKindSorterNamespaceAgainstUnknown(t *testing.T) { } } -// test hook sorting with a small subset of kinds, since it uses the same algorithm as manifestsSortedByKind +// test hook sorting with a small subset of kinds, since it uses the same algorithm as sortManifestsByKind func TestKindSorterForHooks(t *testing.T) { hooks := []*release.Hook{ { @@ -304,9 +310,15 @@ func TestKindSorterForHooks(t *testing.T) { t.Fatalf("Expected %d names in order, got %d", want, got) } defer buf.Reset() - for _, r := range hooksSortedByKind(hooks, test.order) { + orig := hooks + for _, r := range sortHooksByKind(hooks, test.order) { buf.WriteString(r.Name) } + for i, hook := range orig { + if hook != hooks[i] { + t.Fatal("Expected input to sortHooksByKind to stay the same") + } + } if got := buf.String(); got != test.expected { t.Errorf("Expected %q, got %q", test.expected, got) } diff --git a/pkg/releaseutil/manifest_sorter.go b/pkg/releaseutil/manifest_sorter.go index 454977b00..e83414500 100644 --- a/pkg/releaseutil/manifest_sorter.go +++ b/pkg/releaseutil/manifest_sorter.go @@ -108,7 +108,7 @@ func SortManifests(files map[string]string, apis chartutil.VersionSet, ordering } } - return hooksSortedByKind(result.hooks, ordering), manifestsSortedByKind(result.generic, ordering), nil + return sortHooksByKind(result.hooks, ordering), sortManifestsByKind(result.generic, ordering), nil } // sort takes a manifestFile object which may contain multiple resource definition diff --git a/pkg/releaseutil/manifest_sorter_test.go b/pkg/releaseutil/manifest_sorter_test.go index 29b0d85cf..20d809317 100644 --- a/pkg/releaseutil/manifest_sorter_test.go +++ b/pkg/releaseutil/manifest_sorter_test.go @@ -219,7 +219,7 @@ metadata: } } - sorted = manifestsSortedByKind(sorted, InstallOrder) + sorted = sortManifestsByKind(sorted, InstallOrder) for i, m := range generic { if m.Content != sorted[i].Content { t.Errorf("Expected %q, got %q", m.Content, sorted[i].Content) From a29365b3c663f1c182ea78461825ae28b8573915 Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Thu, 20 Feb 2020 19:42:47 -0500 Subject: [PATCH 04/22] Adopt resources into release with correct instance and managed-by labels Signed-off-by: Jacob LeGrone --- pkg/action/install.go | 8 ++++++-- pkg/action/upgrade.go | 13 ++++++++++-- pkg/action/validate.go | 45 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 79abfae33..ac57b921c 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -38,6 +38,7 @@ import ( "helm.sh/helm/v3/pkg/downloader" "helm.sh/helm/v3/pkg/engine" "helm.sh/helm/v3/pkg/getter" + "helm.sh/helm/v3/pkg/kube" kubefake "helm.sh/helm/v3/pkg/kube/fake" "helm.sh/helm/v3/pkg/postrender" "helm.sh/helm/v3/pkg/release" @@ -242,6 +243,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // Mark this release as in-progress rel.SetStatus(release.StatusPendingInstall, "Initial install underway") + var adoptedResources kube.ResourceList resources, err := i.cfg.KubeClient.Build(bytes.NewBufferString(rel.Manifest), !i.DisableOpenAPIValidation) if err != nil { return nil, errors.Wrap(err, "unable to build kubernetes objects from release manifest") @@ -254,9 +256,11 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // deleting the release because the manifest will be pointing at that // resource if !i.ClientOnly && !isUpgrade { - if err := existingResourceConflict(resources); err != nil { + toBeUpdated, err := existingResourceConflict(resources, rel.Name) + if err != nil { return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with install") } + adoptedResources = toBeUpdated } // Bail out here if it is a dry run @@ -291,7 +295,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // At this point, we can do the install. Note that before we were detecting whether to // do an update, but it's not clear whether we WANT to do an update if the re-use is set // to true, since that is basically an upgrade operation. - if _, err := i.cfg.KubeClient.Create(resources); err != nil { + if _, err := i.cfg.KubeClient.Update(adoptedResources, resources, false); err != nil { return i.failRelease(rel, err) } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 08b638171..be64fa618 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -216,10 +216,19 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea } } - if err := existingResourceConflict(toBeCreated); err != nil { - return nil, errors.Wrap(err, "rendered manifests contain a new resource that already exists. Unable to continue with update") + toBeUpdated, err := existingResourceConflict(toBeCreated, upgradedRelease.Name) + if err != nil { + return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with update") } + toBeUpdated.Visit(func(r *resource.Info, err error) error { + if err != nil { + return err + } + current.Append(r) + return nil + }) + if u.DryRun { u.cfg.Log("dry run for %s", upgradedRelease.Name) if len(u.Description) > 0 { diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 6bbfc5e8d..feecc4ff3 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -21,13 +21,37 @@ import ( "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" "k8s.io/cli-runtime/pkg/resource" "helm.sh/helm/v3/pkg/kube" ) -func existingResourceConflict(resources kube.ResourceList) error { - err := resources.Visit(func(info *resource.Info, err error) error { +var ( + managedByReq, _ = labels.NewRequirement("app.kubernetes.io/managed-by", selection.Equals, []string{"Helm"}) + accessor = meta.NewAccessor() +) + +func newReleaseSelector(release string) (labels.Selector, error) { + releaseReq, err := labels.NewRequirement("app.kubernetes.io/instance", selection.Equals, []string{release}) + if err != nil { + return nil, err + } + + return labels.Parse(fmt.Sprintf("%s,%s", releaseReq, managedByReq)) +} + +func existingResourceConflict(resources kube.ResourceList, release string) (kube.ResourceList, error) { + sel, err := newReleaseSelector(release) + if err != nil { + return nil, err + } + + requireUpdate := kube.ResourceList{} + + err = resources.Visit(func(info *resource.Info, err error) error { if err != nil { return err } @@ -42,7 +66,20 @@ func existingResourceConflict(resources kube.ResourceList) error { return errors.Wrap(err, "could not get information about the resource") } - return fmt.Errorf("existing resource conflict: namespace: %s, name: %s, existing_kind: %s, new_kind: %s", info.Namespace, info.Name, existing.GetObjectKind().GroupVersionKind(), info.Mapping.GroupVersionKind) + // Allow adoption of the resource if it already has app.kubernetes.io/instance and app.kubernetes.io/managed-by labels. + lbls, err := accessor.Labels(existing) + if err == nil { + set := labels.Set(lbls) + if sel.Matches(set) { + requireUpdate = append(requireUpdate, info) + return nil + } + } + + return fmt.Errorf( + "existing resource conflict: namespace: %s, name: %s, existing_kind: %s, new_kind: %s", + info.Namespace, info.Name, existing.GetObjectKind().GroupVersionKind(), info.Mapping.GroupVersionKind) }) - return err + + return requireUpdate, err } From 271cb2268bfd94012639f4866e8d52189e12a97b Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Thu, 20 Feb 2020 20:53:27 -0500 Subject: [PATCH 05/22] Alternative: annotation-only solution Signed-off-by: Jacob LeGrone --- pkg/action/install.go | 7 +++++- pkg/action/upgrade.go | 7 +++++- pkg/action/validate.go | 57 +++++++++++++++++++++--------------------- 3 files changed, 41 insertions(+), 30 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index ac57b921c..cab55309c 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -249,6 +249,11 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. return nil, errors.Wrap(err, "unable to build kubernetes objects from release manifest") } + err = resources.Visit(setMetadataVisitor(rel.Name, rel.Namespace)) + if err != nil { + return nil, err + } + // Install requires an extra validation step of checking that resources // don't already exist before we actually create resources. If we continue // forward and create the release object with resources that already exist, @@ -256,7 +261,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // deleting the release because the manifest will be pointing at that // resource if !i.ClientOnly && !isUpgrade { - toBeUpdated, err := existingResourceConflict(resources, rel.Name) + toBeUpdated, err := existingResourceConflict(resources, rel.Name, rel.Namespace) if err != nil { return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with install") } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index be64fa618..b42192cda 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -203,6 +203,11 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea return upgradedRelease, errors.Wrap(err, "unable to build kubernetes objects from new release manifest") } + err = target.Visit(setMetadataVisitor(upgradedRelease.Name, upgradedRelease.Namespace)) + if err != nil { + return upgradedRelease, err + } + // Do a basic diff using gvk + name to figure out what new resources are being created so we can validate they don't already exist existingResources := make(map[string]bool) for _, r := range current { @@ -216,7 +221,7 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea } } - toBeUpdated, err := existingResourceConflict(toBeCreated, upgradedRelease.Name) + toBeUpdated, err := existingResourceConflict(toBeCreated, upgradedRelease.Name, upgradedRelease.Namespace) if err != nil { return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with update") } diff --git a/pkg/action/validate.go b/pkg/action/validate.go index feecc4ff3..17a61863e 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -22,36 +22,22 @@ import ( "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/selection" "k8s.io/cli-runtime/pkg/resource" "helm.sh/helm/v3/pkg/kube" ) -var ( - managedByReq, _ = labels.NewRequirement("app.kubernetes.io/managed-by", selection.Equals, []string{"Helm"}) - accessor = meta.NewAccessor() -) - -func newReleaseSelector(release string) (labels.Selector, error) { - releaseReq, err := labels.NewRequirement("app.kubernetes.io/instance", selection.Equals, []string{release}) - if err != nil { - return nil, err - } - - return labels.Parse(fmt.Sprintf("%s,%s", releaseReq, managedByReq)) -} +var accessor = meta.NewAccessor() -func existingResourceConflict(resources kube.ResourceList, release string) (kube.ResourceList, error) { - sel, err := newReleaseSelector(release) - if err != nil { - return nil, err - } +const ( + helmReleaseNameAnnotation = "meta.helm.sh/release-name" + helmReleaseNamespaceAnnotation = "meta.helm.sh/release-namespace" +) +func existingResourceConflict(resources kube.ResourceList, releaseName, releaseNamespace string) (kube.ResourceList, error) { requireUpdate := kube.ResourceList{} - err = resources.Visit(func(info *resource.Info, err error) error { + err := resources.Visit(func(info *resource.Info, err error) error { if err != nil { return err } @@ -67,13 +53,10 @@ func existingResourceConflict(resources kube.ResourceList, release string) (kube } // Allow adoption of the resource if it already has app.kubernetes.io/instance and app.kubernetes.io/managed-by labels. - lbls, err := accessor.Labels(existing) - if err == nil { - set := labels.Set(lbls) - if sel.Matches(set) { - requireUpdate = append(requireUpdate, info) - return nil - } + anno, err := accessor.Annotations(existing) + if err == nil && anno != nil && anno[helmReleaseNameAnnotation] == releaseName && anno[helmReleaseNamespaceAnnotation] == releaseNamespace { + requireUpdate = append(requireUpdate, info) + return nil } return fmt.Errorf( @@ -83,3 +66,21 @@ func existingResourceConflict(resources kube.ResourceList, release string) (kube return requireUpdate, err } + +func setMetadataVisitor(releaseName, releaseNamespace string) resource.VisitorFunc { + return func(info *resource.Info, err error) error { + if err != nil { + return err + } + anno, err := accessor.Annotations(info.Object) + if err != nil { + return err + } + if anno == nil { + anno = make(map[string]string) + } + anno[helmReleaseNameAnnotation] = releaseName + anno[helmReleaseNamespaceAnnotation] = releaseNamespace + return accessor.SetAnnotations(info.Object, anno) + } +} From b864fcd0f4a86430b867ad3c48544944197d3b29 Mon Sep 17 00:00:00 2001 From: Mikhail Gusarov Date: Mon, 2 Mar 2020 19:05:14 +0100 Subject: [PATCH 06/22] Make the charts cache safe in presence of several Helm instances If several instances of Helm are run at the same moment and try to download the same chart, some of them might see an empty or incomplete file in cache. Prevent that by saving the dowloaded file atomically. Closes #7600 Signed-off-by: Mikhail Gusarov --- pkg/downloader/chart_downloader.go | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index f3d4321c5..a9956f477 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -72,6 +72,31 @@ type ChartDownloader struct { RepositoryCache string } +// atomicWriteFile atomically (as atomic as os.Rename allows) writes a file to a +// disk. +func atomicWriteFile(filename string, body io.Reader, mode os.FileMode) error { + tempFile, err := ioutil.TempFile(filepath.Split(filename)) + if err != nil { + return err + } + tempName := tempFile.Name() + + if _, err := io.Copy(tempFile, body); err != nil { + tempFile.Close() // return value is ignored as we are already on error path + return err + } + + if err := tempFile.Close(); err != nil { + return err + } + + if err := os.Chmod(tempName, mode); err != nil { + return err + } + + return os.Rename(tempName, filename) +} + // DownloadTo retrieves a chart. Depending on the settings, it may also download a provenance file. // // If Verify is set to VerifyNever, the verification will be nil. @@ -101,7 +126,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven name := filepath.Base(u.Path) destfile := filepath.Join(dest, name) - if err := ioutil.WriteFile(destfile, data.Bytes(), 0644); err != nil { + if err := atomicWriteFile(destfile, data, 0644); err != nil { return destfile, nil, err } @@ -117,7 +142,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven return destfile, ver, nil } provfile := destfile + ".prov" - if err := ioutil.WriteFile(provfile, body.Bytes(), 0644); err != nil { + if err := atomicWriteFile(provfile, body, 0644); err != nil { return destfile, nil, err } From d829343c1514db17bee7a90624d06cdfbffde963 Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Thu, 5 Mar 2020 21:41:30 -0500 Subject: [PATCH 07/22] Use Create method if no resources need to be adopted Signed-off-by: Jacob LeGrone --- pkg/action/install.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index cab55309c..d6fd829e5 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -243,7 +243,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // Mark this release as in-progress rel.SetStatus(release.StatusPendingInstall, "Initial install underway") - var adoptedResources kube.ResourceList + var toBeAdopted kube.ResourceList resources, err := i.cfg.KubeClient.Build(bytes.NewBufferString(rel.Manifest), !i.DisableOpenAPIValidation) if err != nil { return nil, errors.Wrap(err, "unable to build kubernetes objects from release manifest") @@ -261,11 +261,10 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // deleting the release because the manifest will be pointing at that // resource if !i.ClientOnly && !isUpgrade { - toBeUpdated, err := existingResourceConflict(resources, rel.Name, rel.Namespace) + toBeAdopted, err = existingResourceConflict(resources, rel.Name, rel.Namespace) if err != nil { return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with install") } - adoptedResources = toBeUpdated } // Bail out here if it is a dry run @@ -300,8 +299,14 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // At this point, we can do the install. Note that before we were detecting whether to // do an update, but it's not clear whether we WANT to do an update if the re-use is set // to true, since that is basically an upgrade operation. - if _, err := i.cfg.KubeClient.Update(adoptedResources, resources, false); err != nil { - return i.failRelease(rel, err) + if len(toBeAdopted) == 0 { + if _, err := i.cfg.KubeClient.Create(resources); err != nil { + return i.failRelease(rel, err) + } + } else { + if _, err := i.cfg.KubeClient.Update(toBeAdopted, resources, false); err != nil { + return i.failRelease(rel, err) + } } if i.Wait { From a6994f6659d9801cca60ebfb1a12366fb7e2617f Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Fri, 6 Mar 2020 16:57:25 +0000 Subject: [PATCH 08/22] Port --devel flag from v2 to v3 Helm v2 PR #5141 Signed-off-by: Martin Hickey --- cmd/helm/show.go | 49 +++++++++++++++++++++++++--------------------- go.mod | 2 ++ go.sum | 4 ++++ pkg/action/show.go | 3 ++- 4 files changed, 35 insertions(+), 23 deletions(-) diff --git a/cmd/helm/show.go b/cmd/helm/show.go index a82ad2777..ac38ed5af 100644 --- a/cmd/helm/show.go +++ b/cmd/helm/show.go @@ -77,11 +77,7 @@ func newShowCmd(out io.Writer) *cobra.Command { Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowAll - cp, err := client.ChartPathOptions.LocateChart(args[0], settings) - if err != nil { - return err - } - output, err := client.Run(cp) + output, err := runShow(args, client) if err != nil { return err } @@ -97,11 +93,7 @@ func newShowCmd(out io.Writer) *cobra.Command { Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowValues - cp, err := client.ChartPathOptions.LocateChart(args[0], settings) - if err != nil { - return err - } - output, err := client.Run(cp) + output, err := runShow(args, client) if err != nil { return err } @@ -117,11 +109,7 @@ func newShowCmd(out io.Writer) *cobra.Command { Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowChart - cp, err := client.ChartPathOptions.LocateChart(args[0], settings) - if err != nil { - return err - } - output, err := client.Run(cp) + output, err := runShow(args, client) if err != nil { return err } @@ -137,11 +125,7 @@ func newShowCmd(out io.Writer) *cobra.Command { Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowReadme - cp, err := client.ChartPathOptions.LocateChart(args[0], settings) - if err != nil { - return err - } - output, err := client.Run(cp) + output, err := runShow(args, client) if err != nil { return err } @@ -152,8 +136,7 @@ func newShowCmd(out io.Writer) *cobra.Command { cmds := []*cobra.Command{all, readmeSubCmd, valuesSubCmd, chartSubCmd} for _, subCmd := range cmds { - addChartPathOptionsFlags(subCmd.Flags(), &client.ChartPathOptions) - showCommand.AddCommand(subCmd) + addShowFlags(showCommand, subCmd, client) // Register the completion function for each subcommand completion.RegisterValidArgsFunc(subCmd, validArgsFunc) @@ -161,3 +144,25 @@ func newShowCmd(out io.Writer) *cobra.Command { return showCommand } + +func addShowFlags(showCmd *cobra.Command, subCmd *cobra.Command, client *action.Show) { + f := subCmd.Flags() + + f.BoolVar(&client.Devel, "devel", false, "use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored") + addChartPathOptionsFlags(f, &client.ChartPathOptions) + showCmd.AddCommand(subCmd) +} + +func runShow(args []string, client *action.Show) (string, error) { + debug("Original chart version: %q", client.Version) + if client.Version == "" && client.Devel { + debug("setting version to >0.0.0-0") + client.Version = ">0.0.0-0" + } + + cp, err := client.ChartPathOptions.LocateChart(args[0], settings) + if err != nil { + return "", err + } + return client.Run(cp) +} diff --git a/go.mod b/go.mod index 7ba7a5542..bfd557ff0 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.13 require ( github.com/BurntSushi/toml v0.3.1 + github.com/Masterminds/semver v1.5.0 // indirect github.com/Masterminds/semver/v3 v3.0.3 github.com/Masterminds/sprig/v3 v3.0.2 github.com/Masterminds/vcs v1.13.1 @@ -34,6 +35,7 @@ require ( k8s.io/apimachinery v0.17.3 k8s.io/cli-runtime v0.17.3 k8s.io/client-go v0.17.3 + k8s.io/helm v2.16.3+incompatible k8s.io/klog v1.0.0 k8s.io/kubectl v0.17.3 sigs.k8s.io/yaml v1.1.0 diff --git a/go.sum b/go.sum index 4d8cc83a2..7495b2263 100644 --- a/go.sum +++ b/go.sum @@ -28,6 +28,8 @@ github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd h1:sjQovDkwrZp github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd/go.mod h1:64YHyfSL2R96J44Nlwm39UHepQbyR5q10x7iYa1ks2E= github.com/Masterminds/goutils v1.1.0 h1:zukEsf/1JZwCMgHiK3GZftabmxiCw4apj3a28RPBiVg= github.com/Masterminds/goutils v1.1.0/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU= +github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= +github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= github.com/Masterminds/semver/v3 v3.0.3 h1:znjIyLfpXEDQjOIEWh+ehwpTU14UzUPub3c3sm36u14= github.com/Masterminds/semver/v3 v3.0.3/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Masterminds/sprig/v3 v3.0.2 h1:wz22D0CiSctrliXiI9ZO3HoNApweeRGftyDN+BQa3B8= @@ -653,6 +655,8 @@ k8s.io/component-base v0.17.3 h1:hQzTSshY14aLSR6WGIYvmw+w+u6V4d+iDR2iDGMrlUg= k8s.io/component-base v0.17.3/go.mod h1:GeQf4BrgelWm64PXkIXiPh/XS0hnO42d9gx9BtbZRp8= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/gengo v0.0.0-20190822140433-26a664648505/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= +k8s.io/helm v2.16.3+incompatible h1:MGUcXcG1uAXWZmxu4vzzgRjZOnfFUsSJbHgqM+kyqzM= +k8s.io/helm v2.16.3+incompatible/go.mod h1:LZzlS4LQBHfciFOurYBFkCMTaZ0D1l+p0teMg7TSULI= k8s.io/klog v0.0.0-20181102134211-b9b56d5dfc92/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= k8s.io/klog v0.3.0/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= k8s.io/klog v1.0.0 h1:Pt+yjF5aB1xDSVbau4VsWe+dQNzA0qv1LlXdC2dF6Q8= diff --git a/pkg/action/show.go b/pkg/action/show.go index 14b59a5ea..cc85477cd 100644 --- a/pkg/action/show.go +++ b/pkg/action/show.go @@ -51,8 +51,9 @@ func (o ShowOutputFormat) String() string { // // It provides the implementation of 'helm show' and its respective subcommands. type Show struct { - OutputFormat ShowOutputFormat ChartPathOptions + Devel bool + OutputFormat ShowOutputFormat } // NewShow creates a new Show object with the given configuration. From c8d8007c7aba5b66d185f4c82cc19a16c8246dd7 Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Fri, 6 Mar 2020 17:06:52 +0000 Subject: [PATCH 09/22] Fix stray modules Signed-off-by: Martin Hickey --- go.mod | 2 -- go.sum | 4 ---- 2 files changed, 6 deletions(-) diff --git a/go.mod b/go.mod index bfd557ff0..7ba7a5542 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,6 @@ go 1.13 require ( github.com/BurntSushi/toml v0.3.1 - github.com/Masterminds/semver v1.5.0 // indirect github.com/Masterminds/semver/v3 v3.0.3 github.com/Masterminds/sprig/v3 v3.0.2 github.com/Masterminds/vcs v1.13.1 @@ -35,7 +34,6 @@ require ( k8s.io/apimachinery v0.17.3 k8s.io/cli-runtime v0.17.3 k8s.io/client-go v0.17.3 - k8s.io/helm v2.16.3+incompatible k8s.io/klog v1.0.0 k8s.io/kubectl v0.17.3 sigs.k8s.io/yaml v1.1.0 diff --git a/go.sum b/go.sum index 7495b2263..4d8cc83a2 100644 --- a/go.sum +++ b/go.sum @@ -28,8 +28,6 @@ github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd h1:sjQovDkwrZp github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd/go.mod h1:64YHyfSL2R96J44Nlwm39UHepQbyR5q10x7iYa1ks2E= github.com/Masterminds/goutils v1.1.0 h1:zukEsf/1JZwCMgHiK3GZftabmxiCw4apj3a28RPBiVg= github.com/Masterminds/goutils v1.1.0/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU= -github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= -github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= github.com/Masterminds/semver/v3 v3.0.3 h1:znjIyLfpXEDQjOIEWh+ehwpTU14UzUPub3c3sm36u14= github.com/Masterminds/semver/v3 v3.0.3/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Masterminds/sprig/v3 v3.0.2 h1:wz22D0CiSctrliXiI9ZO3HoNApweeRGftyDN+BQa3B8= @@ -655,8 +653,6 @@ k8s.io/component-base v0.17.3 h1:hQzTSshY14aLSR6WGIYvmw+w+u6V4d+iDR2iDGMrlUg= k8s.io/component-base v0.17.3/go.mod h1:GeQf4BrgelWm64PXkIXiPh/XS0hnO42d9gx9BtbZRp8= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/gengo v0.0.0-20190822140433-26a664648505/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= -k8s.io/helm v2.16.3+incompatible h1:MGUcXcG1uAXWZmxu4vzzgRjZOnfFUsSJbHgqM+kyqzM= -k8s.io/helm v2.16.3+incompatible/go.mod h1:LZzlS4LQBHfciFOurYBFkCMTaZ0D1l+p0teMg7TSULI= k8s.io/klog v0.0.0-20181102134211-b9b56d5dfc92/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= k8s.io/klog v0.3.0/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= k8s.io/klog v1.0.0 h1:Pt+yjF5aB1xDSVbau4VsWe+dQNzA0qv1LlXdC2dF6Q8= From d59493adffb2fcafb6ed17aaa21ee37498e3a48b Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Mon, 9 Mar 2020 11:46:36 +0000 Subject: [PATCH 10/22] Add unit test Signed-off-by: Martin Hickey --- cmd/helm/show_test.go | 82 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 cmd/helm/show_test.go diff --git a/cmd/helm/show_test.go b/cmd/helm/show_test.go new file mode 100644 index 000000000..00d7c8145 --- /dev/null +++ b/cmd/helm/show_test.go @@ -0,0 +1,82 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "fmt" + "path/filepath" + "strings" + "testing" + + "helm.sh/helm/v3/pkg/repo/repotest" +) + +func TestShowPreReleaseChart(t *testing.T) { + srv, err := repotest.NewTempServer("testdata/testcharts/*.tgz*") + if err != nil { + t.Fatal(err) + } + defer srv.Stop() + + if err := srv.LinkIndices(); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + args string + flags string + fail bool + expectedErr string + }{ + { + name: "show pre-release chart", + args: "test/pre-release-chart", + fail: true, + expectedErr: "failed to download \"test/pre-release-chart\"", + }, + { + name: "show pre-release chart with 'devel' flag", + args: "test/pre-release-chart", + flags: "--devel", + fail: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + outdir := srv.Root() + cmd := fmt.Sprintf("show all '%s' %s --repository-config %s --repository-cache %s", + tt.args, + tt.flags, + filepath.Join(outdir, "repositories.yaml"), + outdir, + ) + //_, out, err := executeActionCommand(cmd) + _, _, err := executeActionCommand(cmd) + if err != nil { + if tt.fail { + if !strings.Contains(err.Error(), tt.expectedErr) { + t.Errorf("%q expected error: %s, got: %s", tt.name, tt.expectedErr, err.Error()) + } + return + } + t.Errorf("%q reported error: %s", tt.name, err) + } + }) + } +} From 51dd8313bcbc03db3f266dc6217d6fce70fe39d3 Mon Sep 17 00:00:00 2001 From: Anshul Verma Date: Tue, 10 Mar 2020 03:35:59 +0530 Subject: [PATCH 11/22] Solve the issue #7749 where proper formating was not being done if --short(-q) option was used with other formating options like json, yaml Signed-off-by: Anshul Verma --- cmd/helm/list.go | 21 ++++++++++++++++++++ cmd/helm/list_test.go | 10 ++++++++++ cmd/helm/testdata/output/list-short-json.txt | 0 cmd/helm/testdata/output/list-short-yaml.txt | 0 4 files changed, 31 insertions(+) create mode 100644 cmd/helm/testdata/output/list-short-json.txt create mode 100644 cmd/helm/testdata/output/list-short-yaml.txt diff --git a/cmd/helm/list.go b/cmd/helm/list.go index 4b652088d..59364381e 100644 --- a/cmd/helm/list.go +++ b/cmd/helm/list.go @@ -83,6 +83,27 @@ func newListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } if client.Short { + + names := make([]string, 0) + for _, res := range results { + //fmt.Fprintln(out, res.Name) + names = append(names, res.Name) + } + + outputFlag := cmd.Flag("output") + if outputFlag.Changed { + switch outputFlag.Value.String() { + case "json": + output.EncodeJSON(out, names) + return nil + case "yaml": + output.EncodeYAML(out, names) + return nil + default: + return outfmt.Write(out, newReleaseListWriter(results)) + } + } + for _, res := range results { fmt.Fprintln(out, res.Name) } diff --git a/cmd/helm/list_test.go b/cmd/helm/list_test.go index fe773a803..dadb57b94 100644 --- a/cmd/helm/list_test.go +++ b/cmd/helm/list_test.go @@ -198,6 +198,16 @@ func TestListCmd(t *testing.T) { cmd: "list --short", golden: "output/list-short.txt", rels: releaseFixture, + }, { + name: "list releases in short output format", + cmd: "list --short --output yaml", + golden: "output/list-short-yaml.txt", + rels: releaseFixture, + }, { + name: "list releases in short output format", + cmd: "list --short --output json", + golden: "output/list-short-json.txt", + rels: releaseFixture, }, { name: "list superseded releases", cmd: "list --superseded", diff --git a/cmd/helm/testdata/output/list-short-json.txt b/cmd/helm/testdata/output/list-short-json.txt new file mode 100644 index 000000000..e69de29bb diff --git a/cmd/helm/testdata/output/list-short-yaml.txt b/cmd/helm/testdata/output/list-short-yaml.txt new file mode 100644 index 000000000..e69de29bb From 7470337d32cef71043a3da1a23a6b3004e618ed6 Mon Sep 17 00:00:00 2001 From: Anshul Verma Date: Tue, 10 Mar 2020 03:42:43 +0530 Subject: [PATCH 12/22] Solve the issue #7749 where proper formating was not being done if --short(-q) option was used with other formating options like json, yaml Signed-off-by: Anshul Verma --- cmd/helm/testdata/output/list-short-json.txt | 1 + cmd/helm/testdata/output/list-short-yaml.txt | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/cmd/helm/testdata/output/list-short-json.txt b/cmd/helm/testdata/output/list-short-json.txt index e69de29bb..1ab52136b 100644 --- a/cmd/helm/testdata/output/list-short-json.txt +++ b/cmd/helm/testdata/output/list-short-json.txt @@ -0,0 +1 @@ +["hummingbird","iguana","rocket","starlord"] \ No newline at end of file diff --git a/cmd/helm/testdata/output/list-short-yaml.txt b/cmd/helm/testdata/output/list-short-yaml.txt index e69de29bb..41459282f 100644 --- a/cmd/helm/testdata/output/list-short-yaml.txt +++ b/cmd/helm/testdata/output/list-short-yaml.txt @@ -0,0 +1,4 @@ +- hummingbird +- iguana +- rocket +- starlord \ No newline at end of file From c354de80e5ab2a4ff07007353451dcb1b50b5801 Mon Sep 17 00:00:00 2001 From: Anshul Verma Date: Tue, 10 Mar 2020 03:47:21 +0530 Subject: [PATCH 13/22] Solve the issue #7749 where proper formating was not being done if --short(-q) option was used with other formating options like json, yaml Signed-off-by: Anshul Verma --- cmd/helm/testdata/output/list-short-json.txt | 2 +- cmd/helm/testdata/output/list-short-yaml.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/helm/testdata/output/list-short-json.txt b/cmd/helm/testdata/output/list-short-json.txt index 1ab52136b..acbf1e44d 100644 --- a/cmd/helm/testdata/output/list-short-json.txt +++ b/cmd/helm/testdata/output/list-short-json.txt @@ -1 +1 @@ -["hummingbird","iguana","rocket","starlord"] \ No newline at end of file +["hummingbird","iguana","rocket","starlord"] diff --git a/cmd/helm/testdata/output/list-short-yaml.txt b/cmd/helm/testdata/output/list-short-yaml.txt index 41459282f..86fb3d670 100644 --- a/cmd/helm/testdata/output/list-short-yaml.txt +++ b/cmd/helm/testdata/output/list-short-yaml.txt @@ -1,4 +1,4 @@ - hummingbird - iguana - rocket -- starlord \ No newline at end of file +- starlord From 4113fc8951a891a934028081417fc70e1a1b1f63 Mon Sep 17 00:00:00 2001 From: Anshul Verma Date: Wed, 11 Mar 2020 01:04:50 +0530 Subject: [PATCH 14/22] Solve the issue #7749 where proper formating was not being done if --short(-q) option was used with other formating options like json, yaml Signed-off-by: Anshul Verma --- cmd/helm/list.go | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/cmd/helm/list.go b/cmd/helm/list.go index 59364381e..08d6beb79 100644 --- a/cmd/helm/list.go +++ b/cmd/helm/list.go @@ -86,28 +86,26 @@ func newListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { names := make([]string, 0) for _, res := range results { - //fmt.Fprintln(out, res.Name) names = append(names, res.Name) } outputFlag := cmd.Flag("output") - if outputFlag.Changed { - switch outputFlag.Value.String() { - case "json": - output.EncodeJSON(out, names) - return nil - case "yaml": - output.EncodeYAML(out, names) - return nil - default: - return outfmt.Write(out, newReleaseListWriter(results)) - } - } - for _, res := range results { - fmt.Fprintln(out, res.Name) + switch outputFlag.Value.String() { + case "json": + output.EncodeJSON(out, names) + return nil + case "yaml": + output.EncodeYAML(out, names) + return nil + case "table": + for _, res := range results { + fmt.Fprintln(out, res.Name) + } + return nil + default: + return outfmt.Write(out, newReleaseListWriter(results)) } - return nil } return outfmt.Write(out, newReleaseListWriter(results)) From 8d1de39fe3234c8925dac6d3efca6aba687ebf87 Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Tue, 10 Mar 2020 22:22:15 -0400 Subject: [PATCH 15/22] Add more detail to error messages and support a non-force mode in metadata visitor Signed-off-by: Jacob LeGrone --- pkg/action/install.go | 3 +- pkg/action/upgrade.go | 3 +- pkg/action/validate.go | 136 +++++++++++++++++++++++++++++++++++------ 3 files changed, 121 insertions(+), 21 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index d6fd829e5..f7e8f77d1 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -249,7 +249,8 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. return nil, errors.Wrap(err, "unable to build kubernetes objects from release manifest") } - err = resources.Visit(setMetadataVisitor(rel.Name, rel.Namespace)) + // It is safe to use "force" here because these are resources currently rendered by the chart. + err = resources.Visit(setMetadataVisitor(rel.Name, rel.Namespace, true)) if err != nil { return nil, err } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index b42192cda..0741ef19f 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -203,7 +203,8 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea return upgradedRelease, errors.Wrap(err, "unable to build kubernetes objects from new release manifest") } - err = target.Visit(setMetadataVisitor(upgradedRelease.Name, upgradedRelease.Namespace)) + // It is safe to use force only on target because these are resources currently rendered by the chart. + err = target.Visit(setMetadataVisitor(upgradedRelease.Name, upgradedRelease.Namespace, true)) if err != nil { return upgradedRelease, err } diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 17a61863e..0c40a9c3c 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -22,6 +22,7 @@ import ( "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/cli-runtime/pkg/resource" "helm.sh/helm/v3/pkg/kube" @@ -30,12 +31,14 @@ import ( var accessor = meta.NewAccessor() const ( + appManagedByLabel = "app.kubernetes.io/managed-by" + appManagedByHelm = "Helm" helmReleaseNameAnnotation = "meta.helm.sh/release-name" helmReleaseNamespaceAnnotation = "meta.helm.sh/release-namespace" ) func existingResourceConflict(resources kube.ResourceList, releaseName, releaseNamespace string) (kube.ResourceList, error) { - requireUpdate := kube.ResourceList{} + var requireUpdate kube.ResourceList err := resources.Visit(func(info *resource.Info, err error) error { if err != nil { @@ -48,39 +51,134 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN if apierrors.IsNotFound(err) { return nil } - return errors.Wrap(err, "could not get information about the resource") } - // Allow adoption of the resource if it already has app.kubernetes.io/instance and app.kubernetes.io/managed-by labels. - anno, err := accessor.Annotations(existing) - if err == nil && anno != nil && anno[helmReleaseNameAnnotation] == releaseName && anno[helmReleaseNamespaceAnnotation] == releaseNamespace { - requireUpdate = append(requireUpdate, info) - return nil + // Allow adoption of the resource if it is managed by Helm and is annotated with correct release name and namespace. + if err := checkOwnership(existing, releaseName, releaseNamespace); err != nil { + return fmt.Errorf("%s exists and cannot be imported into the current release: %s", resourceString(info), err) } - return fmt.Errorf( - "existing resource conflict: namespace: %s, name: %s, existing_kind: %s, new_kind: %s", - info.Namespace, info.Name, existing.GetObjectKind().GroupVersionKind(), info.Mapping.GroupVersionKind) + requireUpdate.Append(info) + return nil }) return requireUpdate, err } -func setMetadataVisitor(releaseName, releaseNamespace string) resource.VisitorFunc { +func checkOwnership(obj runtime.Object, releaseName, releaseNamespace string) error { + lbls, err := accessor.Labels(obj) + if err != nil { + return err + } + annos, err := accessor.Annotations(obj) + if err != nil { + return err + } + + var errs []error + if err := requireValue(lbls, appManagedByLabel, appManagedByHelm); err != nil { + errs = append(errs, fmt.Errorf("label validation error: %s", err)) + } + if err := requireValue(annos, helmReleaseNameAnnotation, releaseName); err != nil { + errs = append(errs, fmt.Errorf("annotation validation error: %s", err)) + } + if err := requireValue(annos, helmReleaseNamespaceAnnotation, releaseNamespace); err != nil { + errs = append(errs, fmt.Errorf("annotation validation error: %s", err)) + } + + if len(errs) > 0 { + err := errors.New("invalid ownership metadata") + for _, e := range errs { + err = fmt.Errorf("%w; %s", err, e) + } + return err + } + + return nil +} + +func requireValue(meta map[string]string, k, v string) error { + actual, ok := meta[k] + if !ok { + return fmt.Errorf("missing key %q: must be set to %q", k, v) + } + if actual != v { + return fmt.Errorf("key %q must equal %q: current value is %q", k, v, actual) + } + return nil +} + +// setMetadataVisitor adds release tracking metadata to all resources. If force is enabled, existing +// ownership metadata will be overwritten. Otherwise an error will be returned if any resource has an +// existing and conflicting value for the managed by label or Helm release/namespace annotations. +func setMetadataVisitor(releaseName, releaseNamespace string, force bool) resource.VisitorFunc { return func(info *resource.Info, err error) error { if err != nil { return err } - anno, err := accessor.Annotations(info.Object) - if err != nil { - return err + + if !force { + if err := checkOwnership(info.Object, releaseName, releaseNamespace); err != nil { + return fmt.Errorf("%s cannot be owned: %s", resourceString(info), err) + } } - if anno == nil { - anno = make(map[string]string) + + if err := mergeLabels(info.Object, map[string]string{ + appManagedByLabel: appManagedByHelm, + }); err != nil { + return fmt.Errorf( + "%s labels could not be updated: %s", + resourceString(info), err, + ) + } + + if err := mergeAnnotations(info.Object, map[string]string{ + helmReleaseNameAnnotation: releaseName, + helmReleaseNamespaceAnnotation: releaseNamespace, + }); err != nil { + return fmt.Errorf( + "%s annotations could not be updated: %s", + resourceString(info), err, + ) } - anno[helmReleaseNameAnnotation] = releaseName - anno[helmReleaseNamespaceAnnotation] = releaseNamespace - return accessor.SetAnnotations(info.Object, anno) + + return nil + } +} + +func resourceString(info *resource.Info) string { + _, k := info.Mapping.GroupVersionKind.ToAPIVersionAndKind() + return fmt.Sprintf( + "%s %q in namespace %q", + k, info.Name, info.Namespace, + ) +} + +func mergeLabels(obj runtime.Object, labels map[string]string) error { + current, err := accessor.Labels(obj) + if err != nil { + return err + } + return accessor.SetLabels(obj, mergeStrStrMaps(current, labels)) +} + +func mergeAnnotations(obj runtime.Object, annotations map[string]string) error { + current, err := accessor.Annotations(obj) + if err != nil { + return err + } + return accessor.SetAnnotations(obj, mergeStrStrMaps(current, annotations)) +} + +// merge two maps, always taking the value on the right +func mergeStrStrMaps(current, desired map[string]string) map[string]string { + result := make(map[string]string) + for k, v := range current { + result[k] = v + } + for k, desiredVal := range desired { + result[k] = desiredVal } + return result } From 9496a7474bf3bcf8bb0b7efc953b85174ae8d8da Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Tue, 10 Mar 2020 22:22:33 -0400 Subject: [PATCH 16/22] Add tests Signed-off-by: Jacob LeGrone --- pkg/action/validate_test.go | 123 ++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 pkg/action/validate_test.go diff --git a/pkg/action/validate_test.go b/pkg/action/validate_test.go new file mode 100644 index 000000000..a9c1cb49c --- /dev/null +++ b/pkg/action/validate_test.go @@ -0,0 +1,123 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "testing" + + "helm.sh/helm/v3/pkg/kube" + + appsv1 "k8s.io/api/apps/v1" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/api/meta" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/cli-runtime/pkg/resource" +) + +func newDeploymentResource(name, namespace string) *resource.Info { + return &resource.Info{ + Name: name, + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployment"}, + GroupVersionKind: schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, + }, + Object: &appsv1.Deployment{ + ObjectMeta: v1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + }, + } +} + +func TestCheckOwnership(t *testing.T) { + deployFoo := newDeploymentResource("foo", "ns-a") + + // Verify that a resource that lacks labels/annotations is not owned + err := checkOwnership(deployFoo.Object, "rel-a", "ns-a") + assert.EqualError(t, err, `invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "rel-a"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "ns-a"`) + + // Set managed by label and verify annotation error message + _ = accessor.SetLabels(deployFoo.Object, map[string]string{ + appManagedByLabel: appManagedByHelm, + }) + err = checkOwnership(deployFoo.Object, "rel-a", "ns-a") + assert.EqualError(t, err, `invalid ownership metadata; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "rel-a"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "ns-a"`) + + // Set only the release name annotation and verify missing release namespace error message + _ = accessor.SetAnnotations(deployFoo.Object, map[string]string{ + helmReleaseNameAnnotation: "rel-a", + }) + err = checkOwnership(deployFoo.Object, "rel-a", "ns-a") + assert.EqualError(t, err, `invalid ownership metadata; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "ns-a"`) + + // Set both release name and namespace annotations and verify no ownership errors + _ = accessor.SetAnnotations(deployFoo.Object, map[string]string{ + helmReleaseNameAnnotation: "rel-a", + helmReleaseNamespaceAnnotation: "ns-a", + }) + err = checkOwnership(deployFoo.Object, "rel-a", "ns-a") + assert.NoError(t, err) + + // Verify ownership error for wrong release name + err = checkOwnership(deployFoo.Object, "rel-b", "ns-a") + assert.EqualError(t, err, `invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-name" must equal "rel-b": current value is "rel-a"`) + + // Verify ownership error for wrong release namespace + err = checkOwnership(deployFoo.Object, "rel-a", "ns-b") + assert.EqualError(t, err, `invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-namespace" must equal "ns-b": current value is "ns-a"`) + + // Verify ownership error for wrong manager label + _ = accessor.SetLabels(deployFoo.Object, map[string]string{ + appManagedByLabel: "helm", + }) + err = checkOwnership(deployFoo.Object, "rel-a", "ns-a") + assert.EqualError(t, err, `invalid ownership metadata; label validation error: key "app.kubernetes.io/managed-by" must equal "Helm": current value is "helm"`) +} + +func TestSetMetadataVisitor(t *testing.T) { + var ( + err error + deployFoo = newDeploymentResource("foo", "ns-a") + deployBar = newDeploymentResource("bar", "ns-a-system") + resources = kube.ResourceList{deployFoo, deployBar} + ) + + // Set release tracking metadata and verify no error + err = resources.Visit(setMetadataVisitor("rel-a", "ns-a", true)) + assert.NoError(t, err) + + // Verify that release "b" cannot take ownership of "a" + err = resources.Visit(setMetadataVisitor("rel-b", "ns-a", false)) + assert.Error(t, err) + + // Force release "b" to take ownership + err = resources.Visit(setMetadataVisitor("rel-b", "ns-a", true)) + assert.NoError(t, err) + + // Check that there is now no ownership error when setting metadata without force + err = resources.Visit(setMetadataVisitor("rel-b", "ns-a", false)) + assert.NoError(t, err) + + // Add a new resource that is missing ownership metadata and verify error + resources.Append(newDeploymentResource("baz", "default")) + err = resources.Visit(setMetadataVisitor("rel-b", "ns-a", false)) + assert.Error(t, err) + assert.Contains(t, err.Error(), `Deployment "baz" in namespace "" cannot be owned`) +} From 4feed2de1b7e031c5a560dded8f69cfbea17d93e Mon Sep 17 00:00:00 2001 From: Bridget Kromhout Date: Thu, 12 Mar 2020 14:15:05 -0500 Subject: [PATCH 17/22] Correcting links for release notes Signed-off-by: Bridget Kromhout --- scripts/release-notes.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/release-notes.sh b/scripts/release-notes.sh index 3299eeddc..dd48d4a17 100755 --- a/scripts/release-notes.sh +++ b/scripts/release-notes.sh @@ -81,14 +81,14 @@ The community keeps growing, and we'd love to see you there! Download Helm ${RELEASE}. The common platform binaries are here: -- [MacOS amd64](https://get.helm.sh/helm-${RELEASE}-darwin-amd64.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-darwin-amd64.tar.gz.sha256) / $(cat _dist/helm-${RELEASE}-darwin-amd64.tar.gz.sha256)) -- [Linux amd64](https://get.helm.sh/helm-${RELEASE}-linux-amd64.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-amd64.tar.gz.sha256) / $(cat _dist/helm-${RELEASE}-linux-amd64.tar.gz.sha256)) -- [Linux arm](https://get.helm.sh/helm-${RELEASE}-linux-arm.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-arm.tar.gz.sha256) / $(cat _dist/helm-${RELEASE}-linux-arm.tar.gz.sha256)) -- [Linux arm64](https://get.helm.sh/helm-${RELEASE}-linux-arm64.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-arm64.tar.gz.sha256) / $(cat _dist/helm-${RELEASE}-linux-arm64.tar.gz.sha256)) -- [Linux i386](https://get.helm.sh/helm-${RELEASE}-linux-386.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-386.tar.gz.sha256) / $(cat _dist/helm-${RELEASE}-linux-386.tar.gz.sha256)) -- [Linux ppc64le](https://get.helm.sh/helm-${RELEASE}-linux-ppc64le.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-ppc64le.tar.gz.sha256) / $(cat _dist/helm-${RELEASE}-linux-ppc64le.tar.gz.sha256)) -- [Linux s390x](https://get.helm.sh/helm-${RELEASE}-linux-s390x.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-s390x.tar.gz.sha256) / $(cat _dist/helm-${RELEASE}-darwin-amd64.tar.gz.sha256)) -- [Windows amd64](https://get.helm.sh/helm-${RELEASE}-windows-amd64.zip) ([checksum](https://get.helm.sh/helm-${RELEASE}-windows-amd64.zip.sha256) / $(cat _dist/helm-${RELEASE}-windows-amd64.zip.sha256)) +- [MacOS amd64](https://get.helm.sh/helm-${RELEASE}-darwin-amd64.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-darwin-amd64.tar.gz.sha256sum) / $(cat _dist/helm-${RELEASE}-darwin-amd64.tar.gz.sha256)) +- [Linux amd64](https://get.helm.sh/helm-${RELEASE}-linux-amd64.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-amd64.tar.gz.sha256sum) / $(cat _dist/helm-${RELEASE}-linux-amd64.tar.gz.sha256)) +- [Linux arm](https://get.helm.sh/helm-${RELEASE}-linux-arm.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-arm.tar.gz.sha256sum) / $(cat _dist/helm-${RELEASE}-linux-arm.tar.gz.sha256)) +- [Linux arm64](https://get.helm.sh/helm-${RELEASE}-linux-arm64.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-arm64.tar.gz.sha256sum) / $(cat _dist/helm-${RELEASE}-linux-arm64.tar.gz.sha256)) +- [Linux i386](https://get.helm.sh/helm-${RELEASE}-linux-386.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-386.tar.gz.sha256sum) / $(cat _dist/helm-${RELEASE}-linux-386.tar.gz.sha256)) +- [Linux ppc64le](https://get.helm.sh/helm-${RELEASE}-linux-ppc64le.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-ppc64le.tar.gz.sha256sum) / $(cat _dist/helm-${RELEASE}-linux-ppc64le.tar.gz.sha256)) +- [Linux s390x](https://get.helm.sh/helm-${RELEASE}-linux-s390x.tar.gz) ([checksum](https://get.helm.sh/helm-${RELEASE}-linux-s390x.tar.gz.sha256sum) / $(cat _dist/helm-${RELEASE}-darwin-amd64.tar.gz.sha256)) +- [Windows amd64](https://get.helm.sh/helm-${RELEASE}-windows-amd64.zip) ([checksum](https://get.helm.sh/helm-${RELEASE}-windows-amd64.zip.sha256sum) / $(cat _dist/helm-${RELEASE}-windows-amd64.zip.sha256)) The [Quickstart Guide](https://docs.helm.sh/using_helm/#quickstart-guide) will get you going from there. For **upgrade instructions** or detailed installation notes, check the [install guide](https://docs.helm.sh/using_helm/#installing-helm). You can also use a [script to install](https://raw.githubusercontent.com/helm/helm/master/scripts/get-helm-3) on any system with \`bash\`. From 06bc18c624c3ce264c926632ce8bc9fe471ce6e6 Mon Sep 17 00:00:00 2001 From: tiendc Date: Sat, 14 Mar 2020 01:35:39 +0700 Subject: [PATCH 18/22] Fix a bug in storage/driver/secrets.go Delete() (#7348) * Fix a bug in storage/driver/secrets.go --- pkg/storage/driver/secrets.go | 6 +----- pkg/storage/driver/secrets_test.go | 6 ++++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/storage/driver/secrets.go b/pkg/storage/driver/secrets.go index dcb2ecfcf..1f1931aed 100644 --- a/pkg/storage/driver/secrets.go +++ b/pkg/storage/driver/secrets.go @@ -185,11 +185,7 @@ func (secrets *Secrets) Update(key string, rls *rspb.Release) error { func (secrets *Secrets) Delete(key string) (rls *rspb.Release, err error) { // fetch the release to check existence if rls, err = secrets.Get(key); err != nil { - if apierrors.IsNotFound(err) { - return nil, ErrReleaseExists - } - - return nil, errors.Wrapf(err, "delete: failed to get release %q", key) + return nil, err } // delete the release err = secrets.impl.Delete(key, &metav1.DeleteOptions{}) diff --git a/pkg/storage/driver/secrets_test.go b/pkg/storage/driver/secrets_test.go index e4420704d..5f0ecc8bb 100644 --- a/pkg/storage/driver/secrets_test.go +++ b/pkg/storage/driver/secrets_test.go @@ -194,6 +194,12 @@ func TestSecretDelete(t *testing.T) { secrets := newTestFixtureSecrets(t, []*rspb.Release{rel}...) + // perform the delete on a non-existing release + _, err := secrets.Delete("nonexistent") + if err != ErrReleaseNotFound { + t.Fatalf("Expected ErrReleaseNotFound, got: {%v}", err) + } + // perform the delete rls, err := secrets.Delete(key) if err != nil { From 26830942d275b3a70edfdc32474230f3499a18e4 Mon Sep 17 00:00:00 2001 From: tiendc Date: Sat, 14 Mar 2020 01:36:14 +0700 Subject: [PATCH 19/22] Fix a bug in Delete() in storage/driver/cfgmaps.go (#7367) --- pkg/storage/driver/cfgmaps.go | 5 ----- pkg/storage/driver/cfgmaps_test.go | 6 ++++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/storage/driver/cfgmaps.go b/pkg/storage/driver/cfgmaps.go index cc2e2416a..f9d4da8c3 100644 --- a/pkg/storage/driver/cfgmaps.go +++ b/pkg/storage/driver/cfgmaps.go @@ -201,11 +201,6 @@ func (cfgmaps *ConfigMaps) Update(key string, rls *rspb.Release) error { func (cfgmaps *ConfigMaps) Delete(key string) (rls *rspb.Release, err error) { // fetch the release to check existence if rls, err = cfgmaps.Get(key); err != nil { - if apierrors.IsNotFound(err) { - return nil, ErrReleaseExists - } - - cfgmaps.Log("delete: failed to get release %q: %s", key, err) return nil, err } // delete the release diff --git a/pkg/storage/driver/cfgmaps_test.go b/pkg/storage/driver/cfgmaps_test.go index a36cee1be..e40247d3c 100644 --- a/pkg/storage/driver/cfgmaps_test.go +++ b/pkg/storage/driver/cfgmaps_test.go @@ -194,6 +194,12 @@ func TestConfigMapDelete(t *testing.T) { cfgmaps := newTestFixtureCfgMaps(t, []*rspb.Release{rel}...) + // perform the delete on a non-existent release + _, err := cfgmaps.Delete("nonexistent") + if err != ErrReleaseNotFound { + t.Fatalf("Expected ErrReleaseNotFound: got {%v}", err) + } + // perform the delete rls, err := cfgmaps.Delete(key) if err != nil { From 9d20e44ad15c0a3e69b7ee2fbf3e50968c00befb Mon Sep 17 00:00:00 2001 From: Kim Bao Long Date: Thu, 27 Feb 2020 14:49:41 +0700 Subject: [PATCH 20/22] Add unit test for lint/values.go Signed-off-by: Kim Bao Long --- pkg/lint/rules/values_test.go | 37 +++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 pkg/lint/rules/values_test.go diff --git a/pkg/lint/rules/values_test.go b/pkg/lint/rules/values_test.go new file mode 100644 index 000000000..901a739fd --- /dev/null +++ b/pkg/lint/rules/values_test.go @@ -0,0 +1,37 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "os" + "path/filepath" + "testing" +) + +var ( + nonExistingValuesFilePath = filepath.Join("/fake/dir", "values.yaml") +) + +func TestValidateValuesYamlNotDirectory(t *testing.T) { + _ = os.Mkdir(nonExistingValuesFilePath, os.ModePerm) + defer os.Remove(nonExistingValuesFilePath) + + err := validateValuesFileExistence(nonExistingValuesFilePath) + if err == nil { + t.Errorf("validateValuesFileExistence to return a linter error, got no error") + } +} From 9a0e7d8a317947afcaa19f1e6fcf8df3df31fa77 Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Wed, 18 Mar 2020 12:22:06 +0000 Subject: [PATCH 21/22] Improve error message to check in unit test Signed-off-by: Martin Hickey --- pkg/lint/lint_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index 2a982d088..b51939d76 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -93,7 +93,7 @@ func TestBadValues(t *testing.T) { if len(m) < 1 { t.Fatalf("All didn't fail with expected errors, got %#v", m) } - if !strings.Contains(m[0].Err.Error(), "cannot unmarshal") { + if !strings.Contains(m[0].Err.Error(), "unable to parse YAML") { t.Errorf("All didn't have the error for invalid key format: %s", m[0].Err) } } From cd9c9922ecee1fabfec51746d9e4b6f7da4ea3e6 Mon Sep 17 00:00:00 2001 From: Ihor Dvoretskyi Date: Thu, 26 Mar 2020 10:35:46 -0400 Subject: [PATCH 22/22] Snapcraft installation instructions added Helm is available as a snap - https://snapcraft.io/helm; added this to the installation options Signed-off-by: Ihor Dvoretskyi --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 745a60c2b..bb6908fdb 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,7 @@ If you want to use a package manager: - [Chocolatey](https://chocolatey.org/) users can use `choco install kubernetes-helm`. - [Scoop](https://scoop.sh/) users can use `scoop install helm`. - [GoFish](https://gofi.sh/) users can use `gofish install helm`. +- [Snapcraft](https://snapcraft.io/) users can use `snap install helm --classic` To rapidly get Helm up and running, start with the [Quick Start Guide](https://docs.helm.sh/using_helm/#quickstart-guide).