diff --git a/app/assets/javascripts/incidents/components/incidents_list.vue b/app/assets/javascripts/incidents/components/incidents_list.vue index 324797ad645..b55cd406adc 100644 --- a/app/assets/javascripts/incidents/components/incidents_list.vue +++ b/app/assets/javascripts/incidents/components/incidents_list.vue @@ -33,6 +33,7 @@ import { TH_CREATED_AT_TEST_ID, TH_INCIDENT_SLA_TEST_ID, TH_SEVERITY_TEST_ID, + TH_ESCALATION_STATUS_TEST_ID, TH_PUBLISHED_TEST_ID, INCIDENT_DETAILS_PATH, trackIncidentCreateNewOptions, @@ -67,8 +68,11 @@ export default { { key: 'escalationStatus', label: s__('IncidentManagement|Status'), - thClass: `${thClass} gl-w-eighth gl-pointer-events-none`, - tdClass, + thClass: `${thClass} gl-w-eighth`, + tdClass: `${tdClass} sortable-cell`, + actualSortKey: 'ESCALATION_STATUS', + sortable: true, + thAttr: TH_ESCALATION_STATUS_TEST_ID, }, { key: 'createdAt', diff --git a/app/assets/javascripts/incidents/constants.js b/app/assets/javascripts/incidents/constants.js index 21cdbef05a1..ee3f30de880 100644 --- a/app/assets/javascripts/incidents/constants.js +++ b/app/assets/javascripts/incidents/constants.js @@ -47,6 +47,7 @@ export const ESCALATION_STATUSES = { export const DEFAULT_PAGE_SIZE = 20; export const TH_CREATED_AT_TEST_ID = { 'data-testid': 'incident-management-created-at-sort' }; export const TH_SEVERITY_TEST_ID = { 'data-testid': 'incident-management-severity-sort' }; +export const TH_ESCALATION_STATUS_TEST_ID = { 'data-testid': 'incident-management-status-sort' }; export const TH_INCIDENT_SLA_TEST_ID = { 'data-testid': 'incident-management-sla' }; export const TH_PUBLISHED_TEST_ID = { 'data-testid': 'incident-management-published-sort' }; export const INCIDENT_DETAILS_PATH = 'incident'; diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb index 773bbf268eb..fbcfcf84d9b 100644 --- a/app/serializers/issue_entity.rb +++ b/app/serializers/issue_entity.rb @@ -61,7 +61,7 @@ class IssueEntity < IssuableEntity end expose :locked_discussion_docs_path, if: -> (issue) { issue.discussion_locked? } do |issue| - help_page_path('user/discussions/index.md', anchor: 'lock-discussions') + help_page_path('user/discussions/index.md', anchor: 'prevent-comments-by-locking-an-issue') end expose :is_project_archived do |issue| diff --git a/app/serializers/merge_request_noteable_entity.rb b/app/serializers/merge_request_noteable_entity.rb index a356b5b5cd4..f8c8e3538da 100644 --- a/app/serializers/merge_request_noteable_entity.rb +++ b/app/serializers/merge_request_noteable_entity.rb @@ -43,7 +43,7 @@ class MergeRequestNoteableEntity < IssuableEntity end expose :locked_discussion_docs_path, if: -> (merge_request) { merge_request.discussion_locked? } do |merge_request| - help_page_path('user/discussions/index.md', anchor: 'lock-discussions') + help_page_path('user/discussions/index.md', anchor: 'prevent-comments-by-locking-an-issue') end expose :is_project_archived do |merge_request| diff --git a/doc/user/application_security/api_fuzzing/index.md b/doc/user/application_security/api_fuzzing/index.md index 5413c28912a..ae2b4ec84c6 100644 --- a/doc/user/application_security/api_fuzzing/index.md +++ b/doc/user/application_security/api_fuzzing/index.md @@ -345,8 +345,8 @@ By default, the API fuzzer uses the Postman file to resolve Postman variable val is set in a GitLab CI/CD variable `FUZZAPI_POSTMAN_COLLECTION_VARIABLES`, then the JSON file takes precedence to get Postman variable values. -Although Postman can export environment variables into a JSON file, the format is not compatible -with the JSON expected by `FUZZAPI_POSTMAN_COLLECTION_VARIABLES`. +WARNING: +Although Postman can export environment variables into a JSON file, the format is not compatible with the JSON expected by `FUZZAPI_POSTMAN_COLLECTION_VARIABLES`. Here is an example of using `FUZZAPI_POSTMAN_COLLECTION_VARIABLES`: diff --git a/doc/user/application_security/dast_api/index.md b/doc/user/application_security/dast_api/index.md index 839833d9d98..2baafc87d1a 100644 --- a/doc/user/application_security/dast_api/index.md +++ b/doc/user/application_security/dast_api/index.md @@ -391,8 +391,8 @@ By default, the DAST API scanner uses the Postman file to resolve Postman variab is set in a GitLab CI environment variable `DAST_API_POSTMAN_COLLECTION_VARIABLES`, then the JSON file takes precedence to get Postman variable values. -Although Postman can export environment variables into a JSON file, the format is not compatible -with the JSON expected by `DAST_API_POSTMAN_COLLECTION_VARIABLES`. +WARNING: +Although Postman can export environment variables into a JSON file, the format is not compatible with the JSON expected by `DAST_API_POSTMAN_COLLECTION_VARIABLES`. Here is an example of using `DAST_API_POSTMAN_COLLECTION_VARIABLES`: diff --git a/doc/user/project/integrations/harbor.md b/doc/user/project/integrations/harbor.md index d66e2222538..0c0743476fb 100644 --- a/doc/user/project/integrations/harbor.md +++ b/doc/user/project/integrations/harbor.md @@ -6,6 +6,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Harbor container registry integration **(FREE)** +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80999) in GitLab 14.9. + Use Harbor as the container registry for your GitLab project. [Harbor](https://goharbor.io/) is an open source registry that can help you manage artifacts across cloud native compute platforms, like Kubernetes and Docker. diff --git a/qa/qa/fixtures/script_extensions/test.html b/qa/qa/fixtures/script_extensions/test.html new file mode 100644 index 00000000000..0be2c080fd8 --- /dev/null +++ b/qa/qa/fixtures/script_extensions/test.html @@ -0,0 +1,6 @@ + + + +

Hello world

+ + diff --git a/qa/qa/runtime/browser.rb b/qa/qa/runtime/browser.rb index f1d93ce376a..89e84f414b1 100644 --- a/qa/qa/runtime/browser.rb +++ b/qa/qa/runtime/browser.rb @@ -194,6 +194,8 @@ module QA def initialize(instance, page_class) @session_address = Runtime::Address.new(instance, page_class) @page_class = page_class + + Session.enable_interception if Runtime::Env.can_intercept? end def url @@ -255,6 +257,27 @@ module QA @network_conditions_configured = false end + def self.enable_interception + script = File.read("#{__dir__}/script_extensions/interceptor.js") + command = { + cmd: 'Page.addScriptToEvaluateOnNewDocument', + params: { + source: script + } + } + @interceptor_script_params = Capybara.current_session.driver.browser.send(:bridge).send_command(command) + end + + def self.disable_interception + return unless @interceptor_script_params + + command = { + cmd: 'Page.removeScriptToEvaluateOnNewDocument', + params: @interceptor_script_params + } + Capybara.current_session.driver.browser.send(:bridge).send_command(command) + end + private def simulate_slow_connection diff --git a/qa/qa/runtime/env.rb b/qa/qa/runtime/env.rb index 63207751c78..e4537009406 100644 --- a/qa/qa/runtime/env.rb +++ b/qa/qa/runtime/env.rb @@ -37,6 +37,14 @@ module QA ENV['QA_PRAEFECT_REPOSITORY_STORAGE'] end + def interception_enabled? + enabled?(ENV['INTERCEPT_REQUESTS'], default: true) + end + + def can_intercept? + browser == :chrome && interception_enabled? + end + def ci_job_url ENV['CI_JOB_URL'] end diff --git a/qa/qa/runtime/script_extensions/interceptor.js b/qa/qa/runtime/script_extensions/interceptor.js new file mode 100644 index 00000000000..9e98b0421b4 --- /dev/null +++ b/qa/qa/runtime/script_extensions/interceptor.js @@ -0,0 +1,158 @@ +(() => { + const CACHE_NAME = 'INTERCEPTOR_CACHE'; + + /** + * Fetches and parses JSON from the sessionStorage cache + * @returns {(Object)} + */ + const getCache = () => { + return JSON.parse(sessionStorage.getItem(CACHE_NAME)); + }; + + /** + * Commits an object to the sessionStorage cache + * @param {Object} data + */ + const saveCache = (data) => { + sessionStorage.setItem(CACHE_NAME, JSON.stringify(data)); + }; + + /** + * Checks if the cache is available + * and if the current context has access to it + * @returns {boolean} can we access the cache? + */ + const checkCache = () => { + try { + getCache(); + return true; + } catch (error) { + // eslint-disable-next-line no-console + console.warn(`Couldn't access cache: ${error.toString()}`); + return false; + } + }; + + /** + * @callback cacheCommitCallback + * @param {object} cache + * @return {object} mutated cache + */ + + /** + * If the cache is available, takes a callback function that is called + * with an object returned from getCache, + * and saves whatever is returned from the callback function + * to the cache + * @param {cacheCommitCallback} cb + */ + const commitToCache = (cb) => { + if (checkCache()) { + const cache = cb(getCache()); + saveCache(cache); + } + }; + + window.Interceptor = { + saveCache, + commitToCache, + getCache, + checkCache, + activeFetchRequests: 0, + }; + + const pureFetch = window.fetch; + const pureXHROpen = window.XMLHttpRequest.prototype.open; + + /** + * Replacement for XMLHttpRequest.prototype.open + * listens for complete xhr events + * if the xhr response has a status code higher than 400 + * then commit request/response metadata to the cache + * @param method intercepted HTTP method (GET|POST|etc..) + * @param url intercepted HTTP url + * @param args intercepted XHR arguments (credentials, headers, options + * @return {Promise} the result of the original XMLHttpRequest.prototype.open implementation + */ + function interceptXhr(method, url, ...args) { + this.addEventListener( + 'readystatechange', + () => { + const self = this; + if (this.readyState === XMLHttpRequest.DONE) { + if (this.status >= 400 || this.status === 0) { + commitToCache((cache) => { + // eslint-disable-next-line no-param-reassign + cache.errors ||= []; + cache.errors.push({ + status: self.status === 0 ? -1 : self.status, + url, + method, + headers: { 'x-request-id': self.getResponseHeader('x-request-id') }, + }); + return cache; + }); + } + } + }, + false, + ); + return pureXHROpen.apply(this, [method, url, ...args]); + } + + /** + * Replacement for fetch implementation + * tracks active requests, and commits metadata to the cache + * if the response is not ok or was cancelled. + * Additionally tracks activeFetchRequests on the Interceptor object + * @param url target HTTP url + * @param opts fetch options, including request method, body, etc + * @param args additional fetch arguments + * @returns {Promise<"success"|"error">} the result of the original fetch call + */ + async function interceptedFetch(url, opts, ...args) { + const method = opts && opts.method ? opts.method : 'GET'; + window.Interceptor.activeFetchRequests += 1; + try { + const response = await pureFetch(url, opts, ...args); + window.Interceptor.activeFetchRequests += -1; + const clone = response.clone(); + + if (!clone.ok) { + commitToCache((cache) => { + // eslint-disable-next-line no-param-reassign + cache.errors ||= []; + cache.errors.push({ + status: clone.status, + url, + method, + headers: { 'x-request-id': clone.headers.get('x-request-id') }, + }); + return cache; + }); + } + return response; + } catch (error) { + commitToCache((cache) => { + // eslint-disable-next-line no-param-reassign + cache.errors ||= []; + cache.errors.push({ + status: -1, + url, + method, + }); + return cache; + }); + + window.Interceptor.activeFetchRequests += -1; + throw error; + } + } + + if (checkCache()) { + saveCache({}); + } + + window.fetch = interceptedFetch; + window.XMLHttpRequest.prototype.open = interceptXhr; +})(); diff --git a/qa/qa/support/page_error_checker.rb b/qa/qa/support/page_error_checker.rb index 27514f3f45c..192b8c147cd 100644 --- a/qa/qa/support/page_error_checker.rb +++ b/qa/qa/support/page_error_checker.rb @@ -61,6 +61,39 @@ module QA end end + # Log request errors triggered from async api calls from the browser + # + # If any errors are found in the session, log them + # using QA::Runtime::Logger + # @param [Capybara::Session] page + def log_request_errors(page) + return if QA::Runtime::Browser.blank_page? + + url = page.driver.browser.current_url + QA::Runtime::Logger.debug "Fetching API error cache for #{url}" + + cache = page.execute_script <<~JS + return !(typeof(Interceptor)==="undefined") ? Interceptor.getCache() : null; + JS + + return unless cache&.dig('errors') + + grouped_errors = group_errors(cache['errors']) + + errors = grouped_errors.map do |error_metadata, request_id_string| + "#{error_metadata} -- #{request_id_string}" + end + + unless errors.nil? || errors.empty? + QA::Runtime::Logger.error "Interceptor Api Errors\n#{errors.join("\n")}" + end + + # clear the cache after logging the errors + page.execute_script <<~JS + Interceptor && Interceptor.saveCache({}); + JS + end + def error_report_for(logs) logs .map(&:message) @@ -70,6 +103,16 @@ module QA def logs(page) page.driver.browser.manage.logs.get(:browser) end + + private + + def group_errors(errors) + errors.each_with_object({}) do |error, memo| + url = error['url']&.split('?')&.first || 'Unknown url' + key = "[#{error['status']}] #{error['method']} #{url}" + memo[key] = "Correlation Id: #{error.dig('headers', 'x-request-id') || 'Correlation Id not found'}" + end + end end end end diff --git a/qa/qa/support/wait_for_requests.rb b/qa/qa/support/wait_for_requests.rb index 16af4bae521..89674a1d5c6 100644 --- a/qa/qa/support/wait_for_requests.rb +++ b/qa/qa/support/wait_for_requests.rb @@ -16,12 +16,16 @@ module QA Waiter.wait_until(log: false) do finished_all_ajax_requests? && (!skip_finished_loading_check ? finished_loading?(wait: 1) : true) end + QA::Support::PageErrorChecker.log_request_errors(Capybara.page) if QA::Runtime::Env.can_intercept? rescue Repeater::WaitExceededError raise $!, 'Page did not fully load. This could be due to an unending async request or loading icon.' end def finished_all_ajax_requests? - Capybara.page.evaluate_script('window.pendingRequests || window.pendingRailsUJSRequests || 0').zero? # rubocop:disable Style/NumericPredicate + requests = %w[window.pendingRequests window.pendingRailsUJSRequests 0] + requests.unshift('(window.Interceptor && window.Interceptor.activeFetchRequests)') if Runtime::Env.can_intercept? + script = requests.join(' || ') + Capybara.page.evaluate_script(script).zero? # rubocop:disable Style/NumericPredicate end def finished_loading?(wait: DEFAULT_MAX_WAIT_TIME) diff --git a/qa/spec/runtime/script_extensions/interceptor_spec.rb b/qa/spec/runtime/script_extensions/interceptor_spec.rb new file mode 100644 index 00000000000..28a368b2d99 --- /dev/null +++ b/qa/spec/runtime/script_extensions/interceptor_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +RSpec.describe 'Interceptor' do + let(:browser) { Capybara.current_session } + # need a real host for the js runtime + let(:url) { "file://#{__dir__}/../../../qa/fixtures/script_extensions/test.html" } + + before(:context) do + skip 'Only can test for chrome' unless QA::Runtime::Env.can_intercept? + + QA::Runtime::Browser::Session.enable_interception + end + + after(:context) do + QA::Runtime::Browser::Session.disable_interception + end + + before do + browser.visit url + + clear_cache + end + + after do + browser.visit 'about:blank' + end + + context 'with Interceptor' do + context 'caching' do + it 'checks the cache' do + expect(check_cache).to be(true) + end + + it 'returns false if the cache cannot be accessed' do + browser.visit 'about:blank' + + expect(check_cache).to be(false) + end + + it 'gets and sets the cache data' do + commit_to_cache({ foo: 'bar' }) + + expect(get_cache['data']).to eql({ 'foo' => 'bar' }) + end + end + + context 'when intercepting' do + let(:resource_url) { 'chrome://chrome-urls' } + + it 'intercepts fetch errors' do + trigger_fetch(resource_url, 'GET') + + errors = get_cache['errors'] + + expect(errors.size).to be(1) + expect(errors[0]['status']).to be(-1) + expect(errors[0]['method']).to eql('GET') + expect(errors[0]['url']).to eql(resource_url) + end + + it 'intercepts xhr' do + trigger_xhr(resource_url, 'POST') + + errors = get_cache['errors'] + + expect(errors.size).to be(1) + expect(errors[0]['status']).to be(-1) + expect(errors[0]['method']).to eql('POST') + expect(errors[0]['url']).to eql(resource_url) + end + end + end + + def clear_cache + browser.execute_script <<~JS + Interceptor.saveCache({}) + JS + end + + def check_cache + browser.execute_script <<~JS + return Interceptor.checkCache() + JS + end + + def trigger_fetch(url, method) + browser.execute_script <<~JS + (() => { + fetch('#{url}', { method: '#{method}' }) + })() + JS + end + + def trigger_xhr(url, method) + browser.execute_script <<~JS + (() => { + let xhr = new XMLHttpRequest(); + xhr.open('#{method}', '#{url}') + xhr.send() + })() + JS + end + + def commit_to_cache(payload) + browser.execute_script <<~JS + Interceptor.commitToCache((cache) => { + cache.data = JSON.parse('#{payload.to_json}'); + return cache + }) + JS + end + + def get_cache + browser.execute_script <<~JS + return Interceptor.getCache() + JS + end +end diff --git a/qa/spec/support/page_error_checker_spec.rb b/qa/spec/support/page_error_checker_spec.rb index b9b085fa7b9..7c8aaeb182a 100644 --- a/qa/spec/support/page_error_checker_spec.rb +++ b/qa/spec/support/page_error_checker_spec.rb @@ -238,6 +238,88 @@ RSpec.describe QA::Support::PageErrorChecker do end end + describe '::log_request_errors' do + let(:page_url) { 'https://baz.foo' } + let(:browser) { double('browser', current_url: page_url) } + let(:driver) { double('driver', browser: browser) } + let(:session) { double('session', driver: driver) } + + before do + allow(Capybara).to receive(:current_session).and_return(session) + end + + it 'logs from the error cache' do + error = { + 'url' => 'https://foo.bar', + 'status' => 500, + 'method' => 'GET', + 'headers' => { 'x-request-id' => '12345' } + } + + expect(page).to receive(:driver).and_return(driver) + expect(page).to receive(:execute_script).and_return({ 'errors' => [error] }) + expect(page).to receive(:execute_script) + + expect(QA::Runtime::Logger).to receive(:debug).with("Fetching API error cache for #{page_url}") + expect(QA::Runtime::Logger).to receive(:error).with(<<~ERROR.chomp) + Interceptor Api Errors + [500] GET https://foo.bar -- Correlation Id: 12345 + ERROR + + QA::Support::PageErrorChecker.log_request_errors(page) + end + + it 'removes duplicates' do + error = { + 'url' => 'https://foo.bar', + 'status' => 500, + 'method' => 'GET', + 'headers' => { 'x-request-id' => '12345' } + } + expect(page).to receive(:driver).and_return(driver) + expect(page).to receive(:execute_script).and_return({ 'errors' => [error, error, error] }) + expect(page).to receive(:execute_script) + + expect(QA::Runtime::Logger).to receive(:debug).with("Fetching API error cache for #{page_url}") + expect(QA::Runtime::Logger).to receive(:error).with(<<~ERROR.chomp).exactly(1).time + Interceptor Api Errors + [500] GET https://foo.bar -- Correlation Id: 12345 + ERROR + + QA::Support::PageErrorChecker.log_request_errors(page) + end + + it 'chops the url query string' do + error = { + 'url' => 'https://foo.bar?query={ sensitive-data: 12345 }', + 'status' => 500, + 'method' => 'GET', + 'headers' => { 'x-request-id' => '12345' } + } + expect(page).to receive(:driver).and_return(driver) + expect(page).to receive(:execute_script).and_return({ 'errors' => [error] }) + expect(page).to receive(:execute_script) + + expect(QA::Runtime::Logger).to receive(:debug).with("Fetching API error cache for #{page_url}") + expect(QA::Runtime::Logger).to receive(:error).with(<<~ERROR.chomp) + Interceptor Api Errors + [500] GET https://foo.bar -- Correlation Id: 12345 + ERROR + + QA::Support::PageErrorChecker.log_request_errors(page) + end + + it 'returns if cache is nil' do + expect(page).to receive(:driver).and_return(driver) + expect(page).to receive(:execute_script).and_return(nil) + + expect(QA::Runtime::Logger).to receive(:debug).with("Fetching API error cache for #{page_url}") + expect(QA::Runtime::Logger).not_to receive(:error) + + QA::Support::PageErrorChecker.log_request_errors(page) + end + end + describe '.logs' do before do logs_class = Class.new do diff --git a/spec/frontend/incidents/components/incidents_list_spec.js b/spec/frontend/incidents/components/incidents_list_spec.js index 9ed0294e876..5f6f7fc2031 100644 --- a/spec/frontend/incidents/components/incidents_list_spec.js +++ b/spec/frontend/incidents/components/incidents_list_spec.js @@ -7,6 +7,7 @@ import { I18N, TH_CREATED_AT_TEST_ID, TH_SEVERITY_TEST_ID, + TH_ESCALATION_STATUS_TEST_ID, TH_PUBLISHED_TEST_ID, TH_INCIDENT_SLA_TEST_ID, trackIncidentCreateNewOptions, @@ -294,11 +295,12 @@ describe('Incidents List', () => { const noneSort = 'none'; it.each` - description | selector | initialSort | firstSort | nextSort - ${'creation date'} | ${TH_CREATED_AT_TEST_ID} | ${descSort} | ${ascSort} | ${descSort} - ${'severity'} | ${TH_SEVERITY_TEST_ID} | ${noneSort} | ${descSort} | ${ascSort} - ${'publish date'} | ${TH_PUBLISHED_TEST_ID} | ${noneSort} | ${descSort} | ${ascSort} - ${'due date'} | ${TH_INCIDENT_SLA_TEST_ID} | ${noneSort} | ${ascSort} | ${descSort} + description | selector | initialSort | firstSort | nextSort + ${'creation date'} | ${TH_CREATED_AT_TEST_ID} | ${descSort} | ${ascSort} | ${descSort} + ${'severity'} | ${TH_SEVERITY_TEST_ID} | ${noneSort} | ${descSort} | ${ascSort} + ${'status'} | ${TH_ESCALATION_STATUS_TEST_ID} | ${noneSort} | ${descSort} | ${ascSort} + ${'publish date'} | ${TH_PUBLISHED_TEST_ID} | ${noneSort} | ${descSort} | ${ascSort} + ${'due date'} | ${TH_INCIDENT_SLA_TEST_ID} | ${noneSort} | ${ascSort} | ${descSort} `( 'updates sort with new direction when sorting by $description', async ({ selector, initialSort, firstSort, nextSort }) => {