Skip to content

Commit b59c76a

Browse files
aryan9600aryan9600
committed
helm: add support for specifying TLS auth via .spec.certSecretRef
Add support for specifying TLS auth data via `.spec.certSecretRef` in HelmRepository and log a deprecation warning if TLS is configured via `.spec.secretRef`. Introduce (and refactor) Helm client builder and auth helpers to reduce duplicated code and increase uniformity and testability. Signed-off-by: Sanskar Jaiswal <[email protected]>
1 parent 0589a3b commit b59c76a

12 files changed

+574
-513
lines changed

‎internal/controller/helmchart_controller.go

Lines changed: 28 additions & 161 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package controller
1818

1919
import (
2020
"context"
21-
"crypto/tls"
2221
"errors"
2322
"fmt"
2423
"net/url"
@@ -28,7 +27,6 @@ import (
2827
"strings"
2928
"time"
3029

31-
"github.com/google/go-containerregistry/pkg/authn"
3230
"github.com/google/go-containerregistry/pkg/v1/remote"
3331
"github.com/opencontainers/go-digest"
3432
helmgetter "helm.sh/helm/v3/pkg/getter"
@@ -54,7 +52,6 @@ import (
5452
eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
5553
"github.com/fluxcd/pkg/apis/meta"
5654
"github.com/fluxcd/pkg/git"
57-
"github.com/fluxcd/pkg/oci"
5855
"github.com/fluxcd/pkg/runtime/conditions"
5956
helper "github.com/fluxcd/pkg/runtime/controller"
6057
"github.com/fluxcd/pkg/runtime/patch"
@@ -68,7 +65,6 @@ import (
6865
serror "github.com/fluxcd/source-controller/internal/error"
6966
"github.com/fluxcd/source-controller/internal/helm/chart"
7067
"github.com/fluxcd/source-controller/internal/helm/getter"
71-
"github.com/fluxcd/source-controller/internal/helm/registry"
7268
"github.com/fluxcd/source-controller/internal/helm/repository"
7369
soci "github.com/fluxcd/source-controller/internal/oci"
7470
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
@@ -506,11 +502,6 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, sp *patch.Ser
506502
// object, and returns early.
507503
func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *helmv1.HelmChart,
508504
repo *helmv1.HelmRepository, b *chart.Build) (sreconcile.Result, error) {
509-
var (
510-
tlsConfig *tls.Config
511-
authenticator authn.Authenticator
512-
keychain authn.Keychain
513-
)
514505
// Used to login with the repository declared provider
515506
ctxTimeout, cancel := context.WithTimeout(ctx, repo.Spec.Timeout.Duration)
516507
defer cancel()
@@ -519,72 +510,16 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
519510
if err != nil {
520511
return chartRepoConfigErrorReturn(err, obj)
521512
}
522-
// Construct the Getter options from the HelmRepository data
523-
clientOpts := []helmgetter.Option{
524-
helmgetter.WithURL(normalizedURL),
525-
helmgetter.WithTimeout(repo.Spec.Timeout.Duration),
526-
helmgetter.WithPassCredentialsAll(repo.Spec.PassCredentials),
527-
}
528-
if secret, err := r.getHelmRepositorySecret(ctx, repo); secret != nil || err != nil {
529-
if err != nil {
530-
e := &serror.Event{
531-
Err: fmt.Errorf("failed to get secret '%s': %w", repo.Spec.SecretRef.Name, err),
532-
Reason: sourcev1.AuthenticationFailedReason,
533-
}
534-
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
535-
// Return error as the world as observed may change
536-
return sreconcile.ResultEmpty, e
537-
}
538-
539-
// Build client options from secret
540-
opts, tlsCfg, err := r.clientOptionsFromSecret(secret, normalizedURL)
541-
if err != nil {
542-
e := &serror.Event{
543-
Err: err,
544-
Reason: sourcev1.AuthenticationFailedReason,
545-
}
546-
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
547-
// Requeue as content of secret might change
548-
return sreconcile.ResultEmpty, e
549-
}
550-
clientOpts = append(clientOpts, opts...)
551-
tlsConfig = tlsCfg
552-
553-
// Build registryClient options from secret
554-
keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret)
555-
if err != nil {
556-
e := &serror.Event{
557-
Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err),
558-
Reason: sourcev1.AuthenticationFailedReason,
559-
}
560-
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
561-
// Requeue as content of secret might change
562-
return sreconcile.ResultEmpty, e
563-
}
564-
} else if repo.Spec.Provider != helmv1.GenericOCIProvider && repo.Spec.Type == helmv1.HelmRepositoryTypeOCI {
565-
auth, authErr := oidcAuth(ctxTimeout, repo.Spec.URL, repo.Spec.Provider)
566-
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
567-
e := &serror.Event{
568-
Err: fmt.Errorf("failed to get credential from %s: %w", repo.Spec.Provider, authErr),
569-
Reason: sourcev1.AuthenticationFailedReason,
570-
}
571-
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
572-
return sreconcile.ResultEmpty, e
573-
}
574-
if auth != nil {
575-
authenticator = auth
576-
}
577-
}
578-
579-
loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL)
580-
if err != nil {
513+
clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL)
514+
if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) {
581515
e := &serror.Event{
582516
Err: err,
583517
Reason: sourcev1.AuthenticationFailedReason,
584518
}
585519
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
586520
return sreconcile.ResultEmpty, e
587521
}
522+
getterOpts := clientOpts.GetterOpts
588523

589524
// Initialize the chart repository
590525
var chartRepo repository.Downloader
@@ -599,7 +534,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
599534
// this is needed because otherwise the credentials are stored in ~/.docker/config.json.
600535
// TODO@souleb: remove this once the registry move to Oras v2
601536
// or rework to enable reusing credentials to avoid the unneccessary handshake operations
602-
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpt != nil)
537+
registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.RegLoginOpt != nil)
603538
if err != nil {
604539
e := &serror.Event{
605540
Err: fmt.Errorf("failed to construct Helm client: %w", err),
@@ -621,7 +556,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
621556
var verifiers []soci.Verifier
622557
if obj.Spec.Verify != nil {
623558
provider := obj.Spec.Verify.Provider
624-
verifiers, err = r.makeVerifiers(ctx, obj, authenticator, keychain)
559+
verifiers, err = r.makeVerifiers(ctx, obj, *clientOpts)
625560
if err != nil {
626561
if obj.Spec.Verify.SecretRef == nil {
627562
provider = fmt.Sprintf("%s keyless", provider)
@@ -636,21 +571,20 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
636571
}
637572

638573
// Tell the chart repository to use the OCI client with the configured getter
639-
clientOpts = append(clientOpts, helmgetter.WithRegistryClient(registryClient))
574+
getterOpts = append(getterOpts, helmgetter.WithRegistryClient(registryClient))
640575
ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL,
641576
repository.WithOCIGetter(r.Getters),
642-
repository.WithOCIGetterOptions(clientOpts),
577+
repository.WithOCIGetterOptions(getterOpts),
643578
repository.WithOCIRegistryClient(registryClient),
644579
repository.WithVerifiers(verifiers))
645580
if err != nil {
646581
return chartRepoConfigErrorReturn(err, obj)
647582
}
648-
chartRepo = ociChartRepo
649583

650584
// If login options are configured, use them to login to the registry
651585
// The OCIGetter will later retrieve the stored credentials to pull the chart
652-
if loginOpt != nil {
653-
err = ociChartRepo.Login(loginOpt)
586+
if clientOpts.RegLoginOpt != nil {
587+
err = ociChartRepo.Login(clientOpts.RegLoginOpt)
654588
if err != nil {
655589
e := &serror.Event{
656590
Err: fmt.Errorf("failed to login to OCI registry: %w", err),
@@ -660,8 +594,9 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
660594
return sreconcile.ResultEmpty, e
661595
}
662596
}
597+
chartRepo = ociChartRepo
663598
default:
664-
httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts...)
599+
httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, clientOpts.TlsConfig, getterOpts...)
665600
if err != nil {
666601
return chartRepoConfigErrorReturn(err, obj)
667602
}
@@ -1024,12 +959,6 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *helmv1.He
1024959
// The callback returns an object with a state, so the caller has to do the necessary cleanup.
1025960
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback {
1026961
return func(url string) (repository.Downloader, error) {
1027-
var (
1028-
tlsConfig *tls.Config
1029-
authenticator authn.Authenticator
1030-
keychain authn.Keychain
1031-
)
1032-
1033962
normalizedURL, err := repository.NormalizeURL(url)
1034963
if err != nil {
1035964
return nil, err
@@ -1052,61 +981,28 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
1052981
ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
1053982
defer cancel()
1054983

1055-
clientOpts := []helmgetter.Option{
1056-
helmgetter.WithURL(normalizedURL),
1057-
helmgetter.WithTimeout(obj.Spec.Timeout.Duration),
1058-
helmgetter.WithPassCredentialsAll(obj.Spec.PassCredentials),
1059-
}
1060-
if secret, err := r.getHelmRepositorySecret(ctx, obj); secret != nil || err != nil {
1061-
if err != nil {
1062-
return nil, err
1063-
}
1064-
1065-
// Build client options from secret
1066-
opts, tlsCfg, err := r.clientOptionsFromSecret(secret, normalizedURL)
1067-
if err != nil {
1068-
return nil, err
1069-
}
1070-
clientOpts = append(clientOpts, opts...)
1071-
tlsConfig = tlsCfg
1072-
1073-
// Build registryClient options from secret
1074-
keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret)
1075-
if err != nil {
1076-
return nil, fmt.Errorf("failed to create login options for HelmRepository '%s': %w", obj.Name, err)
1077-
}
1078-
1079-
} else if obj.Spec.Provider != helmv1.GenericOCIProvider && obj.Spec.Type == helmv1.HelmRepositoryTypeOCI {
1080-
auth, authErr := oidcAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
1081-
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
1082-
return nil, fmt.Errorf("failed to get credential from %s: %w", obj.Spec.Provider, authErr)
1083-
}
1084-
if auth != nil {
1085-
authenticator = auth
1086-
}
1087-
}
1088-
1089-
loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL)
1090-
if err != nil {
984+
clientOpts, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL)
985+
if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) {
1091986
return nil, err
1092987
}
988+
getterOpts := clientOpts.GetterOpts
1093989

1094990
var chartRepo repository.Downloader
1095991
if helmreg.IsOCI(normalizedURL) {
1096-
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpt != nil)
992+
registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.RegLoginOpt != nil)
1097993
if err != nil {
1098-
return nil, fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", obj.Name, err)
994+
return nil, fmt.Errorf("failed to create registry client: %w", err)
1099995
}
1100996

1101997
var errs []error
1102998
// Tell the chart repository to use the OCI client with the configured getter
1103-
clientOpts = append(clientOpts, helmgetter.WithRegistryClient(registryClient))
999+
getterOpts = append(getterOpts, helmgetter.WithRegistryClient(registryClient))
11041000
ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters),
1105-
repository.WithOCIGetterOptions(clientOpts),
1001+
repository.WithOCIGetterOptions(getterOpts),
11061002
repository.WithOCIRegistryClient(registryClient),
11071003
repository.WithCredentialsFile(credentialsFile))
11081004
if err != nil {
1109-
errs = append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", obj.Name, err))
1005+
errs = append(errs, fmt.Errorf("failed to create OCI chart repository: %w", err))
11101006
// clean up the credentialsFile
11111007
if credentialsFile != "" {
11121008
if err := os.Remove(credentialsFile); err != nil {
@@ -1118,10 +1014,10 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
11181014

11191015
// If login options are configured, use them to login to the registry
11201016
// The OCIGetter will later retrieve the stored credentials to pull the chart
1121-
if loginOpt != nil {
1122-
err = ociChartRepo.Login(loginOpt)
1017+
if clientOpts.RegLoginOpt != nil {
1018+
err = ociChartRepo.Login(clientOpts.RegLoginOpt)
11231019
if err != nil {
1124-
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", obj.Name, err))
1020+
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository: %w", err))
11251021
// clean up the credentialsFile
11261022
errs = append(errs, ociChartRepo.Clear())
11271023
return nil, kerrors.NewAggregate(errs)
@@ -1130,7 +1026,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
11301026

11311027
chartRepo = ociChartRepo
11321028
} else {
1133-
httpChartRepo, err := repository.NewChartRepository(normalizedURL, "", r.Getters, tlsConfig, clientOpts...)
1029+
httpChartRepo, err := repository.NewChartRepository(normalizedURL, "", r.Getters, clientOpts.TlsConfig, getterOpts...)
11341030
if err != nil {
11351031
return nil, err
11361032
}
@@ -1178,36 +1074,6 @@ func (r *HelmChartReconciler) resolveDependencyRepository(ctx context.Context, u
11781074
return nil, fmt.Errorf("no HelmRepository found for '%s' in '%s' namespace", url, namespace)
11791075
}
11801076

1181-
func (r *HelmChartReconciler) clientOptionsFromSecret(secret *corev1.Secret, normalizedURL string) ([]helmgetter.Option, *tls.Config, error) {
1182-
opts, err := getter.ClientOptionsFromSecret(*secret)
1183-
if err != nil {
1184-
return nil, nil, fmt.Errorf("failed to configure Helm client with secret data: %w", err)
1185-
}
1186-
1187-
tlsConfig, err := getter.TLSClientConfigFromSecret(*secret, normalizedURL)
1188-
if err != nil {
1189-
return nil, nil, fmt.Errorf("failed to create TLS client config with secret data: %w", err)
1190-
}
1191-
1192-
return opts, tlsConfig, nil
1193-
}
1194-
1195-
func (r *HelmChartReconciler) getHelmRepositorySecret(ctx context.Context, repository *helmv1.HelmRepository) (*corev1.Secret, error) {
1196-
if repository.Spec.SecretRef == nil {
1197-
return nil, nil
1198-
}
1199-
name := types.NamespacedName{
1200-
Namespace: repository.GetNamespace(),
1201-
Name: repository.Spec.SecretRef.Name,
1202-
}
1203-
var secret corev1.Secret
1204-
err := r.Client.Get(ctx, name, &secret)
1205-
if err != nil {
1206-
return nil, err
1207-
}
1208-
return &secret, nil
1209-
}
1210-
12111077
func (r *HelmChartReconciler) indexHelmRepositoryByURL(o client.Object) []string {
12121078
repo, ok := o.(*helmv1.HelmRepository)
12131079
if !ok {
@@ -1412,13 +1278,14 @@ func chartRepoConfigErrorReturn(err error, obj *helmv1.HelmChart) (sreconcile.Re
14121278
}
14131279

14141280
// makeVerifiers returns a list of verifiers for the given chart.
1415-
func (r *HelmChartReconciler) makeVerifiers(ctx context.Context, obj *helmv1.HelmChart, auth authn.Authenticator, keychain authn.Keychain) ([]soci.Verifier, error) {
1281+
func (r *HelmChartReconciler) makeVerifiers(ctx context.Context, obj *helmv1.HelmChart, clientOpts getter.ClientOpts) ([]soci.Verifier, error) {
14161282
var verifiers []soci.Verifier
14171283
verifyOpts := []remote.Option{}
1418-
if auth != nil {
1419-
verifyOpts = append(verifyOpts, remote.WithAuth(auth))
1284+
1285+
if clientOpts.Authenticator != nil {
1286+
verifyOpts = append(verifyOpts, remote.WithAuth(clientOpts.Authenticator))
14201287
} else {
1421-
verifyOpts = append(verifyOpts, remote.WithAuthFromKeychain(keychain))
1288+
verifyOpts = append(verifyOpts, remote.WithAuthFromKeychain(clientOpts.Keychain))
14221289
}
14231290

14241291
switch obj.Spec.Verify.Provider {

0 commit comments

Comments
 (0)