Merge branch 'ac/pages-port' into 'master'

Add gitlab-pages port number to artifacts links, if needed

Closes #44398

See merge request gitlab-org/gitlab-ce!18073

app/models/ci/artifact_blob.rb

@@ -36,16 +36,15 @@ module Ci
     def external_url(project, job)
       return unless external_link?(job)
 
-      full_path_parts = project.full_path_components
-      top_level_group = full_path_parts.shift
+      url_project_path = project.full_path.partition('/').last
 
       artifact_path = [
-        '-', *full_path_parts, '-',
+        '-', url_project_path, '-',
         'jobs', job.id,
         'artifacts', path
       ].join('/')
 
-      "#{pages_config.protocol}://#{top_level_group}.#{pages_config.host}/#{artifact_path}"
+      "#{project.pages_group_url}/#{artifact_path}"
     end
 
     def external_link?(job)

app/models/project.rb

@@ -1346,20 +1346,19 @@ class Project < ActiveRecord::Base
     Dir.exist?(public_pages_path)
   end
 
-  def pages_url
-    subdomain, _, url_path = full_path.partition('/')
-
-    # The hostname always needs to be in downcased
-    # All web servers convert hostname to lowercase
-    host = "#{subdomain}.#{Settings.pages.host}".downcase
-
+  def pages_group_url
     # The host in URL always needs to be downcased
-    url = Gitlab.config.pages.url.sub(%r{^https?://}) do |prefix|
-      "#{prefix}#{subdomain}."
+    Gitlab.config.pages.url.sub(%r{^https?://}) do |prefix|
+      "#{prefix}#{pages_subdomain}."
     end.downcase
+  end
+
+  def pages_url
+    url = pages_group_url
+    url_path = full_path.partition('/').last
 
     # If the project path is the same as host, we serve it as group page
-    return url if host == url_path
+    return url if url == "#{Settings.pages.protocol}://#{url_path}"
 
     "#{url}/#{url_path}"
   end

changelogs/unreleased/ac-pages-port.yml

+---
+title: Add missing port to artifact links
+merge_request:
+author:
+type: fixed

spec/models/ci/artifact_blob_spec.rb

@@ -65,6 +65,19 @@ describe Ci::ArtifactBlob do
         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
+
+      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)
+
+          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
+      end
     end
   end
 

spec/models/project_spec.rb

@@ -1265,6 +1265,34 @@ describe Project do
     end
   end
 
+  describe '#pages_group_url' 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_group_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(:project) { create(:project, description: 'kitten mittens') }