Skip to content

Commit 77968f5

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 <[email protected]>
1 parent b8f9d6e commit 77968f5

10 files changed

+429
-221
lines changed

‎internal/controller/helmchart_controller.go

Lines changed: 17 additions & 119 deletions
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
@@ -1052,50 +983,17 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
1052983
ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
1053984
defer cancel()
1054985

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)
986+
hcOpts, err := repository.GetHelmClientOpts(ctxTimeout, r.Client, obj, normalizedURL, true)
1090987
if err != nil {
1091988
return nil, err
1092989
}
990+
clientOpts := hcOpts.GetterOpts
1093991

1094992
var chartRepo repository.Downloader
1095993
if helmreg.IsOCI(normalizedURL) {
1096-
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpt != nil)
994+
registryClient, credentialsFile, err := r.RegistryClientGenerator(hcOpts.LoginOpt != nil)
1097995
if err != nil {
1098-
return nil, fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", obj.Name, err)
996+
return nil, fmt.Errorf("failed to create registry client: %w", err)
1099997
}
1100998

1101999
var errs []error
@@ -1106,7 +1004,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
11061004
repository.WithOCIRegistryClient(registryClient),
11071005
repository.WithCredentialsFile(credentialsFile))
11081006
if err != nil {
1109-
errs = append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", obj.Name, err))
1007+
errs = append(errs, fmt.Errorf("failed to create OCI chart repository: %w", err))
11101008
// clean up the credentialsFile
11111009
if credentialsFile != "" {
11121010
if err := os.Remove(credentialsFile); err != nil {
@@ -1118,10 +1016,10 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
11181016

11191017
// If login options are configured, use them to login to the registry
11201018
// The OCIGetter will later retrieve the stored credentials to pull the chart
1121-
if loginOpt != nil {
1122-
err = ociChartRepo.Login(loginOpt)
1019+
if hcOpts.LoginOpt != nil {
1020+
err = ociChartRepo.Login(hcOpts.LoginOpt)
11231021
if err != nil {
1124-
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", obj.Name, err))
1022+
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository: %w", err))
11251023
// clean up the credentialsFile
11261024
errs = append(errs, ociChartRepo.Clear())
11271025
return nil, kerrors.NewAggregate(errs)
@@ -1130,7 +1028,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
11301028

11311029
chartRepo = ociChartRepo
11321030
} else {
1133-
httpChartRepo, err := repository.NewChartRepository(normalizedURL, "", r.Getters, tlsConfig, clientOpts...)
1031+
httpChartRepo, err := repository.NewChartRepository(normalizedURL, "", r.Getters, hcOpts.TlsConfig, clientOpts...)
11341032
if err != nil {
11351033
return nil, err
11361034
}

‎internal/controller/helmchart_controller_test.go

Lines changed: 4 additions & 4 deletions
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

Lines changed: 16 additions & 45 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"
@@ -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,33 @@ 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
390+
normalizedURL, err := repository.NormalizeURL(obj.Spec.URL)
391+
if err != nil {
392+
e := &serror.Stalling{
393+
Err: fmt.Errorf("invalid Helm repository URL: %w", err),
394+
Reason: sourcev1.URLInvalidReason,
429395
}
430-
clientOpts = append(clientOpts, opts...)
396+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
397+
return sreconcile.ResultEmpty, e
398+
}
431399

432-
tlsConfig, err = getter.TLSClientConfigFromSecret(secret, obj.Spec.URL)
433-
if err != nil {
400+
hcOpts, err := repository.GetHelmClientOpts(ctx, r.Client, obj, normalizedURL, false)
401+
if err != nil {
402+
if errors.Is(err, repository.DeprecatedTLSConfigErr) {
403+
r.Event(obj, "Warning", "DeprecationWarning",
404+
"specifying TLS authentication data via `.spec.secretRef` is deprecated, please use `.spec.certSecretRef` instead")
405+
} else {
434406
e := &serror.Event{
435-
Err: fmt.Errorf("failed to create TLS client config with secret data: %w", err),
407+
Err: err,
436408
Reason: sourcev1.AuthenticationFailedReason,
437409
}
438410
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
439-
// Requeue as content of secret might change
440411
return sreconcile.ResultEmpty, e
441412
}
442413
}
443414

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

‎internal/controller/helmrepository_controller_oci.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import (
5454
"github.com/fluxcd/source-controller/internal/helm/registry"
5555
"github.com/fluxcd/source-controller/internal/helm/repository"
5656
"github.com/fluxcd/source-controller/internal/object"
57+
soci "github.com/fluxcd/source-controller/internal/oci"
5758
intpredicates "github.com/fluxcd/source-controller/internal/predicates"
5859
)
5960

@@ -318,7 +319,7 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S
318319
return
319320
}
320321
} else if obj.Spec.Provider != helmv1.GenericOCIProvider && obj.Spec.Type == helmv1.HelmRepositoryTypeOCI {
321-
auth, authErr := oidcAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
322+
auth, authErr := soci.OIDCAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
322323
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
323324
e := fmt.Errorf("failed to get credential from %s: %w", obj.Spec.Provider, authErr)
324325
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error())

0 commit comments

Comments
 (0)