From d0252285ed00976b2e446ee6810a5c43d1339dac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Mon, 27 Feb 2023 11:33:19 +0100 Subject: [PATCH] Fix CDN assets for SaaS in OCP (#3199) Detailed info about this commit in it's original PR: https://github.com/3scale/porta/pull/3072 --- app/helpers/application_helper.rb | 13 +++++++ app/views/layouts/error.html.erb | 1 + app/views/layouts/provider.html.slim | 1 + app/views/layouts/provider/iframe.html.slim | 1 + app/views/layouts/provider/login.html.slim | 2 +- .../layouts/provider/suspended.html.slim | 1 + app/views/layouts/wizard.html.slim | 1 + config/environments/production.rb | 5 ++- config/environments/test.rb | 9 +++++ config/webpack/environment.js | 20 +++++++++++ features/asset_host/asset_host.feature | 31 ++++++++++++++++ features/step_definitions/asset_host_steps.rb | 25 +++++++++++++ lib/tasks/webpacker.rake | 13 +++++++ package.json | 2 +- test/unit/helpers/application_helper_test.rb | 36 +++++++++++++++++++ yarn.lock | 18 +--------- 16 files changed, 157 insertions(+), 22 deletions(-) create mode 100644 features/asset_host/asset_host.feature create mode 100644 features/step_definitions/asset_host_steps.rb create mode 100644 lib/tasks/webpacker.rake diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 3062bfe6a8..fdf1e6f0e5 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -364,4 +364,17 @@ def has_out_of_date_configuration?(service) return unless service service.pending_affecting_changes? end + + def rails_asset_url_tag + javascript_tag("window.rails_asset_host = '#{rails_asset_host_url}'", { type: "text/javascript"}) + end + + def rails_asset_host_url + asset_host_enabled = Rails.configuration.asset_host.present? + asset_host_url = Rails.configuration.three_scale.asset_host.presence + + return '' unless asset_host_enabled && asset_host_url + + "#{request.protocol}#{asset_host_url}" + end end diff --git a/app/views/layouts/error.html.erb b/app/views/layouts/error.html.erb index d6a6924d86..c19025ad20 100644 --- a/app/views/layouts/error.html.erb +++ b/app/views/layouts/error.html.erb @@ -6,6 +6,7 @@ <%- admin_domain = Account.is_admin_domain?(request.internal_host) -%> <%- master_domain = Account.is_master_domain?(request.internal_host) -%> <%- site_account ||= Account.master -%> + <%= rails_asset_url_tag %> <%= stylesheet_link_tag 'error' -%> <%= render 'provider/analytics' if admin_domain %> diff --git a/app/views/layouts/provider.html.slim b/app/views/layouts/provider.html.slim index 22d3043545..ecfd606396 100644 --- a/app/views/layouts/provider.html.slim +++ b/app/views/layouts/provider.html.slim @@ -6,6 +6,7 @@ html[lang="en" class="pf-m-redhat-font"] = content_for?(:title) ? yield(:title) : default_title | | Red Hat 3scale API Management = csrf_meta_tag + = rails_asset_url_tag = javascript_pack_tag 'PF4Styles/base' = render 'provider/theme' = render 'provider/analytics' diff --git a/app/views/layouts/provider/iframe.html.slim b/app/views/layouts/provider/iframe.html.slim index 5ea0f875c9..c3854f00e4 100644 --- a/app/views/layouts/provider/iframe.html.slim +++ b/app/views/layouts/provider/iframe.html.slim @@ -6,6 +6,7 @@ html[lang="en"] base href=base_url = stylesheet_link_tag "provider/layouts/iframe" = csrf_meta_tag + = rails_asset_url_tag = yield :head body diff --git a/app/views/layouts/provider/login.html.slim b/app/views/layouts/provider/login.html.slim index a5f2d9c974..d70f6686fb 100644 --- a/app/views/layouts/provider/login.html.slim +++ b/app/views/layouts/provider/login.html.slim @@ -4,7 +4,7 @@ html[lang="en" class="pf-m-redhat-font"] meta http-equiv="Content-Type" content="text/html; charset=utf-8" title 3scale Login = csrf_meta_tag - + = rails_asset_url_tag = javascript_pack_tag 'PF4Styles/base' = javascript_pack_tag 'PF4Styles/loginPage' = javascript_include_tag 'vendor/jquery-1.8.2.min.js', 'vendor/rails.js' diff --git a/app/views/layouts/provider/suspended.html.slim b/app/views/layouts/provider/suspended.html.slim index 8b18d127f6..524d9b99b7 100644 --- a/app/views/layouts/provider/suspended.html.slim +++ b/app/views/layouts/provider/suspended.html.slim @@ -5,6 +5,7 @@ html[lang="en"] title | Account Suspended | Red Hat 3scale API Management = csrf_meta_tag + = rails_asset_url_tag = stylesheet_link_tag "provider/themes/wizard.css" = render 'provider/analytics' = javascript_include_tag 'provider/layout/provider' diff --git a/app/views/layouts/wizard.html.slim b/app/views/layouts/wizard.html.slim index 8cebe09a12..0b89a6ab64 100644 --- a/app/views/layouts/wizard.html.slim +++ b/app/views/layouts/wizard.html.slim @@ -6,6 +6,7 @@ html[lang="en" class="pf-m-redhat-font"] = content_for?(:title) ? yield(:title) : default_title | | Red Hat 3scale API Management = csrf_meta_tag + = rails_asset_url_tag = javascript_pack_tag 'PF4Styles/base' = stylesheet_link_tag "provider/themes/wizard.css" = render 'provider/analytics' diff --git a/config/environments/production.rb b/config/environments/production.rb index ecb29f5c7a..81adc2f1cf 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -110,11 +110,10 @@ asset_host = config.three_scale.asset_host.presence config.asset_host = ->(source) do - # does it exist in /public/assets ? full_path = File.join(Rails.public_path, source) - precompiled = File.exist?(full_path) + exist_in_public_assets = File.exist?(full_path) - break unless precompiled + break unless exist_in_public_assets asset_host end diff --git a/config/environments/test.rb b/config/environments/test.rb index 251d956ec4..559f7eeb92 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -64,6 +64,15 @@ config.assets.compile = ENV.fetch('SKIP_ASSETS', '0') == '0' + config.asset_host = ->(source) do + full_path = File.join(Rails.public_path, source) + exist_in_public_assets = File.exist?(full_path) + + break unless exist_in_public_assets + + config.three_scale.asset_host.presence + end + config.after_initialize do ::GATEWAY = ActiveMerchant::Billing::BogusGateway.new end diff --git a/config/webpack/environment.js b/config/webpack/environment.js index 2fe1059f6e..0f6f4106ec 100644 --- a/config/webpack/environment.js +++ b/config/webpack/environment.js @@ -42,4 +42,24 @@ environment.loaders.append('yaml', { type: 'json' }) +/* The CDN url must be hardcoded at settings.yml:asset_host during assets compilation in order to get static assets + * like CSS files point to the CDN. Otherwise, the assets are generated with relative paths and are not loaded from + * the CDN, even when settings.yml:asset_host is set during runtime. We don't want to have to provide any CDN url when + * building porta container images in order to get the assets correctly precompiled. To avoid that, this trick assumes + * the assets are generated with relative paths (settings.yml:asset_host = null during compilation time). The next code + * is executed by webpack when compiling the assets, and sets the variable `postTransformPublicPath` to an arrow + * function which will prepend the CDN url in runtime. + * + * https://github.com/3scale/porta/pull/3072 + */ +const { output } = environment.config; +const oldPublicPath = output.publicPath +output.publicPath = ''; + +const fileLoader = environment.loaders.get('file'); +Object.assign(fileLoader.use[0].options, { + publicPath: oldPublicPath, + postTransformPublicPath: (p) => `window.rails_asset_host + ${p}` +}); + module.exports = environment diff --git a/features/asset_host/asset_host.feature b/features/asset_host/asset_host.feature new file mode 100644 index 0000000000..7d17e7b757 --- /dev/null +++ b/features/asset_host/asset_host.feature @@ -0,0 +1,31 @@ +@javascript +Feature: Asset host + In order to reduce the network traffic from the web server + As master, provider or developer + I want to load all assets from the configured CDN + + Rule: Master + Background: + Given master is the provider + + Scenario: Asset host not configured + When master admin is logged in + Then assets shouldn't be loaded from the asset host + + Scenario: Master dashboard with asset host configured + When the asset host is set to "cdn.3scale.localhost" + When master admin is logged in + Then assets should be loaded from the asset host + + Rule: Provider + Background: + Given the asset host is set to "cdn.3scale.localhost" + And a provider is logged in + And the provider has one buyer + + Scenario: Provider dashboard with asset host configured + Then assets should be loaded from the asset host + + Scenario: Developer portal with asset host configured + When the buyer logs in to the provider + Then javascript assets should be loaded from the asset host diff --git a/features/step_definitions/asset_host_steps.rb b/features/step_definitions/asset_host_steps.rb new file mode 100644 index 0000000000..f0d6287000 --- /dev/null +++ b/features/step_definitions/asset_host_steps.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +Given /^the asset host is set to "(.*)"$/ do |asset_host| + cdn_url = "#{asset_host}:#{Capybara.current_session.server.port}" + Rails.configuration.asset_host = cdn_url + Rails.configuration.three_scale.asset_host = cdn_url +end + +Then /^((javascript|font)\s)?assets should be loaded from the asset host$/ do |asset_type| + cdn_url = Rails.configuration.asset_host + js_regexp = Regexp.new("https?://#{cdn_url}/packs.*?\\.js") + font_regexp = Regexp.new("https?://#{cdn_url}/packs.*?\\.eot") + + assert_not_nil Capybara.page.source.match js_regexp if ['javascript', nil].include? asset_type + assert_not_nil Capybara.page.source.match font_regexp if ['font', nil].include? asset_type +end + +Then /^assets shouldn't be loaded from the asset host$/ do + # When no CDN is set, we expect to find relative paths for fonts and js + js_regexp = Regexp.new('src="/packs.*?\.js"') + font_regexp = Regexp.new('url\(/packs.*?\.eot\)') + + assert_not_nil Capybara.page.source.match js_regexp + assert_not_nil Capybara.page.source.match font_regexp +end diff --git a/lib/tasks/webpacker.rake b/lib/tasks/webpacker.rake new file mode 100644 index 0000000000..f5327cf6cc --- /dev/null +++ b/lib/tasks/webpacker.rake @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +namespace :webpacker do + + task :check_asset_host do + if Rails.configuration.three_scale.asset_host.present? + STDERR.puts "*** Asset host must be null during compilation time. See https://github.com/3scale/porta/pull/3072 ***" + return false + end + end + + Rake::Task['webpacker:compile'].enhance [:check_asset_host] +end diff --git a/package.json b/package.json index 9a7e07bda0..dbc961b8eb 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "eslint-plugin-import": "^2.26.0", "eslint-plugin-jest": "24.7.0", "eslint-plugin-react": "^7.21.0", - "file-loader": "^1.1.6", + "file-loader": "^6.2.0", "hosted-git-info": "^2.7.1", "jest": "^27", "jest-transform-stub": "^2.0.0", diff --git a/test/unit/helpers/application_helper_test.rb b/test/unit/helpers/application_helper_test.rb index 9d0fe2b948..18a78ac5fb 100644 --- a/test/unit/helpers/application_helper_test.rb +++ b/test/unit/helpers/application_helper_test.rb @@ -22,6 +22,42 @@ def test_css_class assert_equal 'one two three', css_class('one', ['two'], three: true) end + class AssetHostTest < ApplicationHelperTest + setup do + @request = ActionDispatch::TestRequest.create + @asset_host = 'cdn.3scale.test.localhost' + end + + attr_reader :request + + test 'asset host is not configured' do + Rails.configuration.expects(:asset_host).returns(nil) + Rails.configuration.three_scale.expects(:asset_host).returns(@asset_host) + + result = rails_asset_host_url + + assert_equal '', result + end + + test "asset host is configured but it's value is empty" do + Rails.configuration.expects(:asset_host).returns(-> {}) + Rails.configuration.three_scale.expects(:asset_host).returns('') + + result = rails_asset_host_url + + assert_equal '', result + end + + test 'asset host is configured and has a proper value' do + Rails.configuration.expects(:asset_host).returns(-> {}) + Rails.configuration.three_scale.expects(:asset_host).returns(@asset_host) + + result = rails_asset_host_url + + assert_equal "#{request.protocol}#{@asset_host}", result + end + end + private def ability diff --git a/yarn.lock b/yarn.lock index e28e0e3056..8800a17667 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6411,14 +6411,6 @@ file-entry-cache@^6.0.1: dependencies: flat-cache "^3.0.4" -file-loader@^1.1.6: - version "1.1.11" - resolved "https://registry.yarnpkg.com/file-loader/-/file-loader-1.1.11.tgz#6fe886449b0f2a936e43cabaac0cdbfb369506f8" - integrity sha512-TGR4HU7HUsGg6GCOPJnFk06RhWgEWFLAGWiT6rcD+GRC2keU3s9RGJ+b3Z6/U73jwwNb2gKLJ7YCrp+jvU4ALg== - dependencies: - loader-utils "^1.0.2" - schema-utils "^0.4.5" - file-loader@^6.2.0: version "6.2.0" resolved "https://registry.yarnpkg.com/file-loader/-/file-loader-6.2.0.tgz#baef7cf8e1840df325e4390b4484879480eebe4d" @@ -8840,7 +8832,7 @@ loader-runner@^2.4.0: resolved "https://registry.yarnpkg.com/loader-runner/-/loader-runner-2.4.0.tgz#ed47066bfe534d7e84c4c7b9998c2a75607d9357" integrity sha512-Jsmr89RcXGIwivFY21FcRrisYZfvLMTWx5kOLc+JTxtpBOG6xML0vzbc6SEQG2FO9/4Fc3wW4LVcB5DmGflaRw== -loader-utils@^1.0.1, loader-utils@^1.0.2, loader-utils@^1.1.0, loader-utils@^1.2.3, loader-utils@^1.4.0: +loader-utils@^1.0.1, loader-utils@^1.1.0, loader-utils@^1.2.3, loader-utils@^1.4.0: version "1.4.2" resolved "https://registry.yarnpkg.com/loader-utils/-/loader-utils-1.4.2.tgz#29a957f3a63973883eb684f10ffd3d151fec01a3" integrity sha512-I5d00Pd/jwMD2QCduo657+YM/6L3KZu++pmX9VFncxaxvHcru9jx1lBaFft+r4Mt2jK0Yhp41XlRAihzPxHNCg== @@ -12387,14 +12379,6 @@ scheduler@^0.19.1: loose-envify "^1.1.0" object-assign "^4.1.1" -schema-utils@^0.4.5: - version "0.4.7" - resolved "https://registry.yarnpkg.com/schema-utils/-/schema-utils-0.4.7.tgz#ba74f597d2be2ea880131746ee17d0a093c68187" - integrity sha512-v/iwU6wvwGK8HbU9yi3/nhGzP0yGSuhQMzL6ySiec1FSrZZDkhm4noOSWzrNFo/jEc+SJY6jRTwuwbSXJPDUnQ== - dependencies: - ajv "^6.1.0" - ajv-keywords "^3.1.0" - schema-utils@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/schema-utils/-/schema-utils-1.0.0.tgz#0b79a93204d7b600d4b2850d1f66c2a34951c770"