Merge pull request #140 from fluxcd/fix-invalid-regex-handling

Ensure invalid regex errors are reported to user
fluxcd · Apr 29, 2021 · a82216d · a82216d
2 parents 41cf207 + 6ffbfa6
commit a82216d
Show file tree

Hide file tree

Showing 2 changed files with 161 additions and 16 deletions.
diff --git a/controllers/imagepolicy_controller.go b/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,

diff --git a/controllers/policy_test.go b/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() {
 
-	Context("calculates an image from a repository's tags", func() {
-		var registryServer *httptest.Server
+	var registryServer *httptest.Server
 
-		BeforeEach(func() {
-			registryServer = newRegistryServer()
-		})
+	BeforeEach(func() {
+		registryServer = newRegistryServer()
+	})
 
-		AfterEach(func() {
-			registryServer.Close()
-		})
+	AfterEach(func() {
+		registryServer.Close()
+	})
 
+	Context("calculates an image from a repository's tags", func() {
 		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())
+			})
+		})
+	})
 })