controllers: use RFC-0005 format for Git revision

Signed-off-by: Hidde Beydals <[email protected]>

Author: hiddeco

controllers/artifact.go

@@ -21,15 +21,19 @@ import sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
 type artifactSet []*sourcev1.Artifact
 
 // Diff returns true if any of the revisions in the artifactSet does not match any of the given artifacts.
-func (s artifactSet) Diff(set artifactSet) bool {
+func (s artifactSet) Diff(set artifactSet, comp func(x, y *sourcev1.Artifact) bool) bool {
 	if len(s) != len(set) {
 		return true
 	}
 
+	if comp == nil {
+		comp = defaultCompare
+	}
+
 outer:
 	for _, j := range s {
 		for _, k := range set {
-			if k.HasRevision(j.Revision) {
+			if comp(j, k) {
 				continue outer
 			}
 		}
@@ -38,24 +42,10 @@ outer:
 	return false
 }
 
-// hasArtifactUpdated returns true if any of the revisions in the current artifacts
-// does not match any of the artifacts in the updated artifacts
-// NOTE: artifactSet is a replacement for this. Remove this once it's not used
-// anywhere.
-func hasArtifactUpdated(current []*sourcev1.Artifact, updated []*sourcev1.Artifact) bool {
-	if len(current) != len(updated) {
-		return true
+func defaultCompare(x, y *sourcev1.Artifact) bool {
+	if y == nil {
+		return false
 	}
-
-OUTER:
-	for _, c := range current {
-		for _, u := range updated {
-			if u.HasRevision(c.Revision) {
-				continue OUTER
-			}
-		}
-		return true
-	}
-
-	return false
+	return x.HasRevision(y.Revision)
 }
+

controllers/artifact_test.go

@@ -115,7 +115,7 @@ func Test_artifactSet_Diff(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := tt.current.Diff(tt.updated)
+			result := tt.current.Diff(tt.updated, nil)
 			if result != tt.expected {
 				t.Errorf("Archive() result = %v, wantResult %v", result, tt.expected)
 			}

controllers/gitrepository_controller.go

@@ -516,7 +516,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 	}
 
 	// Observe if the artifacts still match the previous included ones
-	if artifacts.Diff(obj.Status.IncludedArtifacts) {
+	if artifacts.Diff(obj.Status.IncludedArtifacts, gitArtifactRevisionEqual) {
 		message := fmt.Sprintf("included artifacts differ from last observed includes")
 		if obj.Status.IncludedArtifacts != nil {
 			conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange", message)
@@ -593,7 +593,8 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 	}
 
 	// Mark observations about the revision on the object
-	if !obj.GetArtifact().HasRevision(commit.String()) {
+	if curArtifact := obj.Status.Artifact; curArtifact == nil ||
+		git.TransformRevision(curArtifact.Revision) != commit.String() {
 		message := fmt.Sprintf("new upstream revision '%s'", commit.String())
 		if obj.GetArtifact() != nil {
 			conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
@@ -626,20 +627,22 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pat
 
 	// Set the ArtifactInStorageCondition if there's no drift.
 	defer func() {
-		if obj.GetArtifact().HasRevision(artifact.Revision) &&
-			!includes.Diff(obj.Status.IncludedArtifacts) &&
+		if curArtifact := obj.GetArtifact(); curArtifact != nil &&
+			git.TransformRevision(curArtifact.Revision) == artifact.Revision &&
+			!includes.Diff(obj.Status.IncludedArtifacts, gitArtifactRevisionEqual) &&
 			!gitContentConfigChanged(obj, includes) {
 			conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition)
 			conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason,
-				"stored artifact for revision '%s'", artifact.Revision)
+				"stored artifact for revision '%s'", curArtifact.Revision)
 		}
 	}()
 
 	// The artifact is up-to-date
-	if obj.GetArtifact().HasRevision(artifact.Revision) &&
-		!includes.Diff(obj.Status.IncludedArtifacts) &&
+	if curArtifact := obj.GetArtifact(); curArtifact != nil &&
+		git.TransformRevision(curArtifact.Revision) == artifact.Revision &&
+		!includes.Diff(obj.Status.IncludedArtifacts, gitArtifactRevisionEqual) &&
 		!gitContentConfigChanged(obj, includes) {
-		r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.ArtifactUpToDateReason, "artifact up-to-date with remote revision: '%s'", artifact.Revision)
+		r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.ArtifactUpToDateReason, "artifact up-to-date with remote revision: '%s'", curArtifact.Revision)
 		return sreconcile.ResultSuccess, nil
 	}
 
@@ -1024,7 +1027,7 @@ func gitContentConfigChanged(obj *sourcev1.GitRepository, includes *artifactSet)
 		}
 
 		// Check if the included repositories are still the same.
-		if observedInclArtifact.Revision != currentIncl.Revision {
+		if git.TransformRevision(observedInclArtifact.Revision) != git.TransformRevision(currentIncl.Revision) {
 			return true
 		}
 		if observedInclArtifact.Checksum != currentIncl.Checksum {
@@ -1047,3 +1050,10 @@ func gitRepositoryIncludeEqual(a, b sourcev1.GitRepositoryInclude) bool {
 	}
 	return true
 }
+
+func gitArtifactRevisionEqual(x, y *sourcev1.Artifact) bool {
+	if x == nil || y == nil {
+		return false
+	}
+	return git.TransformRevision(x.Revision) == git.TransformRevision(y.Revision)
+}