Add characterization tests to understand getter.WithURL()

Signed-off-by: Reinaldo de Souza Jr <github@rei.nal.do>

pkg/downloader/chart_downloader_test.go

@@ -19,6 +19,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"reflect"
 	"testing"
 
 	"helm.sh/helm/v3/internal/test/ensure"
@@ -42,6 +43,7 @@ func TestResolveChartRef(t *testing.T) {
 		{name: "full URL", ref: "http://example.com/foo-1.2.3.tgz", expect: "http://example.com/foo-1.2.3.tgz"},
 		{name: "full URL, HTTPS", ref: "https://example.com/foo-1.2.3.tgz", expect: "https://example.com/foo-1.2.3.tgz"},
 		{name: "full URL, with authentication", ref: "http://username:password@example.com/foo-1.2.3.tgz", expect: "http://username:password@example.com/foo-1.2.3.tgz"},
+		{name: "full URL, no repo", ref: "https://unknown-example.com/foo-1.2.3.tgz", expect: "https://unknown-example.com/foo-1.2.3.tgz"},
 		{name: "reference, testing repo", ref: "testing/alpine", expect: "http://example.com/alpine-1.2.3.tgz"},
 		{name: "reference, version, testing repo", ref: "testing/alpine", version: "0.2.0", expect: "http://example.com/alpine-0.2.0.tgz"},
 		{name: "reference, version, malformed repo", ref: "malformed/alpine", version: "1.2.3", expect: "http://dl.example.com/alpine-1.2.3.tgz"},
@@ -85,15 +87,41 @@ func TestResolveChartOpts(t *testing.T) {
 	tests := []struct {
 		name, ref, version string
 		expect             []getter.Option
+		expectChartURL     string
 	}{
 		{
-			name: "repo with CA-file",
+			name:           "reference",
-			ref:  "testing-ca-file/foo",
+			ref:            "testing/alpine",
+			expectChartURL: "http://example.com/alpine-1.2.3.tgz",
+			expect: []getter.Option{
+				getter.WithURL("http://example.com/alpine-1.2.3.tgz"),
+			},
+		},
+		{
+			name:           "reference, repo with CA-file",
+			ref:            "testing-ca-file/foo",
+			expectChartURL: "https://example.com/foo-1.2.3.tgz",
 			expect: []getter.Option{
 				getter.WithURL("https://example.com/foo-1.2.3.tgz"),
 				getter.WithTLSClientConfig("cert", "key", "ca"),
 			},
 		},
+		{
+			name:           "full URL",
+			ref:            "http://example.com/foo-1.2.3.tgz",
+			expectChartURL: "http://example.com/foo-1.2.3.tgz",
+			expect: []getter.Option{
+				getter.WithURL("http://example.com/foo-1.2.3.tgz"),
+			},
+		},
+		{
+			name:           "full URL, no repo",
+			ref:            "http://unknown-example.com/foo-1.2.3.tgz",
+			expectChartURL: "http://unknown-example.com/foo-1.2.3.tgz",
+			expect: []getter.Option{
+				getter.WithURL("http://unknown-example.com/foo-1.2.3.tgz"),
+			},
+		},
 	}
 
 	c := ChartDownloader{
@@ -136,8 +164,13 @@ func TestResolveChartOpts(t *testing.T) {
 			continue
 		}
 
-		if *(got.(*getter.HTTPGetter)) != *(expect.(*getter.HTTPGetter)) {
+		if got := u.String(); tt.expectChartURL != u.String() {
-			t.Errorf("%s: expected %s, got %s", tt.name, expect, got)
+			t.Errorf("%s: expected %#v, got %#v", tt.name, tt.expectChartURL, got)
+			continue
+		}
+
+		if !reflect.DeepEqual(got.(*getter.HTTPGetter), expect.(*getter.HTTPGetter)) {
+			t.Errorf("%s: expected %#v, got %#v", tt.name, expect, got)
 		}
 	}
 }

pkg/getter/httpgetter_test.go

@@ -180,6 +180,7 @@ func TestDownload(t *testing.T) {
 
 func TestDownloadTLS(t *testing.T) {
 	cd := "../../testdata"
+	// Certificate's server name is "helm.sh"
 	ca, pub, priv := filepath.Join(cd, "rootca.crt"), filepath.Join(cd, "crt.pem"), filepath.Join(cd, "key.pem")
 
 	tlsSrv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
@@ -194,36 +195,40 @@ func TestDownloadTLS(t *testing.T) {
 	defer tlsSrv.Close()
 
 	u, _ := url.ParseRequestURI(tlsSrv.URL)
-	g, err := NewHTTPGetter(
+	ipAddress := u.String()
-		WithURL(u.String()),
+
-		WithTLSClientConfig(pub, priv, ca),
+	u.Host = tlsConf.ServerName
-	)
+	hostnameAddress := u.String()
-	if err != nil {
+
-		t.Fatal(err)
+	// tlsSrv.URL has an IP as host (127.0.0.1)
-	}
+	// for this reason, need to also test with a hostname address
-
+	for _, aURL := range []string{ipAddress, hostnameAddress} {
-	if _, err := g.Get(u.String()); err != nil {
+		g, _ := NewHTTPGetter(
-		t.Error(err)
+			WithURL(aURL),
-	}
+			WithTLSClientConfig(pub, priv, ca),
-
+		)
-	// now test with TLS config being passed along in .Get (see #6635)
+		if _, err := g.Get(ipAddress); err != nil {
-	g, err = NewHTTPGetter()
+			t.Errorf("%s: %s", aURL, err)
-	if err != nil {
+		}
-		t.Fatal(err)
-	}
 
-	if _, err := g.Get(u.String(), WithURL(u.String()), WithTLSClientConfig(pub, priv, ca)); err != nil {
+		// now test with TLS config being passed along in .Get (see #6635)
-		t.Error(err)
+		g, _ = NewHTTPGetter()
-	}
+		if _, err := g.Get(ipAddress, WithURL(aURL), WithTLSClientConfig(pub, priv, ca)); err != nil {
+			t.Errorf("%s: %s", aURL, err)
+		}
 
-	// test with only the CA file (see also #6635)
+		// test with only the CA file (see also #6635)
-	g, err = NewHTTPGetter()
+		g, _ = NewHTTPGetter()
-	if err != nil {
+		if _, err := g.Get(ipAddress, WithURL(aURL), WithTLSClientConfig("", "", ca)); err != nil {
-		t.Fatal(err)
+			t.Errorf("%s: %s", aURL, err)
-	}
+		}
 
-	if _, err := g.Get(u.String(), WithURL(u.String()), WithTLSClientConfig("", "", ca)); err != nil {
+		// test TLS validation actually validates
-		t.Error(err)
+		g, _ = NewHTTPGetter()
+		u.Host = "totally-legit-helm.sh"
+		if _, err := g.Get(ipAddress, WithURL(u.String()), WithTLSClientConfig(pub, priv, ca)); err == nil {
+			t.Errorf("%s: expected to fail with: 'certificate is valid for helm.sh, not totally-legit-helm.sh'", aURL)
+		}
 	}
 }