Skip to content

Commit c5bf164

Browse files
hiddecohiddeco
authored
Merge pull request #157 from fluxcd/helm-index-chart-download
Factor out Helm repo index and chart download
2 parents 8ffffbb + 8bf7d8f commit c5bf164

File tree

9 files changed

+777
-119
lines changed

9 files changed

+777
-119
lines changed

‎controllers/helmchart_controller.go‎

Lines changed: 44 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -231,52 +231,6 @@ func (r *HelmChartReconciler) getSource(ctx context.Context, chart sourcev1.Helm
231231

232232
func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
233233
repository sourcev1.HelmRepository, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) {
234-
cv, err := helm.GetDownloadableChartVersionFromIndex(r.Storage.LocalPath(*repository.GetArtifact()),
235-
chart.Spec.Chart, chart.Spec.Version)
236-
if err != nil {
237-
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
238-
}
239-
240-
// Return early if the revision is still the same as the current artifact
241-
newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), cv.Version,
242-
fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version))
243-
if !force && sourcev1.InReadyCondition(chart.Status.Conditions) && chart.GetArtifact().HasRevision(newArtifact.Revision) {
244-
if newArtifact.URL != repository.GetArtifact().URL {
245-
r.Storage.SetArtifactURL(chart.GetArtifact())
246-
chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
247-
}
248-
return chart, nil
249-
}
250-
251-
// TODO(hidde): according to the Helm source the first item is not
252-
// always the correct one to pick, check for updates once in awhile.
253-
// Ref: https://github.com/helm/helm/blob/v3.3.0/pkg/downloader/chart_downloader.go#L241
254-
ref := cv.URLs[0]
255-
u, err := url.Parse(ref)
256-
if err != nil {
257-
err = fmt.Errorf("invalid chart URL format '%s': %w", ref, err)
258-
}
259-
260-
// Prepend the chart repository base URL if the URL is relative
261-
if !u.IsAbs() {
262-
repoURL, err := url.Parse(repository.Spec.URL)
263-
if err != nil {
264-
err = fmt.Errorf("invalid repository URL format '%s': %w", repository.Spec.URL, err)
265-
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
266-
}
267-
q := repoURL.Query()
268-
// Trailing slash is required for ResolveReference to work
269-
repoURL.Path = strings.TrimSuffix(repoURL.Path, "/") + "/"
270-
u = repoURL.ResolveReference(u)
271-
u.RawQuery = q.Encode()
272-
}
273-
274-
// Get the getter for the protocol
275-
c, err := r.Getters.ByScheme(u.Scheme)
276-
if err != nil {
277-
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
278-
}
279-
280234
var clientOpts []getter.Option
281235
if repository.Spec.SecretRef != nil {
282236
name := types.NamespacedName{
@@ -299,6 +253,46 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
299253
defer cleanup()
300254
clientOpts = opts
301255
}
256+
clientOpts = append(clientOpts, getter.WithTimeout(repository.GetTimeout()))
257+
258+
// Initialize the chart repository and load the index file
259+
chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Getters, clientOpts)
260+
if err != nil {
261+
switch err.(type) {
262+
case *url.Error:
263+
return sourcev1.HelmChartNotReady(chart, sourcev1.URLInvalidReason, err.Error()), err
264+
default:
265+
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
266+
}
267+
}
268+
indexFile, err := os.Open(r.Storage.LocalPath(*repository.GetArtifact()))
269+
if err != nil {
270+
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
271+
}
272+
b, err := ioutil.ReadAll(indexFile)
273+
if err != nil {
274+
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
275+
}
276+
if err = chartRepo.LoadIndex(b); err != nil {
277+
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
278+
}
279+
280+
// Lookup the chart version in the chart repository index
281+
chartVer, err := chartRepo.Get(chart.Spec.Chart, chart.Spec.Version)
282+
if err != nil {
283+
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
284+
}
285+
286+
// Return early if the revision is still the same as the current artifact
287+
newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), chartVer.Version,
288+
fmt.Sprintf("%s-%s.tgz", chartVer.Name, chartVer.Version))
289+
if !force && repository.GetArtifact().HasRevision(newArtifact.Revision) {
290+
if newArtifact.URL != chart.GetArtifact().URL {
291+
r.Storage.SetArtifactURL(chart.GetArtifact())
292+
chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
293+
}
294+
return chart, nil
295+
}
302296

303297
// Ensure artifact directory exists
304298
err = r.Storage.MkdirAll(newArtifact)
@@ -315,9 +309,8 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
315309
}
316310
defer unlock()
317311

318-
// TODO(hidde): implement timeout from the HelmRepository
319-
// https://github.com/helm/helm/pull/7950
320-
res, err := c.Get(u.String(), clientOpts...)
312+
// Attempt to download the chart
313+
res, err := chartRepo.DownloadChart(chartVer)
321314
if err != nil {
322315
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
323316
}
@@ -345,7 +338,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
345338
}
346339

347340
// Overwrite values file
348-
chartPath := path.Join(tmpDir, cv.Name)
341+
chartPath := path.Join(tmpDir, chartVer.Name)
349342
if err := helm.OverwriteChartDefaultValues(chartPath, chart.Spec.ValuesFile); err != nil {
350343
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
351344
}
@@ -376,7 +369,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
376369
}
377370

378371
// Update symlink
379-
chartUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", cv.Name))
372+
chartUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", chartVer.Name))
380373
if err != nil {
381374
err = fmt.Errorf("storage error: %w", err)
382375
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err

‎controllers/helmrepository_controller.go‎

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,12 @@ import (
2020
"bytes"
2121
"context"
2222
"fmt"
23-
"io/ioutil"
2423
"net/url"
25-
"path"
2624
"strings"
2725
"time"
2826

2927
"github.com/go-logr/logr"
3028
"helm.sh/helm/v3/pkg/getter"
31-
"helm.sh/helm/v3/pkg/repo"
3229
corev1 "k8s.io/api/core/v1"
3330
"k8s.io/apimachinery/pkg/runtime"
3431
"k8s.io/apimachinery/pkg/types"
@@ -163,19 +160,6 @@ func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager,
163160
}
164161

165162
func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sourcev1.HelmRepository) (sourcev1.HelmRepository, error) {
166-
u, err := url.Parse(repository.Spec.URL)
167-
if err != nil {
168-
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.URLInvalidReason, err.Error()), err
169-
}
170-
171-
c, err := r.Getters.ByScheme(u.Scheme)
172-
if err != nil {
173-
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.URLInvalidReason, err.Error()), err
174-
}
175-
176-
u.RawPath = path.Join(u.RawPath, "index.yaml")
177-
u.Path = path.Join(u.Path, "index.yaml")
178-
179163
var clientOpts []getter.Option
180164
if repository.Spec.SecretRef != nil {
181165
name := types.NamespacedName{
@@ -198,25 +182,27 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
198182
defer cleanup()
199183
clientOpts = opts
200184
}
201-
202185
clientOpts = append(clientOpts, getter.WithTimeout(repository.GetTimeout()))
203-
res, err := c.Get(u.String(), clientOpts...)
204-
if err != nil {
205-
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
206-
}
207186

208-
b, err := ioutil.ReadAll(res)
187+
chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Getters, clientOpts)
209188
if err != nil {
210-
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
189+
switch err.(type) {
190+
case *url.Error:
191+
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.URLInvalidReason, err.Error()), err
192+
default:
193+
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
194+
}
211195
}
212-
i := repo.IndexFile{}
213-
if err := yaml.Unmarshal(b, &i); err != nil {
196+
if err := chartRepo.DownloadIndex(); err != nil {
197+
err = fmt.Errorf("failed to download repository index: %w", err)
214198
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
215199
}
216200

217201
// return early on unchanged generation
218-
artifact := r.Storage.NewArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), i.Generated.Format(time.RFC3339Nano),
219-
fmt.Sprintf("index-%s.yaml", url.PathEscape(i.Generated.Format(time.RFC3339Nano))))
202+
artifact := r.Storage.NewArtifactFor(repository.Kind,
203+
repository.ObjectMeta.GetObjectMeta(),
204+
chartRepo.Index.Generated.Format(time.RFC3339Nano),
205+
fmt.Sprintf("index-%s.yaml", url.PathEscape(chartRepo.Index.Generated.Format(time.RFC3339Nano))))
220206
if sourcev1.InReadyCondition(repository.Status.Conditions) && repository.GetArtifact().HasRevision(artifact.Revision) {
221207
if artifact.URL != repository.GetArtifact().URL {
222208
r.Storage.SetArtifactURL(repository.GetArtifact())
@@ -225,12 +211,6 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
225211
return repository, nil
226212
}
227213

228-
i.SortEntries()
229-
b, err = yaml.Marshal(&i)
230-
if err != nil {
231-
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
232-
}
233-
234214
// create artifact dir
235215
err = r.Storage.MkdirAll(artifact)
236216
if err != nil {
@@ -247,6 +227,10 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
247227
defer unlock()
248228

249229
// save artifact to storage
230+
b, err := yaml.Marshal(&chartRepo.Index)
231+
if err != nil {
232+
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
233+
}
250234
if err := r.Storage.AtomicWriteFile(&artifact, bytes.NewReader(b), 0644); err != nil {
251235
err = fmt.Errorf("unable to write repository index file: %w", err)
252236
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err

‎controllers/helmrepository_controller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ var _ = Describe("HelmRepositoryReconciler", func() {
121121
Eventually(func() bool {
122122
_ = k8sClient.Get(context.Background(), key, updated)
123123
for _, c := range updated.Status.Conditions {
124-
if c.Reason == sourcev1.URLInvalidReason {
124+
if c.Reason == sourcev1.IndexationFailedReason {
125125
return true
126126
}
127127
}

‎internal/helm/getter.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import (
2626
corev1 "k8s.io/api/core/v1"
2727
)
2828

29+
// ClientOptionsFromSecret constructs a getter.Option slice for the given secret.
30+
// It returns the slice, and a callback to remove temporary files.
2931
func ClientOptionsFromSecret(secret corev1.Secret) ([]getter.Option, func(), error) {
3032
var opts []getter.Option
3133
basicAuth, err := BasicAuthFromSecret(secret)
@@ -45,6 +47,11 @@ func ClientOptionsFromSecret(secret corev1.Secret) ([]getter.Option, func(), err
4547
return opts, cleanup, nil
4648
}
4749

50+
// BasicAuthFromSecret attempts to construct a basic auth getter.Option for the
51+
// given v1.Secret and returns the result.
52+
//
53+
// Secrets with no username AND password are ignored, if only one is defined it
54+
// returns an error.
4855
func BasicAuthFromSecret(secret corev1.Secret) (getter.Option, error) {
4956
username, password := string(secret.Data["username"]), string(secret.Data["password"])
5057
switch {
@@ -56,6 +63,12 @@ func BasicAuthFromSecret(secret corev1.Secret) (getter.Option, error) {
5663
return getter.WithBasicAuth(username, password), nil
5764
}
5865

66+
// TLSClientConfigFromSecret attempts to construct a TLS client config
67+
// getter.Option for the given v1.Secret. It returns the getter.Option and a
68+
// callback to remove the temporary TLS files.
69+
//
70+
// Secrets with no certFile, keyFile, AND caFile are ignored, if only a
71+
// certBytes OR keyBytes is defined it returns an error.
5972
func TLSClientConfigFromSecret(secret corev1.Secret) (getter.Option, func(), error) {
6073
certBytes, keyBytes, caBytes := secret.Data["certFile"], secret.Data["keyFile"], secret.Data["caFile"]
6174
switch {

0 commit comments

Comments
 (0)