fix: Allow to delete repos with invalid urls (#20921) #20975
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
andrii-korotkov-verkada
force-pushed
the
20921-fix-unable-to-delete-repo
branch
from
a4ca51c to
1d7b5e3
Compare
Fixes argoproj#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 Signed-off-by: Andrii Korotkov <[email protected]>
andrii-korotkov-verkada
force-pushed
the
20921-fix-unable-to-delete-repo
branch
from
1d7b5e3 to
6ccd0fc
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #20975 +/- ##
=========================================
Coverage ? 55.02%
=========================================
Files ? 324
Lines ? 55472
Branches ? 0
=========================================
Hits ? 30522
Misses ? 22329
Partials ? 2621 ☔ View full report in Codecov by Sentry. |
reggie-k
approved these changes
Dec 2, 2024
pasha-codefresh
requested changes
Dec 2, 2024
util/git/git.go
Outdated
| // Similar to NormalizeGitURL, except returning a not fully normalized string when 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 { | ||
| return normalizeGitURL(repo, true) |
There was a problem hiding this comment.
I think it will be better not to add allowInvalid parameter to private function. Can we just return original "repo" in case if error happens ? In such case you will not need to modify with additional conditions normalizeGitURL function
There was a problem hiding this comment.
I've made the change.
Signed-off-by: Andrii Korotkov <[email protected]>
pasha-codefresh
approved these changes
Dec 2, 2024
|
@pasha-codefresh, can we cherry pick this into 2.11-2.13, please? |
adriananeci
pushed a commit
to adriananeci/argo-cd
that referenced
this pull request
Dec 4, 2024
…oj#20975) * fix: Allow to delete repos with invalid urls (argoproj#20921) Fixes argoproj#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 Signed-off-by: Andrii Korotkov <[email protected]> * Don't modify the original NormalizeGitURL Signed-off-by: Andrii Korotkov <[email protected]> --------- Signed-off-by: Andrii Korotkov <[email protected]> Signed-off-by: Adrian Aneci <[email protected]>
|
/cherry-pick release-2.13 |
|
/cherry-pick release-2.12 |
gcp-cherry-pick-bot bot
pushed a commit
that referenced
this pull request
Dec 10, 2024
* 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 Signed-off-by: Andrii Korotkov <[email protected]> * Don't modify the original NormalizeGitURL Signed-off-by: Andrii Korotkov <[email protected]> --------- Signed-off-by: Andrii Korotkov <[email protected]>
|
/cherry-pick release-2.11 |
gcp-cherry-pick-bot bot
pushed a commit
that referenced
this pull request
Dec 10, 2024
* 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 Signed-off-by: Andrii Korotkov <[email protected]> * Don't modify the original NormalizeGitURL Signed-off-by: Andrii Korotkov <[email protected]> --------- Signed-off-by: Andrii Korotkov <[email protected]>
|
Cherry-pick failed with |
|
2.11 has some conflicts, need to do it manually @andrii-korotkov-verkada |
pasha-codefresh
pushed a commit
that referenced
this pull request
Dec 10, 2024
* 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 <[email protected]> Co-authored-by: Andrii Korotkov <[email protected]>
pasha-codefresh
pushed a commit
that referenced
this pull request
Dec 10, 2024
* 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 <[email protected]> Co-authored-by: Andrii Korotkov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Checklist: