From 5bb54b8711a6fd0993ab014f6749cbb74c7b071b Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 28 Jun 2022 15:09:17 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .gitlab/ci/rules.gitlab-ci.yml | 27 ++- Gemfile | 4 +- Gemfile.lock | 8 +- app/assets/javascripts/new_branch_form.js | 22 +- .../pages/projects/branches/new/index.js | 3 +- app/controllers/import/fogbugz_controller.rb | 8 +- app/models/ci/build_report_result.rb | 4 - app/views/import/fogbugz/new.html.haml | 2 +- .../import/fogbugz/new_user_map.html.haml | 2 +- app/views/import/fogbugz/status.html.haml | 2 +- .../projects/_import_project_pane.html.haml | 2 +- app/views/shared/milestones/_header.html.haml | 18 +- app/views/shared/web_hooks/_form.html.haml | 2 +- .../development/gitlab_sli_new_counters.yml | 8 + db/docs/post_migration_test_table.yml | 8 + ...20627223041_add_post_migrate_test_table.rb | 13 ++ db/schema_migrations/20220627223041 | 1 + db/structure.sql | 19 ++ doc/api/access_requests.md | 2 +- doc/api/invitations.md | 2 +- doc/api/members.md | 2 +- doc/development/pipelines.md | 1 + doc/user/group/index.md | 4 +- lib/generators/model/model_generator.rb | 25 ++ lib/gitlab/database/gitlab_schemas.yml | 1 + lib/gitlab/metrics/sli.rb | 44 +++- qa/qa/resource/personal_access_token.rb | 2 +- qa/qa/resource/personal_access_token_cache.rb | 1 + .../1_manage/login/register_spec.rb | 5 +- .../import/fogbugz_controller_spec.rb | 41 +++- .../projects/logs_controller_spec.rb | 214 ------------------ .../projects/environments_pod_logs_spec.rb | 45 ---- spec/frontend/new_branch_spec.js | 23 +- .../generators/model/mocks/migration_file.txt | 26 +++ .../lib/generators/model/mocks/model_file.txt | 2 + spec/lib/generators/model/mocks/spec_file.txt | 5 + .../generators/model/model_generator_spec.rb | 47 ++++ spec/lib/gitlab/metrics/sli_spec.rb | 84 ++++++- spec/models/ci/build_report_result_spec.rb | 6 - 39 files changed, 375 insertions(+), 360 deletions(-) create mode 100644 config/feature_flags/development/gitlab_sli_new_counters.yml create mode 100644 db/docs/post_migration_test_table.yml create mode 100644 db/post_migrate/20220627223041_add_post_migrate_test_table.rb create mode 100644 db/schema_migrations/20220627223041 create mode 100644 lib/generators/model/model_generator.rb delete mode 100644 spec/controllers/projects/logs_controller_spec.rb delete mode 100644 spec/features/projects/environments_pod_logs_spec.rb create mode 100644 spec/lib/generators/model/mocks/migration_file.txt create mode 100644 spec/lib/generators/model/mocks/model_file.txt create mode 100644 spec/lib/generators/model/mocks/spec_file.txt create mode 100644 spec/lib/generators/model/model_generator_spec.rb diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 4aacf611123..6b2cc01bfb8 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -328,7 +328,6 @@ - "danger/**/*" - "{,ee/,jh/}fixtures/**/*" - "{,ee/,jh/}rubocop/**/*" - - ".rubocop_todo/**/*.yml" - "{,ee/,jh/}spec/**/*" - "{,spec/}tooling/**/*" @@ -346,8 +345,7 @@ - "Dockerfile.assets" - "vendor/assets/**/*" - ".{eslintignore,gitattributes,nvmrc,prettierrc,stylelintrc,yamllint}" - - ".{codeclimate,eslintrc,gitlab-ci,haml-lint,haml-lint_todo,rubocop,rubocop_todo}.yml" - - ".rubocop_todo/**/*.yml" + - ".gitlab-ci.yml" - "*_VERSION" - "{,jh/}Gemfile{,.lock}" - "Rakefile" @@ -371,8 +369,7 @@ - "Dockerfile.assets" - "vendor/assets/**/*" - ".{eslintignore,gitattributes,nvmrc,prettierrc,stylelintrc,yamllint}" - - ".{codeclimate,eslintrc,gitlab-ci,haml-lint,haml-lint_todo,rubocop,rubocop_todo}.yml" - - ".rubocop_todo/**/*.yml" + - ".gitlab-ci.yml" - "*_VERSION" - "{,jh/}Gemfile{,.lock}" - "Rakefile" @@ -403,8 +400,7 @@ - "Dockerfile.assets" - "vendor/assets/**/*" - ".{eslintignore,gitattributes,nvmrc,prettierrc,stylelintrc,yamllint}" - - ".{codeclimate,eslintrc,gitlab-ci,haml-lint,haml-lint_todo,rubocop,rubocop_todo}.yml" - - ".rubocop_todo/**/*.yml" + - ".gitlab-ci.yml" - "*_VERSION" - "{,jh/}Gemfile{,.lock}" - "Rakefile" @@ -431,8 +427,7 @@ - "Dockerfile.assets" - "vendor/assets/**/*" - ".{eslintignore,gitattributes,nvmrc,prettierrc,stylelintrc,yamllint}" - - ".{codeclimate,eslintrc,gitlab-ci,haml-lint,haml-lint_todo,rubocop,rubocop_todo}.yml" - - ".rubocop_todo/**/*.yml" + - ".gitlab-ci.yml" - "*_VERSION" - "{,jh/}Gemfile{,.lock}" - "Rakefile" @@ -466,8 +461,7 @@ - "Dockerfile.assets" - "vendor/assets/**/*" - ".{eslintignore,gitattributes,nvmrc,prettierrc,stylelintrc,yamllint}" - - ".{codeclimate,eslintrc,gitlab-ci,haml-lint,haml-lint_todo,rubocop,rubocop_todo}.yml" - - ".rubocop_todo/**/*.yml" + - ".gitlab-ci.yml" - "*_VERSION" - "{,jh/}Gemfile{,.lock}" - "Rakefile" @@ -497,6 +491,12 @@ - "scripts/lint_templates_bash.rb" - "lib/gitlab/ci/templates/**/*.gitlab-ci.yml" +.static-analysis-patterns: &static-analysis-patterns + - ".{codeclimate,eslintrc,haml-lint,haml-lint_todo}.yml" + - ".rubocop.yml" + - ".rubocop_todo.yml" + - ".rubocop_todo/**/*.yml" + .danger-patterns: &danger-patterns - "Dangerfile" - "danger/**/*" @@ -1440,16 +1440,19 @@ .static-analysis:rules:ee-and-foss: rules: - changes: *code-backstage-qa-patterns + - changes: *static-analysis-patterns .static-analysis:rules:ee-and-foss-qa: rules: - changes: *qa-patterns + - changes: *static-analysis-patterns .static-analysis:rules:ee: rules: - <<: *if-not-ee when: never - changes: *code-backstage-qa-patterns + - changes: *static-analysis-patterns .static-analysis:rules:as-if-foss: rules: @@ -1461,6 +1464,8 @@ changes: *code-backstage-qa-patterns - <<: *if-merge-request changes: *ci-patterns + - <<: *if-merge-request + changes: *static-analysis-patterns .semgrep-appsec-custom-rules:rules: rules: diff --git a/Gemfile b/Gemfile index c554d509d0e..ddd482b76cb 100644 --- a/Gemfile +++ b/Gemfile @@ -268,7 +268,7 @@ gem 'sanitize', '~> 6.0' gem 'babosa', '~> 1.0.4' # Sanitizes SVG input -gem 'loofah', '~> 2.2' +gem 'loofah', '~> 2.18.0' # Working with license gem 'licensee', '~> 9.14.1' @@ -344,7 +344,7 @@ gem 'prometheus-client-mmap', '~> 0.15.0', require: 'prometheus/client' gem 'warning', '~> 1.2.0' group :development do - gem 'lefthook', '~> 1.0.1', require: false + gem 'lefthook', '~> 1.0.0', require: false gem 'rubocop' gem 'solargraph', '~> 0.44.3', require: false diff --git a/Gemfile.lock b/Gemfile.lock index dd2b7196115..c9a06738760 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -724,7 +724,7 @@ GEM rest-client (~> 2.0) launchy (2.5.0) addressable (~> 2.7) - lefthook (1.0.1) + lefthook (1.0.2) letter_opener (1.7.0) launchy (~> 2.2) letter_opener_web (2.0.0) @@ -749,7 +749,7 @@ GEM activesupport (>= 4) railties (>= 4) request_store (~> 1.0) - loofah (2.16.0) + loofah (2.18.0) crass (~> 1.0.2) nokogiri (>= 1.5.9) lru_redux (1.1.0) @@ -1586,12 +1586,12 @@ DEPENDENCIES knapsack (~> 1.21.1) kramdown (~> 2.3.1) kubeclient (~> 4.9.2) - lefthook (~> 1.0.1) + lefthook (~> 1.0.0) letter_opener_web (~> 2.0.0) licensee (~> 9.14.1) lockbox (~> 0.6.2) lograge (~> 0.5) - loofah (~> 2.2) + loofah (~> 2.18.0) lru_redux mail (= 2.7.1) mail-smtp_pool (~> 0.1.0)! diff --git a/app/assets/javascripts/new_branch_form.js b/app/assets/javascripts/new_branch_form.js index d93db7399e6..ef36e58374c 100644 --- a/app/assets/javascripts/new_branch_form.js +++ b/app/assets/javascripts/new_branch_form.js @@ -1,4 +1,4 @@ -/* eslint-disable func-names, consistent-return, no-return-assign, @gitlab/require-i18n-strings */ +/* eslint-disable func-names, no-return-assign, @gitlab/require-i18n-strings */ import $ from 'jquery'; import RefSelectDropdown from './ref_select_dropdown'; @@ -6,9 +6,9 @@ import RefSelectDropdown from './ref_select_dropdown'; export default class NewBranchForm { constructor(form, availableRefs) { this.validate = this.validate.bind(this); - this.branchNameError = form.find('.js-branch-name-error'); - this.name = form.find('.js-branch-name'); - this.ref = form.find('#ref'); + this.branchNameError = form.querySelector('.js-branch-name-error'); + this.name = form.querySelector('.js-branch-name'); + this.ref = form.querySelector('#ref'); new RefSelectDropdown($('.js-branch-select'), availableRefs); // eslint-disable-line no-new this.setupRestrictions(); this.addBinding(); @@ -16,12 +16,13 @@ export default class NewBranchForm { } addBinding() { - return this.name.on('blur', this.validate); + this.name.addEventListener('blur', this.validate); } init() { - if (this.name.length && this.name.val().length > 0) { - return this.name.trigger('blur'); + if (this.name != null && this.name.value.length > 0) { + const event = new CustomEvent('blur'); + this.name.dispatchEvent(event); } } @@ -52,7 +53,7 @@ export default class NewBranchForm { validate() { const { indexOf } = []; - this.branchNameError.empty(); + this.branchNameError.innerHTML = ''; const unique = function (values, value) { if (indexOf.call(values, value) === -1) { values.push(value); @@ -73,7 +74,7 @@ export default class NewBranchForm { return `${restriction.prefix} ${formatted.join(restriction.conjunction)}`; }; const validator = (errors, restriction) => { - const matched = this.name.val().match(restriction.pattern); + const matched = this.name.value.match(restriction.pattern); if (matched) { return errors.concat(formatter(matched.reduce(unique, []), restriction)); } @@ -81,8 +82,7 @@ export default class NewBranchForm { }; const errors = this.restrictions.reduce(validator, []); if (errors.length > 0) { - const errorMessage = $('').text(errors.join(', ')); - return this.branchNameError.append(errorMessage); + this.branchNameError.textContent = errors.join(', '); } } } diff --git a/app/assets/javascripts/pages/projects/branches/new/index.js b/app/assets/javascripts/pages/projects/branches/new/index.js index 364223f1898..dbae89b5ade 100644 --- a/app/assets/javascripts/pages/projects/branches/new/index.js +++ b/app/assets/javascripts/pages/projects/branches/new/index.js @@ -1,8 +1,7 @@ -import $ from 'jquery'; import NewBranchForm from '~/new_branch_form'; // eslint-disable-next-line no-new new NewBranchForm( - $('.js-create-branch-form'), + document.querySelector('.js-create-branch-form'), JSON.parse(document.getElementById('availableRefs').innerHTML), ); diff --git a/app/controllers/import/fogbugz_controller.rb b/app/controllers/import/fogbugz_controller.rb index b949a99c250..e136f5d2004 100644 --- a/app/controllers/import/fogbugz_controller.rb +++ b/app/controllers/import/fogbugz_controller.rb @@ -22,7 +22,7 @@ class Import::FogbugzController < Import::BaseController session[:fogbugz_token] = res.get_token session[:fogbugz_uri] = params[:uri] - redirect_to new_user_map_import_fogbugz_path + redirect_to new_user_map_import_fogbugz_path(namespace_id: params[:namespace_id]) end def new_user_map @@ -41,12 +41,12 @@ class Import::FogbugzController < Import::BaseController flash[:notice] = _('The user map has been saved. Continue by selecting the projects you want to import.') - redirect_to status_import_fogbugz_path + redirect_to status_import_fogbugz_path(namespace_id: params[:namespace_id]) end def status unless client.valid? - return redirect_to new_import_fogbugz_path + return redirect_to new_import_fogbugz_path(namespace_id: params[:namespace_id]) end super @@ -106,7 +106,7 @@ class Import::FogbugzController < Import::BaseController end def fogbugz_unauthorized(exception) - redirect_to new_import_fogbugz_path, alert: exception.message + redirect_to new_import_fogbugz_path(namespace_id: params[:namespace_id]), alert: exception.message end def import_params diff --git a/app/models/ci/build_report_result.rb b/app/models/ci/build_report_result.rb index 2c08fc4c8bf..b674c1b1a0e 100644 --- a/app/models/ci/build_report_result.rb +++ b/app/models/ci/build_report_result.rb @@ -39,9 +39,5 @@ module Ci def suite_error tests.dig("suite_error") end - - def tests_total - [tests_success, tests_failed, tests_errored, tests_skipped].sum - end end end diff --git a/app/views/import/fogbugz/new.html.haml b/app/views/import/fogbugz/new.html.haml index b74262f2567..bd0e4b51a63 100644 --- a/app/views/import/fogbugz/new.html.haml +++ b/app/views/import/fogbugz/new.html.haml @@ -8,7 +8,7 @@ = _('Import projects from FogBugz') %hr -= form_tag callback_import_fogbugz_path do += form_tag callback_import_fogbugz_path(namespace_id: params[:namespace_id]) do %p = _("To get started you enter your FogBugz URL and login information below. In the next steps, you'll be able to map users and select the projects you want to import.") .form-group.row diff --git a/app/views/import/fogbugz/new_user_map.html.haml b/app/views/import/fogbugz/new_user_map.html.haml index 5caee78b9c4..28836055e0e 100644 --- a/app/views/import/fogbugz/new_user_map.html.haml +++ b/app/views/import/fogbugz/new_user_map.html.haml @@ -8,7 +8,7 @@ = _('Import projects from FogBugz') %hr -= form_tag create_user_map_import_fogbugz_path do += form_tag create_user_map_import_fogbugz_path(namespace_id: params[:namespace_id]) do %p = _("Customize how FogBugz email addresses and usernames are imported into GitLab. In the next step, you'll be able to select the projects you want to import.") %p diff --git a/app/views/import/fogbugz/status.html.haml b/app/views/import/fogbugz/status.html.haml index 3e303d3163d..fb05e8e9724 100644 --- a/app/views/import/fogbugz/status.html.haml +++ b/app/views/import/fogbugz/status.html.haml @@ -8,4 +8,4 @@ - link_to_customize = link_to('customize', new_user_map_import_fogbugz_path) = _('Optionally, you can %{link_to_customize} how FogBugz email addresses and usernames are imported into GitLab.').html_safe % { link_to_customize: link_to_customize } %hr -= render 'import/githubish_status', provider: 'fogbugz', filterable: false += render 'import/githubish_status', provider: 'fogbugz', filterable: false, default_namespace: @namespace diff --git a/app/views/projects/_import_project_pane.html.haml b/app/views/projects/_import_project_pane.html.haml index 9f2c7dd2559..42cdc1d6989 100644 --- a/app/views/projects/_import_project_pane.html.haml +++ b/app/views/projects/_import_project_pane.html.haml @@ -46,7 +46,7 @@ - if fogbugz_import_enabled? %div - = link_to new_import_fogbugz_path, class: 'gl-button btn-default btn import_fogbugz js-import-project-btn', data: { platform: 'fogbugz', **tracking_attrs_data(track_label, 'click_button', 'fogbugz') } do + = link_to new_import_fogbugz_path(namespace_id: namespace_id), class: 'gl-button btn-default btn import_fogbugz js-import-project-btn', data: { platform: 'fogbugz', **tracking_attrs_data(track_label, 'click_button', 'fogbugz') } do .gl-button-icon = sprite_icon('bug') FogBugz diff --git a/app/views/shared/milestones/_header.html.haml b/app/views/shared/milestones/_header.html.haml index 541d7a52385..18db556e024 100644 --- a/app/views/shared/milestones/_header.html.haml +++ b/app/views/shared/milestones/_header.html.haml @@ -11,23 +11,21 @@ .milestone-buttons - if can?(current_user, :admin_milestone, @group || @project) - = link_to _('Edit'), edit_milestone_path(milestone), class: 'btn gl-button btn-grouped' + = render Pajamas::ButtonComponent.new(href: edit_milestone_path(milestone), button_options: { class: 'btn-grouped' }) do + = _('Edit') - if milestone.project_milestone? && milestone.project.group - %button.js-promote-project-milestone-button.btn.gl-button.btn-grouped{ data: { milestone_title: milestone.title, - group_name: milestone.project.group.name, - url: promote_project_milestone_path(milestone.project, milestone)}, - disabled: true, - type: 'button' } + = render Pajamas::ButtonComponent.new(button_options: { class: 'js-promote-project-milestone-button btn-grouped', data: { milestone_title: milestone.title, group_name: milestone.project.group.name, url: promote_project_milestone_path(milestone.project, milestone) }, disabled: true }) do = _('Promote') #promote-milestone-modal - if milestone.active? - = link_to _('Close milestone'), update_milestone_path(milestone, { state_event: :close }), method: :put, class: 'btn gl-button btn-grouped btn-close' + = render Pajamas::ButtonComponent.new(href: update_milestone_path(milestone, { state_event: :close }), button_options: { class: 'btn-grouped btn-close', data: { method: 'put' }, rel: 'nofollow' }) do + = _('Close milestone') - else - = link_to _('Reopen milestone'), update_milestone_path(milestone, { state_event: :activate }), method: :put, class: 'btn gl-button btn-grouped' + = render Pajamas::ButtonComponent.new(href: update_milestone_path(milestone, { state_event: :activate }), button_options: { class: 'btn-grouped', data: { method: 'put' }, rel: 'nofollow' }) do + = _('Reopen milestone') = render 'shared/milestones/delete_button' - %button.btn.gl-button.btn-default.btn-grouped.float-right.d-block.d-sm-none.js-sidebar-toggle{ type: 'button' } - = sprite_icon('chevron-double-lg-left') + = render Pajamas::ButtonComponent.new(icon: 'chevron-double-lg-left', button_options: { 'aria-label' => _('Toggle sidebar'), class: 'btn-grouped gl-float-right! gl-sm-display-none js-sidebar-toggle' }) diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index afe72767b9a..fe68244f1da 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -1,4 +1,4 @@ -= form_errors(hook) += form_errors(hook, pajamas_alert: true) .form-group = form.label :url, s_('Webhooks|URL'), class: 'label-bold' diff --git a/config/feature_flags/development/gitlab_sli_new_counters.yml b/config/feature_flags/development/gitlab_sli_new_counters.yml new file mode 100644 index 00000000000..62d6b82b0db --- /dev/null +++ b/config/feature_flags/development/gitlab_sli_new_counters.yml @@ -0,0 +1,8 @@ +--- +name: gitlab_sli_new_counters +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90977 +rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1760 +milestone: '15.2' +type: development +group: group::scalability +default_enabled: false diff --git a/db/docs/post_migration_test_table.yml b/db/docs/post_migration_test_table.yml new file mode 100644 index 00000000000..60415e15980 --- /dev/null +++ b/db/docs/post_migration_test_table.yml @@ -0,0 +1,8 @@ +--- +table_name: post_migration_test_table +classes: [] +feature_categories: +- database +description: Test table to verify the behavior of the post-deploy independent pipeline +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91119 +milestone: '15.2' diff --git a/db/post_migrate/20220627223041_add_post_migrate_test_table.rb b/db/post_migrate/20220627223041_add_post_migrate_test_table.rb new file mode 100644 index 00000000000..8d97444f8c1 --- /dev/null +++ b/db/post_migrate/20220627223041_add_post_migrate_test_table.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddPostMigrateTestTable < Gitlab::Database::Migration[2.0] + # Fake table to be used for testing the post-deploy pipeline, + # details can be seen on https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/2352. + # + # It should be deleted after the testing is completed. + def change + create_table :post_migration_test_table do |t| + t.integer :status, null: false + end + end +end diff --git a/db/schema_migrations/20220627223041 b/db/schema_migrations/20220627223041 new file mode 100644 index 00000000000..3292e76e1de --- /dev/null +++ b/db/schema_migrations/20220627223041 @@ -0,0 +1 @@ +225606ccdf0979aaf70ff8b9a44269e69b1598718e3d7c1944ed41c07b5e33f6 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 34bfb8380f6..f35efe93d28 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18837,6 +18837,20 @@ CREATE SEQUENCE pool_repositories_id_seq ALTER SEQUENCE pool_repositories_id_seq OWNED BY pool_repositories.id; +CREATE TABLE post_migration_test_table ( + id bigint NOT NULL, + status integer NOT NULL +); + +CREATE SEQUENCE post_migration_test_table_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE post_migration_test_table_id_seq OWNED BY post_migration_test_table.id; + CREATE TABLE postgres_async_indexes ( id bigint NOT NULL, created_at timestamp with time zone NOT NULL, @@ -23235,6 +23249,8 @@ ALTER TABLE ONLY plans ALTER COLUMN id SET DEFAULT nextval('plans_id_seq'::regcl ALTER TABLE ONLY pool_repositories ALTER COLUMN id SET DEFAULT nextval('pool_repositories_id_seq'::regclass); +ALTER TABLE ONLY post_migration_test_table ALTER COLUMN id SET DEFAULT nextval('post_migration_test_table_id_seq'::regclass); + ALTER TABLE ONLY postgres_async_indexes ALTER COLUMN id SET DEFAULT nextval('postgres_async_indexes_id_seq'::regclass); ALTER TABLE ONLY postgres_reindex_actions ALTER COLUMN id SET DEFAULT nextval('postgres_reindex_actions_id_seq'::regclass); @@ -25329,6 +25345,9 @@ ALTER TABLE ONLY plans ALTER TABLE ONLY pool_repositories ADD CONSTRAINT pool_repositories_pkey PRIMARY KEY (id); +ALTER TABLE ONLY post_migration_test_table + ADD CONSTRAINT post_migration_test_table_pkey PRIMARY KEY (id); + ALTER TABLE ONLY postgres_async_indexes ADD CONSTRAINT postgres_async_indexes_pkey PRIMARY KEY (id); diff --git a/doc/api/access_requests.md b/doc/api/access_requests.md index 32411a2f557..adaae78f1b5 100644 --- a/doc/api/access_requests.md +++ b/doc/api/access_requests.md @@ -17,7 +17,7 @@ following levels are recognized: - Reporter (`20`) - Developer (`30`) - Maintainer (`40`) -- Owner (`50`) - Only valid to set for groups +- Owner (`50`). Valid for projects in [GitLab 14.9 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/21432). ## List access requests for a group or project diff --git a/doc/api/invitations.md b/doc/api/invitations.md index 5a39a86d039..51a517ba53a 100644 --- a/doc/api/invitations.md +++ b/doc/api/invitations.md @@ -20,7 +20,7 @@ levels are defined in the `Gitlab::Access` module. Currently, these levels are v - Reporter (`20`) - Developer (`30`) - Maintainer (`40`) -- Owner (`50`) - Only valid to set for groups +- Owner (`50`). Valid for projects in [GitLab 14.9 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/21432). NOTE: From [GitLab 14.9](https://gitlab.com/gitlab-org/gitlab/-/issues/351211) and later, projects have a maximum role of Owner. diff --git a/doc/api/members.md b/doc/api/members.md index 5002e1003e3..a9817918d0b 100644 --- a/doc/api/members.md +++ b/doc/api/members.md @@ -18,7 +18,7 @@ The access levels are defined in the `Gitlab::Access` module. Currently, these l - Reporter (`20`) - Developer (`30`) - Maintainer (`40`) -- Owner (`50`) - Only valid to set for groups +- Owner (`50`). Valid for projects in [GitLab 14.9 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/21432). NOTE: In [GitLab 14.9](https://gitlab.com/gitlab-org/gitlab/-/issues/351211) and later, projects in personal namespaces have an `access_level` of `50`(Owner). diff --git a/doc/development/pipelines.md b/doc/development/pipelines.md index 596af5b777b..9a3a939f51f 100644 --- a/doc/development/pipelines.md +++ b/doc/development/pipelines.md @@ -660,6 +660,7 @@ and included in `rules` definitions via [YAML anchors](../ci/yaml/yaml_optimizat | `code-backstage-patterns` | Combination of `code-patterns` and `backstage-patterns`. | | `code-qa-patterns` | Combination of `code-patterns` and `qa-patterns`. | | `code-backstage-qa-patterns` | Combination of `code-patterns`, `backstage-patterns`, and `qa-patterns`. | +| `static-analysis-patterns` | Only create jobs for Static Analytics configuration-related changes. | ## Performance diff --git a/doc/user/group/index.md b/doc/user/group/index.md index f59e73a383b..d79bd1e33fc 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -628,8 +628,8 @@ You should consider some security implications before configuring IP address res restricted IP address, the IP restriction prevents code from being cloned. - Users may still see some events from the IP restricted groups and projects on their dashboard. Activity may include push, merge, issue, or comment events. -- IP access restrictions for Git operations via SSH are supported only on **(PREMIUM SAAS)** - and higher tiers. IP access restrictions applied to self-managed instances block SSH completely. +- IP access restrictions for Git operations via SSH are supported only on GitLab SaaS. + IP access restrictions applied to self-managed instances block SSH completely. ### Restrict group access by IP address diff --git a/lib/generators/model/model_generator.rb b/lib/generators/model/model_generator.rb new file mode 100644 index 00000000000..533b2ce679d --- /dev/null +++ b/lib/generators/model/model_generator.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'rails/generators' +require 'rails/generators/active_record/model/model_generator' + +module Model + class ModelGenerator < ActiveRecord::Generators::ModelGenerator + source_root File.expand_path('../../../generator_templates/active_record/migration/', __dir__) + + def create_migration_file + return if skip_migration_creation? + + if options[:indexes] == false + attributes.each { |a| a.attr_options.delete(:index) if a.reference? && !a.has_index? } + end + + migration_template "create_table_migration.rb", File.join(db_migrate_path, "create_#{table_name}.rb") + end + + # Override to find templates from superclass as well + def source_paths + super + [self.class.superclass.default_source_root] + end + end +end diff --git a/lib/gitlab/database/gitlab_schemas.yml b/lib/gitlab/database/gitlab_schemas.yml index 0085b578102..52fe921b0c1 100644 --- a/lib/gitlab/database/gitlab_schemas.yml +++ b/lib/gitlab/database/gitlab_schemas.yml @@ -390,6 +390,7 @@ personal_access_tokens: :gitlab_main plan_limits: :gitlab_main plans: :gitlab_main pool_repositories: :gitlab_main +post_migration_test_table: :gitlab_main postgres_async_indexes: :gitlab_shared postgres_autovacuum_activity: :gitlab_shared postgres_foreign_keys: :gitlab_shared diff --git a/lib/gitlab/metrics/sli.rb b/lib/gitlab/metrics/sli.rb index 2de19514354..75734c792d5 100644 --- a/lib/gitlab/metrics/sli.rb +++ b/lib/gitlab/metrics/sli.rb @@ -48,14 +48,24 @@ module Gitlab # This module is effectively an abstract class @initialized_with_combinations = possible_label_combinations.any? # rubocop:disable Gitlab/ModuleWithInstanceVariables possible_label_combinations.each do |label_combination| - total_counter.get(label_combination) - numerator_counter.get(label_combination) + legacy_total_counter.get(label_combination) + legacy_numerator_counter.get(label_combination) + + if ::Feature.enabled?(:gitlab_sli_new_counters) + total_counter.get(label_combination) + numerator_counter.get(label_combination) + end end end def increment(labels:, increment_numerator:) - total_counter.increment(labels) - numerator_counter.increment(labels) if increment_numerator + legacy_total_counter.increment(labels) + legacy_numerator_counter.increment(labels) if increment_numerator + + if ::Feature.enabled?(:gitlab_sli_new_counters) + total_counter.increment(labels) + numerator_counter.increment(labels) if increment_numerator + end end def initialized? @@ -65,7 +75,11 @@ module Gitlab private def total_counter - prometheus.counter(counter_name('total'), "Total number of measurements for #{name}") + prometheus.counter(counter_name('total', '_'), "Total number of measurements for #{name}") + end + + def legacy_total_counter + prometheus.counter(counter_name('total', ':'), "Total number of measurements for #{name}") end def prometheus @@ -81,12 +95,16 @@ module Gitlab private - def counter_name(suffix) - :"#{COUNTER_PREFIX}:#{name}_apdex:#{suffix}" + def counter_name(suffix, separator) + [COUNTER_PREFIX, "#{name}_apdex", suffix].join(separator).to_sym end def numerator_counter - prometheus.counter(counter_name('success_total'), "Number of successful measurements for #{name}") + prometheus.counter(counter_name('success_total', '_'), "Number of successful measurements for #{name}") + end + + def legacy_numerator_counter + prometheus.counter(counter_name('success_total', ':'), "Legacy number of successful measurements for #{name}") end end @@ -99,12 +117,16 @@ module Gitlab private - def counter_name(suffix) - :"#{COUNTER_PREFIX}:#{name}:#{suffix}" + def counter_name(suffix, separator) + [COUNTER_PREFIX, name, suffix].join(separator).to_sym end def numerator_counter - prometheus.counter(counter_name('error_total'), "Number of error measurements for #{name}") + prometheus.counter(counter_name('error_total', '_'), "Number of error measurements for #{name}") + end + + def legacy_numerator_counter + prometheus.counter(counter_name('error_total', ':'), "Number of error measurements for #{name}") end end end diff --git a/qa/qa/resource/personal_access_token.rb b/qa/qa/resource/personal_access_token.rb index d992d7987b4..b7766a3f6de 100644 --- a/qa/qa/resource/personal_access_token.rb +++ b/qa/qa/resource/personal_access_token.rb @@ -30,7 +30,7 @@ module QA fabricate! end - QA::Resource::PersonalAccessTokenCache.set_token_for_username(user.username, token) + QA::Resource::PersonalAccessTokenCache.set_token_for_username(user.username, token) if @user resource end diff --git a/qa/qa/resource/personal_access_token_cache.rb b/qa/qa/resource/personal_access_token_cache.rb index 617779173bd..3e9dc3fd7df 100644 --- a/qa/qa/resource/personal_access_token_cache.rb +++ b/qa/qa/resource/personal_access_token_cache.rb @@ -10,6 +10,7 @@ module QA end def self.set_token_for_username(username, token) + QA::Runtime::Logger.info(%Q[Caching token for username: #{username}, last six chars of token:#{token[-6..]}]) @personal_access_tokens[username] = token end end diff --git a/qa/qa/specs/features/browser_ui/1_manage/login/register_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/login/register_spec.rb index 5487ecff028..85ab466078a 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/login/register_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/login/register_spec.rb @@ -86,7 +86,10 @@ module QA end after do - @recreated_user&.remove_via_api! + if @recreated_user + @recreated_user.api_client = admin_api_client + @recreated_user.remove_via_api! + end end def admin_api_client diff --git a/spec/controllers/import/fogbugz_controller_spec.rb b/spec/controllers/import/fogbugz_controller_spec.rb index 8f8cc9590a5..386bfcb8b3a 100644 --- a/spec/controllers/import/fogbugz_controller_spec.rb +++ b/spec/controllers/import/fogbugz_controller_spec.rb @@ -8,6 +8,7 @@ RSpec.describe Import::FogbugzController do let(:user) { create(:user) } let(:token) { FFaker::Lorem.characters(8) } let(:uri) { 'https://example.com' } + let(:namespace_id) { 5 } before do sign_in(user) @@ -16,9 +17,11 @@ RSpec.describe Import::FogbugzController do describe 'POST #callback' do let(:xml_response) { %Q() } - it 'attempts to contact Fogbugz server' do + before do stub_request(:post, "https://example.com/api.asp").to_return(status: 200, body: xml_response, headers: {}) + end + it 'attempts to contact Fogbugz server' do post :callback, params: { uri: uri, email: 'test@example.com', password: 'mypassword' } expect(session[:fogbugz_token]).to eq(token) @@ -26,6 +29,20 @@ RSpec.describe Import::FogbugzController do expect(response).to redirect_to(new_user_map_import_fogbugz_path) end + it 'preserves namespace_id query param' do + post :callback, params: { uri: uri, email: 'test@example.com', password: 'mypassword', namespace_id: namespace_id } + + expect(response).to redirect_to(new_user_map_import_fogbugz_path(namespace_id: namespace_id)) + end + + it 'redirects to new page form when client raises authentication exception' do + allow(::Gitlab::FogbugzImport::Client).to receive(:new).and_raise(::Fogbugz::AuthenticationException) + + post :callback, params: { uri: uri, email: 'test@example.com', password: 'mypassword' } + + expect(response).to redirect_to(new_import_fogbugz_url) + end + context 'verify url' do shared_examples 'denies local request' do |reason| it 'does not allow requests' do @@ -76,6 +93,16 @@ RSpec.describe Import::FogbugzController do expect(session[:fogbugz_user_map]).to eq(user_map) expect(response).to redirect_to(status_import_fogbugz_path) end + + it 'preserves namespace_id query param' do + client = double(user_map: {}) + expect(controller).to receive(:client).and_return(client) + + post :create_user_map, params: { users: user_map, namespace_id: namespace_id } + + expect(session[:fogbugz_user_map]).to eq(user_map) + expect(response).to redirect_to(status_import_fogbugz_path(namespace_id: namespace_id)) + end end describe 'GET status' do @@ -84,11 +111,19 @@ RSpec.describe Import::FogbugzController do id: 'demo', name: 'vim', safe_name: 'vim', path: 'vim') end - before do - stub_client(valid?: true) + it 'redirects to new page form when client is invalid' do + stub_client(valid?: false) + + get :status + + expect(response).to redirect_to(new_import_fogbugz_path) end it_behaves_like 'import controller status' do + before do + stub_client(valid?: true) + end + let(:repo_id) { repo.id } let(:import_source) { repo.name } let(:provider_name) { 'fogbugz' } diff --git a/spec/controllers/projects/logs_controller_spec.rb b/spec/controllers/projects/logs_controller_spec.rb deleted file mode 100644 index 1c81ae93b42..00000000000 --- a/spec/controllers/projects/logs_controller_spec.rb +++ /dev/null @@ -1,214 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::LogsController do - include KubernetesHelpers - - let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } - - let_it_be(:environment) do - create(:environment, name: 'production', project: project) - end - - let(:pod_name) { "foo" } - let(:container) { 'container-1' } - - before do - sign_in(user) - end - - describe 'GET #index' do - let(:empty_project) { create(:project) } - - it 'returns 404 with reporter access' do - project.add_reporter(user) - - get :index, params: environment_params - - expect(response).to have_gitlab_http_status(:not_found) - end - - it 'renders empty logs page if no environment exists' do - empty_project.add_developer(user) - - get :index, params: { namespace_id: empty_project.namespace, project_id: empty_project } - - expect(response).to be_ok - expect(response).to render_template 'empty_logs' - end - - it 'renders index template' do - project.add_developer(user) - - get :index, params: environment_params - - expect(response).to be_ok - expect(response).to render_template 'index' - end - - context 'with feature flag disabled' do - before do - stub_feature_flags(monitor_logging: false) - end - - it 'returns 404 with reporter access' do - project.add_developer(user) - - get :index, params: environment_params - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - shared_examples 'pod logs service' do |endpoint, service| - let(:service_result) do - { - status: :success, - logs: ['Log 1', 'Log 2', 'Log 3'], - pods: [pod_name], - pod_name: pod_name, - container_name: container - } - end - - let(:service_result_json) { Gitlab::Json.parse(service_result.to_json) } - - let_it_be(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } - - before do - allow_next_instance_of(service) do |instance| - allow(instance).to receive(:execute).and_return(service_result) - end - end - - it 'returns 404 with reporter access' do - project.add_reporter(user) - - get endpoint, params: environment_params(pod_name: pod_name, format: :json) - - expect(response).to have_gitlab_http_status(:not_found) - end - - context 'with developer access' do - before do - project.add_developer(user) - end - - it 'returns the service result' do - get endpoint, params: environment_params(pod_name: pod_name, format: :json) - - expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq(service_result_json) - end - end - - context 'with maintainer access' do - before do - project.add_maintainer(user) - end - - it 'returns the service result' do - get endpoint, params: environment_params(pod_name: pod_name, format: :json) - - expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq(service_result_json) - end - - it 'sets the polling header' do - get endpoint, params: environment_params(pod_name: pod_name, format: :json) - - expect(response).to have_gitlab_http_status(:success) - expect(response.headers['Poll-Interval']).to eq('3000') - end - - context 'with gitlab managed apps logs' do - it 'uses cluster finder services to select cluster', :aggregate_failures do - cluster_list = [cluster] - service_params = { params: ActionController::Parameters.new(pod_name: pod_name).permit! } - request_params = { - namespace_id: project.namespace, - project_id: project, - cluster_id: cluster.id, - pod_name: pod_name, - format: :json - } - - expect_next_instance_of(ClusterAncestorsFinder, project, user) do |finder| - expect(finder).to receive(:execute).and_return(cluster_list) - expect(cluster_list).to receive(:find).and_call_original - end - - expect_next_instance_of(service, cluster, Gitlab::Kubernetes::Helm::NAMESPACE, service_params) do |instance| - expect(instance).to receive(:execute).and_return(service_result) - end - - get endpoint, params: request_params - - expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq(service_result_json) - end - end - - context 'when service is processing' do - let(:service_result) { nil } - - it 'returns a 202' do - get endpoint, params: environment_params(pod_name: pod_name, format: :json) - - expect(response).to have_gitlab_http_status(:accepted) - end - end - - shared_examples 'unsuccessful execution response' do |message| - let(:service_result) do - { - status: :error, - message: message - } - end - - it 'returns the error' do - get endpoint, params: environment_params(pod_name: pod_name, format: :json) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response).to eq(service_result_json) - end - end - - context 'when service is failing' do - it_behaves_like 'unsuccessful execution response', 'some error' - end - - context 'when cluster is nil' do - let!(:cluster) { nil } - - it_behaves_like 'unsuccessful execution response', 'Environment does not have deployments' - end - - context 'when namespace is empty' do - before do - allow(environment).to receive(:deployment_namespace).and_return('') - end - - it_behaves_like 'unsuccessful execution response', 'Environment does not have deployments' - end - end - end - - describe 'GET #k8s' do - it_behaves_like 'pod logs service', :k8s, PodLogs::KubernetesService - end - - describe 'GET #elasticsearch' do - it_behaves_like 'pod logs service', :elasticsearch, PodLogs::ElasticsearchService - end - - def environment_params(opts = {}) - opts.reverse_merge(namespace_id: project.namespace, - project_id: project, - environment_name: environment.name) - end -end diff --git a/spec/features/projects/environments_pod_logs_spec.rb b/spec/features/projects/environments_pod_logs_spec.rb deleted file mode 100644 index ce3b930f92c..00000000000 --- a/spec/features/projects/environments_pod_logs_spec.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Environment > Pod Logs', :js, :kubeclient do - include KubernetesHelpers - - let(:pod_names) { %w(kube-pod) } - let(:pod_name) { pod_names.first } - let(:project) { create(:project, :repository) } - let(:environment) { create(:environment, project: project) } - let(:service) { create(:cluster_platform_kubernetes, :configured) } - - before do - cluster = create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) - create(:deployment, :success, environment: environment) - - stub_kubeclient_pods(environment.deployment_namespace) - stub_kubeclient_deployments(environment.deployment_namespace) - stub_kubeclient_ingresses(environment.deployment_namespace) - stub_kubeclient_nodes_and_nodes_metrics(cluster.platform.api_url) - - sign_in(project.first_owner) - end - - it "shows environments in dropdown" do - create(:environment, project: project) - - visit project_logs_path(environment.project, environment_name: environment.name, pod_name: pod_name) - - wait_for_requests - - page.within('.js-environments-dropdown') do - toggle = find(".dropdown-toggle:not([disabled])") - - expect(toggle).to have_content(environment.name) - - toggle.click - - dropdown_items = find(".dropdown-menu").all(".dropdown-item") - expect(dropdown_items.first).to have_content(environment.name) - expect(dropdown_items.size).to eq(2) - end - end -end diff --git a/spec/frontend/new_branch_spec.js b/spec/frontend/new_branch_spec.js index e4f4b3fa5b5..5a09598059d 100644 --- a/spec/frontend/new_branch_spec.js +++ b/spec/frontend/new_branch_spec.js @@ -1,4 +1,3 @@ -import $ from 'jquery'; import { loadHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; import NewBranchForm from '~/new_branch_form'; @@ -11,17 +10,19 @@ describe('Branch', () => { describe('create a new branch', () => { function fillNameWith(value) { - $('.js-branch-name').val(value).trigger('blur'); + document.querySelector('.js-branch-name').value = value; + const event = new CustomEvent('blur'); + document.querySelector('.js-branch-name').dispatchEvent(event); } function expectToHaveError(error) { - expect($('.js-branch-name-error span').text()).toEqual(error); + expect(document.querySelector('.js-branch-name-error').textContent).toEqual(error); } beforeEach(() => { loadHTMLFixture('branches/new_branch.html'); - $('form').on('submit', (e) => e.preventDefault()); - testContext.form = new NewBranchForm($('.js-create-branch-form'), []); + document.querySelector('form').addEventListener('submit', (e) => e.preventDefault()); + testContext.form = new NewBranchForm(document.querySelector('.js-create-branch-form'), []); }); afterEach(() => { @@ -171,34 +172,34 @@ describe('Branch', () => { it('removes the error message when is a valid name', () => { fillNameWith('foo?bar'); - expect($('.js-branch-name-error span').length).toEqual(1); + expect(document.querySelector('.js-branch-name-error').textContent).not.toEqual(''); fillNameWith('foobar'); - expect($('.js-branch-name-error span').length).toEqual(0); + expect(document.querySelector('.js-branch-name-error').textContent).toEqual(''); }); it('can have dashes anywhere', () => { fillNameWith('-foo-bar-zoo-'); - expect($('.js-branch-name-error span').length).toEqual(0); + expect(document.querySelector('.js-branch-name-error').textContent).toEqual(''); }); it('can have underscores anywhere', () => { fillNameWith('_foo_bar_zoo_'); - expect($('.js-branch-name-error span').length).toEqual(0); + expect(document.querySelector('.js-branch-name-error').textContent).toEqual(''); }); it('can have numbers anywhere', () => { fillNameWith('1foo2bar3zoo4'); - expect($('.js-branch-name-error span').length).toEqual(0); + expect(document.querySelector('.js-branch-name-error').textContent).toEqual(''); }); it('can be only letters', () => { fillNameWith('foo'); - expect($('.js-branch-name-error span').length).toEqual(0); + expect(document.querySelector('.js-branch-name-error').textContent).toEqual(''); }); }); }); diff --git a/spec/lib/generators/model/mocks/migration_file.txt b/spec/lib/generators/model/mocks/migration_file.txt new file mode 100644 index 00000000000..e92c2d2b534 --- /dev/null +++ b/spec/lib/generators/model/mocks/migration_file.txt @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CreateModelGeneratorTestFoos < Gitlab::Database::Migration[2.0] + # When using the methods "add_concurrent_index" or "remove_concurrent_index" + # you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + create_table :model_generator_test_foos do |t| + + t.timestamps null: false + end + end +end diff --git a/spec/lib/generators/model/mocks/model_file.txt b/spec/lib/generators/model/mocks/model_file.txt new file mode 100644 index 00000000000..066db4bfd76 --- /dev/null +++ b/spec/lib/generators/model/mocks/model_file.txt @@ -0,0 +1,2 @@ +class ModelGeneratorTestFoo < ApplicationRecord +end diff --git a/spec/lib/generators/model/mocks/spec_file.txt b/spec/lib/generators/model/mocks/spec_file.txt new file mode 100644 index 00000000000..efd700df0a1 --- /dev/null +++ b/spec/lib/generators/model/mocks/spec_file.txt @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe ModelGeneratorTestFoo, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/lib/generators/model/model_generator_spec.rb b/spec/lib/generators/model/model_generator_spec.rb new file mode 100644 index 00000000000..0e770190d25 --- /dev/null +++ b/spec/lib/generators/model/model_generator_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Model::ModelGenerator do + let(:args) { ['ModelGeneratorTestFoo'] } + let(:options) { { 'migration' => true, 'timestamps' => true, 'indexes' => true, 'test_framework' => :rspec } } + let(:temp_dir) { Dir.mktmpdir } + let(:migration_file_path) { Dir.glob(File.join(temp_dir, 'db/migrate/*create_model_generator_test_foos.rb')).first } + let(:model_file_path) { File.join(temp_dir, 'app/models/model_generator_test_foo.rb') } + let(:spec_file_path) { File.join(temp_dir, 'spec/models/model_generator_test_foo_spec.rb') } + + subject { described_class.new(args, options, { destination_root: temp_dir }) } + + context 'when generating a model' do + after do + FileUtils.rm_rf(temp_dir) + end + + it 'creates the model file with the right content' do + subject.invoke_all + + expect(File).to exist(model_file_path) + mock_model_file_content = File.read(File.expand_path('./mocks/model_file.txt', __dir__)) + model_file_content = File.read(model_file_path) + expect(model_file_content).to eq(mock_model_file_content) + end + + it 'creates the migration file with the right content' do + subject.invoke_all + + expect(File).to exist(migration_file_path) + mock_migration_file_content = File.read(File.expand_path('./mocks/migration_file.txt', __dir__)) + migration_file_content = File.read(migration_file_path) + expect(migration_file_content).to eq(mock_migration_file_content) + end + + it 'creates the spec file with the right content' do + subject.invoke_all + + expect(File).to exist(spec_file_path) + mock_spec_file_content = File.read(File.expand_path('./mocks/spec_file.txt', __dir__)) + spec_file_content = File.read(spec_file_path) + expect(spec_file_content).to eq(mock_spec_file_content) + end + end +end diff --git a/spec/lib/gitlab/metrics/sli_spec.rb b/spec/lib/gitlab/metrics/sli_spec.rb index 102ea442b3a..983b90b9f25 100644 --- a/spec/lib/gitlab/metrics/sli_spec.rb +++ b/spec/lib/gitlab/metrics/sli_spec.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true -require 'fast_spec_helper' +# Switch this back to `fast_spec_helper` when removing the `gitlab_sli_new_counters` +# feature flag +require 'spec_helper' RSpec.describe Gitlab::Metrics::Sli do let(:prometheus) { double("prometheus") } @@ -18,12 +20,16 @@ RSpec.describe Gitlab::Metrics::Sli do it 'allows different SLIs to be defined on each subclass' do apdex_counters = [ fake_total_counter('foo_apdex'), - fake_numerator_counter('foo_apdex', 'success') + fake_numerator_counter('foo_apdex', 'success'), + fake_total_counter('foo_apdex', ':'), + fake_numerator_counter('foo_apdex', 'success', ':') ] error_rate_counters = [ fake_total_counter('foo'), - fake_numerator_counter('foo', 'error') + fake_numerator_counter('foo', 'error'), + fake_total_counter('foo', ':'), + fake_numerator_counter('foo', 'error', ':') ] apdex = described_class::Apdex.initialize_sli(:foo, [{ hello: :world }]) @@ -78,7 +84,9 @@ RSpec.describe Gitlab::Metrics::Sli do it 'returns and stores a new initialized SLI' do counters = [ fake_total_counter("bar#{subclass_info[:suffix]}"), - fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator]) + fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator]), + fake_total_counter("bar#{subclass_info[:suffix]}", ':'), + fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator], ':') ] sli = described_class.initialize_sli(:bar, [{ hello: :world }]) @@ -91,7 +99,9 @@ RSpec.describe Gitlab::Metrics::Sli do it 'does not change labels for an already-initialized SLI' do counters = [ fake_total_counter("bar#{subclass_info[:suffix]}"), - fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator]) + fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator]), + fake_total_counter("bar#{subclass_info[:suffix]}", ':'), + fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator], ':') ] sli = described_class.initialize_sli(:bar, [{ hello: :world }]) @@ -112,6 +122,8 @@ RSpec.describe Gitlab::Metrics::Sli do before do fake_total_counter("boom#{subclass_info[:suffix]}") fake_numerator_counter("boom#{subclass_info[:suffix]}", subclass_info[:numerator]) + fake_total_counter("boom#{subclass_info[:suffix]}", ':') + fake_numerator_counter("boom#{subclass_info[:suffix]}", subclass_info[:numerator], ':') end it 'is true when an SLI was initialized with labels' do @@ -130,7 +142,9 @@ RSpec.describe Gitlab::Metrics::Sli do it 'initializes counters for the passed label combinations' do counters = [ fake_total_counter("hey#{subclass_info[:suffix]}"), - fake_numerator_counter("hey#{subclass_info[:suffix]}", subclass_info[:numerator]) + fake_numerator_counter("hey#{subclass_info[:suffix]}", subclass_info[:numerator]), + fake_total_counter("hey#{subclass_info[:suffix]}", ':'), + fake_numerator_counter("hey#{subclass_info[:suffix]}", subclass_info[:numerator], ':') ] described_class.new(:hey).initialize_counters([{ foo: 'bar' }, { foo: 'baz' }]) @@ -138,18 +152,43 @@ RSpec.describe Gitlab::Metrics::Sli do expect(counters).to all(have_received(:get).with({ foo: 'bar' })) expect(counters).to all(have_received(:get).with({ foo: 'baz' })) end + + context 'when `gitlab_sli_new_counters` is disabled' do + before do + stub_feature_flags(gitlab_sli_new_counters: false) + end + + it 'does not initialize the new counters', :aggregate_failures do + new_total_counter = fake_total_counter("bar#{subclass_info[:suffix]}") + new_numerator_counter = fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator]) + + fake_total_counter("bar#{subclass_info[:suffix]}", ':') + fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator], ':') + + described_class.new(:bar).initialize_counters([{ hello: :world }]) + + expect(new_total_counter).not_to have_received(:get) + expect(new_numerator_counter).not_to have_received(:get) + end + end end describe "#increment" do let!(:sli) { described_class.new(:heyo) } let!(:total_counter) { fake_total_counter("heyo#{subclass_info[:suffix]}") } let!(:numerator_counter) { fake_numerator_counter("heyo#{subclass_info[:suffix]}", subclass_info[:numerator]) } + let!(:legacy_total_counter) { fake_total_counter("heyo#{subclass_info[:suffix]}", ':') } + let!(:legacy_numerator_counter) do + fake_numerator_counter("heyo#{subclass_info[:suffix]}", subclass_info[:numerator], ':') + end it "increments both counters for labels when #{subclass_info[:numerator]} is true" do sli.increment(labels: { hello: "world" }, subclass_info[:numerator] => true) expect(total_counter).to have_received(:increment).with({ hello: 'world' }) expect(numerator_counter).to have_received(:increment).with({ hello: 'world' }) + expect(legacy_total_counter).to have_received(:increment).with({ hello: 'world' }) + expect(legacy_numerator_counter).to have_received(:increment).with({ hello: 'world' }) end it "only increments the total counters for labels when #{subclass_info[:numerator]} is false" do @@ -157,6 +196,31 @@ RSpec.describe Gitlab::Metrics::Sli do expect(total_counter).to have_received(:increment).with({ hello: 'world' }) expect(numerator_counter).not_to have_received(:increment).with({ hello: 'world' }) + expect(legacy_total_counter).to have_received(:increment).with({ hello: 'world' }) + expect(legacy_numerator_counter).not_to have_received(:increment).with({ hello: 'world' }) + end + + context 'when `gitlab_sli_new_counters` is disabled' do + before do + stub_feature_flags(gitlab_sli_new_counters: false) + end + + it 'does increment new counters', :aggregate_failures do + new_total_counter = fake_total_counter("bar#{subclass_info[:suffix]}") + new_numerator_counter = fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator]) + + legacy_total_counter = fake_total_counter("bar#{subclass_info[:suffix]}", ':') + legacy_numerator_counter = fake_numerator_counter( + "bar#{subclass_info[:suffix]}", subclass_info[:numerator], ':' + ) + + described_class.new(:bar).increment(labels: { hello: 'world' }, subclass_info[:numerator] => true) + + expect(new_total_counter).not_to have_received(:increment) + expect(new_numerator_counter).not_to have_received(:increment) + expect(legacy_total_counter).to have_received(:increment) + expect(legacy_numerator_counter).to have_received(:increment) + end end end end @@ -172,11 +236,11 @@ RSpec.describe Gitlab::Metrics::Sli do fake_counter end - def fake_total_counter(name) - fake_prometheus_counter("gitlab_sli:#{name}:total") + def fake_total_counter(name, separator = '_') + fake_prometheus_counter(['gitlab_sli', name, 'total'].join(separator)) end - def fake_numerator_counter(name, numerator_name) - fake_prometheus_counter("gitlab_sli:#{name}:#{numerator_name}_total") + def fake_numerator_counter(name, numerator_name, separator = '_') + fake_prometheus_counter(["gitlab_sli", name, "#{numerator_name}_total"].join(separator)) end end diff --git a/spec/models/ci/build_report_result_spec.rb b/spec/models/ci/build_report_result_spec.rb index 3f53c6c1c0e..09ea19cf077 100644 --- a/spec/models/ci/build_report_result_spec.rb +++ b/spec/models/ci/build_report_result_spec.rb @@ -70,10 +70,4 @@ RSpec.describe Ci::BuildReportResult do expect(build_report_result.tests_skipped).to eq(0) end end - - describe '#tests_total' do - it 'returns the total count' do - expect(build_report_result.tests_total).to eq(2) - end - end end