feat: migrate HelmRepository to runtime/secrets v0.66.0 with strict BasicAuth validation

Implement switch-case pattern for handling secret reference combinations and add strict BasicAuth validation to prevent security issues. Replace hardcoded keys with runtime/secrets constraints and refactor GetClientOpts into smaller focused functions. Eliminate duplicate Secret fetches and update tests to match runtime/secrets v0.66.0 error message format.

BREAKING CHANGE:
- SecretRef with partial BasicAuth data (username-only or password-only) now returns an error instead of silently falling back to other authentication methods
- Invalid CA certificates now cause authentication failures even for public repositories

These changes improve security by preventing unintended authentication bypass.

Signed-off-by: cappyzawa <[email protected]>

Author: cappyzawa

internal/controller/helmchart_controller_test.go

@@ -1035,12 +1035,12 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) {
 				}
 			},
 			want:    sreconcile.ResultEmpty,
-			wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret '/invalid'")},
+			wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret: secrets \"invalid\" not found")},
 			assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) {
 				g.Expect(build.Complete()).To(BeFalse())
 
 				g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
-					*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret '/invalid'"),
+					*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret: secrets \"invalid\" not found"),
 				}))
 			},
 		},
@@ -1304,12 +1304,12 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
 				}
 			},
 			want:    sreconcile.ResultEmpty,
-			wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret '/invalid'")},
+			wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret: secrets \"invalid\" not found")},
 			assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) {
 				g.Expect(build.Complete()).To(BeFalse())
 
 				g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
-					*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret '/invalid'"),
+					*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret: secrets \"invalid\" not found"),
 				}))
 			},
 		},
@@ -2515,7 +2515,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 				},
 			},
 			assertConditions: []metav1.Condition{
-				*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"),
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to construct Helm client's TLS config: failed to parse CA certificate"),
 			},
 		},
 		{

internal/controller/helmrepository_controller_test.go

@@ -426,10 +426,11 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 		assertConditions []metav1.Condition
 	}{
 		{
-			name:     "HTTPS with certSecretRef pointing to non-matching CA cert but public repo URL succeeds",
+			name:     "HTTPS with certSecretRef pointing to non-matching CA cert but public repo URL fails",
 			protocol: "http",
 			url:      "https://stefanprodan.github.io/podinfo",
-			want:     sreconcile.ResultSuccess,
+			want:     sreconcile.ResultEmpty,
+			wantErr:  true,
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "ca-file",
@@ -441,10 +442,18 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			},
 			beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository) {
 				obj.Spec.CertSecretRef = &meta.LocalObjectReference{Name: "ca-file"}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
 			},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
-				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, meta.FailedReason, "tls: failed to verify certificate: x509: certificate signed by unknown authority"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
+			},
+			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
+				t.Expect(chartRepo.Path).To(BeEmpty())
+				t.Expect(chartRepo.Index).To(BeNil())
+				t.Expect(artifact.Revision).To(BeEmpty())
 			},
 		},
 		{
@@ -769,12 +778,11 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			},
 			wantErr: true,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid CA certificate"),
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to construct Helm client's TLS config: failed to parse CA certificate"),
 				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
 				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
-				// No repo index due to fetch fail.
 				t.Expect(chartRepo.Path).To(BeEmpty())
 				t.Expect(chartRepo.Index).To(BeNil())
 				t.Expect(artifact.Revision).To(BeEmpty())
@@ -796,7 +804,6 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
-				// No repo index due to fetch fail.
 				t.Expect(chartRepo.Path).To(BeEmpty())
 				t.Expect(chartRepo.Index).To(BeNil())
 				t.Expect(artifact.Revision).To(BeEmpty())
@@ -818,7 +825,6 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
-				// No repo index due to fetch fail.
 				t.Expect(chartRepo.Path).To(BeEmpty())
 				t.Expect(chartRepo.Index).To(BeNil())
 				t.Expect(artifact.Revision).To(BeEmpty())
@@ -839,14 +845,13 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
-				// No repo index due to fetch fail.
 				t.Expect(chartRepo.Path).To(BeEmpty())
 				t.Expect(chartRepo.Index).To(BeNil())
 				t.Expect(artifact.Revision).To(BeEmpty())
 			},
 		},
 		{
-			name:     "Malformed secret returns FetchFailed=True and returns error",
+			name:     "Malformed Basic Auth secret returns FetchFailed=True and returns error",
 			protocol: "http",
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
@@ -864,12 +869,11 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			},
 			wantErr: true,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "required fields 'username' and 'password"),
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to configure Basic Auth: secret 'default/malformed-basic-auth': key 'password' not found"),
 				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
 				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
-				// No repo index due to fetch fail.
 				t.Expect(chartRepo.Path).To(BeEmpty())
 				t.Expect(chartRepo.Index).To(BeNil())
 				t.Expect(artifact.Revision).To(BeEmpty())