Adding chartname request validation for install

pkg/api/install.go

@@ -16,7 +16,7 @@ type InstallRequest struct {
 
 type InstallResponse struct {
 	Error  string `json:"error,omitempty"`
-	Status string `json:"status"`
+	Status string `json:"status,omitempty"`
 }
 
 // RODO: we could use interface as well if everything's in same package
@@ -34,9 +34,7 @@ func Install(svc Service) http.Handler {
 		cfg := InstallConfig{ChartName: req.Chart, Name: req.Name, Namespace: req.Namespace}
 		res, err := svc.Install(r.Context(), cfg, req.Values)
 		if err != nil {
-			response.Error = err.Error()
+			respondError(w, "error while installing chart: %v", err)
-			logger.Errorf("[Install] error while installing chart: %v", err)
-			w.WriteHeader(http.StatusInternalServerError)
 			return
 		}
 		response.Status = res.status
@@ -47,3 +45,13 @@ func Install(svc Service) http.Handler {
 		}
 	})
 }
+
+func respondError(w http.ResponseWriter, logprefix string, err error) {
+	response := InstallResponse{Error: err.Error()}
+	w.WriteHeader(http.StatusInternalServerError)
+	if err := json.NewEncoder(w).Encode(&response); err != nil {
+		logger.Errorf("[Install] %s %v", logprefix, err)
+		w.WriteHeader(http.StatusInternalServerError)
+		return
+	}
+}

pkg/api/install_api_test.go

@@ -1,6 +1,7 @@
 package api_test
 
 import (
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -62,7 +63,7 @@ func (s *InstallerTestSuite) TestShouldReturnDeployedStatusOnSuccessfulInstall()
 
 	resp, err := http.DefaultClient.Do(req)
 
-	assert.Equal(s.T(), 200, resp.StatusCode)
+	assert.Equal(s.T(), http.StatusOK, resp.StatusCode)
 	expectedResponse := `{"status":"deployed"}` + "\n"
 	respBody, _ := ioutil.ReadAll(resp.Body)
 	assert.Equal(s.T(), expectedResponse, string(respBody))
@@ -71,6 +72,26 @@ func (s *InstallerTestSuite) TestShouldReturnDeployedStatusOnSuccessfulInstall()
 	s.mockChartLoader.AssertExpectations(s.T())
 }
 
+func (s *InstallerTestSuite) TestShouldReturnInternalServerErrorOnFailure() {
+	chartName := "stable/redis-ha"
+	body := fmt.Sprintf(`{
+    "chart":"%s",
+    "name": "redis-v5",
+    "namespace": "something"}`, chartName)
+	req, _ := http.NewRequest("POST", fmt.Sprintf("%s/install", s.server.URL), strings.NewReader(body))
+	s.mockChartLoader.On("LocateChart", chartName, s.appConfig).Return("./testdata/albatross", errors.New("Invalid chart"))
+
+	resp, err := http.DefaultClient.Do(req)
+
+	assert.Equal(s.T(), http.StatusInternalServerError, resp.StatusCode)
+	expectedResponse := `{"error":"error in locating chart: Invalid chart"}` + "\n"
+	respBody, _ := ioutil.ReadAll(resp.Body)
+	assert.Equal(s.T(), expectedResponse, string(respBody))
+	require.NoError(s.T(), err)
+	s.mockInstaller.AssertExpectations(s.T())
+	s.mockChartLoader.AssertExpectations(s.T())
+}
+
 func (s *InstallerTestSuite) TearDownTest() {
 	s.server.Close()
 }

pkg/api/service.go

@@ -2,19 +2,20 @@ package api
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"strings"
 
 	"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/release"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 )
 
 type installer interface {
 	SetConfig(InstallConfig)
-	Run(*chart.Chart, map[string]interface{}) (*release.Release, error)
+	runner
 }
 
 type chartloader interface {
@@ -35,7 +36,7 @@ type InstallConfig struct {
 
 type chartValues map[string]interface{}
 
-type installResult struct {
+type InstallResult struct {
 	status string
 }
 
@@ -47,7 +48,10 @@ func (s Service) getValues(vals chartValues) (chartValues, error) {
 	return vals, nil
 }
 
-func (s Service) Install(ctx context.Context, cfg InstallConfig, values chartValues) (*installResult, error) {
+func (s Service) Install(ctx context.Context, cfg InstallConfig, values chartValues) (*InstallResult, error) {
+	if err := s.validate(cfg, values); err != nil {
+		return nil, fmt.Errorf("error request validation: %v", err)
+	}
 	chart, err := s.loadChart(cfg.ChartName)
 	if err != nil {
 		return nil, err
@@ -72,19 +76,27 @@ func (s Service) loadChart(chartName string) (*chart.Chart, error) {
 	return requestedChart, nil
 }
 
-func (s Service) installChart(icfg InstallConfig, ch *chart.Chart, vals chartValues) (*installResult, error) {
+func (s Service) installChart(icfg InstallConfig, ch *chart.Chart, vals chartValues) (*InstallResult, error) {
 	s.installer.SetConfig(icfg)
 	release, err := s.installer.Run(ch, vals)
 	if err != nil {
 		return nil, fmt.Errorf("error in installing chart: %v", err)
 	}
-	result := new(installResult)
+	result := new(InstallResult)
 	if release.Info != nil {
 		result.status = release.Info.Status.String()
 	}
 	return result, nil
 }
 
+func (s Service) validate(icfg InstallConfig, values chartValues) error {
+	if strings.HasPrefix(icfg.ChartName, ".") ||
+		strings.HasPrefix(icfg.ChartName, "/") {
+		return errors.New("cannot refer local chart")
+	}
+	return nil
+}
+
 func NewService(settings *cli.EnvSettings, cl chartloader, i installer) Service {
 	return Service{
 		settings:    settings,

pkg/api/service_test.go

@@ -53,6 +53,25 @@ func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnInvalidChart() {
 	s.installer.AssertNotCalled(t, "Run")
 }
 
+func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnLocalChartReference() {
+	chartName := "./some/local-chart"
+	cfg := InstallConfig{
+		Name:      "some-component",
+		Namespace: "hermes",
+		ChartName: chartName,
+	}
+	var vals chartValues
+
+	res, err := s.svc.Install(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.installer.AssertNotCalled(t, "SetConfig")
+	s.installer.AssertNotCalled(t, "Run")
+}
+
 func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnFailedIntallRun() {
 	chartName := "stable/valid-chart"
 	cfg := InstallConfig{

pkg/api/upgrade.go

@@ -88,7 +88,7 @@ func (h UpgradeHandler) UpgradeRelease(releaseName, releaseNamespace, chartPath
 		if _, err := history.Run(releaseName); err == driver.ErrReleaseNotFound {
 			fmt.Printf("Release %q does not exist. Installing it now.\n", releaseName)
 
-			//TODO: yet to accomodate namespace and releasename, just refactoring
+			//TODO: yet to accommodate namespace and releasename, just refactoring
 			icfg := InstallConfig{
 				Namespace: releaseNamespace,
 				Name:      releaseName,

pkg/cli/values/options.go

@@ -17,7 +17,6 @@ limitations under the License.
 package values
 
 import (
-	"fmt"
 	"io/ioutil"
 	"net/url"
 	"os"
@@ -40,7 +39,6 @@ type Options struct {
 // MergeValues merges values from files specified via -f/--values and directly
 // via --set, --set-string, or --set-file, marshaling them to YAML
 func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, error) {
-	fmt.Println("%+v", opts)
 	base := map[string]interface{}{}
 
 	// User specified a values files via -f/--values