Remove helm sdk oras v1 workarounds

Signed-off-by: Matheus Pimenta <[email protected]>

Author: matheuscscp

internal/controller/helmchart_controller.go

@@ -143,11 +143,8 @@ type HelmChartReconciler struct {
 	patchOptions []patch.Option
 }
 
-// RegistryClientGeneratorFunc is a function that returns a registry client
-// and an optional file name.
-// The file is used to store the registry client credentials.
-// The caller is responsible for deleting the file.
-type RegistryClientGeneratorFunc func(tlsConfig *tls.Config, isLogin, insecure bool) (*helmreg.Client, string, error)
+// RegistryClientGeneratorFunc is a function that returns a registry client.
+type RegistryClientGeneratorFunc func(tlsConfig *tls.Config, isLogin, insecure bool) (*helmreg.Client, error)
 
 func (r *HelmChartReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
 	return r.SetupWithManagerAndOptions(ctx, mgr, HelmChartReconcilerOptions{})
@@ -552,11 +549,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 			return chartRepoConfigErrorReturn(err, obj)
 		}
 
-		// with this function call, we create a temporary file to store the credentials if needed.
-		// this is needed because otherwise the credentials are stored in ~/.docker/config.json.
-		// TODO@souleb: remove this once the registry move to Oras v2
-		// or rework to enable reusing credentials to avoid the unneccessary handshake operations
-		registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry(), repo.Spec.Insecure)
+		registryClient, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry(), repo.Spec.Insecure)
 		if err != nil {
 			e := serror.NewGeneric(
 				fmt.Errorf("failed to construct Helm client: %w", err),
@@ -566,15 +559,6 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 			return sreconcile.ResultEmpty, e
 		}
 
-		if credentialsFile != "" {
-			defer func() {
-				if err := os.Remove(credentialsFile); err != nil {
-					r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
-						"failed to delete temporary credentials file: %s", err)
-				}
-			}()
-		}
-
 		var verifiers []soci.Verifier
 		if obj.Spec.Verify != nil {
 			provider := obj.Spec.Verify.Provider
@@ -1026,39 +1010,34 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
 
 		var chartRepo repository.Downloader
 		if helmreg.IsOCI(normalizedURL) {
-			registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry(), obj.Spec.Insecure)
+			registryClient, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry(), obj.Spec.Insecure)
 			if err != nil {
 				return nil, fmt.Errorf("failed to create registry client: %w", err)
 			}
 
-			var errs []error
 			// Tell the chart repository to use the OCI client with the configured getter
 			getterOpts = append(getterOpts, helmgetter.WithRegistryClient(registryClient))
 			ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters),
 				repository.WithOCIGetterOptions(getterOpts),
 				repository.WithOCIRegistryClient(registryClient),
-				repository.WithCertificatesStore(certsTmpDir),
-				repository.WithCredentialsFile(credentialsFile))
+				repository.WithCertificatesStore(certsTmpDir))
 			if err != nil {
-				errs = append(errs, fmt.Errorf("failed to create OCI chart repository: %w", err))
-				// clean up the credentialsFile
-				if credentialsFile != "" {
-					if err := os.Remove(credentialsFile); err != nil {
-						errs = append(errs, err)
-					}
-				}
-				return nil, kerrors.NewAggregate(errs)
+				return nil, fmt.Errorf("failed to create OCI chart repository: %w", err)
 			}
 
 			// If login options are configured, use them to login to the registry
 			// The OCIGetter will later retrieve the stored credentials to pull the chart
 			if clientOpts.MustLoginToRegistry() {
 				err = ociChartRepo.Login(clientOpts.RegLoginOpts...)
 				if err != nil {
-					errs = append(errs, fmt.Errorf("failed to login to OCI chart repository: %w", err))
-					// clean up the credentialsFile
-					errs = append(errs, ociChartRepo.Clear())
-					return nil, kerrors.NewAggregate(errs)
+					err = fmt.Errorf("failed to login to OCI chart repository: %w", err)
+					if clearErr := ociChartRepo.Clear(); clearErr != nil {
+						var errs []error
+						errs = append(errs, err)
+						errs = append(errs, clearErr)
+						return nil, kerrors.NewAggregate(errs)
+					}
+					return nil, err
 				}
 			}
 

internal/helm/chart/dependency_manager_test.go

@@ -76,9 +76,7 @@ func (g *mockGetter) Get(_ string, _ ...helmgetter.Option) (*bytes.Buffer, error
 func TestDependencyManager_Clear(t *testing.T) {
 	g := NewWithT(t)
 
-	file, err := os.CreateTemp("", "")
-	g.Expect(err).ToNot(HaveOccurred())
-	ociRepoWithCreds, err := repository.NewOCIChartRepository("oci://example.com", repository.WithCredentialsFile(file.Name()))
+	ociRepoWithCreds, err := repository.NewOCIChartRepository("oci://example.com")
 	g.Expect(err).ToNot(HaveOccurred())
 
 	downloaders := map[string]repository.Downloader{
@@ -99,14 +97,8 @@ func TestDependencyManager_Clear(t *testing.T) {
 		case *repository.ChartRepository:
 			g.Expect(v.Index).To(BeNil())
 		case *repository.OCIChartRepository:
-			g.Expect(v.HasCredentials()).To(BeFalse())
 		}
 	}
-
-	if _, err := os.Stat(file.Name()); !errors.Is(err, os.ErrNotExist) {
-		err = os.Remove(file.Name())
-		g.Expect(err).ToNot(HaveOccurred())
-	}
 }
 
 func TestDependencyManager_Build(t *testing.T) {

internal/helm/registry/client.go

@@ -20,48 +20,29 @@ import (
 	"crypto/tls"
 	"io"
 	"net/http"
-	"os"
 
 	"helm.sh/helm/v3/pkg/registry"
-	"k8s.io/apimachinery/pkg/util/errors"
 )
 
-// ClientGenerator generates a registry client and a temporary credential file.
+// ClientGenerator generates a registry client.
 // The client is meant to be used for a single reconciliation.
-// The file is meant to be used for a single reconciliation and deleted after.
-func ClientGenerator(tlsConfig *tls.Config, isLogin, insecureHTTP bool) (*registry.Client, string, error) {
+func ClientGenerator(tlsConfig *tls.Config, isLogin, insecureHTTP bool) (*registry.Client, error) {
 	if isLogin {
-		// create a temporary file to store the credentials
-		// this is needed because otherwise the credentials are stored in ~/.docker/config.json.
-		credentialsFile, err := os.CreateTemp("", "credentials")
+		rClient, err := newClient(tlsConfig, insecureHTTP)
 		if err != nil {
-			return nil, "", err
+			return nil, err
 		}
-
-		var errs []error
-		rClient, err := newClient("", tlsConfig, insecureHTTP)
-		if err != nil {
-			errs = append(errs, err)
-			// attempt to delete the temporary file
-			if credentialsFile != nil {
-				err := os.Remove(credentialsFile.Name())
-				if err != nil {
-					errs = append(errs, err)
-				}
-			}
-			return nil, "", errors.NewAggregate(errs)
-		}
-		return rClient, credentialsFile.Name(), nil
+		return rClient, nil
 	}
 
-	rClient, err := newClient("", tlsConfig, insecureHTTP)
+	rClient, err := newClient(tlsConfig, insecureHTTP)
 	if err != nil {
-		return nil, "", err
+		return nil, err
 	}
-	return rClient, "", nil
+	return rClient, nil
 }
 
-func newClient(credentialsFile string, tlsConfig *tls.Config, insecureHTTP bool) (*registry.Client, error) {
+func newClient(tlsConfig *tls.Config, insecureHTTP bool) (*registry.Client, error) {
 	opts := []registry.ClientOption{
 		registry.ClientOptWriter(io.Discard),
 	}
@@ -75,9 +56,5 @@ func newClient(credentialsFile string, tlsConfig *tls.Config, insecureHTTP bool)
 			Transport: t,
 		}))
 	}
-	if credentialsFile != "" {
-		opts = append(opts, registry.ClientOptCredentialsFile(credentialsFile))
-	}
-
 	return registry.NewClient(opts...)
 }

internal/helm/repository/oci_chart_repository.go

@@ -20,7 +20,6 @@ import (
 	"bytes"
 	"context"
 	"crypto/tls"
-	"errors"
 	"fmt"
 	"net/url"
 	"os"
@@ -67,9 +66,6 @@ type OCIChartRepository struct {
 	// RegistryClient is a client to use while downloading tags or charts from a registry.
 	RegistryClient RegistryClient
 
-	// credentialsFile is a temporary credentials file to use while downloading tags or charts from a registry.
-	credentialsFile string
-
 	// certificatesStore is a temporary store to use while downloading tags or charts from a registry.
 	certificatesStore string
 
@@ -127,14 +123,6 @@ func WithOCIGetterOptions(getterOpts []getter.Option) OCIChartRepositoryOption {
 	}
 }
 
-// WithCredentialsFile returns a ChartRepositoryOption that will set the credentials file
-func WithCredentialsFile(credentialsFile string) OCIChartRepositoryOption {
-	return func(r *OCIChartRepository) error {
-		r.credentialsFile = credentialsFile
-		return nil
-	}
-}
-
 // WithCertificatesStore returns a ChartRepositoryOption that will set the certificates store
 func WithCertificatesStore(store string) OCIChartRepositoryOption {
 	return func(r *OCIChartRepository) error {
@@ -281,31 +269,13 @@ func (r *OCIChartRepository) Logout() error {
 	return nil
 }
 
-// HasCredentials returns true if the OCIChartRepository has credentials.
-func (r *OCIChartRepository) HasCredentials() bool {
-	return r.credentialsFile != ""
-}
-
-// Clear deletes the OCI registry credentials file.
-func (r *OCIChartRepository) Clear() error {
-	var errs error
-	// clean the credentials file if it exists
-	if r.credentialsFile != "" {
-		if err := os.Remove(r.credentialsFile); err != nil {
-			errs = errors.Join(errs, err)
-		}
-	}
-	r.credentialsFile = ""
-
-	// clean the certificates store if it exists
+// Clear deletes the OCI registry certificates store if it exists.
+func (r *OCIChartRepository) Clear() (err error) {
 	if r.certificatesStore != "" {
-		if err := os.RemoveAll(r.certificatesStore); err != nil {
-			errs = errors.Join(errs, err)
-		}
+		err = os.RemoveAll(r.certificatesStore)
 	}
 	r.certificatesStore = ""
-
-	return errs
+	return
 }
 
 // getLastMatchingVersionOrConstraint returns the last version that matches the given version string.