always return a gitlab-local url for the avatars44775-avatar-on-os-fails-with-cdn

3 files changed, 8 insertions, 30 deletions

diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index f04a81d4b30..13246a774e3 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -29,9 +29,6 @@ module Avatarable
   def avatar_path(only_path: true)
     return unless self[:avatar].present?
 
-    # A file on object storage cannot be served via CDN
-    return avatar.url unless avatar.file_storage?
-
     asset_host = ActionController::Base.asset_host
     use_asset_host = asset_host.present?
     use_authentication = respond_to?(:public?) && !public?
@@ -53,6 +50,6 @@ module Avatarable
       url_base << gitlab_config.relative_url_root
     end
 
-    url_base + avatar.url
+    url_base + avatar.local_url
   end
 end
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index f12f0466a1d..f8a237178d9 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -65,6 +65,10 @@ class GitlabUploader < CarrierWave::Uploader::Base
     !!model
   end
 
+  def local_url
+    File.join('/', self.class.base_dir, dynamic_segment, filename)
+  end
+
   private
 
   # Designed to be overridden by child uploaders that have a dynamic path
diff --git a/spec/models/concerns/avatarable_spec.rb b/spec/models/concerns/avatarable_spec.rb
index 57dbcaa1345..9faf21bfbbd 100644
--- a/spec/models/concerns/avatarable_spec.rb
+++ b/spec/models/concerns/avatarable_spec.rb
@@ -37,43 +37,20 @@ describe Avatarable do
         project.visibility_level = visibility_level
       end
 
-      let(:avatar_path) { (avatar_path_prefix + [project.avatar.url]).join }
+      let(:avatar_path) { (avatar_path_prefix + [project.avatar.local_url]).join }
 
       it 'returns the expected avatar path' do
         expect(project.avatar_path(only_path: only_path)).to eq(avatar_path)
       end
-    end
-
-    context "when avatar is stored remotely" do
-      using RSpec::Parameterized::TableSyntax
 
-      where(:has_asset_host, :visibility_level, :only_path) do
-        true  | Project::PRIVATE  | true
-        true  | Project::PRIVATE  | false
-        true  | Project::INTERNAL | true
-        true  | Project::INTERNAL | false
-        true  | Project::PUBLIC   | true
-        true  | Project::PUBLIC   | false
-        false | Project::PRIVATE  | true
-        false | Project::PRIVATE  | false
-        false | Project::INTERNAL | true
-        false | Project::INTERNAL | false
-        false | Project::PUBLIC   | true
-        false | Project::PUBLIC   | false
-      end
-
-      with_them do
+      context "when avatar is stored remotely" do
         before do
           stub_uploads_object_storage(AvatarUploader)
-          allow(ActionController::Base).to receive(:asset_host) { has_asset_host && asset_host }
 
           project.avatar.migrate!(ObjectStorage::Store::REMOTE)
-          project.visibility_level = visibility_level
         end
 
-        let(:avatar_path) { project.avatar.url }
-
-        it 'returns the remote avatar url' do
+        it 'returns the expected avatar path' do
           expect(project.avatar_url(only_path: only_path)).to eq(avatar_path)
         end
       end