From 7d0f79f41b84efea5d6b0fd6cab64ea58daf149d Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 15 Nov 2021 22:31:33 +0100 Subject: [PATCH] internal/helm: divide into subpackages With all the logic that used to reside in the `controllers` package factored into this package, it became cluttered. This commit tries to bring a bit more structure in place. Signed-off-by: Hidde Beydals --- controllers/helmchart_controller.go | 146 +++++++++--------- controllers/helmrepository_controller.go | 78 +++++----- .../{chart_builder.go => chart/builder.go} | 66 ++++---- .../builder_local.go} | 21 +-- .../builder_local_test.go} | 8 +- .../builder_remote.go} | 25 +-- .../builder_remote_test.go} | 8 +- .../builder_test.go} | 10 +- .../helm/{ => chart}/dependency_manager.go | 61 ++++---- .../{ => chart}/dependency_manager_test.go | 85 ++++------ internal/helm/{chart.go => chart/metadata.go} | 12 +- .../{chart_test.go => chart/metadata_test.go} | 27 +++- internal/helm/{ => getter}/getter.go | 2 +- internal/helm/{ => getter}/getter_test.go | 2 +- internal/helm/getter/mock.go | 41 +++++ .../chart_repository.go} | 10 +- .../chart_repository_test.go} | 46 +++--- internal/helm/{ => repository}/utils.go | 9 +- internal/helm/repository/utils_test.go | 44 ++++++ internal/helm/utils_test.go | 60 ------- 20 files changed, 397 insertions(+), 364 deletions(-) rename internal/helm/{chart_builder.go => chart/builder.go} (70%) rename internal/helm/{chart_builder_local.go => chart/builder_local.go} (90%) rename internal/helm/{chart_builder_local_test.go => chart/builder_local_test.go} (96%) rename internal/helm/{chart_builder_remote.go => chart/builder_remote.go} (91%) rename internal/helm/{chart_builder_remote_test.go => chart/builder_remote_test.go} (92%) rename internal/helm/{chart_builder_test.go => chart/builder_test.go} (89%) rename internal/helm/{ => chart}/dependency_manager.go (81%) rename internal/helm/{ => chart}/dependency_manager_test.go (84%) rename internal/helm/{chart.go => chart/metadata.go} (96%) rename internal/helm/{chart_test.go => chart/metadata_test.go} (85%) rename internal/helm/{ => getter}/getter.go (99%) rename internal/helm/{ => getter}/getter_test.go (99%) create mode 100644 internal/helm/getter/mock.go rename internal/helm/{repository.go => repository/chart_repository.go} (98%) rename internal/helm/{repository_test.go => repository/chart_repository_test.go} (93%) rename internal/helm/{ => repository}/utils.go (77%) create mode 100644 internal/helm/repository/utils_test.go delete mode 100644 internal/helm/utils_test.go diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index bcb8f8e7..d31f6c2b 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -28,7 +28,7 @@ import ( securejoin "github.com/cyphar/filepath-securejoin" "github.com/go-logr/logr" - "helm.sh/helm/v3/pkg/getter" + extgetter "helm.sh/helm/v3/pkg/getter" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -54,7 +54,9 @@ import ( "github.com/fluxcd/pkg/untar" sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" - "github.com/fluxcd/source-controller/internal/helm" + "github.com/fluxcd/source-controller/internal/helm/chart" + "github.com/fluxcd/source-controller/internal/helm/getter" + "github.com/fluxcd/source-controller/internal/helm/repository" ) // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmcharts,verbs=get;list;watch;create;update;patch;delete @@ -67,7 +69,7 @@ type HelmChartReconciler struct { client.Client Scheme *runtime.Scheme Storage *Storage - Getters getter.Providers + Getters extgetter.Providers EventRecorder kuberecorder.EventRecorder ExternalEventRecorder *events.Recorder MetricsRecorder *metrics.Recorder @@ -304,218 +306,218 @@ func (r *HelmChartReconciler) getSource(ctx context.Context, chart sourcev1.Helm return source, nil } -func (r *HelmChartReconciler) fromHelmRepository(ctx context.Context, repository sourcev1.HelmRepository, - chart sourcev1.HelmChart, workDir string, force bool) (sourcev1.HelmChart, error) { - // Configure ChartRepository getter options - clientOpts := []getter.Option{ - getter.WithURL(repository.Spec.URL), - getter.WithTimeout(repository.Spec.Timeout.Duration), - getter.WithPassCredentialsAll(repository.Spec.PassCredentials), +func (r *HelmChartReconciler) fromHelmRepository(ctx context.Context, repo sourcev1.HelmRepository, c sourcev1.HelmChart, + workDir string, force bool) (sourcev1.HelmChart, error) { + // Configure Index getter options + clientOpts := []extgetter.Option{ + extgetter.WithURL(repo.Spec.URL), + extgetter.WithTimeout(repo.Spec.Timeout.Duration), + extgetter.WithPassCredentialsAll(repo.Spec.PassCredentials), } - if secret, err := r.getHelmRepositorySecret(ctx, &repository); err != nil { - return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err + if secret, err := r.getHelmRepositorySecret(ctx, &repo); err != nil { + return sourcev1.HelmChartNotReady(c, sourcev1.AuthenticationFailedReason, err.Error()), err } else if secret != nil { // Create temporary working directory for credentials authDir := filepath.Join(workDir, "creds") if err := os.Mkdir(authDir, 0700); err != nil { err = fmt.Errorf("failed to create temporary directory for repository credentials: %w", err) } - opts, err := helm.ClientOptionsFromSecret(authDir, *secret) + opts, err := getter.ClientOptionsFromSecret(authDir, *secret) if err != nil { - err = fmt.Errorf("failed to create client options for HelmRepository '%s': %w", repository.Name, err) - return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err + err = fmt.Errorf("failed to create client options for HelmRepository '%s': %w", repo.Name, err) + return sourcev1.HelmChartNotReady(c, sourcev1.AuthenticationFailedReason, err.Error()), err } clientOpts = append(clientOpts, opts...) } // Initialize the chart repository - chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Storage.LocalPath(*repository.GetArtifact()), r.Getters, clientOpts) + chartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, clientOpts) if err != nil { switch err.(type) { case *url.Error: - return sourcev1.HelmChartNotReady(chart, sourcev1.URLInvalidReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.URLInvalidReason, err.Error()), err default: - return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.ChartPullFailedReason, err.Error()), err } } var cachedChart string - if artifact := chart.GetArtifact(); artifact != nil { + if artifact := c.GetArtifact(); artifact != nil { cachedChart = artifact.Path } // Build the chart - cBuilder := helm.NewRemoteChartBuilder(chartRepo) - ref := helm.RemoteChartReference{Name: chart.Spec.Chart, Version: chart.Spec.Version} - opts := helm.BuildOptions{ - ValueFiles: chart.GetValuesFiles(), + cBuilder := chart.NewRemoteBuilder(chartRepo) + ref := chart.RemoteReference{Name: c.Spec.Chart, Version: c.Spec.Version} + opts := chart.BuildOptions{ + ValueFiles: c.GetValuesFiles(), CachedChart: cachedChart, Force: force, } build, err := cBuilder.Build(ctx, ref, filepath.Join(workDir, "chart.tgz"), opts) if err != nil { - return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.ChartPullFailedReason, err.Error()), err } - newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), build.Version, + newArtifact := r.Storage.NewArtifactFor(c.Kind, c.GetObjectMeta(), build.Version, fmt.Sprintf("%s-%s.tgz", build.Name, build.Version)) // If the path of the returned build equals the cache path, // there are no changes to the chart if build.Path == cachedChart { // Ensure hostname is updated - if chart.GetArtifact().URL != newArtifact.URL { - r.Storage.SetArtifactURL(chart.GetArtifact()) - chart.Status.URL = r.Storage.SetHostname(chart.Status.URL) + if c.GetArtifact().URL != newArtifact.URL { + r.Storage.SetArtifactURL(c.GetArtifact()) + c.Status.URL = r.Storage.SetHostname(c.Status.URL) } - return chart, nil + return c, nil } // Ensure artifact directory exists err = r.Storage.MkdirAll(newArtifact) if err != nil { err = fmt.Errorf("unable to create chart directory: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } // Acquire a lock for the artifact unlock, err := r.Storage.Lock(newArtifact) if err != nil { err = fmt.Errorf("unable to acquire lock: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } defer unlock() // Copy the packaged chart to the artifact path if err = r.Storage.CopyFromPath(&newArtifact, build.Path); err != nil { err = fmt.Errorf("failed to write chart package to storage: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } // Update symlink cUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", build.Name)) if err != nil { err = fmt.Errorf("storage error: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } - return sourcev1.HelmChartReady(chart, newArtifact, cUrl, sourcev1.ChartPullSucceededReason, build.Summary()), nil + return sourcev1.HelmChartReady(c, newArtifact, cUrl, sourcev1.ChartPullSucceededReason, build.Summary()), nil } -func (r *HelmChartReconciler) fromTarballArtifact(ctx context.Context, source sourcev1.Artifact, - chart sourcev1.HelmChart, workDir string, force bool) (sourcev1.HelmChart, error) { +func (r *HelmChartReconciler) fromTarballArtifact(ctx context.Context, source sourcev1.Artifact, c sourcev1.HelmChart, + workDir string, force bool) (sourcev1.HelmChart, error) { // Create temporary working directory to untar into sourceDir := filepath.Join(workDir, "source") if err := os.Mkdir(sourceDir, 0700); err != nil { err = fmt.Errorf("failed to create temporary directory to untar source into: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } // Open the tarball artifact file and untar files into working directory f, err := os.Open(r.Storage.LocalPath(source)) if err != nil { err = fmt.Errorf("artifact open error: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } if _, err = untar.Untar(f, sourceDir); err != nil { _ = f.Close() err = fmt.Errorf("artifact untar error: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } if err =f.Close(); err != nil { err = fmt.Errorf("artifact close error: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } - chartPath, err := securejoin.SecureJoin(sourceDir, chart.Spec.Chart) + chartPath, err := securejoin.SecureJoin(sourceDir, c.Spec.Chart) if err != nil { - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } // Setup dependency manager authDir := filepath.Join(workDir, "creds") if err = os.Mkdir(authDir, 0700); err != nil { err = fmt.Errorf("failed to create temporaRy directory for dependency credentials: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } - dm := helm.NewDependencyManager( - helm.WithRepositoryCallback(r.getNamespacedChartRepositoryCallback(ctx, authDir, chart.GetNamespace())), + dm := chart.NewDependencyManager( + chart.WithRepositoryCallback(r.getNamespacedChartRepositoryCallback(ctx, authDir, c.GetNamespace())), ) defer dm.Clear() // Get any cached chart var cachedChart string - if artifact := chart.Status.Artifact; artifact != nil { + if artifact := c.Status.Artifact; artifact != nil { cachedChart = artifact.Path } - buildsOpts := helm.BuildOptions{ - ValueFiles: chart.GetValuesFiles(), + buildsOpts := chart.BuildOptions{ + ValueFiles: c.GetValuesFiles(), CachedChart: cachedChart, Force: force, } // Add revision metadata to chart build - if chart.Spec.ReconcileStrategy == sourcev1.ReconcileStrategyRevision { + if c.Spec.ReconcileStrategy == sourcev1.ReconcileStrategyRevision { // Isolate the commit SHA from GitRepository type artifacts by removing the branch/ prefix. splitRev := strings.Split(source.Revision, "/") buildsOpts.VersionMetadata = splitRev[len(splitRev)-1] } // Build chart - chartB := helm.NewLocalChartBuilder(dm) - build, err := chartB.Build(ctx, helm.LocalChartReference{BaseDir: sourceDir, Path: chartPath}, filepath.Join(workDir, "chart.tgz"), buildsOpts) + chartB := chart.NewLocalBuilder(dm) + build, err := chartB.Build(ctx, chart.LocalReference{BaseDir: sourceDir, Path: chartPath}, filepath.Join(workDir, "chart.tgz"), buildsOpts) if err != nil { - return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.ChartPackageFailedReason, err.Error()), err } - newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), build.Version, + newArtifact := r.Storage.NewArtifactFor(c.Kind, c.GetObjectMeta(), build.Version, fmt.Sprintf("%s-%s.tgz", build.Name, build.Version)) // If the path of the returned build equals the cache path, // there are no changes to the chart if build.Path == cachedChart { // Ensure hostname is updated - if chart.GetArtifact().URL != newArtifact.URL { - r.Storage.SetArtifactURL(chart.GetArtifact()) - chart.Status.URL = r.Storage.SetHostname(chart.Status.URL) + if c.GetArtifact().URL != newArtifact.URL { + r.Storage.SetArtifactURL(c.GetArtifact()) + c.Status.URL = r.Storage.SetHostname(c.Status.URL) } - return chart, nil + return c, nil } // Ensure artifact directory exists err = r.Storage.MkdirAll(newArtifact) if err != nil { err = fmt.Errorf("unable to create chart directory: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } // Acquire a lock for the artifact unlock, err := r.Storage.Lock(newArtifact) if err != nil { err = fmt.Errorf("unable to acquire lock: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } defer unlock() // Copy the packaged chart to the artifact path if err = r.Storage.CopyFromPath(&newArtifact, build.Path); err != nil { err = fmt.Errorf("failed to write chart package to storage: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } // Update symlink - cUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", chart.Name)) + cUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", build.Name)) if err != nil { err = fmt.Errorf("storage error: %w", err) - return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmChartNotReady(c, sourcev1.StorageOperationFailedReason, err.Error()), err } - return sourcev1.HelmChartReady(chart, newArtifact, cUrl, sourcev1.ChartPackageSucceededReason, build.Summary()), nil + return sourcev1.HelmChartReady(c, newArtifact, cUrl, sourcev1.ChartPackageSucceededReason, build.Summary()), nil } // TODO(hidde): factor out to helper? -func (r *HelmChartReconciler) getNamespacedChartRepositoryCallback(ctx context.Context, dir, namespace string) helm.GetChartRepositoryCallback { - return func(url string) (*helm.ChartRepository, error) { +func (r *HelmChartReconciler) getNamespacedChartRepositoryCallback(ctx context.Context, dir, namespace string) chart.GetChartRepositoryCallback { + return func(url string) (*repository.ChartRepository, error) { repo, err := r.resolveDependencyRepository(ctx, url, namespace) if err != nil { if errors.ReasonForError(err) != metav1.StatusReasonUnknown { @@ -528,21 +530,21 @@ func (r *HelmChartReconciler) getNamespacedChartRepositoryCallback(ctx context.C }, } } - clientOpts := []getter.Option{ - getter.WithURL(repo.Spec.URL), - getter.WithTimeout(repo.Spec.Timeout.Duration), - getter.WithPassCredentialsAll(repo.Spec.PassCredentials), + clientOpts := []extgetter.Option{ + extgetter.WithURL(repo.Spec.URL), + extgetter.WithTimeout(repo.Spec.Timeout.Duration), + extgetter.WithPassCredentialsAll(repo.Spec.PassCredentials), } if secret, err := r.getHelmRepositorySecret(ctx, repo); err != nil { return nil, err } else if secret != nil { - opts, err := helm.ClientOptionsFromSecret(dir, *secret) + opts, err := getter.ClientOptionsFromSecret(dir, *secret) if err != nil { return nil, err } clientOpts = append(clientOpts, opts...) } - chartRepo, err := helm.NewChartRepository(repo.Spec.URL, "", r.Getters, clientOpts) + chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, clientOpts) if err != nil { return nil, err } @@ -663,7 +665,7 @@ func (r *HelmChartReconciler) indexHelmRepositoryByURL(o client.Object) []string if !ok { panic(fmt.Sprintf("Expected a HelmRepository, got %T", o)) } - u := helm.NormalizeChartRepositoryURL(repo.Spec.URL) + u := repository.NormalizeURL(repo.Spec.URL) if u != "" { return []string{u} } diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 794a912e..8ab87201 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -23,12 +23,8 @@ import ( "os" "time" - "github.com/fluxcd/pkg/apis/meta" - "github.com/fluxcd/pkg/runtime/events" - "github.com/fluxcd/pkg/runtime/metrics" - "github.com/fluxcd/pkg/runtime/predicates" "github.com/go-logr/logr" - "helm.sh/helm/v3/pkg/getter" + extgetter "helm.sh/helm/v3/pkg/getter" corev1 "k8s.io/api/core/v1" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -42,8 +38,14 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/predicate" + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/events" + "github.com/fluxcd/pkg/runtime/metrics" + "github.com/fluxcd/pkg/runtime/predicates" + + "github.com/fluxcd/source-controller/internal/helm/getter" + "github.com/fluxcd/source-controller/internal/helm/repository" sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" - "github.com/fluxcd/source-controller/internal/helm" ) // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmrepositories,verbs=get;list;watch;create;update;patch;delete @@ -56,7 +58,7 @@ type HelmRepositoryReconciler struct { client.Client Scheme *runtime.Scheme Storage *Storage - Getters getter.Providers + Getters extgetter.Providers EventRecorder kuberecorder.EventRecorder ExternalEventRecorder *events.Recorder MetricsRecorder *metrics.Recorder @@ -168,74 +170,74 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{RequeueAfter: repository.GetInterval().Duration}, nil } -func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sourcev1.HelmRepository) (sourcev1.HelmRepository, error) { - clientOpts := []getter.Option{ - getter.WithURL(repository.Spec.URL), - getter.WithTimeout(repository.Spec.Timeout.Duration), - getter.WithPassCredentialsAll(repository.Spec.PassCredentials), +func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repo sourcev1.HelmRepository) (sourcev1.HelmRepository, error) { + clientOpts := []extgetter.Option{ + extgetter.WithURL(repo.Spec.URL), + extgetter.WithTimeout(repo.Spec.Timeout.Duration), + extgetter.WithPassCredentialsAll(repo.Spec.PassCredentials), } - if repository.Spec.SecretRef != nil { + if repo.Spec.SecretRef != nil { name := types.NamespacedName{ - Namespace: repository.GetNamespace(), - Name: repository.Spec.SecretRef.Name, + Namespace: repo.GetNamespace(), + Name: repo.Spec.SecretRef.Name, } var secret corev1.Secret err := r.Client.Get(ctx, name, &secret) if err != nil { err = fmt.Errorf("auth secret error: %w", err) - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.AuthenticationFailedReason, err.Error()), err } authDir, err := os.MkdirTemp("", "helm-repository-") if err != nil { err = fmt.Errorf("failed to create temporary working directory for credentials: %w", err) - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.AuthenticationFailedReason, err.Error()), err } defer os.RemoveAll(authDir) - opts, err := helm.ClientOptionsFromSecret(authDir, secret) + opts, err := getter.ClientOptionsFromSecret(authDir, secret) if err != nil { err = fmt.Errorf("auth options error: %w", err) - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.AuthenticationFailedReason, err.Error()), err } clientOpts = append(clientOpts, opts...) } - chartRepo, err := helm.NewChartRepository(repository.Spec.URL, "", r.Getters, clientOpts) + chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, clientOpts) if err != nil { switch err.(type) { case *url.Error: - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.URLInvalidReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.URLInvalidReason, err.Error()), err default: - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.IndexationFailedReason, err.Error()), err } } revision, err := chartRepo.CacheIndex() if err != nil { err = fmt.Errorf("failed to download repository index: %w", err) - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.IndexationFailedReason, err.Error()), err } defer chartRepo.RemoveCache() - artifact := r.Storage.NewArtifactFor(repository.Kind, - repository.ObjectMeta.GetObjectMeta(), + artifact := r.Storage.NewArtifactFor(repo.Kind, + repo.ObjectMeta.GetObjectMeta(), revision, fmt.Sprintf("index-%s.yaml", revision)) // Return early on unchanged index - if apimeta.IsStatusConditionTrue(repository.Status.Conditions, meta.ReadyCondition) && - repository.GetArtifact().HasRevision(artifact.Revision) { - if artifact.URL != repository.GetArtifact().URL { - r.Storage.SetArtifactURL(repository.GetArtifact()) - repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + if apimeta.IsStatusConditionTrue(repo.Status.Conditions, meta.ReadyCondition) && + repo.GetArtifact().HasRevision(artifact.Revision) { + if artifact.URL != repo.GetArtifact().URL { + r.Storage.SetArtifactURL(repo.GetArtifact()) + repo.Status.URL = r.Storage.SetHostname(repo.Status.URL) } - return repository, nil + return repo, nil } // Load the cached repository index to ensure it passes validation if err := chartRepo.LoadFromCache(); err != nil { - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.IndexationFailedReason, err.Error()), err } defer chartRepo.Unload() @@ -243,14 +245,14 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou err = r.Storage.MkdirAll(artifact) if err != nil { err = fmt.Errorf("unable to create repository index directory: %w", err) - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.StorageOperationFailedReason, err.Error()), err } // Acquire lock unlock, err := r.Storage.Lock(artifact) if err != nil { err = fmt.Errorf("unable to acquire lock: %w", err) - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.StorageOperationFailedReason, err.Error()), err } defer unlock() @@ -258,10 +260,10 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou storageTarget := r.Storage.LocalPath(artifact) if storageTarget == "" { err := fmt.Errorf("failed to calcalute local storage path to store artifact to") - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.StorageOperationFailedReason, err.Error()), err } if err = chartRepo.Index.WriteFile(storageTarget, 0644); err != nil { - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.StorageOperationFailedReason, err.Error()), err } // TODO(hidde): it would be better to make the Storage deal with this artifact.Checksum = chartRepo.Checksum @@ -271,11 +273,11 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou indexURL, err := r.Storage.Symlink(artifact, "index.yaml") if err != nil { err = fmt.Errorf("storage error: %w", err) - return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err + return sourcev1.HelmRepositoryNotReady(repo, sourcev1.StorageOperationFailedReason, err.Error()), err } message := fmt.Sprintf("Fetched revision: %s", artifact.Revision) - return sourcev1.HelmRepositoryReady(repository, artifact, indexURL, sourcev1.IndexationSucceededReason, message), nil + return sourcev1.HelmRepositoryReady(repo, artifact, indexURL, sourcev1.IndexationSucceededReason, message), nil } func (r *HelmRepositoryReconciler) reconcileDelete(ctx context.Context, repository sourcev1.HelmRepository) (ctrl.Result, error) { diff --git a/internal/helm/chart_builder.go b/internal/helm/chart/builder.go similarity index 70% rename from internal/helm/chart_builder.go rename to internal/helm/chart/builder.go index 4177983c..3698d02c 100644 --- a/internal/helm/chart_builder.go +++ b/internal/helm/chart/builder.go @@ -14,49 +14,51 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package chart import ( "context" "fmt" "os" "path/filepath" + "regexp" "strings" - "github.com/fluxcd/source-controller/internal/fs" helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" + + "github.com/fluxcd/source-controller/internal/fs" ) -// ChartReference holds information to locate a chart. -type ChartReference interface { - // Validate returns an error if the ChartReference is not valid according +// Reference holds information to locate a chart. +type Reference interface { + // Validate returns an error if the Reference is not valid according // to the spec of the interface implementation. Validate() error } -// LocalChartReference contains sufficient information to locate a chart on the +// LocalReference contains sufficient information to locate a chart on the // local filesystem. -type LocalChartReference struct { - // BaseDir used as chroot during build operations. +type LocalReference struct { + // WorkDir used as chroot during build operations. // File references are not allowed to traverse outside it. - BaseDir string + WorkDir string // Path of the chart on the local filesystem. Path string } -// Validate returns an error if the LocalChartReference does not have +// Validate returns an error if the LocalReference does not have // a Path set. -func (r LocalChartReference) Validate() error { +func (r LocalReference) Validate() error { if r.Path == "" { return fmt.Errorf("no path set for local chart reference") } return nil } -// RemoteChartReference contains sufficient information to look up a chart in +// RemoteReference contains sufficient information to look up a chart in // a ChartRepository. -type RemoteChartReference struct { +type RemoteReference struct { // Name of the chart. Name string // Version of the chart. @@ -64,25 +66,29 @@ type RemoteChartReference struct { Version string } -// Validate returns an error if the RemoteChartReference does not have +// Validate returns an error if the RemoteReference does not have // a Name set. -func (r RemoteChartReference) Validate() error { +func (r RemoteReference) Validate() error { if r.Name == "" { return fmt.Errorf("no name set for remote chart reference") } + name := regexp.MustCompile("^([-a-z0-9]*)$") + if !name.MatchString(r.Name) { + return fmt.Errorf("invalid chart name '%s': a valid name must be lower case letters and numbers and MAY be separated with dashes (-)", r.Name) + } return nil } -// ChartBuilder is capable of building a (specific) ChartReference. -type ChartBuilder interface { - // Build builds and packages a Helm chart with the given ChartReference - // and BuildOptions and writes it to p. It returns the ChartBuild result, - // or an error. It may return an error for unsupported ChartReference +// Builder is capable of building a (specific) chart Reference. +type Builder interface { + // Build builds and packages a Helm chart with the given Reference + // and BuildOptions and writes it to p. It returns the Build result, + // or an error. It may return an error for unsupported Reference // implementations. - Build(ctx context.Context, ref ChartReference, p string, opts BuildOptions) (*ChartBuild, error) + Build(ctx context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) } -// BuildOptions provides a list of options for ChartBuilder.Build. +// BuildOptions provides a list of options for Builder.Build. type BuildOptions struct { // VersionMetadata can be set to SemVer build metadata as defined in // the spec, and is included during packaging. @@ -109,9 +115,9 @@ func (o BuildOptions) GetValueFiles() []string { return o.ValueFiles } -// ChartBuild contains the ChartBuilder.Build result, including specific +// Build contains the Builder.Build result, including specific // information about the built chart like ResolvedDependencies. -type ChartBuild struct { +type Build struct { // Path is the absolute path to the packaged chart. Path string // Name of the packaged chart. @@ -124,14 +130,14 @@ type ChartBuild struct { // ResolvedDependencies is the number of local and remote dependencies // collected by the DependencyManager before building the chart. ResolvedDependencies int - // Packaged indicates if the ChartBuilder has packaged the chart. + // Packaged indicates if the Builder has packaged the chart. // This can for example be false if ValueFiles is empty and the chart // source was already packaged. Packaged bool } -// Summary returns a human-readable summary of the ChartBuild. -func (b *ChartBuild) Summary() string { +// Summary returns a human-readable summary of the Build. +func (b *Build) Summary() string { if b == nil { return "no chart build" } @@ -155,15 +161,15 @@ func (b *ChartBuild) Summary() string { return s.String() } -// String returns the Path of the ChartBuild. -func (b *ChartBuild) String() string { +// String returns the Path of the Build. +func (b *Build) String() string { if b != nil { return b.Path } return "" } -// packageToPath attempts to package the given chart.Chart to the out filepath. +// packageToPath attempts to package the given chart to the out filepath. func packageToPath(chart *helmchart.Chart, out string) error { o, err := os.MkdirTemp("", "chart-build-*") if err != nil { diff --git a/internal/helm/chart_builder_local.go b/internal/helm/chart/builder_local.go similarity index 90% rename from internal/helm/chart_builder_local.go rename to internal/helm/chart/builder_local.go index 13e5dbe9..037a2fe1 100644 --- a/internal/helm/chart_builder_local.go +++ b/internal/helm/chart/builder_local.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package chart import ( "context" @@ -24,27 +24,28 @@ import ( "github.com/Masterminds/semver/v3" securejoin "github.com/cyphar/filepath-securejoin" - "github.com/fluxcd/pkg/runtime/transform" "helm.sh/helm/v3/pkg/chart/loader" "sigs.k8s.io/yaml" + + "github.com/fluxcd/pkg/runtime/transform" ) type localChartBuilder struct { dm *DependencyManager } -// NewLocalChartBuilder returns a ChartBuilder capable of building a Helm -// chart with a LocalChartReference. For chart references pointing to a +// NewLocalBuilder returns a Builder capable of building a Helm +// chart with a LocalReference. For chart references pointing to a // directory, the DependencyManager is used to resolve missing local and // remote dependencies. -func NewLocalChartBuilder(dm *DependencyManager) ChartBuilder { +func NewLocalBuilder(dm *DependencyManager) Builder { return &localChartBuilder{ dm: dm, } } -func (b *localChartBuilder) Build(ctx context.Context, ref ChartReference, p string, opts BuildOptions) (*ChartBuild, error) { - localRef, ok := ref.(LocalChartReference) +func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) { + localRef, ok := ref.(LocalReference) if !ok { return nil, fmt.Errorf("expected local chart reference") } @@ -53,14 +54,14 @@ func (b *localChartBuilder) Build(ctx context.Context, ref ChartReference, p str return nil, err } - // Load the chart metadata from the LocalChartReference to ensure it points + // Load the chart metadata from the LocalReference to ensure it points // to a chart curMeta, err := LoadChartMetadata(localRef.Path) if err != nil { return nil, err } - result := &ChartBuild{} + result := &Build{} result.Name = curMeta.Name // Set build specific metadata if instructed @@ -101,7 +102,7 @@ func (b *localChartBuilder) Build(ctx context.Context, ref ChartReference, p str // Merge chart values, if instructed var mergedValues map[string]interface{} if len(opts.GetValueFiles()) > 0 { - if mergedValues, err = mergeFileValues(localRef.BaseDir, opts.ValueFiles); err != nil { + if mergedValues, err = mergeFileValues(localRef.WorkDir, opts.ValueFiles); err != nil { return nil, fmt.Errorf("failed to merge value files: %w", err) } } diff --git a/internal/helm/chart_builder_local_test.go b/internal/helm/chart/builder_local_test.go similarity index 96% rename from internal/helm/chart_builder_local_test.go rename to internal/helm/chart/builder_local_test.go index c2f16d69..477d2489 100644 --- a/internal/helm/chart_builder_local_test.go +++ b/internal/helm/chart/builder_local_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package chart import ( "os" @@ -99,16 +99,16 @@ func Test_copyFileToPath(t *testing.T) { }{ { name: "copies input file", - in: "testdata/local-index.yaml", + in: "../testdata/local-index.yaml", }, { name: "invalid input file", - in: "testdata/invalid.tgz", + in: "../testdata/invalid.tgz", wantErr: "failed to open file to copy from", }, { name: "invalid input directory", - in: "testdata/charts", + in: "../testdata/charts", wantErr: "failed to read from source during copy", }, } diff --git a/internal/helm/chart_builder_remote.go b/internal/helm/chart/builder_remote.go similarity index 91% rename from internal/helm/chart_builder_remote.go rename to internal/helm/chart/builder_remote.go index 18ff317d..ce195365 100644 --- a/internal/helm/chart_builder_remote.go +++ b/internal/helm/chart/builder_remote.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package chart import ( "context" @@ -24,28 +24,31 @@ import ( "path/filepath" "github.com/Masterminds/semver/v3" - "github.com/fluxcd/pkg/runtime/transform" - "github.com/fluxcd/source-controller/internal/fs" helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/chartutil" "sigs.k8s.io/yaml" + + "github.com/fluxcd/pkg/runtime/transform" + + "github.com/fluxcd/source-controller/internal/fs" + "github.com/fluxcd/source-controller/internal/helm/repository" ) type remoteChartBuilder struct { - remote *ChartRepository + remote *repository.ChartRepository } -// NewRemoteChartBuilder returns a ChartBuilder capable of building a Helm -// chart with a RemoteChartReference from the given ChartRepository. -func NewRemoteChartBuilder(repository *ChartRepository) ChartBuilder { +// NewRemoteBuilder returns a Builder capable of building a Helm +// chart with a RemoteReference from the given Index. +func NewRemoteBuilder(repository *repository.ChartRepository) Builder { return &remoteChartBuilder{ remote: repository, } } -func (b *remoteChartBuilder) Build(_ context.Context, ref ChartReference, p string, opts BuildOptions) (*ChartBuild, error) { - remoteRef, ok := ref.(RemoteChartReference) +func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) { + remoteRef, ok := ref.(RemoteReference) if !ok { return nil, fmt.Errorf("expected remote chart reference") } @@ -59,13 +62,13 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref ChartReference, p stri } defer b.remote.Unload() - // Get the current version for the RemoteChartReference + // Get the current version for the RemoteReference cv, err := b.remote.Get(remoteRef.Name, remoteRef.Version) if err != nil { return nil, fmt.Errorf("failed to get chart version for remote reference: %w", err) } - result := &ChartBuild{} + result := &Build{} result.Name = cv.Name result.Version = cv.Version // Set build specific metadata if instructed diff --git a/internal/helm/chart_builder_remote_test.go b/internal/helm/chart/builder_remote_test.go similarity index 92% rename from internal/helm/chart_builder_remote_test.go rename to internal/helm/chart/builder_remote_test.go index 260bcbce..b7a2dae2 100644 --- a/internal/helm/chart_builder_remote_test.go +++ b/internal/helm/chart/builder_remote_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package chart import ( "testing" @@ -104,9 +104,9 @@ func Test_pathIsDir(t *testing.T) { p string want bool }{ - {name: "directory", p: "testdata/", want: true}, - {name: "file", p: "testdata/local-index.yaml", want: false}, - {name: "not found error", p: "testdata/does-not-exist.yaml", want: false}, + {name: "directory", p: "../testdata/", want: true}, + {name: "file", p: "../testdata/local-index.yaml", want: false}, + {name: "not found error", p: "../testdata/does-not-exist.yaml", want: false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/helm/chart_builder_test.go b/internal/helm/chart/builder_test.go similarity index 89% rename from internal/helm/chart_builder_test.go rename to internal/helm/chart/builder_test.go index a4252be8..92aec74f 100644 --- a/internal/helm/chart_builder_test.go +++ b/internal/helm/chart/builder_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package chart import ( "encoding/hex" @@ -30,18 +30,18 @@ import ( func TestChartBuildResult_String(t *testing.T) { g := NewWithT(t) - var result *ChartBuild + var result *Build g.Expect(result.String()).To(Equal("")) - result = &ChartBuild{} + result = &Build{} g.Expect(result.String()).To(Equal("")) - result = &ChartBuild{Path: "/foo/"} + result = &Build{Path: "/foo/"} g.Expect(result.String()).To(Equal("/foo/")) } func Test_packageToPath(t *testing.T) { g := NewWithT(t) - chart, err := loader.Load("testdata/charts/helmchart-0.1.0.tgz") + chart, err := loader.Load("../testdata/charts/helmchart-0.1.0.tgz") g.Expect(err).ToNot(HaveOccurred()) g.Expect(chart).ToNot(BeNil()) diff --git a/internal/helm/dependency_manager.go b/internal/helm/chart/dependency_manager.go similarity index 81% rename from internal/helm/dependency_manager.go rename to internal/helm/chart/dependency_manager.go index b8cd7857..2fa1df32 100644 --- a/internal/helm/dependency_manager.go +++ b/internal/helm/chart/dependency_manager.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package chart import ( "context" @@ -31,18 +31,20 @@ import ( "golang.org/x/sync/semaphore" helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" + + "github.com/fluxcd/source-controller/internal/helm/repository" ) -// GetChartRepositoryCallback must return a ChartRepository for the URL, -// or an error describing why it could not be returned. -type GetChartRepositoryCallback func(url string) (*ChartRepository, error) +// GetChartRepositoryCallback must return a repository.ChartRepository for the +// URL, or an error describing why it could not be returned. +type GetChartRepositoryCallback func(url string) (*repository.ChartRepository, error) // DependencyManager manages dependencies for a Helm chart. type DependencyManager struct { - // repositories contains a map of ChartRepository indexed by their + // repositories contains a map of Index indexed by their // normalized URL. It is used as a lookup table for missing // dependencies. - repositories map[string]*ChartRepository + repositories map[string]*repository.ChartRepository // getRepositoryCallback can be set to an on-demand GetChartRepositoryCallback // which returned result is cached to repositories. @@ -56,11 +58,12 @@ type DependencyManager struct { mu sync.Mutex } +// DependencyManagerOption configures an option on a DependencyManager. type DependencyManagerOption interface { applyToDependencyManager(dm *DependencyManager) } -type WithRepositories map[string]*ChartRepository +type WithRepositories map[string]*repository.ChartRepository func (o WithRepositories) applyToDependencyManager(dm *DependencyManager) { dm.repositories = o @@ -98,9 +101,9 @@ func (dm *DependencyManager) Clear() []error { } // Build compiles a set of missing dependencies from chart.Chart, and attempts to -// resolve and build them using the information from ChartReference. +// resolve and build them using the information from Reference. // It returns the number of resolved local and remote dependencies, or an error. -func (dm *DependencyManager) Build(ctx context.Context, ref ChartReference, chart *helmchart.Chart) (int, error) { +func (dm *DependencyManager) Build(ctx context.Context, ref Reference, chart *helmchart.Chart) (int, error) { // Collect dependency metadata var ( deps = chart.Dependencies() @@ -132,9 +135,9 @@ type chartWithLock struct { // build adds the given list of deps to the chart with the configured number of // concurrent workers. If the chart.Chart references a local dependency but no -// LocalChartReference is given, or any dependency could not be added, an error +// LocalReference is given, or any dependency could not be added, an error // is returned. The first error it encounters cancels all other workers. -func (dm *DependencyManager) build(ctx context.Context, ref ChartReference, chart *helmchart.Chart, deps map[string]*helmchart.Dependency) error { +func (dm *DependencyManager) build(ctx context.Context, ref Reference, c *helmchart.Chart, deps map[string]*helmchart.Dependency) error { current := dm.concurrent if current <= 0 { current = 1 @@ -143,7 +146,7 @@ func (dm *DependencyManager) build(ctx context.Context, ref ChartReference, char group, groupCtx := errgroup.WithContext(ctx) group.Go(func() error { sem := semaphore.NewWeighted(current) - chart := &chartWithLock{Chart: chart} + c := &chartWithLock{Chart: c} for name, dep := range deps { name, dep := name, dep if err := sem.Acquire(groupCtx, 1); err != nil { @@ -152,17 +155,17 @@ func (dm *DependencyManager) build(ctx context.Context, ref ChartReference, char group.Go(func() (err error) { defer sem.Release(1) if isLocalDep(dep) { - localRef, ok := ref.(LocalChartReference) + localRef, ok := ref.(LocalReference) if !ok { err = fmt.Errorf("failed to add local dependency '%s': no local chart reference", name) return } - if err = dm.addLocalDependency(localRef, chart, dep); err != nil { + if err = dm.addLocalDependency(localRef, c, dep); err != nil { err = fmt.Errorf("failed to add local dependency '%s': %w", name, err) } return } - if err = dm.addRemoteDependency(chart, dep); err != nil { + if err = dm.addRemoteDependency(c, dep); err != nil { err = fmt.Errorf("failed to add remote dependency '%s': %w", name, err) } return @@ -175,7 +178,7 @@ func (dm *DependencyManager) build(ctx context.Context, ref ChartReference, char // addLocalDependency attempts to resolve and add the given local chart.Dependency // to the chart. -func (dm *DependencyManager) addLocalDependency(ref LocalChartReference, chart *chartWithLock, dep *helmchart.Dependency) error { +func (dm *DependencyManager) addLocalDependency(ref LocalReference, c *chartWithLock, dep *helmchart.Dependency) error { sLocalChartPath, err := dm.secureLocalChartPath(ref, dep) if err != nil { return err @@ -197,7 +200,7 @@ func (dm *DependencyManager) addLocalDependency(ref LocalChartReference, chart * ch, err := loader.Load(sLocalChartPath) if err != nil { return fmt.Errorf("failed to load chart from '%s' (reference '%s'): %w", - strings.TrimPrefix(sLocalChartPath, ref.BaseDir), dep.Repository, err) + strings.TrimPrefix(sLocalChartPath, ref.WorkDir), dep.Repository, err) } ver, err := semver.NewVersion(ch.Metadata.Version) @@ -210,9 +213,9 @@ func (dm *DependencyManager) addLocalDependency(ref LocalChartReference, chart * return err } - chart.mu.Lock() - chart.AddDependency(ch) - chart.mu.Unlock() + c.mu.Lock() + c.AddDependency(ch) + c.mu.Unlock() return nil } @@ -249,19 +252,19 @@ func (dm *DependencyManager) addRemoteDependency(chart *chartWithLock, dep *helm } // resolveRepository first attempts to resolve the url from the repositories, falling back -// to getRepositoryCallback if set. It returns the resolved ChartRepository, or an error. -func (dm *DependencyManager) resolveRepository(url string) (_ *ChartRepository, err error) { +// to getRepositoryCallback if set. It returns the resolved Index, or an error. +func (dm *DependencyManager) resolveRepository(url string) (_ *repository.ChartRepository, err error) { dm.mu.Lock() defer dm.mu.Unlock() - nUrl := NormalizeChartRepositoryURL(url) + nUrl := repository.NormalizeURL(url) if _, ok := dm.repositories[nUrl]; !ok { if dm.getRepositoryCallback == nil { err = fmt.Errorf("no chart repository for URL '%s'", nUrl) return } if dm.repositories == nil { - dm.repositories = map[string]*ChartRepository{} + dm.repositories = map[string]*repository.ChartRepository{} } if dm.repositories[nUrl], err = dm.getRepositoryCallback(nUrl); err != nil { err = fmt.Errorf("failed to get chart repository for URL '%s': %w", nUrl, err) @@ -273,8 +276,8 @@ func (dm *DependencyManager) resolveRepository(url string) (_ *ChartRepository, // secureLocalChartPath returns the secure absolute path of a local dependency. // It does not allow the dependency's path to be outside the scope of -// LocalChartReference.BaseDir. -func (dm *DependencyManager) secureLocalChartPath(ref LocalChartReference, dep *helmchart.Dependency) (string, error) { +// LocalReference.WorkDir. +func (dm *DependencyManager) secureLocalChartPath(ref LocalReference, dep *helmchart.Dependency) (string, error) { localUrl, err := url.Parse(dep.Repository) if err != nil { return "", fmt.Errorf("failed to parse alleged local chart reference: %w", err) @@ -282,11 +285,11 @@ func (dm *DependencyManager) secureLocalChartPath(ref LocalChartReference, dep * if localUrl.Scheme != "" && localUrl.Scheme != "file" { return "", fmt.Errorf("'%s' is not a local chart reference", dep.Repository) } - relPath, err := filepath.Rel(ref.BaseDir, ref.Path) + relPath, err := filepath.Rel(ref.WorkDir, ref.Path) if err != nil { - return "", err + relPath = ref.Path } - return securejoin.SecureJoin(ref.BaseDir, filepath.Join(relPath, localUrl.Host, localUrl.Path)) + return securejoin.SecureJoin(ref.WorkDir, filepath.Join(relPath, localUrl.Host, localUrl.Path)) } // collectMissing returns a map with reqs that are missing from current, diff --git a/internal/helm/dependency_manager_test.go b/internal/helm/chart/dependency_manager_test.go similarity index 84% rename from internal/helm/dependency_manager_test.go rename to internal/helm/chart/dependency_manager_test.go index 388eff1f..825fb3b1 100644 --- a/internal/helm/dependency_manager_test.go +++ b/internal/helm/chart/dependency_manager_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package chart import ( "context" @@ -29,26 +29,9 @@ import ( helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/repo" -) -var ( - // helmPackageFile contains the path to a Helm package in the v2 format - // without any dependencies - helmPackageFile = "testdata/charts/helmchart-0.1.0.tgz" - chartName = "helmchart" - chartVersion = "0.1.0" - chartLocalRepository = "file://../helmchart" - remoteDepFixture = helmchart.Dependency{ - Name: chartName, - Version: chartVersion, - Repository: "https://example.com/charts", - } - // helmPackageV1File contains the path to a Helm package in the v1 format, - // including dependencies in a requirements.yaml file which should be - // loaded - helmPackageV1File = "testdata/charts/helmchartwithdeps-v1-0.3.0.tgz" - chartNameV1 = "helmchartwithdeps-v1" - chartVersionV1 = "0.3.0" + "github.com/fluxcd/source-controller/internal/helm/getter" + "github.com/fluxcd/source-controller/internal/helm/repository" ) func TestDependencyManager_Build(t *testing.T) { @@ -56,7 +39,7 @@ func TestDependencyManager_Build(t *testing.T) { name string baseDir string path string - repositories map[string]*ChartRepository + repositories map[string]*repository.ChartRepository getChartRepositoryCallback GetChartRepositoryCallback want int wantChartFunc func(g *WithT, c *helmchart.Chart) @@ -70,13 +53,13 @@ func TestDependencyManager_Build(t *testing.T) { //}, { name: "build failure returns error", - baseDir: "testdata/charts", + baseDir: "./../testdata/charts", path: "helmchartwithdeps", wantErr: "failed to add remote dependency 'grafana': no chart repository for URL", }, { name: "no dependencies returns zero", - baseDir: "testdata/charts", + baseDir: "./../testdata/charts", path: "helmchart", want: 0, }, @@ -91,7 +74,7 @@ func TestDependencyManager_Build(t *testing.T) { got, err := NewDependencyManager( WithRepositories(tt.repositories), WithRepositoryCallback(tt.getChartRepositoryCallback), - ).Build(context.TODO(), LocalChartReference{BaseDir: tt.baseDir, Path: tt.path}, chart) + ).Build(context.TODO(), LocalReference{WorkDir: tt.baseDir, Path: tt.path}, chart) if tt.wantErr != "" { g.Expect(err).To(HaveOccurred()) @@ -135,7 +118,7 @@ func TestDependencyManager_build(t *testing.T) { g := NewWithT(t) dm := NewDependencyManager() - err := dm.build(context.TODO(), LocalChartReference{}, &helmchart.Chart{}, tt.deps) + err := dm.build(context.TODO(), LocalReference{}, &helmchart.Chart{}, tt.deps) if tt.wantErr != "" { g.Expect(err).To(HaveOccurred()) return @@ -180,7 +163,7 @@ func TestDependencyManager_addLocalDependency(t *testing.T) { Version: chartVersion, Repository: "file://../../../absolutely/invalid", }, - wantErr: "no chart found at 'testdata/charts/absolutely/invalid'", + wantErr: "no chart found at '../testdata/charts/absolutely/invalid'", }, { name: "invalid chart archive", @@ -207,7 +190,7 @@ func TestDependencyManager_addLocalDependency(t *testing.T) { dm := NewDependencyManager() chart := &helmchart.Chart{} - err := dm.addLocalDependency(LocalChartReference{BaseDir: "testdata/charts", Path: "helmchartwithdeps"}, + err := dm.addLocalDependency(LocalReference{WorkDir: "../testdata/charts", Path: "helmchartwithdeps"}, &chartWithLock{Chart: chart}, tt.dep) if tt.wantErr != "" { g.Expect(err).To(HaveOccurred()) @@ -222,23 +205,23 @@ func TestDependencyManager_addLocalDependency(t *testing.T) { func TestDependencyManager_addRemoteDependency(t *testing.T) { g := NewWithT(t) - chartB, err := os.ReadFile("testdata/charts/helmchart-0.1.0.tgz") + chartB, err := os.ReadFile("../testdata/charts/helmchart-0.1.0.tgz") g.Expect(err).ToNot(HaveOccurred()) g.Expect(chartB).ToNot(BeEmpty()) tests := []struct { name string - repositories map[string]*ChartRepository + repositories map[string]*repository.ChartRepository dep *helmchart.Dependency wantFunc func(g *WithT, c *helmchart.Chart) wantErr string }{ { name: "adds remote dependency", - repositories: map[string]*ChartRepository{ + repositories: map[string]*repository.ChartRepository{ "https://example.com/": { - Client: &mockGetter{ - response: chartB, + Client: &getter.MockGetter{ + Response: chartB, }, Index: &repo.IndexFile{ Entries: map[string]repo.ChartVersions{ @@ -266,7 +249,7 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) { }, { name: "resolve repository error", - repositories: map[string]*ChartRepository{}, + repositories: map[string]*repository.ChartRepository{}, dep: &helmchart.Dependency{ Repository: "https://example.com", }, @@ -274,7 +257,7 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) { }, { name: "strategic load error", - repositories: map[string]*ChartRepository{ + repositories: map[string]*repository.ChartRepository{ "https://example.com/": { CachePath: "/invalid/cache/path/foo", RWMutex: &sync.RWMutex{}, @@ -287,7 +270,7 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) { }, { name: "repository get error", - repositories: map[string]*ChartRepository{ + repositories: map[string]*repository.ChartRepository{ "https://example.com/": { Index: &repo.IndexFile{}, RWMutex: &sync.RWMutex{}, @@ -300,7 +283,7 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) { }, { name: "repository version constraint error", - repositories: map[string]*ChartRepository{ + repositories: map[string]*repository.ChartRepository{ "https://example.com/": { Index: &repo.IndexFile{ Entries: map[string]repo.ChartVersions{ @@ -326,7 +309,7 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) { }, { name: "repository chart download error", - repositories: map[string]*ChartRepository{ + repositories: map[string]*repository.ChartRepository{ "https://example.com/": { Index: &repo.IndexFile{ Entries: map[string]repo.ChartVersions{ @@ -352,9 +335,9 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) { }, { name: "chart load error", - repositories: map[string]*ChartRepository{ + repositories: map[string]*repository.ChartRepository{ "https://example.com/": { - Client: &mockGetter{}, + Client: &getter.MockGetter{}, Index: &repo.IndexFile{ Entries: map[string]repo.ChartVersions{ chartName: { @@ -404,40 +387,40 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) { func TestDependencyManager_resolveRepository(t *testing.T) { tests := []struct { name string - repositories map[string]*ChartRepository + repositories map[string]*repository.ChartRepository getChartRepositoryCallback GetChartRepositoryCallback url string - want *ChartRepository - wantRepositories map[string]*ChartRepository + want *repository.ChartRepository + wantRepositories map[string]*repository.ChartRepository wantErr string }{ { name: "resolves from repositories index", url: "https://example.com", - repositories: map[string]*ChartRepository{ + repositories: map[string]*repository.ChartRepository{ "https://example.com/": {URL: "https://example.com"}, }, - want: &ChartRepository{URL: "https://example.com"}, + want: &repository.ChartRepository{URL: "https://example.com"}, }, { name: "resolves from callback", url: "https://example.com", - getChartRepositoryCallback: func(url string) (*ChartRepository, error) { - return &ChartRepository{URL: "https://example.com"}, nil + getChartRepositoryCallback: func(url string) (*repository.ChartRepository, error) { + return &repository.ChartRepository{URL: "https://example.com"}, nil }, - want: &ChartRepository{URL: "https://example.com"}, - wantRepositories: map[string]*ChartRepository{ + want: &repository.ChartRepository{URL: "https://example.com"}, + wantRepositories: map[string]*repository.ChartRepository{ "https://example.com/": {URL: "https://example.com"}, }, }, { name: "error from callback", url: "https://example.com", - getChartRepositoryCallback: func(url string) (*ChartRepository, error) { + getChartRepositoryCallback: func(url string) (*repository.ChartRepository, error) { return nil, errors.New("a very unique error") }, wantErr: "a very unique error", - wantRepositories: map[string]*ChartRepository{}, + wantRepositories: map[string]*repository.ChartRepository{}, }, { name: "error on not found", @@ -518,7 +501,7 @@ func TestDependencyManager_secureLocalChartPath(t *testing.T) { g := NewWithT(t) dm := NewDependencyManager() - got, err := dm.secureLocalChartPath(LocalChartReference{BaseDir: tt.baseDir, Path: tt.path}, tt.dep) + got, err := dm.secureLocalChartPath(LocalReference{WorkDir: tt.baseDir, Path: tt.path}, tt.dep) if tt.wantErr != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) diff --git a/internal/helm/chart.go b/internal/helm/chart/metadata.go similarity index 96% rename from internal/helm/chart.go rename to internal/helm/chart/metadata.go index 4f89cab6..24e45208 100644 --- a/internal/helm/chart.go +++ b/internal/helm/chart/metadata.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package chart import ( "archive/tar" @@ -33,6 +33,8 @@ import ( helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" "sigs.k8s.io/yaml" + + "github.com/fluxcd/source-controller/internal/helm" ) // OverwriteChartDefaultValues overwrites the chart default values file with the given data. @@ -115,8 +117,8 @@ func LoadChartMetadataFromDir(dir string) (*helmchart.Metadata, error) { if stat.IsDir() { return nil, fmt.Errorf("'%s' is a directory", stat.Name()) } - if stat.Size() > MaxChartFileSize { - return nil, fmt.Errorf("size of '%s' exceeds '%d' limit", stat.Name(), MaxChartFileSize) + if stat.Size() > helm.MaxChartFileSize { + return nil, fmt.Errorf("size of '%s' exceeds '%d' limit", stat.Name(), helm.MaxChartFileSize) } } @@ -142,8 +144,8 @@ func LoadChartMetadataFromArchive(archive string) (*helmchart.Metadata, error) { } return nil, err } - if stat.Size() > MaxChartSize { - return nil, fmt.Errorf("size of chart '%s' exceeds '%d' limit", stat.Name(), MaxChartSize) + if stat.Size() > helm.MaxChartSize { + return nil, fmt.Errorf("size of chart '%s' exceeds '%d' limit", stat.Name(), helm.MaxChartSize) } f, err := os.Open(archive) diff --git a/internal/helm/chart_test.go b/internal/helm/chart/metadata_test.go similarity index 85% rename from internal/helm/chart_test.go rename to internal/helm/chart/metadata_test.go index ac7114e8..f2294ff6 100644 --- a/internal/helm/chart_test.go +++ b/internal/helm/chart/metadata_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package chart import ( "testing" @@ -25,6 +25,19 @@ import ( ) var ( + // helmPackageFile contains the path to a Helm package in the v2 format + // without any dependencies + helmPackageFile = "../testdata/charts/helmchart-0.1.0.tgz" + chartName = "helmchart" + chartVersion = "0.1.0" + + // helmPackageV1File contains the path to a Helm package in the v1 format, + // including dependencies in a requirements.yaml file which should be + // loaded + helmPackageV1File = "../testdata/charts/helmchartwithdeps-v1-0.3.0.tgz" + chartNameV1 = "helmchartwithdeps-v1" + chartVersionV1 = "0.3.0" + originalValuesFixture = []byte(`override: original `) chartFilesFixture = []*helmchart.File{ @@ -123,21 +136,21 @@ func TestLoadChartMetadataFromDir(t *testing.T) { }{ { name: "Loads from dir", - dir: "testdata/charts/helmchart", + dir: "../testdata/charts/helmchart", wantName: "helmchart", wantVersion: "0.1.0", }, { name: "Loads from v1 dir including requirements.yaml", - dir: "testdata/charts/helmchartwithdeps-v1", + dir: "../testdata/charts/helmchartwithdeps-v1", wantName: chartNameV1, wantVersion: chartVersionV1, wantDependencyCount: 1, }, { name: "Error if no Chart.yaml", - dir: "testdata/charts/", - wantErr: "testdata/charts/Chart.yaml: no such file or directory", + dir: "../testdata/charts/", + wantErr: "../testdata/charts/Chart.yaml: no such file or directory", }, } for _, tt := range tests { @@ -186,12 +199,12 @@ func TestLoadChartMetadataFromArchive(t *testing.T) { }, { name: "Error on not found", - archive: "testdata/invalid.tgz", + archive: "../testdata/invalid.tgz", wantErr: "no such file or directory", }, { name: "Error if no Chart.yaml", - archive: "testdata/charts/empty.tgz", + archive: "../testdata/charts/empty.tgz", wantErr: "no 'Chart.yaml' found", }, } diff --git a/internal/helm/getter.go b/internal/helm/getter/getter.go similarity index 99% rename from internal/helm/getter.go rename to internal/helm/getter/getter.go index 1ca8b0e9..583bac5f 100644 --- a/internal/helm/getter.go +++ b/internal/helm/getter/getter.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package getter import ( "fmt" diff --git a/internal/helm/getter_test.go b/internal/helm/getter/getter_test.go similarity index 99% rename from internal/helm/getter_test.go rename to internal/helm/getter/getter_test.go index 2c55e7cb..6437e5b3 100644 --- a/internal/helm/getter_test.go +++ b/internal/helm/getter/getter_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package getter import ( "os" diff --git a/internal/helm/getter/mock.go b/internal/helm/getter/mock.go new file mode 100644 index 00000000..91cd2b7b --- /dev/null +++ b/internal/helm/getter/mock.go @@ -0,0 +1,41 @@ +/* +Copyright 2021 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package getter + +import ( + "bytes" + + "helm.sh/helm/v3/pkg/getter" +) + +// MockGetter can be used as a simple mocking getter.Getter implementation. +type MockGetter struct { + Response []byte + + requestedURL string +} + +func (g *MockGetter) Get(u string, _ ...getter.Option) (*bytes.Buffer, error) { + g.requestedURL = u + r := g.Response + return bytes.NewBuffer(r), nil +} + +// LastGet returns the last requested URL for Get. +func (g *MockGetter) LastGet() string { + return g.requestedURL +} diff --git a/internal/helm/repository.go b/internal/helm/repository/chart_repository.go similarity index 98% rename from internal/helm/repository.go rename to internal/helm/repository/chart_repository.go index eb9e668a..638355f8 100644 --- a/internal/helm/repository.go +++ b/internal/helm/repository/chart_repository.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package repository import ( "bytes" @@ -36,6 +36,8 @@ import ( "sigs.k8s.io/yaml" "github.com/fluxcd/pkg/version" + + "github.com/fluxcd/source-controller/internal/helm" ) var ErrNoChartIndex = errors.New("no chart index") @@ -241,8 +243,8 @@ func (r *ChartRepository) LoadFromFile(path string) error { } return err } - if stat.Size() > MaxIndexSize { - return fmt.Errorf("size of index '%s' exceeds '%d' limit", stat.Name(), MaxIndexSize) + if stat.Size() > helm.MaxIndexSize { + return fmt.Errorf("size of index '%s' exceeds '%d' limit", stat.Name(), helm.MaxIndexSize) } b, err := os.ReadFile(path) if err != nil { @@ -350,7 +352,7 @@ func (r *ChartRepository) HasCacheFile() bool { } // Unload can be used to signal the Go garbage collector the Index can -// be freed from memory if the ChartRepository object is expected to +// be freed from memory if the Index object is expected to // continue to exist in the stack for some time. func (r *ChartRepository) Unload() { if r == nil { diff --git a/internal/helm/repository_test.go b/internal/helm/repository/chart_repository_test.go similarity index 93% rename from internal/helm/repository_test.go rename to internal/helm/repository/chart_repository_test.go index 9c124b79..b6f191f3 100644 --- a/internal/helm/repository_test.go +++ b/internal/helm/repository/chart_repository_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package repository import ( "bytes" @@ -27,39 +27,29 @@ import ( . "github.com/onsi/gomega" "helm.sh/helm/v3/pkg/chart" - "helm.sh/helm/v3/pkg/getter" + helmgetter "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/repo" + + "github.com/fluxcd/source-controller/internal/helm/getter" ) var now = time.Now() const ( - testFile = "testdata/local-index.yaml" - chartmuseumTestFile = "testdata/chartmuseum-index.yaml" - unorderedTestFile = "testdata/local-index-unordered.yaml" + testFile = "../testdata/local-index.yaml" + chartmuseumTestFile = "../testdata/chartmuseum-index.yaml" + unorderedTestFile = "../testdata/local-index-unordered.yaml" ) -// mockGetter can be used as a simple mocking getter.Getter implementation. -type mockGetter struct { - requestedURL string - response []byte -} - -func (g *mockGetter) Get(url string, _ ...getter.Option) (*bytes.Buffer, error) { - g.requestedURL = url - r := g.response - return bytes.NewBuffer(r), nil -} - func TestNewChartRepository(t *testing.T) { repositoryURL := "https://example.com" - providers := getter.Providers{ - getter.Provider{ + providers := helmgetter.Providers{ + helmgetter.Provider{ Schemes: []string{"https"}, - New: getter.NewHTTPGetter, + New: helmgetter.NewHTTPGetter, }, } - options := []getter.Option{getter.WithBasicAuth("username", "password")} + options := []helmgetter.Option{helmgetter.WithBasicAuth("username", "password")} t.Run("should construct chart repository", func(t *testing.T) { g := NewWithT(t) @@ -230,7 +220,7 @@ func TestChartRepository_DownloadChart(t *testing.T) { g := NewWithT(t) t.Parallel() - mg := mockGetter{} + mg := getter.MockGetter{} r := &ChartRepository{ URL: tt.url, Client: &mg, @@ -241,7 +231,7 @@ func TestChartRepository_DownloadChart(t *testing.T) { g.Expect(res).To(BeNil()) return } - g.Expect(mg.requestedURL).To(Equal(tt.wantURL)) + g.Expect(mg.LastGet()).To(Equal(tt.wantURL)) g.Expect(res).ToNot(BeNil()) g.Expect(err).ToNot(HaveOccurred()) }) @@ -254,7 +244,7 @@ func TestChartRepository_DownloadIndex(t *testing.T) { b, err := os.ReadFile(chartmuseumTestFile) g.Expect(err).ToNot(HaveOccurred()) - mg := mockGetter{response: b} + mg := getter.MockGetter{Response: b} r := &ChartRepository{ URL: "https://example.com", Client: &mg, @@ -263,7 +253,7 @@ func TestChartRepository_DownloadIndex(t *testing.T) { buf := bytes.NewBuffer([]byte{}) g.Expect(r.DownloadIndex(buf)).To(Succeed()) g.Expect(buf.Bytes()).To(Equal(b)) - g.Expect(mg.requestedURL).To(Equal(r.URL + "/index.yaml")) + g.Expect(mg.LastGet()).To(Equal(r.URL + "/index.yaml")) g.Expect(err).To(BeNil()) } @@ -384,8 +374,8 @@ func TestChartRepository_LoadIndexFromFile(t *testing.T) { func TestChartRepository_CacheIndex(t *testing.T) { g := NewWithT(t) - mg := mockGetter{response: []byte("foo")} - expectSum := fmt.Sprintf("%x", sha256.Sum256(mg.response)) + mg := getter.MockGetter{Response: []byte("foo")} + expectSum := fmt.Sprintf("%x", sha256.Sum256(mg.Response)) r := newChartRepository() r.URL = "https://example.com" @@ -399,7 +389,7 @@ func TestChartRepository_CacheIndex(t *testing.T) { g.Expect(r.CachePath).To(BeARegularFile()) b, _ := os.ReadFile(r.CachePath) - g.Expect(b).To(Equal(mg.response)) + g.Expect(b).To(Equal(mg.Response)) g.Expect(sum).To(BeEquivalentTo(expectSum)) } diff --git a/internal/helm/utils.go b/internal/helm/repository/utils.go similarity index 77% rename from internal/helm/utils.go rename to internal/helm/repository/utils.go index ff2221c6..b02b1378 100644 --- a/internal/helm/utils.go +++ b/internal/helm/repository/utils.go @@ -1,5 +1,5 @@ /* -Copyright 2020 The Flux authors +Copyright 2021 The Flux authors Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -14,12 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -package helm +package repository import "strings" -// NormalizeChartRepositoryURL ensures repository urls are normalized -func NormalizeChartRepositoryURL(url string) string { +// NormalizeURL normalizes a ChartRepository URL by ensuring it ends with a +// single "/". +func NormalizeURL(url string) string { if url != "" { return strings.TrimRight(url, "/") + "/" } diff --git a/internal/helm/repository/utils_test.go b/internal/helm/repository/utils_test.go new file mode 100644 index 00000000..fe4cf80e --- /dev/null +++ b/internal/helm/repository/utils_test.go @@ -0,0 +1,44 @@ +package repository + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestNormalizeURL(t *testing.T) { + tests := []struct { + name string + url string + want string + }{ + { + name: "with slash", + url: "http://example.com/", + want: "http://example.com/", + }, + { + name: "without slash", + url: "http://example.com", + want: "http://example.com/", + }, + { + name: "double slash", + url: "http://example.com//", + want: "http://example.com/", + }, + { + name: "empty", + url: "", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got := NormalizeURL(tt.url) + g.Expect(got).To(Equal(tt.want)) + }) + } +} diff --git a/internal/helm/utils_test.go b/internal/helm/utils_test.go deleted file mode 100644 index 62a9e92c..00000000 --- a/internal/helm/utils_test.go +++ /dev/null @@ -1,60 +0,0 @@ -/* -Copyright 2021 The Flux authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package helm - -import ( - "testing" - - . "github.com/onsi/gomega" -) - -func TestNormalizeChartRepositoryURL(t *testing.T) { - tests := []struct { - name string - url string - want string - }{ - { - name: "with slash", - url: "http://example.com/", - want: "http://example.com/", - }, - { - name: "without slash", - url: "http://example.com", - want: "http://example.com/", - }, - { - name: "double slash", - url: "http://example.com//", - want: "http://example.com/", - }, - { - name: "empty", - url: "", - want: "", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - got := NormalizeChartRepositoryURL(tt.url) - g.Expect(got).To(Equal(tt.want)) - }) - } -}