From 260637b45cf90031bccf65c8310d2c638bf0cc72 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Fri, 25 Feb 2022 15:14:42 +0000 Subject: [PATCH] Reuse transport for helm chart download Reuses the same transport across different helm chart downloads, whilst resetting the tlsconfig to avoid cross-contamination. Crypto material is now only processed in-memory and does not touch the disk. Signed-off-by: Paulo Gomes --- controllers/helmchart_controller.go | 46 +++++----- controllers/helmrepository_controller.go | 28 +++--- controllers/helmrepository_controller_test.go | 4 +- go.mod | 1 + go.sum | 2 + internal/helm/getter/getter.go | 71 +++++---------- internal/helm/getter/getter_test.go | 90 ++++++++++++++----- internal/helm/getter/transport.go | 72 +++++++++++++++ internal/helm/getter/transport_test.go | 63 +++++++++++++ internal/helm/repository/chart_repository.go | 19 +++- .../helm/repository/chart_repository_test.go | 6 +- 11 files changed, 287 insertions(+), 115 deletions(-) create mode 100644 internal/helm/getter/transport.go create mode 100644 internal/helm/getter/transport_test.go diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 5bbe56cd4..8d0da6f37 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "crypto/tls" "errors" "fmt" "net/url" @@ -368,6 +369,7 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1 func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *sourcev1.HelmChart, repo *sourcev1.HelmRepository, b *chart.Build) (sreconcile.Result, error) { + var tlsConfig *tls.Config // Construct the Getter options from the HelmRepository data clientOpts := []helmgetter.Option{ @@ -386,34 +388,33 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * return sreconcile.ResultEmpty, e } - // Create temporary working directory for credentials - authDir, err := util.TempDirForObj("", obj) + // Build client options from secret + opts, err := getter.ClientOptionsFromSecret(*secret) if err != nil { e := &serror.Event{ - Err: fmt.Errorf("failed to create temporary working directory: %w", err), - Reason: sourcev1.StorageOperationFailedReason, + Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), + Reason: sourcev1.AuthenticationFailedReason, } - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedReason, e.Err.Error()) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error()) + // Requeue as content of secret might change return sreconcile.ResultEmpty, e } - defer os.RemoveAll(authDir) + clientOpts = append(clientOpts, opts...) - // Build client options from secret - opts, err := getter.ClientOptionsFromSecret(authDir, *secret) + tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, repo.Spec.URL) if err != nil { e := &serror.Event{ - Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), + Err: fmt.Errorf("failed to create tls client config with secret data: %w", err), Reason: sourcev1.AuthenticationFailedReason, } conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error()) // Requeue as content of secret might change return sreconcile.ResultEmpty, e } - clientOpts = append(clientOpts, opts...) } // Initialize the chart repository - chartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, clientOpts) + chartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts) if err != nil { // Any error requires a change in generation, // which we should be informed about by the watcher @@ -523,15 +524,8 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj } // Setup dependency manager - authDir := filepath.Join(tmpDir, "creds") - if err = os.Mkdir(authDir, 0700); err != nil { - return sreconcile.ResultEmpty, &serror.Event{ - Err: fmt.Errorf("failed to create temporary directory for dependency credentials: %w", err), - Reason: meta.FailedReason, - } - } dm := chart.NewDependencyManager( - chart.WithRepositoryCallback(r.namespacedChartRepositoryCallback(ctx, authDir, obj.GetNamespace())), + chart.WithRepositoryCallback(r.namespacedChartRepositoryCallback(ctx, obj.GetNamespace())), ) defer dm.Clear() @@ -747,11 +741,11 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1. } // namespacedChartRepositoryCallback returns a chart.GetChartRepositoryCallback scoped to the given namespace. -// Credentials for retrieved v1beta1.HelmRepository objects are stored in the given directory. // The returned callback returns a repository.ChartRepository configured with the retrieved v1beta1.HelmRepository, // or a shim with defaults if no object could be found. -func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, dir, namespace string) chart.GetChartRepositoryCallback { +func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, namespace string) chart.GetChartRepositoryCallback { return func(url string) (*repository.ChartRepository, error) { + var tlsConfig *tls.Config repo, err := r.resolveDependencyRepository(ctx, url, namespace) if err != nil { // Return Kubernetes client errors, but ignore others @@ -774,13 +768,19 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont if err != nil { return nil, err } - opts, err := getter.ClientOptionsFromSecret(dir, *secret) + opts, err := getter.ClientOptionsFromSecret(*secret) if err != nil { return nil, err } clientOpts = append(clientOpts, opts...) + + tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, repo.Spec.URL) + if err != nil { + return nil, fmt.Errorf("failed to create tls client config for HelmRepository '%s': %w", repo.Name, err) + } } - chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, clientOpts) + + chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, tlsConfig, clientOpts) if err != nil { return nil, err } diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index bc154da73..671d2902d 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "crypto/tls" "errors" "fmt" "net/url" @@ -260,6 +261,8 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, obj *so // If the download is successful, the given artifact pointer is set to a new artifact with the available metadata, and // the index pointer is set to the newly downloaded index. func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) { + var tlsConfig *tls.Config + // Configure Helm client to access repository clientOpts := []helmgetter.Option{ helmgetter.WithTimeout(obj.Spec.Timeout.Duration), @@ -284,18 +287,8 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou return sreconcile.ResultEmpty, e } - // Get client options from secret - tmpDir, err := os.MkdirTemp("", fmt.Sprintf("%s-%s-auth-", obj.Name, obj.Namespace)) - if err != nil { - return sreconcile.ResultEmpty, &serror.Event{ - Err: fmt.Errorf("failed to create temporary directory for credentials: %w", err), - Reason: sourcev1.StorageOperationFailedReason, - } - } - defer os.RemoveAll(tmpDir) - // Construct actual options - opts, err := getter.ClientOptionsFromSecret(tmpDir, secret) + opts, err := getter.ClientOptionsFromSecret(secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), @@ -306,10 +299,21 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou return sreconcile.ResultEmpty, e } clientOpts = append(clientOpts, opts...) + + tlsConfig, err = getter.TLSClientConfigFromSecret(secret, obj.Spec.URL) + if err != nil { + e := &serror.Event{ + Err: fmt.Errorf("failed to create tls client config with secret data: %w", err), + Reason: sourcev1.AuthenticationFailedReason, + } + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error()) + // Requeue as content of secret might change + return sreconcile.ResultEmpty, e + } } // Construct Helm chart repository with options and download index - newChartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", r.Getters, clientOpts) + newChartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", r.Getters, tlsConfig, clientOpts) if err != nil { switch err.(type) { case *url.Error: diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go index a337d04bb..89a705229 100644 --- a/controllers/helmrepository_controller_test.go +++ b/controllers/helmrepository_controller_test.go @@ -364,7 +364,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) { }, wantErr: true, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.FetchFailedCondition, meta.FailedReason, "can't create TLS config for client: failed to append certificates from file"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to create tls client config with secret data: cannot append certificate into certificate pool: invalid caFile"), }, }, { @@ -600,7 +600,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) g.Expect(cacheFile.Close()).ToNot(HaveOccurred()) - chartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", testGetters, nil) + chartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", testGetters, nil, nil) g.Expect(err).ToNot(HaveOccurred()) chartRepo.CachePath = cachePath diff --git a/go.mod b/go.mod index f3d678a6b..8e671bab3 100644 --- a/go.mod +++ b/go.mod @@ -39,6 +39,7 @@ require ( k8s.io/api v0.23.3 k8s.io/apimachinery v0.23.3 k8s.io/client-go v0.23.3 + k8s.io/helm v2.17.0+incompatible k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 sigs.k8s.io/cli-utils v0.28.0 sigs.k8s.io/controller-runtime v0.11.1 diff --git a/go.sum b/go.sum index acdb7bbd7..73ecc77ed 100644 --- a/go.sum +++ b/go.sum @@ -1759,6 +1759,8 @@ k8s.io/gengo v0.0.0-20200428234225-8167cfdcfc14/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8 k8s.io/gengo v0.0.0-20201113003025-83324d819ded/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/gengo v0.0.0-20201214224949-b6c5ce23f027/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= +k8s.io/helm v2.17.0+incompatible h1:Bpn6o1wKLYqKM3+Osh8e+1/K2g/GsQJ4F4yNF2+deao= +k8s.io/helm v2.17.0+incompatible/go.mod h1:LZzlS4LQBHfciFOurYBFkCMTaZ0D1l+p0teMg7TSULI= k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= k8s.io/klog/v2 v2.4.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= diff --git a/internal/helm/getter/getter.go b/internal/helm/getter/getter.go index 583bac5f7..4412700bb 100644 --- a/internal/helm/getter/getter.go +++ b/internal/helm/getter/getter.go @@ -17,16 +17,18 @@ limitations under the License. package getter import ( + "crypto/tls" + "crypto/x509" "fmt" - "os" "helm.sh/helm/v3/pkg/getter" corev1 "k8s.io/api/core/v1" + "k8s.io/helm/pkg/urlutil" ) // ClientOptionsFromSecret constructs a getter.Option slice for the given secret. // It returns the slice, or an error. -func ClientOptionsFromSecret(dir string, secret corev1.Secret) ([]getter.Option, error) { +func ClientOptionsFromSecret(secret corev1.Secret) ([]getter.Option, error) { var opts []getter.Option basicAuth, err := BasicAuthFromSecret(secret) if err != nil { @@ -35,13 +37,6 @@ func ClientOptionsFromSecret(dir string, secret corev1.Secret) ([]getter.Option, if basicAuth != nil { opts = append(opts, basicAuth) } - tlsClientConfig, err := TLSClientConfigFromSecret(dir, secret) - if err != nil { - return opts, err - } - if tlsClientConfig != nil { - opts = append(opts, tlsClientConfig) - } return opts, nil } @@ -62,13 +57,11 @@ func BasicAuthFromSecret(secret corev1.Secret) (getter.Option, error) { } // TLSClientConfigFromSecret attempts to construct a TLS client config -// getter.Option for the given v1.Secret, placing the required TLS config -// related files in the given directory. It returns the getter.Option, or -// an error. +// for the given v1.Secret. It returns the TLS client config or an error. // // Secrets with no certFile, keyFile, AND caFile are ignored, if only a // certBytes OR keyBytes is defined it returns an error. -func TLSClientConfigFromSecret(dir string, secret corev1.Secret) (getter.Option, error) { +func TLSClientConfigFromSecret(secret corev1.Secret, url string) (*tls.Config, error) { certBytes, keyBytes, caBytes := secret.Data["certFile"], secret.Data["keyFile"], secret.Data["caFile"] switch { case len(certBytes)+len(keyBytes)+len(caBytes) == 0: @@ -78,49 +71,31 @@ func TLSClientConfigFromSecret(dir string, secret corev1.Secret) (getter.Option, secret.Name) } - var certPath, keyPath, caPath string + tlsConf := &tls.Config{} if len(certBytes) > 0 && len(keyBytes) > 0 { - certFile, err := os.CreateTemp(dir, "cert-*.crt") - if err != nil { - return nil, err - } - if _, err = certFile.Write(certBytes); err != nil { - _ = certFile.Close() - return nil, err - } - if err = certFile.Close(); err != nil { - return nil, err - } - certPath = certFile.Name() - - keyFile, err := os.CreateTemp(dir, "key-*.crt") + cert, err := tls.X509KeyPair(certBytes, keyBytes) if err != nil { return nil, err } - if _, err = keyFile.Write(keyBytes); err != nil { - _ = keyFile.Close() - return nil, err - } - if err = keyFile.Close(); err != nil { - return nil, err - } - keyPath = keyFile.Name() + tlsConf.Certificates = append(tlsConf.Certificates, cert) } if len(caBytes) > 0 { - caFile, err := os.CreateTemp(dir, "ca-*.pem") - if err != nil { - return nil, err + cp := x509.NewCertPool() + if !cp.AppendCertsFromPEM(caBytes) { + return nil, fmt.Errorf("cannot append certificate into certificate pool: invalid caFile") } - if _, err = caFile.Write(caBytes); err != nil { - _ = caFile.Close() - return nil, err - } - if err = caFile.Close(); err != nil { - return nil, err - } - caPath = caFile.Name() + + tlsConf.RootCAs = cp + } + + tlsConf.BuildNameToCertificate() + + sni, err := urlutil.ExtractHostname(url) + if err != nil { + return nil, err } + tlsConf.ServerName = sni - return getter.WithTLSClientConfig(certPath, keyPath, caPath), nil + return tlsConf, nil } diff --git a/internal/helm/getter/getter_test.go b/internal/helm/getter/getter_test.go index 6437e5b35..a13c029e3 100644 --- a/internal/helm/getter/getter_test.go +++ b/internal/helm/getter/getter_test.go @@ -17,7 +17,11 @@ limitations under the License. package getter import ( - "os" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + "math/big" "testing" corev1 "k8s.io/api/core/v1" @@ -30,13 +34,6 @@ var ( "password": []byte("password"), }, } - tlsSecretFixture = corev1.Secret{ - Data: map[string][]byte{ - "certFile": []byte(`fixture`), - "keyFile": []byte(`fixture`), - "caFile": []byte(`fixture`), - }, - } ) func TestClientOptionsFromSecret(t *testing.T) { @@ -45,8 +42,6 @@ func TestClientOptionsFromSecret(t *testing.T) { secrets []corev1.Secret }{ {"basic auth", []corev1.Secret{basicAuthSecretFixture}}, - {"TLS", []corev1.Secret{tlsSecretFixture}}, - {"basic auth and TLS", []corev1.Secret{basicAuthSecretFixture, tlsSecretFixture}}, {"empty", []corev1.Secret{}}, } for _, tt := range tests { @@ -58,13 +53,7 @@ func TestClientOptionsFromSecret(t *testing.T) { } } - tmpDir, err := os.MkdirTemp("", "client-opts-secret-") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpDir) - - got, err := ClientOptionsFromSecret(tmpDir, secret) + got, err := ClientOptionsFromSecret(secret) if err != nil { t.Errorf("ClientOptionsFromSecret() error = %v", err) return @@ -109,6 +98,8 @@ func TestBasicAuthFromSecret(t *testing.T) { } func TestTLSClientConfigFromSecret(t *testing.T) { + tlsSecretFixture := validTlsSecret(t) + tests := []struct { name string secret corev1.Secret @@ -129,13 +120,7 @@ func TestTLSClientConfigFromSecret(t *testing.T) { tt.modify(secret) } - tmpDir, err := os.MkdirTemp("", "client-opts-secret-") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpDir) - - got, err := TLSClientConfigFromSecret(tmpDir, *secret) + got, err := TLSClientConfigFromSecret(*secret, "") if (err != nil) != tt.wantErr { t.Errorf("TLSClientConfigFromSecret() error = %v, wantErr %v", err, tt.wantErr) return @@ -147,3 +132,60 @@ func TestTLSClientConfigFromSecret(t *testing.T) { }) } } + +// validTlsSecret creates a secret containing key pair and CA certificate that are +// valid from a syntax (minimum requirements) perspective. +func validTlsSecret(t *testing.T) corev1.Secret { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal("Private key cannot be created.", err.Error()) + } + + certTemplate := x509.Certificate{ + SerialNumber: big.NewInt(1337), + } + cert, err := x509.CreateCertificate(rand.Reader, &certTemplate, &certTemplate, &key.PublicKey, key) + if err != nil { + t.Fatal("Certificate cannot be created.", err.Error()) + } + + ca := &x509.Certificate{ + SerialNumber: big.NewInt(7331), + IsCA: true, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + } + + caPrivKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + t.Fatal("CA private key cannot be created.", err.Error()) + } + + caBytes, err := x509.CreateCertificate(rand.Reader, ca, ca, &caPrivKey.PublicKey, caPrivKey) + if err != nil { + t.Fatal("CA certificate cannot be created.", err.Error()) + } + + keyPem := pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(key), + }) + + certPem := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: cert, + }) + + caPem := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: caBytes, + }) + + return corev1.Secret{ + Data: map[string][]byte{ + "certFile": []byte(certPem), + "keyFile": []byte(keyPem), + "caFile": []byte(caPem), + }, + } +} diff --git a/internal/helm/getter/transport.go b/internal/helm/getter/transport.go new file mode 100644 index 000000000..ad427eeb3 --- /dev/null +++ b/internal/helm/getter/transport.go @@ -0,0 +1,72 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package getter + +import ( + "crypto/tls" + "fmt" + "net" + "net/http" + "sync" + "time" +) + +type TransportPool struct { +} + +var pool = &sync.Pool{ + New: func() interface{} { + return &http.Transport{ + DisableCompression: true, + Proxy: http.ProxyFromEnvironment, + + IdleConnTimeout: 60 * time.Second, + + // use safe defaults based off http.DefaultTransport + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + } + }, +} + +// NewOrIdle tries to return an existing transport that is not currently being used. +// If none is found, creates a new Transport instead. +// +// tlsConfig sets the TLSClientConfig for the transport and can be nil. +func NewOrIdle(tlsConfig *tls.Config) *http.Transport { + t := pool.Get().(*http.Transport) + t.TLSClientConfig = tlsConfig + + return t +} + +// Release releases the transport back to the TransportPool after +// sanitising its sensitive fields. +func Release(transport *http.Transport) error { + if transport == nil { + return fmt.Errorf("cannot release nil transport") + } + + transport.TLSClientConfig = nil + + pool.Put(transport) + return nil +} diff --git a/internal/helm/getter/transport_test.go b/internal/helm/getter/transport_test.go new file mode 100644 index 000000000..aea7ffc1e --- /dev/null +++ b/internal/helm/getter/transport_test.go @@ -0,0 +1,63 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package getter + +import ( + "crypto/tls" + "testing" +) + +func Test_TransportReuse(t *testing.T) { + t1 := NewOrIdle(nil) + t2 := NewOrIdle(nil) + + if t1 == t2 { + t.Errorf("same transported returned twice") + } + + err := Release(t2) + if err != nil { + t.Errorf("error releasing transport t2: %v", err) + } + + t3 := NewOrIdle(nil) + if t2 != t3 { + t.Errorf("transported not reused") + } + + t4 := NewOrIdle(&tls.Config{ + ServerName: "testing", + }) + if t4.TLSClientConfig == nil || t4.TLSClientConfig.ServerName != "testing" { + t.Errorf("TLSClientConfig not properly configured") + } + + err = Release(t4) + if err != nil { + t.Errorf("error releasing transport t4: %v", err) + } + if t4.TLSClientConfig != nil { + t.Errorf("TLSClientConfig not cleared after release") + } + + err = Release(nil) + if err == nil { + t.Errorf("should not allow release nil transport") + } else if err.Error() != "cannot release nil transport" { + t.Errorf("wanted error message: 'cannot release nil transport' got: %q", err.Error()) + } +} diff --git a/internal/helm/repository/chart_repository.go b/internal/helm/repository/chart_repository.go index fd355c0e8..3c183ad6e 100644 --- a/internal/helm/repository/chart_repository.go +++ b/internal/helm/repository/chart_repository.go @@ -19,6 +19,7 @@ package repository import ( "bytes" "crypto/sha256" + "crypto/tls" "encoding/hex" "errors" "fmt" @@ -38,6 +39,7 @@ import ( "github.com/fluxcd/pkg/version" "github.com/fluxcd/source-controller/internal/helm" + transport "github.com/fluxcd/source-controller/internal/helm/getter" ) var ErrNoChartIndex = errors.New("no chart index") @@ -65,6 +67,8 @@ type ChartRepository struct { // index bytes. Checksum string + tlsConfig *tls.Config + *sync.RWMutex } @@ -72,7 +76,7 @@ type ChartRepository struct { // the ChartRepository.Client configured to the getter.Getter for the // repository URL scheme. It returns an error on URL parsing failures, // or if there is no getter available for the scheme. -func NewChartRepository(repositoryURL, cachePath string, providers getter.Providers, opts []getter.Option) (*ChartRepository, error) { +func NewChartRepository(repositoryURL, cachePath string, providers getter.Providers, tlsConfig *tls.Config, opts []getter.Option) (*ChartRepository, error) { u, err := url.Parse(repositoryURL) if err != nil { return nil, err @@ -87,6 +91,7 @@ func NewChartRepository(repositoryURL, cachePath string, providers getter.Provid r.CachePath = cachePath r.Client = c r.Options = opts + r.tlsConfig = tlsConfig return r, nil } @@ -212,7 +217,11 @@ func (r *ChartRepository) DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer u.RawQuery = q.Encode() } - return r.Client.Get(u.String(), r.Options...) + t := transport.NewOrIdle(r.tlsConfig) + clientOpts := append(r.Options, getter.WithTransport(t)) + defer transport.Release(t) + + return r.Client.Get(u.String(), clientOpts...) } // LoadIndexFromBytes loads Index from the given bytes. @@ -324,8 +333,12 @@ func (r *ChartRepository) DownloadIndex(w io.Writer) (err error) { u.RawPath = path.Join(u.RawPath, "index.yaml") u.Path = path.Join(u.Path, "index.yaml") + t := transport.NewOrIdle(r.tlsConfig) + clientOpts := append(r.Options, getter.WithTransport(t)) + defer transport.Release(t) + var res *bytes.Buffer - res, err = r.Client.Get(u.String(), r.Options...) + res, err = r.Client.Get(u.String(), clientOpts...) if err != nil { return err } diff --git a/internal/helm/repository/chart_repository_test.go b/internal/helm/repository/chart_repository_test.go index c0100dd3d..cc5ed452f 100644 --- a/internal/helm/repository/chart_repository_test.go +++ b/internal/helm/repository/chart_repository_test.go @@ -67,7 +67,7 @@ func TestNewChartRepository(t *testing.T) { t.Run("should construct chart repository", func(t *testing.T) { g := NewWithT(t) - r, err := NewChartRepository(repositoryURL, "", providers, options) + r, err := NewChartRepository(repositoryURL, "", providers, nil, options) g.Expect(err).ToNot(HaveOccurred()) g.Expect(r).ToNot(BeNil()) g.Expect(r.URL).To(Equal(repositoryURL)) @@ -77,7 +77,7 @@ func TestNewChartRepository(t *testing.T) { t.Run("should error on URL parsing failure", func(t *testing.T) { g := NewWithT(t) - r, err := NewChartRepository("https://ex ample.com", "", nil, nil) + r, err := NewChartRepository("https://ex ample.com", "", nil, nil, nil) g.Expect(err).To(HaveOccurred()) g.Expect(err).To(BeAssignableToTypeOf(&url.Error{})) g.Expect(r).To(BeNil()) @@ -87,7 +87,7 @@ func TestNewChartRepository(t *testing.T) { t.Run("should error on unsupported scheme", func(t *testing.T) { g := NewWithT(t) - r, err := NewChartRepository("http://example.com", "", providers, nil) + r, err := NewChartRepository("http://example.com", "", providers, nil, nil) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(Equal("scheme \"http\" not supported")) g.Expect(r).To(BeNil())