fixes fetching relative chart urls from index (#388)

* fixes fetching relative chart urls from index

Helm supports repository indexes that use relative URLs to chart
packages (formed by omitting the --url flag in `helm repo index`). This
patch enables Monocular to do the same.

* clarification for possible invalid relative URL

* use raw repo URL string to form url.URL var

When passing the url.URL pointer, we're continuously appending to the
same path. By re-creating the url.URL var each time we ensure we only
append to the base repo URL.

Author: prydonius

src/api/data/cache/cache.go

@@ -178,7 +178,7 @@ func (c *cachedCharts) Refresh() error {
 
 		// 3.1 - parallellize processing
 		for _, chart := range charts {
-			go processChartMetadata(chart, ch)
+			go processChartMetadata(chart, repo.URL, ch)
 		}
 		// 3.2 Channel drain
 		for range charts {
@@ -207,7 +207,7 @@ type chanItem struct {
 // Counting semaphore, 25 downloads max in paralell
 var tokens = make(chan struct{}, 15)
 
-func processChartMetadata(chart *swaggermodels.ChartPackage, out chan<- chanItem) {
+func processChartMetadata(chart *swaggermodels.ChartPackage, repoURL string, out chan<- chanItem) {
 	tokens <- struct{}{}
 	// Semaphore control channel
 	defer func() {
@@ -231,7 +231,7 @@ func processChartMetadata(chart *swaggermodels.ChartPackage, out chan<- chanItem
 			"version": *chart.Version,
 		}).Info("Local cache missing")
 
-		err := charthelper.DownloadAndExtractChartTarball(chart)
+		err := charthelper.DownloadAndExtractChartTarball(chart, repoURL)
 		if err != nil {
 			log.WithFields(log.Fields{
 				"error": err,

src/api/data/cache/cache_test.go

@@ -80,7 +80,7 @@ func TestCachedChartsRefresh(t *testing.T) {
 	// Stubs Download and processing
 	DownloadAndExtractChartTarballOrig := charthelper.DownloadAndExtractChartTarball
 	defer func() { charthelper.DownloadAndExtractChartTarball = DownloadAndExtractChartTarballOrig }()
-	charthelper.DownloadAndExtractChartTarball = func(chart *swaggermodels.ChartPackage) error { return nil }
+	charthelper.DownloadAndExtractChartTarball = func(chart *swaggermodels.ChartPackage, repoURL string) error { return nil }
 
 	DownloadAndProcessChartIconOrig := charthelper.DownloadAndProcessChartIcon
 	defer func() { charthelper.DownloadAndProcessChartIcon = DownloadAndProcessChartIconOrig }()
@@ -120,7 +120,7 @@ func TestCachedChartsRefreshErrorDownloadingPackage(t *testing.T) {
 	DownloadAndExtractChartTarballOrig := charthelper.DownloadAndExtractChartTarball
 	defer func() { charthelper.DownloadAndExtractChartTarball = DownloadAndExtractChartTarballOrig }()
 	knownError := errors.New("error on DownloadAndExtractChartTarball")
-	charthelper.DownloadAndExtractChartTarball = func(chart *swaggermodels.ChartPackage) error {
+	charthelper.DownloadAndExtractChartTarball = func(chart *swaggermodels.ChartPackage, repoURL string) error {
 		return knownError
 	}
 

src/api/data/cache/charthelper/chart_package_helper.go

@@ -7,7 +7,9 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"time"
@@ -22,7 +24,7 @@ const defaultTimeout time.Duration = 10 * time.Second
 
 // DownloadAndExtractChartTarball the chart tar file linked by metadata.Urls and store
 // the wanted files (i.e README.md) under chartDataDir
-var DownloadAndExtractChartTarball = func(chart *models.ChartPackage) (err error) {
+var DownloadAndExtractChartTarball = func(chart *models.ChartPackage, repoURL string) (err error) {
 	if err := ensureChartDataDir(chart); err != nil {
 		return err
 	}
@@ -34,7 +36,7 @@ var DownloadAndExtractChartTarball = func(chart *models.ChartPackage) (err error
 	}()
 
 	if !tarballExists(chart) {
-		if err := downloadTarball(chart); err != nil {
+		if err := downloadTarball(chart, repoURL); err != nil {
 			return err
 		}
 	}
@@ -53,8 +55,17 @@ var tarballExists = func(chart *models.ChartPackage) bool {
 
 // Downloads the tar.gz file associated with the chart version exposed by the index
 // in order to extract specific files for caching
-var downloadTarball = func(chart *models.ChartPackage) error {
+var downloadTarball = func(chart *models.ChartPackage, repoURL string) error {
 	source := chart.Urls[0]
+	if _, err := url.ParseRequestURI(source); err != nil {
+		// If the chart URL is not absolute, join with repo URL. It's fine if the
+		// URL we build here is invalid as we can catch this error when actually
+		// making the request
+		u, _ := url.Parse(repoURL)
+		u.Path = path.Join(u.Path, source)
+		source = u.String()
+	}
+
 	destination := TarballPath(chart)
 
 	// Create output

src/api/data/cache/charthelper/chart_package_helper_test.go

@@ -14,10 +14,18 @@ import (
 	"github.com/kubernetes-helm/monocular/src/api/swagger/models"
 )
 
+var repoURL = "http://storage.googleapis.com/kubernetes-charts/"
+
 func TestDownloadAndExtractChartTarballOk(t *testing.T) {
 	chart, err := getTestChart()
 	assert.NoErr(t, err)
-	assert.NoErr(t, DownloadAndExtractChartTarball(chart))
+	assert.NoErr(t, DownloadAndExtractChartTarball(chart, repoURL))
+
+	// Relative URLs
+	chart, err = getTestChart()
+	assert.NoErr(t, err)
+	chart.Urls[0] = "drupal-0.3.0.tgz"
+	assert.NoErr(t, DownloadAndExtractChartTarball(chart, repoURL))
 }
 
 func TestDownloadAndExtractChartTarballErrorCantWrite(t *testing.T) {
@@ -27,22 +35,22 @@ func TestDownloadAndExtractChartTarballErrorCantWrite(t *testing.T) {
 
 	chart, err := getTestChart()
 	assert.NoErr(t, err)
-	err = DownloadAndExtractChartTarball(chart)
+	err = DownloadAndExtractChartTarball(chart, repoURL)
 	assert.ExistsErr(t, err, "trying to create a non valid directory")
 }
 
 func TestDownloadAndExtractChartTarballErrorDownload(t *testing.T) {
 	// Stubs
 	downloadTarballOrig := downloadTarball
 	defer func() { downloadTarball = downloadTarballOrig }()
-	downloadTarball = func(chart *models.ChartPackage) error { return errors.New("Can't download") }
+	downloadTarball = func(chart *models.ChartPackage, url string) error { return errors.New("Can't download") }
 	tarballExistsOrig := tarballExists
 	defer func() { tarballExists = tarballExistsOrig }()
 	tarballExists = func(chart *models.ChartPackage) bool { return false }
 	// EO stubs
 	chart, err := getTestChart()
 	assert.NoErr(t, err)
-	err = DownloadAndExtractChartTarball(chart)
+	err = DownloadAndExtractChartTarball(chart, repoURL)
 	assert.ExistsErr(t, err, "Error downloading the tar file")
 	_, err = os.Stat(chartDataDir(chart))
 	assert.ExistsErr(t, err, "chart data dir has been removed")
@@ -54,7 +62,7 @@ func TestDownloadAndExtractChartTarballErrorExtract(t *testing.T) {
 	extractFilesFromTarball = func(chart *models.ChartPackage) error { return errors.New("Can't download") }
 	chart, err := getTestChart()
 	assert.NoErr(t, err)
-	err = DownloadAndExtractChartTarball(chart)
+	err = DownloadAndExtractChartTarball(chart, repoURL)
 	assert.ExistsErr(t, err, "Error extracting tar file content")
 	_, err = os.Stat(chartDataDir(chart))
 	assert.ExistsErr(t, err, "chart data dir has been removed")
@@ -79,7 +87,7 @@ func TestDownloadTarballCreatesFileInDestination(t *testing.T) {
 		return pathExists
 	}
 
-	err = downloadTarball(chart)
+	err = downloadTarball(chart, repoURL)
 	assert.NoErr(t, err)
 	_, err = os.Stat(filepath.Join(chartDataDir(chart), "chart.tgz"))
 	assert.NoErr(t, err)
@@ -94,14 +102,9 @@ func TestDownloadTarballErrorDownloading(t *testing.T) {
 	TarballPath = func(chart *models.ChartPackage) string {
 		return filepath.Join(randomPath, "myFile.tar.gz")
 	}
-	// Invald protocol
-	chart.Urls[0] = "./invalid-protocol/bogusUrl"
-	err = downloadTarball(chart)
-	assert.ExistsErr(t, err, "Invalid protocol")
-
 	// 404
 	chart.Urls[0] = "http://localhost/bogusUrl"
-	err = downloadTarball(chart)
+	err = downloadTarball(chart, repoURL)
 	assert.ExistsErr(t, err, "Cant copy")
 }