diff --git a/api/v1beta2/helmrepository_types.go b/api/v1beta2/helmrepository_types.go index e1df71568..1c25a9ebe 100644 --- a/api/v1beta2/helmrepository_types.go +++ b/api/v1beta2/helmrepository_types.go @@ -44,6 +44,7 @@ const ( type HelmRepositorySpec struct { // URL of the Helm repository, a valid URL contains at least a protocol and // host. + // +kubebuilder:validation:Pattern="^(http|https|oci)://.*$" // +required URL string `json:"url"` @@ -88,13 +89,13 @@ type HelmRepositorySpec struct { // efficient use of resources. // +kubebuilder:validation:Type=string // +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ms|s|m|h))+$" - // +required - Interval metav1.Duration `json:"interval"` + // +optional + Interval metav1.Duration `json:"interval,omitempty"` // Timeout is used for the index fetch operation for an HTTPS helm repository, - // and for remote OCI Repository operations like pulling for an OCI helm repository. + // and for remote OCI Repository operations like pulling for an OCI helm + // chart by the associated HelmChart. // Its default value is 60s. - // +kubebuilder:default:="60s" // +kubebuilder:validation:Type=string // +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ms|s|m))+$" // +optional @@ -169,7 +170,19 @@ func (in *HelmRepository) SetConditions(conditions []metav1.Condition) { // GetRequeueAfter returns the duration after which the source must be // reconciled again. func (in HelmRepository) GetRequeueAfter() time.Duration { - return in.Spec.Interval.Duration + if in.Spec.Interval.Duration != 0 { + return in.Spec.Interval.Duration + } + return time.Minute +} + +// GetTimeout returns the timeout duration used for various operations related +// to this HelmRepository. +func (in HelmRepository) GetTimeout() time.Duration { + if in.Spec.Timeout != nil { + return in.Spec.Timeout.Duration + } + return time.Minute } // GetArtifact returns the latest artifact from the source if present in the diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml index 6de6911d8..a17ab56d9 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml @@ -357,10 +357,10 @@ spec: of this HelmRepository. type: boolean timeout: - default: 60s description: Timeout is used for the index fetch operation for an HTTPS helm repository, and for remote OCI Repository operations - like pulling for an OCI helm repository. Its default value is 60s. + like pulling for an OCI helm chart by the associated HelmChart. + Its default value is 60s. pattern: ^([0-9]+(\.[0-9]+)?(ms|s|m))+$ type: string type: @@ -373,9 +373,9 @@ spec: url: description: URL of the Helm repository, a valid URL contains at least a protocol and host. + pattern: ^(http|https|oci)://.*$ type: string required: - - interval - url type: object status: diff --git a/docs/api/v1beta2/source.md b/docs/api/v1beta2/source.md index edfa29a5b..46649e233 100644 --- a/docs/api/v1beta2/source.md +++ b/docs/api/v1beta2/source.md @@ -866,6 +866,7 @@ Kubernetes meta/v1.Duration +(Optional)

Interval at which the HelmRepository URL is checked for updates. This interval is approximate and may be subject to jitter to ensure efficient use of resources.

@@ -883,7 +884,8 @@ Kubernetes meta/v1.Duration (Optional)

Timeout is used for the index fetch operation for an HTTPS helm repository, -and for remote OCI Repository operations like pulling for an OCI helm repository. +and for remote OCI Repository operations like pulling for an OCI helm +chart by the associated HelmChart. Its default value is 60s.

@@ -2583,6 +2585,7 @@ Kubernetes meta/v1.Duration +(Optional)

Interval at which the HelmRepository URL is checked for updates. This interval is approximate and may be subject to jitter to ensure efficient use of resources.

@@ -2600,7 +2603,8 @@ Kubernetes meta/v1.Duration (Optional)

Timeout is used for the index fetch operation for an HTTPS helm repository, -and for remote OCI Repository operations like pulling for an OCI helm repository. +and for remote OCI Repository operations like pulling for an OCI helm +chart by the associated HelmChart. Its default value is 60s.

diff --git a/docs/spec/v1beta2/helmrepositories.md b/docs/spec/v1beta2/helmrepositories.md index ad9e736e0..a05155ebb 100644 --- a/docs/spec/v1beta2/helmrepositories.md +++ b/docs/spec/v1beta2/helmrepositories.md @@ -5,9 +5,9 @@ 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. +- OCI Helm repository, which defines a source that does not produce an Artifact. + It's a data container to store the information about the OCI repository that + can be used by [HelmChart](helmcharts.md) to access OCI Helm charts. ## Examples @@ -113,9 +113,11 @@ 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. +- A HelmChart that refers to this HelmRepository uses the URL in the `.spec.url` + field to access the OCI Helm chart. + +**NOTE:** The `.spec.interval` field is only used by the `default` Helm +repository and is ignored for any value in `oci` Helm repository. You can run this example by saving the manifest into `helmrepository.yaml`. @@ -129,25 +131,12 @@ You can run this example by saving the manifest into `helmrepository.yaml`. ```console NAME URL AGE READY STATUS - podinfo oci://ghcr.io/stefanprodan/charts 3m22s True Helm repository "podinfo" is ready + podinfo oci://ghcr.io/stefanprodan/charts 3m22s ``` -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: - ``` +Because the OCI Helm repository is a data container, there's nothing to report +for `READY` and `STATUS` columns above. The existence of the object can be +considered to be ready for use. ## Writing a HelmRepository spec @@ -360,8 +349,12 @@ for more information about setting up GKE Workload Identity. ### Interval -`.spec.interval` is a required field that specifies the interval which the -Helm repository index must be consulted at. +**Note:** This field is ineffectual for [OCI Helm +Repositories](#helm-oci-repository). + +`.spec.interval` is a an optional field that specifies the interval which the +Helm repository index must be consulted at. When not set, the default value is +`1m`. After successfully reconciling a HelmRepository object, the source-controller requeues the object for inspection after the specified interval. The value @@ -387,11 +380,14 @@ For Helm repositories which require authentication, see [Secret reference](#secr ### Timeout +**Note:** This field is not applicable to [OCI Helm +Repositories](#helm-oci-repository). + `.spec.timeout` is an optional field to specify a timeout for the fetch operation. The value must be in a [Go recognized duration string format](https://pkg.go.dev/time#ParseDuration), -e.g. `1m30s` for a timeout of one minute and thirty seconds. The default value -is `60s`. +e.g. `1m30s` for a timeout of one minute and thirty seconds. When not set, the +default value is `1m`. ### Secret reference @@ -537,6 +533,9 @@ to HTTP/S Helm repositories. ### Suspend +**Note:** This field is not applicable to [OCI Helm +Repositories](#helm-oci-repository). + `.spec.suspend` is an optional field to suspend the reconciliation of a HelmRepository. When set to `true`, the controller will stop reconciling the HelmRepository, and changes to the resource or the Helm repository index will @@ -547,6 +546,10 @@ For practical information, see [suspending and resuming](#suspending-and-resuming). ## Working with HelmRepositories + +**Note:** This section does not apply to [OCI Helm +Repositories](#helm-oci-repository), being a data container, once created, they +are ready to used by [HelmCharts](helmcharts.md). ### Triggering a reconcile @@ -648,6 +651,10 @@ flux resume source helm ### Debugging a HelmRepository +**Note:** This section does not apply to [OCI Helm +Repositories](#helm-oci-repository), being a data container, they are static +objects that don't require debugging if valid. + There are several ways to gather information about a HelmRepository for debugging purposes. @@ -713,9 +720,11 @@ specific HelmRepository, e.g. `flux logs --level=error --kind=HelmRepository --n ## HelmRepository Status -### Artifact +**Note:** This section does not apply to [OCI Helm +Repositories](#helm-oci-repository), they do not contain any information in the +status. -**Note:** This section does not apply to [OCI Helm Repositories](#helm-oci-repository), they do not emit artifacts. +### Artifact The HelmRepository reports the last fetched repository index as an Artifact object in the `.status.artifact` of the resource. @@ -757,9 +766,6 @@ 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/hack/ci/e2e.sh b/hack/ci/e2e.sh index c48393a60..d4bde9a20 100755 --- a/hack/ci/e2e.sh +++ b/hack/ci/e2e.sh @@ -75,7 +75,6 @@ kubectl -n source-system rollout status deploy/source-controller --timeout=1m kubectl -n source-system wait gitrepository/gitrepository-sample --for=condition=ready --timeout=1m kubectl -n source-system wait ocirepository/ocirepository-sample --for=condition=ready --timeout=1m kubectl -n source-system wait helmrepository/helmrepository-sample --for=condition=ready --timeout=1m -kubectl -n source-system wait helmrepository/helmrepository-sample-oci --for=condition=ready --timeout=1m kubectl -n source-system wait helmchart/helmchart-sample --for=condition=ready --timeout=1m kubectl -n source-system wait helmchart/helmchart-sample-oci --for=condition=ready --timeout=1m kubectl -n source-system delete -f "${ROOT_DIR}/config/samples" @@ -145,7 +144,6 @@ kubectl -n source-system wait gitrepository/large-repo --for=condition=ready --t 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 kubectl -n source-system wait helmchart/podinfo-keyless --for=condition=ready --timeout=1m diff --git a/internal/controller/helmchart_controller.go b/internal/controller/helmchart_controller.go index f840a85bc..41b90194c 100644 --- a/internal/controller/helmchart_controller.go +++ b/internal/controller/helmchart_controller.go @@ -18,6 +18,7 @@ package controller import ( "context" + "crypto/tls" "errors" "fmt" "net/url" @@ -139,6 +140,12 @@ type HelmChartReconciler struct { patchOptions []patch.Option } +// 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(tlsConfig *tls.Config, isLogin bool) (*helmreg.Client, string, error) + func (r *HelmChartReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error { return r.SetupWithManagerAndOptions(ctx, mgr, HelmChartReconcilerOptions{}) } @@ -507,7 +514,7 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, sp *patch.Ser func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *helmv1.HelmChart, repo *helmv1.HelmRepository, b *chart.Build) (sreconcile.Result, error) { // Used to login with the repository declared provider - ctxTimeout, cancel := context.WithTimeout(ctx, repo.Spec.Timeout.Duration) + ctxTimeout, cancel := context.WithTimeout(ctx, repo.GetTimeout()) defer cancel() normalizedURL, err := repository.NormalizeURL(repo.Spec.URL) @@ -992,7 +999,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont } // Used to login with the repository declared provider - ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration) + ctxTimeout, cancel := context.WithTimeout(ctx, obj.GetTimeout()) defer cancel() clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL) diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go index 796bfdcd4..e8d4f64ad 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -197,7 +197,7 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) { { name: "Stalling on invalid repository URL", beforeFunc: func(repository *helmv1.HelmRepository) { - repository.Spec.URL = "://unsupported" // Invalid URL + repository.Spec.URL = "https://unsupported/foo://" // Invalid URL }, assertFunc: func(g *WithT, obj *helmv1.HelmChart, _ *helmv1.HelmRepository) { key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} @@ -293,6 +293,7 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) { } g.Expect(testEnv.CreateAndWait(ctx, &repository)).To(Succeed()) + defer func() { g.Expect(testEnv.Delete(ctx, &repository)).To(Succeed()) }() obj := helmv1.HelmChart{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go index 8e252979a..27663c625 100644 --- a/internal/controller/helmrepository_controller.go +++ b/internal/controller/helmrepository_controller.go @@ -22,11 +22,13 @@ import ( "errors" "fmt" "net/url" + "strings" "time" "github.com/docker/go-units" "github.com/opencontainers/go-digest" helmgetter "helm.sh/helm/v3/pkg/getter" + helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" kuberecorder "k8s.io/client-go/tools/record" @@ -138,10 +140,7 @@ func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, For(&helmv1.HelmRepository{}). WithEventFilter( predicate.And( - predicate.Or( - intpredicates.HelmRepositoryTypePredicate{RepositoryType: helmv1.HelmRepositoryTypeDefault}, - intpredicates.HelmRepositoryTypePredicate{RepositoryType: ""}, - ), + intpredicates.HelmRepositoryOCIMigrationPredicate{}, predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}), ), ). @@ -164,6 +163,11 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque // Initialize the patch helper with the current version of the object. serialPatcher := patch.NewSerialPatcher(obj, r.Client) + // If it's of type OCI, migrate the object to static. + if obj.Spec.Type == helmv1.HelmRepositoryTypeOCI { + return r.migrationToStatic(ctx, serialPatcher, obj) + } + // recResult stores the abstracted reconcile result. var recResult sreconcile.Result @@ -193,8 +197,8 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque r.Metrics.RecordDuration(ctx, obj, start) }() - // Examine if the object is under deletion or if a type change has happened. - if !obj.ObjectMeta.DeletionTimestamp.IsZero() || (obj.Spec.Type != "" && obj.Spec.Type != helmv1.HelmRepositoryTypeDefault) { + // Examine if the object is under deletion. + if !obj.ObjectMeta.DeletionTimestamp.IsZero() { recResult, retErr = r.reconcileDelete(ctx, obj) return } @@ -391,6 +395,18 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, sp *pat // pointer is set to the newly fetched index. func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher, obj *helmv1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) { + // Ensure it's not an OCI URL. API validation ensures that only + // http/https/oci scheme are allowed. + if strings.HasPrefix(obj.Spec.URL, helmreg.OCIScheme) { + err := fmt.Errorf("'oci' URL scheme cannot be used with 'default' HelmRepository type") + e := serror.NewStalling( + fmt.Errorf("invalid Helm repository URL: %w", err), + sourcev1.URLInvalidReason, + ) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + } + normalizedURL, err := repository.NormalizeURL(obj.Spec.URL) if err != nil { e := serror.NewStalling( @@ -685,3 +701,33 @@ func (r *HelmRepositoryReconciler) eventLogf(ctx context.Context, obj runtime.Ob } r.Eventf(obj, eventType, reason, msg) } + +// migrateToStatic is HelmRepository OCI migration to static object. +func (r *HelmRepositoryReconciler) migrationToStatic(ctx context.Context, sp *patch.SerialPatcher, obj *helmv1.HelmRepository) (result ctrl.Result, err error) { + // Skip migration if suspended and not being deleted. + if obj.Spec.Suspend && obj.DeletionTimestamp.IsZero() { + return ctrl.Result{}, nil + } + + if !intpredicates.HelmRepositoryOCIRequireMigration(obj) { + // Already migrated, nothing to do. + return ctrl.Result{}, nil + } + + // Delete any artifact. + _, err = r.reconcileDelete(ctx, obj) + if err != nil { + return ctrl.Result{}, err + } + // Delete finalizer and reset the status. + controllerutil.RemoveFinalizer(obj, sourcev1.SourceFinalizer) + obj.Status = helmv1.HelmRepositoryStatus{} + + if err := sp.Patch(ctx, obj); err != nil { + return ctrl.Result{}, err + } + + r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "Migration", + "removed artifact and finalizer to migrate to static HelmRepository of type OCI") + return ctrl.Result{}, nil +} diff --git a/internal/controller/helmrepository_controller_oci.go b/internal/controller/helmrepository_controller_oci.go deleted file mode 100644 index e25eaf4fd..000000000 --- a/internal/controller/helmrepository_controller_oci.go +++ /dev/null @@ -1,418 +0,0 @@ -/* -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 controller - -import ( - "context" - "crypto/tls" - "errors" - "fmt" - "net/url" - "os" - "time" - - helmreg "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" - "k8s.io/apimachinery/pkg/runtime" - kerrors "k8s.io/apimachinery/pkg/util/errors" - 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" - - eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" - "github.com/fluxcd/pkg/apis/meta" - "github.com/fluxcd/pkg/runtime/conditions" - helper "github.com/fluxcd/pkg/runtime/controller" - "github.com/fluxcd/pkg/runtime/jitter" - "github.com/fluxcd/pkg/runtime/patch" - "github.com/fluxcd/pkg/runtime/predicates" - rreconcile "github.com/fluxcd/pkg/runtime/reconcile" - - sourcev1 "github.com/fluxcd/source-controller/api/v1" - helmv1 "github.com/fluxcd/source-controller/api/v1beta2" - "github.com/fluxcd/source-controller/internal/helm/getter" - "github.com/fluxcd/source-controller/internal/helm/repository" - "github.com/fluxcd/source-controller/internal/object" - intpredicates "github.com/fluxcd/source-controller/internal/predicates" -) - -var helmRepositoryOCIOwnedConditions = []string{ - meta.ReadyCondition, - meta.ReconcilingCondition, - meta.StalledCondition, -} - -var helmRepositoryOCINegativeConditions = []string{ - meta.StalledCondition, - meta.ReconcilingCondition, -} - -// +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 - - ControllerName string - RegistryClientGenerator RegistryClientGeneratorFunc - - patchOptions []patch.Option - - // unmanagedConditions are the conditions that are not managed by this - // reconciler and need to be removed from the object before taking ownership - // of the object being reconciled. - unmanagedConditions []string -} - -// 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(tlsConfig *tls.Config, isLogin bool) (*helmreg.Client, string, error) - -func (r *HelmRepositoryOCIReconciler) SetupWithManager(mgr ctrl.Manager) error { - return r.SetupWithManagerAndOptions(mgr, HelmRepositoryReconcilerOptions{}) -} - -func (r *HelmRepositoryOCIReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmRepositoryReconcilerOptions) error { - r.unmanagedConditions = conditionsDiff(helmRepositoryReadyCondition.Owned, helmRepositoryOCIOwnedConditions) - r.patchOptions = getPatchOptions(helmRepositoryOCIOwnedConditions, r.ControllerName) - - return ctrl.NewControllerManagedBy(mgr). - For(&helmv1.HelmRepository{}). - WithEventFilter( - predicate.And( - intpredicates.HelmRepositoryTypePredicate{RepositoryType: helmv1.HelmRepositoryTypeOCI}, - predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}), - ), - ). - WithOptions(controller.Options{ - RateLimiter: opts.RateLimiter, - }). - Complete(r) -} - -func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, retErr error) { - start := time.Now() - log := ctrl.LoggerFrom(ctx) - - // Fetch the HelmRepository - obj := &helmv1.HelmRepository{} - if err := r.Get(ctx, req.NamespacedName, obj); err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) - } - - // If the object contains any of the unmanaged conditions, requeue and wait - // for those conditions to be removed first before processing the object. - // NOTE: This will happen only when a HelmRepository's spec.type is switched - // from "default" to "oci". - if conditions.HasAny(obj, r.unmanagedConditions) { - r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "IncompleteTransition", - "object contains conditions managed by other reconciler") - return ctrl.Result{RequeueAfter: time.Second}, nil - } - - // Initialize the patch helper with the current version of the object. - serialPatcher := patch.NewSerialPatcher(obj, r.Client) - - // Always attempt to patch the object after each reconciliation. - defer func() { - // If a reconcile annotation value is found, set it in the object status - // as status.lastHandledReconcileAt. - if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok { - object.SetStatusLastHandledReconcileAt(obj, v) - } - - patchOpts := []patch.Option{} - patchOpts = append(patchOpts, r.patchOptions...) - - // Set status observed generation option if the object is stalled, or - // if the object is ready. - if conditions.IsStalled(obj) || conditions.IsReady(obj) { - patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{}) - } - - if err := serialPatcher.Patch(ctx, obj, patchOpts...); err != nil { - // Ignore patch error "not found" when the object is being deleted. - if !obj.GetDeletionTimestamp().IsZero() { - err = kerrors.FilterOut(err, func(e error) bool { return apierrors.IsNotFound(e) }) - } - retErr = kerrors.NewAggregate([]error{retErr, err}) - } - - // Always record suspend, readiness and duration metrics. - r.Metrics.RecordSuspend(ctx, obj, obj.Spec.Suspend) - r.Metrics.RecordReadiness(ctx, obj) - r.Metrics.RecordDuration(ctx, obj, start) - }() - - // Examine if the object is under deletion. - if !obj.ObjectMeta.DeletionTimestamp.IsZero() { - return r.reconcileDelete(ctx, obj) - } - - // Add finalizer first if it doesn't exist to avoid the race condition - // between init and delete. - // Note: Finalizers in general can only be added when the deletionTimestamp - // is not set. - if !controllerutil.ContainsFinalizer(obj, sourcev1.SourceFinalizer) { - controllerutil.AddFinalizer(obj, sourcev1.SourceFinalizer) - return ctrl.Result{Requeue: true}, nil - } - - // Return if the object is suspended. - if obj.Spec.Suspend { - log.Info("reconciliation is suspended for this object") - return ctrl.Result{}, nil - } - - // Examine if a type change has happened and act accordingly - if obj.Spec.Type != helmv1.HelmRepositoryTypeOCI { - // Remove any stale condition and ignore the object if the type has - // changed. - obj.Status.Conditions = nil - return ctrl.Result{}, nil - } - - result, retErr = r.reconcile(ctx, serialPatcher, obj) - return -} - -// reconcile reconciles the HelmRepository object. While reconciling, when an -// error is encountered, it sets the failure details in the appropriate status -// condition type and returns the error with appropriate ctrl.Result. The object -// status conditions and the returned results are evaluated in the deferred -// block at the very end to summarize the conditions to be in a consistent -// state. -func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.SerialPatcher, obj *helmv1.HelmRepository) (result ctrl.Result, retErr error) { - ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration) - defer cancel() - - oldObj := obj.DeepCopy() - - defer func() { - // If it's stalled, ensure reconciling is removed. - if sc := conditions.Get(obj, meta.StalledCondition); sc != nil && sc.Status == metav1.ConditionTrue { - conditions.Delete(obj, meta.ReconcilingCondition) - } - - // Check if it's a successful reconciliation. - if result.RequeueAfter == obj.GetRequeueAfter() && !result.Requeue && - retErr == nil { - // Remove reconciling condition if the reconciliation was successful. - conditions.Delete(obj, meta.ReconcilingCondition) - // If it's not ready even though it's not reconciling or stalled, - // set the ready failure message as the error. - // Based on isNonStalledSuccess() from internal/reconcile/summarize. - if ready := conditions.Get(obj, meta.ReadyCondition); ready != nil && - ready.Status == metav1.ConditionFalse && !conditions.IsStalled(obj) { - retErr = errors.New(conditions.GetMessage(obj, meta.ReadyCondition)) - } - } - - // Presence of reconciling means that the reconciliation didn't succeed. - // Set the Reconciling reason to ProgressingWithRetry to indicate a - // failure retry. - if conditions.IsReconciling(obj) { - reconciling := conditions.Get(obj, meta.ReconcilingCondition) - reconciling.Reason = meta.ProgressingWithRetryReason - conditions.Set(obj, reconciling) - } - - // If it's still a successful reconciliation and it's not reconciling or - // stalled, mark Ready=True. - if !conditions.IsReconciling(obj) && !conditions.IsStalled(obj) && - retErr == nil && result.RequeueAfter == obj.GetRequeueAfter() { - conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready") - } - - // Emit events when object's state changes. - ready := conditions.Get(obj, meta.ReadyCondition) - // Became ready from not ready. - if !conditions.IsReady(oldObj) && conditions.IsReady(obj) { - r.eventLogf(ctx, obj, corev1.EventTypeNormal, ready.Reason, ready.Message) - } - // Became not ready from ready. - if conditions.IsReady(oldObj) && !conditions.IsReady(obj) { - r.eventLogf(ctx, obj, corev1.EventTypeWarning, ready.Reason, ready.Message) - } - - // Apply jitter. - if result.RequeueAfter == obj.GetRequeueAfter() { - result.RequeueAfter = jitter.JitteredIntervalDuration(result.RequeueAfter) - } - }() - - // Set reconciling condition. - rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress") - - var reconcileAtVal string - if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok { - reconcileAtVal = v - } - - // Persist reconciling if generation differs or reconciliation is requested. - switch { - case obj.Generation != obj.Status.ObservedGeneration: - rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, - "processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation) - if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - result, retErr = ctrl.Result{}, err - return - } - case reconcileAtVal != obj.Status.GetLastHandledReconcileRequest(): - if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - result, retErr = ctrl.Result{}, err - return - } - } - - // Ensure that it's an OCI URL before continuing. - if !helmreg.IsOCI(obj.Spec.URL) { - u, err := url.Parse(obj.Spec.URL) - if err != nil { - err = fmt.Errorf("failed to parse URL: %w", err) - } else { - err = fmt.Errorf("URL scheme '%s' in '%s' is not supported", u.Scheme, obj.Spec.URL) - } - conditions.MarkStalled(obj, sourcev1.URLInvalidReason, err.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.URLInvalidReason, err.Error()) - ctrl.LoggerFrom(ctx).Error(err, "reconciliation stalled") - result, retErr = ctrl.Result{}, nil - return - } - - normalizedURL, err := repository.NormalizeURL(obj.Spec.URL) - if err != nil { - conditions.MarkStalled(obj, sourcev1.URLInvalidReason, err.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.URLInvalidReason, err.Error()) - result, retErr = ctrl.Result{}, nil - return - } - - conditions.Delete(obj, meta.StalledCondition) - - clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, obj, normalizedURL) - if err != nil { - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error()) - result, retErr = ctrl.Result{}, err - return - } - if certsTmpDir != "" { - defer func() { - if err := os.RemoveAll(certsTmpDir); err != nil { - r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason, - "failed to delete temporary certs directory: %s", err) - } - }() - } - - // Create registry client and login if needed. - registryClient, file, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry()) - if err != nil { - e := fmt.Errorf("failed to create registry client: %w", err) - conditions.MarkFalse(obj, meta.ReadyCondition, meta.FailedReason, e.Error()) - result, retErr = ctrl.Result{}, e - return - } - if file != "" { - defer func() { - if err := os.Remove(file); err != nil { - r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason, - "failed to delete temporary credentials file: %s", err) - } - }() - } - - chartRepo, err := repository.NewOCIChartRepository(obj.Spec.URL, repository.WithOCIRegistryClient(registryClient)) - if err != nil { - e := fmt.Errorf("failed to parse URL '%s': %w", obj.Spec.URL, err) - conditions.MarkStalled(obj, sourcev1.URLInvalidReason, e.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.URLInvalidReason, e.Error()) - result, retErr = ctrl.Result{}, nil - return - } - conditions.Delete(obj, meta.StalledCondition) - - // Attempt to login to the registry if credentials are provided. - if clientOpts.MustLoginToRegistry() { - err = chartRepo.Login(clientOpts.RegLoginOpts...) - if err != nil { - e := fmt.Errorf("failed to login to registry '%s': %w", obj.Spec.URL, err) - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error()) - result, retErr = ctrl.Result{}, e - return - } - } - - // Remove any stale Ready condition, most likely False, set above. Its value - // is derived from the overall result of the reconciliation in the deferred - // block at the very end. - conditions.Delete(obj, meta.ReadyCondition) - - result, retErr = ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil - return -} - -func (r *HelmRepositoryOCIReconciler) reconcileDelete(ctx context.Context, obj *helmv1.HelmRepository) (ctrl.Result, error) { - // Remove our finalizer from the list - controllerutil.RemoveFinalizer(obj, sourcev1.SourceFinalizer) - - // Stop reconciliation as the object is being deleted - return ctrl.Result{}, nil -} - -// eventLogf records events, and logs at the same time. -// -// This log is different from the debug log in the EventRecorder, in the sense -// that this is a simple log. While the debug log contains complete details -// about the event. -func (r *HelmRepositoryOCIReconciler) eventLogf(ctx context.Context, obj runtime.Object, eventType string, reason string, messageFmt string, args ...interface{}) { - msg := fmt.Sprintf(messageFmt, args...) - // Log and emit event. - if eventType == corev1.EventTypeWarning { - ctrl.LoggerFrom(ctx).Error(errors.New(reason), msg) - } else { - ctrl.LoggerFrom(ctx).Info(msg) - } - r.Eventf(obj, eventType, reason, msg) -} - -func conditionsDiff(a, b []string) []string { - bMap := make(map[string]struct{}, len(b)) - for _, j := range b { - bMap[j] = struct{}{} - } - - r := []string{} - for _, i := range a { - if _, exists := bMap[i]; !exists { - r = append(r, i) - } - } - return r -} diff --git a/internal/controller/helmrepository_controller_oci_test.go b/internal/controller/helmrepository_controller_oci_test.go deleted file mode 100644 index 1d5361c91..000000000 --- a/internal/controller/helmrepository_controller_oci_test.go +++ /dev/null @@ -1,478 +0,0 @@ -/* -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 controller - -import ( - "encoding/base64" - "fmt" - "strconv" - "testing" - - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/tools/record" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" - - kstatus "github.com/fluxcd/cli-utils/pkg/kstatus/status" - "github.com/fluxcd/pkg/apis/meta" - "github.com/fluxcd/pkg/runtime/conditions" - conditionscheck "github.com/fluxcd/pkg/runtime/conditions/check" - "github.com/fluxcd/pkg/runtime/patch" - - sourcev1 "github.com/fluxcd/source-controller/api/v1" - helmv1 "github.com/fluxcd/source-controller/api/v1beta2" - "github.com/fluxcd/source-controller/internal/helm/registry" -) - -func TestHelmRepositoryOCIReconciler_deleteBeforeFinalizer(t *testing.T) { - g := NewWithT(t) - - namespaceName := "helmrepo-" + randStringRunes(5) - namespace := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{Name: namespaceName}, - } - g.Expect(k8sClient.Create(ctx, namespace)).ToNot(HaveOccurred()) - t.Cleanup(func() { - g.Expect(k8sClient.Delete(ctx, namespace)).NotTo(HaveOccurred()) - }) - - helmrepo := &helmv1.HelmRepository{} - helmrepo.Name = "test-helmrepo" - helmrepo.Namespace = namespaceName - helmrepo.Spec = helmv1.HelmRepositorySpec{ - Interval: metav1.Duration{Duration: interval}, - URL: "https://example.com", - Type: "oci", - } - // Add a test finalizer to prevent the object from getting deleted. - helmrepo.SetFinalizers([]string{"test-finalizer"}) - g.Expect(k8sClient.Create(ctx, helmrepo)).NotTo(HaveOccurred()) - // Add deletion timestamp by deleting the object. - g.Expect(k8sClient.Delete(ctx, helmrepo)).NotTo(HaveOccurred()) - - r := &HelmRepositoryOCIReconciler{ - Client: k8sClient, - EventRecorder: record.NewFakeRecorder(32), - } - // NOTE: Only a real API server responds with an error in this scenario. - _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(helmrepo)}) - g.Expect(err).NotTo(HaveOccurred()) -} - -func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { - tests := []struct { - name string - secretType corev1.SecretType - secretData map[string][]byte - }{ - { - name: "valid auth data", - secretData: map[string][]byte{ - "username": []byte(testRegistryUsername), - "password": []byte(testRegistryPassword), - }, - }, - { - name: "no auth data", - secretData: nil, - }, - { - name: "dockerconfigjson Secret", - secretType: corev1.SecretTypeDockerConfigJson, - secretData: map[string][]byte{ - ".dockerconfigjson": []byte(`{"auths":{"` + - testRegistryServer.registryHost + `":{"` + - `auth":"` + base64.StdEncoding.EncodeToString([]byte(testRegistryUsername+":"+testRegistryPassword)) + `"}}}`), - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - 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: tt.secretData, - } - if tt.secretType != "" { - secret.Type = tt.secretType - } - - g.Expect(testEnv.CreateAndWait(ctx, secret)).To(Succeed()) - - origObj := &helmv1.HelmRepository{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "helmrepository-oci-reconcile-", - Namespace: ns.Name, - }, - Spec: helmv1.HelmRepositorySpec{ - Interval: metav1.Duration{Duration: interval}, - URL: fmt.Sprintf("oci://%s", testRegistryServer.registryHost), - SecretRef: &meta.LocalObjectReference{ - Name: secret.Name, - }, - Provider: helmv1.GenericOCIProvider, - Type: helmv1.HelmRepositoryTypeOCI, - }, - } - obj := origObj.DeepCopy() - 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 - waitForSourceReadyWithoutArtifact(ctx, g, obj) - - // Check if the object status is valid. - condns := &conditionscheck.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} - checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.WithT(g).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 HelmRepository to be deleted - waitForSourceDeletion(ctx, g, obj) - - // Check if a suspended object gets deleted. - obj = origObj.DeepCopy() - testSuspendedObjectDeleteWithoutArtifact(ctx, g, obj) - }) - } -} - -func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { - type secretOptions struct { - username string - password string - } - - tests := []struct { - name string - url string - registryOpts registryOptions - insecure bool - secretOpts secretOptions - secret *corev1.Secret - certsSecret *corev1.Secret - provider string - providerImg string - want ctrl.Result - wantErr bool - assertConditions []metav1.Condition - }{ - { - name: "HTTP without basic auth", - want: ctrl.Result{RequeueAfter: interval}, - assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"), - }, - }, - { - name: "HTTP with basic auth secret", - want: ctrl.Result{RequeueAfter: interval}, - insecure: true, - registryOpts: registryOptions{ - withBasicAuth: true, - }, - secretOpts: secretOptions{ - username: testRegistryUsername, - password: testRegistryPassword, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "auth-secretref", - }, - Type: corev1.SecretTypeDockerConfigJson, - Data: map[string][]byte{}, - }, - assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"), - }, - }, - { - name: "HTTP registry - basic auth with invalid secret", - want: ctrl.Result{}, - wantErr: true, - insecure: true, - registryOpts: registryOptions{ - withBasicAuth: true, - }, - secretOpts: secretOptions{ - username: "wrong-pass", - password: "wrong-pass", - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "auth-secretref", - }, - Type: corev1.SecretTypeDockerConfigJson, - Data: map[string][]byte{}, - }, - assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation"), - *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to login to registry"), - }, - }, - { - name: "with contextual login provider", - wantErr: true, - insecure: true, - provider: "aws", - providerImg: "oci://123456789000.dkr.ecr.us-east-2.amazonaws.com/test", - assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation"), - *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to get credential from"), - }, - }, - { - name: "with contextual login provider and secretRef", - want: ctrl.Result{RequeueAfter: interval}, - registryOpts: registryOptions{ - withBasicAuth: true, - }, - insecure: true, - secretOpts: secretOptions{ - username: testRegistryUsername, - password: testRegistryPassword, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "auth-secretref", - }, - Type: corev1.SecretTypeDockerConfigJson, - Data: map[string][]byte{}, - }, - provider: "azure", - assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"), - }, - }, - { - name: "HTTPS With invalid CA cert", - wantErr: true, - registryOpts: registryOptions{ - withTLS: true, - withClientCertAuth: true, - }, - secretOpts: secretOptions{ - username: testRegistryUsername, - password: testRegistryPassword, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "auth-secretref", - }, - Type: corev1.SecretTypeDockerConfigJson, - Data: map[string][]byte{}, - }, - certsSecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "certs-secretref", - }, - Data: map[string][]byte{ - "ca.crt": []byte("invalid caFile"), - }, - }, - assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation 0 -> 1"), - *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid CA certificate"), - }, - }, - { - name: "HTTPS With CA cert", - want: ctrl.Result{RequeueAfter: interval}, - registryOpts: registryOptions{ - withTLS: true, - withClientCertAuth: true, - }, - secretOpts: secretOptions{ - username: testRegistryUsername, - password: testRegistryPassword, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "auth-secretref", - }, - Type: corev1.SecretTypeDockerConfigJson, - Data: map[string][]byte{}, - }, - certsSecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "certs-secretref", - }, - Data: map[string][]byte{ - "ca.crt": tlsCA, - "tls.crt": clientPublicKey, - "tls.key": clientPrivateKey, - }, - }, - assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "Helm repository is ready"), - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - clientBuilder := fakeclient.NewClientBuilder(). - WithScheme(testEnv.GetScheme()). - WithStatusSubresource(&helmv1.HelmRepository{}) - - workspaceDir := t.TempDir() - if tt.insecure { - tt.registryOpts.disableDNSMocking = true - } - server, err := setupRegistryServer(ctx, workspaceDir, tt.registryOpts) - g.Expect(err).NotTo(HaveOccurred()) - t.Cleanup(func() { - server.Close() - }) - - obj := &helmv1.HelmRepository{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "auth-strategy-", - Generation: 1, - }, - Spec: helmv1.HelmRepositorySpec{ - Interval: metav1.Duration{Duration: interval}, - Timeout: &metav1.Duration{Duration: timeout}, - Type: helmv1.HelmRepositoryTypeOCI, - Provider: helmv1.GenericOCIProvider, - URL: fmt.Sprintf("oci://%s", server.registryHost), - }, - } - - if tt.provider != "" { - obj.Spec.Provider = tt.provider - } - // If a provider specific image is provided, overwrite existing URL - // set earlier. It'll fail, but it's necessary to set them because - // the login check expects the URLs to be of certain pattern. - if tt.providerImg != "" { - obj.Spec.URL = tt.providerImg - } - - if tt.secretOpts.username != "" && tt.secretOpts.password != "" { - tt.secret.Data[".dockerconfigjson"] = []byte(fmt.Sprintf(`{"auths": {%q: {"username": %q, "password": %q}}}`, - server.registryHost, tt.secretOpts.username, tt.secretOpts.password)) - } - - if tt.secret != nil { - clientBuilder.WithObjects(tt.secret) - obj.Spec.SecretRef = &meta.LocalObjectReference{ - Name: tt.secret.Name, - } - } - - if tt.certsSecret != nil { - clientBuilder.WithObjects(tt.certsSecret) - obj.Spec.CertSecretRef = &meta.LocalObjectReference{ - Name: tt.certsSecret.Name, - } - } - - r := &HelmRepositoryOCIReconciler{ - Client: clientBuilder.Build(), - EventRecorder: record.NewFakeRecorder(32), - RegistryClientGenerator: registry.ClientGenerator, - patchOptions: getPatchOptions(helmRepositoryOCIOwnedConditions, "sc"), - } - - g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred()) - defer func() { - g.Expect(r.Client.Delete(ctx, obj)).ToNot(HaveOccurred()) - }() - - sp := patch.NewSerialPatcher(obj, r.Client) - got, err := r.reconcile(ctx, sp, obj) - g.Expect(err != nil).To(Equal(tt.wantErr)) - g.Expect(got).To(Equal(tt.want)) - g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions)) - - // In-progress status condition validity. - checker := conditionscheck.NewInProgressChecker(r.Client) - // NOTE: Check the object directly as reconcile() doesn't apply the - // final patch, the object has unapplied changes. - checker.DisableFetch = true - checker.WithT(g).CheckErr(ctx, obj) - }) - } -} - -func TestConditionsDiff(t *testing.T) { - tests := []struct { - a, b, want []string - }{ - {[]string{"a", "b", "c"}, []string{"b", "d"}, []string{"a", "c"}}, - {[]string{"a", "b", "c"}, []string{}, []string{"a", "b", "c"}}, - {[]string{}, []string{"b", "d"}, []string{}}, - {[]string{}, []string{}, []string{}}, - {[]string{"a", "b"}, nil, []string{"a", "b"}}, - {nil, []string{"a", "b"}, []string{}}, - {nil, nil, []string{}}, - } - - for i, tt := range tests { - t.Run(strconv.Itoa(i), func(t *testing.T) { - g := NewWithT(t) - g.Expect(conditionsDiff(tt.a, tt.b)).To(Equal(tt.want)) - }) - } -} diff --git a/internal/controller/helmrepository_controller_test.go b/internal/controller/helmrepository_controller_test.go index bae5b0ee3..0da154a15 100644 --- a/internal/controller/helmrepository_controller_test.go +++ b/internal/controller/helmrepository_controller_test.go @@ -40,6 +40,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" kstatus "github.com/fluxcd/cli-utils/pkg/kstatus/status" "github.com/fluxcd/pkg/apis/meta" @@ -54,6 +55,7 @@ import ( intdigest "github.com/fluxcd/source-controller/internal/digest" "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" stls "github.com/fluxcd/source-controller/internal/tls" @@ -1522,50 +1524,22 @@ func TestHelmRepositoryReconciler_ReconcileTypeUpdatePredicateFilter(t *testing. 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(testRegistryUsername), - "password": []byte(testRegistryPassword), - }, - } - g.Expect(testEnv.CreateAndWait(ctx, secret)).To(Succeed()) - obj.Spec.Type = helmv1.HelmRepositoryTypeOCI obj.Spec.URL = fmt.Sprintf("oci://%s", testRegistryServer.registryHost) - obj.Spec.SecretRef = &meta.LocalObjectReference{ - Name: secret.Name, - } oldGen := obj.GetGeneration() g.Expect(testEnv.Update(ctx, obj)).To(Succeed()) newGen := oldGen + 1 - // Wait for HelmRepository to be Ready with new generation. + // Wait for HelmRepository to become static for new generation. 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) - if readyCondition == nil { - return false - } - return readyCondition.Status == metav1.ConditionTrue && - newGen == readyCondition.ObservedGeneration && - newGen == obj.Status.ObservedGeneration + return newGen == obj.Generation && + !intpredicates.HelmRepositoryOCIRequireMigration(obj) }, timeout).Should(BeTrue()) - // Check if the object status is valid. - condns = &conditionscheck.Conditions{NegativePolarity: helmRepositoryOCINegativeConditions} - checker = conditionscheck.NewChecker(testEnv.Client, condns) - checker.WithT(g).CheckErr(ctx, obj) - g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) // Wait for HelmRepository to be deleted @@ -1729,4 +1703,101 @@ func TestHelmRepositoryReconciler_InMemoryCaching(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) _, cacheHit := testCache.Get(helmRepo.GetArtifact().Path) g.Expect(cacheHit).To(BeTrue()) + + g.Expect(testEnv.Delete(ctx, helmRepo)).To(Succeed()) + + // Wait for HelmRepository to be deleted + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, helmRepo); err != nil { + return apierrors.IsNotFound(err) + } + return false + }, timeout).Should(BeTrue()) +} + +func TestHelmRepositoryReconciler_ociMigration(t *testing.T) { + g := NewWithT(t) + + testns, err := testEnv.CreateNamespace(ctx, "hr-oci-migration-test") + g.Expect(err).ToNot(HaveOccurred()) + + t.Cleanup(func() { + g.Expect(testEnv.Cleanup(ctx, testns)).ToNot(HaveOccurred()) + }) + + hr := &helmv1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("hr-%s", randStringRunes(5)), + Namespace: testns.Name, + }, + } + hrKey := client.ObjectKeyFromObject(hr) + + // Migrates newly created object with finalizer. + + hr.ObjectMeta.Finalizers = append(hr.ObjectMeta.Finalizers, "foo.bar", sourcev1.SourceFinalizer) + hr.Spec = helmv1.HelmRepositorySpec{ + Type: helmv1.HelmRepositoryTypeOCI, + URL: "oci://foo/bar", + Interval: metav1.Duration{Duration: interval}, + } + g.Expect(testEnv.Create(ctx, hr)).ToNot(HaveOccurred()) + + g.Eventually(func() bool { + _ = testEnv.Get(ctx, hrKey, hr) + return !intpredicates.HelmRepositoryOCIRequireMigration(hr) + }, timeout, time.Second).Should(BeTrue()) + + // Migrates updated object with finalizer. + + patchHelper, err := patch.NewHelper(hr, testEnv.Client) + g.Expect(err).ToNot(HaveOccurred()) + hr.ObjectMeta.Finalizers = append(hr.ObjectMeta.Finalizers, sourcev1.SourceFinalizer) + hr.Spec.URL = "oci://foo/baz" + g.Expect(patchHelper.Patch(ctx, hr)).ToNot(HaveOccurred()) + + g.Eventually(func() bool { + _ = testEnv.Get(ctx, hrKey, hr) + return !intpredicates.HelmRepositoryOCIRequireMigration(hr) + }, timeout, time.Second).Should(BeTrue()) + + // Migrates deleted object with finalizer. + + patchHelper, err = patch.NewHelper(hr, testEnv.Client) + g.Expect(err).ToNot(HaveOccurred()) + + // Suspend the object to prevent finalizer from getting removed. + // Ensure only flux finalizer is set to allow the object to be garbage + // collected at the end. + // NOTE: Suspending and updating finalizers are done separately here as + // doing them in a single patch results in flaky test where the finalizer + // update doesn't gets registered with the kube-apiserver, resulting in + // timeout waiting for finalizer to appear on the object below. + hr.Spec.Suspend = true + g.Expect(patchHelper.Patch(ctx, hr)).ToNot(HaveOccurred()) + g.Eventually(func() bool { + _ = k8sClient.Get(ctx, hrKey, hr) + return hr.Spec.Suspend == true + }, timeout).Should(BeTrue()) + + patchHelper, err = patch.NewHelper(hr, testEnv.Client) + g.Expect(err).ToNot(HaveOccurred()) + + // Add finalizer and verify that finalizer exists on the object using a live + // client. + hr.ObjectMeta.Finalizers = []string{sourcev1.SourceFinalizer} + g.Expect(patchHelper.Patch(ctx, hr)).ToNot(HaveOccurred()) + g.Eventually(func() bool { + _ = k8sClient.Get(ctx, hrKey, hr) + return controllerutil.ContainsFinalizer(hr, sourcev1.SourceFinalizer) + }, timeout).Should(BeTrue()) + + // Delete the object and verify. + g.Expect(testEnv.Delete(ctx, hr)).ToNot(HaveOccurred()) + g.Eventually(func() bool { + if err := testEnv.Get(ctx, hrKey, hr); err != nil { + return apierrors.IsNotFound(err) + } + return false + }, timeout).Should(BeTrue()) } diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index faa775d8a..c52c8dc30 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -58,7 +58,6 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1" sourcev1beta2 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/internal/cache" - "github.com/fluxcd/source-controller/internal/helm/registry" // +kubebuilder:scaffold:imports ) @@ -372,17 +371,6 @@ 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, - RegistryClientGenerator: registry.ClientGenerator, - }).SetupWithManagerAndOptions(testEnv, HelmRepositoryReconcilerOptions{ - RateLimiter: controller.GetDefaultRateLimiter(), - }); err != nil { - panic(fmt.Sprintf("Failed to start HelmRepositoryOCIReconciler: %v", err)) - } - if err := (&HelmChartReconciler{ Client: testEnv, EventRecorder: record.NewFakeRecorder(32), diff --git a/internal/helm/getter/client_opts.go b/internal/helm/getter/client_opts.go index ce7620ca1..4dfc97b40 100644 --- a/internal/helm/getter/client_opts.go +++ b/internal/helm/getter/client_opts.go @@ -72,7 +72,7 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit hrOpts := &ClientOpts{ GetterOpts: []helmgetter.Option{ helmgetter.WithURL(url), - helmgetter.WithTimeout(obj.Spec.Timeout.Duration), + helmgetter.WithTimeout(obj.GetTimeout()), helmgetter.WithPassCredentialsAll(obj.Spec.PassCredentials), }, } diff --git a/internal/predicates/helmrepository_type_predicate.go b/internal/predicates/helmrepository_type_predicate.go index 76694b82f..cc7c8fc7e 100644 --- a/internal/predicates/helmrepository_type_predicate.go +++ b/internal/predicates/helmrepository_type_predicate.go @@ -18,69 +18,69 @@ package predicates import ( "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "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 - } +// HelmRepositoryOCIMigrationPredicate implements predicate functions to allow +// events for HelmRepository OCI that need migration to static object. Non-OCI +// HelmRepositories are always allowed. +type HelmRepositoryOCIMigrationPredicate struct { + predicate.Funcs +} - return hr.Spec.Type == repositoryType +// Create allows events for objects that need migration to static object. +func (HelmRepositoryOCIMigrationPredicate) Create(e event.CreateEvent) bool { + return HelmRepositoryOCIRequireMigration(e.Object) } -// HelmRepositoryTypePredicate is a predicate that filters events for a given HelmRepository type. -type HelmRepositoryTypePredicate struct { - RepositoryType string - predicate.Funcs +// Update allows events for objects that need migration to static object. +func (HelmRepositoryOCIMigrationPredicate) Update(e event.UpdateEvent) bool { + return HelmRepositoryOCIRequireMigration(e.ObjectNew) } -// 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) +// Delete allows events for objects that need migration to static object. +func (HelmRepositoryOCIMigrationPredicate) Delete(e event.DeleteEvent) bool { + return HelmRepositoryOCIRequireMigration(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 { +// HelmRepositoryOCIRequireMigration returns if a given HelmRepository of type +// OCI requires migration to static object. For non-OCI HelmRepository, it +// returns true. +func HelmRepositoryOCIRequireMigration(o client.Object) bool { + if o == nil { return false } - // check if the old object is a HelmRepository - oldObj, ok := e.ObjectOld.(*sourcev1.HelmRepository) + hr, ok := o.(*sourcev1.HelmRepository) if !ok { return false } - // check if the new object is a HelmRepository - newObj, ok := e.ObjectNew.(*sourcev1.HelmRepository) - if !ok { - return false + if hr.Spec.Type != sourcev1.HelmRepositoryTypeOCI { + // Always allow non-OCI HelmRepository. + return true } - isOfRepositoryType := newObj.Spec.Type == h.RepositoryType - wasOfRepositoryType := oldObj.Spec.Type == h.RepositoryType && !isOfRepositoryType - return isOfRepositoryType || wasOfRepositoryType -} + if controllerutil.ContainsFinalizer(hr, sourcev1.SourceFinalizer) || !hasEmptyHelmRepositoryStatus(hr) { + return true + } -// 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) + return false } -// 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) +// hasEmptyHelmRepositoryStatus checks if the status of a HelmRepository is +// empty. +func hasEmptyHelmRepositoryStatus(obj *sourcev1.HelmRepository) bool { + if obj.Status.ObservedGeneration == 0 && + obj.Status.Conditions == nil && + obj.Status.URL == "" && + obj.Status.Artifact == nil && + obj.Status.ReconcileRequestStatus.LastHandledReconcileAt == "" { + return true + } + return false } diff --git a/internal/predicates/helmrepository_type_predicate_test.go b/internal/predicates/helmrepository_type_predicate_test.go index e54726892..0d3489d1f 100644 --- a/internal/predicates/helmrepository_type_predicate_test.go +++ b/internal/predicates/helmrepository_type_predicate_test.go @@ -19,109 +19,241 @@ 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" + . "github.com/onsi/gomega" "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{} + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" + v1 "github.com/fluxcd/source-controller/api/v1" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" +) + +func TestHelmRepositoryOCIMigrationPredicate_Create(t *testing.T) { tests := []struct { - name string - obj client.Object - want bool + name string + beforeFunc func(o *sourcev1.HelmRepository) + 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}, + { + name: "new oci helm repo no status", + beforeFunc: func(o *sourcev1.HelmRepository) { + o.Spec.Type = sourcev1.HelmRepositoryTypeOCI + }, + want: false, + }, + { + name: "new oci helm repo with default observed gen status", + beforeFunc: func(o *sourcev1.HelmRepository) { + o.Spec.Type = sourcev1.HelmRepositoryTypeOCI + o.Status.ObservedGeneration = -1 + }, + want: true, + }, + { + name: "old oci helm repo with finalizer only", + beforeFunc: func(o *sourcev1.HelmRepository) { + o.Finalizers = []string{sourcev1.SourceFinalizer} + o.Spec.Type = sourcev1.HelmRepositoryTypeOCI + }, + want: true, + }, + { + name: "old oci helm repo with status only", + beforeFunc: func(o *sourcev1.HelmRepository) { + o.Spec.Type = sourcev1.HelmRepositoryTypeOCI + o.Status = sourcev1.HelmRepositoryStatus{ + ObservedGeneration: 3, + } + conditions.MarkTrue(o, meta.ReadyCondition, "foo", "bar") + }, + want: true, + }, + { + name: "old oci helm repo with finalizer and status", + beforeFunc: func(o *sourcev1.HelmRepository) { + o.Finalizers = []string{sourcev1.SourceFinalizer} + o.Spec.Type = sourcev1.HelmRepositoryTypeOCI + o.Status = sourcev1.HelmRepositoryStatus{ + ObservedGeneration: 3, + } + conditions.MarkTrue(o, meta.ReadyCondition, "foo", "bar") + }, + want: true, + }, + { + name: "new default helm repo", + beforeFunc: func(o *sourcev1.HelmRepository) { + o.Spec.Type = sourcev1.HelmRepositoryTypeDefault + }, + want: true, + }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) - so := HelmRepositoryTypePredicate{RepositoryType: "default"} - e := event.CreateEvent{ - Object: tt.obj, + o := &sourcev1.HelmRepository{} + if tt.beforeFunc != nil { + tt.beforeFunc(o) } - g.Expect(so.Create(e)).To(gomega.Equal(tt.want)) + e := event.CreateEvent{Object: o} + p := HelmRepositoryOCIMigrationPredicate{} + g.Expect(p.Create(e)).To(Equal(tt.want)) }) } } -func TestHelmRepositoryTypePredicate_Update(t *testing.T) { - repoA := &sourcev1.HelmRepository{Spec: sourcev1.HelmRepositorySpec{ - Type: sourcev1.HelmRepositoryTypeDefault, - }} +func TestHelmRepositoryOCIMigrationPredicate_Update(t *testing.T) { + tests := []struct { + name string + beforeFunc func(oldObj, newObj *sourcev1.HelmRepository) + want bool + }{ + { + name: "update oci repo", + beforeFunc: func(oldObj, newObj *sourcev1.HelmRepository) { + oldObj.Spec = sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeOCI, + URL: "oci://foo/bar", + } + *newObj = *oldObj.DeepCopy() + newObj.Spec.URL = "oci://foo/baz" + }, + want: false, + }, + { + name: "migrate old oci repo with status only", + beforeFunc: func(oldObj, newObj *sourcev1.HelmRepository) { + oldObj.Generation = 2 + oldObj.Spec = sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeOCI, + } + oldObj.Status = sourcev1.HelmRepositoryStatus{ + ObservedGeneration: 2, + } + conditions.MarkTrue(oldObj, meta.ReadyCondition, "foo", "bar") - repoB := &sourcev1.HelmRepository{Spec: sourcev1.HelmRepositorySpec{ - Type: sourcev1.HelmRepositoryTypeOCI, - }} + *newObj = *oldObj.DeepCopy() + newObj.Generation = 3 + }, + want: true, + }, + { + name: "migrate old oci repo with finalizer only", + beforeFunc: func(oldObj, newObj *sourcev1.HelmRepository) { + oldObj.Generation = 2 + oldObj.Finalizers = []string{sourcev1.SourceFinalizer} + oldObj.Spec = sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeOCI, + } - empty := &sourcev1.HelmRepository{} - not := &unstructured.Unstructured{} + *newObj = *oldObj.DeepCopy() + newObj.Generation = 3 + }, + want: true, + }, + { + name: "type switch default to oci", + beforeFunc: func(oldObj, newObj *sourcev1.HelmRepository) { + oldObj.Spec = sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeDefault, + } + oldObj.Status = sourcev1.HelmRepositoryStatus{ + Artifact: &v1.Artifact{}, + URL: "http://some-address", + ObservedGeneration: 3, + } - 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}, + *newObj = *oldObj.DeepCopy() + newObj.Spec = sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeOCI, + } + }, + want: true, + }, + { + name: "type switch oci to default", + beforeFunc: func(oldObj, newObj *sourcev1.HelmRepository) { + oldObj.Spec = sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeOCI, + } + *newObj = *oldObj.DeepCopy() + newObj.Spec.Type = sourcev1.HelmRepositoryTypeDefault + }, + want: true, + }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) - so := HelmRepositoryTypePredicate{RepositoryType: "default"} + oldObj := &sourcev1.HelmRepository{} + newObj := oldObj.DeepCopy() + if tt.beforeFunc != nil { + tt.beforeFunc(oldObj, newObj) + } e := event.UpdateEvent{ - ObjectOld: tt.old, - ObjectNew: tt.new, + ObjectOld: oldObj, + ObjectNew: newObj, } - g.Expect(so.Update(e)).To(gomega.Equal(tt.want)) + p := HelmRepositoryOCIMigrationPredicate{} + g.Expect(p.Update(e)).To(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{} - +func TestHelmRepositoryOCIMigrationPredicate_Delete(t *testing.T) { tests := []struct { - name string - obj client.Object - want bool + name string + beforeFunc func(obj *sourcev1.HelmRepository) + 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}, + { + name: "oci with finalizer", + beforeFunc: func(obj *sourcev1.HelmRepository) { + obj.Finalizers = []string{sourcev1.SourceFinalizer} + obj.Spec.Type = sourcev1.HelmRepositoryTypeOCI + }, + want: true, + }, + { + name: "oci with status", + beforeFunc: func(obj *sourcev1.HelmRepository) { + obj.Spec.Type = sourcev1.HelmRepositoryTypeOCI + obj.Status.ObservedGeneration = 4 + }, + want: true, + }, + { + name: "oci without finalizer or status", + beforeFunc: func(obj *sourcev1.HelmRepository) { + obj.Spec.Type = sourcev1.HelmRepositoryTypeOCI + }, + want: false, + }, + { + name: "default helm repo", + beforeFunc: func(obj *sourcev1.HelmRepository) { + obj.Spec.Type = sourcev1.HelmRepositoryTypeDefault + }, + want: true, + }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) - so := HelmRepositoryTypePredicate{RepositoryType: "default"} - e := event.DeleteEvent{ - Object: tt.obj, + obj := &sourcev1.HelmRepository{} + if tt.beforeFunc != nil { + tt.beforeFunc(obj) } - g.Expect(so.Delete(e)).To(gomega.Equal(tt.want)) + e := event.DeleteEvent{Object: obj} + p := HelmRepositoryOCIMigrationPredicate{} + g.Expect(p.Delete(e)).To(Equal(tt.want)) }) } } diff --git a/main.go b/main.go index 315ded717..e2bd08efb 100644 --- a/main.go +++ b/main.go @@ -203,19 +203,6 @@ func main() { os.Exit(1) } - if err := (&controller.HelmRepositoryOCIReconciler{ - Client: mgr.GetClient(), - EventRecorder: eventRecorder, - Metrics: metrics, - ControllerName: controllerName, - RegistryClientGenerator: registry.ClientGenerator, - }).SetupWithManagerAndOptions(mgr, controller.HelmRepositoryReconcilerOptions{ - RateLimiter: helper.GetRateLimiter(rateLimiterOptions), - }); err != nil { - setupLog.Error(err, "unable to create controller", "controller", v1beta2.HelmRepositoryKind, "type", "OCI") - os.Exit(1) - } - if err := (&controller.HelmRepositoryReconciler{ Client: mgr.GetClient(), EventRecorder: eventRecorder,