Skip to content

Issue #280 - It is impossible to restrict application access by repository URL#281

Merged
alexmt merged 2 commits intoargoproj:masterfrom
alexmt:app-rbac
Jun 12, 2018
Merged

Issue #280 - It is impossible to restrict application access by repository URL#281
alexmt merged 2 commits intoargoproj:masterfrom
alexmt:app-rbac

Conversation

@alexmt
Copy link
Collaborator

@alexmt alexmt commented Jun 12, 2018

No description provided.

@alexmt alexmt requested a review from merenbach June 12, 2018 15:40
@alexmt alexmt changed the title Issue #280 - It is impossible to restrict application access by repos… Issue #280 - It is impossible to restrict application access by repository URL Jun 12, 2018
Copy link
Contributor

@merenbach merenbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor thought. Good work!


// appRBACName formats fully qualified application name for RBAC check
func appRBACName(app appv1.Application) string {
return fmt.Sprintf("%s/%s", git.NormalizeGitURL(app.Spec.Source.RepoURL), app.Name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if return path.Join(git.NormalizeGitURL(app.Spec.Source.RepoURL), app.Name) might be just slightly more semantically appropriate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. thank you for suggestion!

@alexmt alexmt merged commit 9fa622d into argoproj:master Jun 12, 2018
@alexmt alexmt deleted the app-rbac branch June 12, 2018 17:43
leoluz pushed a commit to leoluz/argo-cd that referenced this pull request Sep 29, 2023
…argoproj#281)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants