REFACTOR: Proxy letter avatars in rails instead of nginx

Co-authored-by: Sam Saffron <sam.saffron@gmail.com>
Co-authored-by: David Taylor <david@taylorhq.com>

This gives more control over the request. In particular we can easily
lookup DNS dynamically, instead of only upon NGINX startup.
Previously, NGINX was looking up IP for the letter avatar service and
caching the CDN IP address, this caused issues if CDN changed IP, in
which letter avatars would be broken till a container restarted.

NGINX config has been updated to add caching. This change will require
a container rebuild.

The proxy will now function in development environments, so the patch
for `letter_avatar_proxy` has been removed.

app/controllers/user_avatars_controller.rb

@@ -35,7 +35,7 @@ class UserAvatarsController < ApplicationController
   def show_proxy_letter
     is_asset_path
 
-    if SiteSetting.external_system_avatars_url !~ /^\/letter_avatar_proxy/
+    if SiteSetting.external_system_avatars_url !~ /^\/letter(_avatar)?_proxy/
       raise Discourse::NotFound
     end
 
@@ -43,17 +43,8 @@ class UserAvatarsController < ApplicationController
     params.require(:color)
     params.require(:version)
     params.require(:size)
-
     hijack do
-      identity = LetterAvatar::Identity.new
-      identity.letter = params[:letter].to_s[0].upcase
-      identity.color = params[:color].scan(/../).map(&:hex)
-      image = LetterAvatar.generate(params[:letter].to_s, params[:size].to_i, identity: identity)
-
-      response.headers["Last-Modified"] = File.ctime(image).httpdate
-      response.headers["Content-Length"] = File.size(image).to_s
-      immutable_for(1.year)
-      send_file image, disposition: nil
+      proxy_avatar("https://avatars.discourse.org/#{params[:version]}/letter/#{params[:letter]}/#{params[:color]}/#{params[:size]}.png", Time.new('1990-01-01'))
     end
   end
 

config/nginx.sample.conf

@@ -189,9 +189,9 @@ server {
     }
 
     # This big block is needed so we can selectively enable
-    # acceleration for backups and avatars
+    # acceleration for backups, avatars, sprites and so on.
     # see note about repetition above
-    location ~ ^/(svg-sprite/|letter_avatar/|user_avatar|highlight-js|stylesheets|theme-javascripts|favicon/proxied|service-worker) {
+    location ~ ^/(svg-sprite/|letter_avatar/|letter_avatar_proxy/|user_avatar|highlight-js|stylesheets|theme-javascripts|favicon/proxied|service-worker) {
       proxy_set_header Host $http_host;
       proxy_set_header X-Real-IP $remote_addr;
       proxy_set_header X-Request-Start "t=${msec}";
@@ -214,30 +214,6 @@ server {
       break;
     }
 
-    location /letter_avatar_proxy/ {
-      # Don't send any client headers to the avatars service
-      proxy_method GET;
-      proxy_pass_request_headers off;
-      proxy_pass_request_body off;
-
-      # Don't let cookies interrupt caching, and don't pass them to the
-      # client
-      proxy_ignore_headers "Set-Cookie";
-      proxy_hide_header "Set-Cookie";
-      proxy_hide_header "X-Discourse-Username";
-      proxy_hide_header "X-Runtime";
-
-      proxy_cache one;
-      # shared in multisite
-      proxy_cache_key $request_uri;
-      proxy_cache_valid 200 7d;
-      proxy_cache_valid 404 1m;
-      proxy_set_header Connection "";
-
-      proxy_pass https://avatars.discourse.org/;
-      break;
-    }
-
     # we need buffering off for message bus
     location /message-bus/ {
       proxy_set_header X-Request-Start "t=${msec}";

config/routes.rb

@@ -451,7 +451,6 @@ Discourse::Application.routes.draw do
   get "letter_avatar/:username/:size/:version.png" => "user_avatars#show_letter", format: false, constraints: { hostname: /[\w\.-]+/, size: /\d+/, username: RouteFormat.username }
   get "user_avatar/:hostname/:username/:size/:version.png" => "user_avatars#show", format: false, constraints: { hostname: /[\w\.-]+/, size: /\d+/, username: RouteFormat.username }
 
-  # in most production settings this is bypassed
   get "letter_avatar_proxy/:version/letter/:letter/:color/:size.png" => "user_avatars#show_proxy_letter"
 
   get "svg-sprite/:hostname/svg-:theme_ids-:version.js" => "svg_sprite#show", format: false, constraints: { hostname: /[\w\.-]+/, version: /\h{40}/, theme_ids: /([0-9]+(,[0-9]+)*)?/ }

config/site_settings.yml

@@ -1077,7 +1077,7 @@ files:
     client: true
     shadowed_by_global: true
   external_system_avatars_url:
-    default: "/letter_avatar_proxy/v2/letter/{first_letter}/{color}/{size}.png"
+    default: "/letter_avatar_proxy/v3/letter/{first_letter}/{color}/{size}.png"
     client: true
     regex: '^((https?:)?\/)?\/.+[^\/]'
     shadowed_by_global: true

spec/models/user_spec.rb

@@ -1147,7 +1147,7 @@ describe User do
       expect(user.small_avatar_url).to eq("//test.localhost/letter_avatar/sam/45/#{LetterAvatar.version}.png")
 
       SiteSetting.external_system_avatars_enabled = true
-      expect(user.small_avatar_url).to eq("//test.localhost/letter_avatar_proxy/v2/letter/s/5f9b8f/45.png")
+      expect(user.small_avatar_url).to eq("//test.localhost/letter_avatar_proxy/v3/letter/s/5f9b8f/45.png")
     end
 
   end

spec/requests/user_avatars_controller_spec.rb

@@ -10,7 +10,8 @@ describe UserAvatarsController do
     end
 
     it 'returns an avatar if we are allowing the proxy' do
-      get "/letter_avatar_proxy/v2/letter/a/aaaaaa/360.png"
+      stub_request(:get, "https://avatars.discourse.org/v3/letter/a/aaaaaa/360.png").to_return(body: 'image')
+      get "/letter_avatar_proxy/v3/letter/a/aaaaaa/360.png"
       expect(response.status).to eq(200)
     end
   end