diff options
11 files changed, 219 insertions, 73 deletions
diff --git a/app/assets/javascripts/pages/admin/users/components/delete_user_modal.vue b/app/assets/javascripts/pages/admin/users/components/delete_user_modal.vue index 78aaa9df0ec..b43d6ba17d7 100644 --- a/app/assets/javascripts/pages/admin/users/components/delete_user_modal.vue +++ b/app/assets/javascripts/pages/admin/users/components/delete_user_modal.vue @@ -109,7 +109,7 @@ export default { <template> <p v-html="text"></p> <p v-html="confirmationTextLabel"></p> - <form ref="form" :action="deleteUserUrl" method="post"> + <form ref="form" :action="deleteUserUrl" method="post" @submit.prevent> <input ref="method" type="hidden" name="_method" value="delete" /> <input :value="csrfToken" type="hidden" name="authenticity_token" /> <gl-form-input diff --git a/app/views/clusters/clusters/gcp/_form.html.haml b/app/views/clusters/clusters/gcp/_form.html.haml index 95670a2ec87..ab01569b8fd 100644 --- a/app/views/clusters/clusters/gcp/_form.html.haml +++ b/app/views/clusters/clusters/gcp/_form.html.haml @@ -64,7 +64,7 @@ %p.form-text.text-muted = s_('ClusterIntegration|Learn more about %{help_link_start_machine_type}machine types%{help_link_end} and %{help_link_start_pricing}pricing%{help_link_end}.').html_safe % { help_link_start_machine_type: help_link_start % { url: machine_type_link_url }, help_link_start_pricing: help_link_start % { url: pricing_link_url }, help_link_end: help_link_end } - - if Feature.enabled?(:create_cloud_run_clusters, clusterable) + - if Feature.enabled?(:create_cloud_run_clusters, clusterable, default_enabled: true) .form-group = provider_gcp_field.check_box :cloud_run, { label: s_('ClusterIntegration|Enable Cloud Run on GKE (beta)'), label_class: 'label-bold' } diff --git a/changelogs/unreleased/28365-bypass-confirmation-box-for-delete-user-and-contributions.yml b/changelogs/unreleased/28365-bypass-confirmation-box-for-delete-user-and-contributions.yml new file mode 100644 index 00000000000..27a85ffe008 --- /dev/null +++ b/changelogs/unreleased/28365-bypass-confirmation-box-for-delete-user-and-contributions.yml @@ -0,0 +1,5 @@ +--- +title: Fix delete user dialog bypass caused by hitting enter +merge_request: 17343 +author: +type: fixed diff --git a/changelogs/unreleased/42738-change-commit-user-mentions-commit-id-column-type.yml b/changelogs/unreleased/42738-change-commit-user-mentions-commit-id-column-type.yml new file mode 100644 index 00000000000..cd6e2ba6770 --- /dev/null +++ b/changelogs/unreleased/42738-change-commit-user-mentions-commit-id-column-type.yml @@ -0,0 +1,5 @@ +--- +title: Change commit_id type on commit_user_mentions table +merge_request: 21651 +author: +type: fixed diff --git a/changelogs/unreleased/cloud_run_feature_enabled_by_default.yml b/changelogs/unreleased/cloud_run_feature_enabled_by_default.yml new file mode 100644 index 00000000000..5ad41ae95ac --- /dev/null +++ b/changelogs/unreleased/cloud_run_feature_enabled_by_default.yml @@ -0,0 +1,5 @@ +--- +title: Re-enable the cloud run feature +merge_request: https://gitlab.com/gitlab-org/gitlab/merge_requests/21762 +author: +type: fixed diff --git a/db/migrate/20191212140117_change_commit_user_mentions_commit_id_column_type.rb b/db/migrate/20191212140117_change_commit_user_mentions_commit_id_column_type.rb new file mode 100644 index 00000000000..f30cdab3441 --- /dev/null +++ b/db/migrate/20191212140117_change_commit_user_mentions_commit_id_column_type.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class ChangeCommitUserMentionsCommitIdColumnType < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + OLD_INDEX = 'commit_user_mentions_on_commit_id_and_note_id_index' + OLD_TMP_INDEX = 'temp_commit_id_and_note_id_index' + NEW_TMP_INDEX = 'temp_commit_id_for_type_change_and_note_id_index' + NEW_INDEX = 'commit_id_and_note_id_index' + + def up + # the initial index name is too long and fails during migration. Renaming the index first. + add_concurrent_index :commit_user_mentions, [:commit_id, :note_id], name: OLD_TMP_INDEX + remove_concurrent_index_by_name :commit_user_mentions, OLD_INDEX + + change_column_type_concurrently :commit_user_mentions, :commit_id, :string + + # change_column_type_concurrently creates a new index for new column `commit_id_for_type` based on existing + # `temp_commit_id_and_note_id_index` naming it `temp_commit_id_for_type_change_and_note_id_index`, yet keeping + # `temp_commit_id_and_note_id_index` for `commit_id`, that will be cleaned + # by `cleanup_concurrent_column_type_change :commit_user_mentions, :commit_id` in a later migration. + # + # So we'll rename `temp_commit_id_for_type_change_and_note_id_index` to initialy intended name: `commit_id_and_note_id_index`. + + add_concurrent_index :commit_user_mentions, [:commit_id_for_type_change, :note_id], name: NEW_INDEX + remove_concurrent_index_by_name :commit_user_mentions, NEW_TMP_INDEX + end + + def down + cleanup_concurrent_column_type_change :commit_user_mentions, :commit_id + end +end diff --git a/db/post_migrate/20191212162434_change_commit_user_mentions_commit_id_column_type_cleanup.rb b/db/post_migrate/20191212162434_change_commit_user_mentions_commit_id_column_type_cleanup.rb new file mode 100644 index 00000000000..aed9d335af9 --- /dev/null +++ b/db/post_migrate/20191212162434_change_commit_user_mentions_commit_id_column_type_cleanup.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class ChangeCommitUserMentionsCommitIdColumnTypeCleanup < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + NEW_INDEX = 'commit_id_for_type_change_and_note_id_index' + OLD_INDEX = 'commit_user_mentions_on_commit_id_and_note_id_index' + + def up + cleanup_concurrent_column_type_change :commit_user_mentions, :commit_id + end + + def down + change_column_type_concurrently :commit_user_mentions, :commit_id, :binary + + # change_column_type_concurrently creates a new index based on existing commit_id_and_note_id_index` naming it + # `commit_id_for_type_change_and_note_id_index` so we'll rename it back to its original name. + add_concurrent_index :commit_user_mentions, [:commit_id_for_type_change, :note_id], name: OLD_INDEX + remove_concurrent_index_by_name :commit_user_mentions, NEW_INDEX + end +end diff --git a/db/schema.rb b/db/schema.rb index 67d305a3505..2422bb35cbd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_12_17_160632) do +ActiveRecord::Schema.define(version: 2019_12_16_183532) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -1217,11 +1217,11 @@ ActiveRecord::Schema.define(version: 2019_12_17_160632) do create_table "commit_user_mentions", force: :cascade do |t| t.integer "note_id", null: false - t.binary "commit_id", null: false t.integer "mentioned_users_ids", array: true t.integer "mentioned_projects_ids", array: true t.integer "mentioned_groups_ids", array: true - t.index ["commit_id", "note_id"], name: "commit_user_mentions_on_commit_id_and_note_id_index" + t.string "commit_id", null: false + t.index ["commit_id", "note_id"], name: "commit_id_and_note_id_index" t.index ["note_id"], name: "index_commit_user_mentions_on_note_id", unique: true end diff --git a/doc/administration/gitaly/praefect.md b/doc/administration/gitaly/praefect.md index 21a1e7ea691..6193a40ac4f 100644 --- a/doc/administration/gitaly/praefect.md +++ b/doc/administration/gitaly/praefect.md @@ -66,7 +66,7 @@ We need to manage the following secrets and make them match across hosts: #### Praefect On the Praefect node we disable all other services, including Gitaly. We list each -Gitaly node that will be connected to Praefect under `praefect['storage_nodes']`. +Gitaly node that will be connected to Praefect as members of the `praefect` hash in `praefect['virtual_storages']`. In the example below, the Gitaly nodes are named `gitaly-N`. Note that one node is designated as primary by setting the primary to `true`. @@ -84,15 +84,6 @@ unicorn['enable'] = false sidekiq['enable'] = false gitlab_workhorse['enable'] = false gitaly['enable'] = false -``` - -##### Set up Praefect and its Gitaly nodes - -In the example below, the Gitaly nodes are named `gitaly-X`. Note that one node is designated as -primary, by setting the primary to `true`: - -```ruby -# /etc/gitlab/gitlab.rb on praefect server # Prevent database connections during 'gitlab-ctl reconfigure' gitlab_rails['rake_cache_clear'] = false @@ -104,27 +95,27 @@ praefect['enable'] = true # firewalls to restrict access to this address/port. praefect['listen_addr'] = '0.0.0.0:2305' -# virtual_storage_name must match the same storage name given to praefect in git_data_dirs -praefect['virtual_storage_name'] = 'praefect' - # Replace PRAEFECT_EXTERNAL_TOKEN with a real secret praefect['auth_token'] = 'PRAEFECT_EXTERNAL_TOKEN' # Replace each instance of PRAEFECT_INTERNAL_TOKEN below with a real # secret, distinct from PRAEFECT_EXTERNAL_TOKEN. -praefect['storage_nodes'] = { - 'gitaly-1' => { - 'address' => 'tcp://gitaly-1.internal:8075', - 'token' => 'PRAEFECT_INTERNAL_TOKEN', - 'primary' => true - }, - 'gitaly-2' => { - 'address' => 'tcp://gitaly-2.internal:8075', - 'token' => 'PRAEFECT_INTERNAL_TOKEN' - }, - 'gitaly-3' => { - 'address' => 'tcp://gitaly-3.internal:8075', - 'token' => 'PRAEFECT_INTERNAL_TOKEN' +# Name of storage hash must match storage name in git_data_dirs on GitLab server. +praefect['virtual_storages'] = { + 'praefect' => { + 'gitaly-1' => { + 'address' => 'tcp://gitaly-1.internal:8075', + 'token' => 'PRAEFECT_INTERNAL_TOKEN', + 'primary' => true + }, + 'gitaly-2' => { + 'address' => 'tcp://gitaly-2.internal:8075', + 'token' => 'PRAEFECT_INTERNAL_TOKEN' + }, + 'gitaly-3' => { + 'address' => 'tcp://gitaly-3.internal:8075', + 'token' => 'PRAEFECT_INTERNAL_TOKEN' + } } } ``` @@ -140,7 +131,7 @@ auth tokens from Praefect instead of GitLab. Below is an example configuration for `gitaly-1`, the only difference for the other Gitaly nodes is the storage name under `git_data_dirs`. -Note that `gitaly['auth_token']` matches the `token` value listed under `praefect['storage_nodes']` +Note that `gitaly['auth_token']` matches the `token` value listed under `praefect['virtual_storages']` on the Praefect node. ```ruby @@ -155,6 +146,7 @@ grafana['enable'] = false unicorn['enable'] = false sidekiq['enable'] = false gitlab_workhorse['enable'] = false +prometheus_monitoring['enable'] = false # Prevent database connections during 'gitlab-ctl reconfigure' gitlab_rails['rake_cache_clear'] = false @@ -197,7 +189,7 @@ is present, there should be two storages available to GitLab: # Replace PRAEFECT_EXTERNAL_TOKEN below with real secret. git_data_dirs({ "default" => { - "gitaly_address" => "tcp://gitaly.internal" + "path" => "/var/opt/gitlab/git-data" }, "praefect" => { "gitaly_address" => "tcp://praefect.internal:2305", @@ -212,7 +204,9 @@ gitlab_shell['secret_token'] = 'GITLAB_SHELL_SECRET_TOKEN' Note that the storage name used is the same as the `praefect['virtual_storage_name']` set on the Praefect node. -Restart GitLab using `gitlab-ctl restart` on the GitLab node. +Save your changes and [reconfigure GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure). + +Run `gitlab-rake gitlab:gitaly:check` to confirm that GitLab can reach Praefect. ### Testing Praefect @@ -220,6 +214,18 @@ To test Praefect, first set it as the default storage node for new projects using **Admin Area > Settings > Repository > Repository storage**. Next, create a new project and check the "Initialize repository with a README" box. -If you receive a 503 error, check `/var/log/gitlab/gitlab-rails/production.log`. -A `GRPC::Unavailable (14:failed to connect to all addresses)` error indicates -that GitLab was unable to connect to Praefect. +If you receive an error, check `/var/log/gitlab/gitlab-rails/production.log`. + +Here are common errors and potential causes: + +- 500 response code + - **ActionView::Template::Error (7:permission denied)** + - `praefect['auth_token']` and `gitlab_rails['gitaly_token']` do not match on the GitLab server. + - **Unable to save project. Error: 7:permission denied** + - Secret token in `praefect['storage_nodes']` on GitLab server does not match the + value in `gitaly['auth_token']` on one or more Gitaly servers. +- 503 response code + - **GRPC::Unavailable (14:failed to connect to all addresses)** + - GitLab was unable to reach Praefect. + - **GRPC::Unavailable (14:all SubCons are in TransientFailure...)** + - Praefect cannot reach one or more of its child Gitaly nodes. diff --git a/doc/api/issue_links.md b/doc/api/issue_links.md index 280431fa87c..b7e21310a19 100644 --- a/doc/api/issue_links.md +++ b/doc/api/issue_links.md @@ -57,7 +57,7 @@ Parameters: Creates a two-way relation between two issues. User must be allowed to update both issues in order to succeed. ``` -POST /projects/:id/issues/:issue_iid/links +POST /projects/:id/issues/:issue_iid/links/:target_project_id/:target_issue_iid ``` | Attribute | Type | Required | Description | diff --git a/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js b/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js index 57802a41bb5..3efebc69011 100644 --- a/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js +++ b/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js @@ -3,26 +3,46 @@ import { GlButton, GlFormInput } from '@gitlab/ui'; import DeleteUserModal from '~/pages/admin/users/components/delete_user_modal.vue'; import ModalStub from './stubs/modal_stub'; +const TEST_DELETE_USER_URL = 'delete-url'; +const TEST_BLOCK_USER_URL = 'block-url'; +const TEST_CSRF = 'csrf'; + describe('User Operation confirmation modal', () => { let wrapper; + let formSubmitSpy; const findButton = variant => wrapper .findAll(GlButton) .filter(w => w.attributes('variant') === variant) .at(0); + const findForm = () => wrapper.find('form'); + const findUsernameInput = () => wrapper.find(GlFormInput); + const findPrimaryButton = () => findButton('danger'); + const findSecondaryButton = () => findButton('warning'); + const findAuthenticityToken = () => new FormData(findForm().element).get('authenticity_token'); + const getUsername = () => findUsernameInput().attributes('value'); + const getMethodParam = () => new FormData(findForm().element).get('_method'); + const getFormAction = () => findForm().attributes('action'); + + const setUsername = username => { + findUsernameInput().vm.$emit('input', username); + }; + + const username = 'username'; + const badUsername = 'bad_username'; const createComponent = (props = {}) => { wrapper = shallowMount(DeleteUserModal, { propsData: { + username, title: 'title', content: 'content', action: 'action', secondaryAction: 'secondaryAction', - deleteUserUrl: 'delete-url', - blockUserUrl: 'block-url', - username: 'username', - csrfToken: 'csrf', + deleteUserUrl: TEST_DELETE_USER_URL, + blockUserUrl: TEST_BLOCK_USER_URL, + csrfToken: TEST_CSRF, ...props, }, stubs: { @@ -32,6 +52,10 @@ describe('User Operation confirmation modal', () => { }); }; + beforeEach(() => { + formSubmitSpy = jest.spyOn(HTMLFormElement.prototype, 'submit').mockImplementation(); + }); + afterEach(() => { wrapper.destroy(); wrapper = null; @@ -42,44 +66,84 @@ describe('User Operation confirmation modal', () => { expect(wrapper.element).toMatchSnapshot(); }); - it.each` - variant | prop | action - ${'danger'} | ${'deleteUserUrl'} | ${'delete'} - ${'warning'} | ${'blockUserUrl'} | ${'block'} - `('closing modal with $variant button triggers $action', ({ variant, prop }) => { - createComponent(); - const form = wrapper.find('form'); - jest.spyOn(form.element, 'submit').mockReturnValue(); - const modalButton = findButton(variant); - modalButton.vm.$emit('click'); - return wrapper.vm.$nextTick().then(() => { - expect(form.element.submit).toHaveBeenCalled(); - expect(form.element.action).toContain(wrapper.props(prop)); - expect(new FormData(form.element).get('authenticity_token')).toEqual( - wrapper.props('csrfToken'), - ); + describe('on created', () => { + beforeEach(() => { + createComponent(); + }); + + it('has disabled buttons', () => { + expect(findPrimaryButton().attributes('disabled')).toBeTruthy(); + expect(findSecondaryButton().attributes('disabled')).toBeTruthy(); }); }); - it('disables buttons by default', () => { - createComponent(); - const blockButton = findButton('warning'); - const deleteButton = findButton('danger'); - expect(blockButton.attributes().disabled).toBeTruthy(); - expect(deleteButton.attributes().disabled).toBeTruthy(); + describe('with incorrect username', () => { + beforeEach(() => { + createComponent(); + setUsername(badUsername); + + return wrapper.vm.$nextTick(); + }); + + it('shows incorrect username', () => { + expect(getUsername()).toEqual(badUsername); + }); + + it('has disabled buttons', () => { + expect(findPrimaryButton().attributes('disabled')).toBeTruthy(); + expect(findSecondaryButton().attributes('disabled')).toBeTruthy(); + }); }); - it('enables button when username is typed', () => { - createComponent({ - username: 'some-username', + describe('with correct username', () => { + beforeEach(() => { + createComponent(); + setUsername(username); + + return wrapper.vm.$nextTick(); + }); + + it('shows correct username', () => { + expect(getUsername()).toEqual(username); + }); + + it('has enabled buttons', () => { + expect(findPrimaryButton().attributes('disabled')).toBeFalsy(); + expect(findSecondaryButton().attributes('disabled')).toBeFalsy(); }); - wrapper.find(GlFormInput).vm.$emit('input', 'some-username'); - const blockButton = findButton('warning'); - const deleteButton = findButton('danger'); - return wrapper.vm.$nextTick().then(() => { - expect(blockButton.attributes().disabled).toBeFalsy(); - expect(deleteButton.attributes().disabled).toBeFalsy(); + describe('when primary action is submitted', () => { + beforeEach(() => { + findPrimaryButton().vm.$emit('click'); + + return wrapper.vm.$nextTick(); + }); + + it('clears the input', () => { + expect(getUsername()).toEqual(''); + }); + + it('has correct form attributes and calls submit', () => { + expect(getFormAction()).toBe(TEST_DELETE_USER_URL); + expect(getMethodParam()).toBe('delete'); + expect(findAuthenticityToken()).toBe(TEST_CSRF); + expect(formSubmitSpy).toHaveBeenCalled(); + }); + }); + + describe('when secondary action is submitted', () => { + beforeEach(() => { + findSecondaryButton().vm.$emit('click'); + + return wrapper.vm.$nextTick(); + }); + + it('has correct form attributes and calls submit', () => { + expect(getFormAction()).toBe(TEST_BLOCK_USER_URL); + expect(getMethodParam()).toBe('put'); + expect(findAuthenticityToken()).toBe(TEST_CSRF); + expect(formSubmitSpy).toHaveBeenCalled(); + }); }); }); }); |