Skip to content

Commit bb83270

Browse files
committed
Refactor to use authn for authentication as OCIrepository does
If implemented the oras registry loginOption will only be used internaly with the specific ChartRepo struct. This will permit reusing more easily feature developped with googlecontainerregistry authn. Signed-off-by: Soule BA <[email protected]>
1 parent d372531 commit bb83270

File tree

5 files changed

+182
-94
lines changed

5 files changed

+182
-94
lines changed

controllers/helmchart_controller.go

Lines changed: 55 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import (
5656
"github.com/fluxcd/pkg/runtime/patch"
5757
"github.com/fluxcd/pkg/runtime/predicates"
5858
"github.com/fluxcd/pkg/untar"
59+
"github.com/google/go-containerregistry/pkg/authn"
5960

6061
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
6162
"github.com/fluxcd/source-controller/internal/cache"
@@ -455,8 +456,9 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1
455456
func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *sourcev1.HelmChart,
456457
repo *sourcev1.HelmRepository, b *chart.Build) (sreconcile.Result, error) {
457458
var (
458-
tlsConfig *tls.Config
459-
loginOpts []helmreg.LoginOption
459+
tlsConfig *tls.Config
460+
authenticator authn.Authenticator
461+
keychain authn.Keychain
460462
)
461463
// Used to login with the repository declared provider
462464
ctxTimeout, cancel := context.WithTimeout(ctx, repo.Spec.Timeout.Duration)
@@ -481,31 +483,21 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
481483
}
482484

483485
// Build client options from secret
484-
opts, err := getter.ClientOptionsFromSecret(*secret)
486+
opts, tls, err := r.clientOptionsFromSecret(secret, normalizedURL)
485487
if err != nil {
486488
e := &serror.Event{
487-
Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err),
489+
Err: err,
488490
Reason: sourcev1.AuthenticationFailedReason,
489491
}
490492
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
491493
// Requeue as content of secret might change
492494
return sreconcile.ResultEmpty, e
493495
}
494496
clientOpts = append(clientOpts, opts...)
495-
496-
tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, normalizedURL)
497-
if err != nil {
498-
e := &serror.Event{
499-
Err: fmt.Errorf("failed to create TLS client config with secret data: %w", err),
500-
Reason: sourcev1.AuthenticationFailedReason,
501-
}
502-
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
503-
// Requeue as content of secret might change
504-
return sreconcile.ResultEmpty, e
505-
}
497+
tlsConfig = tls
506498

507499
// Build registryClient options from secret
508-
loginOpt, err := registry.LoginOptionFromSecret(normalizedURL, *secret)
500+
keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret)
509501
if err != nil {
510502
e := &serror.Event{
511503
Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err),
@@ -515,10 +507,8 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
515507
// Requeue as content of secret might change
516508
return sreconcile.ResultEmpty, e
517509
}
518-
519-
loginOpts = append([]helmreg.LoginOption{}, loginOpt)
520510
} else if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
521-
auth, authErr := oidcAuthFromAdapter(ctxTimeout, repo.Spec.URL, repo.Spec.Provider)
511+
auth, authErr := oidcAuth(ctxTimeout, repo.Spec.URL, repo.Spec.Provider)
522512
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
523513
e := &serror.Event{
524514
Err: fmt.Errorf("failed to get credential from %s: %w", repo.Spec.Provider, authErr),
@@ -528,10 +518,20 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
528518
return sreconcile.ResultEmpty, e
529519
}
530520
if auth != nil {
531-
loginOpts = append([]helmreg.LoginOption{}, auth)
521+
authenticator = auth
532522
}
533523
}
534524

525+
loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL)
526+
if err != nil {
527+
e := &serror.Event{
528+
Err: err,
529+
Reason: sourcev1.AuthenticationFailedReason,
530+
}
531+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
532+
return sreconcile.ResultEmpty, e
533+
}
534+
535535
// Initialize the chart repository
536536
var chartRepo repository.Downloader
537537
switch repo.Spec.Type {
@@ -545,7 +545,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
545545
// this is needed because otherwise the credentials are stored in ~/.docker/config.json.
546546
// TODO@souleb: remove this once the registry move to Oras v2
547547
// or rework to enable reusing credentials to avoid the unneccessary handshake operations
548-
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpts != nil)
548+
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpt != nil)
549549
if err != nil {
550550
e := &serror.Event{
551551
Err: fmt.Errorf("failed to construct Helm client: %w", err),
@@ -574,8 +574,8 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
574574

575575
// If login options are configured, use them to login to the registry
576576
// The OCIGetter will later retrieve the stored credentials to pull the chart
577-
if loginOpts != nil {
578-
err = ociChartRepo.Login(loginOpts...)
577+
if keychain != nil {
578+
err = ociChartRepo.Login(loginOpt)
579579
if err != nil {
580580
e := &serror.Event{
581581
Err: fmt.Errorf("failed to login to OCI registry: %w", err),
@@ -941,8 +941,9 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1.
941941
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback {
942942
return func(url string) (repository.Downloader, error) {
943943
var (
944-
tlsConfig *tls.Config
945-
loginOpts []helmreg.LoginOption
944+
tlsConfig *tls.Config
945+
authenticator authn.Authenticator
946+
keychain authn.Keychain
946947
)
947948
normalizedURL := repository.NormalizeURL(url)
948949
repo, err := r.resolveDependencyRepository(ctx, url, namespace)
@@ -972,37 +973,39 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
972973
if err != nil {
973974
return nil, err
974975
}
975-
opts, err := getter.ClientOptionsFromSecret(*secret)
976+
977+
// Build client options from secret
978+
opts, tls, err := r.clientOptionsFromSecret(secret, normalizedURL)
976979
if err != nil {
977980
return nil, err
978981
}
979982
clientOpts = append(clientOpts, opts...)
980-
981-
tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, normalizedURL)
982-
if err != nil {
983-
return nil, fmt.Errorf("failed to create TLS client config for HelmRepository '%s': %w", repo.Name, err)
984-
}
983+
tlsConfig = tls
985984

986985
// Build registryClient options from secret
987-
loginOpt, err := registry.LoginOptionFromSecret(normalizedURL, *secret)
986+
keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret)
988987
if err != nil {
989988
return nil, fmt.Errorf("failed to create login options for HelmRepository '%s': %w", repo.Name, err)
990989
}
991990

992-
loginOpts = append([]helmreg.LoginOption{}, loginOpt)
993991
} else if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
994-
auth, authErr := oidcAuthFromAdapter(ctxTimeout, repo.Spec.URL, repo.Spec.Provider)
992+
auth, authErr := oidcAuth(ctxTimeout, repo.Spec.URL, repo.Spec.Provider)
995993
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
996994
return nil, fmt.Errorf("failed to get credential from %s: %w", repo.Spec.Provider, authErr)
997995
}
998996
if auth != nil {
999-
loginOpts = append([]helmreg.LoginOption{}, auth)
997+
authenticator = auth
1000998
}
1001999
}
10021000

1001+
loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL)
1002+
if err != nil {
1003+
return nil, err
1004+
}
1005+
10031006
var chartRepo repository.Downloader
10041007
if helmreg.IsOCI(normalizedURL) {
1005-
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpts != nil)
1008+
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpt != nil)
10061009
if err != nil {
10071010
return nil, fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", repo.Name, err)
10081011
}
@@ -1027,8 +1030,8 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
10271030

10281031
// If login options are configured, use them to login to the registry
10291032
// The OCIGetter will later retrieve the stored credentials to pull the chart
1030-
if loginOpts != nil {
1031-
err = ociChartRepo.Login(loginOpts...)
1033+
if keychain != nil {
1034+
err = ociChartRepo.Login(loginOpt)
10321035
if err != nil {
10331036
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", repo.Name, err))
10341037
// clean up the credentialsFile
@@ -1078,6 +1081,20 @@ func (r *HelmChartReconciler) resolveDependencyRepository(ctx context.Context, u
10781081
return nil, fmt.Errorf("no HelmRepository found for '%s' in '%s' namespace", url, namespace)
10791082
}
10801083

1084+
func (r *HelmChartReconciler) clientOptionsFromSecret(secret *corev1.Secret, normalizedURL string) ([]helmgetter.Option, *tls.Config, error) {
1085+
opts, err := getter.ClientOptionsFromSecret(*secret)
1086+
if err != nil {
1087+
return nil, nil, fmt.Errorf("failed to configure Helm client with secret data: %w", err)
1088+
}
1089+
1090+
tlsConfig, err := getter.TLSClientConfigFromSecret(*secret, normalizedURL)
1091+
if err != nil {
1092+
return nil, nil, fmt.Errorf("failed to create TLS client config with secret data: %w", err)
1093+
}
1094+
1095+
return opts, tlsConfig, nil
1096+
}
1097+
10811098
func (r *HelmChartReconciler) getHelmRepositorySecret(ctx context.Context, repository *sourcev1.HelmRepository) (*corev1.Secret, error) {
10821099
if repository.Spec.SecretRef == nil {
10831100
return nil, nil

controllers/helmrepository_controller_oci.go

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import (
4545
helper "github.com/fluxcd/pkg/runtime/controller"
4646
"github.com/fluxcd/pkg/runtime/patch"
4747
"github.com/fluxcd/pkg/runtime/predicates"
48+
"github.com/google/go-containerregistry/pkg/authn"
4849

4950
"github.com/fluxcd/source-controller/api/v1beta2"
5051
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
@@ -263,49 +264,41 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta
263264
}
264265
conditions.Delete(obj, meta.StalledCondition)
265266

266-
var loginOpts []helmreg.LoginOption
267+
var (
268+
authenticator authn.Authenticator
269+
keychain authn.Keychain
270+
err error
271+
)
267272
// Configure any authentication related options.
268273
if obj.Spec.SecretRef != nil {
269-
// Attempt to retrieve secret.
270-
name := types.NamespacedName{
271-
Namespace: obj.GetNamespace(),
272-
Name: obj.Spec.SecretRef.Name,
273-
}
274-
var secret corev1.Secret
275-
if err := r.Client.Get(ctx, name, &secret); err != nil {
276-
e := fmt.Errorf("failed to get secret '%s': %w", name.String(), err)
277-
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error())
278-
result, retErr = ctrl.Result{}, e
279-
return
280-
}
281-
282-
// Construct login options.
283-
loginOpt, err := registry.LoginOptionFromSecret(obj.Spec.URL, secret)
274+
keychain, err = authFromSecret(ctx, r.Client, obj)
284275
if err != nil {
285-
e := fmt.Errorf("failed to configure Helm client with secret data: %w", err)
286-
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error())
287-
result, retErr = ctrl.Result{}, e
276+
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error())
277+
result, retErr = ctrl.Result{}, err
288278
return
289279
}
290-
291-
if loginOpt != nil {
292-
loginOpts = append(loginOpts, loginOpt)
293-
}
294280
} else if obj.Spec.Provider != sourcev1.GenericOCIProvider && obj.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
295-
auth, authErr := oidcAuthFromAdapter(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
281+
auth, authErr := oidcAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
296282
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
297283
e := fmt.Errorf("failed to get credential from %s: %w", obj.Spec.Provider, authErr)
298284
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error())
299285
result, retErr = ctrl.Result{}, e
300286
return
301287
}
302288
if auth != nil {
303-
loginOpts = append(loginOpts, auth)
289+
authenticator = auth
304290
}
305291
}
306292

293+
loginOpt, err := makeLoginOption(authenticator, keychain, obj.Spec.URL)
294+
if err != nil {
295+
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error())
296+
result, retErr = ctrl.Result{}, err
297+
return
298+
}
299+
307300
// Create registry client and login if needed.
308-
registryClient, file, err := r.RegistryClientGenerator(loginOpts != nil)
301+
registryClient, file, err := r.RegistryClientGenerator(loginOpt != nil)
309302
if err != nil {
310303
e := fmt.Errorf("failed to create registry client: %w", err)
311304
conditions.MarkFalse(obj, meta.ReadyCondition, meta.FailedReason, e.Error())
@@ -332,8 +325,8 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta
332325
conditions.Delete(obj, meta.StalledCondition)
333326

334327
// Attempt to login to the registry if credentials are provided.
335-
if loginOpts != nil {
336-
err = chartRepo.Login(loginOpts...)
328+
if loginOpt != nil {
329+
err = chartRepo.Login(loginOpt)
337330
if err != nil {
338331
e := fmt.Errorf("failed to login to registry '%s': %w", obj.Spec.URL, err)
339332
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error())
@@ -375,16 +368,37 @@ func (r *HelmRepositoryOCIReconciler) eventLogf(ctx context.Context, obj runtime
375368
r.Eventf(obj, eventType, reason, msg)
376369
}
377370

378-
// oidcAuthFromAdapter generates the OIDC credential authenticator based on the specified cloud provider.
379-
func oidcAuthFromAdapter(ctx context.Context, url, provider string) (helmreg.LoginOption, error) {
380-
auth, err := oidcAuth(ctx, url, provider)
371+
// authFromSecret returns an authn.Keychain for the given HelmRepository.
372+
// If the HelmRepository does not specify a secretRef, an anonymous keychain is returned.
373+
func authFromSecret(ctx context.Context, client client.Client, obj *sourcev1.HelmRepository) (authn.Keychain, error) {
374+
// Attempt to retrieve secret.
375+
name := types.NamespacedName{
376+
Namespace: obj.GetNamespace(),
377+
Name: obj.Spec.SecretRef.Name,
378+
}
379+
var secret corev1.Secret
380+
if err := client.Get(ctx, name, &secret); err != nil {
381+
return nil, fmt.Errorf("failed to get secret '%s': %w", name.String(), err)
382+
}
383+
384+
// Construct login options.
385+
keychain, err := registry.LoginOptionFromSecret(obj.Spec.URL, secret)
381386
if err != nil {
382-
return nil, err
387+
return nil, fmt.Errorf("failed to configure Helm client with secret data: %w", err)
388+
}
389+
return keychain, nil
390+
}
391+
392+
// makeLoginOption returns a registry login option for the given HelmRepository.
393+
// If the HelmRepository does not specify a secretRef, a nil login option is returned.
394+
func makeLoginOption(auth authn.Authenticator, keychain authn.Keychain, registryURL string) (helmreg.LoginOption, error) {
395+
if auth != nil {
396+
return registry.AuthAdaptHelper(auth)
383397
}
384398

385-
if auth == nil {
386-
return nil, fmt.Errorf("could not validate OCI provider %s with URL %s", provider, url)
399+
if keychain != nil {
400+
return registry.KeychainAdaptHelper(keychain)(registryURL)
387401
}
388402

389-
return registry.OIDCAdaptHelper(auth)
403+
return nil, nil
390404
}

0 commit comments

Comments
 (0)