Skip to content

Commit bcbe7ed

Browse files
stefanprodanstefanprodan
committed
helm: Remove the TLS tmp dir logic
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
1 parent 326a22a commit bcbe7ed

File tree

4 files changed

+18
-113
lines changed

4 files changed

+18
-113
lines changed

‎internal/controller/helmchart_controller.go‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
524524
return chartRepoConfigErrorReturn(err, obj)
525525
}
526526

527-
clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL)
527+
clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL)
528528
if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) {
529529
e := serror.NewGeneric(
530530
err,
@@ -533,14 +533,6 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
533533
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
534534
return sreconcile.ResultEmpty, e
535535
}
536-
if certsTmpDir != "" {
537-
defer func() {
538-
if err := os.RemoveAll(certsTmpDir); err != nil {
539-
r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
540-
"failed to delete temporary certificates directory: %s", err)
541-
}
542-
}()
543-
}
544536

545537
getterOpts := clientOpts.GetterOpts
546538

@@ -1019,7 +1011,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
10191011
ctxTimeout, cancel := context.WithTimeout(ctx, obj.GetTimeout())
10201012
defer cancel()
10211013

1022-
clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL)
1014+
clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL)
10231015
if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) {
10241016
return nil, err
10251017
}
@@ -1038,7 +1030,6 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
10381030
ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters),
10391031
repository.WithOCIGetterOptions(getterOpts),
10401032
repository.WithOCIRegistryClient(registryClient),
1041-
repository.WithCertificatesStore(certsTmpDir),
10421033
repository.WithCredentialsFile(credentialsFile))
10431034
if err != nil {
10441035
errs = append(errs, fmt.Errorf("failed to create OCI chart repository: %w", err))

‎internal/controller/helmrepository_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
416416
return sreconcile.ResultEmpty, e
417417
}
418418

419-
clientOpts, _, err := getter.GetClientOpts(ctx, r.Client, obj, normalizedURL)
419+
clientOpts, err := getter.GetClientOpts(ctx, r.Client, obj, normalizedURL)
420420
if err != nil {
421421
if errors.Is(err, getter.ErrDeprecatedTLSConfig) {
422422
ctrl.LoggerFrom(ctx).

‎internal/helm/getter/client_opts.go‎

Lines changed: 12 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ import (
2121
"crypto/tls"
2222
"errors"
2323
"fmt"
24-
"os"
25-
"path"
2624

2725
"github.com/google/go-containerregistry/pkg/authn"
2826
helmgetter "helm.sh/helm/v3/pkg/getter"
@@ -69,7 +67,7 @@ func (o ClientOpts) MustLoginToRegistry() bool {
6967
// auth mechanisms.
7068
// 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
7169
// caller's responsibility to clean up the directory.
72-
func GetClientOpts(ctx context.Context, c client.Client, obj *sourcev1.HelmRepository, url string) (*ClientOpts, string, error) {
70+
func GetClientOpts(ctx context.Context, c client.Client, obj *sourcev1.HelmRepository, url string) (*ClientOpts, error) {
7371
// This function configures authentication for Helm repositories based on the provided secrets:
7472
// - CertSecretRef: TLS client certificates (always takes priority)
7573
// - SecretRef: Can contain Basic Auth or TLS certificates (deprecated)
@@ -84,17 +82,16 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *sourcev1.HelmRepos
8482
}
8583

8684
// Process secrets and configure authentication
87-
deprecatedTLS, certSecret, authSecret, err := configureAuthentication(ctx, c, obj, opts, url)
85+
deprecatedTLS, _, authSecret, err := configureAuthentication(ctx, c, obj, opts)
8886
if err != nil {
89-
return nil, "", err
87+
return nil, err
9088
}
9189

9290
// Setup OCI registry specific configurations if needed
93-
var tempCertDir string
9491
if obj.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
95-
tempCertDir, err = configureOCIRegistryWithSecrets(ctx, obj, opts, url, certSecret, authSecret)
92+
err = configureOCIRegistryWithSecrets(ctx, obj, opts, url, authSecret)
9693
if err != nil {
97-
return nil, "", err
94+
return nil, err
9895
}
9996
}
10097

@@ -103,15 +100,15 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *sourcev1.HelmRepos
103100
deprecatedErr = ErrDeprecatedTLSConfig
104101
}
105102

106-
return opts, tempCertDir, deprecatedErr
103+
return opts, deprecatedErr
107104
}
108105

109106
// configureAuthentication processes all secret references and sets up authentication.
110107
// Returns (deprecatedTLS, certSecret, authSecret, error) where:
111108
// - deprecatedTLS: true if TLS config comes from SecretRef (deprecated pattern)
112109
// - certSecret: the secret from CertSecretRef (nil if not specified)
113110
// - authSecret: the secret from SecretRef (nil if not specified)
114-
func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1.HelmRepository, opts *ClientOpts, url string) (bool, *corev1.Secret, *corev1.Secret, error) {
111+
func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1.HelmRepository, opts *ClientOpts) (bool, *corev1.Secret, *corev1.Secret, error) {
115112
var deprecatedTLS bool
116113
var certSecret, authSecret *corev1.Secret
117114

@@ -171,12 +168,12 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
171168
}
172169

173170
// configureOCIRegistryWithSecrets sets up OCI-specific configurations using pre-fetched secrets
174-
func configureOCIRegistryWithSecrets(ctx context.Context, obj *sourcev1.HelmRepository, opts *ClientOpts, url string, certSecret, authSecret *corev1.Secret) (string, error) {
171+
func configureOCIRegistryWithSecrets(ctx context.Context, obj *sourcev1.HelmRepository, opts *ClientOpts, url string, authSecret *corev1.Secret) error {
175172
// Configure OCI authentication from authSecret if available
176173
if authSecret != nil {
177174
keychain, err := registry.LoginOptionFromSecret(url, *authSecret)
178175
if err != nil {
179-
return "", fmt.Errorf("failed to configure login options: %w", err)
176+
return fmt.Errorf("failed to configure login options: %w", err)
180177
}
181178
opts.Keychain = keychain
182179
}
@@ -185,48 +182,22 @@ func configureOCIRegistryWithSecrets(ctx context.Context, obj *sourcev1.HelmRepo
185182
if obj.Spec.SecretRef == nil && obj.Spec.Provider != "" && obj.Spec.Provider != sourcev1.GenericOCIProvider {
186183
authenticator, err := soci.OIDCAuth(ctx, url, obj.Spec.Provider)
187184
if err != nil {
188-
return "", fmt.Errorf("failed to get credential from '%s': %w", obj.Spec.Provider, err)
185+
return fmt.Errorf("failed to get credential from '%s': %w", obj.Spec.Provider, err)
189186
}
190187
opts.Authenticator = authenticator
191188
}
192189

193190
// Setup registry login options
194191
loginOpt, err := registry.NewLoginOption(opts.Authenticator, opts.Keychain, url)
195192
if err != nil {
196-
return "", err
193+
return err
197194
}
198195

199196
if loginOpt != nil {
200197
opts.RegLoginOpts = []helmreg.LoginOption{loginOpt, helmreg.LoginOptInsecure(obj.Spec.Insecure)}
201198
}
202199

203-
// Handle TLS certificate files for OCI
204-
var tempCertDir string
205-
if opts.TlsConfig != nil {
206-
tempCertDir, err = os.MkdirTemp("", "helm-repo-oci-certs")
207-
if err != nil {
208-
return "", fmt.Errorf("cannot create temporary directory: %w", err)
209-
}
210-
211-
var tlsSecret *corev1.Secret
212-
if certSecret != nil {
213-
tlsSecret = certSecret
214-
} else if authSecret != nil {
215-
tlsSecret = authSecret
216-
}
217-
218-
certFile, keyFile, caFile, err := storeTLSCertificateFilesForOCI(ctx, tlsSecret, nil, tempCertDir)
219-
if err != nil {
220-
return "", fmt.Errorf("cannot write certs files to path: %w", err)
221-
}
222-
223-
tlsLoginOpt := registry.TLSLoginOption(certFile, keyFile, caFile)
224-
if tlsLoginOpt != nil {
225-
opts.RegLoginOpts = append(opts.RegLoginOpts, tlsLoginOpt)
226-
}
227-
}
228-
229-
return tempCertDir, nil
200+
return nil
230201
}
231202

232203
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) (
240211
}
241212
return &secret, nil
242213
}
243-
244-
// storeTLSCertificateFilesForOCI writes TLS certificate data from secrets to files for OCI registry authentication.
245-
// Helm OCI registry client requires certificate file paths rather than in-memory data,
246-
// so we need to temporarily write the certificate data to disk.
247-
// Returns paths to the written cert, key, and CA files (any of which may be empty if not present).
248-
func storeTLSCertificateFilesForOCI(ctx context.Context, certSecret, authSecret *corev1.Secret, path string) (string, string, string, error) {
249-
var (
250-
certFile string
251-
keyFile string
252-
caFile string
253-
err error
254-
)
255-
256-
// Try to get TLS data from certSecret first, then authSecret
257-
var tlsSecret *corev1.Secret
258-
if certSecret != nil {
259-
tlsSecret = certSecret
260-
} else if authSecret != nil {
261-
tlsSecret = authSecret
262-
}
263-
264-
if tlsSecret != nil {
265-
if certData, exists := tlsSecret.Data[secrets.KeyTLSCert]; exists {
266-
if keyData, keyExists := tlsSecret.Data[secrets.KeyTLSPrivateKey]; keyExists {
267-
certFile, err = writeToFile(certData, certFileName, path)
268-
if err != nil {
269-
return "", "", "", err
270-
}
271-
keyFile, err = writeToFile(keyData, keyFileName, path)
272-
if err != nil {
273-
return "", "", "", err
274-
}
275-
}
276-
}
277-
278-
if caData, exists := tlsSecret.Data[secrets.KeyCACert]; exists {
279-
caFile, err = writeToFile(caData, caFileName, path)
280-
if err != nil {
281-
return "", "", "", err
282-
}
283-
}
284-
}
285-
286-
return certFile, keyFile, caFile, nil
287-
}
288-
289-
func writeToFile(data []byte, filename, tmpDir string) (string, error) {
290-
file := path.Join(tmpDir, filename)
291-
err := os.WriteFile(file, data, 0o600)
292-
if err != nil {
293-
return "", err
294-
}
295-
return file, nil
296-
}

‎internal/helm/getter/client_opts_test.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func TestGetClientOpts(t *testing.T) {
164164
}
165165
c := clientBuilder.Build()
166166

167-
clientOpts, _, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy")
167+
clientOpts, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy")
168168
if tt.err != nil {
169169
g.Expect(err).To(Equal(tt.err))
170170
} else {
@@ -207,7 +207,7 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) {
207207
"password": []byte("pass"),
208208
},
209209
},
210-
loginOptsN: 3,
210+
loginOptsN: 2,
211211
},
212212
{
213213
name: "without caFile",
@@ -271,7 +271,7 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) {
271271
}
272272
c := clientBuilder.Build()
273273

274-
clientOpts, tmpDir, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy")
274+
clientOpts, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy")
275275
if tt.wantErrMsg != "" {
276276
if err == nil {
277277
t.Errorf("GetClientOpts() expected error but got none")
@@ -287,9 +287,6 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) {
287287
t.Errorf("GetClientOpts() error = %v", err)
288288
return
289289
}
290-
if tmpDir != "" {
291-
defer os.RemoveAll(tmpDir)
292-
}
293290
if tt.loginOptsN != len(clientOpts.RegLoginOpts) {
294291
// we should have a login option but no TLS option
295292
t.Errorf("expected length of %d for clientOpts.RegLoginOpts but got %d", tt.loginOptsN, len(clientOpts.RegLoginOpts))

0 commit comments

Comments
 (0)