Merge pull request #6488 from bacongobbler/port-6258

fix(downloader): bypass index cache when repository URL defined

cmd/helm/dependency_update_test.go

@@ -117,33 +117,6 @@ func TestDependencyUpdateCmd(t *testing.T) {
 	}
 }
 
-func TestDependencyUpdateCmd_SkipRefresh(t *testing.T) {
-	defer resetEnv()()
-	defer ensure.HelmHome(t)()
-
-	srv := repotest.NewServer(helmpath.ConfigPath())
-	defer srv.Stop()
-	copied, err := srv.CopyCharts("testdata/testcharts/*.tgz")
-	if err != nil {
-		t.Fatal(err)
-	}
-	t.Logf("Copied charts:\n%s", strings.Join(copied, "\n"))
-	t.Logf("Listening on directory %s", srv.Root())
-
-	chartname := "depup"
-	createTestingChart(t, helmpath.DataPath(), chartname, srv.URL())
-
-	_, out, err := executeActionCommand(fmt.Sprintf("dependency update --skip-refresh %s", helmpath.DataPath(chartname)))
-	if err == nil {
-		t.Fatal("Expected failure to find the repo with skipRefresh")
-	}
-
-	// This is written directly to stdout, so we have to capture as is.
-	if strings.Contains(out, `update from the "test" chart repository`) {
-		t.Errorf("Repo was unexpectedly updated\n%s", out)
-	}
-}
-
 func TestDependencyUpdateCmd_DontDeleteOldChartsOnError(t *testing.T) {
 	defer resetEnv()()
 	defer ensure.HelmHome(t)()

internal/resolver/resolver.go

@@ -84,11 +84,20 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 			return nil, errors.Wrapf(err, "dependency %q has an invalid version/constraint format", d.Name)
 		}
 
-		idx := filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoNames[d.Name]))
+		repoName := repoNames[d.Name]
+		// if the repository was not defined, but the dependency defines a repository url, bypass the cache
+		if repoName == "" && d.Repository != "" {
+			locked[i] = &chart.Dependency{
+				Name:       d.Name,
+				Repository: d.Repository,
+				Version:    d.Version,
+			}
+			continue
+		}
 
-		repoIndex, err := repo.LoadIndexFile(idx)
+		repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName)))
 		if err != nil {
-			return nil, errors.Wrapf(err, "no cached repo found. (try 'helm repo update') %s", idx)
+			return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName)
 		}
 
 		vs, ok := repoIndex.Entries[d.Name]

internal/resolver/resolver_test.go

@@ -40,7 +40,11 @@ func TestResolve(t *testing.T) {
 			req: []*chart.Dependency{
 				{Name: "oedipus-rex", Repository: "http://example.com", Version: "1.0.0"},
 			},
-			err: true,
+			expect: &chart.Lock{
+				Dependencies: []*chart.Dependency{
+					{Name: "oedipus-rex", Repository: "http://example.com", Version: "1.0.0"},
+				},
+			},
 		},
 		{
 			name: "chart not found failure",
@@ -113,39 +117,41 @@ func TestResolve(t *testing.T) {
 	repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"}
 	r := New("testdata/chartpath", "testdata/repository")
 	for _, tt := range tests {
-		l, err := r.Resolve(tt.req, repoNames)
-		if err != nil {
-			if tt.err {
-				continue
+		t.Run(tt.name, func(t *testing.T) {
+			l, err := r.Resolve(tt.req, repoNames)
+			if err != nil {
+				if tt.err {
+					return
+				}
+				t.Fatal(err)
 			}
-			t.Fatal(err)
-		}
 
-		if tt.err {
-			t.Fatalf("Expected error in test %q", tt.name)
-		}
+			if tt.err {
+				t.Fatalf("Expected error in test %q", tt.name)
+			}
 
-		if h, err := HashReq(tt.expect.Dependencies); err != nil {
-			t.Fatal(err)
-		} else if h != l.Digest {
-			t.Errorf("%q: hashes don't match.", tt.name)
-		}
+			if h, err := HashReq(tt.expect.Dependencies); err != nil {
+				t.Fatal(err)
+			} else if h != l.Digest {
+				t.Errorf("%q: hashes don't match.", tt.name)
+			}
 
-		// Check fields.
-		if len(l.Dependencies) != len(tt.req) {
-			t.Errorf("%s: wrong number of dependencies in lock", tt.name)
-		}
-		d0 := l.Dependencies[0]
-		e0 := tt.expect.Dependencies[0]
-		if d0.Name != e0.Name {
-			t.Errorf("%s: expected name %s, got %s", tt.name, e0.Name, d0.Name)
-		}
-		if d0.Repository != e0.Repository {
-			t.Errorf("%s: expected repo %s, got %s", tt.name, e0.Repository, d0.Repository)
-		}
-		if d0.Version != e0.Version {
-			t.Errorf("%s: expected version %s, got %s", tt.name, e0.Version, d0.Version)
-		}
+			// Check fields.
+			if len(l.Dependencies) != len(tt.req) {
+				t.Errorf("%s: wrong number of dependencies in lock", tt.name)
+			}
+			d0 := l.Dependencies[0]
+			e0 := tt.expect.Dependencies[0]
+			if d0.Name != e0.Name {
+				t.Errorf("%s: expected name %s, got %s", tt.name, e0.Name, d0.Name)
+			}
+			if d0.Repository != e0.Repository {
+				t.Errorf("%s: expected repo %s, got %s", tt.name, e0.Repository, d0.Repository)
+			}
+			if d0.Version != e0.Version {
+				t.Errorf("%s: expected version %s, got %s", tt.name, e0.Version, d0.Version)
+			}
+		})
 	}
 }
 

pkg/downloader/manager.go

@@ -245,7 +245,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 
 		// Any failure to resolve/download a chart should fail:
 		// https://github.com/helm/helm/issues/1439
-		churl, username, password, err := findChartURL(dep.Name, dep.Version, dep.Repository, repos)
+		churl, username, password, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos)
 		if err != nil {
 			saveError = errors.Wrapf(err, "could not find %s", churl)
 			break
@@ -421,7 +421,14 @@ func (m *Manager) getRepoNames(deps []*chart.Dependency) (map[string]string, err
 			}
 		}
 		if !found {
-			missing = append(missing, dd.Repository)
+			repository := dd.Repository
+			// Add if URL
+			_, err := url.ParseRequestURI(repository)
+			if err == nil {
+				reposMap[repository] = repository
+				continue
+			}
+			missing = append(missing, repository)
 		}
 	}
 	if len(missing) > 0 {
@@ -492,7 +499,7 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error {
 // repoURL is the repository to search
 //
 // If it finds a URL that is "relative", it will prepend the repoURL.
-func findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRepository) (url, username, password string, err error) {
+func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRepository) (url, username, password string, err error) {
 	for _, cr := range repos {
 		if urlutil.Equal(repoURL, cr.Config.URL) {
 			var entry repo.ChartVersions
@@ -514,6 +521,10 @@ func findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRep
 			return
 		}
 	}
+	url, err = repo.FindChartInRepoURL(repoURL, name, version, "", "", "", m.Getters)
+	if err == nil {
+		return
+	}
 	err = errors.Errorf("chart %s not found in %s", name, repoURL)
 	return
 }

pkg/downloader/manager_test.go

@@ -77,7 +77,7 @@ func TestFindChartURL(t *testing.T) {
 	version := "0.1.0"
 	repoURL := "http://example.com/charts"
 
-	churl, username, password, err := findChartURL(name, version, repoURL, repos)
+	churl, username, password, err := m.findChartURL(name, version, repoURL, repos)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -106,11 +106,11 @@ func TestGetRepoNames(t *testing.T) {
 		err    bool
 	}{
 		{
-			name: "no repo definition failure",
+			name: "no repo definition, but references a url",
 			req: []*chart.Dependency{
 				{Name: "oedipus-rex", Repository: "http://example.com/test"},
 			},
-			err: true,
+			expect: map[string]string{"http://example.com/test": "http://example.com/test"},
 		},
 		{
 			name: "no repo definition failure -- stable repo",