diff options
36 files changed, 783 insertions, 478 deletions
diff --git a/.gitlab/ci/qa.gitlab-ci.yml b/.gitlab/ci/qa.gitlab-ci.yml index a946e34ff42..877d3275edb 100644 --- a/.gitlab/ci/qa.gitlab-ci.yml +++ b/.gitlab/ci/qa.gitlab-ci.yml @@ -16,14 +16,14 @@ qa:internal: extends: - .qa-job-base - - .qa:rules:ee-and-foss + - .qa:rules:internal script: - bundle exec rspec qa:internal-as-if-foss: extends: - qa:internal - - .qa:rules:as-if-foss + - .qa:rules:internal-as-if-foss - .as-if-foss qa:selectors: diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index d02299c48f6..ed60cb9ef17 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -858,6 +858,11 @@ ############ # QA rules # ############ +.qa:rules:internal: + rules: + - <<: *if-default-refs + changes: *qa-patterns + .qa:rules:ee-and-foss: rules: - <<: *if-default-refs @@ -873,6 +878,12 @@ - <<: *if-merge-request changes: *ci-patterns +.qa:rules:internal-as-if-foss: + rules: + - !reference [".strict-ee-only-rules", rules] + - <<: *if-default-refs + changes: *qa-patterns + .qa:rules:package-and-qa: rules: - <<: *if-not-ee diff --git a/.rubocop.yml b/.rubocop.yml index ec2c857e3d7..db37a1247e7 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -95,6 +95,10 @@ Style/FrozenStringLiteralComment: Enabled: true EnforcedStyle: always_true +Style/SpecialGlobalVars: + # https://gitlab.com/gitlab-org/gitlab/-/issues/358427 + EnforcedStyle: use_perl_names + RSpec/FilePath: Exclude: - 'qa/**/*' diff --git a/.rubocop_todo/style/special_global_vars.yml b/.rubocop_todo/style/special_global_vars.yml deleted file mode 100644 index 559d8169f9f..00000000000 --- a/.rubocop_todo/style/special_global_vars.yml +++ /dev/null @@ -1,60 +0,0 @@ ---- -# Cop supports --auto-correct. -Style/SpecialGlobalVars: - Exclude: - - 'app/controllers/help_controller.rb' - - 'app/models/application_setting_implementation.rb' - - 'app/services/prometheus/proxy_variable_substitution_service.rb' - - 'ee/bin/geo_log_cursor' - - 'ee/lib/ee/banzai/filter/references/epic_reference_filter.rb' - - 'ee/lib/ee/banzai/filter/references/iteration_reference_filter.rb' - - 'ee/lib/ee/banzai/filter/references/vulnerability_reference_filter.rb' - - 'lib/backup/database.rb' - - 'lib/banzai/filter/blockquote_fence_filter.rb' - - 'lib/banzai/filter/commit_trailers_filter.rb' - - 'lib/banzai/filter/front_matter_filter.rb' - - 'lib/banzai/filter/inline_metrics_redactor_filter.rb' - - 'lib/banzai/filter/references/abstract_reference_filter.rb' - - 'lib/banzai/filter/references/commit_range_reference_filter.rb' - - 'lib/banzai/filter/references/commit_reference_filter.rb' - - 'lib/banzai/filter/references/external_issue_reference_filter.rb' - - 'lib/banzai/filter/references/label_reference_filter.rb' - - 'lib/banzai/filter/references/milestone_reference_filter.rb' - - 'lib/banzai/filter/references/project_reference_filter.rb' - - 'lib/banzai/filter/references/reference_cache.rb' - - 'lib/banzai/filter/references/user_reference_filter.rb' - - 'lib/banzai/filter/sanitization_filter.rb' - - 'lib/extracts_ref.rb' - - 'lib/gitlab/dependency_linker/godeps_json_linker.rb' - - 'lib/gitlab/gfm/uploads_rewriter.rb' - - 'lib/gitlab/git/gitmodules_parser.rb' - - 'lib/gitlab/graphql/queries.rb' - - 'lib/gitlab/hook_data/base_builder.rb' - - 'lib/gitlab/log_timestamp_formatter.rb' - - 'lib/gitlab/puma_logging/json_formatter.rb' - - 'lib/gitlab/quick_actions/extractor.rb' - - 'lib/gitlab/runtime.rb' - - 'lib/gitlab/string_placeholder_replacer.rb' - - 'lib/prometheus/pid_provider.rb' - - 'lib/tasks/lint.rake' - - 'qa/chemlab-library-gitlab.gemspec' - - 'qa/qa/git/repository.rb' - - 'qa/qa/service/cluster_provider/gcloud.rb' - - 'qa/qa/support/wait_for_requests.rb' - - 'scripts/api/cancel_pipeline.rb' - - 'scripts/api/download_job_artifact.rb' - - 'scripts/api/get_job_id.rb' - - 'scripts/changed-feature-flags' - - 'scripts/failed_tests.rb' - - 'scripts/perf/query_limiting_report.rb' - - 'scripts/pipeline_test_report_builder.rb' - - 'scripts/rubocop-max-files-in-cache-check' - - 'scripts/setup/find-jh-branch.rb' - - 'scripts/static-analysis' - - 'scripts/trigger-build.rb' - - 'spec/fast_spec_helper.rb' - - 'spec/rack_servers/puma_spec.rb' - - 'spec/spec_helper.rb' - - 'spec/support/generate-seed-repo-rb' - - 'spec/support/helpers/test_env.rb' - - 'spec/support/prepare-gitlab-git-test-for-commit' diff --git a/app/assets/javascripts/boards/components/board_list_header.vue b/app/assets/javascripts/boards/components/board_list_header.vue index e256a8d57d6..9f70c84931f 100644 --- a/app/assets/javascripts/boards/components/board_list_header.vue +++ b/app/assets/javascripts/boards/components/board_list_header.vue @@ -57,6 +57,9 @@ export default { currentUserId: { default: null, }, + canCreateEpic: { + default: false, + }, }, props: { list: { @@ -129,7 +132,7 @@ export default { return (this.listType === ListType.backlog || this.showListHeaderButton) && !this.isEpicBoard; }, isNewEpicShown() { - return this.isEpicBoard && this.listType !== ListType.closed; + return this.isEpicBoard && this.canCreateEpic && this.listType !== ListType.closed; }, isSettingsShown() { return ( @@ -448,7 +451,6 @@ export default { icon="settings" @click="openSidebarSettings" /> - <gl-tooltip :target="() => $refs.settingsBtn">{{ $options.i18n.listSettings }}</gl-tooltip> </gl-button-group> </h3> </header> diff --git a/app/assets/javascripts/ide/components/nav_dropdown_button.vue b/app/assets/javascripts/ide/components/nav_dropdown_button.vue index 3699073adb8..6c26cde42e3 100644 --- a/app/assets/javascripts/ide/components/nav_dropdown_button.vue +++ b/app/assets/javascripts/ide/components/nav_dropdown_button.vue @@ -30,12 +30,12 @@ export default { </script> <template> - <dropdown-button> + <dropdown-button class="gl-w-full!"> <span class="row gl-flex-nowrap"> <span class="col-auto flex-fill text-truncate"> <gl-icon :size="16" :aria-label="__('Current Branch')" name="branch" /> {{ branchLabel }} </span> - <span v-if="showMergeRequests" class="col-5 pl-0 text-truncate"> + <span v-if="showMergeRequests" class="col-auto pl-0 text-truncate"> <gl-icon :size="16" :aria-label="__('Merge request')" name="merge-request" /> {{ mergeRequestLabel }} </span> diff --git a/app/assets/javascripts/issues/create_merge_request_dropdown.js b/app/assets/javascripts/issues/create_merge_request_dropdown.js index 949f0033b18..c96af6da720 100644 --- a/app/assets/javascripts/issues/create_merge_request_dropdown.js +++ b/app/assets/javascripts/issues/create_merge_request_dropdown.js @@ -453,7 +453,7 @@ export default class CreateMergeRequestDropdown { removeMessage(target) { const { input, message } = this.getTargetData(target); const inputClasses = ['gl-field-error-outline', 'gl-field-success-outline']; - const messageClasses = ['text-muted', 'text-danger', 'text-success']; + const messageClasses = ['gl-text-gray-600', 'gl-text-red-500', 'gl-text-green-500']; inputClasses.forEach((cssClass) => input.classList.remove(cssClass)); messageClasses.forEach((cssClass) => message.classList.remove(cssClass)); @@ -476,7 +476,7 @@ export default class CreateMergeRequestDropdown { this.removeMessage(target); input.classList.add('gl-field-success-outline'); - message.classList.add('text-success'); + message.classList.add('gl-text-green-500'); message.textContent = sprintf(__('%{text} is available'), { text }); message.style.display = 'inline-block'; } @@ -486,7 +486,7 @@ export default class CreateMergeRequestDropdown { const text = target === 'branch' ? __('branch name') : __('source'); this.removeMessage(target); - message.classList.add('text-muted'); + message.classList.add('gl-text-gray-600'); message.textContent = sprintf(__('Checking %{text} availability…'), { text }); message.style.display = 'inline-block'; } @@ -498,7 +498,7 @@ export default class CreateMergeRequestDropdown { this.removeMessage(target); input.classList.add('gl-field-error-outline'); - message.classList.add('text-danger'); + message.classList.add('gl-text-red-500'); message.textContent = text; message.style.display = 'inline-block'; } diff --git a/app/assets/javascripts/releases/components/release_block_footer.vue b/app/assets/javascripts/releases/components/release_block_footer.vue index cb795b3cba7..91d6d0911a4 100644 --- a/app/assets/javascripts/releases/components/release_block_footer.vue +++ b/app/assets/javascripts/releases/components/release_block_footer.vue @@ -104,9 +104,11 @@ export default { <div v-if="author" class="d-flex"> <span class="text-secondary">{{ __('by') }} </span> <user-avatar-link + class="gl-my-n1" :link-href="author.webUrl" :img-src="author.avatarUrl" :img-alt="userImageAltDescription" + :img-size="24" :tooltip-text="author.username" tooltip-placement="bottom" /> diff --git a/app/views/projects/deploy_keys/edit.html.haml b/app/views/projects/deploy_keys/edit.html.haml index f0214ade313..263b0025fe8 100644 --- a/app/views/projects/deploy_keys/edit.html.haml +++ b/app/views/projects/deploy_keys/edit.html.haml @@ -3,7 +3,7 @@ %hr %div - = form_for [@project, @deploy_key], include_id: false, html: { class: 'js-requires-input' } do |f| + = gitlab_ui_form_for [@project, @deploy_key], include_id: false, html: { class: 'js-requires-input' } do |f| = render partial: 'shared/deploy_keys/form', locals: { form: f, deploy_key: @deploy_key } .form-actions = f.submit _('Save changes'), class: 'gl-button btn btn-confirm' diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml index 630abd20d03..e184bb06cc0 100644 --- a/app/views/projects/issues/_new_branch.html.haml +++ b/app/views/projects/issues/_new_branch.html.haml @@ -55,7 +55,7 @@ %label{ for: 'source-name' } = _('Source (branch or tag)') %input#source-name.js-ref.ref.form-control.gl-form-input{ type: 'text', placeholder: "#{@project.default_branch}", value: "#{@project.default_branch}", data: { value: "#{@project.default_branch}" } } - %span.js-ref-message.form-text.text-muted + %span.js-ref-message.form-text .form-group %button.btn.gl-button.btn-confirm.js-create-target{ type: 'button', data: { action: 'create-mr' } } diff --git a/app/views/projects/pipeline_schedules/_form.html.haml b/app/views/projects/pipeline_schedules/_form.html.haml index e00d8e2c050..5ff0e2ccac3 100644 --- a/app/views/projects/pipeline_schedules/_form.html.haml +++ b/app/views/projects/pipeline_schedules/_form.html.haml @@ -1,4 +1,4 @@ -= form_for [@project, @schedule], as: :schedule, html: { id: "new-pipeline-schedule-form", class: "js-pipeline-schedule-form pipeline-schedule-form" } do |f| += gitlab_ui_form_for [@project, @schedule], as: :schedule, html: { id: "new-pipeline-schedule-form", class: "js-pipeline-schedule-form pipeline-schedule-form" } do |f| = form_errors(@schedule) .form-group.row .col-md-9 @@ -37,8 +37,7 @@ .col-md-9 = f.label :active, s_('PipelineSchedules|Activated'), class: 'label-bold' %div - = f.check_box :active, required: false, value: @schedule.active? - = f.label :active, _('Active'), class: 'gl-font-weight-normal' + = f.gitlab_ui_checkbox_component :active, _('Active'), checkbox_options: { value: @schedule.active, required: false } .footer-block.row-content-block = f.submit _('Save pipeline schedule'), class: 'btn gl-button btn-confirm' = link_to _('Cancel'), pipeline_schedules_path(@project), class: 'btn gl-button btn-default btn-cancel' diff --git a/app/views/shared/deploy_keys/_form.html.haml b/app/views/shared/deploy_keys/_form.html.haml index bf2514f8b0d..b60d433bafa 100644 --- a/app/views/shared/deploy_keys/_form.html.haml +++ b/app/views/shared/deploy_keys/_form.html.haml @@ -27,8 +27,5 @@ .form-group .col-form-label.col-sm-2 .col-sm-10 - = deploy_keys_project_form.label :can_push do - = deploy_keys_project_form.check_box :can_push - %strong= _('Grant write permissions to this key') - %p.light.gl-mb-0 - = _('Allow this key to push to this repository') + = deploy_keys_project_form.gitlab_ui_checkbox_component :can_push, _('Grant write permissions to this key'), + help_text: _('Allow this key to push to this repository') diff --git a/doc/administration/troubleshooting/log_parsing.md b/doc/administration/troubleshooting/log_parsing.md index c5443c564f4..e4d5fa4843c 100644 --- a/doc/administration/troubleshooting/log_parsing.md +++ b/doc/administration/troubleshooting/log_parsing.md @@ -143,6 +143,18 @@ CT: 297 ROUTE: /api/:version/projects/:id/repository/tags DURS: 731.39, CT: 190 ROUTE: /api/:version/projects/:id/repository/commits DURS: 1079.02, 979.68, 958.21 ``` +### Parsing `gitlab-rails/geo.log` + +#### Find most common Geo sync errors + +If [the `geo:status` Rake task](../geo/replication/troubleshooting.md#sync-status-rake-task) +repeatedly reports that some items never reach 100%, +the following command helps to focus on the most common errors. + +```shell +jq --raw-output 'select(.severity == "ERROR") | [.project_path, .message] | @tsv' geo.log | sort | uniq -c | sort | tail +``` + ### Parsing `gitaly/current` #### Find all Gitaly requests sent from web UI diff --git a/doc/user/application_security/dast_api/index.md b/doc/user/application_security/dast_api/index.md index 86f48318395..8b3a594b36f 100644 --- a/doc/user/application_security/dast_api/index.md +++ b/doc/user/application_security/dast_api/index.md @@ -7,65 +7,56 @@ type: reference, howto # DAST API **(ULTIMATE)** -You can add dynamic application security testing of web APIs to your [GitLab CI/CD](../../../ci/index.md) pipelines. -This helps you discover bugs and potential security issues that other QA processes may miss. +You can add dynamic application security testing (DAST) of web APIs to your +[GitLab CI/CD](../../../ci/index.md) pipelines. This helps you discover bugs and potential security +issues that other QA processes may miss. We recommend that you use DAST API testing in addition to [GitLab Secure](../index.md)'s other security scanners and your own test processes. If you're using [GitLab CI/CD](../../../ci/index.md), you can run DAST API tests as part your CI/CD workflow. -## Requirements - -- One of the following web API types: - - REST API - - SOAP - - GraphQL - - Form bodies, JSON, or XML -- One of the following assets to provide APIs to test: - - OpenAPI v2 or v3 API definition - - Postman Collection v2.0 or v2.1 - - HTTP Archive (HAR) of API requests to test +WARNING: +Do not run DAST API testing against a production server. Not only can it perform *any* function that +the API can, it may also trigger bugs in the API. This includes actions like modifying and deleting +data. Only run DAST API against a test server. -## When DAST API scans run +You can run DAST API scanning against the following web API types: -When using the `DAST-API.gitlab-ci.yml` template, the defined jobs use the `dast` stage by default. To enable your `.gitlab-ci.yml` file must include the `dast` stage in your `stages` definition. To ensure DAST API scans the latest code, your CI pipeline should deploy changes to a test environment in a stage before the `dast` stage: +- REST API +- SOAP +- GraphQL +- Form bodies, JSON, or XML -```yaml -stages: - - build - - test - - deploy - - dast -``` - -Note that if your pipeline is configured to deploy to the same web server on each run, running a -pipeline while another is still running could cause a race condition in which one pipeline -overwrites the code from another. The API to scan should be excluded from changes for the duration -of a DAST API scan. The only changes to the API should be from the DAST API scanner. Be aware that -any changes made to the API (for example, by users, scheduled tasks, database changes, code -changes, other pipelines, or other scanners) during a scan could cause inaccurate results. +## When DAST API scans run -## Enable DAST API scanning +DAST API scanning runs in the `dast` stage by default. To ensure DAST API scanning examines the latest +code, ensure your CI/CD pipeline deploys changes to a test environment in a stage before the `dast` +stage. -There are three ways to perform scans. See the configuration section for the one you wish to use: +If your pipeline is configured to deploy to the same web server on each run, running a pipeline +while another is still running could cause a race condition in which one pipeline overwrites the +code from another. The API to be scanned should be excluded from changes for the duration of a +DAST API scan. The only changes to the API should be from the DAST API scanner. Changes made to the +API (for example, by users, scheduled tasks, database changes, code changes, other pipelines, or +other scanners) during a scan could cause inaccurate results. -- [OpenAPI v2 or v3 specification](#openapi-specification) -- [HTTP Archive (HAR)](#http-archive-har) -- [Postman Collection v2.0 or v2.1](#postman-collection) +## Example DAST API scanning configurations -Examples of various configurations can be found here: +The following projects demonstrate DAST API scanning: -- [Example OpenAPI v2 specification project](https://gitlab.com/gitlab-org/security-products/demos/api-dast/openapi-example) +- [Example OpenAPI v2 Specification project](https://gitlab.com/gitlab-org/security-products/demos/api-dast/openapi-example) - [Example HTTP Archive (HAR) project](https://gitlab.com/gitlab-org/security-products/demos/api-dast/har-example) - [Example Postman Collection project](https://gitlab.com/gitlab-org/security-products/demos/api-dast/postman-example) - [Example GraphQL project](https://gitlab.com/gitlab-org/security-products/demos/api-dast/graphql-example) - [Example SOAP project](https://gitlab.com/gitlab-org/security-products/demos/api-dast/soap-example) -WARNING: -GitLab 14.0 will require that you place DAST API configuration files (for example, -`gitlab-dast-api-config.yml`) in your repository's `.gitlab` directory instead of your -repository's root. You can continue using your existing configuration files as they are, but -starting in GitLab 14.0, GitLab will not check your repository's root for configuration files. +## Targeting API for DAST scanning + +You can specify the API you want to scan by using: + +- [OpenAPI v2 or v3 Specification](#openapi-specification) +- [HTTP Archive (HAR)](#http-archive-har) +- [Postman Collection v2.0 or v2.1](#postman-collection) ### OpenAPI Specification @@ -84,52 +75,19 @@ the body generation is limited to these body types: - `application/json` - `application/xml` -To configure DAST API scanning with an OpenAPI specification: - -1. To use DAST API scanning, [include](../../../ci/yaml/index.md#includetemplate) - the [`DAST-API.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/DAST-API.gitlab-ci.yml) - that's provided as part of your GitLab installation. Add the following to your - `.gitlab-ci.yml` file: +#### DAST API scanning with an OpenAPI Specification - ```yaml - stages: - - dast +To configure DAST API scanning with an OpenAPI Specification: - include: - - template: DAST-API.gitlab-ci.yml - ``` +1. [Include](../../../ci/yaml/index.md#includetemplate) + the [`DAST-API.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/DAST-API.gitlab-ci.yml) in your `.gitlab-ci.yml` file. 1. The [configuration file](#configuration-files) has several testing profiles defined with different checks enabled. We recommend that you start with the `Quick` profile. Testing with this profile completes faster, allowing for easier configuration validation. + Provide the profile by adding the `DAST_API_PROFILE` CI/CD variable to your `.gitlab-ci.yml` file. - Provide the profile by adding the `DAST_API_PROFILE` CI/CD variable to your `.gitlab-ci.yml` file, - substituting `Quick` for the profile you choose: - - ```yaml - stages: - - dast - - include: - - template: DAST-API.gitlab-ci.yml - - variables: - DAST_API_PROFILE: Quick - ``` - -1. Provide the location of the OpenAPI specification. You can provide the specification as a file - or URL. Specify the location by adding the `DAST_API_OPENAPI` variable: - - ```yaml - stages: - - dast - - include: - - template: DAST-API.gitlab-ci.yml - - variables: - DAST_API_PROFILE: Quick - DAST_API_OPENAPI: test-api-specification.json - ``` +1. Provide the location of the OpenAPI Specification as either a file or URL. + Specify the location by adding the `DAST_API_OPENAPI` variable. 1. The target API instance's base URL is also required. Provide it by using the `DAST_API_TARGET_URL` variable or an `environment_url.txt` file. @@ -140,20 +98,20 @@ To configure DAST API scanning with an OpenAPI specification: automatically parses that file to find its scan target. You can see an [example of this in our Auto DevOps CI YAML](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml). - Here's an example of using `DAST_API_TARGET_URL`: +Complete example configuration of using an OpenAPI Specification: - ```yaml - stages: - - dast +```yaml +stages: + - dast - include: - - template: DAST-API.gitlab-ci.yml +include: + - template: DAST-API.gitlab-ci.yml - variables: - DAST_API_PROFILE: Quick - DAST_API_OPENAPI: test-api-specification.json - DAST_API_TARGET_URL: http://test-deployment/ - ``` +variables: + DAST_API_PROFILE: Quick + DAST_API_OPENAPI: test-api-specification.json + DAST_API_TARGET_URL: http://test-deployment/ +``` This is a minimal configuration for DAST API. From here you can: @@ -161,14 +119,12 @@ This is a minimal configuration for DAST API. From here you can: - [Add authentication](#authentication). - Learn how to [handle false positives](#handling-false-positives). -WARNING: -**NEVER** run DAST API testing against a production server. Not only can it perform *any* function that the API can, it may also trigger bugs in the API. This includes actions like modifying and deleting data. Only run DAST API scanning against a test server. - ### HTTP Archive (HAR) -The [HTTP Archive format (HAR)](http://www.softwareishard.com/blog/har-12-spec/) -is an archive file format for logging HTTP transactions. When used with the GitLab DAST API scanner, HAR must contain records of calling the web API to test. The DAST API scanner extracts all the requests and -uses them to perform testing. +The [HTTP Archive format (HAR)](../api_fuzzing/create_har_files.md) is an archive file format for +logging HTTP transactions. When used with the GitLab DAST API scanner, the HAR file must contain +records of calling the web API to test. The DAST API scanner extracts all of the requests and uses them +to perform testing. You can use various tools to generate HAR files: @@ -182,52 +138,20 @@ WARNING: HAR files may contain sensitive information such as authentication tokens, API keys, and session cookies. We recommend that you review the HAR file contents before adding them to a repository. -To configure DAST API scanning to use a HAR file: +#### DAST API scanning with a HAR file -1. To use DAST API, you must [include](../../../ci/yaml/index.md#includetemplate) - the [`DAST-API.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/DAST-API.gitlab-ci.yml) - that's provided as part of your GitLab installation. To do so, add the following to your - `.gitlab-ci.yml` file: +To configure DAST API to use a HAR file that provides information about the target API to test: - ```yaml - stages: - - dast - - include: - - template: DAST-API.gitlab-ci.yml - ``` +1. [Include](../../../ci/yaml/index.md#includetemplate) + the [`DAST-API.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/DAST-API.gitlab-ci.yml) in your `.gitlab-ci.yml` file. 1. The [configuration file](#configuration-files) has several testing profiles defined with different checks enabled. We recommend that you start with the `Quick` profile. Testing with this profile completes faster, allowing for easier configuration validation. - Provide the profile by adding the `DAST_API_PROFILE` CI/CD variable to your `.gitlab-ci.yml` file, - substituting `Quick` for the profile you choose: - - ```yaml - stages: - - dast - - include: - - template: DAST-API.gitlab-ci.yml - - variables: - DAST_API_PROFILE: Quick - ``` + Provide the profile by adding the `DAST_API_PROFILE` CI/CD variable to your `.gitlab-ci.yml` file. 1. Provide the location of the HAR file. You can provide the location as a file path - or URL. [URL support was introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/285020) in GitLab 13.10 and later. Specify the location by adding the `DAST_API_HAR` variable: - - ```yaml - stages: - - dast - - include: - - template: DAST-API.gitlab-ci.yml - - variables: - DAST_API_PROFILE: Quick - DAST_API_HAR: test-api-recording.har - ``` + or URL. [URL support was introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/285020) in GitLab 13.10 and later. Specify the location by adding the `DAST_API_HAR` variable. 1. The target API instance's base URL is also required. Provide it by using the `DAST_API_TARGET_URL` variable or an `environment_url.txt` file. @@ -238,20 +162,20 @@ To configure DAST API scanning to use a HAR file: automatically parses that file to find its scan target. You can see an [example of this in our Auto DevOps CI YAML](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml). - Here's an example of using `DAST_API_TARGET_URL`: +Complete example configuration of using an HAR file: - ```yaml - stages: - - dast +```yaml +stages: + - dast - include: - - template: DAST-API.gitlab-ci.yml +include: + - template: DAST-API.gitlab-ci.yml - variables: - DAST_API_PROFILE: Quick - DAST_API_HAR: test-api-recording.har - DAST_API_TARGET_URL: http://test-deployment/ - ``` +variables: + DAST_API_PROFILE: Quick + DAST_API_HAR: test-api-recording.har + DAST_API_TARGET_URL: http://test-deployment/ +``` This is a minimal configuration for DAST API. From here you can: @@ -259,11 +183,6 @@ This is a minimal configuration for DAST API. From here you can: - [Add authentication](#authentication). - Learn how to [handle false positives](#handling-false-positives). -WARNING: -**NEVER** run DAST API testing against a production server. Not only can it perform *any* function that -the API can, it may also trigger bugs in the API. This includes actions like modifying and deleting -data. Only run DAST API against a test server. - ### Postman Collection The [Postman API Client](https://www.postman.com/product/api-client/) is a popular tool that @@ -281,51 +200,20 @@ Postman Collection files may contain sensitive information such as authenticatio and session cookies. We recommend that you review the Postman Collection file contents before adding them to a repository. -To configure DAST API scanning to use a Postman Collection file: +#### DAST API scanning with a Postman Collection file -1. To use DAST API, you must [include](../../../ci/yaml/index.md#includetemplate) - the [`DAST-API.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/DAST-API.gitlab-ci.yml) - that's provided as part of your GitLab installation. To do so, add the following to your - `.gitlab-ci.yml` file: +To configure DAST API to use a Postman Collection file that provides information about the target +API to test: - ```yaml - stages: - - dast - - include: - - template: DAST-API.gitlab-ci.yml - ``` +1. [Include](../../../ci/yaml/index.md#includetemplate) + the [`DAST-API.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/DAST-API.gitlab-ci.yml). 1. The [configuration file](#configuration-files) has several testing profiles defined with different checks enabled. We recommend that you start with the `Quick` profile. Testing with this profile completes faster, allowing for easier configuration validation. - Provide the profile by adding the `DAST_API_PROFILE` CI/CD variable to your `.gitlab-ci.yml` file, - substituting `Quick` for the profile you choose: - - ```yaml - stages: - - dast - - include: - - template: DAST-API.gitlab-ci.yml - - variables: - DAST_API_PROFILE: Quick - ``` - -1. Provide the location of the Postman Collection file. You can provide the location as a file or URL. Specify the location by adding the `DAST_API_POSTMAN_COLLECTION` variable: - - ```yaml - stages: - - dast + Provide the profile by adding the `DAST_API_PROFILE` CI/CD variable to your `.gitlab-ci.yml` file. - include: - - template: DAST-API.gitlab-ci.yml - - variables: - DAST_API_PROFILE: Quick - DAST_API_POSTMAN_COLLECTION: postman-collection_serviceA.json - ``` +1. Provide the location of the Postman Collection file as either a file or URL. Specify the location by adding the `DAST_API_POSTMAN_COLLECTION` variable. 1. The target API instance's base URL is also required. Provide it by using the `DAST_API_TARGET_URL` variable or an `environment_url.txt` file. @@ -336,20 +224,20 @@ To configure DAST API scanning to use a Postman Collection file: automatically parses that file to find its scan target. You can see an [example of this in our Auto DevOps CI YAML](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml). - Here's an example of using `DAST_API_TARGET_URL`: +Complete example configuration of using a Postman collection: - ```yaml - stages: - - dast +```yaml +stages: + - dast - include: - - template: DAST-API.gitlab-ci.yml +include: + - template: DAST-API.gitlab-ci.yml - variables: - DAST_API_PROFILE: Quick - DAST_API_POSTMAN_COLLECTION: postman-collection_serviceA.json - DAST_API_TARGET_URL: http://test-deployment/ - ``` +variables: + DAST_API_PROFILE: Quick + DAST_API_POSTMAN_COLLECTION: postman-collection_serviceA.json + DAST_API_TARGET_URL: http://test-deployment/ +``` This is a minimal configuration for DAST API. From here you can: @@ -357,12 +245,7 @@ This is a minimal configuration for DAST API. From here you can: - [Add authentication](#authentication). - Learn how to [handle false positives](#handling-false-positives). -WARNING: -**NEVER** run DAST API testing against a production server. Not only can it perform *any* function that -the API can, it may also trigger bugs in the API. This includes actions like modifying and deleting -data. Only run DAST API against a test server. - -#### Postman variables +##### Postman variables Postman allows the developer to define placeholders that can be used in different parts of the requests. These placeholders are called variables, as explained in [Using variables](https://learning.postman.com/docs/sending-requests/variables/). @@ -386,7 +269,7 @@ Postman file. For example, Postman does not export environment-scoped variables file. By default, the DAST API scanner uses the Postman file to resolve Postman variable values. If a JSON file -is set in a GitLab CI environment variable `DAST_API_POSTMAN_COLLECTION_VARIABLES`, then the JSON +is set in a GitLab CI/CD environment variable `DAST_API_POSTMAN_COLLECTION_VARIABLES`, then the JSON file takes precedence to get Postman variable values. WARNING: @@ -419,12 +302,12 @@ values. For example: } ``` -### Authentication +## Authentication Authentication is handled by providing the authentication token as a header or cookie. You can provide a script that performs an authentication flow or calculates the token. -#### HTTP Basic Authentication +### HTTP Basic Authentication [HTTP basic authentication](https://en.wikipedia.org/wiki/Basic_access_authentication) is an authentication method built in to the HTTP protocol and used in conjunction with @@ -454,23 +337,23 @@ variables: DAST_API_HTTP_PASSWORD: $TEST_API_PASSWORD ``` -#### Bearer Tokens +### Bearer tokens Bearer tokens are used by several different authentication mechanisms, including OAuth2 and JSON Web -Tokens (JWT). Bearer tokens are transmitted using the `Authorization` HTTP header. To use bearer +Tokens (JWT). Bearer tokens are transmitted using the `Authorization` HTTP header. To use Bearer tokens with DAST API, you need one of the following: -- A token that doesn't expire -- A way to generate a token that lasts the length of testing -- A Python script that DAST API can call to generate the token +- A token that doesn't expire. +- A way to generate a token that lasts the length of testing. +- A Python script that DAST API can call to generate the token. -##### Token doesn't expire +#### Token doesn't expire -If the bearer token doesn't expire, use the `DAST_API_OVERRIDES_ENV` variable to provide it. This +If the Bearer token doesn't expire, use the `DAST_API_OVERRIDES_ENV` variable to provide it. This variable's content is a JSON snippet that provides headers and cookies to add to DAST API's outgoing HTTP requests. -Follow these steps to provide the bearer token with `DAST_API_OVERRIDES_ENV`: +Follow these steps to provide the Bearer token with `DAST_API_OVERRIDES_ENV`: 1. [Create a CI/CD variable](../../../ci/variables/index.md#custom-cicd-variables), for example `TEST_API_BEARERAUTH`, with the value @@ -500,9 +383,9 @@ Follow these steps to provide the bearer token with `DAST_API_OVERRIDES_ENV`: 1. To validate that authentication is working, run an DAST API test and review the job logs and the test API's application logs. -##### Token generated at test runtime +#### Token generated at test runtime -If the bearer token must be generated and doesn't expire during testing, you can provide to DAST API a file containing the token. A prior stage and job, or part of the DAST API job, can +If the Bearer token must be generated and doesn't expire during testing, you can provide DAST API a file that has the token. A prior stage and job, or part of the DAST API job, can generate this file. DAST API expects to receive a JSON file with the following structure: @@ -537,14 +420,14 @@ variables: To validate that authentication is working, run an DAST API test and review the job logs and the test API's application logs. -##### Token has short expiration +#### Token has short expiration -If the bearer token must be generated and expires prior to the scan's completion, you can provide a +If the Bearer token must be generated and expires prior to the scan's completion, you can provide a program or script for the DAST API scanner to execute on a provided interval. The provided script runs in an Alpine Linux container that has Python 3 and Bash installed. If the Python script requires additional packages, it must detect this and install the packages at runtime. -The script must create a JSON file containing the bearer token in a specific format: +The script must create a JSON file containing the Bearer token in a specific format: ```json { @@ -580,7 +463,7 @@ variables: To validate that authentication is working, run an DAST API test and review the job logs and the test API's application logs. See the [overrides section](#overrides) for more information about override commands. -### Configuration files +## Configuration files To get you started quickly, GitLab provides the configuration file [`gitlab-dast-api-config.yml`](https://gitlab.com/gitlab-org/security-products/analyzers/dast/-/blob/master/config/gitlab-dast-api-config.yml). @@ -588,12 +471,12 @@ This file has several testing profiles that perform various numbers of tests. Th profile increases as the test numbers go up. To use a configuration file, add it to your repository's root as `.gitlab/gitlab-dast-api-config.yml`. -#### Profiles +### Profiles The following profiles are pre-defined in the default configuration file. Profiles can be added, removed, and modified by creating a custom configuration. -##### Quick +#### Quick - Application Information Check - Cleartext Authentication Check @@ -608,7 +491,7 @@ can be added, removed, and modified by creating a custom configuration. - Token Check - XML Injection Check -##### Full +#### Full - Application Information Check - Cleartext AuthenticationCheck @@ -628,7 +511,7 @@ can be added, removed, and modified by creating a custom configuration. - Token Check - XML Injection Check -### Available CI/CD variables +## Available CI/CD variables | CI/CD variable | Description | |------------------------------------------------------|--------------------| @@ -654,7 +537,7 @@ can be added, removed, and modified by creating a custom configuration. |`DAST_API_SERVICE_START_TIMEOUT` | How long to wait for target API to become available in seconds. Default is 300 seconds. | |`DAST_API_TIMEOUT` | How long to wait for API responses in seconds. Default is 30 seconds. | -### Overrides +## Overrides DAST API provides a method to add or override specific items in your request, for example: @@ -812,7 +695,7 @@ It is changed to: You can provide this JSON document as a file or environment variable. You may also provide a command to generate the JSON document. The command can run at intervals to support values that expire. -#### Using a file +### Using a file To provide the overrides JSON as a file, the `DAST_API_OVERRIDES_FILE` CI/CD variable is set. The path is relative to the job current working directory. @@ -832,7 +715,7 @@ variables: DAST_API_OVERRIDES_FILE: dast-api-overrides.json ``` -#### Using a CI/CD variable +### Using a CI/CD variable To provide the overrides JSON as a CI/CD variable, use the `DAST_API_OVERRIDES_ENV` variable. This allows you to place the JSON as variables that can be masked and protected. @@ -870,7 +753,7 @@ variables: DAST_API_OVERRIDES_ENV: $SECRET_OVERRIDES ``` -#### Using a command +### Using a command If the value must be generated or regenerated on expiration, you can provide a program or script for the DAST API scanner to execute on a specified interval. The provided command runs in an Alpine Linux @@ -912,7 +795,7 @@ variables: DAST_API_OVERRIDES_INTERVAL: 300 ``` -#### Debugging overrides +### Debugging overrides > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/334578) in GitLab 14.8. @@ -963,10 +846,10 @@ def get_auth_response(): # In our example, access token is retrieved from a given endpoint try: - # Performs a http request, response sample: + # Performs a http request, response sample: # { "Token" : "b5638ae7-6e77-4585-b035-7d9de2e3f6b3" } response = get_auth_response() - + # Check that the request is successful. may raise `requests.exceptions.HTTPError` response.raise_for_status() @@ -993,7 +876,7 @@ except Exception as e: logging.error(f'Error, unknown error while retrieving access token. Error message: {e}') raise -# computes object that holds overrides file content. +# computes object that holds overrides file content. # It uses data fetched from request overrides_data = { "headers": { @@ -1068,7 +951,7 @@ variables: In the previous sample, you could use the script `user-pre-scan-set-up.sh` to also install new runtimes or applications that later on you could use in our overrides command. -### Exclude Paths +## Exclude Paths > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/211892) in GitLab 14.0. @@ -1086,7 +969,7 @@ To verify the paths are excluded, review the `Tested Operations` and `Excluded O 2021-05-27 21:51:08 [INF] API Security: ------------------------------------------------ ``` -#### Examples +### Examples This example excludes the `/auth` resource. This does not exclude child resources (`/auth/child`). @@ -1163,7 +1046,7 @@ pipelines. For more information, see the [Security Dashboard documentation](../s Once a vulnerability is found, you can interact with it. Read more on how to [address the vulnerabilities](../vulnerabilities/index.md). -## Handling False Positives +### Handling False Positives False positives can be handled in several ways: @@ -1175,7 +1058,7 @@ False positives can be handled in several ways: - Turn off the Check producing the false positive. This prevents the check from generating any vulnerabilities. Example checks are the SQL Injection Check, and JSON Hijacking Check. -### Turn off a Check +#### Turn off a Check Checks perform testing of a specific type and can be turned on and off for specific configuration profiles. The provided [configuration files](#configuration-files) define several profiles that you @@ -1233,7 +1116,7 @@ This results in the following YAML: - Name: XmlInjectionCheck ``` -### Turn off an Assertion for a Check +#### Turn off an Assertion for a Check Assertions detect vulnerabilities in tests produced by checks. Many checks support multiple Assertions such as Log Analysis, Response Analysis, and Status Code. When a vulnerability is found, the Assertion used is provided. To identify which Assertions are on by default, see the Checks default configuration in the configuration file. The section is called `Checks`. diff --git a/doc/user/infrastructure/iac/index.md b/doc/user/infrastructure/iac/index.md index 3bc7495d4be..6fa211742b8 100644 --- a/doc/user/infrastructure/iac/index.md +++ b/doc/user/infrastructure/iac/index.md @@ -6,68 +6,98 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Infrastructure as Code with Terraform and GitLab **(FREE)** -With Terraform in GitLab, you can use GitLab authentication and authorization with -your GitOps and Infrastructure-as-Code (IaC) workflows. -Use these features if you want to collaborate on Terraform code within GitLab or would like to use GitLab as a Terraform state storage that incorporates best practices out of the box. +To manage your infrastructure with GitLab, you can use the integration with +Terraform to define resources that you can version, reuse, and share: + +- Manage low-level components like compute, storage, and networking resources. +- Manage high-level components like DNS entries and SaaS features. +- Incorporate GitOps deployments and Infrastructure-as-Code (IaC) workflows. +- Use GitLab as a Terraform state storage. +- Store and use Terraform modules to simplify common and complex infrastructure patterns. + +<i class="fa fa-youtube-play youtube" aria-hidden="true"></i> Watch [a video overview](https://www.youtube.com/watch?v=iGXjUrkkzDI) of the features GitLab provides with the integration with Terraform. ## Integrate your project with Terraform > SAST test was [introduced](https://gitlab.com/groups/gitlab-org/-/epics/6655) in GitLab 14.6. -In GitLab 14.0 and later, to integrate your project with Terraform, add the following -to your `.gitlab-ci.yml` file: +The integration with GitLab and Terraform happens through GitLab CI/CD. +Use an `include` attribute to add the Terraform template to your project and +customize from there. -```yaml -include: - - template: Terraform.latest.gitlab-ci.yml +To get started, choose the template that best suits your needs: -variables: - # If you do not use the GitLab HTTP backend, remove this line and specify TF_HTTP_* variables - TF_STATE_NAME: default - TF_CACHE_KEY: default - # If your terraform files are in a subdirectory, set TF_ROOT accordingly - # TF_ROOT: terraform/production -``` +- [Latest template](#latest-terraform-template) +- [Stable template and advanced template](#stable-and-advanced-terraform-templates) -The `Terraform.latest.gitlab-ci.yml` template: +All templates: -- Uses the latest [GitLab Terraform image](https://gitlab.com/gitlab-org/terraform-images). -- Uses the [GitLab-managed Terraform state](#gitlab-managed-terraform-state) as +- Use the [GitLab-managed Terraform state](#gitlab-managed-terraform-state) as the Terraform state storage backend. -- Creates [four pipeline stages](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Terraform.latest.gitlab-ci.yml): - `test`, `validate`, `build`, and `deploy`. These stages - [run the Terraform commands](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Terraform/Base.latest.gitlab-ci.yml) - `test`, `validate`, `plan`, `plan-json`, and `apply`. The `apply` command only runs on the default branch. -- Runs the [Terraform SAST scanner](../../application_security/iac_scanning/index.md#configure-iac-scanning-manually), - that you can disable by creating a `SAST_DISABLED` environment variable and setting it to `1`. +- Trigger four pipeline stages: `test`, `validate`, `build`, and `deploy`. +- Run Terraform commands: `test`, `validate`, `plan`, and `plan-json`. It also runs the `apply` only on the default branch. +- Run the [Terraform SAST scanner](../../application_security/iac_scanning/index.md#configure-iac-scanning-manually). + +### Latest Terraform template + +The [latest template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Terraform.latest.gitlab-ci.yml) +is compatible with the most recent GitLab version. It provides the most recent +GitLab features, but can potentially include breaking changes. + +You can safely use the latest Terraform template: + +- If you use GitLab.com. +- If you use a self-managed instance updated with every new GitLab release. + +### Stable and advanced Terraform templates + +If you use earlier versions of GitLab, you might face incompatibility errors +between the GitLab version and the template version. In this case, you can opt +to use one of these templates: + +- [The stable template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Terraform.gitlab-ci.yml) with an skeleton that you can built on top of. +- [The advanced template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Terraform/Base.gitlab-ci.yml) to fully customize your setup. + +### Use a Terraform template + +To use a Terraform template: -You can override the values in the default template by updating your `.gitlab-ci.yml` file. +1. On the top bar, select **Menu > Projects** and find the project you want to integrate with Terraform. +1. On the left sidebar, select **Repository > Files**. +1. Edit your `.gitlab-ci.yml` file, use the `include` attribute to fetch the Terraform template: -The latest template might contain breaking changes between major GitLab releases. -For a more stable template, we recommend: + ```yaml + include: + # To fetch the latest template, use: + - template: Terraform.latest.gitlab-ci.yml + # To fetch the stable template, use: + - template: Terraform/Base.gitlab-ci.yml + # To fetch the advanced template, use: + - template: Terraform/Base.latest.gitlab-ci.yml + ``` -- [A ready-to-use version](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Terraform.gitlab-ci.yml) -- [A base template for customized setups](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Terraform/Base.gitlab-ci.yml) +1. Add the variables as described below: -This video from January 2021 walks you through all the GitLab Terraform integration features: + ```yaml + variables: + TF_STATE_NAME: default + TF_CACHE_KEY: default + # If your terraform files are in a subdirectory, set TF_ROOT accordingly. For example: + # TF_ROOT: terraform/production + ``` -<div class="video-fallback"> - See the video: <a href="https://www.youtube.com/watch?v=iGXjUrkkzDI">Terraform with GitLab</a>. -</div> -<figure class="video-container"> - <iframe src="https://www.youtube.com/embed/iGXjUrkkzDI" frameborder="0" allowfullscreen="true"> </iframe> -</figure> +1. (Optional) Override in your `.gitlab-ci.yaml` file the attributes present +in the template you fetched to customize your configuration. ## GitLab-managed Terraform state -[Terraform remote backends](https://www.terraform.io/language/settings/backends) +[Terraform remote backends](https://www.terraform.io/docs/language/settings/backends/index.html) enable you to store the state file in a remote, shared store. GitLab uses the -[Terraform HTTP backend](https://www.terraform.io/language/settings/backends/http) +[Terraform HTTP backend](https://www.terraform.io/docs/language/settings/backends/http.html) to securely store the state files in local storage (the default) or [the remote store of your choice](../../../administration/terraform_state.md). -The GitLab-managed Terraform state backend can store your Terraform state easily and -securely. It spares you from setting up additional remote resources like +The GitLab-managed Terraform state backend can safely store your Terraform state. It spares you from setting up additional remote resources like Amazon S3 or Google Cloud Storage. Its features include: - Supporting encryption of the state file both in transit and at rest. @@ -110,6 +140,7 @@ is available as part of the official Terraform provider documentation. - Learn how to [create a new cluster on Amazon Elastic Kubernetes Service (EKS)](../clusters/connect/new_eks_cluster.md). - Learn how to [create a new cluster on Google Kubernetes Engine (GKE)](../clusters/connect/new_gke_cluster.md). -## Troubleshooting +## Related topics -See the [troubleshooting](troubleshooting.md) documentation. +- [Terraform images](https://gitlab.com/gitlab-org/terraform-images). +- [Troubleshooting](troubleshooting.md) issues with GitLab and Terraform. diff --git a/doc/user/infrastructure/iac/mr_integration.md b/doc/user/infrastructure/iac/mr_integration.md index bcad5c9279a..0fea05a3f03 100644 --- a/doc/user/infrastructure/iac/mr_integration.md +++ b/doc/user/infrastructure/iac/mr_integration.md @@ -23,7 +23,7 @@ recommend encrypting plan output or modifying the project visibility settings. ## Configure Terraform report artifacts -GitLab ships with a [pre-built CI template](index.md#integrate-your-project-with-terraform) that uses GitLab Managed Terraform state and integrates Terraform changes into merge requests. We recommend customizing the pre-built image and relying on the `gitlab-terraform` helper provided within for a quick setup. +GitLab [integrates with Terraform](index.md#integrate-your-project-with-terraform) through CI/CD templates that use GitLab-managed Terraform state and display Terraform changes on merge requests. We recommend customizing the pre-built image and relying on the `gitlab-terraform` helper provided within for a quick setup. To manually configure a GitLab Terraform Report artifact: diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 2b190d89fa4..a640d42a471 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -82,7 +82,13 @@ module Gitlab end def included_templates - @context.expandset.filter_map { |i| i[:template] } + @context.includes.filter_map { |i| i[:location] if i[:type] == :template } + end + + def metadata + { + includes: @context.includes + } end private diff --git a/lib/gitlab/ci/config/external/context.rb b/lib/gitlab/ci/config/external/context.rb index 512cfdde474..2def565bc19 100644 --- a/lib/gitlab/ci/config/external/context.rb +++ b/lib/gitlab/ci/config/external/context.rb @@ -70,16 +70,20 @@ module Gitlab } end - def mask_variables_from(location) - variables.reduce(location.dup) do |loc, variable| + def mask_variables_from(string) + variables.reduce(string.dup) do |str, variable| if variable[:masked] - Gitlab::Ci::MaskSecret.mask!(loc, variable[:value]) + Gitlab::Ci::MaskSecret.mask!(str, variable[:value]) else - loc + str end end end + def includes + expandset.map(&:metadata) + end + protected attr_writer :expandset, :execution_deadline, :logger diff --git a/lib/gitlab/ci/config/external/file/artifact.rb b/lib/gitlab/ci/config/external/file/artifact.rb index 4f79e64ca9a..1244c7f7475 100644 --- a/lib/gitlab/ci/config/external/file/artifact.rb +++ b/lib/gitlab/ci/config/external/file/artifact.rb @@ -28,6 +28,14 @@ module Gitlab end end + def metadata + super.merge( + type: :artifact, + location: masked_location, + extra: { job_name: masked_job_name } + ) + end + private def project @@ -52,7 +60,7 @@ module Gitlab end unless artifact_job.present? - errors.push("Job `#{job_name}` not found in parent pipeline or does not have artifacts!") + errors.push("Job `#{masked_job_name}` not found in parent pipeline or does not have artifacts!") return false end @@ -80,6 +88,12 @@ module Gitlab parent_pipeline: context.parent_pipeline } end + + def masked_job_name + strong_memoize(:masked_job_name) do + context.mask_variables_from(job_name) + end + end end end end diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb index 4d7cf4af9f5..89da0796906 100644 --- a/lib/gitlab/ci/config/external/file/base.rb +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -55,6 +55,21 @@ module Gitlab end end + def metadata + { + context_project: context.project&.full_path, + context_sha: context.sha + } + end + + def eql?(other) + other.hash == hash + end + + def hash + [params, context.project&.full_path, context.sha].hash + end + protected def expanded_content_hash diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb index 3aa665c7d18..ee9cc1552fe 100644 --- a/lib/gitlab/ci/config/external/file/local.rb +++ b/lib/gitlab/ci/config/external/file/local.rb @@ -19,6 +19,14 @@ module Gitlab strong_memoize(:content) { fetch_local_content } end + def metadata + super.merge( + type: :local, + location: masked_location, + extra: {} + ) + end + private def validate_content! diff --git a/lib/gitlab/ci/config/external/file/project.rb b/lib/gitlab/ci/config/external/file/project.rb index 1fc22bf3780..3d4436530a8 100644 --- a/lib/gitlab/ci/config/external/file/project.rb +++ b/lib/gitlab/ci/config/external/file/project.rb @@ -27,17 +27,25 @@ module Gitlab strong_memoize(:content) { fetch_local_content } end + def metadata + super.merge( + type: :file, + location: masked_location, + extra: { project: masked_project_name, ref: masked_ref_name } + ) + end + private def validate_content! if !can_access_local_content? - errors.push("Project `#{project_name}` not found or access denied! Make sure any includes in the pipeline configuration are correctly defined.") + errors.push("Project `#{masked_project_name}` not found or access denied! Make sure any includes in the pipeline configuration are correctly defined.") elsif sha.nil? - errors.push("Project `#{project_name}` reference `#{ref_name}` does not exist!") + errors.push("Project `#{masked_project_name}` reference `#{masked_ref_name}` does not exist!") elsif content.nil? - errors.push("Project `#{project_name}` file `#{masked_location}` does not exist!") + errors.push("Project `#{masked_project_name}` file `#{masked_location}` does not exist!") elsif content.blank? - errors.push("Project `#{project_name}` file `#{masked_location}` is empty!") + errors.push("Project `#{masked_project_name}` file `#{masked_location}` is empty!") end end @@ -76,6 +84,18 @@ module Gitlab variables: context.variables } end + + def masked_project_name + strong_memoize(:masked_project_name) do + context.mask_variables_from(project_name) + end + end + + def masked_ref_name + strong_memoize(:masked_ref_name) do + context.mask_variables_from(ref_name) + end + end end end end diff --git a/lib/gitlab/ci/config/external/file/remote.rb b/lib/gitlab/ci/config/external/file/remote.rb index 8335a9ef625..e7b007b4d8d 100644 --- a/lib/gitlab/ci/config/external/file/remote.rb +++ b/lib/gitlab/ci/config/external/file/remote.rb @@ -18,6 +18,14 @@ module Gitlab strong_memoize(:content) { fetch_remote_content } end + def metadata + super.merge( + type: :remote, + location: masked_location, + extra: {} + ) + end + private def validate_location! diff --git a/lib/gitlab/ci/config/external/file/template.rb b/lib/gitlab/ci/config/external/file/template.rb index c3d120dfdce..9469f09ce13 100644 --- a/lib/gitlab/ci/config/external/file/template.rb +++ b/lib/gitlab/ci/config/external/file/template.rb @@ -20,6 +20,14 @@ module Gitlab strong_memoize(:content) { fetch_template_content } end + def metadata + super.merge( + type: :template, + location: masked_location, + extra: {} + ) + end + private def validate_location! diff --git a/lib/gitlab/ci/config/external/mapper.rb b/lib/gitlab/ci/config/external/mapper.rb index eae0277314b..c1250c82750 100644 --- a/lib/gitlab/ci/config/external/mapper.rb +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -48,7 +48,6 @@ module Gitlab .flat_map(&method(:expand_project_files)) .flat_map(&method(:expand_wildcard_paths)) .map(&method(:expand_variables)) - .each(&method(:verify_duplicates!)) .map(&method(:select_first_matching)) .each(&method(:verify!)) end @@ -112,26 +111,6 @@ module Gitlab end end - def verify_duplicates!(location) - logger.instrument(:config_mapper_verify) do - verify_max_includes_and_add_location!(location) - end - end - - def verify_max_includes_and_add_location!(location) - if expandset.count >= MAX_INCLUDES - raise TooManyIncludesError, "Maximum of #{MAX_INCLUDES} nested includes are allowed!" - end - - # Scope location to context to allow support of - # relative includes - scoped_location = location.merge( - context_project: context.project, - context_sha: context.sha) - - expandset.add(scoped_location) - end - def select_first_matching(location) logger.instrument(:config_mapper_select) do select_first_matching_without_instrumentation(location) @@ -149,7 +128,15 @@ module Gitlab end def verify!(location_object) + verify_max_includes! location_object.validate! + expandset.add(location_object) + end + + def verify_max_includes! + if expandset.count >= MAX_INCLUDES + raise TooManyIncludesError, "Maximum of #{MAX_INCLUDES} nested includes are allowed!" + end end def expand_variables(data) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 31f97b0a13a..6c388b966fc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -24735,13 +24735,13 @@ msgstr "" msgid "NamespaceStorageSize|push to your repository, create pipelines, create issues or add comments. To reduce storage capacity, delete unused repositories, artifacts, wikis, issues, and pipelines." msgstr "" -msgid "NamespaceStorage|%{name_with_link} namespace has approximately %{percent} namespace storage space remaining." +msgid "NamespaceStorage|%{name_with_link} namespace has approximately %{percent} (%{size}) namespace storage space remaining." msgstr "" msgid "NamespaceStorage|%{name_with_link} namespace has exceeded its namespace storage limit." msgstr "" -msgid "NamespaceStorage|%{name}(%{url}) namespace has approximately %{percent} namespace storage space remaining." +msgid "NamespaceStorage|%{name}(%{url}) namespace has approximately %{percent} (%{size}) namespace storage space remaining." msgstr "" msgid "NamespaceStorage|%{name}(%{url}) namespace has exceeded its namespace storage limit." diff --git a/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb b/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb index 0f1dae27293..9da8d106862 100644 --- a/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb @@ -4,8 +4,9 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Config::External::File::Artifact do let(:parent_pipeline) { create(:ci_pipeline) } + let(:variables) {} let(:context) do - Gitlab::Ci::Config::External::Context.new(parent_pipeline: parent_pipeline) + Gitlab::Ci::Config::External::Context.new(variables: variables, parent_pipeline: parent_pipeline) end let(:external_file) { described_class.new(params, context) } @@ -170,6 +171,58 @@ RSpec.describe Gitlab::Ci::Config::External::File::Artifact do end end end + + context 'when job is provided as a variable' do + let(:variables) do + Gitlab::Ci::Variables::Collection.new([ + { key: 'VAR1', value: 'a_secret_variable_value', masked: true } + ]) + end + + let(:params) { { artifact: 'generated.yml', job: 'a_secret_variable_value' } } + + context 'when job does not exist in the parent pipeline' do + let(:expected_error) do + 'Job `xxxxxxxxxxxxxxxxxxxxxxx` not found in parent pipeline or does not have artifacts!' + end + + it_behaves_like 'is invalid' + end + end + end + end + + describe '#metadata' do + let(:params) { { artifact: 'generated.yml' } } + + subject(:metadata) { external_file.metadata } + + it { + is_expected.to eq( + context_project: nil, + context_sha: nil, + type: :artifact, + location: 'generated.yml', + extra: { job_name: nil } + ) + } + + context 'when job name includes a masked variable' do + let(:variables) do + Gitlab::Ci::Variables::Collection.new([{ key: 'VAR1', value: 'a_secret_variable_value', masked: true }]) + end + + let(:params) { { artifact: 'generated.yml', job: 'a_secret_variable_value' } } + + it { + is_expected.to eq( + context_project: nil, + context_sha: nil, + type: :artifact, + location: 'generated.yml', + extra: { job_name: 'xxxxxxxxxxxxxxxxxxxxxxx' } + ) + } end end end diff --git a/spec/lib/gitlab/ci/config/external/file/base_spec.rb b/spec/lib/gitlab/ci/config/external/file/base_spec.rb index 71b1a58c82e..280bebe1a7c 100644 --- a/spec/lib/gitlab/ci/config/external/file/base_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/base_spec.rb @@ -112,4 +112,52 @@ RSpec.describe Gitlab::Ci::Config::External::File::Base do end end end + + describe '#metadata' do + let(:location) { 'some/file/config.yml' } + + subject(:metadata) { file.metadata } + + it { + is_expected.to eq( + context_project: nil, + context_sha: 'HEAD' + ) + } + end + + describe '#eql?' do + let(:location) { 'some/file/config.yml' } + + subject(:eql) { file.eql?(other_file) } + + context 'when the other file has the same params' do + let(:other_file) { test_class.new(location, context) } + + it { is_expected.to eq(true) } + end + + context 'when the other file has not the same params' do + let(:other_file) { test_class.new('some/other/file', context) } + + it { is_expected.to eq(false) } + end + end + + describe '#hash' do + let(:location) { 'some/file/config.yml' } + + subject(:filehash) { file.hash } + + context 'with a project' do + let(:project) { create(:project) } + let(:context_params) { { project: project, sha: 'HEAD', variables: variables } } + + it { is_expected.to eq([location, project.full_path, 'HEAD'].hash) } + end + + context 'without a project' do + it { is_expected.to eq([location, nil, 'HEAD'].hash) } + end + end end diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index f4f713ee46c..c0a0b0009ce 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -187,4 +187,20 @@ RSpec.describe Gitlab::Ci::Config::External::File::Local do end end end + + describe '#metadata' do + let(:location) { '/lib/gitlab/ci/templates/existent-file.yml' } + + subject(:metadata) { local_file.metadata } + + it { + is_expected.to eq( + context_project: project.full_path, + context_sha: '12345', + type: :local, + location: location, + extra: {} + ) + } + end end diff --git a/spec/lib/gitlab/ci/config/external/file/project_spec.rb b/spec/lib/gitlab/ci/config/external/file/project_spec.rb index ca3d731b86f..5d3412a148b 100644 --- a/spec/lib/gitlab/ci/config/external/file/project_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/project_spec.rb @@ -160,6 +160,23 @@ RSpec.describe Gitlab::Ci::Config::External::File::Project do expect(project_file.error_message).to include('Included file `/invalid-file` does not have YAML extension!') end end + + context 'when non-existing project is used with a masked variable' do + let(:variables) do + Gitlab::Ci::Variables::Collection.new([ + { key: 'VAR1', value: 'a_secret_variable_value', masked: true } + ]) + end + + let(:params) do + { project: 'a_secret_variable_value', file: '/file.yml' } + end + + it 'returns false with masked project name' do + expect(valid?).to be_falsy + expect(project_file.error_message).to include("Project `xxxxxxxxxxxxxxxxxxxxxxx` not found or access denied!") + end + end end describe '#expand_context' do @@ -177,6 +194,45 @@ RSpec.describe Gitlab::Ci::Config::External::File::Project do end end + describe '#metadata' do + let(:params) do + { project: project.full_path, file: '/file.yml' } + end + + subject(:metadata) { project_file.metadata } + + it { + is_expected.to eq( + context_project: context_project.full_path, + context_sha: '12345', + type: :file, + location: '/file.yml', + extra: { project: project.full_path, ref: 'HEAD' } + ) + } + + context 'when project name and ref include masked variables' do + let(:variables) do + Gitlab::Ci::Variables::Collection.new([ + { key: 'VAR1', value: 'a_secret_variable_value1', masked: true }, + { key: 'VAR2', value: 'a_secret_variable_value2', masked: true } + ]) + end + + let(:params) { { project: 'a_secret_variable_value1', ref: 'a_secret_variable_value2', file: '/file.yml' } } + + it { + is_expected.to eq( + context_project: context_project.full_path, + context_sha: '12345', + type: :file, + location: '/file.yml', + extra: { project: 'xxxxxxxxxxxxxxxxxxxxxxxx', ref: 'xxxxxxxxxxxxxxxxxxxxxxxx' } + ) + } + end + end + private def stub_project_blob(ref, path) diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index 5f148f6c411..5c07c87fd5a 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -199,4 +199,22 @@ RSpec.describe Gitlab::Ci::Config::External::File::Remote do is_expected.to be_empty end end + + describe '#metadata' do + before do + stub_full_request(location).to_return(body: remote_file_content) + end + + subject(:metadata) { remote_file.metadata } + + it { + is_expected.to eq( + context_project: nil, + context_sha: '12345', + type: :remote, + location: 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.xxxxxxxxxxx.yml', + extra: {} + ) + } + end end diff --git a/spec/lib/gitlab/ci/config/external/file/template_spec.rb b/spec/lib/gitlab/ci/config/external/file/template_spec.rb index 88740d492e8..4da9a933a9f 100644 --- a/spec/lib/gitlab/ci/config/external/file/template_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/template_spec.rb @@ -114,4 +114,18 @@ RSpec.describe Gitlab::Ci::Config::External::File::Template do is_expected.to be_empty end end + + describe '#metadata' do + subject(:metadata) { template_file.metadata } + + it { + is_expected.to eq( + context_project: project.full_path, + context_sha: '12345', + type: :template, + location: template, + extra: {} + ) + } + end end diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index 3da7181ba83..2d2adf09a42 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -21,6 +21,8 @@ RSpec.describe Gitlab::Ci::Config::External::Mapper do HEREDOC end + subject(:mapper) { described_class.new(values, context) } + before do stub_full_request(remote_url).to_return(body: file_content) @@ -30,7 +32,7 @@ RSpec.describe Gitlab::Ci::Config::External::Mapper do end describe '#process' do - subject { described_class.new(values, context).process } + subject(:process) { mapper.process } context "when single 'include' keyword is defined" do context 'when the string is a local file' do @@ -189,7 +191,12 @@ RSpec.describe Gitlab::Ci::Config::External::Mapper do end it 'does not raise an exception' do - expect { subject }.not_to raise_error + expect { process }.not_to raise_error + end + + it 'has expanset with one' do + process + expect(mapper.expandset.size).to eq(1) end end @@ -385,5 +392,27 @@ RSpec.describe Gitlab::Ci::Config::External::Mapper do end end end + + context "when locations are same after masking variables" do + let(:variables) do + Gitlab::Ci::Variables::Collection.new([ + { 'key' => 'GITLAB_TOKEN', 'value' => 'secret-file1', 'masked' => true }, + { 'key' => 'GITLAB_TOKEN', 'value' => 'secret-file2', 'masked' => true } + ]) + end + + let(:values) do + { include: [ + { 'local' => 'hello/secret-file1.yml' }, + { 'local' => 'hello/secret-file2.yml' } + ], + image: 'ruby:2.7' } + end + + it 'has expanset with two' do + process + expect(mapper.expandset.size).to eq(2) + end + end end end diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index 68f5aaee95f..56cd006717e 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Gitlab::Ci::Config::External::Processor do end describe "#perform" do - subject { processor.perform } + subject(:perform) { processor.perform } context 'when no external files defined' do let(:values) { { image: 'image:1.0' } } @@ -262,6 +262,18 @@ RSpec.describe Gitlab::Ci::Config::External::Processor do expect(process_obs_count).to eq(3) end + + it 'stores includes' do + perform + + expect(context.includes).to contain_exactly( + { type: :local, location: '/local/file.yml', extra: {}, context_project: project.full_path, context_sha: '12345' }, + { type: :template, location: 'Ruby.gitlab-ci.yml', extra: {}, context_project: project.full_path, context_sha: '12345' }, + { type: :remote, location: 'http://my.domain.com/config.yml', extra: {}, context_project: project.full_path, context_sha: '12345' }, + { type: :file, location: '/templates/my-workflow.yml', extra: { project: another_project.full_path, ref: 'HEAD' }, context_project: project.full_path, context_sha: '12345' }, + { type: :local, location: '/templates/my-build.yml', extra: {}, context_project: another_project.full_path, context_sha: another_project.commit.sha } + ) + end end context 'when user is reporter of another project' do @@ -377,10 +389,19 @@ RSpec.describe Gitlab::Ci::Config::External::Processor do output = processor.perform expect(output.keys).to match_array([:image, :my_build, :my_test]) end + + it 'stores includes' do + perform + + expect(context.includes).to contain_exactly( + { type: :file, location: '/templates/my-build.yml', extra: { project: another_project.full_path, ref: 'HEAD' }, context_project: project.full_path, context_sha: '12345' }, + { type: :file, location: '/templates/my-test.yml', extra: { project: another_project.full_path, ref: 'HEAD' }, context_project: project.full_path, context_sha: '12345' } + ) + end end context 'when local file path has wildcard' do - let_it_be(:project) { create(:project, :repository) } + let(:project) { create(:project, :repository) } let(:values) do { include: 'myfolder/*.yml', image: 'image:1.0' } @@ -412,6 +433,15 @@ RSpec.describe Gitlab::Ci::Config::External::Processor do output = processor.perform expect(output.keys).to match_array([:image, :my_build, :my_test]) end + + it 'stores includes' do + perform + + expect(context.includes).to contain_exactly( + { type: :local, location: 'myfolder/file1.yml', extra: {}, context_project: project.full_path, context_sha: '12345' }, + { type: :local, location: 'myfolder/file2.yml', extra: {}, context_project: project.full_path, context_sha: '12345' } + ) + end end context 'when rules defined' do diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 05558464a98..3ba6a9059c6 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -104,6 +104,26 @@ RSpec.describe Gitlab::Ci::Config do end it { is_expected.to contain_exactly('Jobs/Deploy.gitlab-ci.yml', 'Jobs/Build.gitlab-ci.yml') } + + it 'stores includes' do + expect(config.metadata[:includes]).to contain_exactly( + { type: :template, + location: 'Jobs/Deploy.gitlab-ci.yml', + extra: {}, + context_project: nil, + context_sha: nil }, + { type: :template, + location: 'Jobs/Build.gitlab-ci.yml', + extra: {}, + context_project: nil, + context_sha: nil }, + { type: :remote, + location: 'https://example.com/gitlab-ci.yml', + extra: {}, + context_project: nil, + context_sha: nil } + ) + end end context 'when using extendable hash' do @@ -403,6 +423,26 @@ RSpec.describe Gitlab::Ci::Config do end end end + + it 'stores includes' do + expect(config.metadata[:includes]).to contain_exactly( + { type: :local, + location: local_location, + extra: {}, + context_project: project.full_path, + context_sha: '12345' }, + { type: :remote, + location: remote_location, + extra: {}, + context_project: project.full_path, + context_sha: '12345' }, + { type: :file, + location: '.gitlab-ci.yml', + extra: { project: main_project.full_path, ref: 'HEAD' }, + context_project: project.full_path, + context_sha: '12345' } + ) + end end context "when gitlab_ci.yml has invalid 'include' defined" do diff --git a/spec/services/notes/build_service_spec.rb b/spec/services/notes/build_service_spec.rb index b7b08390dcd..0e2bbcc8c66 100644 --- a/spec/services/notes/build_service_spec.rb +++ b/spec/services/notes/build_service_spec.rb @@ -5,12 +5,14 @@ require 'spec_helper' RSpec.describe Notes::BuildService do include AdminModeHelper - let(:note) { create(:discussion_note_on_issue) } - let(:project) { note.project } - let(:author) { note.author } - let(:user) { author } - let(:merge_request) { create(:merge_request, source_project: project) } - let(:mr_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: note.author) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:note) { create(:discussion_note_on_issue, project: project) } + let_it_be(:author) { note.author } + let_it_be(:user) { author } + let_it_be(:noteable_author) { create(:user) } + let_it_be(:other_user) { create(:user) } + let_it_be(:external) { create(:user, :external) } + let(:base_params) { { note: 'Test' } } let(:params) { {} } @@ -28,11 +30,10 @@ RSpec.describe Notes::BuildService do end context 'when discussion is resolved' do - let(:params) { { in_reply_to_discussion_id: mr_note.discussion_id } } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + let_it_be(:mr_note) { create(:discussion_note_on_merge_request, :resolved, noteable: merge_request, project: project, author: author) } - before do - mr_note.resolve!(author) - end + let(:params) { { in_reply_to_discussion_id: mr_note.discussion_id } } it 'resolves the note' do expect(new_note).to be_valid @@ -57,7 +58,7 @@ RSpec.describe Notes::BuildService do end context 'when user has no access to discussion' do - let(:user) { create(:user) } + let(:user) { other_user } it 'sets an error' do expect(new_note.errors[:base]).to include('Discussion to reply to cannot be found') @@ -65,16 +66,14 @@ RSpec.describe Notes::BuildService do end context 'personal snippet note' do - def reply(note, user = nil) - user ||= create(:user) - + def reply(note, user = other_user) described_class.new(nil, user, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute end - let(:snippet_author) { create(:user) } + let_it_be(:snippet_author) { noteable_author } context 'when a snippet is public' do it 'creates a reply note' do @@ -89,8 +88,8 @@ RSpec.describe Notes::BuildService do end context 'when a snippet is private' do - let(:snippet) { create(:personal_snippet, :private, author: snippet_author) } - let(:note) { create(:discussion_note_on_personal_snippet, noteable: snippet) } + let_it_be(:snippet) { create(:personal_snippet, :private, author: snippet_author) } + let_it_be(:note) { create(:discussion_note_on_personal_snippet, noteable: snippet) } it 'creates a reply note when the author replies' do new_note = reply(note, snippet_author) @@ -107,8 +106,8 @@ RSpec.describe Notes::BuildService do end context 'when a snippet is internal' do - let(:snippet) { create(:personal_snippet, :internal, author: snippet_author) } - let(:note) { create(:discussion_note_on_personal_snippet, noteable: snippet) } + let_it_be(:snippet) { create(:personal_snippet, :internal, author: snippet_author) } + let_it_be(:note) { create(:discussion_note_on_personal_snippet, noteable: snippet) } it 'creates a reply note when the author replies' do new_note = reply(note, snippet_author) @@ -125,7 +124,7 @@ RSpec.describe Notes::BuildService do end it 'sets an error when an external user replies' do - new_note = reply(note, create(:user, :external)) + new_note = reply(note, external) expect(new_note.errors[:base]).to include('Discussion to reply to cannot be found') end @@ -134,7 +133,8 @@ RSpec.describe Notes::BuildService do end context 'when replying to individual note' do - let(:note) { create(:note_on_issue) } + let_it_be(:note) { create(:note_on_issue, project: project) } + let(:params) { { in_reply_to_discussion_id: note.discussion_id } } it 'sets the note up to be in reply to that note' do @@ -144,7 +144,7 @@ RSpec.describe Notes::BuildService do end context 'when noteable does not support replies' do - let(:note) { create(:note_on_commit) } + let_it_be(:note) { create(:note_on_commit, project: project) } it 'builds another individual note' do expect(new_note).to be_valid @@ -155,87 +155,137 @@ RSpec.describe Notes::BuildService do end context 'confidential comments' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:issuable_assignee) { other_user } + let_it_be(:issue) do + create(:issue, project: project, author: noteable_author, assignees: [issuable_assignee]) + end + before do - project.add_reporter(author) + project.add_guest(guest) + project.add_reporter(reporter) end - context 'when replying to a confidential comment' do - let(:note) { create(:note_on_issue, confidential: true) } - let(:params) { { in_reply_to_discussion_id: note.discussion_id, confidential: false } } + context 'when creating a new confidential comment' do + let(:params) { { confidential: true, noteable: issue } } - context 'when the user can read confidential comments' do - it '`confidential` param is ignored and set to `true`' do - expect(new_note.confidential).to be_truthy - end + shared_examples 'user allowed to set comment as confidential' do + it { expect(new_note.confidential).to be_truthy } end - context 'when the user cannot read confidential comments' do - let(:user) { create(:user) } + shared_examples 'user not allowed to set comment as confidential' do + it { expect(new_note.confidential).to be_falsey } + end - it 'returns `Discussion to reply to cannot be found` error' do - expect(new_note.errors.added?(:base, "Discussion to reply to cannot be found")).to be true + context 'reporter' do + let(:user) { reporter } + + it_behaves_like 'user allowed to set comment as confidential' + end + + context 'issuable author' do + let(:user) { noteable_author } + + it_behaves_like 'user allowed to set comment as confidential' + end + + context 'issuable assignee' do + let(:user) { issuable_assignee } + + it_behaves_like 'user allowed to set comment as confidential' + end + + context 'admin' do + before do + enable_admin_mode!(admin) end + + let(:user) { admin } + + it_behaves_like 'user allowed to set comment as confidential' end - end - context 'when replying to a public comment' do - let(:note) { create(:note_on_issue, confidential: false) } - let(:params) { { in_reply_to_discussion_id: note.discussion_id, confidential: true } } + context 'external' do + let(:user) { external } - it '`confidential` param is ignored and set to `false`' do - expect(new_note.confidential).to be_falsey + it_behaves_like 'user not allowed to set comment as confidential' + end + + context 'guest' do + let(:user) { guest } + + it_behaves_like 'user not allowed to set comment as confidential' end end - context 'when creating a new comment' do - context 'when the `confidential` note flag is set to `true`' do - context 'when the user is allowed (reporter)' do - let(:params) { { confidential: true, noteable: merge_request } } + context 'when replying to a confidential comment' do + let_it_be(:note) { create(:note_on_issue, confidential: true, noteable: issue, project: project) } - it 'note `confidential` flag is set to `true`' do - expect(new_note.confidential).to be_truthy - end - end + let(:params) { { in_reply_to_discussion_id: note.discussion_id, confidential: false } } - context 'when the user is allowed (issuable author)' do - let(:user) { create(:user) } - let(:issue) { create(:issue, author: user) } - let(:params) { { confidential: true, noteable: issue } } + shared_examples 'returns `Discussion to reply to cannot be found` error' do + it do + expect(new_note.errors.added?(:base, "Discussion to reply to cannot be found")).to be true + end + end - it 'note `confidential` flag is set to `true`' do - expect(new_note.confidential).to be_truthy - end + shared_examples 'confidential set to `true`' do + it '`confidential` param is ignored to match the parent note confidentiality' do + expect(new_note.confidential).to be_truthy end + end - context 'when the user is allowed (admin)' do - before do - enable_admin_mode!(admin) - end + context 'with reporter access' do + let(:user) { reporter } + + it_behaves_like 'confidential set to `true`' + end - let(:admin) { create(:admin) } - let(:params) { { confidential: true, noteable: merge_request } } + context 'with admin access' do + let(:user) { admin } - it 'note `confidential` flag is set to `true`' do - expect(new_note.confidential).to be_truthy - end + before do + enable_admin_mode!(admin) end - context 'when the user is not allowed' do - let(:user) { create(:user) } - let(:params) { { confidential: true, noteable: merge_request } } + it_behaves_like 'confidential set to `true`' + end + + context 'with noteable author' do + let(:user) { note.noteable.author } - it 'note `confidential` flag is set to `false`' do - expect(new_note.confidential).to be_falsey - end - end + it_behaves_like 'confidential set to `true`' end - context 'when the `confidential` note flag is set to `false`' do - let(:params) { { confidential: false, noteable: merge_request } } + context 'with noteable assignee' do + let(:user) { issuable_assignee } - it 'note `confidential` flag is set to `false`' do - expect(new_note.confidential).to be_falsey - end + it_behaves_like 'confidential set to `true`' + end + + context 'with guest access' do + let(:user) { guest } + + it_behaves_like 'returns `Discussion to reply to cannot be found` error' + end + + context 'with external user' do + let(:user) { external } + + it_behaves_like 'returns `Discussion to reply to cannot be found` error' + end + end + + context 'when replying to a public comment' do + let_it_be(:note) { create(:note_on_issue, confidential: false, noteable: issue, project: project) } + + let(:params) { { in_reply_to_discussion_id: note.discussion_id, confidential: true } } + + it '`confidential` param is ignored and set to `false`' do + expect(new_note.confidential).to be_falsey end end end |