diff --git a/config/initializers/enumerator_next_patch.rb b/config/initializers/enumerator_next_patch.rb new file mode 100644 index 00000000000..e1fc04731ae --- /dev/null +++ b/config/initializers/enumerator_next_patch.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +# Monkey patch of Enumerator#next to get better stack traces +# when an error is raised from within a Fiber. +# https://bugs.ruby-lang.org/issues/16829 +module EnumeratorNextPatch + %w(next next_values peek peek_values).each do |name| + define_method(name) do |*args| + gitlab_patch_backtrace_marker { super(*args) } + rescue Exception => err # rubocop: disable Lint/RescueException + err.set_backtrace(err.backtrace + caller) unless + has_gitlab_patch_backtrace_marker?(err.backtrace) && backtrace_matches_caller?(err.backtrace) + + raise + end + end + + private + + def gitlab_patch_backtrace_marker + yield + end + + # This function tells us whether the exception was generated by #next itself or by something in + # the Fiber that it invokes. If it's generated by #next, then the backtrace will have + # #gitlab_patch_backtrace_marker as the third item down the trace (since + # #gitlab_patch_backtrace_marker calls a block, which in turn calls #next.) If it's generated + # by the Fiber that #next invokes, then it won't contain this marker. + def has_gitlab_patch_backtrace_marker?(backtrace) + match = %r(^(.*):[0-9]+:in `gitlab_patch_backtrace_marker'$).match(backtrace[2]) + + !!match && match[1] == __FILE__ + end + + # This function makes sure that the rest of the stack trace matches in order to avoid missing + # an exception that was generated by calling #next on another Enumerator inside the Fiber. + # This might miss some *very* contrived scenarios involving recursion, but exceptions don't + # provide Fiber information, so it's the best we can do. + def backtrace_matches_caller?(backtrace) + backtrace[3..] == caller[1..] + end +end + +Enumerator.prepend(EnumeratorNextPatch) diff --git a/db/migrate/20220523162734_add_vulnerability_reads_casted_cluster_agent_column.rb b/db/migrate/20220523162734_add_vulnerability_reads_casted_cluster_agent_column.rb new file mode 100644 index 00000000000..ce222906e25 --- /dev/null +++ b/db/migrate/20220523162734_add_vulnerability_reads_casted_cluster_agent_column.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddVulnerabilityReadsCastedClusterAgentColumn < Gitlab::Database::Migration[2.0] + enable_lock_retries! + + def change + add_column :vulnerability_reads, :casted_cluster_agent_id, :bigint + end +end diff --git a/db/migrate/20220523163734_update_vulnerability_reads_trigger_functions.rb b/db/migrate/20220523163734_update_vulnerability_reads_trigger_functions.rb new file mode 100644 index 00000000000..645d19c8f51 --- /dev/null +++ b/db/migrate/20220523163734_update_vulnerability_reads_trigger_functions.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +class UpdateVulnerabilityReadsTriggerFunctions < Gitlab::Database::Migration[2.0] + AGENT_ID_VALUE = "NEW.location->'kubernetes_resource'->>'agent_id'" + CASTED_AGENT_ID_VALUE = "CAST(#{AGENT_ID_VALUE} AS bigint)" + + def up + update_insert_or_update_vulnerability_reads_function(with_casted_cluster_agent_id: true) + update_update_location_from_vulnerability_occurrences_function(with_casted_cluster_agent_id: true) + end + + def down + update_insert_or_update_vulnerability_reads_function(with_casted_cluster_agent_id: false) + update_update_location_from_vulnerability_occurrences_function(with_casted_cluster_agent_id: false) + end + + private + + def update_insert_or_update_vulnerability_reads_function(with_casted_cluster_agent_id: false) + insert_fields = with_casted_cluster_agent_id ? 'cluster_agent_id, casted_cluster_agent_id' : 'cluster_agent_id' + insert_values = with_casted_cluster_agent_id ? [AGENT_ID_VALUE, CASTED_AGENT_ID_VALUE].join(', ') : AGENT_ID_VALUE + + execute(<<~SQL) + CREATE OR REPLACE FUNCTION insert_or_update_vulnerability_reads() + RETURNS TRIGGER + LANGUAGE plpgsql + AS $$ + DECLARE + severity smallint; + state smallint; + report_type smallint; + resolved_on_default_branch boolean; + BEGIN + IF (NEW.vulnerability_id IS NULL AND (TG_OP = 'INSERT' OR TG_OP = 'UPDATE')) THEN + RETURN NULL; + END IF; + + IF (TG_OP = 'UPDATE' AND OLD.vulnerability_id IS NOT NULL AND NEW.vulnerability_id IS NOT NULL) THEN + RETURN NULL; + END IF; + + SELECT + vulnerabilities.severity, vulnerabilities.state, vulnerabilities.report_type, vulnerabilities.resolved_on_default_branch + INTO + severity, state, report_type, resolved_on_default_branch + FROM + vulnerabilities + WHERE + vulnerabilities.id = NEW.vulnerability_id; + + INSERT INTO vulnerability_reads (vulnerability_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, #{insert_fields}) + VALUES (NEW.vulnerability_id, NEW.project_id, NEW.scanner_id, report_type, severity, state, resolved_on_default_branch, NEW.uuid::uuid, NEW.location->>'image', #{insert_values}) + ON CONFLICT(vulnerability_id) DO NOTHING; + RETURN NULL; + END + $$; + SQL + end + + def update_update_location_from_vulnerability_occurrences_function(with_casted_cluster_agent_id: false) + execute(<<~SQL) + CREATE OR REPLACE FUNCTION update_location_from_vulnerability_occurrences() + RETURNS trigger + LANGUAGE plpgsql + AS $$ + BEGIN + UPDATE + vulnerability_reads + SET + location_image = NEW.location->>'image', + #{with_casted_cluster_agent_id ? "casted_cluster_agent_id = #{CASTED_AGENT_ID_VALUE}," : ''} + cluster_agent_id = #{AGENT_ID_VALUE} + WHERE + vulnerability_id = NEW.vulnerability_id; + RETURN NULL; + + END + $$; + SQL + end +end diff --git a/db/post_migrate/20220523164734_add_foreign_key_to_vulnerability_reads_casted_cluster_agent_id.rb b/db/post_migrate/20220523164734_add_foreign_key_to_vulnerability_reads_casted_cluster_agent_id.rb new file mode 100644 index 00000000000..364570973ab --- /dev/null +++ b/db/post_migrate/20220523164734_add_foreign_key_to_vulnerability_reads_casted_cluster_agent_id.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddForeignKeyToVulnerabilityReadsCastedClusterAgentId < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :vulnerability_reads, :cluster_agents, + column: :casted_cluster_agent_id, on_delete: :nullify + end + + def down + with_lock_retries do + remove_foreign_key :vulnerability_reads, :cluster_agents, column: :casted_cluster_agent_id + end + end +end diff --git a/db/post_migrate/20220523165734_add_index_to_vulnerability_reads_casted_cluster_agent_id.rb b/db/post_migrate/20220523165734_add_index_to_vulnerability_reads_casted_cluster_agent_id.rb new file mode 100644 index 00000000000..51842af3f12 --- /dev/null +++ b/db/post_migrate/20220523165734_add_index_to_vulnerability_reads_casted_cluster_agent_id.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddIndexToVulnerabilityReadsCastedClusterAgentId < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + INDEX_NAME = 'index_cis_vulnerability_reads_on_cluster_agent_id' + + def up + add_concurrent_index :vulnerability_reads, :casted_cluster_agent_id, name: INDEX_NAME, where: 'report_type = 7' + end + + def down + remove_concurrent_index_by_name :vulnerability_reads, INDEX_NAME + end +end diff --git a/db/schema_migrations/20220523162734 b/db/schema_migrations/20220523162734 new file mode 100644 index 00000000000..dd319042965 --- /dev/null +++ b/db/schema_migrations/20220523162734 @@ -0,0 +1 @@ +af6f954426b714649a3b19e80a20cf99475cdc8496c23add8032cda27072d31d \ No newline at end of file diff --git a/db/schema_migrations/20220523163734 b/db/schema_migrations/20220523163734 new file mode 100644 index 00000000000..622ea4abb95 --- /dev/null +++ b/db/schema_migrations/20220523163734 @@ -0,0 +1 @@ +02821039feda457c1fb61dc6ff62756752c0c1c0ad01e12ecf28c265462529d4 \ No newline at end of file diff --git a/db/schema_migrations/20220523164734 b/db/schema_migrations/20220523164734 new file mode 100644 index 00000000000..3538f55712d --- /dev/null +++ b/db/schema_migrations/20220523164734 @@ -0,0 +1 @@ +ef078bbcf8415a7bb49ed919739005d22c21199da1a0e5e5c0971d2a8e1b2a40 \ No newline at end of file diff --git a/db/schema_migrations/20220523165734 b/db/schema_migrations/20220523165734 new file mode 100644 index 00000000000..b1d8fa16dd5 --- /dev/null +++ b/db/schema_migrations/20220523165734 @@ -0,0 +1 @@ +89a03d69c44ed95133446275bb9b39dfe91ad3022cefdfa438ea3c96ab4f8e69 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 2bdc8d52f0c..db8481eae64 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -71,8 +71,8 @@ BEGIN WHERE vulnerabilities.id = NEW.vulnerability_id; - INSERT INTO vulnerability_reads (vulnerability_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id) - VALUES (NEW.vulnerability_id, NEW.project_id, NEW.scanner_id, report_type, severity, state, resolved_on_default_branch, NEW.uuid::uuid, NEW.location->>'image', NEW.location->'kubernetes_resource'->>'agent_id') + INSERT INTO vulnerability_reads (vulnerability_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id) + VALUES (NEW.vulnerability_id, NEW.project_id, NEW.scanner_id, report_type, severity, state, resolved_on_default_branch, NEW.uuid::uuid, NEW.location->>'image', NEW.location->'kubernetes_resource'->>'agent_id', CAST(NEW.location->'kubernetes_resource'->>'agent_id' AS bigint)) ON CONFLICT(vulnerability_id) DO NOTHING; RETURN NULL; END @@ -206,6 +206,7 @@ UPDATE vulnerability_reads SET location_image = NEW.location->>'image', + casted_cluster_agent_id = CAST(NEW.location->'kubernetes_resource'->>'agent_id' AS bigint), cluster_agent_id = NEW.location->'kubernetes_resource'->>'agent_id' WHERE vulnerability_id = NEW.vulnerability_id; @@ -22211,6 +22212,7 @@ CREATE TABLE vulnerability_reads ( uuid uuid NOT NULL, location_image text, cluster_agent_id text, + casted_cluster_agent_id bigint, CONSTRAINT check_380451bdbe CHECK ((char_length(location_image) <= 2048)), CONSTRAINT check_a105eb825a CHECK ((char_length(cluster_agent_id) <= 10)) ); @@ -27655,6 +27657,8 @@ CREATE UNIQUE INDEX index_ci_variables_on_project_id_and_key_and_environment_sco CREATE INDEX index_cicd_settings_on_namespace_id_where_stale_pruning_enabled ON namespace_ci_cd_settings USING btree (namespace_id) WHERE (allow_stale_runner_pruning = true); +CREATE INDEX index_cis_vulnerability_reads_on_cluster_agent_id ON vulnerability_reads USING btree (casted_cluster_agent_id) WHERE (report_type = 7); + CREATE INDEX index_cluster_agent_tokens_on_agent_id_status_last_used_at ON cluster_agent_tokens USING btree (agent_id, status, last_used_at DESC NULLS LAST); CREATE INDEX index_cluster_agent_tokens_on_created_by_user_id ON cluster_agent_tokens USING btree (created_by_user_id); @@ -32030,6 +32034,9 @@ ALTER TABLE ONLY merge_requests ALTER TABLE ONLY merge_request_metrics ADD CONSTRAINT fk_ae440388cc FOREIGN KEY (latest_closed_by_id) REFERENCES users(id) ON DELETE SET NULL; +ALTER TABLE ONLY vulnerability_reads + ADD CONSTRAINT fk_aee839e611 FOREIGN KEY (casted_cluster_agent_id) REFERENCES cluster_agents(id) ON DELETE SET NULL; + ALTER TABLE ONLY dast_profile_schedules ADD CONSTRAINT fk_aef03d62e5 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 181dcc8f00b..9de564449ef 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -16719,8 +16719,10 @@ Represents the scan result policy. | ---- | ---- | ----------- | | `description` | [`String!`](#string) | Description of the policy. | | `enabled` | [`Boolean!`](#boolean) | Indicates whether this policy is enabled. | +| `groupApprovers` | [`[Group!]`](#group) | Approvers of the group type. | | `name` | [`String!`](#string) | Name of the policy. | | `updatedAt` | [`Time!`](#time) | Timestamp of when the policy YAML was last updated. | +| `userApprovers` | [`[UserCore!]`](#usercore) | Approvers of the user type. | | `yaml` | [`String!`](#string) | YAML definition of the policy. | ### `ScannedResource` diff --git a/qa/qa/runtime/browser.rb b/qa/qa/runtime/browser.rb index 03178661826..f705fe54b97 100644 --- a/qa/qa/runtime/browser.rb +++ b/qa/qa/runtime/browser.rb @@ -16,7 +16,7 @@ module QA NotRespondingError = Class.new(RuntimeError) - CAPYBARA_MAX_WAIT_TIME = 10 + CAPYBARA_MAX_WAIT_TIME = Env.max_capybara_wait_time def self.blank_page? ['', 'about:blank', 'data:,'].include?(Capybara.current_session.driver.browser.current_url) diff --git a/qa/qa/runtime/env.rb b/qa/qa/runtime/env.rb index dd0713bc2e8..1b27c822c31 100644 --- a/qa/qa/runtime/env.rb +++ b/qa/qa/runtime/env.rb @@ -461,6 +461,10 @@ module QA enabled?(ENV['QA_SKIP_SMOKE_RELIABLE'], default: false) end + def max_capybara_wait_time + ENV.fetch('MAX_CAPYBARA_WAIT_TIME', 10).to_i + end + private def remote_grid_credentials diff --git a/spec/initializers/enumerator_next_patch_spec.rb b/spec/initializers/enumerator_next_patch_spec.rb new file mode 100644 index 00000000000..99e73af5e86 --- /dev/null +++ b/spec/initializers/enumerator_next_patch_spec.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Enumerator#next patch fix' do + describe 'Enumerator' do + RSpec::Matchers.define :contain_unique_method_calls_in_order do |expected| + attr_reader :actual + + match do |actual| + @actual_err = actual + regexps = expected.map { |method_name| { name: method_name, regexp: make_regexp(method_name) } } + @actual = actual.backtrace.filter_map do |line| + regexp = regexps.find { |r| r[:regexp].match? line } + + regexp[:name] if regexp + end + + expected == @actual + end + + diffable + + failure_message do + "#{super()}\n\nFull error backtrace:\n #{@actual_err.backtrace.join("\n ")}" + end + + private + + def make_regexp(method_name) + Regexp.new("/spec/initializers/enumerator_next_patch_spec\\.rb:[0-9]+:in `#{method_name}'$") + end + end + + def have_been_raised_by_next_and_not_fixed_up + contain_unique_method_calls_in_order %w(call_enum_method) + end + + def have_been_raised_by_enum_object_and_fixed_up + contain_unique_method_calls_in_order %w(make_error call_enum_method) + end + + def have_been_raised_by_nested_next_and_fixed_up + contain_unique_method_calls_in_order %w(call_nested_next call_enum_method) + end + + methods = [ + { + name: 'next', + expected_value: 'Test value' + }, + { + name: 'next_values', + expected_value: ['Test value'] + }, + { + name: 'peek', + expected_value: 'Test value' + }, + { + name: 'peek_values', + expected_value: ['Test value'] + } + ] + + methods.each do |method| + describe "##{method[:name]}" do + def call_enum_method + enumerator.send(method_name) + end + + let(:method_name) { method[:name] } + + subject { call_enum_method } + + describe 'normal yield' do + let(:enumerator) { Enumerator.new { |yielder| yielder << 'Test value' } } + + it 'returns yielded value' do + is_expected.to eq(method[:expected_value]) + end + end + + describe 'end of iteration' do + let(:enumerator) { Enumerator.new { |_| } } + + it 'does not fix up StopIteration' do + expect { subject }.to raise_error do |err| + expect(err).to be_a(StopIteration) + expect(err).to have_been_raised_by_next_and_not_fixed_up + end + end + + context 'nested enum object' do + def call_nested_next + nested_enumerator.next + end + + let(:nested_enumerator) { Enumerator.new { |_| } } + let(:enumerator) { Enumerator.new { |yielder| yielder << call_nested_next } } + + it 'fixes up StopIteration thrown by another instance of #next' do + expect { subject }.to raise_error do |err| + expect(err).to be_a(StopIteration) + expect(err).to have_been_raised_by_nested_next_and_fixed_up + end + end + end + end + + describe 'arguments error' do + def call_enum_method + enumerator.send(method_name, 'extra argument') + end + + let(:enumerator) { Enumerator.new { |_| } } + + it 'does not fix up ArgumentError' do + expect { subject }.to raise_error do |err| + expect(err).to be_a(ArgumentError) + expect(err).to have_been_raised_by_next_and_not_fixed_up + end + end + end + + describe 'error' do + let(:enumerator) { Enumerator.new { |_| raise error } } + let(:error) { make_error } + + it 'fixes up StopIteration' do + def make_error + StopIteration.new.tap { |err| err.set_backtrace(caller) } + end + + expect { subject }.to raise_error do |err| + expect(err).to be(error) + expect(err).to have_been_raised_by_enum_object_and_fixed_up + end + end + + it 'fixes up ArgumentError' do + def make_error + ArgumentError.new.tap { |err| err.set_backtrace(caller) } + end + + expect { subject }.to raise_error do |err| + expect(err).to be(error) + expect(err).to have_been_raised_by_enum_object_and_fixed_up + end + end + + it 'adds backtrace from other errors' do + def make_error + StandardError.new('This is a test').tap { |err| err.set_backtrace(caller) } + end + + expect { subject }.to raise_error do |err| + expect(err).to be(error) + expect(err).to have_been_raised_by_enum_object_and_fixed_up + expect(err.message).to eq('This is a test') + end + end + end + end + end + end +end