Skip to content

Commit

Permalink
controllers: RFC-0005 fmt for HelmRepository rev
Browse files Browse the repository at this point in the history 
This includes changes to the `ChartRepository`, to allow calculating
the revision and digest and tidy things.

In addition, the responsibility of caching the `IndexFile` has been
moved to the reconcilers. As this allowed to remove a lot of
complexities within the `ChartRepository`, and prevented passing on
the cache in general.

Change `HelmRepository`'s Revision to digest

Signed-off-by: Hidde Beydals <[email protected]>
  • Loading branch information
@hiddeco
hiddeco committed Feb 8, 2023
1 parent 93ec32d commit 3649d27
Show file tree
Hide file tree
Showing 7 changed files with 841 additions and 670 deletions.
100 changes: 52 additions & 48 deletions controllers/helmchart_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,20 @@ import (
"crypto/tls"
"errors"
"fmt"
"github.com/fluxcd/pkg/git"
"github.com/opencontainers/go-digest"

"net/url"
"os"
"path/filepath"
"strconv"
"strings"
"time"

eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
soci "github.com/fluxcd/source-controller/internal/oci"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/opencontainers/go-digest"
helmgetter "helm.sh/helm/v3/pkg/getter"
helmreg "helm.sh/helm/v3/pkg/registry"
helmrepo "helm.sh/helm/v3/pkg/repo"
corev1 "k8s.io/api/core/v1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -54,7 +53,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

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"
Expand All @@ -70,6 +71,7 @@ import (
"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"
"github.com/fluxcd/source-controller/internal/reconcile/summarize"
"github.com/fluxcd/source-controller/internal/util"
Expand Down Expand Up @@ -527,7 +529,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
}

// Build client options from secret
opts, tls, err := r.clientOptionsFromSecret(secret, normalizedURL)
opts, tlsCfg, err := r.clientOptionsFromSecret(secret, normalizedURL)
if err != nil {
e := &serror.Event{
Err: err,
Expand All @@ -538,7 +540,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
return sreconcile.ResultEmpty, e
}
clientOpts = append(clientOpts, opts...)
tlsConfig = tls
tlsConfig = tlsCfg

// Build registryClient options from secret
keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret)
Expand Down Expand Up @@ -651,34 +653,26 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
}
}
default:
httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts,
repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
r.IncCacheEvents(event, obj.Name, obj.Namespace)
}))
httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts...)
if err != nil {
return chartRepoConfigErrorReturn(err, obj)
}
chartRepo = httpChartRepo
defer func() {
if httpChartRepo == nil {
return
}
// Cache the index if it was successfully retrieved
// and the chart was successfully built
// and the chart was successfully built.
if r.Cache != nil && httpChartRepo.Index != nil {
// The cache key have to be safe in multi-tenancy environments,
// as otherwise it could be used as a vector to bypass the helm repository's authentication.
// Using r.Storage.LocalPath(*repo.GetArtifact() is safe as the path is in the format /<helm-repository-name>/<chart-name>/<filename>.
err := httpChartRepo.CacheIndexInMemory()
if err != nil {
// The cache key has to be safe in multi-tenancy environments,
// as otherwise it could be used as a vector to bypass
// authentication to the repository.
// Using r.Storage.LocalPath(*repo.GetArtifact()) is safe as the
// path is in the format of: /<repository-name>/<chart-name>/<filename>.
if err = r.Cache.Set(r.Storage.LocalPath(*repo.GetArtifact()), httpChartRepo.Index, r.TTL); err != nil {
r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.CacheOperationFailedReason, "failed to cache index: %s", err)
}
}

// Delete the index reference
if httpChartRepo.Index != nil {
httpChartRepo.Unload()
}
httpChartRepo.Clear()
}()
}

Expand Down Expand Up @@ -845,7 +839,7 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
// early.
// On a successful archive, the Artifact in the Status of the object is set,
// and the symlink in the Storage is updated to its path.
func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmChart, b *chart.Build) (sreconcile.Result, error) {
func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, _ *patch.SerialPatcher, obj *sourcev1.HelmChart, b *chart.Build) (sreconcile.Result, error) {
// Without a complete chart build, there is little to reconcile
if !b.Complete() {
return sreconcile.ResultRequeue, nil
Expand Down Expand Up @@ -1016,14 +1010,15 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
authenticator authn.Authenticator
keychain authn.Keychain
)

normalizedURL := repository.NormalizeURL(url)
repo, err := r.resolveDependencyRepository(ctx, url, namespace)
obj, err := r.resolveDependencyRepository(ctx, url, namespace)
if err != nil {
// Return Kubernetes client errors, but ignore others
if apierrs.ReasonForError(err) != metav1.StatusReasonUnknown {
return nil, err
}
repo = &sourcev1.HelmRepository{
obj = &sourcev1.HelmRepository{
Spec: sourcev1.HelmRepositorySpec{
URL: url,
Timeout: &metav1.Duration{Duration: 60 * time.Second},
Expand All @@ -1032,37 +1027,37 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
}

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

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

// Build client options from secret
opts, tls, err := r.clientOptionsFromSecret(secret, normalizedURL)
opts, tlsCfg, err := r.clientOptionsFromSecret(secret, normalizedURL)
if err != nil {
return nil, err
}
clientOpts = append(clientOpts, opts...)
tlsConfig = tls
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", repo.Name, err)
return nil, fmt.Errorf("failed to create login options for HelmRepository '%s': %w", obj.Name, err)
}

} else if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI {
auth, authErr := oidcAuth(ctxTimeout, repo.Spec.URL, repo.Spec.Provider)
} else if obj.Spec.Provider != sourcev1.GenericOCIProvider && obj.Spec.Type == sourcev1.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", repo.Spec.Provider, authErr)
return nil, fmt.Errorf("failed to get credential from %s: %w", obj.Spec.Provider, authErr)
}
if auth != nil {
authenticator = auth
Expand All @@ -1078,7 +1073,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
if helmreg.IsOCI(normalizedURL) {
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpt != nil)
if err != nil {
return nil, fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", repo.Name, err)
return nil, fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", obj.Name, err)
}

var errs []error
Expand All @@ -1089,7 +1084,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", repo.Name, err))
errs = append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", obj.Name, err))
// clean up the credentialsFile
if credentialsFile != "" {
if err := os.Remove(credentialsFile); err != nil {
Expand All @@ -1104,7 +1099,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
if loginOpt != nil {
err = ociChartRepo.Login(loginOpt)
if err != nil {
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", repo.Name, err))
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", obj.Name, err))
// clean up the credentialsFile
errs = append(errs, ociChartRepo.Clear())
return nil, kerrors.NewAggregate(errs)
Expand All @@ -1113,19 +1108,28 @@ 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, tlsConfig, clientOpts...)
if err != nil {
return nil, err
}

// Ensure that the cache key is the same as the artifact path
// otherwise don't enable caching. We don't want to cache indexes
// for repositories that are not reconciled by the source controller.
if repo.Status.Artifact != nil {
httpChartRepo.CachePath = r.Storage.LocalPath(*repo.GetArtifact())
httpChartRepo.SetMemCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
r.IncCacheEvents(event, name, namespace)
})
if obj.Status.Artifact != nil {
// Attempt to load the index from the cache.
httpChartRepo.Path = r.Storage.LocalPath(*obj.GetArtifact())
if r.Cache != nil {
if index, ok := r.Cache.Get(httpChartRepo.Path); ok {
r.IncCacheEvents(cache.CacheEventTypeHit, name, namespace)
r.Cache.SetExpiration(httpChartRepo.Path, r.TTL)

httpChartRepo.Index = index.(*helmrepo.IndexFile)
} else {
r.IncCacheEvents(cache.CacheEventTypeMiss, name, namespace)
if err := httpChartRepo.LoadFromPath(); err != nil {
return nil, err
}
r.Cache.Set(httpChartRepo.Path, httpChartRepo.Index, r.TTL)
}
}
}

chartRepo = httpChartRepo
Expand Down
Loading

0 comments on commit 3649d27

Please sign in to comment.