Skip to content

Commit 59108f4

Browse files
author
Paulo Gomes
authored
Merge pull request #590 from fluxcd/helm-getter-http-transport
Reuse transport for Helm downloads
2 parents a4012f2 + 7d61553 commit 59108f4

File tree

11 files changed

+426
-154
lines changed

11 files changed

+426
-154
lines changed

‎controllers/helmchart_controller.go

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package controllers
1818

1919
import (
2020
"context"
21+
"crypto/tls"
2122
"errors"
2223
"fmt"
2324
"net/url"
@@ -368,6 +369,7 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1
368369

369370
func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *sourcev1.HelmChart,
370371
repo *sourcev1.HelmRepository, b *chart.Build) (sreconcile.Result, error) {
372+
var tlsConfig *tls.Config
371373

372374
// Construct the Getter options from the HelmRepository data
373375
clientOpts := []helmgetter.Option{
@@ -386,34 +388,33 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
386388
return sreconcile.ResultEmpty, e
387389
}
388390

389-
// Create temporary working directory for credentials
390-
authDir, err := util.TempDirForObj("", obj)
391+
// Build client options from secret
392+
opts, err := getter.ClientOptionsFromSecret(*secret)
391393
if err != nil {
392394
e := &serror.Event{
393-
Err: fmt.Errorf("failed to create temporary working directory: %w", err),
394-
Reason: sourcev1.StorageOperationFailedReason,
395+
Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err),
396+
Reason: sourcev1.AuthenticationFailedReason,
395397
}
396-
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedReason, e.Err.Error())
398+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
399+
// Requeue as content of secret might change
397400
return sreconcile.ResultEmpty, e
398401
}
399-
defer os.RemoveAll(authDir)
402+
clientOpts = append(clientOpts, opts...)
400403

401-
// Build client options from secret
402-
opts, err := getter.ClientOptionsFromSecret(authDir, *secret)
404+
tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, repo.Spec.URL)
403405
if err != nil {
404406
e := &serror.Event{
405-
Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err),
407+
Err: fmt.Errorf("failed to create TLS client config with secret data: %w", err),
406408
Reason: sourcev1.AuthenticationFailedReason,
407409
}
408410
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
409411
// Requeue as content of secret might change
410412
return sreconcile.ResultEmpty, e
411413
}
412-
clientOpts = append(clientOpts, opts...)
413414
}
414415

415416
// Initialize the chart repository
416-
chartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, clientOpts)
417+
chartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts)
417418
if err != nil {
418419
// Any error requires a change in generation,
419420
// which we should be informed about by the watcher
@@ -523,15 +524,8 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
523524
}
524525

525526
// Setup dependency manager
526-
authDir := filepath.Join(tmpDir, "creds")
527-
if err = os.Mkdir(authDir, 0700); err != nil {
528-
return sreconcile.ResultEmpty, &serror.Event{
529-
Err: fmt.Errorf("failed to create temporary directory for dependency credentials: %w", err),
530-
Reason: meta.FailedReason,
531-
}
532-
}
533527
dm := chart.NewDependencyManager(
534-
chart.WithRepositoryCallback(r.namespacedChartRepositoryCallback(ctx, authDir, obj.GetNamespace())),
528+
chart.WithRepositoryCallback(r.namespacedChartRepositoryCallback(ctx, obj.GetNamespace())),
535529
)
536530
defer dm.Clear()
537531

@@ -747,11 +741,11 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1.
747741
}
748742

749743
// namespacedChartRepositoryCallback returns a chart.GetChartRepositoryCallback scoped to the given namespace.
750-
// Credentials for retrieved v1beta1.HelmRepository objects are stored in the given directory.
751744
// The returned callback returns a repository.ChartRepository configured with the retrieved v1beta1.HelmRepository,
752745
// or a shim with defaults if no object could be found.
753-
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, dir, namespace string) chart.GetChartRepositoryCallback {
746+
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, namespace string) chart.GetChartRepositoryCallback {
754747
return func(url string) (*repository.ChartRepository, error) {
748+
var tlsConfig *tls.Config
755749
repo, err := r.resolveDependencyRepository(ctx, url, namespace)
756750
if err != nil {
757751
// Return Kubernetes client errors, but ignore others
@@ -774,13 +768,19 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
774768
if err != nil {
775769
return nil, err
776770
}
777-
opts, err := getter.ClientOptionsFromSecret(dir, *secret)
771+
opts, err := getter.ClientOptionsFromSecret(*secret)
778772
if err != nil {
779773
return nil, err
780774
}
781775
clientOpts = append(clientOpts, opts...)
776+
777+
tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, repo.Spec.URL)
778+
if err != nil {
779+
return nil, fmt.Errorf("failed to create TLS client config for HelmRepository '%s': %w", repo.Name, err)
780+
}
782781
}
783-
chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, clientOpts)
782+
783+
chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, tlsConfig, clientOpts)
784784
if err != nil {
785785
return nil, err
786786
}

‎controllers/helmrepository_controller.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package controllers
1818

1919
import (
2020
"context"
21+
"crypto/tls"
2122
"errors"
2223
"fmt"
2324
"net/url"
@@ -260,6 +261,8 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, obj *so
260261
// If the download is successful, the given artifact pointer is set to a new artifact with the available metadata, and
261262
// the index pointer is set to the newly downloaded index.
262263
func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) {
264+
var tlsConfig *tls.Config
265+
263266
// Configure Helm client to access repository
264267
clientOpts := []helmgetter.Option{
265268
helmgetter.WithTimeout(obj.Spec.Timeout.Duration),
@@ -284,18 +287,8 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
284287
return sreconcile.ResultEmpty, e
285288
}
286289

287-
// Get client options from secret
288-
tmpDir, err := os.MkdirTemp("", fmt.Sprintf("%s-%s-auth-", obj.Name, obj.Namespace))
289-
if err != nil {
290-
return sreconcile.ResultEmpty, &serror.Event{
291-
Err: fmt.Errorf("failed to create temporary directory for credentials: %w", err),
292-
Reason: sourcev1.StorageOperationFailedReason,
293-
}
294-
}
295-
defer os.RemoveAll(tmpDir)
296-
297290
// Construct actual options
298-
opts, err := getter.ClientOptionsFromSecret(tmpDir, secret)
291+
opts, err := getter.ClientOptionsFromSecret(secret)
299292
if err != nil {
300293
e := &serror.Event{
301294
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
306299
return sreconcile.ResultEmpty, e
307300
}
308301
clientOpts = append(clientOpts, opts...)
302+
303+
tlsConfig, err = getter.TLSClientConfigFromSecret(secret, obj.Spec.URL)
304+
if err != nil {
305+
e := &serror.Event{
306+
Err: fmt.Errorf("failed to create TLS client config with secret data: %w", err),
307+
Reason: sourcev1.AuthenticationFailedReason,
308+
}
309+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
310+
// Requeue as content of secret might change
311+
return sreconcile.ResultEmpty, e
312+
}
309313
}
310314

311315
// Construct Helm chart repository with options and download index
312-
newChartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", r.Getters, clientOpts)
316+
newChartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", r.Getters, tlsConfig, clientOpts)
313317
if err != nil {
314318
switch err.(type) {
315319
case *url.Error:

‎controllers/helmrepository_controller_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
366366
},
367367
wantErr: true,
368368
assertConditions: []metav1.Condition{
369-
*conditions.TrueCondition(sourcev1.FetchFailedCondition, meta.FailedReason, "can't create TLS config for client: failed to append certificates from file"),
369+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to create TLS client config with secret data: cannot append certificate into certificate pool: invalid caFile"),
370370
},
371371
},
372372
{
@@ -602,7 +602,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
602602
g.Expect(err).ToNot(HaveOccurred())
603603
g.Expect(cacheFile.Close()).ToNot(HaveOccurred())
604604

605-
chartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", testGetters, nil)
605+
chartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", testGetters, nil, nil)
606606
g.Expect(err).ToNot(HaveOccurred())
607607
chartRepo.CachePath = cachePath
608608

‎go.mod

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ require (
88
cloud.google.com/go/storage v1.16.0
99
github.com/Masterminds/semver/v3 v3.1.1
1010
github.com/ProtonMail/go-crypto v0.0.0-20220113124808-70ae35bab23f
11-
github.com/cyphar/filepath-securejoin v0.2.2
11+
github.com/cyphar/filepath-securejoin v0.2.3
1212
github.com/darkowlzz/controller-check v0.0.0-20220119215126-648356cef22c
1313
github.com/docker/go-units v0.4.0
1414
github.com/elazarl/goproxy v0.0.0-20211114080932-d06c3be7c11b
@@ -36,7 +36,7 @@ require (
3636
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
3737
google.golang.org/api v0.62.0
3838
gotest.tools v2.2.0+incompatible
39-
helm.sh/helm/v3 v3.7.2
39+
helm.sh/helm/v3 v3.8.0
4040
k8s.io/api v0.23.3
4141
k8s.io/apimachinery v0.23.3
4242
k8s.io/client-go v0.23.3
@@ -46,16 +46,21 @@ require (
4646
sigs.k8s.io/yaml v1.3.0
4747
)
4848

49+
// Temporary fork of Helm v3.8.0 with patch applied from
50+
// https://github.com/helm/helm/pull/10568 to solve
51+
// https://github.com/fluxcd/source-controller/issues/578.
52+
// TODO: Remove once Helm version with patch is released.
53+
replace helm.sh/helm/v3 v3.8.0 => github.com/hiddeco/helm/v3 v3.8.1-0.20220223115530-53489c50c9e7
54+
4955
require (
5056
cloud.google.com/go v0.99.0 // indirect
5157
github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
52-
github.com/BurntSushi/toml v0.3.1 // indirect
58+
github.com/BurntSushi/toml v0.4.1 // indirect
5359
github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd // indirect
5460
github.com/Masterminds/goutils v1.1.1 // indirect
5561
github.com/Masterminds/sprig/v3 v3.2.2 // indirect
5662
github.com/Masterminds/squirrel v1.5.2 // indirect
5763
github.com/Microsoft/go-winio v0.5.2 // indirect
58-
github.com/Microsoft/hcsshim v0.8.23 // indirect
5964
github.com/PuerkitoBio/purell v1.1.1 // indirect
6065
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect
6166
github.com/acomagu/bufpipe v1.0.3 // indirect
@@ -66,13 +71,12 @@ require (
6671
github.com/bugsnag/panicwrap v1.3.4 // indirect
6772
github.com/cespare/xxhash/v2 v2.1.2 // indirect
6873
github.com/chai2010/gettext-go v0.0.0-20160711120539-c6fed771bfd5 // indirect
69-
github.com/containerd/containerd v1.5.7 // indirect
70-
github.com/containerd/continuity v0.1.0 // indirect
74+
github.com/containerd/containerd v1.5.9 // indirect
7175
github.com/davecgh/go-spew v1.1.1 // indirect
72-
github.com/docker/cli v20.10.7+incompatible // indirect
76+
github.com/docker/cli v20.10.11+incompatible // indirect
7377
github.com/docker/distribution v2.8.0+incompatible // indirect
74-
github.com/docker/docker v17.12.0-ce-rc1.0.20200618181300-9dc6525e6118+incompatible // indirect
75-
github.com/docker/docker-credential-helpers v0.6.3 // indirect
78+
github.com/docker/docker v20.10.12+incompatible // indirect
79+
github.com/docker/docker-credential-helpers v0.6.4 // indirect
7680
github.com/docker/go-connections v0.4.0 // indirect
7781
github.com/docker/go-metrics v0.0.1 // indirect
7882
github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect
@@ -109,17 +113,17 @@ require (
109113
github.com/imdario/mergo v0.3.12 // indirect
110114
github.com/inconshreveable/mousetrap v1.0.0 // indirect
111115
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
112-
github.com/jmoiron/sqlx v1.3.1 // indirect
116+
github.com/jmoiron/sqlx v1.3.4 // indirect
113117
github.com/josharian/intern v1.0.0 // indirect
114118
github.com/json-iterator/go v1.1.12 // indirect
115119
github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 // indirect
116120
github.com/kevinburke/ssh_config v1.1.0 // indirect
117-
github.com/klauspost/compress v1.13.5 // indirect
121+
github.com/klauspost/compress v1.13.6 // indirect
118122
github.com/klauspost/cpuid v1.3.1 // indirect
119123
github.com/kylelemons/godebug v1.1.0 // indirect
120124
github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect
121125
github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect
122-
github.com/lib/pq v1.10.0 // indirect
126+
github.com/lib/pq v1.10.4 // indirect
123127
github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect
124128
github.com/mailru/easyjson v0.7.6 // indirect
125129
github.com/mattn/go-colorable v0.1.12 // indirect
@@ -128,10 +132,10 @@ require (
128132
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
129133
github.com/minio/md5-simd v1.1.0 // indirect
130134
github.com/minio/sha256-simd v0.1.1 // indirect
131-
github.com/mitchellh/copystructure v1.1.1 // indirect
135+
github.com/mitchellh/copystructure v1.2.0 // indirect
132136
github.com/mitchellh/go-homedir v1.1.0 // indirect
133137
github.com/mitchellh/go-wordwrap v1.0.0 // indirect
134-
github.com/mitchellh/reflectwalk v1.0.1 // indirect
138+
github.com/mitchellh/reflectwalk v1.0.2 // indirect
135139
github.com/moby/locker v1.0.1 // indirect
136140
github.com/moby/spdystream v0.2.0 // indirect
137141
github.com/moby/term v0.0.0-20210610120745-9d4ed1856297 // indirect
@@ -141,7 +145,6 @@ require (
141145
github.com/morikuni/aec v1.0.0 // indirect
142146
github.com/opencontainers/go-digest v1.0.0 // indirect
143147
github.com/opencontainers/image-spec v1.0.2 // indirect
144-
github.com/opencontainers/runc v1.0.2 // indirect
145148
github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
146149
github.com/pkg/errors v0.9.1 // indirect
147150
github.com/pmezard/go-difflib v1.0.0 // indirect
@@ -181,8 +184,8 @@ require (
181184
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
182185
gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect
183186
google.golang.org/appengine v1.6.7 // indirect
184-
google.golang.org/genproto v0.0.0-20211208223120-3a66f561d7aa // indirect
185-
google.golang.org/grpc v1.42.0 // indirect
187+
google.golang.org/genproto v0.0.0-20220107163113-42d7afdf6368 // indirect
188+
google.golang.org/grpc v1.43.0 // indirect
186189
google.golang.org/protobuf v1.27.1 // indirect
187190
gopkg.in/gorp.v1 v1.7.2 // indirect
188191
gopkg.in/inf.v0 v0.9.1 // indirect
@@ -197,7 +200,7 @@ require (
197200
k8s.io/klog/v2 v2.40.1 // indirect
198201
k8s.io/kube-openapi v0.0.0-20220124234850-424119656bbf // indirect
199202
k8s.io/kubectl v0.23.2 // indirect
200-
oras.land/oras-go v0.4.0 // indirect
203+
oras.land/oras-go v1.1.0 // indirect
201204
sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect
202205
sigs.k8s.io/kustomize/api v0.10.1 // indirect
203206
sigs.k8s.io/kustomize/kyaml v0.13.0 // indirect

0 commit comments

Comments
 (0)