diff --git a/.rubocop_todo/rspec/expect_in_hook.yml b/.rubocop_todo/rspec/expect_in_hook.yml index c7d161acaabb48d84e8d9c245dba91ff5b122c9e..efa76b195b2452a452ebf50637ba02ff2c4ae274 100644 --- a/.rubocop_todo/rspec/expect_in_hook.yml +++ b/.rubocop_todo/rspec/expect_in_hook.yml @@ -243,7 +243,6 @@ RSpec/ExpectInHook: - 'spec/lib/gitlab/middleware/memory_report_spec.rb' - 'spec/lib/gitlab/middleware/multipart_spec.rb' - 'spec/lib/gitlab/omniauth_initializer_spec.rb' - - 'spec/lib/gitlab/pages/deployment_update_spec.rb' - 'spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb' - 'spec/lib/gitlab/patch/database_config_spec.rb' - 'spec/lib/gitlab/project_search_results_spec.rb' diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index 6baa1cbeba2a30749562c1f18ecb502a7829efc8..eadce30e199c3fec3dc21e3394c3b97ced0bf8a0 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -612,7 +612,6 @@ RSpec/FeatureCategory: - 'ee/spec/lib/ee/gitlab/metrics/samplers/database_sampler_spec.rb' - 'ee/spec/lib/ee/gitlab/middleware/read_only_spec.rb' - 'ee/spec/lib/ee/gitlab/omniauth_initializer_spec.rb' - - 'ee/spec/lib/ee/gitlab/pages/deployment_update_spec.rb' - 'ee/spec/lib/ee/gitlab/repo_path_spec.rb' - 'ee/spec/lib/ee/gitlab/repository_size_checker_spec.rb' - 'ee/spec/lib/ee/gitlab/scim/attribute_transform_spec.rb' @@ -3704,7 +3703,6 @@ RSpec/FeatureCategory: - 'spec/lib/gitlab/optimistic_locking_spec.rb' - 'spec/lib/gitlab/other_markup_spec.rb' - 'spec/lib/gitlab/otp_key_rotator_spec.rb' - - 'spec/lib/gitlab/pages/deployment_update_spec.rb' - 'spec/lib/gitlab/pages/settings_spec.rb' - 'spec/lib/gitlab/pages_spec.rb' - 'spec/lib/gitlab/pagination/cursor_based_keyset_spec.rb' diff --git a/.rubocop_todo/rspec/named_subject.yml b/.rubocop_todo/rspec/named_subject.yml index 789a831593f6a7af86e3747607c58ef3556ba673..3c419a41447bce120a420850eda2e44a1dcba1e6 100644 --- a/.rubocop_todo/rspec/named_subject.yml +++ b/.rubocop_todo/rspec/named_subject.yml @@ -322,7 +322,6 @@ RSpec/NamedSubject: - 'ee/spec/lib/ee/gitlab/import_export/wiki_repo_saver_spec.rb' - 'ee/spec/lib/ee/gitlab/issuable/clone/copy_resource_events_service_spec.rb' - 'ee/spec/lib/ee/gitlab/metrics/samplers/database_sampler_spec.rb' - - 'ee/spec/lib/ee/gitlab/pages/deployment_update_spec.rb' - 'ee/spec/lib/ee/gitlab/repository_size_checker_spec.rb' - 'ee/spec/lib/ee/gitlab/snippet_search_results_spec.rb' - 'ee/spec/lib/ee/gitlab/url_builder_spec.rb' diff --git a/.rubocop_todo/style/guard_clause.yml b/.rubocop_todo/style/guard_clause.yml index d50a0747de5489c4947ca83c2d8d8166e3da8b88..a0632c06dfe7b9eca2ba24f61e44f526190690b1 100644 --- a/.rubocop_todo/style/guard_clause.yml +++ b/.rubocop_todo/style/guard_clause.yml @@ -490,7 +490,6 @@ Style/GuardClause: - 'lib/gitlab/metrics/subscribers/external_http.rb' - 'lib/gitlab/metrics/web_transaction.rb' - 'lib/gitlab/middleware/read_only/controller.rb' - - 'lib/gitlab/pages/deployment_update.rb' - 'lib/gitlab/pagination/gitaly_keyset_pager.rb' - 'lib/gitlab/pagination/keyset/column_order_definition.rb' - 'lib/gitlab/pagination/keyset/in_operator_optimization/array_scope_columns.rb' diff --git a/.rubocop_todo/style/if_unless_modifier.yml b/.rubocop_todo/style/if_unless_modifier.yml index fa948e474bf7d412ec11f7dbb2f1e89db17cdaf8..e7ef37d06258664caa409cba1993888d8ef07e7c 100644 --- a/.rubocop_todo/style/if_unless_modifier.yml +++ b/.rubocop_todo/style/if_unless_modifier.yml @@ -724,7 +724,6 @@ Style/IfUnlessModifier: - 'lib/gitlab/middleware/same_site_cookies.rb' - 'lib/gitlab/object_hierarchy.rb' - 'lib/gitlab/omniauth_initializer.rb' - - 'lib/gitlab/pages/deployment_update.rb' - 'lib/gitlab/pagination/keyset/in_operator_optimization/query_builder.rb' - 'lib/gitlab/patch/database_config.rb' - 'lib/gitlab/patch/prependable.rb' diff --git a/.rubocop_todo/style/inline_disable_annotation.yml b/.rubocop_todo/style/inline_disable_annotation.yml index 290cc23f831668f3f338dbb4dce9c397589e3045..32bb9ed8ed6e467eba4565d9334088637b8e4088 100644 --- a/.rubocop_todo/style/inline_disable_annotation.yml +++ b/.rubocop_todo/style/inline_disable_annotation.yml @@ -1935,7 +1935,6 @@ Style/InlineDisableAnnotation: - 'ee/spec/lib/ee/gitlab/database/docs/docs_spec.rb' - 'ee/spec/lib/ee/gitlab/import_export/repo_restorer_spec.rb' - 'ee/spec/lib/ee/gitlab/issuable_metadata_spec.rb' - - 'ee/spec/lib/ee/gitlab/pages/deployment_update_spec.rb' - 'ee/spec/lib/ee/gitlab/saas_spec.rb' - 'ee/spec/lib/elastic/latest/project_instance_proxy_spec.rb' - 'ee/spec/lib/gitlab/analytics/value_stream_dashboard/namespace_cursor_spec.rb' @@ -2871,7 +2870,6 @@ Style/InlineDisableAnnotation: - 'spec/lib/gitlab/memory/watchdog/configurator_spec.rb' - 'spec/lib/gitlab/memory/watchdog/handlers/puma_handler_spec.rb' - 'spec/lib/gitlab/merge_requests/message_generator_spec.rb' - - 'spec/lib/gitlab/pages/deployment_update_spec.rb' - 'spec/lib/gitlab/pagination/keyset/iterator_spec.rb' - 'spec/lib/gitlab/pagination/keyset/order_spec.rb' - 'spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb' diff --git a/.rubocop_todo/style/string_concatenation.yml b/.rubocop_todo/style/string_concatenation.yml index 914f7a32f9a3c701fe4f1516cf656cbd9421d3df..2690ffca9e6c98d6cf907692d6d9fdf16bfba169 100644 --- a/.rubocop_todo/style/string_concatenation.yml +++ b/.rubocop_todo/style/string_concatenation.yml @@ -104,7 +104,6 @@ Style/StringConcatenation: - 'lib/gitlab/kubernetes/kubectl_cmd.rb' - 'lib/gitlab/lfs/client.rb' - 'lib/gitlab/logger.rb' - - 'lib/gitlab/pages/deployment_update.rb' - 'lib/gitlab/path_regex.rb' - 'lib/gitlab/prometheus/internal.rb' - 'lib/gitlab/quick_actions/extractor.rb' diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 83b28840d396d227729c6068301f089c4b5e5438..c424ee477aafbb653420aa9ee6b231b76d85a7de 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -9,12 +9,12 @@ class UpdatePagesService < BaseService # 10 minutes is enough, but 30 feels safer OLD_DEPLOYMENTS_DESTRUCTION_DELAY = 30.minutes - attr_reader :build, :deployment_update + attr_reader :build, :deployment_validations def initialize(project, build) @project = project @build = build - @deployment_update = ::Gitlab::Pages::DeploymentUpdate.new(project, build) + @deployment_validations = ::Gitlab::Pages::DeploymentValidations.new(project, build) end def execute @@ -25,13 +25,13 @@ def execute job.run! end - return error(deployment_update.errors.first.full_message) unless deployment_update.valid? + return error(deployment_validations.errors.first.full_message) unless deployment_validations.valid? build.artifacts_file.use_file do |artifacts_path| deployment = create_pages_deployment(artifacts_path, build) break error('The uploaded artifact size does not match the expected value') unless deployment - break error(deployment_update.errors.first.full_message) unless deployment_update.valid? + break error(deployment_validations.errors.first.full_message) unless deployment_validations.valid? deactive_old_deployments(deployment) success @@ -52,7 +52,7 @@ def success def error(message) register_failure log_error("Projects::UpdatePagesService: #{message}") - commit_status.allow_failure = !deployment_update.latest? + commit_status.allow_failure = !deployment_validations.latest_build? commit_status.description = message commit_status.drop(:script_failure) super @@ -95,7 +95,7 @@ def create_pages_deployment(artifacts_path, build) def pages_deployment_attributes(file, build) { file: file, - file_count: deployment_update.entries_count, + file_count: deployment_validations.entries_count, file_sha256: build.job_artifacts_archive.file_sha256, ci_build_id: build.id, root_directory: build.options[:publish] diff --git a/ee/lib/ee/gitlab/pages/deployment_update.rb b/ee/lib/ee/gitlab/pages/deployment_validations.rb similarity index 91% rename from ee/lib/ee/gitlab/pages/deployment_update.rb rename to ee/lib/ee/gitlab/pages/deployment_validations.rb index 028db93e52ea2866abcd0e05456255c720dc77d1..9be6fd8c03b91fc59cf2200e6446413ce2ddfcb3 100644 --- a/ee/lib/ee/gitlab/pages/deployment_update.rb +++ b/ee/lib/ee/gitlab/pages/deployment_validations.rb @@ -3,7 +3,7 @@ module EE module Gitlab module Pages - module DeploymentUpdate + module DeploymentValidations extend ::Gitlab::Utils::Override override :max_size_from_settings diff --git a/ee/spec/lib/ee/gitlab/pages/deployment_update_spec.rb b/ee/spec/lib/ee/gitlab/pages/deployment_update_spec.rb deleted file mode 100644 index 504a83f4d3310742ada42b1284ab2200483cd0fc..0000000000000000000000000000000000000000 --- a/ee/spec/lib/ee/gitlab/pages/deployment_update_spec.rb +++ /dev/null @@ -1,82 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe Gitlab::Pages::DeploymentUpdate do - let(:group) { create(:group, :nested, max_pages_size: 200) } - let(:project) { create(:project, :repository, namespace: group, max_pages_size: 250) } - let(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) } - let(:build) { create(:ci_build, pipeline: pipeline, ref: 'HEAD') } - - subject { described_class.new(project, build) } - - describe 'maximum pages artifacts size' do - let(:metadata) { spy('metadata') } # rubocop: disable RSpec/VerifiedDoubles - - before do - file = fixture_file_upload('spec/fixtures/pages.zip') - metafile = fixture_file_upload('spec/fixtures/pages.zip.meta') - - create(:ci_job_artifact, :archive, :correct_checksum, file: file, job: build) - create(:ci_job_artifact, :metadata, file: metafile, job: build) - - allow(build) - .to receive(:artifacts_metadata_entry) - .and_return(metadata) - - stub_licensed_features(pages_size_limit: true) - end - - context "when size is below the limit" do - before do - allow(metadata).to receive(:total_size).and_return(249.megabyte) - allow(metadata).to receive(:entries).and_return([]) - end - - it 'is valid' do - expect(subject).to be_valid - end - end - - context "when size is above the limit" do - before do - allow(metadata).to receive(:total_size).and_return(251.megabyte) - allow(metadata).to receive(:entries).and_return([]) - end - - it 'is invalid' do - expect(subject).not_to be_valid - expect(subject.errors.full_messages).to include('artifacts for pages are too large: 263192576') - end - end - - context 'when pages_size_limit feature is not available' do - before do - stub_licensed_features(pages_size_limit: false) - end - - context "when size is below the limit" do - before do - allow(metadata).to receive(:total_size).and_return(99.megabyte) - allow(metadata).to receive(:entries).and_return([]) - end - - it 'is valid' do - expect(subject).to be_valid - end - end - - context "when size is above the limit" do - before do - allow(metadata).to receive(:total_size).and_return(101.megabyte) - allow(metadata).to receive(:entries).and_return([]) - end - - it 'is invalid' do - expect(subject).not_to be_valid - expect(subject.errors.full_messages).to include('artifacts for pages are too large: 105906176') - end - end - end - end -end diff --git a/ee/spec/lib/ee/gitlab/pages/deployment_validations_spec.rb b/ee/spec/lib/ee/gitlab/pages/deployment_validations_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..cea95c274b9dacf54145628593d4c20f02ca134c --- /dev/null +++ b/ee/spec/lib/ee/gitlab/pages/deployment_validations_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Gitlab::Pages::DeploymentValidations, feature_category: :pages do + let_it_be(:group) { create(:group, :nested, max_pages_size: 200) } + let_it_be(:project) { create(:project, :repository, namespace: group, max_pages_size: 250) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit("HEAD").sha) } + + let(:build_options) { {} } + let(:build) { create(:ci_build, ref: "HEAD", name: 'pages', pipeline: pipeline, options: build_options) } + + let(:file) { fixture_file_upload("spec/fixtures/pages.zip") } + let(:metafile) { fixture_file_upload("spec/fixtures/pages.zip.meta") } + + let(:metadata) do + instance_double( + ::Gitlab::Ci::Build::Artifacts::Metadata::Entry, + entries: [], + total_size: 50.megabyte + ) + end + + subject(:validations) { described_class.new(project, build) } + + before do + stub_pages_setting(enabled: true) + create(:ci_job_artifact, :archive, :correct_checksum, file: file, job: build) + create(:ci_job_artifact, :metadata, file: metafile, job: build) + + allow(build) + .to receive(:artifacts_metadata_entry) + .and_return(metadata) + end + + shared_examples "valid pages deployment" do + specify do + expect(validations).to be_valid + end + end + + shared_examples "invalid pages deployment" do |message:| + specify do + expect(validations).not_to be_valid + expect(validations.errors.full_messages).to include(message) + end + end + + describe "maximum pages artifacts size" do + context "when pages_size_limit feature is available" do + before do + stub_licensed_features(pages_size_limit: true) + end + + context "when size is below the limit" do + before do + allow(metadata).to receive(:total_size).and_return(249.megabyte) + end + + include_examples "valid pages deployment" + end + + context "when size is above the limit" do + before do + allow(metadata).to receive(:total_size).and_return(251.megabyte) + end + + include_examples "invalid pages deployment", + message: "artifacts for pages are too large: 263192576" + end + end + + context "when pages_size_limit feature is not available" do + before do + stub_licensed_features(pages_size_limit: false) + end + + context "when size is below the limit" do + before do + allow(metadata).to receive(:total_size).and_return(99.megabyte) + end + + include_examples "valid pages deployment" + end + + context "when size is above the limit" do + before do + allow(metadata).to receive(:total_size).and_return(101.megabyte) + end + + include_examples "invalid pages deployment", + message: "artifacts for pages are too large: 105906176" + end + end + end +end diff --git a/lib/gitlab/pages/deployment_update.rb b/lib/gitlab/pages/deployment_validations.rb similarity index 52% rename from lib/gitlab/pages/deployment_update.rb rename to lib/gitlab/pages/deployment_validations.rb index a572a59b2f5169ab6fef88b8d70ad8393618151f..98c61b9e321731be32a608cc9c98af822acb4337 100644 --- a/lib/gitlab/pages/deployment_update.rb +++ b/lib/gitlab/pages/deployment_validations.rb @@ -2,23 +2,26 @@ module Gitlab module Pages - class DeploymentUpdate + class DeploymentValidations include ActiveModel::Validations + include ::Gitlab::Utils::StrongMemoize PUBLIC_DIR = 'public' - validate :validate_state, unless: -> { errors.any? } - validate :validate_outdated_sha, unless: -> { errors.any? } - validate :validate_max_size, unless: -> { errors.any? } - validate :validate_public_folder, unless: -> { errors.any? } - validate :validate_max_entries, unless: -> { errors.any? } + with_options unless: -> { errors.any? } do + validate :validate_state + validate :validate_outdated_sha + validate :validate_max_size + validate :validate_public_folder + validate :validate_max_entries + end def initialize(project, build) @project = project @build = build end - def latest? + def latest_build? # check if sha for the ref is still the most recent one # this helps in case when multiple deployments happens sha == latest_sha @@ -28,8 +31,9 @@ def entries_count # we're using the full archive and pages daemon needs to read it # so we want the total count from entries, not only "public/" directory # because it better approximates work we need to do before we can serve the site - @entries_count = build.artifacts_metadata_entry("", recursive: true).entries.count + build.artifacts_metadata_entry("", recursive: true).entries.count end + strong_memoize_attr :entries_count private @@ -41,22 +45,16 @@ def validate_state end def validate_max_size - if total_size > max_size - errors.add(:base, "artifacts for pages are too large: #{total_size}") - end - end + return if total_size <= max_size - def root_dir - build.options[:publish] || PUBLIC_DIR + errors.add(:base, "artifacts for pages are too large: #{total_size}") end # Calculate page size after extract def total_size - @total_size ||= build.artifacts_metadata_entry("#{root_dir}/", recursive: true).total_size - end + root_dir = build.options[:publish] || PUBLIC_DIR - def max_size_from_settings - Gitlab::CurrentSettings.max_pages_size.megabytes + build.artifacts_metadata_entry("#{root_dir}/", recursive: true).total_size end def max_size @@ -68,30 +66,27 @@ def max_size end def validate_max_entries - if pages_file_entries_limit > 0 && entries_count > pages_file_entries_limit - errors.add( - :base, - "pages site contains #{entries_count} file entries, while limit is set to #{pages_file_entries_limit}" - ) - end + pages_file_entries_limit = project.actual_limits.pages_file_entries + return unless pages_file_entries_limit > 0 && entries_count > pages_file_entries_limit + + errors.add( + :base, + "pages site contains #{entries_count} file entries, while limit is set to #{pages_file_entries_limit}" + ) end def validate_public_folder - if total_size <= 0 - errors.add( - :base, - 'Error: You need to either include a `public/` folder in your artifacts, or specify ' \ - 'which one to use for Pages using `publish` in `.gitlab-ci.yml`') - end - end + return if total_size > 0 - def pages_file_entries_limit - project.actual_limits.pages_file_entries + errors.add( + :base, + 'Error: You need to either include a `public/` folder in your artifacts, or specify ' \ + 'which one to use for Pages using `publish` in `.gitlab-ci.yml`') end # If a newer pipeline already build a PagesDeployment def validate_outdated_sha - return if latest? + return if latest_build? return if latest_pipeline_id.blank? return if latest_pipeline_id <= build.pipeline_id @@ -105,18 +100,23 @@ def latest_sha project.cleanup end - def sha - build.sha - end - def latest_pipeline_id project .active_pages_deployments - .with_path_prefix(build.pages&.dig(:path_prefix)) + .with_path_prefix(path_prefix) .latest_pipeline_id end + + # overridden in EE + def max_size_from_settings = Gitlab::CurrentSettings.max_pages_size.megabytes + + def path_prefix = build.pages&.fetch(:path_prefix, '') + strong_memoize_attr :path_prefix + + def sha = build.sha + strong_memoize_attr :sha end end end -Gitlab::Pages::DeploymentUpdate.prepend_mod_with('Gitlab::Pages::DeploymentUpdate') +Gitlab::Pages::DeploymentValidations.prepend_mod_with('Gitlab::Pages::DeploymentValidations') diff --git a/spec/lib/gitlab/pages/deployment_update_spec.rb b/spec/lib/gitlab/pages/deployment_update_spec.rb deleted file mode 100644 index 48f5b27b7614731a424f53b7cad146aa24a07dca..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/pages/deployment_update_spec.rb +++ /dev/null @@ -1,171 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Pages::DeploymentUpdate, feature_category: :pages do - let_it_be(:project, refind: true) { create(:project, :repository) } - - let_it_be(:old_pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD~~').sha) } - let_it_be(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD~').sha) } - - let(:build) { create(:ci_build, pipeline: pipeline, ref: 'HEAD') } - let(:invalid_file) { fixture_file_upload('spec/fixtures/dk.png') } - - let(:file) { fixture_file_upload("spec/fixtures/pages.zip") } - let(:empty_file) { fixture_file_upload("spec/fixtures/pages_empty.zip") } - let(:empty_metadata_filename) { "spec/fixtures/pages_empty.zip.meta" } - let(:metadata_filename) { "spec/fixtures/pages.zip.meta" } - let(:metadata) { fixture_file_upload(metadata_filename) if File.exist?(metadata_filename) } - - subject(:pages_deployment_update) { described_class.new(project, build) } - - context 'for new artifacts' do - context 'for a valid job' do - let!(:artifacts_archive) { create(:ci_job_artifact, :correct_checksum, file: file, job: build) } - - before do - create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build) - - build.reload - end - - it 'is valid' do - expect(pages_deployment_update).to be_valid - end - - context 'when missing artifacts metadata' do - before do - expect(build).to receive(:artifacts_metadata?).and_return(false) - end - - it 'is invalid' do - expect(pages_deployment_update).not_to be_valid - expect(pages_deployment_update.errors.full_messages).to include('missing artifacts metadata') - end - end - end - - it 'is invalid for invalid archive' do - create(:ci_job_artifact, :archive, file: invalid_file, job: build) - - expect(pages_deployment_update).not_to be_valid - expect(pages_deployment_update.errors.full_messages).to include('missing artifacts metadata') - end - end - - describe 'maximum pages artifacts size' do - let(:metadata) { spy('metadata') } # rubocop: disable RSpec/VerifiedDoubles - - before do - file = fixture_file_upload('spec/fixtures/pages.zip') - metafile = fixture_file_upload('spec/fixtures/pages.zip.meta') - - create(:ci_job_artifact, :archive, :correct_checksum, file: file, job: build) - create(:ci_job_artifact, :metadata, file: metafile, job: build) - - allow(build).to receive(:artifacts_metadata_entry) - .and_return(metadata) - end - - context 'when maximum pages size is set to zero' do - before do - stub_application_setting(max_pages_size: 0) - end - - context "when size is above the limit" do - before do - allow(metadata).to receive(:total_size).and_return(1.megabyte) - allow(metadata).to receive(:entries).and_return([]) - end - - it 'is valid' do - expect(pages_deployment_update).to be_valid - end - end - end - - context 'when size is limited on the instance level' do - before do - stub_application_setting(max_pages_size: 100) - end - - context "when size is below the limit" do - before do - allow(metadata).to receive(:total_size).and_return(1.megabyte) - allow(metadata).to receive(:entries).and_return([]) - end - - it 'is valid' do - expect(pages_deployment_update).to be_valid - end - end - - context "when size is above the limit" do - before do - allow(metadata).to receive(:total_size).and_return(101.megabyte) - allow(metadata).to receive(:entries).and_return([]) - end - - it 'is invalid' do - expect(pages_deployment_update).not_to be_valid - expect(pages_deployment_update.errors.full_messages) - .to include('artifacts for pages are too large: 105906176') - end - end - end - end - - context 'when retrying the job' do - let!(:older_deploy_job) do - create( - :generic_commit_status, - :failed, - pipeline: pipeline, - ref: build.ref, - stage: 'deploy', - name: 'pages:deploy' - ) - end - - before do - create(:ci_job_artifact, :correct_checksum, file: file, job: build) - create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build) - build.reload - end - - it 'marks older pages:deploy jobs retried' do - expect(pages_deployment_update).to be_valid - end - end - - context 'when validating if current build is outdated' do - before do - create(:ci_job_artifact, :correct_checksum, file: file, job: build) - create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build) - build.reload - end - - context 'when there is NOT a newer build' do - it 'does not fail' do - expect(pages_deployment_update).to be_valid - end - end - - context 'when there is a newer build' do - before do - new_pipeline = create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) - new_build = create(:ci_build, name: 'pages', pipeline: new_pipeline, ref: 'HEAD') - create(:ci_job_artifact, :correct_checksum, file: file, job: new_build) - create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: new_build) - create(:pages_deployment, project: project, ci_build: new_build) - new_build.reload - end - - it 'fails with outdated reference message' do - expect(pages_deployment_update).not_to be_valid - expect(pages_deployment_update.errors.full_messages) - .to include('build SHA is outdated for this ref') - end - end - end -end diff --git a/spec/lib/gitlab/pages/deployment_validations_spec.rb b/spec/lib/gitlab/pages/deployment_validations_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e7ab18c36502ba4ffcd6e21c6e5425d1c2ce27a6 --- /dev/null +++ b/spec/lib/gitlab/pages/deployment_validations_spec.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Gitlab::Pages::DeploymentValidations, feature_category: :pages do + let_it_be(:project, refind: true) { create(:project, :repository) } + + let_it_be(:old_pipeline) { create(:ci_pipeline, project: project, sha: project.commit("HEAD~~").sha) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit("HEAD~").sha) } + + let(:build_options) { {} } + let(:build) { create(:ci_build, name: "pages", ref: "HEAD", pipeline: pipeline, options: build_options) } + + let(:invalid_file) { fixture_file_upload("spec/fixtures/dk.png") } + let(:file) { fixture_file_upload("spec/fixtures/pages.zip") } + let(:metadata) { fixture_file_upload("spec/fixtures/pages.zip.meta") } + + subject(:validations) { described_class.new(project, build) } + + before do + stub_pages_setting(enabled: true) + end + + def add_build_artifacts! + create(:ci_job_artifact, :correct_checksum, file: file, job: build) + create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build) + build.reload + end + + shared_examples "valid pages deployment" do + specify do + expect(validations.valid?).to eq(true) + end + end + + shared_examples "invalid pages deployment" do |message:| + specify do + expect(validations.valid?).to eq(false) + expect(validations.errors.full_messages).to include(message) + end + end + + context "for new artifacts" do + context "for a valid job" do + before do + add_build_artifacts! + end + + include_examples "valid pages deployment" + + context "when missing artifacts metadata" do + before do + allow(build).to receive(:artifacts_metadata?).and_return(false) + end + + include_examples "invalid pages deployment", + message: "missing artifacts metadata" + end + end + + context "for an invalid artifact archive" do + before do + create(:ci_job_artifact, :archive, file: invalid_file, job: build) + end + + include_examples "invalid pages deployment", + message: "missing artifacts metadata" + end + end + + describe "maximum pages artifacts size" do + before do + add_build_artifacts! + + allow(build) + .to receive(:artifacts_metadata_entry) + .and_return(metadata) + end + + context "when maximum pages size is set to zero" do + before do + stub_application_setting(max_pages_size: 0) + end + + context "when size is above the limit" do + before do + allow(metadata).to receive(:total_size).and_return(1.megabyte) + allow(metadata).to receive(:entries).and_return([]) + end + + include_examples "valid pages deployment" + end + end + + context "when size is limited on the instance level" do + before do + stub_application_setting(max_pages_size: 100) + end + + context "when size is below the limit" do + before do + allow(metadata).to receive(:total_size).and_return(1.megabyte) + allow(metadata).to receive(:entries).and_return([]) + end + + include_examples "valid pages deployment" + end + + context "when size is above the limit" do + before do + allow(metadata).to receive(:total_size).and_return(101.megabyte) + allow(metadata).to receive(:entries).and_return([]) + end + + include_examples "invalid pages deployment", + message: "artifacts for pages are too large: 105906176" + end + end + end + + context "when retrying the job" do + let!(:older_deploy_job) do + create( + :generic_commit_status, + :failed, + pipeline: pipeline, + ref: build.ref, + stage: "deploy", + name: "pages:deploy" + ) + end + + before do + create(:ci_job_artifact, :correct_checksum, file: file, job: build) + create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build) + build.reload + end + + include_examples "valid pages deployment" + end + + context "when validating if current build is outdated" do + before do + create(:ci_job_artifact, :correct_checksum, file: file, job: build) + create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build) + build.reload + end + + context "when there is NOT a newer build" do + include_examples "valid pages deployment" + end + + context "when there is a newer build" do + before do + new_pipeline = create(:ci_pipeline, project: project, sha: project.commit("HEAD").sha) + new_build = create(:ci_build, name: "pages", pipeline: new_pipeline, ref: "HEAD") + create(:ci_job_artifact, :correct_checksum, file: file, job: new_build) + create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: new_build) + create(:pages_deployment, project: project, ci_build: new_build) + new_build.reload + end + + include_examples "invalid pages deployment", + message: "build SHA is outdated for this ref" + end + end +end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index c103a96552a64657695585552faac08144a0105a..0d4fc54733ab85ec715c643632e9c3089bad39c0 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -1083,7 +1083,6 @@ - './ee/spec/lib/ee/gitlab/middleware/read_only_spec.rb' - './ee/spec/lib/ee/gitlab/namespace_storage_size_error_message_spec.rb' - './ee/spec/lib/ee/gitlab/omniauth_initializer_spec.rb' -- './ee/spec/lib/ee/gitlab/pages/deployment_update_spec.rb' - './ee/spec/lib/ee/gitlab/rack_attack/request_spec.rb' - './ee/spec/lib/ee/gitlab/repo_path_spec.rb' - './ee/spec/lib/ee/gitlab/repository_size_checker_spec.rb' @@ -6373,7 +6372,6 @@ - './spec/lib/gitlab/optimistic_locking_spec.rb' - './spec/lib/gitlab/other_markup_spec.rb' - './spec/lib/gitlab/otp_key_rotator_spec.rb' -- './spec/lib/gitlab/pages/deployment_update_spec.rb' - './spec/lib/gitlab/pages/settings_spec.rb' - './spec/lib/gitlab/pages_spec.rb' - './spec/lib/gitlab/pagination/cursor_based_keyset_spec.rb'