Repository: helm/helm
Pull request #6611 (2019-10-08 20:00:54)

fix(helm): Port accept dependency in requirements.yaml from charts directory
This is a port of #6578 from Helm v2 with modifications to integrate with v3 infrastructure.

**What this PR does / why we need it**:

**Special notes for your reviewer**:
For manual testing, use steps similar to the following:
- Create chart `foo`
- Create chart `sub-foo`
- Move `sub-foo` to `foo/charts`
- Add the following dependencies to `foo/Chart.yaml`:
```console
dependencies:
- name: sub-foo
  version: 0.1.0
- name: mysql
  version: "1.3.2"
  repository: "https://kubernetes-charts.storage.googleapis.com"
```
- Do a dependency update: `helm dep up foo`

It should fail similar to the following without the PR:
```console
$ helm dep up foo
Error: no repository definition for . Please add them via 'helm repo add'
Note that repositories must be URLs or aliases. For example, to refer to the "example"
repository, use "https://charts.example.com/" or "@example" instead of
"example". Don't forget to add the repo, too ('helm repo add').
```

PR should fix it similar to the following:
```console
$ helm dep update foo
Hang tight while we grab the latest from your chart repositories...
..Successfully got an update from the "stable" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 2 charts
Dependency sub-foo did not declare a repository. Assuming it exists in the charts directory
Downloading mysql from repo https://kubernetes-charts.storage.googleapis.com
Deleting outdated charts

$ ls -lrt foo/charts/
total 20
drwxr-xr-x 4 usr1 usr1  4096 Oct  8 20:02 sub-foo
-rw-rw-r-- 1 usr1 usr1  2739 Oct  8 20:05 sub-foo-0.1.0.tgz
-rw-r--r-- 1 usr1 usr1 10635 Oct  8 20:37 mysql-1.3.2.tgz
```
**If applicable**:
- [ ] this PR contains documentation
- [x] this PR contains unit tests
- [x] this PR has been tested for backwards compatibility


hickeyma: Thanks for review @thomastaylor312. Ready again for review.

Patch:
diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go
index c902b0134..79238b22f 100644
--- a/internal/resolver/resolver.go
+++ b/internal/resolver/resolver.go
@@ -53,6 +53,19 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 	locked := make([]*chart.Dependency, len(reqs))
 	missing := []string{}
 	for i, d := range reqs {
+		if d.Repository == "" {
+			// Local chart subfolder
+			if _, err := GetLocalPath(filepath.Join("charts", d.Name), r.chartpath); err != nil {
+				return nil, err
+			}
+
+			locked[i] = &chart.Dependency{
+				Name:       d.Name,
+				Repository: "",
+				Version:    d.Version,
+			}
+			continue
+		}
 		if strings.HasPrefix(d.Repository, "file://") {
 
 			if _, err := GetLocalPath(d.Repository, r.chartpath); err != nil {
diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go
index 4b49a7911..f1dea342f 100644
--- a/pkg/downloader/manager.go
+++ b/pkg/downloader/manager.go
@@ -203,6 +203,31 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 	fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps))
 	var saveError error
 	for _, dep := range deps {
+		// No repository means the chart is in charts directory
+		if dep.Repository == "" {
+			fmt.Fprintf(m.Out, "Dependency %s did not declare a repository. Assuming it exists in the charts directory\n", dep.Name)
+			chartPath := filepath.Join(tmpPath, dep.Name)
+			ch, err := loader.LoadDir(chartPath)
+			if err != nil {
+				return fmt.Errorf("Unable to load chart: %v", err)
+			}
+
+			constraint, err := semver.NewConstraint(dep.Version)
+			if err != nil {
+				return fmt.Errorf("Dependency %s has an invalid version/constraint format: %s", dep.Name, err)
+			}
+
+			v, err := semver.NewVersion(ch.Metadata.Version)
+			if err != nil {
+				return fmt.Errorf("Invalid version %s for dependency %s: %s", dep.Version, dep.Name, err)
+			}
+
+			if !constraint.Check(v) {
+				saveError = fmt.Errorf("Dependency %s at version %s does not satisfy the constraint %s", dep.Name, ch.Metadata.Version, dep.Version)
+				break
+			}
+			continue
+		}
 		if strings.HasPrefix(dep.Repository, "file://") {
 			if m.Debug {
 				fmt.Fprintf(m.Out, "Archiving %s from repo %s\n", dep.Name, dep.Repository)
@@ -247,8 +272,11 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 	if saveError == nil {
 		fmt.Fprintln(m.Out, "Deleting outdated charts")
 		for _, dep := range deps {
-			if err := m.safeDeleteDep(dep.Name, tmpPath); err != nil {
-				return err
+			// Chart from local charts directory stays in place
+			if dep.Repository != "" {
+				if err := m.safeDeleteDep(dep.Name, tmpPath); err != nil {
+					return err
+				}
 			}
 		}
 		if err := move(tmpPath, destPath); err != nil {
@@ -360,6 +388,10 @@ func (m *Manager) getRepoNames(deps []*chart.Dependency) (map[string]string, err
 	// by Helm.
 	missing := []string{}
 	for _, dd := range deps {
+		// Don't map the repository, we don't need to download chart from charts directory
+		if dd.Repository == "" {
+			continue
+		}
 		// if dep chart is from local path, verify the path is valid
 		if strings.HasPrefix(dd.Repository, "file://") {
 			if _, err := resolver.GetLocalPath(dd.Repository, m.ChartPath); err != nil {


Test patch:
diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go
index 3c402aaea..6bb29ba9a 100644
--- a/internal/resolver/resolver_test.go
+++ b/internal/resolver/resolver_test.go
@@ -85,6 +85,29 @@ func TestResolve(t *testing.T) {
 			},
 			err: true,
 		},
+		{
+			name: "repo from valid path under charts path",
+			req: []*chart.Dependency{
+				{Name: "localdependency", Repository: "", Version: "0.1.0"},
+			},
+			expect: &chart.Lock{
+				Dependencies: []*chart.Dependency{
+					{Name: "localdependency", Repository: "", Version: "0.1.0"},
+				},
+			},
+		},
+		{
+			name: "repo from invalid path under charts path",
+			req: []*chart.Dependency{
+				{Name: "nonexistentdependency", Repository: "", Version: "0.1.0"},
+			},
+			expect: &chart.Lock{
+				Dependencies: []*chart.Dependency{
+					{Name: "nonexistentlocaldependency", Repository: "", Version: "0.1.0"},
+				},
+			},
+			err: true,
+		},
 	}
 
 	repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"}
diff --git a/internal/resolver/testdata/chartpath/charts/localdependency/Chart.yaml b/internal/resolver/testdata/chartpath/charts/localdependency/Chart.yaml
new file mode 100644
index 000000000..083c51ee5
--- /dev/null
+++ b/internal/resolver/testdata/chartpath/charts/localdependency/Chart.yaml
@@ -0,0 +1,3 @@
+description: A Helm chart for Kubernetes
+name: localdependency
+version: 0.1.0
diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go
index 06c62ecaa..7939771c9 100644
--- a/pkg/downloader/manager_test.go
+++ b/pkg/downloader/manager_test.go
@@ -147,6 +147,13 @@ func TestGetRepoNames(t *testing.T) {
 			},
 			expect: map[string]string{"oedipus-rex": "testing"},
 		},
+		{
+			name: "repo from local chart under charts path",
+			req: []*chart.Dependency{
+				{Name: "local-subchart", Repository: ""},
+			},
+			expect: map[string]string{},
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/pkg/downloader/testdata/local-subchart/Chart.yaml b/pkg/downloader/testdata/local-subchart/Chart.yaml
new file mode 100644
index 000000000..1e17203e5
--- /dev/null
+++ b/pkg/downloader/testdata/local-subchart/Chart.yaml
@@ -0,0 +1,3 @@
+description: A Helm chart for Kubernetes
+name: local-subchart
+version: 0.1.0