From 8d408876a059223c9a29d99efb35b3497a53504c Mon Sep 17 00:00:00 2001
From: Matthew Fisher <matt.fisher@microsoft.com>
Date: Mon, 10 Sep 2018 11:14:03 -0700
Subject: [PATCH] Fix race condition in `helm list` (#4620)

* Fix race in helm list when partitioning

Problem:
The chunks slice that is passed through the channel is reused for each
partition. This means that encoding the release into a message is racing with
populating the next partition, causing the results to sometimes not fit in the
message, and the release list to be incorrect

Solution:
Allocate a new slice for each partition

Issue #3322

Signed-off-by: Brian Marshall <bmarshall13@users.noreply.github.com>
(cherry picked from commit a0858e29d877b8aa19478133865e45958ff21a9e)

* fix import sorting

Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>

* ref(release_server_test): use NewReleaseServer()

Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>

* add unit test for race condition in `helm list`

Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
(cherry picked from commit 5b236324468ad8958c351dd3c734880c0fa5d561)
---
 pkg/tiller/release_list.go        |  5 +++--
 pkg/tiller/release_list_test.go   | 23 +++++++++++++++++++++++
 pkg/tiller/release_server_test.go | 16 ++++------------
 3 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/pkg/tiller/release_list.go b/pkg/tiller/release_list.go
index 89f7a1100..3299d3ef2 100644
--- a/pkg/tiller/release_list.go
+++ b/pkg/tiller/release_list.go
@@ -18,11 +18,12 @@ package tiller
 
 import (
 	"fmt"
+	"regexp"
+
 	"github.com/golang/protobuf/proto"
 	"k8s.io/helm/pkg/proto/hapi/release"
 	"k8s.io/helm/pkg/proto/hapi/services"
 	relutil "k8s.io/helm/pkg/releaseutil"
-	"regexp"
 )
 
 // ListReleases lists the releases found by the server.
@@ -140,7 +141,7 @@ func (s *ReleaseServer) partition(rels []*release.Release, cap int) <-chan []*re
 				s.Log("partitioned at %d with %d releases (cap=%d)", fill, len(chunk), cap)
 				chunks <- chunk
 				// reset paritioning state
-				chunk = chunk[:0]
+				chunk = nil
 				fill = 0
 			}
 			chunk = append(chunk, rls)
diff --git a/pkg/tiller/release_list_test.go b/pkg/tiller/release_list_test.go
index b9ab6fe55..f2d19cf96 100644
--- a/pkg/tiller/release_list_test.go
+++ b/pkg/tiller/release_list_test.go
@@ -274,3 +274,26 @@ func TestReleasesNamespace(t *testing.T) {
 		t.Errorf("Expected 2 releases, got %d", len(mrs.val.Releases))
 	}
 }
+
+func TestReleasePartition(t *testing.T) {
+	var rl []*release.Release
+	rs := rsFixture()
+	rs.Log = t.Logf
+	num := 7
+	for i := 0; i < num; i++ {
+		rel := releaseStub()
+		rel.Name = fmt.Sprintf("rel-%d", i)
+		rl = append(rl, rel)
+	}
+	visited := map[string]bool{}
+
+	chunks := rs.partition(rl, 0)
+	for chunk := range chunks {
+		for _, rel := range chunk {
+			if visited[rel.Name] {
+				t.Errorf("%s was already visited", rel.Name)
+			}
+			visited[rel.Name] = true
+		}
+	}
+}
diff --git a/pkg/tiller/release_server_test.go b/pkg/tiller/release_server_test.go
index 122ab9dd4..f3fca7390 100644
--- a/pkg/tiller/release_server_test.go
+++ b/pkg/tiller/release_server_test.go
@@ -112,24 +112,16 @@ data:
   name: value
 `
 
-func rsFixture() *ReleaseServer {
-	clientset := fake.NewSimpleClientset()
-	return &ReleaseServer{
-		ReleaseModule: &LocalReleaseModule{
-			clientset: clientset,
-		},
-		env:       MockEnvironment(),
-		clientset: clientset,
-		Log:       func(_ string, _ ...interface{}) {},
-	}
-}
-
 type chartOptions struct {
 	*chart.Chart
 }
 
 type chartOption func(*chartOptions)
 
+func rsFixture() *ReleaseServer {
+	return NewReleaseServer(MockEnvironment(), fake.NewSimpleClientset(), false)
+}
+
 func buildChart(opts ...chartOption) *chart.Chart {
 	c := &chartOptions{
 		Chart: &chart.Chart{