From f3ab2e0d46aa564b06c1baaf06a9f558bca2d9a7 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Thu, 14 Jul 2022 00:38:39 +0200 Subject: [PATCH] Fix Panic when no artifact in source If implemented, the helmrepository type will be used to decide whether a reconciliation can continue in the absence of source artifact, instead of url. Signed-off-by: Soule BA --- controllers/helmchart_controller.go | 3 +- controllers/helmchart_controller_test.go | 150 +++++++++++++++++++++++ 2 files changed, 152 insertions(+), 1 deletion(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index d2c1ad60..a3bd7d6a 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -392,7 +392,8 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1 // Assert source has an artifact if s.GetArtifact() == nil || !r.Storage.ArtifactExist(*s.GetArtifact()) { - if helmRepo, ok := s.(*sourcev1.HelmRepository); !ok || !helmreg.IsOCI(helmRepo.Spec.URL) { + // Set the condition to indicate that the source has no artifact for all types except OCI HelmRepository + if helmRepo, ok := s.(*sourcev1.HelmRepository); !ok || helmRepo.Spec.Type != sourcev1.HelmRepositoryTypeOCI { 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", diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 56795f2b..a9805970 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -515,6 +515,156 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) { } } +func TestHelmChartReconciler_reconcileFromHelmRepository(t *testing.T) { + g := NewWithT(t) + + const ( + chartName = "helmchart" + chartVersion = "0.2.0" + higherChartVersion = "0.3.0" + chartPath = "testdata/charts/helmchart" + ) + + serverFactory, err := helmtestserver.NewTempHelmServer() + g.Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(serverFactory.Root()) + + for _, ver := range []string{chartVersion, higherChartVersion} { + g.Expect(serverFactory.PackageChartWithVersion(chartPath, ver)).To(Succeed()) + } + g.Expect(serverFactory.GenerateIndex()).To(Succeed()) + + tests := []struct { + name string + beforeFunc func(repository *sourcev1.HelmRepository) + assertFunc func(g *WithT, obj *sourcev1.HelmChart) + }{ + { + name: "Reconciles chart build", + assertFunc: func(g *WithT, obj *sourcev1.HelmChart) { + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + // Wait for HelmChart 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 obj.Generation == readyCondition.ObservedGeneration && + obj.Generation == obj.Status.ObservedGeneration + }, timeout).Should(BeTrue()) + + // Check if the object status is valid. + condns := &status.Conditions{NegativePolarity: helmChartReadyCondition.NegativePolarity} + checker := status.NewChecker(testEnv.Client, condns) + checker.CheckErr(ctx, obj) + }, + }, + { + name: "Stalling on invalid repository URL", + beforeFunc: func(repository *sourcev1.HelmRepository) { + repository.Spec.URL = "://unsupported" // Invalid URL + }, + assertFunc: func(g *WithT, obj *sourcev1.HelmChart) { + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + // Wait for HelmChart to be FetchFailed == true + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + if !conditions.IsTrue(obj, sourcev1.FetchFailedCondition) { + return false + } + // observedGeneration is -1 because we have no successful reconciliation + return obj.Status.ObservedGeneration == -1 + }, timeout).Should(BeTrue()) + + // Check if the object status is valid. + condns := &status.Conditions{NegativePolarity: helmChartReadyCondition.NegativePolarity} + checker := status.NewChecker(testEnv.Client, condns) + checker.CheckErr(ctx, obj) + }, + }, + { + name: "Stalling on invalid oci repository URL", + beforeFunc: func(repository *sourcev1.HelmRepository) { + repository.Spec.URL = strings.Replace(repository.Spec.URL, "http", "oci", 1) + }, + assertFunc: func(g *WithT, obj *sourcev1.HelmChart) { + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + // Wait for HelmChart to be Ready + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + if !conditions.IsTrue(obj, sourcev1.FetchFailedCondition) { + return false + } + // observedGeneration is -1 because we have no successful reconciliation + return obj.Status.ObservedGeneration == -1 + }, timeout).Should(BeTrue()) + + // Check if the object status is valid. + condns := &status.Conditions{NegativePolarity: helmChartReadyCondition.NegativePolarity} + checker := status.NewChecker(testEnv.Client, condns) + checker.CheckErr(ctx, obj) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + server := testserver.NewHTTPServer(serverFactory.Root()) + server.Start() + defer server.Stop() + + ns, err := testEnv.CreateNamespace(ctx, "helmchart") + g.Expect(err).ToNot(HaveOccurred()) + defer func() { g.Expect(testEnv.Delete(ctx, ns)).To(Succeed()) }() + + repository := sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-", + Namespace: ns.Name, + }, + Spec: sourcev1.HelmRepositorySpec{ + URL: server.URL(), + }, + } + + if tt.beforeFunc != nil { + tt.beforeFunc(&repository) + } + + g.Expect(testEnv.CreateAndWait(ctx, &repository)).To(Succeed()) + + obj := sourcev1.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-reconcile-", + Namespace: ns.Name, + }, + Spec: sourcev1.HelmChartSpec{ + Chart: chartName, + Version: chartVersion, + SourceRef: sourcev1.LocalHelmChartSourceReference{ + Kind: sourcev1.HelmRepositoryKind, + Name: repository.Name, + }, + }, + } + g.Expect(testEnv.Create(ctx, &obj)).To(Succeed()) + + if tt.assertFunc != nil { + tt.assertFunc(g, &obj) + } + }) + } +} + func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { g := NewWithT(t)