diff options
21 files changed, 334 insertions, 32 deletions
diff --git a/app/models/container_expiration_policy.rb b/app/models/container_expiration_policy.rb index fb1e0ce255e..b1dd720d908 100644 --- a/app/models/container_expiration_policy.rb +++ b/app/models/container_expiration_policy.rb @@ -52,4 +52,8 @@ class ContainerExpirationPolicy < ApplicationRecord def set_next_run_at self.next_run_at = Time.zone.now + ChronicDuration.parse(cadence).seconds end + + def disable! + update_attribute(:enabled, false) + end end diff --git a/app/services/container_expiration_policy_service.rb b/app/services/container_expiration_policy_service.rb index 82274fd8668..80f32298323 100644 --- a/app/services/container_expiration_policy_service.rb +++ b/app/services/container_expiration_policy_service.rb @@ -1,7 +1,14 @@ # frozen_string_literal: true class ContainerExpirationPolicyService < BaseService + InvalidPolicyError = Class.new(StandardError) + def execute(container_expiration_policy) + unless container_expiration_policy.valid? + container_expiration_policy.disable! + raise InvalidPolicyError + end + container_expiration_policy.schedule_next_run! container_expiration_policy.container_repositories.find_each do |container_repository| diff --git a/app/services/projects/container_repository/cleanup_tags_service.rb b/app/services/projects/container_repository/cleanup_tags_service.rb index b53a9c1561e..c5809c11ea9 100644 --- a/app/services/projects/container_repository/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_service.rb @@ -6,6 +6,7 @@ module Projects def execute(container_repository) return error('feature disabled') unless can_use? return error('access denied') unless can_destroy? + return error('invalid regex') unless valid_regex? tags = container_repository.tags tags = without_latest(tags) @@ -76,6 +77,17 @@ module Projects def can_use? Feature.enabled?(:container_registry_cleanup, project, default_enabled: true) end + + def valid_regex? + %w(name_regex_delete name_regex name_regex_keep).each do |param_name| + regex = params[param_name] + Gitlab::UntrustedRegexp.new(regex) unless regex.blank? + end + true + rescue RegexpError => e + Gitlab::ErrorTracking.log_exception(e, project_id: project.id) + false + end end end end diff --git a/app/workers/container_expiration_policy_worker.rb b/app/workers/container_expiration_policy_worker.rb index e1544be5aed..96590e165ae 100644 --- a/app/workers/container_expiration_policy_worker.rb +++ b/app/workers/container_expiration_policy_worker.rb @@ -12,6 +12,8 @@ class ContainerExpirationPolicyWorker # rubocop:disable Scalability/IdempotentWo user: container_expiration_policy.project.owner) do |project:, user:| ContainerExpirationPolicyService.new(project, user) .execute(container_expiration_policy) + rescue ContainerExpirationPolicyService::InvalidPolicyError => e + Gitlab::ErrorTracking.log_exception(e, container_expiration_policy_id: container_expiration_policy.id) end end end diff --git a/changelogs/unreleased/216088-disable-container-expiration-policy-when-invalid-regex-is-present.yml b/changelogs/unreleased/216088-disable-container-expiration-policy-when-invalid-regex-is-present.yml new file mode 100644 index 00000000000..0a8df78d0c7 --- /dev/null +++ b/changelogs/unreleased/216088-disable-container-expiration-policy-when-invalid-regex-is-present.yml @@ -0,0 +1,5 @@ +--- +title: Validate regex before sending them to CleanupContainerRepositoryWorker +merge_request: 34282 +author: +type: added diff --git a/config/initializers/grape_validators.rb b/config/initializers/grape_validators.rb index 9d2b6dc9bd1..22f2c9ecf92 100644 --- a/config/initializers/grape_validators.rb +++ b/config/initializers/grape_validators.rb @@ -7,3 +7,4 @@ Grape::Validations.register_validator(:git_sha, ::API::Validations::Validators:: Grape::Validations.register_validator(:integer_none_any, ::API::Validations::Validators::IntegerNoneAny) Grape::Validations.register_validator(:array_none_any, ::API::Validations::Validators::ArrayNoneAny) Grape::Validations.register_validator(:check_assignees_count, ::API::Validations::Validators::CheckAssigneesCount) +Grape::Validations.register_validator(:untrusted_regexp, ::API::Validations::Validators::UntrustedRegexp) diff --git a/doc/api/container_registry.md b/doc/api/container_registry.md index dbfda65a131..d4a4fc1a733 100644 --- a/doc/api/container_registry.md +++ b/doc/api/container_registry.md @@ -240,9 +240,9 @@ DELETE /projects/:id/registry/repositories/:repository_id/tags | --------- | ---- | -------- | ----------- | | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user. | | `repository_id` | integer | yes | The ID of registry repository. | -| `name_regex` | string | no | The [re2](https://github.com/google/re2/wiki/Syntax) regex of the name to delete. To delete all tags specify `.*`. **Note:** `name_regex` is deprecated in favor of `name_regex_delete`.| -| `name_regex_delete` | string | yes | The [re2](https://github.com/google/re2/wiki/Syntax) regex of the name to delete. To delete all tags specify `.*`.| -| `name_regex_keep` | string | no | The [re2](https://github.com/google/re2/wiki/Syntax) regex of the name to keep. This value will override any matches from `name_regex_delete`. Note: setting to `.*` will result in a no-op. | +| `name_regex` | string | no | The [re2](https://github.com/google/re2/wiki/Syntax) regex of the name to delete. To delete all tags specify `.*`. **Note:** `name_regex` is deprecated in favor of `name_regex_delete`. This field is validated. | +| `name_regex_delete` | string | yes | The [re2](https://github.com/google/re2/wiki/Syntax) regex of the name to delete. To delete all tags specify `.*`. This field is validated. | +| `name_regex_keep` | string | no | The [re2](https://github.com/google/re2/wiki/Syntax) regex of the name to keep. This value will override any matches from `name_regex_delete`. This field is validated. Note: setting to `.*` will result in a no-op. | | `keep_n` | integer | no | The amount of latest tags of given name to keep. | | `older_than` | string | no | Tags to delete that are older than the given time, written in human readable form `1h`, `1d`, `1month`. | diff --git a/doc/ci/pipelines/job_artifacts.md b/doc/ci/pipelines/job_artifacts.md index 82399e4a210..d20bfae8bf3 100644 --- a/doc/ci/pipelines/job_artifacts.md +++ b/doc/ci/pipelines/job_artifacts.md @@ -168,6 +168,18 @@ The collected SAST report will be uploaded to GitLab as an artifact and will be in the merge requests and pipeline view. It's also used to provide data for security dashboards. +#### `artifacts:reports:secret_detection` **(ULTIMATE)** + +> - Introduced in GitLab 13.1. +> - Requires GitLab Runner 11.5 and above. + +The `secret-detection` report collects [detected secrets](../../user/application_security/secret_detection/index.md) +as artifacts. + +The collected Secret Detection report is uploaded to GitLab as an artifact and summarized +in the merge requests and pipeline view. It's also used to provide data for security +dashboards. + #### `artifacts:reports:dependency_scanning` **(ULTIMATE)** > - Introduced in GitLab 11.5. diff --git a/doc/development/fe_guide/frontend_faq.md b/doc/development/fe_guide/frontend_faq.md index 4258ee88390..4f814f3cdde 100644 --- a/doc/development/fe_guide/frontend_faq.md +++ b/doc/development/fe_guide/frontend_faq.md @@ -141,7 +141,7 @@ function initFoo() { }); } -// Vuex action can now reference the path from it's state :) +// Vuex action can now reference the path from its state :) export const fetchFoos = ({ state }) => { return axios.get(state.settings.fooPath); }; diff --git a/doc/user/application_security/secret_detection/index.md b/doc/user/application_security/secret_detection/index.md index 27b58c2a057..85933c31a34 100644 --- a/doc/user/application_security/secret_detection/index.md +++ b/doc/user/application_security/secret_detection/index.md @@ -29,18 +29,30 @@ GitLab displays identified secrets as part of the SAST reports visibly in a few ## Use cases -- Detecting accidental commit of secrets like keys, passwords, and API tokens. +- Detecting unintentional commit of secrets like keys, passwords, and API tokens. - Performing a single or recurring scan of the full history of your repository for secrets. +## Requirements + +To run Secret Detection jobs, by default, you need GitLab Runner with the +[`docker`](https://docs.gitlab.com/runner/executors/docker.html) or +[`kubernetes`](https://docs.gitlab.com/runner/install/kubernetes.html) executor. +If you're using the shared Runners on GitLab.com, this is enabled by default. + +CAUTION: **Caution:** Our Secret Detection jobs currently expect a Linux container type. Windows containers are not yet supported. + +CAUTION: **Caution:** +If you use your own Runners, make sure the Docker version installed +is **not** `19.03.0`. See [troubleshooting information](../sast#error-response-from-daemon-error-processing-tar-file-docker-tar-relocation-error) for details. + ## Configuration -If you already have SAST enabled for your app, you don’t need to take any action to benefit from this -new feature. It is also included in the Auto DevOps default configuration. +NOTE: **Note:** +With GitLab 13.1 Secret Detection was split into its own CI/CD template. -Secret Detection is performed by a [specific analyzer](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml#L180) -during the `sast` job. It runs regardless of the programming -language of your app, and you don't need to change your -CI/CD configuration file to enable it. Results are available in the SAST report. +Secret Detection is performed by a [specific analyzer](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/Secret-Detection.gitlab-ci.yml) +during the `secret-detection` job. It runs regardless of the programming +language of your app. The Secret Detection analyzer includes [Gitleaks](https://github.com/zricethezav/gitleaks) and [TruffleHog](https://github.com/dxa4481/truffleHog) checks. @@ -54,6 +66,86 @@ NOTE: **Note:** You don't have to configure Secret Detection manually as shown in this section if you're using [Auto Secret Detection](../../../topics/autodevops/stages.md#auto-secret-detection-ultimate) provided by [Auto DevOps](../../../topics/autodevops/index.md). +To enable Secret Detection for GitLab 13.1 and later, you must include the `Secret-Detection.gitlab-ci.yml` template that’s provided as a part of your GitLab installation. For GitLab versions earlier than 11.9, you can copy and use the job as defined in that template. + +Add the following to your `.gitlab-ci.yml` file: + +```yaml +include: + - template: Secret-Detection.gitlab-ci.yml +``` + +The included template creates Secret Detection jobs in your CI/CD pipeline and scans +your project's source code for secrets. + +The results are saved as a +[Secret Detection report artifact](../../../ci/pipelines/job_artifacts.md#artifactsreportssecret_detection-ultimate) +that you can later download and analyze. Due to implementation limitations, we +always take the latest Secret Detection artifact available. + +### Using the SAST Template + +Prior to GitLab 13.1, Secret Detection was part of [SAST configuration](../sast#configuration). +If you already have SAST enabled for your app configured before GitLab 13.1, +you don't need to manually configure it. + +CAUTION: **Planned Deprecation:** +In a future GitLab release, configuring Secret Detection with the SAST template will be deprecated. Please begin using `Secret-Detection.gitlab-ci.yml` +to prevent future issues. We have made a +[video to guide you through the process of transitioning](https://www.youtube.com/watch?v=W2tjcQreDwQ) +to this new template. + +<div class="video-fallback"> + See the video: <a href="https://www.youtube.com/watch?v=W2tjcQreDwQ">Walkthrough of historical secret scan</a>. +</div> +<figure class="video-container"> + <iframe src="https://www.youtube.com/embed/W2tjcQreDwQ" frameborder="0" allowfullscreen="true"> </iframe> +</figure> + +When using the SAST template, Secret Detection is performed by a [specific analyzer](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml#L180) +during the `sast` job. It runs regardless of the programming +language of your app, and you don't need to change your +CI/CD configuration file to enable it. Results are available in the SAST report. + +### Customizing settings + +The Secret Detection scan settings can be changed through [environment variables](#available-variables) +by using the +[`variables`](../../../ci/yaml/README.md#variables) parameter in `.gitlab-ci.yml`. + +To override a job definition, (for example, change properties like `variables` or `dependencies`), +declare a job with the same name as the SAST job to override. Place this new job after the template +inclusion and specify any additional keys under it. + +In the following example, we include the Secret Detection template and at the same time we +override the `secret-scan` job with the `SECRET_DETECTION_HISTORIC_SCAN` variable to `true`: + +```yaml +include: + - template: Secret-Detection.gitlab-ci.yml + +secrets-scan: + variables: + SECRET_DETECTION_HISTORIC_SCAN: true +``` + +Because the template is [evaluated before](../../../ci/yaml/README.md#include) +the pipeline configuration, the last mention of the variable takes precedence. + +CAUTION: **Deprecation:** +Beginning in GitLab 13.0, the use of [`only` and `except`](../../../ci/yaml/README.md#onlyexcept-basic) +is no longer supported. When overriding the template, you must use [`rules`](../../../ci/yaml/README.md#rules) instead. + +#### Available variables + +Secret Detection can be customized by defining available variables: + +| Environment variable | Default value | Description | +|-------------------------|---------------|-------------| +| `SECRET_DETECTION_COMMIT_FROM` | - | The commit a Gitleaks scan starts at. | +| `SECRET_DETECTION_COMMIT_TO` | - | The commit a Gitleaks scan ends at. | +| `SECRET_DETECTION_HISTORIC_SCAN` | false | Flag to enable a historic Gitleaks scan. | + ## Full History Secret Scan GitLab 12.11 introduced support for scanning the full history of a repository. This new functionality @@ -62,7 +154,7 @@ want to perform a full secret scan. Running a secret scan on the full history ca especially for larger repositories with lengthy Git histories. We recommend not setting this variable as part of your normal job definition. -A new configuration variable ([`SAST_GITLEAKS_HISTORIC_SCAN`](../sast/#vulnerability-filters)) +A new configuration variable ([`SECRET_DETECTION_HISTORIC_SCAN`](../sast/#vulnerability-filters)) can be set to change the behavior of the GitLab Secret Detection scan to run on the entire Git history of a repository. We have created a [short video walkthrough](https://youtu.be/wDtc_K00Y0A) showcasing how you can perform a full history secret scan. diff --git a/doc/user/project/operations/alert_management.md b/doc/user/project/operations/alert_management.md index db4f0783a3b..bb4e7b44902 100644 --- a/doc/user/project/operations/alert_management.md +++ b/doc/user/project/operations/alert_management.md @@ -1,3 +1,9 @@ +--- +stage: Monitor +group: Health +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +--- + # Alert Management > [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2877) in GitLab 13.0. @@ -95,16 +101,13 @@ See [Alert Management statuses](#alert-management-statuses) for more details abo ### Update an Alert's assignee -NOTE: **Note:** -We currently only support a single assignee per alert. - -The Alert Management detail view allows users to update the Alert Assignee(s). - > [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3066) in GitLab 13.1. +The Alert Management detail view allows users to update the Alert assignee. + In large teams, where there is shared ownership of an alert, it can be difficult to track who is investigating and working on it. The Alert Management detail view -enables you to update the Alert Assignee(s): +enables you to update the Alert assignee: NOTE: **Note:** GitLab currently only supports a single assignee per alert. diff --git a/lib/api/project_container_repositories.rb b/lib/api/project_container_repositories.rb index f378b87d4b6..2a0099018d9 100644 --- a/lib/api/project_container_repositories.rb +++ b/lib/api/project_container_repositories.rb @@ -70,11 +70,11 @@ module API end params do requires :repository_id, type: Integer, desc: 'The ID of the repository' - optional :name_regex_delete, type: String, desc: 'The tag name regexp to delete, specify .* to delete all' - optional :name_regex, type: String, desc: 'The tag name regexp to delete, specify .* to delete all' + optional :name_regex_delete, type: String, untrusted_regexp: true, desc: 'The tag name regexp to delete, specify .* to delete all' + optional :name_regex, type: String, untrusted_regexp: true, desc: 'The tag name regexp to delete, specify .* to delete all' # require either name_regex (deprecated) or name_regex_delete, it is ok to have both at_least_one_of :name_regex, :name_regex_delete - optional :name_regex_keep, type: String, desc: 'The tag name regexp to retain' + optional :name_regex_keep, type: String, untrusted_regexp: true, desc: 'The tag name regexp to retain' optional :keep_n, type: Integer, desc: 'Keep n of latest tags with matching name' optional :older_than, type: String, desc: 'Delete older than: 1h, 1d, 1month' end diff --git a/lib/api/validations/validators/untrusted_regexp.rb b/lib/api/validations/validators/untrusted_regexp.rb new file mode 100644 index 00000000000..ec623684e67 --- /dev/null +++ b/lib/api/validations/validators/untrusted_regexp.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module API + module Validations + module Validators + class UntrustedRegexp < Grape::Validations::Base + def validate_param!(attr_name, params) + value = params[attr_name] + return unless value + + Gitlab::UntrustedRegexp.new(value) + rescue RegexpError => e + message = "is an invalid regexp: #{e.message}" + raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b1efcebd0b4..d6e74c0996d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9695,12 +9695,6 @@ msgstr "" msgid "FeatureFlags|Target environments" msgstr "" -msgid "FeatureFlags|There are no active feature flags" -msgstr "" - -msgid "FeatureFlags|There are no inactive feature flags" -msgstr "" - msgid "FeatureFlags|There was an error fetching the feature flags." msgstr "" @@ -13289,6 +13283,9 @@ msgstr "" msgid "List your Bitbucket Server repositories" msgstr "" +msgid "Lists" +msgstr "" + msgid "Live preview" msgstr "" @@ -26602,6 +26599,9 @@ msgstr "" msgid "created %{timeAgo}" msgstr "" +msgid "created %{timeago}" +msgstr "" + msgid "customize" msgstr "" diff --git a/qa/qa/specs/features/browser_ui/7_configure/kubernetes/kubernetes_integration_spec.rb b/qa/qa/specs/features/browser_ui/7_configure/kubernetes/kubernetes_integration_spec.rb index 04c68598239..0e9c369e97f 100644 --- a/qa/qa/specs/features/browser_ui/7_configure/kubernetes/kubernetes_integration_spec.rb +++ b/qa/qa/specs/features/browser_ui/7_configure/kubernetes/kubernetes_integration_spec.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true module QA - context 'Configure', quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/209085', type: :investigating } do - describe 'Kubernetes Cluster Integration', :orchestrated, :kubernetes, :requires_admin do + context 'Configure' do + describe 'Kubernetes Cluster Integration', :orchestrated, :kubernetes, :requires_admin, :skip_live_env do context 'Project Clusters' do - let(:cluster) { Service::KubernetesCluster.new(provider_class: Service::ClusterProvider::K3s).create! } + let!(:cluster) { Service::KubernetesCluster.new(provider_class: Service::ClusterProvider::K3s).create! } let(:project) do Resource::Project.fabricate_via_api! do |project| project.name = 'project-with-k8s' @@ -13,7 +13,7 @@ module QA end before do - Flow::Login.sign_in + Flow::Login.sign_in_as_admin end after do diff --git a/spec/lib/api/validations/validators/untrusted_regexp_spec.rb b/spec/lib/api/validations/validators/untrusted_regexp_spec.rb new file mode 100644 index 00000000000..491bf94fd79 --- /dev/null +++ b/spec/lib/api/validations/validators/untrusted_regexp_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::Validations::Validators::UntrustedRegexp do + include ApiValidatorsHelpers + + subject do + described_class.new(['test'], {}, false, scope.new) + end + + context 'valid regex' do + it 'does not raise a validation error' do + expect_no_validation_error('test' => 'test') + expect_no_validation_error('test' => '.*') + expect_no_validation_error('test' => Gitlab::Regex.environment_name_regex_chars) + end + end + + context 'invalid regex' do + it 'raises a validation error' do + expect_validation_error('test' => '[') + expect_validation_error('test' => '*foobar') + expect_validation_error('test' => '?foobar') + expect_validation_error('test' => '\A[^/%\s]+(..\z') + end + end +end diff --git a/spec/models/container_expiration_policy_spec.rb b/spec/models/container_expiration_policy_spec.rb index 7d9c7638897..588685b04bf 100644 --- a/spec/models/container_expiration_policy_spec.rb +++ b/spec/models/container_expiration_policy_spec.rb @@ -103,4 +103,14 @@ RSpec.describe ContainerExpirationPolicy, type: :model do end end end + + describe '#disable!' do + let_it_be(:container_expiration_policy) { create(:container_expiration_policy) } + + subject { container_expiration_policy.disable! } + + it 'disables the container expiration policy' do + expect { subject }.to change { container_expiration_policy.reload.enabled }.from(true).to(false) + end + end end diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb index 91905635c3f..471fc99117b 100644 --- a/spec/requests/api/project_container_repositories_spec.rb +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -223,6 +223,40 @@ describe API::ProjectContainerRepositories do expect(response).to have_gitlab_http_status(:accepted) end end + + context 'with invalid regex' do + let(:invalid_regex) { '*v10.' } + let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" } + + RSpec.shared_examples 'rejecting the invalid regex' do |param_name| + it 'does not enqueue a job' do + expect(CleanupContainerRepositoryWorker).not_to receive(:perform_async) + + subject + end + + it_behaves_like 'returning response status', :bad_request + + it 'returns an error message' do + subject + + expect(json_response['error']).to include("#{param_name} is an invalid regexp") + end + end + + before do + stub_last_activity_update + stub_exclusive_lease(lease_key, timeout: 1.hour) + end + + %i[name_regex_delete name_regex name_regex_keep].each do |param_name| + context "for #{param_name}" do + let(:params) { { param_name => invalid_regex } } + + it_behaves_like 'rejecting the invalid regex', param_name + end + end + end end end diff --git a/spec/services/container_expiration_policy_service_spec.rb b/spec/services/container_expiration_policy_service_spec.rb index b2f2b2e1236..97715b990ef 100644 --- a/spec/services/container_expiration_policy_service_spec.rb +++ b/spec/services/container_expiration_policy_service_spec.rb @@ -27,5 +27,20 @@ describe ContainerExpirationPolicyService do expect(container_expiration_policy.next_run_at).to be > Time.zone.now end + + context 'with an invalid container expiration policy' do + before do + allow(container_expiration_policy).to receive(:valid?).and_return(false) + end + + it 'disables it' do + expect(container_expiration_policy).not_to receive(:schedule_next_run!) + expect(CleanupContainerRepositoryWorker).not_to receive(:perform_async) + + expect { subject } + .to change { container_expiration_policy.reload.enabled }.from(true).to(false) + .and raise_error(ContainerExpirationPolicyService::InvalidPolicyError) + end + end end end diff --git a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb index 01f09f208fd..11ea7d51673 100644 --- a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb +++ b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' describe Projects::ContainerRepository::CleanupTagsService do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :private) } + let_it_be(:project, reload: true) { create(:project, :private) } let_it_be(:repository) { create(:container_repository, :root, project: project) } let(:service) { described_class.new(project, user, params) } @@ -72,6 +72,47 @@ describe Projects::ContainerRepository::CleanupTagsService do end end + context 'with invalid regular expressions' do + RSpec.shared_examples 'handling an invalid regex' do + it 'keeps all tags' do + expect(Projects::ContainerRepository::DeleteTagsService) + .not_to receive(:new) + subject + end + + it 'returns an error' do + response = subject + + expect(response[:status]).to eq(:error) + expect(response[:message]).to eq('invalid regex') + end + + it 'calls error tracking service' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).and_call_original + + subject + end + end + + context 'when name_regex_delete is invalid' do + let(:params) { { 'name_regex_delete' => '*test*' } } + + it_behaves_like 'handling an invalid regex' + end + + context 'when name_regex is invalid' do + let(:params) { { 'name_regex' => '*test*' } } + + it_behaves_like 'handling an invalid regex' + end + + context 'when name_regex_keep is invalid' do + let(:params) { { 'name_regex_keep' => '*test*' } } + + it_behaves_like 'handling an invalid regex' + end + end + context 'when delete regex matching specific tags is used' do let(:params) do { 'name_regex_delete' => 'C|D' } diff --git a/spec/workers/container_expiration_policy_worker_spec.rb b/spec/workers/container_expiration_policy_worker_spec.rb index 48ab1614633..b15a28dcdca 100644 --- a/spec/workers/container_expiration_policy_worker_spec.rb +++ b/spec/workers/container_expiration_policy_worker_spec.rb @@ -53,5 +53,22 @@ describe ContainerExpirationPolicyWorker do subject end end + + context 'an invalid policy' do + let_it_be(:container_expiration_policy) { create(:container_expiration_policy, :runnable) } + let_it_be(:user) {container_expiration_policy.project.owner } + + before do + container_expiration_policy.update_column(:name_regex, '*production') + end + + it 'runs the policy and tracks an error' do + expect(ContainerExpirationPolicyService) + .to receive(:new).with(container_expiration_policy.project, user).and_call_original + expect(Gitlab::ErrorTracking).to receive(:log_exception).with(instance_of(ContainerExpirationPolicyService::InvalidPolicyError), container_expiration_policy_id: container_expiration_policy.id) + + expect { subject }.to change { container_expiration_policy.reload.enabled }.from(true).to(false) + end + end end end |