Ensure invalid regex errors are reported to user

The errors generated by ImagePolicy resources having invalid tag filter
regex patterns were not being reported to the user correctly, no event
was recorded and the ReadyCondition message did not reflect the
encountered error.

Fixes #138

Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>

controllers/imagepolicy_controller.go

@@ -118,29 +118,31 @@ func (r *ImagePolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, nil
 	}
 
+	var err error
 	policer, err := policy.PolicerFromSpec(pol.Spec.Policy)
 
 	var latest string
 	if policer != nil {
-		tags, err := r.Database.Tags(repo.Status.CanonicalImageName)
+		var tags []string
+		tags, err = r.Database.Tags(repo.Status.CanonicalImageName)
 		if err == nil {
 			var filter *policy.RegexFilter
 			if pol.Spec.FilterTags != nil {
 				filter, err = policy.NewRegexFilter(pol.Spec.FilterTags.Pattern, pol.Spec.FilterTags.Extract)
-				if err != nil {
-					return ctrl.Result{}, err
-				}
-				filter.Apply(tags)
-				tags = filter.Items()
-				latest, err = policer.Latest(tags)
 				if err == nil {
-					latest = filter.GetOriginalTag(latest)
+					filter.Apply(tags)
+					tags = filter.Items()
+					latest, err = policer.Latest(tags)
+					if err == nil {
+						latest = filter.GetOriginalTag(latest)
+					}
 				}
 			} else {
 				latest, err = policer.Latest(tags)
 			}
 		}
 	}
+
 	if err != nil {
 		imagev1.SetImagePolicyReadiness(
 			&pol,

controllers/policy_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/fluxcd/pkg/apis/meta"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 
@@ -35,17 +36,17 @@ import (
 
 var _ = Describe("ImagePolicy controller", func() {
 
+	var registryServer *httptest.Server
+
+	BeforeEach(func() {
+		registryServer = newRegistryServer()
+	})
+
+	AfterEach(func() {
+		registryServer.Close()
+	})
+
 	Context("calculates an image from a repository's tags", func() {
-		var registryServer *httptest.Server
-
-		BeforeEach(func() {
-			registryServer = newRegistryServer()
-		})
-
-		AfterEach(func() {
-			registryServer.Close()
-		})
-
 		When("Using SemVerPolicy", func() {
 			It("calculates an image from a repository's tags", func() {
 				versions := []string{"0.1.0", "0.1.1", "0.2.0", "1.0.0", "1.0.1", "1.0.2", "1.1.0-alpha"}
@@ -171,4 +172,146 @@ var _ = Describe("ImagePolicy controller", func() {
 			})
 		})
 	})
+
+	Context("Using FilterTags", func() {
+		When("valid regex supplied", func() {
+			It("correctly filters the repo tags", func() {
+				versions := []string{"test-0.1.0", "test-0.1.1", "dev-0.2.0", "1.0.0", "1.0.1", "1.0.2", "1.1.0-alpha"}
+				imgRepo := loadImages(registryServer, "test-semver-policy-"+randStringRunes(5), versions)
+				repo := imagev1.ImageRepository{
+					Spec: imagev1.ImageRepositorySpec{
+						Interval: metav1.Duration{Duration: reconciliationInterval},
+						Image:    imgRepo,
+					},
+				}
+				imageObjectName := types.NamespacedName{
+					Name:      "polimage-" + randStringRunes(5),
+					Namespace: "default",
+				}
+				repo.Name = imageObjectName.Name
+				repo.Namespace = imageObjectName.Namespace
+
+				ctx, cancel := context.WithTimeout(context.Background(), contextTimeout)
+				defer cancel()
+
+				r := imageRepoReconciler
+				Expect(r.Create(ctx, &repo)).To(Succeed())
+
+				Eventually(func() bool {
+					err := r.Get(ctx, imageObjectName, &repo)
+					return err == nil && repo.Status.LastScanResult != nil
+				}, timeout, interval).Should(BeTrue())
+				Expect(repo.Status.CanonicalImageName).To(Equal(imgRepo))
+				Expect(repo.Status.LastScanResult.TagCount).To(Equal(len(versions)))
+
+				polName := types.NamespacedName{
+					Name:      "random-pol-" + randStringRunes(5),
+					Namespace: imageObjectName.Namespace,
+				}
+				pol := imagev1.ImagePolicy{
+					Spec: imagev1.ImagePolicySpec{
+						ImageRepositoryRef: meta.LocalObjectReference{
+							Name: imageObjectName.Name,
+						},
+						FilterTags: &imagev1.TagFilter{
+							Pattern: "^test-(.*)$",
+							Extract: "$1",
+						},
+						Policy: imagev1.ImagePolicyChoice{
+							SemVer: &imagev1.SemVerPolicy{
+								Range: ">=0.x",
+							},
+						},
+					},
+				}
+				pol.Namespace = polName.Namespace
+				pol.Name = polName.Name
+
+				ctx, cancel = context.WithTimeout(context.Background(), contextTimeout)
+				defer cancel()
+
+				Expect(r.Create(ctx, &pol)).To(Succeed())
+
+				Eventually(func() bool {
+					err := r.Get(ctx, polName, &pol)
+					return err == nil && pol.Status.LatestImage != ""
+				}, timeout, interval).Should(BeTrue())
+				Expect(pol.Status.LatestImage).To(Equal(imgRepo + ":test-0.1.1"))
+
+				Expect(r.Delete(ctx, &pol)).To(Succeed())
+			})
+		})
+
+		When("invalid regex supplied", func() {
+			It("fails to reconcile returning error", func() {
+				versions := []string{"test-0.1.0", "test-0.1.1", "dev-0.2.0", "1.0.0", "1.0.1", "1.0.2", "1.1.0-alpha"}
+				imgRepo := loadImages(registryServer, "test-semver-policy-"+randStringRunes(5), versions)
+				repo := imagev1.ImageRepository{
+					Spec: imagev1.ImageRepositorySpec{
+						Interval: metav1.Duration{Duration: reconciliationInterval},
+						Image:    imgRepo,
+					},
+				}
+				imageObjectName := types.NamespacedName{
+					Name:      "polimage-" + randStringRunes(5),
+					Namespace: "default",
+				}
+				repo.Name = imageObjectName.Name
+				repo.Namespace = imageObjectName.Namespace
+
+				ctx, cancel := context.WithTimeout(context.Background(), contextTimeout)
+				defer cancel()
+
+				r := imageRepoReconciler
+				Expect(r.Create(ctx, &repo)).To(Succeed())
+
+				Eventually(func() bool {
+					err := r.Get(ctx, imageObjectName, &repo)
+					return err == nil && repo.Status.LastScanResult != nil
+				}, timeout, interval).Should(BeTrue())
+				Expect(repo.Status.CanonicalImageName).To(Equal(imgRepo))
+				Expect(repo.Status.LastScanResult.TagCount).To(Equal(len(versions)))
+
+				polName := types.NamespacedName{
+					Name:      "random-pol-" + randStringRunes(5),
+					Namespace: imageObjectName.Namespace,
+				}
+				pol := imagev1.ImagePolicy{
+					Spec: imagev1.ImagePolicySpec{
+						ImageRepositoryRef: meta.LocalObjectReference{
+							Name: imageObjectName.Name,
+						},
+						FilterTags: &imagev1.TagFilter{
+							Pattern: "^test-(.*",
+							Extract: "$1",
+						},
+						Policy: imagev1.ImagePolicyChoice{
+							SemVer: &imagev1.SemVerPolicy{
+								Range: ">=0.x",
+							},
+						},
+					},
+				}
+				pol.Namespace = polName.Namespace
+				pol.Name = polName.Name
+
+				ctx, cancel = context.WithTimeout(context.Background(), contextTimeout)
+				defer cancel()
+
+				// Currently succeeds creating the resources as there's no
+				// admission webhook validation
+				Expect(r.Create(ctx, &pol)).To(Succeed())
+
+				Eventually(func() bool {
+					err := r.Get(ctx, polName, &pol)
+					return err == nil && apimeta.IsStatusConditionFalse(pol.Status.Conditions, meta.ReadyCondition)
+				}, timeout, interval).Should(BeTrue())
+
+				ready := apimeta.FindStatusCondition(pol.Status.Conditions, meta.ReadyCondition)
+				Expect(ready.Message).To(ContainSubstring("invalid regular expression pattern"))
+
+				Expect(r.Delete(ctx, &pol)).To(Succeed())
+			})
+		})
+	})
 })