Merge pull request #780 from fluxcd/feat-778

Store checksum of ImageRepository tags and trigger ImagePolicy watch only when it changes

Author: matheuscscp

api/v1beta2/imagerepository_types.go

@@ -28,6 +28,7 @@ import (
 const ImageRepositoryKind = "ImageRepository"
 
 // Deprecated: Use ImageFinalizer.
+// TODO: Remove in v1.
 const ImageRepositoryFinalizer = ImageFinalizer
 
 // ImageRepositorySpec defines the parameters for scanning an image
@@ -115,10 +116,26 @@ type ImageRepositorySpec struct {
 	Insecure bool `json:"insecure,omitempty"`
 }
 
+// ScanResult contains information about the last scan of the image repository.
+// TODO: Make all fields except for LatestTags required in v1.
 type ScanResult struct {
-	TagCount   int         `json:"tagCount"`
-	ScanTime   metav1.Time `json:"scanTime,omitempty"`
-	LatestTags []string    `json:"latestTags,omitempty"`
+	// Revision is a stable hash of the scanned tags.
+	// +optional
+	Revision string `json:"revision"`
+
+	// TagCount is the number of tags found in the last scan.
+	// +required
+	TagCount int `json:"tagCount"`
+
+	// ScanTime is the time when the last scan was performed.
+	// +optional
+	ScanTime metav1.Time `json:"scanTime"`
+
+	// LatestTags is a small sample of the tags found in the last scan.
+	// It's the first 10 tags when sorting all the tags in descending
+	// alphabetical order.
+	// +optional
+	LatestTags []string `json:"latestTags,omitempty"`
 }
 
 // ImageRepositoryStatus defines the observed state of ImageRepository

config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml

@@ -483,13 +483,23 @@ spec:
                 description: LastScanResult contains the number of fetched tags.
                 properties:
                   latestTags:
+                    description: |-
+                      LatestTags is a small sample of the tags found in the last scan.
+                      It's the first 10 tags when sorting all the tags in descending
+                      alphabetical order.
                     items:
                       type: string
                     type: array
+                  revision:
+                    description: Revision is a stable hash of the scanned tags.
+                    type: string
                   scanTime:
+                    description: ScanTime is the time when the last scan was performed.
                     format: date-time
                     type: string
                   tagCount:
+                    description: TagCount is the number of tags found in the last
+                      scan.
                     type: integer
                 required:
                 - tagCount

docs/api/v1beta2/image-reflector.md

@@ -1098,6 +1098,8 @@ would select 0.</p>
 (<em>Appears on:</em>
 <a href="#image.toolkit.fluxcd.io/v1beta2.ImageRepositoryStatus">ImageRepositoryStatus</a>)
 </p>
+<p>ScanResult contains information about the last scan of the image repository.
+TODO: Make all fields except for LatestTags required in v1.</p>
 <div class="md-typeset__scrollwrap">
 <div class="md-typeset__table">
 <table>
@@ -1110,12 +1112,25 @@ would select 0.</p>
 <tbody>
 <tr>
 <td>
+<code>revision</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Revision is a stable hash of the scanned tags.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>tagCount</code><br>
 <em>
 int
 </em>
 </td>
 <td>
+<p>TagCount is the number of tags found in the last scan.</p>
 </td>
 </tr>
 <tr>
@@ -1128,6 +1143,8 @@ Kubernetes meta/v1.Time
 </em>
 </td>
 <td>
+<em>(Optional)</em>
+<p>ScanTime is the time when the last scan was performed.</p>
 </td>
 </tr>
 <tr>
@@ -1138,6 +1155,10 @@ Kubernetes meta/v1.Time
 </em>
 </td>
 <td>
+<em>(Optional)</em>
+<p>LatestTags is a small sample of the tags found in the last scan.
+It&rsquo;s the first 10 tags when sorting all the tags in descending
+alphabetical order.</p>
 </td>
 </tr>
 </tbody>

internal/controller/database.go

@@ -18,7 +18,7 @@ package controller
 
 // DatabaseWriter implementations record the tags for an image repository.
 type DatabaseWriter interface {
-	SetTags(repo string, tags []string) error
+	SetTags(repo string, tags []string) (string, error)
 }
 
 // DatabaseReader implementations get the stored set of tags for an image

internal/controller/imagepolicy_controller.go

@@ -164,15 +164,18 @@ func (imageRepositoryPredicate) Update(e event.UpdateEvent) bool {
 		return false
 	}
 
-	// This is a temporary workaround to avoid reconciling ImagePolicy
-	// when the ImageRepository is not ready. In the near future, we
-	// will implement a digest for the scanned tags in the ImageRepository
-	// and will only return true here if the digest has changed, which
-	// covers not only skipping the reconciliation when the ImageRepository
-	// is not ready, but also when the tags have not changed.
-	repo := e.ObjectNew.(*imagev1.ImageRepository)
-	return conditions.IsReady(repo) &&
-		conditions.GetObservedGeneration(repo, meta.ReadyCondition) == repo.Generation
+	newRepo := e.ObjectNew.(*imagev1.ImageRepository)
+	if newRepo.Status.LastScanResult == nil {
+		return false
+	}
+
+	oldRepo := e.ObjectOld.(*imagev1.ImageRepository)
+	if oldRepo.Status.LastScanResult == nil ||
+		oldRepo.Status.LastScanResult.Revision != newRepo.Status.LastScanResult.Revision {
+		return true
+	}
+
+	return false
 }
 
 func (r *ImagePolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, retErr error) {

internal/controller/imagepolicy_controller_test.go

@@ -185,6 +185,147 @@ func TestImagePolicyReconciler_ignoresImageRepoNotReadyEvent(t *testing.T) {
 	}).Should(BeTrue())
 }
 
+func TestImagePolicyReconciler_imageRepoRevisionLifeCycle(t *testing.T) {
+	g := NewWithT(t)
+
+	namespaceName := "imagepolicy-" + 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())
+	})
+
+	imageRepo := &imagev1.ImageRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespaceName,
+			Name:      "repo",
+		},
+		Spec: imagev1.ImageRepositorySpec{
+			Image: "ghcr.io/stefanprodan/podinfo",
+		},
+	}
+	g.Expect(k8sClient.Create(ctx, imageRepo)).NotTo(HaveOccurred())
+	t.Cleanup(func() {
+		g.Expect(k8sClient.Delete(ctx, imageRepo)).NotTo(HaveOccurred())
+	})
+
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imageRepo), imageRepo)
+		return err == nil && conditions.IsReady(imageRepo) &&
+			imageRepo.Generation == conditions.GetObservedGeneration(imageRepo, meta.ReadyCondition)
+	}, timeout).Should(BeTrue())
+
+	imagePolicy := &imagev1.ImagePolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespaceName,
+			Name:      "test-imagepolicy",
+		},
+		Spec: imagev1.ImagePolicySpec{
+			ImageRepositoryRef: meta.NamespacedObjectReference{
+				Name: imageRepo.Name,
+			},
+			FilterTags: &imagev1.TagFilter{
+				Pattern: `^6\.7\.\d+$`,
+			},
+			Policy: imagev1.ImagePolicyChoice{
+				SemVer: &imagev1.SemVerPolicy{
+					Range: "6.7.x",
+				},
+			},
+		},
+	}
+	g.Expect(k8sClient.Create(ctx, imagePolicy)).NotTo(HaveOccurred())
+	t.Cleanup(func() {
+		g.Expect(k8sClient.Delete(ctx, imagePolicy)).NotTo(HaveOccurred())
+	})
+
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imagePolicy), imagePolicy)
+		return err == nil && conditions.IsReady(imagePolicy) &&
+			imagePolicy.Generation == conditions.GetObservedGeneration(imagePolicy, meta.ReadyCondition) &&
+			imagePolicy.Status.LatestRef != nil &&
+			imagePolicy.Status.LatestRef.Tag == "6.7.1"
+	}, timeout).Should(BeTrue())
+	expectedImagePolicyLastTransitionTime := conditions.GetLastTransitionTime(imagePolicy, meta.ReadyCondition).Time
+
+	// Now force a reconciliation by setting the annotation.
+	var requestedAt string
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imageRepo), imageRepo)
+		if err != nil {
+			return false
+		}
+		p := patch.NewSerialPatcher(imageRepo, k8sClient)
+		requestedAt = time.Now().Format(time.RFC3339Nano)
+		if imageRepo.Annotations == nil {
+			imageRepo.Annotations = make(map[string]string)
+		}
+		imageRepo.Annotations["reconcile.fluxcd.io/requestedAt"] = requestedAt
+		return p.Patch(ctx, imageRepo) == nil
+	}, timeout).Should(BeTrue())
+
+	// Wait for the ImageRepository to reconcile.
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imageRepo), imageRepo)
+		return err == nil && conditions.IsReady(imageRepo) &&
+			imageRepo.Status.LastHandledReconcileAt == requestedAt
+	}, timeout).Should(BeTrue())
+
+	// Check that the ImagePolicy is still ready and does not get updated.
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imagePolicy), imagePolicy)
+		return err == nil && conditions.IsReady(imagePolicy) &&
+			imagePolicy.Status.LatestRef != nil &&
+			imagePolicy.Status.LatestRef.Tag == "6.7.1"
+	}, timeout).Should(BeTrue())
+
+	// Wait a bit and check that the ImagePolicy remains ready.
+	time.Sleep(time.Second)
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imagePolicy), imagePolicy)
+		return err == nil && conditions.IsReady(imagePolicy) &&
+			imagePolicy.Status.LatestRef != nil &&
+			imagePolicy.Status.LatestRef.Tag == "6.7.1"
+	}, timeout).Should(BeTrue())
+
+	// Check that the last transition time of the ImagePolicy Ready condition did not change since the beginning.
+	lastTransitionTime := conditions.GetLastTransitionTime(imagePolicy, meta.ReadyCondition).Time
+	g.Expect(lastTransitionTime).To(Equal(expectedImagePolicyLastTransitionTime))
+
+	// Now add an exclusion rule to force the checksum to change.
+	firstChecksum := imageRepo.Status.LastScanResult.Revision
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imageRepo), imageRepo)
+		if err != nil {
+			return false
+		}
+		p := patch.NewSerialPatcher(imageRepo, k8sClient)
+		imageRepo.Spec.ExclusionList = []string{`^6\.7\.1$`}
+		return p.Patch(ctx, imageRepo) == nil
+	}, timeout).Should(BeTrue())
+
+	// Wait for the ImageRepository to reconcile.
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imageRepo), imageRepo)
+		return err == nil && conditions.IsReady(imageRepo) &&
+			imageRepo.Generation == conditions.GetObservedGeneration(imageRepo, meta.ReadyCondition) &&
+			imageRepo.Status.LastScanResult.Revision != firstChecksum
+	}, timeout).Should(BeTrue())
+
+	// Check that the ImagePolicy receives the update and the latest tag changes.
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imagePolicy), imagePolicy)
+		if err != nil {
+			return false
+		}
+		return conditions.IsReady(imagePolicy) &&
+			imagePolicy.Generation == conditions.GetObservedGeneration(imagePolicy, meta.ReadyCondition) &&
+			imagePolicy.Status.LatestRef.Tag == "6.7.0"
+	}, timeout).Should(BeTrue())
+}
+
 func TestImagePolicyReconciler_invalidImage(t *testing.T) {
 	g := NewWithT(t)