Skip to content

Commit 94e5235

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 emit a deprecation event 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 <jaiswalsanskar078@gmail.com>
1 parent b8f9d6e commit 94e5235

9 files changed

+426
-225
lines changed

‎internal/controller/helmchart_controller.go

+21-119
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ import (
5454
eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
5555
"github.com/fluxcd/pkg/apis/meta"
5656
"github.com/fluxcd/pkg/git"
57-
"github.com/fluxcd/pkg/oci"
5857
"github.com/fluxcd/pkg/runtime/conditions"
5958
helper "github.com/fluxcd/pkg/runtime/controller"
6059
"github.com/fluxcd/pkg/runtime/patch"
@@ -68,7 +67,6 @@ import (
6867
serror "github.com/fluxcd/source-controller/internal/error"
6968
"github.com/fluxcd/source-controller/internal/helm/chart"
7069
"github.com/fluxcd/source-controller/internal/helm/getter"
71-
"github.com/fluxcd/source-controller/internal/helm/registry"
7270
"github.com/fluxcd/source-controller/internal/helm/repository"
7371
soci "github.com/fluxcd/source-controller/internal/oci"
7472
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
@@ -506,11 +504,6 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, sp *patch.Ser
506504
// object, and returns early.
507505
func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *helmv1.HelmChart,
508506
repo *helmv1.HelmRepository, b *chart.Build) (sreconcile.Result, error) {
509-
var (
510-
tlsConfig *tls.Config
511-
authenticator authn.Authenticator
512-
keychain authn.Keychain
513-
)
514507
// Used to login with the repository declared provider
515508
ctxTimeout, cancel := context.WithTimeout(ctx, repo.Spec.Timeout.Duration)
516509
defer cancel()
@@ -519,64 +512,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
519512
if err != nil {
520513
return chartRepoConfigErrorReturn(err, obj)
521514
}
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)
515+
hcOpts, err := repository.GetHelmClientOpts(ctxTimeout, r.Client, repo, normalizedURL, true)
580516
if err != nil {
581517
e := &serror.Event{
582518
Err: err,
@@ -585,6 +521,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
585521
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
586522
return sreconcile.ResultEmpty, e
587523
}
524+
clientOpts := hcOpts.GetterOpts
588525

589526
// Initialize the chart repository
590527
var chartRepo repository.Downloader
@@ -599,7 +536,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
599536
// this is needed because otherwise the credentials are stored in ~/.docker/config.json.
600537
// TODO@souleb: remove this once the registry move to Oras v2
601538
// or rework to enable reusing credentials to avoid the unneccessary handshake operations
602-
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpt != nil)
539+
registryClient, credentialsFile, err := r.RegistryClientGenerator(hcOpts.LoginOpt != nil)
603540
if err != nil {
604541
e := &serror.Event{
605542
Err: fmt.Errorf("failed to construct Helm client: %w", err),
@@ -621,7 +558,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
621558
var verifiers []soci.Verifier
622559
if obj.Spec.Verify != nil {
623560
provider := obj.Spec.Verify.Provider
624-
verifiers, err = r.makeVerifiers(ctx, obj, authenticator, keychain)
561+
verifiers, err = r.makeVerifiers(ctx, obj, hcOpts.Authenticator, hcOpts.Keychain)
625562
if err != nil {
626563
if obj.Spec.Verify.SecretRef == nil {
627564
provider = fmt.Sprintf("%s keyless", provider)
@@ -645,12 +582,11 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
645582
if err != nil {
646583
return chartRepoConfigErrorReturn(err, obj)
647584
}
648-
chartRepo = ociChartRepo
649585

650586
// If login options are configured, use them to login to the registry
651587
// The OCIGetter will later retrieve the stored credentials to pull the chart
652-
if loginOpt != nil {
653-
err = ociChartRepo.Login(loginOpt)
588+
if hcOpts.LoginOpt != nil {
589+
err = ociChartRepo.Login(hcOpts.LoginOpt)
654590
if err != nil {
655591
e := &serror.Event{
656592
Err: fmt.Errorf("failed to login to OCI registry: %w", err),
@@ -660,8 +596,9 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
660596
return sreconcile.ResultEmpty, e
661597
}
662598
}
599+
chartRepo = ociChartRepo
663600
default:
664-
httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts...)
601+
httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, hcOpts.TlsConfig, clientOpts...)
665602
if err != nil {
666603
return chartRepoConfigErrorReturn(err, obj)
667604
}
@@ -1024,12 +961,6 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *helmv1.He
1024961
// The callback returns an object with a state, so the caller has to do the necessary cleanup.
1025962
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback {
1026963
return func(url string) (repository.Downloader, error) {
1027-
var (
1028-
tlsConfig *tls.Config
1029-
authenticator authn.Authenticator
1030-
keychain authn.Keychain
1031-
)
1032-
1033964
normalizedURL, err := repository.NormalizeURL(url)
1034965
if err != nil {
1035966
return nil, err
@@ -1047,55 +978,26 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
1047978
},
1048979
}
1049980
}
981+
objKey := types.NamespacedName{
982+
Name: obj.Name,
983+
Namespace: obj.Namespace,
984+
}
1050985

1051986
// Used to login with the repository declared provider
1052987
ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
1053988
defer cancel()
1054989

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)
990+
hcOpts, err := repository.GetHelmClientOpts(ctxTimeout, r.Client, obj, normalizedURL, true)
1090991
if err != nil {
1091992
return nil, err
1092993
}
994+
clientOpts := hcOpts.GetterOpts
1093995

1094996
var chartRepo repository.Downloader
1095997
if helmreg.IsOCI(normalizedURL) {
1096-
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpt != nil)
998+
registryClient, credentialsFile, err := r.RegistryClientGenerator(hcOpts.LoginOpt != nil)
1097999
if err != nil {
1098-
return nil, fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", obj.Name, err)
1000+
return nil, fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", objKey.String(), err)
10991001
}
11001002

11011003
var errs []error
@@ -1106,7 +1008,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
11061008
repository.WithOCIRegistryClient(registryClient),
11071009
repository.WithCredentialsFile(credentialsFile))
11081010
if err != nil {
1109-
errs = append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", obj.Name, err))
1011+
errs = append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", objKey.String(), err))
11101012
// clean up the credentialsFile
11111013
if credentialsFile != "" {
11121014
if err := os.Remove(credentialsFile); err != nil {
@@ -1118,10 +1020,10 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
11181020

11191021
// If login options are configured, use them to login to the registry
11201022
// The OCIGetter will later retrieve the stored credentials to pull the chart
1121-
if loginOpt != nil {
1122-
err = ociChartRepo.Login(loginOpt)
1023+
if hcOpts.LoginOpt != nil {
1024+
err = ociChartRepo.Login(hcOpts.LoginOpt)
11231025
if err != nil {
1124-
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", obj.Name, err))
1026+
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", objKey.String(), err))
11251027
// clean up the credentialsFile
11261028
errs = append(errs, ociChartRepo.Clear())
11271029
return nil, kerrors.NewAggregate(errs)
@@ -1130,7 +1032,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
11301032

11311033
chartRepo = ociChartRepo
11321034
} else {
1133-
httpChartRepo, err := repository.NewChartRepository(normalizedURL, "", r.Getters, tlsConfig, clientOpts...)
1035+
httpChartRepo, err := repository.NewChartRepository(normalizedURL, "", r.Getters, hcOpts.TlsConfig, clientOpts...)
11341036
if err != nil {
11351037
return nil, err
11361038
}

‎internal/controller/helmchart_controller_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -922,12 +922,12 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) {
922922
}
923923
},
924924
want: sreconcile.ResultEmpty,
925-
wantErr: &serror.Event{Err: errors.New("failed to get secret 'invalid'")},
925+
wantErr: &serror.Event{Err: errors.New("failed to get authentication secret '/invalid'")},
926926
assertFunc: func(g *WithT, obj *helmv1.HelmChart, build chart.Build) {
927927
g.Expect(build.Complete()).To(BeFalse())
928928

929929
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
930-
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret 'invalid'"),
930+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret '/invalid'"),
931931
}))
932932
},
933933
},
@@ -1190,12 +1190,12 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
11901190
}
11911191
},
11921192
want: sreconcile.ResultEmpty,
1193-
wantErr: &serror.Event{Err: errors.New("failed to get secret 'invalid'")},
1193+
wantErr: &serror.Event{Err: errors.New("failed to get authentication secret '/invalid'")},
11941194
assertFunc: func(g *WithT, obj *helmv1.HelmChart, build chart.Build) {
11951195
g.Expect(build.Complete()).To(BeFalse())
11961196

11971197
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
1198-
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret 'invalid'"),
1198+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret '/invalid'"),
11991199
}))
12001200
},
12011201
},

‎internal/controller/helmrepository_controller.go

+11-50
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"
@@ -29,7 +28,6 @@ import (
2928
helmgetter "helm.sh/helm/v3/pkg/getter"
3029
corev1 "k8s.io/api/core/v1"
3130
"k8s.io/apimachinery/pkg/runtime"
32-
"k8s.io/apimachinery/pkg/types"
3331
kuberecorder "k8s.io/client-go/tools/record"
3432
ctrl "sigs.k8s.io/controller-runtime"
3533
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -51,7 +49,6 @@ import (
5149
"github.com/fluxcd/source-controller/internal/cache"
5250
intdigest "github.com/fluxcd/source-controller/internal/digest"
5351
serror "github.com/fluxcd/source-controller/internal/error"
54-
"github.com/fluxcd/source-controller/internal/helm/getter"
5552
"github.com/fluxcd/source-controller/internal/helm/repository"
5653
intpredicates "github.com/fluxcd/source-controller/internal/predicates"
5754
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
@@ -390,59 +387,23 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, sp *pat
390387
// pointer is set to the newly fetched index.
391388
func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher,
392389
obj *helmv1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) {
393-
var tlsConfig *tls.Config
394-
395-
// Configure Helm client to access repository
396-
clientOpts := []helmgetter.Option{
397-
helmgetter.WithTimeout(obj.Spec.Timeout.Duration),
398-
helmgetter.WithURL(obj.Spec.URL),
399-
helmgetter.WithPassCredentialsAll(obj.Spec.PassCredentials),
400-
}
401-
402-
// Configure any authentication related options
403-
if obj.Spec.SecretRef != nil {
404-
// Attempt to retrieve secret
405-
name := types.NamespacedName{
406-
Namespace: obj.GetNamespace(),
407-
Name: obj.Spec.SecretRef.Name,
408-
}
409-
var secret corev1.Secret
410-
if err := r.Client.Get(ctx, name, &secret); err != nil {
411-
e := &serror.Event{
412-
Err: fmt.Errorf("failed to get secret '%s': %w", name.String(), err),
413-
Reason: sourcev1.AuthenticationFailedReason,
414-
}
415-
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
416-
return sreconcile.ResultEmpty, e
417-
}
418-
419-
// Construct actual options
420-
opts, err := getter.ClientOptionsFromSecret(secret)
421-
if err != nil {
422-
e := &serror.Event{
423-
Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err),
424-
Reason: sourcev1.AuthenticationFailedReason,
425-
}
426-
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
427-
// Return err as the content of the secret may change.
428-
return sreconcile.ResultEmpty, e
429-
}
430-
clientOpts = append(clientOpts, opts...)
431-
432-
tlsConfig, err = getter.TLSClientConfigFromSecret(secret, obj.Spec.URL)
433-
if err != nil {
434-
e := &serror.Event{
435-
Err: fmt.Errorf("failed to create TLS client config with secret data: %w", err),
436-
Reason: sourcev1.AuthenticationFailedReason,
437-
}
390+
hcOpts, err := repository.GetHelmClientOpts(ctx, r.Client, obj, obj.Spec.URL, false)
391+
if err != nil {
392+
if errors.Is(err, repository.DeprecatedTLSConfigErr) {
393+
r.Event(obj, "Warning", "DeprecationWarning",
394+
"specifying TLS authentication data via `.spec.secretRef` is deprecated, please use `.spec.certSecretRef` instead")
395+
} else {
396+
e := serror.NewGeneric(
397+
err,
398+
sourcev1.AuthenticationFailedReason,
399+
)
438400
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
439-
// Requeue as content of secret might change
440401
return sreconcile.ResultEmpty, e
441402
}
442403
}
443404

444405
// Construct Helm chart repository with options and download index
445-
newChartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", r.Getters, tlsConfig, clientOpts...)
406+
newChartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", r.Getters, hcOpts.TlsConfig, hcOpts.GetterOpts...)
446407
if err != nil {
447408
switch err.(type) {
448409
case *url.Error:

‎internal/controller/helmrepository_controller_oci.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S
318318
return
319319
}
320320
} else if obj.Spec.Provider != helmv1.GenericOCIProvider && obj.Spec.Type == helmv1.HelmRepositoryTypeOCI {
321-
auth, authErr := oidcAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
321+
auth, authErr := registry.OIDCAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
322322
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
323323
e := fmt.Errorf("failed to get credential from %s: %w", obj.Spec.Provider, authErr)
324324
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error())

0 commit comments

Comments
 (0)