policy: Handle failure due to invalid semver range

controllers/imagepolicy_controller.go

@@ -146,8 +146,21 @@ func (r *ImagePolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, nil
 	}
 
-	var err error
 	policer, err := policy.PolicerFromSpec(pol.Spec.Policy)
+	if err != nil {
+		msg := fmt.Sprintf("invalid policy: %s", err.Error())
+		imagev1.SetImagePolicyReadiness(
+			&pol,
+			metav1.ConditionFalse,
+			"InvalidPolicy",
+			msg,
+		)
+		if err := r.patchStatus(ctx, req, pol.Status); err != nil {
+			return ctrl.Result{Requeue: true}, err
+		}
+		log.Error(err, "invalid policy")
+		return ctrl.Result{}, nil
+	}
 
 	var latest string
 	if policer != nil {

controllers/policy_test.go

@@ -113,6 +113,72 @@ var _ = Describe("ImagePolicy controller", func() {
 			})
 		})
 
+		When("Using SemVerPolicy with invalid range", func() {
+			It("fails with invalid policy error", 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"}
+				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.NamespacedObjectReference{
+							Name: imageObjectName.Name,
+						},
+						Policy: imagev1.ImagePolicyChoice{
+							SemVer: &imagev1.SemVerPolicy{
+								Range: "*-*",
+							},
+						},
+					},
+				}
+				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 && 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 policy"))
+
+				Expect(r.Delete(ctx, &pol)).To(Succeed())
+			})
+		})
+
 		When("Usign AlphabeticalPolicy", func() {
 			It("calculates an image from a repository's tags", func() {
 				versions := []string{"xenial", "yakkety", "zesty", "artful", "bionic"}

internal/policy/factory.go

@@ -38,5 +38,8 @@ func PolicerFromSpec(choice imagev1.ImagePolicyChoice) (Policer, error) {
 		return nil, fmt.Errorf("given ImagePolicyChoice object is invalid")
 	}
 
-	return p, err
+	if err != nil {
+		return nil, err
+	}
+	return p, nil
 }

internal/policy/factory_test.go

@@ -40,4 +40,13 @@ func TestFactory_PolicerFromSpec(t *testing.T) {
 	if err != nil {
 		t.Error("should not return error")
 	}
+
+	// A nil checkable Policer for invalid policy.
+	p, err := PolicerFromSpec(imagev1.ImagePolicyChoice{SemVer: &imagev1.SemVerPolicy{Range: "*-*"}})
+	if err == nil {
+		t.Error("should return error")
+	}
+	if p != nil {
+		t.Error("should be nil")
+	}
 }