refactoring and fix tests

Signed-off-by: Soule BA <soule@weave.works>

docs/spec/v1beta2/helmrepositories.md

@@ -459,10 +459,16 @@ a deprecation warning will be logged.
 
 ### Cert secret reference
 
+<<<<<<< HEAD
 **Note:** TLS authentication is not yet supported by OCI Helm repositories.
 
 `.spec.certSecretRef.name` is an optional field to specify a secret containing TLS
 certificate data. The secret can contain the following keys:
+=======
+To provide TLS credentials to use while connecting with the Helm repository,
+the referenced Secret is expected to contain `.data.certFile` and
+`.data.keyFile`, and/or `.data.caFile` values.
+>>>>>>> 3df4c49 (refactoring and fix tests)
 
 * `certFile` and `keyFile`, to specify the client certificate and private key used for
 TLS client authentication. These must be used in conjunction, i.e. specifying one without
@@ -509,6 +515,28 @@ data:
   caFile: <BASE64>
 ```
 
+#### Provide TLS credentials in a secret of type kubernetes.io/dockerconfigjson
+
+For OCI Helm repositories, Kubernetes secrets of type [kubernetes.io/dockerconfigjson](https://kubernetes.io/docs/concepts/configuration/secret/#secret-types)
+are also supported. It is possible to append TLS credentials to the secret data.
+
+For example:
+
+```yaml
+apiVersion: v1
+kind: Secret
+metadata:
+  name: example-tls
+  namespace: default
+type: kubernetes.io/dockerconfigjson
+data:
+  .dockerconfigjson: <BASE64>
+  certFile: <BASE64>
+  keyFile: <BASE64>
+  # NOTE: Can be supplied without the above values
+  caFile: <BASE64>
+```
+
 ### Pass credentials
 
 `.spec.passCredentials` is an optional field to allow the credentials from the

internal/controller/helmchart_controller_test.go

@@ -1069,7 +1069,7 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
 	g.Expect(err).NotTo(HaveOccurred())
 
 	// Upload the test chart
-	metadata, err := loadTestChartToOCI(chartData, chartPath, testRegistryServer)
+	metadata, err := loadTestChartToOCI(chartData, testRegistryServer, "", "", "")
 	g.Expect(err).NotTo(HaveOccurred())
 
 	storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords)
@@ -2197,14 +2197,15 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 	type secretOptions struct {
 		username string
 		password string
-		ca       []byte
 	}
 
 	tests := []struct {
 		name             string
 		url              string
 		registryOpts     registryOptions
 		secretOpts       secretOptions
+		secret           *corev1.Secret
+		withTLS          bool
 		provider         string
 		providerImg      string
 		want             sreconcile.Result
@@ -2229,6 +2230,13 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 				username: testRegistryUsername,
 				password: testRegistryPassword,
 			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "auth-secretref",
+				},
+				Type: corev1.SecretTypeDockerConfigJson,
+				Data: map[string][]byte{},
+			},
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
 				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
@@ -2245,6 +2253,13 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 				username: "wrong-pass",
 				password: "wrong-pass",
 			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "auth-secretref",
+				},
+				Type: corev1.SecretTypeDockerConfigJson,
+				Data: map[string][]byte{},
+			},
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to login to OCI registry"),
 			},
@@ -2268,6 +2283,13 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 				username: testRegistryUsername,
 				password: testRegistryPassword,
 			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "auth-secretref",
+				},
+				Type: corev1.SecretTypeDockerConfigJson,
+				Data: map[string][]byte{},
+			},
 			provider: "azure",
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
@@ -2278,27 +2300,47 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 			name:    "HTTPS With invalid CA cert",
 			wantErr: true,
 			registryOpts: registryOptions{
-				withBasicAuth: true,
+				withTLS: true,
 			},
+			withTLS: true,
 			secretOpts: secretOptions{
 				username: testRegistryUsername,
 				password: testRegistryPassword,
-				ca:       []byte("invalid-ca"),
+			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "auth-secretref",
+				},
+				Type: corev1.SecretTypeDockerConfigJson,
+				Data: map[string][]byte{
+					"caFile": []byte("invalid caFile"),
+				},
 			},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to login to OCI registry"),
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to create TLS client config with secret data: cannot append certificate into certificate pool: invalid caFile"),
 			},
 		},
 		{
 			name: "HTTPS With CA cert",
 			want: sreconcile.ResultSuccess,
 			registryOpts: registryOptions{
-				withBasicAuth: true,
+				withTLS: true,
 			},
+			withTLS: true,
 			secretOpts: secretOptions{
 				username: testRegistryUsername,
 				password: testRegistryPassword,
-				ca:       []byte(tlsCA),
+			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "auth-secretref",
+				},
+				Type: corev1.SecretTypeDockerConfigJson,
+				Data: map[string][]byte{
+					"caFile":   tlsCA,
+					"certFile": tlsPublicKey,
+					"keyFile":  tlsPrivateKey,
+				},
 			},
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
@@ -2319,13 +2361,16 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 
 			server, err := setupRegistryServer(ctx, workspaceDir, tt.registryOpts)
 			g.Expect(err).NotTo(HaveOccurred())
+			if tt.withTLS {
+				defer server.stopSrv()
+			}
 
 			// Load a test chart
 			chartData, err := os.ReadFile(chartPath)
 			g.Expect(err).ToNot(HaveOccurred())
 
 			// Upload the test chart
-			metadata, err := loadTestChartToOCI(chartData, chartPath, server)
+			metadata, err := loadTestChartToOCI(chartData, server, "testdata/certs/server.pem", "testdata/certs/server-key.pem", "testdata/certs/ca.pem")
 			g.Expect(err).ToNot(HaveOccurred())
 
 			repo := &helmv1.HelmRepository{
@@ -2351,40 +2396,16 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
 				repo.Spec.URL = tt.providerImg
 			}
 
-			var secret *corev1.Secret
 			if tt.secretOpts.username != "" && tt.secretOpts.password != "" {
-				secret = &corev1.Secret{
-					ObjectMeta: metav1.ObjectMeta{
-						Name: "auth-secretref",
-					},
-					Type: corev1.SecretTypeDockerConfigJson,
-					Data: map[string][]byte{
-						".dockerconfigjson": []byte(fmt.Sprintf(`{"auths": {%q: {"username": %q, "password": %q}}}`,
-							server.registryHost, tt.secretOpts.username, tt.secretOpts.password)),
-					},
-				}
-			}
-
-			if tt.secretOpts.ca != nil {
-				if secret == nil {
-					secret = &corev1.Secret{
-						ObjectMeta: metav1.ObjectMeta{
-							Name: "auth-secretref",
-						},
-						Data: map[string][]byte{
-							"caFile": tt.secretOpts.ca,
-						},
-					}
-				} else {
-					secret.Data["caFile"] = tt.secretOpts.ca
-				}
+				tt.secret.Data[".dockerconfigjson"] = []byte(fmt.Sprintf(`{"auths": {%q: {"username": %q, "password": %q}}}`,
+					server.registryHost, tt.secretOpts.username, tt.secretOpts.password))
 			}
 
-			if secret != nil {
+			if tt.secret != nil {
 				repo.Spec.SecretRef = &meta.LocalObjectReference{
-					Name: secret.Name,
+					Name: tt.secret.Name,
 				}
-				clientBuilder.WithObjects(secret, repo)
+				clientBuilder.WithObjects(tt.secret, repo)
 			} else {
 				clientBuilder.WithObjects(repo)
 			}
@@ -2457,7 +2478,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T
 	g.Expect(err).ToNot(HaveOccurred())
 
 	// Upload the test chart
-	metadata, err := loadTestChartToOCI(chartData, chartPath, server)
+	metadata, err := loadTestChartToOCI(chartData, server, "", "", "")
 	g.Expect(err).NotTo(HaveOccurred())
 
 	storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords)
@@ -2688,17 +2709,11 @@ func extractChartMeta(chartData []byte) (*hchart.Metadata, error) {
 	return ch.Metadata, nil
 }
 
-func loadTestChartToOCI(chartData []byte, chartPath string, server *registryClientTestServer) (*hchart.Metadata, error) {
+func loadTestChartToOCI(chartData []byte, server *registryClientTestServer, certFile, keyFile, cafile string) (*hchart.Metadata, error) {
 	// Login to the registry
 	err := server.registryClient.Login(server.registryHost,
 		helmreg.LoginOptBasicAuth(testRegistryUsername, testRegistryPassword),
-		helmreg.LoginOptInsecure(true))
-	if err != nil {
-		return nil, err
-	}
-
-	// Load a test chart
-	chartData, err = os.ReadFile(chartPath)
+		helmreg.LoginOptTLSClientConfig(certFile, keyFile, cafile))
 	if err != nil {
 		return nil, err
 	}