Skip to content

Commit 9950f56

Browse files
cappyzawacappyzawa
committed
migrate HelmRepository to AuthMethodsFromSecret API
This commit upgrades pkg/runtime to v0.73.0 and adopts the unified AuthMethodsFromSecret API for HelmRepository authentication handling. The change replaces complex manual authentication detection with a single API call and improves error handling consistency. Breaking Changes: - TLS certificate validation is now strictly enforced. Invalid CA certificates will cause authentication failures even for public repositories, where they were previously ignored. - Empty TLS certificate secrets now trigger validation errors instead of being silently ignored. This affects certSecretRef with empty Data map - previously ignored, now causes proper error. Signed-off-by: cappyzawa <[email protected]>
1 parent 274a669 commit 9950f56

File tree

8 files changed

+556
-628
lines changed

8 files changed

+556
-628
lines changed

‎go.mod

Lines changed: 107 additions & 106 deletions
Large diffs are not rendered by default.

‎go.sum

Lines changed: 264 additions & 272 deletions
Large diffs are not rendered by default.

‎internal/controller/helmchart_controller_test.go

Lines changed: 5 additions & 5 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 '/invalid'")},
1038+
wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret: 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 '/invalid'"),
1043+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret: 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 '/invalid'")},
1307+
wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret: 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 '/invalid'"),
1312+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret: secrets \"invalid\" not found"),
13131313
}))
13141314
},
13151315
},
@@ -2515,7 +2515,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
25152515
},
25162516
},
25172517
assertConditions: []metav1.Condition{
2518-
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to construct Helm client's TLS config: cannot append certificate into certificate pool: invalid CA certificate"),
2518+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to construct Helm client's TLS config: failed to parse CA certificate"),
25192519
},
25202520
},
25212521
{

‎internal/controller/helmrepository_controller_test.go

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -426,10 +426,11 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
426426
assertConditions []metav1.Condition
427427
}{
428428
{
429-
name: "HTTPS with certSecretRef pointing to non-matching CA cert but public repo URL succeeds",
429+
name: "HTTPS with certSecretRef pointing to non-matching CA cert but public repo URL fails",
430430
protocol: "http",
431431
url: "https://stefanprodan.github.io/podinfo",
432-
want: sreconcile.ResultSuccess,
432+
want: sreconcile.ResultEmpty,
433+
wantErr: true,
433434
secret: &corev1.Secret{
434435
ObjectMeta: metav1.ObjectMeta{
435436
Name: "ca-file",
@@ -441,10 +442,19 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
441442
},
442443
beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository) {
443444
obj.Spec.CertSecretRef = &meta.LocalObjectReference{Name: "ca-file"}
445+
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
446+
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
444447
},
445448
assertConditions: []metav1.Condition{
446-
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
447-
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
449+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, meta.FailedReason, "tls: failed to verify certificate: x509: certificate signed by unknown authority"),
450+
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
451+
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
452+
},
453+
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
454+
// No repo index due to fetch fail.
455+
t.Expect(chartRepo.Path).To(BeEmpty())
456+
t.Expect(chartRepo.Index).To(BeNil())
457+
t.Expect(artifact.Revision).To(BeEmpty())
448458
},
449459
},
450460
{
@@ -658,7 +668,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
658668
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "basic-auth"}
659669
},
660670
revFunc: func(t *WithT, server *helmtestserver.HelmServer, secret *corev1.Secret) digest.Digest {
661-
username, password, err := secrets.BasicAuthFromSecret(context.TODO(), secret)
671+
basicAuth, err := secrets.BasicAuthFromSecret(context.TODO(), secret)
662672
t.Expect(err).ToNot(HaveOccurred())
663673

664674
serverURL := server.URL()
@@ -667,7 +677,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
667677

668678
getterOpts := []helmgetter.Option{
669679
helmgetter.WithURL(repoURL),
670-
helmgetter.WithBasicAuth(username, password),
680+
helmgetter.WithBasicAuth(basicAuth.Username, basicAuth.Password),
671681
}
672682

673683
chartRepo, err := repository.NewChartRepository(repoURL, "", testGetters, nil, getterOpts...)
@@ -713,7 +723,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
713723
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "basic-auth"}
714724
},
715725
revFunc: func(t *WithT, server *helmtestserver.HelmServer, secret *corev1.Secret) digest.Digest {
716-
username, password, err := secrets.BasicAuthFromSecret(context.TODO(), secret)
726+
basicAuth, err := secrets.BasicAuthFromSecret(context.TODO(), secret)
717727
t.Expect(err).ToNot(HaveOccurred())
718728

719729
serverURL := server.URL()
@@ -722,7 +732,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
722732

723733
getterOpts := []helmgetter.Option{
724734
helmgetter.WithURL(repoURL),
725-
helmgetter.WithBasicAuth(username, password),
735+
helmgetter.WithBasicAuth(basicAuth.Username, basicAuth.Password),
726736
}
727737

728738
chartRepo, err := repository.NewChartRepository(repoURL, "", testGetters, nil, getterOpts...)
@@ -769,7 +779,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
769779
},
770780
wantErr: true,
771781
assertConditions: []metav1.Condition{
772-
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid CA certificate"),
782+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to construct Helm client's TLS config: failed to parse CA certificate"),
773783
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
774784
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
775785
},
@@ -864,7 +874,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
864874
},
865875
wantErr: true,
866876
assertConditions: []metav1.Condition{
867-
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "required fields 'username' and 'password"),
877+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "secret 'default/malformed-basic-auth': malformed basic auth - has 'username' but missing 'password'"),
868878
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
869879
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
870880
},

0 commit comments

Comments
 (0)