diff options
Diffstat (limited to 'tooling')
| -rw-r--r-- | tooling/audit_events/docs/templates/audit_event_types.md.erb | 38 | ||||
| -rw-r--r-- | tooling/danger/bulk_database_actions.rb | 34 | ||||
| -rw-r--r-- | tooling/danger/database.rb | 16 | ||||
| -rw-r--r-- | tooling/danger/model_validations.rb | 45 | ||||
| -rw-r--r-- | tooling/danger/project_helper.rb | 26 | ||||
| -rw-r--r-- | tooling/danger/required_stops.rb | 41 | ||||
| -rw-r--r-- | tooling/danger/specs/project_factory_suggestion.rb | 13 | ||||
| -rw-r--r-- | tooling/graphql/docs/templates/default.md.haml | 2 |
8 files changed, 183 insertions, 32 deletions
diff --git a/tooling/audit_events/docs/templates/audit_event_types.md.erb b/tooling/audit_events/docs/templates/audit_event_types.md.erb new file mode 100644 index 00000000000..9376b3c4fcd --- /dev/null +++ b/tooling/audit_events/docs/templates/audit_event_types.md.erb @@ -0,0 +1,38 @@ +<% all_audit_event_types = Gitlab::Audit::Type::Definition.definitions.map(&:last) %> +<% def boolean_to_docs(boolean) %> +<% if boolean %> +<% "**{check-circle}** Yes" %> +<% else %> +<% "**{dotted-circle}** No" %> +<% end %> +<% end %> +--- +stage: Govern +group: Compliance +info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments-to-development-guidelines" +--- + +<!--- + This documentation is auto generated by a Rake task. + + Please do not edit this file directly. To update this file, run: + bundle exec rake gitlab:audit_event_types:compile_docs +---> + +# Audit event types **(ULTIMATE)** + +Audit event types are used to [filter streamed audit events](index.md#update-event-filters). + +Every audit event is associated with an event type. The association with the event type can mean that an audit event is: + +- Saved to database. Audit events associated with these types are retrievable by using the audit events dashboard or the [audit events API](../../api/audit_events.md). +- Streamed. Audit events associated with these types are [streamed to external destinations](../index.md) if a + destination is set. +- Not streamed. Audit events associated with these types are not streamed to external destinations even if a destination is set. + +## Available audit event types + +| Name | Description | Saved to database | Streamed | Feature category | Introduced in | +|:-----|:------------|:------------------|:---------|:-----------------|:--------------| +<% all_audit_event_types.each do |event_type| %>| <%= "[`#{event_type.name}`](#{event_type.introduced_by_mr})" %> | <%= event_type.description %> | <%= boolean_to_docs(event_type.saved_to_database) %> | <%= boolean_to_docs(event_type.streamed) %> | <%= "`#{event_type.feature_category}`" %> | GitLab <%= "[#{event_type.milestone}](#{event_type.introduced_by_issue})" %> | +<% end %> diff --git a/tooling/danger/bulk_database_actions.rb b/tooling/danger/bulk_database_actions.rb new file mode 100644 index 00000000000..0f74e31cdde --- /dev/null +++ b/tooling/danger/bulk_database_actions.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require_relative 'suggestor' + +module Tooling + module Danger + module BulkDatabaseActions + include ::Tooling::Danger::Suggestor + + BULK_UPDATE_METHODS_REGEX = /\.((update|delete|destroy)(_all)?)\b/ + + DOCUMENTATION_LINK = 'https://docs.gitlab.com/ee/development/database_review.html#preparation-when-using-update-delete-update_all-and-destroy_all' + COMMENT_TEXT = + "When using `update`, `delete`, `update_all`, `delete_all` or `destroy_all` you must include the full " \ + "database query and query execution plan in the merge request description, and request a ~database review. " \ + "This comment can be ignored if the object is not an ActiveRecord class, since no database query " \ + "would be generated. For more information, see [Database Review documentation](#{DOCUMENTATION_LINK}).".freeze + + def add_comment_for_bulk_database_action_method_usage + changed_ruby_files.each do |filename| + add_suggestion( + filename: filename, + regex: BULK_UPDATE_METHODS_REGEX, + comment_text: COMMENT_TEXT + ) + end + end + + def changed_ruby_files + helper.added_files.select { |f| f.end_with?('.rb') && !f.start_with?('spec/', 'ee/spec/', 'jh/spec/') } + end + end + end +end diff --git a/tooling/danger/database.rb b/tooling/danger/database.rb index 4cfac7c4af4..71b3ed1a1a5 100644 --- a/tooling/danger/database.rb +++ b/tooling/danger/database.rb @@ -5,6 +5,8 @@ module Tooling module Database TIMESTAMP_MATCHER = /(?<timestamp>\d{14})/ MIGRATION_MATCHER = %r{\A(ee/)?db/(geo/)?(post_)?migrate/} + MODEL_PATHS = %r{\A(ee/)?app/models/} + MODEL_CHANGES = %r{^[^#\n]*?(?:scope :|where\(|joins\()} def find_migration_files_before(file_names, cutoff) migrations = file_names.select { |f| f.match?(MIGRATION_MATCHER) } @@ -15,6 +17,20 @@ module Tooling timestamp < cutoff end end + + def changes + changed_database_paths + changed_model_paths + end + + def changed_database_paths + helper.changes_by_category[:database] + end + + def changed_model_paths + helper.all_changed_files.grep(MODEL_PATHS).select do |file| + helper.changed_lines(file).any? { |change| change =~ MODEL_CHANGES } + end + end end end end diff --git a/tooling/danger/model_validations.rb b/tooling/danger/model_validations.rb new file mode 100644 index 00000000000..410c9dd434e --- /dev/null +++ b/tooling/danger/model_validations.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require_relative 'suggestor' + +module Tooling + module Danger + module ModelValidations + include ::Tooling::Danger::Suggestor + + MODEL_FILES_PATH = 'app/models' + MODEL_CONCERN_FILES_PATH = 'app/models/concerns' + EE_PREFIX = 'ee/' + MODEL_FILES_REGEX = %r{\A(#{EE_PREFIX})?#{MODEL_FILES_PATH}} + MODEL_CONCERN_FILES_REGEX = %r{\A(#{EE_PREFIX})?#{MODEL_CONCERN_FILES_PATH}} + VALIDATION_METHODS = %w[validates validate validates_each validates_with validates_associated].freeze + VALIDATIONS_REGEX = /^\+\s*(.*\.)?(#{VALIDATION_METHODS.join('|')})[( ]/ + + CODE_QUALITY_URL = "https://docs.gitlab.com/ee/development/code_review.html#quality" + SUGGEST_MR_COMMENT = <<~SUGGEST_COMMENT.freeze + Did you consider new validations can break existing records? + Please follow the [code quality guidelines about new model validations](#{CODE_QUALITY_URL}) when adding a new + model validation. + + If you're adding the validations to a model with no records you can ignore this warning. + SUGGEST_COMMENT + + def add_comment_for_added_validations + changed_model_files.each do |filename| + add_suggestion( + filename: filename, + regex: VALIDATIONS_REGEX, + comment_text: SUGGEST_MR_COMMENT + ) + end + end + + def changed_model_files + added_files = helper.added_files + modified_files = helper.modified_files + + modified_files.grep(MODEL_FILES_REGEX) + added_files.grep(MODEL_CONCERN_FILES_REGEX) + end + end + end +end diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb index f7c8f4f5133..633c7b57097 100644 --- a/tooling/danger/project_helper.rb +++ b/tooling/danger/project_helper.rb @@ -40,7 +40,7 @@ module Tooling %r{\Adata/removals/} => :none, %r{\A((ee|jh)/)?app/finders/(.+/)?integrations/} => [:import_integrate_be, :database, :backend], - [%r{\A((ee|jh)/)?db/(geo/)?(migrate|post_migrate)/}, %r{(:integrations|:\w+_tracker_data)\b}] => [:import_integrate_be, :database, :migration], + [%r{\A((ee|jh)/)?db/(geo/)?(migrate|post_migrate)/}, %r{(:integrations|:\w+_tracker_data)\b}] => [:import_integrate_be, :database], [%r{\A((ee|jh)/)?(app|lib)/.+\.rb}, %r{\b(Integrations::|\.execute_(integrations|hooks))\b}] => [:import_integrate_be, :backend], %r{\A( ((ee|jh)/)?app/((?!.*clusters)(?!.*alert_management)(?!.*views)(?!.*assets).+/)?integration.+ | @@ -98,7 +98,7 @@ module Tooling \.gitlab/ci/frontend\.gitlab-ci\.yml )\z}x => %i[frontend tooling], - %r{\A((ee|jh)/)?db/(geo/)?(migrate|post_migrate)/} => [:database, :migration], + %r{\A((ee|jh)/)?db/(geo/)?(migrate|post_migrate)/} => [:database], %r{\A((ee|jh)/)?db/(?!fixtures)[^/]+} => [:database], %r{\A((ee|jh)/)?lib/gitlab/(database|background_migration|sql)(/|\.rb)} => [:database, :backend], %r{\A(app/services/authorized_project_update/find_records_due_for_refresh_service)(/|\.rb)} => [:database, :backend], @@ -106,28 +106,6 @@ module Tooling %r{\A((ee|jh)/)?app/finders/} => [:database, :backend], %r{\Arubocop/cop/migration(/|\.rb)} => :database, - %r{\A(\.ruby-version\z|\.nvmrc\z|\.tool-versions\z)} => :tooling, - %r{\A(\.gitlab-ci\.yml\z|\.gitlab/ci)} => :tooling, - %r{\A\.codeclimate\.yml\z} => :tooling, - %r{\Alefthook.yml\z} => :tooling, - %r{\A\.editorconfig\z} => :tooling, - %r{Dangerfile\z} => :tooling, - %r{\A((ee|jh)/)?(danger/|tooling/danger/)} => :tooling, - %r{\Agems/gem\.gitlab-ci\.yml\z} => :tooling, - %r{\Agems/config/} => :tooling, - - %r{\A((ee|jh)/)?scripts/(lib/)?glfm/.*\.rb} => [:backend], - %r{\A((ee|jh)/)?scripts/(lib/)?glfm/.*\.js} => [:frontend], - %r{\A((ee|jh)/)?scripts/remote_development/.*} => [:remote_development_be], - %r{\A((ee|jh)/)?scripts/.*\.rb} => [:backend, :tooling], - %r{\A((ee|jh)/)?scripts/.*\.js} => [:frontend, :tooling], - %r{\A((ee|jh)/)?scripts/} => :tooling, - - %r{\Atooling/} => :tooling, - %r{(CODEOWNERS)} => :tooling, - %r{(tests.yml)} => :tooling, - %r{\A\.gitpod\.yml} => :tooling, - %r{\Alib/gitlab/ci/templates} => :ci_template, %r{\A((ee|jh)/)?spec/features/} => :test, diff --git a/tooling/danger/required_stops.rb b/tooling/danger/required_stops.rb new file mode 100644 index 00000000000..dcba23021ad --- /dev/null +++ b/tooling/danger/required_stops.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require_relative 'suggestor' + +module Tooling + module Danger + module RequiredStops + include ::Tooling::Danger::Suggestor + + MIGRATION_FILES_REGEX = %r{^db/(post_)?migrate} + + MIGRATION_FINALIZE_METHODS = %w[finalize_background_migration ensure_batched_background_migration_is_finished + finalize_batched_background_migration].freeze + MIGRATION_FINALIZE_REGEX = /^\+\s*(.*\.)?(#{MIGRATION_FINALIZE_METHODS.join('|')})[( ]/ + + DOC_URL = "https://docs.gitlab.com/ee/development/database/required_stops.html" + WARNING_COMMENT = <<~COMMENT.freeze + Finalizing data migration might be time consuming and require a [required stop](#{DOC_URL}). + Check the timings of the underlying data migration. + If possible postpone the finalization to the scheduled required stop. + If postponing is impossible please create new required stop as documentation suggests. + COMMENT + + def add_comment_for_finalized_migrations + migration_files.each do |filename| + add_suggestion( + filename: filename, + regex: MIGRATION_FINALIZE_REGEX, + comment_text: WARNING_COMMENT + ) + end + end + + private + + def migration_files + helper.all_changed_files.grep(MIGRATION_FILES_REGEX) + end + end + end +end diff --git a/tooling/danger/specs/project_factory_suggestion.rb b/tooling/danger/specs/project_factory_suggestion.rb index 78230db9585..8f9a05a0aba 100644 --- a/tooling/danger/specs/project_factory_suggestion.rb +++ b/tooling/danger/specs/project_factory_suggestion.rb @@ -28,17 +28,16 @@ module Tooling REPLACEMENT = '\k<head>let_it_be\k<tail>' SUGGESTION = <<~SUGGEST_COMMENT - Project creations are very slow. Using `let_it_be`, `build` or `build_stubbed` can improve test performance. + Project creations are very slow. To improve test performance, consider using `let_it_be`, `build`, or `build_stubbed` instead. - Warning: `let_it_be` may not be suitable if your test modifies data as this could result in state leaks! + ⚠️ **Warning**: If your test modifies data, `let_it_be` may be unsuitable, and cause state leaks! Use `let_it_be_with_reload` or `let_it_be_with_refind` instead. - In those cases, please use `let_it_be_with_reload` or `let_it_be_with_refind` instead. - - If your are unsure which is the right method to use, - please refer to [testing best practices](https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#optimize-factory-usage) + Unsure which method to use? See the [testing best practices](https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#optimize-factory-usage) for background information and alternative options for optimizing factory usage. - Feel free to ignore this comment if you know `let` or `let!` are the better options and/or worry about causing state leaks. + If you're concerned about causing state leaks, or if you know `let` or `let!` are the better options, ignore this comment. + + ([Improve this message?](https://gitlab.com/gitlab-org/gitlab/-/blob/master/tooling/danger/specs/project_factory_suggestion.rb)) SUGGEST_COMMENT end end diff --git a/tooling/graphql/docs/templates/default.md.haml b/tooling/graphql/docs/templates/default.md.haml index fad954ebb01..57c3ca7a476 100644 --- a/tooling/graphql/docs/templates/default.md.haml +++ b/tooling/graphql/docs/templates/default.md.haml @@ -3,7 +3,7 @@ = auto_generated_comment :plain - # GraphQL API Resources + # GraphQL API resources This documentation is self-generated based on GitLab current GraphQL schema. |