Skip to content

Commit 3cfbcc0

Browse files
soulebsouleb
committed
Enable remote dependencies from OCI repositories
If implemented, the source controller will be able to resolve charts dependencies from OCI repositories. The remote builder has been refactored as part of this work. Signed-off-by: Soule BA <soule@weave.works>
1 parent e77eda5 commit 3cfbcc0

14 files changed

+547
-167
lines changed

‎controllers/helmchart_controller.go

+96-32
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3636
"k8s.io/apimachinery/pkg/runtime"
3737
"k8s.io/apimachinery/pkg/types"
38+
kerrors "k8s.io/apimachinery/pkg/util/errors"
3839
"k8s.io/apimachinery/pkg/util/uuid"
3940
kuberecorder "k8s.io/client-go/tools/record"
4041
ctrl "sigs.k8s.io/controller-runtime"
@@ -461,9 +462,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
461462
loginOpts []helmreg.LoginOption
462463
)
463464

465+
normalizedURL := repository.NormalizeURL(repo.Spec.URL)
464466
// Construct the Getter options from the HelmRepository data
465467
clientOpts := []helmgetter.Option{
466-
helmgetter.WithURL(repo.Spec.URL),
468+
helmgetter.WithURL(normalizedURL),
467469
helmgetter.WithTimeout(repo.Spec.Timeout.Duration),
468470
helmgetter.WithPassCredentialsAll(repo.Spec.PassCredentials),
469471
}
@@ -491,7 +493,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
491493
}
492494
clientOpts = append(clientOpts, opts...)
493495

494-
tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, repo.Spec.URL)
496+
tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, normalizedURL)
495497
if err != nil {
496498
e := &serror.Event{
497499
Err: fmt.Errorf("failed to create TLS client config with secret data: %w", err),
@@ -503,7 +505,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
503505
}
504506

505507
// Build registryClient options from secret
506-
loginOpt, err := registry.LoginOptionFromSecret(repo.Spec.URL, *secret)
508+
loginOpt, err := registry.LoginOptionFromSecret(normalizedURL, *secret)
507509
if err != nil {
508510
e := &serror.Event{
509511
Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err),
@@ -518,19 +520,19 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
518520
}
519521

520522
// Initialize the chart repository
521-
var chartRepo chart.Repository
523+
var chartRepo repository.Downloader
522524
switch repo.Spec.Type {
523525
case sourcev1.HelmRepositoryTypeOCI:
524-
if !helmreg.IsOCI(repo.Spec.URL) {
525-
err := fmt.Errorf("invalid OCI registry URL: %s", repo.Spec.URL)
526+
if !helmreg.IsOCI(normalizedURL) {
527+
err := fmt.Errorf("invalid OCI registry URL: %s", normalizedURL)
526528
return chartRepoConfigErrorReturn(err, obj)
527529
}
528530

529531
// with this function call, we create a temporary file to store the credentials if needed.
530532
// this is needed because otherwise the credentials are stored in ~/.docker/config.json.
531533
// TODO@souleb: remove this once the registry move to Oras v2
532534
// or rework to enable reusing credentials to avoid the unneccessary handshake operations
533-
registryClient, file, err := r.RegistryClientGenerator(loginOpts != nil)
535+
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpts != nil)
534536
if err != nil {
535537
e := &serror.Event{
536538
Err: fmt.Errorf("failed to construct Helm client: %w", err),
@@ -540,9 +542,9 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
540542
return sreconcile.ResultEmpty, e
541543
}
542544

543-
if file != "" {
545+
if credentialsFile != "" {
544546
defer func() {
545-
if err := os.Remove(file); err != nil {
547+
if err := os.Remove(credentialsFile); err != nil {
546548
r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
547549
"failed to delete temporary credentials file: %s", err)
548550
}
@@ -551,7 +553,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
551553

552554
// Tell the chart repository to use the OCI client with the configured getter
553555
clientOpts = append(clientOpts, helmgetter.WithRegistryClient(registryClient))
554-
ociChartRepo, err := repository.NewOCIChartRepository(repo.Spec.URL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(clientOpts), repository.WithOCIRegistryClient(registryClient))
556+
ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(clientOpts), repository.WithOCIRegistryClient(registryClient))
555557
if err != nil {
556558
return chartRepoConfigErrorReturn(err, obj)
557559
}
@@ -571,7 +573,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
571573
}
572574
}
573575
default:
574-
httpChartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts,
576+
httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts,
575577
repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
576578
r.IncCacheEvents(event, obj.Name, obj.Namespace)
577579
}))
@@ -684,9 +686,15 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
684686

685687
// Setup dependency manager
686688
dm := chart.NewDependencyManager(
687-
chart.WithRepositoryCallback(r.namespacedChartRepositoryCallback(ctx, obj.GetName(), obj.GetNamespace())),
689+
chart.WithDownloaderCallback(r.namespacedChartRepositoryCallback(ctx, obj.GetName(), obj.GetNamespace())),
688690
)
689-
defer dm.Clear()
691+
defer func() {
692+
err := dm.Clear()
693+
if err != nil {
694+
r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
695+
"dependency manager cleanup error: %s", err)
696+
}
697+
}()
690698

691699
// Configure builder options, including any previously cached chart
692700
opts := chart.BuildOptions{
@@ -913,12 +921,17 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1.
913921
return nil
914922
}
915923

916-
// namespacedChartRepositoryCallback returns a chart.GetChartRepositoryCallback scoped to the given namespace.
917-
// The returned callback returns a repository.ChartRepository configured with the retrieved v1beta1.HelmRepository,
924+
// namespacedChartRepositoryCallback returns a chart.GetChartDownloaderCallback scoped to the given namespace.
925+
// The returned callback returns a repository.Downloader configured with the retrieved v1beta1.HelmRepository,
918926
// or a shim with defaults if no object could be found.
919-
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartRepositoryCallback {
920-
return func(url string) (*repository.ChartRepository, error) {
921-
var tlsConfig *tls.Config
927+
// The callback returns an object with a state, so the caller has to do the necessary cleanup.
928+
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback {
929+
return func(url string) (repository.Downloader, error) {
930+
var (
931+
tlsConfig *tls.Config
932+
loginOpts []helmreg.LoginOption
933+
)
934+
normalizedURL := repository.NormalizeURL(url)
922935
repo, err := r.resolveDependencyRepository(ctx, url, namespace)
923936
if err != nil {
924937
// Return Kubernetes client errors, but ignore others
@@ -933,7 +946,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
933946
}
934947
}
935948
clientOpts := []helmgetter.Option{
936-
helmgetter.WithURL(repo.Spec.URL),
949+
helmgetter.WithURL(normalizedURL),
937950
helmgetter.WithTimeout(repo.Spec.Timeout.Duration),
938951
helmgetter.WithPassCredentialsAll(repo.Spec.PassCredentials),
939952
}
@@ -947,26 +960,77 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
947960
}
948961
clientOpts = append(clientOpts, opts...)
949962

950-
tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, repo.Spec.URL)
963+
tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, normalizedURL)
951964
if err != nil {
952965
return nil, fmt.Errorf("failed to create TLS client config for HelmRepository '%s': %w", repo.Name, err)
953966
}
954-
}
955967

956-
chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, tlsConfig, clientOpts)
957-
if err != nil {
958-
return nil, err
968+
// Build registryClient options from secret
969+
loginOpt, err := registry.LoginOptionFromSecret(normalizedURL, *secret)
970+
if err != nil {
971+
return nil, fmt.Errorf("failed to create login options for HelmRepository '%s': %w", repo.Name, err)
972+
}
973+
974+
loginOpts = append([]helmreg.LoginOption{}, loginOpt)
959975
}
960976

961-
// Ensure that the cache key is the same as the artifact path
962-
// otherwise don't enable caching. We don't want to cache indexes
963-
// for repositories that are not reconciled by the source controller.
964-
if repo.Status.Artifact != nil {
965-
chartRepo.CachePath = r.Storage.LocalPath(*repo.GetArtifact())
966-
chartRepo.SetMemCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
967-
r.IncCacheEvents(event, name, namespace)
968-
})
977+
var chartRepo repository.Downloader
978+
if helmreg.IsOCI(normalizedURL) {
979+
registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpts != nil)
980+
if err != nil {
981+
return nil, fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", repo.Name, err)
982+
}
983+
984+
var errs []error
985+
// Tell the chart repository to use the OCI client with the configured getter
986+
clientOpts = append(clientOpts, helmgetter.WithRegistryClient(registryClient))
987+
ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters),
988+
repository.WithOCIGetterOptions(clientOpts),
989+
repository.WithOCIRegistryClient(registryClient),
990+
repository.WithCredentialsFile(credentialsFile))
991+
if err != nil {
992+
errs = append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", repo.Name, err))
993+
// clean up the credentialsFile
994+
if credentialsFile != "" {
995+
if err := os.Remove(credentialsFile); err != nil {
996+
errs = append(errs, err)
997+
}
998+
}
999+
return nil, kerrors.NewAggregate(errs)
1000+
}
1001+
1002+
// If login options are configured, use them to login to the registry
1003+
// The OCIGetter will later retrieve the stored credentials to pull the chart
1004+
if loginOpts != nil {
1005+
err = ociChartRepo.Login(loginOpts...)
1006+
if err != nil {
1007+
errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", repo.Name, err))
1008+
// clean up the credentialsFile
1009+
errs = append(errs, ociChartRepo.Clear())
1010+
return nil, kerrors.NewAggregate(errs)
1011+
}
1012+
}
1013+
1014+
chartRepo = ociChartRepo
1015+
} else {
1016+
httpChartRepo, err := repository.NewChartRepository(normalizedURL, "", r.Getters, tlsConfig, clientOpts)
1017+
if err != nil {
1018+
return nil, err
1019+
}
1020+
1021+
// Ensure that the cache key is the same as the artifact path
1022+
// otherwise don't enable caching. We don't want to cache indexes
1023+
// for repositories that are not reconciled by the source controller.
1024+
if repo.Status.Artifact != nil {
1025+
httpChartRepo.CachePath = r.Storage.LocalPath(*repo.GetArtifact())
1026+
httpChartRepo.SetMemCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
1027+
r.IncCacheEvents(event, name, namespace)
1028+
})
1029+
}
1030+
1031+
chartRepo = httpChartRepo
9691032
}
1033+
9701034
return chartRepo, nil
9711035
}
9721036
}

‎controllers/helmchart_controller_test.go

+6-8
Original file line numberDiff line numberDiff line change
@@ -411,9 +411,6 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
411411
}))
412412
},
413413
},
414-
//{
415-
// name: "Error on transient build error",
416-
//},
417414
{
418415
name: "Stalling on persistent build error",
419416
source: &sourcev1.GitRepository{
@@ -1070,7 +1067,7 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
10701067
assertFunc: func(g *WithT, build chart.Build) {
10711068
g.Expect(build.Name).To(Equal("helmchartwithdeps"))
10721069
g.Expect(build.Version).To(Equal("0.1.0"))
1073-
g.Expect(build.ResolvedDependencies).To(Equal(3))
1070+
g.Expect(build.ResolvedDependencies).To(Equal(4))
10741071
g.Expect(build.Path).To(BeARegularFile())
10751072
},
10761073
cleanFunc: func(g *WithT, build *chart.Build) {
@@ -1178,10 +1175,11 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
11781175
g := NewWithT(t)
11791176

11801177
r := &HelmChartReconciler{
1181-
Client: fake.NewClientBuilder().Build(),
1182-
EventRecorder: record.NewFakeRecorder(32),
1183-
Storage: storage,
1184-
Getters: testGetters,
1178+
Client: fake.NewClientBuilder().Build(),
1179+
EventRecorder: record.NewFakeRecorder(32),
1180+
Storage: storage,
1181+
Getters: testGetters,
1182+
RegistryClientGenerator: registry.ClientGenerator,
11851183
}
11861184

11871185
obj := &sourcev1.HelmChart{

‎controllers/helmrepository_controller_oci.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta
326326
if loginOpts != nil {
327327
err = chartRepo.Login(loginOpts...)
328328
if err != nil {
329-
e := fmt.Errorf("failed to log into registry '%s': %w", obj.Spec.URL, err)
329+
e := fmt.Errorf("failed to login to registry '%s': %w", obj.Spec.URL, err)
330330
conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error())
331331
result, retErr = ctrl.Result{}, e
332332
return

‎controllers/testdata/charts/helmchartwithdeps/Chart.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,6 @@ dependencies:
3131
- name: grafana
3232
version: ">=5.7.0"
3333
repository: "https://grafana.github.io/helm-charts"
34+
- name: podinfo
35+
version: ">=6.1.*"
36+
repository: "oci://ghcr.io/stefanprodan/charts"

‎internal/helm/chart/builder_local_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func TestLocalBuilder_Build(t *testing.T) {
6767
reference Reference
6868
buildOpts BuildOptions
6969
valuesFiles []helmchart.File
70-
repositories map[string]*repository.ChartRepository
70+
repositories map[string]repository.Downloader
7171
dependentChartPaths []string
7272
wantValues chartutil.Values
7373
wantVersion string
@@ -146,7 +146,7 @@ fullnameOverride: "full-foo-name-override"`),
146146
{
147147
name: "chart with dependencies",
148148
reference: LocalReference{Path: "../testdata/charts/helmchartwithdeps"},
149-
repositories: map[string]*repository.ChartRepository{
149+
repositories: map[string]repository.Downloader{
150150
"https://grafana.github.io/helm-charts/": mockRepo(),
151151
},
152152
dependentChartPaths: []string{"./../testdata/charts/helmchart"},
@@ -165,7 +165,7 @@ fullnameOverride: "full-foo-name-override"`),
165165
{
166166
name: "v1 chart with dependencies",
167167
reference: LocalReference{Path: "../testdata/charts/helmchartwithdeps-v1"},
168-
repositories: map[string]*repository.ChartRepository{
168+
repositories: map[string]repository.Downloader{
169169
"https://grafana.github.io/helm-charts/": mockRepo(),
170170
},
171171
dependentChartPaths: []string{"../testdata/charts/helmchart-v1"},

‎internal/helm/chart/builder_remote.go

+5-13
Original file line numberDiff line numberDiff line change
@@ -34,24 +34,16 @@ import (
3434

3535
"github.com/fluxcd/source-controller/internal/fs"
3636
"github.com/fluxcd/source-controller/internal/helm/chart/secureloader"
37+
"github.com/fluxcd/source-controller/internal/helm/repository"
3738
)
3839

39-
// Repository is a repository.ChartRepository or a repository.OCIChartRepository.
40-
// It is used to download a chart from a remote Helm repository or OCI registry.
41-
type Repository interface {
42-
// GetChartVersion returns the repo.ChartVersion for the given name and version.
43-
GetChartVersion(name, version string) (*repo.ChartVersion, error)
44-
// GetChartVersion returns a chart.ChartVersion from the remote repository.
45-
DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer, error)
46-
}
47-
4840
type remoteChartBuilder struct {
49-
remote Repository
41+
remote repository.Downloader
5042
}
5143

5244
// NewRemoteBuilder returns a Builder capable of building a Helm
53-
// chart with a RemoteReference in the given repository.ChartRepository.
54-
func NewRemoteBuilder(repository Repository) Builder {
45+
// chart with a RemoteReference in the given repository.Downloader.
46+
func NewRemoteBuilder(repository repository.Downloader) Builder {
5547
return &remoteChartBuilder{
5648
remote: repository,
5749
}
@@ -132,7 +124,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
132124
return result, nil
133125
}
134126

135-
func (b *remoteChartBuilder) downloadFromRepository(remote Repository, remoteRef RemoteReference, opts BuildOptions) (*bytes.Buffer, *Build, error) {
127+
func (b *remoteChartBuilder) downloadFromRepository(remote repository.Downloader, remoteRef RemoteReference, opts BuildOptions) (*bytes.Buffer, *Build, error) {
136128
// Get the current version for the RemoteReference
137129
cv, err := remote.GetChartVersion(remoteRef.Name, remoteRef.Version)
138130
if err != nil {

0 commit comments

Comments
 (0)