Skip to content

Warn about unknown abilities per policy in specs

Peter Leitzen requested to merge pl-find-undefined-policy-abilities into master

What does this MR do and why?

This MRs introduces spec/support/ability_check_todo.yml to track unused policy abilities.

This YAML file acts as a TODO list and each TODO entry means that an ability wasn't found in the particular policy class or its delegations.

An entry is considered as "fixed" if code which uses the ability was adjusted. Alternatively, the ability could be moved into the Permanent excludes section of this YAML file.

See #369511 and #393451

Example

When a unknown ability of a policy is called the following deprecation warning is printed:

bin/rspec ./spec/helpers/invite_members_helper_spec.rb:51

InviteMembersHelper
  # order random
  #common_invite_group_modal_data
    when sharing with groups outside the hierarchy is enabled
Missing metadata feature_category: ./spec/helpers/invite_members_helper_spec.rb:49 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
DEPRECATION WARNING: Ability :manage_owners in GroupPolicy not found.
user=nil, subject=#<Group:0x000055adc8487cb0>, opts={}"

To exclude this check add this entry to /home/peter/devel/gitlab/gdk/gitlab/spec/support/ability_check_todo.yml:
GroupPolicy:
- manage_owners
 (called from allowed? at /home/peter/devel/gitlab/gdk/gitlab/app/models/ability.rb:80)
      does not return filter attributes

To add this policy and ability to spec/support/ability_check_todo.yml.

Statistics

Take 3

Changes:

  • Exclude remaining lookup failures.
  • Some work has been extracted into separate MRs

Take 2

Changes:

   2368 Ability :override_project_member in ProjectMemberPolicy not found
    738 Ability :maintainer_access in Namespaces::UserNamespacePolicy not found
    676 Ability :admin_merge_request in GroupPolicy not found
    364 Ability :update_max_pages_size in GlobalPolicy not found
    220 Ability :read_group_saml_identity in ProjectPolicy not found
    112 Ability :admin_project in IntegrationPolicy not found
    107 Ability :read_project in PersonalSnippetPolicy not found
     68 Ability :read_project in TodoPolicy not found
     64 Ability :read_award_emoji in AwardEmojiPolicy not found
     64 Ability :admin_vulnerability in ProjectPolicy not found
     48 Ability :read_job_artifacts in Ci::BridgePolicy not found
     36 Ability :read_emoji in EpicPolicy not found
     32 Ability :read_crm_contact in Namespaces::UserNamespacePolicy not found
     32 Ability :admin_group_member in Namespaces::UserNamespacePolicy not found
     24 Ability :admin_vulnerability_issue_link in Vulnerabilities::IssueLinkPolicy not found
     16 Ability :read_vulnerability_scanner in Vulnerabilities::ScannerPolicy not found
     16 Ability :manage_owners in GroupPolicy not found
     16 Ability :create_timelog in EpicPolicy not found
     12 Ability :admin_terraform_state in UserPolicy not found
      8 Ability :read_on_demand_dast_scan in GlobalPolicy not found
      8 Ability :read_cluster_environments in Clusters::ClusterPolicy not found
      8 Ability :create_test_case in ProjectPolicy not found
      8 Ability :create_test_case in IssuePolicy not found
      8 Ability :create_requirement in ProjectPolicy not found

Take 1

No code changes.

Click to expand 
$ sort .local/ci_logs.abilities2.txt  | uniq -c | sort -gr
  12793 Ability :set_confidentiality in MergeRequestPolicy not found
   7381 Ability :admin_feature_flags_issue_links in ProjectPolicy not found
   2284 Ability :override_project_member in ProjectMemberPolicy not found
   1565 Ability :set_issue_crm_contacts in EpicPolicy not found
    738 Ability :maintainer_access in Namespaces::UserNamespacePolicy not found
    676 Ability :admin_merge_request in GroupPolicy not found
    364 Ability :update_max_pages_size in GlobalPolicy not found
    247 Ability :read_group_saml_identity in ProjectPolicy not found
    112 Ability :read_project in PersonalSnippetPolicy not found
    112 Ability :admin_project in IntegrationPolicy not found
     64 Ability :read_award_emoji in AwardEmojiPolicy not found
     64 Ability :admin_vulnerability in ProjectPolicy not found
     62 Ability :read_project in TodoPolicy not found
     48 Ability :read_job_artifacts in Ci::BridgePolicy not found
     32 Ability :read_emoji in EpicPolicy not found
     32 Ability :read_crm_contact in Namespaces::UserNamespacePolicy not found
     32 Ability :admin_group_member in Namespaces::UserNamespacePolicy not found
     24 Ability :admin_vulnerability_issue_link in Vulnerabilities::IssueLinkPolicy not found
     16 Ability :manage_owners in GroupPolicy not found
     16 Ability :create_timelog in EpicPolicy not found
     12 Ability :admin_terraform_state in UserPolicy not found
      8 Ability :read_vulnerability_scanner in Vulnerabilities::ScannerPolicy not found
      8 Ability :read_on_demand_dast_scan in GlobalPolicy not found
      8 Ability :read_cluster_environments in Clusters::ClusterPolicy not found
      8 Ability :create_test_case in ProjectPolicy not found
      8 Ability :create_test_case in IssuePolicy not found
      8 Ability :create_requirement in ProjectPolicy not found

Example failure

🔴 https://gitlab.com/gitlab-org/gitlab/-/jobs/2811100042#L296

Failures:
  1) Analytics (JavaScript fixtures) Projects::Analytics::CycleAnalytics::StagesController projects/analytics/value_stream_analytics/stages.json
     Failure/Error: raise message
     RuntimeError:
       Ability :set_confidentiality in MergeRequestPolicy not found.
       user=#<User id:2 @user2>, subject=#<MergeRequest:0x00007fe88e8742f8>, opts={}"
     # ./app/models/ability.rb:112:in `ensure_ability_exists!'
     # ./app/models/ability.rb:80:in `allowed?'
     # ./lib/gitlab/allowable.rb:6:in `can?'
     # ./app/services/issuable_base_service.rb:58:in `filter_params'
     # ./ee/app/services/ee/issuable_base_service.rb:37:in `filter_params'
     # ./app/services/merge_requests/base_service.rb:139:in `filter_params'
     # ./ee/app/services/ee/merge_requests/base_service.rb:30:in `filter_params'
     # ./app/services/issuable_base_service.rb:214:in `create'
     # ./app/services/merge_requests/base_service.rb:112:in `create'
     # ./app/services/merge_requests/create_service.rb:13:in `execute'
     # ./spec/support/helpers/cycle_analytics_helpers.rb:181:in `create_merge_request_closing_issue'
     # ./spec/support/shared_contexts/fixtures/analytics_shared_context.rb:35:in `prepare_cycle_analytics_data'
     # ./spec/support/shared_contexts/fixtures/analytics_shared_context.rb:69:in `block (2 levels) in <top (required)>'
     # ./spec/support/helpers/javascript_fixtures_helpers.rb:24:in `block (3 levels) in <module:JavaScriptFixturesHelpers>'
     # ./spec/support/helpers/javascript_fixtures_helpers.rb:24:in `block (2 levels) in <module:JavaScriptFixturesHelpers>'
     # ./spec/spec_helper.rb:426:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:417:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:413:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:52:in `with_raw_context'
     # ./spec/spec_helper.rb:413:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:264:in `block (2 levels) in <top (required)>'
     # ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <top (required)>'
     # ./spec/support/sidekiq.rb:21:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq.rb:8:in `gitlab_sidekiq_inline'
     # ./spec/support/sidekiq.rb:21:in `block (2 levels) in <top (required)>'
     # ./spec/support/flaky_tests.rb:27:in `block (2 levels) in <top (required)>'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <top (required)>'
     # ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <top (required)>'
Edited by Peter Leitzen

Merge request reports