Fix accidental enable of mattermost by deprecation check
What does this MR do?
Fixes bundled Mattermost being enabled on gitlab-ctl reconfigure for users who never
configured it, introduced by the 19.0 Mattermost removal deprecation check.
Gitlab::Deprecations.remove_mattermost reads the config with plain []:
config = existing_config['mattermost']
return [] unless config.is_a?(Hash)
return [] unless config['enable'] == trueOn a real run existing_config is Gitlab['node'].normal, a Chef::Node::VividMash, where
reading a missing key creates it. And because package::config does
Gitlab[:node] = node, that object is the attributes of the node being converged — so the
read writes to them.
Which of the two reads misses is the whole story. GitlabMattermost.parse_secrets populates
the mattermost block on every reconfigure (but never enable), so mattermost is always
present and only enable is missing. config['enable'] therefore creates enable => {},
which is truthy and outranks default['mattermost']['enable'] = false, and
gitlab::default selects mattermost::enable. Vivifying mattermost itself would be
harmless — the merge falls back to the default when no enable key exists — so fixing
only the outer read changes nothing on a real install.
This affects every platform, but only fails the pipeline on arm64, where the bundled binary cannot start:
18.11.7+ee.0arm64 (job 37522486):Recipe: mattermost::disable18.11.8+ee.0arm64 (job 38095068):Recipe: mattermost::enable, thenrunit_service[mattermost] ... Expected process to exit with [0], but received '1'
Fix
Use dig, the non-mutating accessor, for both reads — matching every other helper in
this file, which already reads via dig or guards with key?:
config = existing_config.dig('mattermost') # rubocop:disable Style/SingleArgumentDig
return [] unless config.is_a?(Hash)
return [] unless config.dig('enable') == true # rubocop:disable Style/SingleArgumentDigThe rubocop:disable comments are required: that cop assumes [] and single-argument dig
are interchangeable, which is precisely the assumption that fails on a VividMash.
is_a?(Hash) and == true are kept — gitlab-ctl check_config passes a plain Hash where
mattermost can be absent, and ConfigMash fills root blocks with empty hashes, so a
truthiness test would fire on {}. Users who do enable Mattermost still get the 19.0
removal message.
Verification
gitlab-ee 18.11.8-ee.0 on Ubuntu arm64, gitlab.rb containing only external_url:
- as shipped →
mattermost::enable, run fails - outer read using
digonly →mattermost::enable, run fails (also with/opt/gitlab/embedded/nodes/*.jsonremoved first, ruling out stale cached attributes) - both reads using
dig→Recipe: mattermost::disable,runit_service[mattermost] action disable, exit 0
Verified on live ARM instance !9650 (comment 3649484460), and on Linux machine using 18.11.8 docker image and hot patching the code from MR
Issue
Mattermost recipe is enabled by default (#10036 - closed)
Related jobs (dev.gitlab.org)
- https://dev.gitlab.org/gitlab/omnibus-gitlab/-/jobs/37522486 (18.11.7, success)
- https://dev.gitlab.org/gitlab/omnibus-gitlab/-/jobs/38095068 (18.11.8, failed)