helmrepo: fix Secret type check for TLS via .spec.secretRef

This is a regression fix introduced in a302c71 which would wrongly check
for the type of the Secret specified in `.spec.secretRef` while
configuring TLS data.

This moves the Secret type check after checking whether the Secret has
any TLS data, to avoid unnecessary type enforcement.

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>

internal/controller/helmrepository_controller_test.go

@@ -550,6 +550,38 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				t.Expect(artifact.Revision).ToNot(BeEmpty())
 			},
 		},
+		{
+			// Regression test for: https://github.com/fluxcd/source-controller/issues/1218
+			name:     "HTTP with docker config secretRef makes ArtifactOutdated=true",
+			protocol: "http",
+			server: options{
+				username: "git",
+				password: "1234",
+			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "basic-auth",
+				},
+				Data: map[string][]byte{
+					"username": []byte("git"),
+					"password": []byte("1234"),
+				},
+				Type: corev1.SecretTypeDockerConfigJson,
+			},
+			beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
+				obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "basic-auth"}
+			},
+			want: sreconcile.ResultSuccess,
+			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"),
+			},
+			afterFunc: func(t *WithT, obj *helmv1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
+				t.Expect(chartRepo.Path).ToNot(BeEmpty())
+				t.Expect(chartRepo.Index).ToNot(BeNil())
+				t.Expect(artifact.Revision).ToNot(BeEmpty())
+			},
+		},
 		{
 			name:     "HTTPS with invalid CAFile in certSecretRef makes FetchFailed=True and returns error",
 			protocol: "https",

internal/tls/config.go

@@ -76,15 +76,6 @@ func TLSClientConfigFromSecret(secret corev1.Secret, url string) (*tls.Config, *
 // The keys should adhere to a single convention, i.e. a Secret with tls.key
 // and certFile is invalid.
 func tlsClientConfigFromSecret(secret corev1.Secret, url string, kubernetesTLSKeys bool) (*tls.Config, *TLSBytes, error) {
-	// Only Secrets of type Opaque and TLS are allowed. We also allow Secrets with a blank
-	// type, to avoid having to specify the type of the Secret for every test case.
-	// Since a real Kubernetes Secret is of type Opaque by default, its safe to allow this.
-	switch secret.Type {
-	case corev1.SecretTypeOpaque, corev1.SecretTypeTLS, "":
-	default:
-		return nil, nil, fmt.Errorf("cannot use secret '%s' to construct TLS config: invalid secret type: '%s'", secret.Name, secret.Type)
-	}
-
 	var certBytes, keyBytes, caBytes []byte
 	if kubernetesTLSKeys {
 		certBytes, keyBytes, caBytes = secret.Data[corev1.TLSCertKey], secret.Data[corev1.TLSPrivateKeyKey], secret.Data[CACrtKey]
@@ -100,6 +91,15 @@ func tlsClientConfigFromSecret(secret corev1.Secret, url string, kubernetesTLSKe
 			secret.Name)
 	}
 
+	// Only Secrets of type Opaque and TLS are allowed. We also allow Secrets with a blank
+	// type, to avoid having to specify the type of the Secret for every test case.
+	// Since a real Kubernetes Secret is of type Opaque by default, its safe to allow this.
+	switch secret.Type {
+	case corev1.SecretTypeOpaque, corev1.SecretTypeTLS, "":
+	default:
+		return nil, nil, fmt.Errorf("cannot use secret '%s' to construct TLS config: invalid secret type: '%s'", secret.Name, secret.Type)
+	}
+
 	tlsConf := &tls.Config{
 		MinVersion: tls.VersionTLS12,
 	}

internal/tls/config_test.go

@@ -86,8 +86,15 @@ func Test_tlsClientConfigFromSecret(t *testing.T) {
 			wantNil: true,
 		},
 		{
-			name:    "invalid secret type",
+			name:    "invalid secret type with no TLS data",
 			secret:  corev1.Secret{Type: corev1.SecretTypeDockerConfigJson},
+			wantNil: true,
+		},
+		{
+			name:    "invalid secret type",
+			secret:  kubernetesTlsSecretFixture,
+			modify:  func(s *corev1.Secret) { s.Type = corev1.SecretTypeDockerConfigJson },
+			tlsKeys: true,
 			wantErr: true,
 			wantNil: true,
 		},