helm: Remove TLS tmp dir logic for HelmRepository type: oci

hack/ci/e2e.sh

@@ -6,15 +6,10 @@ CREATE_CLUSTER="${CREATE_CLUSTER:-true}"
 KIND_CLUSTER_NAME="${KIND_CLUSTER_NAME:-kind}"
 LOAD_IMG_INTO_KIND="${LOAD_IMG_INTO_KIND:-true}"
 BUILD_PLATFORM="${BUILD_PLATFORM:-linux/amd64}"
-MINIO_HELM_VER="${MINIO_HELM_VER:-12.10.3}"
 
 IMG=test/source-controller
 TAG=latest
 
-MC_RELEASE=mc.RELEASE.2023-11-20T16-30-59Z
-MC_AMD64_SHA256=fdd901a5169d676f32483f9a2de977b7ff3a4fe83e254dcbc35e7a1545591565
-MC_ARM64_SHA256=09816180f560875d344dc436ed4ec1348b3ff0c836ae9cf0415fef602489cc11
-
 ROOT_DIR="$(git rev-parse --show-toplevel)"
 BUILD_DIR="${ROOT_DIR}/build"
 
@@ -39,8 +34,6 @@ function cleanup(){
         kubectl -n source-system get helmcharts -oyaml
         kubectl -n source-system get all
         kubectl -n source-system logs deploy/source-controller
-        kubectl -n minio get all
-        kubectl -n minio describe pods
     else
         echo "All E2E tests passed!"
     fi
@@ -83,58 +76,6 @@ kubectl -n source-system wait helmchart/podinfo --for=condition=ready --timeout=
 kubectl -n source-system wait helmchart/podinfo-git --for=condition=ready --timeout=5m
 kubectl -n source-system delete -f "${ROOT_DIR}/config/testdata/helmchart-valuesfile"
 
-echo "Setup Minio"
-kubectl create ns minio
-helm upgrade minio oci://registry-1.docker.io/bitnamicharts/minio --wait -i \
-    --version "${MINIO_HELM_VER}" \
-    --timeout 10m0s \
-    --namespace minio \
-    --set auth.rootUser=myaccesskey \
-    --set auth.rootPassword=mysecretkey \
-    --set resources.requests.memory=128Mi \
-    --set persistence.enable=false
-kubectl -n minio port-forward svc/minio 9000:9000 &>/dev/null &
-
-sleep 2
-
-if [ ! -f "${BUILD_DIR}/mc" ]; then
-    MC_SHA256="${MC_AMD64_SHA256}"
-    ARCH="amd64"
-    if [ "${BUILD_PLATFORM}" = "linux/arm64" ]; then
-        MC_SHA256="${MC_ARM64_SHA256}"
-        ARCH="arm64"
-    fi
-
-    mkdir -p "${BUILD_DIR}"
-    curl -o "${BUILD_DIR}/mc" -LO "https://dl.min.io/client/mc/release/linux-${ARCH}/archive/${MC_RELEASE}"
-    if ! echo "${MC_SHA256}  ${BUILD_DIR}/mc" | sha256sum --check; then
-        echo "Checksum failed for mc."
-        rm "${BUILD_DIR}/mc"
-        exit 1
-    fi
-
-    chmod +x "${BUILD_DIR}/mc"
-fi
-
-"${BUILD_DIR}/mc" alias set minio http://localhost:9000 myaccesskey mysecretkey --api S3v4
-kubectl -n source-system apply -f "${ROOT_DIR}/config/testdata/minio/secret.yaml"
-
-echo "Run Bucket tests"
-"${BUILD_DIR}/mc" mb minio/podinfo
-"${BUILD_DIR}/mc" mirror "${ROOT_DIR}/config/testdata/minio/manifests/" minio/podinfo
-
-kubectl -n source-system apply -f "${ROOT_DIR}/config/testdata/bucket/source.yaml"
-kubectl -n source-system wait bucket/podinfo --for=condition=ready --timeout=1m
-
-
-echo "Run HelmChart from Bucket tests"
-"${BUILD_DIR}/mc" mb minio/charts
-"${BUILD_DIR}/mc" mirror "${ROOT_DIR}/internal/controller/testdata/charts/helmchart/" minio/charts/helmchart
-
-kubectl -n source-system apply -f "${ROOT_DIR}/config/testdata/helmchart-from-bucket/source.yaml"
-kubectl -n source-system wait bucket/charts --for=condition=ready --timeout=1m
-kubectl -n source-system wait helmchart/helmchart-bucket --for=condition=ready --timeout=1m
-
 echo "Run large Git repo tests"
 kubectl -n source-system apply -f "${ROOT_DIR}/config/testdata/git/large-repo.yaml"
 kubectl -n source-system wait gitrepository/large-repo --for=condition=ready --timeout=2m15s

internal/controller/helmchart_controller.go

@@ -524,7 +524,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		return chartRepoConfigErrorReturn(err, obj)
 	}
 
-	clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL)
+	clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL)
 	if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) {
 		e := serror.NewGeneric(
 			err,
@@ -533,14 +533,6 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
 		return sreconcile.ResultEmpty, e
 	}
-	if certsTmpDir != "" {
-		defer func() {
-			if err := os.RemoveAll(certsTmpDir); err != nil {
-				r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
-					"failed to delete temporary certificates directory: %s", err)
-			}
-		}()
-	}
 
 	getterOpts := clientOpts.GetterOpts
 
@@ -1019,7 +1011,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
 		ctxTimeout, cancel := context.WithTimeout(ctx, obj.GetTimeout())
 		defer cancel()
 
-		clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL)
+		clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL)
 		if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) {
 			return nil, err
 		}
@@ -1038,7 +1030,6 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
 			ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters),
 				repository.WithOCIGetterOptions(getterOpts),
 				repository.WithOCIRegistryClient(registryClient),
-				repository.WithCertificatesStore(certsTmpDir),
 				repository.WithCredentialsFile(credentialsFile))
 			if err != nil {
 				errs = append(errs, fmt.Errorf("failed to create OCI chart repository: %w", err))

internal/controller/helmrepository_controller.go

@@ -416,7 +416,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
 		return sreconcile.ResultEmpty, e
 	}
 
-	clientOpts, _, err := getter.GetClientOpts(ctx, r.Client, obj, normalizedURL)
+	clientOpts, err := getter.GetClientOpts(ctx, r.Client, obj, normalizedURL)
 	if err != nil {
 		if errors.Is(err, getter.ErrDeprecatedTLSConfig) {
 			ctrl.LoggerFrom(ctx).

internal/helm/getter/client_opts.go

@@ -21,8 +21,6 @@ import (
 	"crypto/tls"
 	"errors"
 	"fmt"
-	"os"
-	"path"
 
 	"github.com/google/go-containerregistry/pkg/authn"
 	helmgetter "helm.sh/helm/v3/pkg/getter"
@@ -69,7 +67,7 @@ func (o ClientOpts) MustLoginToRegistry() bool {
 // auth mechanisms.
 // A temporary directory is created to store the certs files if needed and its path is returned along with the options object. It is the
 // caller's responsibility to clean up the directory.
-func GetClientOpts(ctx context.Context, c client.Client, obj *sourcev1.HelmRepository, url string) (*ClientOpts, string, error) {
+func GetClientOpts(ctx context.Context, c client.Client, obj *sourcev1.HelmRepository, url string) (*ClientOpts, error) {
 	// This function configures authentication for Helm repositories based on the provided secrets:
 	// - CertSecretRef: TLS client certificates (always takes priority)
 	// - SecretRef: Can contain Basic Auth or TLS certificates (deprecated)
@@ -84,17 +82,16 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *sourcev1.HelmRepos
 	}
 
 	// Process secrets and configure authentication
-	deprecatedTLS, certSecret, authSecret, err := configureAuthentication(ctx, c, obj, opts, url)
+	deprecatedTLS, _, authSecret, err := configureAuthentication(ctx, c, obj, opts)
 	if err != nil {
-		return nil, "", err
+		return nil, err
 	}
 
 	// Setup OCI registry specific configurations if needed
-	var tempCertDir string
 	if obj.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
-		tempCertDir, err = configureOCIRegistryWithSecrets(ctx, obj, opts, url, certSecret, authSecret)
+		err = configureOCIRegistryWithSecrets(ctx, obj, opts, url, authSecret)
 		if err != nil {
-			return nil, "", err
+			return nil, err
 		}
 	}
 
@@ -103,15 +100,15 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *sourcev1.HelmRepos
 		deprecatedErr = ErrDeprecatedTLSConfig
 	}
 
-	return opts, tempCertDir, deprecatedErr
+	return opts, deprecatedErr
 }
 
 // configureAuthentication processes all secret references and sets up authentication.
 // Returns (deprecatedTLS, certSecret, authSecret, error) where:
 // - deprecatedTLS: true if TLS config comes from SecretRef (deprecated pattern)
 // - certSecret: the secret from CertSecretRef (nil if not specified)
 // - authSecret: the secret from SecretRef (nil if not specified)
-func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1.HelmRepository, opts *ClientOpts, url string) (bool, *corev1.Secret, *corev1.Secret, error) {
+func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1.HelmRepository, opts *ClientOpts) (bool, *corev1.Secret, *corev1.Secret, error) {
 	var deprecatedTLS bool
 	var certSecret, authSecret *corev1.Secret
 
@@ -171,12 +168,12 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
 }
 
 // configureOCIRegistryWithSecrets sets up OCI-specific configurations using pre-fetched secrets
-func configureOCIRegistryWithSecrets(ctx context.Context, obj *sourcev1.HelmRepository, opts *ClientOpts, url string, certSecret, authSecret *corev1.Secret) (string, error) {
+func configureOCIRegistryWithSecrets(ctx context.Context, obj *sourcev1.HelmRepository, opts *ClientOpts, url string, authSecret *corev1.Secret) error {
 	// Configure OCI authentication from authSecret if available
 	if authSecret != nil {
 		keychain, err := registry.LoginOptionFromSecret(url, *authSecret)
 		if err != nil {
-			return "", fmt.Errorf("failed to configure login options: %w", err)
+			return fmt.Errorf("failed to configure login options: %w", err)
 		}
 		opts.Keychain = keychain
 	}
@@ -185,48 +182,22 @@ func configureOCIRegistryWithSecrets(ctx context.Context, obj *sourcev1.HelmRepo
 	if obj.Spec.SecretRef == nil && obj.Spec.Provider != "" && obj.Spec.Provider != sourcev1.GenericOCIProvider {
 		authenticator, err := soci.OIDCAuth(ctx, url, obj.Spec.Provider)
 		if err != nil {
-			return "", fmt.Errorf("failed to get credential from '%s': %w", obj.Spec.Provider, err)
+			return fmt.Errorf("failed to get credential from '%s': %w", obj.Spec.Provider, err)
 		}
 		opts.Authenticator = authenticator
 	}
 
 	// Setup registry login options
 	loginOpt, err := registry.NewLoginOption(opts.Authenticator, opts.Keychain, url)
 	if err != nil {
-		return "", err
+		return err
 	}
 
 	if loginOpt != nil {
 		opts.RegLoginOpts = []helmreg.LoginOption{loginOpt, helmreg.LoginOptInsecure(obj.Spec.Insecure)}
 	}
 
-	// Handle TLS certificate files for OCI
-	var tempCertDir string
-	if opts.TlsConfig != nil {
-		tempCertDir, err = os.MkdirTemp("", "helm-repo-oci-certs")
-		if err != nil {
-			return "", fmt.Errorf("cannot create temporary directory: %w", err)
-		}
-
-		var tlsSecret *corev1.Secret
-		if certSecret != nil {
-			tlsSecret = certSecret
-		} else if authSecret != nil {
-			tlsSecret = authSecret
-		}
-
-		certFile, keyFile, caFile, err := storeTLSCertificateFilesForOCI(ctx, tlsSecret, nil, tempCertDir)
-		if err != nil {
-			return "", fmt.Errorf("cannot write certs files to path: %w", err)
-		}
-
-		tlsLoginOpt := registry.TLSLoginOption(certFile, keyFile, caFile)
-		if tlsLoginOpt != nil {
-			opts.RegLoginOpts = append(opts.RegLoginOpts, tlsLoginOpt)
-		}
-	}
-
-	return tempCertDir, nil
+	return nil
 }
 
 func fetchSecret(ctx context.Context, c client.Client, name, namespace string) (*corev1.Secret, error) {
@@ -240,57 +211,3 @@ func fetchSecret(ctx context.Context, c client.Client, name, namespace string) (
 	}
 	return &secret, nil
 }
-
-// storeTLSCertificateFilesForOCI writes TLS certificate data from secrets to files for OCI registry authentication.
-// Helm OCI registry client requires certificate file paths rather than in-memory data,
-// so we need to temporarily write the certificate data to disk.
-// Returns paths to the written cert, key, and CA files (any of which may be empty if not present).
-func storeTLSCertificateFilesForOCI(ctx context.Context, certSecret, authSecret *corev1.Secret, path string) (string, string, string, error) {
-	var (
-		certFile string
-		keyFile  string
-		caFile   string
-		err      error
-	)
-
-	// Try to get TLS data from certSecret first, then authSecret
-	var tlsSecret *corev1.Secret
-	if certSecret != nil {
-		tlsSecret = certSecret
-	} else if authSecret != nil {
-		tlsSecret = authSecret
-	}
-
-	if tlsSecret != nil {
-		if certData, exists := tlsSecret.Data[secrets.KeyTLSCert]; exists {
-			if keyData, keyExists := tlsSecret.Data[secrets.KeyTLSPrivateKey]; keyExists {
-				certFile, err = writeToFile(certData, certFileName, path)
-				if err != nil {
-					return "", "", "", err
-				}
-				keyFile, err = writeToFile(keyData, keyFileName, path)
-				if err != nil {
-					return "", "", "", err
-				}
-			}
-		}
-
-		if caData, exists := tlsSecret.Data[secrets.KeyCACert]; exists {
-			caFile, err = writeToFile(caData, caFileName, path)
-			if err != nil {
-				return "", "", "", err
-			}
-		}
-	}
-
-	return certFile, keyFile, caFile, nil
-}
-
-func writeToFile(data []byte, filename, tmpDir string) (string, error) {
-	file := path.Join(tmpDir, filename)
-	err := os.WriteFile(file, data, 0o600)
-	if err != nil {
-		return "", err
-	}
-	return file, nil
-}

internal/helm/getter/client_opts_test.go

@@ -164,7 +164,7 @@ func TestGetClientOpts(t *testing.T) {
 			}
 			c := clientBuilder.Build()
 
-			clientOpts, _, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy")
+			clientOpts, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy")
 			if tt.err != nil {
 				g.Expect(err).To(Equal(tt.err))
 			} else {
@@ -207,7 +207,7 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) {
 					"password": []byte("pass"),
 				},
 			},
-			loginOptsN: 3,
+			loginOptsN: 2,
 		},
 		{
 			name: "without caFile",
@@ -271,7 +271,7 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) {
 			}
 			c := clientBuilder.Build()
 
-			clientOpts, tmpDir, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy")
+			clientOpts, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy")
 			if tt.wantErrMsg != "" {
 				if err == nil {
 					t.Errorf("GetClientOpts() expected error but got none")
@@ -287,9 +287,6 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) {
 				t.Errorf("GetClientOpts() error = %v", err)
 				return
 			}
-			if tmpDir != "" {
-				defer os.RemoveAll(tmpDir)
-			}
 			if tt.loginOptsN != len(clientOpts.RegLoginOpts) {
 				// we should have a login option but no TLS option
 				t.Errorf("expected length of %d for clientOpts.RegLoginOpts but got %d", tt.loginOptsN, len(clientOpts.RegLoginOpts))