Set a limit of 255 characters for security policy names

changed milestone to %15.2

Suggested Reviewers (beta)

The individuals below may be good candidates to participate in the review based on various factors.

You can use slash commands in comments to quickly assign /assign_reviewer @user1.

Suggested Reviewers
`@mparuszewski`, `@kushalpandya`, `@mkaeppler`, `@bwill`, `@toupeira`

If you do not believe these suggestions are useful, please apply the label Bad Suggested Reviewer. You can also provide feedback for this feature on this issue: https://gitlab.com/gitlab-org/gitlab/-/issues/357923.

Automatically generated by Suggested Reviewers Bot - an experimental ML-based recommendation engine created by ~"group::applied ml".

added Category:Security Policy Management devopsgovern featureenhancement groupsecurity policies sectionsec typefeature labels

	1 Message
	This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge.

Documentation review

The following files require a review from a technical writer:

doc/user/application_security/policies/scan-execution-policies.md
doc/user/application_security/policies/scan-result-policies.md

The review does not need to block merging this merge request. See the:

Metadata for the *.md files that you've changed. The first few lines of each *.md file identify the stage and group most closely associated with your docs change.
The Technical Writer assigned for that stage and group.
Documentation workflows for information on when to assign a merge request for review.

Reviewer roulette

Changes that require review have been detected!

Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

Category	Reviewer	Maintainer
backend	Valery Sizov (`@vsizov`) (UTC+1)	Sincheol (David) Kim (`@dskim_gitlab`) (UTC+9.5)
UX	Michael Fangman (`@mfangman`) (UTC-5)	Amelia Bauerly (`@ameliabauerly`) (UTC-7)

To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

If needed, you can retry the danger-review job that generated this comment.

Generated by Danger

added Enterprise Edition GitLab Ultimate workflowin dev labels

added 1 commit

a4b02844 - Move truncation for approval rules to the frontend

Compare with previous version

requested review from @zmartins

added 1 commit

bdcf87bc - Move truncation for approval rules to the frontend

Compare with previous version

removed review request for @zmartins

assigned to @zmartins

added Technical Writing documentation labels

added 1 commit

7c25d75e - Move approval rule truncation to the frontend

Compare with previous version

added 1 commit

a10ce9ed - Move approval rule truncation to the frontend

Compare with previous version

mentioned in merge request !88679 (merged)

changed the description

Bundle size analysis [beta]

This compares changes in bundle size for entry points between the commits 23a9c9b4 and 26ad37e4

Special assets

Entrypoint / Name	Size before	Size after	Diff	Diff in percent
average	3.54 MB	3.54 MB	-	0.0 %
mainChunk	1.98 MB	1.98 MB	-	0.0 %

Note: We do not have exact data for 23a9c9b4. So we have used data from: 2e888a8a.
The target commit was too new, so we used the latest commit from master we have info on.
It might help to rerun the bundle-size-review job
This might mean that you have a few false positives in this report. If something unrelated to your code changes is reported, you can check this comparison in order to see if they caused this change.

Please look at the full report for more details

Allure report

allure-report-publisher generated test report!

review-qa-blocking: test report for ac3cd05e

expand test summary

+---------------------------------------------------------------------------+
|                              suites summary                               |
+----------------------+--------+--------+---------+-------+-------+--------+
|                      | passed | failed | skipped | flaky | total | result |
+----------------------+--------+--------+---------+-------+-------+--------+
| Plan                 | 0      | 48     | 1       | 48    | 49    | ❌     |
| Create               | 0      | 23     | 2       | 23    | 25    | ❌     |
| Manage               | 0      | 39     | 0       | 38    | 39    | ❌     |
| Verify               | 0      | 12     | 1       | 12    | 13    | ❌     |
| Secure               | 0      | 2      | 0       | 2     | 2     | ❌     |
| Package              | 0      | 0      | 1       | 0     | 1     | ➖     |
| Version sanity check | 0      | 0      | 1       | 0     | 1     | ➖     |
| Configure            | 0      | 0      | 1       | 0     | 1     | ➖     |
| Protect              | 0      | 2      | 0       | 2     | 2     | ❌     |
+----------------------+--------+--------+---------+-------+-------+--------+
| Total                | 0      | 126    | 7       | 125   | 133   | ❌     |
+----------------------+--------+--------+---------+-------+-------+--------+

added 1 commit

26ad37e4 - Add the string interpolation basing truncation

Compare with previous version

changed the description

@sam.white Thanks a lot for getting this started. I have updated the interpolation, added the changes in the schema and some of the docs.

I will add specs tomorrow and get the review process going.

@annabeldunstone @pedroms @phikai upon further investigation related to this conversation, it appears that we were only truncating approval rule names for Security Policy Rules. All the other approval types were effectively unbounded (the max limit is huge and supports up to 10,485,760 characters).

What are your thoughts about the proposal in this MR to truncate he approval rule name in the UI at 50 characters? The full name would still be viewable on hover as a tooltip. If you disagree, we can easily adjust the character limit where we start truncating or even remove the truncation entirely and leave the UI as it is currently.

added 1 commit

d7a2163b - Remove frontend truncation

Compare with previous version

changed title from Move truncation for approval rules to the frontend to Set a limit of 255 characters for approval rule names

changed the description

removed frontend label

added 1 commit

ac3cd05e - Add validation for all approval rules

Compare with previous version

resolved all threads

added 111 commits

ac3cd05e...78e805d2 - 107 commits from branch master
5844b397 - Move approval rule truncation to the frontend
b93b046e - Add the string interpolation basing truncation
0dd16228 - Remove frontend truncation
823727b7 - Add validation for all approval rules

Compare with previous version

added 1 commit

26eb739c - Fix rubocop and try to add tests

Compare with previous version

added 1 commit

904981b3 - Fix rubocop and try to add tests

Compare with previous version

changed the description

@sam.white I have tested how this change would impact existing policies and new ones. Those can be found as part of the screenshoot section in the description. TLDR; existing policies with name longer than 256 would make policies invalid therefore not being displayed in policy list and merge request approval section.

It brought one point up -- invalid policies can only be fixed by directly changing the security management project repository. Should we present a message to the user to inform that one or more policies might be invalid ?

@sam.white As a follow-up we might have to add a message informing users of other report approver rules of the limit:

WDYT?

added 1 commit

00b297e7 - Moves max length validation to shared concern

Compare with previous version

I have pushed this commit in order to keep both project and merge request approval rules aligned with this new validation. Feel free to revert the commit if you see fit.

added 1 commit

9f462d4f - Moves max length validation to shared concern

Compare with previous version

added 1 commit

217af50f - Moves max length validation to shared concern

Compare with previous version

@acook.gitlab @mfangman and @claytoncornell would you please review this MR?

requested review from @mfangman, @claytoncornell, and @acook.gitlab

removed workflowin dev label

added workflowin review label

added 1 commit

b2e2305e - Clarify max characters

Compare with previous version

added 1 commit

e8b1ff96 - Clarify max characters

Compare with previous version

approved this merge request

@mfangman, thanks for approving this merge request.

This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.

For more info, please refer to the following links:

approved this merge request

mentioned in issue gitlab-com/www-gitlab-com#13239 (closed)

added 1 commit

1ee172b4 - Remove length limit for all approval rules

Compare with previous version

changed title from Set a limit of 255 characters for approval rule names to Set a limit of 255 characters for security policy names

changed the description

mentioned in issue #366669 (closed)

requested review from @dskim_gitlab and removed review request for @acook.gitlab

approved this merge request

added 1 commit

bee8cb05 - Increase limit to 255

Compare with previous version

resolved all threads

approved this merge request

enabled an automatic merge when the pipeline for 5d001b07 succeeds

merged

mentioned in commit 2e69038c

added workflowstaging-canary label and removed workflowin review label

added workflowcanary label and removed workflowstaging-canary label

added workflowstaging label and removed workflowcanary label

added workflowproduction label and removed workflowstaging label

added workflowpost-deploy-db-staging label and removed workflowproduction label

added workflowpost-deploy-db-production label and removed workflowpost-deploy-db-staging label

@sam.white is this change live on gitlab.com now? I think from the production workflow label that means yes but I don't fully understand the promotion process. If it is live is there anything we need to do so it takes effect on our MRs? I see in the diffs that a background service was modified, do we need to reapply policies or anything like that?

@dominic.bishop

@sam.white is this change live on gitlab.com now? I think from the production workflow label that means yes but I don't fully understand the promotion process.

Yes it is live in gitlab.com !

If it is live is there anything we need to do so it takes effect on our MRs? I see in the diffs that a background service was modified, do we need to reapply policies or anything like that?

As this change involves truncation on a DB level, it will be applicable to scan result policies that have been either created or updated after this MR has made into production.

Note that although this MR addressed all approval rules initially, it has been updated to only affect scan result policies related rules.

If it is live is there anything we need to do so it takes effect on our MRs? I see in the diffs that a background service was modified, do we need to reapply policies or anything like that?

@dominic.bishop yes, you will need to reapply your policy. This can be done by creating any change to the policy, such as first disabling and then re-enabling the policy - or even just by changing one letter of the title or description.

thanks @sam.white

@dominic.bishop yes, you will need to reapply your policy. This can be done by creating any change to the policy, such as first disabling and then re-enabling the policy - or even just by changing one letter of the title or description.

I have made a change to our policy YAML and I can confirm that on MRs created after pushing the change the full titles are now shown for our scan result policy rules.

Thanks @sam.white and @zmartins for your help in getting this one resolved.

added releasedcandidate label

added releasedpublished label and removed releasedcandidate label

added docsfeature label

Set a limit of 255 characters for security policy names

What does this MR do and why?

Screenshots or screen recordings

Previous behavior

Current behaviour

Existing policies (with name length longer than the new limit)

New policies

How to set up and validate locally

MR acceptance checklist

Activity

Suggested Reviewers (beta)

Documentation review

Reviewer roulette

Bundle size analysis [beta]

Special assets

Allure report

Set a limit of 255 characters for security policy names

What does this MR do and why?

Screenshots or screen recordings

Previous behavior

Current behaviour

Existing policies (with name length longer than the new limit)

New policies

How to set up and validate locally

MR acceptance checklist

Merge request reports

Activity

Suggested Reviewers (beta)

Documentation review

Reviewer roulette

Bundle size analysis [beta]

Special assets

Allure report