diff --git a/api/v1/artifact_types.go b/api/v1/artifact_types.go index 21e44bfa..e4dc00b8 100644 --- a/api/v1/artifact_types.go +++ b/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 "" -} diff --git a/api/v1/artifact_types_test.go b/api/v1/artifact_types_test.go deleted file mode 100644 index 844bef2c..00000000 --- a/api/v1/artifact_types_test.go +++ /dev/null @@ -1,78 +0,0 @@ -/* -Copyright 2023 The Flux authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1 - -import "testing" - -func TestTransformLegacyRevision(t *testing.T) { - tests := []struct { - rev string - want string - }{ - { - rev: "HEAD/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", - want: "sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", - }, - { - rev: "main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", - want: "main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", - }, - { - rev: "main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", - want: "main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", - }, - { - rev: "feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", - want: "feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", - }, - { - rev: "feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", - want: "feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", - }, - { - rev: "5ac85ca617f3774baff4ae0a420b810b2546dbc9af9f346b1d55c5ed9873c55c", - want: "sha256:5ac85ca617f3774baff4ae0a420b810b2546dbc9af9f346b1d55c5ed9873c55c", - }, - { - rev: "v1.0.0", - want: "v1.0.0", - }, - { - rev: "v1.0.0-rc1", - want: "v1.0.0-rc1", - }, - { - rev: "v1.0.0-rc1+metadata", - want: "v1.0.0-rc1+metadata", - }, - { - rev: "arbitrary/revision", - want: "arbitrary/revision", - }, - { - rev: "5394cb7f48332b2de7c17dd8b8384bbc84b7xxxx", - want: "5394cb7f48332b2de7c17dd8b8384bbc84b7xxxx", - }, - } - for _, tt := range tests { - t.Run(tt.rev, func(t *testing.T) { - if got := TransformLegacyRevision(tt.rev); got != tt.want { - t.Errorf("TransformLegacyRevision() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/api/v1/gitrepository_types.go b/api/v1/gitrepository_types.go index 2235c9bc..70b38461 100644 --- a/api/v1/gitrepository_types.go +++ b/api/v1/gitrepository_types.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 ( diff --git a/controllers/bucket_controller.go b/controllers/bucket_controller.go index ad54781d..c761a71f 100644 --- a/controllers/bucket_controller.go +++ b/controllers/bucket_controller.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 diff --git a/controllers/bucket_controller_test.go b/controllers/bucket_controller_test.go index 80b7967d..5050e348 100644 --- a/controllers/bucket_controller_test.go +++ b/controllers/bucket_controller_test.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") diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index bbea4e73..14d840c2 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -732,27 +732,6 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) wantRevision: "staging@sha1:", 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:", - wantReconciling: false, - }, { name: "Optimized clone different ignore", reference: &sourcev1.GitRepositoryRef{ @@ -775,28 +754,6 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) wantRevision: "staging@sha1:", 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:", - 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", diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 5826313d..acf3b4e3 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.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() } } diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index bd4d0077..868a12ef 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_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()) } diff --git a/controllers/ocirepository_controller_test.go b/controllers/ocirepository_controller_test.go index 7f3b172f..d3b73279 100644 --- a/controllers/ocirepository_controller_test.go +++ b/controllers/ocirepository_controller_test.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) {