Skip to content

Commit 7268c8b

Browse files
committed
Refactor and factor out chart values replacement
1 parent fd36d2d commit 7268c8b

File tree

2 files changed

+75
-58
lines changed

2 files changed

+75
-58
lines changed

controllers/helmchart_controller.go

Lines changed: 23 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package controllers
1919
import (
2020
"context"
2121
"fmt"
22-
"io"
2322
"io/ioutil"
2423
"net/url"
2524
"os"
@@ -106,7 +105,8 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
106105
}
107106

108107
// Conditionally set progressing condition in status
109-
if resetChart, ok := r.resetStatus(chart); ok {
108+
resetChart, changed := r.resetStatus(chart)
109+
if changed {
110110
chart = resetChart
111111
if err := r.Status().Update(ctx, &chart); err != nil {
112112
log.Error(err, "unable to update status")
@@ -132,7 +132,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
132132
}
133133
return ctrl.Result{Requeue: true}, err
134134
}
135-
reconciledChart, reconcileErr = r.reconcileFromHelmRepository(ctx, repository, *chart.DeepCopy())
135+
reconciledChart, reconcileErr = r.reconcileFromHelmRepository(ctx, repository, *chart.DeepCopy(), changed)
136136
case sourcev1.GitRepositoryKind:
137137
repository, err := r.getGitRepositoryWithArtifact(ctx, chart)
138138
if err != nil {
@@ -142,7 +142,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
142142
}
143143
return ctrl.Result{Requeue: true}, err
144144
}
145-
reconciledChart, reconcileErr = r.reconcileFromGitRepository(ctx, repository, *chart.DeepCopy())
145+
reconciledChart, reconcileErr = r.reconcileFromGitRepository(ctx, repository, *chart.DeepCopy(), changed)
146146
default:
147147
err := fmt.Errorf("unable to reconcile unsupported source reference kind '%s'", chart.Spec.SourceRef.Kind)
148148
return ctrl.Result{}, err
@@ -189,7 +189,7 @@ func (r *HelmChartReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts
189189
}
190190

191191
func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
192-
repository sourcev1.HelmRepository, chart sourcev1.HelmChart) (sourcev1.HelmChart, error) {
192+
repository sourcev1.HelmRepository, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) {
193193
cv, err := helm.GetDownloadableChartVersionFromIndex(r.Storage.LocalPath(*repository.GetArtifact()),
194194
chart.Spec.Chart, chart.Spec.Version)
195195
if err != nil {
@@ -199,7 +199,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
199199
// Return early if the revision is still the same as the current artifact
200200
artifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), cv.Version,
201201
fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version))
202-
if repository.GetArtifact() != nil && repository.GetArtifact().Revision == cv.Version {
202+
if !force && repository.GetArtifact() != nil && repository.GetArtifact().Revision == cv.Version {
203203
if artifact.URL != repository.GetArtifact().URL {
204204
r.Storage.SetArtifactURL(repository.GetArtifact())
205205
repository.Status.URL = r.Storage.SetHostname(repository.Status.URL)
@@ -283,6 +283,12 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
283283
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
284284
}
285285

286+
// Either repackage the chart with the declared default values file,
287+
// or write the chart directly to storage.
288+
var (
289+
readyReason = sourcev1.ChartPullSucceededReason
290+
readyMessage = fmt.Sprintf("Fetched revision: %s", artifact.Revision)
291+
)
286292
switch {
287293
case chart.Spec.ValuesFile != "" && chart.Spec.ValuesFile != chartutil.ValuesfileName:
288294
// Create temporary working directory
@@ -301,33 +307,11 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
301307

302308
// Overwrite values file
303309
chartPath := path.Join(tmpDir, cv.Name)
304-
srcPath := path.Join(chartPath, chart.Spec.ValuesFile)
305-
if f, err := os.Stat(srcPath); os.IsNotExist(err) || !f.Mode().IsRegular() {
306-
err = fmt.Errorf("invalid values file: %s", chart.Spec.ValuesFile)
310+
if err := helm.OverwriteChartDefaultValues(chartPath, chart.Spec.ValuesFile); err != nil {
307311
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
308312
}
309-
src, err := os.Open(srcPath)
310-
if err != nil {
311-
err = fmt.Errorf("failed to open values file '%s': %w", chart.Spec.ValuesFile, err)
312-
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
313-
}
314-
t, err := os.OpenFile(path.Join(chartPath, chartutil.ValuesfileName), os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
315-
if err != nil {
316-
src.Close()
317-
err = fmt.Errorf("failed to open default values: %w", err)
318-
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
319-
}
320-
if _, err := io.Copy(t, src); err != nil {
321-
t.Close()
322-
src.Close()
323-
err = fmt.Errorf("failed to overwrite default values with '%s: %w", chart.Spec.ValuesFile, err)
324-
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
325-
}
326-
t.Close()
327-
src.Close()
328313

329-
// Package the chart, we use the action here instead of relying on the
330-
// chartutil.Save method as the action performs a dependency check for us
314+
// Package the chart with the new default values
331315
pkg := action.NewPackage()
332316
pkg.Destination = tmpDir
333317
pkgPath, err := pkg.Run(chartPath, nil)
@@ -348,6 +332,9 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
348332
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
349333
}
350334
cf.Close()
335+
336+
readyMessage = fmt.Sprintf("Fetched and packaged revision: %s", artifact.Revision)
337+
readyReason = sourcev1.ChartPackageSucceededReason
351338
default:
352339
// Write artifact to storage
353340
if err := r.Storage.AtomicWriteFile(&artifact, res, 0644); err != nil {
@@ -363,8 +350,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
363350
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
364351
}
365352

366-
message := fmt.Sprintf("Fetched revision: %s", artifact.Revision)
367-
return sourcev1.HelmChartReady(chart, artifact, chartUrl, sourcev1.ChartPullSucceededReason, message), nil
353+
return sourcev1.HelmChartReady(chart, artifact, chartUrl, readyReason, readyMessage), nil
368354
}
369355

370356
// getChartRepositoryWithArtifact attempts to get the v1alpha1.HelmRepository
@@ -388,14 +374,14 @@ func (r *HelmChartReconciler) getChartRepositoryWithArtifact(ctx context.Context
388374
}
389375

390376
if repository.GetArtifact() == nil {
391-
err = fmt.Errorf("no repository index artifact found in HelmRepository '%s'", name)
377+
err = fmt.Errorf("no repository index artifact found for HelmRepository '%s'", name)
392378
}
393379

394380
return repository, err
395381
}
396382

397383
func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context,
398-
repository sourcev1.GitRepository, chart sourcev1.HelmChart) (sourcev1.HelmChart, error) {
384+
repository sourcev1.GitRepository, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) {
399385
// Create temporary working directory
400386
tmpDir, err := ioutil.TempDir("", fmt.Sprintf("%s-%s-", chart.Namespace, chart.Name))
401387
if err != nil {
@@ -434,7 +420,7 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context,
434420
// Return early if the revision is still the same as the current chart artifact
435421
artifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version,
436422
fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version))
437-
if chart.GetArtifact() != nil && chart.GetArtifact().Revision == chartMetadata.Version {
423+
if !force && chart.GetArtifact() != nil && chart.GetArtifact().Revision == chartMetadata.Version {
438424
if artifact.URL != repository.GetArtifact().URL {
439425
r.Storage.SetArtifactURL(repository.GetArtifact())
440426
repository.Status.URL = r.Storage.SetHostname(repository.Status.URL)
@@ -443,31 +429,10 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context,
443429
}
444430

445431
// Overwrite default values if instructed to
446-
if chart.Spec.ValuesFile != "" && chart.Spec.ValuesFile != chartutil.ValuesfileName {
447-
srcPath := path.Join(chartPath, chart.Spec.ValuesFile)
448-
if f, err := os.Stat(srcPath); os.IsNotExist(err) || !f.Mode().IsRegular() {
449-
err = fmt.Errorf("invalid values file path: %s", chart.Spec.ValuesFile)
450-
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
451-
}
452-
src, err := os.Open(srcPath)
453-
if err != nil {
454-
err = fmt.Errorf("failed to open values file '%s': %w", chart.Spec.ValuesFile, err)
455-
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
456-
}
457-
t, err := os.OpenFile(path.Join(tmpDir, chartutil.ValuesfileName), os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
458-
if err != nil {
459-
src.Close()
460-
err = fmt.Errorf("failed to open values file '%s': %w", chartutil.ValuesfileName, err)
461-
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
462-
}
463-
if _, err := io.Copy(t, src); err != nil {
464-
t.Close()
465-
src.Close()
466-
err = fmt.Errorf("failed to copy values file '%s' to '%s: %w", chart.Spec.ValuesFile, chartutil.ValuesfileName, err)
432+
if chart.Spec.ValuesFile != "" {
433+
if err := helm.OverwriteChartDefaultValues(chartPath, chart.Spec.ValuesFile); err != nil {
467434
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
468435
}
469-
t.Close()
470-
src.Close()
471436
}
472437

473438
// Ensure artifact directory exists

internal/helm/chart.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
Copyright 2020 The Flux CD contributors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package helm
18+
19+
import (
20+
"fmt"
21+
"io"
22+
"os"
23+
"path"
24+
25+
"helm.sh/helm/v3/pkg/chartutil"
26+
)
27+
28+
// OverwriteChartDefaultValues overwrites the chart default values file in the
29+
// given chartPath with the contents of the given valuesFile.
30+
func OverwriteChartDefaultValues(chartPath, valuesFile string) error {
31+
if valuesFile == chartutil.ValuesfileName {
32+
return nil
33+
}
34+
srcPath := path.Join(chartPath, valuesFile)
35+
if f, err := os.Stat(srcPath); os.IsNotExist(err) || !f.Mode().IsRegular() {
36+
return fmt.Errorf("invalid values file path: %s", valuesFile)
37+
}
38+
src, err := os.Open(srcPath)
39+
if err != nil {
40+
return fmt.Errorf("failed to open values file '%s': %w", valuesFile, err)
41+
}
42+
defer src.Close()
43+
t, err := os.OpenFile(path.Join(chartPath, chartutil.ValuesfileName), os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
44+
if err != nil {
45+
return fmt.Errorf("failed to open values file '%s': %w", chartutil.ValuesfileName, err)
46+
}
47+
defer t.Close()
48+
if _, err := io.Copy(t, src); err != nil {
49+
return fmt.Errorf("failed to overwrite default values with '%s': %w", valuesFile, err)
50+
}
51+
return nil
52+
}

0 commit comments

Comments
 (0)