From 8b28dce82ac244c6c5e67d8379e68e5742bfe875 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 12 Jan 2021 16:31:35 -0600 Subject: [PATCH 01/18] Deprecate Pleroma.Uploaders.S3, :public_endpoint --- config/config.exs | 15 +++++++++--- config/description.exs | 8 +------ config/test.exs | 3 +-- docs/configuration/cheatsheet.md | 5 +--- lib/pleroma/config/deprecation_warnings.ex | 24 ++++++++++++++++++- priv/templates/sample_config.eex | 10 ++++++-- .../config/deprecation_warnings_test.exs | 9 +++++++ 7 files changed, 55 insertions(+), 19 deletions(-) diff --git a/config/config.exs b/config/config.exs index 7b14fbfe5..2a0c6302c 100644 --- a/config/config.exs +++ b/config/config.exs @@ -64,14 +64,23 @@ config :pleroma, Pleroma.Upload, link_name: false, proxy_remote: false, filename_display_max_length: 30, - default_description: nil + default_description: nil, + base_url: nil config :pleroma, Pleroma.Uploaders.Local, uploads: "uploads" config :pleroma, Pleroma.Uploaders.S3, bucket: nil, - streaming_enabled: true, - public_endpoint: "https://s3.amazonaws.com" + bucket_namespace: nil, + truncated_namespace: false, + streaming_enabled: true + +config :ex_aws, :s3, + # host: "s3.wasabisys.com", # required if not Amazon AWS + access_key_id: nil, + secret_access_key: nil, + # region: nil, # example: "us-east-1" + scheme: "https://" config :pleroma, :emoji, shortcode_globs: ["/emoji/custom/**/*.png"], diff --git a/config/description.exs b/config/description.exs index f438a88ab..493d362d3 100644 --- a/config/description.exs +++ b/config/description.exs @@ -149,18 +149,12 @@ config :pleroma, :config_description, [ description: "S3 bucket namespace", suggestions: ["pleroma"] }, - %{ - key: :public_endpoint, - type: :string, - description: "S3 endpoint", - suggestions: ["https://s3.amazonaws.com"] - }, %{ key: :truncated_namespace, type: :string, description: "If you use S3 compatible service such as Digital Ocean Spaces or CDN, set folder name or \"\" etc." <> - " For example, when using CDN to S3 virtual host format, set \"\". At this time, write CNAME to CDN in public_endpoint." + " For example, when using CDN to S3 virtual host format, set \"\". At this time, write CNAME to CDN in Upload base_url." }, %{ key: :streaming_enabled, diff --git a/config/test.exs b/config/test.exs index 7fc457463..e482f38c8 100644 --- a/config/test.exs +++ b/config/test.exs @@ -117,8 +117,7 @@ config :pleroma, Pleroma.Web.ApiSpec.CastAndValidate, strict: true config :pleroma, Pleroma.Uploaders.S3, bucket: nil, - streaming_enabled: true, - public_endpoint: nil + streaming_enabled: true config :tzdata, :autoupdate, :disabled diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index 85551362c..c7d8a2dae 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -549,7 +549,7 @@ the source code is here: [kocaptcha](https://github.com/koto-bank/kocaptcha). Th * `uploader`: Which one of the [uploaders](#uploaders) to use. * `filters`: List of [upload filters](#upload-filters) to use. * `link_name`: When enabled Pleroma will add a `name` parameter to the url of the upload, for example `https://instance.tld/media/corndog.png?name=corndog.png`. This is needed to provide the correct filename in Content-Disposition headers when using filters like `Pleroma.Upload.Filter.Dedupe` -* `base_url`: The base URL to access a user-uploaded file. Useful when you want to proxy the media files via another host. +* `base_url`: The base URL to access a user-uploaded file. Useful when you want to host the media files via another domain or are using a 3rd party S3 provider. * `proxy_remote`: If you're using a remote uploader, Pleroma will proxy media requests instead of redirecting to it. * `proxy_opts`: Proxy options, see `Pleroma.ReverseProxy` documentation. * `filename_display_max_length`: Set max length of a filename to display. 0 = no limit. Default: 30. @@ -570,10 +570,7 @@ Don't forget to configure [Ex AWS S3](#ex-aws-s3-settings) * `bucket`: S3 bucket name. * `bucket_namespace`: S3 bucket namespace. -* `public_endpoint`: S3 endpoint that the user finally accesses(ex. "https://s3.dualstack.ap-northeast-1.amazonaws.com") * `truncated_namespace`: If you use S3 compatible service such as Digital Ocean Spaces or CDN, set folder name or "" etc. -For example, when using CDN to S3 virtual host format, set "". -At this time, write CNAME to CDN in public_endpoint. * `streaming_enabled`: Enable streaming uploads, when enabled the file will be sent to the server in chunks as it's being read. This may be unsupported by some providers, try disabling this if you have upload problems. #### Ex AWS S3 settings diff --git a/lib/pleroma/config/deprecation_warnings.ex b/lib/pleroma/config/deprecation_warnings.ex index 59c6b0f58..703a5273f 100644 --- a/lib/pleroma/config/deprecation_warnings.ex +++ b/lib/pleroma/config/deprecation_warnings.ex @@ -40,7 +40,8 @@ defmodule Pleroma.Config.DeprecationWarnings do :ok <- check_welcome_message_config(), :ok <- check_gun_pool_options(), :ok <- check_activity_expiration_config(), - :ok <- check_remote_ip_plug_name() do + :ok <- check_remote_ip_plug_name(), + :ok <- check_uploders_s3_public_endpoint() do :ok else _ -> @@ -193,4 +194,25 @@ defmodule Pleroma.Config.DeprecationWarnings do warning_preface ) end + + @spec check_uploders_s3_public_endpoint() :: :ok | nil + def check_uploders_s3_public_endpoint do + s3_config = Pleroma.Config.get([Pleroma.Uploaders.S3]) + + use_old_config = Keyword.has_key?(s3_config, :public_endpoint) + + if use_old_config do + Logger.error(""" + !!!DEPRECATION WARNING!!! + Your config is using the old setting for controlling the URL of media uploaded to your S3 bucket.\n + Please make the following change at your earliest convenience.\n + \n* `config :pleroma, Pleroma.Uploaders.S3, public_endpoint` is now equal to: + \n* `config :pleroma, Pleroma.Upload, base_url` + """) + + :error + else + :ok + end + end end diff --git a/priv/templates/sample_config.eex b/priv/templates/sample_config.eex index 2f5952ef1..0c2477e2c 100644 --- a/priv/templates/sample_config.eex +++ b/priv/templates/sample_config.eex @@ -49,12 +49,18 @@ config :pleroma, Pleroma.Uploaders.Local, uploads: "<%= uploads_dir %>" # sts: true # Configure S3 support if desired. -# The public S3 endpoint is different depending on region and provider, +# The public S3 endpoint (base_url) is different depending on region and provider, # consult your S3 provider's documentation for details on what to use. # +# config :pleroma, Pleroma.Upload, +# uploader: Pleroma.Uploaders.S3, +# base_url: "https://s3.amazonaws.com" +# # config :pleroma, Pleroma.Uploaders.S3, # bucket: "some-bucket", -# public_endpoint: "https://s3.amazonaws.com" +# bucket_namespace: "my-namespace", +# truncated_namespace: false, +# streaming_enabled: true # # Configure S3 credentials: # config :ex_aws, :s3, diff --git a/test/pleroma/config/deprecation_warnings_test.exs b/test/pleroma/config/deprecation_warnings_test.exs index f52629f8a..161bf6e90 100644 --- a/test/pleroma/config/deprecation_warnings_test.exs +++ b/test/pleroma/config/deprecation_warnings_test.exs @@ -94,6 +94,15 @@ defmodule Pleroma.Config.DeprecationWarningsTest do end) =~ "Your config is using old namespace for activity expiration configuration." end + test "check_uploders_s3_public_endpoint/0" do + clear_config(Pleroma.Uploaders.S3, public_endpoint: "https://fake.amazonaws.com/bucket/") + + assert capture_log(fn -> + DeprecationWarnings.check_uploders_s3_public_endpoint() + end) =~ + "Your config is using the old setting for controlling the URL of media uploaded to your S3 bucket." + end + describe "check_gun_pool_options/0" do test "await_up_timeout" do config = Config.get(:connections_pool) From 12528edc349a6ec10b1a1d9a7daf461823fdf928 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 12 Jan 2021 16:32:52 -0600 Subject: [PATCH 02/18] Fix another ad-hoc construction of the upload base_url --- lib/pleroma/upload.ex | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/lib/pleroma/upload.ex b/lib/pleroma/upload.ex index 51ca97f41..619a85e93 100644 --- a/lib/pleroma/upload.ex +++ b/lib/pleroma/upload.ex @@ -131,12 +131,7 @@ defmodule Pleroma.Upload do uploader: Keyword.get(opts, :uploader, Pleroma.Config.get([__MODULE__, :uploader])), filters: Keyword.get(opts, :filters, Pleroma.Config.get([__MODULE__, :filters])), description: Keyword.get(opts, :description), - base_url: - Keyword.get( - opts, - :base_url, - Pleroma.Config.get([__MODULE__, :base_url], Pleroma.Web.base_url()) - ) + base_url: base_url() } end @@ -217,14 +212,7 @@ defmodule Pleroma.Upload do "" end - prefix = - if is_nil(Pleroma.Config.get([__MODULE__, :base_url])) do - "media" - else - "" - end - - [base_url, prefix, path] + [base_url, path] |> Path.join() end From c35e6fb51615fa3d22cfedeac2158ee62ea9b663 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 12 Jan 2021 16:34:24 -0600 Subject: [PATCH 03/18] Provide a non-nil fallback for Upload.base_url/0 for tests using TestUploaderSuccess as the uploader --- lib/pleroma/upload.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/upload.ex b/lib/pleroma/upload.ex index 619a85e93..e714dc57b 100644 --- a/lib/pleroma/upload.ex +++ b/lib/pleroma/upload.ex @@ -249,7 +249,7 @@ defmodule Pleroma.Upload do end _ -> - public_endpoint || upload_base_url + public_endpoint || upload_base_url || Pleroma.Web.base_url() <> "/media/" end end end From e87cca97e62d8464c87c7335741f54c2299cc0d6 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 12 Jan 2021 16:35:10 -0600 Subject: [PATCH 04/18] Fix tests relying on old behavior. Use the Upload.base_url, Luke. --- test/pleroma/upload_test.exs | 4 ++-- test/pleroma/uploaders/s3_test.exs | 13 +++++++++---- test/pleroma/user/backup_test.exs | 4 ++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/test/pleroma/upload_test.exs b/test/pleroma/upload_test.exs index f52d4dff6..cea161d8c 100644 --- a/test/pleroma/upload_test.exs +++ b/test/pleroma/upload_test.exs @@ -148,8 +148,8 @@ defmodule Pleroma.UploadTest do {:ok, data} = Upload.store(file, filters: [Pleroma.Upload.Filter.Dedupe]) assert List.first(data["url"])["href"] == - Pleroma.Web.base_url() <> - "/media/e30397b58d226d6583ab5b8b3c5defb0c682bda5c31ef07a9f57c1c4986e3781.jpg" + Pleroma.Upload.base_url() <> + "e30397b58d226d6583ab5b8b3c5defb0c682bda5c31ef07a9f57c1c4986e3781.jpg" end test "copies the file to the configured folder without deduping" do diff --git a/test/pleroma/uploaders/s3_test.exs b/test/pleroma/uploaders/s3_test.exs index 344cf7abe..f399f8ae5 100644 --- a/test/pleroma/uploaders/s3_test.exs +++ b/test/pleroma/uploaders/s3_test.exs @@ -16,9 +16,12 @@ defmodule Pleroma.Uploaders.S3Test do uploader: Pleroma.Uploaders.S3 ) + clear_config(Pleroma.Upload, + base_url: "https://s3.amazonaws.com" + ) + clear_config(Pleroma.Uploaders.S3, - bucket: "test_bucket", - public_endpoint: "https://s3.amazonaws.com" + bucket: "test_bucket" ) end @@ -33,10 +36,11 @@ defmodule Pleroma.Uploaders.S3Test do test "it returns path without bucket when truncated_namespace set to ''" do Config.put([Pleroma.Uploaders.S3], bucket: "test_bucket", - public_endpoint: "https://s3.amazonaws.com", truncated_namespace: "" ) + Config.put([Pleroma.Upload], base_url: "https://s3.amazonaws.com") + assert S3.get_file("test_image.jpg") == { :ok, {:url, "https://s3.amazonaws.com/test_image.jpg"} @@ -46,10 +50,11 @@ defmodule Pleroma.Uploaders.S3Test do test "it returns path with bucket namespace when namespace is set" do Config.put([Pleroma.Uploaders.S3], bucket: "test_bucket", - public_endpoint: "https://s3.amazonaws.com", bucket_namespace: "family" ) + Config.put([Pleroma.Upload], base_url: "https://s3.amazonaws.com") + assert S3.get_file("test_image.jpg") == { :ok, {:url, "https://s3.amazonaws.com/family:test_bucket/test_image.jpg"} diff --git a/test/pleroma/user/backup_test.exs b/test/pleroma/user/backup_test.exs index f68e4a029..01a1ed962 100644 --- a/test/pleroma/user/backup_test.exs +++ b/test/pleroma/user/backup_test.exs @@ -196,11 +196,11 @@ defmodule Pleroma.User.BackupTest do describe "it uploads and deletes a backup archive" do setup do clear_config(Pleroma.Uploaders.S3, - bucket: "test_bucket", - public_endpoint: "https://s3.amazonaws.com" + bucket: "test_bucket" ) clear_config([Pleroma.Upload, :uploader]) + clear_config([Pleroma.Upload, base_url: "https://s3.amazonaws.com"]) user = insert(:user, %{nickname: "cofe", name: "Cofe", ap_id: "http://cofe.io/users/cofe"}) From 2b93351bd7b1377793256a14c0356e1dccf36d2e Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 12 Jan 2021 16:40:29 -0600 Subject: [PATCH 05/18] Document deprecation --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25b24bf07..31d6a7561 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Search: When using Postgres 11+, Pleroma will use the `websearch_to_tsvector` function to parse search queries. - Emoji: Support the full Unicode 13.1 set of Emoji for reactions, plus regional indicators. - Admin API: Reports now ordered by newest +- Deprecated `Pleroma.Uploaders.S3, :public_endpoint`. Now `Pleroma.Upload, :base_url` is the standard configuration key for all uploaders. ### Added From 67e888498c16c8bba434afe91cb3e0a83b9da8bb Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 12 Jan 2021 16:42:43 -0600 Subject: [PATCH 06/18] Switch another test to Upload.base_url/0 --- test/pleroma/upload_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/pleroma/upload_test.exs b/test/pleroma/upload_test.exs index cea161d8c..8f84a0be1 100644 --- a/test/pleroma/upload_test.exs +++ b/test/pleroma/upload_test.exs @@ -133,7 +133,7 @@ defmodule Pleroma.UploadTest do assert %{"url" => [%{"href" => url}]} = data - assert String.starts_with?(url, Pleroma.Web.base_url() <> "/media/") + assert String.starts_with?(url, Pleroma.Upload.base_url()) end test "copies the file to the configured folder with deduping" do From f0ab60189e0749ca207b483b291c90f892dce6a3 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 13 Jan 2021 11:54:00 -0600 Subject: [PATCH 07/18] truncated_namespace should default to nil --- config/config.exs | 2 +- lib/pleroma/upload.ex | 6 ++++-- priv/templates/sample_config.eex | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/config/config.exs b/config/config.exs index 2a0c6302c..ef3baed93 100644 --- a/config/config.exs +++ b/config/config.exs @@ -72,7 +72,7 @@ config :pleroma, Pleroma.Uploaders.Local, uploads: "uploads" config :pleroma, Pleroma.Uploaders.S3, bucket: nil, bucket_namespace: nil, - truncated_namespace: false, + truncated_namespace: nil, streaming_enabled: true config :ex_aws, :s3, diff --git a/lib/pleroma/upload.ex b/lib/pleroma/upload.ex index e714dc57b..e13d40c5a 100644 --- a/lib/pleroma/upload.ex +++ b/lib/pleroma/upload.ex @@ -229,13 +229,15 @@ defmodule Pleroma.Upload do Pleroma.Uploaders.S3 -> bucket = Config.get([Pleroma.Uploaders.S3, :bucket]) + truncated_namespace = Config.get([Pleroma.Uploaders.S3, :truncated_namespace]) + namespace = Config.get([Pleroma.Uploaders.S3, :bucket_namespace]) bucket_with_namespace = cond do - truncated_namespace = Config.get([Pleroma.Uploaders.S3, :truncated_namespace]) -> + !is_nil(truncated_namespace) -> truncated_namespace - namespace = Config.get([Pleroma.Uploaders.S3, :bucket_namespace]) -> + !is_nil(namespace) -> namespace <> ":" <> bucket true -> diff --git a/priv/templates/sample_config.eex b/priv/templates/sample_config.eex index 0c2477e2c..42f496ded 100644 --- a/priv/templates/sample_config.eex +++ b/priv/templates/sample_config.eex @@ -59,7 +59,7 @@ config :pleroma, Pleroma.Uploaders.Local, uploads: "<%= uploads_dir %>" # config :pleroma, Pleroma.Uploaders.S3, # bucket: "some-bucket", # bucket_namespace: "my-namespace", -# truncated_namespace: false, +# truncated_namespace: nil, # streaming_enabled: true # # Configure S3 credentials: From 5627f3642fd96b678bdd5c3b9f3da0dbb038d75c Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 13 Jan 2021 11:54:45 -0600 Subject: [PATCH 08/18] Not needed in test.exs --- config/test.exs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/config/test.exs b/config/test.exs index e482f38c8..76c7a2c67 100644 --- a/config/test.exs +++ b/config/test.exs @@ -115,10 +115,6 @@ config :pleroma, Pleroma.Web.Plugs.RemoteIp, enabled: false config :pleroma, Pleroma.Web.ApiSpec.CastAndValidate, strict: true -config :pleroma, Pleroma.Uploaders.S3, - bucket: nil, - streaming_enabled: true - config :tzdata, :autoupdate, :disabled config :pleroma, :mrf, policies: [] From 94e51808461cd5a6148c6782159fa3f0ecc14638 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 13 Jan 2021 12:00:48 -0600 Subject: [PATCH 09/18] Consistent style --- test/pleroma/uploaders/s3_test.exs | 9 ++------- test/pleroma/user/backup_test.exs | 9 +++------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/test/pleroma/uploaders/s3_test.exs b/test/pleroma/uploaders/s3_test.exs index f399f8ae5..da3a57163 100644 --- a/test/pleroma/uploaders/s3_test.exs +++ b/test/pleroma/uploaders/s3_test.exs @@ -12,13 +12,8 @@ defmodule Pleroma.Uploaders.S3Test do import ExUnit.CaptureLog setup do - clear_config(Pleroma.Upload, - uploader: Pleroma.Uploaders.S3 - ) - - clear_config(Pleroma.Upload, - base_url: "https://s3.amazonaws.com" - ) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.S3) + clear_config([Pleroma.Upload, :base_url], "https://s3.amazonaws.com") clear_config(Pleroma.Uploaders.S3, bucket: "test_bucket" diff --git a/test/pleroma/user/backup_test.exs b/test/pleroma/user/backup_test.exs index 01a1ed962..64a92cb7d 100644 --- a/test/pleroma/user/backup_test.exs +++ b/test/pleroma/user/backup_test.exs @@ -195,12 +195,9 @@ defmodule Pleroma.User.BackupTest do describe "it uploads and deletes a backup archive" do setup do - clear_config(Pleroma.Uploaders.S3, - bucket: "test_bucket" - ) - - clear_config([Pleroma.Upload, :uploader]) - clear_config([Pleroma.Upload, base_url: "https://s3.amazonaws.com"]) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.S3) + clear_config([Pleroma.Upload, :base_url], "https://s3.amazonaws.com") + clear_config([Pleroma.Uploaders.S3, :bucket], "test_bucket") user = insert(:user, %{nickname: "cofe", name: "Cofe", ap_id: "http://cofe.io/users/cofe"}) From ba234d3c73ee6d6e96150928d0853c51783abd1d Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 13 Jan 2021 12:01:01 -0600 Subject: [PATCH 10/18] Unnecessary duplication here --- test/pleroma/uploaders/s3_test.exs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/pleroma/uploaders/s3_test.exs b/test/pleroma/uploaders/s3_test.exs index da3a57163..30653aad2 100644 --- a/test/pleroma/uploaders/s3_test.exs +++ b/test/pleroma/uploaders/s3_test.exs @@ -48,8 +48,6 @@ defmodule Pleroma.Uploaders.S3Test do bucket_namespace: "family" ) - Config.put([Pleroma.Upload], base_url: "https://s3.amazonaws.com") - assert S3.get_file("test_image.jpg") == { :ok, {:url, "https://s3.amazonaws.com/family:test_bucket/test_image.jpg"} From fd9a0ac32943f7869e950524d4ed7a052f609e5c Mon Sep 17 00:00:00 2001 From: Lain Soykaf Date: Thu, 14 Jan 2021 20:18:45 +0100 Subject: [PATCH 11/18] BackupTest: Fix s3 test. --- test/pleroma/user/backup_test.exs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/pleroma/user/backup_test.exs b/test/pleroma/user/backup_test.exs index 64a92cb7d..108928c09 100644 --- a/test/pleroma/user/backup_test.exs +++ b/test/pleroma/user/backup_test.exs @@ -195,7 +195,6 @@ defmodule Pleroma.User.BackupTest do describe "it uploads and deletes a backup archive" do setup do - clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.S3) clear_config([Pleroma.Upload, :base_url], "https://s3.amazonaws.com") clear_config([Pleroma.Uploaders.S3, :bucket], "test_bucket") @@ -216,7 +215,8 @@ defmodule Pleroma.User.BackupTest do end test "S3", %{path: path, backup: backup} do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.S3) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.S3) + clear_config([Pleroma.Uploaders.S3, :streaming_enabled], false) with_mock ExAws, request: fn @@ -226,13 +226,10 @@ defmodule Pleroma.User.BackupTest do assert {:ok, %Pleroma.Upload{}} = Backup.upload(backup, path) assert {:ok, _backup} = Backup.delete(backup) end - - with_mock ExAws, request: fn %{http_method: :delete} -> {:ok, %{status_code: 204}} end do - end end test "Local", %{path: path, backup: backup} do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) assert {:ok, %Pleroma.Upload{}} = Backup.upload(backup, path) assert {:ok, _backup} = Backup.delete(backup) From fb47e83adc074f994714c83618b6de17915d0556 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 14 Jan 2021 13:53:35 -0600 Subject: [PATCH 12/18] Add ConfigDB migration --- ...210113225652_deprecate_public_endpoint.exs | 57 ++++++++++++++++++ .../public_endpoint_deprecation_test.exs | 60 +++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 priv/repo/migrations/20210113225652_deprecate_public_endpoint.exs create mode 100644 test/pleroma/repo/migrations/public_endpoint_deprecation_test.exs diff --git a/priv/repo/migrations/20210113225652_deprecate_public_endpoint.exs b/priv/repo/migrations/20210113225652_deprecate_public_endpoint.exs new file mode 100644 index 000000000..d2e6e3c56 --- /dev/null +++ b/priv/repo/migrations/20210113225652_deprecate_public_endpoint.exs @@ -0,0 +1,57 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Repo.Migrations.DeprecatePublicEndpoint do + use Ecto.Migration + + def up do + with %Pleroma.ConfigDB{} = s3_config <- + Pleroma.ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Uploaders.S3}), + %Pleroma.ConfigDB{} = upload_config <- + Pleroma.ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Upload}) do + public_endpoint = s3_config.value[:public_endpoint] + + if !is_nil(public_endpoint) do + upload_value = upload_config.value |> Keyword.merge(base_url: public_endpoint) + + upload_config + |> Ecto.Changeset.change(value: upload_value) + |> Pleroma.Repo.update() + + s3_value = s3_config.value |> Keyword.delete(:public_endpoint) + + s3_config + |> Ecto.Changeset.change(value: s3_value) + |> Pleroma.Repo.update() + end + else + _ -> :ok + end + end + + def down do + with %Pleroma.ConfigDB{} = upload_config <- + Pleroma.ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Upload}), + %Pleroma.ConfigDB{} = s3_config <- + Pleroma.ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Uploaders.S3}) do + base_url = upload_config.value[:base_url] + + if !is_nil(base_url) do + s3_value = s3_config.value |> Keyword.merge(public_endpoint: base_url) + + s3_config + |> Ecto.Changeset.change(value: s3_value) + |> Pleroma.Repo.update() + + upload_value = upload_config.value |> Keyword.delete(:base_url) + + upload_config + |> Ecto.Changeset.change(value: upload_value) + |> Pleroma.Repo.update() + end + else + _ -> :ok + end + end +end diff --git a/test/pleroma/repo/migrations/public_endpoint_deprecation_test.exs b/test/pleroma/repo/migrations/public_endpoint_deprecation_test.exs new file mode 100644 index 000000000..b68d24bfc --- /dev/null +++ b/test/pleroma/repo/migrations/public_endpoint_deprecation_test.exs @@ -0,0 +1,60 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Repo.Migrations.DeprecatePublicEndpointTest do + use Pleroma.DataCase + import Pleroma.Factory + import Pleroma.Tests.Helpers + alias Pleroma.ConfigDB + + setup do: clear_config(Pleroma.Upload) + setup do: clear_config(Pleroma.Uploaders.S3) + setup_all do: require_migration("20210113225652_deprecate_public_endpoint") + + test "up/0 migrates public_endpoint to base_url", %{migration: migration} do + s3_values = [ + public_endpoint: "https://coolhost.com/", + bucket: "secret_bucket" + ] + + insert(:config, group: :pleroma, key: Pleroma.Uploaders.S3, value: s3_values) + + upload_values = [ + uploader: Pleroma.Uploaders.S3 + ] + + insert(:config, group: :pleroma, key: Pleroma.Upload, value: upload_values) + + migration.up() + + assert [bucket: "secret_bucket"] == + ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Uploaders.S3}).value + + assert [uploader: Pleroma.Uploaders.S3, base_url: "https://coolhost.com/"] == + ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Upload}).value + end + + test "down/0 reverts base_url to public_endpoint", %{migration: migration} do + s3_values = [ + bucket: "secret_bucket" + ] + + insert(:config, group: :pleroma, key: Pleroma.Uploaders.S3, value: s3_values) + + upload_values = [ + uploader: Pleroma.Uploaders.S3, + base_url: "https://coolhost.com/" + ] + + insert(:config, group: :pleroma, key: Pleroma.Upload, value: upload_values) + + migration.down() + + assert [bucket: "secret_bucket", public_endpoint: "https://coolhost.com/"] == + ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Uploaders.S3}).value + + assert [uploader: Pleroma.Uploaders.S3] == + ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Upload}).value + end +end From 12c8ce481c1afec69a9f401bcfffae63744dfb09 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 14 Jan 2021 13:58:52 -0600 Subject: [PATCH 13/18] Bump Copyright year --- .../migrations/20210113225652_deprecate_public_endpoint.exs | 2 +- .../repo/migrations/public_endpoint_deprecation_test.exs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/priv/repo/migrations/20210113225652_deprecate_public_endpoint.exs b/priv/repo/migrations/20210113225652_deprecate_public_endpoint.exs index d2e6e3c56..6f470a459 100644 --- a/priv/repo/migrations/20210113225652_deprecate_public_endpoint.exs +++ b/priv/repo/migrations/20210113225652_deprecate_public_endpoint.exs @@ -1,5 +1,5 @@ # Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors +# Copyright © 2017-2021 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Repo.Migrations.DeprecatePublicEndpoint do diff --git a/test/pleroma/repo/migrations/public_endpoint_deprecation_test.exs b/test/pleroma/repo/migrations/public_endpoint_deprecation_test.exs index b68d24bfc..2ffc1b145 100644 --- a/test/pleroma/repo/migrations/public_endpoint_deprecation_test.exs +++ b/test/pleroma/repo/migrations/public_endpoint_deprecation_test.exs @@ -1,5 +1,5 @@ # Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors +# Copyright © 2017-2021 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Repo.Migrations.DeprecatePublicEndpointTest do From 0b725f5d216cfd2b11f81cddd792338c23161a60 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 14 Jan 2021 16:00:32 -0600 Subject: [PATCH 14/18] Lint --- ...nt_deprecation_test.exs => deprecate_public_endpoint_test.exs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/pleroma/repo/migrations/{public_endpoint_deprecation_test.exs => deprecate_public_endpoint_test.exs} (100%) diff --git a/test/pleroma/repo/migrations/public_endpoint_deprecation_test.exs b/test/pleroma/repo/migrations/deprecate_public_endpoint_test.exs similarity index 100% rename from test/pleroma/repo/migrations/public_endpoint_deprecation_test.exs rename to test/pleroma/repo/migrations/deprecate_public_endpoint_test.exs From d0e0396528c55f1b61c1d48452e855ea69ec3e89 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 14 Jan 2021 17:49:37 -0600 Subject: [PATCH 15/18] Hack to fix tests not passing. Unclear why the filters are being set to nil. Both of these changes are needed or it doesn't work. --- lib/pleroma/upload/filter.ex | 2 ++ test/support/data_case.ex | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/upload/filter.ex b/lib/pleroma/upload/filter.ex index 661135634..367acd214 100644 --- a/lib/pleroma/upload/filter.ex +++ b/lib/pleroma/upload/filter.ex @@ -43,4 +43,6 @@ defmodule Pleroma.Upload.Filter do error end end + + def filter(nil, upload), do: filter([], upload) end diff --git a/test/support/data_case.ex b/test/support/data_case.ex index 0b41f0f63..23c858d2a 100644 --- a/test/support/data_case.ex +++ b/test/support/data_case.ex @@ -107,7 +107,7 @@ defmodule Pleroma.DataCase do def ensure_local_uploader(context) do test_uploader = Map.get(context, :uploader, Pleroma.Uploaders.Local) uploader = Pleroma.Config.get([Pleroma.Upload, :uploader]) - filters = Pleroma.Config.get([Pleroma.Upload, :filters]) + filters = Pleroma.Config.get([Pleroma.Upload, :filters]) || [] Pleroma.Config.put([Pleroma.Upload, :uploader], test_uploader) Pleroma.Config.put([Pleroma.Upload, :filters], []) From 3f88e33a71ce02cdea722c322f1e86672aa5ff69 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sat, 16 Jan 2021 23:05:31 +0300 Subject: [PATCH 16/18] [#3251] Fixed wrong test-env config setting for [Pleroma.Upload]. Refactoring. Added warning to `clear_config/_` to minimize such issues in future. --- lib/pleroma/upload/filter.ex | 2 -- test/pleroma/object_test.exs | 24 ++++++++++++------------ test/pleroma/scheduled_activity_test.exs | 7 +++---- test/pleroma/uploaders/s3_test.exs | 8 +++----- test/support/data_case.ex | 15 +++++---------- test/support/helpers.ex | 12 ++++++++++++ 6 files changed, 35 insertions(+), 33 deletions(-) diff --git a/lib/pleroma/upload/filter.ex b/lib/pleroma/upload/filter.ex index 367acd214..661135634 100644 --- a/lib/pleroma/upload/filter.ex +++ b/lib/pleroma/upload/filter.ex @@ -43,6 +43,4 @@ defmodule Pleroma.Upload.Filter do error end end - - def filter(nil, upload), do: filter([], upload) end diff --git a/test/pleroma/object_test.exs b/test/pleroma/object_test.exs index fe7f37e7c..3150c8e01 100644 --- a/test/pleroma/object_test.exs +++ b/test/pleroma/object_test.exs @@ -78,8 +78,8 @@ defmodule Pleroma.ObjectTest do setup do: clear_config([:instance, :cleanup_attachments]) test "Disabled via config" do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) - Pleroma.Config.put([:instance, :cleanup_attachments], false) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([:instance, :cleanup_attachments], false) file = %Plug.Upload{ content_type: "image/jpeg", @@ -112,8 +112,8 @@ defmodule Pleroma.ObjectTest do end test "in subdirectories" do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) - Pleroma.Config.put([:instance, :cleanup_attachments], true) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([:instance, :cleanup_attachments], true) file = %Plug.Upload{ content_type: "image/jpeg", @@ -146,9 +146,9 @@ defmodule Pleroma.ObjectTest do end test "with dedupe enabled" do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) - Pleroma.Config.put([Pleroma.Upload, :filters], [Pleroma.Upload.Filter.Dedupe]) - Pleroma.Config.put([:instance, :cleanup_attachments], true) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([Pleroma.Upload, :filters], [Pleroma.Upload.Filter.Dedupe]) + clear_config([:instance, :cleanup_attachments], true) uploads_dir = Pleroma.Config.get!([Pleroma.Uploaders.Local, :uploads]) @@ -184,8 +184,8 @@ defmodule Pleroma.ObjectTest do end test "with objects that have legacy data.url attribute" do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) - Pleroma.Config.put([:instance, :cleanup_attachments], true) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([:instance, :cleanup_attachments], true) file = %Plug.Upload{ content_type: "image/jpeg", @@ -220,9 +220,9 @@ defmodule Pleroma.ObjectTest do end test "With custom base_url" do - Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) - Pleroma.Config.put([Pleroma.Upload, :base_url], "https://sub.domain.tld/dir/") - Pleroma.Config.put([:instance, :cleanup_attachments], true) + clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + clear_config([Pleroma.Upload, :base_url], "https://sub.domain.tld/dir/") + clear_config([:instance, :cleanup_attachments], true) file = %Plug.Upload{ content_type: "image/jpeg", diff --git a/test/pleroma/scheduled_activity_test.exs b/test/pleroma/scheduled_activity_test.exs index 7faa5660d..902d1d99c 100644 --- a/test/pleroma/scheduled_activity_test.exs +++ b/test/pleroma/scheduled_activity_test.exs @@ -4,15 +4,14 @@ defmodule Pleroma.ScheduledActivityTest do use Pleroma.DataCase - alias Pleroma.DataCase + alias Pleroma.ScheduledActivity + import Pleroma.Factory setup do: clear_config([ScheduledActivity, :enabled]) - setup context do - DataCase.ensure_local_uploader(context) - end + setup [:ensure_local_uploader] describe "creation" do test "scheduled activities with jobs when ScheduledActivity enabled" do diff --git a/test/pleroma/uploaders/s3_test.exs b/test/pleroma/uploaders/s3_test.exs index 30653aad2..242dc0d50 100644 --- a/test/pleroma/uploaders/s3_test.exs +++ b/test/pleroma/uploaders/s3_test.exs @@ -14,10 +14,8 @@ defmodule Pleroma.Uploaders.S3Test do setup do clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.S3) clear_config([Pleroma.Upload, :base_url], "https://s3.amazonaws.com") - - clear_config(Pleroma.Uploaders.S3, - bucket: "test_bucket" - ) + clear_config([Pleroma.Uploaders.S3]) + clear_config([Pleroma.Uploaders.S3, :bucket], "test_bucket") end describe "get_file/1" do @@ -34,7 +32,7 @@ defmodule Pleroma.Uploaders.S3Test do truncated_namespace: "" ) - Config.put([Pleroma.Upload], base_url: "https://s3.amazonaws.com") + Config.put([Pleroma.Upload, :base_url], "https://s3.amazonaws.com") assert S3.get_file("test_image.jpg") == { :ok, diff --git a/test/support/data_case.ex b/test/support/data_case.ex index 23c858d2a..0427682a2 100644 --- a/test/support/data_case.ex +++ b/test/support/data_case.ex @@ -18,6 +18,8 @@ defmodule Pleroma.DataCase do use ExUnit.CaseTemplate + import Pleroma.Tests.Helpers, only: [clear_config: 2] + using do quote do alias Pleroma.Repo @@ -105,17 +107,10 @@ defmodule Pleroma.DataCase do end def ensure_local_uploader(context) do - test_uploader = Map.get(context, :uploader, Pleroma.Uploaders.Local) - uploader = Pleroma.Config.get([Pleroma.Upload, :uploader]) - filters = Pleroma.Config.get([Pleroma.Upload, :filters]) || [] + test_uploader = Map.get(context, :uploader) || Pleroma.Uploaders.Local - Pleroma.Config.put([Pleroma.Upload, :uploader], test_uploader) - Pleroma.Config.put([Pleroma.Upload, :filters], []) - - on_exit(fn -> - Pleroma.Config.put([Pleroma.Upload, :uploader], uploader) - Pleroma.Config.put([Pleroma.Upload, :filters], filters) - end) + clear_config([Pleroma.Upload, :uploader], test_uploader) + clear_config([Pleroma.Upload, :filters], []) :ok end diff --git a/test/support/helpers.ex b/test/support/helpers.ex index 15e8cbd9d..db38a1e81 100644 --- a/test/support/helpers.ex +++ b/test/support/helpers.ex @@ -8,6 +8,8 @@ defmodule Pleroma.Tests.Helpers do """ alias Pleroma.Config + require Logger + defmacro clear_config(config_path) do quote do clear_config(unquote(config_path)) do @@ -18,6 +20,7 @@ defmodule Pleroma.Tests.Helpers do defmacro clear_config(config_path, do: yield) do quote do initial_setting = Config.fetch(unquote(config_path)) + unquote(yield) on_exit(fn -> @@ -35,6 +38,15 @@ defmodule Pleroma.Tests.Helpers do end defmacro clear_config(config_path, temp_setting) do + # NOTE: `clear_config([section, key], value)` != `clear_config([section], key: value)` (!) + # Displaying a warning to prevent unintentional clearing of all but one keys in section + if Keyword.keyword?(temp_setting) and length(temp_setting) == 1 do + Logger.warn( + "Please change to `clear_config([section]); clear_config([section, key], value)`: " <> + "#{inspect(config_path)}, #{inspect(temp_setting)}" + ) + end + quote do clear_config(unquote(config_path)) do Config.put(unquote(config_path), unquote(temp_setting)) From 9988d9261c2c933ecb595d0b98790b40f3b44f52 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 20 Jan 2021 16:33:00 -0600 Subject: [PATCH 17/18] Add bucket_namespace to be extra certain truncated_namespace works --- test/pleroma/uploaders/s3_test.exs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/pleroma/uploaders/s3_test.exs b/test/pleroma/uploaders/s3_test.exs index 242dc0d50..991052596 100644 --- a/test/pleroma/uploaders/s3_test.exs +++ b/test/pleroma/uploaders/s3_test.exs @@ -29,6 +29,7 @@ defmodule Pleroma.Uploaders.S3Test do test "it returns path without bucket when truncated_namespace set to ''" do Config.put([Pleroma.Uploaders.S3], bucket: "test_bucket", + bucket_namespace: "myaccount", truncated_namespace: "" ) From 086100e3b7cad827c0f377fdcc4ae9d3b66327c7 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 20 Jan 2021 16:39:39 -0600 Subject: [PATCH 18/18] Consistent comment style for :ex_aws --- config/config.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.exs b/config/config.exs index ef3baed93..dc5964fc9 100644 --- a/config/config.exs +++ b/config/config.exs @@ -79,7 +79,7 @@ config :ex_aws, :s3, # host: "s3.wasabisys.com", # required if not Amazon AWS access_key_id: nil, secret_access_key: nil, - # region: nil, # example: "us-east-1" + # region: "us-east-1", # may be required for Amazon AWS scheme: "https://" config :pleroma, :emoji,