Without this change the IdpMetaParser would give an error
in the Devise initializer when starting the application.
I found it annoying to have to connect to the VPN so
I decided to add this condition.
Reviewer, feel free to consider this commit unnecessary
and ask to revert it.
We were having an issue because there was a difference of about 11
seconds between the local times of our machines and the time of the IDP
server. Since right now we can't guarantee the time of these machines is
fully synchronized, for now we're adding a margin of error of one
minute.
The default "puma_bind" setting was reintroduced in capistrano3-puma 6.2.0
as "unix://.../tmp/sockets/puma.sock". We no longer need to define it
explicitly in deploy.rb.
NOTE: Although the official capistrano3-puma README still states
that "puma_bind" must be explicitly set after upgrading to v6,
this is no longer true since version 6.2.0. The default value
("unix://.../tmp/sockets/puma.sock") has been restored.
We remove our custom puma.service.erb template located at
lib/capistrano/templates, as the upstream capistrano3-puma gem (v6.2.0)
now supports all the adjustments we previously needed.
This template was originally copied and modified in
PR #5842 (Bump capistrano3-puma from 5.2.0 to 6.0.0),
with the following relevant changes:
1. WatchdogSec=0
We disabled systemd's watchdog feature by setting WatchdogSec=0.
This is now covered upstream since v6.1.0 via:
> <% if fetch(:puma_systemd_watchdog_sec) && fetch(:puma_systemd_watchdog_sec) > 0 %>
> WatchdogSec=<%= fetch(:puma_systemd_watchdog_sec) %>
> <% end %>
To preserve the same behavior, we now explicitly set:
> set :puma_systemd_watchdog_sec, 0
in config/deploy.rb, instead of relying on the template override.
2. SyslogIdentifier=puma
In our custom template, we replaced the default line:
> SyslogIdentifier=<%= fetch(:puma_service_unit_name) %>
with:
> SyslogIdentifier=puma
This was done to avoid introducing a new config variable
in the installer. However, now that we want to remove the overridden template,
we must accept the default behavior from the gem:
> set :puma_service_unit_name, -> { "puma_#{fetch(:application)}_#{fetch(:stage)}" }
As a result, the generated SyslogIdentifier will change from "puma" to something
like "puma_consul_staging".
The only implication is that we'll open a PR in the installer
to replace the hardcoded "puma" with the dynamic default,
so everything stays consistent.
Another option would be to override puma_service_unit_name
with "puma" to keep the SyslogIdentifier exactly as before.
However, this would also affect the names of the systemd services and sockets
(e.g., puma.service instead of puma_consul_staging.service),
which may lead to conflicts or require manual cleanup steps like disabling
and removing the old units. For now, we prefer to avoid that.
3. After=network.target
We had removed the "syslog.target" from the "After=" directive,
as we didn't depend on syslog.
However, keeping "After=syslog.target network.target" does not negatively impact us,
since Puma logs to files directly using StandardOutput=append:....
So we can also adopt the default value from template here.
4. ExecStart
In version 6.2.0, a small change was made to the template logic for the ExecStart line.
This change does not affect us. Since we don't define puma_use_login_shell,
the behavior remains exactly the same as before.
With these changes already supported in the gem, we no longer need
a local copy of the puma service template.
Bumps [rubocop](https://github.com/rubocop/rubocop) from 1.71.2 to 1.75.8.
- [Release notes](https://github.com/rubocop/rubocop/releases)
- [Changelog](https://github.com/rubocop/rubocop/blob/master/CHANGELOG.md)
- [Commits](https://github.com/rubocop/rubocop/compare/v1.71.2...v1.75.8)
---
updated-dependencies:
- dependency-name: rubocop
dependency-version: 1.75.8
dependency-type: direct:development
update-type: version-update:semver-minor
...
Notes:
This commit also includes several style and lint fixes required after
updating RuboCop:
- Removed redundant parentheses now detected by improved
'Style/RedundantParentheses' (1.72 and 1.75.3).
- Replaced ternary expressions with logical OR when the ternary was
returning 'true', as flagged by 'Style/RedundantCondition' (1.73).
- Adjusted block variables to resolve new 'Lint/ShadowingOuterLocalVariable'
offenses (1.75), helping avoid future conflicts during upgrades with
'rails app:updates'
Signed-off-by: dependabot[bot] <support@github.com>
Ahoy 2.0.0 [1] introduced automatic UUID generation for visit_token and
visitor_token. As a result, the custom ensure_uuid method is no longer
needed and can be safely removed from the initializer.
Since we aren't manually generating UUIDs anymore, we no longer need
the uuidtools dependency.
[1] https://github.com/ankane/ahoy/blob/v2.0.0/README.md#token-generation
IMHO, Spring no longer provides benefits in this project and:
- Spring was already disabled in the test environment since commit e4e0cb5d47
- Rails removed Spring as a default installation option in 2021 [1]
[1] PR #42997 from https://github.com/rails/rails/
capistrano3-puma v6.0.0 updated the defaults for puma_access_log and
puma_error_log to use a single file based on puma_env, like:
> set_if_empty :puma_access_log, -> { File.join(shared_path, 'log', "#{fetch(:puma_env)}.log") }
> set_if_empty :puma_error_log, -> { File.join(shared_path, 'log', "#{fetch(:puma_env)}.log") }
However, our installer expect:
- puma_access.log
- puma_error.log
To keep the existing behavior aligned with the installer, we define
the Puma log paths in config/deploy.rb
In capistrano3-puma v6.0.0, the default for 'puma_service_unit_name' changed to:
> "#{application}_puma_#{stage}"
But the installer uses the older convention:
> "puma_#{application}_#{stage}"
To ensure consistency and avoid unit name conflicts when switching between
versions or deploying older branches, we now define the variable explicitly
in config/deploy.rb:
> set :puma_service_unit_name, -> { "puma_#{fetch(:application)}_#{fetch(:stage)}" }
This commit copies the default puma.service.erb template from the
capistrano3-puma gem into lib/capistrano/templates. This allows us to
customize the generated systemd unit file during deploy.
Note that we are also removing the `:puma_conf` variable from `config/deploy.rb`,
as the new ExecStart line in the systemd template (based on capistrano3-puma 6.0.0)
does not rely on a separate Puma config file. The command now directly invokes:
ExecStart=<%= expanded_bundle_command %> exec puma -e <%= fetch(:puma_env) %>
This replaces the older format used in 5.2.0:
ExecStart=<%= expanded_bundle_command %> exec --keep-file-descriptors puma -C <%= fetch(:puma_conf) %>
which required explicitly setting the Puma config path.
In earlier versions of capistrano3-puma, the puma_bind has a default value to:
unix://.../tmp/sockets/puma.sock via set_if_empty in lib/capistrano/puma.rb.
This default was removed in 6.0.0, requiring to explicitly set :puma_bind in deploy.rb.
This caused the following runtime error during deploy:
> Failed to restart consul_puma_staging.service: Unit
> consul_puma_staging.socket has a bad unit file setting.
capistrano3-puma 6.0.0 removed the `puma:systemd:config` and
`puma:systemd:enable` tasks. This commit updates the deploy script to use
the new `puma:install` and `puma:enable` tasks instead.
We're still using YAML to serialize the legislation_annotations ranges
column. I'm not sure whether changing the serializer can have
consequences on existing data, and I'm not sure which serializer we
should provide instead. Quoting the Rails configuration guide [1]:
> Unfortunately there isn't really any suitable defaults available in
> Ruby's standard library. JSON could work as a format, but the json
> gems will cast unsupported types to strings which may lead to bugs.
[1] https://guides.rubyonrails.org/v7.1/configuring.html#config-active-record-default-column-serializer
I think this doesn't affect us because we use RSpec instead of Rails
test classes. In any case, if it ever affects us, we'll get notified
when a test fails.
As mentioned in the pull request introducing this change [1]:
> FATAL is documented in the Ruby Logger docs as being for "An
> unhandleable error that results in a program crash.", which does not
> really apply to this case since DebugExceptions is handling the error.
So we're using the new default value, which makes more sense.
[1] Pull request 48575 in https://github.com/rails/rails
Since we use a version of Loofah supporting HTML5 since db2d0bb80, the
`Rails::HTML::Sanitizer.best_supported_vendor` method will return the
HTML5 sanitizer. As mentioned in the pull request introducting this
change [1], the libxml2 maintainer wrote:
> it's still a bad idea to use a 20+ years old, unmaintained HTML 4
> parser to sanitize input for the modern web
So we're going with the new default sanitizer.
Note we aren't uncommenting the `action_text.sanitizer_vendor` option
because we don't use Action Text and so it doesn't affect us , and
uncommeting it will raise an error.
Also note we need to change one test because the new sanitizer handles
whitespace slightly differently.
[1] Pull request 48293 in https://github.com/rails/rails
Just like we mentioned in commit 001eee3d6, according to the Rails
configuration guide [1], with this format, Rails serializes cache
entries more efficiently. Most importantly:
> All formats are backward and forward compatible, meaning cache entries
> written in one format can be read when using another format. This
> behavior makes it easy to migrate between formats without invalidating
> the entire cache.
[1] https://guides.rubyonrails.org/v7.1/configuring.html#config-active-support-cache-format-version
It doesn't really affect us because we never use `return`, `break` or
`throw` inside transactions, since it would be confusing exactly because
it wouldn't be 100% clear whether the transaction is committed or not.
So we're using the new default value, which will be the only available
option in Rails 7.2 [1].
[1] Commit eccc6061f4 in https://github.com/rails/rails
This change doesn't affect us, since we don't use `after_commit`
callbacks, and, among our dependencies, AFAIK only the Devise gem uses
them, and it only defines one after_commit callback when creating a
record and another one when updating it, so we're never going to have
more than one callback being executed after a transaction is finished.
This doesn't really affect us because we don't use `before_committed`
callbacks (and neither do any of our dependencies), so we're using the
new default value.
This is done for performance reasons. Quoting the pull request
introducing this option [1]:
> A config might be overkill, but I wanted to provide an escape hatch
> for any upgraded apps that might be testing the exact value of the
> action_dispatch.parameter_filter header.
Since we don't test the exact value of action_dispatch.parameter_filter,
we can enable this option.
[1] Pull request 46452 in https://github.com/rails/rails
Before this change, every time we saved a record, the association was
validated if we had `belongs_to :something, required: true`. After this
change [1], it's only validated if the `something_id` column is nil (or
`something_type` for polymorphic associations) or if the `something_id`
attribute has changed.
The main difference is that we no longer get validation errors if the
associated record has been deleted. Doesn't affect us much, so we're
going with the new default value.
[1] Pull request 46522 in https://github.com/rails/rails
This is the new default option, and its only dangerous when deploying to
applications with multiple servers. Since this isn't our case, we can
enable it.
The new serializer can decrypt legacy messages using the `marshal`
serializer, so there's no risk of losing data when upgrading. Since we
aren't using applications with several servers, where upgrading some
servers might cause issues on the servers that aren't upgraded yet,
we're enabling the option.
[1] See comments in pull request 42846 in https://github.com/rails/rails
It doesn't really affect us (unless some of our dependencies make this
mistake) because we only use `expires_in/expires_at` once and we do it
correctly, but it might be help us detect this issue if we ever
introduce it in the future.
This option won't even exist in Rails 7.2 [1], and the possibility to
disable it was only added to guarantee safe upgrades in Rails
applications with multiple replicas [2].
Since we don't have applications with multiple replicas, where one
replica could be using Active Job 7.0 and another one could be using
Active Job 7.1 while upgrading, we can enable this options.
[1] Commit 2a761d23d2 in https://github.com/rails/rails
[2] Commit bc1f323338 in https://github.com/rails/rails
The only change between these headers and the ones sent by Rails 7.0
application is that the `"X-Download-Options" => "noopen"` is no longer
sent. Only Internet Explorer used that header, and uploading, previewing
and downloading attachments still works fine on Internet Explorer 11
after this change.
[1] Pull request 43968 in https://github.com/rails/rails
Quoting the Rails configuration guide [1]:
> applications running in :zeitwerk mode do not need require_dependency,
> so models, controllers, jobs, etc. do not need to be in $LOAD_PATH.
> Setting this to false saves Ruby from checking these directories when
> resolving require calls with relative paths, and saves Bootsnap work
> and RAM, since it does not need to build an index for them.
[1] https://guides.rubyonrails.org/v7.1/configuring.html#config-add-autoload-paths-to-load-path
We were getting a ton of deprecation warnings:
```
DEPRECATION WARNING: `Rails.application.secrets` is deprecated in favor
of `Rails.application.credentials` and will be removed in Rails 7.2
```
However, we don't plan to replace the secrets with credentials in the
foreseeable future because it would affect existing Consul Democracy
installations, so for now we're simply silencing the warnings.
This is an error we've only been able to reproduce on one specific
machine and only when using the development environment.
It looks like Rails 7.1.5.1 uses `ActiveStorage::PreviewImageJob` when
we attach a PDF. However, that raises an IntegrityError because we're
removing the metadata from PDFs. That is, the final PDF is no longer the
same PDF that was uploaded.
This issue wasn't present in the original Rails 7.1.0 release, but was
introduced in Rails 7.1.4 [1] and has already been fixed in Rails 7.2.0
[2].
So we're applying the same solution that was applied in Rails 7.2.0:
disabling automatic previews for PDFs when no variants require them by
changing a method in `ActiveStorage::Attachment`.
[1] See commit 6f729dd39 in https://github.com/rails/rails/
[2] See pull request 51351 in https://github.com/rails/rails/
We're disabling `action_controller.raise_on_missing_callback_actions`
because sometimes we include `before_action :something, only: actions`
in concerns, and we include these concerns in controllers that don't
have all these actions.
Note that Rails 7.1 logs to STDOUT by default [1]; we're re-adding the
condition `if ENV["RAILS_LOG_TO_STDOUT"].present?` because we're still
using files and we're rotating the logs to avoid running out of space.
Also note that the GraphQL controller test (which is actually a request
test, since it's got `type: :request`) that was raising an exception no
longer does it thanks to the new default value for the
`config.action_dispatch.show_exceptions` configuration option. So we're
updating the test accordingly. This option doesn't affect regular
controller tests (without the `type: :request` option), so in other
tests we're still checking exceptions.
[1] Pull request 47138 in https://github.com/rails/rails
The "foundation_rails_helper" gem is no longer maintained and is
incompatible with Rails 7.1. To avoid blocking the upgrade, we've vendored
the vendor/foundation_rails_helper/form_builder.rb as a copy of the
original FormBuilder class.
To mantain compatibility with auto_labels and button_class variables, that
are used in the original builder, we are overwriting in the foundation
form builder initializer.
The gem has been removed from the Gemfile and replaced with this vendored
fallback. This workaround is safe to remove once legacy Foundation CSS
support is dropped.
All vendored code retains the original MIT license and attribution.
