helmrepo: adopt Kubernetes TLS secrets for .spec.certSecretRef

Adopt Kubernetes TLS secrets API to check for TLS data in the Secret
referred to by `.spec.certSecretRef`, i.e. check for keys `tls.crt` and
`tls.key` for the certificate and private key. Use `ca.crt` for the CA
certificate.

Signed-off-by: Sanskar Jaiswal <[email protected]>

Author: aryan9600

api/v1beta2/helmrepository_types.go

@@ -56,10 +56,21 @@ type HelmRepositorySpec struct {
 	// +optional
 	SecretRef *meta.LocalObjectReference `json:"secretRef,omitempty"`
 
-	// CertSecretRef specifies the Secret containing the TLS authentication
-	// data. The secret must contain a 'certFile' and 'keyFile', and/or 'caFile'
-	// fields. It takes precedence over the values specified in the Secret
-	// referred to by `.spec.secretRef`.
+	// CertSecretRef can be given the name of a Secret containing
+	// either or both of
+	//
+	// - a PEM-encoded client certificate (`tls.crt`) and private
+	// key (`tls.key`);
+	// - a PEM-encoded CA certificate (`ca.crt`)
+	//
+	// and whichever are supplied, will be used for connecting to the
+	// registry. The client cert and key are useful if you are
+	// authenticating with a certificate; the CA cert is useful if
+	// you are using a self-signed server certificate. The Secret must
+	// be of type `Opaque` or `kubernetes.io/tls`.
+	//
+	// It takes precedence over the values specified in the Secret referred
+	// to by `.spec.secretRef`.
 	// +optional
 	CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"`
 

config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml

@@ -297,10 +297,15 @@ spec:
                 - namespaceSelectors
                 type: object
               certSecretRef:
-                description: CertSecretRef specifies the Secret containing the TLS
-                  authentication data. The secret must contain a 'certFile' and 'keyFile',
-                  and/or 'caFile' fields. It takes precedence over the values specified
-                  in the Secret referred to by `.spec.secretRef`.
+                description: "CertSecretRef can be given the name of a Secret containing
+                  either or both of \n - a PEM-encoded client certificate (`tls.crt`)
+                  and private key (`tls.key`); - a PEM-encoded CA certificate (`ca.crt`)
+                  \n and whichever are supplied, will be used for connecting to the
+                  registry. The client cert and key are useful if you are authenticating
+                  with a certificate; the CA cert is useful if you are using a self-signed
+                  server certificate. The Secret must be of type `Opaque` or `kubernetes.io/tls`.
+                  \n It takes precedence over the values specified in the Secret referred
+                  to by `.spec.secretRef`."
                 properties:
                   name:
                     description: Name of the referent.

docs/api/v1beta2/source.md

@@ -811,10 +811,20 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference
 </td>
 <td>
 <em>(Optional)</em>
-<p>CertSecretRef specifies the Secret containing the TLS authentication
-data. The secret must contain a &lsquo;certFile&rsquo; and &lsquo;keyFile&rsquo;, and/or &lsquo;caFile&rsquo;
-fields. It takes precedence over the values specified in the Secret
-referred to by <code>.spec.secretRef</code>.</p>
+<p>CertSecretRef can be given the name of a Secret containing
+either or both of</p>
+<ul>
+<li>a PEM-encoded client certificate (<code>tls.crt</code>) and private
+key (<code>tls.key</code>);</li>
+<li>a PEM-encoded CA certificate (<code>ca.crt</code>)</li>
+</ul>
+<p>and whichever are supplied, will be used for connecting to the
+registry. The client cert and key are useful if you are
+authenticating with a certificate; the CA cert is useful if
+you are using a self-signed server certificate. The Secret must
+be of type <code>Opaque</code> or <code>kubernetes.io/tls</code>.</p>
+<p>It takes precedence over the values specified in the Secret referred
+to by <code>.spec.secretRef</code>.</p>
 </td>
 </tr>
 <tr>
@@ -2503,10 +2513,20 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference
 </td>
 <td>
 <em>(Optional)</em>
-<p>CertSecretRef specifies the Secret containing the TLS authentication
-data. The secret must contain a &lsquo;certFile&rsquo; and &lsquo;keyFile&rsquo;, and/or &lsquo;caFile&rsquo;
-fields. It takes precedence over the values specified in the Secret
-referred to by <code>.spec.secretRef</code>.</p>
+<p>CertSecretRef can be given the name of a Secret containing
+either or both of</p>
+<ul>
+<li>a PEM-encoded client certificate (<code>tls.crt</code>) and private
+key (<code>tls.key</code>);</li>
+<li>a PEM-encoded CA certificate (<code>ca.crt</code>)</li>
+</ul>
+<p>and whichever are supplied, will be used for connecting to the
+registry. The client cert and key are useful if you are
+authenticating with a certificate; the CA cert is useful if
+you are using a self-signed server certificate. The Secret must
+be of type <code>Opaque</code> or <code>kubernetes.io/tls</code>.</p>
+<p>It takes precedence over the values specified in the Secret referred
+to by <code>.spec.secretRef</code>.</p>
 </td>
 </tr>
 <tr>

docs/spec/v1beta2/helmrepositories.md

@@ -467,32 +467,33 @@ flux create secret oci ghcr-auth \
   --password=${GITHUB_PAT}
 ```
 
-**Note:** Support for specifying TLS authentication data using this API has been
+**Warning:** Support for specifying TLS authentication data using this API has been
 deprecated. Please use [`.spec.certSecretRef`](#cert-secret-reference) instead.
 If the controller uses the secret specfied by this field to configure TLS, then
 a deprecation warning will be logged.
 
 ### Cert secret reference
 
-`.spec.certSecretRef.name` is an optional field to specify a secret containing TLS
-certificate data. The secret can contain the following keys:
+`.spec.certSecretRef.name` is an optional field to specify a secret containing
+TLS certificate data. The secret can contain the following keys:
 
-* `certFile` and `keyFile`, to specify the client certificate and private key used for
-TLS client authentication. These must be used in conjunction, i.e. specifying one without
-the other will lead to an error.
-* `caFile`, to specify the CA certificate used to verify the server, which is required
-if the server is using a self-signed certificate.
+* `tls.crt` and `tls.key`, to specify the client certificate and private key used
+for TLS client authentication. These must be used in conjunction, i.e.
+specifying one without the other will lead to an error.
+* `ca.crt`, to specify the CA certificate used to verify the server, which is
+required if the server is using a self-signed certificate.
 
-If the server is using a self-signed certificate and has TLS client authentication enabled,
-all three values are required.
+If the server is using a self-signed certificate and has TLS client
+authentication enabled, all three values are required.
 
-All the files in the secret are expected to be [PEM-encoded][pem-encoding]. Assuming you have
-three files; `client.key`, `client.crt` and `ca.crt` for the client private key, client
-certificate and the CA certificate respectively, you can generate the required secret using
-the `flux creat secret helm` command:
+The Secret should be of type `Opaque` or `kubernetes.io/tls`. All the files in
+the Secret are expected to be [PEM-encoded][pem-encoding]. Assuming you have
+three files; `client.key`, `client.crt` and `ca.crt` for the client private key,
+client certificate and the CA certificate respectively, you can generate the
+required Secret using the `flux create secret tls` command:
 
 ```sh
-flux create secret helm tls --key-file=client.key --cert-file=client.crt --ca-file=ca.crt
+flux create secret tls --tls-key-file=client.key --tls-crt-file=client.crt --ca-crt-file=ca.crt
 ```
 
 Example usage:
@@ -515,11 +516,12 @@ kind: Secret
 metadata:
   name: example-tls
   namespace: default
+type: kubernetes.io/tls # or Opaque
 data:
-  certFile: <BASE64>
-  keyFile: <BASE64>
+  tls.crt: <BASE64>
+  tls.key: <BASE64>
   # NOTE: Can be supplied without the above values
-  caFile: <BASE64>
+  ca.crt: <BASE64>
 ```
 
 ### Pass credentials

internal/controller/helmchart_controller_test.go

@@ -2248,7 +2248,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 		registryOpts     registryOptions
 		secretOpts       secretOptions
 		secret           *corev1.Secret
-		certsecret       *corev1.Secret
+		certSecret       *corev1.Secret
 		insecure         bool
 		provider         string
 		providerImg      string
@@ -2363,16 +2363,16 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 				Type: corev1.SecretTypeDockerConfigJson,
 				Data: map[string][]byte{},
 			},
-			certsecret: &corev1.Secret{
+			certSecret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "certs-secretref",
 				},
 				Data: map[string][]byte{
-					"caFile": []byte("invalid caFile"),
+					"ca.crt": []byte("invalid caFile"),
 				},
 			},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to construct Helm client's TLS config: cannot append certificate into certificate pool: invalid caFile"),
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to construct Helm client's TLS config: cannot append certificate into certificate pool: invalid CA certificate"),
 			},
 		},
 		{
@@ -2393,14 +2393,14 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 				Type: corev1.SecretTypeDockerConfigJson,
 				Data: map[string][]byte{},
 			},
-			certsecret: &corev1.Secret{
+			certSecret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "certs-secretref",
 				},
 				Data: map[string][]byte{
-					"caFile":   tlsCA,
-					"certFile": clientPublicKey,
-					"keyFile":  clientPrivateKey,
+					"ca.crt":  tlsCA,
+					"tls.crt": clientPublicKey,
+					"tls.key": clientPrivateKey,
 				},
 			},
 			assertConditions: []metav1.Condition{
@@ -2472,11 +2472,11 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 				clientBuilder.WithObjects(tt.secret)
 			}
 
-			if tt.certsecret != nil {
+			if tt.certSecret != nil {
 				repo.Spec.CertSecretRef = &meta.LocalObjectReference{
-					Name: tt.certsecret.Name,
+					Name: tt.certSecret.Name,
 				}
-				clientBuilder.WithObjects(tt.certsecret)
+				clientBuilder.WithObjects(tt.certSecret)
 			}
 
 			clientBuilder.WithObjects(repo)

internal/controller/helmrepository_controller_oci_test.go

@@ -325,12 +325,12 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
 					Name: "certs-secretref",
 				},
 				Data: map[string][]byte{
-					"caFile": []byte("invalid caFile"),
+					"ca.crt": []byte("invalid caFile"),
 				},
 			},
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation 0 -> 1"),
-				*conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid caFile"),
+				*conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid CA certificate"),
 			},
 		},
 		{
@@ -356,9 +356,9 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
 					Name: "certs-secretref",
 				},
 				Data: map[string][]byte{
-					"caFile":   tlsCA,
-					"certFile": clientPublicKey,
-					"keyFile":  clientPrivateKey,
+					"ca.crt":  tlsCA,
+					"tls.crt": clientPublicKey,
+					"tls.key": clientPrivateKey,
 				},
 			},
 			assertConditions: []metav1.Condition{

internal/controller/helmrepository_controller_test.go

@@ -56,6 +56,7 @@ import (
 	"github.com/fluxcd/source-controller/internal/helm/repository"
 	sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
 	"github.com/fluxcd/source-controller/internal/reconcile/summarize"
+	stls "github.com/fluxcd/source-controller/internal/tls"
 )
 
 func TestHelmRepositoryReconciler_deleteBeforeFinalizer(t *testing.T) {
@@ -434,16 +435,76 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 					Name: "ca-file",
 				},
 				Data: map[string][]byte{
-					"caFile": tlsCA,
+					"ca.crt": tlsCA,
+				},
+			},
+			beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
+				obj.Spec.CertSecretRef = &meta.LocalObjectReference{Name: "ca-file"}
+			},
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
+			},
+		},
+		{
+			name:     "HTTPS with certSecretRef makes ArtifactOutdated=True",
+			protocol: "https",
+			server: options{
+				publicKey:  tlsPublicKey,
+				privateKey: tlsPrivateKey,
+				ca:         tlsCA,
+			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "ca-file",
+				},
+				Data: map[string][]byte{
+					"ca.crt": tlsCA,
 				},
 			},
 			beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
 				obj.Spec.CertSecretRef = &meta.LocalObjectReference{Name: "ca-file"}
 			},
+			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
 				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
 			},
+			afterFunc: func(t *WithT, obj *helmv1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
+				t.Expect(chartRepo.Path).ToNot(BeEmpty())
+				t.Expect(chartRepo.Index).ToNot(BeNil())
+				t.Expect(artifact.Revision).ToNot(BeEmpty())
+			},
+		},
+		{
+			name:     "HTTPS with secretRef and caFile key makes ArtifactOutdated=True",
+			protocol: "https",
+			server: options{
+				publicKey:  tlsPublicKey,
+				privateKey: tlsPrivateKey,
+				ca:         tlsCA,
+			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "ca-file",
+				},
+				Data: map[string][]byte{
+					"caFile": tlsCA,
+				},
+			},
+			beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
+				obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "ca-file"}
+			},
+			want: sreconcile.ResultSuccess,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
+			},
+			afterFunc: func(t *WithT, obj *helmv1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
+				t.Expect(chartRepo.Path).ToNot(BeEmpty())
+				t.Expect(chartRepo.Index).ToNot(BeNil())
+				t.Expect(artifact.Revision).ToNot(BeEmpty())
+			},
 		},
 		{
 			name:     "HTTP without secretRef makes ArtifactOutdated=True",
@@ -502,7 +563,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 					Name: "invalid-ca",
 				},
 				Data: map[string][]byte{
-					"caFile": []byte("invalid"),
+					"ca.crt": []byte("invalid"),
 				},
 			},
 			beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
@@ -512,7 +573,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			},
 			wantErr: true,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid caFile"),
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid CA certificate"),
 				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
 				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
@@ -769,10 +830,16 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				if tt.url != "" {
 					repoURL = tt.url
 				}
-				tlsConf, _, serr = getter.TLSClientConfigFromSecret(*secret, repoURL)
+				tlsConf, _, serr = stls.KubeTLSClientConfigFromSecret(*secret, repoURL)
 				if serr != nil {
 					validSecret = false
 				}
+				if tlsConf == nil {
+					tlsConf, _, serr = stls.TLSClientConfigFromSecret(*secret, repoURL)
+					if serr != nil {
+						validSecret = false
+					}
+				}
 				newChartRepo, err = repository.NewChartRepository(obj.Spec.URL, "", testGetters, tlsConf, getterOpts...)
 			} else {
 				newChartRepo, err = repository.NewChartRepository(obj.Spec.URL, "", testGetters, nil)