diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index d96a83282..5c2169a33 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -577,14 +577,11 @@ func (in *HelmRepositorySpec) DeepCopyInto(out *HelmRepositorySpec) { *out = new(meta.LocalObjectReference) **out = **in } -<<<<<<< HEAD if in.CertSecretRef != nil { in, out := &in.CertSecretRef, &out.CertSecretRef *out = new(meta.LocalObjectReference) **out = **in } -======= ->>>>>>> 3df4c49 (refactoring and fix tests) out.Interval = in.Interval if in.Timeout != nil { in, out := &in.Timeout, &out.Timeout diff --git a/docs/spec/v1beta2/helmrepositories.md b/docs/spec/v1beta2/helmrepositories.md index ec216c0a1..e121b01e3 100644 --- a/docs/spec/v1beta2/helmrepositories.md +++ b/docs/spec/v1beta2/helmrepositories.md @@ -459,16 +459,8 @@ 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 @@ -515,28 +507,6 @@ data: caFile: ``` -#### 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: - certFile: - keyFile: - # NOTE: Can be supplied without the above values - caFile: -``` - ### Pass credentials `.spec.passCredentials` is an optional field to allow the credentials from the diff --git a/internal/controller/helmchart_controller.go b/internal/controller/helmchart_controller.go index d393fcb32..3e702e7da 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,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } + if certsTmpDir != "" { + // this happens when the tls certs are stored in a temporary directory + defer os.RemoveAll(certsTmpDir) + } getterOpts := clientOpts.GetterOpts // Initialize the chart repository @@ -536,7 +541,8 @@ 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) + shouldLogin := len(clientOpts.RegLoginOpts) > 0 && clientOpts.RegLoginOpts[0] != nil + registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.TlsConfig, shouldLogin) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to construct Helm client: %w", err), @@ -585,8 +591,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 len(clientOpts.RegLoginOpts) > 0 && clientOpts.RegLoginOpts[0] != nil { + err = ociChartRepo.Login(clientOpts.RegLoginOpts...) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to login to OCI registry: %w", err), @@ -983,15 +989,20 @@ 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 } + if certsTmpDir != "" { + // this happens when the tls certs are stored in a temporary directory + defer os.RemoveAll(certsTmpDir) + } getterOpts := clientOpts.GetterOpts var chartRepo repository.Downloader if helmreg.IsOCI(normalizedURL) { - registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.RegLoginOpt != nil) + shouldLogin := len(clientOpts.RegLoginOpts) > 0 && clientOpts.RegLoginOpts[0] != nil + registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.TlsConfig, shouldLogin) if err != nil { return nil, fmt.Errorf("failed to create registry client: %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 len(clientOpts.RegLoginOpts) > 0 && clientOpts.RegLoginOpts[0] != nil { + 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 9cbc4ca8b..6956d12b7 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -2245,7 +2245,8 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { registryOpts registryOptions secretOpts secretOptions secret *corev1.Secret - withTLS bool + certsecret *corev1.Secret + insecure bool provider string providerImg string want sreconcile.Result @@ -2253,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, }, @@ -2283,9 +2286,10 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { }, }, { - 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, }, @@ -2307,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{ @@ -2319,6 +2324,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { registryOpts: registryOptions{ withBasicAuth: true, }, + insecure: true, secretOpts: secretOptions{ username: testRegistryUsername, password: testRegistryPassword, @@ -2340,9 +2346,9 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { name: "HTTPS With invalid CA cert", wantErr: true, registryOpts: registryOptions{ - withTLS: true, + withTLS: true, + withClientCertAuth: true, }, - withTLS: true, secretOpts: secretOptions{ username: testRegistryUsername, password: testRegistryPassword, @@ -2357,16 +2363,16 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { }, }, assertConditions: []metav1.Condition{ - *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"), + *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, + withTLS: true, + withClientCertAuth: true, }, - withTLS: true, secretOpts: secretOptions{ username: testRegistryUsername, password: testRegistryPassword, @@ -2376,10 +2382,17 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { Name: "auth-secretref", }, Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{}, + }, + certsecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cert-secretref", + }, + Type: corev1.SecretTypeTLS, Data: map[string][]byte{ "caFile": tlsCA, - "certFile": tlsPublicKey, - "keyFile": tlsPrivateKey, + "certFile": clientPublicKey, + "keyFile": clientPrivateKey, }, }, assertConditions: []metav1.Condition{ @@ -2399,7 +2412,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() { @@ -2411,7 +2426,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) // Upload the test chart - metadata, err := loadTestChartToOCI(chartData, server, "testdata/certs/server.pem", "testdata/certs/server-key.pem", "testdata/certs/ca.pem") + 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{ @@ -2446,11 +2461,18 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { repo.Spec.SecretRef = &meta.LocalObjectReference{ Name: tt.secret.Name, } - clientBuilder.WithObjects(tt.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-", @@ -2761,18 +2783,18 @@ func loadTestChartToOCI(chartData []byte, server *registryClientTestServer, cert helmreg.LoginOptBasicAuth(testRegistryUsername, testRegistryPassword), 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 61e7923c9..c21f7630f 100644 --- a/internal/controller/helmrepository_controller_oci.go +++ b/internal/controller/helmrepository_controller_oci.go @@ -42,7 +42,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" @@ -55,7 +54,6 @@ import ( "github.com/fluxcd/source-controller/internal/helm/registry" "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" ) @@ -308,73 +306,28 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S } conditions.Delete(obj, meta.StalledCondition) - var ( - authenticator authn.Authenticator - keychain authn.Keychain - tlsConfig *tls.Config - tmpCertsDir string - tlsLoginOpt helmreg.LoginOption - err error - ) - // Configure any authentication related options. - if obj.Spec.SecretRef != nil { - // Attempt to retrieve secret. - name := types.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: obj.Spec.SecretRef.Name, - } - var secret corev1.Secret - if err := r.Client.Get(ctx, name, &secret); err != nil { - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error()) - result, retErr = ctrl.Result{}, err - return - } - keychain, err = authFromSecret(ctx, r.Client, obj.Spec.URL, secret) - if err != nil { - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error()) - result, retErr = ctrl.Result{}, err - return - } - tlsConfig, err = getter.TLSClientConfigFromSecret(secret, obj.Spec.URL) - if err != nil { - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error()) - result, retErr = ctrl.Result{}, err - return - } - tlsLoginOpt, tmpCertsDir, err = makeTLSLoginOption(&secret) - if err != nil { - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error()) - result, retErr = ctrl.Result{}, err - return - } - defer func() { - if err := os.RemoveAll(tmpCertsDir); err != nil { - r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason, - "failed to delete temporary certificates directory: %s", err) - } - }() - } 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.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error()) + result, retErr = ctrl.Result{}, err + return } - loginOpt, err := makeLoginOption(authenticator, keychain, obj.Spec.URL) + 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 != "" { + // this happens when the tls certs are stored in a temporary directory + defer os.RemoveAll(certsTmpDir) + } + // Create registry client and login if needed. - registryClient, file, err := r.RegistryClientGenerator(tlsConfig, loginOpt != nil) + shouldLogin := len(clientOpts.RegLoginOpts) > 0 && clientOpts.RegLoginOpts[0] != nil + registryClient, file, err := r.RegistryClientGenerator(clientOpts.TlsConfig, shouldLogin) if err != nil { e := fmt.Errorf("failed to create registry client: %w", err) conditions.MarkFalse(obj, meta.ReadyCondition, meta.FailedReason, e.Error()) @@ -401,12 +354,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 { - opts := []helmreg.LoginOption{loginOpt} - if tlsLoginOpt != nil { - opts = append(opts, tlsLoginOpt) - } - err = chartRepo.Login(opts...) + if len(clientOpts.RegLoginOpts) > 0 && clientOpts.RegLoginOpts[0] != nil { + 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()) @@ -432,6 +381,22 @@ func (r *HelmRepositoryOCIReconciler) reconcileDelete(ctx context.Context, obj * return ctrl.Result{}, nil } +func (r *HelmRepositoryOCIReconciler) getSecret(ctx context.Context, obj *helmv1.HelmRepository) (*corev1.Secret, error) { + if obj.Spec.SecretRef == nil { + return nil, nil + } + name := types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: obj.Spec.SecretRef.Name, + } + var secret corev1.Secret + err := r.Client.Get(ctx, name, &secret) + if err != nil { + return nil, err + } + return &secret, nil +} + // eventLogf records events, and logs at the same time. // // This log is different from the debug log in the EventRecorder, in the sense @@ -473,80 +438,6 @@ func makeLoginOption(auth authn.Authenticator, keychain authn.Keychain, registry return nil, nil } -func makeTLSLoginOption(secret *corev1.Secret) (helmreg.LoginOption, string, error) { - var errs []error - certFile, keyFile, caFile, tmpDir, err := certsFilesFromSecret(secret) - if err != nil { - errs = append(errs, err) - if tmpDir != "" { - if err := os.RemoveAll(tmpDir); err != nil { - errs = append(errs, err) - } - } - return nil, "", kerrors.NewAggregate(errs) - } - - if (certFile != "" && keyFile != "") || caFile != "" { - return helmreg.LoginOptTLSClientConfig(certFile, keyFile, caFile), tmpDir, nil - } - - return nil, "", nil -} - -func certsFilesFromSecret(secret *corev1.Secret) (string, string, string, string, error) { - certBytes, keyBytes, caBytes := secret.Data["certFile"], secret.Data["keyFile"], secret.Data["caFile"] - switch { - case len(certBytes)+len(keyBytes)+len(caBytes) == 0: - return "", "", "", "", nil - case (len(certBytes) > 0 && len(keyBytes) == 0) || (len(keyBytes) > 0 && len(certBytes) == 0): - return "", "", "", "", fmt.Errorf("invalid '%s' secret data: fields 'certFile' and 'keyFile' require each other's presence", - secret.Name) - } - - var ( - certFile string - keyFile string - caFile string - err error - ) - - // create temporary folder to store the certs - tmpDir, err := os.MkdirTemp("", "helm-repo-oci-certs") - if err != nil { - return "", "", "", "", err - } - - if len(certBytes) > 0 && len(keyBytes) > 0 { - certFile, err = writeTofile(certBytes, "cert.pem", tmpDir) - if err != nil { - return "", "", "", "", err - } - keyFile, err = writeTofile(keyBytes, "key.pem", tmpDir) - if err != nil { - return "", "", "", "", err - } - } - if len(caBytes) > 0 { - caFile, err = writeTofile(caBytes, "ca.pem", tmpDir) - if err != nil { - return "", "", "", "", err - } - } - return certFile, keyFile, caFile, tmpDir, nil -} - -func writeTofile(data []byte, filename, tmpDir string) (string, error) { - file, err := os.CreateTemp(tmpDir, filename) - if err != nil { - return "", err - } - defer file.Close() - if _, err := file.Write(data); err != nil { - return "", err - } - return file.Name(), 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 860508bb9..321edc6d1 100644 --- a/internal/controller/helmrepository_controller_oci_test.go +++ b/internal/controller/helmrepository_controller_oci_test.go @@ -205,9 +205,10 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { name string url string registryOpts registryOptions - withTLS bool + insecure bool secretOpts secretOptions secret *corev1.Secret + certsSecret *corev1.Secret provider string providerImg string want ctrl.Result @@ -222,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, }, @@ -243,9 +245,10 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { }, }, { - 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, }, @@ -268,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{ @@ -281,6 +285,7 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { registryOpts: registryOptions{ withBasicAuth: true, }, + insecure: true, secretOpts: secretOptions{ username: testRegistryUsername, password: testRegistryPassword, @@ -301,7 +306,8 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { name: "HTTPS With invalid CA cert", wantErr: true, registryOpts: registryOptions{ - withTLS: true, + withTLS: true, + withClientCertAuth: true, }, secretOpts: secretOptions{ username: testRegistryUsername, @@ -312,20 +318,28 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { Name: "auth-secretref", }, Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{}, + }, + certsSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "certs-secretref", + }, + Type: corev1.SecretTypeTLS, Data: map[string][]byte{ "caFile": []byte("invalid caFile"), }, }, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation"), + *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 missing CA cert", - wantErr: true, + name: "HTTPS With CA cert", + want: ctrl.Result{RequeueAfter: interval}, registryOpts: registryOptions{ - withTLS: true, + withTLS: true, + withClientCertAuth: true, }, secretOpts: secretOptions{ username: testRegistryUsername, @@ -338,31 +352,15 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { 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"), - }, - }, - { - name: "HTTPS With CA cert", - want: ctrl.Result{RequeueAfter: interval}, - registryOpts: registryOptions{ - withTLS: true, - }, - withTLS: true, - secretOpts: secretOptions{ - username: testRegistryUsername, - password: testRegistryPassword, - }, - secret: &corev1.Secret{ + certsSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "auth-secretref", + Name: "certs-secretref", }, - Type: corev1.SecretTypeDockerConfigJson, + Type: corev1.SecretTypeTLS, Data: map[string][]byte{ "caFile": tlsCA, - "certFile": tlsPublicKey, - "keyFile": tlsPrivateKey, + "certFile": clientPublicKey, + "keyFile": clientPrivateKey, }, }, assertConditions: []metav1.Condition{ @@ -380,7 +378,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() { @@ -423,6 +423,13 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { } } + 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), 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/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go index e8b3b6c76..ee8f3af80 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/internal/controller/ocirepository_controller_test.go @@ -406,12 +406,6 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) { pool := x509.NewCertPool() pool.AppendCertsFromPEM(tlsCA) - cert, err := tls.LoadX509KeyPair("testdata/certs/server.pem", "testdata/certs/server-key.pem") - if err != nil { - t.Fatal(err) - } - certs := []tls.Certificate{cert} - tests := []struct { name string url string @@ -557,11 +551,9 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) { registryOpts: registryOptions{ withTLS: true, }, - insecure: true, craneOpts: []crane.Option{crane.WithTransport(&http.Transport{ TLSClientConfig: &tls.Config{ - RootCAs: pool, - Certificates: certs, + RootCAs: pool, }, }), }, @@ -570,9 +562,7 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) { Name: "ca-file", }, Data: map[string][]byte{ - "caFile": tlsCA, - "certFile": tlsPublicKey, - "keyFile": tlsPrivateKey, + "caFile": tlsCA, }, }, assertConditions: []metav1.Condition{ @@ -587,11 +577,9 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) { registryOpts: registryOptions{ withTLS: true, }, - insecure: true, craneOpts: []crane.Option{crane.WithTransport(&http.Transport{ TLSClientConfig: &tls.Config{ - RootCAs: pool, - Certificates: certs, + RootCAs: pool, }, }), }, @@ -606,11 +594,9 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) { registryOpts: registryOptions{ withTLS: true, }, - insecure: true, craneOpts: []crane.Option{crane.WithTransport(&http.Transport{ TLSClientConfig: &tls.Config{ - RootCAs: pool, - Certificates: certs, + RootCAs: pool, }, }), }, diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index fbe41d3aa..321a5ce1b 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -151,43 +151,9 @@ func setupRegistryServer(ctx context.Context, workspaceDir string, opts registry var out bytes.Buffer server.out = &out - // init test client - if opts.withTLS { - pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM(tlsCA) { - return nil, fmt.Errorf("failed to append CA certificate") - } - cert, err := tls.X509KeyPair(tlsPublicKey, tlsPrivateKey) - if err != nil { - return nil, fmt.Errorf("failed to load TLS key pair: %s", err) - } - httpClient := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - RootCAs: pool, - Certificates: []tls.Certificate{cert}, - }, - }, - } - - client, err := helmreg.NewClient( - helmreg.ClientOptDebug(true), - helmreg.ClientOptWriter(server.out), - helmreg.ClientOptHTTPClient(httpClient), - ) - if err != nil { - return nil, fmt.Errorf("failed to create registry client: %s", err) - } - server.registryClient = client - } else { - client, err := helmreg.NewClient( - helmreg.ClientOptDebug(true), - helmreg.ClientOptWriter(server.out), - ) - if err != nil { - return nil, fmt.Errorf("failed to create registry client: %s", err) - } - server.registryClient = client + clientOpts := []helmreg.ClientOption{ // init test client options + helmreg.ClientOptDebug(true), + helmreg.ClientOptWriter(server.out), } config := &configuration.Configuration{} @@ -195,9 +161,7 @@ func setupRegistryServer(ctx context.Context, workspaceDir string, opts registry if err != nil { return nil, fmt.Errorf("failed to get free port: %s", err) } - config.HTTP.Addr = fmt.Sprintf("127.0.0.1:%d", port) - config.HTTP.DrainTimeout = time.Duration(10) * time.Second - config.Storage = map[string]configuration.Parameters{"inmemory": map[string]interface{}{}} + server.registryHost = fmt.Sprintf("localhost:%d", port) // Change the registry host to a host which is not localhost and @@ -252,6 +216,28 @@ func setupRegistryServer(ctx context.Context, workspaceDir string, opts registry if opts.withClientCertAuth { config.HTTP.TLS.ClientCAs = []string{"testdata/certs/ca.pem"} } + + // Client options + 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, + }, + }, + }, + } + + clientOpts = append(clientOpts, helmreg.ClientOptHTTPClient(httpClient)) } // setup logger options @@ -266,6 +252,13 @@ 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() diff --git a/internal/controller/testdata/certs/ca-csr.json b/internal/controller/testdata/certs/ca-csr.json index 7cffb0e30..941277bb1 100644 --- a/internal/controller/testdata/certs/ca-csr.json +++ b/internal/controller/testdata/certs/ca-csr.json @@ -4,7 +4,6 @@ "127.0.0.1", "localhost", "example.com", - "www.example.com", - "0x7f000001" + "www.example.com" ] } diff --git a/internal/controller/testdata/certs/ca-key.pem b/internal/controller/testdata/certs/ca-key.pem index 16bca20ab..b69de5ab5 100644 --- a/internal/controller/testdata/certs/ca-key.pem +++ b/internal/controller/testdata/certs/ca-key.pem @@ -1,5 +1,5 @@ -----BEGIN EC PRIVATE KEY----- -MHcCAQEEIOXssMUfnHMMuufQpshjlYrR3zAbgcrkIrMqGVFQK6BpoAoGCCqGSM49 -AwEHoUQDQgAE5FNAUf6HpeTiM58p6N3NAsstI0RCvptmQy9b0eBUuW1+2ZGfQf2D -80FbuFw6zR4wr7A4PMLeLrVJNY5EY5b/TA== +MHcCAQEEIOH/u9dMcpVcZ0+X9Fc78dCTj8SHuXawhLjhu/ej64WToAoGCCqGSM49 +AwEHoUQDQgAEruH/kPxtX3cyYR2G7TYmxLq6AHyzo/NGXc9XjGzdJutE2SQzn37H +dvSJbH+Lvqo9ik0uiJVRVdCYD1j7gNszGA== -----END EC PRIVATE KEY----- diff --git a/internal/controller/testdata/certs/ca.csr b/internal/controller/testdata/certs/ca.csr index be5c862b9..baa8aeb26 100644 --- a/internal/controller/testdata/certs/ca.csr +++ b/internal/controller/testdata/certs/ca.csr @@ -1,9 +1,9 @@ -----BEGIN CERTIFICATE REQUEST----- -MIIBKzCB0gIBADAZMRcwFQYDVQQDEw5leGFtcGxlLmNvbSBDQTBZMBMGByqGSM49 -AgEGCCqGSM49AwEHA0IABORTQFH+h6Xk4jOfKejdzQLLLSNEQr6bZkMvW9HgVLlt -ftmRn0H9g/NBW7hcOs0eMK+wODzC3i61STWORGOW/0ygVzBVBgkqhkiG9w0BCQ4x -SDBGMEQGA1UdEQQ9MDuCCWxvY2FsaG9zdIILZXhhbXBsZS5jb22CD3d3dy5leGFt -cGxlLmNvbYIKMHg3ZjAwMDAwMYcEfwAAATAKBggqhkjOPQQDAgNIADBFAiBQLSRn -qPbbMyBZSlLcCH23gBTcR4axsAw4fCazBjMJ2wIhAPQ671kzMSn63wInDjWxsdF1 -t0c4ZH8jHpkUEAQ74UVW +MIIBIDCBxgIBADAZMRcwFQYDVQQDEw5leGFtcGxlLmNvbSBDQTBZMBMGByqGSM49 +AgEGCCqGSM49AwEHA0IABK7h/5D8bV93MmEdhu02JsS6ugB8s6PzRl3PV4xs3Sbr +RNkkM59+x3b0iWx/i76qPYpNLoiVUVXQmA9Y+4DbMxigSzBJBgkqhkiG9w0BCQ4x +PDA6MDgGA1UdEQQxMC+CCWxvY2FsaG9zdIILZXhhbXBsZS5jb22CD3d3dy5leGFt +cGxlLmNvbYcEfwAAATAKBggqhkjOPQQDAgNJADBGAiEAkw85nyLhJssyCYsaFvRU +EErhu66xHPJug/nG50uV5OoCIQCUorrflOSxfChPeCe4xfwcPv7FpcCYbKVYtGzz +b34Wow== -----END CERTIFICATE REQUEST----- diff --git a/internal/controller/testdata/certs/ca.pem b/internal/controller/testdata/certs/ca.pem index 188443c3d..080bd24e6 100644 --- a/internal/controller/testdata/certs/ca.pem +++ b/internal/controller/testdata/certs/ca.pem @@ -1,11 +1,11 @@ -----BEGIN CERTIFICATE----- -MIIBhjCCAS2gAwIBAgIUE/XiwPHY1izwrIwbQiXxqO/Q8KswCgYIKoZIzj0EAwIw -GTEXMBUGA1UEAxMOZXhhbXBsZS5jb20gQ0EwHhcNMjMwNTI0MTQyMTAwWhcNMjgw -NTIyMTQyMTAwWjAZMRcwFQYDVQQDEw5leGFtcGxlLmNvbSBDQTBZMBMGByqGSM49 -AgEGCCqGSM49AwEHA0IABORTQFH+h6Xk4jOfKejdzQLLLSNEQr6bZkMvW9HgVLlt -ftmRn0H9g/NBW7hcOs0eMK+wODzC3i61STWORGOW/0yjUzBRMA4GA1UdDwEB/wQE -AwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBRSGfT6hp/EUOgqOjRm4Vzs -Pt8QiTAPBgNVHREECDAGhwR/AAABMAoGCCqGSM49BAMCA0cAMEQCIGqg/rOrI9Oq -EBEmPyCtehmKWLcpKDBMTzZAponByVlwAiATsP/meJNiEsMvOK2Q7UFvi2+c3Rcj -NUQAcAm8iEtJQQ== +MIIBhzCCAS2gAwIBAgIUdsAtiX3gN0uk7ddxASWYE/tdv0wwCgYIKoZIzj0EAwIw +GTEXMBUGA1UEAxMOZXhhbXBsZS5jb20gQ0EwHhcNMjAwNDE3MDgxODAwWhcNMjUw +NDE2MDgxODAwWjAZMRcwFQYDVQQDEw5leGFtcGxlLmNvbSBDQTBZMBMGByqGSM49 +AgEGCCqGSM49AwEHA0IABK7h/5D8bV93MmEdhu02JsS6ugB8s6PzRl3PV4xs3Sbr +RNkkM59+x3b0iWx/i76qPYpNLoiVUVXQmA9Y+4DbMxijUzBRMA4GA1UdDwEB/wQE +AwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBQGyUiU1QEZiMAqjsnIYTwZ +4yp5wzAPBgNVHREECDAGhwR/AAABMAoGCCqGSM49BAMCA0gAMEUCIQDzdtvKdE8O +1+WRTZ9MuSiFYcrEz7Zne7VXouDEKqKEigIgM4WlbDeuNCKbqhqj+xZV0pa3rweb +OD8EjjCMY69RMO0= -----END CERTIFICATE----- diff --git a/internal/controller/testdata/certs/server-csr.json b/internal/controller/testdata/certs/server-csr.json index 9fb0c7962..0baf11601 100644 --- a/internal/controller/testdata/certs/server-csr.json +++ b/internal/controller/testdata/certs/server-csr.json @@ -4,7 +4,6 @@ "127.0.0.1", "localhost", "example.com", - "www.example.com", - "0x7f000001" + "www.example.com" ] } diff --git a/internal/controller/testdata/certs/server-key.pem b/internal/controller/testdata/certs/server-key.pem index d147ad6a5..5054ff39f 100644 --- a/internal/controller/testdata/certs/server-key.pem +++ b/internal/controller/testdata/certs/server-key.pem @@ -1,5 +1,5 @@ -----BEGIN EC PRIVATE KEY----- -MHcCAQEEIJ68GBu37yyAfzMnKMgo950R8OYoGrt6DUwVg1Wy5c+ZoAoGCCqGSM49 -AwEHoUQDQgAE/2T6ANTLUGqz/QWJi35H6ijcaDabFwU2IxqNu9iwwQz7e+CkCqcu -6wLAn1Q8URpG3CGTS2td0a9v2zPqSi9eAQ== +MHcCAQEEIKQbEXV6nljOHMmPrWVWQ+JrAE5wsbE9iMhfY7wlJgXOoAoGCCqGSM49 +AwEHoUQDQgAE+53oBGlrvVUTelSGYji8GNHVhVg8jOs1PeeLuXCIZjQmctHLFEq3 +fE+mGxCL93MtpYzlwIWBf0m7pEGQre6bzg== -----END EC PRIVATE KEY----- diff --git a/internal/controller/testdata/certs/server.csr b/internal/controller/testdata/certs/server.csr index 941dd4f92..5caf7b39c 100644 --- a/internal/controller/testdata/certs/server.csr +++ b/internal/controller/testdata/certs/server.csr @@ -1,9 +1,8 @@ -----BEGIN CERTIFICATE REQUEST----- -MIIBJzCBzwIBADAWMRQwEgYDVQQDEwtleGFtcGxlLmNvbTBZMBMGByqGSM49AgEG -CCqGSM49AwEHA0IABP9k+gDUy1Bqs/0FiYt+R+oo3Gg2mxcFNiMajbvYsMEM+3vg -pAqnLusCwJ9UPFEaRtwhk0trXdGvb9sz6kovXgGgVzBVBgkqhkiG9w0BCQ4xSDBG -MEQGA1UdEQQ9MDuCCWxvY2FsaG9zdIILZXhhbXBsZS5jb22CD3d3dy5leGFtcGxl -LmNvbYIKMHg3ZjAwMDAwMYcEfwAAATAKBggqhkjOPQQDAgNHADBEAiAyNdC5xrMi -E3MopSm30C0D3WI0dV2ygCC6NsB/VyKkyAIgYGG4A1F8EDn4e9T7237GIg21a0tc -oD7WyWEgeLOzb9c= +MIIBHDCBwwIBADAWMRQwEgYDVQQDEwtleGFtcGxlLmNvbTBZMBMGByqGSM49AgEG +CCqGSM49AwEHA0IABPud6ARpa71VE3pUhmI4vBjR1YVYPIzrNT3ni7lwiGY0JnLR +yxRKt3xPphsQi/dzLaWM5cCFgX9Ju6RBkK3um86gSzBJBgkqhkiG9w0BCQ4xPDA6 +MDgGA1UdEQQxMC+CCWxvY2FsaG9zdIILZXhhbXBsZS5jb22CD3d3dy5leGFtcGxl +LmNvbYcEfwAAATAKBggqhkjOPQQDAgNIADBFAiB5A6wvQ5x6g/zhiyn+wLzXsOaB +Gb/F25p/zTHHQqZbkwIhAPUgWzy/2bs6eZEi97bSlaRdmrqHwqT842t5sEwGyXNV -----END CERTIFICATE REQUEST----- diff --git a/internal/controller/testdata/certs/server.pem b/internal/controller/testdata/certs/server.pem index 232a57eb4..11c655a0b 100644 --- a/internal/controller/testdata/certs/server.pem +++ b/internal/controller/testdata/certs/server.pem @@ -1,13 +1,13 @@ -----BEGIN CERTIFICATE----- -MIIB+TCCAZ6gAwIBAgIUIGxnexcJoQ+H1wPEkwe5c7gwxbAwCgYIKoZIzj0EAwIw -GTEXMBUGA1UEAxMOZXhhbXBsZS5jb20gQ0EwHhcNMjMwNTI0MTQyMTAwWhcNMzMw -NTIxMTQyMTAwWjAWMRQwEgYDVQQDEwtleGFtcGxlLmNvbTBZMBMGByqGSM49AgEG -CCqGSM49AwEHA0IABP9k+gDUy1Bqs/0FiYt+R+oo3Gg2mxcFNiMajbvYsMEM+3vg -pAqnLusCwJ9UPFEaRtwhk0trXdGvb9sz6kovXgGjgcYwgcMwDgYDVR0PAQH/BAQD +MIIB7TCCAZKgAwIBAgIUB+17B8PU05wVTzRHLeG+S+ybZK4wCgYIKoZIzj0EAwIw +GTEXMBUGA1UEAxMOZXhhbXBsZS5jb20gQ0EwHhcNMjAwNDE3MDgxODAwWhcNMzAw +NDE1MDgxODAwWjAWMRQwEgYDVQQDEwtleGFtcGxlLmNvbTBZMBMGByqGSM49AgEG +CCqGSM49AwEHA0IABPud6ARpa71VE3pUhmI4vBjR1YVYPIzrNT3ni7lwiGY0JnLR +yxRKt3xPphsQi/dzLaWM5cCFgX9Ju6RBkK3um86jgbowgbcwDgYDVR0PAQH/BAQD AgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAMBgNVHRMBAf8EAjAA -MB0GA1UdDgQWBBQ1Z+HHTrEjwJsOvNaXPhoiC3whBjAfBgNVHSMEGDAWgBRSGfT6 -hp/EUOgqOjRm4VzsPt8QiTBEBgNVHREEPTA7gglsb2NhbGhvc3SCC2V4YW1wbGUu -Y29tgg93d3cuZXhhbXBsZS5jb22CCjB4N2YwMDAwMDGHBH8AAAEwCgYIKoZIzj0E -AwIDSQAwRgIhAIlapRW3ve2Q0I+dkUkJZM3pSjEUj5Vnr5+XAGa1Hj7vAiEAtoXs -AGr5+WetlgkeRmxIqEAxtm8DED72dg5rTDueU04= +MB0GA1UdDgQWBBTM8HS5EIlVMBYv/300jN8PEArUgDAfBgNVHSMEGDAWgBQGyUiU +1QEZiMAqjsnIYTwZ4yp5wzA4BgNVHREEMTAvgglsb2NhbGhvc3SCC2V4YW1wbGUu +Y29tgg93d3cuZXhhbXBsZS5jb22HBH8AAAEwCgYIKoZIzj0EAwIDSQAwRgIhAOgB +5W82FEgiTTOmsNRekkK5jUPbj4D4eHtb2/BI7ph4AiEA2AxHASIFBdv5b7Qf5prb +bdNmUCzAvVuCAKuMjg2OPrE= -----END CERTIFICATE----- diff --git a/internal/helm/getter/client_opts.go b/internal/helm/getter/client_opts.go index 2af928c8e..770e9f62c 100644 --- a/internal/helm/getter/client_opts.go +++ b/internal/helm/getter/client_opts.go @@ -23,6 +23,7 @@ import ( "errors" "fmt" "net/url" + "os" "github.com/fluxcd/pkg/oci" "github.com/google/go-containerregistry/pkg/authn" @@ -37,6 +38,12 @@ import ( soci "github.com/fluxcd/source-controller/internal/oci" ) +const ( + certFile = "cert.pem" + keyFile = "key.pem" + caFile = "ca.pem" +) + var ErrDeprecatedTLSConfig = errors.New("TLS configured in a deprecated manner") // ClientOpts contains the various options to use while constructing @@ -44,16 +51,28 @@ var ErrDeprecatedTLSConfig = errors.New("TLS configured in a deprecated manner") type ClientOpts struct { Authenticator authn.Authenticator Keychain authn.Keychain - RegLoginOpt helmreg.LoginOption + RegLoginOpts []helmreg.LoginOption TlsConfig *tls.Config GetterOpts []helmgetter.Option } +// 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 +} + // 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. 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 +82,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 +109,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 { @@ -107,15 +133,27 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit } if ociRepo { + // Now that we have checked for TLS config, 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 = StoreCertsFiles(tlsBytes, dir) + if err != nil { + return nil, "", fmt.Errorf("cannot write certs files to path: %w", err) + } + } + 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 +161,21 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit } if ociRepo { - hrOpts.RegLoginOpt, err = registry.NewLoginOption(hrOpts.Authenticator, hrOpts.Keychain, url) + loginOpt, err := registry.NewLoginOption(hrOpts.Authenticator, hrOpts.Keychain, url) if err != nil { - return nil, err + return nil, "", err + } + 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 +195,14 @@ 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) { +// If shouldStore is true, it will write the certs files to the path. +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 +210,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 +218,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 +231,53 @@ 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 +} + +// StoreCertsFiles writes the certs files to the path and returns the path of the files. +func StoreCertsFiles(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, certFile, path) + if err != nil { + return "", "", "", err + } + keyFile, err = writeTofile(tlsBytes.KeyBytes, keyFile, path) + if err != nil { + return "", "", "", err + } + } + if len(tlsBytes.CaBytes) > 0 { + caFile, err = writeTofile(tlsBytes.CaBytes, caFile, path) + if err != nil { + return "", "", "", err + } + } + return certFile, keyFile, caFile, nil +} + +func writeTofile(data []byte, filename, tmpDir string) (string, error) { + file, err := os.CreateTemp(tmpDir, filename) + if err != nil { + return "", err + } + defer file.Close() + if _, err := file.Write(data); err != nil { + return "", err + } + return file.Name(), nil } diff --git a/internal/helm/getter/client_opts_test.go b/internal/helm/getter/client_opts_test.go index 2231e2a52..e97f29d0f 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 Test_registryTLSoginOption(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 + wantNil bool + }{ + { + "caFile", + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca-file", + }, + Data: map[string][]byte{ + "caFile": tlsCA, + }, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-oci", + }, + Data: map[string][]byte{ + "username": []byte("user"), + "password": []byte("pass"), + }, + }, + false, + }, + { + "without caFile", + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca-file", + }, + Data: map[string][]byte{}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-oci", + }, + Data: map[string][]byte{ + "username": []byte("user"), + "password": []byte("pass"), + }, + }, + true, + }, + { + "empty", + nil, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-oci", + }, + Data: map[string][]byte{ + "username": []byte("user"), + "password": []byte("pass"), + }, + }, + true, + }, + } + 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.wantNil && len(clientOpts.RegLoginOpts) != 1 { + // we should have a login option but no TLS option + t.Error("registryTLSoginOption() != 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..08b1a98e6 100644 --- a/internal/helm/registry/auth.go +++ b/internal/helm/registry/auth.go @@ -154,3 +154,12 @@ func NewLoginOption(auth authn.Authenticator, keychain authn.Keychain, registryU return nil, nil } + +// TLSLoginOptionFromSecret returns a LoginOption that can be used to configure the TLS client. +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 e584f6b3b..7ac0d3d0b 100644 --- a/internal/helm/registry/client.go +++ b/internal/helm/registry/client.go @@ -62,34 +62,19 @@ func ClientGenerator(tlsConfig *tls.Config, isLogin bool) (*registry.Client, str } func newClient(credentialsFile string, tlsConfig *tls.Config) (*registry.Client, error) { - if tlsConfig != nil { - return newClientWithTLS(credentialsFile, tlsConfig) - } - return newDefaultClient(credentialsFile) -} - -func newDefaultClient(credentialsFile string) (*registry.Client, error) { - if credentialsFile == "" { - return registry.NewClient(registry.ClientOptWriter(io.Discard)) + opts := []registry.ClientOption{ + registry.ClientOptWriter(io.Discard), } - return registry.NewClient(registry.ClientOptWriter(io.Discard), - registry.ClientOptCredentialsFile(credentialsFile)) -} - -func newClientWithTLS(credentialsFile string, tlsConfig *tls.Config) (*registry.Client, error) { - if credentialsFile == "" { - return registry.NewClient(registry.ClientOptWriter(io.Discard), - registry.ClientOptHTTPClient(&http.Client{ - Transport: &http.Transport{ - TLSClientConfig: tlsConfig, - }, - })) - } - return registry.NewClient(registry.ClientOptWriter(io.Discard), - registry.ClientOptCredentialsFile(credentialsFile), - registry.ClientOptHTTPClient(&http.Client{ + 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...) }