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 <hidde@hhh.computer>
fluxcd · Mar 28, 2023 · 161806b · 161806b
1 parent e4d47ec
commit 161806b
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 42 deletions.
diff --git 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/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
@@ -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{