diff options
Diffstat (limited to 'spec')
22 files changed, 390 insertions, 742 deletions
diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index d04cd20f4e6..35bfb829bf7 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -102,39 +102,6 @@ RSpec.describe Admin::ClustersController, feature_category: :deployment_manageme end end - it_behaves_like 'GET #metrics_dashboard for dashboard', 'Cluster health' do - let(:cluster) { create(:cluster, :instance, :provided_by_gcp) } - - let(:metrics_dashboard_req_params) do - { - id: cluster.id - } - end - end - - describe 'GET #prometheus_proxy' do - let(:user) { admin } - let(:proxyable) do - create(:cluster, :instance, :provided_by_gcp) - end - - it_behaves_like 'metrics dashboard prometheus api proxy' do - context 'with anonymous user' do - let(:prometheus_body) { nil } - - before do - sign_out(admin) - end - - it 'returns 404' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - end - describe 'POST #create_user' do let(:params) do { @@ -283,41 +250,6 @@ RSpec.describe Admin::ClustersController, feature_category: :deployment_manageme let(:subject) { get_show } end - describe 'functionality' do - context 'when remove_monitor_metrics FF is disabled' do - before do - stub_feature_flags(remove_monitor_metrics: false) - end - - render_views - - it 'responds successfully' do - get_show - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:cluster)).to eq(cluster) - end - - it 'renders integration tab view' do - get_show(tab: 'integrations') - - expect(response).to render_template('clusters/clusters/_integrations') - expect(response).to have_gitlab_http_status(:ok) - end - end - - context 'when remove_monitor_metrics FF is enabled' do - render_views - - it 'renders details tab view' do - get_show(tab: 'integrations') - - expect(response).to render_template('clusters/clusters/_details') - expect(response).to have_gitlab_http_status(:ok) - end - end - end - describe 'security' do it { expect { get_show }.to be_allowed_for(:admin) } it { expect { get_show }.to be_denied_for(:user) } diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index f36494c3d78..6c747d4f00f 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -115,46 +115,6 @@ RSpec.describe Groups::ClustersController, feature_category: :deployment_managem end end - it_behaves_like 'GET #metrics_dashboard for dashboard', 'Cluster health' do - let(:cluster) { create(:cluster, :provided_by_gcp, cluster_type: :group_type, groups: [group]) } - - let(:metrics_dashboard_req_params) do - { - id: cluster.id, - group_id: group.name - } - end - end - - describe 'GET #prometheus_proxy' do - let(:proxyable) do - create(:cluster, :provided_by_gcp, cluster_type: :group_type, groups: [group]) - end - - it_behaves_like 'metrics dashboard prometheus api proxy' do - let(:proxyable_params) do - { - id: proxyable.id.to_s, - group_id: group.name - } - end - - context 'with anonymous user' do - let(:prometheus_body) { nil } - - before do - sign_out(user) - end - - it 'returns 404' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - end - describe 'POST create for existing cluster' do let(:params) do { @@ -353,41 +313,6 @@ RSpec.describe Groups::ClustersController, feature_category: :deployment_managem let(:subject) { go } end - describe 'functionality' do - context 'when remove_monitor_metrics FF is disabled' do - before do - stub_feature_flags(remove_monitor_metrics: false) - end - - render_views - - it 'renders view' do - go - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:cluster)).to eq(cluster) - end - - it 'renders integration tab view', :aggregate_failures do - go(tab: 'integrations') - - expect(response).to render_template('clusters/clusters/_integrations') - expect(response).to have_gitlab_http_status(:ok) - end - end - - context 'when remove_monitor_metrics FF is enabled' do - render_views - - it 'renders details tab view', :aggregate_failures do - go(tab: 'integrations') - - expect(response).to render_template('clusters/clusters/_details') - expect(response).to have_gitlab_http_status(:ok) - end - end - end - describe 'security' do it('is allowed for admin when admin mode is enabled', :enable_admin_mode) { expect { go }.to be_allowed_for(:admin) } it('is denied for admin when admin mode is disabled') { expect { go }.to be_denied_for(:admin) } diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index f976b5bfe67..bface886674 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -113,57 +113,6 @@ RSpec.describe Projects::ClustersController, feature_category: :deployment_manag end end - describe 'GET #prometheus_proxy' do - let(:proxyable) do - create(:cluster, :provided_by_gcp, projects: [project]) - end - - it_behaves_like 'metrics dashboard prometheus api proxy' do - let(:proxyable_params) do - { - id: proxyable.id.to_s, - namespace_id: project.namespace.full_path, - project_id: project.path - } - end - - context 'with anonymous user' do - let(:prometheus_body) { nil } - - before do - sign_out(user) - end - - it 'redirects to signin page' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to redirect_to(new_user_session_path) - end - end - - context 'with a public project' do - before do - project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::ENABLED) - end - - context 'with guest user' do - let(:prometheus_body) { nil } - - before do - project.add_guest(user) - end - - it 'returns 404' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - end - end - it_behaves_like 'GET #metrics_dashboard for dashboard', 'Cluster health' do let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } @@ -396,41 +345,6 @@ RSpec.describe Projects::ClustersController, feature_category: :deployment_manag let(:subject) { go } end - describe 'functionality' do - context 'when remove_monitor_metrics FF is disabled' do - before do - stub_feature_flags(remove_monitor_metrics: false) - end - - render_views - - it "renders view" do - go - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:cluster)).to eq(cluster) - end - - it 'renders integration tab view' do - go(tab: 'integrations') - - expect(response).to render_template('clusters/clusters/_integrations') - expect(response).to have_gitlab_http_status(:ok) - end - end - - context 'when remove_monitor_metrics FF is enabled' do - render_views - - it 'renders details tab view', :aggregate_failures do - go(tab: 'integrations') - - expect(response).to render_template('clusters/clusters/_details') - expect(response).to have_gitlab_http_status(:ok) - end - end - end - describe 'security' do it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb deleted file mode 100644 index ef2d743c82f..00000000000 --- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb +++ /dev/null @@ -1,96 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::Environments::PrometheusApiController do - let_it_be(:user) { create(:user) } - let_it_be_with_reload(:project) { create(:project) } - let_it_be(:proxyable) { create(:environment, project: project) } - - before do - project.add_reporter(user) - sign_in(user) - end - - describe 'GET #prometheus_proxy' do - it_behaves_like 'metrics dashboard prometheus api proxy' do - let(:proxyable_params) do - { - id: proxyable.id.to_s, - namespace_id: project.namespace.full_path, - project_id: project.path - } - end - - context 'with variables' do - let(:prometheus_body) { '{"status":"success"}' } - let(:pod_name) { "pod1" } - - before do - expected_params[:query] = %{up{pod_name="#{pod_name}"}} - expected_params[:variables] = { 'pod_name' => pod_name } - end - - it 'replaces variables with values' do - get :prometheus_proxy, params: prometheus_proxy_params.merge( - query: 'up{pod_name="{{pod_name}}"}', variables: { 'pod_name' => pod_name } - ) - - expect(response).to have_gitlab_http_status(:success) - expect(Prometheus::ProxyService).to have_received(:new) - .with(proxyable, 'GET', 'query', expected_params) - end - - context 'with invalid variables' do - let(:params_with_invalid_variables) do - prometheus_proxy_params.merge( - query: 'up{pod_name="{{pod_name}}"}', variables: ['a'] - ) - end - - it 'returns 400' do - get :prometheus_proxy, params: params_with_invalid_variables - - expect(response).to have_gitlab_http_status(:bad_request) - expect(Prometheus::ProxyService).not_to receive(:new) - end - end - end - - context 'with anonymous user' do - let(:prometheus_body) { nil } - - before do - sign_out(user) - end - - it 'redirects to signin page' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to redirect_to(new_user_session_path) - end - end - - context 'with a public project' do - before do - project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::ENABLED) - end - - context 'with guest user' do - let(:prometheus_body) { nil } - - before do - project.add_guest(user) - end - - it 'returns 404' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - end - end -end diff --git a/spec/controllers/projects/prometheus/alerts_controller_spec.rb b/spec/controllers/projects/prometheus/alerts_controller_spec.rb index 44292b9ce19..3e64631fbf1 100644 --- a/spec/controllers/projects/prometheus/alerts_controller_spec.rb +++ b/spec/controllers/projects/prometheus/alerts_controller_spec.rb @@ -6,7 +6,6 @@ RSpec.describe Projects::Prometheus::AlertsController, feature_category: :incide let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project) } let_it_be(:environment) { create(:environment, project: project) } - let_it_be(:metric) { create(:prometheus_metric, project: project) } before do project.add_maintainer(user) @@ -43,16 +42,6 @@ RSpec.describe Projects::Prometheus::AlertsController, feature_category: :incide end end - shared_examples 'project non-specific metric' do |status| - let(:other) { create(:prometheus_alert) } - - it "returns #{status}" do - make_request(id: other.prometheus_metric_id) - - expect(response).to have_gitlab_http_status(status) - end - end - describe 'POST #notify' do let(:alert_1) { build(:alert_management_alert, :prometheus, project: project) } let(:alert_2) { build(:alert_management_alert, :prometheus, project: project) } @@ -115,67 +104,7 @@ RSpec.describe Projects::Prometheus::AlertsController, feature_category: :incide end end - describe 'GET #metrics_dashboard', feature_category: :metrics do - let!(:alert) do - create(:prometheus_alert, project: project, environment: environment, prometheus_metric: metric) - end - - before do - stub_feature_flags(remove_monitor_metrics: false) - end - - it 'returns a json object with the correct keys' do - get :metrics_dashboard, params: request_params(id: metric.id, environment_id: alert.environment.id), format: :json - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.keys).to contain_exactly('dashboard', 'status', 'metrics_data') - end - - it 'is the correct embed' do - get :metrics_dashboard, params: request_params(id: metric.id, environment_id: alert.environment.id), format: :json - - title = json_response['dashboard']['panel_groups'][0]['panels'][0]['title'] - - expect(title).to eq(metric.title) - end - - it 'finds the first alert embed without environment_id' do - get :metrics_dashboard, params: request_params(id: metric.id), format: :json - - title = json_response['dashboard']['panel_groups'][0]['panels'][0]['title'] - - expect(title).to eq(metric.title) - end - - it 'returns 404 for non-existant alerts' do - get :metrics_dashboard, params: request_params(id: 0), format: :json - - expect(response).to have_gitlab_http_status(:not_found) - end - - it 'returns 404 when metrics dashboard feature is unavailable' do - stub_feature_flags(remove_monitor_metrics: true) - - get :metrics_dashboard, params: request_params(id: 0), format: :json - - expect(response).to have_gitlab_http_status(:not_found) - end - end - def project_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project) end - - def request_params(opts = {}, defaults = {}) - project_params(opts.reverse_merge(defaults)) - end - - def alert_path(alert) - project_prometheus_alert_path( - project, - alert.prometheus_metric_id, - environment_id: alert.environment, - format: :json - ) - end end diff --git a/spec/factories/merge_request_diffs.rb b/spec/factories/merge_request_diffs.rb index f93f3f22109..d81f355737e 100644 --- a/spec/factories/merge_request_diffs.rb +++ b/spec/factories/merge_request_diffs.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :merge_request_diff do - association :merge_request, factory: :merge_request_without_merge_request_diff + association :merge_request, :skip_diff_creation state { :collected } commits_count { 1 } diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 4941a31982f..390db24dde8 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -313,6 +313,12 @@ FactoryBot.define do sequence(:source_branch) { |n| "feature#{n}" } end + trait :skip_diff_creation do + before(:create) do |merge_request, _| + merge_request.skip_ensure_merge_request_diff = true + end + end + after(:build) do |merge_request| target_project = merge_request.target_project source_project = merge_request.source_project @@ -357,7 +363,5 @@ FactoryBot.define do merge_request.update!(labels: evaluator.labels) end end - - factory :merge_request_without_merge_request_diff, class: 'MergeRequestWithoutMergeRequestDiff' end end diff --git a/spec/features/clusters/cluster_health_dashboard_spec.rb b/spec/features/clusters/cluster_health_dashboard_spec.rb deleted file mode 100644 index e932f8c6b98..00000000000 --- a/spec/features/clusters/cluster_health_dashboard_spec.rb +++ /dev/null @@ -1,126 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Cluster Health board', :js, :kubeclient, :use_clean_rails_memory_store_caching, :sidekiq_inline, -feature_category: :deployment_management do - include KubernetesHelpers - include PrometheusHelpers - - let_it_be(:current_user) { create(:user) } - let_it_be(:clusterable) { create(:project) } - let_it_be(:cluster) { create(:cluster, :provided_by_gcp, :project, projects: [clusterable]) } - let_it_be(:cluster_path) { project_cluster_path(clusterable, cluster) } - - before do - stub_feature_flags(remove_monitor_metrics: false) - - clusterable.add_maintainer(current_user) - - sign_in(current_user) - end - - it 'shows cluster board section within the page' do - visit cluster_path - - expect(page).to have_text('Health') - - click_link 'Health' - - expect(page).to have_css('.cluster-health-graphs') - end - - context 'feature remove_monitor_metrics enabled' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - - it 'does not show the cluster health tab' do - visit cluster_path - - expect(page).not_to have_text('Health') - end - - it 'does not show the cluster health section' do - visit project_cluster_path(clusterable, cluster, { tab: 'health' }) - - expect(page).not_to have_text('you must first enable Prometheus in the Integrations tab') - end - end - - context 'no prometheus available' do - it 'shows enable Prometheus message' do - visit cluster_path - - click_link 'Health' - - expect(page).to have_text('you must first enable Prometheus in the Integrations tab') - end - end - - context 'when there is cluster with enabled prometheus' do - before do - create(:clusters_integrations_prometheus, enabled: true, cluster: cluster) - stub_kubeclient_discover(cluster.platform.api_url) - end - - context 'waiting for data' do - before do - stub_empty_response - end - - it 'shows container and waiting for data message' do - visit cluster_path - - click_link 'Health' - - wait_for_requests - - expect(page).to have_css('.prometheus-graphs') - expect(page).to have_text('Waiting for performance data') - end - end - - context 'connected, prometheus returns data' do - before do - stub_connected - - visit cluster_path - - click_link 'Health' - - wait_for_requests - end - - it 'renders charts' do - expect(page).to have_css('.prometheus-graphs') - expect(page).to have_css('.prometheus-graph') - expect(page).to have_css('.prometheus-graph-title') - expect(page).to have_css('[_echarts_instance_]') - expect(page).to have_css('.prometheus-graph', count: 2) - expect(page).to have_content('Avg') - end - - it 'focuses the single panel on toggle' do - click_button('More actions', match: :first) - click_button('Expand panel') - - expect(page).to have_css('.prometheus-graph', count: 1) - - click_button('Collapse panel') - - expect(page).to have_css('.prometheus-graph', count: 2) - end - end - - def stub_empty_response - stub_prometheus_request(/prometheus-prometheus-server/, status: 204, body: {}) - stub_prometheus_request(%r{prometheus/api/v1}, status: 204, body: {}) - end - - def stub_connected - stub_prometheus_request(/prometheus-prometheus-server/, body: prometheus_values_body) - stub_prometheus_request(%r{prometheus/api/v1}, body: prometheus_values_body) - end - end -end diff --git a/spec/lib/generators/gitlab/analytics/internal_events_generator_spec.rb b/spec/lib/generators/gitlab/analytics/internal_events_generator_spec.rb index 38992a29dcb..0afd3201853 100644 --- a/spec/lib/generators/gitlab/analytics/internal_events_generator_spec.rb +++ b/spec/lib/generators/gitlab/analytics/internal_events_generator_spec.rb @@ -6,8 +6,8 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat include UsageDataHelpers let(:temp_dir) { Dir.mktmpdir } - let(:tmpfile) { Tempfile.new('test-metadata') } let(:ee_temp_dir) { Dir.mktmpdir } + let(:tmpfile) { Tempfile.new('test-metadata') } let(:existing_key_paths) { {} } let(:description) { "This metric counts unique users viewing analytics metrics dashboard section" } let(:group) { "group::analytics instrumentation" } @@ -16,6 +16,8 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat let(:mr) { "https://gitlab.com/some-group/some-project/-/merge_requests/123" } let(:event) { "view_analytics_dashboard" } let(:unique_on) { "user_id" } + let(:time_frames) { %w[7d] } + let(:include_default_identifiers) { 'yes' } let(:options) do { time_frames: time_frames, @@ -34,7 +36,6 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat let(:metric_definition_7d) do { "key_path" => key_path_7d, - "name" => key_path_7d, "description" => description, "product_section" => section, "product_stage" => stage, @@ -54,10 +55,12 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat end before do - stub_const("#{described_class}::TOP_LEVEL_DIR", temp_dir) stub_const("#{described_class}::TOP_LEVEL_DIR_EE", ee_temp_dir) + stub_const("#{described_class}::TOP_LEVEL_DIR", temp_dir) stub_const("#{described_class}::KNOWN_EVENTS_PATH", tmpfile.path) stub_const("#{described_class}::KNOWN_EVENTS_PATH_EE", tmpfile.path) + # Stub version so that `milestone` key remains constant between releases to prevent flakiness. + stub_const('Gitlab::VERSION', '13.9.0') allow_next_instance_of(described_class) do |instance| allow(instance).to receive(:ask) @@ -65,8 +68,8 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat .and_return(description) end - allow(Gitlab::Usage::MetricDefinition) - .to receive(:definitions).and_return(existing_key_paths) + allow(Gitlab::TaskHelpers).to receive(:prompt).and_return(include_default_identifiers) + allow(Gitlab::Usage::MetricDefinition).to receive(:definitions).and_return(existing_key_paths) end after do @@ -75,12 +78,85 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat FileUtils.rm_rf(tmpfile.path) end - describe 'Creating metric definition file' do - before do - # Stub version so that `milestone` key remains constant between releases to prevent flakiness. - stub_const('Gitlab::VERSION', '13.9.0') + describe 'Creating event definition file' do + let(:event_definition_path) { Dir.glob(File.join(temp_dir, "events/#{event}.yml")).first } + let(:identifiers) { %w[project user namespace] } + let(:event_definition) do + { + "category" => "GitlabInternalEvents", + "action" => event, + "description" => description, + "product_section" => section, + "product_stage" => stage, + "product_group" => group, + "label_description" => nil, + "property_description" => nil, + "value_description" => nil, + "extra_properties" => nil, + "identifiers" => identifiers, + "milestone" => "13.9", + "introduced_by_url" => mr, + "distributions" => %w[ce ee], + "tiers" => %w[free premium ultimate] + } end + it 'creates an event definition file using the template' do + described_class.new([], options).invoke_all + + expect(YAML.safe_load(File.read(event_definition_path))).to eq(event_definition) + end + + context 'for ultimate only feature' do + let(:event_definition_path) do + Dir.glob(File.join(ee_temp_dir, temp_dir, "events/#{event}.yml")).first + end + + it 'creates an event definition file using the template' do + described_class.new([], options.merge(tiers: %w[ultimate])).invoke_all + + expect(YAML.safe_load(File.read(event_definition_path))) + .to eq(event_definition.merge("tiers" => ["ultimate"], "distributions" => ["ee"])) + end + end + + context 'without default identifiers' do + let(:include_default_identifiers) { 'no' } + + it 'creates an event definition file using the template' do + described_class.new([], options).invoke_all + + expect(YAML.safe_load(File.read(event_definition_path))) + .to eq(event_definition.merge("identifiers" => nil)) + end + end + + context 'with duplicated event' do + context 'in known_events files' do + before do + allow(::Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:known_event?).with(event).and_return(true) + end + + it 'raises error' do + expect { described_class.new([], options).invoke_all }.to raise_error(RuntimeError) + end + end + + context 'in event definition files' do + before do + Dir.mkdir(File.join(temp_dir, "events")) + File.write(File.join(temp_dir, "events", "#{event}.yml"), { action: event }.to_yaml) + end + + it 'raises error' do + expect { described_class.new([], options).invoke_all }.to raise_error(RuntimeError) + end + end + end + end + + describe 'Creating metric definition file' do context 'for single time frame' do let(:time_frames) { %w[7d] } @@ -146,10 +222,14 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat it 'asks again for description' do allow_next_instance_of(described_class) do |instance| allow(instance).to receive(:ask) + .with(/By convention all events automatically include the following properties/) + .and_return(include_default_identifiers) + + allow(instance).to receive(:ask).twice .with(/Please describe in at least 50 characters/) .and_return("I am to short") - expect(instance).to receive(:ask) + expect(instance).to receive(:ask).twice .with(/Please provide description that is 50 characters long/) .and_return(description) end @@ -166,7 +246,6 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat let(:metric_definition_28d) do metric_definition_7d.merge( "key_path" => key_path_28d, - "name" => key_path_28d, "time_frame" => "28d" ) end @@ -186,7 +265,6 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat let(:metric_definition_28d) do metric_definition_7d.merge( "key_path" => key_path_28d, - "name" => key_path_28d, "time_frame" => "28d" ) end diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index 718b20c59ed..53dc145dcc4 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -258,6 +258,23 @@ RSpec.describe Gitlab::PathRegex do end end + describe '.organization_path_regex' do + subject { described_class.organization_path_regex } + + it 'rejects reserved words' do + expect(subject).not_to match('admin/') + expect(subject).not_to match('api/') + expect(subject).not_to match('create/') + expect(subject).not_to match('new/') + end + + it 'accepts other words' do + expect(subject).to match('simple/') + expect(subject).to match('org/') + expect(subject).to match('my_org/') + end + end + describe '.full_namespace_path_regex' do subject { described_class.full_namespace_path_regex } diff --git a/spec/lib/google_cloud/authentication_spec.rb b/spec/lib/google_cloud/authentication_spec.rb new file mode 100644 index 00000000000..5c7f3e51152 --- /dev/null +++ b/spec/lib/google_cloud/authentication_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GoogleCloud::Authentication, feature_category: :audit_events do + describe '#generate_access_token' do + let_it_be(:client_email) { 'test@example.com' } + let_it_be(:private_key) { 'private_key' } + let_it_be(:scope) { 'https://www.googleapis.com/auth/logging.write' } + let_it_be(:json_key_io) { StringIO.new({ client_email: client_email, private_key: private_key }.to_json) } + + let(:service_account_credentials) { instance_double('Google::Auth::ServiceAccountCredentials') } + + subject(:generate_access_token) do + described_class.new(scope: scope).generate_access_token(client_email, private_key) + end + + before do + allow(Google::Auth::ServiceAccountCredentials).to receive(:make_creds).with(json_key_io: json_key_io, + scope: scope).and_return(service_account_credentials) + allow(StringIO).to receive(:new).with({ client_email: client_email, + private_key: private_key }.to_json).and_return(json_key_io) + end + + context 'when credentials are valid' do + before do + allow(service_account_credentials).to receive(:fetch_access_token!).and_return({ 'access_token' => 'token' }) + end + + it 'calls make_creds with correct parameters' do + expect(Google::Auth::ServiceAccountCredentials).to receive(:make_creds).with(json_key_io: json_key_io, + scope: scope) + + generate_access_token + end + + it 'fetches access token' do + expect(generate_access_token).to eq('token') + end + end + + context 'when an error occurs' do + before do + allow(service_account_credentials).to receive(:fetch_access_token!).and_raise(StandardError) + end + + it 'handles the exception and returns nil' do + expect(Gitlab::ErrorTracking).to receive(:track_exception) + expect(generate_access_token).to be_nil + end + end + end +end diff --git a/spec/lib/google_cloud/logging_service/logger_spec.rb b/spec/lib/google_cloud/logging_service/logger_spec.rb new file mode 100644 index 00000000000..31f8bb27ec5 --- /dev/null +++ b/spec/lib/google_cloud/logging_service/logger_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GoogleCloud::LoggingService::Logger, feature_category: :audit_events do + let_it_be(:client_email) { 'test@example.com' } + let_it_be(:private_key) { 'private_key' } + let_it_be(:payload) { [{ logName: 'test-log' }.to_json] } + let_it_be(:access_token) { 'access_token' } + let_it_be(:expected_headers) do + { 'Authorization' => "Bearer #{access_token}", 'Content-Type' => 'application/json' } + end + + subject(:log) { described_class.new.log(client_email, private_key, payload) } + + describe '#log' do + context 'when access token is available' do + before do + allow_next_instance_of(GoogleCloud::Authentication) do |instance| + allow(instance).to receive(:generate_access_token).with(client_email, private_key).and_return(access_token) + end + end + + it 'generates access token and calls Gitlab::HTTP.post with correct parameters' do + expect(Gitlab::HTTP).to receive(:post).with( + described_class::WRITE_URL, + body: payload, + headers: expected_headers + ) + + log + end + + context 'when URI::InvalidURIError is raised' do + before do + allow(Gitlab::HTTP).to receive(:post).and_raise(URI::InvalidURIError) + end + + it 'logs the exception' do + expect(Gitlab::ErrorTracking).to receive(:log_exception) + + log + end + end + end + + context 'when access token is not available' do + let(:access_token) { nil } + + it 'does not call Gitlab::HTTP.post' do + allow_next_instance_of(GoogleCloud::Authentication) do |instance| + allow(instance).to receive(:generate_access_token).with(client_email, private_key).and_return(access_token) + end + + expect(Gitlab::HTTP).not_to receive(:post) + + log + end + end + end +end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index e2c87b0d85c..57a9963c0f1 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -1178,14 +1178,14 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end describe '.latest_diff_for_merge_requests' do - let_it_be(:merge_request_1) { create(:merge_request_without_merge_request_diff) } + let_it_be(:merge_request_1) { create(:merge_request, :skip_diff_creation) } let_it_be(:merge_request_1_diff_1) { create(:merge_request_diff, merge_request: merge_request_1, created_at: 3.days.ago) } let_it_be(:merge_request_1_diff_2) { create(:merge_request_diff, merge_request: merge_request_1, created_at: 1.day.ago) } - let_it_be(:merge_request_2) { create(:merge_request_without_merge_request_diff) } + let_it_be(:merge_request_2) { create(:merge_request, :skip_diff_creation) } let_it_be(:merge_request_2_diff_1) { create(:merge_request_diff, merge_request: merge_request_2, created_at: 3.days.ago) } - let_it_be(:merge_request_3) { create(:merge_request_without_merge_request_diff) } + let_it_be(:merge_request_3) { create(:merge_request, :skip_diff_creation) } subject { described_class.latest_diff_for_merge_requests([merge_request_1, merge_request_2]) } diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb index bb3d0c2307d..3b202b85b48 100644 --- a/spec/models/organizations/organization_spec.rb +++ b/spec/models/organizations/organization_spec.rb @@ -18,6 +18,37 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_length_of(:path).is_at_least(2).is_at_most(255) } + + describe 'path validator' do + using RSpec::Parameterized::TableSyntax + + let(:default_path_error) do + "can contain only letters, digits, '_', '-' and '.'. Cannot start with '-' or end in '.', '.git' or '.atom'." + end + + let(:reserved_path_error) do + "is a reserved name" + end + + where(:path, :valid, :error_message) do + 'path.' | false | ref(:default_path_error) + 'path.git' | false | ref(:default_path_error) + 'new' | false | ref(:reserved_path_error) + '.path' | true | nil + 'org__path' | true | nil + 'some-name' | true | nil + 'simple' | true | nil + end + + with_them do + it 'validates organization path' do + organization = build(:organization, name: 'Default', path: path) + + expect(organization.valid?).to be(valid) + expect(organization.errors.full_messages.to_sentence).to include(error_message) if error_message.present? + end + end + end end context 'when using scopes' do diff --git a/spec/models/preloaders/merge_request_diff_preloader_spec.rb b/spec/models/preloaders/merge_request_diff_preloader_spec.rb index 9a76d42e73f..9ca5039c4e6 100644 --- a/spec/models/preloaders/merge_request_diff_preloader_spec.rb +++ b/spec/models/preloaders/merge_request_diff_preloader_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Preloaders::MergeRequestDiffPreloader do let_it_be(:merge_request_1) { create(:merge_request) } let_it_be(:merge_request_2) { create(:merge_request) } - let_it_be(:merge_request_3) { create(:merge_request_without_merge_request_diff) } + let_it_be(:merge_request_3) { create(:merge_request, :skip_diff_creation) } let(:merge_requests) { [merge_request_1, merge_request_2, merge_request_3] } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d10a8afc51e..0e3382b4c6f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe User, feature_category: :user_profile do + using RSpec::Parameterized::TableSyntax + include ProjectForksHelper include TermsHelper include ExclusiveLeaseHelpers @@ -4168,8 +4170,6 @@ RSpec.describe User, feature_category: :user_profile do end describe '#following_users_allowed?' do - using RSpec::Parameterized::TableSyntax - let_it_be(:user) { create(:user) } let_it_be(:followee) { create(:user) } @@ -6021,27 +6021,42 @@ RSpec.describe User, feature_category: :user_profile do let(:user) { create(:user, note: "existing note") } let(:deleted_by) { create(:user) } - it 'blocks the user then schedules them for deletion if a hard delete is specified' do - expect(DeleteUserWorker).to receive(:perform_async).with(deleted_by.id, user.id, { hard_delete: true }) + shared_examples 'schedules user for deletion without delay' do + it 'schedules user for deletion without delay' do + expect(DeleteUserWorker).to receive(:perform_async).with(deleted_by.id, user.id, {}) + expect(DeleteUserWorker).not_to receive(:perform_in) + + user.delete_async(deleted_by: deleted_by) + end + end + + shared_examples 'it does not block the user' do + it 'does not block the user' do + user.delete_async(deleted_by: deleted_by) + expect(user).not_to be_blocked + end + end + + it 'blocks the user if hard delete is specified' do user.delete_async(deleted_by: deleted_by, params: { hard_delete: true }) expect(user).to be_blocked end - it 'schedules user for deletion without blocking them' do - expect(DeleteUserWorker).to receive(:perform_async).with(deleted_by.id, user.id, {}) - - user.delete_async(deleted_by: deleted_by) + it_behaves_like 'schedules user for deletion without delay' - expect(user).not_to be_blocked - end + it_behaves_like 'it does not block the user' context 'when target user is the same as deleted_by' do let(:deleted_by) { user } subject { user.delete_async(deleted_by: deleted_by) } + before do + allow(user).to receive(:has_possible_spam_contributions?).and_return(true) + end + shared_examples 'schedules the record for deletion with the correct delay' do it 'schedules the record for deletion with the correct delay' do freeze_time do @@ -6061,12 +6076,64 @@ RSpec.describe User, feature_category: :user_profile do expect(user).not_to be_banned end + context 'with possible spam contribution' do + context 'with comments' do + it_behaves_like 'schedules the record for deletion with the correct delay' do + before do + allow(user).to receive(:has_possible_spam_contributions?).and_call_original + + note = create(:note_on_issue, author: user) + create(:event, :commented, target: note, author: user) + end + end + end + + context 'with other types' do + where(:resource, :action, :delayed) do + 'Issue' | :created | true + 'MergeRequest' | :created | true + 'Issue' | :closed | false + 'MergeRequest' | :closed | false + 'WorkItem' | :created | false + end + + with_them do + before do + allow(user).to receive(:has_possible_spam_contributions?).and_call_original + + case resource + when 'Issue' + create(:event, action, :for_issue, author: user) + when 'MergeRequest' + create(:event, action, :for_merge_request, author: user) + when 'WorkItem' + create(:event, action, :for_work_item, author: user) + end + end + + if params[:delayed] + it_behaves_like 'schedules the record for deletion with the correct delay' + else + it_behaves_like 'schedules user for deletion without delay' + end + end + end + end + + context 'when user has no possible spam contributions' do + before do + allow(user).to receive(:has_possible_spam_contributions?).and_return(false) + end + + it_behaves_like 'schedules user for deletion without delay' + end + context 'when the user is a spammer' do before do allow(user).to receive(:spammer?).and_return(true) end - context 'when the user acount is less than 7 days old' do + context 'when the user account is less than 7 days old' do it_behaves_like 'schedules the record for deletion with the correct delay' it 'creates an abuse report with the correct data' do @@ -6140,13 +6207,9 @@ RSpec.describe User, feature_category: :user_profile do stub_feature_flags(delay_delete_own_user: false) end - it 'schedules user for deletion without blocking them' do - expect(DeleteUserWorker).to receive(:perform_async).with(user.id, user.id, {}) - - subject + it_behaves_like 'schedules user for deletion without delay' - expect(user).not_to be_blocked - end + it_behaves_like 'it does not block the user' it 'does not update the note' do expect { user.delete_async(deleted_by: deleted_by) }.not_to change { user.note } @@ -7303,8 +7366,6 @@ RSpec.describe User, feature_category: :user_profile do let(:user_id) { user.id } describe 'update user' do - using RSpec::Parameterized::TableSyntax - where(:attributes) do [ { state: 'blocked' }, diff --git a/spec/requests/api/ml/mlflow/experiments_spec.rb b/spec/requests/api/ml/mlflow/experiments_spec.rb index 1a2577e69e7..fc2e814752c 100644 --- a/spec/requests/api/ml/mlflow/experiments_spec.rb +++ b/spec/requests/api/ml/mlflow/experiments_spec.rb @@ -20,7 +20,6 @@ RSpec.describe API::Ml::Mlflow::Experiments, feature_category: :mlops do end let(:current_user) { developer } - let(:ff_value) { true } let(:access_token) { tokens[:write] } let(:headers) { { 'Authorization' => "Bearer #{access_token.token}" } } let(:project_id) { project.id } @@ -52,10 +51,6 @@ RSpec.describe API::Ml::Mlflow::Experiments, feature_category: :mlops do response end - before do - stub_feature_flags(ml_experiment_tracking: ff_value) - end - describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/get' do let(:experiment_iid) { experiment.iid.to_s } let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get?experiment_id=#{experiment_iid}" } diff --git a/spec/requests/api/ml/mlflow/runs_spec.rb b/spec/requests/api/ml/mlflow/runs_spec.rb index 746372b7978..a85fe4d867a 100644 --- a/spec/requests/api/ml/mlflow/runs_spec.rb +++ b/spec/requests/api/ml/mlflow/runs_spec.rb @@ -26,7 +26,6 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end let(:current_user) { developer } - let(:ff_value) { true } let(:access_token) { tokens[:write] } let(:headers) { { 'Authorization' => "Bearer #{access_token.token}" } } let(:project_id) { project.id } @@ -40,10 +39,6 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do response end - before do - stub_feature_flags(ml_experiment_tracking: ff_value) - end - RSpec.shared_examples 'MLflow|run_id param error cases' do context 'when run id is not passed' do let(:params) { {} } diff --git a/spec/support/helpers/models/merge_request_without_merge_request_diff.rb b/spec/support/helpers/models/merge_request_without_merge_request_diff.rb deleted file mode 100644 index e9f97a2c95a..00000000000 --- a/spec/support/helpers/models/merge_request_without_merge_request_diff.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -class MergeRequestWithoutMergeRequestDiff < ::MergeRequest # rubocop:disable Gitlab/NamespacedClass - self.inheritance_column = :_type_disabled - - def ensure_merge_request_diff; end -end diff --git a/spec/support/shared_examples/controllers/metrics/dashboard/prometheus_api_proxy_shared_examples.rb b/spec/support/shared_examples/controllers/metrics/dashboard/prometheus_api_proxy_shared_examples.rb deleted file mode 100644 index 9cdde13b36b..00000000000 --- a/spec/support/shared_examples/controllers/metrics/dashboard/prometheus_api_proxy_shared_examples.rb +++ /dev/null @@ -1,163 +0,0 @@ -# frozen_string_literal: true - -RSpec.shared_examples_for 'metrics dashboard prometheus api proxy' do - let(:service_params) { [proxyable, 'GET', 'query', expected_params] } - let(:service_result) { { status: :success, body: prometheus_body } } - let(:prometheus_proxy_service) { instance_double(Prometheus::ProxyService) } - let(:proxyable_params) do - { - id: proxyable.id.to_s - } - end - - let(:expected_params) do - ActionController::Parameters.new( - prometheus_proxy_params( - proxy_path: 'query', - controller: described_class.controller_path, - action: 'prometheus_proxy' - ) - ).permit! - end - - before do - stub_feature_flags(remove_monitor_metrics: false) - - allow_next_instance_of(Prometheus::ProxyService, *service_params) do |proxy_service| - allow(proxy_service).to receive(:execute).and_return(service_result) - end - end - - context 'with valid requests' do - context 'with success result' do - let(:prometheus_body) { '{"status":"success"}' } - let(:prometheus_json_body) { Gitlab::Json.parse(prometheus_body) } - - it 'returns prometheus response' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(Prometheus::ProxyService).to have_received(:new).with(*service_params) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq(prometheus_json_body) - end - - context 'with nil query' do - let(:params_without_query) do - prometheus_proxy_params.except(:query) - end - - before do - expected_params.delete(:query) - end - - it 'does not raise error' do - get :prometheus_proxy, params: params_without_query - - expect(Prometheus::ProxyService).to have_received(:new).with(*service_params) - end - end - end - - context 'with nil result' do - let(:service_result) { nil } - - it 'returns 204 no_content' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(json_response['status']).to eq(_('processing')) - expect(json_response['message']).to eq(_('Not ready yet. Try again later.')) - expect(response).to have_gitlab_http_status(:no_content) - end - end - - context 'with 404 result' do - let(:service_result) { { http_status: 404, status: :success, body: '{"body": "value"}' } } - - it 'returns body' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['body']).to eq('value') - end - end - - context 'with error result' do - context 'with http_status' do - let(:service_result) do - { http_status: :service_unavailable, status: :error, message: 'error message' } - end - - it 'sets the http response status code' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to have_gitlab_http_status(:service_unavailable) - expect(json_response['status']).to eq('error') - expect(json_response['message']).to eq('error message') - end - end - - context 'without http_status' do - let(:service_result) { { status: :error, message: 'error message' } } - - it 'returns bad_request' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['status']).to eq('error') - expect(json_response['message']).to eq('error message') - end - end - end - - context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - - it 'returns 404 not found' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to have_gitlab_http_status(:not_found) - expect(response.body).to be_empty - end - end - end - - context 'with inappropriate requests' do - let(:prometheus_body) { nil } - - context 'without correct permissions' do - let(:user2) { create(:user) } - - before do - sign_out(user) - sign_in(user2) - end - - it 'returns 404' do - get :prometheus_proxy, params: prometheus_proxy_params - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - context 'with invalid proxyable id' do - let(:prometheus_body) { nil } - - it 'returns 404' do - get :prometheus_proxy, params: prometheus_proxy_params(id: proxyable.id + 1) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - private - - def prometheus_proxy_params(params = {}) - { - proxy_path: 'query', - query: '1' - }.merge(proxyable_params).merge(params) - end -end diff --git a/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb b/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb index 2ca62698daf..f2c38d70508 100644 --- a/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb @@ -47,8 +47,13 @@ RSpec.shared_examples 'MLflow|shared error cases' do end end - context 'when ff is disabled' do - let(:ff_value) { false } + context 'when model experiments is unavailable' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?) + .with(current_user, :read_model_experiments, project) + .and_return(false) + end it "is Not Found" do is_expected.to have_gitlab_http_status(:not_found) diff --git a/spec/validators/organizations/path_validator_spec.rb b/spec/validators/organizations/path_validator_spec.rb new file mode 100644 index 00000000000..415c10d98df --- /dev/null +++ b/spec/validators/organizations/path_validator_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Organizations::PathValidator, feature_category: :cell do + let(:validator) { described_class.new(attributes: [:path]) } + + describe '.valid_path?' do + it 'handles invalid utf8' do + expect(described_class.valid_path?(+"a\0weird\255path")).to be_falsey + end + end + + describe '#validates_each' do + it 'adds a message when the path is not in the correct format' do + organization = build(:organization) + + validator.validate_each(organization, :path, "Path with spaces, and comma's!") + + expect(organization.errors[:path]).to include(Gitlab::PathRegex.namespace_format_message) + end + + it 'adds a message when the path is reserved when creating' do + organization = build(:organization, path: 'help') + + validator.validate_each(organization, :path, 'help') + + expect(organization.errors[:path]).to include('help is a reserved name') + end + + it 'adds a message when the path is reserved when updating' do + organization = create(:organization) + organization.path = 'help' + + validator.validate_each(organization, :path, 'help') + + expect(organization.errors[:path]).to include('help is a reserved name') + end + end +end |