Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FullNameByRepoURL util function doesn't work for gitlab nested projects urls #13570

Open
riteshnanda09 opened this issue May 12, 2023 · 6 comments · May be fixed by #16234
Open

FullNameByRepoURL util function doesn't work for gitlab nested projects urls #13570

riteshnanda09 opened this issue May 12, 2023 · 6 comments · May be fixed by #16234
Labels
bug Something isn't working

Comments

@riteshnanda09
Copy link

riteshnanda09 commented May 12, 2023

argocd version : v2.7.0.

Describe the bug

When using FullNameByRepoURL function in notification template to get the repo full name this returns only first two nesting. Now in case of gitlab repo url where gitlab allows to have multiple nesting by projects in repos .

path: /projects/{{call .repo.FullNameByRepoURL .app.spec.source.repoURL}}/statuses/{{.app.status.operationState.operation.sync.revision}}

eg.
https://gitlab.example.com/project1/project2/project3/argocd-demo.git

This returns project1/project2 but expected is to get project1/project2/project3/argocd-demo

Looking at code here https://github.com/argoproj/argo-cd/blob/master/util/notification/expression/repo/repo.go#L73

func FullNameByRepoURL(rawURL string) string {
	parsed, err := giturls.Parse(rawURL)
	if err != nil {
		panic(err)
	}

	path := gitSuffix.ReplaceAllString(parsed.Path, "")
	if pathParts := text.SplitRemoveEmpty(path, "/"); len(pathParts) >= 2 {
		return strings.Join(pathParts[:2], "/")
	}

	return path
}

this will always return first 2 elements. Not sure on the history and motivation for this , but for me the fix would removing the condition checking for greater than.

@riteshnanda09 riteshnanda09 added the bug Something isn't working label May 12, 2023
@riteshnanda09
Copy link
Author

riteshnanda09 commented May 15, 2023

Also looks If trying to use notification webhook for gitlab commit api , gitlab requires the path to be sent as url encoded and there is no easy way to generate that with the use of notification functions or templating , has anyone successfully tried integration with gitlab commit status api ?

@aslafy-z
Copy link
Contributor

aslafy-z commented Nov 3, 2023

I opened #16234 to address this issue.

@riteshnanda09 If you're not using any special characters to your repository name, you can use call .strings.ReplaceAll .app.spec.source.repoURL "/" "%2F". If so, you can still replace additional characters the same way.

@aslafy-z
Copy link
Contributor

As @janschumann suggested, both text/template and html/template have default apis to urlencode :

urlquery is available for text/template: https://pkg.go.dev/text/template#hdr-Functions

urlescaper is available for html/template:
https://pkg.go.dev/html/template#hdr-Contexts

I'm not sure which one ArgoCD uses in this context.

@janschumann
Copy link
Contributor

As @janschumann suggested, both text/template and html/template have default apis to urlencode :

urlquery is available for text/template: https://pkg.go.dev/text/template#hdr-Functions

urlescaper is available for html/template: https://pkg.go.dev/html/template#hdr-Contexts

I'm not sure which one ArgoCD uses in this context.

Please consider #16343. Instead of adding the function to the repo package, it could also be added to the strings package. Would that make sense?

I recognised that there are no tests in the repo package. That is due to the argued.Service dependency, correct? Can't that be mocked for the tests?

@aslafy-z
Copy link
Contributor

@janschumann please check https://go.dev/play/p/eGKR5wfI-UN, these functions are already available as builtins in the template context. Can you give a try within argocd-notifications?

@nuno-silva
Copy link

confirming ArgoCD v2.10.4 is still affected by this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants