Duplicated chart entries in Helm

@@ -71,7 +71,7 @@ func TestResolveChartRef(t *testing.T) {
 			if tt.fail {
 				continue
 			}
-			t.Errorf("%s: failed with error %s", tt.name, err)
+			t.Errorf("%s: failed with error %q", tt.name, err)
 			continue
 		}
 		if got := u.String(); got != tt.expect {

@@ -228,6 +228,23 @@ type ChartVersion struct {
 	Created time.Time `json:"created,omitempty"`
 	Removed bool      `json:"removed,omitempty"`
 	Digest  string    `json:"digest,omitempty"`
+
+	// ChecksumDeprecated is deprecated in Helm 3, and therefore ignored. Helm 3 replaced
+	// this with Digest. However, with a strict YAML parser enabled, a field must be
+	// present on the struct for backwards compatibility.
+	ChecksumDeprecated string `json:"checksum,omitempty"`
+
+	// EngineDeprecated is deprecated in Helm 3, and therefore ignored. However, with a strict
+	// YAML parser enabled, this field must be present.
+	EngineDeprecated string `json:"engine,omitempty"`
+
+	// TillerVersionDeprecated is deprecated in Helm 3, and therefore ignored. However, with a strict
+	// YAML parser enabled, this field must be present.
+	TillerVersionDeprecated string `json:"tillerVersion,omitempty"`
+
+	// URLDeprecated is deprectaed in Helm 3, superseded by URLs. It is ignored. However,
+	// with a strict YAML parser enabled, this must be present on the struct.
+	URLDeprecated string `json:"url,omitempty"`
 }
 
 // IndexDirectory reads a (flat) directory and generates an index.
@@ -281,7 +298,7 @@ func IndexDirectory(dir, baseURL string) (*IndexFile, error) {
 // This will fail if API Version is not set (ErrNoAPIVersion) or if the unmarshal fails.
 func loadIndex(data []byte) (*IndexFile, error) {
 	i := &IndexFile{}
-	if err := yaml.Unmarshal(data, i); err != nil {
+	if err := yaml.UnmarshalStrict(data, i); err != nil {
 		return i, err
 	}
 	i.SortEntries()

@@ -95,6 +95,35 @@ func TestLoadIndex(t *testing.T) {
 	verifyLocalIndex(t, i)
 }
 
+const indexWithDuplicates = `
+apiVersion: v1
+entries:
+  nginx:
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/nginx-0.2.0.tgz
+      name: nginx
+      description: string
+      version: 0.2.0
+      home: https://github.com/something/else
+      digest: "sha256:1234567890abcdef"
+  nginx:
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/alpine-1.0.0.tgz
+        - http://storage2.googleapis.com/kubernetes-charts/alpine-1.0.0.tgz
+      name: alpine
+      description: string
+      version: 1.0.0
+      home: https://github.com/something
+      digest: "sha256:1234567890abcdef"
+`
+
+// TestLoadIndex_Duplicates is a regression to make sure that we don't non-deterministically allow duplicate packages.
+func TestLoadIndex_Duplicates(t *testing.T) {
+	if _, err := loadIndex([]byte(indexWithDuplicates)); err == nil {
+		t.Errorf("Expected an error when duplicate entries are present")
+	}
+}
+
 func TestLoadIndexFile(t *testing.T) {
 	i, err := LoadIndexFile(testfile)
 	if err != nil {

@@ -17,7 +17,9 @@ package chartutil
 
 import (
 	"errors"
+	"fmt"
 	"log"
+	"regexp"
 	"strings"
 	"time"
 
@@ -219,6 +221,9 @@ func ProcessRequirementsTags(reqs *Requirements, cvals Values) {
 
 }
 
+// Validate alias names against this regexp
+var aliasRegexp = regexp.MustCompile("^[a-zA-Z0-9-_]+$")
+
 func getAliasDependency(charts []*chart.Chart, aliasChart *Dependency) *chart.Chart {
 	var chartFound chart.Chart
 	for _, existingChart := range charts {
@@ -237,6 +242,11 @@ func getAliasDependency(charts []*chart.Chart, aliasChart *Dependency) *chart.Ch
 		chartFound = *existingChart
 		newMetadata := *existingChart.Metadata
 		if aliasChart.Alias != "" {
+			// Make sure Alias is well-formed
+			if !aliasRegexp.MatchString(aliasChart.Alias) {
+				fmt.Printf("Invalid alias in dependency %q. Skipping.", aliasChart.Name)
+				continue
+			}
 			newMetadata.Name = aliasChart.Alias
 		}
 		chartFound.Metadata = &newMetadata
@@ -286,6 +296,9 @@ func doProcessRequirementsEnabled(c *chart.Chart, v *chart.Config, path string)
 			chartDependencies = append(chartDependencies, chartDependency)
 		}
 		if req.Alias != "" {
+			if !aliasRegexp.MatchString(req.Alias) {
+				return fmt.Errorf("illegal alias name in %q", req.Name)
+			}
 			req.Name = req.Alias
 		}
 	}

@@ -370,11 +370,19 @@ func TestGetAliasDependency(t *testing.T) {
 	}
 
 	// Failure case
+	resetName := req.Dependencies[0].Name
 	req.Dependencies[0].Name = "something-else"
 	if aliasChart := getAliasDependency(c.Dependencies, req.Dependencies[0]); aliasChart != nil {
 		t.Fatalf("expected no chart but got %s", aliasChart.Metadata.Name)
 	}
 
+	// Add a bad alias name
+	req.Dependencies[0].Name = resetName
+	req.Dependencies[0].Alias = "$foobar"
+	if aliasChart := getAliasDependency(c.Dependencies, req.Dependencies[0]); aliasChart != nil {
+		t.Fatalf("expected no chart but got %s", aliasChart.Metadata.Name)
+	}
+
 	req.Dependencies[0].Version = "something else which is not in the compatible range"
 	if version.IsCompatibleRange(req.Dependencies[0].Version, aliasChart.Metadata.Version) {
 		t.Fatalf("Dependency chart version which is not in the compatible range should cause a failure other than a success ")
@@ -516,3 +524,16 @@ func TestDependentChartsWithSomeSubchartsSpecifiedInRequirements(t *testing.T) {
 	}
 
 }
+
+func TestAliasRegexp(t *testing.T) {
+	for name, shouldPass := range map[string]bool{
+		"abcdefghijklmnopqrstuvwxyzABCDEFG0987654321_-": true,
+		"$foo":     false,
+		"bar$":     false,
+		"foo\nbar": false,
+	} {
+		if aliasRegexp.MatchString(name) != shouldPass {
+			t.Errorf("name %q failed to pass its test", name)
+		}
+	}
+}

@@ -16,6 +16,7 @@ limitations under the License.
 package plugin // import "k8s.io/helm/pkg/plugin"
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -141,13 +142,22 @@ func LoadAll(basedir string) ([]*Plugin, error) {
 		return plugins, nil
 	}
 
+	loaded := map[string]bool{}
 	for _, yaml := range matches {
 		dir := filepath.Dir(yaml)
 		p, err := LoadDir(dir)
+		pname := p.Metadata.Name
 		if err != nil {
 			return plugins, err
 		}
+
+		if _, ok := loaded[pname]; ok {
+			fmt.Fprintf(os.Stderr, "A plugin named %q already exists. Skipping.", pname)
+			continue
+		}
+
 		plugins = append(plugins, p)
+		loaded[pname] = true
 	}
 	return plugins, nil
 }

@@ -137,6 +137,7 @@ func TestLoadAll(t *testing.T) {
 		t.Fatalf("Could not load %q: %s", basedir, err)
 	}
 
+	// This would fail if the duplicate plugin were loaded.
 	if l := len(plugs); l != 3 {
 		t.Fatalf("expected 3 plugins, found %d", l)
 	}

@@ -0,0 +1,13 @@
+#!/bin/bash
+
+echo "Hello from a Helm plugin"
+
+echo "PARAMS"
+echo $*
+
+echo "ENVIRONMENT"
+echo $TILLER_HOST
+echo $HELM_HOME
+
+$HELM_BIN --host $TILLER_HOST ls --all
+

@@ -0,0 +1,11 @@
+name: "hello"
+version: "0.1.0"
+usage: "usage"
+description: |-
+  description
+command: "$HELM_PLUGIN_SELF/hello.sh"
+useTunnel: true
+ignoreFlags: true
+install: "echo installing..."
+hooks:
+  install: "echo installing..."

@@ -30,6 +30,7 @@ import (
 
 	"github.com/Masterminds/semver"
 	"github.com/ghodss/yaml"
+	yaml2 "gopkg.in/yaml.v2"
 
 	"k8s.io/helm/pkg/chartutil"
 	"k8s.io/helm/pkg/proto/hapi/chart"
@@ -83,6 +84,14 @@ type IndexFile struct {
 	PublicKeys []string                 `json:"publicKeys,omitempty"`
 }
 
+// IndexValidation is used to validate the integrity of an index file
+type IndexValidation struct {
+	APIVersion string                 `yaml:"apiVersion"`
+	Generated  time.Time              `yaml:"generated"`
+	Entries    map[string]interface{} `yaml:"entries"`
+	PublicKeys []string               `yaml:"publicKeys,omitempty"`
+}
+
 // NewIndexFile initializes an index.
 func NewIndexFile() *IndexFile {
 	return &IndexFile{
@@ -283,9 +292,14 @@ func IndexDirectory(dir, baseURL string) (*IndexFile, error) {
 // This will fail if API Version is not set (ErrNoAPIVersion) or if the unmarshal fails.
 func loadIndex(data []byte) (*IndexFile, error) {
 	i := &IndexFile{}
+	if err := validateIndex(data); err != nil {
+		return i, err
+	}
+
 	if err := yaml.Unmarshal(data, i); err != nil {
 		return i, err
 	}
+
 	i.SortEntries()
 	if i.APIVersion == "" {
 		// When we leave Beta, we should remove legacy support and just
@@ -296,6 +310,16 @@ func loadIndex(data []byte) (*IndexFile, error) {
 	return i, nil
 }
 
+// validateIndex validates that the index is well-formed.
+func validateIndex(data []byte) error {
+	// This is done ONLY for validation. We need to use ghodss/yaml for the actual parsing.
+	validation := &IndexValidation{}
+	if err := yaml2.UnmarshalStrict(data, validation); err != nil {
+		return err
+	}
+	return nil
+}
+
 // unversionedEntry represents a deprecated pre-Alpha.5 format.
 //
 // This will be removed prior to v2.0.0

@@ -422,3 +422,19 @@ func TestIndexAdd(t *testing.T) {
 		t.Errorf("Expected http://example.com/charts/deis-0.1.0.tgz, got %s", i.Entries["deis"][0].URLs[0])
 	}
 }
+
+const mockDuplicateIndex = `
+entries:
+  foo: {}
+  bar: {}
+  baz: {}
+  bar: {}
+`
+
+func TestValidateIndex(t *testing.T) {
+	expect := `key "bar" already set in map`
+	err := validateIndex([]byte(mockDuplicateIndex))
+	if strings.Contains(expect, err.Error()) {
+		t.Errorf("Unexpected error: %s", err)
+	}
+}