From 7a139647a20d6d443561cf69c1faa3f75f321a18 Mon Sep 17 00:00:00 2001
From: Max Jonas Werner <max@e13.dev>
Date: Tue, 20 Sep 2022 10:31:09 +0200
Subject: [PATCH] add custom CA certificates to system certificates

When a custom CA certificate is provided in a Secret's `caCert` field
referenced in `HelmRelease.spec.secretRef` then that CA cert is now
added to the list of system certificates instead of it replacing the
system certificates. This makes HelmRepositories work in mixed
environments where charts are pulled from both, a public repository
and a private repository (e.g. through a chart dependency).

The test that is added as part of this change will fail without the
change and passes with it.

closes #866
closes fluxcd/helm-controller#519

Signed-off-by: Max Jonas Werner <max@e13.dev>
---
 controllers/helmrepository_controller_test.go | 31 ++++++++++++++++++-
 internal/helm/getter/getter.go                |  5 ++-
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go
index 2e8df4873..7b5525039 100644
--- a/controllers/helmrepository_controller_test.go
+++ b/controllers/helmrepository_controller_test.go
@@ -290,6 +290,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 		name             string
 		protocol         string
 		server           options
+		url              string
 		secret           *corev1.Secret
 		beforeFunc       func(t *WithT, obj *sourcev1.HelmRepository, checksum string)
 		afterFunc        func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository)
@@ -297,6 +298,24 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 		wantErr          bool
 		assertConditions []metav1.Condition
 	}{
+		{
+			name:     "HTTPS with secretRef pointing to CA cert but public repo URL succeeds",
+			protocol: "http",
+			url:      "https://stefanprodan.github.io/podinfo",
+			want:     sreconcile.ResultSuccess,
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "ca-file",
+				},
+				Data: map[string][]byte{
+					"caFile": tlsCA,
+				},
+			},
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
+			},
+		},
 		{
 			name:     "HTTP without secretRef makes ArtifactOutdated=True",
 			protocol: "http",
@@ -565,10 +584,16 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				server.Start()
 				defer server.Stop()
 				obj.Spec.URL = server.URL()
+				if tt.url != "" {
+					obj.Spec.URL = tt.url
+				}
 			case "https":
 				g.Expect(server.StartTLS(tt.server.publicKey, tt.server.privateKey, tt.server.ca, "example.com")).To(Succeed())
 				defer server.Stop()
 				obj.Spec.URL = server.URL()
+				if tt.url != "" {
+					obj.Spec.URL = tt.url
+				}
 			default:
 				t.Fatalf("unsupported protocol %q", tt.protocol)
 			}
@@ -596,7 +621,11 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 					validSecret = false
 				}
 				clientOpts = append(clientOpts, cOpts...)
-				tOpts, serr = getter.TLSClientConfigFromSecret(*secret, server.URL())
+				repoURL := server.URL()
+				if tt.url != "" {
+					repoURL = tt.url
+				}
+				tOpts, serr = getter.TLSClientConfigFromSecret(*secret, repoURL)
 				if serr != nil {
 					validSecret = false
 				}
diff --git a/internal/helm/getter/getter.go b/internal/helm/getter/getter.go
index 0b45f0101..25214372f 100644
--- a/internal/helm/getter/getter.go
+++ b/internal/helm/getter/getter.go
@@ -81,7 +81,10 @@ func TLSClientConfigFromSecret(secret corev1.Secret, repositoryUrl string) (*tls
 	}
 
 	if len(caBytes) > 0 {
-		cp := x509.NewCertPool()
+		cp, err := x509.SystemCertPool()
+		if err != nil {
+			return nil, fmt.Errorf("cannot retrieve system certificate pool: %w", err)
+		}
 		if !cp.AppendCertsFromPEM(caBytes) {
 			return nil, fmt.Errorf("cannot append certificate into certificate pool: invalid caFile")
 		}