From 49036171da29acaf9c3660a2440183e4f5b06f1a Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Thu, 14 Apr 2022 16:07:10 +0200 Subject: [PATCH 1/7] wip: OCI Helm support Signed-off-by: Max Jonas Werner --- api/v1beta1/helmrepository_types.go | 6 + api/v1beta2/helmrepository_types.go | 9 + ...ce.toolkit.fluxcd.io_helmrepositories.yaml | 10 ++ controllers/helmrepository_controller.go | 6 +- controllers/helmrepository_controller_oci.go | 163 ++++++++++++++++++ controllers/helmrepository_predicate.go | 42 +++++ main.go | 19 ++ 7 files changed, 254 insertions(+), 1 deletion(-) create mode 100644 controllers/helmrepository_controller_oci.go create mode 100644 controllers/helmrepository_predicate.go diff --git a/api/v1beta1/helmrepository_types.go b/api/v1beta1/helmrepository_types.go index 62b0e9a6d..5cbfe573f 100644 --- a/api/v1beta1/helmrepository_types.go +++ b/api/v1beta1/helmrepository_types.go @@ -72,6 +72,12 @@ type HelmRepositorySpec struct { // AccessFrom defines an Access Control List for allowing cross-namespace references to this object. // +optional AccessFrom *acl.AccessFrom `json:"accessFrom,omitempty"` + + // Type of the HelmRepository. + // When this field is set to "OCI", the URL field value must be prefixed with "oci://". + // +kubebuilder:default:="Default" + // +optional + Type string `json:"type,omitempty"` } // HelmRepositoryStatus defines the observed state of the HelmRepository. diff --git a/api/v1beta2/helmrepository_types.go b/api/v1beta2/helmrepository_types.go index 1601885c5..1c001a9b5 100644 --- a/api/v1beta2/helmrepository_types.go +++ b/api/v1beta2/helmrepository_types.go @@ -31,6 +31,9 @@ const ( // HelmRepositoryURLIndexKey is the key used for indexing HelmRepository // objects by their HelmRepositorySpec.URL. HelmRepositoryURLIndexKey = ".metadata.helmRepositoryURL" + + HelmRepositoryTypeDefault = "Default" + HelmRepositoryTypeOCI = "OCI" ) // HelmRepositorySpec specifies the required configuration to produce an @@ -78,6 +81,12 @@ type HelmRepositorySpec struct { // NOTE: Not implemented, provisional as of https://github.com/fluxcd/flux2/pull/2092 // +optional AccessFrom *acl.AccessFrom `json:"accessFrom,omitempty"` + + // Type of the HelmRepository. + // When this field is set to "OCI", the URL field value must be prefixed with "oci://". + // +kubebuilder:default:="Default" + // +optional + Type string `json:"type,omitempty"` } // HelmRepositoryStatus records the observed state of the HelmRepository. diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml index a2308eef6..6e0825221 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml @@ -109,6 +109,11 @@ spec: default: 60s description: The timeout of index downloading, defaults to 60s. type: string + type: + default: Default + description: Type of the HelmRepository. When this field is set to "OCI", + the URL field value must be prefixed with "oci://". + type: string url: description: The Helm repository URL, a valid URL contains at least a protocol and host. @@ -330,6 +335,11 @@ spec: default: 60s description: Timeout of the index fetch operation, defaults to 60s. type: string + type: + default: Default + description: Type of the HelmRepository. When this field is set to "OCI", + the URL field value must be prefixed with "oci://". + type: string url: description: URL of the Helm repository, a valid URL contains at least a protocol and host. diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 9b9db4968..45627cc2a 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -123,7 +123,11 @@ func (r *HelmRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error { func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmRepositoryReconcilerOptions) error { return ctrl.NewControllerManagedBy(mgr). For(&sourcev1.HelmRepository{}). - WithEventFilter(predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{})). + WithEventFilter( + predicate.And( + DefaultHelmRepositoryPredicate{}, + predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{})), + ). WithOptions(controller.Options{ MaxConcurrentReconciles: opts.MaxConcurrentReconciles, RateLimiter: opts.RateLimiter, diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go new file mode 100644 index 000000000..6ea16b085 --- /dev/null +++ b/controllers/helmrepository_controller_oci.go @@ -0,0 +1,163 @@ +package controllers + +import ( + "context" + "time" + + "github.com/fluxcd/pkg/runtime/conditions" + helper "github.com/fluxcd/pkg/runtime/controller" + "github.com/fluxcd/pkg/runtime/patch" + "github.com/fluxcd/pkg/runtime/predicates" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" + "github.com/fluxcd/source-controller/internal/helm/repository" + sreconcile "github.com/fluxcd/source-controller/internal/reconcile" + "github.com/fluxcd/source-controller/internal/reconcile/summarize" + helmgetter "helm.sh/helm/v3/pkg/getter" + kuberecorder "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +type HelmRepositoryOCI struct { + client.Client + kuberecorder.EventRecorder + helper.Metrics + Getters helmgetter.Providers + // Storage *Storage + ControllerName string +} + +func (r *HelmRepositoryOCI) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmRepositoryReconcilerOptions) error { + return ctrl.NewControllerManagedBy(mgr). + For(&sourcev1.HelmRepository{}). + WithEventFilter( + predicate.And( + OCIHelmRepositoryPredicate{}, + predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{})), + ). + WithOptions(controller.Options{ + MaxConcurrentReconciles: opts.MaxConcurrentReconciles, + RateLimiter: opts.RateLimiter, + }). + Complete(r) +} + +func (r *HelmRepositoryOCI) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, retErr error) { + start := time.Now() + log := ctrl.LoggerFrom(ctx) + + // Fetch the HelmRepository + obj := &sourcev1.HelmRepository{} + if err := r.Get(ctx, req.NamespacedName, obj); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + // Record suspended status metric + r.RecordSuspend(ctx, obj, obj.Spec.Suspend) + + // Return early if the object is suspended + if obj.Spec.Suspend { + log.Info("reconciliation is suspended for this object") + return ctrl.Result{}, nil + } + + // Initialize the patch helper with the current version of the object. + patchHelper, err := patch.NewHelper(obj, r.Client) + if err != nil { + return ctrl.Result{}, err + } + + // recResult stores the abstracted reconcile result. + var recResult sreconcile.Result + + // Always attempt to patch the object after each reconciliation. + // NOTE: The final runtime result and error are set in this block. + defer func() { + summarizeHelper := summarize.NewHelper(r.EventRecorder, patchHelper) + summarizeOpts := []summarize.Option{ + summarize.WithConditions(helmRepositoryReadyCondition), + summarize.WithReconcileResult(recResult), + summarize.WithReconcileError(retErr), + summarize.WithIgnoreNotFound(), + summarize.WithProcessors( + summarize.RecordContextualError, + summarize.RecordReconcileReq, + ), + summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{RequeueAfter: obj.GetRequeueAfter()}), + summarize.WithPatchFieldOwner(r.ControllerName), + } + result, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...) + + // Always record readiness and duration metrics + r.Metrics.RecordReadiness(ctx, obj) + r.Metrics.RecordDuration(ctx, obj, start) + }() + + // Add finalizer first if not exist to avoid the race condition + // between init and delete + if !controllerutil.ContainsFinalizer(obj, sourcev1.SourceFinalizer) { + controllerutil.AddFinalizer(obj, sourcev1.SourceFinalizer) + recResult = sreconcile.ResultRequeue + return + } + + // Examine if the object is under deletion + if !obj.ObjectMeta.DeletionTimestamp.IsZero() { + recResult, retErr = r.reconcileDelete(ctx, obj) + return + } + + // Reconcile actual object + reconcilers := []helmRepositoryReconcileFunc{ + r.reconcileSource, + } + recResult, retErr = r.reconcile(ctx, obj, reconcilers) + return +} + +// reconcileDelete handles the deletion of the object. +// Removing the finalizer from the object if successful. +func (r *HelmRepositoryOCI) reconcileDelete(ctx context.Context, obj *sourcev1.HelmRepository) (sreconcile.Result, error) { + // Remove our finalizer from the list + controllerutil.RemoveFinalizer(obj, sourcev1.SourceFinalizer) + + // Stop reconciliation as the object is being deleted + return sreconcile.ResultEmpty, nil +} + +func (r *HelmRepositoryOCI) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) { +} + +func (r *HelmRepositoryOCI) reconcile(ctx context.Context, obj *sourcev1.HelmRepository, reconcilers []helmRepositoryReconcileFunc) (sreconcile.Result, error) { + // Mark as reconciling if generation differs. + if obj.Generation != obj.Status.ObservedGeneration { + conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation) + } + + var chartRepo repository.ChartRepository + + // Run the sub-reconcilers and build the result of reconciliation. + var res sreconcile.Result + var resErr error + for _, rec := range reconcilers { + recResult, err := rec(ctx, obj, &artifact, &chartRepo) + // Exit immediately on ResultRequeue. + if recResult == sreconcile.ResultRequeue { + return sreconcile.ResultRequeue, nil + } + // If an error is received, prioritize the returned results because an + // error also means immediate requeue. + if err != nil { + resErr = err + res = recResult + break + } + // Prioritize requeue request in the result for successful results. + res = sreconcile.LowestRequeuingResult(res, recResult) + } + + return res, resErr +} diff --git a/controllers/helmrepository_predicate.go b/controllers/helmrepository_predicate.go new file mode 100644 index 000000000..86694635b --- /dev/null +++ b/controllers/helmrepository_predicate.go @@ -0,0 +1,42 @@ +package controllers + +import ( + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" +) + +type OCIHelmRepositoryPredicate struct { + predicate.Funcs +} + +func (OCIHelmRepositoryPredicate) Update(e event.UpdateEvent) bool { + if e.ObjectNew == nil { + return false + } + + newHR, ok := e.ObjectNew.(*sourcev1.HelmRepository) + if !ok { + return false + } + + return newHR.Spec.Type == sourcev1.HelmRepositoryTypeOCI +} + +type DefaultHelmRepositoryPredicate struct { + predicate.Funcs +} + +func (DefaultHelmRepositoryPredicate) Update(e event.UpdateEvent) bool { + if e.ObjectNew == nil { + return false + } + + newHR, ok := e.ObjectNew.(*sourcev1.HelmRepository) + if !ok { + return false + } + + return newHR.Spec.Type == sourcev1.HelmRepositoryTypeDefault +} diff --git a/main.go b/main.go index 7b003f461..b9245bd71 100644 --- a/main.go +++ b/main.go @@ -62,6 +62,10 @@ var ( Schemes: []string{"http", "https"}, New: getter.NewHTTPGetter, }, + getter.Provider{ + Schemes: []string{"oci"}, + New: getter.NewOCIGetter, + }, } ) @@ -228,6 +232,21 @@ func main() { os.Exit(1) } + if err = (&controllers.HelmRepositoryOCI{ + Client: mgr.GetClient(), + EventRecorder: eventRecorder, + Metrics: metricsH, + // Storage: storage, + Getters: getters, + ControllerName: controllerName, + }).SetupWithManagerAndOptions(mgr, controllers.HelmRepositoryReconcilerOptions{ + MaxConcurrentReconciles: concurrent, + RateLimiter: helper.GetRateLimiter(rateLimiterOptions), + }); err != nil { + setupLog.Error(err, "unable to create controller", "controller", sourcev1.HelmRepositoryKind) + os.Exit(1) + } + var c *cache.Cache var ttl time.Duration if helmCacheMaxSize > 0 { From 5b01a92e603ebd76477bfe2c5324d096310702d4 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Thu, 14 Apr 2022 18:06:59 +0200 Subject: [PATCH 2/7] Add registryTestServer in the test suite and OCI HelmRepository test case Signed-off-by: Soule BA --- controllers/helmrepository_controller_oci.go | 139 +++++++++++++++++- .../helmrepository_controller_oci_test.go | 135 +++++++++++++++++ controllers/suite_test.go | 104 +++++++++++++ go.mod | 10 ++ go.sum | 1 + 5 files changed, 385 insertions(+), 4 deletions(-) create mode 100644 controllers/helmrepository_controller_oci_test.go diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 6ea16b085..6ebf3e429 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -2,17 +2,24 @@ package controllers import ( "context" + "fmt" + "net/url" "time" + "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" helper "github.com/fluxcd/pkg/runtime/controller" "github.com/fluxcd/pkg/runtime/patch" "github.com/fluxcd/pkg/runtime/predicates" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" + serror "github.com/fluxcd/source-controller/internal/error" "github.com/fluxcd/source-controller/internal/helm/repository" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" helmgetter "helm.sh/helm/v3/pkg/getter" + "helm.sh/helm/v3/pkg/registry" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" kuberecorder "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -21,6 +28,34 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" ) +var helmRepositoryOCIReadyCondition = summarize.Conditions{ + Target: meta.ReadyCondition, + Owned: []string{ + sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, + sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, + meta.ReadyCondition, + meta.ReconcilingCondition, + meta.StalledCondition, + }, + Summarize: []string{ + sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, + sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, + meta.StalledCondition, + meta.ReconcilingCondition, + }, + NegativePolarity: []string{ + sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, + sourcev1.ArtifactOutdatedCondition, + meta.StalledCondition, + meta.ReconcilingCondition, + }, +} + type HelmRepositoryOCI struct { client.Client kuberecorder.EventRecorder @@ -30,6 +65,12 @@ type HelmRepositoryOCI struct { ControllerName string } +// helmRepositoryReconcileOCIFunc is the function type for all the +// v1beta2.HelmRepository (sub)reconcile functions for OCI type. The type implementations +// are grouped and executed serially to perform the complete reconcile of the +// object. +type helmRepositoryReconcileOCIFunc func(ctx context.Context, obj *sourcev1.HelmRepository, repo *repository.ChartRepository) (sreconcile.Result, error) + func (r *HelmRepositoryOCI) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmRepositoryReconcilerOptions) error { return ctrl.NewControllerManagedBy(mgr). For(&sourcev1.HelmRepository{}). @@ -111,7 +152,7 @@ func (r *HelmRepositoryOCI) Reconcile(ctx context.Context, req ctrl.Request) (re } // Reconcile actual object - reconcilers := []helmRepositoryReconcileFunc{ + reconcilers := []helmRepositoryReconcileOCIFunc{ r.reconcileSource, } recResult, retErr = r.reconcile(ctx, obj, reconcilers) @@ -128,10 +169,64 @@ func (r *HelmRepositoryOCI) reconcileDelete(ctx context.Context, obj *sourcev1.H return sreconcile.ResultEmpty, nil } -func (r *HelmRepositoryOCI) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) { +func (r *HelmRepositoryOCI) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository, chartRepo *repository.ChartRepository) (sreconcile.Result, error) { + var logOpts []registry.LoginOption + // Configure any authentication related options + if obj.Spec.SecretRef != nil { + // Attempt to retrieve secret + name := types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: obj.Spec.SecretRef.Name, + } + var secret corev1.Secret + if err := r.Client.Get(ctx, name, &secret); err != nil { + e := &serror.Event{ + Err: fmt.Errorf("failed to get secret '%s': %w", name.String(), err), + Reason: sourcev1.AuthenticationFailedReason, + } + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + } + + // Construct actual options + logOpt, err := r.loginOptionFromSecret(secret) + if err != nil { + e := &serror.Event{ + Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), + Reason: sourcev1.AuthenticationFailedReason, + } + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + // Return err as the content of the secret may change. + return sreconcile.ResultEmpty, e + } + + logOpts = append(logOpts, logOpt) + } + + err := r.Validate(obj.Spec.URL, logOpts...) + if err != nil { + switch err.(type) { + case *url.Error: + e := &serror.Stalling{ + Err: fmt.Errorf("invalid Helm repository URL: %w", err), + Reason: sourcev1.URLInvalidReason, + } + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + default: + e := &serror.Stalling{ + Err: fmt.Errorf("failed to validate Helm repository: %w", err), + Reason: meta.FailedReason, + } + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + } + } + + return sreconcile.ResultSuccess, nil } -func (r *HelmRepositoryOCI) reconcile(ctx context.Context, obj *sourcev1.HelmRepository, reconcilers []helmRepositoryReconcileFunc) (sreconcile.Result, error) { +func (r *HelmRepositoryOCI) reconcile(ctx context.Context, obj *sourcev1.HelmRepository, reconcilers []helmRepositoryReconcileOCIFunc) (sreconcile.Result, error) { // Mark as reconciling if generation differs. if obj.Generation != obj.Status.ObservedGeneration { conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation) @@ -143,7 +238,7 @@ func (r *HelmRepositoryOCI) reconcile(ctx context.Context, obj *sourcev1.HelmRep var res sreconcile.Result var resErr error for _, rec := range reconcilers { - recResult, err := rec(ctx, obj, &artifact, &chartRepo) + recResult, err := rec(ctx, obj, &chartRepo) // Exit immediately on ResultRequeue. if recResult == sreconcile.ResultRequeue { return sreconcile.ResultRequeue, nil @@ -161,3 +256,39 @@ func (r *HelmRepositoryOCI) reconcile(ctx context.Context, obj *sourcev1.HelmRep return res, resErr } + +// Validate the HelmRepository object by checking the url and trying to connect to the repository +// using the provided credentials. +func (r *HelmRepositoryOCI) Validate(u string, loginOpts ...registry.LoginOption) error { + target, err := url.Parse(u) + if err != nil { + return err + } + if target.Scheme != registry.OCIScheme { + return fmt.Errorf("wrong scheme type: %s", target.Scheme) + } + + registryClient, err := registry.NewClient() + if err != nil { + return err + } + + err = registryClient.Login(target.Host+target.Path, loginOpts...) + if err != nil { + return fmt.Errorf("failed to login to: %s", target.Host+target.Path) + } + + return nil + +} + +func (r *HelmRepositoryOCI) loginOptionFromSecret(secret corev1.Secret) (registry.LoginOption, error) { + username, password := string(secret.Data["username"]), string(secret.Data["password"]) + switch { + case username == "" && password == "": + return nil, nil + case username == "" || password == "": + return nil, fmt.Errorf("invalid '%s' secret data: required fields 'username' and 'password'", secret.Name) + } + return registry.LoginOptBasicAuth(username, password), nil +} diff --git a/controllers/helmrepository_controller_oci_test.go b/controllers/helmrepository_controller_oci_test.go new file mode 100644 index 000000000..6875bda49 --- /dev/null +++ b/controllers/helmrepository_controller_oci_test.go @@ -0,0 +1,135 @@ +/* +Copyright 2022 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 controllers + +import ( + "testing" + + "github.com/darkowlzz/controller-check/status" + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" + "github.com/fluxcd/pkg/runtime/patch" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" + . "github.com/onsi/gomega" + "helm.sh/helm/v3/pkg/registry" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { + g := NewWithT(t) + + // Login to the registry + err := testRegistryserver.RegistryClient.Login(testRegistryserver.DockerRegistryHost, + registry.LoginOptBasicAuth(testUsername, testPassword), + registry.LoginOptInsecure(true)) + g.Expect(err).NotTo(HaveOccurred()) + + ns, err := testEnv.CreateNamespace(ctx, "helmrepository-oci-reconcile-test") + g.Expect(err).ToNot(HaveOccurred()) + defer func() { g.Expect(testEnv.Delete(ctx, ns)).To(Succeed()) }() + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-", + Namespace: ns.Name, + }, + Data: map[string][]byte{ + "username": []byte(testUsername), + "password": []byte(testPassword), + }, + } + + g.Expect(testEnv.CreateAndWait(ctx, secret)).To(Succeed()) + + obj := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-oci-reconcile-", + Namespace: ns.Name, + }, + Spec: sourcev1.HelmRepositorySpec{ + Interval: metav1.Duration{Duration: interval}, + URL: testServer.URL(), + SecretRef: &meta.LocalObjectReference{ + Name: secret.Name, + }, + Type: sourcev1.HelmRepositoryTypeOCI, + }, + } + g.Expect(testEnv.Create(ctx, obj)).To(Succeed()) + + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + // Wait for finalizer to be set + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + return len(obj.Finalizers) > 0 + }, timeout).Should(BeTrue()) + + // Wait for HelmRepository to be Ready + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + if !conditions.IsReady(obj) { + return false + } + readyCondition := conditions.Get(obj, meta.ReadyCondition) + return obj.Generation == readyCondition.ObservedGeneration && + obj.Generation == obj.Status.ObservedGeneration + }, timeout).Should(BeTrue()) + + // Check if the object status is valid. + condns := &status.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} + checker := status.NewChecker(testEnv.Client, testEnv.GetScheme(), condns) + checker.CheckErr(ctx, obj) + + // kstatus client conformance check. + u, err := patch.ToUnstructured(obj) + g.Expect(err).ToNot(HaveOccurred()) + res, err := kstatus.Compute(u) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(res.Status).To(Equal(kstatus.CurrentStatus)) + + // Patch the object with reconcile request annotation. + patchHelper, err := patch.NewHelper(obj, testEnv.Client) + g.Expect(err).ToNot(HaveOccurred()) + annotations := map[string]string{ + meta.ReconcileRequestAnnotation: "now", + } + obj.SetAnnotations(annotations) + g.Expect(patchHelper.Patch(ctx, obj)).ToNot(HaveOccurred()) + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + return obj.Status.LastHandledReconcileAt == "now" + }, timeout).Should(BeTrue()) + + g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) + + // Wait for OCIArtifact to be deleted + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return apierrors.IsNotFound(err) + } + return false + }, timeout).Should(BeTrue()) + +} diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 9ca821381..361277962 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -17,14 +17,20 @@ limitations under the License. package controllers import ( + "bytes" + "context" "fmt" + "io" + "io/ioutil" "math/rand" "os" "path/filepath" "testing" "time" + "golang.org/x/crypto/bcrypt" "helm.sh/helm/v3/pkg/getter" + "helm.sh/helm/v3/pkg/registry" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" @@ -33,6 +39,12 @@ import ( "github.com/fluxcd/pkg/runtime/controller" "github.com/fluxcd/pkg/runtime/testenv" "github.com/fluxcd/pkg/testserver" + "github.com/phayes/freeport" + + "github.com/distribution/distribution/v3/configuration" + dockerRegistry "github.com/distribution/distribution/v3/registry" + _ "github.com/distribution/distribution/v3/registry/auth/htpasswd" + _ "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/internal/cache" @@ -75,10 +87,90 @@ var ( tlsCA []byte ) +var ( + testRegistryClient *registry.Client + testRegistryserver *RegistryClientTestServer +) + +var ( + testWorkspaceDir = "registry-test" + testHtpasswdFileBasename = "authtest.htpasswd" + testUsername = "myuser" + testPassword = "mypass" +) + func init() { rand.Seed(time.Now().UnixNano()) } +type RegistryClientTestServer struct { + Out io.Writer + DockerRegistryHost string + WorkspaceDir string + RegistryClient *registry.Client +} + +func SetupServer(server *RegistryClientTestServer) string { + // Create a temporary workspace directory for the registry + server.WorkspaceDir = testWorkspaceDir + os.RemoveAll(server.WorkspaceDir) + err := os.Mkdir(server.WorkspaceDir, 0700) + if err != nil { + panic(fmt.Sprintf("failed to create workspace directory: %s", err)) + } + + var out bytes.Buffer + server.Out = &out + + // init test client + server.RegistryClient, err = registry.NewClient( + registry.ClientOptDebug(true), + registry.ClientOptWriter(server.Out), + ) + if err != nil { + panic(fmt.Sprintf("failed to create registry client: %s", err)) + } + + // create htpasswd file (w BCrypt, which is required) + pwBytes, err := bcrypt.GenerateFromPassword([]byte(testPassword), bcrypt.DefaultCost) + if err != nil { + panic(fmt.Sprintf("failed to generate password: %s", err)) + } + + htpasswdPath := filepath.Join(testWorkspaceDir, testHtpasswdFileBasename) + err = ioutil.WriteFile(htpasswdPath, []byte(fmt.Sprintf("%s:%s\n", testUsername, string(pwBytes))), 0644) + if err != nil { + panic(fmt.Sprintf("failed to create htpasswd file: %s", err)) + } + + // Registry config + config := &configuration.Configuration{} + port, err := freeport.GetFreePort() + if err != nil { + panic(fmt.Sprintf("failed to get free port: %s", err)) + } + + server.DockerRegistryHost = fmt.Sprintf("localhost:%d", port) + config.HTTP.Addr = fmt.Sprintf("127.0.0.1:%d", port) + config.HTTP.DrainTimeout = time.Duration(10) * time.Second + config.Storage = map[string]configuration.Parameters{"inmemory": map[string]interface{}{}} + config.Auth = configuration.Auth{ + "htpasswd": configuration.Parameters{ + "realm": "localhost", + "path": htpasswdPath, + }, + } + dockerRegistry, err := dockerRegistry.NewRegistry(context.Background(), config) + if err != nil { + panic(fmt.Sprintf("failed to create docker registry: %s", err)) + } + + // Start Docker registry + go dockerRegistry.ListenAndServe() + + return server.WorkspaceDir +} + func TestMain(m *testing.M) { initTestTLS() @@ -101,6 +193,14 @@ func TestMain(m *testing.M) { testMetricsH = controller.MustMakeMetrics(testEnv) + testRegistryserver = &RegistryClientTestServer{} + registryWorskpaceDir := SetupServer(testRegistryserver) + + testRegistryClient, err = registry.NewClient(registry.ClientOptWriter(os.Stdout)) + if err != nil { + panic(fmt.Sprintf("Failed to create OCI registry client")) + } + if err := (&GitRepositoryReconciler{ Client: testEnv, EventRecorder: record.NewFakeRecorder(32), @@ -165,6 +265,10 @@ func TestMain(m *testing.M) { panic(fmt.Sprintf("Failed to remove storage server dir: %v", err)) } + if err := os.RemoveAll(registryWorskpaceDir); err != nil { + panic(fmt.Sprintf("Failed to remove registry workspace dir: %v", err)) + } + os.Exit(code) } diff --git a/go.mod b/go.mod index cd2fce114..009a6a29a 100644 --- a/go.mod +++ b/go.mod @@ -73,6 +73,11 @@ replace github.com/opencontainers/image-spec => github.com/opencontainers/image- // Fix CVE-2021-43816 replace github.com/containerd/containerd => github.com/containerd/containerd v1.6.1 +require ( + github.com/distribution/distribution/v3 v3.0.0-20211118083504-a29a3c99a684 + github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2 +) + require ( cloud.google.com/go v0.100.2 // indirect cloud.google.com/go/compute v1.6.0 // indirect @@ -88,6 +93,7 @@ require ( github.com/Microsoft/go-winio v0.5.2 // indirect github.com/PuerkitoBio/purell v1.1.1 // indirect github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect + github.com/Shopify/logrus-bugsnag v0.0.0-20171204204709-577dee27f20d // indirect github.com/acomagu/bufpipe v1.0.3 // indirect github.com/asaskevich/govalidator v0.0.0-20200428143746-21a406dcc535 // indirect github.com/beorn7/perks v1.0.1 // indirect @@ -103,6 +109,7 @@ require ( github.com/docker/docker v20.10.12+incompatible // indirect github.com/docker/docker-credential-helpers v0.6.4 // indirect github.com/docker/go-connections v0.4.0 // indirect + github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect github.com/docker/go-metrics v0.0.1 // indirect github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect github.com/dustin/go-humanize v1.0.0 // indirect @@ -110,6 +117,7 @@ require ( github.com/evanphx/json-patch v5.6.0+incompatible // indirect github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d // indirect github.com/fatih/color v1.13.0 // indirect + github.com/felixge/httpsnoop v1.0.1 // indirect github.com/fluxcd/pkg/apis/acl v0.0.3 // indirect github.com/fsnotify/fsnotify v1.5.1 // indirect github.com/go-errors/errors v1.0.1 // indirect @@ -124,6 +132,7 @@ require ( github.com/golang-jwt/jwt v3.2.1+incompatible // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.2 // indirect + github.com/gomodule/redigo v1.8.2 // indirect github.com/google/btree v1.0.1 // indirect github.com/google/go-cmp v0.5.7 // indirect github.com/google/gofuzz v1.2.0 // indirect @@ -131,6 +140,7 @@ require ( github.com/googleapis/gax-go/v2 v2.3.0 // indirect github.com/googleapis/gnostic v0.5.5 // indirect github.com/googleapis/go-type-adapters v1.0.0 // indirect + github.com/gorilla/handlers v1.5.1 // indirect github.com/gorilla/mux v1.8.0 // indirect github.com/gosuri/uitable v0.0.4 // indirect github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect diff --git a/go.sum b/go.sum index a62206821..758426687 100644 --- a/go.sum +++ b/go.sum @@ -171,6 +171,7 @@ github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+Ce github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= +github.com/bitly/go-simplejson v0.5.0 h1:6IH+V8/tVMab511d5bn4M7EwGXZf9Hj6i2xSwkNEM+Y= github.com/bitly/go-simplejson v0.5.0/go.mod h1:cXHtHw4XUPsvGaxgjIAn8PhEWG9NfngEKAMDJEczWVA= github.com/bketelsen/crypt v0.0.3-0.20200106085610-5cbc8cc4026c/go.mod h1:MKsuJmJgSg28kpZDP6UIiPt0e0Oz0kqKNGyRaWEPv84= github.com/bketelsen/crypt v0.0.4/go.mod h1:aI6NrJ0pMGgvZKL1iVgXLnfIFJtfV+bKCoqOes/6LfM= From b3cf1e39ee0f8271a775ab56fc4e3c7b1146eed8 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Fri, 15 Apr 2022 19:38:06 +0200 Subject: [PATCH 3/7] fix event filters for HelmRepository reconcilers We need to filter all events, not just update. Signed-off-by: Max Jonas Werner --- controllers/helmrepository_controller.go | 5 ++- controllers/helmrepository_controller_oci.go | 2 +- .../helmrepository_controller_oci_test.go | 2 +- controllers/helmrepository_predicate.go | 42 +++++-------------- 4 files changed, 16 insertions(+), 35 deletions(-) diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 45627cc2a..6970cb772 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -125,8 +125,9 @@ func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, For(&sourcev1.HelmRepository{}). WithEventFilter( predicate.And( - DefaultHelmRepositoryPredicate{}, - predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{})), + predicate.NewPredicateFuncs(HelmRepositoryTypeFilter(sourcev1.HelmRepositoryTypeDefault)), + predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}), + ), ). WithOptions(controller.Options{ MaxConcurrentReconciles: opts.MaxConcurrentReconciles, diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 6ebf3e429..184c7172b 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -76,7 +76,7 @@ func (r *HelmRepositoryOCI) SetupWithManagerAndOptions(mgr ctrl.Manager, opts He For(&sourcev1.HelmRepository{}). WithEventFilter( predicate.And( - OCIHelmRepositoryPredicate{}, + predicate.NewPredicateFuncs(HelmRepositoryTypeFilter(sourcev1.HelmRepositoryTypeOCI)), predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{})), ). WithOptions(controller.Options{ diff --git a/controllers/helmrepository_controller_oci_test.go b/controllers/helmrepository_controller_oci_test.go index 6875bda49..d5b4c4d5f 100644 --- a/controllers/helmrepository_controller_oci_test.go +++ b/controllers/helmrepository_controller_oci_test.go @@ -97,7 +97,7 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { // Check if the object status is valid. condns := &status.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} - checker := status.NewChecker(testEnv.Client, testEnv.GetScheme(), condns) + checker := status.NewChecker(testEnv.Client, condns) checker.CheckErr(ctx, obj) // kstatus client conformance check. diff --git a/controllers/helmrepository_predicate.go b/controllers/helmrepository_predicate.go index 86694635b..b8c1c9709 100644 --- a/controllers/helmrepository_predicate.go +++ b/controllers/helmrepository_predicate.go @@ -1,42 +1,22 @@ package controllers import ( - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/client" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" ) -type OCIHelmRepositoryPredicate struct { - predicate.Funcs -} +func HelmRepositoryTypeFilter(typ string) func(client.Object) bool { + return func(o client.Object) bool { + if o == nil { + return false + } -func (OCIHelmRepositoryPredicate) Update(e event.UpdateEvent) bool { - if e.ObjectNew == nil { - return false - } + hr, ok := o.(*sourcev1.HelmRepository) + if !ok { + return false + } - newHR, ok := e.ObjectNew.(*sourcev1.HelmRepository) - if !ok { - return false + return hr.Spec.Type == typ } - - return newHR.Spec.Type == sourcev1.HelmRepositoryTypeOCI -} - -type DefaultHelmRepositoryPredicate struct { - predicate.Funcs -} - -func (DefaultHelmRepositoryPredicate) Update(e event.UpdateEvent) bool { - if e.ObjectNew == nil { - return false - } - - newHR, ok := e.ObjectNew.(*sourcev1.HelmRepository) - if !ok { - return false - } - - return newHR.Spec.Type == sourcev1.HelmRepositoryTypeDefault } From 5cc7f20de75ff4573e420fed44c27768d5bd3d89 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Wed, 20 Apr 2022 13:12:39 +0200 Subject: [PATCH 4/7] Add a new OCI chart repository type that manage tags and charts from an OCI registry. Adapat RemoteBuilder to accept both repository types Signed-off-by: Soule BA --- api/v1beta2/condition_types.go | 4 + controllers/helmchart_controller.go | 79 ++++--- controllers/helmrepository_controller_oci.go | 184 +++++++++------- .../helmrepository_controller_oci_test.go | 14 +- controllers/suite_test.go | 10 + internal/helm/chart/builder_remote.go | 203 ++++++++++++++---- .../helm/repository/oci_chart_repository.go | 189 ++++++++++++++++ main.go | 20 +- 8 files changed, 536 insertions(+), 167 deletions(-) create mode 100644 internal/helm/repository/oci_chart_repository.go diff --git a/api/v1beta2/condition_types.go b/api/v1beta2/condition_types.go index 711469eb8..4bcd3d745 100644 --- a/api/v1beta2/condition_types.go +++ b/api/v1beta2/condition_types.go @@ -39,6 +39,10 @@ const ( // is enabled. SourceVerifiedCondition string = "SourceVerified" + //SourceValidCondition indicates the validity of the Source. + // If True, the Source is valid. If False, it is not valid. + SourceValidCondition string = "SourceValid" + // FetchFailedCondition indicates a transient or persistent fetch failure // of an upstream Source. // If True, observations on the upstream Source revision may be impossible, diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 68085044b..460ceae6f 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -29,6 +29,7 @@ import ( "time" helmgetter "helm.sh/helm/v3/pkg/getter" + "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -116,6 +117,7 @@ type HelmChartReconciler struct { kuberecorder.EventRecorder helper.Metrics + RegistryClient *registry.Client Storage *Storage Getters helmgetter.Providers ControllerName string @@ -378,15 +380,19 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1 // Assert source has an artifact if s.GetArtifact() == nil || !r.Storage.ArtifactExist(*s.GetArtifact()) { - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, "NoSourceArtifact", - "no artifact available for %s source '%s'", obj.Spec.SourceRef.Kind, obj.Spec.SourceRef.Name) - r.eventLogf(ctx, obj, events.EventTypeTrace, "NoSourceArtifact", - "no artifact available for %s source '%s'", obj.Spec.SourceRef.Kind, obj.Spec.SourceRef.Name) - return sreconcile.ResultRequeue, nil + if helmRepo, ok := s.(*sourcev1.HelmRepository); !ok || !registry.IsOCI(helmRepo.Spec.URL) { + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, "NoSourceArtifact", + "no artifact available for %s source '%s'", obj.Spec.SourceRef.Kind, obj.Spec.SourceRef.Name) + r.eventLogf(ctx, obj, events.EventTypeTrace, "NoSourceArtifact", + "no artifact available for %s source '%s'", obj.Spec.SourceRef.Kind, obj.Spec.SourceRef.Name) + return sreconcile.ResultRequeue, nil + } } - // Record current artifact revision as last observed - obj.Status.ObservedSourceArtifactRevision = s.GetArtifact().Revision + if s.GetArtifact() != nil { + // Record current artifact revision as last observed + obj.Status.ObservedSourceArtifactRevision = s.GetArtifact().Revision + } // Defer observation of build result defer func() { @@ -484,10 +490,42 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * } // Initialize the chart repository - chartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts, - repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) { - r.IncCacheEvents(event, obj.Name, obj.Namespace) - })) + var chartRepo chart.Remote + var err error + fmt.Printf("CREATING REPO FROM %s\n", repo.Spec.URL) + if registry.IsOCI(repo.Spec.URL) { + fmt.Printf("CREATING OCI REPO\n") + chartRepo, err = repository.NewOCIChartRepository(repo.Spec.URL, repository.WithOCIGetter(r.Getters), repository.WithOCIRegistryClient(r.RegistryClient)) + } else { + fmt.Printf("CREATING HTTP REPO\n") + var httpChartRepo *repository.ChartRepository + httpChartRepo, err = repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts, + repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) { + r.IncCacheEvents(event, obj.Name, obj.Namespace) + })) + chartRepo = httpChartRepo + defer func() { + if httpChartRepo == nil { + return + } + // Cache the index if it was successfully retrieved + // and the chart was successfully built + if r.Cache != nil && httpChartRepo.Index != nil { + // The cache key have to be safe in multi-tenancy environments, + // as otherwise it could be used as a vector to bypass the helm repository's authentication. + // Using r.Storage.LocalPath(*repo.GetArtifact() is safe as the path is in the format ///. + err := httpChartRepo.CacheIndexInMemory() + if err != nil { + r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.CacheOperationFailedReason, "failed to cache index: %s", err) + } + } + + // Delete the index reference + if httpChartRepo.Index != nil { + httpChartRepo.Unload() + } + }() + } if err != nil { // Any error requires a change in generation, // which we should be informed about by the watcher @@ -532,25 +570,6 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * return sreconcile.ResultEmpty, err } - defer func() { - // Cache the index if it was successfully retrieved - // and the chart was successfully built - if r.Cache != nil && chartRepo.Index != nil { - // The cache key have to be safe in multi-tenancy environments, - // as otherwise it could be used as a vector to bypass the helm repository's authentication. - // Using r.Storage.LocalPath(*repo.GetArtifact() is safe as the path is in the format ///. - err := chartRepo.CacheIndexInMemory() - if err != nil { - r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.CacheOperationFailedReason, "failed to cache index: %s", err) - } - } - - // Delete the index reference - if chartRepo.Index != nil { - chartRepo.Unload() - } - }() - *b = *build return sreconcile.ResultSuccess, nil } diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 184c7172b..45d1f198b 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -13,7 +13,6 @@ import ( "github.com/fluxcd/pkg/runtime/predicates" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" serror "github.com/fluxcd/source-controller/internal/error" - "github.com/fluxcd/source-controller/internal/helm/repository" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" helmgetter "helm.sh/helm/v3/pkg/getter" @@ -31,47 +30,58 @@ import ( var helmRepositoryOCIReadyCondition = summarize.Conditions{ Target: meta.ReadyCondition, Owned: []string{ - sourcev1.StorageOperationFailedCondition, sourcev1.FetchFailedCondition, - sourcev1.ArtifactOutdatedCondition, - sourcev1.ArtifactInStorageCondition, + sourcev1.SourceValidCondition, meta.ReadyCondition, meta.ReconcilingCondition, meta.StalledCondition, }, Summarize: []string{ - sourcev1.StorageOperationFailedCondition, sourcev1.FetchFailedCondition, - sourcev1.ArtifactOutdatedCondition, - sourcev1.ArtifactInStorageCondition, + sourcev1.SourceValidCondition, meta.StalledCondition, meta.ReconcilingCondition, }, NegativePolarity: []string{ - sourcev1.StorageOperationFailedCondition, sourcev1.FetchFailedCondition, - sourcev1.ArtifactOutdatedCondition, meta.StalledCondition, meta.ReconcilingCondition, }, } -type HelmRepositoryOCI struct { +// helmRepositoryOCIFailConditions contains the conditions that represent a +// failure. +var helmRepositoryOCIFailConditions = []string{ + sourcev1.FetchFailedCondition, +} + +// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmrepositories,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmrepositories/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmrepositories/finalizers,verbs=get;create;update;patch;delete +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch + +// HelmRepositoryOCI Reconciler reconciles a v1beta2.HelmRepository object of type OCI. +type HelmRepositoryOCIReconciler struct { client.Client kuberecorder.EventRecorder helper.Metrics Getters helmgetter.Providers // Storage *Storage ControllerName string + RegistryClient *registry.Client } -// helmRepositoryReconcileOCIFunc is the function type for all the +// helmRepositoryOCIReconcileFunc is the function type for all the // v1beta2.HelmRepository (sub)reconcile functions for OCI type. The type implementations // are grouped and executed serially to perform the complete reconcile of the // object. -type helmRepositoryReconcileOCIFunc func(ctx context.Context, obj *sourcev1.HelmRepository, repo *repository.ChartRepository) (sreconcile.Result, error) +type helmRepositoryOCIReconcileFunc func(ctx context.Context, obj *sourcev1.HelmRepository) (sreconcile.Result, error) + +func (r *HelmRepositoryOCIReconciler) SetupWithManager(mgr ctrl.Manager) error { + return r.SetupWithManagerAndOptions(mgr, HelmRepositoryReconcilerOptions{}) +} -func (r *HelmRepositoryOCI) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmRepositoryReconcilerOptions) error { +func (r *HelmRepositoryOCIReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmRepositoryReconcilerOptions) error { return ctrl.NewControllerManagedBy(mgr). For(&sourcev1.HelmRepository{}). WithEventFilter( @@ -86,7 +96,7 @@ func (r *HelmRepositoryOCI) SetupWithManagerAndOptions(mgr ctrl.Manager, opts He Complete(r) } -func (r *HelmRepositoryOCI) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, retErr error) { +func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, retErr error) { start := time.Now() log := ctrl.LoggerFrom(ctx) @@ -119,7 +129,7 @@ func (r *HelmRepositoryOCI) Reconcile(ctx context.Context, req ctrl.Request) (re defer func() { summarizeHelper := summarize.NewHelper(r.EventRecorder, patchHelper) summarizeOpts := []summarize.Option{ - summarize.WithConditions(helmRepositoryReadyCondition), + summarize.WithConditions(helmRepositoryOCIReadyCondition), summarize.WithReconcileResult(recResult), summarize.WithReconcileError(retErr), summarize.WithIgnoreNotFound(), @@ -152,7 +162,7 @@ func (r *HelmRepositoryOCI) Reconcile(ctx context.Context, req ctrl.Request) (re } // Reconcile actual object - reconcilers := []helmRepositoryReconcileOCIFunc{ + reconcilers := []helmRepositoryOCIReconcileFunc{ r.reconcileSource, } recResult, retErr = r.reconcile(ctx, obj, reconcilers) @@ -161,7 +171,7 @@ func (r *HelmRepositoryOCI) Reconcile(ctx context.Context, req ctrl.Request) (re // reconcileDelete handles the deletion of the object. // Removing the finalizer from the object if successful. -func (r *HelmRepositoryOCI) reconcileDelete(ctx context.Context, obj *sourcev1.HelmRepository) (sreconcile.Result, error) { +func (r *HelmRepositoryOCIReconciler) reconcileDelete(ctx context.Context, obj *sourcev1.HelmRepository) (sreconcile.Result, error) { // Remove our finalizer from the list controllerutil.RemoveFinalizer(obj, sourcev1.SourceFinalizer) @@ -169,7 +179,51 @@ func (r *HelmRepositoryOCI) reconcileDelete(ctx context.Context, obj *sourcev1.H return sreconcile.ResultEmpty, nil } -func (r *HelmRepositoryOCI) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository, chartRepo *repository.ChartRepository) (sreconcile.Result, error) { +// notify emits notification related to the reconciliation. +func (r *HelmRepositoryOCIReconciler) notify(oldObj, newObj *sourcev1.HelmRepository, res sreconcile.Result, resErr error) { + // Notify successful recovery from any failure. + if resErr == nil && res == sreconcile.ResultSuccess { + if sreconcile.FailureRecovery(oldObj, newObj, helmRepositoryOCIFailConditions) { + r.Eventf(newObj, corev1.EventTypeNormal, + meta.SucceededReason, "Helm repository %q has been successfully reconciled", newObj.Name) + } + } +} + +func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *sourcev1.HelmRepository, reconcilers []helmRepositoryOCIReconcileFunc) (sreconcile.Result, error) { + oldObj := obj.DeepCopy() + + // Mark as reconciling if generation differs. + if obj.Generation != obj.Status.ObservedGeneration { + conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation) + } + + // Run the sub-reconcilers and build the result of reconciliation. + var res sreconcile.Result + var resErr error + for _, rec := range reconcilers { + recResult, err := rec(ctx, obj) + // Exit immediately on ResultRequeue. + if recResult == sreconcile.ResultRequeue { + return sreconcile.ResultRequeue, nil + } + // If an error is received, prioritize the returned results because an + // error also means immediate requeue. + if err != nil { + resErr = err + res = recResult + break + } + // Prioritize requeue request in the result for successful results. + res = sreconcile.LowestRequeuingResult(res, recResult) + } + + r.notify(oldObj, obj, res, resErr) + + return res, resErr +} + +func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository) (sreconcile.Result, error) { var logOpts []registry.LoginOption // Configure any authentication related options if obj.Spec.SecretRef != nil { @@ -203,86 +257,52 @@ func (r *HelmRepositoryOCI) reconcileSource(ctx context.Context, obj *sourcev1.H logOpts = append(logOpts, logOpt) } - err := r.Validate(obj.Spec.URL, logOpts...) - if err != nil { - switch err.(type) { - case *url.Error: - e := &serror.Stalling{ - Err: fmt.Errorf("invalid Helm repository URL: %w", err), - Reason: sourcev1.URLInvalidReason, - } - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) - return sreconcile.ResultEmpty, e - default: - e := &serror.Stalling{ - Err: fmt.Errorf("failed to validate Helm repository: %w", err), - Reason: meta.FailedReason, - } - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) - return sreconcile.ResultEmpty, e - } + if result, err := r.validateSource(ctx, obj, logOpts...); err != nil || result == sreconcile.ResultEmpty { + return result, err } return sreconcile.ResultSuccess, nil } -func (r *HelmRepositoryOCI) reconcile(ctx context.Context, obj *sourcev1.HelmRepository, reconcilers []helmRepositoryReconcileOCIFunc) (sreconcile.Result, error) { - // Mark as reconciling if generation differs. - if obj.Generation != obj.Status.ObservedGeneration { - conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation) - } - - var chartRepo repository.ChartRepository - - // Run the sub-reconcilers and build the result of reconciliation. - var res sreconcile.Result - var resErr error - for _, rec := range reconcilers { - recResult, err := rec(ctx, obj, &chartRepo) - // Exit immediately on ResultRequeue. - if recResult == sreconcile.ResultRequeue { - return sreconcile.ResultRequeue, nil - } - // If an error is received, prioritize the returned results because an - // error also means immediate requeue. - if err != nil { - resErr = err - res = recResult - break - } - // Prioritize requeue request in the result for successful results. - res = sreconcile.LowestRequeuingResult(res, recResult) - } - - return res, resErr -} - -// Validate the HelmRepository object by checking the url and trying to connect to the repository -// using the provided credentials. -func (r *HelmRepositoryOCI) Validate(u string, loginOpts ...registry.LoginOption) error { - target, err := url.Parse(u) +// validateSource the HelmRepository object by checking the url and connecting to the underlying registry +// with he provided credentials. +func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *sourcev1.HelmRepository, loginOpts ...registry.LoginOption) (sreconcile.Result, error) { + target, err := url.Parse(obj.Spec.URL) if err != nil { - return err - } - if target.Scheme != registry.OCIScheme { - return fmt.Errorf("wrong scheme type: %s", target.Scheme) + e := &serror.Event{ + Err: fmt.Errorf("failed to parse URL '%s': %w", obj.Spec.URL, err), + Reason: "ValidationError", + } + conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e } - registryClient, err := registry.NewClient() - if err != nil { - return err + // Check if the registry is supported + if !registry.IsOCI(obj.Spec.URL) { + e := &serror.Event{ + Err: fmt.Errorf("unsupported registry scheme '%s'", target.Scheme), + Reason: "ValidationError", + } + conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e } - err = registryClient.Login(target.Host+target.Path, loginOpts...) + err = r.RegistryClient.Login(target.Host+target.Path, loginOpts...) if err != nil { - return fmt.Errorf("failed to login to: %s", target.Host+target.Path) + e := &serror.Event{ + Err: fmt.Errorf("failed to login to registry '%s': %w", target.String(), err), + Reason: "ValidationError", + } + conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e } - return nil + conditions.MarkTrue(obj, sourcev1.SourceValidCondition, meta.SucceededReason, "Helm repository %q is valid", obj.Name) + return sreconcile.ResultSuccess, nil } -func (r *HelmRepositoryOCI) loginOptionFromSecret(secret corev1.Secret) (registry.LoginOption, error) { +func (r *HelmRepositoryOCIReconciler) loginOptionFromSecret(secret corev1.Secret) (registry.LoginOption, error) { username, password := string(secret.Data["username"]), string(secret.Data["password"]) switch { case username == "" && password == "": diff --git a/controllers/helmrepository_controller_oci_test.go b/controllers/helmrepository_controller_oci_test.go index d5b4c4d5f..8f63cd4f4 100644 --- a/controllers/helmrepository_controller_oci_test.go +++ b/controllers/helmrepository_controller_oci_test.go @@ -14,6 +14,7 @@ limitations under the License. package controllers import ( + "fmt" "testing" "github.com/darkowlzz/controller-check/status" @@ -22,7 +23,6 @@ import ( "github.com/fluxcd/pkg/runtime/patch" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" . "github.com/onsi/gomega" - "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -34,10 +34,10 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { g := NewWithT(t) // Login to the registry - err := testRegistryserver.RegistryClient.Login(testRegistryserver.DockerRegistryHost, - registry.LoginOptBasicAuth(testUsername, testPassword), - registry.LoginOptInsecure(true)) - g.Expect(err).NotTo(HaveOccurred()) + // err := testRegistryserver.RegistryClient.Login(testRegistryserver.DockerRegistryHost, + // registry.LoginOptBasicAuth(testUsername, testPassword), + // registry.LoginOptInsecure(true)) + // g.Expect(err).NotTo(HaveOccurred()) ns, err := testEnv.CreateNamespace(ctx, "helmrepository-oci-reconcile-test") g.Expect(err).ToNot(HaveOccurred()) @@ -63,7 +63,7 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { }, Spec: sourcev1.HelmRepositorySpec{ Interval: metav1.Duration{Duration: interval}, - URL: testServer.URL(), + URL: fmt.Sprintf("oci://%s", testRegistryserver.DockerRegistryHost), SecretRef: &meta.LocalObjectReference{ Name: secret.Name, }, @@ -124,7 +124,7 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) - // Wait for OCIArtifact to be deleted + // Wait for HelmRepository to be deleted g.Eventually(func() bool { if err := testEnv.Get(ctx, key, obj); err != nil { return apierrors.IsNotFound(err) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 361277962..61cb7374b 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -229,6 +229,16 @@ func TestMain(m *testing.M) { panic(fmt.Sprintf("Failed to start HelmRepositoryReconciler: %v", err)) } + if err = (&HelmRepositoryOCIReconciler{ + Client: testEnv, + EventRecorder: record.NewFakeRecorder(32), + Metrics: testMetricsH, + Getters: testGetters, + RegistryClient: testRegistryClient, + }).SetupWithManager(testEnv); err != nil { + panic(fmt.Sprintf("Failed to start HelmRepositoryOCIReconciler: %v", err)) + } + c := cache.New(5, 1*time.Second) cacheRecorder := cache.MustMakeMetrics() if err := (&HelmChartReconciler{ diff --git a/internal/helm/chart/builder_remote.go b/internal/helm/chart/builder_remote.go index 00b83d71a..df8beb158 100644 --- a/internal/helm/chart/builder_remote.go +++ b/internal/helm/chart/builder_remote.go @@ -17,6 +17,7 @@ limitations under the License. package chart import ( + "bytes" "context" "fmt" "io" @@ -24,24 +25,34 @@ import ( "path/filepath" "github.com/Masterminds/semver/v3" + "github.com/fluxcd/source-controller/internal/helm/repository" helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/repo" "sigs.k8s.io/yaml" "github.com/fluxcd/pkg/runtime/transform" "github.com/fluxcd/source-controller/internal/fs" "github.com/fluxcd/source-controller/internal/helm/chart/secureloader" - "github.com/fluxcd/source-controller/internal/helm/repository" ) +// Remote is a repository.ChartRepository or a repository.OCIChartRegistry. +// It is used to download a chart from a remote repository or registry. +type Remote interface { + // GetChart returns a chart.Chart from the remote repository. + Get(name, version string) (*repo.ChartVersion, error) + // GetChartVersion returns a chart.ChartVersion from the remote repository. + DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer, error) +} + type remoteChartBuilder struct { - remote *repository.ChartRepository + remote Remote } // NewRemoteBuilder returns a Builder capable of building a Helm // chart with a RemoteReference in the given repository.ChartRepository. -func NewRemoteBuilder(repository *repository.ChartRepository) Builder { +func NewRemoteBuilder(repository Remote) Builder { return &remoteChartBuilder{ remote: repository, } @@ -72,14 +83,139 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o return nil, &BuildError{Reason: ErrChartReference, Err: err} } - // Load the repository index if not already present. - if err := b.remote.StrategicallyLoadIndex(); err != nil { + var ( + res *bytes.Buffer + err error + ) + + result := &Build{} + switch b.remote.(type) { + case *repository.ChartRepository: + res, err = b.downloadFromRepository(b.remote.(*repository.ChartRepository), remoteRef, result, opts) + if err != nil { + return nil, &BuildError{Reason: ErrChartPull, Err: err} + } + if res == nil { + return result, nil + } + case *repository.OCIChartRepository: + res, err = b.downloadFromOCIRepository(b.remote.(*repository.OCIChartRepository), remoteRef, result, opts) + if err != nil { + return nil, &BuildError{Reason: ErrChartPull, Err: err} + } + if res == nil { + return result, nil + } + default: + return nil, &BuildError{Reason: ErrChartReference, Err: fmt.Errorf("unsupported remote type %T", b.remote)} + } + + requiresPackaging := len(opts.GetValuesFiles()) != 0 || opts.VersionMetadata != "" + + // Use literal chart copy from remote if no custom values files options are + // set or version metadata isn't set. + if !requiresPackaging { + if err = validatePackageAndWriteToPath(res, p); err != nil { + return nil, &BuildError{Reason: ErrChartPull, Err: err} + } + result.Path = p + return result, nil + } + + // Load the chart and merge chart values + var chart *helmchart.Chart + if chart, err = secureloader.LoadArchive(res); err != nil { + err = fmt.Errorf("failed to load downloaded chart: %w", err) + return result, &BuildError{Reason: ErrChartPackage, Err: err} + } + chart.Metadata.Version = result.Version + + mergedValues, err := mergeChartValues(chart, opts.ValuesFiles) + if err != nil { + err = fmt.Errorf("failed to merge chart values: %w", err) + return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err} + } + // Overwrite default values with merged values, if any + if ok, err = OverwriteChartDefaultValues(chart, mergedValues); ok || err != nil { + if err != nil { + return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err} + } + result.ValuesFiles = opts.GetValuesFiles() + } + + // Package the chart with the custom values + if err = packageToPath(chart, p); err != nil { + return nil, &BuildError{Reason: ErrChartPackage, Err: err} + } + result.Path = p + result.Packaged = true + return result, nil +} + +func (b *remoteChartBuilder) downloadFromOCIRepository(remote *repository.OCIChartRepository, remoteRef RemoteReference, buildResult *Build, opts BuildOptions) (*bytes.Buffer, error) { + // TODO: verify if login is required + cv, err := remote.Get(remoteRef.Name, remoteRef.Version) + if err != nil { + err = fmt.Errorf("failed to get chart version for remote reference: %w", err) + return nil, &BuildError{Reason: ErrChartPull, Err: err} + } + + result := &Build{} + result.Version = cv.Version + result.Name = cv.Name + + // Set build specific metadata if instructed + if opts.VersionMetadata != "" { + ver, err := setBuildMetaData(result.Version, opts.VersionMetadata) + if err != nil { + return nil, &BuildError{Reason: ErrChartMetadataPatch, Err: err} + } + result.Version = ver.String() + } + + requiresPackaging := len(opts.GetValuesFiles()) != 0 || opts.VersionMetadata != "" + + // If all the following is true, we do not need to download and/or build the chart: + // - Chart name from cached chart matches resolved name + // - Chart version from cached chart matches calculated version + // - BuildOptions.Force is False + if opts.CachedChart != "" && !opts.Force { + if curMeta, err := LoadChartMetadataFromArchive(opts.CachedChart); err == nil { + // If the cached metadata is corrupt, we ignore its existence + // and continue the build + if err = curMeta.Validate(); err == nil { + if result.Name == curMeta.Name && result.Version == curMeta.Version { + result.Path = opts.CachedChart + result.ValuesFiles = opts.GetValuesFiles() + result.Packaged = requiresPackaging + *buildResult = *result + return nil, nil + } + } + } + } + + // Download the package for the resolved version + res, err := remote.DownloadChart(cv) + if err != nil { + err = fmt.Errorf("failed to download chart for remote reference: %w", err) + return nil, &BuildError{Reason: ErrChartPull, Err: err} + } + + *buildResult = *result + + return res, nil +} + +func (b *remoteChartBuilder) downloadFromRepository(remote *repository.ChartRepository, remoteRef RemoteReference, buildResult *Build, opts BuildOptions) (*bytes.Buffer, error) { + if err := remote.StrategicallyLoadIndex(); err != nil { err = fmt.Errorf("could not load repository index for remote chart reference: %w", err) return nil, &BuildError{Reason: ErrChartPull, Err: err} } + defer remote.Unload() // Get the current version for the RemoteReference - cv, err := b.remote.Get(remoteRef.Name, remoteRef.Version) + cv, err := remote.Get(remoteRef.Name, remoteRef.Version) if err != nil { err = fmt.Errorf("failed to get chart version for remote reference: %w", err) return nil, &BuildError{Reason: ErrChartReference, Err: err} @@ -91,13 +227,8 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o // Set build specific metadata if instructed if opts.VersionMetadata != "" { - ver, err := semver.NewVersion(result.Version) + ver, err := setBuildMetaData(result.Version, opts.VersionMetadata) if err != nil { - err = fmt.Errorf("failed to parse version from chart metadata as SemVer: %w", err) - return nil, &BuildError{Reason: ErrChartMetadataPatch, Err: err} - } - if *ver, err = ver.SetMetadata(opts.VersionMetadata); err != nil { - err = fmt.Errorf("failed to set SemVer metadata on chart version: %w", err) return nil, &BuildError{Reason: ErrChartMetadataPatch, Err: err} } result.Version = ver.String() @@ -118,57 +249,35 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o result.Path = opts.CachedChart result.ValuesFiles = opts.GetValuesFiles() result.Packaged = requiresPackaging - return result, nil + *buildResult = *result + return nil, nil } } } } // Download the package for the resolved version - res, err := b.remote.DownloadChart(cv) + res, err := remote.DownloadChart(cv) if err != nil { err = fmt.Errorf("failed to download chart for remote reference: %w", err) - return result, &BuildError{Reason: ErrChartPull, Err: err} + return nil, &BuildError{Reason: ErrChartPull, Err: err} } - // Use literal chart copy from remote if no custom values files options are - // set or version metadata isn't set. - if !requiresPackaging { - if err = validatePackageAndWriteToPath(res, p); err != nil { - return nil, &BuildError{Reason: ErrChartPull, Err: err} - } - result.Path = p - return result, nil - } + *buildResult = *result - // Load the chart and merge chart values - var chart *helmchart.Chart - if chart, err = secureloader.LoadArchive(res); err != nil { - err = fmt.Errorf("failed to load downloaded chart: %w", err) - return result, &BuildError{Reason: ErrChartPackage, Err: err} - } - chart.Metadata.Version = result.Version + return res, nil +} - mergedValues, err := mergeChartValues(chart, opts.ValuesFiles) +func setBuildMetaData(version, versionMetadata string) (*semver.Version, error) { + ver, err := semver.NewVersion(version) if err != nil { - err = fmt.Errorf("failed to merge chart values: %w", err) - return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err} + return nil, fmt.Errorf("failed to parse version from chart metadata as SemVer: %w", err) } - // Overwrite default values with merged values, if any - if ok, err = OverwriteChartDefaultValues(chart, mergedValues); ok || err != nil { - if err != nil { - return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err} - } - result.ValuesFiles = opts.GetValuesFiles() + if *ver, err = ver.SetMetadata(versionMetadata); err != nil { + return nil, fmt.Errorf("failed to set SemVer metadata on chart version: %w", err) } - // Package the chart with the custom values - if err = packageToPath(chart, p); err != nil { - return nil, &BuildError{Reason: ErrChartPackage, Err: err} - } - result.Path = p - result.Packaged = true - return result, nil + return ver, nil } // mergeChartValues merges the given chart.Chart Files paths into a single "values.yaml" map. diff --git a/internal/helm/repository/oci_chart_repository.go b/internal/helm/repository/oci_chart_repository.go new file mode 100644 index 000000000..61aeee438 --- /dev/null +++ b/internal/helm/repository/oci_chart_repository.go @@ -0,0 +1,189 @@ +/* +Copyright 2020 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 repository + +import ( + "bytes" + "crypto/tls" + "fmt" + "net/url" + "strings" + + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/getter" + "helm.sh/helm/v3/pkg/registry" + "helm.sh/helm/v3/pkg/repo" + + "github.com/fluxcd/source-controller/internal/transport" +) + +// OCIChartRepository represents a Helm chart repository, and the configuration +// required to download the repository tags and charts from the repository. +// All methods are thread safe unless defined otherwise. +type OCIChartRepository struct { + // URL the ChartRepository's index.yaml can be found at, + // without the index.yaml suffix. + URL url.URL + // Client to use while downloading the Index or a chart from the URL. + Client getter.Getter + // Options to configure the Client with while downloading the Index + // or a chart from the URL. + Options []getter.Option + + tlsConfig *tls.Config + + // RegistryClient is a client to use while downloading tags or charts from a registry. + RegistryClient *registry.Client +} + +// OCIChartRepositoryOption is a function that can be passed to NewOCIChartRepository +// to configure an OCIChartRepository. +type OCIChartRepositoryOption func(*OCIChartRepository) error + +// WithOCIRegistryClient returns a ChartRepositoryOption that will set the registry client +func WithOCIRegistryClient(client *registry.Client) OCIChartRepositoryOption { + return func(r *OCIChartRepository) error { + r.RegistryClient = client + return nil + } +} + +// WithOCIGetter returns a ChartRepositoryOption that will set the getter.Getter +func WithOCIGetter(providers getter.Providers) OCIChartRepositoryOption { + return func(r *OCIChartRepository) error { + c, err := providers.ByScheme(r.URL.Scheme) + if err != nil { + return err + } + r.Client = c + return nil + } +} + +// WithOCIGetterOptions returns a ChartRepositoryOption that will set the getter.Options +func WithOCIGetterOptions(getterOpts []getter.Option) OCIChartRepositoryOption { + return func(r *OCIChartRepository) error { + r.Options = getterOpts + return nil + } +} + +// WithOCITlsConfig returns a ChartRepositoryOption that will set the tls.Config +func WithTlsConfig(tlsConfig *tls.Config) OCIChartRepositoryOption { + return func(r *OCIChartRepository) error { + r.tlsConfig = tlsConfig + return nil + } +} + +// NewOCIChartRepository constructs and returns a new ChartRepository with +// the ChartRepository.Client configured to the getter.Getter for the +// repository URL scheme. It returns an error on URL parsing failures, +// or if there is no getter available for the scheme. +func NewOCIChartRepository(repositoryURL string, chartRepoOpts ...OCIChartRepositoryOption) (*OCIChartRepository, error) { + u, err := url.Parse(repositoryURL) + if err != nil { + return nil, err + } + + if !registry.IsOCI(repositoryURL) { + return nil, fmt.Errorf("the url scheme is not supported: %s", u.Scheme) + } + + r := newOCIChartRepository() + r.URL = *u + for _, opt := range chartRepoOpts { + if err := opt(r); err != nil { + return nil, err + } + } + + return r, nil +} + +func newOCIChartRepository() *OCIChartRepository { + return &OCIChartRepository{} +} + +// Get returns the repo.ChartVersion for the given name, the version is expected +// to be a semver.Constraints compatible string. If version is empty, the latest +// stable version will be returned and prerelease versions will be ignored. +// adapted from https://github.com/helm/helm/blob/49819b4ef782e80b0c7f78c30bd76b51ebb56dc8/pkg/downloader/chart_downloader.go#L162 +func (r *OCIChartRepository) Get(name, ver string) (*repo.ChartVersion, error) { + // Find chart versions matching the given name. + // Either in an index file or from a registry. + cvs, err := r.getTags(fmt.Sprintf("%s/%s", r.URL.String(), name)) + if err != nil { + return nil, err + } + + if len(cvs) == 0 { + return nil, fmt.Errorf("unable to locate any tags in provided repository: %s", name) + } + + // Determine if version provided + // If empty, try to get the highest available tag + // If exact version, try to find it + // If semver constraint string, try to find a match + tag, err := registry.GetTagMatchingVersionOrConstraint(cvs, ver) + return &repo.ChartVersion{ + URLs: []string{fmt.Sprintf("%s/%s:%s", r.URL.String(), name, tag)}, + Metadata: &chart.Metadata{ + Name: name, + Version: tag, + }, + }, err +} + +// this function shall be called for OCI registries only +// It assumes that the ref has been validated to be an OCI reference. +func (r *OCIChartRepository) getTags(ref string) ([]string, error) { + // Retrieve list of repository tags + tags, err := r.RegistryClient.Tags(strings.TrimPrefix(ref, fmt.Sprintf("%s://", registry.OCIScheme))) + if err != nil { + return nil, err + } + if len(tags) == 0 { + return nil, fmt.Errorf("unable to locate any tags in provided repository: %s", ref) + } + + return tags, nil +} + +// DownloadChart confirms the given repo.ChartVersion has a downloadable URL, +// and then attempts to download the chart using the Client and Options of the +// ChartRepository. It returns a bytes.Buffer containing the chart data. +// In case of an OCI hosted chart, this function assumes that the chartVersion url is valid. +func (r *OCIChartRepository) DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer, error) { + if len(chart.URLs) == 0 { + return nil, fmt.Errorf("chart '%s' has no downloadable URLs", chart.Name) + } + + ref := chart.URLs[0] + u, err := url.Parse(ref) + if err != nil { + err = fmt.Errorf("invalid chart URL format '%s': %w", ref, err) + return nil, err + } + + t := transport.NewOrIdle(r.tlsConfig) + clientOpts := append(r.Options, getter.WithTransport(t)) + defer transport.Release(t) + + // trim the oci scheme prefix if needed + return r.Client.Get(strings.TrimPrefix(u.String(), fmt.Sprintf("%s://", registry.OCIScheme)), clientOpts...) +} diff --git a/main.go b/main.go index b9245bd71..cc4d613fd 100644 --- a/main.go +++ b/main.go @@ -27,6 +27,7 @@ import ( "github.com/go-logr/logr" flag "github.com/spf13/pflag" "helm.sh/helm/v3/pkg/getter" + "helm.sh/helm/v3/pkg/registry" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -69,6 +70,15 @@ var ( } ) +type LogWriter struct { + log logr.Logger +} + +func (l LogWriter) Write(p []byte) (n int, err error) { + l.log.Info(string(p)) + return len(p), nil +} + func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) @@ -232,13 +242,20 @@ func main() { os.Exit(1) } - if err = (&controllers.HelmRepositoryOCI{ + rClient, err := registry.NewClient(registry.ClientOptWriter(LogWriter{logger.NewLogger(logger.Options{}).WithName("registry-client")})) + if err != nil { + setupLog.Error(err, "unable to create OCI registry client") + os.Exit(1) + } + + if err = (&controllers.HelmRepositoryOCIReconciler{ Client: mgr.GetClient(), EventRecorder: eventRecorder, Metrics: metricsH, // Storage: storage, Getters: getters, ControllerName: controllerName, + RegistryClient: rClient, }).SetupWithManagerAndOptions(mgr, controllers.HelmRepositoryReconcilerOptions{ MaxConcurrentReconciles: concurrent, RateLimiter: helper.GetRateLimiter(rateLimiterOptions), @@ -269,6 +286,7 @@ func main() { if err = (&controllers.HelmChartReconciler{ Client: mgr.GetClient(), + RegistryClient: rClient, Storage: storage, Getters: getters, EventRecorder: eventRecorder, From d31337bd9ee5a5b25ba7188e6f0b2e3052efd965 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Wed, 27 Apr 2022 09:57:22 +0200 Subject: [PATCH 5/7] remove debug statements Signed-off-by: Max Jonas Werner --- controllers/helmchart_controller.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 460ceae6f..cf44dc9f8 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -492,12 +492,9 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // Initialize the chart repository var chartRepo chart.Remote var err error - fmt.Printf("CREATING REPO FROM %s\n", repo.Spec.URL) if registry.IsOCI(repo.Spec.URL) { - fmt.Printf("CREATING OCI REPO\n") chartRepo, err = repository.NewOCIChartRepository(repo.Spec.URL, repository.WithOCIGetter(r.Getters), repository.WithOCIRegistryClient(r.RegistryClient)) } else { - fmt.Printf("CREATING HTTP REPO\n") var httpChartRepo *repository.ChartRepository httpChartRepo, err = repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts, repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) { From cf408f79feb358d00241d3b1ced73509b7b289c2 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Thu, 28 Apr 2022 17:57:11 +0200 Subject: [PATCH 6/7] For backwards-compatibility, an empty `.spec.type` field now leads to the HelmRepository being treated as a plain old HTTP Helm repository. discard output from OCI registry client The client has no way to set a verbosity level and spamming the controller logs with "Login succeeded" every time the object is reconciled doesn't help much. Signed-off-by: Max Jonas Werner --- api/v1beta1/helmrepository_types.go | 2 +- api/v1beta2/helmrepository_types.go | 6 +++--- .../source.toolkit.fluxcd.io_helmrepositories.yaml | 8 ++++++-- controllers/helmrepository_controller.go | 7 ++++++- main.go | 12 ++---------- 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/api/v1beta1/helmrepository_types.go b/api/v1beta1/helmrepository_types.go index 5cbfe573f..bb88e9efb 100644 --- a/api/v1beta1/helmrepository_types.go +++ b/api/v1beta1/helmrepository_types.go @@ -75,7 +75,7 @@ type HelmRepositorySpec struct { // Type of the HelmRepository. // When this field is set to "OCI", the URL field value must be prefixed with "oci://". - // +kubebuilder:default:="Default" + // +kubebuilder:validation:Enum=default;oci // +optional Type string `json:"type,omitempty"` } diff --git a/api/v1beta2/helmrepository_types.go b/api/v1beta2/helmrepository_types.go index 1c001a9b5..92e9d74ea 100644 --- a/api/v1beta2/helmrepository_types.go +++ b/api/v1beta2/helmrepository_types.go @@ -32,8 +32,8 @@ const ( // objects by their HelmRepositorySpec.URL. HelmRepositoryURLIndexKey = ".metadata.helmRepositoryURL" - HelmRepositoryTypeDefault = "Default" - HelmRepositoryTypeOCI = "OCI" + HelmRepositoryTypeDefault = "default" + HelmRepositoryTypeOCI = "oci" ) // HelmRepositorySpec specifies the required configuration to produce an @@ -84,7 +84,7 @@ type HelmRepositorySpec struct { // Type of the HelmRepository. // When this field is set to "OCI", the URL field value must be prefixed with "oci://". - // +kubebuilder:default:="Default" + // +kubebuilder:validation:Enum=default;oci // +optional Type string `json:"type,omitempty"` } diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml index 6e0825221..bb4e77e17 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml @@ -110,9 +110,11 @@ spec: description: The timeout of index downloading, defaults to 60s. type: string type: - default: Default description: Type of the HelmRepository. When this field is set to "OCI", the URL field value must be prefixed with "oci://". + enum: + - default + - oci type: string url: description: The Helm repository URL, a valid URL contains at least @@ -336,9 +338,11 @@ spec: description: Timeout of the index fetch operation, defaults to 60s. type: string type: - default: Default description: Type of the HelmRepository. When this field is set to "OCI", the URL field value must be prefixed with "oci://". + enum: + - default + - oci type: string url: description: URL of the Helm repository, a valid URL contains at least diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 6970cb772..954f4b969 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -125,7 +125,11 @@ func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, For(&sourcev1.HelmRepository{}). WithEventFilter( predicate.And( - predicate.NewPredicateFuncs(HelmRepositoryTypeFilter(sourcev1.HelmRepositoryTypeDefault)), + predicate.Or( + predicate.NewPredicateFuncs(HelmRepositoryTypeFilter(sourcev1.HelmRepositoryTypeDefault)), + // an empty type field defaults to handling the repo as traditional HTTP Helm repo + predicate.NewPredicateFuncs(HelmRepositoryTypeFilter("")), + ), predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}), ), ). @@ -601,3 +605,4 @@ func (r *HelmRepositoryReconciler) eventLogf(ctx context.Context, obj runtime.Ob } r.Eventf(obj, eventType, reason, msg) } +r \ No newline at end of file diff --git a/main.go b/main.go index cc4d613fd..28a024a7b 100644 --- a/main.go +++ b/main.go @@ -18,6 +18,7 @@ package main import ( "fmt" + "io" "net" "net/http" "os" @@ -70,15 +71,6 @@ var ( } ) -type LogWriter struct { - log logr.Logger -} - -func (l LogWriter) Write(p []byte) (n int, err error) { - l.log.Info(string(p)) - return len(p), nil -} - func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) @@ -242,7 +234,7 @@ func main() { os.Exit(1) } - rClient, err := registry.NewClient(registry.ClientOptWriter(LogWriter{logger.NewLogger(logger.Options{}).WithName("registry-client")})) + rClient, err := registry.NewClient(registry.ClientOptWriter(io.Discard)) if err != nil { setupLog.Error(err, "unable to create OCI registry client") os.Exit(1) From 482ec504006f7da3ee8c06ddbaaf3d4ec3075dd8 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Tue, 3 May 2022 16:13:03 +0200 Subject: [PATCH 7/7] Add OCI Helm support If implemented: - users will be able to declare OCI HelmRepository by using the .Spec.Type field of the HelmRepository API. Contrary to the HTTP/S HelmRepository no index.yaml is reconciled from source, instead a simple url and credentials validation is performed. - users will be able to declare the new OCI HelmRepository type as source using the .Spec.SourceRef field of the HelmChart API. This will result in reconciling a chart from an OCI repository. Signed-off-by: Soule BA --- api/v1beta1/helmrepository_types.go | 6 - api/v1beta2/condition_types.go | 4 - api/v1beta2/helmrepository_types.go | 8 +- ...ce.toolkit.fluxcd.io_helmrepositories.yaml | 9 +- .../testdata/helmchart-from-oci/source.yaml | 21 ++ controllers/helmchart_controller.go | 114 ++++++--- controllers/helmchart_controller_test.go | 224 ++++++++++++++++- controllers/helmrepository_controller.go | 29 ++- controllers/helmrepository_controller_oci.go | 112 ++++++--- .../helmrepository_controller_oci_test.go | 9 +- controllers/helmrepository_controller_test.go | 207 +++++++++++++++ controllers/helmrepository_predicate.go | 22 -- controllers/suite_test.go | 19 +- docs/api/source.md | 26 ++ docs/spec/v1beta2/helmrepositories.md | 121 ++++++++- go.mod | 7 +- hack/ci/e2e.sh | 6 + internal/helm/chart/builder_remote.go | 83 +++--- internal/helm/chart/builder_remote_test.go | 162 +++++++++++- .../helm/repository/oci_chart_repository.go | 119 ++++++--- .../repository/oci_chart_repository_test.go | 238 ++++++++++++++++++ internal/helm/repository/utils.go | 4 +- internal/helm/util/client.go | 50 ++++ .../helmrepository_type_predicate.go | 86 +++++++ .../helmrepository_type_predicate_test.go | 127 ++++++++++ main.go | 42 ++-- 26 files changed, 1610 insertions(+), 245 deletions(-) create mode 100644 config/testdata/helmchart-from-oci/source.yaml delete mode 100644 controllers/helmrepository_predicate.go create mode 100644 internal/helm/repository/oci_chart_repository_test.go create mode 100644 internal/helm/util/client.go create mode 100644 internal/predicates/helmrepository_type_predicate.go create mode 100644 internal/predicates/helmrepository_type_predicate_test.go diff --git a/api/v1beta1/helmrepository_types.go b/api/v1beta1/helmrepository_types.go index bb88e9efb..62b0e9a6d 100644 --- a/api/v1beta1/helmrepository_types.go +++ b/api/v1beta1/helmrepository_types.go @@ -72,12 +72,6 @@ type HelmRepositorySpec struct { // AccessFrom defines an Access Control List for allowing cross-namespace references to this object. // +optional AccessFrom *acl.AccessFrom `json:"accessFrom,omitempty"` - - // Type of the HelmRepository. - // When this field is set to "OCI", the URL field value must be prefixed with "oci://". - // +kubebuilder:validation:Enum=default;oci - // +optional - Type string `json:"type,omitempty"` } // HelmRepositoryStatus defines the observed state of the HelmRepository. diff --git a/api/v1beta2/condition_types.go b/api/v1beta2/condition_types.go index 4bcd3d745..711469eb8 100644 --- a/api/v1beta2/condition_types.go +++ b/api/v1beta2/condition_types.go @@ -39,10 +39,6 @@ const ( // is enabled. SourceVerifiedCondition string = "SourceVerified" - //SourceValidCondition indicates the validity of the Source. - // If True, the Source is valid. If False, it is not valid. - SourceValidCondition string = "SourceValid" - // FetchFailedCondition indicates a transient or persistent fetch failure // of an upstream Source. // If True, observations on the upstream Source revision may be impossible, diff --git a/api/v1beta2/helmrepository_types.go b/api/v1beta2/helmrepository_types.go index 92e9d74ea..87c0b16b8 100644 --- a/api/v1beta2/helmrepository_types.go +++ b/api/v1beta2/helmrepository_types.go @@ -31,9 +31,11 @@ const ( // HelmRepositoryURLIndexKey is the key used for indexing HelmRepository // objects by their HelmRepositorySpec.URL. HelmRepositoryURLIndexKey = ".metadata.helmRepositoryURL" - + // HelmRepositoryTypeDefault is the default HelmRepository type. + // It is used when no type is specified and corresponds to a Helm repository. HelmRepositoryTypeDefault = "default" - HelmRepositoryTypeOCI = "oci" + // HelmRepositoryTypeOCI is the type for an OCI repository. + HelmRepositoryTypeOCI = "oci" ) // HelmRepositorySpec specifies the required configuration to produce an @@ -83,7 +85,7 @@ type HelmRepositorySpec struct { AccessFrom *acl.AccessFrom `json:"accessFrom,omitempty"` // Type of the HelmRepository. - // When this field is set to "OCI", the URL field value must be prefixed with "oci://". + // When this field is set to "oci", the URL field value must be prefixed with "oci://". // +kubebuilder:validation:Enum=default;oci // +optional Type string `json:"type,omitempty"` diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml index bb4e77e17..bde30e786 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml @@ -109,13 +109,6 @@ spec: default: 60s description: The timeout of index downloading, defaults to 60s. type: string - type: - description: Type of the HelmRepository. When this field is set to "OCI", - the URL field value must be prefixed with "oci://". - enum: - - default - - oci - type: string url: description: The Helm repository URL, a valid URL contains at least a protocol and host. @@ -338,7 +331,7 @@ spec: description: Timeout of the index fetch operation, defaults to 60s. type: string type: - description: Type of the HelmRepository. When this field is set to "OCI", + description: Type of the HelmRepository. When this field is set to "oci", the URL field value must be prefixed with "oci://". enum: - default diff --git a/config/testdata/helmchart-from-oci/source.yaml b/config/testdata/helmchart-from-oci/source.yaml new file mode 100644 index 000000000..9d9945ff6 --- /dev/null +++ b/config/testdata/helmchart-from-oci/source.yaml @@ -0,0 +1,21 @@ +--- +apiVersion: source.toolkit.fluxcd.io/v1beta2 +kind: HelmRepository +metadata: + name: podinfo +spec: + url: oci://ghcr.io/stefanprodan/charts + type: "oci" + interval: 1m +--- +apiVersion: source.toolkit.fluxcd.io/v1beta2 +kind: HelmChart +metadata: + name: podinfo +spec: + chart: podinfo + sourceRef: + kind: HelmRepository + name: podinfo + version: '6.1.*' + interval: 1m diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index cf44dc9f8..8afb96c77 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -117,10 +117,10 @@ type HelmChartReconciler struct { kuberecorder.EventRecorder helper.Metrics - RegistryClient *registry.Client - Storage *Storage - Getters helmgetter.Providers - ControllerName string + RegistryClientGenerator RegistryClientGeneratorFunc + Storage *Storage + Getters helmgetter.Providers + ControllerName string Cache *cache.Cache TTL time.Duration @@ -445,7 +445,10 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1 // object, and returns early. func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *sourcev1.HelmChart, repo *sourcev1.HelmRepository, b *chart.Build) (sreconcile.Result, error) { - var tlsConfig *tls.Config + var ( + tlsConfig *tls.Config + logOpts []registry.LoginOption + ) // Construct the Getter options from the HelmRepository data clientOpts := []helmgetter.Option{ @@ -487,19 +490,71 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // Requeue as content of secret might change return sreconcile.ResultEmpty, e } + + // Build registryClient options from secret + logOpt, err := loginOptionFromSecret(*secret) + if err != nil { + e := &serror.Event{ + Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), + Reason: sourcev1.AuthenticationFailedReason, + } + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + // Requeue as content of secret might change + return sreconcile.ResultEmpty, e + } + + logOpts = append([]registry.LoginOption{}, logOpt) } // Initialize the chart repository var chartRepo chart.Remote - var err error - if registry.IsOCI(repo.Spec.URL) { - chartRepo, err = repository.NewOCIChartRepository(repo.Spec.URL, repository.WithOCIGetter(r.Getters), repository.WithOCIRegistryClient(r.RegistryClient)) - } else { + switch repo.Spec.Type { + case sourcev1.HelmRepositoryTypeOCI: + if !registry.IsOCI(repo.Spec.URL) { + err := fmt.Errorf("invalid OCI registry URL: %s", repo.Spec.URL) + return chartRepoErrorReturn(err, obj) + } + + // with this function call, we create a temporary file to store the credentials if needed. + // this is needed because otherwise the credentials are stored in ~/.docker/config.json. + // TODO@souleb: remove this once the registry move to Oras v2 + // or rework to enable reusing credentials to avoid the unneccessary handshake operations + registryClient, file, err := r.RegistryClientGenerator(logOpts != nil) + if err != nil { + return chartRepoErrorReturn(err, obj) + } + + if file != "" { + defer func() { + os.Remove(file) + }() + } + + // Tell the chart repository to use the OCI client with the configured getter + clientOpts = append(clientOpts, helmgetter.WithRegistryClient(registryClient)) + ociChartRepo, err := repository.NewOCIChartRepository(repo.Spec.URL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(clientOpts), repository.WithOCIRegistryClient(registryClient)) + if err != nil { + return chartRepoErrorReturn(err, obj) + } + chartRepo = ociChartRepo + + // If login options are configured, use them to login to the registry + // The OCIGetter will later retrieve the stored credentials to pull the chart + if logOpts != nil { + err = ociChartRepo.Login(logOpts...) + if err != nil { + return chartRepoErrorReturn(err, obj) + } + } + default: var httpChartRepo *repository.ChartRepository - httpChartRepo, err = repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts, + httpChartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts, repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) { r.IncCacheEvents(event, obj.Name, obj.Namespace) })) + if err != nil { + return chartRepoErrorReturn(err, obj) + } chartRepo = httpChartRepo defer func() { if httpChartRepo == nil { @@ -523,26 +578,6 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * } }() } - if err != nil { - // Any error requires a change in generation, - // which we should be informed about by the watcher - switch err.(type) { - case *url.Error: - e := &serror.Stalling{ - Err: fmt.Errorf("invalid Helm repository URL: %w", err), - Reason: sourcev1.URLInvalidReason, - } - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) - return sreconcile.ResultEmpty, e - default: - e := &serror.Stalling{ - Err: fmt.Errorf("failed to construct Helm client: %w", err), - Reason: meta.FailedReason, - } - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) - return sreconcile.ResultEmpty, e - } - } // Construct the chart builder with scoped configuration cb := chart.NewRemoteBuilder(chartRepo) @@ -1106,3 +1141,22 @@ func reasonForBuild(build *chart.Build) string { } return sourcev1.ChartPullSucceededReason } + +func chartRepoErrorReturn(err error, obj *sourcev1.HelmChart) (sreconcile.Result, error) { + switch err.(type) { + case *url.Error: + e := &serror.Stalling{ + Err: fmt.Errorf("invalid Helm repository URL: %w", err), + Reason: sourcev1.URLInvalidReason, + } + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + default: + e := &serror.Stalling{ + Err: fmt.Errorf("failed to construct Helm client: %w", err), + Reason: meta.FailedReason, + } + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + } +} diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 5fe93e86c..9bc5a39e1 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -17,10 +17,12 @@ limitations under the License. package controllers import ( + "bytes" "context" "errors" "fmt" "io" + "io/ioutil" "net/http" "os" "path/filepath" @@ -31,6 +33,9 @@ import ( "github.com/darkowlzz/controller-check/status" . "github.com/onsi/gomega" + hchart "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chart/loader" + "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -45,10 +50,10 @@ import ( "github.com/fluxcd/pkg/runtime/conditions" "github.com/fluxcd/pkg/runtime/patch" "github.com/fluxcd/pkg/testserver" - sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" serror "github.com/fluxcd/source-controller/internal/error" "github.com/fluxcd/source-controller/internal/helm/chart" + "github.com/fluxcd/source-controller/internal/helm/util" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" ) @@ -776,6 +781,214 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { } } +func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) { + g := NewWithT(t) + + tmpDir := t.TempDir() + + const ( + chartPath = "testdata/charts/helmchart-0.1.0.tgz" + ) + + // Login to the registry + err := testRegistryserver.RegistryClient.Login(testRegistryserver.DockerRegistryHost, + registry.LoginOptBasicAuth(testUsername, testPassword), + registry.LoginOptInsecure(true)) + g.Expect(err).NotTo(HaveOccurred()) + + // Load a test chart + chartData, err := ioutil.ReadFile(chartPath) + g.Expect(err).NotTo(HaveOccurred()) + metadata, err := extractChartMeta(chartData) + g.Expect(err).NotTo(HaveOccurred()) + + // Upload the test chart + ref := fmt.Sprintf("%s/testrepo/%s:%s", testRegistryserver.DockerRegistryHost, metadata.Name, metadata.Version) + _, err = testRegistryserver.RegistryClient.Push(chartData, ref) + g.Expect(err).NotTo(HaveOccurred()) + + storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords) + g.Expect(err).ToNot(HaveOccurred()) + + cachedArtifact := &sourcev1.Artifact{ + Revision: "0.1.0", + Path: metadata.Name + "-" + metadata.Version + ".tgz", + } + g.Expect(storage.CopyFromPath(cachedArtifact, "testdata/charts/helmchart-0.1.0.tgz")).To(Succeed()) + + tests := []struct { + name string + secret *corev1.Secret + beforeFunc func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) + want sreconcile.Result + wantErr error + assertFunc func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) + cleanFunc func(g *WithT, build *chart.Build) + }{ + { + name: "Reconciles chart build with repository credentials", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth", + }, + Data: map[string][]byte{ + "username": []byte(testUsername), + "password": []byte(testPassword), + }, + }, + beforeFunc: func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) { + obj.Spec.Chart = metadata.Name + obj.Spec.Version = metadata.Version + repository.Spec.SecretRef = &meta.LocalObjectReference{Name: "auth"} + }, + want: sreconcile.ResultSuccess, + assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build) { + g.Expect(build.Name).To(Equal(metadata.Name)) + g.Expect(build.Version).To(Equal(metadata.Version)) + g.Expect(build.Path).ToNot(BeEmpty()) + g.Expect(build.Path).To(BeARegularFile()) + }, + cleanFunc: func(g *WithT, build *chart.Build) { + g.Expect(os.Remove(build.Path)).To(Succeed()) + }, + }, + { + name: "Uses artifact as build cache", + beforeFunc: func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) { + obj.Spec.Chart = metadata.Name + obj.Spec.Version = metadata.Version + obj.Status.Artifact = &sourcev1.Artifact{Path: metadata.Name + "-" + metadata.Version + ".tgz"} + }, + want: sreconcile.ResultSuccess, + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + g.Expect(build.Name).To(Equal(metadata.Name)) + g.Expect(build.Version).To(Equal(metadata.Version)) + g.Expect(build.Path).To(Equal(storage.LocalPath(*cachedArtifact.DeepCopy()))) + g.Expect(build.Path).To(BeARegularFile()) + }, + }, + { + name: "Forces build on generation change", + beforeFunc: func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) { + obj.Generation = 3 + obj.Spec.Chart = metadata.Name + obj.Spec.Version = metadata.Version + + obj.Status.ObservedGeneration = 2 + obj.Status.Artifact = &sourcev1.Artifact{Path: metadata.Name + "-" + metadata.Version + ".tgz"} + }, + want: sreconcile.ResultSuccess, + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + g.Expect(build.Name).To(Equal(metadata.Name)) + g.Expect(build.Version).To(Equal(metadata.Version)) + fmt.Println("buildpath", build.Path) + fmt.Println("storage Path", storage.LocalPath(*cachedArtifact.DeepCopy())) + g.Expect(build.Path).ToNot(Equal(storage.LocalPath(*cachedArtifact.DeepCopy()))) + g.Expect(build.Path).To(BeARegularFile()) + }, + cleanFunc: func(g *WithT, build *chart.Build) { + g.Expect(os.Remove(build.Path)).To(Succeed()) + }, + }, + { + name: "Event on unsuccessful secret retrieval", + beforeFunc: func(_ *sourcev1.HelmChart, repository *sourcev1.HelmRepository) { + repository.Spec.SecretRef = &meta.LocalObjectReference{ + Name: "invalid", + } + }, + want: sreconcile.ResultEmpty, + wantErr: &serror.Event{Err: errors.New("failed to get secret 'invalid'")}, + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + g.Expect(build.Complete()).To(BeFalse()) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret 'invalid'"), + })) + }, + }, + { + name: "Stalling on invalid client options", + beforeFunc: func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) { + repository.Spec.URL = "https://unsupported" // Unsupported protocol + }, + want: sreconcile.ResultEmpty, + wantErr: &serror.Stalling{Err: errors.New("failed to construct Helm client: invalid OCI registry URL: https://unsupported")}, + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + g.Expect(build.Complete()).To(BeFalse()) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(sourcev1.FetchFailedCondition, meta.FailedReason, "failed to construct Helm client"), + })) + }, + }, + { + name: "BuildError on temporary build error", + beforeFunc: func(obj *sourcev1.HelmChart, _ *sourcev1.HelmRepository) { + obj.Spec.Chart = "invalid" + }, + want: sreconcile.ResultEmpty, + wantErr: &chart.BuildError{Err: errors.New("failed to get chart version for remote reference")}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + clientBuilder := fake.NewClientBuilder() + if tt.secret != nil { + clientBuilder.WithObjects(tt.secret.DeepCopy()) + } + + r := &HelmChartReconciler{ + Client: clientBuilder.Build(), + EventRecorder: record.NewFakeRecorder(32), + Getters: testGetters, + Storage: storage, + RegistryClientGenerator: util.RegistryClientGenerator, + } + + repository := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-", + }, + Spec: sourcev1.HelmRepositorySpec{ + URL: fmt.Sprintf("oci://%s/testrepo", testRegistryserver.DockerRegistryHost), + Timeout: &metav1.Duration{Duration: timeout}, + Type: sourcev1.HelmRepositoryTypeOCI, + }, + } + obj := &sourcev1.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-", + }, + Spec: sourcev1.HelmChartSpec{}, + } + + if tt.beforeFunc != nil { + tt.beforeFunc(obj, repository) + } + + var b chart.Build + if tt.cleanFunc != nil { + defer tt.cleanFunc(g, &b) + } + got, err := r.buildFromHelmRepository(context.TODO(), obj, repository, &b) + + g.Expect(err != nil).To(Equal(tt.wantErr != nil)) + if tt.wantErr != nil { + g.Expect(reflect.TypeOf(err).String()).To(Equal(reflect.TypeOf(tt.wantErr).String())) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr.Error())) + } + g.Expect(got).To(Equal(tt.want)) + + if tt.assertFunc != nil { + tt.assertFunc(g, obj, b) + } + }) + } +} + func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) { g := NewWithT(t) @@ -1690,3 +1903,12 @@ func TestHelmChartReconciler_notify(t *testing.T) { }) } } + +// extractChartMeta is used to extract a chart metadata from a byte array +func extractChartMeta(chartData []byte) (*hchart.Metadata, error) { + ch, err := loader.LoadArchive(bytes.NewReader(chartData)) + if err != nil { + return nil, err + } + return ch.Metadata, nil +} diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 954f4b969..5e117d825 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -48,6 +48,7 @@ import ( serror "github.com/fluxcd/source-controller/internal/error" "github.com/fluxcd/source-controller/internal/helm/getter" "github.com/fluxcd/source-controller/internal/helm/repository" + intpredicates "github.com/fluxcd/source-controller/internal/predicates" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" ) @@ -126,9 +127,8 @@ func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, WithEventFilter( predicate.And( predicate.Or( - predicate.NewPredicateFuncs(HelmRepositoryTypeFilter(sourcev1.HelmRepositoryTypeDefault)), - // an empty type field defaults to handling the repo as traditional HTTP Helm repo - predicate.NewPredicateFuncs(HelmRepositoryTypeFilter("")), + intpredicates.HelmRepositoryTypePredicate{RepositoryType: sourcev1.HelmRepositoryTypeDefault}, + intpredicates.HelmRepositoryTypePredicate{RepositoryType: ""}, ), predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}), ), @@ -200,7 +200,8 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque } // Examine if the object is under deletion - if !obj.ObjectMeta.DeletionTimestamp.IsZero() { + // or if a type change has happened + if !obj.ObjectMeta.DeletionTimestamp.IsZero() || (obj.Spec.Type != "" && obj.Spec.Type != sourcev1.HelmRepositoryTypeDefault) { recResult, retErr = r.reconcileDelete(ctx, obj) return } @@ -547,8 +548,10 @@ func (r *HelmRepositoryReconciler) reconcileDelete(ctx context.Context, obj *sou return sreconcile.ResultEmpty, err } - // Remove our finalizer from the list - controllerutil.RemoveFinalizer(obj, sourcev1.SourceFinalizer) + // Remove our finalizer from the list if we are deleting the object + if !obj.DeletionTimestamp.IsZero() { + controllerutil.RemoveFinalizer(obj, sourcev1.SourceFinalizer) + } // Stop reconciliation as the object is being deleted return sreconcile.ResultEmpty, nil @@ -556,11 +559,12 @@ func (r *HelmRepositoryReconciler) reconcileDelete(ctx context.Context, obj *sou // garbageCollect performs a garbage collection for the given object. // -// It removes all but the current Artifact from the Storage, unless the -// deletion timestamp on the object is set. Which will result in the -// removal of all Artifacts for the objects. +// It removes all but the current Artifact from the Storage, unless: +// - the deletion timestamp on the object is set +// - the obj.Spec.Type has changed and artifacts are not supported by the new type +// Which will result in the removal of all Artifacts for the objects. func (r *HelmRepositoryReconciler) garbageCollect(ctx context.Context, obj *sourcev1.HelmRepository) error { - if !obj.DeletionTimestamp.IsZero() { + if !obj.DeletionTimestamp.IsZero() || (obj.Spec.Type != "" && obj.Spec.Type != sourcev1.HelmRepositoryTypeDefault) { if deleted, err := r.Storage.RemoveAll(r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), "", "*")); err != nil { return &serror.Event{ Err: fmt.Errorf("garbage collection for deleted resource failed: %w", err), @@ -570,7 +574,11 @@ func (r *HelmRepositoryReconciler) garbageCollect(ctx context.Context, obj *sour r.eventLogf(ctx, obj, events.EventTypeTrace, "GarbageCollectionSucceeded", "garbage collected artifacts for deleted resource") } + // Clean status sub-resource obj.Status.Artifact = nil + obj.Status.URL = "" + // Remove the condition as the artifact doesn't exist. + conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) return nil } if obj.GetArtifact() != nil { @@ -605,4 +613,3 @@ func (r *HelmRepositoryReconciler) eventLogf(ctx context.Context, obj runtime.Ob } r.Eventf(obj, eventType, reason, msg) } -r \ No newline at end of file diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 45d1f198b..05da9af0c 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -1,9 +1,26 @@ +/* +Copyright 2022 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 controllers import ( "context" "fmt" - "net/url" + "os" + "strings" "time" "github.com/fluxcd/pkg/apis/meta" @@ -13,6 +30,8 @@ import ( "github.com/fluxcd/pkg/runtime/predicates" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" serror "github.com/fluxcd/source-controller/internal/error" + "github.com/fluxcd/source-controller/internal/helm/repository" + intpredicates "github.com/fluxcd/source-controller/internal/predicates" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" helmgetter "helm.sh/helm/v3/pkg/getter" @@ -31,14 +50,12 @@ var helmRepositoryOCIReadyCondition = summarize.Conditions{ Target: meta.ReadyCondition, Owned: []string{ sourcev1.FetchFailedCondition, - sourcev1.SourceValidCondition, meta.ReadyCondition, meta.ReconcilingCondition, meta.StalledCondition, }, Summarize: []string{ sourcev1.FetchFailedCondition, - sourcev1.SourceValidCondition, meta.StalledCondition, meta.ReconcilingCondition, }, @@ -65,12 +82,17 @@ type HelmRepositoryOCIReconciler struct { client.Client kuberecorder.EventRecorder helper.Metrics - Getters helmgetter.Providers - // Storage *Storage - ControllerName string - RegistryClient *registry.Client + Getters helmgetter.Providers + ControllerName string + RegistryClientGenerator RegistryClientGeneratorFunc } +// RegistryClientGeneratorFunc is a function that returns a registry client +// and an optional file name. +// The file is used to store the registry client credentials. +// The caller is responsible for deleting the file. +type RegistryClientGeneratorFunc func(isLogin bool) (*registry.Client, string, error) + // helmRepositoryOCIReconcileFunc is the function type for all the // v1beta2.HelmRepository (sub)reconcile functions for OCI type. The type implementations // are grouped and executed serially to perform the complete reconcile of the @@ -86,8 +108,9 @@ func (r *HelmRepositoryOCIReconciler) SetupWithManagerAndOptions(mgr ctrl.Manage For(&sourcev1.HelmRepository{}). WithEventFilter( predicate.And( - predicate.NewPredicateFuncs(HelmRepositoryTypeFilter(sourcev1.HelmRepositoryTypeOCI)), - predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{})), + intpredicates.HelmRepositoryTypePredicate{RepositoryType: sourcev1.HelmRepositoryTypeOCI}, + predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}), + ), ). WithOptions(controller.Options{ MaxConcurrentReconciles: opts.MaxConcurrentReconciles, @@ -161,6 +184,13 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re return } + // Examine if a type change has happened and act accordingly + if obj.Spec.Type != sourcev1.HelmRepositoryTypeOCI { + // just ignore the object if the type has changed + recResult, retErr = sreconcile.ResultEmpty, nil + return + } + // Reconcile actual object reconcilers := []helmRepositoryOCIReconcileFunc{ r.reconcileSource, @@ -243,7 +273,7 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * } // Construct actual options - logOpt, err := r.loginOptionFromSecret(secret) + logOpt, err := loginOptionFromSecret(secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), @@ -266,43 +296,61 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * // validateSource the HelmRepository object by checking the url and connecting to the underlying registry // with he provided credentials. -func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *sourcev1.HelmRepository, loginOpts ...registry.LoginOption) (sreconcile.Result, error) { - target, err := url.Parse(obj.Spec.URL) +func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *sourcev1.HelmRepository, logOpts ...registry.LoginOption) (sreconcile.Result, error) { + registryClient, file, err := r.RegistryClientGenerator(logOpts != nil) if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to parse URL '%s': %w", obj.Spec.URL, err), - Reason: "ValidationError", + e := &serror.Stalling{ + Err: fmt.Errorf("failed to create registry client:: %w", err), + Reason: meta.FailedReason, } - conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error()) + conditions.MarkFalse(obj, meta.ReadyCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } - // Check if the registry is supported - if !registry.IsOCI(obj.Spec.URL) { - e := &serror.Event{ - Err: fmt.Errorf("unsupported registry scheme '%s'", target.Scheme), - Reason: "ValidationError", - } - conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error()) - return sreconcile.ResultEmpty, e + if file != "" { + defer func() { + os.Remove(file) + }() } - err = r.RegistryClient.Login(target.Host+target.Path, loginOpts...) + chartRepo, err := repository.NewOCIChartRepository(obj.Spec.URL, repository.WithOCIRegistryClient(registryClient)) if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to login to registry '%s': %w", target.String(), err), - Reason: "ValidationError", + if strings.Contains(err.Error(), "parse") { + e := &serror.Stalling{ + Err: fmt.Errorf("failed to parse URL '%s': %w", obj.Spec.URL, err), + Reason: sourcev1.URLInvalidReason, + } + conditions.MarkFalse(obj, meta.ReadyCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + } else if strings.Contains(err.Error(), "the url scheme is not supported") { + e := &serror.Event{ + Err: err, + Reason: sourcev1.URLInvalidReason, + } + conditions.MarkFalse(obj, meta.ReadyCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + } + } + + // Attempt to login to the registry if credentials are provided. + if logOpts != nil { + err = chartRepo.Login(logOpts...) + if err != nil { + e := &serror.Event{ + Err: fmt.Errorf("failed to create temporary file: %w", err), + Reason: meta.FailedReason, + } + conditions.MarkFalse(obj, meta.ReadyCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e } - conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error()) - return sreconcile.ResultEmpty, e } - conditions.MarkTrue(obj, sourcev1.SourceValidCondition, meta.SucceededReason, "Helm repository %q is valid", obj.Name) + conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "Helm repository %q is ready", obj.Name) return sreconcile.ResultSuccess, nil } -func (r *HelmRepositoryOCIReconciler) loginOptionFromSecret(secret corev1.Secret) (registry.LoginOption, error) { +func loginOptionFromSecret(secret corev1.Secret) (registry.LoginOption, error) { username, password := string(secret.Data["username"]), string(secret.Data["password"]) switch { case username == "" && password == "": diff --git a/controllers/helmrepository_controller_oci_test.go b/controllers/helmrepository_controller_oci_test.go index 8f63cd4f4..6069fe8ca 100644 --- a/controllers/helmrepository_controller_oci_test.go +++ b/controllers/helmrepository_controller_oci_test.go @@ -1,9 +1,12 @@ /* Copyright 2022 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. @@ -33,12 +36,6 @@ import ( func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { g := NewWithT(t) - // Login to the registry - // err := testRegistryserver.RegistryClient.Login(testRegistryserver.DockerRegistryHost, - // registry.LoginOptBasicAuth(testUsername, testPassword), - // registry.LoginOptInsecure(true)) - // g.Expect(err).NotTo(HaveOccurred()) - ns, err := testEnv.CreateNamespace(ctx, "helmrepository-oci-reconcile-test") g.Expect(err).ToNot(HaveOccurred()) defer func() { g.Expect(testEnv.Delete(ctx, ns)).To(Succeed()) }() diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go index 488ff1c4b..2230a72e3 100644 --- a/controllers/helmrepository_controller_test.go +++ b/controllers/helmrepository_controller_test.go @@ -1085,3 +1085,210 @@ func TestHelmRepositoryReconciler_notify(t *testing.T) { }) } } + +func TestHelmRepositoryReconciler_ReconcileTypeUpdatePredicateFilter(t *testing.T) { + g := NewWithT(t) + + testServer, err := helmtestserver.NewTempHelmServer() + g.Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(testServer.Root()) + + g.Expect(testServer.PackageChart("testdata/charts/helmchart")).To(Succeed()) + g.Expect(testServer.GenerateIndex()).To(Succeed()) + + testServer.Start() + defer testServer.Stop() + + obj := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-reconcile-", + Namespace: "default", + }, + Spec: sourcev1.HelmRepositorySpec{ + Interval: metav1.Duration{Duration: interval}, + URL: testServer.URL(), + }, + } + g.Expect(testEnv.Create(ctx, obj)).To(Succeed()) + + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + // Wait for finalizer to be set + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + return len(obj.Finalizers) > 0 + }, timeout).Should(BeTrue()) + + // Wait for HelmRepository to be Ready + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + if !conditions.IsReady(obj) && obj.Status.Artifact == nil { + return false + } + readyCondition := conditions.Get(obj, meta.ReadyCondition) + return readyCondition.Status == metav1.ConditionTrue && + obj.Generation == readyCondition.ObservedGeneration && + obj.Generation == obj.Status.ObservedGeneration + }, timeout).Should(BeTrue()) + + // Check if the object status is valid. + condns := &status.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} + checker := status.NewChecker(testEnv.Client, condns) + checker.CheckErr(ctx, obj) + + // kstatus client conformance check. + u, err := patch.ToUnstructured(obj) + g.Expect(err).ToNot(HaveOccurred()) + res, err := kstatus.Compute(u) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(res.Status).To(Equal(kstatus.CurrentStatus)) + + // Switch to a OCI helm repository type + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-reconcile-", + Namespace: "default", + }, + Data: map[string][]byte{ + "username": []byte(testUsername), + "password": []byte(testPassword), + }, + } + g.Expect(testEnv.CreateAndWait(ctx, secret)).To(Succeed()) + + obj.Spec.Type = sourcev1.HelmRepositoryTypeOCI + obj.Spec.URL = fmt.Sprintf("oci://%s", testRegistryserver.DockerRegistryHost) + obj.Spec.SecretRef = &meta.LocalObjectReference{ + Name: secret.Name, + } + + g.Expect(testEnv.Update(ctx, obj)).To(Succeed()) + + // Wait for HelmRepository to be Ready + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + if !conditions.IsReady(obj) && obj.Status.Artifact != nil { + return false + } + readyCondition := conditions.Get(obj, meta.ReadyCondition) + return readyCondition.Status == metav1.ConditionTrue && + obj.Generation == readyCondition.ObservedGeneration && + obj.Generation == obj.Status.ObservedGeneration + }, timeout).Should(BeTrue()) + + // Check if the object status is valid. + condns = &status.Conditions{NegativePolarity: helmRepositoryOCIReadyCondition.NegativePolarity} + checker = status.NewChecker(testEnv.Client, condns) + checker.CheckErr(ctx, obj) + + g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) + + // Wait for HelmRepository to be deleted + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return apierrors.IsNotFound(err) + } + return false + }, timeout).Should(BeTrue()) +} + +func TestHelmRepositoryReconciler_ReconcileSpecUpdatePredicateFilter(t *testing.T) { + g := NewWithT(t) + + testServer, err := helmtestserver.NewTempHelmServer() + g.Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(testServer.Root()) + + g.Expect(testServer.PackageChart("testdata/charts/helmchart")).To(Succeed()) + g.Expect(testServer.GenerateIndex()).To(Succeed()) + + testServer.Start() + defer testServer.Stop() + + obj := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-reconcile-", + Namespace: "default", + }, + Spec: sourcev1.HelmRepositorySpec{ + Interval: metav1.Duration{Duration: interval}, + URL: testServer.URL(), + }, + } + g.Expect(testEnv.Create(ctx, obj)).To(Succeed()) + + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + // Wait for finalizer to be set + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + return len(obj.Finalizers) > 0 + }, timeout).Should(BeTrue()) + + // Wait for HelmRepository to be Ready + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + if !conditions.IsReady(obj) && obj.Status.Artifact == nil { + return false + } + readyCondition := conditions.Get(obj, meta.ReadyCondition) + return readyCondition.Status == metav1.ConditionTrue && + obj.Generation == readyCondition.ObservedGeneration && + obj.Generation == obj.Status.ObservedGeneration + }, timeout).Should(BeTrue()) + + // Check if the object status is valid. + condns := &status.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} + checker := status.NewChecker(testEnv.Client, condns) + checker.CheckErr(ctx, obj) + + // kstatus client conformance check. + u, err := patch.ToUnstructured(obj) + g.Expect(err).ToNot(HaveOccurred()) + res, err := kstatus.Compute(u) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(res.Status).To(Equal(kstatus.CurrentStatus)) + + // Change spec Interval to validate spec update + obj.Spec.Interval = metav1.Duration{Duration: interval + time.Second} + g.Expect(testEnv.Update(ctx, obj)).To(Succeed()) + + // Wait for HelmRepository to be Ready + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + if !conditions.IsReady(obj) { + return false + } + readyCondition := conditions.Get(obj, meta.ReadyCondition) + return readyCondition.Status == metav1.ConditionTrue && + obj.Generation == readyCondition.ObservedGeneration && + obj.Generation == obj.Status.ObservedGeneration + }, timeout).Should(BeTrue()) + + // Check if the object status is valid. + condns = &status.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} + checker = status.NewChecker(testEnv.Client, condns) + checker.CheckErr(ctx, obj) + + g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) + + // Wait for HelmRepository to be deleted + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return apierrors.IsNotFound(err) + } + return false + }, timeout).Should(BeTrue()) +} diff --git a/controllers/helmrepository_predicate.go b/controllers/helmrepository_predicate.go deleted file mode 100644 index b8c1c9709..000000000 --- a/controllers/helmrepository_predicate.go +++ /dev/null @@ -1,22 +0,0 @@ -package controllers - -import ( - "sigs.k8s.io/controller-runtime/pkg/client" - - sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" -) - -func HelmRepositoryTypeFilter(typ string) func(client.Object) bool { - return func(o client.Object) bool { - if o == nil { - return false - } - - hr, ok := o.(*sourcev1.HelmRepository) - if !ok { - return false - } - - return hr.Spec.Type == typ - } -} diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 61cb7374b..288d06010 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -48,6 +48,7 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/internal/cache" + "github.com/fluxcd/source-controller/internal/helm/util" // +kubebuilder:scaffold:imports ) @@ -78,6 +79,10 @@ var ( Schemes: []string{"http", "https"}, New: getter.NewHTTPGetter, }, + getter.Provider{ + Schemes: []string{"oci"}, + New: getter.NewOCIGetter, + }, } ) @@ -194,7 +199,7 @@ func TestMain(m *testing.M) { testMetricsH = controller.MustMakeMetrics(testEnv) testRegistryserver = &RegistryClientTestServer{} - registryWorskpaceDir := SetupServer(testRegistryserver) + registryWorkspaceDir := SetupServer(testRegistryserver) testRegistryClient, err = registry.NewClient(registry.ClientOptWriter(os.Stdout)) if err != nil { @@ -230,11 +235,11 @@ func TestMain(m *testing.M) { } if err = (&HelmRepositoryOCIReconciler{ - Client: testEnv, - EventRecorder: record.NewFakeRecorder(32), - Metrics: testMetricsH, - Getters: testGetters, - RegistryClient: testRegistryClient, + Client: testEnv, + EventRecorder: record.NewFakeRecorder(32), + Metrics: testMetricsH, + Getters: testGetters, + RegistryClientGenerator: util.RegistryClientGenerator, }).SetupWithManager(testEnv); err != nil { panic(fmt.Sprintf("Failed to start HelmRepositoryOCIReconciler: %v", err)) } @@ -275,7 +280,7 @@ func TestMain(m *testing.M) { panic(fmt.Sprintf("Failed to remove storage server dir: %v", err)) } - if err := os.RemoveAll(registryWorskpaceDir); err != nil { + if err := os.RemoveAll(registryWorkspaceDir); err != nil { panic(fmt.Sprintf("Failed to remove registry workspace dir: %v", err)) } diff --git a/docs/api/source.md b/docs/api/source.md index 52c3013f2..f10fd0019 100644 --- a/docs/api/source.md +++ b/docs/api/source.md @@ -848,6 +848,19 @@ references to this object. NOTE: Not implemented, provisional as of https://github.com/fluxcd/flux2/pull/2092

+ + +type
+ +string + + + +(Optional) +

Type of the HelmRepository. +When this field is set to “oci”, the URL field value must be prefixed with “oci://”.

+ + @@ -2093,6 +2106,19 @@ references to this object. NOTE: Not implemented, provisional as of https://github.com/fluxcd/flux2/pull/2092

+ + +type
+ +string + + + +(Optional) +

Type of the HelmRepository. +When this field is set to “oci”, the URL field value must be prefixed with “oci://”.

+ + diff --git a/docs/spec/v1beta2/helmrepositories.md b/docs/spec/v1beta2/helmrepositories.md index f4dd41dfd..a77902882 100644 --- a/docs/spec/v1beta2/helmrepositories.md +++ b/docs/spec/v1beta2/helmrepositories.md @@ -1,9 +1,15 @@ # Helm Repositories -The `HelmRepository` API defines a Source to produce an Artifact for a Helm -repository index YAML (`index.yaml`). +There are 2 [Helm repository types](#type) defined by the `HelmRepository` API: +- Helm HTTP/S repository, which defines a Source to produce an Artifact for a Helm +repository index YAML (`index.yaml`). +- OCI Helm repository, which defines a source that does not produce an Artifact. +Instead a validation of the Helm repository is performed and the outcome is reported in the +`.status.conditions` field. -## Example +## Examples + +### Helm HTTP/S repository The following is an example of a HelmRepository. It creates a YAML (`.yaml`) Artifact from the fetched Helm repository index (in this example the [podinfo @@ -83,6 +89,63 @@ You can run this example by saving the manifest into `helmrepository.yaml`. Normal NewArtifact 1m source-controller fetched index of size 30.88kB from 'https://stefanprodan.github.io/podinfo' ``` +### Helm OCI repository + +The following is an example of an OCI HelmRepository. + +```yaml +--- +apiVersion: source.toolkit.fluxcd.io/v1beta2 +kind: HelmRepository +metadata: + name: podinfo + namespace: default +spec: + type: "oci" + interval: 5m0s + url: oci://ghcr.io/stefanprodan/charts +``` + +In the above example: + +- A HelmRepository named `podinfo` is created, indicated by the + `.metadata.name` field. +- The source-controller performs the Helm repository url validation i.e. the url +is a valid OCI registry url, every five minutes with the information indicated by the +`.spec.interval` and `.spec.url` fields. + +You can run this example by saving the manifest into `helmrepository.yaml`. + +1. Apply the resource on the cluster: + + ```sh + kubectl apply -f helmrepository.yaml + ``` + +2. Run `kubectl get helmrepository` to see the HelmRepository: + + ```console + NAME URL AGE READY STATUS + podinfo oci://ghcr.io/stefanprodan/charts 3m22s True Helm repository "podinfo" is ready + ``` + +3. Run `kubectl describe helmrepository podinfo` to see the [Conditions](#conditions) +in the HelmRepository's Status: + + ```console + ... + Status: + Conditions: + Last Transition Time: 2022-05-12T14:02:12Z + Message: Helm repository "podinfo" is ready + Observed Generation: 1 + Reason: Succeeded + Status: True + Type: Ready + Observed Generation: 1 + Events: + ``` + ## Writing a HelmRepository spec As with all other Kubernetes config, a HelmRepository needs `apiVersion`, @@ -92,6 +155,13 @@ valid [DNS subdomain name](https://kubernetes.io/docs/concepts/overview/working- A HelmRepository also needs a [`.spec` section](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#spec-and-status). + +### Type + +`.spec.type` is an optional field that specifies the Helm repository type. + +Possible values are `default` for a Helm HTTP/S repository, or `oci` for an OCI Helm repository. + ### Interval `.spec.interval` is a required field that specifies the interval which the @@ -107,9 +177,12 @@ change to the spec), this is handled instantly outside the interval window. ### URL -`.spec.url` is a required field that specifies the HTTP/S address of the Helm -repository. For Helm repositories which require authentication, see -[Secret reference](#secret-reference). +`.spec.url` is a required field that depending on the [type of the HelmRepository object](#type) +specifies the HTTP/S or OCI address of a Helm repository. + +For OCI, the URL is expected to point to a registry repository, e.g. `oci://ghcr.io/fluxcd/source-controller`. + +For Helm repositories which require authentication, see [Secret reference](#secret-reference). ### Timeout @@ -156,8 +229,36 @@ stringData: password: 123456 ``` +OCI Helm repository example: + +```yaml +--- +apiVersion: source.toolkit.fluxcd.io/v1beta2 +kind: HelmRepository +metadata: + name: podinfo + namespace: default +spec: + interval: 5m0s + url: oci://ghcr.io/stefanprodan/charts + type: "oci" + secretRef: + name: oci-creds +--- +apiVersion: v1 +kind: Secret +metadata: + name: oci-creds + namespace: default +stringData: + username: example + password: 123456 +``` + #### TLS authentication +**Note:** TLS authentication is not yet supported by OCI Helm repositories. + To provide TLS credentials to use while connecting with the Helm repository, the referenced Secret is expected to contain `.data.certFile` and `.data.keyFile`, and/or `.data.caFile` values. @@ -197,7 +298,8 @@ match the host as defined in URL. This may for example be required if the host advertised chart URLs in the index differ from the specified URL. Enabling this should be done with caution, as it can potentially result in -credentials getting stolen in a man-in-the-middle attack. +credentials getting stolen in a man-in-the-middle attack. This feature only applies +to HTTP/S Helm repositories. ### Suspend @@ -379,6 +481,8 @@ specific HelmRepository, e.g. `flux logs --level=error --kind=HelmRepository --n ### Artifact +**Note:** This section does not apply to [OCI Helm Repositories](#oci-helm-repositories), they do not emit artifacts. + The HelmRepository reports the last fetched repository index as an Artifact object in the `.status.artifact` of the resource. @@ -418,6 +522,9 @@ and reports `Reconciling` and `Stalled` conditions where applicable to provide better (timeout) support to solutions polling the HelmRepository to become `Ready`. + OCI Helm repositories use only `Reconciling`, `Ready`, `FetchFailed`, and `Stalled` + condition types. + #### Reconciling HelmRepository The source-controller marks a HelmRepository as _reconciling_ when one of the following diff --git a/go.mod b/go.mod index 009a6a29a..0cc5df239 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/ProtonMail/go-crypto v0.0.0-20220407094043-a94812496cf5 github.com/cyphar/filepath-securejoin v0.2.3 github.com/darkowlzz/controller-check v0.0.0-20220325122359-11f5827b7981 + github.com/distribution/distribution/v3 v3.0.0-20211118083504-a29a3c99a684 github.com/docker/go-units v0.4.0 github.com/elazarl/goproxy v0.0.0-20220417044921-416226498f94 github.com/fluxcd/gitkit v0.5.0 @@ -39,6 +40,7 @@ require ( github.com/minio/minio-go/v7 v7.0.24 github.com/onsi/gomega v1.19.0 github.com/otiai10/copy v1.7.0 + github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2 github.com/prometheus/client_golang v1.12.1 github.com/spf13/pflag v1.0.5 golang.org/x/crypto v0.0.0-20220427172511-eb4f295cb31f @@ -73,11 +75,6 @@ replace github.com/opencontainers/image-spec => github.com/opencontainers/image- // Fix CVE-2021-43816 replace github.com/containerd/containerd => github.com/containerd/containerd v1.6.1 -require ( - github.com/distribution/distribution/v3 v3.0.0-20211118083504-a29a3c99a684 - github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2 -) - require ( cloud.google.com/go v0.100.2 // indirect cloud.google.com/go/compute v1.6.0 // indirect diff --git a/hack/ci/e2e.sh b/hack/ci/e2e.sh index 4afb28fde..cbeac1d82 100755 --- a/hack/ci/e2e.sh +++ b/hack/ci/e2e.sh @@ -155,3 +155,9 @@ kubectl -n source-system wait --for=condition=ready --timeout=1m -l app=source-c echo "Re-run large libgit2 repo test with managed transport" kubectl -n source-system wait gitrepository/large-repo-libgit2 --for=condition=ready --timeout=2m15s kubectl -n source-system exec deploy/source-controller -- printenv | grep EXPERIMENTAL_GIT_TRANSPORT=true + + +echo "Run HelmChart from OCI registry tests" +kubectl -n source-system apply -f "${ROOT_DIR}/config/testdata/helmchart-from-oci/source.yaml" +kubectl -n source-system wait helmrepository/podinfo --for=condition=ready --timeout=1m +kubectl -n source-system wait helmchart/podinfo --for=condition=ready --timeout=1m diff --git a/internal/helm/chart/builder_remote.go b/internal/helm/chart/builder_remote.go index df8beb158..97de68137 100644 --- a/internal/helm/chart/builder_remote.go +++ b/internal/helm/chart/builder_remote.go @@ -37,8 +37,8 @@ import ( "github.com/fluxcd/source-controller/internal/helm/chart/secureloader" ) -// Remote is a repository.ChartRepository or a repository.OCIChartRegistry. -// It is used to download a chart from a remote repository or registry. +// Remote is a repository.ChartRepository or a repository.OCIChartRepository. +// It is used to download a chart from a remote Helm repository or OCI registry. type Remote interface { // GetChart returns a chart.Chart from the remote repository. Get(name, version string) (*repo.ChartVersion, error) @@ -153,46 +153,20 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o } func (b *remoteChartBuilder) downloadFromOCIRepository(remote *repository.OCIChartRepository, remoteRef RemoteReference, buildResult *Build, opts BuildOptions) (*bytes.Buffer, error) { - // TODO: verify if login is required cv, err := remote.Get(remoteRef.Name, remoteRef.Version) if err != nil { err = fmt.Errorf("failed to get chart version for remote reference: %w", err) return nil, &BuildError{Reason: ErrChartPull, Err: err} } - result := &Build{} - result.Version = cv.Version - result.Name = cv.Name - - // Set build specific metadata if instructed - if opts.VersionMetadata != "" { - ver, err := setBuildMetaData(result.Version, opts.VersionMetadata) - if err != nil { - return nil, &BuildError{Reason: ErrChartMetadataPatch, Err: err} - } - result.Version = ver.String() + result, shouldReturn, err := generateBuildResult(cv, opts) + if err != nil { + return nil, err } - requiresPackaging := len(opts.GetValuesFiles()) != 0 || opts.VersionMetadata != "" - - // If all the following is true, we do not need to download and/or build the chart: - // - Chart name from cached chart matches resolved name - // - Chart version from cached chart matches calculated version - // - BuildOptions.Force is False - if opts.CachedChart != "" && !opts.Force { - if curMeta, err := LoadChartMetadataFromArchive(opts.CachedChart); err == nil { - // If the cached metadata is corrupt, we ignore its existence - // and continue the build - if err = curMeta.Validate(); err == nil { - if result.Name == curMeta.Name && result.Version == curMeta.Version { - result.Path = opts.CachedChart - result.ValuesFiles = opts.GetValuesFiles() - result.Packaged = requiresPackaging - *buildResult = *result - return nil, nil - } - } - } + if shouldReturn { + *buildResult = *result + return nil, nil } // Download the package for the resolved version @@ -221,15 +195,38 @@ func (b *remoteChartBuilder) downloadFromRepository(remote *repository.ChartRepo return nil, &BuildError{Reason: ErrChartReference, Err: err} } + result, shouldReturn, err := generateBuildResult(cv, opts) + if err != nil { + return nil, err + } + + if shouldReturn { + *buildResult = *result + return nil, nil + } + + // Download the package for the resolved version + res, err := remote.DownloadChart(cv) + if err != nil { + err = fmt.Errorf("failed to download chart for remote reference: %w", err) + return nil, &BuildError{Reason: ErrChartPull, Err: err} + } + + *buildResult = *result + + return res, nil +} + +func generateBuildResult(cv *repo.ChartVersion, opts BuildOptions) (*Build, bool, error) { result := &Build{} - result.Name = cv.Name result.Version = cv.Version + result.Name = cv.Name // Set build specific metadata if instructed if opts.VersionMetadata != "" { ver, err := setBuildMetaData(result.Version, opts.VersionMetadata) if err != nil { - return nil, &BuildError{Reason: ErrChartMetadataPatch, Err: err} + return nil, false, &BuildError{Reason: ErrChartMetadataPatch, Err: err} } result.Version = ver.String() } @@ -249,23 +246,13 @@ func (b *remoteChartBuilder) downloadFromRepository(remote *repository.ChartRepo result.Path = opts.CachedChart result.ValuesFiles = opts.GetValuesFiles() result.Packaged = requiresPackaging - *buildResult = *result - return nil, nil + return result, true, nil } } } } - // Download the package for the resolved version - res, err := remote.DownloadChart(cv) - if err != nil { - err = fmt.Errorf("failed to download chart for remote reference: %w", err) - return nil, &BuildError{Reason: ErrChartPull, Err: err} - } - - *buildResult = *result - - return res, nil + return result, false, nil } func setBuildMetaData(version, versionMetadata string) (*semver.Version, error) { diff --git a/internal/helm/chart/builder_remote_test.go b/internal/helm/chart/builder_remote_test.go index f1b669bff..e76503e43 100644 --- a/internal/helm/chart/builder_remote_test.go +++ b/internal/helm/chart/builder_remote_test.go @@ -19,6 +19,8 @@ package chart import ( "bytes" "context" + "fmt" + "net/url" "os" "path/filepath" "strings" @@ -29,11 +31,35 @@ import ( helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" helmgetter "helm.sh/helm/v3/pkg/getter" + "helm.sh/helm/v3/pkg/registry" "github.com/fluxcd/source-controller/internal/helm/chart/secureloader" "github.com/fluxcd/source-controller/internal/helm/repository" ) +type mockRegistryClient struct { + tags map[string][]string + requestedURL string +} + +func (m *mockRegistryClient) Tags(url string) ([]string, error) { + m.requestedURL = url + if tags, ok := m.tags[url]; ok { + return tags, nil + } + return nil, fmt.Errorf("no tags found for %s", url) +} + +func (m *mockRegistryClient) Login(url string, opts ...registry.LoginOption) error { + m.requestedURL = url + return nil +} + +func (m *mockRegistryClient) Logout(url string, opts ...registry.LogoutOption) error { + m.requestedURL = url + return nil +} + // mockIndexChartGetter returns specific response for index and chart queries. type mockIndexChartGetter struct { IndexResponse []byte @@ -54,7 +80,7 @@ func (g *mockIndexChartGetter) LastGet() string { return g.requestedURL } -func TestRemoteBuilder_Build(t *testing.T) { +func TestRemoteBuilder__BuildFromChartRepository(t *testing.T) { g := NewWithT(t) chartGrafana, err := os.ReadFile("./../testdata/charts/helmchart-0.1.0.tgz") @@ -195,6 +221,140 @@ entries: } } +func TestRemoteBuilder_BuildFromOCIChatRepository(t *testing.T) { + g := NewWithT(t) + + chartGrafana, err := os.ReadFile("./../testdata/charts/helmchart-0.1.0.tgz") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(chartGrafana).ToNot(BeEmpty()) + + registryClient := &mockRegistryClient{ + tags: map[string][]string{ + "localhost:5000/my_repo/grafana": {"6.17.4"}, + }, + } + + mockGetter := &mockIndexChartGetter{ + ChartResponse: chartGrafana, + } + + u, err := url.Parse("oci://localhost:5000/my_repo") + g.Expect(err).ToNot(HaveOccurred()) + + mockRepo := func() *repository.OCIChartRepository { + return &repository.OCIChartRepository{ + URL: *u, + Client: mockGetter, + RegistryClient: registryClient, + } + } + + tests := []struct { + name string + reference Reference + buildOpts BuildOptions + repository *repository.OCIChartRepository + wantValues chartutil.Values + wantVersion string + wantPackaged bool + wantErr string + }{ + { + name: "invalid reference", + reference: LocalReference{}, + wantErr: "expected remote chart reference", + }, + { + name: "invalid reference - no name", + reference: RemoteReference{}, + wantErr: "no name set for remote chart reference", + }, + { + name: "chart not in repository", + reference: RemoteReference{Name: "foo"}, + repository: mockRepo(), + wantErr: "failed to get chart version for remote reference", + }, + { + name: "chart version not in repository", + reference: RemoteReference{Name: "grafana", Version: "1.1.1"}, + repository: mockRepo(), + wantErr: "failed to get chart version for remote reference", + }, + { + name: "invalid version metadata", + reference: RemoteReference{Name: "grafana"}, + repository: mockRepo(), + buildOpts: BuildOptions{VersionMetadata: "^"}, + wantErr: "Invalid Metadata string", + }, + { + name: "with version metadata", + reference: RemoteReference{Name: "grafana"}, + repository: mockRepo(), + buildOpts: BuildOptions{VersionMetadata: "foo"}, + wantVersion: "6.17.4+foo", + wantPackaged: true, + }, + { + name: "default values", + reference: RemoteReference{Name: "grafana"}, + repository: mockRepo(), + wantVersion: "0.1.0", + wantValues: chartutil.Values{ + "replicaCount": float64(1), + }, + }, + { + name: "merge values", + reference: RemoteReference{Name: "grafana"}, + buildOpts: BuildOptions{ + ValuesFiles: []string{"a.yaml", "b.yaml", "c.yaml"}, + }, + repository: mockRepo(), + wantVersion: "6.17.4", + wantValues: chartutil.Values{ + "a": "b", + "b": "d", + }, + wantPackaged: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + tmpDir, err := os.MkdirTemp("", "remote-chart-builder-") + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmpDir) + targetPath := filepath.Join(tmpDir, "chart.tgz") + + b := NewRemoteBuilder(tt.repository) + + cb, err := b.Build(context.TODO(), tt.reference, targetPath, tt.buildOpts) + + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + g.Expect(cb).To(BeZero()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cb.Packaged).To(Equal(tt.wantPackaged), "unexpected Build.Packaged value") + g.Expect(cb.Path).ToNot(BeEmpty(), "empty Build.Path") + + // Load the resulting chart and verify the values. + resultChart, err := secureloader.LoadFile(cb.Path) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(resultChart.Metadata.Version).To(Equal(tt.wantVersion)) + + for k, v := range tt.wantValues { + g.Expect(v).To(Equal(resultChart.Values[k])) + } + }) + } +} + func TestRemoteBuilder_Build_CachedChart(t *testing.T) { g := NewWithT(t) diff --git a/internal/helm/repository/oci_chart_repository.go b/internal/helm/repository/oci_chart_repository.go index 61aeee438..af987c35c 100644 --- a/internal/helm/repository/oci_chart_repository.go +++ b/internal/helm/repository/oci_chart_repository.go @@ -1,5 +1,5 @@ /* -Copyright 2020 The Flux authors +Copyright 2022 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. @@ -21,6 +21,7 @@ import ( "crypto/tls" "fmt" "net/url" + "sort" "strings" "helm.sh/helm/v3/pkg/chart" @@ -28,26 +29,36 @@ import ( "helm.sh/helm/v3/pkg/registry" "helm.sh/helm/v3/pkg/repo" + "github.com/Masterminds/semver/v3" + "github.com/fluxcd/pkg/version" "github.com/fluxcd/source-controller/internal/transport" ) +// RegistryClient is an interface for interacting with OCI registries +// It is used by the OCIChartRepository to retrieve chart versions +// from OCI registries +type RegistryClient interface { + Login(host string, opts ...registry.LoginOption) error + Logout(host string, opts ...registry.LogoutOption) error + Tags(url string) ([]string, error) +} + // OCIChartRepository represents a Helm chart repository, and the configuration // required to download the repository tags and charts from the repository. // All methods are thread safe unless defined otherwise. type OCIChartRepository struct { - // URL the ChartRepository's index.yaml can be found at, - // without the index.yaml suffix. + // URL is the location of the repository. URL url.URL - // Client to use while downloading the Index or a chart from the URL. + // Client to use while accessing the repository's contents. Client getter.Getter - // Options to configure the Client with while downloading the Index + // Options to configure the Client with while downloading tags // or a chart from the URL. Options []getter.Option tlsConfig *tls.Config // RegistryClient is a client to use while downloading tags or charts from a registry. - RegistryClient *registry.Client + RegistryClient RegistryClient } // OCIChartRepositoryOption is a function that can be passed to NewOCIChartRepository @@ -55,7 +66,7 @@ type OCIChartRepository struct { type OCIChartRepositoryOption func(*OCIChartRepository) error // WithOCIRegistryClient returns a ChartRepositoryOption that will set the registry client -func WithOCIRegistryClient(client *registry.Client) OCIChartRepositoryOption { +func WithOCIRegistryClient(client RegistryClient) OCIChartRepositoryOption { return func(r *OCIChartRepository) error { r.RegistryClient = client return nil @@ -82,29 +93,17 @@ func WithOCIGetterOptions(getterOpts []getter.Option) OCIChartRepositoryOption { } } -// WithOCITlsConfig returns a ChartRepositoryOption that will set the tls.Config -func WithTlsConfig(tlsConfig *tls.Config) OCIChartRepositoryOption { - return func(r *OCIChartRepository) error { - r.tlsConfig = tlsConfig - return nil - } -} - // NewOCIChartRepository constructs and returns a new ChartRepository with // the ChartRepository.Client configured to the getter.Getter for the -// repository URL scheme. It returns an error on URL parsing failures, -// or if there is no getter available for the scheme. +// repository URL scheme. It returns an error on URL parsing failures. +// It assumes that the url scheme has been validated to be an OCI scheme. func NewOCIChartRepository(repositoryURL string, chartRepoOpts ...OCIChartRepositoryOption) (*OCIChartRepository, error) { u, err := url.Parse(repositoryURL) if err != nil { return nil, err } - if !registry.IsOCI(repositoryURL) { - return nil, fmt.Errorf("the url scheme is not supported: %s", u.Scheme) - } - - r := newOCIChartRepository() + r := &OCIChartRepository{} r.URL = *u for _, opt := range chartRepoOpts { if err := opt(r); err != nil { @@ -115,10 +114,6 @@ func NewOCIChartRepository(repositoryURL string, chartRepoOpts ...OCIChartReposi return r, nil } -func newOCIChartRepository() *OCIChartRepository { - return &OCIChartRepository{} -} - // Get returns the repo.ChartVersion for the given name, the version is expected // to be a semver.Constraints compatible string. If version is empty, the latest // stable version will be returned and prerelease versions will be ignored. @@ -139,7 +134,7 @@ func (r *OCIChartRepository) Get(name, ver string) (*repo.ChartVersion, error) { // If empty, try to get the highest available tag // If exact version, try to find it // If semver constraint string, try to find a match - tag, err := registry.GetTagMatchingVersionOrConstraint(cvs, ver) + tag, err := getLastMatchingVersionOrConstraint(cvs, ver) return &repo.ChartVersion{ URLs: []string{fmt.Sprintf("%s/%s:%s", r.URL.String(), name, tag)}, Metadata: &chart.Metadata{ @@ -149,7 +144,7 @@ func (r *OCIChartRepository) Get(name, ver string) (*repo.ChartVersion, error) { }, err } -// this function shall be called for OCI registries only +// This function shall be called for OCI registries only // It assumes that the ref has been validated to be an OCI reference. func (r *OCIChartRepository) getTags(ref string) ([]string, error) { // Retrieve list of repository tags @@ -187,3 +182,71 @@ func (r *OCIChartRepository) DownloadChart(chart *repo.ChartVersion) (*bytes.Buf // trim the oci scheme prefix if needed return r.Client.Get(strings.TrimPrefix(u.String(), fmt.Sprintf("%s://", registry.OCIScheme)), clientOpts...) } + +// Login attempts to login to the OCI registry. +// It returns an error on failure. +func (r *OCIChartRepository) Login(opts ...registry.LoginOption) error { + err := r.RegistryClient.Login(r.URL.Host, opts...) + if err != nil { + return err + } + return nil +} + +// Logout attempts to logout from the OCI registry. +// It returns an error on failure. +func (r *OCIChartRepository) Logout() error { + err := r.RegistryClient.Logout(r.URL.Host) + if err != nil { + return err + } + return nil +} + +// getLastMatchingVersionOrConstraint returns the last version that matches the given version string. +// If the version string is empty, the highest available version is returned. +func getLastMatchingVersionOrConstraint(cvs []string, ver string) (string, error) { + // Check for exact matches first + if ver != "" { + for _, cv := range cvs { + if ver == cv { + return cv, nil + } + } + } + + // Continue to look for a (semantic) version match + verConstraint, err := semver.NewConstraint("*") + if err != nil { + return "", err + } + latestStable := ver == "" || ver == "*" + if !latestStable { + verConstraint, err = semver.NewConstraint(ver) + if err != nil { + return "", err + } + } + + matchingVersions := make([]string, 0, len(cvs)) + for _, cv := range cvs { + v, err := version.ParseVersion(cv) + if err != nil { + continue + } + + if !verConstraint.Check(v) { + continue + } + + matchingVersions = append(matchingVersions, cv) + } + if len(matchingVersions) == 0 { + return "", fmt.Errorf("could not locate a version matching provided version string %s", ver) + } + + // Sort versions + sort.Sort(sort.Reverse(sort.StringSlice(matchingVersions))) + + return matchingVersions[0], nil +} diff --git a/internal/helm/repository/oci_chart_repository_test.go b/internal/helm/repository/oci_chart_repository_test.go new file mode 100644 index 000000000..140416537 --- /dev/null +++ b/internal/helm/repository/oci_chart_repository_test.go @@ -0,0 +1,238 @@ +/* +Copyright 2022 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 repository + +import ( + "bytes" + "fmt" + "net/url" + "strings" + "testing" + + . "github.com/onsi/gomega" + "helm.sh/helm/v3/pkg/chart" + helmgetter "helm.sh/helm/v3/pkg/getter" + "helm.sh/helm/v3/pkg/registry" + "helm.sh/helm/v3/pkg/repo" +) + +type OCIMockGetter struct { + Response []byte + LastCalledURL string +} + +func (g *OCIMockGetter) Get(u string, _ ...helmgetter.Option) (*bytes.Buffer, error) { + r := g.Response + g.LastCalledURL = u + return bytes.NewBuffer(r), nil +} + +type mockRegistryClient struct { + tags []string + LastCalledURL string +} + +func (m *mockRegistryClient) Tags(url string) ([]string, error) { + m.LastCalledURL = url + return m.tags, nil +} + +func (m *mockRegistryClient) Login(url string, opts ...registry.LoginOption) error { + m.LastCalledURL = url + return nil +} + +func (m *mockRegistryClient) Logout(url string, opts ...registry.LogoutOption) error { + m.LastCalledURL = url + return nil +} + +func TestNewOCIChartRepository(t *testing.T) { + registryClient := &mockRegistryClient{} + url := "oci://localhost:5000/my_repo" + providers := helmgetter.Providers{ + helmgetter.Provider{ + Schemes: []string{"oci"}, + New: helmgetter.NewOCIGetter, + }, + } + options := []helmgetter.Option{helmgetter.WithBasicAuth("username", "password")} + t.Run("should construct chart registry", func(t *testing.T) { + g := NewWithT(t) + r, err := NewOCIChartRepository(url, WithOCIGetter(providers), WithOCIGetterOptions(options), WithOCIRegistryClient(registryClient)) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(r).ToNot(BeNil()) + g.Expect(r.URL.Host).To(Equal("localhost:5000")) + g.Expect(r.Client).ToNot(BeNil()) + g.Expect(r.Options).To(Equal(options)) + g.Expect(r.RegistryClient).To(Equal(registryClient)) + }) + + t.Run("should return error on invalid url", func(t *testing.T) { + g := NewWithT(t) + r, err := NewOCIChartRepository("oci://localhost:5000 /my_repo", WithOCIGetter(providers), WithOCIGetterOptions(options), WithOCIRegistryClient(registryClient)) + g.Expect(err).To(HaveOccurred()) + g.Expect(r).To(BeNil()) + }) + +} + +func TestOCIChartRepoisitory_Get(t *testing.T) { + registryClient := &mockRegistryClient{ + tags: []string{ + "0.0.1", + "0.1.0", + "0.1.1", + "0.1.5+b.min.minute", + "0.1.5+a.min.hour", + "0.1.5+c.now", + "0.2.0", + "1.0.0", + "1.1.0-rc.1", + }, + } + + providers := helmgetter.Providers{ + helmgetter.Provider{ + Schemes: []string{"oci"}, + New: helmgetter.NewOCIGetter, + }, + } + + testCases := []struct { + name string + version string + expected string + expectedErr string + }{ + { + name: "should return latest stable version", + version: "", + expected: "1.0.0", + }, + { + name: "should return latest stable version (asterisk)", + version: "*", + expected: "1.0.0", + }, + { + name: "should return latest stable version (semver range)", + version: ">=0.1.5", + expected: "1.0.0", + }, + { + name: "should return 0.2.0 (semver range)", + version: "0.2.x", + expected: "0.2.0", + }, + { + name: "should return a perfect match", + version: "0.1.0", + expected: "0.1.0", + }, + { + name: "should an error for unfunfilled range", + version: ">2.0.0", + expectedErr: "could not locate a version matching provided version string >2.0.0", + }, + } + + url := "oci://localhost:5000/my_repo" + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + r, err := NewOCIChartRepository(url, WithOCIRegistryClient(registryClient), WithOCIGetter(providers)) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(r).ToNot(BeNil()) + + chart := "podinfo" + cv, err := r.Get(chart, tc.version) + if tc.expectedErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(Equal(tc.expectedErr)) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cv.URLs[0]).To(Equal(fmt.Sprintf("%s/%s:%s", url, chart, tc.expected))) + g.Expect(registryClient.LastCalledURL).To(Equal(fmt.Sprintf("%s/%s", strings.TrimPrefix(url, fmt.Sprintf("%s://", registry.OCIScheme)), chart))) + }) + } +} + +func TestOCIChartRepoisitory_DownloadChart(t *testing.T) { + client := &mockRegistryClient{} + testCases := []struct { + name string + url string + chartVersion *repo.ChartVersion + expected string + expectedErr bool + }{ + { + name: "should download chart", + url: "oci://localhost:5000/my_repo", + chartVersion: &repo.ChartVersion{ + Metadata: &chart.Metadata{Name: "chart"}, + URLs: []string{"oci://localhost:5000/my_repo/podinfo:1.0.0"}, + }, + expected: "oci://localhost:5000/my_repo/podinfo:1.0.0", + }, + { + name: "no chart URL", + url: "", + chartVersion: &repo.ChartVersion{Metadata: &chart.Metadata{Name: "chart"}}, + expectedErr: true, + }, + { + name: "invalid chart URL", + url: "oci://localhost:5000/my_repo", + chartVersion: &repo.ChartVersion{ + Metadata: &chart.Metadata{Name: "chart"}, + URLs: []string{"oci://localhost:5000 /my_repo/podinfo:1.0.0"}, + }, + expectedErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + t.Parallel() + mg := OCIMockGetter{} + u, err := url.Parse(tc.url) + g.Expect(err).ToNot(HaveOccurred()) + r := OCIChartRepository{ + Client: &mg, + URL: *u, + } + r.Client = &mg + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(r).ToNot(BeNil()) + res, err := r.DownloadChart(tc.chartVersion) + if tc.expectedErr { + g.Expect(err).To(HaveOccurred()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(client.LastCalledURL).To(Equal(tc.expected)) + g.Expect(res).ToNot(BeNil()) + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} diff --git a/internal/helm/repository/utils.go b/internal/helm/repository/utils.go index b02b13782..1abc9dffb 100644 --- a/internal/helm/repository/utils.go +++ b/internal/helm/repository/utils.go @@ -16,7 +16,9 @@ limitations under the License. package repository -import "strings" +import ( + "strings" +) // NormalizeURL normalizes a ChartRepository URL by ensuring it ends with a // single "/". diff --git a/internal/helm/util/client.go b/internal/helm/util/client.go new file mode 100644 index 000000000..1bd8944f6 --- /dev/null +++ b/internal/helm/util/client.go @@ -0,0 +1,50 @@ +/* +Copyright 2022 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 util + +import ( + "io" + "os" + + "helm.sh/helm/v3/pkg/registry" +) + +// RegistryClientGenerator generates a registry client and a temporary credential file. +// The client is meant to be used for a single reconciliation. +// The file is meant to be used for a single reconciliation and deleted after. +func RegistryClientGenerator(isLogin bool) (*registry.Client, string, error) { + if isLogin { + // create a temporary file to store the credentials + // this is needed because otherwise the credentials are stored in ~/.docker/config.json. + credentialFile, err := os.CreateTemp("", "credentials") + if err != nil { + return nil, "", err + } + + rClient, err := registry.NewClient(registry.ClientOptWriter(io.Discard), registry.ClientOptCredentialsFile(credentialFile.Name())) + if err != nil { + return nil, "", err + } + return rClient, credentialFile.Name(), nil + } + + rClient, err := registry.NewClient(registry.ClientOptWriter(io.Discard)) + if err != nil { + return nil, "", err + } + return rClient, "", nil +} diff --git a/internal/predicates/helmrepository_type_predicate.go b/internal/predicates/helmrepository_type_predicate.go new file mode 100644 index 000000000..76694b82f --- /dev/null +++ b/internal/predicates/helmrepository_type_predicate.go @@ -0,0 +1,86 @@ +/* +Copyright 2022 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 predicates + +import ( + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" +) + +// helmRepositoryTypeFilter filters events for a given HelmRepository type. +// It returns true if the event is for a HelmRepository of the given type. +func helmRepositoryTypeFilter(repositoryType string, o client.Object) bool { + if o == nil { + return false + } + + // return true if the object is a HelmRepository + // and the type is the same as the one we are looking for. + hr, ok := o.(*sourcev1.HelmRepository) + if !ok { + return false + } + + return hr.Spec.Type == repositoryType +} + +// HelmRepositoryTypePredicate is a predicate that filters events for a given HelmRepository type. +type HelmRepositoryTypePredicate struct { + RepositoryType string + predicate.Funcs +} + +// Create returns true if the Create event is for a HelmRepository of the given type. +func (h HelmRepositoryTypePredicate) Create(e event.CreateEvent) bool { + return helmRepositoryTypeFilter(h.RepositoryType, e.Object) +} + +// Update returns true if the Update event is for a HelmRepository of the given type. +func (h HelmRepositoryTypePredicate) Update(e event.UpdateEvent) bool { + if e.ObjectOld == nil || e.ObjectNew == nil { + return false + } + + // check if the old object is a HelmRepository + oldObj, ok := e.ObjectOld.(*sourcev1.HelmRepository) + if !ok { + return false + } + + // check if the new object is a HelmRepository + newObj, ok := e.ObjectNew.(*sourcev1.HelmRepository) + if !ok { + return false + } + + isOfRepositoryType := newObj.Spec.Type == h.RepositoryType + wasOfRepositoryType := oldObj.Spec.Type == h.RepositoryType && !isOfRepositoryType + return isOfRepositoryType || wasOfRepositoryType +} + +// Delete returns true if the Delete event is for a HelmRepository of the given type. +func (h HelmRepositoryTypePredicate) Delete(e event.DeleteEvent) bool { + return helmRepositoryTypeFilter(h.RepositoryType, e.Object) +} + +// Generic returns true if the Generic event is for a HelmRepository of the given type. +func (h HelmRepositoryTypePredicate) Generic(e event.GenericEvent) bool { + return helmRepositoryTypeFilter(h.RepositoryType, e.Object) +} diff --git a/internal/predicates/helmrepository_type_predicate_test.go b/internal/predicates/helmrepository_type_predicate_test.go new file mode 100644 index 000000000..e54726892 --- /dev/null +++ b/internal/predicates/helmrepository_type_predicate_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2022 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 predicates + +import ( + "testing" + + sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" + "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" +) + +func TestHelmRepositoryTypePredicate_Create(t *testing.T) { + obj := &sourcev1.HelmRepository{Spec: sourcev1.HelmRepositorySpec{}} + http := &sourcev1.HelmRepository{Spec: sourcev1.HelmRepositorySpec{Type: "default"}} + oci := &sourcev1.HelmRepository{Spec: sourcev1.HelmRepositorySpec{Type: "oci"}} + not := &unstructured.Unstructured{} + + tests := []struct { + name string + obj client.Object + want bool + }{ + {name: "new", obj: obj, want: false}, + {name: "http", obj: http, want: true}, + {name: "oci", obj: oci, want: false}, + {name: "not a HelmRepository", obj: not, want: false}, + {name: "nil", obj: nil, want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := gomega.NewWithT(t) + + so := HelmRepositoryTypePredicate{RepositoryType: "default"} + e := event.CreateEvent{ + Object: tt.obj, + } + g.Expect(so.Create(e)).To(gomega.Equal(tt.want)) + }) + } +} + +func TestHelmRepositoryTypePredicate_Update(t *testing.T) { + repoA := &sourcev1.HelmRepository{Spec: sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeDefault, + }} + + repoB := &sourcev1.HelmRepository{Spec: sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeOCI, + }} + + empty := &sourcev1.HelmRepository{} + not := &unstructured.Unstructured{} + + tests := []struct { + name string + old client.Object + new client.Object + want bool + }{ + {name: "diff type", old: repoA, new: repoB, want: true}, + {name: "new with type", old: empty, new: repoA, want: true}, + {name: "old with type", old: repoA, new: empty, want: true}, + {name: "old not a HelmRepository", old: not, new: repoA, want: false}, + {name: "new not a HelmRepository", old: repoA, new: not, want: false}, + {name: "old nil", old: nil, new: repoA, want: false}, + {name: "new nil", old: repoA, new: nil, want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := gomega.NewWithT(t) + + so := HelmRepositoryTypePredicate{RepositoryType: "default"} + e := event.UpdateEvent{ + ObjectOld: tt.old, + ObjectNew: tt.new, + } + g.Expect(so.Update(e)).To(gomega.Equal(tt.want)) + }) + } +} + +func TestHelmRepositoryTypePredicate_Delete(t *testing.T) { + obj := &sourcev1.HelmRepository{Spec: sourcev1.HelmRepositorySpec{}} + http := &sourcev1.HelmRepository{Spec: sourcev1.HelmRepositorySpec{Type: "default"}} + oci := &sourcev1.HelmRepository{Spec: sourcev1.HelmRepositorySpec{Type: "oci"}} + not := &unstructured.Unstructured{} + + tests := []struct { + name string + obj client.Object + want bool + }{ + {name: "new", obj: obj, want: false}, + {name: "http", obj: http, want: true}, + {name: "oci", obj: oci, want: false}, + {name: "not a HelmRepository", obj: not, want: false}, + {name: "nil", obj: nil, want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := gomega.NewWithT(t) + + so := HelmRepositoryTypePredicate{RepositoryType: "default"} + e := event.DeleteEvent{ + Object: tt.obj, + } + g.Expect(so.Delete(e)).To(gomega.Equal(tt.want)) + }) + } +} diff --git a/main.go b/main.go index 28a024a7b..88f4ad2d0 100644 --- a/main.go +++ b/main.go @@ -18,7 +18,6 @@ package main import ( "fmt" - "io" "net" "net/http" "os" @@ -28,7 +27,6 @@ import ( "github.com/go-logr/logr" flag "github.com/spf13/pflag" "helm.sh/helm/v3/pkg/getter" - "helm.sh/helm/v3/pkg/registry" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -44,6 +42,7 @@ import ( "github.com/fluxcd/pkg/runtime/pprof" "github.com/fluxcd/pkg/runtime/probes" "github.com/fluxcd/source-controller/internal/features" + "github.com/fluxcd/source-controller/internal/helm/util" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/controllers" @@ -234,20 +233,13 @@ func main() { os.Exit(1) } - rClient, err := registry.NewClient(registry.ClientOptWriter(io.Discard)) - if err != nil { - setupLog.Error(err, "unable to create OCI registry client") - os.Exit(1) - } - if err = (&controllers.HelmRepositoryOCIReconciler{ - Client: mgr.GetClient(), - EventRecorder: eventRecorder, - Metrics: metricsH, - // Storage: storage, - Getters: getters, - ControllerName: controllerName, - RegistryClient: rClient, + Client: mgr.GetClient(), + EventRecorder: eventRecorder, + Metrics: metricsH, + Getters: getters, + ControllerName: controllerName, + RegistryClientGenerator: util.RegistryClientGenerator, }).SetupWithManagerAndOptions(mgr, controllers.HelmRepositoryReconcilerOptions{ MaxConcurrentReconciles: concurrent, RateLimiter: helper.GetRateLimiter(rateLimiterOptions), @@ -277,16 +269,16 @@ func main() { cacheRecorder := cache.MustMakeMetrics() if err = (&controllers.HelmChartReconciler{ - Client: mgr.GetClient(), - RegistryClient: rClient, - Storage: storage, - Getters: getters, - EventRecorder: eventRecorder, - Metrics: metricsH, - ControllerName: controllerName, - Cache: c, - TTL: ttl, - CacheRecorder: cacheRecorder, + Client: mgr.GetClient(), + RegistryClientGenerator: util.RegistryClientGenerator, + Storage: storage, + Getters: getters, + EventRecorder: eventRecorder, + Metrics: metricsH, + ControllerName: controllerName, + Cache: c, + TTL: ttl, + CacheRecorder: cacheRecorder, }).SetupWithManagerAndOptions(mgr, controllers.HelmChartReconcilerOptions{ MaxConcurrentReconciles: concurrent, RateLimiter: helper.GetRateLimiter(rateLimiterOptions),