Add additional tests and fix golangci-lint

cmd/service/service.go

@@ -22,21 +22,14 @@ func startServer() {
 	//TODO: use gorilla mux and add middleware to write content type and other headers
 	app := servercontext.App()
 	logger.Setup("debug")
-<<<<<<< HEAD
+
+	actionList := action.NewList(app.ActionConfig)
 	actionInstall := action.NewInstall(app.ActionConfig)
 	actionUpgrade := action.NewUpgrade(app.ActionConfig)
 	actionHistory := action.NewHistory(app.ActionConfig)
 
-	service := api.NewService(app.Config, new(action.ChartPathOptions), api.NewInstaller(actionInstall), api.NewUpgrader(actionUpgrade), api.NewHistory(actionHistory))
+	service := api.NewService(app.Config, new(action.ChartPathOptions), api.NewList(actionList), api.NewInstall(actionInstall), api.NewUpgrader(actionUpgrade), api.NewHistory(actionHistory))
-
-=======
-
-	service := api.NewService(app.Config,
-		new(action.ChartPathOptions),
-		api.NewInstall(action.NewInstall(app.ActionConfig)),
-		api.NewList(action.NewList(app.ActionConfig)))
 
->>>>>>> helm-service-refactor
 	router.Handle("/ping", ping.Handler())
 	router.Handle("/list", api.List(service))
 	router.Handle("/install", api.Install(service))

pkg/api/install_api_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"gotest.tools/assert"
+
 	"helm.sh/helm/v3/pkg/api"
 	"helm.sh/helm/v3/pkg/api/logger"
 	"helm.sh/helm/v3/pkg/cli"
@@ -23,9 +24,9 @@ type InstallerTestSuite struct {
 	suite.Suite
 	recorder        *httptest.ResponseRecorder
 	server          *httptest.Server
-	mockInstall	    *mockInstall
+	mockInstall     *mockInstall
 	mockChartLoader *mockChartLoader
-	mockList		*mockList
+	mockList        *mockList
 	appConfig       *cli.EnvSettings
 }
 
@@ -42,7 +43,7 @@ func (s *InstallerTestSuite) SetupTest() {
 		RepositoryConfig: "./testdata/helm",
 		PluginsDirectory: "./testdata/helm/plugin",
 	}
-	service := api.NewService(s.appConfig, s.mockChartLoader,nil, s.mockInstall, nil, nil)
+	service := api.NewService(s.appConfig, s.mockChartLoader, nil, s.mockInstall, nil, nil)
 	handler := api.Install(service)
 	s.server = httptest.NewServer(handler)
 }

pkg/api/list.go

@@ -2,12 +2,13 @@ package api
 
 import (
 	"encoding/json"
-	"helm.sh/helm/v3/pkg/api/logger"
 	"net/http"
+
+	"helm.sh/helm/v3/pkg/api/logger"
 )
 
 type ListRequest struct {
-	NameSpace string `json:"namespace"`
+	NameSpace     string `json:"namespace"`
 	ReleaseStatus string `json:"release_status"`
 }
 
@@ -63,4 +64,4 @@ func List(svc Service) http.Handler {
 		w.WriteHeader(http.StatusOK)
 		w.Write(payload)
 	})
-}
+}

pkg/api/list_api_test.go

@@ -2,30 +2,30 @@ package api_test
 
 import (
 	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"gotest.tools/assert"
+
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/api"
 	"helm.sh/helm/v3/pkg/api/logger"
 	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/release"
-	"io/ioutil"
-	"net/http"
-	"net/http/httptest"
-	"strings"
-	"testing"
 )
 
 type ListTestSuite struct {
 	suite.Suite
-	recorder        *httptest.ResponseRecorder
+	recorder  *httptest.ResponseRecorder
-	server          *httptest.Server
+	server    *httptest.Server
-	mockInstall	    *mockInstall
+	mockList  *mockList
-	mockChartLoader *mockChartLoader
+	appConfig *cli.EnvSettings
-	mockList		*mockList
-	appConfig       *cli.EnvSettings
 }
 
 type mockList struct{ mock.Mock }
@@ -67,7 +67,7 @@ func (s *ListTestSuite) TestShouldReturnReleasesWhenSuccessfulAPICall() {
 	releases = append(releases,
 		&release.Release{Name: "test-release",
 			Namespace: "test-namespace",
-			Info: &release.Info{Status: release.StatusDeployed}})
+			Info:      &release.Info{Status: release.StatusDeployed}})
 
 	s.mockList.On("SetStateMask")
 	s.mockList.On("SetState", action.ListDeployed)
@@ -89,7 +89,6 @@ func (s *ListTestSuite) TestShouldReturnBadRequestErrorIfItHasInvalidCharacter()
 	body := `{"request_id":"test-request-id""""}`
 	req, _ := http.NewRequest("POST", fmt.Sprintf("%s/list", s.server.URL), strings.NewReader(body))
 
-
 	resp, err := http.DefaultClient.Do(req)
 
 	assert.Equal(s.T(), 400, resp.StatusCode)
@@ -106,4 +105,4 @@ func (s *ListTestSuite) TearDownTest() {
 
 func TestListAPI(t *testing.T) {
 	suite.Run(t, new(ListTestSuite))
-}
+}

pkg/api/lister.go

@@ -15,10 +15,10 @@ type lister interface {
 	SetState(state action.ListStates)
 }
 
-func NewList(action *action.List) *list{
+func NewList(action *action.List) *list {
 	return &list{action}
 }
 
-func (l *list) SetState(state action.ListStates){
+func (l *list) SetState(state action.ListStates) {
 	l.StateMask = state
 }

pkg/api/ping/pingcontract.go

@@ -1,10 +1,10 @@
 package ping
 
-type PingReq struct {
+type Req struct {
 	RequestID string
 }
 
-type PingResponse struct {
+type Response struct {
 	Status bool
 	Data   string
 }

pkg/api/ping/pinghandler.go

@@ -12,7 +12,7 @@ func Handler() http.Handler {
 		res.Header().Set("Content-Type", "application/json")
 		defer req.Body.Close()
 
-		var request PingReq
+		var request Req
 		decoder := json.NewDecoder(req.Body)
 		decoder.UseNumber()
 
@@ -23,7 +23,7 @@ func Handler() http.Handler {
 
 		request.RequestID = req.Header.Get("Request-Id")
 
-		response := PingResponse{Status: true, Data: "pong"}
+		response := Response{Status: true, Data: "pong"}
 		payload, err := json.Marshal(response)
 		if err != nil {
 			fmt.Println("error parsing response")

pkg/api/service.go

@@ -9,16 +9,13 @@ import (
 
 	"helm.sh/helm/v3/pkg/action"
 
-
 	"helm.sh/helm/v3/pkg/api/logger"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chart/loader"
 	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/storage/driver"
-	_ "k8s.io/client-go/plugin/pkg/client/auth"
 )
 
-
 type upgrader interface {
 	SetConfig(ReleaseConfig)
 	GetInstall() bool
@@ -58,7 +55,6 @@ func (s Service) getValues(vals ChartValues) (ChartValues, error) {
 	return vals, nil
 }
 
-
 func (s Service) Install(ctx context.Context, cfg ReleaseConfig, values ChartValues) (*ReleaseResult, error) {
 	if err := s.validate(cfg, values); err != nil {
 		return nil, fmt.Errorf("error request validation: %v", err)
@@ -110,7 +106,6 @@ func (s Service) loadChart(chartName string) (*chart.Chart, error) {
 	return requestedChart, nil
 }
 
-
 func (s Service) installChart(icfg ReleaseConfig, ch *chart.Chart, vals ChartValues) (*ReleaseResult, error) {
 	s.Installer.SetConfig(icfg)
 	release, err := s.Installer.Run(ch, vals)
@@ -124,7 +119,6 @@ func (s Service) installChart(icfg ReleaseConfig, ch *chart.Chart, vals ChartVal
 	return result, nil
 }
 
-
 func (s Service) upgradeRelease(ucfg ReleaseConfig, ch *chart.Chart, vals ChartValues) (*ReleaseResult, error) {
 	s.upgrader.SetConfig(ucfg)
 	release, err := s.upgrader.Run(ucfg.Name, ch, vals)
@@ -175,5 +169,5 @@ func (s Service) List(releaseStatus string) ([]Release, error) {
 }
 
 func NewService(settings *cli.EnvSettings, cl chartloader, l lister, i Installer, u upgrader, h history) Service {
-	return Service{settings,cl, l,i, u, h}
+	return Service{settings, cl, l, i, u, h}
 }

pkg/api/service_test.go

@@ -3,14 +3,16 @@ package api_test
 import (
 	"context"
 	"errors"
+	"testing"
+
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/api"
-	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
+
 	"helm.sh/helm/v3/pkg/api/logger"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/cli"
@@ -25,7 +27,7 @@ type ServiceTestSuite struct {
 	history     *mockHistory
 	installer   *mockInstall
 	chartloader *mockChartLoader
-	lister		*mockList
+	lister      *mockList
 	svc         api.Service
 	settings    *cli.EnvSettings
 }
@@ -147,7 +149,10 @@ func (s *ServiceTestSuite) TestUpgradeInstallTrueShouldInstallChart() {
 	assert.NoError(t, err)
 	require.NotNil(t, res)
 	assert.Equal(t, res.Status, "deployed")
+	s.upgrader.AssertNotCalled(t, "Run")
 	s.chartloader.AssertExpectations(t)
+	s.upgrader.AssertExpectations(t)
+	s.history.AssertExpectations(t)
 	s.installer.AssertExpectations(t)
 }
 
@@ -222,6 +227,44 @@ func (s *ServiceTestSuite) TestUpgradeShouldReturnResultOnSuccess() {
 	s.upgrader.AssertExpectations(t)
 }
 
+func (s *ServiceTestSuite) TestUpgradeValidateFailShouldResultFailure() {
+	chartName := "./some/local-chart"
+	cfg := api.ReleaseConfig{
+		Name:      "some-component",
+		Namespace: "hermes",
+		ChartName: chartName,
+	}
+	var vals api.ChartValues
+
+	res, err := s.svc.Upgrade(s.ctx, cfg, vals)
+
+	t := s.T()
+	assert.Nil(t, res)
+	assert.EqualError(t, err, "error request validation: cannot refer local chart")
+	s.chartloader.AssertNotCalled(t, "LocateChart")
+	s.upgrader.AssertNotCalled(t, "SetConfig")
+	s.upgrader.AssertNotCalled(t, "Run")
+}
+
+func (s *ServiceTestSuite) TestUpgradeShouldReturnErrorOnInvalidChart() {
+	chartName := "stable/invalid-chart"
+	cfg := api.ReleaseConfig{
+		Name:      "some-component",
+		Namespace: "hermes",
+		ChartName: chartName,
+	}
+	var vals api.ChartValues
+	s.chartloader.On("LocateChart", chartName, s.settings).Return("", errors.New("Unable to find chart"))
+
+	res, err := s.svc.Upgrade(s.ctx, cfg, vals)
+
+	t := s.T()
+	assert.Nil(t, res)
+	assert.EqualError(t, err, "error in locating chart: Unable to find chart")
+	s.chartloader.AssertExpectations(t)
+	s.upgrader.AssertNotCalled(t, "SetConfig")
+	s.upgrader.AssertNotCalled(t, "Run")
+}
 
 func (s *ServiceTestSuite) TestListShouldReturnErrorOnFailureOfListRun() {
 	s.lister.On("SetState", action.ListDeployed)
@@ -249,7 +292,7 @@ func (s *ServiceTestSuite) TestListShouldReturnAllReleasesIfNoFilterIsPassed() {
 	releases = append(releases,
 		&release.Release{Name: "test-release",
 			Namespace: "test-namespace",
-			Info: &release.Info{Status: release.StatusDeployed}})
+			Info:      &release.Info{Status: release.StatusDeployed}})
 
 	s.lister.On("Run").Return(releases, nil)
 
@@ -293,7 +336,6 @@ func (s *ServiceTestSuite) TestListShouldReturnDeployedReleasesIfDeployedIsPasse
 	s.lister.AssertExpectations(t)
 }
 
-
 type mockInstall struct{ mock.Mock }
 
 func (m *mockInstall) SetConfig(cfg api.ReleaseConfig) {
@@ -305,7 +347,6 @@ func (m *mockInstall) Run(c *chart.Chart, vals map[string]interface{}) (*release
 	return args.Get(0).(*release.Release), args.Error(1)
 }
 
-
 type mockChartLoader struct{ mock.Mock }
 
 func (m *mockChartLoader) LocateChart(name string, settings *cli.EnvSettings) (string, error) {
@@ -313,7 +354,6 @@ func (m *mockChartLoader) LocateChart(name string, settings *cli.EnvSettings) (s
 	return args.String(0), args.Error(1)
 }
 
-
 type mockUpgrader struct{ mock.Mock }
 
 func (m *mockUpgrader) Run(name string, chart *chart.Chart, vals map[string]interface{}) (*release.Release, error) {
@@ -330,7 +370,6 @@ func (m *mockUpgrader) GetInstall() bool {
 	return args.Get(0).(bool)
 }
 
-
 type mockHistory struct{ mock.Mock }
 
 func (m *mockHistory) Run(name string) ([]*release.Release, error) {

pkg/api/upgrade.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"encoding/json"
+	"io"
 	"net/http"
 
 	"helm.sh/helm/v3/pkg/api/logger"
@@ -25,8 +26,9 @@ func Upgrade(svc Service) http.Handler {
 		defer r.Body.Close()
 
 		var req UpgradeRequest
-		if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+
-			logger.Errorf("[Upgrade] error decoding request: %v", err)
+		if err := json.NewDecoder(r.Body).Decode(&req); err == io.EOF || err != nil {
+			logger.Errorf("[Upgrade] error decoding request: %v", err.Error())
 			w.WriteHeader(http.StatusBadRequest)
 			return
 		}

pkg/api/upgrade_api_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"gotest.tools/assert"
+
 	"helm.sh/helm/v3/pkg/api"
 	"helm.sh/helm/v3/pkg/api/logger"
 	"helm.sh/helm/v3/pkg/cli"