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

[JENKINS-19934] fix project naming strategy #179

Merged
merged 15 commits into from
Jul 28, 2022

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented May 25, 2022

The project naming strategy is currently not considering if the user
has the permission to create a job with a given name at all. It just
checks if the passed name matches any of the patterns.
Also it doesn't consider the full name, e.g. when a project is created
inside a folder.
This can lead to the situation that a user creates a job but is then
unable to configure the job.

This change will do the following:
Prerequisites:

  • Rolebased project naming strategy is enabled.
  • for each role when create is enabled also configure and read should
    be enabled

If a user has either globally or in any item role the create permission
then he will see the "New item" link in the side panel.
A user that has global create permissions can create jobs with any name.
For a user that has create permission on one or more item roles the
entered name is matched against the role and the users permissions.

Current limitation is that it only works reliably via the UI. When
trying to create a job via the CLI, there is no staplerrequest that can
be used to find the parent item. So job creation via cli will fall back
to the old behaviour and just check the item name itself.
In case JENKINS-68602 (see core PR 6598) gets resolved requests coming in via the CLI would
also be properly checked.

This also adds an admin monitor that warns when the role based project
naming strategy is not enabled.

Other PR (#9, #16) also tried to solve the problem but mainly lack the fact that without global create permissions you do not get the corresponding link.

Considering performance I think that the looping over the authorities is not a big problem as the method is not called so frequently. Not sure if there will be a noticeable delay when typing a new job name and many item roles are configured.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@mawinter69 mawinter69 changed the title [JENKINS-19934] WIP fix project naming strategy [JENKINS-19934] fix project naming strategy Jun 3, 2022
@oleg-nenashev oleg-nenashev self-requested a review June 20, 2022 20:33
The project naming strategy is currently not considering if the user
has the permission to create a job with a given name at all. I just
checks if the passed name matches any of the patterns.
Also it doesn't consider the full name, e.g. when a project is created
inside a folder.
This can lead to the situtation that a user creates a job but is then
unable to configure the job.

This change will do the following:
Prerequisites:
 - Rolebased project naming strategy is enabled.
 - for each role when create is enabled also configure and read should
   be enabled

If a user has either globally or in any item role the create permission
then he will see the "New item" link in the side panel.
A user that has global create permissions can create jobs with any name.
For a user that has create permission on one or more item roles the
entered name is matched against the role and the users permissions.

Current limitation is that it only works reliably via the UI. When
trying to create a job via the CLI, there is no staplerrequest that can
be used to find the parent item. So job creation via cli will fall back
to the old behaviour and just check the item name itself.
In case JENKINS-68602 gets resolved requests coming in via the CLI would
also be properly checked.

This also adds an admin monitor that warns when the role based project
naming strategy is not enabled.
can't use the code path from authorities as roles that is wrong.
Add tests that verify group permissions work properly
@mawinter69 mawinter69 force-pushed the fix-namingstrategy branch from 870e59b to f61f7b8 Compare July 4, 2022 15:11
@mawinter69 mawinter69 force-pushed the fix-namingstrategy branch from a8d1180 to 6f1f1e8 Compare July 4, 2022 16:47
@mawinter69 mawinter69 force-pushed the fix-namingstrategy branch from 6f1f1e8 to acc0357 Compare July 4, 2022 17:05
@mawinter69 mawinter69 requested a review from a team as a code owner July 27, 2022 13:12
@mawinter69 mawinter69 deleted the fix-namingstrategy branch August 21, 2022 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant