From 03ce9d96daed9b46e85b1e4b93c7aba6c09375a2 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 18 Sep 2020 18:07:30 +0200 Subject: [PATCH 1/3] Support Helm charts from Bucket sources --- api/v1alpha1/helmchart_types.go | 4 +- .../source.toolkit.fluxcd.io_helmcharts.yaml | 3 +- controllers/helmchart_controller.go | 171 ++++++++---------- controllers/helmchart_controller_test.go | 2 +- docs/api/source.md | 2 +- 5 files changed, 85 insertions(+), 97 deletions(-) diff --git a/api/v1alpha1/helmchart_types.go b/api/v1alpha1/helmchart_types.go index c5e731fd..bc7dd6bb 100644 --- a/api/v1alpha1/helmchart_types.go +++ b/api/v1alpha1/helmchart_types.go @@ -55,8 +55,8 @@ type LocalHelmChartSourceReference struct { // +optional APIVersion string `json:"apiVersion,omitempty"` - // Kind of the referent, valid values are ('HelmRepository', 'GitRepository'). - // +kubebuilder:validation:Enum=HelmRepository;GitRepository + // Kind of the referent, valid values are ('HelmRepository', 'GitRepository', 'Bucket'). + // +kubebuilder:validation:Enum=HelmRepository;GitRepository;Bucket // +required Kind string `json:"kind"` diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml index 9bae385d..2e2774ce 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml @@ -73,10 +73,11 @@ spec: type: string kind: description: Kind of the referent, valid values are ('HelmRepository', - 'GitRepository'). + 'GitRepository', 'Bucket'). enum: - HelmRepository - GitRepository + - Bucket type: string name: description: Name of the referent. diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 1e6eced5..135506fb 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -119,33 +119,39 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { log.Error(err, "unable to purge old artifacts") } + // Retrieve the source + source, err := r.getSource(ctx, chart) + if err != nil { + chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error()) + if err := r.Status().Update(ctx, &chart); err != nil { + log.Error(err, "unable to update status") + } + return ctrl.Result{Requeue: true}, err + } + + // Assert source is ready + if source.GetArtifact() == nil { + err = fmt.Errorf("no artifact found for source `%s` kind '%s'", + chart.Spec.SourceRef.Name, chart.Spec.SourceRef.Kind) + chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error()) + if err := r.Status().Update(ctx, &chart); err != nil { + log.Error(err, "unable to update status") + } + return ctrl.Result{Requeue: true}, err + } + // Perform the reconciliation for the chart source type var reconciledChart sourcev1.HelmChart var reconcileErr error - switch chart.Spec.SourceRef.Kind { - case sourcev1.HelmRepositoryKind: - repository, err := r.getChartRepositoryWithArtifact(ctx, chart) - if err != nil { - chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error()) - if err := r.Status().Update(ctx, &chart); err != nil { - log.Error(err, "unable to update status") - } - return ctrl.Result{Requeue: true}, err - } - reconciledChart, reconcileErr = r.reconcileFromHelmRepository(ctx, repository, *chart.DeepCopy(), changed) - case sourcev1.GitRepositoryKind: - repository, err := r.getGitRepositoryWithArtifact(ctx, chart) - if err != nil { - chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error()) - if err := r.Status().Update(ctx, &chart); err != nil { - log.Error(err, "unable to update status") - } - return ctrl.Result{Requeue: true}, err - } - reconciledChart, reconcileErr = r.reconcileFromGitRepository(ctx, repository, *chart.DeepCopy(), changed) + switch typedSource := source.(type) { + case *sourcev1.HelmRepository: + reconciledChart, reconcileErr = r.reconcileFromHelmRepository(ctx, *typedSource, *chart.DeepCopy(), changed) + case *sourcev1.GitRepository, *sourcev1.Bucket: + reconciledChart, reconcileErr = r.reconcileFromTarballArtifact(ctx, *typedSource.GetArtifact(), + *chart.DeepCopy(), changed) default: err := fmt.Errorf("unable to reconcile unsupported source reference kind '%s'", chart.Spec.SourceRef.Kind) - return ctrl.Result{}, err + return ctrl.Result{Requeue: false}, err } // Update status with the reconciliation result @@ -188,6 +194,41 @@ func (r *HelmChartReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts Complete(r) } +func (r *HelmChartReconciler) getSource(ctx context.Context, chart sourcev1.HelmChart) (sourcev1.Source, error) { + var source sourcev1.Source + namespacedName := types.NamespacedName{ + Namespace: chart.GetNamespace(), + Name: chart.Spec.SourceRef.Name, + } + switch chart.Spec.SourceRef.Kind { + case sourcev1.HelmRepositoryKind: + var repository sourcev1.HelmRepository + err := r.Client.Get(ctx, namespacedName, &repository) + if err != nil { + return source, fmt.Errorf("failed to retrieve source: %w", err) + } + source = &repository + case sourcev1.GitRepositoryKind: + var repository sourcev1.GitRepository + err := r.Client.Get(ctx, namespacedName, &repository) + if err != nil { + return source, fmt.Errorf("failed to retrieve source: %w", err) + } + source = &repository + case sourcev1.BucketKind: + var bucket sourcev1.Bucket + err := r.Client.Get(ctx, namespacedName, &bucket) + if err != nil { + return source, fmt.Errorf("failed to retrieve source: %w", err) + } + source = &bucket + default: + return source, fmt.Errorf("source `%s` kind '%s' not supported", + chart.Spec.SourceRef.Name, chart.Spec.SourceRef.Kind) + } + return source, nil +} + func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, repository sourcev1.HelmRepository, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) { cv, err := helm.GetDownloadableChartVersionFromIndex(r.Storage.LocalPath(*repository.GetArtifact()), @@ -201,8 +242,8 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version)) if !force && repository.GetArtifact() != nil && repository.GetArtifact().Revision == cv.Version { if artifact.URL != repository.GetArtifact().URL { - r.Storage.SetArtifactURL(repository.GetArtifact()) - repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + r.Storage.SetArtifactURL(chart.GetArtifact()) + repository.Status.URL = r.Storage.SetHostname(chart.Status.URL) } return chart, nil } @@ -353,35 +394,8 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, return sourcev1.HelmChartReady(chart, artifact, chartUrl, readyReason, readyMessage), nil } -// getChartRepositoryWithArtifact attempts to get the v1alpha1.HelmRepository -// for the given chart. It returns an error if the HelmRepository could -// not be retrieved or if does not have an artifact. -func (r *HelmChartReconciler) getChartRepositoryWithArtifact(ctx context.Context, chart sourcev1.HelmChart) (sourcev1.HelmRepository, error) { - if chart.Spec.SourceRef.Name == "" { - return sourcev1.HelmRepository{}, fmt.Errorf("no HelmRepository reference given") - } - - name := types.NamespacedName{ - Namespace: chart.GetNamespace(), - Name: chart.Spec.SourceRef.Name, - } - - var repository sourcev1.HelmRepository - err := r.Client.Get(ctx, name, &repository) - if err != nil { - err = fmt.Errorf("failed to get HelmRepository '%s': %w", name, err) - return repository, err - } - - if repository.GetArtifact() == nil { - err = fmt.Errorf("no repository index artifact found for HelmRepository '%s'", name) - } - - return repository, err -} - -func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, - repository sourcev1.GitRepository, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) { +func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context, + artifact sourcev1.Artifact, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) { // Create temporary working directory tmpDir, err := ioutil.TempDir("", fmt.Sprintf("%s-%s-", chart.Namespace, chart.Name)) if err != nil { @@ -390,8 +404,8 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, } defer os.RemoveAll(tmpDir) - // Open GitRepository artifact file and untar files into working directory - f, err := os.Open(r.Storage.LocalPath(*repository.GetArtifact())) + // Open the tarball artifact file and untar files into working directory + f, err := os.Open(r.Storage.LocalPath(artifact)) if err != nil { err = fmt.Errorf("artifact open error: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err @@ -418,12 +432,12 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, } // Return early if the revision is still the same as the current chart artifact - artifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version, + chartArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version, fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version)) if !force && chart.GetArtifact() != nil && chart.GetArtifact().Revision == chartMetadata.Version { - if artifact.URL != repository.GetArtifact().URL { - r.Storage.SetArtifactURL(repository.GetArtifact()) - repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + if chartArtifact.URL != artifact.URL { + r.Storage.SetArtifactURL(&chartArtifact) + chart.Status.URL = r.Storage.SetHostname(chart.Status.URL) } return chart, nil } @@ -436,14 +450,14 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, } // Ensure artifact directory exists - err = r.Storage.MkdirAll(artifact) + err = r.Storage.MkdirAll(chartArtifact) if err != nil { err = fmt.Errorf("unable to create artifact directory: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err } // Acquire a lock for the artifact - unlock, err := r.Storage.Lock(artifact) + unlock, err := r.Storage.Lock(chartArtifact) if err != nil { err = fmt.Errorf("unable to acquire lock: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err @@ -466,7 +480,7 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, err = fmt.Errorf("failed to open chart package: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err } - if err := r.Storage.Copy(&artifact, cf); err != nil { + if err := r.Storage.Copy(&chartArtifact, cf); err != nil { cf.Close() err = fmt.Errorf("failed to copy chart package to storage: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err @@ -474,41 +488,14 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, cf.Close() // Update symlink - cUrl, err := r.Storage.Symlink(artifact, fmt.Sprintf("%s-latest.tgz", chartMetadata.Name)) + cUrl, err := r.Storage.Symlink(chartArtifact, fmt.Sprintf("%s-latest.tgz", chartMetadata.Name)) if err != nil { err = fmt.Errorf("storage error: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err } - message := fmt.Sprintf("Fetched and packaged revision: %s", artifact.Revision) - return sourcev1.HelmChartReady(chart, artifact, cUrl, sourcev1.ChartPackageSucceededReason, message), nil -} - -// getGitRepositoryWithArtifact attempts to get the GitRepository for the given -// chart. It returns an error if the v1alpha1.GitRepository could not be retrieved -// or does not have an artifact. -func (r *HelmChartReconciler) getGitRepositoryWithArtifact(ctx context.Context, chart sourcev1.HelmChart) (sourcev1.GitRepository, error) { - if chart.Spec.SourceRef.Name == "" { - return sourcev1.GitRepository{}, fmt.Errorf("no GitRepository reference given") - } - - name := types.NamespacedName{ - Namespace: chart.GetNamespace(), - Name: chart.Spec.SourceRef.Name, - } - - var repository sourcev1.GitRepository - err := r.Client.Get(ctx, name, &repository) - if err != nil { - err = fmt.Errorf("failed to get GitRepository '%s': %w", name, err) - return repository, err - } - - if repository.GetArtifact() == nil { - err = fmt.Errorf("no artifact found for GitRepository '%s'", repository.Name) - } - - return repository, err + message := fmt.Sprintf("Fetched and packaged revision: %s", chartArtifact.Revision) + return sourcev1.HelmChartReady(chart, chartArtifact, cUrl, sourcev1.ChartPackageSucceededReason, message), nil } // resetStatus returns a modified v1alpha1.HelmChart and a boolean indicating diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 1b51427d..2c282ab4 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -152,7 +152,7 @@ var _ = Describe("HelmChartReconciler", func() { _ = k8sClient.Get(context.Background(), key, updated) for _, c := range updated.Status.Conditions { if c.Reason == sourcev1.ChartPullFailedReason && - strings.Contains(c.Message, "failed to get HelmRepository") { + strings.Contains(c.Message, "failed to retrieve source") { return true } } diff --git a/docs/api/source.md b/docs/api/source.md index a264f4bb..f53222a2 100644 --- a/docs/api/source.md +++ b/docs/api/source.md @@ -1600,7 +1600,7 @@ string -

Kind of the referent, valid values are (‘HelmRepository’, ‘GitRepository’).

+

Kind of the referent, valid values are (‘HelmRepository’, ‘GitRepository’, ‘Bucket’).

From 1798b200c969ddbbb902f8f5608bc653885d87e4 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 21 Sep 2020 11:21:56 +0200 Subject: [PATCH 2/3] e2e: test HelmChart from Bucket --- .github/workflows/e2e.yaml | 18 ++++++++++--- config/testdata/{minio => bucket}/source.yaml | 9 +------ .../helmchart-from-bucket/source.yaml | 25 +++++++++++++++++++ config/testdata/minio/secret.yaml | 8 ++++++ 4 files changed, 48 insertions(+), 12 deletions(-) rename config/testdata/{minio => bucket}/source.yaml (68%) create mode 100644 config/testdata/helmchart-from-bucket/source.yaml create mode 100644 config/testdata/minio/secret.yaml diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 4aab0cde..11139f91 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -81,12 +81,22 @@ jobs: wget -q https://dl.min.io/client/mc/release/linux-amd64/mc chmod +x mc ./mc alias set minio http://localhost:9000 myaccesskey mysecretkey --api S3v4 - ./mc mb minio/podinfo - ./mc cp --recursive ./config/testdata/minio/manifests minio/podinfo - - name: Run S3 tests + kubectl -n source-system apply -f ./config/testdata/minio/secret.yaml + - name: Run Bucket tests run: | - kubectl -n source-system apply -f ./config/testdata/minio/source.yaml + ./mc mb minio/podinfo + ./mc mirror ./config/testdata/minio/manifests/ minio/podinfo + + kubectl -n source-system apply -f ./config/testdata/bucket/source.yaml kubectl -n source-system wait bucket/podinfo --for=condition=ready --timeout=1m + - name: Run HelmChart from Bucket tests + run: | + ./mc mb minio/charts + ./mc mirror ./controllers/testdata/helmchart/ minio/charts/helmchart + + kubectl -n source-system apply -f ./config/testdata/helmchart-from-bucket/source.yaml + kubectl -n source-system wait bucket/charts --for=condition=ready --timeout=1m + kubectl -n source-system wait helmchart/helmchart-bucket --for=condition=ready --timeout=1m - name: Debug failure if: failure() run: | diff --git a/config/testdata/minio/source.yaml b/config/testdata/bucket/source.yaml similarity index 68% rename from config/testdata/minio/source.yaml rename to config/testdata/bucket/source.yaml index ccd1ed8b..3bc569d3 100644 --- a/config/testdata/minio/source.yaml +++ b/config/testdata/bucket/source.yaml @@ -1,3 +1,4 @@ +--- apiVersion: source.toolkit.fluxcd.io/v1alpha1 kind: Bucket metadata: @@ -11,11 +12,3 @@ spec: insecure: true secretRef: name: minio-credentials ---- -apiVersion: v1 -kind: Secret -metadata: - name: minio-credentials -data: - accesskey: bXlhY2Nlc3NrZXk= - secretkey: bXlzZWNyZXRrZXk= diff --git a/config/testdata/helmchart-from-bucket/source.yaml b/config/testdata/helmchart-from-bucket/source.yaml new file mode 100644 index 00000000..9c09321f --- /dev/null +++ b/config/testdata/helmchart-from-bucket/source.yaml @@ -0,0 +1,25 @@ +--- +apiVersion: source.toolkit.fluxcd.io/v1alpha1 +kind: Bucket +metadata: + name: charts +spec: + interval: 1m + provider: generic + bucketName: charts + endpoint: minio.minio.svc.cluster.local:9000 + region: us-east-1 + insecure: true + secretRef: + name: minio-credentials +--- +apiVersion: source.toolkit.fluxcd.io/v1alpha1 +kind: HelmChart +metadata: + name: helmchart-bucket +spec: + chart: ./helmchart + sourceRef: + kind: Bucket + name: charts + interval: 1m diff --git a/config/testdata/minio/secret.yaml b/config/testdata/minio/secret.yaml new file mode 100644 index 00000000..35cc5a39 --- /dev/null +++ b/config/testdata/minio/secret.yaml @@ -0,0 +1,8 @@ +--- +apiVersion: v1 +kind: Secret +metadata: + name: minio-credentials +data: + accesskey: bXlhY2Nlc3NrZXk= + secretkey: bXlzZWNyZXRrZXk= From 8cdb821946a372868456cad0504b213d354f0832 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 21 Sep 2020 12:38:53 +0200 Subject: [PATCH 3/3] docs: document Bucket support HelmChart --- api/v1alpha1/helmchart_types.go | 2 +- .../source.toolkit.fluxcd.io_helmcharts.yaml | 2 +- docs/api/source.md | 4 ++-- docs/spec/v1alpha1/helmcharts.md | 22 ++++++++++++++++--- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/api/v1alpha1/helmchart_types.go b/api/v1alpha1/helmchart_types.go index bc7dd6bb..9b4c919b 100644 --- a/api/v1alpha1/helmchart_types.go +++ b/api/v1alpha1/helmchart_types.go @@ -30,7 +30,7 @@ type HelmChartSpec struct { Chart string `json:"chart"` // The chart version semver expression, ignored for charts from GitRepository - // sources. Defaults to latest when omitted. + // and Bucket sources. Defaults to latest when omitted. // +optional Version string `json:"version,omitempty"` diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml index 2e2774ce..456f4418 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml @@ -92,7 +92,7 @@ spec: type: string version: description: The chart version semver expression, ignored for charts - from GitRepository sources. Defaults to latest when omitted. + from GitRepository and Bucket sources. Defaults to latest when omitted. type: string required: - chart diff --git a/docs/api/source.md b/docs/api/source.md index f53222a2..33ae2986 100644 --- a/docs/api/source.md +++ b/docs/api/source.md @@ -459,7 +459,7 @@ string (Optional)

The chart version semver expression, ignored for charts from GitRepository -sources. Defaults to latest when omitted.

+and Bucket sources. Defaults to latest when omitted.

@@ -1292,7 +1292,7 @@ string (Optional)

The chart version semver expression, ignored for charts from GitRepository -sources. Defaults to latest when omitted.

+and Bucket sources. Defaults to latest when omitted.

diff --git a/docs/spec/v1alpha1/helmcharts.md b/docs/spec/v1alpha1/helmcharts.md index c00829a4..aa810a64 100644 --- a/docs/spec/v1alpha1/helmcharts.md +++ b/docs/spec/v1alpha1/helmcharts.md @@ -16,7 +16,7 @@ type HelmChartSpec struct { Chart string `json:"chart"` // The chart version semver expression, ignored for charts from GitRepository - // sources. Defaults to latest when omitted. + // and Bucket sources. Defaults to latest when omitted. // +optional Version string `json:"version,omitempty"` @@ -40,8 +40,8 @@ type LocalHelmChartSourceReference struct { // +optional APIVersion string `json:"apiVersion,omitempty"` - // Kind of the referent, valid values are ('HelmRepository', 'GitRepository'). - // +kubebuilder:validation:Enum=HelmRepository;GitRepository + // Kind of the referent, valid values are ('HelmRepository', 'GitRepository', 'Bucket'). + // +kubebuilder:validation:Enum=HelmRepository;GitRepository;Bucket // +required Kind string `json:"kind"` @@ -141,6 +141,22 @@ spec: interval: 10m ``` +Check a S3 compatible bucket every ten minutes for a new `version` in the +`Chart.yaml`, and package a new chart if the revision differs: + +```yaml +apiVersion: source.toolkit.fluxcd.io/v1alpha1 +kind: HelmChart +metadata: + name: podinfo +spec: + chart: ./podinfo + sourceRef: + name: charts + kind: Bucket + interval: 10m +``` + ## Status examples Successful chart pull: