From e83a5eaa44f547c00bda2b604a9a849d9c5fa72c Mon Sep 17 00:00:00 2001 From: Kassio Borges <kassioborgesm@gmail.com> Date: Wed, 28 Jun 2023 16:09:53 +0100 Subject: [PATCH] Move GitLab Pages URL logic to a single place To avoid spreading GitLab Pages URL logic around the monolith, move the existing logic from `models/project.rb` and `models/ci/artifact_blob.rb` to `lib/gitlab/pages/url_builder.rb`. Related to: https://docs.gitlab.com/ee/development/software_design.html#taming-omniscient-classes Changelog: other --- app/helpers/projects/pages_helper.rb | 12 + app/models/ci/artifact_blob.rb | 25 +- app/models/pages/lookup_path.rb | 11 +- app/models/project.rb | 50 +--- .../artifacts/external_file.html.haml | 5 +- app/views/projects/pages/_access.html.haml | 2 +- app/views/projects/pages/show.html.haml | 2 +- .../projects/pages_domains/_dns.html.haml | 2 +- .../operations/rails_console.md | 2 +- lib/gitlab/pages/url_builder.rb | 94 ++++++++ .../projects/pages/user_adds_domain_spec.rb | 2 +- spec/lib/gitlab/ci/variables/builder_spec.rb | 2 +- spec/lib/gitlab/pages/url_builder_spec.rb | 227 ++++++++++++++++++ spec/models/ci/artifact_blob_spec.rb | 70 +++--- spec/models/ci/build_spec.rb | 2 +- spec/models/pages/lookup_path_spec.rb | 15 +- spec/models/project_spec.rb | 218 ----------------- 17 files changed, 393 insertions(+), 348 deletions(-) create mode 100644 lib/gitlab/pages/url_builder.rb create mode 100644 spec/lib/gitlab/pages/url_builder_spec.rb diff --git a/app/helpers/projects/pages_helper.rb b/app/helpers/projects/pages_helper.rb index f46c11db1db05..d90ea0ec598a1 100644 --- a/app/helpers/projects/pages_helper.rb +++ b/app/helpers/projects/pages_helper.rb @@ -7,5 +7,17 @@ def can_create_pages_custom_domains?(current_user, project) (Gitlab.config.pages.external_http || Gitlab.config.pages.external_https) && project.can_create_custom_domains? end + + def pages_subdomain(project) + Gitlab::Pages::UrlBuilder + .new(project) + .project_namespace + end + + def build_pages_url(project, with_unique_domain:) + Gitlab::Pages::UrlBuilder + .new(project) + .pages_url(with_unique_domain: with_unique_domain) + end end end diff --git a/app/models/ci/artifact_blob.rb b/app/models/ci/artifact_blob.rb index f87b18d516f6a..1f6d218b015d6 100644 --- a/app/models/ci/artifact_blob.rb +++ b/app/models/ci/artifact_blob.rb @@ -4,8 +4,6 @@ module Ci class ArtifactBlob include BlobLike - EXTENSIONS_SERVED_BY_PAGES = %w[.html .htm .txt .json .xml .log].freeze - attr_reader :entry def initialize(entry) @@ -35,31 +33,18 @@ def external_storage :build_artifact end - def external_url(project, job) - return unless external_link?(job) - - url_project_path = project.full_path.partition('/').last - - artifact_path = [ - '-', url_project_path, '-', - 'jobs', job.id, - 'artifacts', path - ].join('/') - - "#{project.pages_namespace_url}/#{artifact_path}" + def external_url(job) + pages_url_builder(job.project).artifact_url(entry, job) end def external_link?(job) - pages_config.enabled && - pages_config.artifacts_server && - EXTENSIONS_SERVED_BY_PAGES.include?(File.extname(name)) && - (pages_config.access_control || job.project.public?) + pages_url_builder(job.project).artifact_url_available?(entry, job) end private - def pages_config - Gitlab.config.pages + def pages_url_builder(project) + @pages_url_builder ||= Gitlab::Pages::UrlBuilder.new(project) end end end diff --git a/app/models/pages/lookup_path.rb b/app/models/pages/lookup_path.rb index 864ea04c01903..2ffb2e84cbfff 100644 --- a/app/models/pages/lookup_path.rb +++ b/app/models/pages/lookup_path.rb @@ -46,7 +46,7 @@ def source strong_memoize_attr :source def prefix - if project.pages_namespace_url == project.pages_url + if url_builder.namespace_pages? '/' else "#{project.full_path.delete_prefix(trim_prefix)}/" @@ -55,9 +55,7 @@ def prefix strong_memoize_attr :prefix def unique_host - return unless project.project_setting.pages_unique_domain_enabled? - - project.pages_unique_host + url_builder.unique_host end strong_memoize_attr :unique_host @@ -76,5 +74,10 @@ def deployment project.pages_metadatum.pages_deployment end strong_memoize_attr :deployment + + def url_builder + Gitlab::Pages::UrlBuilder.new(project) + end + strong_memoize_attr :url_builder end end diff --git a/app/models/project.rb b/app/models/project.rb index cd107fb8c35ff..931f4db3a546a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2218,42 +2218,6 @@ def pages_deployed? pages_metadatum&.deployed? end - def pages_url(with_unique_domain: false) - return pages_unique_url if with_unique_domain && pages_unique_domain_enabled? - - url = pages_namespace_url - url_path = full_path.partition('/').last - namespace_url = "#{Settings.pages.protocol}://#{url_path}".downcase - - if Rails.env.development? - url_without_port = URI.parse(url) - url_without_port.port = nil - - return url if url_without_port.to_s == namespace_url - end - - # If the project path is the same as host, we serve it as group page - return url if url == namespace_url - - "#{url}/#{url_path}" - end - - def pages_unique_url - pages_url_for(project_setting.pages_unique_domain) - end - - def pages_unique_host - URI(pages_unique_url).host - end - - def pages_namespace_url - pages_url_for(pages_subdomain) - end - - def pages_subdomain - full_path.partition('/').first - end - def pages_path # TODO: when we migrate Pages to work with new storage types, change here to use disk_path File.join(Settings.pages.path, full_path) @@ -2500,7 +2464,7 @@ def pages_variables break unless pages_enabled? variables.append(key: 'CI_PAGES_DOMAIN', value: Gitlab.config.pages.host) - variables.append(key: 'CI_PAGES_URL', value: pages_url) + variables.append(key: 'CI_PAGES_URL', value: Gitlab::Pages::UrlBuilder.new(self).pages_url) end end @@ -3259,18 +3223,6 @@ def crm_enabled? private - def pages_unique_domain_enabled? - Feature.enabled?(:pages_unique_domain, self) && - project_setting.pages_unique_domain_enabled? - end - - def pages_url_for(domain) - # The host in URL always needs to be downcased - Gitlab.config.pages.url.sub(%r{^https?://}) do |prefix| - "#{prefix}#{domain}." - end.downcase - end - # overridden in EE def project_group_links_with_preload project_group_links diff --git a/app/views/projects/artifacts/external_file.html.haml b/app/views/projects/artifacts/external_file.html.haml index a014d134e31bf..67f6ccd5695df 100644 --- a/app/views/projects/artifacts/external_file.html.haml +++ b/app/views/projects/artifacts/external_file.html.haml @@ -1,3 +1,4 @@ +- external_url = @blob.external_url(@build) - page_title @path, _('Artifacts'), "#{@build.name} (##{@build.id})", _('Jobs') = render "projects/jobs/header" @@ -8,8 +9,8 @@ %h2= _("You are being redirected away from GitLab") %p= _("This page is hosted on GitLab pages but contains user-generated content and may contain malicious code. Do not accept unless you trust the author and source.") - = link_to @blob.external_url(@project, @build), - @blob.external_url(@project, @build), + = link_to external_url, + external_url, target: '_blank', title: _('Opens in a new window'), rel: 'noopener noreferrer' diff --git a/app/views/projects/pages/_access.html.haml b/app/views/projects/pages/_access.html.haml index 50e48187be314..6eab31075d405 100644 --- a/app/views/projects/pages/_access.html.haml +++ b/app/views/projects/pages/_access.html.haml @@ -1,5 +1,5 @@ - if @project.pages_deployed? - - pages_url = @project.pages_url(with_unique_domain: true) + - pages_url = build_pages_url(@project, with_unique_domain: true) = render Pajamas::CardComponent.new(card_options: { class: 'gl-mb-5', data: { qa_selector: 'access_page_container' } }, footer_options: { class: 'gl-alert-warning' }) do |c| - c.with_header do diff --git a/app/views/projects/pages/show.html.haml b/app/views/projects/pages/show.html.haml index e43f6007a8b83..698ce404be89d 100644 --- a/app/views/projects/pages/show.html.haml +++ b/app/views/projects/pages/show.html.haml @@ -11,7 +11,7 @@ = render 'pages_settings' %hr.clearfix - = render 'ssl_limitations_warning' if @project.pages_subdomain.include?(".") + = render 'ssl_limitations_warning' if pages_subdomain(@project).include?(".") = render 'access' - if Gitlab.config.pages.external_http || Gitlab.config.pages.external_https = render 'list' diff --git a/app/views/projects/pages_domains/_dns.html.haml b/app/views/projects/pages_domains/_dns.html.haml index 7c34c3ff6e1d1..0edce28bb9d2a 100644 --- a/app/views/projects/pages_domains/_dns.html.haml +++ b/app/views/projects/pages_domains/_dns.html.haml @@ -1,5 +1,5 @@ - verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled? -- dns_record = "#{domain_presenter.domain} ALIAS #{domain_presenter.project.pages_subdomain}.#{Settings.pages.host}." +- dns_record = "#{domain_presenter.domain} ALIAS #{pages_subdomain(domain_presenter.project)}.#{Settings.pages.host}." .form-group.border-section .row diff --git a/doc/administration/operations/rails_console.md b/doc/administration/operations/rails_console.md index 311bae0a0406c..ac0a7e5870b9a 100644 --- a/doc/administration/operations/rails_console.md +++ b/doc/administration/operations/rails_console.md @@ -208,7 +208,7 @@ Adding a semicolon(`;`) and a follow-up statement at the end of a statement prev ```ruby puts ActiveRecord::Base.descendants; :ok -Project.select(&:pages_deployed?).each {|p| puts p.pages_url }; true +Project.select(&:pages_deployed?).each {|p| puts p.path }; true ``` ## Get or store the result of last operation diff --git a/lib/gitlab/pages/url_builder.rb b/lib/gitlab/pages/url_builder.rb new file mode 100644 index 0000000000000..215154b724828 --- /dev/null +++ b/lib/gitlab/pages/url_builder.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module Gitlab + module Pages + class UrlBuilder + attr_reader :project_namespace + + ALLOWED_ARTIFACT_EXTENSIONS = %w[.html .htm .txt .json .xml .log].freeze + ARTIFACT_URL = "%{host}/-/%{project_path}/-/jobs/%{job_id}/artifacts/%{artifact_path}" + + def initialize(project) + @project = project + @project_namespace, _, @project_path = project.full_path.partition('/') + end + + def pages_url(with_unique_domain: false) + return unique_url if with_unique_domain && unique_domain_enabled? + + project_path_url = "#{config.protocol}://#{project_path}".downcase + + # If the project path is the same as host, we serve it as group page + # On development we ignore the URL port to make it work on GDK + return namespace_url if Rails.env.development? && portless(namespace_url) == project_path_url + # If the project path is the same as host, we serve it as group page + return namespace_url if namespace_url == project_path_url + + "#{namespace_url}/#{project_path}" + end + + def unique_host + return unless unique_domain_enabled? + + URI(unique_url).host + end + + def namespace_pages? + namespace_url == pages_url + end + + def artifact_url(artifact, job) + return unless artifact_url_available?(artifact, job) + + format( + ARTIFACT_URL, + host: namespace_url, + project_path: project_path, + job_id: job.id, + artifact_path: artifact.path) + end + + def artifact_url_available?(artifact, job) + config.enabled && + config.artifacts_server && + ALLOWED_ARTIFACT_EXTENSIONS.include?(File.extname(artifact.name)) && + (config.access_control || job.project.public?) + end + + private + + attr_reader :project, :project_path + + def namespace_url + @namespace_url ||= url_for(project_namespace) + end + + def unique_url + @unique_url ||= url_for(project.project_setting.pages_unique_domain) + end + + def url_for(subdomain) + URI(config.url) + .tap { |url| url.port = config.port } + .tap { |url| url.host.prepend("#{subdomain}.") } + .to_s + .downcase + end + + def portless(url) + URI(url) + .tap { |u| u.port = nil } + .to_s + end + + def unique_domain_enabled? + Feature.enabled?(:pages_unique_domain, project) && + project.project_setting.pages_unique_domain_enabled? + end + + def config + Gitlab.config.pages + end + end + end +end diff --git a/spec/features/projects/pages/user_adds_domain_spec.rb b/spec/features/projects/pages/user_adds_domain_spec.rb index 708210e669c56..ae459197b38f9 100644 --- a/spec/features/projects/pages/user_adds_domain_spec.rb +++ b/spec/features/projects/pages/user_adds_domain_spec.rb @@ -178,7 +178,7 @@ visit project_pages_path(project) within('#content-body') { click_link 'Edit' } - expect(page).to have_field :domain_dns, with: "#{domain.domain} ALIAS #{domain.project.pages_subdomain}.#{Settings.pages.host}." + expect(page).to have_field :domain_dns, with: "#{domain.domain} ALIAS namespace1.example.com." end end end diff --git a/spec/lib/gitlab/ci/variables/builder_spec.rb b/spec/lib/gitlab/ci/variables/builder_spec.rb index 6b296924b6df4..28c9bdc4c4b8e 100644 --- a/spec/lib/gitlab/ci/variables/builder_spec.rb +++ b/spec/lib/gitlab/ci/variables/builder_spec.rb @@ -98,7 +98,7 @@ { key: 'CI_PAGES_DOMAIN', value: Gitlab.config.pages.host }, { key: 'CI_PAGES_URL', - value: project.pages_url }, + value: Gitlab::Pages::UrlBuilder.new(project).pages_url }, { key: 'CI_API_V4_URL', value: API::Helpers::Version.new('v4').root_url }, { key: 'CI_API_GRAPHQL_URL', diff --git a/spec/lib/gitlab/pages/url_builder_spec.rb b/spec/lib/gitlab/pages/url_builder_spec.rb new file mode 100644 index 0000000000000..8e1581704cb75 --- /dev/null +++ b/spec/lib/gitlab/pages/url_builder_spec.rb @@ -0,0 +1,227 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Pages::UrlBuilder, feature_category: :pages do + let(:pages_enabled) { true } + let(:artifacts_server) { true } + let(:access_control) { true } + + let(:port) { nil } + let(:host) { 'example.com' } + + let(:full_path) { 'group/project' } + let(:project_public) { true } + let(:unique_domain) { 'unique-domain' } + let(:unique_domain_enabled) { false } + + let(:project_setting) do + instance_double( + ProjectSetting, + pages_unique_domain: unique_domain, + pages_unique_domain_enabled?: unique_domain_enabled + ) + end + + let(:project) do + instance_double( + Project, + flipper_id: 'project:1', # required for the feature flag check + public?: project_public, + project_setting: project_setting, + full_path: full_path + ) + end + + subject(:builder) { described_class.new(project) } + + before do + stub_pages_setting( + enabled: pages_enabled, + host: host, + url: 'http://example.com', + protocol: 'http', + artifacts_server: artifacts_server, + access_control: access_control, + port: port + ) + end + + describe '#pages_url' do + subject(:pages_url) { builder.pages_url } + + it { is_expected.to eq('http://group.example.com/project') } + + context 'when namespace is upper cased' do + let(:full_path) { 'Group/project' } + + it { is_expected.to eq('http://group.example.com/project') } + end + + context 'when project is in a nested group page' do + let(:full_path) { 'group/subgroup/project' } + + it { is_expected.to eq('http://group.example.com/subgroup/project') } + end + + context 'when using domain pages' do + let(:full_path) { 'group/group.example.com' } + + it { is_expected.to eq('http://group.example.com') } + + context 'in development mode' do + let(:port) { 3010 } + + before do + stub_rails_env('development') + end + + it { is_expected.to eq('http://group.example.com:3010') } + end + end + + context 'when not using pages_unique_domain' do + subject(:pages_url) { builder.pages_url(with_unique_domain: false) } + + context 'when pages_unique_domain feature flag is disabled' do + before do + stub_feature_flags(pages_unique_domain: false) + end + + it { is_expected.to eq('http://group.example.com/project') } + end + + context 'when pages_unique_domain feature flag is enabled' do + before do + stub_feature_flags(pages_unique_domain: true) + end + + context 'when pages_unique_domain_enabled is false' do + let(:unique_domain_enabled) { false } + + it { is_expected.to eq('http://group.example.com/project') } + end + + context 'when pages_unique_domain_enabled is true' do + let(:unique_domain_enabled) { true } + + it { is_expected.to eq('http://group.example.com/project') } + end + end + end + + context 'when using pages_unique_domain' do + subject(:pages_url) { builder.pages_url(with_unique_domain: true) } + + context 'when pages_unique_domain feature flag is disabled' do + before do + stub_feature_flags(pages_unique_domain: false) + end + + it { is_expected.to eq('http://group.example.com/project') } + end + + context 'when pages_unique_domain feature flag is enabled' do + before do + stub_feature_flags(pages_unique_domain: true) + end + + context 'when pages_unique_domain_enabled is false' do + let(:unique_domain_enabled) { false } + + it { is_expected.to eq('http://group.example.com/project') } + end + + context 'when pages_unique_domain_enabled is true' do + let(:unique_domain_enabled) { true } + + it { is_expected.to eq('http://unique-domain.example.com') } + end + end + end + end + + describe '#unique_host' do + subject(:unique_host) { builder.unique_host } + + context 'when pages_unique_domain feature flag is disabled' do + before do + stub_feature_flags(pages_unique_domain: false) + end + + it { is_expected.to be_nil } + end + + context 'when pages_unique_domain feature flag is enabled' do + before do + stub_feature_flags(pages_unique_domain: true) + end + + context 'when pages_unique_domain_enabled is false' do + let(:unique_domain_enabled) { false } + + it { is_expected.to be_nil } + end + + context 'when pages_unique_domain_enabled is true' do + let(:unique_domain_enabled) { true } + + it { is_expected.to eq('unique-domain.example.com') } + end + end + end + + describe '#artifact_url' do + let(:job) { instance_double(Ci::Build, id: 1) } + let(:artifact) do + instance_double( + Gitlab::Ci::Build::Artifacts::Metadata::Entry, + name: artifact_name, + path: "path/#{artifact_name}") + end + + subject(:artifact_url) { builder.artifact_url(artifact, job) } + + context 'with not allowed extension' do + let(:artifact_name) { 'file.gif' } + + it { is_expected.to be_nil } + end + + context 'with allowed extension' do + let(:artifact_name) { 'file.txt' } + + it { is_expected.to eq("http://group.example.com/-/project/-/jobs/1/artifacts/path/file.txt") } + + context 'when port is configured' do + let(:port) { 1234 } + + it { is_expected.to eq("http://group.example.com:1234/-/project/-/jobs/1/artifacts/path/file.txt") } + end + end + end + + describe '#artifact_url_available?' do + let(:job) { instance_double(Ci::Build, id: 1) } + let(:artifact) do + instance_double( + Gitlab::Ci::Build::Artifacts::Metadata::Entry, + name: artifact_name, + path: "path/#{artifact_name}") + end + + subject(:artifact_url_available) { builder.artifact_url_available?(artifact, job) } + + context 'with not allowed extensions' do + let(:artifact_name) { 'file.gif' } + + it { is_expected.to be false } + end + + context 'with allowed extensions' do + let(:artifact_name) { 'file.txt' } + + it { is_expected.to be true } + end + end +end diff --git a/spec/models/ci/artifact_blob_spec.rb b/spec/models/ci/artifact_blob_spec.rb index 82d43a1deef90..2e0b13c35ea6a 100644 --- a/spec/models/ci/artifact_blob_spec.rb +++ b/spec/models/ci/artifact_blob_spec.rb @@ -3,104 +3,92 @@ require 'spec_helper' RSpec.describe Ci::ArtifactBlob, feature_category: :continuous_integration do - let_it_be(:project) { create(:project, :public) } + let_it_be(:project) { create(:project, :public, path: 'project1') } let_it_be(:build) { create(:ci_build, :artifacts, project: project) } + let(:pages_port) { nil } let(:entry) { build.artifacts_metadata_entry('other_artifacts_0.1.2/another-subdirectory/banana_sample.gif') } - subject { described_class.new(entry) } + subject(:blob) { described_class.new(entry) } + + before do + stub_pages_setting( + enabled: true, + artifacts_server: true, + access_control: true, + port: pages_port + ) + end describe '#id' do it 'returns a hash of the path' do - expect(subject.id).to eq(Digest::SHA1.hexdigest(entry.path)) + expect(blob.id).to eq(Digest::SHA1.hexdigest(entry.path)) end end describe '#name' do it 'returns the entry name' do - expect(subject.name).to eq(entry.name) + expect(blob.name).to eq(entry.name) end end describe '#path' do it 'returns the entry path' do - expect(subject.path).to eq(entry.path) + expect(blob.path).to eq(entry.path) end end describe '#size' do it 'returns the entry size' do - expect(subject.size).to eq(entry.metadata[:size]) + expect(blob.size).to eq(entry.metadata[:size]) end end describe '#mode' do it 'returns the entry mode' do - expect(subject.mode).to eq(entry.metadata[:mode]) + expect(blob.mode).to eq(entry.metadata[:mode]) end end describe '#external_storage' do it 'returns :build_artifact' do - expect(subject.external_storage).to eq(:build_artifact) + expect(blob.external_storage).to eq(:build_artifact) end end describe '#external_url' do - before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(true) - allow(Gitlab.config.pages).to receive(:artifacts_server).and_return(true) - end + subject(:url) { blob.external_url(build) } - describe '.gif extension' do - it 'returns nil' do - expect(subject.external_url(build.project, build)).to be_nil - end + context 'with not allowed extension' do + it { is_expected.to be_nil } end - context 'txt extensions' do + context 'with allowed extension' do let(:path) { 'other_artifacts_0.1.2/doc_sample.txt' } let(:entry) { build.artifacts_metadata_entry(path) } - it 'returns a URL' do - url = subject.external_url(build.project, build) - - expect(url).not_to be_nil - expect(url).to eq("http://#{project.namespace.path}.#{Gitlab.config.pages.host}/-/#{project.path}/-/jobs/#{build.id}/artifacts/#{path}") - end + it { is_expected.to eq("http://#{project.namespace.path}.example.com/-/project1/-/jobs/#{build.id}/artifacts/other_artifacts_0.1.2/doc_sample.txt") } context 'when port is configured' do - let(:port) { 1234 } - - it 'returns an URL with port number' do - allow(Gitlab.config.pages).to receive(:url).and_return("#{Gitlab.config.pages.url}:#{port}") - - url = subject.external_url(build.project, build) + let(:pages_port) { 1234 } - expect(url).not_to be_nil - expect(url).to eq("http://#{project.namespace.path}.#{Gitlab.config.pages.host}:#{port}/-/#{project.path}/-/jobs/#{build.id}/artifacts/#{path}") - end + it { is_expected.to eq("http://#{project.namespace.path}.example.com:1234/-/project1/-/jobs/#{build.id}/artifacts/other_artifacts_0.1.2/doc_sample.txt") } end end end describe '#external_link?' do - before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(true) - allow(Gitlab.config.pages).to receive(:artifacts_server).and_return(true) - end - - context 'gif extensions' do + context 'with not allowed extensions' do it 'returns false' do - expect(subject.external_link?(build)).to be false + expect(blob.external_link?(build)).to be false end end - context 'txt extensions' do + context 'with allowed extensions' do let(:entry) { build.artifacts_metadata_entry('other_artifacts_0.1.2/doc_sample.txt') } it 'returns true' do - expect(subject.external_link?(build)).to be true + expect(blob.external_link?(build)).to be true end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index ee14f77009dc4..b7f457962a016 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2899,7 +2899,7 @@ { key: 'CI_DEFAULT_BRANCH', value: project.default_branch, public: true, masked: false }, { key: 'CI_CONFIG_PATH', value: project.ci_config_path_or_default, public: true, masked: false }, { key: 'CI_PAGES_DOMAIN', value: Gitlab.config.pages.host, public: true, masked: false }, - { key: 'CI_PAGES_URL', value: project.pages_url, public: true, masked: false }, + { key: 'CI_PAGES_URL', value: Gitlab::Pages::UrlBuilder.new(project).pages_url, public: true, masked: false }, { key: 'CI_DEPENDENCY_PROXY_SERVER', value: Gitlab.host_with_port, public: true, masked: false }, { key: 'CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX', value: "#{Gitlab.host_with_port}/#{project.namespace.root_ancestor.path.downcase}#{DependencyProxy::URL_SUFFIX}", diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 88fd1bd9e560a..62152f9d3a480 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -8,7 +8,12 @@ subject(:lookup_path) { described_class.new(project) } before do - stub_pages_setting(access_control: true, external_https: ["1.1.1.1:443"]) + stub_pages_setting( + access_control: true, + external_https: ["1.1.1.1:443"], + url: 'http://example.com', + protocol: 'http' + ) stub_pages_object_storage(::Pages::DeploymentUploader) end @@ -120,18 +125,14 @@ describe '#prefix' do it 'returns "/" for pages group root projects' do - project = instance_double(Project, pages_namespace_url: "namespace.test", pages_url: "namespace.test") + project = instance_double(Project, full_path: "namespace/namespace.example.com") lookup_path = described_class.new(project, trim_prefix: 'mygroup') expect(lookup_path.prefix).to eq('/') end it 'returns the project full path with the provided prefix removed' do - project = instance_double( - Project, - pages_namespace_url: "namespace.test", - pages_url: "namespace.other", - full_path: 'mygroup/myproject') + project = instance_double(Project, full_path: 'mygroup/myproject') lookup_path = described_class.new(project, trim_prefix: 'mygroup') expect(lookup_path.prefix).to eq('/myproject/') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1a7a0688abe98..044408e86e93a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2806,224 +2806,6 @@ def has_external_wiki end end - describe '#pages_url', feature_category: :pages do - let(:group_name) { 'group' } - let(:project_name) { 'project' } - - let(:group) { create(:group, name: group_name) } - let(:nested_group) { create(:group, parent: group) } - - let(:project_path) { project_name.downcase } - let(:project) do - create( - :project, - namespace: group, - name: project_name, - path: project_path) - end - - let(:domain) { 'Example.com' } - let(:port) { nil } - - subject { project.pages_url } - - before do - allow(Settings.pages).to receive(:host).and_return(domain) - allow(Gitlab.config.pages) - .to receive(:url) - .and_return(['http://example.com', port].compact.join(':')) - end - - context 'when not using pages_unique_domain' do - subject { project.pages_url(with_unique_domain: false) } - - context 'when pages_unique_domain feature flag is disabled' do - before do - stub_feature_flags(pages_unique_domain: false) - end - - it { is_expected.to eq('http://group.example.com/project') } - end - - context 'when pages_unique_domain feature flag is enabled' do - before do - stub_feature_flags(pages_unique_domain: true) - - project.project_setting.update!( - pages_unique_domain_enabled: pages_unique_domain_enabled, - pages_unique_domain: 'unique-domain' - ) - end - - context 'when pages_unique_domain_enabled is false' do - let(:pages_unique_domain_enabled) { false } - - it { is_expected.to eq('http://group.example.com/project') } - end - - context 'when pages_unique_domain_enabled is true' do - let(:pages_unique_domain_enabled) { true } - - it { is_expected.to eq('http://group.example.com/project') } - end - end - end - - context 'when using pages_unique_domain' do - subject { project.pages_url(with_unique_domain: true) } - - context 'when pages_unique_domain feature flag is disabled' do - before do - stub_feature_flags(pages_unique_domain: false) - end - - it { is_expected.to eq('http://group.example.com/project') } - end - - context 'when pages_unique_domain feature flag is enabled' do - before do - stub_feature_flags(pages_unique_domain: true) - - project.project_setting.update!( - pages_unique_domain_enabled: pages_unique_domain_enabled, - pages_unique_domain: 'unique-domain' - ) - end - - context 'when pages_unique_domain_enabled is false' do - let(:pages_unique_domain_enabled) { false } - - it { is_expected.to eq('http://group.example.com/project') } - end - - context 'when pages_unique_domain_enabled is true' do - let(:pages_unique_domain_enabled) { true } - - it { is_expected.to eq('http://unique-domain.example.com') } - end - end - end - - context 'with nested group' do - let(:project) { create(:project, namespace: nested_group, name: project_name) } - let(:expected_url) { "http://group.example.com/#{nested_group.path}/#{project.path}" } - - context 'group page' do - let(:project_name) { 'group.example.com' } - - it { is_expected.to eq(expected_url) } - end - - context 'project page' do - let(:project_name) { 'Project' } - - it { is_expected.to eq(expected_url) } - end - end - - context 'when the project matches its namespace url' do - let(:project_name) { 'group.example.com' } - - it { is_expected.to eq('http://group.example.com') } - - context 'with different group name capitalization' do - let(:group_name) { 'Group' } - - it { is_expected.to eq("http://group.example.com") } - end - - context 'with different project path capitalization' do - let(:project_path) { 'Group.example.com' } - - it { is_expected.to eq("http://group.example.com") } - end - - context 'with different project name capitalization' do - let(:project_name) { 'Project' } - - it { is_expected.to eq("http://group.example.com/project") } - end - - context 'when there is an explicit port' do - let(:port) { 3000 } - - context 'when not in dev mode' do - before do - stub_rails_env('production') - end - - it { is_expected.to eq('http://group.example.com:3000/group.example.com') } - end - - context 'when in dev mode' do - before do - stub_rails_env('development') - end - - it { is_expected.to eq('http://group.example.com:3000') } - end - end - end - end - - describe '#pages_unique_url', feature_category: :pages do - let(:project_settings) { create(:project_setting, pages_unique_domain: 'unique-domain') } - let(:project) { build(:project, project_setting: project_settings) } - let(:domain) { 'example.com' } - - before do - allow(Settings.pages).to receive(:host).and_return(domain) - allow(Gitlab.config.pages).to receive(:url).and_return("http://#{domain}") - end - - it 'returns the pages unique url' do - expect(project.pages_unique_url).to eq('http://unique-domain.example.com') - end - end - - describe '#pages_unique_host', feature_category: :pages do - let(:project_settings) { create(:project_setting, pages_unique_domain: 'unique-domain') } - let(:project) { build(:project, project_setting: project_settings) } - let(:domain) { 'example.com' } - - before do - allow(Settings.pages).to receive(:host).and_return(domain) - allow(Gitlab.config.pages).to receive(:url).and_return("http://#{domain}") - end - - it 'returns the pages unique url' do - expect(project.pages_unique_host).to eq('unique-domain.example.com') - end - end - - describe '#pages_namespace_url', feature_category: :pages do - let(:group) { create(:group, name: group_name) } - let(:project) { create(:project, namespace: group, name: project_name) } - let(:domain) { 'Example.com' } - let(:port) { 1234 } - - subject { project.pages_namespace_url } - - before do - allow(Settings.pages).to receive(:host).and_return(domain) - allow(Gitlab.config.pages).to receive(:url).and_return("http://example.com:#{port}") - end - - context 'group page' do - let(:group_name) { 'Group' } - let(:project_name) { 'group.example.com' } - - it { is_expected.to eq("http://group.example.com:#{port}") } - end - - context 'project page' do - let(:group_name) { 'Group' } - let(:project_name) { 'Project' } - - it { is_expected.to eq("http://group.example.com:#{port}") } - end - end - describe '.search' do let_it_be(:project) { create(:project, description: 'kitten mittens') } -- GitLab