diff --git a/docs/spec/v1beta2/helmrepositories.md b/docs/spec/v1beta2/helmrepositories.md index b48f4ff4a..e121b01e3 100644 --- a/docs/spec/v1beta2/helmrepositories.md +++ b/docs/spec/v1beta2/helmrepositories.md @@ -459,8 +459,6 @@ a deprecation warning will be logged. ### Cert secret reference -**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: diff --git a/internal/controller/helmchart_controller.go b/internal/controller/helmchart_controller.go index d393fcb32..f2b9eaa18 100644 --- a/internal/controller/helmchart_controller.go +++ b/internal/controller/helmchart_controller.go @@ -512,7 +512,8 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * if err != nil { return chartRepoConfigErrorReturn(err, obj) } - clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL) + + clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL) if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) { e := &serror.Event{ Err: err, @@ -521,6 +522,15 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } + if certsTmpDir != "" { + defer func() { + if err := os.RemoveAll(certsTmpDir); err != nil { + r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason, + "failed to delete temporary certificates directory: %s", err) + } + }() + } + getterOpts := clientOpts.GetterOpts // Initialize the chart repository @@ -536,7 +546,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // this is needed because otherwise the credentials are stored in ~/.docker/config.json. // TODO@souleb: remove this once the registry move to Oras v2 // or rework to enable reusing credentials to avoid the unneccessary handshake operations - registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.RegLoginOpt != nil) + registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry()) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to construct Helm client: %w", err), @@ -585,8 +595,8 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // If login options are configured, use them to login to the registry // The OCIGetter will later retrieve the stored credentials to pull the chart - if clientOpts.RegLoginOpt != nil { - err = ociChartRepo.Login(clientOpts.RegLoginOpt) + if clientOpts.MustLoginToRegistry() { + err = ociChartRepo.Login(clientOpts.RegLoginOpts...) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to login to OCI registry: %w", err), @@ -983,7 +993,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration) defer cancel() - clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL) + clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL) if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) { return nil, err } @@ -991,7 +1001,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont var chartRepo repository.Downloader if helmreg.IsOCI(normalizedURL) { - registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.RegLoginOpt != nil) + registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry()) if err != nil { return nil, fmt.Errorf("failed to create registry client: %w", err) } @@ -1002,6 +1012,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(getterOpts), repository.WithOCIRegistryClient(registryClient), + repository.WithCertificatesStore(certsTmpDir), repository.WithCredentialsFile(credentialsFile)) if err != nil { errs = append(errs, fmt.Errorf("failed to create OCI chart repository: %w", err)) @@ -1016,8 +1027,8 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont // If login options are configured, use them to login to the registry // The OCIGetter will later retrieve the stored credentials to pull the chart - if clientOpts.RegLoginOpt != nil { - err = ociChartRepo.Login(clientOpts.RegLoginOpt) + if clientOpts.MustLoginToRegistry() { + err = ociChartRepo.Login(clientOpts.RegLoginOpts...) if err != nil { errs = append(errs, fmt.Errorf("failed to login to OCI chart repository: %w", err)) // clean up the credentialsFile diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go index c6f030170..ec067fd50 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -1109,7 +1109,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) @@ -2244,6 +2244,9 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { url string registryOpts registryOptions secretOpts secretOptions + secret *corev1.Secret + certsecret *corev1.Secret + insecure bool provider string providerImg string want sreconcile.Result @@ -2251,16 +2254,18 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { assertConditions []metav1.Condition }{ { - name: "HTTP without basic auth", - want: sreconcile.ResultSuccess, + name: "HTTP without basic auth", + want: sreconcile.ResultSuccess, + insecure: true, 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'"), }, }, { - name: "HTTP with basic auth secret", - want: sreconcile.ResultSuccess, + name: "HTTP with basic auth secret", + want: sreconcile.ResultSuccess, + insecure: true, registryOpts: registryOptions{ withBasicAuth: true, }, @@ -2268,15 +2273,23 @@ 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'"), }, }, { - name: "HTTP registry - basic auth with invalid secret", - want: sreconcile.ResultEmpty, - wantErr: true, + name: "HTTP registry - basic auth with invalid secret", + want: sreconcile.ResultEmpty, + wantErr: true, + insecure: true, registryOpts: registryOptions{ withBasicAuth: true, }, @@ -2284,6 +2297,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"), }, @@ -2291,6 +2311,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { { name: "with contextual login provider", wantErr: true, + insecure: true, provider: "aws", providerImg: "oci://123456789000.dkr.ecr.us-east-2.amazonaws.com/test", assertConditions: []metav1.Condition{ @@ -2303,16 +2324,87 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { registryOpts: registryOptions{ withBasicAuth: true, }, + insecure: true, secretOpts: secretOptions{ 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'"), *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"), }, }, + { + name: "HTTPS With invalid CA cert", + wantErr: true, + registryOpts: registryOptions{ + withTLS: true, + withClientCertAuth: true, + }, + secretOpts: secretOptions{ + username: testRegistryUsername, + password: testRegistryPassword, + }, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-secretref", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{}, + }, + certsecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "certs-secretref", + }, + Data: map[string][]byte{ + "caFile": []byte("invalid caFile"), + }, + }, + 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 caFile"), + }, + }, + { + name: "HTTPS With CA cert", + want: sreconcile.ResultSuccess, + registryOpts: registryOptions{ + withTLS: true, + withClientCertAuth: true, + }, + secretOpts: secretOptions{ + username: testRegistryUsername, + password: testRegistryPassword, + }, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-secretref", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{}, + }, + certsecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "certs-secretref", + }, + Data: map[string][]byte{ + "caFile": tlsCA, + "certFile": clientPublicKey, + "keyFile": clientPrivateKey, + }, + }, + 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'"), + }, + }, } for _, tt := range tests { @@ -2325,7 +2417,9 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { workspaceDir := t.TempDir() - tt.registryOpts.disableDNSMocking = true + if tt.insecure { + tt.registryOpts.disableDNSMocking = true + } server, err := setupRegistryServer(ctx, workspaceDir, tt.registryOpts) g.Expect(err).NotTo(HaveOccurred()) t.Cleanup(func() { @@ -2337,7 +2431,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) // Upload the test chart - metadata, err := loadTestChartToOCI(chartData, chartPath, server) + metadata, err := loadTestChartToOCI(chartData, server, "testdata/certs/client.pem", "testdata/certs/client-key.pem", "testdata/certs/ca.pem") g.Expect(err).ToNot(HaveOccurred()) repo := &helmv1.HelmRepository{ @@ -2364,25 +2458,26 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { } 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)), - }, - } + tt.secret.Data[".dockerconfigjson"] = []byte(fmt.Sprintf(`{"auths": {%q: {"username": %q, "password": %q}}}`, + server.registryHost, tt.secretOpts.username, tt.secretOpts.password)) + } + if tt.secret != nil { repo.Spec.SecretRef = &meta.LocalObjectReference{ - Name: secret.Name, + Name: tt.secret.Name, } - clientBuilder.WithObjects(secret, repo) - } else { - clientBuilder.WithObjects(repo) + clientBuilder.WithObjects(tt.secret) + } + + if tt.certsecret != nil { + repo.Spec.CertSecretRef = &meta.LocalObjectReference{ + Name: tt.certsecret.Name, + } + clientBuilder.WithObjects(tt.certsecret) } + clientBuilder.WithObjects(repo) + obj := &helmv1.HelmChart{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "auth-strategy-", @@ -2456,7 +2551,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) @@ -2687,30 +2782,24 @@ 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 + return nil, fmt.Errorf("failed to login to OCI registry: %w", err) } metadata, err := extractChartMeta(chartData) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to extract chart metadata: %w", err) } // Upload the test chart ref := fmt.Sprintf("%s/testrepo/%s:%s", server.registryHost, metadata.Name, metadata.Version) _, err = server.registryClient.Push(chartData, ref) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to push chart: %w", err) } return metadata, nil diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go index 99ace6ec4..dd75ff915 100644 --- a/internal/controller/helmrepository_controller.go +++ b/internal/controller/helmrepository_controller.go @@ -399,7 +399,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc return sreconcile.ResultEmpty, e } - clientOpts, err := getter.GetClientOpts(ctx, r.Client, obj, normalizedURL) + clientOpts, _, err := getter.GetClientOpts(ctx, r.Client, obj, normalizedURL) if err != nil { if errors.Is(err, getter.ErrDeprecatedTLSConfig) { ctrl.LoggerFrom(ctx). diff --git a/internal/controller/helmrepository_controller_oci.go b/internal/controller/helmrepository_controller_oci.go index 2752a612c..87f504bef 100644 --- a/internal/controller/helmrepository_controller_oci.go +++ b/internal/controller/helmrepository_controller_oci.go @@ -18,20 +18,18 @@ package controller import ( "context" + "crypto/tls" "errors" "fmt" "net/url" "os" "time" - "github.com/google/go-containerregistry/pkg/authn" - helmgetter "helm.sh/helm/v3/pkg/getter" helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" kerrors "k8s.io/apimachinery/pkg/util/errors" kuberecorder "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -42,7 +40,6 @@ import ( eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" "github.com/fluxcd/pkg/apis/meta" - "github.com/fluxcd/pkg/oci" "github.com/fluxcd/pkg/runtime/conditions" helper "github.com/fluxcd/pkg/runtime/controller" "github.com/fluxcd/pkg/runtime/patch" @@ -51,10 +48,9 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1" helmv1 "github.com/fluxcd/source-controller/api/v1beta2" - "github.com/fluxcd/source-controller/internal/helm/registry" + "github.com/fluxcd/source-controller/internal/helm/getter" "github.com/fluxcd/source-controller/internal/helm/repository" "github.com/fluxcd/source-controller/internal/object" - soci "github.com/fluxcd/source-controller/internal/oci" intpredicates "github.com/fluxcd/source-controller/internal/predicates" ) @@ -79,7 +75,7 @@ type HelmRepositoryOCIReconciler struct { client.Client kuberecorder.EventRecorder helper.Metrics - Getters helmgetter.Providers + ControllerName string RegistryClientGenerator RegistryClientGeneratorFunc @@ -95,7 +91,7 @@ type HelmRepositoryOCIReconciler struct { // and an optional file name. // The file is used to store the registry client credentials. // The caller is responsible for deleting the file. -type RegistryClientGeneratorFunc func(isLogin bool) (*helmreg.Client, string, error) +type RegistryClientGeneratorFunc func(tlsConfig *tls.Config, isLogin bool) (*helmreg.Client, string, error) func (r *HelmRepositoryOCIReconciler) SetupWithManager(mgr ctrl.Manager) error { return r.SetupWithManagerAndOptions(mgr, HelmRepositoryReconcilerOptions{}) @@ -226,7 +222,7 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S } // Check if it's a successful reconciliation. - if result.RequeueAfter == obj.GetRequeueAfter() && result.Requeue == false && + if result.RequeueAfter == obj.GetRequeueAfter() && !result.Requeue && retErr == nil { // Remove reconciling condition if the reconciliation was successful. conditions.Delete(obj, meta.ReconcilingCondition) @@ -305,43 +301,34 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S result, retErr = ctrl.Result{}, nil return } - conditions.Delete(obj, meta.StalledCondition) - var ( - authenticator authn.Authenticator - keychain authn.Keychain - err error - ) - // Configure any authentication related options. - if obj.Spec.SecretRef != nil { - keychain, err = authFromSecret(ctx, r.Client, obj) - if err != nil { - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error()) - result, retErr = ctrl.Result{}, err - return - } - } else if obj.Spec.Provider != helmv1.GenericOCIProvider && obj.Spec.Type == helmv1.HelmRepositoryTypeOCI { - auth, authErr := soci.OIDCAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider) - if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) { - e := fmt.Errorf("failed to get credential from %s: %w", obj.Spec.Provider, authErr) - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error()) - result, retErr = ctrl.Result{}, e - return - } - if auth != nil { - authenticator = auth - } + normalizedURL, err := repository.NormalizeURL(obj.Spec.URL) + if err != nil { + conditions.MarkStalled(obj, sourcev1.URLInvalidReason, err.Error()) + conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.URLInvalidReason, err.Error()) + result, retErr = ctrl.Result{}, nil + return } - loginOpt, err := makeLoginOption(authenticator, keychain, obj.Spec.URL) + conditions.Delete(obj, meta.StalledCondition) + + clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL) if err != nil { conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error()) result, retErr = ctrl.Result{}, err return } + if certsTmpDir != "" { + defer func() { + if err := os.RemoveAll(certsTmpDir); err != nil { + r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason, + "failed to delete temporary certs directory: %s", err) + } + }() + } // Create registry client and login if needed. - registryClient, file, err := r.RegistryClientGenerator(loginOpt != nil) + registryClient, file, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry()) if err != nil { e := fmt.Errorf("failed to create registry client: %w", err) conditions.MarkFalse(obj, meta.ReadyCondition, meta.FailedReason, e.Error()) @@ -368,8 +355,8 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S conditions.Delete(obj, meta.StalledCondition) // Attempt to login to the registry if credentials are provided. - if loginOpt != nil { - err = chartRepo.Login(loginOpt) + if clientOpts.MustLoginToRegistry() { + err = chartRepo.Login(clientOpts.RegLoginOpts...) if err != nil { e := fmt.Errorf("failed to login to registry '%s': %w", obj.Spec.URL, err) conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error()) @@ -411,41 +398,6 @@ func (r *HelmRepositoryOCIReconciler) eventLogf(ctx context.Context, obj runtime r.Eventf(obj, eventType, reason, msg) } -// authFromSecret returns an authn.Keychain for the given HelmRepository. -// If the HelmRepository does not specify a secretRef, an anonymous keychain is returned. -func authFromSecret(ctx context.Context, client client.Client, obj *helmv1.HelmRepository) (authn.Keychain, error) { - // Attempt to retrieve secret. - name := types.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: obj.Spec.SecretRef.Name, - } - var secret corev1.Secret - if err := client.Get(ctx, name, &secret); err != nil { - return nil, fmt.Errorf("failed to get secret '%s': %w", name.String(), err) - } - - // Construct login options. - keychain, err := registry.LoginOptionFromSecret(obj.Spec.URL, secret) - if err != nil { - return nil, fmt.Errorf("failed to configure Helm client with secret data: %w", err) - } - return keychain, nil -} - -// makeLoginOption returns a registry login option for the given HelmRepository. -// If the HelmRepository does not specify a secretRef, a nil login option is returned. -func makeLoginOption(auth authn.Authenticator, keychain authn.Keychain, registryURL string) (helmreg.LoginOption, error) { - if auth != nil { - return registry.AuthAdaptHelper(auth) - } - - if keychain != nil { - return registry.KeychainAdaptHelper(keychain)(registryURL) - } - - return nil, nil -} - func conditionsDiff(a, b []string) []string { bMap := make(map[string]struct{}, len(b)) for _, j := range b { diff --git a/internal/controller/helmrepository_controller_oci_test.go b/internal/controller/helmrepository_controller_oci_test.go index 88f1c0aaf..d1252e709 100644 --- a/internal/controller/helmrepository_controller_oci_test.go +++ b/internal/controller/helmrepository_controller_oci_test.go @@ -205,7 +205,10 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { name string url string registryOpts registryOptions + insecure bool secretOpts secretOptions + secret *corev1.Secret + certsSecret *corev1.Secret provider string providerImg string want ctrl.Result @@ -220,8 +223,9 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { }, }, { - name: "HTTP with basic auth secret", - want: ctrl.Result{RequeueAfter: interval}, + name: "HTTP with basic auth secret", + want: ctrl.Result{RequeueAfter: interval}, + insecure: true, registryOpts: registryOptions{ withBasicAuth: true, }, @@ -229,14 +233,22 @@ func TestHelmRepositoryOCIReconciler_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.ReadyCondition, meta.SucceededReason, "Helm repository is ready"), }, }, { - name: "HTTP registry - basic auth with invalid secret", - want: ctrl.Result{}, - wantErr: true, + name: "HTTP registry - basic auth with invalid secret", + want: ctrl.Result{}, + wantErr: true, + insecure: true, registryOpts: registryOptions{ withBasicAuth: true, }, @@ -244,6 +256,13 @@ func TestHelmRepositoryOCIReconciler_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(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation"), *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to login to registry"), @@ -252,6 +271,7 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { { name: "with contextual login provider", wantErr: true, + insecure: true, provider: "aws", providerImg: "oci://123456789000.dkr.ecr.us-east-2.amazonaws.com/test", assertConditions: []metav1.Condition{ @@ -265,15 +285,86 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { registryOpts: registryOptions{ withBasicAuth: true, }, + insecure: true, secretOpts: secretOptions{ 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.ReadyCondition, meta.SucceededReason, "Helm repository is ready"), }, }, + { + name: "HTTPS With invalid CA cert", + wantErr: true, + registryOpts: registryOptions{ + withTLS: true, + withClientCertAuth: true, + }, + secretOpts: secretOptions{ + username: testRegistryUsername, + password: testRegistryPassword, + }, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-secretref", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{}, + }, + certsSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "certs-secretref", + }, + Data: map[string][]byte{ + "caFile": []byte("invalid caFile"), + }, + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation 0 -> 1"), + *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid caFile"), + }, + }, + { + name: "HTTPS With CA cert", + want: ctrl.Result{RequeueAfter: interval}, + registryOpts: registryOptions{ + withTLS: true, + withClientCertAuth: true, + }, + secretOpts: secretOptions{ + username: testRegistryUsername, + password: testRegistryPassword, + }, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-secretref", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{}, + }, + certsSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "certs-secretref", + }, + Data: map[string][]byte{ + "caFile": tlsCA, + "certFile": clientPublicKey, + "keyFile": clientPrivateKey, + }, + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"), + }, + }, } for _, tt := range tests { @@ -285,7 +376,9 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { WithStatusSubresource(&helmv1.HelmRepository{}) workspaceDir := t.TempDir() - tt.registryOpts.disableDNSMocking = true + if tt.insecure { + tt.registryOpts.disableDNSMocking = true + } server, err := setupRegistryServer(ctx, workspaceDir, tt.registryOpts) g.Expect(err).NotTo(HaveOccurred()) t.Cleanup(func() { @@ -317,28 +410,27 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { } 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)), - }, - } - - clientBuilder.WithObjects(secret) + tt.secret.Data[".dockerconfigjson"] = []byte(fmt.Sprintf(`{"auths": {%q: {"username": %q, "password": %q}}}`, + server.registryHost, tt.secretOpts.username, tt.secretOpts.password)) + } + if tt.secret != nil { + clientBuilder.WithObjects(tt.secret) obj.Spec.SecretRef = &meta.LocalObjectReference{ - Name: secret.Name, + Name: tt.secret.Name, + } + } + + if tt.certsSecret != nil { + clientBuilder.WithObjects(tt.certsSecret) + obj.Spec.CertSecretRef = &meta.LocalObjectReference{ + Name: tt.certsSecret.Name, } } r := &HelmRepositoryOCIReconciler{ Client: clientBuilder.Build(), EventRecorder: record.NewFakeRecorder(32), - Getters: testGetters, RegistryClientGenerator: registry.ClientGenerator, patchOptions: getPatchOptions(helmRepositoryOCIOwnedConditions, "sc"), } @@ -349,7 +441,6 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { }() sp := patch.NewSerialPatcher(obj, r.Client) - got, err := r.reconcile(ctx, sp, obj) g.Expect(err != nil).To(Equal(tt.wantErr)) g.Expect(got).To(Equal(tt.want)) diff --git a/internal/controller/helmrepository_controller_test.go b/internal/controller/helmrepository_controller_test.go index 9e8fc5d47..3d3e914c2 100644 --- a/internal/controller/helmrepository_controller_test.go +++ b/internal/controller/helmrepository_controller_test.go @@ -796,7 +796,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) { if tt.url != "" { repoURL = tt.url } - tlsConf, serr = getter.TLSClientConfigFromSecret(*secret, repoURL) + tlsConf, _, serr = getter.TLSClientConfigFromSecret(*secret, repoURL) if serr != nil { validSecret = false } diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 2200fe123..6b8e4b996 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -19,12 +19,15 @@ package controller import ( "bytes" "context" + "crypto/tls" + "crypto/x509" "fmt" "io" "io/ioutil" "log" "math/rand" "net" + "net/http" "os" "path/filepath" "testing" @@ -148,15 +151,11 @@ func setupRegistryServer(ctx context.Context, workspaceDir string, opts registry var out bytes.Buffer server.out = &out - // init test client - client, err := helmreg.NewClient( + // init test client options + clientOpts := []helmreg.ClientOption{ helmreg.ClientOptDebug(true), helmreg.ClientOptWriter(server.out), - ) - if err != nil { - return nil, fmt.Errorf("failed to create registry client: %s", err) } - server.registryClient = client config := &configuration.Configuration{} port, err := freeport.GetFreePort() @@ -218,6 +217,13 @@ func setupRegistryServer(ctx context.Context, workspaceDir string, opts registry if opts.withClientCertAuth { config.HTTP.TLS.ClientCAs = []string{"testdata/certs/ca.pem"} } + + // add TLS configured HTTP client option to clientOpts + httpClient, err := tlsConfiguredHTTPCLient() + if err != nil { + return nil, fmt.Errorf("failed to create TLS configured HTTP client: %s", err) + } + clientOpts = append(clientOpts, helmreg.ClientOptHTTPClient(httpClient)) } // setup logger options @@ -232,12 +238,41 @@ func setupRegistryServer(ctx context.Context, workspaceDir string, opts registry return nil, fmt.Errorf("failed to create docker registry: %w", err) } + // init test client + client, err := helmreg.NewClient(clientOpts...) + if err != nil { + return nil, fmt.Errorf("failed to create registry client: %s", err) + } + server.registryClient = client + // Start Docker registry go dockerRegistry.ListenAndServe() return server, nil } +func tlsConfiguredHTTPCLient() (*http.Client, error) { + pool := x509.NewCertPool() + if !pool.AppendCertsFromPEM(tlsCA) { + return nil, fmt.Errorf("failed to append CA certificate to pool") + } + cert, err := tls.LoadX509KeyPair("testdata/certs/server.pem", "testdata/certs/server-key.pem") + if err != nil { + return nil, fmt.Errorf("failed to load server certificate: %s", err) + } + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: pool, + Certificates: []tls.Certificate{ + cert, + }, + }, + }, + } + return httpClient, nil +} + func (r *registryClientTestServer) Close() { if r.dnsServer != nil { mockdns.UnpatchNet(net.DefaultResolver) @@ -345,7 +380,6 @@ func TestMain(m *testing.M) { Client: testEnv, EventRecorder: record.NewFakeRecorder(32), Metrics: testMetricsH, - Getters: testGetters, RegistryClientGenerator: registry.ClientGenerator, }).SetupWithManagerAndOptions(testEnv, HelmRepositoryReconcilerOptions{ RateLimiter: controller.GetDefaultRateLimiter(), diff --git a/internal/helm/getter/client_opts.go b/internal/helm/getter/client_opts.go index 2af928c8e..58248d5b6 100644 --- a/internal/helm/getter/client_opts.go +++ b/internal/helm/getter/client_opts.go @@ -23,6 +23,8 @@ import ( "errors" "fmt" "net/url" + "os" + "path" "github.com/fluxcd/pkg/oci" "github.com/google/go-containerregistry/pkg/authn" @@ -37,23 +39,47 @@ import ( soci "github.com/fluxcd/source-controller/internal/oci" ) +const ( + certFileName = "cert.pem" + keyFileName = "key.pem" + caFileName = "ca.pem" +) + var ErrDeprecatedTLSConfig = errors.New("TLS configured in a deprecated manner") +// TLSBytes contains the bytes of the TLS files. +type TLSBytes struct { + // CertBytes is the bytes of the certificate file. + CertBytes []byte + // KeyBytes is the bytes of the key file. + KeyBytes []byte + // CABytes is the bytes of the CA file. + CABytes []byte +} + // ClientOpts contains the various options to use while constructing // a Helm repository client. type ClientOpts struct { Authenticator authn.Authenticator Keychain authn.Keychain - RegLoginOpt helmreg.LoginOption + RegLoginOpts []helmreg.LoginOption TlsConfig *tls.Config GetterOpts []helmgetter.Option } +// MustLoginToRegistry returns true if the client options contain at least +// one registry login option. +func (o ClientOpts) MustLoginToRegistry() bool { + return len(o.RegLoginOpts) > 0 && o.RegLoginOpts[0] != nil +} + // GetClientOpts uses the provided HelmRepository object and a normalized // URL to construct a HelmClientOpts object. If obj is an OCI HelmRepository, // then the returned options object will also contain the required registry // auth mechanisms. -func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmRepository, url string) (*ClientOpts, error) { +// A temporary directory is created to store the certs files if needed and its path is returned along with the options object. It is the +// caller's responsibility to clean up the directory. +func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmRepository, url string) (*ClientOpts, string, error) { hrOpts := &ClientOpts{ GetterOpts: []helmgetter.Option{ helmgetter.WithURL(url), @@ -63,18 +89,25 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit } ociRepo := obj.Spec.Type == helmv1.HelmRepositoryTypeOCI - var certSecret *corev1.Secret - var err error + var ( + certSecret *corev1.Secret + tlsBytes *TLSBytes + certFile string + keyFile string + caFile string + dir string + err error + ) // Check `.spec.certSecretRef` first for any TLS auth data. if obj.Spec.CertSecretRef != nil { certSecret, err = fetchSecret(ctx, c, obj.Spec.CertSecretRef.Name, obj.GetNamespace()) if err != nil { - return nil, fmt.Errorf("failed to get TLS authentication secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.CertSecretRef.Name, err) + return nil, "", fmt.Errorf("failed to get TLS authentication secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.CertSecretRef.Name, err) } - hrOpts.TlsConfig, err = TLSClientConfigFromSecret(*certSecret, url) + hrOpts.TlsConfig, tlsBytes, err = TLSClientConfigFromSecret(*certSecret, url) if err != nil { - return nil, fmt.Errorf("failed to construct Helm client's TLS config: %w", err) + return nil, "", fmt.Errorf("failed to construct Helm client's TLS config: %w", err) } } @@ -83,22 +116,22 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit if obj.Spec.SecretRef != nil { authSecret, err = fetchSecret(ctx, c, obj.Spec.SecretRef.Name, obj.GetNamespace()) if err != nil { - return nil, fmt.Errorf("failed to get authentication secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.SecretRef.Name, err) + return nil, "", fmt.Errorf("failed to get authentication secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.SecretRef.Name, err) } // Construct actual Helm client options. opts, err := GetterOptionsFromSecret(*authSecret) if err != nil { - return nil, fmt.Errorf("failed to configure Helm client: %w", err) + return nil, "", fmt.Errorf("failed to configure Helm client: %w", err) } hrOpts.GetterOpts = append(hrOpts.GetterOpts, opts...) // If the TLS config is nil, i.e. one couldn't be constructed using `.spec.certSecretRef` - // then try to use `.spec.certSecretRef`. + // then try to use `.spec.secretRef`. if hrOpts.TlsConfig == nil { - hrOpts.TlsConfig, err = TLSClientConfigFromSecret(*authSecret, url) + hrOpts.TlsConfig, tlsBytes, err = TLSClientConfigFromSecret(*authSecret, url) if err != nil { - return nil, fmt.Errorf("failed to construct Helm client's TLS config: %w", err) + return nil, "", fmt.Errorf("failed to construct Helm client's TLS config: %w", err) } // Constructing a TLS config using the auth secret is deprecated behavior. if hrOpts.TlsConfig != nil { @@ -109,13 +142,13 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit if ociRepo { hrOpts.Keychain, err = registry.LoginOptionFromSecret(url, *authSecret) if err != nil { - return nil, fmt.Errorf("failed to configure login options: %w", err) + return nil, "", fmt.Errorf("failed to configure login options: %w", err) } } } else if obj.Spec.Provider != helmv1.GenericOCIProvider && obj.Spec.Type == helmv1.HelmRepositoryTypeOCI && ociRepo { authenticator, authErr := soci.OIDCAuth(ctx, obj.Spec.URL, obj.Spec.Provider) if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) { - return nil, fmt.Errorf("failed to get credential from '%s': %w", obj.Spec.Provider, authErr) + return nil, "", fmt.Errorf("failed to get credential from '%s': %w", obj.Spec.Provider, authErr) } if authenticator != nil { hrOpts.Authenticator = authenticator @@ -123,16 +156,34 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit } if ociRepo { - hrOpts.RegLoginOpt, err = registry.NewLoginOption(hrOpts.Authenticator, hrOpts.Keychain, url) + // Persist the certs files to the path if needed. + if tlsBytes != nil { + dir, err = os.MkdirTemp("", "helm-repo-oci-certs") + if err != nil { + return nil, "", fmt.Errorf("cannot create temporary directory: %w", err) + } + certFile, keyFile, caFile, err = StoreTLSCertificateFiles(tlsBytes, dir) + if err != nil { + return nil, "", fmt.Errorf("cannot write certs files to path: %w", err) + } + } + loginOpt, err := registry.NewLoginOption(hrOpts.Authenticator, hrOpts.Keychain, url) if err != nil { - return nil, err + return nil, "", err + } + if loginOpt != nil { + hrOpts.RegLoginOpts = []helmreg.LoginOption{loginOpt} + } + tlsLoginOpt := registry.TLSLoginOption(certFile, keyFile, caFile) + if tlsLoginOpt != nil { + hrOpts.RegLoginOpts = append(hrOpts.RegLoginOpts, tlsLoginOpt) } } if deprecatedTLSConfig { err = ErrDeprecatedTLSConfig } - return hrOpts, err + return hrOpts, dir, err } func fetchSecret(ctx context.Context, c client.Client, name, namespace string) (*corev1.Secret, error) { @@ -152,13 +203,13 @@ func fetchSecret(ctx context.Context, c client.Client, name, namespace string) ( // // Secrets with no certFile, keyFile, AND caFile are ignored, if only a // certBytes OR keyBytes is defined it returns an error. -func TLSClientConfigFromSecret(secret corev1.Secret, repositoryUrl string) (*tls.Config, error) { +func TLSClientConfigFromSecret(secret corev1.Secret, repositoryUrl string) (*tls.Config, *TLSBytes, error) { certBytes, keyBytes, caBytes := secret.Data["certFile"], secret.Data["keyFile"], secret.Data["caFile"] switch { case len(certBytes)+len(keyBytes)+len(caBytes) == 0: - return nil, nil + return nil, nil, nil case (len(certBytes) > 0 && len(keyBytes) == 0) || (len(keyBytes) > 0 && len(certBytes) == 0): - return nil, fmt.Errorf("invalid '%s' secret data: fields 'certFile' and 'keyFile' require each other's presence", + return nil, nil, fmt.Errorf("invalid '%s' secret data: fields 'certFile' and 'keyFile' require each other's presence", secret.Name) } @@ -166,7 +217,7 @@ func TLSClientConfigFromSecret(secret corev1.Secret, repositoryUrl string) (*tls if len(certBytes) > 0 && len(keyBytes) > 0 { cert, err := tls.X509KeyPair(certBytes, keyBytes) if err != nil { - return nil, err + return nil, nil, err } tlsConf.Certificates = append(tlsConf.Certificates, cert) } @@ -174,10 +225,10 @@ func TLSClientConfigFromSecret(secret corev1.Secret, repositoryUrl string) (*tls if len(caBytes) > 0 { cp, err := x509.SystemCertPool() if err != nil { - return nil, fmt.Errorf("cannot retrieve system certificate pool: %w", err) + return nil, nil, fmt.Errorf("cannot retrieve system certificate pool: %w", err) } if !cp.AppendCertsFromPEM(caBytes) { - return nil, fmt.Errorf("cannot append certificate into certificate pool: invalid caFile") + return nil, nil, fmt.Errorf("cannot append certificate into certificate pool: invalid caFile") } tlsConf.RootCAs = cp @@ -187,10 +238,50 @@ func TLSClientConfigFromSecret(secret corev1.Secret, repositoryUrl string) (*tls u, err := url.Parse(repositoryUrl) if err != nil { - return nil, fmt.Errorf("cannot parse repository URL: %w", err) + return nil, nil, fmt.Errorf("cannot parse repository URL: %w", err) } tlsConf.ServerName = u.Hostname() - return tlsConf, nil + return tlsConf, &TLSBytes{ + CertBytes: certBytes, + KeyBytes: keyBytes, + CABytes: caBytes, + }, nil +} + +// StoreTLSCertificateFiles writes the certs files to the given path and returns the files paths. +func StoreTLSCertificateFiles(tlsBytes *TLSBytes, path string) (string, string, string, error) { + var ( + certFile string + keyFile string + caFile string + err error + ) + if len(tlsBytes.CertBytes) > 0 && len(tlsBytes.KeyBytes) > 0 { + certFile, err = writeToFile(tlsBytes.CertBytes, certFileName, path) + if err != nil { + return "", "", "", err + } + keyFile, err = writeToFile(tlsBytes.KeyBytes, keyFileName, path) + if err != nil { + return "", "", "", err + } + } + if len(tlsBytes.CABytes) > 0 { + caFile, err = writeToFile(tlsBytes.CABytes, caFileName, path) + if err != nil { + return "", "", "", err + } + } + return certFile, keyFile, caFile, nil +} + +func writeToFile(data []byte, filename, tmpDir string) (string, error) { + file := path.Join(tmpDir, filename) + err := os.WriteFile(file, data, 0o644) + if err != nil { + return "", err + } + return file, nil } diff --git a/internal/helm/getter/client_opts_test.go b/internal/helm/getter/client_opts_test.go index 2231e2a52..6b031851d 100644 --- a/internal/helm/getter/client_opts_test.go +++ b/internal/helm/getter/client_opts_test.go @@ -149,7 +149,7 @@ func TestGetClientOpts(t *testing.T) { } c := clientBuilder.Build() - clientOpts, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy") + clientOpts, _, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy") if tt.err != nil { g.Expect(err).To(Equal(tt.err)) } else { @@ -183,7 +183,7 @@ func Test_tlsClientConfigFromSecret(t *testing.T) { tt.modify(secret) } - got, err := TLSClientConfigFromSecret(*secret, "") + got, _, err := TLSClientConfigFromSecret(*secret, "") if (err != nil) != tt.wantErr { t.Errorf("TLSClientConfigFromSecret() error = %v, wantErr %v", err, tt.wantErr) return @@ -196,6 +196,118 @@ func Test_tlsClientConfigFromSecret(t *testing.T) { } } +func TestGetClientOpts_registryTLSLoginOption(t *testing.T) { + tlsCA, err := os.ReadFile("../../controller/testdata/certs/ca.pem") + if err != nil { + t.Errorf("could not read CA file: %s", err) + } + + tests := []struct { + name string + certSecret *corev1.Secret + authSecret *corev1.Secret + loginOptsN int + }{ + { + name: "with valid caFile", + certSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca-file", + }, + Data: map[string][]byte{ + "caFile": tlsCA, + }, + }, + authSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-oci", + }, + Data: map[string][]byte{ + "username": []byte("user"), + "password": []byte("pass"), + }, + }, + loginOptsN: 2, + }, + { + name: "without caFile", + certSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca-file", + }, + Data: map[string][]byte{}, + }, + authSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-oci", + }, + Data: map[string][]byte{ + "username": []byte("user"), + "password": []byte("pass"), + }, + }, + loginOptsN: 1, + }, + { + name: "without cert secret", + certSecret: nil, + authSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-oci", + }, + Data: map[string][]byte{ + "username": []byte("user"), + "password": []byte("pass"), + }, + }, + loginOptsN: 1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + helmRepo := &helmv1.HelmRepository{ + Spec: helmv1.HelmRepositorySpec{ + Timeout: &metav1.Duration{ + Duration: time.Second, + }, + Type: helmv1.HelmRepositoryTypeOCI, + }, + } + + clientBuilder := fakeclient.NewClientBuilder() + + if tt.authSecret != nil { + clientBuilder.WithObjects(tt.authSecret.DeepCopy()) + helmRepo.Spec.SecretRef = &meta.LocalObjectReference{ + Name: tt.authSecret.Name, + } + } + + if tt.certSecret != nil { + clientBuilder.WithObjects(tt.certSecret.DeepCopy()) + helmRepo.Spec.CertSecretRef = &meta.LocalObjectReference{ + Name: tt.certSecret.Name, + } + } + c := clientBuilder.Build() + + clientOpts, tmpDir, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy") + if err != nil { + t.Errorf("GetClientOpts() error = %v", err) + return + } + if tmpDir != "" { + defer os.RemoveAll(tmpDir) + } + if tt.loginOptsN != len(clientOpts.RegLoginOpts) { + // we should have a login option but no TLS option + t.Error("registryTLSLoginOption() != nil") + return + } + }) + } +} + // validTlsSecret creates a secret containing key pair and CA certificate that are // valid from a syntax (minimum requirements) perspective. func validTlsSecret(t *testing.T) corev1.Secret { diff --git a/internal/helm/registry/auth.go b/internal/helm/registry/auth.go index c48ec0b2b..d6a567d24 100644 --- a/internal/helm/registry/auth.go +++ b/internal/helm/registry/auth.go @@ -154,3 +154,13 @@ func NewLoginOption(auth authn.Authenticator, keychain authn.Keychain, registryU return nil, nil } + +// TLSLoginOption returns a LoginOption that can be used to configure the TLS client. +// It requires either the caFile or both certFile and keyFile to be not blank. +func TLSLoginOption(certFile, keyFile, caFile string) registry.LoginOption { + if (certFile != "" && keyFile != "") || caFile != "" { + return registry.LoginOptTLSClientConfig(certFile, keyFile, caFile) + } + + return nil +} diff --git a/internal/helm/registry/client.go b/internal/helm/registry/client.go index 1247347ab..7ac0d3d0b 100644 --- a/internal/helm/registry/client.go +++ b/internal/helm/registry/client.go @@ -17,7 +17,9 @@ limitations under the License. package registry import ( + "crypto/tls" "io" + "net/http" "os" "helm.sh/helm/v3/pkg/registry" @@ -27,7 +29,7 @@ import ( // ClientGenerator generates a registry client and a temporary credential file. // The client is meant to be used for a single reconciliation. // The file is meant to be used for a single reconciliation and deleted after. -func ClientGenerator(isLogin bool) (*registry.Client, string, error) { +func ClientGenerator(tlsConfig *tls.Config, isLogin bool) (*registry.Client, string, error) { if isLogin { // create a temporary file to store the credentials // this is needed because otherwise the credentials are stored in ~/.docker/config.json. @@ -37,7 +39,7 @@ func ClientGenerator(isLogin bool) (*registry.Client, string, error) { } var errs []error - rClient, err := registry.NewClient(registry.ClientOptWriter(io.Discard), registry.ClientOptCredentialsFile(credentialsFile.Name())) + rClient, err := newClient(credentialsFile.Name(), tlsConfig) if err != nil { errs = append(errs, err) // attempt to delete the temporary file @@ -52,9 +54,27 @@ func ClientGenerator(isLogin bool) (*registry.Client, string, error) { return rClient, credentialsFile.Name(), nil } - rClient, err := registry.NewClient(registry.ClientOptWriter(io.Discard)) + rClient, err := newClient("", tlsConfig) if err != nil { return nil, "", err } return rClient, "", nil } + +func newClient(credentialsFile string, tlsConfig *tls.Config) (*registry.Client, error) { + opts := []registry.ClientOption{ + registry.ClientOptWriter(io.Discard), + } + if tlsConfig != nil { + opts = append(opts, registry.ClientOptHTTPClient(&http.Client{ + Transport: &http.Transport{ + TLSClientConfig: tlsConfig, + }, + })) + } + if credentialsFile != "" { + opts = append(opts, registry.ClientOptCredentialsFile(credentialsFile)) + } + + return registry.NewClient(opts...) +} diff --git a/internal/helm/repository/oci_chart_repository.go b/internal/helm/repository/oci_chart_repository.go index 0e76ee0c4..6a119183b 100644 --- a/internal/helm/repository/oci_chart_repository.go +++ b/internal/helm/repository/oci_chart_repository.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "crypto/tls" + "errors" "fmt" "net/url" "os" @@ -65,9 +66,13 @@ type OCIChartRepository struct { // RegistryClient is a client to use while downloading tags or charts from a registry. RegistryClient RegistryClient + // credentialsFile is a temporary credentials file to use while downloading tags or charts from a registry. credentialsFile string + // certificatesStore is a temporary store to use while downloading tags or charts from a registry. + certificatesStore string + // verifiers is a list of verifiers to use when verifying a chart. verifiers []oci.Verifier } @@ -120,6 +125,14 @@ func WithCredentialsFile(credentialsFile string) OCIChartRepositoryOption { } } +// WithCertificatesStore returns a ChartRepositoryOption that will set the certificates store +func WithCertificatesStore(store string) OCIChartRepositoryOption { + return func(r *OCIChartRepository) error { + r.certificatesStore = store + return nil + } +} + // NewOCIChartRepository constructs and returns a new ChartRepository with // the ChartRepository.Client configured to the getter.Getter for the // repository URL scheme. It returns an error on URL parsing failures. @@ -265,14 +278,24 @@ func (r *OCIChartRepository) HasCredentials() bool { // Clear deletes the OCI registry credentials file. func (r *OCIChartRepository) Clear() error { + var errs error // clean the credentials file if it exists if r.credentialsFile != "" { if err := os.Remove(r.credentialsFile); err != nil { - return err + errs = errors.Join(errs, err) } } r.credentialsFile = "" - return nil + + // clean the certificates store if it exists + if r.certificatesStore != "" { + if err := os.RemoveAll(r.certificatesStore); err != nil { + errs = errors.Join(errs, err) + } + } + r.certificatesStore = "" + + return errs } // getLastMatchingVersionOrConstraint returns the last version that matches the given version string. diff --git a/main.go b/main.go index ea840ace2..5071f8111 100644 --- a/main.go +++ b/main.go @@ -198,7 +198,6 @@ func main() { Client: mgr.GetClient(), EventRecorder: eventRecorder, Metrics: metrics, - Getters: getters, ControllerName: controllerName, RegistryClientGenerator: registry.ClientGenerator, }).SetupWithManagerAndOptions(mgr, controller.HelmRepositoryReconcilerOptions{