From bcbe7edab83be17357992c78a7092fa745fcecd7 Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Thu, 2 Oct 2025 22:52:24 +0300 Subject: [PATCH 1/2] helm: Remove the TLS tmp dir logic Signed-off-by: Stefan Prodan --- internal/controller/helmchart_controller.go | 13 +-- .../controller/helmrepository_controller.go | 2 +- internal/helm/getter/client_opts.go | 107 ++---------------- internal/helm/getter/client_opts_test.go | 9 +- 4 files changed, 18 insertions(+), 113 deletions(-) diff --git a/internal/controller/helmchart_controller.go b/internal/controller/helmchart_controller.go index e969bf67a..9f65502cb 100644 --- a/internal/controller/helmchart_controller.go +++ b/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)) diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go index 06c4494cf..36716b1fb 100644 --- a/internal/controller/helmrepository_controller.go +++ b/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). diff --git a/internal/helm/getter/client_opts.go b/internal/helm/getter/client_opts.go index e40811b39..1929ab13a 100644 --- a/internal/helm/getter/client_opts.go +++ b/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,7 +100,7 @@ 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. @@ -111,7 +108,7 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *sourcev1.HelmRepos // - 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,7 +182,7 @@ 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 } @@ -193,40 +190,14 @@ func configureOCIRegistryWithSecrets(ctx context.Context, obj *sourcev1.HelmRepo // 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 -} diff --git a/internal/helm/getter/client_opts_test.go b/internal/helm/getter/client_opts_test.go index bf40e7f86..a524cd05e 100644 --- a/internal/helm/getter/client_opts_test.go +++ b/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)) From 12a1464306e0fe2fba7ce935da1f7de4685b0807 Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Fri, 3 Oct 2025 01:04:06 +0300 Subject: [PATCH 2/2] e2e: Delete Bitnami deps Signed-off-by: Stefan Prodan --- hack/ci/e2e.sh | 59 -------------------------------------------------- 1 file changed, 59 deletions(-) diff --git a/hack/ci/e2e.sh b/hack/ci/e2e.sh index b00eda00c..ba7c4a6c1 100755 --- a/hack/ci/e2e.sh +++ b/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