Skip to content

Commit

Permalink
helm: add support for specifying TLS auth via .spec.certSecretRef
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@aryan9600
aryan9600 committed Jul 18, 2023
1 parent 6fb9749 commit f7298cd
Show file tree
Hide file tree
Showing 9 changed files with 385 additions and 227 deletions.
140 changes: 21 additions & 119 deletions internal/controller/helmchart_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ import (
eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/git"
"github.com/fluxcd/pkg/oci"
"github.com/fluxcd/pkg/runtime/conditions"
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/patch"
Expand All @@ -68,7 +67,6 @@ import (
serror "github.com/fluxcd/source-controller/internal/error"
"github.com/fluxcd/source-controller/internal/helm/chart"
"github.com/fluxcd/source-controller/internal/helm/getter"
"github.com/fluxcd/source-controller/internal/helm/registry"
"github.com/fluxcd/source-controller/internal/helm/repository"
soci "github.com/fluxcd/source-controller/internal/oci"
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
Expand Down Expand Up @@ -506,11 +504,6 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, sp *patch.Ser
// object, and returns early.
func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *helmv1.HelmChart,
repo *helmv1.HelmRepository, b *chart.Build) (sreconcile.Result, error) {
var (
tlsConfig *tls.Config
authenticator authn.Authenticator
keychain authn.Keychain
)
// Used to login with the repository declared provider
ctxTimeout, cancel := context.WithTimeout(ctx, repo.Spec.Timeout.Duration)
defer cancel()
Expand All @@ -519,64 +512,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
if err != nil {
return chartRepoConfigErrorReturn(err, obj)
}
// Construct the Getter options from the HelmRepository data
clientOpts := []helmgetter.Option{
helmgetter.WithURL(normalizedURL),
helmgetter.WithTimeout(repo.Spec.Timeout.Duration),
helmgetter.WithPassCredentialsAll(repo.Spec.PassCredentials),
}
if secret, err := r.getHelmRepositorySecret(ctx, repo); secret != nil || err != nil {
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to get secret '%s': %w", repo.Spec.SecretRef.Name, err),
Reason: sourcev1.AuthenticationFailedReason,
}
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
// Return error as the world as observed may change
return sreconcile.ResultEmpty, e
}

// Build client options from secret
opts, tlsCfg, err := r.clientOptionsFromSecret(secret, normalizedURL)
if err != nil {
e := &serror.Event{
Err: err,
Reason: sourcev1.AuthenticationFailedReason,
}
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
// Requeue as content of secret might change
return sreconcile.ResultEmpty, e
}
clientOpts = append(clientOpts, opts...)
tlsConfig = tlsCfg

// Build registryClient options from secret
keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret)
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err),
Reason: sourcev1.AuthenticationFailedReason,
}
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
// Requeue as content of secret might change
return sreconcile.ResultEmpty, e
}
} else if repo.Spec.Provider != helmv1.GenericOCIProvider && repo.Spec.Type == helmv1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuth(ctxTimeout, repo.Spec.URL, repo.Spec.Provider)
if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) {
e := &serror.Event{
Err: fmt.Errorf("failed to get credential from %s: %w", repo.Spec.Provider, authErr),
Reason: sourcev1.AuthenticationFailedReason,
}
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}
if auth != nil {
authenticator = auth
}
}

loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL)
hcOpts, err := repository.GetHelmClientOpts(ctxTimeout, r.Client, repo, normalizedURL, true)
if err != nil {
e := &serror.Event{
Err: err,
Expand All @@ -585,6 +521,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}
clientOpts := hcOpts.GetterOpts

// Initialize the chart repository
var chartRepo repository.Downloader
Expand All @@ -599,7 +536,7 @@ 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(loginOpt != nil)
registryClient, credentialsFile, err := r.RegistryClientGenerator(hcOpts.LoginOpt != nil)
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to construct Helm client: %w", err),
Expand All @@ -621,7 +558,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
var verifiers []soci.Verifier
if obj.Spec.Verify != nil {
provider := obj.Spec.Verify.Provider
verifiers, err = r.makeVerifiers(ctx, obj, authenticator, keychain)
verifiers, err = r.makeVerifiers(ctx, obj, hcOpts.Authenticator, hcOpts.Keychain)
if err != nil {
if obj.Spec.Verify.SecretRef == nil {
provider = fmt.Sprintf("%s keyless", provider)
Expand All @@ -645,12 +582,11 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
if err != nil {
return chartRepoConfigErrorReturn(err, obj)
}
chartRepo = ociChartRepo

// 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 loginOpt != nil {
err = ociChartRepo.Login(loginOpt)
if hcOpts.LoginOpt != nil {
err = ociChartRepo.Login(hcOpts.LoginOpt)
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to login to OCI registry: %w", err),
Expand All @@ -660,8 +596,9 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
return sreconcile.ResultEmpty, e
}
}
chartRepo = ociChartRepo
default:
httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts...)
httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, hcOpts.TlsConfig, clientOpts...)
if err != nil {
return chartRepoConfigErrorReturn(err, obj)
}
Expand Down Expand Up @@ -1024,12 +961,6 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *helmv1.He
// The callback returns an object with a state, so the caller has to do the necessary cleanup.
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback {
return func(url string) (repository.Downloader, error) {
var (
tlsConfig *tls.Config
authenticator authn.Authenticator
keychain authn.Keychain
)

normalizedURL, err := repository.NormalizeURL(url)
if err != nil {
return nil, err
Expand All @@ -1047,55 +978,26 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
},
}
}
objKey := types.NamespacedName{
Name: obj.Name,
Namespace: obj.Namespace,
}

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

clientOpts := []helmgetter.Option{
helmgetter.WithURL(normalizedURL),
helmgetter.WithTimeout(obj.Spec.Timeout.Duration),
helmgetter.WithPassCredentialsAll(obj.Spec.PassCredentials),
}
if secret, err := r.getHelmRepositorySecret(ctx, obj); secret != nil || err != nil {
if err != nil {
return nil, err
}

// Build client options from secret
opts, tlsCfg, err := r.clientOptionsFromSecret(secret, normalizedURL)
if err != nil {
return nil, err
}
clientOpts = append(clientOpts, opts...)
tlsConfig = tlsCfg

// Build registryClient options from secret
keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret)
if err != nil {
return nil, fmt.Errorf("failed to create login options for HelmRepository '%s': %w", obj.Name, err)
}

} else if obj.Spec.Provider != helmv1.GenericOCIProvider && obj.Spec.Type == helmv1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuth(ctxTimeout, 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)
}
if auth != nil {
authenticator = auth
}
}

loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL)
hcOpts, err := repository.GetHelmClientOpts(ctxTimeout, r.Client, obj, normalizedURL, true)
if err != nil {
return nil, err
}
clientOpts := hcOpts.GetterOpts

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

var errs []error
Expand All @@ -1106,7 +1008,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
repository.WithOCIRegistryClient(registryClient),
repository.WithCredentialsFile(credentialsFile))
if err != nil {
errs = append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", obj.Name, err))
errs = append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", objKey.String(), err))
// clean up the credentialsFile
if credentialsFile != "" {
if err := os.Remove(credentialsFile); err != nil {
Expand All @@ -1118,10 +1020,10 @@ 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 loginOpt != nil {
err = ociChartRepo.Login(loginOpt)
if hcOpts.LoginOpt != nil {
err = ociChartRepo.Login(hcOpts.LoginOpt)
if err != nil {
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", obj.Name, err))
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", objKey.String(), err))
// clean up the credentialsFile
errs = append(errs, ociChartRepo.Clear())
return nil, kerrors.NewAggregate(errs)
Expand All @@ -1130,7 +1032,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont

chartRepo = ociChartRepo
} else {
httpChartRepo, err := repository.NewChartRepository(normalizedURL, "", r.Getters, tlsConfig, clientOpts...)
httpChartRepo, err := repository.NewChartRepository(normalizedURL, "", r.Getters, hcOpts.TlsConfig, clientOpts...)
if err != nil {
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions internal/controller/helmchart_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -922,12 +922,12 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) {
}
},
want: sreconcile.ResultEmpty,
wantErr: &serror.Event{Err: errors.New("failed to get secret 'invalid'")},
wantErr: &serror.Event{Err: errors.New("failed to get authentication secret '/invalid'")},
assertFunc: func(g *WithT, obj *helmv1.HelmChart, build chart.Build) {
g.Expect(build.Complete()).To(BeFalse())

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

g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret 'invalid'"),
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get authentication secret '/invalid'"),
}))
},
},
Expand Down
64 changes: 12 additions & 52 deletions internal/controller/helmrepository_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package controller

import (
"context"
"crypto/tls"
"errors"
"fmt"
"net/url"
Expand All @@ -29,7 +28,6 @@ import (
helmgetter "helm.sh/helm/v3/pkg/getter"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
kuberecorder "k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -51,7 +49,6 @@ import (
"github.com/fluxcd/source-controller/internal/cache"
intdigest "github.com/fluxcd/source-controller/internal/digest"
serror "github.com/fluxcd/source-controller/internal/error"
"github.com/fluxcd/source-controller/internal/helm/getter"
"github.com/fluxcd/source-controller/internal/helm/repository"
intpredicates "github.com/fluxcd/source-controller/internal/predicates"
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
Expand Down Expand Up @@ -390,59 +387,22 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, sp *pat
// pointer is set to the newly fetched index.
func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher,
obj *helmv1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) {
var tlsConfig *tls.Config

// Configure Helm client to access repository
clientOpts := []helmgetter.Option{
helmgetter.WithTimeout(obj.Spec.Timeout.Duration),
helmgetter.WithURL(obj.Spec.URL),
helmgetter.WithPassCredentialsAll(obj.Spec.PassCredentials),
hcOpts, err := repository.GetHelmClientOpts(ctx, r.Client, obj, obj.Spec.URL, false)
if err != nil {
e := serror.NewGeneric(
err,
sourcev1.AuthenticationFailedReason,
)
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}

// 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 {
e := &serror.Event{
Err: fmt.Errorf("failed to get secret '%s': %w", name.String(), err),
Reason: sourcev1.AuthenticationFailedReason,
}
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}

// Construct actual options
opts, err := getter.ClientOptionsFromSecret(secret)
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err),
Reason: sourcev1.AuthenticationFailedReason,
}
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
// Return err as the content of the secret may change.
return sreconcile.ResultEmpty, e
}
clientOpts = append(clientOpts, opts...)

tlsConfig, err = getter.TLSClientConfigFromSecret(secret, obj.Spec.URL)
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to create TLS client config with secret data: %w", err),
Reason: sourcev1.AuthenticationFailedReason,
}
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
// Requeue as content of secret might change
return sreconcile.ResultEmpty, e
}
if hcOpts.EmitDeprecationWarning {
r.Event(obj, "Warning", "DeprecationWarning",
"specifying TLS authentication data via `.spec.secretRef` is deprecated, please use `.spec.certSecretRef` instead")
}

// Construct Helm chart repository with options and download index
newChartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", r.Getters, tlsConfig, clientOpts...)
newChartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", r.Getters, hcOpts.TlsConfig, hcOpts.GetterOpts...)
if err != nil {
switch err.(type) {
case *url.Error:
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/helmrepository_controller_oci.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.S
return
}
} else if obj.Spec.Provider != helmv1.GenericOCIProvider && obj.Spec.Type == helmv1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider)
auth, authErr := registry.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())
Expand Down
Loading

0 comments on commit f7298cd

Please sign in to comment.