Skip to content

Commit c26ecbe

Browse files
committed
Enable multiple remote dependency
Signed-off-by: Soule BA <[email protected]>
1 parent f29876b commit c26ecbe

File tree

10 files changed

+534
-128
lines changed

10 files changed

+534
-128
lines changed

controllers/helmchart_controller.go

Lines changed: 72 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import (
5454
"github.com/fluxcd/pkg/runtime/patch"
5555
"github.com/fluxcd/pkg/runtime/predicates"
5656
"github.com/fluxcd/pkg/untar"
57+
"github.com/hashicorp/go-multierror"
5758

5859
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
5960
"github.com/fluxcd/source-controller/internal/cache"
@@ -510,7 +511,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
510511
}
511512

512513
// Initialize the chart repository
513-
var chartRepo chart.Repository
514+
var chartRepo chart.Downloader
514515
switch repo.Spec.Type {
515516
case sourcev1.HelmRepositoryTypeOCI:
516517
if !helmreg.IsOCI(repo.Spec.URL) {
@@ -676,7 +677,7 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
676677

677678
// Setup dependency manager
678679
dm := chart.NewDependencyManager(
679-
chart.WithRepositoryCallback(r.namespacedChartRepositoryCallback(ctx, obj.GetName(), obj.GetNamespace())),
680+
chart.WithDownloaderCallback(r.namespacedChartRepositoryCallback(ctx, obj.GetName(), obj.GetNamespace())),
680681
)
681682
defer dm.Clear()
682683

@@ -905,17 +906,21 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1.
905906
return nil
906907
}
907908

908-
// namespacedChartRepositoryCallback returns a chart.GetChartRepositoryCallback scoped to the given namespace.
909+
// namespacedChartRepositoryCallback returns a chart.GetChartDownloaderCallback scoped to the given namespace.
909910
// The returned callback returns a repository.ChartRepository configured with the retrieved v1beta1.HelmRepository,
910911
// or a shim with defaults if no object could be found.
911-
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartRepositoryCallback {
912-
return func(url string) (*repository.ChartRepository, error) {
913-
var tlsConfig *tls.Config
912+
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback {
913+
return func(url string) (chart.CleanDownloader, string, error) {
914+
var (
915+
tlsConfig *tls.Config
916+
loginOpts []helmreg.LoginOption
917+
credentialFile string
918+
)
914919
repo, err := r.resolveDependencyRepository(ctx, url, namespace)
915920
if err != nil {
916921
// Return Kubernetes client errors, but ignore others
917922
if apierrs.ReasonForError(err) != metav1.StatusReasonUnknown {
918-
return nil, err
923+
return nil, "", err
919924
}
920925
repo = &sourcev1.HelmRepository{
921926
Spec: sourcev1.HelmRepositorySpec{
@@ -931,35 +936,79 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
931936
}
932937
if secret, err := r.getHelmRepositorySecret(ctx, repo); secret != nil || err != nil {
933938
if err != nil {
934-
return nil, err
939+
return nil, "", err
935940
}
936941
opts, err := getter.ClientOptionsFromSecret(*secret)
937942
if err != nil {
938-
return nil, err
943+
return nil, "", err
939944
}
940945
clientOpts = append(clientOpts, opts...)
941946

942947
tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, repo.Spec.URL)
943948
if err != nil {
944-
return nil, fmt.Errorf("failed to create TLS client config for HelmRepository '%s': %w", repo.Name, err)
949+
return nil, "", fmt.Errorf("failed to create TLS client config for HelmRepository '%s': %w", repo.Name, err)
945950
}
946-
}
947951

948-
chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, tlsConfig, clientOpts)
949-
if err != nil {
950-
return nil, err
952+
// Build registryClient options from secret
953+
loginOpt, err := registry.LoginOptionFromSecret(repo.Spec.URL, *secret)
954+
if err != nil {
955+
return nil, "", fmt.Errorf("failed to create login options for HelmRepository '%s': %w", repo.Name, err)
956+
}
957+
958+
loginOpts = append([]helmreg.LoginOption{}, loginOpt)
951959
}
952960

953-
// Ensure that the cache key is the same as the artifact path
954-
// otherwise don't enable caching. We don't want to cache indexes
955-
// for repositories that are not reconciled by the source controller.
956-
if repo.Status.Artifact != nil {
957-
chartRepo.CachePath = r.Storage.LocalPath(*repo.GetArtifact())
958-
chartRepo.SetMemCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
959-
r.IncCacheEvents(event, name, namespace)
960-
})
961+
var chartRepo chart.CleanDownloader
962+
if helmreg.IsOCI(repo.Spec.URL) {
963+
registryClient, file, err := r.RegistryClientGenerator(loginOpts != nil)
964+
if err != nil {
965+
return nil, "", fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", repo.Name, err)
966+
}
967+
968+
var errs error
969+
// Tell the chart repository to use the OCI client with the configured getter
970+
clientOpts = append(clientOpts, helmgetter.WithRegistryClient(registryClient))
971+
ociChartRepo, err := repository.NewOCIChartRepository(repo.Spec.URL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(clientOpts), repository.WithOCIRegistryClient(registryClient))
972+
if err != nil {
973+
// clean up the file
974+
if err := os.Remove(file); err != nil {
975+
errs = multierror.Append(errs, err)
976+
}
977+
errs = multierror.Append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", repo.Name, err))
978+
return nil, "", errs
979+
}
980+
981+
// If login options are configured, use them to login to the registry
982+
// The OCIGetter will later retrieve the stored credentials to pull the chart
983+
if loginOpts != nil {
984+
err = ociChartRepo.Login(loginOpts...)
985+
if err != nil {
986+
return nil, "", fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", repo.Name, err)
987+
}
988+
}
989+
990+
credentialFile = file
991+
chartRepo = ociChartRepo
992+
} else {
993+
httpChartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, tlsConfig, clientOpts)
994+
if err != nil {
995+
return nil, "", err
996+
}
997+
998+
// Ensure that the cache key is the same as the artifact path
999+
// otherwise don't enable caching. We don't want to cache indexes
1000+
// for repositories that are not reconciled by the source controller.
1001+
if repo.Status.Artifact != nil {
1002+
httpChartRepo.CachePath = r.Storage.LocalPath(*repo.GetArtifact())
1003+
httpChartRepo.SetMemCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
1004+
r.IncCacheEvents(event, name, namespace)
1005+
})
1006+
}
1007+
1008+
chartRepo = httpChartRepo
9611009
}
962-
return chartRepo, nil
1010+
1011+
return chartRepo, credentialFile, nil
9631012
}
9641013
}
9651014

go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@ require (
132132
github.com/gorilla/mux v1.8.0 // indirect
133133
github.com/gosuri/uitable v0.0.4 // indirect
134134
github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
135+
github.com/hashicorp/errwrap v1.1.0 // indirect
135136
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
137+
github.com/hashicorp/go-multierror v1.1.1 // indirect
136138
github.com/hashicorp/go-retryablehttp v0.7.1 // indirect
137139
github.com/huandu/xstrings v1.3.2 // indirect
138140
github.com/imdario/mergo v0.3.12 // indirect

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,8 @@ github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFb
489489
github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q=
490490
github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
491491
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
492+
github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
493+
github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
492494
github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
493495
github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
494496
github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
@@ -497,6 +499,8 @@ github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrj
497499
github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60=
498500
github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM=
499501
github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
502+
github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
503+
github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
500504
github.com/hashicorp/go-retryablehttp v0.7.1 h1:sUiuQAnLlbvmExtFQs72iFW/HXeUn8Z1aJLQ4LJJbTQ=
501505
github.com/hashicorp/go-retryablehttp v0.7.1/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY=
502506
github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU=

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]CleanDownloader
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]CleanDownloader{
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]CleanDownloader{
169169
"https://grafana.github.io/helm-charts/": mockRepo(),
170170
},
171171
dependentChartPaths: []string{"../testdata/charts/helmchart-v1"},

internal/helm/chart/builder_remote.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,22 @@ import (
3636
"github.com/fluxcd/source-controller/internal/helm/chart/secureloader"
3737
)
3838

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.
39+
// Downloader is used to download a chart from a remote Helm repository or OCI registry.
40+
type Downloader interface {
41+
// GetChartVersion returns the repo.ChartVersion for the given name and version
42+
// from the remote repository.ChartRepository.
4343
GetChartVersion(name, version string) (*repo.ChartVersion, error)
44-
// GetChartVersion returns a chart.ChartVersion from the remote repository.
44+
// DownloadChart downloads a chart from the remote Helm repository or OCI registry.
4545
DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer, error)
4646
}
4747

4848
type remoteChartBuilder struct {
49-
remote Repository
49+
remote Downloader
5050
}
5151

5252
// 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 {
53+
// chart with a RemoteReference in the given Downloader.
54+
func NewRemoteBuilder(repository Downloader) Builder {
5555
return &remoteChartBuilder{
5656
remote: repository,
5757
}
@@ -132,7 +132,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
132132
return result, nil
133133
}
134134

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

internal/helm/chart/dependency_manager.go

Lines changed: 52 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -35,45 +35,57 @@ import (
3535
"github.com/fluxcd/source-controller/internal/helm/repository"
3636
)
3737

38-
// GetChartRepositoryCallback must return a repository.ChartRepository for the
39-
// URL, or an error describing why it could not be returned.
40-
type GetChartRepositoryCallback func(url string) (*repository.ChartRepository, error)
38+
// CleanDownloader is a Downloader that cleans temporary files created by the downloader,
39+
// caching the files if the cache is configured,
40+
// and calling gc to remove unused files.
41+
type CleanDownloader interface {
42+
Clean() []error
43+
Downloader
44+
}
45+
46+
// GetChartDownloaderCallback must return a Downloader for the
47+
// URL and an optional credential file name,
48+
// or an error describing why it could not be returned.
49+
type GetChartDownloaderCallback func(url string) (CleanDownloader, string, error)
4150

4251
// DependencyManager manages dependencies for a Helm chart.
4352
type DependencyManager struct {
44-
// repositories contains a map of repository.ChartRepository objects
53+
// downloaders contains a map of Downloader objects
4554
// indexed by their repository.NormalizeURL.
4655
// It is consulted as a lookup table for missing dependencies, based on
4756
// the (repository) URL the dependency refers to.
48-
repositories map[string]*repository.ChartRepository
57+
downloaders map[string]CleanDownloader
4958

50-
// getRepositoryCallback can be set to an on-demand GetChartRepositoryCallback
51-
// whose returned result is cached to repositories.
52-
getRepositoryCallback GetChartRepositoryCallback
59+
// getChartDownloaderCallback can be set to an on-demand GetChartDownloaderCallback
60+
// whose returned result is cached to downloaders.
61+
getChartDownloaderCallback GetChartDownloaderCallback
5362

5463
// concurrent is the number of concurrent chart-add operations during
5564
// Build. Defaults to 1 (non-concurrent).
5665
concurrent int64
5766

5867
// mu contains the lock for chart writes.
5968
mu sync.Mutex
69+
70+
// credentialFiles is a list of temporary credential files created by the downloaders.
71+
credentialFiles map[string]string
6072
}
6173

6274
// DependencyManagerOption configures an option on a DependencyManager.
6375
type DependencyManagerOption interface {
6476
applyToDependencyManager(dm *DependencyManager)
6577
}
6678

67-
type WithRepositories map[string]*repository.ChartRepository
79+
type WithRepositories map[string]CleanDownloader
6880

6981
func (o WithRepositories) applyToDependencyManager(dm *DependencyManager) {
70-
dm.repositories = o
82+
dm.downloaders = o
7183
}
7284

73-
type WithRepositoryCallback GetChartRepositoryCallback
85+
type WithDownloaderCallback GetChartDownloaderCallback
7486

75-
func (o WithRepositoryCallback) applyToDependencyManager(dm *DependencyManager) {
76-
dm.getRepositoryCallback = GetChartRepositoryCallback(o)
87+
func (o WithDownloaderCallback) applyToDependencyManager(dm *DependencyManager) {
88+
dm.getChartDownloaderCallback = GetChartDownloaderCallback(o)
7789
}
7890

7991
type WithConcurrent int64
@@ -92,18 +104,21 @@ func NewDependencyManager(opts ...DependencyManagerOption) *DependencyManager {
92104
return dm
93105
}
94106

95-
// Clear iterates over the repositories, calling Unload and RemoveCache on all
96-
// items. It returns a collection of (cache removal) errors.
107+
// Clear iterates over the downloaders, calling Clean on all
108+
// items.
109+
// It removes all temporary files created by the downloaders.
110+
// It returns a collection of errors.
97111
func (dm *DependencyManager) Clear() []error {
98112
var errs []error
99-
for _, v := range dm.repositories {
100-
if err := v.CacheIndexInMemory(); err != nil {
101-
errs = append(errs, err)
102-
}
103-
v.Unload()
104-
if err := v.RemoveCache(); err != nil {
105-
errs = append(errs, err)
113+
for k, v := range dm.downloaders {
114+
// clean the downloader temp credential file
115+
if file, ok := dm.credentialFiles[k]; ok && file != "" {
116+
if err := os.Remove(file); err != nil {
117+
errs = append(errs, err)
118+
}
106119
}
120+
121+
errs = append(errs, v.Clean()...)
107122
}
108123
return errs
109124
}
@@ -236,10 +251,6 @@ func (dm *DependencyManager) addRemoteDependency(chart *chartWithLock, dep *helm
236251
return err
237252
}
238253

239-
if err = repo.StrategicallyLoadIndex(); err != nil {
240-
return fmt.Errorf("failed to load index for '%s': %w", dep.Name, err)
241-
}
242-
243254
ver, err := repo.GetChartVersion(dep.Name, dep.Version)
244255
if err != nil {
245256
return err
@@ -259,27 +270,33 @@ func (dm *DependencyManager) addRemoteDependency(chart *chartWithLock, dep *helm
259270
return nil
260271
}
261272

262-
// resolveRepository first attempts to resolve the url from the repositories, falling back
263-
// to getRepositoryCallback if set. It returns the resolved Index, or an error.
264-
func (dm *DependencyManager) resolveRepository(url string) (_ *repository.ChartRepository, err error) {
273+
// resolveRepository first attempts to resolve the url from the downloaders, falling back
274+
// to getDownloaderCallback if set. It returns the resolved Index, or an error.
275+
func (dm *DependencyManager) resolveRepository(url string) (_ Downloader, err error) {
265276
dm.mu.Lock()
266277
defer dm.mu.Unlock()
267278

268279
nUrl := repository.NormalizeURL(url)
269-
if _, ok := dm.repositories[nUrl]; !ok {
270-
if dm.getRepositoryCallback == nil {
280+
if _, ok := dm.downloaders[nUrl]; !ok {
281+
if dm.getChartDownloaderCallback == nil {
271282
err = fmt.Errorf("no chart repository for URL '%s'", nUrl)
272283
return
273284
}
274-
if dm.repositories == nil {
275-
dm.repositories = map[string]*repository.ChartRepository{}
285+
286+
if dm.downloaders == nil {
287+
dm.downloaders = map[string]CleanDownloader{}
288+
}
289+
290+
if dm.credentialFiles == nil {
291+
dm.credentialFiles = map[string]string{}
276292
}
277-
if dm.repositories[nUrl], err = dm.getRepositoryCallback(nUrl); err != nil {
293+
294+
if dm.downloaders[nUrl], dm.credentialFiles[nUrl], err = dm.getChartDownloaderCallback(nUrl); err != nil {
278295
err = fmt.Errorf("failed to get chart repository for URL '%s': %w", nUrl, err)
279296
return
280297
}
281298
}
282-
return dm.repositories[nUrl], nil
299+
return dm.downloaders[nUrl], nil
283300
}
284301

285302
// secureLocalChartPath returns the secure absolute path of a local dependency.

0 commit comments

Comments
 (0)