Skip to content

Commit

Permalink
Remove TransformLegacyRevision from v1
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
hiddeco committed Mar 28, 2023
1 parent 861343d commit 19ba61a
Show file tree
Hide file tree
Showing 9 changed files with 8 additions and 225 deletions.
60 changes: 1 addition & 59 deletions api/v1/artifact_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package v1

import (
"path"
"regexp"
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -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
Expand All @@ -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 ""
}
78 changes: 0 additions & 78 deletions api/v1/artifact_types_test.go

This file was deleted.

5 changes: 0 additions & 5 deletions api/v1/gitrepository_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
6 changes: 3 additions & 3 deletions controllers/bucket_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions controllers/bucket_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
65 changes: 0 additions & 65 deletions controllers/gitrepository_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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()
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion controllers/helmchart_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/helmrepository_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand Down
11 changes: 0 additions & 11 deletions controllers/ocirepository_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 19ba61a

Please sign in to comment.