Remove TransformLegacyRevision from v1

Consumers still relying on this should make use of `v1beta2` to
facilitate any transition.

In addition, remove the `*Implementation` constants for now removed
Git implemenations.

Signed-off-by: Hidde Beydals <hidde@hhh.computer>

Author: hiddeco

api/v1/artifact_types.go

@@ -18,7 +18,6 @@ package v1
 
 import (
 	"path"
-	"regexp"
 	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -68,7 +67,7 @@ func (in *Artifact) HasRevision(revision string) bool {
 	if in == nil {
 		return false
 	}
-	return TransformLegacyRevision(in.Revision) == TransformLegacyRevision(revision)
+	return in.Revision == revision
 }
 
 // HasDigest returns if the given digest matches the current Digest of the
@@ -92,60 +91,3 @@ func ArtifactDir(kind, namespace, name string) string {
 func ArtifactPath(kind, namespace, name, filename string) string {
 	return path.Join(ArtifactDir(kind, namespace, name), filename)
 }
-
-// TransformLegacyRevision transforms a "legacy" revision string into a "new"
-// revision string. It accepts the following formats:
-//
-//   - main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738
-//   - feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738
-//   - HEAD/5394cb7f48332b2de7c17dd8b8384bbc84b7e738
-//   - tag/55609ff9d959589ed917ce32e6bc0f0a36809565f308602c15c3668965979edc
-//   - d52bde83c5b2bd0fa7910264e0afc3ac9cfe9b6636ca29c05c09742f01d5a4bd
-//
-// Which are transformed into the following formats respectively:
-//
-//   - main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738
-//   - feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738
-//   - sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738
-//   - tag@sha256:55609ff9d959589ed917ce32e6bc0f0a36809565f308602c15c3668965979edc
-//   - sha256:d52bde83c5b2bd0fa7910264e0afc3ac9cfe9b6636ca29c05c09742f01d5a4bd
-//
-// Deprecated, this function exists for backwards compatibility with existing
-// resources, and to provide a transition period. Will be removed in a future
-// release.
-func TransformLegacyRevision(rev string) string {
-	if rev != "" && strings.LastIndex(rev, ":") == -1 {
-		if i := strings.LastIndex(rev, "/"); i >= 0 {
-			sha := rev[i+1:]
-			if algo := determineSHAType(sha); algo != "" {
-				if name := rev[:i]; name != "HEAD" {
-					return name + "@" + algo + ":" + sha
-				}
-				return algo + ":" + sha
-			}
-		}
-		if algo := determineSHAType(rev); algo != "" {
-			return algo + ":" + rev
-		}
-	}
-	return rev
-}
-
-// isAlphaNumHex returns true if the given string only contains 0-9 and a-f
-// characters.
-var isAlphaNumHex = regexp.MustCompile(`^[0-9a-f]+$`).MatchString
-
-// determineSHAType returns the SHA algorithm used to compute the provided hex.
-// The determination is heuristic and based on the length of the hex string. If
-// the size is not recognized, an empty string is returned.
-func determineSHAType(hex string) string {
-	if isAlphaNumHex(hex) {
-		switch len(hex) {
-		case 40:
-			return "sha1"
-		case 64:
-			return "sha256"
-		}
-	}
-	return ""
-}

api/v1/artifact_types_test.go

@@ -27,11 +27,6 @@ import (
 const (
 	// GitRepositoryKind is the string representation of a GitRepository.
 	GitRepositoryKind = "GitRepository"
-
-	// GoGitImplementation for performing Git operations using go-git.
-	GoGitImplementation = "go-git"
-	// LibGit2Implementation for performing Git operations using libgit2.
-	LibGit2Implementation = "libgit2"
 )
 
 const (

api/v1/gitrepository_types.go

@@ -459,7 +459,7 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial
 	// Check if index has changed compared to current Artifact revision.
 	var changed bool
 	if artifact := obj.Status.Artifact; artifact != nil && artifact.Revision != "" {
-		curRev := digest.Digest(sourcev1.TransformLegacyRevision(artifact.Revision))
+		curRev := digest.Digest(artifact.Revision)
 		changed = curRev.Validate() != nil || curRev != index.Digest(curRev.Algorithm())
 	}
 
@@ -512,7 +512,7 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, sp *patch.Seri
 	// Set the ArtifactInStorageCondition if there's no drift.
 	defer func() {
 		if curArtifact := obj.GetArtifact(); curArtifact != nil && curArtifact.Revision != "" {
-			curRev := digest.Digest(sourcev1.TransformLegacyRevision(curArtifact.Revision))
+			curRev := digest.Digest(curArtifact.Revision)
 			if curRev.Validate() == nil && index.Digest(curRev.Algorithm()) == curRev {
 				conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition)
 				conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason,
@@ -523,7 +523,7 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, sp *patch.Seri
 
 	// The artifact is up-to-date
 	if curArtifact := obj.GetArtifact(); curArtifact != nil && curArtifact.Revision != "" {
-		curRev := digest.Digest(sourcev1.TransformLegacyRevision(curArtifact.Revision))
+		curRev := digest.Digest(curArtifact.Revision)
 		if curRev.Validate() == nil && index.Digest(curRev.Algorithm()) == curRev {
 			r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.ArtifactUpToDateReason, "artifact up-to-date with remote revision: '%s'", artifact.Revision)
 			return sreconcile.ResultSuccess, nil

controllers/bucket_controller.go

@@ -514,7 +514,7 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
 			bucketName: "dummy",
 			beforeFunc: func(obj *bucketv1.Bucket) {
 				obj.Status.Artifact = &sourcev1.Artifact{
-					Revision: "b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479",
+					Revision: "sha256:b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479",
 				}
 				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
 				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
@@ -856,7 +856,7 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
 			bucketName: "dummy",
 			beforeFunc: func(obj *bucketv1.Bucket) {
 				obj.Status.Artifact = &sourcev1.Artifact{
-					Revision: "b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479",
+					Revision: "sha256:b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479",
 				}
 				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
 				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")

controllers/bucket_controller_test.go

@@ -732,27 +732,6 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 			wantRevision:    "staging@sha1:<commit>",
 			wantReconciling: false,
 		},
-		{
-			name: "Optimized clone (legacy revision format)",
-			reference: &sourcev1.GitRepositoryRef{
-				Branch: "staging",
-			},
-			beforeFunc: func(obj *sourcev1.GitRepository, latestRev string) {
-				// Add existing artifact on the object and storage.
-				obj.Status = sourcev1.GitRepositoryStatus{
-					Artifact: &sourcev1.Artifact{
-						Revision: "staging/" + latestRev,
-						Path:     randStringRunes(10),
-					},
-				}
-				conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
-				conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "foo")
-			},
-			want:            sreconcile.ResultEmpty,
-			wantErr:         true,
-			wantRevision:    "staging@sha1:<commit>",
-			wantReconciling: false,
-		},
 		{
 			name: "Optimized clone different ignore",
 			reference: &sourcev1.GitRepositoryRef{
@@ -775,28 +754,6 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 			wantRevision:    "staging@sha1:<commit>",
 			wantReconciling: false,
 		},
-		{
-			name: "Optimized clone different ignore (legacy revision format)",
-			reference: &sourcev1.GitRepositoryRef{
-				Branch: "staging",
-			},
-			beforeFunc: func(obj *sourcev1.GitRepository, latestRev string) {
-				// Set new ignore value.
-				obj.Spec.Ignore = pointer.StringPtr("foo")
-				// Add existing artifact on the object and storage.
-				obj.Status = sourcev1.GitRepositoryStatus{
-					Artifact: &sourcev1.Artifact{
-						Revision: "staging/" + latestRev,
-						Path:     randStringRunes(10),
-					},
-				}
-				conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
-				conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "foo")
-			},
-			want:            sreconcile.ResultSuccess,
-			wantRevision:    "staging@sha1:<commit>",
-			wantReconciling: false,
-		},
 	}
 
 	server, err := gittestserver.NewTempGitServer()
@@ -956,28 +913,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main@sha1:b9b3feadba509cb9b22e968a5d27e96c2bc2ff91'"),
 			},
 		},
-		{
-			name:     "Up-to-date artifact with legacy revision format should not update status",
-			dir:      "testdata/git/repository",
-			includes: artifactSet{&sourcev1.Artifact{Revision: "main@sha1:b9b3feadba509cb9b22e968a5d27e96c2bc2ff91", Digest: "some-checksum"}},
-			beforeFunc: func(obj *sourcev1.GitRepository) {
-				obj.Spec.Interval = metav1.Duration{Duration: interval}
-				obj.Spec.Include = []sourcev1.GitRepositoryInclude{
-					{GitRepositoryRef: meta.LocalObjectReference{Name: "foo"}},
-				}
-				obj.Status.Artifact = &sourcev1.Artifact{Revision: "main/b9b3feadba509cb9b22e968a5d27e96c2bc2ff91"}
-				obj.Status.IncludedArtifacts = []*sourcev1.Artifact{{Revision: "main/b9b3feadba509cb9b22e968a5d27e96c2bc2ff91", Digest: "some-checksum"}}
-				obj.Status.ObservedInclude = obj.Spec.Include
-			},
-			afterFunc: func(t *WithT, obj *sourcev1.GitRepository) {
-				t.Expect(obj.Status.URL).To(BeEmpty())
-				t.Expect(obj.Status.Artifact.Revision).To(Equal("main/b9b3feadba509cb9b22e968a5d27e96c2bc2ff91"))
-			},
-			want: sreconcile.ResultSuccess,
-			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main/b9b3feadba509cb9b22e968a5d27e96c2bc2ff91'"),
-			},
-		},
 		{
 			name: "Spec ignore overwrite is taken into account",
 			dir:  "testdata/git/repository",

controllers/gitrepository_controller_test.go

@@ -794,7 +794,7 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
 			rev = git.ExtractHashFromRevision(rev).String()
 		}
 		if obj.Spec.SourceRef.Kind == helmv1.BucketKind {
-			if dig := digest.Digest(sourcev1.TransformLegacyRevision(rev)); dig.Validate() == nil {
+			if dig := digest.Digest(rev); dig.Validate() == nil {
 				rev = dig.Encoded()
 			}
 		}

controllers/helmchart_controller.go

@@ -489,7 +489,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
 	// Check if index has changed compared to current Artifact revision.
 	var changed bool
 	if artifact := obj.Status.Artifact; artifact != nil {
-		curRev := digest.Digest(sourcev1.TransformLegacyRevision(artifact.Revision))
+		curRev := digest.Digest(artifact.Revision)
 		changed = curRev.Validate() != nil || curRev != chartRepo.Digest(curRev.Algorithm())
 	}
 

controllers/helmrepository_controller.go

@@ -1317,17 +1317,6 @@ func TestOCIRepository_reconcileSource_noop(t *testing.T) {
 				g.Expect(artifact.Metadata).To(BeEmpty())
 			},
 		},
-		{
-			name: "noop - artifact revisions match (legacy)",
-			beforeFunc: func(obj *ociv1.OCIRepository) {
-				obj.Status.Artifact = &sourcev1.Artifact{
-					Revision: "6.1.5/8e4057c22d531d40e12b065443cb0d80394b7257c4dc557cb1fbd4dce892b86d",
-				}
-			},
-			afterFunc: func(g *WithT, artifact *sourcev1.Artifact) {
-				g.Expect(artifact.Metadata).To(BeEmpty())
-			},
-		},
 		{
 			name: "full reconcile - same rev, unobserved ignore",
 			beforeFunc: func(obj *ociv1.OCIRepository) {