From e3f49085ccb7b8f35e988554392d5f7ec5e066ad Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Mon, 25 Nov 2019 12:38:57 +0100 Subject: [PATCH 1/5] chart_downloader: add test to verify that http opts are used correctly. (#7055) Signed-off-by: Andreas Stenius --- pkg/downloader/chart_downloader_test.go | 61 +++++++++++++++++++ pkg/downloader/testdata/repositories.yaml | 7 ++- .../repository/testing-ca-file-index.yaml | 14 +++++ 3 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 pkg/downloader/testdata/repository/testing-ca-file-index.yaml diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 80249e240..abecf81eb 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -80,6 +80,67 @@ func TestResolveChartRef(t *testing.T) { } } +func TestResolveChartOpts(t *testing.T) { + tests := []struct { + name, ref, version string + expect []getter.Option + }{ + { + name: "repo with CA-file", + ref: "testing-ca-file/foo", + expect: []getter.Option{ + getter.WithURL("https://example.com/foo-1.2.3.tgz"), + getter.WithTLSClientConfig("cert", "key", "ca"), + }, + }, + } + + c := ChartDownloader{ + Out: os.Stderr, + RepositoryConfig: repoConfig, + RepositoryCache: repoCache, + Getters: getter.All(&cli.EnvSettings{ + RepositoryConfig: repoConfig, + RepositoryCache: repoCache, + }), + } + + // snapshot options + snapshot_opts := c.Options + + for _, tt := range tests { + // reset chart downloader options for each test case + c.Options = snapshot_opts + + expect, err := getter.NewHTTPGetter(tt.expect...) + if err != nil { + t.Errorf("%s: failed to setup http client: %s", tt.name, err) + continue + } + + u, err := c.ResolveChartVersion(tt.ref, tt.version) + if err != nil { + t.Errorf("%s: failed with error %s", tt.name, err) + continue + } + + got, err := getter.NewHTTPGetter( + append( + c.Options, + getter.WithURL(u.String()), + )... + ) + if err != nil { + t.Errorf("%s: failed to create http client: %s", tt.name, err) + continue + } + + if got != expect { + t.Errorf("%s: expected %s, got %s", tt.name, expect, got) + } + } +} + func TestVerifyChart(t *testing.T) { v, err := VerifyChart("testdata/signtest-0.1.0.tgz", "testdata/helm-test-key.pub") if err != nil { diff --git a/pkg/downloader/testdata/repositories.yaml b/pkg/downloader/testdata/repositories.yaml index 374d95c8a..430865269 100644 --- a/pkg/downloader/testdata/repositories.yaml +++ b/pkg/downloader/testdata/repositories.yaml @@ -15,4 +15,9 @@ repositories: - name: testing-relative url: "http://example.com/helm" - name: testing-relative-trailing-slash - url: "http://example.com/helm/" \ No newline at end of file + url: "http://example.com/helm/" + - name: testing-ca-file + url: "https://example.com" + certFile: "cert" + keyFile: "key" + caFile: "ca" diff --git a/pkg/downloader/testdata/repository/testing-ca-file-index.yaml b/pkg/downloader/testdata/repository/testing-ca-file-index.yaml new file mode 100644 index 000000000..17cdde1c6 --- /dev/null +++ b/pkg/downloader/testdata/repository/testing-ca-file-index.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +entries: + foo: + - name: foo + description: Foo Chart + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://github.com/helm/charts + urls: + - https://example.com/foo-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d From d6c13616fa7beff70931255cce62ab336ce1531d Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Mon, 25 Nov 2019 13:10:15 +0100 Subject: [PATCH 2/5] chart_downloader: add TLS client config to options from repo config. (#7055) Signed-off-by: Andreas Stenius --- pkg/downloader/chart_downloader.go | 2 ++ pkg/downloader/chart_downloader_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 8e251bc89..9b9293455 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -214,6 +214,8 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er c.Options = append(c.Options, getter.WithBasicAuth(r.Config.Username, r.Config.Password)) } + c.Options = append(c.Options, getter.WithTLSClientConfig(r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile)) + // Next, we need to load the index, and actually look up the chart. idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name)) i, err := repo.LoadIndexFile(idxFile) diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index abecf81eb..71a56d482 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -135,7 +135,7 @@ func TestResolveChartOpts(t *testing.T) { continue } - if got != expect { + if *(got.(*getter.HTTPGetter)) != *(expect.(*getter.HTTPGetter)) { t.Errorf("%s: expected %s, got %s", tt.name, expect, got) } } From d3ad6f9c78113fd4c577b31e41277796220e4c0f Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Mon, 25 Nov 2019 13:23:08 +0100 Subject: [PATCH 3/5] cli/pull: pass TLS config to chart downloader from flags. (#7055) Signed-off-by: Andreas Stenius --- pkg/action/pull.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index aaf63861e..b0a3d2598 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -63,6 +63,7 @@ func (p *Pull) Run(chartRef string) (string, error) { Getters: getter.All(p.Settings), Options: []getter.Option{ getter.WithBasicAuth(p.Username, p.Password), + getter.WithTLSClientConfig(p.CertFile, p.KeyFile, p.CaFile), }, RepositoryConfig: p.Settings.RepositoryConfig, RepositoryCache: p.Settings.RepositoryCache, From 0f0aa6b43261b3f4b95fd7ee14107de2630dd446 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Mon, 25 Nov 2019 13:37:20 +0100 Subject: [PATCH 4/5] chart_downloader: avoid overriding TLS options from command flags when not setup in repo config. Signed-off-by: Andreas Stenius --- pkg/downloader/chart_downloader.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 9b9293455..f3d4321c5 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -214,7 +214,9 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er c.Options = append(c.Options, getter.WithBasicAuth(r.Config.Username, r.Config.Password)) } - c.Options = append(c.Options, getter.WithTLSClientConfig(r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile)) + if r.Config.CertFile != "" || r.Config.KeyFile != "" || r.Config.CAFile != "" { + c.Options = append(c.Options, getter.WithTLSClientConfig(r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile)) + } // Next, we need to load the index, and actually look up the chart. idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name)) From 4c4328398ecc4f6eef07524f4bcddaca153b4570 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Mon, 25 Nov 2019 14:17:24 +0100 Subject: [PATCH 5/5] chart_downloader: fix lint issue. Signed-off-by: Andreas Stenius --- pkg/downloader/chart_downloader_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 71a56d482..e0692c8c8 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -87,7 +87,7 @@ func TestResolveChartOpts(t *testing.T) { }{ { name: "repo with CA-file", - ref: "testing-ca-file/foo", + ref: "testing-ca-file/foo", expect: []getter.Option{ getter.WithURL("https://example.com/foo-1.2.3.tgz"), getter.WithTLSClientConfig("cert", "key", "ca"), @@ -106,11 +106,11 @@ func TestResolveChartOpts(t *testing.T) { } // snapshot options - snapshot_opts := c.Options + snapshotOpts := c.Options for _, tt := range tests { // reset chart downloader options for each test case - c.Options = snapshot_opts + c.Options = snapshotOpts expect, err := getter.NewHTTPGetter(tt.expect...) if err != nil { @@ -128,7 +128,7 @@ func TestResolveChartOpts(t *testing.T) { append( c.Options, getter.WithURL(u.String()), - )... + )..., ) if err != nil { t.Errorf("%s: failed to create http client: %s", tt.name, err)