From a3624a3f20855557e0dc5673d68f9f78495cdb26 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Tue, 10 Dec 2024 16:28:27 +0200 Subject: [PATCH] fix: Allow to delete repos with invalid urls (#20921) (#20975) (#21116) * fix: Allow to delete repos with invalid urls (#20921) Fixes #20921 Normalization of repo url is attempted during repo deletion before comparison with existing repos. Before this change, it'd fail on invalid urls and return "", resulting in permission denied. This ended up in a state where people can add repos with invalid urls but couldn't delete them. Return raw repo url if url parsing failed. Add a test to validate the deletion can be performed and make sure it fails without the main change. This needs to be cherry-picked to v2.11-2.13 * Don't modify the original NormalizeGitURL --------- Signed-off-by: Andrii Korotkov Co-authored-by: Andrii Korotkov <137232734+andrii-korotkov-verkada@users.noreply.github.com> --- server/repository/repository_test.go | 33 ++++++++++++++++++++++++++++ util/git/git.go | 14 ++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/server/repository/repository_test.go b/server/repository/repository_test.go index 72354633048dc..2b69aff2671a0 100644 --- a/server/repository/repository_test.go +++ b/server/repository/repository_test.go @@ -21,6 +21,7 @@ import ( "github.com/argoproj/argo-cd/v2/common" "github.com/argoproj/argo-cd/v2/pkg/apiclient/repository" + repositorypkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/repository" appsv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" fakeapps "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned/fake" appinformer "github.com/argoproj/argo-cd/v2/pkg/client/informers/externalversions" @@ -1057,3 +1058,35 @@ func TestGetRepository(t *testing.T) { }) } } + +func TestDeleteRepository(t *testing.T) { + repositories := map[string]string{ + "valid": "https://bitbucket.org/workspace/repo.git", + // Check a wrongly formatter repo as well, see https://github.com/argoproj/argo-cd/issues/20921 + "invalid": "git clone https://bitbucket.org/workspace/repo.git", + } + + kubeclientset := fake.NewSimpleClientset(&argocdCM, &argocdSecret) + settingsMgr := settings.NewSettingsManager(context.Background(), kubeclientset, testNamespace) + + for name, repo := range repositories { + t.Run(name, func(t *testing.T) { + repoServerClient := mocks.RepoServerServiceClient{} + repoServerClient.On("TestRepository", mock.Anything, mock.Anything).Return(&apiclient.TestRepositoryResponse{}, nil) + + repoServerClientset := mocks.Clientset{RepoServerServiceClient: &repoServerClient} + enforcer := newEnforcer(kubeclientset) + + db := &dbmocks.ArgoDB{} + db.On("DeleteRepository", context.TODO(), repo, "default").Return(nil) + db.On("ListRepositories", context.TODO()).Return([]*appsv1.Repository{{Repo: repo, Project: "default"}}, nil) + db.On("GetRepository", context.TODO(), repo, "default").Return(&appsv1.Repository{Repo: repo, Project: "default"}, nil) + appLister, projLister := newAppAndProjLister(defaultProj) + + s := NewServer(&repoServerClientset, db, enforcer, newFixtures().Cache, appLister, projLister, testNamespace, settingsMgr) + resp, err := s.DeleteRepository(context.TODO(), &repository.RepoQuery{Repo: repo, AppProject: "default"}) + require.NoError(t, err) + assert.Equal(t, repositorypkg.RepoResponse{}, *resp) + }) + } +} diff --git a/util/git/git.go b/util/git/git.go index 3a087aeb00096..0c81791cf6105 100644 --- a/util/git/git.go +++ b/util/git/git.go @@ -35,11 +35,21 @@ func IsTruncatedCommitSHA(sha string) bool { // SameURL returns whether or not the two repository URLs are equivalent in location func SameURL(leftRepo, rightRepo string) bool { - normalLeft := NormalizeGitURL(leftRepo) - normalRight := NormalizeGitURL(rightRepo) + normalLeft := NormalizeGitURLAllowInvalid(leftRepo) + normalRight := NormalizeGitURLAllowInvalid(rightRepo) return normalLeft != "" && normalRight != "" && normalLeft == normalRight } +// Similar to NormalizeGitURL, except returning an original url if the url is invalid. +// Needed to allow a deletion of repos with invalid urls. See https://github.com/argoproj/argo-cd/issues/20921. +func NormalizeGitURLAllowInvalid(repo string) string { + normalized := NormalizeGitURL(repo) + if normalized == "" { + return repo + } + return normalized +} + // NormalizeGitURL normalizes a git URL for purposes of comparison, as well as preventing redundant // local clones (by normalizing various forms of a URL to a consistent location). // Prefer using SameURL() over this function when possible. This algorithm may change over time