diff options
33 files changed, 586 insertions, 138 deletions
diff --git a/app/assets/javascripts/pages/groups/settings/integrations/edit/index.js b/app/assets/javascripts/pages/groups/settings/integrations/edit/index.js new file mode 100644 index 00000000000..2d77f2686f7 --- /dev/null +++ b/app/assets/javascripts/pages/groups/settings/integrations/edit/index.js @@ -0,0 +1,16 @@ +import IntegrationSettingsForm from '~/integrations/integration_settings_form'; +import PrometheusMetrics from '~/prometheus_metrics/prometheus_metrics'; +import initAlertsSettings from '~/alerts_service_settings'; + +document.addEventListener('DOMContentLoaded', () => { + const prometheusSettingsWrapper = document.querySelector('.js-prometheus-metrics-monitoring'); + const integrationSettingsForm = new IntegrationSettingsForm('.js-integration-settings-form'); + integrationSettingsForm.init(); + + if (prometheusSettingsWrapper) { + const prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring'); + prometheusMetrics.loadActiveMetrics(); + } + + initAlertsSettings(document.querySelector('.js-alerts-service-settings')); +}); diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index dd338a7134b..0e507fd0988 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -553,6 +553,7 @@ vertical-align: text-top; } + a.ci-minutes-emoji gl-emoji, a.trial-link gl-emoji { font-size: $gl-font-size; vertical-align: baseline; diff --git a/app/controllers/admin/integrations_controller.rb b/app/controllers/admin/integrations_controller.rb index 715aa882bda..0d79032233f 100644 --- a/app/controllers/admin/integrations_controller.rb +++ b/app/controllers/admin/integrations_controller.rb @@ -1,67 +1,15 @@ # frozen_string_literal: true class Admin::IntegrationsController < Admin::ApplicationController - include ServiceParams - - before_action :not_found, unless: :instance_level_integrations_enabled? - before_action :service, only: [:edit, :update, :test] - - def edit - end - - def update - @service.attributes = service_params[:service] - - if @service.save(context: :manual_change) - redirect_to edit_admin_application_settings_integration_path(@service), notice: success_message - else - render :edit - end - end - - def test - if @service.can_test? - render json: service_test_response, status: :ok - else - render json: {}, status: :not_found - end - end + include IntegrationsActions private - def instance_level_integrations_enabled? + def integrations_enabled? Feature.enabled?(:instance_level_integrations) end - def project - # TODO: Change to something more meaningful - Project.first - end - - def service - @service ||= project.find_or_initialize_service(params[:id]) - end - - def success_message - message = @service.active? ? _('activated') : _('settings saved, but not activated') - - _('%{service_title} %{message}.') % { service_title: @service.title, message: message } - end - - def service_test_response - unless @service.update(service_params[:service]) - return { error: true, message: _('Validations failed.'), service_response: @service.errors.full_messages.join(','), test_failed: false } - end - - data = @service.test_data(project, current_user) - outcome = @service.test(data) - - unless outcome[:success] - return { error: true, message: _('Test failed.'), service_response: outcome[:result].to_s, test_failed: true } - end - - {} - rescue Gitlab::HTTP::BlockedUrlError => e - { error: true, message: _('Test failed.'), service_response: e.message, test_failed: true } + def scoped_edit_integration_path(integration) + edit_admin_application_settings_integration_path(integration) end end diff --git a/app/controllers/concerns/integrations_actions.rb b/app/controllers/concerns/integrations_actions.rb new file mode 100644 index 00000000000..ffb5d7a8086 --- /dev/null +++ b/app/controllers/concerns/integrations_actions.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module IntegrationsActions + extend ActiveSupport::Concern + + included do + include ServiceParams + + before_action :not_found, unless: :integrations_enabled? + before_action :integration, only: [:edit, :update, :test] + end + + def edit + render 'shared/integrations/edit' + end + + def update + integration.attributes = service_params[:service] + + saved = integration.save(context: :manual_change) + + respond_to do |format| + format.html do + if saved + redirect_to scoped_edit_integration_path(integration), notice: success_message + else + render 'shared/integrations/edit' + end + end + + format.json do + status = saved ? :ok : :unprocessable_entity + + render json: serialize_as_json, status: status + end + end + end + + def test + if integration.can_test? + render json: service_test_response, status: :ok + else + render json: {}, status: :not_found + end + end + + private + + def integrations_enabled? + false + end + + # TODO: Use actual integrations on the group / instance level + # To be completed in https://gitlab.com/groups/gitlab-org/-/epics/2430 + def project + Project.first + end + + def integration + # Using instance variable `@service` still required as it's used in ServiceParams + # and app/views/shared/_service_settings.html.haml. Should be removed once + # those 2 are refactored to use `@integration`. + @integration = @service ||= project.find_or_initialize_service(params[:id]) # rubocop:disable Gitlab/ModuleWithInstanceVariables + end + + def success_message + message = integration.active? ? _('activated') : _('settings saved, but not activated') + + _('%{service_title} %{message}.') % { service_title: integration.title, message: message } + end + + def serialize_as_json + integration + .as_json(only: integration.json_fields) + .merge(errors: integration.errors.as_json) + end + + def service_test_response + unless integration.update(service_params[:service]) + return { error: true, message: _('Validations failed.'), service_response: integration.errors.full_messages.join(','), test_failed: false } + end + + data = integration.test_data(project, current_user) + outcome = integration.test(data) + + unless outcome[:success] + return { error: true, message: _('Test failed.'), service_response: outcome[:result].to_s, test_failed: true } + end + + {} + rescue Gitlab::HTTP::BlockedUrlError => e + { error: true, message: _('Test failed.'), service_response: e.message, test_failed: true } + end +end diff --git a/app/controllers/groups/settings/integrations_controller.rb b/app/controllers/groups/settings/integrations_controller.rb new file mode 100644 index 00000000000..43f8a7118d4 --- /dev/null +++ b/app/controllers/groups/settings/integrations_controller.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Groups + module Settings + class IntegrationsController < Groups::ApplicationController + include IntegrationsActions + + before_action :authorize_admin_group! + + private + + def integrations_enabled? + Feature.enabled?(:group_level_integrations, group) + end + + def scoped_edit_integration_path(integration) + edit_group_settings_integration_path(group, integration) + end + end + end +end diff --git a/app/helpers/services_helper.rb b/app/helpers/services_helper.rb index d2336de7193..fe2df918819 100644 --- a/app/helpers/services_helper.rb +++ b/app/helpers/services_helper.rb @@ -66,6 +66,36 @@ module ServicesHelper edit_admin_application_settings_integration_path(integration) end + def scoped_integrations_path + if @project.present? + project_settings_integrations_path(@project) + elsif @group.present? + group_settings_integrations_path(@group) + else + integrations_admin_application_settings_path + end + end + + def scoped_integration_path(integration) + if @project.present? + project_settings_integration_path(@project, integration) + elsif @group.present? + group_settings_integration_path(@group, integration) + else + admin_application_settings_integration_path(integration) + end + end + + def scoped_test_integration_path(integration) + if @project.present? + test_project_settings_integration_path(@project, integration) + elsif @group.present? + test_group_settings_integration_path(@group, integration) + else + test_admin_application_settings_integration_path(integration) + end + end + extend self end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index f9d17bfc8b7..fb9e341dff7 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -873,7 +873,7 @@ module Ci def collect_test_reports!(test_reports) test_reports.get_suite(group_name).tap do |test_suite| each_report(Ci::JobArtifact::TEST_REPORT_FILE_TYPES) do |file_type, blob| - Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, test_suite) + Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, test_suite, job: self) end end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index ef22b429df9..e9cd0d91bc0 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -813,7 +813,7 @@ module Ci def test_reports Gitlab::Ci::Reports::TestReports.new.tap do |test_reports| - builds.latest.with_reports(Ci::JobArtifact.test_reports).each do |build| + builds.latest.with_reports(Ci::JobArtifact.test_reports).preload(:project).find_each do |build| build.collect_test_reports!(test_reports) end end diff --git a/app/serializers/test_case_entity.rb b/app/serializers/test_case_entity.rb index 5c915c1302c..d2e08590ef0 100644 --- a/app/serializers/test_case_entity.rb +++ b/app/serializers/test_case_entity.rb @@ -1,10 +1,23 @@ # frozen_string_literal: true class TestCaseEntity < Grape::Entity + include API::Helpers::RelatedResourcesHelpers + expose :status expose :name expose :classname expose :execution_time expose :system_output expose :stack_trace + expose :attachment_url, if: -> (*) { can_read_screenshots? } do |test_case| + expose_url(test_case.attachment_url) + end + + private + + alias_method :test_case, :object + + def can_read_screenshots? + Feature.enabled?(:junit_pipeline_screenshots_view, options[:project]) && test_case.has_attachment? + end end diff --git a/app/views/admin/integrations/_form.html.haml b/app/views/admin/integrations/_form.html.haml deleted file mode 100644 index aa865c3b052..00000000000 --- a/app/views/admin/integrations/_form.html.haml +++ /dev/null @@ -1,12 +0,0 @@ -%h3.page-title - = @service.title - -%p= @service.description - -= form_for @service, as: :service, url: admin_application_settings_integration_path, method: :put, html: { class: 'gl-show-field-errors fieldset-form integration-settings-form js-integration-settings-form', data: { 'can-test' => @service.can_test?, 'test-url' => test_admin_application_settings_integration_path(@service) } } do |form| - = render 'shared/service_settings', form: form, service: @service - - - if @service.editable? - .footer-block.row-content-block - = service_save_button(@service) - = link_to _('Cancel'), admin_application_settings_integration_path, class: 'btn btn-cancel' diff --git a/app/views/admin/integrations/edit.html.haml b/app/views/admin/integrations/edit.html.haml deleted file mode 100644 index b19d00d7a16..00000000000 --- a/app/views/admin/integrations/edit.html.haml +++ /dev/null @@ -1,5 +0,0 @@ -- add_to_breadcrumbs _('Integrations'), integrations_admin_application_settings_path -- breadcrumb_title @service.title -- page_title @service.title, _('Integrations') - -= render 'form' diff --git a/app/views/layouts/header/_current_user_dropdown.html.haml b/app/views/layouts/header/_current_user_dropdown.html.haml index 84906c305a7..c6299f244ec 100644 --- a/app/views/layouts/header/_current_user_dropdown.html.haml +++ b/app/views/layouts/header/_current_user_dropdown.html.haml @@ -26,6 +26,7 @@ - if current_user_menu?(:settings) %li = link_to s_("CurrentUser|Settings"), profile_path, data: { qa_selector: 'settings_link' } + = render_if_exists 'layouts/header/buy_ci_minutes' - if current_user_menu?(:help) %li.divider.d-md-none diff --git a/app/views/shared/integrations/_form.html.haml b/app/views/shared/integrations/_form.html.haml new file mode 100644 index 00000000000..0ddab1368c2 --- /dev/null +++ b/app/views/shared/integrations/_form.html.haml @@ -0,0 +1,14 @@ +- integration = local_assigns.fetch(:integration) + +%h3.page-title + = integration.title + +%p= integration.description + += form_for integration, as: :service, url: scoped_integration_path(integration), method: :put, html: { class: 'gl-show-field-errors fieldset-form integration-settings-form js-integration-settings-form', data: { 'can-test' => integration.can_test?, 'test-url' => scoped_test_integration_path(integration) } } do |form| + = render 'shared/service_settings', form: form, integration: integration + + - if integration.editable? + .footer-block.row-content-block + = service_save_button(integration) + = link_to _('Cancel'), scoped_integration_path(integration), class: 'btn btn-cancel' diff --git a/app/views/shared/integrations/edit.html.haml b/app/views/shared/integrations/edit.html.haml new file mode 100644 index 00000000000..927d2410132 --- /dev/null +++ b/app/views/shared/integrations/edit.html.haml @@ -0,0 +1,5 @@ +- add_to_breadcrumbs _('Integrations'), scoped_integrations_path +- breadcrumb_title @integration.title +- page_title @integration.title, _('Integrations') + += render 'shared/integrations/form', integration: @integration diff --git a/config/routes/admin.rb b/config/routes/admin.rb index c92484316e4..116c607c2cb 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -121,7 +121,7 @@ namespace :admin do get '/', to: redirect('admin/application_settings/general'), as: nil resources :services, only: [:index, :edit, :update] - resources :integrations, only: [:edit, :update, :test] do + resources :integrations, only: [:edit, :update] do member do put :test end diff --git a/config/routes/group.rb b/config/routes/group.rb index 1d51b3fb6fe..97d339fea98 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -31,6 +31,12 @@ constraints(::Constraints::GroupUrlConstrainer.new) do patch :update_auto_devops post :create_deploy_token, path: 'deploy_token/create' end + + resources :integrations, only: [:index, :edit, :update] do + member do + put :test + end + end end resource :variables, only: [:show, :update] diff --git a/doc/development/README.md b/doc/development/README.md index bc5f50b0499..94b67ee0dfa 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -164,6 +164,7 @@ Complementary reads: - [Jira Connect app](integrations/jira_connect.md) - [Security Scanners](integrations/secure.md) - [Secure Partner Integration](integrations/secure_partner_integration.md) +- [How to run Jenkins in development environment](integrations/jenkins.md) ## Testing guides diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md index 31d99813061..a4c55cdbd1b 100644 --- a/doc/development/contributing/issue_workflow.md +++ b/doc/development/contributing/issue_workflow.md @@ -273,45 +273,25 @@ or ~"Stretch". Any open issue for a previous milestone should be labeled ### Priority labels -Priority labels help us define the time a ~bug fix should be completed. Priority determines how quickly the defect turnaround time must be. -If there are multiple defects, the priority decides which defect has to be fixed immediately versus later. -This label documents the planned timeline & urgency which is used to measure against our target SLO on delivering ~bug fixes. +We have the following priority labels: -| Label | Meaning | Target SLO (applies only to ~bug and ~security defects) | -|-------|-----------------|----------------------------------------------------------------------------| -| ~P1 | Urgent Priority | The current release + potentially immediate hotfix to GitLab.com (30 days) | -| ~P2 | High Priority | The next release (60 days) | -| ~P3 | Medium Priority | Within the next 3 releases (approx one quarter or 90 days) | -| ~P4 | Low Priority | Anything outside the next 3 releases (more than one quarter or 120 days) | +- ~P1 +- ~P2 +- ~P3 +- ~P4 + +Please refer to the issue triage [priority label](https://about.gitlab.com/handbook/engineering/quality/issue-triage/#priority) section in our handbook to see how it's used. ### Severity labels -Severity labels help us clearly communicate the impact of a ~bug on users. -There can be multiple facets of the impact. The below is a guideline. - -| Label | Meaning | Functionality | Affected Users | GitLab.com Availability | Performance Degradation | API/Web Response time[^1] | -|-------|-------------------|-------------------------------------------------------|----------------------------------|----------------------------------------------------|-------------------------------------------------------|----------------------------| -| ~S1 | Blocker | Unusable feature with no workaround, user is blocked | Impacts 50% or more of users | Outage, Significant impact on all of GitLab.com | | Above 9000ms to timing out | -| ~S2 | Critical Severity | Broken Feature, workaround too complex & unacceptable | Impacts between 25%-50% of users | Significant impact on large portions of GitLab.com | Degradation is guaranteed to occur in the near future | Between 2000ms and 9000ms | -| ~S3 | Major Severity | Broken feature with an acceptable workaround | Impacts up to 25% of users | Limited impact on important portions of GitLab.com | Degradation is likely to occur in the near future | Between 1000ms and 2000ms | -| ~S4 | Low Severity | Functionality inconvenience or cosmetic issue | Impacts less than 5% of users | Minor impact on GitLab.com | Degradation _may_ occur but it's not likely | Between 500ms and 1000ms | - -If a bug seems to fall between two severity labels, assign it to the higher-severity label. - -- Example(s) of ~S1 - - Data corruption/loss. - - Security breach. - - Unable to create an issue or merge request. - - Unable to add a comment or thread to the issue or merge request. -- Example(s) of ~S2 - - Cannot submit changes through the web IDE but the commandline works. - - A status widget on the merge request page is not working but information can be seen in the test pipeline page. -- Example(s) of ~S3 - - Can create merge requests only from the Merge Requests list view, not from an Issue page. - - Status is not updated in real time and needs a page refresh. -- Example(s) of ~S4 - - Label colors are incorrect. - - UI elements are not fully aligned. +We have the following severity labels: + +- ~S1 +- ~S2 +- ~S3 +- ~S4 + +Please refer to the issue triage [severity label](https://about.gitlab.com/handbook/engineering/quality/issue-triage/#severity) section in our handbook to see how it's used. ### Label for community contributors @@ -505,8 +485,3 @@ to be involved in some capacity when work begins on the follow-up issue. --- [Return to Contributing documentation](index.md) - -[^1]: Our current response time standard is based on the TTFB P90 results of the - GitLab Performance Tool (GPT) being run against the 10k-user reference - environment. This run happens nightly and results are outputted to the - [wiki on the GPT project.](https://gitlab.com/gitlab-org/quality/performance/-/wikis/Benchmarks/Latest/10k) diff --git a/doc/development/integrations/jenkins.md b/doc/development/integrations/jenkins.md new file mode 100644 index 00000000000..44ee80b7b3f --- /dev/null +++ b/doc/development/integrations/jenkins.md @@ -0,0 +1,92 @@ +# How to run Jenkins in development environment (on OSX) **(STARTER)** + +This is a step by step guide on how to set up [Jenkins](https://jenkins.io/) on your local machine and connect to it from your GitLab instance. GitLab triggers webhooks on Jenkins, and Jenkins is connects to GitLab using the API. By running both applications on the same machine, we can make sure they are able to access each other. + +## Install Jenkins + +Install Jenkins and start the service using Homebrew. + +```shell +brew install jenkins +brew services start jenkins +``` + +## Configure GitLab + +GitLab does not allow requests to localhost or the local network by default. When running Jenkins on your local machine, you need to enable local access. + +1. Log into your GitLab instance as an admin. +1. Go to **{admin}** **Admin Area > Settings > Network**. +1. Expand `Outbound requests` and check the following checkboxes: + +- **Allow requests to the local network from web hooks and services** +- **Allow requests to the local network from system hooks** + +For more details about GitLab webhooks, see [Webhooks and insecure internal web services](../../security/webhooks.md). + +Jenkins uses the GitLab API and needs an access token. + +1. Log in to your GitLab instance. +1. Click on your profile picture, then click **Settings**. +1. Click **Access Tokens**. +1. Create a new Access Token with the **API** scope enabled. Note the value of the token. + +## Configure Jenkins + +Configure your GitLab API connection in Jenkins. + +1. Make sure the GitLab plugin is installed on Jenkins. You can manage plugins in **Manage Jenkins > Manage Plugins**. +1. Set up the GitLab connection: Go to **Manage Jenkins > Configure System**, find the **GitLab** section and check the `Enable authentication for '/project' end-point` checkbox. +1. To add your credentials, click **Add** then choose **Jenkins Credential Provider**. +1. Choose **GitLab API token** as the type of token. +1. Paste your GitLab access token and click **Add**. +1. Choose your credentials from the dropdown menu. +1. Add your GitLab host URL. Normally `http://localhost:3000/`. +1. Click **Save Settings**. + +For more details, see [GitLab documentation about Jenkins CI](../../integration/jenkins.md). + +## Configure Jenkins Project + +Set up the Jenkins project you are going to run your build on. + +1. On your Jenkins instance, go to **New Item**. +1. Pick a name, choose **Pipeline** and click **ok**. +1. Choose your GitLab connection from the dropdown. +1. Check the **Build when a change is pushed to GitLab** checkbox. +1. Check the following checkboxes: + +- **Accepted Merge Request Events** +- **Closed Merge Request Events** + +1. Updating the status on GitLab must be done by a pipeline script. Add GitLab update steps, using the following as an example: + +**Example Pipeline Script:** + +```groovy +pipeline { + agent any + + stages { + stage('gitlab') { + steps { + echo 'Notify GitLab' + updateGitlabCommitStatus name: 'build', state: 'pending' + updateGitlabCommitStatus name: 'build', state: 'success' + } + } + } +} +``` + +## Configure your GitLab project + +To activate the Jenkins service you must have a Starter subscription or higher. + +1. Go to your project's page, then **Settings > Integrations > Jenkins CI**. +1. Check the `Active` checkbox and the triggers for `Push` and `Merge request`. +1. Fill in your Jenkins host, project name, username and password and click **Test settings and save changes**. + +## Test your setup + +Make a change in your repository and open an MR. In your Jenkins project it should have triggered a new build and on your MR, there should be a widget saying "Pipeline #NUMBER passed". It will also include a link to your Jenkins build. diff --git a/doc/user/clusters/applications.md b/doc/user/clusters/applications.md index 14223cbc0e0..cea0baa7319 100644 --- a/doc/user/clusters/applications.md +++ b/doc/user/clusters/applications.md @@ -592,6 +592,8 @@ will be saved as a [CI job artifact](../../ci/pipelines/job_artifacts.md). Note the following: +- We recommend using the cluster management project exclusively for managing deployments to a cluster. + Do not add your application's source code to such projects. - When you set the value for `installed` key back to `false`, the application will be unprovisioned from the cluster. - If you update `.gitlab/managed-apps/<application>/values.yaml` with new values, the diff --git a/lib/gitlab/ci/parsers/test/junit.rb b/lib/gitlab/ci/parsers/test/junit.rb index 0ce901fa5aa..c3e4c88d077 100644 --- a/lib/gitlab/ci/parsers/test/junit.rb +++ b/lib/gitlab/ci/parsers/test/junit.rb @@ -8,11 +8,11 @@ module Gitlab JunitParserError = Class.new(Gitlab::Ci::Parsers::ParserError) ATTACHMENT_TAG_REGEX = /\[\[ATTACHMENT\|(?<path>.+?)\]\]/.freeze - def parse!(xml_data, test_suite) + def parse!(xml_data, test_suite, **args) root = Hash.from_xml(xml_data) all_cases(root) do |test_case| - test_case = create_test_case(test_case) + test_case = create_test_case(test_case, args) test_suite.add_test_case(test_case) end rescue Nokogiri::XML::SyntaxError @@ -46,7 +46,7 @@ module Gitlab [testcase].flatten.compact.map(&blk) end - def create_test_case(data) + def create_test_case(data, args) if data['failure'] status = ::Gitlab::Ci::Reports::TestCase::STATUS_FAILED system_output = data['failure'] @@ -66,7 +66,8 @@ module Gitlab execution_time: data['time'], status: status, system_output: system_output, - attachment: attachment + attachment: attachment, + job: args.fetch(:job) ) end diff --git a/lib/gitlab/ci/reports/test_case.rb b/lib/gitlab/ci/reports/test_case.rb index 55856f64533..d95941059ff 100644 --- a/lib/gitlab/ci/reports/test_case.rb +++ b/lib/gitlab/ci/reports/test_case.rb @@ -10,9 +10,10 @@ module Gitlab STATUS_ERROR = 'error' STATUS_TYPES = [STATUS_SUCCESS, STATUS_FAILED, STATUS_SKIPPED, STATUS_ERROR].freeze - attr_reader :name, :classname, :execution_time, :status, :file, :system_output, :stack_trace, :key, :attachment + attr_reader :name, :classname, :execution_time, :status, :file, :system_output, :stack_trace, :key, :attachment, :job - def initialize(name:, classname:, execution_time:, status:, file: nil, system_output: nil, stack_trace: nil, attachment: nil) + # rubocop: disable Metrics/ParameterLists + def initialize(name:, classname:, execution_time:, status:, file: nil, system_output: nil, stack_trace: nil, attachment: nil, job: nil) @name = name @classname = classname @file = file @@ -22,12 +23,24 @@ module Gitlab @stack_trace = stack_trace @key = sanitize_key_name("#{classname}_#{name}") @attachment = attachment + @job = job end + # rubocop: enable Metrics/ParameterLists def has_attachment? attachment.present? end + def attachment_url + return unless has_attachment? + + Rails.application.routes.url_helpers.file_project_job_artifacts_path( + job.project, + job.id, + attachment + ) + end + private def sanitize_key_name(key) diff --git a/lib/gitlab/experimentation.rb b/lib/gitlab/experimentation.rb index f1b952760b5..b8c630d1b12 100644 --- a/lib/gitlab/experimentation.rb +++ b/lib/gitlab/experimentation.rb @@ -34,6 +34,12 @@ module Gitlab environment: ::Gitlab.dev_env_or_com?, enabled_ratio: 0.1, tracking_category: 'Growth::Expansion::Experiment::CiNotificationDot' + }, + buy_ci_minutes_version_a: { + feature_toggle: :buy_ci_minutes_version_a, + environment: ::Gitlab.dev_env_or_com?, + enabled_ratio: 0.2, + tracking_category: 'Growth::Expansion::Experiment::BuyCiMinutesVersionA' } }.freeze diff --git a/lib/gitlab/graphql/present.rb b/lib/gitlab/graphql/present.rb index 7f69bf601d6..6d86d632ab4 100644 --- a/lib/gitlab/graphql/present.rb +++ b/lib/gitlab/graphql/present.rb @@ -15,7 +15,7 @@ module Gitlab end def self.use(schema_definition) - schema_definition.instrument(:field, Instrumentation.new) + schema_definition.instrument(:field, ::Gitlab::Graphql::Present::Instrumentation.new) end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 81179a55e80..d4f1cc75a49 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5992,6 +5992,9 @@ msgstr "" msgid "Current vulnerabilities count" msgstr "" +msgid "CurrentUser|Buy CI minutes" +msgstr "" + msgid "CurrentUser|Profile" msgstr "" diff --git a/spec/controllers/admin/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb index 0641f64b0e3..50748918893 100644 --- a/spec/controllers/admin/integrations_controller_spec.rb +++ b/spec/controllers/admin/integrations_controller_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Admin::IntegrationsController do + let_it_be(:project) { create(:project) } let(:admin) { create(:admin) } - let!(:project) { create(:project) } before do sign_in(admin) @@ -13,7 +13,7 @@ describe Admin::IntegrationsController do describe '#edit' do context 'when instance_level_integrations not enabled' do it 'returns not_found' do - allow(Feature).to receive(:enabled?).with(:instance_level_integrations) { false } + stub_feature_flags(instance_level_integrations: false) get :edit, params: { id: Service.available_services_names.sample } diff --git a/spec/controllers/groups/settings/integrations_controller_spec.rb b/spec/controllers/groups/settings/integrations_controller_spec.rb new file mode 100644 index 00000000000..bbf215a4bb9 --- /dev/null +++ b/spec/controllers/groups/settings/integrations_controller_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Groups::Settings::IntegrationsController do + let_it_be(:project) { create(:project) } + let(:user) { create(:user) } + let(:group) { create(:group) } + + before do + sign_in(user) + end + + describe '#edit' do + context 'when group_level_integrations not enabled' do + it 'returns not_found' do + stub_feature_flags(group_level_integrations: { enabled: false, thing: group }) + + get :edit, params: { group_id: group, id: Service.available_services_names.sample } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is not owner' do + it 'renders not_found' do + get :edit, params: { group_id: group, id: Service.available_services_names.sample } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is owner' do + before do + group.add_owner(user) + end + + Service.available_services_names.each do |integration_name| + context "#{integration_name}" do + it 'successfully displays the template' do + get :edit, params: { group_id: group, id: integration_name } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:edit) + end + end + end + end + end + + describe '#update' do + let(:integration) { create(:jira_service, project: project) } + + before do + group.add_owner(user) + + put :update, params: { group_id: group, id: integration.class.to_param, service: { url: url } } + end + + context 'valid params' do + let(:url) { 'https://jira.gitlab-example.com' } + + it 'updates the integration' do + expect(response).to have_gitlab_http_status(:found) + expect(integration.reload.url).to eq(url) + end + end + + context 'invalid params' do + let(:url) { 'ftp://jira.localhost' } + + it 'does not update the integration' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:edit) + expect(integration.reload.url).not_to eq(url) + end + end + end + + describe '#test' do + context 'testable' do + let(:integration) { create(:jira_service, project: project) } + + before do + group.add_owner(user) + end + + it 'returns ok' do + allow_any_instance_of(integration.class).to receive(:test) { { success: true } } + + put :test, params: { group_id: group, id: integration.class.to_param } + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'not testable' do + let(:integration) { create(:alerts_service, project: project) } + + it 'returns not found' do + put :test, params: { group_id: group, id: integration.class.to_param } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index a929eaeba3f..74931fcdeb2 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -705,13 +705,45 @@ describe Projects::PipelinesController do end describe 'GET test_report.json' do - subject(:get_test_report_json) do - get :test_report, params: { - namespace_id: project.namespace, - project_id: project, - id: pipeline.id - }, - format: :json + let(:pipeline) { create(:ci_pipeline, project: project) } + + context 'with attachments' do + let(:blob) do + <<~EOF + <testsuites> + <testsuite> + <testcase classname='Calculator' name='sumTest1' time='0.01'> + <failure>Some failure</failure> + <system-out>[[ATTACHMENT|some/path.png]]</system-out> + </testcase> + </testsuite> + </testsuites> + EOF + end + + before do + allow_any_instance_of(Ci::JobArtifact).to receive(:each_blob).and_yield(blob) + end + + it 'does not have N+1 problem with attachments' do + get_test_report_json + + create(:ci_build, name: 'rspec', pipeline: pipeline).tap do |build| + create(:ci_job_artifact, :junit, job: build) + end + + clear_controller_memoization + + control_count = ActiveRecord::QueryRecorder.new { get_test_report_json }.count + + create(:ci_build, name: 'karma', pipeline: pipeline).tap do |build| + create(:ci_job_artifact, :junit, job: build) + end + + clear_controller_memoization + + expect { get_test_report_json }.not_to exceed_query_limit(control_count) + end end context 'when feature is enabled' do @@ -772,6 +804,20 @@ describe Projects::PipelinesController do expect(response.body).to be_empty end end + + def get_test_report_json + get :test_report, params: { + namespace_id: project.namespace, + project_id: project, + id: pipeline.id + }, + format: :json + end + + def clear_controller_memoization + controller.clear_memoization(:pipeline_test_report) + controller.instance_variable_set(:@pipeline, nil) + end end describe 'GET test_report_count.json' do diff --git a/spec/factories/ci/test_case.rb b/spec/factories/ci/test_case.rb index 8017111bcc7..ce6bd0f3d7d 100644 --- a/spec/factories/ci/test_case.rb +++ b/spec/factories/ci/test_case.rb @@ -9,6 +9,7 @@ FactoryBot.define do status { "success" } system_output { nil } attachment { nil } + association :job, factory: :ci_build trait :with_attachment do attachment { "some/path.png" } @@ -24,7 +25,8 @@ FactoryBot.define do execution_time: execution_time, status: status, system_output: system_output, - attachment: attachment + attachment: attachment, + job: job ) end end diff --git a/spec/fixtures/api/schemas/entities/test_case.json b/spec/fixtures/api/schemas/entities/test_case.json index 70f6edeeeb7..0dd3c5d472f 100644 --- a/spec/fixtures/api/schemas/entities/test_case.json +++ b/spec/fixtures/api/schemas/entities/test_case.json @@ -10,7 +10,8 @@ "classname": { "type": "string" }, "execution_time": { "type": "float" }, "system_output": { "type": ["string", "null"] }, - "stack_trace": { "type": ["string", "null"] } + "stack_trace": { "type": ["string", "null"] }, + "attachment_url": { "type": ["string", "null"] } }, "additionalProperties": false } diff --git a/spec/lib/gitlab/ci/parsers/test/junit_spec.rb b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb index 9a486c312d4..da168f6daad 100644 --- a/spec/lib/gitlab/ci/parsers/test/junit_spec.rb +++ b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb @@ -4,10 +4,11 @@ require 'fast_spec_helper' describe Gitlab::Ci::Parsers::Test::Junit do describe '#parse!' do - subject { described_class.new.parse!(junit, test_suite) } + subject { described_class.new.parse!(junit, test_suite, args) } let(:test_suite) { Gitlab::Ci::Reports::TestSuite.new('rspec') } let(:test_cases) { flattened_test_cases(test_suite) } + let(:args) { { job: { id: 1, project: "project" } } } context 'when data is JUnit style XML' do context 'when there are no <testcases> in <testsuite>' do @@ -205,7 +206,7 @@ describe Gitlab::Ci::Parsers::Test::Junit do end end - context 'when data contains an attachment tag' do + context 'when attachment is specified in failed test case' do let(:junit) do <<~EOF <testsuites> @@ -219,11 +220,15 @@ describe Gitlab::Ci::Parsers::Test::Junit do EOF end - it 'add attachment to a test case' do + it 'assigns correct attributes to the test case' do expect { subject }.not_to raise_error expect(test_cases[0].has_attachment?).to be_truthy expect(test_cases[0].attachment).to eq("some/path.png") + + expect(test_cases[0].job).to be_present + expect(test_cases[0].job[:id]).to eq(1) + expect(test_cases[0].job[:project]).to eq("project") end end diff --git a/spec/lib/gitlab/ci/reports/test_case_spec.rb b/spec/lib/gitlab/ci/reports/test_case_spec.rb index c13161f3e7c..69fe05d573e 100644 --- a/spec/lib/gitlab/ci/reports/test_case_spec.rb +++ b/spec/lib/gitlab/ci/reports/test_case_spec.rb @@ -15,7 +15,8 @@ describe Gitlab::Ci::Reports::TestCase do file: 'spec/trace_spec.rb', execution_time: 1.23, status: described_class::STATUS_SUCCESS, - system_output: nil + system_output: nil, + job: build(:ci_build) } end @@ -28,6 +29,7 @@ describe Gitlab::Ci::Reports::TestCase do expect(test_case.execution_time).to eq(1.23) expect(test_case.status).to eq(described_class::STATUS_SUCCESS) expect(test_case.system_output).to be_nil + expect(test_case.job).to be_present end end @@ -99,6 +101,22 @@ describe Gitlab::Ci::Reports::TestCase do it '#has_attachment?' do expect(attachment_test_case.has_attachment?).to be_truthy end + + it '#attachment_url' do + expect(attachment_test_case.attachment_url).to match(/file\/some\/path.png/) + end + end + + context 'when attachment is missing' do + let(:test_case) { build(:test_case) } + + it '#has_attachment?' do + expect(test_case.has_attachment?).to be_falsy + end + + it '#attachment_url' do + expect(test_case.attachment_url).to be_nil + end end end end diff --git a/spec/serializers/test_case_entity_spec.rb b/spec/serializers/test_case_entity_spec.rb index 84203adea2c..f16c271be4d 100644 --- a/spec/serializers/test_case_entity_spec.rb +++ b/spec/serializers/test_case_entity_spec.rb @@ -31,5 +31,49 @@ describe TestCaseEntity do expect(subject[:execution_time]).to eq(2.22) end end + + context 'when feature is enabled' do + before do + stub_feature_flags(junit_pipeline_screenshots_view: true) + end + + context 'when attachment is present' do + let(:test_case) { build(:test_case, :with_attachment) } + + it 'returns the attachment_url' do + expect(subject).to include(:attachment_url) + end + end + + context 'when attachment is not present' do + let(:test_case) { build(:test_case) } + + it 'returns a nil attachment_url' do + expect(subject[:attachment_url]).to be_nil + end + end + end + + context 'when feature is disabled' do + before do + stub_feature_flags(junit_pipeline_screenshots_view: false) + end + + context 'when attachment is present' do + let(:test_case) { build(:test_case, :with_attachment) } + + it 'returns no attachment_url' do + expect(subject).not_to include(:attachment_url) + end + end + + context 'when attachment is not present' do + let(:test_case) { build(:test_case) } + + it 'returns no attachment_url' do + expect(subject).not_to include(:attachment_url) + end + end + end end end |