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

Strip whitespace from project role group names #10919

Closed
crenshaw-dev opened this issue Oct 12, 2022 · 6 comments · Fixed by #10988
Closed

Strip whitespace from project role group names #10919

crenshaw-dev opened this issue Oct 12, 2022 · 6 comments · Fixed by #10988
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest type:usability Enhancement of an existing feature

Comments

@crenshaw-dev
Copy link
Collaborator

Summary

I think the API server should strip the leading/trailing whitespace from group names in project roles.

Motivation

Intuit had a project role where a group name had a trailing whitespace. I believe this was the result of an accidental copy/paste to the UI.

  roles:
  - description: admin
    groups:
    - 'group:Admins_99999 '
    name: admin
    policies:
    - p, proj:some-proj:admin, applications, *, some-proj/*, allow

Proposal

The API server should strip leading/trailing whitespace from group names before adding them.

Note: I'm assuming that leading/trailing whitespace is always invalid around group names. I think that's true, but please disagree if I'm wrong!

@crenshaw-dev crenshaw-dev added enhancement New feature or request good first issue Good for newcomers type:usability Enhancement of an existing feature hacktoberfest labels Oct 12, 2022
@utopian-monkey
Copy link

i wanna work on this

@crenshaw-dev
Copy link
Collaborator Author

@utopian-monkey go for it! Let me know if you need any help!

@The-Debarghya
Copy link

Where are the changes to be implemented?

@crenshaw-dev
Copy link
Collaborator Author

@The-Debarghya I think either in server/project/project.go's Update function (stripping whitespace on project update) or on the ProjectRole struct as a .GetTrimmedGroups() function. The latter approach is a bit more involved, but more thorough.

@rishu1601
Copy link

Hey @crenshaw-dev , let me know if this is up for grab, I don't see this issue as assigned

crenshaw-dev pushed a commit that referenced this issue Nov 14, 2022
…names (#10919) (#10988)

* fix: add check for trailing/leading whitespace in project role group names

Signed-off-by: Ferenc <ferenc.horvay@web.de>

* fix: change expected output on whitespace test

Signed-off-by: Ferenc <ferenc.horvay@web.de>

* fix: apply requested changes

Signed-off-by: Ferenc <ferenc.horvay@web.de>

Signed-off-by: Ferenc <ferenc.horvay@web.de>
ashutosh16 pushed a commit to ashutosh16/argo-cd that referenced this issue Nov 23, 2022
…names (argoproj#10919) (argoproj#10988)

* fix: add check for trailing/leading whitespace in project role group names

Signed-off-by: Ferenc <ferenc.horvay@web.de>

* fix: change expected output on whitespace test

Signed-off-by: Ferenc <ferenc.horvay@web.de>

* fix: apply requested changes

Signed-off-by: Ferenc <ferenc.horvay@web.de>

Signed-off-by: Ferenc <ferenc.horvay@web.de>
emirot pushed a commit to emirot/argo-cd that referenced this issue Jan 27, 2023
…names (argoproj#10919) (argoproj#10988)

* fix: add check for trailing/leading whitespace in project role group names

Signed-off-by: Ferenc <ferenc.horvay@web.de>

* fix: change expected output on whitespace test

Signed-off-by: Ferenc <ferenc.horvay@web.de>

* fix: apply requested changes

Signed-off-by: Ferenc <ferenc.horvay@web.de>

Signed-off-by: Ferenc <ferenc.horvay@web.de>
Signed-off-by: emirot <emirot.nolan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest type:usability Enhancement of an existing feature
Projects
None yet
5 participants
@crenshaw-dev @rishu1601 @utopian-monkey @The-Debarghya and others