helmrepo: add .spec.certSecretReffor specifying TLS auth data

Add `.spec.certSecretRef` for specifying TLS authentication data using the `certFile`, `keyFile` and `caFile` keys. Deprecate the usage of these keys in the secret specified by `.spec.secretRef`. Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
fluxcd · Jul 7, 2023 · 92253fc · 92253fc
1 parent d141456
commit 92253fc
Show file tree

Hide file tree

Showing 6 changed files with 203 additions and 48 deletions.
diff --git a/api/v1beta2/helmrepository_types.go b/api/v1beta2/helmrepository_types.go
@@ -51,11 +51,18 @@ type HelmRepositorySpec struct {
 	// for the HelmRepository.
 	// For HTTP/S basic auth the secret must contain 'username' and 'password'
 	// fields.
-	// For TLS the secret must contain a 'certFile' and 'keyFile', and/or
-	// 'caFile' fields.
+	// Support for TLS auth using the 'certFile' and 'keyFile', and/or 'caFile'
+	// keys has been DEPRECATED. Please use `.spec.certSecretRef` instead.
 	// +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`.
+	// +optional
+	CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"`
+
 	// PassCredentials allows the credentials from the SecretRef to be passed
 	// on to a host that does not match the host as defined in URL.
 	// This may be required if the host of the advertised chart URLs in the

diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml
@@ -296,6 +296,18 @@ spec:
                 required:
                 - 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`.
+                properties:
+                  name:
+                    description: Name of the referent.
+                    type: string
+                required:
+                - name
+                type: object
               interval:
                 description: Interval at which to check the URL for updates.
                 pattern: ^([0-9]+(\.[0-9]+)?(ms|s|m|h))+$
@@ -323,8 +335,9 @@ spec:
               secretRef:
                 description: SecretRef specifies the Secret containing authentication
                   credentials for the HelmRepository. For HTTP/S basic auth the secret
-                  must contain 'username' and 'password' fields. For TLS the secret
-                  must contain a 'certFile' and 'keyFile', and/or 'caFile' fields.
+                  must contain 'username' and 'password' fields. Support for TLS auth
+                  using the 'certFile' and 'keyFile', and/or 'caFile' keys has been
+                  DEPRECATED. Please use `.spec.certSecretRef` instead.
                 properties:
                   name:
                     description: Name of the referent.

diff --git a/docs/api/v1beta2/source.md b/docs/api/v1beta2/source.md
@@ -792,8 +792,25 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference
 for the HelmRepository.
 For HTTP/S basic auth the secret must contain &lsquo;username&rsquo; and &lsquo;password&rsquo;
 fields.
-For TLS the secret must contain a &lsquo;certFile&rsquo; and &lsquo;keyFile&rsquo;, and/or
-&lsquo;caFile&rsquo; fields.</p>
+Support for TLS auth using the &lsquo;certFile&rsquo; and &lsquo;keyFile&rsquo;, and/or &lsquo;caFile&rsquo;
+keys has been DEPRECATED. Please use <code>.spec.certSecretRef</code> instead.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>certSecretRef</code><br>
+<em>
+<a href="https://pkg.go.dev/github.com/fluxcd/pkg/apis/meta#LocalObjectReference">
+github.com/fluxcd/pkg/apis/meta.LocalObjectReference
+</a>
+</em>
+</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>
 </td>
 </tr>
 <tr>
@@ -2459,8 +2476,25 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference
 for the HelmRepository.
 For HTTP/S basic auth the secret must contain &lsquo;username&rsquo; and &lsquo;password&rsquo;
 fields.
-For TLS the secret must contain a &lsquo;certFile&rsquo; and &lsquo;keyFile&rsquo;, and/or
-&lsquo;caFile&rsquo; fields.</p>
+Support for TLS auth using the &lsquo;certFile&rsquo; and &lsquo;keyFile&rsquo;, and/or &lsquo;caFile&rsquo;
+keys has been DEPRECATED. Please use <code>.spec.certSecretRef</code> instead.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>certSecretRef</code><br>
+<em>
+<a href="https://pkg.go.dev/github.com/fluxcd/pkg/apis/meta#LocalObjectReference">
+github.com/fluxcd/pkg/apis/meta.LocalObjectReference
+</a>
+</em>
+</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>
 </td>
 </tr>
 <tr>

diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go
@@ -380,6 +380,18 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, sp *pat
 	return sreconcile.ResultSuccess, nil
 }
 
+func (r *HelmRepositoryReconciler) getSecret(ctx context.Context, name, namespace string) (*corev1.Secret, error) {
+	key := types.NamespacedName{
+		Namespace: namespace,
+		Name:      name,
+	}
+	var secret corev1.Secret
+	if err := r.Get(ctx, key, &secret); err != nil {
+		return nil, err
+	}
+	return &secret, nil
+}
+
 // reconcileSource attempts to fetch the Helm repository index using the
 // specified configuration on the v1beta2.HelmRepository object.
 //
@@ -399,25 +411,52 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
 		helmgetter.WithPassCredentialsAll(obj.Spec.PassCredentials),
 	}
 
-	// Configure any authentication related options
-	if obj.Spec.SecretRef != nil {
-		// Attempt to retrieve secret
-		name := types.NamespacedName{
-			Namespace: obj.GetNamespace(),
-			Name:      obj.Spec.SecretRef.Name,
-		}
-		var secret corev1.Secret
-		if err := r.Client.Get(ctx, name, &secret); err != nil {
+	getTLSConfig := func(secret *corev1.Secret) error {
+		var err error
+		tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, obj.Spec.URL)
+		if err != nil {
 			e := &serror.Event{
-				Err:    fmt.Errorf("failed to get secret '%s': %w", name.String(), err),
+				Err:    fmt.Errorf("failed to create TLS client config with secret data: %w", err),
 				Reason: sourcev1.AuthenticationFailedReason,
 			}
 			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
-			return sreconcile.ResultEmpty, e
+			return e
+		}
+		return nil
+	}
+
+	// Check certSecretRef first as it takes precedence over secretRef for TLS.
+	if obj.Spec.CertSecretRef != nil {
+		secret, err := r.getSecret(ctx, obj.Spec.CertSecretRef.Name, obj.GetNamespace())
+		if err != nil {
+			e := serror.NewGeneric(
+				fmt.Errorf("failed to get TLS authentication secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.CertSecretRef.Name, err),
+				sourcev1.AuthenticationFailedReason,
+			)
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
+			return sreconcile.ResultEmpty, err
+		}
+
+		if err := getTLSConfig(secret); err != nil {
+			// Requeue as content of secret might change
+			return sreconcile.ResultEmpty, err
+		}
+	}
+
+	// Configure any authentication related options
+	if obj.Spec.SecretRef != nil {
+		secret, err := r.getSecret(ctx, obj.Spec.SecretRef.Name, obj.GetNamespace())
+		if err != nil {
+			e := serror.NewGeneric(
+				fmt.Errorf("failed to get authentication secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.SecretRef.Name, err),
+				sourcev1.AuthenticationFailedReason,
+			)
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
+			return sreconcile.ResultEmpty, err
 		}
 
 		// Construct actual options
-		opts, err := getter.ClientOptionsFromSecret(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),
@@ -429,15 +468,17 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
 		}
 		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,
+		if tlsConfig == nil {
+			if err := getTLSConfig(secret); err != nil {
+				// Return err as the content of the secret may change.
+				return sreconcile.ResultEmpty, err
+			}
+			// If we constructed a TLS config using the secret specified in `.spec.secretRef`,
+			// then alert users that this behavior is deprecated.
+			if tlsConfig != nil {
+				r.Event(obj, "Warning", "DeprecatedBehaviorReason",
+					"specifying TLS authentication data via `.spec.secretRef` is deprecated, please use `.spec.certSecretRef` instead")
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
-			// Requeue as content of secret might change
-			return sreconcile.ResultEmpty, e
 		}
 	}