Skip to content

Commit 224092e

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 <[email protected]>
1 parent e3ca5d1 commit 224092e

14 files changed

+550
-167
lines changed

‎controllers/helmchart_controller.go

Lines changed: 96 additions & 32 deletions
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"
@@ -460,9 +461,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
460461
loginOpts []helmreg.LoginOption
461462
)
462463

464+
normalizedURL := repository.NormalizeURL(repo.Spec.URL)
463465
// Construct the Getter options from the HelmRepository data
464466
clientOpts := []helmgetter.Option{
465-
helmgetter.WithURL(repo.Spec.URL),
467+
helmgetter.WithURL(normalizedURL),
466468
helmgetter.WithTimeout(repo.Spec.Timeout.Duration),
467469
helmgetter.WithPassCredentialsAll(repo.Spec.PassCredentials),
468470
}
@@ -490,7 +492,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
490492
}
491493
clientOpts = append(clientOpts, opts...)
492494

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

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

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

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

542-
if file != "" {
544+
if credentialsFile != "" {
543545
defer func() {
544-
if err := os.Remove(file); err != nil {
546+
if err := os.Remove(credentialsFile); err != nil {
545547
r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
546548
"failed to delete temporary credentials file: %s", err)
547549
}
@@ -550,7 +552,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
550552

551553
// Tell the chart repository to use the OCI client with the configured getter
552554
clientOpts = append(clientOpts, helmgetter.WithRegistryClient(registryClient))
553-
ociChartRepo, err := repository.NewOCIChartRepository(repo.Spec.URL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(clientOpts), repository.WithOCIRegistryClient(registryClient))
555+
ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(clientOpts), repository.WithOCIRegistryClient(registryClient))
554556
if err != nil {
555557
return chartRepoConfigErrorReturn(err, obj)
556558
}
@@ -570,7 +572,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
570572
}
571573
}
572574
default:
573-
httpChartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts,
575+
httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts,
574576
repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
575577
r.IncCacheEvents(event, obj.Name, obj.Namespace)
576578
}))
@@ -683,9 +685,15 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
683685

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

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

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

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

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

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

‎controllers/helmchart_controller_test.go

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

Lines changed: 1 addition & 1 deletion
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

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

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

Lines changed: 5 additions & 13 deletions
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)