Skip to content

Commit df06de9

Browse files
committed
refactor: eliminate hidden mutations in HelmRepository client opts
Replace hidden mutation functions with explicit configuration pattern to eliminate side effects where ClientOpts was modified through configuration functions. Adds CertsTempDir field to ClientOpts struct. Signed-off-by: cappyzawa <[email protected]>
1 parent a0b4969 commit df06de9

File tree

5 files changed

+125
-125
lines changed

5 files changed

+125
-125
lines changed

internal/controller/helmchart_controller.go

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

526-
clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL)
526+
clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL)
527527
if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) {
528528
e := serror.NewGeneric(
529529
err,
@@ -532,9 +532,9 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
532532
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
533533
return sreconcile.ResultEmpty, e
534534
}
535-
if certsTmpDir != "" {
535+
if clientOpts.CertsTempDir != "" {
536536
defer func() {
537-
if err := os.RemoveAll(certsTmpDir); err != nil {
537+
if err := os.RemoveAll(clientOpts.CertsTempDir); err != nil {
538538
r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
539539
"failed to delete temporary certificates directory: %s", err)
540540
}
@@ -1018,7 +1018,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
10181018
ctxTimeout, cancel := context.WithTimeout(ctx, obj.GetTimeout())
10191019
defer cancel()
10201020

1021-
clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL)
1021+
clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL)
10221022
if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) {
10231023
return nil, err
10241024
}
@@ -1037,7 +1037,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
10371037
ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters),
10381038
repository.WithOCIGetterOptions(getterOpts),
10391039
repository.WithOCIRegistryClient(registryClient),
1040-
repository.WithCertificatesStore(certsTmpDir),
1040+
repository.WithCertificatesStore(clientOpts.CertsTempDir),
10411041
repository.WithCredentialsFile(credentialsFile))
10421042
if err != nil {
10431043
errs = append(errs, fmt.Errorf("failed to create OCI chart repository: %w", err))

internal/controller/helmchart_controller_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,12 +1035,12 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) {
10351035
}
10361036
},
10371037
want: sreconcile.ResultEmpty,
1038-
wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret: secrets \"invalid\" not found")},
1038+
wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret '/invalid': secrets \"invalid\" not found")},
10391039
assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) {
10401040
g.Expect(build.Complete()).To(BeFalse())
10411041

10421042
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
1043-
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret: secrets \"invalid\" not found"),
1043+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret '/invalid': secrets \"invalid\" not found"),
10441044
}))
10451045
},
10461046
},
@@ -1304,12 +1304,12 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
13041304
}
13051305
},
13061306
want: sreconcile.ResultEmpty,
1307-
wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret: secrets \"invalid\" not found")},
1307+
wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret '/invalid': secrets \"invalid\" not found")},
13081308
assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) {
13091309
g.Expect(build.Complete()).To(BeFalse())
13101310

13111311
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
1312-
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret: secrets \"invalid\" not found"),
1312+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret '/invalid': secrets \"invalid\" not found"),
13131313
}))
13141314
},
13151315
},

internal/controller/helmrepository_controller.go

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

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

0 commit comments

Comments
 (0)