From 4ab3c21dd83941e3f4e9e3927545ac5e60e66a00 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 28 Mar 2023 12:25:51 +0200 Subject: [PATCH] Delete `Status.URL` field from `GitRepository` v1 Usage of this field has not been recommended for a long time as it was best-effort based. Signed-off-by: Hidde Beydals --- api/v1/gitrepository_types.go | 6 ----- ...rce.toolkit.fluxcd.io_gitrepositories.yaml | 5 ---- controllers/gitrepository_controller.go | 22 +++++++++------- controllers/gitrepository_controller_test.go | 26 ------------------- docs/api/v1/source.md | 14 ---------- 5 files changed, 12 insertions(+), 61 deletions(-) diff --git a/api/v1/gitrepository_types.go b/api/v1/gitrepository_types.go index 70b38461..82353ee3 100644 --- a/api/v1/gitrepository_types.go +++ b/api/v1/gitrepository_types.go @@ -183,12 +183,6 @@ type GitRepositoryStatus struct { // +optional Conditions []metav1.Condition `json:"conditions,omitempty"` - // URL is the dynamic fetch link for the latest Artifact. - // It is provided on a "best effort" basis, and using the precise - // GitRepositoryStatus.Artifact data is recommended. - // +optional - URL string `json:"url,omitempty"` - // Artifact represents the last successful GitRepository reconciliation. // +optional Artifact *Artifact `json:"artifact,omitempty"` diff --git a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml index e5610057..2a8c334e 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml @@ -391,11 +391,6 @@ spec: description: ObservedRecurseSubmodules is the observed resource submodules configuration used to produce the current Artifact. type: boolean - url: - description: URL is the dynamic fetch link for the latest Artifact. - It is provided on a "best effort" basis, and using the precise GitRepositoryStatus.Artifact - data is recommended. - type: string type: object type: object served: true diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index e2e74b04..20fbbf85 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -392,7 +392,6 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patc var artifactMissing bool if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) { obj.Status.Artifact = nil - obj.Status.URL = "" artifactMissing = true // Remove the condition as the artifact doesn't exist. conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) @@ -415,7 +414,6 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patc // Always update URLs to ensure hostname is up-to-date // TODO(hidde): we may want to send out an event only if we notice the URL has changed r.Storage.SetArtifactURL(obj.GetArtifact()) - obj.Status.URL = r.Storage.SetHostname(obj.Status.URL) return sreconcile.ResultSuccess, nil } @@ -700,15 +698,19 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pat obj.Status.ObservedRecurseSubmodules = obj.Spec.RecurseSubmodules obj.Status.ObservedInclude = obj.Spec.Include - // Update symlink on a "best effort" basis - url, err := r.Storage.Symlink(artifact, "latest.tar.gz") - if err != nil { - r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.SymlinkUpdateFailedReason, - "failed to update status URL symlink: %s", err) - } - if url != "" { - obj.Status.URL = url + // Remove the deprecated symlink. + // TODO(hidde): remove 2 minor versions from introduction of v1. + symArtifact := artifact.DeepCopy() + symArtifact.Path = filepath.Join(filepath.Dir(symArtifact.Path), "latest.tar.gz") + if fi, err := os.Lstat(r.Storage.LocalPath(artifact)); err == nil { + if fi.Mode()&os.ModeSymlink != 0 { + if err := os.Remove(r.Storage.LocalPath(*symArtifact)); err != nil { + r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.SymlinkUpdateFailedReason, + "failed to remove (deprecated) symlink: %s", err) + } + } } + conditions.Delete(obj, sourcev1.StorageOperationFailedCondition) return sreconcile.ResultSuccess, nil } diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 14d840c2..da96b1f8 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -864,7 +864,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { }, afterFunc: func(t *WithT, obj *sourcev1.GitRepository) { t.Expect(obj.GetArtifact()).ToNot(BeNil()) - t.Expect(obj.Status.URL).ToNot(BeEmpty()) }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ @@ -885,7 +884,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { t.Expect(obj.GetArtifact()).ToNot(BeNil()) t.Expect(obj.GetArtifact().Digest).To(Equal("sha256:60a3bf69f337cb5ec9ebd00abefbb6e7f2a2cf27158ecf438d52b2035b184172")) t.Expect(obj.Status.IncludedArtifacts).ToNot(BeEmpty()) - t.Expect(obj.Status.URL).ToNot(BeEmpty()) }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ @@ -905,9 +903,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { obj.Status.IncludedArtifacts = []*sourcev1.Artifact{{Revision: "main@sha1:b9b3feadba509cb9b22e968a5d27e96c2bc2ff91", Digest: "some-checksum"}} obj.Status.ObservedInclude = obj.Spec.Include }, - afterFunc: func(t *WithT, obj *sourcev1.GitRepository) { - t.Expect(obj.Status.URL).To(BeEmpty()) - }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main@sha1:b9b3feadba509cb9b22e968a5d27e96c2bc2ff91'"), @@ -954,27 +949,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { afterFunc: func(t *WithT, obj *sourcev1.GitRepository) { t.Expect(obj.GetArtifact()).ToNot(BeNil()) t.Expect(obj.GetArtifact().Digest).To(Equal("sha256:60a3bf69f337cb5ec9ebd00abefbb6e7f2a2cf27158ecf438d52b2035b184172")) - t.Expect(obj.Status.URL).ToNot(BeEmpty()) - }, - want: sreconcile.ResultSuccess, - assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main@sha1:b9b3feadba509cb9b22e968a5d27e96c2bc2ff91'"), - }, - }, - { - name: "Creates latest symlink to the created artifact", - dir: "testdata/git/repository", - beforeFunc: func(obj *sourcev1.GitRepository) { - obj.Spec.Interval = metav1.Duration{Duration: interval} - }, - afterFunc: func(t *WithT, obj *sourcev1.GitRepository) { - t.Expect(obj.GetArtifact()).ToNot(BeNil()) - - localPath := testStorage.LocalPath(*obj.GetArtifact()) - symlinkPath := filepath.Join(filepath.Dir(localPath), "latest.tar.gz") - targetFile, err := os.Readlink(symlinkPath) - t.Expect(err).NotTo(HaveOccurred()) - t.Expect(localPath).To(Equal(targetFile)) }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ diff --git a/docs/api/v1/source.md b/docs/api/v1/source.md index 384c135c..820e956a 100644 --- a/docs/api/v1/source.md +++ b/docs/api/v1/source.md @@ -695,20 +695,6 @@ object.

-url
- -string - - - -(Optional) -

URL is the dynamic fetch link for the latest Artifact. -It is provided on a “best effort” basis, and using the precise -GitRepositoryStatus.Artifact data is recommended.

- - - - artifact