refactor TLSconfig funcs

Signed-off-by: Soule BA <[email protected]>

Author: souleb

docs/spec/v1beta2/helmrepositories.md

@@ -485,7 +485,7 @@ data:
   caFile: <BASE64>
 ```
 
-#### Provide TLS credentials in a secret of type kubernetes.io/dockerconfigjson
+##### Provide TLS credentials in a secret of type kubernetes.io/dockerconfigjson
 
 For OCI Helm repositories, Kubernetes secrets of type [kubernetes.io/dockerconfigjson](https://kubernetes.io/docs/concepts/configuration/secret/#secret-types)
 are also supported. It is possible to append TLS credentials to the secret data.

internal/controller/helmchart_controller.go

@@ -510,8 +510,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		tlsConfig     *tls.Config
 		authenticator authn.Authenticator
 		keychain      authn.Keychain
-		tlsLoginOpt   helmreg.LoginOption
-		tmpCertsDir   string
+		secret        *corev1.Secret
 	)
 	// Used to login with the repository declared provider
 	ctxTimeout, cancel := context.WithTimeout(ctx, repo.Spec.Timeout.Duration)
@@ -527,7 +526,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		helmgetter.WithTimeout(repo.Spec.Timeout.Duration),
 		helmgetter.WithPassCredentialsAll(repo.Spec.PassCredentials),
 	}
-	if secret, err := r.getHelmRepositorySecret(ctx, repo); secret != nil || err != nil {
+	if secret, err = r.getHelmRepositorySecret(ctx, repo); secret != nil || err != nil {
 		if err != nil {
 			e := &serror.Event{
 				Err:    fmt.Errorf("failed to get secret '%s': %w", repo.Spec.SecretRef.Name, err),
@@ -551,22 +550,6 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		}
 		clientOpts = append(clientOpts, opts...)
 		tlsConfig = tlsCfg
-		tlsLoginOpt, tmpCertsDir, err = makeTLSLoginOption(secret)
-		if err != nil {
-			e := &serror.Event{
-				Err:    err,
-				Reason: sourcev1.AuthenticationFailedReason,
-			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
-			// Requeue as content of secret might change
-			return sreconcile.ResultEmpty, e
-		}
-		defer func() {
-			if err := os.RemoveAll(tmpCertsDir); err != nil {
-				r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
-					"failed to delete temporary certificates directory: %s", err)
-			}
-		}()
 
 		// Build registryClient options from secret
 		keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret)
@@ -669,8 +652,26 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		// The OCIGetter will later retrieve the stored credentials to pull the chart
 		if loginOpt != nil {
 			opts := []helmreg.LoginOption{loginOpt}
-			if tlsLoginOpt != nil {
-				opts = append(opts, tlsLoginOpt)
+			if tlsConfig != nil && secret != nil {
+				tlsLoginOpt, tmpCertsDir, err := registry.TLSLoginOptionFromSecret(secret)
+				if err != nil {
+					e := &serror.Event{
+						Err:    err,
+						Reason: sourcev1.AuthenticationFailedReason,
+					}
+					conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
+					// Requeue as content of secret might change
+					return sreconcile.ResultEmpty, e
+				}
+				defer func() {
+					if err := os.RemoveAll(tmpCertsDir); err != nil {
+						r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
+							"failed to delete temporary certificates directory: %s", err)
+					}
+				}()
+				if tlsLoginOpt != nil {
+					opts = append(opts, tlsLoginOpt)
+				}
 			}
 			err = ociChartRepo.Login(opts...)
 			if err != nil {
@@ -1048,8 +1049,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
 	return func(url string) (repo repository.Downloader, err error) {
 		var (
 			tlsConfig     *tls.Config
-			tlsLoginOpt   helmreg.LoginOption
-			tmpCertsDir   string
+			secret        *corev1.Secret
 			authenticator authn.Authenticator
 			keychain      authn.Keychain
 		)
@@ -1081,7 +1081,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
 			helmgetter.WithTimeout(obj.Spec.Timeout.Duration),
 			helmgetter.WithPassCredentialsAll(obj.Spec.PassCredentials),
 		}
-		if secret, err := r.getHelmRepositorySecret(ctx, obj); secret != nil || err != nil {
+		if secret, err = r.getHelmRepositorySecret(ctx, obj); secret != nil || err != nil {
 			if err != nil {
 				return nil, err
 			}
@@ -1093,19 +1093,6 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
 			}
 			clientOpts = append(clientOpts, opts...)
 			tlsConfig = tlsCfg
-			tlsLoginOpt, tmpCertsDir, err = makeTLSLoginOption(secret)
-			if err != nil {
-				return nil, err
-			}
-			defer func() {
-				var errs []error
-				if errf := os.RemoveAll(tmpCertsDir); errf != nil {
-					errs = append(errs, errf)
-				}
-				errs = append(errs, err)
-				err = kerrors.NewAggregate(errs)
-				return
-			}()
 
 			// Build registryClient options from secret
 			keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret)
@@ -1157,8 +1144,23 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
 			// The OCIGetter will later retrieve the stored credentials to pull the chart
 			if loginOpt != nil {
 				opts := []helmreg.LoginOption{loginOpt}
-				if tlsLoginOpt != nil {
-					opts = append(opts, tlsLoginOpt)
+				if tlsConfig != nil && secret != nil {
+					tlsLoginOpt, tmpCertsDir, err := registry.TLSLoginOptionFromSecret(secret)
+					if err != nil {
+						return nil, err
+					}
+					defer func() {
+						var errs []error
+						if errf := os.RemoveAll(tmpCertsDir); errf != nil {
+							errs = append(errs, errf)
+						}
+						errs = append(errs, err)
+						err = kerrors.NewAggregate(errs)
+						return
+					}()
+					if tlsLoginOpt != nil {
+						opts = append(opts, tlsLoginOpt)
+					}
 				}
 				err = ociChartRepo.Login(opts...)
 				if err != nil {

internal/controller/helmrepository_controller_oci.go

@@ -309,47 +309,29 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S
 		authenticator authn.Authenticator
 		keychain      authn.Keychain
 		tlsConfig     *tls.Config
-		tmpCertsDir   string
-		tlsLoginOpt   helmreg.LoginOption
+		secret        *corev1.Secret
 		err           error
 	)
 	// 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 {
-			conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error())
-			result, retErr = ctrl.Result{}, err
-			return
-		}
-		keychain, err = authFromSecret(ctx, r.Client, obj.Spec.URL, secret)
+		secret, err = r.getSecret(ctx, obj)
 		if err != nil {
 			conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error())
 			result, retErr = ctrl.Result{}, err
 			return
 		}
-		tlsConfig, err = getter.TLSClientConfigFromSecret(secret, obj.Spec.URL)
+		keychain, err = authFromSecret(ctx, r.Client, obj.Spec.URL, *secret)
 		if err != nil {
 			conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error())
 			result, retErr = ctrl.Result{}, err
 			return
 		}
-		tlsLoginOpt, tmpCertsDir, err = makeTLSLoginOption(&secret)
+		tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, obj.Spec.URL)
 		if err != nil {
 			conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error())
 			result, retErr = ctrl.Result{}, err
 			return
 		}
-		defer func() {
-			if err := os.RemoveAll(tmpCertsDir); err != nil {
-				r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
-					"failed to delete temporary certificates directory: %s", err)
-			}
-		}()
 	} else if obj.Spec.Provider != helmv1.GenericOCIProvider && obj.Spec.Type == helmv1.HelmRepositoryTypeOCI {
 		auth, authErr := oidcAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
 		if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
@@ -400,8 +382,22 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S
 	// Attempt to login to the registry if credentials are provided.
 	if loginOpt != nil {
 		opts := []helmreg.LoginOption{loginOpt}
-		if tlsLoginOpt != nil {
-			opts = append(opts, tlsLoginOpt)
+		if tlsConfig != nil && secret != nil {
+			tlsLoginOpt, tmpCertsDir, err := registry.TLSLoginOptionFromSecret(secret)
+			if err != nil {
+				conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error())
+				result, retErr = ctrl.Result{}, err
+				return
+			}
+			defer func() {
+				if err := os.RemoveAll(tmpCertsDir); err != nil {
+					r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
+						"failed to delete temporary certificates directory: %s", err)
+				}
+			}()
+			if tlsLoginOpt != nil {
+				opts = append(opts, tlsLoginOpt)
+			}
 		}
 		err = chartRepo.Login(opts...)
 		if err != nil {
@@ -429,6 +425,22 @@ func (r *HelmRepositoryOCIReconciler) reconcileDelete(ctx context.Context, obj *
 	return ctrl.Result{}, nil
 }
 
+func (r *HelmRepositoryOCIReconciler) getSecret(ctx context.Context, obj *helmv1.HelmRepository) (*corev1.Secret, error) {
+	if obj.Spec.SecretRef == nil {
+		return nil, nil
+	}
+	name := types.NamespacedName{
+		Namespace: obj.GetNamespace(),
+		Name:      obj.Spec.SecretRef.Name,
+	}
+	var secret corev1.Secret
+	err := r.Client.Get(ctx, name, &secret)
+	if err != nil {
+		return nil, err
+	}
+	return &secret, nil
+}
+
 // eventLogf records events, and logs at the same time.
 //
 // This log is different from the debug log in the EventRecorder, in the sense
@@ -470,80 +482,6 @@ func makeLoginOption(auth authn.Authenticator, keychain authn.Keychain, registry
 	return nil, nil
 }
 
-func makeTLSLoginOption(secret *corev1.Secret) (helmreg.LoginOption, string, error) {
-	var errs []error
-	certFile, keyFile, caFile, tmpDir, err := certsFilesFromSecret(secret)
-	if err != nil {
-		errs = append(errs, err)
-		if tmpDir != "" {
-			if err := os.RemoveAll(tmpDir); err != nil {
-				errs = append(errs, err)
-			}
-		}
-		return nil, "", kerrors.NewAggregate(errs)
-	}
-
-	if (certFile != "" && keyFile != "") || caFile != "" {
-		return helmreg.LoginOptTLSClientConfig(certFile, keyFile, caFile), tmpDir, nil
-	}
-
-	return nil, "", nil
-}
-
-func certsFilesFromSecret(secret *corev1.Secret) (string, string, string, string, error) {
-	certBytes, keyBytes, caBytes := secret.Data["certFile"], secret.Data["keyFile"], secret.Data["caFile"]
-	switch {
-	case len(certBytes)+len(keyBytes)+len(caBytes) == 0:
-		return "", "", "", "", nil
-	case (len(certBytes) > 0 && len(keyBytes) == 0) || (len(keyBytes) > 0 && len(certBytes) == 0):
-		return "", "", "", "", fmt.Errorf("invalid '%s' secret data: fields 'certFile' and 'keyFile' require each other's presence",
-			secret.Name)
-	}
-
-	var (
-		certFile string
-		keyFile  string
-		caFile   string
-		err      error
-	)
-
-	// create temporary folder to store the certs
-	tmpDir, err := os.MkdirTemp("", "helm-repo-oci-certs")
-	if err != nil {
-		return "", "", "", "", err
-	}
-
-	if len(certBytes) > 0 && len(keyBytes) > 0 {
-		certFile, err = writeTofile(certBytes, "cert.pem", tmpDir)
-		if err != nil {
-			return "", "", "", "", err
-		}
-		keyFile, err = writeTofile(keyBytes, "key.pem", tmpDir)
-		if err != nil {
-			return "", "", "", "", err
-		}
-	}
-	if len(caBytes) > 0 {
-		caFile, err = writeTofile(caBytes, "ca.pem", tmpDir)
-		if err != nil {
-			return "", "", "", "", err
-		}
-	}
-	return certFile, keyFile, caFile, tmpDir, nil
-}
-
-func writeTofile(data []byte, filename, tmpDir string) (string, error) {
-	file, err := os.CreateTemp(tmpDir, filename)
-	if err != nil {
-		return "", err
-	}
-	defer file.Close()
-	if _, err := file.Write(data); err != nil {
-		return "", err
-	}
-	return file.Name(), nil
-}
-
 func conditionsDiff(a, b []string) []string {
 	bMap := make(map[string]struct{}, len(b))
 	for _, j := range b {