Merge pull request #3258 from mparry/fix-ca-file-option

(fix) Handle caFile alone being set for repos

pkg/getter/httpgetter.go

@@ -23,7 +23,6 @@ import (
 	"strings"
 
 	"k8s.io/helm/pkg/tlsutil"
-	"k8s.io/helm/pkg/urlutil"
 	"k8s.io/helm/pkg/version"
 )
 
@@ -82,19 +81,11 @@ func newHTTPGetter(URL, CertFile, KeyFile, CAFile string) (Getter, error) {
 // NewHTTPGetter constructs a valid http/https client as HttpGetter
 func NewHTTPGetter(URL, CertFile, KeyFile, CAFile string) (*HttpGetter, error) {
 	var client HttpGetter
-	if CertFile != "" && KeyFile != "" {
+	if (CertFile != "" && KeyFile != "") || CAFile != "" {
-		tlsConf, err := tlsutil.NewClientTLS(CertFile, KeyFile, CAFile)
+		tlsConf, err := tlsutil.NewTLSConfig(URL, CertFile, KeyFile, CAFile)
 		if err != nil {
-			return &client, fmt.Errorf("can't create TLS config for client: %s", err.Error())
+			return &client, fmt.Errorf("can't create TLS config: %s", err.Error())
 		}
-		tlsConf.BuildNameToCertificate()
-
-		sni, err := urlutil.ExtractHostname(URL)
-		if err != nil {
-			return &client, err
-		}
-		tlsConf.ServerName = sni
-
 		client.client = &http.Client{
 			Transport: &http.Transport{
 				TLSClientConfig: tlsConf,

pkg/getter/httpgetter_test.go

@@ -28,7 +28,7 @@ func TestHTTPGetter(t *testing.T) {
 	}
 
 	if hg, ok := g.(*HttpGetter); !ok {
-		t.Fatal("Expected newHTTPGetter to produce an httpGetter")
+		t.Fatal("Expected newHTTPGetter to produce an HttpGetter")
 	} else if hg.client != http.DefaultClient {
 		t.Fatal("Expected newHTTPGetter to return a default HTTP client.")
 	}
@@ -37,12 +37,24 @@ func TestHTTPGetter(t *testing.T) {
 	cd := "../../testdata"
 	join := filepath.Join
 	ca, pub, priv := join(cd, "ca.pem"), join(cd, "crt.pem"), join(cd, "key.pem")
-	g, err = newHTTPGetter("http://example.com/", pub, priv, ca)
+	g, err = newHTTPGetter("https://example.com/", pub, priv, ca)
 	if err != nil {
 		t.Fatal(err)
 	}
+	if hg, ok := g.(*HttpGetter); !ok {
+		t.Fatal("Expected newHTTPGetter to produce an HttpGetter")
+	} else if hg.client == http.DefaultClient {
+		t.Fatal("Expected newHTTPGetter to return a non-default HTTP client")
+	}
 
-	if _, ok := g.(*HttpGetter); !ok {
+	// Test with SSL, caFile only
-		t.Fatal("Expected newHTTPGetter to produce an httpGetter")
+	g, err = newHTTPGetter("https://example.com/", "", "", ca)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if hg, ok := g.(*HttpGetter); !ok {
+		t.Fatal("Expected newHTTPGetter to produce an HttpGetter")
+	} else if hg.client == http.DefaultClient {
+		t.Fatal("Expected newHTTPGetter to return a non-default HTTP client")
 	}
 }

pkg/tlsutil/tls.go

@@ -21,17 +21,20 @@ import (
 	"crypto/x509"
 	"fmt"
 	"io/ioutil"
+	"k8s.io/helm/pkg/urlutil"
 )
 
-// NewClientTLS returns tls.Config appropriate for client auth.
+func newTLSConfigCommon(certFile, keyFile, caFile string) (*tls.Config, error) {
-func NewClientTLS(certFile, keyFile, caFile string) (*tls.Config, error) {
+	config := tls.Config{}
+
+	if certFile != "" && keyFile != "" {
 		cert, err := CertFromFilePair(certFile, keyFile)
 		if err != nil {
 			return nil, err
 		}
-	config := tls.Config{
+		config.Certificates = []tls.Certificate{*cert}
-		Certificates: []tls.Certificate{*cert},
 	}
+
 	if caFile != "" {
 		cp, err := CertPoolFromFile(caFile)
 		if err != nil {
@@ -39,9 +42,32 @@ func NewClientTLS(certFile, keyFile, caFile string) (*tls.Config, error) {
 		}
 		config.RootCAs = cp
 	}
+
 	return &config, nil
 }
 
+// NewClientTLS returns tls.Config appropriate for client auth.
+func NewClientTLS(certFile, keyFile, caFile string) (*tls.Config, error) {
+	return newTLSConfigCommon(certFile, keyFile, caFile)
+}
+
+// NewTLSConfig returns tls.Config appropriate for client and/or server auth.
+func NewTLSConfig(url, certFile, keyFile, caFile string) (*tls.Config, error) {
+	config, err := newTLSConfigCommon(certFile, keyFile, caFile)
+	if err != nil {
+		return nil, err
+	}
+	config.BuildNameToCertificate()
+
+	serverName, err := urlutil.ExtractHostname(url)
+	if err != nil {
+		return nil, err
+	}
+	config.ServerName = serverName
+
+	return config, nil
+}
+
 // CertPoolFromFile returns an x509.CertPool containing the certificates
 // in the given PEM-encoded file.
 // Returns an error if the file could not be read, a certificate could not